Re: [PATCH v2] modify the pl011 driver to let it work as wakeup source

[Daniel Lezcano <daniel.lezcano@linaro.org>]

Hi Zhaoyang,

added Sudeep in Cc.

You should run checkpatch.pl on this patch. There are 11 errors and 3 
warnings.

On 09/01/2015 11:35 AM, Zhaoyang Huang wrote:
> the commit use the latest dev_pm_set_wake_irq API instead
> of the enable_irq_wake and IRQF_NO_SUSPEND to configure the
> ttyAMA device to work as the wakeup source
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@linaro.org>
> ---
>   drivers/tty/serial/amba-pl011.c |   55 ++++++++++++++++++++++++++++++++++++---
>   1 file changed, 52 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/tty/serial/amba-pl011.c b/drivers/tty/serial/amba-pl011.c
> index 50cf5b1..33a479a 100644
> --- a/drivers/tty/serial/amba-pl011.c
> +++ b/drivers/tty/serial/amba-pl011.c
> @@ -59,6 +59,10 @@
>   #include <linux/sizes.h>
>   #include <linux/io.h>
>   #include <linux/acpi.h>
> +#include <linux/suspend.h>
> +#include <linux/pm_wakeirq.h>
> +#include "../../base/power/power.h"

I don't think it is usual to do this kind of inclusion. Why do you need 
to do that ?

> +#include <linux/irq.h>
>
>   #define UART_NR			14
>
> @@ -127,6 +131,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 +1381,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();

I don't think this is needed.

> +
>   			}
>   			if (status & (UART011_DSRMIS|UART011_DCDMIS|
>   				      UART011_CTSMIS|UART011_RIMIS))
> @@ -2353,11 +2362,29 @@ static int pl011_register_port(struct uart_amba_port *uap)
>   	return ret;
>   }
>
> +struct uart_match {
> +	struct uart_port *port;
> +	struct uart_driver *driver;
> +};
> +
> +static int match_uart_port(struct device * dev, void * data)
> +{
> +	struct uart_match *match = data;
> +
> +	dev_t devt = MKDEV(match->driver->major, match->driver->minor) +
> +		match->port->line;
> +
> +	pr_info("the match data of ttyAMA0 is %d %d\n",(int)devt,(int)dev->devt);
> +
> +	return dev->devt == devt; /* Actually, only one tty per port */
> +}
>   static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>   {
>   	struct uart_amba_port *uap;
>   	struct vendor_data *vendor = id->data;
>   	int portnr, ret;
> +	struct device * uart_dev;
> +	struct uart_match match;
>
>   	portnr = pl011_find_free_port();
>   	if (portnr < 0)
> @@ -2387,7 +2414,17 @@ static int pl011_probe(struct amba_device *dev, const struct amba_id *id)
>
>   	amba_set_drvdata(dev, uap);
>
> -	return pl011_register_port(uap);
> +	ret = pl011_register_port(uap);
> +
> +	match.port = &uap->port;
> +	match.driver = &amba_reg;
> +
> +	uart_dev = device_find_child(&dev->dev, &match,match_uart_port);
> +
> +       device_init_wakeup(uart_dev, true);
> +       dev_pm_set_wake_irq(uart_dev, uap->port.irq);

I am sending a patch to move device_init_wakeup into dev_pm_set_wake_irq.


-- 
  <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