Message ID | 20240215-feature_poe-v4-14-35bb4c23266c@bootlin.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | net: Add support for Power over Ethernet (PoE) | expand |
On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > bindings documentation. > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- ... > + pse_pis { > + #address-cells = <1>; > + #size-cells = <0>; > + > + pse_pi0: pse_pi@0 { > + reg = <0>; > + #pse-cells = <0>; > + pairset-names = "alternative-a", "alternative-b"; > + pairsets = <&phys0>, <&phys1>; > + }; > + pse_pi1: pse_pi@1 { > + reg = <1>; > + #pse-cells = <0>; > + pairset-names = "alternative-a"; > + pairsets = <&phys2>; According to latest discussions, PSE PI nodes will need some additional, board specific, information: - this controller do not implements polarity switching, we need to know what polarity is implemented on this board. The 802.3 spec provide not really consistent names for polarity configurations: - Alternative A MDI-X - Alternative A MDI - Alternative B X - Alternative B S The board may implement one of polarity configurations per alternative or have additional helpers to switch them without using PSE controller. Even if specification explicitly say: "The PD shall be implemented to be insensitive to the polarity of the power supply and shall be able to operate per the PD Mode A column and the PD Mode B column in Table 33–13" it is possible to find reports like this: https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 Probably this kind of property is a good fit: polarity-supported = "MDI-X", "MDI", "X", "S"; - Except of polarity, we have alternative-b variant with direct or phantom feeding (No idea if it is proper description). Theoretically, this difference would affect electrical rating specifications. For example direct path for alternate-b (10/100Mbit only), would have higher rating as the path over coils/magnetics. Practically, vendors do not make different ratings for this paths, so no need to care about it for now until someone will be able to provide good reason. Here is example of RJ45 connector with integrated magnetics with PoE support where alternative-a feed over magnetics and alternative-b is feed directly: https://www.te.com/commerce/DocumentDelivery/DDEController?Action=srchrtrv&DocNm=5-2337992-4&DocType=Customer+Drawing&DocLang=English&PartCntxt=5-2337992-4&DocFormat=pdf (the last topic is more an answer to my self and for archive :))
On 15/02/2024 17:02, Kory Maincent wrote: > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > bindings documentation. > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > --- > > Changes in v2: > - Enhance ports-matrix description. > - Replace additionalProperties by unevaluatedProperties. > - Drop i2c suffix. > > Changes in v3: > - Remove ports-matrix parameter. > - Add description of all physical ports and managers. > - Add pse_pis subnode moving to the API of pse-controller binding. > - Remove the MAINTAINERS section for this driver as I will be maintaining > all pse-pd subsystem. > --- > .../bindings/net/pse-pd/microchip,pd692x0.yaml | 157 +++++++++++++++++++++ > 1 file changed, 157 insertions(+) > > diff --git a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml > new file mode 100644 > index 000000000000..57ba5365157c > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml > @@ -0,0 +1,157 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/pse-pd/microchip,pd692x0.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Microchip PD692x0 Power Sourcing Equipment controller > + > +maintainers: > + - Kory Maincent <kory.maincent@bootlin.com> > + > +allOf: > + - $ref: pse-controller.yaml# > + > +properties: > + compatible: > + enum: > + - microchip,pd69200 > + - microchip,pd69210 > + - microchip,pd69220 > + > + reg: > + maxItems: 1 > + > + managers: > + $ref: "#/$defs/managers" > + description: > + List of the PD69208T4/PD69204T4/PD69208M PSE managers. Each manager > + have 4 or 8 physical ports according to the chip version. No need to > + specify the SPI chip select as it is automatically detected by the > + PD692x0 PSE controller. The PSE managers have to be described from > + the lowest chip select to the greatest one, which is the detection > + behavior of the PD692x0 PSE controller. The PD692x0 support up to > + 12 PSE managers which can expose up to 96 physical ports. All > + physical ports available on a manager have to be described in the > + incremental order even if they are not used. > + > +required: > + - compatible > + - reg > + - pse_pis > + > +$defs: Why do you need defs for it? You use it only in one place. ... > + > + pse_pis { Again... no underscores in node names. > + #address-cells = <1>; > + #size-cells = <0>; > + > + pse_pi0: pse_pi@0 { Just compile your DTS with W=2. Best regards, Krzysztof
On Sat, 17 Feb 2024 13:14:29 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > > bindings documentation. > > > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > --- > ... > > + pse_pis { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pse_pi0: pse_pi@0 { > > + reg = <0>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a", "alternative-b"; > > + pairsets = <&phys0>, <&phys1>; > > + }; > > + pse_pi1: pse_pi@1 { > > + reg = <1>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a"; > > + pairsets = <&phys2>; > > According to latest discussions, PSE PI nodes will need some > additional, board specific, information: > - this controller do not implements polarity switching, we need to know > what polarity is implemented on this board. The 802.3 spec provide not > really consistent names for polarity configurations: > - Alternative A MDI-X > - Alternative A MDI > - Alternative B X > - Alternative B S > The board may implement one of polarity configurations per alternative > or have additional helpers to switch them without using PSE > controller. > Even if specification explicitly say: > "The PD shall be implemented to be insensitive to the polarity of the power > supply and shall be able to operate per the PD Mode A column and the PD > Mode B column in Table 33–13" > it is possible to find reports like this: > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 Mmh not sure we want to support broken cases that does not follow the spec. Should we? Regards,
> Mmh not sure we want to support broken cases that does not follow the spec. > Should we? We should specify the properties a device following the spec should use. These are the common properties we expect all devices should be using. Broken devices can however have additional properties, defined in their own binding. And if we see a pattern for broken properties, we could pull them out into a -broken.yaml binding, which the broken devices could share. Andrew
On Sat, 17 Feb 2024 13:14:29 +0100 Oleksij Rempel <o.rempel@pengutronix.de> wrote: > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > > bindings documentation. > > > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > --- > ... > > + pse_pis { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pse_pi0: pse_pi@0 { > > + reg = <0>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a", "alternative-b"; > > + pairsets = <&phys0>, <&phys1>; > > + }; > > + pse_pi1: pse_pi@1 { > > + reg = <1>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a"; > > + pairsets = <&phys2>; > > According to latest discussions, PSE PI nodes will need some > additional, board specific, information: > - this controller do not implements polarity switching, we need to know > what polarity is implemented on this board. The 802.3 spec provide not > really consistent names for polarity configurations: > - Alternative A MDI-X > - Alternative A MDI > - Alternative B X > - Alternative B S > The board may implement one of polarity configurations per alternative > or have additional helpers to switch them without using PSE > controller. > Even if specification explicitly say: > "The PD shall be implemented to be insensitive to the polarity of the power > supply and shall be able to operate per the PD Mode A column and the PD > Mode B column in Table 33–13" > it is possible to find reports like this: > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 > > Probably this kind of property is a good fit: > polarity-supported = "MDI-X", "MDI", "X", "S"; This property should be on the PD side. Isn't it better to name it "polarity-provided" for each PSE PIs binding? What do you think? We agreed that it is mainly for ethtool to show the polarity of a PI, right? Regards,
On Tue, Feb 20, 2024 at 11:40:29AM +0100, Köry Maincent wrote: > On Sat, 17 Feb 2024 13:14:29 +0100 > Oleksij Rempel <o.rempel@pengutronix.de> wrote: > > > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > > > bindings documentation. > > > > > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > > --- > > ... > > > + pse_pis { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + pse_pi0: pse_pi@0 { > > > + reg = <0>; > > > + #pse-cells = <0>; > > > + pairset-names = "alternative-a", "alternative-b"; > > > + pairsets = <&phys0>, <&phys1>; > > > + }; > > > + pse_pi1: pse_pi@1 { > > > + reg = <1>; > > > + #pse-cells = <0>; > > > + pairset-names = "alternative-a"; > > > + pairsets = <&phys2>; > > > > According to latest discussions, PSE PI nodes will need some > > additional, board specific, information: > > - this controller do not implements polarity switching, we need to know > > what polarity is implemented on this board. The 802.3 spec provide not > > really consistent names for polarity configurations: > > - Alternative A MDI-X > > - Alternative A MDI > > - Alternative B X > > - Alternative B S > > The board may implement one of polarity configurations per alternative > > or have additional helpers to switch them without using PSE > > controller. > > Even if specification explicitly say: > > "The PD shall be implemented to be insensitive to the polarity of the power > > supply and shall be able to operate per the PD Mode A column and the PD > > Mode B column in Table 33–13" > > it is possible to find reports like this: > > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 > > > > Probably this kind of property is a good fit: > > polarity-supported = "MDI-X", "MDI", "X", "S"; > > This property should be on the PD side. Probably. Right now we are on PSE side. > Isn't it better to name it "polarity-provided" for each PSE PIs binding? What > do you think? Yes, this suggestion was directed for PSE PI nodes. In the PHY world, we use "supported" capabilities for what HW can actually do and "advertised" for how the HW is configured. If we use word "provided", i would interpret it as subset of "supported", which at the end is a user space policy. Since I'm not native englisch speaker, my feeling can be wrong. So, any one with stronger opinion may have here other preferences. > We agreed that it is mainly for ethtool to show the polarity of a PI, right? We have two kind of information here: - polarity supported by HW. PSE PI may support more then one. - actually configured polarity. Regards, Oleksij
On Sat, Feb 17, 2024 at 01:14:29PM +0100, Oleksij Rempel wrote: > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > > bindings documentation. > > > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > --- > ... > > + pse_pis { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + pse_pi0: pse_pi@0 { > > + reg = <0>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a", "alternative-b"; > > + pairsets = <&phys0>, <&phys1>; > > + }; > > + pse_pi1: pse_pi@1 { > > + reg = <1>; > > + #pse-cells = <0>; > > + pairset-names = "alternative-a"; > > + pairsets = <&phys2>; > > According to latest discussions, PSE PI nodes will need some > additional, board specific, information: > - this controller do not implements polarity switching, we need to know > what polarity is implemented on this board. The 802.3 spec provide not > really consistent names for polarity configurations: > - Alternative A MDI-X > - Alternative A MDI > - Alternative B X > - Alternative B S > The board may implement one of polarity configurations per alternative > or have additional helpers to switch them without using PSE > controller. > Even if specification explicitly say: > "The PD shall be implemented to be insensitive to the polarity of the power > supply and shall be able to operate per the PD Mode A column and the PD > Mode B column in Table 33–13" > it is possible to find reports like this: > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 > > Probably this kind of property is a good fit: > polarity-supported = "MDI-X", "MDI", "X", "S"; Where does that live? Looks like a property of the consumers defined in the provider. Generally, that's not the right way for DT. I'll say it again, I think you should be expanding #pse-cells (>1), not getting rid of them (==0). Rob
On Wed, Feb 21, 2024 at 07:41:35AM -0700, Rob Herring wrote: > On Sat, Feb 17, 2024 at 01:14:29PM +0100, Oleksij Rempel wrote: > > On Thu, Feb 15, 2024 at 05:02:55PM +0100, Kory Maincent wrote: > > > Add the PD692x0 I2C Power Sourcing Equipment controller device tree > > > bindings documentation. > > > > > > This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. > > > > > > Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> > > > --- > > ... > > > + pse_pis { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + pse_pi0: pse_pi@0 { > > > + reg = <0>; > > > + #pse-cells = <0>; > > > + pairset-names = "alternative-a", "alternative-b"; > > > + pairsets = <&phys0>, <&phys1>; > > > + }; > > > + pse_pi1: pse_pi@1 { > > > + reg = <1>; > > > + #pse-cells = <0>; > > > + pairset-names = "alternative-a"; > > > + pairsets = <&phys2>; > > > > According to latest discussions, PSE PI nodes will need some > > additional, board specific, information: > > - this controller do not implements polarity switching, we need to know > > what polarity is implemented on this board. The 802.3 spec provide not > > really consistent names for polarity configurations: > > - Alternative A MDI-X > > - Alternative A MDI > > - Alternative B X > > - Alternative B S > > The board may implement one of polarity configurations per alternative > > or have additional helpers to switch them without using PSE > > controller. > > Even if specification explicitly say: > > "The PD shall be implemented to be insensitive to the polarity of the power > > supply and shall be able to operate per the PD Mode A column and the PD > > Mode B column in Table 33–13" > > it is possible to find reports like this: > > https://community.ui.com/questions/M5-cant-take-reversed-power-polarity-/d834d9a8-579d-4f08-80b1-623806cc5070 > > > > Probably this kind of property is a good fit: > > polarity-supported = "MDI-X", "MDI", "X", "S"; > > Where does that live? Looks like a property of the consumers defined in > the provider. Generally, that's not the right way for DT. This is property of PSE PI (Power Interface) Ethernet PHY --\ PSE (provider) ----> PSE PI (consumer of multiple PSE's) ----> Physial port PSE - provides power lines. PSE PI - switches (or not) power lines in different configurations. This is different part of the board/system. PSE PI can have combination or one of following configurations: "MDI-X", "MDI", "X", "S"; This is not something what PSE actually do. PSE PI and PSE are described in IEEE802.3 specification. > I'll say it > again, I think you should be expanding #pse-cells (>1), not getting rid > of them (==0). Did you took time to read my last explanation? Sorry for making it long description, this topic is a bit complex. Regards, Oleksij
diff --git a/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml new file mode 100644 index 000000000000..57ba5365157c --- /dev/null +++ b/Documentation/devicetree/bindings/net/pse-pd/microchip,pd692x0.yaml @@ -0,0 +1,157 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/net/pse-pd/microchip,pd692x0.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Microchip PD692x0 Power Sourcing Equipment controller + +maintainers: + - Kory Maincent <kory.maincent@bootlin.com> + +allOf: + - $ref: pse-controller.yaml# + +properties: + compatible: + enum: + - microchip,pd69200 + - microchip,pd69210 + - microchip,pd69220 + + reg: + maxItems: 1 + + managers: + $ref: "#/$defs/managers" + description: + List of the PD69208T4/PD69204T4/PD69208M PSE managers. Each manager + have 4 or 8 physical ports according to the chip version. No need to + specify the SPI chip select as it is automatically detected by the + PD692x0 PSE controller. The PSE managers have to be described from + the lowest chip select to the greatest one, which is the detection + behavior of the PD692x0 PSE controller. The PD692x0 support up to + 12 PSE managers which can expose up to 96 physical ports. All + physical ports available on a manager have to be described in the + incremental order even if they are not used. + +required: + - compatible + - reg + - pse_pis + +$defs: + manager: + $ref: /schemas/graph.yaml#/properties/ports + + properties: + reg: + maxItems: 1 + + patternProperties: + '^port@[0-7]$': + type: object + required: + - reg + + required: + - reg + + managers: + type: object + + properties: + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + + patternProperties: + "^manager@0[0-9]|1[0-2]$": + $ref: "#/$defs/manager" + + required: + - "#address-cells" + - "#size-cells" + +unevaluatedProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + ethernet-pse@3c { + compatible = "microchip,pd69200"; + reg = <0x3c>; + + managers { + #address-cells = <1>; + #size-cells = <0>; + + manager@0 { + reg = <0>; + #address-cells = <1>; + #size-cells = <0>; + + phys0: port@0 { + reg = <0>; + }; + + phys1: port@1 { + reg = <1>; + }; + + phys2: port@2 { + reg = <2>; + }; + + phys3: port@3 { + reg = <3>; + }; + }; + + manager@1 { + reg = <1>; + #address-cells = <1>; + #size-cells = <0>; + + phys4: port@0 { + reg = <0>; + }; + + phys5: port@1 { + reg = <1>; + }; + + phys6: port@2 { + reg = <2>; + }; + + phys7: port@3 { + reg = <3>; + }; + }; + }; + + pse_pis { + #address-cells = <1>; + #size-cells = <0>; + + pse_pi0: pse_pi@0 { + reg = <0>; + #pse-cells = <0>; + pairset-names = "alternative-a", "alternative-b"; + pairsets = <&phys0>, <&phys1>; + }; + pse_pi1: pse_pi@1 { + reg = <1>; + #pse-cells = <0>; + pairset-names = "alternative-a"; + pairsets = <&phys2>; + }; + }; + }; + };
Add the PD692x0 I2C Power Sourcing Equipment controller device tree bindings documentation. This patch is sponsored by Dent Project <dentproject@linuxfoundation.org>. Signed-off-by: Kory Maincent <kory.maincent@bootlin.com> --- Changes in v2: - Enhance ports-matrix description. - Replace additionalProperties by unevaluatedProperties. - Drop i2c suffix. Changes in v3: - Remove ports-matrix parameter. - Add description of all physical ports and managers. - Add pse_pis subnode moving to the API of pse-controller binding. - Remove the MAINTAINERS section for this driver as I will be maintaining all pse-pd subsystem. --- .../bindings/net/pse-pd/microchip,pd692x0.yaml | 157 +++++++++++++++++++++ 1 file changed, 157 insertions(+)