[2/5] dt-bindings: net: add schema for NXP S32CC dwmac glue driver

  Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
Chassis.

Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
Signed-off-by: Chester Lin <clin@suse.com>
---
 .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
 1 file changed, 145 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
  

On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> Chassis.
> 
> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> Signed-off-by: Chester Lin <clin@suse.com>
> ---
>  .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
>  1 file changed, 145 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> new file mode 100644
> index 000000000000..f6b8486f9d42
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> @@ -0,0 +1,145 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright 2021-2022 NXP
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: NXP S32CC DWMAC Ethernet controller
> +
> +maintainers:
> +  - Jan Petrous <jan.petrous@nxp.com>
> +  - Chester Lin <clin@suse.com>
> +
> +select:

Don't need this.

> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - nxp,s32cc-dwmac
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: "snps,dwmac.yaml#"
> +
> +properties:
> +  compatible:
> +    contains:

Drop 'contains'.

> +      enum:
> +        - nxp,s32cc-dwmac
> +
> +  reg:
> +    items:
> +      - description: Main GMAC registers
> +      - description: S32 MAC control registers
> +
> +  dma-coherent:
> +    description:
> +      Declares GMAC device as DMA coherent

Don't need a generic description. Just 'true' is enough.

> +
> +  clocks:
> +    items:
> +      - description: Main GMAC clock
> +      - description: Peripheral registers clock
> +      - description: Transmit SGMII clock
> +      - description: Transmit RGMII clock
> +      - description: Transmit RMII clock
> +      - description: Transmit MII clock
> +      - description: Receive SGMII clock
> +      - description: Receive RGMII clock
> +      - description: Receive RMII clock
> +      - description: Receive MII clock
> +      - description:
> +          PTP reference clock. This clock is used for programming the
> +          Timestamp Addend Register. If not passed then the system
> +          clock will be used.

If optional, then you need 'minItems'.

> +
> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: pclk
> +      - const: tx_sgmii
> +      - const: tx_rgmii
> +      - const: tx_rmii
> +      - const: tx_mii
> +      - const: rx_sgmii
> +      - const: rx_rgmii
> +      - const: rx_rmii
> +      - const: rx_mii
> +      - const: ptp_ref
> +
> +  tx-fifo-depth:
> +    const: 20480
> +
> +  rx-fifo-depth:
> +    const: 20480
> +
> +required:
> +  - compatible
> +  - reg
> +  - tx-fifo-depth
> +  - rx-fifo-depth
> +  - clocks
> +  - clock-names
> +
> +additionalProperties: true

'true' is only allowed for common, incomplete schemas. Should be:

unevaluatedProperties: false

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    #define S32GEN1_SCMI_CLK_GMAC0_AXI
> +    #define S32GEN1_SCMI_CLK_GMAC0_TX_SGMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_TX_RGMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_TX_RMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_TX_MII
> +    #define S32GEN1_SCMI_CLK_GMAC0_RX_SGMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_RX_RGMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_RX_RMII
> +    #define S32GEN1_SCMI_CLK_GMAC0_RX_MII
> +    #define S32GEN1_SCMI_CLK_GMAC0_TS
> +
> +    soc {
> +      #address-cells = <1>;
> +      #size-cells = <1>;
> +
> +      gmac0: ethernet@4033c000 {
> +        compatible = "nxp,s32cc-dwmac";
> +        reg = <0x4033c000 0x2000>, /* gmac IP */
> +              <0x4007C004 0x4>;    /* S32 CTRL_STS reg */
> +        interrupt-parent = <&gic>;
> +        interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "macirq";
> +        phy-mode = "rgmii-id";
> +        tx-fifo-depth = <20480>;
> +        rx-fifo-depth = <20480>;
> +        dma-coherent;
> +        clocks = <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_AXI>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_TX_SGMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RGMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_TX_RMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_TX_MII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_RX_SGMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RGMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_RX_RMII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_RX_MII>,
> +                 <&clks S32GEN1_SCMI_CLK_GMAC0_TS>;
> +        clock-names = "stmmaceth", "pclk",
> +                      "tx_sgmii", "tx_rgmii", "tx_rmii", "tx_mii",
> +                      "rx_sgmii", "rx_rgmii", "rx_rmii", "rx_mii",
> +                      "ptp_ref";
> +
> +        gmac0_mdio: mdio {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          compatible = "snps,dwmac-mdio";
> +
> +          ethernet-phy@4 {
> +            reg = <0x04>;
> +          };
> +        };
> +      };
> +    };
> -- 
> 2.37.3
> 
>
  

