Re: [PATCH net-next v3 5/5] net: macb: Add "mobileye,eyeq5-gem" compatible

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Thu, Feb 26, 2026 at 08:20:41PM +0000, Conor Dooley wrote:
> On Thu, Feb 26, 2026 at 07:24:47PM +0000, Russell King (Oracle) wrote:
> > On Thu, Feb 26, 2026 at 10:46:24AM +0000, Conor Dooley wrote:
> > > On Thu, Oct 23, 2025 at 06:22:55PM +0200, Théo Lebrun wrote:
> > > > Add support for the two GEM instances inside Mobileye EyeQ5 SoCs, using
> > > > compatible "mobileye,eyeq5-gem". With it, add a custom init sequence
> > > > that must grab a generic PHY and initialise it.
> > > > 
> > > > We use bp->phy in both RGMII and SGMII cases. Tell our mode by adding a
> > > > phy_set_mode_ext() during macb_open(), before phy_power_on(). We are
> > > > the first users of bp->phy that use it in non-SGMII cases.
> > > > 
> > > > The phy_set_mode_ext() call is made unconditionally. It cannot cause
> > > > issues on platforms where !bp->phy or !bp->phy->ops->set_mode as, in
> > > > those cases, the call is a no-op (returning zero). From reading
> > > > upstream DTS, we can figure out that no platform has a bp->phy and a
> > > > PHY driver that has a .set_mode() implementation:
> > > >  - cdns,zynqmp-gem: no DTS upstream.
> > > >  - microchip,mpfs-macb: microchip/mpfs.dtsi, &mac0..1, no PHY attached.
> > > >  - xlnx,versal-gem: xilinx/versal-net.dtsi, &gem0..1, no PHY attached.
> > > >  - xlnx,zynqmp-gem: xilinx/zynqmp.dtsi, &gem0..3, PHY attached to
> > > >    drivers/phy/xilinx/phy-zynqmp.c which has no .set_mode().
> > > 
> > > Ran into this patch while looking at other stuff. Theo could you explain
> > > this analysis to someone not really au fait with phys? Looking at
> > > soc.dtsi files won't show you phys, since that's a board level decision,
> > > but you have found one for the zynqmp-gem so I guess that's just the way
> > > you presented the data?
> > > mpfs definitely has phys attached, so is you not finding one for it but
> > > finding for zynqmp, an indication that you were only looking for rgmii
> > > phys? Also, is the analysis of the connected phy driver accurate for
> > > zynmqmp?
> > > zynqmp-zc1751-xm018-dc4.dts seems to have 4 ethernet phys:
> > > 		ethernet_phy0: ethernet-phy@0 { /* Marvell 88e1512 */
> > > 			reg = <0>;
> > > 		};
> > > 		ethernet_phy7: ethernet-phy@7 { /* Vitesse VSC8211 */
> > > 			reg = <7>;
> > > 		};
> > > 		ethernet_phy3: ethernet-phy@3 { /* Realtek RTL8211DN */
> > > 			reg = <3>;
> > > 		};
> > > 		ethernet_phy8: ethernet-phy@8 { /* Vitesse VSC8211 */
> > > 			reg = <8>;
> > > 		};
> > 
> > Ethernet PHYs (drivers/net/phy/) are different from generic PHYs
> > (drivers/phy/). Ethernet PHYs are completely different beast with a
> > completely separate subsystem, which doesn't have a "set_mode" method.
> > 
> > Théo is referring to generic PHYs not Ethernet PHYs.
> 
> Right, that's pretty much what I figured and cos of that the patch
> itself seemed like it was fine to me. It is the analysis of users in
> devicetrees that I don't understand - the "no PHY attached" bits
> seemed to me like they should be saying "ethernet-only PHY attached, so
> no .set_mode()". Ultimately, I think it makes no difference to the patch
> itself, I just wanted to understand the commit message.

I think you're still thinking that ethernet PHYs and generic PHYs are
the same, but only differ in e.g. whether .set_mode() is populated.
This is completely wrong.

Ethernet PHYs are _completely_ separate from Generic PHYs. Forget the
"PHY" bit, that's totally irrelevant that they happen to be called
the same. The two classes share _no_ common infrastructure in the
kernel.

Ethernet PHYs can't have phy_set_mode_ext() called on them, because
ethernet PHYs have no "struct phy" associated with them. They also
can't have phy_power_on() either for the same reason, nor any of the
other generic PHY API calls.

Ethernet PHYs aren't a sub-class of generic PHYs. As I said,
Ethernet PHYs and generic PHYs are two entirely separate and different
subsystems with no commonality.

So... the commit message states "generic PHY". It states the generic
PHY API calls of phy_set_mode_ext() and phy_power_up(). From that we
deduce that bp->phy is a generic PHY. Thus, the commit message is
talking about generic PHYs not ethernet PHYs.

Ethernet PHYs are described in Ethernet controller DT using the
"phy-handle" property (and previously "phy" or "phy-device"

Generic PHYs are described using the "phys" property.

The presence of an ethernet-phy node in DT does not automatically
associate that device with anything - it has to be referenced by
a "phy-handle" property to connect it to an Ethernet controller.

However, as I've been trying to point out, as this commit is about
generic PHYs, Ethernet PHYs are just not relevant to this commit.

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