[PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Anurag Kumar Vulisha]

In order to conserve battery energy, during the PS operation,
it is expected that the supply for the battery-powered domain
to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
automatically be switched back to battery when VCC_PSAUX voltage
drops below a limit, doing so prevents the logic within
the battery-powered domain from functioning incorrectly.

This patch enables that feature.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 8b28762..6adb603 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -45,6 +45,7 @@
 #define RTC_INT_SEC		BIT(0)
 #define RTC_INT_ALRM		BIT(1)
 #define RTC_OSC_EN		BIT(24)
+#define RTC_BATT_EN		BIT(31)
 
 #define RTC_CALIB_DEF		0x198233
 #define RTC_CALIB_MASK		0x1FFFFF
@@ -122,6 +123,13 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 {
+	u32 rtc_ctrl;
+
+	/* Enable RTC switch to battery when VCC_PSAUX is not available */
+	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
+	rtc_ctrl |= RTC_BATT_EN;
+	writel(rtc_ctrl, xrtcdev->reg_base + RTC_CTRL);
+
 	/*
 	 * Based on crystal freq of 33.330 KHz
 	 * set the seconds counter and enable, set fractions counter
-- 
2.1.2

[rtc-linux] [PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Anurag Kumar Vulisha]

In order to conserve battery energy, during the PS operation,
it is expected that the supply for the battery-powered domain
to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
automatically be switched back to battery when VCC_PSAUX voltage
drops below a limit, doing so prevents the logic within
the battery-powered domain from functioning incorrectly.

This patch enables that feature.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 8b28762..6adb603 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -45,6 +45,7 @@
 #define RTC_INT_SEC		BIT(0)
 #define RTC_INT_ALRM		BIT(1)
 #define RTC_OSC_EN		BIT(24)
+#define RTC_BATT_EN		BIT(31)
 
 #define RTC_CALIB_DEF		0x198233
 #define RTC_CALIB_MASK		0x1FFFFF
@@ -122,6 +123,13 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 {
+	u32 rtc_ctrl;
+
+	/* Enable RTC switch to battery when VCC_PSAUX is not available */
+	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
+	rtc_ctrl |= RTC_BATT_EN;
+	writel(rtc_ctrl, xrtcdev->reg_base + RTC_CTRL);
+
 	/*
 	 * Based on crystal freq of 33.330 KHz
 	 * set the seconds counter and enable, set fractions counter
-- 
2.1.2

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Anurag Kumar Vulisha]

In order to conserve battery energy, during the PS operation,
it is expected that the supply for the battery-powered domain
to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
automatically be switched back to battery when VCC_PSAUX voltage
drops below a limit, doing so prevents the logic within
the battery-powered domain from functioning incorrectly.

This patch enables that feature.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 8b28762..6adb603 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -45,6 +45,7 @@
 #define RTC_INT_SEC		BIT(0)
 #define RTC_INT_ALRM		BIT(1)
 #define RTC_OSC_EN		BIT(24)
+#define RTC_BATT_EN		BIT(31)
 
 #define RTC_CALIB_DEF		0x198233
 #define RTC_CALIB_MASK		0x1FFFFF
@@ -122,6 +123,13 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 {
+	u32 rtc_ctrl;
+
+	/* Enable RTC switch to battery when VCC_PSAUX is not available */
+	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
+	rtc_ctrl |= RTC_BATT_EN;
+	writel(rtc_ctrl, xrtcdev->reg_base + RTC_CTRL);
+
 	/*
 	 * Based on crystal freq of 33.330 KHz
 	 * set the seconds counter and enable, set fractions counter
-- 
2.1.2

[PATCH 2/3] RTC: Write Calibration value before set time

[Anurag Kumar Vulisha]

It is suggested to programe CALIB_WRITE register with the calibration
value before updating the SET_TIME_WRITE register, doing so will
clear the Tick Counter and force the next second to be signaled
exactly in 1 second.

This patch updates the same.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 6adb603..f87f971 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -56,6 +56,7 @@ struct xlnx_rtc_dev {
 	void __iomem		*reg_base;
 	int			alarm_irq;
 	int			sec_irq;
+	int			calibval;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -68,6 +69,13 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (new_time > RTC_SEC_MAX_VAL)
 		return -EINVAL;
 
+	/*
+	 * Writing into calibration register will clear the Tick Counter and
+	 * force the next second to be signaled exactly in 1 second period
+	 */
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+
 	writel(new_time, xrtcdev->reg_base + RTC_SET_TM_WR);
 
 	return 0;
@@ -121,7 +129,7 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return 0;
 }
 
-static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
+static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
@@ -136,8 +144,8 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 	 * to default value suggested as per design spec
 	 * to correct RTC delay in frequency over period of time.
 	 */
-	calibval &= RTC_CALIB_MASK;
-	writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
 }
 
 static const struct rtc_class_ops xlnx_rtc_ops = {
@@ -174,7 +182,6 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	struct xlnx_rtc_dev *xrtcdev;
 	struct resource *res;
 	int ret;
-	unsigned int calibvalue;
 
 	xrtcdev = devm_kzalloc(&pdev->dev, sizeof(*xrtcdev), GFP_KERNEL);
 	if (!xrtcdev)
@@ -215,11 +222,11 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = of_property_read_u32(pdev->dev.of_node, "calibration",
-				   &calibvalue);
+				   &xrtcdev->calibval);
 	if (ret)
-		calibvalue = RTC_CALIB_DEF;
+		xrtcdev->calibval = RTC_CALIB_DEF;
 
-	xlnx_init_rtc(xrtcdev, calibvalue);
+	xlnx_init_rtc(xrtcdev);
 
 	device_init_wakeup(&pdev->dev, 1);
 
-- 
2.1.2

[rtc-linux] [PATCH 2/3] RTC: Write Calibration value before set time

[Anurag Kumar Vulisha]

It is suggested to programe CALIB_WRITE register with the calibration
value before updating the SET_TIME_WRITE register, doing so will
clear the Tick Counter and force the next second to be signaled
exactly in 1 second.

This patch updates the same.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 6adb603..f87f971 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -56,6 +56,7 @@ struct xlnx_rtc_dev {
 	void __iomem		*reg_base;
 	int			alarm_irq;
 	int			sec_irq;
+	int			calibval;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -68,6 +69,13 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (new_time > RTC_SEC_MAX_VAL)
 		return -EINVAL;
 
+	/*
+	 * Writing into calibration register will clear the Tick Counter and
+	 * force the next second to be signaled exactly in 1 second period
+	 */
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+
 	writel(new_time, xrtcdev->reg_base + RTC_SET_TM_WR);
 
 	return 0;
@@ -121,7 +129,7 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return 0;
 }
 
-static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
+static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
@@ -136,8 +144,8 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 	 * to default value suggested as per design spec
 	 * to correct RTC delay in frequency over period of time.
 	 */
-	calibval &= RTC_CALIB_MASK;
-	writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
 }
 
 static const struct rtc_class_ops xlnx_rtc_ops = {
@@ -174,7 +182,6 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	struct xlnx_rtc_dev *xrtcdev;
 	struct resource *res;
 	int ret;
-	unsigned int calibvalue;
 
 	xrtcdev = devm_kzalloc(&pdev->dev, sizeof(*xrtcdev), GFP_KERNEL);
 	if (!xrtcdev)
@@ -215,11 +222,11 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = of_property_read_u32(pdev->dev.of_node, "calibration",
-				   &calibvalue);
+				   &xrtcdev->calibval);
 	if (ret)
-		calibvalue = RTC_CALIB_DEF;
+		xrtcdev->calibval = RTC_CALIB_DEF;
 
-	xlnx_init_rtc(xrtcdev, calibvalue);
+	xlnx_init_rtc(xrtcdev);
 
 	device_init_wakeup(&pdev->dev, 1);
 
-- 
2.1.2

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2/3] RTC: Write Calibration value before set time

[Anurag Kumar Vulisha]

It is suggested to programe CALIB_WRITE register with the calibration
value before updating the SET_TIME_WRITE register, doing so will
clear the Tick Counter and force the next second to be signaled
exactly in 1 second.

This patch updates the same.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index 6adb603..f87f971 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -56,6 +56,7 @@ struct xlnx_rtc_dev {
 	void __iomem		*reg_base;
 	int			alarm_irq;
 	int			sec_irq;
+	int			calibval;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -68,6 +69,13 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	if (new_time > RTC_SEC_MAX_VAL)
 		return -EINVAL;
 
+	/*
+	 * Writing into calibration register will clear the Tick Counter and
+	 * force the next second to be signaled exactly in 1 second period
+	 */
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+
 	writel(new_time, xrtcdev->reg_base + RTC_SET_TM_WR);
 
 	return 0;
@@ -121,7 +129,7 @@ static int xlnx_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	return 0;
 }
 
-static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
+static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
@@ -136,8 +144,8 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev, u32 calibval)
 	 * to default value suggested as per design spec
 	 * to correct RTC delay in frequency over period of time.
 	 */
-	calibval &= RTC_CALIB_MASK;
-	writel(calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
+	xrtcdev->calibval &= RTC_CALIB_MASK;
+	writel(xrtcdev->calibval, (xrtcdev->reg_base + RTC_CALIB_WR));
 }
 
 static const struct rtc_class_ops xlnx_rtc_ops = {
@@ -174,7 +182,6 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	struct xlnx_rtc_dev *xrtcdev;
 	struct resource *res;
 	int ret;
-	unsigned int calibvalue;
 
 	xrtcdev = devm_kzalloc(&pdev->dev, sizeof(*xrtcdev), GFP_KERNEL);
 	if (!xrtcdev)
@@ -215,11 +222,11 @@ static int xlnx_rtc_probe(struct platform_device *pdev)
 	}
 
 	ret = of_property_read_u32(pdev->dev.of_node, "calibration",
-				   &calibvalue);
+				   &xrtcdev->calibval);
 	if (ret)
-		calibvalue = RTC_CALIB_DEF;
+		xrtcdev->calibval = RTC_CALIB_DEF;
 
-	xlnx_init_rtc(xrtcdev, calibvalue);
+	xlnx_init_rtc(xrtcdev);
 
 	device_init_wakeup(&pdev->dev, 1);
 
-- 
2.1.2

[PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

We programe RTC time using SET_TIME_WRITE register and read the RTC
current time using CURRENT_TIME register. When we set the time by
writing into SET_TIME_WRITE Register and immediately try to read the
rtc time from CURRENT_TIME register, the previous old value is
returned instead of the new loaded time. This is because RTC takes
nearly 1 sec to update the  new loaded value into the CURRENT_TIME
register. This behaviour is expected in our RTC IP.

This patch updates the driver to read the current time from SET_TIME_WRITE
register instead of CURRENT_TIME when rtc time is requested within an 1sec
period after setting the RTC time. Doing so will ensure the correct time
is given to the user.

Since there is an delay of 1sec in updating the CURRENT_TIME we are loading
set time +1sec while programming the SET_TIME_WRITE register, doing this
will give correct time without any delay when read from CURRENT_TIME.

This patch updates the above said.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index f87f971..b98cebe 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -57,6 +57,7 @@ struct xlnx_rtc_dev {
 	int			alarm_irq;
 	int			sec_irq;
 	int			calibval;
+	int			time_updated;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -64,6 +65,12 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 	unsigned long new_time;
 
+	/*
+	 * The value written will be updated after 1 sec into the
+	 * seconds read register, so we need to program time +1 sec
+	 * to get the correct time on read.
+	 */
+	tm->tm_sec += 1;
 	new_time = rtc_tm_to_time64(tm);
 
 	if (new_time > RTC_SEC_MAX_VAL)
@@ -78,6 +85,17 @@ 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
+	 */
+	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
+	xrtcdev->time_updated = 0;
+
 	return 0;
 }
 
@@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 
-	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	if (xrtcdev->time_updated == 0) {
+		/*
+		 * Time written in SET_TIME_WRITE has not yet updated into
+		 * the seconds read register, so read the time from the
+		 * SET_TIME_WRITE instead of CURRENT_TIME register.
+		 */
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
+		tm->tm_sec -= 1;
+	} else {
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	}
 
 	return rtc_valid_tm(tm);
 }
@@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
+	/* Enable RTC SEC interrupts */
+	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
+
 	/* Enable RTC switch to battery when VCC_PSAUX is not available */
 	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
 	rtc_ctrl |= RTC_BATT_EN;
@@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
 	/* Clear interrupt */
 	writel(status, xrtcdev->reg_base + RTC_INT_STS);
 
-	if (status & RTC_INT_SEC)
+	if (status & RTC_INT_SEC) {
+		if (xrtcdev->time_updated == 0) {
+			/* RTC updated the seconds read register */
+			xrtcdev->time_updated = 1;
+		}
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
+	}
 	if (status & RTC_INT_ALRM)
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
 
-- 
2.1.2

[rtc-linux] [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

We programe RTC time using SET_TIME_WRITE register and read the RTC
current time using CURRENT_TIME register. When we set the time by
writing into SET_TIME_WRITE Register and immediately try to read the
rtc time from CURRENT_TIME register, the previous old value is
returned instead of the new loaded time. This is because RTC takes
nearly 1 sec to update the  new loaded value into the CURRENT_TIME
register. This behaviour is expected in our RTC IP.

This patch updates the driver to read the current time from SET_TIME_WRITE
register instead of CURRENT_TIME when rtc time is requested within an 1sec
period after setting the RTC time. Doing so will ensure the correct time
is given to the user.

Since there is an delay of 1sec in updating the CURRENT_TIME we are loading
set time +1sec while programming the SET_TIME_WRITE register, doing this
will give correct time without any delay when read from CURRENT_TIME.

This patch updates the above said.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index f87f971..b98cebe 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -57,6 +57,7 @@ struct xlnx_rtc_dev {
 	int			alarm_irq;
 	int			sec_irq;
 	int			calibval;
+	int			time_updated;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -64,6 +65,12 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 	unsigned long new_time;
 
+	/*
+	 * The value written will be updated after 1 sec into the
+	 * seconds read register, so we need to program time +1 sec
+	 * to get the correct time on read.
+	 */
+	tm->tm_sec += 1;
 	new_time = rtc_tm_to_time64(tm);
 
 	if (new_time > RTC_SEC_MAX_VAL)
@@ -78,6 +85,17 @@ 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
+	 */
+	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
+	xrtcdev->time_updated = 0;
+
 	return 0;
 }
 
@@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 
-	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	if (xrtcdev->time_updated == 0) {
+		/*
+		 * Time written in SET_TIME_WRITE has not yet updated into
+		 * the seconds read register, so read the time from the
+		 * SET_TIME_WRITE instead of CURRENT_TIME register.
+		 */
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
+		tm->tm_sec -= 1;
+	} else {
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	}
 
 	return rtc_valid_tm(tm);
 }
@@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
+	/* Enable RTC SEC interrupts */
+	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
+
 	/* Enable RTC switch to battery when VCC_PSAUX is not available */
 	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
 	rtc_ctrl |= RTC_BATT_EN;
@@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
 	/* Clear interrupt */
 	writel(status, xrtcdev->reg_base + RTC_INT_STS);
 
-	if (status & RTC_INT_SEC)
+	if (status & RTC_INT_SEC) {
+		if (xrtcdev->time_updated == 0) {
+			/* RTC updated the seconds read register */
+			xrtcdev->time_updated = 1;
+		}
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
+	}
 	if (status & RTC_INT_ALRM)
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
 
-- 
2.1.2

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

We programe RTC time using SET_TIME_WRITE register and read the RTC
current time using CURRENT_TIME register. When we set the time by
writing into SET_TIME_WRITE Register and immediately try to read the
rtc time from CURRENT_TIME register, the previous old value is
returned instead of the new loaded time. This is because RTC takes
nearly 1 sec to update the  new loaded value into the CURRENT_TIME
register. This behaviour is expected in our RTC IP.

This patch updates the driver to read the current time from SET_TIME_WRITE
register instead of CURRENT_TIME when rtc time is requested within an 1sec
period after setting the RTC time. Doing so will ensure the correct time
is given to the user.

Since there is an delay of 1sec in updating the CURRENT_TIME we are loading
set time +1sec while programming the SET_TIME_WRITE register, doing this
will give correct time without any delay when read from CURRENT_TIME.

This patch updates the above said.

Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
---
 drivers/rtc/rtc-zynqmp.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/drivers/rtc/rtc-zynqmp.c b/drivers/rtc/rtc-zynqmp.c
