* [PATCH AUTOSEL 6.19-6.12] phy: cadence-torrent: restore parent clock for refclk during resume
[not found] <20260219020422.1539798-1-sashal@kernel.org>
@ 2026-02-19 2:03 ` Sasha Levin
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-6.12] phy: ti: phy-j721e-wiz: restore mux selection " Sasha Levin
` (2 subsequent siblings)
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19 2:03 UTC (permalink / raw)
To: patches, stable
Cc: Thomas Richard (TI.com), Neil Armstrong, Vinod Koul, Sasha Levin,
linux-phy, linux-kernel
From: "Thomas Richard (TI.com)" <thomas.richard@bootlin.com>
[ Upstream commit 434e1a0ee145d0389b192252be4c993f86cf1134 ]
While suspend and resume, parent clock config for refclk was getting lost.
So save and restore it in suspend and resume operations.
Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>
Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
Link: https://patch.msgid.link/20251216-phy-cadence-torrent-resume-restore-refclk-parent-v3-1-8a7ed84b47e3@bootlin.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
The verification confirms all the key elements:
1. **`cdns_torrent_refclk_driver_get_parent` and
`cdns_torrent_refclk_driver_set_parent`** exist and are well-defined
clock operations that read/write register fields for the clock mux
configuration.
2. **`CDNS_TORRENT_REFCLK_DRIVER`** is defined as index `0` in
`include/dt-bindings/phy/phy-cadence.h` and is used to index into
`clk_hw_data->hws[]`.
3. **Suspend/resume was introduced in commit `0da27ed080b2c`** (April
2024) by the same author (Thomas Richard). This means the
suspend/resume feature already exists in recent kernels, and this is
a bug fix to that existing feature.
4. All structures and fields referenced by the new code are actively
used in the driver.
## Verification Summary
- **Verified**: `cdns_torrent_refclk_driver_get_parent()` and
`cdns_torrent_refclk_driver_set_parent()` exist and operate on
register fields via regmap - these are the standard clock parent
operations.
- **Verified**: `CDNS_TORRENT_REFCLK_DRIVER` = 0, used as clock index in
`clk_hw_data->hws[]`.
- **Verified**: Suspend/resume functions were added in commit
`0da27ed080b2c` (April 2024) by the same author - this is a follow-up
fix to that code.
- **Verified**: The `parent_index` field and new suspend/resume helpers
only use existing, proven APIs.
- **Verified**: The patch is reviewed by Neil Armstrong and signed off
by the PHY subsystem maintainer Vinod Koul.
- **Note**: The suspend/resume support was introduced in v6.11 merge
window timeframe (April 2024), so this fix is relevant for stable
trees that include that commit.
## Classification
This is a **clear bug fix** for suspend/resume functionality:
- **Bug type**: State loss during power transitions (clock parent
configuration lost)
- **Impact**: PHY may not function correctly after resume, potentially
breaking PCIe, USB, or DisplayPort links
- **Scope**: Small, contained, single-driver fix (~22 lines)
- **Risk**: Very low - uses existing APIs, standard save/restore pattern
- **Dependencies**: Requires the suspend/resume support commit
(`0da27ed080b2c`) to be present in the stable tree
## Conclusion
This commit fixes a real suspend/resume bug where clock parent
configuration is lost, causing the PHY to malfunction after resume. The
fix is small, obviously correct, well-reviewed, and follows standard
kernel suspend/resume patterns. It meets all stable kernel criteria:
fixes a real bug, is small and contained, introduces no new features,
and has minimal regression risk.
The only consideration is that the stable tree must already contain the
initial suspend/resume support (commit `0da27ed080b2c` from ~April
2024), which limits this to newer stable branches (6.11+).
**YES**
drivers/phy/cadence/phy-cadence-torrent.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/phy/cadence/phy-cadence-torrent.c b/drivers/phy/cadence/phy-cadence-torrent.c
index 37fa4bad6bd72..877f22177c699 100644
--- a/drivers/phy/cadence/phy-cadence-torrent.c
+++ b/drivers/phy/cadence/phy-cadence-torrent.c
@@ -397,6 +397,7 @@ struct cdns_torrent_refclk_driver {
struct clk_hw hw;
struct regmap_field *cmn_fields[REFCLK_OUT_NUM_CMN_CONFIG];
struct clk_init_data clk_data;
+ u8 parent_index;
};
#define to_cdns_torrent_refclk_driver(_hw) \
@@ -3326,11 +3327,29 @@ static const struct cdns_torrent_vals sgmii_qsgmii_xcvr_diag_ln_vals = {
.num_regs = ARRAY_SIZE(sgmii_qsgmii_xcvr_diag_ln_regs),
};
+static void cdns_torrent_refclk_driver_suspend(struct cdns_torrent_phy *cdns_phy)
+{
+ struct clk_hw *hw = cdns_phy->clk_hw_data->hws[CDNS_TORRENT_REFCLK_DRIVER];
+ struct cdns_torrent_refclk_driver *refclk_driver = to_cdns_torrent_refclk_driver(hw);
+
+ refclk_driver->parent_index = cdns_torrent_refclk_driver_get_parent(hw);
+}
+
+static int cdns_torrent_refclk_driver_resume(struct cdns_torrent_phy *cdns_phy)
+{
+ struct clk_hw *hw = cdns_phy->clk_hw_data->hws[CDNS_TORRENT_REFCLK_DRIVER];
+ struct cdns_torrent_refclk_driver *refclk_driver = to_cdns_torrent_refclk_driver(hw);
+
+ return cdns_torrent_refclk_driver_set_parent(hw, refclk_driver->parent_index);
+}
+
static int cdns_torrent_phy_suspend_noirq(struct device *dev)
{
struct cdns_torrent_phy *cdns_phy = dev_get_drvdata(dev);
int i;
+ cdns_torrent_refclk_driver_suspend(cdns_phy);
+
reset_control_assert(cdns_phy->phy_rst);
reset_control_assert(cdns_phy->apb_rst);
for (i = 0; i < cdns_phy->nsubnodes; i++)
@@ -3352,6 +3371,10 @@ static int cdns_torrent_phy_resume_noirq(struct device *dev)
int node = cdns_phy->nsubnodes;
int ret, i;
+ ret = cdns_torrent_refclk_driver_resume(cdns_phy);
+ if (ret)
+ return ret;
+
ret = cdns_torrent_clk(cdns_phy);
if (ret)
return ret;
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.19-6.12] phy: ti: phy-j721e-wiz: restore mux selection during resume
[not found] <20260219020422.1539798-1-sashal@kernel.org>
2026-02-19 2:03 ` [PATCH AUTOSEL 6.19-6.12] phy: cadence-torrent: restore parent clock for refclk during resume Sasha Levin
@ 2026-02-19 2:04 ` Sasha Levin
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-5.10] phy: fsl-imx8mq-usb: disable bind/unbind platform driver feature Sasha Levin
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-5.15] phy: mvebu-cp110-utmi: fix dr_mode property read from dts Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19 2:04 UTC (permalink / raw)
To: patches, stable
Cc: Thomas Richard (TI.com), Vinod Koul, Sasha Levin, linux-phy,
linux-kernel
From: "Thomas Richard (TI.com)" <thomas.richard@bootlin.com>
[ Upstream commit 53f6240e88c9e8715e09fc19942f13450db4cb33 ]
While suspend and resume mux selection was getting lost. So save and
restore these values in suspend and resume operations.
Signed-off-by: Thomas Richard (TI.com) <thomas.richard@bootlin.com>
Link: https://patch.msgid.link/20251216-phy-ti-phy-j721e-wiz-resume-restore-mux-sel-v1-1-771d564db966@bootlin.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Good. The current tree has the resume function but no suspend function
(the suspend slot is NULL), which matches the pre-patch state in the
diff.
## Analysis Summary
### What the commit does:
1. **Adds a new field** `mux_sel_status[WIZ_MUX_NUM_CLOCKS]` (3 ints) to
the `struct wiz` to store mux selection state.
2. **Adds a `wiz_suspend_noirq()` callback** that reads the 3 mux
selection values from hardware registers before suspend, saving them
in the new array.
3. **Modifies `wiz_resume_noirq()`** to write the saved mux selection
values back to hardware registers at the beginning of resume, before
the rest of the initialization.
4. **Updates the PM ops macro** from
`DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, NULL, wiz_resume_noirq)` to
`DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, wiz_suspend_noirq,
wiz_resume_noirq)`.
### Bug being fixed:
The mux clock selection values (which control how the SERDES reference
clocks are routed) are lost during suspend because the hardware state is
not preserved. On resume, `wiz_resume_noirq()` called `wiz_clock_init()`
and `wiz_init()`, but these re-initialize the clocks to default values
rather than restoring the user/driver-configured mux selections. This
means after resume, the SERDES PHY may not work correctly because the
clock routing is wrong.
### Classification: **Suspend/resume bug fix**
This is a real hardware bug that affects users who suspend and resume
systems using the TI J721E SERDES PHY (common in TI K3 SoC
automotive/industrial platforms). Without this fix, the PHY may not
function correctly after resume, potentially breaking network, PCIe,
USB, or other SERDES-based interfaces.
### Stable kernel rule compliance:
1. **Obviously correct and tested**: Simple save/restore pattern using
existing regmap_field_read/write APIs. The author works at Bootlin
for TI platforms.
2. **Fixes a real bug**: Mux selection lost after resume = broken PHY
after suspend/resume.
3. **Important issue**: Broken hardware functionality after
suspend/resume.
4. **Small and contained**: ~20 lines of new code, single file, simple
logic.
5. **No new features**: Restoring state that was lost is a bug fix, not
a feature.
### Risk assessment:
- **Very low risk**: The save/restore pattern is straightforward and
well-understood.
- **WIZ_MUX_NUM_CLOCKS is 3**: Only 3 register reads and 3 register
writes are added.
- **Uses existing regmap_field APIs**: No new infrastructure needed.
- **Only affects TI J721E platforms**: Cannot cause regressions on other
platforms.
- **Only affects suspend/resume path**: Cannot affect boot or normal
operation.
### Dependencies:
The resume support was added in commit `b5539abdd013f` (v6.11). The
`wiz_clock_init()` function split was in commit `3c4e13cf9ead7` (also
v6.11). So this fix is applicable to stable trees v6.11+, and possibly
v6.12.y and v6.13.y LTS trees if they exist.
## Verification
- **git log** confirmed the driver was first added in commit
`091876cc355d6` (phy: ti: j721e-wiz: Add support for WIZ module
present in TI J721E SoC)
- **git tag --contains** confirmed resume support (`b5539abdd013f`) was
introduced in v6.11
- **git tag --contains** confirmed `wiz_clock_init()` split
(`3c4e13cf9ead7`) was also introduced in v6.11
- **Grep for `WIZ_MUX_NUM_CLOCKS`** confirmed it is defined as 3, making
the save/restore loop iterate only 3 times
- **Grep for `mux_sel_status`** confirmed the field does not yet exist
in the working tree (commit not yet applied)
- **Grep for `wiz_resume_noirq`** confirmed the current code has resume
handler but NULL suspend handler, matching the pre-patch state
- **Read of the mux_sel_field usage** (lines 1031, 1056, 1172) confirmed
these fields control clock mux selection in the WIZ initialization
path
- **Unverified**: Whether any downstream users (e.g., Android, embedded
distros) have reported this specific suspend/resume issue. The commit
message states the problem directly but doesn't include "Reported-by:"
tags.
## Conclusion
This is a small, well-contained suspend/resume bug fix for the TI J721E
WIZ PHY driver. It fixes a real hardware issue where clock mux
selections are lost during suspend, rendering the SERDES PHY non-
functional after resume. The fix uses a standard save/restore pattern
with minimal code additions (~20 lines in a single file). The risk of
regression is very low since it only affects the suspend/resume path on
TI K3 platforms. It meets all stable kernel criteria.
**YES**
drivers/phy/ti/phy-j721e-wiz.c | 19 +++++++++++++++++--
1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/ti/phy-j721e-wiz.c b/drivers/phy/ti/phy-j721e-wiz.c
index a8b440c6c46bb..ba31b0a1f7f79 100644
--- a/drivers/phy/ti/phy-j721e-wiz.c
+++ b/drivers/phy/ti/phy-j721e-wiz.c
@@ -393,6 +393,7 @@ struct wiz {
struct clk *output_clks[WIZ_MAX_OUTPUT_CLOCKS];
struct clk_onecell_data clk_data;
const struct wiz_data *data;
+ int mux_sel_status[WIZ_MUX_NUM_CLOCKS];
};
static int wiz_reset(struct wiz *wiz)
@@ -1654,11 +1655,25 @@ static void wiz_remove(struct platform_device *pdev)
pm_runtime_disable(dev);
}
+static int wiz_suspend_noirq(struct device *dev)
+{
+ struct wiz *wiz = dev_get_drvdata(dev);
+ int i;
+
+ for (i = 0; i < WIZ_MUX_NUM_CLOCKS; i++)
+ regmap_field_read(wiz->mux_sel_field[i], &wiz->mux_sel_status[i]);
+
+ return 0;
+}
+
static int wiz_resume_noirq(struct device *dev)
{
struct device_node *node = dev->of_node;
struct wiz *wiz = dev_get_drvdata(dev);
- int ret;
+ int ret, i;
+
+ for (i = 0; i < WIZ_MUX_NUM_CLOCKS; i++)
+ regmap_field_write(wiz->mux_sel_field[i], wiz->mux_sel_status[i]);
/* Enable supplemental Control override if available */
if (wiz->sup_legacy_clk_override)
@@ -1680,7 +1695,7 @@ static int wiz_resume_noirq(struct device *dev)
return ret;
}
-static DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, NULL, wiz_resume_noirq);
+static DEFINE_NOIRQ_DEV_PM_OPS(wiz_pm_ops, wiz_suspend_noirq, wiz_resume_noirq);
static struct platform_driver wiz_driver = {
.probe = wiz_probe,
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.19-5.10] phy: fsl-imx8mq-usb: disable bind/unbind platform driver feature
[not found] <20260219020422.1539798-1-sashal@kernel.org>
2026-02-19 2:03 ` [PATCH AUTOSEL 6.19-6.12] phy: cadence-torrent: restore parent clock for refclk during resume Sasha Levin
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-6.12] phy: ti: phy-j721e-wiz: restore mux selection " Sasha Levin
@ 2026-02-19 2:04 ` Sasha Levin
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-5.15] phy: mvebu-cp110-utmi: fix dr_mode property read from dts Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19 2:04 UTC (permalink / raw)
To: patches, stable
Cc: Xu Yang, Frank Li, Vinod Koul, Sasha Levin, linux-phy, imx,
linux-arm-kernel, linux-kernel
From: Xu Yang <xu.yang_2@nxp.com>
[ Upstream commit 27ee0869d77b2cb404770ac49bdceae3aedf658b ]
Disabling PHYs in runtime usually causes the client with external abort
exception or similar issue due to lack of API to notify clients about PHY
removal. This patch removes the possibility to unbind i.MX PHY drivers in
runtime.
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Link: https://patch.msgid.link/20260120111712.3159782-1-xu.yang_2@nxp.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Analysis Summary
### What the commit does
This commit adds `.suppress_bind_attrs = true` to the i.MX8MQ USB PHY
platform driver. This single line prevents the kernel from creating
`/sys/bus/platform/drivers/imx8mq-usb-phy/bind` and
`/sys/bus/platform/drivers/imx8mq-usb-phy/unbind` sysfs files, thereby
preventing runtime unbinding of the PHY driver.
### Problem being fixed
The commit message states that unbinding PHYs at runtime causes
"external abort exception or similar issue" because there is no API to
notify PHY clients about PHY removal. An external abort is a serious ARM
exception that typically results in a kernel crash/oops. If a user or
management tool writes to the `unbind` sysfs file for this PHY driver,
the USB controller client would try to access the now-missing PHY
resources, resulting in a crash.
### Stable kernel rules assessment
1. **Obviously correct and tested**: Yes - this is a well-established
one-line pattern used in 385+ files in the kernel. It's reviewed by
the PHY maintainer (Vinod Koul) and reviewed by Frank Li.
2. **Fixes a real bug**: Yes - runtime unbinding causes a crash
(external abort exception). This is a real, triggerable issue.
3. **Important issue**: Moderate - it prevents a crash, but only
triggered by explicit sysfs manipulation (not normal operation).
However, system management tools or user error could trigger this.
4. **Small and contained**: Yes - single one-line addition to a driver
struct.
5. **No new features**: Correct - this removes functionality (ability to
unbind), it doesn't add any.
### Risk assessment
- **Risk**: Extremely low. This is literally a single boolean field
being set in the driver struct. It only affects sysfs attributes. The
same pattern exists in 385+ other drivers.
- **Benefit**: Prevents a crash scenario when the PHY is unbound at
runtime.
- **Dependencies**: None - this is entirely self-contained.
### Concerns
- The bug is only triggerable by explicit sysfs manipulation (writing to
`/sys/bus/platform/drivers/imx8mq-usb-phy/unbind`), not by normal
system operation.
- The fix is a defensive measure rather than fixing a root cause in the
PHY framework. However, this is the standard kernel approach - the PHY
framework doesn't have removal notification, so drivers that can't
handle unbinding should suppress it.
- The change is straightforward and would apply cleanly to any stable
tree that has this driver.
### Classification
This falls into the **hardware quirk/workaround** category. It's a
defensive fix that prevents a crash scenario. While the trigger requires
explicit user action (sysfs unbind), this is a well-established pattern
in the kernel for exactly this purpose.
## Verification
- Verified `suppress_bind_attrs = true` is used in 385+ files across the
drivers/ directory via Grep search
- Verified 7 other PHY drivers (Samsung, HiSilicon) already use this
pattern
- Verified no other Freescale PHY drivers currently have this attribute
(this is the first)
- Verified via git log that similar commits exist for other subsystems
(gpio, perf)
- Verified via lore.kernel.org that the patch was reviewed by Frank Li
and accepted by PHY maintainer Vinod Koul
- Verified the commit message describes a real crash scenario (external
abort exception)
- Could NOT verify the specific crash reproduction steps (unverified,
but the mechanism is well-understood in the kernel community)
The fix is minimal (one line), extremely low risk, prevents a crash
scenario, uses a well-established kernel pattern, and was properly
reviewed. It meets all stable kernel criteria.
**YES**
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 91b3e62743d3a..b30d01f345d20 100644
--- a/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
+++ b/drivers/phy/freescale/phy-fsl-imx8mq-usb.c
@@ -730,6 +730,7 @@ static struct platform_driver imx8mq_usb_phy_driver = {
.driver = {
.name = "imx8mq-usb-phy",
.of_match_table = imx8mq_usb_phy_of_match,
+ .suppress_bind_attrs = true,
}
};
module_platform_driver(imx8mq_usb_phy_driver);
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH AUTOSEL 6.19-5.15] phy: mvebu-cp110-utmi: fix dr_mode property read from dts
[not found] <20260219020422.1539798-1-sashal@kernel.org>
` (2 preceding siblings ...)
2026-02-19 2:04 ` [PATCH AUTOSEL 6.19-5.10] phy: fsl-imx8mq-usb: disable bind/unbind platform driver feature Sasha Levin
@ 2026-02-19 2:04 ` Sasha Levin
3 siblings, 0 replies; 4+ messages in thread
From: Sasha Levin @ 2026-02-19 2:04 UTC (permalink / raw)
To: patches, stable
Cc: Aleksandar Gerasimovski, Vinod Koul, Sasha Levin, linux-phy,
linux-kernel
From: Aleksandar Gerasimovski <aleksandar.gerasimovski@belden.com>
[ Upstream commit e2ce913452ab56b3330539cc443b97b7ea8c3a1a ]
The problem with the current implementation is that it does not consider
that the USB controller can have multiple PHY handles with different
arguments count, as for example we have in our cn9131 based platform:
"phys = <&cp0_comphy1 0>, <&cp0_utmi0>;".
In such case calling "of_usb_get_dr_mode_by_phy" with -1 (no phy-cells)
leads to not proper phy detection, taking the "marvell,cp110-utmi-phy"
dts definition we can call the "of_usb_get_dr_mode_by_phy" with 0
(#phy-cells = <0>) and safely look for that phy.
Signed-off-by: Aleksandar Gerasimovski <aleksandar.gerasimovski@belden.com>
Link: https://patch.msgid.link/20260106150643.922110-1-aleksandar.gerasimovski@belden.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Now I have a clear understanding of the bug. Let me verify the behavior
difference more carefully:
## Understanding the Bug
When `arg0 = -1`:
- The function uses `of_parse_phandle()` (line 303-306), which simply
returns the nth phandle node **without** parsing `#phy-cells`. It sets
`args.args_count = 0`.
- The matching at line 316-317: `args.np == np && (args.args_count == 0
|| args.args[0] == arg0)` — since `args_count = 0`, it matches on the
`np` comparison alone.
The problem: When the controller has `phys = <&cp0_comphy1 0>,
<&cp0_utmi0>;`, there are two phandle entries. The first (`cp0_comphy1`)
has `#phy-cells = <1>` (it takes argument `0`), and the second
(`cp0_utmi0`) has `#phy-cells = <0>`.
With `arg0 = -1`, `of_parse_phandle()` is used, which doesn't understand
`#phy-cells`. It simply iterates through raw phandle entries. Since
`cp0_comphy1` has an extra cell (`0`), `of_parse_phandle()` at index 0
returns `cp0_comphy1`, at index 1 it might return the `0` argument cell
(not a valid phandle) rather than `cp0_utmi0`. This leads to incorrect
PHY matching — the function can't properly find the UTMI PHY when mixed
`#phy-cells` counts are present.
With `arg0 = 0`:
- The function uses `of_parse_phandle_with_args()` (line 307-312), which
correctly parses `#phy-cells` for each phandle and properly skips over
argument cells. This means index 0 correctly refers to `cp0_comphy1`
(with its argument `0`) and index 1 correctly refers to `cp0_utmi0`.
- At line 316-317: for `cp0_utmi0`, `args.args_count = 0` (since `#phy-
cells = <0>`), so the match succeeds correctly.
This is a real functional bug fix — on platforms with mixed PHY types,
the UTMI PHY won't be correctly identified for its dual-role mode,
potentially causing the USB port to be configured incorrectly
(defaulting to HOST mode with a warning instead of detecting the correct
mode).
## Stable Kernel Criteria Assessment
1. **Fixes a real bug**: Yes — on cn9131-based platforms (and likely
others) with mixed PHY types, the UTMI PHY dr_mode detection fails,
leading to incorrect USB configuration.
2. **Obviously correct and tested**: The fix is a one-line change from
`-1` to `0`. The commit author is from Belden (likely has the
hardware). The change matches the documented `#phy-cells = <0>` for
this PHY type. Using `of_parse_phandle_with_args()` (triggered by
arg0 >= 0) is the correct approach when different PHYs have different
`#phy-cells` counts.
3. **Small and contained**: Single character change in one file. Minimal
risk.
4. **No new features**: This is purely a bug fix.
5. **User impact**: Without this fix, USB dual-role mode detection fails
on affected platforms. The PHY defaults to HOST mode when it might
need to be PERIPHERAL mode (or vice versa), which means USB
functionality is broken for those configurations.
## Risk Assessment
- **Risk**: Extremely low. The change from `-1` to `0` for a PHY with
`#phy-cells = <0>` is semantically equivalent for single-PHY
configurations. For multi-PHY configurations with mixed `#phy-cells`,
it's a correctness fix.
- **Regression potential**: Very low. The `of_parse_phandle_with_args()`
path is well-tested across the kernel. The only behavior change is
that mixed-PHY configurations now work correctly.
- **Scope**: Single driver, single line, Marvell ARM platform specific.
## Verification
- **Function `of_usb_get_dr_mode_by_phy` analyzed**: Read the
implementation at `drivers/usb/common/common.c:290-331`. Verified that
`arg0 = -1` uses `of_parse_phandle()` (no `#phy-cells` awareness)
while `arg0 >= 0` uses `of_parse_phandle_with_args()` (proper `#phy-
cells` parsing).
- **Device tree binding confirmed**: The Explore agent verified that
`Documentation/devicetree/bindings/phy/marvell,armada-cp110-utmi-
phy.yaml` specifies `#phy-cells: const: 0`, meaning arg0 = 0 is the
correct semantic match.
- **Driver history verified**: `git log` shows the driver was introduced
in commit `2fc989f74b8da` ("drivers: phy: add support for Armada CP110
UTMI PHY"), and the `-1` argument has been there since inception. The
file has had only 3 subsequent commits before this fix, none related
to dr_mode.
- **Matching logic verified**: At line 316-317, when `args.args_count ==
0` (which is the case for `#phy-cells = <0>`), the match succeeds on
`args.np == np` alone, so the value `0` doesn't cause a spurious
mismatch.
- **Bug mechanism verified**: With mixed `#phy-cells` (e.g., `phys =
<&cp0_comphy1 0>, <&cp0_utmi0>`), `of_parse_phandle()` (used with
arg0=-1) doesn't account for the argument cell `0` after
`cp0_comphy1`, leading to incorrect phandle indexing.
## Conclusion
This is a clear, minimal, one-line bug fix for incorrect device tree PHY
matching on Marvell CP110 UTMI platforms. It fixes a real functional
issue (incorrect USB dual-role mode detection) that affects users with
mixed-PHY configurations. The fix is obviously correct, has near-zero
regression risk, and follows the established pattern used by other PHY
drivers.
**YES**
drivers/phy/marvell/phy-mvebu-cp110-utmi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
index 59903f86b13f5..dd3e515a8e865 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-utmi.c
@@ -338,7 +338,7 @@ static int mvebu_cp110_utmi_phy_probe(struct platform_device *pdev)
return -ENOMEM;
}
- port->dr_mode = of_usb_get_dr_mode_by_phy(child, -1);
+ port->dr_mode = of_usb_get_dr_mode_by_phy(child, 0);
if ((port->dr_mode != USB_DR_MODE_HOST) &&
(port->dr_mode != USB_DR_MODE_PERIPHERAL)) {
dev_err(&pdev->dev,
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 4+ messages in thread