On 25/12/2019 15:32, Niteesh wrote: > What about the framebuffer it still uses minor and major's?
I'm not entirely sure but I don't think that it is a problem to mix legacy and new style drivers. > > On Wed, Dec 25, 2019 at 3:20 PM Niteesh <gsnb...@gmail.com > <mailto:gsnb...@gmail.com>> 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. > > 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; > + > 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); > } > > -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; > + } > } > > -static int usart_last_close(int major, int minor, void *arg) > +static int usart_last_close( > + 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); > + > + 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>> wrote: > > > > On Tue, Dec 24, 2019, 12:19 PM Niteesh <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>> 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>> 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>>> 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>>> 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>>>> 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>>>> 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>>>> 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>>>>> 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>>>>> 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>>>> > > > > > http://lists.rtems.org/mailman/listinfo/devel > > > > > > > > > > _______________________________________________ devel mailing list devel@rtems.org http://lists.rtems.org/mailman/listinfo/devel