* [PATCH] rtc-pm8xxx: Clear Alarm register on resume
@ 2020-03-18 9:53 Mohit Aggarwal
2020-03-22 21:31 ` Alexandre Belloni
0 siblings, 1 reply; 9+ messages in thread
From: Mohit Aggarwal @ 2020-03-18 9:53 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: Mohit Aggarwal, linux-rtc, linux-kernel
Currently, alarm register is not cleared on resume
leading to reboot during power off charging mode.
Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index bbe013f..bfcd878 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All rights reserved. */
#include <linux/of.h>
#include <linux/module.h>
@@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
unsigned int ctrl_reg;
+ u8 value[NUM_8_BIT_RTC_REGS] = {0};
spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
@@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
goto rtc_rw_fail;
}
+ /* Clear Alarm register */
+ if (!enable) {
+ rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
+ sizeof(value));
+ if (rc) {
+ dev_err(dev, "Write to RTC ALARM register failed\n");
+ goto rtc_rw_fail;
+ }
+ }
+
rtc_rw_fail:
spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
return rc;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
2020-03-18 9:53 [PATCH] rtc-pm8xxx: Clear Alarm register on resume Mohit Aggarwal
@ 2020-03-22 21:31 ` Alexandre Belloni
0 siblings, 0 replies; 9+ messages in thread
From: Alexandre Belloni @ 2020-03-22 21:31 UTC (permalink / raw)
To: Mohit Aggarwal; +Cc: a.zummo, linux-rtc, linux-kernel
Hi,
On 18/03/2020 15:23:38+0530, Mohit Aggarwal wrote:
> Currently, alarm register is not cleared on resume
> leading to reboot during power off charging mode.
>
> Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
> ---
> drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index bbe013f..bfcd878 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights reserved. */
> +/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All rights reserved. */
>
This part doesn't apply and is not based on upstream.
> #include <linux/of.h>
> #include <linux/module.h>
> @@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> unsigned int ctrl_reg;
> + u8 value[NUM_8_BIT_RTC_REGS] = {0};
>
> spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
>
> @@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> goto rtc_rw_fail;
> }
>
> + /* Clear Alarm register */
> + if (!enable) {
> + rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
> + sizeof(value));
> + if (rc) {
> + dev_err(dev, "Write to RTC ALARM register failed\n");
> + goto rtc_rw_fail;
> + }
> + }
> +
> rtc_rw_fail:
> spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
> return rc;
> --
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH] rtc-pm8xxx: Clear Alarm register on resume
@ 2020-03-23 7:16 Mohit Aggarwal
0 siblings, 0 replies; 9+ messages in thread
From: Mohit Aggarwal @ 2020-03-23 7:16 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: Mohit Aggarwal, linux-rtc, linux-kernel
Currently, alarm register is not cleared on resume
leading to reboot during power off charging mode.
Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index bbe013f..f0d89a4 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
unsigned int ctrl_reg;
+ u8 value[NUM_8_BIT_RTC_REGS] = {0};
spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
@@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
goto rtc_rw_fail;
}
+ /* Clear Alarm register */
+ if (!enable) {
+ rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
+ sizeof(value));
+ if (rc) {
+ dev_err(dev, "Write to RTC ALARM register failed\n");
+ goto rtc_rw_fail;
+ }
+ }
+
rtc_rw_fail:
spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
return rc;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] rtc-pm8xxx: Clear Alarm register on resume
@ 2020-03-17 8:04 Mohit Aggarwal
0 siblings, 0 replies; 9+ messages in thread
From: Mohit Aggarwal @ 2020-03-17 8:04 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: Mohit Aggarwal, linux-rtc, linux-kernel
Currently, alarm register is not cleared on resume
leading to reboot during power off charging mode.
Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index bbe013f..96e7985 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All rights reserved. */
#include <linux/of.h>
#include <linux/module.h>
@@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
unsigned int ctrl_reg;
+ u8 value[NUM_8_BIT_RTC_REGS] = {0};
spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
@@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
goto rtc_rw_fail;
}
+ /* Clear Alarm register */
+ if (!enable) {
+ rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
+ sizeof(value));
+ if (rc) {
+ dev_err(dev, "Write to RTC ALARM register failed\n");
+ goto rtc_rw_fail;
+ }
+ }
+
rtc_rw_fail:
spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
return rc;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH] rtc-pm8xxx: Clear Alarm register on resume
@ 2020-03-16 7:11 Mohit Aggarwal
2020-03-16 10:29 ` Alexandre Belloni
0 siblings, 1 reply; 9+ messages in thread
From: Mohit Aggarwal @ 2020-03-16 7:11 UTC (permalink / raw)
To: a.zummo, alexandre.belloni; +Cc: Mohit Aggarwal, linux-rtc, linux-kernel
Currently, alarm register is not cleared on resume
leading to reboot during power off charging mode.
Change-Id: Ie2e6bbab8aa46e4e9b9cc984181ffab557cbbdae
Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
---
drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
index bbe013f..96e7985 100644
--- a/drivers/rtc/rtc-pm8xxx.c
+++ b/drivers/rtc/rtc-pm8xxx.c
@@ -1,5 +1,5 @@
// SPDX-License-Identifier: GPL-2.0-only
-/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights reserved. */
+/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All rights reserved. */
#include <linux/of.h>
#include <linux/module.h>
@@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
unsigned int ctrl_reg;
+ u8 value[NUM_8_BIT_RTC_REGS] = {0};
spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
@@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
goto rtc_rw_fail;
}
+ /* Clear Alarm register */
+ if (!enable) {
+ rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
+ sizeof(value));
+ if (rc) {
+ dev_err(dev, "Write to RTC ALARM register failed\n");
+ goto rtc_rw_fail;
+ }
+ }
+
rtc_rw_fail:
spin_unlock_irqrestore(&rtc_dd->ctrl_reg_lock, irq_flags);
return rc;
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
2020-03-16 7:11 Mohit Aggarwal
@ 2020-03-16 10:29 ` Alexandre Belloni
2020-03-17 4:50 ` maggarwa
0 siblings, 1 reply; 9+ messages in thread
From: Alexandre Belloni @ 2020-03-16 10:29 UTC (permalink / raw)
To: Mohit Aggarwal; +Cc: a.zummo, linux-rtc, linux-kernel
Hi,
On 16/03/2020 12:41:28+0530, Mohit Aggarwal wrote:
> Currently, alarm register is not cleared on resume
> leading to reboot during power off charging mode.
>
> Change-Id: Ie2e6bbab8aa46e4e9b9cc984181ffab557cbbdae
No Change-Id upstream please.
> Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
> ---
> drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> index bbe013f..96e7985 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights reserved. */
> +/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All rights reserved. */
>
> #include <linux/of.h>
> #include <linux/module.h>
> @@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> unsigned int ctrl_reg;
> + u8 value[NUM_8_BIT_RTC_REGS] = {0};
>
> spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
>
> @@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device *dev, unsigned int enable)
> goto rtc_rw_fail;
> }
>
> + /* Clear Alarm register */
> + if (!enable) {
> + rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw, value,
> + sizeof(value));
This is not properly aligned.
> + if (rc) {
> + dev_err(dev, "Write to RTC ALARM register failed\n");
Is that error message necessary? What would be the user action after
seeing that in the logs? Will the logs actually be seen?
> + goto rtc_rw_fail;
> + }
> + }
> +
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
2020-03-16 10:29 ` Alexandre Belloni
@ 2020-03-17 4:50 ` maggarwa
2020-03-17 11:55 ` Alexandre Belloni
0 siblings, 1 reply; 9+ messages in thread
From: maggarwa @ 2020-03-17 4:50 UTC (permalink / raw)
To: 'Alexandre Belloni'; +Cc: a.zummo, linux-rtc, linux-kernel
Hi,
Comments inline.
Thanks & Regards,
Mohit
-----Original Message-----
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Sent: Monday, March 16, 2020 3:59 PM
To: Mohit Aggarwal <maggarwa@codeaurora.org>
Cc: a.zummo@towertech.it; linux-rtc@vger.kernel.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
Hi,
On 16/03/2020 12:41:28+0530, Mohit Aggarwal wrote:
> Currently, alarm register is not cleared on resume leading to reboot
> during power off charging mode.
>
> Change-Id: Ie2e6bbab8aa46e4e9b9cc984181ffab557cbbdae
No Change-Id upstream please.
[Mohit]: Will fix in next patch.
> Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
> ---
> drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c index
> bbe013f..96e7985 100644
> --- a/drivers/rtc/rtc-pm8xxx.c
> +++ b/drivers/rtc/rtc-pm8xxx.c
> @@ -1,5 +1,5 @@
> // SPDX-License-Identifier: GPL-2.0-only
> -/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights
> reserved. */
> +/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All
> +rights reserved. */
>
> #include <linux/of.h>
> #include <linux/module.h>
> @@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device
*dev, unsigned int enable)
> struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> unsigned int ctrl_reg;
> + u8 value[NUM_8_BIT_RTC_REGS] = {0};
>
> spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
>
> @@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device
*dev, unsigned int enable)
> goto rtc_rw_fail;
> }
>
> + /* Clear Alarm register */
> + if (!enable) {
> + rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw,
value,
> + sizeof(value));
This is not properly aligned.
[Mohit]: I don't see any alignment issue at my end. I can see proper tabs
are present.
> + if (rc) {
> + dev_err(dev, "Write to RTC ALARM register
failed\n");
Is that error message necessary? What would be the user action after seeing
that in the logs? Will the logs actually be seen?
[Mohit]: In case issue in question reproduces even after this change then
for debugging purposes user can look out for this error log in kernel logs
which can help to triage the issue.
> + goto rtc_rw_fail;
> + }
> + }
> +
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
2020-03-17 4:50 ` maggarwa
@ 2020-03-17 11:55 ` Alexandre Belloni
2020-03-18 9:56 ` maggarwa
0 siblings, 1 reply; 9+ messages in thread
From: Alexandre Belloni @ 2020-03-17 11:55 UTC (permalink / raw)
To: maggarwa; +Cc: a.zummo, linux-rtc, linux-kernel
On 17/03/2020 10:20:36+0530, maggarwa@codeaurora.org wrote:
> Hi,
>
> Comments inline.
>
>
> Thanks & Regards,
> Mohit
>
> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Monday, March 16, 2020 3:59 PM
> To: Mohit Aggarwal <maggarwa@codeaurora.org>
> Cc: a.zummo@towertech.it; linux-rtc@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
>
> Hi,
>
> On 16/03/2020 12:41:28+0530, Mohit Aggarwal wrote:
> > Currently, alarm register is not cleared on resume leading to reboot
> > during power off charging mode.
> >
> > Change-Id: Ie2e6bbab8aa46e4e9b9cc984181ffab557cbbdae
>
> No Change-Id upstream please.
> [Mohit]: Will fix in next patch.
>
> > Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
> > ---
> > drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c index
> > bbe013f..96e7985 100644
> > --- a/drivers/rtc/rtc-pm8xxx.c
> > +++ b/drivers/rtc/rtc-pm8xxx.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> > -/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights
> > reserved. */
> > +/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All
> > +rights reserved. */
> >
> > #include <linux/of.h>
> > #include <linux/module.h>
> > @@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device
> *dev, unsigned int enable)
> > struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> > const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> > unsigned int ctrl_reg;
> > + u8 value[NUM_8_BIT_RTC_REGS] = {0};
> >
> > spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
> >
> > @@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct device
> *dev, unsigned int enable)
> > goto rtc_rw_fail;
> > }
> >
> > + /* Clear Alarm register */
> > + if (!enable) {
> > + rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw,
> value,
> > + sizeof(value));
>
> This is not properly aligned.
> [Mohit]: I don't see any alignment issue at my end. I can see proper tabs
> are present.
>
The alignment should match the opening parenthesis.
> > + if (rc) {
> > + dev_err(dev, "Write to RTC ALARM register
> failed\n");
>
> Is that error message necessary? What would be the user action after seeing
> that in the logs? Will the logs actually be seen?
> [Mohit]: In case issue in question reproduces even after this change then
> for debugging purposes user can look out for this error log in kernel logs
> which can help to triage the issue.
>
Who in the field on the final product will see this error message? For
debugging purposes, regmap already provides plenty of tracing
facilities.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
2020-03-17 11:55 ` Alexandre Belloni
@ 2020-03-18 9:56 ` maggarwa
0 siblings, 0 replies; 9+ messages in thread
From: maggarwa @ 2020-03-18 9:56 UTC (permalink / raw)
To: 'Alexandre Belloni'; +Cc: a.zummo, linux-rtc, linux-kernel
Comments inline.
-----Original Message-----
From: Alexandre Belloni <alexandre.belloni@bootlin.com>
Sent: Tuesday, March 17, 2020 5:25 PM
To: maggarwa@codeaurora.org
Cc: a.zummo@towertech.it; linux-rtc@vger.kernel.org;
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
On 17/03/2020 10:20:36+0530, maggarwa@codeaurora.org wrote:
> Hi,
>
> Comments inline.
>
>
> Thanks & Regards,
> Mohit
>
> -----Original Message-----
> From: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Sent: Monday, March 16, 2020 3:59 PM
> To: Mohit Aggarwal <maggarwa@codeaurora.org>
> Cc: a.zummo@towertech.it; linux-rtc@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] rtc-pm8xxx: Clear Alarm register on resume
>
> Hi,
>
> On 16/03/2020 12:41:28+0530, Mohit Aggarwal wrote:
> > Currently, alarm register is not cleared on resume leading to reboot
> > during power off charging mode.
> >
> > Change-Id: Ie2e6bbab8aa46e4e9b9cc984181ffab557cbbdae
>
> No Change-Id upstream please.
> [Mohit]: Will fix in next patch.
>
> > Signed-off-by: Mohit Aggarwal <maggarwa@codeaurora.org>
> > ---
> > drivers/rtc/rtc-pm8xxx.c | 13 ++++++++++++-
> > 1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/rtc/rtc-pm8xxx.c b/drivers/rtc/rtc-pm8xxx.c
> > index
> > bbe013f..96e7985 100644
> > --- a/drivers/rtc/rtc-pm8xxx.c
> > +++ b/drivers/rtc/rtc-pm8xxx.c
> > @@ -1,5 +1,5 @@
> > // SPDX-License-Identifier: GPL-2.0-only
> > -/* Copyright (c) 2010-2011, 2019, The Linux Foundation. All rights
> > reserved. */
> > +/* Copyright (c) 2010-2011, 2019-2020, The Linux Foundation. All
> > +rights reserved. */
> >
> > #include <linux/of.h>
> > #include <linux/module.h>
> > @@ -301,6 +301,7 @@ static int pm8xxx_rtc_alarm_irq_enable(struct
> > device
> *dev, unsigned int enable)
> > struct pm8xxx_rtc *rtc_dd = dev_get_drvdata(dev);
> > const struct pm8xxx_rtc_regs *regs = rtc_dd->regs;
> > unsigned int ctrl_reg;
> > + u8 value[NUM_8_BIT_RTC_REGS] = {0};
> >
> > spin_lock_irqsave(&rtc_dd->ctrl_reg_lock, irq_flags);
> >
> > @@ -319,6 +320,16 @@ static int pm8xxx_rtc_alarm_irq_enable(struct
> > device
> *dev, unsigned int enable)
> > goto rtc_rw_fail;
> > }
> >
> > + /* Clear Alarm register */
> > + if (!enable) {
> > + rc = regmap_bulk_write(rtc_dd->regmap, regs->alarm_rw,
> value,
> > + sizeof(value));
>
> This is not properly aligned.
> [Mohit]: I don't see any alignment issue at my end. I can see proper
> tabs are present.
>
The alignment should match the opening parenthesis.
[Mohit]: Thanks for clarification. Uploaded new patch.
> > + if (rc) {
> > + dev_err(dev, "Write to RTC ALARM register
> failed\n");
>
> Is that error message necessary? What would be the user action after
> seeing that in the logs? Will the logs actually be seen?
> [Mohit]: In case issue in question reproduces even after this change
> then for debugging purposes user can look out for this error log in
> kernel logs which can help to triage the issue.
>
Who in the field on the final product will see this error message? For
debugging purposes, regmap already provides plenty of tracing facilities.
[Mohit]: This is mostly for debugging issues reported during internal tests
using kernel logs. Also added the debug error log to remain in-line with
other error logs in file.
--
Alexandre Belloni, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-03-23 7:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-18 9:53 [PATCH] rtc-pm8xxx: Clear Alarm register on resume Mohit Aggarwal
2020-03-22 21:31 ` Alexandre Belloni
-- strict thread matches above, loose matches on Subject: below --
2020-03-23 7:16 Mohit Aggarwal
2020-03-17 8:04 Mohit Aggarwal
2020-03-16 7:11 Mohit Aggarwal
2020-03-16 10:29 ` Alexandre Belloni
2020-03-17 4:50 ` maggarwa
2020-03-17 11:55 ` Alexandre Belloni
2020-03-18 9:56 ` maggarwa
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).