Linux-RTC Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable
@ 2019-12-12 13:00 Srinivas Neeli
  2020-02-10 11:48 ` Michal Simek
  0 siblings, 1 reply; 4+ messages in thread
From: Srinivas Neeli @ 2019-12-12 13:00 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, michal.simek, sgoud, shubhraj
  Cc: linux-rtc, linux-arm-kernel, linux-kernel, git, Srinivas Neeli

Fix multiple occurring interrupts for alarm interrupt. RTC module doesn't
clear the alarm interrupt status bit immediately after the interrupt is
triggered.This is due to the sticky nature of the alarm interrupt status
register. The alarm interrupt status register can be cleared only after
the second counter outruns the set alarm value. To fix multiple spurious
interrupts, disable alarm interrupt in the handler and clear the status
bit before enabling the alarm interrupt.

Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 29 ++++++++++++++++++++++++-----
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 5786866c09e9..d311e3ef1f21 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -38,6 +38,8 @@
 
 #define RTC_CALIB_DEF		0x198233
 #define RTC_CALIB_MASK		0x1FFFFF
+#define RTC_ALRM_MASK          BIT(1)
+#define RTC_MSEC               1000
 
 struct xlnx_rtc_dev {
 	struct rtc_device	*rtc;
@@ -124,11 +126,28 @@ static int xlnx_rtc_alarm_irq_enable(struct device *dev, u32 enabled)
 {
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 
-	if (enabled)
+	unsigned int status;
+	ulong timeout;
+
+	timeout = jiffies + msecs_to_jiffies(RTC_MSEC);
+
+	if (enabled) {
+		while (1) {
+			status = readl(xrtcdev->reg_base + RTC_INT_STS);
+			if (!((status & RTC_ALRM_MASK) == RTC_ALRM_MASK))
+				break;
+
+			if (time_after_eq(jiffies, timeout)) {
+				dev_err(dev, "Time out occur, while clearing alarm status bit\n");
+				return -ETIMEDOUT;
+			}
+			writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
+		}
+
 		writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_EN);
-	else
+	} else {
 		writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
-
+	}
 	return 0;
 }
 
@@ -183,8 +202,8 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
 	if (!(status & (RTC_INT_SEC | RTC_INT_ALRM)))
 		return IRQ_NONE;
 
-	/* Clear RTC_INT_ALRM interrupt only */
-	writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
+	/* Disable RTC_INT_ALRM interrupt only */
+	writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
 
 	if (status & RTC_INT_ALRM)
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
-- 
2.7.4


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

* Re: [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable
  2019-12-12 13:00 [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable Srinivas Neeli
@ 2020-02-10 11:48 ` Michal Simek
  2020-02-11 10:39   ` Alexandre Belloni
  0 siblings, 1 reply; 4+ messages in thread
From: Michal Simek @ 2020-02-10 11:48 UTC (permalink / raw)
  To: Srinivas Neeli
  Cc: Alessandro Zummo, Alexandre Belloni, Srinivas Goud,
	Shubhrajyoti Datta, linux-rtc, LKML, linux-arm, git

Hi,

čt 12. 12. 2019 v 14:01 odesílatel Srinivas Neeli
<srinivas.neeli@xilinx.com> napsal:
>
> Fix multiple occurring interrupts for alarm interrupt. RTC module doesn't
> clear the alarm interrupt status bit immediately after the interrupt is
> triggered.This is due to the sticky nature of the alarm interrupt status
> register. The alarm interrupt status register can be cleared only after
> the second counter outruns the set alarm value. To fix multiple spurious
> interrupts, disable alarm interrupt in the handler and clear the status
> bit before enabling the alarm interrupt.
>
> Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 29 ++++++++++++++++++++++++-----
>  1 file changed, 24 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> index 5786866c09e9..d311e3ef1f21 100644
> --- a/drivers/rtc/rtc-zynqmp.c
> +++ b/drivers/rtc/rtc-zynqmp.c
> @@ -38,6 +38,8 @@
>
>  #define RTC_CALIB_DEF          0x198233
>  #define RTC_CALIB_MASK         0x1FFFFF
> +#define RTC_ALRM_MASK          BIT(1)
> +#define RTC_MSEC               1000
>
>  struct xlnx_rtc_dev {
>         struct rtc_device       *rtc;
> @@ -124,11 +126,28 @@ static int xlnx_rtc_alarm_irq_enable(struct device *dev, u32 enabled)
>  {
>         struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>

here shouldn't be empty line.

> -       if (enabled)
> +       unsigned int status;
> +       ulong timeout;
> +
> +       timeout = jiffies + msecs_to_jiffies(RTC_MSEC);
> +
> +       if (enabled) {
> +               while (1) {
> +                       status = readl(xrtcdev->reg_base + RTC_INT_STS);
> +                       if (!((status & RTC_ALRM_MASK) == RTC_ALRM_MASK))
> +                               break;
> +
> +                       if (time_after_eq(jiffies, timeout)) {
> +                               dev_err(dev, "Time out occur, while clearing alarm status bit\n");
> +                               return -ETIMEDOUT;
> +                       }
> +                       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
> +               }
> +
>                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_EN);
> -       else
> +       } else {
>                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
> -
> +       }

And here it was good to have empty line.

>         return 0;
>  }
>
> @@ -183,8 +202,8 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
>         if (!(status & (RTC_INT_SEC | RTC_INT_ALRM)))
>                 return IRQ_NONE;
>
> -       /* Clear RTC_INT_ALRM interrupt only */
> -       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
> +       /* Disable RTC_INT_ALRM interrupt only */
> +       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
>
>         if (status & RTC_INT_ALRM)
>                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> --
> 2.7.4

Other then these two above things look good.

Alexandre: Any issue with this patch?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

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

* Re: [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable
  2020-02-10 11:48 ` Michal Simek
@ 2020-02-11 10:39   ` Alexandre Belloni
  2020-02-11 10:40     ` Michal Simek
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Belloni @ 2020-02-11 10:39 UTC (permalink / raw)
  To: Michal Simek
  Cc: Srinivas Neeli, Alessandro Zummo, Srinivas Goud,
	Shubhrajyoti Datta, linux-rtc, LKML, linux-arm, git

On 10/02/2020 12:48:14+0100, Michal Simek wrote:
> Hi,
> 
> čt 12. 12. 2019 v 14:01 odesílatel Srinivas Neeli
> <srinivas.neeli@xilinx.com> napsal:
> >
> > Fix multiple occurring interrupts for alarm interrupt. RTC module doesn't
> > clear the alarm interrupt status bit immediately after the interrupt is
> > triggered.This is due to the sticky nature of the alarm interrupt status
> > register. The alarm interrupt status register can be cleared only after
> > the second counter outruns the set alarm value. To fix multiple spurious
> > interrupts, disable alarm interrupt in the handler and clear the status
> > bit before enabling the alarm interrupt.
> >
> > Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
> > Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
> > ---
> >  drivers/rtc/rtc-zynqmp.c | 29 ++++++++++++++++++++++++-----
> >  1 file changed, 24 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
> > index 5786866c09e9..d311e3ef1f21 100644
> > --- a/drivers/rtc/rtc-zynqmp.c
> > +++ b/drivers/rtc/rtc-zynqmp.c
> > @@ -38,6 +38,8 @@
> >
> >  #define RTC_CALIB_DEF          0x198233
> >  #define RTC_CALIB_MASK         0x1FFFFF
> > +#define RTC_ALRM_MASK          BIT(1)
> > +#define RTC_MSEC               1000
> >
> >  struct xlnx_rtc_dev {
> >         struct rtc_device       *rtc;
> > @@ -124,11 +126,28 @@ static int xlnx_rtc_alarm_irq_enable(struct device *dev, u32 enabled)
> >  {
> >         struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >
> 
> here shouldn't be empty line.
> 
> > -       if (enabled)
> > +       unsigned int status;
> > +       ulong timeout;
> > +
> > +       timeout = jiffies + msecs_to_jiffies(RTC_MSEC);
> > +
> > +       if (enabled) {
> > +               while (1) {
> > +                       status = readl(xrtcdev->reg_base + RTC_INT_STS);
> > +                       if (!((status & RTC_ALRM_MASK) == RTC_ALRM_MASK))
> > +                               break;
> > +
> > +                       if (time_after_eq(jiffies, timeout)) {
> > +                               dev_err(dev, "Time out occur, while clearing alarm status bit\n");
> > +                               return -ETIMEDOUT;
> > +                       }
> > +                       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
> > +               }
> > +
> >                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_EN);
> > -       else
> > +       } else {
> >                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
> > -
> > +       }
> 
> And here it was good to have empty line.
> 
> >         return 0;
> >  }
> >
> > @@ -183,8 +202,8 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
> >         if (!(status & (RTC_INT_SEC | RTC_INT_ALRM)))
> >                 return IRQ_NONE;
> >
> > -       /* Clear RTC_INT_ALRM interrupt only */
> > -       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
> > +       /* Disable RTC_INT_ALRM interrupt only */
> > +       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
> >
> >         if (status & RTC_INT_ALRM)
> >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > --
> > 2.7.4
> 
> Other then these two above things look good.
> 
> Alexandre: Any issue with this patch?
> 

No issue, I was kind of waiting for your review. I'll take the patch
once your comments are addressed.

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

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

* Re: [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable
  2020-02-11 10:39   ` Alexandre Belloni
@ 2020-02-11 10:40     ` Michal Simek
  0 siblings, 0 replies; 4+ messages in thread
From: Michal Simek @ 2020-02-11 10:40 UTC (permalink / raw)
  To: Alexandre Belloni, Michal Simek
  Cc: Srinivas Neeli, Alessandro Zummo, Srinivas Goud,
	Shubhrajyoti Datta, linux-rtc, LKML, linux-arm, git

On 11. 02. 20 11:39, Alexandre Belloni wrote:
> On 10/02/2020 12:48:14+0100, Michal Simek wrote:
>> Hi,
>>
>> čt 12. 12. 2019 v 14:01 odesílatel Srinivas Neeli
>> <srinivas.neeli@xilinx.com> napsal:
>>>
>>> Fix multiple occurring interrupts for alarm interrupt. RTC module doesn't
>>> clear the alarm interrupt status bit immediately after the interrupt is
>>> triggered.This is due to the sticky nature of the alarm interrupt status
>>> register. The alarm interrupt status register can be cleared only after
>>> the second counter outruns the set alarm value. To fix multiple spurious
>>> interrupts, disable alarm interrupt in the handler and clear the status
>>> bit before enabling the alarm interrupt.
>>>
>>> Fixes: 11143c19eb57 ("rtc: add xilinx zynqmp rtc driver")
>>> Signed-off-by: Srinivas Neeli <srinivas.neeli@xilinx.com>
>>> ---
>>>  drivers/rtc/rtc-zynqmp.c | 29 ++++++++++++++++++++++++-----
>>>  1 file changed, 24 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
>>> index 5786866c09e9..d311e3ef1f21 100644
>>> --- a/drivers/rtc/rtc-zynqmp.c
>>> +++ b/drivers/rtc/rtc-zynqmp.c
>>> @@ -38,6 +38,8 @@
>>>
>>>  #define RTC_CALIB_DEF          0x198233
>>>  #define RTC_CALIB_MASK         0x1FFFFF
>>> +#define RTC_ALRM_MASK          BIT(1)
>>> +#define RTC_MSEC               1000
>>>
>>>  struct xlnx_rtc_dev {
>>>         struct rtc_device       *rtc;
>>> @@ -124,11 +126,28 @@ static int xlnx_rtc_alarm_irq_enable(struct device *dev, u32 enabled)
>>>  {
>>>         struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>>>
>>
>> here shouldn't be empty line.
>>
>>> -       if (enabled)
>>> +       unsigned int status;
>>> +       ulong timeout;
>>> +
>>> +       timeout = jiffies + msecs_to_jiffies(RTC_MSEC);
>>> +
>>> +       if (enabled) {
>>> +               while (1) {
>>> +                       status = readl(xrtcdev->reg_base + RTC_INT_STS);
>>> +                       if (!((status & RTC_ALRM_MASK) == RTC_ALRM_MASK))
>>> +                               break;
>>> +
>>> +                       if (time_after_eq(jiffies, timeout)) {
>>> +                               dev_err(dev, "Time out occur, while clearing alarm status bit\n");
>>> +                               return -ETIMEDOUT;
>>> +                       }
>>> +                       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
>>> +               }
>>> +
>>>                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_EN);
>>> -       else
>>> +       } else {
>>>                 writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
>>> -
>>> +       }
>>
>> And here it was good to have empty line.
>>
>>>         return 0;
>>>  }
>>>
>>> @@ -183,8 +202,8 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
>>>         if (!(status & (RTC_INT_SEC | RTC_INT_ALRM)))
>>>                 return IRQ_NONE;
>>>
>>> -       /* Clear RTC_INT_ALRM interrupt only */
>>> -       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
>>> +       /* Disable RTC_INT_ALRM interrupt only */
>>> +       writel(RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_DIS);
>>>
>>>         if (status & RTC_INT_ALRM)
>>>                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
>>> --
>>> 2.7.4
>>
>> Other then these two above things look good.
>>
>> Alexandre: Any issue with this patch?
>>
> 
> No issue, I was kind of waiting for your review. I'll take the patch
> once your comments are addressed.

Ok. Thanks,
Michal



^ 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-12-12 13:00 [PATCH] rtc: zynqmp: Clear alarm interrupt status before interrupt enable Srinivas Neeli
2020-02-10 11:48 ` Michal Simek
2020-02-11 10:39   ` Alexandre Belloni
2020-02-11 10:40     ` Michal Simek

Linux-RTC Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-rtc/0 linux-rtc/git/0.git

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

Example config snippet for mirrors

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


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