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

[Mathieu Dubois-Briand]

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