| Message ID | 20180216124455.20491-1-linus.walleij@linaro.org |
|---|---|
| State | Rejected |
| Headers | show |
| Series | i2c: algo-bit: Move debug variable | expand |
On Fri, Feb 16, 2018 at 01:44:55PM +0100, Linus Walleij wrote: > The variable "i2c_debug" is in a real weird place. If DEBUG > happens to be defined (because someone wants to do debugging) > the macro bit_dbg() is already using it some lines up in > the code, so compilation will fail. ? This just defines the bit_dbg macro which will not be used until i2c_debug gets defined, too. It compiles fine for me. Am I missing something? > After this patch, it is possible to define DEBUG and then > pass a debug level to the module with the i2c_debug > parameter, which is probably what is intended. We could still move it to reduce ifdeffery...
On Wed, Feb 21, 2018 at 9:49 AM, Wolfram Sang <wsa@the-dreams.de> wrote: > On Fri, Feb 16, 2018 at 01:44:55PM +0100, Linus Walleij wrote: >> The variable "i2c_debug" is in a real weird place. If DEBUG >> happens to be defined (because someone wants to do debugging) >> the macro bit_dbg() is already using it some lines up in >> the code, so compilation will fail. > > ? This just defines the bit_dbg macro which will not be used until > i2c_debug gets defined, too. It compiles fine for me. Am I missing > something? I guess I just did something wrong. I tested now and it works fine. >> After this patch, it is possible to define DEBUG and then >> pass a debug level to the module with the i2c_debug >> parameter, which is probably what is intended. > > We could still move it to reduce ifdeffery... Feel free to apply with copyedited commit message! Can also be dropped. Yours, Linus Walleij
diff --git a/drivers/i2c/algos/i2c-algo-bit.c b/drivers/i2c/algos/i2c-algo-bit.c index 3df0efd69ae3..47e0088ab904 100644 --- a/drivers/i2c/algos/i2c-algo-bit.c +++ b/drivers/i2c/algos/i2c-algo-bit.c @@ -29,6 +29,10 @@ /* ----- global defines ----------------------------------------------- */ #ifdef DEBUG +static int i2c_debug = 1; +module_param(i2c_debug, int, S_IRUGO | S_IWUSR); +MODULE_PARM_DESC(i2c_debug, + "debug level - 0 off; 1 normal; 2 verbose; 3 very verbose"); #define bit_dbg(level, dev, format, args...) \ do { \ if (i2c_debug >= level) \ @@ -45,13 +49,6 @@ static int bit_test; /* see if the line-setting functions work */ module_param(bit_test, int, S_IRUGO); MODULE_PARM_DESC(bit_test, "lines testing - 0 off; 1 report; 2 fail if stuck"); -#ifdef DEBUG -static int i2c_debug = 1; -module_param(i2c_debug, int, S_IRUGO | S_IWUSR); -MODULE_PARM_DESC(i2c_debug, - "debug level - 0 off; 1 normal; 2 verbose; 3 very verbose"); -#endif - /* --- setting states on the bus with the right timing: --------------- */ #define setsda(adap, val) adap->setsda(adap->data, val)
The variable "i2c_debug" is in a real weird place. If DEBUG happens to be defined (because someone wants to do debugging) the macro bit_dbg() is already using it some lines up in the code, so compilation will fail. After this patch, it is possible to define DEBUG and then pass a debug level to the module with the i2c_debug parameter, which is probably what is intended. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/i2c/algos/i2c-algo-bit.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)