Re: [PATCH net-next v2 4/8] net: phylink: update supported_interfaces with modes from fwnode

["Marek Behún" <kabel@kernel.org>]

On Wed, 24 Nov 2021 00:54:18 +0200
Vladimir Oltean <olteanv@gmail.com> wrote:

> > As I said above, the best example is with SerDeses which use multiple
> > lanes where not all may be wired. But this backward compatibility code
> > concerns only one-lane SerDeses: usxgmii, 10gbaser-r, 5gbase-r,
> > 2500base-x, 1000base-x, sgmii.  
> 
> Right, why so? From phy-mode = "xaui", you could also infer support for
> single-lane SERDES protocols up to 3.125 GHz, aren't you interested in
> doing that too?

OK, this would seem to make sense if I dropped the DT binding change
for now, and just updated the code to change the supported_interfaces
member so that marvell10g driver could select appropriate mode.

I will think about it. Thanks.

> > 2) the mode is not supported by the board because the generic PHY
> >    driver uses SMC calls to the firmware to change the SerDes mode, but
> >    the board may have old version of firmware which does not support
> >    SMC call correctly (or at all). This was a real issue with TF-A
> >    firmware on Macchiatobin, where the 5gbase-r mode was not supported
> >    in older versions  
> 
> Ok, so in your proposal, U-Boot would have to fix up the device tree
> based on a certain version of ATF, and remove "5gbase-r" from the
> phy-mode array. Whereas in my proposal, the mvpp2 driver would need to
> find out about the ATF version in use, and remove "5gbase-r" from the
> supported interfaces.
> 
> As a user, I'd certainly prefer if Linux could figure this one out.

You are right here, it would really be better for the mvpp2 driver to
do this discovering.

> This implies that when you bring up a board and write the device tree
> for it, you know that PHY mode X works without ever testing it. What if
> it doesn't work when you finally add support for it? Now you already
> have one DT blob in circulation. That's why I'm saying that maybe it
> could be better if we could think in terms that are a bit more physical
> and easy to characterize.

The thing is that this same could happen with your proposal of
max-data-rate + number-of-lanes, as Russell explained in his reply.

> > The whole idea of this code is to guarantee backward compatibility with
> > older device-trees. In my opinion (and I think Russell agrees with me),
> > this should be at one place, instead of putting various weird
> > exceptions into various MAC drivers.  
> 
> Yes, but they're more flexible in the driver... What if the check is not
> as simple as a machine compatible (think about the ATF firmware example
> you gave).

You persuaded me, it makes more sense in the driver.

Marek