[PATCH net-next] dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

[PATCH net-next] dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

[Rob Herring (Arm)]

Convert the hisilicon,hns-mdio binding to DT schema format. Add the
missing 'subctrl-vbase' property which is already in use.

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

diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml
new file mode 100644
index 000000000000..b8350794650d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml
@@ -0,0 +1,59 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/hisilicon,hns-mdio.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Hisilicon HNS MDIO bus controller
+
+maintainers:
+  - Jian Shen <shenjian15@huawei.com>
+  - Jijie Shao <shaojijie@huawei.com>
+
+allOf:
+  - $ref: mdio.yaml#
+
+properties:
+  compatible:
+    enum:
+      - hisilicon,hns-mdio
+      - hisilicon,mdio
+
+  reg:
+    maxItems: 1
+
+  subctrl-vbase:
+    $ref: /schemas/types.yaml#/definitions/phandle-array
+    items:
+      - items:
+          - description: syscon phandle
+          - description: MDIO clock enable register offset
+          - description: MDIO reset request register offset
+          - description: MDIO reset done register offset
+          - description: MDIO reset deassert register offset
+
+required:
+  - compatible
+  - reg
+  - '#address-cells'
+  - '#size-cells'
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    bus {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        mdio@803c0000 {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            compatible = "hisilicon,hns-mdio";
+            reg = <0x0 0x803c0000 0x0 0x10000>;
+
+            ethernet-phy@0 {
+                reg = <0>;
+            };
+        };
+    };
diff --git a/Documentation/devicetree/bindings/net/hisilicon-hns-mdio.txt b/Documentation/devicetree/bindings/net/hisilicon-hns-mdio.txt
deleted file mode 100644
index 4a7ede9657b0..000000000000
--- a/Documentation/devicetree/bindings/net/hisilicon-hns-mdio.txt
+++ /dev/null
@@ -1,27 +0,0 @@
-Hisilicon MDIO bus controller
-
-Properties:
-- compatible: can be one of:
-	"hisilicon,hns-mdio"
-	"hisilicon,mdio"
-  "hisilicon,hns-mdio" is recommended to be used for hip05 and later SOCs,
-  while "hisilicon,mdio" is optional for backwards compatibility only on
-  hip04 Soc.
-- reg: The base address of the MDIO bus controller register bank.
-- #address-cells: Must be <1>.
-- #size-cells: Must be <0>.  MDIO addresses have no size component.
-
-Typically an MDIO bus might have several children.
-
-Example:
-         mdio@803c0000 {
-                   #address-cells = <1>;
-                   #size-cells = <0>;
-                   compatible = "hisilicon,hns-mdio","hisilicon,mdio";
-                   reg = <0x0 0x803c0000 0x0 0x10000>;
-
-                   ethernet-phy@0 {
-                            ...
-                            reg = <0>;
-                   };
-         };
-- 
2.53.0

Re: [PATCH net-next] dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] dt-bindings: net: hisilicon,hns-mdio documents incorrect subctrl-vbase register offsets
- [Medium] dt-bindings: net: hisilicon,hns-mdio restricts compatible property breaking backwards compatibility
- [Medium] dt-bindings: net: hisilicon,hns-mdio omits functionally required property subctrl-vbase
--

commit a57c7bf53ebe7bce1c71d128ab3f7430b9797694
Author: Rob Herring (Arm) <robh@kernel.org>

dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

This commit converts the hisilicon,hns-mdio binding to DT schema format.
It also adds the subctrl-vbase property which is currently in use.

> diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml
> new file mode 100644
> index 0000000000000..b8350794650d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml

[ ... ]

> +properties:
> +  compatible:
> +    enum:
> +      - hisilicon,hns-mdio
> +      - hisilicon,mdio

[Severity: Medium]
Does this new enum restriction unintentionally break backwards compatibility?

The original txt binding documented a fallback array format that is now
rejected by this schema. As shown in the deleted txt example, arrays like:

compatible = "hisilicon,hns-mdio", "hisilicon,mdio";

were previously valid. Existing device trees using that format will now fail
local dtbs validation.

[ ... ]

> +  subctrl-vbase:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: syscon phandle
> +          - description: MDIO clock enable register offset
> +          - description: MDIO reset request register offset
> +          - description: MDIO reset done register offset
> +          - description: MDIO reset deassert register offset

[Severity: High]
Are these register descriptions for subctrl-vbase accurate?

The driver implementation maps the third argument to clock status and the
fourth argument to reset status, computing the reset deassert offset
internally by adding 4 to the second argument.

Documenting the third argument as reset done and the fourth argument as reset
deassert could lead hardware engineers to provide incorrect offsets. Following
this documentation causes the driver to poll incorrect syscon registers
during reset, leading to timeouts and functional failures during probe.

[ ... ]

> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'

[Severity: Medium]
Should subctrl-vbase be included in this list of required properties?

The driver unconditionally checks for subctrl-vbase during probe on DT
systems and fails with -ENODEV if it is missing. Device trees authored
without this property will pass dt-schema validation, but the hardware will
fail to initialize at runtime.

[ ... ]

> +examples:
> +  - |
> +    bus {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +
> +        mdio@803c0000 {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +            compatible = "hisilicon,hns-mdio";
> +            reg = <0x0 0x803c0000 0x0 0x10000>;
> +
> +            ethernet-phy@0 {
> +                reg = <0>;
> +            };
> +        };
> +    };

[Severity: Medium]
Since subctrl-vbase appears to be functionally required for initialization,
should it be included in this example? Adding it here would ensure the
example still passes validation if the property is correctly marked as
required.

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

Re: [PATCH net-next] dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

[netdev-bot+sashiko]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 1

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] In the new
  Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml, two of…
- [Low] The new schema lists 'subctrl-vbase' but doesn't make it
  required; the required list is only compatible, reg, #address-cells…

--

dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

This converts the hisilicon,hns-mdio text binding to DT schema format.
It also adds the subctrl-vbase property. The driver and the hip06/hip07
device trees already use that property.

> diff --git a/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml
> new file mode 100644
> index 0000000000000..b8350794650d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml
> @@ -0,0 +1,59 @@

[ ... ]

> +  subctrl-vbase:
> +    $ref: /schemas/types.yaml#/definitions/phandle-array
> +    items:
> +      - items:
> +          - description: syscon phandle
> +          - description: MDIO clock enable register offset
> +          - description: MDIO reset request register offset
> +          - description: MDIO reset done register offset
> +          - description: MDIO reset deassert register offset

[Severity: Medium]
Do the last two argument cell descriptions match how the driver reads
them?

hns_mdio_probe() in drivers/net/ethernet/hisilicon/hns_mdio.c reads the
last two cells as status registers. It gets the disable and deassert
offsets by adding 4 to the first two cells:

drivers/net/ethernet/hisilicon/hns_mdio.c:hns_mdio_probe() {
    ...
	mdio_dev->sc_reg.mdio_clk_en =
		(u16)reg_args.args[0];
	mdio_dev->sc_reg.mdio_clk_dis =
		(u16)reg_args.args[0] + 4;
	mdio_dev->sc_reg.mdio_reset_req =
		(u16)reg_args.args[1];
	mdio_dev->sc_reg.mdio_reset_dreq =
		(u16)reg_args.args[1] + 4;
	mdio_dev->sc_reg.mdio_clk_st =
		(u16)reg_args.args[2];
	mdio_dev->sc_reg.mdio_reset_st =
		(u16)reg_args.args[3];
    ...
}

hns_mdio_reset() then polls mdio_clk_st and mdio_reset_st after it
writes the enable/disable and request/deassert registers.

The in-tree users agree with the driver. hip06.dtsi and hip07.dtsi have:

    subctrl-vbase = <&peri_c_subctrl 0x338 0xa38 0x531c 0x5a1c>;

These values are MDIO_SC_CLK_EN, MDIO_SC_RESET_REQ, MDIO_SC_CLK_ST and
MDIO_SC_RESET_ST. MDIO_SC_RESET_DREQ (0xa3c) is RESET_REQ + 4, not a
cell.

Suppose a new DT follows this schema and puts the reset deassert offset
in the last cell. Then hns_mdio_reset() would poll the wrong register and
time out, and the MDIO bus would fail to register.

Should the last two cells be described as the MDIO clock status and
MDIO reset status register offsets? It could also help to say that the
clock disable and reset deassert registers sit at +4 from the first two
offsets.

> +
> +required:
> +  - compatible
> +  - reg
> +  - '#address-cells'
> +  - '#size-cells'

[Severity: Low]
Should subctrl-vbase be added to the required list?

On DT systems the driver does not seem to work without it. In
hns_mdio_probe(), if of_parse_phandle_with_fixed_args() fails to find
subctrl-vbase, the driver only warns. It leaves subctrl_vbase as NULL
and still calls of_mdiobus_register(). The bus reset callback then
fails:

hns_mdio_probe()
  of_mdiobus_register()
    __mdiobus_register()
      bus->reset() == hns_mdio_reset()

drivers/net/ethernet/hisilicon/hns_mdio.c:hns_mdio_reset() {
    ...
	if (dev_of_node(bus->parent)) {
		if (!mdio_dev->subctrl_vbase) {
			dev_err(&bus->dev, "mdio sys ctl reg has not mapped\n");
			return -ENODEV;
		}
    ...
}

__mdiobus_register() then fails with that error.

So a node without subctrl-vbase passes dt_binding_check, but the bus
can never come up. The example in this schema is one such node.

Both in-tree users, hip06.dtsi and hip07.dtsi, already set the property.
Making it required would not break any existing DTs.

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

Re: [PATCH net-next] dt-bindings: net: Convert hisilicon,hns-mdio to DT schema

[Rob Herring]

On Tue, Sep 29, 2026 at 10:37:02PM +0000, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider.
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 1
> 
> 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] In the new
>   Documentation/devicetree/bindings/net/hisilicon,hns-mdio.yaml, two of…
> - [Low] The new schema lists 'subctrl-vbase' but doesn't make it
>   required; the required list is only compatible, reg, #address-cells…

pw-bot: cr