[PATCH] modify pl011 driver to work as wakeup source

[PATCH] modify pl011 driver to work as wakeup source

[Zhaoyang Huang]

add IRQF_NO_SUSPEND flag when requiring irq line and call
the pm_system_wakeup when RX interrupt happens

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@linaro.org>
---
 drivers/tty/serial/amba-pl011.c |   24 +++++++++++++++++++++---
 1 file changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
index 50cf5b1..6285819 100644
--- a/drivers/tty/serial/amba-pl011.c
+++ b/drivers/tty/serial/amba-pl011.c
@@ -59,6 +59,7 @@
 #include <linux/sizes.h>
 #include <linux/io.h>
 #include <linux/acpi.h>
+#include <linux/suspend.h>
 
 #define UART_NR			14
 
@@ -127,6 +128,7 @@ static struct vendor_data vendor_st = {
 	.get_fifosize		= get_fifosize_st,
 };
 
+static int is_suspended = 0;
 /* Deals with DMA transactions */
 
 struct pl011_sgbuf {
@@ -1376,6 +1378,10 @@ static irqreturn_t pl011_int(int irq, void *dev_id)
 					pl011_dma_rx_irq(uap);
 				else
 					pl011_rx_chars(uap);
+
+				if(is_suspended)
+					pm_system_wakeup();
+
 			}
 			if (status & (UART011_DSRMIS|UART011_DCDMIS|
 				      UART011_CTSMIS|UART011_RIMIS))
