When reaching the read_time implementation before 1 second has elapsed
since power on, SEC bit from status register is not yet set. read_time
incorrectly assumes that there is a pending set_time not yet reflected
in CURRENT_TIME and proceeds to use SET_TIME_READ (RTC_SET_TM_RD in the
code). This register contains garbage at this moment and this is
returned as the current time.
Here we switch to using the SEC interrupt to signal the pending set time
operation. The old ISR is renamed xlnx_rtc_alarm_interrupt and a new one
xlnx_rtc_sec_interrupt is created to handle disabling the SEC interrupt
once the pending write is known to be complete.
read_time now detects whether a pending write exists or not using the
interrupt mask (!interrupt enabled). The interrupt is only enabled when
actually performing a set_time.
Signed-off-by: Guillaume Champagne <champagne.guillaume.c@xxxxxxxxx>
Signed-off-by: Jean-Francois Dagenais <jeff.dagenais@xxxxxxxxx>
Tested-by: Maxime Roussin-Bélanger <maxime.roussinbelanger@xxxxxxxxx>
---
drivers/rtc/rtc-zynqmp.c | 69 +++++++++++++++++++++++++++++-----------
1 file changed, 50 insertions(+), 19 deletions(-)
diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index cb78900ec1f5..9fec89c4f573 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -13,6 +13,7 @@
#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/rtc.h>
+#include <linux/spinlock.h>
/* RTC Registers */
#define RTC_SET_TM_WR 0x00
@@ -45,6 +46,7 @@ struct xlnx_rtc_dev {
int alarm_irq;
int sec_irq;
int calibval;
+ spinlock_t lock;
};
static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -59,6 +61,8 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
*/
new_time = rtc_tm_to_time64(tm) + 1;
+ spin_lock_irq(&xrtcdev->lock);
+
/*
* Writing into calibration register will clear the Tick Counter and
* force the next second to be signaled exactly in 1 second period
@@ -68,32 +72,32 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
writel(new_time, xrtcdev->reg_base + RTC_SET_TM_WR);
- /*
- * Clear the rtc interrupt status register after setting the
- * time. During a read_time function, the code should read the
- * RTC_INT_STATUS register and if bit 0 is still 0, it means
- * that one second has not elapsed yet since RTC was set and
- * the current time should be read from SET_TIME_READ register;
- * otherwise, CURRENT_TIME register is read to report the time
- */
+ /* Clear sec interrupt status */
writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_STS);
+ /* Now enable sec interrupt. This ensures our isr will get called, but
+ * also signals the read_time function that a SET time is pending, in
+ * which case time should be read from SET_TIME_READ instead of
+ * CURRENT_TIME registers.
+ */
+ writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
+
+ spin_unlock_irq(&xrtcdev->lock);
+
return 0;
}
static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
{
- u32 status;
+ u32 int_mask;
unsigned long read_time;
struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
- status = readl(xrtcdev->reg_base + RTC_INT_STS);
+ spin_lock_irq(&xrtcdev->lock);
+ int_mask = readl(xrtcdev->reg_base + RTC_INT_MASK);
- if (status & RTC_INT_SEC) {
- /*
- * RTC has updated the CURRENT_TIME with the time written into
- * SET_TIME_WRITE register.
- */
+ if (int_mask & RTC_INT_SEC) {
+ /* No set_time pending, use the CURRENT_TIME register */
read_time = readl(xrtcdev->reg_base + RTC_CUR_TM);
} else {
/*
@@ -105,6 +109,9 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
*/
read_time = readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1;
}
+
+ spin_unlock_irq(&xrtcdev->lock);
+
rtc_time64_to_tm(read_time, tm);
return 0;
@@ -173,14 +180,36 @@ static const struct rtc_class_ops xlnx_rtc_ops = {
.alarm_irq_enable = xlnx_rtc_alarm_irq_enable,
};
-static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
+static irqreturn_t xlnx_rtc_sec_interrupt(int irq, void *id)
+{
+ struct xlnx_rtc_dev *xrtcdev = (struct xlnx_rtc_dev *)id;
+ unsigned int status;
+
+ status = readl(xrtcdev->reg_base + RTC_INT_STS);
+ /* Check if interrupt asserted */
+ if (!(status & RTC_INT_SEC))
+ return IRQ_NONE;
+
+ /* Clear RTC_INT_SEC interrupt only */
+ writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_STS);
+
+ spin_lock(&xrtcdev->lock);
+
+ writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_DIS);
+
+ spin_unlock(&xrtcdev->lock);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t xlnx_rtc_alarm_interrupt(int irq, void *id)
{
struct xlnx_rtc_dev *xrtcdev = (struct xlnx_rtc_dev *)id;
unsigned int status;
status = readl(xrtcdev->reg_base + RTC_INT_STS);
/* Check if interrupt asserted */
- if (!(status & (RTC_INT_SEC | RTC_INT_ALRM)))
+ if (!(status & RTC_INT_ALRM))
return IRQ_NONE;
/* Clear RTC_INT_ALRM interrupt only */
@@ -202,6 +231,8 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
if (!xrtcdev)
return -ENOMEM;
+ spin_lock_init(&xrtcdev->lock);
+
platform_set_drvdata(pdev, xrtcdev);
xrtcdev->rtc = devm_rtc_allocate_device(&pdev->dev);
@@ -221,7 +252,7 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
if (xrtcdev->alarm_irq < 0)
return xrtcdev->alarm_irq;
ret = devm_request_irq(&pdev->dev, xrtcdev->alarm_irq,
- xlnx_rtc_interrupt, 0,
+ xlnx_rtc_alarm_interrupt, 0,
dev_name(&pdev->dev), xrtcdev);
if (ret) {
dev_err(&pdev->dev, "request irq failed\n");
@@ -232,7 +263,7 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
if (xrtcdev->sec_irq < 0)
return xrtcdev->sec_irq;
ret = devm_request_irq(&pdev->dev, xrtcdev->sec_irq,
- xlnx_rtc_interrupt, 0,
+ xlnx_rtc_sec_interrupt, 0,
dev_name(&pdev->dev), xrtcdev);
if (ret) {
dev_err(&pdev->dev, "request irq failed\n");
--
2.23.0
