[PATCH v2 0/9] Add Ethernet driver for StarFive JH7110 SoC

[PATCH v2 0/9] Add Ethernet driver for StarFive JH7110 SoC

[Yanhong Wang]

This series adds ethernet support for the StarFive JH7110 RISC-V SoC.
The series includes PHY and MAC drivers. The PHY model is
YT8531 (from Motorcomm Inc), and the MAC version is dwmac-5.20
(from Synopsys DesignWare). For more information and support,
you can visit RVspace wiki[1].
	
This patchset should be applied after the patchset [2], [3], [4].

[1] https://wiki.rvspace.org/
[2] https://lore.kernel.org/all/20221118010627.70576-1-hal.feng@starfivetech.com/
[3] https://lore.kernel.org/all/20221118011108.70715-1-hal.feng@starfivetech.com/
[4] https://lore.kernel.org/all/20221118011714.70877-1-hal.feng@starfivetech.com/

Changes in v2:
- Recovered the author of the 1st and 3rd patches back to Emil Renner Berthing.
- Added a new patch to update maxitems number of resets property in 'snps,dwmac.yaml'.
- Fixed the check errors reported by "make dt_binding_check".
- Renamed the dt-binding 'starfive,dwmac-plat.yaml' to 'starfive,jh71x0-dwmac.yaml'.
- Updated the example context in the dt-binding 'starfive,jh71x0-dwmac.yaml'.
- Added new dt-binding 'motorcomm,yt8531.yaml' to describe details of phy clock
  delay configuration parameters.
- Added more comments for PHY driver setting. For more details, see
  'motorcomm,yt8531.yaml'.
- Moved mdio device tree node from 'jh7110-starfive-visionfive-v2.dts' to 'jh7110.dtsi'.
- Re-worded the commit message of several patches.
- Renamed all the functions with starfive_eth_plat prefix in 'dwmac-starfive-plat.c'.
- Added "starfive,jh7100-dwmac" compatible string and special init to support JH7100.

v1: https://patchwork.kernel.org/project/linux-riscv/patch/20221201090242.2381-8-yanhong.wang@starfivetech.com/

Emil Renner Berthing (2):
  dt-bindings: net: snps,dwmac: Add dwmac-5.20 version
  net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

Yanhong Wang (7):
  dt-bindings: net: snps,dwmac: Update the maxitems number of resets and
    reset-names
  dt-bindings: net: Add bindings for StarFive dwmac
  dt-bindings: net: motorcomm: add support for Motorcomm YT8531
  net: phy: motorcomm: Add YT8531 phy support
  net: stmmac: Add glue layer for StarFive JH71x0 SoCs
  riscv: dts: starfive: jh7110: Add ethernet device node
  riscv: dts: starfive: visionfive-v2: Add phy clock delay train
    configuration

 .../bindings/net/motorcomm,yt8531.yaml        | 111 ++++++++++
 .../devicetree/bindings/net/snps,dwmac.yaml   |  20 +-
 .../bindings/net/starfive,jh71x0-dwmac.yaml   | 103 +++++++++
 MAINTAINERS                                   |   7 +
 .../jh7110-starfive-visionfive-v2.dts         |  28 +++
 arch/riscv/boot/dts/starfive/jh7110.dtsi      |  93 ++++++++
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  12 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-starfive-plat.c      | 167 +++++++++++++++
 .../ethernet/stmicro/stmmac/stmmac_platform.c |   3 +-
 drivers/net/phy/Kconfig                       |   3 +-
 drivers/net/phy/motorcomm.c                   | 202 ++++++++++++++++++
 12 files changed, 744 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
 create mode 100644 Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c


base-commit: 094226ad94f471a9f19e8f8e7140a09c2625abaa
prerequisite-patch-id: 8ebfffa09b478904bf7c516f76e2d824ddb60140
prerequisite-patch-id: e8dd8258a4c4062eee2cf07c4607d52baea71f3a
prerequisite-patch-id: d050d884d7b091ff30508a70f5ce5164bb3b72e5
prerequisite-patch-id: 0e41f8cfd4861fcbf6f2e6a2559ce28f0450299e
prerequisite-patch-id: 6e1652501859b85f101ff3b15ced585d43c71c1b
prerequisite-patch-id: 587628a67adad5c655e5f998bf6c4a368ec07d3c
prerequisite-patch-id: 596490c0e397df6c0249c1306fbb1d5bf00b5b83
prerequisite-patch-id: dc873317826b50364344b25ac5cd74e811403f3d
prerequisite-patch-id: a50150f41d8e874553023187e22eb24dffae8d16
prerequisite-patch-id: 735e62255c75801bdc4c0b4107850bce821ff7f5
prerequisite-patch-id: 9d2e83a2dd43e193f534283fab73e90b4f435043
prerequisite-patch-id: 7a43e0849a9afa3c6f83547fd16d9271b07619e5
prerequisite-patch-id: e7aa6fb05314bad6d94c465f3f59969871bf3d2e
prerequisite-patch-id: 6276b2a23818c65ff2ad3d65b562615690cffee9
prerequisite-patch-id: d834ece14ffb525b8c3e661e78736692f33fca9b
prerequisite-patch-id: 4c17a3ce4dae9b788795d915bf775630f5c43c53
prerequisite-patch-id: dabb913fd478e97593e45c23fee4be9fd807f851
prerequisite-patch-id: ba61df106fbe2ada21e8f22c3d2cfaf7809c84b6
prerequisite-patch-id: 287572fb64f83f5d931034f7c75674907584a087
prerequisite-patch-id: 536114f0732646095ef5302a165672b3290d4c75
prerequisite-patch-id: 258ea5f9b8bf41b6981345dcc81795f25865d38f
prerequisite-patch-id: 8b6f2c9660c0ac0ee4e73e4c21aca8e6b75e81b9
prerequisite-patch-id: e09e995700a814a763aa304ad3881a7222acf556
prerequisite-patch-id: 841cd71b556b480d6a5a5e332eeca70d6a76ec3f
prerequisite-patch-id: d074c7ffa2917a9f754d5801e3f67bc980f9de4c
prerequisite-patch-id: 5f59bc7cbbf1230e5ff4761fa7c1116d4e6e5d71
prerequisite-patch-id: d5da3475c6a3588e11a1678feb565bdd459b548e
-- 
2.17.1

[PATCH v2 1/9] dt-bindings: net: snps,dwmac: Add dwmac-5.20 version

[Yanhong Wang]

From: Emil Renner Berthing <kernel@esmil.dk>

Add dwmac-5.20 IP version to snps.dwmac.yaml

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 Documentation/devicetree/bindings/net/snps,dwmac.yaml | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 13b984076af5..e26c3e76ebb7 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -30,6 +30,7 @@ select:
           - snps,dwmac-4.10a
           - snps,dwmac-4.20a
           - snps,dwmac-5.10a
+          - snps,dwmac-5.20
           - snps,dwxgmac
           - snps,dwxgmac-2.10
 
@@ -87,6 +88,7 @@ properties:
         - snps,dwmac-4.10a
         - snps,dwmac-4.20a
         - snps,dwmac-5.10a
+        - snps,dwmac-5.20
         - snps,dwxgmac
         - snps,dwxgmac-2.10
 
@@ -393,6 +395,7 @@ allOf:
               - snps,dwmac-3.50a
               - snps,dwmac-4.10a
               - snps,dwmac-4.20a
+              - snps,dwmac-5.20
               - snps,dwxgmac
               - snps,dwxgmac-2.10
               - st,spear600-gmac
@@ -447,6 +450,7 @@ allOf:
               - snps,dwmac-4.10a
               - snps,dwmac-4.20a
               - snps,dwmac-5.10a
+              - snps,dwmac-5.20
               - snps,dwxgmac
               - snps,dwxgmac-2.10
               - st,spear600-gmac
-- 
2.17.1

[PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[Yanhong Wang]

Some boards(such as StarFive VisionFive v2) require more than one value
which defined by resets property, so the original definition can not
meet the requirements. In order to adapt to different requirements,
adjust the maxitems number from 1 to 3..

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml       | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index e26c3e76ebb7..7870228b4cd3 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -133,12 +133,19 @@ properties:
         - ptp_ref
 
   resets:
-    maxItems: 1
-    description:
-      MAC Reset signal.
+    minItems: 1
+    maxItems: 3
+    additionalItems: true
+    items:
+      - description: MAC Reset signal
 
   reset-names:
-    const: stmmaceth
+    minItems: 1
+    maxItems: 3
+    additionalItems: true
+    contains:
+      enum:
+        - stmmaceth
 
   power-domains:
     maxItems: 1
-- 
2.17.1

[PATCH v2 3/9] net: stmmac: platform: Add snps,dwmac-5.20 IP compatible string

[Yanhong Wang]

From: Emil Renner Berthing <kernel@esmil.dk>

Add "snps,dwmac-5.20" compatible string for 5.20 version that can avoid
to define some platform data in the glue layer.

Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 50f6b4a14be4..cc3b701af802 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -519,7 +519,8 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
 	if (of_device_is_compatible(np, "snps,dwmac-4.00") ||
 	    of_device_is_compatible(np, "snps,dwmac-4.10a") ||
 	    of_device_is_compatible(np, "snps,dwmac-4.20a") ||
-	    of_device_is_compatible(np, "snps,dwmac-5.10a")) {
+	    of_device_is_compatible(np, "snps,dwmac-5.10a") ||
+	    of_device_is_compatible(np, "snps,dwmac-5.20")) {
 		plat->has_gmac4 = 1;
 		plat->has_gmac = 0;
 		plat->pmt = 1;
-- 
2.17.1

[PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[Yanhong Wang]

Add documentation to describe StarFive dwmac driver(GMAC).

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
 .../bindings/net/starfive,jh71x0-dwmac.yaml   | 103 ++++++++++++++++++
 MAINTAINERS                                   |   5 +
 3 files changed, 109 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml

diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 7870228b4cd3..cdb045d1c618 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -91,6 +91,7 @@ properties:
         - snps,dwmac-5.20
         - snps,dwxgmac
         - snps,dwxgmac-2.10
+        - starfive,jh7110-dwmac
 
   reg:
     minItems: 1
diff --git a/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
new file mode 100644
index 000000000000..5cb1272fe959
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
@@ -0,0 +1,103 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2022 StarFive Technology Co., Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/starfive,jh71x0-dwmac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: StarFive JH71x0 DWMAC glue layer
+
+maintainers:
+  - Yanhong Wang <yanhong.wang@starfivetech.com>
+
+select:
+  properties:
+    compatible:
+      contains:
+        enum:
+          - starfive,jh7110-dwmac
+  required:
+    - compatible
+
+allOf:
+  - $ref: snps,dwmac.yaml#
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - starfive,jh7110-dwmac
+      - const: snps,dwmac-5.20
+
+  clocks:
+    items:
+      - description: GMAC main clock
+      - description: GMAC AHB clock
+      - description: PTP clock
+      - description: TX clock
+      - description: GTXC clock
+      - description: GTX clock
+
+  clock-names:
+    items:
+      - const: stmmaceth
+      - const: pclk
+      - const: ptp_ref
+      - const: tx
+      - const: gtxc
+      - const: gtx
+
+required:
+  - compatible
+  - clocks
+  - clock-names
+  - resets
+  - reset-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    stmmac_axi_setup: stmmac-axi-config {
+        snps,lpi_en;
+        snps,wr_osr_lmt = <4>;
+        snps,rd_osr_lmt = <4>;
+        snps,blen = <256 128 64 32 0 0 0>;
+    };
+
+    gmac0: ethernet@16030000 {
+        compatible = "starfive,jh7110-dwmac", "snps,dwmac-5.20";
+        reg = <0x16030000 0x10000>;
+        clocks = <&clk 3>, <&clk 2>, <&clk 109>,
+                 <&clk 5>, <&clk 111>, <&clk 108>;
+        clock-names = "stmmaceth", "pclk", "ptp_ref",
+                      "tx", "gtxc", "gtx";
+        resets = <&rst 1>, <&rst 2>;
+        reset-names = "stmmaceth", "ahb";
+        interrupts = <7>, <6>, <5>;
+        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+        phy-mode = "rgmii-id";
+        snps,multicast-filter-bins = <64>;
+        snps,perfect-filter-entries = <8>;
+        rx-fifo-depth = <2048>;
+        tx-fifo-depth = <2048>;
+        snps,fixed-burst;
+        snps,no-pbl-x8;
+        snps,tso;
+        snps,force_thresh_dma_mode;
+        snps,axi-config = <&stmmac_axi_setup>;
+        snps,en-tx-lpi-clockgating;
+        snps,txpbl = <16>;
+        snps,rxpbl = <16>;
+        phy-handle = <&phy0>;
+
+        mdio {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "snps,dwmac-mdio";
+
+            phy0: ethernet-phy@0 {
+                reg = <0>;
+            };
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index a70c1d0f303e..166b0009f63c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19606,6 +19606,11 @@ F:	Documentation/devicetree/bindings/clock/starfive*
 F:	drivers/clk/starfive/
 F:	include/dt-bindings/clock/starfive*
 
+STARFIVE DWMAC GLUE LAYER
+M:	Yanhong Wang <yanhong.wang@starfivetech.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
+
 STARFIVE PINCTRL DRIVER
 M:	Emil Renner Berthing <kernel@esmil.dk>
 M:	Jianlong Huang <jianlong.huang@starfivetech.com>
-- 
2.17.1

[PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[Yanhong Wang]

Add support for Motorcomm Technology YT8531 10/100/1000 Ethernet PHY.
The document describe details of clock delay train configuration.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../bindings/net/motorcomm,yt8531.yaml        | 111 ++++++++++++++++++
 MAINTAINERS                                   |   1 +
 2 files changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml

diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
new file mode 100644
index 000000000000..c5b8a09a78bb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
@@ -0,0 +1,111 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/motorcomm,yt8531.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Motorcomm YT8531 Gigabit Ethernet PHY
+
+maintainers:
+  - Yanhong Wang <yanhong.wang@starfivetech.com>
+
+select:
+  properties:
+    $nodename:
+      pattern: "^ethernet-phy(@[a-f0-9]+)?$"
+
+  required:
+    - $nodename
+
+properties:
+  $nodename:
+    pattern: "^ethernet-phy(@[a-f0-9]+)?$"
+
+  reg:
+    minimum: 0
+    maximum: 31
+    description:
+      The ID number for the PHY.
+
+  rxc_dly_en:
+    description: |
+      RGMII Receive PHY Clock Delay defined with fixed 2ns.This is used for
+      PHY that have configurable RX internal delays. If this property set
+      to 1, then automatically add 2ns delay pad for Receive PHY clock.
+    enum: [0, 1]
+    default: 0
+
+  rx_delay_sel:
+    description: |
+      This is supplement to rxc_dly_en property,and it can
+      be specified in 150ps(pico seconds) steps. The effective
+      delay is: 150ps * N.
+    minimum: 0
+    maximum: 15
+    default: 0
+
+  tx_delay_sel_fe:
+    description: |
+      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
+      PHY's that have configurable TX internal delays when speed is 100Mbps
+      or 10Mbps. It can be specified in 150ps steps, the effective delay
+      is: 150ps * N.
+    minimum: 0
+    maximum: 15
+    default: 15
+
+  tx_delay_sel:
+    description: |
+      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
+      PHY's that have configurable TX internal delays when speed is 1000Mbps.
+      It can be specified in 150ps steps, the effective delay is: 150ps * N.
+    minimum: 0
+    maximum: 15
+    default: 1
+
+  tx_inverted_10:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 10Mbps.
+      0: original   1: inverted
+    enum: [0, 1]
+    default: 0
+
+  tx_inverted_100:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 100Mbps.
+      0: original   1: inverted
+    enum: [0, 1]
+    default: 0
+
+  tx_inverted_1000:
+    description: |
+      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
+      Transmit PHY Clock delay train configuration when speed is 1000Mbps.
+      0: original   1: inverted
+    enum: [0, 1]
+    default: 0
+
+required:
+  - reg
+
+additionalProperties: true
+
+examples:
+  - |
+    ethernet {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        ethernet-phy@0 {
+            reg = <0>;
+
+            rxc_dly_en = <1>;
+            tx_delay_sel_fe = <5>;
+            tx_delay_sel = <0xa>;
+            tx_inverted_10 = <0x1>;
+            tx_inverted_100 = <0x1>;
+            tx_inverted_1000 = <0x1>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 166b0009f63c..1ff68b8524d2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19609,6 +19609,7 @@ F:	include/dt-bindings/clock/starfive*
 STARFIVE DWMAC GLUE LAYER
 M:	Yanhong Wang <yanhong.wang@starfivetech.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
 F:	Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
 
 STARFIVE PINCTRL DRIVER
-- 
2.17.1

[PATCH v2 6/9] net: phy: motorcomm: Add YT8531 phy support

[Yanhong Wang]

This adds basic support for the Motorcomm YT8531
Gigabit Ethernet PHY.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 drivers/net/phy/Kconfig     |   3 +-
 drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++
 2 files changed, 204 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index c57a0262fb64..86399254d9ff 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -258,9 +258,10 @@ config MICROSEMI_PHY
 
 config MOTORCOMM_PHY
 	tristate "Motorcomm PHYs"
+	default SOC_STARFIVE
 	help
 	  Enables support for Motorcomm network PHYs.
-	  Currently supports the YT8511 gigabit PHY.
+	  Currently supports the YT8511 and YT8531 gigabit PHYs.
 
 config NATIONAL_PHY
 	tristate "National Semiconductor PHYs"
diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index 7e6ac2c5e27e..bca03185b338 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -3,13 +3,17 @@
  * Driver for Motorcomm PHYs
  *
  * Author: Peter Geis <pgwipeout@gmail.com>
+ *
  */
 
+#include <linux/bitops.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/phy.h>
 
 #define PHY_ID_YT8511		0x0000010a
+#define PHY_ID_YT8531		0x4f51e91b
 
 #define YT8511_PAGE_SELECT	0x1e
 #define YT8511_PAGE		0x1f
@@ -17,6 +21,10 @@
 #define YT8511_EXT_DELAY_DRIVE	0x0d
 #define YT8511_EXT_SLEEP_CTRL	0x27
 
+#define YTPHY_EXT_SMI_SDS_PHY		0xa000
+#define YTPHY_EXT_CHIP_CONFIG		0xa001
+#define YTPHY_EXT_RGMII_CONFIG1	0xa003
+
 /* 2b00 25m from pll
  * 2b01 25m from xtl *default*
  * 2b10 62.m from pll
@@ -38,6 +46,51 @@
 #define YT8511_DELAY_FE_TX_EN	(0xf << 12)
 #define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
 
+struct ytphy_reg_field {
+	char *name;
+	u32 mask;
+	u8	dflt;	/* Default value */
+};
+
+struct ytphy_priv_t {
+	u32 tx_inverted_1000;
+	u32 tx_inverted_100;
+	u32 tx_inverted_10;
+};
+
+/* rx_delay_sel: RGMII rx clock delay train configuration, about 150ps per
+ *               step. Delay = 150ps * N
+ *
+ * tx_delay_sel_fe: RGMII tx clock delay train configuration when speed is
+ *                  100Mbps or 10Mbps, it's 150ps per step. Delay = 150ps * N
+ *
+ * tx_delay_sel: RGMII tx clock delay train configuration when speed is
+ *               1000Mbps, it's 150ps per step. Delay = 150ps * N
+ */
+static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
+	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
+	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
+	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
+};
+
+/* tx_inverted_x: Use original or inverted RGMII TX_CLK to drive the RGMII
+ *                TX_CLK delay train configuration when speed is
+ *                xMbps(10/100/1000Mbps).
+ *                0: original,  1: inverted
+ */
+static const struct ytphy_reg_field ytphy_txinver_grp[] = {
+	{ "tx_inverted_1000", BIT(14), 0x0 },
+	{ "tx_inverted_100", BIT(14), 0x0 },
+	{ "tx_inverted_10", BIT(14), 0x0 }
+};
+
+/* rxc_dly_en: RGMII clk 2ns delay control bit.
+ *             0: disable   1: enable
+ */
+static const struct ytphy_reg_field ytphy_rxden_grp[] = {
+	{ "rxc_dly_en", BIT(8), 0x1 }
+};
+
 static int yt8511_read_page(struct phy_device *phydev)
 {
 	return __phy_read(phydev, YT8511_PAGE_SELECT);
@@ -48,6 +101,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page)
 	return __phy_write(phydev, YT8511_PAGE_SELECT, page);
 };
 
+static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
+{
+	int ret;
+	int val;
+
+	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	val = __phy_read(phydev, YT8511_PAGE);
+
+	return val;
+}
+
+static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val)
+{
+	int ret;
+
+	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
+	if (ret < 0)
+		return ret;
+
+	ret = __phy_write(phydev, YT8511_PAGE, val);
+
+	return ret;
+}
+
 static int yt8511_config_init(struct phy_device *phydev)
 {
 	int oldpage, ret = 0;
@@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device *phydev)
 	return phy_restore_page(phydev, oldpage, ret);
 }
 
+static int ytphy_config_init(struct phy_device *phydev)
+{
+	struct device_node *of_node;
+	u32 val;
+	u32 mask;
+	u32 cfg;
+	int ret;
+	int i = 0;
+
+	of_node = phydev->mdio.dev.of_node;
+	if (of_node) {
+		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
+		if (!ret) {
+			mask = ytphy_rxden_grp[0].mask;
+			val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
+
+			/* check the cfg overflow or not */
+			cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
+
+			val &= ~mask;
+			val |= FIELD_PREP(mask, cfg);
+			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);
+		}
+
+		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
+		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
+			ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
+			if (!ret) {
+				mask = ytphy_rxtxd_grp[i].mask;
+
+				/* check the cfg overflow or not */
+				cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
+
+				val &= ~mask;
+				val |= cfg << (ffs(mask) - 1);
+			}
+		}
+		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
+	}
+
+	phydev_err(phydev, "Get of node fail\n");
+
+	return -EINVAL;
+}
+
+static void ytphy_link_change_notify(struct phy_device *phydev)
+{
+	u32 val;
+	struct ytphy_priv_t *ytphy_priv = phydev->priv;
+
+	if (phydev->speed < 0)
+		return;
+
+	val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
+	switch (phydev->speed) {
+	case SPEED_1000:
+		val  &= ~ytphy_txinver_grp[0].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[0].mask,
+				ytphy_priv->tx_inverted_1000);
+		break;
+
+	case SPEED_100:
+		val  &= ~ytphy_txinver_grp[1].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[1].mask,
+				ytphy_priv->tx_inverted_100);
+		break;
+
+	case SPEED_10:
+		val  &= ~ytphy_txinver_grp[2].mask;
+		val |= FIELD_PREP(ytphy_txinver_grp[2].mask,
+				ytphy_priv->tx_inverted_10);
+		break;
+
+	default:
+		break;
+	}
+
+	ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
+}
+
+static int yt8531_probe(struct phy_device *phydev)
+{
+	struct ytphy_priv_t *priv;
+	const struct device_node *of_node;
+	u32 val;
+	int ret;
+
+	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	of_node = phydev->mdio.dev.of_node;
+	if (of_node) {
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val);
+		if (!ret)
+			priv->tx_inverted_1000 = val;
+
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val);
+		if (!ret)
+			priv->tx_inverted_100 = val;
+
+		ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val);
+		if (!ret)
+			priv->tx_inverted_10 = val;
+	}
+	phydev->priv = priv;
+
+	return 0;
+}
+
 static struct phy_driver motorcomm_phy_drvs[] = {
 	{
 		PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
@@ -120,6 +310,17 @@ static struct phy_driver motorcomm_phy_drvs[] = {
 		.resume		= genphy_resume,
 		.read_page	= yt8511_read_page,
 		.write_page	= yt8511_write_page,
+	}, {
+		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
+		.name		= "YT8531 Gigabit Ethernet",
+		.probe		= yt8531_probe,
+		.config_init	= ytphy_config_init,
+		.read_status	= genphy_read_status,
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+		.read_page	= yt8511_read_page,
+		.write_page	= yt8511_write_page,
+		.link_change_notify = ytphy_link_change_notify,
 	},
 };
 
@@ -131,6 +332,7 @@ MODULE_LICENSE("GPL");
 
 static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
 	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8511) },
+	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
 	{ /* sentinal */ }
 };
 
-- 
2.17.1

[PATCH v2 7/9] net: stmmac: Add glue layer for StarFive JH71x0 SoCs

