Input: da7280 - protect OF match table with CONFIG_OF

PATCH In review Dmitry Torokhov 3 participants 5 messages Dec 18, 2020 – Jan 4, 2021

Subsystems (3)

HID Input drivers Dialog semiconductor drivers

lore

Share & tools

lore

Dmitry Torokhov

author maintainer

2020-12-18 06:51 UTC #0 lore raw

PATCH 1/1 Input: da7280 - protect OF match table with CONFIG_OF

The OF match table is only used when OF is enabled.

Fixes cd3f609823a5 ("Input: new da7280 haptic driver")

Reported-by kernel test robot <[email protected]>

Signed-off-by Dmitry Torokhov <[email protected]>

---
 drivers/input/misc/da7280.c | 2 ++
 1 file changed, 2 insertions(+)

drivers/input/misc/da7280.c +2 -0

index 2f698a8c1d65..b08610d6e575 100644
@@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
 	return retval;
 }
 
+#ifdef CONFIG_OF
 static const struct of_device_id da7280_of_match[] = {
 	{ .compatible = "dlg,da7280", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7280_of_match);
+#endif
 
 static const struct i2c_device_id da7280_i2c_id[] = {
 	{ "da7280", },

Signature · 5 lines

2.29.2.729.g45daf8777d-goog


-- 
Dmitry

Roy Im

Acked-by on v1 1/1

2020-12-18 16:50 UTC #1 lore raw

Quoted 30 lines · Dmitry Torokhov

The OF match table is only used when OF is enabled.
Fixes cd3f609823a5 ("Input: new da7280 haptic driver")
Reported-by kernel test robot <[email protected]>
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/da7280.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/input/misc/da7280.c +2 -1
@@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
 	return retval;
 }
 
+#ifdef CONFIG_OF
 static const struct of_device_id da7280_of_match[] = {
 	{ .compatible = "dlg,da7280", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7280_of_match);
+#endif
 
 static const struct i2c_device_id da7280_i2c_id[] = {
 	{ "da7280", },
--
2.29.2.729.g45daf8777d-goog

Quoted 2 lines

--
Dmitry

Thanks!

Acked-by Roy Im <[email protected]>

Jeff LaBundy

2020-12-20 02:02 UTC #2 lore raw

Hi Dmitry,

Quoted 18 lines · Roy Im

Quoted 16 lines · Dmitry Torokhov
The OF match table is only used when OF is enabled.
Fixes cd3f609823a5 ("Input: new da7280 haptic driver")
Reported-by kernel test robot <[email protected]>
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/da7280.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/input/misc/da7280.c +0 -0
@@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
 	return retval;
 }
 

Quoted 7 lines

drivers/input/misc/da7280.c static int __maybe_unused da7280_resume(struct device *dev) · line 1309
Quoted 7 lines
drivers/input/misc/da7280.c +2 -0
+#ifdef CONFIG_OF
 static const struct of_device_id da7280_of_match[] = {
 	{ .compatible = "dlg,da7280", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7280_of_match);
+#endif

Just for my own understanding, would it not work just as well to include of_device.h? This includes mod_devicetable.h which in turn defines the of_device_id struct (even if CONFIG_OF is not set).

The reason for asking is because it seems many drivers do not include these guards.

Quoted 7 lines

Quoted 7 lines
drivers/input/misc/da7280.c +0 -1
 static const struct i2c_device_id da7280_i2c_id[] = {
 	{ "da7280", },
--
2.29.2.729.g45daf8777d-goog

Quoted 7 lines

Quoted 2 lines
--
Dmitry
Thanks!
Acked-by Roy Im <[email protected]>

Kind regards,
Jeff LaBundy

Dmitry Torokhov

author maintainer

2021-01-04 01:59 UTC #3 lore raw

Hi Jeff,

Quoted 21 lines · Jeff LaBundy

Hi Dmitry,
Quoted 18 lines · Roy Im
Quoted 16 lines · Dmitry Torokhov
The OF match table is only used when OF is enabled.
Fixes cd3f609823a5 ("Input: new da7280 haptic driver")
Reported-by kernel test robot <[email protected]>
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/da7280.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/input/misc/da7280.c +0 -0
@@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
 	return retval;
 }
 

Quoted 12 lines

Quoted 7 lines
drivers/input/misc/da7280.c static int __maybe_unused da7280_resume(struct device *dev) · line 1309
Quoted 7 lines
drivers/input/misc/da7280.c +2 -0
+#ifdef CONFIG_OF
 static const struct of_device_id da7280_of_match[] = {
 	{ .compatible = "dlg,da7280", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7280_of_match);
+#endif
Just for my own understanding, would it not work just as well to include of_device.h? This includes mod_devicetable.h which in turn defines the of_device_id struct (even if CONFIG_OF is not set).

The issue here is not that the structure is undefined, but the variable is unused. We could also fix this by not using of_match_ptr() when assigning the match table to the driver structure, making the variable referenced even if CONFIG_OF is off.

Quoted 3 lines

The reason for asking is because it seems many drivers do not include these guards.

It could be that they are either only compiled with OF, or they decided it is not worth saving a few bytes, or maybe they are used on ACPI-based systems with PRP0001 bindings in which case the match table in the driver might still be needed.

Thanks.

Signature · 1 line

Dmitry

Jeff LaBundy

2021-01-04 04:05 UTC #4 lore raw

Hi Dmitry,

Quoted 37 lines · Dmitry Torokhov

Hi Jeff,
Quoted 21 lines · Jeff LaBundy
Hi Dmitry,
Quoted 18 lines · Roy Im
Quoted 16 lines · Dmitry Torokhov
The OF match table is only used when OF is enabled.
Fixes cd3f609823a5 ("Input: new da7280 haptic driver")
Reported-by kernel test robot <[email protected]>
Signed-off-by Dmitry Torokhov <[email protected]>
---
 drivers/input/misc/da7280.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/input/misc/da7280.c +0 -0
@@ -1300,11 +1300,13 @@ static int __maybe_unused da7280_resume(struct device *dev)
 	return retval;
 }
 
Quoted 12 lines
Quoted 7 lines
drivers/input/misc/da7280.c static int __maybe_unused da7280_resume(struct device *dev) · line 1309
Quoted 7 lines
drivers/input/misc/da7280.c +2 -0
+#ifdef CONFIG_OF
 static const struct of_device_id da7280_of_match[] = {
 	{ .compatible = "dlg,da7280", },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, da7280_of_match);
+#endif
Just for my own understanding, would it not work just as well to include of_device.h? This includes mod_devicetable.h which in turn defines the of_device_id struct (even if CONFIG_OF is not set).

Quoted 4 lines

The issue here is not that the structure is undefined, but the variable is unused. We could also fix this by not using of_match_ptr() when assigning the match table to the driver structure, making the variable referenced even if CONFIG_OF is off.

ACK. The call to of_match_ptr() is what I was missing; the other drivers I was looking at do not use it which must be why the bot has not complained.

Quoted 9 lines

Quoted 4 lines
The reason for asking is because it seems many drivers do not include these guards.
It could be that they are either only compiled with OF, or they decided it is not worth saving a few bytes, or maybe they are used on ACPI-based systems with PRP0001 bindings in which case the match table in the driver might still be needed.

Makes perfect sense; thank you for the follow-up.

Quoted 5 lines

Thanks.
Signature · 1 line
Dmitry

Kind regards,
Jeff LaBundy