Hello,

This is the V2 of a series to add isolation support for PHY devices.

As a remainder, this mode allows a PHY to set its MII lines in
high-impedance mode to avoid interferences on this bus.

So far, I've identified that :

 - Marvell 88e1512 isolation works fine
 - LXT973 claims to support isolation, but it's actually broken
 - Marvell 88x3310 doesn't support isolation, by design
 - Marvell 88e1111 claims to support isolation in GMII, RGMII, TBI
   (untested) but doesn't in SGMII (tested).

Changes in V2 :

 - Removed the loopback mode that was included in the first iteration
 - Added phy_shutdown, to make sure we de-isolate the PHY when rebooting
 - Changes the "PHY_NO_ISOLATE" flag to a phy driver ops. Testing showed
   that some PHYs may or may not support isolation based on the
   interface that's being used.
 - Added isolation support reporting for the Marvell 88e1111 PHY.

V1 : https://lore.kernel.org/netdev/20240911212713.2178943-1-maxime.chevallier@bootlin.com/

Maxime Chevallier (9):
  net: phy: allow isolating PHY devices
  net: phy: Introduce phy_shutdown for device quiescence.
  net: phy: Allow PHY drivers to report isolation support
  net: phy: lxt: Mark LXT973 PHYs as having a broken isolate mode
  net: phy: marvell10g: 88x3310 and 88x3340 don't support isolate mode
  net: phy: marvell: mv88e1111 doesn't support isolate in SGMII mode
  net: phy: introduce ethtool_phy_ops to get and set phy configuration
  net: ethtool: phy: allow reporting and setting the phy isolate status
  netlink: specs: introduce phy-set command along with configurable
    attributes

 Documentation/netlink/specs/ethtool.yaml     |  15 +++
 Documentation/networking/ethtool-netlink.rst |   1 +
 drivers/net/phy/lxt.c                        |   2 +
 drivers/net/phy/marvell.c                    |   9 ++
 drivers/net/phy/marvell10g.c                 |   2 +
 drivers/net/phy/phy.c                        |  44 ++++++++
 drivers/net/phy/phy_device.c                 | 101 +++++++++++++++++--
 include/linux/ethtool.h                      |   8 ++
 include/linux/phy.h                          |  42 ++++++++
 include/uapi/linux/ethtool_netlink.h         |   2 +
 net/ethtool/netlink.c                        |   8 ++
 net/ethtool/netlink.h                        |   1 +
 net/ethtool/phy.c                            |  68 +++++++++++++
 13 files changed, 297 insertions(+), 6 deletions(-)

-- 
2.46.1

I'm going to ask a very basic question concerning this.