[Yanhong Wang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="n", Size: 7343 bytes --]

This adds StarFive dwmac driver support on the StarFive JH71x0 SoCs.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
---
 MAINTAINERS                                   |   1 +
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |  12 ++
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../stmicro/stmmac/dwmac-starfive-plat.c      | 167 ++++++++++++++++++
 4 files changed, 181 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 1ff68b8524d2..c7d3229330aa 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19609,6 +19609,7 @@ F:	include/dt-bindings/clock/starfive*
 STARFIVE DWMAC GLUE LAYER
 M:	Yanhong Wang <yanhong.wang@starfivetech.com>
 S:	Maintained
+F:	Documentation/devicetree/bindings/net/dwmac-starfive-plat.c
 F:	Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
 F:	Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 31ff35174034..ca3d4dd95a95 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -235,6 +235,18 @@ config DWMAC_INTEL_PLAT
 	  the stmmac device driver. This driver is used for the Intel Keem Bay
 	  SoC.
 
+config DWMAC_STARFIVE_PLAT
+	tristate "StarFive dwmac support"
+	depends on OF && COMMON_CLK
+	depends on STMMAC_ETH
+	default SOC_STARFIVE
+	help
+	  Support for ethernet controllers on StarFive RISC-V SoCs
+
+	  This selects the StarFive platform specific glue layer support for
+	  the stmmac device driver. This driver is used for StarFive JH71x0
+	  ethernet controller.
+
 config DWMAC_VISCONTI
 	tristate "Toshiba Visconti DWMAC support"
 	default ARCH_VISCONTI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index d4e12e9ace4f..a63ab0ab5071 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -31,6 +31,7 @@ obj-$(CONFIG_DWMAC_DWC_QOS_ETH)	+= dwmac-dwc-qos-eth.o
 obj-$(CONFIG_DWMAC_INTEL_PLAT)	+= dwmac-intel-plat.o
 obj-$(CONFIG_DWMAC_GENERIC)	+= dwmac-generic.o
 obj-$(CONFIG_DWMAC_IMX8)	+= dwmac-imx.o
+obj-$(CONFIG_DWMAC_STARFIVE_PLAT)	+= dwmac-starfive-plat.o
 obj-$(CONFIG_DWMAC_VISCONTI)	+= dwmac-visconti.o
 stmmac-platform-objs:= stmmac_platform.o
 dwmac-altr-socfpga-objs := altr_tse_pcs.o dwmac-socfpga.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
new file mode 100644
index 000000000000..f8b68dd53c91
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive-plat.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * StarFive DWMAC platform driver
+ *
+ * Copyright(C) 2022 StarFive Technology Co., Ltd.
+ *
+ */
+
+#include <linux/mfd/syscon.h>
+#include <linux/of_device.h>
+#include <linux/regmap.h>
+#include "stmmac_platform.h"
+
+struct starfive_dwmac {
+	struct device *dev;
+	struct clk *clk_tx;
+	struct clk *clk_gtx;
+	struct clk *clk_gtxc;
+};
+
+#define JH7100_SYSMAIN_REGISTER28 0x70
+/* The value below is not a typo, just really bad naming by StarFive ¯\_(ツ)_/¯ */
+#define JH7100_SYSMAIN_REGISTER49 0xc8
+
+static int starfive_eth_plat_jh7100_syscon_init(struct device *dev)
+{
+	struct device_node *np = dev->of_node;
+	struct regmap *sysmain;
+	u32 gtxclk_dlychain;
+	int ret;
+
+	sysmain = syscon_regmap_lookup_by_phandle(np, "starfive,syscon");
+	if (IS_ERR(sysmain))
+		return dev_err_probe(dev, PTR_ERR(sysmain),
+				     "error getting sysmain registers\n");
+
+	/* Choose RGMII interface to the phy.
+	 * TODO: support other interfaces once we know the meaning of other
+	 * values in the register
+	 */
+	ret = regmap_update_bits(sysmain, JH7100_SYSMAIN_REGISTER28, 0x7, 1);
+	if (ret)
+		return dev_err_probe(dev, ret, "error selecting gmac interface\n");
+
+	if (!of_property_read_u32(np, "starfive,gtxclk-dlychain", &gtxclk_dlychain)) {
+		ret = regmap_write(sysmain, JH7100_SYSMAIN_REGISTER49, gtxclk_dlychain);
+		if (ret)
+			return dev_err_probe(dev, ret, "error selecting gtxclk delay chain\n");
+	}
+
+	return 0;
+}
+
+static void starfive_eth_plat_fix_mac_speed(void *priv, unsigned int speed)
+{
+	struct starfive_dwmac *dwmac = priv;
+	unsigned long rate;
+	int err;
+
+	switch (speed) {
+	case SPEED_1000:
+		rate = 125000000;
+		break;
+	case SPEED_100:
+		rate = 25000000;
+		break;
+	case SPEED_10:
+		rate = 2500000;
+		break;
+	default:
+		dev_err(dwmac->dev, "invalid speed %u\n", speed);
+		return;
+	}
+
+	err = clk_set_rate(dwmac->clk_gtx, rate);
+	if (err)
+		dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
+}
+
+static int starfive_eth_plat_probe(struct platform_device *pdev)
+{
+	struct plat_stmmacenet_data *plat_dat;
+	struct stmmac_resources stmmac_res;
+	struct starfive_dwmac *dwmac;
+	int (*syscon_init)(struct device *dev);
+	int err;
+
+	err = stmmac_get_platform_resources(pdev, &stmmac_res);
+	if (err)
+		return err;
+
+	plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
+	if (IS_ERR(plat_dat)) {
+		dev_err(&pdev->dev, "dt configuration failed\n");
+		return PTR_ERR(plat_dat);
+	}
+
+	dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
+	if (!dwmac)
+		return -ENOMEM;
+
+	syscon_init = of_device_get_match_data(&pdev->dev);
+	if (syscon_init) {
+		err = syscon_init(&pdev->dev);
+		if (err)
+			return err;
+	}
+
+	dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
+	if (IS_ERR(dwmac->clk_tx))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
+						"error getting tx clock\n");
+
+	dwmac->clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
+	if (IS_ERR(dwmac->clk_gtx))
+		return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
+						"error getting gtx clock\n");
+
+	/* Only StarFive JH7110 SoC support gtxc clock */
+	if (of_device_is_compatible(pdev->dev.of_node, "starfive,jh7110-dwmac")) {
+		dwmac->clk_gtxc = devm_clk_get_enabled(&pdev->dev, "gtxc");
+		if (IS_ERR(dwmac->clk_gtxc))
+			return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtxc),
+							"error getting gtxc clock\n");
+	}
+
+	dwmac->dev = &pdev->dev;
+	plat_dat->fix_mac_speed = starfive_eth_plat_fix_mac_speed;
+	plat_dat->init = NULL;
+	plat_dat->bsp_priv = dwmac;
+	plat_dat->dma_cfg->dche = true;
+
+	err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
+	if (err) {
+		stmmac_remove_config_dt(pdev, plat_dat);
+		return err;
+	}
+
+	return 0;
+}
+
+static const struct of_device_id starfive_eth_plat_match[] = {
+	{
+		.compatible = "starfive,jh7110-dwmac"
+	},
+	{
+		.compatible = "starfive,jh7100-dwmac",
+		.data = starfive_eth_plat_jh7100_syscon_init,
+	},
+	{ }
+};
+
+static struct platform_driver starfive_eth_plat_driver = {
+	.probe  = starfive_eth_plat_probe,
+	.remove = stmmac_pltfr_remove,
+	.driver = {
+		.name = "starfive-eth-plat",
+		.pm = &stmmac_pltfr_pm_ops,
+		.of_match_table = starfive_eth_plat_match,
+	},
+};
+
+module_platform_driver(starfive_eth_plat_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("StarFive DWMAC platform driver");
+MODULE_AUTHOR("Yanhong Wang <yanhong.wang@starfivetech.com>");
-- 
2.17.1

[PATCH v2 8/9] riscv: dts: starfive: jh7110: Add ethernet device node

[Yanhong Wang]

Add JH7110 ethernet device node to support gmac driver for the JH7110
RISC-V SoC.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 arch/riscv/boot/dts/starfive/jh7110.dtsi | 93 ++++++++++++++++++++++++
 1 file changed, 93 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi
index c22e8f1d2640..c6de6e3b1a25 100644
--- a/arch/riscv/boot/dts/starfive/jh7110.dtsi
+++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
@@ -433,5 +433,98 @@
 			reg-shift = <2>;
 			status = "disabled";
 		};
+
+		stmmac_axi_setup: stmmac-axi-config {
+			snps,lpi_en;
+			snps,wr_osr_lmt = <4>;
+			snps,rd_osr_lmt = <4>;
+			snps,blen = <256 128 64 32 0 0 0>;
+		};
+
+		gmac0: ethernet@16030000 {
+			compatible = "starfive,jh7110-dwmac", "snps,dwmac-5.20";
+			reg = <0x0 0x16030000 0x0 0x10000>;
+			clocks = <&aoncrg JH7110_AONCLK_GMAC0_AXI>,
+				 <&aoncrg JH7110_AONCLK_GMAC0_AHB>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_PTP>,
+				 <&aoncrg JH7110_AONCLK_GMAC0_TX>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_GTXC>,
+				 <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>;
+			clock-names = "stmmaceth", "pclk", "ptp_ref",
+						"tx", "gtxc", "gtx";
+			resets = <&aoncrg JH7110_AONRST_GMAC0_AXI>,
+				 <&aoncrg JH7110_AONRST_GMAC0_AHB>;
+			reset-names = "stmmaceth", "ahb";
+			interrupts = <7>, <6>, <5>;
+			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+			phy-mode = "rgmii-id";
+			snps,multicast-filter-bins = <64>;
+			snps,perfect-filter-entries = <8>;
+			rx-fifo-depth = <2048>;
+			tx-fifo-depth = <2048>;
+			snps,fixed-burst;
+			snps,no-pbl-x8;
+			snps,force_thresh_dma_mode;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,tso;
+			snps,en-tx-lpi-clockgating;
+			snps,txpbl = <16>;
+			snps,rxpbl = <16>;
+			status = "disabled";
+			phy-handle = <&phy0>;
+
+			mdio0: mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "snps,dwmac-mdio";
+
+				phy0: ethernet-phy@0 {
+					reg = <0>;
+				};
+			};
+		};
+
+		gmac1: ethernet@16040000 {
+			compatible = "starfive,jh7110-dwmac", "snps,dwmac-5.20";
+			reg = <0x0 0x16040000 0x0 0x10000>;
+			clocks = <&syscrg JH7110_SYSCLK_GMAC1_AXI>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_AHB>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_PTP>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_TX>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_GTXC>,
+				 <&syscrg JH7110_SYSCLK_GMAC1_GTXCLK>;
+			clock-names = "stmmaceth", "pclk", "ptp_ref",
+					"tx", "gtxc", "gtx";
+			resets = <&syscrg JH7110_SYSRST_GMAC1_AXI>,
+				 <&syscrg JH7110_SYSRST_GMAC1_AHB>;
+			reset-names = "stmmaceth", "ahb";
+			interrupts = <78>, <77>, <76>;
+			interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
+			phy-mode = "rgmii-id";
+			snps,multicast-filter-bins = <64>;
+			snps,perfect-filter-entries = <8>;
+			rx-fifo-depth = <2048>;
+			tx-fifo-depth = <2048>;
+			snps,fixed-burst;
+			snps,no-pbl-x8;
+			snps,force_thresh_dma_mode;
+			snps,axi-config = <&stmmac_axi_setup>;
+			snps,tso;
+			snps,en-tx-lpi-clockgating;
+			snps,txpbl = <16>;
+			snps,rxpbl = <16>;
+			status = "disabled";
+			phy-handle = <&phy1>;
+
+			mdio1: mdio {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "snps,dwmac-mdio";
+
+				phy1: ethernet-phy@1 {
+					reg = <1>;
+				};
+			};
+		};
 	};
 };
-- 
2.17.1

[PATCH v2 9/9] riscv: dts: starfive: visionfive-v2: Add phy clock delay train configuration

[Yanhong Wang]

In view of the particularity of StarFive JH7110 SoC, the PHY clock delay
train configuration parameters must be adjusted for StarFive VisionFive
v2 board.

Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
---
 .../jh7110-starfive-visionfive-v2.dts         | 28 +++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
