Re: [PATCH 6/9] staging: vc04_services: Drop vchiq_log_error() in favour of dev_dbg

Greg Kroah-Hartman
2023-11-23T17:31:34Z

On Thu, Nov 23, 2023 at 07:28:25PM +0200, Laurent Pinchart wrote:
> On Thu, Nov 23, 2023 at 01:53:42PM +0000, Greg Kroah-Hartman wrote:
> > On Thu, Nov 23, 2023 at 03:49:41PM +0200, Laurent Pinchart wrote:
> > > On Thu, Nov 23, 2023 at 01:02:45PM +0000, Greg Kroah-Hartman wrote:
> > > > On Tue, Nov 07, 2023 at 04:51:53AM -0500, Umang Jain wrote:
> > > > > Drop vchiq_log_error() macro which wraps dev_dbg(). Introduce the usage
> > > > > of dev_dbg() directly.
> > > > >
> > > > > Add a new enum vchiq_log_type and log_type() helper to faciliate the
> > > > > type of logging in dev_dbg(). This will help to determine the type of
> > > > > logging("error", "warning", "debug", "trace") to dynamic debug.
> > > > >
> > > > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> > > > > ---
> > > > > .../interface/vchiq_arm/vchiq_arm.c | 54 ++++----
> > > > > .../interface/vchiq_arm/vchiq_connected.c | 6 +-
> > > > > .../interface/vchiq_arm/vchiq_core.c | 126 ++++++++++--------
> > > > > .../interface/vchiq_arm/vchiq_core.h | 23 +++-
> > > > > .../interface/vchiq_arm/vchiq_dev.c | 47 ++++---
> > > > > 5 files changed, 143 insertions(+), 113 deletions(-)
> > > > >
> > > > > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > > > index 9fb3e240d9de..2cb2a6503058 100644
> > > > > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > > > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > > > > @@ -696,8 +696,8 @@ int vchiq_initialise(struct vchiq_instance **instance_out)
> > > > >
> > > > > instance = kzalloc(sizeof(*instance), GFP_KERNEL);
> > > > > if (!instance) {
> > > > > - vchiq_log_error(state->dev, VCHIQ_CORE,
> > > > > - "%s: error allocating vchiq instance\n", __func__);
> > > > > + dev_dbg(state->dev, "%s: %s: %s: error allocating vchiq instance\n",
> > > > > + log_cat(VCHIQ_CORE), log_type(ERROR), __func__);
> > > >
> > > > All dev_dbg() calls have __func__ in them automatically, you never need
> > > > to duplicate it again as that's redundant :(
> > >
> > > Oh ? I didn't know that, and can't find it in the code. I may be missing
> > > something though. Are you referring to the +f flag for dynamic debug
> > > entries ? It won't work if dynamic debug isn't enabled though, but maybe
> > > we don't care about that ?
> >
> > Yes, the "f" flag is what controls this, and if dynamic debug isn't
> > enabled, you don't get any message here and we don't care about it :)
>
> You do if you #define DEBUG, that's one of the three options for
> dev_dbg() (dynamic debug and no_printk() being the other two). Maybe
> __func__ should be added to the dev_printk() version of dev_dbg() to
> have a consistent behaviour.

Drivers should NOT be defining DEBUG for anything in the tree, just use
the normal interfaces, as no one will be selecting debug options from
Kconfig. DEBUG is really only good for out-of-tree work.

thanks,

greg k-h