All of lore.kernel.org
 help / color / mirror / Atom feed
From: Soumya Negi <soumya.negi97@gmail.com>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Martyn Welch <martyn@welchs.me.uk>,
	Manohar Vanga <manohar.vanga@gmail.com>,
	outreachy@lists.linux.dev, linux-kernel@vger.kernel.org,
	linux-staging@lists.linux.dev
Subject: Re: [PATCH 1/2] staging: vme_user: Replace printk() with pr_*(),dev_*()
Date: Thu, 19 Oct 2023 12:06:18 -0700	[thread overview]
Message-ID: <20231019190618.GA29750@Negi> (raw)
In-Reply-To: <2023101925-kudos-playful-7c5a@gregkh>

On Thu, Oct 19, 2023 at 05:34:01PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Oct 18, 2023 at 12:38:56PM -0700, Soumya Negi wrote:
> > Hi Greg,
> > 
> > On Wed, Oct 18, 2023 at 03:26:07PM +0200, Greg Kroah-Hartman wrote:
> > > On Tue, Oct 17, 2023 at 09:36:32PM -0700, Soumya Negi wrote:
> > > > vme.c uses printk() to log messages. To improve and standardize message
> > > > formatting, use logging mechanisms pr_err()/pr_warn() and
> > > > dev_err()/dev_warn() instead. Retain the printk log levels of the
> > > > messages during replacement.
> > > > 
> > > > Issue found by checkpatch.pl
> > > > 
> > > > Signed-off-by: Soumya Negi <soumya.negi97@gmail.com>
> > > > ---
> > > >  drivers/staging/vme_user/vme.c | 175 ++++++++++++++++++---------------
> > > >  1 file changed, 94 insertions(+), 81 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/vme_user/vme.c b/drivers/staging/vme_user/vme.c
> > > > index 6519a7c994a0..e8c2c1e77b7d 100644
> > > > --- a/drivers/staging/vme_user/vme.c
> > > > +++ b/drivers/staging/vme_user/vme.c
> > > > @@ -9,6 +9,8 @@
> > > >   * Copyright 2004 Motorola Inc.
> > > >   */
> > > >  
> > > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> > > 
> > > No, this is a driver, as others have pointed out, always use dev_*()
> > > calls instead.
> > 
> > Some of the pr_ fns can be dev_, but I don't think all can.
> > e.g. device NULL-check error messages
> 
> I would argue that those are pointless and can be removed and also the
> check is probably not needed either.

Got it. The pr_() in find_bridge() can't be converted to dev_ so I'll remove 
the message entirely in another patch.

I understand that the device-NULL checks should be done on the caller's side. 
Since empty devices would mean something went wrong, would it be better to
put in an assertion(..WARN_ON) when removing the check? 

> > Also, there are portions of the driver where we have no access to
> > any 'struct device' to feed into dev_.
> 
> Then you can fix that :)

Will do this. 

Regards,
Soumya

> thanks,
> 
> greg k-h

  reply	other threads:[~2023-10-19 19:06 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-18  4:36 [PATCH 0/2] staging: vme_user: Replace printk's & cleanup log messages Soumya Negi
2023-10-18  4:36 ` [PATCH 1/2] staging: vme_user: Replace printk() with pr_*(),dev_*() Soumya Negi
2023-10-18  5:47   ` Julia Lawall
2023-10-18  6:30     ` Soumya Negi
2023-10-18  7:00       ` Julia Lawall
2023-10-18 13:26   ` Greg Kroah-Hartman
2023-10-18 19:38     ` Soumya Negi
2023-10-19 15:34       ` Greg Kroah-Hartman
2023-10-19 19:06         ` Soumya Negi [this message]
2023-10-19 19:42           ` Greg Kroah-Hartman
2023-10-19 20:55             ` Soumya Negi
2023-10-18  4:36 ` [PATCH 2/2] staging: vme_user: Use __func__ instead of function name Soumya Negi
2023-10-18 11:00   ` Andi Shyti
2023-10-18 20:30     ` Soumya Negi
2023-10-18 22:13       ` Andi Shyti
2023-10-18  5:49 ` [PATCH 0/2] staging: vme_user: Replace printk's & cleanup log messages Julia Lawall
2023-10-18  6:06   ` Soumya Negi

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=20231019190618.GA29750@Negi \
    --to=soumya.negi97@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-staging@lists.linux.dev \
    --cc=manohar.vanga@gmail.com \
    --cc=martyn@welchs.me.uk \
    --cc=outreachy@lists.linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.