linux-kernel - Re: [PATCH net-next v3 10/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]

Date: Wed, 14 Feb 2024 14:13:10 +0100
From: Köry 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 v3 10/17] dt-bindings: net: pse-pd: Add another
 way of describing several PSE PIs

Hello Rob,

Thanks for your review!

On Fri, 9 Feb 2024 14:43:49 +0000
Rob Herring <robh@...nel.org> wrote:

> On Thu, Feb 08, 2024 at 02:08:47PM +0100, Kory Maincent wrote:
> > Before hand we set "#pse-cell" to 1 to define a PSE controller with  
> 
> #pse-cells
> 
> > several PIs (Power Interface). The drawback of this was that we could not
> > have any information on the PI except its number.  
> 
> Then increase it to what you need. The whole point of #foo-cells is that 
> it is variable depending on what the provider needs. 
> 
> > Add support for pse_pis and pse_pi node to be able to have more information
> > on the PI like the number of pairset used and the pairset pinout.  
> 
> Please explain the problem you are trying to solve, not your solution. I 
> don't understand what the problem is to provide any useful suggestions 
> on the design.

Please see Oleksij's reply.
Thank you Oleksij, for the documentation!!

> > 
> > Sponsored-by: Dent Project <dentproject@...uxfoundation.org>  
> 
> Is this a recognized tag? First I've seen it.

This is not a standard tag but it has been used several times in the past.

> >  
> > -required:
> > -  - "#pse-cells"
> > +  pse_pis:
> > +    $ref: "#/$defs/pse_pis"
> > +
> > +$defs:  
> 
> $defs is for when you need multiple copies of the same thing. I don't 
> see that here.

I made this choice for better readability but indeed it is used only once.
I will remove it then.

> > +  pse_pis:
> > +    type: object
> > +    description:
> > +      Kind of a matrix to identify the concordance between a PSE Power
> > +      Interface and one or two (PoE4) physical ports.
> > +
> > +    properties:
> > +      "#address-cells":
> > +        const: 1
> > +
> > +      "#size-cells":
> > +        const: 0
> > +
> > +    patternProperties:
> > +      "^pse_pi@[0-9]+$":  
> 
> Unit-addresses are hex.

Oops sorry for the mistake.

> 
> > +        $ref: "#/$defs/pse_pi"
> > +
> > +    required:
> > +      - "#address-cells"
> > +      - "#size-cells"
> > +
> > +  pse_pi:
> > +    description:
> > +      PSE PI device for power delivery via pairsets, compliant with IEEE
> > +      802.3-2022, Section 145.2.4. Each pairset comprises a positive and a
> > +      negative VPSE pair, adhering to the pinout configurations detailed in
> > +      the standard.
> > +    type: object
> > +    properties:
> > +      reg:
> > +        maxItems: 1  
> 
> As you are defining the addressing here, you need to define what the 
> "addresses" are.

Yes I will add some documentation in next version.

> > +          values are "alternative-a" and "alternative-b". Each name should
> > +          correspond to a phandle in the 'pairset' property pointing to the
> > +          power supply for that pairset.
> > +        $ref: /schemas/types.yaml#/definitions/string-array
> > +        minItems: 1
> > +        maxItems: 2
> > +        items:
> > +          - enum:
> > +            - "alternative-a"
> > +            - "alternative-b"  
> 
> This leaves the 2nd entry undefined. You need the dictionary form of 
> 'items' rather than a list. IOW, Drop the '-' under items.

Oh thanks! That is what I was looking for. I was struggling using the right
description.

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