Re: [PATCH v4 03/12] phy: phy-mtk-dp: Allow probing with devicetree match
From: sashiko-bot@kernel.org
Date: 2026-09-10 09:41:10
Also in:
dri-devel, linux-devicetree
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: New issues: - [High] Hardcoded register offsets cause incorrect MMIO access or resource conflicts in the new OF probe path. Pre-existing issues: - [High] NULL pointer dereference when accessing platform_data before validation. -- commit e5a7ed1fddd4b70c5e3f6f679c0fa4e43da6f22b Author: AngeloGioacchino Del Regno [off-list ref] phy: phy-mtk-dp: Allow probing with devicetree match This commit decouples the DisplayPort PHY driver from the DisplayPort IP driver by allowing it to probe via a devicetree match. When probed via devicetree, it registers an OF PHY provider and maps its own MMIO regmap, while retaining the legacy behavior in mtk_dp_phy_legacy_probe().
quoted hunk ↗ jump to hunk
diff --git a/drivers/phy/mediatek/phy-mtk-dp.c b/drivers/phy/mediatek/phy-mtk-dp.c index bf7b3a95e72d2..d241004e28286 100644 --- a/drivers/phy/mediatek/phy-mtk-dp.c +++ b/drivers/phy/mediatek/phy-mtk-dp.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -160,43 +162,109 @@ static const struct phy_ops mtk_dp_phy_dev_ops = { .owner = THIS_MODULE, }; +static void mtk_dp_phy_legacy_remove_lookup(void *data) +{ + struct phy *phy = data; + struct mtk_dp_phy *dp_phy = phy_get_drvdata(phy); + + phy_remove_lookup(phy, "dp", dev_name(dp_phy->dev)); +} + +static int mtk_dp_phy_legacy_probe(struct platform_device *pdev, struct mtk_dp_phy *dp_phy) +{ + struct device *dev = &pdev->dev; + struct phy *phy; + int ret; + + dp_phy->regmap = *(struct regmap **)dev->platform_data; + if (!dp_phy->regmap) + return dev_err_probe(dev, -EINVAL, "No platform data available\n");
[Severity: High] This is a pre-existing issue, but is it possible for dev->platform_data to be NULL here? If the platform device is instantiated without platform_data (for example, via a sysfs manual bind or driver core testing), dereferencing it unconditionally before the validation check will cause a crash. The code currently does: dp_phy->regmap = *(struct regmap **)dev->platform_data; The subsequent check if (!dp_phy->regmap) cannot prevent the crash if dev->platform_data itself is NULL. Could we validate dev->platform_data before attempting to dereference it?
+ + phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops); + if (IS_ERR(phy)) + return dev_err_probe(dev, PTR_ERR(phy), + "Failed to create DP PHY\n");
[ ... ]
+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");
+ int ret;
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);
+
+ 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);[Severity: High] When mapping the dedicated MMIO region for the PHY in the new devicetree match path, will the existing hardcoded register offsets cause out-of-bounds accesses? Looking at the existing phy data structures in this driver, offsets such as .off_dig_glb = 0x1000 are used. These offsets appear to be relative to the DP IP base address from the legacy shared regmap. If the devicetree 'reg' property points to the PHY's actual physical base address, using these hardcoded offsets on top of the new dedicated MMIO mapping will overshoot the PHY registers by an additional 0x1000. Alternatively, if the devicetree tries to compensate by pointing to the DP IP base, wouldn't this cause a resource conflict (-EBUSY) with the DP driver's own devm_platform_ioremap_resource() call?
+ if (IS_ERR(dp_phy->regmap)) + return PTR_ERR(dp_phy->regmap); + + ret = devm_pm_runtime_enable(dev); + if (ret) + return ret; + phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260910092038.48291-1-angelogioacchino.delregno@collabora.com?part=3 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy