[U-Boot] [PATCH 3/7] STiH410: Add STi serial driver

Patrice CHOTARD patrice.chotard at st.com
Tue Feb 14 13:24:59 CET 2017


Hi Simon

On 02/10/2017 05:22 PM, Simon Glass wrote:
> Hi Patrice,
>
> On 2 February 2017 at 10:00,  <patrice.chotard at st.com> wrote:
>> From: Patrice Chotard <patrice.chotard at st.com>
>>
>> This patch adds support to ASC (asynchronous serial controller)
>> driver, which is basically a standard serial driver. This IP
>> is common across other STMicroelectronics SoCs
>>
>> Signed-off-by: Patrice Chotard <patrice.chotard at st.com>
>> ---
>>  arch/arm/Kconfig                          |   2 +
>>  arch/arm/include/asm/arch-stih410/sti.h   |   6 +
>>  board/st/stih410-b2260/board.c            |  12 ++
>>  drivers/serial/Kconfig                    |   7 +
>>  drivers/serial/Makefile                   |   1 +
>>  drivers/serial/serial_sti_asc.c           | 219 ++++++++++++++++++++++++++++++
>>  include/configs/stih410-b2260.h           |   1 +
>>  include/dm/platform_data/serial_sti_asc.h |  17 +++
>>  8 files changed, 265 insertions(+)
>>  create mode 100644 drivers/serial/serial_sti_asc.c
>>  create mode 100644 include/dm/platform_data/serial_sti_asc.h
>>
>> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> index 4aa5eb9..b91a5b7 100644
>> --- a/arch/arm/Kconfig
>> +++ b/arch/arm/Kconfig
>> @@ -985,6 +985,8 @@ config STM32
>>  config ARCH_STI
>>         bool "Support STMicrolectronics SoCs"
>>         select CPU_V7
>> +       select DM
>> +       select DM_SERIAL
>>         help
>>           Support for STMicroelectronics STiH407/10 SoC family.
>>           This SoC is used on Linaro 96Board STiH410-B2260
>> diff --git a/arch/arm/include/asm/arch-stih410/sti.h b/arch/arm/include/asm/arch-stih410/sti.h
>> index d35c4f0..f167560 100644
>> --- a/arch/arm/include/asm/arch-stih410/sti.h
>> +++ b/arch/arm/include/asm/arch-stih410/sti.h
>> @@ -11,4 +11,10 @@
>>  #define STI_A9_CONFIG_BASE                     0x08760000
>>  #define STI_A9_GLOBAL_TIMER_BASE               (STI_A9_CONFIG_BASE + 0x0200)
>>
>> +/* STiH410 control registers */
>> +#define STIH410_COMMS_BASE                     0x09800000
>> +
>> +/* ASC UART located in the main "COMMs" block */
>> +#define STIH410_ASC1_BASE (STIH410_COMMS_BASE + 0x00031000)
>
> Can you get the address of the serial port from the device tree
> instead, e.g. with dev_get_addr_ptr()?

Will be fixed

>
>> +
>>  #endif /* _STI_H_ */
>> diff --git a/board/st/stih410-b2260/board.c b/board/st/stih410-b2260/board.c
>> index 0c06bca..b2cfa7f 100644
>> --- a/board/st/stih410-b2260/board.c
>> +++ b/board/st/stih410-b2260/board.c
>> @@ -7,6 +7,9 @@
>>   */
>>
>>  #include <common.h>
>> +#include <asm/arch/sti.h>
>> +#include <dm/platdata.h>
>> +#include <dm/platform_data/serial_sti_asc.h>
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> @@ -26,3 +29,12 @@ int board_init(void)
>>  {
>>         return 0;
>>  }
>> +
>> +static const struct sti_asc_serial_platdata serial_platdata = {
>> +       .base = (struct sti_asc_uart *)STIH410_ASC1_BASE,
>> +};
>> +
>> +U_BOOT_DEVICE(sti_asc) = {
>> +       .name = "serial_sti_asc",
>> +       .platdata = &serial_platdata,
>> +};
>
> Can you use DT instead of platform data? You have the code for it, so
> why is this needed?

Yes, i simply forgot to fully convert this driver to DT, i will fix this.

