* [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused
@ 2025-02-25 14:53 Arnd Bergmann
2025-02-25 15:47 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2025-02-25 14:53 UTC (permalink / raw)
To: Dmitry Torokhov, Maxime Coquelin, Alexandre Torgue
Cc: Krzysztof Kozlowski, Arnd Bergmann, Yu Jiaoliang, Oliver Graute,
Uwe Kleine-König, linux-input, linux-stm32, linux-arm-kernel,
linux-kernel
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
When compile tested with W=1 on x86_64 with driver as built-in:
stmpe-ts.c:371:34: error: unused variable 'stmpe_ts_ids' [-Werror,-Wunused-const-variable]
Ideally this would be referenced from the platform_driver, but since
the compatible string is already matched by the mfd driver for its
parent device, that would break probing.
In this case, the of_device_id table just serves as a module alias
for loading the driver, while the device itself is probed using
the platform device name.
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Link: https://lore.kernel.org/lkml/20240403080702.3509288-8-arnd@kernel.org/
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/input/touchscreen/stmpe-ts.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c
index a94a1997f96b..3900aa2e3a90 100644
--- a/drivers/input/touchscreen/stmpe-ts.c
+++ b/drivers/input/touchscreen/stmpe-ts.c
@@ -366,7 +366,7 @@ static struct platform_driver stmpe_ts_driver = {
};
module_platform_driver(stmpe_ts_driver);
-static const struct of_device_id stmpe_ts_ids[] = {
+static const struct of_device_id stmpe_ts_ids[] __maybe_unused = {
{ .compatible = "st,stmpe-ts", },
{ },
};
--
2.39.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused
2025-02-25 14:53 [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused Arnd Bergmann
@ 2025-02-25 15:47 ` Uwe Kleine-König
2025-02-25 16:25 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2025-02-25 15:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Torokhov, Maxime Coquelin, Alexandre Torgue,
Krzysztof Kozlowski, Arnd Bergmann, Yu Jiaoliang, Oliver Graute,
linux-input, linux-stm32, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2412 bytes --]
On Tue, Feb 25, 2025 at 03:53:26PM +0100, Arnd Bergmann wrote:
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>
> When compile tested with W=1 on x86_64 with driver as built-in:
>
> stmpe-ts.c:371:34: error: unused variable 'stmpe_ts_ids' [-Werror,-Wunused-const-variable]
>
> Ideally this would be referenced from the platform_driver, but since
> the compatible string is already matched by the mfd driver for its
> parent device, that would break probing.
>
> In this case, the of_device_id table just serves as a module alias
> for loading the driver, while the device itself is probed using
> the platform device name.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Link: https://lore.kernel.org/lkml/20240403080702.3509288-8-arnd@kernel.org/
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/input/touchscreen/stmpe-ts.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c
> index a94a1997f96b..3900aa2e3a90 100644
> --- a/drivers/input/touchscreen/stmpe-ts.c
> +++ b/drivers/input/touchscreen/stmpe-ts.c
> @@ -366,7 +366,7 @@ static struct platform_driver stmpe_ts_driver = {
> };
> module_platform_driver(stmpe_ts_driver);
>
> -static const struct of_device_id stmpe_ts_ids[] = {
> +static const struct of_device_id stmpe_ts_ids[] __maybe_unused = {
> { .compatible = "st,stmpe-ts", },
> { },
> };
Following this we have
MODULE_DEVICE_TABLE(of, stmpe_ts_ids);
.
With
diff --git a/include/linux/module.h b/include/linux/module.h
index 30e5b19bafa9..014f033ef1ba 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -250,7 +250,8 @@ extern void cleanup_module(void);
extern typeof(name) __mod_device_table__##type##__##name \
__attribute__ ((unused, alias(__stringify(name))))
#else /* !MODULE */
-#define MODULE_DEVICE_TABLE(type, name)
+#define MODULE_DEVICE_TABLE(type, name) \
+static const typeof(name) *__mod_device_table__##type##__##name##_ptr __attribute__((unused)) = &(name)
#endif
/* Version of form [<epoch>:]<version>[-<extra-version>].
the warning goes away and stmpe_ts_ids isn't included in the .o file
without having to add __maybe_unused to the driver.
I would consider that a superior approach.
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused
2025-02-25 15:47 ` Uwe Kleine-König
@ 2025-02-25 16:25 ` Arnd Bergmann
2025-02-25 20:17 ` Uwe Kleine-König
0 siblings, 1 reply; 5+ messages in thread
From: Arnd Bergmann @ 2025-02-25 16:25 UTC (permalink / raw)
To: Uwe Kleine-König, Arnd Bergmann
Cc: Dmitry Torokhov, Maxime Coquelin, Alexandre Torgue,
Krzysztof Kozlowski, Yu Jiaoliang, Oliver Graute, linux-input,
linux-stm32, linux-arm-kernel, linux-kernel
On Tue, Feb 25, 2025, at 16:47, Uwe Kleine-König wrote:
> On Tue, Feb 25, 2025 at 03:53:26PM +0100, Arnd Bergmann wrote:
>> diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/
>
> With
>
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 30e5b19bafa9..014f033ef1ba 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -250,7 +250,8 @@ extern void cleanup_module(void);
> extern typeof(name) __mod_device_table__##type##__##name \
> __attribute__ ((unused, alias(__stringify(name))))
> #else /* !MODULE */
> -#define MODULE_DEVICE_TABLE(type, name)
> +#define MODULE_DEVICE_TABLE(type, name) \
> +static const typeof(name) *__mod_device_table__##type##__##name##_ptr
> __attribute__((unused)) = &(name)
> #endif
>
> /* Version of form [<epoch>:]<version>[-<extra-version>].
>
> the warning goes away and stmpe_ts_ids isn't included in the .o file
> without having to add __maybe_unused to the driver.
>
> I would consider that a superior approach.
Not sure, I can see how this avoids some warnings, but this is
currently the only remaining instance of this problem (I fixed
another two recently), and in most cases a MODULE_DEVICE_TABLE()
entry that is completely unused ends up pointing to a real bug,
where there is a table but it's not also part of the
device_driver definition.
Arnd
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused
2025-02-25 16:25 ` Arnd Bergmann
@ 2025-02-25 20:17 ` Uwe Kleine-König
2025-02-25 21:27 ` Arnd Bergmann
0 siblings, 1 reply; 5+ messages in thread
From: Uwe Kleine-König @ 2025-02-25 20:17 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Arnd Bergmann, Dmitry Torokhov, Maxime Coquelin, Alexandre Torgue,
Krzysztof Kozlowski, Yu Jiaoliang, Oliver Graute, linux-input,
linux-stm32, linux-arm-kernel, linux-kernel
[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]
Hello Arnd,
On Tue, Feb 25, 2025 at 05:25:05PM +0100, Arnd Bergmann wrote:
> On Tue, Feb 25, 2025, at 16:47, Uwe Kleine-König wrote:
> > On Tue, Feb 25, 2025 at 03:53:26PM +0100, Arnd Bergmann wrote:
> >> diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/
> >
> > With
> >
> > diff --git a/include/linux/module.h b/include/linux/module.h
> > index 30e5b19bafa9..014f033ef1ba 100644
> > --- a/include/linux/module.h
> > +++ b/include/linux/module.h
> > @@ -250,7 +250,8 @@ extern void cleanup_module(void);
> > extern typeof(name) __mod_device_table__##type##__##name \
> > __attribute__ ((unused, alias(__stringify(name))))
> > #else /* !MODULE */
> > -#define MODULE_DEVICE_TABLE(type, name)
> > +#define MODULE_DEVICE_TABLE(type, name) \
> > +static const typeof(name) *__mod_device_table__##type##__##name##_ptr
> > __attribute__((unused)) = &(name)
> > #endif
> >
> > /* Version of form [<epoch>:]<version>[-<extra-version>].
Hu?
> > the warning goes away and stmpe_ts_ids isn't included in the .o file
> > without having to add __maybe_unused to the driver.
> >
> > I would consider that a superior approach.
>
> Not sure, I can see how this avoids some warnings, but this is
> currently the only remaining instance of this problem (I fixed
> another two recently), and in most cases a MODULE_DEVICE_TABLE()
> entry that is completely unused ends up pointing to a real bug,
> where there is a table but it's not also part of the
> device_driver definition.
It might be the only instance without __maybe_unused and so triggering a
warning. But there is also:
$ git grep -E 'of_device_id.*__maybe_unused' | wc -l
231
$ git grep -E 'mdio_device_id.*__maybe_unused' | wc -l
58
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused
2025-02-25 20:17 ` Uwe Kleine-König
@ 2025-02-25 21:27 ` Arnd Bergmann
0 siblings, 0 replies; 5+ messages in thread
From: Arnd Bergmann @ 2025-02-25 21:27 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Arnd Bergmann, Dmitry Torokhov, Maxime Coquelin, Alexandre Torgue,
Krzysztof Kozlowski, Yu Jiaoliang, Oliver Graute, linux-input,
linux-stm32, linux-arm-kernel, linux-kernel
On Tue, Feb 25, 2025, at 21:17, Uwe Kleine-König wrote:
> On Tue, Feb 25, 2025 at 05:25:05PM +0100, Arnd Bergmann wrote:
>> On Tue, Feb 25, 2025, at 16:47, Uwe Kleine-König wrote:
>> > On Tue, Feb 25, 2025 at 03:53:26PM +0100, Arnd Bergmann wrote:
>> > the warning goes away and stmpe_ts_ids isn't included in the .o file
>> > without having to add __maybe_unused to the driver.
>> >
>> > I would consider that a superior approach.
>>
>> Not sure, I can see how this avoids some warnings, but this is
>> currently the only remaining instance of this problem (I fixed
>> another two recently), and in most cases a MODULE_DEVICE_TABLE()
>> entry that is completely unused ends up pointing to a real bug,
>> where there is a table but it's not also part of the
>> device_driver definition.
>
> It might be the only instance without __maybe_unused and so triggering a
> warning. But there is also:
>
> $ git grep -E 'of_device_id.*__maybe_unused' | wc -l
> 231
>
> $ git grep -E 'mdio_device_id.*__maybe_unused' | wc -l
> 58
I'm not really worried about these at the moment, other than not
wanting to pile on to that mess with more __maybe_unused
annotations.
My goal here is to get the point of enabling -Wunused-const-variable
by default in order to find other bugs before they make it into
the kernel.
Andy Shevchenko really wants to remove the of_match_ptr()
macro so we can stop adding pointless __maybe_unused annotations
for every driver that accidentally uses of_match_ptr(). This
is certainly a good idea as well, just not what I'm trying to
do this time.
Apparently we have already accumulated a bunch of drivers that
ended up with __maybe_unused but no actual reference from
of_match_ptr():
$ git grep -l 'of_device_id.*__maybe_unused' |xargs grep -Lw of_match_ptr | wc -l
but only a couple of drivers that don't use of_match_ptr()
or of_match_node():
$ git grep -l 'of_device_id.*__maybe_unused' |xargs grep -Lw 'of_match_table\|of_match_node'
drivers/cpufreq/armada-37xx-cpufreq.c
drivers/cpufreq/armada-8k-cpufreq.c
drivers/cpufreq/highbank-cpufreq.c
drivers/cpufreq/sti-cpufreq.c
drivers/hwmon/isl28022.c
drivers/input/touchscreen/stmpe-ts.c
drivers/mfd/twl6030-irq.c
drivers/tty/serial/sc16is7xx.c
I do think it makes sense to change of_match_node() to have a
reference to its arguments, as in the patch below. That
probably needs a few extra fixups.
Arnd
diff --git a/include/linux/of.h b/include/linux/of.h
index 9d6b8a61607f..83cfa6c26ee4 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -907,7 +907,11 @@ static inline const void *of_device_get_match_data(const struct device *dev)
}
#define of_match_ptr(_ptr) NULL
-#define of_match_node(_matches, _node) NULL
+static inline const struct of_device_id *of_match_node(
+ const struct of_device_id *matches, const struct device_node *node)
+{
+ return NULL;
+}
#endif /* CONFIG_OF */
/* Default string compare functions, Allow arch asm/prom.h to override */
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index d4b39184dbdb..bd418dea586d 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -80,7 +80,6 @@ static void build_deinstantiation_desc(u32 *desc, int handle)
append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TYPE_HALT);
}
-#ifdef CONFIG_OF
static const struct of_device_id imx8m_machine_match[] = {
{ .compatible = "fsl,imx8mm", },
{ .compatible = "fsl,imx8mn", },
@@ -89,7 +88,6 @@ static const struct of_device_id imx8m_machine_match[] = {
{ .compatible = "fsl,imx8ulp", },
{ }
};
-#endif
/*
* run_descriptor_deco0 - runs a descriptor on DECO0, under direct control of
diff --git a/drivers/dma/dw/rzn1-dmamux.c b/drivers/dma/dw/rzn1-dmamux.c
index 4fb8508419db..9dcba3a3ffaa 100644
--- a/drivers/dma/dw/rzn1-dmamux.c
+++ b/drivers/dma/dw/rzn1-dmamux.c
@@ -104,12 +104,10 @@ static void *rzn1_dmamux_route_allocate(struct of_phandle_args *dma_spec,
return ERR_PTR(ret);
}
-#ifdef CONFIG_OF
static const struct of_device_id rzn1_dmac_match[] = {
{ .compatible = "renesas,rzn1-dma" },
{}
};
-#endif
static int rzn1_dmamux_probe(struct platform_device *pdev)
{
diff --git a/drivers/i2c/busses/i2c-at91-core.c b/drivers/i2c/busses/i2c-at91-core.c
index edc047e3e535..3a9be06dd967 100644
--- a/drivers/i2c/busses/i2c-at91-core.c
+++ b/drivers/i2c/busses/i2c-at91-core.c
@@ -108,7 +108,6 @@ static const struct platform_device_id at91_twi_devtypes[] = {
}
};
-#if defined(CONFIG_OF)
static struct at91_twi_pdata at91sam9x5_config = {
.clk_max_div = 7,
.clk_offset = 4,
@@ -178,7 +177,6 @@ static const struct of_device_id atmel_twi_dt_ids[] = {
}
};
MODULE_DEVICE_TABLE(of, atmel_twi_dt_ids);
-#endif
static struct at91_twi_pdata *at91_twi_get_driver_data(
struct platform_device *pdev)
diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
index dc1e46d834dc..969e08e4d4f4 100644
--- a/drivers/i2c/busses/i2c-xiic.c
+++ b/drivers/i2c/busses/i2c-xiic.c
@@ -1409,7 +1409,6 @@ static const struct i2c_adapter xiic_adapter = {
.algo = &xiic_algorithm,
};
-#if defined(CONFIG_OF)
static const struct xiic_version_data xiic_2_00 = {
.quirks = DYNAMIC_MODE_READ_BROKEN_BIT,
};
@@ -1420,7 +1419,6 @@ static const struct of_device_id xiic_of_match[] = {
{},
};
MODULE_DEVICE_TABLE(of, xiic_of_match);
-#endif
static int xiic_i2c_probe(struct platform_device *pdev)
{
diff --git a/drivers/misc/atmel-ssc.c b/drivers/misc/atmel-ssc.c
index 35a196341534..3db592f3b451 100644
--- a/drivers/misc/atmel-ssc.c
+++ b/drivers/misc/atmel-ssc.c
@@ -111,7 +111,6 @@ static const struct platform_device_id atmel_ssc_devtypes[] = {
}
};
-#ifdef CONFIG_OF
static const struct of_device_id atmel_ssc_dt_ids[] = {
{
.compatible = "atmel,at91rm9200-ssc",
@@ -127,7 +126,6 @@ static const struct of_device_id atmel_ssc_dt_ids[] = {
}
};
MODULE_DEVICE_TABLE(of, atmel_ssc_dt_ids);
-#endif
static inline const struct atmel_ssc_platform_data *
atmel_ssc_get_driver_data(struct platform_device *pdev)
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 191707d7e3da..9fbbf3587b0c 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1013,7 +1013,6 @@ static const struct attribute_group at91_sysfs_attr_group = {
.attrs = at91_sysfs_attrs,
};
-#if defined(CONFIG_OF)
static const struct of_device_id at91_can_dt_ids[] = {
{
.compatible = "atmel,at91sam9x5-can",
@@ -1026,7 +1025,6 @@ static const struct of_device_id at91_can_dt_ids[] = {
}
};
MODULE_DEVICE_TABLE(of, at91_can_dt_ids);
-#endif
static const struct at91_devtype_data *at91_can_get_driver_data(struct platform_device *pdev)
{
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index 6c462de81f20..f942c6e54a1b 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c
@@ -4425,7 +4425,6 @@ static const struct macb_usrio_config macb_default_usrio = {
.refclk = MACB_BIT(CLKEN),
};
-#if defined(CONFIG_OF)
/* 1518 rounded up */
#define AT91ETHER_MAX_RBUFF_SZ 0x600
/* max number of receive buffers */
@@ -5144,7 +5143,6 @@ static const struct of_device_id macb_dt_ids[] = {
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, macb_dt_ids);
-#endif /* CONFIG_OF */
static const struct macb_config default_gem_config = {
.caps = MACB_CAPS_GIGABIT_MODE_AVAILABLE |
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-02-25 21:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-25 14:53 [PATCH] [v2] Input: stmpe-ts - mark OF related data as maybe unused Arnd Bergmann
2025-02-25 15:47 ` Uwe Kleine-König
2025-02-25 16:25 ` Arnd Bergmann
2025-02-25 20:17 ` Uwe Kleine-König
2025-02-25 21:27 ` Arnd Bergmann
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).