RE: [PATCH net-next 1/4] net: phy: xpcs: Clear latched value of RX/TX fault

[Jose Abreu]

From: Russell King - ARM Linux admin <linux@xxxxxxxxxxxxxxx>
Date: Mar/13/2020, 14:01:22 (UTC+00:00)

> On Fri, Mar 13, 2020 at 02:39:40PM +0100, Jose Abreu wrote:
> > When reading RX/TX fault register we may have latched values from Link
> > down. Clear the latched value first and then read it again to make sure
> > no old errors are flagged and that new errors are caught.
> 
> The purpose of the latched link down is so that software can respond
> to a momentary loss of link with a possible change in the negotiation
> results.  That is why IEEE 802.3 wants a link loss to be a latched
> event.
> 
> Double-reading the status register loses that information, and hides
> it from phylink.  A change in negotiation, which can occur very
> quickly on fiber links) can go unnoticed if the latching is not
> propagated up through phylink.
> 
> If the negotiation parameters have changed, and pcs_get_state() does
> not report that the link has failed, then mac_link_up() will _not_ be
> called with the new link parameters, and the MAC will continue using
> the old ones.  Therefore, it is very important that any link-down
> event is reported to phylink.
> 
> Phylink currently doesn't respond to a link-down event reported via
> PCS by re-checking after processing the link loss, but it could do,
> which would improve it's behaviour in that scenario.  I would prefer
> this resolution, rather than your proposed double-reading of the
> status register to "lose" the link-down event.
> 
> I do have some patches that make that easier, but they're delayed
> behind the mass of patches that I still have outstanding - and trying
> to get progress on getting phylink patches merged has been glacial,
> and fraught with problems this time around.

This is not link status register. Its TX / RX fault and its latched high. 
Link status is another register and we only read it once because of the 
above reasons you mentioned.

When in 10GKR, this seems to always go up after link transition, hence we 
added the double read.

I just read your reply to patch 2/4 of this series and it looks like the 
two patches are correlated.

---
Thanks,
Jose Miguel Abreu