Re: [PATCH 1/3] media: rzg2l-cru: Skip ICnMC configuration when ICnSVC is used

[Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>]

Hi Jacopo,
Thanks for your review!

On Mon, Feb 09, 2026 at 03:21:59PM +0100, Jacopo Mondi wrote:
> Hi Tommaso
> 
> On Tue, Dec 30, 2025 at 06:09:15PM +0100, Tommaso Merciai wrote:
> > When the CRU is configured to use ICnSVC for virtual channel mapping,
> > as on the RZ/{G3E, V2H/P} SoC, the ICnMC register must not be
> > programmed.
> 
> I see a difference indeed between the [G3E, V2H/P] and the G2L version
> of the IP in the presence of the VCSEL[1:0] field in register ICnMC.
> 
> On [G3E, V2H/P] the selection of which virtual channel to accept
> doesn't go through VCSEL[1:0] in ICnMC but a dedicated register ICnSVC
> is provided for that purpose. So far so good.
> 
> >
> > Return early after setting up ICnSVC to avoid overriding the ICnMC
> > register, which is not applicable in this mode.
> >
> > This prevents unintended register programming when ICnSVC is enabled.
> >
> > Fixes: 3c5ca0a48bb0 ("media: rzg2l-cru: Drop function pointer to configure CSI")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> > ---
> >  drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > index 162e2ace6931..480e9b5dbcfe 100644
> > --- a/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > +++ b/drivers/media/platform/renesas/rzg2l-cru/rzg2l-video.c
> > @@ -268,6 +268,8 @@ static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
> >  		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
> >  		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
> >  				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
> > +
> > +		return;
> >  	}
> >
> >  	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
> 
> However, the rest of the rzg2l_cru_csi2_setup() doesn't only program
> the virtual channel filtering through ICnMC (which should be skept for
> G3E, V2H/P) but also programs DT filtering in INF[5:0] of register
> ICnIPMC_C0 which seems to be available on G3E and V2H/P as well.
> 
> Section "9.2.4.3.6 VC/Data Type Selector (for Image Processing)" of
> the chip manual (V2H/P) prescribes:
> 
> Only one data type can be handled by each SVC and the data types are
> selected in the ICnIPMC_C0 to 3.INF[5:0] registers.

You are correct, thanks.
I miss that part.

Maybe we can go for:

static void rzg2l_cru_csi2_setup(struct rzg2l_cru_dev *cru,
				 const struct rzg2l_cru_ip_format *ip_fmt,
				 u8 csi_vc)
{
	const struct rzg2l_cru_info *info = cru->info;
	u32 icnmc = ICnMC_INF(ip_fmt->datatype);

	if (cru->info->regs[ICnSVC]) {
		rzg2l_cru_write(cru, ICnSVCNUM, csi_vc);
		rzg2l_cru_write(cru, ICnSVC, ICnSVC_SVC0(0) | ICnSVC_SVC1(1) |
				ICnSVC_SVC2(2) | ICnSVC_SVC3(3));
	} else {
		/* Set virtual channel CSI2 */
		icnmc |= ICnMC_VCSEL(csi_vc);
	}

	icnmc |= rzg2l_cru_read(cru, info->image_conv) & ~ICnMC_INF_MASK;
	rzg2l_cru_write(cru, info->image_conv, icnmc);
}

?

> 
> And this patch makes the driver skips that part.
> Has this patch been tested ? It breaks my V2H/P setup:
> 
>  rzg2l-cru 16010000.cru1: Invalid MB address 0x0 (out of range)

I've not seeing that on RZ/G3E.
Thanks for sharing.


Kind Regards,
Tommaso

> 
> 
> > --
> > 2.43.0
> >
> >