Re: [PATCH v4 4/6] media: synopsys: Add PHY stopstate wait for i.MX93

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Waiting for the PHY stop state (LP-11) after enabling the remote sensor streams introduces a severe race condition, causing initialization timeouts and capture failures.
--

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

media: synopsys: Add PHY stopstate wait for i.MX93

This commit implements waiting for D-PHY lanes to enter stop state on
i.MX93. This ensures proper PHY initialization by verifying that the clock
lane and all active data lanes have entered the stop state before proceeding
with further operations.

> diff --git a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> index 92178a3dec5d5..8a34aec550ade 100644
> --- a/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
> +++ b/drivers/media/platform/synopsys/dw-mipi-csi2rx.c
[ ... ]
> @@ -556,10 +562,19 @@ static int dw_mipi_csi2rx_enable_streams(struct v4l2_subdev *sd,
>  	if (ret)
>  		goto err_csi_stop;
>  
> +	if (!csi2->enabled_streams &&
> +	    csi2->drvdata->wait_for_phy_stopstate) {
> +		ret = csi2->drvdata->wait_for_phy_stopstate(csi2);
> +		if (ret)
> +			goto err_disable_streams;
> +	}

Is there a race condition between enabling the remote streams and waiting
for the PHY to enter the stop state (LP-11)?

In dw_mipi_csi2rx_enable_streams(), v4l2_subdev_enable_streams() is called
just before this wait_for_phy_stopstate() block. Enabling the sensor stream
commands it to leave the stop state and enter High Speed (HS) mode to
transmit data.

By the time the polling loop in wait_for_phy_stopstate() reads the
DW_MIPI_CSI2RX_PHY_STOPSTATE register, the sensor might have already
transitioned to HS mode, especially since the polling loop uses
sleep_before_read = true. In HS mode, the lanes are actively toggling and
are no longer in the stop state.

Could this cause the polling loop to never detect the required stop state,
resulting in a -ETIMEDOUT error and causing the stream enablement to fail?

Should the stop state check occur before enabling the remote subdev streams?

> +
>  	csi2->enabled_streams |= streams_mask;
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519-csi2_imx95-v4-0-84ea4bb78a88@oss.nxp.com?part=4