[PATCH net] amd-xgbe: read link status twice to avoid inconsistencies

[PATCH net] amd-xgbe: read link status twice to avoid inconsistencies

[Raju Rangoju]

The link status is latched low, so read the register twice to get the
current status and avoid link inconsistencies.

As per IEEE 802.3 "Table 22-8 - Status register bit definitions"
1.2  Link Status  1 = link is up    0 = link is down    RO/LL

Fixes: 4f3b20bfbb75 ("amd-xgbe: add support for rx-adaptation")
Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
index 268399dfcf22..d233e3faa1a9 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
@@ -2914,6 +2914,10 @@ static int xgbe_phy_link_status(struct xgbe_prv_data *pdata, int *an_restart)
 		}
 
 		/* check again for the link and adaptation status */
+		/* Link status is latched low, so read once to clear
+		 * and then read again to get current state
+		 */
+		reg = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_STAT1);
 		reg = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_STAT1);
 		if ((reg & MDIO_STAT1_LSTATUS) && pdata->rx_adapt_done)
 			return 1;
-- 
2.34.1

Re: [PATCH net] amd-xgbe: read link status twice to avoid inconsistencies

[Andrew Lunn]

On Thu, May 15, 2025 at 01:11:45AM +0530, Raju Rangoju wrote:
> The link status is latched low, so read the register twice to get the
> current status and avoid link inconsistencies.
> 
> As per IEEE 802.3 "Table 22-8 - Status register bit definitions"
> 1.2  Link Status  1 = link is up    0 = link is down    RO/LL
> 
> Fixes: 4f3b20bfbb75 ("amd-xgbe: add support for rx-adaptation")
> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
> ---
>  drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> index 268399dfcf22..d233e3faa1a9 100644
> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
> @@ -2914,6 +2914,10 @@ static int xgbe_phy_link_status(struct xgbe_prv_data *pdata, int *an_restart)
>  		}
>  
>  		/* check again for the link and adaptation status */
> +		/* Link status is latched low, so read once to clear
> +		 * and then read again to get current state
> +		 */
> +		reg = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_STAT1);


Since this is not a phylib user, i don't care too much, but:

https://elixir.bootlin.com/linux/v6.14.6/source/drivers/net/phy/phy_device.c#L2514

	/* The link state is latched low so that momentary link
	 * drops can be detected. Do not double-read the status
	 * in polling mode to detect such short link drops except
	 * the link was already down.
	 */

So you don't care about short link drops? You don't want to tell user
space the link went away and came back? It never happened.

	Andrew

Re: [PATCH net] amd-xgbe: read link status twice to avoid inconsistencies

[Rangoju, Raju]

On 5/15/2025 5:37 AM, Andrew Lunn wrote:
> On Thu, May 15, 2025 at 01:11:45AM +0530, Raju Rangoju wrote:
>> The link status is latched low, so read the register twice to get the
>> current status and avoid link inconsistencies.
>>
>> As per IEEE 802.3 "Table 22-8 - Status register bit definitions"
>> 1.2  Link Status  1 = link is up    0 = link is down    RO/LL
>>
>> Fixes: 4f3b20bfbb75 ("amd-xgbe: add support for rx-adaptation")
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
>> ---
>>   drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> index 268399dfcf22..d233e3faa1a9 100644
>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c
>> @@ -2914,6 +2914,10 @@ static int xgbe_phy_link_status(struct xgbe_prv_data *pdata, int *an_restart)
>>   		}
>>   
>>   		/* check again for the link and adaptation status */
>> +		/* Link status is latched low, so read once to clear
>> +		 * and then read again to get current state
>> +		 */
>> +		reg = XMDIO_READ(pdata, MDIO_MMD_PCS, MDIO_STAT1);
> 
> 
> Since this is not a phylib user, i don't care too much, but:
> 
> https://elixir.bootlin.com/linux/v6.14.6/source/drivers/net/phy/phy_device.c#L2514
> 
> 	/* The link state is latched low so that momentary link
> 	 * drops can be detected. Do not double-read the status
> 	 * in polling mode to detect such short link drops except
> 	 * the link was already down.
> 	 */
> 
> So you don't care about short link drops? You don't want to tell user
> space the link went away and came back? It never happened.

Thanks Andrew for point this out. I'll re-spin v2 addressing these.

> 
> 	Andrew