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

2020-01-16 Thread Christian Mauderer



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.

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


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

2020-01-16 Thread Christian Mauderer
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"
>  #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(

"too many" vs "unsatisfied" status from msg send

2020-01-16 Thread Martin Erik Werner
Hi,

In the documentation for rtems_message_queue_{send,urgent} there are
two limit-related statuses:

* - ``RTEMS_UNSATISFIED`` 
  - out of message buffers
* - ``RTEMS_TOO_MANY``
  - queue's limit has been reached

What is the difference between these?

Superficially I would guess that they both indicate that the queue
already has the maximum amount of messages pending?

>From a brief look at the code, my guess would be that RTEMS_UNSATISFIED
cannot be returned by this function?

While digging in the code, I also noticed that if
RTEMS_SCORE_COREMSG_ENABLE_BLOCKING_SEND is set, the
_CORE_message_queue_Submit() function can return
STATUS_MESSAGE_QUEUE_WAIT_IN_ISR which translates to
RTEMS_INTERNAL_ERROR, is this status deliberately omitted from the
documentation for rtems_message_queue_{send,urgent}?

-- 
Martin Erik Werner 

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


Re: "too many" vs "unsatisfied" status from msg send

2020-01-16 Thread Joel Sherrill
On Thu, Jan 16, 2020 at 2:31 PM Martin Erik Werner <
martinerikwer...@gmail.com> wrote:

> Hi,
>
> In the documentation for rtems_message_queue_{send,urgent} there are
> two limit-related statuses:
>
> * - ``RTEMS_UNSATISFIED``
>   - out of message buffers
> * - ``RTEMS_TOO_MANY``
>   - queue's limit has been reached
>
> What is the difference between these?


It looks like a documentation error to me. RTEMS_UNSATISFIED is returned
on receive when you poll and there isn't a message pending. Looking at
coremsgsubmit.c, I don't see it being returned.

Can you raise a ticket, please?

Superficially I would guess that they both indicate that the queue
> already has the maximum amount of messages pending?
>
> From a brief look at the code, my guess would be that RTEMS_UNSATISFIED
> cannot be returned by this function?
>

Agreed. :)


>
> While digging in the code, I also noticed that if
> RTEMS_SCORE_COREMSG_ENABLE_BLOCKING_SEND is set, the
> _CORE_message_queue_Submit() function can return
> STATUS_MESSAGE_QUEUE_WAIT_IN_ISR which translates to
> RTEMS_INTERNAL_ERROR, is this status deliberately omitted from the
> documentation for rtems_message_queue_{send,urgent}?
>

This is only enabled for POSIX message queues which enable blocking
send.See mq_send man page or Google "mq_send site:opengroup.org"
for the POSIX definition.

You can never block on a Classic API message queue send/urgent, so
the RTEMS_INTERNAL_ERROR seems reasonable. And I wonder

Looking at the Linux man page, it returns EAGAIN if "The queue was
full, and the O_NONBLOCK flag was set for the message queue
description referred to by mqdes."

Since the table maps STATUS_MESSAGE_QUEUE_WAIT_IN_ISR
to ENOMEM, this also looks like a bug which is also (hopefully) documented
to return the same error in the POSIX mq_send RTEMS documentation.

So it looks like another bug to file. Please.

Good eye and analysis. Documentation bugs rarely get reported and I
really do appreciate it.

--joel

> --
> Martin Erik Werner 
>
> ___
> 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

How does the dynamic loader (libdl) hook in to gdb?

2020-01-16 Thread Peter Dufault
I'm trying to hook the SLAC / Til Straumann PowerPC remote debugger stub in to 
what's loaded by "libdl" via "dlopen()".  *I know* this should be integrated 
into "libdebugger", and I do keep my eye on that, but I need to keep what is 
working working and working well.

The "libdl" code has support for GDB both finding out when shared libraries are 
loaded and resolving the new symbols in "rtl-debugger.c".  It's based on NetBSD 
and Android.  I don't know how to get this code to kick in.

I hacked the "libdl" "rtl" command to add an option for "rtl list -m", an 
option that is "rtl list -gm" (Run time loader list a map in a GDB flavor) so 
that after I load a library such as "loaded_lib" the command will produce 
something like:

add-symbol-file loaded_lib  -s .text 0x12c0708 -s .rodata 0x12cbb88 -s .data 
0x12ce910 -s .bss 0x12cea18

After pasting the output of the "rtl list -gm" command into GDB everything 
works wonderfully. There are no remote-stub commands that support this, as far 
as I can tell, and I want to figure out how to do this based on the existing 
remote-stub interface.  Editorial note:  If I can do it with "add-symbol-file" 
I should be able to do it from the remote interface, but so it goes.

My question is: How, without modifying GDB, can I add a symbol table after 
loading something via "dlopen()"?  The interface in "rtl-debugger.c" imply that 
GDB will poke around and set breakpoints based on a SVR4 dynamic loader in 
order to locate the information, but I can't figure out how to do it.

Peter
-
Peter Dufault
HD Associates, Inc.  Software and System Engineering

This email is delivered through the public internet using protocols subject to 
interception and tampering.

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