Re: [PATCH 2/3] drm/bridge: dw-mipi-dsi: add PHY write function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/rockchip: dsi: uninitialized dsi->dmd in dw_mipi_dsi_dphy_power_on()
--

commit bdfb343509e10a4a68b931e1345f0a0c640cfd6f
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/bridge: dw-mipi-dsi: add PHY write function

This commit moves dw_mipi_dsi_phy_write() from the Rockchip glue driver
to the common dw-mipi-dsi bridge code and updates the Rockchip DSI driver
to call the shared helper using its struct dw_mipi_dsi pointer.

> diff --git a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> index 268d4993a04fd..469123620f302 100644
> --- a/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c

[ ... ]

> @@ -1224,6 +1203,7 @@ static int dw_mipi_dsi_dphy_configure(struct phy *phy, union phy_configure_opts
>  static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
>  {
>  	struct dw_mipi_dsi_rockchip *dsi = phy_get_drvdata(phy);
> +	struct dw_mipi_dsi *dmd = dsi->dmd;
>  	int i, ret;
>  
>  	DRM_DEV_DEBUG(dsi->dev, "lanes %d - data_rate_mbps %u\n",
> @@ -1273,15 +1253,15 @@ static int dw_mipi_dsi_dphy_power_on(struct phy *phy)
>  	 * Configure hsfreqrange according to frequency values
>  	 * Set clock lane and hsfreqrange by lane0(test code 0x44)
>  	 */
> -	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_CLK, 0);
> -	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_0,
> +	dw_mipi_dsi_phy_write(dmd, HS_RX_CONTROL_OF_LANE_CLK, 0);
> +	dw_mipi_dsi_phy_write(dmd, HS_RX_CONTROL_OF_LANE_0,
>  			      HSFREQRANGE_SEL(dppa_map[i].hsfreqrange));
> -	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_1, 0);
> -	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_2, 0);
> -	dw_mipi_dsi_phy_write(dsi, HS_RX_CONTROL_OF_LANE_3, 0);
> +	dw_mipi_dsi_phy_write(dmd, HS_RX_CONTROL_OF_LANE_1, 0);
> +	dw_mipi_dsi_phy_write(dmd, HS_RX_CONTROL_OF_LANE_2, 0);
> +	dw_mipi_dsi_phy_write(dmd, HS_RX_CONTROL_OF_LANE_3, 0);
>  
>  	/* Normal operation */
> -	dw_mipi_dsi_phy_write(dsi, 0x0, 0);
> +	dw_mipi_dsi_phy_write(dmd, 0x0, 0);

[Severity: High]
Can dsi->dmd be NULL or an ERR_PTR when dw_mipi_dsi_dphy_power_on() is
called?

In dw_mipi_dsi_rockchip_probe(), the PHY provider is registered before
dsi->dmd is initialized:

drivers/gpu/drm/rockchip/dw-mipi-dsi-rockchip.c:dw_mipi_dsi_rockchip_probe() {
	...
	phy_set_drvdata(dsi->dphy, dsi);
	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
	if (IS_ERR(phy_provider))
		return PTR_ERR(phy_provider);

	dsi->dmd = dw_mipi_dsi_probe(pdev, &dsi->pdata);
	if (IS_ERR(dsi->dmd)) {
	...
}

Previously, dw_mipi_dsi_dphy_power_on() wrote through dsi->base, which is
already mapped before devm_of_phy_provider_register(). With this change,
dw_mipi_dsi_dphy_power_on() passes dsi->dmd to dw_mipi_dsi_phy_write(),
which dereferences dmd->base.

If a PHY consumer (such as rkisp1 on RK3399) acquires the PHY and calls
phy_power_on() -> dw_mipi_dsi_dphy_power_on() while dw_mipi_dsi_probe() is
still running, or after dw_mipi_dsi_probe() fails and before devm unwinds
the PHY provider registration, wouldn't dw_mipi_dsi_phy_write() dereference
a NULL or ERR_PTR dmd?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-th1520-dw-mipi-dsi-v1-0-753f34fd304f@iscas.ac.cn?part=2