On Tue, Feb 11, 2020 at 04:41:12PM +0800, haibo.chen@xxxxxxx wrote:
> From: Haibo Chen <haibo.chen@xxxxxxx>
>
> If GPIO is connected to an IRQ then it should not request it as
> GPIO function only when free its IRQ resource.
>
> Signed-off-by: Haibo Chen <haibo.chen@xxxxxxx>
> ---
> drivers/input/touchscreen/egalax_ts.c | 44 +++++++++++++++++++--------
> 1 file changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/input/touchscreen/egalax_ts.c b/drivers/input/touchscreen/egalax_ts.c
> index 5e35ca5edc7b..c7983104a0b9 100644
> --- a/drivers/input/touchscreen/egalax_ts.c
> +++ b/drivers/input/touchscreen/egalax_ts.c
> @@ -116,6 +116,26 @@ static irqreturn_t egalax_ts_interrupt(int irq, void *dev_id)
> return IRQ_HANDLED;
> }
>
> +static int egalax_irq_request(struct egalax_ts *ts)
> +{
> + int ret;
> + struct i2c_client *client = ts->client;
> +
> + ret = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> + egalax_ts_interrupt,
> + IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> + "egalax_ts", ts);
> + if (ret < 0)
> + dev_err(&client->dev, "Failed to register interrupt\n");
> +
> + return ret;
> +}
> +
> +static void egalax_free_irq(struct egalax_ts *ts)
> +{
> + devm_free_irq(&ts->client->dev, ts->client->irq, ts);
> +}
> +
> /* wake up controller by an falling edge of interrupt gpio. */
> static int egalax_wake_up_device(struct i2c_client *client)
> {
> @@ -225,19 +245,15 @@ static int egalax_ts_probe(struct i2c_client *client,
> ABS_MT_POSITION_Y, 0, EGALAX_MAX_Y, 0, 0);
> input_mt_init_slots(input_dev, MAX_SUPPORT_POINTS, 0);
>
> - error = devm_request_threaded_irq(&client->dev, client->irq, NULL,
> - egalax_ts_interrupt,
> - IRQF_TRIGGER_LOW | IRQF_ONESHOT,
> - "egalax_ts", ts);
> - if (error < 0) {
> - dev_err(&client->dev, "Failed to register interrupt\n");
> + error = egalax_irq_request(ts);
> + if (error)
> return error;
> - }
>
> error = input_register_device(ts->input_dev);
> if (error)
> return error;
>
> + i2c_set_clientdata(client, ts);
> return 0;
> }
>
> @@ -253,11 +269,10 @@ static int __maybe_unused egalax_ts_suspend(struct device *dev)
> 0x3, 0x6, 0xa, 0x3, 0x36, 0x3f, 0x2, 0, 0, 0
> };
> struct i2c_client *client = to_i2c_client(dev);
> + struct egalax_ts *ts = i2c_get_clientdata(client);
> int ret;
>
> - if (device_may_wakeup(dev))
> - return enable_irq_wake(client->irq);
> -
Why are you removing handling of the interrupt as a wakeup source if
device is configured for wakeup? I guess it is a noop and I2C core does
it for us on newer kernels anyways, but such cleanup should be in a
separate patch. Still, you do not want to put the controller to sleep if
it is supposed to be a wakeup source.
> + egalax_free_irq(ts);
It sounds to me you want simply disable interrupts in suspend. Does not
calling disable_irq() here suffice?
Thanks.
--
Dmitry
