[PATCH v5] Raspberrypi: updated the console interface.

Christian Mauderer list at c-mauderer.de
Thu Jan 2 16:37:28 UTC 2020


There is a whitespace error:

> git am ~/\[PATCH\ v5\]\ Raspberrypi:\ updated\ the\ console\
interface..eml
Applying: Raspberrypi: updated the console interface.
/home/christian/rtems/rtems-bbb/.git/modules/rtems/rebase-apply/patch:732:
trailing whitespace.
 * @name Bus to Physical address translation
warning: 1 line adds whitespace errors.

The patch starts to get a good direction. Most of the points I found are
only small stuff.

On 01/01/2020 10:14, G S Niteesh wrote:
> Replaced the older console api with newer FDT based
> console driver.
> Replaces the custom pl011 driver with RTEMS arm-pl011
> driver.
> ---
>  bsps/arm/raspberrypi/console/console-config.c | 161 +++++++++++++----
>  bsps/arm/raspberrypi/console/console_select.c | 114 ------------
>  bsps/arm/raspberrypi/console/fbcons.c         |  78 ++++----
>  bsps/arm/raspberrypi/console/usart.c          | 167 ------------------
>  bsps/arm/raspberrypi/include/bsp.h            |   4 +
>  bsps/arm/raspberrypi/include/bsp/fbcons.h     |  21 ++-
>  .../arm/raspberrypi/include/bsp/raspberrypi.h |  64 +++----
>  bsps/arm/raspberrypi/include/bsp/usart.h      |   5 +-
>  c/src/lib/libbsp/arm/raspberrypi/Makefile.am  |   7 +-
>  c/src/lib/libbsp/arm/raspberrypi/configure.ac |  13 ++
>  10 files changed, 228 insertions(+), 406 deletions(-)
>  delete mode 100644 bsps/arm/raspberrypi/console/console_select.c
>  delete mode 100644 bsps/arm/raspberrypi/console/usart.c
> 
> diff --git a/bsps/arm/raspberrypi/console/console-config.c b/bsps/arm/raspberrypi/console/console-config.c
> index d2186c918b..c47021e54e 100644
> --- a/bsps/arm/raspberrypi/console/console-config.c
> +++ b/bsps/arm/raspberrypi/console/console-config.c
> @@ -19,50 +19,145 @@
>   */
>  
>  #include <rtems/bspIo.h>
> +#include <rtems/console.h>
> +#include <rtems/sysinit.h>
>  
> -#include <libchip/serial.h>
>  
> -#include <bspopts.h>
> -#include <bsp/irq.h>
> -#include <bsp/usart.h>
> +#include <bsp.h>
>  #include <bsp/raspberrypi.h>
> +#include <bsp/usart.h>
> +#include <bsp/arm-pl011.h>
>  #include <bsp/fbcons.h>
> +#include <bsp/console-termios.h>
> +#include <bsp/fdt.h>
> +#include <bsp/fatal.h>
> +
> +#include <bspopts.h>

Why did you move the bspopts.h from top to bottom?

>  
> -console_tbl Console_Configuration_Ports [] = {
> -    {
> -      .sDeviceName = "/dev/ttyS0",
> -      .deviceType = SERIAL_CUSTOM,
> -      .pDeviceFns = &bcm2835_usart_fns,
> -      .deviceProbe = NULL,
> -      .pDeviceFlow = NULL,
> -      .ulCtrlPort1 = BCM2835_UART0_BASE,
> -      .ulCtrlPort2 = 0,
> -      .ulClock = USART0_DEFAULT_BAUD,
> -      .ulIntVector = BCM2835_IRQ_ID_UART
> -    },
> -    {
> -      .sDeviceName ="/dev/fbcons",
> -      .deviceType = SERIAL_CUSTOM,
> -      .pDeviceFns = &fbcons_fns,
> -      .deviceProbe = fbcons_probe,
> -      .pDeviceFlow = NULL,
> -    },
> -};
> -
> -#define PORT_COUNT \
> -  (sizeof(Console_Configuration_Ports) \
> -    / sizeof(Console_Configuration_Ports [0]))
> -
> -unsigned long Console_Configuration_Count = PORT_COUNT;
> +#include <libfdt.h>
> +#include <libchip/serial.h>
> +
> +arm_pl011_context pl011_context;
> +
> +rpi_fb_context fb_context;
> +
> +static void output_char_serial(char c)
> +{
> +  arm_pl011_write_polled(&pl011_context.base, c);
> +}
> +
> +void output_char_fb(char c)
> +{
> +  fbcons_write_polled(&fb_context.base, c);
> +}
> +
> +
> +static void *get_reg_of_node(const void *fdt, int node)
> +{
> +  int len;
> +  const uint32_t *val;
> +
> +  val = fdt_getprop(fdt, node, "reg", &len);
> +  if (val == NULL || len < 4) {
> +    return NULL;
> +  }
> +
> +  return (void *) fdt32_to_cpu(val[0]);
> +}

