Message ID | 20230808-i2c-amd_static-v1-1-1902f608bba1@google.com |
---|---|
State | Changes Requested |
Headers | show |
Series | i2c/busses: fix -Wmissing-variable-declarations | expand |
Hi Nick, On Tue, Aug 08, 2023 at 09:56:16AM -0700, Nick Desaulniers wrote: > I'm looking to enable -Wmissing-variable-declarations behind W=1. 0day > bot spotted the following instance: > > drivers/i2c/busses/i2c-amd756.c:286:20: warning: no previous extern > declaration for non-static variable 'amd756_smbus' > [-Wmissing-variable-declarations] > 286 | struct i2c_adapter amd756_smbus = { > | ^ > drivers/i2c/busses/i2c-amd756.c:286:1: note: declare 'static' if the > variable is not intended to be used outside of this translation unit > 286 | struct i2c_adapter amd756_smbus = { > | ^ > > This symbol is referenced by more than one translation unit, so create > then include the correct header for their declarations. > > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/ > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> You might want to have a Fixes tag here and Cc: Jean Delvare <jdelvare@suse.com> [...] > --- /dev/null > +++ b/drivers/i2c/busses/i2c-amd756.h > @@ -0,0 +1,3 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ Please, leave a space here. And you might also want to have something like: #ifndef __I2C_AMD_756_H__ #define __I2C_AMD_756_H__ > +#include <linux/i2c.h> space here. > +extern struct i2c_adapter amd756_smbus; #endif /* __I2C_AMD_756_H__ */ Jean, any opinion on this patch, I don't really know this driver, but is there a way to avoid this extern declaration. Thanks, Andi
Hi Andi, Nick, On Wed, 09 Aug 2023 21:13:10 +0200, Andi Shyti wrote: > On Tue, Aug 08, 2023 at 09:56:16AM -0700, Nick Desaulniers wrote: > > I'm looking to enable -Wmissing-variable-declarations behind W=1. 0day > > bot spotted the following instance: > > > > drivers/i2c/busses/i2c-amd756.c:286:20: warning: no previous extern > > declaration for non-static variable 'amd756_smbus' > > [-Wmissing-variable-declarations] > > 286 | struct i2c_adapter amd756_smbus = { > > | ^ > > drivers/i2c/busses/i2c-amd756.c:286:1: note: declare 'static' if the > > variable is not intended to be used outside of this translation unit > > 286 | struct i2c_adapter amd756_smbus = { > > | ^ > > > > This symbol is referenced by more than one translation unit, so create > > then include the correct header for their declarations. > > > > Reported-by: kernel test robot <lkp@intel.com> > > Closes: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/ > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > > You might want to have a Fixes tag here and > > Cc: Jean Delvare <jdelvare@suse.com> Fixes tag would cause unnecessary worry, with people backporting the patch while it doesn't actually fix anything. No need for that. > > [...] > > > --- /dev/null > > +++ b/drivers/i2c/busses/i2c-amd756.h > > @@ -0,0 +1,3 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > Please, leave a space here. > > And you might also want to have something like: > > #ifndef __I2C_AMD_756_H__ > #define __I2C_AMD_756_H__ > > > +#include <linux/i2c.h> > > space here. > > > +extern struct i2c_adapter amd756_smbus; > > #endif /* __I2C_AMD_756_H__ */ > > Jean, any opinion on this patch, I don't really know this driver, > but is there a way to avoid this extern declaration. Thanks for your review. I would personally not bother with a header file, this is unnecessary burden. Just add the extern declaration to i2c-amd756.c as was already done in i2c-amd756-s4882.c. The whole thing is a horrible hack anyway and there won't be more occurrences of it, so the more simple it stays, the better. Some history: I wrote the pseudo-driver i2c-amd756-s4882 when the i2c-core did not yet support multiplexing. Nowadays, proper support would be implemented using i2c-mux-* and i2c-amd756-s4882.c would go away entirely. Actually, I very much doubt any of these 2007 Tyan server boards is still in activity today, so maybe we should just delete the driver. Note that i2c-nforce2.c will need the same fix as struct i2c_adapter nforce2_smbus is extern as well.
diff --git a/drivers/i2c/busses/i2c-amd756-s4882.c b/drivers/i2c/busses/i2c-amd756-s4882.c index 063274388a75..8156cfc43df3 100644 --- a/drivers/i2c/busses/i2c-amd756-s4882.c +++ b/drivers/i2c/busses/i2c-amd756-s4882.c @@ -26,8 +26,7 @@ #include <linux/init.h> #include <linux/i2c.h> #include <linux/mutex.h> - -extern struct i2c_adapter amd756_smbus; +#include "i2c-amd756.h" static struct i2c_adapter *s4882_adapter; static struct i2c_algorithm *s4882_algo; diff --git a/drivers/i2c/busses/i2c-amd756.c b/drivers/i2c/busses/i2c-amd756.c index ef1307a258e9..af77374d2ab3 100644 --- a/drivers/i2c/busses/i2c-amd756.c +++ b/drivers/i2c/busses/i2c-amd756.c @@ -31,6 +31,7 @@ #include <linux/i2c.h> #include <linux/acpi.h> #include <linux/io.h> +#include "i2c-amd756.h" /* AMD756 SMBus address offsets */ #define SMB_ADDR_OFFSET 0xE0 diff --git a/drivers/i2c/busses/i2c-amd756.h b/drivers/i2c/busses/i2c-amd756.h new file mode 100644 index 000000000000..88698266d6d8 --- /dev/null +++ b/drivers/i2c/busses/i2c-amd756.h @@ -0,0 +1,3 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#include <linux/i2c.h> +extern struct i2c_adapter amd756_smbus;
I'm looking to enable -Wmissing-variable-declarations behind W=1. 0day bot spotted the following instance: drivers/i2c/busses/i2c-amd756.c:286:20: warning: no previous extern declaration for non-static variable 'amd756_smbus' [-Wmissing-variable-declarations] 286 | struct i2c_adapter amd756_smbus = { | ^ drivers/i2c/busses/i2c-amd756.c:286:1: note: declare 'static' if the variable is not intended to be used outside of this translation unit 286 | struct i2c_adapter amd756_smbus = { | ^ This symbol is referenced by more than one translation unit, so create then include the correct header for their declarations. Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/llvm/202308081000.tTL1ElTr-lkp@intel.com/ Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- drivers/i2c/busses/i2c-amd756-s4882.c | 3 +-- drivers/i2c/busses/i2c-amd756.c | 1 + drivers/i2c/busses/i2c-amd756.h | 3 +++ 3 files changed, 5 insertions(+), 2 deletions(-) --- base-commit: 14f9643dc90adea074a0ffb7a17d337eafc6a5cc change-id: 20230808-i2c-amd_static-81e5c27a84ce Best regards,