Re: [PATCH] dt-bindings: net: snps,dwmac: allow stmmaceth-ocp reset name
From: netdev-bot+sashiko@kernel.org
Date: 2026-09-10 08:08:51
Also in:
linux-devicetree, lkml, netdev
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1) - [Medium] The SoCFPGA-only reset name "stmmaceth-ocp" is added unconditionally to the *generic* snps,dwmac.yaml second-reset slot… Pre-existing issues: - [Low] After this patch, Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml… -- dt-bindings: net: snps,dwmac: allow stmmaceth-ocp reset name The patch relaxes the second entry of reset-names in the generic snps,dwmac.yaml schema so that "stmmaceth-ocp" is accepted alongside "ahb", and removes the Arria10 TODO note from the Altera SoCFPGA DWMAC binding description.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml b/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml index db93436c96fa..d8f90d36d246 100644 --- a/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml +++ b/Documentation/devicetree/bindings/net/altr,socfpga-stmmac.yaml@@ -13,8 +13,6 @@ description: This binding describes the Altera SOCFPGA SoC implementation of the Synopsys DWMAC for the Cyclone5, Arria5, Stratix10, Agilex5 and Agilex7 families of chips. - # TODO: Determine how to handle the Arria10 reset-name, stmmaceth-ocp, that - # does not validate against net/snps,dwmac.yaml.
[Severity: Low] This isn't a bug introduced by this patch, but after the TODO is removed this file no longer mentions Arria10 anywhere, while the family list in the same description block still reads: Synopsys DWMAC for the Cyclone5, Arria5, Stratix10, Agilex5 and Agilex7 families of chips. The compatible documented in this same file, altr,socfpga-stmmac-a10-s10, is the string used by the Arria10 gmac nodes in arch/arm/boot/dts/intel/socfpga/socfpga_arria10.dtsi. Could Arria10 be added to that family list along with dropping the TODO?
quoted hunk ↗ jump to hunk
select: properties:diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml index 2449311c6d28..c7f1a7c11c8d 100644 --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml@@ -174,7 +174,7 @@ properties: minItems: 1 items: - description: GMAC stmmaceth reset - - description: AHB reset + - description: AHB or OCP reset reset-names: oneOf:@@ -182,7 +182,7 @@ properties: - enum: [stmmaceth, ahb] - items: - const: stmmaceth - - const: ahb + - enum: [ahb, stmmaceth-ocp]
[Severity: Medium] Should this new name be gated on the Altera compatible instead of being allowed for every DWMAC binding? "stmmaceth-ocp" only has one in-tree consumer, drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c:socfpga_dwmac_probe(): dwmac->stmmac_ocp_rst = devm_reset_control_get_optional(dev, "stmmaceth-ocp"); The generic parser, stmmac_probe_config_dt() in stmmac_platform.c, only ever asks for the second reset by the name "ahb": plat->stmmac_ahb_rst = devm_reset_control_get_optional_shared( &pdev->dev, "ahb"); Since every other DWMAC binding (mediatek, rockchip, starfive, st, qcom, ...) $refs snps,dwmac.yaml, a device tree for one of those platforms can now write reset-names = "stmmaceth", "stmmaceth-ocp" and still pass dt_binding_check, while no driver on those platforms looks that name up, so that reset is never deasserted and validation reports nothing. Would an if/then on the compatible be preferable? snps,dwmac.yaml already carries such blocks (for example the compatible-gated snps,tso: false block), and altr,socfpga-stmmac.yaml already has an allOf with an if: on altr,socfpga-stmmac-agilex5, which looks like the natural place to constrain the Arria10 (altr,socfpga-stmmac-a10-s10) reset naming.
quoted hunk ↗ jump to hunk
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml@@ -174,7 +174,7 @@ properties: minItems: 1 items: - description: GMAC stmmaceth reset - - description: AHB reset + - description: AHB or OCP reset
[Severity: Medium] Related to the above: this description now covers two different resets with different acquisition semantics in one slot. The AHB reset is taken as a shared control via devm_reset_control_get_optional_shared() in stmmac_platform.c, while the OCP reset is taken as an exclusive control via devm_reset_control_get_optional() in dwmac-socfpga.c. Would separate per-compatible descriptions make the contract clearer? [ ... ] -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260909080559.2028-1-muhammad.nazim.amirul.nazle.asmade%40altera.com