| Message ID | 1624456597-9486-1-git-send-email-loic.poulain@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] Input: atmel_mxt_ts: Add wake-up support | expand |
Hi Loic, On Wed, Jun 23, 2021 at 03:56:36PM +0200, Loic Poulain wrote: > Add capability for the touchscreen to wakeup the host on touch event. > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > --- > drivers/input/touchscreen/atmel_mxt_ts.c | 18 ++++++++++++++---- > 1 file changed, 14 insertions(+), 4 deletions(-) > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > index 05de92c..807f449 100644 > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > @@ -3223,6 +3223,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) > return error; > } > > + device_set_wakeup_capable(&client->dev, true); We do not want to make the touch controller be wakeup source unconditionally. I2C core recognized "wakeup-source" in device tree, other platforms may employ different techniques setting I2C_CLIENT_WAKE when registering I2C devices to mark them as wakeup capable/enabled. > + > error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), > data->regulators); > if (error) { > @@ -3309,8 +3311,12 @@ static int __maybe_unused mxt_suspend(struct device *dev) > > mutex_lock(&input_dev->mutex); > > - if (input_device_enabled(input_dev)) > - mxt_stop(data); > + if (input_device_enabled(input_dev)) { > + if (device_may_wakeup(dev)) > + enable_irq_wake(data->irq); For devices that are registered as I2C_CLIENT_WAKE i2c core ensures that their interrupts are configured for wakeup when system transitions to sleep state, so you do not need to call enable_irq_wake() and disable_irq_wake(). You also need to make sure the controller is powered up when it is configured for wakeup. > + else > + mxt_stop(data); > + } > > mutex_unlock(&input_dev->mutex); > > @@ -3332,8 +3338,12 @@ static int __maybe_unused mxt_resume(struct device *dev) > > mutex_lock(&input_dev->mutex); > > - if (input_device_enabled(input_dev)) > - mxt_start(data); > + if (input_device_enabled(input_dev)) { > + if (device_may_wakeup(dev)) > + disable_irq_wake(data->irq); > + else > + mxt_start(data); > + } > > mutex_unlock(&input_dev->mutex); > > -- > 2.7.4 > Thanks. -- Dmitry
Hi Dmitry, On Thu, 24 Jun 2021 at 02:41, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote: > > Hi Loic, > > On Wed, Jun 23, 2021 at 03:56:36PM +0200, Loic Poulain wrote: > > Add capability for the touchscreen to wakeup the host on touch event. > > > > Signed-off-by: Loic Poulain <loic.poulain@linaro.org> > > --- > > drivers/input/touchscreen/atmel_mxt_ts.c | 18 ++++++++++++++---- > > 1 file changed, 14 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c > > index 05de92c..807f449 100644 > > --- a/drivers/input/touchscreen/atmel_mxt_ts.c > > +++ b/drivers/input/touchscreen/atmel_mxt_ts.c > > @@ -3223,6 +3223,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) > > return error; > > } > > > > + device_set_wakeup_capable(&client->dev, true); > > We do not want to make the touch controller be wakeup source > unconditionally. I2C core recognized "wakeup-source" in device tree, > other platforms may employ different techniques setting I2C_CLIENT_WAKE > when registering I2C devices to mark them as wakeup capable/enabled. Contrary to device_init_wakeup(), used in some other input drivers, device_set_wakeup_capable() does not enable the device as a wakeup source but just sets it as wakeup capable, and it's up to the user or distro policy to enable it as a wakeup source or not. It's a quite common way to do, and it does not change the behavior of this driver. The I2C_CLIENT_WAKE forces enabling wakeup source, which is maybe not what we want by default for a touchscreen. remote-wakeup enabling is a device configuration not a hardware property. Thoughts? I should probably also add dev_pm_set_wake_irq() for auto-enabling wake on suspend instead of doing it manually. Regards, Loic
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c index 05de92c..807f449 100644 --- a/drivers/input/touchscreen/atmel_mxt_ts.c +++ b/drivers/input/touchscreen/atmel_mxt_ts.c @@ -3223,6 +3223,8 @@ static int mxt_probe(struct i2c_client *client, const struct i2c_device_id *id) return error; } + device_set_wakeup_capable(&client->dev, true); + error = regulator_bulk_enable(ARRAY_SIZE(data->regulators), data->regulators); if (error) { @@ -3309,8 +3311,12 @@ static int __maybe_unused mxt_suspend(struct device *dev) mutex_lock(&input_dev->mutex); - if (input_device_enabled(input_dev)) - mxt_stop(data); + if (input_device_enabled(input_dev)) { + if (device_may_wakeup(dev)) + enable_irq_wake(data->irq); + else + mxt_stop(data); + } mutex_unlock(&input_dev->mutex); @@ -3332,8 +3338,12 @@ static int __maybe_unused mxt_resume(struct device *dev) mutex_lock(&input_dev->mutex); - if (input_device_enabled(input_dev)) - mxt_start(data); + if (input_device_enabled(input_dev)) { + if (device_may_wakeup(dev)) + disable_irq_wake(data->irq); + else + mxt_start(data); + } mutex_unlock(&input_dev->mutex);
Add capability for the touchscreen to wakeup the host on touch event. Signed-off-by: Loic Poulain <loic.poulain@linaro.org> --- drivers/input/touchscreen/atmel_mxt_ts.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) -- 2.7.4