netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Andrew Lunn <andrew@lunn.ch>
Cc: Robert Hancock <robert.hancock@calian.com>,
	netdev@vger.kernel.org, hkallweit1@gmail.com,
	davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com
Subject: Re: [PATCH net-next] net: phy: marvell: update abilities and advertising when switching to SGMII
Date: Fri, 29 Apr 2022 17:17:57 +0100	[thread overview]
Message-ID: <YmwPtZ4akTgPRYlq@shell.armlinux.org.uk> (raw)
In-Reply-To: <YmtA7/OjKIQD/vuD@lunn.ch>

On Fri, Apr 29, 2022 at 03:35:43AM +0200, Andrew Lunn wrote:
> On Wed, Apr 27, 2022 at 01:39:28PM -0600, Robert Hancock wrote:
> > With some SFP modules, such as Finisar FCLF8522P2BTL, the PHY hardware
> > strapping defaults to 1000BaseX mode, but the kernel prefers to set them
> > for SGMII mode.
> 
> Is this the SFP code determining this? Its copper == use SGMII?
> 
> > When this happens and the PHY is soft reset, the BMSR
> > status register is updated, but this happens after the kernel has already
> > read the PHY abilities during probing. This results in support not being
> > detected for, and the PHY not advertising support for, 10 and 100 Mbps
> > modes, preventing the link from working with a non-gigabit link partner.
> > 
> > When the PHY is being configured for SGMII mode, call genphy_read_abilities
> > again in order to re-read the capabilities, and update the advertising
> > field accordingly.
> 
> Is this actually a generic problem? There are other PHYs used in SFP
> modules, and i assume they also could have their mode changed. So
> should the re-reading of the abilities be in the core, not each
> driver?

The most common PHY in SFPs is the 88E1111, because that offers both
MDIO and I2C hookup. Some other modules use the AR8035, but that
doesn't support I2C, and consequently doesn't tend to be accessible.

Re-reading the capabilities when the PHY changes host interface mode
between 1000baseX and SGMII makes sense because of the fixed-speed vs
10/100/1000 capability of the other.

One of the problems is the core code doesn't know how the PHY is
initially configured, so we can't actually detect mode changes. We
would need all PHY drivers to read the MAC mode at probe time, save
it away as the current mode, and only then can we detect changes.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!

  reply	other threads:[~2022-04-29 16:18 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-27 19:39 [PATCH net-next] net: phy: marvell: update abilities and advertising when switching to SGMII Robert Hancock
2022-04-29  1:35 ` Andrew Lunn
2022-04-29 16:17   ` Russell King (Oracle) [this message]
2022-04-30 15:05 ` Andrew Lunn
2022-05-01 11:10 ` patchwork-bot+netdevbpf

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=YmwPtZ4akTgPRYlq@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=robert.hancock@calian.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).