Re: [PATCH v2] tty: serial core: decouple pm states from ACPI

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

On 12/07/2012 11:36 AM, Linus Walleij wrote:
> From: Linus Walleij <linus.walleij@linaro.org>
> 
> The serial core is using power states lifted from ACPI for no
> good reason. Remove this reference from the documentation and
> alter all users to use an enum specific to the serial core
> instead, and define it in <linux/serial_core.h>.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v2:
> - rename states from _DEFAULT and _SLEEP to simply _ON
>   and _OFF as Rafael suggests.
> ---

Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>

One suggestion below.


[ ... ]

> @@ -1620,7 +1621,7 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
>  {
>  	struct uart_state *state = drv->state + i;
>  	struct tty_port *port = &state->port;
> -	int pm_state;
> +	enum uart_pm_state pm_state;
>  	struct uart_port *uport = state->uart_port;
>  	char stat_buf[32];
>  	unsigned int status;
> @@ -1645,12 +1646,12 @@ static void uart_line_info(struct seq_file *m, struct uart_driver *drv, int i)
>  	if (capable(CAP_SYS_ADMIN)) {
>  		mutex_lock(&port->mutex);
>  		pm_state = state->pm_state;
> -		if (pm_state)
> -			uart_change_pm(state, 0);
> +		if (pm_state != UART_PM_STATE_ON)
> +			uart_change_pm(state, UART_PM_STATE_ON);

The check is already done in the uart_change_pm function, no ?

>  		spin_lock_irq(&uport->lock);
>  		status = uport->ops->get_mctrl(uport);
>  		spin_unlock_irq(&uport->lock);
> -		if (pm_state)
> +		if (pm_state != UART_PM_STATE_ON)
>  			uart_change_pm(state, pm_state);

Maybe it could be worth to change uart_change_pm to return the old state
value.

So the code will be simpler:

pm_state = uart_change_pm(state, UART_PM_STATE_ON);

...

uart_change_pm(state, pm_state);


The uart_change_pm already takes care of changing the value if the
current one is different.

Well, maybe that could come on a different patch.

[ ... ]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-serial" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html