>
>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>> index b11f3ff..7557632 100644
>> --- a/drivers/serial/Kconfig
>> +++ b/drivers/serial/Kconfig
>> @@ -413,4 +413,11 @@ config PXA_SERIAL
>>           If you have a machine based on a Marvell XScale PXA2xx CPU you
>>           can enable its onboard serial ports by enabling this option.
>>
>> +config STI_ASC_SERIAL
>> +       bool "STMicroelectronics on-chip UART"
>> +       depends on DM_SERIAL && ARCH_STI
>> +       help
>> +         Select this to enable Asynchronous Serial Controller available
>> +         on STiH410 SoC.
>
> Anything more to say? Maximum baud rate, or what hardware features the
> driver supports? E.g. it only seems to support a subset of baud rates.

Will be fixed

>
>> +
>>  endmenu
>> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
>> index 8430668..84a22ce 100644
>> --- a/drivers/serial/Makefile
>> +++ b/drivers/serial/Makefile
>> @@ -41,6 +41,7 @@ obj-$(CONFIG_FSL_LINFLEXUART) += serial_linflexuart.o
>>  obj-$(CONFIG_ARC_SERIAL) += serial_arc.o
>>  obj-$(CONFIG_UNIPHIER_SERIAL) += serial_uniphier.o
>>  obj-$(CONFIG_STM32_SERIAL) += serial_stm32.o
>> +obj-$(CONFIG_STI_ASC_SERIAL) += serial_sti_asc.o
>>  obj-$(CONFIG_PIC32_SERIAL) += serial_pic32.o
>>  obj-$(CONFIG_STM32X7_SERIAL) += serial_stm32x7.o
>>  obj-$(CONFIG_BCM283X_MU_SERIAL) += serial_bcm283x_mu.o
>> diff --git a/drivers/serial/serial_sti_asc.c b/drivers/serial/serial_sti_asc.c
>> new file mode 100644
>> index 0000000..5279836
>> --- /dev/null
>> +++ b/drivers/serial/serial_sti_asc.c
>> @@ -0,0 +1,219 @@
>> +/*
>> + * Support for Serial I/O using STMicroelectronics' on-chip ASC.
>> + *
>> + *  Copyright (c) 2017
>> + *  Patrice Chotard <patrice.chotard at st.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0
>> + */
>> +
>> +#include <common.h>
>> +#include <dm.h>
>> +#include <serial.h>
>> +#include <asm/arch/sti.h>
>> +#include <asm/io.h>
>
> move that up one line

ok

>
>> +#include <dm/platform_data/serial_sti_asc.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define BAUDMODE       0x00001000
>> +#define RXENABLE       0x00000100
>> +#define RUN            0x00000080
>> +#define MODE           0x00000001
>> +#define MODE_8BIT      0x0001
>> +#define STOP_1BIT      0x0008
>> +#define PARITYODD      0x0020
>> +
>> +#define STA_TF         0x0200
>> +#define STA_RBF                0x0001
>
> You could use BIT() if you like

ok

>
>> +
>> +struct sti_asc_uart {
>> +       u32 baudrate;
>> +       u32 txbuf;
>> +       u32 rxbuf;
>> +       u32 control;
>> +       u32 inten;
>> +       u32 status;
>> +       u32 guardtime;
>> +       u32 timeout;
>> +       u32 txreset;
>> +       u32 rxreset;
>> +};
>> +
>> +/*---- Values for the BAUDRATE Register -----------------------*/
>
> /* Values for the BAUDRATE Register */

ok

