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

[Sean Anderson <sean.anderson@xxxxxxxx>]

Hi Vladmir,

On 5/1/23 11:03, Sean Anderson wrote:
> On 4/29/23 13:24, Vladimir Oltean wrote:
>> On Wed, Apr 26, 2023 at 10:50:17AM -0400, Sean Anderson wrote:
>>> > I need to catch up with 14 rounds of patches from you and with the
>>> > discussions that took place on each version, and understand how you
>>> > responded to feedback like "don't remove PHY interrupts without finding
>>> > out why they don't work"
>>> 
>>> All I can say is that
>>> 
>>> - It doesn't work on my board
>>> - The traces are on the bottom of the PCB
>>> - The signal goes through an FPGA which (unlike the LS1046ARDB) is closed-source
>> 
>> I don't understand the distinction you are making here. Are the sources
>> for QIXIS bit streams public for any Layerscape board?
> 
> Correct. The sources for the LS1046ARDB QIXIS are available for download.
> 
>>> - The alternative is polling once a second (not terribly intensive)
>> 
>> It makes a difference to performance (forwarded packets per second), believe it or not.
> 
> I don't. Please elaborate how link status latency from the phy affects performance.
> 
>>> 
>>> I think it's very reasonable to make this change. Anyway, it's in a separate
>>> patch so that it can be applied independently.
>> 
>> Perhaps better phrased: "discussed separately"...
>> 
>>> > Even if the SERDES and PLL drivers "work for you" in the current form,
>>> > I doubt the usefulness of a PLL driver if you have to disconnect the
>>> > SoC's reset request signal on the board to not be stuck in a reboot loop.
>>> 
>>> I would like to emphasize that this has *nothing to do with this driver*.
>>> This behavior is part of the boot ROM (or something like it) and occurs before
>>> any user code has ever executed. The problem of course is that certain RCWs
>>> expect the reference clocks to be in certain (incompatible) configurations,
>>> and will fail the boot without a lock. I think this is rather silly (since
>>> you only need PLL lock when you actually want to use the serdes), but that's
>>> how it is. And of course, this is only necessary because I was unable to get
>>> major reconfiguration to work. In an ideal world, you could always boot with
>>> the same RCW (with PLL config matching the board) and choose the major protocol
>>> at runtime.
>> 
>> Could you please tell me what are the reference clock frequencies that
>> your board provides at boot time to the 2 PLLs, and which SERDES
>> protocol out of those 2 (1133 and 3333) boots correctly (no RESET_REQ
>> hacks necessary) with those refclks? I will try to get a LS1046A-QDS
>> where I boot from the same refclk + SERDES protocol configuration as
>> you, and use PBI commands in the RCW to reconfigure the lanes (PLL
>> selection and protocol registers) for the other mode, while keeping the
>> FRATE_SEL of the PLLs unmodified.
> 
>  From table 31-1 in the RM, the PLL mapping for 1133 is 2211, and the
>  PLL mapping for 3333 is 2222. As a consequence, for 1133, PLL 2 must be
>  156.25 MHz and PLL 1 must be either 100 or 125 MHz. And for 3333, PLL 2
>  must be either 100 or 125 MHz, and PLL 1 should be shut down (as it is
>  unused). This conflict for PLL 2 means that the same reference clock
>  configuration cannot work for both 1133 and 3333. In one of the
>  configurations, SRDS_RST_RR will be set in RSTRQSR1. On our board,
>  reference clock 1 is 156.25 MHz, and reference clock 2 is 125 MHz.
>  Therefore, 3333 will fail to boot. Unfortunately, this reset request
>  occurs before any user-configurable code has run (except the RCW), so
>  it is not possible to fix this issue with e.g. PBI.

Have you had a chance to review this driver?

--Sean