From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl0-x242.google.com ([2607:f8b0:400e:c01::242]) by bombadil.infradead.org with esmtps (Exim 4.89 #1 (Red Hat Linux)) id 1enVSP-0005Ng-6o for linux-mtd@lists.infradead.org; Sun, 18 Feb 2018 20:21:10 +0000 Received: by mail-pl0-x242.google.com with SMTP id 31so4542854ple.9 for ; Sun, 18 Feb 2018 12:20:58 -0800 (PST) Message-ID: <1518985251.22603.6.camel@gmail.com> Subject: Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros From: Shreeya Patel To: Boris Brezillon Cc: Julia Lawall , Boris Brezillon , Richard Weinberger , David Woodhouse , Brian Norris , Marek Vasut , Cyrille Pitchen , outreachy-kernel , linux-mtd@lists.infradead.org, Ezequiel Garcia Date: Mon, 19 Feb 2018 01:50:51 +0530 In-Reply-To: <20180218210950.30fe9310@bbrezillon> References: <20180216184840.096e534c@bbrezillon> <1518976870.2784.4.camel@gmail.com> <20180218191347.76f4a287@bbrezillon> <1518978365.2969.6.camel@gmail.com> <20180218210950.30fe9310@bbrezillon> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit List-Id: Linux MTD discussion mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Sun, 2018-02-18 at 21:09 +0100, Boris Brezillon wrote: > 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. I'll keep this in mind. > > > On Sun, 18 Feb 2018 23:56:05 +0530 > Shreeya Patel wrote: > > > > > On Sun, 2018-02-18 at 19:13 +0100, Boris Brezillon wrote: > > > > > > On Sun, 18 Feb 2018 23:31:10 +0530 > > > Shreeya Patel 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 wrote: > > > > >      > > > > > > > > > > > > > > > > > > > > > > > > On 16 February 2018 at 14:23, Julia Lawall > > > > > p6.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 > > > > > > > > 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_(). 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_(). 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_(/pr_(/ 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_/dev_(&pdev->dev, / is likely to > cause build failures. But I guess you compile-tested the changes > you're > about to submit. Yes, it wasn't directly accessible at some places. Also, I was unable to compile ams-delta.c file. I am working on it right now as I will have to cross compile for arm. Thanks > > Regards, > > Boris > >