[PATCH net-next] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

[PATCH net-next] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

[Rob Herring (Arm)]

Convert the HiSilicon hns-dsaf binding to DT schema format.

Drop 'phy-handle' at top level as there are no users. Add undocumented
'media-type' property.

Assisted-by: LLM
Signed-off-by: Rob Herring (Arm) <robh@kernel.org>
---
 .../bindings/net/hisilicon,hns-dsaf-v1.yaml   | 168 ++++++++++++++++++
 .../bindings/net/hisilicon-hns-dsaf.txt       |  90 ----------
 2 files changed, 168 insertions(+), 90 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
 delete mode 100644 Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt

diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
new file mode 100644
index 000000000000..4edc6cafcbed
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
@@ -0,0 +1,168 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright 2025 Hisilicon Ltd.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hisilicon,hns-dsaf-v1.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon DSA Fabric device controller
+
+maintainers:
+  - Jian Shen <shenjian15@huawei.com>
+  - Jijie Shao <shaojijie@huawei.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - hisilicon,hns-dsaf-v1
+          - hisilicon,hns-dsaf-v2
+
+  reg:
+    minItems: 1
+    items:
+      - description:
+          external interface control register base and size (optional, only
+          used when subctrl-syscon does not exist)
+      - description:
+          SerDes base register and size (optional, only used when serdes-syscon
+          in port node does not exist)
+      - description:
+          PPE register base and size
+      - description:
+          DSA Fabric base register and size (not required for single-port mode)
+
+  reg-names:
+    minItems: 1
+    items:
+      - const: ppe-base
+      - const: dsaf-base
+
+  interrupts:
+    minItems: 1
+    maxItems: 409
+
+  dma-coherent: true
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  mode:
+    description: DSA Fabric mode string
+    $ref: /schemas/types.yaml#/definitions/string
+    enum:
+      - 2port-64vf
+      - 6port-16rss
+      - 6port-16vf
+      - single-port
+
+  subctrl-syscon:
+    description: syscon handle for external interface control register
+    $ref: /schemas/types.yaml#/definitions/phandle
+
+  reset-field-offset:
+    description: offset of reset field in the control register
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  buf-size:
+    description: RX buffer size (bytes)
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  desc-num:
+    description: number of descriptors in TX and RX queue
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+patternProperties:
+  "^port@[0-5]$":
+    description: DSA Fabric port node
+    $ref: ethernet-switch-port.yaml#
+    additionalProperties: false
+
+    properties:
+      reg:
+        maximum: 5
+
+      phy-handle: true
+
+      serdes-syscon:
+        description: syscon handle for SerDes register
+        $ref: /schemas/types.yaml#/definitions/phandle
+
+      cpld-syscon:
+        description: syscon handle plus register offset for CPLD register
+        $ref: /schemas/types.yaml#/definitions/phandle-array
+        items:
+          - items:
+              - description: CPLD phandle
+              - description: offset
+
+      port-rst-offset:
+        description: reset field offset for this port
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      port-mode-offset:
+        description: port mode field offset for this port
+        $ref: /schemas/types.yaml#/definitions/uint32
+
+      mc-mac-mask:
+        description: multicast MAC address mask
+        $ref: /schemas/types.yaml#/definitions/uint8-array
+        maxItems: 6
+
+      media-type:
+        $ref: /schemas/types.yaml#/definitions/string
+        enum:
+          - copper
+          - fiber
+
+    required:
+      - reg
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - mode
+
+additionalProperties: false
+
+examples:
+  - |
+    dsa@c7000000 {
+        compatible = "hisilicon,hns-dsaf-v1";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        mode = "6port-16rss";
+        reg = <0x0 0xc5000000 0x0 0x890000
+               0x0 0xc7000000 0x0 0x60000>;
+        reg-names = "ppe-base", "dsaf-base";
+        subctrl-syscon = <&subctrl>;
+        reset-field-offset = <0>;
+        interrupts = <131 4>, <132 4>, <133 4>, <134 4>,
+                     <135 4>, <136 4>, <137 4>, <138 4>,
+                     <139 4>, <140 4>, <141 4>, <142 4>,
+                     <143 4>, <144 4>, <145 4>, <146 4>,
+                     <147 4>, <148 4>, <384 1>, <385 1>,
+                     <386 1>, <387 1>, <388 1>, <389 1>,
+                     <390 1>, <391 1>;
+        buf-size = <4096>;
+        desc-num = <1024>;
+        dma-coherent;
+
+        port@0 {
+            reg = <0>;
+            phy-handle = <&phy0>;
+            serdes-syscon = <&serdes>;
+            mc-mac-mask = [ff f0 00 00 00 00];
+        };
+
+        port@1 {
+            reg = <1>;
+            serdes-syscon = <&serdes>;
+            mc-mac-mask = [ff f0 00 00 00 00];
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt b/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
deleted file mode 100644
index 8ee4b1cedae8..000000000000
--- a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
+++ /dev/null
@@ -1,90 +0,0 @@
-Hisilicon DSA Fabric device controller
-
-Required properties:
-- compatible: should be "hisilicon,hns-dsaf-v1" or "hisilicon,hns-dsaf-v2".
-  "hisilicon,hns-dsaf-v1" is for hip05.
-  "hisilicon,hns-dsaf-v2" is for Hi1610 and Hi1612.
-- mode: dsa fabric mode string. only support one of dsaf modes like these:
-		"2port-64vf",
-		"6port-16rss",
-		"6port-16vf",
-		"single-port".
-- interrupts: should contain the DSA Fabric and rcb interrupt.
-- reg: specifies base physical address(es) and size of the device registers.
-  The first region is external interface control register base and size(optional,
-  only used when subctrl-syscon does not exist). It is recommended using
-  subctrl-syscon rather than this address.
-  The second region is SerDes base register and size(optional, only used when
-  serdes-syscon in port node does not exist). It is recommended using
-  serdes-syscon rather than this address.
-  The third region is the PPE register base and size.
-  The fourth region is dsa fabric base register and size. It is not required for
-  single-port mode.
-- reg-names: may be ppe-base and(or) dsaf-base. It is used to find the
-  corresponding reg's index.
-
-- phy-handle: phy handle of physical port, 0 if not any phy device. It is optional
-  attribute. If port node exists, phy-handle in each port node will be used.
-  see ethernet.txt [1].
-- subctrl-syscon: is syscon handle for external interface control register.
-- reset-field-offset: is offset of reset field. Its value depends on the hardware
-  user manual.
-- buf-size: rx buffer size, should be 16-1024.
-- desc-num: number of description in TX and RX queue, should be 512, 1024, 2048 or 4096.
-
-- port: subnodes of dsaf. A dsaf node may contain several port nodes(Depending
-  on mode of dsaf). Port node contain some attributes listed below:
-- reg: is physical port index in one dsaf.
-- phy-handle: phy handle of physical port. It is not required if there isn't
-  phy device. see ethernet.txt [1].
-- serdes-syscon: is syscon handle for SerDes register.
-- cpld-syscon: is syscon handle + register offset pair for cpld register. It is
-  not required if there isn't cpld device.
-- port-rst-offset: is offset of reset field for each port in dsaf. Its value
-  depends on the hardware user manual.
-- port-mode-offset: is offset of port mode field for each port in dsaf. Its
-  value depends on the hardware user manual.
-- mc-mac-mask: mask of multicast address, determines bit in multicast address
-  to set:
-  1 stands for this bit will be precisely matched, TCAM will check this bit of
-    MAC address.
-  0 stands for this bit will be fuzzy matched, TCAM won't care about this bit
-    of MAC address.
-
-[1] Documentation/devicetree/bindings/net/phy.txt
-
-Example:
-
-dsaf0: dsa@c7000000 {
-	compatible = "hisilicon,hns-dsaf-v1";
-	mode = "6port-16rss";
-	interrupt-parent = <&mbigen_dsa>;
-	reg = <0x0 0xc5000000 0x0 0x890000
-	       0x0 0xc7000000 0x0 0x60000>;
-	reg-names = "ppe-base", "dsaf-base";
-	subctrl-syscon = <&subctrl>;
-	reset-field-offset = 0;
-	interrupts = <131 4>,<132 4>, <133 4>,<134 4>,
-		     <135 4>,<136 4>, <137 4>,<138 4>,
-		     <139 4>,<140 4>, <141 4>,<142 4>,
-		     <143 4>,<144 4>, <145 4>,<146 4>,
-		     <147 4>,<148 4>, <384 1>,<385 1>,
-		     <386 1>,<387 1>, <388 1>,<389 1>,
-		     <390 1>,<391 1>,
-	buf-size = <4096>;
-	desc-num = <1024>;
-	dma-coherent;
-
-	port@0 {
-		reg = 0;
-		phy-handle = <&phy0>;
-		serdes-syscon = <&serdes>;
-		mc-mac-mask = [ff f0 00 00 00 00];
-	};
-
-	port@1 {
-                reg = 1;
-                serdes-syscon = <&serdes>;
-		mc-mac-mask = [ff f0 00 00 00 00];
-        };
-};
-- 
2.53.0

Re: [PATCH net-next] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 5 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 3

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The schema leaves out properties the driver needs and drops
  value constraints the hardware relies on: (1) 'buf-size' and 'desc-…
- [Medium] The 'reg' description in the new schema (lines 21-33) gives a
  fixed four-entry positional layout: [0] external interface control…
- [Low] The patch deletes
  Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt without…
- [Low] In the new binding, the port 'media-type' enum (lines 115-119)
  allows only 'copper' and 'fiber'.
- [Low] The schema example (lines 135-142) sits at the top level with no
  parent bus node that sets '#address-cells = <2>; #size-cells = <2>;'.

--

dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

This converts the HiSilicon hns-dsaf binding from text to DT schema
format. It drops the unused top-level phy-handle property and documents
the previously undocumented media-type port property.

> diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
> new file mode 100644
> index 0000000000000..4edc6cafcbed3
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
> @@ -0,0 +1,168 @@

[ ... ]

> +  reg:
> +    minItems: 1
> +    items:
> +      - description:
> +          external interface control register base and size (optional, only
> +          used when subctrl-syscon does not exist)
> +      - description:
> +          SerDes base register and size (optional, only used when serdes-syscon
> +          in port node does not exist)
> +      - description:
> +          PPE register base and size
> +      - description:
> +          DSA Fabric base register and size (not required for single-port mode)
> +
> +  reg-names:
> +    minItems: 1
> +    items:
> +      - const: ppe-base
> +      - const: dsaf-base

[Severity: Medium]

Do the reg item descriptions match reg-names? The reg list describes a
four-entry layout by position (subctrl, SerDes, PPE, DSAF). But reg-names
is required, and it only allows ppe-base at index 0 and dsaf-base at
index 1.

The in-tree users in hip06.dtsi and hip07.dtsi look like this:

    reg = <0x0 0xc5000000 0x0 0x890000>,
          <0x0 0xc7000000 0x0 0x600000>;
    reg-names = "ppe-base", "dsaf-base";

So reg[0] is really the PPE block, but the schema describes it as the
external interface control block.

The four-region layout also can't be written with these reg-names. When
subctrl-syscon is absent, hns_dsaf_get_cfg() takes the first regions by
position and then looks up PPE by name:

drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c:hns_dsaf_get_cfg() {
    ...
    if (IS_ERR_OR_NULL(syscon)) {
        res = platform_get_resource(pdev, IORESOURCE_MEM, res_idx++);
        ...
    }
    ...
    res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ppe-base");
    ...
}

With ppe-base forced to index 0, wouldn't this lookup return the region
already mapped as sc_base? The second devm_ioremap_resource() would then
fail.

Should the reg items describe PPE and then DSAF instead, keeping
minItems: 1 for single-port mode?

[ ... ]

> +  buf-size:
> +    description: RX buffer size (bytes)
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  desc-num:
> +    description: number of descriptors in TX and RX queue
> +    $ref: /schemas/types.yaml#/definitions/uint32

[ ... ]

> +      media-type:
> +        $ref: /schemas/types.yaml#/definitions/string
> +        enum:
> +          - copper
> +          - fiber

[Severity: Low]

Should backplane be in this enum too? hns_mac_get_info() reads media-type
with fwnode_property_read_string() for both OF and ACPI port nodes. It
then matches the string against media_type_defs in hns_dsaf_mac.c:

    {HNAE_MEDIA_TYPE_UNKNOWN,   "unknown" },
    {HNAE_MEDIA_TYPE_FIBER,     "fiber" },
    {HNAE_MEDIA_TYPE_COPPER,    "copper" },
    {HNAE_MEDIA_TYPE_BACKPLANE, "backplane" },

The driver handles a DT port with media-type = "backplane", but that port
would fail dtbs_check with this schema. In-tree DTs only use fiber and
copper, and the commit message doesn't say why backplane was left out.

> +
> +    required:
> +      - reg
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - mode

[Severity: Medium]

The old text binding listed buf-size and desc-num under "Required
properties". Neither one is in this required list, and the commit message
doesn't mention the change. Should they be required here?

hns_dsaf_get_cfg() fails probe if either one is missing:

    ret = device_property_read_u32(dsaf_dev->dev, "desc-num", &desc_num);
    if (ret < 0 || desc_num < HNS_DSAF_MIN_DESC_CNT ||
        desc_num > HNS_DSAF_MAX_DESC_CNT) {
    ...
    ret = device_property_read_u32(dsaf_dev->dev, "buf-size", &buf_size);
    if (ret < 0) {

The buf-size and desc-num definitions above also have no value
constraints. The driver only accepts a desc-num between
HNS_DSAF_MIN_DESC_CNT (16) and HNS_DSAF_MAX_DESC_CNT (1024).
hns_rcb_buf_size2type() only accepts a buf-size of 512, 1024, 2048 or
4096.

The old text had the two ranges swapped: "16-1024" for buf-size and
"512, 1024, 2048 or 4096" for desc-num. The conversion dropped both
ranges instead of fixing them.

Similarly, serdes-syscon isn't in the port required list, but for OF
port nodes hns_mac_get_info() does:

    if (IS_ERR_OR_NULL(syscon)) {
        dev_err(mac_cb->dev, "serdes-syscon is needed!\n");
        return -EINVAL;

The following DTs would pass dtbs_check but fail to probe:

  - buf-size = <1500> or desc-num = <0>
  - buf-size or desc-num missing
  - a port without serdes-syscon

The in-tree hip06 and hip07 DTs already set desc-num = <0x400>,
buf-size = <0x1000>, and serdes-syscon in every port. Would something
like this work?

  buf-size:
    enum: [512, 1024, 2048, 4096]
  desc-num:
    minimum: 16
    maximum: 1024

Both would then go in the top-level required list, and serdes-syscon in
the port required list.

> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    dsa@c7000000 {
> +        compatible = "hisilicon,hns-dsaf-v1";
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        mode = "6port-16rss";
> +        reg = <0x0 0xc5000000 0x0 0x890000
> +               0x0 0xc7000000 0x0 0x60000>;
> +        reg-names = "ppe-base", "dsaf-base";

[Severity: Low]

Is this reg decoded as intended? The example has no parent bus node with
#address-cells = <2> and #size-cells = <2>, and dt_binding_check wraps
examples in a node with 1/1 cells. So the eight cells become four
(address, size) pairs:

    (0x0, 0xc5000000), (0x0, 0x890000), (0x0, 0xc7000000), (0x0, 0x60000)

There are only two reg-names, so ppe-base decodes as address 0x0 with
size 0xc5000000. Validation still passes because reg allows up to four
items and the reg count isn't checked against reg-names.

Also, the unit address @c7000000 doesn't match the first reg address with
either cell size. The in-tree nodes are named dsa@c5000000.

Could the example be wrapped in a bus node with 2/2 cells and renamed to
dsa@c5000000?

[ ... ]

> diff --git a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt b/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> deleted file mode 100644
> index 8ee4b1cedae80..0000000000000
> --- a/Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt
> +++ /dev/null
> @@ -1,90 +0,0 @@
> -Hisilicon DSA Fabric device controller

[ ... ]

[Severity: Low]

This file is deleted, but the ae-handle description in
hisilicon-hns-nic.txt still points to it:

Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt:
- ae-handle: accelerator engine handle for hns,
  specifies a reference to the associating hardware driver node.
  see Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt

Should this reference point to hisilicon,hns-dsaf-v1.yaml instead?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925215848.2949157-2-robh%40kernel.org