index f87f971..b98cebe 100644
--- a/drivers/rtc/rtc-zynqmp.c
+++ b/drivers/rtc/rtc-zynqmp.c
@@ -57,6 +57,7 @@ struct xlnx_rtc_dev {
 	int			alarm_irq;
 	int			sec_irq;
 	int			calibval;
+	int			time_updated;
 };
 
 static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
@@ -64,6 +65,12 @@ static int xlnx_rtc_set_time(struct device *dev, struct rtc_time *tm)
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 	unsigned long new_time;
 
+	/*
+	 * The value written will be updated after 1 sec into the
+	 * seconds read register, so we need to program time +1 sec
+	 * to get the correct time on read.
+	 */
+	tm->tm_sec += 1;
 	new_time = rtc_tm_to_time64(tm);
 
 	if (new_time > RTC_SEC_MAX_VAL)
@@ -78,6 +85,17 @@ 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
+	 */
+	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);
+	xrtcdev->time_updated = 0;
+
 	return 0;
 }
 
@@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
 {
 	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
 
-	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	if (xrtcdev->time_updated == 0) {
+		/*
+		 * Time written in SET_TIME_WRITE has not yet updated into
+		 * the seconds read register, so read the time from the
+		 * SET_TIME_WRITE instead of CURRENT_TIME register.
+		 */
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
+		tm->tm_sec -= 1;
+	} else {
+		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
+	}
 
 	return rtc_valid_tm(tm);
 }
@@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
 {
 	u32 rtc_ctrl;
 
+	/* Enable RTC SEC interrupts */
+	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
+
 	/* Enable RTC switch to battery when VCC_PSAUX is not available */
 	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
 	rtc_ctrl |= RTC_BATT_EN;
@@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
 	/* Clear interrupt */
 	writel(status, xrtcdev->reg_base + RTC_INT_STS);
 
-	if (status & RTC_INT_SEC)
+	if (status & RTC_INT_SEC) {
+		if (xrtcdev->time_updated == 0) {
+			/* RTC updated the seconds read register */
+			xrtcdev->time_updated = 1;
+		}
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
+	}
 	if (status & RTC_INT_ALRM)
 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
 
-- 
2.1.2

Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

Hi,

Please use rtc: zynqmp in your subject line.

On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> @@ -78,6 +85,17 @@ 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
> +	 */
> +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);

You probably shouldn't clear RTC_INT_ALRM here but it should be done in
the init and when enabling/disabling alarm. Or maybe easier would be to
ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
RTC_INT_MASK.

Or, instead of using interrupts, can't you simply read RTC_INT_STS in
xlnx_rtc_read_time()? Is it updated even when the interrupt is not
enabled?

> +	xrtcdev->time_updated = 0;
> +
>  	return 0;
>  }
>  
> @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>  
> -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	if (xrtcdev->time_updated == 0) {
> +		/*
> +		 * Time written in SET_TIME_WRITE has not yet updated into
> +		 * the seconds read register, so read the time from the
> +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> +		 */
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
> +		tm->tm_sec -= 1;
> +	} else {
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	}
>  
>  	return rtc_valid_tm(tm);
>  }
> @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
>  {
>  	u32 rtc_ctrl;
>  
> +	/* Enable RTC SEC interrupts */
> +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> +
>  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
>  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
>  	rtc_ctrl |= RTC_BATT_EN;
> @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
>  	/* Clear interrupt */
>  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
>  
> -	if (status & RTC_INT_SEC)
> +	if (status & RTC_INT_SEC) {
> +		if (xrtcdev->time_updated == 0) {
> +			/* RTC updated the seconds read register */
> +			xrtcdev->time_updated = 1;
> +		}
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> +	}
>  	if (status & RTC_INT_ALRM)
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
>  
> -- 
> 2.1.2
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

Hi,

Please use rtc: zynqmp in your subject line.

On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> @@ -78,6 +85,17 @@ 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
> +	 */
> +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);

You probably shouldn't clear RTC_INT_ALRM here but it should be done in
the init and when enabling/disabling alarm. Or maybe easier would be to
ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
RTC_INT_MASK.

Or, instead of using interrupts, can't you simply read RTC_INT_STS in
xlnx_rtc_read_time()? Is it updated even when the interrupt is not
enabled?

> +	xrtcdev->time_updated = 0;
> +
>  	return 0;
>  }
>  
> @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>  
> -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	if (xrtcdev->time_updated == 0) {
> +		/*
> +		 * Time written in SET_TIME_WRITE has not yet updated into
> +		 * the seconds read register, so read the time from the
> +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> +		 */
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
> +		tm->tm_sec -= 1;
> +	} else {
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	}
>  
>  	return rtc_valid_tm(tm);
>  }
> @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
>  {
>  	u32 rtc_ctrl;
>  
> +	/* Enable RTC SEC interrupts */
> +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> +
>  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
>  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
>  	rtc_ctrl |= RTC_BATT_EN;
> @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
>  	/* Clear interrupt */
>  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
>  
> -	if (status & RTC_INT_SEC)
> +	if (status & RTC_INT_SEC) {
> +		if (xrtcdev->time_updated == 0) {
> +			/* RTC updated the seconds read register */
> +			xrtcdev->time_updated = 1;
> +		}
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> +	}
>  	if (status & RTC_INT_ALRM)
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
>  
> -- 
> 2.1.2
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

Hi,

Please use rtc: zynqmp in your subject line.

On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> @@ -78,6 +85,17 @@ 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
> +	 */
> +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base + RTC_INT_STS);

You probably shouldn't clear RTC_INT_ALRM here but it should be done in
the init and when enabling/disabling alarm. Or maybe easier would be to
ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
RTC_INT_MASK.

Or, instead of using interrupts, can't you simply read RTC_INT_STS in
xlnx_rtc_read_time()? Is it updated even when the interrupt is not
enabled?

> +	xrtcdev->time_updated = 0;
> +
>  	return 0;
>  }
>  
> @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev, struct rtc_time *tm)
>  {
>  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
>  
> -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	if (xrtcdev->time_updated == 0) {
> +		/*
> +		 * Time written in SET_TIME_WRITE has not yet updated into
> +		 * the seconds read register, so read the time from the
> +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> +		 */
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD), tm);
> +		tm->tm_sec -= 1;
> +	} else {
> +		rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> +	}
>  
>  	return rtc_valid_tm(tm);
>  }
> @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev *xrtcdev)
>  {
>  	u32 rtc_ctrl;
>  
> +	/* Enable RTC SEC interrupts */
> +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> +
>  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
>  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
>  	rtc_ctrl |= RTC_BATT_EN;
> @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void *id)
>  	/* Clear interrupt */
>  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
>  
> -	if (status & RTC_INT_SEC)
> +	if (status & RTC_INT_SEC) {
> +		if (xrtcdev->time_updated == 0) {
> +			/* RTC updated the seconds read register */
> +			xrtcdev->time_updated = 1;
> +		}
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> +	}
>  	if (status & RTC_INT_ALRM)
>  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
>  
> -- 
> 2.1.2
> 

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Alexandre Belloni]

On 12/04/2016 at 17:45:44 +0530, Anurag Kumar Vulisha wrote :
> In order to conserve battery energy, during the PS operation,
> it is expected that the supply for the battery-powered domain
> to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
> automatically be switched back to battery when VCC_PSAUX voltage
> drops below a limit, doing so prevents the logic within
> the battery-powered domain from functioning incorrectly.
> 
> This patch enables that feature.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Alexandre Belloni]

On 12/04/2016 at 17:45:44 +0530, Anurag Kumar Vulisha wrote :
> In order to conserve battery energy, during the PS operation,
> it is expected that the supply for the battery-powered domain
> to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
> automatically be switched back to battery when VCC_PSAUX voltage
> drops below a limit, doing so prevents the logic within
> the battery-powered domain from functioning incorrectly.
> 
> This patch enables that feature.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 1/3] RTC: Enable RTC switching to battery power when VCC_PSAUX is N/A

[Alexandre Belloni]

On 12/04/2016 at 17:45:44 +0530, Anurag Kumar Vulisha wrote :
> In order to conserve battery energy, during the PS operation,
> it is expected that the supply for the battery-powered domain
> to be switched from the battery (VCC_PSBATT) to (VCC_PSAUX) and
> automatically be switched back to battery when VCC_PSAUX voltage
> drops below a limit, doing so prevents the logic within
> the battery-powered domain from functioning incorrectly.
> 
> This patch enables that feature.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/3] RTC: Write Calibration value before set time

