From: Gregory Fuchedgi <gfuchedgi@gmail.com>
Update schema after per-port poe class restrictions and a few other options
were implemented.
Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
---
.../devicetree/bindings/hwmon/ti,tps23861.yaml | 93 +++++++++++++++++++++-
1 file changed, 92 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
index ee7de53e19184d4c3df7564624532306d885f6e4..7538d1a9c19905ec90c48d34f84a92c1972f566b 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
@@ -24,12 +24,60 @@ 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: GPIO for the reset pin.
+ maxItems: 1
+
+ ti,ports-shutdown-gpios:
+ description:
+ GPIO for the shutdown pin. Used to prevent PoE activity before the driver
+ had a chance to configure the chip.
+ maxItems: 1
+
+ interrupts:
+ description:
+ Only required if PoE class is restricted to less than class 4 in the
+ device tree.
+ maxItems: 1
+
+patternProperties:
+ "^poe-port@[0-3]$":
+ type: object
+ description: Port specific nodes.
+ unevaluatedProperties: false
+ properties:
+ reg:
+ description: Port index.
+ items:
+ maximum: 3
+
+ ti,class:
+ description: The maximum power class a port should accept.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ maximum: 4
+
+ ti,off-by-default:
+ description: Indicates the port is off by default.
+ type: boolean
+
+ label:
+ description: Port label.
+
+ required:
+ - reg
+
required:
- compatible
- reg
@@ -45,9 +93,52 @@ examples:
#address-cells = <1>;
#size-cells = <0>;
- tps23861@30 {
+ poe_controller@30 {
compatible = "ti,tps23861";
reg = <0x30>;
shunt-resistor-micro-ohms = <255000>;
};
};
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ poe_controller@28 {
+ compatible = "ti,tps23861";
+ reg = <0x28>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ shunt-resistor-micro-ohms = <255000>;
+ interrupt-parent = <&gpio1>;
+ interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
+ label = "cabinet_poe_controller";
+ reset-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
+ ti,ports-shutdown-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
+
+ poe-port@0 {
+ reg = <0>;
+ ti,class = <2>; // Max PoE class allowed.
+ ti,off-by-default;
+ label = "cabinet_port_a";
+ };
+
+ poe-port@1 {
+ reg = <1>;
+ status = "disabled";
+ };
+
+ poe-port@2 {
+ reg = <2>;
+ status = "disabled";
+ };
+
+ poe-port@3 {
+ reg = <3>;
+ status = "disabled";
+ };
+ };
+ };
--
2.43.0On Thu, Sep 04, 2025 at 10:33:44AM -0700, Gregory Fuchedgi wrote:
> Update schema after per-port poe class restrictions and a few other options
> were implemented.
>
> Signed-off-by: Gregory Fuchedgi <gfuchedgi@gmail.com>
> ---
> .../devicetree/bindings/hwmon/ti,tps23861.yaml | 93 +++++++++++++++++++++-
> 1 file changed, 92 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> index ee7de53e19184d4c3df7564624532306d885f6e4..7538d1a9c19905ec90c48d34f84a92c1972f566b 100644
> --- a/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/ti,tps23861.yaml
> @@ -24,12 +24,60 @@ 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: GPIO for the reset pin.
> + maxItems: 1
> +
> + ti,ports-shutdown-gpios:
You can drop the vendor prefix, we do not have them for pins and
supplies.
> + description:
> + GPIO for the shutdown pin. Used to prevent PoE activity before the driver
> + had a chance to configure the chip.
> + maxItems: 1
> +
> + interrupts:
> + description:
> + Only required if PoE class is restricted to less than class 4 in the
> + device tree.
> + maxItems: 1
> +
> +patternProperties:
> + "^poe-port@[0-3]$":
Keep consistent quotes, either ' or ".
> + type: object
> + description: Port specific nodes.
> + unevaluatedProperties: false
> + properties:
> + reg:
> + description: Port index.
> + items:
Drop items, you want here a scalar, not array, and this suggests you
miss array constrain.
> + maximum: 3
> +
> + ti,class:
> + description: The maximum power class a port should accept.
What's the meaning of values? There are no other generic properties like
that? Last time it was a generic property, but maybe the answer to my
question should be that there is or should be such generic one?
Also, why exactly wouldn't you want to accept here always the highest
power class? What makes it a board-level property?
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 4
default: [ ... ]
> +
> + ti,off-by-default:
> + description: Indicates the port is off by default.
I fail to see how this is property of a board... unless you wanted to
figure out which ports are not connected, but status=disabled could be
used for that.
Sorry, but device has FIXED reset values for registers, so whether
something is off or on by default is defined by compatible.
> + type: boolean
> +
> + label:
> + description: Port label.
> +
> + required:
> + - reg
> +
> required:
> - compatible
> - reg
> @@ -45,9 +93,52 @@ examples:
> #address-cells = <1>;
> #size-cells = <0>;
>
> - tps23861@30 {
> + poe_controller@30 {
See DTS coding style.
> compatible = "ti,tps23861";
> reg = <0x30>;
> shunt-resistor-micro-ohms = <255000>;
> };
> };
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + poe_controller@28 {
> + compatible = "ti,tps23861";
> + reg = <0x28>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + shunt-resistor-micro-ohms = <255000>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <14 IRQ_TYPE_EDGE_FALLING>;
> + label = "cabinet_poe_controller";
> + reset-gpios = <&gpio1 13 GPIO_ACTIVE_LOW>;
> + ti,ports-shutdown-gpios = <&gpio1 12 GPIO_ACTIVE_LOW>;
> +
> + poe-port@0 {
> + reg = <0>;
Mixed up indentation.
> + ti,class = <2>; // Max PoE class allowed.
> + ti,off-by-default;
> + label = "cabinet_port_a";
Best regards,
Krzysztof
On Fri, Sep 5, 2025 at 1:10 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On Thu, Sep 04, 2025 at 10:33:44AM -0700, Gregory Fuchedgi wrote: > What's the meaning of values? There are no other generic properties like > that? Last time it was a generic property, but maybe the answer to my > question should be that there is or should be such generic one? > > Also, why exactly wouldn't you want to accept here always the highest > power class? What makes it a board-level property? poe class (values from 0 to 8, this chip only supports 0 to 4) defines maximum power of a poe session. The higher the class the higher the power (with an exception of 0 for historical reasons). A board may have a power budget of let's say 7W allocated for a poe device. In that case the board should only provide power to devices which ask/negotiate poe class 1 (up to 4W) or class 2 (up to 7W). Devices that ask for more should be rejected to prevent brown out issues. I think some of my questions last time got lost in the noise. Given the info above, should this be a generic property? And if yes where do I put it? I haven't found an existing one. > I fail to see how this is property of a board... unless you wanted to > figure out which ports are not connected, but status=disabled could be > used for that. yes, status=disabled is used for ports that are not connected at all. off-by-default property is different. Most boards want simple automatic operation, no userspace involved. E.g. enable power as soon as acceptable class was negotiated. For some boards, however, it is critical to have control of poe from the userspace. Without this property the driver may enable power before userspace is ready. > Sorry, but device has FIXED reset values for registers, so whether > something is off or on by default is defined by compatible. yes, but it is also defined by ports-shutdown pin state. Here's our board startup sequence (see 2/2 patch): reset pin has pull resistor keeping reset active until driver takes over the pin. The driver activates ports-shutdown pin first and only then deactivates reset. Then configuration over i2c happens, while ports are shut down. Then the driver either enables a port based on off-by-default property or doesn't, leaving this up to the userspace. This setup guarantees that from soc reset until userspace is ready there's no poe activity on the ports. This implementation is also flexible and backwards compatible.
On 05/09/2025 19:22, Gregory Fuchedgi wrote: > On Fri, Sep 5, 2025 at 1:10 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: >> On Thu, Sep 04, 2025 at 10:33:44AM -0700, Gregory Fuchedgi wrote: >> What's the meaning of values? There are no other generic properties like Where is context here? To which part was I replying / commenting on? You are not making the process easy. I receive a lot of emails and have no clue what this refers to. Best regards, Krzysztof
On Sat, Sep 6, 2025 at 12:19 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > On 05/09/2025 19:22, Gregory Fuchedgi wrote: > > On Fri, Sep 5, 2025 at 1:10 AM Krzysztof Kozlowski <krzk@kernel.org> wrote: > >> On Thu, Sep 04, 2025 at 10:33:44AM -0700, Gregory Fuchedgi wrote: > >> What's the meaning of values? There are no other generic properties like > > Where is context here? To which part was I replying / commenting on? > > You are not making the process easy. I receive a lot of emails and have > no clue what this refers to. You were asking about meaning of ti,class property values, commenting on this piece: > + ti,class: > + description: The maximum power class a port should accept.
© 2016 - 2025 Red Hat, Inc.