Hi Rob,

On 02.11.22 16:55, Rob Herring wrote:
> On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
>> Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
>> Chassis.
>>
>> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
>> Signed-off-by: Chester Lin <clin@suse.com>
>> ---
>>   .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
>>   1 file changed, 145 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>> new file mode 100644
>> index 000000000000..f6b8486f9d42
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
>> @@ -0,0 +1,145 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright 2021-2022 NXP
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
>> +
>> +title: NXP S32CC DWMAC Ethernet controller
>> +
>> +maintainers:
>> +  - Jan Petrous <jan.petrous@nxp.com>
>> +  - Chester Lin <clin@suse.com>
[...]
>> +properties:
>> +  compatible:
>> +    contains:
> 
> Drop 'contains'.
> 
>> +      enum:
>> +        - nxp,s32cc-dwmac

In the past you were adamant that we use concrete SoC-specific strings. 
Here that would mean s32g2 or s32g274 instead of s32cc (which aims to 
share with S32G3 IIUC).

[...]
>> +  clocks:
>> +    items:
>> +      - description: Main GMAC clock
>> +      - description: Peripheral registers clock
>> +      - description: Transmit SGMII clock
>> +      - description: Transmit RGMII clock
>> +      - description: Transmit RMII clock
>> +      - description: Transmit MII clock
>> +      - description: Receive SGMII clock
>> +      - description: Receive RGMII clock
>> +      - description: Receive RMII clock
>> +      - description: Receive MII clock
>> +      - description:
>> +          PTP reference clock. This clock is used for programming the
>> +          Timestamp Addend Register. If not passed then the system
>> +          clock will be used.
> 
> If optional, then you need 'minItems'.
[snip]

Do we have any precedence of bindings with *MII clocks like these?

AFAIU the reason there are so many here is that there are in fact 
physically just five, but different parent clock configurations that 
SCMI does not currently expose to Linux. Thus I was raising that we may 
want to extend the SCMI protocol with some SET_PARENT operation that 
could allow us to use less input clocks here, but obviously such a 
standardization process will take time...

What are your thoughts on how to best handle this here?

