| Message ID | 20201104162427.2984742-7-lee.jones@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Rid W=1 issues from Input | expand |
Hi Lee, On Wed, Nov 04, 2020 at 04:24:13PM +0000, Lee Jones wrote: > Some drivers which include 'elan-i2c-ids.h' make use of > 'elan_acpi_id', but not all do. Tell the compiler that this is > expected behaviour. > > Fixes the following W=1 kernel build warning(s): > > include/linux/input/elan-i2c-ids.h:26:36: warning: ‘elan_acpi_id’ defined but not used [-Wunused-const-variable=] > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > Cc: dusonlin@emc.com.tw > Cc: KT Liao <kt.liao@emc.com.tw> > Cc: linux-input@vger.kernel.org > Signed-off-by: Lee Jones <lee.jones@linaro.org> > --- > include/linux/input/elan-i2c-ids.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h > index 520858d126808..b6976d99b6b75 100644 > --- a/include/linux/input/elan-i2c-ids.h > +++ b/include/linux/input/elan-i2c-ids.h > @@ -23,7 +23,7 @@ > > #include <linux/mod_devicetable.h> > > -static const struct acpi_device_id elan_acpi_id[] = { > +static const struct acpi_device_id __maybe_unused elan_acpi_id[] = { > { "ELAN0000", 0 }, > { "ELAN0100", 0 }, > { "ELAN0600", 0 }, I think I'd prefer something like this instead: diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c index c599e21a8478..65d21a050cea 100644 --- a/drivers/input/mouse/elan_i2c_core.c +++ b/drivers/input/mouse/elan_i2c_core.c @@ -34,7 +34,6 @@ #include <linux/completion.h> #include <linux/of.h> #include <linux/property.h> -#include <linux/input/elan-i2c-ids.h> #include <linux/regulator/consumer.h> #include <asm/unaligned.h> @@ -1413,6 +1412,7 @@ static const struct i2c_device_id elan_id[] = { MODULE_DEVICE_TABLE(i2c, elan_id); #ifdef CONFIG_ACPI +#include <linux/input/elan-i2c-ids.h> MODULE_DEVICE_TABLE(acpi, elan_acpi_id); #endif Thanks.
On Wed, 04 Nov 2020, Dmitry Torokhov wrote: > Hi Lee, > > On Wed, Nov 04, 2020 at 04:24:13PM +0000, Lee Jones wrote: > > Some drivers which include 'elan-i2c-ids.h' make use of > > 'elan_acpi_id', but not all do. Tell the compiler that this is > > expected behaviour. > > > > Fixes the following W=1 kernel build warning(s): > > > > include/linux/input/elan-i2c-ids.h:26:36: warning: ‘elan_acpi_id’ defined but not used [-Wunused-const-variable=] > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > Cc: dusonlin@emc.com.tw > > Cc: KT Liao <kt.liao@emc.com.tw> > > Cc: linux-input@vger.kernel.org > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > --- > > include/linux/input/elan-i2c-ids.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h > > index 520858d126808..b6976d99b6b75 100644 > > --- a/include/linux/input/elan-i2c-ids.h > > +++ b/include/linux/input/elan-i2c-ids.h > > @@ -23,7 +23,7 @@ > > > > #include <linux/mod_devicetable.h> > > > > -static const struct acpi_device_id elan_acpi_id[] = { > > +static const struct acpi_device_id __maybe_unused elan_acpi_id[] = { > > { "ELAN0000", 0 }, > > { "ELAN0100", 0 }, > > { "ELAN0600", 0 }, > > I think I'd prefer something like this instead: > > diff --git a/drivers/input/mouse/elan_i2c_core.c > b/drivers/input/mouse/elan_i2c_core.c > index c599e21a8478..65d21a050cea 100644 > --- a/drivers/input/mouse/elan_i2c_core.c > +++ b/drivers/input/mouse/elan_i2c_core.c > @@ -34,7 +34,6 @@ > #include <linux/completion.h> > #include <linux/of.h> > #include <linux/property.h> > -#include <linux/input/elan-i2c-ids.h> > #include <linux/regulator/consumer.h> > #include <asm/unaligned.h> > > @@ -1413,6 +1412,7 @@ static const struct i2c_device_id elan_id[] = { > MODULE_DEVICE_TABLE(i2c, elan_id); > > #ifdef CONFIG_ACPI > +#include <linux/input/elan-i2c-ids.h> > MODULE_DEVICE_TABLE(acpi, elan_acpi_id); > #endif Moving an #include file to the bottom of a source file, really? I can do as you wish, but it's a pretty 'interesting' solution. :)
On Thu, Nov 05, 2020 at 09:13:05AM +0000, Lee Jones wrote: > On Wed, 04 Nov 2020, Dmitry Torokhov wrote: > > > Hi Lee, > > > > On Wed, Nov 04, 2020 at 04:24:13PM +0000, Lee Jones wrote: > > > Some drivers which include 'elan-i2c-ids.h' make use of > > > 'elan_acpi_id', but not all do. Tell the compiler that this is > > > expected behaviour. > > > > > > Fixes the following W=1 kernel build warning(s): > > > > > > include/linux/input/elan-i2c-ids.h:26:36: warning: ‘elan_acpi_id’ defined but not used [-Wunused-const-variable=] > > > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > Cc: dusonlin@emc.com.tw > > > Cc: KT Liao <kt.liao@emc.com.tw> > > > Cc: linux-input@vger.kernel.org > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > --- > > > include/linux/input/elan-i2c-ids.h | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h > > > index 520858d126808..b6976d99b6b75 100644 > > > --- a/include/linux/input/elan-i2c-ids.h > > > +++ b/include/linux/input/elan-i2c-ids.h > > > @@ -23,7 +23,7 @@ > > > > > > #include <linux/mod_devicetable.h> > > > > > > -static const struct acpi_device_id elan_acpi_id[] = { > > > +static const struct acpi_device_id __maybe_unused elan_acpi_id[] = { > > > { "ELAN0000", 0 }, > > > { "ELAN0100", 0 }, > > > { "ELAN0600", 0 }, > > > > I think I'd prefer something like this instead: > > > > diff --git a/drivers/input/mouse/elan_i2c_core.c > > b/drivers/input/mouse/elan_i2c_core.c > > index c599e21a8478..65d21a050cea 100644 > > --- a/drivers/input/mouse/elan_i2c_core.c > > +++ b/drivers/input/mouse/elan_i2c_core.c > > @@ -34,7 +34,6 @@ > > #include <linux/completion.h> > > #include <linux/of.h> > > #include <linux/property.h> > > -#include <linux/input/elan-i2c-ids.h> > > #include <linux/regulator/consumer.h> > > #include <asm/unaligned.h> > > > > @@ -1413,6 +1412,7 @@ static const struct i2c_device_id elan_id[] = { > > MODULE_DEVICE_TABLE(i2c, elan_id); > > > > #ifdef CONFIG_ACPI > > +#include <linux/input/elan-i2c-ids.h> > > MODULE_DEVICE_TABLE(acpi, elan_acpi_id); > > #endif > > Moving an #include file to the bottom of a source file, really? > > I can do as you wish, but it's a pretty 'interesting' solution. :) I see absolutely nothing wrong with it, and if you check kernel sources you will see #includes used this way. What is your concern? Thanks.
On Thu, 05 Nov 2020, Dmitry Torokhov wrote: > On Thu, Nov 05, 2020 at 09:13:05AM +0000, Lee Jones wrote: > > On Wed, 04 Nov 2020, Dmitry Torokhov wrote: > > > > > Hi Lee, > > > > > > On Wed, Nov 04, 2020 at 04:24:13PM +0000, Lee Jones wrote: > > > > Some drivers which include 'elan-i2c-ids.h' make use of > > > > 'elan_acpi_id', but not all do. Tell the compiler that this is > > > > expected behaviour. > > > > > > > > Fixes the following W=1 kernel build warning(s): > > > > > > > > include/linux/input/elan-i2c-ids.h:26:36: warning: ‘elan_acpi_id’ defined but not used [-Wunused-const-variable=] > > > > > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> > > > > Cc: dusonlin@emc.com.tw > > > > Cc: KT Liao <kt.liao@emc.com.tw> > > > > Cc: linux-input@vger.kernel.org > > > > Signed-off-by: Lee Jones <lee.jones@linaro.org> > > > > --- > > > > include/linux/input/elan-i2c-ids.h | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h > > > > index 520858d126808..b6976d99b6b75 100644 > > > > --- a/include/linux/input/elan-i2c-ids.h > > > > +++ b/include/linux/input/elan-i2c-ids.h > > > > @@ -23,7 +23,7 @@ > > > > > > > > #include <linux/mod_devicetable.h> > > > > > > > > -static const struct acpi_device_id elan_acpi_id[] = { > > > > +static const struct acpi_device_id __maybe_unused elan_acpi_id[] = { > > > > { "ELAN0000", 0 }, > > > > { "ELAN0100", 0 }, > > > > { "ELAN0600", 0 }, > > > > > > I think I'd prefer something like this instead: > > > > > > diff --git a/drivers/input/mouse/elan_i2c_core.c > > > b/drivers/input/mouse/elan_i2c_core.c > > > index c599e21a8478..65d21a050cea 100644 > > > --- a/drivers/input/mouse/elan_i2c_core.c > > > +++ b/drivers/input/mouse/elan_i2c_core.c > > > @@ -34,7 +34,6 @@ > > > #include <linux/completion.h> > > > #include <linux/of.h> > > > #include <linux/property.h> > > > -#include <linux/input/elan-i2c-ids.h> > > > #include <linux/regulator/consumer.h> > > > #include <asm/unaligned.h> > > > > > > @@ -1413,6 +1412,7 @@ static const struct i2c_device_id elan_id[] = { > > > MODULE_DEVICE_TABLE(i2c, elan_id); > > > > > > #ifdef CONFIG_ACPI > > > +#include <linux/input/elan-i2c-ids.h> > > > MODULE_DEVICE_TABLE(acpi, elan_acpi_id); > > > #endif > > > > Moving an #include file to the bottom of a source file, really? > > > > I can do as you wish, but it's a pretty 'interesting' solution. :) > > I see absolutely nothing wrong with it, and if you check kernel sources > you will see #includes used this way. > > What is your concern? I guess just the unfamiliarity of it. It's the first time I've seen such a solution. Will fix.
diff --git a/include/linux/input/elan-i2c-ids.h b/include/linux/input/elan-i2c-ids.h index 520858d126808..b6976d99b6b75 100644 --- a/include/linux/input/elan-i2c-ids.h +++ b/include/linux/input/elan-i2c-ids.h @@ -23,7 +23,7 @@ #include <linux/mod_devicetable.h> -static const struct acpi_device_id elan_acpi_id[] = { +static const struct acpi_device_id __maybe_unused elan_acpi_id[] = { { "ELAN0000", 0 }, { "ELAN0100", 0 }, { "ELAN0600", 0 },
Some drivers which include 'elan-i2c-ids.h' make use of 'elan_acpi_id', but not all do. Tell the compiler that this is expected behaviour. Fixes the following W=1 kernel build warning(s): include/linux/input/elan-i2c-ids.h:26:36: warning: ‘elan_acpi_id’ defined but not used [-Wunused-const-variable=] Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com> Cc: dusonlin@emc.com.tw Cc: KT Liao <kt.liao@emc.com.tw> Cc: linux-input@vger.kernel.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- include/linux/input/elan-i2c-ids.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)