[U-Boot] [PATCH v2 0/9] Introduce driver model serial uclass

[U-Boot] [PATCH v2 0/9] Introduce driver model serial uclass

[Simon Glass]

This series adds support for a serial uclass, enabling serial drivers to be
converted to use driver model.

Unfortunately this is quite a complicated process for a number of reasons:

- serial is used before relocation, but driver model does not support this
- stdio member functions are not passed a device pointer, but driver model
    requires this (so does serial, but it uses an ugly work-around)
- driver model requires malloc() but this is not available before
  relocation
- for sandbox, if something goes wrong with the console, we still need to
  get an error message out through the fallback console

So this series relies on quite a few patches to address the above, as well
as the serial uclass and an implementation for sandbox.

If you have limited time, please take a look at least at the uclass patch
which is 'dm: Add a uclass for serial devices' (see include/serial.h).

Note: this series breaks 4 exynos boards. I will figure out how to get them
to build as part of the exynos5-dt common board work.

To see the current state of driver model, look at u-boot-dm.git branch
'working'.

Changes in v2:
- Add exynos serial support
- Remove RFC status
- Rename struct device to struct udevice
- Split out core driver model patches into a separate set

Simon Glass (9):
  serial: Set up the 'priv' pointer when creating a serial device
  dm: Add a uclass for serial devices
  Set up stdio earlier when using driver model
  sandbox: Convert serial driver to use driver model
  sandbox: serial: Support a coloured console
  sandbox: dts: Add a serial console node
  dm: exynos: Mark exynos5 console as pre-reloc
  dm: exynos: Move serial to driver model
  dm: Make driver model available before board_init()

 arch/arm/dts/exynos5.dtsi      |   1 +
 arch/sandbox/dts/sandbox.dts   |  10 ++
 common/board_r.c               |  19 +--
 common/stdio.c                 |  18 ++-
 drivers/serial/Makefile        |   4 +
 drivers/serial/sandbox.c       | 140 +++++++++++++++----
 drivers/serial/serial-uclass.c | 197 +++++++++++++++++++++++++++
 drivers/serial/serial.c        |   1 +
 drivers/serial/serial_s5p.c    | 295 +++++++++++++----------------------------
 include/configs/exynos5-dt.h   |   1 +
 include/configs/sandbox.h      |   3 +
 include/dm/uclass-id.h         |   1 +
 include/serial.h               |  92 +++++++++++++
 include/stdio_dev.h            |  24 +++-
 14 files changed, 566 insertions(+), 240 deletions(-)
 create mode 100644 drivers/serial/serial-uclass.c

-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 1/9] serial: Set up the 'priv' pointer when creating a serial device

[Simon Glass]

The stdio_dev structure has a private pointer for its creater, but it is
not set up the serial system. Set it to point to the serial device so
that it can be found by code called by stdio.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 drivers/serial/serial.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/serial/serial.c b/drivers/serial/serial.c
index d2eb752..bbe60af 100644
--- a/drivers/serial/serial.c
+++ b/drivers/serial/serial.c
@@ -320,6 +320,7 @@ void serial_stdio_init(void)
 		dev.puts = serial_stub_puts;
 		dev.getc = serial_stub_getc;
 		dev.tstc = serial_stub_tstc;
+		dev.priv = s;
 
 		stdio_register(&dev);
 
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 2/9] dm: Add a uclass for serial devices

[Simon Glass]

Serial devices support simple byte input/output and a few operations to find
out whether data is available. Add a basic uclass for serial devices to be
used by drivers that are converted to driver model.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Rename struct device to struct udevice

 drivers/serial/Makefile        |   4 +
 drivers/serial/serial-uclass.c | 197 +++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h         |   1 +
 include/serial.h               |  92 +++++++++++++++++++
 4 files changed, 294 insertions(+)
 create mode 100644 drivers/serial/serial-uclass.c

diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 571c18f..4720e1d 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -5,7 +5,11 @@
 # SPDX-License-Identifier:	GPL-2.0+
 #
 
+ifdef CONFIG_DM_SERIAL
+obj-y += serial-uclass.o
+else
 obj-y += serial.o
