[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Johan Hovold]

This is an update of the shadow-interrupt-mask series against v3.10-rc2.

I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
affected. If not, then some probing mechanism as the one Doug suggested
could be implemented on top of (a subset of) these patches. What do you
say, Nicolas?

Note that the first patch (adding a missing OF compile guard) could be
applied straight away.

Thanks,
Johan


v3:
 - rebase against v3.10-rc2
 - remove some comments

v2:
 - rebase on top of DT-support patch by Joachim Eastwood
 - add missing brace in DT-id table


Johan Hovold (5):
  rtc-at91rm9200: add match-table compile guard
  rtc-at91rm9200: add configuration support
  rtc-at91rm9200: refactor interrupt-register handling
  rtc-at91rm9200: add shadow interrupt mask
  rtc-at91rm9200: use shadow IMR on at91sam9x5

 .../bindings/rtc/atmel,at91rm9200-rtc.txt          |   2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi                  |   2 +-
 drivers/rtc/rtc-at91rm9200.c                       | 131 +++++++++++++++++----
 3 files changed, 113 insertions(+), 22 deletions(-)

-- 
1.8.2.1

[PATCH v3 1/5] rtc-at91rm9200: add match-table compile guard

[Johan Hovold]

Add missing match-table compile guard.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/rtc/rtc-at91rm9200.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 0eab77b..eeeb73f 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -383,11 +383,13 @@ static int at91_rtc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(at91_rtc_pm_ops, at91_rtc_suspend, at91_rtc_resume);
 
+#ifdef CONFIG_OF
 static const struct of_device_id at91_rtc_dt_ids[] = {
 	{ .compatible = "atmel,at91rm9200-rtc" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+#endif
 
 static struct platform_driver at91_rtc_driver = {
 	.remove		= __exit_p(at91_rtc_remove),
-- 
1.8.2.1

[PATCH v3 2/5] rtc-at91rm9200: add configuration support

[Johan Hovold]

Add configuration support which can be used to implement SoC-specific
workarounds for broken hardware.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/rtc/rtc-at91rm9200.c | 46 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index eeeb73f..ab2024b 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -42,6 +42,10 @@
 
 #define AT91_RTC_EPOCH		1900UL	/* just like arch/arm/common/rtctime.c */
 
+struct at91_rtc_config {
+};
+
+static const struct at91_rtc_config *at91_rtc_config;
 static DECLARE_COMPLETION(at91_rtc_updated);
 static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
 static void __iomem *at91_rtc_regs;
@@ -250,6 +254,36 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
 	return IRQ_NONE;		/* not handled */
 }
 
+static const struct at91_rtc_config at91rm9200_config = {
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id at91_rtc_dt_ids[] = {
+	{
+		.compatible = "atmel,at91rm9200-rtc",
+		.data = &at91rm9200_config,
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
+#endif
+
+static const struct at91_rtc_config *
+at91_rtc_get_config(struct platform_device *pdev)
+{
+	const struct of_device_id *match;
+
+	if (pdev->dev.of_node) {
+		match = of_match_node(at91_rtc_dt_ids, pdev->dev.of_node);
+		if (!match)
+			return NULL;
+		return (const struct at91_rtc_config *)match->data;
+	}
+
+	return &at91rm9200_config;
+}
+
 static const struct rtc_class_ops at91_rtc_ops = {
 	.read_time	= at91_rtc_readtime,
 	.set_time	= at91_rtc_settime,
@@ -268,6 +302,10 @@ static int __init at91_rtc_probe(struct platform_device *pdev)
 	struct resource *regs;
 	int ret = 0;
 
+	at91_rtc_config = at91_rtc_get_config(pdev);
+	if (!at91_rtc_config)
+		return -ENODEV;
+
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	if (!regs) {
 		dev_err(&pdev->dev, "no mmio resource defined\n");
@@ -383,14 +421,6 @@ static int at91_rtc_resume(struct device *dev)
 
 static SIMPLE_DEV_PM_OPS(at91_rtc_pm_ops, at91_rtc_suspend, at91_rtc_resume);
 
-#ifdef CONFIG_OF
-static const struct of_device_id at91_rtc_dt_ids[] = {
-	{ .compatible = "atmel,at91rm9200-rtc" },
-	{ /* sentinel */ }
-};
-MODULE_DEVICE_TABLE(of, at91_rtc_dt_ids);
-#endif
-
 static struct platform_driver at91_rtc_driver = {
 	.remove		= __exit_p(at91_rtc_remove),
 	.driver		= {
-- 
1.8.2.1

[PATCH v3 3/5] rtc-at91rm9200: refactor interrupt-register handling

[Johan Hovold]

Add accessors for the interrupt register.

This will allow us to easily add a shadow interrupt-mask register to
use on SoCs where the interrupt-mask register cannot be used.

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/rtc/rtc-at91rm9200.c | 43 +++++++++++++++++++++++++++++--------------
 1 file changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index ab2024b..9592d08 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -51,6 +51,21 @@ static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
 static void __iomem *at91_rtc_regs;
 static int irq;
 
+static void at91_rtc_write_ier(u32 mask)
+{
+	at91_rtc_write(AT91_RTC_IER, mask);
+}
+
+static void at91_rtc_write_idr(u32 mask)
+{
+	at91_rtc_write(AT91_RTC_IDR, mask);
+}
+
+static u32 at91_rtc_read_imr(void)
+{
+	return at91_rtc_read(AT91_RTC_IMR);
+}
+
 /*
  * Decode time/date into rtc_time structure
  */
@@ -114,9 +129,9 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm)
 	cr = at91_rtc_read(AT91_RTC_CR);
 	at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM);
 
-	at91_rtc_write(AT91_RTC_IER, AT91_RTC_ACKUPD);
+	at91_rtc_write_ier(AT91_RTC_ACKUPD);
 	wait_for_completion(&at91_rtc_updated);	/* wait for ACKUPD interrupt */
-	at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD);
+	at91_rtc_write_idr(AT91_RTC_ACKUPD);
 
 	at91_rtc_write(AT91_RTC_TIMR,
 			  bin2bcd(tm->tm_sec) << 0
@@ -148,7 +163,7 @@ static int at91_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	tm->tm_yday = rtc_year_days(tm->tm_mday, tm->tm_mon, tm->tm_year);
 	tm->tm_year = at91_alarm_year - 1900;
 
-	alrm->enabled = (at91_rtc_read(AT91_RTC_IMR) & AT91_RTC_ALARM)
+	alrm->enabled = (at91_rtc_read_imr() & AT91_RTC_ALARM)
 			? 1 : 0;
 
 	dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -173,7 +188,7 @@ static int at91_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 	tm.tm_min = alrm->time.tm_min;
 	tm.tm_sec = alrm->time.tm_sec;
 
-	at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+	at91_rtc_write_idr(AT91_RTC_ALARM);
 	at91_rtc_write(AT91_RTC_TIMALR,
 		  bin2bcd(tm.tm_sec) << 0
 		| bin2bcd(tm.tm_min) << 8
@@ -186,7 +201,7 @@ static int at91_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 	if (alrm->enabled) {
 		at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
-		at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+		at91_rtc_write_ier(AT91_RTC_ALARM);
 	}
 
 	dev_dbg(dev, "%s(): %4d-%02d-%02d %02d:%02d:%02d\n", __func__,
@@ -202,9 +217,9 @@ static int at91_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
 
 	if (enabled) {
 		at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_ALARM);
-		at91_rtc_write(AT91_RTC_IER, AT91_RTC_ALARM);
+		at91_rtc_write_ier(AT91_RTC_ALARM);
 	} else
-		at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ALARM);
+		at91_rtc_write_idr(AT91_RTC_ALARM);
 
 	return 0;
 }
@@ -213,7 +228,7 @@ static int at91_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
  */
 static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
 {
-	unsigned long imr = at91_rtc_read(AT91_RTC_IMR);
+	unsigned long imr = at91_rtc_read_imr();
 
 	seq_printf(seq, "update_IRQ\t: %s\n",
 			(imr & AT91_RTC_ACKUPD) ? "yes" : "no");
@@ -233,7 +248,7 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
 	unsigned int rtsr;
 	unsigned long events = 0;
 
-	rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read(AT91_RTC_IMR);
+	rtsr = at91_rtc_read(AT91_RTC_SR) & at91_rtc_read_imr();
 	if (rtsr) {		/* this interrupt is shared!  Is it ours? */
 		if (rtsr & AT91_RTC_ALARM)
 			events |= (RTC_AF | RTC_IRQF);
@@ -328,7 +343,7 @@ static int __init at91_rtc_probe(struct platform_device *pdev)
 	at91_rtc_write(AT91_RTC_MR, 0);		/* 24 hour mode */
 
 	/* Disable all interrupts */
-	at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+	at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
 					AT91_RTC_SECEV | AT91_RTC_TIMEV |
 					AT91_RTC_CALEV);
 
@@ -373,7 +388,7 @@ static int __exit at91_rtc_remove(struct platform_device *pdev)
 	struct rtc_device *rtc = platform_get_drvdata(pdev);
 
 	/* Disable all interrupts */
-	at91_rtc_write(AT91_RTC_IDR, AT91_RTC_ACKUPD | AT91_RTC_ALARM |
+	at91_rtc_write_idr(AT91_RTC_ACKUPD | AT91_RTC_ALARM |
 					AT91_RTC_SECEV | AT91_RTC_TIMEV |
 					AT91_RTC_CALEV);
 	free_irq(irq, pdev);
@@ -396,13 +411,13 @@ static int at91_rtc_suspend(struct device *dev)
 	/* this IRQ is shared with DBGU and other hardware which isn't
 	 * necessarily doing PM like we are...
 	 */
-	at91_rtc_imr = at91_rtc_read(AT91_RTC_IMR)
+	at91_rtc_imr = at91_rtc_read_imr()
 			& (AT91_RTC_ALARM|AT91_RTC_SECEV);
 	if (at91_rtc_imr) {
 		if (device_may_wakeup(dev))
 			enable_irq_wake(irq);
 		else
-			at91_rtc_write(AT91_RTC_IDR, at91_rtc_imr);
+			at91_rtc_write_idr(at91_rtc_imr);
 	}
 	return 0;
 }
@@ -413,7 +428,7 @@ static int at91_rtc_resume(struct device *dev)
 		if (device_may_wakeup(dev))
 			disable_irq_wake(irq);
 		else
-			at91_rtc_write(AT91_RTC_IER, at91_rtc_imr);
+			at91_rtc_write_ier(at91_rtc_imr);
 	}
 	return 0;
 }
-- 
1.8.2.1

[PATCH v3 4/5] rtc-at91rm9200: add shadow interrupt mask

[Johan Hovold]

Add shadow interrupt-mask register which can be used on SoCs where the
actual hardware register is broken.

Note that some care needs to be taken to make sure the shadow mask
corresponds to the actual hardware state. The added overhead is not an
issue for the non-broken SoCs due to the relatively infrequent
interrupt-mask updates. We do, however, only use the shadow mask value
as a fall-back when it actually needed as there is still a theoretical
possibility that the mask is incorrect (see the code for details).

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 drivers/rtc/rtc-at91rm9200.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 9592d08..205701e 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -30,6 +30,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/spinlock.h>
 
 #include <asm/uaccess.h>
 
@@ -43,6 +44,7 @@
 #define AT91_RTC_EPOCH		1900UL	/* just like arch/arm/common/rtctime.c */
 
 struct at91_rtc_config {
+	bool use_shadow_imr;
 };
 
 static const struct at91_rtc_config *at91_rtc_config;
@@ -50,20 +52,55 @@ static DECLARE_COMPLETION(at91_rtc_updated);
 static unsigned int at91_alarm_year = AT91_RTC_EPOCH;
 static void __iomem *at91_rtc_regs;
 static int irq;
+static DEFINE_SPINLOCK(at91_rtc_lock);
+static u32 at91_rtc_shadow_imr;
 
 static void at91_rtc_write_ier(u32 mask)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&at91_rtc_lock, flags);
+	at91_rtc_shadow_imr |= mask;
 	at91_rtc_write(AT91_RTC_IER, mask);
+	spin_unlock_irqrestore(&at91_rtc_lock, flags);
 }
 
 static void at91_rtc_write_idr(u32 mask)
 {
+	unsigned long flags;
+
+	spin_lock_irqsave(&at91_rtc_lock, flags);
 	at91_rtc_write(AT91_RTC_IDR, mask);
+	/*
+	 * Register read back (of any RTC-register) needed to make sure
+	 * IDR-register write has reached the peripheral before updating
+	 * shadow mask.
+	 *
+	 * Note that there is still a possibility that the mask is updated
+	 * before interrupts have actually been disabled in hardware. The only
+	 * way to be certain would be to poll the IMR-register, which is is
+	 * the very register we are trying to emulate. The register read back
+	 * is a reasonable heuristic.
+	 */
+	at91_rtc_read(AT91_RTC_SR);
+	at91_rtc_shadow_imr &= ~mask;
+	spin_unlock_irqrestore(&at91_rtc_lock, flags);
 }
 
 static u32 at91_rtc_read_imr(void)
 {
-	return at91_rtc_read(AT91_RTC_IMR);
+	unsigned long flags;
+	u32 mask;
+
+	if (at91_rtc_config->use_shadow_imr) {
+		spin_lock_irqsave(&at91_rtc_lock, flags);
+		mask = at91_rtc_shadow_imr;
+		spin_unlock_irqrestore(&at91_rtc_lock, flags);
+	} else {
+		mask = at91_rtc_read(AT91_RTC_IMR);
+	}
+
+	return mask;
 }
 
 /*
-- 
1.8.2.1

[PATCH v3 5/5] rtc-at91rm9200: use shadow IMR on at91sam9x5

[Johan Hovold]

Add support for the at91sam9x5-family which must use the shadow
interrupt mask due to a hardware issue (causing RTC_IMR to always be
zero).

Signed-off-by: Johan Hovold <jhovold@gmail.com>
---
 Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt | 2 +-
 arch/arm/boot/dts/at91sam9x5.dtsi                              | 2 +-
 drivers/rtc/rtc-at91rm9200.c                                   | 7 +++++++
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
index 2a3feab..34c1505 100644
--- a/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
+++ b/Documentation/devicetree/bindings/rtc/atmel,at91rm9200-rtc.txt
@@ -1,7 +1,7 @@
 Atmel AT91RM9200 Real Time Clock
 
 Required properties:
-- compatible: should be: "atmel,at91rm9200-rtc"
+- compatible: should be: "atmel,at91rm9200-rtc" or "atmel,at91sam9x5-rtc"
 - reg: physical base address of the controller and length of memory mapped
   region.
 - interrupts: rtc alarm/event interrupt
diff --git a/arch/arm/boot/dts/at91sam9x5.dtsi b/arch/arm/boot/dts/at91sam9x5.dtsi
index 1145ac3..b5833d1f 100644
--- a/arch/arm/boot/dts/at91sam9x5.dtsi
+++ b/arch/arm/boot/dts/at91sam9x5.dtsi
@@ -643,7 +643,7 @@
 			};
 
 			rtc at fffffeb0 {
-				compatible = "atmel,at91rm9200-rtc";
+				compatible = "atmel,at91sam9x5-rtc";
 				reg = <0xfffffeb0 0x40>;
 				interrupts = <1 4 7>;
 				status = "disabled";
diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 205701e..47416e9 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -309,12 +309,19 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
 static const struct at91_rtc_config at91rm9200_config = {
 };
 
+static const struct at91_rtc_config at91sam9x5_config = {
+	.use_shadow_imr	= true,
+};
+
 #ifdef CONFIG_OF
 static const struct of_device_id at91_rtc_dt_ids[] = {
 	{
 		.compatible = "atmel,at91rm9200-rtc",
 		.data = &at91rm9200_config,
 	}, {
+		.compatible = "atmel,at91sam9x5-rtc",
+		.data = &at91sam9x5_config,
+	}, {
 		/* sentinel */
 	}
 };
-- 
1.8.2.1

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Andrew Morton]

On Thu, 23 May 2013 10:38:50 +0200 Johan Hovold <jhovold@gmail.com> wrote:

> This is an update of the shadow-interrupt-mask series against v3.10-rc2.
> 
> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
> affected. If not, then some probing mechanism as the one Doug suggested
> could be implemented on top of (a subset of) these patches. What do you
> say, Nicolas?
> 
> Note that the first patch (adding a missing OF compile guard) could be
> applied straight away.

At this stage it is unclear to me how to proceed with patches 2-5.

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Robert Nelson]

On Wed, May 29, 2013 at 3:33 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Thu, 23 May 2013 10:38:50 +0200 Johan Hovold <jhovold@gmail.com> wrote:
>
>> This is an update of the shadow-interrupt-mask series against v3.10-rc2.
>>
>> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
>> affected. If not, then some probing mechanism as the one Doug suggested
>> could be implemented on top of (a subset of) these patches. What do you
>> say, Nicolas?
>>
>> Note that the first patch (adding a missing OF compile guard) could be
>> applied straight away.
>
> At this stage it is unclear to me how to proceed with patches 2-5.

fyi:

A version of these patches had been applied once before:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0ef1594c017521ea89278e80fe3f80dafb17abde

But due to a few issues it was later reverted:
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e24b0bfa2f0446ffaad2661040be23668133aef8

Regards,

-- 
Robert Nelson
http://www.rcn-ee.com/

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Douglas Gilbert]

On 13-05-29 04:41 PM, Robert Nelson wrote:
> On Wed, May 29, 2013 at 3:33 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
>> On Thu, 23 May 2013 10:38:50 +0200 Johan Hovold <jhovold@gmail.com> wrote:
>>
>>> This is an update of the shadow-interrupt-mask series against v3.10-rc2.
>>>
>>> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
>>> affected. If not, then some probing mechanism as the one Doug suggested
>>> could be implemented on top of (a subset of) these patches. What do you
>>> say, Nicolas?
>>>
>>> Note that the first patch (adding a missing OF compile guard) could be
>>> applied straight away.
>>
>> At this stage it is unclear to me how to proceed with patches 2-5.
>
> fyi:
>
> A version of these patches had been applied once before:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0ef1594c017521ea89278e80fe3f80dafb17abde
>
> But due to a few issues it was later reverted:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e24b0bfa2f0446ffaad2661040be23668133aef8

Strange life of a patch. Mine was the original, Johan Hovold
objected and had it reverted. Johan then presented his first
patch then v2. They got lost in the weeds.

My hardware was still broken and this bug caused collateral
damage. My original patch no longer applied to lk 3.10.0-rc1
so I rewrote it, borrowing some of Johan's ideas and doing a
probe time check for the broken RTC_IMR. That patch was
presented about a week ago:
   http://marc.info/?l=linux-arm-kernel&m=136917492531478&w=2
The top of that post gives some more background.

That prompted Johan to produce v3 of his patch which is the
subject of this thread. I was hoping that Nicolas Ferre would
comment or ack one of these patches. Still waiting.


I have a copy of the original, publicly released manual for
the at91sam9g25 (a member of the at91sam9x5 family) marked
"11032A?ATARM?27-Jul-11". It contains the following:
   Errata
   49.3.1
     RTC: Interrupt Mask Register cannot be used
          Interrupt Mask Register reading always returns 0.

Both Rev B and Rev C of that manual drop that particular
erratum. My g25 SoC-based subsystems come from an Atmel
partner and still have the RTC IMR bug.

Doug Gilbert

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Nicolas Ferre]

On 30/05/2013 01:22, Douglas Gilbert :
> On 13-05-29 04:41 PM, Robert Nelson wrote:
>> On Wed, May 29, 2013 at 3:33 PM, Andrew Morton
>> <akpm@linux-foundation.org> wrote:
>>> On Thu, 23 May 2013 10:38:50 +0200 Johan Hovold <jhovold@gmail.com>
>>> wrote:
>>>
>>>> This is an update of the shadow-interrupt-mask series against
>>>> v3.10-rc2.
>>>>
>>>> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
>>>> affected. If not, then some probing mechanism as the one Doug suggested
>>>> could be implemented on top of (a subset of) these patches. What do you
>>>> say, Nicolas?
>>>>
>>>> Note that the first patch (adding a missing OF compile guard) could be
>>>> applied straight away.
>>>
>>> At this stage it is unclear to me how to proceed with patches 2-5.
>>
>> fyi:
>>
>> A version of these patches had been applied once before:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0ef1594c017521ea89278e80fe3f80dafb17abde
>>
>>
>> But due to a few issues it was later reverted:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e24b0bfa2f0446ffaad2661040be23668133aef8
>>
>
> Strange life of a patch. Mine was the original, Johan Hovold
> objected and had it reverted. Johan then presented his first
> patch then v2. They got lost in the weeds.

No, they were not lost. No patch is ever lost and this thread is the proof.

> My hardware was still broken and this bug caused collateral
> damage. My original patch no longer applied to lk 3.10.0-rc1
> so I rewrote it, borrowing some of Johan's ideas and doing a
> probe time check for the broken RTC_IMR. That patch was
> presented about a week ago:
>    http://marc.info/?l=linux-arm-kernel&m=136917492531478&w=2
> The top of that post gives some more background.
>
> That prompted Johan to produce v3 of his patch which is the
> subject of this thread. I was hoping that Nicolas Ferre would
> comment or ack one of these patches. Still waiting.

Sure that all this did not progressed at the speed you expected. I 
understand that. But even if I did not answered in a timely manner, that 
does not mean that I didn't considered it and marked it as "things to be 
done before next merge window"...

So, today, too late, I gave my "Acked-by". Sorry for the delay. Let's 
still monitor the progress of this series upstream.

> I have a copy of the original, publicly released manual for
> the at91sam9g25 (a member of the at91sam9x5 family) marked
> "11032A?ATARM?27-Jul-11". It contains the following:
>    Errata
>    49.3.1
>      RTC: Interrupt Mask Register cannot be used
>           Interrupt Mask Register reading always returns 0.
>
> Both Rev B and Rev C of that manual drop that particular
> erratum. My g25 SoC-based subsystems come from an Atmel
> partner and still have the RTC IMR bug.

We already talked about this Douglas. Why are you saying this again. So, 
to summarize:

1/ each and every at91sam9x5 family SoC have and will probably always 
have this IMR bug (including 9g25 which is part of the family).

2/ you kindly reported the errata disappearing in the documentation. It 
is an error with document appearance which you probably noted. I have 
made the necessary actions to correct this. But here again, you have to 
be patient waiting for the datasheet's next revision.

Best regards,
-- 
Nicolas Ferre

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Nicolas Ferre]

On 29/05/2013 22:41, Robert Nelson :
> On Wed, May 29, 2013 at 3:33 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
>> On Thu, 23 May 2013 10:38:50 +0200 Johan Hovold <jhovold@gmail.com> wrote:
>>
>>> This is an update of the shadow-interrupt-mask series against v3.10-rc2.
>>>
>>> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
>>> affected. If not, then some probing mechanism as the one Doug suggested
>>> could be implemented on top of (a subset of) these patches. What do you
>>> say, Nicolas?
>>>
>>> Note that the first patch (adding a missing OF compile guard) could be
>>> applied straight away.
>>
>> At this stage it is unclear to me how to proceed with patches 2-5.
>
> fyi:
>
> A version of these patches had been applied once before:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=0ef1594c017521ea89278e80fe3f80dafb17abde
>
> But due to a few issues it was later reverted:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=e24b0bfa2f0446ffaad2661040be23668133aef8

This new revision doesn't have the issue encountered by first version.

Andrew,

The review of this patch series was in my TODO list for some time...

Today, I magically took time to review it ;-)
The patch series is good and I (even if it is too late) here is my:

Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>

I do not know if the series can be stacked for inclusion in 3.10-rc but 
the resolution of this bug can help a lot (as Douglas is saying in 
subsequent email...).

Best regards,
-- 
Nicolas Ferre

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Andrew Morton]

On Thu, 30 May 2013 09:50:27 +0200 Nicolas Ferre <nicolas.ferre@atmel.com> wrote:

> The review of this patch series was in my TODO list for some time...
> 
> Today, I magically took time to review it ;-)
> The patch series is good and I (even if it is too late) here is my:
> 
> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
> 
> I do not know if the series can be stacked for inclusion in 3.10-rc but 
> the resolution of this bug can help a lot (as Douglas is saying in 
> subsequent email...).

We can do that, but looking through the discussion and changelogs I
can't seem to find a usable description of what impact the bug (and its
fix) have upon end-users.

A nicely packaged description of that impact would help grease the
wheels, please.

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Douglas Gilbert]

On 13-05-30 03:36 PM, Andrew Morton wrote:
> On Thu, 30 May 2013 09:50:27 +0200 Nicolas Ferre <nicolas.ferre@atmel.com> wrote:
>
>> The review of this patch series was in my TODO list for some time...
>>
>> Today, I magically took time to review it ;-)
>> The patch series is good and I (even if it is too late) here is my:
>>
>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>
>> I do not know if the series can be stacked for inclusion in 3.10-rc but
>> the resolution of this bug can help a lot (as Douglas is saying in
>> subsequent email...).
>
> We can do that, but looking through the discussion and changelogs I
> can't seem to find a usable description of what impact the bug (and its
> fix) have upon end-users.
>
> A nicely packaged description of that impact would help grease the
> wheels, please.

