Message-ID: <fe705ffc-9a4c-462c-a1bf-e14c55cdb2cd@altera.com>
Date: Thu, 26 Jun 2025 18:40:32 -0700
From: Matthew Gerlach <matthew.gerlach@...era.com>
To: Rob Herring <robh@...nel.org>, Dinh Nguyen <dinguyen@...nel.org>
Cc: andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, krzk+dt@...nel.org, conor+dt@...nel.org,
 maxime.chevallier@...tlin.com, mcoquelin.stm32@...il.com,
 alexandre.torgue@...s.st.com, richardcochran@...il.com,
 netdev@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
 linux-arm-kernel@...ts.infradead.org, Mun Yew Tham <mun.yew.tham@...era.com>
Subject: Re: [PATCH v6] dt-bindings: net: Convert socfpga-dwmac bindings to
 yaml



On 6/26/25 4:48 PM, Rob Herring wrote:
> On Fri, Jun 13, 2025 at 03:58:44PM -0700, Matthew Gerlach wrote:
> > Convert the bindings for socfpga-dwmac to yaml. Since the original
> > text contained descriptions for two separate nodes, two separate
> > yaml files were created.
>
> Sigh I just reviewed a conversion from Dinh:
>
> https://lore.kernel.org/all/20250624191549.474686-1-dinguyen@kernel.org/
>
> I prefer this one as it has altr,gmii-to-sgmii-2.0.yaml, but I see some
> issues compared to Dinh's.
I am sorry for my part in the duplicate review. I just rechecked the 
output of get_maintainers.pl, and Dinh was not listed, and I should have 
known better.

I am happy to do the work to resolve the differences and resubmit with 
Dinh as the maintainer.

