Re: [PATCH v2 8/9] media: qcom: camss: Fix set CSI2_RX_CFG1_VC_MODE when VC is greater than 3

From: Laurent Pinchart
Date: Mon Aug 28 2023 - 13:23:05 EST


Hi Bryan,

Thank you for the patch.

On Tue, Aug 22, 2023 at 09:06:25PM +0100, Bryan O'Donoghue wrote:
> VC_MODE = 0 implies a two bit VC address.
> VC_MODE = 1 is required for VCs with a larger address than two bits.
>
> Fixes: eebe6d00e9bf ("media: camss: Add support for CSID hardware version Titan 170")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@xxxxxxxxxx>
> ---
> drivers/media/platform/qcom/camss/camss-csid-gen2.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/camss/camss-csid-gen2.c b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> index 45c7986d4a8d0..140c584bfb8b1 100644
> --- a/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> +++ b/drivers/media/platform/qcom/camss/camss-csid-gen2.c
> @@ -449,6 +449,8 @@ static void __csid_configure_stream(struct csid_device *csid, u8 enable, u8 vc)
> writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG0);
>
> val = 1 << CSI2_RX_CFG1_PACKET_ECC_CORRECTION_EN;
> + if (vc > 3)
> + val |= 1 << CSI2_RX_CFG1_VC_MODE;

It looks like CSI2_RX_CFG1_VC_MODE should be defined as BIT(2) instead
of 2, and this line should drop the '1 <<'. Same for lots of other bits.
Could you fix this in a separate patch ?

Reviewed-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>

> val |= 1 << CSI2_RX_CFG1_MISR_EN;
> writel_relaxed(val, csid->base + CSID_CSI2_RX_CFG1);
>

--
Regards,

Laurent Pinchart