>
>> +#define PCLK                   (200ul * 1000000ul)
>> +#define BAUDRATE_VAL_M0(bps)   (PCLK / (16 * (bps)))
>> +#define BAUDRATE_VAL_M1(bps)   ((bps * (1 << 14)) + (1<<13)) / (PCLK/(1 << 6))
>> +
>> +/*
>> + * MODE 0
>> + *                       ICCLK
>> + * ASCBaudRate =   ----------------
>> + *                   baudrate * 16
>> + *
>> + * MODE 1
>> + *                   baudrate * 16 * 2^16
>> + * ASCBaudRate =   ------------------------
>> + *                          ICCLK
>> + *
>> + * NOTE:
>> + * Mode 1 should be used for baudrates of 19200, and above, as it
>> + * has a lower deviation error than Mode 0 for higher frequencies.
>> + * Mode 0 should be used for all baudrates below 19200.
>> + */
>> +
>> +static int sti_asc_pending(struct udevice *dev, bool input)
>> +{
>> +       struct sti_asc_serial_platdata *plat = dev->platdata;
>> +       struct sti_asc_uart *const uart = plat->base;
>> +       unsigned long status;
>> +
>> +       status = readl(&uart->status);
>> +       if (input)
>> +               return status & STA_RBF;
>> +       else
>> +               return status & STA_TF;
>> +}
>> +
>> +/* called to adjust baud-rate */
>> +static int sti_asc_serial_setbrg(struct udevice *dev, int baudrate)
>> +{
>> +       struct sti_asc_serial_platdata *plat = dev->platdata;
>> +       struct sti_asc_uart *const uart = plat->base;
>> +
>
> drop blank line

ok

>
>> +       unsigned long val;
>> +       int t, mode = 1;
>> +
>> +       switch (baudrate) {
>> +       case 9600:
>> +               t = BAUDRATE_VAL_M0(9600);
>> +               mode = 0;
>> +               break;
>> +       case 19200:
>> +               t = BAUDRATE_VAL_M1(19200);
>> +               break;
>> +       case 38400:
>> +               t = BAUDRATE_VAL_M1(38400);
>> +               break;
>> +       case 57600:
>> +               t = BAUDRATE_VAL_M1(57600);
>> +               break;
>> +       default:
>> +               printf("ASC: unsupported baud rate: %d, using 115200 instead.\n",
>> +                      baudrate);
>
> debug() ?

ok

>
>> +       case 115200:
>> +               t = BAUDRATE_VAL_M1(115200);
>> +               break;
>> +       }
>> +
>> +       /* wait for end of current transmission */
>> +       while (sti_asc_pending(dev, false))
>> +               ;
>
> Hmm this should not be here. We should be able to call
> serial_pending() to check that the device is idle, then avoid the loop
> here.
>
> Maybe there is something needed in serial_uclass.c?

you are right, it's useless

>
>> +
>> +       /* disable the baudrate generator */
>> +       val = readl(&uart->control);
>> +       writel((val & ~RUN), &uart->control);
>
> Drop extra brackets

ok

>
>> +
>> +       /* set baud generator reload value */
>> +       writel(t, &uart->baudrate);
>> +       /* reset the RX & TX buffers */
>> +       writel(1, &uart->txreset);
>> +       writel(1, &uart->rxreset);
>> +
>> +       /* set baud generator mode */
>> +       if (mode)
>> +               val |= BAUDMODE;
>> +
>> +       /* finally, write value and enable ASC */
>> +       writel(val, &uart->control);
>> +
>> +       return 0;
>> +}
>> +
>> +/* initialize the ASC */
>> +static int sti_asc_serial_probe(struct udevice *dev)
>> +{
>> +       struct sti_asc_serial_platdata *plat = dev->platdata;
>> +       struct sti_asc_uart *const uart = plat->base;
>> +       unsigned long val;
>> +
>> +       sti_asc_serial_setbrg(dev, gd->baudrate);
>> +
>> +       /*
>> +        * build up the value to be written to CONTROL
>> +        * set character length, bit stop number, odd parity
>> +        */
>> +       val = RXENABLE | RUN | MODE_8BIT | STOP_1BIT | PARITYODD;
>> +       writel(val, &uart->control);
>> +
>> +       return 0;
>> +}
>> +
>> +/* blocking function, that returns next char */
>> +static int sti_asc_serial_getc(struct udevice *dev)
>> +{
>> +       struct sti_asc_serial_platdata *plat = dev->platdata;
>> +       struct sti_asc_uart *const uart = plat->base;
>> +
>> +       /* polling wait: for a char to be read */
>> +       if (!sti_asc_pending(dev, true))
>> +               return -EAGAIN;
>> +
>> +       return readl(&uart->rxbuf);
>> +}
>> +
>> +/* write write out a single char */
>> +static int sti_asc_serial_putc(struct udevice *dev, const char c)
>> +{
>> +       struct sti_asc_serial_platdata *plat = dev->platdata;
>> +       struct sti_asc_uart *const uart = plat->base;
>> +
>> +       /* Stream-LF to CR+LF conversion */
>> +       if (c == 10)
>> +               sti_asc_serial_putc(dev, '\r');
>
> This happens in the uclass, you don't need it here.

ok

>
>> +
>> +       /* wait till safe to write next char */
>> +       while (sti_asc_pending(dev, false))
>> +               ;
>
> if (sti_asc_pending(dev, false))
>    return -EAGAIN
>
> (no loops needed / wanted in drivers)

ok

>
>> +
>> +       /* finally, write next char */
>> +       writel(c, &uart->txbuf);
>> +
>> +       return 0;
>> +}
>> +
>> +static int sti_ofdata_to_platdata(struct udevice *dev)
>> +{
>> +       struct sti_asc_serial_platdata *priv = dev_get_priv(dev);
>> +       fdt_addr_t base;
>> +
>> +       base = dev_get_addr(dev);
>> +       if (base == FDT_ADDR_T_NONE)
>> +               return -EINVAL;
>> +
>> +       priv->base = (struct sti_asc_uart *)base;
>
> Can I suggest the name 'regs' for the registers? Normally 'base' is
> the base address. It's the same value, but regs is a pointer and base
> is a ulong.

ok will fix this

>
>> +
>> +       return 0;
>> +}
>> +
>> +static const struct dm_serial_ops sti_asc_serial_ops = {
>> +       .putc = sti_asc_serial_putc,
>> +       .pending = sti_asc_pending,
>> +       .getc = sti_asc_serial_getc,
>> +       .setbrg = sti_asc_serial_setbrg,
>> +};
>> +
>> +static const struct udevice_id sti_serial_of_match[] = {
>> +       { .compatible = "st,asc" },
>> +       { }
>> +};
>> +
>> +U_BOOT_DRIVER(serial_sti_asc) = {
>> +       .name = "serial_sti_asc",
>> +       .id = UCLASS_SERIAL,
>> +       .of_match = sti_serial_of_match,
>> +       .ops = &sti_asc_serial_ops,
>> +       .ofdata_to_platdata = sti_ofdata_to_platdata,
>> +       .probe = sti_asc_serial_probe,
>> +       .flags = DM_FLAG_PRE_RELOC,
>> +};
>> diff --git a/include/configs/stih410-b2260.h b/include/configs/stih410-b2260.h
>> index fade7a1..4fea22a 100644
>> --- a/include/configs/stih410-b2260.h
>> +++ b/include/configs/stih410-b2260.h
>> @@ -17,6 +17,7 @@
>>  #define CONFIG_SYS_LOAD_ADDR           PHYS_SDRAM_1    /* default load addr */
>>
>>  #define CONFIG_BAUDRATE                        115200
>> +#define CONFIG_SYS_BAUDRATE_TABLE       { 9600, 19200, 38400, 57600, 115200 }
>>
>>  #define CONFIG_SYS_HZ_CLOCK            1000000000      /* 1 GHz */
>>
>> diff --git a/include/dm/platform_data/serial_sti_asc.h b/include/dm/platform_data/serial_sti_asc.h
>> new file mode 100644
>> index 0000000..c8631f1
>> --- /dev/null
>> +++ b/include/dm/platform_data/serial_sti_asc.h
>> @@ -0,0 +1,17 @@
>> +/*
>> + * (C) Copyright 2017
>> + * Patrice Chotard <patrice.chotard at st.com>
>> + *
>> + * SPDX-License-Identifier:    GPL-2.0+
>> + */
>> +
>> +#ifndef __SERIAL_STI_ASC_H
>> +#define __SERIAL_STI_ASC_H
>> +
>> +/* Information about a serial port */
>> +struct sti_asc_serial_platdata {
>> +       /* address of registers in physical memory */
>> +       struct sti_asc_uart *base;
>> +};
>> +
>> +#endif /* __SERIAL_STI_ASC_H */
>> --
>> 1.9.1
>>
>
> Regards,
> Simon
>


More information about the U-Boot mailing list