[PATCH 0/4] RTC: Updates for RTC.

[PATCH 0/4] RTC: Updates for RTC.

[Marcelo Roberto Jimenez]

The following set of patches are updates to the RTC code to take into account
the new emulated periodic interrupts using hrtimers.

[PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.
[PATCH 2/4] RTC: Remove UIE and PIE information from the sa1100 driver proc.
[PATCH 3/4] RTC: Fix the cross interrupt issue on rtc-test.
[PATCH 4/4] RTC: sa1100: Update the sa1100 RTC driver.

 drivers/rtc/rtc-proc.c   |   22 ++++++---
 drivers/rtc/rtc-sa1100.c |  122 ++--------------------------------------------
 drivers/rtc/rtc-test.c   |   13 +++--
 3 files changed, 29 insertions(+), 128 deletions(-)

[PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.

[Marcelo Roberto Jimenez]

Generic RTC code is always able to provide the necessary information
about update and periodic interrupts. This patch add such information to
the proc interface.

Signed-off-by: Marcelo Roberto Jimenez <mroberto@cpti.cetuc.puc-rio.br>
---
 drivers/rtc/rtc-proc.c |   22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/drivers/rtc/rtc-proc.c b/drivers/rtc/rtc-proc.c
index c086fc3..b4ecec0 100644
--- a/drivers/rtc/rtc-proc.c
+++ b/drivers/rtc/rtc-proc.c
@@ -30,15 +30,15 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
 	err = rtc_read_time(rtc, &tm);
 	if (err == 0) {
 		seq_printf(seq,
-			"rtc_time\t: %02d:%02d:%02d\n"
-			"rtc_date\t: %04d-%02d-%02d\n",
+			"rtc_time\t\t: %02d:%02d:%02d\n"
+			"rtc_date\t\t: %04d-%02d-%02d\n",
 			tm.tm_hour, tm.tm_min, tm.tm_sec,
 			tm.tm_year + 1900, tm.tm_mon + 1, tm.tm_mday);
 	}
 
 	err = rtc_read_alarm(rtc, &alrm);
 	if (err == 0) {
-		seq_printf(seq, "alrm_time\t: ");
+		seq_printf(seq, "alrm_time\t\t: ");
 		if ((unsigned int)alrm.time.tm_hour <= 24)
 			seq_printf(seq, "%02d:", alrm.time.tm_hour);
 		else
@@ -52,7 +52,7 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
 		else
 			seq_printf(seq, "**\n");
 
-		seq_printf(seq, "alrm_date\t: ");
+		seq_printf(seq, "alrm_date\t\t: ");
 		if ((unsigned int)alrm.time.tm_year <= 200)
 			seq_printf(seq, "%04d-", alrm.time.tm_year + 1900);
 		else
@@ -65,13 +65,21 @@ static int rtc_proc_show(struct seq_file *seq, void *offset)
 			seq_printf(seq, "%02d\n", alrm.time.tm_mday);
 		else
 			seq_printf(seq, "**\n");
-		seq_printf(seq, "alarm_IRQ\t: %s\n",
+		seq_printf(seq, "alarm IRQ enabled\t: %s\n",
 				alrm.enabled ? "yes" : "no");
-		seq_printf(seq, "alrm_pending\t: %s\n",
+		seq_printf(seq, "alarm pending\t\t: %s\n",
 				alrm.pending ? "yes" : "no");
+		seq_printf(seq, "update IRQ enabled\t: %s\n",
+			(rtc->uie_rtctimer.enabled) ? "yes" : "no");
+		seq_printf(seq, "periodic IRQ enabled\t: %s\n",
+			(rtc->pie_enabled) ? "yes" : "no");
+		seq_printf(seq, "periodic IRQ frequency\t: %d\n",
+			rtc->irq_freq);
+		seq_printf(seq, "max user IRQ frequency\t: %d\n",
+			rtc->max_user_freq);
 	}
 
-	seq_printf(seq, "24hr\t\t: yes\n");
+	seq_printf(seq, "24hr\t\t\t: yes\n");
 
 	if (ops->proc)
 		ops->proc(rtc->dev.parent, seq);
-- 
1.7.3.4

Re: [PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.

[John Stultz]

On Mon, 2011-02-07 at 19:16 -0200, Marcelo Roberto Jimenez wrote:
> Generic RTC code is always able to provide the necessary information
> about update and periodic interrupts. This patch add such information to
> the proc interface.

This seems to be mostly formatting changes. Are you sure we wont' break
applications expecting the existing format?

thanks
-john

Re: [PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.

[Marcelo Roberto Jimenez]

Hi John,

On Thu, Feb 10, 2011 at 21:06, John Stultz <john.stultz@linaro.org> wrote:
>
> This seems to be mostly formatting changes. Are you sure we wont' break
> applications expecting the existing format?

No, I can't be 100% sure, that will depend upon how the application
parses the proc output. The formatting changes are all white space, so
I could remove them. If the value is read using scanf, white spaces
are skipped and the "\t" that were added to make the output pleasant
to the view are not a problem. That part can be reworked if you think
the breakage is serious, it is not really important.

The important thing in the patch are the information additions to the
proc interface: update IRQ enabled, periodic IRQ enabled, periodic IRQ
frequency and max user IRQ frequency. What do you think about these?

> thanks
> -john

Regards,
Marcelo.

[PATCH 2/4] RTC: Remove UIE and PIE information from the sa1100 driver proc.

[Marcelo Roberto Jimenez]

This patch removes the UIE and PIE information that is now being
supplied directly in the generic RTC code.

Signed-off-by: Marcelo Roberto Jimenez <mroberto@cpti.cetuc.puc-rio.br>
---
 drivers/rtc/rtc-sa1100.c |   11 ++---------
 1 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index df1804f..6c8c74a 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -358,15 +358,8 @@ static int sa1100_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static int sa1100_rtc_proc(struct device *dev, struct seq_file *seq)
 {
-	struct rtc_device *rtc = (struct rtc_device *)dev;
-
-	seq_printf(seq, "trim/divider\t: 0x%08x\n", (u32) RTTR);
-	seq_printf(seq, "update_IRQ\t: %s\n",
-			(RTSR & RTSR_HZE) ? "yes" : "no");
-	seq_printf(seq, "periodic_IRQ\t: %s\n",
-			(OIER & OIER_E1) ? "yes" : "no");
-	seq_printf(seq, "periodic_freq\t: %d\n", rtc->irq_freq);
-	seq_printf(seq, "RTSR\t\t: 0x%08x\n", (u32)RTSR);
+	seq_printf(seq, "trim/divider\t\t: 0x%08x\n", (u32) RTTR);
+	seq_printf(seq, "RTSR\t\t\t: 0x%08x\n", (u32)RTSR);
 
 	return 0;
 }
-- 
1.7.3.4

[PATCH 3/4] RTC: Fix the cross interrupt issue on rtc-test.

[Marcelo Roberto Jimenez]

The rtc-test driver is meant to provide a test/debug code for the RTC
subsystem.

The rtc-test driver simulates specific interrupts by echoing to the
sys interface. Those were the update, alarm and periodic interrupts.

As a side effect of the new implementation, any interrupt generated in
the rtc-test driver would trigger the same code path in the generic
code, and thus the distinction among interrupts gets lost.

This patch preserves the previous behaviour of the rtc-test driver,
where e.g. an update interrupt would not trigger an alarm or periodic
interrupt, and vice-versa. In real world RTC drivers, this is not an
issue, but in the rtc-test driver it may be interesting to distinguish
these interrupts for testing purposes.

Signed-off-by: Marcelo Roberto Jimenez <mroberto@cpti.cetuc.puc-rio.br>
---
 drivers/rtc/rtc-test.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/rtc-test.c b/drivers/rtc/rtc-test.c
index 61ad5ab..34d5631 100644
--- a/drivers/rtc/rtc-test.c
+++ b/drivers/rtc/rtc-test.c
@@ -71,11 +71,16 @@ static ssize_t test_irq_store(struct device *dev,
 	struct rtc_device *rtc = platform_get_drvdata(plat_dev);
 
 	retval = count;
-	if (strncmp(buf, "tick", 4) == 0)
+	if (strncmp(buf, "tick", 4) == 0 && rtc->pie_enabled)
 		rtc_update_irq(rtc, 1, RTC_PF | RTC_IRQF);
-	else if (strncmp(buf, "alarm", 5) == 0)
-		rtc_update_irq(rtc, 1, RTC_AF | RTC_IRQF);
-	else if (strncmp(buf, "update", 6) == 0)
+	else if (strncmp(buf, "alarm", 5) == 0) {
+		struct rtc_wkalrm alrm;
+		int err = rtc_read_alarm(rtc, &alrm);
+
+		if (!err && alrm.enabled)
+			rtc_update_irq(rtc, 1, RTC_AF | RTC_IRQF);
+
+	} else if (strncmp(buf, "update", 6) == 0 && rtc->uie_rtctimer.enabled)
 		rtc_update_irq(rtc, 1, RTC_UF | RTC_IRQF);
 	else
 		retval = -EINVAL;
-- 
1.7.3.4

[PATCH 4/4] RTC: sa1100: Update the sa1100 RTC driver.

[Marcelo Roberto Jimenez]

Since PIE interrupts are now emulated, this patch removes the previous
code that used the hardware counters.

The removal of read_callback() also fixes a wrong user space behaviour
of this driver, which was not returning the right value to read().

Signed-off-by: Marcelo Roberto Jimenez <mroberto@cpti.cetuc.puc-rio.br>
---
 drivers/rtc/rtc-sa1100.c |  111 +--------------------------------------------
 1 files changed, 3 insertions(+), 108 deletions(-)

diff --git a/drivers/rtc/rtc-sa1100.c b/drivers/rtc/rtc-sa1100.c
index 6c8c74a..1ff0fac 100644
--- a/drivers/rtc/rtc-sa1100.c
+++ b/drivers/rtc/rtc-sa1100.c
@@ -43,7 +43,6 @@
 #define RTC_DEF_TRIM		0
 
 static const unsigned long RTC_FREQ = 1024;
-static unsigned long timer_freq;
 static struct rtc_time rtc_alarm;
 static DEFINE_SPINLOCK(sa1100_rtc_lock);
 
@@ -156,97 +155,11 @@ static irqreturn_t sa1100_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
-static int sa1100_irq_set_freq(struct device *dev, int freq)
-{
-	if (freq < 1 || freq > timer_freq) {
-		return -EINVAL;
-	} else {
-		struct rtc_device *rtc = (struct rtc_device *)dev;
-
-		rtc->irq_freq = freq;
-
-		return 0;
-	}
-}
-
-static int rtc_timer1_count;
-
-static inline int sa1100_timer1_retrigger(struct rtc_device *rtc)
-{
-	unsigned long diff;
-	unsigned long period = timer_freq / rtc->irq_freq;
-
-	spin_lock_irq(&sa1100_rtc_lock);
-
-	do {
-		OSMR1 += period;
-		diff = OSMR1 - OSCR;
-		/* If OSCR > OSMR1, diff is a very large number (unsigned
-		 * math). This means we have a lost interrupt. */
-	} while (diff > period);
-	OIER |= OIER_E1;
-
-	spin_unlock_irq(&sa1100_rtc_lock);
-
-	return 0;
-}
-
-static irqreturn_t timer1_interrupt(int irq, void *dev_id)
-{
-	struct platform_device *pdev = to_platform_device(dev_id);
-	struct rtc_device *rtc = platform_get_drvdata(pdev);
-
-	/*
-	 * If we match for the first time, rtc_timer1_count will be 1.
-	 * Otherwise, we wrapped around (very unlikely but
-	 * still possible) so compute the amount of missed periods.
-	 * The match reg is updated only when the data is actually retrieved
-	 * to avoid unnecessary interrupts.
-	 */
-	OSSR = OSSR_M1;	/* clear match on timer1 */
-
-	rtc_update_irq(rtc, rtc_timer1_count, RTC_PF | RTC_IRQF);
-
-	if (rtc_timer1_count == 1)
-		rtc_timer1_count =
-			(rtc->irq_freq * ((1 << 30) / (timer_freq >> 2)));
-
-	/* retrigger. */
-	sa1100_timer1_retrigger(rtc);
-
-	return IRQ_HANDLED;
-}
-
-static int sa1100_rtc_read_callback(struct device *dev, int data)
-{
-	if (data & RTC_PF) {
-		struct rtc_device *rtc = (struct rtc_device *)dev;
-
-		/* interpolate missed periods and set match for the next */
-		unsigned long period = timer_freq / rtc->irq_freq;
-		unsigned long oscr = OSCR;
-		unsigned long osmr1 = OSMR1;
-		unsigned long missed = (oscr - osmr1)/period;
-		data += missed << 8;
-		OSSR = OSSR_M1;	/* clear match on timer 1 */
-		OSMR1 = osmr1 + (missed + 1)*period;
-		/* Ensure we didn't miss another match in the mean time.
-		 * Here we compare (match - OSCR) 8 instead of 0 --
-		 * see comment in pxa_timer_interrupt() for explanation.
-		 */
-		while ((signed long)((osmr1 = OSMR1) - OSCR) <= 8) {
-			data += 0x100;
-			OSSR = OSSR_M1;	/* clear match on timer 1 */
-			OSMR1 = osmr1 + period;
-		}
-	}
-	return data;
-}
-
 static int sa1100_rtc_open(struct device *dev)
 {
 	int ret;
-	struct rtc_device *rtc = (struct rtc_device *)dev;
+	struct platform_device *plat_dev = to_platform_device(dev);
+	struct rtc_device *rtc = platform_get_drvdata(plat_dev);
 
 	ret = request_irq(IRQ_RTC1Hz, sa1100_rtc_interrupt, IRQF_DISABLED,
 		"rtc 1Hz", dev);
@@ -260,19 +173,11 @@ static int sa1100_rtc_open(struct device *dev)
 		dev_err(dev, "IRQ %d already in use.\n", IRQ_RTCAlrm);
 		goto fail_ai;
 	}
-	ret = request_irq(IRQ_OST1, timer1_interrupt, IRQF_DISABLED,
-		"rtc timer", dev);
-	if (ret) {
-		dev_err(dev, "IRQ %d already in use.\n", IRQ_OST1);
-		goto fail_pi;
-	}
 	rtc->max_user_freq = RTC_FREQ;
-	sa1100_irq_set_freq(dev, RTC_FREQ);
+	rtc_irq_set_freq(rtc, NULL, RTC_FREQ);
 
 	return 0;
 
- fail_pi:
-	free_irq(IRQ_RTCAlrm, dev);
  fail_ai:
 	free_irq(IRQ_RTC1Hz, dev);
  fail_ui:
@@ -287,12 +192,10 @@ static void sa1100_rtc_release(struct device *dev)
 	OSSR = OSSR_M1;
 	spin_unlock_irq(&sa1100_rtc_lock);
 
-	free_irq(IRQ_OST1, dev);
 	free_irq(IRQ_RTCAlrm, dev);
 	free_irq(IRQ_RTC1Hz, dev);
 }
 
-
 static int sa1100_rtc_ioctl(struct device *dev, unsigned int cmd,
 		unsigned long arg)
 {
@@ -366,7 +269,6 @@ static int sa1100_rtc_proc(struct device *dev, struct seq_file *seq)
 
 static const struct rtc_class_ops sa1100_rtc_ops = {
 	.open = sa1100_rtc_open,
-	.read_callback = sa1100_rtc_read_callback,
 	.release = sa1100_rtc_release,
 	.ioctl = sa1100_rtc_ioctl,
 	.read_time = sa1100_rtc_read_time,
@@ -380,8 +282,6 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 {
 	struct rtc_device *rtc;
 
-	timer_freq = get_clock_tick_rate();
-
 	/*
 	 * According to the manual we should be able to let RTTR be zero
 	 * and then a default diviser for a 32.768KHz clock is used.
@@ -407,11 +307,6 @@ static int sa1100_rtc_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, rtc);
 
-	/* Set the irq_freq */
-	/*TODO: Find out who is messing with this value after we initialize
-	 * it here.*/
-	rtc->irq_freq = RTC_FREQ;
-
 	/* Fix for a nasty initialization problem the in SA11xx RTSR register.
 	 * See also the comments in sa1100_rtc_interrupt().
 	 *
-- 
1.7.3.4

Re: [PATCH 0/4] RTC: Updates for RTC.

[Russell King - ARM Linux]

I think it would be more relevant for you to send your patches _to_
Alessandro Zummo rather than to me, as I have much interest in this,
and it's Alessandro who you want to merge it.  Alessandro is the RTC
maintainer.

On Mon, Feb 07, 2011 at 07:16:04PM -0200, Marcelo Roberto Jimenez wrote:
> The following set of patches are updates to the RTC code to take into account
> the new emulated periodic interrupts using hrtimers.
> 
> [PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.
> [PATCH 2/4] RTC: Remove UIE and PIE information from the sa1100 driver proc.
> [PATCH 3/4] RTC: Fix the cross interrupt issue on rtc-test.
> [PATCH 4/4] RTC: sa1100: Update the sa1100 RTC driver.
> 
>  drivers/rtc/rtc-proc.c   |   22 ++++++---
>  drivers/rtc/rtc-sa1100.c |  122 ++--------------------------------------------
>  drivers/rtc/rtc-test.c   |   13 +++--
>  3 files changed, 29 insertions(+), 128 deletions(-)
> 

Re: [PATCH 0/4] RTC: Updates for RTC.

[John Stultz]

On Mon, 2011-02-07 at 19:16 -0200, Marcelo Roberto Jimenez wrote:
> The following set of patches are updates to the RTC code to take into account
> the new emulated periodic interrupts using hrtimers.
> 
> [PATCH 1/4] RTC: Include information about UIE and PIE in RTC driver proc.
> [PATCH 2/4] RTC: Remove UIE and PIE information from the sa1100 driver proc.
> [PATCH 3/4] RTC: Fix the cross interrupt issue on rtc-test.
> [PATCH 4/4] RTC: sa1100: Update the sa1100 RTC driver.

The first one worries me a little bit (see earlier reply), but the
others look ok to me. The last one collided with my RTC tree, but I
think I've fixed it up. You can find the tree here:

git://git.linaro.org/people/jstultz/linux.git dev/rtc-cleanups

Would you mind testing that branch and letting me know if anything
breaks for you?

thanks
-john

Re: [rtc-linux] Re: [PATCH 0/4] RTC: Updates for RTC.

[Marcelo Roberto Jimenez]

Hi John,

On Thu, Feb 10, 2011 at 21:18, John Stultz <john.stultz@linaro.org> wrote:
> The first one worries me a little bit (see earlier reply), but the
> others look ok to me. The last one collided with my RTC tree, but I
> think I've fixed it up. You can find the tree here:
>
> git://git.linaro.org/people/jstultz/linux.git dev/rtc-cleanups
>
> Would you mind testing that branch and letting me know if anything
> breaks for you?

Sure, I was already testing using your branch, I don't know what
caused the collision.

I just tested it and my tests work fine in the rtc-cleanups branch.

> thanks
> -john

Regards,
Marcelo.