diff mbox series

[net-next,v4,6/7] dt-bindings: net: pse-dt: add bindings for generic PSE controller

Message ID 20220828063021.3963761-7-o.rempel@pengutronix.de
State New
Headers show
Series add generic PSE support | expand

Commit Message

Oleksij Rempel Aug. 28, 2022, 6:30 a.m. UTC
Add binding for generic Ethernet PSE controller.

Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
---
changes v4:
- rename to PSE regulator
- drop currently unused properties
- use own compatible for PoDL PSE
changes v2:
- rename compatible to more generic "ieee802.3-pse"
- add class and type properties for PoDL and PoE variants
- add pairs property
---
 .../bindings/net/pse-pd/pse-regulator.yaml    | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml

Comments

Krzysztof Kozlowski Aug. 30, 2022, 5:41 p.m. UTC | #1
On 28/08/2022 09:30, Oleksij Rempel wrote:
> Add binding for generic Ethernet PSE controller.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v4:
> - rename to PSE regulator
> - drop currently unused properties
> - use own compatible for PoDL PSE
> changes v2:
> - rename compatible to more generic "ieee802.3-pse"
> - add class and type properties for PoDL and PoE variants
> - add pairs property
> ---
>  .../bindings/net/pse-pd/pse-regulator.yaml    | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> new file mode 100644
> index 0000000000000..1a906d2135a7a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pse-pd/pse-regulator.yaml#

The convention is filename based on compatible, so "podl-pse-regulator.yaml"

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator based Power Sourcing Equipment
> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: Regulator based PSE controller. The device must be referenced by
> +  the PHY node to control power injection to the Ethernet cable.
> +
> +properties:
> +  compatible:
> +    description: Regulator based PoDL PSE controller for a single twisted-pair
> +      link.

Why description of compatible? Description of hardware goes to top-level
description.

> +    const: podl-pse-regulator
> +
> +  '#pse-cells':
> +    const: 0
> +
> +  pse-supply:
> +    description: Power supply for the PSE controller
> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - '#pse-cells'
> +  - pse-supply
> +
> +examples:
> +  - |
> +    pse_t1l2: ethernet-pse-1 {

Node name: ethernet-pse
(unless -1 stands for something generic?)

Also, no need for label.

Best regards,
Krzysztof
Rob Herring (Arm) Aug. 30, 2022, 8:10 p.m. UTC | #2
On Sun, Aug 28, 2022 at 08:30:20AM +0200, Oleksij Rempel wrote:
> Add binding for generic Ethernet PSE controller.
> 
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---
> changes v4:
> - rename to PSE regulator
> - drop currently unused properties
> - use own compatible for PoDL PSE
> changes v2:
> - rename compatible to more generic "ieee802.3-pse"
> - add class and type properties for PoDL and PoE variants
> - add pairs property
> ---
>  .../bindings/net/pse-pd/pse-regulator.yaml    | 40 +++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> new file mode 100644
> index 0000000000000..1a906d2135a7a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
> @@ -0,0 +1,40 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pse-pd/pse-regulator.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Regulator based Power Sourcing Equipment
> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@pengutronix.de>
> +
> +description: Regulator based PSE controller. The device must be referenced by
> +  the PHY node to control power injection to the Ethernet cable.
> +
> +properties:
> +  compatible:
> +    description: Regulator based PoDL PSE controller for a single twisted-pair
> +      link.
> +    const: podl-pse-regulator
> +
> +  '#pse-cells':
> +    const: 0

Do you have intentions that this would be non-zero? This series is 
defining a new common binding, but only creating a specific schema. 
You need to define in a common schema what the purpose of the cells may 
be and if there's common constraints defining those in the common 
schema. There's several examples where only 0 or 1 is allowed for 
example.

There's a standard pattern of '#foo-cells' for the provider and 'foos' 
for the consumer. Ultimately, anything that doesn't follow that pattern 
needs explicit support in dtschema to decode the consumers. IOW, try to 
follow the pattern. Either '#ieee802.3-pse-cells'/'ieee802.3-pses' or 
'#pse-cells'/'pses'. Neither seems great to me. The former is a bit too 
specific and the latter a bit too short. Shrug.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
new file mode 100644
index 0000000000000..1a906d2135a7a
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pse-pd/pse-regulator.yaml
@@ -0,0 +1,40 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pse-pd/pse-regulator.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Regulator based Power Sourcing Equipment
+
+maintainers:
+  - Oleksij Rempel <o.rempel@pengutronix.de>
+
+description: Regulator based PSE controller. The device must be referenced by
+  the PHY node to control power injection to the Ethernet cable.
+
+properties:
+  compatible:
+    description: Regulator based PoDL PSE controller for a single twisted-pair
+      link.
+    const: podl-pse-regulator
+
+  '#pse-cells':
+    const: 0
+
+  pse-supply:
+    description: Power supply for the PSE controller
+
+additionalProperties: false
+
+required:
+  - compatible
+  - '#pse-cells'
+  - pse-supply
+
+examples:
+  - |
+    pse_t1l2: ethernet-pse-1 {
+      compatible = "podl-pse-regulator";
+      pse-supply = <&reg_t1l1>;
+      #pse-cells = <0>;
+    };