Re: [PATCH 1/5] 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] Concern: at this commit, "aspeed,ast2700-mac" is in
  Documentation/devicetree/bindings/net/faraday,ftgmac100.yaml…

--

dt-bindings: net: ftgmac100: Add AST2700 compatible

This commit adds aspeed,ast2700-mac to the compatible list in the FTGMAC100
binding. It also extends the RMII RCLK clock description to cover AST2700,
and it allows the resets property for AST2700 the same way it is allowed
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]
At this commit, ftgmac100_of_match[] in
drivers/net/ethernet/faraday/ftgmac100.c has no entry for
aspeed,ast2700-mac. Would an AST2700 node bind through the
faraday,ftgmac100 fallback and be treated as FTGMAC100_FARADAY?

If it did:

  - is_aspeed would stay false.
  - EDORR/EDOTR would use BIT(15) instead of BIT(30).
  - ftgmac100_setup_clk() would never run.
  - The in-MAC MDIO would be set up.
  - The RMII reset path would be skipped.

Later patches in the series seem to fix this. The patch "net: ftgmac100:
Add AST2700 compatible support" adds:

	{ .compatible = "aspeed,ast2700-mac",
	  .data = &ftgmac100_match_data_ast2700 },

The same patch adds FTGMAC100_AST2700 to the Aspeed branch in
ftgmac100_probe(), which sets BIT(30) and is_aspeed. Separately, "net:
ftgmac100: Add AST2700 upper DMA address support" adds the high address
bits.

No in-tree AST2700 DTS under arch/arm64/boot/dts/aspeed/ has an ethernet
node. So nothing in the tree should be affected between the binding patch
and the driver patch.

One small leftover: this patch changes the binding text to "RMII RCLK
gate for AST2500/2600/2700". The rclk comment in struct ftgmac100 still
says:

	/* AST2500/AST2600 RMII ref clock gate */

Could that comment be updated to include AST2700 too?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-support_ast2700-v1-0-9f03f29d2c82%40aspeedtech.com