lkml.org 
[lkml]   [2020]   [Jul]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 0/4] drm: core: Convert logging to drm_* functions.
    On Sun, Jul 12, 2020 at 12:07:45PM -0700, Joe Perches wrote:
    > On Mon, 2020-07-13 at 00:24 +0530, Suraj Upadhyay wrote:
    > > On Sat, Jul 11, 2020 at 11:16:33AM -0700, Joe Perches wrote:
    > []
    > > > Perhaps change the __drm_printk macro to not
    > > > dereference the drm argument when NULL.
    > > >
    > > > A trivial but perhaps inefficient way might be
    > > > used like:
    > > >
    > > > drm_<level>(NULL, fmt, ...)
    > []
    > > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
    > []
    > > > @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
    > > >
    > > > /* Helper for struct drm_device based logging. */
    > > > #define __drm_printk(drm, level, type, fmt, ...) \
    > > > - dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
    > > > -
    > > > + dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt, \
    > > > + ##__VA_ARGS__)
    > > >
    > > > #define drm_info(drm, fmt, ...) \
    > > > __drm_printk((drm), info,, fmt, ##__VA_ARGS__)
    > > >
    > >
    > > Hi Joe,
    > > Thanks for your input.
    > > But I don't think that this change would be a good idea as we are
    > > supposed to find or make a substitute of WARN_* macros which
    > > take a `condition` as an argument and check for its truth.
    > > And I guess passing a NULL to dev_<level> would cause a format warning.
    > >
    > > Also, the WARN_* macros are doing their job fine, and passing a NULL
    > > value everytime you want to warn about a certain condition at a
    > > particular line, doesn't seem good to me.
    > >
    > > Thus, I think that WARN_* macros should be untouched.
    >
    > So do I but the suggestion was not about WARN macros
    > only about drm_<level> macros and possibly unnecessary
    > conversions to dev_<level> when a drm_device context
    > is unavailable.
    >
    > Also, you don't have to guess, the code is there for
    > you to inspect.
    >
    > dev_<level> when a NULL is used as the first argument
    > emits "(NULL device *)" instead of dev_driver_string(dev)
    > and dev_name(dev).
    >
    > See: drivers/base/core.c::__dev_printk()

    Yes, Thanks my bad.
    But the dev_<level> usages in drm/* always have a context and doesn't need
    NULL to be passed, i.e. some of them have only a `struct device` context which
    cannot be simply converted into drm_<level> since they take a struct
    pointer with a `dev` member and not a `dev` itself.
    What we can convert is calls to dev_<level> with a drm_device context or
    with a struct pointer which has a dev member.

    And, I really want the MAINTAINERS to look into it.

    Thanks and Cheers,

    Suraj Upadhyay.

    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2020-07-13 15:58    [W:8.498 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site