[PATCH] ns16550: Respect CONFIG_BAUDRATE < 0 in _debug_uart_init

[PATCH] ns16550: Respect CONFIG_BAUDRATE < 0 in _debug_uart_init

[Joakim Tjernlund]

CONFIG_BAUDRATE less than 0 means do not touch baudrate settings.

Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
---
 drivers/serial/ns16550.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 702109b23b..286d8aecca 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -333,15 +333,19 @@ static inline void _debug_uart_init(void)
 	 * feasible. The better fix is to move all users of this driver to
 	 * driver model.
 	 */
-	baud_divisor = ns16550_calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK,
-					    CONFIG_BAUDRATE);
 	serial_dout(&com_port->ier, CONFIG_SYS_NS16550_IER);
 	serial_dout(&com_port->mcr, UART_MCRVAL);
 	serial_dout(&com_port->fcr, UART_FCR_DEFVAL);
 
-	serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
-	serial_dout(&com_port->dll, baud_divisor & 0xff);
-	serial_dout(&com_port->dlm, (baud_divisor >> 8) & 0xff);
+	if (CONFIG_BAUDRATE > 0) {
+		baud_divisor = ns16550_calc_divisor(com_port,
+						    CONFIG_DEBUG_UART_CLOCK,
+						    CONFIG_BAUDRATE);
+
+		serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
+		serial_dout(&com_port->dll, baud_divisor & 0xff);
+		serial_dout(&com_port->dlm, (baud_divisor >> 8) & 0xff);
+	}
 	serial_dout(&com_port->lcr, UART_LCRVAL);
 }
 
-- 
2.34.1

Re: [PATCH] ns16550: Respect CONFIG_BAUDRATE < 0 in _debug_uart_init

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 420 bytes --]

On Fri, Apr 08, 2022 at 04:29:02PM +0200, Joakim Tjernlund wrote:

> CONFIG_BAUDRATE less than 0 means do not touch baudrate settings.
> 
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>  drivers/serial/ns16550.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

I don't see this in doc/ so please do a follow-up to note it in there as
well, thanks!

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH] ns16550: Respect CONFIG_BAUDRATE < 0 in _debug_uart_init

[Heinrich Schuchardt]

On 4/8/22 16:29, Joakim Tjernlund wrote:
> CONFIG_BAUDRATE less than 0 means do not touch baudrate settings.
>
> Signed-off-by: Joakim Tjernlund <joakim.tjernlund@infinera.com>
> ---
>   drivers/serial/ns16550.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
> index 702109b23b..286d8aecca 100644
> --- a/drivers/serial/ns16550.c
> +++ b/drivers/serial/ns16550.c
> @@ -333,15 +333,19 @@ static inline void _debug_uart_init(void)
>   	 * feasible. The better fix is to move all users of this driver to
>   	 * driver model.
>   	 */
> -	baud_divisor = ns16550_calc_divisor(com_port, CONFIG_DEBUG_UART_CLOCK,
> -					    CONFIG_BAUDRATE);
>   	serial_dout(&com_port->ier, CONFIG_SYS_NS16550_IER);
>   	serial_dout(&com_port->mcr, UART_MCRVAL);
>   	serial_dout(&com_port->fcr, UART_FCR_DEFVAL);
>
> -	serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
> -	serial_dout(&com_port->dll, baud_divisor & 0xff);
> -	serial_dout(&com_port->dlm, (baud_divisor >> 8) & 0xff);
> +	if (CONFIG_BAUDRATE > 0) {

The general idea that CONFIG_BAUDRATE == 0 should be ignored is good.

Negative value of CONFIG_BAUDRATE are not consistently treated in the
coding. We should not use them.

It may not enough to change _debug_uart_init(). Please, also consider
serial_setbrg() in the uclass if you want to avoid that the baudrate is
touched.

How about the other debug UART drivers?

Your commit title and the commit message don't match the code which
ignores CONFIG_BAUDRATE == 0.

Best regards

Heinrich

> +		baud_divisor = ns16550_calc_divisor(com_port,
> +						    CONFIG_DEBUG_UART_CLOCK,
> +						    CONFIG_BAUDRATE);
> +
> +		serial_dout(&com_port->lcr, UART_LCR_BKSE | UART_LCRVAL);
> +		serial_dout(&com_port->dll, baud_divisor & 0xff);
> +		serial_dout(&com_port->dlm, (baud_divisor >> 8) & 0xff);
> +	}
>   	serial_dout(&com_port->lcr, UART_LCRVAL);
>   }
>