Re: [PATCH v18 3/3] Input: new da7280 haptic driver

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

Hi Uwe,

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? In the meantime I would prefer using %d when we have integer
error. We should not see these error messages anyway ;)

Thanks.

-- 
Dmitry