date:20200117

Re: [PATCH] bsp/raspberrypi: AUX uart driver

2020-01-17 Thread Niteesh

On Fri, Jan 17, 2020 at 1:40 AM Christian Mauderer 
wrote:

> 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 
> >  #include 
> > +#include 
> >
> >  #include 
> >  #include 
> > @@ -34,35 +35,103 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> >
> > -
> > +/**
> > + * 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"
>

Should I leave them as UART0 and UART1 or change them
to their respective peripheral names?

>  #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(FBCON

Re: [PATCH] bsp/raspberrypi: AUX uart driver

2020-01-17 Thread Niteesh

And also should I change everything else to mini_uart or just leave
it as mini, for example, init_ctx_mini or init_ctx_mini_uart.
mini_uart seems good to me. So should I change everything to
mini_uart_context, output_char_mini_uart or leave them as
mini_context, mini_uart?

On Fri, Jan 17, 2020 at 7:27 PM Niteesh  wrote:

> On Fri, Jan 17, 2020 at 1:40 AM Christian Mauderer 
> wrote:
>
>> 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 
>> >  #include 
>> > +#include 
>> >
>> >  #include 
>> >  #include 
>> > @@ -34,35 +35,103 @@
>> >  #include 
>> >  #include 
>> >  #include 
>> > +#include 
>> > +#include 
>> >
>> > -
>> > +/**
>> > + * 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"
>>
>
> Should I leave them as UART0 and UART1 or change them
> to their respective peripheral names?
>
> >  #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_A

Re: [PATCH] bsp/raspberrypi: AUX uart driver

2020-01-17 Thread Christian Mauderer



On 17/01/2020 14:57, Niteesh wrote:
> On Fri, Jan 17, 2020 at 1:40 AM Christian Mauderer  > wrote:
> 
> 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 
> >  #include 
> > +#include 
> > 
> >  #include 
> >  #include 
> > @@ -34,35 +35,103 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> > +#include 
> > 
> > -
> > +/**
> > + * 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"
> 
>  
> Should I leave them as UART0 and UART1 or change them
> to their respective peripheral names?

I would rename them. Random numbers are allways a bit confusing.

> 
> >  #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 =

[PATCH 2/2] bsp/raspberrypi: Mini UART driver

2020-01-17 Thread G S Niteesh

This patch adds driver for Mini UART present in Raspberry Pi 3
and above, this UART is currently used as the primary UART in
these models.
The Mini UART is similar to ns16550, this driver is built
upon libchip/ns16550.
---
 bsps/arm/raspberrypi/console/console-config.c | 117 --
 bsps/arm/raspberrypi/include/bsp/usart.h  |   1 +
 2 files changed, 109 insertions(+), 9 deletions(-)

diff --git a/bsps/arm/raspberrypi/console/console-config.c 
b/bsps/arm/raspberrypi/console/console-config.c
index 48c4c6a3ec7..720033cb295 100644
--- a/bsps/arm/raspberrypi/console/console-config.c
+++ b/bsps/arm/raspberrypi/console/console-config.c
@@ -24,6 +24,7 @@
 
 #include 
 #include 
+#include 
 
 #include 
 #include 
@@ -34,35 +35,103 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
-
-#define UART0 "/dev/ttyS0"
+/**
+ * UART0 - PL011
+ * UART1 - Mini UART
+ */
+#define PL011 "/dev/ttyAMA0"
+#define MINIUART  "/dev/ttyS0"
 #define FBCONS"/dev/fbcons"
 
 arm_pl011_context pl011_context;
+ns16550_context mini_uart_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_mini_uart(char c)
+{
+  ns16550_polled_putchar(&mini_uart_context.base, c);
+}
+
 void output_char_fb(char c)
 {
   fbcons_write_polled(&fb_context.base, c);
 }
 
