| Message ID | c9684a22b661fdeff9efcd5b18be01653911fc89.1588013366.git.gustavo@embeddedor.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | mtd: lpddr: Fix bad logic bug in print_drs_error | expand |
On Mon, 2020-04-27 at 14:03 -0500, Gustavo A. R. Silva wrote: > pr_notice is preferred over printk. So is coalescing formats ? diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h [] > @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) > int prog_status = (dsr & DSR_RPS) >> 8; > > if (!(dsr & DSR_AVAILABLE)) > - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); > + pr_notice("DSR.15: (0) Device not Available\n"); > if ((prog_status & 0x03) == 0x03) > - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " > + pr_notice("DSR.9,8: (11) Attempt to program invalid " > "half with 41h command\n"); pr_notice("DSR.9,8: (11) Attempt to program invalid half with 41h command\n"); etc...
On 4/27/20 14:01, Joe Perches wrote: > On Mon, 2020-04-27 at 14:03 -0500, Gustavo A. R. Silva wrote: >> pr_notice is preferred over printk. > > So is coalescing formats > > ? diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h > [] >> @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) >> int prog_status = (dsr & DSR_RPS) >> 8; >> >> if (!(dsr & DSR_AVAILABLE)) >> - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); >> + pr_notice("DSR.15: (0) Device not Available\n"); >> if ((prog_status & 0x03) == 0x03) >> - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " >> + pr_notice("DSR.9,8: (11) Attempt to program invalid " >> "half with 41h command\n"); > > pr_notice("DSR.9,8: (11) Attempt to program invalid half with 41h command\n"); > I didn't want to mess with the rest of format, because some maintainers don't like that. If Miquel is OK with that, I can fix that up, too. Thanks -- Gustavo
On Mon, 2020-04-27 at 14:10 -0500, Gustavo A. R. Silva wrote: > > On 4/27/20 14:01, Joe Perches wrote: > > On Mon, 2020-04-27 at 14:03 -0500, Gustavo A. R. Silva wrote: > > > pr_notice is preferred over printk. > > > > So is coalescing formats > > > > ? diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h > > [] > > > @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) > > > int prog_status = (dsr & DSR_RPS) >> 8; > > > > > > if (!(dsr & DSR_AVAILABLE)) > > > - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); > > > + pr_notice("DSR.15: (0) Device not Available\n"); > > > if ((prog_status & 0x03) == 0x03) > > > - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " > > > + pr_notice("DSR.9,8: (11) Attempt to program invalid " > > > "half with 41h command\n"); > > > > pr_notice("DSR.9,8: (11) Attempt to program invalid half with 41h command\n"); > > > > I didn't want to mess with the rest of format, because some maintainers > don't like that. If Miquel is OK with that, I can fix that up, too. He should. Coalescing is part of coding-style. "never break user-visible strings such as printk messages" cheers, Joe
Hi Gustavo, "Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote on Mon, 27 Apr 2020 14:10:36 -0500: > On 4/27/20 14:01, Joe Perches wrote: > > On Mon, 2020-04-27 at 14:03 -0500, Gustavo A. R. Silva wrote: > >> pr_notice is preferred over printk. > > > > So is coalescing formats > > > > ? diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h > > [] > >> @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) > >> int prog_status = (dsr & DSR_RPS) >> 8; > >> > >> if (!(dsr & DSR_AVAILABLE)) > >> - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); > >> + pr_notice("DSR.15: (0) Device not Available\n"); > >> if ((prog_status & 0x03) == 0x03) > >> - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " > >> + pr_notice("DSR.9,8: (11) Attempt to program invalid " > >> "half with 41h command\n"); > > > > pr_notice("DSR.9,8: (11) Attempt to program invalid half with 41h command\n"); > > > > I didn't want to mess with the rest of format, because some maintainers > don't like that. If Miquel is OK with that, I can fix that up, too. > > Thanks > -- > Gustavo I'm fine with it in this case, just mention it in the commit log, please. Thanks, Miquèl
Hi Joe, Joe Perches <joe@perches.com> wrote on Mon, 27 Apr 2020 12:15:02 -0700: > On Mon, 2020-04-27 at 14:10 -0500, Gustavo A. R. Silva wrote: > > > > On 4/27/20 14:01, Joe Perches wrote: > > > On Mon, 2020-04-27 at 14:03 -0500, Gustavo A. R. Silva wrote: > > > > pr_notice is preferred over printk. > > > > > > So is coalescing formats > > > > > > ? diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h > > > [] > > > > @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) > > > > int prog_status = (dsr & DSR_RPS) >> 8; > > > > > > > > if (!(dsr & DSR_AVAILABLE)) > > > > - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); > > > > + pr_notice("DSR.15: (0) Device not Available\n"); > > > > if ((prog_status & 0x03) == 0x03) > > > > - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " > > > > + pr_notice("DSR.9,8: (11) Attempt to program invalid " > > > > "half with 41h command\n"); > > > > > > pr_notice("DSR.9,8: (11) Attempt to program invalid half with 41h command\n"); > > > > > > > I didn't want to mess with the rest of format, because some maintainers > > don't like that. If Miquel is OK with that, I can fix that up, too. > > He should. Coalescing is part of coding-style. > > "never break user-visible strings such as printk messages" > > cheers, Joe > I suppose Gustavo meant that there are maintainers in the community (and I am part of it) that do not like when people fix style issues aside their own changes -specifically without writing it in commit logs. But in this situation I think s/printk(KEN_NOTICE/pr_notice(/ needs the second line to be re-aligned with the first one. While touching it, it seems reasonable to also fix this style issue and avoid keeping broken strings. Thanks, Miquèl
diff --git a/include/linux/mtd/pfow.h b/include/linux/mtd/pfow.h index c65d7a3be3c6..b1fe2bbfdb04 100644 --- a/include/linux/mtd/pfow.h +++ b/include/linux/mtd/pfow.h @@ -127,31 +127,31 @@ static inline void print_drs_error(unsigned dsr) int prog_status = (dsr & DSR_RPS) >> 8; if (!(dsr & DSR_AVAILABLE)) - printk(KERN_NOTICE"DSR.15: (0) Device not Available\n"); + pr_notice("DSR.15: (0) Device not Available\n"); if ((prog_status & 0x03) == 0x03) - printk(KERN_NOTICE"DSR.9,8: (11) Attempt to program invalid " + pr_notice("DSR.9,8: (11) Attempt to program invalid " "half with 41h command\n"); else if (prog_status & 0x02) - printk(KERN_NOTICE"DSR.9,8: (10) Object Mode Program attempt " + pr_notice("DSR.9,8: (10) Object Mode Program attempt " "in region with Control Mode data\n"); else if (prog_status & 0x01) - printk(KERN_NOTICE"DSR.9,8: (01) Program attempt in region " + pr_notice("DSR.9,8: (01) Program attempt in region " "with Object Mode data\n"); if (!(dsr & DSR_READY_STATUS)) - printk(KERN_NOTICE"DSR.7: (0) Device is Busy\n"); + pr_notice("DSR.7: (0) Device is Busy\n"); if (dsr & DSR_ESS) - printk(KERN_NOTICE"DSR.6: (1) Erase Suspended\n"); + pr_notice("DSR.6: (1) Erase Suspended\n"); if (dsr & DSR_ERASE_STATUS) - printk(KERN_NOTICE"DSR.5: (1) Erase/Blank check error\n"); + pr_notice("DSR.5: (1) Erase/Blank check error\n"); if (dsr & DSR_PROGRAM_STATUS) - printk(KERN_NOTICE"DSR.4: (1) Program Error\n"); + pr_notice("DSR.4: (1) Program Error\n"); if (dsr & DSR_VPPS) - printk(KERN_NOTICE"DSR.3: (1) Vpp low detect, operation " + pr_notice("DSR.3: (1) Vpp low detect, operation " "aborted\n"); if (dsr & DSR_PSS) - printk(KERN_NOTICE"DSR.2: (1) Program suspended\n"); + pr_notice("DSR.2: (1) Program suspended\n"); if (dsr & DSR_DPS) - printk(KERN_NOTICE"DSR.1: (1) Aborted Erase/Program attempt " + pr_notice("DSR.1: (1) Aborted Erase/Program attempt " "on locked block\n"); } #endif /* __LINUX_MTD_PFOW_H */
pr_notice is preferred over printk. Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com> --- include/linux/mtd/pfow.h | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-)