On Tue, Dec 26, 2023 at 12:21:07PM -0700, Mark Hasemeyer wrote:
> The i2c_acpi_irq_context structure provides redundant information that
> can be provided with struct resource.
>
> Refactor i2c_acpi_get_irq() to use struct resource instead of struct
> i2c_acpi_irq_context.
...
> Suggested-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxx>
>
> Signed-off-by: Mark Hasemeyer <markhas@xxxxxxxxxxxx>
No blank line.
...
> ret = acpi_dev_get_resources(adev, &resource_list,
> - i2c_acpi_add_irq_resource, &irq_ctx);
> + i2c_acpi_add_irq_resource, r);
> if (ret < 0)
> return ret;
>
> acpi_dev_free_resource_list(&resource_list);
>
> - if (irq_ctx.irq == -ENOENT) {
> - ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, &irqres);
> - if (ret)
> - return ret;
> - irq_ctx.irq = irqres.start;
> - irq_ctx.wake_capable = irqres.flags & IORESOURCE_IRQ_WAKECAPABLE;
> - }
> -
> - if (irq_ctx.irq < 0)
> - return irq_ctx.irq;
> + if (!r->flags)
> + ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r);
>
> - if (wake_capable)
> - *wake_capable = irq_ctx.wake_capable;
> + if (!r->flags)
> + return ret;
>
> - return irq_ctx.irq;
> + return r->start;
Wondering if we can refactor above as
if (r->flags)
return r->start;
ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r);
if (ret)
return ret;
return r->start;
Note also the 'if (ret)' check, the check for flags to return ret seems
counter intuitive and possible prone to errors in the future.
--
With Best Regards,
Andy Shevchenko
