| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <2b1f112e-d533-46ae-a9a0-e5874c35c1fc@solid-run.com>
Date: Mon, 8 Sep 2025 14:02:35 +0000
From: Josua Mayer <josua@...id-run.com>
To: Vladimir Oltean <vladimir.oltean@....com>, Conor Dooley
<conor@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>
CC: Krzysztof Kozlowski <krzk@...nel.org>, "linux-phy@...ts.infradead.org"
<linux-phy@...ts.infradead.org>, Ioana Ciornei <ioana.ciornei@....com>, Vinod
Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Rob Herring
<robh@...nel.org>, Conor Dooley <conor+dt@...nel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>
Subject: Re: [PATCH phy 13/14] dt-bindings: phy: lynx-28g: add compatible
strings per SerDes and instantiation
Am 08.09.25 um 11:37 schrieb Vladimir Oltean:
> On Fri, Sep 05, 2025 at 08:02:59PM +0100, Conor Dooley wrote:
>> On Fri, Sep 05, 2025 at 06:41:50PM +0300, Vladimir Oltean wrote:
>>> On Fri, Sep 05, 2025 at 10:29:33AM +0200, Krzysztof Kozlowski wrote:
>>>>> properties:
>>>>> compatible:
>>>>> - enum:
>>>>> - - fsl,lynx-28g
>>>>> + oneOf:
>>>>> + - items:
>>>>> + - const: fsl,lynx-28g
>>>> Don't change that part. Previous enum was correct. You want oneOf and
>>>> enum.
>>> Combining the feedback from Conor and Josua, I should only be permitting
>>> the use of "fsl,lynx-28g" as a fallback to "fsl,lx216{0,2}a-serdes{1,2}",
>>> or standalone. The description below achieves just that. Does it look ok
>>> to you?
>>>
>>> properties:
>>> compatible:
>>> oneOf:
>>> - enum:
>>> - fsl,lx2160a-serdes1
>>> - fsl,lx2160a-serdes2
>>> - fsl,lx2160a-serdes3
>>> - fsl,lx2162a-serdes1
>>> - fsl,lx2162a-serdes2
>>> - const: fsl,lynx-28g
>>> deprecated: true
>>> - items:
>>> - const: fsl,lx2160a-serdes1
>>> - const: fsl,lynx-28g
>>> deprecated: true
>>> - items:
>>> - const: fsl,lx2160a-serdes2
>>> - const: fsl,lynx-28g
>>> deprecated: true
>>> - items:
>>> - const: fsl,lx2162a-serdes1
>>> - const: fsl,lynx-28g
>>> deprecated: true
>>> - items:
>>> - const: fsl,lx2162a-serdes2
>>> - const: fsl,lynx-28g
>>> deprecated: true
>> This doesn't really make sense, none of these are currently in use
>> right? Everything is just using fsl,lynx-28g right?
>> Adding new stuff and immediately marking it deprecated is a
>> contradiction, just don't add it at all if you don't want people using
>> it. Any users of it would be something you're going to retrofit in now,
>> so you may as well just retrofit to use what you want people to use
>> going forward, which has no fallbacks.
> You're right that it doesn't make sense to deprecate a newly introduced
> compatible string pair - my mistake for misunderstanding "deprecated".
>
>> I didn't read the back and forth with Josua (sorry!) but is the fallback
>> even valid? Do those devices have a common minimum set of features that
>> they share?
> I'll try to make an argument based on the facts presented below.
>
> The current Linux driver, which recognizes only "fsl,lynx-28g", supports
> only 1GbE and 10GbE. There are other SerDes protocols supported by the
> SerDes, but they are irrelevant for the purpose of discussing
> compatibility. Also, LX2160A SerDes #3 is also irrelevant, because it is
> not currently described in the device tree.
>
> 1GbE compatibility table
>
> SerDes Lane 0 Lane 1 Lane 2 Lane 3 Lane 4 Lane 5 Lane 6 Lane 7 Comments
> LX2160A SerDes #1 y y y y y y y y
> LX2160A SerDes #2 y y y y y y y y
> LX2162A SerDes #1 n/a n/a n/a n/a y y y y LX2162A currently uses lx2160a.dtsi
> LX2162A SerDes #2 y y y y y y y y LX2162A currently uses lx2160a.dtsi
>
> 10GbE compatibility table
>
> SerDes Lane 0 Lane 1 Lane 2 Lane 3 Lane 4 Lane 5 Lane 6 Lane 7 Comments
> LX2160A SerDes #1 y y y y y y y y
> LX2160A SerDes #2 n n n n n n y y
> LX2162A SerDes #1 n/a n/a n/a n/a y y y y LX2162A currently uses lx2160a.dtsi
> LX2162A SerDes #2 n n n n n n y y LX2162A currently uses lx2160a.dtsi
>
> As LX2160A SerDes #2 is treated like #1, the device tree is telling the
> driver that all lanes support 10GbE (which is false for lanes 0-5).
>
> If one of the SerDes PLLs happens to be provisioned for the 10GbE clock
> net frequency, as for example with the RCW[SRDS_PRTCL_S2]=6 setting,
> this will make the driver think that it can reconfigure lanes 0-5 as
> 10GbE.
>
> This will directly affect upper layers (phylink), which will advertise
> 10GbE modes to its link partner on ports which support only 1GbE, and
> the non-functional link mode might be resolved through negotiation, when
> a lower speed but functional link could have been established.
>
> You mention a common minimum feature set. That would be supporting 10GbE
> only on lanes 6-7, which would be disadvantageous to existing uses of
> 10GbE on lanes 0-5 of SerDes #1. In some cases, the change might also be
> breaking - there might be a PHY attached to these lanes whose firmware
> is hardcoded to expect 10GbE, so there won't be a graceful degradation
> to 1GbE in all cases.
>
> With Josua's permission, I would consider commit 2f2900176b44 ("arm64:
> dts: lx2160a: describe the SerDes block #2") as broken, for being an
> incorrect description of hardware - it is presented as identical to
> another device, which has a different supported feature set. I will not
> try to keep SerDes #2 compatible with "fsl,lynx-28g". This will remain
> synonymous only with SerDes #1. The users of the fsl-lx2162a-clearfog.dts
> will need updating if the "undetected lack of support for 10GbE" becomes
> an issue.
>
> My updated plan is to describe the schema rules for the compatible as
> follows. Is that ok with everyone?
>
> properties:
> compatible:
> oneOf:
> - const: fsl,lynx-28g
> deprecated: true
> - items:
> - const: fsl,lx2160a-serdes1
> - const: fsl,lynx-28g
> - enum:
> - fsl,lx2160a-serdes2
> - fsl,lx2160a-serdes3
> - fsl,lx2162a-serdes1
> - fsl,lx2162a-serdes2
Weak objection, I think this is more complex than it should be.
Perhaps it was discussed before to keep two compatible strings ...:
properties:
compatible:
items:
- enum:
- fsl,lx2160a-serdes2
- fsl,lx2160a-serdes3
- fsl,lx2162a-serdes1
- fsl,lx2162a-serdes2
- const: fsl,lynx-28g
This will cause the dtbs_check to complain about anyone in the future
using it wrong.
The driver can still probe on fsl,lynx-28g alone for backwards compatibility,
and you can limit the feature-set as you see fit in such case.
Main argument for always specifying lynx-28g is that the serdes blocks
do share a common programming model and register definitions.
>
> Also, I will limit the driver support for the "fsl,lynx-28g" compatible
> to just 1GbE and 10GbE. The 25GbE feature introduced by this series will
> require a more precise compatible string.
Okay
Powered by blists - more mailing lists