index c8946cf3a268..81329d67ef0f 100644
--- a/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
+++ b/arch/riscv/boot/dts/starfive/jh7110-starfive-visionfive-v2.dts
@@ -15,6 +15,8 @@
 
 	aliases {
 		serial0 = &uart0;
+		ethernet0 = &gmac0;
+		ethernet1 = &gmac1;
 	};
 
 	chosen {
@@ -114,3 +116,29 @@
 	pinctrl-0 = <&uart0_pins>;
 	status = "okay";
 };
+
+&gmac0 {
+	status = "okay";
+};
+
+&phy0 {
+	rxc_dly_en = <1>;
+	tx_delay_sel_fe = <5>;
+	tx_delay_sel = <0xa>;
+	tx_inverted_10 = <0x1>;
+	tx_inverted_100 = <0x1>;
+	tx_inverted_1000 = <0x1>;
+};
+
+&gmac1 {
+	status = "okay";
+};
+
+&phy1 {
+	rxc_dly_en = <0>;
+	tx_delay_sel_fe = <5>;
+	tx_delay_sel = <0>;
+	tx_inverted_10 = <0x1>;
+	tx_inverted_100 = <0x1>;
+	tx_inverted_1000 = <0x0>;
+};
-- 
2.17.1

Re: [PATCH v2 6/9] net: phy: motorcomm: Add YT8531 phy support

[Arun.Ramadoss]

Hi Yanhong,

On Fri, 2022-12-16 at 15:06 +0800, Yanhong Wang wrote:
> This adds basic support for the Motorcomm YT8531
> Gigabit Ethernet PHY.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  drivers/net/phy/Kconfig     |   3 +-
>  drivers/net/phy/motorcomm.c | 202
> ++++++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+), 1 deletion(-)
> 
> 
> +static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
> +{
> +	int ret;
> +	int val;
> +
> +	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
> +	if (ret < 0)
> +		return ret;

You are returning the error value as well as the read value in the
function. But in the config_init, you are not checking the error value,
just using the value directly. So instead of returning both the value
and error, you may consider return error and for value use call by
reference.

> +
> +	val = __phy_read(phydev, YT8511_PAGE);
> +
> +	return val;
> +}
> +
> +
>  static int yt8511_config_init(struct phy_device *phydev)
>  {
>  	int oldpage, ret = 0;
> @@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device
> *phydev)
>  	return phy_restore_page(phydev, oldpage, ret);
>  }
>  
> +static int ytphy_config_init(struct phy_device *phydev)
> +{
> +	struct device_node *of_node;
> +	u32 val;
> +	u32 mask;
> +	u32 cfg;
> +	int ret;
> +	int i = 0;

i initialized here as well as in for loop.

> +
> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {

to reduce the ident level, you may consider returning here by checking
if(!of_node)
	return -EINVAL;

> +		ret = of_property_read_u32(of_node,
> ytphy_rxden_grp[0].name, &cfg);
> +		if (!ret) {
> +			mask = ytphy_rxden_grp[0].mask;
> +			val = ytphy_read_ext(phydev,
> YTPHY_EXT_CHIP_CONFIG);
> +
> +			/* check the cfg overflow or not */
> +			cfg = cfg > mask >> (ffs(mask) - 1) ? mask :
> cfg;
> +
> +			val &= ~mask;
> +			val |= FIELD_PREP(mask, cfg);
> +			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG,
> val);
> +		}
> +
> +		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
> +		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
> +			ret = of_property_read_u32(of_node,
> ytphy_rxtxd_grp[i].name, &cfg);
> +			if (!ret) {
> +				mask = ytphy_rxtxd_grp[i].mask;
> +
> +				/* check the cfg overflow or not */
> +				cfg = cfg > mask >> (ffs(mask) - 1) ?
> mask : cfg;
> +
> +				val &= ~mask;
> +				val |= cfg << (ffs(mask) - 1);
> +			}
> +		}
> +		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1,
> val);
> +	}
> +
> +	phydev_err(phydev, "Get of node fail\n");
> +
> +	return -EINVAL;
> +}
> +
> +static void ytphy_link_change_notify(struct phy_device *phydev)
> +{
> +	u32 val;
> +	struct ytphy_priv_t *ytphy_priv = phydev->priv;

reverse christmas tree

> +
> +	if (phydev->speed < 0)
> +		return;
> +
> +	
> +static int yt8531_probe(struct phy_device *phydev)
> +{
> +	struct ytphy_priv_t *priv;
> +	const struct device_node *of_node;

Reverse christmas tree.

> +	u32 val;
> +	int ret;
> +
> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv),
> GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {
> +		ret = of_property_read_u32(of_node,
> ytphy_txinver_grp[0].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_1000 = val;
> +
> +		ret = of_property_read_u32(of_node,
> ytphy_txinver_grp[1].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_100 = val;
> +
> +		ret = of_property_read_u32(of_node,
> ytphy_txinver_grp[2].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_10 = val;
> +	}
> +	phydev->priv = priv;
> +
> +	return 0;
> +}
> +
>  

Re: [PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[Krzysztof Kozlowski]

On 16/12/2022 08:06, Yanhong Wang wrote:
> Some boards(such as StarFive VisionFive v2) require more than one value
> which defined by resets property, so the original definition can not
> meet the requirements. In order to adapt to different requirements,
> adjust the maxitems number from 1 to 3..
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml       | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index e26c3e76ebb7..7870228b4cd3 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -133,12 +133,19 @@ properties:
>          - ptp_ref
>  
>    resets:
> -    maxItems: 1
> -    description:
> -      MAC Reset signal.
> +    minItems: 1
> +    maxItems: 3
> +    additionalItems: true
> +    items:
> +      - description: MAC Reset signal
>  
>    reset-names:
> -    const: stmmaceth
> +    minItems: 1
> +    maxItems: 3
> +    additionalItems: true
> +    contains:
> +      enum:
> +        - stmmaceth

No, this is highly unspecific and you know affect all the schemas using
snps,dwmac.yaml. Both lists must be specific - for your device and for
others.

Best regards,
Krzysztof

Re: [PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[Krzysztof Kozlowski]

On 16/12/2022 08:06, Yanhong Wang wrote:
> Add documentation to describe StarFive dwmac driver(GMAC).
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/starfive,jh71x0-dwmac.yaml   | 103 ++++++++++++++++++
>  MAINTAINERS                                   |   5 +
>  3 files changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 7870228b4cd3..cdb045d1c618 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -91,6 +91,7 @@ properties:
>          - snps,dwmac-5.20
>          - snps,dwxgmac
>          - snps,dwxgmac-2.10
> +        - starfive,jh7110-dwmac
>  
>    reg:
>      minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> new file mode 100644
> index 000000000000..5cb1272fe959
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/starfive,jh71x0-dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH71x0 DWMAC glue layer
> +
> +maintainers:
> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - starfive,jh7110-dwmac
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - starfive,jh7110-dwmac

Is it going to grow with new models? If yes, when? If not, filename does
not match compatible.

> +      - const: snps,dwmac-5.20
> +
> +  clocks:
> +    items:
> +      - description: GMAC main clock
> +      - description: GMAC AHB clock
> +      - description: PTP clock
> +      - description: TX clock
> +      - description: GTXC clock
> +      - description: GTX clock
> +
> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: pclk
> +      - const: ptp_ref
> +      - const: tx
> +      - const: gtxc
> +      - const: gtx

missing resets and reset-names.

> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false
> +
Best regards,
Krzysztof

Re: [PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[Krzysztof Kozlowski]

On 16/12/2022 08:06, Yanhong Wang wrote:
> Add documentation to describe StarFive dwmac driver(GMAC).
> 

Subject: drop second, redundant "bindings for".

Best regards,
Krzysztof

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[Krzysztof Kozlowski]

On 16/12/2022 08:06, Yanhong Wang wrote:
> Add support for Motorcomm Technology YT8531 10/100/1000 Ethernet PHY.
> The document describe details of clock delay train configuration.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>

Missing vendor prefix documentation. I don't think you tested this at
all with checkpatch and dt_binding_check.

> ---
>  .../bindings/net/motorcomm,yt8531.yaml        | 111 ++++++++++++++++++
>  MAINTAINERS                                   |   1 +
>  2 files changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
> new file mode 100644
> index 000000000000..c5b8a09a78bb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
> @@ -0,0 +1,111 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/motorcomm,yt8531.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Motorcomm YT8531 Gigabit Ethernet PHY
> +
> +maintainers:
> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
> +

Why there is no reference to ethernet-phy.yaml?

> +select:
> +  properties:
> +    $nodename:
> +      pattern: "^ethernet-phy(@[a-f0-9]+)?$"

I don't think that's correct approach. You know affect all phys.

> +
> +  required:
> +    - $nodename
> +
> +properties:
> +  $nodename:
> +    pattern: "^ethernet-phy(@[a-f0-9]+)?$"

Just reference ethernet-phy.yaml.

> +
> +  reg:
> +    minimum: 0
> +    maximum: 31
> +    description:
> +      The ID number for the PHY.

Drop duplicated properties.

> +
> +  rxc_dly_en:

No underscores in node names. Missing vendor prefix. Both apply to all
your other custom properties, unless they are not custom but generic.

Missing ref.

> +    description: |
> +      RGMII Receive PHY Clock Delay defined with fixed 2ns.This is used for

After every full stop goes space.

> +      PHY that have configurable RX internal delays. If this property set
> +      to 1, then automatically add 2ns delay pad for Receive PHY clock.

Nope, this is wrong. You wrote now boolean property as enum.

> +    enum: [0, 1]
> +    default: 0
> +
> +  rx_delay_sel:
> +    description: |
> +      This is supplement to rxc_dly_en property,and it can
> +      be specified in 150ps(pico seconds) steps. The effective
> +      delay is: 150ps * N.

Nope. Use proper units and drop all this register stuff.

> +    minimum: 0
> +    maximum: 15
> +    default: 0
> +
> +  tx_delay_sel_fe:
> +    description: |
> +      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
> +      PHY's that have configurable TX internal delays when speed is 100Mbps
> +      or 10Mbps. It can be specified in 150ps steps, the effective delay
> +      is: 150ps * N.

The binding is in very poor shape. Please look carefully in
example-schema. All my previous comments apply everywhere.

> +    minimum: 0
> +    maximum: 15
> +    default: 15
> +
> +  tx_delay_sel:
> +    description: |
> +      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
> +      PHY's that have configurable TX internal delays when speed is 1000Mbps.
> +      It can be specified in 150ps steps, the effective delay is: 150ps * N.
> +    minimum: 0
> +    maximum: 15
> +    default: 1
> +
> +  tx_inverted_10:
> +    description: |
> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
> +      Transmit PHY Clock delay train configuration when speed is 10Mbps.
> +      0: original   1: inverted
> +    enum: [0, 1]
> +    default: 0
> +
> +  tx_inverted_100:
> +    description: |
> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
> +      Transmit PHY Clock delay train configuration when speed is 100Mbps.
> +      0: original   1: inverted
> +    enum: [0, 1]
> +    default: 0
> +
> +  tx_inverted_1000:
> +    description: |
> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
> +      Transmit PHY Clock delay train configuration when speed is 1000Mbps.
> +      0: original   1: inverted
> +    enum: [0, 1]
> +    default: 0
> +
> +required:
> +  - reg
> +
> +additionalProperties: true

This must be false. After referencing ethernet-phy this should be
unevaluatedProperties: false.


Best regards,
Krzysztof

Re: [PATCH v2 7/9] net: stmmac: Add glue layer for StarFive JH71x0 SoCs

[Krzysztof Kozlowski]

On 16/12/2022 08:06, Yanhong Wang wrote:
> This adds StarFive dwmac driver support on the StarFive JH71x0 SoCs.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>


> +
> +static const struct of_device_id starfive_eth_plat_match[] = {
> +	{
> +		.compatible = "starfive,jh7110-dwmac"
> +	},
> +	{
> +		.compatible = "starfive,jh7100-dwmac",

NAK.

This wasn't even checked with checkpatch.

Best regards,
Krzysztof

Re: [PATCH v2 6/9] net: phy: motorcomm: Add YT8531 phy support

[Heiner Kallweit]

On 16.12.2022 08:06, Yanhong Wang wrote:
> This adds basic support for the Motorcomm YT8531
> Gigabit Ethernet PHY.
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  drivers/net/phy/Kconfig     |   3 +-
>  drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index c57a0262fb64..86399254d9ff 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -258,9 +258,10 @@ config MICROSEMI_PHY
>  
>  config MOTORCOMM_PHY
>  	tristate "Motorcomm PHYs"
> +	default SOC_STARFIVE

Both are completely independent. This default should be removed.

>  	help
>  	  Enables support for Motorcomm network PHYs.
> -	  Currently supports the YT8511 gigabit PHY.
> +	  Currently supports the YT8511 and YT8531 gigabit PHYs.
>  

This doesn't apply. Parts of your patch exist already in net-next.
Support for YT8531S has been added in the meantime. Please rebase
your patch on net-next and annotate your patch as net-next.

>  config NATIONAL_PHY
>  	tristate "National Semiconductor PHYs"
> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
> index 7e6ac2c5e27e..bca03185b338 100644
> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -3,13 +3,17 @@
>   * Driver for Motorcomm PHYs
>   *
>   * Author: Peter Geis <pgwipeout@gmail.com>
> + *
>   */
>  
> +#include <linux/bitops.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/phy.h>
>  
>  #define PHY_ID_YT8511		0x0000010a
> +#define PHY_ID_YT8531		0x4f51e91b
>  
>  #define YT8511_PAGE_SELECT	0x1e
>  #define YT8511_PAGE		0x1f
> @@ -17,6 +21,10 @@
>  #define YT8511_EXT_DELAY_DRIVE	0x0d
>  #define YT8511_EXT_SLEEP_CTRL	0x27
>  
> +#define YTPHY_EXT_SMI_SDS_PHY		0xa000
> +#define YTPHY_EXT_CHIP_CONFIG		0xa001
> +#define YTPHY_EXT_RGMII_CONFIG1	0xa003
> +
>  /* 2b00 25m from pll
>   * 2b01 25m from xtl *default*
>   * 2b10 62.m from pll
> @@ -38,6 +46,51 @@
>  #define YT8511_DELAY_FE_TX_EN	(0xf << 12)
>  #define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
>  
> +struct ytphy_reg_field {
> +	char *name;
> +	u32 mask;
> +	u8	dflt;	/* Default value */
> +};
> +
> +struct ytphy_priv_t {
> +	u32 tx_inverted_1000;
> +	u32 tx_inverted_100;
> +	u32 tx_inverted_10;
> +};
> +
> +/* rx_delay_sel: RGMII rx clock delay train configuration, about 150ps per
> + *               step. Delay = 150ps * N
> + *
> + * tx_delay_sel_fe: RGMII tx clock delay train configuration when speed is
> + *                  100Mbps or 10Mbps, it's 150ps per step. Delay = 150ps * N
> + *
> + * tx_delay_sel: RGMII tx clock delay train configuration when speed is
> + *               1000Mbps, it's 150ps per step. Delay = 150ps * N
> + */
> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
> +	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
> +	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
> +	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
> +};
> +
> +/* tx_inverted_x: Use original or inverted RGMII TX_CLK to drive the RGMII
> + *                TX_CLK delay train configuration when speed is
> + *                xMbps(10/100/1000Mbps).
> + *                0: original,  1: inverted
> + */
> +static const struct ytphy_reg_field ytphy_txinver_grp[] = {
> +	{ "tx_inverted_1000", BIT(14), 0x0 },
> +	{ "tx_inverted_100", BIT(14), 0x0 },
> +	{ "tx_inverted_10", BIT(14), 0x0 }

Copy & Paste error that mask is the same for all entries?

> +};
> +
> +/* rxc_dly_en: RGMII clk 2ns delay control bit.
> + *             0: disable   1: enable
> + */
> +static const struct ytphy_reg_field ytphy_rxden_grp[] = {
> +	{ "rxc_dly_en", BIT(8), 0x1 }
> +};
> +
>  static int yt8511_read_page(struct phy_device *phydev)
>  {
>  	return __phy_read(phydev, YT8511_PAGE_SELECT);
> @@ -48,6 +101,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page)
>  	return __phy_write(phydev, YT8511_PAGE_SELECT, page);
>  };
>  
> +static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
> +{
> +	int ret;
> +	int val;
> +
> +	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
> +	if (ret < 0)
> +		return ret;
> +
> +	val = __phy_read(phydev, YT8511_PAGE);
> +
> +	return val;
> +}
> +
> +static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val)
> +{
> +	int ret;
> +
> +	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
> +	if (ret < 0)
> +		return ret;
> +
> +	ret = __phy_write(phydev, YT8511_PAGE, val);
> +
> +	return ret;
> +}
> +
>  static int yt8511_config_init(struct phy_device *phydev)
>  {
>  	int oldpage, ret = 0;
> @@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device *phydev)
>  	return phy_restore_page(phydev, oldpage, ret);
>  }
>  
> +static int ytphy_config_init(struct phy_device *phydev)
> +{
> +	struct device_node *of_node;
> +	u32 val;
> +	u32 mask;
> +	u32 cfg;
> +	int ret;
> +	int i = 0;
> +
> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {
> +		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
> +		if (!ret) {
> +			mask = ytphy_rxden_grp[0].mask;
> +			val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
> +
> +			/* check the cfg overflow or not */
> +			cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
> +
> +			val &= ~mask;
> +			val |= FIELD_PREP(mask, cfg);
> +			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);

This is the unlocked version. MDIO bus locking is missing.

> +		}
> +
> +		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
> +		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
> +			ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
> +			if (!ret) {
> +				mask = ytphy_rxtxd_grp[i].mask;
> +
> +				/* check the cfg overflow or not */
> +				cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
> +
> +				val &= ~mask;
> +				val |= cfg << (ffs(mask) - 1);
> +			}
> +		}
> +		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
> +	}
> +
> +	phydev_err(phydev, "Get of node fail\n");
> +

Please consider that the PHY may be used on non-DT systems.

> +	return -EINVAL;
> +}
> +
> +static void ytphy_link_change_notify(struct phy_device *phydev)
> +{
> +	u32 val;
> +	struct ytphy_priv_t *ytphy_priv = phydev->priv;
> +
> +	if (phydev->speed < 0)
> +		return;
> +
> +	val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
> +	switch (phydev->speed) {
> +	case SPEED_1000:
> +		val  &= ~ytphy_txinver_grp[0].mask;
> +		val |= FIELD_PREP(ytphy_txinver_grp[0].mask,
> +				ytphy_priv->tx_inverted_1000);
> +		break;
> +
> +	case SPEED_100:
> +		val  &= ~ytphy_txinver_grp[1].mask;
> +		val |= FIELD_PREP(ytphy_txinver_grp[1].mask,
> +				ytphy_priv->tx_inverted_100);
> +		break;
> +
> +	case SPEED_10:
> +		val  &= ~ytphy_txinver_grp[2].mask;
> +		val |= FIELD_PREP(ytphy_txinver_grp[2].mask,
> +				ytphy_priv->tx_inverted_10);
> +		break;
> +
> +	default:
> +		break;
> +	}
> +
> +	ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
> +}
> +
> +static int yt8531_probe(struct phy_device *phydev)
> +{
> +	struct ytphy_priv_t *priv;
> +	const struct device_node *of_node;
> +	u32 val;
> +	int ret;
> +
> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return -ENOMEM;
> +
> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {
> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_1000 = val;
> +
> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_100 = val;
> +
> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val);
> +		if (!ret)
> +			priv->tx_inverted_10 = val;
> +	}
> +	phydev->priv = priv;
> +
> +	return 0;
> +}
> +
>  static struct phy_driver motorcomm_phy_drvs[] = {
>  	{
>  		PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
> @@ -120,6 +310,17 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>  		.resume		= genphy_resume,
>  		.read_page	= yt8511_read_page,
>  		.write_page	= yt8511_write_page,
> +	}, {
> +		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
> +		.name		= "YT8531 Gigabit Ethernet",
> +		.probe		= yt8531_probe,
> +		.config_init	= ytphy_config_init,
> +		.read_status	= genphy_read_status,
> +		.suspend	= genphy_suspend,
> +		.resume		= genphy_resume,
> +		.read_page	= yt8511_read_page,
> +		.write_page	= yt8511_write_page,
> +		.link_change_notify = ytphy_link_change_notify,
>  	},
>  };
>  
> @@ -131,6 +332,7 @@ MODULE_LICENSE("GPL");
>  
>  static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
>  	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8511) },
> +	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
>  	{ /* sentinal */ }
>  };
>  