>
> > 
> > Signed-off-by: Mun Yew Tham <mun.yew.tham@...era.com>
> > Signed-off-by: Matthew Gerlach <matthew.gerlach@...era.com>
> > Reviewed-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> > ---
> > v6:
> >  - Fix reference to altr,gmii-to-sgmii-2.0.yaml in MAINTAINERS.
> >  - Add Reviewed-by:
> > 
> > v5:
> >  - Fix dt_binding_check error: comptabile.
> >  - Rename altr,gmii-to-sgmii.yaml to altr,gmii-to-sgmii-2.0.yaml
> > 
> > v4:
> >  - Change filename from socfpga,dwmac.yaml to altr,socfpga-stmmac.yaml.
> >  - Updated compatible in select properties and main properties.
> >  - Fixed clocks so stmmaceth clock is required.
> >  - Added binding for altr,gmii-to-sgmii.
> >  - Update MAINTAINERS.
> > 
> > v3:
> >  - Add missing supported phy-modes.
> > 
> > v2:
> >  - Add compatible to required.
> >  - Add descriptions for clocks.
> >  - Add clock-names.
> >  - Clean up items: in altr,sysmgr-syscon.
> >  - Change "additionalProperties: true" to "unevaluatedProperties: false".
> >  - Add properties needed for "unevaluatedProperties: false".
> >  - Fix indentation in examples.
> >  - Drop gmac0: label in examples.
> >  - Exclude support for Arria10 that is not validating.
> > ---
> >  .../bindings/net/altr,gmii-to-sgmii-2.0.yaml  |  49 ++++++
> >  .../bindings/net/altr,socfpga-stmmac.yaml     | 162 ++++++++++++++++++
> >  .../devicetree/bindings/net/socfpga-dwmac.txt |  57 ------
> >  MAINTAINERS                                   |   7 +-
> >  4 files changed, 217 insertions(+), 58 deletions(-)
> >  create mode 100644 Documentation/devicetree/bindings/net/altr,gmii-to-sgmii-2.0.yaml
> >  create mode 100644 Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml
> >  delete mode 100644 Documentation/devicetree/bindings/net/socfpga-dwmac.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/net/altr,gmii-to-sgmii-2.0.yaml b/Documentation/devicetree/bindings/net/altr,gmii-to-sgmii-2.0.yaml
> > new file mode 100644
> > index 000000000000..aafb6447b6c2
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/altr,gmii-to-sgmii-2.0.yaml
> > @@ -0,0 +1,49 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +# Copyright (C) 2025 Altera Corporation
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/altr,gmii-to-sgmii-2.0.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Altera GMII to SGMII Converter
> > +
> > +maintainers:
> > +  - Matthew Gerlach <matthew.gerlach@...era.com>
> > +
> > +description:
> > +  This binding describes the Altera GMII to SGMII converter.
> > +
> > +properties:
> > +  compatible:
> > +    const: altr,gmii-to-sgmii-2.0
> > +
> > +  reg:
> > +    items:
> > +      - description: Registers for the emac splitter IP
> > +      - description: Registers for the GMII to SGMII converter.
> > +      - description: Registers for TSE control.
> > +
> > +  reg-names:
> > +    items:
> > +      - const: hps_emac_interface_splitter_avalon_slave
> > +      - const: gmii_to_sgmii_adapter_avalon_slave
> > +      - const: eth_tse_control_port
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - reg-names
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    phy@...00240 {
> > +        compatible = "altr,gmii-to-sgmii-2.0";
> > +        reg = <0xff000240 0x00000008>,
> > +              <0xff000200 0x00000040>,
> > +              <0xff000250 0x00000008>;
> > +        reg-names = "hps_emac_interface_splitter_avalon_slave",
> > +                    "gmii_to_sgmii_adapter_avalon_slave",
> > +                    "eth_tse_control_port";
> > +    };
> > diff --git a/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml b/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml
> > new file mode 100644
> > index 000000000000..ccbbdb870755
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml
> > @@ -0,0 +1,162 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/altr,socfpga-stmmac.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Altera SOCFPGA SoC DWMAC controller
> > +
> > +maintainers:
> > +  - Matthew Gerlach <matthew.gerlach@...era.com>
> > +
> > +description:
> > +  This binding describes the Altera SOCFPGA SoC implementation of the
> > +  Synopsys DWMAC for the Cyclone5, Arria5, Stratix10, and Agilex7 families
> > +  of chips.
> > +  # TODO: Determine how to handle the Arria10 reset-name, stmmaceth-ocp, that
> > +  # does not validate against net/snps,dwmac.yaml.
> > +
> > +select:
> > +  properties:
> > +    compatible:
> > +      contains:
> > +        enum:
> > +          - altr,socfpga-stmmac
> > +          - altr,socfpga-stmmac-a10-s10
> > +
> > +  required:
> > +    - compatible
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - items:
> > +          - const: altr,socfpga-stmmac
> > +          - const: snps,dwmac-3.70a
> > +          - const: snps,dwmac
> > +      - items:
> > +          - const: altr,socfpga-stmmac-a10-s10
> > +          - const: snps,dwmac-3.74a
> > +          - const: snps,dwmac
>
> You are missing the snps,dwmac-3.72a version.
Yes, leaving out the snps,dwmac-3.72a version was a conscience decision. 
Please see TODO above. Since I was stuck figuring out how to address the 
reset-names issue with the Arria10, I thought it was better to leave it 
out to keep the Arria10 schema errors the same rather than changing the 
error. Should net/snps,dwmac.yaml be changed to handle the stmmaceth-ocp 
reset-name, or is there something that can be done in this file to 
handle it?
>
>
> > +
> > +  clocks:
> > +    minItems: 1
> > +    items:
> > +      - description: GMAC main 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 and this is fine on some platforms.
> > +
> > +  clock-names:
> > +    minItems: 1
> > +    items:
> > +      - const: stmmaceth
> > +      - const: ptp_ref
> > +
> > +  iommus:
> > +    maxItems: 1
>
> Dinh's says there can be 2?
I don't see any dts or dtsi with 2 iommus, but more importantly does the 
hardware support 2 iommus? Dinh can you comment?

