Hi Simon On 02/10/2017 05:22 PM, Simon Glass wrote: > Hi Patrice, > > On 2 February 2017 at 10:00, <patrice.chot...@st.com> wrote: >> From: Patrice Chotard <patrice.chot...@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.chot...@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.chot...@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.chot...@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 > _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot