linux-rtc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [Resend PATCH v2 1/1] rtc: rtc-pm8xxx: Retrigger RTC alarm if it's fired
@ 2024-01-24  2:24 jianbinz
  2024-01-24  2:40 ` Alexandre Belloni
  0 siblings, 1 reply; 2+ messages in thread
From: jianbinz @ 2024-01-24  2:24 UTC (permalink / raw)
  To: alexandre.belloni, a.zummo; +Cc: jianbinz, linux-rtc, linux-kernel

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);
+
+	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;
 }
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [Resend PATCH v2 1/1] rtc: rtc-pm8xxx: Retrigger RTC alarm if it's fired
  2024-01-24  2:24 [Resend PATCH v2 1/1] rtc: rtc-pm8xxx: Retrigger RTC alarm if it's fired jianbinz
@ 2024-01-24  2:40 ` Alexandre Belloni
  0 siblings, 0 replies; 2+ messages in thread
From: Alexandre Belloni @ 2024-01-24  2:40 UTC (permalink / raw)
  To: jianbinz; +Cc: a.zummo, linux-rtc, linux-kernel

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
> 

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-01-24  2:40 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-24  2:24 [Resend PATCH v2 1/1] rtc: rtc-pm8xxx: Retrigger RTC alarm if it's fired jianbinz
2024-01-24  2:40 ` Alexandre Belloni

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).