* [PATCH] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
@ 2026-08-21 2:44 Triet Hoang
2026-08-21 2:55 ` sashiko-bot
0 siblings, 1 reply; 4+ messages in thread
From: Triet Hoang @ 2026-08-21 2:44 UTC (permalink / raw)
To: dmitry.torokhov
Cc: michael.hennerich, linusw, linux-input, linux-kernel, Triet Hoang
Convert deprecated SIMPLE_DEV_PM_OPS
to DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr().
This lets us drop the __maybe_unused annotations
from the resume and suspend callbacks.
Also reduces kernel size when CONFIG_PM
or CONFIG_PM_SLEEP is disabled.
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
drivers/input/touchscreen/ad7879-i2c.c | 2 +-
drivers/input/touchscreen/ad7879-spi.c | 2 +-
drivers/input/touchscreen/ad7879.c | 8 ++++----
drivers/input/touchscreen/cyttsp5.c | 8 ++++----
4 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index c1ccdf86b74c..7ee5c0abd551 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -60,7 +60,7 @@ static struct i2c_driver ad7879_i2c_driver = {
.driver = {
.name = "ad7879",
.dev_groups = ad7879_groups,
- .pm = &ad7879_pm_ops,
+ .pm = pm_sleep_ptr(&ad7879_pm_ops),
.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
},
.probe = ad7879_i2c_probe,
diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
index 064968fe57cf..66dcd4aa4711 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -58,7 +58,7 @@ static struct spi_driver ad7879_spi_driver = {
.driver = {
.name = "ad7879",
.dev_groups = ad7879_groups,
- .pm = &ad7879_pm_ops,
+ .pm = pm_sleep_ptr(&ad7879_pm_ops),
.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
},
.probe = ad7879_spi_probe,
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index 31d2a3029d5f..f686f028d4bc 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -301,7 +301,7 @@ static void ad7879_close(struct input_dev *input)
__ad7879_disable(ts);
}
-static int __maybe_unused ad7879_suspend(struct device *dev)
+static int ad7879_suspend(struct device *dev)
{
struct ad7879 *ts = dev_get_drvdata(dev);
@@ -315,7 +315,7 @@ static int __maybe_unused ad7879_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused ad7879_resume(struct device *dev)
+static int ad7879_resume(struct device *dev)
{
struct ad7879 *ts = dev_get_drvdata(dev);
@@ -329,8 +329,8 @@ static int __maybe_unused ad7879_resume(struct device *dev)
return 0;
}
-SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
-EXPORT_SYMBOL(ad7879_pm_ops);
+DEFINE_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
+EXPORT_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
static void ad7879_toggle(struct ad7879 *ts, bool disable)
{
diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 9266c07314be..7b0613156fc1 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -942,7 +942,7 @@ static const struct i2c_device_id cyttsp5_i2c_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
-static int __maybe_unused cyttsp5_suspend(struct device *dev)
+static int cyttsp5_suspend(struct device *dev)
{
struct cyttsp5 *ts = dev_get_drvdata(dev);
@@ -952,7 +952,7 @@ static int __maybe_unused cyttsp5_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused cyttsp5_resume(struct device *dev)
+static int cyttsp5_resume(struct device *dev)
{
struct cyttsp5 *ts = dev_get_drvdata(dev);
@@ -962,13 +962,13 @@ static int __maybe_unused cyttsp5_resume(struct device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(cyttsp5_pm, cyttsp5_suspend, cyttsp5_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(cyttsp5_pm, cyttsp5_suspend, cyttsp5_resume);
static struct i2c_driver cyttsp5_i2c_driver = {
.driver = {
.name = CYTTSP5_NAME,
.of_match_table = cyttsp5_of_match,
- .pm = &cyttsp5_pm,
+ .pm = pm_sleep_ptr(&cyttsp5_pm),
},
.probe = cyttsp5_i2c_probe,
.id_table = cyttsp5_i2c_id,
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
2026-08-21 2:44 [PATCH] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS() Triet Hoang
@ 2026-08-21 2:55 ` sashiko-bot
2026-08-21 4:09 ` [PATCH v2] " Triet Hoang
0 siblings, 1 reply; 4+ messages in thread
From: sashiko-bot @ 2026-08-21 2:55 UTC (permalink / raw)
To: Triet Hoang; +Cc: dmitry.torokhov, linux-input
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a strict C compiler redefinition error for `ad7879_pm_ops` when `CONFIG_PM_SLEEP` is enabled.
--
--- Patch [1]: [PATCH] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS() ---
commit 44d9a5ca9f2aa03594a9658d8105747b57b227ba
Author: Triet Hoang <triet.hoang.dev@gmail.com>
input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
This commit updates several touchscreen drivers to use the newer
DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr() macros instead of the deprecated
SIMPLE_DEV_PM_OPS. This allows the removal of __maybe_unused annotations on
suspend and resume callbacks.
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index 31d2a3029d5f6..f686f028d4bc9 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
[ ... ]
> @@ -329,8 +329,8 @@ static int __maybe_unused ad7879_resume(struct device *dev)
> return 0;
> }
>
> -SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
> -EXPORT_SYMBOL(ad7879_pm_ops);
> +DEFINE_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
> +EXPORT_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
[Severity: Low]
Does this cause a compilation error when CONFIG_PM_SLEEP is enabled?
Both DEFINE_SIMPLE_DEV_PM_OPS and EXPORT_SIMPLE_DEV_PM_OPS expand to a
complete initialization of the ad7879_pm_ops struct. Using both sequentially
redefines the same global variable, which will result in a build failure.
Should this just be EXPORT_SIMPLE_DEV_PM_OPS by itself?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260821024450.57369-1-triet.hoang.dev@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
2026-08-21 2:55 ` sashiko-bot
@ 2026-08-21 4:09 ` Triet Hoang
2026-08-21 4:19 ` sashiko-bot
0 siblings, 1 reply; 4+ messages in thread
From: Triet Hoang @ 2026-08-21 4:09 UTC (permalink / raw)
To: sashiko-bot
Cc: dmitry.torokhov, linux-input, sashiko-reviews, triet.hoang.dev
Convert deprecated SIMPLE_DEV_PM_OPS
to DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr().
This lets us drop the __maybe_unused annotations
from the resume and suspend callbacks.
Also reduces kernel size when CONFIG_PM
or CONFIG_PM_SLEEP is disabled.
Signed-off-by: Triet Hoang <triet.hoang.dev@gmail.com>
---
Changes in v2:
- Remove DEFINE_SIMPLE_DEV_PM_OPS, use only EXPORT_SIMPLE_DEV_PM_OPS for ad7879_pm_ops
---
drivers/input/touchscreen/ad7879-i2c.c | 2 +-
drivers/input/touchscreen/ad7879-spi.c | 2 +-
drivers/input/touchscreen/ad7879.c | 7 +++----
drivers/input/touchscreen/cyttsp5.c | 8 ++++----
4 files changed, 9 insertions(+), 10 deletions(-)
diff --git a/drivers/input/touchscreen/ad7879-i2c.c b/drivers/input/touchscreen/ad7879-i2c.c
index c1ccdf86b74c..7ee5c0abd551 100644
--- a/drivers/input/touchscreen/ad7879-i2c.c
+++ b/drivers/input/touchscreen/ad7879-i2c.c
@@ -60,7 +60,7 @@ static struct i2c_driver ad7879_i2c_driver = {
.driver = {
.name = "ad7879",
.dev_groups = ad7879_groups,
- .pm = &ad7879_pm_ops,
+ .pm = pm_sleep_ptr(&ad7879_pm_ops),
.of_match_table = of_match_ptr(ad7879_i2c_dt_ids),
},
.probe = ad7879_i2c_probe,
diff --git a/drivers/input/touchscreen/ad7879-spi.c b/drivers/input/touchscreen/ad7879-spi.c
index 064968fe57cf..66dcd4aa4711 100644
--- a/drivers/input/touchscreen/ad7879-spi.c
+++ b/drivers/input/touchscreen/ad7879-spi.c
@@ -58,7 +58,7 @@ static struct spi_driver ad7879_spi_driver = {
.driver = {
.name = "ad7879",
.dev_groups = ad7879_groups,
- .pm = &ad7879_pm_ops,
+ .pm = pm_sleep_ptr(&ad7879_pm_ops),
.of_match_table = of_match_ptr(ad7879_spi_dt_ids),
},
.probe = ad7879_spi_probe,
diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
index 31d2a3029d5f..10f8574f478e 100644
--- a/drivers/input/touchscreen/ad7879.c
+++ b/drivers/input/touchscreen/ad7879.c
@@ -301,7 +301,7 @@ static void ad7879_close(struct input_dev *input)
__ad7879_disable(ts);
}
-static int __maybe_unused ad7879_suspend(struct device *dev)
+static int ad7879_suspend(struct device *dev)
{
struct ad7879 *ts = dev_get_drvdata(dev);
@@ -315,7 +315,7 @@ static int __maybe_unused ad7879_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused ad7879_resume(struct device *dev)
+static int ad7879_resume(struct device *dev)
{
struct ad7879 *ts = dev_get_drvdata(dev);
@@ -329,8 +329,7 @@ static int __maybe_unused ad7879_resume(struct device *dev)
return 0;
}
-SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
-EXPORT_SYMBOL(ad7879_pm_ops);
+EXPORT_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
static void ad7879_toggle(struct ad7879 *ts, bool disable)
{
diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 9266c07314be..7b0613156fc1 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -942,7 +942,7 @@ static const struct i2c_device_id cyttsp5_i2c_id[] = {
};
MODULE_DEVICE_TABLE(i2c, cyttsp5_i2c_id);
-static int __maybe_unused cyttsp5_suspend(struct device *dev)
+static int cyttsp5_suspend(struct device *dev)
{
struct cyttsp5 *ts = dev_get_drvdata(dev);
@@ -952,7 +952,7 @@ static int __maybe_unused cyttsp5_suspend(struct device *dev)
return 0;
}
-static int __maybe_unused cyttsp5_resume(struct device *dev)
+static int cyttsp5_resume(struct device *dev)
{
struct cyttsp5 *ts = dev_get_drvdata(dev);
@@ -962,13 +962,13 @@ static int __maybe_unused cyttsp5_resume(struct device *dev)
return 0;
}
-static SIMPLE_DEV_PM_OPS(cyttsp5_pm, cyttsp5_suspend, cyttsp5_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(cyttsp5_pm, cyttsp5_suspend, cyttsp5_resume);
static struct i2c_driver cyttsp5_i2c_driver = {
.driver = {
.name = CYTTSP5_NAME,
.of_match_table = cyttsp5_of_match,
- .pm = &cyttsp5_pm,
+ .pm = pm_sleep_ptr(&cyttsp5_pm),
},
.probe = cyttsp5_i2c_probe,
.id_table = cyttsp5_i2c_id,
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
2026-08-21 4:09 ` [PATCH v2] " Triet Hoang
@ 2026-08-21 4:19 ` sashiko-bot
0 siblings, 0 replies; 4+ messages in thread
From: sashiko-bot @ 2026-08-21 4:19 UTC (permalink / raw)
To: Triet Hoang; +Cc: linux-input, dmitry.torokhov
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The logic in `ad7879_toggle()` is completely inverted, causing the sysfs `disable` attribute to malfunction and desynchronize the hardware state from software tracking.
--
--- Patch [1]: [PATCH v2] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS() ---
commit b2cacedba4aaf53cb97b0b4bbb302d78a3a37daf
Author: Triet Hoang <triet.hoang.dev@gmail.com>
input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS()
This commit converts the deprecated SIMPLE_DEV_PM_OPS to
DEFINE_SIMPLE_DEV_PM_OPS and pm_sleep_ptr() across several touchscreen
drivers. This allows dropping the __maybe_unused annotations and reduces
kernel size when PM features are disabled.
> diff --git a/drivers/input/touchscreen/ad7879.c b/drivers/input/touchscreen/ad7879.c
> index 31d2a3029d5f6..10f8574f478e2 100644
> --- a/drivers/input/touchscreen/ad7879.c
> +++ b/drivers/input/touchscreen/ad7879.c
[ ... ]
> @@ -329,8 +329,7 @@ static int __maybe_unused ad7879_resume(struct device *dev)
> return 0;
> }
>
> -SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
> -EXPORT_SYMBOL(ad7879_pm_ops);
> +EXPORT_SIMPLE_DEV_PM_OPS(ad7879_pm_ops, ad7879_suspend, ad7879_resume);
>
> static void ad7879_toggle(struct ad7879 *ts, bool disable)
> {
[Severity: High]
This is a pre-existing issue, but the logic in this function appears to be
completely inverted.
Looking at the full ad7879_toggle() function in
drivers/input/touchscreen/ad7879.c:
static void ad7879_toggle(struct ad7879 *ts, bool disable)
{
guard(mutex)(&ts->input->mutex);
if (!ts->suspended && input_device_enabled(ts->input)) {
if (disable) {
if (ts->disabled)
__ad7879_enable(ts);
} else {
if (!ts->disabled)
__ad7879_disable(ts);
}
}
ts->disabled = disable;
}
If a user writes 1 to the disable sysfs attribute (disable == true), and the
device is already marked disabled (ts->disabled == true), won't this logic
actually enable the hardware by calling __ad7879_enable(ts)?
Conversely, if the user writes 0 to enable the device (disable == false)
while the device is currently enabled (ts->disabled == false), won't this
unexpectedly disable the hardware?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260821040905.104590-1-triet.hoang.dev@gmail.com?part=1
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-08-21 4:19 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 2:44 [PATCH] input: touchscreen: Convert to DEFINE_SIMPLE_DEV_PM_OPS() Triet Hoang
2026-08-21 2:55 ` sashiko-bot
2026-08-21 4:09 ` [PATCH v2] " Triet Hoang
2026-08-21 4:19 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox