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

[Maxime Chevallier]

On Mon, 7 Oct 2024 18:37:29 +0200
Andrew Lunn <andrew@xxxxxxx> wrote:

> > 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.  
> 
> kAPI add a lot of Maintenance burden. So we should not add them unless
> they are justified. to me, there is not a good justification for this.

That's fine by me.

> 
> > 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  
> 
> So you have at least regulators under Linux control? Is that what you
> mean by power down? Pulling the plug and putting it back again is
> somewhat different to isolation. All its state is going to be lost,
> meaning phylib needs to completely initialise it again. Or can you
> hide this using PM? Just suspend/resume it?

Ah no, I wasn't referring to regulators but rather the BMCR PDOWN bit to
just shut the PHY down, as in suspend.

Indeed the state is lost. The way I'm supporting this is :

 - If one PHY has the link, it keeps it until link-down
 - When link-down, I round-robin between the 2 phys: 

  - Attach the PHY to the netdev
  - See if it can establish link and negotiate with LP
  - If there's nothing after a given period ( 2 seconds default ), then
I detach the PHY, attach the other one, and start again, until one of
them has link.

That's very limited indeed, we have no way of saying "first that has
link wins".


> > 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).  
> 
> That explains the hardware, but what are the use cases? How did the
> hardware designer envision this hardware being used?

The use-case is link redundancy, if one PHY loses the link, we hope
that we still have link on the other one and switchover. This is one of
the things I discussed at netdev 0x17.

> If you need to power the PHY off, you cannot have dynamic behaviour
> where the first to have link wins. But if you can have the media side
> functional, you can do some dynamic behaviours.

True.

> Although, is it wise
> for the link to come up, yet to be functionally dead because it has no
> MAC connected?

Good point. What would you think ? I already deal with the identified
issue which is that both PHYs are link-up with LP, both connected to
the same switch. When we switch between the active PHYs, we send a
gratuitous ARP on the new PHY to refresh the switch's FDB.

Do you see that as being an issue, having the LP see link-up when the
link cannot actually convey data ? Besides the energy detect feature
you mention, I don't see what other options we can have unfortunately :(

> There are some Marvell Switches which support both internal Copper
> PHYs and a SERDES port. The hardware allows first to get link to have
> a functional MAC. But in Linux we have not supported that, and we
> leave the unused part down so it does not get link.

My plan is to support these as well. For the end-user, it makes no
difference wether the HW internally has 2 PHYs each with one port, or 1
phy with 2 ports. So to me, if we want to support phy_mux, we should
also support the case you mention above. I have some code to support
this, but that's the part where I'm still getting things ironed-out,
this is pretty tricky to represent that properly, especially in DT.

>
> Maybe we actually want energy detect, not link, to decide which PHY
> should get the MAC?  But i have no real idea what you can do with
> energy detect, and it would also mean building out the read_status()
> call to report additional things, etc.

Note that I'm trying to support a bigger set of use-cases besides the
pure 2-PHY setup. One being that we have a MUX within the SoC on the
SERDES lanes, allowing to steer the MII interface between a PHY and an
SFP bus (Turris Omnia has such a setup). Is it possible to have an
equivalent "energy detect" on all kinds of SFPs ?

As a note, I do see that both Russell and you may think you're being
"drip-fed" (I learned that term today) information, that's not my
intent at all, I wasn't expecting this discussion now, sorry about that.

I was saying to Russell that I would start a new thread, but we already
have a discussion going here, let me know if we shall continue the
discussion here or on a new thread.

Thanks,

Maxime