These patches make changes to clean up style issues
as identified by checkpatch
Mitali Borkar (2):
media: zoran: add spaces around '<<'
media: zoran: replace bit shifts by BIT() macro
drivers/staging/media/zoran/zr36057.h | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
--
2.30.2
Added spaces around '<<' operator to improve readability and meet linux
kernel coding style.
Reported by checkpatch
Signed-off-by: Mitali Borkar <[email protected]>
---
drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
index 71b651add35a..a2a75fd9f535 100644
--- a/drivers/staging/media/zoran/zr36057.h
+++ b/drivers/staging/media/zoran/zr36057.h
@@ -30,13 +30,13 @@
#define ZR36057_VFESPFR_HOR_DCM 14
#define ZR36057_VFESPFR_VER_DCM 8
#define ZR36057_VFESPFR_DISP_MODE 6
-#define ZR36057_VFESPFR_YUV422 (0<<3)
-#define ZR36057_VFESPFR_RGB888 (1<<3)
-#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_YUV422 (0 << 3)
+#define ZR36057_VFESPFR_RGB888 (1 << 3)
+#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_VDTR 0x00c /* Video Display "Top" Register */
--
2.30.2
Added #include <linux/bitops.h> 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 <[email protected]>
---
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 <linux/bitops.h>
+
/* 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_MODE 6
#define ZR36057_VFESPFR_YUV422 (0 << 3)
-#define ZR36057_VFESPFR_RGB888 (1 << 3)
+#define ZR36057_VFESPFR_RGB888 BIT(3)
#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_ENDIAN BIT(0)
#define ZR36057_VDTR 0x00c /* Video Display "Top" Register */
--
2.30.2
On Fri, 9 Apr 2021, Mitali Borkar wrote:
> Added #include <linux/bitops.h> 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 <[email protected]>
> ---
> 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 <linux/bitops.h>
> +
> /* 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_MODE 6
> #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_ENDIAN BIT(0)
>
> #define ZR36057_VDTR 0x00c /* 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com.
>
On Fri, 9 Apr 2021, Mitali Borkar wrote:
> Added spaces around '<<' operator to improve readability and meet linux
> kernel coding style.
> Reported by checkpatch
>
> Signed-off-by: Mitali Borkar <[email protected]>
> ---
> drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> index 71b651add35a..a2a75fd9f535 100644
> --- a/drivers/staging/media/zoran/zr36057.h
> +++ b/drivers/staging/media/zoran/zr36057.h
> @@ -30,13 +30,13 @@
> #define ZR36057_VFESPFR_HOR_DCM 14
> #define ZR36057_VFESPFR_VER_DCM 8
> #define ZR36057_VFESPFR_DISP_MODE 6
> -#define ZR36057_VFESPFR_YUV422 (0<<3)
> -#define ZR36057_VFESPFR_RGB888 (1<<3)
> -#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_YUV422 (0 << 3)
> +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> +#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)
Are these all aligned in the actual file?
julia
> #define ZR36057_VDTR 0x00c /* 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/8e8ac690d97478f7cbb9b91d46ef7a95e4444e5f.1617912177.git.mitaliborkar810%40gmail.com.
>
On Thu, Apr 08, 2021 at 11:16:41PM +0200, Julia Lawall wrote:
>
>
> On Fri, 9 Apr 2021, Mitali Borkar wrote:
>
> > Added spaces around '<<' operator to improve readability and meet linux
> > kernel coding style.
> > Reported by checkpatch
> >
> > Signed-off-by: Mitali Borkar <[email protected]>
> > ---
> > drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > index 71b651add35a..a2a75fd9f535 100644
> > --- a/drivers/staging/media/zoran/zr36057.h
> > +++ b/drivers/staging/media/zoran/zr36057.h
> > @@ -30,13 +30,13 @@
> > #define ZR36057_VFESPFR_HOR_DCM 14
> > #define ZR36057_VFESPFR_VER_DCM 8
> > #define ZR36057_VFESPFR_DISP_MODE 6
> > -#define ZR36057_VFESPFR_YUV422 (0<<3)
> > -#define ZR36057_VFESPFR_RGB888 (1<<3)
> > -#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_YUV422 (0 << 3)
> > +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> > +#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)
>
> Are these all aligned in the actual file?
>
No Ma'am, they were not aligned in the actual file.
> julia
>
> > #define ZR36057_VDTR 0x00c /* 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 [email protected].
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/8e8ac690d97478f7cbb9b91d46ef7a95e4444e5f.1617912177.git.mitaliborkar810%40gmail.com.
> >
On Thu, Apr 08, 2021 at 11:15:07PM +0200, Julia Lawall wrote:
>
>
> On Fri, 9 Apr 2021, Mitali Borkar wrote:
>
> > Added #include <linux/bitops.h> 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 <[email protected]>
> > ---
> > 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 <linux/bitops.h>
> > +
> > /* 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_MODE 6
> > #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_ENDIAN BIT(0)
> >
> > #define ZR36057_VDTR 0x00c /* 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 [email protected].
> > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com.
> >
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 <linux/bitops.h> 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 <[email protected]>
> > > ---
> > > 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 <linux/bitops.h>
> > > +
> > > /* 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_MODE 6
> > > #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_ENDIAN BIT(0)
> > >
> > > #define ZR36057_VDTR 0x00c /* 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 [email protected].
> > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com.
> > >
>
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 <linux/bitops.h> 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 <[email protected]>
> > > > ---
> > > > 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 <linux/bitops.h>
> > > > +
> > > > /* 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_MODE 6
> > > > #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_ENDIAN BIT(0)
> > > >
> > > > #define ZR36057_VDTR 0x00c /* 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 [email protected].
> > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com.
> > > >
> >
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 <linux/bitops.h> 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 <[email protected]>
> > > > > ---
> > > > > 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 <linux/bitops.h>
> > > > > +
> > > > > /* 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_MODE 6
> > > > > #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_ENDIAN BIT(0)
> > > > >
> > > > > #define ZR36057_VDTR 0x00c /* 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 [email protected].
> > > > > To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/ac8ec2b70ac2cc7c541c05a1d9a8db1fe79df793.1617912177.git.mitaliborkar810%40gmail.com.
> > > > >
> > >
>
Hi Mitali,
On 08/04/2021 22:38, Mitali Borkar wrote:
> Added spaces around '<<' operator to improve readability and meet linux
> kernel coding style.
> Reported by checkpatch
>
> Signed-off-by: Mitali Borkar <[email protected]>
> ---
> drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> index 71b651add35a..a2a75fd9f535 100644
> --- a/drivers/staging/media/zoran/zr36057.h
> +++ b/drivers/staging/media/zoran/zr36057.h
> @@ -30,13 +30,13 @@
> #define ZR36057_VFESPFR_HOR_DCM 14
> #define ZR36057_VFESPFR_VER_DCM 8
> #define ZR36057_VFESPFR_DISP_MODE 6
> -#define ZR36057_VFESPFR_YUV422 (0<<3)
> -#define ZR36057_VFESPFR_RGB888 (1<<3)
> -#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_YUV422 (0 << 3)
> +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> +#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_VDTR 0x00c /* Video Display "Top" Register */
>
>
I looked at that header and it is very messy.
Can you make two new patches? The first aligns every nicely, e.g. this:
#define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
#define ZR36057_VFEHCR_HS_POL BIT(30)
#define ZR36057_VFEHCR_H_START 10
#define ZR36057_VFEHCR_H_END 0
#define ZR36057_VFEHCR_HMASK 0x3ff
should become:
/* Video Front End, Horizontal Configuration Register */
#define ZR36057_VFEHCR 0x000
#define ZR36057_VFEHCR_HS_POL BIT(30)
#define ZR36057_VFEHCR_H_START 10
#define ZR36057_VFEHCR_H_END 0
#define ZR36057_VFEHCR_HMASK 0x3ff
Same for all the other register blocks. Use tabs to do the alignment
instead of spaces, as is currently the case.
The second patch can replace the (0<<3) etc. to BIT(0).
That would be a nice cleanup of this rather messy header.
Thanks!
Hans
On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
> Hi Mitali,
>
> On 08/04/2021 22:38, Mitali Borkar wrote:
> > Added spaces around '<<' operator to improve readability and meet linux
> > kernel coding style.
> > Reported by checkpatch
> >
> > Signed-off-by: Mitali Borkar <[email protected]>
> > ---
> > drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > index 71b651add35a..a2a75fd9f535 100644
> > --- a/drivers/staging/media/zoran/zr36057.h
> > +++ b/drivers/staging/media/zoran/zr36057.h
> > @@ -30,13 +30,13 @@
> > #define ZR36057_VFESPFR_HOR_DCM 14
> > #define ZR36057_VFESPFR_VER_DCM 8
> > #define ZR36057_VFESPFR_DISP_MODE 6
> > -#define ZR36057_VFESPFR_YUV422 (0<<3)
> > -#define ZR36057_VFESPFR_RGB888 (1<<3)
> > -#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_YUV422 (0 << 3)
> > +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> > +#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_VDTR 0x00c /* Video Display "Top" Register */
> >
> >
>
> I looked at that header and it is very messy.
>
> Can you make two new patches? The first aligns every nicely, e.g. this:
>
> #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
> #define ZR36057_VFEHCR_HS_POL BIT(30)
> #define ZR36057_VFEHCR_H_START 10
> #define ZR36057_VFEHCR_H_END 0
> #define ZR36057_VFEHCR_HMASK 0x3ff
>
> should become:
>
> /* Video Front End, Horizontal Configuration Register */
> #define ZR36057_VFEHCR 0x000
> #define ZR36057_VFEHCR_HS_POL BIT(30)
> #define ZR36057_VFEHCR_H_START 10
> #define ZR36057_VFEHCR_H_END 0
> #define ZR36057_VFEHCR_HMASK 0x3ff
>
> Same for all the other register blocks. Use tabs to do the alignment
> instead of spaces, as is currently the case.
>
Okay Sir, will do this.
> The second patch can replace the (0<<3) etc. to BIT(0).
>
Sir may I know how to write (2<<3) in BIT() form? Like I am still
learning and not sure how to convert this. Should it be BIT(2)? But this
is only possible for (1<<nr), PLease help me out.
Thanks.
> That would be a nice cleanup of this rather messy header.
>
> Thanks!
>
> Hans
On Fri, 9 Apr 2021, Mitali Borkar wrote:
> On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
> > Hi Mitali,
> >
> > On 08/04/2021 22:38, Mitali Borkar wrote:
> > > Added spaces around '<<' operator to improve readability and meet linux
> > > kernel coding style.
> > > Reported by checkpatch
> > >
> > > Signed-off-by: Mitali Borkar <[email protected]>
> > > ---
> > > drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > > index 71b651add35a..a2a75fd9f535 100644
> > > --- a/drivers/staging/media/zoran/zr36057.h
> > > +++ b/drivers/staging/media/zoran/zr36057.h
> > > @@ -30,13 +30,13 @@
> > > #define ZR36057_VFESPFR_HOR_DCM 14
> > > #define ZR36057_VFESPFR_VER_DCM 8
> > > #define ZR36057_VFESPFR_DISP_MODE 6
> > > -#define ZR36057_VFESPFR_YUV422 (0<<3)
> > > -#define ZR36057_VFESPFR_RGB888 (1<<3)
> > > -#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_YUV422 (0 << 3)
> > > +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> > > +#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_VDTR 0x00c /* Video Display "Top" Register */
> > >
> > >
> >
> > I looked at that header and it is very messy.
> >
> > Can you make two new patches? The first aligns every nicely, e.g. this:
> >
> > #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
> > #define ZR36057_VFEHCR_HS_POL BIT(30)
> > #define ZR36057_VFEHCR_H_START 10
> > #define ZR36057_VFEHCR_H_END 0
> > #define ZR36057_VFEHCR_HMASK 0x3ff
> >
> > should become:
> >
> > /* Video Front End, Horizontal Configuration Register */
> > #define ZR36057_VFEHCR 0x000
> > #define ZR36057_VFEHCR_HS_POL BIT(30)
> > #define ZR36057_VFEHCR_H_START 10
> > #define ZR36057_VFEHCR_H_END 0
> > #define ZR36057_VFEHCR_HMASK 0x3ff
> >
> > Same for all the other register blocks. Use tabs to do the alignment
> > instead of spaces, as is currently the case.
> >
> Okay Sir, will do this.
>
> > The second patch can replace the (0<<3) etc. to BIT(0).
> >
> Sir may I know how to write (2<<3) in BIT() form? Like I am still
> learning and not sure how to convert this. Should it be BIT(2)? But this
> is only possible for (1<<nr), PLease help me out.
I think that you are correct. BIT is not usable her.
julia
>
> Thanks.
>
> > That would be a nice cleanup of this rather messy header.
> >
> > Thanks!
> >
> > Hans
>
> --
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/YHBukmdxSiRc%2BK6I%40kali.
>
On 09/04/2021 17:11, Mitali Borkar wrote:
> On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
>> Hi Mitali,
>>
>> On 08/04/2021 22:38, Mitali Borkar wrote:
>>> Added spaces around '<<' operator to improve readability and meet linux
>>> kernel coding style.
>>> Reported by checkpatch
>>>
>>> Signed-off-by: Mitali Borkar <[email protected]>
>>> ---
>>> drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
>>> 1 file changed, 7 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
>>> index 71b651add35a..a2a75fd9f535 100644
>>> --- a/drivers/staging/media/zoran/zr36057.h
>>> +++ b/drivers/staging/media/zoran/zr36057.h
>>> @@ -30,13 +30,13 @@
>>> #define ZR36057_VFESPFR_HOR_DCM 14
>>> #define ZR36057_VFESPFR_VER_DCM 8
>>> #define ZR36057_VFESPFR_DISP_MODE 6
>>> -#define ZR36057_VFESPFR_YUV422 (0<<3)
>>> -#define ZR36057_VFESPFR_RGB888 (1<<3)
>>> -#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_YUV422 (0 << 3)
>>> +#define ZR36057_VFESPFR_RGB888 (1 << 3)
>>> +#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_VDTR 0x00c /* Video Display "Top" Register */
>>>
>>>
>>
>> I looked at that header and it is very messy.
>>
>> Can you make two new patches? The first aligns every nicely, e.g. this:
>>
>> #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
>> #define ZR36057_VFEHCR_HS_POL BIT(30)
>> #define ZR36057_VFEHCR_H_START 10
>> #define ZR36057_VFEHCR_H_END 0
>> #define ZR36057_VFEHCR_HMASK 0x3ff
>>
>> should become:
>>
>> /* Video Front End, Horizontal Configuration Register */
>> #define ZR36057_VFEHCR 0x000
>> #define ZR36057_VFEHCR_HS_POL BIT(30)
>> #define ZR36057_VFEHCR_H_START 10
>> #define ZR36057_VFEHCR_H_END 0
>> #define ZR36057_VFEHCR_HMASK 0x3ff
>>
>> Same for all the other register blocks. Use tabs to do the alignment
>> instead of spaces, as is currently the case.
>>
> Okay Sir, will do this.
>
>> The second patch can replace the (0<<3) etc. to BIT(0).
>>
> Sir may I know how to write (2<<3) in BIT() form? Like I am still
> learning and not sure how to convert this. Should it be BIT(2)? But this
> is only possible for (1<<nr), PLease help me out.
The << notation is kept for that. So this would be kept as-is:
#define ZR36057_VFESPFR_YUV422 (0 << 3)
#define ZR36057_VFESPFR_RGB888 (1 << 3)
#define ZR36057_VFESPFR_RGB565 (2 << 3)
#define ZR36057_VFESPFR_RGB555 (3 << 3)
BIT() is only used for single bits.
Regards,
Hans
>
> Thanks.
>
>> That would be a nice cleanup of this rather messy header.
>>
>> Thanks!
>>
>> Hans
On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
> Hi Mitali,
>
> On 08/04/2021 22:38, Mitali Borkar wrote:
> > Added spaces around '<<' operator to improve readability and meet linux
> > kernel coding style.
> > Reported by checkpatch
> >
> > Signed-off-by: Mitali Borkar <[email protected]>
> > ---
> > drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > index 71b651add35a..a2a75fd9f535 100644
> > --- a/drivers/staging/media/zoran/zr36057.h
> > +++ b/drivers/staging/media/zoran/zr36057.h
> > @@ -30,13 +30,13 @@
> > #define ZR36057_VFESPFR_HOR_DCM 14
> > #define ZR36057_VFESPFR_VER_DCM 8
> > #define ZR36057_VFESPFR_DISP_MODE 6
> > -#define ZR36057_VFESPFR_YUV422 (0<<3)
> > -#define ZR36057_VFESPFR_RGB888 (1<<3)
> > -#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_YUV422 (0 << 3)
> > +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> > +#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_VDTR 0x00c /* Video Display "Top" Register */
> >
> >
>
> I looked at that header and it is very messy.
>
> Can you make two new patches? The first aligns every nicely, e.g. this:
>
> #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
> #define ZR36057_VFEHCR_HS_POL BIT(30)
> #define ZR36057_VFEHCR_H_START 10
> #define ZR36057_VFEHCR_H_END 0
> #define ZR36057_VFEHCR_HMASK 0x3ff
>
> should become:
>
> /* Video Front End, Horizontal Configuration Register */
> #define ZR36057_VFEHCR 0x000
> #define ZR36057_VFEHCR_HS_POL BIT(30)
> #define ZR36057_VFEHCR_H_START 10
> #define ZR36057_VFEHCR_H_END 0
> #define ZR36057_VFEHCR_HMASK 0x3ff
>
> Same for all the other register blocks. Use tabs to do the alignment
> instead of spaces, as is currently the case.
>
> The second patch can replace the (0<<3) etc. to BIT(0).
>
Then I guess only one new patch would be needed for proper alignment, am
i right? I have to rename it as v2 or should send as a completely new
patch?
> That would be a nice cleanup of this rather messy header.
>
> Thanks!
>
> Hans
On Fri, 9 Apr 2021, Mitali Borkar wrote:
> On Fri, Apr 09, 2021 at 09:23:22AM +0200, Hans Verkuil wrote:
> > Hi Mitali,
> >
> > On 08/04/2021 22:38, Mitali Borkar wrote:
> > > Added spaces around '<<' operator to improve readability and meet linux
> > > kernel coding style.
> > > Reported by checkpatch
> > >
> > > Signed-off-by: Mitali Borkar <[email protected]>
> > > ---
> > > drivers/staging/media/zoran/zr36057.h | 14 +++++++-------
> > > 1 file changed, 7 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > > index 71b651add35a..a2a75fd9f535 100644
> > > --- a/drivers/staging/media/zoran/zr36057.h
> > > +++ b/drivers/staging/media/zoran/zr36057.h
> > > @@ -30,13 +30,13 @@
> > > #define ZR36057_VFESPFR_HOR_DCM 14
> > > #define ZR36057_VFESPFR_VER_DCM 8
> > > #define ZR36057_VFESPFR_DISP_MODE 6
> > > -#define ZR36057_VFESPFR_YUV422 (0<<3)
> > > -#define ZR36057_VFESPFR_RGB888 (1<<3)
> > > -#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_YUV422 (0 << 3)
> > > +#define ZR36057_VFESPFR_RGB888 (1 << 3)
> > > +#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_VDTR 0x00c /* Video Display "Top" Register */
> > >
> > >
> >
> > I looked at that header and it is very messy.
> >
> > Can you make two new patches? The first aligns every nicely, e.g. this:
> >
> > #define ZR36057_VFEHCR 0x000 /* Video Front End, Horizontal Configuration Register */
> > #define ZR36057_VFEHCR_HS_POL BIT(30)
> > #define ZR36057_VFEHCR_H_START 10
> > #define ZR36057_VFEHCR_H_END 0
> > #define ZR36057_VFEHCR_HMASK 0x3ff
> >
> > should become:
> >
> > /* Video Front End, Horizontal Configuration Register */
> > #define ZR36057_VFEHCR 0x000
> > #define ZR36057_VFEHCR_HS_POL BIT(30)
> > #define ZR36057_VFEHCR_H_START 10
> > #define ZR36057_VFEHCR_H_END 0
> > #define ZR36057_VFEHCR_HMASK 0x3ff
> >
> > Same for all the other register blocks. Use tabs to do the alignment
> > instead of spaces, as is currently the case.
> >
> > The second patch can replace the (0<<3) etc. to BIT(0).
> >
> Then I guess only one new patch would be needed for proper alignment, am
> i right? I have to rename it as v2 or should send as a completely new
> patch?
v2 might reduce confusion.
julia
> > That would be a nice cleanup of this rather messy header.
> >
> > Thanks!
> >
> > Hans
>
> --
> 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 [email protected].
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/YHCFsNZVGfjjyHDi%40kali.
>