From: sashiko-bot@kernel.org
To: "AngeloGioacchino Del Regno" <angelogioacchino.delregno@collabora.com>
Cc: vkoul@kernel.org, neil.armstrong@linaro.org, olteanv@gmail.com,
linux-phy@lists.infradead.org, conor+dt@kernel.org,
dri-devel@lists.freedesktop.org, devicetree@vger.kernel.org,
robh@kernel.org
Subject: Re: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match
Date: Tue, 07 Jul 2026 16:06:46 +0000 [thread overview]
Message-ID: <20260707160646.813771F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707154245.198361-4-angelogioacchino.delregno@collabora.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Hardcoded `PHY_OFFSET` causes out-of-bounds register accesses or resource conflicts when the PHY is probed as a standalone devicetree node.
- [High] Hardware registers are accessed in `mtk_dp_phy_configure()` while the PHY device is runtime-suspended, causing a system crash or silent loss of configuration.
Pre-existing issues:
- [High] Dereferencing `dev->platform_data` before checking if it is NULL leads to a kernel crash.
--
--- Patch [3]: [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match ---
commit 710ab5488fd0b66b2ea62388b977a4308f99a0c6
Author: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
phy: phy-mtk-dp: Allow probing with devicetree match
Make it possible to decouple the registration of the DisplayPort
PHY driver from the DisplayPort IP driver by adding a devicetree
match to probe the PHY. In order to retain compatibility with older
devicetrees, the legacy code was moved in a mtk_dp_phy_legacy_probe()
function.
> 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
[ ... ]
> +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 does casting and dereferencing
dev->platform_data before checking if it is NULL lead to a kernel crash?
If the driver is bound to a device without platform data via sysfs, this
unvalidated pointer will be dereferenced before the check for
!dp_phy->regmap can execute.
[ ... ]
> static int mtk_dp_phy_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
[ ... ]
> 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]
Since the driver hardcodes PHY_OFFSET (0x1000) into its register macros like
MTK_DP_PHY_DIG_PLL_CTL_1, will this independent regmap cause out-of-bounds
accesses if the devicetree maps the PHY node to its true physical base?
If the devicetree instead sets the PHY node's reg to the parent DP IP base to
counteract this offset, won't this cause devm_platform_ioremap_resource() to
fail with -EBUSY due to resource conflicts between the PHY driver and the DP
IP driver?
> + if (IS_ERR(dp_phy->regmap))
> + return PTR_ERR(dp_phy->regmap);
> +
> + ret = devm_pm_runtime_enable(dev);
> + if (ret)
> + return ret;
[Severity: High]
By enabling runtime PM here, could mtk_dp_phy_configure() access hardware
registers while the PHY device is runtime-suspended?
The consumer driver calls phy_configure() without calling phy_power_on().
The PHY framework takes a runtime PM reference during .init and .reset,
but explicitly drops it right before returning, and does not take a reference
during .configure.
When the consumer calls phy_configure(), it writes directly to hardware via
regmap_write() and regmap_update_bits() while the device PM usage count
is zero. Can this crash the system or cause silent loss of configuration?
> +
> phy = devm_phy_create(dev, NULL, &mtk_dp_phy_dev_ops);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260707154245.198361-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 16:06 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-07 15:42 [PATCH v3 00/12] PHY: MediaTek DP PHY refactor and MT8196 eDP AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 01/12] dt-bindings: phy: Document MT8195 and MT8196 DisplayPort PHYs AngeloGioacchino Del Regno
2026-07-11 15:41 ` Krzysztof Kozlowski
2026-07-07 15:42 ` [PATCH v3 02/12] phy: phy-mtk-dp: Rename regs to regmap in struct mtk_dp_phy AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 03/12] phy: phy-mtk-dp: Allow probing with devicetree match AngeloGioacchino Del Regno
2026-07-07 16:06 ` sashiko-bot [this message]
2026-07-07 16:15 ` AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 04/12] phy: phy-mtk-dp: Migrate register offsets to SoC specific pdata AngeloGioacchino Del Regno
2026-07-07 15:56 ` sashiko-bot
2026-07-07 16:07 ` AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 05/12] phy: phy-mtk-dp: Implement power_on and power_off PHY callbacks AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 06/12] phy: phy-mtk-dp: Support set_lanes in configure and properly cleanup AngeloGioacchino Del Regno
2026-07-07 15:56 ` sashiko-bot
2026-07-07 15:42 ` [PATCH v3 07/12] phy: phy-mtk-dp: Support setting volt swing and preemphasis values AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 08/12] phy: phy-mtk-dp: Add support for digital and analog calibration AngeloGioacchino Del Regno
2026-07-07 16:03 ` sashiko-bot
2026-07-07 15:42 ` [PATCH v3 09/12] phy: phy-mtk-dp: Rewrite and document default driving param macros AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 10/12] phy: phy-mtk-dp: Add bitrate register val definitions to SoC data AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 11/12] phy: phy-mtk-dp: Add PHYD Lane EN register mask " AngeloGioacchino Del Regno
2026-07-07 15:42 ` [PATCH v3 12/12] phy: phy-mtk-dp: Add support for MT8196 eDP PHY AngeloGioacchino Del Regno
2026-07-07 16:05 ` sashiko-bot
2026-07-07 16:16 ` 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=20260707160646.813771F000E9@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