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

[Sean Anderson]

On 4/25/23 15:50, Vladimir Oltean wrote:
> Hello,
> 
> On Thu, 13 Apr 2023 12:05:52 -0400, Sean Anderson wrote:
>> This series is ready for review by the phy maintainers. I have addressed
>> all known feedback and there are no outstanding issues.
>> 
>> Major reconfiguration of baud rate (e.g. 1G->10G) does not work.
>> 
>> There are several stand-alone commits in this series. Please feel free
>> to pick them as appropriate. In particular, commits 1, 3, 4, 12, 13, and
>> 14 are all good candidates for picking.
>> 
>> - Although this is untested, it should support 2.5G SGMII as well as
>>   1000BASE-KX. The latter needs MAC and PCS support, but the former
>>   should work out of the box.
>> - It allows for clock configurations not supported by the RCW. This is
>>   very useful if you want to use e.g. SRDS_PRTCL_S1=0x3333 and =0x1133
>>   on the same board. This is because the former setting will use PLL1
>>   as the 1G reference, but the latter will use PLL1 as the 10G
>>   reference. Because we can reconfigure the PLLs, it is possible to
>>   always use PLL1 as the 1G reference.
> 
> I am an engineer working for NXP and I have access to the majority of
> hardware that includes the Lynx 10G SERDES, as well as to block guides
> that are not visible to customers, and to people from the hardware
> design and validation teams.
> 
> I have an interest in adding a driver for this SERDES to support dynamic
> Ethernet protocol reconfiguration, and perhaps the internal PHY for
> copper backplane modes (1000Base-KX, 10GBase-KR) and its link training
> procedure - although the latter will need more work.
> 
> I would like to thank you for starting the effort, but please, stop
> submitting code for modes that are untested, and do not submit features
> that do not work. 

The features which do not work (major protocol changes) are disabled :)

If it would cause this series to be immediately merged, I would remove
KX/KR and 2.5G which are the only untested link modes.

That said, PCS support is necessary for these modes, so it is not even
possible to select them.

> If you do not have the time to fix up the loose ends
> for this patch submission now

I have time to fix up any loose ends preventing this series from being
applied. However, I am not very sympathetic to larger requests, since
there has been extensive time to supply feedback already.

> , you won't have the time to debug
> regressions on boards you might not even have access to, which worked
> fine previously due to the static RCW/PBL configuration.

I have gotten no substantive feedback on this driver. I have been
working on this series since June last year, and no one has commented on
the core algotithms thus far. My capacity for making large changes has
decreased because the project funding the development has ended. I
appreciate that you are taking interest now, but frankly I think it is
rather past time...

> I have downloaded your patches, and although I have objections to some
> of them already, I will be in the process of evaluating, testing,
> changing them, for the coming weeks, perhaps even more. Please consider
> this a NACK for the current patch set due to the SERDES related material,
> although the unrelated patches (like "dt-bindings: Convert gpio-mmio to
> yaml") can and should have been submitted separately, so they can be
> analyzed by their respective maintainers based on their own merit and
> not as part of an overall problematic set.

This patchset has been ready to merge for several revisions now. I do
not consider it problematic. However, I do consider the (nonexistant)
review process for this subsystem extremely problematic.

--Sean