Re: [PATCH net-next v2 0/9] Allow isolating PHY devices

[Russell King (Oracle)]

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@xxxxxxxxxxx/
> 
> 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!