* Re: [PATCH] dt-bindings: net: snps,dwmac: allow stmmaceth-ocp reset name
2026-09-09 8:05 [PATCH] dt-bindings: net: snps,dwmac: allow stmmaceth-ocp reset name muhammad.nazim.amirul.nazle.asmade
@ 2026-09-10 8:08 ` netdev-bot+sashiko
0 siblings, 0 replies; 2+ messages in thread
From: netdev-bot+sashiko @ 2026-09-10 8:08 UTC (permalink / raw)
To: muhammad.nazim.amirul.nazle.asmade
Cc: maxime.chevallier, andrew+netdev, davem, edumazet, kuba, pabeni,
robh, krzk+dt, conor+dt, netdev, devicetree, dinguyen,
mcoquelin.stm32, alexandre.torgue, peppe.cavallaro, joabreu,
linux-stm32, linux-arm-kernel, linux-kernel
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.
> 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?
>
> 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.
> 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
^ permalink raw reply [flat|nested] 2+ messages in thread