[PATCH v7 0/6] phy: fsl-imx8mq-usb: few improvements

[PATCH v7 0/6] phy: fsl-imx8mq-usb: few improvements

[Xu Yang]

Patch #1 fix Type-C switch resource leak if probe() fails.
Patch #3 add runtime PM support to avoid register access issue if the
      USB controller enters into runtime suspended state, in this state
      accessing USB PHY register may lack some resources. This will also
      avoid regulator leak if power_on() fails.
Patch #4 add debug control register regmap
Patch #5 add imx8mq_usb_phy_drvdata drvdata for better extension
Patch #6 correct i.MX8MP USB runtime wakeup issue after introduce runtime
      PM support.

---
Changes in v7:
- replase PM_RUNTIME_ACQUIRE()  with PM_RUNTIME_ACQUIRE_IF_ENABLED()
- use non-devm PM runtime callback finally, because the devm version will still cause the clock
  be disabled twice when probe() is failed or driver removed at device suspended state.
- Link to v6: https://patch.msgid.link/20260715-imx8mp-usb-phy-improvement-v6-0-00d95e270e4c@nxp.com

Changes in v6:
- use devm_pm_runtime_enable() to disable runtime PM if probe fails
- add imx8mq_usb_phy_drvdata drvdata and need_genpd_rpm_on flag for better extension
- Link to v5: https://patch.msgid.link/20260630-imx8mp-usb-phy-improvement-v5-0-25d616403844@nxp.com

Changes in v5:
- not use devm runtime callback to avoid clk enable/disable imblance
- Link to v4: https://patch.msgid.link/20260605-imx8mp-usb-phy-improvement-v4-0-b2ddf2f3862c@nxp.com

Changes in v4:
- add Rb tag
- replace guard() with PM_RUNTIME_ACQUIRE()
- Link to v3: https://patch.msgid.link/20260603-imx8mp-usb-phy-improvement-v3-0-7afb8f89abc6@nxp.com

Link to v2:
 - https://lore.kernel.org/linux-phy/20260512101046.1498096-1-xu.yang_2@nxp.com/
 - https://lore.kernel.org/linux-phy/20260512101212.1498223-1-xu.yang_2@nxp.com/

---
Felix Gu (1):
      phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

Xu Yang (5):
      phy: fsl-imx8mq-usb: set usb phy to be wakeup capable
      phy: fsl-imx8mq-usb: add runtime PM support
      phy: fsl-imx8mq-usb: add control register regmap
      phy: fsl-imx8mq-usb: introduce per-variant driver data structure
      phy: fsl-imx8mq-usb: keep PHY power domain runtime always-on for i.MX8MP

 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 186 +++++++++++++++++++++--------
 1 file changed, 137 insertions(+), 49 deletions(-)
---
base-commit: 3652b49adac266a3d27cb41cdfdb7d8790fc3633
change-id: 20260602-imx8mp-usb-phy-improvement-4272d308d862

Best regards,
--  
Xu Yang <xu.yang_2@nxp.com>


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 1/6] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

[Xu Yang]

From: Felix Gu <ustc.gu@gmail.com>

If probe fails after imx95_usb_phy_get_tca() succeeds, the typec
switch leaks because the only cleanup path was in .remove(), which
never runs on probe failure.

Use devm_add_action_or_reset() so the switch is cleaned up on both
probe failure and driver removal. The imx95_usb_phy_put_tca() is no
longer needed, it will be removed in .remove() too.

Fixes: b58f0f86fd61 ("phy: fsl-imx8mq-usb: add tca function driver for imx95")
Cc: stable@vger.kernel.org
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Xu Yang <xu.yang_2@nxp.com>
Signed-off-by: Felix Gu <ustc.gu@gmail.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v7:
 - no changes
Changes in v6:
 - delete .remove() is deleted words in commit message
Changes in v5:
 - keep remove() callback as patch #3 needs it
Changes in v4:
 - add my signed-off tag
Changes in v3:
 - add R-b tag
 - cc statble
 - drop "sw = data" conversion
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 21 +++++++--------------
 1 file changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index b05d80e849a1..9a33c06d6fc3 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -173,9 +173,9 @@ static struct typec_switch_dev *tca_blk_get_typec_switch(struct platform_device
 	return sw;
 }
 
-static void tca_blk_put_typec_switch(struct typec_switch_dev *sw)
+static void tca_blk_put_typec_switch(void *data)
 {
-	typec_switch_unregister(sw);
+	typec_switch_unregister(data);
 }
 
 static void tca_blk_orientation_set(struct tca_blk *tca,
@@ -248,6 +248,7 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct tca_blk *tca;
+	int ret;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
 	if (!res)
@@ -266,17 +267,11 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
 	tca->orientation = TYPEC_ORIENTATION_NORMAL;
 	tca->sw = tca_blk_get_typec_switch(pdev, imx_phy);
 
-	return tca;
-}
-
-static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
-{
-	struct tca_blk *tca = imx_phy->tca;
-
-	if (!tca)
-		return;
+	ret = devm_add_action_or_reset(&pdev->dev, tca_blk_put_typec_switch, tca->sw);
+	if (ret)
+		return ERR_PTR(ret);
 
-	tca_blk_put_typec_switch(tca->sw);
+	return tca;
 }
 
 static u32 phy_tx_vref_tune_from_property(u32 percent)
@@ -741,9 +736,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 
 static void imx8mq_usb_phy_remove(struct platform_device *pdev)
 {
-	struct imx8mq_usb_phy *imx_phy = platform_get_drvdata(pdev);
 
-	imx95_usb_phy_put_tca(imx_phy);
 }
 
 static struct platform_driver imx8mq_usb_phy_driver = {

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v7 1/6] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] A race condition during probe allows `imx_phy->tca` to be dereferenced by a Type-C consumer before it is actually assigned, leading to a NULL pointer dereference.
- [High] If the PHY device is unbound via sysfs, devm-managed structures are freed while Type-C consumers might still hold references to the switch, leading to a Use-After-Free.
--

--- Patch [1]: [PATCH v7 1/6] phy: fsl-imx8mq-usb: fix typec switch leak on probe error path ---
commit bdfb57c97d01c6ce3c92e388f5a6cf33401f0d53
Author: Felix Gu <ustc.gu@gmail.com>

phy: fsl-imx8mq-usb: fix typec switch leak on probe error path

This commit fixes a typec switch leak that occurs on the probe error path.
It transitions the cleanup mechanism to use devm_add_action_or_reset(),
ensuring the switch is properly unregistered on both probe failure and
driver removal. The manual imx95_usb_phy_put_tca() cleanup is completely
removed.

Cc: stable@vger.kernel.org

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index b05d80e849a14..9a33c06d6fc39 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
[ ... ]
> @@ -266,17 +267,11 @@ static struct tca_blk *imx95_usb_phy_get_tca(struct platform_device *pdev,
>  	tca->orientation = TYPEC_ORIENTATION_NORMAL;
>  	tca->sw = tca_blk_get_typec_switch(pdev, imx_phy);

[Severity: High]
This is a pre-existing issue, but does this create a race condition where
imx_phy->tca is dereferenced before it is assigned?

When tca_blk_get_typec_switch() calls typec_switch_register(), the switch
device is immediately exposed to the system. A Type-C port controller could
discover it and call typec_switch_set(), which triggers
tca_blk_typec_switch_set().

That callback reads imx_phy->tca:

tca_blk_typec_switch_set() {
    ...
    struct tca_blk *tca = imx_phy->tca;

    if (tca->orientation == orientation)
    ...
}

However, imx_phy->tca is not assigned until imx95_usb_phy_get_tca()
completes and returns to the probe function, leaving a window where the
callback might dereference a NULL pointer.

> -	return tca;
> -}
> -
> -static void imx95_usb_phy_put_tca(struct imx8mq_usb_phy *imx_phy)
> -{
> -	struct tca_blk *tca = imx_phy->tca;
> -
> -	if (!tca)
> -		return;
> +	ret = devm_add_action_or_reset(&pdev->dev, tca_blk_put_typec_switch, tca->sw);
> +	if (ret)
> +		return ERR_PTR(ret);

[Severity: High]
This isn't a bug introduced by this patch, but does relying on devm
allocation for imx_phy and tca lead to a use-after-free if the device is
unbound?

When the PHY device is unbound via sysfs, devres automatically frees the
imx_phy and tca structures. Although typec_switch_unregister() removes the
switch from the system, Type-C consumers that already acquired references
(holding a kref via typec_switch_get()) can still invoke the switch
callback.

If a consumer calls typec_switch_set() after unbind, it will trigger
tca_blk_typec_switch_set(), which accesses the freed memory:

tca_blk_typec_switch_set() {
    struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
    struct tca_blk *tca = imx_phy->tca;
    ...
}

> -	tca_blk_put_typec_switch(tca->sw);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-imx8mp-usb-phy-improvement-v7-0-c385ee40ade6@nxp.com?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 2/6] phy: fsl-imx8mq-usb: set usb phy to be wakeup capable

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

Set PHY wakeup capable because this PHY supports remote wakeup function.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v7:
 - no changes
Changes in v6:
 - no changes
Changes in v5:
 - no changes
Changes in v4:
 - add Rb tag
Changes in v3:
 - no changes
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 9a33c06d6fc3..3a5788c609e1 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -728,6 +728,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


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

Add runtime PM support to ensure the PHY clocks are properly gated
when the PHY is not in use, reducing power consumption.

Clock management is moved from power_on()/power_off() callbacks into
the runtime_resume()/runtime_suspend() callbacks respectively. The PHY
subsystem core already holds a runtime PM reference around init() and
power_on/off() calls, so no explicit clock handling is needed there.

Use devm_clk_get_enabled() and devm_clk_get_optional_enabled() in
probe() to keep clocks enabled initially. This ensures the PHY remains
functional when CONFIG_PM is disabled, where runtime suspend/resume
callbacks are never invoked.

In tca_blk_typec_switch_set(), replace the manual clk_prepare_enable()
/ clk_disable_unprepare() pair with PM_RUNTIME_ACQUIRE_IF_ENABLED() to
guard register access against a concurrently suspended PHY.

Move devm_regulator_get() before pm_runtime_enable() to avoid having
to clean up runtime PM state on regulator acquisition failure.

In remove(), call pm_runtime_get_sync() before pm_runtime_disable() to
ensure the PHY is resumed and clocks are enabled before the devres
teardown disables them.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v7:
 - replase PM_RUNTIME_ACQUIRE()  with PM_RUNTIME_ACQUIRE_IF_ENABLED()
 - use non-devm PM runtime callback because it's hard to avoid double clock
   disable issue when probe fails or remove the driver
 - improve commit message
Changes in v6:
 - use devm_pm_runtime_enable() to disable runtime PM when probe fails
 - simply pm_runtime_get_sync/disable/put_noidle() to pm_runtime_resume()
Changes in v5:
 - use non-devm PM runtime callback to correctly enable/disable clocks
   when unbind the device
Changes in v4:
 - replace guard() with PM_RUNTIME_ACQUIRE()
Changes in v3:
 - new patch
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 103 +++++++++++++++++++++--------
 1 file changed, 74 insertions(+), 29 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index 3a5788c609e1..74e665525ec4 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -9,6 +9,7 @@
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/usb/typec_mux.h>
 
@@ -136,17 +137,15 @@ static int tca_blk_typec_switch_set(struct typec_switch_dev *sw,
 {
 	struct imx8mq_usb_phy *imx_phy = typec_switch_get_drvdata(sw);
 	struct tca_blk *tca = imx_phy->tca;
-	int ret;
 
 	if (tca->orientation == orientation)
 		return 0;
 
-	ret = clk_prepare_enable(imx_phy->clk);
-	if (ret)
-		return ret;
+	PM_RUNTIME_ACQUIRE_IF_ENABLED(&imx_phy->phy->dev, pm);
+	if (PM_RUNTIME_ACQUIRE_ERR(&pm))
+		return -ENXIO;
 
 	tca_blk_orientation_set(tca, orientation);
-	clk_disable_unprepare(imx_phy->clk);
 
 	return 0;
 }
@@ -620,16 +619,6 @@ static int imx8mq_phy_power_on(struct phy *phy)
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(imx_phy->clk);
-	if (ret)
-		return ret;
-
-	ret = clk_prepare_enable(imx_phy->alt_clk);
-	if (ret) {
-		clk_disable_unprepare(imx_phy->clk);
-		return ret;
-	}
-
 	/* Disable rx term override */
 	value = readl(imx_phy->base + PHY_CTRL6);
 	value &= ~PHY_CTRL6_RXTERM_OVERRIDE_SEL;
@@ -648,8 +637,6 @@ static int imx8mq_phy_power_off(struct phy *phy)
 	value |= PHY_CTRL6_RXTERM_OVERRIDE_SEL;
 	writel(value, imx_phy->base + PHY_CTRL6);
 
-	clk_disable_unprepare(imx_phy->alt_clk);
-	clk_disable_unprepare(imx_phy->clk);
 	regulator_disable(imx_phy->vbus);
 
 	return 0;
@@ -686,6 +673,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct imx8mq_usb_phy *imx_phy;
 	const struct phy_ops *phy_ops;
+	int ret;
 
 	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
 	if (!imx_phy)
@@ -693,13 +681,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, imx_phy);
 
