Messages in this thread | | | Date | Fri, 2 Jun 2023 14:28:04 +0300 | From | Ioana Ciornei <> | Subject | Re: [PATCH net-next v6 1/1] net: dsa: mv88e6xxx: implement USXGMII mode for mv88e6393x |
| |
On Fri, Jun 02, 2023 at 11:30:36AM +0100, Russell King (Oracle) wrote: > On Thu, Jun 01, 2023 at 05:17:04PM -0700, msmulski2@gmail.com wrote: > > +/* USXGMII registers for Marvell switch 88e639x are undocumented and this function is based > > + * on some educated guesses. It appears that there are no status bits related to > > + * autonegotiation complete or flow control. > > + */ > > +static int mv88e639x_serdes_pcs_get_state_usxgmii(struct mv88e6xxx_chip *chip, > > + int port, int lane, > > + struct phylink_link_state *state) > > +{ > > + u16 status, lp_status; > > + int err; > > + > > + err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS, > > + MV88E6390_USXGMII_PHY_STATUS, &status); > > + if (err) { > > + dev_err(chip->dev, "can't read Serdes USXGMII PHY status: %d\n", err); > > + return err; > > + } > > + dev_dbg(chip->dev, "USXGMII PHY status: 0x%x\n", status); > > + > > + state->link = !!(status & MDIO_USXGMII_LINK); > > + > > + if (state->link) { > > + err = mv88e6390_serdes_read(chip, lane, MDIO_MMD_PHYXS, > > + MV88E6390_USXGMII_LP_STATUS, &lp_status); > > What's the difference between these two registers? Specifically, what > I'm asking is why the USXGMII partner status seems to be split between > two separate registers. > > Note that I think phylink_decode_usxgmii_word() is probably missing a > check for MDIO_USXGMII_LINK, based on how Cisco SGMII works (and > USXGMII is pretty similar.) > > MDIO_USXGMII_LINK indicates whether the attached PHY has link on the > media side or not. It's entirely possible for the USXGMII link to be > up (thus allowing us to receive the "LPA" from the PHY) but for the > PHY to be reporting to us that it has no media side link. > > So, I think phylink_decode_usxgmii_word() at least needs at the > beginning this added: > > if (!(lpa & MDIO_USXGMII_LINK)) { > state->link = false; > return; > } > > The only user of this has been the Lynx PCS, so I'll add Ioana to this > email as well to see whether there's any objection from Lynx PCS users > to adding it. >
I just tested this snippet on a LX2160ARDB that has USXGMII on two of its lanes which go to Aquantia AQR113C PHYs.
The lpa read is 0x5601 and, with the patch, the interface does not link up. I am not sure what is happening, if it's this PHY in particular that does not properly set MDIO_USXGMII_LINK.
Ioana
| |