[Alexandre Belloni]

On 12/04/2016 at 17:45:45 +0530, Anurag Kumar Vulisha wrote :
> It is suggested to programe CALIB_WRITE register with the calibration
> value before updating the SET_TIME_WRITE register, doing so will
> clear the Tick Counter and force the next second to be signaled
> exactly in 1 second.
> 
> This patch updates the same.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 2/3] RTC: Write Calibration value before set time

[Alexandre Belloni]

On 12/04/2016 at 17:45:45 +0530, Anurag Kumar Vulisha wrote :
> It is suggested to programe CALIB_WRITE register with the calibration
> value before updating the SET_TIME_WRITE register, doing so will
> clear the Tick Counter and force the next second to be signaled
> exactly in 1 second.
> 
> This patch updates the same.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 2/3] RTC: Write Calibration value before set time

[Alexandre Belloni]

On 12/04/2016 at 17:45:45 +0530, Anurag Kumar Vulisha wrote :
> It is suggested to programe CALIB_WRITE register with the calibration
> value before updating the SET_TIME_WRITE register, doing so will
> clear the Tick Counter and force the next second to be signaled
> exactly in 1 second.
> 
> This patch updates the same.
> 
> Signed-off-by: Anurag Kumar Vulisha <anuragku@xilinx.com>
> ---
>  drivers/rtc/rtc-zynqmp.c | 21 ++++++++++++++-------
>  1 file changed, 14 insertions(+), 7 deletions(-)
> 
Applied, thanks.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 4:01 AM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Anurag Kumar Vulisha <anuragku@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> 
> Hi,
> 
> Please use rtc: zynqmp in your subject line.
> 
> On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> > @@ -78,6 +85,17 @@ 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
> > +	 */
> > +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base +
> RTC_INT_STS);
> 
> You probably shouldn't clear RTC_INT_ALRM here but it should be done in
> the init and when enabling/disabling alarm. Or maybe easier would be to
> ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
> RTC_INT_MASK.
>

Thanks for reviewing the patch. I will remove this clearing  RTC_INT_ALRM interrupt logic
and send the next version of the patch.
 
> Or, instead of using interrupts, can't you simply read RTC_INT_STS in
> xlnx_rtc_read_time()? Is it updated even when the interrupt is not enabled?
> 

The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
load time + 1sec into the write register. So when read we would be getting the correct time without
any delay. If any request comes from user to read time before RTC updating the read register, we
need to give the previous loaded time instead of giving the time from the read register. 
For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
Clear the RTC_INT_STS register while programming the time into the write register . If we get a
request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
is set in RTC_INT_STS, we need to give the previous loaded time.
This should be done if time is requested from user space within  1 sec period after writing time, after
the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
occurance in the interrupt handler.
Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.

Thanks,
Anurag Kumar V
  
> > +	xrtcdev->time_updated = 0;
> > +
> >  	return 0;
> >  }
> >
> > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > struct rtc_time *tm)  {
> >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >
> > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > +	if (xrtcdev->time_updated == 0) {
> > +		/*
> > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > +		 * the seconds read register, so read the time from the
> > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > +		 */
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_SET_TM_RD), tm);
> > +		tm->tm_sec -= 1;
> > +	} else {
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_CUR_TM), tm);
> > +	}
> >
> >  	return rtc_valid_tm(tm);
> >  }
> > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > *xrtcdev)  {
> >  	u32 rtc_ctrl;
> >
> > +	/* Enable RTC SEC interrupts */
> > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > +
> >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> >  	rtc_ctrl |= RTC_BATT_EN;
> > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> *id)
> >  	/* Clear interrupt */
> >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> >
> > -	if (status & RTC_INT_SEC)
> > +	if (status & RTC_INT_SEC) {
> > +		if (xrtcdev->time_updated == 0) {
> > +			/* RTC updated the seconds read register */
> > +			xrtcdev->time_updated = 1;
> > +		}
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > +	}
> >  	if (status & RTC_INT_ALRM)
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> >
> > --
> > 2.1.2
> >
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

[rtc-linux] RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 4:01 AM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Anurag Kumar Vulisha <anuragku@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> 
> Hi,
> 
> Please use rtc: zynqmp in your subject line.
> 
> On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> > @@ -78,6 +85,17 @@ 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
> > +	 */
> > +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base +
> RTC_INT_STS);
> 
> You probably shouldn't clear RTC_INT_ALRM here but it should be done in
> the init and when enabling/disabling alarm. Or maybe easier would be to
> ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
> RTC_INT_MASK.
>

Thanks for reviewing the patch. I will remove this clearing  RTC_INT_ALRM interrupt logic
and send the next version of the patch.
 
> Or, instead of using interrupts, can't you simply read RTC_INT_STS in
> xlnx_rtc_read_time()? Is it updated even when the interrupt is not enabled?
> 

The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
load time + 1sec into the write register. So when read we would be getting the correct time without
any delay. If any request comes from user to read time before RTC updating the read register, we
need to give the previous loaded time instead of giving the time from the read register. 
For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
Clear the RTC_INT_STS register while programming the time into the write register . If we get a
request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
is set in RTC_INT_STS, we need to give the previous loaded time.
This should be done if time is requested from user space within  1 sec period after writing time, after
the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
occurance in the interrupt handler.
Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.

Thanks,
Anurag Kumar V
  
> > +	xrtcdev->time_updated = 0;
> > +
> >  	return 0;
> >  }
> >
> > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > struct rtc_time *tm)  {
> >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >
> > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > +	if (xrtcdev->time_updated == 0) {
> > +		/*
> > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > +		 * the seconds read register, so read the time from the
> > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > +		 */
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_SET_TM_RD), tm);
> > +		tm->tm_sec -= 1;
> > +	} else {
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_CUR_TM), tm);
> > +	}
> >
> >  	return rtc_valid_tm(tm);
> >  }
> > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > *xrtcdev)  {
> >  	u32 rtc_ctrl;
> >
> > +	/* Enable RTC SEC interrupts */
> > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > +
> >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> >  	rtc_ctrl |= RTC_BATT_EN;
> > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> *id)
> >  	/* Clear interrupt */
> >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> >
> > -	if (status & RTC_INT_SEC)
> > +	if (status & RTC_INT_SEC) {
> > +		if (xrtcdev->time_updated == 0) {
> > +			/* RTC updated the seconds read register */
> > +			xrtcdev->time_updated = 1;
> > +		}
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > +	}
> >  	if (status & RTC_INT_ALRM)
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> >
> > --
> > 2.1.2
> >
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni at free-electrons.com]
> Sent: Wednesday, April 20, 2016 4:01 AM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux at googlegroups.com; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Anurag Kumar Vulisha <anuragku@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> 
> Hi,
> 
> Please use rtc: zynqmp in your subject line.
> 
> On 12/04/2016 at 17:45:46 +0530, Anurag Kumar Vulisha wrote :
> > @@ -78,6 +85,17 @@ 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
> > +	 */
> > +	writel(RTC_INT_SEC | RTC_INT_ALRM, xrtcdev->reg_base +
> RTC_INT_STS);
> 
> You probably shouldn't clear RTC_INT_ALRM here but it should be done in
> the init and when enabling/disabling alarm. Or maybe easier would be to
> ignore RTC_INT_ALRM in xlnx_rtc_interrupt when it is not set in
> RTC_INT_MASK.
>

Thanks for reviewing the patch. I will remove this clearing  RTC_INT_ALRM interrupt logic
and send the next version of the patch.
 
> Or, instead of using interrupts, can't you simply read RTC_INT_STS in
> xlnx_rtc_read_time()? Is it updated even when the interrupt is not enabled?
> 

The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
load time + 1sec into the write register. So when read we would be getting the correct time without
any delay. If any request comes from user to read time before RTC updating the read register, we
need to give the previous loaded time instead of giving the time from the read register. 
For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
Clear the RTC_INT_STS register while programming the time into the write register . If we get a
request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
is set in RTC_INT_STS, we need to give the previous loaded time.
This should be done if time is requested from user space within  1 sec period after writing time, after
the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
occurance in the interrupt handler.
Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.

Thanks,
Anurag Kumar V
  
