Re: [PATCH v3 3/4] rtc: pcf85063: preserve the alarm flag in clkout register updates

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch correctly fixes the read-modify-write race condition for the alarm flag (AF) in the clkout helpers, but it completely misses the exact same bug in `pcf85063_rtc_alarm_irq_enable()`.
--

commit 10941c8a0cce0c4a8065c1c9da8c092b66df0282
Author: Alexander Sverdlin <alexander.sverdlin@siemens.com>

rtc: pcf85063: preserve the alarm flag in clkout register updates

This commit fixes a read-modify-write race condition for the alarm flag (AF)
in the clkout helpers by ensuring AF is preserved during regmap updates.

> diff --git a/drivers/rtc/rtc-pcf85063.c b/drivers/rtc/rtc-pcf85063.c
> index af900fe8ae15e..a3af86456ccfc 100644
> --- a/drivers/rtc/rtc-pcf85063.c
> +++ b/drivers/rtc/rtc-pcf85063.c
> @@ -431,7 +431,8 @@ static int pcf85063_clkout_set_rate(struct clk_hw *hw, unsigned long rate,
>  		if (clkout_rates[i] == rate)
>  			return regmap_update_bits(pcf85063->regmap,
>  				PCF85063_REG_CTRL2,
> -				PCF85063_REG_CLKO_F_MASK, i);
> +				PCF85063_REG_CLKO_F_MASK | PCF85063_CTRL2_AF,
> +				i | PCF85063_CTRL2_AF);
>  
>  	return -EINVAL;
>  }
> @@ -460,7 +461,8 @@ static int pcf85063_clkout_control(struct clk_hw *hw, bool enable)
>  	}
>  
>  	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
> -					PCF85063_REG_CLKO_F_MASK, buf);
> +				  PCF85063_REG_CLKO_F_MASK | PCF85063_CTRL2_AF,
> +				  buf | PCF85063_CTRL2_AF);
>  }

[Severity: High]
This is a pre-existing issue, but does pcf85063_rtc_alarm_irq_enable() also
suffer from the exact same race condition that this patch addresses?

Looking at drivers/rtc/rtc-pcf85063.c:pcf85063_rtc_alarm_irq_enable():

	return regmap_update_bits(pcf85063->regmap, PCF85063_REG_CTRL2,
				  PCF85063_CTRL2_AIE,
				  enabled ? PCF85063_CTRL2_AIE : 0);

If the alarm fires exactly during this read-modify-write operation, wouldn't
the hardware alarm flag (AF) be inadvertently cleared since it is omitted from
the mask? This could lead to a lost RTC alarm interrupt and cause the system
to miss critical wakeups.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260828164445.3907839-1-alexander.sverdlin@siemens.com?part=3