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

[Frank Li <Frank.li@oss.nxp.com>]

On Tue, Oct 20, 2026 at 08:01:00AM +0000, sashiko-bot@kernel.org wrote:
> 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()?

Yes, please fix it. ndelay() is not necessary have mmio. So it may not
delay 15ns after value reach hardware.

Frank

>
> 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