* [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
* Re: [PATCH 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
From: sashiko-bot @ 2026-05-18 11:32 UTC (permalink / raw)
To: Konrad Dybcio
Cc: conor+dt, neil.armstrong, olteanv, linux-phy, vkoul, robh,
devicetree
In-Reply-To: <20260518-topic-usb4phy-v1-3-71d827c49dca@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [Critical] Accessing hardware registers before enabling regulators and clocks causes a system crash.
- [High] Unconditional dereference of the `pcs_aon` pointer in `qmp_combo_com_init()`.
- [High] Register mismatch when checking for an active DP tunnel breaks the DP PLL.
- [High] Incomplete masking of DP reset bits in `qmp_combo_com_init()` actively forces the DP PHY into reset.
- [High] Type-C mux events are permanently dropped before the PHY is initialized, breaking alt modes.
- [High] Double hardware initialization and `pipe_clk` reference leak in `qmp_combo_usb4_init()`.
- [High] Type-C cable orientation state is discarded for USB3 connections following a USB4 disconnect.
- [High] Error paths in `qmp_combo_usb4_init()` leak the `p2rr2p_pipe_clk` reference.
- [High] TOCTOU data race on `qmpphy_mode` and `orientation` in `qmp_combo_typec_switch_set()`.
- [High] Autonomous mode functions access unclocked `pcs_usb` registers, causing a hardware hang.
- [High] Missing lock in `qmp_combo_typec_mux_set()` leads to concurrent PHY reconfiguration races.
- [High] `init_count` underflows on error paths in `qmp_combo_com_init()` when `force` is true.
--
commit f92b7738f7f78a59373363a1b4f2191edf48f3e5
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
phy: qualcomm: qmp-combo: Add preliminary USB4 support
This commit adds preliminary USB4 support for Qualcomm QMP combo PHYs,
introducing a new USB4 submode and handling mutually exclusive USB4 and
USB3/DP operation modes.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 93f1aa10d4008..898f42de4a086 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> @@ -3691,14 +3691,26 @@ 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 force is true, this bypasses the init_count increment. However, if a
failure occurs later in the function (e.g., regulator_bulk_enable fails),
the error path jumps to err_decrement_count and executes qmp->init_count--.
Could this decrement a count that was never incremented, corrupting the
driver state?
>
> + if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
> + pcs = qmp->usb4_pcs;
> +
> + qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
Could pcs_aon be NULL here? Other functions like
qmp_combo_enable_autonomous_mode() check if pcs_aon is valid before using it.
If a platform lacks this memory region in the device tree, will this cause a
kernel panic?
Can accessing these hardware registers before enabling regulators and clocks
cause a system crash? The readl() and qphy_setbits() are called before
regulator_bulk_enable() and clk_bulk_prepare_enable(). Accessing unpowered
or unclocked hardware domains often triggers a synchronous external abort.
> +
> + /* 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)
Does this read the wrong register? DP_TUNNELING_CLOCK_GEN_EN is defined as a
bit in QPHY_V3_DP_COM_PHY_MODE_CTRL, rather than QPHY_V3_DP_COM_RESET_OVRD_CTRL.
This might evaluate SW_USB3PHY_RESET_MUX instead, leading to incorrect DP reset
logic.
> + dp_reset_val &= ~SW_DPPHY_RESET_MUX;
> + }
Does this incomplete mask force the DP PHY into reset? dp_reset_val is
initially SW_DPPHY_RESET_MUX | SW_DPPHY_RESET, so this clears the mux bit
but leaves SW_DPPHY_RESET set to 1.
> +
> ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
[ ... ]
> @@ -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);
When this is applied, since qphy_setbits() performs a bitwise OR, it cannot
clear SW_DPPHY_RESET_MUX in the hardware if it was already 1. Combined with
SW_DPPHY_RESET still being 1 in dp_reset_val, won't this actively put the
DP PHY into reset and destroy the state it was intended to protect?
[ ... ]
> @@ -3991,6 +4098,119 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
[ ... ]
> +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;
Do the error paths in this function leak the p2rr2p_pipe_clk reference?
If qmp_combo_com_init(), qmp_combo_reconfigure_phy(), or
qmp_combo_usb_power_on() fail, the function returns directly without calling
clk_disable_unprepare() for p2rr2p_pipe_clk.
> +
> + /* 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);
Can this cause a double hardware initialization and leak the pipe_clk reference?
If the USB3 PHY is already active, qmp_combo_reconfigure_phy() internally
calls qmp_combo_usb_power_on(qmp->usb_phy). Immediately after, this function
unconditionally calls qmp_combo_usb_power_on(phy) a second time. This would
enable the hardware twice and increment the pipe_clk reference count twice,
but usb4_exit() only calls power_off once.
[ ... ]
> @@ -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);
> }
In USB4 mode, qmp_combo_com_init() does not clear SW_USB3PHY_RESET, leaving
the USB3 PCS block in hardware reset. However,
qmp_combo_enable_autonomous_mode() unconditionally accesses pcs_usb registers
earlier in the function (e.g., QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR). Will
accessing an IP block held in reset cause an external abort?
[ ... ]
> @@ -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);
Is there a data race here? The check for qmp->qmpphy_mode and the
modification of qmp->orientation are performed before acquiring
qmp->phy_mutex. If an orientation switch event races with
qmp_combo_usb4_init() modifying qmpphy_mode, switch_set could read a stale
mode and enter the critical section to execute a full PHY reset while the
USB4 router is actively using it.
Also, if a USB4 device is unplugged, usb4_exit is called but qmpphy_mode
remains QMPPHY_MODE_USB4. If a USB3 device is then plugged in, switch_set()
is typically called before mux_set(). Because it sees the stale USB4 mode,
it drops the valid orientation and forces TYPEC_ORIENTATION_NONE.
When mux_set() is subsequently called, it configures USB3 with no orientation.
Will this cause enumeration to fail if the Type-C cable is flipped?
[ ... ]
> @@ -4495,41 +4755,11 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
[ ... ]
> + /* The mux still receives Type-C events, even if all PHYs are uninitialized */
> + if (!qmp->init_count)
> + return 0;
>
> - return 0;
> + return qmp_combo_reconfigure_phy(qmp, new_mode);
> }
Does this early return break alt modes by dropping Type-C events? It
bypasses saving the requested mode (qmp->qmpphy_mode = new_mode). Since
Type-C events often arrive before the USB controller initializes the PHY,
the requested mode is lost. When phy_init() is later called, it will
initialize with the stale default mode, breaking USB3 or DP routing.
Additionally, qmp_combo_reconfigure_phy() is called here without acquiring
qmp->phy_mutex. Concurrently, qmp_combo_usb4_init() uses the mutex and also
calls qmp_combo_reconfigure_phy(). Doesn't this lack of synchronization
permit concurrent execution of PHY hardware power down/up sequences,
potentially corrupting register states?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
From: sashiko-bot @ 2026-05-18 11:49 UTC (permalink / raw)
To: Konrad Dybcio
Cc: olteanv, linux-phy, devicetree, conor+dt, robh, neil.armstrong,
vkoul
In-Reply-To: <20260518-topic-usb4phy-v1-4-71d827c49dca@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] Incorrect `TX_` prefix for RX UCDR fastlock registers.
- [Low] Typo in register macro name: `LPFS` instead of `LFPS`.
- [Low] Redundant `PCS_PCS` prefix in register macro name.
- [Low] Missing `_N4_` infix in the `QSERDES_V6_TX_LDO_TIMER_CTRL` macro.
--
commit 13a3731f3b7c12c298d2f498011a7ef852fd4a42
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
This commit adds the offsets and configuration tables to support USB4
and Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 898f42de4a086..0916d9a4599ed 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> @@ -1873,6 +1952,68 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_tx_tbl[] = {
> [ ... ]
> +static const struct qmp_phy_init_tbl x1e80100_usb4_rx_tbl[] = {
> [ ... ]
> + 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),
This isn't a bug, but should these use the RX_ prefix instead of TX_?
These registers appear to map to the RX block address space and are used
here to configure the RX hardware.
[ ... ]
> 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 df670143feb1e..30fdb437146c4 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_PCS_PCS_TX_RX_CONFIG1 0x1a8
This isn't a bug, but is there a duplicate PCS_ prefix in this macro name?
> +#define QPHY_V6_PCS_USB4_EQ_CONFIG1 0x1b8
> [ ... ]
> +#define QPHY_V6_PCS_USB4_LPFS_TX_ECSTART 0x220
This isn't a bug, but is this a typo for LFPS (Low Frequency Periodic
Signaling)?
[ ... ]
> 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 d37cc0d4fd365..e72ae2cb460ac 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_VMODE_CTRL1 0xb0
> +#define QSERDES_V6_TX_LDO_TIMER_CTRL 0x120
This isn't a bug, but should this macro include the _N4_ infix for
consistency with the rest of the V6 N4 block definitions?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-topic-usb4phy-v1-0-71d827c49dca@oss.qualcomm.com?part=4
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: qcom: Add Shikra CQM SoM platform
From: Kamal Wadhwa @ 2026-05-18 11:49 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Rakesh Kota, linux-arm-msm, sashiko-reviews, Komal Bajaj, robh,
linux-phy, neil.armstrong, vkoul, olteanv, krzk+dt, conor+dt,
devicetree
In-Reply-To: <obdxony34ckfbtsqj66siszw44sgieskcye5hg66ulkisctpwl@fyqczpze5tnw>
On Sun, May 17, 2026 at 08:18:15PM +0300, Dmitry Baryshkov wrote:
> On Thu, May 14, 2026 at 04:09:18PM +0530, Kamal Wadhwa wrote:
> > On Wed, May 13, 2026 at 06:14:20PM +0300, Dmitry Baryshkov wrote:
> > > On 13/05/2026 17:29, Rakesh Kota wrote:
> > > > On Wed, May 13, 2026 at 03:01:47PM +0300, Dmitry Baryshkov wrote:
> > > > > On Wed, May 13, 2026 at 04:28:35AM +0000, sashiko-bot@kernel.org wrote:
> > > > > > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > > > > > - [High] The PMIC regulator definitions omit their required input supply dependencies (e.g., `vdd_s2-supply`, `vdd_l3-supply`), breaking the power hierarchy.
> > > > > > - [Medium] The device tree inaccurately hardcodes the `compatible` string to a different PMIC model (`qcom,rpm-pm2250-regulators`) instead of explicitly identifying the actual hardware (PM4125).
> > > > > > --
> > > > > >
> > > > > > commit 457abf6e7ac410430c866842e08e602ec3daaa51
> > > > > > Author: Komal Bajaj <komal.bajaj@oss.qualcomm.com>
> > > > > >
> > > > > > arm64: dts: qcom: Add Shikra CQM SoM platform
> > > > > >
> > > > > > Add device tree include for the CQM variant of the Shikra System-on-Module.
> > > > > > This variant integrates the Shikra SoC and PM4125 PMIC. The DTSI includes
> > > > > > the common shikra.dtsi and adds PM4125 PMIC regulator definitions.
> > > > > >
> > > > > > > diff --git a/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > > new file mode 100644
> > > > > > > index 0000000000000..401e71720519d
> > > > > > > --- /dev/null
> > > > > > > +++ b/arch/arm64/boot/dts/qcom/shikra-cqm-som.dtsi
> > > > > > [ ... ]
> > > > > > > +&rpm_requests {
> > > > > > > + regulators {
> > > > > > > + compatible = "qcom,rpm-pm2250-regulators";
> > > > > >
> > > > > > Since the commit message indicates this is a PM4125 PMIC, should the
> > > > > > compatible string include a specific identifier for the actual hardware?
> > > > > >
> > > > > > Using a specific identifier like "qcom,rpm-pm4125-regulators" alongside
> > > > > > the fallback allows the kernel to apply specific quirks if PM4125 errata
> > > > > > are discovered later.
> > > > >
> > > > > This can be ignored. The compat is a leftover from the historically
> > > > > incorrect naming.
> > > > >
> > > > > >
> > > > > > > +
> > > > > > > + pm4125_s2: s2 {
> > > > > > > + regulator-min-microvolt = <1000000>;
> > > > > > > + regulator-max-microvolt = <1200000>;
> > > > > > > + };
> > > > > >
> > > > > > Do these regulators need to explicitly define their input supply dependencies
> > > > > > such as vdd_s2-supply?
> > > > > >
> > > > > > Without these properties, the regulator framework might be unaware that the
> > > > > > PMIC regulators draw power from upstream supplies.
> > > > > >
> > > > > > If the kernel dynamically manages the upstream supply and its reference count
> > > > > > drops to zero, could it be disabled, causing an unexpected power loss for
> > > > > > downstream components?
> > > > >
> > > > > And this is a correct comment. Please provide missing supplies.
> > > > >
> > > > As per the Qualcomm system design, the parent-child supply relationship
> > > > is managed by the RPM firmware, not the Linux regulator framework. The
> > > > RPM ensures the parent supply is never disabled until all subsystem
> > > > votes are cleared.
> > >
> > > How is this different from other, previous platforms?
> >
> > This is not different. In the previous platforms too this is taken care from the
> > RPM/RPMH firmware side, the only case where we may need explicit vote to parent
> > is for non-rpmh/rpm regulator rails (like i2c based regulator pm8008), which
> > may have a RPM/RPMH regulator as a parent.
> >
> > Even on those previous targets the parent rail of all RPM/RPMH regulators are
> > internally voted by RPM/RPMH FW at proper voltage with required headroom
> > calculated based on the active child rails. This was done for all the
> > subsystems (including APPS) regulators.
> >
> > So no explicit handling from the APPS is required for parent supply.
>
> You are explaining the driver behaviour. But the question is about the
> hardware description. If there is no difference, please add necessary
> supplies back.
I understand your concern about descibing the parent-child relation in the
devicetree, and given that we have been almost always followed this for all
the previous targets, it will expected of us to add them.
However, we want to avoid the unnecessary access to the parent from APPS.
At the moment, I do not see a way to avoid that, if we add the parent
regulators.
@Bjorn, @Konrad - can you please also share your suggestion, how we can add
parent-child desciption, but avoid accessing parent supply from APPS, as its
Qualcomm's system design to handle this on RPM/RPMH firmware side (you may
recall we had a verbal/offline discussion about same concern in context of
RPMH regulators earlier).
Regards,
Kamal
>
> --
> With best wishes
> Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v1 3/6] dt-bindings: net: Document Infineon/Intel XMM6260 modem
From: Krzysztof Kozlowski @ 2026-05-18 12:04 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Neil Armstrong, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Peter Chen, netdev, devicetree, linux-kernel,
linux-phy, linux-tegra, linux-usb
In-Reply-To: <CAPVz0n3mvBLracMWGNuJ8kKUvAVZ+JRTJVkZGyq5MkuSobd8NQ@mail.gmail.com>
On Fri, May 15, 2026 at 11:39:49AM +0300, Svyatoslav Ryhel wrote:
> пт, 15 трав. 2026 р. о 11:18 Krzysztof Kozlowski <krzk@kernel.org> пише:
> >
> > On Mon, May 11, 2026 at 04:56:58PM +0300, Svyatoslav Ryhel wrote:
> > > + ap-wake-gpios:
> > > + description: GPIO connected to the EINT3 pin
> > > + maxItems: 1
> > > +
> > > + cp-wake-gpios:
> > > + description: GPIO connected to the EINT2 pin
> > > + maxItems: 1
> > > +
> > > + phys:
> > > + maxItems: 1
> > > +
> > > + vbat-supply:
> > > + description: Supply connected to the VBAT lines.
> >
> > This should be rather name of the pin on this device, not the provider
> > name. VBAT suggests the latter. Please double check.
> >
>
> There is no documentation on this device from Infineon, I have used
> schematics of the P895 where this modem is used. According to it power
> supply is connected to pin with name VBAT (and yes, I am not mixing it
> up since supplies have different names)
ack
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v1 5/6] dt-bindings: phy: tegra: Document Nvidia Tegra XMM6260 PHY
From: Krzysztof Kozlowski @ 2026-05-18 12:14 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Neil Armstrong, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Peter Chen, netdev, devicetree, linux-kernel,
linux-phy, linux-tegra, linux-usb
In-Reply-To: <CAPVz0n2wrAdU0JKx7eb7uosCcoGayqNchK591VPph-5_nBAMXg@mail.gmail.com>
On Fri, May 15, 2026 at 11:37:34AM +0300, Svyatoslav Ryhel wrote:
> пт, 15 трав. 2026 р. о 11:20 Krzysztof Kozlowski <krzk@kernel.org> пише:
> >
> > On Mon, May 11, 2026 at 04:57:00PM +0300, Svyatoslav Ryhel wrote:
> > > Document the XMM6260 PHY used by various devices based on the Nvidia Tegra
> > > SoC, describing its usage
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > .../bindings/phy/nvidia,tegra-xmm6260.yaml | 58 +++++++++++++++++++
> > > 1 file changed, 58 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml b/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > > new file mode 100644
> > > index 000000000000..0346433c9772
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > > @@ -0,0 +1,58 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/phy/nvidia,tegra-xmm6260.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Nvidia Tegra PHY for XMM6260 modem
> >
> > XMM6260 is Infineon modem, so any combination with nvidia,tegra is very
> > confusing.
> >
>
> May you please suggest how to adjust the name then? Thank you.
Depending what is that. Start describing hardware, not driver behavior
to help in that.
>
> > > +
> > > +description:
> > > + A hardware configuration used in Tegra SoCs to provide proper interaction
> > > + between the application processor and the modem, as well as control over
> > > + one of the SoC's USB lines for the modem.
> > > +
> > > +maintainers:
> > > + - Svyatoslav Ryhel <clamor95@gmail.com>
> > > +
> > > +properties:
> > > + compatible:
> > > + const: nvidia,tegra-xmm6260
> >
> > Also here.
> >
> > What sort of phy is this? So far looks more like a software construct.
> >
>
> Infineon XMM6260 does not work as an ordinary USB modem, it is a
> standalone CPU which just exposes itself to AP via USB. In order to do
> so, it has to have control over a USB bus of AP which is dedicated to
> it. In case of Tegra - XMM6260 interaction it looks like this: second
> Tegra USB controller is set into HSIC mode and is dedicated solely to
> the modem, modem controls this USB bus. Then the main XMM6260 driver
> performs power and init sequence and once it is ready it calls phy to
> register controller. Phy has its own supply, controls USB controller
> de/register and using enable GPIO sends signal to modem to proceed.
> Additionally, since some XMM626 versions have a few steps to setup
> exposing different USB devices, phy handles controller reinit for each
> step. If treat XMM6260 as an simple USB modem it will never init.
>
> One more benefit of having PHY is that modem driver itself is generic
> and PHY handles SoC specific configurations required by the modem.
> Since this modem was used on a variety of different SoC's (Exynos and
> OMAP for example) they can reuse modem's driver and provide only PHY
> which handles modem interactions with the USB bus.
Without any registers here, this is not a PHY but a power sequencing,
just like we do for other USB or PCI devices.
Optionally, it could be part of existing USB phy, when configuring it in
HSIC mode, but it seems you add here supplies for the modem, not actual
phy as the phy is undefined.
The problem is that in the patch and explanation you mix driver model
and driver behavior, so I really don't know what is this hardware. And
it is not my job to guess, btw. A partial argument/proof why this is not
a PHY, is that you reference the USB in the node, so phy-provider
references the phy-consumer. That's reverse. If this is PHY, it's USB's
HSIC phy, thus this needs to be referenced by USB.
If this is power sequencing, then it can be represented as USB device,
just like we do for all USB devices, but then it is not PHY and
phy-cells are not appropriate.
Best regards,
Krzysztof
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v1 5/6] dt-bindings: phy: tegra: Document Nvidia Tegra XMM6260 PHY
From: Svyatoslav Ryhel @ 2026-05-18 12:23 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Neil Armstrong, Thierry Reding, Jonathan Hunter,
Greg Kroah-Hartman, Peter Chen, netdev, devicetree, linux-kernel,
linux-phy, linux-tegra, linux-usb
In-Reply-To: <20260518-mustard-rabbit-of-ecstasy-eed3b6@quoll>
пн, 18 трав. 2026 р. о 15:14 Krzysztof Kozlowski <krzk@kernel.org> пише:
>
> On Fri, May 15, 2026 at 11:37:34AM +0300, Svyatoslav Ryhel wrote:
> > пт, 15 трав. 2026 р. о 11:20 Krzysztof Kozlowski <krzk@kernel.org> пише:
> > >
> > > On Mon, May 11, 2026 at 04:57:00PM +0300, Svyatoslav Ryhel wrote:
> > > > Document the XMM6260 PHY used by various devices based on the Nvidia Tegra
> > > > SoC, describing its usage
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > .../bindings/phy/nvidia,tegra-xmm6260.yaml | 58 +++++++++++++++++++
> > > > 1 file changed, 58 insertions(+)
> > > > create mode 100644 Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml b/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > > > new file mode 100644
> > > > index 000000000000..0346433c9772
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/phy/nvidia,tegra-xmm6260.yaml
> > > > @@ -0,0 +1,58 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/phy/nvidia,tegra-xmm6260.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Nvidia Tegra PHY for XMM6260 modem
> > >
> > > XMM6260 is Infineon modem, so any combination with nvidia,tegra is very
> > > confusing.
> > >
> >
> > May you please suggest how to adjust the name then? Thank you.
>
> Depending what is that. Start describing hardware, not driver behavior
> to help in that.
>
> >
> > > > +
> > > > +description:
> > > > + A hardware configuration used in Tegra SoCs to provide proper interaction
> > > > + between the application processor and the modem, as well as control over
> > > > + one of the SoC's USB lines for the modem.
> > > > +
> > > > +maintainers:
> > > > + - Svyatoslav Ryhel <clamor95@gmail.com>
> > > > +
> > > > +properties:
> > > > + compatible:
> > > > + const: nvidia,tegra-xmm6260
> > >
> > > Also here.
> > >
> > > What sort of phy is this? So far looks more like a software construct.
> > >
> >
> > Infineon XMM6260 does not work as an ordinary USB modem, it is a
> > standalone CPU which just exposes itself to AP via USB. In order to do
> > so, it has to have control over a USB bus of AP which is dedicated to
> > it. In case of Tegra - XMM6260 interaction it looks like this: second
> > Tegra USB controller is set into HSIC mode and is dedicated solely to
> > the modem, modem controls this USB bus. Then the main XMM6260 driver
> > performs power and init sequence and once it is ready it calls phy to
> > register controller. Phy has its own supply, controls USB controller
> > de/register and using enable GPIO sends signal to modem to proceed.
> > Additionally, since some XMM626 versions have a few steps to setup
> > exposing different USB devices, phy handles controller reinit for each
> > step. If treat XMM6260 as an simple USB modem it will never init.
> >
> > One more benefit of having PHY is that modem driver itself is generic
> > and PHY handles SoC specific configurations required by the modem.
> > Since this modem was used on a variety of different SoC's (Exynos and
> > OMAP for example) they can reuse modem's driver and provide only PHY
> > which handles modem interactions with the USB bus.
>
> Without any registers here, this is not a PHY but a power sequencing,
> just like we do for other USB or PCI devices.
>
> Optionally, it could be part of existing USB phy, when configuring it in
> HSIC mode, but it seems you add here supplies for the modem, not actual
> phy as the phy is undefined.
>
> The problem is that in the patch and explanation you mix driver model
> and driver behavior, so I really don't know what is this hardware. And
> it is not my job to guess, btw. A partial argument/proof why this is not
> a PHY, is that you reference the USB in the node, so phy-provider
> references the phy-consumer. That's reverse. If this is PHY, it's USB's
> HSIC phy, thus this needs to be referenced by USB.
>
> If this is power sequencing, then it can be represented as USB device,
> just like we do for all USB devices, but then it is not PHY and
> phy-cells are not appropriate.
>
If I integrate USB controller reference into the modem, modem will not
be generic since Exynos and OMAP don't use Chipidea USB controller
(both those SoC use this modem). Modem has to control USB controllers
init and deinit in order to operate properly. I have tried to resolve
this with PHY but if you find this inappropriate, maybe you can
suggest better suited way?
>
> Best regards,
> Krzysztof
>
--
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: Dmitry Baryshkov @ 2026-05-18 12:25 UTC (permalink / raw)
To: Konrad Dybcio
Cc: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, 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-2-71d827c49dca@oss.qualcomm.com>
On Mon, May 18, 2026 at 12:29:49PM +0200, Konrad Dybcio wrote:
> 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,
Do we need TBT2 as a separate submode?
Other than that:
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> +};
--
With best wishes
Dmitry
--
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