> > +	xrtcdev->time_updated = 0;
> > +
> >  	return 0;
> >  }
> >
> > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > struct rtc_time *tm)  {
> >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> >
> > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > +	if (xrtcdev->time_updated == 0) {
> > +		/*
> > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > +		 * the seconds read register, so read the time from the
> > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > +		 */
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_SET_TM_RD), tm);
> > +		tm->tm_sec -= 1;
> > +	} else {
> > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> RTC_CUR_TM), tm);
> > +	}
> >
> >  	return rtc_valid_tm(tm);
> >  }
> > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > *xrtcdev)  {
> >  	u32 rtc_ctrl;
> >
> > +	/* Enable RTC SEC interrupts */
> > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > +
> >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> >  	rtc_ctrl |= RTC_BATT_EN;
> > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> *id)
> >  	/* Clear interrupt */
> >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> >
> > -	if (status & RTC_INT_SEC)
> > +	if (status & RTC_INT_SEC) {
> > +		if (xrtcdev->time_updated == 0) {
> > +			/* RTC updated the seconds read register */
> > +			xrtcdev->time_updated = 1;
> > +		}
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > +	}
> >  	if (status & RTC_INT_ALRM)
> >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> >
> > --
> > 2.1.2
> >
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering
> http://free-electrons.com

Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
> delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
> load time + 1sec into the write register. So when read we would be getting the correct time without
> any delay. If any request comes from user to read time before RTC updating the read register, we
> need to give the previous loaded time instead of giving the time from the read register. 
> For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
> Clear the RTC_INT_STS register while programming the time into the write register . If we get a
> request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
> is set in RTC_INT_STS, we need to give the previous loaded time.
> This should be done if time is requested from user space within  1 sec period after writing time, after
> the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
> the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
> depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
> occurance in the interrupt handler.
> Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
> the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.
> 

Yeas, I understood that. But my question was whether the interrupt
handling was necessary at all.
Instead of waiting for an interrupt to set time_updated, can't you
simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
xlnx_rtc_read_time() ?

Something like:

status = readl(xrtcdev->reg_base + RTC_INT_STS)
if (status & RTC_INT_SEC)
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
else
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1, tm);

It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
updated even when it is not enabled as an interrupt.

> Thanks,
> Anurag Kumar V
>   
> > > +	xrtcdev->time_updated = 0;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > > struct rtc_time *tm)  {
> > >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > >
> > > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > +	if (xrtcdev->time_updated == 0) {
> > > +		/*
> > > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > > +		 * the seconds read register, so read the time from the
> > > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > +		 */
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_SET_TM_RD), tm);
> > > +		tm->tm_sec -= 1;
> > > +	} else {
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_CUR_TM), tm);
> > > +	}
> > >
> > >  	return rtc_valid_tm(tm);
> > >  }
> > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > *xrtcdev)  {
> > >  	u32 rtc_ctrl;
> > >
> > > +	/* Enable RTC SEC interrupts */
> > > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > +
> > >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> > >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > >  	rtc_ctrl |= RTC_BATT_EN;
> > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> > *id)
> > >  	/* Clear interrupt */
> > >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > >
> > > -	if (status & RTC_INT_SEC)
> > > +	if (status & RTC_INT_SEC) {
> > > +		if (xrtcdev->time_updated == 0) {
> > > +			/* RTC updated the seconds read register */
> > > +			xrtcdev->time_updated = 1;
> > > +		}
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > +	}
> > >  	if (status & RTC_INT_ALRM)
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > >
> > > --
> > > 2.1.2
> > >
> > 
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux, Kernel and Android engineering
> > http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
> delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
> load time + 1sec into the write register. So when read we would be getting the correct time without
> any delay. If any request comes from user to read time before RTC updating the read register, we
> need to give the previous loaded time instead of giving the time from the read register. 
> For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
> Clear the RTC_INT_STS register while programming the time into the write register . If we get a
> request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
> is set in RTC_INT_STS, we need to give the previous loaded time.
> This should be done if time is requested from user space within  1 sec period after writing time, after
> the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
> the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
> depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
> occurance in the interrupt handler.
> Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
> the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.
> 

Yeas, I understood that. But my question was whether the interrupt
handling was necessary at all.
Instead of waiting for an interrupt to set time_updated, can't you
simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
xlnx_rtc_read_time() ?

Something like:

status = readl(xrtcdev->reg_base + RTC_INT_STS)
if (status & RTC_INT_SEC)
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
else
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1, tm);

It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
updated even when it is not enabled as an interrupt.

> Thanks,
> Anurag Kumar V
>   
> > > +	xrtcdev->time_updated = 0;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > > struct rtc_time *tm)  {
> > >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > >
> > > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > +	if (xrtcdev->time_updated == 0) {
> > > +		/*
> > > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > > +		 * the seconds read register, so read the time from the
> > > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > +		 */
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_SET_TM_RD), tm);
> > > +		tm->tm_sec -= 1;
> > > +	} else {
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_CUR_TM), tm);
> > > +	}
> > >
> > >  	return rtc_valid_tm(tm);
> > >  }
> > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > *xrtcdev)  {
> > >  	u32 rtc_ctrl;
> > >
> > > +	/* Enable RTC SEC interrupts */
> > > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > +
> > >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> > >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > >  	rtc_ctrl |= RTC_BATT_EN;
> > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> > *id)
> > >  	/* Clear interrupt */
> > >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > >
> > > -	if (status & RTC_INT_SEC)
> > > +	if (status & RTC_INT_SEC) {
> > > +		if (xrtcdev->time_updated == 0) {
> > > +			/* RTC updated the seconds read register */
> > > +			xrtcdev->time_updated = 1;
> > > +		}
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > +	}
> > >  	if (status & RTC_INT_ALRM)
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > >
> > > --
> > > 2.1.2
> > >
> > 
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux, Kernel and Android engineering
> > http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> The reason for me keeping this logic is, our RTC controller updates the read register after 1 sec
> delay, so when read , it gives 1 sec delay(correct time - 1 sec). So to avoid that we are programming
> load time + 1sec into the write register. So when read we would be getting the correct time without
> any delay. If any request comes from user to read time before RTC updating the read register, we
> need to give the previous loaded time instead of giving the time from the read register. 
> For doing the above said, we are relaying on seconds interrupt in  RTC_INT_STS register. We
> Clear the RTC_INT_STS register while programming the time into the write register . If we get a
> request from user to read time within the 1 sec period i.e before the RTC_INT_SEC interrupt bit
> is set in RTC_INT_STS, we need to give the previous loaded time.
> This should be done if time is requested from user space within  1 sec period after writing time, after
> the 1 sec  delay if user requested the time , we can  give the give time from read register . This is because
> the correct time is being updated in the read register after 1 sec delay. For this logic to happen we are
> depending on  xrtcdev->time_updated  variable to get updated after the very fist RTC_INT_SEC interrupt
> occurance in the interrupt handler.
> Since we are relaying on  xrtcdev->time_updated  to get updated from interrupt handler, I think reading
> the RTC_INT_STS in xlnx_rtc_read_time() is not helpful.
> 

Yeas, I understood that. But my question was whether the interrupt
handling was necessary at all.
Instead of waiting for an interrupt to set time_updated, can't you
simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
xlnx_rtc_read_time() ?

Something like:

status = readl(xrtcdev->reg_base + RTC_INT_STS)
if (status & RTC_INT_SEC)
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
else
	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1, tm);

It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
updated even when it is not enabled as an interrupt.

> Thanks,
> Anurag Kumar V
>   
> > > +	xrtcdev->time_updated = 0;
> > > +
> > >  	return 0;
> > >  }
> > >
> > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device *dev,
> > > struct rtc_time *tm)  {
> > >  	struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > >
> > > -	rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > +	if (xrtcdev->time_updated == 0) {
> > > +		/*
> > > +		 * Time written in SET_TIME_WRITE has not yet updated into
> > > +		 * the seconds read register, so read the time from the
> > > +		 * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > +		 */
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_SET_TM_RD), tm);
> > > +		tm->tm_sec -= 1;
> > > +	} else {
> > > +		rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > RTC_CUR_TM), tm);
> > > +	}
> > >
> > >  	return rtc_valid_tm(tm);
> > >  }
> > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > *xrtcdev)  {
> > >  	u32 rtc_ctrl;
> > >
> > > +	/* Enable RTC SEC interrupts */
> > > +	writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > +
> > >  	/* Enable RTC switch to battery when VCC_PSAUX is not available */
> > >  	rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > >  	rtc_ctrl |= RTC_BATT_EN;
> > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int irq, void
> > *id)
> > >  	/* Clear interrupt */
> > >  	writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > >
> > > -	if (status & RTC_INT_SEC)
> > > +	if (status & RTC_INT_SEC) {
> > > +		if (xrtcdev->time_updated == 0) {
> > > +			/* RTC updated the seconds read register */
> > > +			xrtcdev->time_updated = 1;
> > > +		}
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > +	}
> > >  	if (status & RTC_INT_ALRM)
> > >  		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > >
> > > --
> > > 2.1.2
> > >
> > 
> > --
> > Alexandre Belloni, Free Electrons
> > Embedded Linux, Kernel and Android engineering
> > http://free-electrons.com

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 1:28 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
>
> On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> > The reason for me keeping this logic is, our RTC controller updates
> > the read register after 1 sec delay, so when read , it gives 1 sec
> > delay(correct time - 1 sec). So to avoid that we are programming load
> > time + 1sec into the write register. So when read we would be getting
> > the correct time without any delay. If any request comes from user to read
> time before RTC updating the read register, we need to give the previous
> loaded time instead of giving the time from the read register.
> > For doing the above said, we are relaying on seconds interrupt in
> > RTC_INT_STS register. We Clear the RTC_INT_STS register while
> > programming the time into the write register . If we get a request
> > from user to read time within the 1 sec period i.e before the RTC_INT_SEC
> interrupt bit is set in RTC_INT_STS, we need to give the previous loaded
> time.
> > This should be done if time is requested from user space within  1 sec
> > period after writing time, after the 1 sec  delay if user requested
> > the time , we can  give the give time from read register . This is
> > because the correct time is being updated in the read register after 1
> > sec delay. For this logic to happen we are depending on  xrtcdev-
> >time_updated  variable to get updated after the very fist RTC_INT_SEC
> interrupt occurance in the interrupt handler.
> > Since we are relaying on  xrtcdev->time_updated  to get updated from
> > interrupt handler, I think reading the RTC_INT_STS in xlnx_rtc_read_time()
> is not helpful.
> >
>
> Yeas, I understood that. But my question was whether the interrupt handling
> was necessary at all.
> Instead of waiting for an interrupt to set time_updated, can't you simply read
> RTC_INT_STS and check for the RTC_INT_SEC bit in
> xlnx_rtc_read_time() ?
>
> Something like:
>
> status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SEC)
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> else
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> tm);
>
> It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> updated even when it is not enabled as an interrupt.
>

The above said logic will work if we doesn't clear the RTC_INT_STS register after the
RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If interrupts
are enabled we will be clearing the RTC_INT_STS every time in the interrupt handler.
And moreover we need to return time from RTC_SET_TM_RD only if time is requested
within 1 sec span after programming the time only , so this is required only for one time.
Since we are clearing the RTC_INT_STS in our interrupt handler, we might end up in giving
the wrong time to the user when requested.So I think this logic might not work.
Please correct me if am wrong.

Thanks,
Anurag Kumar V

> > Thanks,
> > Anurag Kumar V
> >
> > > > +       xrtcdev->time_updated = 0;
> > > > +
> > > >         return 0;
> > > >  }
> > > >
> > > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device
> > > > *dev, struct rtc_time *tm)  {
> > > >         struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > > >
> > > > -       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > > +       if (xrtcdev->time_updated == 0) {
> > > > +               /*
> > > > +                * Time written in SET_TIME_WRITE has not yet updated into
> > > > +                * the seconds read register, so read the time from the
> > > > +                * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > > +                */
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_SET_TM_RD), tm);
> > > > +               tm->tm_sec -= 1;
> > > > +       } else {
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_CUR_TM), tm);
> > > > +       }
> > > >
> > > >         return rtc_valid_tm(tm);
> > > >  }
> > > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > > *xrtcdev)  {
> > > >         u32 rtc_ctrl;
> > > >
> > > > +       /* Enable RTC SEC interrupts */
> > > > +       writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > > +
> > > >         /* Enable RTC switch to battery when VCC_PSAUX is not available */
> > > >         rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > > >         rtc_ctrl |= RTC_BATT_EN;
> > > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int
> > > > irq, void
> > > *id)
> > > >         /* Clear interrupt */
> > > >         writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > > >
> > > > -       if (status & RTC_INT_SEC)
> > > > +       if (status & RTC_INT_SEC) {
> > > > +               if (xrtcdev->time_updated == 0) {
> > > > +                       /* RTC updated the seconds read register */
> > > > +                       xrtcdev->time_updated = 1;
> > > > +               }
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > > +       }
> > > >         if (status & RTC_INT_ALRM)
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > > >
> > > > --
> > > > 2.1.2
> > > >
> > >
> > > --
> > > Alexandre Belloni, Free Electrons
> > > Embedded Linux, Kernel and Android engineering
> > > http://free-electrons.com
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

[rtc-linux] RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 1:28 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
>
> On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> > The reason for me keeping this logic is, our RTC controller updates
> > the read register after 1 sec delay, so when read , it gives 1 sec
> > delay(correct time - 1 sec). So to avoid that we are programming load
> > time + 1sec into the write register. So when read we would be getting
> > the correct time without any delay. If any request comes from user to r=
ead
> time before RTC updating the read register, we need to give the previous
> loaded time instead of giving the time from the read register.
> > For doing the above said, we are relaying on seconds interrupt in
> > RTC_INT_STS register. We Clear the RTC_INT_STS register while
> > programming the time into the write register . If we get a request
> > from user to read time within the 1 sec period i.e before the RTC_INT_S=
EC
> interrupt bit is set in RTC_INT_STS, we need to give the previous loaded
> time.
> > This should be done if time is requested from user space within  1 sec
> > period after writing time, after the 1 sec  delay if user requested
> > the time , we can  give the give time from read register . This is
> > because the correct time is being updated in the read register after 1
> > sec delay. For this logic to happen we are depending on  xrtcdev-
> >time_updated  variable to get updated after the very fist RTC_INT_SEC
> interrupt occurance in the interrupt handler.
> > Since we are relaying on  xrtcdev->time_updated  to get updated from
> > interrupt handler, I think reading the RTC_INT_STS in xlnx_rtc_read_tim=
e()
> is not helpful.
> >
>
> Yeas, I understood that. But my question was whether the interrupt handli=
ng
> was necessary at all.
> Instead of waiting for an interrupt to set time_updated, can't you simply=
 read
> RTC_INT_STS and check for the RTC_INT_SEC bit in
> xlnx_rtc_read_time() ?
>
> Something like:
>
> status =3D readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SE=
C)
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> else
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> tm);
>
> It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> updated even when it is not enabled as an interrupt.
>

The above said logic will work if we doesn't clear the RTC_INT_STS register=
 after the
RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If=
 interrupts
are enabled we will be clearing the RTC_INT_STS every time in the interrupt=
 handler.
And moreover we need to return time from RTC_SET_TM_RD only if time is requ=
ested
within 1 sec span after programming the time only , so this is required onl=
y for one time.
Since we are clearing the RTC_INT_STS in our interrupt handler, we might en=
d up in giving
the wrong time to the user when requested.So I think this logic might not w=
ork.
Please correct me if am wrong.

Thanks,
Anurag Kumar V

> > Thanks,
> > Anurag Kumar V
> >
> > > > +       xrtcdev->time_updated =3D 0;
> > > > +
> > > >         return 0;
> > > >  }
> > > >
> > > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device
> > > > *dev, struct rtc_time *tm)  {
> > > >         struct xlnx_rtc_dev *xrtcdev =3D dev_get_drvdata(dev);
> > > >
> > > > -       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm)=
;
> > > > +       if (xrtcdev->time_updated =3D=3D 0) {
> > > > +               /*
> > > > +                * Time written in SET_TIME_WRITE has not yet updat=
ed into
> > > > +                * the seconds read register, so read the time from=
 the
> > > > +                * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > > +                */
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_SET_TM_RD), tm);
> > > > +               tm->tm_sec -=3D 1;
> > > > +       } else {
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_CUR_TM), tm);
> > > > +       }
> > > >
> > > >         return rtc_valid_tm(tm);
> > > >  }
> > > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > > *xrtcdev)  {
> > > >         u32 rtc_ctrl;
> > > >
> > > > +       /* Enable RTC SEC interrupts */
> > > > +       writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > > +
> > > >         /* Enable RTC switch to battery when VCC_PSAUX is not avail=
able */
> > > >         rtc_ctrl =3D readl(xrtcdev->reg_base + RTC_CTRL);
> > > >         rtc_ctrl |=3D RTC_BATT_EN;
> > > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int
> > > > irq, void
> > > *id)
> > > >         /* Clear interrupt */
> > > >         writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > > >
> > > > -       if (status & RTC_INT_SEC)
> > > > +       if (status & RTC_INT_SEC) {
> > > > +               if (xrtcdev->time_updated =3D=3D 0) {
> > > > +                       /* RTC updated the seconds read register */
> > > > +                       xrtcdev->time_updated =3D 1;
> > > > +               }
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > > +       }
> > > >         if (status & RTC_INT_ALRM)
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > > >
> > > > --
> > > > 2.1.2
> > > >
> > >
> > > --
> > > Alexandre Belloni, Free Electrons
> > > Embedded Linux, Kernel and Android engineering
> > > http://free-electrons.com
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com


