Re: [PATCH net-next v3 4/4] net: phy: add X-Powers AC200/AC300 EPHY driver

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing `phy_restore_page()` on the error path of `phy_select_page()` leaves the MDIO bus permanently locked.
--

--- Patch [4]: [PATCH net-next v3 4/4] net: phy: add X-Powers AC200/AC300 EPHY driver ---
commit fc0883818634a8eab77474550b0a3aac9604dc17
Author: James Hilliard <[email protected]>

net: phy: add X-Powers AC200/AC300 EPHY driver
    
This commit introduces a new Ethernet PHY driver for the X-Powers AC200
and AC300 companion ICs. It provides package-specific I2C or MDIO control
initialization and implements the standard PHY driver callbacks.

> diff --git a/drivers/net/phy/xpowers-acx00-main.c b/drivers/net/phy/xpowers-acx00-main.c
> new file mode 100644
> index 0000000000000..2260ddea4b315
> --- /dev/null
> +++ b/drivers/net/phy/xpowers-acx00-main.c
[ ... ]
> @@ -0,0 +1,536 @@
[ ... ]
> +static int acx00_ephy_config_init(struct phy_device *phydev)
> +{
[ ... ]
> +	if (priv->is_ac300 && priv->use_low_calibration_tuning) {
> +		afe_eq_rx_detect =
> +			ACX00_PAGE6_AFE_EQ_RX_DETECT_LOW_CAL_VALUE;
> +		tx_level_value = ACX00_PAGE6_TX_LEVEL_LOW_CAL_VALUE;
> +	}
> +
> +	oldpage = phy_select_page(phydev, ACX00_PAGE_1);
> +	if (oldpage < 0)
> +		return oldpage;

[Severity: High]
Does this error path leave the MDIO bus permanently locked?

Looking at phy_select_page(), it holds the MDIO bus lock even when returning
a negative error code so that the caller can safely clean up. Since this path
returns directly instead of jumping to the out_restore_page label, could this
result in an unrecoverable deadlock for any future accesses on the same bus?

> +
> +	ret = __phy_write(phydev, ACX00_PAGE1_APS_CONTROL_REG,
> +			  ACX00_PAGE1_APS_DISABLED_4S_VALUE);
> +	if (ret)
> +		goto out_restore_page;
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4