Raspberrypi3: Mini UART driver

Christian Mauderer list at c-mauderer.de
Fri Dec 27 09:23:23 UTC 2019


Hello Niteesh,

let me repeat two important questions:

- How do you plan to test the changes? Simulator is OK for me but
untested is not OK.

- Isn't it possible to just use the driver from
bsps/arm/shared/serial/arm-pl011.c and remove the BSP specific completely?


On 27/12/2019 06:29, Niteesh wrote:
> On Fri, Dec 27, 2019 at 1:58 AM Christian Mauderer <list at c-mauderer.de
> <mailto:list at c-mauderer.de>> wrote:
> 
>     Hello Niteesh,
> 
>     sorry for not answering earlier. During this time of the year you have
>     to expect some delays on the mailing list due to public holydays and
>     vacations.
> 
> That's okay, I understand. 
> 
>      
> 
>     On 25/12/2019 10:50, Niteesh wrote:
>     > Just to make sure I am going in the right track.
>     > I moved the uart register definitions to bsp/usart.h into a struct of
>     > uint32_t called usart0_regs
>     > here is git diff of usart.c after changing it to the latest console
>     > interface.
> 
>     Do you have a plan how you want to test these changes?
> 
>     The direction looks OK. Some notes below.
> 
>     By the way: Maybe it would be a better idea to just remove it completely
>     and use the bsps/arm/shared/serial/arm-pl011.c driver instead? That
>     reduces the ammount of code and therefore ammount of bugs we have in
>     this BSP. 
> 
>      
> 
>     >
>     > diff --git a/bsps/arm/raspberrypi/console/usart.c
>     > b/bsps/arm/raspberrypi/console/usart.c
>     > index 25fb523621..b12f375a1c 100644
>     > --- a/bsps/arm/raspberrypi/console/usart.c
>     > +++ b/bsps/arm/raspberrypi/console/usart.c
>     > @@ -47,6 +47,12 @@ static uint32_t usart_get_baud(const
>     console_tbl *ct)
>     >  }
>     >  #endif
>     >  
>     > +typedef struct {
>     > +  rtems_termios_device_context base;
>     > +  const char *device_name;
>     > +  volatile usart0_regs *regs;
>     > +}uart0_context;
> 
>     Why uart0_context and not usart_context? All other names in this file
>     are called usart_...
> 
> Sorry, for the inconsistent naming, should I rename it as pl011_context
> since we will be adding
> mini uart for rpi3, IMHO it would be better.

If you touch nearly all locations where the name is used: Yes. But
again: There is already a generic pl011 driver. Use that driver if possible.

