Re: [PATCH net-next v8] net: phy: Add driver for Motorcomm Quad 2.5GbE phy

On 8/4/26 04:49, Andrew Lunn wrote:
>> +static int yt8824_config_aneg(struct phy_device *phydev)
>> +{
>> +	int phy_ctrl = 0;
>> +	int ret = 0;
>> +
>> +	ret = phy8824_page_write_with_lock(phydev, YT8824_RSSR_UTP_SPACE);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	if (linkmode_test_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>> +			      phydev->advertising))
>> +		phy_ctrl = MDIO_AN_10GBT_CTRL_ADV2_5G;
>> +
>> +	ret = phy_modify_mmd_changed(phydev, MDIO_MMD_AN,
>> +				     MDIO_AN_10GBT_CTRL,
>> +				     MDIO_AN_10GBT_CTRL_ADV2_5G,
>> +				     phy_ctrl);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return genphy_config_aneg(phydev);
>> +}
> 
> What i'm missing now is some explanation why this is safe. What is
> stopping something from changing the page after the call to
> phy8824_page_write_with_lock() before genphy_config_aneg() completes?

Ans: The assumption that there is no problem here is based on the handling of 
     the USXGMII reg space being error-free, especially that the swap back to 
     the old reg space has no errors. However, this may not be correct. 
     For example, if yt8824_sds_isolate_paged() completes its processing, but 
     an error occurs when swapping back to old_page—meaning the switch to the 
     UTP reg space does not happen—then this assumption would be incorrect. I
     miss this scenario.
     You may be right that a high-level lock is needed to ensure that UTPs do 
     not interfere with each other. I will update it in v9.

> 
> 	Andrew