> -----Original Message-----
> From: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> Sent: 2020年3月10日 12:27
> To: BOUGH CHEN <haibo.chen@xxxxxxx>
> Cc: linux-input@xxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>
> Subject: Re: [PATCH 2/2] input: egalax_ts: free irq resource before request
> the line as GPIO
>
> 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.
Agree, I will keep the code there. I will add a code in the egalax_ts_resume(), if the device my wakeup, skip egalax_wake_up_device(), return directly.
>
> > + egalax_free_irq(ts);
>
> It sounds to me you want simply disable interrupts in suspend. Does not
> calling disable_irq() here suffice?
Here why I want to disable interrupts here is because in the newest gpio system, if the gpio is request as an irq, it can't be request as a gpio anymore.
In the function egalax_wake_up_device(), we need to request the irq pin as a gpio for a while, generate a signal to wake up the device. So before request the pad as gpio, need first free irq resource.
Seems there are two methods, one is free irq in egalax_ts_suspend(), anther is just free irq in egalax_ts_suspend(), after the gpio operation done, request the irq resource back, which one do you prefer?
Best Regards
Haibo Chen
>
> Thanks.
>
> --
> Dmitry
