Re: [PATCH v14 00/15] phy: Add support for Lynx 10G SerDes

[Sean Anderson <sean.anderson@xxxxxxxx>]

On 8/21/23 08:49, Vladimir Oltean wrote:
> Hi Sean,
> 
> On Fri, Aug 11, 2023 at 07:36:37PM +0300, Vladimir Oltean wrote:
>> Let me explain that approach, because your mention of "swapping out the
>> bootloaders" makes it appear as if you are not visualising what I am
>> proposing.
>> 
>> The Lynx SerDes family has 2 PLLs, and more lanes (4 or 8). Each lane
>> uses one PLL or the other, to derive its protocol frequency. Through the
>> RCW, you provision the 2 PLL frequencies that may be used by the lanes
>> at runtime.
>> 
>> The Lynx 28G SerDes driver reads the PLL frequencies in
>> lynx_28g_pll_read_configuration(), and determines the interface modes
>> supportable by each PLL (this is used by phylink). But it never changes
>> those PLL frequencies, since that operation is practically impossible in
>> the general sense (PLLs are shared by multiple lanes, so changing a PLL
>> frequency disrupts all lanes that use it).
> 
> Is my high-level feedback clear and actionable to you? I am suggesting
> to keep the look and feel the same between the lynx-10g and lynx-28g
> drivers, and to not use "fsl,type" protocols listed in the device tree
> as the immutable source of information for populating mode->protos, but
> instead the current PLL frequency configuration. So this implies that I
> am requesting that the dt-bindings should not contain a listing of the
> supported protocols.

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.
- We can have one compatible for each SoC, and determine the serdes
  based on the address. I would like to avoid this...
- 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.

I'm open to any other suggestions you have, but this was the best way I
could figure out to get this information to the driver in a way which
would be acceptable to the devicetree folks.

--Sean