[PATCH] tty: serial: imx: return error when clk_per fails in imx_uart_poll_init()

[PATCH] tty: serial: imx: return error when clk_per fails in imx_uart_poll_init()

[Gu Bin]

In imx_uart_poll_init(), when clk_prepare_enable(sport->clk_per) fails
the error branch only disables the previously enabled clk_ipg and then
falls through: the function keeps programming the UART (imx_uart_setup_ufcr
and the register writes below) with both clocks disabled and eventually
returns 0, reporting success for a port that never came up.

The sibling imx_uart_startup() handles the same situation by returning
the error after the rollback.

Return the error after rolling back clk_ipg, matching
imx_uart_startup().

Fixes: 6b8bdad96164 ("serial: imx: add imx_poll_init()")
Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
---
 drivers/tty/serial/imx.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
index 0831bd5ee3dc..2a931361d17c 100644
--- a/drivers/tty/serial/imx.c
+++ b/drivers/tty/serial/imx.c
@@ -1954,8 +1954,10 @@ static int imx_uart_poll_init(struct uart_port *port)
 	if (retval)
 		return retval;
 	retval = clk_prepare_enable(sport->clk_per);
-	if (retval)
+	if (retval) {
 		clk_disable_unprepare(sport->clk_ipg);
+		return retval;
+	}
 
 	imx_uart_setup_ufcr(sport, TXTL_DEFAULT, sport->rxtl);
 
-- 
2.50.1.windows.1

Re: [PATCH] tty: serial: imx: return error when clk_per fails in imx_uart_poll_init()

[Jiri Slaby]

On 10. 11. 26, 11:54, Gu Bin wrote:
> In imx_uart_poll_init(), when clk_prepare_enable(sport->clk_per) fails
> the error branch only disables the previously enabled clk_ipg and then
> falls through: the function keeps programming the UART (imx_uart_setup_ufcr
> and the register writes below) with both clocks disabled and eventually
> returns 0, reporting success for a port that never came up.
> 
> The sibling imx_uart_startup() handles the same situation by returning
> the error after the rollback.
> 
> Return the error after rolling back clk_ipg, matching
> imx_uart_startup().
> 
> Fixes: 6b8bdad96164 ("serial: imx: add imx_poll_init()")
> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>

I am not sure this is correct at all.

Ccin the author of the code would've helped. Done now.

> ---
>   drivers/tty/serial/imx.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
> index 0831bd5ee3dc..2a931361d17c 100644
> --- a/drivers/tty/serial/imx.c
> +++ b/drivers/tty/serial/imx.c
> @@ -1954,8 +1954,10 @@ static int imx_uart_poll_init(struct uart_port *port)
>   	if (retval)
>   		return retval;
>   	retval = clk_prepare_enable(sport->clk_per);
> -	if (retval)
> +	if (retval) {
>   		clk_disable_unprepare(sport->clk_ipg);
> +		return retval;
> +	}
>   
>   	imx_uart_setup_ufcr(sport, TXTL_DEFAULT, sport->rxtl);
>   


-- 
js
suse labs

Re: [PATCH] tty: serial: imx: return error when clk_per fails in imx_uart_poll_init()

[Jiri Slaby]

One more try using .mailmap.

On 11. 11. 26, 10:21, Jiri Slaby wrote:
> On 10. 11. 26, 11:54, Gu Bin wrote:
>> In imx_uart_poll_init(), when clk_prepare_enable(sport->clk_per) fails
>> the error branch only disables the previously enabled clk_ipg and then
>> falls through: the function keeps programming the UART 
>> (imx_uart_setup_ufcr
>> and the register writes below) with both clocks disabled and eventually
>> returns 0, reporting success for a port that never came up.
>>
>> The sibling imx_uart_startup() handles the same situation by returning
>> the error after the rollback.
>>
>> Return the error after rolling back clk_ipg, matching
>> imx_uart_startup().
>>
>> Fixes: 6b8bdad96164 ("serial: imx: add imx_poll_init()")
>> Signed-off-by: Gu Bin <gubin@cmss.chinamobile.com>
> 
> I am not sure this is correct at all.
> 
> Ccin the author of the code would've helped. Done now.
> 
>> ---
>>   drivers/tty/serial/imx.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/imx.c b/drivers/tty/serial/imx.c
>> index 0831bd5ee3dc..2a931361d17c 100644
>> --- a/drivers/tty/serial/imx.c
>> +++ b/drivers/tty/serial/imx.c
>> @@ -1954,8 +1954,10 @@ static int imx_uart_poll_init(struct uart_port 
>> *port)
>>       if (retval)
>>           return retval;
>>       retval = clk_prepare_enable(sport->clk_per);
>> -    if (retval)
>> +    if (retval) {
>>           clk_disable_unprepare(sport->clk_ipg);
>> +        return retval;
>> +    }
>>       imx_uart_setup_ufcr(sport, TXTL_DEFAULT, sport->rxtl);
> 
> 

-- 
js
suse labs