+static uint8_t mini_uart_get_reg(uintptr_t port, uint8_t index)
+{
+  volatile uint32_t *val = (volatile uint32_t *)port + index;
+  return (uint8_t) *val;
+}
+
+static void mini_uart_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, "PL011UART");
   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_mini_uart(
+  const void *fdt,
+  int node
+)
+{
+  const char *status;
+  int len;
+  ns16550_context *ctx;
+
+  memset(&mini_uart_context, 0, sizeof(mini_uart_context));
+  ctx = &mini_uart_context;
+
+  rtems_termios_device_context_initialize(&ctx->base, "MiniUART");
+
+  status = fdt_getprop(fdt, node, "status", &len);
+  if ( status == NULL || strcmp(status, "disabled" ) == 0){
+return ;
+  }
+
+  ctx->port = (uintptr_t) raspberrypi_get_reg_of_node(fdt, node);
+  ctx->initial_baud = MINI_UART_DEFAULT_BAUD;
+  ctx->clock = BCM2835_CLOCK_FREQ;
+  ctx->calculate_baud_divisor = calculate_baud_divisor;
+  ctx->get_reg = mini_uart_get_reg;
+  ctx->set_reg = mini_uart_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, MINIUART, sizeof(MINIUART) - 1 ) == 0) {
+  BSP_output_char = output_char_mini_uart;
+  link(MINIUART, CONSOLE_DEVICE_NAME);
+} else if ( strncmp( opt, PL011, sizeof(PL011) - 1 ) == 0) {
+  BSP_output_char = output_char_pl011;
+  link(PL011, CONSOLE_DEVICE_NAME);
 }
   }
-  BSP_output_char = output_char_serial;
-  link(UART0, CONSOLE_DEVICE_NAME);
+  /**
+   * If no command line option was given, default to PL011.
+   */
+  BSP_output_char = output_char_pl011;
+  link(PL011, CONSOLE_DEVICE_NAME);
 }
 
 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_mini_uart(fdt, node);
+
+  node = fdt_path_offset(fdt, "/aliases");
+  console = fdt_getprop(fdt, node, "serial0", &len);
+
+  if ( strcmp(console, "/soc/serial@7e215040" ) == 0) {
+BSP_output_char = output_char_mini_uart;
+  }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_co

[PATCH 1/2] libchip/ns16550: Allow user calculate baud divisor

2020-01-17 Thread G S Niteesh

This patch will allow the user to pass a function to calculate
the baud divisor.
This is will allow for more flexibility, since for some BSP's
like raspberrypi, the calculate of baud divisor is different
from what is in the current driver.
---
 bsps/include/libchip/ns16550.h   | 9 +++--
 bsps/shared/dev/serial/ns16550-context.c | 2 ++
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/bsps/include/libchip/ns16550.h b/bsps/include/libchip/ns16550.h
index f053c8767f9..a08c58b7b6e 100644
--- a/bsps/include/libchip/ns16550.h
+++ b/bsps/include/libchip/ns16550.h
@@ -60,7 +60,11 @@ typedef uint8_t (*ns16550_get_reg)(uintptr_t port, uint8_t 
reg);
 
 typedef void (*ns16550_set_reg)(uintptr_t port, uint8_t reg, uint8_t value);
 
-typedef struct {
+typedef struct ns16550_context ns16550_context;
+
+typedef uint32_t (*ns16550_calculate_baud_divisor)(ns16550_context *ctx, 
uint32_t baud);
+
+struct ns16550_context{
   rtems_termios_device_context base;
   ns16550_get_reg get_reg;
   ns16550_set_reg set_reg;
@@ -70,6 +74,7 @@ typedef struct {
   uint32_t initial_baud;
   bool has_fractional_divider_register;
   bool has_precision_clock_synthesizer;
+  ns16550_calculate_baud_divisor calculate_baud_divisor;
   uint8_t modem_control;
   uint8_t line_control;
   uint32_t baud_divisor;
@@ -78,7 +83,7 @@ typedef struct {
   size_t out_current;
   const char *out_buf;
   rtems_termios_tty *tty;
-} ns16550_context;
+};
 
 extern const rtems_termios_device_handler ns16550_handler_interrupt;
 extern const rtems_termios_device_handler ns16550_handler_polled;
diff --git a/bsps/shared/dev/serial/ns16550-context.c 
b/bsps/shared/dev/serial/ns16550-context.c
index ce55b8309cc..dbf6c64af4a 100644
--- a/bsps/shared/dev/serial/ns16550-context.c
+++ b/bsps/shared/dev/serial/ns16550-context.c
@@ -112,6 +112,8 @@ static uint32_t NS16550_GetBaudDivisor(ns16550_context 
*ctx, uint32_t baud)
   NS16550_FRACTIONAL_DIVIDER,
   fractionalDivider
 );
+  } else if (ctx->calculate_baud_divisor != NULL) {
+baudDivisor = ctx->calculate_baud_divisor(ctx, baud);
   }
 
   return baudDivisor;
-- 
2.17.1

___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] libchip/ns16550: Allow user calculate baud divisor

2020-01-17 Thread Gedare Bloom

On Thu, Jan 16, 2020 at 12:48 PM Christian Mauderer  wrote:
>
>
>
> On 15/01/2020 12:50, G S Niteesh wrote:
> > This patch will allow the user to pass a function to calculate
> > the baud divisor.
> > This is will allow for more flexibility, since for some BSP's
> > like raspberrypi, the calculate of baud divisor is different
> > from what is in the current driver.
> > ---
> >  bsps/include/libchip/ns16550.h   | 11 ---
> >  bsps/shared/dev/serial/ns16550-context.c |  6 --
> >  2 files changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/bsps/include/libchip/ns16550.h b/bsps/include/libchip/ns16550.h
> > index f053c8767f9..ab7992fd30b 100644
> > --- a/bsps/include/libchip/ns16550.h
> > +++ b/bsps/include/libchip/ns16550.h
> > @@ -1,6 +1,6 @@
> >  /**
> >   *  @file
> > - *
> > + *
>
> Please don't change lines that don't concern the target of your patch.
> If you want to fix the white space please do so in an extra patch.
>
> By the way: You can also send patch sets. Just pass all patches to "git
> send-email". Then git will number patches that belong together (like
> this and the AUX uart driver). That makes it easier to apply all in the
> right order.
>
> >   */
> >
> >  /*
> > @@ -60,7 +60,11 @@ typedef uint8_t (*ns16550_get_reg)(uintptr_t port, 
> > uint8_t reg);
> >
> >  typedef void (*ns16550_set_reg)(uintptr_t port, uint8_t reg, uint8_t 
> > value);
> >
> > -typedef struct {
> > +typedef struct ns16550_context ns16550_context;
> > +
> > +typedef uint32_t (*ns16550_calculate_baud_divisor)(ns16550_context *ctx, 
> > uint32_t baud);
> > +
> > +struct ns16550_context{
> >rtems_termios_device_context base;
> >ns16550_get_reg get_reg;
> >ns16550_set_reg set_reg;
> > @@ -70,6 +74,7 @@ typedef struct {
> >uint32_t initial_baud;
> >bool has_fractional_divider_register;
> >bool has_precision_clock_synthesizer;
> > +  ns16550_calculate_baud_divisor calculate_baud_divisor;
> >uint8_t modem_control;
> >uint8_t line_control;
> >uint32_t baud_divisor;
> > @@ -78,7 +83,7 @@ typedef struct {
> >size_t out_current;
> >const char *out_buf;
> >rtems_termios_tty *tty;
> > -} ns16550_context;
> > +};
> >
> >  extern const rtems_termios_device_handler ns16550_handler_interrupt;
> >  extern const rtems_termios_device_handler ns16550_handler_polled;
> > diff --git a/bsps/shared/dev/serial/ns16550-context.c 
> > b/bsps/shared/dev/serial/ns16550-context.c
> > index ce55b8309cc..70e73fc8c93 100644
> > --- a/bsps/shared/dev/serial/ns16550-context.c
> > +++ b/bsps/shared/dev/serial/ns16550-context.c
> > @@ -1,6 +1,6 @@
> >  /**
> >   *  @file
> > - *
> > + *
>
> Again: Please only change if it is necessary for your patch.
>
> >   *  This file contains the TTY driver for the National Semiconductor 
> > NS16550.
> >   *
> >   *  This part is widely cloned and second sourced.  It is found in a number
> > @@ -112,6 +112,8 @@ static uint32_t NS16550_GetBaudDivisor(ns16550_context 
> > *ctx, uint32_t baud)
> >NS16550_FRACTIONAL_DIVIDER,
> >fractionalDivider
> >  );
> > +  } else if (ctx->calculate_baud_divisor != NULL) {
> > +baudDivisor = ctx->calculate_baud_divisor(ctx, baud);
> >}
> >
> >return baudDivisor;
> > @@ -165,7 +167,7 @@ bool ns16550_probe(rtems_termios_device_context *base)
> >
> >ctx->modem_control = SP_MODEM_IRQ;
> >
> > -  pNS16550 = ctx->port;
> > +  pNS16550 = ctx->port;
>
> Again: Please only change if it is necessary for your patch.
>

It is however good to make style changes when you rework a file. Just
layer the patch on top of your functional changes.

> >setReg   = ctx->set_reg;
> >getReg   = ctx->get_reg;
> >
> >
> ___
> devel mailing list
> devel@rtems.org
> http://lists.rtems.org/mailman/listinfo/devel
___
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel

Re: [PATCH] bsps/arm/shared: Add GICv3 implementation

2020-01-17 Thread Jeff Kubascik

Hello,

Have there been any updates on this patch? I do not see it in mainline.

I am currently using it for work on the arm/xen BSP to add support for platforms
with a GICv3 interrupt controller. With a few minor changes, I have confirm that
it works with the Xen hypervisor on qemu. Before I can submit these patches,
though, this patch would need to be accepted first.

On 8/16/2019 3:14 PM, Kinsey Moore wrote:
> This adds support for the GICv3 interrupt controller along with the
> redistributor to control SGIs and PPIs which wasn't present in GICv2
> implementations. GICv3 implementations only optionally support
> memory-mapped GICC interface interaction and require system register
> access be implemented, so the GICC interface is accessed only
> through system registers.
> ---
>  bsps/arm/include/bsp/arm-gic-irq.h  |  15 +-
>  bsps/arm/include/bsp/arm-gic-regs.h |  76 -
>  bsps/arm/shared/irq/irq-gic.c   |  16 ++
>  bsps/arm/shared/irq/irq-gicv3.c | 329 
> 
>  4 files changed, 427 insertions(+), 9 deletions(-)
>  create mode 100644 bsps/arm/shared/irq/irq-gicv3.c
> 
> diff --git a/bsps/arm/include/bsp/arm-gic-irq.h 
> b/bsps/arm/include/bsp/arm-gic-irq.h
> index b3e893de72..219c3c7189 100644
> --- a/bsps/arm/include/bsp/arm-gic-irq.h
> +++ b/bsps/arm/include/bsp/arm-gic-irq.h
> @@ -85,6 +85,12 @@ typedef enum {
>ARM_GIC_IRQ_SOFTWARE_IRQ_TO_SELF
>  } arm_gic_irq_software_irq_target_filter;
>  
> +void arm_gic_trigger_sgi(
> +  rtems_vector_number vector,
> +  arm_gic_irq_software_irq_target_filter filter,
> +  uint8_t targets
> +);
> +
>  static inline rtems_status_code arm_gic_irq_generate_software_irq(
>rtems_vector_number vector,
>arm_gic_irq_software_irq_target_filter filter,
> @@ -94,14 +100,7 @@ static inline rtems_status_code 
> arm_gic_irq_generate_software_irq(
>rtems_status_code sc = RTEMS_SUCCESSFUL;
>  
>if (vector <= ARM_GIC_IRQ_SGI_15) {
> -volatile gic_dist *dist = ARM_GIC_DIST;
> -
> -dist->icdsgir = GIC_DIST_ICDSGIR_TARGET_LIST_FILTER(filter)
> -  | GIC_DIST_ICDSGIR_CPU_TARGET_LIST(targets)
> -#ifdef BSP_ARM_GIC_ENABLE_FIQ_FOR_GROUP_0
> -  | GIC_DIST_ICDSGIR_NSATT
> -#endif
> -  | GIC_DIST_ICDSGIR_SGIINTID(vector);
> +arm_gic_trigger_sgi(vector, filter, targets);
>} else {
>  sc = RTEMS_INVALID_ID;
>}
> diff --git a/bsps/arm/include/bsp/arm-gic-regs.h 
> b/bsps/arm/include/bsp/arm-gic-regs.h
> index 2915313b71..8a65294b6f 100644
> --- a/bsps/arm/include/bsp/arm-gic-regs.h
> +++ b/bsps/arm/include/bsp/arm-gic-regs.h
> @@ -86,7 +86,18 @@ typedef struct {
>  } gic_cpuif;
>  
>  typedef struct {
> +  /* GICD_CTLR */
>uint32_t icddcr;
> +/* GICv3 only */
> +#define GIC_DIST_ICDDCR_RWP BSP_BIT32(31)
> +#define GIC_DIST_ICDDCR_E1NWF BSP_BIT32(7)
> +#define GIC_DIST_ICDDCR_DS BSP_BIT32(6)
> +#define GIC_DIST_ICDDCR_ARE_NS BSP_BIT32(5)
> +#define GIC_DIST_ICDDCR_ARE_S BSP_BIT32(4)
> +#define GIC_DIST_ICDDCR_ENABLE_GRP1S BSP_BIT32(2)
> +#define GIC_DIST_ICDDCR_ENABLE_GRP1NS BSP_BIT32(1)
> +#define GIC_DIST_ICDDCR_ENABLE_GRP0 BSP_BIT32(0)
> +/* GICv1/GICv2 */
>  #define GIC_DIST_ICDDCR_ENABLE_GRP_1 BSP_BIT32(1)
>  #define GIC_DIST_ICDDCR_ENABLE BSP_BIT32(0)
>uint32_t icdictr;
> @@ -126,7 +137,9 @@ typedef struct {
>uint8_t icdiptr[256];
>uint32_t reserved_900[192];
>uint32_t icdicfr[64];
> -  uint32_t reserved_d00[128];
> +  /* GICD_IGRPMODR GICv3 only, reserved in GICv1/GICv2 */
> +  uint32_t icdigmr[32];
> +  uint32_t reserved_d80[96];
>uint32_t icdsgir;
>  #define GIC_DIST_ICDSGIR_TARGET_LIST_FILTER(val) BSP_FLD32(val, 24, 25)
>  #define GIC_DIST_ICDSGIR_TARGET_LIST_FILTER_GET(reg) BSP_FLD32GET(reg, 24, 
> 25)
> @@ -140,4 +153,65 @@ typedef struct {
>  #define GIC_DIST_ICDSGIR_SGIINTID_SET(reg, val) BSP_FLD32SET(reg, val, 0, 3)
>  } gic_dist;
>  
> +/* GICv3 only */
> +typedef struct {
> +  /* GICR_CTLR */
> +  uint32_t icrrcr;
> +#define GIC_REDIST_ICRRCR_UWP BSP_BIT32(31)
> +#define GIC_REDIST_ICRRCR_DPG1S BSP_BIT32(26)
> +#define GIC_REDIST_ICRRCR_DPG1NS BSP_BIT32(25)
> +#define GIC_REDIST_ICRRCR_DPG0 BSP_BIT32(24)
> +#define GIC_REDIST_ICRRCR_RWP BSP_BIT32(4)
> +#define GIC_REDIST_ICRRCR_ENABLE_LPI BSP_BIT32(0)
> +  uint32_t icriidr;
> +  uint64_t icrtyper;
> +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE(val) BSP_FLD64(val, 32, 63)
> +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE_GET(reg) BSP_FLD64GET(reg, 32, 63)
> +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE_SET(reg, val) BSP_FLD64SET(reg, 
> val, 32, 63)
> +#define GIC_REDIST_ICRTYPER_COMMON_LPI_AFFINITY(val) BSP_FLD64(val, 24, 25)
> +#define GIC_REDIST_ICRTYPER_COMMON_LPI_AFFINITY_GET(reg) BSP_FLD64GET(reg, 
> 24, 25)
> +#define GIC_REDIST_ICRTYPER_COMMON_LPI_AFFINITY_SET(reg, val) 
> BSP_FLD64SET(reg, val, 24, 25)
> +#define GIC_REDIST_ICRTYPER_CPU_NUMBER(val) BSP_FLD64(val, 8, 23)
> +#define GIC_REDIST_ICRTYPER_CPU_NUMBER_GET(reg) BSP_FLD64GET(reg, 8, 23)
> +#define GIC_REDIST_ICRTYPER_CPU_NUMBER_SET(reg, val) BS

Re: [PATCH] bsps/arm/shared: Add GICv3 implementation

2020-01-17 Thread Joel Sherrill

On Fri, Jan 17, 2020 at 1:24 PM Jeff Kubascik 
wrote:

> Hello,
>
> Have there been any updates on this patch? I do not see it in mainline.
>

I was hoping someone would comment. :)


>
> I am currently using it for work on the arm/xen BSP to add support for
> platforms
> with a GICv3 interrupt controller. With a few minor changes, I have
> confirm that
> it works with the Xen hypervisor on qemu. Before I can submit these
> patches,
> though, this patch would need to be accepted first.
>

Thank you for using it and reporting back. I just pushed it and look
forward to your
patch.

CC'ing Kinsey so he is on the lookout for your patch and verifies it still
works
on HPSC Qemu.

--joel


> On 8/16/2019 3:14 PM, Kinsey Moore wrote:
> > This adds support for the GICv3 interrupt controller along with the
> > redistributor to control SGIs and PPIs which wasn't present in GICv2
> > implementations. GICv3 implementations only optionally support
> > memory-mapped GICC interface interaction and require system register
> > access be implemented, so the GICC interface is accessed only
> > through system registers.
> > ---
> >  bsps/arm/include/bsp/arm-gic-irq.h  |  15 +-
> >  bsps/arm/include/bsp/arm-gic-regs.h |  76 -
> >  bsps/arm/shared/irq/irq-gic.c   |  16 ++
> >  bsps/arm/shared/irq/irq-gicv3.c | 329
> 
> >  4 files changed, 427 insertions(+), 9 deletions(-)
> >  create mode 100644 bsps/arm/shared/irq/irq-gicv3.c
> >
> > diff --git a/bsps/arm/include/bsp/arm-gic-irq.h
> b/bsps/arm/include/bsp/arm-gic-irq.h
> > index b3e893de72..219c3c7189 100644
> > --- a/bsps/arm/include/bsp/arm-gic-irq.h
> > +++ b/bsps/arm/include/bsp/arm-gic-irq.h
> > @@ -85,6 +85,12 @@ typedef enum {
> >ARM_GIC_IRQ_SOFTWARE_IRQ_TO_SELF
> >  } arm_gic_irq_software_irq_target_filter;
> >
> > +void arm_gic_trigger_sgi(
> > +  rtems_vector_number vector,
> > +  arm_gic_irq_software_irq_target_filter filter,
> > +  uint8_t targets
> > +);
> > +
> >  static inline rtems_status_code arm_gic_irq_generate_software_irq(
> >rtems_vector_number vector,
> >arm_gic_irq_software_irq_target_filter filter,
> > @@ -94,14 +100,7 @@ static inline rtems_status_code
> arm_gic_irq_generate_software_irq(
> >rtems_status_code sc = RTEMS_SUCCESSFUL;
> >
> >if (vector <= ARM_GIC_IRQ_SGI_15) {
> > -volatile gic_dist *dist = ARM_GIC_DIST;
> > -
> > -dist->icdsgir = GIC_DIST_ICDSGIR_TARGET_LIST_FILTER(filter)
> > -  | GIC_DIST_ICDSGIR_CPU_TARGET_LIST(targets)
> > -#ifdef BSP_ARM_GIC_ENABLE_FIQ_FOR_GROUP_0
> > -  | GIC_DIST_ICDSGIR_NSATT
> > -#endif
> > -  | GIC_DIST_ICDSGIR_SGIINTID(vector);
> > +arm_gic_trigger_sgi(vector, filter, targets);
> >} else {
> >  sc = RTEMS_INVALID_ID;
> >}
> > diff --git a/bsps/arm/include/bsp/arm-gic-regs.h
> b/bsps/arm/include/bsp/arm-gic-regs.h
> > index 2915313b71..8a65294b6f 100644
> > --- a/bsps/arm/include/bsp/arm-gic-regs.h
> > +++ b/bsps/arm/include/bsp/arm-gic-regs.h
> > @@ -86,7 +86,18 @@ typedef struct {
> >  } gic_cpuif;
> >
> >  typedef struct {
> > +  /* GICD_CTLR */
> >uint32_t icddcr;
> > +/* GICv3 only */
> > +#define GIC_DIST_ICDDCR_RWP BSP_BIT32(31)
> > +#define GIC_DIST_ICDDCR_E1NWF BSP_BIT32(7)
> > +#define GIC_DIST_ICDDCR_DS BSP_BIT32(6)
> > +#define GIC_DIST_ICDDCR_ARE_NS BSP_BIT32(5)
> > +#define GIC_DIST_ICDDCR_ARE_S BSP_BIT32(4)
> > +#define GIC_DIST_ICDDCR_ENABLE_GRP1S BSP_BIT32(2)
> > +#define GIC_DIST_ICDDCR_ENABLE_GRP1NS BSP_BIT32(1)
> > +#define GIC_DIST_ICDDCR_ENABLE_GRP0 BSP_BIT32(0)
> > +/* GICv1/GICv2 */
> >  #define GIC_DIST_ICDDCR_ENABLE_GRP_1 BSP_BIT32(1)
> >  #define GIC_DIST_ICDDCR_ENABLE BSP_BIT32(0)
> >uint32_t icdictr;
> > @@ -126,7 +137,9 @@ typedef struct {
> >uint8_t icdiptr[256];
> >uint32_t reserved_900[192];
> >uint32_t icdicfr[64];
> > -  uint32_t reserved_d00[128];
> > +  /* GICD_IGRPMODR GICv3 only, reserved in GICv1/GICv2 */
> > +  uint32_t icdigmr[32];
> > +  uint32_t reserved_d80[96];
> >uint32_t icdsgir;
> >  #define GIC_DIST_ICDSGIR_TARGET_LIST_FILTER(val) BSP_FLD32(val, 24, 25)
> >  #define GIC_DIST_ICDSGIR_TARGET_LIST_FILTER_GET(reg) BSP_FLD32GET(reg,
> 24, 25)
> > @@ -140,4 +153,65 @@ typedef struct {
> >  #define GIC_DIST_ICDSGIR_SGIINTID_SET(reg, val) BSP_FLD32SET(reg, val,
> 0, 3)
> >  } gic_dist;
> >
> > +/* GICv3 only */
> > +typedef struct {
> > +  /* GICR_CTLR */
> > +  uint32_t icrrcr;
> > +#define GIC_REDIST_ICRRCR_UWP BSP_BIT32(31)
> > +#define GIC_REDIST_ICRRCR_DPG1S BSP_BIT32(26)
> > +#define GIC_REDIST_ICRRCR_DPG1NS BSP_BIT32(25)
> > +#define GIC_REDIST_ICRRCR_DPG0 BSP_BIT32(24)
> > +#define GIC_REDIST_ICRRCR_RWP BSP_BIT32(4)
> > +#define GIC_REDIST_ICRRCR_ENABLE_LPI BSP_BIT32(0)
> > +  uint32_t icriidr;
> > +  uint64_t icrtyper;
> > +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE(val) BSP_FLD64(val, 32, 63)
> > +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE_GET(reg) BSP_FLD64GET(reg,
> 32, 63)
> > +#define GIC_REDIST_ICRTYPER_AFFINITY_VALUE_SET(reg

Re: [PATCH] bsp/raspberrypi: AUX uart driver

Re: [PATCH] bsp/raspberrypi: AUX uart driver

Re: [PATCH] bsp/raspberrypi: AUX uart driver

[PATCH 2/2] bsp/raspberrypi: Mini UART driver

[PATCH 1/2] libchip/ns16550: Allow user calculate baud divisor

Re: [PATCH] libchip/ns16550: Allow user calculate baud divisor

Re: [PATCH] bsps/arm/shared: Add GICv3 implementation

Re: [PATCH] bsps/arm/shared: Add GICv3 implementation

8 matches

Site Navigation

Mail list logo

Footer information