* Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros [not found] <cover.1518797480.git.shreeya.patel23498@gmail.com> @ 2018-02-16 17:19 ` Ezequiel Garcia 2018-02-16 17:23 ` [Outreachy kernel] " Julia Lawall ` (2 more replies) [not found] ` <1963507.Bpu1nn9g4S@blindfold> 1 sibling, 3 replies; 11+ messages in thread From: Ezequiel Garcia @ 2018-02-16 17:19 UTC (permalink / raw) To: Shreeya Patel Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd Hi Shreeya, Thanks for the contribution. On 16 February 2018 at 13:50, Shreeya Patel <shreeya.patel23498@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. > > Shreeya Patel (5): > mtd/nand: Replace printk with pr_warn > mtd/nand: Replace printk with pr_notice > mtd/nand: Replace printk with pr_err > mtd/nand: Replace printk with pr_info > mtd/nand: Replace printk with pr_debug > This is a non-usual split. We usually split patches per-driver, instead of grouping all the pr_{} of a given type together. Can you re-do the series? Grouping the changes per-driver, i.e. all the pr_{} changes in cafe_nand.c together, and so on. > drivers/mtd/nand/ams-delta.c | 4 +-- > drivers/mtd/nand/cafe_nand.c | 4 +-- > drivers/mtd/nand/cs553x_nand.c | 8 ++--- > drivers/mtd/nand/diskonchip.c | 76 +++++++++++++++++++++------------------- > drivers/mtd/nand/fsl_elbc_nand.c | 4 +-- > drivers/mtd/nand/fsl_ifc_nand.c | 2 +- > drivers/mtd/nand/mxc_nand.c | 2 +- > drivers/mtd/nand/nand_bch.c | 12 +++---- > drivers/mtd/nand/nandsim.c | 10 +++--- > drivers/mtd/nand/r852.c | 2 +- > drivers/mtd/nand/r852.h | 6 ++-- > drivers/mtd/nand/sh_flctl.c | 2 +- > drivers/mtd/nand/sm_common.c | 5 ++- > 13 files changed, 70 insertions(+), 67 deletions(-) > > -- > 2.7.4 > -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:19 ` [PATCH NAND 0/5] Replace printk statements with pr_*macros Ezequiel Garcia @ 2018-02-16 17:23 ` Julia Lawall 2018-02-16 17:26 ` Ezequiel Garcia 2018-02-16 17:35 ` Shreeya Patel 2018-02-16 17:45 ` Boris Brezillon 2 siblings, 1 reply; 11+ messages in thread From: Julia Lawall @ 2018-02-16 17:23 UTC (permalink / raw) To: Ezequiel Garcia Cc: Shreeya Patel, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd [-- Attachment #1: Type: text/plain, Size: 2717 bytes --] 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@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? julia > > > > > Shreeya Patel (5): > > mtd/nand: Replace printk with pr_warn > > mtd/nand: Replace printk with pr_notice > > mtd/nand: Replace printk with pr_err > > mtd/nand: Replace printk with pr_info > > mtd/nand: Replace printk with pr_debug > > > > This is a non-usual split. We usually split patches per-driver, > instead of grouping all the pr_{} of a given type together. > > Can you re-do the series? Grouping the changes per-driver, > i.e. all the pr_{} changes in cafe_nand.c together, and so on. > > > drivers/mtd/nand/ams-delta.c | 4 +-- > > drivers/mtd/nand/cafe_nand.c | 4 +-- > > drivers/mtd/nand/cs553x_nand.c | 8 ++--- > > drivers/mtd/nand/diskonchip.c | 76 +++++++++++++++++++++------------------- > > drivers/mtd/nand/fsl_elbc_nand.c | 4 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 2 +- > > drivers/mtd/nand/mxc_nand.c | 2 +- > > drivers/mtd/nand/nand_bch.c | 12 +++---- > > drivers/mtd/nand/nandsim.c | 10 +++--- > > drivers/mtd/nand/r852.c | 2 +- > > drivers/mtd/nand/r852.h | 6 ++-- > > drivers/mtd/nand/sh_flctl.c | 2 +- > > drivers/mtd/nand/sm_common.c | 5 ++- > > 13 files changed, 70 insertions(+), 67 deletions(-) > > > > -- > > 2.7.4 > > > > > > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar > > -- > You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. > To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. > To post to this group, send email to outreachy-kernel@googlegroups.com. > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAAEAJfAg_hei2PF37_oX2tf%2BMv43ye-nEOt%2BKRVFerp1Jfzn2Q%40mail.gmail.com. > For more options, visit https://groups.google.com/d/optout. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:23 ` [Outreachy kernel] " Julia Lawall @ 2018-02-16 17:26 ` Ezequiel Garcia 2018-02-16 17:48 ` Boris Brezillon 0 siblings, 1 reply; 11+ messages in thread From: Ezequiel Garcia @ 2018-02-16 17:26 UTC (permalink / raw) To: Julia Lawall Cc: Shreeya Patel, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@lip6.fr> 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@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. 2. Richard is not too opposed to the idea. Then, yes. -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:26 ` Ezequiel Garcia @ 2018-02-16 17:48 ` Boris Brezillon 2018-02-16 17:59 ` Shreeya Patel [not found] ` <1518976870.2784.4.camel@gmail.com> 0 siblings, 2 replies; 11+ messages in thread From: Boris Brezillon @ 2018-02-16 17:48 UTC (permalink / raw) To: Ezequiel Garcia Cc: Julia Lawall, Shreeya Patel, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd On Fri, 16 Feb 2018 14:26:56 -0300 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@lip6.fr> 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@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. > 2. Richard is not too opposed to the idea. > > Then, yes. -- Boris Brezillon, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:48 ` Boris Brezillon @ 2018-02-16 17:59 ` Shreeya Patel [not found] ` <1518976870.2784.4.camel@gmail.com> 1 sibling, 0 replies; 11+ messages in thread From: Shreeya Patel @ 2018-02-16 17:59 UTC (permalink / raw) To: Boris Brezillon, Ezequiel Garcia Cc: Julia Lawall, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd On Fri, 2018-02-16 at 18:48 +0100, Boris Brezillon wrote: > On Fri, 16 Feb 2018 14:26:56 -0300 > Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > > > > > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@lip6.fr> > > 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@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. This information is quite useful, thanks. I'll see that what will be the best thing to use here. > > > > > 2. Richard is not too opposed to the idea. > > > > Then, yes. > > ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1518976870.2784.4.camel@gmail.com>]
[parent not found: <20180218191347.76f4a287@bbrezillon>]
[parent not found: <1518978365.2969.6.camel@gmail.com>]
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros [not found] ` <1518978365.2969.6.camel@gmail.com> @ 2018-02-18 20:09 ` Boris Brezillon 2018-02-18 20:20 ` Shreeya Patel 2018-02-18 20:30 ` Shreeya Patel 0 siblings, 2 replies; 11+ messages in thread From: Boris Brezillon @ 2018-02-18 20:09 UTC (permalink / raw) To: Shreeya Patel Cc: Julia Lawall, Shreeya Patel, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd, Ezequiel Garcia 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@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@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@vanguardiasur.com.ar> wrote: > > > > > > > > > > > > > > > > > > > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@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@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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-18 20:09 ` Boris Brezillon @ 2018-02-18 20:20 ` Shreeya Patel 2018-02-18 20:30 ` Shreeya Patel 1 sibling, 0 replies; 11+ messages in thread From: Shreeya Patel @ 2018-02-18 20:20 UTC (permalink / raw) To: Boris Brezillon Cc: Julia Lawall, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd, Ezequiel Garcia 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 <shreeya.patel23498@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@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@vanguardiasur.com.ar> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@li > > > > > > 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 > > > > > > > > <shreeya.patel23498@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. 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 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [Outreachy kernel] Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-18 20:09 ` Boris Brezillon 2018-02-18 20:20 ` Shreeya Patel @ 2018-02-18 20:30 ` Shreeya Patel 1 sibling, 0 replies; 11+ messages in thread From: Shreeya Patel @ 2018-02-18 20:30 UTC (permalink / raw) To: Boris Brezillon Cc: Julia Lawall, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd, Ezequiel Garcia 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 <shreeya.patel23498@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@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@vanguardiasur.com.ar> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > On 16 February 2018 at 14:23, Julia Lawall <julia.lawall@li > > > > > > 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 > > > > > > > > <shreeya.patel23498@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. 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 > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:19 ` [PATCH NAND 0/5] Replace printk statements with pr_*macros Ezequiel Garcia 2018-02-16 17:23 ` [Outreachy kernel] " Julia Lawall @ 2018-02-16 17:35 ` Shreeya Patel 2018-02-16 17:45 ` Boris Brezillon 2 siblings, 0 replies; 11+ messages in thread From: Shreeya Patel @ 2018-02-16 17:35 UTC (permalink / raw) To: Ezequiel Garcia Cc: Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd On Fri, 2018-02-16 at 14:19 -0300, Ezequiel Garcia wrote: > Hi Shreeya, > > Thanks for the contribution. > > On 16 February 2018 at 13:50, Shreeya Patel > <shreeya.patel23498@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. Ok. I'll take care of this. > > > > > > > Shreeya Patel (5): > > mtd/nand: Replace printk with pr_warn > > mtd/nand: Replace printk with pr_notice > > mtd/nand: Replace printk with pr_err > > mtd/nand: Replace printk with pr_info > > mtd/nand: Replace printk with pr_debug > > > This is a non-usual split. We usually split patches per-driver, > instead of grouping all the pr_{} of a given type together. > > Can you re-do the series? Grouping the changes per-driver, > i.e. all the pr_{} changes in cafe_nand.c together, and so on. Yes, I'll do it again. :) Also, I wanted to ask you that there are still printk statements present in the driver which are just printing the message i.e. they don't include any log levels. So should I leave them as it is? Thanks for your reviews > > > > > drivers/mtd/nand/ams-delta.c | 4 +-- > > drivers/mtd/nand/cafe_nand.c | 4 +-- > > drivers/mtd/nand/cs553x_nand.c | 8 ++--- > > drivers/mtd/nand/diskonchip.c | 76 +++++++++++++++++++++------- > > ------------ > > drivers/mtd/nand/fsl_elbc_nand.c | 4 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 2 +- > > drivers/mtd/nand/mxc_nand.c | 2 +- > > drivers/mtd/nand/nand_bch.c | 12 +++---- > > drivers/mtd/nand/nandsim.c | 10 +++--- > > drivers/mtd/nand/r852.c | 2 +- > > drivers/mtd/nand/r852.h | 6 ++-- > > drivers/mtd/nand/sh_flctl.c | 2 +- > > drivers/mtd/nand/sm_common.c | 5 ++- > > 13 files changed, 70 insertions(+), 67 deletions(-) > > > > -- > > 2.7.4 > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros 2018-02-16 17:19 ` [PATCH NAND 0/5] Replace printk statements with pr_*macros Ezequiel Garcia 2018-02-16 17:23 ` [Outreachy kernel] " Julia Lawall 2018-02-16 17:35 ` Shreeya Patel @ 2018-02-16 17:45 ` Boris Brezillon 2 siblings, 0 replies; 11+ messages in thread From: Boris Brezillon @ 2018-02-16 17:45 UTC (permalink / raw) To: Ezequiel Garcia Cc: Shreeya Patel, Boris Brezillon, Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd On Fri, 16 Feb 2018 14:19:41 -0300 Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > Hi Shreeya, > > Thanks for the contribution. > > On 16 February 2018 at 13:50, Shreeya Patel > <shreeya.patel23498@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. > > > > > Shreeya Patel (5): > > mtd/nand: Replace printk with pr_warn > > mtd/nand: Replace printk with pr_notice > > mtd/nand: Replace printk with pr_err > > mtd/nand: Replace printk with pr_info > > mtd/nand: Replace printk with pr_debug Nitpick: prefix should be "mtd: nand: ". > > > > This is a non-usual split. We usually split patches per-driver, > instead of grouping all the pr_{} of a given type together. I agree. > > Can you re-do the series? Grouping the changes per-driver, > i.e. all the pr_{} changes in cafe_nand.c together, and so on. Actually, for such simple/automatic changes you can even group things in a single patch. I'm fine either way. > > > drivers/mtd/nand/ams-delta.c | 4 +-- > > drivers/mtd/nand/cafe_nand.c | 4 +-- > > drivers/mtd/nand/cs553x_nand.c | 8 ++--- > > drivers/mtd/nand/diskonchip.c | 76 +++++++++++++++++++++------------------- > > drivers/mtd/nand/fsl_elbc_nand.c | 4 +-- > > drivers/mtd/nand/fsl_ifc_nand.c | 2 +- > > drivers/mtd/nand/mxc_nand.c | 2 +- > > drivers/mtd/nand/nand_bch.c | 12 +++---- > > drivers/mtd/nand/nandsim.c | 10 +++--- > > drivers/mtd/nand/r852.c | 2 +- > > drivers/mtd/nand/r852.h | 6 ++-- > > drivers/mtd/nand/sh_flctl.c | 2 +- > > drivers/mtd/nand/sm_common.c | 5 ++- > > 13 files changed, 70 insertions(+), 67 deletions(-) > > > > -- > > 2.7.4 > > > > > -- Boris Brezillon, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com ^ permalink raw reply [flat|nested] 11+ messages in thread
[parent not found: <1963507.Bpu1nn9g4S@blindfold>]
[parent not found: <CAAEAJfD6CbWOfrYOY4QDW0tte-yOebcZTGjUDQhR8uE+fsOs3w@mail.gmail.com>]
* Re: [PATCH NAND 0/5] Replace printk statements with pr_*macros [not found] ` <CAAEAJfD6CbWOfrYOY4QDW0tte-yOebcZTGjUDQhR8uE+fsOs3w@mail.gmail.com> @ 2018-02-16 17:25 ` Ezequiel Garcia 0 siblings, 0 replies; 11+ messages in thread From: Ezequiel Garcia @ 2018-02-16 17:25 UTC (permalink / raw) To: Richard Weinberger Cc: Shreeya Patel, Boris Brezillon, David Woodhouse, Brian Norris, Marek Vasut, Cyrille Pitchen, outreachy-kernel, linux-mtd (Ccing MTD) On 16 February 2018 at 14:24, Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote: > Hey Richard, > > On 16 February 2018 at 14:19, Richard Weinberger <richard@nod.at> wrote: >> Am Freitag, 16. Februar 2018, 17:50:09 CET schrieb Shreeya Patel: >>> 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. >> >> Beside of that, how does it improve the code? >> Don't get me wrong, pr_* is the way to go for new code, but I don't think it >> is worth "fixing" in existing code and make working with git blame more >> painful. >> > > Shreeya's intention is to work on [1], migrating drivers to exec_op. > As a first task, before being accepted, she is required to submit > cleanup patches (or small tasks). > > Since I couldn't come up with small tasks for MTD, I suggested > starting with printk cleaning. So this was my idea. > > Printk to pr_{} or dev_{} is not only aesthetical, it's also a consolidation > change. Centralizing prints allows for other improvements. > > See for instance Wolfram's work [2, 3]. > > In any case, does it hurt git-blame so much? It only affects > the lines where the print is performed. > > BTW, should you have ideas for small cleanup tasks in MTD, > let us know. > > [1] https://www.outreachy.org/communities/cfp/linux-kernel/project/migrate-nand-driver-to-new-exec_op-framework/ > [2] https://elinux.org/Refactor_kernel_strings > [3] https://elinux.org/Refactor_kernel_strings > > Thanks, > -- > Ezequiel García, VanguardiaSur > www.vanguardiasur.com.ar -- Ezequiel García, VanguardiaSur www.vanguardiasur.com.ar ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-02-18 20:31 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <cover.1518797480.git.shreeya.patel23498@gmail.com>
2018-02-16 17:19 ` [PATCH NAND 0/5] Replace printk statements with pr_*macros Ezequiel Garcia
2018-02-16 17:23 ` [Outreachy kernel] " Julia Lawall
2018-02-16 17:26 ` Ezequiel Garcia
2018-02-16 17:48 ` Boris Brezillon
2018-02-16 17:59 ` Shreeya Patel
[not found] ` <1518976870.2784.4.camel@gmail.com>
[not found] ` <20180218191347.76f4a287@bbrezillon>
[not found] ` <1518978365.2969.6.camel@gmail.com>
2018-02-18 20:09 ` Boris Brezillon
2018-02-18 20:20 ` Shreeya Patel
2018-02-18 20:30 ` Shreeya Patel
2018-02-16 17:35 ` Shreeya Patel
2018-02-16 17:45 ` Boris Brezillon
[not found] ` <1963507.Bpu1nn9g4S@blindfold>
[not found] ` <CAAEAJfD6CbWOfrYOY4QDW0tte-yOebcZTGjUDQhR8uE+fsOs3w@mail.gmail.com>
2018-02-16 17:25 ` Ezequiel Garcia
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).