[PATCH 2/7] media: rzv2h-ivc: Fix AXIRX_VBLANK register write

Jacopo Mondi posted 7 patches 3 weeks, 4 days ago
[PATCH 2/7] media: rzv2h-ivc: Fix AXIRX_VBLANK register write
Posted by Jacopo Mondi 3 weeks, 4 days ago
From: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>

According to the documentation there are writable reserved bits in the
register and those should not be set to 0. So use `rzv2h_ivc_update_bits()`
with a proper bitmask.

Cc: stable@vger.kernel.org
Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
---
 drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 7 +++++--
 drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h       | 2 +-
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
index 1e016b17dcee..bfe5b0c7045e 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
@@ -7,6 +7,7 @@
 
 #include "rzv2h-ivc.h"
 
+#include <linux/bitfield.h>
 #include <linux/cleanup.h>
 #include <linux/iopoll.h>
 #include <linux/lockdep.h>
@@ -235,8 +236,10 @@ static void rzv2h_ivc_format_configure(struct rzv2h_ivc *ivc)
 	hts = pix->width + RZV2H_IVC_FIXED_HBLANK;
 	vblank = RZV2H_IVC_MIN_VBLANK(hts);
 
-	rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
-			RZV2H_IVC_VBLANK(vblank));
+	rzv2h_ivc_update_bits(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
+			      RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
+			      FIELD_PREP(RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
+					 vblank));
 }
 
 static void rzv2h_ivc_return_buffers(struct rzv2h_ivc *ivc,
diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
index 3bcaab990b0f..4ef44c8b4656 100644
--- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
+++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
@@ -34,7 +34,7 @@
 #define RZV2H_IVC_REG_AXIRX_HSIZE			0x0020
 #define RZV2H_IVC_REG_AXIRX_VSIZE			0x0024
 #define RZV2H_IVC_REG_AXIRX_BLANK			0x0028
-#define RZV2H_IVC_VBLANK(x)				((x) << 16)
+#define RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK		GENMASK(25, 16)
 #define RZV2H_IVC_REG_AXIRX_STRD			0x0030
 #define RZV2H_IVC_REG_AXIRX_ISSU			0x0040
 #define RZV2H_IVC_REG_AXIRX_ERACT			0x0048

-- 
2.53.0

Re: [PATCH 2/7] media: rzv2h-ivc: Fix AXIRX_VBLANK register write
Posted by Barnabás Pőcze 2 weeks, 6 days ago
2026. 03. 13. 12:13 keltezéssel, Jacopo Mondi írta:
> From: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> 
> According to the documentation there are writable reserved bits in the
> register and those should not be set to 0. So use `rzv2h_ivc_update_bits()`
> with a proper bitmask.
> 
> Cc: stable@vger.kernel.org
> Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---
>   drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 7 +++++--
>   drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h       | 2 +-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> index 1e016b17dcee..bfe5b0c7045e 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> @@ -7,6 +7,7 @@
>   
>   #include "rzv2h-ivc.h"
>   
> +#include <linux/bitfield.h>
>   #include <linux/cleanup.h>
>   #include <linux/iopoll.h>
>   #include <linux/lockdep.h>
> @@ -235,8 +236,10 @@ static void rzv2h_ivc_format_configure(struct rzv2h_ivc *ivc)
>   	hts = pix->width + RZV2H_IVC_FIXED_HBLANK;
>   	vblank = RZV2H_IVC_MIN_VBLANK(hts);
>   
> -	rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
> -			RZV2H_IVC_VBLANK(vblank));
> +	rzv2h_ivc_update_bits(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
> +			      RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
> +			      FIELD_PREP(RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
> +					 vblank));
>   }
>   
>   static void rzv2h_ivc_return_buffers(struct rzv2h_ivc *ivc,
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> index 3bcaab990b0f..4ef44c8b4656 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> @@ -34,7 +34,7 @@
>   #define RZV2H_IVC_REG_AXIRX_HSIZE			0x0020
>   #define RZV2H_IVC_REG_AXIRX_VSIZE			0x0024
>   #define RZV2H_IVC_REG_AXIRX_BLANK			0x0028
> -#define RZV2H_IVC_VBLANK(x)				((x) << 16)
> +#define RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK		GENMASK(25, 16)

On second look, I have a small comment: I now greatly dislike how I have named this.
Especially that it is inconsistent with `RZV2H_IVC_REG_FM_STOP_FSTOP` in patch 4/7.
Possibly `RZV2H_IVC_REG_AXIRX_BLANK_VBLANK` or `RZV2H_IVC_REG_AXIRX_BLANK_FIELD_VBLANK`
would be better (although in the latter case I would rename the macro in patch 4/7 to
`RZV2H_IVC_REG_FM_STOP_FIELD_FSTOP`). The same applies to patch 3/7 as well, I'd now do
`RZV2H_IVC_REG_AXIRX_PXFMT_(FIELD_){CLFMT,DTYPE}` or similar. Sorry about that.


Regards,
Barnabás Pőcze


>   #define RZV2H_IVC_REG_AXIRX_STRD			0x0030
>   #define RZV2H_IVC_REG_AXIRX_ISSU			0x0040
>   #define RZV2H_IVC_REG_AXIRX_ERACT			0x0048
> 

Re: [PATCH 2/7] media: rzv2h-ivc: Fix AXIRX_VBLANK register write
Posted by Dan Scally 3 weeks, 3 days ago
Hi Jacopo and Barnabás

On 13/03/2026 11:13, Jacopo Mondi wrote:
> From: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> 
> According to the documentation there are writable reserved bits in the
> register and those should not be set to 0. So use `rzv2h_ivc_update_bits()`
> with a proper bitmask.
> 
> Cc: stable@vger.kernel.org
> Fixes: f0b3984d821b ("media: platform: Add Renesas Input Video Control block driver")
> Signed-off-by: Barnabás Pőcze <barnabas.pocze+renesas@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi+renesas@ideasonboard.com>
> ---

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>   drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c | 7 +++++--
>   drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h       | 2 +-
>   2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> index 1e016b17dcee..bfe5b0c7045e 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc-video.c
> @@ -7,6 +7,7 @@
>   
>   #include "rzv2h-ivc.h"
>   
> +#include <linux/bitfield.h>
>   #include <linux/cleanup.h>
>   #include <linux/iopoll.h>
>   #include <linux/lockdep.h>
> @@ -235,8 +236,10 @@ static void rzv2h_ivc_format_configure(struct rzv2h_ivc *ivc)
>   	hts = pix->width + RZV2H_IVC_FIXED_HBLANK;
>   	vblank = RZV2H_IVC_MIN_VBLANK(hts);
>   
> -	rzv2h_ivc_write(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
> -			RZV2H_IVC_VBLANK(vblank));
> +	rzv2h_ivc_update_bits(ivc, RZV2H_IVC_REG_AXIRX_BLANK,
> +			      RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
> +			      FIELD_PREP(RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK,
> +					 vblank));
>   }
>   
>   static void rzv2h_ivc_return_buffers(struct rzv2h_ivc *ivc,
> diff --git a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> index 3bcaab990b0f..4ef44c8b4656 100644
> --- a/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> +++ b/drivers/media/platform/renesas/rzv2h-ivc/rzv2h-ivc.h
> @@ -34,7 +34,7 @@
>   #define RZV2H_IVC_REG_AXIRX_HSIZE			0x0020
>   #define RZV2H_IVC_REG_AXIRX_VSIZE			0x0024
>   #define RZV2H_IVC_REG_AXIRX_BLANK			0x0028
> -#define RZV2H_IVC_VBLANK(x)				((x) << 16)
> +#define RZV2H_IVC_AXIRX_BLANK_FIELD_VBLANK		GENMASK(25, 16)
>   #define RZV2H_IVC_REG_AXIRX_STRD			0x0030
>   #define RZV2H_IVC_REG_AXIRX_ISSU			0x0040
>   #define RZV2H_IVC_REG_AXIRX_ERACT			0x0048
>