LKML Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle
@ 2019-01-11  7:09 Anson Huang
  2019-01-11 14:31 ` Alexandre Belloni
  2019-02-05 22:10 ` Alexandre Belloni
  0 siblings, 2 replies; 4+ messages in thread
From: Anson Huang @ 2019-01-11  7:09 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, linux-rtc, linux-kernel; +Cc: dl-linux-imx

During system suspend, the SNVS RTC's clock will be disabled in
noirq suspend phase, but SNVS RTC's alarm interrupt could still
arrive, system will hang if SNVS RTC driver tries to access register
without clock enabled, this patch fixes the issue of this scenario.

Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
---
 drivers/rtc/rtc-snvs.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/rtc/rtc-snvs.c b/drivers/rtc/rtc-snvs.c
index b2483a7..0b9eff1 100644
--- a/drivers/rtc/rtc-snvs.c
+++ b/drivers/rtc/rtc-snvs.c
@@ -239,6 +239,9 @@ static irqreturn_t snvs_rtc_irq_handler(int irq, void *dev_id)
 	u32 lpsr;
 	u32 events = 0;
 
+	if (data->clk)
+		clk_enable(data->clk);
+
 	regmap_read(data->regmap, data->offset + SNVS_LPSR, &lpsr);
 
 	if (lpsr & SNVS_LPSR_LPTA) {
@@ -253,6 +256,9 @@ static irqreturn_t snvs_rtc_irq_handler(int irq, void *dev_id)
 	/* clear interrupt status */
 	regmap_write(data->regmap, data->offset + SNVS_LPSR, lpsr);
 
+	if (data->clk)
+		clk_disable(data->clk);
+
 	return events ? IRQ_HANDLED : IRQ_NONE;
 }
 
-- 
2.7.4


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

* Re: [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle
  2019-01-11  7:09 [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle Anson Huang
@ 2019-01-11 14:31 ` Alexandre Belloni
  2019-01-14  2:10   ` Anson Huang
  2019-02-05 22:10 ` Alexandre Belloni
  1 sibling, 1 reply; 4+ messages in thread
From: Alexandre Belloni @ 2019-01-11 14:31 UTC (permalink / raw)
  To: Anson Huang; +Cc: a.zummo, linux-rtc, linux-kernel, dl-linux-imx

Hi,

On 11/01/2019 07:09:02+0000, Anson Huang wrote:
> During system suspend, the SNVS RTC's clock will be disabled in
> noirq suspend phase, but SNVS RTC's alarm interrupt could still
> arrive, system will hang if SNVS RTC driver tries to access register
> without clock enabled, this patch fixes the issue of this scenario.
> 

Are you sure this is the real issue? I don't think the handler can be
called before the resume_noirq callback. Isn't the issue that your clock
driver has not yet resumed by the time you call the rtc driver
resume_noirq callback ?

> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/rtc/rtc-snvs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/rtc/rtc-snvs.c b/drivers/rtc/rtc-snvs.c
> index b2483a7..0b9eff1 100644
> --- a/drivers/rtc/rtc-snvs.c
> +++ b/drivers/rtc/rtc-snvs.c
> @@ -239,6 +239,9 @@ static irqreturn_t snvs_rtc_irq_handler(int irq, void *dev_id)
>  	u32 lpsr;
>  	u32 events = 0;
>  
> +	if (data->clk)
> +		clk_enable(data->clk);
> +

Anyway, won't that need a clk_prepare_enable because it has been
clk_disable_unprepare in suspend_noirq? And this is something you can
not do in atomic context

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

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

* RE: [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle
  2019-01-11 14:31 ` Alexandre Belloni
@ 2019-01-14  2:10   ` Anson Huang
  0 siblings, 0 replies; 4+ messages in thread
From: Anson Huang @ 2019-01-14  2:10 UTC (permalink / raw)
  To: Alexandre Belloni; +Cc: a.zummo, linux-rtc, linux-kernel, dl-linux-imx

Hi, Alexandre

Best Regards!
Anson Huang

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@bootlin.com]
> Sent: 2019Äê1ÔÂ11ÈÕ 22:32
> To: Anson Huang <anson.huang@nxp.com>
> Cc: a.zummo@towertech.it; linux-rtc@vger.kernel.org;
> linux-kernel@vger.kernel.org; dl-linux-imx <linux-imx@nxp.com>
> Subject: Re: [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle
> 
> Hi,
> 
> On 11/01/2019 07:09:02+0000, Anson Huang wrote:
> > During system suspend, the SNVS RTC's clock will be disabled in noirq
> > suspend phase, but SNVS RTC's alarm interrupt could still arrive,
> > system will hang if SNVS RTC driver tries to access register without
> > clock enabled, this patch fixes the issue of this scenario.
> >
> 
> Are you sure this is the real issue? I don't think the handler can be called
> before the resume_noirq callback. Isn't the issue that your clock driver has not
> yet resumed by the time you call the rtc driver resume_noirq callback ?

Yes, since the RTC alarm is wakeup source, it is NOT being disabled during suspend
flow at all, after rtc drvier's noirq suspend callback is being called, rtc clock is disabled
in this callback, rtc alarm could arrive at this point then ARM IRQ is NOT disabled yet,
so rtc irq handler will be called, so the handler will access RTC register with RTC clock disabled
and cause system hang. The point is RTC clock is disabled in noirq suspend callback.


> 
> > Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> > ---
> >  drivers/rtc/rtc-snvs.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/rtc/rtc-snvs.c b/drivers/rtc/rtc-snvs.c index
> > b2483a7..0b9eff1 100644
> > --- a/drivers/rtc/rtc-snvs.c
> > +++ b/drivers/rtc/rtc-snvs.c
> > @@ -239,6 +239,9 @@ static irqreturn_t snvs_rtc_irq_handler(int irq, void
> *dev_id)
> >  	u32 lpsr;
> >  	u32 events = 0;
> >
> > +	if (data->clk)
> > +		clk_enable(data->clk);
> > +
> 
> Anyway, won't that need a clk_prepare_enable because it has been
> clk_disable_unprepare in suspend_noirq? And this is something you can not
> do in atomic context

Yes, but the prepare/unprepared can NOT be called in atomic context, since the SNVS RTC
clock on i.MX SoC is always from a clock source without prepare/unprepared needed, such as
from ipg or OSC etc., so just doing enable/disable is good enough for it.

Anson.

> 
> --
> Alexandre Belloni, Bootlin
> Embedded Linux and Kernel engineering
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbootl
> in.com&amp;data=02%7C01%7Canson.huang%40nxp.com%7Ca90608144e90
> 494a6f3608d677d1818a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C
> 0%7C636828139042107931&amp;sdata=oX%2Bqnj4X3nS%2Ba88otedt9WyIy
> VhBNVl5eixVWeV44TA%3D&amp;reserved=0

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

* Re: [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle
  2019-01-11  7:09 [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle Anson Huang
  2019-01-11 14:31 ` Alexandre Belloni
@ 2019-02-05 22:10 ` Alexandre Belloni
  1 sibling, 0 replies; 4+ messages in thread
From: Alexandre Belloni @ 2019-02-05 22:10 UTC (permalink / raw)
  To: Anson Huang; +Cc: a.zummo, linux-rtc, linux-kernel, dl-linux-imx

On 11/01/2019 07:09:02+0000, Anson Huang wrote:
> During system suspend, the SNVS RTC's clock will be disabled in
> noirq suspend phase, but SNVS RTC's alarm interrupt could still
> arrive, system will hang if SNVS RTC driver tries to access register
> without clock enabled, this patch fixes the issue of this scenario.
> 
> Signed-off-by: Anson Huang <Anson.Huang@nxp.com>
> ---
>  drivers/rtc/rtc-snvs.c | 6 ++++++
>  1 file changed, 6 insertions(+)
Applied, thanks.

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

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

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-11  7:09 [PATCH] rtc: snvs: make sure clock is enabled for interrupt handle Anson Huang
2019-01-11 14:31 ` Alexandre Belloni
2019-01-14  2:10   ` Anson Huang
2019-02-05 22:10 ` Alexandre Belloni

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lore.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lore.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lore.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lore.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lore.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lore.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lore.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lore.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox