Re: [PATCH net-next v0] net: phy: aquantia: poll status register

[Andrew Lunn]

On Mon, Oct 07, 2024 at 10:35:36AM +1300, Aryan Srivastava wrote:
> The system interface connection status register is not immediately
> correct upon line side link up. This results in the status being read as
> OFF and then transitioning to the correct host side link mode with a
> short delay. This results in the phylink framework passing the OFF
> status down to all MAC config drivers, resulting in the host side link
> being misconfigured, which in turn can lead to link flapping or complete
> packet loss in some cases.
> 
> Mitigate this by periodically polling the register until it not showing
> the OFF state. This will be done every 1ms for 10ms, using the same
> poll/timeout as the processor intensive operation reads.

Does the datasheet say anything about when MDIO_PHYXS_VEND_IF_STATUS
is valid?

>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_XAUI	4
>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_SGMII	6
>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_RXAUI	7
> +#define MDIO_PHYXS_VEND_IF_STATUS_TYPE_OFF	9
>  #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_OCSGMII	10
>  
>  #define MDIO_AN_VEND_PROV			0xc400
> @@ -342,9 +343,18 @@ static int aqr107_read_status(struct phy_device *phydev)
>  	if (!phydev->link || phydev->autoneg == AUTONEG_DISABLE)
>  		return 0;
>  
> -	val = phy_read_mmd(phydev, MDIO_MMD_PHYXS, MDIO_PHYXS_VEND_IF_STATUS);
> -	if (val < 0)
> -		return val;
> +	/**
> +	 * The status register is not immediately correct on line side link up.
> +	 * Poll periodically until it reflects the correct ON state.
> +	 */
> +	ret = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_PHYXS,
> +					MDIO_PHYXS_VEND_IF_STATUS, val,
> +					(FIELD_GET(MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK, val) !=
> +					MDIO_PHYXS_VEND_IF_STATUS_TYPE_OFF),
> +					AQR107_OP_IN_PROG_SLEEP,
> +					AQR107_OP_IN_PROG_TIMEOUT, false);
> +	if (ret)
> +		return ret;

I don't know if returning ETIMEDOUT is the correct thing to do
here. It might be better to set phydev->link to false, since there is
no end to end link yet.

	Andrew