Re: [PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[yanhong wang]

On 2022/12/16 19:03, Krzysztof Kozlowski wrote:
> On 16/12/2022 08:06, Yanhong Wang wrote:
>> Some boards(such as StarFive VisionFive v2) require more than one value
>> which defined by resets property, so the original definition can not
>> meet the requirements. In order to adapt to different requirements,
>> adjust the maxitems number from 1 to 3..
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  .../devicetree/bindings/net/snps,dwmac.yaml       | 15 +++++++++++----
>>  1 file changed, 11 insertions(+), 4 deletions(-)
>> 
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index e26c3e76ebb7..7870228b4cd3 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -133,12 +133,19 @@ properties:
>>          - ptp_ref
>>  
>>    resets:
>> -    maxItems: 1
>> -    description:
>> -      MAC Reset signal.
>> +    minItems: 1
>> +    maxItems: 3
>> +    additionalItems: true
>> +    items:
>> +      - description: MAC Reset signal
>>  
>>    reset-names:
>> -    const: stmmaceth
>> +    minItems: 1
>> +    maxItems: 3
>> +    additionalItems: true
>> +    contains:
>> +      enum:
>> +        - stmmaceth
> 
> No, this is highly unspecific and you know affect all the schemas using
> snps,dwmac.yaml. Both lists must be specific - for your device and for
> others.
> 

I have tried to define the resets in "starfive,jh71x0-dwmac.yaml", but it can not over-write the maxItems limit in "snps,dwmac.yaml",therefore, it will report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check". Do you have any suggestions to deal with this situation?

> Best regards,
> Krzysztof
> 

Re: [PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[yanhong wang]

On 2022/12/16 19:05, Krzysztof Kozlowski wrote:
> On 16/12/2022 08:06, Yanhong Wang wrote:
>> Add documentation to describe StarFive dwmac driver(GMAC).
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>>  .../bindings/net/starfive,jh71x0-dwmac.yaml   | 103 ++++++++++++++++++
>>  MAINTAINERS                                   |   5 +
>>  3 files changed, 109 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> index 7870228b4cd3..cdb045d1c618 100644
>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>> @@ -91,6 +91,7 @@ properties:
>>          - snps,dwmac-5.20
>>          - snps,dwxgmac
>>          - snps,dwxgmac-2.10
>> +        - starfive,jh7110-dwmac
>>  
>>    reg:
>>      minItems: 1
>> diff --git a/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
>> new file mode 100644
>> index 000000000000..5cb1272fe959
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
>> @@ -0,0 +1,103 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/starfive,jh71x0-dwmac.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: StarFive JH71x0 DWMAC glue layer
>> +
>> +maintainers:
>> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
>> +
>> +select:
>> +  properties:
>> +    compatible:
>> +      contains:
>> +        enum:
>> +          - starfive,jh7110-dwmac
>> +  required:
>> +    - compatible
>> +
>> +allOf:
>> +  - $ref: snps,dwmac.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    items:
>> +      - enum:
>> +          - starfive,jh7110-dwmac
> 
> Is it going to grow with new models? If yes, when? If not, filename does
> not match compatible.

I will update the filename in the next version.

> 
>> +      - const: snps,dwmac-5.20
>> +
>> +  clocks:
>> +    items:
>> +      - description: GMAC main clock
>> +      - description: GMAC AHB clock
>> +      - description: PTP clock
>> +      - description: TX clock
>> +      - description: GTXC clock
>> +      - description: GTX clock
>> +
>> +  clock-names:
>> +    items:
>> +      - const: stmmaceth
>> +      - const: pclk
>> +      - const: ptp_ref
>> +      - const: tx
>> +      - const: gtxc
>> +      - const: gtx
> 
> missing resets and reset-names.
> 

I will add resets and reset-names in the next version.

>> +
>> +required:
>> +  - compatible
>> +  - clocks
>> +  - clock-names
>> +  - resets
>> +  - reset-names
>> +
>> +unevaluatedProperties: false
>> +
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[yanhong wang]

On 2022/12/16 19:06, Krzysztof Kozlowski wrote:
> On 16/12/2022 08:06, Yanhong Wang wrote:
>> Add documentation to describe StarFive dwmac driver(GMAC).
>> 
> 
> Subject: drop second, redundant "bindings for".
> 

Thanks,  i will fix.

> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[Krzysztof Kozlowski]

On 20/12/2022 07:48, yanhong wang wrote:

>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> index e26c3e76ebb7..7870228b4cd3 100644
>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>> @@ -133,12 +133,19 @@ properties:
>>>          - ptp_ref
>>>  
>>>    resets:
>>> -    maxItems: 1
>>> -    description:
>>> -      MAC Reset signal.
>>> +    minItems: 1
>>> +    maxItems: 3
>>> +    additionalItems: true
>>> +    items:
>>> +      - description: MAC Reset signal
>>>  
>>>    reset-names:
>>> -    const: stmmaceth
>>> +    minItems: 1
>>> +    maxItems: 3
>>> +    additionalItems: true
>>> +    contains:
>>> +      enum:
>>> +        - stmmaceth
>>
>> No, this is highly unspecific and you know affect all the schemas using
>> snps,dwmac.yaml. Both lists must be specific - for your device and for
>> others.
>>
> 
> I have tried to define the resets in "starfive,jh71x0-dwmac.yaml", but it can not over-write the maxItems limit in "snps,dwmac.yaml",therefore, it will report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check". Do you have any suggestions to deal with this situation?

The solution is not to affect all schemas with allowing anything as reset.

If you need more items for your case, you can change snps,dwmac.yaml and
add constraints in allOf:if:then: allowing it only for your compatible.
There are plenty of examples how this is done, e.g.:

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

> 
>> Best regards,
>> Krzysztof
>>

Best regards,
Krzysztof

Re: [PATCH v2 6/9] net: phy: motorcomm: Add YT8531 phy support

[Andrew Lunn]

> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  drivers/net/phy/Kconfig     |   3 +-
>  drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 204 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index c57a0262fb64..86399254d9ff 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -258,9 +258,10 @@ config MICROSEMI_PHY
>  
>  config MOTORCOMM_PHY
>  	tristate "Motorcomm PHYs"
> +	default SOC_STARFIVE

Please don't do this. Look at the other PHY drivers. How many have a
default like this?

Generally, if you are doing something which no other driver does, you
are doing something wrong.

> --- a/drivers/net/phy/motorcomm.c
> +++ b/drivers/net/phy/motorcomm.c
> @@ -3,13 +3,17 @@
>   * Driver for Motorcomm PHYs
>   *
>   * Author: Peter Geis <pgwipeout@gmail.com>
> + *
>   */

Please avoid changes like this. It distracts from the real code
changes.

>  
> +#include <linux/bitops.h>
>  #include <linux/kernel.h>
>  #include <linux/module.h>
> +#include <linux/of.h>
>  #include <linux/phy.h>
>  
>  #define PHY_ID_YT8511		0x0000010a
> +#define PHY_ID_YT8531		0x4f51e91b
>  
>  #define YT8511_PAGE_SELECT	0x1e
>  #define YT8511_PAGE		0x1f
> @@ -17,6 +21,10 @@
>  #define YT8511_EXT_DELAY_DRIVE	0x0d
>  #define YT8511_EXT_SLEEP_CTRL	0x27
>  
> +#define YTPHY_EXT_SMI_SDS_PHY		0xa000
> +#define YTPHY_EXT_CHIP_CONFIG		0xa001
> +#define YTPHY_EXT_RGMII_CONFIG1	0xa003
> +
>  /* 2b00 25m from pll
>   * 2b01 25m from xtl *default*
>   * 2b10 62.m from pll
> @@ -38,6 +46,51 @@
>  #define YT8511_DELAY_FE_TX_EN	(0xf << 12)
>  #define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
>  
> +struct ytphy_reg_field {
> +	char *name;
> +	u32 mask;
> +	u8	dflt;	/* Default value */
> +};

This should be next to where it is used. But once you have delay in
ps, i suspect you will throw this away.

>  
> +static int ytphy_config_init(struct phy_device *phydev)

Is this specific to the 8531? If so, put 8531 in the function name?

Do any of these delay configurations also apply to the 8511?

> +{
> +	struct device_node *of_node;
> +	u32 val;
> +	u32 mask;
> +	u32 cfg;
> +	int ret;
> +	int i = 0;

Reverse Christmas tree. i needs to be earlier, val needs to be later.

> +	of_node = phydev->mdio.dev.of_node;
> +	if (of_node) {
> +		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
> +		if (!ret) {
> +			mask = ytphy_rxden_grp[0].mask;
> +			val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
> +
> +			/* check the cfg overflow or not */
> +			cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
> +
> +			val &= ~mask;
> +			val |= FIELD_PREP(mask, cfg);
> +			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);
> +		}
> +
> +		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
> +		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
> +			ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
> +			if (!ret) {
> +				mask = ytphy_rxtxd_grp[i].mask;
> +
> +				/* check the cfg overflow or not */
> +				cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
> +
> +				val &= ~mask;
> +				val |= cfg << (ffs(mask) - 1);
> +			}
> +		}
> +		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
> +	}
> +
> +	phydev_err(phydev, "Get of node fail\n");

