lkml.org 
[lkml]   [2020]   [Jul]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v18 3/3] Input: new da7280 haptic driver
On Thu, Jul 30, 2020 at 08:16:31AM +0200, Uwe Kleine-König wrote:
> [Adding vsprintf maintainers to Cc:]
>
> Hello,
>
> On Wed, Jul 29, 2020 at 10:06:53PM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 29, 2020 at 09:21:45AM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > On Tue, Jul 28, 2020 at 11:36:38PM -0700, Dmitry Torokhov wrote:
> > > > > v9:
> > > > > - Removed the header file and put the definitions into the c file.
> > > > > - Updated the pwm code and error logs with %pE
> > > >
> > > > I believe the %pE is to format an escaped buffer, you probably want to
> > > > %pe (lowercase) to print errors. I am also not quite sure if we want to
> > > > use it in cases when we have non-pointer error, or we should stick with
> > > > %d as most of the kernel does.
> > >
> > > compared with %d %pe is easier to understand as it emits "-ETIMEOUT"
> > > instead of "-110". And yes, %pE is wrong.
> >
> > While I can see that symbolic name instead of a numeric constant might
> > be appealing, I do not believe that we want fragments like this with
> > endless conversions between integer and pointer errors:
> >
> > if (haptics->const_op_mode == DA7280_PWM_MODE) {
> > haptics->pwm_dev = devm_pwm_get(dev, NULL);
> > if (IS_ERR(haptics->pwm_dev)) {
> > error = PTR_ERR(haptics->pwm_dev);
> > if (error != -EPROBE_DEFER)
> > dev_err(dev, "unable to request PWM: %pE\n",
> > ERR_PTR(error));
> > return error;
> > }
> >
> > Maybe we should introduce something like '%de' for the integer error
> > case?
>
> I suggested that some time ago with limited success, see
> https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gheiw@pathway.suse.cz/
> .
>
> > In the meantime I would prefer using %d when we have integer
> > error. We should not see these error messages anyway ;)
>
> I don't agree. Error messages are supposed to be helpful and I prefer
> some casting between error pointer and error int over emitting bare
> numbers to the kernel log. (And additionally the uglyness might help to
> convince the vsprintf guys that %de might be a good idea after all :-)

Sorry, but I do not think that I want to introduce this pointless
casting just to sway printk folks. Either we have proper way of
formatting error codes, or we do not and we stick with what was working
since forever.

Thanks.

--
Dmitry

\
 
 \ /
  Last update: 2020-07-30 08:35    [W:0.088 / U:0.984 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site