Re: [PATCH net-next 1/2] dt-bindings: net: altr,socfpga-stmmac: Add altr,socfpga-stmmac-agilex5-tsn compatible

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] The binding patch makes the new compatible "altr,socfpga-
  stmmac-agilex5-tsn" a peer of "altr,socfpga-stmmac-agilex5" in the…
- [Low] At this commit, socfpga_dwmac_match in
  drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c has no entry for…

--

dt-bindings: net: altr,socfpga-stmmac: Add altr,socfpga-stmmac-agilex5-tsn compatible

This adds the altr,socfpga-stmmac-agilex5-tsn compatible for the Agilex5
gmac1 port, which has an FPGA GMII-to-RGMII converter in front of the PHY.
The new string goes in the same enum as altr,socfpga-stmmac-agilex5, with
snps,dwxgmac-2.10 as the fallback. Plain agilex5 is intentionally not used
as a fallback.

[ ... ]

[Severity: Medium]
Should the allOf conditional further down in this file also cover the new
compatible? It still matches only the plain agilex5 string:

  - if:
      properties:
        compatible:
          contains:
            const: altr,socfpga-stmmac-agilex5
    then:
      properties:
        interrupts:
          minItems: 17

        interrupt-names:
          minItems: 17

        dma-coherent: true
    else:
      properties:
        interrupts:
          maxItems: 1

        interrupt-names:
          maxItems: 1

        dma-coherent: false

The TSN port deliberately does not list agilex5 as a fallback. A node with
compatible = "altr,socfpga-stmmac-agilex5-tsn", "snps,dwxgmac-2.10] will
therefore never match the if. It falls into the else branch, which holds
the Cyclone5/Arria10/Stratix10 limits.

The TSN board would override the gmac1: ethernet@10820000 node in
arch/arm64/boot/dts/intel/socfpga_agilex5.dtsi. That node has 17
interrupts (macirq, tx-queue-0..7, rx-queue-0..7) and dma-coherent.

If a board DTS switches gmac1 to the TSN compatible, wouldn't it fail
dtbs_check even though it describes the hardware correctly?

In the companion driver patch, socfpga_agilex5_tsn_gmac_setup_plat_dat()
calls socfpga_agilex5_setup_plat_dat() and changes only mac_interface.
That suggests the MAC-side constraints match plain agilex5. Would
something like this work instead:

          contains:
            enum:
              - altr,socfpga-stmmac-agilex5
              - altr,socfpga-stmmac-agilex5-tsn

[Severity: Low]
At this commit, socfpga_dwmac_match in
drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c has no entry for
altr,socfpga-stmmac-agilex5-tsn.

Would a node using the new compatible bind through the
snps,dwxgmac-2.10 entry in dwmac_generic_match instead? That path would
skip the SoCFPGA sysmgr, reset and platform data setup.

The next patch in the series, "net: stmmac: dwmac-socfpga: Add support
for Agilex5 TSN GMAC with FPGA converter", adds the match entry. No
in-tree DTS uses the new compatible before that patch.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923021823.46649-1-muhammad.nazim.amirul.nazle.asmade%40altera.com