| Message ID | 1365749635-7025-4-git-send-email-b18965@freescale.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
On Fri, Apr 12, 2013 at 3:53 AM, Alison Wang <b18965@freescale.com> wrote: > This patch adds uart driver support for vybrid platform. > > Signed-off-by: TsiChung Liew <tsicliew@gmail.com> > Signed-off-by: Jason Jin <Jason.jin@freescale.com> > Signed-off-by: Alison Wang <b18965@freescale.com> > --- > drivers/serial/Makefile | 1 + > drivers/serial/serial.c | 2 + > drivers/serial/serial_vybrid.c | 129 +++++++++++++++++++++++++++++++++++++++++ Do we really need to add a new serial driver? Can't we re-use the imx serial driver? Regards, Fabio Estevam
On 12/04/2013 08:53, Alison Wang wrote: > This patch adds uart driver support for vybrid platform. > > Signed-off-by: TsiChung Liew <tsicliew@gmail.com> > Signed-off-by: Jason Jin <Jason.jin@freescale.com> > Signed-off-by: Alison Wang <b18965@freescale.com> > --- Hi Alison > diff --git a/drivers/serial/serial_vybrid.c b/drivers/serial/serial_vybrid.c > new file mode 100644 > index 0000000..4dd9b52 > --- /dev/null > +++ b/drivers/serial/serial_vybrid.c > @@ -0,0 +1,129 @@ > +/* > + * Copyright 2012-2013 Freescale Semiconductor, Inc. > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA > + * > + */ > + > +#include <common.h> > +#include <watchdog.h> > +#include <asm/io.h> > +#include <serial.h> > +#include <linux/compiler.h> > +#include <asm/arch/vybrid-regs.h> > +#include <asm/arch/serial-vybrid.h> > +#include <asm/arch/clock.h> > + > +#ifndef CONFIG_VYBRID_UART_BASE > +#error "define CONFIG_VYBRID_UART_BASE to use the VYBRID UART driver" > +#endif > + > +#define UART_CONSOLE \ > + (CONFIG_VYBRID_UART_BASE + (CONFIG_SYS_UART_PORT * 0x1000)) > + > +#ifdef CONFIG_SERIAL_MULTI > +#warning "Vybrid driver does not support MULTI serials." > +#endif > + > +DECLARE_GLOBAL_DATA_PTR; > + > +static void vybrid_serial_setbrg(void) > +{ > + u32 clk = vybrid_get_uartclk(); > + u16 sbr; > + > + if (!gd->baudrate) > + gd->baudrate = CONFIG_BAUDRATE; > + > + sbr = (u16)(clk / (16 * gd->baudrate)); > + /* place adjustment later - n/32 BRFA */ > + > + out_8((UART_CONSOLE + UBDH), (sbr >> 8)); > + out_8((UART_CONSOLE + UBDL), (sbr & 0xFF)); > +} > + > +static int vybrid_serial_getc(void) > +{ > + while (!(in_8(UART_CONSOLE + US1) & US1_RDRF)) > + WATCHDOG_RESET(); > + Generally : do not use BASE + offset as here. Use C structures, instead. We have already ARM accessors (in8 is defined for powerpc). Use functions in io.h This driver is very simple, and it is similar to already implemented drivers. Sure that we cannot reuse other code ? Best regards, Stefano Babic
> On 12/04/2013 08:53, Alison Wang wrote: > > This patch adds uart driver support for vybrid platform. > > > > Signed-off-by: TsiChung Liew <tsicliew@gmail.com> > > Signed-off-by: Jason Jin <Jason.jin@freescale.com> > > Signed-off-by: Alison Wang <b18965@freescale.com> > > --- > > Hi Alison > > > diff --git a/drivers/serial/serial_vybrid.c > > b/drivers/serial/serial_vybrid.c new file mode 100644 index > > 0000000..4dd9b52 > > --- /dev/null > > +++ b/drivers/serial/serial_vybrid.c > > @@ -0,0 +1,129 @@ > > +/* > > + * Copyright 2012-2013 Freescale Semiconductor, Inc. > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License as published > > +by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + * > > + * This program is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > > + * GNU General Public License for more details. > > + * > > + * You should have received a copy of the GNU General Public License > > + * along with this program; if not, write to the Free Software > > + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA > > +02111-1307 USA > > + * > > + */ > > + > > +#include <common.h> > > +#include <watchdog.h> > > +#include <asm/io.h> > > +#include <serial.h> > > +#include <linux/compiler.h> > > +#include <asm/arch/vybrid-regs.h> > > +#include <asm/arch/serial-vybrid.h> > > +#include <asm/arch/clock.h> > > + > > +#ifndef CONFIG_VYBRID_UART_BASE > > +#error "define CONFIG_VYBRID_UART_BASE to use the VYBRID UART driver" > > +#endif > > + > > +#define UART_CONSOLE \ > > + (CONFIG_VYBRID_UART_BASE + (CONFIG_SYS_UART_PORT * 0x1000)) > > + > > +#ifdef CONFIG_SERIAL_MULTI > > +#warning "Vybrid driver does not support MULTI serials." > > +#endif > > + > > +DECLARE_GLOBAL_DATA_PTR; > > + > > +static void vybrid_serial_setbrg(void) { > > + u32 clk = vybrid_get_uartclk(); > > + u16 sbr; > > + > > + if (!gd->baudrate) > > + gd->baudrate = CONFIG_BAUDRATE; > > + > > + sbr = (u16)(clk / (16 * gd->baudrate)); > > + /* place adjustment later - n/32 BRFA */ > > + > > + out_8((UART_CONSOLE + UBDH), (sbr >> 8)); > > + out_8((UART_CONSOLE + UBDL), (sbr & 0xFF)); } > > + > > +static int vybrid_serial_getc(void) > > +{ > > + while (!(in_8(UART_CONSOLE + US1) & US1_RDRF)) > > + WATCHDOG_RESET(); > > + > > Generally : do not use BASE + offset as here. Use C structures, instead. > We have already ARM accessors (in8 is defined for powerpc). Use functions > in io.h [Alison Wang] Agree. I will modify to use C structures and use functions in io.h in the next version patches. Thanks a lot. > > This driver is very simple, and it is similar to already implemented > drivers. Sure that we cannot reuse other code ? [Alison Wang] As they are different IPs, I don't think we can reuse the UART driver for I.MX family. > Thanks! Best Regards, Alison Wang
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile index de3f471..776e018 100644 --- a/drivers/serial/Makefile +++ b/drivers/serial/Makefile @@ -53,6 +53,7 @@ COBJS-$(CONFIG_XILINX_UARTLITE) += serial_xuartlite.o COBJS-$(CONFIG_SANDBOX_SERIAL) += sandbox.o COBJS-$(CONFIG_SCIF_CONSOLE) += serial_sh.o COBJS-$(CONFIG_ZYNQ_SERIAL) += serial_zynq.o +COBJS-$(CONFIG_VYBRID_UART) += serial_vybrid.o ifndef CONFIG_SPL_BUILD COBJS-$(CONFIG_USB_TTY) += usbtty.o diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c index 7922bf0..903a520 100644 --- a/drivers/serial/serial.c +++ b/drivers/serial/serial.c @@ -178,6 +178,7 @@ serial_initfunc(pl01x_serial_initialize); serial_initfunc(s3c44b0_serial_initialize); serial_initfunc(sa1100_serial_initialize); serial_initfunc(sh_serial_initialize); +serial_initfunc(vybrid_serial_initialize); /** * serial_register() - Register serial driver with serial driver core @@ -272,6 +273,7 @@ void serial_initialize(void) s3c44b0_serial_initialize(); sa1100_serial_initialize(); sh_serial_initialize(); + vybrid_serial_initialize(); serial_assign(default_serial_console()->name); } diff --git a/drivers/serial/serial_vybrid.c b/drivers/serial/serial_vybrid.c new file mode 100644 index 0000000..4dd9b52 --- /dev/null +++ b/drivers/serial/serial_vybrid.c @@ -0,0 +1,129 @@ +/* + * Copyright 2012-2013 Freescale Semiconductor, Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + * + */ + +#include <common.h> +#include <watchdog.h> +#include <asm/io.h> +#include <serial.h> +#include <linux/compiler.h> +#include <asm/arch/vybrid-regs.h> +#include <asm/arch/serial-vybrid.h> +#include <asm/arch/clock.h> + +#ifndef CONFIG_VYBRID_UART_BASE +#error "define CONFIG_VYBRID_UART_BASE to use the VYBRID UART driver" +#endif + +#define UART_CONSOLE \ + (CONFIG_VYBRID_UART_BASE + (CONFIG_SYS_UART_PORT * 0x1000)) + +#ifdef CONFIG_SERIAL_MULTI +#warning "Vybrid driver does not support MULTI serials." +#endif + +DECLARE_GLOBAL_DATA_PTR; + +static void vybrid_serial_setbrg(void) +{ + u32 clk = vybrid_get_uartclk(); + u16 sbr; + + if (!gd->baudrate) + gd->baudrate = CONFIG_BAUDRATE; + + sbr = (u16)(clk / (16 * gd->baudrate)); + /* place adjustment later - n/32 BRFA */ + + out_8((UART_CONSOLE + UBDH), (sbr >> 8)); + out_8((UART_CONSOLE + UBDL), (sbr & 0xFF)); +} + +static int vybrid_serial_getc(void) +{ + while (!(in_8(UART_CONSOLE + US1) & US1_RDRF)) + WATCHDOG_RESET(); + + setbits_8((UART_CONSOLE + US1), US1_RDRF); + + return in_8(UART_CONSOLE + UD); +} + +static void vybrid_serial_putc(const char c) +{ + if (c == '\n') + serial_putc('\r'); + + while (!(in_8(UART_CONSOLE + US1) & US1_TDRE)) + WATCHDOG_RESET(); + + out_8((UART_CONSOLE + UD), c); +} + +/* + * Test whether a character is in the RX buffer + */ +static int vybrid_serial_tstc(void) +{ + if (in_8(UART_CONSOLE + URCFIFO) == 0) + return 0; + + return 1; +} + +/* + * Initialise the serial port with the given baudrate. The settings + * are always 8 data bits, no parity, 1 stop bit, no start bits. + */ +static int vybrid_serial_init(void) +{ + clrbits_8((UART_CONSOLE + UC2), UC2_RE); + clrbits_8((UART_CONSOLE + UC2), UC2_TE); + + out_8((UART_CONSOLE + UMODEM), 0); + out_8((UART_CONSOLE + UC1), 0); + + /* provide data bits, parity, stop bit, etc */ + + serial_setbrg(); + + out_8((UART_CONSOLE + UC2), (UC2_RE | UC2_TE)); + + return 0; +} + +static struct serial_device vybrid_serial_drv = { + .name = "vybrid_serial", + .start = vybrid_serial_init, + .stop = NULL, + .setbrg = vybrid_serial_setbrg, + .putc = vybrid_serial_putc, + .puts = default_serial_puts, + .getc = vybrid_serial_getc, + .tstc = vybrid_serial_tstc, +}; + +void vybrid_serial_initialize(void) +{ + serial_register(&vybrid_serial_drv); +} + +__weak struct serial_device *default_serial_console(void) +{ + return &vybrid_serial_drv; +}