Re: [PATCH 5/5] net: ftgmac100: Allow building on ARM64

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

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] In the FTGMAC100 Kconfig entry
  (drivers/net/ethernet/faraday/Kconfig, line 34), the new condition…

--

net: ftgmac100: Allow building on ARM64

This commit lets the Faraday vendor menu and the FTGMAC100 driver be
selected on ARM64 so AST2700 platforms can use the MAC. It also removes
the !64BIT || BROKEN dependency. The select of MDIO_ASPEED is widened
from MACH_ASPEED_G6 to MACH_ASPEED_G6 || ARCH_ASPEED.

> diff --git a/drivers/net/ethernet/faraday/Kconfig b/drivers/net/ethernet/faraday/Kconfig
> index 474073c7f94d7..dea6dcfc9ab57 100644
> --- a/drivers/net/ethernet/faraday/Kconfig
> +++ b/drivers/net/ethernet/faraday/Kconfig

[ ... ]

> @@ -28,11 +28,10 @@ config FTMAC100
>  
>  config FTGMAC100
>  	tristate "Faraday FTGMAC100 Gigabit Ethernet support"
> -	depends on ARM || COMPILE_TEST
> -	depends on !64BIT || BROKEN
> +	depends on ARM || ARM64 || COMPILE_TEST
>  	select PHYLIB
>  	select FIXED_PHY
> -	select MDIO_ASPEED if MACH_ASPEED_G6
> +	select MDIO_ASPEED if MACH_ASPEED_G6 || ARCH_ASPEED

[Severity: Medium]

Does this select now cover every 32-bit Aspeed generation, not only the
newer SoCs?

On ARM32, arch/arm/mach-aspeed/Kconfig puts all generations under
ARCH_ASPEED:

menuconfig ARCH_ASPEED
	bool "Aspeed BMC architectures"
	...
if ARCH_ASPEED

config MACH_ASPEED_G4
...
config MACH_ASPEED_G5
...
config MACH_ASPEED_G6
...
endif

So MACH_ASPEED_G6 already implies ARCH_ASPEED, and the condition becomes
"if ARCH_ASPEED". The MACH_ASPEED_G6 term has no effect, and MDIO_ASPEED
is now forced on in AST2400/AST2500-only kernels.

Two in-tree configs change this way. aspeed_g4_defconfig has
CONFIG_MACH_ASPEED_G4=y and CONFIG_FTGMAC100=y with no G6, so it now gets
MDIO_ASPEED=y. multi_v5_defconfig (MACH_ASPEED_G4=y, FTGMAC100=m) now
gets MDIO_ASPEED=m. Neither had MDIO_ASPEED before. Also,
mdio-aspeed.c only matches "aspeed,ast2600-mdio", so those kernels build
a driver that never binds.

The MDIO_ASPEED help text in drivers/net/mdio/Kconfig says:

	  AST2500, embedded in the MAC. For legacy reasons, FTGMAC100 driver
	  continues to drive the embedded MDIO controller for the AST2400 and
	  AST2500 SoCs, so say N if AST2600 support is not required.

With this select, users can no longer choose N on those platforms. The
help text also still does not mention AST2700.

The commit message says:

    Keep selecting the Aspeed MDIO controller for MACH_ASPEED_G6 and also
    select it for ARCH_ASPEED. This preserves the existing AST2600
    dependency while covering newer Aspeed SoCs that use the common ARCH
    symbol.

It does not say that older 32-bit AST2400/AST2500 configs are affected
too.

Would a narrower condition keep the old ARM32 behaviour? For example:

	select MDIO_ASPEED if MACH_ASPEED_G6 || (ARM64 && ARCH_ASPEED)

If the wider scope is intended, could the redundant MACH_ASPEED_G6 term
be dropped, and the commit message and MDIO_ASPEED help text updated to
match?

>  	select CRC32
>  	help
>  	  This driver supports the FTGMAC100 Gigabit Ethernet controller

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