On 27/12/2019 10:42, Niteesh wrote: > On Fri, Dec 27, 2019 at 2:53 PM Christian Mauderer <l...@c-mauderer.de > <mailto:l...@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?
Not the one in the simulator but the one in real hardware: https://lists.rtems.org/pipermail/devel/2019-December/056581.html I would like to wait for some feedback on this. I assume that the simulator doesn't simulate a revision code and therefore hit the same bug like my old pi 1. > > > - 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. Sounds great. > > > On 27/12/2019 06:29, Niteesh wrote: > > On Fri, Dec 27, 2019 at 1:58 AM Christian Mauderer > <l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@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 > <j...@rtems.org <mailto:j...@rtems.org> > > <mailto:j...@rtems.org <mailto:j...@rtems.org>> > > > <mailto:j...@rtems.org <mailto:j...@rtems.org> > <mailto:j...@rtems.org <mailto:j...@rtems.org>>>> wrote: > > > > > > > > > > > > On Tue, Dec 24, 2019, 12:19 PM Niteesh > <gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@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...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@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 > > > <l...@c-mauderer.de <mailto:l...@c-mauderer.de> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > <mailto:l...@c-mauderer.de <mailto:l...@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...@gmail.com > <mailto:gsnb...@gmail.com> <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com>> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@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 > > > > <l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@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...@gmail.com > <mailto:gsnb...@gmail.com> <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com>> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com>>>> > > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@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...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>>> > > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@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 > > > > > <l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>>> > > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>>>>> wrote: > > > > > > > > > > On 22/12/2019 19:45, > Joel > > > Sherrill wrote: > > > > > > > > > > > > > > > > > > On Sun, Dec 22, 2019, > > 12:29 PM > > > Niteesh > > > > <gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>>> > > > > > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>>>> > > > > > > > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com>>> > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>>> > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com>>> > > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>> > > > <mailto:gsnb...@gmail.com > <mailto:gsnb...@gmail.com> > > <mailto:gsnb...@gmail.com <mailto:gsnb...@gmail.com>>>>>>> wrote: > > > > > > > > > > > > On Sun, Dec > 22, 2019 at > > > 8:44 PM Christian > > > > Mauderer > > > > > > > <l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>>>> > > > > > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>>> > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>>> > > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@c-mauderer.de>> > > > <mailto:l...@c-mauderer.de > <mailto:l...@c-mauderer.de> > > <mailto:l...@c-mauderer.de <mailto:l...@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@rtems.org <mailto:devel@rtems.org> > > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>> > > <mailto:devel@rtems.org <mailto:devel@rtems.org> > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>>> > > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> > > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>> > > <mailto:devel@rtems.org <mailto:devel@rtems.org> > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>>>> > > > > > > <mailto:devel@rtems.org <mailto:devel@rtems.org> > > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>> > > <mailto:devel@rtems.org <mailto:devel@rtems.org> > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>>> > > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> > > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>> > > <mailto:devel@rtems.org <mailto:devel@rtems.org> > <mailto:devel@rtems.org <mailto:devel@rtems.org>> > > > <mailto:devel@rtems.org > <mailto:devel@rtems.org> <mailto:devel@rtems.org > <mailto:devel@rtems.org>>>>>> > > > > > > > > > http://lists.rtems.org/mailman/listinfo/devel > > > > > > > > > > > > > > > > > > > > > _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel