Subject: Re: [PATCH RFC net-next 5/5] net: dsa: always use phylink for CPU and DSA ports

From: Vladimir Oltean <[email-masked]>
Date: Thu, 7 Jul 2022 18:43:03 +0300

On Thu, Jul 07, 2022 at 12:00:54PM +0100, Russell King (Oracle) wrote:
> More importantly, we need your input on Ocelot, which you are listed as
> a maintainer for, and Ocelot is the only DSA driver that does stuff
> differently (due to the rate adapting PCS). It doesn't set
> mac_capabilities, and therefore phylink_set_max_fixed_link() will not
> work here.
> 
> Has Ocelot ever made use of this DSA feature where, when nothing is
> specified for a CPU or DSA port, we use an effective fixed-link setup
> with an interface mode that gives the highest speed? Or does this not
> apply to this DSA driver?
> 
> Thanks.

I'm fine with both the ocelot and sja1105 drivers.

The ocelot driver has 3 users:

- felix_vsc9959 (arch/arm64/boot/dts/freescale/fsl-ls1028a.dtsi) on NXP
  LS1028A, where the CPU ports have and have always had a fixed-link
  node in the SoC dtsi. LS1028A based boards should include the SoC
  dtsi. If other board DT writers don't do that or if they delete the
  fixed-link node from the CPU ports, that's not my problem and I don't
  really want to help them.

- seville_vsc9953 (arch/powerpc/boot/dts/fsl/t1040si-post.dtsi) on NXP
  T1040. Same thing, embedded switch, not my fault if the fixed-link
  disappears from the SoC dtsi.

- Colin Foster's SPI-controlled VSC7512 (still downstream). He has an
  Ethernet cable connecting the CPU port to a Beaglebone Black, so he
  has a phy-handle on the CPU port, so definitely not nothing. I believe
  his work hasn't made it to production in any case, so enforcing
  validation now shouldn't bother him too much if at all.

As for sja1105, there is DT validation that checks for the presence of
all required properties in sja1105_parse_ports_node().

There is some DT validation in felix_parse_ports_node() too, but it
doesn't check that all specifiers that phylink might use are there.
I'd really like to add some validation before I gain any involuntary
users, but all open-coded constructs I can come up with are clumsy.
What would you suggest, if I explicitly don't want to rely on
context-specific phylink interpretation of empty OF nodes, and rather
error out?