* Re: [PATCH] MAINTAINERS: Add Radhey Shyam Pandey as ZynqMP PHY maintainer
From: Laurent Pinchart @ 2026-06-28 21:12 UTC (permalink / raw)
To: Radhey Shyam Pandey
Cc: tomi.valkeinen, vkoul, michal.simek, linux-kernel, linux-phy,
linux-arm-kernel
In-Reply-To: <20260627162233.2803425-1-radhey.shyam.pandey@amd.com>
On Sat, Jun 27, 2026 at 09:52:33PM +0530, Radhey Shyam Pandey wrote:
> I am maintaining phy-zynqmp driver in xilinx tree and would like to
> maintain it in the mainline kernel as well. Hence adding myself as a
> maintainer.
>
> Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
Thank you for volunteering.
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
> MAINTAINERS | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1705eb823dd0..90dd86437c5c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29645,6 +29645,7 @@ F: drivers/edac/zynqmp_edac.c
>
> XILINX ZYNQMP PSGTR PHY DRIVER
> M: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> +M: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
> L: linux-kernel@vger.kernel.org
> S: Supported
> T: git https://github.com/Xilinx/linux-xlnx.git
>
> base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
--
Regards,
Laurent Pinchart
--
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: Dmitry Baryshkov @ 2026-06-28 12:33 UTC (permalink / raw)
To: Kamal Wadhwa
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: <bvankzuazswuigrm7hvyjqigzfaru5o43wi3zvyphgme2pgqwz@pyspqky3u6cn>
On Thu, Jun 25, 2026 at 09:11:19PM +0530, Kamal Wadhwa wrote:
> On Wed, Jun 17, 2026 at 03:48:14PM +0300, Dmitry Baryshkov wrote:
> > On Mon, 18 May 2026 at 14:49, Kamal Wadhwa
> > <kamal.wadhwa@oss.qualcomm.com> wrote:
> > >
> > > 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).
> > > > > > > > > --
> > > > > > > > > > +
> > > > > > > > > > + 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.
> >
> > Yes.
> >
> > >
> > > However, we want to avoid the unnecessary access to the parent from APPS.
> >
> > Why? What is the reason? Do we want to do the same for all the
> > platforms? Only for Shikra? Something else?
> >
> > > At the moment, I do not see a way to avoid that, if we add the parent
> > > regulators.
> >
> > That depend on the answer to the previous question. In the end, we can
> > make the driver ignore the parents by removing them from the regulator
> > desc.
>
> Ok, this seems like a good suggestion, so you mean its ok if we define the
> regulator desc's supply column with NULL? And only keep that in the DT?
>
> you mean like this?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/regulator/qcom-rpmh-regulator.c?h=v7.1#n1453
>
> (please let me know if i got that right. thanks)
Yes. Don't forget to explain in the commit message, why you are doing
so.
--
With best wishes
Dmitry
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH net v5 2/2] Revert "net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c"
From: Petr Wozniak @ 2026-06-27 17:32 UTC (permalink / raw)
To: linux, andrew, hkallweit1
Cc: kuba, davem, edumazet, pabeni, netdev, linux-kernel, linux-phy,
maxime.chevallier, bjorn, olek2, kabel, Petr Wozniak
In-Reply-To: <cover.1782581445.git.petr.wozniak@gmail.com>
This reverts commit 8fe125892f40 ("net: phy: sfp: probe for RollBall
I2C-to-MDIO bridge in mdio-i2c").
That commit added a RollBall bridge probe at MDIO bus creation time, in
i2c_mii_init_rollball(), to avoid a multi-minute PHY probe retry loop on
modules without a bridge (e.g. RTL8261BE). The probe runs in SFP_S_INIT,
before genuine RollBall modules have finished their firmware/bridge
initialization, so the bridge does not yet answer CMD_READ/CMD_DONE. The
probe times out, mdio_protocol is set to MDIO_I2C_NONE, and PHY detection
is then skipped for genuine RollBall modules that worked before the commit.
This was confirmed on hardware by Maxime Chevallier and Aleksander
Bajkowski: their RollBall modules no longer detect a PHY, and work again
on v7.0 (before the bridge probing was introduced). The Sashiko static
review flagged the same path.
Deferring the probe to PHY discovery time does not fix it either: at that
point a slow module may still be initializing, so the probe still returns
-ENODEV. A proper fix needs per-module init timing (a longer module_t_wait
or a per-module quirk, per SFF-8472 the host must also wait at least 300 ms
after insertion), which requires genuine RollBall hardware to develop and
validate. Revert to restore the previous, working behaviour in the meantime.
The RTL8261BE retry-loop latency that the reverted commit addressed is
handled in our downstream tree, so reverting upstream is safe on our side.
Fixes: 8fe125892f40 ("net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in mdio-i2c")
Reported-by: Aleksander Bajkowski <olek2@wp.pl>
Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://lore.kernel.org/netdev/20260624084814.20972-1-petr.wozniak@gmail.com/
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
---
drivers/net/mdio/mdio-i2c.c | 59 +++++--------------------------------
drivers/net/phy/sfp.c | 14 ++-------
2 files changed, 10 insertions(+), 63 deletions(-)
diff --git a/drivers/net/mdio/mdio-i2c.c b/drivers/net/mdio/mdio-i2c.c
index b88f63234b4e..ed20352a589a 100644
--- a/drivers/net/mdio/mdio-i2c.c
+++ b/drivers/net/mdio/mdio-i2c.c
@@ -419,50 +419,6 @@ static int i2c_mii_write_rollball(struct mii_bus *bus, int phy_id, int devad,
return 0;
}
-static int i2c_mii_probe_rollball(struct i2c_adapter *i2c)
-{
- u8 data_buf[] = { ROLLBALL_DATA_ADDR, 0x01, 0x00, 0x00 };
- u8 cmd_buf[] = { ROLLBALL_CMD_ADDR, ROLLBALL_CMD_READ };
- u8 cmd_addr = ROLLBALL_CMD_ADDR;
- struct i2c_msg msgs[2];
- u8 result;
- int ret;
- int i;
-
- msgs[0].addr = ROLLBALL_PHY_I2C_ADDR;
- msgs[0].flags = 0;
- msgs[0].len = sizeof(data_buf);
- msgs[0].buf = data_buf;
- msgs[1].addr = ROLLBALL_PHY_I2C_ADDR;
- msgs[1].flags = 0;
- msgs[1].len = sizeof(cmd_buf);
- msgs[1].buf = cmd_buf;
-
- ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs));
- if (ret < 0)
- return -ENODEV;
-
- msgs[0].addr = ROLLBALL_PHY_I2C_ADDR;
- msgs[0].flags = 0;
- msgs[0].len = 1;
- msgs[0].buf = &cmd_addr;
- msgs[1].addr = ROLLBALL_PHY_I2C_ADDR;
- msgs[1].flags = I2C_M_RD;
- msgs[1].len = 1;
- msgs[1].buf = &result;
-
- for (i = 0; i < 10; i++) {
- msleep(20);
- ret = i2c_transfer_rollball(i2c, msgs, ARRAY_SIZE(msgs));
- if (ret < 0)
- return -ENODEV;
- if (result == ROLLBALL_CMD_DONE)
- return 0;
- }
-
- return -ENODEV;
-}
-
static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
{
struct i2c_msg msg;
@@ -482,11 +438,11 @@ static int i2c_mii_init_rollball(struct i2c_adapter *i2c)
ret = i2c_transfer(i2c, &msg, 1);
if (ret < 0)
- return -ENODEV;
- if (ret != 1)
+ return ret;
+ else if (ret != 1)
return -EIO;
-
- return i2c_mii_probe_rollball(i2c);
+ else
+ return 0;
}
static bool mdio_i2c_check_functionality(struct i2c_adapter *i2c,
@@ -531,10 +487,9 @@ struct mii_bus *mdio_i2c_alloc(struct device *parent, struct i2c_adapter *i2c,
case MDIO_I2C_ROLLBALL:
ret = i2c_mii_init_rollball(i2c);
if (ret < 0) {
- if (ret != -ENODEV)
- dev_err(parent,
- "Cannot initialize RollBall MDIO I2C protocol: %d\n",
- ret);
+ dev_err(parent,
+ "Cannot initialize RollBall MDIO I2C protocol: %d\n",
+ ret);
mdiobus_free(mii);
return ERR_PTR(ret);
}
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index c4d274ab651e..f520206734da 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -597,7 +597,6 @@ static const struct sfp_quirk sfp_quirks[] = {
// OEM SFP-GE-T is a 1000Base-T module with broken TX_FAULT indicator
SFP_QUIRK_F("OEM", "SFP-GE-T", sfp_fixup_ignore_tx_fault),
- SFP_QUIRK_F("OEM", "SFP-10G-T-I", sfp_fixup_rollball),
SFP_QUIRK_F("OEM", "SFP-10G-T", sfp_fixup_rollball_cc),
SFP_QUIRK_S("OEM", "SFP-2.5G-T", sfp_quirk_oem_2_5g),
SFP_QUIRK_S("OEM", "SFP-2.5G-BX10-D", sfp_quirk_2500basex),
@@ -2174,17 +2173,10 @@ static void sfp_sm_fault(struct sfp *sfp, unsigned int next_state, bool warn)
static int sfp_sm_add_mdio_bus(struct sfp *sfp)
{
- int ret;
-
- if (sfp->mdio_protocol == MDIO_I2C_NONE)
- return 0;
+ if (sfp->mdio_protocol != MDIO_I2C_NONE)
+ return sfp_i2c_mdiobus_create(sfp);
- ret = sfp_i2c_mdiobus_create(sfp);
- if (ret == -ENODEV) {
- sfp->mdio_protocol = MDIO_I2C_NONE;
- return 0;
- }
- return ret;
+ return 0;
}
/* Probe a SFP for a PHY device if the module supports copper - the PHY
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH net v5 1/2] net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy
From: Petr Wozniak @ 2026-06-27 17:32 UTC (permalink / raw)
To: linux, andrew, hkallweit1
Cc: kuba, davem, edumazet, pabeni, netdev, linux-kernel, linux-phy,
maxime.chevallier, bjorn, olek2, kabel, Petr Wozniak,
Larysa Zaremba
In-Reply-To: <cover.1782581445.git.petr.wozniak@gmail.com>
sfp_i2c_mdiobus_create() allocates the I2C MDIO bus with mdio_i2c_alloc(),
a plain (non-devm) allocation, and registers it. sfp_i2c_mdiobus_destroy()
only unregisters the bus and clears sfp->i2c_mii without calling
mdiobus_free(). As the only reference to the bus is then cleared, the
struct mii_bus is leaked.
This is hit whenever a copper/RollBall SFP module that instantiated an MDIO
bus is removed: sfp_sm_main() takes the global teardown path and calls
sfp_i2c_mdiobus_destroy(). sfp_cleanup(), on driver unbind, frees
sfp->i2c_mii directly, which is why the leak only triggered on module
hot-removal and not on unbind.
Free the bus in sfp_i2c_mdiobus_destroy() to match the allocation done in
sfp_i2c_mdiobus_create().
Fixes: e85b1347ace6 ("net: sfp: create/destroy I2C mdiobus before PHY probe/after PHY release")
Signed-off-by: Petr Wozniak <petr.wozniak@gmail.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Larysa Zaremba <larysa.zaremba@intel.com>
---
drivers/net/phy/sfp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/sfp.c b/drivers/net/phy/sfp.c
index 03bfd8640db9..c4d274ab651e 100644
--- a/drivers/net/phy/sfp.c
+++ b/drivers/net/phy/sfp.c
@@ -963,6 +963,7 @@ static int sfp_i2c_mdiobus_create(struct sfp *sfp)
static void sfp_i2c_mdiobus_destroy(struct sfp *sfp)
{
mdiobus_unregister(sfp->i2c_mii);
+ mdiobus_free(sfp->i2c_mii);
sfp->i2c_mii = NULL;
}
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [PATCH net v5 0/2] net: phy: sfp: fix mii_bus leak and revert RollBall bridge probe
From: Petr Wozniak @ 2026-06-27 17:32 UTC (permalink / raw)
To: linux, andrew, hkallweit1
Cc: kuba, davem, edumazet, pabeni, netdev, linux-kernel, linux-phy,
maxime.chevallier, bjorn, olek2, kabel, Petr Wozniak
v4 tried to fix the RollBall regression from 8fe125892f40 by deferring the
bridge probe to PHY discovery time. Maxime Chevallier and Aleksander
Bajkowski both tested that on genuine RollBall hardware and confirmed it
does not restore PHY detection (the module still is not ready when the
probe runs), and the Sashiko static review flagged the same path.
So this version drops the deferred-probe patch and instead reverts
8fe125892f40, restoring the pre-regression behaviour for genuine RollBall
modules. A proper fix for slow-initializing modules needs per-module init
timing (a longer module_t_wait / a per-module quirk) and genuine RollBall
hardware to validate; that is better owned as a follow-up by someone with
such a module.
Patch 1 is the independent mii_bus leak fix, unchanged, now carrying
Reviewed-by from Maxime and Larysa.
Patch 2 reverts 8fe125892f40.
v4: https://lore.kernel.org/netdev/20260624084814.20972-1-petr.wozniak@gmail.com/
Petr Wozniak (2):
net: phy: sfp: free mii_bus in sfp_i2c_mdiobus_destroy
Revert "net: phy: sfp: probe for RollBall I2C-to-MDIO bridge in
mdio-i2c"
drivers/net/mdio/mdio-i2c.c | 59 +++++--------------------------------
drivers/net/phy/sfp.c | 15 +++-------
2 files changed, 11 insertions(+), 63 deletions(-)
--
2.51.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [PATCH] MAINTAINERS: Add Radhey Shyam Pandey as ZynqMP PHY maintainer
From: Radhey Shyam Pandey @ 2026-06-27 16:22 UTC (permalink / raw)
To: laurent.pinchart, tomi.valkeinen, vkoul, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, Radhey Shyam Pandey
I am maintaining phy-zynqmp driver in xilinx tree and would like to
maintain it in the mainline kernel as well. Hence adding myself as a
maintainer.
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
MAINTAINERS | 1 +
1 file changed, 1 insertion(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 1705eb823dd0..90dd86437c5c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29645,6 +29645,7 @@ F: drivers/edac/zynqmp_edac.c
XILINX ZYNQMP PSGTR PHY DRIVER
M: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
+M: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
L: linux-kernel@vger.kernel.org
S: Supported
T: git https://github.com/Xilinx/linux-xlnx.git
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [RESEND PATCH 1/3] phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask
From: sashiko-bot @ 2026-06-27 16:02 UTC (permalink / raw)
To: Radhey Shyam Pandey; +Cc: vkoul, neil.armstrong, linux-phy, olteanv
In-Reply-To: <20260627155229.2791113-2-radhey.shyam.pandey@amd.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The return value of clk_prepare_enable() is not captured, leading to a masked error and potential hardware faults.
- [Medium] Missing clk_disable_unprepare() on error paths causes a clock reference count leak.
--
commit b562ec219de1a77ec468ae7c48ab00117de6c7d1
Author: Nava kishore Manne <nava.kishore.manne@amd.com>
phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask
This patch fixes the L0_TM_DISABLE_SCRAMBLE_ENCODER mask in the ZynqMP
PHY driver. The previous mask incorrectly included a reserved read-only
field, which has now been excluded.
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index fe6b4925d1662..c037d7c13d485 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -53,7 +53,7 @@
> #define L0_TM_DIG_6 0x106c
> #define L0_TM_DIS_DESCRAMBLE_DECODER 0x0f
> #define L0_TX_DIG_61 0x00f4
> -#define L0_TM_DISABLE_SCRAMBLE_ENCODER 0x0f
> +#define L0_TM_DISABLE_SCRAMBLE_ENCODER (BIT(3) | GENMASK(1, 0))
>
> /* PLL Test Mode register parameters */
> #define L0_TM_PLL_DIG_37 0x2094
[Severity: High]
This is a pre-existing issue, but does xpsgtr_phy_init() improperly mask
errors if the clock fails to enable?
drivers/phy/xilinx/phy-zynqmp.c:xpsgtr_phy_init() {
...
/* Configure and enable the clock when peripheral phy_init call */
if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
goto out;
...
out:
mutex_unlock(>r_dev->gtr_mutex);
return ret;
}
Since ret is initialized to 0 and the error from clk_prepare_enable() isn't
assigned to it, this function will return 0 on failure.
This could cause the PHY framework to mistakenly believe the PHY is
initialized, potentially leading to unclocked register accesses and system
hangs.
[Severity: Medium]
This is also a pre-existing issue, but could this result in a clock
reference count leak on error paths?
drivers/phy/xilinx/phy-zynqmp.c:xpsgtr_phy_init() {
...
if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
goto out;
/* Skip initialization if not required. */
if (!xpsgtr_phy_init_required(gtr_phy))
goto out;
if (gtr_dev->tx_term_fix) {
ret = xpsgtr_phy_tx_term_fix(gtr_phy);
if (ret < 0)
goto out;
...
If a subsequent step like xpsgtr_phy_tx_term_fix() or xpsgtr_configure_pll()
fails, the function jumps to out and returns an error without calling
clk_disable_unprepare().
Because the initialization failed, phy_exit() will not be called by the
framework, leaving the clock prepared and enabled indefinitely.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627155229.2791113-1-radhey.shyam.pandey@amd.com?part=1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [RESEND PATCH 2/3] phy: zynqmp: use read-modify-write for SERDES scrambler bypass
From: sashiko-bot @ 2026-06-27 16:02 UTC (permalink / raw)
To: Radhey Shyam Pandey; +Cc: neil.armstrong, vkoul, olteanv, linux-phy
In-Reply-To: <20260627155229.2791113-3-radhey.shyam.pandey@amd.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The error code from clk_prepare_enable() is not propagated, causing a false success return.
- [High] Missing clk_disable_unprepare() on error paths causes a clock resource leak.
--
commit 5cbba5539e34550743b9636376f1a41a65d04bb8
Author: Nava kishore Manne <nava.kishore.manne@amd.com>
phy: zynqmp: use read-modify-write for SERDES scrambler bypass
This commit switches xpsgtr_bypass_scrambler_8b10b() to use
xpsgtr_clr_set_phy() instead of xpsgtr_write_phy(). This modifies only the
bypass bits while preserving the rest of the register fields.
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index c037d7c13d485..6c56c4df8523c 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
> @@ -505,8 +505,12 @@ static void xpsgtr_lane_set_protocol(struct xpsgtr_phy *gtr_phy)
> /* Bypass (de)scrambler and 8b/10b decoder and encoder. */
> static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy)
> {
[ ... ]
> static int xpsgtr_phy_init(struct phy *phy)
> {
> struct xpsgtr_phy *gtr_phy = phy_get_drvdata(phy);
> struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
> int ret = 0;
>
> mutex_lock(>r_dev->gtr_mutex);
>
> /* Configure and enable the clock when peripheral phy_init call */
> if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
> goto out;
[Severity: High]
This is a pre-existing issue, but does this error path fail to set the
return code?
When clk_prepare_enable() fails, it jumps to 'out' while 'ret'
is still 0, leading to a false success return. This can cause the PHY
framework to assume the PHY is initialized.
>
> /* Skip initialization if not required. */
> if (!xpsgtr_phy_init_required(gtr_phy))
> goto out;
>
> if (gtr_dev->tx_term_fix) {
> ret = xpsgtr_phy_tx_term_fix(gtr_phy);
> if (ret < 0)
> goto out;
[Severity: High]
This is a pre-existing issue, but does this error path jump to 'out' without
disabling the previously enabled clock?
Because 'ret' is negative here, the PHY framework considers the
initialization failed and won't call xpsgtr_phy_exit(). This results in
a permanent clock resource leak.
>
> gtr_dev->tx_term_fix = false;
> }
>
> /* Enable coarse code saturation limiting logic. */
> xpsgtr_write_phy(gtr_phy, L0_TM_PLL_DIG_37, L0_TM_COARSE_CODE_LIMIT);
>
> /*
> * Configure the PLL, the lane protocol, and perform protocol-specific
> * initialization.
> */
> ret = xpsgtr_configure_pll(gtr_phy);
> if (ret)
> goto out;
[Severity: High]
This is a pre-existing issue, but does this error path also leak the clock?
Like the tx_term_fix failure above, this jumps to 'out' without calling
clk_disable_unprepare() on the clock enabled at the start of the function.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627155229.2791113-1-radhey.shyam.pandey@amd.com?part=2
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [RESEND PATCH 3/3] phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB
From: sashiko-bot @ 2026-06-27 16:02 UTC (permalink / raw)
To: Radhey Shyam Pandey; +Cc: linux-phy, vkoul, neil.armstrong, olteanv
In-Reply-To: <20260627155229.2791113-4-radhey.shyam.pandey@amd.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] PCIe and DisplayPort (DP) protocols lack explicit initialization of the scrambler/encoder bypass state, leaving them vulnerable to state pollution.
- [High] Silently ignores clock enable failure, masking the error and causing unbalanced clock disables later.
- [High] Bus width registers modified by SGMII are not reset by other protocols, risking state inheritance bugs.
--
commit e6d5f06ce31b70c624ec3e6a69c5b7e7ef55746a
Author: Nava kishore Manne <nava.kishore.manne@amd.com>
Subject: phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB
This commit ensures that the scrambler and 8b/10b encoder/decoder are not
bypassed for USB operation, as mandated by the USB 3.x specification. The
bypass functionality is restricted to SATA and SGMII modes where encoding is
handled in the controller.
Cc: stable@vger.kernel.org
> diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
> index 6c56c4df8523c..087fe402e4e20 100644
> --- a/drivers/phy/xilinx/phy-zynqmp.c
> +++ b/drivers/phy/xilinx/phy-zynqmp.c
[ ... ]
> @@ -547,7 +562,7 @@ static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
> xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, mask, val);
> xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, mask, val);
[Severity: High]
This isn't a bug introduced by this patch, but SGMII alters the global bus
width registers here without other protocols guaranteeing they are restored to
default states.
If a lane previously used for SGMII is reassigned to another protocol (e.g.,
via warm reboot or bootloader handoff), will the 10-bit bus width setting
persist in the hardware and silently break other protocols (like USB or PCIe)
that expect their default bus widths?
>
> - xpsgtr_bypass_scrambler_8b10b(gtr_phy);
> + xpsgtr_bypass_scrambler_8b10b(gtr_phy, true);
> }
[ ... ]
> @@ -707,6 +722,10 @@ static int xpsgtr_phy_init(struct phy *phy)
> case ICM_PROTOCOL_SGMII:
> xpsgtr_phy_init_sgmii(gtr_phy);
> break;
> +
> + case ICM_PROTOCOL_USB:
> + xpsgtr_bypass_scrambler_8b10b(gtr_phy, false);
> + break;
[Severity: High]
This isn't a bug introduced by this patch, but the switch statement in
xpsgtr_phy_init() misses explicit bypass configuration for DP and PCIe.
If a lane is dynamically reassigned or left bypassed by previous boot stages,
will DP and PCIe silently fail as they inherit a stale bypass = true state?
> }
>
> out:
[Severity: High]
This isn't a bug introduced by this patch, but looking at xpsgtr_phy_init(),
could the clock error path cause issues here?
drivers/phy/xilinx/phy-zynqmp.c:xpsgtr_phy_init() {
int ret = 0;
...
if (clk_prepare_enable(gtr_dev->clk[gtr_phy->refclk]))
goto out;
...
out:
mutex_unlock(>r_dev->gtr_mutex);
return ret;
}
If clk_prepare_enable() fails, we branch to 'out', but 'ret' remains 0. Will
this silently ignore the clock enable failure, mask the error, and cause
unbalanced clock disables later when xpsgtr_phy_exit() calls
clk_disable_unprepare() on a clock that was never successfully enabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260627155229.2791113-1-radhey.shyam.pandey@amd.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [RESEND PATCH 2/3] phy: zynqmp: use read-modify-write for SERDES scrambler bypass
From: Radhey Shyam Pandey @ 2026-06-27 15:52 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, Nava kishore Manne,
stable, Radhey Shyam Pandey
In-Reply-To: <20260627155229.2791113-1-radhey.shyam.pandey@amd.com>
From: Nava kishore Manne <nava.kishore.manne@amd.com>
xpsgtr_bypass_scrambler_8b10b() used xpsgtr_write_phy() which performs
a full register write, silently clearing any bits beyond the intended
bypass control fields.
Switch to xpsgtr_clr_set_phy() with clr=mask, set=mask to set only
the bypass bits while preserving the remaining bits in each register.
Fixes: 4a33bea00314 ("phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver")
Cc: stable@vger.kernel.org
Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
drivers/phy/xilinx/phy-zynqmp.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index c037d7c13d48..6c56c4df8523 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -505,8 +505,12 @@ static void xpsgtr_lane_set_protocol(struct xpsgtr_phy *gtr_phy)
/* Bypass (de)scrambler and 8b/10b decoder and encoder. */
static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy)
{
- xpsgtr_write_phy(gtr_phy, L0_TM_DIG_6, L0_TM_DIS_DESCRAMBLE_DECODER);
- xpsgtr_write_phy(gtr_phy, L0_TX_DIG_61, L0_TM_DISABLE_SCRAMBLE_ENCODER);
+ xpsgtr_clr_set_phy(gtr_phy, L0_TM_DIG_6,
+ L0_TM_DIS_DESCRAMBLE_DECODER,
+ L0_TM_DIS_DESCRAMBLE_DECODER);
+ xpsgtr_clr_set_phy(gtr_phy, L0_TX_DIG_61,
+ L0_TM_DISABLE_SCRAMBLE_ENCODER,
+ L0_TM_DISABLE_SCRAMBLE_ENCODER);
}
/* DP-specific initialization. */
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [RESEND PATCH 3/3] phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB
From: Radhey Shyam Pandey @ 2026-06-27 15:52 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, Nava kishore Manne,
stable, Radhey Shyam Pandey
In-Reply-To: <20260627155229.2791113-1-radhey.shyam.pandey@amd.com>
From: Nava kishore Manne <nava.kishore.manne@amd.com>
USB Gen1 requires scrambling and 8b/10b encoding to be performed in the
physical layer. Do not bypass PHY-side scrambler or encoder/decoder for
USB operation, as mandated by the USB 3.x specification.
Scrambler and 8b/10b bypass remain restricted to SATA and SGMII
modes, where encoding is handled in the controller.
Fixes: 4a33bea00314 ("phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver")
Cc: stable@vger.kernel.org
Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
drivers/phy/xilinx/phy-zynqmp.c | 39 ++++++++++++++++++++++++---------
1 file changed, 29 insertions(+), 10 deletions(-)
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index 6c56c4df8523..087fe402e4e2 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -502,15 +502,30 @@ static void xpsgtr_lane_set_protocol(struct xpsgtr_phy *gtr_phy)
}
}
-/* Bypass (de)scrambler and 8b/10b decoder and encoder. */
-static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy)
+/**
+ * xpsgtr_bypass_scrambler_8b10b - Configure scrambler/encoder behavior
+ * @gtr_phy: pointer to lane context
+ * @bypass: true to enable scrambler/encoder bypass (SATA/SGMII),
+ * false to disable scrambler/encoder bypass (USB3)
+ *
+ * Uses RMW to preserve reserved and unrelated register fields.
+ */
+static void xpsgtr_bypass_scrambler_8b10b(struct xpsgtr_phy *gtr_phy,
+ bool bypass)
{
- xpsgtr_clr_set_phy(gtr_phy, L0_TM_DIG_6,
- L0_TM_DIS_DESCRAMBLE_DECODER,
- L0_TM_DIS_DESCRAMBLE_DECODER);
- xpsgtr_clr_set_phy(gtr_phy, L0_TX_DIG_61,
- L0_TM_DISABLE_SCRAMBLE_ENCODER,
- L0_TM_DISABLE_SCRAMBLE_ENCODER);
+ if (bypass) {
+ xpsgtr_clr_set_phy(gtr_phy, L0_TM_DIG_6,
+ L0_TM_DIS_DESCRAMBLE_DECODER,
+ L0_TM_DIS_DESCRAMBLE_DECODER);
+ xpsgtr_clr_set_phy(gtr_phy, L0_TX_DIG_61,
+ L0_TM_DISABLE_SCRAMBLE_ENCODER,
+ L0_TM_DISABLE_SCRAMBLE_ENCODER);
+ } else {
+ xpsgtr_clr_set_phy(gtr_phy, L0_TM_DIG_6,
+ L0_TM_DIS_DESCRAMBLE_DECODER, 0);
+ xpsgtr_clr_set_phy(gtr_phy, L0_TX_DIG_61,
+ L0_TM_DISABLE_SCRAMBLE_ENCODER, 0);
+ }
}
/* DP-specific initialization. */
@@ -531,7 +546,7 @@ static void xpsgtr_phy_init_sata(struct xpsgtr_phy *gtr_phy)
{
struct xpsgtr_dev *gtr_dev = gtr_phy->dev;
- xpsgtr_bypass_scrambler_8b10b(gtr_phy);
+ xpsgtr_bypass_scrambler_8b10b(gtr_phy, true);
writel(gtr_phy->lane, gtr_dev->siou + SATA_CONTROL_OFFSET);
}
@@ -547,7 +562,7 @@ static void xpsgtr_phy_init_sgmii(struct xpsgtr_phy *gtr_phy)
xpsgtr_clr_set(gtr_dev, TX_PROT_BUS_WIDTH, mask, val);
xpsgtr_clr_set(gtr_dev, RX_PROT_BUS_WIDTH, mask, val);
- xpsgtr_bypass_scrambler_8b10b(gtr_phy);
+ xpsgtr_bypass_scrambler_8b10b(gtr_phy, true);
}
/* Configure TX de-emphasis and margining for DP. */
@@ -707,6 +722,10 @@ static int xpsgtr_phy_init(struct phy *phy)
case ICM_PROTOCOL_SGMII:
xpsgtr_phy_init_sgmii(gtr_phy);
break;
+
+ case ICM_PROTOCOL_USB:
+ xpsgtr_bypass_scrambler_8b10b(gtr_phy, false);
+ break;
}
out:
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* [RESEND PATCH 0/3] phy: zynqmp: fix SERDES scrambler register handling and enable for USB
From: Radhey Shyam Pandey @ 2026-06-27 15:52 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, Radhey Shyam Pandey
This series fixes three related issues in the ZynqMP SERDES PHY
scrambler/encoder bypass path:
1. The L0_TM_DISABLE_SCRAMBLE_ENCODER mask incorrectly included bit 2
of L0_TX_DIG_61, which is a reserved read-only field. Correct the
mask to (BIT(3) | GENMASK(1, 0)).
2. xpsgtr_bypass_scrambler_8b10b() used xpsgtr_write_phy() which
performs a full register write, clobbering unrelated bits. Switch
to xpsgtr_clr_set_phy() with clr=mask, set=mask to preserve other
register fields.
3. USB Gen1 requires PHY-side scrambling and 8b/10b encoding as
mandated by the USB 3.x specification. The driver was incorrectly
bypassing these for USB, the same as SATA and SGMII where encoding
is handled in the controller.
Issues reported by sashiko[1] are pre-existing issues and will
taken as separate followup series.
[1]:
https://lore.kernel.org/all/20260512213513.852EEC2BCB0@smtp.kernel.org
https://lore.kernel.org/all/20260512223142.1AFC4C2BCB0@smtp.kernel.org
Nava kishore Manne (3):
phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask
phy: zynqmp: use read-modify-write for SERDES scrambler bypass
phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB
drivers/phy/xilinx/phy-zynqmp.c | 37 ++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
base-commit: 3d5670d672ae08b8c534b7beed6f57c8b44e7b43
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* [RESEND PATCH 1/3] phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask
From: Radhey Shyam Pandey @ 2026-06-27 15:52 UTC (permalink / raw)
To: tomi.valkeinen, vkoul, michal.simek
Cc: linux-kernel, linux-phy, linux-arm-kernel, Nava kishore Manne,
stable, Radhey Shyam Pandey
In-Reply-To: <20260627155229.2791113-1-radhey.shyam.pandey@amd.com>
From: Nava kishore Manne <nava.kishore.manne@amd.com>
The L0_TX_DIG_61 register bit 2 is a reserved read-only field.
The previous mask value 0x0f incorrectly included bit 2, causing
unintended writes to a reserved bit on every scrambler bypass
operation.
Correct the mask to (BIT(3) | GENMASK(1, 0)) to cover only the
valid scramble bypass control bits.
Fixes: 4a33bea00314 ("phy: zynqmp: Add PHY driver for the Xilinx ZynqMP Gigabit Transceiver")
Cc: stable@vger.kernel.org
Signed-off-by: Nava kishore Manne <nava.kishore.manne@amd.com>
Signed-off-by: Radhey Shyam Pandey <radhey.shyam.pandey@amd.com>
---
drivers/phy/xilinx/phy-zynqmp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/xilinx/phy-zynqmp.c b/drivers/phy/xilinx/phy-zynqmp.c
index fe6b4925d166..c037d7c13d48 100644
--- a/drivers/phy/xilinx/phy-zynqmp.c
+++ b/drivers/phy/xilinx/phy-zynqmp.c
@@ -53,7 +53,7 @@
#define L0_TM_DIG_6 0x106c
#define L0_TM_DIS_DESCRAMBLE_DECODER 0x0f
#define L0_TX_DIG_61 0x00f4
-#define L0_TM_DISABLE_SCRAMBLE_ENCODER 0x0f
+#define L0_TM_DISABLE_SCRAMBLE_ENCODER (BIT(3) | GENMASK(1, 0))
/* PLL Test Mode register parameters */
#define L0_TM_PLL_DIG_37 0x2094
--
2.43.0
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related
* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Maxime Chevallier @ 2026-06-27 12:03 UTC (permalink / raw)
To: Petr Wozniak, olek2, linux, andrew, hkallweit1
Cc: kuba, davem, edumazet, pabeni, netdev, linux-kernel, linux-phy,
bjorn, kabel
In-Reply-To: <20260626163519.10678-1-petr.wozniak@gmail.com>
Hi Petr
On 6/26/26 18:35, Petr Wozniak wrote:
> Maxime Chevallier wrote:
>> I finally got time to test this with a RollBall module, and I
>> confirm what Aleksander says, the RollBall module's PHY doesn't
>> get detected even with this patch. It does work on v7.0 though,
>> so before the bridge probing was introduced.
>
> Thanks a lot for taking the time to test, Maxime - and Aleksander
> for the original report.
>
> That settles it: the deferred-probe approach in patch 2/2 doesn't
> actually restore genuine RollBall PHY detection, and as you both
> confirm it worked before 8fe125892f40 introduced the bridge probing.
> Sashiko's static review flagged the same thing (the probe bypasses the
> PHY discovery retry loop for slow-initializing modules), so the static
> analysis and the two hardware reports all point at the same flaw.
>
> I only have RTL8261BE-based copper modules here, not a genuine RollBall
> one, so I can't develop and verify a proper slow-init timing fix
> (module_t_wait / a retry that waits for the bridge) without the
> hardware to test against.
>
> Given that, my suggestion:
>
> - Please drop patch 2/2 from the series.
>
> - Since 8fe125892f40 regressed genuine RollBall detection and the
> deferred probe doesn't restore it, I think the cleanest fix is to
> revert 8fe125892f40. I'm happy to send that revert if you'd prefer.
> The 5-minute RTL8261BE probe loop it was addressing is handled in our
> downstream tree, so reverting it upstream is fine on our side.
>
> - Patch 1/2 (the mii_bus leak fix) is independent of all this and
> already has Reviewed-by from Maxime and Larysa - it would be good to
> take that one regardless. I can resend it standalone if that's easier.
>
> A proper fix covering slow-firmware modules really needs a genuine
> RollBall module to validate, so it's better owned by someone who has
> that hardware - happy to help review.
>
I only have a single rollball module here, and none with the missing
bridge, so I can't test the full thing either.
In that case, I agree with reverting 8fe125892f40 as this is breaking
all rollball modules right now.
Can you send the revert ?
Maxime
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH net v4 2/2] net: phy: mdio-i2c: defer RollBall bridge probe to PHY discovery
From: Aleksander Jan Bajkowski @ 2026-06-27 10:51 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Petr Wozniak, Russell King, Andrew Lunn, Heiner Kallweit,
David S . Miller, Eric Dumazet, Paolo Abeni, netdev, linux-kernel,
linux-phy, Maxime Chevallier, Bjorn Mork, Marek Behun
In-Reply-To: <20260625082335.3d13d875@kernel.org>
Hi Jakub,
On 25/06/2026 17:23, Jakub Kicinski wrote:
> On Wed, 24 Jun 2026 23:44:19 +0200 Aleksander Jan Bajkowski wrote:
>>> For genuine RollBall modules (e.g. FLYPRO SFP-10GT-CS-30M with Aquantia
>>> AQR113C) the probe now runs after initialization is complete and
>>> correctly returns 0, so PHY detection proceeds normally.
>> The FLPRO SFP module still fails to detect the PHY. It is necessary to
>> increase `module_t_wait` to 20 seconds. Most likely, during this time
>> the module loads the PHY firmware from SPI memory or from the
>> microcontroller (rollball bridge) via MDIO. Same probably applies to
>> most SFP modules with a PHY that load firmware at start-up (AQR113,
>> RTL8261C etc.).
> Just to clarify is FLPRO a typo or a knock off ?
Typo on my comment FLPRO->FLYPRO :)
> Do you want something to be changed here or you're just flagging that
> more follow ups are needed if we want to cover more modules?
I don’t know how this should be fixed. I’m just sharing information.
Even with this patch, most Rollball modules still don’t work. This
patch at least fixes one bug. According to SFF-8472[1], we shouldn’t
communicate with the SFP module until 300ms have elapsed since the
module was inserted. The second problem still remains. 300ms is still
not enough for most Rollball modules. Unfortunately, we’d need to test
a quirk for each module to find out what delay is required. We need a
quirk similar to sfp_fixup_rollball_wait4s. But with a longer delay.
Should the fix be included in this series or follow up?
1. SFF-8472 Rev 12.5a. Table 8-7
Best regards,
Aleksander
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v2] phy: Add USB3 PHY support to Google Tensor SoC USB PHY driver
From: RD Babiera @ 2026-06-26 21:41 UTC (permalink / raw)
To: sashiko-reviews
Cc: linux-phy, neil.armstrong, olteanv, vkoul, Badhri Jagan Sridharan
In-Reply-To: <20260622180542.4108F1F000E9@smtp.kernel.org>
On Mon, Jun 22, 2026 at 11:05 AM <sashiko-bot@kernel.org> wrote:
> [Severity: High]
> Will this error path leak the reference clock request?
>
> Since set_pmgt_ref_clk_req_n(gphy, true) was called earlier, if
> clk_bulk_prepare_enable() fails we return without deasserting the clock
> request.
>
> The driver core doesn't invoke phy_exit() if phy_init() fails, so the
> request will remain permanently asserted.
>
> [Severity: High]
> Does this error path also leak the reference clock request?
>
> It properly unwinds the clocks and resets, but it misses calling
> set_pmgt_ref_clk_req_n(gphy, false) before returning.
I addressed this in V1 where a phy_init failure would likely result
in the hardware powering off and on again and the request would
clear, but I'll change it for the next version for correctness.
> [Severity: High]
> If program_tca_locked() fails, will hardware programming retries fail
> because the software state was prematurely updated?
>
> If the USB subsystem retries powering on the PHY after an error, the early
> success check at the top of google_usb3_phy_power_on() will hit and return 0
> without actually re-attempting the hardware configuration.
I'll change this so that COMBO_PHY_TCA_READY is set when the initial
PoR => NC check passes, but subsequent power_on calls skip waiting
for the PoR => NC request and allow the program_tca_locked() call
to be made while the internal PHY state is COMBO_PHY_TCA_READY.
> [Severity: High]
> Is the missing .power_off callback going to cause the state machine to
> desync on suspend and resume?
>
> During suspend, without a power off hook, gphy->phy_state remains
> COMBO_PHY_TCA_READY. On resume, google_usb3_phy_power_on() checks this
> state and returns 0 without reprogramming the TCA hardware, which could
> permanently break the link.
The change above should cover this case.
> [Severity: Critical]
> Will unconditionally requesting the usb3 clocks break backwards compatibility
> on systems using older device trees?
>
> If an older device tree only provides USB2 resources, devm_clk_bulk_get()
> will fail here and abort the entire probe sequence.
>
> [Severity: Critical]
> Similarly, will unconditionally requiring the usb3_tca resource break probe
> for older device trees that only define USB2 resources?
>
> If the memory region is missing from the DT, this will return an error and
> prevent the USB2 PHY from working on those devices.
The PHY IP supports the USB3 resources by default, so it makes sense
to bake in the requirements.
Best,
RD
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 29/29] phy: rockchip: usbdp: Disable USB3 on probe
From: sashiko-bot @ 2026-06-26 21:24 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, linux-phy, vkoul, robh, neil.armstrong, olteanv,
conor+dt
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-29-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but missing USB3 port disable in the power-off path recreates the U-Boot failure state on subsequent initialization, which will now permanently fail due to the removal of the -EPROBE_DEFER retry mechanism.
--
Note: The format of this report has been altered to use a free-form summary
due to strict anti-recitation policies preventing direct quoting of the patch diff.
commit 8c774704e1cc3fa0c1dd6e73117e4641a08efb29
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
Subject: phy: rockchip: usbdp: Disable USB3 on probe
This commit disables USB3 support during probe to start with a clean state,
and removes the -EPROBE_DEFER retry mechanism for cmn ana lcpll lock timeouts
since they should only happen on the first try if U-Boot had enabled USB.
[Severity: High]
This isn't a bug introduced by this patch, but removing the -EPROBE_DEFER
retry mechanism makes a pre-existing power-off issue permanently fatal.
In drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_power_off(), when the
PHY transitions to UDPHY_MODE_NONE, the code disables the PHY but does not
disable the USB3 port in the GRF:
rk_udphy_power_off()
udphy->status &= ~mode;
if (udphy->status == UDPHY_MODE_NONE)
rk_udphy_disable(udphy);
When the USB subsystem resumes from suspend or driver unbind/bind, it calls
rk_udphy_power_on(). Since udphy->status is UDPHY_MODE_NONE, it calls
rk_udphy_setup(), which executes the PLL lock:
rk_udphy_power_on()
if (udphy->status == UDPHY_MODE_NONE) {
ret = rk_udphy_setup(udphy);
Because the USB3 port was never disabled in the GRF during power-off, this
recreates the exact unclean state that caused the U-Boot lock timeout. Without
the -EPROBE_DEFER retry mechanism removed by this patch, the PLL lock will
time out and result in a permanent failure to resume.
Should rk_udphy_power_off() also disable the USB3 port by calling
rk_udphy_u3_port_disable(udphy, true) when the status reaches UDPHY_MODE_NONE,
so the next power-on starts with a clean state?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=29
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 28/29] phy: rockchip: usbdp: Avoid xHCI SErrors
From: sashiko-bot @ 2026-06-26 21:24 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, conor+dt, devicetree, robh, linux-phy, olteanv,
vkoul
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-28-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unconditionally re-enabling the USB3 port during PHY re-initialization overrides the High-Speed only (hs) configuration, potentially leading to an SError on subsequent PHY power off.
--
commit 9d1b08c311cabde1792e9f8ef67ab6b7a5f76005
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Avoid xHCI SErrors
This patch avoids xHCI SErrors by disabling the USB3 port before
re-initializing it when switching between USB3-only, USB3 + DP and DP-only
modes.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index dc166392ba199..7c8b9eaaf3526 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1033,8 +1033,8 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
[Severity: High]
This is a pre-existing issue, but does this unconditionally re-enable the
USB3 port and override the High-Speed only (hs) configuration?
If the system is configured with a high-speed only USB device, the PHY
initialization via rk_udphy_usb3_phy_init() correctly disables the USB3 port
to safely route the xHCI PIPE clock to the UTMI fallback.
When a DisplayPort cable is plugged in, rk_udphy_power_on() is called. Since
udphy->mode still contains UDPHY_MODE_USB for combo modes, this code
switches the clock source back to the PHY, overriding the disablement.
When the DP cable is disconnected, rk_udphy_dp_phy_power_off() drops the
status to UDPHY_MODE_NONE and powers off the PHY. At this point, the active
xHCI controller runs without a clock, causing an immediate SError on its
next register access.
Should the re-enable conditions here, and similarly after the PHY
re-initialization further down in this function, check if high-speed only
is disabled?
For example:
if ((udphy->mode & UDPHY_MODE_USB) && !udphy->hs)
> udphy->phy_needs_reinit = false;
> } else if (udphy->phy_needs_reinit) {
> - if (udphy->mode == UDPHY_MODE_DP)
> - rk_udphy_u3_port_disable(udphy, true);
> + rk_udphy_u3_port_disable(udphy, true);
> + udelay(10);
>
> ret = rk_udphy_init(udphy);
> if (ret)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=28
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 12/29] phy: rockchip: usbdp: Support single-lane DP
From: sashiko-bot @ 2026-06-26 21:19 UTC (permalink / raw)
To: Sebastian Reichel
Cc: neil.armstrong, vkoul, robh, conor+dt, devicetree, olteanv,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-12-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Unclocked PHY register access caused by returning 0 on unsupported mode in `rk_udphy_power_on`.
--
commit 835e45901918a9b24aa9d7ccd7b8538db264146b
Author: Zhang Yubing <yubing.zhang@rock-chips.com>
phy: rockchip: usbdp: Support single-lane DP
Implement support for using just a single DisplayPort line.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1083,18 +1072,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
> static int rk_udphy_dp_phy_power_on(struct phy *phy)
> {
[ ... ]
> ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
> if (ret)
> goto unlock;
>
> - rk_udphy_dplane_enable(udphy, dp_lanes);
> + rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
>
> rk_udphy_dplane_select(udphy);
[Severity: High]
This is a pre-existing issue, but I noticed a potential problem in how
we handle the return value of rk_udphy_power_on() here.
If the DP controller powers on the DP PHY when DP mode is not currently
enabled, such as before Type-C negotiation completes or when forced by
userspace bypassing HPD checks, rk_udphy_power_on() returns 0 for
unsupported modes.
Because the return value is 0, execution proceeds to rk_udphy_dplane_enable()
and rk_udphy_dplane_select(). Can this cause a kernel crash or SError
resulting from an MMIO access to unclocked UDPHY_PMA registers?
Should rk_udphy_power_on() return an error code like -EOPNOTSUPP instead of
0 when the mode is not supported, so the check here can properly abort?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=12
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 20/29] phy: rockchip: usbdp: Re-init the PHY on orientation change
From: sashiko-bot @ 2026-06-26 21:18 UTC (permalink / raw)
To: Sebastian Reichel
Cc: linux-phy, vkoul, neil.armstrong, robh, conor+dt, devicetree,
olteanv
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-20-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware lane muxing is never actively updated for USB-only devices upon replug or orientation change, breaking USB SuperSpeed in reverse orientation on ports that do not power-cycle the PHY.
--
commit 52b656ef41c03fa9932c56f88505027225e05d96
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Re-init the PHY on orientation change
This commit updates the driver to record cable orientation changes and set the
phy_needs_reinit flag. It aims to fix DP functionality breaking when the cable
is re-plugged with a swapped orientation.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 1bb22fc18c9f9..3d222768dc3c9 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -626,6 +626,7 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> enum typec_orientation orien)
> {
> struct rk_udphy *udphy = typec_switch_get_drvdata(sw);
> + bool flipped = orien == TYPEC_ORIENTATION_REVERSE;
>
> mutex_lock(&udphy->mutex);
>
> @@ -637,7 +638,10 @@ static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> goto unlock_ret;
> }
>
> - udphy->flip = orien == TYPEC_ORIENTATION_REVERSE;
> + if (udphy->flip != flipped)
> + udphy->phy_needs_reinit = true;
> +
> + udphy->flip = flipped;
[Severity: High]
This is a pre-existing issue, but does this actually flush the re-initialization
to the hardware if the USB PHY is already running?
When a user replugs a USB-only Type-C device in reverse orientation on a port
where the USB controller does not power-cycle the PHY on unplug, for example in
host mode, rk_udphy_orien_sw_set() notes the orientation change and sets
udphy->phy_needs_reinit = true.
However, because the USB controller keeps the PHY powered on,
udphy->status & UDPHY_MODE_USB remains true.
When rk_udphy_typec_mux_set() is called later to handle the mux event, it
appears to skip actively re-initializing the PHY if the USB mode is already
active:
if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
udphy->usb_in_use && !udphy->hs)
return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
return 0;
Could this leave the hardware stuck in the old orientation state and break USB
SuperSpeed, causing it to fall back to USB 2.0 upon reverse replug?
> rk_udphy_set_typec_default_mapping(udphy);
> rk_udphy_usb_bvalid_enable(udphy, true);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=20
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 24/29] phy: rockchip: usbdp: Support going from DP-only mode to USB mode
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: vkoul, conor+dt, neil.armstrong, robh, olteanv, devicetree,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-24-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic introduced to restore USB functionality fails on subsequent adapter replugs because `udphy->status` permanently retains the `UDPHY_MODE_USB` flag.
--
commit b83098c186ec9041e5338df8dff07f7fa436a60c
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Support going from DP-only mode to USB mode
When a USB-C adapter, which maps all Superspeed lanes to DP is plugged
in, the USB support is disabled in the PHY. When the adapter is
unplugged and a different adapter with USB functionality is plugged in
afterwards, USB functionality is not restored as the USB controller
keeps the PHY enabled for the entire time.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 837a4cb3e4b60..4566822d70c4a 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
[ ... ]
> @@ -1347,6 +1362,17 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>
> rk_udphy_set_typec_state(udphy, state->mode);
>
> + /*
> + * If the new mode includes USB, but it has not yet been powered
> + * (because the previous mode was DP-only) and the USB PHY was
> + * already initialized by the USB controller, we need to power on
> + * the USB side now since no subsequent phy_init call will come
> + * from the controller.
> + */
> + if ((udphy->mode & UDPHY_MODE_USB) && !(udphy->status & UDPHY_MODE_USB) &&
> + udphy->usb_in_use && !udphy->hs)
> + return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
[Severity: High]
Does this logic handle subsequent replugs correctly if a USB device is plugged
in first?
Since the USB controller keeps the PHY enabled during hotplug (as noted in the
commit message), the UDPHY_MODE_USB flag in udphy->status is not cleared.
If the user then plugs in a DP-only adapter, the PHY is reinitialized for DP
and physically disables the USB3 port.
If the user later unplugs the DP-only adapter and plugs in a USB-only adapter,
won't !(udphy->status & UDPHY_MODE_USB) evaluate to false because the flag was
never cleared? It seems this would skip the call to rk_udphy_power_on() and
leave the USB3 port disabled.
> +
> return 0;
> }
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=24
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 13/29] phy: rockchip: usbdp: Limit DP lane count to muxed lanes
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: olteanv, robh, conor+dt, devicetree, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-13-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the patch's fix is incomplete. The lane count validation added to prevent DP configurations from corrupting USB3 PHY state is bypassed during voltage-only updates. If the Type-C state dynamically reduces the available DP lanes, `udphy->dp_lanes` is updated but the cached `udphy->lanes` remains stale. A subsequent voltage-only update will iterate up to the stale `udphy->lanes` and write DP voltage configurations to the physically reassigned USB3 lanes, corrupting the USB3 PHY state.
--
commit e11a3cd1b181c4fdc9e18a16f2893d3ae7465cb5
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Limit DP lane count to muxed lanes
This commit limits the DisplayPort lane count to 2 if the PHY is
restricted due to USB3 usage. It aims to prevent USB3 state corruption
during voltage configuration.
Closes: https://lore.kernel.org/linux-phy/20260612165546.98E1F1F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 5c38da0dadd22..6de8ac3f91441 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1132,6 +1132,9 @@ static int rk_udphy_dp_phy_verify_link_rate(struct rk_udphy *udphy,
> static int rk_udphy_dp_phy_verify_lanes(struct rk_udphy *udphy,
> struct phy_configure_opts_dp *dp)
> {
> + if (dp->lanes > udphy->dp_lanes)
> + return -EINVAL;
> +
[Severity: High]
This isn't a bug introduced by this patch, but is the fix incomplete for
voltage-only updates?
If a Type-C event renegotiates the connection and restricts udphy->dp_lanes
to 2, udphy->lanes could still hold a stale value (like 4).
Looking at rk_udphy_dp_phy_configure(), the lane verification is skipped
if dp->set_lanes is false:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
if (dp->set_lanes) {
ret = rk_udphy_dp_phy_verify_lanes(udphy, dp);
if (ret)
return ret;
}
...
}
If the driver then issues a phy_configure() with set_lanes=false and
set_voltages=true, the voltage loop will iterate using the stale
udphy->lanes value:
drivers/phy/rockchip/phy-rockchip-usbdp.c:rk_udphy_dp_phy_configure() {
...
if (dp->set_voltages) {
for (i = 0; i < udphy->lanes; i++) {
lane = udphy->dp_lane_sel[i];
...
}
Could this still result in writing DP voltage configurations to reassigned
USB3 lanes, corrupting the USB3 PHY state?
> switch (dp->lanes) {
> case 1:
> case 2:
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=13
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 25/29] phy: rockchip: usbdp: Clear USB status on PHY exit
From: sashiko-bot @ 2026-06-26 21:17 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, olteanv, conor+dt, devicetree, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-25-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Clearing `udphy->status` and returning early in `rk_udphy_usb3_phy_exit` bypasses hardware disablement when the status drops to 0, leading to a permanent clock enable count leak and keeping the PHY hardware incorrectly powered on.
--
commit 0522ec66e66f7228817b6bf6fcd2ff4639f6af01
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Clear USB status on PHY exit
This commit modifies the USB3 PHY exit routine to ensure the USB status
flag is cleared when the system is in DP-only mode. This addresses a
case where the USB3 controller is unbound while a DP-only adapter is
plugged in.
Closes: https://sashiko.dev/#/message/20260625-rockchip-usbdp-cleanup-v7-24-38eb3cf654fd%40collabora.com
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 4566822d70c4a..15c18c6e30935 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1322,8 +1322,10 @@ static int rk_udphy_usb3_phy_exit(struct phy *phy)
> udphy->usb_in_use = false;
>
> /* DP only or high-speed */
> - if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs)
> + if (!(udphy->mode & UDPHY_MODE_USB) || udphy->hs) {
> + udphy->status &= ~UDPHY_MODE_USB;
> return 0;
[Severity: Medium]
If udphy->status drops to 0 after this bitwise clear, does returning early
bypass hardware disablement?
It appears this might lead to a permanent clock enable count leak and keep
the PHY hardware incorrectly powered on when a USB3 connection is unbound
or exited while the PHY is in a DP-only mode.
> + }
>
> rk_udphy_power_off(udphy, UDPHY_MODE_USB);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=25
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 06/29] phy: rockchip: usbdp: Drop seamless DP takeover
From: sashiko-bot @ 2026-06-26 21:16 UTC (permalink / raw)
To: Sebastian Reichel
Cc: devicetree, olteanv, robh, conor+dt, vkoul, neil.armstrong,
linux-phy
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-6-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditionally disabling the active DP PHY while the upstream display controller is still streaming data causes an AXI bus stall and system hang on Rockchip SoCs.
- [High] Forcing DP HPD to disconnected during probe permanently breaks fixed eDP/DP displays.
--
commit 31addd0a9ff6667916e1a40ca73d061ababa8583
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Drop seamless DP takeover
This commit drops support for seamless DP takeover from the bootloader. If a
pre-enabled DP PHY is detected, it logs a warning, forces HPD low, and
unconditionally disables the PHY.
Link: https://lore.kernel.org/linux-phy/20260612164107.C7DB21F000E9@smtp.kernel.org/
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 3fc8222fcaec2..f7ff55f03b7cf 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -938,10 +939,12 @@ static int rk_udphy_get_initial_status(struct rk_udphy *udphy)
> rk_udphy_reset_deassert_all(udphy);
>
> regmap_read(udphy->pma_regmap, CMN_LANE_MUX_AND_EN_OFFSET, &value);
> - if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value))
> - udphy->status = UDPHY_MODE_DP;
> - else
> - rk_udphy_disable(udphy);
> + if (FIELD_GET(CMN_DP_LANE_MUX_ALL, value) && FIELD_GET(CMN_DP_LANE_EN_ALL, value)) {
> + dev_info(udphy->dev, "Started with DP PHY pre-enabled; seamless takeover unsupported\n");
> + rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, false);
[Severity: High]
Will forcing HPD low here permanently break fixed eDP or DP displays?
While this pulls HPD low to signal a disconnect, fixed eDP/DP boards do not
use Type-C muxes and will never trigger a re-assertion via Type-C events.
Since the upstream DP controller driver does not know about the VO GRF
override, the HPD signal remains permanently forced low, and the display
will be seen as disconnected by the kernel.
> + }
> +
> + rk_udphy_disable(udphy);
[Severity: High]
Could calling rk_udphy_disable() synchronously right after forcing HPD low
trigger an AXI bus stall?
If the DP PHY was left enabled by the bootloader, the upstream display
controller (VOP) might still be actively issuing AXI/AHB transactions to the
PHY.
The HPD state change via GRF is asynchronous, but this disables the PHY clocks
immediately on the very next line. Removing the clock from an active AXI slave
often causes the bus to stall indefinitely, resulting in a hard system freeze.
>
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=6
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply
* Re: [PATCH v8 03/29] phy: rockchip: usbdp: Do not lose USB3 PHY status
From: sashiko-bot @ 2026-06-26 21:16 UTC (permalink / raw)
To: Sebastian Reichel
Cc: robh, conor+dt, linux-phy, devicetree, vkoul, neil.armstrong,
olteanv
In-Reply-To: <20260626-rockchip-usbdp-cleanup-v8-3-47f682987895@collabora.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] USB3 reverse orientation changes are never programmed into the PHY hardware because `rk_udphy_orien_sw_set` updates the software structs but fails to trigger a hardware re-initialization.
--
commit 2e0448207186d0a4562665690cfdad4abd051059
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
phy: rockchip: usbdp: Do not lose USB3 PHY status
This commit fixes an issue where the PHY status is lost during a DisplayPort
mode change. It ensures that the tracking information for USB3 is preserved
so the PHY isn't inadvertently disabled when DP is toggled off.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a5ffdf7a6e76b..12dfdd470a782 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1008,7 +1008,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
> rk_udphy_u3_port_disable(udphy, false);
> udphy->mode_change = false;
> } else if (udphy->mode_change) {
> - udphy->status = UDPHY_MODE_NONE;
[Severity: High]
This isn't a bug introduced by this patch, but does keeping the USB mode bit
in the status flag prevent USB3 reverse orientation changes from being
programmed into the PHY hardware?
When a USB-C cable is flipped, the orientation switch handler
rk_udphy_orien_sw_set() updates the software state and requests a re-init:
udphy->phy_needs_reinit = true;
udphy->flip = flipped;
rk_udphy_set_typec_default_mapping(udphy);
Later, rk_udphy_typec_mux_set() evaluates whether to apply this to the
hardware. However, it only calls rk_udphy_power_on() if the USB mode is not
already set in the status flag:
if (... && !(udphy->status & UDPHY_MODE_USB) && ...)
return rk_udphy_power_on(udphy, UDPHY_MODE_USB);
Since DWC3 keeps the PHY enabled permanently and the status flag retains
the USB mode, it appears rk_udphy_power_on() is never invoked to apply the
new lane mapping.
Could this cause the USB3 port to silently fail when plugged in reverse
orientation, since the hardware is never reconfigured?
> if (udphy->mode == UDPHY_MODE_DP)
> rk_udphy_u3_port_disable(udphy, true);
>
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-rockchip-usbdp-cleanup-v8-0-47f682987895@collabora.com?part=3
--
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