+endif
 
 obj-$(CONFIG_ALTERA_UART) += altera_uart.o
 obj-$(CONFIG_ALTERA_JTAG_UART) += altera_jtag_uart.o
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
new file mode 100644
index 0000000..c7439f3
--- /dev/null
+++ b/drivers/serial/serial-uclass.c
@@ -0,0 +1,197 @@
+/*
+ * Copyright (c) 2014 The Chromium OS Authors.
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
+#include <os.h>
+#include <serial.h>
+#include <stdio_dev.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+/* The currently-selected console serial device */
+struct udevice *cur_dev __attribute__ ((section(".data")));
+
+#ifndef CONFIG_SYS_MALLOC_F_LEN
+#error "Serial is required before relocation - define CONFIG_SYS_MALLOC_F_LEN to make this work"
+#endif
+
+static void serial_find_console_or_panic(void)
+{
+	/* Check for a console alias */
+	if (!uclass_get_device_by_of_offset(UCLASS_SERIAL,
+			fdtdec_get_alias_node(gd->fdt_blob, "console"),
+			&cur_dev))
+		return;
+
+	/*
+	 * Failing that, get the device with sequence number 0, or in extremis
+	 * just the first serial device we can find. But we insist on having
+	 * a console (even if it is silent).
+	 */
+	if (uclass_get_device_by_seq(UCLASS_SERIAL, 0, &cur_dev) &&
+	    (uclass_first_device(UCLASS_SERIAL, &cur_dev) || !cur_dev))
+		panic("No serial driver found");
+}
+
+/* Called prior to relocation */
+int serial_init(void)
+{
+	serial_find_console_or_panic();
+	gd->flags |= GD_FLG_SERIAL_READY;
+
+	return 0;
+}
+
+/* Called after relocation */
+void serial_initialize(void)
+{
+	serial_find_console_or_panic();
+}
+
+void serial_putc(char ch)
+{
+	struct dm_serial_ops *ops = serial_get_ops(cur_dev);
+	int err;
+
+	do {
+		err = ops->putc(cur_dev, ch);
+	} while (err == -EAGAIN);
+	if (ch == '\n')
+		serial_putc('\r');
+}
+
+void serial_setbrg(void)
+{
+	struct dm_serial_ops *ops = serial_get_ops(cur_dev);
+
+	if (ops->setbrg)
+		ops->setbrg(cur_dev, gd->baudrate);
+}
+
+void serial_puts(const char *str)
+{
+	while (*str)
+		serial_putc(*str++);
+}
+
+int serial_tstc(void)
+{
+	struct dm_serial_ops *ops = serial_get_ops(cur_dev);
+
+	if (ops->pending)
+		return ops->pending(cur_dev, true);
+
+	return 1;
+}
+
+int serial_getc(void)
+{
+	struct dm_serial_ops *ops = serial_get_ops(cur_dev);
+	int err;
+
+	do {
+		err = ops->getc(cur_dev);
+	} while (err == -EAGAIN);
+
+	return err >= 0 ? err : 0;
+}
+
+void serial_stdio_init(void)
+{
+}
+
+void serial_stub_putc(struct stdio_dev *sdev, const char ch)
+{
+	struct udevice *dev = sdev->priv;
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+
+	ops->putc(dev, ch);
+}
+
+void serial_stub_puts(struct stdio_dev *sdev, const char *str)
+{
+	while (*str)
+		serial_stub_putc(sdev, *str++);
+}
+
+int serial_stub_getc(struct stdio_dev *sdev)
+{
+	struct udevice *dev = sdev->priv;
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+
+	int err;
+
+	do {
+		err = ops->getc(dev);
+	} while (err == -EAGAIN);
+
+	return err >= 0 ? err : 0;
+}
+
+int serial_stub_tstc(struct stdio_dev *sdev)
+{
+	struct udevice *dev = sdev->priv;
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+
+	if (ops->pending)
+		return ops->pending(dev, true);
+
+	return 1;
+}
+
+static int serial_post_probe(struct udevice *dev)
+{
+	struct stdio_dev sdev;
+	struct dm_serial_ops *ops = serial_get_ops(dev);
+	struct serial_dev_priv *upriv = dev->uclass_priv;
+	int ret;
+
+	/* Set the baud rate */
+	if (ops->setbrg) {
+		ret = ops->setbrg(dev, gd->baudrate);
+		if (ret)
+			return ret;
+	}
+
+	if (!(gd->flags & GD_FLG_RELOC))
+		return 0;
+
+	memset(&sdev, '\0', sizeof(sdev));
+
+	strncpy(sdev.name, dev->name, sizeof(sdev.name));
+	sdev.flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT;
+	sdev.priv = dev;
+	sdev.putc = serial_stub_putc;
+	sdev.puts = serial_stub_puts;
+	sdev.getc = serial_stub_getc;
+	sdev.tstc = serial_stub_tstc;
+	stdio_register_dev(&sdev, &upriv->sdev);
+
+	return 0;
+}
+
+static int serial_pre_remove(struct udevice *dev)
+{
+#ifdef CONFIG_SYS_STDIO_DEREGISTER
+	struct serial_dev_priv *upriv = dev->uclass_priv;
+
+	if (stdio_deregister_dev(upriv->sdev))
+		return -EPERM;
+#endif
+
+	return 0;
+}
+
+UCLASS_DRIVER(serial) = {
+	.id		= UCLASS_SERIAL,
+	.name		= "serial",
+	.post_probe	= serial_post_probe,
+	.pre_remove	= serial_pre_remove,
+	.per_device_auto_alloc_size = sizeof(struct serial_dev_priv),
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index dd95fca..7f0e37b 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -21,6 +21,7 @@ enum uclass_id {
 
 	/* U-Boot uclasses start here */
 	UCLASS_GPIO,		/* Bank of general-purpose I/O pins */
+	UCLASS_SERIAL,		/* Serial UART */
 
 	UCLASS_COUNT,
 	UCLASS_INVALID = -1,
diff --git a/include/serial.h b/include/serial.h
index d232d47..8f574e4 100644
--- a/include/serial.h
+++ b/include/serial.h
@@ -72,4 +72,96 @@ extern int write_port(struct stdio_dev *port, char *buf);
 extern int read_port(struct stdio_dev *port, char *buf, int size);
 #endif
 
+struct udevice;
+
+/**
+ * struct struct dm_serial_ops - Driver model serial operations
+ *
+ * The uclass interface is implemented by all serial devices which use
+ * driver model.
+ */
+struct dm_serial_ops {
+	/**
+	 * setbrg() - Set up the baud rate generator
+	 *
+	 * Adjust baud rate divisors to set up a new baud rate for this
+	 * device. Not all devices will support all rates. If the rate
+	 * cannot be supported, the driver is free to select the nearest
+	 * available rate. or return -EINVAL if this is not possible.
+	 *
+	 * @dev: Device pointer
+	 * @baudrate: New baud rate to use
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*setbrg)(struct udevice *dev, int baudrate);
+	/**
+	 * getc() - Read a character and return it
+	 *
+	 * If no character is available, this should return -EAGAIN without
+	 * waiting.
+	 *
+	 * @dev: Device pointer
+	 * @return character (0..255), -ve on error
+	 */
+	int (*getc)(struct udevice *dev);
+	/**
+	 * putc() - Write a character
+	 *
+	 * @dev: Device pointer
+	 * @ch: character to write
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*putc)(struct udevice *dev, const char ch);
+	/**
+	 * pending() - Check if input/output characters are waiting
+	 *
+	 * This can be used to return an indication of the number of waiting
+	 * characters if the driver knows this (e.g. by looking at the FIFO
+	 * level). It is acceptable to return 1 if an indeterminant number
+	 * of characters is waiting.
+	 *
+	 * This method is optional.
+	 *
+	 * @dev: Device pointer
+	 * @input: true to check input characters, false for output
+	 * @return number of waiting characters, 0 for none, -ve on error
+	 */
+	int (*pending)(struct udevice *dev, bool input);
+	/**
+	 * clear() - Clear the serial FIFOs/holding registers
+	 *
+	 * This method is optional.
+	 *
+	 * This quickly clears any input/output characters from the UART.
+	 * If this is not possible, but characters still exist, then it
+	 * is acceptable to return -EAGAIN (try again) or -EINVAL (not
+	 * supported).
+	 *
+	 * @dev: Device pointer
+	 * @return 0 if OK, -ve on error
+	 */
+	int (*clear)(struct udevice *dev);
+#if CONFIG_POST & CONFIG_SYS_POST_UART
+	/**
+	 * loop() - Control serial device loopback mode
+	 *
+	 * @dev: Device pointer
+	 * @on: 1 to turn loopback on, 0 to turn if off
+	 */
+	int (*loop)(struct udevice *dev, int on);
+#endif
+};
+
+/**
+ * struct serial_dev_priv - information about a device used by the uclass
+ *
+ * @sdev: stdio device attached to this uart
+ */
+struct serial_dev_priv {
+	struct stdio_dev *sdev;
+};
+
+/* Access the serial operations for a device */
+#define serial_get_ops(dev)	((struct dm_serial_ops *)(dev)->driver->ops)
+
 #endif
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 3/9] Set up stdio earlier when using driver model

[Simon Glass]

Since driver model registers itself with the stdio subsystem, and we
want to avoid delayed registration and other complexity associated with
the current serial console, move the stdio subsystem init earlier when
driver model is used for serial.

This simplifies the implementation. Should there be any problems with
this approach they can be dealt with as boards are converted over to
use driver model for serial.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 common/board_r.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index a35ffa5..6ea8bce 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -729,6 +729,15 @@ init_fnc_t init_sequence_r[] = {
 	set_cpu_clk_info, /* Setup clock information */
 #endif
 	initr_reloc_global_data,
+	initr_barrier,
+	initr_malloc,
+	bootstage_relocate,
+#ifdef CONFIG_DM_SERIAL
+	stdio_init,
+#endif
+#ifdef CONFIG_DM
+	initr_dm,
+#endif
 	initr_serial,
 	initr_announce,
 	INIT_FUNC_WATCHDOG_RESET
@@ -765,12 +774,6 @@ init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_WINBOND_83C553
 	initr_w83c553f,
 #endif
-	initr_barrier,
-	initr_malloc,
-	bootstage_relocate,
-#ifdef CONFIG_DM
-	initr_dm,
-#endif
 #ifdef CONFIG_ARCH_EARLY_INIT_R
 	arch_early_init_r,
 #endif
@@ -820,7 +823,9 @@ init_fnc_t init_sequence_r[] = {
 	 */
 	initr_pci,
 #endif
+#ifndef CONFIG_DM_SERIAL
 	stdio_init,
+#endif
 	initr_jumptable,
 #ifdef CONFIG_API
 	initr_api,
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 4/9] sandbox: Convert serial driver to use driver model

[Simon Glass]

Adjust the sandbox serial driver to use the new driver model uclass. The
driver works much as before, but within the new framework.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Rename struct device to struct udevice

 drivers/serial/sandbox.c  | 67 +++++++++++++++++++++++++----------------------
 include/configs/sandbox.h |  3 +++
 2 files changed, 39 insertions(+), 31 deletions(-)

diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index 51fd871..ac54e01 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -11,12 +11,16 @@
  */
 
 #include <common.h>
+#include <dm.h>
+#include <fdtdec.h>
 #include <lcd.h>
 #include <os.h>
 #include <serial.h>
 #include <linux/compiler.h>
 #include <asm/state.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 /*
  *
  *   serial_buf: A buffer that holds keyboard characters for the
@@ -30,27 +34,21 @@ static char serial_buf[16];
 static unsigned int serial_buf_write;
 static unsigned int serial_buf_read;
 
-static int sandbox_serial_init(void)
+static int sandbox_serial_probe(struct udevice *dev)
 {
 	struct sandbox_state *state = state_get_current();
 
 	if (state->term_raw != STATE_TERM_COOKED)
 		os_tty_raw(0, state->term_raw == STATE_TERM_RAW_WITH_SIGS);
-	return 0;
-}
 
-static void sandbox_serial_setbrg(void)
-{
+	return 0;
 }
 
-static void sandbox_serial_putc(const char ch)
+static int sandbox_serial_putc(struct udevice *dev, const char ch)
 {
 	os_write(1, &ch, 1);
-}
 
-static void sandbox_serial_puts(const char *str)
-{
-	os_write(1, str, strlen(str));
+	return 0;
 }
 
 static unsigned int increment_buffer_index(unsigned int index)
@@ -58,12 +56,15 @@ static unsigned int increment_buffer_index(unsigned int index)
 	return (index + 1) % ARRAY_SIZE(serial_buf);
 }
 
-static int sandbox_serial_tstc(void)
+static int sandbox_serial_pending(struct udevice *dev, bool input)
 {
 	const unsigned int next_index =
 		increment_buffer_index(serial_buf_write);
 	ssize_t count;
 
+	if (!input)
+		return 0;
+
 	os_usleep(100);
 #ifdef CONFIG_LCD
 	lcd_sync();
@@ -74,38 +75,42 @@ static int sandbox_serial_tstc(void)
 	count = os_read_no_block(0, &serial_buf[serial_buf_write], 1);
 	if (count == 1)
 		serial_buf_write = next_index;
+
 	return serial_buf_write != serial_buf_read;
 }
 
-static int sandbox_serial_getc(void)
+static int sandbox_serial_getc(struct udevice *dev)
 {
 	int result;
 
-	while (!sandbox_serial_tstc())
-		;	/* buffer empty */
+	if (!sandbox_serial_pending(dev, true))
+		return -EAGAIN;	/* buffer empty */
 
 	result = serial_buf[serial_buf_read];
 	serial_buf_read = increment_buffer_index(serial_buf_read);
 	return result;
 }
 
-static struct serial_device sandbox_serial_drv = {
-	.name	= "sandbox_serial",
-	.start	= sandbox_serial_init,
-	.stop	= NULL,
-	.setbrg	= sandbox_serial_setbrg,
-	.putc	= sandbox_serial_putc,
-	.puts	= sandbox_serial_puts,
-	.getc	= sandbox_serial_getc,
-	.tstc	= sandbox_serial_tstc,
+static const struct dm_serial_ops sandbox_serial_ops = {
+	.putc = sandbox_serial_putc,
+	.pending = sandbox_serial_pending,
+	.getc = sandbox_serial_getc,
 };
 
-void sandbox_serial_initialize(void)
-{
-	serial_register(&sandbox_serial_drv);
-}
+static const struct udevice_id sandbox_serial_ids[] = {
+	{ .compatible = "sandbox,serial" },
+	{ }
+};
 
-__weak struct serial_device *default_serial_console(void)
-{
-	return &sandbox_serial_drv;
-}
+U_BOOT_DRIVER(serial_sandbox) = {
+	.name	= "serial_sandbox",
+	.id	= UCLASS_SERIAL,
+	.of_match = sandbox_serial_ids,
+	.probe = sandbox_serial_probe,
+	.ops	= &sandbox_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
+
+U_BOOT_DEVICE(serial_sandbox_non_fdt) = {
+	.name = "serial_sandbox",
+};
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index bf2d25c..f5fa4b3 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -31,6 +31,9 @@
 #define CONFIG_DM_DEMO_SHAPE
 #define CONFIG_DM_GPIO
 #define CONFIG_DM_TEST
+#define CONFIG_DM_SERIAL
+
+#define CONFIG_SYS_STDIO_DEREGISTER
 
 /* Number of bits in a C 'long' on this architecture */
 #define CONFIG_SANDBOX_BITS_PER_LONG	64
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 5/9] sandbox: serial: Support a coloured console

[Simon Glass]

The current sandbox serial driver is a pretty trivial example and does not
have the featues that might be needed for other board serial drivers. To
help provide a better example, add a text colour property to the device
tree for sandbox. This uses platform data, a device tree node, driver
private data and a remove() method.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Rename struct device to struct udevice

 drivers/serial/sandbox.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index ac54e01..9139104 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -34,19 +34,67 @@ static char serial_buf[16];
 static unsigned int serial_buf_write;
 static unsigned int serial_buf_read;
 
+struct sandbox_serial_platdata {
+	int colour;	/* Text colour to use for output, -1 for none */
+};
+
+struct sandbox_serial_priv {
+	bool start_of_line;
+};
+
+/**
+ * output_ansi_colour() - Output an ANSI colour code
+ *
+ * @colour: Colour to output (0-7)
+ */
+static void output_ansi_colour(int colour)
+{
+	char ansi_code[] = "\x1b[1;3Xm";
+
+	ansi_code[5] = '0' + colour;
+	os_write(1, ansi_code, sizeof(ansi_code) - 1);
+}
+
+static void output_ansi_reset(void)
+{
+	os_write(1, "\x1b[0m", 4);
+}
+
 static int sandbox_serial_probe(struct udevice *dev)
 {
 	struct sandbox_state *state = state_get_current();
+	struct sandbox_serial_priv *priv = dev_get_priv(dev);
 
 	if (state->term_raw != STATE_TERM_COOKED)
 		os_tty_raw(0, state->term_raw == STATE_TERM_RAW_WITH_SIGS);
+	priv->start_of_line = 0;
+
+	return 0;
+}
+
+static int sandbox_serial_remove(struct udevice *dev)
+{
+	struct sandbox_serial_platdata *plat = dev->platdata;
+
+	if (plat->colour != -1)
+		output_ansi_reset();
 
 	return 0;
 }
 
 static int sandbox_serial_putc(struct udevice *dev, const char ch)
 {
+	struct sandbox_serial_priv *priv = dev_get_priv(dev);
+	struct sandbox_serial_platdata *plat = dev->platdata;
+
+	if (priv->start_of_line && plat->colour != -1) {
+		priv->start_of_line = false;
+		output_ansi_colour(plat->colour);
+	}
+
 	os_write(1, &ch, 1);
+	if (ch == '\n')
+		priv->start_of_line = true;
 
 	return 0;
 }
@@ -91,6 +139,32 @@ static int sandbox_serial_getc(struct udevice *dev)
 	return result;
 }
 
+static const char * const ansi_colour[] = {
+	"black", "red", "green", "yellow", "blue", "megenta", "cyan",
+	"white",
+};
+
+static int sandbox_serial_ofdata_to_platdata(struct udevice *dev)
+{
+	struct sandbox_serial_platdata *plat = dev->platdata;
+	const char *colour;
+	int i;
+
+	plat->colour = -1;
+	colour = fdt_getprop(gd->fdt_blob, dev->of_offset, "text-colour",
+			     NULL);
+	if (colour) {
+		for (i = 0; i < ARRAY_SIZE(ansi_colour); i++) {
+			if (!strcmp(colour, ansi_colour[i])) {
+				plat->colour = i;
+				break;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static const struct dm_serial_ops sandbox_serial_ops = {
 	.putc = sandbox_serial_putc,
 	.pending = sandbox_serial_pending,
@@ -106,11 +180,20 @@ U_BOOT_DRIVER(serial_sandbox) = {
 	.name	= "serial_sandbox",
 	.id	= UCLASS_SERIAL,
 	.of_match = sandbox_serial_ids,
+	.ofdata_to_platdata = sandbox_serial_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct sandbox_serial_platdata),
+	.priv_auto_alloc_size = sizeof(struct sandbox_serial_priv),
 	.probe = sandbox_serial_probe,
+	.remove = sandbox_serial_remove,
 	.ops	= &sandbox_serial_ops,
 	.flags = DM_FLAG_PRE_RELOC,
 };
 
+static const struct sandbox_serial_platdata platdata_non_fdt = {
+	.colour = -1,
+};
+
 U_BOOT_DEVICE(serial_sandbox_non_fdt) = {
 	.name = "serial_sandbox",
+	.platdata = &platdata_non_fdt,
 };
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 6/9] sandbox: dts: Add a serial console node

[Simon Glass]

If the sandbox device tree is provided to U-Boot (with the -d flag) then it
will use the device tree version in preference to the built-in device. The
only difference is the colour.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/sandbox/dts/sandbox.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index f3f951b..daf5e73 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -1,6 +1,16 @@
 /dts-v1/;
 
 / {
+	aliases {
+		console = "/serial";
+	};
+
+	uart0: serial {
+		compatible = "sandbox,serial";
+		dm,pre-reloc;
+		text-colour = "cyan";
+	};
+
 	triangle {
 		compatible = "demo-shape";
 		colour = "cyan";
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 7/9] dm: exynos: Mark exynos5 console as pre-reloc

[Simon Glass]

We will need the console before relocation, so mark it that way.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 arch/arm/dts/exynos5.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/exynos5.dtsi b/arch/arm/dts/exynos5.dtsi
index dc5405b..ed1af83 100644
--- a/arch/arm/dts/exynos5.dtsi
+++ b/arch/arm/dts/exynos5.dtsi
@@ -244,6 +244,7 @@
 		compatible = "samsung,exynos4210-uart";
 		reg = <0x12C30000 0x100>;
 		interrupts = <0 54 0>;
+		dm,pre-reloc;
 		id = <3>;
 	};
 
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 8/9] dm: exynos: Move serial to driver model

[Simon Glass]

Change the Exynos serial driver to work with driver model and switch over
all Exynos5 boards to use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2: None

 drivers/serial/serial_s5p.c  | 295 ++++++++++++++-----------------------------
 include/configs/exynos5-dt.h |   1 +
 2 files changed, 93 insertions(+), 203 deletions(-)

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
index 98c62b4..a2719e2 100644
--- a/drivers/serial/serial_s5p.c
+++ b/drivers/serial/serial_s5p.c
@@ -9,6 +9,8 @@
  */
 
 #include <common.h>
+#include <dm.h>
+#include <errno.h>
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
@@ -18,26 +20,18 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
+#define RX_FIFO_COUNT_SHIFT	0
+#define RX_FIFO_COUNT_MASK	(0xff << RX_FIFO_COUNT_SHIFT)
+#define RX_FIFO_FULL		(1 << 8)
+#define TX_FIFO_COUNT_SHIFT	16
+#define TX_FIFO_COUNT_MASK	(0xff << TX_FIFO_COUNT_SHIFT)
+#define TX_FIFO_FULL		(1 << 24)
 
 /* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory */
+struct s5p_serial_platdata {
+	struct s5p_uart *reg;  /* address of registers in physical memory */
 	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
-
-static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
-{
-#ifdef CONFIG_OF_CONTROL
-	return (struct s5p_uart *)(config.base_addr);
-#else
-	u32 offset = dev_index * sizeof(struct s5p_uart);
-	return (struct s5p_uart *)(samsung_get_base_uart() + offset);
-#endif
-}
+};
 
 /*
  * The coefficient, used to calculate the baudrate on S5P UARTs is
@@ -65,57 +59,8 @@ static const int udivslot[] = {
 	0xffdf,
 };
 
-static void serial_setbrg_dev(const int dev_index)
-{
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
-	u32 uclk = get_uart_clk(dev_index);
-	u32 baudrate = gd->baudrate;
-	u32 val;
-
-#if defined(CONFIG_SILENT_CONSOLE) && \
-		defined(CONFIG_OF_CONTROL) && \
-		!defined(CONFIG_SPL_BUILD)
-	if (fdtdec_get_config_int(gd->fdt_blob, "silent_console", 0))
-		gd->flags |= GD_FLG_SILENT;
-#endif
-
-	if (!config.enabled)
-		return;
-
-	val = uclk / baudrate;
-
-	writel(val / 16 - 1, &uart->ubrdiv);
-
-	if (s5p_uart_divslot())
-		writew(udivslot[val % 16], &uart->rest.slot);
-	else
-		writeb(val % 16, &uart->rest.value);
-}
-
-/*
- * Initialise the serial port with the given baudrate. The settings
- * are always 8 data bits, no parity, 1 stop bit, no start bits.
- */
-static int serial_init_dev(const int dev_index)
+static int serial_err_check(const struct s5p_uart *const uart, int op)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
-
-	/* enable FIFOs, auto clear Rx FIFO */
-	writel(0x3, &uart->ufcon);
-	writel(0, &uart->umcon);
-	/* 8N1 */
-	writel(0x3, &uart->ulcon);
-	/* No interrupts, no DMA, pure polling */
-	writel(0x245, &uart->ucon);
-
-	serial_setbrg_dev(dev_index);
-
-	return 0;
-}
-
-static int serial_err_check(const int dev_index, int op)
-{
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
 	unsigned int mask;
 
 	/*
@@ -133,169 +78,113 @@ static int serial_err_check(const int dev_index, int op)
 	return readl(&uart->uerstat) & mask;
 }
 
-/*
- * 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.
- */
-static int serial_getc_dev(const int dev_index)
+static int s5p_serial_putc(struct udevice *dev, const char ch)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	if (!config.enabled)
-		return 0;
+	if (readl(&uart->ufstat) & TX_FIFO_FULL)
+		return -EAGAIN;
 
-	/* wait for character to arrive */
-	while (!(readl(&uart->ufstat) & (RX_FIFO_COUNT_MASK |
-					 RX_FIFO_FULL_MASK))) {
-		if (serial_err_check(dev_index, 0))
-			return 0;
-	}
+	writeb(ch, &uart->utxh);
+	serial_err_check(uart, 1);
 
-	return (int)(readb(&uart->urxh) & 0xff);
+	return 0;
 }
 
-/*
- * Output a single byte to the serial port.
- */
-static void serial_putc_dev(const char c, const int dev_index)
+static int s5p_serial_pending(struct udevice *dev, bool input)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+	uint32_t ufstat = readl(&uart->ufstat);
 
-	if (!config.enabled)
-		return;
+	if (input)
+		return (ufstat & RX_FIFO_COUNT_MASK) >> RX_FIFO_COUNT_SHIFT;
+	else
+		return (ufstat & TX_FIFO_COUNT_MASK) >> TX_FIFO_COUNT_SHIFT;
+}
 
-	/* wait for room in the tx FIFO */
-	while ((readl(&uart->ufstat) & TX_FIFO_FULL_MASK)) {
-		if (serial_err_check(dev_index, 1))
-			return;
-	}
+static int s5p_serial_getc(struct udevice *dev)
+{
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	writeb(c, &uart->utxh);
+	if (!(readl(&uart->ufstat) & RX_FIFO_COUNT_MASK))
+		return -EAGAIN;
 
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc('\r');
+	serial_err_check(uart, 0);
+	return (int)(readb(&uart->urxh) & 0xff);
 }
 
-/*
- * Test whether a character is in the RX buffer
- */
-static int serial_tstc_dev(const int dev_index)
+int s5p_serial_setbrg(struct udevice *dev, int baudrate)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+	u32 uclk = get_uart_clk(plat->port_id);
+	u32 val;
 
-	if (!config.enabled)
-		return 0;
+	val = uclk / baudrate;
+
+	writel(val / 16 - 1, &uart->ubrdiv);
 
-	return (int)(readl(&uart->utrstat) & 0x1);
+	if (s5p_uart_divslot())
+		writew(udivslot[val % 16], &uart->rest.slot);
+	else
+		writeb(val % 16, &uart->rest.value);
+
+	return 0;
 }
 
