[PATCH v3 3/3] input: touchscreen: st1232: add system wakeup support

phucduc.bui@gmail.com posted 3 patches 1 month ago
There is a newer version of this series
[PATCH v3 3/3] input: touchscreen: st1232: add system wakeup support
Posted by phucduc.bui@gmail.com 1 month ago
From: bui duc phuc <phucduc.bui@gmail.com>

The ST1232 touchscreen controller can generate an interrupt when the
panel is touched, which may be used as a wakeup source for the system.

Add support for system wakeup by initializing the device wakeup
capability in probe() based on the "wakeup-source" device property.
When wakeup is enabled, the driver enables IRQ wake during suspend
so that touch events can wake the system.

If wakeup is not enabled, the driver retains the existing behavior of
disabling the IRQ and powering down the controller during suspend.

Signed-off-by: bui duc phuc <phucduc.bui@gmail.com>
---
Changes in v3:
 - Remove debug dev_info() messages to clean up the code and comply with
   upstream coding standards.

 drivers/input/touchscreen/st1232.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index 9b3901eec0a5..8fce17d8bdc0 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -183,6 +183,9 @@ static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
 	int count;
 	int error;
 
+	if (device_may_wakeup(&ts->client->dev))
+		pm_wakeup_event(&ts->client->dev, 0);
+
 	error = st1232_ts_read_data(ts, REG_XY_COORDINATES, ts->read_buf_len);
 	if (error)
 		goto out;
@@ -356,6 +359,9 @@ static int st1232_ts_probe(struct i2c_client *client)
 
 	i2c_set_clientdata(client, ts);
 
+	device_init_wakeup(&client->dev,
+			device_property_read_bool(&client->dev, "wakeup-source"));
+
 	return 0;
 }
 
@@ -364,10 +370,12 @@ static int st1232_ts_suspend(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct st1232_ts_data *ts = i2c_get_clientdata(client);
 
-	disable_irq(client->irq);
-
-	if (!device_may_wakeup(&client->dev))
+	if (device_may_wakeup(dev)) {
+		enable_irq_wake(client->irq);
+	} else {
+		disable_irq(client->irq);
 		st1232_ts_power(ts, false);
+	}
 
 	return 0;
 }
@@ -377,10 +385,12 @@ static int st1232_ts_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct st1232_ts_data *ts = i2c_get_clientdata(client);
 
-	if (!device_may_wakeup(&client->dev))
+	if (device_may_wakeup(dev)) {
+		disable_irq_wake(client->irq);
+	} else {
 		st1232_ts_power(ts, true);
-
-	enable_irq(client->irq);
+		enable_irq(client->irq);
+	}
 
 	return 0;
 }
-- 
2.43.0
Re: [PATCH v3 3/3] input: touchscreen: st1232: add system wakeup support
Posted by Dmitry Torokhov 1 week ago
On Fri, Mar 06, 2026 at 06:19:12PM +0700, phucduc.bui@gmail.com wrote:
> From: bui duc phuc <phucduc.bui@gmail.com>
> 
> The ST1232 touchscreen controller can generate an interrupt when the
> panel is touched, which may be used as a wakeup source for the system.
> 
> Add support for system wakeup by initializing the device wakeup
> capability in probe() based on the "wakeup-source" device property.
> When wakeup is enabled, the driver enables IRQ wake during suspend
> so that touch events can wake the system.
> 
> If wakeup is not enabled, the driver retains the existing behavior of
> disabling the IRQ and powering down the controller during suspend.

I do not believe this patch is needed: i2c core already handles
"wakeup-source" property and manages wakeup IRQ.

Thanks.

-- 
Dmitry
Re: [PATCH v3 3/3] input: touchscreen: st1232: add system wakeup support
Posted by Geert Uytterhoeven 1 week ago
Hi Dmitry,

On Thu, 2 Apr 2026 at 07:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Fri, Mar 06, 2026 at 06:19:12PM +0700, phucduc.bui@gmail.com wrote:
> > From: bui duc phuc <phucduc.bui@gmail.com>
> >
> > The ST1232 touchscreen controller can generate an interrupt when the
> > panel is touched, which may be used as a wakeup source for the system.
> >
> > Add support for system wakeup by initializing the device wakeup
> > capability in probe() based on the "wakeup-source" device property.
> > When wakeup is enabled, the driver enables IRQ wake during suspend
> > so that touch events can wake the system.
> >
> > If wakeup is not enabled, the driver retains the existing behavior of
> > disabling the IRQ and powering down the controller during suspend.
>
> I do not believe this patch is needed: i2c core already handles
> "wakeup-source" property and manages wakeup IRQ.

No, it is not needed, as mentioned in the cover letter of v4[1],
and as tested by me[2].

[1] https://lore.kernel.org/20260309000319.74880-1-phucduc.bui@gmail.com
[2] https://lore.kernel.org/CAMuHMdUqiaP=COTkKU_jK6Hdii+YJ5+zXnxFkOOnhLri5NakTw@mail.gmail.com

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH v3 3/3] input: touchscreen: st1232: add system wakeup support
Posted by Bui Duc Phuc 6 days, 6 hours ago
Hi Dmitry, Geert,

Thank you, Dmitry, for the review and the explanation. You are
absolutely right; I realized the I2C core handles this automatically,
which is tại sao I dropped those changes in the v4 series [1] as Geert
mentioned.

Thank you, Geert, for pointing that out and for your support.

While working on this, I also noticed similar redundant wakeup
handling in the mpr121 driver and sent a cleanup patch to remove
it [2].

[1] https://lore.kernel.org/20260309000319.74880-1-phucduc.bui@gmail.com
[2] https://lore.kernel.org/all/20260309071413.92709-1-phucduc.bui@gmail.com/

Thanks,
Phuc

On Thu, Apr 2, 2026 at 1:56 PM Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
> Hi Dmitry,
>
> On Thu, 2 Apr 2026 at 07:17, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > On Fri, Mar 06, 2026 at 06:19:12PM +0700, phucduc.bui@gmail.com wrote:
> > > From: bui duc phuc <phucduc.bui@gmail.com>
> > >
> > > The ST1232 touchscreen controller can generate an interrupt when the
> > > panel is touched, which may be used as a wakeup source for the system.
> > >
> > > Add support for system wakeup by initializing the device wakeup
> > > capability in probe() based on the "wakeup-source" device property.
> > > When wakeup is enabled, the driver enables IRQ wake during suspend
> > > so that touch events can wake the system.
> > >
> > > If wakeup is not enabled, the driver retains the existing behavior of
> > > disabling the IRQ and powering down the controller during suspend.
> >
> > I do not believe this patch is needed: i2c core already handles
> > "wakeup-source" property and manages wakeup IRQ.
>
> No, it is not needed, as mentioned in the cover letter of v4[1],
> and as tested by me[2].
>
> [1] https://lore.kernel.org/20260309000319.74880-1-phucduc.bui@gmail.com
> [2] https://lore.kernel.org/CAMuHMdUqiaP=COTkKU_jK6Hdii+YJ5+zXnxFkOOnhLri5NakTw@mail.gmail.com
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds