[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/ |