On Wed, 24 Nov 2021 00:54:18 +0200 Vladimir Oltean <olteanv@xxxxxxxxx> 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