Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Wed, 2014-03-19 at 12:46 +1100, Finn Thain wrote: > On Tue, 18 Mar 2014, Joe Perches wrote: > > > But using "if (0)" prevents the no_printk from occurring at all so there > > would be no side-effects and the format & args would still be verified > > by the compiler. > > I'd prefer this (for

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Tue, 18 Mar 2014, Joe Perches wrote: > But using "if (0)" prevents the no_printk from occurring at all so there > would be no side-effects and the format & args would still be verified > by the compiler. I'd prefer this (for symmetry and clarity): #if NDEBUG #define dprintk(flg, fmt, ...)

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Wed, 2014-03-19 at 10:14 +1100, Finn Thain wrote: > As for side-effects, chip register accesses would be affected if dprintk() > expanded to no_printk() when NDEBUG & flg == 0. > > E.g. NCR5380.c line 1213: > dprintk(NDEBUG_INTR, "scsi : unknown interrupt, BASR 0x%X, MR 0x%X, SR > 0x%x\n",

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Tue, 18 Mar 2014, Joe Perches wrote: > On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: > > > > #define dprintk(flg, fmt, ...) no_printk(fmt, ##__VA_ARGS__) > > Fine, but with a correction. > > no_printk keeps all side effects like performing any function calls made > by the

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 14:13 +0100, Geert Uytterhoeven wrote: > > no_printk keeps all side effects like > > performing any function calls made by the > > statement or accessing any volatiles. > That's true... > > Using > > do { if (0) no_printk(fmt, ##__VA_ARGS__); } while (0) > > does not have any

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Geert Uytterhoeven
Hi Joe, On Tue, Mar 18, 2014 at 2:07 PM, Joe Perches wrote: > On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: >> On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches wrote: >> > #define dprintk(flg, fmt, ...) \ >> > do { if (0) pr_debug(fmt, ##__VA_ARGS__); } while (0) >> >> Na,

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: > On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches wrote: Hi Geert. > > #define dprintk(flg, fmt, ...) \ > > do { if (0) pr_debug(fmt, ##__VA_ARGS__); } while (0) > > Na, no_printk(): > > #define dprintk(flg, fmt, ...) no_printk(fmt,

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Geert Uytterhoeven
On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches wrote: > NCR does use pr_debug for the dprintk call, but > it also doesn't verify fmt/arg matching when not > compiled in > > drivers/scsi/NCR5380.h-#if NDEBUG > drivers/scsi/NCR5380.h:#define dprintk(flg, fmt, args...) \ > drivers/scsi/NCR5380.h- do {

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 23:00 +1100, Finn Thain wrote: > On Mon, 17 Mar 2014, Joe Perches wrote: > > My preference would be to change dprintk to scsi_dbg > Can you be more specific? I gather you're not referring to the debugging > routines in include/scsi/scsi_dbg.h as they aren't equivalent. > >

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Mon, 17 Mar 2014, Joe Perches wrote: > > My preference would be to change dprintk to scsi_dbg Can you be more specific? I gather you're not referring to the debugging routines in include/scsi/scsi_dbg.h as they aren't equivalent. Is it the name "dprintk" you object to? I went looking in

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Mon, 17 Mar 2014, Joe Perches wrote: My preference would be to change dprintk to scsi_dbg Can you be more specific? I gather you're not referring to the debugging routines in include/scsi/scsi_dbg.h as they aren't equivalent. Is it the name dprintk you object to? I went looking in

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 23:00 +1100, Finn Thain wrote: On Mon, 17 Mar 2014, Joe Perches wrote: My preference would be to change dprintk to scsi_dbg Can you be more specific? I gather you're not referring to the debugging routines in include/scsi/scsi_dbg.h as they aren't equivalent. Is it

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Geert Uytterhoeven
On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches j...@perches.com wrote: NCR does use pr_debug for the dprintk call, but it also doesn't verify fmt/arg matching when not compiled in drivers/scsi/NCR5380.h-#if NDEBUG drivers/scsi/NCR5380.h:#define dprintk(flg, fmt, args...) \

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches j...@perches.com wrote: Hi Geert. #define dprintk(flg, fmt, ...) \ do { if (0) pr_debug(fmt, ##__VA_ARGS__); } while (0) Na, no_printk(): #define dprintk(flg, fmt, ...)

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Geert Uytterhoeven
Hi Joe, On Tue, Mar 18, 2014 at 2:07 PM, Joe Perches j...@perches.com wrote: On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: On Tue, Mar 18, 2014 at 1:45 PM, Joe Perches j...@perches.com wrote: #define dprintk(flg, fmt, ...) \ do { if (0) pr_debug(fmt, ##__VA_ARGS__); } while

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Tue, 2014-03-18 at 14:13 +0100, Geert Uytterhoeven wrote: no_printk keeps all side effects like performing any function calls made by the statement or accessing any volatiles. That's true... Using do { if (0) no_printk(fmt, ##__VA_ARGS__); } while (0) does not have any

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Tue, 18 Mar 2014, Joe Perches wrote: On Tue, 2014-03-18 at 13:55 +0100, Geert Uytterhoeven wrote: #define dprintk(flg, fmt, ...) no_printk(fmt, ##__VA_ARGS__) Fine, but with a correction. no_printk keeps all side effects like performing any function calls made by the statement

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Wed, 2014-03-19 at 10:14 +1100, Finn Thain wrote: As for side-effects, chip register accesses would be affected if dprintk() expanded to no_printk() when NDEBUG flg == 0. E.g. NCR5380.c line 1213: dprintk(NDEBUG_INTR, scsi : unknown interrupt, BASR 0x%X, MR 0x%X, SR 0x%x\n,

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Finn Thain
On Tue, 18 Mar 2014, Joe Perches wrote: But using if (0) prevents the no_printk from occurring at all so there would be no side-effects and the format args would still be verified by the compiler. I'd prefer this (for symmetry and clarity): #if NDEBUG #define dprintk(flg, fmt, ...) \

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-18 Thread Joe Perches
On Wed, 2014-03-19 at 12:46 +1100, Finn Thain wrote: On Tue, 18 Mar 2014, Joe Perches wrote: But using if (0) prevents the no_printk from occurring at all so there would be no side-effects and the format args would still be verified by the compiler. I'd prefer this (for symmetry and

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-17 Thread Joe Perches
On Tue, 2014-03-18 at 11:28 +1100, Finn Thain wrote: > This patch series addresses several issues with NCR5380 drivers: [] > 3. Broken debugging code. My preference would be to change dprintk to scsi_dbg Seems sensible otherwise. -- To unsubscribe from this list: send the line "unsubscribe

Re: [PATCH 00/12] scsi/NCR5380: fix debugging macros and #include structure

2014-03-17 Thread Joe Perches
On Tue, 2014-03-18 at 11:28 +1100, Finn Thain wrote: This patch series addresses several issues with NCR5380 drivers: [] 3. Broken debugging code. My preference would be to change dprintk to scsi_dbg Seems sensible otherwise. -- To unsubscribe from this list: send the line unsubscribe