Re: [PATCH v5 05/11] regmap: irq: Add support for chips without separate IRQ status

Mathieu Dubois-Briand
2025-03-20T08:45:31Z

On Tue Mar 18, 2025 at 5:39 PM CET, Andy Shevchenko wrote:
> On Tue, Mar 18, 2025 at 05:26:21PM +0100, Mathieu Dubois-Briand wrote:
> > Some GPIO chips allow to rise an IRQ on GPIO level changes but do not
> > provide an IRQ status for each separate line: only the current gpio
> > level can be retrieved.
> >
> > Add support for these chips, emulating IRQ status by comparing GPIO
> > levels with the levels during the previous interrupt.
>
>
> Some nit-picks below, but either way
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>
> ...
>
> > default:
> > BUG();
> > - goto exit;
> > + return ret;
>
> Hmm... BUG() implies unreachable, perhaps just a precursor patch to drop this
> goto completely?
>

Ok, I will add a separate patch to remove the goto.

> ...
>
> > + /* Store current levels */
> > + if (chip->status_is_level) {
> > + ret = read_irq_data(d);
> > + if (ret < 0)
> > + goto err_alloc;
> > +
> > + memcpy(d->prev_status_buf, d->status_buf,
> > + d->chip->num_regs * sizeof(d->prev_status_buf[0]));
>
> Perhaps array_size()?
>

OK

> > + }

Thanks for your review, and thanks for the tag.
Mathieu

--
Mathieu Dubois-Briand, Bootlin
Embedded Linux and Kernel engineering