On Mon, Aug 21, 2023 at 02:46:53PM -0400, Sean Anderson wrote: > On 8/21/23 14:13, Ioana Ciornei wrote: > > On Mon, Aug 21, 2023 at 01:45:44PM -0400, Sean Anderson wrote: > >> Well, we have two pieces of information we need > >> > >> - What values do we need to program in the PCCRs to select a particular > >> mode? This includes whether to e.g. set the KX bits. > >> - Implied by the above, what protocols are supported on which lanes? > >> This is not strictly necessary, but will certainly solve a lot of > >> headscratching. > >> > >> This information varies between different socs, and different serdes on > >> the same socs. We can't really look at the RCW or the clocks and figure > >> out what we need to program. So what are our options? > >> > >> - We can have a separate compatible for each serdes on each SoC (e.g. > >> "fsl,lynx-10g-a"). This was rejected by the devicetree maintainers. I previously took this statement at face value and didn't further investigate. After a bit of digging through the first versions of this patch set it's evident that you left out a big piece of information. The devicetree maintainers have indeed rejected compatible strings of the "fsl,<soc-name>-serdes-<instance>" form but they also suggested to move the numbering to a property instead: https://lore.kernel.org/all/db9d9455-37af-1616-8f7f-3d752e7930f1@xxxxxxxxxx/ But instead of doing that, you chose to move all the different details that vary between SerDes blocks/SoCs from the driver to the DTS. I don't see that this was done in response to explicit feedback. > >> - We can have one compatible for each SoC, and determine the serdes > >> based on the address. I would like to avoid this... > > > > To me this really seems like a straightforward approach. > > Indeed it would be straightforward, but what's the point of having a > devicetree in the first place then? We could just go back to being a > (non-dt) platform device. > I am confused why you are now so adamant to have these details into the DTS. Your first approach was to put them into the driver, not the DTS: https://lore.kernel.org/netdev/20220628221404.1444200-5-sean.anderson@xxxxxxxx/ And this approach could still work now and get accepted by the device tree maintainers. The only change that would be needed is to add a property like "fsl,serdes-block-id = <1>". > >> - We can stick all the details which vary between serdes/socs into the > >> device tree. This is very flexible, since supporting new SoCs is > >> mostly a matter of adding a new compatible and writing a new > >> devicetree. On the other hand, if you have a bug in your devicetree, > >> it's not easy to fix it in the kernel. > >> - Just don't support protocol switching. The 28G driver does this, which > >> is why it only has one compatible. However, supporting protocol > >> switching is a core goal of this driver, so dropping support is not an > >> option. > >> Ioana