Not clear to me has been whether the PHY mode can be switched at runtime 
(like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by 
board design. If the latter, the two out of six SCMI IDs could get 
selected in TF-A, to have only physical clocks here in the binding.

Regards,
Andreas
  

On Wed, Nov 02, 2022 at 06:13:35PM +0100, Andreas Färber wrote:
> Hi Rob,
> 
> On 02.11.22 16:55, Rob Herring wrote:
> > On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
> > > Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> > > Chassis.
> > > 
> > > Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> > > Signed-off-by: Chester Lin <clin@suse.com>
> > > ---
> > >   .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
> > >   1 file changed, 145 insertions(+)
> > >   create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > new file mode 100644
> > > index 000000000000..f6b8486f9d42
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > @@ -0,0 +1,145 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +# Copyright 2021-2022 NXP
> > > +%YAML 1.2
> > > +---
> > > +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
> > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > +
> > > +title: NXP S32CC DWMAC Ethernet controller
> > > +
> > > +maintainers:
> > > +  - Jan Petrous <jan.petrous@nxp.com>
> > > +  - Chester Lin <clin@suse.com>
> [...]
> > > +properties:
> > > +  compatible:
> > > +    contains:
> > 
> > Drop 'contains'.
> > 
> > > +      enum:
> > > +        - nxp,s32cc-dwmac
> 
> In the past you were adamant that we use concrete SoC-specific strings. Here
> that would mean s32g2 or s32g274 instead of s32cc (which aims to share with
> S32G3 IIUC).

Yes they should be SoC specific. Really, 1 per maskset or die is fine if 
that level of detail is known. No need for different compatibles for 
different part numbers created by fused off features or package pinout 
differences.


> [...]
> > > +  clocks:
> > > +    items:
> > > +      - description: Main GMAC clock
> > > +      - description: Peripheral registers clock
> > > +      - description: Transmit SGMII clock
> > > +      - description: Transmit RGMII clock
> > > +      - description: Transmit RMII clock
> > > +      - description: Transmit MII clock
> > > +      - description: Receive SGMII clock
> > > +      - description: Receive RGMII clock
> > > +      - description: Receive RMII clock
> > > +      - description: Receive MII clock
> > > +      - description:
> > > +          PTP reference clock. This clock is used for programming the
> > > +          Timestamp Addend Register. If not passed then the system
> > > +          clock will be used.
> > 
> > If optional, then you need 'minItems'.
> [snip]
> 
> Do we have any precedence of bindings with *MII clocks like these?

Don't know...

> AFAIU the reason there are so many here is that there are in fact physically
> just five, but different parent clock configurations that SCMI does not
> currently expose to Linux. Thus I was raising that we may want to extend the
> SCMI protocol with some SET_PARENT operation that could allow us to use less
> input clocks here, but obviously such a standardization process will take
> time...
> 
> What are your thoughts on how to best handle this here?

Perhaps use assigned-clocks if it is static for a board.

> Not clear to me has been whether the PHY mode can be switched at runtime
> (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by board
> design. If the latter, the two out of six SCMI IDs could get selected in
> TF-A, to have only physical clocks here in the binding.
> 
> Regards,
> Andreas
  

> > > +      - description: Main GMAC clock
> > > +      - description: Peripheral registers clock
> > > +      - description: Transmit SGMII clock
> > > +      - description: Transmit RGMII clock
> > > +      - description: Transmit RMII clock
> > > +      - description: Transmit MII clock
> > > +      - description: Receive SGMII clock
> > > +      - description: Receive RGMII clock
> > > +      - description: Receive RMII clock
> > > +      - description: Receive MII clock
> > > +      - description:
> > > +          PTP reference clock. This clock is used for programming the
> > > +          Timestamp Addend Register. If not passed then the system
> > > +          clock will be used.

> Not clear to me has been whether the PHY mode can be switched at runtime
> (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by board
> design.

Does the hardware support 1000BaseX? Often the hardware implementing
SGMII can also do 1000BaseX, since SGMII is an extended/hacked up
1000BaseX.

If you have an SFP connected to the SERDES, a fibre module will want
1000BaseX and a copper module will want SGMII. phylink will tell you
what phy-mode you need to use depending on what module is in the
socket. This however might be a mute point, since both of these are
probably using the SGMII clocks.

Of the other MII modes listed, it is very unlikely a runtime swap will
occur.

	Andrew
  

Hi Rob and Andreas,

On Wed, Nov 02, 2022 at 04:44:56PM -0500, Rob Herring wrote:
> On Wed, Nov 02, 2022 at 06:13:35PM +0100, Andreas Färber wrote:
> > Hi Rob,
> > 
> > On 02.11.22 16:55, Rob Herring wrote:
> > > On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
> > > > Add the DT schema for the DWMAC Ethernet controller on NXP S32 Common
> > > > Chassis.
> > > > 
> > > > Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> > > > Signed-off-by: Chester Lin <clin@suse.com>
> > > > ---
> > > >   .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
> > > >   1 file changed, 145 insertions(+)
> > > >   create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > > new file mode 100644
> > > > index 000000000000..f6b8486f9d42
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> > > > @@ -0,0 +1,145 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +# Copyright 2021-2022 NXP
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: "http://devicetree.org/schemas/net/nxp,s32cc-dwmac.yaml#"
> > > > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > > > +
> > > > +title: NXP S32CC DWMAC Ethernet controller
> > > > +
> > > > +maintainers:
> > > > +  - Jan Petrous <jan.petrous@nxp.com>
> > > > +  - Chester Lin <clin@suse.com>
> > [...]
> > > > +properties:
> > > > +  compatible:
> > > > +    contains:
> > > 
> > > Drop 'contains'.
> > > 
> > > > +      enum:
> > > > +        - nxp,s32cc-dwmac
> > 
> > In the past you were adamant that we use concrete SoC-specific strings. Here
> > that would mean s32g2 or s32g274 instead of s32cc (which aims to share with
> > S32G3 IIUC).
> 
> Yes they should be SoC specific. Really, 1 per maskset or die is fine if 
> that level of detail is known. No need for different compatibles for 
> different part numbers created by fused off features or package pinout 
> differences.
> 

If I understand correctly from NXP, the GMAC0 belongs to a common hardware
sub-architecture called "S32 Common Chassis", which is a common IP set applied
in many S32 SoC series, such as S32G2/G3 and S32R45. Therefore this binding is
not specifically for S32G2 but it supports all S32 SoC series who adopt S32CC
sub-arch if they could all be upstreamed in the future.

Logically S32G2 and S32R45 have the same subset *S32CC* but it doesn't mean that
S32G2 and S32R45 are derived from each other.

Regards,
Chester

> > [...]
> > > > +  clocks:
> > > > +    items:
> > > > +      - description: Main GMAC clock
> > > > +      - description: Peripheral registers clock
> > > > +      - description: Transmit SGMII clock
> > > > +      - description: Transmit RGMII clock
> > > > +      - description: Transmit RMII clock
> > > > +      - description: Transmit MII clock
> > > > +      - description: Receive SGMII clock
> > > > +      - description: Receive RGMII clock
> > > > +      - description: Receive RMII clock
> > > > +      - description: Receive MII clock
> > > > +      - description:
> > > > +          PTP reference clock. This clock is used for programming the
> > > > +          Timestamp Addend Register. If not passed then the system
> > > > +          clock will be used.
> > > 
> > > If optional, then you need 'minItems'.
> > [snip]
> > 
> > Do we have any precedence of bindings with *MII clocks like these?
> 
> Don't know...
> 
> > AFAIU the reason there are so many here is that there are in fact physically
> > just five, but different parent clock configurations that SCMI does not
> > currently expose to Linux. Thus I was raising that we may want to extend the
> > SCMI protocol with some SET_PARENT operation that could allow us to use less
> > input clocks here, but obviously such a standardization process will take
> > time...
> > 
> > What are your thoughts on how to best handle this here?
> 
> Perhaps use assigned-clocks if it is static for a board.
> 
> > Not clear to me has been whether the PHY mode can be switched at runtime
> > (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by board
> > design. If the latter, the two out of six SCMI IDs could get selected in
> > TF-A, to have only physical clocks here in the binding.
> > 
> > Regards,
> > Andreas
  

Hi Andrew and Andreas,

On Thu, Nov 03, 2022 at 11:05:30PM +0100, Andrew Lunn wrote:
> > > > +      - description: Main GMAC clock
> > > > +      - description: Peripheral registers clock
> > > > +      - description: Transmit SGMII clock
> > > > +      - description: Transmit RGMII clock
> > > > +      - description: Transmit RMII clock
> > > > +      - description: Transmit MII clock
> > > > +      - description: Receive SGMII clock
> > > > +      - description: Receive RGMII clock
> > > > +      - description: Receive RMII clock
> > > > +      - description: Receive MII clock
> > > > +      - description:
> > > > +          PTP reference clock. This clock is used for programming the
> > > > +          Timestamp Addend Register. If not passed then the system
> > > > +          clock will be used.
> 
> > Not clear to me has been whether the PHY mode can be switched at runtime
> > (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by board
> > design.
> 
> Does the hardware support 1000BaseX? Often the hardware implementing
> SGMII can also do 1000BaseX, since SGMII is an extended/hacked up
> 1000BaseX.
> 
> If you have an SFP connected to the SERDES, a fibre module will want
> 1000BaseX and a copper module will want SGMII. phylink will tell you
> what phy-mode you need to use depending on what module is in the
> socket. This however might be a mute point, since both of these are
> probably using the SGMII clocks.
> 
> Of the other MII modes listed, it is very unlikely a runtime swap will
> occur.
> 
> 	Andrew

Here I just focus on GMAC since there are other LAN interfaces that S32 family
uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1] provided
on NXP website, theoretically GMAC can run SGMII in 1000Mbps and 2500Mbps so I
assume that supporting 1000BASE-X could be achievable. I'm not sure if any S32
board variant might have SFP ports but RJ-45 [1000BASE-T] should be the major
type used on S32G-EVB and S32G-RDB2.

@NXP, please feel free to correct me if anything wrong.

Thanks,
Chester

[1] https://www.nxp.com/webapp/Download?colCode=GMACSUBSYSRM -> Membership
subscription is required although it's free IIRC.
  

> Here I just focus on GMAC since there are other LAN interfaces that S32 family
> uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1] provided
> on NXP website, theoretically GMAC can run SGMII in 1000Mbps and 2500Mbps so I
> assume that supporting 1000BASE-X could be achievable. I'm not sure if any S32
> board variant might have SFP ports but RJ-45 [1000BASE-T] should be the major
> type used on S32G-EVB and S32G-RDB2.

SGMII at 2500Mbps does not exist. Lots of people get this wrong. It
will be 2500Base-X.

Does the clock need to change in order to support 2500Base-X? If i
understand you correctly, Linux does not control the clocks, and so
cannot change the clocks? So that probably means you cannot actually
support 2500Base-X? Once you have Linux actually controlling the
hardware, you can then make use of an SFP or a copper PHY which
supports 2.5G. The PHY will swap its host side between SGMII and
2500Base-X depending on what the line side negotiates, 1000Base-T or
2500Base-T. The MAC driver then needs to change its configuration to
suite.

	Andrew
  

Hi Andrew,

On Fri, Nov 04, 2022 at 02:30:11PM +0100, Andrew Lunn wrote:
> > Here I just focus on GMAC since there are other LAN interfaces that S32 family
> > uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1] provided
> > on NXP website, theoretically GMAC can run SGMII in 1000Mbps and 2500Mbps so I
> > assume that supporting 1000BASE-X could be achievable. I'm not sure if any S32
> > board variant might have SFP ports but RJ-45 [1000BASE-T] should be the major
> > type used on S32G-EVB and S32G-RDB2.
> 
> SGMII at 2500Mbps does not exist. Lots of people get this wrong. It
> will be 2500Base-X.
> 

Thanks for your correction.

> Does the clock need to change in order to support 2500Base-X? If i

Since I'm not a hardware designer from NXP and I can't find any board that
S32G2 CPUs could integrate SFP, so I am not able to tell how the clock could
be configured while supporting 2500Base-X.

> understand you correctly, Linux does not control the clocks, and so
> cannot change the clocks? So that probably means you cannot actually

To be more precise, the SCMI clock protocol in ATF [ARM Trusted Firmware]
doesn't design any interface to get/set parents of a clock so that re-parenting
clocks via the SCMI clk driver [clk-scmi.c] in Linux Kernel is impossible
for this case. That means, if any board design allows run-time swap on different
phys, the dedicated clks which represent each phy-mode are required in order
to trigger clock re-parenting in ATF since different phy connections would
need different clock sources.

> support 2500Base-X? Once you have Linux actually controlling the
> hardware, you can then make use of an SFP or a copper PHY which
> supports 2.5G. The PHY will swap its host side between SGMII and
> 2500Base-X depending on what the line side negotiates, 1000Base-T or
> 2500Base-T. The MAC driver then needs to change its configuration to
> suite.
> 
> 	Andrew
  

Hi Chester,
 
> Hi Andrew and Andreas,
> 
> On Thu, Nov 03, 2022 at 11:05:30PM +0100, Andrew Lunn wrote:
> > > > > +      - description: Main GMAC clock
> > > > > +      - description: Peripheral registers clock
> > > > > +      - description: Transmit SGMII clock
> > > > > +      - description: Transmit RGMII clock
> > > > > +      - description: Transmit RMII clock
> > > > > +      - description: Transmit MII clock
> > > > > +      - description: Receive SGMII clock
> > > > > +      - description: Receive RGMII clock
> > > > > +      - description: Receive RMII clock
> > > > > +      - description: Receive MII clock
> > > > > +      - description:
> > > > > +          PTP reference clock. This clock is used for programming the
> > > > > +          Timestamp Addend Register. If not passed then the system
> > > > > +          clock will be used.
> >
> > > Not clear to me has been whether the PHY mode can be switched at
> runtime
> > > (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by
> board
> > > design.
> >
> > Does the hardware support 1000BaseX? Often the hardware implementing
> > SGMII can also do 1000BaseX, since SGMII is an extended/hacked up
> > 1000BaseX.
> >
> > If you have an SFP connected to the SERDES, a fibre module will want
> > 1000BaseX and a copper module will want SGMII. phylink will tell you
> > what phy-mode you need to use depending on what module is in the
> > socket. This however might be a mute point, since both of these are
> > probably using the SGMII clocks.
> >
> > Of the other MII modes listed, it is very unlikely a runtime swap will
> > occur.
> >
> >       Andrew
> 
> Here I just focus on GMAC since there are other LAN interfaces that S32
> family
> uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1]
> provided
> on NXP website, theoretically GMAC can run SGMII in 1000Mbps and
> 2500Mbps so I
> assume that supporting 1000BASE-X could be achievable. I'm not sure if any
> S32
> board variant might have SFP ports but RJ-45 [1000BASE-T] should be the
> major
> type used on S32G-EVB and S32G-RDB2.
> 
> @NXP, please feel free to correct me if anything wrong.
> 

NXP eval boards (EVB or RDB) have also 2.5G PHYs, so together with SerDes
driver we support 100M/1G/2.5G on such copper PHYs. 

/Jan

--
NXP Czechia, AP Ethernet
  

Hi Andreas,

> Hi Rob,
> 
> On 02.11.22 16:55, Rob Herring wrote:
> > On Mon, Oct 31, 2022 at 06:10:49PM +0800, Chester Lin wrote:
> >> Add the DT schema for the DWMAC Ethernet controller on NXP S32
> Common
> >> Chassis.
> >>
> >> Signed-off-by: Jan Petrous <jan.petrous@nxp.com>
> >> Signed-off-by: Chester Lin <clin@suse.com>
> >> ---
> >>   .../bindings/net/nxp,s32cc-dwmac.yaml         | 145 ++++++++++++++++++
> >>   1 file changed, 145 insertions(+)
> >>   create mode 100644 Documentation/devicetree/bindings/net/nxp,s32cc-
> dwmac.yaml
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/nxp,s32cc-
> dwmac.yaml b/Documentation/devicetree/bindings/net/nxp,s32cc-
> dwmac.yaml
> >> new file mode 100644
> >> index 000000000000..f6b8486f9d42
> >> --- /dev/null
> >> +++ b/Documentation/devicetree/bindings/net/nxp,s32cc-dwmac.yaml
> >> @@ -0,0 +1,145 @@
> >> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> >> +# Copyright 2021-2022 NXP
> >> +%YAML 1.2
> >> +---
> >> +$id:
[...]
> >> +title: NXP S32CC DWMAC Ethernet controller
> >> +
> >> +maintainers:
> >> +  - Jan Petrous <jan.petrous@nxp.com>
> >> +  - Chester Lin <clin@suse.com>
> [...]
> >> +properties:
> >> +  compatible:
> >> +    contains:
> >
> > Drop 'contains'.
> >
> >> +      enum:
> >> +        - nxp,s32cc-dwmac
> 
> In the past you were adamant that we use concrete SoC-specific strings.
> Here that would mean s32g2 or s32g274 instead of s32cc (which aims to
> share with S32G3 IIUC).
> 
> [...]
> >> +  clocks:
> >> +    items:
> >> +      - description: Main GMAC clock
> >> +      - description: Peripheral registers clock
> >> +      - description: Transmit SGMII clock
> >> +      - description: Transmit RGMII clock
> >> +      - description: Transmit RMII clock
> >> +      - description: Transmit MII clock
> >> +      - description: Receive SGMII clock
> >> +      - description: Receive RGMII clock
> >> +      - description: Receive RMII clock
> >> +      - description: Receive MII clock
> >> +      - description:
> >> +          PTP reference clock. This clock is used for programming the
> >> +          Timestamp Addend Register. If not passed then the system
> >> +          clock will be used.
> >
> > If optional, then you need 'minItems'.
> [snip]
> 
> Do we have any precedence of bindings with *MII clocks like these?
> 
> AFAIU the reason there are so many here is that there are in fact
> physically just five, but different parent clock configurations that
> SCMI does not currently expose to Linux. Thus I was raising that we may

Correct. The different clock names represent different configs of the same
clocks.

> want to extend the SCMI protocol with some SET_PARENT operation that
> could allow us to use less input clocks here, but obviously such a
> standardization process will take time...
> 
> What are your thoughts on how to best handle this here?
> 
> Not clear to me has been whether the PHY mode can be switched at runtime
> (like DPAA2 on Layerscape allows for SFPs) or whether this is fixed by
> board design. If the latter, the two out of six SCMI IDs could get
> selected in TF-A, to have only physical clocks here in the binding.

The eval board allows to use different PHYs/switches connected by RGMII
or SGMII to the GMAC. Some combinations require change of board's
hw switches, but not all of them. Anyway, until we get a board with SFP,
the connection type can be treated as fixed (declared in DT). 

/Jan

--
NXP Czechia, AP Ethernet
  

> > Here I just focus on GMAC since there are other LAN interfaces that S32
> > family
> > uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1]
> > provided
> > on NXP website, theoretically GMAC can run SGMII in 1000Mbps and
> > 2500Mbps so I
> > assume that supporting 1000BASE-X could be achievable. I'm not sure if any
> > S32
> > board variant might have SFP ports but RJ-45 [1000BASE-T] should be the
> > major
> > type used on S32G-EVB and S32G-RDB2.
> > 
> > @NXP, please feel free to correct me if anything wrong.
> > 
> 
> NXP eval boards (EVB or RDB) have also 2.5G PHYs, so together with SerDes
> driver we support 100M/1G/2.5G on such copper PHYs. 

Hi Jan

Does the SERDES clock need to change when going between 1000BaseX and
2500BaseX?

If so, it sounds like Linux not having control of that clock is going
to limit what can be supported.

   Andrew
  

Hi Andrew,

> > > Here I just focus on GMAC since there are other LAN interfaces that S32
> > > family
> > > uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1]
> > > provided
> > > on NXP website, theoretically GMAC can run SGMII in 1000Mbps and
> > > 2500Mbps so I
> > > assume that supporting 1000BASE-X could be achievable. I'm not sure if
> any
> > > S32
> > > board variant might have SFP ports but RJ-45 [1000BASE-T] should be the
> > > major
> > > type used on S32G-EVB and S32G-RDB2.
> > >
> > > @NXP, please feel free to correct me if anything wrong.
> > >
> >
> > NXP eval boards (EVB or RDB) have also 2.5G PHYs, so together with SerDes
> > driver we support 100M/1G/2.5G on such copper PHYs.
> 
> Hi Jan
> 
> Does the SERDES clock need to change when going between 1000BaseX and
> 2500BaseX?
> 
> If so, it sounds like Linux not having control of that clock is going
> to limit what can be supported.

No, the SerDes clock remains the same, the change is done internally, without
any necessity of clock change intervention by GMAC driver.

/Jan
  

On Thu, Nov 10, 2022 at 08:51:43AM +0000, Jan Petrous wrote:
> Hi Andrew,
> 
> > > > Here I just focus on GMAC since there are other LAN interfaces that S32
> > > > family
> > > > uses [e.g. PFE]. According to the public GMACSUBSYS ref manual rev2[1]
> > > > provided
> > > > on NXP website, theoretically GMAC can run SGMII in 1000Mbps and
> > > > 2500Mbps so I
> > > > assume that supporting 1000BASE-X could be achievable. I'm not sure if
> > any
> > > > S32
> > > > board variant might have SFP ports but RJ-45 [1000BASE-T] should be the
> > > > major
> > > > type used on S32G-EVB and S32G-RDB2.
> > > >
> > > > @NXP, please feel free to correct me if anything wrong.
> > > >
> > >
> > > NXP eval boards (EVB or RDB) have also 2.5G PHYs, so together with SerDes
> > > driver we support 100M/1G/2.5G on such copper PHYs.
> > 
> > Hi Jan
> > 
> > Does the SERDES clock need to change when going between 1000BaseX and
> > 2500BaseX?
> > 
> > If so, it sounds like Linux not having control of that clock is going
> > to limit what can be supported.
> 
> No, the SerDes clock remains the same, the change is done internally, without
> any necessity of clock change intervention by GMAC driver.

Hi Jan

Thanks for the information. So this binding should work.

The only suggestion i have is that the binding does not call is SGMII
clock, because it is used for more than SGMII, also 1000base-X, and
2500Base-X. So PCS clock might be better.

	Andrew