Skip to contentSkip to conversation

serial: omap: Add support for optional wake-up interrupt

PATCH New Tony Lindgren 3 participants 8 messages Oct 17, 2013 – Oct 22, 2013 TTY layer and serial drivers

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

With the recent pinctrl-single changes, omaps can treat wake-up events from deeper idle states as interrupts.

There's a separate "io chain" controller on most omaps that stays enabled when the device hits off-idle and the regular interrupt controller is powered off.

Let's add support for the optional second interrupt for wake-up events. And then serial-omap can manage the wake-up interrupt from it's runtime PM calls to avoid spurious interrupts during runtime.

Note that the wake interrupt is board specific as it uses the UART RX pin, and for omap3, there are six pin options for UART3 RX pin.

Also Note that the legacy platform based booting handles the wake-ups in the legacy mux driver and does not need to pass the wake-up interrupt to the driver.

And finally, to pass the wake-up interrupt in the dts file, either interrupt-map or the pending interrupts-extended property needs to be passed. It's probably best to use interrupts-extended when it's available.

Cc Felipe Balbi <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Roger Quadros <[email protected]>
drivers/tty/serial/omap-serial.c +62 -17
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_data/serial-omap.h>
@@ -134,7 +135,7 @@ struct uart_omap_port {
 	struct uart_port	port;
 	struct uart_omap_dma	uart_dma;
 	struct device		*dev;
-
+	int			irqs[2];
 	unsigned char		ier;
 	unsigned char		lcr;
 	unsigned char		mcr;
@@ -176,6 +177,8 @@ struct uart_omap_port {
 };
 
 #define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
+#define omap_uartirq		(up->irqs[0])
+#define omap_wakeirq		(up->irqs[1])
 
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
@@ -214,10 +217,23 @@ static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
 	return pdata->get_context_loss_count(up->dev);
 }
 
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+				       bool enable)
+{
+	if (!omap_wakeirq)
+		return;
+
+	if (enable)
+		enable_irq(omap_wakeirq);
+	else
+		disable_irq(omap_wakeirq);
+}
+
 static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
 {
 	struct omap_uart_port_info *pdata = dev_get_platdata(up->dev);
 
+	serial_omap_enable_wakeirq(up, enable);
 	if (!pdata || !pdata->enable_wakeup)
 		return;
 
@@ -689,15 +705,25 @@ static int serial_omap_startup(struct uart_port *port)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned long flags = 0;
-	int retval;
+	int i, retval;
 
 	/*
-	 * Allocate the IRQ
+	 * Allocate the IRQs, the second IRQ is the optional wakeirq
 	 */
-	retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags,
-				up->name, up);
-	if (retval)
-		return retval;
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++) {
+		if (i == 1 && !omap_wakeirq) {
+			dev_info(up->port.dev, "no wakeirq for uart%d\n",
+				 up->port.line);
+			break;
+		}
+		retval = devm_request_irq(up->port.dev, up->irqs[i],
+					  serial_omap_irq, up->port.irqflags,
+					  up->name, up);
+		if (retval)
+			return retval;
+	}
+	if (omap_wakeirq)
+		disable_irq(omap_wakeirq);
 
 	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);
 
@@ -757,6 +783,7 @@ static void serial_omap_shutdown(struct uart_port *port)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned long flags = 0;
+	int i;
 
 	dev_dbg(up->port.dev, "serial_omap_shutdown+%d\n", up->port.line);
 
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);
 }
 
 static void serial_omap_uart_qos_work(struct work_struct *work)
@@ -1570,13 +1600,25 @@ static int serial_omap_probe_rs485(struct uart_omap_port *up,
 static int serial_omap_probe(struct platform_device *pdev)
 {
 	struct uart_omap_port	*up;
-	struct resource		*mem, *irq;
+	struct resource		*mem, *irq = NULL;
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
-	int ret;
+	int of_irqs[2], ret, i;
 
+	/* The optional wakeirq may be specified in the board dts file */
 	if (pdev->dev.of_node) {
+		for (i = 0; i < ARRAY_SIZE(up->irqs); i++) {
+			of_irqs[i] = irq_of_parse_and_map(pdev->dev.of_node, i);
+			if (i == 0 && !of_irqs[i])
+				return -EPROBE_DEFER;
+		}
 		omap_up_info = of_get_uart_port_info(&pdev->dev);
 		pdev->dev.platform_data = omap_up_info;
+	} else {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		if (!irq) {
+			dev_err(&pdev->dev, "no irq resource?\n");
+			return -ENODEV;
+		}
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1585,12 +1627,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!irq) {
-		dev_err(&pdev->dev, "no irq resource?\n");
-		return -ENODEV;
-	}
-
 	if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
 				pdev->dev.driver->name)) {
 		dev_err(&pdev->dev, "memory region already claimed\n");
@@ -1612,6 +1648,16 @@ static int serial_omap_probe(struct platform_device *pdev)
 	if (!up)
 		return -ENOMEM;
 
+	if (pdev->dev.of_node) {
+		up->port.irq = of_irqs[0];
+		for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+			up->irqs[i] = of_irqs[i];
+	} else {
+		up->port.irq = irq->start;
+		up->irqs[0] = irq->start;
+		up->irqs[1] = 0;
+	}
+
 	if (gpio_is_valid(omap_up_info->DTR_gpio) &&
 	    omap_up_info->DTR_present) {
 		up->DTR_gpio = omap_up_info->DTR_gpio;
@@ -1624,7 +1670,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.dev = &pdev->dev;
 	up->port.type = PORT_OMAP;
 	up->port.iotype = UPIO_MEM;
-	up->port.irq = irq->start;
 
 	up->port.regshift = 2;
 	up->port.fifosize = 64;
#1 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Hi,

Quoted 25 lines · Tony Lindgren

With the recent pinctrl-single changes, omaps can treat wake-up events from deeper idle states as interrupts.

There's a separate "io chain" controller on most omaps that stays enabled when the device hits off-idle and the regular interrupt controller is powered off.

Let's add support for the optional second interrupt for wake-up events. And then serial-omap can manage the wake-up interrupt from it's runtime PM calls to avoid spurious interrupts during runtime.

Note that the wake interrupt is board specific as it uses the UART RX pin, and for omap3, there are six pin options for UART3 RX pin.

Also Note that the legacy platform based booting handles the wake-ups in the legacy mux driver and does not need to pass the wake-up interrupt to the driver.

And finally, to pass the wake-up interrupt in the dts file, either interrupt-map or the pending interrupts-extended property needs to be passed. It's probably best to use interrupts-extended when it's available.

Quoted 5 lines
Cc Felipe Balbi <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Roger Quadros <[email protected]>

looks good, minor nits below

Quoted 29 lines
drivers/tty/serial/omap-serial.c +4 -1
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_data/serial-omap.h>
@@ -134,7 +135,7 @@ struct uart_omap_port {
 	struct uart_port	port;
 	struct uart_omap_dma	uart_dma;
 	struct device		*dev;
-
+	int			irqs[2];
 	unsigned char		ier;
 	unsigned char		lcr;
 	unsigned char		mcr;
@@ -176,6 +177,8 @@ struct uart_omap_port {
 };
 
 #define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
+#define omap_uartirq		(up->irqs[0])
+#define omap_wakeirq		(up->irqs[1])
 
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 

drivers/tty/serial/omap-serial.c static int serial_omap_get_context_loss_count(struct uart_omap_port *up) · line 224

Quoted 8 lines
drivers/tty/serial/omap-serial.c +5 -0
@@ -214,10 +217,23 @@ static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
 	return pdata->get_context_loss_count(up->dev);
 }
 
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+				       bool enable)
+{
+	if (!omap_wakeirq)
+		return;

can we drop the pointless obfuscation ?

Quoted 14 lines
drivers/tty/serial/omap-serial.c +2 -5
@@ -689,15 +705,25 @@ static int serial_omap_startup(struct uart_port *port)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned long flags = 0;
-	int retval;
+	int i, retval;
 
 	/*
-	 * Allocate the IRQ
+	 * Allocate the IRQs, the second IRQ is the optional wakeirq
 	 */
-	retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags,
-				up->name, up);
-	if (retval)

drivers/tty/serial/omap-serial.c static int serial_omap_startup(struct uart_port *port) · line 721

Quoted 10 lines
drivers/tty/serial/omap-serial.c +9 -1
-		return retval;
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++) {
+		if (i == 1 && !omap_wakeirq) {
+			dev_info(up->port.dev, "no wakeirq for uart%d\n",
+				 up->port.line);
+			break;
+		}
+		retval = devm_request_irq(up->port.dev, up->irqs[i],
+					  serial_omap_irq, up->port.irqflags,
+					  up->name, up);

conversion to devm_* should be done as another patch, it seems.

drivers/tty/serial/omap-serial.c static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
drivers/tty/serial/omap-serial.c +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131018/f6a8e99a/attachment-0001.sig>
#2 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Quoted 32 lines · Felipe Balbi
Quoted 32 lines
drivers/tty/serial/omap-serial.c +4 -1
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_data/serial-omap.h>
@@ -134,7 +135,7 @@ struct uart_omap_port {
 	struct uart_port	port;
 	struct uart_omap_dma	uart_dma;
 	struct device		*dev;
-
+	int			irqs[2];
 	unsigned char		ier;
 	unsigned char		lcr;
 	unsigned char		mcr;
@@ -176,6 +177,8 @@ struct uart_omap_port {
 };
 
 #define to_uart_omap_port(p)	((container_of((p), struct uart_omap_port, port)))
+#define omap_uartirq		(up->irqs[0])
+#define omap_wakeirq		(up->irqs[1])
 
 static struct uart_omap_port *ui[OMAP_MAX_HSUART_PORTS];
 
@@ -214,10 +217,23 @@ static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
 	return pdata->get_context_loss_count(up->dev);
 }
 
Quoted 7 lines

drivers/tty/serial/omap-serial.c static int serial_omap_get_context_loss_count(struct uart_omap_port *up) · line 224

Quoted 5 lines
drivers/tty/serial/omap-serial.c +5 -0
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+				       bool enable)
+{
+	if (!omap_wakeirq)
+		return;

can we drop the pointless obfuscation ?

Hmm care to specify what exactly you have in mind here? The wakeirq is optional so we need to check for it.

Do you mean get rid of serial_omap_enable_wakeirq() or get rid of the defines for omap_uartirq and omap_wakeirq?

Quoted 18 lines
Quoted 18 lines
drivers/tty/serial/omap-serial.c +5 -6
@@ -689,15 +705,25 @@ static int serial_omap_startup(struct uart_port *port)
 {
 	struct uart_omap_port *up = to_uart_omap_port(port);
 	unsigned long flags = 0;
-	int retval;
+	int i, retval;
 
 	/*
-	 * Allocate the IRQ
+	 * Allocate the IRQs, the second IRQ is the optional wakeirq
 	 */
-	retval = request_irq(up->port.irq, serial_omap_irq, up->port.irqflags,
-				up->name, up);
-	if (retval)
-		return retval;
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++) {
+		if (i == 1 && !omap_wakeirq) {
+			dev_info(up->port.dev, "no wakeirq for uart%d\n",
Quoted 8 lines

drivers/tty/serial/omap-serial.c static int serial_omap_startup(struct uart_port *port) · line 721

Quoted 6 lines
drivers/tty/serial/omap-serial.c +6 -0
+				 up->port.line);
+			break;
+		}
+		retval = devm_request_irq(up->port.dev, up->irqs[i],
+					  serial_omap_irq, up->port.irqflags,
+					  up->name, up);

conversion to devm_* should be done as another patch, it seems.

OK, I'll separate the devm_* changes from this patch. It's seems that we're currently mostly devm_*, so let's do that first.

Quoted 11 lines

drivers/tty/serial/omap-serial.c static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
drivers/tty/serial/omap-serial.c +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

So it seems, startup and shutdown are managed by serial_core and that's what at least clps711x.c serial driver is doing.

Regards,

Tony

#3 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Quoted 13 lines · Tony Lindgren
Quoted 11 lines · Felipe Balbi

drivers/tty/serial/omap-serial.c static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
drivers/tty/serial/omap-serial.c +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

Quoted 2 lines

So it seems, startup and shutdown are managed by serial_core and that's what at least clps711x.c serial driver is doing.

And that means devm_* in this case does not really help us here..

I guess we could keep the IRQ requested from probe, but there's probably a reason why it's done in startup/shutdown.

So I'll just drop the devm_* changes for now.

Regards,

Tony

#4 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Quoted 17 lines · Tony Lindgren
Quoted 13 lines · Tony Lindgren
Quoted 11 lines · Felipe Balbi

Review static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
(quoted diff) +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

Quoted 2 lines

So it seems, startup and shutdown are managed by serial_core and that's what at least clps711x.c serial driver is doing.

Quoted 7 lines

And that means devm_* in this case does not really help us here..

I guess we could keep the IRQ requested from probe, but there's probably a reason why it's done in startup/shutdown.

So I'll just drop the devm_* changes for now.

Here's an updated simplified version. I also got rid of the for loops as the wake-up interrupt is optional and it made the code a bit of a pain to read.

Regards,

Tony

8< ----------------------------------------
From: Tony Lindgren <[email protected]>
Date: Wed, 16 Oct 2013 10:27:28 -0700
Subject: [PATCH] serial: omap: Add support for optional wake-up interrupt

With the recent pinctrl-single changes, omaps can treat wake-up events from deeper idle states as interrupts.

There's a separate "io chain" controller on most omaps that stays enabled when the device hits off-idle and the regular interrupt controller is powered off.

Let's add support for the optional second interrupt for wake-up events. And then serial-omap can manage the wake-up interrupt from it's runtime PM calls to avoid spurious interrupts during runtime.

Note that the wake interrupt is board specific as it uses the UART RX pin, and for omap3, there are six pin options for UART3 RX pin.

Also Note that the legacy platform based booting handles the wake-ups in the legacy mux driver and does not need to pass the wake-up interrupt to the driver.

And finally, to pass the wake-up interrupt in the dts file, either interrupt-map or the pending interrupts-extended property needs to be passed. It's probably best to use interrupts-extended when it's available.

Cc Felipe Balbi <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Roger Quadros <[email protected]>
drivers/tty/serial/omap-serial.c +46 -8
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_data/serial-omap.h>
@@ -134,6 +135,7 @@ struct uart_omap_port {
 	struct uart_port	port;
 	struct uart_omap_dma	uart_dma;
 	struct device		*dev;
+	int			wakeirq;
 
 	unsigned char		ier;
 	unsigned char		lcr;
@@ -214,10 +216,23 @@ static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
 	return pdata->get_context_loss_count(up->dev);
 }
 
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+				       bool enable)
+{
+	if (!up->wakeirq)
+		return;
+
+	if (enable)
+		enable_irq(up->wakeirq);
+	else
+		disable_irq(up->wakeirq);
+}
+
 static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
 {
 	struct omap_uart_port_info *pdata = dev_get_platdata(up->dev);
 
+	serial_omap_enable_wakeirq(up, enable);
 	if (!pdata || !pdata->enable_wakeup)
 		return;
 
@@ -699,6 +714,20 @@ static int serial_omap_startup(struct uart_port *port)
 	if (retval)
 		return retval;
 
+	/* Optional wake-up IRQ */
+	if (up->wakeirq) {
+		retval = request_irq(up->wakeirq, serial_omap_irq,
+				     up->port.irqflags, up->name, up);
+		if (retval) {
+			free_irq(up->port.irq, up);
+			return retval;
+		}
+		disable_irq(up->wakeirq);
+	} else {
+		dev_info(up->port.dev, "no wakeirq for uart%d\n",
+			 up->port.line);
+	}
+
 	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);
 
 	pm_runtime_get_sync(up->dev);
@@ -787,6 +816,8 @@ static void serial_omap_shutdown(struct uart_port *port)
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
 	free_irq(up->port.irq, up);
+	if (up->wakeirq)
+		free_irq(up->wakeirq, up);
 }
 
 static void serial_omap_uart_qos_work(struct work_struct *work)
@@ -1572,11 +1603,23 @@ static int serial_omap_probe(struct platform_device *pdev)
 	struct uart_omap_port	*up;
 	struct resource		*mem, *irq;
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
-	int ret;
+	int ret, uartirq = 0, wakeirq = 0;
 
+	/* The optional wakeirq may be specified in the board dts file */
 	if (pdev->dev.of_node) {
+		uartirq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+		if (!uartirq)
+			return -EPROBE_DEFER;
+		wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
 		omap_up_info = of_get_uart_port_info(&pdev->dev);
 		pdev->dev.platform_data = omap_up_info;
+	} else {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		if (!irq) {
+			dev_err(&pdev->dev, "no irq resource?\n");
+			return -ENODEV;
+		}
+		uartirq = irq->start;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1585,12 +1628,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!irq) {
-		dev_err(&pdev->dev, "no irq resource?\n");
-		return -ENODEV;
-	}
-
 	if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
 				pdev->dev.driver->name)) {
 		dev_err(&pdev->dev, "memory region already claimed\n");
@@ -1624,7 +1661,8 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.dev = &pdev->dev;
 	up->port.type = PORT_OMAP;
 	up->port.iotype = UPIO_MEM;
-	up->port.irq = irq->start;
+	up->port.irq = uartirq;
+	up->wakeirq = wakeirq;
 
 	up->port.regshift = 2;
 	up->port.fifosize = 64;
#5 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Quoted 65 lines · Tony Lindgren
Quoted 17 lines · Tony Lindgren
Quoted 13 lines · Tony Lindgren
Quoted 11 lines · Felipe Balbi

Review static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
(quoted diff) +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

Quoted 2 lines

So it seems, startup and shutdown are managed by serial_core and that's what at least clps711x.c serial driver is doing.

Quoted 7 lines

And that means devm_* in this case does not really help us here..

I guess we could keep the IRQ requested from probe, but there's probably a reason why it's done in startup/shutdown.

So I'll just drop the devm_* changes for now.

Here's an updated simplified version. I also got rid of the for loops as the wake-up interrupt is optional and it made the code a bit of a pain to read.

Regards,

Tony

8< ----------------------------------------
From: Tony Lindgren <[email protected]>
Date: Wed, 16 Oct 2013 10:27:28 -0700
Subject: [PATCH] serial: omap: Add support for optional wake-up interrupt

With the recent pinctrl-single changes, omaps can treat wake-up events from deeper idle states as interrupts.

There's a separate "io chain" controller on most omaps that stays enabled when the device hits off-idle and the regular interrupt controller is powered off.

Let's add support for the optional second interrupt for wake-up events. And then serial-omap can manage the wake-up interrupt from it's runtime PM calls to avoid spurious interrupts during runtime.

Note that the wake interrupt is board specific as it uses the UART RX pin, and for omap3, there are six pin options for UART3 RX pin.

Also Note that the legacy platform based booting handles the wake-ups in the legacy mux driver and does not need to pass the wake-up interrupt to the driver.

And finally, to pass the wake-up interrupt in the dts file, either interrupt-map or the pending interrupts-extended property needs to be passed. It's probably best to use interrupts-extended when it's available.

Quoted 5 lines
Cc Felipe Balbi <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Roger Quadros <[email protected]>

Looks good to me. So,

Quoted 122 lines
drivers/tty/serial/omap-serial.c +46 -8
@@ -39,6 +39,7 @@
 #include <linux/irq.h>
 #include <linux/pm_runtime.h>
 #include <linux/of.h>
+#include <linux/of_irq.h>
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <linux/platform_data/serial-omap.h>
@@ -134,6 +135,7 @@ struct uart_omap_port {
 	struct uart_port	port;
 	struct uart_omap_dma	uart_dma;
 	struct device		*dev;
+	int			wakeirq;
 
 	unsigned char		ier;
 	unsigned char		lcr;
@@ -214,10 +216,23 @@ static int serial_omap_get_context_loss_count(struct uart_omap_port *up)
 	return pdata->get_context_loss_count(up->dev);
 }
 
+static inline void serial_omap_enable_wakeirq(struct uart_omap_port *up,
+				       bool enable)
+{
+	if (!up->wakeirq)
+		return;
+
+	if (enable)
+		enable_irq(up->wakeirq);
+	else
+		disable_irq(up->wakeirq);
+}
+
 static void serial_omap_enable_wakeup(struct uart_omap_port *up, bool enable)
 {
 	struct omap_uart_port_info *pdata = dev_get_platdata(up->dev);
 
+	serial_omap_enable_wakeirq(up, enable);
 	if (!pdata || !pdata->enable_wakeup)
 		return;
 
@@ -699,6 +714,20 @@ static int serial_omap_startup(struct uart_port *port)
 	if (retval)
 		return retval;
 
+	/* Optional wake-up IRQ */
+	if (up->wakeirq) {
+		retval = request_irq(up->wakeirq, serial_omap_irq,
+				     up->port.irqflags, up->name, up);
+		if (retval) {
+			free_irq(up->port.irq, up);
+			return retval;
+		}
+		disable_irq(up->wakeirq);
+	} else {
+		dev_info(up->port.dev, "no wakeirq for uart%d\n",
+			 up->port.line);
+	}
+
 	dev_dbg(up->port.dev, "serial_omap_startup+%d\n", up->port.line);
 
 	pm_runtime_get_sync(up->dev);
@@ -787,6 +816,8 @@ static void serial_omap_shutdown(struct uart_port *port)
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
 	free_irq(up->port.irq, up);
+	if (up->wakeirq)
+		free_irq(up->wakeirq, up);
 }
 
 static void serial_omap_uart_qos_work(struct work_struct *work)
@@ -1572,11 +1603,23 @@ static int serial_omap_probe(struct platform_device *pdev)
 	struct uart_omap_port	*up;
 	struct resource		*mem, *irq;
 	struct omap_uart_port_info *omap_up_info = dev_get_platdata(&pdev->dev);
-	int ret;
+	int ret, uartirq = 0, wakeirq = 0;
 
+	/* The optional wakeirq may be specified in the board dts file */
 	if (pdev->dev.of_node) {
+		uartirq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+		if (!uartirq)
+			return -EPROBE_DEFER;
+		wakeirq = irq_of_parse_and_map(pdev->dev.of_node, 1);
 		omap_up_info = of_get_uart_port_info(&pdev->dev);
 		pdev->dev.platform_data = omap_up_info;
+	} else {
+		irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+		if (!irq) {
+			dev_err(&pdev->dev, "no irq resource?\n");
+			return -ENODEV;
+		}
+		uartirq = irq->start;
 	}
 
 	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -1585,12 +1628,6 @@ static int serial_omap_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!irq) {
-		dev_err(&pdev->dev, "no irq resource?\n");
-		return -ENODEV;
-	}
-
 	if (!devm_request_mem_region(&pdev->dev, mem->start, resource_size(mem),
 				pdev->dev.driver->name)) {
 		dev_err(&pdev->dev, "memory region already claimed\n");
@@ -1624,7 +1661,8 @@ static int serial_omap_probe(struct platform_device *pdev)
 	up->port.dev = &pdev->dev;
 	up->port.type = PORT_OMAP;
 	up->port.iotype = UPIO_MEM;
-	up->port.irq = irq->start;
+	up->port.irq = uartirq;
+	up->wakeirq = wakeirq;
 
 	up->port.regshift = 2;
 	up->port.fifosize = 64;
#6 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

In reply to Tony Lindgren
Quoted 65 lines · Tony Lindgren
Quoted 17 lines · Tony Lindgren
Quoted 13 lines · Tony Lindgren
Quoted 11 lines · Felipe Balbi

Review static void serial_omap_shutdown(struct uart_port *port) · line 819

Quoted 9 lines
(quoted diff) +4 -1
@@ -786,7 +813,10 @@ static void serial_omap_shutdown(struct uart_port *port)
 
 	pm_runtime_mark_last_busy(up->dev);
 	pm_runtime_put_autosuspend(up->dev);
-	free_irq(up->port.irq, up);
+
+	for (i = 0; i < ARRAY_SIZE(up->irqs); i++)
+		if (up->irqs[i])
+			devm_free_irq(up->port.dev, up->irqs[i], up);

do you need this at all if you're using devm_* ?

Quoted 2 lines

So it seems, startup and shutdown are managed by serial_core and that's what at least clps711x.c serial driver is doing.

Quoted 7 lines

And that means devm_* in this case does not really help us here..

I guess we could keep the IRQ requested from probe, but there's probably a reason why it's done in startup/shutdown.

So I'll just drop the devm_* changes for now.

Here's an updated simplified version. I also got rid of the for loops as the wake-up interrupt is optional and it made the code a bit of a pain to read.

Regards,

Tony

8< ----------------------------------------
From: Tony Lindgren <[email protected]>
Date: Wed, 16 Oct 2013 10:27:28 -0700
Subject: [PATCH] serial: omap: Add support for optional wake-up interrupt

With the recent pinctrl-single changes, omaps can treat wake-up events from deeper idle states as interrupts.

There's a separate "io chain" controller on most omaps that stays enabled when the device hits off-idle and the regular interrupt controller is powered off.

Let's add support for the optional second interrupt for wake-up events. And then serial-omap can manage the wake-up interrupt from it's runtime PM calls to avoid spurious interrupts during runtime.

Note that the wake interrupt is board specific as it uses the UART RX pin, and for omap3, there are six pin options for UART3 RX pin.

Also Note that the legacy platform based booting handles the wake-ups in the legacy mux driver and does not need to pass the wake-up interrupt to the driver.

And finally, to pass the wake-up interrupt in the dts file, either interrupt-map or the pending interrupts-extended property needs to be passed. It's probably best to use interrupts-extended when it's available.

Quoted 5 lines
Cc Felipe Balbi <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Linus Walleij <[email protected]>
Cc Roger Quadros <[email protected]>

looks much nicer

Signature · 8 lines
balbi
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131021/8a88a95b/attachment-0001.sig>
#7 lore raw

PATCH 1/1 serial: omap: Add support for optional wake-up interrupt

Quoted 4 lines · Felipe Balbi

looks much nicer

Thanks, will repost with acks to make Greg's life easier.

Regards,

Tony