| 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: <20250908093709.owcha6ypm5lqqdwz@skbuf>
Date: Mon, 8 Sep 2025 12:37:09 +0300
From: Vladimir Oltean <vladimir.oltean@....com>
To: Conor Dooley <conor@...nel.org>, Josua Mayer <josua@...id-run.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>
Cc: Krzysztof Kozlowski <krzk@...nel.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, Rob Herring <robh@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org
Subject: Re: [PATCH phy 13/14] dt-bindings: phy: lynx-28g: add compatible
strings per SerDes and instantiation
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
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.
Powered by blists - more mailing lists