Re: [PATCH net 6/6] net: dsa: mv88e6xxx: Link in pcs_get_state() if AN is bypassed

["Marek Behún" <kabel@kernel.org>]

On Mon, 29 Nov 2021 23:14:17 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Nov 29, 2021 at 08:58:23PM +0100, Marek Behún wrote:
> >  static int mv88e6xxx_serdes_pcs_get_state(struct mv88e6xxx_chip *chip,
> > -					  u16 status, u16 lpa,
> > +					  u16 ctrl, u16 status, u16 lpa,
> >  					  struct phylink_link_state *state)
> >  {
> > +	state->link = !!(status & MV88E6390_SGMII_PHY_STATUS_LINK);
> > +
> >  	if (status & MV88E6390_SGMII_PHY_STATUS_SPD_DPL_VALID) {
> > -		state->link = !!(status & MV88E6390_SGMII_PHY_STATUS_LINK);
> > +		state->an_complete = !!(ctrl & BMCR_ANENABLE);  
> 
> I think I'd much rather report the value of BMSR_ANEGCAPABLE - since
> an_complete controls the BMSR_ANEGCAPABLE bit in the emulated PHY
> that userspace sees. Otherwise, an_complete is not used.
> 
> state->link is the key that phylink uses to know whether it can
> trust the status being reported.

Isn't BMSR_ANEGCAPABLE set to 1 even if aneg is disabled in BMCR?
I will test tomorrow.

The reason why I used BMCR_ANENABLE is that if we don't enable AN, the
PHY will report SPD_DPL_VALID if link is up. But clearly AN is not
completed in that case, because it was never enabled in the first place.

Marek