lkml.org 
[lkml]   [2022]   [Sep]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v4 07/13] i2c: acpi: Use ACPI wake capability bit to set wake_irq
On Wed, Sep 21, 2022 at 09:18:34AM -0600, Raul Rangel wrote:
> On Tue, Sep 20, 2022 at 6:32 AM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Mon, Sep 19, 2022 at 09:59:09AM -0600, Raul E Rangel wrote:

...

> > > + if (irq_ctx.irq == -ENOENT)
> > > + irq_ctx.irq = acpi_dev_gpio_irq_wake_get(adev, 0, &irq_ctx.wake_capable);
> >
> > I just realized, that there is an inconsistency on how we fill the wake_capable
> > parameter. In some cases we check for IRQ for an error condition (IRQ not found)
> > and in some the wake_capable still be filled.
> >
> > Here the best approach I believe is to add
> >
> > if (irq_ctx.irq < 0)
> > return irq_ctx.irq;
> >
> > I.o.w. we apply the rule "do not fill the output parameters when it's known
> > to be an error condition".
> >
> > > + if (wake_capable)
> > > + *wake_capable = irq_ctx.wake_capable;
> >
> > > + return irq_ctx.irq;
> >
>
> I applied the following:
> diff --git a/drivers/i2c/i2c-core-acpi.c b/drivers/i2c/i2c-core-acpi.c
> index ba64e505183595..1618f5619d5ed9 100644
> --- a/drivers/i2c/i2c-core-acpi.c
> +++ b/drivers/i2c/i2c-core-acpi.c
> @@ -220,7 +220,7 @@ int i2c_acpi_get_irq(struct i2c_client *client,
> bool *wake_capable)
> if (irq_ctx.irq == -ENOENT)
> irq_ctx.irq = acpi_dev_gpio_irq_wake_get(adev, 0,
> &irq_ctx.wake_capable);
>
> - if (wake_capable)
> + if (irq_ctx.irq > 0 && wake_capable)
> *wake_capable = irq_ctx.wake_capable;
>
> return irq_ctx.irq;

While it's working solution it is not so flexible since basically any addition
of a new code will require if (irq > 0), that's why I'm in favour of my
proposal rather than yours approach.


--
With Best Regards,
Andy Shevchenko


\
 
 \ /
  Last update: 2022-09-22 11:44    [W:0.097 / U:0.016 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site