Raspberrypi3: Mini UART driver
Niteesh
gsnb.gn at gmail.com
Fri Dec 27 09:42:47 UTC 2019
On Fri, Dec 27, 2019 at 2:53 PM Christian Mauderer <list at c-mauderer.de>
wrote:
> 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.
>
Since I don't have the older pi's, the simulator will be my only option.
And did you
take a look at the exception?
>
> - Isn't it possible to just use the driver from
> bsps/arm/shared/serial/arm-pl011.c and remove the BSP specific completely?
>
Yes, I have done this. I usart.c file is now completely empty. All we have
to define was
the start address for the PL011 peripheral in usart.h.
>
> 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
> > > > > >
> > > > >
> > > >
> > >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20191227/e87ce4ef/attachment-0001.html>
More information about the devel
mailing list