Re: [Outreachy kernel] [PATCH 2/2] media: zoran: replace bit shifts by BIT() macro
On Fri, 9 Apr 2021, Mitali Borkar wrote: > On Fri, Apr 09, 2021 at 12:10:06AM +0200, Julia Lawall wrote: > > > > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > > > On Thu, Apr 08, 2021 at 11:15:07PM +0200, Julia Lawall wrote: > > > > > > > > > > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > > > > > > > Added #include and replaced bit shifts by BIT() > > > > > macro. > > > > > This BIT() macro from linux/bitops.h is used to define > > > > > ZR36057_VFESPFR_* bitmasks. > > > > > Use of macro is better and neater. It maintains consistency. > > > > > Reported by checkpatch. > > > > > > > > > > Signed-off-by: Mitali Borkar > > > > > --- > > > > > drivers/staging/media/zoran/zr36057.h | 10 ++ > > > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > > > > > diff --git a/drivers/staging/media/zoran/zr36057.h > > > > > b/drivers/staging/media/zoran/zr36057.h > > > > > index a2a75fd9f535..93075459f910 100644 > > > > > --- a/drivers/staging/media/zoran/zr36057.h > > > > > +++ b/drivers/staging/media/zoran/zr36057.h > > > > > @@ -8,6 +8,8 @@ > > > > > #ifndef _ZR36057_H_ > > > > > #define _ZR36057_H_ > > > > > > > > > > +#include > > > > > + > > > > > /* Zoran ZR36057 registers */ > > > > > > > > > > #define ZR36057_VFEHCR 0x000/* Video Front End, > > > > > Horizontal Configuration Register */ > > > > > @@ -31,12 +33,12 @@ > > > > > #define ZR36057_VFESPFR_VER_DCM 8 > > > > > #define ZR36057_VFESPFR_DISP_MODE6 > > > > > #define ZR36057_VFESPFR_YUV422 (0 << 3) > > > > > -#define ZR36057_VFESPFR_RGB888 (1 << 3) > > > > > +#define ZR36057_VFESPFR_RGB888 BIT(3) > > > > > > > > Uniformity is generally considered to be more important than using BIT. > > > > Having only a few constants defined using BIT is a bit strange. > > > > > > > Okay Ma'am. Can you please tell me on how to proceed now? I am not sure > > > how to proceed. > > > > I think that this code should just be left as is. > > > > Checkpatch makes suggestions. Its suggestions are not always appropriate > > for the context. > > > Okay Ma'am. This means I should not do any changes now. Do I need to do > something in this patch now? Or move onto next one? You can move on. julia > > > julia > > > > > > > > > julia > > > > > > > > > #define ZR36057_VFESPFR_RGB565 (2 << 3) > > > > > #define ZR36057_VFESPFR_RGB555 (3 << 3) > > > > > -#define ZR36057_VFESPFR_ERR_DIF (1 << 2) > > > > > -#define ZR36057_VFESPFR_PACK24 (1 << 1) > > > > > -#define ZR36057_VFESPFR_LITTLE_ENDIAN(1 << 0) > > > > > +#define ZR36057_VFESPFR_ERR_DIF BIT(2) > > > > > +#define ZR36057_VFESPFR_PACK24 BIT(1) > > > > > +#define ZR36057_VFESPFR_LITTLE_ENDIANBIT(0) > > > > > > > > > > #define ZR36057_VDTR0x00c/* Video Display "Top" > > > > > Register */ > > > > > > > > > > -- > > > > > 2.30.2 > > > > > > > > > > -- > > > > > 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+unsubscr...@googlegroups.com. > > > > > To view this discussion on the web visit > > > > > https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com. > > > > > > > > >
Re: [Outreachy kernel] [PATCH 2/2] media: zoran: replace bit shifts by BIT() macro
On Fri, Apr 09, 2021 at 12:10:06AM +0200, Julia Lawall wrote: > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > On Thu, Apr 08, 2021 at 11:15:07PM +0200, Julia Lawall wrote: > > > > > > > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > > > > > Added #include and replaced bit shifts by BIT() macro. > > > > This BIT() macro from linux/bitops.h is used to define > > > > ZR36057_VFESPFR_* bitmasks. > > > > Use of macro is better and neater. It maintains consistency. > > > > Reported by checkpatch. > > > > > > > > Signed-off-by: Mitali Borkar > > > > --- > > > > drivers/staging/media/zoran/zr36057.h | 10 ++ > > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/staging/media/zoran/zr36057.h > > > > b/drivers/staging/media/zoran/zr36057.h > > > > index a2a75fd9f535..93075459f910 100644 > > > > --- a/drivers/staging/media/zoran/zr36057.h > > > > +++ b/drivers/staging/media/zoran/zr36057.h > > > > @@ -8,6 +8,8 @@ > > > > #ifndef _ZR36057_H_ > > > > #define _ZR36057_H_ > > > > > > > > +#include > > > > + > > > > /* Zoran ZR36057 registers */ > > > > > > > > #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal > > > > Configuration Register */ > > > > @@ -31,12 +33,12 @@ > > > > #define ZR36057_VFESPFR_VER_DCM 8 > > > > #define ZR36057_VFESPFR_DISP_MODE6 > > > > #define ZR36057_VFESPFR_YUV422 (0 << 3) > > > > -#define ZR36057_VFESPFR_RGB888 (1 << 3) > > > > +#define ZR36057_VFESPFR_RGB888 BIT(3) > > > > > > Uniformity is generally considered to be more important than using BIT. > > > Having only a few constants defined using BIT is a bit strange. > > > > > Okay Ma'am. Can you please tell me on how to proceed now? I am not sure > > how to proceed. > > I think that this code should just be left as is. > > Checkpatch makes suggestions. Its suggestions are not always appropriate > for the context. > Okay Ma'am. This means I should not do any changes now. Do I need to do something in this patch now? Or move onto next one? > julia > > > > > > julia > > > > > > > #define ZR36057_VFESPFR_RGB565 (2 << 3) > > > > #define ZR36057_VFESPFR_RGB555 (3 << 3) > > > > -#define ZR36057_VFESPFR_ERR_DIF (1 << 2) > > > > -#define ZR36057_VFESPFR_PACK24 (1 << 1) > > > > -#define ZR36057_VFESPFR_LITTLE_ENDIAN(1 << 0) > > > > +#define ZR36057_VFESPFR_ERR_DIF BIT(2) > > > > +#define ZR36057_VFESPFR_PACK24 BIT(1) > > > > +#define ZR36057_VFESPFR_LITTLE_ENDIANBIT(0) > > > > > > > > #define ZR36057_VDTR0x00c /* Video Display "Top" Register > > > > */ > > > > > > > > -- > > > > 2.30.2 > > > > > > > > -- > > > > 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+unsubscr...@googlegroups.com. > > > > To view this discussion on the web visit > > > > https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com. > > > > > >
Re: [Outreachy kernel] [PATCH 2/2] media: zoran: replace bit shifts by BIT() macro
On Fri, 9 Apr 2021, Mitali Borkar wrote: > On Thu, Apr 08, 2021 at 11:15:07PM +0200, Julia Lawall wrote: > > > > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > > > Added #include and replaced bit shifts by BIT() macro. > > > This BIT() macro from linux/bitops.h is used to define ZR36057_VFESPFR_* > > > bitmasks. > > > Use of macro is better and neater. It maintains consistency. > > > Reported by checkpatch. > > > > > > Signed-off-by: Mitali Borkar > > > --- > > > drivers/staging/media/zoran/zr36057.h | 10 ++ > > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > > > diff --git a/drivers/staging/media/zoran/zr36057.h > > > b/drivers/staging/media/zoran/zr36057.h > > > index a2a75fd9f535..93075459f910 100644 > > > --- a/drivers/staging/media/zoran/zr36057.h > > > +++ b/drivers/staging/media/zoran/zr36057.h > > > @@ -8,6 +8,8 @@ > > > #ifndef _ZR36057_H_ > > > #define _ZR36057_H_ > > > > > > +#include > > > + > > > /* Zoran ZR36057 registers */ > > > > > > #define ZR36057_VFEHCR 0x000/* Video Front End, Horizontal > > > Configuration Register */ > > > @@ -31,12 +33,12 @@ > > > #define ZR36057_VFESPFR_VER_DCM 8 > > > #define ZR36057_VFESPFR_DISP_MODE6 > > > #define ZR36057_VFESPFR_YUV422 (0 << 3) > > > -#define ZR36057_VFESPFR_RGB888 (1 << 3) > > > +#define ZR36057_VFESPFR_RGB888 BIT(3) > > > > Uniformity is generally considered to be more important than using BIT. > > Having only a few constants defined using BIT is a bit strange. > > > Okay Ma'am. Can you please tell me on how to proceed now? I am not sure > how to proceed. I think that this code should just be left as is. Checkpatch makes suggestions. Its suggestions are not always appropriate for the context. julia > > > julia > > > > > #define ZR36057_VFESPFR_RGB565 (2 << 3) > > > #define ZR36057_VFESPFR_RGB555 (3 << 3) > > > -#define ZR36057_VFESPFR_ERR_DIF (1 << 2) > > > -#define ZR36057_VFESPFR_PACK24 (1 << 1) > > > -#define ZR36057_VFESPFR_LITTLE_ENDIAN(1 << 0) > > > +#define ZR36057_VFESPFR_ERR_DIF BIT(2) > > > +#define ZR36057_VFESPFR_PACK24 BIT(1) > > > +#define ZR36057_VFESPFR_LITTLE_ENDIANBIT(0) > > > > > > #define ZR36057_VDTR0x00c/* Video Display "Top" Register > > > */ > > > > > > -- > > > 2.30.2 > > > > > > -- > > > 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+unsubscr...@googlegroups.com. > > > To view this discussion on the web visit > > > https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com. > > > >
Re: [Outreachy kernel] [PATCH 2/2] media: zoran: replace bit shifts by BIT() macro
On Thu, Apr 08, 2021 at 11:15:07PM +0200, Julia Lawall wrote: > > > On Fri, 9 Apr 2021, Mitali Borkar wrote: > > > Added #include and replaced bit shifts by BIT() macro. > > This BIT() macro from linux/bitops.h is used to define ZR36057_VFESPFR_* > > bitmasks. > > Use of macro is better and neater. It maintains consistency. > > Reported by checkpatch. > > > > Signed-off-by: Mitali Borkar > > --- > > drivers/staging/media/zoran/zr36057.h | 10 ++ > > 1 file changed, 6 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/staging/media/zoran/zr36057.h > > b/drivers/staging/media/zoran/zr36057.h > > index a2a75fd9f535..93075459f910 100644 > > --- a/drivers/staging/media/zoran/zr36057.h > > +++ b/drivers/staging/media/zoran/zr36057.h > > @@ -8,6 +8,8 @@ > > #ifndef _ZR36057_H_ > > #define _ZR36057_H_ > > > > +#include > > + > > /* Zoran ZR36057 registers */ > > > > #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal > > Configuration Register */ > > @@ -31,12 +33,12 @@ > > #define ZR36057_VFESPFR_VER_DCM 8 > > #define ZR36057_VFESPFR_DISP_MODE6 > > #define ZR36057_VFESPFR_YUV422 (0 << 3) > > -#define ZR36057_VFESPFR_RGB888 (1 << 3) > > +#define ZR36057_VFESPFR_RGB888 BIT(3) > > Uniformity is generally considered to be more important than using BIT. > Having only a few constants defined using BIT is a bit strange. > Okay Ma'am. Can you please tell me on how to proceed now? I am not sure how to proceed. > julia > > > #define ZR36057_VFESPFR_RGB565 (2 << 3) > > #define ZR36057_VFESPFR_RGB555 (3 << 3) > > -#define ZR36057_VFESPFR_ERR_DIF (1 << 2) > > -#define ZR36057_VFESPFR_PACK24 (1 << 1) > > -#define ZR36057_VFESPFR_LITTLE_ENDIAN(1 << 0) > > +#define ZR36057_VFESPFR_ERR_DIF BIT(2) > > +#define ZR36057_VFESPFR_PACK24 BIT(1) > > +#define ZR36057_VFESPFR_LITTLE_ENDIANBIT(0) > > > > #define ZR36057_VDTR0x00c /* Video Display "Top" Register > > */ > > > > -- > > 2.30.2 > > > > -- > > 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+unsubscr...@googlegroups.com. > > To view this discussion on the web visit > > https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com. > >
Re: [Outreachy kernel] [PATCH 2/2] media: zoran: replace bit shifts by BIT() macro
On Fri, 9 Apr 2021, Mitali Borkar wrote: > Added #include and replaced bit shifts by BIT() macro. > This BIT() macro from linux/bitops.h is used to define ZR36057_VFESPFR_* > bitmasks. > Use of macro is better and neater. It maintains consistency. > Reported by checkpatch. > > Signed-off-by: Mitali Borkar > --- > drivers/staging/media/zoran/zr36057.h | 10 ++ > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/drivers/staging/media/zoran/zr36057.h > b/drivers/staging/media/zoran/zr36057.h > index a2a75fd9f535..93075459f910 100644 > --- a/drivers/staging/media/zoran/zr36057.h > +++ b/drivers/staging/media/zoran/zr36057.h > @@ -8,6 +8,8 @@ > #ifndef _ZR36057_H_ > #define _ZR36057_H_ > > +#include > + > /* Zoran ZR36057 registers */ > > #define ZR36057_VFEHCR 0x000/* Video Front End, Horizontal > Configuration Register */ > @@ -31,12 +33,12 @@ > #define ZR36057_VFESPFR_VER_DCM 8 > #define ZR36057_VFESPFR_DISP_MODE6 > #define ZR36057_VFESPFR_YUV422 (0 << 3) > -#define ZR36057_VFESPFR_RGB888 (1 << 3) > +#define ZR36057_VFESPFR_RGB888 BIT(3) Uniformity is generally considered to be more important than using BIT. Having only a few constants defined using BIT is a bit strange. julia > #define ZR36057_VFESPFR_RGB565 (2 << 3) > #define ZR36057_VFESPFR_RGB555 (3 << 3) > -#define ZR36057_VFESPFR_ERR_DIF (1 << 2) > -#define ZR36057_VFESPFR_PACK24 (1 << 1) > -#define ZR36057_VFESPFR_LITTLE_ENDIAN(1 << 0) > +#define ZR36057_VFESPFR_ERR_DIF BIT(2) > +#define ZR36057_VFESPFR_PACK24 BIT(1) > +#define ZR36057_VFESPFR_LITTLE_ENDIANBIT(0) > > #define ZR36057_VDTR0x00c/* Video Display "Top" Register > */ > > -- > 2.30.2 > > -- > 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+unsubscr...@googlegroups.com. > To view this discussion on the web visit > https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com. >
