Re: [PATCH] counter: 104-quad-8: Fix incorrect return value in IRQ handler

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Sat, Dec 06, 2025 at 01:24:16PM +0900, William Breathitt Gray wrote:
> On Tue, Dec 02, 2025 at 04:39:52PM +0800, Haotian Zhang wrote:
> > quad8_irq_handler() should return irqreturn_t enum values, but it
> > directly returns negative errno codes from regmap operations on error.
> > 
> > Return IRQ_NONE instead of raw errno codes on regmap operation failures.

...

> >  	ret = regmap_read(priv->map, QUAD8_INTERRUPT_STATUS, &status);
> >  	if (ret)
> > -		return ret;
> > +		return IRQ_NONE;

This return is correct and we can't do much, if we got again to this handler
due to unserviced IRQ, hopefully the second attempt will succeed. At the end it
will mean something really bad happened to the HW state.

> >  	if (!status)
> >  		return IRQ_NONE;

...

> >  	ret = regmap_write(priv->map, QUAD8_CHANNEL_OPERATION, CLEAR_PENDING_INTERRUPTS);
> >  	if (ret)
> > -		return ret;
> > +		return IRQ_NONE;
> > 
> >  	return IRQ_HANDLED;
> 
> You are correct, we should return a value of irqreturn_t and not raw
> errno codes. However, it would be nice to indicate to users why the IRQ
> was left unserviced before return IRQ_NONE. Is there a way to indicate
> the regmap_read failure, perhaps via WARN_ONCE() or similar? Is
> regmap_read actually capable of failing in this context, or should we
> just remove the conditional check entirely?

I'm not sure about this case, clearing pending interrupts is something
that should not affect much the flow, I think if we return IRQ_HANDLED here
we can re-enter to the handler and re-read the status. Yes, it will be
spurious interrupt, but at least it will reduce the probability of IRQ storm
registration (when when we return IRQ_NONE).

-- 
With Best Regards,
Andy Shevchenko