linux-kernel - Re: [PATCH] Input: elan_i2c: Disable irq on shutdown

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAE-0n50PZ+vUH_dSQnunViWrkEdQNHyLNHjKbrsbEbO-YL8DRA@mail.gmail.com>
Date:   Wed, 2 Jun 2021 23:13:40 -0700
From:   Stephen Boyd <swboyd@...omium.org>
To:     Dmitry Torokhov <dmitry.torokhov@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-input@...r.kernel.org,
        Jingle Wu <jingle.wu@....com.tw>, Wolfram Sang <wsa@...nel.org>
Subject: Re: [PATCH] Input: elan_i2c: Disable irq on shutdown

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)
>