Devicetree
 help / color / mirror / Atom feed
* [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
@ 2026-09-30 20:19 Rob Herring (Arm)
  2026-10-02  9:13 ` sashiko-bot
  2026-10-04 22:15 ` netdev-bot+sashiko
  0 siblings, 2 replies; 5+ messages in thread
From: Rob Herring (Arm) @ 2026-09-30 20:19 UTC (permalink / raw)
  To: Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Krzysztof Kozlowski, Conor Dooley, Jian Shen,
	Jijie Shao
  Cc: netdev, devicetree, linux-kernel

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>
---
v2:
 - Add constraints for buf-size and desc-num
 - Drop erroneous 1st 2 'reg' entries
 - Fix example 'reg' cell sizes
 - Add 'backplane' to 'media-type' property
---
 .../bindings/net/hisilicon,hns-dsaf-v1.yaml   | 167 ++++++++++++++++++
 .../bindings/net/hisilicon-hns-dsaf.txt       |  90 ----------
 2 files changed, 167 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..36251b45abeb
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
@@ -0,0 +1,167 @@
+# 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:
+          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
+    enum: [512, 1024, 2048, 4096]
+
+  desc-num:
+    description: number of descriptors in TX and RX queue
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 16
+    maximum: 1024
+
+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:
+          - backplane
+          - copper
+          - fiber
+
+    required:
+      - reg
+      - serdes-syscon
+
+required:
+  - compatible
+  - reg
+  - reg-names
+  - interrupts
+  - mode
+
+additionalProperties: false
+
+examples:
+  - |
+    dsa@c5000000 {
+        compatible = "hisilicon,hns-dsaf-v1";
+        #address-cells = <1>;
+        #size-cells = <0>;
+        mode = "6port-16rss";
+        reg = <0xc5000000 0x890000
+               0xc7000000 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
  2026-09-30 20:19 [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema Rob Herring (Arm)
@ 2026-10-02  9:13 ` sashiko-bot
  2026-10-02 16:50   ` Rob Herring
  2026-10-04 22:15 ` netdev-bot+sashiko
  1 sibling, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-10-02  9:13 UTC (permalink / raw)
  To: Rob Herring (Arm); +Cc: conor+dt, devicetree, robh

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: additionalProperties false ignores properties from $ref
- [Medium] dt-bindings: missing required properties desc-num, buf-size and subctrl-syscon
- [Low] dt-bindings: driver still uses removed top-level phy-handle property
--

commit d25b601aef3405df25a4432415cfd3e767a90222
Author: Rob Herring (Arm) <robh@kernel.org>
Subject: dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema

This commit converts the HiSilicon hns-dsaf device tree binding from
the old text format to the new YAML schema format. It drops the
top-level phy-handle property and adds the media-type property.

> 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.

[Severity: Low]
Is it entirely accurate that there are no users of the top-level
phy-handle property?

Looking at hns_mac_init_phy_handle() in
drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c:

    np = of_parse_phandle(mac_cb->dev->of_node, "phy-handle",
                          mac_cb->mac_id);

The driver still explicitly parses this property from the top-level DSAF
node as a fallback if port subnodes are not present. Removing this from
the schema without removing the fallback logic in the driver will cause
out-of-tree or legacy device trees relying on this feature to needlessly
fail schema validation.

Should it remain in the schema (possibly marked as deprecated) until the
C driver is patched to drop support entirely?

[ ... ]
> 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..36251b45abeb9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
[ ... ]
> +patternProperties:
> +  "^port@[0-5]$":
> +    description: DSA Fabric port node
> +    $ref: ethernet-switch-port.yaml#
> +    additionalProperties: false

[Severity: Medium]
Does additionalProperties: false correctly allow properties inherited
from the $ref?

Under JSON Schema semantics, additionalProperties only restricts the
properties explicitly listed in the local object, completely ignoring
any properties evaluated via a $ref.

Would it be better to use unevaluatedProperties: false here so standard
Ethernet port properties aren't incorrectly rejected by dt_binding_check
when present in the device tree?

> +
> +    properties:
> +      reg:
> +        maximum: 5
[ ... ]
> +    required:
> +      - reg
> +      - serdes-syscon
> +
> +required:
> +  - compatible
> +  - reg
> +  - reg-names
> +  - interrupts
> +  - mode

[Severity: Medium]
Should desc-num, buf-size, and subctrl-syscon be included in this
required list?

Looking at hns_dsaf_get_cfg() in
drivers/net/ethernet/hisilicon/hns/hns_dsaf_main.c, the driver strictly
fails the probe if desc-num or buf-size are omitted.

Furthermore, if subctrl-syscon is missing, the driver's fallback logic
consumes reg index 0 (ppe-base) as the syscon base. Later, when it
attempts to map ppe-base again by name, it fails with an -EBUSY error
and aborts the probe.

Since omitting any of these three properties causes a guaranteed driver
probe failure, shouldn't they be marked as required to ensure invalid
device trees are properly flagged?

> +
> +additionalProperties: false
> +
> +examples:
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930201919.76465-2-robh@kernel.org?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
  2026-10-02  9:13 ` sashiko-bot
@ 2026-10-02 16:50   ` Rob Herring
  0 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2026-10-02 16:50 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: conor+dt, devicetree

On Fri, Oct 02, 2026 at 09:13:40AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
> - [Medium] dt-bindings: additionalProperties false ignores properties from $ref
> - [Medium] dt-bindings: missing required properties desc-num, buf-size and subctrl-syscon
> - [Low] dt-bindings: driver still uses removed top-level phy-handle property
> --
> 
> commit d25b601aef3405df25a4432415cfd3e767a90222
> Author: Rob Herring (Arm) <robh@kernel.org>
> Subject: dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
> 
> This commit converts the HiSilicon hns-dsaf device tree binding from
> the old text format to the new YAML schema format. It drops the
> top-level phy-handle property and adds the media-type property.
> 
> > 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.
> 
> [Severity: Low]
> Is it entirely accurate that there are no users of the top-level
> phy-handle property?

Yes, there are no .dts users. It is not even clear if these platforms 
are even being used. HiSilicon folks can add it back if they care.

> 
> Looking at hns_mac_init_phy_handle() in
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_mac.c:
> 
>     np = of_parse_phandle(mac_cb->dev->of_node, "phy-handle",
>                           mac_cb->mac_id);
> 
> The driver still explicitly parses this property from the top-level DSAF
> node as a fallback if port subnodes are not present. Removing this from
> the schema without removing the fallback logic in the driver will cause
> out-of-tree or legacy device trees relying on this feature to needlessly
> fail schema validation.
> 
> Should it remain in the schema (possibly marked as deprecated) until the
> C driver is patched to drop support entirely?
> 
> [ ... ]
> > 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..36251b45abeb9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml
> [ ... ]
> > +patternProperties:
> > +  "^port@[0-5]$":
> > +    description: DSA Fabric port node
> > +    $ref: ethernet-switch-port.yaml#
> > +    additionalProperties: false
> 
> [Severity: Medium]
> Does additionalProperties: false correctly allow properties inherited
> from the $ref?
> 
> Under JSON Schema semantics, additionalProperties only restricts the
> properties explicitly listed in the local object, completely ignoring
> any properties evaluated via a $ref.
> 
> Would it be better to use unevaluatedProperties: false here so standard
> Ethernet port properties aren't incorrectly rejected by dt_binding_check
> when present in the device tree?

There aren't any properties from ethernet-controller.yaml used. Again, 
old platform, no new users, and possibly unused.

Rob

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
  2026-09-30 20:19 [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema Rob Herring (Arm)
  2026-10-02  9:13 ` sashiko-bot
@ 2026-10-04 22:15 ` netdev-bot+sashiko
  2026-10-05 20:16   ` Rob Herring
  1 sibling, 1 reply; 5+ messages in thread
From: netdev-bot+sashiko @ 2026-10-04 22:15 UTC (permalink / raw)
  To: robh
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt,
	shenjian15, shaojijie, netdev, devicetree, linux-kernel

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

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

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 conversion drops the two leading legacy 'reg' regions
  (sub-control and SerDes) that the old binding documented as the…
- [Medium] The top-level 'required' list in hisilicon,hns-dsaf-v1.yaml
  leaves out 'desc-num' and 'buf-size', although the deleted text…
- [Low] The patch deletes
  Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt, but…
- [Low] The deleted text binding had the buf-size and desc-num ranges
  swapped ('buf-size: ... should be 16-1024', 'desc-num: ... should be…
- [Low] The concern says that 'mc-mac-mask' with only 'maxItems: 6'
  accepts 1–5 bytes, while hns_mac_get_info() needs exactly ETH_ALEN (6)…
- [Low] reg and reg-names use 'minItems: 1' for every 'mode'.

--

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

This commit converts the HiSilicon hns-dsaf text binding to a YAML DT
schema. 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..36251b45abeb9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-dsaf-v1.yaml

[ ... ]

> +  reg:
> +    minItems: 1
> +    items:
> +      - 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: Low]
minItems: 1 applies to every mode here. Would the schema accept a node
with mode = "6port-16rss" and only reg-names = "ppe-base"?

The description says dsaf-base may only be left out in single-port mode.
hns_dsaf_get_cfg() skips it only when HNS_DSAF_IS_DEBUG() is true, which
means dsaf_mode == DSAF_MODE_DISABLE_SP:

	if (!HNS_DSAF_IS_DEBUG(dsaf_dev)) {
		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
						   "dsaf-base");
		if (!res) {
			res = platform_get_resource(pdev, IORESOURCE_MEM,
						    res_idx);
			...
		}
		dsaf_dev->io_base = devm_ioremap_resource(&pdev->dev, res);

When subctrl-syscon is used, res_idx is still 0 at this point. The
fallback therefore picks the ppe-base region again, and
devm_ioremap_resource() fails with -EBUSY.

Could an if/then on mode require two reg and reg-names entries for
2port-64vf, 6port-16rss and 6port-16vf?

[ ... ]

> +  buf-size:
> +    description: RX buffer size (bytes)
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [512, 1024, 2048, 4096]
> +
> +  desc-num:
> +    description: number of descriptors in TX and RX queue
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 16
> +    maximum: 1024

[Severity: Low]
This isn't a bug, but these ranges are the reverse of what the deleted
text binding said:

- 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.

The new constraints match the driver. hns_dsaf_get_cfg() checks desc-num
against HNS_DSAF_MIN_DESC_CNT and HNS_DSAF_MAX_DESC_CNT, and it checks
buf-size through hns_rcb_buf_size2type().

The only related note is "Add constraints for buf-size and desc-num" in
the v2 changelog, and that text is dropped when the patch is applied.
Could the commit message say that the old text had these ranges swapped?

[ ... ]

> +      mc-mac-mask:
> +        description: multicast MAC address mask
> +        $ref: /schemas/types.yaml#/definitions/uint8-array
> +        maxItems: 6

[Severity: Low]
hns_mac_get_info() needs exactly ETH_ALEN bytes for this property. With
any other length it falls back to the broadcast mask:

	if (fwnode_property_read_u8_array(mac_cb->fw_port, "mc-mac-mask",
					  mac_cb->mc_mask, ETH_ALEN)) {
		dev_warn(mac_cb->dev,
			 "no mc-mac-mask property, set to default value.\n");
		eth_broadcast_addr(mac_cb->mc_mask);
	}

Does maxItems: 6 on its own reject a mask shorter than 6 bytes? If the
dtschema fixups set minItems equal to maxItems here, this is already
fine. If they don't, is an explicit minItems: 6 needed?

[ ... ]

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

[Severity: Medium]
Should subctrl-syscon be in this list now?

The old binding allowed two leading reg regions, for sub-control and
SerDes, as an alternative to subctrl-syscon. This conversion drops those
regions, so reg can now only describe ppe-base and dsaf-base.

The OF path in hns_dsaf_get_cfg() still uses the old layout when the
phandle is missing:

	np_temp = of_parse_phandle(np, "subctrl-syscon", 0);
	syscon = syscon_node_to_regmap(np_temp);
	of_node_put(np_temp);
	if (IS_ERR_OR_NULL(syscon)) {
		res = platform_get_resource(pdev, IORESOURCE_MEM,
					    res_idx++);
		...
		dsaf_dev->sc_base = devm_ioremap_resource(&pdev->dev,
							  res);
		...
		res = platform_get_resource(pdev, IORESOURCE_MEM,
					    res_idx++);
		...
		dsaf_dev->sds_base = devm_ioremap_resource(&pdev->dev,
							   res);

Under the new schema reg[0] is ppe-base, so the lookup by name that
follows requests the same region a second time:

	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ppe-base");
	...
	dsaf_dev->ppe_base = devm_ioremap_resource(&pdev->dev, res);

That call fails with -EBUSY. In single-port mode, where only ppe-base is
present, probe fails even earlier with "serdes-ctrl info is needed!".

So a node without subctrl-syscon passes dt_binding_check but can never
probe. The in-tree hip06 and hip07 dtsi files already set
subctrl-syscon, so requiring it would not break them.

Also, the narrowing of reg appears only in the v2 changelog ("Drop
erroneous 1st 2 'reg' entries") and not in the commit body. Could the
body mention it, as it does for the phy-handle and media-type changes?

[Severity: Medium]
The deleted text binding listed buf-size and desc-num under "Required
properties". 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) {
		...
		return -EINVAL;
	}
	...
	ret = device_property_read_u32(dsaf_dev->dev, "buf-size", &buf_size);
	if (ret < 0) {
		...
		return ret;
	}

Should desc-num and buf-size be added to this list as well?

[ ... ]

> 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

[Severity: Low]
The ae-handle description in
Documentation/devicetree/bindings/net/hisilicon-hns-nic.txt still points
to this file:

- 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 it point to hisilicon,hns-dsaf-v1.yaml instead?

[ ... ]

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema
  2026-10-04 22:15 ` netdev-bot+sashiko
@ 2026-10-05 20:16   ` Rob Herring
  0 siblings, 0 replies; 5+ messages in thread
From: Rob Herring @ 2026-10-05 20:16 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: andrew+netdev, davem, edumazet, kuba, pabeni, krzk+dt, conor+dt,
	shenjian15, shaojijie, netdev, devicetree, linux-kernel

On Sun, Oct 04, 2026 at 10:15:37PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 6 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 2 · Low: 4
> 
> 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 conversion drops the two leading legacy 'reg' regions
>   (sub-control and SerDes) that the old binding documented as the…

The old binding was deprecated for a long time and there are no users. 
Will detail in commit msg.

> - [Medium] The top-level 'required' list in hisilicon,hns-dsaf-v1.yaml
>   leaves out 'desc-num' and 'buf-size', although the deleted text…

Will fix.

> - [Low] The patch deletes
>   Documentation/devicetree/bindings/net/hisilicon-hns-dsaf.txt, but…

Not an issue. Fixed when other binding is converted.

> - [Low] The deleted text binding had the buf-size and desc-num ranges
>   swapped ('buf-size: ... should be 16-1024', 'desc-num: ... should be…

Based on the .dts files and driver, the text binding is wrong.

> - [Low] The concern says that 'mc-mac-mask' with only 'maxItems: 6'
>   accepts 1–5 bytes, while hns_mac_get_info() needs exactly ETH_ALEN (6)…

Not an issue.

> - [Low] reg and reg-names use 'minItems: 1' for every 'mode'.

Not worth the complexity to try to express that.

pw-bot: cr

Rob

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-05 20:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30 20:19 [PATCH net-next v2] dt-bindings: net: Convert HiSilicon hns-dsaf to DT schema Rob Herring (Arm)
2026-10-02  9:13 ` sashiko-bot
2026-10-02 16:50   ` Rob Herring
2026-10-04 22:15 ` netdev-bot+sashiko
2026-10-05 20:16   ` Rob Herring

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox