Hello Rob, Thanks for your review! On Fri, 9 Feb 2024 14:43:49 +0000 Rob Herring <robh@xxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx> > > 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