Messages in this thread | | | Date | Sun, 19 Feb 2023 14:31:33 +0100 | From | Simon Horman <> | Subject | Re: [PATCH] sfc: use IS_ENABLED() checks for CONFIG_SFC_SRIOV |
| |
On Fri, Feb 17, 2023 at 05:19:26PM +0100, Arnd Bergmann wrote: > On Fri, Feb 17, 2023, at 17:13, Edward Cree wrote: > > On 17/02/2023 09:56, Arnd Bergmann wrote: > >> From: Arnd Bergmann <arnd@arndb.de> > >> > >> One local variable has become unused after a recent change: > >> > >> drivers/net/ethernet/sfc/ef100_nic.c: In function 'ef100_probe_netdev_pf': > >> drivers/net/ethernet/sfc/ef100_nic.c:1155:21: error: unused variable 'net_dev' [-Werror=unused-variable] > >> struct net_device *net_dev = efx->net_dev; > >> ^~~~~~~ > >> > >> The variable is still used in an #ifdef. Replace the #ifdef with > >> an if(IS_ENABLED()) check that lets the compiler see where it is > >> used, rather than adding another #ifdef. > > > > So we've had Leon telling us[1] to use __maybe_unused, and you're > > saying to use IS_ENABLED() instead. Which is right? > > (And does it make any difference to build time? I'm assuming the > > compiler is smart enough that this change doesn't affect text > > size...?) > > -ed > > Both are correct, but I prefer the IS_ENABLED() change because it > improves build coverage. The resulting object code should be the > same, as the dead-code-elimination in gcc takes care of removing > it the same way. > > If you use the __maybe_uninitialized annotation, you still need > an extra fix to initialize the ef100_probe_netdev_pf() return > code.
FWIIW, IS_ENABLED() is the approach that is more familiar to me. Though I have nothing in particular against other approaches.
Questions of consistency aside, this patch does look good to me and does appear to address the build problem in question - on x86_64.
Reviewed-by: Simon Horman <simon.horman@corigine.com>
| |