@@ -1585,7 +1591,7 @@ static int pl011_allocate_irq(struct uart_amba_port *uap)
 {
 	writew(uap->im, uap->port.membase + UART011_IMSC);
 
-	return request_irq(uap->port.irq, pl011_int, 0, "uart-pl011", uap);
+	return request_irq(uap->port.irq, pl011_int, IRQF_NO_SUSPEND, "uart-pl011", uap);
 }
 
 /*
@@ -2403,21 +2409,33 @@ static int pl011_remove(struct amba_device *dev)
 static int pl011_suspend(struct device *dev)
 {
 	struct uart_amba_port *uap = dev_get_drvdata(dev);
+	int ret = 0;
 
 	if (!uap)
 		return -EINVAL;
 
-	return uart_suspend_port(&amba_reg, &uap->port);
+	ret =uart_suspend_port(&amba_reg, &uap->port);
+
+	if(!ret)
+		is_suspended = 1;
+
+	return ret;
 }
 
 static int pl011_resume(struct device *dev)
 {
 	struct uart_amba_port *uap = dev_get_drvdata(dev);
+	int ret = 0;
 
 	if (!uap)
 		return -EINVAL;
 
-	return uart_resume_port(&amba_reg, &uap->port);
+	ret = uart_resume_port(&amba_reg, &uap->port);
+
+	if(!ret)
+		is_suspended = 0;
+
+	return ret;
 }
 #endif
 
-- 
1.7.9.5

Re: [PATCH] modify pl011 driver to work as wakeup source

[Daniel Lezcano]

On 08/14/2015 12:24 PM, Zhaoyang Huang wrote:
> add IRQF_NO_SUSPEND flag when requiring irq line and call
> the pm_system_wakeup when RX interrupt happens
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@linaro.org>

Did you look at the 'dev_pm_set_wake_irq' function ?



-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] modify pl011 driver to work as wakeup source

[Amit Kucheria]

On Wed, Aug 26, 2015 at 6:32 PM, Zhaoyang Huang
<zhaoyang.huang@linaro.org> wrote:
> The following functions don't work on pl011 for setting it as wakeup irq.
> device_init_wakeup(dev, true); dev_pm_set_wake_irq(dev, irq);

Can you elaborate how it doesn't work?

>
> On 14 August 2015 at 18:58, Daniel Lezcano <daniel.lezcano@linaro.org>
> wrote:
>>
>> On 08/14/2015 12:24 PM, Zhaoyang Huang wrote:
>>>
>>> add IRQF_NO_SUSPEND flag when requiring irq line and call
>>> the pm_system_wakeup when RX interrupt happens
>>>
>>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@linaro.org>
>>
>>
>> Did you look at the 'dev_pm_set_wake_irq' function ?
>>

Re: [PATCH] modify pl011 driver to work as wakeup source

[Daniel Lezcano]

On 08/26/2015 03:02 PM, Zhaoyang Huang wrote:
> The following functions don't work on pl011 for setting it as wakeup irq.
> device_init_wakeup(dev, true);	dev_pm_set_wake_irq(dev, irq);

Hi Zhaoyang,

if I understood correctly, dev_pm_set_wake_irq is the new API to set a 
wake up device up. You are the first one using it, so perhaps there is 
something missing.

I recommend you have a look at the slides [1] showed at the LPC2015.

I added Sudeep so he can give more informations about that.

   -- Daniel

[1] 
https://linuxplumbersconf.org/2015/ocw//system/presentations/3051/original/wakeup_config.pdf

>
> On 14 August 2015 at 18:58, Daniel Lezcano <daniel.lezcano@linaro.org
> <mailto:daniel.lezcano@linaro.org>> wrote:
>
>     On 08/14/2015 12:24 PM, Zhaoyang Huang wrote:
>
>         add IRQF_NO_SUSPEND flag when requiring irq line and call
>         the pm_system_wakeup when RX interrupt happens
>
>         Signed-off-by: Zhaoyang Huang <zhaoyang.huang@linaro.org
>         <mailto:zhaoyang.huang@linaro.org>>
>
>
>     Did you look at the 'dev_pm_set_wake_irq' function ?
>
>
>
>     --
>       <http://www.linaro.org/> Linaro.org │ Open source software for ARM
>     SoCs
>
>     Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
>     <http://twitter.com/#!/linaroorg
>     <http://twitter.com/#%21/linaroorg>> Twitter |
>     <http://www.linaro.org/linaro-blog/> Blog
>
>


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH] modify pl011 driver to work as wakeup source

[Sudeep Holla]

On 26/08/15 15:00, Daniel Lezcano wrote:
> On 08/26/2015 03:02 PM, Zhaoyang Huang wrote:
>> The following functions don't work on pl011 for setting it as wakeup irq.
>> device_init_wakeup(dev, true);	dev_pm_set_wake_irq(dev, irq);

I did a quick hack in pl011 driver and it works fine to me, though I am
not sure if that feature is an upstream material as intended by your
patch. I don't this PL011 especially when used as a console needs to be
configured as wakeup(may be when used for bluetooth ?)

>
> Hi Zhaoyang,
>
> if I understood correctly, dev_pm_set_wake_irq is the new API to set a
> wake up device up. You are the first one using it, so perhaps there is
> something missing.
>

I had tested those APIs with RTC a while ago, it work just fine.

> I recommend you have a look at the slides [1] showed at the LPC2015.
>
> I added Sudeep so he can give more informations about that.
>

Thanks Daniel for the add.

Regards,
Sudeep

Re: [PATCH] modify pl011 driver to work as wakeup source

[Sudeep Holla]

On 27/08/15 09:25, Zhaoyang Huang wrote:
> Sudeep,
> Have you check if the pl011 can wakeup the system. I notice that the
> calling of dev_pm_set_wake_irq can set the node of <device>/power/wakeup
> to be enabled but can not wakeup the system.
>

Yes I did, and it does wakeup the system.

> By basic debugging, I find that when calling of dev_pm_set_wake_irq
> within pl011_probe, the real device which we operate is the
> /sys/devices/platform/7ff80000.uart instead of the ttyAMA0.
>

No, if you are expecting the wakeup by sending some character on the
console, then it's ttyAMA0 which uses the pl011 UART. May be you can
also generate wakeup by toggling RTS/CTS hardware flow signals.

> By checking the sysfs, I find that the value of
> "sys/devices/platform/7ff80000.uart/power/wakeup" is "enabled", which
> can also prove above information. However, the value of
> "dev.power.wakeup->wakeirq->irq" for uart is 24 which is the same with
> ttyAMA0.

Ah, you need to use /sys/class/tty/ttyAMA0/power/wakeup

Regards,
Sudeep

Re: [PATCH] modify pl011 driver to work as wakeup source

[Sudeep Holla]

On 28/08/15 10:43, Zhaoyang Huang wrote:
> I would like loop Tony for more discussion.
>
> Hi Tony,
> I met a problem when using your latest API for setting wakeup source.
> The basic phenomenon is the ttyAMA0 can not wakeup the whole system even
> if the wakeup node of its </power/wakeup> has been set to be enabled. By
> doing some basic debug, I find that the corresponding isr was set to
> suspend even with above setting have been take effect.
>

I don't quite understand the issue you are facing, but for sure
IRQF_NO_SUSPEND is not the right way to fix this. Please read the
document, it clearly states ".. but does not guarantee that the
interrupt will wake the system from a suspended state, for such cases
it is necessary to use enable_irq_wake()".

dev_pm_set_wake_irq is just new alternative for enable_irq_wake.
I have tried this API with RTC and it works fine, so I don't see any
issue unless you are exploring something new in the code path you are
using this API. I even tried with UART though it was a hack.

So this patch is wrong and you also fails to explain why you need a
console as a wake up source, for just testing ?

Regards,
Sudeep