[PATCH 0/3] phy: zynqmp: fix SERDES scrambler register handling and enable for USB

[PATCH 0/3] phy: zynqmp: fix SERDES scrambler register handling and enable for USB

[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.

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: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.44.4


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

[PATCH 1/3] phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask

[Radhey Shyam Pandey]

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.44.4


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

[PATCH 2/3] phy: zynqmp: use read-modify-write for SERDES scrambler bypass

[Radhey Shyam Pandey]

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.44.4


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

[PATCH 3/3] phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB

[Radhey Shyam Pandey]

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.44.4


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

Re: [PATCH 1/3] phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unhandled Error Path in `xpsgtr_phy_init()` masks clock initialization failures.
--

commit f38f7c8479703290a0f18153b9a9207f41500566
Author: Nava kishore Manne <nava.kishore.manne@amd.com>

phy: zynqmp: fix L0_TM_DISABLE_SCRAMBLE_ENCODER mask

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.

> 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

This isn't a bug introduced by this patch, but there is a pre-existing
issue in xpsgtr_phy_init().

If clk_prepare_enable() fails, the error path appears to mask the failure:

drivers/phy/xilinx/phy-zynqmp.c:xpsgtr_phy_init() {
	int ret = 0;

	mutex_lock(&gtr_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;
...
out:
	mutex_unlock(&gtr_dev->gtr_mutex);
	return ret;
}

Since ret is initialized to 0, will this return 0 instead of the actual error
code from clk_prepare_enable()?

If the function falsely reports success, the PHY framework will consider the
device initialized.

During teardown, could xpsgtr_phy_exit() unconditionally call
clk_disable_unprepare() on a clock that was never enabled, causing a clock
reference count underflow and a kernel WARN_ON?

Furthermore, would allowing the upper layer to access PHY registers without a
running clock risk hanging the AXI bus on ZynqMP, potentially leading to an
asynchronous external abort (system panic)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511163135.2924642-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

Re: [PATCH 3/3] phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] This problem wasn't introduced by this patch, but the scrambler and 8b/10b bypass state is not explicitly initialized for DisplayPort (DP) and PCIe, leaving them vulnerable to inheriting an incorrect hardware state.
--

commit bd13b5595b6bf913b77d4b61fb704c7e56fa4847
Author: Nava kishore Manne <nava.kishore.manne@amd.com>

phy: zynqmp: keep SERDES scrambler and 8b/10b enabled for USB

This commit ensures that physical layer scrambling and 8b/10b encoding are
not bypassed for USB operations. It explicitly disables the scrambler bypass
for USB3 while preserving it for SATA and SGMII modes where encoding is
handled by the controller.

> 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

[ ... ]

> @@ -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;
>  	}

Does this leave the scrambler and 8b/10b bypass state uninitialized for
DisplayPort (DP) and PCIe?

While this commit resolves the state leak for USB by explicitly clearing the
bypass bits, it appears DP and PCIe might still be vulnerable to similar
issues. 

If a lane is repurposed from SATA or SGMII to DP or PCIe (for example, if a
bootloader probes an M.2 slot for SATA, and Linux later configures it for
PCIe NVMe), could the PHY inherit an erroneously bypassed state?

Should the bypass bits also be explicitly cleared for ICM_PROTOCOL_DP and
ICM_PROTOCOL_PCIE to ensure they do not rely on potentially corrupted hardware
defaults?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511163135.2924642-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