Re: [PATCH] media: synopsys: dw-mipi-csi2rx: Fix DPHY testclr reset sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: missing MMIO flush before ndelay() in imx93_csi2rx_dphy_assert_reset()
--

commit b3d3c4c8602e49d8fd933f7b1d722f45a1287b01
Author: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

media: synopsys: dw-mipi-csi2rx: Fix DPHY testclr reset sequence

This commit updates imx93_csi2rx_dphy_assert_reset() so that the DPHY test
clear bit is asserted first, held for 15ns, and then deasserted.

> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index b7247d597c9cc..b9f7ba25bd3ba 100644
> --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> @@ -830,18 +830,23 @@ static void imx93_csi2rx_dphy_assert_reset(struct dw_mipi_csi2rx_device *csi2)
>  {
>  	u32 val;
>  
> -	/* Release Synopsys DPHY test codes from reset */
> +	/* Put PHY in shutdown mode for configuration */
>  	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_DPHY_RSTZ, 0);
>  	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_PHY_SHUTDOWNZ, 0);
>  
> +	/*
> +	 * Apply testclr pulse to load analog programmability defaults
> +	 * and release the test interface for configuration.
> +	 */
>  	val = dw_mipi_csi2rx_read(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0);
> -	val &= ~DPHY_TEST_CTRL0_TEST_CLR;
> +	val |= DPHY_TEST_CTRL0_TEST_CLR;
>  	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0, val);
>  
> -	val = dw_mipi_csi2rx_read(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0);
>  	/* Wait for at least 15ns */
>  	ndelay(15);
> -	val |= DPHY_TEST_CTRL0_TEST_CLR;
> +
> +	val = dw_mipi_csi2rx_read(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0);
> +	val &= ~DPHY_TEST_CTRL0_TEST_CLR;
>  	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0, val);
>  }

[Severity: Medium]
Should dw_mipi_csi2rx_read(csi2, DW_MIPI_CSI2RX_PHY_TST_CTRL0) remain before
ndelay(15) in imx93_csi2rx_dphy_assert_reset()?

When streaming is started via dw_mipi_csi2rx_start() ->
imx93_csi2rx_dphy_assert_reset(), moving the read of
DW_MIPI_CSI2RX_PHY_TST_CTRL0 to after ndelay(15) means the posted writel()
asserting DPHY_TEST_CTRL0_TEST_CLR is no longer flushed to the target
register before the 15ns CPU delay loop executes.

If that write remains buffered in the interconnect while ndelay(15) runs, the
subsequent read will flush the assertion and immediately clear and write
DPHY_TEST_CTRL0_TEST_CLR back without waiting 15ns, which could violate the
minimum pulse width.

Note that imx93_csi2rx_dphy_deassert_reset() right below this function
specifically performs a readback before ndelay() for this reason:

	dw_mipi_csi2rx_write(csi2, DW_MIPI_CSI2RX_PHY_SHUTDOWNZ, 0x1);
	/*
	 * ndelay() is not necessary have MMIO operation, need dummy read to
	 * ensure that the write operation above reaches its target.
	 */
	dw_mipi_csi2rx_read(csi2, DW_MIPI_CSI2RX_PHY_SHUTDOWNZ);
	ndelay(5);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-dw-csi2rx-v1-1-c58a2e5cd43d@oss.nxp.com?part=1