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;
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
