Re: [PATCH net-next v2 1/6] dt-bindings: net: ftgmac100: Add AST2700 compatible

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Already fixed later in this series, so the baseline decision is
  false.

--

dt-bindings: net: ftgmac100: Add AST2700 compatible

This adds aspeed,ast2700-mac to the FTGMAC100 compatible list, which
requires the faraday,ftgmac100 fallback. It also updates the RMII RCLK
clock description to include AST2700, and allows the resets property for
AST2700 in the same way as for AST2600.

> diff --git a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> index d14410018bcf6..bee9570a6f63f 100644
> --- a/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> +++ b/Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml
> @@ -18,6 +18,7 @@ properties:
>                - aspeed,ast2400-mac
>                - aspeed,ast2500-mac
>                - aspeed,ast2600-mac
> +              - aspeed,ast2700-mac
>            - const: faraday,ftgmac100

[Severity: Low]
With this patch, "aspeed,ast2700-mac", "faraday,ftgmac100" becomes a
valid compatible pair. However, at this commit, ftgmac100_of_match[] in
drivers/net/ethernet/faraday/ftgmac100.c has no aspeed,ast2700-mac entry.
Such a node could only bind through the generic faraday,ftgmac100 entry
(FTGMAC100_FARADAY).

On that path, ftgmac100_probe() would do the following:

  - program the ring-end bits at BIT(15) instead of the Aspeed BIT(30)
  - leave is_aspeed false, so ftgmac100_setup_clk() never claims
    MACCLK/RCLK
  - register the embedded MDIO bus through ftgmac100_setup_mdio()
  - never set a DMA mask wider than 32 bits

Would an AST2700 node work correctly if it fell back to the Faraday
match?

Later patches in this series fix this. "net: ftgmac100: Add AST2700
compatible support" adds FTGMAC100_AST2700 and an aspeed,ast2700-mac
match entry, ahead of the faraday fallback. That entry selects the
BIT(30) and is_aspeed path.

"net: ftgmac100: Add AST2700 upper DMA address support" then adds
dma_set_mask_and_coherent(..., DMA_BIT_MASK(35)).

So the problem only shows up when bisecting to this commit, and only with
an out-of-tree AST2700 DTS on an ARM64 build. ARM64 builds are only
enabled by "net: ftgmac100: Allow building on ARM64". Landing the binding
before the driver match is the usual order for dt-bindings series.

[ ... ]

^ annotation: none needed for the [ ... ] above; snipped hunks are the RCLK description and the resets allOf block.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261006-support_ast2700-v2-0-36de51fb8066%40aspeedtech.com