From: sashiko-bot@kernel.org
To: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
Cc: olteanv@gmail.com, neil.armstrong@linaro.org,
linux-phy@lists.infradead.org, vkoul@kernel.org, robh@kernel.org,
conor+dt@kernel.org, dri-devel@lists.freedesktop.org,
devicetree@vger.kernel.org
Subject: Re: [PATCH v2 03/12] phy: phy-mtk-dp: Allow probing with devicetree match
Date: Tue, 07 Jul 2026 14:31:47 +0000 [thread overview]
Message-ID: <20260707143147.AC76D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707141931.191172-4-angelogioacchino.delregno@collabora.com>
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
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
next prev parent reply other threads:[~2026-07-07 14:31 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 14:19 [PATCH v2 00/12] PHY: MediaTek DP PHY refactor and MT8196 eDP AngeloGioacchino Del Regno
2026-07-07 14:19 ` [PATCH v2 01/12] dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs AngeloGioacchino Del Regno
2026-07-07 14:35 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 02/12] phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy AngeloGioacchino Del Regno
2026-07-07 14:27 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 03/12] phy: phy-mtk-dp: Allow probing with devicetree match AngeloGioacchino Del Regno
2026-07-07 14:31 ` sashiko-bot [this message]
2026-07-07 14:19 ` [PATCH v2 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata AngeloGioacchino Del Regno
2026-07-07 14:34 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 05/12] phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks AngeloGioacchino Del Regno
2026-07-07 14:33 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup AngeloGioacchino Del Regno
2026-07-07 14:34 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 07/12] phy: phy-mtk-dp: Support setting volt swing and preemphasis values AngeloGioacchino Del Regno
2026-07-07 14:33 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration AngeloGioacchino Del Regno
2026-07-07 14:31 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 09/12] phy: phy-mtk-dp: Rewrite and document default driving param macros AngeloGioacchino Del Regno
2026-07-07 14:28 ` sashiko-bot
2026-07-07 14:19 ` [PATCH v2 10/12] phy: phy-mtk-dp: Add bitrate register val definitions to SoC data AngeloGioacchino Del Regno
2026-07-07 14:19 ` [PATCH v2 11/12] phy: phy-mtk-dp: Add PHYD Lane EN register mask " AngeloGioacchino Del Regno
2026-07-07 14:19 ` [PATCH v2 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY AngeloGioacchino Del Regno
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260707143147.AC76D1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=angelogioacchino.delregno@collabora.com \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-phy@lists.infradead.org \
--cc=neil.armstrong@linaro.org \
--cc=olteanv@gmail.com \
--cc=robh@kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=vkoul@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox