Re: [PATCH v5 3/6] rtc: s35390a: Fix alarm not disabling

[Markus Probst]

On Thu, 2026-08-20 at 00:25 +0200, Alexandre Belloni wrote:
> On 19/08/2026 22:05:55+0000, Markus Probst wrote:
> > Implement alarm_irq_enable callback.
> > 
> > Fixes: 542dd33a4925 ("drivers/rtc/rtc-s35390a.c: add wakealarm support for rtc-s35390A rtc chip")
> > Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
> > ---
> >  drivers/rtc/rtc-s35390a.c | 29 ++++++++++++++++++++++++-----
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/rtc/rtc-s35390a.c b/drivers/rtc/rtc-s35390a.c
> > index 4cfe7034c516..575bb256eb25 100644
> > --- a/drivers/rtc/rtc-s35390a.c
> > +++ b/drivers/rtc/rtc-s35390a.c
> > @@ -270,6 +270,24 @@ static int s35390a_rtc_read_time(struct device *dev, struct rtc_time *tm)
> >  	return 0;
> >  }
> >  
> > +static int s35390a_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> > +{
> > +	struct s35390a *s35390a = dev_get_drvdata(dev);
> > +	u8 sts;
> > +	int err;
> > +
> > +	if (enabled)
> > +		sts = S35390A_INT2_MODE_ALARM;
> > +	else
> > +		sts = S35390A_INT2_MODE_NOINTR;
> > +
> > +	err = s35390a_set_reg(s35390a, S35390A_CMD_STATUS2, &sts, sizeof(sts));
> > +	if (err < 0)
> > +		return err;
> > +
> > +	return 0;
> > +}
> 
> 
> You can definitively call this from s35390a_rtc_set_alarm instead of
> duplicating code.
At this stage yes.

With the pinctrl patch, it needs to be considered that the other pin
might be used as well, so the state needs to be preserved.

This will be done by reading the S35390A_CMD_STATUS2 register first.

This function will not be called in set_alarm, because this would
introduce additional read operations, as the current
S35390A_CMD_STATUS2 reg is already read inside set_alarm.

Thanks
- Markus Probst

> 
> > +
> >  static int s35390a_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> >  {
> >  	struct i2c_client *client = to_i2c_client(dev);
> > @@ -410,11 +428,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
> >

Attachment: signature.asc
Description: This is a digitally signed message part