How about this:

The members of Atmel's at91sam9x5 family (9x5) have
a broken RTC interrupt mask register (AT91_RTC_IMR).
It does not reflect enabled interrupts but instead
always returns zero.

The kernel's rtc-at91rm9200 driver handles the RTC
for the 9x5 family. Currently when the date/time is
set, an interrupt is generated and this driver neglects
to handle the interrupt. The kernel complains about the
un-handled interrupt and disables it henceforth. This
not only breaks the RTC function, but since that
interrupt is shared (Atmel's SYS interrupt) then other
things break as well (e.g. the debug port no longer
accepts characters).

Tested on the at91sam9g25. Bug confirmed by Atmel.

Edit as you please.

Doug Gilbert

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Nicolas Ferre]

On 31/05/2013 01:17, Douglas Gilbert :
> On 13-05-30 03:36 PM, Andrew Morton wrote:
>> On Thu, 30 May 2013 09:50:27 +0200 Nicolas Ferre
>> <nicolas.ferre@atmel.com> wrote:
>>
>>> The review of this patch series was in my TODO list for some time...
>>>
>>> Today, I magically took time to review it ;-)
>>> The patch series is good and I (even if it is too late) here is my:
>>>
>>> Acked-by: Nicolas Ferre <nicolas.ferre@atmel.com>
>>>
>>> I do not know if the series can be stacked for inclusion in 3.10-rc but
>>> the resolution of this bug can help a lot (as Douglas is saying in
>>> subsequent email...).
>>
>> We can do that, but looking through the discussion and changelogs I
>> can't seem to find a usable description of what impact the bug (and its
>> fix) have upon end-users.
>>
>> A nicely packaged description of that impact would help grease the
>> wheels, please.
>
> How about this:
>
> The members of Atmel's at91sam9x5 family (9x5) have
> a broken RTC interrupt mask register (AT91_RTC_IMR).
> It does not reflect enabled interrupts but instead
> always returns zero.
>
> The kernel's rtc-at91rm9200 driver handles the RTC
> for the 9x5 family. Currently when the date/time is
> set, an interrupt is generated and this driver neglects
> to handle the interrupt. The kernel complains about the
> un-handled interrupt and disables it henceforth. This
> not only breaks the RTC function, but since that
> interrupt is shared (Atmel's SYS interrupt) then other
> things break as well (e.g. the debug port no longer
> accepts characters).
>
> Tested on the at91sam9g25. Bug confirmed by Atmel.

Absolutely. Thank you Douglas for the detailed description.

>
> Edit as you please.
>
> Doug Gilbert
>
>


-- 
Nicolas Ferre

[PATCH v3 0/5] rtc-at91rm9200: add shadow interrupt mask

[Nicolas Ferre]

On 23/05/2013 10:38, Johan Hovold :
> This is an update of the shadow-interrupt-mask series against v3.10-rc2.
>
> I guess we need Atmel to confirm that all sam9x5 SoCs are indeed
> affected. If not, then some probing mechanism as the one Doug suggested
> could be implemented on top of (a subset of) these patches. What do you
> say, Nicolas?

No probing mechanism is needed: only sam9x5 are affected.

> Note that the first patch (adding a missing OF compile guard) could be
> applied straight away.
>
> Thanks,
> Johan
>
>
> v3:
>   - rebase against v3.10-rc2
>   - remove some comments

Thanks for rebasing this series.

Best regards,

> v2:
>   - rebase on top of DT-support patch by Joachim Eastwood
>   - add missing brace in DT-id table
>
>
> Johan Hovold (5):
>    rtc-at91rm9200: add match-table compile guard
>    rtc-at91rm9200: add configuration support
>    rtc-at91rm9200: refactor interrupt-register handling
>    rtc-at91rm9200: add shadow interrupt mask
>    rtc-at91rm9200: use shadow IMR on at91sam9x5
>
>   .../bindings/rtc/atmel,at91rm9200-rtc.txt          |   2 +-
>   arch/arm/boot/dts/at91sam9x5.dtsi                  |   2 +-
>   drivers/rtc/rtc-at91rm9200.c                       | 131 +++++++++++++++++----
>   3 files changed, 113 insertions(+), 22 deletions(-)
>


-- 
Nicolas Ferre