Re: [PATCH v4 3/5] platform/chrome: cros_ec: initialize `wake_enabled` in cros_ec_register()

[Tzung-Bi Shih]

On Tue, Feb 15, 2022 at 09:47:09PM -0800, Prashant Malani wrote:
> On Tue, Feb 15, 2022 at 8:37 PM Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote:
> >
> > `wake_enabled` indicates cros_ec_resume() needs to call
> > disable_irq_wake() to undo enable_irq_wake() in cros_ec_suspend().
> >
> > Initialize `wake_enabled` in cros_ec_register() and determine the flag
> > in cros_ec_suspend() instead of reset-after-used in cros_ec_resume().

After reconsidering the 2 options in [1], I feel the flag needs to be set in
cros_ec_suspend() just in case if someone changes the wakeup capability per
[2].

[1]: https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-4-tzungbi@xxxxxxxxxx/#24739778
[2]: https://patchwork.kernel.org/project/chrome-platform/patch/20220209045035.380615-4-tzungbi@xxxxxxxxxx/#24740205

Will change it back in the next version, pardon me.

> > @@ -383,10 +384,9 @@ int cros_ec_resume(struct cros_ec_device *ec_dev)
> >                 dev_dbg(ec_dev->dev, "Error %d sending resume event to ec",
> >                         ret);
> >
> > -       if (ec_dev->wake_enabled) {
> > +       if (ec_dev->wake_enabled)
> >                 disable_irq_wake(ec_dev->irq);
> > -               ec_dev->wake_enabled = 0;
> > -       }
> > +
> 
> Better to leave it as is, and ensure "wake_enabled" is cleared after resume?
> Will result in a smaller diff.

No, cros_ec_suspend() uses the flag to tell cros_ec_resume(): don't forget to
call disable_irq_wake().  It shouldn't be reset after used by
cros_ec_resume().