On Thu, Dec 20, 2018 at 11:23 AM Andrzej Hajda <a.ha...@samsung.com> wrote: > > During probe every time driver gets resource it should usually check for error > printk some message if it is not -EPROBE_DEFER and return the error. This > pattern is simple but requires adding few lines after any resource acquisition > code, as a result it is often omited or implemented only partially. > probe_err helps to replace such code sequences with simple call, so code: > if (err != -EPROBE_DEFER) > dev_err(dev, ...); > return err; > becomes: > return probe_err(dev, err, ...); > > Signed-off-by: Andrzej Hajda <a.ha...@samsung.com> > Reviewed-by: Javier Martinez Canillas <javi...@redhat.com> > Reviewed-by: Mark Brown <broo...@kernel.org> > Reviewed-by: Andy Shevchenko <andy.shevche...@gmail.com>
I'm not entirely convinced that the dev_err() log level is adequate for this purpose, but anyway it looks like this may reduce code duplication quite a bit, so Reviewed-by: Rafael J. Wysocki <rafael.j.wyso...@intel.com> > --- > v3: > - added 'extern __printf(3, 4)' decorators to probe_err, > - changed error logging format - newline at the end, > - added empty lines in probe_err around dev_err, > - added R-b by Mark and Andy. > v2: > - added error value to log message, > - fixed code style, > - added EXPORT_SYMBOL_GPL, > - Added R-B by Javier (I hope the changes did not invalidate it). > --- > drivers/base/core.c | 39 +++++++++++++++++++++++++++++++++++++++ > include/linux/device.h | 3 +++ > 2 files changed, 42 insertions(+) > > diff --git a/drivers/base/core.c b/drivers/base/core.c > index 0073b09bb99f..7f644f3c41d3 100644 > --- a/drivers/base/core.c > +++ b/drivers/base/core.c > @@ -3099,6 +3099,45 @@ define_dev_printk_level(_dev_info, KERN_INFO); > > #endif > > +/** > + * probe_err - probe error check and log helper > + * @dev: the pointer to the struct device > + * @err: error value to test > + * @fmt: printf-style format string > + * @...: arguments as specified in the format string > + * > + * This helper implements common pattern present in probe functions for error > + * checking: print message if the error is not -EPROBE_DEFER and propagate > it. > + * It replaces code sequence: > + * if (err != -EPROBE_DEFER) > + * dev_err(dev, ...); > + * return err; > + * with > + * return probe_err(dev, err, ...); > + * > + * Returns @err. > + * > + */ > +int probe_err(const struct device *dev, int err, const char *fmt, ...) > +{ > + struct va_format vaf; > + va_list args; > + > + if (err == -EPROBE_DEFER) > + return err; > + > + va_start(args, fmt); > + vaf.fmt = fmt; > + vaf.va = &args; > + > + dev_err(dev, "error %d: %pV", err, &vaf); > + > + va_end(args); > + > + return err; > +} > +EXPORT_SYMBOL_GPL(probe_err); > + > static inline bool fwnode_is_primary(struct fwnode_handle *fwnode) > { > return fwnode && !IS_ERR(fwnode->secondary); > diff --git a/include/linux/device.h b/include/linux/device.h > index 6cb4640b6160..2d3a1cc6f5da 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -1591,6 +1591,9 @@ do { > \ > WARN_ONCE(condition, "%s %s: " format, \ > dev_driver_string(dev), dev_name(dev), ## arg) > > +extern __printf(3, 4) > +int probe_err(const struct device *dev, int err, const char *fmt, ...); > + > /* Create alias, so I can be autoloaded. */ > #define MODULE_ALIAS_CHARDEV(major,minor) \ > MODULE_ALIAS("char-major-" __stringify(major) "-" __stringify(minor)) > -- > 2.17.1 >