Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
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

  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