From: Ioana Ciornei <ioana.ciornei@nxp.com>
To: "Russell King (Oracle)" <linux@armlinux.org.uk>
Cc: msmulski2@gmail.com, andrew@lunn.ch, f.fainelli@gmail.com,
olteanv@gmail.com, davem@davemloft.net, edumazet@google.com,
kuba@kernel.org, pabeni@redhat.com, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, simon.horman@corigine.com,
kabel@kernel.org, Michal Smulski <michal.smulski@ooma.com>
Subject: Re: [PATCH net-next v6 1/1] net: dsa: mv88e6xxx: implement USXGMII mode for mv88e6393x
Date: Fri, 2 Jun 2023 17:31:38 +0300 [thread overview]
Message-ID: <20230602143138.aaq22cmwinrrhtrv@LXL00007.wbi.nxp.com> (raw)
In-Reply-To: <ZHnqcvP8hv19RBr8@shell.armlinux.org.uk>
On Fri, Jun 02, 2023 at 02:11:14PM +0100, Russell King (Oracle) wrote:
> On Fri, Jun 02, 2023 at 02:28:04PM +0300, Ioana Ciornei wrote:
> > 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.
>
> Thanks for testing. I wonder if the USXGMII control word that the PHY
> transmits can be read from one of its registers?
>
I had a look into the AQR gen4 datasheet and the only thing that I could
find is the "PCS USX0 Auto-Neg Control Register" which has the following
field:
USX0 Auto-Negotiation Message code [7:0]
Configure the message opcode for USXGMII Auto-Negotiation.
It sounded promising but it's not making any sense to me because it's
just 8 bits long.
> If the PHY is correctly setting the link bit, but it's not appearing
> in the Lynx PCS registers as set, that would be weird, and suggest the
> PCS is handling that itself. Does the Lynx link status bit change
> according to the media link status, while the AN complete bit stays
> set?
>
No, the Lynx link status bit is 1 even though the media side is no
longer connected, for example. Here I just removed the cable, the link
on the PHY is down but the PCS link is up.
[ 133.011696] fsl_dpaa2_eth dpni.2 endpmac3: Link is Down
[ 133.555343] phylink_decode_usxgmii_word 3331: lpa 0x5601
[ 133.560672] mdio_bus 0x0000000008c0f000:00: mode=usxgmii/10Gbps/Full link=1 an_complete=1
[ 134.579348] phylink_decode_usxgmii_word 3331: lpa 0x5601
Ioana
next prev parent reply other threads:[~2023-06-02 14:31 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-02 0:17 [PATCH net-next v6 0/1] net: dsa: mv88e6xxx: implement USXGMII mode for mv88e6393x msmulski2
2023-06-02 0:17 ` [PATCH net-next v6 1/1] " msmulski2
2023-06-02 7:53 ` Marek Behún
2023-06-03 4:32 ` Michal Smulski
2023-06-02 10:30 ` Russell King (Oracle)
2023-06-02 11:28 ` Ioana Ciornei
2023-06-02 13:11 ` Russell King (Oracle)
2023-06-02 14:31 ` Ioana Ciornei [this message]
2023-06-03 5:27 ` Michal Smulski
2023-06-03 4:56 ` Michal Smulski
2023-06-02 11:08 ` Russell King (Oracle)
2023-06-03 4:59 ` Michal Smulski
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230602143138.aaq22cmwinrrhtrv@LXL00007.wbi.nxp.com \
--to=ioana.ciornei@nxp.com \
--cc=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=f.fainelli@gmail.com \
--cc=kabel@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=michal.smulski@ooma.com \
--cc=msmulski2@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=olteanv@gmail.com \
--cc=pabeni@redhat.com \
--cc=simon.horman@corigine.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).