Isolation was present in PHYs early on when speeds were low, and thus
electrical reflections weren't too much of a problem, and thus star
topologies didn't have too much of an effect. A star topology is
multi-drop. Even if the PCB tracks go from MAC to PHY1 and then onto
PHY2, if PHY2 is isolated, there are two paths that the signal will
take, one to MAC and the other to PHY2. If there's no impediance match
at PHY2 (e.g. because it's in high-impedance mode) then that
transmission line is unterminated, and thus will reflect back towards
the MAC.

As speeds get faster, then reflections from unterminated ends become
more of an issue.

I suspect the reason why e.g. 88x3310, 88E1111 etc do not support
isolate mode is because of this - especially when being used in
serdes mode, the topology is essentially point-to-point and any
side branches can end up causing data corruption.

So my questions would be, is adding support for isolation mode in
PHYs given todays network speeds something that is realistic, and
do we have actual hardware out there where there is more than one
PHY in the bus. If there is, it may be useful to include details
of that (such as PHY interface type) in the patch series description.

On Fri, Oct 04, 2024 at 06:15:50PM +0200, Maxime Chevallier wrote:
> Hello,
> 
> This is the V2 of a series to add isolation support for PHY devices.
> 
> As a remainder, this mode allows a PHY to set its MII lines in
> high-impedance mode to avoid interferences on this bus.
> 
> So far, I've identified that :
> 
>  - Marvell 88e1512 isolation works fine
>  - LXT973 claims to support isolation, but it's actually broken
>  - Marvell 88x3310 doesn't support isolation, by design
>  - Marvell 88e1111 claims to support isolation in GMII, RGMII, TBI
>    (untested) but doesn't in SGMII (tested).
> 
> Changes in V2 :
> 
>  - Removed the loopback mode that was included in the first iteration
>  - Added phy_shutdown, to make sure we de-isolate the PHY when rebooting
>  - Changes the "PHY_NO_ISOLATE" flag to a phy driver ops. Testing showed
>    that some PHYs may or may not support isolation based on the
>    interface that's being used.
>  - Added isolation support reporting for the Marvell 88e1111 PHY.
> 
> V1 : https://lore.kernel.org/netdev/20240911212713.2178943-1-maxime.chevallier@bootlin.com/
> 
> Maxime Chevallier (9):
>   net: phy: allow isolating PHY devices
>   net: phy: Introduce phy_shutdown for device quiescence.
>   net: phy: Allow PHY drivers to report isolation support
>   net: phy: lxt: Mark LXT973 PHYs as having a broken isolate mode
>   net: phy: marvell10g: 88x3310 and 88x3340 don't support isolate mode
>   net: phy: marvell: mv88e1111 doesn't support isolate in SGMII mode
>   net: phy: introduce ethtool_phy_ops to get and set phy configuration
>   net: ethtool: phy: allow reporting and setting the phy isolate status
>   netlink: specs: introduce phy-set command along with configurable
>     attributes
> 
>  Documentation/netlink/specs/ethtool.yaml     |  15 +++
>  Documentation/networking/ethtool-netlink.rst |   1 +
>  drivers/net/phy/lxt.c                        |   2 +
>  drivers/net/phy/marvell.c                    |   9 ++
>  drivers/net/phy/marvell10g.c                 |   2 +
>  drivers/net/phy/phy.c                        |  44 ++++++++
>  drivers/net/phy/phy_device.c                 | 101 +++++++++++++++++--
>  include/linux/ethtool.h                      |   8 ++
>  include/linux/phy.h                          |  42 ++++++++
>  include/uapi/linux/ethtool_netlink.h         |   2 +
>  net/ethtool/netlink.c                        |   8 ++
>  net/ethtool/netlink.h                        |   1 +
>  net/ethtool/phy.c                            |  68 +++++++++++++
>  13 files changed, 297 insertions(+), 6 deletions(-)
> 
> -- 
> 2.46.1
> 
> 

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

Hello Russell

On Fri, 4 Oct 2024 18:02:25 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> I'm going to ask a very basic question concerning this.
> 
> Isolation was present in PHYs early on when speeds were low, and thus
> electrical reflections weren't too much of a problem, and thus star
> topologies didn't have too much of an effect. A star topology is
> multi-drop. Even if the PCB tracks go from MAC to PHY1 and then onto
> PHY2, if PHY2 is isolated, there are two paths that the signal will
> take, one to MAC and the other to PHY2. If there's no impediance match
> at PHY2 (e.g. because it's in high-impedance mode) then that
> transmission line is unterminated, and thus will reflect back towards
> the MAC.
> 
> As speeds get faster, then reflections from unterminated ends become
> more of an issue.
> 
> I suspect the reason why e.g. 88x3310, 88E1111 etc do not support
> isolate mode is because of this - especially when being used in
> serdes mode, the topology is essentially point-to-point and any
> side branches can end up causing data corruption.

I suspect indeed that this won't work on serdes interfaces. I didn't
find any reliable information on that, but so far the few PHYs I've
seen seem to work that way.

The 88e1512 supports that, but I was testing in RGMII.

> 
> So my questions would be, is adding support for isolation mode in
> PHYs given todays network speeds something that is realistic, and
> do we have actual hardware out there where there is more than one
> PHY in the bus. If there is, it may be useful to include details
> of that (such as PHY interface type) in the patch series description.

I do have some hardware with this configuration (I'd like to support
that upstream, the topology work was preliminary work for that, and the
next move would be to send an RFC for these topolopgies exactly).

I am working with 3 different HW platforms with this layout :

      /--- PHY
      |
MAC  -|  phy_interface_mode == MII so, 100Mbps Max.
      |
      \--- PHY

and another that is similar but with RMII. I finally have one last case
with MII interface, same layout, but the PHYs can't isolate so we need
to make sure all but one PHYs are powered-down at any given time.

I will include that in the cover.

Could we consider limiting the isolation to non-serdes interfaces ?
that would be :

 - MII
 - RMII
 - GMII
 - RGMII and its -[TX|RX] ID flavours
 - TBI and RTBI ?? (I'm not sure about these)

Trying to isolate a PHY that doesn't have any of the interfaces above
would result in -EOPNOTSUPP ?

Thanks,

Maxime

On Mon, Oct 07, 2024 at 12:25:13PM +0200, Maxime Chevallier wrote:
> Hello Russell
> 
> On Fri, 4 Oct 2024 18:02:25 +0100
> "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> 
> > I'm going to ask a very basic question concerning this.
> > 
> > Isolation was present in PHYs early on when speeds were low, and thus
> > electrical reflections weren't too much of a problem, and thus star
> > topologies didn't have too much of an effect. A star topology is
> > multi-drop. Even if the PCB tracks go from MAC to PHY1 and then onto
> > PHY2, if PHY2 is isolated, there are two paths that the signal will
> > take, one to MAC and the other to PHY2. If there's no impediance match
> > at PHY2 (e.g. because it's in high-impedance mode) then that
> > transmission line is unterminated, and thus will reflect back towards
> > the MAC.
> > 
> > As speeds get faster, then reflections from unterminated ends become
> > more of an issue.
> > 
> > I suspect the reason why e.g. 88x3310, 88E1111 etc do not support
> > isolate mode is because of this - especially when being used in
> > serdes mode, the topology is essentially point-to-point and any
> > side branches can end up causing data corruption.
> 
> I suspect indeed that this won't work on serdes interfaces. I didn't
> find any reliable information on that, but so far the few PHYs I've
> seen seem to work that way.
> 
> The 88e1512 supports that, but I was testing in RGMII.

Looking at 802.3, there is no support for isolation in the clause 45
register set - the isolate bit only appears in the clause 22 BMCR.
Clause 22 registers are optional for clause 45 PHYs.

My reading of this is that 802.3 has phased out isolation support on
the MII side of the PHY on more modern PHYs, so this seems to be a
legacy feature.

Andrew has already suggested that we should default to isolate not
being supported - given that it's legacy, I agree with that.

> > So my questions would be, is adding support for isolation mode in
> > PHYs given todays network speeds something that is realistic, and
> > do we have actual hardware out there where there is more than one
> > PHY in the bus. If there is, it may be useful to include details
> > of that (such as PHY interface type) in the patch series description.
> 
> I do have some hardware with this configuration (I'd like to support
> that upstream, the topology work was preliminary work for that, and the
> next move would be to send an RFC for these topolopgies exactly).
> 
> I am working with 3 different HW platforms with this layout :
> 
>       /--- PHY
>       |
> MAC  -|  phy_interface_mode == MII so, 100Mbps Max.
>       |
>       \--- PHY
> 
> and another that is similar but with RMII. I finally have one last case
> with MII interface, same layout, but the PHYs can't isolate so we need
> to make sure all but one PHYs are powered-down at any given time.

You have given further details in other response to Andrew. I'll
comment further there.

> I will include that in the cover.

Yes, it would be good to include all these details in the cover message
so that it isn't spread out over numerous replies.

> Could we consider limiting the isolation to non-serdes interfaces ?
> that would be :
> 
>  - MII
>  - RMII
>  - GMII
>  - RGMII and its -[TX|RX] ID flavours
>  - TBI and RTBI ?? (I'm not sure about these)
> 
> Trying to isolate a PHY that doesn't have any of the interfaces above
> would result in -EOPNOTSUPP ?

I think the question should be: which MII interfaces can electrically
support multi-drop setups.

22.2.4.1.6 describes the Clause 22 Isolate bit, which only suggests
at one use case - for a PHY connected via an 802.3 defined connector
which shall power up in isolated state "to avoid the possibility of
having multiple MII output drivers actively driving the same signal
path simultaneously". This connector only supports four data signals
in each direction, which precludes GMII over this defined connector.

However, it talks about isolating the MII and GMII signals in this
section.

Putting that all together, 802.3 suggests that it is possible to
have multiple PHYs on a MII or GMII (which in an explanatory note
elsewhere, MII means 100Mb/s, GMII for 1Gb/s.) However, it is
vague.

Now... I want to say more, but this thread is fragmented and the
next bit of the reply needs to go elsewhere in this thread,
which is going to make reviewing this discussion later on rather
difficult... but we're being drip-fed the technical details.


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

Hi Russell,

On Mon, 7 Oct 2024 16:53:32 +0100
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Mon, Oct 07, 2024 at 12:25:13PM +0200, Maxime Chevallier wrote:
> > Hello Russell
> > 
> > On Fri, 4 Oct 2024 18:02:25 +0100
> > "Russell King (Oracle)" <linux@armlinux.org.uk> wrote:
> >   
> > > I'm going to ask a very basic question concerning this.
> > > 
> > > Isolation was present in PHYs early on when speeds were low, and thus
> > > electrical reflections weren't too much of a problem, and thus star
> > > topologies didn't have too much of an effect. A star topology is
> > > multi-drop. Even if the PCB tracks go from MAC to PHY1 and then onto
> > > PHY2, if PHY2 is isolated, there are two paths that the signal will
> > > take, one to MAC and the other to PHY2. If there's no impediance match
> > > at PHY2 (e.g. because it's in high-impedance mode) then that
> > > transmission line is unterminated, and thus will reflect back towards
> > > the MAC.
> > > 
> > > As speeds get faster, then reflections from unterminated ends become
> > > more of an issue.
> > > 
> > > I suspect the reason why e.g. 88x3310, 88E1111 etc do not support
> > > isolate mode is because of this - especially when being used in
> > > serdes mode, the topology is essentially point-to-point and any
> > > side branches can end up causing data corruption.  
> > 
> > I suspect indeed that this won't work on serdes interfaces. I didn't
> > find any reliable information on that, but so far the few PHYs I've
> > seen seem to work that way.
> > 
> > The 88e1512 supports that, but I was testing in RGMII.  
> 
> Looking at 802.3, there is no support for isolation in the clause 45
> register set - the isolate bit only appears in the clause 22 BMCR.
> Clause 22 registers are optional for clause 45 PHYs.
> 
> My reading of this is that 802.3 has phased out isolation support on
> the MII side of the PHY on more modern PHYs, so this seems to be a
> legacy feature.
> 
> Andrew has already suggested that we should default to isolate not
> being supported - given that it's legacy, I agree with that.
> 
> > > So my questions would be, is adding support for isolation mode in
> > > PHYs given todays network speeds something that is realistic, and
> > > do we have actual hardware out there where there is more than one
> > > PHY in the bus. If there is, it may be useful to include details
> > > of that (such as PHY interface type) in the patch series description.  
> > 
> > I do have some hardware with this configuration (I'd like to support
> > that upstream, the topology work was preliminary work for that, and the
> > next move would be to send an RFC for these topolopgies exactly).
> > 
> > I am working with 3 different HW platforms with this layout :
> > 
> >       /--- PHY
> >       |
> > MAC  -|  phy_interface_mode == MII so, 100Mbps Max.
> >       |
> >       \--- PHY
> > 
> > and another that is similar but with RMII. I finally have one last case
> > with MII interface, same layout, but the PHYs can't isolate so we need
> > to make sure all but one PHYs are powered-down at any given time.  
> 
> You have given further details in other response to Andrew. I'll
> comment further there.
> 
> > I will include that in the cover.  
> 
> Yes, it would be good to include all these details in the cover message
> so that it isn't spread out over numerous replies.
> 
> > Could we consider limiting the isolation to non-serdes interfaces ?
> > that would be :
> > 
> >  - MII
> >  - RMII
> >  - GMII
> >  - RGMII and its -[TX|RX] ID flavours
> >  - TBI and RTBI ?? (I'm not sure about these)
> > 
> > Trying to isolate a PHY that doesn't have any of the interfaces above
> > would result in -EOPNOTSUPP ?  
> 
> I think the question should be: which MII interfaces can electrically
> support multi-drop setups.
> 
> 22.2.4.1.6 describes the Clause 22 Isolate bit, which only suggests
> at one use case - for a PHY connected via an 802.3 defined connector
> which shall power up in isolated state "to avoid the possibility of
> having multiple MII output drivers actively driving the same signal
> path simultaneously". This connector only supports four data signals
> in each direction, which precludes GMII over this defined connector.
> 
> However, it talks about isolating the MII and GMII signals in this
> section.
> 
> Putting that all together, 802.3 suggests that it is possible to
> have multiple PHYs on a MII or GMII (which in an explanatory note
> elsewhere, MII means 100Mb/s, GMII for 1Gb/s.) However, it is
> vague.

Yes it's vague, as as testing showed, vendors are pretty liberal with
how/if they implement this feature :(

> Now... I want to say more, but this thread is fragmented and the
> next bit of the reply needs to go elsewhere in this thread,
> which is going to make reviewing this discussion later on rather
> difficult... but we're being drip-fed the technical details.

TBH I wasn't expecting this series on isolation to be the place to
discuss the multiplexing use-cases, hence why I didn't include a full
descriptin of every setup I have in the cover.

Given what Andrew replied, this whole series on controling isolation
from userspace isn't relevant.

Let me start a proper discussion thread and summarize what has been
said so far.

Maxime

> Looking at 802.3, there is no support for isolation in the clause 45
> register set - the isolate bit only appears in the clause 22 BMCR.
> Clause 22 registers are optional for clause 45 PHYs.

That was also an observation i had, the code goes straight to C22
operations, without even considering if the PHY has phydev->is_c45 is
set.  I think we need an op for this, which will default to NULL. The
driver can then opt-in by using the genphy function, or its own driver
method.

But lets get the big picture understood first, before we focus on the
details.

	Andrew