| Message ID | 1576155618-7933-1-git-send-email-srinivas.neeli@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rtc: zynqmp: Clear alarm interrupt status before interrupt enable | expand |
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
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.
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
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);
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(-)