[net-next,v2,06/10] dt-bindings: net: Add Synopsys DW xPCS bindings

Commit Message

Serge Semin June 2, 2024, 2:36 p.m. UTC

Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
providing an interface between the Media Access Control (MAC) and Physical
Medium Attachment Sublayer (PMA) through a Media independent interface.
From software point of view it exposes IEEE std. Clause 45 CSR space and
can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
case the PCS device is supposed to be defined under the respective MDIO
bus DT-node. In the later case the DW xPCS will be just a normal IO
memory-mapped device.

Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
source properties specified. The former one indicates the Clause 73/37
auto-negotiation events like: negotiation page received, AN is completed
or incompatible link partner. The clock DT-properties can describe up to
three clock sources: peripheral bus clock source, internal reference clock
and the externally connected reference clock.

Finally the DW XPCS IP-core can be optionally synthesized with a
vendor-specific interface connected to the Synopsys PMA (also called
DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
portable way. So if the DW XPCS device has the respective PMA attached
then it should be reflected in the DT-node compatible string so the driver
would be aware of the PMA-specific device capabilities (mainly connected
with CSRs available for the fine-tunings).

Signed-off-by: Serge Semin <[e-mail]>

---

Changelog v2:
- Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
  interface is just a normal memory-mapped device.
---
 .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
 1 file changed, 133 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml

Comments

Rob Herring (Arm) June 5, 2024, 11:29 p.m. UTC | #1

On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> providing an interface between the Media Access Control (MAC) and Physical
> Medium Attachment Sublayer (PMA) through a Media independent interface.
> >From software point of view it exposes IEEE std. Clause 45 CSR space and
> can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> case the PCS device is supposed to be defined under the respective MDIO
> bus DT-node. In the later case the DW xPCS will be just a normal IO
> memory-mapped device.
> 
> Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> source properties specified. The former one indicates the Clause 73/37
> auto-negotiation events like: negotiation page received, AN is completed
> or incompatible link partner. The clock DT-properties can describe up to
> three clock sources: peripheral bus clock source, internal reference clock
> and the externally connected reference clock.
> 
> Finally the DW XPCS IP-core can be optionally synthesized with a
> vendor-specific interface connected to the Synopsys PMA (also called
> DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> portable way. So if the DW XPCS device has the respective PMA attached
> then it should be reflected in the DT-node compatible string so the driver
> would be aware of the PMA-specific device capabilities (mainly connected
> with CSRs available for the fine-tunings).
> 
> Signed-off-by: Serge Semin <[e-mail]>
> 
> ---
> 
> Changelog v2:
> - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
>   interface is just a normal memory-mapped device.
> ---
>  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
>  1 file changed, 133 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> new file mode 100644
> index 000000000000..7927bceefbf3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> @@ -0,0 +1,133 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Synopsys DesignWare Ethernet PCS
> +
> +maintainers:
> +  - Serge Semin <[e-mail]>
> +
> +description:
> +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> +  between Media Access Control and Physical Medium Attachment Sublayer through
> +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> +  optionally synthesized with a vendor-specific interface connected to
> +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> +  general it can be used to communicate with any compatible PHY.
> +
> +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> +  right to the system IO memory space.
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> +        const: snps,dw-xpcs
> +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> +        const: snps,dw-xpcs-gen1-3g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> +        const: snps,dw-xpcs-gen2-3g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> +        const: snps,dw-xpcs-gen2-6g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> +        const: snps,dw-xpcs-gen4-3g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> +        const: snps,dw-xpcs-gen4-6g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> +        const: snps,dw-xpcs-gen5-10g
> +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> +        const: snps,dw-xpcs-gen5-12g
> +
> +  reg:
> +    items:
> +      - description:
> +          In case of the MDIO management interface this just a 5-bits ID
> +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> +          MCI or APB3 management interfaces, then the space mapping can be
> +          either 'direct' or 'indirect'. In the former case all Clause 45
> +          registers are contiguously mapped within the address space
> +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> +          to the multiple 256 register sets. There is a special viewport CSR
> +          which is responsible for the set selection. The upper part of
> +          the CSR address MMD+REG[20:8] is supposed to be written in there
> +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> +
> +  reg-names:
> +    items:
> +      - enum: [ direct, indirect ]
> +
> +  reg-io-width:
> +    description:
> +      The way the CSRs are mapped to the memory is platform depended. Since
> +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> +      two bytes aligned at least.
> +    default: 2
> +    enum: [ 2, 4 ]
> +
> +  interrupts:
> +    description:
> +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> +      auto-negotiation events':' Page received, AN is completed or incompatible
> +      link partner.
> +    maxItems: 1
> +
> +  clocks:
> +    description:
> +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> +      source connected via the clk_csr_i line.
> +
> +      PCS/PMA layer can be clocked by an internal reference clock source
> +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> +      generator. Both clocks can be supplied at a time.
> +    minItems: 1
> +    maxItems: 3
> +
> +  clock-names:
> +    minItems: 1
> +    maxItems: 3
> +    anyOf:
> +      - items:
> +          enum: [ core, pad ]

This has no effect. If it is true, then the 2nd entry is too.

You are saying all the clocks are optional and any combination/order is 
valid. Do we really need it so flexible? Doubtful the h/w is that 
flexible.

> +      - items:
> +          enum: [ pclk, core, pad ]
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    ethernet-pcs@1f05d000 {
> +      compatible = "snps,dw-xpcs";
> +      reg = <0x1f05d000 0x1000>;
> +      reg-names = "indirect";
> +
> +      reg-io-width = <4>;
> +
> +      interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
> +
> +      clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
> +      clock-names = "pclk", "core", "pad";
> +    };
> +  - |
> +    mdio-bus {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      ethernet-pcs@0 {
> +        compatible = "snps,dw-xpcs";
> +        reg = <0>;
> +
> +        clocks = <&ccu_core>, <&ccu_pad>;
> +        clock-names = "core", "pad";
> +      };
> +    };
> +...
> -- 
> 2.43.0
>

Serge Semin June 6, 2024, 9:54 a.m. UTC | #2

On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > providing an interface between the Media Access Control (MAC) and Physical
> > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > case the PCS device is supposed to be defined under the respective MDIO
> > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > memory-mapped device.
> > 
> > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > source properties specified. The former one indicates the Clause 73/37
> > auto-negotiation events like: negotiation page received, AN is completed
> > or incompatible link partner. The clock DT-properties can describe up to
> > three clock sources: peripheral bus clock source, internal reference clock
> > and the externally connected reference clock.
> > 
> > Finally the DW XPCS IP-core can be optionally synthesized with a
> > vendor-specific interface connected to the Synopsys PMA (also called
> > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > portable way. So if the DW XPCS device has the respective PMA attached
> > then it should be reflected in the DT-node compatible string so the driver
> > would be aware of the PMA-specific device capabilities (mainly connected
> > with CSRs available for the fine-tunings).
> > 
> > Signed-off-by: Serge Semin <[e-mail]>
> > 
> > ---
> > 
> > Changelog v2:
> > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> >   interface is just a normal memory-mapped device.
> > ---
> >  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
> >  1 file changed, 133 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > new file mode 100644
> > index 000000000000..7927bceefbf3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > @@ -0,0 +1,133 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Synopsys DesignWare Ethernet PCS
> > +
> > +maintainers:
> > +  - Serge Semin <[e-mail]>
> > +
> > +description:
> > +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > +  between Media Access Control and Physical Medium Attachment Sublayer through
> > +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > +  optionally synthesized with a vendor-specific interface connected to
> > +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > +  general it can be used to communicate with any compatible PHY.
> > +
> > +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > +  right to the system IO memory space.
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> > +        const: snps,dw-xpcs
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > +        const: snps,dw-xpcs-gen1-3g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > +        const: snps,dw-xpcs-gen2-3g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > +        const: snps,dw-xpcs-gen2-6g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > +        const: snps,dw-xpcs-gen4-3g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > +        const: snps,dw-xpcs-gen4-6g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > +        const: snps,dw-xpcs-gen5-10g
> > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > +        const: snps,dw-xpcs-gen5-12g
> > +
> > +  reg:
> > +    items:
> > +      - description:
> > +          In case of the MDIO management interface this just a 5-bits ID
> > +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > +          MCI or APB3 management interfaces, then the space mapping can be
> > +          either 'direct' or 'indirect'. In the former case all Clause 45
> > +          registers are contiguously mapped within the address space
> > +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > +          to the multiple 256 register sets. There is a special viewport CSR
> > +          which is responsible for the set selection. The upper part of
> > +          the CSR address MMD+REG[20:8] is supposed to be written in there
> > +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> > +
> > +  reg-names:
> > +    items:
> > +      - enum: [ direct, indirect ]
> > +
> > +  reg-io-width:
> > +    description:
> > +      The way the CSRs are mapped to the memory is platform depended. Since
> > +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> > +      two bytes aligned at least.
> > +    default: 2
> > +    enum: [ 2, 4 ]
> > +
> > +  interrupts:
> > +    description:
> > +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > +      auto-negotiation events':' Page received, AN is completed or incompatible
> > +      link partner.
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    description:
> > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > +      source connected via the clk_csr_i line.
> > +
> > +      PCS/PMA layer can be clocked by an internal reference clock source
> > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > +      generator. Both clocks can be supplied at a time.
> > +    minItems: 1
> > +    maxItems: 3
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    maxItems: 3
> > +    anyOf:
> > +      - items:
> > +          enum: [ core, pad ]
> 

> This has no effect. If it is true, then the 2nd entry is too.

Yeah, from the anyOf logic it's redundant indeed. But the idea was to
signify that the DT-node may have one the next clock-names
combination:
   clock-names = "pad";
or clock-names = "core";
or clock-names = "core", "pad";
or clock-names = "pclk";
or clock-names = "pclk", "core";
or clock-names = "pclk", "pad";
or clock-names = "pclk", "core", "pad";
> 
> You are saying all the clocks are optional and any combination/order is 
> valid. Do we really need it so flexible? Doubtful the h/w is that 
> flexible.

Well, I failed to figure out a more restrictive but still simple
constraint. Here are the conditions which need to be taken into
account:
1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
found under normal system bus super-node). DT-nodes placed under the
MDIO-bus super-node obviously have the MDIO-bus communication channel
which is clocked by the internal clock generator.
2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
v3.11a HW-manual.) Both of these clock sources can be specified at a
time. So it's the software responsibility to choose which one to use.

So based on the notes above it's still possible to have no clock
source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
ref-clock required.

Any idea of how to implement the constraint with these conditions
followed?

-Serge(y)

> 
> > +      - items:
> > +          enum: [ pclk, core, pad ]
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +
> > +    ethernet-pcs@1f05d000 {
> > +      compatible = "snps,dw-xpcs";
> > +      reg = <0x1f05d000 0x1000>;
> > +      reg-names = "indirect";
> > +
> > +      reg-io-width = <4>;
> > +
> > +      interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
> > +
> > +      clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
> > +      clock-names = "pclk", "core", "pad";
> > +    };
> > +  - |
> > +    mdio-bus {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      ethernet-pcs@0 {
> > +        compatible = "snps,dw-xpcs";
> > +        reg = <0>;
> > +
> > +        clocks = <&ccu_core>, <&ccu_pad>;
> > +        clock-names = "core", "pad";
> > +      };
> > +    };
> > +...
> > -- 
> > 2.43.0
> >

Rob Herring (Arm) June 10, 2024, 9:49 p.m. UTC | #3

On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > providing an interface between the Media Access Control (MAC) and Physical
> > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > case the PCS device is supposed to be defined under the respective MDIO
> > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > memory-mapped device.
> > > 
> > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > source properties specified. The former one indicates the Clause 73/37
> > > auto-negotiation events like: negotiation page received, AN is completed
> > > or incompatible link partner. The clock DT-properties can describe up to
> > > three clock sources: peripheral bus clock source, internal reference clock
> > > and the externally connected reference clock.
> > > 
> > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > vendor-specific interface connected to the Synopsys PMA (also called
> > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > portable way. So if the DW XPCS device has the respective PMA attached
> > > then it should be reflected in the DT-node compatible string so the driver
> > > would be aware of the PMA-specific device capabilities (mainly connected
> > > with CSRs available for the fine-tunings).
> > > 
> > > Signed-off-by: Serge Semin <[e-mail]>
> > > 
> > > ---
> > > 
> > > Changelog v2:
> > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > >   interface is just a normal memory-mapped device.
> > > ---
> > >  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
> > >  1 file changed, 133 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > new file mode 100644
> > > index 000000000000..7927bceefbf3
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > @@ -0,0 +1,133 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Synopsys DesignWare Ethernet PCS
> > > +
> > > +maintainers:
> > > +  - Serge Semin <[e-mail]>
> > > +
> > > +description:
> > > +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > +  between Media Access Control and Physical Medium Attachment Sublayer through
> > > +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > +  optionally synthesized with a vendor-specific interface connected to
> > > +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > +  general it can be used to communicate with any compatible PHY.
> > > +
> > > +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > +  right to the system IO memory space.
> > > +
> > > +properties:
> > > +  compatible:
> > > +    oneOf:
> > > +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > +        const: snps,dw-xpcs
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > +        const: snps,dw-xpcs-gen1-3g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > +        const: snps,dw-xpcs-gen2-3g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > +        const: snps,dw-xpcs-gen2-6g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > +        const: snps,dw-xpcs-gen4-3g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > +        const: snps,dw-xpcs-gen4-6g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > +        const: snps,dw-xpcs-gen5-10g
> > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > +        const: snps,dw-xpcs-gen5-12g
> > > +
> > > +  reg:
> > > +    items:
> > > +      - description:
> > > +          In case of the MDIO management interface this just a 5-bits ID
> > > +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > +          MCI or APB3 management interfaces, then the space mapping can be
> > > +          either 'direct' or 'indirect'. In the former case all Clause 45
> > > +          registers are contiguously mapped within the address space
> > > +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > +          to the multiple 256 register sets. There is a special viewport CSR
> > > +          which is responsible for the set selection. The upper part of
> > > +          the CSR address MMD+REG[20:8] is supposed to be written in there
> > > +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > +
> > > +  reg-names:
> > > +    items:
> > > +      - enum: [ direct, indirect ]
> > > +
> > > +  reg-io-width:
> > > +    description:
> > > +      The way the CSRs are mapped to the memory is platform depended. Since
> > > +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > +      two bytes aligned at least.
> > > +    default: 2
> > > +    enum: [ 2, 4 ]
> > > +
> > > +  interrupts:
> > > +    description:
> > > +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > +      auto-negotiation events':' Page received, AN is completed or incompatible
> > > +      link partner.
> > > +    maxItems: 1
> > > +
> > > +  clocks:
> > > +    description:
> > > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > +      source connected via the clk_csr_i line.
> > > +
> > > +      PCS/PMA layer can be clocked by an internal reference clock source
> > > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > +      generator. Both clocks can be supplied at a time.
> > > +    minItems: 1
> > > +    maxItems: 3
> > > +
> > > +  clock-names:
> > > +    minItems: 1
> > > +    maxItems: 3
> > > +    anyOf:
> > > +      - items:
> > > +          enum: [ core, pad ]
> > 
> 
> > This has no effect. If it is true, then the 2nd entry is too.
> 
> Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> signify that the DT-node may have one the next clock-names
> combination:
>    clock-names = "pad";
> or clock-names = "core";
> or clock-names = "core", "pad";
> or clock-names = "pclk";
> or clock-names = "pclk", "core";
> or clock-names = "pclk", "pad";
> or clock-names = "pclk", "core", "pad";

That would be:

oneOf:
  - minItems: 1
    items:
      - enum: [core, pad]
      - const: pad
  - minItems: 1
    items:
      - const: pclk
      - enum: [core, pad]
      - const: pad

*-names is enforced to be 'uniqueItems: true', so we don't have to worry 
about repeated entries.

This also nicely splits between MMIO and MDIO.

> > 
> > You are saying all the clocks are optional and any combination/order is 
> > valid. Do we really need it so flexible? Doubtful the h/w is that 
> > flexible.
> 
> Well, I failed to figure out a more restrictive but still simple
> constraint. Here are the conditions which need to be taken into
> account:
> 1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
> found under normal system bus super-node). DT-nodes placed under the
> MDIO-bus super-node obviously have the MDIO-bus communication channel
> which is clocked by the internal clock generator.
> 2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
> clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
> and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
> v3.11a HW-manual.) Both of these clock sources can be specified at a
> time. So it's the software responsibility to choose which one to use.
> 
> So based on the notes above it's still possible to have no clock
> source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
> ref-clock required.
> 
> Any idea of how to implement the constraint with these conditions
> followed?
> 
> -Serge(y)

Serge Semin June 11, 2024, 10:45 a.m. UTC | #4

Hi Rob

On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > memory-mapped device.
> > > > 
> > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > source properties specified. The former one indicates the Clause 73/37
> > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > and the externally connected reference clock.
> > > > 
> > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > then it should be reflected in the DT-node compatible string so the driver
> > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > with CSRs available for the fine-tunings).
> > > > 
> > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > 
> > > > ---
> > > > 
> > > > Changelog v2:
> > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > >   interface is just a normal memory-mapped device.
> > > > ---
> > > >  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
> > > >  1 file changed, 133 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > new file mode 100644
> > > > index 000000000000..7927bceefbf3
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > @@ -0,0 +1,133 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: Synopsys DesignWare Ethernet PCS
> > > > +
> > > > +maintainers:
> > > > +  - Serge Semin <[e-mail]>
> > > > +
> > > > +description:
> > > > +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > +  between Media Access Control and Physical Medium Attachment Sublayer through
> > > > +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > +  optionally synthesized with a vendor-specific interface connected to
> > > > +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > +  general it can be used to communicate with any compatible PHY.
> > > > +
> > > > +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > +  right to the system IO memory space.
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > +        const: snps,dw-xpcs
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > +        const: snps,dw-xpcs-gen1-3g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > +        const: snps,dw-xpcs-gen2-3g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > +        const: snps,dw-xpcs-gen2-6g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > +        const: snps,dw-xpcs-gen4-3g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > +        const: snps,dw-xpcs-gen4-6g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > +        const: snps,dw-xpcs-gen5-10g
> > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > +        const: snps,dw-xpcs-gen5-12g
> > > > +
> > > > +  reg:
> > > > +    items:
> > > > +      - description:
> > > > +          In case of the MDIO management interface this just a 5-bits ID
> > > > +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > +          MCI or APB3 management interfaces, then the space mapping can be
> > > > +          either 'direct' or 'indirect'. In the former case all Clause 45
> > > > +          registers are contiguously mapped within the address space
> > > > +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > +          to the multiple 256 register sets. There is a special viewport CSR
> > > > +          which is responsible for the set selection. The upper part of
> > > > +          the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > +
> > > > +  reg-names:
> > > > +    items:
> > > > +      - enum: [ direct, indirect ]
> > > > +
> > > > +  reg-io-width:
> > > > +    description:
> > > > +      The way the CSRs are mapped to the memory is platform depended. Since
> > > > +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > +      two bytes aligned at least.
> > > > +    default: 2
> > > > +    enum: [ 2, 4 ]
> > > > +
> > > > +  interrupts:
> > > > +    description:
> > > > +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > +      auto-negotiation events':' Page received, AN is completed or incompatible
> > > > +      link partner.
> > > > +    maxItems: 1
> > > > +
> > > > +  clocks:
> > > > +    description:
> > > > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > +      source connected via the clk_csr_i line.
> > > > +
> > > > +      PCS/PMA layer can be clocked by an internal reference clock source
> > > > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > +      generator. Both clocks can be supplied at a time.
> > > > +    minItems: 1
> > > > +    maxItems: 3
> > > > +
> > > > +  clock-names:
> > > > +    minItems: 1
> > > > +    maxItems: 3
> > > > +    anyOf:
> > > > +      - items:
> > > > +          enum: [ core, pad ]
> > > 
> > 
> > > This has no effect. If it is true, then the 2nd entry is too.
> > 
> > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > signify that the DT-node may have one the next clock-names
> > combination:
> >    clock-names = "pad";
> > or clock-names = "core";
> > or clock-names = "core", "pad";
> > or clock-names = "pclk";
> > or clock-names = "pclk", "core";
> > or clock-names = "pclk", "pad";
> > or clock-names = "pclk", "core", "pad";
> 
> That would be:
> 
> oneOf:
>   - minItems: 1
>     items:
>       - enum: [core, pad]
>       - const: pad
>   - minItems: 1
>     items:
>       - const: pclk
>       - enum: [core, pad]
>       - const: pad
> 
> *-names is enforced to be 'uniqueItems: true', so we don't have to worry 
> about repeated entries.
> 
> This also nicely splits between MMIO and MDIO.

I had such approach in mind, but it seemed to me more complicated and
weakly scaleable (should we need to add some more clocks). Isn't the
next constraint look more readable:

anyOf:
  - description: DW XPCS accessible over MDIO-bus
    minItems: 1
    maxItems: 2
    items:
      enum: [core, pad]
  - description: DW XPCS with the MCI/APB3 CSRs IO interface
    minItems: 1
    maxItems: 3
    items:
      enum: [pclk, core, pad]
    contains:
      const: pclk
?

AFAICS The only reason of using the pattern suggested by you would be
to define the ordered clock phandle settings. Is it so necessary
that we should sacrifice the readability in favor of the more strict
and less scalable solution?

-Serge(y)

> 
> > > 
> > > You are saying all the clocks are optional and any combination/order is 
> > > valid. Do we really need it so flexible? Doubtful the h/w is that 
> > > flexible.
> > 
> > Well, I failed to figure out a more restrictive but still simple
> > constraint. Here are the conditions which need to be taken into
> > account:
> > 1. "pclk" is specific for the memory-mapped DW XPCS only (DT-nodes
> > found under normal system bus super-node). DT-nodes placed under the
> > MDIO-bus super-node obviously have the MDIO-bus communication channel
> > which is clocked by the internal clock generator.
> > 2. "core" (also mentioned as "alt" in the HW-databooks) and "pad"
> > clock sources can be found on XPCS with DW Enterprise Gen2, Gen4, Gen5
> > and Gen6 PMAs. (At least that's what I managed to find in the DW XPCS
> > v3.11a HW-manual.) Both of these clock sources can be specified at a
> > time. So it's the software responsibility to choose which one to use.
> > 
> > So based on the notes above it's still possible to have no clock
> > source specified if it's an MDIO-based DW XPCS with a PMA/PHY with no
> > ref-clock required.
> > 
> > Any idea of how to implement the constraint with these conditions
> > followed?
> > 
> > -Serge(y)

Rob Herring (Arm) June 13, 2024, 3:41 p.m. UTC | #5

On Tue, Jun 11, 2024 at 01:45:16PM +0300, Serge Semin wrote:
> Hi Rob
> 
> On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> > On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > > memory-mapped device.
> > > > > 
> > > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > > source properties specified. The former one indicates the Clause 73/37
> > > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > > and the externally connected reference clock.
> > > > > 
> > > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > > then it should be reflected in the DT-node compatible string so the driver
> > > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > > with CSRs available for the fine-tunings).
> > > > > 
> > > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > > 
> > > > > ---
> > > > > 
> > > > > Changelog v2:
> > > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > > >   interface is just a normal memory-mapped device.
> > > > > ---
> > > > >  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
> > > > >  1 file changed, 133 insertions(+)
> > > > >  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > new file mode 100644
> > > > > index 000000000000..7927bceefbf3
> > > > > --- /dev/null
> > > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > @@ -0,0 +1,133 @@
> > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Synopsys DesignWare Ethernet PCS
> > > > > +
> > > > > +maintainers:
> > > > > +  - Serge Semin <[e-mail]>
> > > > > +
> > > > > +description:
> > > > > +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > > +  between Media Access Control and Physical Medium Attachment Sublayer through
> > > > > +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > > +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > > +  optionally synthesized with a vendor-specific interface connected to
> > > > > +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > > +  general it can be used to communicate with any compatible PHY.
> > > > > +
> > > > > +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > > +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > > +  right to the system IO memory space.
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +    oneOf:
> > > > > +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > > +        const: snps,dw-xpcs
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > > +        const: snps,dw-xpcs-gen1-3g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > > +        const: snps,dw-xpcs-gen2-3g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > > +        const: snps,dw-xpcs-gen2-6g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > > +        const: snps,dw-xpcs-gen4-3g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > > +        const: snps,dw-xpcs-gen4-6g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > > +        const: snps,dw-xpcs-gen5-10g
> > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > > +        const: snps,dw-xpcs-gen5-12g
> > > > > +
> > > > > +  reg:
> > > > > +    items:
> > > > > +      - description:
> > > > > +          In case of the MDIO management interface this just a 5-bits ID
> > > > > +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > > +          MCI or APB3 management interfaces, then the space mapping can be
> > > > > +          either 'direct' or 'indirect'. In the former case all Clause 45
> > > > > +          registers are contiguously mapped within the address space
> > > > > +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > > +          to the multiple 256 register sets. There is a special viewport CSR
> > > > > +          which is responsible for the set selection. The upper part of
> > > > > +          the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > > +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > > +
> > > > > +  reg-names:
> > > > > +    items:
> > > > > +      - enum: [ direct, indirect ]
> > > > > +
> > > > > +  reg-io-width:
> > > > > +    description:
> > > > > +      The way the CSRs are mapped to the memory is platform depended. Since
> > > > > +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > > +      two bytes aligned at least.
> > > > > +    default: 2
> > > > > +    enum: [ 2, 4 ]
> > > > > +
> > > > > +  interrupts:
> > > > > +    description:
> > > > > +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > > +      auto-negotiation events':' Page received, AN is completed or incompatible
> > > > > +      link partner.
> > > > > +    maxItems: 1
> > > > > +
> > > > > +  clocks:
> > > > > +    description:
> > > > > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > > +      source connected via the clk_csr_i line.
> > > > > +
> > > > > +      PCS/PMA layer can be clocked by an internal reference clock source
> > > > > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > > +      generator. Both clocks can be supplied at a time.
> > > > > +    minItems: 1
> > > > > +    maxItems: 3
> > > > > +
> > > > > +  clock-names:
> > > > > +    minItems: 1
> > > > > +    maxItems: 3
> > > > > +    anyOf:
> > > > > +      - items:
> > > > > +          enum: [ core, pad ]
> > > > 
> > > 
> > > > This has no effect. If it is true, then the 2nd entry is too.
> > > 
> > > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > > signify that the DT-node may have one the next clock-names
> > > combination:
> > >    clock-names = "pad";
> > > or clock-names = "core";
> > > or clock-names = "core", "pad";
> > > or clock-names = "pclk";
> > > or clock-names = "pclk", "core";
> > > or clock-names = "pclk", "pad";
> > > or clock-names = "pclk", "core", "pad";
> > 
> > That would be:
> > 
> > oneOf:
> >   - minItems: 1
> >     items:
> >       - enum: [core, pad]
> >       - const: pad
> >   - minItems: 1
> >     items:
> >       - const: pclk
> >       - enum: [core, pad]
> >       - const: pad
> > 
> > *-names is enforced to be 'uniqueItems: true', so we don't have to worry 
> > about repeated entries.
> > 
> > This also nicely splits between MMIO and MDIO.
> 
> I had such approach in mind, but it seemed to me more complicated and
> weakly scaleable (should we need to add some more clocks). Isn't the
> next constraint look more readable:

Hardware is magically growing more clocks?

 
> anyOf:
>   - description: DW XPCS accessible over MDIO-bus
>     minItems: 1
>     maxItems: 2
>     items:
>       enum: [core, pad]
>   - description: DW XPCS with the MCI/APB3 CSRs IO interface
>     minItems: 1
>     maxItems: 3
>     items:
>       enum: [pclk, core, pad]
>     contains:
>       const: pclk
> ?

I don't see how that is much better in simplicity or scaleability. I 
would just do this over the above:

minItems: 1
maxItems: 3
items:
  enum: [pclk, core, pad]

Either you define the order or you don't. The former is strongly 
preferred. The latter is done when it's too much a mess or we just don't 
care to discuss it any more.

Rob

Serge Semin June 13, 2024, 8:19 p.m. UTC | #6

On Thu, Jun 13, 2024 at 09:41:25AM -0600, Rob Herring wrote:
> On Tue, Jun 11, 2024 at 01:45:16PM +0300, Serge Semin wrote:
> > Hi Rob
> > 
> > On Mon, Jun 10, 2024 at 03:49:16PM -0600, Rob Herring wrote:
> > > On Thu, Jun 06, 2024 at 12:54:33PM +0300, Serge Semin wrote:
> > > > On Wed, Jun 05, 2024 at 05:29:16PM -0600, Rob Herring wrote:
> > > > > On Sun, Jun 02, 2024 at 05:36:20PM +0300, Serge Semin wrote:
> > > > > > Synopsys DesignWare XPCS IP-core is a Physical Coding Sublayer (PCS) layer
> > > > > > providing an interface between the Media Access Control (MAC) and Physical
> > > > > > Medium Attachment Sublayer (PMA) through a Media independent interface.
> > > > > > >From software point of view it exposes IEEE std. Clause 45 CSR space and
> > > > > > can be accessible either by MDIO or MCI/APB3 bus interfaces. In the former
> > > > > > case the PCS device is supposed to be defined under the respective MDIO
> > > > > > bus DT-node. In the later case the DW xPCS will be just a normal IO
> > > > > > memory-mapped device.
> > > > > > 
> > > > > > Besides of that DW XPCS DT-nodes can have an interrupt signal and clock
> > > > > > source properties specified. The former one indicates the Clause 73/37
> > > > > > auto-negotiation events like: negotiation page received, AN is completed
> > > > > > or incompatible link partner. The clock DT-properties can describe up to
> > > > > > three clock sources: peripheral bus clock source, internal reference clock
> > > > > > and the externally connected reference clock.
> > > > > > 
> > > > > > Finally the DW XPCS IP-core can be optionally synthesized with a
> > > > > > vendor-specific interface connected to the Synopsys PMA (also called
> > > > > > DesignWare Consumer/Enterprise PHY). Alas that isn't auto-detectable in a
> > > > > > portable way. So if the DW XPCS device has the respective PMA attached
> > > > > > then it should be reflected in the DT-node compatible string so the driver
> > > > > > would be aware of the PMA-specific device capabilities (mainly connected
> > > > > > with CSRs available for the fine-tunings).
> > > > > > 
> > > > > > Signed-off-by: Serge Semin <[e-mail]>
> > > > > > 
> > > > > > ---
> > > > > > 
> > > > > > Changelog v2:
> > > > > > - Drop the Management Interface DT-node bindings. DW xPCS with MCI/APB3
> > > > > >   interface is just a normal memory-mapped device.
> > > > > > ---
> > > > > >  .../bindings/net/pcs/snps,dw-xpcs.yaml        | 133 ++++++++++++++++++
> > > > > >  1 file changed, 133 insertions(+)
> > > > > >  create mode 100644 Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > > 
> > > > > > diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > > new file mode 100644
> > > > > > index 000000000000..7927bceefbf3
> > > > > > --- /dev/null
> > > > > > +++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
> > > > > > @@ -0,0 +1,133 @@
> > > > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Synopsys DesignWare Ethernet PCS
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Serge Semin <[e-mail]>
> > > > > > +
> > > > > > +description:
> > > > > > +  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
> > > > > > +  between Media Access Control and Physical Medium Attachment Sublayer through
> > > > > > +  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
> > > > > > +  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
> > > > > > +  optionally synthesized with a vendor-specific interface connected to
> > > > > > +  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
> > > > > > +  general it can be used to communicate with any compatible PHY.
> > > > > > +
> > > > > > +  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
> > > > > > +  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
> > > > > > +  right to the system IO memory space.
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +    oneOf:
> > > > > > +      - description: Synopsys DesignWare XPCS with none or unknown PMA
> > > > > > +        const: snps,dw-xpcs
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
> > > > > > +        const: snps,dw-xpcs-gen1-3g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
> > > > > > +        const: snps,dw-xpcs-gen2-3g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
> > > > > > +        const: snps,dw-xpcs-gen2-6g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
> > > > > > +        const: snps,dw-xpcs-gen4-3g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
> > > > > > +        const: snps,dw-xpcs-gen4-6g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
> > > > > > +        const: snps,dw-xpcs-gen5-10g
> > > > > > +      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
> > > > > > +        const: snps,dw-xpcs-gen5-12g
> > > > > > +
> > > > > > +  reg:
> > > > > > +    items:
> > > > > > +      - description:
> > > > > > +          In case of the MDIO management interface this just a 5-bits ID
> > > > > > +          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
> > > > > > +          MCI or APB3 management interfaces, then the space mapping can be
> > > > > > +          either 'direct' or 'indirect'. In the former case all Clause 45
> > > > > > +          registers are contiguously mapped within the address space
> > > > > > +          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
> > > > > > +          to the multiple 256 register sets. There is a special viewport CSR
> > > > > > +          which is responsible for the set selection. The upper part of
> > > > > > +          the CSR address MMD+REG[20:8] is supposed to be written in there
> > > > > > +          so the corresponding subset would be mapped to the lowest 255 CSRs.
> > > > > > +
> > > > > > +  reg-names:
> > > > > > +    items:
> > > > > > +      - enum: [ direct, indirect ]
> > > > > > +
> > > > > > +  reg-io-width:
> > > > > > +    description:
> > > > > > +      The way the CSRs are mapped to the memory is platform depended. Since
> > > > > > +      each Clause 45 CSR is of 16-bits wide the access instructions must be
> > > > > > +      two bytes aligned at least.
> > > > > > +    default: 2
> > > > > > +    enum: [ 2, 4 ]
> > > > > > +
> > > > > > +  interrupts:
> > > > > > +    description:
> > > > > > +      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
> > > > > > +      auto-negotiation events':' Page received, AN is completed or incompatible
> > > > > > +      link partner.
> > > > > > +    maxItems: 1
> > > > > > +
> > > > > > +  clocks:
> > > > > > +    description:
> > > > > > +      Both MCI and APB3 interfaces are supposed to be equipped with a clock
> > > > > > +      source connected via the clk_csr_i line.
> > > > > > +
> > > > > > +      PCS/PMA layer can be clocked by an internal reference clock source
> > > > > > +      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
> > > > > > +      generator. Both clocks can be supplied at a time.
> > > > > > +    minItems: 1
> > > > > > +    maxItems: 3
> > > > > > +
> > > > > > +  clock-names:
> > > > > > +    minItems: 1
> > > > > > +    maxItems: 3
> > > > > > +    anyOf:
> > > > > > +      - items:
> > > > > > +          enum: [ core, pad ]
> > > > > 
> > > > 
> > > > > This has no effect. If it is true, then the 2nd entry is too.
> > > > 
> > > > Yeah, from the anyOf logic it's redundant indeed. But the idea was to
> > > > signify that the DT-node may have one the next clock-names
> > > > combination:
> > > >    clock-names = "pad";
> > > > or clock-names = "core";
> > > > or clock-names = "core", "pad";
> > > > or clock-names = "pclk";
> > > > or clock-names = "pclk", "core";
> > > > or clock-names = "pclk", "pad";
> > > > or clock-names = "pclk", "core", "pad";
> > > 
> > > That would be:
> > > 
> > > oneOf:
> > >   - minItems: 1
> > >     items:
> > >       - enum: [core, pad]
> > >       - const: pad
> > >   - minItems: 1
> > >     items:
> > >       - const: pclk
> > >       - enum: [core, pad]
> > >       - const: pad
> > > 
> > > *-names is enforced to be 'uniqueItems: true', so we don't have to worry 
> > > about repeated entries.
> > > 
> > > This also nicely splits between MMIO and MDIO.
> > 
> > I had such approach in mind, but it seemed to me more complicated and
> > weakly scaleable (should we need to add some more clocks). Isn't the
> > next constraint look more readable:
> 
> Hardware is magically growing more clocks?

There is a non-zero probability I could have missed some additional
clocks defined in the DW XPCS IP-core databooks or there are some
vendor-specific clock sources we can't predict. Moreover the new
IP-core releases may have the clock sources list extended. So the
magic may happen.)

> 
>  
> > anyOf:
> >   - description: DW XPCS accessible over MDIO-bus
> >     minItems: 1
> >     maxItems: 2
> >     items:
> >       enum: [core, pad]
> >   - description: DW XPCS with the MCI/APB3 CSRs IO interface
> >     minItems: 1
> >     maxItems: 3
> >     items:
> >       enum: [pclk, core, pad]
> >     contains:
> >       const: pclk
> > ?
> 
> I don't see how that is much better in simplicity or scaleability. I 
> would just do this over the above:
> 
> minItems: 1
> maxItems: 3
> items:
>   enum: [pclk, core, pad]
> 
> Either you define the order or you don't. The former is strongly 
> preferred. The latter is done when it's too much a mess or we just don't 
> care to discuss it any more.

Ok. Since the order-based constraint is strongly preferable, then
there is nothing to discuss. I'll make sure the order is defined.
Thanks for review.

-Serge(y)

> 
> Rob

Patch

diff --git a/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
new file mode 100644
index 000000000000..7927bceefbf3
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/pcs/snps,dw-xpcs.yaml
@@ -0,0 +1,133 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/pcs/snps,dw-xpcs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare Ethernet PCS
+
+maintainers:
+  - Serge Semin <[e-mail]>
+
+description:
+  Synopsys DesignWare Ethernet Physical Coding Sublayer provides an interface
+  between Media Access Control and Physical Medium Attachment Sublayer through
+  the Media Independent Interface (XGMII, USXGMII, XLGMII, GMII, etc)
+  controlled by means of the IEEE std. Clause 45 registers set. The PCS can be
+  optionally synthesized with a vendor-specific interface connected to
+  Synopsys PMA (also called DesignWare Consumer/Enterprise PHY) although in
+  general it can be used to communicate with any compatible PHY.
+
+  The PCS CSRs can be accessible either over the Ethernet MDIO bus or directly
+  by means of the APB3/MCI interfaces. In the later case the XPCS can be mapped
+  right to the system IO memory space.
+
+properties:
+  compatible:
+    oneOf:
+      - description: Synopsys DesignWare XPCS with none or unknown PMA
+        const: snps,dw-xpcs
+      - description: Synopsys DesignWare XPCS with Consumer Gen1 3G PMA
+        const: snps,dw-xpcs-gen1-3g
+      - description: Synopsys DesignWare XPCS with Consumer Gen2 3G PMA
+        const: snps,dw-xpcs-gen2-3g
+      - description: Synopsys DesignWare XPCS with Consumer Gen2 6G PMA
+        const: snps,dw-xpcs-gen2-6g
+      - description: Synopsys DesignWare XPCS with Consumer Gen4 3G PMA
+        const: snps,dw-xpcs-gen4-3g
+      - description: Synopsys DesignWare XPCS with Consumer Gen4 6G PMA
+        const: snps,dw-xpcs-gen4-6g
+      - description: Synopsys DesignWare XPCS with Consumer Gen5 10G PMA
+        const: snps,dw-xpcs-gen5-10g
+      - description: Synopsys DesignWare XPCS with Consumer Gen5 12G PMA
+        const: snps,dw-xpcs-gen5-12g
+
+  reg:
+    items:
+      - description:
+          In case of the MDIO management interface this just a 5-bits ID
+          of the MDIO bus device. If DW XPCS CSRs space is accessed over the
+          MCI or APB3 management interfaces, then the space mapping can be
+          either 'direct' or 'indirect'. In the former case all Clause 45
+          registers are contiguously mapped within the address space
+          MMD '[20:16]', Reg '[15:0]'. In the later case the space is divided
+          to the multiple 256 register sets. There is a special viewport CSR
+          which is responsible for the set selection. The upper part of
+          the CSR address MMD+REG[20:8] is supposed to be written in there
+          so the corresponding subset would be mapped to the lowest 255 CSRs.
+
+  reg-names:
+    items:
+      - enum: [ direct, indirect ]
+
+  reg-io-width:
+    description:
+      The way the CSRs are mapped to the memory is platform depended. Since
+      each Clause 45 CSR is of 16-bits wide the access instructions must be
+      two bytes aligned at least.
+    default: 2
+    enum: [ 2, 4 ]
+
+  interrupts:
+    description:
+      System interface interrupt output (sbd_intr_o) indicating Clause 73/37
+      auto-negotiation events':' Page received, AN is completed or incompatible
+      link partner.
+    maxItems: 1
+
+  clocks:
+    description:
+      Both MCI and APB3 interfaces are supposed to be equipped with a clock
+      source connected via the clk_csr_i line.
+
+      PCS/PMA layer can be clocked by an internal reference clock source
+      (phyN_core_refclk) or by an externally connected (phyN_pad_refclk) clock
+      generator. Both clocks can be supplied at a time.
+    minItems: 1
+    maxItems: 3
+
+  clock-names:
+    minItems: 1
+    maxItems: 3
+    anyOf:
+      - items:
+          enum: [ core, pad ]
+      - items:
+          enum: [ pclk, core, pad ]
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    ethernet-pcs@1f05d000 {
+      compatible = "snps,dw-xpcs";
+      reg = <0x1f05d000 0x1000>;
+      reg-names = "indirect";
+
+      reg-io-width = <4>;
+
+      interrupts = <79 IRQ_TYPE_LEVEL_HIGH>;
+
+      clocks = <&ccu_pclk>, <&ccu_core>, <&ccu_pad>;
+      clock-names = "pclk", "core", "pad";
+    };
+  - |
+    mdio-bus {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      ethernet-pcs@0 {
+        compatible = "snps,dw-xpcs";
+        reg = <0>;
+
+        clocks = <&ccu_core>, <&ccu_pad>;
+        clock-names = "core", "pad";
+      };
+    };
+...