Re: [PATCH v3 07/11] drm/mediatek: mtk_dp: Add support for PHY from devicetree

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Double unregister of the PHY platform device in the error path of `mtk_dp_register_phy()`.
- [High] `phy_power_on()` is called before `phy_init()` during eDP probe on new devicetrees.
--

--- Patch [7]: [PATCH v3 07/11] drm/mediatek: mtk_dp: Add support for PHY from devicetree ---
commit 325a8fa9ebb5392753c0df0c9aa94be702f67e6e
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

drm/mediatek: mtk_dp: Add support for PHY from devicetree

This commit adds support for passing a displayport PHY handle from the
devicetree. It introduces a check for the phys property to support newer
configurations while maintaining legacy fallback behavior.

> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index b4cb1996a3314..cabfc833c0a80 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c

[ ... ]

> @@ -2754,6 +2775,7 @@ static int mtk_dp_register_phy(struct mtk_dp *mtk_dp)
>  		return dev_err_probe(dev, ret,
>  				     "Failed to add phy unregister devm action");
>  
> +	/* PHY calibration data is in mtk_dp only for legacy devicetree */
>  	mtk_dp_get_calibration_data(mtk_dp);
>  
>  	mtk_dp->phy = devm_phy_get(&mtk_dp->phy_dev->dev, "dp");

[Severity: High]
Could this error path trigger a double-free of the PHY platform device?

In mtk_dp_register_phy(), we register a devres action to unregister the
platform device via mtk_dp_unregister_pdevs(). If devm_phy_get() fails
and we return an error (like -EPROBE_DEFER), the explicit call to
platform_device_unregister() in the error path will free the device. When
the devres cleanup runs later, it will execute mtk_dp_unregister_pdevs()
and call platform_device_unregister() a second time on the same pointer.

[ ... ]

> @@ -2857,9 +2884,18 @@ static int mtk_dp_probe(struct platform_device *pdev)
>  					     "Failed to register audio driver\n");
>  	}
>  
> -	ret = mtk_dp_register_phy(mtk_dp);
> -	if (ret)
> -		return ret;
> +	if (!mtk_dp->legacy_regoff) {
> +		mtk_dp->phy = devm_phy_get(dev, NULL);
> +		if (IS_ERR(mtk_dp->phy))
> +			return dev_err_probe(dev, PTR_ERR(mtk_dp->phy),
> +					     "Failed to get phy\n");
> +
> +		mtk_dp->phy_dev = NULL;

[Severity: High]
Are we calling phy_power_on() before phy_init() for eDP displays on new
devicetrees?

During mtk_dp_probe() for eDP connectors, the driver calls
mtk_dp_power_enable() to read the EDID before the bridge is attached.
With the new PHY model, this executes phy_power_on(mtk_dp->phy):

mtk_dp_probe()
  mtk_dp_power_enable()
    phy_power_on(mtk_dp->phy);

However, phy_init() is not called until mtk_dp_bridge_attach() via
mtk_dp_poweron(). Violating the PHY API contract by powering on before
initialization can cause hardware initialization failures, as clocks
or regulators managed by phy_init() are not yet enabled.

> +	} else {
> +		ret = mtk_dp_register_phy(mtk_dp);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	mtk_dp->bridge.of_node = dev->of_node;
>  	mtk_dp->bridge.type = mtk_dp->data->bridge_type;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709103431.46616-1-angelogioacchino.delregno@collabora.com?part=7