linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4 0/2] staging: media: zoran: Cleanup patchset for style issues in zr36057.h
@ 2021-04-13 19:50 Mitali Borkar
  2021-04-13 19:50 ` [PATCH v4 1/2] staging: media: zoran: add spaces around '<<' operator Mitali Borkar
  2021-04-13 19:50 ` [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code Mitali Borkar
  0 siblings, 2 replies; 6+ messages in thread
From: Mitali Borkar @ 2021-04-13 19:50 UTC (permalink / raw)
  To: clabbe, mchehab, gregkh
  Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel, mitali_s

Changes from v3:- Rebased these patches and made changes against
mainline code.

Changes from v2:-
[PATCH 1/2]:- No changes.
[PATCH 2/2]:- Aligned the code using tabs.

Changes from v1:- 
[PATCH 1/2]:- No changes.
[PATCH 2/2]:- Aligned the code using tabs and readjusted the
comments line.

Mitali Borkar (2):
  staging: media: zoran: add spaces around '<<' operator
  staging: media: zoran: add BIT() macro and align code

 drivers/staging/media/zoran/zr36057.h | 312 ++++++++++++++------------
 1 file changed, 172 insertions(+), 140 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/2] staging: media: zoran: add spaces around '<<' operator
  2021-04-13 19:50 [PATCH v4 0/2] staging: media: zoran: Cleanup patchset for style issues in zr36057.h Mitali Borkar