This email and any attachments are intended for the sole use of the named r=
ecipient(s) and contain(s) confidential information that may be proprietary=
, privileged or copyrighted under applicable law. If you are not the intend=
ed recipient, do not read, copy, or forward this email message or any attac=
hments. Delete this email message and any attachments immediately.

--=20
--=20
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---=20
You received this message because you are subscribed to the Google Groups "=
rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an e=
mail to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni at free-electrons.com]
> Sent: Wednesday, April 20, 2016 1:28 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux at googlegroups.com; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
>
> On 20/04/2016 at 07:10:22 +0000, Anurag Kumar Vulisha wrote :
> > The reason for me keeping this logic is, our RTC controller updates
> > the read register after 1 sec delay, so when read , it gives 1 sec
> > delay(correct time - 1 sec). So to avoid that we are programming load
> > time + 1sec into the write register. So when read we would be getting
> > the correct time without any delay. If any request comes from user to read
> time before RTC updating the read register, we need to give the previous
> loaded time instead of giving the time from the read register.
> > For doing the above said, we are relaying on seconds interrupt in
> > RTC_INT_STS register. We Clear the RTC_INT_STS register while
> > programming the time into the write register . If we get a request
> > from user to read time within the 1 sec period i.e before the RTC_INT_SEC
> interrupt bit is set in RTC_INT_STS, we need to give the previous loaded
> time.
> > This should be done if time is requested from user space within  1 sec
> > period after writing time, after the 1 sec  delay if user requested
> > the time , we can  give the give time from read register . This is
> > because the correct time is being updated in the read register after 1
> > sec delay. For this logic to happen we are depending on  xrtcdev-
> >time_updated  variable to get updated after the very fist RTC_INT_SEC
> interrupt occurance in the interrupt handler.
> > Since we are relaying on  xrtcdev->time_updated  to get updated from
> > interrupt handler, I think reading the RTC_INT_STS in xlnx_rtc_read_time()
> is not helpful.
> >
>
> Yeas, I understood that. But my question was whether the interrupt handling
> was necessary at all.
> Instead of waiting for an interrupt to set time_updated, can't you simply read
> RTC_INT_STS and check for the RTC_INT_SEC bit in
> xlnx_rtc_read_time() ?
>
> Something like:
>
> status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SEC)
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> else
>       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> tm);
>
> It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> updated even when it is not enabled as an interrupt.
>

The above said logic will work if we doesn't clear the RTC_INT_STS register after the
RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If interrupts
are enabled we will be clearing the RTC_INT_STS every time in the interrupt handler.
And moreover we need to return time from RTC_SET_TM_RD only if time is requested
within 1 sec span after programming the time only , so this is required only for one time.
Since we are clearing the RTC_INT_STS in our interrupt handler, we might end up in giving
the wrong time to the user when requested.So I think this logic might not work.
Please correct me if am wrong.

Thanks,
Anurag Kumar V

> > Thanks,
> > Anurag Kumar V
> >
> > > > +       xrtcdev->time_updated = 0;
> > > > +
> > > >         return 0;
> > > >  }
> > > >
> > > > @@ -85,7 +103,17 @@ static int xlnx_rtc_read_time(struct device
> > > > *dev, struct rtc_time *tm)  {
> > > >         struct xlnx_rtc_dev *xrtcdev = dev_get_drvdata(dev);
> > > >
> > > > -       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > > +       if (xrtcdev->time_updated == 0) {
> > > > +               /*
> > > > +                * Time written in SET_TIME_WRITE has not yet updated into
> > > > +                * the seconds read register, so read the time from the
> > > > +                * SET_TIME_WRITE instead of CURRENT_TIME register.
> > > > +                */
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_SET_TM_RD), tm);
> > > > +               tm->tm_sec -= 1;
> > > > +       } else {
> > > > +               rtc_time64_to_tm(readl(xrtcdev->reg_base +
> > > RTC_CUR_TM), tm);
> > > > +       }
> > > >
> > > >         return rtc_valid_tm(tm);
> > > >  }
> > > > @@ -133,6 +161,9 @@ static void xlnx_init_rtc(struct xlnx_rtc_dev
> > > > *xrtcdev)  {
> > > >         u32 rtc_ctrl;
> > > >
> > > > +       /* Enable RTC SEC interrupts */
> > > > +       writel(RTC_INT_SEC, xrtcdev->reg_base + RTC_INT_EN);
> > > > +
> > > >         /* Enable RTC switch to battery when VCC_PSAUX is not available */
> > > >         rtc_ctrl = readl(xrtcdev->reg_base + RTC_CTRL);
> > > >         rtc_ctrl |= RTC_BATT_EN;
> > > > @@ -169,8 +200,13 @@ static irqreturn_t xlnx_rtc_interrupt(int
> > > > irq, void
> > > *id)
> > > >         /* Clear interrupt */
> > > >         writel(status, xrtcdev->reg_base + RTC_INT_STS);
> > > >
> > > > -       if (status & RTC_INT_SEC)
> > > > +       if (status & RTC_INT_SEC) {
> > > > +               if (xrtcdev->time_updated == 0) {
> > > > +                       /* RTC updated the seconds read register */
> > > > +                       xrtcdev->time_updated = 1;
> > > > +               }
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> > > > +       }
> > > >         if (status & RTC_INT_ALRM)
> > > >                 rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_AF);
> > > >
> > > > --
> > > > 2.1.2
> > > >
> > >
> > > --
> > > Alexandre Belloni, Free Electrons
> > > Embedded Linux, Kernel and Android engineering
> > > http://free-electrons.com
>
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com


This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > Yeas, I understood that. But my question was whether the interrupt handling
> > was necessary at all.
> > Instead of waiting for an interrupt to set time_updated, can't you simply read
> > RTC_INT_STS and check for the RTC_INT_SEC bit in
> > xlnx_rtc_read_time() ?
> >
> > Something like:
> >
> > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SEC)
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > else
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > tm);
> >
> > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> > updated even when it is not enabled as an interrupt.
> >
> 
> The above said logic will work if we doesn't clear the RTC_INT_STS register after the
> RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If interrupts
> are enabled we will be clearing the RTC_INT_STS every time in the interrupt handler.
> And moreover we need to return time from RTC_SET_TM_RD only if time is requested
> within 1 sec span after programming the time only , so this is required only for one time.
> Since we are clearing the RTC_INT_STS in our interrupt handler, we might end up in giving
> the wrong time to the user when requested.So I think this logic might not work.
> Please correct me if am wrong.
> 

Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
handler.

You can also remove
	if (status & RTC_INT_SEC)
		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);

as it will not be used. Update interrupts are handled by the core using
timers anyway. And as you can see, there was no code enabling
RTC_INT_SEC in RTC_INT_EN before your patch.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[rtc-linux] Re: [PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > Yeas, I understood that. But my question was whether the interrupt handling
> > was necessary at all.
> > Instead of waiting for an interrupt to set time_updated, can't you simply read
> > RTC_INT_STS and check for the RTC_INT_SEC bit in
> > xlnx_rtc_read_time() ?
> >
> > Something like:
> >
> > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SEC)
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > else
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > tm);
> >
> > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> > updated even when it is not enabled as an interrupt.
> >
> 
> The above said logic will work if we doesn't clear the RTC_INT_STS register after the
> RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If interrupts
> are enabled we will be clearing the RTC_INT_STS every time in the interrupt handler.
> And moreover we need to return time from RTC_SET_TM_RD only if time is requested
> within 1 sec span after programming the time only , so this is required only for one time.
> Since we are clearing the RTC_INT_STS in our interrupt handler, we might end up in giving
> the wrong time to the user when requested.So I think this logic might not work.
> Please correct me if am wrong.
> 

Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
handler.

