[PATCH v2] bsp/beagle: Update console to new Termios device API.

[Christian Mauderer]

Hello Niteesh,


On 15/03/2020 20:44, G S Niteesh Babu wrote:
> This patch updates the console to use new Termios device API.
> Update #3034
> ---
>  bsps/arm/beagle/console/console-config.c | 66 +++++++++++++++---------
>  1 file changed, 41 insertions(+), 25 deletions(-)
> 
> diff --git a/bsps/arm/beagle/console/console-config.c b/bsps/arm/beagle/console/console-config.c
> index 78af5f6a93..860a44cb00 100644
> --- a/bsps/arm/beagle/console/console-config.c
> +++ b/bsps/arm/beagle/console/console-config.c
> @@ -41,6 +41,8 @@
>  #define TX_FIFO_E (1<<5)
>  #define RX_FIFO_E (1<<0)
>  
> +#define UART0 "/dev/ttyS0"
> +

I'm not a big fan of such a generic name for a define. It's quite
possible that it conflicts with another define in the future.

I would suggest BEAGLE_UART0 or similar.

>  static uint8_t beagle_uart_get_register(uintptr_t addr, uint8_t i)
>  {
>    uint8_t v;
> @@ -65,34 +67,27 @@ static void beagle_uart_set_register(uintptr_t addr, uint8_t i, uint8_t val)
>    reg [i] = val;
>  }
>  
> -console_tbl Console_Configuration_Ports [] = {
> -    {
> -      .sDeviceName = "/dev/ttyS0",
> -      .deviceType = SERIAL_NS16550,
> -#if CONSOLE_POLLED	/* option to facilitate running the tests */
> -      .pDeviceFns = &ns16550_fns_polled,
> -#else
> -      .pDeviceFns = &ns16550_fns,
> -#endif
> -      .ulMargin = 16,
> -      .ulHysteresis = 8,
> -      .pDeviceParams = (void *) CONSOLE_BAUD,
> -      .ulCtrlPort1 = BSP_CONSOLE_UART_BASE,
> -      .ulDataPort = BSP_CONSOLE_UART_BASE,
> -      .ulIntVector = BSP_CONSOLE_UART_IRQ,
> -      .getRegister = beagle_uart_get_register,
> -      .setRegister = beagle_uart_set_register,
> -      .ulClock = UART_CLOCK,  /* 48MHz base clock */
> -    },
> -};
> -
> -unsigned long Console_Configuration_Count = 1;
> +ns16550_context uart_context;
>  
>  static int init_needed = 1; // don't rely on bss being 0
>  
>  static void beagle_console_init(void)
>  {
>    if(init_needed) {
> +    ns16550_context *ctx;
> +
> +    /*
> +   *  Don't rely on BSS being 0
> +    */
> +    memset(&uart_context, 0, sizeof(uart_context));
> +    ctx = &uart_context;
> +
> +    ctx->port = BSP_CONSOLE_UART_BASE;
> +    ctx->get_reg = beagle_uart_get_register;
> +    ctx->set_reg = beagle_uart_set_register;
> +    ctx->clock = UART_CLOCK;
> +    ctx->initial_baud = CONSOLE_BAUD;
> +
>      const uint32_t div = UART_CLOCK / 16 / CONSOLE_BAUD;
>      CONSOLE_SYSC = 2;
>      while ((CONSOLE_SYSS & 1) == 0)
> @@ -120,6 +115,8 @@ static void beagle_console_init(void)
>      CONSOLE_LCR = 0x03;
>      CONSOLE_ACR = 0x00;
>      init_needed = 0;
> +
> +    BSP_output_char = uart_write_polled;
>    }
>  }
>  
> @@ -127,15 +124,17 @@ static void beagle_console_init(void)
>  
>  static void uart_write_polled( char c )
>  {
> -  if(init_needed) beagle_console_init();
> -
>    while( ( CONSOLE_LSR & TX_FIFO_E ) == 0 )
>      ;
>    CONSOLE_THR8 = c;
>  }
>  
>  static void _BSP_put_char( char c ) {
> -   uart_write_polled( c );
> +
> +  if ( init_needed ) {
> +    beagle_console_init();
> +  }
> +  uart_write_polled( c );

Why did you have to move the "if (init_needed)" from uart_write_polled
to _BSP_put_char? It seems that it does the same as long as
_BSP_put_char is called first. But if uart_write_polled is called first,
it will lead to an unitialised hardware, wouldn't it?

By the way: Did you have a look whether there are two functions? They
are both static and it seems that only _BSP_put_char is used. So it
seems a bit odd...

>  }
>  
>  static int _BSP_get_char(void)
> @@ -147,6 +146,23 @@ static int _BSP_get_char(void)
>    }
>  }
>  
> +rtems_status_code console_initialize(
> +  rtems_device_major_number major,
> +  rtems_device_minor_number minor,
> +  void *arg
> +)
> +{
> +  rtems_termios_initialize();
> +  rtems_termios_device_install(
> +    UART0,
> +    &ns16550_handler_polled,
> +    NULL,
> +    &uart_context.base
> +  );
> +
> +  return RTEMS_SUCCESSFUL;
> +}
> +
>  BSP_output_char_function_type BSP_output_char = _BSP_put_char;
>  
>  BSP_polling_getchar_function_type BSP_poll_char = _BSP_get_char;
>