-static void serial_puts_dev(const char *s, const int dev_index)
+static int s5p_serial_probe(struct udevice *dev)
 {
-	while (*s)
-		serial_putc_dev(*s++, dev_index);
-}
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+
+	/* enable FIFOs, auto clear Rx FIFO */
+	writel(0x3, &uart->ufcon);
+	writel(0, &uart->umcon);
+	/* 8N1 */
+	writel(0x3, &uart->ulcon);
+	/* No interrupts, no DMA, pure polling */
+	writel(0x245, &uart->ucon);
 
-/* Multi serial device functions */
-#define DECLARE_S5P_SERIAL_FUNCTIONS(port) \
-static int s5p_serial##port##_init(void) { return serial_init_dev(port); } \
-static void s5p_serial##port##_setbrg(void) { serial_setbrg_dev(port); } \
-static int s5p_serial##port##_getc(void) { return serial_getc_dev(port); } \
-static int s5p_serial##port##_tstc(void) { return serial_tstc_dev(port); } \
-static void s5p_serial##port##_putc(const char c) { serial_putc_dev(c, port); } \
-static void s5p_serial##port##_puts(const char *s) { serial_puts_dev(s, port); }
-
-#define INIT_S5P_SERIAL_STRUCTURE(port, __name) {	\
-	.name	= __name,				\
-	.start	= s5p_serial##port##_init,		\
-	.stop	= NULL,					\
-	.setbrg	= s5p_serial##port##_setbrg,		\
-	.getc	= s5p_serial##port##_getc,		\
-	.tstc	= s5p_serial##port##_tstc,		\
-	.putc	= s5p_serial##port##_putc,		\
-	.puts	= s5p_serial##port##_puts,		\
+	return 0;
 }
 
-DECLARE_S5P_SERIAL_FUNCTIONS(0);
-struct serial_device s5p_serial0_device =
-	INIT_S5P_SERIAL_STRUCTURE(0, "s5pser0");
-DECLARE_S5P_SERIAL_FUNCTIONS(1);
-struct serial_device s5p_serial1_device =
-	INIT_S5P_SERIAL_STRUCTURE(1, "s5pser1");
-DECLARE_S5P_SERIAL_FUNCTIONS(2);
-struct serial_device s5p_serial2_device =
-	INIT_S5P_SERIAL_STRUCTURE(2, "s5pser2");
-DECLARE_S5P_SERIAL_FUNCTIONS(3);
-struct serial_device s5p_serial3_device =
-	INIT_S5P_SERIAL_STRUCTURE(3, "s5pser3");
-
-#ifdef CONFIG_OF_CONTROL
-int fdtdec_decode_console(int *index, struct fdt_serial *uart)
+static int s5p_serial_ofdata_to_platdata(struct udevice *dev)
 {
-	const void *blob = gd->fdt_blob;
-	int node;
+	struct s5p_serial_platdata *plat = dev->platdata;
+	fdt_addr_t addr;
 
-	node = fdt_path_offset(blob, "console");
-	if (node < 0)
-		return node;
+	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
 
-	uart->base_addr = fdtdec_get_addr(blob, node, "reg");
-	if (uart->base_addr == FDT_ADDR_T_NONE)
-		return -FDT_ERR_NOTFOUND;
-
-	uart->port_id = fdtdec_get_int(blob, node, "id", -1);
-	uart->enabled = fdtdec_get_is_enabled(blob, node);
+	plat->reg = (struct s5p_uart *)addr;
+	plat->port_id = fdtdec_get_int(gd->fdt_blob, dev->of_offset, "id", -1);
 
 	return 0;
 }
-#endif
 
-__weak struct serial_device *default_serial_console(void)
-{
-#ifdef CONFIG_OF_CONTROL
-	int index = 0;
-
-	if ((!config.base_addr) && (fdtdec_decode_console(&index, &config))) {
-		debug("Cannot decode default console node\n");
-		return NULL;
-	}
-
-	switch (config.port_id) {
-	case 0:
-		return &s5p_serial0_device;
-	case 1:
-		return &s5p_serial1_device;
-	case 2:
-		return &s5p_serial2_device;
-	case 3:
-		return &s5p_serial3_device;
-	default:
-		debug("Unknown config.port_id: %d", config.port_id);
-		break;
-	}
-
-	return NULL;
-#else
-	config.enabled = 1;
-#if defined(CONFIG_SERIAL0)
-	return &s5p_serial0_device;
-#elif defined(CONFIG_SERIAL1)
-	return &s5p_serial1_device;
-#elif defined(CONFIG_SERIAL2)
-	return &s5p_serial2_device;
-#elif defined(CONFIG_SERIAL3)
-	return &s5p_serial3_device;
-#else
-#error "CONFIG_SERIAL? missing."
-#endif
-#endif
-}
+static const struct dm_serial_ops s5p_serial_ops = {
+	.putc = s5p_serial_putc,
+	.pending = s5p_serial_pending,
+	.getc = s5p_serial_getc,
+	.setbrg = s5p_serial_setbrg,
+};
 
-void s5p_serial_initialize(void)
-{
-	serial_register(&s5p_serial0_device);
-	serial_register(&s5p_serial1_device);
-	serial_register(&s5p_serial2_device);
-	serial_register(&s5p_serial3_device);
-}
+static const struct udevice_id s5p_serial_ids[] = {
+	{ .compatible = "samsung,exynos4210-uart" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_s5p) = {
+	.name	= "serial_s5p",
+	.id	= UCLASS_SERIAL,
+	.of_match = s5p_serial_ids,
+	.ofdata_to_platdata = s5p_serial_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct s5p_serial_platdata),
+	.probe = s5p_serial_probe,
+	.ops	= &s5p_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
diff --git a/include/configs/exynos5-dt.h b/include/configs/exynos5-dt.h
index cc3a409..12b086b 100644
--- a/include/configs/exynos5-dt.h
+++ b/include/configs/exynos5-dt.h
@@ -19,6 +19,7 @@
 #define CONFIG_DM
 #define CONFIG_CMD_DM
 #define CONFIG_DM_GPIO
+#define CONFIG_DM_SERIAL
 
 #define CONFIG_SYS_GENERIC_BOARD
 #define CONFIG_ARCH_CPU_INIT
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 9/9] dm: Make driver model available before board_init()

[Simon Glass]

For some boards board_init() will change GPIOs, so we need to have driver
model available before then. Adjust the board init to arrange this, but
enable it for driver model only, just to be safe.

This does create additional #ifdef logic, but it is safer than trying to
make a pervasive change which may cause some boards to break.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add exynos serial support
- Remove RFC status
- Split out core driver model patches into a separate set

 common/board_r.c    | 24 +++++++++++-------------
 common/stdio.c      | 18 ++++++++++++++++--
 include/stdio_dev.h | 24 +++++++++++++++++++++++-
 3 files changed, 50 insertions(+), 16 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 6ea8bce..5c92664 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -717,6 +717,15 @@ init_fnc_t init_sequence_r[] = {
 	/* TODO: could x86/PPC have this also perhaps? */
 #ifdef CONFIG_ARM
 	initr_caches,
+#endif
+	initr_reloc_global_data,
+	initr_barrier,
+	initr_malloc,
+	bootstage_relocate,
+#ifdef CONFIG_DM
+	initr_dm,
+#endif
+#ifdef CONFIG_ARM
 	board_init,	/* Setup chipselects */
 #endif
 	/*
@@ -728,16 +737,7 @@ init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_CLOCKS
 	set_cpu_clk_info, /* Setup clock information */
 #endif
-	initr_reloc_global_data,
-	initr_barrier,
-	initr_malloc,
-	bootstage_relocate,
-#ifdef CONFIG_DM_SERIAL
-	stdio_init,
-#endif
-#ifdef CONFIG_DM
-	initr_dm,
-#endif
+	stdio_init_tables,
 	initr_serial,
 	initr_announce,
 	INIT_FUNC_WATCHDOG_RESET
@@ -823,9 +823,7 @@ init_fnc_t init_sequence_r[] = {
 	 */
 	initr_pci,
 #endif
-#ifndef CONFIG_DM_SERIAL
-	stdio_init,
-#endif
+	stdio_add_devices,
 	initr_jumptable,
 #ifdef CONFIG_API
 	initr_api,
diff --git a/common/stdio.c b/common/stdio.c
index 692ca7f..c878103 100644
--- a/common/stdio.c
+++ b/common/stdio.c
@@ -215,7 +215,7 @@ int stdio_deregister(const char *devname)
 }
 #endif	/* CONFIG_SYS_STDIO_DEREGISTER */
 
-int stdio_init (void)
+int stdio_init_tables(void)
 {
 #if defined(CONFIG_NEEDS_MANUAL_RELOC)
 	/* already relocated for current ARM implementation */
@@ -232,6 +232,11 @@ int stdio_init (void)
 	/* Initialize the list */
 	INIT_LIST_HEAD(&(devs.list));
 
+	return 0;
+}
+
+int stdio_add_devices(void)
+{
 #ifdef CONFIG_SYS_I2C
 	i2c_init_all();
 #else
@@ -265,5 +270,14 @@ int stdio_init (void)
 #ifdef CONFIG_CBMEM_CONSOLE
 	cbmemc_init();
 #endif
-	return (0);
+
+	return 0;
+}
+
+int stdio_init(void)
+{
+	stdio_init_tables();
+	stdio_add_devices();
+
+	return 0;
 }
diff --git a/include/stdio_dev.h b/include/stdio_dev.h
index a7d0825..268de8e 100644
--- a/include/stdio_dev.h
+++ b/include/stdio_dev.h
@@ -78,7 +78,29 @@ extern char *stdio_names[MAX_FILES];
  */
 int	stdio_register (struct stdio_dev * dev);
 int stdio_register_dev(struct stdio_dev *dev, struct stdio_dev **devp);
-int	stdio_init (void);
+
+/**
+ * stdio_init_tables() - set up stdio tables ready for devices
+ *
+ * This does not add any devices, but just prepares stdio for use.
+ */
+int stdio_init_tables(void);
+
+/**
+ * stdio_add_devices() - Add stdio devices to the table
+ *
+ * This makes calls to all the various subsystems that use stdio, to make
+ * them register with stdio.
+ */
+int stdio_add_devices(void);
+
+/**
+ * stdio_init() - Sets up stdio ready for use
+ *
+ * This calls stdio_init_tables() and stdio_add_devices()
+ */
+int stdio_init(void);
+
 void	stdio_print_current_devices(void);
 #ifdef CONFIG_SYS_STDIO_DEREGISTER
 int	stdio_deregister(const char *devname);
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v2 0/9] Introduce driver model serial uclass

[Simon Glass]

Hi,

On 13 July 2014 19:27, Simon Glass <sjg@chromium.org> wrote:
> This series adds support for a serial uclass, enabling serial drivers to be
> converted to use driver model.
>
> Unfortunately this is quite a complicated process for a number of reasons:
>
> - serial is used before relocation, but driver model does not support this
> - stdio member functions are not passed a device pointer, but driver model
>     requires this (so does serial, but it uses an ugly work-around)
> - driver model requires malloc() but this is not available before
>   relocation
> - for sandbox, if something goes wrong with the console, we still need to
>   get an error message out through the fallback console
>
> So this series relies on quite a few patches to address the above, as well
> as the serial uclass and an implementation for sandbox.
>
> If you have limited time, please take a look at least at the uclass patch
> which is 'dm: Add a uclass for serial devices' (see include/serial.h).
>
> Note: this series breaks 4 exynos boards. I will figure out how to get them
> to build as part of the exynos5-dt common board work.
>
> To see the current state of driver model, look at u-boot-dm.git branch
> 'working'.
>
> Changes in v2:
> - Add exynos serial support
> - Remove RFC status
> - Rename struct device to struct udevice
> - Split out core driver model patches into a separate set

Are there any comments on this series? I'm particularly interested in
comments on include/serial.h - the uclass API for serial.

I plan to apply the uclass and sandbox support soon, with exynos
support coming later once the 'common' serial has been reviewed.

http://patchwork.ozlabs.org/patch/372891/

Regards,
Simon

[U-Boot] [PATCH v2 8/9] dm: exynos: Move serial to driver model

[Tom Rini]

On Sun, Jul 13, 2014 at 12:27:41PM -0600, Simon Glass wrote:

> Change the Exynos serial driver to work with driver model and switch over
> all Exynos5 boards to use it.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

So, maybe it's just the way the diff was generated, but did you re-order
the functions in the file?  Trying to see for example how 'set_brg'
changed here isn't easy since it used to be at the top, but is now near
the bottom?  And other functions too perhaps.

IOW, assuming the function locations changed in the driver, can you
re-shuffle things so the function order is the same?  This will help
both with review and future conversions I think.  Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140728/eb8da35a/attachment.pgp>

[U-Boot] [PATCH v2 0/9] Introduce driver model serial uclass

[Tom Rini]

On Mon, Jul 28, 2014 at 05:06:20AM +0100, Simon Glass wrote:
> Hi,
> 
> On 13 July 2014 19:27, Simon Glass <sjg@chromium.org> wrote:
> > This series adds support for a serial uclass, enabling serial drivers to be
> > converted to use driver model.
> >
> > Unfortunately this is quite a complicated process for a number of reasons:
> >
> > - serial is used before relocation, but driver model does not support this
> > - stdio member functions are not passed a device pointer, but driver model
> >     requires this (so does serial, but it uses an ugly work-around)
> > - driver model requires malloc() but this is not available before
> >   relocation
> > - for sandbox, if something goes wrong with the console, we still need to
> >   get an error message out through the fallback console
> >
> > So this series relies on quite a few patches to address the above, as well
> > as the serial uclass and an implementation for sandbox.
> >
> > If you have limited time, please take a look at least at the uclass patch
> > which is 'dm: Add a uclass for serial devices' (see include/serial.h).
> >
> > Note: this series breaks 4 exynos boards. I will figure out how to get them
> > to build as part of the exynos5-dt common board work.
> >
> > To see the current state of driver model, look at u-boot-dm.git branch
> > 'working'.
> >
> > Changes in v2:
> > - Add exynos serial support
> > - Remove RFC status
> > - Rename struct device to struct udevice
> > - Split out core driver model patches into a separate set
> 
> Are there any comments on this series? I'm particularly interested in
> comments on include/serial.h - the uclass API for serial.
> 
> I plan to apply the uclass and sandbox support soon, with exynos
> support coming later once the 'common' serial has been reviewed.

Seems good enough to start with for sure, I'm usually a believer in
needing a few things converted over to something, to start to see where
designs need tweaking.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20140728/95fd3f1d/attachment.pgp>

[U-Boot] [PATCH v2 8/9] dm: exynos: Move serial to driver model

[Simon Glass]

Hi Tom,

On 28 July 2014 19:05, Tom Rini <trini@ti.com> wrote:
> On Sun, Jul 13, 2014 at 12:27:41PM -0600, Simon Glass wrote:
>
>> Change the Exynos serial driver to work with driver model and switch over
>> all Exynos5 boards to use it.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
> So, maybe it's just the way the diff was generated, but did you re-order
> the functions in the file?  Trying to see for example how 'set_brg'
> changed here isn't easy since it used to be at the top, but is now near
> the bottom?  And other functions too perhaps.
>
> IOW, assuming the function locations changed in the driver, can you
> re-shuffle things so the function order is the same?  This will help
> both with review and future conversions I think.  Thanks!

Yes I think i can improve it as you say.

But however you look at it this is essentially a rewrite. All the old
boilerplate for multi-serial is gone (DECLARE_S5P_SERIAL_FUNCTIONS and
so on). The new driver model structures are added. All the functions
change signature and quite a bit of contents. The old driver was 300
lines, the new one is 190.

Unfortunately I think this is going to be a complete pain to review
whatever I do. For the driver model conversion the serial drivers are
all going to be quite different if they currently support multi-serial
using the #define work-around.

Regards,
Simon

[U-Boot] [PATCH v2 0/9] Introduce driver model serial uclass

[Simon Glass]

Hi Tom,

On 28 July 2014 19:14, Tom Rini <trini@ti.com> wrote:
> On Mon, Jul 28, 2014 at 05:06:20AM +0100, Simon Glass wrote:
>> Hi,
>>
>> On 13 July 2014 19:27, Simon Glass <sjg@chromium.org> wrote:
>> > This series adds support for a serial uclass, enabling serial drivers to be
>> > converted to use driver model.
>> >
>> > Unfortunately this is quite a complicated process for a number of reasons:
>> >
>> > - serial is used before relocation, but driver model does not support this
>> > - stdio member functions are not passed a device pointer, but driver model
>> >     requires this (so does serial, but it uses an ugly work-around)
>> > - driver model requires malloc() but this is not available before
>> >   relocation
>> > - for sandbox, if something goes wrong with the console, we still need to
>> >   get an error message out through the fallback console
>> >
>> > So this series relies on quite a few patches to address the above, as well
>> > as the serial uclass and an implementation for sandbox.
>> >
>> > If you have limited time, please take a look at least at the uclass patch
>> > which is 'dm: Add a uclass for serial devices' (see include/serial.h).
>> >
>> > Note: this series breaks 4 exynos boards. I will figure out how to get them
>> > to build as part of the exynos5-dt common board work.
>> >
>> > To see the current state of driver model, look at u-boot-dm.git branch
>> > 'working'.
>> >
>> > Changes in v2:
>> > - Add exynos serial support
>> > - Remove RFC status
>> > - Rename struct device to struct udevice
>> > - Split out core driver model patches into a separate set
>>
>> Are there any comments on this series? I'm particularly interested in
>> comments on include/serial.h - the uclass API for serial.
>>
>> I plan to apply the uclass and sandbox support soon, with exynos
>> support coming later once the 'common' serial has been reviewed.
>
> Seems good enough to start with for sure, I'm usually a believer in
> needing a few things converted over to something, to start to see where
> designs need tweaking.

OK good, thanks for the review. I really like the incremental approach.

Regards,
Simon