Re: [PATCH v2 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing NULL check on device_get_match_data() result leads to potential NULL pointer dereference.
- [Low] Undeclared variable 'ret' causes a build failure.
--

--- Patch [6]: [PATCH v2 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup ---
commit 564fc9b179c0513c9ad23d3689aaf0c8211c61c3
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup

Add support for enabling a specifically requested number of lanes
in the .configure() callback and disable all lanes in power off
and reset callbacks for proper hardware cleanup.

> 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

[ ... ]

> +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;
> +
> +	dp_phy = devm_kzalloc(dev, sizeof(*dp_phy), GFP_KERNEL);
> +	if (!dp_phy)
> +		return -ENOMEM;
> +
> +	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);
> +
> +	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);
> +
> +	dp_phy->pdata = device_get_match_data(dev);
> +
> +	phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);

[Severity: High]
Can dp_phy->pdata be NULL here if the driver is bound manually?

If a user writes the driver name to 'driver_override' in sysfs for an
unrelated platform device, device_get_match_data() will return NULL since
the compatible string won't match.

The probe will succeed, but subsequent PHY framework calls to .init or
.configure will unconditionally dereference dp_phy->pdata, which could lead
to a kernel panic.

Would it be safer to verify the result of device_get_match_data() before
proceeding?

> +	if (IS_ERR(phy))
> +		return dev_err_probe(dev, PTR_ERR(phy),
> +				     "Failed to create DP PHY\n");
> +
> +	phy_set_drvdata(phy, dp_phy);
> +
> +	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);

[Severity: Low]
Is the variable 'ret' missing a declaration in this function?

It appears 'ret' is assigned and checked here, but it wasn't declared in
the variable block at the start of mtk_dp_phy_probe(), which will cause a
build failure.

> +	if (ret)
> +		return ret;
> +
> +	return 0;
> +}

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy