linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Umang Jain <umang.jain@ideasonboard.com>,
	linux-staging@lists.linux.dev,
	linux-rpi-kernel@lists.infradead.org,
	linux-arm-kernel@lists.infradead.org,
	linux-media@vger.kernel.org,
	Stefan Wahren <stefan.wahren@i2se.com>,
	Dan Carpenter <error27@gmail.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Phil Elwell <phil@raspberrypi.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>
Subject: Re: [PATCH 6/9] staging: vc04_services: Drop vchiq_log_error() in favour of dev_dbg
Date: Thu, 23 Nov 2023 17:31:32 +0000	[thread overview]
Message-ID: <2023112343-bobbed-throbbing-2c36@gregkh> (raw)
In-Reply-To: <20231123172825.GJ16377@pendragon.ideasonboard.com>

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-11-23 17:32 UTC|newest]

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-07  9:51 [PATCH 0/9] staging: vc04_services: Smatch fixes and remove custom logging Umang Jain
2023-11-07  9:51 ` [PATCH 1/9] staging: vc04_services: vchiq_core: Log through struct vchiq_instance Umang Jain
2023-11-23 12:57   ` Greg Kroah-Hartman
2023-11-23 13:42     ` Laurent Pinchart
2023-11-23 12:58   ` Greg Kroah-Hartman
2023-11-07  9:51 ` [PATCH 2/9] staging: vc04_services: Log using pr_err() when vchiq_state is unset Umang Jain
2023-11-23 13:02   ` Greg Kroah-Hartman
2023-11-07  9:51 ` [PATCH 3/9] staging: vc04_services: bcm2835-camera: Remove redundant null check Umang Jain
2023-11-07  9:51 ` [PATCH 4/9] staging: vc04_services: Shorten helper function name Umang Jain
2023-11-07 12:32   ` Kieran Bingham
2023-11-23 13:11   ` Greg Kroah-Hartman
2023-11-07  9:51 ` [PATCH 5/9] staging: vc04_services: Do not pass NULL to vchiq_log_error() Umang Jain
2023-11-07 12:25   ` Laurent Pinchart
2023-11-07 12:31     ` Umang Jain
2023-11-07 12:38       ` Laurent Pinchart
2023-11-23 12:57       ` Greg Kroah-Hartman
2023-11-23 13:41         ` Laurent Pinchart
2023-11-23 13:55           ` Greg Kroah-Hartman
2023-11-07 12:36     ` Laurent Pinchart
2023-11-10 10:21       ` Stefan Wahren
2023-11-13 13:44         ` Umang Jain
2023-11-28  6:22       ` Umang Jain
2023-11-07  9:51 ` [PATCH 6/9] staging: vc04_services: Drop vchiq_log_error() in favour of dev_dbg Umang Jain
2023-11-23 13:02   ` Greg Kroah-Hartman
2023-11-23 13:49     ` Laurent Pinchart
2023-11-23 13:53       ` Greg Kroah-Hartman
2023-11-23 17:28         ` Laurent Pinchart
2023-11-23 17:31           ` Greg Kroah-Hartman [this message]
2023-11-23 18:00             ` Laurent Pinchart
2023-11-26 10:26               ` Greg Kroah-Hartman
2023-11-26 14:52                 ` Laurent Pinchart
2023-11-07  9:51 ` [PATCH 7/9] staging: vc04_services: Drop vchiq_log_warning() " Umang Jain
2023-11-23 13:04   ` Greg Kroah-Hartman
2023-11-07  9:51 ` [PATCH 8/9] staging: vc04_services: Drop vchiq_log_trace() " Umang Jain
2023-11-07  9:51 ` [PATCH 9/9] staging: vc04_services: Drop vchiq_log_debug() " Umang Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2023112343-bobbed-throbbing-2c36@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=error27@gmail.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=phil@raspberrypi.com \
    --cc=stefan.wahren@i2se.com \
    --cc=umang.jain@ideasonboard.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).