What about the case of the PHY is used on a board without device tree?
ACPI could be used, or there could be nothing because it is on a USB
dongle etc.

You should of defined in your device tree binding what the defaults
are when properties are missing. So apply them when there is no DT
available. Then we at least have the PHY in a known state.

       Andrew

Re: [PATCH v2 4/9] dt-bindings: net: Add bindings for StarFive dwmac

[Rob Herring]

On Fri, Dec 16, 2022 at 03:06:27PM +0800, Yanhong Wang wrote:
> Add documentation to describe StarFive dwmac driver(GMAC).
> 
> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> ---
>  .../devicetree/bindings/net/snps,dwmac.yaml   |   1 +
>  .../bindings/net/starfive,jh71x0-dwmac.yaml   | 103 ++++++++++++++++++
>  MAINTAINERS                                   |   5 +
>  3 files changed, 109 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 7870228b4cd3..cdb045d1c618 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -91,6 +91,7 @@ properties:
>          - snps,dwmac-5.20
>          - snps,dwxgmac
>          - snps,dwxgmac-2.10
> +        - starfive,jh7110-dwmac
>  
>    reg:
>      minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> new file mode 100644
> index 000000000000..5cb1272fe959
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> @@ -0,0 +1,103 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2022 StarFive Technology Co., Ltd.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/starfive,jh71x0-dwmac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: StarFive JH71x0 DWMAC glue layer
> +
> +maintainers:
> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
> +
> +select:
> +  properties:
> +    compatible:
> +      contains:
> +        enum:
> +          - starfive,jh7110-dwmac
> +  required:
> +    - compatible
> +
> +allOf:
> +  - $ref: snps,dwmac.yaml#
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - starfive,jh7110-dwmac
> +      - const: snps,dwmac-5.20
> +
> +  clocks:
> +    items:
> +      - description: GMAC main clock
> +      - description: GMAC AHB clock
> +      - description: PTP clock
> +      - description: TX clock
> +      - description: GTXC clock
> +      - description: GTX clock
> +
> +  clock-names:
> +    items:
> +      - const: stmmaceth
> +      - const: pclk
> +      - const: ptp_ref
> +      - const: tx
> +      - const: gtxc
> +      - const: gtx
> +
> +required:
> +  - compatible
> +  - clocks
> +  - clock-names
> +  - resets
> +  - reset-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    stmmac_axi_setup: stmmac-axi-config {

The schema says put this as a child node of ethernet@16030000.

> +        snps,lpi_en;
> +        snps,wr_osr_lmt = <4>;
> +        snps,rd_osr_lmt = <4>;
> +        snps,blen = <256 128 64 32 0 0 0>;
> +    };
> +
> +    gmac0: ethernet@16030000 {

Drop unused labels.

> +        compatible = "starfive,jh7110-dwmac", "snps,dwmac-5.20";
> +        reg = <0x16030000 0x10000>;
> +        clocks = <&clk 3>, <&clk 2>, <&clk 109>,
> +                 <&clk 5>, <&clk 111>, <&clk 108>;
> +        clock-names = "stmmaceth", "pclk", "ptp_ref",
> +                      "tx", "gtxc", "gtx";
> +        resets = <&rst 1>, <&rst 2>;
> +        reset-names = "stmmaceth", "ahb";
> +        interrupts = <7>, <6>, <5>;
> +        interrupt-names = "macirq", "eth_wake_irq", "eth_lpi";
> +        phy-mode = "rgmii-id";
> +        snps,multicast-filter-bins = <64>;
> +        snps,perfect-filter-entries = <8>;
> +        rx-fifo-depth = <2048>;
> +        tx-fifo-depth = <2048>;
> +        snps,fixed-burst;
> +        snps,no-pbl-x8;
> +        snps,tso;
> +        snps,force_thresh_dma_mode;
> +        snps,axi-config = <&stmmac_axi_setup>;
> +        snps,en-tx-lpi-clockgating;
> +        snps,txpbl = <16>;
> +        snps,rxpbl = <16>;
> +        phy-handle = <&phy0>;
> +
> +        mdio {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "snps,dwmac-mdio";
> +
> +            phy0: ethernet-phy@0 {
> +                reg = <0>;
> +            };
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a70c1d0f303e..166b0009f63c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19606,6 +19606,11 @@ F:	Documentation/devicetree/bindings/clock/starfive*
>  F:	drivers/clk/starfive/
>  F:	include/dt-bindings/clock/starfive*
>  
> +STARFIVE DWMAC GLUE LAYER
> +M:	Yanhong Wang <yanhong.wang@starfivetech.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/net/starfive,jh71x0-dwmac.yaml
> +
>  STARFIVE PINCTRL DRIVER
>  M:	Emil Renner Berthing <kernel@esmil.dk>
>  M:	Jianlong Huang <jianlong.huang@starfivetech.com>
> -- 
> 2.17.1
> 
> 

Re: [PATCH v2 6/9] net: phy: motorcomm: Add YT8531 phy support

[yanhong wang]

On 2022/12/16 19:58, Heiner Kallweit wrote:
> On 16.12.2022 08:06, Yanhong Wang wrote:
>> This adds basic support for the Motorcomm YT8531
>> Gigabit Ethernet PHY.
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
>> ---
>>  drivers/net/phy/Kconfig     |   3 +-
>>  drivers/net/phy/motorcomm.c | 202 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 204 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
>> index c57a0262fb64..86399254d9ff 100644
>> --- a/drivers/net/phy/Kconfig
>> +++ b/drivers/net/phy/Kconfig
>> @@ -258,9 +258,10 @@ config MICROSEMI_PHY
>>  
>>  config MOTORCOMM_PHY
>>  	tristate "Motorcomm PHYs"
>> +	default SOC_STARFIVE
> 
> Both are completely independent. This default should be removed.
> 
>>  	help
>>  	  Enables support for Motorcomm network PHYs.
>> -	  Currently supports the YT8511 gigabit PHY.
>> +	  Currently supports the YT8511 and YT8531 gigabit PHYs.
>>  
> 
> This doesn't apply. Parts of your patch exist already in net-next.
> Support for YT8531S has been added in the meantime. Please rebase
> your patch on net-next and annotate your patch as net-next.
> 

Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
i will remove the parts of YT8531 in the next version.

>>  config NATIONAL_PHY
>>  	tristate "National Semiconductor PHYs"
>> diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
>> index 7e6ac2c5e27e..bca03185b338 100644
>> --- a/drivers/net/phy/motorcomm.c
>> +++ b/drivers/net/phy/motorcomm.c
>> @@ -3,13 +3,17 @@
>>   * Driver for Motorcomm PHYs
>>   *
>>   * Author: Peter Geis <pgwipeout@gmail.com>
>> + *
>>   */
>>  
>> +#include <linux/bitops.h>
>>  #include <linux/kernel.h>
>>  #include <linux/module.h>
>> +#include <linux/of.h>
>>  #include <linux/phy.h>
>>  
>>  #define PHY_ID_YT8511		0x0000010a
>> +#define PHY_ID_YT8531		0x4f51e91b
>>  
>>  #define YT8511_PAGE_SELECT	0x1e
>>  #define YT8511_PAGE		0x1f
>> @@ -17,6 +21,10 @@
>>  #define YT8511_EXT_DELAY_DRIVE	0x0d
>>  #define YT8511_EXT_SLEEP_CTRL	0x27
>>  
>> +#define YTPHY_EXT_SMI_SDS_PHY		0xa000
>> +#define YTPHY_EXT_CHIP_CONFIG		0xa001
>> +#define YTPHY_EXT_RGMII_CONFIG1	0xa003
>> +
>>  /* 2b00 25m from pll
>>   * 2b01 25m from xtl *default*
>>   * 2b10 62.m from pll
>> @@ -38,6 +46,51 @@
>>  #define YT8511_DELAY_FE_TX_EN	(0xf << 12)
>>  #define YT8511_DELAY_FE_TX_DIS	(0x2 << 12)
>>  
>> +struct ytphy_reg_field {
>> +	char *name;
>> +	u32 mask;
>> +	u8	dflt;	/* Default value */
>> +};
>> +
>> +struct ytphy_priv_t {
>> +	u32 tx_inverted_1000;
>> +	u32 tx_inverted_100;
>> +	u32 tx_inverted_10;
>> +};
>> +
>> +/* rx_delay_sel: RGMII rx clock delay train configuration, about 150ps per
>> + *               step. Delay = 150ps * N
>> + *
>> + * tx_delay_sel_fe: RGMII tx clock delay train configuration when speed is
>> + *                  100Mbps or 10Mbps, it's 150ps per step. Delay = 150ps * N
>> + *
>> + * tx_delay_sel: RGMII tx clock delay train configuration when speed is
>> + *               1000Mbps, it's 150ps per step. Delay = 150ps * N
>> + */
>> +static const struct ytphy_reg_field ytphy_rxtxd_grp[] = {
>> +	{ "rx_delay_sel", GENMASK(13, 10), 0x0 },
>> +	{ "tx_delay_sel_fe", GENMASK(7, 4), 0xf },
>> +	{ "tx_delay_sel", GENMASK(3, 0), 0x1 }
>> +};
>> +
>> +/* tx_inverted_x: Use original or inverted RGMII TX_CLK to drive the RGMII
>> + *                TX_CLK delay train configuration when speed is
>> + *                xMbps(10/100/1000Mbps).
>> + *                0: original,  1: inverted
>> + */
>> +static const struct ytphy_reg_field ytphy_txinver_grp[] = {
>> +	{ "tx_inverted_1000", BIT(14), 0x0 },
>> +	{ "tx_inverted_100", BIT(14), 0x0 },
>> +	{ "tx_inverted_10", BIT(14), 0x0 }
> 
> Copy & Paste error that mask is the same for all entries?
> 
>> +};
>> +
>> +/* rxc_dly_en: RGMII clk 2ns delay control bit.
>> + *             0: disable   1: enable
>> + */
>> +static const struct ytphy_reg_field ytphy_rxden_grp[] = {
>> +	{ "rxc_dly_en", BIT(8), 0x1 }
>> +};
>> +
>>  static int yt8511_read_page(struct phy_device *phydev)
>>  {
>>  	return __phy_read(phydev, YT8511_PAGE_SELECT);
>> @@ -48,6 +101,33 @@ static int yt8511_write_page(struct phy_device *phydev, int page)
>>  	return __phy_write(phydev, YT8511_PAGE_SELECT, page);
>>  };
>>  
>> +static int ytphy_read_ext(struct phy_device *phydev, u32 regnum)
>> +{
>> +	int ret;
>> +	int val;
>> +
>> +	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	val = __phy_read(phydev, YT8511_PAGE);
>> +
>> +	return val;
>> +}
>> +
>> +static int ytphy_write_ext(struct phy_device *phydev, u32 regnum, u16 val)
>> +{
>> +	int ret;
>> +
>> +	ret = __phy_write(phydev, YT8511_PAGE_SELECT, regnum);
>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	ret = __phy_write(phydev, YT8511_PAGE, val);
>> +
>> +	return ret;
>> +}
>> +
>>  static int yt8511_config_init(struct phy_device *phydev)
>>  {
>>  	int oldpage, ret = 0;
>> @@ -111,6 +191,116 @@ static int yt8511_config_init(struct phy_device *phydev)
>>  	return phy_restore_page(phydev, oldpage, ret);
>>  }
>>  
>> +static int ytphy_config_init(struct phy_device *phydev)
>> +{
>> +	struct device_node *of_node;
>> +	u32 val;
>> +	u32 mask;
>> +	u32 cfg;
>> +	int ret;
>> +	int i = 0;
>> +
>> +	of_node = phydev->mdio.dev.of_node;
>> +	if (of_node) {
>> +		ret = of_property_read_u32(of_node, ytphy_rxden_grp[0].name, &cfg);
>> +		if (!ret) {
>> +			mask = ytphy_rxden_grp[0].mask;
>> +			val = ytphy_read_ext(phydev, YTPHY_EXT_CHIP_CONFIG);
>> +
>> +			/* check the cfg overflow or not */
>> +			cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
>> +
>> +			val &= ~mask;
>> +			val |= FIELD_PREP(mask, cfg);
>> +			ytphy_write_ext(phydev, YTPHY_EXT_CHIP_CONFIG, val);
> 
> This is the unlocked version. MDIO bus locking is missing.
> 
>> +		}
>> +
>> +		val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
>> +		for (i = 0; i < ARRAY_SIZE(ytphy_rxtxd_grp); i++) {
>> +			ret = of_property_read_u32(of_node, ytphy_rxtxd_grp[i].name, &cfg);
>> +			if (!ret) {
>> +				mask = ytphy_rxtxd_grp[i].mask;
>> +
>> +				/* check the cfg overflow or not */
>> +				cfg = cfg > mask >> (ffs(mask) - 1) ? mask : cfg;
>> +
>> +				val &= ~mask;
>> +				val |= cfg << (ffs(mask) - 1);
>> +			}
>> +		}
>> +		return ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
>> +	}
>> +
>> +	phydev_err(phydev, "Get of node fail\n");
>> +
> 
> Please consider that the PHY may be used on non-DT systems.
> 
>> +	return -EINVAL;
>> +}
>> +
>> +static void ytphy_link_change_notify(struct phy_device *phydev)
>> +{
>> +	u32 val;
>> +	struct ytphy_priv_t *ytphy_priv = phydev->priv;
>> +
>> +	if (phydev->speed < 0)
>> +		return;
>> +
>> +	val = ytphy_read_ext(phydev, YTPHY_EXT_RGMII_CONFIG1);
>> +	switch (phydev->speed) {
>> +	case SPEED_1000:
>> +		val  &= ~ytphy_txinver_grp[0].mask;
>> +		val |= FIELD_PREP(ytphy_txinver_grp[0].mask,
>> +				ytphy_priv->tx_inverted_1000);
>> +		break;
>> +
>> +	case SPEED_100:
>> +		val  &= ~ytphy_txinver_grp[1].mask;
>> +		val |= FIELD_PREP(ytphy_txinver_grp[1].mask,
>> +				ytphy_priv->tx_inverted_100);
>> +		break;
>> +
>> +	case SPEED_10:
>> +		val  &= ~ytphy_txinver_grp[2].mask;
>> +		val |= FIELD_PREP(ytphy_txinver_grp[2].mask,
>> +				ytphy_priv->tx_inverted_10);
>> +		break;
>> +
>> +	default:
>> +		break;
>> +	}
>> +
>> +	ytphy_write_ext(phydev, YTPHY_EXT_RGMII_CONFIG1, val);
>> +}
>> +
>> +static int yt8531_probe(struct phy_device *phydev)
>> +{
>> +	struct ytphy_priv_t *priv;
>> +	const struct device_node *of_node;
>> +	u32 val;
>> +	int ret;
>> +
>> +	priv = devm_kzalloc(&phydev->mdio.dev, sizeof(*priv), GFP_KERNEL);
>> +	if (!priv)
>> +		return -ENOMEM;
>> +
>> +	of_node = phydev->mdio.dev.of_node;
>> +	if (of_node) {
>> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[0].name, &val);
>> +		if (!ret)
>> +			priv->tx_inverted_1000 = val;
>> +
>> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[1].name, &val);
>> +		if (!ret)
>> +			priv->tx_inverted_100 = val;
>> +
>> +		ret = of_property_read_u32(of_node, ytphy_txinver_grp[2].name, &val);
>> +		if (!ret)
>> +			priv->tx_inverted_10 = val;
>> +	}
>> +	phydev->priv = priv;
>> +
>> +	return 0;
>> +}
>> +
>>  static struct phy_driver motorcomm_phy_drvs[] = {
>>  	{
>>  		PHY_ID_MATCH_EXACT(PHY_ID_YT8511),
>> @@ -120,6 +310,17 @@ static struct phy_driver motorcomm_phy_drvs[] = {
>>  		.resume		= genphy_resume,
>>  		.read_page	= yt8511_read_page,
>>  		.write_page	= yt8511_write_page,
>> +	}, {
>> +		PHY_ID_MATCH_EXACT(PHY_ID_YT8531),
>> +		.name		= "YT8531 Gigabit Ethernet",
>> +		.probe		= yt8531_probe,
>> +		.config_init	= ytphy_config_init,
>> +		.read_status	= genphy_read_status,
>> +		.suspend	= genphy_suspend,
>> +		.resume		= genphy_resume,
>> +		.read_page	= yt8511_read_page,
>> +		.write_page	= yt8511_write_page,
>> +		.link_change_notify = ytphy_link_change_notify,
>>  	},
>>  };
>>  
>> @@ -131,6 +332,7 @@ MODULE_LICENSE("GPL");
>>  
>>  static const struct mdio_device_id __maybe_unused motorcomm_tbl[] = {
>>  	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8511) },
>> +	{ PHY_ID_MATCH_EXACT(PHY_ID_YT8531) },
>>  	{ /* sentinal */ }
>>  };
>>  
> 

Re: [PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[yanhong wang]

On 2022/12/20 17:21, Krzysztof Kozlowski wrote:
> On 20/12/2022 07:48, yanhong wang wrote:
> 
>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> index e26c3e76ebb7..7870228b4cd3 100644
>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>> @@ -133,12 +133,19 @@ properties:
>>>>          - ptp_ref
>>>>  
>>>>    resets:
>>>> -    maxItems: 1
>>>> -    description:
>>>> -      MAC Reset signal.
>>>> +    minItems: 1
>>>> +    maxItems: 3
>>>> +    additionalItems: true
>>>> +    items:
>>>> +      - description: MAC Reset signal
>>>>  
>>>>    reset-names:
>>>> -    const: stmmaceth
>>>> +    minItems: 1
>>>> +    maxItems: 3
>>>> +    additionalItems: true
>>>> +    contains:
>>>> +      enum:
>>>> +        - stmmaceth
>>>
>>> No, this is highly unspecific and you know affect all the schemas using
>>> snps,dwmac.yaml. Both lists must be specific - for your device and for
>>> others.
>>>
>> 
>> I have tried to define the resets in "starfive,jh71x0-dwmac.yaml", but it can not over-write the maxItems limit in "snps,dwmac.yaml",therefore, it will report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check". Do you have any suggestions to deal with this situation?
> 
> The solution is not to affect all schemas with allowing anything as reset.
> 
> If you need more items for your case, you can change snps,dwmac.yaml and
> add constraints in allOf:if:then: allowing it only for your compatible.
> There are plenty of examples how this is done, e.g.:
> 
> https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
> 

Thanks. Refer to the definition in the example and update the definition as follows:

snps,dwmac.yaml[Partial Content]:

properties:
  resets:
    maxItems: 1
    description:
      MAC Reset signal.

  reset-names:
    const: stmmaceth

allOf:
  - if:
      properties:
        compatible:
          contains:
            const: starfive,jh7110-dwmac

    then:
      properties:
        resets:
          minItems: 2
          maxItems: 2
        reset-names:
          items:
            - const: stmmaceth
            - const: ahb
      required:
        - resets
        - reset-names


starfive,jh7110-dwmac.yaml[Partial Content]:

properties:
  resets:
    items:
      - description: MAC Reset signal.
      - description: AHB Reset signal.

  reset-names:
    items:
      - const: stmmaceth
      - const: ahb

allOf:
  - $ref: snps,dwmac.yaml#

It will also report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check" with 'starfive,jh7110-dwmac.yaml'. Do you have any better suggestions to solve this problem?

>> 
>>> Best regards,
>>> Krzysztof
>>>
> 
> Best regards,
> Krzysztof
> 

Re: [PATCH v2 2/9] dt-bindings: net: snps,dwmac: Update the maxitems number of resets and reset-names

[Krzysztof Kozlowski]

On 27/12/2022 08:48, yanhong wang wrote:
> 
> 
> On 2022/12/20 17:21, Krzysztof Kozlowski wrote:
>> On 20/12/2022 07:48, yanhong wang wrote:
>>
>>>>> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> index e26c3e76ebb7..7870228b4cd3 100644
>>>>> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
>>>>> @@ -133,12 +133,19 @@ properties:
>>>>>          - ptp_ref
>>>>>  
>>>>>    resets:
>>>>> -    maxItems: 1
>>>>> -    description:
>>>>> -      MAC Reset signal.
>>>>> +    minItems: 1
>>>>> +    maxItems: 3
>>>>> +    additionalItems: true
>>>>> +    items:
>>>>> +      - description: MAC Reset signal
>>>>>  
>>>>>    reset-names:
>>>>> -    const: stmmaceth
>>>>> +    minItems: 1
>>>>> +    maxItems: 3
>>>>> +    additionalItems: true
>>>>> +    contains:
>>>>> +      enum:
>>>>> +        - stmmaceth
>>>>
>>>> No, this is highly unspecific and you know affect all the schemas using
>>>> snps,dwmac.yaml. Both lists must be specific - for your device and for
>>>> others.
>>>>
>>>
>>> I have tried to define the resets in "starfive,jh71x0-dwmac.yaml", but it can not over-write the maxItems limit in "snps,dwmac.yaml",therefore, it will report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check". Do you have any suggestions to deal with this situation?
>>
>> The solution is not to affect all schemas with allowing anything as reset.
>>
>> If you need more items for your case, you can change snps,dwmac.yaml and
>> add constraints in allOf:if:then: allowing it only for your compatible.
>> There are plenty of examples how this is done, e.g.:
>>
>> https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57
>>
> 
> Thanks. Refer to the definition in the example and update the definition as follows:
> 
> snps,dwmac.yaml[Partial Content]:
> 
> properties:
>   resets:
>     maxItems: 1
>     description:
>       MAC Reset signal.
> 
>   reset-names:
>     const: stmmaceth
> 
> allOf:
>   - if:
>       properties:
>         compatible:
>           contains:
>             const: starfive,jh7110-dwmac
> 
>     then:
>       properties:
>         resets:
>           minItems: 2
>           maxItems: 2
>         reset-names:
>           items:
>             - const: stmmaceth
>             - const: ahb
>       required:
>         - resets
>         - reset-names
> 
> 
> starfive,jh7110-dwmac.yaml[Partial Content]:
> 
> properties:
>   resets:
>     items:
>       - description: MAC Reset signal.
>       - description: AHB Reset signal.
> 
>   reset-names:
>     items:
>       - const: stmmaceth
>       - const: ahb
> 
> allOf:
>   - $ref: snps,dwmac.yaml#
> 
> It will also report error "reset-names: ['stmmaceth', 'ahb'] is too long"  running "make dt_binding_check" with 'starfive,jh7110-dwmac.yaml'. Do you have any better suggestions to solve this problem?

Because it is not correct. The top-level properties must have the widest
constraints which in allOf:if:then:else you are further narrowing for
all of variants. ALL.

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

Best regards,
Krzysztof

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[yanhong wang]

On 2022/12/16 19:15, Krzysztof Kozlowski wrote:
> On 16/12/2022 08:06, Yanhong Wang wrote:
>> Add support for Motorcomm Technology YT8531 10/100/1000 Ethernet PHY.
>> The document describe details of clock delay train configuration.
>> 
>> Signed-off-by: Yanhong Wang <yanhong.wang@starfivetech.com>
> 
> Missing vendor prefix documentation. I don't think you tested this at
> all with checkpatch and dt_binding_check.
> 
>> ---
>>  .../bindings/net/motorcomm,yt8531.yaml        | 111 ++++++++++++++++++
>>  MAINTAINERS                                   |   1 +
>>  2 files changed, 112 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
>> 
>> diff --git a/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
>> new file mode 100644
>> index 000000000000..c5b8a09a78bb
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/motorcomm,yt8531.yaml
>> @@ -0,0 +1,111 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/motorcomm,yt8531.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Motorcomm YT8531 Gigabit Ethernet PHY
>> +
>> +maintainers:
>> +  - Yanhong Wang <yanhong.wang@starfivetech.com>
>> +
> 
> Why there is no reference to ethernet-phy.yaml?
> 
>> +select:
>> +  properties:
>> +    $nodename:
>> +      pattern: "^ethernet-phy(@[a-f0-9]+)?$"
> 
> I don't think that's correct approach. You know affect all phys.
> 
>> +
>> +  required:
>> +    - $nodename
>> +
>> +properties:
>> +  $nodename:
>> +    pattern: "^ethernet-phy(@[a-f0-9]+)?$"
> 
> Just reference ethernet-phy.yaml.
> 
>> +
>> +  reg:
>> +    minimum: 0
>> +    maximum: 31
>> +    description:
>> +      The ID number for the PHY.
> 
> Drop duplicated properties.
> 
>> +
>> +  rxc_dly_en:
> 
> No underscores in node names. Missing vendor prefix. Both apply to all
> your other custom properties, unless they are not custom but generic.
> 
> Missing ref.
> 
>> +    description: |
>> +      RGMII Receive PHY Clock Delay defined with fixed 2ns.This is used for
> 
> After every full stop goes space.
> 
>> +      PHY that have configurable RX internal delays. If this property set
>> +      to 1, then automatically add 2ns delay pad for Receive PHY clock.
> 
> Nope, this is wrong. You wrote now boolean property as enum.
> 
>> +    enum: [0, 1]
>> +    default: 0
>> +
>> +  rx_delay_sel:
>> +    description: |
>> +      This is supplement to rxc_dly_en property,and it can
>> +      be specified in 150ps(pico seconds) steps. The effective
>> +      delay is: 150ps * N.
> 
> Nope. Use proper units and drop all this register stuff.
> 
>> +    minimum: 0
>> +    maximum: 15
>> +    default: 0
>> +
>> +  tx_delay_sel_fe:
>> +    description: |
>> +      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
>> +      PHY's that have configurable TX internal delays when speed is 100Mbps
>> +      or 10Mbps. It can be specified in 150ps steps, the effective delay
>> +      is: 150ps * N.
> 
> The binding is in very poor shape. Please look carefully in
> example-schema. All my previous comments apply everywhere.
> 
>> +    minimum: 0
>> +    maximum: 15
>> +    default: 15
>> +
>> +  tx_delay_sel:
>> +    description: |
>> +      RGMII Transmit PHY Clock Delay defined in pico seconds.This is used for
>> +      PHY's that have configurable TX internal delays when speed is 1000Mbps.
>> +      It can be specified in 150ps steps, the effective delay is: 150ps * N.
>> +    minimum: 0
>> +    maximum: 15
>> +    default: 1
>> +
>> +  tx_inverted_10:
>> +    description: |
>> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
>> +      Transmit PHY Clock delay train configuration when speed is 10Mbps.
>> +      0: original   1: inverted
>> +    enum: [0, 1]
>> +    default: 0
>> +
>> +  tx_inverted_100:
>> +    description: |
>> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
>> +      Transmit PHY Clock delay train configuration when speed is 100Mbps.
>> +      0: original   1: inverted
>> +    enum: [0, 1]
>> +    default: 0
>> +
>> +  tx_inverted_1000:
>> +    description: |
>> +      Use original or inverted RGMII Transmit PHY Clock to drive the RGMII
>> +      Transmit PHY Clock delay train configuration when speed is 1000Mbps.
>> +      0: original   1: inverted
>> +    enum: [0, 1]
>> +    default: 0
>> +
>> +required:
>> +  - reg
>> +
>> +additionalProperties: true
> 
> This must be false. After referencing ethernet-phy this should be
> unevaluatedProperties: false.
> 
> 

Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
i will remove the parts of YT8531 in the next version.

> Best regards,
> Krzysztof
> 

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[Krzysztof Kozlowski]

On 27/12/2022 10:38, yanhong wang wrote:
>>
>> This must be false. After referencing ethernet-phy this should be
>> unevaluatedProperties: false.
>>
>>
> 
> Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
> i will remove the parts of YT8531 in the next version.

I don't understand what does it mean. You sent duplicated patch? If so,
please do not... you waste reviewers time.

Anyway this entire patch does not meet criteria for submission at all,
so please start over from example-schema.

Best regards,
Krzysztof

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[yanhong wang]

On 2022/12/27 17:52, Krzysztof Kozlowski wrote:
> On 27/12/2022 10:38, yanhong wang wrote:
>>>
>>> This must be false. After referencing ethernet-phy this should be
>>> unevaluatedProperties: false.
>>>
>>>
>> 
>> Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
>> i will remove the parts of YT8531 in the next version.
> 
> I don't understand what does it mean. You sent duplicated patch? If so,
> please do not... you waste reviewers time.
> 
> Anyway this entire patch does not meet criteria for submission at all,
> so please start over from example-schema.
> 

Sorry, maybe I didn't make it clear, which led to misunderstanding. Motorcomm Inc is also 
carrying out the upstream of YT8531, and my patch will be duplicated and conflicted 
with their submission. By communicating with the developers of Motorcomm Inc, the part 
of YT8531 will be submitted by Motorcomm Inc, so my submission about YT8531 will be withdrawn.

> Best regards,
> Krzysztof
> 

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[Krzysztof Kozlowski]

On 28/12/2022 04:23, yanhong wang wrote:
> 
> 
> On 2022/12/27 17:52, Krzysztof Kozlowski wrote:
>> On 27/12/2022 10:38, yanhong wang wrote:
>>>>
>>>> This must be false. After referencing ethernet-phy this should be
>>>> unevaluatedProperties: false.
>>>>
>>>>
>>>
>>> Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
>>> i will remove the parts of YT8531 in the next version.
>>
>> I don't understand what does it mean. You sent duplicated patch? If so,
>> please do not... you waste reviewers time.
>>
>> Anyway this entire patch does not meet criteria for submission at all,
>> so please start over from example-schema.
>>
> 
> Sorry, maybe I didn't make it clear, which led to misunderstanding. Motorcomm Inc is also 
> carrying out the upstream of YT8531, and my patch will be duplicated and conflicted 
> with their submission. By communicating with the developers of Motorcomm Inc, the part 
> of YT8531 will be submitted by Motorcomm Inc, so my submission about YT8531 will be withdrawn.

Are they going to apply the feedback received for this series?

Best regards,
Krzysztof

Re: [PATCH v2 5/9] dt-bindings: net: motorcomm: add support for Motorcomm YT8531

[yanhong wang]

On 2022/12/28 17:11, Krzysztof Kozlowski wrote:
> On 28/12/2022 04:23, yanhong wang wrote:
>> 
>> 
>> On 2022/12/27 17:52, Krzysztof Kozlowski wrote:
>>> On 27/12/2022 10:38, yanhong wang wrote:
>>>>>
>>>>> This must be false. After referencing ethernet-phy this should be
>>>>> unevaluatedProperties: false.
>>>>>
>>>>>
>>>>
>>>> Thanks. Parts of this patch exist already, after discussion unanimity was achieved,
>>>> i will remove the parts of YT8531 in the next version.
>>>
>>> I don't understand what does it mean. You sent duplicated patch? If so,
>>> please do not... you waste reviewers time.
>>>
>>> Anyway this entire patch does not meet criteria for submission at all,
>>> so please start over from example-schema.
>>>
>> 
>> Sorry, maybe I didn't make it clear, which led to misunderstanding. Motorcomm Inc is also 
>> carrying out the upstream of YT8531, and my patch will be duplicated and conflicted 
>> with their submission. By communicating with the developers of Motorcomm Inc, the part 
>> of YT8531 will be submitted by Motorcomm Inc, so my submission about YT8531 will be withdrawn.
> 
> Are they going to apply the feedback received for this series?
> 

Yes, they support not only YT8531, but also other models of their company.

> Best regards,
> Krzysztof
>