Re: [PATCH] staging: media: omap4iss: code style - avoid macro argument precedence issues
Hi Nikolay, Thank you for the patch. On Sun, Feb 21, 2021 at 10:53:08PM +0300, Nikolay Kyx wrote: > This patch fixes the following checkpatch.pl check: > > CHECK: Macro argument 'i' may be better as '(i)' to avoid precedence issues > > in file iss_regs.h > > Signed-off-by: Nikolay Kyx Reviewed-by: Laurent Pinchart > --- > > Additionally some style warnings remain valid here and could be fixed by > another patch. > > drivers/staging/media/omap4iss/iss_regs.h | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/media/omap4iss/iss_regs.h > b/drivers/staging/media/omap4iss/iss_regs.h > index 09a7375c89ac..cfe0bb075072 100644 > --- a/drivers/staging/media/omap4iss/iss_regs.h > +++ b/drivers/staging/media/omap4iss/iss_regs.h > @@ -197,7 +197,7 @@ > #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK (0x1fff << 0) > #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_SHIFT 0 > > -#define CSI2_CTX_CTRL1(i)(0x70 + (0x20 * i)) > +#define CSI2_CTX_CTRL1(i)(0x70 + (0x20 * (i))) > #define CSI2_CTX_CTRL1_GENERIC BIT(30) > #define CSI2_CTX_CTRL1_TRANSCODE (0xf << 24) > #define CSI2_CTX_CTRL1_FEC_NUMBER_MASK (0xff << 16) > @@ -210,7 +210,7 @@ > #define CSI2_CTX_CTRL1_PING_PONG BIT(3) > #define CSI2_CTX_CTRL1_CTX_ENBIT(0) > > -#define CSI2_CTX_CTRL2(i)(0x74 + (0x20 * i)) > +#define CSI2_CTX_CTRL2(i)(0x74 + (0x20 * (i))) > #define CSI2_CTX_CTRL2_FRAME_MASK(0x << 16) > #define CSI2_CTX_CTRL2_FRAME_SHIFT 16 > #define CSI2_CTX_CTRL2_USER_DEF_MAP_SHIFT13 > @@ -222,19 +222,19 @@ > #define CSI2_CTX_CTRL2_FORMAT_MASK (0x3ff << 0) > #define CSI2_CTX_CTRL2_FORMAT_SHIFT 0 > > -#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * i)) > +#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * (i))) > #define CSI2_CTX_DAT_OFST_MASK (0xfff << 5) > > -#define CSI2_CTX_PING_ADDR(i)(0x7c + (0x20 * > i)) > +#define CSI2_CTX_PING_ADDR(i)(0x7c + (0x20 * > (i))) > #define CSI2_CTX_PING_ADDR_MASK 0xffe0 > > -#define CSI2_CTX_PONG_ADDR(i)(0x80 + (0x20 * > i)) > +#define CSI2_CTX_PONG_ADDR(i)(0x80 + (0x20 * > (i))) > #define CSI2_CTX_PONG_ADDR_MASK > CSI2_CTX_PING_ADDR_MASK > > -#define CSI2_CTX_IRQENABLE(i)(0x84 + (0x20 * > i)) > -#define CSI2_CTX_IRQSTATUS(i)(0x88 + (0x20 * > i)) > +#define CSI2_CTX_IRQENABLE(i)(0x84 + (0x20 * > (i))) > +#define CSI2_CTX_IRQSTATUS(i)(0x88 + (0x20 * > (i))) > > -#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * i)) > +#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * (i))) > #define CSI2_CTX_CTRL3_ALPHA_SHIFT 5 > #define CSI2_CTX_CTRL3_ALPHA_MASK\ > (0x3fff << CSI2_CTX_CTRL3_ALPHA_SHIFT) -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: omap4iss: code style - avoid macro argument precedence issues
Hi Nikolay, On Mon, Feb 22, 2021 at 12:21:36AM +0300, Nikolay K. wrote: > Hi Laurent, > > Thank you for the review. > I think that if we drop the unneeded parentheses here, we need to drop > them everywhere in the file for consistency, even in places checkpatch.pl That's a good point. > doesn't warn about. It'll increase patch size without actual usefullness > gain, as for me. I am very (very) novice to the kernel, > but who wants slightly more readable one-line simple macros? Let's keep your patch as-is, we can drop the unneeded parentheses in a subsequent patch if desired. -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: omap4iss: code style - avoid macro argument precedence issues
Hi Laurent, Thank you for the review. I think that if we drop the unneeded parentheses here, we need to drop them everywhere in the file for consistency, even in places checkpatch.pl doesn't warn about. It'll increase patch size without actual usefullness gain, as for me. I am very (very) novice to the kernel, but who wants slightly more readable one-line simple macros? ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: media: omap4iss: code style - avoid macro argument precedence issues
Hi Nikolay, Thank you for the patch. On Sun, Feb 21, 2021 at 10:53:08PM +0300, Nikolay Kyx wrote: > This patch fixes the following checkpatch.pl check: > > CHECK: Macro argument 'i' may be better as '(i)' to avoid precedence issues > > in file iss_regs.h > > Signed-off-by: Nikolay Kyx > --- > > Additionally some style warnings remain valid here and could be fixed by > another patch. > > drivers/staging/media/omap4iss/iss_regs.h | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/drivers/staging/media/omap4iss/iss_regs.h > b/drivers/staging/media/omap4iss/iss_regs.h > index 09a7375c89ac..cfe0bb075072 100644 > --- a/drivers/staging/media/omap4iss/iss_regs.h > +++ b/drivers/staging/media/omap4iss/iss_regs.h > @@ -197,7 +197,7 @@ > #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK (0x1fff << 0) > #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_SHIFT 0 > > -#define CSI2_CTX_CTRL1(i)(0x70 + (0x20 * i)) > +#define CSI2_CTX_CTRL1(i)(0x70 + (0x20 * (i))) This is a good change, as it fixes potential issues, but maybe we could go one step forward and drop the unneeded parentheses ? -#define CSI2_CTX_CTRL1(i) (0x70 + (0x20 * i)) +#define CSI2_CTX_CTRL1(i) (0x70 + 0x20 * (i)) What do you think ? > #define CSI2_CTX_CTRL1_GENERIC BIT(30) > #define CSI2_CTX_CTRL1_TRANSCODE (0xf << 24) > #define CSI2_CTX_CTRL1_FEC_NUMBER_MASK (0xff << 16) > @@ -210,7 +210,7 @@ > #define CSI2_CTX_CTRL1_PING_PONG BIT(3) > #define CSI2_CTX_CTRL1_CTX_ENBIT(0) > > -#define CSI2_CTX_CTRL2(i)(0x74 + (0x20 * i)) > +#define CSI2_CTX_CTRL2(i)(0x74 + (0x20 * (i))) > #define CSI2_CTX_CTRL2_FRAME_MASK(0x << 16) > #define CSI2_CTX_CTRL2_FRAME_SHIFT 16 > #define CSI2_CTX_CTRL2_USER_DEF_MAP_SHIFT13 > @@ -222,19 +222,19 @@ > #define CSI2_CTX_CTRL2_FORMAT_MASK (0x3ff << 0) > #define CSI2_CTX_CTRL2_FORMAT_SHIFT 0 > > -#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * i)) > +#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * (i))) > #define CSI2_CTX_DAT_OFST_MASK (0xfff << 5) > > -#define CSI2_CTX_PING_ADDR(i)(0x7c + (0x20 * > i)) > +#define CSI2_CTX_PING_ADDR(i)(0x7c + (0x20 * > (i))) > #define CSI2_CTX_PING_ADDR_MASK 0xffe0 > > -#define CSI2_CTX_PONG_ADDR(i)(0x80 + (0x20 * > i)) > +#define CSI2_CTX_PONG_ADDR(i)(0x80 + (0x20 * > (i))) > #define CSI2_CTX_PONG_ADDR_MASK > CSI2_CTX_PING_ADDR_MASK > > -#define CSI2_CTX_IRQENABLE(i)(0x84 + (0x20 * > i)) > -#define CSI2_CTX_IRQSTATUS(i)(0x88 + (0x20 * > i)) > +#define CSI2_CTX_IRQENABLE(i)(0x84 + (0x20 * > (i))) > +#define CSI2_CTX_IRQSTATUS(i)(0x88 + (0x20 * > (i))) > > -#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * i)) > +#define CSI2_CTX_CTRL3(i)(0x8c + (0x20 * (i))) > #define CSI2_CTX_CTRL3_ALPHA_SHIFT 5 > #define CSI2_CTX_CTRL3_ALPHA_MASK\ > (0x3fff << CSI2_CTX_CTRL3_ALPHA_SHIFT) -- Regards, Laurent Pinchart ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: media: omap4iss: code style - avoid macro argument precedence issues
This patch fixes the following checkpatch.pl check: CHECK: Macro argument 'i' may be better as '(i)' to avoid precedence issues in file iss_regs.h Signed-off-by: Nikolay Kyx --- Additionally some style warnings remain valid here and could be fixed by another patch. drivers/staging/media/omap4iss/iss_regs.h | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/staging/media/omap4iss/iss_regs.h b/drivers/staging/media/omap4iss/iss_regs.h index 09a7375c89ac..cfe0bb075072 100644 --- a/drivers/staging/media/omap4iss/iss_regs.h +++ b/drivers/staging/media/omap4iss/iss_regs.h @@ -197,7 +197,7 @@ #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_MASK(0x1fff << 0) #define CSI2_TIMING_STOP_STATE_COUNTER_IO1_SHIFT 0 -#define CSI2_CTX_CTRL1(i) (0x70 + (0x20 * i)) +#define CSI2_CTX_CTRL1(i) (0x70 + (0x20 * (i))) #define CSI2_CTX_CTRL1_GENERIC BIT(30) #define CSI2_CTX_CTRL1_TRANSCODE (0xf << 24) #define CSI2_CTX_CTRL1_FEC_NUMBER_MASK (0xff << 16) @@ -210,7 +210,7 @@ #define CSI2_CTX_CTRL1_PING_PONG BIT(3) #define CSI2_CTX_CTRL1_CTX_EN BIT(0) -#define CSI2_CTX_CTRL2(i) (0x74 + (0x20 * i)) +#define CSI2_CTX_CTRL2(i) (0x74 + (0x20 * (i))) #define CSI2_CTX_CTRL2_FRAME_MASK (0x << 16) #define CSI2_CTX_CTRL2_FRAME_SHIFT 16 #define CSI2_CTX_CTRL2_USER_DEF_MAP_SHIFT 13 @@ -222,19 +222,19 @@ #define CSI2_CTX_CTRL2_FORMAT_MASK (0x3ff << 0) #define CSI2_CTX_CTRL2_FORMAT_SHIFT0 -#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * i)) +#define CSI2_CTX_DAT_OFST(i) (0x78 + (0x20 * (i))) #define CSI2_CTX_DAT_OFST_MASK (0xfff << 5) -#define CSI2_CTX_PING_ADDR(i) (0x7c + (0x20 * i)) +#define CSI2_CTX_PING_ADDR(i) (0x7c + (0x20 * (i))) #define CSI2_CTX_PING_ADDR_MASK0xffe0 -#define CSI2_CTX_PONG_ADDR(i) (0x80 + (0x20 * i)) +#define CSI2_CTX_PONG_ADDR(i) (0x80 + (0x20 * (i))) #define CSI2_CTX_PONG_ADDR_MASK CSI2_CTX_PING_ADDR_MASK -#define CSI2_CTX_IRQENABLE(i) (0x84 + (0x20 * i)) -#define CSI2_CTX_IRQSTATUS(i) (0x88 + (0x20 * i)) +#define CSI2_CTX_IRQENABLE(i) (0x84 + (0x20 * (i))) +#define CSI2_CTX_IRQSTATUS(i) (0x88 + (0x20 * (i))) -#define CSI2_CTX_CTRL3(i) (0x8c + (0x20 * i)) +#define CSI2_CTX_CTRL3(i) (0x8c + (0x20 * (i))) #define CSI2_CTX_CTRL3_ALPHA_SHIFT 5 #define CSI2_CTX_CTRL3_ALPHA_MASK \ (0x3fff << CSI2_CTX_CTRL3_ALPHA_SHIFT) -- 2.30.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
