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
[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 :-)

Best regards
Uwe

--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2020-07-30 08:17    [W:0.089 / U:1.468 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site