[PATCH 0/2] Input: st1232: wakeup in Suspend to idle

[PATCH 0/2] Input: st1232: wakeup in Suspend to idle

[Javier Carrasco]

The st1232 touchscreen provides an interrupt line that can be configured
as a wakeup source, and currently it is possible to use this mechanism in
"Suspend to RAM" and "Hibernate" power states. 

Unfortunately, that does not work in "Suspend to idle" (freeze) because
the device driver disables the interrupts in its suspend callback.
Given that the interrupt handling prior to entering the mentioned power
state modes is managed by the power subsystem, the irq enabling/disabling
can be removed from the touchscreen driver, allowing the device to work
as a wakeup source in "Suspend to idle".

Given that the st1232 device driver does not reflect its wakeup events
in sysfs, this series also adds pm_wakeup_event to the interrupt
handler.

These changes have been successfully tested with an ST1624-N32C and a
Rockchip-based platform.

Javier Carrasco (2):
  Input: st1232 - remove enable/disable irq in resume/suspend callbacks
  Input: st1232 - add wake up event counting

 drivers/input/touchscreen/st1232.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

-- 
2.37.2

[PATCH 1/2] Input: st1232 - remove enable/disable irq in resume/suspend callbacks

[Javier Carrasco]

Disabling the interrupts in the suspend callback leads to a loss of the
wakeup capability in suspend to idle (freeze) mode.

In commit 95dc58a9a02f ("Input: st1232 - rely on I2C core to configure
wakeup interrupt") redundancy was removed by letting the I2C core manage
the wake interrupt handling. On the other hand, the irq enabling/disabling
was generalized to all devices, regardless of their wakeup capabilities.

The interrupt enabling/disabling is carried out by the power subsystem in
the resume/suspend states and therefore there is no need to manage it
in the device driver. Remove the irq handling in the driver-specific
resume/suspend callbacks and rely on the power subsystem.

Fixes: 95dc58a9a02f ("Input: st1232 - rely on I2C core to configure wakeup interrupt")
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 drivers/input/touchscreen/st1232.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index f49566dc96f8..faa0be3993f4 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -345,8 +345,6 @@ 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))
 		st1232_ts_power(ts, false);
 
@@ -361,8 +359,6 @@ static int st1232_ts_resume(struct device *dev)
 	if (!device_may_wakeup(&client->dev))
 		st1232_ts_power(ts, true);
 
-	enable_irq(client->irq);
-
 	return 0;
 }
 
-- 
2.37.2

[PATCH 2/2] Input: st1232 - add wake up event counting

[Javier Carrasco]

This device driver provides wakeup capabilities but the wakeup events
are not reflected in sysfs. Add pm_wakeup_event to the interrupt handler
in order to do so.

Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
 drivers/input/touchscreen/st1232.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index faa0be3993f4..f175bea464e1 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -59,6 +59,7 @@ struct st1232_ts_data {
 	const struct st_chip_info *chip_info;
 	int read_buf_len;
 	u8 *read_buf;
+	bool suspended;
 };
 
 static int st1232_ts_read_data(struct st1232_ts_data *ts, u8 reg,
@@ -173,9 +174,13 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
 static irqreturn_t st1232_ts_irq_handler(int irq, void *dev_id)
 {
 	struct st1232_ts_data *ts = dev_id;
+	struct i2c_client *client = ts->client;
 	int count;
 	int error;
 
+	if (ts->suspended && device_may_wakeup(&client->dev))
+		pm_wakeup_event(&client->dev, 0);
+
 	error = st1232_ts_read_data(ts, REG_XY_COORDINATES, ts->read_buf_len);
 	if (error)
 		goto out;
@@ -344,10 +349,16 @@ 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);
+	struct input_dev *input = ts->input_dev;
+
+	mutex_lock(&input->mutex);
+	ts->suspended = true;
 
 	if (!device_may_wakeup(&client->dev))
 		st1232_ts_power(ts, false);
 
+	mutex_unlock(&input->mutex);
+
 	return 0;
 }
 
@@ -355,10 +366,16 @@ 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);
+	struct input_dev *input = ts->input_dev;
+
+	mutex_lock(&input->mutex);
+	ts->suspended = false;
 
 	if (!device_may_wakeup(&client->dev))
 		st1232_ts_power(ts, true);
 
+	mutex_unlock(&input->mutex);
+
 	return 0;
 }
 
-- 
2.37.2

Re: [PATCH 0/2] Input: st1232: wakeup in Suspend to idle

[Javier Carrasco Cruz]

On 03.04.23 14:47, Javier Carrasco wrote:
> The st1232 touchscreen provides an interrupt line that can be configured
> as a wakeup source, and currently it is possible to use this mechanism in
> "Suspend to RAM" and "Hibernate" power states.
> 
> Unfortunately, that does not work in "Suspend to idle" (freeze) because
> the device driver disables the interrupts in its suspend callback.
> Given that the interrupt handling prior to entering the mentioned power
> state modes is managed by the power subsystem, the irq enabling/disabling
> can be removed from the touchscreen driver, allowing the device to work
> as a wakeup source in "Suspend to idle".
> 
> Given that the st1232 device driver does not reflect its wakeup events
> in sysfs, this series also adds pm_wakeup_event to the interrupt
> handler.
> 
> These changes have been successfully tested with an ST1624-N32C and a
> Rockchip-based platform.
> 
> Javier Carrasco (2):
>    Input: st1232 - remove enable/disable irq in resume/suspend callbacks
>    Input: st1232 - add wake up event counting
> 
>   drivers/input/touchscreen/st1232.c | 17 +++++++++++++++--
>   1 file changed, 15 insertions(+), 2 deletions(-)
> 

Hi, I submitted this series 2 weeks ago and given that I did not get any 
feedback yet, I would like to make sure that the patches arrived in the 
right lists and if so, that they are "queued for review" and I just have 
to wait, which would be indeed alright.

Thanks and best regards.

Re: [PATCH 0/2] Input: st1232: wakeup in Suspend to idle

[Javier Carrasco]

Gentle ping.

On 03.04.23 14:47, Javier Carrasco wrote:
> The st1232 touchscreen provides an interrupt line that can be configured
> as a wakeup source, and currently it is possible to use this mechanism in
> "Suspend to RAM" and "Hibernate" power states. 
> 
> Unfortunately, that does not work in "Suspend to idle" (freeze) because
> the device driver disables the interrupts in its suspend callback.
> Given that the interrupt handling prior to entering the mentioned power
> state modes is managed by the power subsystem, the irq enabling/disabling
> can be removed from the touchscreen driver, allowing the device to work
> as a wakeup source in "Suspend to idle".
> 
> Given that the st1232 device driver does not reflect its wakeup events
> in sysfs, this series also adds pm_wakeup_event to the interrupt
> handler.
> 
> These changes have been successfully tested with an ST1624-N32C and a
> Rockchip-based platform.
> 
> Javier Carrasco (2):
>   Input: st1232 - remove enable/disable irq in resume/suspend callbacks
>   Input: st1232 - add wake up event counting
> 
>  drivers/input/touchscreen/st1232.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>