-	imx_phy->clk = devm_clk_get(dev, "phy");
+	imx_phy->clk = devm_clk_get_enabled(dev, "phy");
 	if (IS_ERR(imx_phy->clk)) {
 		dev_err(dev, "failed to get imx8mq usb phy clock\n");
 		return PTR_ERR(imx_phy->clk);
 	}
 
-	imx_phy->alt_clk = devm_clk_get_optional(dev, "alt");
+	imx_phy->alt_clk = devm_clk_get_optional_enabled(dev, "alt");
 	if (IS_ERR(imx_phy->alt_clk))
 		return dev_err_probe(dev, PTR_ERR(imx_phy->alt_clk),
 				    "Failed to get alt clk\n");
@@ -708,44 +696,101 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->base))
 		return PTR_ERR(imx_phy->base);
 
+	imx_phy->vbus = devm_regulator_get(dev, "vbus");
+	if (IS_ERR(imx_phy->vbus))
+		return dev_err_probe(dev, PTR_ERR(imx_phy->vbus), "failed to get vbus\n");
+
 	phy_ops = of_device_get_match_data(dev);
 	if (!phy_ops)
 		return -EINVAL;
 
-	imx_phy->phy = devm_phy_create(dev, NULL, phy_ops);
-	if (IS_ERR(imx_phy->phy))
-		return PTR_ERR(imx_phy->phy);
+	pm_runtime_set_active(dev);
+	pm_runtime_enable(dev);
 
-	imx_phy->vbus = devm_regulator_get(dev, "vbus");
-	if (IS_ERR(imx_phy->vbus))
-		return dev_err_probe(dev, PTR_ERR(imx_phy->vbus), "failed to get vbus\n");
+	imx_phy->phy = devm_phy_create(dev, NULL, phy_ops);
+	if (IS_ERR(imx_phy->phy)) {
+		ret = dev_err_probe(dev, PTR_ERR(imx_phy->phy),
+				    "failed to create PHY\n");
+		goto disable_rpm;
+	}
 
 	phy_set_drvdata(imx_phy->phy, imx_phy);
 
 	imx_phy->tca = imx95_usb_phy_get_tca(pdev, imx_phy);
-	if (IS_ERR(imx_phy->tca))
-		return dev_err_probe(dev, PTR_ERR(imx_phy->tca),
-					"failed to get tca\n");
+	if (IS_ERR(imx_phy->tca)) {
+		ret = dev_err_probe(dev, PTR_ERR(imx_phy->tca),
+				    "failed to get tca\n");
+		goto disable_rpm;
+	}
 
 	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);
+	if (IS_ERR(phy_provider)) {
+		ret = dev_err_probe(dev, PTR_ERR(phy_provider),
+				    "failed to register phy provider\n");
+		goto disable_rpm;
+	}
 
-	return PTR_ERR_OR_ZERO(phy_provider);
+	return 0;
+
+disable_rpm:
+	pm_runtime_disable(dev);
+	return ret;
 }
 
 static void imx8mq_usb_phy_remove(struct platform_device *pdev)
 {
+	struct device *dev = &pdev->dev;
+	int ret;
+
+	ret = pm_runtime_get_sync(dev);
+	if (ret)
+		dev_warn(dev, "failed to resume on remove: %d\n", ret);
+
+	pm_runtime_disable(dev);
+	pm_runtime_put_noidle(dev);
+}
+
+static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
+{
+	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(imx_phy->alt_clk);
+	clk_disable_unprepare(imx_phy->clk);
+
+	return 0;
+}
+
+static int imx8mq_usb_phy_runtime_resume(struct device *dev)
+{
+	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(imx_phy->clk);
+	if (ret)
+		return ret;
 
+	ret = clk_prepare_enable(imx_phy->alt_clk);
+	if (ret) {
+		clk_disable_unprepare(imx_phy->clk);
+		return ret;
+	}
+
+	return 0;
 }
 
+static DEFINE_RUNTIME_DEV_PM_OPS(imx8mq_usb_phy_pm_ops, imx8mq_usb_phy_runtime_suspend,
+				 imx8mq_usb_phy_runtime_resume, NULL);
+
 static struct platform_driver imx8mq_usb_phy_driver = {
 	.probe	= imx8mq_usb_phy_probe,
 	.remove = imx8mq_usb_phy_remove,
 	.driver = {
 		.name	= "imx8mq-usb-phy",
 		.of_match_table	= imx8mq_usb_phy_of_match,
+		.pm = pm_ptr(&imx8mq_usb_phy_pm_ops),
 		.suppress_bind_attrs = true,
 	}
 };

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect error checking of pm_runtime_get_sync() return value causes spurious warnings when the device is already active.
- [Low] Failure of pm_runtime_get_sync() in remove() leads to an unbalanced clock disable by devres, triggering a kernel WARN.
--

--- Patch [3]: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support ---
commit f054765f7ebd59735a4592c4d68b74584abdbeeb
Author: Xu Yang <xu.yang_2@nxp.com>

phy: fsl-imx8mq-usb: add runtime PM support

Add runtime PM support to ensure the PHY clocks are properly gated
when the PHY is not in use, reducing power consumption. Clock
management is moved from power_on()/power_off() callbacks into the
runtime_resume()/runtime_suspend() callbacks.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 3a5788c609e1b..74e665525ec48 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
[ ... ]
> @@ -708,44 +696,101 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
[ ... ]
>  static void imx8mq_usb_phy_remove(struct platform_device *pdev)
>  {
> +	struct device *dev = &pdev->dev;
> +	int ret;
> +
> +	ret = pm_runtime_get_sync(dev);
> +	if (ret)
> +		dev_warn(dev, "failed to resume on remove: %d\n", ret);

[Severity: Medium]
Is it intended to warn when pm_runtime_get_sync() returns 1?

Since pm_runtime_get_sync() returns 1 when the device is already active
(or when CONFIG_PM is disabled), checking "if (ret)" instead of
"if (ret < 0)" will trigger this warning even on successful unbinds
where the device was already active.

> +
> +	pm_runtime_disable(dev);
> +	pm_runtime_put_noidle(dev);
>  }

[Severity: Low]
Could a failure in pm_runtime_get_sync() lead to a clock enable count
underflow?

The clocks are acquired using devm_clk_get_enabled() in probe(), which
registers a devres cleanup action to disable the clocks on teardown. If
the runtime resume fails here in remove(), the clocks will remain disabled
by the runtime PM framework. When devres subsequently attempts to disable
them again during cleanup, it will cause an unbalanced clock disable and
a kernel WARN.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-imx8mp-usb-phy-improvement-v7-0-c385ee40ade6@nxp.com?part=3

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support

[Xu Yang]

On Wed, Jul 29, 2026 at 06:55:20AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Incorrect error checking of pm_runtime_get_sync() return value causes spurious warnings when the device is already active.
> - [Low] Failure of pm_runtime_get_sync() in remove() leads to an unbalanced clock disable by devres, triggering a kernel WARN.
> --
> 
> --- Patch [3]: [PATCH v7 3/6] phy: fsl-imx8mq-usb: add runtime PM support ---
> commit f054765f7ebd59735a4592c4d68b74584abdbeeb
> Author: Xu Yang <xu.yang_2@nxp.com>
> 
> phy: fsl-imx8mq-usb: add runtime PM support
> 
> Add runtime PM support to ensure the PHY clocks are properly gated
> when the PHY is not in use, reducing power consumption. Clock
> management is moved from power_on()/power_off() callbacks into the
> runtime_resume()/runtime_suspend() callbacks.
> 
> > diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > index 3a5788c609e1b..74e665525ec48 100644
> > --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> > +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> [ ... ]
> > @@ -708,44 +696,101 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
> [ ... ]
> >  static void imx8mq_usb_phy_remove(struct platform_device *pdev)
> >  {
> > +	struct device *dev = &pdev->dev;
> > +	int ret;
> > +
> > +	ret = pm_runtime_get_sync(dev);
> > +	if (ret)
> > +		dev_warn(dev, "failed to resume on remove: %d\n", ret);
> 
> [Severity: Medium]
> Is it intended to warn when pm_runtime_get_sync() returns 1?
> 
> Since pm_runtime_get_sync() returns 1 when the device is already active
> (or when CONFIG_PM is disabled), checking "if (ret)" instead of
> "if (ret < 0)" will trigger this warning even on successful unbinds
> where the device was already active.

My bad. It should be "if (ret < 0)". Will fix it in v8.

Thanks,
Xu Yang

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 4/6] phy: fsl-imx8mq-usb: add control register regmap

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

The CR port is a simple 16-bit data/address parallel port that is
accessed through 32-bit MMIO registers for on-chip access to the
control registers inside the USB 3.0 femtoPHY. Add control register
regmap and export these registers by debugfs to help PHY's diagnostic.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v7:
 - no changes
Changes in v6:
 - no changes
Changes in v5:
 - add Rb tag
Changes in v4:
 - improve commit message as Haibo's suggestion
Changes in v3:
 - drop Frank's tag because it includes other changes
 - new patch
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index 74e665525ec4..83d5a44c4084 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0+
-/* Copyright (c) 2017 NXP. */
+/* Copyright 2017-2026 NXP. */
 
 #include <linux/bitfield.h>
 #include <linux/clk.h>
@@ -11,6 +11,7 @@
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/regmap.h>
 #include <linux/usb/typec_mux.h>
 
 #define PHY_CTRL0			0x0
@@ -56,6 +57,8 @@
 #define PHY_CTRL6_ALT_CLK_EN		BIT(1)
 #define PHY_CTRL6_ALT_CLK_SEL		BIT(0)
 
+#define PHY_CRCTL			0x30
+
 #define PHY_TUNE_DEFAULT		0xffffffff
 
 #define TCA_CLK_RST			0x00
@@ -119,6 +122,7 @@ struct imx8mq_usb_phy {
 	void __iomem *base;
 	struct regulator *vbus;
 	struct tca_blk *tca;
+	struct regmap *cr_regmap;
 	u32 pcs_tx_swing_full;
 	u32 pcs_tx_deemph_3p5db;
 	u32 tx_vref_tune;
@@ -667,6 +671,14 @@ static const struct of_device_id imx8mq_usb_phy_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
 
+static const struct regmap_config imx_cr_regmap_config = {
+	.name = "cr",
+	.reg_bits = 32,
+	.val_bits = 32,
+	.reg_stride = 4,
+	.max_register = 0x7,
+};
+
 static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 {
 	struct phy_provider *phy_provider;
@@ -696,6 +708,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->base))
 		return PTR_ERR(imx_phy->base);
 
+	imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
+						   &imx_cr_regmap_config);
+	if (IS_ERR(imx_phy->cr_regmap)) {
+		dev_warn(dev, "Fail to init debug register regmap\n");
+		imx_phy->cr_regmap = NULL;
+	}
+
 	imx_phy->vbus = devm_regulator_get(dev, "vbus");
 	if (IS_ERR(imx_phy->vbus))
 		return dev_err_probe(dev, PTR_ERR(imx_phy->vbus), "failed to get vbus\n");
@@ -757,6 +776,9 @@ static int imx8mq_usb_phy_runtime_suspend(struct device *dev)
 {
 	struct imx8mq_usb_phy *imx_phy = dev_get_drvdata(dev);
 
+	if (imx_phy->cr_regmap)
+		regcache_cache_only(imx_phy->cr_regmap, true);
+
 	clk_disable_unprepare(imx_phy->alt_clk);
 	clk_disable_unprepare(imx_phy->clk);
 
@@ -778,6 +800,9 @@ static int imx8mq_usb_phy_runtime_resume(struct device *dev)
 		return ret;
 	}
 
+	if (imx_phy->cr_regmap)
+		regcache_cache_only(imx_phy->cr_regmap, false);
+
 	return 0;
 }
 

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v7 4/6] phy: fsl-imx8mq-usb: add control register regmap

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incorrect error handling of pm_runtime_get_sync() treats a successful active state as a failure.
- [Low] Missing Kconfig dependency for REGMAP_MMIO.
--

--- Patch [4]: [PATCH v7 4/6] phy: fsl-imx8mq-usb: add control register regmap ---
commit 3be8b97176fdd5e159a3f1a8541ee32a8755658b
Author: Xu Yang <xu.yang_2@nxp.com>

phy: fsl-imx8mq-usb: add control register regmap

This commit adds a 32-bit MMIO control register regmap for on-chip access
to the control registers inside the USB 3.0 femtoPHY. It exports these
registers via debugfs to help with PHY diagnostic.

> diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> index 74e665525ec48..83d5a44c40848 100644
> --- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
> +++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
[ ... ]
> @@ -696,6 +708,13 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
>  	if (IS_ERR(imx_phy->base))
>  		return PTR_ERR(imx_phy->base);
>  
> +	imx_phy->cr_regmap = devm_regmap_init_mmio(dev, imx_phy->base + PHY_CRCTL,
> +						   &imx_cr_regmap_config);

[Severity: Low]
Does this new regmap initialization require a Kconfig update?

Since the driver now uses devm_regmap_init_mmio(), it depends on the MMIO
regmap subsystem. However, PHY_FSL_IMX8MQ_USB in drivers/phy/freescale/Kconfig
does not select REGMAP_MMIO.

Could this lead to build failures (such as missing module or undefined
reference to __devm_regmap_init_mmio_clk) in configurations where no other
component selects REGMAP_MMIO?

> +	if (IS_ERR(imx_phy->cr_regmap)) {
> +		dev_warn(dev, "Fail to init debug register regmap\n");
> +		imx_phy->cr_regmap = NULL;
> +	}
> +
[ ... ]

[Severity: Medium]
Looking at the driver's unbind path in imx8mq_usb_phy_remove():

static void imx8mq_usb_phy_remove(struct platform_device *pdev)
{
	...
	ret = pm_runtime_get_sync(dev);
	if (ret)
		dev_warn(dev, "failed to resume on remove: %d\n", ret);

According to the PM subsystem rules, pm_runtime_get_sync() returns 1 when the
device is already active. Does checking if (ret) instead of if (ret < 0)
treat this success condition as a failure, potentially causing a spurious
kernel warning during driver unbind?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729-imx8mp-usb-phy-improvement-v7-0-c385ee40ade6@nxp.com?part=4

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 5/6] phy: fsl-imx8mq-usb: introduce per-variant driver data structure

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

Replace direct use of phy_ops pointer in of_device_id .data with a
dedicated imx8mq_usb_phy_drvdata structure. This allows per-variant
driver data to be extended in the future without changing the match
table.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
---
Changes in v7:
 - no changes
Changes in v6:
 - new patch
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index 83d5a44c4084..eda6064de005 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -132,6 +132,9 @@ struct imx8mq_usb_phy {
 	u32 comp_dis_tune;
 };
 
+struct imx8mq_usb_phy_drvdata {
+	const struct phy_ops *ops;
+};
 
 static void tca_blk_orientation_set(struct tca_blk *tca,
 				enum typec_orientation orientation);
@@ -660,13 +663,25 @@ static const struct phy_ops imx8mp_usb_phy_ops = {
 	.owner		= THIS_MODULE,
 };
 
+static const struct imx8mq_usb_phy_drvdata imx8mq_usb_phy_data = {
+	.ops = &imx8mq_usb_phy_ops,
+};
+
+static const struct imx8mq_usb_phy_drvdata imx8mp_usb_phy_data = {
+	.ops = &imx8mp_usb_phy_ops,
+};
+
+static const struct imx8mq_usb_phy_drvdata imx95_usb_phy_data = {
+	.ops = &imx8mp_usb_phy_ops,
+};
+
 static const struct of_device_id imx8mq_usb_phy_of_match[] = {
 	{.compatible = "fsl,imx8mq-usb-phy",
-	 .data = &imx8mq_usb_phy_ops,},
+	 .data = &imx8mq_usb_phy_data,},
 	{.compatible = "fsl,imx8mp-usb-phy",
-	 .data = &imx8mp_usb_phy_ops,},
+	 .data = &imx8mp_usb_phy_data,},
 	{.compatible = "fsl,imx95-usb-phy",
-	 .data = &imx8mp_usb_phy_ops,},
+	 .data = &imx95_usb_phy_data,},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, imx8mq_usb_phy_of_match);
@@ -684,7 +699,7 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	struct phy_provider *phy_provider;
 	struct device *dev = &pdev->dev;
 	struct imx8mq_usb_phy *imx_phy;
-	const struct phy_ops *phy_ops;
+	const struct imx8mq_usb_phy_drvdata *phy_data;
 	int ret;
 
 	imx_phy = devm_kzalloc(dev, sizeof(*imx_phy), GFP_KERNEL);
@@ -719,14 +734,14 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (IS_ERR(imx_phy->vbus))
 		return dev_err_probe(dev, PTR_ERR(imx_phy->vbus), "failed to get vbus\n");
 
-	phy_ops = of_device_get_match_data(dev);
-	if (!phy_ops)
+	phy_data = of_device_get_match_data(dev);
+	if (!phy_data)
 		return -EINVAL;
 
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 
-	imx_phy->phy = devm_phy_create(dev, NULL, phy_ops);
+	imx_phy->phy = devm_phy_create(dev, NULL, phy_data->ops);
 	if (IS_ERR(imx_phy->phy)) {
 		ret = dev_err_probe(dev, PTR_ERR(imx_phy->phy),
 				    "failed to create PHY\n");

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v7 6/6] phy: fsl-imx8mq-usb: keep PHY power domain runtime always-on for i.MX8MP

[Xu Yang]

From: Xu Yang <xu.yang_2@nxp.com>

On i.MX8MP, the USB PHY has a dedicated power domain that was previously
never powered off at runtime. With the introduction of runtime PM support,
the power domain will be powered off if the device is runtime suspended,
which breaks USB wakeup functionality.

To preserve wakeup functionality, mark the PHY power domain as runtime
always-on for i.MX8MP platform. To limit the behavior to i.MX8MP, add a
need_genpd_rpm_on to imx8mq_usb_phy_drvdata and set it as true for i.MX8MP.

Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v7:
 - replace dev_err_probe() with dev_warn()
Changes in v6:
 - set need_genpd_rpm_on flag for i.MX8MP and call dev_pm_genpd_rpm_always_on()
   if the flag is true
Changes in v5:
 - no changes
Changes in v4:
 - no changes
Changes in v3:
 - new patch
---
 drivers/phy/freescale/phy-fsl-imx8mq-usb.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
index eda6064de005..f4260d6b85fe 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -9,6 +9,7 @@
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
 #include <linux/regmap.h>
@@ -134,6 +135,7 @@ struct imx8mq_usb_phy {
 
 struct imx8mq_usb_phy_drvdata {
 	const struct phy_ops *ops;
+	bool need_genpd_rpm_on;
 };
 
 static void tca_blk_orientation_set(struct tca_blk *tca,
@@ -669,6 +671,7 @@ static const struct imx8mq_usb_phy_drvdata imx8mq_usb_phy_data = {
 
 static const struct imx8mq_usb_phy_drvdata imx8mp_usb_phy_data = {
 	.ops = &imx8mp_usb_phy_ops,
+	.need_genpd_rpm_on = true,
 };
 
 static const struct imx8mq_usb_phy_drvdata imx95_usb_phy_data = {
@@ -738,6 +741,12 @@ static int imx8mq_usb_phy_probe(struct platform_device *pdev)
 	if (!phy_data)
 		return -EINVAL;
 
+	if (phy_data->need_genpd_rpm_on) {
+		ret = dev_pm_genpd_rpm_always_on(dev, true);
+		if (ret && ret != -EOPNOTSUPP)
+			dev_warn(dev, "failed to set genpd rpm always on\n");
+	}
+
 	pm_runtime_set_active(dev);
 	pm_runtime_enable(dev);
 

-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy