Re: [PATCH net] net: macb: drop CONFIG_OF #if block
Nicolai Buchwitz <[email protected]>
| Newsgroups | org.kernel.vger.netdev,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi Théo On August 20, 2026 7:08:20 PM GMT+02:00, "Théo Lebrun" <[email protected]> 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 <[email protected]> >--- >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/[email protected]/ >[1]: https://lore.kernel.org/netdev/[email protected]/ >[2]: https://lore.kernel.org/netdev/[email protected]/ >--- > 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 <[email protected]> > Reviewed-by: Nicolai Buchwitz <[email protected]> Thanks Nicolai