Re: [PATCH v2 3/5] rtc: s35390a: Fix alarm not disabling

On 01/08/2026 18:43:02+0000, Markus Probst wrote:
> Implement alarm_irq_enable callback to disable alarm if requested.
> On reenable restore last used alarm time.
> 
> Fixes: 542dd33a4925 ("drivers/rtc/rtc-s35390a.c: add wakealarm support for rtc-s35390A rtc chip")
> Signed-off-by: Markus Probst <[email protected]>
> ---
>  drivers/rtc/rtc-s35390a.c | 28 +++++++++++++++++++++++-----
>  1 file changed, 23 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> index 4cfe7034c516..fbf5471eb3d0 100644
> --- a/drivers/rtc/rtc-s35390a.c
> +++ b/drivers/rtc/rtc-s35390a.c
> @@ -64,6 +64,7 @@ MODULE_DEVICE_TABLE(of, s35390a_of_match);
>  
>  struct s35390a {
>  	struct i2c_client *client[8];
> +	struct rtc_time tm_alarm;
>  	int twentyfourhour;
>  };
>  
> @@ -320,6 +321,9 @@ static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  	err = s35390a_set_reg(s35390a, S35390A_CMD_INT2_REG1, buf,
>  								sizeof(buf));
>  
> +	if (!err)
> +		s35390a->tm_alarm = alm->time;
> +
>  	return err;
>  }
>  
> @@ -379,6 +383,19 @@ static int s35390a_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
>  	return 0;
>  }
>  
> +static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct s35390a *s35390a = dev_get_drvdata(dev);
> +	struct rtc_wkalrm alm;
> +
> +	alm.enabled = enabled;
> +
> +	if (enabled)
> +		alm.time = s35390a->tm_alarm;
> +
> +	return s35390a_rtc_set_alarm(dev, &alm);

You should do the opposite, set_alarm needs to call
s35390a_rtc_alarm_irq_enable instead or simply set S35390A_CMD_STATUS2
here.

> +}
> +
>  static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
>  			     unsigned long arg)
>  {
> @@ -410,11 +427,12 @@ static int s35390a_rtc_ioctl(struct device *dev, unsigned int cmd,
>  }
>  
>  static const struct rtc_class_ops s35390a_rtc_ops = {
> -	.read_time	= s35390a_rtc_read_time,
> -	.set_time	= s35390a_rtc_set_time,
> -	.set_alarm	= s35390a_rtc_set_alarm,
> -	.read_alarm	= s35390a_rtc_read_alarm,
> -	.ioctl          = s35390a_rtc_ioctl,
> +	.read_time		= s35390a_rtc_read_time,
> +	.set_time		= s35390a_rtc_set_time,
> +	.set_alarm		= s35390a_rtc_set_alarm,
> +	.read_alarm		= s35390a_rtc_read_alarm,
> +	.alarm_irq_enable	= s35390a_rtc_alarm_irq_enable,
> +	.ioctl			= s35390a_rtc_ioctl,
>  };
>  
>  static int s35390a_nvmem_read(void *priv, unsigned int offset, void *val,
> 
> -- 
> 2.54.0
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com