diff mbox

[1/6] serial: meson: fix setting number of stop bits

Message ID d1091d2e-8f6c-618b-556b-400e4b8db3c3@gmail.com (mailing list archive)
State Superseded
Headers show

Commit Message

Heiner Kallweit April 16, 2017, 8:06 p.m. UTC
The stop bit value as to be or'ed, so far this worked only just by chance
because AML_UART_STOP_BIN_1SB is 0.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/tty/serial/meson_uart.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Neil Armstrong April 17, 2017, 3:24 p.m. UTC | #1
On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
> The stop bit value as to be or'ed, so far this worked only just by chance
> because AML_UART_STOP_BIN_1SB is 0.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>  drivers/tty/serial/meson_uart.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
> index 60f16795..e2e25da1 100644
> --- a/drivers/tty/serial/meson_uart.c
> +++ b/drivers/tty/serial/meson_uart.c
> @@ -355,7 +355,7 @@ static void meson_uart_set_termios(struct uart_port *port,
>  	if (cflags & CSTOPB)
>  		val |= AML_UART_STOP_BIN_2SB;
>  	else
> -		val &= ~AML_UART_STOP_BIN_1SB;
> +		val |= AML_UART_STOP_BIN_1SB;
>  
>  	if (cflags & CRTSCTS)
>  		val &= ~AML_UART_TWO_WIRE_EN;
> 

Hi,

in fact you can totally drop the else statement since it's a no-op in both cases.

Neil
Heiner Kallweit April 17, 2017, 3:54 p.m. UTC | #2
Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
> On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
>> The stop bit value as to be or'ed, so far this worked only just by chance
>> because AML_UART_STOP_BIN_1SB is 0.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  drivers/tty/serial/meson_uart.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>> index 60f16795..e2e25da1 100644
>> --- a/drivers/tty/serial/meson_uart.c
>> +++ b/drivers/tty/serial/meson_uart.c
>> @@ -355,7 +355,7 @@ static void meson_uart_set_termios(struct uart_port *port,
>>  	if (cflags & CSTOPB)
>>  		val |= AML_UART_STOP_BIN_2SB;
>>  	else
>> -		val &= ~AML_UART_STOP_BIN_1SB;
>> +		val |= AML_UART_STOP_BIN_1SB;
>>  
>>  	if (cflags & CRTSCTS)
>>  		val &= ~AML_UART_TWO_WIRE_EN;
>>
> 
> Hi,
> 
> in fact you can totally drop the else statement since it's a no-op in both cases.
> 
That's right. However I think leaving this in makes the code better understandable.
And most likely the compiler will remove this no-op anyway.

Heiner

> Neil
>
Kevin Hilman April 21, 2017, 10:03 p.m. UTC | #3
Heiner Kallweit <hkallweit1@gmail.com> writes:

> Am 17.04.2017 um 17:24 schrieb Neil Armstrong:
>> On 04/16/2017 10:06 PM, Heiner Kallweit wrote:
>>> The stop bit value as to be or'ed, so far this worked only just by chance
>>> because AML_UART_STOP_BIN_1SB is 0.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>>  drivers/tty/serial/meson_uart.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
>>> index 60f16795..e2e25da1 100644
>>> --- a/drivers/tty/serial/meson_uart.c
>>> +++ b/drivers/tty/serial/meson_uart.c
>>> @@ -355,7 +355,7 @@ static void meson_uart_set_termios(struct uart_port *port,
>>>  	if (cflags & CSTOPB)
>>>  		val |= AML_UART_STOP_BIN_2SB;
>>>  	else
>>> -		val &= ~AML_UART_STOP_BIN_1SB;
>>> +		val |= AML_UART_STOP_BIN_1SB;
>>>  
>>>  	if (cflags & CRTSCTS)
>>>  		val &= ~AML_UART_TWO_WIRE_EN;
>>>
>> 
>> Hi,
>> 
>> in fact you can totally drop the else statement since it's a no-op in both cases.
>> 
> That's right. However I think leaving this in makes the code better understandable.
> And most likely the compiler will remove this no-op anyway.

Agreed.  I like it for readability.

Kevin
diff mbox

Patch

diff --git a/drivers/tty/serial/meson_uart.c b/drivers/tty/serial/meson_uart.c
index 60f16795..e2e25da1 100644
--- a/drivers/tty/serial/meson_uart.c
+++ b/drivers/tty/serial/meson_uart.c
@@ -355,7 +355,7 @@  static void meson_uart_set_termios(struct uart_port *port,
 	if (cflags & CSTOPB)
 		val |= AML_UART_STOP_BIN_2SB;
 	else
-		val &= ~AML_UART_STOP_BIN_1SB;
+		val |= AML_UART_STOP_BIN_1SB;
 
 	if (cflags & CRTSCTS)
 		val &= ~AML_UART_TWO_WIRE_EN;