* [PATCH] net: tulip: avoid unused variable warning @ 2025-03-09 21:42 deller 2025-03-12 13:14 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: deller @ 2025-03-09 21:42 UTC (permalink / raw) To: netdev, linux-parisc, linux-kernel; +Cc: Helge 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: tulip: avoid unused variable warning 2025-03-09 21:42 [PATCH] net: tulip: avoid unused variable warning deller @ 2025-03-12 13:14 ` Simon Horman 2025-03-12 14:08 ` Helge Deller 0 siblings, 1 reply; 4+ messages in thread From: Simon Horman @ 2025-03-12 13:14 UTC (permalink / raw) To: deller; +Cc: netdev, linux-parisc, linux-kernel, Helge Deller 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); ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] net: tulip: avoid unused variable warning 2025-03-12 13:14 ` Simon Horman @ 2025-03-12 14:08 ` Helge Deller 2025-03-13 10:50 ` Simon Horman 0 siblings, 1 reply; 4+ messages in thread From: Helge Deller @ 2025-03-12 14:08 UTC (permalink / raw) To: Simon Horman, deller; +Cc: netdev, linux-parisc, linux-kernel 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); > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] net: tulip: avoid unused variable warning 2025-03-12 14:08 ` Helge Deller @ 2025-03-13 10:50 ` Simon Horman 0 siblings, 0 replies; 4+ messages in thread From: Simon Horman @ 2025-03-13 10:50 UTC (permalink / raw) To: Helge Deller; +Cc: deller, netdev, linux-parisc, linux-kernel 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-13 10:50 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-03-09 21:42 [PATCH] net: tulip: avoid unused variable warning deller 2025-03-12 13:14 ` Simon Horman 2025-03-12 14:08 ` Helge Deller 2025-03-13 10:50 ` Simon Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).