[PATCH] bsp/raspberrypi: AUX uart driver
Christian Mauderer
list at c-mauderer.de
Thu Jan 16 20:10:51 UTC 2020
On 15/01/2020 13:05, G S Niteesh wrote:
> This patch adds the driver for aux driver present in Raspberry
> Pi 3 and above, this uart is currently used as the primary uart.
Most of the time UART is written all upper case.
> The AUX uart is similar to ns16550, it uses the libchip/ns16550
> driver.
> ---
> bsps/arm/raspberrypi/console/console-config.c | 113 ++++++++++++++++--
> bsps/arm/raspberrypi/include/bsp/usart.h | 1 +
> 2 files changed, 107 insertions(+), 7 deletions(-)
>
> diff --git a/bsps/arm/raspberrypi/console/console-config.c b/bsps/arm/raspberrypi/console/console-config.c
> index 48c4c6a3ec7..6ff018fb8ef 100644
> --- a/bsps/arm/raspberrypi/console/console-config.c
> +++ b/bsps/arm/raspberrypi/console/console-config.c
> @@ -24,6 +24,7 @@
>
> #include <libchip/serial.h>
> #include <libfdt.h>
> +#include <libchip/ns16550.h>
>
> #include <bspopts.h>
> #include <bsp/usart.h>
> @@ -34,35 +35,103 @@
> #include <bsp/console-termios.h>
> #include <bsp/fdt.h>
> #include <bsp/fatal.h>
> +#include <bsp/gpio.h>
> +#include <bsp/rpi-gpio.h>
>
> -
> +/**
> + * UART0 - PL011
> + * UART1 - AUX UART
Isn't it called "Mini UART" in the peripheral manual? That would affect
all following "aux" too.
> + */
> #define UART0 "/dev/ttyS0"
> +#define UART1 "/dev/ttyS1"
> #define FBCONS "/dev/fbcons"
>
> arm_pl011_context pl011_context;
> +ns16550_context aux_context;
>
> rpi_fb_context fb_context;
>
> -static void output_char_serial(char c)
> +static void output_char_pl011(char c)
> {
> arm_pl011_write_polled(&pl011_context.base, c);
> }
>
> +static void output_char_aux(char c)
> +{
> + ns16550_polled_putchar(&aux_context.base, c);
> +}
> +
> void output_char_fb(char c)
> {
> fbcons_write_polled(&fb_context.base, c);
> }
>
> +static uint8_t raspberrypi_get_reg(uintptr_t port, uint8_t index)
Is it a "raspberrypi_get_reg" or a "mini_uart_get_reg"?
> +{
> + volatile uint32_t *val = (volatile uint32_t *)port + index;
> + return (uint8_t) *val;
> +}
> +
> +static void raspberrypi_set_reg(uintptr_t port, uint8_t index, uint8_t val)
> +{
> + volatile uint32_t *reg = (volatile uint32_t *)port + index;
> + *reg = val;
> +}
> +
> static void init_ctx_arm_pl011(
> const void *fdt,
> int node
> )
> {
> arm_pl011_context *ctx = &pl011_context;
> - rtems_termios_device_context_initialize(&ctx->base, "UART");
> + rtems_termios_device_context_initialize(&ctx->base, "UART 0");
Maybe "PL011UART" would be a better name?
> ctx->regs = raspberrypi_get_reg_of_node(fdt, node);
> }
>
> +static uint32_t calculate_baud_divisor(
> + ns16550_context *ctx,
> + uint32_t baud
> +)
> +{
> + uint32_t baudDivisor = (ctx->clock / (8 * baud)) - 1;
> + return baudDivisor;
> +}
> +
> +static void init_ctx_aux(
> + const void *fdt,
> + int node
> +)
> +{
> + const char *status;
> + int len;
> + ns16550_context *ctx = &aux_context;
Although your ctx is a global variable and therefore (most likely) zero
already: If you don't write all fields of a context (you missed for
example has_fractional_divider_register) please set it to zero
explicitly (with memset). It would be possible that someone allocates
that context for example for rpi4 and forgets it. That leads to odd
effects that only happen sometimes.
> + rtems_termios_device_context_initialize(&ctx->base, "UART 1");
I would suggest "MiniUART" for the name. It's just easier if you debug
something and you get told "it's this or that hardware module" without
having to look at some mapping.
> +
> + status = fdt_getprop(fdt, node, "status", &len);
> + if (status == NULL){
> + return ;
> + }
> +
> + if ( strcmp(status, "disabled") == 0 ) {
> + return ;
> + }
You could put that into one if. Would be a bit shorter to read:
if (status == NULL || strcmp(status, "disabled") == 0) {
return;
}
> +
> + ctx->port = (uintptr_t) raspberrypi_get_reg_of_node(fdt, node);
> + ctx->initial_baud = AUX_DEFAULT_BAUD;
> + ctx->clock = BCM2835_CLOCK_FREQ;
> + ctx->calculate_baud_divisor = calculate_baud_divisor;
> + ctx->get_reg = raspberrypi_get_reg;
> + ctx->set_reg = raspberrypi_set_reg;
> +
> + rtems_gpio_bsp_select_specific_io(0, 14, RPI_ALT_FUNC_5, NULL);
> + rtems_gpio_bsp_select_specific_io(0, 15, RPI_ALT_FUNC_5, NULL);
> + rtems_gpio_bsp_set_resistor_mode(0, 14, NO_PULL_RESISTOR);
> + rtems_gpio_bsp_set_resistor_mode(0, 15, NO_PULL_RESISTOR);
> +
> + BCM2835_REG(AUX_ENABLES) |= 0x1;
> + ns16550_probe(&ctx->base);
> +}
> +
> static void register_fb( void )
> {
> if (fbcons_probe(&fb_context.base) == true) {
> @@ -87,16 +156,27 @@ static void console_select( void )
> link(FBCONS, CONSOLE_DEVICE_NAME);
> return ;
> }
> + } else if ( strncmp( opt, UART1, sizeof(UART1) - 1 ) == 0) {
Hm. UART1 is "/dev/ttyS1". Linux calls the miniUART "/dev/ttyS0" and the
PL011 "/dev/ttyAMA0", right? So the command line will be different for
Linux and RTEMS?
I think I would preferre to use the Linux names.
> + BSP_output_char = output_char_aux;
> + link(UART1, CONSOLE_DEVICE_NAME);
> + } else if ( strncmp( opt, UART0, sizeof(UART0) - 1 ) == 0) {
> + BSP_output_char = output_char_pl011;
> + link(UART0, CONSOLE_DEVICE_NAME);
> }
> }
> - BSP_output_char = output_char_serial;
> - link(UART0, CONSOLE_DEVICE_NAME);
> + /**
> + * If no command line option was given, default to AUX uart.
> + */
> + BSP_output_char = output_char_aux;
> + link(UART1, CONSOLE_DEVICE_NAME);
Is it really a good idea to default to the AUX uart? The BSP is still
named raspberrypi2. So I think the default should match the RPi2.
> }
>
> static void uart_probe(void)
> {
> static bool initialized = false;
> const void *fdt;
> + const char *console;
> + int len;
> int node;
>
> if ( initialized ) {
> @@ -104,17 +184,29 @@ static void uart_probe(void)
> }
>
> fdt = bsp_fdt_get();
> - node = fdt_node_offset_by_compatible(fdt, -1, "brcm,bcm2835-pl011");
>
> + node = fdt_node_offset_by_compatible(fdt, -1, "brcm,bcm2835-pl011");
> init_ctx_arm_pl011(fdt, node);
>
> + node = fdt_node_offset_by_compatible(fdt, -1, "brcm,bcm2835-aux-uart");
> + init_ctx_aux(fdt, node);
> +
> + node = fdt_path_offset(fdt, "/aliases");
> + console = fdt_getprop(fdt, node, "serial0", &len);
> +
> + if (strcmp(console, "/soc/serial at 7e215040") == 0) {
> + BSP_output_char = output_char_aux;
> + }else {
> + BSP_output_char = output_char_pl011;
> + }
> +
> initialized = true;
> }
>
> static void output_char(char c)
> {
> uart_probe();
> - output_char_serial(c);
> + (*BSP_output_char)(c);
> }
>
> rtems_status_code console_initialize(
> @@ -133,6 +225,13 @@ rtems_status_code console_initialize(
> &pl011_context.base
> );
>
> + rtems_termios_device_install(
> + UART1,
> + &ns16550_handler_polled,
> + NULL,
> + &aux_context.base
> + );
> +
> register_fb();
>
> console_select();
> diff --git a/bsps/arm/raspberrypi/include/bsp/usart.h b/bsps/arm/raspberrypi/include/bsp/usart.h
> index abbf53626c9..c73cebc86bd 100644
> --- a/bsps/arm/raspberrypi/include/bsp/usart.h
> +++ b/bsps/arm/raspberrypi/include/bsp/usart.h
> @@ -33,6 +33,7 @@ extern "C" {
> #endif /* __cplusplus */
>
> #define PL011_DEFAULT_BAUD 115000
> +#define AUX_DEFAULT_BAUD 115200
> #define BCM2835_PL011_BASE (RPI_PERIPHERAL_BASE + 0x201000)
>
> #ifdef __cplusplus
>
More information about the devel
mailing list