On 6/12/23 12:33, Vladimir Oltean wrote:

On Mon, Jun 12, 2023 at 10:35:21AM -0400, Sean Anderson wrote:

quoted

quoted

And if SERDES protocol switching was not physically possible, would this
patch set still have value?

More to the point, did you make any progress in this area?

quoted

Yes. To e.g. set up SGMII25 or to fix the clocking situation.

Let me analyze the reasons one by one.

The clocking situation only exists due to a hope that protocol changing
would be possible. If you were sure it wasn't possible, your board would
have had refclks set up for protocol 3333 via the supported PLL mapping 2222.
In essence, I consider this almost a non-argument, as it fixes a
self-inflicted problem.

2222 also does not work, as outlined above.

The clocking is incompatible, and must be fixed by software.

The only thing self-inflicted is NXP's conflicting design.

Have you actually tested changing individual lanes from SGMII to SGMII 2.5?
I know it works on LS1028A, but I don't know if that's also true on LS1046A.
Your comments do say "LYNX_PROTO_SGMII25, /* Not tested */".

Not yet. 

This driver would also be a good place to add the KR link training with
NXP tried to upstream a few years ago.

Assuming a start from SERDES protocol 3333 with PLL mapping 2222,
this protocol change implies powering on PLL 1 (reset state machine
wants it to be disabled) and moving those lanes from PLL 2 to PLL 1.

At first sight you might appear to have a point related to the fact that
PLL register writes are necessary, and thus this whole shebang is necessary.
But this can all be done using PBI commands, with the added benefit that
U-Boot can also use those SERDES networking ports, and not just Linux.
You can use the RCW+PBL specific to your board to inform the SoC that
your platform's refclk 1 is 156 MHz (something which the reset state
machine seems unable to learn, with protocol 0x3333). You don't have to
put that in the device tree. You don't have to push code to any open
source project to expose your platform specific details. Then, just like
in the case of the Lynx 28G driver on LX2160, the SERDES driver could
just treat the PLL configuration as read-only, which would greatly
simplify things and eliminate the need for a clk driver.

Here is an illustrative example (sorry, I don't have a board with the
right refclk on that PLL, to verify all the way):

... snip ...

(which of course complicates the process of building the PBIs...)

In short, I believe the reasons you have cited do not justify the
complexity of the solution that you propose.

The work is done, and it is certainly more flexible than what you
propose. E.g. imagine a clock which needs to be configured before it has
the correct frequency. Such a thing is difficult to do in a PBI solution,
but is trivial in Linux.

--Sean