Re: [PATCH v2 03/12] phy: phy-mtk-dp: Allow probing with devicetree match

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] PM Runtime on the parent device is enabled after the PHY device is created, preventing PM runtime from propagating to the parent and causing unpowered hardware accesses.
- [Medium] The DP PHY driver fails to parse and apply the `nvmem-cells` calibration data specified in the new device tree binding.
- [Low] The variable `ret` is used but not declared in `mtk_dp_phy_probe()`, causing a build failure.
--

--- Patch [3]: [PATCH v2 03/12] phy: phy-mtk-dp: Allow probing with devicetree match ---
commit e7cb0174c7cb05364cf12874f491ce4c799f2c61
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Allow probing with devicetree match

This commit adds a devicetree match to decouple the MediaTek DP PHY driver
from the DisplayPort IP driver. It implements a standalone probe path while
retaining a legacy probe function for backwards compatibility.

> diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c
> --- a/drivers/phy/mediatek/phy-mtk-dp.c
> +++ b/drivers/phy/mediatek/phy-mtk-dp.c

[ ... ]

> @@ -160,43 +161,106 @@ static const struct phy_ops mtk_dp_phy_dev_ops = {

[ ... ]

>  static int mtk_dp_phy_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct phy_provider *provider;
>  	struct mtk_dp_phy *dp_phy;
> +	void __iomem *base;
>  	struct phy *phy;
> -	struct regmap *regs;
> -
> -	regs = *(struct regmap **)dev->platform_data;
> -	if (!regs)
> -		return dev_err_probe(dev, -EINVAL,
> -				     "No data passed, requires struct regmap**\n");
>  
>  	dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
>  	if (!dp_phy)
>  		return -ENOMEM;
>  
> -	dp_phy->regmap = regs;
> +	dp_phy->dev = dev;
> +
> +	/* If there's no devicetree, go for legacy pdev probe */
> +	if (!dev->of_node)
> +		return mtk_dp_phy_legacy_probe(pdev, dp_phy);

[Severity: Medium]
Does the new devicetree probe path need to parse and apply the nvmem-cells
calibration data?

It looks like the new DT bindings specify nvmem-cells for optimal EYE signal
pattern calibration, but the standalone probe path doesn't retrieve them,
which might leave the hardware running with sub-optimal defaults.

> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	dp_phy->regmap = devm_regmap_init_mmio(dev, base, &mtk_dp_phy_regmap_cfg);
> +	if (IS_ERR(dp_phy->regmap))
> +		return PTR_ERR(dp_phy->regmap);
> +
>  	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);

[Severity: High]
Will creating the PHY device before enabling PM runtime on the parent
prevent the PHY framework from propagating PM runtime to the child PHY
device?

In phy_create(), the framework checks pm_runtime_enabled(dev) on the parent
to decide whether to enable PM runtime on the newly created PHY device. Since
devm_pm_runtime_enable(dev) is called later, this check evaluates to false.

Could this lead to phy_pm_runtime_get_sync() returning -ENOTSUPP and
bypassing hardware wake-ups, resulting in unpowered register accesses?

>  	if (IS_ERR(phy))
>  		return dev_err_probe(dev, PTR_ERR(phy),
>  				     "Failed to create DP PHY\n");
>  
>  	phy_set_drvdata(phy, dp_phy);
> -	if (!dev->of_node)
> -		phy_create_lookup(phy, "dp", dev_name(dev));
> +
> +	provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	if (IS_ERR(provider))
> +		return PTR_ERR(provider);
> +
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret)
> +		return ret;

[Severity: Low]
Is the variable ret missing a declaration here?

It appears int ret; is not declared in mtk_dp_phy_probe(), which will cause
a build failure when compiling the driver.

>  
>  	return 0;
>  }

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