It would be good to make that function more widely available. Other
drivers will need it too sooner or later. Please follow the convention
used in imx to add the bsp-name as a prefix. So this will be
raspberry_get_reg_of_node or bcm2835_get_reg_of_node (I think both
prefixes are already used in the BSP).

> +
> +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");
> +  ctx->regs = BUS_TO_PHY(get_reg_of_node(fdt, node));

Why do you do the BUS_TO_PHY here and not in the get_reg_of_node()? I
would expect the get_reg_of_node to allways return the usable address
which is the physical address for RTEMS.

> +}
> +
> +static void register_fb( void )
> +{
> +  if (fbcons_probe(&fb_context.base) == true) {
> +    rtems_termios_device_install(
> +      "/dev/fbcons",
> +      &fbcons_fns,
> +      NULL,
> +      &fb_context.base);
> +  }
> +}
> +
> +static void console_select( const char *console )
> +{
> +  const char *opt;
> +
> +  opt = rpi_cmdline_get_arg("--console=");
> +
> +  if ( opt ) {
> +    if ( strncmp( opt, "fbcons", sizeof( "fbcons" ) - 1 ) == 0 ) {
> +      if ( rpi_video_is_initialized() > 0 ) {
> +        BSP_output_char = output_char_fb;
> +        link("/dev/fbcons", CONSOLE_DEVICE_NAME);
> +        return ;
> +      }
> +    }else{
> +      if ( console == NULL ){
> +        bsp_fatal( BSP_FATAL_CONSOLE_NO_DEV );
> +      }
> +      link("/dev/ttyuart0", CONSOLE_DEVICE_NAME);
> +    }
> +  }
> +  BSP_output_char = output_char_serial;
> +}
> +
> +static void uart_probe(void)
> +{


Currently uart_probe is reached multiple times:

1. Via the initial output_char function because it is called in bsp_start().

2. Via the SYSINIT_ITEM.

Therefore you should add some logic to prevent a double execution of
this code. I'm not sure whether initializeing the pl011 twice is healthy
(although it seems to work most of the time).

> +  const void *fdt;
> +  const char *console;
> +  int node;
> +
> +  fdt = bsp_fdt_get();
> +  node = fdt_path_offset(fdt, "/chosen");
> +
> +  console = fdt_getprop(fdt, node, "stdout-path", NULL);
> +
> +  node = fdt_node_offset_by_compatible(fdt, -1, "brcm,bcm2835-pl011");
> +  init_ctx_arm_pl011(fdt, node);
> +
> +  console_select(console);

Is it a good idea to do the console_select here? The frame buffer isn't
necessarily initialized yet but it can get selected. I would move it to
console_initialize().

> +}
>  
>  static void output_char(char c)
>  {
> -  const console_fns *con =
> -    Console_Configuration_Ports [Console_Port_Minor].pDeviceFns;
> +  uart_probe();
> +  (*BSP_output_char)(c);
> +}
>  
> -  con->deviceWritePolled((int) Console_Port_Minor, c);
> +rtems_status_code console_initialize(
> +  rtems_device_major_number major,
> +  rtems_device_minor_number minor,
> +  void *arg
> +)
> +{
> +  rtems_termios_initialize();
> +
> +  uart_probe();
> +
> +  rtems_termios_device_install( "/dev/ttyuart0",


Seems that my suggestions were not clear. Pleathe either be backward
compatible and use the same name as before ("ttyS0"). Or generate the
name based on the FDT.

As soon as multiple ports could be supported (Pi4?) generating the name
would be the better solution. But with the still fixed context it isn't
that usefull. So please use the backward compatible name for now so that
this patch can be finalized.


> +    &arm_pl011_fns,
> +    NULL,
> +    &pl011_context.base
> +  );> +  register_fb();
> +
> +  return RTEMS_SUCCESSFUL;
>  }
>  
>  BSP_output_char_function_type BSP_output_char = output_char;
>  
>  BSP_polling_getchar_function_type BSP_poll_char = NULL;
> +
> +RTEMS_SYSINIT_ITEM(
> +  uart_probe,
> +  RTEMS_SYSINIT_BSP_START,
> +  RTEMS_SYSINIT_ORDER_LAST
> +);
> \ No newline at end of file
> diff --git a/bsps/arm/raspberrypi/console/console_select.c b/bsps/arm/raspberrypi/console/console_select.c
> deleted file mode 100644
> index bd246ca868..0000000000
> --- a/bsps/arm/raspberrypi/console/console_select.c
> +++ /dev/null
> @@ -1,114 +0,0 @@
> -/**
> - * @file
> - *
> - * @ingroup raspberrypi_console
> - *
> - * @brief console select
> - */
> -
> -/*
> - * Copyright (c) 2015 Yang Qiao
> - *
> - *  The license and distribution terms for this file may be
> - *  found in the file LICENSE in this distribution or at
> - *
> - *  http://www.rtems.org/license/LICENSE
> - *
> - */
> -
> -#include <bsp.h>
> -#include <bsp/fatal.h>
> -#include <rtems/libio.h>
> -#include <stdlib.h>
> -#include <string.h>
> -#include <assert.h>
> -#include <termios.h>
> -
> -#include <rtems/termiostypes.h>
> -#include <libchip/serial.h>
> -#include "../../shared/dev/serial/legacy-console.h"
> -#include <bsp/rpi-fb.h>
> -
> -rtems_device_minor_number BSPPrintkPort = 0;
> -
> -/*
> - * Method to return true if the device associated with the
> - * minor number probs available.
> - */
> -static bool bsp_Is_Available( rtems_device_minor_number minor )
> -{
> -  console_tbl *cptr = Console_Port_Tbl[ minor ];
> -
> -  /*
> -   * First perform the configuration dependent probe, then the
> -   * device dependent probe
> -   */
> -  if ( ( !cptr->deviceProbe || cptr->deviceProbe( minor ) ) &&
> -       cptr->pDeviceFns->deviceProbe( minor ) ) {
> -    return true;
> -  }
> -
> -  return false;
> -}
> -
> -/*
> - * Method to return the first available device.
> - */
> -static rtems_device_minor_number bsp_First_Available_Device( void )
> -{
> -  rtems_device_minor_number minor;
> -
> -  for ( minor = 0; minor < Console_Port_Count; minor++ ) {
> -    console_tbl *cptr = Console_Port_Tbl[ minor ];
> -
> -    /*
> -     * First perform the configuration dependent probe, then the
> -     * device dependent probe
> -     */
> -
> -    if ( ( !cptr->deviceProbe || cptr->deviceProbe( minor ) ) &&
> -         cptr->pDeviceFns->deviceProbe( minor ) ) {
> -      return minor;
> -    }
> -  }
> -
> -  /*
> -   *  Error No devices were found.  We will want to bail here.
> -   */
> -  bsp_fatal( BSP_FATAL_CONSOLE_NO_DEV );
> -}
> -
> -void bsp_console_select( void )
> -{
> -  /*
> -   *  Reset Console_Port_Minor and
> -   *  BSPPrintkPort here if desired.
> -   *
> -   *  This default version allows the bsp to set these
> -   *  values at creation and will not touch them again
> -   *  unless the selected port number is not available.
> -   */
> -  const char *opt;
> -
> -  Console_Port_Minor = BSP_CONSOLE_UART0;
> -  BSPPrintkPort = BSP_CONSOLE_UART0;
> -
> -  opt = rpi_cmdline_get_arg( "--console=" );
> -
> -  if ( opt ) {
> -    if ( strncmp( opt, "fbcons", sizeof( "fbcons" ) - 1 ) == 0 ) {
> -      if ( rpi_video_is_initialized() > 0 ) {
> -        Console_Port_Minor = BSP_CONSOLE_FB;
> -        BSPPrintkPort = BSP_CONSOLE_FB;
> -      }
> -    }
> -  }
> -
> -  /*
> -   * If the device that was selected isn't available then
> -   * let the user know and select the first available device.
> -   */
> -  if ( !bsp_Is_Available( Console_Port_Minor ) ) {
> -    Console_Port_Minor = bsp_First_Available_Device();
> -  }
> -}
> diff --git a/bsps/arm/raspberrypi/console/fbcons.c b/bsps/arm/raspberrypi/console/fbcons.c
> index 3669ba458d..0e6a430c54 100644
> --- a/bsps/arm/raspberrypi/console/fbcons.c
> +++ b/bsps/arm/raspberrypi/console/fbcons.c
> @@ -18,6 +18,7 @@
>  
>  #include <rtems.h>
>  #include <rtems/libio.h>
> +#include <rtems/termiostypes.h>
>  
>  #include <stdlib.h>
>  
> @@ -29,15 +30,6 @@
>  #include <bsp/vc.h>
>  #include <bsp/rpi-fb.h>
>  
> -/*
> - *  fbcons_init
> - *
> - *  This function initializes the fb console to a quiecsent state.
> - */
> -static void fbcons_init( int minor )
> -{
> -}
> -
>  /*
>   *  fbcons_open
>   *
> @@ -45,13 +37,14 @@ static void fbcons_init( int minor )
>   *
>   *  Default state is 9600 baud, 8 bits, No parity, and 1 stop bit.
>   */
> -static int fbcons_open(
> -  int   major,
> -  int   minor,
> -  void *arg
> +static bool fbcons_open(
> +  struct rtems_termios_tty *tty,
> +  rtems_termios_device_context *base,
> +  struct termios *term,
> +  rtems_libio_open_close_args_t *args
>  )
>  {
> -  return RTEMS_SUCCESSFUL;
> +  return true;
>  }
>  
>  /*
> @@ -59,13 +52,12 @@ static int fbcons_open(
>   *
>   *  This function shuts down the requested port.
>   */
> -static int fbcons_close(
> -  int   major,
> -  int   minor,
> -  void *arg
> +static void fbcons_close(
> +  struct rtems_termios_tty *tty,
> +  rtems_termios_device_context *base,
> +  rtems_libio_open_close_args_t *args
>  )
>  {
> -  return ( RTEMS_SUCCESSFUL );
>  }
>  
>  /*
> @@ -73,8 +65,8 @@ static int fbcons_close(
>   *
>   *  This routine polls out the requested character.
>   */
> -static void fbcons_write_polled(
> -  int  minor,
> +void fbcons_write_polled(
> +  rtems_termios_device_context *base,
>    char c
>  )
>  {
> @@ -90,8 +82,8 @@ static void fbcons_write_polled(
>   *  Console Termios output entry point when using polled output.
>   *
>   */
> -static ssize_t fbcons_write_support_polled(
> -  int         minor,
> +static void fbcons_write_support_polled(
> +  rtems_termios_device_context *base,
>    const char *buf,
>    size_t      len
>  )
> @@ -102,14 +94,9 @@ static ssize_t fbcons_write_support_polled(
>     * poll each byte in the string out of the port.
>     */
>    while ( nwrite < len ) {
> -    fbcons_write_polled( minor, *buf++ );
> +    fbcons_write_polled( base, *buf++ );
>      nwrite++;
>    }
> -
> -  /*
> -   * return the number of bytes written.
> -   */
> -  return nwrite;
>  }
>  
>  /*
> @@ -117,7 +104,9 @@ static ssize_t fbcons_write_support_polled(
>   *
>   *  Console Termios polling input entry point.
>   */
> -static int fbcons_inbyte_nonblocking_polled( int minor )
> +int fbcons_inbyte_nonblocking_polled(
> +  rtems_termios_device_context *base
> +)
>  {
>    // if( rtems_kbpoll() ) {
>    //   int c = getch();
> @@ -133,15 +122,17 @@ static int fbcons_inbyte_nonblocking_polled( int minor )
>   *  This function sets the UART channel to reflect the requested termios
>   *  port settings.
>   */
> -static int fbcons_set_attributes(
> -  int                   minor,
> +static bool fbcons_set_attributes(
> +  rtems_termios_device_context *base,
>    const struct termios *t
>  )
>  {
> -  return 0;
> +  return true;
>  }
>  
> -bool fbcons_probe( int minor )
> +bool fbcons_probe(
> +  rtems_termios_device_context *context
> +)
>  {
>    // rtems_status_code status;
>    static bool firstTime = true;
> @@ -163,15 +154,12 @@ bool fbcons_probe( int minor )
>    return ret;
>  }
>  
> -const console_fns fbcons_fns =
> +const rtems_termios_device_handler fbcons_fns =
>  {
> -  .deviceProbe = libchip_serial_default_probe,     /* deviceProbe */
> -  .deviceFirstOpen = fbcons_open,                 /* deviceFirstOpen */
> -  .deviceLastClose = fbcons_close,                /* deviceLastClose */
> -  .deviceRead = fbcons_inbyte_nonblocking_polled, /* deviceRead */
> -  .deviceWrite = fbcons_write_support_polled,     /* deviceWrite */
> -  .deviceInitialize = fbcons_init,                /* deviceInitialize */
> -  .deviceWritePolled = fbcons_write_polled,       /* deviceWritePolled */
> -  .deviceSetAttributes = fbcons_set_attributes,   /* deviceSetAttributes */
> -  .deviceOutputUsesInterrupts = FALSE,           /* deviceOutputUsesInterrupts*/
> -};
> +  .first_open = fbcons_open,
> +  .last_close = fbcons_close,
> +  .poll_read = fbcons_inbyte_nonblocking_polled,
> +  .write = fbcons_write_support_polled,
> +  .set_attributes = fbcons_set_attributes,
> +  .mode = TERMIOS_POLLED
> +};
> \ No newline at end of file
> diff --git a/bsps/arm/raspberrypi/console/usart.c b/bsps/arm/raspberrypi/console/usart.c
> deleted file mode 100644
> index 25fb523621..0000000000
> --- a/bsps/arm/raspberrypi/console/usart.c
> +++ /dev/null
> @@ -1,167 +0,0 @@
> -/**
> - * @file
> - *
> - * @ingroup raspberrypi_usart
> - *
> - * @brief USART support.
> - */
> -
> -/*
> - * Copyright (c) 2013 Alan Cudmore
> - *
> - *  The license and distribution terms for this file may be
> - *  found in the file LICENSE in this distribution or at
> - *
> - *  http://www.rtems.org/license/LICENSE
> - *
> - */
> -
> -#include <libchip/sersupp.h>
> -
> -#include <bsp.h>
> -#include <bsp/irq.h>
> -#include <bsp/usart.h>
> -#include <bsp/raspberrypi.h>
> -#include <rtems/bspIo.h>
> -
> -static void usart_delay(uint32_t n)
> -{
> - volatile uint32_t i = 0;
> -
> - for(i = 0; i < n; i++)
> -   ;
> -}
> -
> -#if 0
> -/*
> - *  These will be useful when the driver supports interrupt driven IO.
> - */
> -static rtems_vector_number usart_get_irq_number(const console_tbl *ct)
> -{
> -  return ct->ulIntVector;
> -}
> -
> -static uint32_t usart_get_baud(const console_tbl *ct)
> -{
> -  return ct->ulClock;
> -}
> -#endif
> -
> -static void usart_set_baud(int minor, int baud)
> -{
> - /*
> -  * Nothing for now
> -  */
> - return;
> -}
> -
> -static void usart_initialize(int minor)
> -{
> -  unsigned int gpio_reg;
> -
> -  /*
> -  ** Program GPIO pins for UART 0
> -  */
> -  gpio_reg = BCM2835_REG(BCM2835_GPIO_GPFSEL1);
> -  gpio_reg &= ~(7<<12);    /* gpio14 */
> -  gpio_reg |=  (4<<12);    /* alt0   */
> -  gpio_reg &= ~(7<<15);    /* gpio15 */
> -  gpio_reg |=  (4<<15);    /* alt0   */
> -  BCM2835_REG(BCM2835_GPIO_GPFSEL1) = gpio_reg;
> -
> -  BCM2835_REG(BCM2835_GPIO_GPPUD) = 0;
> -  usart_delay(150);
> -  BCM2835_REG(BCM2835_GPIO_GPPUDCLK0) = (1<<14)|(1<<15);
> -  usart_delay(150);
> -  BCM2835_REG(BCM2835_GPIO_GPPUDCLK0) = 0;
> -
> -  /*
> -  ** Init the PL011 UART
> -  */
> -  BCM2835_REG(BCM2835_UART0_CR)   = 0;
> -  BCM2835_REG(BCM2835_UART0_ICR)  = 0x7FF;
> -  BCM2835_REG(BCM2835_UART0_IMSC) = 0;
> -  BCM2835_REG(BCM2835_UART0_IBRD) = 1;
> -  BCM2835_REG(BCM2835_UART0_FBRD) = 40;
> -  BCM2835_REG(BCM2835_UART0_LCRH) = 0x70;
> -  BCM2835_REG(BCM2835_UART0_RSRECR) =  0;
> -
> -  BCM2835_REG(BCM2835_UART0_CR)   = 0x301;
> -
> -  BCM2835_REG(BCM2835_UART0_IMSC) = BCM2835_UART0_IMSC_RX;
> -
> -  usart_set_baud(minor, 115000);
> -}
> -
> -static int usart_first_open(int major, int minor, void *arg)
> -{
> -  rtems_libio_open_close_args_t *oc = (rtems_libio_open_close_args_t *) arg;
> -  struct rtems_termios_tty *tty = (struct rtems_termios_tty *) oc->iop->data1;
> -  const console_tbl *ct = Console_Port_Tbl [minor];
> -  console_data *cd = &Console_Port_Data [minor];
> -
> -  cd->termios_data = tty;
> -  rtems_termios_set_initial_baud(tty, ct->ulClock);
> -
> -  return 0;
> -}
> -
> -static int usart_last_close(int major, int minor, void *arg)
> -{
> -  return 0;
> -}
> -
> -static int usart_read_polled(int minor)
> -{
> -  if (minor == 0) {
> -    if (((BCM2835_REG(BCM2835_UART0_FR)) & BCM2835_UART0_FR_RXFE) == 0) {
> -       return((BCM2835_REG(BCM2835_UART0_DR)) & 0xFF );
> -    } else {
> -      return -1;
> -    }
> -  } else {
> -    printk("Unknown console minor number: %d\n", minor);
> -    return -1;
> -  }
> -}
> -
> -static void usart_write_polled(int minor, char c)
> -{
> -   while (1) {
> -     if ((BCM2835_REG(BCM2835_UART0_FR) & BCM2835_UART0_FR_TXFF) == 0)
> -       break;
> -   }
> -   BCM2835_REG(BCM2835_UART0_DR) = c;
> -}
> -
> -static ssize_t usart_write_support_polled(
> -  int minor,
> -  const char *s,
> -  size_t n
> -)
> -{
> -  ssize_t i = 0;
> -
> -  for (i = 0; i < n; ++i) {
> -    usart_write_polled(minor, s [i]);
> -  }
> -
> -  return n;
> -}
> -
> -static int usart_set_attributes(int minor, const struct termios *term)
> -{
> -  return -1;
> -}
> -
> -const console_fns bcm2835_usart_fns = {
> -  .deviceProbe = libchip_serial_default_probe,
> -  .deviceFirstOpen = usart_first_open,
> -  .deviceLastClose = usart_last_close,
> -  .deviceRead = usart_read_polled,
> -  .deviceWrite = usart_write_support_polled,
> -  .deviceInitialize = usart_initialize,
> -  .deviceWritePolled = usart_write_polled,
> -  .deviceSetAttributes = usart_set_attributes,
> -  .deviceOutputUsesInterrupts = false
> -};
> diff --git a/bsps/arm/raspberrypi/include/bsp.h b/bsps/arm/raspberrypi/include/bsp.h
> index 1075cb1771..b81d19cd76 100644
> --- a/bsps/arm/raspberrypi/include/bsp.h
> +++ b/bsps/arm/raspberrypi/include/bsp.h
> @@ -41,6 +41,10 @@ extern "C" {
>  
>  #define BSP_FEATURE_IRQ_EXTENSION
>  
> +#if BSP_START_COPY_FDT_FROM_U_BOOT
> +#define BSP_FDT_IS_SUPPORTED
> +#endif
> +
>  #define RPI_L2_CACHE_ENABLE 1
>  
>  #define BSP_GPIO_PIN_COUNT 32
> diff --git a/bsps/arm/raspberrypi/include/bsp/fbcons.h b/bsps/arm/raspberrypi/include/bsp/fbcons.h
> index d0e126699a..9bdb4b115e 100644
> --- a/bsps/arm/raspberrypi/include/bsp/fbcons.h
> +++ b/bsps/arm/raspberrypi/include/bsp/fbcons.h
> @@ -33,12 +33,29 @@ extern "C" {
>  
>  #define FB_CONSOLE 0x50492835
>  
> -bool fbcons_probe( int minor );
> +bool fbcons_probe(
> +  rtems_termios_device_context *base
> + );
> +
> +void fbcons_write_polled(
> +  rtems_termios_device_context *base,
> +  char c
> +);
> +
> +int fbcons_inbyte_nonblocking_polled(
> +  rtems_termios_device_context *base
> +);

Why did add this function to the header? It isn't used outside of
fbcons.c, is it? So it should be possible to make it static.

> +
> +void output_char_fb(char c);
> +
> +typedef struct {
> +    rtems_termios_device_context base;
> +} rpi_fb_context ;
>  
>  /*
>   * Driver function table
>   */
> -extern const console_fns fbcons_fns;
> +extern const rtems_termios_device_handler fbcons_fns;
>  
>  #ifdef __cplusplus
>  }
> diff --git a/bsps/arm/raspberrypi/include/bsp/raspberrypi.h b/bsps/arm/raspberrypi/include/bsp/raspberrypi.h
> index 40c80cf408..68888041e2 100644
> --- a/bsps/arm/raspberrypi/include/bsp/raspberrypi.h
> +++ b/bsps/arm/raspberrypi/include/bsp/raspberrypi.h
> @@ -24,6 +24,7 @@
>  #include <bspopts.h>
>  #include <stdint.h>
>  #include <bsp/utility.h>
> +#include <rtems/termiostypes.h>
>  
>  /**
>   * @defgroup raspberrypi_reg Register Definitions
> @@ -53,13 +54,36 @@
>   */
>  
>  #if (BSP_IS_RPI2 == 1)
> -   #define RPI_PERIPHERAL_BASE      0x3F000000
> +  #define RPI_PERIPHERAL_BASE      0x3F000000
> +  #define BASE_OFFSET              0X3F000000
>  #else
> -   #define RPI_PERIPHERAL_BASE      0x20000000
> +  #define RPI_PERIPHERAL_BASE      0x20000000
> +  #define BASE_OFFSET              0X5E000000
>  #endif
>  
>  #define RPI_PERIPHERAL_SIZE         0x01000000
>  
> +/**
> + * @name Bus to Physical address translation 
> + *       Macro.
> + * @{
> + */
> +
> +#define BUS_TO_PHY(x)            (x - BASE_OFFSET)

Please change that to

   #define BUS_TO_PHY(x)            ((x) - BASE_OFFSET)

Note the brackets around x. Otherwise what happens if I call it with

    BUS_TO_PHY(some_address & ~0xF)

Answer: The operator & (as bitwise and) has a later precedence than
subtraction. So you would end up with basically the following line:

    some_address & (~0xF - BASE_OFFSET)

which is not what you want. That's a general rule for macros: If you use
a parameter allways put brackets arround it. It prevents lots of odd bugs.

> +
> +/** @} */
> +
> +/**
> + * @name Console select definitions
> + *
> + * @{
> + */
> +
> +#define PL011 0
> +#define FB    1

I think these two are never used? Besides that: The names are a bit
short for widely visible defines (especially "FB"). In such a case a
namespace prefix would be good.

> +
> +/** @} */
> +
>  /**
>   * @name Internal ARM Timer Registers
>   *
> @@ -184,42 +208,6 @@
>  
>  /** @} */
>  
> -/**
> - * @name UART 0 (PL011) Registers
> - *
> - * @{
> - */
> -
> -#define BCM2835_UART0_BASE       (RPI_PERIPHERAL_BASE + 0x201000)
> -
> -#define BCM2835_UART0_DR         (BCM2835_UART0_BASE + 0x00)
> -#define BCM2835_UART0_RSRECR     (BCM2835_UART0_BASE + 0x04)
> -#define BCM2835_UART0_FR         (BCM2835_UART0_BASE + 0x18)
> -#define BCM2835_UART0_ILPR       (BCM2835_UART0_BASE + 0x20)
> -#define BCM2835_UART0_IBRD       (BCM2835_UART0_BASE + 0x24)
> -#define BCM2835_UART0_FBRD       (BCM2835_UART0_BASE + 0x28)
> -#define BCM2835_UART0_LCRH       (BCM2835_UART0_BASE + 0x2C)
> -#define BCM2835_UART0_CR         (BCM2835_UART0_BASE + 0x30)
> -#define BCM2835_UART0_IFLS       (BCM2835_UART0_BASE + 0x34)
> -#define BCM2835_UART0_IMSC       (BCM2835_UART0_BASE + 0x38)
> -#define BCM2835_UART0_RIS        (BCM2835_UART0_BASE + 0x3C)
> -#define BCM2835_UART0_MIS        (BCM2835_UART0_BASE + 0x40)
> -#define BCM2835_UART0_ICR        (BCM2835_UART0_BASE + 0x44)
> -#define BCM2835_UART0_DMACR      (BCM2835_UART0_BASE + 0x48)
> -#define BCM2835_UART0_ITCR       (BCM2835_UART0_BASE + 0x80)
> -#define BCM2835_UART0_ITIP       (BCM2835_UART0_BASE + 0x84)
> -#define BCM2835_UART0_ITOP       (BCM2835_UART0_BASE + 0x88)
> -#define BCM2835_UART0_TDR        (BCM2835_UART0_BASE + 0x8C)
> -
> -#define BCM2835_UART0_MIS_RX    0x10
> -#define BCM2835_UART0_MIS_TX    0x20
> -#define BCM2835_UART0_IMSC_RX   0x10
> -#define BCM2835_UART0_IMSC_TX   0x20
> -#define BCM2835_UART0_FR_RXFE   0x10
> -#define BCM2835_UART0_FR_TXFF   0x20
> -#define BCM2835_UART0_ICR_RX    0x10
> -#define BCM2835_UART0_ICR_TX    0x20
> -
>  /** @} */
>  
>  /**
> diff --git a/bsps/arm/raspberrypi/include/bsp/usart.h b/bsps/arm/raspberrypi/include/bsp/usart.h
> index d3e710c5e9..abbf53626c 100644
> --- a/bsps/arm/raspberrypi/include/bsp/usart.h
> +++ b/bsps/arm/raspberrypi/include/bsp/usart.h
> @@ -32,9 +32,8 @@
>  extern "C" {
>  #endif /* __cplusplus */
>  
> -#define USART0_DEFAULT_BAUD 115000
> -
> -extern const console_fns bcm2835_usart_fns;
> +#define PL011_DEFAULT_BAUD 115000
> +#define BCM2835_PL011_BASE (RPI_PERIPHERAL_BASE + 0x201000)
>  
>  #ifdef __cplusplus
>  }
> diff --git a/c/src/lib/libbsp/arm/raspberrypi/Makefile.am b/c/src/lib/libbsp/arm/raspberrypi/Makefile.am
> index 11a22f89e3..70b19178b4 100644
> --- a/c/src/lib/libbsp/arm/raspberrypi/Makefile.am
> +++ b/c/src/lib/libbsp/arm/raspberrypi/Makefile.am
> @@ -42,6 +42,7 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/start/bspfatal-default.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/dev/cpucounter/cpucounterfrequency.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/dev/cpucounter/cpucounterread.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/start/sbrk.c
> +librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/start/bsp-fdt.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/start/stackalloc.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/start/bsp-start-memcpy.S
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/cp15/arm-cp15-set-ttb-entries.c
> @@ -63,14 +64,12 @@ librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/cp15/arm-cp15-set-exc
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/irq/irq.c
>  
>  # Console
> -librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/dev/serial/legacy-console.c
> -librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/dev/serial/legacy-console-control.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/console-config.c
> -librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/console_select.c
> -librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/usart.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/fb.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/fbcons.c
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/console/outch.c
> +librtemsbsp_a_SOURCES += ../../../../../../bsps/shared/dev/serial/console-termios.c
> +librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/shared/serial/arm-pl011.c
>  
>  # Mailbox
>  librtemsbsp_a_SOURCES += ../../../../../../bsps/arm/raspberrypi/start/mailbox.c
> diff --git a/c/src/lib/libbsp/arm/raspberrypi/configure.ac b/c/src/lib/libbsp/arm/raspberrypi/configure.ac
> index 452aa4d029..b1aa0bf417 100644
> --- a/c/src/lib/libbsp/arm/raspberrypi/configure.ac
> +++ b/c/src/lib/libbsp/arm/raspberrypi/configure.ac
> @@ -15,6 +15,19 @@ RTEMS_CANONICAL_TARGET_CPU
>  AM_INIT_AUTOMAKE([no-define nostdinc foreign 1.12.2])
>  RTEMS_BSP_CONFIGURE
>  
> +RTEMS_BSPOPTS_SET([BSP_START_COPY_FDT_FROM_U_BOOT],[*],[1])
> +RTEMS_BSPOPTS_HELP([BSP_START_COPY_FDT_FROM_U_BOOT],[copy the U-Boot provided FDT to an internal storage])
> +
> +RTEMS_BSPOPTS_SET([BSP_FDT_BLOB_SIZE_MAX],[*],[262144])
> +RTEMS_BSPOPTS_HELP([BSP_FDT_BLOB_SIZE_MAX],[maximum size of the FDT blob in bytes])
> +
> +RTEMS_BSPOPTS_SET([BSP_FDT_BLOB_READ_ONLY],[*],[1])
> +RTEMS_BSPOPTS_HELP([BSP_FDT_BLOB_READ_ONLY],[place the FDT blob into the read-only data area])
> +
> +RTEMS_BSPOPTS_SET([BSP_FDT_BLOB_COPY_TO_READ_ONLY_LOAD_AREA],[*],[1])
> +RTEMS_BSPOPTS_HELP([BSP_FDT_BLOB_COPY_TO_READ_ONLY_LOAD_AREA],[copy the FDT blob into the read-only load area via bsp_fdt_copy()])
> +
> +

Please put these changes in an extra patch (something like
"bsp/raspberry: Enable FDT support.". It's an important change (you now
need an fdt) so it should be clearly visible in the history.

>  RTEMS_BSPOPTS_SET([BSP_START_RESET_VECTOR],[*],[])
>  RTEMS_BSPOPTS_HELP([BSP_START_RESET_VECTOR],[reset vector address for BSP start])
>  
> 


More information about the devel mailing list