linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING
@ 2015-02-27 15:09 Boris Brezillon
  2015-02-27 15:09 ` [PATCH 1/6] PM / wakeup: export pm_system_wakeup symbol Boris Brezillon
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

Commit cab303be91dc47942bc25de33dc1140123540800 [1] introduced a WARN_ON
test which triggers a WARNING backtrace on at91 platforms.
While this WARN_ON is absolutely necessary to warn users that they should
not mix request with and without IRQF_NO_SUSPEND flags on shared IRQs,
there is no easy way to solve this issue on at91 platforms.

The main reason is that the init timer is often using a shared irq line
and thus request this irq with IRQF_NO_SUSPEND flag set, while other
peripherals request the same irq line without this flag.

This problem has recently been addressed by this patch [2] which adds
a new IRQF_COND_SUSPEND flag, that authorize mixing IRQF_NO_SUSPEND and
!IRQF_NO_SUSPEND as long as irq handlers setting IRQF_COND_SUSPEND
can safely be called in suspended state.

Doing this also implies taking care of system wakeup in devices handlers
if they tag the IRQ line as a wakeup source.
The first patch of this series exports the pm_system_wakeup symbol so
that drivers can call pm_system_wakeup from their interrupt handler.

This series then patches all at91 drivers that can have devices sharing
their IRQ line with a timer.

This series depends on [2].

Best Regards,

Boris

[1]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/irq/pm.c?id=cab303be91dc47942bc25de33dc1140123540800
[2]https://lkml.org/lkml/2015/2/26/675

Boris Brezillon (6):
  PM / wakeup: export pm_system_wakeup symbol
  rtc: at91sam9: rework wakeup and interrupt handling
  rtc: at91rm9200: rework wakeup and interrupt handling
  clk: at91: implement suspend/resume for the PMC irqchip
  watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND
  tty: serial: atmel: rework interrupt and wakeup handling

 drivers/base/power/wakeup.c       |  1 +
 drivers/clk/at91/pmc.c            | 20 ++++++++++-
 drivers/clk/at91/pmc.h            |  1 +
 drivers/rtc/rtc-at91rm9200.c      | 62 +++++++++++++++++++++++++--------
 drivers/rtc/rtc-at91sam9.c        | 73 ++++++++++++++++++++++++++++++++-------
 drivers/tty/serial/atmel_serial.c | 49 +++++++++++++++++++++++---
 drivers/watchdog/at91sam9_wdt.c   |  3 +-
 7 files changed, 177 insertions(+), 32 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/6] PM / wakeup: export pm_system_wakeup symbol
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 15:09 ` [PATCH 2/6] rtc: at91sam9: rework wakeup and interrupt handling Boris Brezillon
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

Export pm_system_wakeup function to allow irq handlers to deal with system
wakeup.
This is needed for shared IRQ lines where one of the handler is registered
with IRQF_NO_SUSPEND, while the other ones want to configure it as a wakeup
source.
In this specific case, irq core does not handle the wakeup process and
leave the decision to each irq handler.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/base/power/wakeup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/base/power/wakeup.c b/drivers/base/power/wakeup.c
index c2744b3..aab7158 100644
--- a/drivers/base/power/wakeup.c
+++ b/drivers/base/power/wakeup.c
@@ -730,6 +730,7 @@ void pm_system_wakeup(void)
 	pm_abort_suspend = true;
 	freeze_wake();
 }
+EXPORT_SYMBOL_GPL(pm_system_wakeup);
 
 void pm_wakeup_clear(void)
 {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/6] rtc: at91sam9: rework wakeup and interrupt handling
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
  2015-02-27 15:09 ` [PATCH 1/6] PM / wakeup: export pm_system_wakeup symbol Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 15:09 ` [PATCH 3/6] rtc: at91rm9200: " Boris Brezillon
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

The IRQ line used by the RTC device is usually shared with the system timer
(PIT) on at91 platforms.
Since timers are registering their handlers with IRQF_NO_SUSPEND, we should
expect being called in suspended state, and properly wake the system up
when this is the case.

Set IRQF_COND_SUSPEND flag when registering the IRQ handler to inform
irq core that it can safely be called while the system is suspended.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/rtc/rtc-at91sam9.c | 73 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 12 deletions(-)

diff --git a/drivers/rtc/rtc-at91sam9.c b/drivers/rtc/rtc-at91sam9.c
index 2183fd2..5ccaee3 100644
--- a/drivers/rtc/rtc-at91sam9.c
+++ b/drivers/rtc/rtc-at91sam9.c
@@ -23,6 +23,7 @@
 #include <linux/io.h>
 #include <linux/mfd/syscon.h>
 #include <linux/regmap.h>
+#include <linux/suspend.h>
 #include <linux/clk.h>
 
 /*
@@ -77,6 +78,9 @@ struct sam9_rtc {
 	unsigned int		gpbr_offset;
 	int 			irq;
 	struct clk		*sclk;
+	bool			suspended;
+	unsigned long		events;
+	spinlock_t		lock;
 };
 
 #define rtt_readl(rtc, field) \
@@ -271,14 +275,9 @@ static int at91_rtc_proc(struct device *dev, struct seq_file *seq)
 	return 0;
 }
 
-/*
- * IRQ handler for the RTC
- */
-static irqreturn_t at91_rtc_interrupt(int irq, void *_rtc)
+static irqreturn_t at91_rtc_cache_events(struct sam9_rtc *rtc)
 {
-	struct sam9_rtc *rtc = _rtc;
 	u32 sr, mr;
-	unsigned long events = 0;
 
 	/* Shared interrupt may be for another device.  Note: reading
 	 * SR clears it, so we must only read it in this irq handler!
@@ -290,18 +289,54 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *_rtc)
 
 	/* alarm status */
 	if (sr & AT91_RTT_ALMS)
-		events |= (RTC_AF | RTC_IRQF);
+		rtc->events |= (RTC_AF | RTC_IRQF);
 
 	/* timer update/increment */
 	if (sr & AT91_RTT_RTTINC)
-		events |= (RTC_UF | RTC_IRQF);
+		rtc->events |= (RTC_UF | RTC_IRQF);
+
+	return IRQ_HANDLED;
+}
+
+static void at91_rtc_flush_events(struct sam9_rtc *rtc)
+{
+	if (!rtc->events)
+		return;
 
-	rtc_update_irq(rtc->rtcdev, 1, events);
+	rtc_update_irq(rtc->rtcdev, 1, rtc->events);
+	rtc->events = 0;
 
 	pr_debug("%s: num=%ld, events=0x%02lx\n", __func__,
-		events >> 8, events & 0x000000FF);
+		rtc->events >> 8, rtc->events & 0x000000FF);
+}
 
-	return IRQ_HANDLED;
+/*
+ * IRQ handler for the RTC
+ */
+static irqreturn_t at91_rtc_interrupt(int irq, void *_rtc)
+{
+	struct sam9_rtc *rtc = _rtc;
+	int ret;
+
+	spin_lock(&rtc->lock);
+
+	ret = at91_rtc_cache_events(rtc);
+
+	/* We're called in suspended state */
+	if (rtc->suspended) {
+		/* Mask irqs coming from this peripheral */
+		rtt_writel(rtc, MR,
+			   rtt_readl(rtc, MR) &
+			   ~(AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN));
+		/* Trigger a system wakeup */
+		pm_system_wakeup();
+	} else {
+		at91_rtc_flush_events(rtc);
+	}
+
+	spin_unlock(&rtc->lock);
+
+	return ret;
 }
 
 static const struct rtc_class_ops at91_rtc_ops = {
@@ -421,7 +456,8 @@ static int at91_rtc_probe(struct platform_device *pdev)
 
 	/* register irq handler after we know what name we'll use */
 	ret = devm_request_irq(&pdev->dev, rtc->irq, at91_rtc_interrupt,
-				IRQF_SHARED, dev_name(&rtc->rtcdev->dev), rtc);
+			       IRQF_SHARED | IRQF_COND_SUSPEND,
+			       dev_name(&rtc->rtcdev->dev), rtc);
 	if (ret) {
 		dev_dbg(&pdev->dev, "can't share IRQ %d?\n", rtc->irq);
 		return ret;
@@ -482,7 +518,12 @@ static int at91_rtc_suspend(struct device *dev)
 	rtc->imr = mr & (AT91_RTT_ALMIEN | AT91_RTT_RTTINCIEN);
 	if (rtc->imr) {
 		if (device_may_wakeup(dev) && (mr & AT91_RTT_ALMIEN)) {
+			unsigned long flags;
+
 			enable_irq_wake(rtc->irq);
+			spin_lock_irqsave(&rtc->lock, flags);
+			rtc->suspended = true;
+			spin_unlock_irqrestore(&rtc->lock, flags);
 			/* don't let RTTINC cause wakeups */
 			if (mr & AT91_RTT_RTTINCIEN)
 				rtt_writel(rtc, MR, mr & ~AT91_RTT_RTTINCIEN);
@@ -499,10 +540,18 @@ static int at91_rtc_resume(struct device *dev)
 	u32		mr;
 
 	if (rtc->imr) {
+		unsigned long flags;
+
 		if (device_may_wakeup(dev))
 			disable_irq_wake(rtc->irq);
 		mr = rtt_readl(rtc, MR);
 		rtt_writel(rtc, MR, mr | rtc->imr);
+
+		spin_lock_irqsave(&rtc->lock, flags);
+		rtc->suspended = false;
+		at91_rtc_cache_events(rtc);
+		at91_rtc_flush_events(rtc);
+		spin_unlock_irqrestore(&rtc->lock, flags);
 	}
 
 	return 0;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/6] rtc: at91rm9200: rework wakeup and interrupt handling
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
  2015-02-27 15:09 ` [PATCH 1/6] PM / wakeup: export pm_system_wakeup symbol Boris Brezillon
  2015-02-27 15:09 ` [PATCH 2/6] rtc: at91sam9: rework wakeup and interrupt handling Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 15:09 ` [PATCH 4/6] clk: at91: implement suspend/resume for the PMC irqchip Boris Brezillon
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

The IRQ line used by the RTC device is usually shared with the system
timer (PIT) on at91 platforms.
Since timers are registering their handlers with IRQF_NO_SUSPEND, we
should expect being called in suspended state, and properly wake the
system up when this is the case.

Set IRQF_COND_SUSPEND flag when registering the IRQ handler to inform
irq core that it can safely be called while the system is suspended.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/rtc/rtc-at91rm9200.c | 62 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 48 insertions(+), 14 deletions(-)

diff --git a/drivers/rtc/rtc-at91rm9200.c b/drivers/rtc/rtc-at91rm9200.c
index 70a5d94..b4f7744 100644
--- a/drivers/rtc/rtc-at91rm9200.c
+++ b/drivers/rtc/rtc-at91rm9200.c
@@ -31,6 +31,7 @@
 #include <linux/io.h>
 #include <linux/of.h>
 #include <linux/of_device.h>
+#include <linux/suspend.h>
 #include <linux/uaccess.h>
 
 #include "rtc-at91rm9200.h"
@@ -54,6 +55,10 @@ static void __iomem *at91_rtc_regs;
 static int irq;
 static DEFINE_SPINLOCK(at91_rtc_lock);
 static u32 at91_rtc_shadow_imr;
+static bool suspended;
+static DEFINE_SPINLOCK(suspended_lock);
+static unsigned long cached_events;
+static u32 at91_rtc_imr;
 
 static void at91_rtc_write_ier(u32 mask)
 {
@@ -290,7 +295,9 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
 	struct rtc_device *rtc = platform_get_drvdata(pdev);
 	unsigned int rtsr;
 	unsigned long events = 0;
+	int ret = IRQ_NONE;
 
+	spin_lock(&suspended_lock);
 	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)
@@ -304,14 +311,22 @@ static irqreturn_t at91_rtc_interrupt(int irq, void *dev_id)
 
 		at91_rtc_write(AT91_RTC_SCCR, rtsr);	/* clear status reg */
 
-		rtc_update_irq(rtc, 1, events);
+		if (!suspended) {
+			rtc_update_irq(rtc, 1, events);
 
-		dev_dbg(&pdev->dev, "%s(): num=%ld, events=0x%02lx\n", __func__,
-			events >> 8, events & 0x000000FF);
+			dev_dbg(&pdev->dev, "%s(): num=%ld, events=0x%02lx\n",
+				__func__, events >> 8, events & 0x000000FF);
+		} else {
+			cached_events |= events;
+			at91_rtc_write_idr(at91_rtc_imr);
+			pm_system_wakeup();
+		}
 
-		return IRQ_HANDLED;
+		ret = IRQ_HANDLED;
 	}
-	return IRQ_NONE;		/* not handled */
+	spin_lock(&suspended_lock);
+
+	return ret;
 }
 
 static const struct at91_rtc_config at91rm9200_config = {
@@ -401,8 +416,8 @@ static int __init at91_rtc_probe(struct platform_device *pdev)
 					AT91_RTC_CALEV);
 
 	ret = devm_request_irq(&pdev->dev, irq, at91_rtc_interrupt,
-				IRQF_SHARED,
-				"at91_rtc", pdev);
+			       IRQF_SHARED | IRQF_COND_SUSPEND,
+			       "at91_rtc", pdev);
 	if (ret) {
 		dev_err(&pdev->dev, "IRQ %d already in use.\n", irq);
 		return ret;
@@ -454,8 +469,6 @@ static void at91_rtc_shutdown(struct platform_device *pdev)
 
 /* AT91RM9200 RTC Power management control */
 
-static u32 at91_rtc_imr;
-
 static int at91_rtc_suspend(struct device *dev)
 {
 	/* this IRQ is shared with DBGU and other hardware which isn't
@@ -464,21 +477,42 @@ static int at91_rtc_suspend(struct device *dev)
 	at91_rtc_imr = at91_rtc_read_imr()
 			& (AT91_RTC_ALARM|AT91_RTC_SECEV);
 	if (at91_rtc_imr) {
-		if (device_may_wakeup(dev))
+		if (device_may_wakeup(dev)) {
+			unsigned long flags;
+
 			enable_irq_wake(irq);
-		else
+
+			spin_lock_irqsave(&suspended_lock, flags);
+			suspended = true;
+			spin_unlock_irqrestore(&suspended_lock, flags);
+		} else {
 			at91_rtc_write_idr(at91_rtc_imr);
+		}
 	}
 	return 0;
 }
 
 static int at91_rtc_resume(struct device *dev)
 {
+	struct rtc_device *rtc = dev_get_drvdata(dev);
+
 	if (at91_rtc_imr) {
-		if (device_may_wakeup(dev))
+		if (device_may_wakeup(dev)) {
+			unsigned long flags;
+
+			spin_lock_irqsave(&suspended_lock, flags);
+
+			if (cached_events) {
+				rtc_update_irq(rtc, 1, cached_events);
+				cached_events = 0;
+			}
+
+			suspended = false;
+			spin_unlock_irqrestore(&suspended_lock, flags);
+
 			disable_irq_wake(irq);
-		else
-			at91_rtc_write_ier(at91_rtc_imr);
+		}
+		at91_rtc_write_ier(at91_rtc_imr);
 	}
 	return 0;
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/6] clk: at91: implement suspend/resume for the PMC irqchip
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
                   ` (2 preceding siblings ...)
  2015-02-27 15:09 ` [PATCH 3/6] rtc: at91rm9200: " Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 15:09 ` [PATCH 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND Boris Brezillon
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

The irq line used by the PMC block is shared with several peripherals
including the init timer which is registering its handler with
IRQF_NO_SUSPEND.
Implement the appropriate suspend/resume callback for the PMC irqchip,
and inform irq core that PMC irq handler can be safely called while
the system is suspended by setting IRQF_COND_SUSPEND.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/clk/at91/pmc.c | 20 +++++++++++++++++++-
 drivers/clk/at91/pmc.h |  1 +
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/at91/pmc.c b/drivers/clk/at91/pmc.c
index f07c815..3f27d21 100644
--- a/drivers/clk/at91/pmc.c
+++ b/drivers/clk/at91/pmc.c
@@ -89,12 +89,29 @@ static int pmc_irq_set_type(struct irq_data *d, unsigned type)
 	return 0;
 }
 
+static void pmc_irq_suspend(struct irq_data *d)
+{
+	struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
+
+	pmc->imr = pmc_read(pmc, AT91_PMC_IMR);
+	pmc_write(pmc, AT91_PMC_IDR, pmc->imr);
+}
+
+static void pmc_irq_resume(struct irq_data *d)
+{
+	struct at91_pmc *pmc = irq_data_get_irq_chip_data(d);
+
+	pmc_write(pmc, AT91_PMC_IER, pmc->imr);
+}
+
 static struct irq_chip pmc_irq = {
 	.name = "PMC",
 	.irq_disable = pmc_irq_mask,
 	.irq_mask = pmc_irq_mask,
 	.irq_unmask = pmc_irq_unmask,
 	.irq_set_type = pmc_irq_set_type,
+	.irq_suspend = pmc_irq_suspend,
+	.irq_resume = pmc_irq_resume,
 };
 
 static struct lock_class_key pmc_lock_class;
@@ -224,7 +241,8 @@ static struct at91_pmc *__init at91_pmc_init(struct device_node *np,
 		goto out_free_pmc;
 
 	pmc_write(pmc, AT91_PMC_IDR, 0xffffffff);
-	if (request_irq(pmc->virq, pmc_irq_handler, IRQF_SHARED, "pmc", pmc))
+	if (request_irq(pmc->virq, pmc_irq_handler,
+			IRQF_SHARED | IRQF_COND_SUSPEND, "pmc", pmc))
 		goto out_remove_irqdomain;
 
 	return pmc;
diff --git a/drivers/clk/at91/pmc.h b/drivers/clk/at91/pmc.h
index 52d2041..69abb08 100644
--- a/drivers/clk/at91/pmc.h
+++ b/drivers/clk/at91/pmc.h
@@ -33,6 +33,7 @@ struct at91_pmc {
 	spinlock_t lock;
 	const struct at91_pmc_caps *caps;
 	struct irq_domain *irqdomain;
+	u32 imr;
 };
 
 static inline void pmc_lock(struct at91_pmc *pmc)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
                   ` (3 preceding siblings ...)
  2015-02-27 15:09 ` [PATCH 4/6] clk: at91: implement suspend/resume for the PMC irqchip Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 15:09 ` [PATCH 6/6] tty: serial: atmel: rework interrupt and wakeup handling Boris Brezillon
  2015-02-27 22:33 ` [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Rafael J. Wysocki
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

The watchdog interrupt (only used when activating software watchdog)
shouldn't be suspended when entering suspend mode, because it is shared
with a timer device (which request the line with IRQF_NO_SUSPEND) and once
the watchdog "Mode Register" has been written, it cannot be changed (which
means we cannot disable the watchdog interrupt when entering suspend).

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/watchdog/at91sam9_wdt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/watchdog/at91sam9_wdt.c b/drivers/watchdog/at91sam9_wdt.c
index 6df9405..1443b3c 100644
--- a/drivers/watchdog/at91sam9_wdt.c
+++ b/drivers/watchdog/at91sam9_wdt.c
@@ -208,7 +208,8 @@ static int at91_wdt_init(struct platform_device *pdev, struct at91wdt *wdt)
 
 	if ((tmp & AT91_WDT_WDFIEN) && wdt->irq) {
 		err = request_irq(wdt->irq, wdt_interrupt,
-				  IRQF_SHARED | IRQF_IRQPOLL,
+				  IRQF_SHARED | IRQF_IRQPOLL |
+				  IRQF_NO_SUSPEND,
 				  pdev->name, wdt);
 		if (err)
 			return err;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 6/6] tty: serial: atmel: rework interrupt and wakeup handling
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
                   ` (4 preceding siblings ...)
  2015-02-27 15:09 ` [PATCH 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND Boris Brezillon
@ 2015-02-27 15:09 ` Boris Brezillon
  2015-02-27 22:33 ` [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Rafael J. Wysocki
  6 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-02-27 15:09 UTC (permalink / raw)
  To: linux-arm-kernel

The IRQ line connected to the DBGU UART is often shared with a timer device
which request the IRQ with IRQF_NO_SUSPEND.
Since the UART driver is correctly disabling IRQs when entering suspend
we can safely request the IRQ with IRQF_COND_SUSPEND so that irq core
will not complain about mixing IRQF_NO_SUSPEND and !IRQF_NO_SUSPEND.

Rework the interrupt handler to wake the system up when an interrupt
happens on the DEBUG_UART while the system is suspended.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 drivers/tty/serial/atmel_serial.c | 49 +++++++++++++++++++++++++++++++++++----
 1 file changed, 45 insertions(+), 4 deletions(-)

diff --git a/drivers/tty/serial/atmel_serial.c b/drivers/tty/serial/atmel_serial.c
index 846552b..4e959c4 100644
--- a/drivers/tty/serial/atmel_serial.c
+++ b/drivers/tty/serial/atmel_serial.c
@@ -47,6 +47,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/err.h>
 #include <linux/irq.h>
+#include <linux/suspend.h>
 
 #include <asm/io.h>
 #include <asm/ioctls.h>
@@ -173,6 +174,12 @@ struct atmel_uart_port {
 	bool			ms_irq_enabled;
 	bool			is_usart;	/* usart or uart */
 	struct timer_list	uart_timer;	/* uart timer */
+
+	bool			suspended;
+	unsigned int		pending;
+	unsigned int		pending_status;
+	spinlock_t		lock_suspended;
+
 	int (*prepare_rx)(struct uart_port *port);
 	int (*prepare_tx)(struct uart_port *port);
 	void (*schedule_rx)(struct uart_port *port);
@@ -1179,12 +1186,15 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 {
 	struct uart_port *port = dev_id;
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
-	unsigned int status, pending, pass_counter = 0;
+	unsigned int status, pending, mask, pass_counter = 0;
 	bool gpio_handled = false;
 
+	spin_lock(&atmel_port->lock_suspended);
+
 	do {
 		status = atmel_get_lines_status(port);
-		pending = status & UART_GET_IMR(port);
+		mask = UART_GET_IMR(port);
+		pending = status & mask;
 		if (!gpio_handled) {
 			/*
 			 * Dealing with GPIO interrupt
@@ -1206,11 +1216,21 @@ static irqreturn_t atmel_interrupt(int irq, void *dev_id)
 		if (!pending)
 			break;
 
+		if (atmel_port->suspended) {
+			atmel_port->pending |= pending;
+			atmel_port->pending_status = status;
+			UART_PUT_IDR(port, mask);
+			pm_system_wakeup();
+			break;
+		}
+
 		atmel_handle_receive(port, pending);
 		atmel_handle_status(port, pending, status);
 		atmel_handle_transmit(port, pending);
 	} while (pass_counter++ < ATMEL_ISR_PASS_LIMIT);
 
+	spin_unlock(&atmel_port->lock_suspended);
+
 	return pass_counter ? IRQ_HANDLED : IRQ_NONE;
 }
 
@@ -1742,7 +1762,8 @@ static int atmel_startup(struct uart_port *port)
 	/*
 	 * Allocate the IRQ
 	 */
-	retval = request_irq(port->irq, atmel_interrupt, IRQF_SHARED,
+	retval = request_irq(port->irq, atmel_interrupt,
+			IRQF_SHARED | IRQF_COND_SUSPEND,
 			tty ? tty->name : "atmel_serial", port);
 	if (retval) {
 		dev_err(port->dev, "atmel_startup - Can't get irq\n");
@@ -2513,8 +2534,14 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 
 	/* we can not wake up if we're running on slow clock */
 	atmel_port->may_wakeup = device_may_wakeup(&pdev->dev);
-	if (atmel_serial_clk_will_stop())
+	if (atmel_serial_clk_will_stop()) {
+		unsigned long flags;
+
+		spin_lock_irqsave(&atmel_port->lock_suspended, flags);
+		atmel_port->suspended = true;
+		spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
 		device_set_wakeup_enable(&pdev->dev, 0);
+	}
 
 	uart_suspend_port(&atmel_uart, port);
 
@@ -2525,6 +2552,18 @@ static int atmel_serial_resume(struct platform_device *pdev)
 {
 	struct uart_port *port = platform_get_drvdata(pdev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
+	unsigned long flags;
+
+	spin_lock_irqsave(&atmel_port->lock_suspended, flags);
+	if (atmel_port->pending) {
+		atmel_handle_receive(port, atmel_port->pending);
+		atmel_handle_status(port, atmel_port->pending,
+				    atmel_port->pending_status);
+		atmel_handle_transmit(port, atmel_port->pending);
+		atmel_port->pending = 0;
+	}
+	atmel_port->suspended = false;
+	spin_unlock_irqrestore(&atmel_port->lock_suspended, flags);
 
 	uart_resume_port(&atmel_uart, port);
 	device_set_wakeup_enable(&pdev->dev, atmel_port->may_wakeup);
@@ -2593,6 +2632,8 @@ static int atmel_serial_probe(struct platform_device *pdev)
 	port->backup_imr = 0;
 	port->uart.line = ret;
 
+	spin_lock_init(&port->lock_suspended);
+
 	ret = atmel_init_gpios(port, &pdev->dev);
 	if (ret < 0)
 		dev_err(&pdev->dev, "%s",
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING
  2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
                   ` (5 preceding siblings ...)
  2015-02-27 15:09 ` [PATCH 6/6] tty: serial: atmel: rework interrupt and wakeup handling Boris Brezillon
@ 2015-02-27 22:33 ` Rafael J. Wysocki
  2015-03-02  9:05   ` Boris Brezillon
  6 siblings, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2015-02-27 22:33 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday, February 27, 2015 04:09:52 PM Boris Brezillon wrote:
> Commit cab303be91dc47942bc25de33dc1140123540800 [1] introduced a WARN_ON
> test which triggers a WARNING backtrace on at91 platforms.
> While this WARN_ON is absolutely necessary to warn users that they should
> not mix request with and without IRQF_NO_SUSPEND flags on shared IRQs,
> there is no easy way to solve this issue on at91 platforms.
> 
> The main reason is that the init timer is often using a shared irq line
> and thus request this irq with IRQF_NO_SUSPEND flag set, while other
> peripherals request the same irq line without this flag.
> 
> This problem has recently been addressed by this patch [2] which adds
> a new IRQF_COND_SUSPEND flag, that authorize mixing IRQF_NO_SUSPEND and
> !IRQF_NO_SUSPEND as long as irq handlers setting IRQF_COND_SUSPEND
> can safely be called in suspended state.
> 
> Doing this also implies taking care of system wakeup in devices handlers
> if they tag the IRQ line as a wakeup source.
> The first patch of this series exports the pm_system_wakeup symbol so
> that drivers can call pm_system_wakeup from their interrupt handler.
> 
> This series then patches all at91 drivers that can have devices sharing
> their IRQ line with a timer.
> 
> This series depends on [2].

Series looks good to me (although admittedly I'm not familiar with the
drivers in question).

Do you want me to apply it on top of [2]?

 
> [1]https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/kernel/irq/pm.c?id=cab303be91dc47942bc25de33dc1140123540800
> [2]https://lkml.org/lkml/2015/2/26/675
> 
> Boris Brezillon (6):
>   PM / wakeup: export pm_system_wakeup symbol
>   rtc: at91sam9: rework wakeup and interrupt handling
>   rtc: at91rm9200: rework wakeup and interrupt handling
>   clk: at91: implement suspend/resume for the PMC irqchip
>   watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND
>   tty: serial: atmel: rework interrupt and wakeup handling
> 
>  drivers/base/power/wakeup.c       |  1 +
>  drivers/clk/at91/pmc.c            | 20 ++++++++++-
>  drivers/clk/at91/pmc.h            |  1 +
>  drivers/rtc/rtc-at91rm9200.c      | 62 +++++++++++++++++++++++++--------
>  drivers/rtc/rtc-at91sam9.c        | 73 ++++++++++++++++++++++++++++++++-------
>  drivers/tty/serial/atmel_serial.c | 49 +++++++++++++++++++++++---
>  drivers/watchdog/at91sam9_wdt.c   |  3 +-
>  7 files changed, 177 insertions(+), 32 deletions(-)
> 
> 

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING
  2015-02-27 22:33 ` [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Rafael J. Wysocki
@ 2015-03-02  9:05   ` Boris Brezillon
  0 siblings, 0 replies; 9+ messages in thread
From: Boris Brezillon @ 2015-03-02  9:05 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Rafael,

On Fri, 27 Feb 2015 23:33:35 +0100
"Rafael J. Wysocki" <rjw@rjwysocki.net> wrote:

> On Friday, February 27, 2015 04:09:52 PM Boris Brezillon wrote:
> > Commit cab303be91dc47942bc25de33dc1140123540800 [1] introduced a WARN_ON
> > test which triggers a WARNING backtrace on at91 platforms.
> > While this WARN_ON is absolutely necessary to warn users that they should
> > not mix request with and without IRQF_NO_SUSPEND flags on shared IRQs,
> > there is no easy way to solve this issue on at91 platforms.
> > 
> > The main reason is that the init timer is often using a shared irq line
> > and thus request this irq with IRQF_NO_SUSPEND flag set, while other
> > peripherals request the same irq line without this flag.
> > 
> > This problem has recently been addressed by this patch [2] which adds
> > a new IRQF_COND_SUSPEND flag, that authorize mixing IRQF_NO_SUSPEND and
> > !IRQF_NO_SUSPEND as long as irq handlers setting IRQF_COND_SUSPEND
> > can safely be called in suspended state.
> > 
> > Doing this also implies taking care of system wakeup in devices handlers
> > if they tag the IRQ line as a wakeup source.
> > The first patch of this series exports the pm_system_wakeup symbol so
> > that drivers can call pm_system_wakeup from their interrupt handler.
> > 
> > This series then patches all at91 drivers that can have devices sharing
> > their IRQ line with a timer.
> > 
> > This series depends on [2].
> 
> Series looks good to me (although admittedly I'm not familiar with the
> drivers in question).
> 
> Do you want me to apply it on top of [2]?

I'd like to wait for Nicolas' ack.
BTW, I realized I didn't send the series to the relevant subsystem
maintainers and I forgot to increment the patch version (I already
posted a v1 a few weeks ago), so I'll probably resend it.

You can take the first patch though.

Thanks,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-03-02  9:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-02-27 15:09 [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Boris Brezillon
2015-02-27 15:09 ` [PATCH 1/6] PM / wakeup: export pm_system_wakeup symbol Boris Brezillon
2015-02-27 15:09 ` [PATCH 2/6] rtc: at91sam9: rework wakeup and interrupt handling Boris Brezillon
2015-02-27 15:09 ` [PATCH 3/6] rtc: at91rm9200: " Boris Brezillon
2015-02-27 15:09 ` [PATCH 4/6] clk: at91: implement suspend/resume for the PMC irqchip Boris Brezillon
2015-02-27 15:09 ` [PATCH 5/6] watchdog: at91sam9: request the irq with IRQF_NO_SUSPEND Boris Brezillon
2015-02-27 15:09 ` [PATCH 6/6] tty: serial: atmel: rework interrupt and wakeup handling Boris Brezillon
2015-02-27 22:33 ` [PATCH 0/6] ARM: at91: fix irq_pm_install_action WARNING Rafael J. Wysocki
2015-03-02  9:05   ` Boris Brezillon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).