[PATCH] net: tulip: avoid unused variable warning

[PATCH] net: tulip: avoid unused variable warning

[deller]

From: Helge Deller <deller@gmx.de>

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@gmx.de>
---
 drivers/net/ethernet/dec/tulip/tulip_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index 27e01d780cd0..1ab65deb280c 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -1306,7 +1306,7 @@ static int tulip_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
 	const char *chip_name = tulip_tbl[chip_idx].chip_name;
 	unsigned int eeprom_missing = 0;
 	u8 addr[ETH_ALEN] __aligned(2);
-	unsigned int force_csr0 = 0;
+	unsigned int force_csr0 __maybe_unused = 0;
 
 	board_idx++;
 
-- 
2.47.0

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

[Simon Horman]

On Sun, Mar 09, 2025 at 10:42:38PM +0100, deller@kernel.org wrote:
> From: Helge Deller <deller@gmx.de>
> 
> 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@gmx.de>

Hi Helge,

A few thoughts 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.

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.

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);

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@kernel.org wrote:
>> From: Helge Deller <deller@gmx.de>
>>
>> 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@gmx.de>
>
> 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@gmx.de>

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);
>

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

[Simon Horman]

On Wed, Mar 12, 2025 at 03:08:35PM +0100, Helge Deller wrote:
> On 3/12/25 14:14, Simon Horman wrote:
> > On Sun, Mar 09, 2025 at 10:42:38PM +0100, deller@kernel.org wrote:
> > > From: Helge Deller <deller@gmx.de>
> > > 
> > > 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@gmx.de>
> > 
> > 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@gmx.de>

Thanks, I posted v2 here:

https://lore.kernel.org/netdev/20250313-tulip-w1-v2-1-2ac0d3d909f9@kernel.org/T/#u