[PATCH] serial_core: Change UART PM state to OFF on failure

[PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

uart_change_pm is used to turn on the UART controller resources and
change UART's PM status. On failure to allocate pages the controller
be left in ON state. This will change the state to OFF on failure.

Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
---
 drivers/tty/serial/serial_core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
index 62fe368..58af2e9 100644
--- a/drivers/tty/serial/serial_core.c
+++ b/drivers/tty/serial/serial_core.c
@@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
 	if (!state->xmit.buf) {
 		/* This is protected by the per port mutex */
 		page = get_zeroed_page(GFP_KERNEL);
-		if (!page)
+		if (!page) {
+			uart_change_pm(state, UART_PM_STATE_OFF);
 			return -ENOMEM;
-
+		}
 		state->xmit.buf = (unsigned char *) page;
 		uart_circ_clear(&state->xmit);
 	}
-- 
1.8.2.1

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

On 6 May 2016 at 15:16, Pramod Gurav <pramod.gurav@linaro.org> wrote:
> uart_change_pm is used to turn on the UART controller resources and
> change UART's PM status. On failure to allocate pages the controller
> be left in ON state. This will change the state to OFF on failure.
>
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
> ---
>  drivers/tty/serial/serial_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 62fe368..58af2e9 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
>         if (!state->xmit.buf) {
>                 /* This is protected by the per port mutex */
>                 page = get_zeroed_page(GFP_KERNEL);
> -               if (!page)
> +               if (!page) {
> +                       uart_change_pm(state, UART_PM_STATE_OFF);
>                         return -ENOMEM;
> -
> +               }
>                 state->xmit.buf = (unsigned char *) page;
>                 uart_circ_clear(&state->xmit);
>         }

Greg,
Any comments on this change?

> --
> 1.8.2.1
>

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Peter Hurley]

Hi Pramod,

On 05/06/2016 02:46 AM, Pramod Gurav wrote:
> uart_change_pm is used to turn on the UART controller resources and
> change UART's PM status. On failure to allocate pages the controller
> be left in ON state. This will change the state to OFF on failure.
> 
> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
> ---
>  drivers/tty/serial/serial_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
> index 62fe368..58af2e9 100644
> --- a/drivers/tty/serial/serial_core.c
> +++ b/drivers/tty/serial/serial_core.c
> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
>  	if (!state->xmit.buf) {
>  		/* This is protected by the per port mutex */
>  		page = get_zeroed_page(GFP_KERNEL);
> -		if (!page)
> +		if (!page) {

	if (!uart_console(uport))

Otherwise, you'll be powering off the console.

Just out of curiosity, did you actually hit this error?

Regards,
Peter Hurley

> +			uart_change_pm(state, UART_PM_STATE_OFF);
>  			return -ENOMEM;
> -
> +		}
>  		state->xmit.buf = (unsigned char *) page;
>  		uart_circ_clear(&state->xmit);
>  	}
> 

Re: [PATCH] serial_core: Change UART PM state to OFF on failure

[Pramod Gurav]

Hi Peter,

On 8 June 2016 at 22:17, Peter Hurley <peter@hurleysoftware.com> wrote:
> Hi Pramod,
>
> On 05/06/2016 02:46 AM, Pramod Gurav wrote:
>> uart_change_pm is used to turn on the UART controller resources and
>> change UART's PM status. On failure to allocate pages the controller
>> be left in ON state. This will change the state to OFF on failure.
>>
>> Signed-off-by: Pramod Gurav <pramod.gurav@linaro.org>
>> ---
>>  drivers/tty/serial/serial_core.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c
>> index 62fe368..58af2e9 100644
>> --- a/drivers/tty/serial/serial_core.c
>> +++ b/drivers/tty/serial/serial_core.c
>> @@ -156,9 +156,10 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state,
>>       if (!state->xmit.buf) {
>>               /* This is protected by the per port mutex */
>>               page = get_zeroed_page(GFP_KERNEL);
>> -             if (!page)
>> +             if (!page) {
>
>         if (!uart_console(uport))
>
> Otherwise, you'll be powering off the console.
>

Agree. Should take care console is not disabled.
> Just out of curiosity, did you actually hit this error?
No, I did not. I thought we should not be leaving port power enabled
in error case.

Thanks for review and comments.

Regards,
Pramod