[Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros

[Boris Brezillon]

Hi Shreeya,

Please try to keep everyone in the loop when you reply to an email. All
discussions should happen publicly to keep everyone aware of the
progress and let other developers/maintainers take part to the
discussion if they have something to add.


On Sun, 18 Feb 2018 23:56:05 +0530
Shreeya Patel <shreeya.patel23498 at gmail.com> wrote:

> On Sun, 2018-02-18 at 19:13 +0100, Boris Brezillon wrote:
> > On Sun, 18 Feb 2018 23:31:10 +0530
> > Shreeya Patel <shreeya.patel23498 at gmail.com> wrote:
> >   
> > > 
> > > On Fri, 2018-02-16 at 18:48 +0100, Boris Brezillon wrote:
> > > 
> > > Hello Sir,
> > >   
> > > > 
> > > > On Fri, 16 Feb 2018 14:26:56 -0300
> > > > Ezequiel Garcia <ezequiel at vanguardiasur.com.ar> wrote:
> > > >     
> > > > > 
> > > > > 
> > > > > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall at lip6.f  
> > > > > r>  
> > > > > wrote:    
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On Fri, 16 Feb 2018, Ezequiel Garcia wrote:
> > > > > >      
> > > > > > > 
> > > > > > > 
> > > > > > > Hi Shreeya,
> > > > > > > 
> > > > > > > Thanks for the contribution.
> > > > > > > 
> > > > > > > On 16 February 2018 at 13:50, Shreeya Patel
> > > > > > > <shreeya.patel23498 at gmail.com> wrote:      
> > > > > > > > 
> > > > > > > > 
> > > > > > > > This patchset removes all the log levels i.e. KERN_WARN,
> > > > > > > > KERN_NOTICE, KERN_ERR, KERN_INFO, KERN_DEBUG used in the
> > > > > > > > printk
> > > > > > > > statements and replaces the printk statements with
> > > > > > > > appropriate
> > > > > > > > pr_*macros.
> > > > > > > > According to the kernel coding style, pr_*macro is the
> > > > > > > > preferred
> > > > > > > > way to print the message.
> > > > > > > >      
> > > > > > > So, two things to begin with.
> > > > > > > 
> > > > > > > First of all, despite this contribution being part of
> > > > > > > outreachy,
> > > > > > > I believe you can include mailing lists in your case.
> > > > > > > 
> > > > > > > In other words, don't use the "nol" option in
> > > > > > > get_maintainer
> > > > > > > script and Cc the MTD mailing list: linux-mtd at
> > > > > > > lists.infradead.org.      
> > > > > > Shouldn't the dev_* functions also be usable?
> > > > > >      
> > > > > Provided that:
> > > > > 1. it's applicable, i.e. if in the context of a device.    
> > > > Yep, be careful with that. The MTD/NAND subsystem initializes
> > > > mtd->dev.name quite late, so it's not safe to use &mtd->dev with
> > > > dev_<loglevel>(). Note that you can use the NAND controller pdev-  
> > > > >dev   
> > > > if
> > > > available.    
> > > I would like to ask here that what will be the benefit or good
> > > cause if
> > > I use pdev->dev?
> > > How is it different from others?  
> > Well, pdev->dev is guaranteed to be properly initialized when you
> > pass
> > it to dev_<loglevel>(). This is not the case with mtd->dev which is
> > initialized in mtd_device_register(), but the NAND controller driver
> > usually does some operations on the NAND device before reaching this
> > point (reading the ID, reading the bad block markers to determine
> > which
> > blocks are bad, ...).
> >   
> Thanks for making me understand.
> 
> > Anyway, I think it's better if you first do the
> > s/printk(KERN_<LOGLEVEL>(/pr_<loglevel>(/ replacement. Replacing
> > pr_xxx
> > by dev_xxx is something we can do afterwards.  
> 
> I've created one patch where I have replaced printk
> with dev_* and used pdev->dev.

Okay. pdev->dev is not always directly accessible so doing
s/printk(KERN_<LOGLEVEL>/dev_<loglevel>(&pdev->dev, / is likely to
cause build failures. But I guess you compile-tested the changes you're
about to submit.

Regards,

Boris


-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com