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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

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>;
		};

I'm not sure that this matters because they probably don't actually
implement mode changes as they're dedicated ethernet phys. The zynmp phy
seems to only be used for !ethernet devices. The phys I'm aware of being
used with mpfs are all also dedicated ethernet ones. Calling
phy_set_mode_ext() seems to be to be the right decision, but the rationale
confused me a little.

Cheers,
Conor.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]