Re: [PATCH] Input: elan_i2c: Disable irq on shutdown

Stephen Boyd
2021-06-03T06:14:50Z

Quoting Dmitry Torokhov (2021-06-02 21:33:41)
> On Wed, Jun 02, 2021 at 06:33:49PM -0700, Stephen Boyd wrote:
> > Quoting Dmitry Torokhov (2021-06-02 18:18:17)
> > >
> > > I do not think keeping counter balanced would be important here, as we
> > > are shutting down, and upon reboot everything will be reinitialized from
> > > scratch. Also, we are lucky in that there is just a handful of I2C
> > > drivers defining shutdown() methods.
> > >
> > > > Please don't make me shave this yak.
> > >
> > > I'm afraid someone has to... I'm adding Wolfram to CC to get his take on
> > > this.
> > >
> >
> > I suppose another option would be to introduce some common function that
> > i2c drivers can use for their shutdown op, like i2c_generic_shutdown()
> > that would disable the irq? I would guess that it isn't a great idea to
> > blanket disable the irq in case some i2c driver wants to do something
> > that may require that irq to come in once more during shutdown to signal
> > that things are off or something like that.
> >
> > Would having this common function that this driver opts into work for
> > you?
>
> Opting in in this fashion will still require changes in the majority
> of drivers (any I2C touchscreen or touchpad may be touched while system
> is being shut down, so all of them will need to have interrupt freed or
> disabled, or they may initiate I2C transfer). How about something like
> this;

Yes, this approach should work. I assume a device that doesn't have a
shutdown function will be happy to let the i2c core disable the irq for
it so it looks low risk.

Will you send a proper patch to Wolfram or would you like me to wrap it
up and resend?

>
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 38107c0c318c..c835e7bb71de 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -603,9 +603,12 @@ static void i2c_device_shutdown(struct device *dev)
>
> if (!client || !dev->driver)
> return;
> +
> driver = to_i2c_driver(dev->driver);
> if (driver->shutdown)
> driver->shutdown(client);
> + else if (client->irq > 0)
> + disable_irq(client->irq);
> }
>
> static void i2c_client_dev_release(struct device *dev)
>