* [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD @ 2014-05-07 10:28 ` Boris BREZILLON 0 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-07 10:28 UTC (permalink / raw) To: linux-arm-kernel The rtc user must wait at least 1 sec between each time/calandar update (see atmel's datasheet chapter "Updating Time/Calendar"). Use the SECEV interrupt (as suggested by the datasheet) to wait for the RTC to be ready for new updates. Cc: <stable@vger.kernel.org> # v3.10+ Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> --- Hello, This patch fixes a deadlock in an uninterruptible wait when the RTC is updated more than once every second. AFAICT the bug is here from the beginning, but I think we should at least backport this fix to 3.10 and the following longterm and stable releases. Best Regards, Boris Changes since v1: - disable SECEV interrupt when the RTC is ready to accept new time updates drivers/rtc/rtc-at91rm9200.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c index 3281c90..44fe83e 100644 --- a/drivers/rtc/rtc-at91rm9200.c +++ b/drivers/rtc/rtc-at91rm9200.c @@ -48,6 +48,7 @@ struct at91_rtc_config { static const struct at91_rtc_config *at91_rtc_config; static DECLARE_COMPLETION(at91_rtc_updated); +static DECLARE_COMPLETION(at91_rtc_upd_rdy); static unsigned int at91_alarm_year = AT91_RTC_EPOCH; static void __iomem *at91_rtc_regs; static int irq; @@ -161,6 +162,8 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm) 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); + wait_for_completion(&at91_rtc_upd_rdy); + /* Stop Time/Calendar from counting */ cr = at91_rtc_read(AT91_RTC_CR); at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM); @@ -183,7 +186,9 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm) /* Restart Time/Calendar */ cr = at91_rtc_read(AT91_RTC_CR); + at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_SECEV); at91_rtc_write(AT91_RTC_CR, cr & ~(AT91_RTC_UPDCAL | AT91_RTC_UPDTIM)); + at91_rtc_write_ier(AT91_RTC_SECEV); return 0; } @@ -290,8 +295,10 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id) if (rtsr) { /* this interrupt is shared! Is it ours? */ if (rtsr & AT91_RTC_ALARM) events |= (RTC_AF | RTC_IRQF); - if (rtsr & AT91_RTC_SECEV) - events |= (RTC_UF | RTC_IRQF); + if (rtsr & AT91_RTC_SECEV) { + complete(&at91_rtc_upd_rdy); + at91_rtc_write_idr(AT91_RTC_SECEV); + } if (rtsr & AT91_RTC_ACKUPD) complete(&at91_rtc_updated); @@ -413,6 +420,11 @@ static int __init at91_rtc_probe(struct platform_device *pdev) return PTR_ERR(rtc); platform_set_drvdata(pdev, rtc); + /* enable SECEV interrupt in order to initialize at91_rtc_upd_rdy + * completion. + */ + at91_rtc_write_ier(AT91_RTC_SECEV); + dev_info(&pdev->dev, "AT91 Real Time Clock driver.\n"); return 0; } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD @ 2014-05-07 10:28 ` Boris BREZILLON 0 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-07 10:28 UTC (permalink / raw) To: Bryan Evenson Cc: Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, Alessandro Zummo, rtc-linux, linux-kernel, Boris BREZILLON, stable The rtc user must wait at least 1 sec between each time/calandar update (see atmel's datasheet chapter "Updating Time/Calendar"). Use the SECEV interrupt (as suggested by the datasheet) to wait for the RTC to be ready for new updates. Cc: <stable@vger.kernel.org> # v3.10+ Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> --- Hello, This patch fixes a deadlock in an uninterruptible wait when the RTC is updated more than once every second. AFAICT the bug is here from the beginning, but I think we should at least backport this fix to 3.10 and the following longterm and stable releases. Best Regards, Boris Changes since v1: - disable SECEV interrupt when the RTC is ready to accept new time updates drivers/rtc/rtc-at91rm9200.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c index 3281c90..44fe83e 100644 --- a/drivers/rtc/rtc-at91rm9200.c +++ b/drivers/rtc/rtc-at91rm9200.c @@ -48,6 +48,7 @@ struct at91_rtc_config { static const struct at91_rtc_config *at91_rtc_config; static DECLARE_COMPLETION(at91_rtc_updated); +static DECLARE_COMPLETION(at91_rtc_upd_rdy); static unsigned int at91_alarm_year = AT91_RTC_EPOCH; static void __iomem *at91_rtc_regs; static int irq; @@ -161,6 +162,8 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm) 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_hour, tm->tm_min, tm->tm_sec); + wait_for_completion(&at91_rtc_upd_rdy); + /* Stop Time/Calendar from counting */ cr = at91_rtc_read(AT91_RTC_CR); at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | AT91_RTC_UPDTIM); @@ -183,7 +186,9 @@ static int at91_rtc_settime(struct device *dev, struct rtc_time *tm) /* Restart Time/Calendar */ cr = at91_rtc_read(AT91_RTC_CR); + at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_SECEV); at91_rtc_write(AT91_RTC_CR, cr & ~(AT91_RTC_UPDCAL | AT91_RTC_UPDTIM)); + at91_rtc_write_ier(AT91_RTC_SECEV); return 0; } @@ -290,8 +295,10 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id) if (rtsr) { /* this interrupt is shared! Is it ours? */ if (rtsr & AT91_RTC_ALARM) events |= (RTC_AF | RTC_IRQF); - if (rtsr & AT91_RTC_SECEV) - events |= (RTC_UF | RTC_IRQF); + if (rtsr & AT91_RTC_SECEV) { + complete(&at91_rtc_upd_rdy); + at91_rtc_write_idr(AT91_RTC_SECEV); + } if (rtsr & AT91_RTC_ACKUPD) complete(&at91_rtc_updated); @@ -413,6 +420,11 @@ static int __init at91_rtc_probe(struct platform_device *pdev) return PTR_ERR(rtc); platform_set_drvdata(pdev, rtc); + /* enable SECEV interrupt in order to initialize at91_rtc_upd_rdy + * completion. + */ + at91_rtc_write_ier(AT91_RTC_SECEV); + dev_info(&pdev->dev, "AT91 Real Time Clock driver.\n"); return 0; } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD 2014-05-07 10:28 ` Boris BREZILLON @ 2014-05-07 14:51 ` Bryan Evenson -1 siblings, 0 replies; 24+ messages in thread From: Bryan Evenson @ 2014-05-07 14:51 UTC (permalink / raw) To: linux-arm-kernel Boris, > -----Original Message----- > From: Boris BREZILLON [mailto:boris.brezillon at free-electrons.com] > Sent: Wednesday, May 07, 2014 6:28 AM > To: Bryan Evenson > Cc: Andrew Victor; Nicolas Ferre; Jean-Christophe Plagniol-Villard; linux-arm- > kernel at lists.infradead.org; Alessandro Zummo; rtc- > linux at googlegroups.com; linux-kernel at vger.kernel.org; Boris BREZILLON; > stable at vger.kernel.org > Subject: [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD > > The rtc user must wait at least 1 sec between each time/calandar update > (see atmel's datasheet chapter "Updating Time/Calendar"). > > Use the SECEV interrupt (as suggested by the datasheet) to wait for the RTC > to be ready for new updates. > > Cc: <stable@vger.kernel.org> # v3.10+ > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello, > > This patch fixes a deadlock in an uninterruptible wait when the RTC is > updated more than once every second. > AFAICT the bug is here from the beginning, but I think we should at least > backport this fix to 3.10 and the following longterm and stable releases. > > Best Regards, > > Boris Hang on, I found a problem. My system works fine on a soft reboot, but during a power cycle my system hangs. Last thing I see to print is "Uncompressing Linux... done, booting the kernel." If I removde the backup battery for the RTC my system boots just fine. Luckily this is very repeatable on my system. This looks similar to the early rtt-interrupt bug which was fixed about 7 months ago (linux-stable commits 6ce3ee9af1ad697c71766067ffa1a35352dfdcfd and 54f830b7337ed016b6708cf5a2ac297d4cee227d). I confirmed that the kernel I'm using has the relevant fix applied from 7 months ago. Did this fix reintroduce the early rtt-interrupt bug? Let me know if you would like me to test anything or more information. Regards, Bryan > > Changes since v1: > - disable SECEV interrupt when the RTC is ready to accept new time updates > > drivers/rtc/rtc-at91rm9200.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c index > 3281c90..44fe83e 100644 > --- a/drivers/rtc/rtc-at91rm9200.c > +++ b/drivers/rtc/rtc-at91rm9200.c > @@ -48,6 +48,7 @@ struct at91_rtc_config { > > static const struct at91_rtc_config *at91_rtc_config; static > DECLARE_COMPLETION(at91_rtc_updated); > +static DECLARE_COMPLETION(at91_rtc_upd_rdy); > static unsigned int at91_alarm_year = AT91_RTC_EPOCH; static void > __iomem *at91_rtc_regs; static int irq; @@ -161,6 +162,8 @@ static int > at91_rtc_settime(struct device *dev, struct rtc_time *tm) > 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday, > tm->tm_hour, tm->tm_min, tm->tm_sec); > > + wait_for_completion(&at91_rtc_upd_rdy); > + > /* Stop Time/Calendar from counting */ > cr = at91_rtc_read(AT91_RTC_CR); > at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | > AT91_RTC_UPDTIM); @@ -183,7 +186,9 @@ static int > at91_rtc_settime(struct device *dev, struct rtc_time *tm) > > /* Restart Time/Calendar */ > cr = at91_rtc_read(AT91_RTC_CR); > + at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_SECEV); > at91_rtc_write(AT91_RTC_CR, cr & ~(AT91_RTC_UPDCAL | > AT91_RTC_UPDTIM)); > + at91_rtc_write_ier(AT91_RTC_SECEV); > > return 0; > } > @@ -290,8 +295,10 @@ static irqreturn_t at91_rtc_interrupt(int irq, void > *dev_id) > if (rtsr) { /* this interrupt is shared! Is it ours? */ > if (rtsr & AT91_RTC_ALARM) > events |= (RTC_AF | RTC_IRQF); > - if (rtsr & AT91_RTC_SECEV) > - events |= (RTC_UF | RTC_IRQF); > + if (rtsr & AT91_RTC_SECEV) { > + complete(&at91_rtc_upd_rdy); > + at91_rtc_write_idr(AT91_RTC_SECEV); > + } > if (rtsr & AT91_RTC_ACKUPD) > complete(&at91_rtc_updated); > > @@ -413,6 +420,11 @@ static int __init at91_rtc_probe(struct > platform_device *pdev) > return PTR_ERR(rtc); > platform_set_drvdata(pdev, rtc); > > + /* enable SECEV interrupt in order to initialize at91_rtc_upd_rdy > + * completion. > + */ > + at91_rtc_write_ier(AT91_RTC_SECEV); > + > dev_info(&pdev->dev, "AT91 Real Time Clock driver.\n"); > return 0; > } > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD @ 2014-05-07 14:51 ` Bryan Evenson 0 siblings, 0 replies; 24+ messages in thread From: Bryan Evenson @ 2014-05-07 14:51 UTC (permalink / raw) To: Boris BREZILLON Cc: Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel@lists.infradead.org, Alessandro Zummo, rtc-linux@googlegroups.com, linux-kernel@vger.kernel.org, stable@vger.kernel.org Boris, > -----Original Message----- > From: Boris BREZILLON [mailto:boris.brezillon@free-electrons.com] > Sent: Wednesday, May 07, 2014 6:28 AM > To: Bryan Evenson > Cc: Andrew Victor; Nicolas Ferre; Jean-Christophe Plagniol-Villard; linux-arm- > kernel@lists.infradead.org; Alessandro Zummo; rtc- > linux@googlegroups.com; linux-kernel@vger.kernel.org; Boris BREZILLON; > stable@vger.kernel.org > Subject: [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD > > The rtc user must wait at least 1 sec between each time/calandar update > (see atmel's datasheet chapter "Updating Time/Calendar"). > > Use the SECEV interrupt (as suggested by the datasheet) to wait for the RTC > to be ready for new updates. > > Cc: <stable@vger.kernel.org> # v3.10+ > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello, > > This patch fixes a deadlock in an uninterruptible wait when the RTC is > updated more than once every second. > AFAICT the bug is here from the beginning, but I think we should at least > backport this fix to 3.10 and the following longterm and stable releases. > > Best Regards, > > Boris Hang on, I found a problem. My system works fine on a soft reboot, but during a power cycle my system hangs. Last thing I see to print is "Uncompressing Linux... done, booting the kernel." If I removde the backup battery for the RTC my system boots just fine. Luckily this is very repeatable on my system. This looks similar to the early rtt-interrupt bug which was fixed about 7 months ago (linux-stable commits 6ce3ee9af1ad697c71766067ffa1a35352dfdcfd and 54f830b7337ed016b6708cf5a2ac297d4cee227d). I confirmed that the kernel I'm using has the relevant fix applied from 7 months ago. Did this fix reintroduce the early rtt-interrupt bug? Let me know if you would like me to test anything or more information. Regards, Bryan > > Changes since v1: > - disable SECEV interrupt when the RTC is ready to accept new time updates > > drivers/rtc/rtc-at91rm9200.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c index > 3281c90..44fe83e 100644 > --- a/drivers/rtc/rtc-at91rm9200.c > +++ b/drivers/rtc/rtc-at91rm9200.c > @@ -48,6 +48,7 @@ struct at91_rtc_config { > > static const struct at91_rtc_config *at91_rtc_config; static > DECLARE_COMPLETION(at91_rtc_updated); > +static DECLARE_COMPLETION(at91_rtc_upd_rdy); > static unsigned int at91_alarm_year = AT91_RTC_EPOCH; static void > __iomem *at91_rtc_regs; static int irq; @@ -161,6 +162,8 @@ static int > at91_rtc_settime(struct device *dev, struct rtc_time *tm) > 1900 + tm->tm_year, tm->tm_mon, tm->tm_mday, > tm->tm_hour, tm->tm_min, tm->tm_sec); > > + wait_for_completion(&at91_rtc_upd_rdy); > + > /* Stop Time/Calendar from counting */ > cr = at91_rtc_read(AT91_RTC_CR); > at91_rtc_write(AT91_RTC_CR, cr | AT91_RTC_UPDCAL | > AT91_RTC_UPDTIM); @@ -183,7 +186,9 @@ static int > at91_rtc_settime(struct device *dev, struct rtc_time *tm) > > /* Restart Time/Calendar */ > cr = at91_rtc_read(AT91_RTC_CR); > + at91_rtc_write(AT91_RTC_SCCR, AT91_RTC_SECEV); > at91_rtc_write(AT91_RTC_CR, cr & ~(AT91_RTC_UPDCAL | > AT91_RTC_UPDTIM)); > + at91_rtc_write_ier(AT91_RTC_SECEV); > > return 0; > } > @@ -290,8 +295,10 @@ static irqreturn_t at91_rtc_interrupt(int irq, void > *dev_id) > if (rtsr) { /* this interrupt is shared! Is it ours? */ > if (rtsr & AT91_RTC_ALARM) > events |= (RTC_AF | RTC_IRQF); > - if (rtsr & AT91_RTC_SECEV) > - events |= (RTC_UF | RTC_IRQF); > + if (rtsr & AT91_RTC_SECEV) { > + complete(&at91_rtc_upd_rdy); > + at91_rtc_write_idr(AT91_RTC_SECEV); > + } > if (rtsr & AT91_RTC_ACKUPD) > complete(&at91_rtc_updated); > > @@ -413,6 +420,11 @@ static int __init at91_rtc_probe(struct > platform_device *pdev) > return PTR_ERR(rtc); > platform_set_drvdata(pdev, rtc); > > + /* enable SECEV interrupt in order to initialize at91_rtc_upd_rdy > + * completion. > + */ > + at91_rtc_write_ier(AT91_RTC_SECEV); > + > dev_info(&pdev->dev, "AT91 Real Time Clock driver.\n"); > return 0; > } > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-07 14:51 ` Bryan Evenson @ 2014-05-07 16:20 ` Boris BREZILLON -1 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-07 16:20 UTC (permalink / raw) To: linux-arm-kernel The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to mask all interrupts no matter what IMR claims about already masked irqs. Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> --- Hello Bryan, Yet another patch for you ;-). As usual, could you tell me if it fixes your bug. BTW, thanks for your tests. Best Regards, Boris arch/arm/mach-at91/sysirq_mask.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c index 2ba694f..eb3d2a5 100644 --- a/arch/arm/mach-at91/sysirq_mask.c +++ b/arch/arm/mach-at91/sysirq_mask.c @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) if (!base) return; - mask = readl_relaxed(base + AT91_RTC_IMR); - if (mask) { - pr_info("AT91: Disabling rtc irq\n"); - writel_relaxed(mask, base + AT91_RTC_IDR); - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ - } + writel_relaxed(0x1f, base + AT91_RTC_IDR); iounmap(base); } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-07 16:20 ` Boris BREZILLON 0 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-07 16:20 UTC (permalink / raw) To: Bryan Evenson Cc: Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel, Boris BREZILLON The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to mask all interrupts no matter what IMR claims about already masked irqs. Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> --- Hello Bryan, Yet another patch for you ;-). As usual, could you tell me if it fixes your bug. BTW, thanks for your tests. Best Regards, Boris arch/arm/mach-at91/sysirq_mask.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c index 2ba694f..eb3d2a5 100644 --- a/arch/arm/mach-at91/sysirq_mask.c +++ b/arch/arm/mach-at91/sysirq_mask.c @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) if (!base) return; - mask = readl_relaxed(base + AT91_RTC_IMR); - if (mask) { - pr_info("AT91: Disabling rtc irq\n"); - writel_relaxed(mask, base + AT91_RTC_IDR); - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ - } + writel_relaxed(0x1f, base + AT91_RTC_IDR); iounmap(base); } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-07 16:20 ` Boris BREZILLON @ 2014-05-07 18:44 ` Bryan Evenson -1 siblings, 0 replies; 24+ messages in thread From: Bryan Evenson @ 2014-05-07 18:44 UTC (permalink / raw) To: linux-arm-kernel Boris, > -----Original Message----- > From: Boris BREZILLON [mailto:boris.brezillon at free-electrons.com] > Sent: Wednesday, May 07, 2014 12:21 PM > To: Bryan Evenson > Cc: Andrew Victor; Nicolas Ferre; Jean-Christophe Plagniol-Villard; linux-arm- > kernel at lists.infradead.org; linux-kernel at vger.kernel.org; Boris BREZILLON > Subject: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs > > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris This fixes the issue on my system. I disconnected my system from the network to guarantee ntp didn't adjust the time. I power cycled my system 15 times with the RTC backup battery in place, and each time it booted without issue and the RTC kept the correct time. Before this patch my system would never complete booting with the RTC battery installed. I also used my previous test script for the previous patch to verify that I could not lockup access to the RTC. Tested-by: Bryan Evenson <bevenson@melinkcorp.com> Regards, Bryan > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach- > at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); > > iounmap(base); > } > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 24+ messages in thread
* RE: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-07 18:44 ` Bryan Evenson 0 siblings, 0 replies; 24+ messages in thread From: Bryan Evenson @ 2014-05-07 18:44 UTC (permalink / raw) To: Boris BREZILLON Cc: Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Boris, > -----Original Message----- > From: Boris BREZILLON [mailto:boris.brezillon@free-electrons.com] > Sent: Wednesday, May 07, 2014 12:21 PM > To: Bryan Evenson > Cc: Andrew Victor; Nicolas Ferre; Jean-Christophe Plagniol-Villard; linux-arm- > kernel@lists.infradead.org; linux-kernel@vger.kernel.org; Boris BREZILLON > Subject: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs > > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. > > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris This fixes the issue on my system. I disconnected my system from the network to guarantee ntp didn't adjust the time. I power cycled my system 15 times with the RTC backup battery in place, and each time it booted without issue and the RTC kept the correct time. Before this patch my system would never complete booting with the RTC battery installed. I also used my previous test script for the previous patch to verify that I could not lockup access to the RTC. Tested-by: Bryan Evenson <bevenson@melinkcorp.com> Regards, Bryan > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach- > at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); > > iounmap(base); > } > -- > 1.8.3.2 ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-07 18:44 ` Bryan Evenson @ 2014-05-08 3:10 ` Mark Roszko -1 siblings, 0 replies; 24+ messages in thread From: Mark Roszko @ 2014-05-08 3:10 UTC (permalink / raw) To: linux-arm-kernel Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 datasheets which might be worth referencing in the description? >49.7.1 RTC: Interrupt Mask Register cannot be used >Interrupt Mask Register read always returns 0. ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-08 3:10 ` Mark Roszko 0 siblings, 0 replies; 24+ messages in thread From: Mark Roszko @ 2014-05-08 3:10 UTC (permalink / raw) To: Bryan Evenson Cc: Boris BREZILLON, Jean-Christophe Plagniol-Villard, Nicolas Ferre, Andrew Victor, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 datasheets which might be worth referencing in the description? >49.7.1 RTC: Interrupt Mask Register cannot be used >Interrupt Mask Register read always returns 0. ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-08 3:10 ` Mark Roszko @ 2014-05-08 17:19 ` Boris BREZILLON -1 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-08 17:19 UTC (permalink / raw) To: linux-arm-kernel On 08/05/2014 05:10, Mark Roszko wrote: > Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 > datasheets which might be worth referencing in the description? > >> 49.7.1 RTC: Interrupt Mask Register cannot be used >> Interrupt Mask Register read always returns 0. Sure, I'll quote atmel's datasheet and state that it only impacts sam9g25 and g35 SoCs. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-08 17:19 ` Boris BREZILLON 0 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-08 17:19 UTC (permalink / raw) To: Mark Roszko, Bryan Evenson Cc: Jean-Christophe Plagniol-Villard, Nicolas Ferre, Andrew Victor, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org On 08/05/2014 05:10, Mark Roszko wrote: > Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 > datasheets which might be worth referencing in the description? > >> 49.7.1 RTC: Interrupt Mask Register cannot be used >> Interrupt Mask Register read always returns 0. Sure, I'll quote atmel's datasheet and state that it only impacts sam9g25 and g35 SoCs. Thanks, Boris -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-07 16:20 ` Boris BREZILLON @ 2014-05-08 15:49 ` Johan Hovold -1 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-08 15:49 UTC (permalink / raw) To: linux-arm-kernel On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. Crap, I totally forgot about this. Doug reported the problem off-list back in December, but it got lost somehow. Sorry. > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); I believe this is the right way to handle this hardware bug (IMR is always read as 0 on one particular SoC), but please document this in a comment. You should also keep the flush (read of IMR) regardless (to make sure the write has reached the peripheral), and remember to remove the now unused mask variable. > iounmap(base); > } Thanks, Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-08 15:49 ` Johan Hovold 0 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-08 15:49 UTC (permalink / raw) To: Boris BREZILLON Cc: Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. Crap, I totally forgot about this. Doug reported the problem off-list back in December, but it got lost somehow. Sorry. > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); I believe this is the right way to handle this hardware bug (IMR is always read as 0 on one particular SoC), but please document this in a comment. You should also keep the flush (read of IMR) regardless (to make sure the write has reached the peripheral), and remember to remove the now unused mask variable. > iounmap(base); > } Thanks, Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-08 15:49 ` Johan Hovold @ 2014-05-08 17:28 ` Boris BREZILLON -1 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-08 17:28 UTC (permalink / raw) To: linux-arm-kernel On 08/05/2014 17:49, Johan Hovold wrote: > On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: >> The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to >> mask all interrupts no matter what IMR claims about already masked irqs. > Crap, I totally forgot about this. Doug reported the problem off-list > back in December, but it got lost somehow. Sorry. No problem. BTW, I started to work on a more generic solution to handle these muxed irqs issues (see https://lkml.org/lkml/2014/3/28/353). Could you take a look at it (I'm still not happy with the proposed DT bindings, but this can be discussed)? >> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> >> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> >> --- >> Hello Bryan, >> >> Yet another patch for you ;-). >> >> As usual, could you tell me if it fixes your bug. >> >> BTW, thanks for your tests. >> >> Best Regards, >> >> Boris >> >> arch/arm/mach-at91/sysirq_mask.c | 7 +------ >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c >> index 2ba694f..eb3d2a5 100644 >> --- a/arch/arm/mach-at91/sysirq_mask.c >> +++ b/arch/arm/mach-at91/sysirq_mask.c >> @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) >> if (!base) >> return; >> >> - mask = readl_relaxed(base + AT91_RTC_IMR); >> - if (mask) { >> - pr_info("AT91: Disabling rtc irq\n"); >> - writel_relaxed(mask, base + AT91_RTC_IDR); >> - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ >> - } >> + writel_relaxed(0x1f, base + AT91_RTC_IDR); > I believe this is the right way to handle this hardware bug (IMR is > always read as 0 on one particular SoC), but please document this in a > comment. Sure, I'll quote atmel's datasheet describing the errata. > > You should also keep the flush (read of IMR) regardless (to make sure > the write has reached the peripheral), and remember to remove the now > unused mask variable. Does it has something to do with memory barriers ? If so, why not using writel instead of writel_relaxed ? If not, could you point out where it is described in the datasheet ? Best Regards, Boris > >> iounmap(base); >> } > Thanks, > Johan -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-08 17:28 ` Boris BREZILLON 0 siblings, 0 replies; 24+ messages in thread From: Boris BREZILLON @ 2014-05-08 17:28 UTC (permalink / raw) To: Johan Hovold Cc: Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel On 08/05/2014 17:49, Johan Hovold wrote: > On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: >> The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to >> mask all interrupts no matter what IMR claims about already masked irqs. > Crap, I totally forgot about this. Doug reported the problem off-list > back in December, but it got lost somehow. Sorry. No problem. BTW, I started to work on a more generic solution to handle these muxed irqs issues (see https://lkml.org/lkml/2014/3/28/353). Could you take a look at it (I'm still not happy with the proposed DT bindings, but this can be discussed)? >> Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> >> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> >> --- >> Hello Bryan, >> >> Yet another patch for you ;-). >> >> As usual, could you tell me if it fixes your bug. >> >> BTW, thanks for your tests. >> >> Best Regards, >> >> Boris >> >> arch/arm/mach-at91/sysirq_mask.c | 7 +------ >> 1 file changed, 1 insertion(+), 6 deletions(-) >> >> diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c >> index 2ba694f..eb3d2a5 100644 >> --- a/arch/arm/mach-at91/sysirq_mask.c >> +++ b/arch/arm/mach-at91/sysirq_mask.c >> @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) >> if (!base) >> return; >> >> - mask = readl_relaxed(base + AT91_RTC_IMR); >> - if (mask) { >> - pr_info("AT91: Disabling rtc irq\n"); >> - writel_relaxed(mask, base + AT91_RTC_IDR); >> - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ >> - } >> + writel_relaxed(0x1f, base + AT91_RTC_IDR); > I believe this is the right way to handle this hardware bug (IMR is > always read as 0 on one particular SoC), but please document this in a > comment. Sure, I'll quote atmel's datasheet describing the errata. > > You should also keep the flush (read of IMR) regardless (to make sure > the write has reached the peripheral), and remember to remove the now > unused mask variable. Does it has something to do with memory barriers ? If so, why not using writel instead of writel_relaxed ? If not, could you point out where it is described in the datasheet ? Best Regards, Boris > >> iounmap(base); >> } > Thanks, > Johan -- Boris Brezillon, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-08 17:28 ` Boris BREZILLON @ 2014-05-09 16:36 ` Johan Hovold -1 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-09 16:36 UTC (permalink / raw) To: linux-arm-kernel On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > You should also keep the flush (read of IMR) regardless (to make sure > > the write has reached the peripheral), and remember to remove the now > > unused mask variable. > > Does it has something to do with memory barriers ? > If so, why not using writel instead of writel_relaxed ? You only need to use the non-relaxed version when synchronising with DMA operations. The read-back of a register on the same device is a common technique to make sure that preceding write has actually reached the peripheral (write posting or flushing). In this case, it is used to make (reasonably) sure that interrupts have actually been masked before returning. (In the general case, you'd even need to verify the read-back value to be certain that the device has changed its state.) Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-09 16:36 ` Johan Hovold 0 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-09 16:36 UTC (permalink / raw) To: Boris BREZILLON Cc: Johan Hovold, Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > You should also keep the flush (read of IMR) regardless (to make sure > > the write has reached the peripheral), and remember to remove the now > > unused mask variable. > > Does it has something to do with memory barriers ? > If so, why not using writel instead of writel_relaxed ? You only need to use the non-relaxed version when synchronising with DMA operations. The read-back of a register on the same device is a common technique to make sure that preceding write has actually reached the peripheral (write posting or flushing). In this case, it is used to make (reasonably) sure that interrupts have actually been masked before returning. (In the general case, you'd even need to verify the read-back value to be certain that the device has changed its state.) Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-09 16:36 ` Johan Hovold @ 2014-05-29 23:09 ` Andrew Morton -1 siblings, 0 replies; 24+ messages in thread From: Andrew Morton @ 2014-05-29 23:09 UTC (permalink / raw) To: linux-arm-kernel On Fri, 9 May 2014 18:36:52 +0200 Johan Hovold <johan@hovold.com> wrote: > On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > > > You should also keep the flush (read of IMR) regardless (to make sure > > > the write has reached the peripheral), and remember to remove the now > > > unused mask variable. > > > > Does it has something to do with memory barriers ? > > If so, why not using writel instead of writel_relaxed ? > > You only need to use the non-relaxed version when synchronising with DMA > operations. > > The read-back of a register on the same device is a common technique to > make sure that preceding write has actually reached the peripheral > (write posting or flushing). In this case, it is used to make > (reasonably) sure that interrupts have actually been masked before > returning. (In the general case, you'd even need to verify the read-back > value to be certain that the device has changed its state.) > So I grabbed this patch as it's tied to "rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD" and is not in linux-next. Someone shout at me if that was a mistake. From: Boris BREZILLON <boris.brezillon@free-electrons.com> Subject: ARM: at91: fix rtc irq mask for sam9x5 SoCs The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to mask all interrupts no matter what IMR claims about already masked irqs. Mark said: : Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 : datasheets which might be worth referencing in the description? Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> Tested-by: Bryan Evenson <bevenson@melinkcorp.com> Cc: Andrew Victor <linux@maxim.org.za> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Mark Roszko <mark.roszko@gmail.com> Cc: Johan Hovold <johan@hovold.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- arch/arm/mach-at91/sysirq_mask.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff -puN arch/arm/mach-at91/sysirq_mask.c~arm-at91-fix-rtc-irq-mask-for-sam9x5-socs arch/arm/mach-at91/sysirq_mask.c --- a/arch/arm/mach-at91/sysirq_mask.c~arm-at91-fix-rtc-irq-mask-for-sam9x5-socs +++ a/arch/arm/mach-at91/sysirq_mask.c @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc if (!base) return; - mask = readl_relaxed(base + AT91_RTC_IMR); - if (mask) { - pr_info("AT91: Disabling rtc irq\n"); - writel_relaxed(mask, base + AT91_RTC_IDR); - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ - } + writel_relaxed(0x1f, base + AT91_RTC_IDR); iounmap(base); } _ ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-29 23:09 ` Andrew Morton 0 siblings, 0 replies; 24+ messages in thread From: Andrew Morton @ 2014-05-29 23:09 UTC (permalink / raw) To: Johan Hovold Cc: Boris BREZILLON, Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel On Fri, 9 May 2014 18:36:52 +0200 Johan Hovold <johan@hovold.com> wrote: > On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > > > You should also keep the flush (read of IMR) regardless (to make sure > > > the write has reached the peripheral), and remember to remove the now > > > unused mask variable. > > > > Does it has something to do with memory barriers ? > > If so, why not using writel instead of writel_relaxed ? > > You only need to use the non-relaxed version when synchronising with DMA > operations. > > The read-back of a register on the same device is a common technique to > make sure that preceding write has actually reached the peripheral > (write posting or flushing). In this case, it is used to make > (reasonably) sure that interrupts have actually been masked before > returning. (In the general case, you'd even need to verify the read-back > value to be certain that the device has changed its state.) > So I grabbed this patch as it's tied to "rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD" and is not in linux-next. Someone shout at me if that was a mistake. From: Boris BREZILLON <boris.brezillon@free-electrons.com> Subject: ARM: at91: fix rtc irq mask for sam9x5 SoCs The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to mask all interrupts no matter what IMR claims about already masked irqs. Mark said: : Atmel actually has this issue in the Errata of the SAM9G25 and SAM9G35 : datasheets which might be worth referencing in the description? Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> Reported-by: Bryan Evenson <bevenson@melinkcorp.com> Tested-by: Bryan Evenson <bevenson@melinkcorp.com> Cc: Andrew Victor <linux@maxim.org.za> Cc: Nicolas Ferre <nicolas.ferre@atmel.com> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com> Cc: Alessandro Zummo <a.zummo@towertech.it> Cc: Mark Roszko <mark.roszko@gmail.com> Cc: Johan Hovold <johan@hovold.com> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- arch/arm/mach-at91/sysirq_mask.c | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff -puN arch/arm/mach-at91/sysirq_mask.c~arm-at91-fix-rtc-irq-mask-for-sam9x5-socs arch/arm/mach-at91/sysirq_mask.c --- a/arch/arm/mach-at91/sysirq_mask.c~arm-at91-fix-rtc-irq-mask-for-sam9x5-socs +++ a/arch/arm/mach-at91/sysirq_mask.c @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc if (!base) return; - mask = readl_relaxed(base + AT91_RTC_IMR); - if (mask) { - pr_info("AT91: Disabling rtc irq\n"); - writel_relaxed(mask, base + AT91_RTC_IDR); - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ - } + writel_relaxed(0x1f, base + AT91_RTC_IDR); iounmap(base); } _ ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-29 23:09 ` Andrew Morton @ 2014-05-30 12:09 ` Johan Hovold -1 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-30 12:09 UTC (permalink / raw) To: linux-arm-kernel On Thu, May 29, 2014 at 04:09:48PM -0700, Andrew Morton wrote: > On Fri, 9 May 2014 18:36:52 +0200 Johan Hovold <johan@hovold.com> wrote: > > > On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > > > > > You should also keep the flush (read of IMR) regardless (to make sure > > > > the write has reached the peripheral), and remember to remove the now > > > > unused mask variable. > > > > > > Does it has something to do with memory barriers ? > > > If so, why not using writel instead of writel_relaxed ? > > > > You only need to use the non-relaxed version when synchronising with DMA > > operations. > > > > The read-back of a register on the same device is a common technique to > > make sure that preceding write has actually reached the peripheral > > (write posting or flushing). In this case, it is used to make > > (reasonably) sure that interrupts have actually been masked before > > returning. (In the general case, you'd even need to verify the read-back > > value to be certain that the device has changed its state.) > > So I grabbed this patch as it's tied to "rtc: rtc-at91rm9200: fix > uninterruptible wait for ACKUPD" and is not in linux-next. Someone > shout at me if that was a mistake. The patch is needed, but it seems you got the wrong version. There was an updated v4 posted the next day in this thread: http://marc.info/?l=linux-kernel&m=139964712229420&w=2 Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-30 12:09 ` Johan Hovold 0 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-30 12:09 UTC (permalink / raw) To: Andrew Morton Cc: Johan Hovold, Boris BREZILLON, Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel On Thu, May 29, 2014 at 04:09:48PM -0700, Andrew Morton wrote: > On Fri, 9 May 2014 18:36:52 +0200 Johan Hovold <johan@hovold.com> wrote: > > > On Thu, May 08, 2014 at 07:28:04PM +0200, Boris BREZILLON wrote: > > > > > > You should also keep the flush (read of IMR) regardless (to make sure > > > > the write has reached the peripheral), and remember to remove the now > > > > unused mask variable. > > > > > > Does it has something to do with memory barriers ? > > > If so, why not using writel instead of writel_relaxed ? > > > > You only need to use the non-relaxed version when synchronising with DMA > > operations. > > > > The read-back of a register on the same device is a common technique to > > make sure that preceding write has actually reached the peripheral > > (write posting or flushing). In this case, it is used to make > > (reasonably) sure that interrupts have actually been masked before > > returning. (In the general case, you'd even need to verify the read-back > > value to be certain that the device has changed its state.) > > So I grabbed this patch as it's tied to "rtc: rtc-at91rm9200: fix > uninterruptible wait for ACKUPD" and is not in linux-next. Someone > shout at me if that was a mistake. The patch is needed, but it seems you got the wrong version. There was an updated v4 posted the next day in this thread: http://marc.info/?l=linux-kernel&m=139964712229420&w=2 Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs 2014-05-07 16:20 ` Boris BREZILLON @ 2014-05-08 15:54 ` Johan Hovold -1 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-08 15:54 UTC (permalink / raw) To: linux-arm-kernel [ Sorry for the resend -- forgot to add Doug as CC. ] On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. Crap, I totally forgot about this. Doug reported the problem off-list back in December, but it got lost somehow. Sorry. > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); I believe this is the right way to handle this hardware bug (IMR is always read as 0 on one particular SoC), but please document this in a comment. You should also keep the flush (read of IMR) regardless (to make sure the write has reached the peripheral), and remember to remove the now unused mask variable. > iounmap(base); > } Thanks, Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs @ 2014-05-08 15:54 ` Johan Hovold 0 siblings, 0 replies; 24+ messages in thread From: Johan Hovold @ 2014-05-08 15:54 UTC (permalink / raw) To: Boris BREZILLON Cc: Bryan Evenson, Andrew Victor, Nicolas Ferre, Jean-Christophe Plagniol-Villard, linux-arm-kernel, linux-kernel, Douglas Gilbert, Johan Hovold [ Sorry for the resend -- forgot to add Doug as CC. ] On Wed, May 07, 2014 at 06:20:49PM +0200, Boris BREZILLON wrote: > The RTC IMR register is not reliable on sam9x5 SoCs, hence why me have to > mask all interrupts no matter what IMR claims about already masked irqs. Crap, I totally forgot about this. Doug reported the problem off-list back in December, but it got lost somehow. Sorry. > Signed-off-by: Boris BREZILLON <boris.brezillon@free-electrons.com> > Reported-by: Bryan Evenson <bevenson@melinkcorp.com> > --- > Hello Bryan, > > Yet another patch for you ;-). > > As usual, could you tell me if it fixes your bug. > > BTW, thanks for your tests. > > Best Regards, > > Boris > > arch/arm/mach-at91/sysirq_mask.c | 7 +------ > 1 file changed, 1 insertion(+), 6 deletions(-) > > diff --git a/arch/arm/mach-at91/sysirq_mask.c b/arch/arm/mach-at91/sysirq_mask.c > index 2ba694f..eb3d2a5 100644 > --- a/arch/arm/mach-at91/sysirq_mask.c > +++ b/arch/arm/mach-at91/sysirq_mask.c > @@ -37,12 +37,7 @@ void __init at91_sysirq_mask_rtc(u32 rtc_base) > if (!base) > return; > > - mask = readl_relaxed(base + AT91_RTC_IMR); > - if (mask) { > - pr_info("AT91: Disabling rtc irq\n"); > - writel_relaxed(mask, base + AT91_RTC_IDR); > - (void)readl_relaxed(base + AT91_RTC_IMR); /* flush */ > - } > + writel_relaxed(0x1f, base + AT91_RTC_IDR); I believe this is the right way to handle this hardware bug (IMR is always read as 0 on one particular SoC), but please document this in a comment. You should also keep the flush (read of IMR) regardless (to make sure the write has reached the peripheral), and remember to remove the now unused mask variable. > iounmap(base); > } Thanks, Johan ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2014-05-30 12:10 UTC | newest] Thread overview: 24+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-05-07 10:28 [PATCH v2] rtc: rtc-at91rm9200: fix uninterruptible wait for ACKUPD Boris BREZILLON 2014-05-07 10:28 ` Boris BREZILLON 2014-05-07 14:51 ` Bryan Evenson 2014-05-07 14:51 ` Bryan Evenson 2014-05-07 16:20 ` [PATCH] ARM: at91: fix rtc irq mask for sam9x5 SoCs Boris BREZILLON 2014-05-07 16:20 ` Boris BREZILLON 2014-05-07 18:44 ` Bryan Evenson 2014-05-07 18:44 ` Bryan Evenson 2014-05-08 3:10 ` Mark Roszko 2014-05-08 3:10 ` Mark Roszko 2014-05-08 17:19 ` Boris BREZILLON 2014-05-08 17:19 ` Boris BREZILLON 2014-05-08 15:49 ` Johan Hovold 2014-05-08 15:49 ` Johan Hovold 2014-05-08 17:28 ` Boris BREZILLON 2014-05-08 17:28 ` Boris BREZILLON 2014-05-09 16:36 ` Johan Hovold 2014-05-09 16:36 ` Johan Hovold 2014-05-29 23:09 ` Andrew Morton 2014-05-29 23:09 ` Andrew Morton 2014-05-30 12:09 ` Johan Hovold 2014-05-30 12:09 ` Johan Hovold 2014-05-08 15:54 ` Johan Hovold 2014-05-08 15:54 ` Johan Hovold
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.