lkml.org 
[lkml]   [2020]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v9 1/4] driver core: add device probe log helper
    On Mon, Jul 13, 2020 at 5:43 PM Andrzej Hajda <a.hajda@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 omitted or implemented only
    > partially.
    > dev_err_probe helps to replace such code sequences with simple call,
    > so code:
    > if (err != -EPROBE_DEFER)
    > dev_err(dev, ...);
    > return err;
    > becomes:
    > return dev_err_probe(dev, err, ...);
    >

    FWIW,
    Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

    > Signed-off-by: Andrzej Hajda <a.hajda@samsung.com>
    > Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>
    > Reviewed-by: Mark Brown <broonie@kernel.org>
    > ---
    > drivers/base/core.c | 42 ++++++++++++++++++++++++++++++++++++++++++
    > include/linux/device.h | 3 +++
    > 2 files changed, 45 insertions(+)
    >
    > diff --git a/drivers/base/core.c b/drivers/base/core.c
    > index 67d39a90b45c..3a827c82933f 100644
    > --- a/drivers/base/core.c
    > +++ b/drivers/base/core.c
    > @@ -3953,6 +3953,48 @@ define_dev_printk_level(_dev_info, KERN_INFO);
    >
    > #endif
    >
    > +/**
    > + * dev_err_probe - 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 debug or error message depending if the error value is
    > + * -EPROBE_DEFER and propagate error upwards.
    > + * It replaces code sequence:
    > + * if (err != -EPROBE_DEFER)
    > + * dev_err(dev, ...);
    > + * else
    > + * dev_dbg(dev, ...);
    > + * return err;
    > + * with
    > + * return dev_err_probe(dev, err, ...);
    > + *
    > + * Returns @err.
    > + *
    > + */
    > +int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
    > +{
    > + struct va_format vaf;
    > + va_list args;
    > +
    > + va_start(args, fmt);
    > + vaf.fmt = fmt;
    > + vaf.va = &args;

    > + if (err != -EPROBE_DEFER)

    I would rather see positive conditional (slightly better to parse when
    read), but here I think it's more or less equal.

    > + dev_err(dev, "error %d: %pV", err, &vaf);
    > + else
    > + dev_dbg(dev, "error %d: %pV", err, &vaf);

    > + va_end(args);
    > +
    > + return err;
    > +}
    > +EXPORT_SYMBOL_GPL(dev_err_probe);
    > +
    > 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 15460a5ac024..6b2272ae9af8 100644
    > --- a/include/linux/device.h
    > +++ b/include/linux/device.h
    > @@ -964,6 +964,9 @@ void device_link_remove(void *consumer, struct device *supplier);
    > void device_links_supplier_sync_state_pause(void);
    > void device_links_supplier_sync_state_resume(void);
    >
    > +extern __printf(3, 4)
    > +int dev_err_probe(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
    >


    --
    With Best Regards,
    Andy Shevchenko

    \
     
     \ /
      Last update: 2020-07-13 16:59    [W:5.071 / U:0.240 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site