@ 2021-04-13 19:50 ` Mitali Borkar
  2021-04-13 19:50 ` [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code Mitali Borkar
  1 sibling, 0 replies; 6+ messages in thread
From: Mitali Borkar @ 2021-04-13 19:50 UTC (permalink / raw)
  To: clabbe, mchehab, gregkh
  Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel, mitali_s

Added spaces around '<<' operator to improve readability and meet linux
kernel coding style.
Reported by checkpatch

Signed-off-by: Mitali Borkar <mitaliborkar810@gmail.com>
---

Changes from v3:- No changes.
Changes from v2:- No changes.
Changes from v1:- No changes.

 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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code
  2021-04-13 19:50 [PATCH v4 0/2] staging: media: zoran: Cleanup patchset for style issues in zr36057.h Mitali Borkar
  2021-04-13 19:50 ` [PATCH v4 1/2] staging: media: zoran: add spaces around '<<' operator Mitali Borkar
@ 2021-04-13 19:50 ` Mitali Borkar
  2021-04-14 10:38   ` zzam
  1 sibling, 1 reply; 6+ messages in thread
From: Mitali Borkar @ 2021-04-13 19:50 UTC (permalink / raw)
  To: clabbe, mchehab, gregkh
  Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel, mitali_s

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.
Removed comments from the same line and added them to new line above the
blocks, aligned everything properly by using tabs to make code neater
and improve readability.
Reported by checkpatch.

Signed-off-by: Mitali Borkar <mitaliborkar810@gmail.com>
---

Changes from v3:- No changes.
Changes from v2:- Aligned the code using tabs.
Changes from v1:- Aligned the code using tabs and readjusted the
comments line. 

 drivers/staging/media/zoran/zr36057.h | 312 ++++++++++++++------------
 1 file changed, 172 insertions(+), 140 deletions(-)

diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
index a2a75fd9f535..f51e9ea2c3d3 100644
--- a/drivers/staging/media/zoran/zr36057.h
+++ b/drivers/staging/media/zoran/zr36057.h
@@ -7,148 +7,180 @@
 
 #ifndef _ZR36057_H_
 #define _ZR36057_H_
+#include <linux/bitops.h>
 
 /* Zoran ZR36057 registers */
 
-#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
-
-#define ZR36057_VFEVCR          0x004	/* Video Front End, Vertical Configuration Register */
-#define ZR36057_VFEVCR_VS_POL             BIT(30)
-#define ZR36057_VFEVCR_V_START           10
-#define ZR36057_VFEVCR_V_END		0
-#define ZR36057_VFEVCR_VMASK		0x3ff
-
-#define ZR36057_VFESPFR         0x008	/* Video Front End, Scaler and Pixel Format Register */
-#define ZR36057_VFESPFR_EXT_FL            BIT(26)
-#define ZR36057_VFESPFR_TOP_FIELD         BIT(25)
-#define ZR36057_VFESPFR_VCLK_POL          BIT(24)
-#define ZR36057_VFESPFR_H_FILTER         21
-#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_VDTR            0x00c	/* Video Display "Top" Register */
-
-#define ZR36057_VDBR            0x010	/* Video Display "Bottom" Register */
-
-#define ZR36057_VSSFGR          0x014	/* Video Stride, Status, and Frame Grab Register */
-#define ZR36057_VSSFGR_DISP_STRIDE       16
-#define ZR36057_VSSFGR_VID_OVF            BIT(8)
-#define ZR36057_VSSFGR_SNAP_SHOT          BIT(1)
-#define ZR36057_VSSFGR_FRAME_GRAB         BIT(0)
-
-#define ZR36057_VDCR            0x018	/* Video Display Configuration Register */
-#define ZR36057_VDCR_VID_EN               BIT(31)
-#define ZR36057_VDCR_MIN_PIX             24
-#define ZR36057_VDCR_TRITON              BIT(24)
-#define ZR36057_VDCR_VID_WIN_HT           12
-#define ZR36057_VDCR_VID_WIN_WID          0
-
-#define ZR36057_MMTR            0x01c	/* Masking Map "Top" Register */
-
-#define ZR36057_MMBR            0x020	/* Masking Map "Bottom" Register */
-
-#define ZR36057_OCR             0x024	/* Overlay Control Register */
-#define ZR36057_OCR_OVL_ENABLE            BIT(15)
-#define ZR36057_OCR_MASK_STRIDE          0
-
-#define ZR36057_SPGPPCR         0x028	/* System, PCI, and General Purpose Pins Control Register */
-#define ZR36057_SPGPPCR_SOFT_RESET	 BIT(24)
-
-#define ZR36057_GPPGCR1         0x02c	/* General Purpose Pins and GuestBus Control Register (1) */
-
-#define ZR36057_MCSAR           0x030	/* MPEG Code Source Address Register */
-
-#define ZR36057_MCTCR           0x034	/* MPEG Code Transfer Control Register */
-#define ZR36057_MCTCR_COD_TIME            BIT(30)
-#define ZR36057_MCTCR_C_EMPTY             BIT(29)
-#define ZR36057_MCTCR_C_FLUSH             BIT(28)
-#define ZR36057_MCTCR_COD_GUEST_ID	20
-#define ZR36057_MCTCR_COD_GUEST_REG	16
-
-#define ZR36057_MCMPR           0x038	/* MPEG Code Memory Pointer Register */
-
-#define ZR36057_ISR             0x03c	/* Interrupt Status Register */
-#define ZR36057_ISR_GIRQ1                BIT(30)
-#define ZR36057_ISR_GIRQ0                BIT(29)
-#define ZR36057_ISR_COD_REP_IRQ            BIT(28)
-#define ZR36057_ISR_JPEG_REP_IRQ           BIT(27)
-
-#define ZR36057_ICR             0x040	/* Interrupt Control Register */
-#define ZR36057_ICR_GIRQ1                BIT(30)
-#define ZR36057_ICR_GIRQ0                BIT(29)
-#define ZR36057_ICR_COD_REP_IRQ            BIT(28)
-#define ZR36057_ICR_JPEG_REP_IRQ           BIT(27)
-#define ZR36057_ICR_INT_PIN_EN             BIT(24)
-
-#define ZR36057_I2CBR           0x044	/* I2C Bus Register */
-#define ZR36057_I2CBR_SDA		 BIT(1)
-#define ZR36057_I2CBR_SCL		 BIT(0)
-
-#define ZR36057_JMC             0x100	/* JPEG Mode and Control */
-#define ZR36057_JMC_JPG                  BIT(31)
-#define ZR36057_JMC_JPG_EXP_MODE          (0 << 29)
-#define ZR36057_JMC_JPG_CMP_MODE           BIT(29)
-#define ZR36057_JMC_MJPG_EXP_MODE         (2 << 29)
-#define ZR36057_JMC_MJPG_CMP_MODE         (3 << 29)
-#define ZR36057_JMC_RTBUSY_FB            BIT(6)
-#define ZR36057_JMC_GO_EN                BIT(5)
-#define ZR36057_JMC_SYNC_MSTR             BIT(4)
-#define ZR36057_JMC_FLD_PER_BUFF         BIT(3)
-#define ZR36057_JMC_VFIFO_FB             BIT(2)
-#define ZR36057_JMC_CFIFO_FB             BIT(1)
-#define ZR36057_JMC_STLL_LIT_ENDIAN       BIT(0)
-
-#define ZR36057_JPC             0x104	/* JPEG Process Control */
-#define ZR36057_JPC_P_RESET              BIT(7)
-#define ZR36057_JPC_COD_TRNS_EN            BIT(5)
-#define ZR36057_JPC_ACTIVE               BIT(0)
-
-#define ZR36057_VSP             0x108	/* Vertical Sync Parameters */
-#define ZR36057_VSP_VSYNC_SIZE           16
-#define ZR36057_VSP_FRM_TOT              0
-
-#define ZR36057_HSP             0x10c	/* Horizontal Sync Parameters */
-#define ZR36057_HSP_HSYNC_START          16
-#define ZR36057_HSP_LINE_TOT             0
-
-#define ZR36057_FHAP            0x110	/* Field Horizontal Active Portion */
-#define ZR36057_FHAP_NAX                16
-#define ZR36057_FHAP_PAX                0
-
-#define ZR36057_FVAP            0x114	/* Field Vertical Active Portion */
-#define ZR36057_FVAP_NAY                16
-#define ZR36057_FVAP_PAY                0
-
-#define ZR36057_FPP             0x118	/* Field Process Parameters */
-#define ZR36057_FPP_ODD_EVEN             BIT(0)
-
-#define ZR36057_JCBA            0x11c	/* JPEG Code Base Address */
-
-#define ZR36057_JCFT            0x120	/* JPEG Code FIFO Threshold */
-
-#define ZR36057_JCGI            0x124	/* JPEG Codec Guest ID */
-#define ZR36057_JCGI_JPE_GUEST_ID         4
-#define ZR36057_JCGI_JPE_GUEST_REG        0
-
-#define ZR36057_GCR2            0x12c	/* GuestBus Control Register (2) */
-
-#define ZR36057_POR             0x200	/* Post Office Register */
-#define ZR36057_POR_PO_PEN                BIT(25)
-#define ZR36057_POR_PO_TIME               BIT(24)
-#define ZR36057_POR_PO_DIR                BIT(23)
-
-#define ZR36057_STR             0x300	/* "Still" Transfer Register */
+/* 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
+
+/* Video Front End, Vertical Configuration Register */
+#define ZR36057_VFEVCR				0x004
+#define ZR36057_VFEVCR_VS_POL			BIT(30)
+#define ZR36057_VFEVCR_V_START			10
+#define ZR36057_VFEVCR_V_END			0
+#define ZR36057_VFEVCR_VMASK			0x3ff
+
+/* Video Front End, Scaler and Pixel Format Register */
+#define ZR36057_VFESPFR			0x008
+#define ZR36057_VFESPFR_EXT_FL			BIT(26)
+#define ZR36057_VFESPFR_TOP_FIELD		BIT(25)
+#define ZR36057_VFESPFR_VCLK_POL		BIT(24)
+#define ZR36057_VFESPFR_H_FILTER		21
+#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			BIT(3)
+#define ZR36057_VFESPFR_RGB565			(2 << 3)
+#define ZR36057_VFESPFR_RGB555			(3 << 3)
+#define ZR36057_VFESPFR_ERR_DIF		BIT(2)
+#define ZR36057_VFESPFR_PACK24			BIT(1)
+#define ZR36057_VFESPFR_LITTLE_ENDIAN		BIT(0)
+
+/* Video Display "Top" Register */
+#define ZR36057_VDTR				0x00c
+
+/* Video Display "Bottom" Register */
+#define ZR36057_VDBR				0x010
+
+/* Video Stride, Status, and Frame Grab Register */
+#define ZR36057_VSSFGR				0x014
+#define ZR36057_VSSFGR_DISP_STRIDE		16
+#define ZR36057_VSSFGR_VID_OVF			BIT(8)
+#define ZR36057_VSSFGR_SNAP_SHOT		BIT(1)
+#define ZR36057_VSSFGR_FRAME_GRAB		BIT(0)
+
+/* Video Display Configuration Register */
+#define ZR36057_VDCR				0x018
+#define ZR36057_VDCR_VID_EN			BIT(31)
+#define ZR36057_VDCR_MIN_PIX			24
+#define ZR36057_VDCR_TRITON			BIT(24)
+#define ZR36057_VDCR_VID_WIN_HT		12
+#define ZR36057_VDCR_VID_WIN_WID		0
+
+/* Masking Map "Top" Register */
+#define ZR36057_MMTR				0x01c
+
+/* Masking Map "Bottom" Register */
+#define ZR36057_MMBR				0x020
+
+/* Overlay Control Register */
+#define ZR36057_OCR				0x024
+#define ZR36057_OCR_OVL_ENABLE			BIT(15)
+#define ZR36057_OCR_MASK_STRIDE		0
+
+/* System, PCI, and General Purpose Pins Control Register */
+#define ZR36057_SPGPPCR			0x028
+#define ZR36057_SPGPPCR_SOFT_RESET		BIT(24)
+
+/* General Purpose Pins and GuestBus Control Register (1) */
+#define ZR36057_GPPGCR1			0x02c
+
+/* MPEG Code Source Address Register */
+#define ZR36057_MCSAR				0x030
+
+/* MPEG Code Transfer Control Register */
+#define ZR36057_MCTCR				0x034
+#define ZR36057_MCTCR_COD_TIME			BIT(30)
+#define ZR36057_MCTCR_C_EMPTY			BIT(29)
+#define ZR36057_MCTCR_C_FLUSH			BIT(28)
+#define ZR36057_MCTCR_COD_GUEST_ID		20
+#define ZR36057_MCTCR_COD_GUEST_REG		16
+
+/* MPEG Code Memory Pointer Register */
+#define ZR36057_MCMPR				0x038
+
+/* Interrupt Status Register */
+#define ZR36057_ISR				0x03c
+#define ZR36057_ISR_GIRQ1			BIT(30)
+#define ZR36057_ISR_GIRQ0			BIT(29)
+#define ZR36057_ISR_COD_REP_IRQ		BIT(28)
+#define ZR36057_ISR_JPEG_REP_IRQ		BIT(27)
+
+/* Interrupt Control Register */
+#define ZR36057_ICR				0x040
+#define ZR36057_ICR_GIRQ1			BIT(30)
+#define ZR36057_ICR_GIRQ0			BIT(29)
+#define ZR36057_ICR_COD_REP_IRQ		BIT(28)
+#define ZR36057_ICR_JPEG_REP_IRQ		BIT(27)
+#define ZR36057_ICR_INT_PIN_EN			BIT(24)
+
+/* I2C Bus Register */
+#define ZR36057_I2CBR				0x044
+#define ZR36057_I2CBR_SDA			BIT(1)
+#define ZR36057_I2CBR_SCL			BIT(0)
+
+/* JPEG Mode and Control */
+#define ZR36057_JMC				0x100
+#define ZR36057_JMC_JPG			BIT(31)
+#define ZR36057_JMC_JPG_EXP_MODE		(0 << 29)
+#define ZR36057_JMC_JPG_CMP_MODE		BIT(29)
+#define ZR36057_JMC_MJPG_EXP_MODE		(2 << 29)
+#define ZR36057_JMC_MJPG_CMP_MODE		(3 << 29)
+#define ZR36057_JMC_RTBUSY_FB			BIT(6)
+#define ZR36057_JMC_GO_EN			BIT(5)
+#define ZR36057_JMC_SYNC_MSTR			BIT(4)
+#define ZR36057_JMC_FLD_PER_BUFF		BIT(3)
+#define ZR36057_JMC_VFIFO_FB			BIT(2)
+#define ZR36057_JMC_CFIFO_FB			BIT(1)
+#define ZR36057_JMC_STLL_LIT_ENDIAN		BIT(0)
+
+/* JPEG Process Control */
+#define ZR36057_JPC				0x104
+#define ZR36057_JPC_P_RESET			BIT(7)
+#define ZR36057_JPC_COD_TRNS_EN		BIT(5)
+#define ZR36057_JPC_ACTIVE			BIT(0)
+
+/* Vertical Sync Parameters */
+#define ZR36057_VSP				0x108
+#define ZR36057_VSP_VSYNC_SIZE			16
+#define ZR36057_VSP_FRM_TOT			0
+
+/* Horizontal Sync Parameters */
+#define ZR36057_HSP				0x10c
+#define ZR36057_HSP_HSYNC_START		16
+#define ZR36057_HSP_LINE_TOT			0
+
+/* Field Horizontal Active Portion */
+#define ZR36057_FHAP				0x110
+#define ZR36057_FHAP_NAX			16
+#define ZR36057_FHAP_PAX			0
+
+/* Field Vertical Active Portion */
+#define ZR36057_FVAP				0x114
+#define ZR36057_FVAP_NAY			16
+#define ZR36057_FVAP_PAY			0
+
+/* Field Process Parameters */
+#define ZR36057_FPP				0x118
+#define ZR36057_FPP_ODD_EVEN			BIT(0)
+
+/* JPEG Code Base Address */
+#define ZR36057_JCBA				0x11c
+
+/* JPEG Code FIFO Threshold */
+#define ZR36057_JCFT				0x120
+
+/* JPEG Codec Guest ID */
+#define ZR36057_JCGI				0x124
+#define ZR36057_JCGI_JPE_GUEST_ID		4
+#define ZR36057_JCGI_JPE_GUEST_REG		0
+
+/* GuestBus Control Register (2) */
+#define ZR36057_GCR2				0x12c
+
+/* Post Office Register */
+#define ZR36057_POR				0x200
+#define ZR36057_POR_PO_PEN			BIT(25)
+#define ZR36057_POR_PO_TIME			BIT(24)
+#define ZR36057_POR_PO_DIR			BIT(23)
+
+/* "Still" Transfer Register */
+#define ZR36057_STR				0x300
 
 #endif
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code
  2021-04-13 19:50 ` [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code Mitali Borkar
@ 2021-04-14 10:38   ` zzam
  2021-04-15 13:12     ` Mitali Borkar
  0 siblings, 1 reply; 6+ messages in thread
From: zzam @ 2021-04-14 10:38 UTC (permalink / raw)
  To: Mitali Borkar, clabbe, mchehab, gregkh
  Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel, mitali_s

Am 13.04.21 um 21:50 schrieb Mitali Borkar:
> 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.
> Removed comments from the same line and added them to new line above the
> blocks, aligned everything properly by using tabs to make code neater
> and improve readability.
> Reported by checkpatch.
> 
> Signed-off-by: Mitali Borkar <mitaliborkar810@gmail.com>
> ---
> 
> Changes from v3:- No changes.
> Changes from v2:- Aligned the code using tabs.
> Changes from v1:- Aligned the code using tabs and readjusted the
> comments line.
> 
>   drivers/staging/media/zoran/zr36057.h | 312 ++++++++++++++------------
>   1 file changed, 172 insertions(+), 140 deletions(-)
> 

General comment:
The patch is hard to read without applying it and viewing it with ignore 
whitespace.

> diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> index a2a75fd9f535..f51e9ea2c3d3 100644
> --- a/drivers/staging/media/zoran/zr36057.h
> +++ b/drivers/staging/media/zoran/zr36057.h
> @@ -7,148 +7,180 @@
>   
>   #ifndef _ZR36057_H_
>   #define _ZR36057_H_
> +#include <linux/bitops.h>
>   
>   /* Zoran ZR36057 registers */
>   
> -#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
> -
> -#define ZR36057_VFEVCR          0x004	/* Video Front End, Vertical Configuration Register */
> -#define ZR36057_VFEVCR_VS_POL             BIT(30)
> -#define ZR36057_VFEVCR_V_START           10
> -#define ZR36057_VFEVCR_V_END		0
> -#define ZR36057_VFEVCR_VMASK		0x3ff
> -
> -#define ZR36057_VFESPFR         0x008	/* Video Front End, Scaler and Pixel Format Register */
> -#define ZR36057_VFESPFR_EXT_FL            BIT(26)
> -#define ZR36057_VFESPFR_TOP_FIELD         BIT(25)
> -#define ZR36057_VFESPFR_VCLK_POL          BIT(24)
> -#define ZR36057_VFESPFR_H_FILTER         21
> -#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_VDTR            0x00c	/* Video Display "Top" Register */
> -
> -#define ZR36057_VDBR            0x010	/* Video Display "Bottom" Register */
> -
> -#define ZR36057_VSSFGR          0x014	/* Video Stride, Status, and Frame Grab Register */
> -#define ZR36057_VSSFGR_DISP_STRIDE       16
> -#define ZR36057_VSSFGR_VID_OVF            BIT(8)
> -#define ZR36057_VSSFGR_SNAP_SHOT          BIT(1)
> -#define ZR36057_VSSFGR_FRAME_GRAB         BIT(0)
> -
> -#define ZR36057_VDCR            0x018	/* Video Display Configuration Register */
> -#define ZR36057_VDCR_VID_EN               BIT(31)
> -#define ZR36057_VDCR_MIN_PIX             24
> -#define ZR36057_VDCR_TRITON              BIT(24)
> -#define ZR36057_VDCR_VID_WIN_HT           12
> -#define ZR36057_VDCR_VID_WIN_WID          0
> -
> -#define ZR36057_MMTR            0x01c	/* Masking Map "Top" Register */
> -
> -#define ZR36057_MMBR            0x020	/* Masking Map "Bottom" Register */
> -
> -#define ZR36057_OCR             0x024	/* Overlay Control Register */
> -#define ZR36057_OCR_OVL_ENABLE            BIT(15)
> -#define ZR36057_OCR_MASK_STRIDE          0
> -
> -#define ZR36057_SPGPPCR         0x028	/* System, PCI, and General Purpose Pins Control Register */
> -#define ZR36057_SPGPPCR_SOFT_RESET	 BIT(24)
> -
> -#define ZR36057_GPPGCR1         0x02c	/* General Purpose Pins and GuestBus Control Register (1) */
> -
> -#define ZR36057_MCSAR           0x030	/* MPEG Code Source Address Register */
> -
> -#define ZR36057_MCTCR           0x034	/* MPEG Code Transfer Control Register */
> -#define ZR36057_MCTCR_COD_TIME            BIT(30)
> -#define ZR36057_MCTCR_C_EMPTY             BIT(29)
> -#define ZR36057_MCTCR_C_FLUSH             BIT(28)
> -#define ZR36057_MCTCR_COD_GUEST_ID	20
> -#define ZR36057_MCTCR_COD_GUEST_REG	16
> -
> -#define ZR36057_MCMPR           0x038	/* MPEG Code Memory Pointer Register */
> -
> -#define ZR36057_ISR             0x03c	/* Interrupt Status Register */
> -#define ZR36057_ISR_GIRQ1                BIT(30)
> -#define ZR36057_ISR_GIRQ0                BIT(29)
> -#define ZR36057_ISR_COD_REP_IRQ            BIT(28)
> -#define ZR36057_ISR_JPEG_REP_IRQ           BIT(27)
> -
> -#define ZR36057_ICR             0x040	/* Interrupt Control Register */
> -#define ZR36057_ICR_GIRQ1                BIT(30)
> -#define ZR36057_ICR_GIRQ0                BIT(29)
> -#define ZR36057_ICR_COD_REP_IRQ            BIT(28)
> -#define ZR36057_ICR_JPEG_REP_IRQ           BIT(27)
> -#define ZR36057_ICR_INT_PIN_EN             BIT(24)
> -
> -#define ZR36057_I2CBR           0x044	/* I2C Bus Register */
> -#define ZR36057_I2CBR_SDA		 BIT(1)
> -#define ZR36057_I2CBR_SCL		 BIT(0)
> -
> -#define ZR36057_JMC             0x100	/* JPEG Mode and Control */
> -#define ZR36057_JMC_JPG                  BIT(31)
> -#define ZR36057_JMC_JPG_EXP_MODE          (0 << 29)
> -#define ZR36057_JMC_JPG_CMP_MODE           BIT(29)
> -#define ZR36057_JMC_MJPG_EXP_MODE         (2 << 29)
> -#define ZR36057_JMC_MJPG_CMP_MODE         (3 << 29)
> -#define ZR36057_JMC_RTBUSY_FB            BIT(6)
> -#define ZR36057_JMC_GO_EN                BIT(5)
> -#define ZR36057_JMC_SYNC_MSTR             BIT(4)
> -#define ZR36057_JMC_FLD_PER_BUFF         BIT(3)
> -#define ZR36057_JMC_VFIFO_FB             BIT(2)
> -#define ZR36057_JMC_CFIFO_FB             BIT(1)
> -#define ZR36057_JMC_STLL_LIT_ENDIAN       BIT(0)
> -
> -#define ZR36057_JPC             0x104	/* JPEG Process Control */
> -#define ZR36057_JPC_P_RESET              BIT(7)
> -#define ZR36057_JPC_COD_TRNS_EN            BIT(5)
> -#define ZR36057_JPC_ACTIVE               BIT(0)
> -
> -#define ZR36057_VSP             0x108	/* Vertical Sync Parameters */
> -#define ZR36057_VSP_VSYNC_SIZE           16
> -#define ZR36057_VSP_FRM_TOT              0
> -
> -#define ZR36057_HSP             0x10c	/* Horizontal Sync Parameters */
> -#define ZR36057_HSP_HSYNC_START          16
> -#define ZR36057_HSP_LINE_TOT             0
> -
> -#define ZR36057_FHAP            0x110	/* Field Horizontal Active Portion */
> -#define ZR36057_FHAP_NAX                16
> -#define ZR36057_FHAP_PAX                0
> -
> -#define ZR36057_FVAP            0x114	/* Field Vertical Active Portion */
> -#define ZR36057_FVAP_NAY                16
> -#define ZR36057_FVAP_PAY                0
> -
> -#define ZR36057_FPP             0x118	/* Field Process Parameters */
> -#define ZR36057_FPP_ODD_EVEN             BIT(0)
> -
> -#define ZR36057_JCBA            0x11c	/* JPEG Code Base Address */
> -
> -#define ZR36057_JCFT            0x120	/* JPEG Code FIFO Threshold */
> -
> -#define ZR36057_JCGI            0x124	/* JPEG Codec Guest ID */
> -#define ZR36057_JCGI_JPE_GUEST_ID         4
> -#define ZR36057_JCGI_JPE_GUEST_REG        0
> -
> -#define ZR36057_GCR2            0x12c	/* GuestBus Control Register (2) */
> -
> -#define ZR36057_POR             0x200	/* Post Office Register */
> -#define ZR36057_POR_PO_PEN                BIT(25)
> -#define ZR36057_POR_PO_TIME               BIT(24)
> -#define ZR36057_POR_PO_DIR                BIT(23)
> -
> -#define ZR36057_STR             0x300	/* "Still" Transfer Register */
> +/* 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
> +
> +/* Video Front End, Vertical Configuration Register */
> +#define ZR36057_VFEVCR				0x004
> +#define ZR36057_VFEVCR_VS_POL			BIT(30)
> +#define ZR36057_VFEVCR_V_START			10
> +#define ZR36057_VFEVCR_V_END			0
> +#define ZR36057_VFEVCR_VMASK			0x3ff
> +
> +/* Video Front End, Scaler and Pixel Format Register */
> +#define ZR36057_VFESPFR			0x008
> +#define ZR36057_VFESPFR_EXT_FL			BIT(26)
> +#define ZR36057_VFESPFR_TOP_FIELD		BIT(25)
> +#define ZR36057_VFESPFR_VCLK_POL		BIT(24)
> +#define ZR36057_VFESPFR_H_FILTER		21
> +#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			BIT(3)
> +#define ZR36057_VFESPFR_RGB565			(2 << 3)
> +#define ZR36057_VFESPFR_RGB555			(3 << 3)
The single BIT occurrence in this list is confusing at best. Please keep 
ZR36057_VFESPFR_RGB888 to be (1 << 3). It is not a single bit, but 
defining a 2-bit field in the middle with 4 different values. It should 
be consistent.

> +#define ZR36057_VFESPFR_ERR_DIF		BIT(2)
> +#define ZR36057_VFESPFR_PACK24			BIT(1)
> +#define ZR36057_VFESPFR_LITTLE_ENDIAN		BIT(0)
> +
> +/* Video Display "Top" Register */
> +#define ZR36057_VDTR				0x00c
> +
> +/* Video Display "Bottom" Register */
> +#define ZR36057_VDBR				0x010
> +
> +/* Video Stride, Status, and Frame Grab Register */
> +#define ZR36057_VSSFGR				0x014
> +#define ZR36057_VSSFGR_DISP_STRIDE		16
> +#define ZR36057_VSSFGR_VID_OVF			BIT(8)
> +#define ZR36057_VSSFGR_SNAP_SHOT		BIT(1)
> +#define ZR36057_VSSFGR_FRAME_GRAB		BIT(0)
> +
> +/* Video Display Configuration Register */
> +#define ZR36057_VDCR				0x018
> +#define ZR36057_VDCR_VID_EN			BIT(31)
> +#define ZR36057_VDCR_MIN_PIX			24
> +#define ZR36057_VDCR_TRITON			BIT(24)
> +#define ZR36057_VDCR_VID_WIN_HT		12
> +#define ZR36057_VDCR_VID_WIN_WID		0
> +
> +/* Masking Map "Top" Register */
> +#define ZR36057_MMTR				0x01c
> +
> +/* Masking Map "Bottom" Register */
> +#define ZR36057_MMBR				0x020
> +
> +/* Overlay Control Register */
> +#define ZR36057_OCR				0x024
> +#define ZR36057_OCR_OVL_ENABLE			BIT(15)
> +#define ZR36057_OCR_MASK_STRIDE		0
> +
> +/* System, PCI, and General Purpose Pins Control Register */
> +#define ZR36057_SPGPPCR			0x028
> +#define ZR36057_SPGPPCR_SOFT_RESET		BIT(24)
> +
> +/* General Purpose Pins and GuestBus Control Register (1) */
> +#define ZR36057_GPPGCR1			0x02c
> +
> +/* MPEG Code Source Address Register */
> +#define ZR36057_MCSAR				0x030
> +
> +/* MPEG Code Transfer Control Register */
> +#define ZR36057_MCTCR				0x034
> +#define ZR36057_MCTCR_COD_TIME			BIT(30)
> +#define ZR36057_MCTCR_C_EMPTY			BIT(29)
> +#define ZR36057_MCTCR_C_FLUSH			BIT(28)
> +#define ZR36057_MCTCR_COD_GUEST_ID		20
> +#define ZR36057_MCTCR_COD_GUEST_REG		16
> +
> +/* MPEG Code Memory Pointer Register */
> +#define ZR36057_MCMPR				0x038
> +
> +/* Interrupt Status Register */
> +#define ZR36057_ISR				0x03c
> +#define ZR36057_ISR_GIRQ1			BIT(30)
> +#define ZR36057_ISR_GIRQ0			BIT(29)
> +#define ZR36057_ISR_COD_REP_IRQ		BIT(28)
> +#define ZR36057_ISR_JPEG_REP_IRQ		BIT(27)
> +
> +/* Interrupt Control Register */
> +#define ZR36057_ICR				0x040
> +#define ZR36057_ICR_GIRQ1			BIT(30)
> +#define ZR36057_ICR_GIRQ0			BIT(29)
> +#define ZR36057_ICR_COD_REP_IRQ		BIT(28)
> +#define ZR36057_ICR_JPEG_REP_IRQ		BIT(27)
> +#define ZR36057_ICR_INT_PIN_EN			BIT(24)
> +
> +/* I2C Bus Register */
> +#define ZR36057_I2CBR				0x044
> +#define ZR36057_I2CBR_SDA			BIT(1)
> +#define ZR36057_I2CBR_SCL			BIT(0)
> +
> +/* JPEG Mode and Control */
> +#define ZR36057_JMC				0x100
> +#define ZR36057_JMC_JPG			BIT(31)

> +#define ZR36057_JMC_JPG_EXP_MODE		(0 << 29)
> +#define ZR36057_JMC_JPG_CMP_MODE		BIT(29)
> +#define ZR36057_JMC_MJPG_EXP_MODE		(2 << 29)
> +#define ZR36057_JMC_MJPG_CMP_MODE		(3 << 29)
Same as above. Please change back ZR36057_JMC_JPG_CMP_MODE to be (1 << 
29). Then this 2 bit field is consistent.

> +#define ZR36057_JMC_RTBUSY_FB			BIT(6)
> +#define ZR36057_JMC_GO_EN			BIT(5)
> +#define ZR36057_JMC_SYNC_MSTR			BIT(4)
> +#define ZR36057_JMC_FLD_PER_BUFF		BIT(3)
> +#define ZR36057_JMC_VFIFO_FB			BIT(2)
> +#define ZR36057_JMC_CFIFO_FB			BIT(1)
> +#define ZR36057_JMC_STLL_LIT_ENDIAN		BIT(0)
> +
> +/* JPEG Process Control */
> +#define ZR36057_JPC				0x104
> +#define ZR36057_JPC_P_RESET			BIT(7)
> +#define ZR36057_JPC_COD_TRNS_EN		BIT(5)
> +#define ZR36057_JPC_ACTIVE			BIT(0)
> +
> +/* Vertical Sync Parameters */
> +#define ZR36057_VSP				0x108
> +#define ZR36057_VSP_VSYNC_SIZE			16
> +#define ZR36057_VSP_FRM_TOT			0
> +
> +/* Horizontal Sync Parameters */
> +#define ZR36057_HSP				0x10c
> +#define ZR36057_HSP_HSYNC_START		16
> +#define ZR36057_HSP_LINE_TOT			0
> +
> +/* Field Horizontal Active Portion */
> +#define ZR36057_FHAP				0x110
> +#define ZR36057_FHAP_NAX			16
> +#define ZR36057_FHAP_PAX			0
> +
> +/* Field Vertical Active Portion */
> +#define ZR36057_FVAP				0x114
> +#define ZR36057_FVAP_NAY			16
> +#define ZR36057_FVAP_PAY			0
> +
> +/* Field Process Parameters */
> +#define ZR36057_FPP				0x118
> +#define ZR36057_FPP_ODD_EVEN			BIT(0)
> +
> +/* JPEG Code Base Address */
> +#define ZR36057_JCBA				0x11c
> +
> +/* JPEG Code FIFO Threshold */
> +#define ZR36057_JCFT				0x120
> +
> +/* JPEG Codec Guest ID */
> +#define ZR36057_JCGI				0x124
> +#define ZR36057_JCGI_JPE_GUEST_ID		4
> +#define ZR36057_JCGI_JPE_GUEST_REG		0
> +
> +/* GuestBus Control Register (2) */
> +#define ZR36057_GCR2				0x12c
> +
> +/* Post Office Register */
> +#define ZR36057_POR				0x200
> +#define ZR36057_POR_PO_PEN			BIT(25)
> +#define ZR36057_POR_PO_TIME			BIT(24)
> +#define ZR36057_POR_PO_DIR			BIT(23)
> +
> +/* "Still" Transfer Register */
> +#define ZR36057_STR				0x300
>   
>   #endif
> 

Regards
Matthias

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code
  2021-04-14 10:38   ` zzam
@ 2021-04-15 13:12     ` Mitali Borkar
  2021-04-15 13:41       ` [Outreachy kernel] " Julia Lawall
  0 siblings, 1 reply; 6+ messages in thread
From: Mitali Borkar @ 2021-04-15 13:12 UTC (permalink / raw)
  To: zzam, clabbe, mchehab, gregkh
  Cc: linux-media, linux-staging, linux-kernel, outreachy-kernel, mitali_s

On Wed, Apr 14, 2021 at 12:38:41PM +0200, zzam@gentoo.org wrote:
> Am 13.04.21 um 21:50 schrieb Mitali Borkar:
> > 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.
> > Removed comments from the same line and added them to new line above the
> > blocks, aligned everything properly by using tabs to make code neater
> > and improve readability.
> > Reported by checkpatch.
> > 
> > Signed-off-by: Mitali Borkar <mitaliborkar810@gmail.com>
> > ---
> > 
> > Changes from v3:- No changes.
> > Changes from v2:- Aligned the code using tabs.
> > Changes from v1:- Aligned the code using tabs and readjusted the
> > comments line.
> > 
> >   drivers/staging/media/zoran/zr36057.h | 312 ++++++++++++++------------
> >   1 file changed, 172 insertions(+), 140 deletions(-)
> > 
> 
> General comment:
> The patch is hard to read without applying it and viewing it with ignore
> whitespace.
> 
> > diff --git a/drivers/staging/media/zoran/zr36057.h b/drivers/staging/media/zoran/zr36057.h
> > index a2a75fd9f535..f51e9ea2c3d3 100644
> > --- a/drivers/staging/media/zoran/zr36057.h
> > +++ b/drivers/staging/media/zoran/zr36057.h
> > @@ -7,148 +7,180 @@
> >   #ifndef _ZR36057_H_
> >   #define _ZR36057_H_
> > +#include <linux/bitops.h>
> >   /* Zoran ZR36057 registers */
> > -#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
> > -
> > -#define ZR36057_VFEVCR          0x004	/* Video Front End, Vertical Configuration Register */
> > -#define ZR36057_VFEVCR_VS_POL             BIT(30)
> > -#define ZR36057_VFEVCR_V_START           10
> > -#define ZR36057_VFEVCR_V_END		0
> > -#define ZR36057_VFEVCR_VMASK		0x3ff
> > -
> > -#define ZR36057_VFESPFR         0x008	/* Video Front End, Scaler and Pixel Format Register */
> > -#define ZR36057_VFESPFR_EXT_FL            BIT(26)
> > -#define ZR36057_VFESPFR_TOP_FIELD         BIT(25)
> > -#define ZR36057_VFESPFR_VCLK_POL          BIT(24)
> > -#define ZR36057_VFESPFR_H_FILTER         21
> > -#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_VDTR            0x00c	/* Video Display "Top" Register */
> > -
> > -#define ZR36057_VDBR            0x010	/* Video Display "Bottom" Register */
> > -
> > -#define ZR36057_VSSFGR          0x014	/* Video Stride, Status, and Frame Grab Register */
> > -#define ZR36057_VSSFGR_DISP_STRIDE       16
> > -#define ZR36057_VSSFGR_VID_OVF            BIT(8)
> > -#define ZR36057_VSSFGR_SNAP_SHOT          BIT(1)
> > -#define ZR36057_VSSFGR_FRAME_GRAB         BIT(0)
> > -
> > -#define ZR36057_VDCR            0x018	/* Video Display Configuration Register */
> > -#define ZR36057_VDCR_VID_EN               BIT(31)
> > -#define ZR36057_VDCR_MIN_PIX             24
> > -#define ZR36057_VDCR_TRITON              BIT(24)
> > -#define ZR36057_VDCR_VID_WIN_HT           12
> > -#define ZR36057_VDCR_VID_WIN_WID          0
> > -
> > -#define ZR36057_MMTR            0x01c	/* Masking Map "Top" Register */
> > -
> > -#define ZR36057_MMBR            0x020	/* Masking Map "Bottom" Register */
> > -
> > -#define ZR36057_OCR             0x024	/* Overlay Control Register */
> > -#define ZR36057_OCR_OVL_ENABLE            BIT(15)
> > -#define ZR36057_OCR_MASK_STRIDE          0
> > -
> > -#define ZR36057_SPGPPCR         0x028	/* System, PCI, and General Purpose Pins Control Register */
> > -#define ZR36057_SPGPPCR_SOFT_RESET	 BIT(24)
> > -
> > -#define ZR36057_GPPGCR1         0x02c	/* General Purpose Pins and GuestBus Control Register (1) */
> > -
> > -#define ZR36057_MCSAR           0x030	/* MPEG Code Source Address Register */
> > -
> > -#define ZR36057_MCTCR           0x034	/* MPEG Code Transfer Control Register */
> > -#define ZR36057_MCTCR_COD_TIME            BIT(30)
> > -#define ZR36057_MCTCR_C_EMPTY             BIT(29)
> > -#define ZR36057_MCTCR_C_FLUSH             BIT(28)
> > -#define ZR36057_MCTCR_COD_GUEST_ID	20
> > -#define ZR36057_MCTCR_COD_GUEST_REG	16
> > -
> > -#define ZR36057_MCMPR           0x038	/* MPEG Code Memory Pointer Register */
> > -
> > -#define ZR36057_ISR             0x03c	/* Interrupt Status Register */
> > -#define ZR36057_ISR_GIRQ1                BIT(30)
> > -#define ZR36057_ISR_GIRQ0                BIT(29)
> > -#define ZR36057_ISR_COD_REP_IRQ            BIT(28)
> > -#define ZR36057_ISR_JPEG_REP_IRQ           BIT(27)
> > -
> > -#define ZR36057_ICR             0x040	/* Interrupt Control Register */
> > -#define ZR36057_ICR_GIRQ1                BIT(30)
> > -#define ZR36057_ICR_GIRQ0                BIT(29)
> > -#define ZR36057_ICR_COD_REP_IRQ            BIT(28)
> > -#define ZR36057_ICR_JPEG_REP_IRQ           BIT(27)
> > -#define ZR36057_ICR_INT_PIN_EN             BIT(24)
> > -
> > -#define ZR36057_I2CBR           0x044	/* I2C Bus Register */
> > -#define ZR36057_I2CBR_SDA		 BIT(1)
> > -#define ZR36057_I2CBR_SCL		 BIT(0)
> > -
> > -#define ZR36057_JMC             0x100	/* JPEG Mode and Control */
> > -#define ZR36057_JMC_JPG                  BIT(31)
> > -#define ZR36057_JMC_JPG_EXP_MODE          (0 << 29)
> > -#define ZR36057_JMC_JPG_CMP_MODE           BIT(29)
> > -#define ZR36057_JMC_MJPG_EXP_MODE         (2 << 29)
> > -#define ZR36057_JMC_MJPG_CMP_MODE         (3 << 29)
> > -#define ZR36057_JMC_RTBUSY_FB            BIT(6)
> > -#define ZR36057_JMC_GO_EN                BIT(5)
> > -#define ZR36057_JMC_SYNC_MSTR             BIT(4)
> > -#define ZR36057_JMC_FLD_PER_BUFF         BIT(3)
> > -#define ZR36057_JMC_VFIFO_FB             BIT(2)
> > -#define ZR36057_JMC_CFIFO_FB             BIT(1)
> > -#define ZR36057_JMC_STLL_LIT_ENDIAN       BIT(0)
> > -
> > -#define ZR36057_JPC             0x104	/* JPEG Process Control */
> > -#define ZR36057_JPC_P_RESET              BIT(7)
> > -#define ZR36057_JPC_COD_TRNS_EN            BIT(5)
> > -#define ZR36057_JPC_ACTIVE               BIT(0)
> > -
> > -#define ZR36057_VSP             0x108	/* Vertical Sync Parameters */
> > -#define ZR36057_VSP_VSYNC_SIZE           16
> > -#define ZR36057_VSP_FRM_TOT              0
> > -
> > -#define ZR36057_HSP             0x10c	/* Horizontal Sync Parameters */
> > -#define ZR36057_HSP_HSYNC_START          16
> > -#define ZR36057_HSP_LINE_TOT             0
> > -
> > -#define ZR36057_FHAP            0x110	/* Field Horizontal Active Portion */
> > -#define ZR36057_FHAP_NAX                16
> > -#define ZR36057_FHAP_PAX                0
> > -
> > -#define ZR36057_FVAP            0x114	/* Field Vertical Active Portion */
> > -#define ZR36057_FVAP_NAY                16
> > -#define ZR36057_FVAP_PAY                0
> > -
> > -#define ZR36057_FPP             0x118	/* Field Process Parameters */
> > -#define ZR36057_FPP_ODD_EVEN             BIT(0)
> > -
> > -#define ZR36057_JCBA            0x11c	/* JPEG Code Base Address */
> > -
> > -#define ZR36057_JCFT            0x120	/* JPEG Code FIFO Threshold */
> > -
> > -#define ZR36057_JCGI            0x124	/* JPEG Codec Guest ID */
> > -#define ZR36057_JCGI_JPE_GUEST_ID         4
> > -#define ZR36057_JCGI_JPE_GUEST_REG        0
> > -
> > -#define ZR36057_GCR2            0x12c	/* GuestBus Control Register (2) */
> > -
> > -#define ZR36057_POR             0x200	/* Post Office Register */
> > -#define ZR36057_POR_PO_PEN                BIT(25)
> > -#define ZR36057_POR_PO_TIME               BIT(24)
> > -#define ZR36057_POR_PO_DIR                BIT(23)
> > -
> > -#define ZR36057_STR             0x300	/* "Still" Transfer Register */
> > +/* 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
> > +
> > +/* Video Front End, Vertical Configuration Register */
> > +#define ZR36057_VFEVCR				0x004
> > +#define ZR36057_VFEVCR_VS_POL			BIT(30)
> > +#define ZR36057_VFEVCR_V_START			10
> > +#define ZR36057_VFEVCR_V_END			0
> > +#define ZR36057_VFEVCR_VMASK			0x3ff
> > +
> > +/* Video Front End, Scaler and Pixel Format Register */
> > +#define ZR36057_VFESPFR			0x008
> > +#define ZR36057_VFESPFR_EXT_FL			BIT(26)
> > +#define ZR36057_VFESPFR_TOP_FIELD		BIT(25)
> > +#define ZR36057_VFESPFR_VCLK_POL		BIT(24)
> > +#define ZR36057_VFESPFR_H_FILTER		21
> > +#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			BIT(3)
> > +#define ZR36057_VFESPFR_RGB565			(2 << 3)
> > +#define ZR36057_VFESPFR_RGB555			(3 << 3)
> The single BIT occurrence in this list is confusing at best. Please keep
> ZR36057_VFESPFR_RGB888 to be (1 << 3). It is not a single bit, but defining
> a 2-bit field in the middle with 4 different values. It should be
> consistent.
>
Okay Sir.

> > +#define ZR36057_VFESPFR_ERR_DIF		BIT(2)
> > +#define ZR36057_VFESPFR_PACK24			BIT(1)
> > +#define ZR36057_VFESPFR_LITTLE_ENDIAN		BIT(0)
> > +
> > +/* Video Display "Top" Register */
> > +#define ZR36057_VDTR				0x00c
> > +
> > +/* Video Display "Bottom" Register */
> > +#define ZR36057_VDBR				0x010
> > +
> > +/* Video Stride, Status, and Frame Grab Register */
> > +#define ZR36057_VSSFGR				0x014
> > +#define ZR36057_VSSFGR_DISP_STRIDE		16
> > +#define ZR36057_VSSFGR_VID_OVF			BIT(8)
> > +#define ZR36057_VSSFGR_SNAP_SHOT		BIT(1)
> > +#define ZR36057_VSSFGR_FRAME_GRAB		BIT(0)
> > +
> > +/* Video Display Configuration Register */
> > +#define ZR36057_VDCR				0x018
> > +#define ZR36057_VDCR_VID_EN			BIT(31)
> > +#define ZR36057_VDCR_MIN_PIX			24
> > +#define ZR36057_VDCR_TRITON			BIT(24)
> > +#define ZR36057_VDCR_VID_WIN_HT		12
> > +#define ZR36057_VDCR_VID_WIN_WID		0
> > +
> > +/* Masking Map "Top" Register */
> > +#define ZR36057_MMTR				0x01c
> > +
> > +/* Masking Map "Bottom" Register */
> > +#define ZR36057_MMBR				0x020
> > +
> > +/* Overlay Control Register */
> > +#define ZR36057_OCR				0x024
> > +#define ZR36057_OCR_OVL_ENABLE			BIT(15)
> > +#define ZR36057_OCR_MASK_STRIDE		0
> > +
> > +/* System, PCI, and General Purpose Pins Control Register */
> > +#define ZR36057_SPGPPCR			0x028
> > +#define ZR36057_SPGPPCR_SOFT_RESET		BIT(24)
> > +
> > +/* General Purpose Pins and GuestBus Control Register (1) */
> > +#define ZR36057_GPPGCR1			0x02c
> > +
> > +/* MPEG Code Source Address Register */
> > +#define ZR36057_MCSAR				0x030
> > +
> > +/* MPEG Code Transfer Control Register */
> > +#define ZR36057_MCTCR				0x034
> > +#define ZR36057_MCTCR_COD_TIME			BIT(30)
> > +#define ZR36057_MCTCR_C_EMPTY			BIT(29)
> > +#define ZR36057_MCTCR_C_FLUSH			BIT(28)
> > +#define ZR36057_MCTCR_COD_GUEST_ID		20
> > +#define ZR36057_MCTCR_COD_GUEST_REG		16
> > +
> > +/* MPEG Code Memory Pointer Register */
> > +#define ZR36057_MCMPR				0x038
> > +
> > +/* Interrupt Status Register */
> > +#define ZR36057_ISR				0x03c
> > +#define ZR36057_ISR_GIRQ1			BIT(30)
> > +#define ZR36057_ISR_GIRQ0			BIT(29)
> > +#define ZR36057_ISR_COD_REP_IRQ		BIT(28)
> > +#define ZR36057_ISR_JPEG_REP_IRQ		BIT(27)
> > +
> > +/* Interrupt Control Register */
> > +#define ZR36057_ICR				0x040
> > +#define ZR36057_ICR_GIRQ1			BIT(30)
> > +#define ZR36057_ICR_GIRQ0			BIT(29)
> > +#define ZR36057_ICR_COD_REP_IRQ		BIT(28)
> > +#define ZR36057_ICR_JPEG_REP_IRQ		BIT(27)
> > +#define ZR36057_ICR_INT_PIN_EN			BIT(24)
> > +
> > +/* I2C Bus Register */
> > +#define ZR36057_I2CBR				0x044
> > +#define ZR36057_I2CBR_SDA			BIT(1)
> > +#define ZR36057_I2CBR_SCL			BIT(0)
> > +
> > +/* JPEG Mode and Control */
> > +#define ZR36057_JMC				0x100
> > +#define ZR36057_JMC_JPG			BIT(31)
> 
> > +#define ZR36057_JMC_JPG_EXP_MODE		(0 << 29)
> > +#define ZR36057_JMC_JPG_CMP_MODE		BIT(29)
> > +#define ZR36057_JMC_MJPG_EXP_MODE		(2 << 29)
> > +#define ZR36057_JMC_MJPG_CMP_MODE		(3 << 29)
> Same as above. Please change back ZR36057_JMC_JPG_CMP_MODE to be (1 << 29).
> Then this 2 bit field is consistent.
> 
Sir, I guess this BIT(29) was already present. I didn't changed this. 
I will change this as you said.

> > +#define ZR36057_JMC_RTBUSY_FB			BIT(6)
> > +#define ZR36057_JMC_GO_EN			BIT(5)
> > +#define ZR36057_JMC_SYNC_MSTR			BIT(4)
> > +#define ZR36057_JMC_FLD_PER_BUFF		BIT(3)
> > +#define ZR36057_JMC_VFIFO_FB			BIT(2)
> > +#define ZR36057_JMC_CFIFO_FB			BIT(1)
> > +#define ZR36057_JMC_STLL_LIT_ENDIAN		BIT(0)
> > +
> > +/* JPEG Process Control */
> > +#define ZR36057_JPC				0x104
> > +#define ZR36057_JPC_P_RESET			BIT(7)
> > +#define ZR36057_JPC_COD_TRNS_EN		BIT(5)
> > +#define ZR36057_JPC_ACTIVE			BIT(0)
> > +
> > +/* Vertical Sync Parameters */
> > +#define ZR36057_VSP				0x108
> > +#define ZR36057_VSP_VSYNC_SIZE			16
> > +#define ZR36057_VSP_FRM_TOT			0
> > +
> > +/* Horizontal Sync Parameters */
> > +#define ZR36057_HSP				0x10c
> > +#define ZR36057_HSP_HSYNC_START		16
> > +#define ZR36057_HSP_LINE_TOT			0
> > +
> > +/* Field Horizontal Active Portion */
> > +#define ZR36057_FHAP				0x110
> > +#define ZR36057_FHAP_NAX			16
> > +#define ZR36057_FHAP_PAX			0
> > +
> > +/* Field Vertical Active Portion */
> > +#define ZR36057_FVAP				0x114
> > +#define ZR36057_FVAP_NAY			16
> > +#define ZR36057_FVAP_PAY			0
> > +
> > +/* Field Process Parameters */
> > +#define ZR36057_FPP				0x118
> > +#define ZR36057_FPP_ODD_EVEN			BIT(0)
> > +
> > +/* JPEG Code Base Address */
> > +#define ZR36057_JCBA				0x11c
> > +
> > +/* JPEG Code FIFO Threshold */
> > +#define ZR36057_JCFT				0x120
> > +
> > +/* JPEG Codec Guest ID */
> > +#define ZR36057_JCGI				0x124
> > +#define ZR36057_JCGI_JPE_GUEST_ID		4
> > +#define ZR36057_JCGI_JPE_GUEST_REG		0
> > +
> > +/* GuestBus Control Register (2) */
> > +#define ZR36057_GCR2				0x12c
> > +
> > +/* Post Office Register */
> > +#define ZR36057_POR				0x200
> > +#define ZR36057_POR_PO_PEN			BIT(25)
> > +#define ZR36057_POR_PO_TIME			BIT(24)
> > +#define ZR36057_POR_PO_DIR			BIT(23)
> > +
> > +/* "Still" Transfer Register */
> > +#define ZR36057_STR				0x300
> >   #endif
> > 
> 
> Regards
> Matthias

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Outreachy kernel] Re: [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code
  2021-04-15 13:12     ` Mitali Borkar
@ 2021-04-15 13:41       ` Julia Lawall
  0 siblings, 0 replies; 6+ messages in thread
From: Julia Lawall @ 2021-04-15 13:41 UTC (permalink / raw)
  To: Mitali Borkar
  Cc: zzam, clabbe, mchehab, gregkh, linux-media, linux-staging,
	linux-kernel, outreachy-kernel, mitali_s

> > > +#define ZR36057_JMC_JPG_EXP_MODE		(0 << 29)
> > > +#define ZR36057_JMC_JPG_CMP_MODE		BIT(29)
> > > +#define ZR36057_JMC_MJPG_EXP_MODE		(2 << 29)
> > > +#define ZR36057_JMC_MJPG_CMP_MODE		(3 << 29)
> > Same as above. Please change back ZR36057_JMC_JPG_CMP_MODE to be (1 << 29).
> > Then this 2 bit field is consistent.
> >
> Sir, I guess this BIT(29) was already present. I didn't changed this.
> I will change this as you said.

It comes from this patch:

5e195bbddabdd94b15eeb60439cece996d58b329

Probably putting it back should be a different patch in the series.

julia

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-04-15 13:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-13 19:50 [PATCH v4 0/2] staging: media: zoran: Cleanup patchset for style issues in zr36057.h Mitali Borkar
2021-04-13 19:50 ` [PATCH v4 1/2] staging: media: zoran: add spaces around '<<' operator Mitali Borkar
2021-04-13 19:50 ` [PATCH v4 2/2] staging: media: zoran: add BIT() macro and align code Mitali Borkar
2021-04-14 10:38   ` zzam
2021-04-15 13:12     ` Mitali Borkar
2021-04-15 13:41       ` [Outreachy kernel] " Julia Lawall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).