On Mon, Aug 21, 2023 at 02:46:53PM -0400, Sean Anderson wrote:

On 8/21/23 14:13, Ioana Ciornei wrote:

quoted

On Mon, Aug 21, 2023 at 01:45:44PM -0400, Sean Anderson wrote:

quoted

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@linaro.org/ (local)

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.

quoted

quoted

- 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@seco.com/ (local)

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>".

quoted

quoted

- 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel