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@xxxxxxxxxxxxxxx>
>
> ...
>
> > 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
https://bootlin.com
