On 5/13/2026 3:05 AM, sashiko-bot@kernel.org wrote:

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

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

For this pre-existing issue is it ok to send a separate series and will
include others error handling gaps. So far i have identified two issues.

phy: zynqmp: balance pm_runtime on probe allocation failure
phy: zynqmp: fix xpsgtr_phy_init() clock error handling

Thanks,
Radhey

commit f38f7c8479703290a0f18153b9a9207f41500566
Author: Nava kishore Manne [off-list ref]

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.

quoted

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)?