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

[sashiko-bot@kernel.org]

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(&gtr_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