Re: [PATCH v7 7/9] rtc: abx80x: replace read-modify-write pattern with regmap helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rtc: abx80x: hardware unlock broken by regmap RMW helpers
--

commit aad0cc39d854acf9e4ccd3b1fa7b17484a428c9a
Author: Antoni Pokusinski <apokusinski01@gmail.com>

rtc: abx80x: replace read-modify-write pattern with regmap helpers

This commit refactors the driver to replace open-coded read-modify-write
sequences with the regmap_update_bits() and regmap_write_bits() helpers to
simplify the code.

> 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
[ ... ]
> @@ -403,13 +388,15 @@ static int abx80x_rtc_set_autocalibration(struct device *dev,
>  		flags |= (ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512);
>  	}
>  
> +	guard(mutex)(&priv->lock);
> +
>  	/* Unlock write access to Oscillator Control Register */
>  	if (abx80x_write_config_key(dev, ABX8XX_CFG_KEY_OSC) < 0)
>  		return -EIO;
>  
> -	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
> -
> -	return retval;
> +	return regmap_write_bits(priv->regmap, ABX8XX_REG_OSC,
> +				 ABX8XX_OSC_ACAL_1024 | ABX8XX_OSC_ACAL_512,
> +				 flags);
>  }

[Severity: High]
Since regmap_write_bits() performs an internal read-modify-write cycle, will
the initial read operation consume the configuration key unlock? 

It looks like writing the configuration key only unlocks the very next 
transaction. If the regmap helper reads first, the actual write might fail 
silently and the hardware would reject the autocalibration settings.

[ ... ]
> @@ -486,24 +472,14 @@ static ssize_t oscillator_store(struct device *dev,
>  
>  	guard(mutex)(&priv->lock);
>  
> -	retval = regmap_read(priv->regmap, ABX8XX_REG_OSC, &flags);
> -	if (retval < 0)
> -		return retval;
> -
> -	if (rc_mode == 0)
> -		flags &= ~(ABX8XX_OSC_OSEL);
> -	else
> -		flags |= (ABX8XX_OSC_OSEL);
> -
>  	/* Unlock write access on Oscillator Control register */
>  	if (abx80x_write_config_key(dev->parent, ABX8XX_CFG_KEY_OSC) < 0)
>  		return -EIO;
>  
> -	retval = regmap_write(priv->regmap, ABX8XX_REG_OSC, flags);
> -	if (retval < 0) {
> +	retval = regmap_write_bits(priv->regmap, ABX8XX_REG_OSC, ABX8XX_OSC_OSEL,
> +				   rc_mode == 0 ? 0 : (ABX8XX_OSC_OSEL));
> +	if (retval < 0)
>  		dev_err(dev, "Failed to write Oscillator Control register\n");

[Severity: High]
Does the same issue apply here in oscillator_store()? The internal read 
within regmap_write_bits() may consume the unlock key, preventing the 
oscillator selection from being properly updated.

[ ... ]
> @@ -946,8 +889,14 @@ static int abx80x_probe(struct i2c_client *client)
>  		if (abx80x_write_config_key(&client->dev, ABX8XX_CFG_KEY_MISC) < 0)
>  			return -EIO;
>  
> -		err = regmap_write(regmap, ABX8XX_REG_OUT_CTRL,
> -				   data | ABX8XX_OUT_CTRL_EXDS);
> +		/*
> +		 * Avoid extra power leakage. The RV1805 uses smaller
> +		 * 10pin package and the EXTI input is not present.
> +		 * Disable it to avoid leakage.
> +		 */
> +		err = regmap_write_bits(regmap, ABX8XX_REG_OUT_CTRL,
> +					ABX8XX_OUT_CTRL_EXDS,
> +					ABX8XX_OUT_CTRL_EXDS);
>  		if (err < 0) {

[Severity: High]
Could this cause the power leakage mitigation write in abx80x_probe() to be 
rejected by the hardware because the unlock transaction was already consumed 
by the regmap_write_bits() read phase?

[ ... ]
> @@ -1014,11 +949,10 @@ static int abx80x_probe(struct i2c_client *client)
>  	if (abx80x_write_config_key(client, ABX8XX_CFG_KEY_OSC) < 0)
>  		return -EIO;
>  
> -	err = regmap_write(regmap, ABX8XX_REG_OSC,
> -			   data & ~(ABX8XX_OSC_ACIE |
> -				    ABX8XX_OSC_OFIE));
> +	err = regmap_write_bits(regmap, ABX8XX_REG_OSC,
> +				ABX8XX_OSC_ACIE | ABX8XX_OSC_OFIE, 0);
>  	if (err < 0) {

[Severity: High]
Will the interrupt disable write also fail silently here due to the unlock 
being consumed by the regmap read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916205138.16771-1-apokusinski01@gmail.com?part=7