On Wed, Dec 20, 2023 at 04:54:16PM -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? ... > static int i2c_acpi_add_irq_resource(struct acpi_resource *ares, void *data) > { > - struct i2c_acpi_irq_context *irq_ctx = data; > - struct resource r; > + struct resource *r = data; > - if (irq_ctx->irq > 0) > + if (r->start > 0) > return 1; Checking flags is more robust. if (r->flags) return 1; > - if (!acpi_dev_resource_interrupt(ares, 0, &r)) > + if (!acpi_dev_resource_interrupt(ares, 0, r)) > return 1; > > - irq_ctx->irq = i2c_dev_irq_from_resources(&r, 1); > - irq_ctx->wake_capable = r.flags & IORESOURCE_IRQ_WAKECAPABLE; > + i2c_dev_irq_from_resources(r, 1); > > return 1; /* No need to add resource to the list */ > } ... > + if (IS_ERR_OR_NULL(r)) > + return -EINVAL; Hmm... Do we expect this to be an error pointer in some cases? ... > + ret = acpi_dev_get_gpio_irq_resource(adev, NULL, 0, r); > + if (!ret) > + return r->start; > > - return irq_ctx.irq; > + return ret; What's wrong with the standard pattern? if (ret) return ret; ... return ...; ... > + struct resource r = {0}; '0' is redundant. ... > + irq = i2c_acpi_get_irq(client, &r); > + if (irq > 0 && r.flags & IORESOURCE_IRQ_WAKECAPABLE) Why checking just flags is not enough? > client->flags |= I2C_CLIENT_WAKE; -- With Best Regards, Andy Shevchenko