Re: [PATCH] net: tulip: avoid unused variable warning

[Helge Deller]

On 3/12/25 14:14, Simon Horman wrote:
> On Sun, Mar 09, 2025 at 10:42:38PM +0100, deller@xxxxxxxxxx wrote:
> > From: Helge Deller <deller@xxxxxx>
> >
> > When compiling with W=1 and CONFIG_TULIP_MWI=n one gets this warning:
> >   drivers/net/ethernet/dec/tulip/tulip_core.c: In function ‘tulip_init_one’:
> >   drivers/net/ethernet/dec/tulip/tulip_core.c:1309:22: warning: variable ‘force_csr0’ set but not used
> >
> > Avoid it by annotating the variable __maybe_unused, which seems to be
> > the easiest solution.
> >
> > Signed-off-by: Helge Deller <deller@xxxxxx>
>
> Hi Helge,
>
> A few thoughts on this:

Hi Simon,

Thanks for following up on this!

> Firstly, thanks for your patch, which I agree addresses the problem you
> have described.
>
> However, AFAIK, this is a rather old driver and I'm not sure that
> addressing somewhat cosmetic problems are worth the churn they cause:
> maybe it's best to leave it be.

Well, the only reason why I sent this patch is, because some people
are interested to get a Linux kernel build without any warnings when "W=1"
option is enabled.
This code in the tulip driver is one of the last 10 places in the kernel where
I see a warning at all, so I think it's worth fixing it, although it's just
cosmetic.


> But if we do want to fix this problem, I do wonder if the following
> solution, which leverages IS_ENABLED, is somehow nicer as
> it slightly reduces the amount of conditionally compiled code,
> thus increasing compile test coverage.

Full Ack from my side!
I wanted to keep my patch small, but your proposed patch is the better one.

I did not compile-test it, but if it builds you may add my:
Acked-by: Helge Deller <deller@xxxxxx>

Helge

> diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
> index 27e01d780cd0..75eac18ff246 100644
> --- a/drivers/net/ethernet/dec/tulip/tulip_core.c
> +++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
> @@ -1177,7 +1177,6 @@ static void set_rx_mode(struct net_device *dev)
>   	iowrite32(csr6, ioaddr + CSR6);
>   }
>
> -#ifdef CONFIG_TULIP_MWI
>   static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>   {
>   	struct tulip_private *tp = netdev_priv(dev);
> @@ -1251,7 +1250,6 @@ static void tulip_mwi_config(struct pci_dev *pdev, struct net_device *dev)
>   		netdev_dbg(dev, "MWI config cacheline=%d, csr0=%08x\n",
>   			   cache, csr0);
>   }
> -#endif
>
>   /*
>    *	Chips that have the MRM/reserved bit quirk and the burst quirk. That
> @@ -1463,10 +1461,9 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
>
>   	INIT_WORK(&tp->media_work, tulip_tbl[tp->chip_id].media_task);
>
> -#ifdef CONFIG_TULIP_MWI
> -	if (!force_csr0 && (tp->flags & HAS_PCI_MWI))
> +	if (IS_ENABLED(CONFIG_TULIP_MWI) && !force_csr0 &&
> +	    (tp->flags & HAS_PCI_MWI))
>   		tulip_mwi_config (pdev, dev);
> -#endif
>
>   	/* Stop the chip's Tx and Rx processes. */
>   	tulip_stop_rxtx(tp);