Re: [PATCH net] net: macb: drop CONFIG_OF #if block

[Nicolai Buchwitz]

Hi Théo

On August 20, 2026 7:08:20 PM GMT+02:00, "Théo Lebrun" <theo.lebrun@xxxxxxxxxxx> wrote:
>Fix -Wimplicit-function-declaration error on CONFIG_OF=n builds:
>
>   drivers/net/ethernet/cadence/macb_main.c: In function ‘macb_probe’:
>   drivers/net/ethernet/cadence/macb_main.c:5951:15: error: implicit
>   declaration of function ‘macb_alloc_tieoff’ [...]
>    5951 |         err = macb_alloc_tieoff(bp);
>         |               ^~~~~~~~~~~~~~~~~
>   drivers/net/ethernet/cadence/macb_main.c:5973:9: error: implicit
>   declaration of function ‘macb_free_tieoff’ [...]
>    5973 |         macb_free_tieoff(bp);
>         |         ^~~~~~~~~~~~~~~~
>
>Error got introduced because functions are mistakenly declared in a
>`#if defined(CONFIG_OF)` block. Instead of moving functions around,
>avoid any future mistake and drop the block entirely.
>
>Change the module content slightly on CONFIG_OF=n. Previously match
>tables were ignored. Now they appear in the resulting build. This is
>considered trivial in size by most and is the common case:
>
>   ⟩ 18 out of 254 OF net drivers reference CONFIG_OF
>   ⟩ rg -lF 'MODULE_DEVICE_TABLE(of,' drivers/net/ | tee /tmp/a | wc -l
>   254
>   ⟩ xargs -a /tmp/a rg -l CONFIG_OF | wc -l
>   18
>
>Tangent: no, of_match_ptr() does not imply that the compiler can
>optimize out match tables, because MODULE_DEVICE_TABLE(of, ...)
>unconditionally puts the match tables in the binary. It is only meant
>to avoid undefined declaration issues when match tables are hidden
>behind a #ifdef, as was done before. We therefore drop the macro call.
>
>Fixes: 5262eab9462a ("net: macb: allocate tieoff descriptor once across device lifetime")
>Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
>---
>This is a fix to a build issue present on net/main (91ec20351349) and
>linux-next/master(7079a12d7506). The problematic just landed in
>net/main.
>
>Two fixes [0][1] were sent previously (both the same solution):
>[PATCH net] net: macb: Move macb_{alloc,free}_tieoff() out of CONFIG_OF block
>[PATCH net-next] net: macb: fix CONFIG_OF=n build
>
>Instead I'm suggesting [2] we drop the error-prone preprocessor blocks
>and do as everyone else.
>
>[0]: https://lore.kernel.org/netdev/20260818-macb-fix-no-of-build-v1-1-f2a009616384@xxxxxxxxxx/
>[1]: https://lore.kernel.org/netdev/20260819040913.109367-1-running910@xxxxxxxxx/
>[2]: https://lore.kernel.org/netdev/DKT4CBU8YB5G.2W0766H2NU6Y8@xxxxxxxxxxx/
>---
> drivers/net/ethernet/cadence/macb_main.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
>diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>index 1476bce77f34..76ee4f506033 100644
>--- a/drivers/net/ethernet/cadence/macb_main.c
>+++ b/drivers/net/ethernet/cadence/macb_main.c
>@@ -4926,7 +4926,6 @@ static const struct macb_usrio_config at91_default_usrio = {
> 	.clken = MACB_BIT(CLKEN),
> };
> 
>-#if defined(CONFIG_OF)
> /* 1518 rounded up */
> #define AT91ETHER_MAX_RBUFF_SZ	0x600
> /* max number of receive buffers */
>@@ -5754,7 +5753,6 @@ static const struct of_device_id macb_dt_ids[] = {
> 	{ /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, macb_dt_ids);
>-#endif /* CONFIG_OF */
> 
> static const struct macb_config default_gem_config = {
> 	.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
>@@ -6267,7 +6265,7 @@ static struct platform_driver macb_driver = {
> 	.remove		= macb_remove,
> 	.driver		= {
> 		.name		= "macb",
>-		.of_match_table	= of_match_ptr(macb_dt_ids),
>+		.of_match_table	= macb_dt_ids,
> 		.pm	= &macb_pm_ops,
> 	},
> 	.shutdown	= macb_shutdown,
>
>---
>base-commit: 91ec2035134982b98fab0609a9fd8480e8217dc1
>change-id: 20260820-macb-fix-x86-eb78a1fcc63e
>
>Best regards,
>--  
>Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
>

Reviewed-by: Nicolai Buchwitz <nb@xxxxxxxxxxx>

Thanks
Nicolai