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

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

[Guoniu Zhou]

The Synopsys DPHY datasheet recommends applying a testclr pulse at
first power-up to reset the PHY configuration to its default values
before any test control register is programmed.

The existing code deasserts testclr first and then asserts it, which
is the reverse of the sequence recommended by the DPHY documentation.

Generate the required TESTCLR pulse by asserting TESTCLR, waiting
for the required pulse width, and then deasserting it.

Fixes: ec40b431f0ab ("media: synopsys: csi2rx: add i.MX93 support")
Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>
---
 drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
index b7247d597c9c..b9f7ba25bd3b 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);
 }
 

---
base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
change-id: 20261020-dw-csi2rx-6225697f45bb

Best regards,
-- 
Guoniu Zhou <guoniu.zhou@oss.nxp.com>

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

[sashiko-bot]

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

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

[Frank Li]

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

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

[Michael Riesch]

Hi Guoniu,

Thanks for the patch.

On 10/20/26 05:04, Guoniu Zhou wrote:
> The Synopsys DPHY datasheet recommends applying a testclr pulse at
> first power-up to reset the PHY configuration to its default values
> before any test control register is programmed.
> 
> The existing code deasserts testclr first and then asserts it, which
> is the reverse of the sequence recommended by the DPHY documentation.
> 
> Generate the required TESTCLR pulse by asserting TESTCLR, waiting
> for the required pulse width, and then deasserting it.
> 
> Fixes: ec40b431f0ab ("media: synopsys: csi2rx: add i.MX93 support")
> Signed-off-by: Guoniu Zhou <guoniu.zhou@oss.nxp.com>

Acked-by: Michael Riesch <michael.riesch@collabora.com>

Best regards,
Michael

> ---
>  drivers/media/platform/synopsys/dw-mipi-csi2rx.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index b7247d597c9c..b9f7ba25bd3b 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);
>  }
>  
> 
> ---
> base-commit: d7e7bca7169873b46e857c025c10f3336c1258d8
> change-id: 20261020-dw-csi2rx-6225697f45bb
> 
> Best regards,