| Message ID | 20240124022443.21867-1-quic_jianbinz@quicinc.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [Resend,v2,1/1] rtc: rtc-pm8xxx: Retrigger RTC alarm if it's fired | expand |
Hello, On 24/01/2024 10:24:43+0800, jianbinz wrote: > If the alarm is triggered before the driver gets probed, the alarm > interrupt will be missed and it won't be detected, and the stale > alarm settings will be still retained because of not being cleared. > Check this condition during driver probe, retrigger the alarm and > clear the settings manually if it's such case. > > Changes in v2: > *Adapt the V1 patch according to the newest rtc-pm8xxx > > Changes in v1: > *During driver probe: read ALARM_EN, read ALARM_DATA, read RTC_RDATA, > if (ALARM_DATA < RTC_DATA), Trigger the alarm event and clear the alarm settins > Link to v1:https://lore.kernel.org/linux-rtc/20220321090514.4523-1-quic_jianbinz@quicinc.com/ > > Signed-off-by: jianbinz <quic_jianbinz@quicinc.com> > --- > drivers/rtc/rtc-pm8xxx.c | 57 +++++++++++++++++++++++++++++++++++++--- > 1 file changed, 53 insertions(+), 4 deletions(-) > > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c > index f6b779c12ca7..eac4e7f23aaa 100644 > --- a/drivers/rtc/rtc-pm8xxx.c > +++ b/drivers/rtc/rtc-pm8xxx.c > @@ -309,21 +309,33 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) > return 0; > } > > +static int pm8xxx_rtc_read_alarm_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) > +{ > + const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; > + u8 value[NUM_8_BIT_RTC_REGS]; > + int rc; > + > + rc = regmap_bulk_read(rtc_dd->regmap, regs->read, value, sizeof(value)); > + if (rc) > + return rc; > + > + *secs = get_unaligned_le32(value); > + > + return 0; > +} > + > static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) > { > struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); > const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; > - u8 value[NUM_8_BIT_RTC_REGS]; > unsigned int ctrl_reg; > u32 secs; > int rc; > > - rc = regmap_bulk_read(rtc_dd->regmap, regs->alarm_rw, value, > - sizeof(value)); > + rc = pm8xxx_rtc_read_alarm_raw(rtc_dd, &secs); > if (rc) > return rc; > > - secs = get_unaligned_le32(value); > secs += rtc_dd->offset; > rtc_time64_to_tm(secs, &alarm->time); > > @@ -398,6 +410,39 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +/* > + * Trigger the alarm event and clear the alarm settings > + * if the alarm data has been behind the RTC data which > + * means the alarm has been triggered before the driver > + * is probed. > + */ > +static int pm8xxx_rtc_init_alarm(struct pm8xxx_rtc *rtc_dd) > +{ > + int rc; > + u32 alarm_sec, rtc_sec; > + unsigned int ctrl_reg, alarm_en; > + const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; > + > + rc = pm8xxx_rtc_read_raw(rtc_dd, &rtc_sec); > + if (rc) > + return rc; > + > + rc = pm8xxx_rtc_read_alarm_raw(rtc_dd, &alarm_sec); > + if (rc) > + return rc; > + > + rc = regmap_read(rtc_dd->regmap, regs->alarm_ctrl, &ctrl_reg); > + if (rc) > + return rc; > + > + alarm_en = !!(ctrl_reg & PM8xxx_RTC_ALARM_ENABLE); > + > + if (alarm_en && rtc_sec >= alarm_sec) > + pm8xxx_alarm_trigger(0, rtc_dd); I have not been replying because I had to go back to the original description of the problem which you are not linking here. Anyway, all of this doesn't do want you explain, you are looking at whether the alarm is enabled but not whether it has actually triggered... > + > + return 0; > +} > + > static int pm8xxx_rtc_enable(struct pm8xxx_rtc *rtc_dd) > { > const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; > @@ -527,6 +572,10 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) > if (rc) > return rc; > > + rc = pm8xxx_rtc_init_alarm(rtc_dd); ...But isn't it useless? I guess that what you can do and what I was referring to in my first reply is clear the interrupt in your probe unconditionally, here: rc = regmap_update_bits(rtc_dd->regmap, regs->alarm_ctrl2, PM8xxx_RTC_ALARM_CLEAR, 0); That should do it just fine. There is no point in calling pm8xxx_alarm_trigger because there is no chance that userspace is ready to handle the rtc update. > + if (rc) > + return rc; > + > return 0; > } > > -- > 2.17.1 >
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c index f6b779c12ca7..eac4e7f23aaa 100644 --- a/drivers/rtc/rtc-pm8xxx.c +++ b/drivers/rtc/rtc-pm8xxx.c @@ -309,21 +309,33 @@ static int pm8xxx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alarm) return 0; } +static int pm8xxx_rtc_read_alarm_raw(struct pm8xxx_rtc *rtc_dd, u32 *secs) +{ + const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; + u8 value[NUM_8_BIT_RTC_REGS]; + int rc; + + rc = regmap_bulk_read(rtc_dd->regmap, regs->read, value, sizeof(value)); + if (rc) + return rc; + + *secs = get_unaligned_le32(value); + + return 0; +} + static int pm8xxx_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alarm) { struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev); const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; - u8 value[NUM_8_BIT_RTC_REGS]; unsigned int ctrl_reg; u32 secs; int rc; - rc = regmap_bulk_read(rtc_dd->regmap, regs->alarm_rw, value, - sizeof(value)); + rc = pm8xxx_rtc_read_alarm_raw(rtc_dd, &secs); if (rc) return rc; - secs = get_unaligned_le32(value); secs += rtc_dd->offset; rtc_time64_to_tm(secs, &alarm->time); @@ -398,6 +410,39 @@ static irqreturn_t pm8xxx_alarm_trigger(int irq, void *dev_id) return IRQ_HANDLED; } +/* + * Trigger the alarm event and clear the alarm settings + * if the alarm data has been behind the RTC data which + * means the alarm has been triggered before the driver + * is probed. + */ +static int pm8xxx_rtc_init_alarm(struct pm8xxx_rtc *rtc_dd) +{ + int rc; + u32 alarm_sec, rtc_sec; + unsigned int ctrl_reg, alarm_en; + const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; + + rc = pm8xxx_rtc_read_raw(rtc_dd, &rtc_sec); + if (rc) + return rc; + + rc = pm8xxx_rtc_read_alarm_raw(rtc_dd, &alarm_sec); + if (rc) + return rc; + + rc = regmap_read(rtc_dd->regmap, regs->alarm_ctrl, &ctrl_reg); + if (rc) + return rc; + + alarm_en = !!(ctrl_reg & PM8xxx_RTC_ALARM_ENABLE); + + if (alarm_en && rtc_sec >= alarm_sec) + pm8xxx_alarm_trigger(0, rtc_dd); + + return 0; +} + static int pm8xxx_rtc_enable(struct pm8xxx_rtc *rtc_dd) { const struct pm8xxx_rtc_regs *regs = rtc_dd->regs; @@ -527,6 +572,10 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev) if (rc) return rc; + rc = pm8xxx_rtc_init_alarm(rtc_dd); + if (rc) + return rc; + return 0; }
If the alarm is triggered before the driver gets probed, the alarm interrupt will be missed and it won't be detected, and the stale alarm settings will be still retained because of not being cleared. Check this condition during driver probe, retrigger the alarm and clear the settings manually if it's such case. Changes in v2: *Adapt the V1 patch according to the newest rtc-pm8xxx Changes in v1: *During driver probe: read ALARM_EN, read ALARM_DATA, read RTC_RDATA, if (ALARM_DATA < RTC_DATA), Trigger the alarm event and clear the alarm settins Link to v1:https://lore.kernel.org/linux-rtc/20220321090514.4523-1-quic_jianbinz@quicinc.com/ Signed-off-by: jianbinz <quic_jianbinz@quicinc.com> --- drivers/rtc/rtc-pm8xxx.c | 57 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 53 insertions(+), 4 deletions(-)