[U-Boot] [PATCH 3/4 v3] s5pc1xx: support serial driver
Minkyu Kang
promsoft at gmail.com
Sat Sep 26 10:18:02 CEST 2009
Dear Tom
2009/9/23 Tom <Tom.Rix at windriver.com>:
> Minkyu Kang wrote:
>> This patch includes the serial driver for s5pc1xx
>>
>> Signed-off-by: Minkyu Kang <mk7.kang at samsung.com>
>
> Add a better commit log.
> Explain why a special serial driver is needed instead
> of using the generic serial driver.
ok.
>
>> ---
>> Changes since v1:
>> - use serial multi API
>> - use writel/readl function
>> - remove duplicate code
>>
>> Changes since v2:
>> - use sizeof() instead of the constant
>>
>> common/serial.c | 18 +++
>> drivers/serial/Makefile | 1 +
>> drivers/serial/serial_s5pc1xx.c | 307 +++++++++++++++++++++++++++++++++++++++
>> include/serial.h | 7 +
>> 4 files changed, 333 insertions(+), 0 deletions(-)
>> create mode 100644 drivers/serial/serial_s5pc1xx.c
>>
>> diff --git a/common/serial.c b/common/serial.c
>> index 41a24c2..e5ce9fd 100644
>> --- a/common/serial.c
>> +++ b/common/serial.c
>> @@ -69,6 +69,18 @@ struct serial_device *__default_serial_console (void)
>> #else
>> #error "CONFIG_SERIAL? missing."
>> #endif
>> +#elif defined(CONFIG_S5PC1XX)
>> +#if defined(CONFIG_SERIAL0)
>> + return &s5pc1xx_serial0_device;
>> +#elif defined(CONFIG_SERIAL1)
>> + return &s5pc1xx_serial1_device;
>> +#elif defined(CONFIG_SERIAL2)
>> + return &s5pc1xx_serial2_device;
>> +#elif defined(CONFIG_SERIAL3)
>> + return &s5pc1xx_serial3_device;
>
> This can be condensed down to
>
> return S5PC1XX_DEFAULT_SERIAL_DEVICE
>
> S5PC1XX_DEFAULT_SERIAL_DEVICE to be defined the board file.
>
>
>
>> +#else
>> +#error "CONFIG_SERIAL? missing."
>> +#endif
>> #elif defined(CONFIG_OMAP3_ZOOM2)
>> return ZOOM2_DEFAULT_SERIAL_DEVICE;
>> #else
>> @@ -139,6 +151,12 @@ void serial_initialize (void)
>> serial_register(&s3c24xx_serial1_device);
>> serial_register(&s3c24xx_serial2_device);
>> #endif
>> +#if defined(CONFIG_S5PC1XX)
>> + serial_register(&s5pc1xx_serial0_device);
>> + serial_register(&s5pc1xx_serial1_device);
>> + serial_register(&s5pc1xx_serial2_device);
>> + serial_register(&s5pc1xx_serial3_device);
>> +#endif
>> serial_assign (default_serial_console ()->name);
>> }
>>
>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>> index 64882a2..3c77a7c 100644
>> --- a/drivers/serial/Makefile
>> +++ b/drivers/serial/Makefile
>> @@ -33,6 +33,7 @@ COBJS-$(CONFIG_NS9750_UART) += ns9750_serial.o
>> COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
>> COBJS-$(CONFIG_DRIVER_S3C4510_UART) += s3c4510b_uart.o
>> COBJS-$(CONFIG_S3C64XX) += s3c64xx.o
>> +COBJS-$(CONFIG_S5PC1XX) += serial_s5pc1xx.o
>> COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial.o
>> COBJS-$(CONFIG_CLPS7111_SERIAL) += serial_clps7111.o
>> COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
>> diff --git a/drivers/serial/serial_s5pc1xx.c b/drivers/serial/serial_s5pc1xx.c
>> new file mode 100644
>> index 0000000..48feb4e
>> --- /dev/null
>> +++ b/drivers/serial/serial_s5pc1xx.c
>> @@ -0,0 +1,307 @@
>> +/*
>> + * (C) Copyright 2009 SAMSUNG Electronics
>> + * Minkyu Kang <mk7.kang at samsung.com>
>> + * Heungjun Kim <riverful.kim at samsung.com>
>> + *
>> + * based on drivers/serial/s3c64xx.c
>> + *
>> + * 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 <asm/io.h>
>> +#include <asm/arch/uart.h>
>> +#include <asm/arch/clk.h>
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +#include <serial.h>
>> +
>> +/* Multi serial device functions */
>> +#define DECLARE_S5P_SERIAL_FUNCTIONS(port) \
>> + int s5p_serial##port##_init(void) { \
>> + return serial_init_dev(port); } \
>> + void s5p_serial##port##_setbrg(void) { \
>> + serial_setbrg_dev(port); } \
>> + int s5p_serial##port##_getc(void) { \
>> + return serial_getc_dev(port); } \
>> + int s5p_serial##port##_tstc(void) { \
>> + return serial_tstc_dev(port); } \
>> + void s5p_serial##port##_putc(const char c) { \
>> + serial_putc_dev(port, c); } \
>> + void s5p_serial##port##_puts(const char *s) { \
>> + serial_puts_dev(port, s); }
>
> Mixing whitespace blanks and tabs.
> Please convert to tabs.
ok, i'll fix it
>
>> +
>> +#define INIT_S5P_SERIAL_STRUCTURE(port, name, bus) { \
>> + name, \
>> + bus, \
>> + s5p_serial##port##_init, \
>> + s5p_serial##port##_setbrg, \
>> + s5p_serial##port##_getc, \
>> + s5p_serial##port##_tstc, \
>> + s5p_serial##port##_putc, \
>> + s5p_serial##port##_puts, }
>> +#else
>> +
>> +#ifdef CONFIG_SERIAL0
>> +#define UART_NR S5PC1XX_UART0
>> +#elif defined(CONFIG_SERIAL1)
>> +#define UART_NR S5PC1XX_UART1
>> +#elif defined(CONFIG_SERIAL2)
>> +#define UART_NR S5PC1XX_UART2
>> +#elif defined(CONFIG_SERIAL3)
>> +#define UART_NR S5PC1XX_UART3
>> +#else
>> +#error "Bad: you didn't configure serial ..."
>> +#endif
>> +
>> +#endif /* CONFIG_SERIAL_MULTI */
>
> Why do you need support two types of serial?
Actually no need.
but other drivers (s3c24x0 and pxa) support two types both.
so I did.
If no need to support two types of serial, I'll drop non multi serial codes.
>
>> +
>> +static inline struct s5pc1xx_uart *s5pc1xx_get_base_uart(int dev_index)
>> +{
>> + u32 offset = dev_index * sizeof(struct s5pc1xx_uart);
>> +
>> + if (cpu_is_s5pc100())
>> + return (struct s5pc1xx_uart *)(S5PC100_UART_BASE + offset);
>> + else
>> + return (struct s5pc1xx_uart *)(S5PC110_UART_BASE + offset);
>> +}
>> +
>> +/*
>> + * The coefficient, used to calculate the baudrate on S5PC1XX UARTs is
>> + * calculated as
>> + * C = UBRDIV * 16 + number_of_set_bits_in_UDIVSLOT
>> + * however, section 31.6.11 of the datasheet doesn't recomment using 1 for 1,
>> + * 3 for 2, ... (2^n - 1) for n, instead, they suggest using these constants:
>> + */
> Good comment
>
>> +static const int udivslot[] = {
>> + 0,
>> + 0x0080,
>> + 0x0808,
>> + 0x0888,
>> + 0x2222,
>> + 0x4924,
>> + 0x4a52,
>> + 0x54aa,
>> + 0x5555,
>> + 0xd555,
>> + 0xd5d5,
>> + 0xddd5,
>> + 0xdddd,
>> + 0xdfdd,
>> + 0xdfdf,
>> + 0xffdf,
>> +};
>> +
>> +void _serial_setbrg(const int dev_index)
>> +{
>> + DECLARE_GLOBAL_DATA_PTR;
>> + struct s5pc1xx_uart *const uart = s5pc1xx_get_base_uart(dev_index);
>> + u32 pclk = get_pclk();
>> + u32 baudrate = gd->baudrate;
>> + u32 val;
>> +
>> + val = pclk / baudrate;
>> +
>> + writel(val / 16 - 1, &uart->ubrdiv);
>> + writel(udivslot[val % 16], &uart->udivslot);
>> +}
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +static inline void serial_setbrg_dev(unsigned int dev_index)
>> +{
>> + _serial_setbrg(dev_index);
>> +}
>> +#else
>> +void serial_setbrg(void)
>> +{
>> + _serial_setbrg(UART_NR);
>> +}
>> +#endif
>> +
>> +/*
>> + * Initialise the serial port with the given baudrate. The settings
>> + * are always 8 data bits, no parity, 1 stop bit, no start bits.
>> + */
>> +int serial_init_dev(const int dev_index)
>> +{
>> + struct s5pc1xx_uart *const uart = s5pc1xx_get_base_uart(dev_index);
>> +
>> + /* reset and enable FIFOs, set triggers to the maximum */
>> + writel(0, &uart->ufcon);
>> + writel(0, &uart->umcon);
>> + /* 8N1 */
>> + writel(0x3, &uart->ulcon);
>> + /* No interrupts, no DMA, pure polling */
>> + writel(0x245, &uart->ucon);
>> +
>> + _serial_setbrg(dev_index);
>> +
>> + return 0;
>> +}
>> +
>> +#if !defined(CONFIG_SERIAL_MULTI)
>> +int serial_init(void)
>> +{
>> + return serial_init_dev(UART_NR);
>> +}
>> +#endif
>> +
>> +/*
>> + * Read a single byte from the serial port. Returns 1 on success, 0
>> + * otherwise. When the function is succesfull, the character read is
>> + * written into its argument c.
>> + */
>> +int _serial_getc(const int dev_index)
>> +{
>> + struct s5pc1xx_uart *const uart = s5pc1xx_get_base_uart(dev_index);
>> +
>> + /* wait for character to arrive */
>> + while (!(readl(&uart->utrstat) & 0x1))
>> + ;
>
> Will this block ?
hm.. low probability..
I''ll add error checking logic.
>
>> +
>> + return (int)(readl(&uart->urxh) & 0xff);
>> +}
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +static inline int serial_getc_dev(unsigned int dev_index)
>> +{
>> + return _serial_getc(dev_index);
>> +}
>> +#else
>> +int serial_getc(void)
>> +{
>> + return _serial_getc(UART_NR);
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_MODEM_SUPPORT
>> +static int be_quiet;
>> +void disable_putc(void)
>> +{
>> + be_quiet = 1;
>> +}
>> +
>> +void enable_putc(void)
>> +{
>> + be_quiet = 0;
>> +}
>> +#endif
>
> Where is CONFIG_MODEM_SUPPORT used ?
> There should be #define stubs for the non-modem case if these are to
> exported.
> Also externals should be in a H file.
I'll check it
>
>> +
>> +/*
>> + * Output a single byte to the serial port.
>> + */
>> +void _serial_putc(const char c, const int dev_index)
>> +{
>> + struct s5pc1xx_uart *const uart = s5pc1xx_get_base_uart(dev_index);
>> +
>> +#ifdef CONFIG_MODEM_SUPPORT
>> + if (be_quiet)
>> + return;
>> +#endif
>
> The CONFIG_MODEM_SUPPORT is not needed.
> The non-modem case does not change the state of the be_quiet.
>
>> +
>> + /* wait for room in the tx FIFO */
>> + while (!(readl(&uart->utrstat) & 0x2))
>> + ;
>> +
>> + writel(c, &uart->utxh);
>> +
>> + /* If \n, also do \r */
>> + if (c == '\n')
>> + serial_putc('\r');
>> +}
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +static inline void serial_putc_dev(unsigned int dev_index, const char c)
>> +{
>> + _serial_putc(c, dev_index);
>> +}
>> +#else
>> +void serial_putc(const char c)
>> +{
>> + _serial_putc(c, UART_NR);
>> +}
>> +#endif
>> +
>> +/*
>> + * Test whether a character is in the RX buffer
>> + */
>> +int _serial_tstc(const int dev_index)
>> +{
>> + struct s5pc1xx_uart *const uart = s5pc1xx_get_base_uart(dev_index);
>> +
>> + return (int)(readl(&uart->utrstat) & 0x1);
>> +}
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +static inline int serial_tstc_dev(unsigned int dev_index)
>> +{
>> + return _serial_tstc(dev_index);
>> +}
>> +#else
>> +int serial_tstc(void)
>> +{
>> + return _serial_tstc(UART_NR);
>> +}
>> +#endif
>> +
>> +void _serial_puts(const char *s, const int dev_index)
>> +{
>> + while (*s)
>> + _serial_putc(*s++, dev_index);
>> +}
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +static inline void serial_puts_dev(int dev_index, const char *s)
>> +{
>> + _serial_puts(s, dev_index);
>> +}
>> +#else
>> +void serial_puts(const char *s)
>> +{
>> + _serial_puts(s, UART_NR);
>> +}
>> +#endif
>> +
>> +#ifdef CONFIG_HWFLOW
>> +static int hwflow; /* turned off by default */
>
> tab before comment, here and other places.
>
>> +int hwflow_onoff(int on)
>> +{
>> + switch (on) {
>> + case 1:
>> + hwflow = 1; /* turn on */
>> + break;
>> + case -1:
>> + hwflow = 0; /* turn off */
>> + break;
>> + }
>> + return hwflow;
>> +}
>> +#endif
>
> Similar to the CONFIG_MODEM_SUPPORT.
> Add #define stubs for non-hwflow
> Add decl to H file
>
> Tom
>
>
>
>> +
>> +#if defined(CONFIG_SERIAL_MULTI)
>> +DECLARE_S5P_SERIAL_FUNCTIONS(0);
>> +struct serial_device s5pc1xx_serial0_device =
>> + INIT_S5P_SERIAL_STRUCTURE(0, "s5pser0", "S5PUART0");
>> +DECLARE_S5P_SERIAL_FUNCTIONS(1);
>> +struct serial_device s5pc1xx_serial1_device =
>> + INIT_S5P_SERIAL_STRUCTURE(1, "s5pser1", "S5PUART1");
>> +DECLARE_S5P_SERIAL_FUNCTIONS(2);
>> +struct serial_device s5pc1xx_serial2_device =
>> + INIT_S5P_SERIAL_STRUCTURE(2, "s5pser2", "S5PUART2");
>> +DECLARE_S5P_SERIAL_FUNCTIONS(3);
>> +struct serial_device s5pc1xx_serial3_device =
>> + INIT_S5P_SERIAL_STRUCTURE(3, "s5pser3", "S5PUART3");
>> +#endif
>> diff --git a/include/serial.h b/include/serial.h
>> index 821b583..bbda3f0 100644
>> --- a/include/serial.h
>> +++ b/include/serial.h
>> @@ -43,6 +43,13 @@ extern struct serial_device s3c24xx_serial1_device;
>> extern struct serial_device s3c24xx_serial2_device;
>> #endif
>>
>> +#if defined(CONFIG_S5PC1XX)
>> +extern struct serial_device s5pc1xx_serial0_device;
>> +extern struct serial_device s5pc1xx_serial1_device;
>> +extern struct serial_device s5pc1xx_serial2_device;
>> +extern struct serial_device s5pc1xx_serial3_device;
>> +#endif
>> +
>> #if defined(CONFIG_OMAP3_ZOOM2)
>> extern struct serial_device zoom2_serial_device0;
>> extern struct serial_device zoom2_serial_device1;
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>
Thanks
Minkyu Kang
--
from. prom.
www.promsoft.net
More information about the U-Boot
mailing list