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

[William Breathitt Gray <wbg@kernel.org>]

On Sun, Dec 07, 2025 at 02:00:07AM +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 if the interrupt status cannot be read. If clearing the
> interrupt fails, return IRQ_HANDLED to prevent the kernel from disabling
> the IRQ line due to a spurious interrupt storm. Also, log these regmap
> failures with WARN_ONCE.
> 
> Fixes: 98ffe0252911 ("counter: 104-quad-8: Migrate to the regmap API")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
> Changes in v2:
>   -Return IRQ_HANDLED if regmap_write() fails.
>   -Add WARN_ONCE() to log the error messages.

Hi Haotian,

Thank you for making the requested changes. I have a couple more
suggestions below, and then I think this patch will be ready to merge.

>  	ret = regmap_read(priv->map, QUAD8_INTERRUPT_STATUS, &status);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		WARN_ONCE(true, "quad8: regmap_read failed: %d\n", ret);

The warning should indicate the purpose of the operation so users know
what failed for the hardware. So perhaps "quad8: Attempt to read
Interrupt Status Register failed: %d\n" is better.

>  	ret = regmap_write(priv->map, QUAD8_CHANNEL_OPERATION, CLEAR_PENDING_INTERRUPTS);
> -	if (ret)
> -		return ret;
> +	if (ret) {
> +		WARN_ONCE(true, "quad8: regmap_write failed: %d\n", ret);

For the same reason as above, perhaps "quad8: Attempt to clear pending
interrupts by writing to Channel Operation Register failed: %d\n" is
better.

William Breathitt Gray