On 8/22/23 10:55, Ioana Ciornei wrote:

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

quoted

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://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2fall%2fdb9d9455%2d37af%2d1616%2d8f7f%2d3d752e7930f1%40linaro.org%2f&umid=2d629417-3b95-49e4-8cdb-34737cc93582&auth=d807158c60b7d2502abde8a2fc01f40662980862-895c2dfe1c33719569d44ae2b51e21f626f39d39

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

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://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flore.kernel.org%2fnetdev%2f20220628221404.1444200%2d5%2dsean.anderson%40seco.com%2f&umid=2d629417-3b95-49e4-8cdb-34737cc93582&auth=d807158c60b7d2502abde8a2fc01f40662980862-64ea8fa45172282f676b7463a5401e8a7c5bdcbf

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

https://lore.kernel.org/linux-phy/1c2bbc12-0aa5-6d2a-c701-577ce70f7502@linaro.org/ (local)

Despite what he says in your link, I explicitly proposed doing exactly
that and he rejected it. I suspect that despite accusing me of
"twisting" the conversation, he did not clearly remember that
exchange...

That said, maybe we could do something like 

serdes: phy@1ea0000 {
	compatible = "fsl,ls1046a-serdes";
	reg = <0x1ea0000 0x1000>, <0x1eb0000 0x1000>;
};

--Sean

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