Re: [PATCH net-next v2 7/9] net: phy: introduce ethtool_phy_ops to get and set phy configuration

[Maxime Chevallier <maxime.chevallier@bootlin.com>]

Hi Andrew,

On Mon, 7 Oct 2024 15:01:50 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

> > It seems I am missing details in my cover and the overall work I'm
> > trying to achieve.
> > 
> > This series focuses on isolating the PHY in the case where only one
> > PHY is attached to the MAC.  
> 
> I can understand implementing building blocks, but this patchset seems
> to be more than that, it seems to be a use case of its own. But is
> isolating a single PHY a useful use case? Do we want a kAPI for this?

That's a legit point. I mentioned in the cover for V1 that this in
itself doesn't really bring anything useful. The only point being that
it makes it easy to test if a PHY has a working isolation mode, but
given that we'll assume that it doesn't by default, that whole point
is moot.

I would therefore understand if you consider that having a kAPI for
that isn't very interesting and that I shall include this work as part
of the multi-PHY support.

> > I have followup work to support multi-PHY
> > interfaces. I will do my best to send the RFC this week so that you can
> > take a look. I'm definitely not saying the current code supports that.
> > 
> > To tell you some details, it indeed works as Russell says, I
> > detach/re-attach the PHYs, ndev->phydev is the "currently active" PHY.
> > 
> > I'm using a new dedicated "struct phy_mux" for that, which has :
> > 
> >  - Parent ops (that would be filled either by the MAC, or by phylink,
> > in the same spirit as phylink can be an sfp_upstream), which manages
> > PHY attach / detach to the netdev, but also the state-machine or the
> > currently inactive PHY.
> > 
> >  - multiplexer ops, that implement the switching logic, if any (drive a
> > GPIO, write a register, this is in the case of real multiplexers like
> > we have on some of the Turris Omnia boards, which the phy_mux framework
> > would support)
> > 
> >  - child ops, that would be hooks to activate/deactivate a PHY itself
> > (isoalte/unisolate, power-up/power-down).  
> 
> Does the kAPI for a single PHY get used, and extended, in this setup?

For isolation, no.

> 
> > I'll send the RFC ASAP, I still have a few rough edges that I will
> > mention in the cover.
> >   
> > > However, I still want to hear whether multiple PHYs can be on the same
> > > MII bus from a functional electrical perspective.  
> > 
> > Yup, I have that hardware.  
> 
> Can you talk a bit more about that hardware? What PHYs do you have?
> What interface modes are they using?

Sure thing. There are multiple devices out-there that may have multiple
PHYs accessible from the MAC, through muxers (I'm trying to be generic
enough to address all cases, gpio muxers, mmio-controlled muxers, etc.),
but let me describe the HW I'm working on that's a bit more problematic.

The first such platform I have has an fs_enet MAC, a pair of LXT973
PHYs for which the isolate mode doesn't work, and no on-board circuitry to
perform the isolation. Here, we have to power one PHY down when unused :

                /--- LXT973
fs_enet -- MII--|
                \--- LXT973


The second board has a fs_enet MAC and a pair of KSZ8041 PHYs connected
in MII.

The third one has a pair of KSZ8041 PHYs connected to a
ucc_geth MAC in RMII.

On both these boards, we isolate the PHYs when unused, and we also
drive a GPIO to toggle some on-board circuitry to disconnect the MII
lines as well for the unused PHY. I'd have to run some tests to see if
this circuitry could be enough, without relying at all on PHY
isolation :

                   /--- KSZ8041
                   |
      MAC ------ MUX
                 | | 
  to SoC <-gpio--/ \--- KSZ8041


One point is, if you look at the first case (no mux), we need to know
if the PHYs are able to isolate or not in order to use the proper
switching strategy (isolate or power-down).

I hope this clarifies the approach a little bit ?

Thanks,

Maxime