* [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
@ 2026-05-12 10:10 Xu Yang
2026-05-12 15:03 ` Frank Li
2026-05-13 20:28 ` sashiko-bot
0 siblings, 2 replies; 3+ messages in thread
From: Xu Yang @ 2026-05-12 10:10 UTC (permalink / raw)
To: vkoul, neil.armstrong, Frank.Li, s.hauer, kernel, festevam,
jun.li, a.fatoum, franz.schnyder, stefano.radaelli21, linux-phy,
imx, linux-arm-kernel, linux-kernel
USB remote wakeup need its PHY power domain to be active,
so set PHY to be wakeup capable.
Signed-off-by: Li Jun <jun.li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v2:
- no changes
---
drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index 958d114b0c83..50b67f1e6a90 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -750,6 +750,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
"failed to get tca\n");
imx8m_get_phy_tuning_data(imx_phy);
+ device_set_wakeup_capable(dev, true);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
2026-05-12 10:10 [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable Xu Yang
@ 2026-05-12 15:03 ` Frank Li
2026-05-13 20:28 ` sashiko-bot
1 sibling, 0 replies; 3+ messages in thread
From: Frank Li @ 2026-05-12 15:03 UTC (permalink / raw)
To: Xu Yang
Cc: vkoul, neil.armstrong, s.hauer, kernel, festevam, jun.li,
a.fatoum, franz.schnyder, stefano.radaelli21, linux-phy, imx,
linux-arm-kernel, linux-kernel
On Tue, May 12, 2026 at 06:10:46PM +0800, Xu Yang wrote:
> USB remote wakeup need its PHY power domain to be active,
> so set PHY to be wakeup capable.
The reason should usb phy support USB wakeup.
"Set PHY wakeup capable because this PHY supports remote wakeup function." ?
Frank
>
> Signed-off-by: Li Jun <jun.li@nxp.com>
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
>
> ---
> Changes in v2:
> - no changes
> ---
> drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 958d114b0c83..50b67f1e6a90 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -750,6 +750,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> "failed to get tca\n");
>
> imx8m_get_phy_tuning_data(imx_phy);
> + device_set_wakeup_capable(dev, true);
>
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
2026-05-12 10:10 [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable Xu Yang
2026-05-12 15:03 ` Frank Li
@ 2026-05-13 20:28 ` sashiko-bot
1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-05-13 20:28 UTC (permalink / raw)
To: Xu Yang; +Cc: olteanv, vkoul, Frank.Li, imx, neil.armstrong, linux-phy
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] Missing error handling in the probe function leads to a Type-C switch leak and a critical Use-After-Free.
- [High] Using device_set_wakeup_capable() fails to enable wakeup by default, causing the PHY power domain to still be powered down during suspend.
- [High] Failing to enable the clocks in imx8mq_phy_power_on() leaves the vbus regulator permanently enabled.
--
commit c0b045ff9746b6affbecc02df5970fc02bc6a3ed
Author: Xu Yang <xu.yang_2@nxp.com>
phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
The patch intends to keep the PHY power domain active during suspend
by setting the PHY to be wakeup capable.
> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a14..b2e1e3749fc19 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> @@ -733,6 +733,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> "failed to get tca\n");
>
> imx8m_get_phy_tuning_data(imx_phy);
> + device_set_wakeup_capable(dev, true);
Will this successfully enable wakeup by default?
The PM domain framework (genpd) checks genpd_is_active_wakeup(genpd) and
device_awake_path(dev) (which depends on device_may_wakeup(dev)) to determine
if the domain should remain powered on.
device_may_wakeup(dev) evaluates to dev->power.can_wakeup &&
dev->power.should_wakeup. While device_set_wakeup_capable(dev, true) sets
can_wakeup = true, it leaves should_wakeup = false. As a result,
device_may_wakeup() returns false by default, and genpd will still power off
the PHY domain.
Should this use device_init_wakeup(dev, true) to enable it by default,
or should the USB subsystem dynamically propagate the controller's wakeup state
to the PHY instead?
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
This isn't a bug introduced by this patch, but does this code leave a Type-C
switch registered if devm_of_phy_provider_register() fails?
If devm_of_phy_provider_register() returns an error, the probe function exits
without unregistering the Type-C switch (e.g., via imx95_usb_phy_put_tca()).
The imx_phy structure allocated with devm_kzalloc() would then be freed by the
driver core, but the switch remains active.
If the Type-C subsystem triggers the switch's set callback
(tca_blk_typec_switch_set()), it will access the freed imx_phy,
potentially resulting in a use-after-free.
>
> return PTR_ERR_OR_ZERO(phy_provider);
> }
This isn't a bug introduced by this patch, but does imx8mq_phy_power_on() leak
a regulator reference on error paths?
Looking at imx8mq_phy_power_on():
drivers/phy/freescale/phy-fsl-imx8mq-usb.c:imx8mq_phy_power_on() {
...
ret = regulator_enable(imx_phy->vbus);
if (ret)
return ret;
ret = clk_prepare_enable(imx_phy->clk);
if (ret)
return ret;
...
}
If clk_prepare_enable(imx_phy->clk) or clk_prepare_enable(imx_phy->alt_clk)
fails, the function returns directly without calling
regulator_disable(imx_phy->vbus).
Since the function returns an error, the PHY framework will not call
imx8mq_phy_power_off(). Could this leave the vbus regulator permanently
enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260512101046.1498096-1-xu.yang_2@nxp.com?part=1
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-13 20:28 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-12 10:10 [PATCH v2] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable Xu Yang
2026-05-12 15:03 ` Frank Li
2026-05-13 20:28 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox