[U-Boot] [PATCH 0/3] Moving Uartlite to DM

[U-Boot] [PATCH 0/3] Moving Uartlite to DM

[Michal Simek]

Hi,

This patch series depends on microblaze cleanup series.
All these patches will be available in
git://git.denx.de/u-boot-microblaze.git
also with patches which I am going to send now.

Thanks,
Michal


Michal Simek (3):
  serial: uartlite: Move driver to DM
  serial: uartlite: Add support for debug console
  serial: uartlite: Add uartlite to Kconfig

 arch/microblaze/Kconfig                       |   1 +
 board/xilinx/microblaze-generic/xparameters.h |   4 -
 configs/microblaze-generic_defconfig          |   3 +
 doc/driver-model/serial-howto.txt             |   1 -
 drivers/serial/Kconfig                        |  14 ++
 drivers/serial/serial_xuartlite.c             | 191 ++++++++++++--------------
 include/configs/microblaze-generic.h          |   7 +-
 7 files changed, 106 insertions(+), 115 deletions(-)

-- 
1.9.1

[U-Boot] [PATCH 1/3] serial: uartlite: Move driver to DM

[Michal Simek]

Enable SPL DM too.

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 arch/microblaze/Kconfig              |   1 +
 configs/microblaze-generic_defconfig |   2 +
 doc/driver-model/serial-howto.txt    |   1 -
 drivers/serial/serial_xuartlite.c    | 176 ++++++++++++++---------------------
 4 files changed, 71 insertions(+), 109 deletions(-)

diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
index 604f6815af5b..30ea484f48aa 100644
--- a/arch/microblaze/Kconfig
+++ b/arch/microblaze/Kconfig
@@ -13,6 +13,7 @@ config TARGET_MICROBLAZE_GENERIC
 	select SUPPORT_SPL
 	select OF_CONTROL
 	select DM
+	select DM_SERIAL
 
 endchoice
 
diff --git a/configs/microblaze-generic_defconfig b/configs/microblaze-generic_defconfig
index 54aa3ef3d26f..5df080b6a87c 100644
--- a/configs/microblaze-generic_defconfig
+++ b/configs/microblaze-generic_defconfig
@@ -1,9 +1,11 @@
 CONFIG_MICROBLAZE=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+CONFIG_SPL_DM=y
 CONFIG_TARGET_MICROBLAZE_GENERIC=y
 CONFIG_DEFAULT_DEVICE_TREE="microblaze-generic"
 CONFIG_SPL=y
 CONFIG_SYS_PROMPT="U-Boot-mONStR> "
 CONFIG_CMD_GPIO=y
 # CONFIG_CMD_SETEXPR is not set
+CONFIG_SPL_OF_CONTROL=y
 CONFIG_OF_EMBED=y
diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
index 60483a4c49bc..6688abc4d9e3 100644
--- a/doc/driver-model/serial-howto.txt
+++ b/doc/driver-model/serial-howto.txt
@@ -19,7 +19,6 @@ is time for maintainers to start converting over the remaining serial drivers:
    serial_s3c24x0.c
    serial_sa1100.c
    serial_stm32.c
-   serial_xuartlite.c
    usbtty.c
 
 You should complete this by the end of January 2016.
diff --git a/drivers/serial/serial_xuartlite.c b/drivers/serial/serial_xuartlite.c
index 988438e75471..10089f5a34b5 100644
--- a/drivers/serial/serial_xuartlite.c
+++ b/drivers/serial/serial_xuartlite.c
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2008-2011 Michal Simek <monstr@monstr.eu>
+ * (C) Copyright 2008 - 2015 Michal Simek <monstr@monstr.eu>
  * Clean driver and add xilinx constant from header file
  *
  * (C) Copyright 2004 Atmark Techno, Inc.
@@ -10,11 +10,17 @@
 
 #include <config.h>
 #include <common.h>
+#include <debug_uart.h>
+#include <dm.h>
 #include <asm/io.h>
 #include <linux/compiler.h>
 #include <serial.h>
+#include <watchdog.h>
+
+DECLARE_GLOBAL_DATA_PTR;
 
 #define SR_TX_FIFO_FULL		0x08 /* transmit FIFO full */
+#define SR_TX_FIFO_EMPTY	0x04 /* transmit FIFO empty */
 #define SR_RX_FIFO_VALID_DATA	0x01 /* data in receive FIFO */
 #define SR_RX_FIFO_FULL		0x02 /* receive FIFO full */
 
@@ -28,135 +34,89 @@ struct uartlite {
 	unsigned int control;
 };
 
-static struct uartlite *userial_ports[4] = {
-#ifdef XILINX_UARTLITE_BASEADDR
-	[0] = (struct uartlite *)XILINX_UARTLITE_BASEADDR,
-#endif
-#ifdef XILINX_UARTLITE_BASEADDR1
-	[1] = (struct uartlite *)XILINX_UARTLITE_BASEADDR1,
-#endif
-#ifdef XILINX_UARTLITE_BASEADDR2
-	[2] = (struct uartlite *)XILINX_UARTLITE_BASEADDR2,
-#endif
-#ifdef XILINX_UARTLITE_BASEADDR3
-	[3] = (struct uartlite *)XILINX_UARTLITE_BASEADDR3
-#endif
+struct uartlite_priv {
+	struct uartlite *regs;
 };
 
-static void uartlite_serial_putc(const char c, const int port)
+static int uartlite_serial_putc(struct udevice *dev, const char ch)
 {
-	struct uartlite *regs = userial_ports[port];
+	struct uartlite_priv *priv = dev_get_priv(dev);
+	struct uartlite *regs = priv->regs;
 
-	if (c == '\n')
-		uartlite_serial_putc('\r', port);
+	if (in_be32(&regs->status) & SR_TX_FIFO_FULL)
+		return -EAGAIN;
 
-	while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
-		;
-	out_be32(&regs->tx_fifo, c & 0xff);
-}
+	out_be32(&regs->tx_fifo, ch & 0xff);
 
-static void uartlite_serial_puts(const char *s, const int port)
-{
-	while (*s)
-		uartlite_serial_putc(*s++, port);
+	return 0;
 }
 
-static int uartlite_serial_getc(const int port)
+static int uartlite_serial_getc(struct udevice *dev)
 {
-	struct uartlite *regs = userial_ports[port];
+	struct uartlite_priv *priv = dev_get_priv(dev);
+	struct uartlite *regs = priv->regs;
+
+	if (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
+		return -EAGAIN;
 
-	while (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
-		;
 	return in_be32(&regs->rx_fifo) & 0xff;
 }
 
-static int uartlite_serial_tstc(const int port)
+static int uartlite_serial_pending(struct udevice *dev, bool input)
 {
-	struct uartlite *regs = userial_ports[port];
+	struct uartlite_priv *priv = dev_get_priv(dev);
+	struct uartlite *regs = priv->regs;
+
+	if (input)
+		return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
 
-	return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
+	return in_be32(&regs->status) & SR_TX_FIFO_EMPTY;
 }
 
-static int uartlite_serial_init(const int port)
+static int uartlite_serial_probe(struct udevice *dev)
 {
-	struct uartlite *regs = userial_ports[port];
+	struct uartlite_priv *priv = dev_get_priv(dev);
+	struct uartlite *regs = priv->regs;
 
-	if (regs) {
-		out_be32(&regs->control, 0);
-		out_be32(&regs->control,
-			 ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
-		in_be32(&regs->control);
-		return 0;
-	}
+	out_be32(&regs->control, 0);
+	out_be32(&regs->control, ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
+	in_be32(&regs->control);
 
-	return -1;
+	return 0;
 }
 
-/* Multi serial device functions */
-#define DECLARE_ESERIAL_FUNCTIONS(port) \
-	static int userial##port##_init(void) \
-				{ return uartlite_serial_init(port); } \
-	static void userial##port##_setbrg(void) {} \
-	static int userial##port##_getc(void) \
-				{ return uartlite_serial_getc(port); } \
-	static int userial##port##_tstc(void) \
-				{ return uartlite_serial_tstc(port); } \
-	static void userial##port##_putc(const char c) \
-				{ uartlite_serial_putc(c, port); } \
-	static void userial##port##_puts(const char *s) \
-				{ uartlite_serial_puts(s, port); }
-
-/* Serial device descriptor */
-#define INIT_ESERIAL_STRUCTURE(port, __name) {	\
-	.name	= __name,			\
-	.start	= userial##port##_init,		\
-	.stop	= NULL,				\
-	.setbrg	= userial##port##_setbrg,	\
-	.getc	= userial##port##_getc,		\
-	.tstc	= userial##port##_tstc,		\
-	.putc	= userial##port##_putc,		\
-	.puts	= userial##port##_puts,		\
-}
-
-DECLARE_ESERIAL_FUNCTIONS(0);
-struct serial_device uartlite_serial0_device =
-	INIT_ESERIAL_STRUCTURE(0, "ttyUL0");
-DECLARE_ESERIAL_FUNCTIONS(1);
-struct serial_device uartlite_serial1_device =
-	INIT_ESERIAL_STRUCTURE(1, "ttyUL1");
-DECLARE_ESERIAL_FUNCTIONS(2);
-struct serial_device uartlite_serial2_device =
-	INIT_ESERIAL_STRUCTURE(2, "ttyUL2");
-DECLARE_ESERIAL_FUNCTIONS(3);
-struct serial_device uartlite_serial3_device =
-	INIT_ESERIAL_STRUCTURE(3, "ttyUL3");
-
-__weak struct serial_device *default_serial_console(void)
+static int uartlite_serial_ofdata_to_platdata(struct udevice *dev)
 {
-	if (userial_ports[0])
-		return &uartlite_serial0_device;
-	if (userial_ports[1])
-		return &uartlite_serial1_device;
-	if (userial_ports[2])
-		return &uartlite_serial2_device;
-	if (userial_ports[3])
-		return &uartlite_serial3_device;
-
-	return NULL;
-}
+	struct uartlite_priv *priv = dev_get_priv(dev);
+	fdt_addr_t addr;
 
-void uartlite_serial_initialize(void)
-{
-#ifdef XILINX_UARTLITE_BASEADDR
-	serial_register(&uartlite_serial0_device);
-#endif /* XILINX_UARTLITE_BASEADDR */
-#ifdef XILINX_UARTLITE_BASEADDR1
-	serial_register(&uartlite_serial1_device);
-#endif /* XILINX_UARTLITE_BASEADDR1 */
-#ifdef XILINX_UARTLITE_BASEADDR2
-	serial_register(&uartlite_serial2_device);
-#endif /* XILINX_UARTLITE_BASEADDR2 */
-#ifdef XILINX_UARTLITE_BASEADDR3
-	serial_register(&uartlite_serial3_device);
-#endif /* XILINX_UARTLITE_BASEADDR3 */
+	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	priv->regs = (struct uartlite *)addr;
+
+	return 0;
 }
+
+static const struct dm_serial_ops uartlite_serial_ops = {
+	.putc = uartlite_serial_putc,
+	.pending = uartlite_serial_pending,
+	.getc = uartlite_serial_getc,
+};
+
+static const struct udevice_id uartlite_serial_ids[] = {
+	{ .compatible = "xlnx,xps-uartlite-1.00.a" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_uartlite) = {
+	.name	= "serial_uartlite",
+	.id	= UCLASS_SERIAL,
+	.of_match = uartlite_serial_ids,
+	.ofdata_to_platdata = uartlite_serial_ofdata_to_platdata,
+	.priv_auto_alloc_size = sizeof(struct uartlite_priv),
+	.probe = uartlite_serial_probe,
+	.ops	= &uartlite_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
-- 
1.9.1

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Michal Simek]

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 drivers/serial/Kconfig            |  7 +++++++
 drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index 1fc287ee98ec..f1e221799b81 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -92,6 +92,13 @@ config DEBUG_UART_S5P
 	  will need to provide parameters to make this work. The driver will
 	  be available until the real driver-model serial is running.
 
+config DEBUG_UART_UARTLITE
+	bool "Xilinx Uartlite"
+	help
+	  Select this to enable a debug UART using the serial_uartlite driver.
+	  You will need to provide parameters to make this work. The driver will
+	  be available until the real driver-model serial is running.
+
 config DEBUG_UART_ZYNQ
 	bool "Xilinx Zynq"
 	help
diff --git a/drivers/serial/serial_xuartlite.c b/drivers/serial/serial_xuartlite.c
index 10089f5a34b5..fe87b515d902 100644
--- a/drivers/serial/serial_xuartlite.c
+++ b/drivers/serial/serial_xuartlite.c
@@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
 	.ops	= &uartlite_serial_ops,
 	.flags = DM_FLAG_PRE_RELOC,
 };
+
+#ifdef CONFIG_DEBUG_UART_UARTLITE
+void _debug_uart_init(void)
+{
+	struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
+
+	out_be32(&regs->control, 0);
+	out_be32(&regs->control, ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
+	in_be32(&regs->control);
+}
+
+static inline void _debug_uart_putc(int ch)
+{
+	struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
+
+	while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
+		WATCHDOG_RESET();
+
+	out_be32(&regs->tx_fifo, ch & 0xff);
+}
+
+DEBUG_UART_FUNCS
+#endif
-- 
1.9.1

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Michal Simek]

- Move config option out of board file.
- Remove uartlite address from config file

Signed-off-by: Michal Simek <michal.simek@xilinx.com>
---

 board/xilinx/microblaze-generic/xparameters.h | 4 ----
 configs/microblaze-generic_defconfig          | 1 +
 drivers/serial/Kconfig                        | 7 +++++++
 include/configs/microblaze-generic.h          | 7 +------
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/board/xilinx/microblaze-generic/xparameters.h b/board/xilinx/microblaze-generic/xparameters.h
index 8ba146cb88db..11b3c9a4846e 100644
--- a/board/xilinx/microblaze-generic/xparameters.h
+++ b/board/xilinx/microblaze-generic/xparameters.h
@@ -28,10 +28,6 @@
 #define XILINX_TIMER_BASEADDR	0x41c00000
 #define XILINX_TIMER_IRQ	0
 
-/* Uart pheriphery is RS232_Uart */
-#define XILINX_UARTLITE_BASEADDR	0x40600000
-#define XILINX_UARTLITE_BAUDRATE	115200
-
 /* IIC pheriphery is IIC_EEPROM */
 #define XILINX_IIC_0_BASEADDR	0x40800000
 #define XILINX_IIC_0_FREQ	100000
diff --git a/configs/microblaze-generic_defconfig b/configs/microblaze-generic_defconfig
index 5df080b6a87c..9a7bb915466f 100644
--- a/configs/microblaze-generic_defconfig
+++ b/configs/microblaze-generic_defconfig
@@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
 # CONFIG_CMD_SETEXPR is not set
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_OF_EMBED=y
+CONFIG_XILINX_UARTLITE=y
diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
index f1e221799b81..ddf49ba9cef3 100644
--- a/drivers/serial/Kconfig
+++ b/drivers/serial/Kconfig
@@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
 	  If you have a UniPhier based board and want to use the on-chip
 	  serial ports, say Y to this option. If unsure, say N.
 
+config XILINX_UARTLITE
+	bool "Xilinx Uarlite support"
+	depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
+	help
+	  If you have a Xilinx based board and want to use the uartlite
+	  serial ports, say Y to this option. If unsure, say N.
+
 endmenu
diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h
index 10ac8328b8ff..6e3c80b14350 100644
--- a/include/configs/microblaze-generic.h
+++ b/include/configs/microblaze-generic.h
@@ -37,10 +37,7 @@
 # define CONFIG_SYS_BAUDRATE_TABLE \
 	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
 
-#ifdef XILINX_UARTLITE_BASEADDR
-# define CONFIG_XILINX_UARTLITE
-# define CONFIG_SERIAL_BASE	XILINX_UARTLITE_BASEADDR
-#elif XILINX_UART16550_BASEADDR
+#if XILINX_UART16550_BASEADDR
 # define CONFIG_SYS_NS16550_SERIAL
 # if defined(__MICROBLAZEEL__)
 #  define CONFIG_SYS_NS16550_REG_SIZE	-4
@@ -51,8 +48,6 @@
 # define CONFIG_SYS_NS16550_COM1 \
 		((XILINX_UART16550_BASEADDR & ~0xF) + 0x1000)
 # define CONFIG_SYS_NS16550_CLK	XILINX_UART16550_CLOCK_HZ
-#else
-# error Undefined uart
 #endif
 
 /* setting reset address */
-- 
1.9.1

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Thomas Chou]

Hi Michal,

On 2015?12?11? 19:54, Michal Simek wrote:
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>   drivers/serial/Kconfig            |  7 +++++++
>   drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>   2 files changed, 30 insertions(+)
>
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index 1fc287ee98ec..f1e221799b81 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -92,6 +92,13 @@ config DEBUG_UART_S5P
>   	  will need to provide parameters to make this work. The driver will
>   	  be available until the real driver-model serial is running.
>
> +config DEBUG_UART_UARTLITE
> +	bool "Xilinx Uartlite"
> +	help
> +	  Select this to enable a debug UART using the serial_uartlite driver.
> +	  You will need to provide parameters to make this work. The driver will
> +	  be available until the real driver-model serial is running.
> +
>   config DEBUG_UART_ZYNQ
>   	bool "Xilinx Zynq"
>   	help
> diff --git a/drivers/serial/serial_xuartlite.c b/drivers/serial/serial_xuartlite.c
> index 10089f5a34b5..fe87b515d902 100644
> --- a/drivers/serial/serial_xuartlite.c
> +++ b/drivers/serial/serial_xuartlite.c
> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>   	.ops	= &uartlite_serial_ops,
>   	.flags = DM_FLAG_PRE_RELOC,
>   };
> +
> +#ifdef CONFIG_DEBUG_UART_UARTLITE

Better move the "#include <debug_uart.h>" here.

> +void _debug_uart_init(void)

Please add "static inline" to void _debug_uart_init(void).

> +{
> +	struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
> +
> +	out_be32(&regs->control, 0);
> +	out_be32(&regs->control, ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
> +	in_be32(&regs->control);
> +}
> +
> +static inline void _debug_uart_putc(int ch)
> +{
> +	struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
> +
> +	while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
> +		WATCHDOG_RESET();

WATCHDOG_RESET() is not really needed for debug serial output.

> +
> +	out_be32(&regs->tx_fifo, ch & 0xff);
> +}
> +
> +DEBUG_UART_FUNCS
> +#endif
>

Best regards,
Thomas

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Thomas Chou]

Hi Michal,

On 2015?12?11? 19:54, Michal Simek wrote:
> - Move config option out of board file.
> - Remove uartlite address from config file
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>   board/xilinx/microblaze-generic/xparameters.h | 4 ----
>   configs/microblaze-generic_defconfig          | 1 +
>   drivers/serial/Kconfig                        | 7 +++++++
>   include/configs/microblaze-generic.h          | 7 +------
>   4 files changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/board/xilinx/microblaze-generic/xparameters.h b/board/xilinx/microblaze-generic/xparameters.h
> index 8ba146cb88db..11b3c9a4846e 100644
> --- a/board/xilinx/microblaze-generic/xparameters.h
> +++ b/board/xilinx/microblaze-generic/xparameters.h
> @@ -28,10 +28,6 @@
>   #define XILINX_TIMER_BASEADDR	0x41c00000
>   #define XILINX_TIMER_IRQ	0
>
> -/* Uart pheriphery is RS232_Uart */
> -#define XILINX_UARTLITE_BASEADDR	0x40600000
> -#define XILINX_UARTLITE_BAUDRATE	115200
> -
>   /* IIC pheriphery is IIC_EEPROM */
>   #define XILINX_IIC_0_BASEADDR	0x40800000
>   #define XILINX_IIC_0_FREQ	100000
> diff --git a/configs/microblaze-generic_defconfig b/configs/microblaze-generic_defconfig
> index 5df080b6a87c..9a7bb915466f 100644
> --- a/configs/microblaze-generic_defconfig
> +++ b/configs/microblaze-generic_defconfig
> @@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
>   # CONFIG_CMD_SETEXPR is not set
>   CONFIG_SPL_OF_CONTROL=y
>   CONFIG_OF_EMBED=y
> +CONFIG_XILINX_UARTLITE=y
> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
> index f1e221799b81..ddf49ba9cef3 100644
> --- a/drivers/serial/Kconfig
> +++ b/drivers/serial/Kconfig
> @@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
>   	  If you have a UniPhier based board and want to use the on-chip
>   	  serial ports, say Y to this option. If unsure, say N.
>
> +config XILINX_UARTLITE
> +	bool "Xilinx Uarlite support"
> +	depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
> +	help
> +	  If you have a Xilinx based board and want to use the uartlite
> +	  serial ports, say Y to this option. If unsure, say N.
> +
>   endmenu

Kconfig should be with the driver 1/3. Others are boards related.

> diff --git a/include/configs/microblaze-generic.h b/include/configs/microblaze-generic.h
> index 10ac8328b8ff..6e3c80b14350 100644
> --- a/include/configs/microblaze-generic.h
> +++ b/include/configs/microblaze-generic.h
> @@ -37,10 +37,7 @@
>   # define CONFIG_SYS_BAUDRATE_TABLE \
>   	{300, 600, 1200, 2400, 4800, 9600, 19200, 38400, 57600, 115200, 230400}
>
> -#ifdef XILINX_UARTLITE_BASEADDR
> -# define CONFIG_XILINX_UARTLITE
> -# define CONFIG_SERIAL_BASE	XILINX_UARTLITE_BASEADDR
> -#elif XILINX_UART16550_BASEADDR
> +#if XILINX_UART16550_BASEADDR
>   # define CONFIG_SYS_NS16550_SERIAL
>   # if defined(__MICROBLAZEEL__)
>   #  define CONFIG_SYS_NS16550_REG_SIZE	-4
> @@ -51,8 +48,6 @@
>   # define CONFIG_SYS_NS16550_COM1 \
>   		((XILINX_UART16550_BASEADDR & ~0xF) + 0x1000)
>   # define CONFIG_SYS_NS16550_CLK	XILINX_UART16550_CLOCK_HZ
> -#else
> -# error Undefined uart
>   #endif
>
>   /* setting reset address */
>

Thanks.

Best regards,
Thomas

[U-Boot] [PATCH 1/3] serial: uartlite: Move driver to DM

[Thomas Chou]

Hi Michal,

On 2015?12?11? 19:54, Michal Simek wrote:
> Enable SPL DM too.
>
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
>
>   arch/microblaze/Kconfig              |   1 +
>   configs/microblaze-generic_defconfig |   2 +
>   doc/driver-model/serial-howto.txt    |   1 -
>   drivers/serial/serial_xuartlite.c    | 176 ++++++++++++++---------------------
>   4 files changed, 71 insertions(+), 109 deletions(-)
>
> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
> index 604f6815af5b..30ea484f48aa 100644
> --- a/arch/microblaze/Kconfig
> +++ b/arch/microblaze/Kconfig
> @@ -13,6 +13,7 @@ config TARGET_MICROBLAZE_GENERIC
>   	select SUPPORT_SPL
>   	select OF_CONTROL
>   	select DM
> +	select DM_SERIAL
>
>   endchoice
>
> diff --git a/configs/microblaze-generic_defconfig b/configs/microblaze-generic_defconfig
> index 54aa3ef3d26f..5df080b6a87c 100644
> --- a/configs/microblaze-generic_defconfig
> +++ b/configs/microblaze-generic_defconfig
> @@ -1,9 +1,11 @@
>   CONFIG_MICROBLAZE=y
>   CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> +CONFIG_SPL_DM=y
>   CONFIG_TARGET_MICROBLAZE_GENERIC=y
>   CONFIG_DEFAULT_DEVICE_TREE="microblaze-generic"
>   CONFIG_SPL=y
>   CONFIG_SYS_PROMPT="U-Boot-mONStR> "
>   CONFIG_CMD_GPIO=y
>   # CONFIG_CMD_SETEXPR is not set
> +CONFIG_SPL_OF_CONTROL=y
>   CONFIG_OF_EMBED=y
> diff --git a/doc/driver-model/serial-howto.txt b/doc/driver-model/serial-howto.txt
> index 60483a4c49bc..6688abc4d9e3 100644
> --- a/doc/driver-model/serial-howto.txt
> +++ b/doc/driver-model/serial-howto.txt
> @@ -19,7 +19,6 @@ is time for maintainers to start converting over the remaining serial drivers:
>      serial_s3c24x0.c
>      serial_sa1100.c
>      serial_stm32.c
> -   serial_xuartlite.c
>      usbtty.c
>
>   You should complete this by the end of January 2016.
> diff --git a/drivers/serial/serial_xuartlite.c b/drivers/serial/serial_xuartlite.c
> index 988438e75471..10089f5a34b5 100644
> --- a/drivers/serial/serial_xuartlite.c
> +++ b/drivers/serial/serial_xuartlite.c
> @@ -1,5 +1,5 @@
>   /*
> - * (C) Copyright 2008-2011 Michal Simek <monstr@monstr.eu>
> + * (C) Copyright 2008 - 2015 Michal Simek <monstr@monstr.eu>
>    * Clean driver and add xilinx constant from header file
>    *
>    * (C) Copyright 2004 Atmark Techno, Inc.
> @@ -10,11 +10,17 @@
>
>   #include <config.h>
>   #include <common.h>
> +#include <debug_uart.h>

Move to debug uart section.

> +#include <dm.h>
>   #include <asm/io.h>
>   #include <linux/compiler.h>
>   #include <serial.h>
> +#include <watchdog.h>

watchdog.h is not needed.

> +
> +DECLARE_GLOBAL_DATA_PTR;
>
>   #define SR_TX_FIFO_FULL		0x08 /* transmit FIFO full */
> +#define SR_TX_FIFO_EMPTY	0x04 /* transmit FIFO empty */
>   #define SR_RX_FIFO_VALID_DATA	0x01 /* data in receive FIFO */
>   #define SR_RX_FIFO_FULL		0x02 /* receive FIFO full */
>
> @@ -28,135 +34,89 @@ struct uartlite {
>   	unsigned int control;
>   };
>
> -static struct uartlite *userial_ports[4] = {
> -#ifdef XILINX_UARTLITE_BASEADDR
> -	[0] = (struct uartlite *)XILINX_UARTLITE_BASEADDR,
> -#endif
> -#ifdef XILINX_UARTLITE_BASEADDR1
> -	[1] = (struct uartlite *)XILINX_UARTLITE_BASEADDR1,
> -#endif
> -#ifdef XILINX_UARTLITE_BASEADDR2
> -	[2] = (struct uartlite *)XILINX_UARTLITE_BASEADDR2,
> -#endif
> -#ifdef XILINX_UARTLITE_BASEADDR3
> -	[3] = (struct uartlite *)XILINX_UARTLITE_BASEADDR3
> -#endif
> +struct uartlite_priv {
> +	struct uartlite *regs;
>   };
>
> -static void uartlite_serial_putc(const char c, const int port)
> +static int uartlite_serial_putc(struct udevice *dev, const char ch)
>   {
> -	struct uartlite *regs = userial_ports[port];
> +	struct uartlite_priv *priv = dev_get_priv(dev);
> +	struct uartlite *regs = priv->regs;
>
> -	if (c == '\n')
> -		uartlite_serial_putc('\r', port);
> +	if (in_be32(&regs->status) & SR_TX_FIFO_FULL)
> +		return -EAGAIN;
>
> -	while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
> -		;
> -	out_be32(&regs->tx_fifo, c & 0xff);
> -}
> +	out_be32(&regs->tx_fifo, ch & 0xff);
>
> -static void uartlite_serial_puts(const char *s, const int port)
> -{
> -	while (*s)
> -		uartlite_serial_putc(*s++, port);
> +	return 0;
>   }
>
> -static int uartlite_serial_getc(const int port)
> +static int uartlite_serial_getc(struct udevice *dev)
>   {
> -	struct uartlite *regs = userial_ports[port];
> +	struct uartlite_priv *priv = dev_get_priv(dev);
> +	struct uartlite *regs = priv->regs;
> +
> +	if (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
> +		return -EAGAIN;
>
> -	while (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
> -		;
>   	return in_be32(&regs->rx_fifo) & 0xff;
>   }
>
> -static int uartlite_serial_tstc(const int port)
> +static int uartlite_serial_pending(struct udevice *dev, bool input)
>   {
> -	struct uartlite *regs = userial_ports[port];
> +	struct uartlite_priv *priv = dev_get_priv(dev);
> +	struct uartlite *regs = priv->regs;
> +
> +	if (input)
> +		return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>
> -	return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
> +	return in_be32(&regs->status) & SR_TX_FIFO_EMPTY;
>   }
>
> -static int uartlite_serial_init(const int port)
> +static int uartlite_serial_probe(struct udevice *dev)
>   {
> -	struct uartlite *regs = userial_ports[port];
> +	struct uartlite_priv *priv = dev_get_priv(dev);
> +	struct uartlite *regs = priv->regs;
>
> -	if (regs) {
> -		out_be32(&regs->control, 0);
> -		out_be32(&regs->control,
> -			 ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
> -		in_be32(&regs->control);
> -		return 0;
> -	}
> +	out_be32(&regs->control, 0);
> +	out_be32(&regs->control, ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
> +	in_be32(&regs->control);
>
> -	return -1;
> +	return 0;
>   }
>
> -/* Multi serial device functions */
> -#define DECLARE_ESERIAL_FUNCTIONS(port) \
> -	static int userial##port##_init(void) \
> -				{ return uartlite_serial_init(port); } \
> -	static void userial##port##_setbrg(void) {} \
> -	static int userial##port##_getc(void) \
> -				{ return uartlite_serial_getc(port); } \
> -	static int userial##port##_tstc(void) \
> -				{ return uartlite_serial_tstc(port); } \
> -	static void userial##port##_putc(const char c) \
> -				{ uartlite_serial_putc(c, port); } \
> -	static void userial##port##_puts(const char *s) \
> -				{ uartlite_serial_puts(s, port); }
> -
> -/* Serial device descriptor */
> -#define INIT_ESERIAL_STRUCTURE(port, __name) {	\
> -	.name	= __name,			\
> -	.start	= userial##port##_init,		\
> -	.stop	= NULL,				\
> -	.setbrg	= userial##port##_setbrg,	\
> -	.getc	= userial##port##_getc,		\
> -	.tstc	= userial##port##_tstc,		\
> -	.putc	= userial##port##_putc,		\
> -	.puts	= userial##port##_puts,		\
> -}
> -
> -DECLARE_ESERIAL_FUNCTIONS(0);
> -struct serial_device uartlite_serial0_device =
> -	INIT_ESERIAL_STRUCTURE(0, "ttyUL0");
> -DECLARE_ESERIAL_FUNCTIONS(1);
> -struct serial_device uartlite_serial1_device =
> -	INIT_ESERIAL_STRUCTURE(1, "ttyUL1");
> -DECLARE_ESERIAL_FUNCTIONS(2);
> -struct serial_device uartlite_serial2_device =
> -	INIT_ESERIAL_STRUCTURE(2, "ttyUL2");
> -DECLARE_ESERIAL_FUNCTIONS(3);
> -struct serial_device uartlite_serial3_device =
> -	INIT_ESERIAL_STRUCTURE(3, "ttyUL3");
> -
> -__weak struct serial_device *default_serial_console(void)
> +static int uartlite_serial_ofdata_to_platdata(struct udevice *dev)
>   {
> -	if (userial_ports[0])
> -		return &uartlite_serial0_device;
> -	if (userial_ports[1])
> -		return &uartlite_serial1_device;
> -	if (userial_ports[2])
> -		return &uartlite_serial2_device;
> -	if (userial_ports[3])
> -		return &uartlite_serial3_device;
> -
> -	return NULL;
> -}
> +	struct uartlite_priv *priv = dev_get_priv(dev);

Since the conversion is "ofdatta to platdata", this should be platdata 
rather than priv. If there is not variable part, you may simply use 
platdata in the ops. In some cases, you may add device from platdata 
without ofdata.

> +	fdt_addr_t addr;
>
> -void uartlite_serial_initialize(void)
> -{
> -#ifdef XILINX_UARTLITE_BASEADDR
> -	serial_register(&uartlite_serial0_device);
> -#endif /* XILINX_UARTLITE_BASEADDR */
> -#ifdef XILINX_UARTLITE_BASEADDR1
> -	serial_register(&uartlite_serial1_device);
> -#endif /* XILINX_UARTLITE_BASEADDR1 */
> -#ifdef XILINX_UARTLITE_BASEADDR2
> -	serial_register(&uartlite_serial2_device);
> -#endif /* XILINX_UARTLITE_BASEADDR2 */
> -#ifdef XILINX_UARTLITE_BASEADDR3
> -	serial_register(&uartlite_serial3_device);
> -#endif /* XILINX_UARTLITE_BASEADDR3 */
> +	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");

Use dev_get_addr() is preferred.

> +	if (addr == FDT_ADDR_T_NONE)
> +		return -EINVAL;
> +
> +	priv->regs = (struct uartlite *)addr;
> +
> +	return 0;
>   }
> +
> +static const struct dm_serial_ops uartlite_serial_ops = {
> +	.putc = uartlite_serial_putc,
> +	.pending = uartlite_serial_pending,
> +	.getc = uartlite_serial_getc,
> +};
> +
> +static const struct udevice_id uartlite_serial_ids[] = {
> +	{ .compatible = "xlnx,xps-uartlite-1.00.a" },
> +	{ }
> +};
> +

Please add binding to doc/device-tree-bindings/serial/ .

> +U_BOOT_DRIVER(serial_uartlite) = {
> +	.name	= "serial_uartlite",
> +	.id	= UCLASS_SERIAL,
> +	.of_match = uartlite_serial_ids,
> +	.ofdata_to_platdata = uartlite_serial_ofdata_to_platdata,
> +	.priv_auto_alloc_size = sizeof(struct uartlite_priv),

maybe platdata.

> +	.probe = uartlite_serial_probe,
> +	.ops	= &uartlite_serial_ops,
> +	.flags = DM_FLAG_PRE_RELOC,
> +};
>

Thanks.

Best regards,
Thomas

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Michal Simek]

On 14.12.2015 15:35, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015?12?11? 19:54, Michal Simek wrote:
>> - Move config option out of board file.
>> - Remove uartlite address from config file
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>   board/xilinx/microblaze-generic/xparameters.h | 4 ----
>>   configs/microblaze-generic_defconfig          | 1 +
>>   drivers/serial/Kconfig                        | 7 +++++++
>>   include/configs/microblaze-generic.h          | 7 +------
>>   4 files changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/board/xilinx/microblaze-generic/xparameters.h
>> b/board/xilinx/microblaze-generic/xparameters.h
>> index 8ba146cb88db..11b3c9a4846e 100644
>> --- a/board/xilinx/microblaze-generic/xparameters.h
>> +++ b/board/xilinx/microblaze-generic/xparameters.h
>> @@ -28,10 +28,6 @@
>>   #define XILINX_TIMER_BASEADDR    0x41c00000
>>   #define XILINX_TIMER_IRQ    0
>>
>> -/* Uart pheriphery is RS232_Uart */
>> -#define XILINX_UARTLITE_BASEADDR    0x40600000
>> -#define XILINX_UARTLITE_BAUDRATE    115200
>> -
>>   /* IIC pheriphery is IIC_EEPROM */
>>   #define XILINX_IIC_0_BASEADDR    0x40800000
>>   #define XILINX_IIC_0_FREQ    100000
>> diff --git a/configs/microblaze-generic_defconfig
>> b/configs/microblaze-generic_defconfig
>> index 5df080b6a87c..9a7bb915466f 100644
>> --- a/configs/microblaze-generic_defconfig
>> +++ b/configs/microblaze-generic_defconfig
>> @@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
>>   # CONFIG_CMD_SETEXPR is not set
>>   CONFIG_SPL_OF_CONTROL=y
>>   CONFIG_OF_EMBED=y
>> +CONFIG_XILINX_UARTLITE=y
>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>> index f1e221799b81..ddf49ba9cef3 100644
>> --- a/drivers/serial/Kconfig
>> +++ b/drivers/serial/Kconfig
>> @@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
>>         If you have a UniPhier based board and want to use the on-chip
>>         serial ports, say Y to this option. If unsure, say N.
>>
>> +config XILINX_UARTLITE
>> +    bool "Xilinx Uarlite support"
>> +    depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
>> +    help
>> +      If you have a Xilinx based board and want to use the uartlite
>> +      serial ports, say Y to this option. If unsure, say N.
>> +
>>   endmenu
> 
> Kconfig should be with the driver 1/3. Others are boards related.

It can be. Adding Kconfig fragment can be separate patch out of move to
DM. There is no connection too.
My intention was to show all related changes which are done by this one
step.

Thanks,
Michal

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Michal Simek]

Hi,

On 14.12.2015 14:14, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015?12?11? 19:54, Michal Simek wrote:
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>   drivers/serial/Kconfig            |  7 +++++++
>>   drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>>   2 files changed, 30 insertions(+)
>>
>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>> index 1fc287ee98ec..f1e221799b81 100644
>> --- a/drivers/serial/Kconfig
>> +++ b/drivers/serial/Kconfig
>> @@ -92,6 +92,13 @@ config DEBUG_UART_S5P
>>         will need to provide parameters to make this work. The driver
>> will
>>         be available until the real driver-model serial is running.
>>
>> +config DEBUG_UART_UARTLITE
>> +    bool "Xilinx Uartlite"
>> +    help
>> +      Select this to enable a debug UART using the serial_uartlite
>> driver.
>> +      You will need to provide parameters to make this work. The
>> driver will
>> +      be available until the real driver-model serial is running.
>> +
>>   config DEBUG_UART_ZYNQ
>>       bool "Xilinx Zynq"
>>       help
>> diff --git a/drivers/serial/serial_xuartlite.c
>> b/drivers/serial/serial_xuartlite.c
>> index 10089f5a34b5..fe87b515d902 100644
>> --- a/drivers/serial/serial_xuartlite.c
>> +++ b/drivers/serial/serial_xuartlite.c
>> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>>       .ops    = &uartlite_serial_ops,
>>       .flags = DM_FLAG_PRE_RELOC,
>>   };
>> +
>> +#ifdef CONFIG_DEBUG_UART_UARTLITE
> 
> Better move the "#include <debug_uart.h>" here.

This is the patch I sent some days ago about removing it from this
location and it was Reviewed twice.
http://lists.denx.de/pipermail/u-boot/2015-December/236341.html

> 
>> +void _debug_uart_init(void)
> 
> Please add "static inline" to void _debug_uart_init(void).

Ok. Will fix this for uartlite and zynq uart in follow up patch.


>> +{
>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>> +
>> +    out_be32(&regs->control, 0);
>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>> ULITE_CONTROL_RST_TX);
>> +    in_be32(&regs->control);
>> +}
>> +
>> +static inline void _debug_uart_putc(int ch)
>> +{
>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>> +
>> +    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>> +        WATCHDOG_RESET();
> 
> WATCHDOG_RESET() is not really needed for debug serial output.

TBH I don't think so. There could be watchdog running from early
bootloader and needs to be service even for debugging purpose.

Thanks,
Michal

[U-Boot] [PATCH 1/3] serial: uartlite: Move driver to DM

[Michal Simek]

On 14.12.2015 15:47, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015?12?11? 19:54, Michal Simek wrote:
>> Enable SPL DM too.
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>>   arch/microblaze/Kconfig              |   1 +
>>   configs/microblaze-generic_defconfig |   2 +
>>   doc/driver-model/serial-howto.txt    |   1 -
>>   drivers/serial/serial_xuartlite.c    | 176
>> ++++++++++++++---------------------
>>   4 files changed, 71 insertions(+), 109 deletions(-)
>>
>> diff --git a/arch/microblaze/Kconfig b/arch/microblaze/Kconfig
>> index 604f6815af5b..30ea484f48aa 100644
>> --- a/arch/microblaze/Kconfig
>> +++ b/arch/microblaze/Kconfig
>> @@ -13,6 +13,7 @@ config TARGET_MICROBLAZE_GENERIC
>>       select SUPPORT_SPL
>>       select OF_CONTROL
>>       select DM
>> +    select DM_SERIAL
>>
>>   endchoice
>>
>> diff --git a/configs/microblaze-generic_defconfig
>> b/configs/microblaze-generic_defconfig
>> index 54aa3ef3d26f..5df080b6a87c 100644
>> --- a/configs/microblaze-generic_defconfig
>> +++ b/configs/microblaze-generic_defconfig
>> @@ -1,9 +1,11 @@
>>   CONFIG_MICROBLAZE=y
>>   CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>> +CONFIG_SPL_DM=y
>>   CONFIG_TARGET_MICROBLAZE_GENERIC=y
>>   CONFIG_DEFAULT_DEVICE_TREE="microblaze-generic"
>>   CONFIG_SPL=y
>>   CONFIG_SYS_PROMPT="U-Boot-mONStR> "
>>   CONFIG_CMD_GPIO=y
>>   # CONFIG_CMD_SETEXPR is not set
>> +CONFIG_SPL_OF_CONTROL=y
>>   CONFIG_OF_EMBED=y
>> diff --git a/doc/driver-model/serial-howto.txt
>> b/doc/driver-model/serial-howto.txt
>> index 60483a4c49bc..6688abc4d9e3 100644
>> --- a/doc/driver-model/serial-howto.txt
>> +++ b/doc/driver-model/serial-howto.txt
>> @@ -19,7 +19,6 @@ is time for maintainers to start converting over the
>> remaining serial drivers:
>>      serial_s3c24x0.c
>>      serial_sa1100.c
>>      serial_stm32.c
>> -   serial_xuartlite.c
>>      usbtty.c
>>
>>   You should complete this by the end of January 2016.
>> diff --git a/drivers/serial/serial_xuartlite.c
>> b/drivers/serial/serial_xuartlite.c
>> index 988438e75471..10089f5a34b5 100644
>> --- a/drivers/serial/serial_xuartlite.c
>> +++ b/drivers/serial/serial_xuartlite.c
>> @@ -1,5 +1,5 @@
>>   /*
>> - * (C) Copyright 2008-2011 Michal Simek <monstr@monstr.eu>
>> + * (C) Copyright 2008 - 2015 Michal Simek <monstr@monstr.eu>
>>    * Clean driver and add xilinx constant from header file
>>    *
>>    * (C) Copyright 2004 Atmark Techno, Inc.
>> @@ -10,11 +10,17 @@
>>
>>   #include <config.h>
>>   #include <common.h>
>> +#include <debug_uart.h>
> 
> Move to debug uart section.
> 
>> +#include <dm.h>
>>   #include <asm/io.h>
>>   #include <linux/compiler.h>
>>   #include <serial.h>
>> +#include <watchdog.h>
> 
> watchdog.h is not needed.

ok these two should be added when debug_uart is added.
The rest is in the second reply.

> 
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>>
>>   #define SR_TX_FIFO_FULL        0x08 /* transmit FIFO full */
>> +#define SR_TX_FIFO_EMPTY    0x04 /* transmit FIFO empty */
>>   #define SR_RX_FIFO_VALID_DATA    0x01 /* data in receive FIFO */
>>   #define SR_RX_FIFO_FULL        0x02 /* receive FIFO full */
>>
>> @@ -28,135 +34,89 @@ struct uartlite {
>>       unsigned int control;
>>   };
>>
>> -static struct uartlite *userial_ports[4] = {
>> -#ifdef XILINX_UARTLITE_BASEADDR
>> -    [0] = (struct uartlite *)XILINX_UARTLITE_BASEADDR,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR1
>> -    [1] = (struct uartlite *)XILINX_UARTLITE_BASEADDR1,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR2
>> -    [2] = (struct uartlite *)XILINX_UARTLITE_BASEADDR2,
>> -#endif
>> -#ifdef XILINX_UARTLITE_BASEADDR3
>> -    [3] = (struct uartlite *)XILINX_UARTLITE_BASEADDR3
>> -#endif
>> +struct uartlite_priv {
>> +    struct uartlite *regs;
>>   };
>>
>> -static void uartlite_serial_putc(const char c, const int port)
>> +static int uartlite_serial_putc(struct udevice *dev, const char ch)
>>   {
>> -    struct uartlite *regs = userial_ports[port];
>> +    struct uartlite_priv *priv = dev_get_priv(dev);
>> +    struct uartlite *regs = priv->regs;
>>
>> -    if (c == '\n')
>> -        uartlite_serial_putc('\r', port);
>> +    if (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>> +        return -EAGAIN;
>>
>> -    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>> -        ;
>> -    out_be32(&regs->tx_fifo, c & 0xff);
>> -}
>> +    out_be32(&regs->tx_fifo, ch & 0xff);
>>
>> -static void uartlite_serial_puts(const char *s, const int port)
>> -{
>> -    while (*s)
>> -        uartlite_serial_putc(*s++, port);
>> +    return 0;
>>   }
>>
>> -static int uartlite_serial_getc(const int port)
>> +static int uartlite_serial_getc(struct udevice *dev)
>>   {
>> -    struct uartlite *regs = userial_ports[port];
>> +    struct uartlite_priv *priv = dev_get_priv(dev);
>> +    struct uartlite *regs = priv->regs;
>> +
>> +    if (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
>> +        return -EAGAIN;
>>
>> -    while (!(in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA))
>> -        ;
>>       return in_be32(&regs->rx_fifo) & 0xff;
>>   }
>>
>> -static int uartlite_serial_tstc(const int port)
>> +static int uartlite_serial_pending(struct udevice *dev, bool input)
>>   {
>> -    struct uartlite *regs = userial_ports[port];
>> +    struct uartlite_priv *priv = dev_get_priv(dev);
>> +    struct uartlite *regs = priv->regs;
>> +
>> +    if (input)
>> +        return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>>
>> -    return in_be32(&regs->status) & SR_RX_FIFO_VALID_DATA;
>> +    return in_be32(&regs->status) & SR_TX_FIFO_EMPTY;
>>   }
>>
>> -static int uartlite_serial_init(const int port)
>> +static int uartlite_serial_probe(struct udevice *dev)
>>   {
>> -    struct uartlite *regs = userial_ports[port];
>> +    struct uartlite_priv *priv = dev_get_priv(dev);
>> +    struct uartlite *regs = priv->regs;
>>
>> -    if (regs) {
>> -        out_be32(&regs->control, 0);
>> -        out_be32(&regs->control,
>> -             ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
>> -        in_be32(&regs->control);
>> -        return 0;
>> -    }
>> +    out_be32(&regs->control, 0);
>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>> ULITE_CONTROL_RST_TX);
>> +    in_be32(&regs->control);
>>
>> -    return -1;
>> +    return 0;
>>   }
>>
>> -/* Multi serial device functions */
>> -#define DECLARE_ESERIAL_FUNCTIONS(port) \
>> -    static int userial##port##_init(void) \
>> -                { return uartlite_serial_init(port); } \
>> -    static void userial##port##_setbrg(void) {} \
>> -    static int userial##port##_getc(void) \
>> -                { return uartlite_serial_getc(port); } \
>> -    static int userial##port##_tstc(void) \
>> -                { return uartlite_serial_tstc(port); } \
>> -    static void userial##port##_putc(const char c) \
>> -                { uartlite_serial_putc(c, port); } \
>> -    static void userial##port##_puts(const char *s) \
>> -                { uartlite_serial_puts(s, port); }
>> -
>> -/* Serial device descriptor */
>> -#define INIT_ESERIAL_STRUCTURE(port, __name) {    \
>> -    .name    = __name,            \
>> -    .start    = userial##port##_init,        \
>> -    .stop    = NULL,                \
>> -    .setbrg    = userial##port##_setbrg,    \
>> -    .getc    = userial##port##_getc,        \
>> -    .tstc    = userial##port##_tstc,        \
>> -    .putc    = userial##port##_putc,        \
>> -    .puts    = userial##port##_puts,        \
>> -}
>> -
>> -DECLARE_ESERIAL_FUNCTIONS(0);
>> -struct serial_device uartlite_serial0_device =
>> -    INIT_ESERIAL_STRUCTURE(0, "ttyUL0");
>> -DECLARE_ESERIAL_FUNCTIONS(1);
>> -struct serial_device uartlite_serial1_device =
>> -    INIT_ESERIAL_STRUCTURE(1, "ttyUL1");
>> -DECLARE_ESERIAL_FUNCTIONS(2);
>> -struct serial_device uartlite_serial2_device =
>> -    INIT_ESERIAL_STRUCTURE(2, "ttyUL2");
>> -DECLARE_ESERIAL_FUNCTIONS(3);
>> -struct serial_device uartlite_serial3_device =
>> -    INIT_ESERIAL_STRUCTURE(3, "ttyUL3");
>> -
>> -__weak struct serial_device *default_serial_console(void)
>> +static int uartlite_serial_ofdata_to_platdata(struct udevice *dev)
>>   {
>> -    if (userial_ports[0])
>> -        return &uartlite_serial0_device;
>> -    if (userial_ports[1])
>> -        return &uartlite_serial1_device;
>> -    if (userial_ports[2])
>> -        return &uartlite_serial2_device;
>> -    if (userial_ports[3])
>> -        return &uartlite_serial3_device;
>> -
>> -    return NULL;
>> -}
>> +    struct uartlite_priv *priv = dev_get_priv(dev);
> 
> Since the conversion is "ofdatta to platdata", this should be platdata
> rather than priv. If there is not variable part, you may simply use
> platdata in the ops. In some cases, you may add device from platdata
> without ofdata.

look below.

> 
>> +    fdt_addr_t addr;
>>
>> -void uartlite_serial_initialize(void)
>> -{
>> -#ifdef XILINX_UARTLITE_BASEADDR
>> -    serial_register(&uartlite_serial0_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR */
>> -#ifdef XILINX_UARTLITE_BASEADDR1
>> -    serial_register(&uartlite_serial1_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR1 */
>> -#ifdef XILINX_UARTLITE_BASEADDR2
>> -    serial_register(&uartlite_serial2_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR2 */
>> -#ifdef XILINX_UARTLITE_BASEADDR3
>> -    serial_register(&uartlite_serial3_device);
>> -#endif /* XILINX_UARTLITE_BASEADDR3 */
>> +    addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
> 
> Use dev_get_addr() is preferred.

Will fix thanks.


> 
>> +    if (addr == FDT_ADDR_T_NONE)
>> +        return -EINVAL;
>> +
>> +    priv->regs = (struct uartlite *)addr;
>> +
>> +    return 0;
>>   }
>> +
>> +static const struct dm_serial_ops uartlite_serial_ops = {
>> +    .putc = uartlite_serial_putc,
>> +    .pending = uartlite_serial_pending,
>> +    .getc = uartlite_serial_getc,
>> +};
>> +
>> +static const struct udevice_id uartlite_serial_ids[] = {
>> +    { .compatible = "xlnx,xps-uartlite-1.00.a" },
>> +    { }
>> +};
>> +
> 
> Please add binding to doc/device-tree-bindings/serial/ .

ok.

> 
>> +U_BOOT_DRIVER(serial_uartlite) = {
>> +    .name    = "serial_uartlite",
>> +    .id    = UCLASS_SERIAL,
>> +    .of_match = uartlite_serial_ids,
>> +    .ofdata_to_platdata = uartlite_serial_ofdata_to_platdata,
>> +    .priv_auto_alloc_size = sizeof(struct uartlite_priv),
> 
> maybe platdata.

no problem with it.

Thanks,
Michal

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Thomas Chou]

Hi Michal,

On 2015?12?14? 23:50, Michal Simek wrote:
> Hi,
>
> On 14.12.2015 14:14, Thomas Chou wrote:
>> Hi Michal,
>>
>> On 2015?12?11? 19:54, Michal Simek wrote:
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>    drivers/serial/Kconfig            |  7 +++++++
>>>    drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>>>    2 files changed, 30 insertions(+)
>>>
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index 1fc287ee98ec..f1e221799b81 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -92,6 +92,13 @@ config DEBUG_UART_S5P
>>>          will need to provide parameters to make this work. The driver
>>> will
>>>          be available until the real driver-model serial is running.
>>>
>>> +config DEBUG_UART_UARTLITE
>>> +    bool "Xilinx Uartlite"
>>> +    help
>>> +      Select this to enable a debug UART using the serial_uartlite
>>> driver.
>>> +      You will need to provide parameters to make this work. The
>>> driver will
>>> +      be available until the real driver-model serial is running.
>>> +
>>>    config DEBUG_UART_ZYNQ
>>>        bool "Xilinx Zynq"
>>>        help
>>> diff --git a/drivers/serial/serial_xuartlite.c
>>> b/drivers/serial/serial_xuartlite.c
>>> index 10089f5a34b5..fe87b515d902 100644
>>> --- a/drivers/serial/serial_xuartlite.c
>>> +++ b/drivers/serial/serial_xuartlite.c
>>> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>>>        .ops    = &uartlite_serial_ops,
>>>        .flags = DM_FLAG_PRE_RELOC,
>>>    };
>>> +
>>> +#ifdef CONFIG_DEBUG_UART_UARTLITE
>>
>> Better move the "#include <debug_uart.h>" here.
>
> This is the patch I sent some days ago about removing it from this
> location and it was Reviewed twice.
> http://lists.denx.de/pipermail/u-boot/2015-December/236341.html
>
>>

This is because commit 42800ffa7997 ("arm: zynq: Move serial driver to 
driver model") added the extra #include <debug_uart.h>. It is this one 
should be removed. The original one in commit c54c0a4c1c74 ("arm: zynq: 
Support the debug UART") is good. The wrong one was removed.


>>> +void _debug_uart_init(void)
>>
>> Please add "static inline" to void _debug_uart_init(void).
>
> Ok. Will fix this for uartlite and zynq uart in follow up patch.
>
>
>>> +{
>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>> +
>>> +    out_be32(&regs->control, 0);
>>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>>> ULITE_CONTROL_RST_TX);
>>> +    in_be32(&regs->control);
>>> +}
>>> +
>>> +static inline void _debug_uart_putc(int ch)
>>> +{
>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>> +
>>> +    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>>> +        WATCHDOG_RESET();
>>
>> WATCHDOG_RESET() is not really needed for debug serial output.
>
> TBH I don't think so. There could be watchdog running from early
> bootloader and needs to be service even for debugging purpose.
>

Unless the serial input clock is dead, the serial shift out in UART 
won't take so long to trigger watchdog.

Best regards,
Thomas

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Thomas Chou]

Hi Michal,

On 2015?12?14? 23:42, Michal Simek wrote:
> On 14.12.2015 15:35, Thomas Chou wrote:
>> Hi Michal,
>>
>> On 2015?12?11? 19:54, Michal Simek wrote:
>>> - Move config option out of board file.
>>> - Remove uartlite address from config file
>>>
>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>> ---
>>>
>>>    board/xilinx/microblaze-generic/xparameters.h | 4 ----
>>>    configs/microblaze-generic_defconfig          | 1 +
>>>    drivers/serial/Kconfig                        | 7 +++++++
>>>    include/configs/microblaze-generic.h          | 7 +------
>>>    4 files changed, 9 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/board/xilinx/microblaze-generic/xparameters.h
>>> b/board/xilinx/microblaze-generic/xparameters.h
>>> index 8ba146cb88db..11b3c9a4846e 100644
>>> --- a/board/xilinx/microblaze-generic/xparameters.h
>>> +++ b/board/xilinx/microblaze-generic/xparameters.h
>>> @@ -28,10 +28,6 @@
>>>    #define XILINX_TIMER_BASEADDR    0x41c00000
>>>    #define XILINX_TIMER_IRQ    0
>>>
>>> -/* Uart pheriphery is RS232_Uart */
>>> -#define XILINX_UARTLITE_BASEADDR    0x40600000
>>> -#define XILINX_UARTLITE_BAUDRATE    115200
>>> -
>>>    /* IIC pheriphery is IIC_EEPROM */
>>>    #define XILINX_IIC_0_BASEADDR    0x40800000
>>>    #define XILINX_IIC_0_FREQ    100000
>>> diff --git a/configs/microblaze-generic_defconfig
>>> b/configs/microblaze-generic_defconfig
>>> index 5df080b6a87c..9a7bb915466f 100644
>>> --- a/configs/microblaze-generic_defconfig
>>> +++ b/configs/microblaze-generic_defconfig
>>> @@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
>>>    # CONFIG_CMD_SETEXPR is not set
>>>    CONFIG_SPL_OF_CONTROL=y
>>>    CONFIG_OF_EMBED=y
>>> +CONFIG_XILINX_UARTLITE=y
>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>> index f1e221799b81..ddf49ba9cef3 100644
>>> --- a/drivers/serial/Kconfig
>>> +++ b/drivers/serial/Kconfig
>>> @@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
>>>          If you have a UniPhier based board and want to use the on-chip
>>>          serial ports, say Y to this option. If unsure, say N.
>>>
>>> +config XILINX_UARTLITE
>>> +    bool "Xilinx Uarlite support"
>>> +    depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
>>> +    help
>>> +      If you have a Xilinx based board and want to use the uartlite
>>> +      serial ports, say Y to this option. If unsure, say N.
>>> +
>>>    endmenu
>>
>> Kconfig should be with the driver 1/3. Others are boards related.
>
> It can be. Adding Kconfig fragment can be separate patch out of move to
> DM. There is no connection too.
> My intention was to show all related changes which are done by this one
> step.

Please also make sure the patch series are bisectable.

Best regards,
Thomas

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Michal Simek]

Hi,

On 15.12.2015 05:01, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015?12?14? 23:50, Michal Simek wrote:
>> Hi,
>>
>> On 14.12.2015 14:14, Thomas Chou wrote:
>>> Hi Michal,
>>>
>>> On 2015?12?11? 19:54, Michal Simek wrote:
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>>    drivers/serial/Kconfig            |  7 +++++++
>>>>    drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>>>>    2 files changed, 30 insertions(+)
>>>>
>>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>>> index 1fc287ee98ec..f1e221799b81 100644
>>>> --- a/drivers/serial/Kconfig
>>>> +++ b/drivers/serial/Kconfig
>>>> @@ -92,6 +92,13 @@ config DEBUG_UART_S5P
>>>>          will need to provide parameters to make this work. The driver
>>>> will
>>>>          be available until the real driver-model serial is running.
>>>>
>>>> +config DEBUG_UART_UARTLITE
>>>> +    bool "Xilinx Uartlite"
>>>> +    help
>>>> +      Select this to enable a debug UART using the serial_uartlite
>>>> driver.
>>>> +      You will need to provide parameters to make this work. The
>>>> driver will
>>>> +      be available until the real driver-model serial is running.
>>>> +
>>>>    config DEBUG_UART_ZYNQ
>>>>        bool "Xilinx Zynq"
>>>>        help
>>>> diff --git a/drivers/serial/serial_xuartlite.c
>>>> b/drivers/serial/serial_xuartlite.c
>>>> index 10089f5a34b5..fe87b515d902 100644
>>>> --- a/drivers/serial/serial_xuartlite.c
>>>> +++ b/drivers/serial/serial_xuartlite.c
>>>> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>>>>        .ops    = &uartlite_serial_ops,
>>>>        .flags = DM_FLAG_PRE_RELOC,
>>>>    };
>>>> +
>>>> +#ifdef CONFIG_DEBUG_UART_UARTLITE
>>>
>>> Better move the "#include <debug_uart.h>" here.
>>
>> This is the patch I sent some days ago about removing it from this
>> location and it was Reviewed twice.
>> http://lists.denx.de/pipermail/u-boot/2015-December/236341.html
>>
>>>
> 
> This is because commit 42800ffa7997 ("arm: zynq: Move serial driver to
> driver model") added the extra #include <debug_uart.h>. It is this one
> should be removed. The original one in commit c54c0a4c1c74 ("arm: zynq:
> Support the debug UART") is good. The wrong one was removed.

I tend to have headers in the same location for the whole file
It will be easier to include this file when CONFIG_DEBUG_UART is enabled.
But no problem to move it. It is not causing any difference for me.


>>>> +void _debug_uart_init(void)
>>>
>>> Please add "static inline" to void _debug_uart_init(void).
>>
>> Ok. Will fix this for uartlite and zynq uart in follow up patch.
>>
>>
>>>> +{
>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>> +
>>>> +    out_be32(&regs->control, 0);
>>>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>>>> ULITE_CONTROL_RST_TX);
>>>> +    in_be32(&regs->control);
>>>> +}
>>>> +
>>>> +static inline void _debug_uart_putc(int ch)
>>>> +{
>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>> +
>>>> +    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>>>> +        WATCHDOG_RESET();
>>>
>>> WATCHDOG_RESET() is not really needed for debug serial output.
>>
>> TBH I don't think so. There could be watchdog running from early
>> bootloader and needs to be service even for debugging purpose.
>>
> 
> Unless the serial input clock is dead, the serial shift out in UART
> won't take so long to trigger watchdog.

Uartlite driver is also the part of MDM which is console over jtag which
can take some time.
But TBH if you like, I will remove it in v2.

Thanks,
Michal

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Michal Simek]

On 15.12.2015 05:02, Thomas Chou wrote:
> Hi Michal,
> 
> On 2015?12?14? 23:42, Michal Simek wrote:
>> On 14.12.2015 15:35, Thomas Chou wrote:
>>> Hi Michal,
>>>
>>> On 2015?12?11? 19:54, Michal Simek wrote:
>>>> - Move config option out of board file.
>>>> - Remove uartlite address from config file
>>>>
>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>> ---
>>>>
>>>>    board/xilinx/microblaze-generic/xparameters.h | 4 ----
>>>>    configs/microblaze-generic_defconfig          | 1 +
>>>>    drivers/serial/Kconfig                        | 7 +++++++
>>>>    include/configs/microblaze-generic.h          | 7 +------
>>>>    4 files changed, 9 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/board/xilinx/microblaze-generic/xparameters.h
>>>> b/board/xilinx/microblaze-generic/xparameters.h
>>>> index 8ba146cb88db..11b3c9a4846e 100644
>>>> --- a/board/xilinx/microblaze-generic/xparameters.h
>>>> +++ b/board/xilinx/microblaze-generic/xparameters.h
>>>> @@ -28,10 +28,6 @@
>>>>    #define XILINX_TIMER_BASEADDR    0x41c00000
>>>>    #define XILINX_TIMER_IRQ    0
>>>>
>>>> -/* Uart pheriphery is RS232_Uart */
>>>> -#define XILINX_UARTLITE_BASEADDR    0x40600000
>>>> -#define XILINX_UARTLITE_BAUDRATE    115200
>>>> -
>>>>    /* IIC pheriphery is IIC_EEPROM */
>>>>    #define XILINX_IIC_0_BASEADDR    0x40800000
>>>>    #define XILINX_IIC_0_FREQ    100000
>>>> diff --git a/configs/microblaze-generic_defconfig
>>>> b/configs/microblaze-generic_defconfig
>>>> index 5df080b6a87c..9a7bb915466f 100644
>>>> --- a/configs/microblaze-generic_defconfig
>>>> +++ b/configs/microblaze-generic_defconfig
>>>> @@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
>>>>    # CONFIG_CMD_SETEXPR is not set
>>>>    CONFIG_SPL_OF_CONTROL=y
>>>>    CONFIG_OF_EMBED=y
>>>> +CONFIG_XILINX_UARTLITE=y
>>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>>> index f1e221799b81..ddf49ba9cef3 100644
>>>> --- a/drivers/serial/Kconfig
>>>> +++ b/drivers/serial/Kconfig
>>>> @@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
>>>>          If you have a UniPhier based board and want to use the on-chip
>>>>          serial ports, say Y to this option. If unsure, say N.
>>>>
>>>> +config XILINX_UARTLITE
>>>> +    bool "Xilinx Uarlite support"
>>>> +    depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
>>>> +    help
>>>> +      If you have a Xilinx based board and want to use the uartlite
>>>> +      serial ports, say Y to this option. If unsure, say N.
>>>> +
>>>>    endmenu
>>>
>>> Kconfig should be with the driver 1/3. Others are boards related.
>>
>> It can be. Adding Kconfig fragment can be separate patch out of move to
>> DM. There is no connection too.
>> My intention was to show all related changes which are done by this one
>> step.
> 
> Please also make sure the patch series are bisectable.

I am not aware about anything what would caused that this series is not
bisectable. Do you see something like that?

Thanks,
Michal

[U-Boot] [PATCH 3/3] serial: uartlite: Add uartlite to Kconfig

[Thomas Chou]

Hi Michal,

On 2015?12?15? 23:39, Michal Simek wrote:
> On 15.12.2015 05:02, Thomas Chou wrote:
>> Hi Michal,
>>
>> On 2015?12?14? 23:42, Michal Simek wrote:
>>> On 14.12.2015 15:35, Thomas Chou wrote:
>>>> Hi Michal,
>>>>
>>>> On 2015?12?11? 19:54, Michal Simek wrote:
>>>>> - Move config option out of board file.
>>>>> - Remove uartlite address from config file
>>>>>
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>>     board/xilinx/microblaze-generic/xparameters.h | 4 ----
>>>>>     configs/microblaze-generic_defconfig          | 1 +
>>>>>     drivers/serial/Kconfig                        | 7 +++++++
>>>>>     include/configs/microblaze-generic.h          | 7 +------
>>>>>     4 files changed, 9 insertions(+), 10 deletions(-)
>>>>>
>>>>> diff --git a/board/xilinx/microblaze-generic/xparameters.h
>>>>> b/board/xilinx/microblaze-generic/xparameters.h
>>>>> index 8ba146cb88db..11b3c9a4846e 100644
>>>>> --- a/board/xilinx/microblaze-generic/xparameters.h
>>>>> +++ b/board/xilinx/microblaze-generic/xparameters.h
>>>>> @@ -28,10 +28,6 @@
>>>>>     #define XILINX_TIMER_BASEADDR    0x41c00000
>>>>>     #define XILINX_TIMER_IRQ    0
>>>>>
>>>>> -/* Uart pheriphery is RS232_Uart */
>>>>> -#define XILINX_UARTLITE_BASEADDR    0x40600000
>>>>> -#define XILINX_UARTLITE_BAUDRATE    115200
>>>>> -
>>>>>     /* IIC pheriphery is IIC_EEPROM */
>>>>>     #define XILINX_IIC_0_BASEADDR    0x40800000
>>>>>     #define XILINX_IIC_0_FREQ    100000
>>>>> diff --git a/configs/microblaze-generic_defconfig
>>>>> b/configs/microblaze-generic_defconfig
>>>>> index 5df080b6a87c..9a7bb915466f 100644
>>>>> --- a/configs/microblaze-generic_defconfig
>>>>> +++ b/configs/microblaze-generic_defconfig
>>>>> @@ -9,3 +9,4 @@ CONFIG_CMD_GPIO=y
>>>>>     # CONFIG_CMD_SETEXPR is not set
>>>>>     CONFIG_SPL_OF_CONTROL=y
>>>>>     CONFIG_OF_EMBED=y
>>>>> +CONFIG_XILINX_UARTLITE=y
>>>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>>>> index f1e221799b81..ddf49ba9cef3 100644
>>>>> --- a/drivers/serial/Kconfig
>>>>> +++ b/drivers/serial/Kconfig
>>>>> @@ -230,4 +230,11 @@ config UNIPHIER_SERIAL
>>>>>           If you have a UniPhier based board and want to use the on-chip
>>>>>           serial ports, say Y to this option. If unsure, say N.
>>>>>
>>>>> +config XILINX_UARTLITE
>>>>> +    bool "Xilinx Uarlite support"
>>>>> +    depends on DM_SERIAL && (MICROBLAZE || ARCH_ZYNQ || ARCH_ZYNQMP)
>>>>> +    help
>>>>> +      If you have a Xilinx based board and want to use the uartlite
>>>>> +      serial ports, say Y to this option. If unsure, say N.
>>>>> +
>>>>>     endmenu
>>>>
>>>> Kconfig should be with the driver 1/3. Others are boards related.
>>>
>>> It can be. Adding Kconfig fragment can be separate patch out of move to
>>> DM. There is no connection too.
>>> My intention was to show all related changes which are done by this one
>>> step.
>>
>> Please also make sure the patch series are bisectable.
>
> I am not aware about anything what would caused that this series is not
> bisectable. Do you see something like that?

No. Sorry. I didn't find DM and DM_serial in the microblaze repo at 
first. Then I found them in the patches earlier.

Best regards,
Thomas

[U-Boot] [PATCH 2/3] serial: uartlite: Add support for debug console

[Thomas Chou]

Hi Michal,

On 2015?12?15? 23:38, Michal Simek wrote:
> Hi,
>
> On 15.12.2015 05:01, Thomas Chou wrote:
>> Hi Michal,
>>
>> On 2015?12?14? 23:50, Michal Simek wrote:
>>> Hi,
>>>
>>> On 14.12.2015 14:14, Thomas Chou wrote:
>>>> Hi Michal,
>>>>
>>>> On 2015?12?11? 19:54, Michal Simek wrote:
>>>>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>>>>> ---
>>>>>
>>>>>     drivers/serial/Kconfig            |  7 +++++++
>>>>>     drivers/serial/serial_xuartlite.c | 23 +++++++++++++++++++++++
>>>>>     2 files changed, 30 insertions(+)
>>>>>
>>>>> diff --git a/drivers/serial/Kconfig b/drivers/serial/Kconfig
>>>>> index 1fc287ee98ec..f1e221799b81 100644
>>>>> --- a/drivers/serial/Kconfig
>>>>> +++ b/drivers/serial/Kconfig
>>>>> @@ -92,6 +92,13 @@ config DEBUG_UART_S5P
>>>>>           will need to provide parameters to make this work. The driver
>>>>> will
>>>>>           be available until the real driver-model serial is running.
>>>>>
>>>>> +config DEBUG_UART_UARTLITE
>>>>> +    bool "Xilinx Uartlite"
>>>>> +    help
>>>>> +      Select this to enable a debug UART using the serial_uartlite
>>>>> driver.
>>>>> +      You will need to provide parameters to make this work. The
>>>>> driver will
>>>>> +      be available until the real driver-model serial is running.
>>>>> +
>>>>>     config DEBUG_UART_ZYNQ
>>>>>         bool "Xilinx Zynq"
>>>>>         help
>>>>> diff --git a/drivers/serial/serial_xuartlite.c
>>>>> b/drivers/serial/serial_xuartlite.c
>>>>> index 10089f5a34b5..fe87b515d902 100644
>>>>> --- a/drivers/serial/serial_xuartlite.c
>>>>> +++ b/drivers/serial/serial_xuartlite.c
>>>>> @@ -120,3 +120,26 @@ U_BOOT_DRIVER(serial_uartlite) = {
>>>>>         .ops    = &uartlite_serial_ops,
>>>>>         .flags = DM_FLAG_PRE_RELOC,
>>>>>     };
>>>>> +
>>>>> +#ifdef CONFIG_DEBUG_UART_UARTLITE
>>>>
>>>> Better move the "#include <debug_uart.h>" here.
>>>
>>> This is the patch I sent some days ago about removing it from this
>>> location and it was Reviewed twice.
>>> http://lists.denx.de/pipermail/u-boot/2015-December/236341.html
>>>
>>>>
>>
>> This is because commit 42800ffa7997 ("arm: zynq: Move serial driver to
>> driver model") added the extra #include <debug_uart.h>. It is this one
>> should be removed. The original one in commit c54c0a4c1c74 ("arm: zynq:
>> Support the debug UART") is good. The wrong one was removed.
>
> I tend to have headers in the same location for the whole file
> It will be easier to include this file when CONFIG_DEBUG_UART is enabled.
> But no problem to move it. It is not causing any difference for me.
>
>
>>>>> +void _debug_uart_init(void)
>>>>
>>>> Please add "static inline" to void _debug_uart_init(void).
>>>
>>> Ok. Will fix this for uartlite and zynq uart in follow up patch.
>>>
>>>
>>>>> +{
>>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>>> +
>>>>> +    out_be32(&regs->control, 0);
>>>>> +    out_be32(&regs->control, ULITE_CONTROL_RST_RX |
>>>>> ULITE_CONTROL_RST_TX);
>>>>> +    in_be32(&regs->control);
>>>>> +}
>>>>> +
>>>>> +static inline void _debug_uart_putc(int ch)
>>>>> +{
>>>>> +    struct uartlite *regs = (struct uartlite *)CONFIG_DEBUG_UART_BASE;
>>>>> +
>>>>> +    while (in_be32(&regs->status) & SR_TX_FIFO_FULL)
>>>>> +        WATCHDOG_RESET();
>>>>
>>>> WATCHDOG_RESET() is not really needed for debug serial output.
>>>
>>> TBH I don't think so. There could be watchdog running from early
>>> bootloader and needs to be service even for debugging purpose.
>>>
>>
>> Unless the serial input clock is dead, the serial shift out in UART
>> won't take so long to trigger watchdog.
>
> Uartlite driver is also the part of MDM which is console over jtag which
> can take some time.
> But TBH if you like, I will remove it in v2.
>

Now I understand. Either is fine. Thanks for the explanation.

Best regards,
Thomas