Re: [PATCH AUTOSEL 6.18-5.10] rtc: bq32000: add delay between RTC reads

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The delay workaround is bypassed if `client->irq > 0`, but because the driver lacks IRQ handling, userspace will always poll and trigger the hardware bug.
--

commit 50c036b36c688887d406f6107b0ade6653a75e5c
Author: Adriana Stancu <adriana@arista.com>

rtc: bq32000: add delay between RTC reads

This patch adds a delay before reading the BQ32000 RTC registers to prevent
stale reads or select timeouts during aggressive polling. The hardware needs
idle time to refresh its register buffer, which is starved when tools like
hwclock continuously poll on systems without an interrupt line.

Link: https://patch.msgid.link/20260416142151.3385827-1-adriana@arista.com

> diff --git a/drivers/rtc/rtc-bq32k.c b/drivers/rtc/rtc-bq32k.c
> index 7ad34539be4d2..245e5e45d6a16 100644
> --- a/drivers/rtc/rtc-bq32k.c
> +++ b/drivers/rtc/rtc-bq32k.c

[ ... ]

> @@ -89,9 +90,17 @@ static int bq32k_write(struct device *dev, void *data, uint8_t off, uint8_t len)
>  
>  static int bq32k_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
> +	struct i2c_client *client = to_i2c_client(dev);
>  	struct bq32k_regs regs;
>  	int error;
>  
> +	/*
> +	 * When the device doesn't have the interrupt connected, prevent
> +	 * userpace from polling the RTC registers too frequently.
> +	 */
> +	if (client->irq <= 0)

[Severity: Medium]
Will this condition incorrectly skip the delay workaround on systems that have
an interrupt defined in the device tree?

Because the BQ32000 driver itself lacks IRQ handling (RTC_FEATURE_ALARM is not
set and no alarm_irq_enable callback exists), the RTC core will always fall
back to UIE emulation. This emulation aggressively polls rtc_read_time()
regardless of whether client->irq > 0.

If client->irq > 0, the workaround is bypassed, but the hardware is still
subject to the exact same aggressive polling from userspace. Does this leave
these systems vulnerable to the same hardware register lockup (stale reads and
select timeouts) that this patch intends to fix?

> +		usleep_range(2000, 2500);
> +
>  	error = bq32k_read(dev, &regs, 0, sizeof(regs));
>  	if (error)
>  		return error;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-423-sashal@kernel.org?part=1