Re: [PATCH 1/3] dt-bindings: net: nuvoton: Add schema for Nuvoton MA35 family GMAC

From: Joey Lu
Date: Thu Nov 07 2024 - 05:16:10 EST

Dear Conor,

Thank you for your reply.

Conor Dooley 於 11/6/2024 11:44 PM 寫道:
On Wed, Nov 06, 2024 at 07:19:28PM +0800, Joey Lu wrote:
Create initial schema for Nuvoton MA35 family Gigabit MAC.

Signed-off-by: Joey Lu <a0987203069@xxxxxxxxx>
---
.../bindings/net/nuvoton,ma35xx-dwmac.yaml | 163 ++++++++++++++++++
1 file changed, 163 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/nuvoton,ma35xx-dwmac.yaml

diff --git a/Documentation/devicetree/bindings/net/nuvoton,ma35xx-dwmac.yaml b/Documentation/devicetree/bindings/net/nuvoton,ma35xx-dwmac.yaml
new file mode 100644
index 000000000000..f4d24ca872b2
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/nuvoton,ma35xx-dwmac.yaml
@@ -0,0 +1,163 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/nuvoton,ma35xx-dwmac.yaml#
The filename needs to match the compatible.
I will fix it.
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton DWMAC glue layer controller
+
+maintainers:
+ - Joey Lu <yclu4@xxxxxxxxxxx>
+
+description:
+ Nuvoton 10/100/1000Mbps Gigabit Ethernet MAC Controller is based on
+ Synopsys DesignWare MAC (version 3.73a).
+
+# We need a select here so we don't match all nodes with 'snps,dwmac'
+select:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - nuvoton,ma35d1-dwmac
+ required:
+ - compatible
+
+allOf:
+ - $ref: snps,dwmac.yaml#
+
+properties:
+ compatible:
+ - items:
+ - enum:
+ - nuvoton,ma35d1-dwmac
+ - const: snps,dwmac-3.70a
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ minItems: 2
+ items:
+ - description: MAC clock
+ - description: PTP clock
+
+ clock-names:
+ minItems: 2
+ contains:
+ - enum:
+ - stmmaceth
+ - ptp_ref
This can just be an items list like interrupt-names, since the clocks
property has a fixed order.
I will fix it.
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ items:
+ - const: macirq
This name carries no information, this is an interrupt for a mac after
all. You don't need a name since you only have one interrupt.
This interrupt name is an argument required by the stmmac driver to obtain interrupt information.
+ nuvoton,sys:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description: phandle to access GCR (Global Control Register) registers.
Why do you need a phandle to this? You appear to have multiple dwmacs on
your device if the example is anything to go by, how come you don't need
to access different portions of this depending on which dwmac instance
you are?
On our platform, a system register is required to specify the TX/RX clock path delay control, switch modes between RMII and RGMII, and configure other related settings.
+ resets:
+ maxItems: 1
+
+ reset-names:
+ items:
+ - const: stmmaceth
+
+ mac-id:
+ maxItems: 1
+ description:
+ The interface of MAC.
A vendor prefix is required for custom properties, but I don't think you
need this and actually it is a bandaid for some other information you're
missing. Probably related to your nuvoton,sys property only being a
phandle with no arguments.
This property will be removed.
+
+ phy-mode:
+ enum:
+ - rmii
+ - rgmii-id
+
+ tx_delay:
Needs constraints, a type, a vendor prefix and a unit suffix. No
underscores in property names either. See the amlogic dwmac binding for
an example.
I will fix it.
+ maxItems: 1
+ description:
+ Control transmit clock path delay in nanoseconds.
+
+ rx_delay:
Ditto here.
I will fix it.

+ maxItems: 1
+ description:
+ Control receive clock path delay in nanoseconds.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - interrupt-names
+ - clocks
+ - clock-names
+ - nuvoton,sys
+ - resets
+ - reset-names
+ - mac-id
+ - phy-mode
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+ #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
+ //Example 1
+ eth0: ethernet@40120000 {
The eth0 label is not used, drop it.
The label is used in dtsi and dts.
+ compatible = "nuvoton,ma35d1-dwmac";
+ reg = <0x0 0x40120000 0x0 0x10000>;
+ interrupts = <GIC_SPI 23 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ clocks = <&clk EMAC0_GATE>, <&clk EPLL_DIV8>;
+ clock-names = "stmmaceth", "ptp_ref";
+
+ nuvoton,sys = <&sys>;
+ resets = <&sys MA35D1_RESET_GMAC0>;
+ reset-names = "stmmaceth";
+ mac-id = <0>;
+
+ clk_csr = <4>;
This property is not documented.
This unused property will be removed.

Cheers,
Conor.

+ phy-mode = "rgmii-id";
+ phy-handle = <&eth_phy0>;
+ mdio0 {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eth_phy0: ethernet-phy@0 {
+ reg = <0>;
+ };
+ };
+ };
+
+ - |
+ //Example 2
+ eth1: ethernet@40130000 {
+ compatible = "nuvoton,ma35d1-dwmac";
+ reg = <0x0 0x40130000 0x0 0x10000>;
+ interrupts = <GIC_SPI 24 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "macirq";
+ clocks = <&clk EMAC1_GATE>, <&clk EPLL_DIV8>;
+ clock-names = "stmmaceth", "ptp_ref";
+
+ nuvoton,sys = <&sys>;
+ resets = <&sys MA35D1_RESET_GMAC1>;
+ reset-names = "stmmaceth";
+ mac-id = <1>;
+
+ clk_csr = <4>;
+ phy-mode = "rmii";
+ phy-handle = <&eth_phy1>;
+ mdio1 {
+ compatible = "snps,dwmac-mdio";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ eth_phy1: ethernet-phy@1 {
+ reg = <1>;
+ };
+ };
+ };
--
2.34.1

Thanks!

BR,

Joey