You can also remove
	if (status & RTC_INT_SEC)
		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);

as it will not be used. Update interrupts are handled by the core using
timers anyway. And as you can see, there was no code enabling
RTC_INT_SEC in RTC_INT_EN before your patch.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Alexandre Belloni]

On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > Yeas, I understood that. But my question was whether the interrupt handling
> > was necessary at all.
> > Instead of waiting for an interrupt to set time_updated, can't you simply read
> > RTC_INT_STS and check for the RTC_INT_SEC bit in
> > xlnx_rtc_read_time() ?
> >
> > Something like:
> >
> > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status & RTC_INT_SEC)
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > else
> >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > tm);
> >
> > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is being
> > updated even when it is not enabled as an interrupt.
> >
> 
> The above said logic will work if we doesn't clear the RTC_INT_STS register after the
> RTC_INT_SEC bit is set, this happens only if interrupts are not enabled. If interrupts
> are enabled we will be clearing the RTC_INT_STS every time in the interrupt handler.
> And moreover we need to return time from RTC_SET_TM_RD only if time is requested
> within 1 sec span after programming the time only , so this is required only for one time.
> Since we are clearing the RTC_INT_STS in our interrupt handler, we might end up in giving
> the wrong time to the user when requested.So I think this logic might not work.
> Please correct me if am wrong.
> 

Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
handler.

You can also remove
	if (status & RTC_INT_SEC)
		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);

as it will not be used. Update interrupts are handled by the core using
timers anyway. And as you can see, there was no code enabling
RTC_INT_SEC in RTC_INT_EN before your patch.

-- 
Alexandre Belloni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

Thanks for reviewing the patch, I will sent v2 with the changes updated.

Thanks,
Anurag Kumar V

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 5:32 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > > Yeas, I understood that. But my question was whether the interrupt
> > > handling was necessary at all.
> > > Instead of waiting for an interrupt to set time_updated, can't you
> > > simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
> > > xlnx_rtc_read_time() ?
> > >
> > > Something like:
> > >
> > > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status &
> RTC_INT_SEC)
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > else
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > > tm);
> > >
> > > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is
> > > being updated even when it is not enabled as an interrupt.
> > >
> >
> > The above said logic will work if we doesn't clear the RTC_INT_STS
> > register after the RTC_INT_SEC bit is set, this happens only if
> > interrupts are not enabled. If interrupts are enabled we will be clearing the
> RTC_INT_STS every time in the interrupt handler.
> > And moreover we need to return time from RTC_SET_TM_RD only if time is
> > requested within 1 sec span after programming the time only , so this is
> required only for one time.
> > Since we are clearing the RTC_INT_STS in our interrupt handler, we
> > might end up in giving the wrong time to the user when requested.So I
> think this logic might not work.
> > Please correct me if am wrong.
> >
> 
> Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
> handler.
> 
> You can also remove
> 	if (status & RTC_INT_SEC)
> 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> 
> as it will not be used. Update interrupts are handled by the core using timers
> anyway. And as you can see, there was no code enabling RTC_INT_SEC in
> RTC_INT_EN before your patch.
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com

[rtc-linux] RE: [PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

Thanks for reviewing the patch, I will sent v2 with the changes updated.

Thanks,
Anurag Kumar V

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni@free-electrons.com]
> Sent: Wednesday, April 20, 2016 5:32 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux@googlegroups.com; linux-arm-kernel@lists.infradead.org; linux-
> kernel@vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > > Yeas, I understood that. But my question was whether the interrupt
> > > handling was necessary at all.
> > > Instead of waiting for an interrupt to set time_updated, can't you
> > > simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
> > > xlnx_rtc_read_time() ?
> > >
> > > Something like:
> > >
> > > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status &
> RTC_INT_SEC)
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > else
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > > tm);
> > >
> > > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is
> > > being updated even when it is not enabled as an interrupt.
> > >
> >
> > The above said logic will work if we doesn't clear the RTC_INT_STS
> > register after the RTC_INT_SEC bit is set, this happens only if
> > interrupts are not enabled. If interrupts are enabled we will be clearing the
> RTC_INT_STS every time in the interrupt handler.
> > And moreover we need to return time from RTC_SET_TM_RD only if time is
> > requested within 1 sec span after programming the time only , so this is
> required only for one time.
> > Since we are clearing the RTC_INT_STS in our interrupt handler, we
> > might end up in giving the wrong time to the user when requested.So I
> think this logic might not work.
> > Please correct me if am wrong.
> >
> 
> Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
> handler.
> 
> You can also remove
> 	if (status & RTC_INT_SEC)
> 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> 
> as it will not be used. Update interrupts are handled by the core using timers
> anyway. And as you can see, there was no code enabling RTC_INT_SEC in
> RTC_INT_EN before your patch.
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com

-- 
-- 
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
--- 
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[PATCH 3/3] RTC: Update seconds time programming logic

[Anurag Kumar Vulisha]

Hi Alexandre,

Thanks for reviewing the patch, I will sent v2 with the changes updated.

Thanks,
Anurag Kumar V

> -----Original Message-----
> From: Alexandre Belloni [mailto:alexandre.belloni at free-electrons.com]
> Sent: Wednesday, April 20, 2016 5:32 PM
> To: Anurag Kumar Vulisha <anuragku@xilinx.com>
> Cc: Alessandro Zummo <a.zummo@towertech.it>; Soren Brinkmann
> <sorenb@xilinx.com>; Michal Simek <michals@xilinx.com>; rtc-
> linux at googlegroups.com; linux-arm-kernel at lists.infradead.org; linux-
> kernel at vger.kernel.org; Punnaiah Choudary Kalluri <punnaia@xilinx.com>;
> Anirudha Sarangi <anirudh@xilinx.com>; Srikanth Vemula
> <svemula@xilinx.com>; Srinivas Goud <sgoud@xilinx.com>
> Subject: Re: [PATCH 3/3] RTC: Update seconds time programming logic
> 
> On 20/04/2016 at 10:31:06 +0000, Anurag Kumar Vulisha wrote :
> > > Yeas, I understood that. But my question was whether the interrupt
> > > handling was necessary at all.
> > > Instead of waiting for an interrupt to set time_updated, can't you
> > > simply read RTC_INT_STS and check for the RTC_INT_SEC bit in
> > > xlnx_rtc_read_time() ?
> > >
> > > Something like:
> > >
> > > status = readl(xrtcdev->reg_base + RTC_INT_STS) if (status &
> RTC_INT_SEC)
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_CUR_TM), tm);
> > > else
> > >       rtc_time64_to_tm(readl(xrtcdev->reg_base + RTC_SET_TM_RD) - 1,
> > > tm);
> > >
> > > It all depends on whether the RTC_INT_SEC bit in RTC_INT_STS is
> > > being updated even when it is not enabled as an interrupt.
> > >
> >
> > The above said logic will work if we doesn't clear the RTC_INT_STS
> > register after the RTC_INT_SEC bit is set, this happens only if
> > interrupts are not enabled. If interrupts are enabled we will be clearing the
> RTC_INT_STS every time in the interrupt handler.
> > And moreover we need to return time from RTC_SET_TM_RD only if time is
> > requested within 1 sec span after programming the time only , so this is
> required only for one time.
> > Since we are clearing the RTC_INT_STS in our interrupt handler, we
> > might end up in giving the wrong time to the user when requested.So I
> think this logic might not work.
> > Please correct me if am wrong.
> >
> 
> Simply stop clearing RTC_INT_SEC from RTC_INT_STS in the interrupt
> handler.
> 
> You can also remove
> 	if (status & RTC_INT_SEC)
> 		rtc_update_irq(xrtcdev->rtc, 1, RTC_IRQF | RTC_UF);
> 
> as it will not be used. Update interrupts are handled by the core using timers
> anyway. And as you can see, there was no code enabling RTC_INT_SEC in
> RTC_INT_EN before your patch.
> 
> --
> Alexandre Belloni, Free Electrons
> Embedded Linux, Kernel and Android engineering http://free-electrons.com