* Re: [PATCH v4 1/2] phy: ti: pipe3: Fix clock resource leak on probe errors
From: sashiko-bot @ 2026-05-18 2:36 UTC (permalink / raw)
To: Hongling Zeng; +Cc: vkoul, olteanv, neil.armstrong, linux-phy
In-Reply-To: <20260518021539.14890-2-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clock reference leak in ti_pipe3_init() on PCIe error path.
--
commit 26f79663d08d9c0cec90795a1b32d3e5e37b7841
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti: pipe3: Fix clock resource leak on probe errors
When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.
Fix this by adding an error handling path to disable the clock
when these functions fail.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Fix this by adding an error handling path to disable the clock
> when these functions fail.
This isn't a bug in this patch, but while fixing clock leaks in this driver,
does ti_pipe3_init() also leak the clock references on the PCIe error path?
If regmap_update_bits() fails in ti_pipe3_init(), the function returns the
error without calling ti_pipe3_disable_clocks():
drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_init() {
ti_pipe3_enable_clocks(phy);
...
if (phy->mode == PIPE3_MODE_PCIE) {
if (!phy->pcs_syscon) {
omap_control_pcie_pcs(phy->control_dev, 0x96);
return 0;
}
val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
PCIE_PCS_MASK, val);
return ret;
}
...
}
Because the initialization fails, the core PHY framework will not invoke the
.exit callback, potentially leaving the clocks permanently enabled.
Unlike the ti_pipe3_dpll_program() error path later in the same function
which correctly calls ti_pipe3_disable_clocks(phy), this PCIe error path
appears to miss the cleanup.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021539.14890-1-zenghongling@kylinos.cn?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v4 2/2] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: sashiko-bot @ 2026-05-18 2:59 UTC (permalink / raw)
To: Hongling Zeng; +Cc: vkoul, olteanv, linux-phy, neil.armstrong
In-Reply-To: <20260518021539.14890-3-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Log spam during boot due to lack of dev_err_probe() for deferred clocks.
- [Low] Unnecessary braces used for single-statement if/else branches.
--
commit e5fb8e810f7245e72a8e8c129ec34b40381c5a33
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
This commit fixes probe deferral handling in the ti-pipe3 driver by returning
the actual error code for sysclk failures instead of masking it. It also
propagates non-ENOENT errors for the SATA refclk.
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -605,14 +605,21 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
> struct clk *clk;
> struct device *dev = phy->dev;
>
> - phy->refclk = devm_clk_get(dev, "refclk");
> + /*
> + * refclk is optional for SATA PHY to support older DTBs, but
> + * required for other modes. Use devm_clk_get_optional() for SATA
> + * which returns NULL for -ENOENT, allowing us to propagate all
> + * other errors including -EPROBE_DEFER.
> + */
> + if (phy->mode == PIPE3_MODE_SATA) {
> + phy->refclk = devm_clk_get_optional(dev, "refclk");
> + } else {
> + phy->refclk = devm_clk_get(dev, "refclk");
> + }
This isn't a bug, but is it necessary to use braces here? The Linux kernel
coding style typically recommends omitting braces for single-statement
if/else branches.
> +
> if (IS_ERR(phy->refclk)) {
> dev_err(dev, "unable to get refclk\n");
> - /* older DTBs have missing refclk in SATA PHY
> - * so don't bail out in case of SATA PHY.
> - */
> - if (phy->mode != PIPE3_MODE_SATA)
> - return PTR_ERR(phy->refclk);
> + return PTR_ERR(phy->refclk);
> }
Since PTR_ERR() now correctly propagates -EPROBE_DEFER when the clock is not
yet available, will this dev_err() call cause repetitive log spam regressions
during normal boot sequence deferrals?
Could this be replaced with dev_err_probe() to properly suppress the error
message when the clock provider is simply deferred?
[ ... ]
> @@ -629,7 +636,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
> phy->sys_clk = devm_clk_get(dev, "sysclk");
> if (IS_ERR(phy->sys_clk)) {
> dev_err(dev, "unable to get sysclk\n");
> - return -EINVAL;
> + return PTR_ERR(phy->sys_clk);
> }
> }
Does the same log spam regression apply here now that sysclk errors,
including -EPROBE_DEFER, are correctly propagated?
Using dev_err_probe() here might also help keep the kernel log clean during
probe deferral.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518021539.14890-1-zenghongling@kylinos.cn?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] phy: ti-pipe3: Fix clock leak in init error path
From: Hongling Zeng @ 2026-05-18 3:51 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng, Sashiko AI,
stable
When regmap_update_bits() fails in ti_pipe3_init() for PCIe mode,
the function returns the error without calling ti_pipe3_disable_clocks().
This leaves the clocks permanently enabled since the PHY framework won't
invoke the .exit callback on init failure.
Fix this by adding proper clock cleanup in the PCIe error path, consistent
with how the DPLL program error path handles cleanup.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Reported-by: Sashiko AI <sashiko@kernel.org>
Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Cc: stable@vger.kernel.org
---
drivers/phy/ti/phy-ti-pipe3.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index e0ab7d21e99c..ba1c937272b1 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -515,6 +515,8 @@ static int ti_pipe3_init(struct phy *x)
val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
PCIE_PCS_MASK, val);
+ if (ret)
+ ti_pipe3_disable_clocks(phy);
return ret;
}
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH] phy: ti-pipe3: Fix clock leak in init error path
From: sashiko-bot @ 2026-05-18 4:12 UTC (permalink / raw)
To: Hongling Zeng; +Cc: vkoul, neil.armstrong, linux-phy, olteanv
In-Reply-To: <20260518035105.26607-1-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ti_pipe3_init() completely ignores the return value of ti_pipe3_enable_clocks(), leading to unclocked hardware access and unbalanced clock disables.
--
commit 6e3b53b5ea9af504f2a62f9abfc933bbbb671e25
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti-pipe3: Fix clock leak in init error path
This commit adds a clock cleanup in the PCIe error path of ti_pipe3_init()
to prevent clocks from being permanently enabled when regmap_update_bits()
fails.
Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index b5543b5c674c8..b5ee0af77c9f6 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -515,6 +515,8 @@ static int ti_pipe3_init(struct phy *x)
> val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
> ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
> PCIE_PCS_MASK, val);
> + if (ret)
> + ti_pipe3_disable_clocks(phy);
Could this error path trigger an unbalanced clock disable warning if the clocks
were never successfully enabled?
Earlier in ti_pipe3_init(), the return value of ti_pipe3_enable_clocks() is
ignored:
drivers/phy/ti/phy-ti-pipe3.c:ti_pipe3_init() {
...
int ret = 0;
ti_pipe3_enable_clocks(phy);
/*
* Set pcie_pcs register to 0x96 for proper functioning of phy
...
}
If ti_pipe3_enable_clocks() fails, it rolls back and cleanly disables any
partially enabled clocks.
The initialization then continues anyway, interacting with unclocked hardware,
which might cause a bus error.
If it reaches this new error path because regmap_update_bits() fails,
ti_pipe3_disable_clocks() will be called unconditionally on already-disabled
clocks.
> return ret;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518035105.26607-1-zenghongling@kylinos.cn?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v5 2/4] phy: ti: pipe3: Fix clock resource leak on probe errors
From: Hongling Zeng @ 2026-05-18 6:29 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>
When devm_phy_create() or devm_of_phy_provider_register() fails,
the refclk that was enabled earlier is not disabled, causing a
resource leak.
Fix this by adding an error handling path to disable the clock
when these functions fail.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change in v2:
-Add pm_runtime_disable() in error path (reported by Sashiko AI).
---
change in v3:
-Fix unbalanced clock disable by checking clk_prepare_enable()return value and
setting sata_refclk_enabledonly on success.
-Fix error path teardown order to match ti_pipe3_remove()(disable clock before
disabling runtime PM).
---
change in v4:
-Fix the patch version
---
Change in v5:
-Add Fix ignored clock enable return value in init patch
---
drivers/phy/ti/phy-ti-pipe3.c | 26 ++++++++++++++++++++++----
1 file changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b8c055893742..2d36fe4c4218 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -834,21 +834,39 @@ static int ti_pipe3_probe(struct platform_device *pdev)
*/
if (phy->mode == PIPE3_MODE_SATA) {
if (!IS_ERR(phy->refclk)) {
- clk_prepare_enable(phy->refclk);
+ ret = clk_prepare_enable(phy->refclk);
+ if (ret) {
+ dev_err(dev, "Failed to enable refclk %d\n", ret);
+ goto err_pm_disable;
+ }
phy->sata_refclk_enabled = true;
}
}
generic_phy = devm_phy_create(dev, NULL, &ops);
- if (IS_ERR(generic_phy))
- return PTR_ERR(generic_phy);
+ if (IS_ERR(generic_phy)) {
+ ret = PTR_ERR(generic_phy);
+ goto err_clk_disable;
+ }
phy_set_drvdata(generic_phy, phy);
ti_pipe3_power_off(generic_phy);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
- return PTR_ERR_OR_ZERO(phy_provider);
+ if (IS_ERR(phy_provider)) {
+ ret = PTR_ERR(phy_provider);
+ goto err_clk_disable;
+ }
+
+ return 0;
+
+err_clk_disable:
+ if (phy->sata_refclk_enabled && !IS_ERR(phy->refclk))
+ clk_disable_unprepare(phy->refclk);
+err_pm_disable:
+ pm_runtime_disable(dev);
+ return ret;
}
static void ti_pipe3_remove(struct platform_device *pdev)
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 0/4] phy: ti-pipe3: Fix clock resource handling issues
From: Hongling Zeng @ 2026-05-18 6:29 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
This patch series fixes multiple clock resource handling issues in the
ti-pipe3 PHY driver.
Patch 1 fixes a critical issue where ti_pipe3_init() was ignoring the
return value of ti_pipe3_enable_clocks(), which could lead to unclocked
hardware access and unbalanced clock disables.
Patch 2 fixes a clock resource leak when probe fails after enabling
the SATA refclk. The error path now properly disables the clock and
cleans up runtime PM resources.
Patch 3 fixes EPROBE_DEFER handling to prevent masking probe deferral
errors. It uses devm_clk_get_optional() for SATA refclk to properly
handle optional clocks while still propagating -EPROBE_DEFER and other
error codes.
Patch 4 fixes a clock leak in the init error path when regmap_update_bits()
fails in PCIe mode, adding proper clock cleanup consistent with other
error paths.
These fixes ensure proper resource cleanup on probe and init failures,
and prevent permanent driver initialization failures due to incorrect
error handling.
Changes in v5:
- Add patch to fix ignored clock enable return value in ti_pipe3_init()
- Improve error handling consistency across all paths
Changes in v4:
- Use devm_clk_get_optional() for SATA refclk
- Drop manual -ENOENT handling
Hongling Zeng (4):
phy: ti-pipe3: Fix ignored clock enable return value in init
phy: ti: pipe3: Fix clock resource leak on probe errors
phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
phy: ti-pipe3: Fix clock leak in init error path
drivers/phy/ti/phy-ti-pipe3.c | 58 ++++++++++++++++++++++++++++--------
1 file changed, 46 insertions(+), 12 deletions(-)
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH v5 1/4] phy: ti-pipe3: Fix ignored clock enable return value in init
From: Hongling Zeng @ 2026-05-18 6:29 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng, Sashiko AI
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>
ti_pipe3_init() ignores the return value of ti_pipe3_enable_clocks(),
which can lead to:
1. Unclocked hardware access if clock enable fails
2. Unbalanced clock disables in error paths
ti_pipe3_enable_clocks() returns an error code when clock enable fails
and rolls back any partially enabled clocks. If we ignore this error
and continue, we access hardware without proper clocking, which can
cause bus errors.
Additionally, if we reach error paths later in the function and call
ti_pipe3_disable_clocks(), we'll be disabling already-disabled clocks,
causing unbalanced disable warnings.
Fix this by checking the return value of ti_pipe3_enable_clocks()
and returning early if it fails.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Reported-by: Sashiko AI <sashiko@kernel.org>
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change in v5:
-Add Fix ignored clock enable return value in init patch
---
drivers/phy/ti/phy-ti-pipe3.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index b5543b5c674c..b8c055893742 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -500,7 +500,10 @@ static int ti_pipe3_init(struct phy *x)
u32 val;
int ret = 0;
- ti_pipe3_enable_clocks(phy);
+ ret = ti_pipe3_enable_clocks(phy);
+ if (ret)
+ return ret;
+
/*
* Set pcie_pcs register to 0x96 for proper functioning of phy
* as recommended in AM572x TRM SPRUHZ6, section 18.5.2.2, table
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 3/4] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: Hongling Zeng @ 2026-05-18 6:29 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>
ti_pipe3_get_clk() has two issues with -EPROBE_DEFER error handling:
1. When devm_clk_get() for sysclk fails, the function returns -EINVAL
instead of propagating the actual error code. This masks -EPROBE_DEFER
to -EINVAL, breaking the probe deferral mechanism and causing permanent
driver initialization failure on systems with non-deterministic probe
ordering.
2. For SATA PHY refclk, the function ignores all errors to support older
DTBs missing the refclk property. However, this incorrectly ignores
-EPROBE_DEFER as well, causing the driver to proceed without waiting
for the clock provider to become available.
Fix both issues:
- Return PTR_ERR(phy->sys_clk) instead of -EINVAL to propagate all
error codes including -EPROBE_DEFER
- Use devm_clk_get_optional() for SATA refclk to handle optional
clocks while propagating -EPROBE_DEFER and other errors
Fixes: a70143bbef6b ("drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework")
Fixes: 7f33912d2978 ("phy: ti-pipe3: Fix SATA across suspend/resume")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
---
Change in v4:
- Merge refclk leak fix and EPROBE_DEFER fix into a single patch
- Use devm_clk_get_optional() for SATA refclk
- Drop manual -ENOENT handling
- Ensure error paths are fully symmetric
---
Change in v5:
-Add Fix ignored clock enable return value in init patch
---
drivers/phy/ti/phy-ti-pipe3.c | 20 +++++++++++++-------
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index 2d36fe4c4218..9ec228c2a940 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -608,14 +608,20 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
struct clk *clk;
struct device *dev = phy->dev;
- phy->refclk = devm_clk_get(dev, "refclk");
+ /*
+ * refclk is optional for SATA PHY to support older DTBs, but
+ * required for other modes. Use devm_clk_get_optional() for SATA
+ * which returns NULL for -ENOENT, allowing us to propagate all
+ * other errors including -EPROBE_DEFER.
+ */
+ if (phy->mode == PIPE3_MODE_SATA)
+ phy->refclk = devm_clk_get_optional(dev, "refclk");
+ else
+ phy->refclk = devm_clk_get(dev, "refclk");
+
if (IS_ERR(phy->refclk)) {
dev_err(dev, "unable to get refclk\n");
- /* older DTBs have missing refclk in SATA PHY
- * so don't bail out in case of SATA PHY.
- */
- if (phy->mode != PIPE3_MODE_SATA)
- return PTR_ERR(phy->refclk);
+ return PTR_ERR(phy->refclk);
}
if (phy->mode != PIPE3_MODE_SATA) {
@@ -632,7 +638,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
phy->sys_clk = devm_clk_get(dev, "sysclk");
if (IS_ERR(phy->sys_clk)) {
dev_err(dev, "unable to get sysclk\n");
- return -EINVAL;
+ return PTR_ERR(phy->sys_clk);
}
}
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH v5 4/4] phy: ti-pipe3: Fix clock leak in init error path
From: Hongling Zeng @ 2026-05-18 6:29 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: linux-phy, linux-kernel, zhongling0719, Hongling Zeng, Sashiko AI,
stable
In-Reply-To: <20260518062938.48114-1-zenghongling@kylinos.cn>
When regmap_update_bits() fails in ti_pipe3_init() for PCIe mode,
the function returns the error without calling ti_pipe3_disable_clocks().
This leaves the clocks permanently enabled since the PHY framework won't
invoke the .exit callback on init failure.
Fix this by adding proper clock cleanup in the PCIe error path, consistent
with how the DPLL program error path handles cleanup.
Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Reported-by: Sashiko AI <sashiko@kernel.org>
Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
Cc: stable@vger.kernel.org
---
Change in v5:
-Add Fix ignored clock enable return value in init patch
---
drivers/phy/ti/phy-ti-pipe3.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
index 9ec228c2a940..4897e4ba2d7d 100644
--- a/drivers/phy/ti/phy-ti-pipe3.c
+++ b/drivers/phy/ti/phy-ti-pipe3.c
@@ -518,6 +518,8 @@ static int ti_pipe3_init(struct phy *x)
val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
PCIE_PCS_MASK, val);
+ if (ret)
+ ti_pipe3_disable_clocks(phy);
return ret;
}
--
2.25.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH v3 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Ioana Ciornei @ 2026-05-18 7:22 UTC (permalink / raw)
To: Vinod Koul
Cc: neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy,
devicetree, linux-kernel
In-Reply-To: <agnu5Mmf_d8zvWXH@vaman>
On Sun, May 17, 2026 at 10:07:56PM +0530, Vinod Koul wrote:
> On 16-05-26, 09:03, Ioana Ciornei wrote:
> > Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> > Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> > speeds but it can easily extended to also cover other usecases.
> >
> > Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> > does not name the registers, the name for the macros were determined by
> > their usage pattern.
> >
> > A PHY device is created for each of the two channels present on the
> > retimer. This allows for independent configuration of the two channels.
> > This capability is especially important on retimers which have more than
> > 2 channels that can be, depending on the board design, connected in
> > multiple different ways to the SerDes lanes.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v3:
> > - Use reverse Christmas tree ordering
> > - Print a symbolic description in case of error
> > - Some words do not need to be capitalized
> > - Remove duplicated exit code path
> > - Return -EINVAL in case of unsupported submode received in .set_mode()
> > - Add a .validate() callback
> > - Remove comma after sentinel entry
> > - Add a ds125df111_rmw() helper
> > - Use read_poll_timeout() to wait for channel reset to complete
> >
> > Changes in v2:
> > - Explicitly include all the needed headers
> > - Change ds125df111_xlate() so that it returns an error if args_count is
> > not exactly 1
> > - Add a MAINTAINERS entry
> > ---
> > MAINTAINERS | 7 +
> > drivers/phy/ti/Kconfig | 10 ++
> > drivers/phy/ti/Makefile | 1 +
> > drivers/phy/ti/phy-ds125df111.c | 294 ++++++++++++++++++++++++++++++++
> > 4 files changed, 312 insertions(+)
> > create mode 100644 drivers/phy/ti/phy-ds125df111.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f877e5aaf2c7..58f410b666e7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -26781,6 +26781,13 @@ T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
> > F: drivers/media/platform/ti/davinci/
> > F: include/media/davinci/
> >
> > +TI DS125DF111 RETIMER PHY DRIVER
> > +M: Ioana Ciornei <ioana.ciornei@nxp.com>
> > +L: linux-phy@lists.infradead.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
> > +F: drivers/phy/ti/phy-ds125df111.c
> > +
> > TI ENHANCED CAPTURE (eCAP) DRIVER
> > M: Vignesh Raghavendra <vigneshr@ti.com>
> > R: Julien Panis <jpanis@baylibre.com>
> > diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
> > index b40f28019131..475e80fcd52d 100644
> > --- a/drivers/phy/ti/Kconfig
> > +++ b/drivers/phy/ti/Kconfig
> > @@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
> > help
> > This driver supports configuring of the TI CPSW Port mode depending on
> > the Ethernet PHY connected to the CPSW Port.
> > +
> > +config PHY_TI_DS125DF111
>
> This should be in alphabetical order, so I guess before PHY_TI_G...
Ok, will reorder them.
>
> > + tristate "DS125DF111 2-Channel Retimer Driver"
> > + depends on OF && I2C
> > + select GENERIC_PHY
> > + help
> > + Enable this to add support for configuration and runtime management
> > + of the TI DS125DF111 Multi-Protocol 2-Channel Retimer.
> > + The retimer is modeled as a Generic PHY and supports both 10G and 1G
> > + link speeds.
> > diff --git a/drivers/phy/ti/Makefile b/drivers/phy/ti/Makefile
> > index dcba2571c9bd..e68445ddd848 100644
> > --- a/drivers/phy/ti/Makefile
> > +++ b/drivers/phy/ti/Makefile
> > @@ -9,3 +9,4 @@ obj-$(CONFIG_TWL4030_USB) += phy-twl4030-usb.o
> > obj-$(CONFIG_PHY_AM654_SERDES) += phy-am654-serdes.o
> > obj-$(CONFIG_PHY_TI_GMII_SEL) += phy-gmii-sel.o
> > obj-$(CONFIG_PHY_J721E_WIZ) += phy-j721e-wiz.o
> > +obj-$(CONFIG_PHY_TI_DS125DF111) += phy-ds125df111.o
>
> Here as well
Sure.
>
> > diff --git a/drivers/phy/ti/phy-ds125df111.c b/drivers/phy/ti/phy-ds125df111.c
> > new file mode 100644
> > index 000000000000..8788f340e3b0
> > --- /dev/null
> > +++ b/drivers/phy/ti/phy-ds125df111.c
> > @@ -0,0 +1,294 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/* Copyright 2026 NXP */
> > +
> > +#include <linux/bitfield.h>
> > +#include <linux/bits.h>
> > +#include <linux/i2c.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/phy.h>
> > +#include <linux/phy/phy.h>
> > +#include <linux/slab.h>
> > +
> > +#define DS125DF111_NUM_CH 2
> > +#define DS125DF111_NUM_VCO_GROUP_REG 5
> > +
> > +#define DS125DF111_CH_SELECT 0xff
> > +#define DS125DF111_CH_SELECT_TARGET_MASK GENMASK(3, 0)
> > +#define DS125DF111_CH_SELECT_EN BIT(2)
> > +
> > +#define DS125DF111_CH_CTRL 0x00
> > +#define DS125DF111_CH_CTRL_RESET BIT(2) /* self clearing */
> > +
> > +#define DS125DF111_CH_RST_SLEEP_US 10
> > +#define DS125DF111_CH_RST_TIMEOUT_US 10000
> > +
> > +#define DS125DF111_VCO_GROUP_BASE 0x60
> > +
> > +#define DS125DF111_RATIOS 0x2F
>
> Lower case for the hex values please
>
> > +#define DS125DF111_RATIOS_RATE_MASK GENMASK(7, 6)
> > +#define DS125DF111_RATIOS_SUBRATE_MASK GENMASK(5, 4)
> > +#define DS125DF111_RATIOS_MASK GENMASK(7, 4)
> > +
> > +struct ds125df111_ch {
> > + struct phy *phy;
> > + struct ds125df111_priv *priv;
> > + int idx;
> > +};
> > +
> > +struct ds125df111_priv {
> > + struct ds125df111_ch ch[DS125DF111_NUM_CH];
> > + struct i2c_client *client;
> > + struct mutex mutex; /* protects access to shared registers */
> > +};
> > +
> > +enum ds125df111_mode {
> > + FREQ_1G,
> > + FREQ_10G,
> > +};
> > +
> > +static const struct ds125df111_config {
> > + u8 vco_group[DS125DF111_NUM_VCO_GROUP_REG];
> > + u8 rate;
> > + u8 subrate;
> > +} ds125df111_cfg[] = {
> > + [FREQ_1G] = {
> > + /* VCO group #0 = 10GHz, VCO group #1 = 10GHz */
> > + .vco_group = {0x00, 0xB2, 0x00, 0xB2, 0xCC},
> > + /* By using the following combination of rate and subrate we
> > + * select divide ratios of 1, 2, 4, 8 on both groups
> > + */
> > + .rate = 0x1,
> > + .subrate = 0x2,
> > + },
> > +
> > + [FREQ_10G] = {
> > + /* VCO group #0 = 10.3125GHz, VCO group #1 = 10.3125GHz */
> > + .vco_group = {0x90, 0xB3, 0x90, 0xB3, 0xCD},
> > + /* By using the following combination of rate and subrate we
> > + * select divide ratios of 1 on both groups
> > + */
> > + .rate = 0x1,
> > + .subrate = 0x3,
> > + },
> > +};
> > +
> > +static int ds125df111_rmw(struct ds125df111_priv *priv, u8 reg, u8 clr, u8 set)
> > +{
> > + struct i2c_client *i2c = priv->client;
> > + int err;
> > + u8 val;
> > +
> > + err = i2c_smbus_read_byte_data(i2c, reg);
> > + if (err < 0)
> > + return err;
> > +
> > + val = (u8)err;
> > + val &= ~clr;
> > + val |= set;
> > +
> > + err = i2c_smbus_write_byte_data(i2c, reg, val);
> > + if (err < 0)
> > + return err;
> > +
> > + return 0;
> > +}
> > +
> > +static int ds125df111_configure(struct phy *phy,
> > + const struct ds125df111_config *cfg)
>
> This should start at preceding line open braces (here and few other
> places)
And it does start after the open braces of the previous line.
The .patch format adds that '+' which messes up with the alignment,
mainly when tabs are used. Once you apply the patch you can see that the
arguments start where they should.
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 3/4] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: sashiko-bot @ 2026-05-18 7:26 UTC (permalink / raw)
To: Hongling Zeng; +Cc: vkoul, linux-phy, neil.armstrong, olteanv
In-Reply-To: <20260518062938.48114-4-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch correctly fixes -EPROBE_DEFER propagation for `sysclk` and `refclk` but retains `dev_err()` for logging. This introduces dmesg spam because the probe will now be repeatedly deferred and retried, printing the error on every attempt.
--
commit 3f69854dd952e4e5aee73b24e6d27c10a31a39dc
Author: Hongling Zeng <zenghongling@kylinos.cn>
phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
This commit fixes probe deferral handling by propagating the actual error
codes for sysclk and refclk instead of returning -EINVAL or ignoring errors
completely.
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index 2d36fe4c4218c..9ec228c2a940c 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -608,14 +608,20 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
> struct clk *clk;
> struct device *dev = phy->dev;
>
> - phy->refclk = devm_clk_get(dev, "refclk");
> + /*
> + * refclk is optional for SATA PHY to support older DTBs, but
> + * required for other modes. Use devm_clk_get_optional() for SATA
> + * which returns NULL for -ENOENT, allowing us to propagate all
> + * other errors including -EPROBE_DEFER.
> + */
> + if (phy->mode == PIPE3_MODE_SATA)
> + phy->refclk = devm_clk_get_optional(dev, "refclk");
> + else
> + phy->refclk = devm_clk_get(dev, "refclk");
> +
> if (IS_ERR(phy->refclk)) {
> dev_err(dev, "unable to get refclk\n");
Now that the code correctly propagates -EPROBE_DEFER, will this dev_err()
call cause dmesg spam on systems where the clock provider is deferred and
retried multiple times?
Would it be better to replace dev_err() with dev_err_probe() here to cleanly
suppress the error message on deferrals and record the deferral reason?
> - /* older DTBs have missing refclk in SATA PHY
> - * so don't bail out in case of SATA PHY.
> - */
> - if (phy->mode != PIPE3_MODE_SATA)
> - return PTR_ERR(phy->refclk);
> + return PTR_ERR(phy->refclk);
> }
[ ... ]
> @@ -632,7 +638,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
> phy->sys_clk = devm_clk_get(dev, "sysclk");
> if (IS_ERR(phy->sys_clk)) {
> dev_err(dev, "unable to get sysclk\n");
Does this sysclk error handling also have the same logging issue when
returning -EPROBE_DEFER?
> - return -EINVAL;
> + return PTR_ERR(phy->sys_clk);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518062938.48114-1-zenghongling@kylinos.cn?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 phy-next 2/2] phy: ti: add PHY driver for TI DS125DF111 Dual-Channel Retimer
From: Ioana Ciornei @ 2026-05-18 7:29 UTC (permalink / raw)
To: Vinod Koul
Cc: neil.armstrong, robh, krzk+dt, conor+dt, johan, linux-phy,
devicetree, linux-kernel
In-Reply-To: <agnu5Mmf_d8zvWXH@vaman>
On Sun, May 17, 2026 at 10:07:56PM +0530, Vinod Koul wrote:
> On 16-05-26, 09:03, Ioana Ciornei wrote:
> > Add a generic PHY driver for the TI DS125DF111 Multi-Protocol
> > Dual-Channel Retimer. The driver currently supports only 10G and 1G link
> > speeds but it can easily extended to also cover other usecases.
> >
> > Since the available datasheet (https://www.ti.com/lit/gpn/DS125DF111)
> > does not name the registers, the name for the macros were determined by
> > their usage pattern.
> >
> > A PHY device is created for each of the two channels present on the
> > retimer. This allows for independent configuration of the two channels.
> > This capability is especially important on retimers which have more than
> > 2 channels that can be, depending on the board design, connected in
> > multiple different ways to the SerDes lanes.
> >
> > Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
> > ---
> > Changes in v3:
> > - Use reverse Christmas tree ordering
> > - Print a symbolic description in case of error
> > - Some words do not need to be capitalized
> > - Remove duplicated exit code path
> > - Return -EINVAL in case of unsupported submode received in .set_mode()
> > - Add a .validate() callback
> > - Remove comma after sentinel entry
> > - Add a ds125df111_rmw() helper
> > - Use read_poll_timeout() to wait for channel reset to complete
> >
> > Changes in v2:
> > - Explicitly include all the needed headers
> > - Change ds125df111_xlate() so that it returns an error if args_count is
> > not exactly 1
> > - Add a MAINTAINERS entry
> > ---
> > MAINTAINERS | 7 +
> > drivers/phy/ti/Kconfig | 10 ++
> > drivers/phy/ti/Makefile | 1 +
> > drivers/phy/ti/phy-ds125df111.c | 294 ++++++++++++++++++++++++++++++++
> > 4 files changed, 312 insertions(+)
> > create mode 100644 drivers/phy/ti/phy-ds125df111.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index f877e5aaf2c7..58f410b666e7 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -26781,6 +26781,13 @@ T: git git://linuxtv.org/mhadli/v4l-dvb-davinci_devices.git
> > F: drivers/media/platform/ti/davinci/
> > F: include/media/davinci/
> >
> > +TI DS125DF111 RETIMER PHY DRIVER
> > +M: Ioana Ciornei <ioana.ciornei@nxp.com>
> > +L: linux-phy@lists.infradead.org (moderated for non-subscribers)
> > +S: Maintained
> > +F: Documentation/devicetree/bindings/phy/ti,ds125df111.yaml
> > +F: drivers/phy/ti/phy-ds125df111.c
> > +
> > TI ENHANCED CAPTURE (eCAP) DRIVER
> > M: Vignesh Raghavendra <vigneshr@ti.com>
> > R: Julien Panis <jpanis@baylibre.com>
> > diff --git a/drivers/phy/ti/Kconfig b/drivers/phy/ti/Kconfig
> > index b40f28019131..475e80fcd52d 100644
> > --- a/drivers/phy/ti/Kconfig
> > +++ b/drivers/phy/ti/Kconfig
> > @@ -111,3 +111,13 @@ config PHY_TI_GMII_SEL
> > help
> > This driver supports configuring of the TI CPSW Port mode depending on
> > the Ethernet PHY connected to the CPSW Port.
> > +
> > +config PHY_TI_DS125DF111
>
> This should be in alphabetical order, so I guess before PHY_TI_G...
Now that I actually tried to make the change that you requested, I
realised that the Kconfig is not following any alphabetical order. And
neither does the Makefile.
Do you still want me to move the entries?
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 4/4] phy: ti-pipe3: Fix clock leak in init error path
From: sashiko-bot @ 2026-05-18 7:48 UTC (permalink / raw)
To: Hongling Zeng; +Cc: olteanv, neil.armstrong, linux-phy, vkoul
In-Reply-To: <20260518062938.48114-5-zenghongling@kylinos.cn>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Resource leak in ti_pipe3_init() for SATA mode late-lock race.
- [High] Unbalanced clock lifecycle operations for SATA mode due to early return in ti_pipe3_exit().
--
commit d2822879f79b4be953a465847a8621060caa36ac
Author: Hongling Zeng <zenghongling@kylinos.cn>
Subject: phy: ti-pipe3: Fix clock leak in init error path
This commit fixes a clock reference count leak in the PCIe error path of
ti_pipe3_init(). It explicitly calls ti_pipe3_disable_clocks() when
regmap_update_bits() fails, ensuring proper cleanup since the PHY framework
won't invoke the .exit callback on init failure.
Link: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index 9ec228c2a940c..4897e4ba2d7db 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -518,6 +518,8 @@ static int ti_pipe3_init(struct phy *x)
> val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
> ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
> PCIE_PCS_MASK, val);
> + if (ret)
> + ti_pipe3_disable_clocks(phy);
> return ret;
> }
>
This problem wasn't introduced by this patch, but a similar regression
exists in the error handling path of ti_pipe3_init() just below this block.
When bringing the DPLL out of IDLE, if ti_pipe3_dpll_wait_lock() times out,
it sets ret to -EBUSY. Immediately after, PLL_STATUS is read:
val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_CONFIGURATION2);
if (val & PLL_IDLE) {
val &= ~PLL_IDLE;
ti_pipe3_writel(phy->pll_ctrl_base, PLL_CONFIGURATION2, val);
ret = ti_pipe3_dpll_wait_lock(phy);
}
/* SATA has issues if re-programmed when locked */
val = ti_pipe3_readl(phy->pll_ctrl_base, PLL_STATUS);
if ((val & PLL_LOCK) && phy->mode == PIPE3_MODE_SATA)
return ret;
If the DPLL locked just after the timeout, the PLL_LOCK bit will be set.
For SATA mode, the function returns -EBUSY early without calling
ti_pipe3_disable_clocks().
Since the PHY framework won't invoke the exit callback on an init failure,
will this leak the clock reference counts?
Should the function either return 0 here to treat the late lock as a success,
or call ti_pipe3_disable_clocks() before returning the error?
Another regression that wasn't introduced by this patch exists in the driver's
lifecycle management for SATA mode.
In ti_pipe3_exit(), if the mode is SATA and dpll_reset_syscon is not present,
the function returns early to avoid powering down the SATA DPLL due to
Errata i783:
static int ti_pipe3_exit(struct phy *x)
{
struct ti_pipe3 *phy = phy_get_drvdata(x);
/* If dpll_reset_syscon is not present we wont power down SATA DPLL
* due to Errata i783
*/
if (phy->mode == PIPE3_MODE_SATA && !phy->dpll_reset_syscon)
return 0;
However, it returns without calling ti_pipe3_disable_clocks().
Because ti_pipe3_init() unconditionally calls ti_pipe3_enable_clocks(),
could repeated initialization and exit cycles, such as during suspend and
resume or module reload, invoke clk_prepare_enable() without matching
clk_disable_unprepare() calls?
If the errata strictly requires keeping the clocks enabled, does the driver
need to track the initialization state to avoid double-enabling? Alternatively,
should it properly disable the clocks before returning?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518062938.48114-1-zenghongling@kylinos.cn?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 4/4] phy: ti-pipe3: Fix clock leak in init error path
From: Hongling Zeng @ 2026-05-18 8:44 UTC (permalink / raw)
To: vkoul, neil.armstrong, johan, kishon, rogerq
Cc: Hongling Zeng, linux-phy, linux-kernel, Sashiko AI, stable
In-Reply-To: <20260518062938.48114-5-zenghongling@kylinos.cn>
--
Sashiko AI review ·https://sashiko.dev/#/patchset/20260518062938.48114-1-zenghongling@kylinos.cn?part=4
--
Hi:
Resend to fix threading / delivery issues.
Thank you for identifying these SATA mode issues. Both are pre-existing bugs in
the current codebase and were not introduced by this series.
This series focuses on specific EPROBE_DEFER and resource leak fixes. The SATA
mode issues you've found are valid but affect different code paths and should be
addressed separately to keep the patch series focused.
I can submit follow-up patches to fix these SATA mode issues. Would you prefer
them in v6 or as separate patches after this series?
Best regards,
Hongling
在 2026年05月18日 14:29, Hongling Zeng 写道:
> When regmap_update_bits() fails in ti_pipe3_init() for PCIe mode,
> the function returns the error without calling ti_pipe3_disable_clocks().
> This leaves the clocks permanently enabled since the PHY framework won't
> invoke the .exit callback on init failure.
>
> Fix this by adding proper clock cleanup in the PCIe error path, consistent
> with how the DPLL program error path handles cleanup.
>
> Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
> Reported-by: Sashiko AI <sashiko@kernel.org>
> Closes: https://lore.kernel.org/all/20260518023657.41852C2BCB0@smtp.kernel.org/
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> Cc: stable@vger.kernel.org
>
> ---
> Change in v5:
> -Add Fix ignored clock enable return value in init patch
> ---
> drivers/phy/ti/phy-ti-pipe3.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
> index 9ec228c2a940..4897e4ba2d7d 100644
> --- a/drivers/phy/ti/phy-ti-pipe3.c
> +++ b/drivers/phy/ti/phy-ti-pipe3.c
> @@ -518,6 +518,8 @@ static int ti_pipe3_init(struct phy *x)
> val = 0x96 << OMAP_CTRL_PCIE_PCS_DELAY_COUNT_SHIFT;
> ret = regmap_update_bits(phy->pcs_syscon, phy->pcie_pcs_reg,
> PCIE_PCS_MASK, val);
> + if (ret)
> + ti_pipe3_disable_clocks(phy);
> return ret;
> }
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v5 3/4] phy: ti-pipe3: Fix EPROBE_DEFER handling for clock resources
From: Hongling Zeng @ 2026-05-18 8:52 UTC (permalink / raw)
To: Vinod Koul, neil.armstrong, johan, kishon, rogerq, sashiko-bo
Cc: zenghongling, linux-phy, linux-kernel
In-Reply-To: <6A0AC942.40201@126.com>
--
Sashiko AI review ·https://sashiko.dev/#/patchset/20260518062938.48114-1-zenghongling@kylinos.cn?part=3
--
Hi,
Resend to fix threading / delivery issues.
Thank you for the review. You raise a valid point about using dev_err_probe()
to avoid dmesg spam during probe deferral.
I've kept dev_err() in this patch to maintain consistency with the existing
code style and focus on the core EPROBE_DEFER fixes. However, I agree that
dev_err_probe() would be better and can submit a follow-up cleanup patch to
address the logging improvement.
Does this approach work for you, or would you prefer I include the dev_err_probe()
change in this series?
Best regards,
Hongling
在 2026年05月18日 16:09, Hongling Zeng 写道:
> --
> Sashiko AI review ·https://sashiko.dev/#/patchset/20260518062938.48114-1-zenghongling@kylinos.cn?part=3
>
> --
> Hi,
>
> Thank you for the review. You raise a valid point about using dev_err_probe()
> to avoid dmesg spam during probe deferral.
>
> I've kept dev_err() in this patch to maintain consistency with the existing
> code style and focus on the core EPROBE_DEFER fixes. However, I agree that
> dev_err_probe() would be better and can submit a follow-up cleanup patch to
> address the logging improvement.
>
> Does this approach work for you, or would you prefer I include the dev_err_probe()
> change in this series?
>
> Best regards,
> Hongling
>
>
> 在 2026年05月18日 14:29, Hongling Zeng 写道:
>> ti_pipe3_get_clk() has two issues with -EPROBE_DEFER error handling:
>>
>> 1. When devm_clk_get() for sysclk fails, the function returns -EINVAL
>> instead of propagating the actual error code. This masks -EPROBE_DEFER
>> to -EINVAL, breaking the probe deferral mechanism and causing permanent
>> driver initialization failure on systems with non-deterministic probe
>> ordering.
>>
>> 2. For SATA PHY refclk, the function ignores all errors to support older
>> DTBs missing the refclk property. However, this incorrectly ignores
>> -EPROBE_DEFER as well, causing the driver to proceed without waiting
>> for the clock provider to become available.
>>
>> Fix both issues:
>> - Return PTR_ERR(phy->sys_clk) instead of -EINVAL to propagate all
>> error codes including -EPROBE_DEFER
>> - Use devm_clk_get_optional() for SATA refclk to handle optional
>> clocks while propagating -EPROBE_DEFER and other errors
>>
>> Fixes: a70143bbef6b ("drivers: phy: usb3/pipe3: Adapt pipe3 driver to Generic PHY Framework")
>> Fixes: 7f33912d2978 ("phy: ti-pipe3: Fix SATA across suspend/resume")
>> Signed-off-by: Hongling Zeng<zenghongling@kylinos.cn>
>>
>> ---
>> Change in v4:
>> - Merge refclk leak fix and EPROBE_DEFER fix into a single patch
>> - Use devm_clk_get_optional() for SATA refclk
>> - Drop manual -ENOENT handling
>> - Ensure error paths are fully symmetric
>> ---
>> Change in v5:
>> -Add Fix ignored clock enable return value in init patch
>> ---
>> drivers/phy/ti/phy-ti-pipe3.c | 20 +++++++++++++-------
>> 1 file changed, 13 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/phy/ti/phy-ti-pipe3.c b/drivers/phy/ti/phy-ti-pipe3.c
>> index 2d36fe4c4218..9ec228c2a940 100644
>> --- a/drivers/phy/ti/phy-ti-pipe3.c
>> +++ b/drivers/phy/ti/phy-ti-pipe3.c
>> @@ -608,14 +608,20 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>> struct clk *clk;
>> struct device *dev = phy->dev;
>>
>> - phy->refclk = devm_clk_get(dev, "refclk");
>> + /*
>> + * refclk is optional for SATA PHY to support older DTBs, but
>> + * required for other modes. Use devm_clk_get_optional() for SATA
>> + * which returns NULL for -ENOENT, allowing us to propagate all
>> + * other errors including -EPROBE_DEFER.
>> + */
>> + if (phy->mode == PIPE3_MODE_SATA)
>> + phy->refclk = devm_clk_get_optional(dev, "refclk");
>> + else
>> + phy->refclk = devm_clk_get(dev, "refclk");
>> +
>> if (IS_ERR(phy->refclk)) {
>> dev_err(dev, "unable to get refclk\n");
>> - /* older DTBs have missing refclk in SATA PHY
>> - * so don't bail out in case of SATA PHY.
>> - */
>> - if (phy->mode != PIPE3_MODE_SATA)
>> - return PTR_ERR(phy->refclk);
>> + return PTR_ERR(phy->refclk);
>> }
>>
>> if (phy->mode != PIPE3_MODE_SATA) {
>> @@ -632,7 +638,7 @@ static int ti_pipe3_get_clk(struct ti_pipe3 *phy)
>> phy->sys_clk = devm_clk_get(dev, "sysclk");
>> if (IS_ERR(phy->sys_clk)) {
>> dev_err(dev, "unable to get sysclk\n");
>> - return -EINVAL;
>> + return PTR_ERR(phy->sys_clk);
>> }
>> }
>>
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4 v2] phy: s32g: Add serdes xpcs subsystem
From: Enric Balletbo i Serra @ 2026-05-18 9:11 UTC (permalink / raw)
To: Vincent Guittot
Cc: Russell King (Oracle), vkoul, neil.armstrong, krzk+dt, conor+dt,
ciprianmarian.costea, s32, p.zabel, ghennadi.procopciuc,
Ionut.Vicovan, linux-phy, devicetree, linux-kernel,
linux-arm-kernel, netdev, horms, Frank.li
In-Reply-To: <CAKfTPtAAmwjwSJ_5kfARaOns-g5tnXdeGM4s2JOy5vnhNTTXYQ@mail.gmail.com>
Hi all,
Just in case I missed something.
On Thu, Feb 5, 2026 at 6:03 PM Vincent Guittot
<vincent.guittot@linaro.org> wrote:
>
Is there a new version of this patchset? I tried applying this one on
top of the current mainline, building it as a module, and I received
different build errors:
ERROR: modpost: missing MODULE_LICENSE() in drivers/net/pcs/pcs-nxp-s32g-xpcs.o
WARNING: modpost: missing MODULE_DESCRIPTION() in
drivers/net/pcs/pcs-nxp-s32g-xpcs.o
ERROR: modpost: "s32g_xpcs_init"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_init_plls"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_pre_pcie_2g5"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_vreset"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_wait_vreset"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_reset_rx"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
ERROR: modpost: "s32g_xpcs_disable_an"
[drivers/phy/freescale/phy-nxp-s32g-serdes.ko] undefined!
Thanks,
Enric
> On Wed, 4 Feb 2026 at 16:29, Russell King (Oracle)
> <linux@armlinux.org.uk> wrote:
> >
> > Sorry, I don't have time to finish this review, nor cut down the context
> > as I normally would do... I'm being bothered on company Slack, which now
> > has the really bloody annoying feature of popping up a window rather than
> > using KDE's notification subsystem, and that steals keyboard focus away
> > from whatever one is trying to do at the time.
> >
> > On Tue, Feb 03, 2026 at 05:19:16PM +0100, Vincent Guittot wrote:
> > > +static bool s32g_xpcs_poll_timeout(struct s32g_xpcs *xpcs, xpcs_poll_func_t func,
> > > + ktime_t timeout)
> > > +{
> > > + ktime_t cur = ktime_get();
> > > +
> > > + return func(xpcs) || ktime_after(cur, timeout);
> > > +}
> > > +
> > > +static int s32g_xpcs_wait(struct s32g_xpcs *xpcs, xpcs_poll_func_t func)
> > > +{
> > > + ktime_t timeout = ktime_add_ms(ktime_get(), XPCS_TIMEOUT_MS);
> > > +
> > > + spin_until_cond(s32g_xpcs_poll_timeout(xpcs, func, timeout));
> > > + if (!func(xpcs))
> > > + return -ETIMEDOUT;
> >
> > XPCS_TIMEOUT_MS is 300ms. spin_until_cond() spins until the condition is
> > true. Do you need to tie up this CPU for up to 300ms? That seems
> > excessive. What is the reason that read_poll_timeout() or similar
> > couldn't be used?
>
> This needs additional tests because some instabilities have been
> reported when using read_poll_timeout in some places
>
> >
> > The advantage of read_poll_timeout() is that it will correctly handle
> > the timeout vs condition being satisfied witout need for special code.
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int s32g_xpcs_wait_bits(struct s32g_xpcs *xpcs, unsigned int reg,
> > > + unsigned int mask, unsigned int bits)
> > > +{
> > > + ktime_t cur;
> > > + ktime_t timeout = ktime_add_ms(ktime_get(), XPCS_TIMEOUT_MS);
> > > +
> > > + spin_until_cond((cur = ktime_get(),
> > > + (s32g_xpcs_read(xpcs, reg) & mask) == bits ||
> > > + ktime_after(cur, timeout)));
> > > + if ((s32g_xpcs_read(xpcs, reg) & mask) != bits)
> > > + return -ETIMEDOUT;
> >
> > Same here:
> >
> > return read_poll_timeout(s32g_xpcs_read, val, (val & mask) == bits,
> > 0, XPCS_TIMEOUT_MS, false,
> > xpcs, reg);
> >
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static unsigned int s32g_xpcs_digital_status(struct s32g_xpcs *xpcs)
> > > +{
> > > + return s32g_xpcs_read(xpcs, VR_MII_DIG_STS);
> > > +}
> > > +
> > > +static int s32g_xpcs_wait_power_good_state(struct s32g_xpcs *xpcs)
> > > +{
> > > + unsigned int val;
> > > +
> > > + return read_poll_timeout(s32g_xpcs_digital_status, val,
> > > + FIELD_GET(PSEQ_STATE_MASK, val) == POWER_GOOD_STATE,
> > > + 0,
> > > + XPCS_TIMEOUT_MS, false, xpcs);
> >
> > This could be:
> > return read_poll_timeout(s32g_xpcs_read, val,
> > FIELD_GET(PSEQ_STATE_MASK, val) == POWER_GOOD_STATE,
> > 0, XPCS_TIMEOUT_MS, false,
> > xpcs, VR_MII_DIG_STS);
> >
> > eliminating the need for s32g_xpcs_digital_status().
>
> fair enough
>
> >
> > > +}
> > > +
> > > +void s32g_xpcs_vreset(struct s32g_xpcs *xpcs)
> > > +{
> > > + /* Step 19 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, VR_RST, VR_RST);
> > > +}
> > > +
> > > +static bool s32g_xpcs_is_not_in_reset(struct s32g_xpcs *xpcs)
> > > +{
> > > + unsigned int val;
> > > +
> > > + val = s32g_xpcs_read(xpcs, VR_MII_DIG_CTRL1);
> > > +
> > > + return !(val & VR_RST);
> > > +}
> > > +
> > > +int s32g_xpcs_wait_vreset(struct s32g_xpcs *xpcs)
> > > +{
> > > + int ret;
> > > +
> > > + /* Step 20 */
> > > + ret = s32g_xpcs_wait(xpcs, s32g_xpcs_is_not_in_reset);
> > > + if (ret)
> > > + dev_err(xpcs->dev, "XPCS%d is in reset\n", xpcs->id);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +int s32g_xpcs_reset_rx(struct s32g_xpcs *xpcs)
> > > +{
> > > + int ret = 0;
> > > +
> > > + ret = s32g_xpcs_wait_power_good_state(xpcs);
> > > + if (ret) {
> > > + dev_err(xpcs->dev, "Failed to enter in PGOOD state after vendor reset\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Step 21 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_RX_GENCTRL1,
> > > + RX_RST_0, RX_RST_0);
> > > +
> > > + /* Step 22 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_RX_GENCTRL1,
> > > + RX_RST_0, 0);
> > > +
> > > + /* Step 23 */
> > > + /* Wait until SR_MII_STS[LINK_STS] = 1 */
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +static int s32g_xpcs_ref_clk_sel(struct s32g_xpcs *xpcs,
> > > + enum s32g_xpcs_pll ref_pll)
> > > +{
> > > + switch (ref_pll) {
> > > + case XPCS_PLLA:
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLL_CMN_CTRL,
> > > + MPLLB_SEL_0, 0);
> > > + xpcs->ref = XPCS_PLLA;
> > > + break;
> > > + case XPCS_PLLB:
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLL_CMN_CTRL,
> > > + MPLLB_SEL_0, MPLLB_SEL_0);
> > > + xpcs->ref = XPCS_PLLB;
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void s32g_xpcs_electrical_configure(struct s32g_xpcs *xpcs)
> > > +{
> > > + /* Step 2 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_EQ_CTRL0,
> > > + TX_EQ_MAIN_MASK, FIELD_PREP(TX_EQ_MAIN_MASK, 0xC));
> >
> > Prefer hex numbers to be lower case.
>
> ok
>
> >
> > > +
> > > + /* Step 3 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_CONSUMER_10G_TX_TERM_CTRL,
> > > + TX0_TERM_MASK, FIELD_PREP(TX0_TERM_MASK, 0x4));
> > > +}
> > > +
> > > +static int s32g_xpcs_vco_cfg(struct s32g_xpcs *xpcs, enum s32g_xpcs_pll vco_pll)
> > > +{
> > > + unsigned int vco_ld = 0;
> > > + unsigned int vco_ref = 0;
> > > + unsigned int rx_baud = 0;
> > > + unsigned int tx_baud = 0;
> > > +
> > > + switch (vco_pll) {
> > > + case XPCS_PLLA:
> > > + if (xpcs->mhz125) {
> > > + vco_ld = FIELD_PREP(VCO_LD_VAL_0_MASK, 1360);
> > > + vco_ref = FIELD_PREP(VCO_REF_LD_0_MASK, 17);
> > > + } else {
> > > + vco_ld = FIELD_PREP(VCO_LD_VAL_0_MASK, 1350);
> > > + vco_ref = FIELD_PREP(VCO_REF_LD_0_MASK, 27);
> > > + }
> > > +
> > > + rx_baud = FIELD_PREP(RX0_RATE_MASK, RX0_BAUD_DIV_8);
> > > + tx_baud = FIELD_PREP(TX0_RATE_MASK, TX0_BAUD_DIV_4);
> > > + break;
> > > + case XPCS_PLLB:
> > > + if (xpcs->mhz125) {
> > > + vco_ld = FIELD_PREP(VCO_LD_VAL_0_MASK, 1350);
> > > + vco_ref = FIELD_PREP(VCO_REF_LD_0_MASK, 27);
> > > + } else {
> > > + vco_ld = FIELD_PREP(VCO_LD_VAL_0_MASK, 1344);
> > > + vco_ref = FIELD_PREP(VCO_REF_LD_0_MASK, 43);
> > > + }
> > > +
> > > + rx_baud = FIELD_PREP(RX0_RATE_MASK, RX0_BAUD_DIV_2);
> > > + tx_baud = FIELD_PREP(TX0_RATE_MASK, TX0_BAUD_DIV_1);
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_VCO_CAL_LD0,
> > > + VCO_LD_VAL_0_MASK, vco_ld);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_VCO_CAL_REF0,
> > > + VCO_REF_LD_0_MASK, vco_ref);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_RATE_CTRL,
> > > + TX0_RATE_MASK, tx_baud);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_RX_RATE_CTRL,
> > > + RX0_RATE_MASK, rx_baud);
> > > +
> > > + if (vco_pll == XPCS_PLLB) {
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_CDR_CTRL,
> > > + VCO_LOW_FREQ_0, VCO_LOW_FREQ_0);
> > > + } else {
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_CDR_CTRL,
> > > + VCO_LOW_FREQ_0, 0);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int s32g_xpcs_init_mplla(struct s32g_xpcs *xpcs)
> > > +{
> > > + unsigned int val;
> > > +
> > > + if (!xpcs)
> > > + return -EINVAL;
> > > +
> > > + /* Step 7 */
> > > + val = 0;
> > > + if (xpcs->ext_clk)
> > > + val |= REF_USE_PAD;
> > > +
> > > + if (xpcs->mhz125) {
> > > + val |= REF_MPLLA_DIV2;
> > > + val |= REF_CLK_DIV2;
> > > + val |= FIELD_PREP(REF_RANGE_MASK, RANGE_52_78_MHZ);
> > > + } else {
> > > + val |= FIELD_PREP(REF_RANGE_MASK, RANGE_26_53_MHZ);
> > > + }
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_REF_CLK_CTRL,
> > > + REF_MPLLA_DIV2 | REF_USE_PAD | REF_RANGE_MASK |
> > > + REF_CLK_DIV2, val);
> > > +
> > > + /* Step 8 */
> > > + if (xpcs->mhz125)
> > > + val = FIELD_PREP(MLLA_MULTIPLIER_MASK, 80);
> > > + else
> > > + val = FIELD_PREP(MLLA_MULTIPLIER_MASK, 25);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLLA_CTRL0,
> > > + MPLLA_CAL_DISABLE | MLLA_MULTIPLIER_MASK,
> > > + val);
> > > +
> > > + /* Step 9 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_MPLLA_CTRL1,
> > > + MPLLA_FRACN_CTRL_MASK, 0);
> > > +
> > > + /* Step 10 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLLA_CTRL2,
> > > + MPLLA_TX_CLK_DIV_MASK | MPLLA_DIV10_CLK_EN,
> > > + FIELD_PREP(MPLLA_TX_CLK_DIV_MASK, 1) | MPLLA_DIV10_CLK_EN);
> > > +
> > > + /* Step 11 */
> > > + if (xpcs->mhz125)
> > > + val = FIELD_PREP(MPLLA_BANDWIDTH_MASK, 43);
> > > + else
> > > + val = FIELD_PREP(MPLLA_BANDWIDTH_MASK, 357);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_MPLLA_CTRL3,
> > > + MPLLA_BANDWIDTH_MASK, val);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int s32g_xpcs_init_mpllb(struct s32g_xpcs *xpcs)
> > > +{
> > > + unsigned int val;
> > > +
> > > + if (!xpcs)
> > > + return -EINVAL;
> > > +
> > > + /* Step 7 */
> > > + val = 0;
> > > + if (xpcs->ext_clk)
> > > + val |= REF_USE_PAD;
> > > +
> > > + if (xpcs->mhz125) {
> > > + val |= REF_MPLLB_DIV2;
> > > + val |= REF_CLK_DIV2;
> > > + val |= FIELD_PREP(REF_RANGE_MASK, RANGE_52_78_MHZ);
> > > + } else {
> > > + val |= FIELD_PREP(REF_RANGE_MASK, RANGE_26_53_MHZ);
> > > + }
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_REF_CLK_CTRL,
> > > + REF_MPLLB_DIV2 | REF_USE_PAD | REF_RANGE_MASK |
> > > + REF_CLK_DIV2, val);
> > > +
> > > + /* Step 8 */
> > > + if (xpcs->mhz125)
> > > + val = 125 << MLLB_MULTIPLIER_OFF;
> > > + else
> > > + val = 39 << MLLB_MULTIPLIER_OFF;
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLLB_CTRL0,
> > > + MPLLB_CAL_DISABLE | MLLB_MULTIPLIER_MASK,
> > > + val);
> > > +
> > > + /* Step 9 */
> > > + if (xpcs->mhz125)
> > > + val = FIELD_PREP(MPLLB_FRACN_CTRL_MASK, 0);
> > > + else
> > > + val = FIELD_PREP(MPLLB_FRACN_CTRL_MASK, 1044);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_MPLLB_CTRL1,
> > > + MPLLB_FRACN_CTRL_MASK, val);
> > > +
> > > + /* Step 10 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MPLLB_CTRL2,
> > > + MPLLB_TX_CLK_DIV_MASK | MPLLB_DIV10_CLK_EN,
> > > + FIELD_PREP(MPLLB_TX_CLK_DIV_MASK, 5) | MPLLB_DIV10_CLK_EN);
> > > +
> > > + /* Step 11 */
> > > + if (xpcs->mhz125)
> > > + val = FIELD_PREP(MPLLB_BANDWIDTH_MASK, 68);
> > > + else
> > > + val = FIELD_PREP(MPLLB_BANDWIDTH_MASK, 102);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_MPLLB_CTRL3,
> > > + MPLLB_BANDWIDTH_MASK, val);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void s32g_serdes_pma_high_freq_recovery(struct s32g_xpcs *xpcs)
> > > +{
> > > + /* PCS signal protection, PLL railout recovery */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DBG_CTRL, SUPPRESS_LOS_DET | RX_DT_EN_CTL,
> > > + SUPPRESS_LOS_DET | RX_DT_EN_CTL);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_MISC_CTRL0,
> > > + PLL_CTRL, PLL_CTRL);
> > > +}
> > > +
> > > +static void s32g_serdes_pma_configure_tx_eq_post(struct s32g_xpcs *xpcs)
> > > +{
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_EQ_CTRL1,
> > > + TX_EQ_OVR_RIDE, TX_EQ_OVR_RIDE);
> > > +}
> > > +
> > > +static int s32g_serdes_bifurcation_pll_transit(struct s32g_xpcs *xpcs,
> > > + enum s32g_xpcs_pll target_pll)
> > > +{
> > > + int ret = 0;
> > > + struct device *dev = xpcs->dev;
> > > +
> > > + /* Configure XPCS speed and VCO */
> > > + if (target_pll == XPCS_PLLA) {
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, EN_2_5G_MODE, 0);
> > > + s32g_xpcs_vco_cfg(xpcs, XPCS_PLLA);
> > > + } else {
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1,
> > > + EN_2_5G_MODE, EN_2_5G_MODE);
> > > + s32g_xpcs_vco_cfg(xpcs, XPCS_PLLB);
> > > + }
> >
> > I am really not happy with this driver being
> > PHY_INTERFACE_MODE_SGMII-only but supporting running that at 2.5Gbps.
> > In the kernel, PHY_INTERFACE_MODE_SGMII is strictly _Cisco_ SGMII only,
> > which means the version of it clocked at 1.25GHz, not 3.125GHz.
> >
> > OCSGMII or whatever random name you call it tends to be only supported
> > without inband AN, and we have pushed everyone to adopt
> > PHY_INTERFACE_MODE_2500BASEX for that. Please do the same.
> >
> > Should this SerDes be connected to a SFP cage, it will need to support
> > dynamically switching between Cisco SGMII and 2500BASE-X.
>
> okay, this needs to be checked with SoC Team
>
> >
> > > +
> > > + /* Signal that clock are not available */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_GENCTRL1,
> > > + TX_CLK_RDY_0, 0);
> > > +
> > > + /* Select PLL reference */
> > > + if (target_pll == XPCS_PLLA)
> > > + s32g_xpcs_ref_clk_sel(xpcs, XPCS_PLLA);
> > > + else
> > > + s32g_xpcs_ref_clk_sel(xpcs, XPCS_PLLB);
> > > +
> > > + /* Initiate transmitter TX reconfiguration request */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_GENCTRL2,
> > > + TX_REQ_0, TX_REQ_0);
> > > +
> > > + /* Wait for transmitter to reconfigure */
> > > + ret = s32g_xpcs_wait_bits(xpcs, VR_MII_GEN5_12G_16G_TX_GENCTRL2,
> > > + TX_REQ_0, 0);
> > > + if (ret) {
> > > + dev_err(dev, "Switch to TX_REQ_0 failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Initiate transmitter RX reconfiguration request */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_RX_GENCTRL2,
> > > + RX_REQ_0, RX_REQ_0);
> > > +
> > > + /* Wait for receiver to reconfigure */
> > > + ret = s32g_xpcs_wait_bits(xpcs, VR_MII_GEN5_12G_16G_RX_GENCTRL2,
> > > + RX_REQ_0, 0);
> > > + if (ret) {
> > > + dev_err(dev, "Switch to RX_REQ_0 failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + /* Signal that clock are available */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_GENCTRL1,
> > > + TX_CLK_RDY_0, TX_CLK_RDY_0);
> > > +
> > > + /* Flush internal logic */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, INIT, INIT);
> > > +
> > > + /* Wait for init */
> > > + ret = s32g_xpcs_wait_bits(xpcs, VR_MII_DIG_CTRL1, INIT, 0);
> > > + if (ret) {
> > > + dev_err(dev, "XPCS INIT failed\n");
> > > + return ret;
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/*
> > > + * phylink_pcs_ops
> > > + */
> > > +
> > > +static unsigned int s32cc_phylink_pcs_inband_caps(struct phylink_pcs *pcs,
> > > + phy_interface_t interface)
> > > +{
> > > + switch (interface) {
> > > + case PHY_INTERFACE_MODE_SGMII:
> > > + return LINK_INBAND_DISABLE | LINK_INBAND_ENABLE;
> > > +
> > > + default:
> > > + return 0;
> > > + }
> > > +}
> > > +
> > > +static int s32cc_phylink_pcs_config(struct phylink_pcs *pcs,
> > > + unsigned int neg_mode,
> > > + phy_interface_t interface,
> > > + const unsigned long *advertising,
> > > + bool permit_pause_to_mac)
> > > +{
> > > + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> > > +
> > > + /* Step 1: Disable SGMII AN */
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, 0);
> > > +
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL,
> > > + MII_AN_INTR_EN,
> > > + 0);
> > > +
> > > + if (!(neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED))
> > > + return 0;
> > > +
> > > + /* Step 2 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL,
> > > + PCS_MODE_MASK,
> > > + FIELD_PREP(PCS_MODE_MASK, PCS_MODE_SGMII));
> > > +
> > > + /* Step 3 */
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL,
> > > + SS13 | SS6,
> > > + SS6);
> > > +
> > > + /* Step 4 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL,
> > > + MII_CTRL,
> > > + 0);
> > > + /* Step 5 and 8 */
> > > + if (xpcs->pcie_shared == PCIE_XPCS_2G5) {
> > > + s32g_xpcs_write(xpcs, VR_MII_LINK_TIMER_CTRL, 0x2faf);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1,
> > > + MAC_AUTO_SW, MAC_AUTO_SW);
> > > + } else {
> > > + s32g_xpcs_write(xpcs, VR_MII_LINK_TIMER_CTRL, 0x7a1);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, MAC_AUTO_SW, 0);
> > > + }
> > > +
> > > + /* Step 6 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1,
> > > + CL37_TMR_OVRRIDE, CL37_TMR_OVRRIDE);
> > > +
> > > + /* Step 7 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL,
> > > + MII_AN_INTR_EN,
> > > + MII_AN_INTR_EN);
> > > +
> > > + /* Step 9: Enable SGMII AN */
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, AN_ENABLE);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static void s32cc_phylink_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
> > > + struct phylink_link_state *state)
> > > +{
> > > + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> > > + bool ss6, ss13, an_enabled;
> > > + struct device *dev = xpcs->dev;
> > > + unsigned int val, ss;
> > > +
> > > + an_enabled = (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED);
> > > +
> > > + if (an_enabled) {
> > > + state->link = 0;
> > > + val = s32g_xpcs_read(xpcs, VR_MII_AN_INTR_STS);
> > > +
> > > + /* Interrupt is raised with each SGMII AN that is in cases
> > > + * Link down - Every SGMII link timer expire
> > > + * Link up - Once before link goes up
> > > + * So either linkup or raised interrupt mean AN was completed
> > > + */
> > > + if ((val & CL37_ANCMPLT_INTR) || (val & CL37_ANSGM_STS_LINK)) {
> > > + state->an_complete = 1;
> > > + if (val & CL37_ANSGM_STS_LINK)
> > > + state->link = 1;
> > > + else
> > > + return;
> > > + if (val & CL37_ANSGM_STS_DUPLEX)
> > > + state->duplex = DUPLEX_FULL;
> > > + else
> > > + state->duplex = DUPLEX_HALF;
> > > + ss = FIELD_GET(CL37_ANSGM_STS_SPEED_MASK, val);
> > > + } else {
> > > + return;
> > > + }
> > > +
> > > + } else {
> > > + val = s32g_xpcs_read(xpcs, SR_MII_STS);
> > > + state->link = !!(val & LINK_STS);
> > > + state->an_complete = 0;
> > > + state->pause = MLO_PAUSE_NONE;
> > > +
> > > + val = s32g_xpcs_read(xpcs, SR_MII_CTRL);
> > > + if (val & DUPLEX_MODE)
> > > + state->duplex = DUPLEX_FULL;
> > > + else
> > > + state->duplex = DUPLEX_HALF;
> > > +
> > > + /*
> > > + * Build similar value as CL37_ANSGM_STS_SPEED with
> > > + * SS6 and SS13 of SR_MII_CTRL:
> > > + * - 0 for 10 Mbps
> > > + * - 1 for 100 Mbps
> > > + * - 2 for 1000 Mbps
> > > + */
> > > + ss6 = !!(val & SS6);
> > > + ss13 = !!(val & SS13);
> > > + ss = ss6 << 1 | ss13;
> > > + }
> > > +
> > > + switch (ss) {
> > > + case CL37_ANSGM_10MBPS:
> > > + state->speed = SPEED_10;
> > > + break;
> > > + case CL37_ANSGM_100MBPS:
> > > + state->speed = SPEED_100;
> > > + break;
> > > + case CL37_ANSGM_1000MBPS:
> > > + state->speed = SPEED_1000;
> > > + break;
> > > + default:
> > > + dev_err(dev, "Failed to interpret the value of SR_MII_CTRL\n");
> > > + break;
> > > + }
> > > +
> > > + val = s32g_xpcs_read(xpcs, VR_MII_DIG_CTRL1);
> > > + if ((val & EN_2_5G_MODE) && state->speed == SPEED_1000)
> > > + state->speed = SPEED_2500;
> > > +
> > > + /* Cover SGMII AN inability to distigunish between 1G and 2.5G */
> > > + if ((val & EN_2_5G_MODE) &&
> > > + state->speed != SPEED_2500 && an_enabled) {
> > > + dev_err(dev, "Speed not supported in SGMII AN mode\n");
> > > + }
> > > +}
> > > +
> > > +static void s32cc_phylink_pcs_link_up(struct phylink_pcs *pcs,
> > > + unsigned int neg_mode,
> > > + phy_interface_t interface, int speed,
> > > + int duplex)
> > > +{
> > > + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> > > + struct device *dev = xpcs->dev;
> > > + bool an_enabled = (neg_mode == PHYLINK_PCS_NEG_INBAND_ENABLED);
> > > + unsigned int val;
> > > + int ret;
> > > +
> > > + dev_dbg(dev, "xpcs_%d: speed=%u duplex=%d an=%d\n", xpcs->id,
> > > + speed, duplex, an_enabled);
> > > +
> > > + if (an_enabled)
> > > + return;
> > > +
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, 0);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL, MII_AN_INTR_EN, 0);
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_AN_CTRL, MII_CTRL, 0);
> >
> > Haven't you already disabled AN in .pcs_config() ? This method doesn't
> > change the AN enable state, the only time that happens is when
> > .pcs_config() will be called. All other cases of passing neg_mode are
> > merely informational.
>
> Fair enough, this is probably not needed anymore with the changes in .pcs_config
>
> >
> > > +
> > > + if (duplex == DUPLEX_FULL)
> > > + val = DUPLEX_MODE;
> > > + else
> > > + val = 0;
> > > +
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, DUPLEX_MODE, val);
> > > +
> > > + switch (speed) {
> > > + case SPEED_10:
> > > + val = 0;
> > > + break;
> > > + case SPEED_100:
> > > + val = SS13;
> > > + break;
> > > + case SPEED_1000:
> > > + val = SS6;
> > > + break;
> > > + case SPEED_2500:
> > > + val = SS6;
> > > + break;
> > > + default:
> > > + dev_err(dev, "Speed not supported\n");
> > > + break;
> > > + }
> > > +
> > > + if (speed == SPEED_2500) {
> > > + ret = s32g_serdes_bifurcation_pll_transit(xpcs, XPCS_PLLB);
> > > + if (ret)
> > > + dev_err(dev, "Switch to PLLB failed\n");
> > > + } else {
> > > + ret = s32g_serdes_bifurcation_pll_transit(xpcs, XPCS_PLLA);
> > > + if (ret)
> > > + dev_err(dev, "Switch to PLLA failed\n");
> > > + }
> >
> > This is a protocol transition, and isn't something that can be handled
> > here. Cisco SGMII (PHY_INTERFACE_MODE_SGMII) does not support 2500Mbps
> > and phylink will not allow it.
> >
> > See my comments for s32g_serdes_bifurcation_pll_transit().
> >
> > > +
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, SS6 | SS13, val);
> > > +}
> > > +
> > > +static const struct phylink_pcs_ops s32cc_phylink_pcs_ops = {
> > > + .pcs_inband_caps = s32cc_phylink_pcs_inband_caps,
> > > + .pcs_get_state = s32cc_phylink_pcs_get_state,
> > > + .pcs_config = s32cc_phylink_pcs_config,
> > > + .pcs_link_up = s32cc_phylink_pcs_link_up,
> > > +};
> > > +
> > > +/*
> > > + * Serdes functions for initializing/configuring/releasing the xpcs
> > > + */
> > > +
> > > +int s32g_xpcs_pre_pcie_2g5(struct s32g_xpcs *xpcs)
> > > +{
> > > + int ret;
> > > +
> > > + /* Enable voltage boost */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_GENCTRL1, VBOOST_EN_0,
> > > + VBOOST_EN_0);
> > > +
> > > + /* TX rate baud */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_TX_RATE_CTRL, 0x7, 0x0U);
> > > +
> > > + /* Rx rate baud/2 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_RX_RATE_CTRL, 0x3U, 0x1U);
> > > +
> > > + /* Set low-frequency operating band */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_GEN5_12G_16G_CDR_CTRL, CDR_SSC_EN_0,
> > > + VCO_LOW_FREQ_0);
> > > +
> > > + ret = s32g_serdes_bifurcation_pll_transit(xpcs, XPCS_PLLB);
> > > + if (ret)
> > > + dev_err(xpcs->dev, "Switch to PLLB failed\n");
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +int s32g_xpcs_init_plls(struct s32g_xpcs *xpcs)
> > > +{
> > > + int ret;
> > > + struct device *dev = xpcs->dev;
> > > +
> > > + if (!xpcs->ext_clk) {
> > > + /* Step 1 */
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, BYP_PWRUP, BYP_PWRUP);
> > > + } else if (xpcs->pcie_shared == NOT_SHARED) {
> > > + ret = s32g_xpcs_wait_power_good_state(xpcs);
> > > + if (ret)
> > > + return ret;
> > > + } else if (xpcs->pcie_shared == PCIE_XPCS_2G5) {
> > > + ret = s32g_xpcs_wait_power_good_state(xpcs);
> > > + if (ret)
> > > + return ret;
> > > + /* Configure equalization */
> > > + s32g_serdes_pma_configure_tx_eq_post(xpcs);
> > > + s32g_xpcs_electrical_configure(xpcs);
> > > +
> > > + /* Enable receiver recover */
> > > + s32g_serdes_pma_high_freq_recovery(xpcs);
> > > + return 0;
> > > + }
> > > +
> > > + s32g_xpcs_electrical_configure(xpcs);
> > > +
> > > + s32g_xpcs_ref_clk_sel(xpcs, XPCS_PLLA);
> > > + ret = s32g_xpcs_init_mplla(xpcs);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to initialize PLLA\n");
> > > + return ret;
> > > + }
> > > + ret = s32g_xpcs_init_mpllb(xpcs);
> > > + if (ret) {
> > > + dev_err(dev, "Failed to initialize PLLB\n");
> > > + return ret;
> > > + }
> > > + s32g_xpcs_vco_cfg(xpcs, XPCS_PLLA);
> > > +
> > > + /* Step 18 */
> > > + if (!xpcs->ext_clk)
> > > + s32g_xpcs_write_bits(xpcs, VR_MII_DIG_CTRL1, BYP_PWRUP, 0);
> > > +
> > > + /* Will be cleared by Step 19 Vreset */
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, 0);
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, DUPLEX_MODE, DUPLEX_MODE);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +void s32g_xpcs_disable_an(struct s32g_xpcs *xpcs)
> > > +{
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, DUPLEX_MODE, DUPLEX_MODE);
> > > + s32g_xpcs_write_bits(xpcs, SR_MII_CTRL, AN_ENABLE, 0);
> > > +}
> >
> > Sorry, but why? You should never override phylink's requests.
>
> Serdes sometimes needs to be reset and the AN is enabled by default
> after the reset and even before pcs_config has been called. This is
> called during the inti of serdes after an ip reset.
> That being said this might not be needed anymore after the change in
> pcs_config that disables AN
>
>
> >
> > > +
> > > +int s32g_xpcs_init(struct s32g_xpcs *xpcs, struct device *dev,
> > > + unsigned char id, void __iomem *base, bool ext_clk,
> > > + unsigned long rate, enum s32g_xpcs_shared pcie_shared)
> > > +{
> > > + struct regmap_config conf;
> > > +
> > > + if (rate != (125 * HZ_PER_MHZ) && rate != (100 * HZ_PER_MHZ)) {
> > > + dev_err(dev, "XPCS cannot operate @%lu HZ\n", rate);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + xpcs->base = base;
> > > + xpcs->ext_clk = ext_clk;
> > > + xpcs->id = id;
> > > + xpcs->dev = dev;
> > > + xpcs->pcie_shared = pcie_shared;
> > > +
> > > + if (rate == (125 * HZ_PER_MHZ))
> > > + xpcs->mhz125 = true;
> > > + else
> > > + xpcs->mhz125 = false;
> > > +
> > > + conf = s32g_xpcs_regmap_config;
> > > +
> > > + if (!id)
> > > + conf.name = "xpcs0";
> > > + else
> > > + conf.name = "xpcs1";
> > > +
> > > + xpcs->regmap = devm_regmap_init(dev, NULL, xpcs, &conf);
> > > + if (IS_ERR(xpcs->regmap))
> > > + return dev_err_probe(dev, PTR_ERR(xpcs->regmap),
> > > + "Failed to init register amp\n");
> > > +
> > > + /* Phylink PCS */
> > > + xpcs->pcs.ops = &s32cc_phylink_pcs_ops;
> > > + xpcs->pcs.poll = true;
> > > + __set_bit(PHY_INTERFACE_MODE_SGMII, xpcs->pcs.supported_interfaces);
> > > +
> > > + return 0;
> > > +}
> > > diff --git a/drivers/phy/freescale/Kconfig b/drivers/phy/freescale/Kconfig
> > > index 45184a3cdd69..bb7f59897faf 100644
> > > --- a/drivers/phy/freescale/Kconfig
> > > +++ b/drivers/phy/freescale/Kconfig
> > > @@ -66,6 +66,7 @@ config PHY_S32G_SERDES
> > > tristate "NXP S32G SERDES support"
> > > depends on ARCH_S32 || COMPILE_TEST
> > > select GENERIC_PHY
> > > + select REGMAP
> > > help
> > > This option enables support for S23G SerDes PHY used for
> > > PCIe & Ethernet
> > > diff --git a/drivers/phy/freescale/phy-nxp-s32g-serdes.c b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> > > index 321a80c02be5..f2f7eb5aa327 100644
> > > --- a/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> > > +++ b/drivers/phy/freescale/phy-nxp-s32g-serdes.c
> > > @@ -12,12 +12,14 @@
> > > #include <linux/module.h>
> > > #include <linux/of_platform.h>
> > > #include <linux/of_address.h>
> > > +#include <linux/pcs/pcs-nxp-s32g-xpcs.h>
> > > #include <linux/phy/phy.h>
> > > #include <linux/platform_device.h>
> > > #include <linux/processor.h>
> > > #include <linux/reset.h>
> > > #include <linux/units.h>
> > >
> > > +#define S32G_SERDES_XPCS_MAX 2
> > > #define S32G_SERDES_MODE_MAX 5
> > >
> > > #define EXTERNAL_CLK_NAME "ext"
> > > @@ -32,6 +34,52 @@
> > > #define S32G_PCIE_PHY_MPLLA_CTRL 0x10
> > > #define MPLL_STATE BIT(30)
> > >
> > > +#define S32G_PCIE_PHY_MPLLB_CTRL 0x14
> > > +#define MPLLB_SSC_EN BIT(1)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_CTRL_SEL 0x18
> > > +#define EXT_PHY_CTRL_SEL BIT(0)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_BS_CTRL 0x1C
> > > +#define EXT_BS_TX_LOWSWING BIT(6)
> > > +#define EXT_BS_RX_BIGSWING BIT(5)
> > > +#define EXT_BS_RX_LEVEL_MASK GENMASK(4, 0)
> > > +
> > > +#define S32G_PCIE_PHY_REF_CLK_CTRL 0x20
> > > +#define EXT_REF_RANGE_MASK GENMASK(5, 3)
> > > +#define REF_CLK_DIV2_EN BIT(2)
> > > +#define REF_CLK_MPLLB_DIV2_EN BIT(1)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MPLLA_CTRL_1 0x30
> > > +#define EXT_MPLLA_BANDWIDTH_MASK GENMASK(15, 0)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MPLLB_CTRL_1 0x40
> > > +#define EXT_MPLLB_DIV_MULTIPLIER_MASK GENMASK(31, 24)
> > > +#define EXT_MPLLB_DIV_CLK_EN BIT(19)
> > > +#define EXT_MPLLB_DIV8_CLK_EN BIT(18)
> > > +#define EXT_MPLLB_DIV10_CLK_EN BIT(16)
> > > +#define EXT_MPLLB_BANDWIDTH_MASK GENMASK(15, 0)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MPLLB_CTRL_2 0x44
> > > +#define EXT_MPLLB_FRACN_CTRL_MASK GENMASK(22, 12)
> > > +#define MPLLB_MULTIPLIER_MASK GENMASK(8, 0)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MPLLB_CTRL_3 0x48
> > > +#define EXT_MPLLB_WORD_DIV2_EN BIT(31)
> > > +#define EXT_MPLLB_TX_CLK_DIV_MASK GENMASK(30, 28)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MISC_CTRL_1 0xA0
> > > +#define EXT_RX_LOS_THRESHOLD_MASK GENMASK(7, 1)
> > > +#define EXT_RX_VREF_CTRL_MASK GENMASK(28, 24)
> > > +
> > > +#define S32G_PCIE_PHY_EXT_MISC_CTRL_2 0xA4
> > > +#define EXT_TX_VBOOST_LVL_MASK GENMASK(18, 16)
> > > +#define EXT_TX_TERM_CTRL_MASK GENMASK(26, 24)
> > > +
> > > +#define S32G_PCIE_PHY_XPCS1_RX_OVRD_CTRL 0xD0
> > > +#define XPCS1_RX_VCO_LD_VAL_MASK GENMASK(28, 16)
> > > +#define XPCS1_RX_REF_LD_VAL_MASK GENMASK(14, 8)
> > > +
> > > #define S32G_SS_RW_REG_0 0xF0
> > > #define SUBMODE_MASK GENMASK(3, 0)
> > > #define CLKEN_MASK BIT(23)
> > > @@ -44,6 +92,9 @@
> > >
> > > #define S32G_PHY_REG_DATA 0x4
> > >
> > > +#define S32G_PHY_RST_CTRL 0x8
> > > +#define WARM_RST BIT(1)
> > > +
> > > #define RAWLANE0_DIG_PCS_XF_RX_EQ_DELTA_IQ_OVRD_IN 0x3019
> > > #define RAWLANE1_DIG_PCS_XF_RX_EQ_DELTA_IQ_OVRD_IN 0x3119
> > >
> > > @@ -76,16 +127,33 @@ struct s32g_pcie_ctrl {
> > > bool powered_on;
> > > };
> > >
> > > +struct s32g_xpcs_ctrl {
> > > + struct s32g_xpcs *phys[2];
> > > + void __iomem *base0, *base1;
> > > +};
> > > +
> > > struct s32g_serdes {
> > > struct s32g_serdes_ctrl ctrl;
> > > struct s32g_pcie_ctrl pcie;
> > > + struct s32g_xpcs_ctrl xpcs;
> > > struct device *dev;
> > > + u8 lanes_status;
> > > };
> > >
> > > /* PCIe phy subsystem */
> > >
> > > #define S32G_SERDES_PCIE_FREQ (100 * HZ_PER_MHZ)
> > >
> > > +static void s32g_pcie_phy_reset(struct s32g_serdes *serdes)
> > > +{
> > > + u32 val;
> > > +
> > > + val = readl(serdes->pcie.phy_base + S32G_PHY_RST_CTRL);
> > > + writel(val | WARM_RST, serdes->pcie.phy_base + S32G_PHY_RST_CTRL);
> > > + usleep_range(1000, 1100);
> > > + writel(val, serdes->pcie.phy_base + S32G_PHY_RST_CTRL);
> > > +}
> > > +
> > > static int s32g_pcie_check_clk(struct s32g_serdes *serdes)
> > > {
> > > struct s32g_serdes_ctrl *sctrl = &serdes->ctrl;
> > > @@ -277,6 +345,192 @@ static struct phy *s32g_serdes_phy_xlate(struct device *dev,
> > > return phy;
> > > }
> > >
> > > +/* XPCS subsystem */
> > > +
> > > +static int s32g_serdes_xpcs_init(struct s32g_serdes *serdes, int id)
> > > +{
> > > + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> > > + struct s32g_xpcs_ctrl *xpcs = &serdes->xpcs;
> > > + enum s32g_xpcs_shared shared = NOT_SHARED;
> > > + unsigned long rate = ctrl->ref_clk_rate;
> > > + struct device *dev = serdes->dev;
> > > + void __iomem *base;
> > > +
> > > + if (!id)
> > > + base = xpcs->base0;
> > > + else
> > > + base = xpcs->base1;
> > > +
> > > + if (ctrl->ss_mode == 1 || ctrl->ss_mode == 2)
> > > + shared = PCIE_XPCS_1G;
> > > + else if (ctrl->ss_mode == 5)
> > > + shared = PCIE_XPCS_2G5;
> > > +
> > > + return s32g_xpcs_init(xpcs->phys[id], dev, id, base,
> > > + ctrl->ext_clk, rate, shared);
> > > +}
> > > +
> > > +static void s32g_serdes_prepare_pma_mode5(struct s32g_serdes *serdes)
> > > +{
> > > + u32 val;
> > > + /* Configure TX_VBOOST_LVL and TX_TERM_CTRL */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
> > > + val &= ~(EXT_TX_VBOOST_LVL_MASK | EXT_TX_TERM_CTRL_MASK);
> > > + val |= FIELD_PREP(EXT_TX_VBOOST_LVL_MASK, 0x3) |
> > > + FIELD_PREP(EXT_TX_TERM_CTRL_MASK, 0x4);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_2);
> > > +
> > > + /* Enable phy external control */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_CTRL_SEL);
> > > + val |= EXT_PHY_CTRL_SEL;
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_CTRL_SEL);
> > > +
> > > + /* Configure ref range, disable PLLB/ref div2 */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_REF_CLK_CTRL);
> > > + val &= ~(REF_CLK_DIV2_EN | REF_CLK_MPLLB_DIV2_EN | EXT_REF_RANGE_MASK);
> > > + val |= FIELD_PREP(EXT_REF_RANGE_MASK, 0x3);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_REF_CLK_CTRL);
> > > +
> > > + /* Configure multiplier */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_2);
> > > + val &= ~(MPLLB_MULTIPLIER_MASK | EXT_MPLLB_FRACN_CTRL_MASK | BIT(24) | BIT(28));
> > > + val |= FIELD_PREP(MPLLB_MULTIPLIER_MASK, 0x27U) |
> > > + FIELD_PREP(EXT_MPLLB_FRACN_CTRL_MASK, 0x414);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_2);
> > > +
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_MPLLB_CTRL);
> > > + val &= ~MPLLB_SSC_EN;
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_MPLLB_CTRL);
> > > +
> > > + /* Configure tx lane division, disable word clock div2*/
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_3);
> > > + val &= ~(EXT_MPLLB_WORD_DIV2_EN | EXT_MPLLB_TX_CLK_DIV_MASK);
> > > + val |= FIELD_PREP(EXT_MPLLB_TX_CLK_DIV_MASK, 0x5);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_3);
> > > +
> > > + /* Configure bandwidth for filtering and div10*/
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_1);
> > > + val &= ~(EXT_MPLLB_BANDWIDTH_MASK | EXT_MPLLB_DIV_CLK_EN |
> > > + EXT_MPLLB_DIV8_CLK_EN | EXT_MPLLB_DIV_MULTIPLIER_MASK);
> > > + val |= FIELD_PREP(EXT_MPLLB_BANDWIDTH_MASK, 0x5f) | EXT_MPLLB_DIV10_CLK_EN;
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLB_CTRL_1);
> > > +
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLA_CTRL_1);
> > > + val &= ~(EXT_MPLLA_BANDWIDTH_MASK);
> > > + val |= FIELD_PREP(EXT_MPLLA_BANDWIDTH_MASK, 0xc5);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MPLLA_CTRL_1);
> > > +
> > > + /* Configure VCO */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_XPCS1_RX_OVRD_CTRL);
> > > + val &= ~(XPCS1_RX_VCO_LD_VAL_MASK | XPCS1_RX_REF_LD_VAL_MASK);
> > > + val |= FIELD_PREP(XPCS1_RX_VCO_LD_VAL_MASK, 0x540) |
> > > + FIELD_PREP(XPCS1_RX_REF_LD_VAL_MASK, 0x2b);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_XPCS1_RX_OVRD_CTRL);
> > > +
> > > + /* Boundary scan control */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_BS_CTRL);
> > > + val &= ~(EXT_BS_RX_LEVEL_MASK | EXT_BS_TX_LOWSWING);
> > > + val |= FIELD_PREP(EXT_BS_RX_LEVEL_MASK, 0xB) | EXT_BS_RX_BIGSWING;
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_BS_CTRL);
> > > +
> > > + /* Rx loss threshold */
> > > + val = readl(serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_1);
> > > + val &= ~(EXT_RX_LOS_THRESHOLD_MASK | EXT_RX_VREF_CTRL_MASK);
> > > + val |= FIELD_PREP(EXT_RX_LOS_THRESHOLD_MASK, 0x3U) |
> > > + FIELD_PREP(EXT_RX_VREF_CTRL_MASK, 0x11U);
> > > + writel(val, serdes->ctrl.ss_base + S32G_PCIE_PHY_EXT_MISC_CTRL_1);
> > > +}
> > > +
> > > +static int s32g_serdes_enable_mode5(struct s32g_serdes *serdes, struct s32g_xpcs *xpcs)
> > > +{
> > > + int ret;
> > > +
> > > + s32g_serdes_prepare_pma_mode5(serdes);
> > > +
> > > + ret = s32g_xpcs_pre_pcie_2g5(xpcs);
> > > + if (ret) {
> > > + dev_err(serdes->dev,
> > > + "Failed to prepare SerDes for PCIE & XPCS @ 2G5 mode\n");
> > > + return ret;
> > > + }
> > > +
> > > + s32g_pcie_phy_reset(serdes);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int s32g_serdes_init_clks(struct s32g_serdes *serdes)
> > > +{
> > > + struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> > > + struct s32g_xpcs_ctrl *xpcs = &serdes->xpcs;
> > > + struct s32g_xpcs *order[2];
> > > + size_t i;
> > > + int ret;
> > > +
> > > + switch (ctrl->ss_mode) {
> > > + case 0:
> > > + return 0;
> > > + case 1:
> > > + order[0] = xpcs->phys[0];
> > > + order[1] = NULL;
> > > + break;
> > > + case 2:
> > > + case 5:
> > > + order[0] = xpcs->phys[1];
> > > + order[1] = NULL;
> > > + break;
> > > + case 3:
> > > + order[0] = xpcs->phys[1];
> > > + order[1] = xpcs->phys[0];
> > > + break;
> > > + case 4:
> > > + order[0] = xpcs->phys[0];
> > > + order[1] = xpcs->phys[1];
> > > + break;
> > > + default:
> > > + return -EINVAL;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(order); i++) {
> > > + if (!order[i])
> > > + continue;
> > > +
> > > + ret = s32g_xpcs_init_plls(order[i]);
> > > + if (ret)
> > > + return ret;
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(order); i++) {
> > > + if (!order[i])
> > > + continue;
> > > +
> > > + if (ctrl->ss_mode == 5) {
> > > + ret = s32g_serdes_enable_mode5(serdes, order[i]);
> > > + if (ret)
> > > + return ret;
> > > + } else {
> > > + s32g_xpcs_vreset(order[i]);
> > > + }
> > > + }
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(order); i++) {
> > > + if (!order[i])
> > > + continue;
> > > +
> > > + ret = s32g_xpcs_wait_vreset(order[i]);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = s32g_xpcs_reset_rx(order[i]);
> > > + if (ret)
> > > + return ret;
> > > +
> > > + s32g_xpcs_disable_an(order[i]);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > /* Serdes subsystem */
> > >
> > > static int s32g_serdes_assert_reset(struct s32g_serdes *serdes)
> > > @@ -331,6 +585,10 @@ static int s32g_serdes_init(struct s32g_serdes *serdes)
> > > return ret;
> > > }
> > >
> > > + /*
> > > + * We have a tight timing for the init sequence and any delay linked to
> > > + * printk as an example can fail the init after reset
> > > + */
> > > ret = s32g_serdes_assert_reset(serdes);
> > > if (ret)
> > > goto disable_clks;
> > > @@ -363,7 +621,13 @@ static int s32g_serdes_init(struct s32g_serdes *serdes)
> > > dev_info(serdes->dev, "Using mode %d for SerDes subsystem\n",
> > > ctrl->ss_mode);
> > >
> > > - return 0;
> > > + ret = s32g_serdes_init_clks(serdes);
> > > + if (ret) {
> > > + dev_err(serdes->dev, "XPCS init failed\n");
> > > + goto disable_clks;
> > > + }
> > > +
> > > + return ret;
> > >
> > > disable_clks:
> > > clk_bulk_disable_unprepare(serdes->ctrl.nclks,
> > > @@ -449,12 +713,32 @@ static int s32g_serdes_get_pcie_resources(struct platform_device *pdev, struct s
> > > return 0;
> > > }
> > >
> > > +static int s32g_serdes_get_xpcs_resources(struct platform_device *pdev, struct s32g_serdes *serdes)
> > > +{
> > > + struct s32g_xpcs_ctrl *xpcs = &serdes->xpcs;
> > > + struct device *dev = &pdev->dev;
> > > +
> > > + xpcs->base0 = devm_platform_ioremap_resource_byname(pdev, "xpcs0");
> > > + if (IS_ERR(xpcs->base0)) {
> > > + dev_err(dev, "Failed to map 'xpcs0'\n");
> > > + return PTR_ERR(xpcs->base0);
> > > + }
> > > +
> > > + xpcs->base1 = devm_platform_ioremap_resource_byname(pdev, "xpcs1");
> > > + if (IS_ERR(xpcs->base1)) {
> > > + dev_err(dev, "Failed to map 'xpcs1'\n");
> > > + return PTR_ERR(xpcs->base1);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static int s32g2_serdes_create_phy(struct s32g_serdes *serdes, struct device_node *child_node)
> > > {
> > > struct s32g_serdes_ctrl *ctrl = &serdes->ctrl;
> > > struct phy_provider *phy_provider;
> > > struct device *dev = serdes->dev;
> > > - int ss_mode = ctrl->ss_mode;
> > > + int ret, ss_mode = ctrl->ss_mode;
> > > struct phy *phy;
> > >
> > > if (of_device_is_compatible(child_node, "nxp,s32g2-serdes-pcie-phy")) {
> > > @@ -476,6 +760,37 @@ static int s32g2_serdes_create_phy(struct s32g_serdes *serdes, struct device_nod
> > > if (IS_ERR(phy_provider))
> > > return PTR_ERR(phy_provider);
> > >
> > > + } else if (of_device_is_compatible(child_node, "nxp,s32g2-serdes-xpcs")) {
> > > + struct s32g_xpcs_ctrl *xpcs_ctrl = &serdes->xpcs;
> > > + struct s32g_xpcs *xpcs;
> > > + int port;
> > > +
> > > + /* no Ethernet phy lane */
> > > + if (ss_mode == 0)
> > > + return 0;
> > > +
> > > + /* Get XPCS port number connected to the lane */
> > > + if (of_property_read_u32(child_node, "reg", &port))
> > > + return -EINVAL;
> > > +
> > > + /* XPCS1 is not used */
> > > + if (ss_mode == 1 && port == 1)
> > > + return -EINVAL;
> > > +
> > > + /* XPCS0 is not used */
> > > + if (ss_mode == 2 && port == 0)
> > > + return -EINVAL;
> > > +
> > > + xpcs = devm_kmalloc(dev, sizeof(*xpcs), GFP_KERNEL);
> > > + if (!xpcs)
> > > + return -ENOMEM;
> > > +
> > > + xpcs_ctrl->phys[port] = xpcs;
> > > +
> > > + ret = s32g_serdes_xpcs_init(serdes, port);
> > > + if (ret)
> > > + return ret;
> > > +
> > > } else {
> > > dev_warn(dev, "Skipping unknown child node %pOFn\n", child_node);
> > > }
> > > @@ -517,6 +832,10 @@ static int s32g_serdes_probe(struct platform_device *pdev)
> > > if (ret)
> > > return ret;
> > >
> > > + ret = s32g_serdes_get_xpcs_resources(pdev, serdes);
> > > + if (ret)
> > > + return ret;
> > > +
> > > ret = s32g_serdes_parse_lanes(dev, serdes);
> > > if (ret)
> > > return ret;
> > > @@ -555,6 +874,57 @@ static int __maybe_unused s32g_serdes_resume(struct device *device)
> > > return ret;
> > > }
> > >
> > > +struct phylink_pcs *s32g_serdes_pcs_create(struct device *dev, struct device_node *np)
> > > +{
> > > + struct platform_device *pdev;
> > > + struct device_node *pcs_np;
> > > + struct s32g_serdes *serdes;
> > > + u32 port;
> > > +
> > > + if (of_property_read_u32(np, "reg", &port))
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + if (port >= S32G_SERDES_XPCS_MAX)
> > > + return ERR_PTR(-EINVAL);
> > > +
> > > + /* The PCS pdev is attached to the parent node */
> > > + pcs_np = of_get_parent(np);
> > > + if (!pcs_np)
> > > + return ERR_PTR(-ENODEV);
> > > +
> > > + if (!of_device_is_available(pcs_np)) {
> > > + of_node_put(pcs_np);
> > > + return ERR_PTR(-ENODEV);
> > > + }
> > > +
> > > + pdev = of_find_device_by_node(pcs_np);
> > > + of_node_put(pcs_np);
> > > + if (!pdev)
> > > + return ERR_PTR(-EPROBE_DEFER);
> > > +
> > > + serdes = platform_get_drvdata(pdev);
> > > + if (!serdes) {
> > > + put_device(&pdev->dev);
> > > + return ERR_PTR(-EPROBE_DEFER);
> > > + }
> > > +
> > > + if (!serdes->xpcs.phys[port]) {
> > > + put_device(&pdev->dev);
> > > + return ERR_PTR(-EPROBE_DEFER);
> > > + }
> > > +
> > > + return &serdes->xpcs.phys[port]->pcs;
> > > +}
> > > +EXPORT_SYMBOL(s32g_serdes_pcs_create);
> > > +
> > > +void s32g_serdes_pcs_destroy(struct phylink_pcs *pcs)
> > > +{
> > > + struct s32g_xpcs *xpcs = phylink_pcs_to_s32g_xpcs(pcs);
> > > +
> > > + put_device(xpcs->dev);
> > > +}
> > > +EXPORT_SYMBOL(s32g_serdes_pcs_destroy);
> > > +
> > > static const struct of_device_id s32g_serdes_match[] = {
> > > {
> > > .compatible = "nxp,s32g2-serdes",
> > > diff --git a/include/linux/pcs/pcs-nxp-s32g-xpcs.h b/include/linux/pcs/pcs-nxp-s32g-xpcs.h
> > > new file mode 100644
> > > index 000000000000..96a0049b93a6
> > > --- /dev/null
> > > +++ b/include/linux/pcs/pcs-nxp-s32g-xpcs.h
> > > @@ -0,0 +1,50 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 */
> > > +/**
> > > + * Copyright 2021-2026 NXP
> > > + */
> > > +#ifndef PCS_NXP_S32G_XPCS_H
> > > +#define PCS_NXP_S32G_XPCS_H
> > > +
> > > +#include <linux/phylink.h>
> > > +
> > > +enum s32g_xpcs_shared {
> > > + NOT_SHARED,
> > > + PCIE_XPCS_1G,
> > > + PCIE_XPCS_2G5,
> > > +};
> > > +
> > > +enum s32g_xpcs_pll {
> > > + XPCS_PLLA, /* Slow PLL */
> > > + XPCS_PLLB, /* Fast PLL */
> > > +};
> > > +
> > > +struct s32g_xpcs {
> > > + void __iomem *base;
> > > + struct device *dev;
> > > + unsigned char id;
> > > + struct regmap *regmap;
> > > + enum s32g_xpcs_pll ref;
> > > + bool ext_clk;
> > > + bool mhz125;
> > > + enum s32g_xpcs_shared pcie_shared;
> > > + struct phylink_pcs pcs;
> > > +};
> > > +
> > > +#define phylink_pcs_to_s32g_xpcs(pl_pcs) \
> > > + container_of((pl_pcs), struct s32g_xpcs, pcs)
> > > +
> > > +int s32g_xpcs_init(struct s32g_xpcs *xpcs, struct device *dev,
> > > + unsigned char id, void __iomem *base, bool ext_clk,
> > > + unsigned long rate, enum s32g_xpcs_shared pcie_shared);
> > > +int s32g_xpcs_init_plls(struct s32g_xpcs *xpcs);
> > > +int s32g_xpcs_pre_pcie_2g5(struct s32g_xpcs *xpcs);
> > > +void s32g_xpcs_vreset(struct s32g_xpcs *xpcs);
> > > +int s32g_xpcs_wait_vreset(struct s32g_xpcs *xpcs);
> > > +int s32g_xpcs_reset_rx(struct s32g_xpcs *xpcs);
> > > +void s32g_xpcs_disable_an(struct s32g_xpcs *xpcs);
> > > +
> > > +struct phylink_pcs *s32g_serdes_pcs_create(struct device *dev, struct device_node *np);
> > > +void s32g_serdes_pcs_destroy(struct phylink_pcs *pcs);
> > > +
> > > +#endif /* PCS_NXP_S32G_XPCS_H */
> > > +
> > > --
> > > 2.43.0
> > >
> > >
> >
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 0/5] USB4 mode programming for QMMPHY on X1E
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
The USB43DP QMP PHY, as the name suggests, supports USB4 in addition to
DP/USB. This series provides the necessary plumbing to switch to that
mode of operation on hamoa/purwa.
This is achieved by exposing a third `struct phy *` for the USB4 sub-
PHY, which will be toggled by the USB4 router driver, living within the
Thunderbolt framework, which will be posted separately.
Find more details in the individual commit messages. Tested on a
X1E80100 CRD.
No external dependencies.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
Konrad Dybcio (5):
dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
phy: core: Define TBT phy_mode
phy: qualcomm: qmp-combo: Add preliminary USB4 support
phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
.../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 3 +-
arch/arm64/boot/dts/qcom/hamoa.dtsi | 18 +-
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 592 ++++++++++++++++++---
drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h | 2 +
drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h | 15 +
.../phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h | 45 ++
include/dt-bindings/phy/phy-qcom-qmp.h | 1 +
include/linux/phy/phy-tbt.h | 14 +
include/linux/phy/phy.h | 2 +
9 files changed, 609 insertions(+), 83 deletions(-)
---
base-commit: e98d21c170b01ddef366f023bbfcf6b31509fa83
change-id: 20260518-topic-usb4phy-e9612f5ba008
Best regards,
--
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Some instances of the QMP combo PHY (called USB43DP) feature a third
functional sub-block, responsible for USB4/Thunderbolt 3 communication.
Compared to the today's state of the binding, one more clock (P2RR2P -
PHY-to-Router, Router-to-PHY) needs to be enabled for the PHY to be
able to switch to USB4 mode. Allow that for X1E.
Also, add a bindings define to let consumers access it.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
.../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 3 ++-
include/dt-bindings/phy/phy-qcom-qmp.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index 3d537b7f9985..eba4bee474fb 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -52,7 +52,7 @@ properties:
- const: ref
- const: com_aux
- const: usb3_pipe
- - const: cfg_ahb
+ - enum: [ p2rr2p_pipe, cfg_ahb ]
power-domains:
maxItems: 1
@@ -186,6 +186,7 @@ allOf:
enum:
- qcom,sc7180-qmp-usb3-dp-phy
- qcom,sdm845-qmp-usb3-dp-phy
+ - qcom,x1e80100-qmp-usb3-dp-phy
then:
properties:
clocks:
diff --git a/include/dt-bindings/phy/phy-qcom-qmp.h b/include/dt-bindings/phy/phy-qcom-qmp.h
index 6b43ea9e0051..1c3ce0c02b0c 100644
--- a/include/dt-bindings/phy/phy-qcom-qmp.h
+++ b/include/dt-bindings/phy/phy-qcom-qmp.h
@@ -16,6 +16,7 @@
/* QMP USB4-USB3-DP PHYs */
#define QMP_USB43DP_USB3_PHY 0
#define QMP_USB43DP_DP_PHY 1
+#define QMP_USB43DP_USB4_PHY 2
/* QMP PCIE PHYs */
#define QMP_PCIE_PIPE_CLK 0
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 2/5] phy: core: Define TBT phy_mode
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
There exist OS-accessible USB4 and Thunderbolt PHYs that need specific
configuration. Define a new phy_mode for them.
Currently, USB4 and TBT3 are defined as submodes, because they're quite
distinct, most visibly in the electrical/analog aspects (slightly
different frequencies, timings, etc.). This results in a need to make
the PHY aware of the actual mode needed (at least in the Qualcomm
implementation, but I believe that'll be a general need).
Newer versions of TBT are basically supersets of USB4 with higher
host-side requirements, so these are not defined. This can always be
changed as necessary.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
include/linux/phy/phy-tbt.h | 14 ++++++++++++++
include/linux/phy/phy.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/include/linux/phy/phy-tbt.h b/include/linux/phy/phy-tbt.h
new file mode 100644
index 000000000000..5f48059814e1
--- /dev/null
+++ b/include/linux/phy/phy-tbt.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef __PHY_TBT_H_
+#define __PHY_TBT_H_
+
+enum tbt_phy_submode {
+ PHY_SUBMODE_TBT3,
+ PHY_SUBMODE_USB4,
+};
+
+#endif
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288a..26a91c070f8c 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -20,6 +20,7 @@
#include <linux/phy/phy-hdmi.h>
#include <linux/phy/phy-lvds.h>
#include <linux/phy/phy-mipi-dphy.h>
+#include <linux/phy/phy-tbt.h>
struct phy;
@@ -45,6 +46,7 @@ enum phy_mode {
PHY_MODE_LVDS,
PHY_MODE_DP,
PHY_MODE_HDMI,
+ PHY_MODE_TBT,
};
enum phy_media {
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Some Combo PHYs (so far only on SC8280XP, X1E80100 and Glymur), come in
a flavor called USB43DP, which as the name implies, features USB4, USB3
and DP signal processing capabilities. In that architecture, USB3 and
USB4 PHYs share the same USB_PLL while featuring separate logic spaces.
The DP part is roughly the same as on the instances without USB4.
The USB4 and USB3/DP operation modes of the PHY are mutually exclusive.
Only one USB protocol (and flavor of pipe clock) can be active at a
given moment (not to be confused with USB3 not being able to be
tunneled as USB4 packets - that of course remains possible).
The DP PLL is still used for clocking tunneled DP links. It may be
turned off to save power when no tunnels are active, but that's left as
a TODO item for now.
Due to the nature of USB4, the Type-C handling happens entirely inside
the Host Router, and as such the QMPPHY's mux_set() function is
nullified for the period when USB4 PHY remains active. This is strictly
necessary, as the Host Router driver is going to excercise manual
control over the USB4 PHY's power state, which is needed by the suspend
and resume flows. Failure to control that synchronously with other
parts of the code results in a SoC crash by unlocked access.
Because of that, a new struct phy is spawned to expose the USB4 mode,
along with a .set_mode callback to allow toggling between USB4 and TBT3
submodes.
Thunderbolt 3, having a number of differences vs USB4, requires a
couple specific overrides, pertaining to electrical characteristics,
which are easily accommodated for.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 392 ++++++++++++++++++++++++------
1 file changed, 322 insertions(+), 70 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 93f1aa10d400..898f42de4a08 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -22,6 +22,7 @@
#include <linux/usb/typec.h>
#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_tbt.h>
#include <drm/bridge/aux-bridge.h>
@@ -61,10 +62,14 @@
#define SW_USB3PHY_RESET BIT(2)
/* mux to select USB3 PHY reset control, 0:HW control, 1: software reset */
#define SW_USB3PHY_RESET_MUX BIT(3)
+#define SW_USB4PHY_RESET BIT(4)
+#define SW_USB4PHY_RESET_MUX BIT(5)
/* QPHY_V3_DP_COM_PHY_MODE_CTRL register bits */
#define USB3_MODE BIT(0) /* enables USB3 mode */
#define DP_MODE BIT(1) /* enables DP mode */
+#define USB4_MODE BIT(2) /* mutually exclusive with the above */
+#define DP_TUNNELING_CLOCK_GEN_EN BIT(3)
/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
#define SW_PORTSELECT_VAL BIT(0)
@@ -77,6 +82,8 @@ enum qmpphy_mode {
QMPPHY_MODE_USB3DP = 0,
QMPPHY_MODE_DP_ONLY,
QMPPHY_MODE_USB3_ONLY,
+ /* USB4 QMPPHY mode refers to both USB4 and TBT3 */
+ QMPPHY_MODE_USB4,
};
/* set of registers with offsets different per-PHY */
@@ -89,6 +96,7 @@ enum qphy_reg_layout {
QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR,
QPHY_PCS_POWER_DOWN_CONTROL,
QPHY_PCS_CLAMP_ENABLE,
+ QPHY_PCS_USB4_CLAMP_ENABLE,
QPHY_COM_RESETSM_CNTRL,
QPHY_COM_C_READY_STATUS,
@@ -2198,6 +2206,8 @@ struct qmp_combo_offsets {
u16 dp_txa;
u16 dp_txb;
u16 dp_dp_phy;
+ u16 usb4_serdes;
+ u16 usb4_pcs;
};
struct qmp_phy_cfg {
@@ -2245,6 +2255,18 @@ struct qmp_phy_cfg {
int (*calibrate_dp_phy)(struct qmp_combo *qmp);
void (*dp_aux_init)(struct qmp_combo *qmp);
+ /* USB4 specifics */
+ const struct qmp_phy_init_tbl *usb4_serdes_tbl;
+ int usb4_serdes_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_serdes_tb3_ovrd_tbl;
+ int usb4_serdes_tb3_ovrd_num;
+ const struct qmp_phy_init_tbl *usb4_tx_tbl;
+ int usb4_tx_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_rx_tbl;
+ int usb4_rx_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_pcs_tbl;
+ int usb4_pcs_tbl_num;
+
/* resets to be requested */
const char * const *reset_list;
int num_resets;
@@ -2286,8 +2308,12 @@ struct qmp_combo {
void __iomem *dp_tx2;
void __iomem *dp_dp_phy;
+ void __iomem *usb4_serdes;
+ void __iomem *usb4_pcs;
+
struct clk *pipe_clk;
struct clk_bulk_data *clks;
+ struct clk *p2rr2p_pipe_clk;
int num_clks;
struct reset_control_bulk_data *resets;
struct regulator_bulk_data *vregs;
@@ -2306,6 +2332,10 @@ struct qmp_combo {
unsigned int dp_init_count;
bool dp_powered_on;
+ struct phy *usb4_phy;
+ enum tbt_phy_submode usb4_phy_submode;
+ unsigned int usb4phy_init_count;
+
struct clk_fixed_rate pipe_clk_fixed;
struct clk_hw dp_link_hw;
struct clk_hw dp_pixel_hw;
@@ -3661,14 +3691,26 @@ static int qmp_combo_dp_calibrate(struct phy *phy)
static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
{
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ u32 dp_reset_val = SW_DPPHY_RESET_MUX | SW_DPPHY_RESET;
void __iomem *com = qmp->com;
void __iomem *pcs_aon = qmp->pcs_aon;
+ void __iomem *pcs = qmp->pcs;
int ret;
u32 val;
if (!force && qmp->init_count++)
return 0;
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ pcs = qmp->usb4_pcs;
+
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
+
+ /* Do not disturb the DP PLL in case there's an active DP tunnel */
+ if (readl(com + QPHY_V3_DP_COM_RESET_OVRD_CTRL) & DP_TUNNELING_CLOCK_GEN_EN)
+ dp_reset_val &= ~SW_DPPHY_RESET_MUX;
+ }
+
ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
if (ret) {
dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
@@ -3695,8 +3737,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
/* override hardware control for reset of qmp phy */
qphy_setbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
- SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
- SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+ dp_reset_val |
+ SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET |
+ SW_USB4PHY_RESET_MUX | SW_USB4PHY_RESET);
/* override hardware control for reset of qmp phy */
if (pcs_aon && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
@@ -3712,6 +3755,10 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
if (cfg->invert_cc_polarity)
val |= INVERT_CC_POLARITY;
+ /*
+ * Note that in USB4 mode, the router controls pin assignments instead
+ * and the values of the PORTSELECT registers are ignored.
+ */
writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
switch (qmp->qmpphy_mode) {
@@ -3739,12 +3786,21 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
break;
+ case QMPPHY_MODE_USB4:
+ writel(USB4_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+ /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
+ /* TODO: disable DP PLL if there are no active tunnels after router setup */
+ qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+ SW_USB4PHY_RESET_MUX | SW_USB4PHY_RESET |
+ SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
+ break;
}
qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
- qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+ qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
SW_PWRDN);
return 0;
@@ -3859,6 +3915,10 @@ static int qmp_combo_usb_power_on(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
void __iomem *serdes = qmp->serdes;
+ const struct qmp_phy_init_tbl *pcs_tbl;
+ const struct qmp_phy_init_tbl *rx_tbl;
+ const struct qmp_phy_init_tbl *serdes_tbl;
+ const struct qmp_phy_init_tbl *tx_tbl;
void __iomem *tx = qmp->tx;
void __iomem *rx = qmp->rx;
void __iomem *tx2 = qmp->tx2;
@@ -3866,10 +3926,39 @@ static int qmp_combo_usb_power_on(struct phy *phy)
void __iomem *pcs = qmp->pcs;
void __iomem *pcs_usb = qmp->pcs_usb;
void __iomem *status;
+ int pcs_tbl_num, rx_tbl_num, serdes_tbl_num, tx_tbl_num;
unsigned int val;
int ret;
- qmp_configure(qmp->dev, serdes, cfg->serdes_tbl, cfg->serdes_tbl_num);
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ pcs = qmp->usb4_pcs;
+ serdes = qmp->usb4_serdes;
+ pcs_tbl = cfg->usb4_pcs_tbl;
+ pcs_tbl_num = cfg->usb4_pcs_tbl_num;
+
+ serdes_tbl = cfg->usb4_serdes_tbl;
+ serdes_tbl_num = cfg->usb4_serdes_tbl_num;
+
+ rx_tbl = cfg->usb4_rx_tbl;
+ rx_tbl_num = cfg->usb4_rx_tbl_num;
+
+ tx_tbl = cfg->usb4_tx_tbl;
+ tx_tbl_num = cfg->usb4_tx_tbl_num;
+ } else {
+ pcs_tbl = cfg->pcs_tbl;
+ pcs_tbl_num = cfg->pcs_tbl_num;
+
+ serdes_tbl = cfg->serdes_tbl;
+ serdes_tbl_num = cfg->serdes_tbl_num;
+
+ rx_tbl = cfg->rx_tbl;
+ rx_tbl_num = cfg->rx_tbl_num;
+
+ tx_tbl = cfg->tx_tbl;
+ tx_tbl_num = cfg->tx_tbl_num;
+ }
+
+ qmp_configure(qmp->dev, serdes, serdes_tbl, serdes_tbl_num);
ret = clk_prepare_enable(qmp->pipe_clk);
if (ret) {
@@ -3878,17 +3967,19 @@ static int qmp_combo_usb_power_on(struct phy *phy)
}
/* Tx, Rx, and PCS configurations */
- qmp_configure_lane(qmp->dev, tx, cfg->tx_tbl, cfg->tx_tbl_num, 1);
- qmp_configure_lane(qmp->dev, tx2, cfg->tx_tbl, cfg->tx_tbl_num, 2);
+ qmp_configure_lane(qmp->dev, tx, tx_tbl, tx_tbl_num, 1);
+ qmp_configure_lane(qmp->dev, tx2, tx_tbl, tx_tbl_num, 2);
- qmp_configure_lane(qmp->dev, rx, cfg->rx_tbl, cfg->rx_tbl_num, 1);
- qmp_configure_lane(qmp->dev, rx2, cfg->rx_tbl, cfg->rx_tbl_num, 2);
+ qmp_configure_lane(qmp->dev, rx, rx_tbl, rx_tbl_num, 1);
+ qmp_configure_lane(qmp->dev, rx2, rx_tbl, rx_tbl_num, 2);
- qmp_configure(qmp->dev, pcs, cfg->pcs_tbl, cfg->pcs_tbl_num);
+ qmp_configure(qmp->dev, pcs, pcs_tbl, pcs_tbl_num);
qmp_configure(qmp->dev, qmp->pcs_misc, cfg->pcs_misc_tbl, cfg->pcs_misc_tbl_num);
-
- if (pcs_usb)
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4 && qmp->usb4_phy_submode == PHY_SUBMODE_TBT3)
+ qmp_configure(qmp->dev, serdes, cfg->usb4_serdes_tb3_ovrd_tbl,
+ cfg->usb4_serdes_tb3_ovrd_num);
+ else if (qmp->qmpphy_mode != QMPPHY_MODE_USB4 && pcs_usb)
qmp_configure(qmp->dev, pcs_usb, cfg->pcs_usb_tbl,
cfg->pcs_usb_tbl_num);
@@ -3921,18 +4012,22 @@ static int qmp_combo_usb_power_off(struct phy *phy)
{
struct qmp_combo *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ void __iomem *pcs = qmp->pcs;
+
+ if (qmp->usb4phy_init_count)
+ pcs = qmp->usb4_pcs;
clk_disable_unprepare(qmp->pipe_clk);
/* PHY reset */
- qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
+ qphy_setbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
/* stop SerDes and Phy-Coding-Sublayer */
- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_START_CTRL],
+ qphy_clrbits(pcs, cfg->regs[QPHY_START_CTRL],
SERDES_START | PCS_START);
/* Put PHY into POWER DOWN state: active low */
- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+ qphy_clrbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
SW_PWRDN);
return 0;
@@ -3943,21 +4038,27 @@ static int qmp_combo_usb_init(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
int ret;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
+
+ /* USB4 mode takes precedence, do not reprogram the PHY in that case */
+ if (qmp->usb4phy_init_count) {
+ qmp->init_count++;
+ qmp->usb_init_count++;
+ return 0;
+ }
+
ret = qmp_combo_com_init(qmp, false);
if (ret)
- goto out_unlock;
+ return ret;
ret = qmp_combo_usb_power_on(phy);
if (ret) {
qmp_combo_com_exit(qmp, false);
- goto out_unlock;
+ return ret;
}
qmp->usb_init_count++;
-out_unlock:
- mutex_unlock(&qmp->phy_mutex);
return ret;
}
@@ -3966,20 +4067,26 @@ static int qmp_combo_usb_exit(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
int ret;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
+
+ /* USB4 mode takes precedence, do not reprogram the PHY in that case */
+ if (qmp->usb4phy_init_count) {
+ qmp->init_count--;
+ qmp->usb_init_count--;
+ return 0;
+ }
+
ret = qmp_combo_usb_power_off(phy);
if (ret)
- goto out_unlock;
+ return ret;
ret = qmp_combo_com_exit(qmp, false);
if (ret)
- goto out_unlock;
+ return ret;
qmp->usb_init_count--;
-out_unlock:
- mutex_unlock(&qmp->phy_mutex);
- return ret;
+ return 0;
}
static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submode)
@@ -3991,6 +4098,119 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
return 0;
}
+static int qmp_combo_reconfigure_phy(struct qmp_combo *qmp, enum qmpphy_mode new_mode)
+{
+ dev_dbg(qmp->dev, "qmp_combo_reconfigure_phy: switching from qmpphy mode %d to %d\n",
+ qmp->qmpphy_mode, new_mode);
+
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
+ qmp_combo_usb_power_off(qmp->usb_phy);
+
+ if (qmp->dp_init_count)
+ writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
+
+ qmp_combo_com_exit(qmp, true);
+
+ qmp->qmpphy_mode = new_mode;
+
+ /* Now everything's powered down, power up the right PHYs */
+ qmp_combo_com_init(qmp, true);
+
+ if ((qmp->usb_init_count || qmp->usb4phy_init_count) &&
+ new_mode != QMPPHY_MODE_DP_ONLY)
+ qmp_combo_usb_power_on(qmp->usb_phy);
+
+ if ((new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_DP_ONLY) &&
+ qmp->dp_init_count)
+ qmp->cfg->dp_aux_init(qmp);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_init(struct phy *phy)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+ int ret;
+
+ guard(mutex)(&qmp->phy_mutex);
+
+ if (!qmp->p2rr2p_pipe_clk) {
+ dev_err(qmp->dev, "missing p2rr2p_pipe clock handle. Update your Device Tree.\n");
+ return -EINVAL;
+ }
+
+ ret = clk_prepare_enable(qmp->p2rr2p_pipe_clk);
+ if (ret) {
+ dev_err(qmp->dev, "p2rr2p_pipe enable failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = qmp_combo_com_init(qmp, false);
+ if (ret)
+ return ret;
+
+ /* USB4 mode takes precedence to USB3(+DP), force reconfigure the PHY */
+ ret = qmp_combo_reconfigure_phy(qmp, QMPPHY_MODE_USB4);
+ if (ret)
+ return ret;
+
+ ret = qmp_combo_usb_power_on(phy);
+ if (ret) {
+ qmp_combo_com_exit(qmp, false);
+ return ret;
+ }
+
+ /*
+ * Due to the SoC design, the PHY only has a single valid consumer and
+ * preventing it from having sole ownership of the PHY's power state
+ * makes suspending/resuming the router impossible.
+ */
+ WARN_ON(qmp->usb4phy_init_count++);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_exit(struct phy *phy)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+ int ret;
+
+ guard(mutex)(&qmp->phy_mutex);
+
+ ret = qmp_combo_usb_power_off(qmp->usb_phy);
+ if (ret)
+ return ret;
+
+ ret = qmp_combo_com_exit(qmp, false);
+ if (ret)
+ return ret;
+
+ /*
+ * Mark the USB4 PHY uninitialized and wait for a mux_set event to determine the correct
+ * setting. This will always be possible because USB4 requires Type-C.
+ */
+ qmp->usb4phy_init_count--;
+
+ clk_disable_unprepare(qmp->p2rr2p_pipe_clk);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+
+ if (mode != PHY_MODE_TBT)
+ return -EINVAL;
+
+ if (submode == PHY_SUBMODE_USB4 || submode == PHY_SUBMODE_TBT3) {
+ qmp->usb4_phy_submode = submode;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
static const struct phy_ops qmp_combo_usb_phy_ops = {
.init = qmp_combo_usb_init,
.exit = qmp_combo_usb_exit,
@@ -3998,6 +4218,13 @@ static const struct phy_ops qmp_combo_usb_phy_ops = {
.owner = THIS_MODULE,
};
+static const struct phy_ops qmp_combo_usb4_phy_ops = {
+ .init = qmp_combo_usb4_init,
+ .exit = qmp_combo_usb4_exit,
+ .set_mode = qmp_combo_usb4_set_mode,
+ .owner = THIS_MODULE,
+};
+
static const struct phy_ops qmp_combo_dp_phy_ops = {
.init = qmp_combo_dp_init,
.configure = qmp_combo_dp_configure,
@@ -4037,9 +4264,12 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
* Enable i/o clamp_n for autonomous mode
* V6 and later versions use pcs aon clamp register
*/
- if (pcs_aon)
- qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
- else if (pcs_misc)
+ if (pcs_aon) {
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
+ qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
+ else
+ qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
+ } else if (pcs_misc)
qphy_clrbits(pcs_misc, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
}
@@ -4051,9 +4281,12 @@ static void qmp_combo_disable_autonomous_mode(struct qmp_combo *qmp)
void __iomem *pcs_aon = qmp->pcs_aon;
/* Disable i/o clamp_n on resume for normal mode */
- if (pcs_aon)
- qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
- else if (pcs_misc)
+ if (pcs_aon) {
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
+ else
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
+ } else if (pcs_misc)
qphy_setbits(pcs_misc, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
qphy_clrbits(pcs_usb, cfg->regs[QPHY_PCS_AUTONOMOUS_MODE_CTRL],
@@ -4427,24 +4660,29 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ /* QMPPHY has no orientation handling in USB4 mode, don't cache the setting */
+ qmp->orientation = TYPEC_ORIENTATION_NONE;
+ return 0;
+ }
+
if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
return 0;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
qmp->orientation = orientation;
if (qmp->init_count) {
- if (qmp->usb_init_count)
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
qmp_combo_usb_power_off(qmp->usb_phy);
qmp_combo_com_exit(qmp, true);
qmp_combo_com_init(qmp, true);
- if (qmp->usb_init_count)
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
qmp_combo_usb_power_on(qmp->usb_phy);
if (qmp->dp_init_count)
cfg->dp_aux_init(qmp);
}
- mutex_unlock(&qmp->phy_mutex);
return 0;
}
@@ -4452,7 +4690,6 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
{
struct qmp_combo *qmp = typec_mux_get_drvdata(mux);
- const struct qmp_phy_cfg *cfg = qmp->cfg;
enum qmpphy_mode new_mode;
unsigned int svid;
@@ -4463,6 +4700,29 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
else
svid = 0;
+ /*
+ * The USB4 router driver must excercise fine-grained control over the timing of
+ * USB4 QMPPHY mode entry/exit, which is difficult to otherwise ensure within Linux's
+ * Type-C framework if the PHY acts as a self-decisive mode mux in parallel.
+ *
+ * Keep the hardware in QMPPHY_MODE_USB4 at all times between .init and .exit of the
+ * (Linux) USB4 PHY and wait for the router driver to turn it off at its discretion.
+ * Once that happens, fall back to the usual USB3/DP/Combo mode logic.
+ *
+ * After the QMPPHY has been turned off through phy_exit(usb4_phy), the next mux_set
+ * will initialize it in the right mode.
+ */
+ if (qmp->usb4phy_init_count)
+ return 0;
+
+ /*
+ * Explicitly ignore TBT/USB4 mode requests that may come if the USB4 PHY hasn't been
+ * initialized, either due to the USB4 drivers being disabled or due to this PHY instance
+ * lacking USB4 support.
+ */
+ if (svid == USB_TYPEC_TBT_SID || (!state->alt && state->mode == TYPEC_MODE_USB4))
+ return 0;
+
if (svid == USB_TYPEC_DP_SID) {
switch (state->mode) {
/* DP Only */
@@ -4495,41 +4755,11 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
return 0;
}
- dev_dbg(qmp->dev, "typec_mux_set: switching from qmpphy mode %d to %d\n",
- qmp->qmpphy_mode, new_mode);
+ /* The mux still receives Type-C events, even if all PHYs are uninitialized */
+ if (!qmp->init_count)
+ return 0;
- qmp->qmpphy_mode = new_mode;
-
- if (qmp->init_count) {
- if (qmp->usb_init_count)
- qmp_combo_usb_power_off(qmp->usb_phy);
-
- if (qmp->dp_init_count)
- writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
-
- qmp_combo_com_exit(qmp, true);
-
- /* Now everything's powered down, power up the right PHYs */
- qmp_combo_com_init(qmp, true);
-
- if (new_mode == QMPPHY_MODE_DP_ONLY) {
- if (qmp->usb_init_count)
- qmp->usb_init_count--;
- }
-
- if (new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_USB3_ONLY) {
- qmp_combo_usb_power_on(qmp->usb_phy);
- if (!qmp->usb_init_count)
- qmp->usb_init_count++;
- }
-
- if (new_mode == QMPPHY_MODE_DP_ONLY || new_mode == QMPPHY_MODE_USB3DP) {
- if (qmp->dp_init_count)
- cfg->dp_aux_init(qmp);
- }
- }
-
- return 0;
+ return qmp_combo_reconfigure_phy(qmp, new_mode);
}
static void qmp_combo_typec_switch_unregister(void *data)
@@ -4733,6 +4963,9 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
}
qmp->dp_dp_phy = base + offs->dp_dp_phy;
+ qmp->usb4_serdes = base + offs->usb4_serdes;
+ qmp->usb4_pcs = base + offs->usb4_pcs;
+
ret = qmp_combo_clk_init(qmp);
if (ret)
return ret;
@@ -4743,6 +4976,12 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
"failed to get usb3_pipe clock\n");
}
+ qmp->p2rr2p_pipe_clk = devm_clk_get_optional(dev, "p2rr2p_pipe");
+ if (IS_ERR(qmp->p2rr2p_pipe_clk)) {
+ return dev_err_probe(dev, PTR_ERR(qmp->p2rr2p_pipe_clk),
+ "failed to get p2rr2p_pipe clock\n");
+ }
+
return 0;
}
@@ -4758,6 +4997,8 @@ static struct phy *qmp_combo_phy_xlate(struct device *dev, const struct of_phand
return qmp->usb_phy;
case QMP_USB43DP_DP_PHY:
return qmp->dp_phy;
+ case QMP_USB43DP_USB4_PHY:
+ return qmp->usb4_phy ?: ERR_PTR(-EINVAL);
}
return ERR_PTR(-EINVAL);
@@ -4965,6 +5206,17 @@ static int qmp_combo_probe(struct platform_device *pdev)
phy_set_drvdata(qmp->dp_phy, qmp);
+ if (qmp->cfg->usb4_serdes_tbl) {
+ qmp->usb4_phy = devm_phy_create(dev, dev->of_node, &qmp_combo_usb4_phy_ops);
+ if (IS_ERR(qmp->usb4_phy)) {
+ ret = PTR_ERR(qmp->usb4_phy);
+ dev_err(dev, "failed to create USB4 PHY: %d\n", ret);
+ goto err_node_put;
+ }
+
+ phy_set_drvdata(qmp->usb4_phy, qmp);
+ }
+
if (usb_np == dev->of_node)
phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
else
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Add the offsets and configuration tables to support USB4 and
Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 200 ++++++++++++++++++++-
drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h | 2 +
drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h | 15 ++
.../phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h | 45 +++++
4 files changed, 256 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 898f42de4a08..0916d9a4599e 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -235,6 +235,7 @@ static const unsigned int qmp_v6_n4_usb3phy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR] = QPHY_V6_PCS_USB3_LFPS_RXTERM_IRQ_CLEAR,
[QPHY_PCS_CLAMP_ENABLE] = QPHY_V6_PCS_AON_CLAMP_ENABLE,
+ [QPHY_PCS_USB4_CLAMP_ENABLE] = QPHY_V6_PCS_AON_USB4_CLAMP_ENABLE,
[QPHY_COM_RESETSM_CNTRL] = QSERDES_V6_COM_RESETSM_CNTRL,
[QPHY_COM_C_READY_STATUS] = QSERDES_V6_COM_C_READY_STATUS,
@@ -1812,7 +1813,85 @@ static const struct qmp_phy_init_tbl sc8280xp_usb43dp_pcs_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_RXEQTRAINING_DFE_TIME_S2, 0x07),
};
-static const struct qmp_phy_init_tbl x1e80100_usb43dp_serdes_tbl[] = {
+static const struct qmp_phy_init_tbl x1e80100_usb4_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0, 0xd9),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE1, 0xb2),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_BUF_ENABLE, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE1, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE1, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE1, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_EN, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_CFG, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x41),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE1, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x43),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x82),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MSB_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE1, 0x86),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MSB_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE0, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE0, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE0, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE1, 0xa7),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE0, 0xba),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE2_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE1, 0x3c),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE2_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_SEL_1, 0x11),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORECLK_DIV_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x76),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORE_CLK_EN, 0xb0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO_MODE1, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE0, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE1, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_IPTRIM, 0x17),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SVS_MODE_CLK_SEL, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_MODE, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_VCO_DC_LEVEL_CTRL, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_ADDITIONAL_MISC_2, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_INITVAL2, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAXVAL2, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_POST_DIV_MUX, 0x60),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_BG_TIMER, 0x0a),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb4_tb3_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x15),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x54),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb4_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_1, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_2, 0x50),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_3, 0x40),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_TX, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_RX, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_LDO_TIMER_CTRL, 0x03),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb3dp_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
@@ -1873,6 +1952,68 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_tx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_RX, 0x0a),
};
+static const struct qmp_phy_init_tbl x1e80100_usb4_rx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_CNTRL, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_ENABLES, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B0, 0xd3),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B1, 0xee),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B2, 0x58),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B4, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B0, 0xbf),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B1, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B2, 0xa2),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B3, 0xdd),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B4, 0x6a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B5, 0xdb),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B6, 0xe3),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2, 0x30),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE2, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE3, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE3, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CTRL1, 0xd0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CTRL2, 0x48),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_THRESH1_RATE3, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE2, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN1_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_IVCM_CAL_CODE_OVERRIDE, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_IVCM_CAL_CTRL2, 0x80),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_SUMMER_CAL_SPD_MODE, 0x2f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_CTLE_POST_CAL_OFFSET, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DCC_CMUX_POSTCAL_OFFSET, 0x1d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE2, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE3, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE2, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE3, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CONTROLS, 0x15),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FASTLOCK_FO_GAIN_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FASTLOCK_SO_GAIN_RATE3, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_SATURATION, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_IVCM_POSTCAL_OFFSET, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VGA_CAL_CNTRL1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VGA_CAL_MAN_VAL, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_DAC_ENABLE1, 0x88),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_DAC_ENABLE2, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_2, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_3, 0x45),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP3_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP4_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP5_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_GM_CAL, 0x0d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_OFFSET_ADAPTOR_CNTRL3, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_EOM_CTRL1, 0x10),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_EOM_CTRL2, 0x21),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP3_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP4_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP5_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VTHRESH_CAL_MAN_VAL_RATE3, 0x78),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_Q_PI_INTRINSIC_BIAS_RATE32, 0x27),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_BKUP_CTRL1, 0x14),
+};
+
static const struct qmp_phy_init_tbl x1e80100_usb43dp_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_CNTRL, 0x04),
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
@@ -1911,6 +2052,23 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_BKUP_CTRL1, 0x14),
};
+static const struct qmp_phy_init_tbl x1e80100_usb4_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_POWER_STATE_CONFIG3, 0x4f),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG1, 0x03),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG2, 0xc0),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_RX_SIGDET_LVL, 0x55),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_PCS_PCS_TX_RX_CONFIG1, 0x51),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_EQ_CONFIG1, 0x2e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_EQ_CONFIG1, 0x4b),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_EQ_CONFIG5, 0x1e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG1, 0x83),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG3, 0x2e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG5, 0x10),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG3, 0x25),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG5, 0x22),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LPFS_TX_ECSTART, 0x0f),
+};
+
static const struct qmp_phy_init_tbl x1e80100_usb43dp_pcs_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V6_N4_PCS_RCVR_DTCT_DLY_P1U2_L, 0xe7),
QMP_PHY_INIT_CFG(QPHY_V6_N4_PCS_RCVR_DTCT_DLY_P1U2_H, 0x03),
@@ -2429,6 +2587,23 @@ static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
.dp_dp_phy = 0x2200,
};
+static const struct qmp_combo_offsets qmp_combo_offsets_v6_n4 = {
+ .com = 0x0000,
+ .usb3_pcs_aon = 0x0100,
+ .txa = 0x0400,
+ .rxa = 0x0600,
+ .txb = 0x0a00,
+ .rxb = 0x0c00,
+ .usb3_serdes = 0x1000,
+ .usb3_pcs_misc = 0x1200,
+ .usb3_pcs = 0x1400,
+ .usb3_pcs_usb = 0x1700,
+ .dp_serdes = 0x2000,
+ .dp_dp_phy = 0x2200,
+ .usb4_serdes = 0x3000,
+ .usb4_pcs = 0x3400,
+};
+
static const struct qmp_combo_offsets qmp_combo_offsets_v8 = {
.com = 0x0000,
.txa = 0x1400,
@@ -2689,11 +2864,11 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
.regs = qmp_v5_5nm_usb3phy_regs_layout,
};
-static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
- .offsets = &qmp_combo_offsets_v5,
+static const struct qmp_phy_cfg x1e80100_usb43dpphy_cfg = {
+ .offsets = &qmp_combo_offsets_v6_n4,
- .serdes_tbl = x1e80100_usb43dp_serdes_tbl,
- .serdes_tbl_num = ARRAY_SIZE(x1e80100_usb43dp_serdes_tbl),
+ .serdes_tbl = x1e80100_usb3dp_serdes_tbl,
+ .serdes_tbl_num = ARRAY_SIZE(x1e80100_usb3dp_serdes_tbl),
.tx_tbl = x1e80100_usb43dp_tx_tbl,
.tx_tbl_num = ARRAY_SIZE(x1e80100_usb43dp_tx_tbl),
.rx_tbl = x1e80100_usb43dp_rx_tbl,
@@ -2728,6 +2903,19 @@ static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
.configure_dp_phy = qmp_v4_configure_dp_phy,
.calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
+ .usb4_serdes_tbl = x1e80100_usb4_serdes_tbl,
+ .usb4_serdes_tbl_num = ARRAY_SIZE(x1e80100_usb4_serdes_tbl),
+ .usb4_serdes_tb3_ovrd_tbl = x1e80100_usb4_tb3_serdes_tbl,
+ .usb4_serdes_tb3_ovrd_num = ARRAY_SIZE(x1e80100_usb4_tb3_serdes_tbl),
+
+ .usb4_tx_tbl = x1e80100_usb4_tx_tbl,
+ .usb4_tx_tbl_num = ARRAY_SIZE(x1e80100_usb4_tx_tbl),
+ .usb4_rx_tbl = x1e80100_usb4_rx_tbl,
+ .usb4_rx_tbl_num = ARRAY_SIZE(x1e80100_usb4_rx_tbl),
+
+ .usb4_pcs_tbl = x1e80100_usb4_pcs_tbl,
+ .usb4_pcs_tbl_num = ARRAY_SIZE(x1e80100_usb4_pcs_tbl),
+
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list = qmp_phy_vreg_l,
@@ -5296,7 +5484,7 @@ static const struct of_device_id qmp_combo_of_match_table[] = {
},
{
.compatible = "qcom,x1e80100-qmp-usb3-dp-phy",
- .data = &x1e80100_usb3dpphy_cfg,
+ .data = &x1e80100_usb43dpphy_cfg,
},
{ }
};
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
index 52db31a7cf22..f19f461297b6 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
@@ -8,5 +8,7 @@
/* Only for QMP V6 PHY - PCS_AON registers */
#define QPHY_V6_PCS_AON_CLAMP_ENABLE 0x00
+/* Valid only for USB43DP variants */
+#define QPHY_V6_PCS_AON_USB4_CLAMP_ENABLE 0x04
#endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
index df670143feb1..30fdb437146c 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
@@ -14,4 +14,19 @@
#define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_L 0x40
#define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_H 0x44
+#define QPHY_V6_PCS_USB4_POWER_STATE_CONFIG3 0x98
+#define QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG1 0xd0
+#define QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG2 0xd4
+#define QPHY_V6_PCS_USB4_RX_SIGDET_LVL 0x17c
+#define QPHY_V6_PCS_USB4_PCS_PCS_TX_RX_CONFIG1 0x1a8
+#define QPHY_V6_PCS_USB4_EQ_CONFIG1 0x1b8
+#define QPHY_V6_PCS_USB4_G3_EQ_CONFIG1 0x1d8
+#define QPHY_V6_PCS_USB4_G3_EQ_CONFIG5 0x1e8
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG1 0x1f4
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG3 0x1fc
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG5 0x204
+#define QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG3 0x210
+#define QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG5 0x218
+#define QPHY_V6_PCS_USB4_LPFS_TX_ECSTART 0x220
+
#endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
index d37cc0d4fd36..e72ae2cb460a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
@@ -23,27 +23,63 @@
#define QSERDES_V6_N4_TX_TRAN_DRVR_EMP_EN 0xac
#define QSERDES_V6_N4_TX_TX_BAND 0xd8
#define QSERDES_V6_N4_TX_INTERFACE_SELECT 0xe4
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE210 0xe8
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE3 0xec
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE210 0xf0
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE3 0xf4
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE210 0xf8
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE3 0xfc
#define QSERDES_V6_N4_TX_VMODE_CTRL1 0xb0
+#define QSERDES_V6_TX_LDO_TIMER_CTRL 0x120
#define QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE2 0x8
+#define QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE3 0xc
+#define QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE3 0x1c
#define QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE2 0x18
#define QSERDES_V6_N4_RX_UCDR_PI_CONTROLS 0x20
+#define QSERDES_V6_N4_RX_EOM_CTRL1 0x44
+#define QSERDES_V6_N4_RX_EOM_CTRL2 0x48
#define QSERDES_V6_N4_RX_IVCM_CAL_CODE_OVERRIDE 0x94
#define QSERDES_V6_N4_RX_RX_IVCM_CAL_CTRL2 0x9c
#define QSERDES_V6_N4_RX_RX_IVCM_POSTCAL_OFFSET 0xa0
+#define QSERDES_V6_N4_RX_DFE_1 0xac
+#define QSERDES_V6_N4_RX_DFE_2 0xb0
#define QSERDES_V6_N4_RX_DFE_3 0xb4
+#define QSERDES_V6_N4_RX_DFE_TAP3_CTRL 0xbc
+#define QSERDES_V6_N4_RX_DFE_TAP3_MANVAL_KTAP 0xc0
+#define QSERDES_V6_N4_RX_DFE_TAP4_CTRL 0xc4
+#define QSERDES_V6_N4_RX_DFE_TAP4_MANVAL_KTAP 0xc8
+#define QSERDES_V6_N4_RX_DFE_TAP5_CTRL 0xcc
+#define QSERDES_V6_N4_RX_DFE_TAP5_MANVAL_KTAP 0xd0
#define QSERDES_V6_N4_RX_VGA_CAL_CNTRL1 0xe0
#define QSERDES_V6_N4_RX_VGA_CAL_MAN_VAL 0xe8
+#define QSERDES_V6_N4_RX_VTHRESH_CAL_MAN_VAL_RATE3 0x108
#define QSERDES_V6_N4_RX_GM_CAL 0x10c
+#define QSERDES_V6_N4_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1 0x13c
+#define QSERDES_V6_N4_RX_RX_OFFSET_ADAPTOR_CNTRL3 0x144
#define QSERDES_V6_N4_RX_SIGDET_ENABLES 0x148
#define QSERDES_V6_N4_RX_SIGDET_CNTRL 0x14c
#define QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL 0x154
#define QSERDES_V6_N4_RX_DFE_CTLE_POST_CAL_OFFSET 0x194
+#define QSERDES_V6_N4_RX_DCC_CMUX_POSTCAL_OFFSET 0x1a8
#define QSERDES_V6_N4_RX_Q_PI_INTRINSIC_BIAS_RATE32 0x1dc
+#define QSERDES_V6_N4_RX_UCDR_FASTLOCK_FO_GAIN_RATE3 0x200
+#define QSERDES_V6_N4_RX_UCDR_FASTLOCK_SO_GAIN_RATE3 0x210
+#define QSERDES_V6_N4_RX_UCDR_SO_SATURATION 0x214
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE1 0x224
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE1 0x228
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2 0x22c
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE2 0x230
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE3 0x234
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE3 0x238
#define QSERDES_V6_N4_RX_UCDR_PI_CTRL1 0x23c
#define QSERDES_V6_N4_RX_UCDR_PI_CTRL2 0x240
+#define QSERDES_V6_N4_RX_UCDR_SB2_THRESH1_RATE3 0x250
+#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN1_RATE3 0x270
#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE2 0x27c
+#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE3 0x280
#define QSERDES_V6_N4_RX_DFE_DAC_ENABLE1 0x298
+#define QSERDES_V6_N4_RX_DFE_DAC_ENABLE2 0x29c
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B0 0x2b8
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B1 0x2bc
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B2 0x2c0
@@ -58,6 +94,15 @@
#define QSERDES_V6_N4_RX_MODE_RATE2_B4 0x2e4
#define QSERDES_V6_N4_RX_MODE_RATE2_B5 0x2e8
#define QSERDES_V6_N4_RX_MODE_RATE2_B6 0x2ec
+
+#define QSERDES_V6_N4_RX_MODE_RATE3_B0 0x2f0
+#define QSERDES_V6_N4_RX_MODE_RATE3_B1 0x2f4
+#define QSERDES_V6_N4_RX_MODE_RATE3_B2 0x2f8
+#define QSERDES_V6_N4_RX_MODE_RATE3_B3 0x2fc
+#define QSERDES_V6_N4_RX_MODE_RATE3_B4 0x300
+#define QSERDES_V6_N4_RX_MODE_RATE3_B5 0x304
+#define QSERDES_V6_N4_RX_MODE_RATE3_B6 0x308
+
#define QSERDES_V6_N4_RX_RX_SUMMER_CAL_SPD_MODE 0x30c
#define QSERDES_V6_N4_RX_RX_BKUP_CTRL1 0x310
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
From: Konrad Dybcio @ 2026-05-18 10:29 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
In-Reply-To: <20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com>
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The USB4 part of the QMPPHY requires that one more GCC clock (P2RR2P -
PHY-to-Router, Router-to-PHY) is enabled for the PHY to initialize
successfully. Describe that.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/hamoa.dtsi | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index 4ba751a65142..63430f49ba2a 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -2883,11 +2883,13 @@ usb_1_ss0_qmpphy: phy@fd5000 {
clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
<&rpmhcc RPMH_CXO_CLK>,
<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_0_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_0_PHY_GDSC>;
@@ -2954,11 +2956,13 @@ usb_1_ss1_qmpphy: phy@fda000 {
clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK>,
<&tcsr TCSR_USB4_1_CLKREF_EN>,
<&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_1_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_1_PHY_GDSC>;
@@ -3025,11 +3029,13 @@ usb_1_ss2_qmpphy: phy@fdf000 {
clocks = <&gcc GCC_USB3_TERT_PHY_AUX_CLK>,
<&tcsr TCSR_USB4_2_CLKREF_EN>,
<&gcc GCC_USB3_TERT_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_TERT_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_TERT_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_2_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_2_PHY_GDSC>;
--
2.54.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
From: sashiko-bot @ 2026-05-18 10:47 UTC (permalink / raw)
To: Konrad Dybcio
Cc: conor+dt, neil.armstrong, devicetree, olteanv, vkoul, linux-phy,
robh
In-Reply-To: <20260518-topic-usb4phy-v1-1-71d827c49dca@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The binding incorrectly relaxes clock constraints by grouping 'x1e80100' with 'sc7180' and 'sdm845' and globally changing the 5th 'clock-names' item to an enum.
--
commit 54d1f0fd2346d0dbf486ca5212767ad33f59fde0
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
This commit extends the QMP combo PHY binding for X1E80100 to support USB4
communication. It updates the schema to allow a new clock, P2RR2P, which is
required for switching to USB4 mode, and adds a corresponding definition.
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> @@ -52,7 +52,7 @@ properties:
> - const: ref
> - const: com_aux
> - const: usb3_pipe
> - - const: cfg_ahb
> + - enum: [ p2rr2p_pipe, cfg_ahb ]
Does this global enum change inadvertently relax the constraint for all
variants?
It appears that older variants which previously strictly required cfg_ahb
could now pass schema validation using p2rr2p_pipe instead, and vice versa.
[ ... ]
> @@ -186,6 +186,7 @@ allOf:
> enum:
> - qcom,sc7180-qmp-usb3-dp-phy
> - qcom,sdm845-qmp-usb3-dp-phy
> + - qcom,x1e80100-qmp-usb3-dp-phy
> then:
> properties:
> clocks:
Should x1e80100 be placed in its own if/then block?
Grouping x1e80100 with sc7180 and sdm845 means the schema only enforces the
maximum number of items, but doesn't strictly tie the 5th clock to the
specific hardware generation. A separate block would ensure x1e80100 is
restricted to p2rr2p_pipe, while the older variants remain restricted to
cfg_ahb.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 2/5] phy: core: Define TBT phy_mode
From: sashiko-bot @ 2026-05-18 11:03 UTC (permalink / raw)
To: Konrad Dybcio
Cc: olteanv, conor+dt, devicetree, vkoul, linux-phy, robh,
neil.armstrong
In-Reply-To: <20260518-topic-usb4phy-v1-2-71d827c49dca@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The tbt_phy_submode enum implicitly sets PHY_SUBMODE_TBT3 to 0, making it the hidden default submode for phy_set_mode().
- [Low] Using PHY_MODE_TBT as the generic mode with USB4 as a submode conceptually inverts the industry standard hierarchy.
--
commit f1ad33ca1ed62aa8d4ab806ced09908d4e9f3625
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
phy: core: Define TBT phy_mode
This commit introduces a new PHY mode for Thunderbolt (TBT) and USB4 PHYs
that require specific electrical and analog configurations. It defines
PHY_MODE_TBT along with PHY_SUBMODE_TBT3 and PHY_SUBMODE_USB4 submodes.
> diff --git a/include/linux/phy/phy-tbt.h b/include/linux/phy/phy-tbt.h
> new file mode 100644
> index 0000000000000..5f48059814e19
> --- /dev/null
> +++ b/include/linux/phy/phy-tbt.h
> @@ -0,0 +1,14 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
> + */
> +
> +#ifndef __PHY_TBT_H_
> +#define __PHY_TBT_H_
> +
> +enum tbt_phy_submode {
> + PHY_SUBMODE_TBT3,
Since PHY_SUBMODE_TBT3 is the first element, it implicitly evaluates to 0.
The generic phy_set_mode(phy, mode) macro in include/linux/phy/phy.h wraps
phy_set_mode_ext(phy, mode, 0).
Does this mean any driver calling phy_set_mode(phy, PHY_MODE_TBT) will
silently default to requesting the legacy TBT3 electrical/timing
configuration instead of USB4?
Would it be safer to reserve 0 for an unspecified state (e.g.,
PHY_SUBMODE_UNSPECIFIED = 0) to force explicit submode selection, or to
make the modern standard (USB4) the default?
> + PHY_SUBMODE_USB4,
> +};
> +
> +#endif
> diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
> index ea47975e288ae..26a91c070f8ce 100644
> --- a/include/linux/phy/phy.h
> +++ b/include/linux/phy/phy.h
[ ... ]
> @@ -45,6 +46,7 @@ enum phy_mode {
> PHY_MODE_LVDS,
> PHY_MODE_DP,
> PHY_MODE_HDMI,
> + PHY_MODE_TBT,
Is using PHY_MODE_TBT as the generic mode with USB4 as a submode the best
hierarchy here?
USB4 is the foundational open standard established by the USB-IF, and the
Linux kernel USB and Type-C subsystems natively treat USB4 as the primary
mode (e.g., TYPEC_MODE_USB4).
Could grouping USB4 under a Thunderbolt PHY mode cause semantic confusion
and contradict existing kernel USB nomenclature? Might it be more
consistent to use PHY_MODE_USB4 as the primary mode, with TBT3 as a
legacy fallback submode?
> };
>
> enum phy_media {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v3 1/2] phy: rockchip: inno-hdmi: Add configure() and validate() ops
From: Jonas Karlman @ 2026-05-18 11:19 UTC (permalink / raw)
To: Vinod Koul
Cc: Neil Armstrong, Heiko Stuebner, linux-phy, linux-rockchip,
linux-arm-kernel, linux-kernel
In-Reply-To: <agny1ER5j6om9hIT@vaman>
On 5/17/2026 6:54 PM, Vinod Koul wrote:
> On 15-05-26, 23:04, Jonas Karlman wrote:
>> Hi,
>>
>> On 5/15/2026 9:55 PM, Jonas Karlman wrote:
>>> The commit 10ed34d6eaaf ("phy: Add HDMI configuration options")
>>> introduced a way for HDMI PHYs to be configured through the generic
>>> phy_configure() function.
>>>
>>> This driver derives the TMDS character rate from the pixel clock and the
>>> PHY bus width setting. However, no in-tree consumer of this PHY has ever
>>> called phy_set_bus_width() to change the TMDS character rate as only
>>> 8-bit RGB output is supported by the HDMI display driver.
>>>
>>> Add configure() and validate() ops to allow consumers to configure the
>>> TMDS character rate using phy_configure(). Fallback to the deprecated
>>> way of using the PHY bus width to configure the TMDS character rate.
>>>
>>> A typical call chain during DRM modeset on a RK3328 device:
>>>
>>> dw_hdmi_rockchip_encoder_atomic_check():
>>> - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
>>>
>>> dw_hdmi_rockchip_encoder_atomic_mode_set():
>>> - inno_hdmi_phy_configure(): pixclock 148500000
>>> - inno_hdmi_phy_validate(): pixclock 148500000 tmdsclock 594000000
>>>
>>> vop_crtc_atomic_enable():
>>> - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
>>> - inno_hdmi_phy_rk3328_clk_set_rate(): pixclock 594000000 tmdsclock 594000000
>>> - inno_hdmi_phy_rk3328_clk_recalc_rate(): pixclock 594000000 vco 594000000
>>>
>>> dw_hdmi_rockchip_encoder_enable():
>>> - inno_hdmi_phy_power_on(): Inno HDMI PHY Power On
>>> - inno_hdmi_phy_rk3328_clk_set_rate(): rate 594000000 tmdsclk 594000000
>>>
>>> Signed-off-by: Jonas Karlman <jonas@kwiboo.se>
>>> ---
>>> Changes in v3:
>>> - Change validate() ops to only validate tmdsclock
>>> - Add comments about expected consumer usage
>>> - Update commit message with a typical call chain
>>> Changes in v2:
>>> - Add validate() ops to validate that the TMDS rate is supported
>>> - Split out parts that remove the old workaround into a separate patch
>>>
>>> Patch "drm/rockchip: dw_hdmi: Configure HDMI PHY in atomic_mode_set()"
>>> at [1] adds phy_validate() and phy_configure() calls for this HDMI PHY.
>>>
>>> [1] https://lore.kernel.org/dri-devel/20260510183114.1248840-10-jonas@kwiboo.se/
>>> ---
>>> drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 60 ++++++++++++++++++-
>>> 1 file changed, 59 insertions(+), 1 deletion(-)
>>
>> [snip]
>>
>>> +static int inno_hdmi_phy_validate(struct phy *phy, enum phy_mode mode,
>>> + int submode, union phy_configure_opts *opts)
>>> +{
>>> + const struct pre_pll_config *cfg = pre_pll_cfg_table;
>>> + unsigned long tmdsclock;
>>> +
>>> + if (!(mode == PHY_MODE_HDMI && submode == PHY_HDMI_MODE_TMDS))
>>> + return -EINVAL;
>>> +
>>> + if (!opts->hdmi.tmds_char_rate || opts->hdmi.tmds_char_rate > 594000000)
>>> + return -EINVAL;
>>
>> Sashiko reasoning log pointed out that a consumer of phy_validate() or
>> phy_configure() can make a call with opts=NULL, so I may likely send a
>> v4 of this series to fix such possible NULL pointer dereference here.
>
> Yes please
I will send a v4 that includes a NULL check, thanks.
>
>>
>> Or is that something that possible should be checked before phy core
>> calls the .validate()/.configure() ops?
>
> The opts is an optional argument depending upon the mode to be valid. So
> we cant do that in the core...
>
>>
>> Multiple other phy .configure() ops seem to dereference opts members
>> without any type of opts NULL check. (next-20260508)
>
> We should fix these
>
>
> PS: Any reason why you are sending encypted emails?
Hopefully you find a way to read this :-)
My email provider tries to be smart and encrypts any outgoing mail when
there is a PGP key published for a recipient in a Web Key Directory,
something that kernel.org has [2].
I raised an issue with the provider and was hoping that the change they
made [3] would fix the issue, and it looks like your published PGP key
is not sign-only and is in fact encryption-capable.
Will open a new issue and ask them to add some way to fully disable use
of automatic encryption on outgoing mails.
[2] https://www.kernel.org/category/signatures.html
[3] https://github.com/forwardemail/forwardemail.net/commit/88def5438a8da678252c234e876d3cdd469a8202
Regards,
Jonas
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox