Hello again,
On Wed, Apr 03, 2024 at 03:17:32PM +0200, Uwe Kleine-König wrote:
> On Wed, Apr 03, 2024 at 10:06:25AM +0200, 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]
> >
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.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 b204fdb2d22c..022b3e94266d 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", },
> > { },
> > };
>
> I'd suggest the following instead:
>
> diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c
> index b204fdb2d22c..e1afebc641ec 100644
> --- a/drivers/input/touchscreen/stmpe-ts.c
> +++ b/drivers/input/touchscreen/stmpe-ts.c
> @@ -357,21 +357,22 @@ static void stmpe_ts_remove(struct platform_device *pdev)
> stmpe_disable(ts->stmpe, STMPE_BLOCK_TOUCHSCREEN);
> }
>
> -static struct platform_driver stmpe_ts_driver = {
> - .driver = {
> - .name = STMPE_TS_NAME,
> - },
> - .probe = stmpe_input_probe,
> - .remove_new = stmpe_ts_remove,
> -};
> -module_platform_driver(stmpe_ts_driver);
> -
> static const struct of_device_id stmpe_ts_ids[] = {
> { .compatible = "st,stmpe-ts", },
> { },
> };
> MODULE_DEVICE_TABLE(of, stmpe_ts_ids);
>
> +static struct platform_driver stmpe_ts_driver = {
> + .driver = {
> + .name = STMPE_TS_NAME,
> + .of_match_table = stmpe_ts_ids,
> + },
> + .probe = stmpe_input_probe,
> + .remove_new = stmpe_ts_remove,
> +};
> +module_platform_driver(stmpe_ts_driver);
> +
> MODULE_AUTHOR("Luotao Fu <l.fu@pengutronix.de>");
> MODULE_DESCRIPTION("STMPEXXX touchscreen driver");
> MODULE_LICENSE("GPL");
>
> I wonder if with the status quo binding via dt works at all with
> stmpe_ts_driver.driver.of_match_table unset?!
I missed the discussion between Andy and Krzysztof when I wrote my mail.
I still think this should be considered and if .of_match_table should
stay unassigned (e.g. to allow dropping stmpe_ts_ids in case the driver
is built-in?) I think adding a code comment would be appropriate because
having an of_device_id array but not adding it to the driver is unusuall
and generally a bad template for new drivers.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |