devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHv2] serial: of-serial: fix up PM ops on no_console_suspend and port type
@ 2014-10-13  7:37 Jingchang Lu
  2014-10-13  9:41 ` Joseph Lo
  0 siblings, 1 reply; 3+ messages in thread
From: Jingchang Lu @ 2014-10-13  7:37 UTC (permalink / raw)
  To: gregkh
  Cc: peter, arnd, linux-serial, linux-kernel, linux-arm-kernel,
	devicetree, Jingchang Lu

This patch fixes commit 2dea53bf57783f243c892e99c10c6921e956aa7e,
"serial: of-serial: add PM suspend/resume support", which disables
the uart clock on suspend, but also causes a hardware hang on register
access if no_console_suspend command line option is used.

Also, not every of_serial device is an 8250 port, so the serial8250
suspend/resume functions should only be applied to a real 8250 port.

Signed-off-by: Jingchang Lu <jingchang.lu@freescale.com>
---
changes in v2:
 add switch selection on uart type.

 drivers/tty/serial/of_serial.c | 42 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index 8bc2563..b525383 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -245,9 +245,24 @@ static int of_serial_suspend(struct device *dev)
 {
 	struct of_serial_info *info = dev_get_drvdata(dev);
 
-	serial8250_suspend_port(info->line);
-	if (info->clk)
-		clk_disable_unprepare(info->clk);
+	switch(info->type) {
+#ifdef CONFIG_SERIAL_8250
+	case PORT_8250 ... PORT_MAX_8250:
+	{
+		struct uart_8250_port port8250;
+		port8250 = serial8250_get_port(info->line);
+
+		serial8250_suspend_port(info->line);
+		if (info->clk &&
+		    (!uart_console(port8250) ||
+		     (uart_console(port8250) && console_suspend_enabled)))
+			clk_disable_unprepare(info->clk);
+		break;
+	}
+#endif
+	default:
+		break;
+	}
 
 	return 0;
 }
@@ -256,10 +271,25 @@ static int of_serial_resume(struct device *dev)
 {
 	struct of_serial_info *info = dev_get_drvdata(dev);
 
-	if (info->clk)
-		clk_prepare_enable(info->clk);
+	switch(info->type) {
+#ifdef CONFIG_SERIAL_8250
+	case PORT_8250 ... PORT_MAX_8250:
+	{
+		struct uart_8250_port port8250;
+		port8250 = serial8250_get_port(info->line);
+
+		if (info->clk &&
+		    (!uart_console(port8250) ||
+		     (uart_console(port8250) && console_suspend_enabled)))
+			clk_prepare_enable(info->clk);
 
-	serial8250_resume_port(info->line);
+		serial8250_resume_port(info->line);
+		break;
+	}
+#endif
+	default:
+		break;
+	}
 
 	return 0;
 }
-- 
1.8.0


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

* Re: [PATCHv2] serial: of-serial: fix up PM ops on no_console_suspend and port type
  2014-10-13  7:37 [PATCHv2] serial: of-serial: fix up PM ops on no_console_suspend and port type Jingchang Lu
@ 2014-10-13  9:41 ` Joseph Lo
       [not found]   ` <543B9E44.109-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Joseph Lo @ 2014-10-13  9:41 UTC (permalink / raw)
  To: Jingchang Lu, gregkh@linuxfoundation.org
  Cc: devicetree@vger.kernel.org, peter@hurleysoftware.com,
	arnd@arndb.de, linux-kernel@vger.kernel.org,
	linux-serial@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org

Hi,

On 10/13/2014 03:37 PM, Jingchang Lu wrote:
> This patch fixes commit 2dea53bf57783f243c892e99c10c6921e956aa7e,
> "serial: of-serial: add PM suspend/resume support", which disables
> the uart clock on suspend, but also causes a hardware hang on register
> access if no_console_suspend command line option is used.
>
> Also, not every of_serial device is an 8250 port, so the serial8250
> suspend/resume functions should only be applied to a real 8250 port.
>
> Signed-off-by: Jingchang Lu <jingchang.lu@freescale.com>
> ---
> changes in v2:
>   add switch selection on uart type.

After apply this, it can't build for me. I still need to apply the fix 
like something below.

diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
index e535f9c1563d..14c1ff4e7816 100644
--- a/drivers/tty/serial/of_serial.c
+++ b/drivers/tty/serial/of_serial.c
@@ -9,6 +9,7 @@
   *  2 of the License, or (at your option) any later version.
   *
   */
+#include <linux/console.h>
  #include <linux/module.h>
  #include <linux/slab.h>
  #include <linux/delay.h>
@@ -248,13 +249,13 @@ static int of_serial_suspend(struct device *dev)
  #ifdef CONFIG_SERIAL_8250
         case PORT_8250 ... PORT_MAX_8250:
         {
-               struct uart_8250_port port8250;
+               struct uart_8250_port *port8250;
                 port8250 = serial8250_get_port(info->line);

                 serial8250_suspend_port(info->line);
                 if (info->clk &&
-                   (!uart_console(port8250) ||
-                    (uart_console(port8250) && console_suspend_enabled)))
+                   (!uart_console(&port8250->port) ||
+                    (uart_console(&port8250->port) && 
console_suspend_enabled)))
                         clk_disable_unprepare(info->clk);
                 break;
         }
@@ -274,12 +275,12 @@ static int of_serial_resume(struct device *dev)
  #ifdef CONFIG_SERIAL_8250
         case PORT_8250 ... PORT_MAX_8250:
         {
-               struct uart_8250_port port8250;
+               struct uart_8250_port *port8250;
                 port8250 = serial8250_get_port(info->line);

                 if (info->clk &&
-                   (!uart_console(port8250) ||
-                    (uart_console(port8250) && console_suspend_enabled)))
+                   (!uart_console(&port8250->port) ||
+                    (uart_console(&port8250->port) && 
console_suspend_enabled)))
                         clk_prepare_enable(info->clk);

                 serial8250_resume_port(info->line);

-Joseph

>
>   drivers/tty/serial/of_serial.c | 42 ++++++++++++++++++++++++++++++++++++------
>   1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/tty/serial/of_serial.c b/drivers/tty/serial/of_serial.c
> index 8bc2563..b525383 100644
> --- a/drivers/tty/serial/of_serial.c
> +++ b/drivers/tty/serial/of_serial.c
> @@ -245,9 +245,24 @@ static int of_serial_suspend(struct device *dev)
>   {
>   	struct of_serial_info *info = dev_get_drvdata(dev);
>
> -	serial8250_suspend_port(info->line);
> -	if (info->clk)
> -		clk_disable_unprepare(info->clk);
> +	switch(info->type) {
> +#ifdef CONFIG_SERIAL_8250
> +	case PORT_8250 ... PORT_MAX_8250:
> +	{
> +		struct uart_8250_port port8250;
> +		port8250 = serial8250_get_port(info->line);
> +
> +		serial8250_suspend_port(info->line);
> +		if (info->clk &&
> +		    (!uart_console(port8250) ||
> +		     (uart_console(port8250) && console_suspend_enabled)))
> +			clk_disable_unprepare(info->clk);
> +		break;
> +	}
> +#endif
> +	default:
> +		break;
> +	}
>
>   	return 0;
>   }
> @@ -256,10 +271,25 @@ static int of_serial_resume(struct device *dev)
>   {
>   	struct of_serial_info *info = dev_get_drvdata(dev);
>
> -	if (info->clk)
> -		clk_prepare_enable(info->clk);
> +	switch(info->type) {
> +#ifdef CONFIG_SERIAL_8250
> +	case PORT_8250 ... PORT_MAX_8250:
> +	{
> +		struct uart_8250_port port8250;
> +		port8250 = serial8250_get_port(info->line);
> +
> +		if (info->clk &&
> +		    (!uart_console(port8250) ||
> +		     (uart_console(port8250) && console_suspend_enabled)))
> +			clk_prepare_enable(info->clk);
>
> -	serial8250_resume_port(info->line);
> +		serial8250_resume_port(info->line);
> +		break;
> +	}
> +#endif
> +	default:
> +		break;
> +	}
>
>   	return 0;
>   }
>

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

* RE: [PATCHv2] serial: of-serial: fix up PM ops on no_console_suspend and port type
       [not found]   ` <543B9E44.109-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
@ 2014-10-13  9:49     ` Jingchang Lu
  0 siblings, 0 replies; 3+ messages in thread
From: Jingchang Lu @ 2014-10-13  9:49 UTC (permalink / raw)
  To: Joseph Lo,
	gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8@public.gmane.org,
	arnd-r2nGTMty4D4@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-serial-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org

My mistake, I will fix it, many thanks.

Best Regards,
Jingchang

>-----Original Message-----
>From: Joseph Lo [mailto:josephl@nvidia.com]
>Sent: Monday, October 13, 2014 5:41 PM
>To: Lu Jingchang-B35083; gregkh@linuxfoundation.org
>Cc: devicetree@vger.kernel.org; peter@hurleysoftware.com; arnd@arndb.de;
>linux-kernel@vger.kernel.org; linux-serial@vger.kernel.org; linux-arm-
>kernel@lists.infradead.org
>Subject: Re: [PATCHv2] serial: of-serial: fix up PM ops on
>no_console_suspend and port type
>
>Hi,
>
>On 10/13/2014 03:37 PM, Jingchang Lu wrote:
>> This patch fixes commit 2dea53bf57783f243c892e99c10c6921e956aa7e,
>> "serial: of-serial: add PM suspend/resume support", which disables
>> the uart clock on suspend, but also causes a hardware hang on register
>> access if no_console_suspend command line option is used.
>>
>> Also, not every of_serial device is an 8250 port, so the serial8250
>> suspend/resume functions should only be applied to a real 8250 port.
>>
>> Signed-off-by: Jingchang Lu <jingchang.lu@freescale.com>
>> ---
>> changes in v2:
>>   add switch selection on uart type.
>
>After apply this, it can't build for me. I still need to apply the fix
>like something below.
>
>diff --git a/drivers/tty/serial/of_serial.c
>b/drivers/tty/serial/of_serial.c
>index e535f9c1563d..14c1ff4e7816 100644
>--- a/drivers/tty/serial/of_serial.c
>+++ b/drivers/tty/serial/of_serial.c
>@@ -9,6 +9,7 @@
>   *  2 of the License, or (at your option) any later version.
>   *
>   */
>+#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/slab.h>
>  #include <linux/delay.h>
>@@ -248,13 +249,13 @@ static int of_serial_suspend(struct device *dev)
>  #ifdef CONFIG_SERIAL_8250
>         case PORT_8250 ... PORT_MAX_8250:
>         {
>-               struct uart_8250_port port8250;
>+               struct uart_8250_port *port8250;
>                 port8250 = serial8250_get_port(info->line);
>
>                 serial8250_suspend_port(info->line);
>                 if (info->clk &&
>-                   (!uart_console(port8250) ||
>-                    (uart_console(port8250) && console_suspend_enabled)))
>+                   (!uart_console(&port8250->port) ||
>+                    (uart_console(&port8250->port) &&
>console_suspend_enabled)))
>                         clk_disable_unprepare(info->clk);
>                 break;
>         }
>@@ -274,12 +275,12 @@ static int of_serial_resume(struct device *dev)
>  #ifdef CONFIG_SERIAL_8250
>         case PORT_8250 ... PORT_MAX_8250:
>         {
>-               struct uart_8250_port port8250;
>+               struct uart_8250_port *port8250;
>                 port8250 = serial8250_get_port(info->line);
>
>                 if (info->clk &&
>-                   (!uart_console(port8250) ||
>-                    (uart_console(port8250) && console_suspend_enabled)))
>+                   (!uart_console(&port8250->port) ||
>+                    (uart_console(&port8250->port) &&
>console_suspend_enabled)))
>                         clk_prepare_enable(info->clk);
>
>                 serial8250_resume_port(info->line);
>
>-Joseph
>
>>
>>   drivers/tty/serial/of_serial.c | 42
>++++++++++++++++++++++++++++++++++++------
>>   1 file changed, 36 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/tty/serial/of_serial.c
>b/drivers/tty/serial/of_serial.c
>> index 8bc2563..b525383 100644
>> --- a/drivers/tty/serial/of_serial.c
>> +++ b/drivers/tty/serial/of_serial.c
>> @@ -245,9 +245,24 @@ static int of_serial_suspend(struct device *dev)
>>   {
>>   	struct of_serial_info *info = dev_get_drvdata(dev);
>>
>> -	serial8250_suspend_port(info->line);
>> -	if (info->clk)
>> -		clk_disable_unprepare(info->clk);
>> +	switch(info->type) {
>> +#ifdef CONFIG_SERIAL_8250
>> +	case PORT_8250 ... PORT_MAX_8250:
>> +	{
>> +		struct uart_8250_port port8250;
>> +		port8250 = serial8250_get_port(info->line);
>> +
>> +		serial8250_suspend_port(info->line);
>> +		if (info->clk &&
>> +		    (!uart_console(port8250) ||
>> +		     (uart_console(port8250) && console_suspend_enabled)))
>> +			clk_disable_unprepare(info->clk);
>> +		break;
>> +	}
>> +#endif
>> +	default:
>> +		break;
>> +	}
>>
>>   	return 0;
>>   }
>> @@ -256,10 +271,25 @@ static int of_serial_resume(struct device *dev)
>>   {
>>   	struct of_serial_info *info = dev_get_drvdata(dev);
>>
>> -	if (info->clk)
>> -		clk_prepare_enable(info->clk);
>> +	switch(info->type) {
>> +#ifdef CONFIG_SERIAL_8250
>> +	case PORT_8250 ... PORT_MAX_8250:
>> +	{
>> +		struct uart_8250_port port8250;
>> +		port8250 = serial8250_get_port(info->line);
>> +
>> +		if (info->clk &&
>> +		    (!uart_console(port8250) ||
>> +		     (uart_console(port8250) && console_suspend_enabled)))
>> +			clk_prepare_enable(info->clk);
>>
>> -	serial8250_resume_port(info->line);
>> +		serial8250_resume_port(info->line);
>> +		break;
>> +	}
>> +#endif
>> +	default:
>> +		break;
>> +	}
>>
>>   	return 0;
>>   }
>>

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

end of thread, other threads:[~2014-10-13  9:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-10-13  7:37 [PATCHv2] serial: of-serial: fix up PM ops on no_console_suspend and port type Jingchang Lu
2014-10-13  9:41 ` Joseph Lo
     [not found]   ` <543B9E44.109-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
2014-10-13  9:49     ` Jingchang Lu

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