Re: [PATCH net-next 07/15] net: dsa: mt7530: do not run mt7530_setup_port5() if port 5 is disabled

[Arınç ÜNAL]

On 9.01.2024 17:57, Vladimir Oltean wrote:
> Yes, well _now_ it is a false positive, probably because smatch cannot
> determine that when priv->p5_intf_sel has been set to P5_INTF_SEL_PHY_P0
> or P5_INTF_SEL_PHY_P4, "interface" should have been also initialized.
> But it doesn't matter, you can ignore a false positive. I'm also seeing it.
> Although you should check whether treating -ENODEV as a hard error is fine
> and won't cause regressions.
>
> > Just so you know, I intend to remove this whole PHY muxing feature once I
> > bring changing DSA conduit support to this subdriver. I've got two strong
> > reasons for this.
> > - Changing the DSA conduit achieves the same result with the only overhead
> >    being the DSA header included on every frame.
> >
> > - There can't be proper dt-bindings for it as the nature of the feature
> >    shows that it represents an optional way to operate the hardware, it does
> >    not represent a hardware design. Overall, the implementation is a hack to
> >    make it work for specific hardware (switch must be connected to gmac1 of
> >    a MediaTek SoC, no PHY must be present at address 0 or 4 on the MDIO bus
> >    of the SoC. It should rather be configurable on userspace. Which will
> >    never happen as it is specific to this switch and the changing DSA
> >    conduit feature is the perfect substitute for this.
>
> Is PHY muxing a "true" switch bypass, or is it just a route through the
> switch for all packets coming from GMAC5 to go to phy0 or phy4? If the
> latter, I agree that dynamic conduit changing is a more flexible option,
> not to mention the user space tooling is already there.

It's the latter, and that's exactly what I think.

> Are there existing systems that use PHY muxing? The possible problem I
> see is breaking those boards which have a phy-handle on gmac5, if the
> mt7530 driver is no longer going to modify its HWTRAP register.

Ah see, for PHY muxing, the driver actually wants the phy-handle to be put
on the SoC MAC, and the PHY to be defined on the SoC ethernet's MDIO bus.
We don't even define gmac5 as a port on the switch dt-bindings.

While none of the DTs on the Linux repository utilise this, some of the
mt7621 DTs on OpenWrt do. The change in behaviour will only be that phy0/4
will be inaccessible from the SoC MAC's network interface. I de-facto
maintain the mt7621 device tree source files there. I intend to revert it
along with adding port 5 as a CPU port so that the conduit changing feature
becomes available.

> > Let me know if you've got any suggestions that can get rid of the warning
> > without reworking the whole code block. Otherwise, I'm just going to ignore
> > it until I get rid of the whole code block.
>
> The obvious way would be to leave the initialization to PHY_INTERFACE_MODE_NA
> there. Or to just ignore the warning.

I'll ignore.

Arınç