netdev - Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240403111548.30e780b5@kmaincent-XPS-13-7390>
Date: Wed, 3 Apr 2024 11:15:48 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Rob Herring <robh@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni
 <pabeni@...hat.com>, Jonathan Corbet <corbet@....net>, Luis Chamberlain
 <mcgrof@...nel.org>, Russ Weight <russ.weight@...ux.dev>, Greg
 Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki"
 <rafael@...nel.org>, Krzysztof Kozlowski
 <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>,
 Oleksij Rempel <o.rempel@...gutronix.de>, Mark Brown <broonie@...nel.org>,
 Frank Rowand <frowand.list@...il.com>, Andrew Lunn <andrew@...n.ch>, Heiner
 Kallweit <hkallweit1@...il.com>, Russell King <linux@...linux.org.uk>,
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
 devicetree@...r.kernel.org, Dent Project <dentproject@...uxfoundation.org>
Subject: Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another
 way of describing several PSE PIs

On Tue, 2 Apr 2024 08:26:37 -0500
Rob Herring <robh@...nel.org> wrote:

> > +          pairset-names:
> > +            $ref: /schemas/types.yaml#/definitions/string-array
> > +            description:
> > +              Names of the pairsets as per IEEE 802.3-2022, Section
> > 145.2.4.
> > +              Valid values are "alternative-a" and "alternative-b". Each
> > name  
> 
> Don't state constraints in prose which are defined as schema 
> constraints.

Ok, I will remove the line.

> > +          pairsets:
> > +            $ref: /schemas/types.yaml#/definitions/phandle-array
> > +            description:
> > +              List of phandles, each pointing to the power supply for the
> > +              corresponding pairset named in 'pairset-names'. This property
> > +              aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4.
> > +              PSE Pinout Alternatives (as per IEEE 802.3-2022 Table
> > 145\u20133)
> > +
> > |-----------|---------------|---------------|---------------|---------------|
> > +              | Conductor | Alternative A | Alternative A | Alternative B
> > | Alternative B |
> > +              |           |    (MDI-X)    |     (MDI)     |      (X)
> > |      (S)      |
> > +
> > |-----------|---------------|---------------|---------------|---------------|
> > +              | 1         | Negative VPSE | Positive VPSE | \u2014
> >     | \u2014             |
> > +              | 2         | Negative VPSE | Positive VPSE | \u2014
> >     | \u2014             |
> > +              | 3         | Positive VPSE | Negative VPSE | \u2014
> >     | \u2014             |
> > +              | 4         | \u2014             | \u2014             |
> > Negative VPSE | Positive VPSE |
> > +              | 5         | \u2014             | \u2014             |
> > Negative VPSE | Positive VPSE |
> > +              | 6         | Positive VPSE | Negative VPSE | \u2014
> >     | \u2014             |
> > +              | 7         | \u2014             | \u2014             |
> > Positive VPSE | Negative VPSE |
> > +              | 8         | \u2014             | \u2014             |
> > Positive VPSE | Negative VPSE |
> > +            minItems: 1
> > +            maxItems: 2  
> 
> "pairsets" does not follow the normal design pattern of foos, foo-names, 
> and #foo-cells. You could add #foo-cells I suppose, but what would cells 
> convey? I don't think it's a good fit for what you need.
> 
> The other oddity is the number of entries and the names are fixed. That 
> is usually defined per consumer. 

Theoretically if the RJ45 port binding was supported it would make more sense,
but in reality it's not feasible as the PSE controller need this information
in its init process.
The PSE controller reset all its port to apply a configuration so we can't do
it when the consumer (RJ45) probe. It would reset the other ports if one
consumer is probed later in the process.

> As each entry is just a power rail, why can't the regulator binding be 
> used here?

Olekisj already answered about it.
PSE PI is like a regulator but with few different features and more information
like the pinout and the polarity, so we could not really fully rely on the
regulator binding style.

> > +
> > +          polarity-supported:
> > +            $ref: /schemas/types.yaml#/definitions/string-array
> > +            description:
> > +              Polarity configuration supported by the PSE PI pairsets.
> > +            minItems: 1
> > +            maxItems: 4
> > +            items:
> > +              enum:
> > +                - MDI-X
> > +                - MDI
> > +                - X
> > +                - S
> > +
> > +          vpwr-supply:
> > +            description: Regulator power supply for the PSE PI.  
> 
> I don't see this being used anywhere.

Right, I forgot to add it to the PD692x0 and TPS23881 binding example!

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com