On 11/08/2025 18:13, Gregory Fuchedgi via B4 Relay wrote: > From: Gregory Fuchedgi <gfuchedgi@xxxxxxxxx> > > Update schema after per-port poe class restrictions and a few other options > were implemented. A nit, subject: drop second/last, redundant "bindings". The "dt-bindings" prefix is already stating that these are bindings. See also: https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18 > > Signed-off-by: Gregory Fuchedgi <gfuchedgi@xxxxxxxxx> > --- > .../devicetree/bindings/hwmon/ti,tps23861.yaml | 86 ++++++++++++++++++++++ > 1 file changed, 86 insertions(+) > > diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml > index ee7de53e19184d4c3df7564624532306d885f6e4..578f4dad7eab630b218e9e30b23fc611a760d332 100644 > --- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml > +++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml > @@ -24,12 +24,62 @@ properties: > reg: > maxItems: 1 > > + '#address-cells': > + const: 1 > + > + '#size-cells': > + const: 0 > + > shunt-resistor-micro-ohms: > description: The value of current sense resistor in microohms. > default: 255000 > minimum: 250000 > maximum: 255000 > > + reset-gpios: > + description: Optional GPIO for the reset pin. > + maxItems: 1 > + > + shutdown-gpios: powerdown-gpios, see gpio-consumer-common.yaml > + description: | Drop | > + Optional GPIO for the shutdown pin. Used to prevent PoE activity before > + the driver had a chance to configure the chip. > + maxItems: 1 > + > + interrupts: > + description: | > + The interrupt specifier. Only required if PoE class is restricted to less Drop first sentence, redundant. Interrupts property cannot be anything else than interrupt specifier. > + than class 4 in the device tree. > + maxItems: 1 > + > +patternProperties: > + "^port@[0-3]$": This goes to ports property. > + type: object > + description: Port specific nodes. > + unevaluatedProperties: false > + required: > + - reg required goes to the end. > + > + properties: > + reg: > + description: Port index. > + items: > + minimum: 0 Drop minimum. > + maximum: 3 > + > + class: > + description: The maximum power class a port should accept. What are the values? Where is the property defined - which schema - that you do not use vendor prefix? > + $ref: /schemas/types.yaml#/definitions/uint32 > + minimum: 0 Drop minimum. > + maximum: 4 > + > + off-by-default: Same question - which common schema defines this? > + description: Indicates the port is off by default. > + type: boolean > + > + label: > + description: Optional port label Skip all "optional" here and other places. Schema tells it, not free form text. Say something useful here or just ": true". > + > required: > - compatible > - reg > @@ -51,3 +101,39 @@ examples: > shunt-resistor-micro-ohms = <255000>; > }; > }; > + - | > + #include <dt-bindings/gpio/gpio.h> > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + tps23861@28 { Node names should be generic. See also an explanation and list of examples (not exhaustive) in DT specification: https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation > + #address-cells = <1>; > + #size-cells = <0>; Follow closely DTS coding style. > + compatible = "ti,tps23861"; > + reg = <0x28>; > + shunt-resistor-micro-ohms = <255000>; > + reset-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>; > + shutdown-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>; > + interrupt-parent = <&gpio1>; > + interrupts = <14 0>; 0 looks like invalid flag. Use proper defines and proper values. > + label = "my_poe_controller"; Use useful names or just drop it. > + port@0 { > + reg = <0>; > + class = <2>; // Max PoE class allowed. > + off-by-default; > + label = "myport"; Also not useful. Best regards, Krzysztof