> 
>     > +
>     >  static void usart_set_baud(int minor, int baud)
>     >  {
>     >   /*
>     > @@ -55,10 +61,17 @@ static void usart_set_baud(int minor, int baud)
>     >   return;
>     >  }
>     >  
>     > -static void usart_initialize(int minor)
>     > +static volatile usart0_regs
>     > *rpi_uart_get_regs(rtems_termios_device_context *base)
>     >  {
>     > -  unsigned int gpio_reg;
>     > +  uart0_context *ctx;
>     > +
>     > +  ctx = (usart0_regs *) base;
>     > +  return ctx->regs;
>     > +}
>     >  
>     > +static void usart_initialize(rtems_termios_device_context *base)
>     > +{
>     > +  unsigned int gpio_reg;
>     >    /*
>     >    ** Program GPIO pins for UART 0
>     >    */
>     > @@ -75,67 +88,81 @@ static void usart_initialize(int minor)
>     >    usart_delay(150);
>     >    BCM2835_REG(BCM2835_GPIO_GPPUDCLK0) = 0;
>     >  
>     > +  volatile uint32_t *uart_regs = rpi_uart_get_regs(base);
>     > +
>     >    /*
>     >    ** 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);
>     > +  uart_regs->cr  = 0;
>     > +  uart_regs->icr = 0x7ff;
>     > +  uart_regs->imsc = 0;
>     > +  uart_regs->ibrd = 1;
>     > +  uart_regs->fbrd= 40;
>     > +  uart_regs->lcrh= 0x70;
>     > +  uart_regs->rsrecr= 0;
>     > +  uart_regs->cr = 0x301;
>     > +  uart_regs->imsc = BCM2835_UART0_IMSC_RX;
>     > +  // usart_set_baud(minor, 115000);
> 
>     Why is this line commented now?
> 
> It actually does nothing. The function body was empty. The current baud
> rate is set directly
> in the initialization function. I was planning to update it once, I was
> finished with the interface. 
> 

If it is empty: Is it necessary? There is a
rtems_termios_set_initial_baud(...) which should do the same.

> 
>     >  }
>     >  
>     > -static int usart_first_open(int major, int minor, void *arg)
>     > +static bool usart_first_open(
>     > +  rtems_termios_tty *tty,
>     > +  rtems_termios_device_context *base,
>     > +  struct termios *term,
>     > +  rtems_libio_open_close_args_t *args
>     > +)
>     >  {
>     > -  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];
>     > +  rtems_status_code sc;
>     > +  uart0_context *ctx;
>     > +  bool ok;
>     >  
>     > -  cd->termios_data = tty;
>     > -  rtems_termios_set_initial_baud(tty, ct->ulClock);
>     > +  ctx = (uart0_context *) base;
>     >  
>     > -  return 0;
>     > +  usart_initialize(base);
>     > +
>     > +  sc = rtems_termios_set_initial_baud(tty,  USART0_DEFAULT_BAUD);
>     > +  if ( sc != RTEMS_SUCCESSFUL ){
>     > +    printk("Error setting the baud for termios\n");
>     > +    return false;
>     > +  }
> 
>     There is a return missing here. Did you compile the code? The compiler
>     should give you a warning about that. 
> 
>     >  }
>     >  
>     > -static int usart_last_close(int major, int minor, void *arg)
>     > +static int usart_last_close(
> 
>     The first_open returns a bool but last_close returns still an int? Is
>     this correct? I don't have the interface memorized.
> 
> last_close return type is void. 

Then you should fix that here.

>  
> 
>     > +  rtems_termios_tty *tty,
>     > +  rtems_termios_device_context *base,
>     > +  rtems_termios_open_close_args_t *arg)
>     >  {
>     >    return 0;
>     >  }
>     >  
>     > -static int usart_read_polled(int minor)
>     > +static int usart_read_polled(rtems_termios_device_context *base)
>     >  {
>     > -  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;
>     > +  volatile usart0_regs *regs;
>     > +
>     > +  regs = rpi_uart_get_regs(base);
> 
>     Just noted that here: Why rpi_uart_get_regs and not usart_get_regs?
>     Please use a consitent naming scheme.
> 
>     > +
>     > +  if ((regs->fr & BCM2835_UART0_FR_RXFE) == 0) {
>     > +    return (regs->dr & 0xFF);
>     >    }
>     > +
>     > +  return -1;
>     >  }
>     >  
>     > -static void usart_write_polled(int minor, char c)
>     > +static void usart_write_polled(rtems_termios_device_context
>     *base, char c)
>     >  {
>     > -   while (1) {
>     > -     if ((BCM2835_REG(BCM2835_UART0_FR) & BCM2835_UART0_FR_TXFF)
>     == 0)
>     > -       break;
>     > -   }
>     > -   BCM2835_REG(BCM2835_UART0_DR) = c;
>     > +  volatile usart0_regs *regs;
>     > +
>     > +  regs = rpi_uart_get_regs(base);
>     > +
>     > +  while (1) {
>     > +    if (((regs->fr) & BCM2835_UART0_FR_TXFF) == 0)
>     > +      break;
>     > +  }
>     > +  regs->dr = c;
>     >  }
>     >  
>     >  static ssize_t usart_write_support_polled(
>     > -  int minor,
>     > +  rtems_termios_device_context *base,
>     >    const char *s,
>     >    size_t n
>     >  )
>     > @@ -143,7 +170,7 @@ static ssize_t usart_write_support_polled(
>     >    ssize_t i = 0;
>     >  
>     >    for (i = 0; i < n; ++i) {
>     > -    usart_write_polled(minor, s [i]);
>     > +    usart_write_polled(base, s[i]);
>     >    }
>     >  
>     >    return n;
>     > @@ -154,14 +181,11 @@ 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
>     > -};
>     > +const rtems_termios_device_handler bcm2835_uart0_handler_polled = {
>     > +  .first_open = usart_first_open,
>     > +  .last_close = usart_last_close,
>     > +  .poll_read = usart_read_polled,
>     > +  .set_attributes = usart_set_attributes,
>     > +  .write = usart_write_support_polled,
>     > +  .mode = TERMIOS_POLLED
>     > +}
>     > \ No newline at end of file
>     >
>     > On Wed, Dec 25, 2019 at 12:36 AM Joel Sherrill <joel at rtems.org
>     <mailto:joel at rtems.org>
>     > <mailto:joel at rtems.org <mailto:joel at rtems.org>>> wrote:
>     >
>     >
>     >
>     >     On Tue, Dec 24, 2019, 12:19 PM Niteesh <gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >     <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>> wrote:
>     >
>     >         And also the register definitions are in raspberrpi.h file
>     >         should I move them to usart.h.
>     >
>     >
>     >     Sounds right if you mean bsp/usart.h
>     >
>     >         I have a doubt we have a register field in device_context
>     >         typedef struct { 
>     >
>     >             rtems_termios_device_context base; 
>     >
>     >             const char *device_name; 
>     >
>     >             volatile some_chip_registers *regs;
>     >
>     >         } my_driver_context;
>     >
>     >         How does the reg field point to the correct
>     memory location? for
>     >         instance in IMX BSP,
>     >         there is a struct with register field's but none of the
>     define a
>     >         memory location?
>     >
>     >
>     >     Make sure the structure has volatiles and proper alignment. :)
>     >
>     >
>     >         On Tue, Dec 24, 2019 at 11:37 PM Niteesh
>     <gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>
>     >         <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>> wrote:
>     >
>     >             How to handle different serial devices? In other BSPs the
>     >             uart devices are the same, so
>     >             they were able to put it under a single array? But here we
>     >             have 2 uarts and a FB?
>     >
>     >
>     >             On Tue, Dec 24, 2019 at 8:18 PM Christian Mauderer
>     >             <list at c-mauderer.de <mailto:list at c-mauderer.de>
>     <mailto:list at c-mauderer.de <mailto:list at c-mauderer.de>>> wrote:
>     >
>     >                 On 24/12/2019 12:06, Niteesh wrote:
>     >                 > The current raspi console section is like this:
>     >                 > The bsp_console_select in console_select.c is
>     >                 responsible for selecting
>     >                 > between uart and the framebuffer. It does so
>     >                 > by setting the Console_port_minor.
>     >                 > The console_config is responsible for output_char
>     >                 function.
>     >                 > And other files are driver code.
>     >                 > If rewriting, this would be my approach,
>     >                 > Rewrite the bsp_console_select to set some kind of a
>     >                 variable like in
>     >                 > IMX, then in console_initialize function
>     >                 > link the right driver to /dev/console.
>     >                 > Replace the console_tbl with the device_context and
>     >                 console_fns with
>     >                 > termios_device_handlers and
>     >                 > finally add in the console_initialization function.
>     >
>     >                 I agree that this would be a clean solution. So if you
>     >                 want you can do
>     >                 that. But there might is a hurdle: As far as I
>     >                 understood you you only
>     >                 have a Pi3? So you might have a hard time testing the
>     >                 changes. Maybe the
>     >                 simulator could work.
>     >
>     >                 Another possibility could be to set the
>     >                 "Console_port_minor" to
>     >                 something unused (for example -1). In that case
>     you can
>     >                 define another
>     >                 /dev/console.
>     >
>     >                 Best regards and merry Christmas (in case you
>     celebrate)
>     >
>     >                 Christian
>     >
>     >                 >
>     >                 > On Tue, Dec 24, 2019 at 2:13 PM Niteesh
>     >                 <gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>
>     <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>
>     >                 > <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>>
>     >                 wrote:
>     >                 >
>     >                 >     Thank you so much, for such a detailed
>     answer. Now
>     >                 things make
>     >                 >     really good sense to me, 
>     >                 >     going through the code now is just a breeze.
>     But I
>     >                 still have one
>     >                 >     question
>     >                 >     for the newer driver interface is
>     >                 console_initialize the function
>     >                 >     which RTEMS calls while initializing
>     >                 >     the console? Which means I can't mess with the
>     >                 name right? It is
>     >                 >     similar to the main function, right?
>     >                 >
>     >                 >     The current driver is a legacy one, how do you
>     >                 want me to proceed,
>     >                 >     shall I rewrite the legacy to a
>     >                 >     the new one, this is will be a great
>     >                 learning experience for me also
>     >                 >     and we also get the BSP updated to the latest
>     >                 interface.
>     >                 >
>     >                 >
>     >                 >     On Tue, Dec 24, 2019 at 3:20 AM Christian
>     Mauderer
>     >                 >     <list at c-mauderer.de
>     <mailto:list at c-mauderer.de> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>>>
>     >                 wrote:
>     >                 >
>     >                 >         Hello Niteesh,
>     >                 >
>     >                 >         quite a lot of questions. I'll try to answer
>     >                 them. Note that it
>     >                 >         has been
>     >                 >         some time since I had a detailed look at
>     that
>     >                 code so if something I
>     >                 >         tell seems odd please don't hesitate to
>     >                 question it.
>     >                 >
>     >                 >         Please note that in RTEMS their are more or
>     >                 less two "levels" of
>     >                 >         support
>     >                 >         for a serial console:
>     >                 >
>     >                 >         1. A very basic polled system console (also
>     >                 known as
>     >                 >         "debug-console" in
>     >                 >         some BSPs). This one is used for printk and
>     >                 should work in basically
>     >                 >         every case. It is used for critical system
>     >                 messages like
>     >                 >         printing the
>     >                 >         exception frame. For that a BSP has to
>     provide a
>     >                 >         "BSP_output_char" function.
>     >                 >
>     >                 >         2. A full featured UART driver
>     integrated into
>     >                 Termios. That one
>     >                 >         will be
>     >                 >         used for all normal I/O on the UARTs.
>     >                 >
>     >                 >         As far as I know the "console_tbl
>     >                 Console_Configuration_Ports"
>     >                 >         belongs
>     >                 >         to a table based legacy interface. It is
>     >                 handled in the file
>     >                 >         bsps/shared/dev/serial/legacy-console.c. I'm
>     >                 not sure whether it is
>     >                 >         documented in the BSP guide because it
>     >                 shouldn't be used for new
>     >                 >         BSPs.
>     >                 >         Same is true for the "major" and "minor"
>     >                 stuff: It's not really
>     >                 >         used for
>     >                 >         new drivers.
>     >                 >
>     >                 >         Newer drivers use the initialization that is
>     >                 described in the manual
>     >                 >         that you have already found. Basically
>     they use
>     >                 >         "rtems_termios_device_install" to register a
>     >                 new UART as
>     >                 >         "/dev/ttySomething". Some recent (ARM) BSPs
>     >                 that do that are the
>     >                 >         imx or
>     >                 >         the atsam.
>     >                 >
>     >                 >         The console that is used for stdin,
>     stdout and
>     >                 stderr (printf,
>     >                 >         scanf,
>     >                 >         ...) is the one called "/dev/console"
>     (defined in
>     >                 >         CONSOLE_DEVICE_NAME).
>     >                 >         For the legacy table based interface
>     it's the
>     >                 one with the index of
>     >                 >         "Console_Port_Minor".
>     >                 >
>     >                 >
>     >                 >         If you want to access any UART other
>     than the
>     >                 one for stdin and
>     >                 >         stdout
>     >                 >         you do that the same way like on Linux: Just
>     >                 use the "open"
>     >                 >         function on
>     >                 >         the "/dev/ttySomething" and use "read",
>     >                 "write" and simmilar or use
>     >                 >         "fopen" together with "fread", "fwrite",
>     >                 "fprintf", ...
>     >                 >
>     >                 >
>     >                 >         "printf" (and family) is a function
>     belonging
>     >                 to the C library.
>     >                 >         In our
>     >                 >         case that's newlib. It will format your
>     >                 message and after some other
>     >                 >         preprocessing will call the "write" function
>     >                 of the file that is
>     >                 >         opened
>     >                 >         as stdout (which is "/dev/console" in the
>     >                 default case).
>     >                 >
>     >                 >
>     >                 >         I hope that I helped you with that
>     >                 explanation. Please feel free
>     >                 >         to ask
>     >                 >         anything if it isn't clear.
>     >                 >
>     >                 >         Best regards
>     >                 >
>     >                 >         Christian
>     >                 >
>     >                 >         On 23/12/2019 19:50, Niteesh wrote:
>     >                 >         > And finally, how does printf work? It is a
>     >                 macro? In that
>     >                 >         case, how does
>     >                 >         > any write to
>     >                 >         > a console work?
>     >                 >         >
>     >                 >         > On Tue, Dec 24, 2019 at 12:18 AM Niteesh
>     >                 <gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>
>     <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>
>     >                 >         <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>>
>     >                 >         > <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>>> wrote:
>     >                 >         >
>     >                 >         >     Is the correct port minor number set
>     >                 during the
>     >                 >         initialization? What
>     >                 >         >     is the application want's to
>     >                 >         >     access some other port?
>     >                 >         >
>     >                 >         >     On Tue, Dec 24, 2019 at 12:16 AM
>     Niteesh
>     >                 >         <gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>
>     >                 >         >     <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>>> wrote:
>     >                 >         >
>     >                 >         >         I would like to clarify my doubts
>     >                 regarding the
>     >                 >         console driver.
>     >                 >         >         I went through the documentation
>     >                 >         >         for the console
>     >                 >         >       
>     >                 >       
>     >               
>        driver https://docs.rtems.org/branches/master/bsp-howto/console.html#introduction.
>     >                 >         >         But it is quite different from how
>     >                 some BSPs initialize.
>     >                 >         >         Correct me if I am wrong
>     >                 >         >         The console_tbl contains the
>     various
>     >                 entries of serial
>     >                 >         ports.
>     >                 >         >         The console_fns is a struct of
>     >                 function pointers,
>     >                 >         which point to
>     >                 >         >         the BSP uart functions.
>     >                 >         >         The
>     BSP_output_char_function_type is
>     >                 what will be
>     >                 >         called for
>     >                 >         >         printing a char on to the console.
>     >                 >         >         How does RTEMS initialize the
>     uart?
>     >                 It's seems not to
>     >                 >         be same
>     >                 >         >         for all BSPs.
>     >                 >         >         The doc says that the driver's
>     >                 initialization function
>     >                 >         is called
>     >                 >         >         once during the rtems
>     initialization
>     >                 process.
>     >                 >         >         The console init function install
>     >                 the serial driver using
>     >                 >         >         rtems_termios_device_install but
>     >                 there seems to be
>     >                 >         >         no such function in the raspberry
>     >                 pi? But there is a
>     >                 >         entry in
>     >                 >         >         console_fns for init function, but
>     >                 then how does it
>     >                 >         >         gets called?
>     >                 >         >         And for BSP's with multiple
>     >                 serial's, the output function
>     >                 >         >         chooses the right serial using
>     >                 console_port_minor,
>     >                 >         >         Is it during initialization?
>     >                 >         >         What is the need for get and set
>     >                 register functions? 
>     >                 >         >
>     >                 >         >         On Mon, Dec 23, 2019 at 1:04 AM
>     >                 Christian Mauderer
>     >                 >         >         <list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>>
>     >                 >         <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>>>> wrote:
>     >                 >         >
>     >                 >         >             On 22/12/2019 19:45, Joel
>     >                 Sherrill wrote:
>     >                 >         >             >
>     >                 >         >             >
>     >                 >         >             > On Sun, Dec 22, 2019,
>     12:29 PM
>     >                 Niteesh
>     >                 >         <gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>
>     >                 >         >             <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>>
>     >                 >         >             >
>     <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>
>     >                 >         <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>> <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com <mailto:gsnb.gn at gmail.com>>
>     >                 >         <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>
>     >                 <mailto:gsnb.gn at gmail.com
>     <mailto:gsnb.gn at gmail.com>>>>>> wrote:
>     >                 >         >             >
>     >                 >         >             >     On Sun, Dec 22, 2019 at
>     >                 8:44 PM Christian
>     >                 >         Mauderer
>     >                 >         >             >     <list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>
>     >                 >         <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>
>     >                 >         <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>>>
>     >                 >         >             <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>
>     >                 >         <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>> <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>
>     >                 >         <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>
>     >                 <mailto:list at c-mauderer.de
>     <mailto:list at c-mauderer.de>>>>>> wrote:
>     >                 >         >             >
>     >                 >         >             >         Hello Niteesh,
>     >                 >         >             >
>     >                 >         >             >         thanks for doing
>     that
>     >                 work.
>     >                 >         >             >
>     >                 >         >             >         On 22/12/2019 12:10,
>     >                 Niteesh wrote:
>     >                 >         >             >         > The rpi1 and rpi2
>     >                 use the PL011 UART,
>     >                 >         whereas,
>     >                 >         >             with RPI's
>     >                 >         >             >         equipped with
>     >                 >         >             >         > wireless/Bluetooth
>     >                 module, the PL011 is
>     >                 >         >             connected to the Bluetooth
>     >                 >         >             >         > module, and
>     the mini
>     >                 UART is used as
>     >                 >         the primary
>     >                 >         >             UART.
>     >                 >         >             >
>     >                 >         >             >         In my opinion it
>     would
>     >                 be great if you
>     >                 >         could use
>     >                 >         >             the FDT to
>     >                 >         >             >         distinguish
>     >                 >         >             >         between the boards.
>     >                 That should allow to add
>     >                 >         >             raspberry 3 (and
>     >                 >         >             >         maybe 4)
>     >                 >         >             >         support without
>     adding
>     >                 another BSP. More
>     >                 >         BSPs mean
>     >                 >         >             a bigger
>     >                 >         >             >         maintenance
>     >                 >         >             >         effort for the RTEMS
>     >                 community.
>     >                 >         >             >
>     >                 >         >             >     Learning more about
>     FDT is
>     >                 on my list for a long
>     >                 >         >             time.  I would love
>     >                 >         >             >     to work on that
>     >                 >         >             >     but I have almost no exp
>     >                 with FDT's.
>     >                 >         >             >     But another thing could
>     >                 also be done, in
>     >                 >         >             >   
>     >                  raspberrypi/start/bspstart.c we get the
>     >                 >         revision and
>     >                 >         >             >     model of the board using
>     >                 the mailbox. Every
>     >                 >         board has
>     >                 >         >             a unique id,
>     >                 >         >             >     which we could use to
>     >                 initialize
>     >                 >         >             >     the BSP. But using FDT
>     >                 seems to be a more
>     >                 >         elegant
>     >                 >         >             option, it is a
>     >                 >         >             >     lot of work I think, but
>     >                 we could take
>     >                 >         >             >     help from libbsd and
>     linux
>     >                 I suppose. What
>     >                 >         do you think?
>     >                 >         >             >
>     >                 >         >             >
>     >                 >         >             > I think there are almost
>     >                 always two steps to a
>     >                 >         project
>     >                 >         >             like this: get it
>     >                 >         >             > to work and make it nice. :)
>     >                 >         >             >
>     >                 >         >             > If you fix the startup
>     code to
>     >                 read the board
>     >                 >         revision and
>     >                 >         >             memory size,
>     >                 >         >             > you can get a working
>     BSP that
>     >                 dynamically
>     >                 >         adapts to the
>     >                 >         >             models and
>     >                 >         >             > memory variations with
>     minimal
>     >                 modifications. If
>     >                 >         you want
>     >                 >         >             to then
>     >                 >         >             > convert the BSP to FDT, it
>     >                 will be a LOT easier
>     >                 >         to debug
>     >                 >         >             with a working BSP.
>     >                 >         >             >
>     >                 >         >             > Plus you may be able to
>     >                 identify every variation
>     >                 >         point
>     >                 >         >             based on just the
>     >                 >         >             > model info. Then FDT is
>     just a
>     >                 matter of
>     >                 >         switching the
>     >                 >         >             source of
>     >                 >         >             > some/all of the info.
>     >                 >         >             >
>     >                 >         >             > That would be my work
>     plan anyway.
>     >                 >         >
>     >                 >         >             I agree with Joel that a
>     secure
>     >                 development basis
>     >                 >         (also
>     >                 >         >             known as "hack")
>     >                 >         >             as a first step is a good
>     idea.
>     >                 You maybe even
>     >                 >         just make the
>     >                 >         >             mini UART
>     >                 >         >             the default driver while
>     you are
>     >                 developing. Then
>     >                 >         you can be
>     >                 >         >             sure that
>     >                 >         >             you have the right driver.
>     >                 >         >
>     >                 >         >             As soon as that works you can
>     >                 either change to the
>     >                 >         revision
>     >                 >         >             method or
>     >                 >         >             (better) to the FDT one and
>     >                 after that the patches
>     >                 >         can be
>     >                 >         >             merged. Using
>     >                 >         >             the FDT isn't that
>     complicated.
>     >                 Basically you
>     >                 >         search for a
>     >                 >         >             node based on
>     >                 >         >             different parameters. For an
>     >                 example you can take
>     >                 >         a look at
>     >                 >         >             the imx BSP.
>     >                 >         >             In imx_uart_probe
>     >                 >         (bsps/arm/imx/console/console-config.c) a
>     >                 >         >             fdt node is
>     >                 >         >             searched and based on that a
>     >                 UART driver is used.
>     >                 >         But again:
>     >                 >         >             Follow
>     >                 >         >             Joels suggestion to start
>     simple
>     >                 and secure.
>     >                 >         >
>     >                 >         >             >
>     >                 >         >             >         >
>     >                 >         >             >         >
>     >                 >         >             >       
>     >                 >         >           
>     >                 >       
>     >               
>         https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>     >                 >         >             >         > But from the above
>     >                 doc (PAGE 10), the
>     >                 >         mini uart
>     >                 >         >             has 16550 like
>     >                 >         >             >         registers
>     >                 >         >             >         > and RTEMS already
>     >                 has the driver for it
>     >                 >         >             >         >
>     >                 bsps/shared/dev/serial/ns16550.c. But
>     >                 >         I am not
>     >                 >         >             sure how
>     >                 >         >             >         compatible they
>     >                 >         >             >         > are? Should a new
>     >                 driver be
>     >                 >         implemented from
>     >                 >         >             scratch or use
>     >                 >         >             >         ns16550 if
>     >                 >         >             >         > possible?
>     >                 >         >             >
>     >                 >         >             >         In general it's
>     better
>     >                 to re-use
>     >                 >         existing code.
>     >                 >         >             That has multiple
>     >                 >         >             >         advantages:
>     >                 >         >             >
>     >                 >         >             >         - It reduces the
>     >                 maintenance effort.
>     >                 >         Fewer code
>     >                 >         >             means fewer work.
>     >                 >         >             >         - If you have
>     multiple
>     >                 driver for the
>     >                 >         same or
>     >                 >         >             similar hardware
>     >                 >         >             >         it can
>     >                 >         >             >         happen that a bug is
>     >                 fixed in one but
>     >                 >         not the other.
>     >                 >         >             >         - It's simpler
>     to find
>     >                 a hardware to
>     >                 >         test changes.
>     >                 >         >             >         - The driver becomes
>     >                 more universal with
>     >                 >         every new
>     >                 >         >             supported
>     >                 >         >             >         hardware.
>     >                 >         >             >         That increases the
>     >                 chance that it fits
>     >                 >         the next
>     >                 >         >             new hardware.
>     >                 >         >             >
>     >                 >         >             >         I'm sure there are
>     >                 some more if you ask
>     >                 >         someone else.
>     >                 >         >             >
>     >                 >         >             >     I do understand the
>     >                 issues, I just spent
>     >                 >         some time
>     >                 >         >             reading the
>     >                 >         >             >     driver code.
>     >                 >         >             >     I think we could most
>     >                 probably use it. I
>     >                 >         will take a
>     >                 >         >             closer look and
>     >                 >         >             >     will update.
>     >                 >         >             >
>     >                 >         >
>     >                 >         >             Great.
>     >                 >         >
>     >                 >         >             >          
>     >                 >         >             >
>     >                 >         >             >         >
>     >                 >         >             >         > Also, the core
>     clock
>     >                 on which the
>     >                 >         PL011 is based
>     >                 >         >             on is changed
>     >                 >         >             >         in rpi3.
>     >                 >         >             >         > Rpi1 and 2 use
>     >                 250Mhz as the default
>     >                 >         clock but
>     >                 >         >             it was changed
>     >                 >         >             >         to 400Mhz
>     >                 >         >             >         > in Rpi3 and newer
>     >                 >         >             >
>     >                 >         >             >         Again: Would be
>     great
>     >                 if that could be
>     >                 >         adapted
>     >                 >         >             based on FDT or by
>     >                 >         >             >         reading the right
>     >                 registers.
>     >                 >         >             >
>     >                 >         >             >         >
>     >                 >         >             >         > Few differences
>     >                 between PL011 and Mini
>     >                 >         uart
>     >                 >         >             >         > The mini UART has
>     >                 smaller FIFOs.
>     >                 >         Combined with
>     >                 >         >             the lack of
>     >                 >         >             >         flow control,
>     >                 >         >             >         > this makes it more
>     >                 prone to losing
>     >                 >         characters at
>     >                 >         >             higher baud
>     >                 >         >             >         rates. It
>     >                 >         >             >         > is also generally
>     >                 less capable than
>     >                 >         the PL011,
>     >                 >         >             mainly due to
>     >                 >         >             >         its baud
>     >                 >         >             >         > rate link to
>     the VPU
>     >                 clock speed.
>     >                 >         >             >
>     >                 >         >             >         That shouldn't
>     really
>     >                 be a problem for
>     >                 >         the system
>     >                 >         >             console.
>     >                 >         >             >
>     >                 >         >             >         >
>     >                 >         >             >         > The particular
>     >                 deficiencies of the
>     >                 >         mini UART
>     >                 >         >             compared to the
>     >                 >         >             >         PL011 are :
>     >                 >         >             >         >
>     >                 >         >             >         > No break detection
>     >                 >         >             >         > No framing errors
>     >                 detection
>     >                 >         >             >         > No parity bit
>     >                 >         >             >         > No receive timeout
>     >                 interrupt
>     >                 >         >             >         > No DCD, DSR,
>     DTR or
>     >                 RI signals
>     >                 >         >             >         >
>     >                 >         >             >
>     >                 >         >             >   
>     >                  _______________________________________________
>     >                 >         >             >     devel mailing list
>     >                 >         >             >     devel at rtems.org
>     <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>
>     <mailto:devel at rtems.org <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>>
>     >                 >         <mailto:devel at rtems.org
>     <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>
>     <mailto:devel at rtems.org <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>>>
>     >                 >         >             <mailto:devel at rtems.org
>     <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>
>     <mailto:devel at rtems.org <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>>
>     >                 >         <mailto:devel at rtems.org
>     <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>
>     <mailto:devel at rtems.org <mailto:devel at rtems.org>
>     >                 <mailto:devel at rtems.org <mailto:devel at rtems.org>>>>>
>     >                 >         >             >   
>     >                  http://lists.rtems.org/mailman/listinfo/devel
>     >                 >         >             >
>     >                 >         >
>     >                 >
>     >
> 


More information about the devel mailing list