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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3168 bytes --]

On Sat, Oct 10, 2026 at 07:58:38AM +0000, netdev-bot+sashiko@kernel.org wrote:
> 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?

Sounds like no, can the submitter please clarify if the fallback is
valid?

> 
> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]