2024-04-01 01:57:20

by jianbin zhang

[permalink] [raw]
Subject: [PATCH V4 1/2] rtc-pm8xxx: clear the triggered alarm interrupt during driver probe

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.

Issue reproduce step:
(1) set the alarm and poweroff the device
(2) alarm happens and the device boots
(3) poweroff the device again
(4) alarm irq not be cleard, device boots again

the fixing here is clear the interrupt during the step(3) unconditionally.

Signed-off-by: jianbin zhang <[email protected]>
---
Changes in v4:
- add the cover letter
- modify the patch to conform to the specification

Changes in v3:
- clear the interrupt in driver probe unconditionally
- link: https://lore.kernel.org/linux-rtc/[email protected]/T/#t

Changes in v2:
- Adapt the V1 patch according to the newest rtc-pm8xxx
- link: https://lore.kernel.org/linux-rtc/[email protected]/

Changes in v1:
- fixing is as below logic, 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: https://lore.kernel.org/linux-rtc/[email protected]/

Changes in original:
- link to original: https://lore.kernel.org/linux-rtc/[email protected]/
---
drivers/rtc/rtc-pm8xxx.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index f6b779c12ca7..e4e2307445cf 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -527,6 +527,11 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
if (rc)
return rc;

+ rc = regmap_update_bits(rtc_dd->regmap, rtc_dd->regs->alarm_ctrl2,
+ PM8xxx_RTC_ALARM_CLEAR, 1);
+ if (rc)
+ return rc;
+
return 0;
}


--
2.43.2



2024-04-05 02:49:31

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH V4 1/2] rtc-pm8xxx: clear the triggered alarm interrupt during driver probe

On Mon, Apr 01, 2024 at 09:56:29AM +0800, jianbin zhang wrote:

Sorry, I think the patch looks good now, but the subject prefix
(rtc-pm8xxx) does not match other changes to this file.

> 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.
>
> Issue reproduce step:
> (1) set the alarm and poweroff the device
> (2) alarm happens and the device boots
> (3) poweroff the device again
> (4) alarm irq not be cleard, device boots again
>
> the fixing here is clear the interrupt during the step(3) unconditionally.
>
> Signed-off-by: jianbin zhang <[email protected]>
> ---
> Changes in v4:
> - add the cover letter
> - modify the patch to conform to the specification
>
> Changes in v3:
> - clear the interrupt in driver probe unconditionally
> - link: https://lore.kernel.org/linux-rtc/[email protected]/T/#t

These are expected to be links to the previous revisions of your patch,
not people's answers.

Please consult go/upstream and switch to b4 for the future, which does
this automatically for you. Please also use the internal review list!

Regards,
Bjorn

>
> Changes in v2:
> - Adapt the V1 patch according to the newest rtc-pm8xxx
> - link: https://lore.kernel.org/linux-rtc/[email protected]/
>
> Changes in v1:
> - fixing is as below logic, 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: https://lore.kernel.org/linux-rtc/[email protected]/
>
> Changes in original:
> - link to original: https://lore.kernel.org/linux-rtc/[email protected]/
> ---
> drivers/rtc/rtc-pm8xxx.c | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index f6b779c12ca7..e4e2307445cf 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -527,6 +527,11 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
> if (rc)
> return rc;
>
> + rc = regmap_update_bits(rtc_dd->regmap, rtc_dd->regs->alarm_ctrl2,
> + PM8xxx_RTC_ALARM_CLEAR, 1);
> + if (rc)
> + return rc;
> +
> return 0;
> }
>
>
> --
> 2.43.2
>

2024-04-08 09:02:44

by jianbin zhang

[permalink] [raw]
Subject: Re: [PATCH V4 1/2] rtc-pm8xxx: clear the triggered alarm interrupt during driver probe


On 4/5/24 10:45, Bjorn Andersson wrote:
> On Mon, Apr 01, 2024 at 09:56:29AM +0800, jianbin zhang wrote:
>
> Sorry, I think the patch looks good now, but the subject prefix
> (rtc-pm8xxx) does not match other changes to this file.
>
Sure, thanks for reviewing, I will update the subject prefix to "rtc:
pm8xxx:" as other changes to this file.
>> 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.
>>
>> Issue reproduce step:
>> (1) set the alarm and poweroff the device
>> (2) alarm happens and the device boots
>> (3) poweroff the device again
>> (4) alarm irq not be cleard, device boots again
>>
>> the fixing here is clear the interrupt during the step(3) unconditionally.
>>
>> Signed-off-by: jianbin zhang <[email protected]>
>> ---
>> Changes in v4:
>> - add the cover letter
>> - modify the patch to conform to the specification
>>
>> Changes in v3:
>> - clear the interrupt in driver probe unconditionally
>> - link: https://lore.kernel.org/linux-rtc/[email protected]/T/#t
> These are expected to be links to the previous revisions of your patch,
> not people's answers.
>
> Please consult go/upstream and switch to b4 for the future, which does
> this automatically for you. Please also use the internal review list!
>
> Regards,
> Bjorn
Yeah, will use the internal review list in next patch version and will
switch to b4 in the future.
>> Changes in v2:
>> - Adapt the V1 patch according to the newest rtc-pm8xxx
>> - link: https://lore.kernel.org/linux-rtc/[email protected]/
>>
>> Changes in v1:
>> - fixing is as below logic, 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: https://lore.kernel.org/linux-rtc/[email protected]/
>>
>> Changes in original:
>> - link to original: https://lore.kernel.org/linux-rtc/[email protected]/
>> ---
>> drivers/rtc/rtc-pm8xxx.c | 5 +++++
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
>> index f6b779c12ca7..e4e2307445cf 100644
>> --- a/drivers/rtc/rtc-pm8xxx.c
>> +++ b/drivers/rtc/rtc-pm8xxx.c
>> @@ -527,6 +527,11 @@ static int pm8xxx_rtc_probe(struct platform_device *pdev)
>> if (rc)
>> return rc;
>>
>> + rc = regmap_update_bits(rtc_dd->regmap, rtc_dd->regs->alarm_ctrl2,
>> + PM8xxx_RTC_ALARM_CLEAR, 1);
>> + if (rc)
>> + return rc;
>> +
>> return 0;
>> }
>>
>>
>> --
>> 2.43.2
>>