Re: [PATCH 2/2] input: egalax_ts: free irq resource before request the line as GPIO
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-10 04:26:48
On Tue, Feb 11, 2020 at 04:41:12PM +0800, haibo.chen@nxp.com wrote:
quoted hunk ↗ jump to hunk
From: Haibo Chen <haibo.chen@nxp.com> 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@nxp.com> --- 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