>
> > +
> > +  phy-mode:
> > +    enum:
> > +      - gmii
> > +      - mii
> > +      - rgmii
> > +      - rgmii-id
> > +      - rgmii-rxid
> > +      - rgmii-txid
> > +      - sgmii
> > +      - 1000base-x
>
> Dinh's says only rgmii, gmii, and mii supported?
The example in the original text lists sgmii, and there was some good 
feedback from Maxime Chevallier and Andrew Lunn on this subject in 
https://lore.kernel.org/lkml/20250528144650.48343-1-matthew.gerlach@altera.com/T/#u 


Thanks for the feedback,
Matthew Gerlach

>
> > +
> > +  rxc-skew-ps:
> > +    description: Skew control of RXC pad
> > +
> > +  rxd0-skew-ps:
> > +    description: Skew control of RX data 0 pad
> > +
> > +  rxd1-skew-ps:
> > +    description: Skew control of RX data 1 pad
> > +
> > +  rxd2-skew-ps:
> > +    description: Skew control of RX data 2 pad
> > +
> > +  rxd3-skew-ps:
> > +    description: Skew control of RX data 3 pad
> > +
> > +  rxdv-skew-ps:
> > +    description: Skew control of RX CTL pad
> > +
> > +  txc-skew-ps:
> > +    description: Skew control of TXC pad
> > +
> > +  txen-skew-ps:
> > +    description: Skew control of TXC pad
> > +
> > +  altr,emac-splitter:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Should be the phandle to the emac splitter soft IP node if DWMAC
> > +      controller is connected an emac splitter.
> > +
> > +  altr,f2h_ptp_ref_clk:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Phandle to Precision Time Protocol reference clock. This clock is
> > +      common to gmac instances and defaults to osc1.
> > +
> > +  altr,gmii-to-sgmii-converter:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      Should be the phandle to the gmii to sgmii converter soft IP.
> > +
> > +  altr,sysmgr-syscon:
> > +    $ref: /schemas/types.yaml#/definitions/phandle-array
> > +    description:
> > +      Should be the phandle to the system manager node that encompass
> > +      the glue register, the register offset, and the register shift.
> > +      On Cyclone5/Arria5, the register shift represents the PHY mode
> > +      bits, while on the Arria10/Stratix10/Agilex platforms, the
> > +      register shift represents bit for each emac to enable/disable
> > +      signals from the FPGA fabric to the EMAC modules.
> > +    items:
> > +      - items:
> > +          - description: phandle to the system manager node
> > +          - description: offset of the control register
> > +          - description: shift within the control register
> > +
> > +patternProperties:
> > +  "^mdio[0-9]$":
> > +    type: object
> > +
> > +required:
> > +  - compatible
> > +  - clocks
> > +  - clock-names
> > +  - altr,sysmgr-syscon
> > +
> > +allOf:
> > +  - $ref: snps,dwmac.yaml#
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    soc {
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +        ethernet@...00000 {
> > +            compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a",
> > +            "snps,dwmac";
> > +            altr,sysmgr-syscon = <&sysmgr 0x60 0>;
> > +            reg = <0xff700000 0x2000>;
> > +            interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "macirq";
> > +            mac-address = [00 00 00 00 00 00]; /* Filled in by U-Boot */
> > +            clocks = <&emac_0_clk>;
> > +            clock-names = "stmmaceth";
> > +            phy-mode = "sgmii";
> > +        };
> > +    };
> > diff --git a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt b/Documentation/devicetree/bindings/net/socfpga-dwmac.txt
> > deleted file mode 100644
> > index 612a8e8abc88..000000000000
> > --- a/Documentation/devicetree/bindings/net/socfpga-dwmac.txt
> > +++ /dev/null
> > @@ -1,57 +0,0 @@
> > -Altera SOCFPGA SoC DWMAC controller
> > -
> > -This is a variant of the dwmac/stmmac driver an inherits all descriptions
> > -present in Documentation/devicetree/bindings/net/stmmac.txt.
> > -
> > -The device node has additional properties:
> > -
> > -Required properties:
> > - - compatible	: For Cyclone5/Arria5 SoCs it should contain
> > -		  "altr,socfpga-stmmac". For Arria10/Agilex/Stratix10 SoCs
> > -		  "altr,socfpga-stmmac-a10-s10".
> > -		  Along with "snps,dwmac" and any applicable more detailed
> > -		  designware version numbers documented in stmmac.txt
> > - - altr,sysmgr-syscon : Should be the phandle to the system manager node that
> > -   encompasses the glue register, the register offset, and the register shift.
> > -   On Cyclone5/Arria5, the register shift represents the PHY mode bits, while
> > -   on the Arria10/Stratix10/Agilex platforms, the register shift represents
> > -   bit for each emac to enable/disable signals from the FPGA fabric to the
> > -   EMAC modules.
> > - - altr,f2h_ptp_ref_clk use f2h_ptp_ref_clk instead of default eosc1 clock
> > -   for ptp ref clk. This affects all emacs as the clock is common.
> > -
> > -Optional properties:
> > -altr,emac-splitter: Should be the phandle to the emac splitter soft IP node if
> > -		DWMAC controller is connected emac splitter.
> > -phy-mode: The phy mode the ethernet operates in
> > -altr,sgmii-to-sgmii-converter: phandle to the TSE SGMII converter
> > -
> > -This device node has additional phandle dependency, the sgmii converter:
> > -
> > -Required properties:
> > - - compatible	: Should be altr,gmii-to-sgmii-2.0
> > - - reg-names	: Should be "eth_tse_control_port"
> > -
> > -Example:
> > -
> > -gmii_to_sgmii_converter: phy@...000240 {
> > -	compatible = "altr,gmii-to-sgmii-2.0";
> > -	reg = <0x00000001 0x00000240 0x00000008>,
> > -		<0x00000001 0x00000200 0x00000040>;
> > -	reg-names = "eth_tse_control_port";
> > -	clocks = <&sgmii_1_clk_0 &emac1 1 &sgmii_clk_125 &sgmii_clk_125>;
> > -	clock-names = "tse_pcs_ref_clk_clock_connection", "tse_rx_cdr_refclk";
> > -};
> > -
> > -gmac0: ethernet@...00000 {
> > -	compatible = "altr,socfpga-stmmac", "snps,dwmac-3.70a", "snps,dwmac";
> > -	altr,sysmgr-syscon = <&sysmgr 0x60 0>;
> > -	reg = <0xff700000 0x2000>;
> > -	interrupts = <0 115 4>;
> > -	interrupt-names = "macirq";
> > -	mac-address = [00 00 00 00 00 00];/* Filled in by U-Boot */
> > -	clocks = <&emac_0_clk>;
> > -	clock-names = "stmmaceth";
> > -	phy-mode = "sgmii";
> > -	altr,gmii-to-sgmii-converter = <&gmii_to_sgmii_converter>;
> > -};
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index c2b570ed5f2f..d308789d9877 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -3262,10 +3262,15 @@ M:	Dinh Nguyen <dinguyen@...nel.org>
> >  S:	Maintained
> >  F:	drivers/clk/socfpga/
> >  
> > +ARM/SOCFPGA DWMAC GLUE LAYER BINDINGS
> > +M:	Matthew Gerlach <matthew.gerlach@...era.com>
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/net/altr,gmii-to-sgmii-2.0.yaml
> > +F:	Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml
> > +
> >  ARM/SOCFPGA DWMAC GLUE LAYER
> >  M:	Maxime Chevallier <maxime.chevallier@...tlin.com>
> >  S:	Maintained
> > -F:	Documentation/devicetree/bindings/net/socfpga-dwmac.txt
> >  F:	drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
> >  
> >  ARM/SOCFPGA EDAC BINDINGS
> > -- 
> > 2.35.3
> > 

Powered by blists - more mailing lists