[net-next,v4,14/17] dt-bindings: net: pse-pd: Add bindings for PD692x0 PSE controller

  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(+)
  

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