Re: [PATCH v7 4/9] rtc: abx80x: properly handle shared IRQs

[Antoni Pokusinski <apokusinski01@gmail.com>]

On Wed, Sep 16, 2026 at 09:05:27PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] rtc: abx80x: race condition loses interrupts due to flawed W0C status register clearing
> --
> 
> commit 27cfcb7135851594344aa538f01f8ef1328d2392
> Author: Antoni Pokusinski <apokusinski01@gmail.com>
> 
> rtc: abx80x: properly handle shared IRQs
> 
> This commit updates the abx80x RTC interrupt handler to only return
> IRQ_HANDLED and clear the status register when the alarm or watchdog flags
> are set. It aims to properly support IRQF_SHARED by returning IRQ_NONE
> for unrelated interrupts.
> 
> > diff --git a/drivers/rtc/rtc-abx80x.c b/drivers/rtc/rtc-abx80x.c
> > --- a/drivers/rtc/rtc-abx80x.c
> > +++ b/drivers/rtc/rtc-abx80x.c
> 
> [ ... ]
> 
> > @@ -284,9 +291,15 @@ static irqreturn_t abx80x_handle_irq(int irq, void *dev_id)
> >  	if (status & ABX8XX_STATUS_WDT)
> >  		dev_alert(&client->dev, "watchdog timeout interrupt.\n");
> >  
> > -	i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS, 0);
> > +	status_flags_clear = status & (ABX8XX_STATUS_AF | ABX8XX_STATUS_WDT);
> >  
> > -	return IRQ_HANDLED;
> > +	if (status_flags_clear) {
> > +		i2c_smbus_write_byte_data(client, ABX8XX_REG_STATUS,
> > +					  status & ~status_flags_clear);
> 
> [Severity: High]
> Does this logic introduce a race condition that loses late-arriving interrupts
> in abx80x_handle_irq()?
> 
> Because the ABX8XX_REG_STATUS register is W0C (write-0-to-clear), any bit
> written as 0 will be cleared in hardware. 
> 
> The expression status & ~status_flags_clear forces a 0 for all bits that
> were 0 during the initial read. If a new hardware interrupt asserts between
> the read and this write, its status bit will be newly set to 1 in hardware.
> However, writing back the old status value forces a 0 for that new bit,
> silently clearing it and losing the event.
> 
> To preserve unhandled or newly asserted bits, the correct write value for a
> W0C register is ~status_flags_clear instead.
>
Not really: if the register bits are written with 1, then during the
next call to `handle_irq()` all the interrupts are reported as pending
(in other words the written ones are preserved). 
In particular, when I was testing the alarm interrupt and the written value was
`~status_flags_clear` as sashiko suggests, I was always getting the WDT
bit enabled and hence the alert "watchdog timeout interrupt" was
printed.
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260916205138.16771-1-apokusinski01@gmail.com?part=4