| Message ID | 20240326-feature_poe-v6-11-c1011b6ea1cb@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next,v6,01/17] MAINTAINERS: net: Add Oleksij to pse-pd maintainers | expand |
On Tue, 26 Mar 2024 15:04:48 +0100, Kory Maincent wrote: > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > PSE PI setup may encompass multiple PSE controllers or auxiliary circuits > that collectively manage power delivery to one Ethernet port. > Such configurations might support a range of PoE standards and require > the capability to dynamically configure power delivery based on the > operational mode (e.g., PoE2 versus PoE4) or specific requirements of > connected devices. In these instances, a dedicated PSE PI node becomes > essential for accurately documenting the system architecture. This node > would serve to detail the interactions between different PSE controllers, > the support for various PoE modes, and any additional logic required to > coordinate power delivery across the network infrastructure. > > The old usage of "#pse-cells" is unsuficient as it carries only the PSE PI > index information. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- > > Changes in v3: > - New patch > > Changes in v4: > - Remove $def > - Fix pairset-names item list > - Upgrade few properties description > - Update the commit message > > Changes in v5: > - Fix yamllint error. > - Replace underscore by dash in properties names. > - Add polarity-supported property. > > Changes in v6: > - Reorder the pairset pinout table documentation to shrink the lines size. > - Remove pairset and polarity as required fields. > - Add vpwr-supply regulator supply. > --- > .../bindings/net/pse-pd/pse-controller.yaml | 102 ++++++++++++++++++++- > 1 file changed, 99 insertions(+), 3 deletions(-) > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13): yamllint warnings/errors: dtschema/dtc warnings/errors: doc reference errors (make refcheckdocs): Warning: Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml references a file that doesn't exist: Documentation/networking/pse-pd/pse-pi.rst Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml: Documentation/networking/pse-pd/pse-pi.rst See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240326-feature_poe-v6-11-c1011b6ea1cb@bootlin.com The base for the series is generally the latest rc1. A different dependency should be noted in *this* patch. If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date: pip3 install dtschema --upgrade Please check and re-submit after running the above command yourself. Note that DT_SCHEMA_FILES can be set to your schema file to speed up checking your schema. However, it must be unset to test all examples with your schema.
> + pairsets: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + List of phandles, each pointing to the power supply for the > + corresponding pairset named in 'pairset-names'. This property > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) > + |-----------|---------------|---------------|---------------|---------------| > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | > + | | (MDI-X) | (MDI) | (X) | (S) | > + |-----------|---------------|---------------|---------------|---------------| > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | Is it possible to avoid \u encoding? Ideally this documentation should be understandable without having to render it using a toolset. I just want to use less(1). Or is this a email problem? Has something converted your UTF-8 file to this \u notation? Andrew
On Tue, Mar 26, 2024 at 10:39:28AM -0500, Rob Herring wrote: > > On Tue, 26 Mar 2024 15:04:48 +0100, Kory Maincent wrote: > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > > > PSE PI setup may encompass multiple PSE controllers or auxiliary circuits > > that collectively manage power delivery to one Ethernet port. > > Such configurations might support a range of PoE standards and require > > the capability to dynamically configure power delivery based on the > > operational mode (e.g., PoE2 versus PoE4) or specific requirements of > > connected devices. In these instances, a dedicated PSE PI node becomes > > essential for accurately documenting the system architecture. This node > > would serve to detail the interactions between different PSE controllers, > > the support for various PoE modes, and any additional logic required to > > coordinate power delivery across the network infrastructure. > > > > The old usage of "#pse-cells" is unsuficient as it carries only the PSE PI > > index information. > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > --- > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > yamllint warnings/errors: > > dtschema/dtc warnings/errors: > > > doc reference errors (make refcheckdocs): > Warning: Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml references a file that doesn't exist: Documentation/networking/pse-pd/pse-pi.rst > Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml: Documentation/networking/pse-pd/pse-pi.rst Is this a false positive? Andrew
On Thu, 28 Mar 2024 13:32:09 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > On Tue, Mar 26, 2024 at 10:39:28AM -0500, Rob Herring wrote: > > > > On Tue, 26 Mar 2024 15:04:48 +0100, Kory Maincent wrote: > > > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > > > > > PSE PI setup may encompass multiple PSE controllers or auxiliary circuits > > > that collectively manage power delivery to one Ethernet port. > > > Such configurations might support a range of PoE standards and require > > > the capability to dynamically configure power delivery based on the > > > operational mode (e.g., PoE2 versus PoE4) or specific requirements of > > > connected devices. In these instances, a dedicated PSE PI node becomes > > > essential for accurately documenting the system architecture. This node > > > would serve to detail the interactions between different PSE controllers, > > > the support for various PoE modes, and any additional logic required to > > > coordinate power delivery across the network infrastructure. > > > > > > The old usage of "#pse-cells" is unsuficient as it carries only the PSE PI > > > index information. > > > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > > --- > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' > > on your patch (DT_CHECKER_FLAGS is new in v5.13): > > > > yamllint warnings/errors: > > > > dtschema/dtc warnings/errors: > > > > > > doc reference errors (make refcheckdocs): > > Warning: Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml > > references a file that doesn't exist: > > Documentation/networking/pse-pd/pse-pi.rst > > Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml: > > Documentation/networking/pse-pd/pse-pi.rst > > Is this a false positive? I suppose so as the file is added in the patch 10. Regards,
On Thu, 28 Mar 2024 13:31:06 +0100 Andrew Lunn <andrew@lunn.ch> wrote: > > + pairsets: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: > > + List of phandles, each pointing to the power supply for the > > + corresponding pairset named in 'pairset-names'. This property > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table > > 145\u20133) > > + > > |-----------|---------------|---------------|---------------|---------------| > > + | Conductor | Alternative A | Alternative A | Alternative B > > | Alternative B | > > + | | (MDI-X) | (MDI) | (X) > > | (S) | > > + > > |-----------|---------------|---------------|---------------|---------------| > > + | 1 | Negative VPSE | Positive VPSE | \u2014 > > | \u2014 | > > + | 2 | Negative VPSE | Positive VPSE | \u2014 > > | \u2014 | > > + | 3 | Positive VPSE | Negative VPSE | \u2014 > > | \u2014 | > > + | 4 | \u2014 | \u2014 | > > Negative VPSE | Positive VPSE | > > + | 5 | \u2014 | \u2014 | > > Negative VPSE | Positive VPSE | > > + | 6 | Positive VPSE | Negative VPSE | \u2014 > > | \u2014 | > > + | 7 | \u2014 | \u2014 | > > Positive VPSE | Negative VPSE | > > + | 8 | \u2014 | \u2014 | > > Positive VPSE | Negative VPSE | > > Is it possible to avoid \u encoding? Ideally this documentation should > be understandable without having to render it using a toolset. I just > want to use less(1). > > Or is this a email problem? Has something converted your UTF-8 file to > this \u notation? It seems to come from the documentation I copied pasted from Oleksij mail. Will fix it. Regards,
On Tue, Mar 26, 2024 at 03:04:48PM +0100, Kory Maincent wrote: > From: Kory Maincent (Dent Project) <kory.maincent@bootlin.com> > > PSE PI setup may encompass multiple PSE controllers or auxiliary circuits > that collectively manage power delivery to one Ethernet port. > Such configurations might support a range of PoE standards and require > the capability to dynamically configure power delivery based on the > operational mode (e.g., PoE2 versus PoE4) or specific requirements of > connected devices. In these instances, a dedicated PSE PI node becomes > essential for accurately documenting the system architecture. This node > would serve to detail the interactions between different PSE controllers, > the support for various PoE modes, and any additional logic required to > coordinate power delivery across the network infrastructure. > > The old usage of "#pse-cells" is unsuficient as it carries only the PSE PI > index information. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- > > Changes in v3: > - New patch > > Changes in v4: > - Remove $def > - Fix pairset-names item list > - Upgrade few properties description > - Update the commit message > > Changes in v5: > - Fix yamllint error. > - Replace underscore by dash in properties names. > - Add polarity-supported property. > > Changes in v6: > - Reorder the pairset pinout table documentation to shrink the lines size. > - Remove pairset and polarity as required fields. > - Add vpwr-supply regulator supply. > --- > .../bindings/net/pse-pd/pse-controller.yaml | 102 ++++++++++++++++++++- > 1 file changed, 99 insertions(+), 3 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml b/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml > index 2d382faca0e6..03f7f215c162 100644 > --- a/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml > +++ b/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml > @@ -13,6 +13,7 @@ description: Binding for the Power Sourcing Equipment (PSE) as defined in the > > maintainers: > - Oleksij Rempel <o.rempel@pengutronix.de> > + - Kory Maincent <kory.maincent@bootlin.com> > > properties: > $nodename: > @@ -22,11 +23,106 @@ properties: > description: > Used to uniquely identify a PSE instance within an IC. Will be > 0 on PSE nodes with only a single output and at least 1 on nodes > - controlling several outputs. > + controlling several outputs which are not described in the pse-pis > + subnode. This property is deprecated, please use pse-pis instead. > enum: [0, 1] > > -required: > - - "#pse-cells" > + pse-pis: > + type: object > + description: > + Overview of the PSE PIs provided by the controller. > + > + properties: > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > + required: > + - "#address-cells" > + - "#size-cells" > + > + patternProperties: > + "^pse-pi@[0-9a-f]+$": > + type: object > + description: > + PSE PI 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. > + See Documentation/networking/pse-pd/pse-pi.rst for details. > + > + properties: > + reg: > + description: > + Address describing the PSE PI index. > + maxItems: 1 > + > + "#pse-cells": > + const: 0 > + > + pairset-names: > + $ref: /schemas/types.yaml#/definitions/string-array > + description: > + Names of the pairsets as per IEEE 802.3-2022, Section 145.2.4. > + Valid values are "alternative-a" and "alternative-b". Each name Don't state constraints in prose which are defined as schema constraints. > + should correspond to a phandle in the 'pairset' property > + pointing to the power supply for that pairset. > + minItems: 1 > + maxItems: 2 > + items: > + enum: > + - alternative-a > + - alternative-b > + > + pairsets: > + $ref: /schemas/types.yaml#/definitions/phandle-array > + description: > + List of phandles, each pointing to the power supply for the > + corresponding pairset named in 'pairset-names'. This property > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) > + |-----------|---------------|---------------|---------------|---------------| > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | > + | | (MDI-X) | (MDI) | (X) | (S) | > + |-----------|---------------|---------------|---------------|---------------| > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > + minItems: 1 > + maxItems: 2 "pairsets" does not follow the normal design pattern of foos, foo-names, and #foo-cells. You could add #foo-cells I suppose, but what would cells convey? I don't think it's a good fit for what you need. The other oddity is the number of entries and the names are fixed. That is usually defined per consumer. As each entry is just a power rail, why can't the regulator binding be used here? > + > + polarity-supported: > + $ref: /schemas/types.yaml#/definitions/string-array > + description: > + Polarity configuration supported by the PSE PI pairsets. > + minItems: 1 > + maxItems: 4 > + items: > + enum: > + - MDI-X > + - MDI > + - X > + - S > + > + vpwr-supply: > + description: Regulator power supply for the PSE PI. I don't see this being used anywhere. > + > + required: > + - reg > + - "#pse-cells" > + > +oneOf: > + - required: > + - "#pse-cells" > + - required: > + - pse-pis > > additionalProperties: true > > > -- > 2.25.1 >
On Tue, Apr 02, 2024 at 08:26:37AM -0500, Rob Herring wrote: > > + pairsets: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: > > + List of phandles, each pointing to the power supply for the > > + corresponding pairset named in 'pairset-names'. This property > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) > > + |-----------|---------------|---------------|---------------|---------------| > > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | > > + | | (MDI-X) | (MDI) | (X) | (S) | > > + |-----------|---------------|---------------|---------------|---------------| > > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > + minItems: 1 > > + maxItems: 2 > > "pairsets" does not follow the normal design pattern of foos, foo-names, > and #foo-cells. You could add #foo-cells I suppose, but what would cells > convey? I don't think it's a good fit for what you need. > > The other oddity is the number of entries and the names are fixed. That > is usually defined per consumer. > > As each entry is just a power rail, why can't the regulator binding be > used here? I'm not against describing it consequent with regulator till the wire end, but right now I have no idea how it should be described by using regulator bindings. There are maximum 2 rails going in to PSE PI on one side and 4 rails with at least 5 combinations supported by standard on other side. Instead of inventing anything new, I suggested to describe supported output combinations by using IEEE 802.3 standard. Regards, Oleksij
On Tue, 2 Apr 2024 08:26:37 -0500 Rob Herring <robh@kernel.org> wrote: > > + pairset-names: > > + $ref: /schemas/types.yaml#/definitions/string-array > > + description: > > + Names of the pairsets as per IEEE 802.3-2022, Section > > 145.2.4. > > + Valid values are "alternative-a" and "alternative-b". Each > > name > > Don't state constraints in prose which are defined as schema > constraints. Ok, I will remove the line. > > + pairsets: > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > + description: > > + List of phandles, each pointing to the power supply for the > > + corresponding pairset named in 'pairset-names'. This property > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table > > 145\u20133) > > + > > |-----------|---------------|---------------|---------------|---------------| > > + | Conductor | Alternative A | Alternative A | Alternative B > > | Alternative B | > > + | | (MDI-X) | (MDI) | (X) > > | (S) | > > + > > |-----------|---------------|---------------|---------------|---------------| > > + | 1 | Negative VPSE | Positive VPSE | \u2014 > > | \u2014 | > > + | 2 | Negative VPSE | Positive VPSE | \u2014 > > | \u2014 | > > + | 3 | Positive VPSE | Negative VPSE | \u2014 > > | \u2014 | > > + | 4 | \u2014 | \u2014 | > > Negative VPSE | Positive VPSE | > > + | 5 | \u2014 | \u2014 | > > Negative VPSE | Positive VPSE | > > + | 6 | Positive VPSE | Negative VPSE | \u2014 > > | \u2014 | > > + | 7 | \u2014 | \u2014 | > > Positive VPSE | Negative VPSE | > > + | 8 | \u2014 | \u2014 | > > Positive VPSE | Negative VPSE | > > + minItems: 1 > > + maxItems: 2 > > "pairsets" does not follow the normal design pattern of foos, foo-names, > and #foo-cells. You could add #foo-cells I suppose, but what would cells > convey? I don't think it's a good fit for what you need. > > The other oddity is the number of entries and the names are fixed. That > is usually defined per consumer. Theoretically if the RJ45 port binding was supported it would make more sense, but in reality it's not feasible as the PSE controller need this information in its init process. The PSE controller reset all its port to apply a configuration so we can't do it when the consumer (RJ45) probe. It would reset the other ports if one consumer is probed later in the process. > As each entry is just a power rail, why can't the regulator binding be > used here? Olekisj already answered about it. PSE PI is like a regulator but with few different features and more information like the pinout and the polarity, so we could not really fully rely on the regulator binding style. > > + > > + polarity-supported: > > + $ref: /schemas/types.yaml#/definitions/string-array > > + description: > > + Polarity configuration supported by the PSE PI pairsets. > > + minItems: 1 > > + maxItems: 4 > > + items: > > + enum: > > + - MDI-X > > + - MDI > > + - X > > + - S > > + > > + vpwr-supply: > > + description: Regulator power supply for the PSE PI. > > I don't see this being used anywhere. Right, I forgot to add it to the PD692x0 and TPS23881 binding example! Regards,
On Wed, Apr 03, 2024 at 11:15:48AM +0200, Kory Maincent wrote: > On Tue, 2 Apr 2024 08:26:37 -0500 > Rob Herring <robh@kernel.org> wrote: > > > > + pairset-names: > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > + description: > > > + Names of the pairsets as per IEEE 802.3-2022, Section > > > 145.2.4. > > > + Valid values are "alternative-a" and "alternative-b". Each > > > name > > > > Don't state constraints in prose which are defined as schema > > constraints. > > Ok, I will remove the line. > > > > + pairsets: > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > + description: > > > + List of phandles, each pointing to the power supply for the > > > + corresponding pairset named in 'pairset-names'. This property > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table > > > 145\u20133) > > > + > > > |-----------|---------------|---------------|---------------|---------------| > > > + | Conductor | Alternative A | Alternative A | Alternative B > > > | Alternative B | > > > + | | (MDI-X) | (MDI) | (X) > > > | (S) | > > > + > > > |-----------|---------------|---------------|---------------|---------------| > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 > > > | \u2014 | > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 > > > | \u2014 | > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 > > > | \u2014 | > > > + | 4 | \u2014 | \u2014 | > > > Negative VPSE | Positive VPSE | > > > + | 5 | \u2014 | \u2014 | > > > Negative VPSE | Positive VPSE | > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 > > > | \u2014 | > > > + | 7 | \u2014 | \u2014 | > > > Positive VPSE | Negative VPSE | > > > + | 8 | \u2014 | \u2014 | > > > Positive VPSE | Negative VPSE | > > > + minItems: 1 > > > + maxItems: 2 > > > > "pairsets" does not follow the normal design pattern of foos, foo-names, > > and #foo-cells. You could add #foo-cells I suppose, but what would cells > > convey? I don't think it's a good fit for what you need. > > > > The other oddity is the number of entries and the names are fixed. That > > is usually defined per consumer. > > Theoretically if the RJ45 port binding was supported it would make more sense, > but in reality it's not feasible as the PSE controller need this information > in its init process. > The PSE controller reset all its port to apply a configuration so we can't do > it when the consumer (RJ45) probe. It would reset the other ports if one > consumer is probed later in the process. There is no reason other than convenience that all information some driver needs has to be in one node or one hierarchy of nodes. You can fetch anything from anywhere in the DT. It does feel like some of this belongs in a connector node. We often haven't described connectors in DT and stick connector properties in the controller node associated with the connector. Then as things get more complicated, it becomes a mess. > > As each entry is just a power rail, why can't the regulator binding be > > used here? > > Olekisj already answered about it. > PSE PI is like a regulator but with few different features and more information > like the pinout and the polarity, so we could not really fully rely on the > regulator binding style. > > > > + > > > + polarity-supported: > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > + description: > > > + Polarity configuration supported by the PSE PI pairsets. > > > + minItems: 1 > > > + maxItems: 4 > > > + items: > > > + enum: > > > + - MDI-X > > > + - MDI > > > + - X > > > + - S > > > + > > > + vpwr-supply: > > > + description: Regulator power supply for the PSE PI. > > > > I don't see this being used anywhere. > > Right, I forgot to add it to the PD692x0 and TPS23881 binding example! But is this really common/generic? I would think input power rails would be chip specific. Rob
On Tue, Apr 02, 2024 at 05:47:58PM +0200, Oleksij Rempel wrote: > On Tue, Apr 02, 2024 at 08:26:37AM -0500, Rob Herring wrote: > > > + pairsets: > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > + description: > > > + List of phandles, each pointing to the power supply for the > > > + corresponding pairset named in 'pairset-names'. This property > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) > > > + |-----------|---------------|---------------|---------------|---------------| > > > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | > > > + | | (MDI-X) | (MDI) | (X) | (S) | > > > + |-----------|---------------|---------------|---------------|---------------| > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > > + minItems: 1 > > > + maxItems: 2 > > > > "pairsets" does not follow the normal design pattern of foos, foo-names, > > and #foo-cells. You could add #foo-cells I suppose, but what would cells > > convey? I don't think it's a good fit for what you need. > > > > The other oddity is the number of entries and the names are fixed. That > > is usually defined per consumer. > > > > As each entry is just a power rail, why can't the regulator binding be > > used here? > > I'm not against describing it consequent with regulator till the wire > end, but right now I have no idea how it should be described by using > regulator bindings. There are maximum 2 rails going in to PSE PI on one > side and 4 rails with at least 5 combinations supported by standard on > other side. Instead of inventing anything new, I suggested to describe > supported output combinations by using IEEE 802.3 standard. There's 4 combinations above, what's the 5th combination? SPE? Seems to me you just describe the 2 rails going to the connector and then describe all the variations the connector supports. The PSE (h/w) has little to do with which variations are supported, right? For example, MDI-X vs. MDI support is determined by the PHY, right? Or it has to be supported by both the PHY and PSE? Rob
On Wed, Apr 03, 2024 at 09:44:48AM -0500, Rob Herring wrote: > On Tue, Apr 02, 2024 at 05:47:58PM +0200, Oleksij Rempel wrote: > > On Tue, Apr 02, 2024 at 08:26:37AM -0500, Rob Herring wrote: > > > > + pairsets: > > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > > + description: > > > > + List of phandles, each pointing to the power supply for the > > > > + corresponding pairset named in 'pairset-names'. This property > > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) > > > > + |-----------|---------------|---------------|---------------|---------------| > > > > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | > > > > + | | (MDI-X) | (MDI) | (X) | (S) | > > > > + |-----------|---------------|---------------|---------------|---------------| > > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | > > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > > > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > > > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | > > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | > > > > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > > > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | > > > > + minItems: 1 > > > > + maxItems: 2 > > > > > > "pairsets" does not follow the normal design pattern of foos, foo-names, > > > and #foo-cells. You could add #foo-cells I suppose, but what would cells > > > convey? I don't think it's a good fit for what you need. > > > > > > The other oddity is the number of entries and the names are fixed. That > > > is usually defined per consumer. > > > > > > As each entry is just a power rail, why can't the regulator binding be > > > used here? > > > > I'm not against describing it consequent with regulator till the wire > > end, but right now I have no idea how it should be described by using > > regulator bindings. There are maximum 2 rails going in to PSE PI on one > > side and 4 rails with at least 5 combinations supported by standard on > > other side. Instead of inventing anything new, I suggested to describe > > supported output combinations by using IEEE 802.3 standard. > > There's 4 combinations above, what's the 5th combination? SPE? The 5th combination is PoE4 where two rails are supplying power at same time. First 4 variants for PoE: one or two positive rails are attached (but only one is used at same time) to pairs 1-2 or 3-4, or 5-6, or 7-8. Or support all of combinations if some advanced PSE PI is present. PSE PI is kind of MUX for regulators. One more variant in case of PoE4: two positive rail are attached at same time, one to 1-2, second to 5-6. May be one more variant with opposite polarity, this will be the 6th combination. > Seems to me you just describe the 2 rails going to the connector and > then describe all the variations the connector supports. The PSE > (h/w) has little to do with which variations are supported, right? No. In case of mutli-channel PSE, it needs to know if channels are attached to one port or to different ports. PSE is not only responsible to enable the power, it runs classification of devices attached to the port, so it will decide, which rail should be enabled. > For example, MDI-X vs. MDI support is determined by the PHY, right? Yes and No. Until PSE do not start supplying power, PHY will not be able to start communication with the remote PHY, so it will not be able to detect MDI/X configuration. Polarity configuration is important for user space or user to get information about supported pin configuration and if possible, change the configuration. > Or it has to be supported by both the PHY and PSE? In most cases PSE and PHY work independently from each other, they just share same port. Potential exception are: - in case data line should not be shared with power lines, we need to know what pins are used for power, this information would help to provide PHY configuration. - in case PHY autoneg signals disturb PoE classification, we need to coordinate PHY and PSE states. Regards, Oleksij
On Wed, 3 Apr 2024 09:31:42 -0500 Rob Herring <robh@kernel.org> wrote: > On Wed, Apr 03, 2024 at 11:15:48AM +0200, Kory Maincent wrote: > > On Tue, 2 Apr 2024 08:26:37 -0500 > > Rob Herring <robh@kernel.org> wrote: > > > > > > + pairset-names: > > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > > + description: > > > > + Names of the pairsets as per IEEE 802.3-2022, Section > > > > 145.2.4. > > > > + Valid values are "alternative-a" and "alternative-b". > > > > Each name > > > > > > Don't state constraints in prose which are defined as schema > > > constraints. > > > > Ok, I will remove the line. > > > > > > + pairsets: > > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > > + description: > > > > + List of phandles, each pointing to the power supply for > > > > the > > > > + corresponding pairset named in 'pairset-names'. This > > > > property > > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. > > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table > > > > 145\u20133) > > > > + > > > > |-----------|---------------|---------------|---------------|---------------| > > > > + | Conductor | Alternative A | Alternative A | > > > > Alternative B | Alternative B | > > > > + | | (MDI-X) | (MDI) | (X) > > > > | (S) | > > > > + > > > > |-----------|---------------|---------------|---------------|---------------| > > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 > > > > | \u2014 | > > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 > > > > | \u2014 | > > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 > > > > | \u2014 | > > > > + | 4 | \u2014 | \u2014 | > > > > Negative VPSE | Positive VPSE | > > > > + | 5 | \u2014 | \u2014 | > > > > Negative VPSE | Positive VPSE | > > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 > > > > | \u2014 | > > > > + | 7 | \u2014 | \u2014 | > > > > Positive VPSE | Negative VPSE | > > > > + | 8 | \u2014 | \u2014 | > > > > Positive VPSE | Negative VPSE | > > > > + minItems: 1 > > > > + maxItems: 2 > > > > > > "pairsets" does not follow the normal design pattern of foos, foo-names, > > > and #foo-cells. You could add #foo-cells I suppose, but what would cells > > > convey? I don't think it's a good fit for what you need. > > > > > > The other oddity is the number of entries and the names are fixed. That > > > is usually defined per consumer. > > > > Theoretically if the RJ45 port binding was supported it would make more > > sense, but in reality it's not feasible as the PSE controller need this > > information in its init process. > > The PSE controller reset all its port to apply a configuration so we can't > > do it when the consumer (RJ45) probe. It would reset the other ports if one > > consumer is probed later in the process. > > There is no reason other than convenience that all information some > driver needs has to be in one node or one hierarchy of nodes. You can > fetch anything from anywhere in the DT. It does feel like some of this > belongs in a connector node. We often haven't described connectors in DT > and stick connector properties in the controller node associated with > the connector. Then as things get more complicated, it becomes a mess. Right, we could indeed put all the informations of the pse_pi node in the future RJ45 port abstraction node. Then, this series will be put aside until we manage to have the port abstraction get merged. I am not glad about this as it will stuck my work until then, but indeed removing this pse_pi wrapper node which is between the pse_controller node and the connector node seems cleaner. Regards,
On Wed, 3 Apr 2024 09:31:42 -0500 Rob Herring <robh@kernel.org> wrote: > > > > > > + > > > > + polarity-supported: > > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > > + description: > > > > + Polarity configuration supported by the PSE PI pairsets. > > > > + minItems: 1 > > > > + maxItems: 4 > > > > + items: > > > > + enum: > > > > + - MDI-X > > > > + - MDI > > > > + - X > > > > + - S > > > > + > > > > + vpwr-supply: > > > > + description: Regulator power supply for the PSE PI. > > > > > > I don't see this being used anywhere. > > > > Right, I forgot to add it to the PD692x0 and TPS23881 binding example! > > But is this really common/generic? I would think input power rails would > be chip specific. I think as each PSE PI are seen as a regulator we may want it generic to track each PI parent. Having the parent regulator described like that would force the devicetree to describe where the power come from. In contrary, for example, on the pd692x0 controller the regulators are connected to the managers (PD69208) and not directly to the PIs. So the devicetree would not really fit the hardware. It is indeed chip specific but having described like that would be more simple. If we decided to make it chip specific the core would have a callback to ask the driver to fill the regulator_init_data structure for each PI before registering the regulators. It is feasible. Mmh in fact I am still unsure about the solution. Oleksij as you were the first to push the idea. Have you more argument in mind to make it generic? https://lore.kernel.org/netdev/ZeObuKHkPN3tiWz_@pengutronix.de/ Regards,
On Thu, Apr 04, 2024 at 11:25:06AM +0200, Kory Maincent wrote: > On Wed, 3 Apr 2024 09:31:42 -0500 > Rob Herring <robh@kernel.org> wrote: > > > > > > > > > + > > > > > + polarity-supported: > > > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > > > + description: > > > > > + Polarity configuration supported by the PSE PI pairsets. > > > > > + minItems: 1 > > > > > + maxItems: 4 > > > > > + items: > > > > > + enum: > > > > > + - MDI-X > > > > > + - MDI > > > > > + - X > > > > > + - S > > > > > + > > > > > + vpwr-supply: > > > > > + description: Regulator power supply for the PSE PI. > > > > > > > > I don't see this being used anywhere. > > > > > > Right, I forgot to add it to the PD692x0 and TPS23881 binding example! > > > > But is this really common/generic? I would think input power rails would > > be chip specific. > > I think as each PSE PI are seen as a regulator we may want it generic to track > each PI parent. Having the parent regulator described like that would force the > devicetree to describe where the power come from. > In contrary, for example, on the pd692x0 controller the regulators are connected > to the managers (PD69208) and not directly to the PIs. So the devicetree would > not really fit the hardware. It is indeed chip specific but having described > like that would be more simple. > > If we decided to make it chip specific the core would have a callback to ask > the driver to fill the regulator_init_data structure for each PI before > registering the regulators. It is feasible. > > Mmh in fact I am still unsure about the solution. > > Oleksij as you were the first to push the idea. Have you more argument in mind > to make it generic? > https://lore.kernel.org/netdev/ZeObuKHkPN3tiWz_@pengutronix.de/ There can be different, chip specific power consumer, for example the one which is feeding the PSE controller it self, but also there are common providers/consumers those which are used to feed PSE PIs. In case of pd692x0 based setup, the managers are actual regulator responsible to control power rails connected to PSE PIs, so managers should use this common provider. Not sure how TI is designed, but it will have same type of consumer to feed PSE PIs as well. Regards, Oleksij
On Thu, 4 Apr 2024 10:38:54 +0200 Kory Maincent <kory.maincent@bootlin.com> wrote: > On Wed, 3 Apr 2024 09:31:42 -0500 > Rob Herring <robh@kernel.org> wrote: > > > On Wed, Apr 03, 2024 at 11:15:48AM +0200, Kory Maincent wrote: > > > On Tue, 2 Apr 2024 08:26:37 -0500 > > > Rob Herring <robh@kernel.org> wrote: > > > > > > > > + pairset-names: > > > > > + $ref: /schemas/types.yaml#/definitions/string-array > > > > > + description: > > > > > + Names of the pairsets as per IEEE 802.3-2022, Section > > > > > 145.2.4. > > > > > + Valid values are "alternative-a" and "alternative-b". > > > > > Each name > > > > > > > > Don't state constraints in prose which are defined as schema > > > > constraints. > > > > > > Ok, I will remove the line. > > > > > > > > + pairsets: > > > > > + $ref: /schemas/types.yaml#/definitions/phandle-array > > > > > + description: > > > > > + List of phandles, each pointing to the power supply for > > > > > the > > > > > + corresponding pairset named in 'pairset-names'. This > > > > > property > > > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and > > > > > 145.2.4. > > > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table > > > > > 145\u20133) > > > > > + > > > > > |-----------|---------------|---------------|---------------|---------------| > > > > > + | Conductor | Alternative A | Alternative A | > > > > > Alternative B | Alternative B | > > > > > + | | (MDI-X) | (MDI) | (X) > > > > > | (S) | > > > > > + > > > > > |-----------|---------------|---------------|---------------|---------------| > > > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 > > > > > | \u2014 | > > > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 > > > > > | \u2014 | > > > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 > > > > > | \u2014 | > > > > > + | 4 | \u2014 | \u2014 | > > > > > Negative VPSE | Positive VPSE | > > > > > + | 5 | \u2014 | \u2014 | > > > > > Negative VPSE | Positive VPSE | > > > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 > > > > > | \u2014 | > > > > > + | 7 | \u2014 | \u2014 | > > > > > Positive VPSE | Negative VPSE | > > > > > + | 8 | \u2014 | \u2014 | > > > > > Positive VPSE | Negative VPSE | > > > > > + minItems: 1 > > > > > + maxItems: 2 > > > > > > > > "pairsets" does not follow the normal design pattern of foos, > > > > foo-names, and #foo-cells. You could add #foo-cells I suppose, but what > > > > would cells convey? I don't think it's a good fit for what you need. > > > > > > > > The other oddity is the number of entries and the names are fixed. That > > > > is usually defined per consumer. > > > > > > Theoretically if the RJ45 port binding was supported it would make more > > > sense, but in reality it's not feasible as the PSE controller need this > > > information in its init process. > > > The PSE controller reset all its port to apply a configuration so we can't > > > do it when the consumer (RJ45) probe. It would reset the other ports if > > > one consumer is probed later in the process. > > > > There is no reason other than convenience that all information some > > driver needs has to be in one node or one hierarchy of nodes. You can > > fetch anything from anywhere in the DT. It does feel like some of this > > belongs in a connector node. We often haven't described connectors in DT > > and stick connector properties in the controller node associated with > > the connector. Then as things get more complicated, it becomes a mess. > > Right, we could indeed put all the informations of the pse_pi node in the > future RJ45 port abstraction node. Then, this series will be put aside until > we manage to have the port abstraction get merged. > I am not glad about this as it will stuck my work until then, but indeed > removing this pse_pi wrapper node which is between the pse_controller node and > the connector node seems cleaner. After some new thought, I thinks it is quite similar on the devicetree side to have it in a pse_pi node or in the connector node. Here are my agruments to continue using this pse_pi binding description: - The connector abstraction is in its early work and won't really see a v1 soon while the PoE series got mainly all reviewed-by thanks to Andrew. This would stuck the PoE series until maybe one or two Linux version. - It allows to use the "Power Interface" name like described in the standards. - Even if this is in the PSE controller node, it is generic to all PSEs so it shouldn't become a mess. - It allows to have the PSE controller and Power Interfaces parameters grouped together and it will be easier to read. May not really be an argument! ;) - It will keep the logic of PoDL with the PHY using a single reference to the PSE PI through the pses parameter. Is it okay for you to continue with it? Regards,
diff --git a/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml b/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml index 2d382faca0e6..03f7f215c162 100644 --- a/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml +++ b/Documentation/devicetree/bindings/net/pse-pd/pse-controller.yaml @@ -13,6 +13,7 @@ description: Binding for the Power Sourcing Equipment (PSE) as defined in the maintainers: - Oleksij Rempel <o.rempel@pengutronix.de> + - Kory Maincent <kory.maincent@bootlin.com> properties: $nodename: @@ -22,11 +23,106 @@ properties: description: Used to uniquely identify a PSE instance within an IC. Will be 0 on PSE nodes with only a single output and at least 1 on nodes - controlling several outputs. + controlling several outputs which are not described in the pse-pis + subnode. This property is deprecated, please use pse-pis instead. enum: [0, 1] -required: - - "#pse-cells" + pse-pis: + type: object + description: + Overview of the PSE PIs provided by the controller. + + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + required: + - "#address-cells" + - "#size-cells" + + patternProperties: + "^pse-pi@[0-9a-f]+$": + type: object + description: + PSE PI 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. + See Documentation/networking/pse-pd/pse-pi.rst for details. + + properties: + reg: + description: + Address describing the PSE PI index. + maxItems: 1 + + "#pse-cells": + const: 0 + + pairset-names: + $ref: /schemas/types.yaml#/definitions/string-array + description: + Names of the pairsets as per IEEE 802.3-2022, Section 145.2.4. + Valid 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. + minItems: 1 + maxItems: 2 + items: + enum: + - alternative-a + - alternative-b + + pairsets: + $ref: /schemas/types.yaml#/definitions/phandle-array + description: + List of phandles, each pointing to the power supply for the + corresponding pairset named in 'pairset-names'. This property + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4. + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133) + |-----------|---------------|---------------|---------------|---------------| + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B | + | | (MDI-X) | (MDI) | (X) | (S) | + |-----------|---------------|---------------|---------------|---------------| + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 | + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE | + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 | + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE | + minItems: 1 + maxItems: 2 + + polarity-supported: + $ref: /schemas/types.yaml#/definitions/string-array + description: + Polarity configuration supported by the PSE PI pairsets. + minItems: 1 + maxItems: 4 + items: + enum: + - MDI-X + - MDI + - X + - S + + vpwr-supply: + description: Regulator power supply for the PSE PI. + + required: + - reg + - "#pse-cells" + +oneOf: + - required: + - "#pse-cells" + - required: + - pse-pis additionalProperties: true