[U-Boot] [PATCH v3 0/16] Introduce driver model serial uclass

[U-Boot] [PATCH v3 0/16] 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).

With v3, exynos boards all build and an attempt has been made to add Tegra
support via the ns16550 driver. This is so-far untested but is offered for
review.

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

Changes in v3:
- Add new change to enhance lists_bind_fdt()
- Add new patch for tegra serial port details
- Add new patch to collect common baud rate code in ns16550
- Add new patch to enable driver model for seial on tegra
- Add new patch to move baud rate calculation to ns16550.c
- Add new patch to support driver model in ns16550
- Add new patch to use V_NS16550_CLK only in SPL builds
- Automatically bind the console even if not marked for pre-relocation
- Avoid reordering functions
- Change pre-reloc fdt property to 'u-boot,dm-pre-reloc'
- Fix typo in commit message

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 (16):
  serial: Set up the 'priv' pointer when creating a serial device
  dm: Adjust lists_bind_fdt() to return the bound 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()
  dm: serial: Move baud rate calculation to ns16550.c
  dm: serial: Collect common baud rate code in ns16550
  dm: serial: Add driver model support for ns16550
  tegra: dts: Add serial port details
  RFC: dm: tegra: Enable driver model for serial
  dm: tegra: Use V_NS16550_CLK only in SPL builds

 arch/arm/dts/exynos5.dtsi                   |   1 +
 arch/arm/dts/tegra114-dalmore.dts           |   1 +
 arch/arm/dts/tegra114.dtsi                  |  40 +++++
 arch/arm/dts/tegra124-jetson-tk1.dts        |   1 +
 arch/arm/dts/tegra124-venice2.dts           |   1 +
 arch/arm/dts/tegra124.dtsi                  |  40 +++++
 arch/arm/dts/tegra20-colibri_t20_iris.dts   |   1 +
 arch/arm/dts/tegra20-harmony.dts            |   1 +
 arch/arm/dts/tegra20-medcom-wide.dts        |   1 +
 arch/arm/dts/tegra20-paz00.dts              |   1 +
 arch/arm/dts/tegra20-plutux.dts             |   1 +
 arch/arm/dts/tegra20-seaboard.dts           |   1 +
 arch/arm/dts/tegra20-tec.dts                |   1 +
 arch/arm/dts/tegra20-trimslice.dts          |   1 +
 arch/arm/dts/tegra20-ventana.dts            |   1 +
 arch/arm/dts/tegra20-whistler.dts           |   1 +
 arch/arm/dts/tegra20.dtsi                   |  15 +-
 arch/arm/dts/tegra30-beaver.dts             |   1 +
 arch/arm/dts/tegra30-cardhu.dts             |   1 +
 arch/arm/dts/tegra30-tamonten.dtsi          |   1 +
 arch/arm/dts/tegra30.dtsi                   |  40 +++++
 arch/sandbox/dts/sandbox.dts                |  10 ++
 common/board_r.c                            |  19 ++-
 common/stdio.c                              |  18 +-
 doc/device-tree-bindings/serial/ns16550.txt |  10 ++
 drivers/core/lists.c                        |  10 +-
 drivers/core/root.c                         |   2 +-
 drivers/serial/Makefile                     |   6 +-
 drivers/serial/ns16550.c                    | 224 +++++++++++++++++++++---
 drivers/serial/sandbox.c                    | 140 ++++++++++++---
 drivers/serial/serial-uclass.c              | 211 +++++++++++++++++++++++
 drivers/serial/serial.c                     |   1 +
 drivers/serial/serial_ns16550.c             |  23 +--
 drivers/serial/serial_s5p.c                 | 255 ++++++++--------------------
 include/configs/exynos-common.h             |   1 +
 include/configs/sandbox.h                   |   3 +
 include/configs/tegra-common.h              |   8 +-
 include/configs/tegra124-common.h           |   2 +
 include/configs/tegra20-common.h            |   2 +
 include/configs/tegra30-common.h            |   2 +
 include/dm/lists.h                          |   6 +-
 include/dm/uclass-id.h                      |   1 +
 include/ns16550.h                           |  37 ++++
 include/serial.h                            |  92 ++++++++++
 include/stdio_dev.h                         |  24 ++-
 45 files changed, 987 insertions(+), 272 deletions(-)
 create mode 100644 doc/device-tree-bindings/serial/ns16550.txt
 create mode 100644 drivers/serial/serial-uclass.c

-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 01/16] serial: Set up the 'priv' pointer when creating a serial device

[Simon Glass]

The stdio_dev structure has a private pointer for its creator, but it is
not set up by 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 v3:
- Fix typo in commit message

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 v3 02/16] dm: Adjust lists_bind_fdt() to return the bound device

[Simon Glass]

Allow the caller to find out the device that was bound in response to this
call.

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

Changes in v3:
- Add new change to enhance lists_bind_fdt()

Changes in v2: None

 drivers/core/lists.c | 10 +++++++---
 drivers/core/root.c  |  2 +-
 include/dm/lists.h   |  6 +++++-
 3 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 0f08bfd..699f94b 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -118,7 +118,8 @@ static int driver_check_compatible(const void *blob, int offset,
 	return -ENOENT;
 }
 
-int lists_bind_fdt(struct udevice *parent, const void *blob, int offset)
+int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
+		   struct udevice **devp)
 {
 	struct driver *driver = ll_entry_start(struct driver, driver);
 	const int n_ents = ll_entry_count(struct driver, driver);
@@ -130,6 +131,8 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset)
 	int ret = 0;
 
 	dm_dbg("bind node %s\n", fdt_get_name(blob, offset, NULL));
+	if (devp)
+		*devp = NULL;
 	for (entry = driver; entry != driver + n_ents; entry++) {
 		ret = driver_check_compatible(blob, offset, entry->of_match);
 		name = fdt_get_name(blob, offset, NULL);
@@ -149,10 +152,11 @@ int lists_bind_fdt(struct udevice *parent, const void *blob, int offset)
 		ret = device_bind(parent, entry, name, NULL, offset, &dev);
 		if (ret) {
 			dm_warn("Error binding driver '%s'\n", entry->name);
-			if (!result || ret != -ENOENT)
-				result = ret;
+			return ret;
 		} else {
 			found = true;
+			if (devp)
+				*devp = dev;
 		}
 		break;
 	}
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 393dd98..a328a48 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -91,7 +91,7 @@ int dm_scan_fdt_node(struct udevice *parent, const void *blob, int offset,
 		if (pre_reloc_only &&
 		    !fdt_getprop(blob, offset, "u-boot,dm-pre-reloc", NULL))
 			continue;
-		err = lists_bind_fdt(parent, blob, offset);
+		err = lists_bind_fdt(parent, blob, offset, NULL);
 		if (err && !ret)
 			ret = err;
 	}
diff --git a/include/dm/lists.h b/include/dm/lists.h
index 87a3af5..2356895 100644
--- a/include/dm/lists.h
+++ b/include/dm/lists.h
@@ -53,7 +53,11 @@ int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only);
  * @parent: parent driver (root)
  * @blob: device tree blob
  * @offset: offset of this device tree node
+ * @devp: if non-NULL, returns a pointer to the bound device
+ * @return 0 if device was bound, -EINVAL if the device tree is invalid,
+ * other -ve value on error
  */
-int lists_bind_fdt(struct udevice *parent, const void *blob, int offset);
+int lists_bind_fdt(struct udevice *parent, const void *blob, int offset,
+		   struct udevice **devp);
 
 #endif
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 03/16] 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 v3:
- Automatically bind the console even if not marked for pre-relocation

Changes in v2:
- Rename struct device to struct udevice

 drivers/serial/Makefile        |   4 +
 drivers/serial/serial-uclass.c | 211 +++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h         |   1 +
 include/serial.h               |  92 ++++++++++++++++++
 4 files changed, 308 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..64e0c94
--- /dev/null
+++ b/drivers/serial/serial-uclass.c
@@ -0,0 +1,211 @@
+/*
+ * 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>
+#include <dm/lists.h>
+#include <dm/device-internal.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)
+{
+	int node;
+
+	/* Check for a console alias */
+	node = fdtdec_get_alias_node(gd->fdt_blob, "console");
+	if (!uclass_get_device_by_of_offset(UCLASS_SERIAL, node, &cur_dev))
+		return;
+
+	/*
+	 * If the console is not marked to be bound before relocation, bind
+	 * it anyway.
+	 */
+	if (node > 0 &&
+	    !lists_bind_fdt(gd->dm_root, gd->fdt_blob, node, &cur_dev)) {
+		if (!device_probe(cur_dev))
+			return;
+		cur_dev = NULL;
+	}
+
+	/*
+	 * 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 v3 04/16] 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 v3: None
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 8e7a3ac..2eed569 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -726,6 +726,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
@@ -762,12 +771,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
@@ -817,7 +820,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 v3 05/16] 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 v3: None
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 v3 06/16] 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 v3: None
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 v3 07/16] 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 v3:
- Change pre-reloc fdt property to 'u-boot,dm-pre-reloc'

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 efffacb..fd2b44b 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";
+		u-boot,dm-pre-reloc;
+		text-colour = "cyan";
+	};
+
 	triangle {
 		compatible = "demo-shape";
 		colour = "cyan";
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Stephen Warren]

On 07/30/2014 03:49 AM, Simon Glass wrote:
> 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.

> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts

> +	uart0: serial {
> +		compatible = "sandbox,serial";
> +		u-boot,dm-pre-reloc;

Shouldn't that be handled by the driver. It's certainly something that's 
only relevant to the internals of U-Boot, and hence inappropriate to put 
into DT.

> +		text-colour = "cyan";

That's property should likely have a uboot, prefix, since it's non-standard.

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Simon Glass]

Hi Stephen,

On 31 July 2014 21:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>
>> 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.
>
>
>> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
>
>
>> +       uart0: serial {
>> +               compatible = "sandbox,serial";
>> +               u-boot,dm-pre-reloc;
>
>
> Shouldn't that be handled by the driver. It's certainly something that's
> only relevant to the internals of U-Boot, and hence inappropriate to put
> into DT.

Hence the u-boot prefix. This is described in the driver model docs. I
have found a work-around (which forces a driver to be inited pre-reloc
if none is found) but I'm not 100% happy with it.

>
>> +               text-colour = "cyan";
>
>
> That's property should likely have a uboot, prefix, since it's non-standard.

Can I not just declare a binding for 'sandbox,serial'?

Regards,
Simon

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Stephen Warren]

On 07/31/2014 04:13 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 31 July 2014 21:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>
>>> 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.
>>
>>
>>> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
>>
>>
>>> +       uart0: serial {
>>> +               compatible = "sandbox,serial";
>>> +               u-boot,dm-pre-reloc;
>>
>>
>> Shouldn't that be handled by the driver. It's certainly something that's
>> only relevant to the internals of U-Boot, and hence inappropriate to put
>> into DT.
>
> Hence the u-boot prefix. This is described in the driver model docs. I
> have found a work-around (which forces a driver to be inited pre-reloc
> if none is found) but I'm not 100% happy with it.

I'm arguing that the property shouldn't exist in DT at all. DT is 
supposed to be a pure description of the HW, and not encode details that 
are specific to the implementation of particular SW. The fact that 
U-Boot performs relocation of its code during boot is completely 
irrelevant to a HW description.

As such, the issue isn't whether there is a u-boot, prefix on that 
property, but whether it's there at all.

>>> +               text-colour = "cyan";
>>
>>
>> That's property should likely have a uboot, prefix, since it's non-standard.
>
> Can I not just declare a binding for 'sandbox,serial'?

Properties that are relevant only to a particular binding, rather than 
being something quite generic an applicable to a whole class of devices, 
typically have a vendor prefix.

A binding should/must exist for every node or compatible value. So, 
whether you actually write the binding document or not makes no 
difference to the names or vendor prefixes of the properties the binding 
uses.

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Jon Loeliger]

>>>> +       uart0: serial {
>>>> +               compatible = "sandbox,serial";
>>>> +               u-boot,dm-pre-reloc;
>>>
>>> Shouldn't that be handled by the driver. It's certainly something that's
>>> only relevant to the internals of U-Boot, and hence inappropriate to put
>>> into DT.
>>
>> Hence the u-boot prefix. This is described in the driver model docs. I
>> have found a work-around (which forces a driver to be inited pre-reloc
>> if none is found) but I'm not 100% happy with it.
>
> I'm arguing that the property shouldn't exist in DT at all. DT is supposed
> to be a pure description of the HW, and not encode details that are specific
> to the implementation of particular SW. The fact that U-Boot performs
> relocation of its code during boot is completely irrelevant to a HW
> description.
>
> As such, the issue isn't whether there is a u-boot, prefix on that property,
> but whether it's there at all.

Right.  And I've arguing that U-Boot should use exactly the same
descriptions that are in the Kernel even.  Those DTS descriptions should
be common, applicable to both or neither, exactly because they do
describe the HW and are agnostic WRT the SW that is using them.

HTH,
jdl

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Tom Rini]

On Fri, Aug 01, 2014 at 10:46:31AM -0500, Jon Loeliger wrote:
> >>>> +       uart0: serial {
> >>>> +               compatible = "sandbox,serial";
> >>>> +               u-boot,dm-pre-reloc;
> >>>
> >>> Shouldn't that be handled by the driver. It's certainly something that's
> >>> only relevant to the internals of U-Boot, and hence inappropriate to put
> >>> into DT.
> >>
> >> Hence the u-boot prefix. This is described in the driver model docs. I
> >> have found a work-around (which forces a driver to be inited pre-reloc
> >> if none is found) but I'm not 100% happy with it.
> >
> > I'm arguing that the property shouldn't exist in DT at all. DT is supposed
> > to be a pure description of the HW, and not encode details that are specific
> > to the implementation of particular SW. The fact that U-Boot performs
> > relocation of its code during boot is completely irrelevant to a HW
> > description.
> >
> > As such, the issue isn't whether there is a u-boot, prefix on that property,
> > but whether it's there at all.
> 
> Right.  And I've arguing that U-Boot should use exactly the same
> descriptions that are in the Kernel even.  Those DTS descriptions should
> be common, applicable to both or neither, exactly because they do
> describe the HW and are agnostic WRT the SW that is using them.

I also agree with this.  We'll have to sort out the fall-out.

-- 
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/20140801/fedbfacf/attachment.pgp>

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Simon Glass]

Hi,

On 1 August 2014 17:53, Tom Rini <trini@ti.com> wrote:
> On Fri, Aug 01, 2014 at 10:46:31AM -0500, Jon Loeliger wrote:
>> >>>> +       uart0: serial {
>> >>>> +               compatible = "sandbox,serial";
>> >>>> +               u-boot,dm-pre-reloc;
>> >>>
>> >>> Shouldn't that be handled by the driver. It's certainly something that's
>> >>> only relevant to the internals of U-Boot, and hence inappropriate to put
>> >>> into DT.
>> >>
>> >> Hence the u-boot prefix. This is described in the driver model docs. I
>> >> have found a work-around (which forces a driver to be inited pre-reloc
>> >> if none is found) but I'm not 100% happy with it.
>> >
>> > I'm arguing that the property shouldn't exist in DT at all. DT is supposed
>> > to be a pure description of the HW, and not encode details that are specific
>> > to the implementation of particular SW. The fact that U-Boot performs
>> > relocation of its code during boot is completely irrelevant to a HW
>> > description.
>> >
>> > As such, the issue isn't whether there is a u-boot, prefix on that property,
>> > but whether it's there at all.
>>
>> Right.  And I've arguing that U-Boot should use exactly the same
>> descriptions that are in the Kernel even.  Those DTS descriptions should
>> be common, applicable to both or neither, exactly because they do
>> describe the HW and are agnostic WRT the SW that is using them.
>
> I also agree with this.  We'll have to sort out the fall-out.

As mentioned I think I have found a way (with serial at least) to work
around this - when the serial probe files, this series has code which
forces binding of the node referenced by the 'console' alias. It's a
bit of a hack but it might be good enough...

Regards,
Simon

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Simon Glass]

Hi Stephen,

On 1 August 2014 00:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/31/2014 04:13 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 31 July 2014 21:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>>
>>>>
>>>> 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.
>>>
>>>
>>>
>>>> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
>>>
>>>
>>>
>>>> +       uart0: serial {
>>>> +               compatible = "sandbox,serial";
>>>> +               u-boot,dm-pre-reloc;
>>>
>>>
>>>
>>> Shouldn't that be handled by the driver. It's certainly something that's
>>> only relevant to the internals of U-Boot, and hence inappropriate to put
>>> into DT.
>>
>>
>> Hence the u-boot prefix. This is described in the driver model docs. I
>> have found a work-around (which forces a driver to be inited pre-reloc
>> if none is found) but I'm not 100% happy with it.
>
>
> I'm arguing that the property shouldn't exist in DT at all. DT is supposed
> to be a pure description of the HW, and not encode details that are specific
> to the implementation of particular SW. The fact that U-Boot performs
> relocation of its code during boot is completely irrelevant to a HW
> description.
>
> As such, the issue isn't whether there is a u-boot, prefix on that property,
> but whether it's there at all.

I completely agree in theory, although in practice this can become a
huge pain. I'm open to suggestions. On the other hand as you well know
I'm more inclined to use a convenient solution and iterate/improve
later than sit on my hands and fret :-) Please check the dm README for
the issue involved.

>
>
>>>> +               text-colour = "cyan";
>>>
>>>
>>>
>>> That's property should likely have a uboot, prefix, since it's
>>> non-standard.
>>
>>
>> Can I not just declare a binding for 'sandbox,serial'?
>
>
> Properties that are relevant only to a particular binding, rather than being
> something quite generic an applicable to a whole class of devices, typically
> have a vendor prefix.
>
> A binding should/must exist for every node or compatible value. So, whether
> you actually write the binding document or not makes no difference to the
> names or vendor prefixes of the properties the binding uses.

So here you are saying it should be:

   sandbox,text-colour = "cyan";

Is that right?

Regards,
Simon

[U-Boot] [PATCH v3 07/16] sandbox: dts: Add a serial console node

[Stephen Warren]

On 08/01/2014 03:40 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 1 August 2014 00:09, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/31/2014 04:13 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 31 July 2014 21:20, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> 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.
>>>>
>>>>> diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
>>>>> +       uart0: serial {
>>>>> +               compatible = "sandbox,serial";
>>>>> +               u-boot,dm-pre-reloc;
>>>>
...
>>>>> +               text-colour = "cyan";
>>>>
>>>>
>>>>
>>>> That's property should likely have a uboot, prefix, since it's
>>>> non-standard.
>>>
>>> Can I not just declare a binding for 'sandbox,serial'?
>>
>> Properties that are relevant only to a particular binding, rather than being
>> something quite generic an applicable to a whole class of devices, typically
>> have a vendor prefix.
>>
>> A binding should/must exist for every node or compatible value. So, whether
>> you actually write the binding document or not makes no difference to the
>> names or vendor prefixes of the properties the binding uses.
>
> So here you are saying it should be:
>
>     sandbox,text-colour = "cyan";
>
> Is that right?

I think that sounds right yes.

[U-Boot] [PATCH v3 08/16] 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 v3:
- Change pre-reloc fdt property to 'u-boot,dm-pre-reloc'

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..e539068 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>;
+		u-boot,dm-pre-reloc;
 		id = <3>;
 	};
 
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 09/16] 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 v3:
- Avoid reordering functions

Changes in v2: None

 drivers/serial/serial_s5p.c     | 255 ++++++++++++----------------------------
 include/configs/exynos-common.h |   1 +
 2 files changed, 73 insertions(+), 183 deletions(-)

diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
index 98c62b4..8469afd 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,23 +59,13 @@ static const int udivslot[] = {
 	0xffdf,
 };
 
-static void serial_setbrg_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);
-	u32 uclk = get_uart_clk(dev_index);
-	u32 baudrate = gd->baudrate;
+	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 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);
@@ -90,15 +74,14 @@ static void serial_setbrg_dev(const int dev_index)
 		writew(udivslot[val % 16], &uart->rest.slot);
 	else
 		writeb(val % 16, &uart->rest.value);
+
+	return 0;
 }
 
-/*
- * 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 s5p_serial_probe(struct udevice *dev)
 {
-	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;
 
 	/* enable FIFOs, auto clear Rx FIFO */
 	writel(0x3, &uart->ufcon);
@@ -108,14 +91,11 @@ static int serial_init_dev(const int dev_index)
 	/* 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)
+static int serial_err_check(const struct s5p_uart *const uart, int op)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
 	unsigned int mask;
 
 	/*
@@ -133,169 +113,78 @@ 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_getc(struct udevice *dev)
 {
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
-
-	if (!config.enabled)
-		return 0;
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	/* 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;
-	}
+	if (!(readl(&uart->ufstat) & RX_FIFO_COUNT_MASK))
+		return -EAGAIN;
 
+	serial_err_check(uart, 0);
 	return (int)(readb(&uart->urxh) & 0xff);
 }
 
-/*
- * Output a single byte to the serial port.
- */
-static void serial_putc_dev(const char c, 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);
-
-	if (!config.enabled)
-		return;
-
-	/* wait for room in the tx FIFO */
-	while ((readl(&uart->ufstat) & TX_FIFO_FULL_MASK)) {
-		if (serial_err_check(dev_index, 1))
-			return;
-	}
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
 
-	writeb(c, &uart->utxh);
+	if (readl(&uart->ufstat) & TX_FIFO_FULL)
+		return -EAGAIN;
 
-	/* If \n, also do \r */
-	if (c == '\n')
-		serial_putc('\r');
-}
-
-/*
- * Test whether a character is in the RX buffer
- */
-static int serial_tstc_dev(const int dev_index)
-{
-	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	writeb(ch, &uart->utxh);
+	serial_err_check(uart, 1);
 
-	if (!config.enabled)
-		return 0;
-
-	return (int)(readl(&uart->utrstat) & 0x1);
+	return 0;
 }
 
-static void serial_puts_dev(const char *s, const int dev_index)
+static int s5p_serial_pending(struct udevice *dev, bool input)
 {
-	while (*s)
-		serial_putc_dev(*s++, dev_index);
-}
+	struct s5p_serial_platdata *plat = dev->platdata;
+	struct s5p_uart *const uart = plat->reg;
+	uint32_t ufstat = readl(&uart->ufstat);
 
-/* 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,		\
+	if (input)
+		return (ufstat & RX_FIFO_COUNT_MASK) >> RX_FIFO_COUNT_SHIFT;
+	else
+		return (ufstat & TX_FIFO_COUNT_MASK) >> TX_FIFO_COUNT_SHIFT;
 }
 
-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/exynos-common.h b/include/configs/exynos-common.h
index 79dd9dc..b8d81c5 100644
--- a/include/configs/exynos-common.h
+++ b/include/configs/exynos-common.h
@@ -20,6 +20,7 @@
 #define CONFIG_DM
 #define CONFIG_CMD_DM
 #define CONFIG_DM_GPIO
+#define CONFIG_DM_SERIAL
 
 #define CONFIG_ARCH_CPU_INIT
 #define CONFIG_DISPLAY_CPUINFO
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 09/16] dm: exynos: Move serial to driver model

[Tom Rini]

On Wed, Jul 30, 2014 at 03:49:46AM -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>
> ---
> 
> Changes in v3:
> - Avoid reordering functions

Yay.

> @@ -108,14 +91,11 @@ static int serial_init_dev(const int dev_index)
>  	/* No interrupts, no DMA, pure polling */
>  	writel(0x245, &uart->ucon);
>  
> -	serial_setbrg_dev(dev_index);
> -
>  	return 0;
>  }

So setbrg is called elsewhere, in a more common area?  That's documented
somewhere right?  This seems otherwise to be a straight-forward
conversion.  Things we used to loop for in the driver are now handled a
bit higher up and otherwise we find what to talk to in a slightly
different manner.

-- 
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/20140730/41d8c6f2/attachment.pgp>

[U-Boot] [PATCH v3 09/16] dm: exynos: Move serial to driver model

[Simon Glass]

Hi Tom,

On 30 July 2014 16:38, Tom Rini <trini@ti.com> wrote:
> On Wed, Jul 30, 2014 at 03:49:46AM -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>
>> ---
>>
>> Changes in v3:
>> - Avoid reordering functions
>
> Yay.
>
>> @@ -108,14 +91,11 @@ static int serial_init_dev(const int dev_index)
>>       /* No interrupts, no DMA, pure polling */
>>       writel(0x245, &uart->ucon);
>>
>> -     serial_setbrg_dev(dev_index);
>> -
>>       return 0;
>>  }
>
> So setbrg is called elsewhere, in a more common area?  That's documented
> somewhere right?

Yes, and sort-of. Since you don't have a baud rate at init time you
could only set a default value anyway. I'll add a comment to the
serial.h uclass.

> This seems otherwise to be a straight-forward
> conversion.  Things we used to loop for in the driver are now handled a
> bit higher up and otherwise we find what to talk to in a slightly
> different manner.

Yes it's not too bad, if you ignore all the multi-serial mush being
deleted. I felt that looping in a driver waiting for a character was a
bad idea (how serial_getc() used to work).

Regards,
Simon

[U-Boot] [PATCH v3 10/16] 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 v3: None
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 2eed569..e98ac9c 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -714,6 +714,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
 	/*
@@ -725,16 +734,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
@@ -820,9 +820,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 v3 11/16] dm: serial: Move baud rate calculation to ns16550.c

[Simon Glass]

Move the function that calculates the baud rate divisor into ns16550.c so
it can be used by that file.

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

Changes in v3:
- Add new patch to move baud rate calculation to ns16550.c

Changes in v2: None

 drivers/serial/ns16550.c        | 18 +++++++++++++++++-
 drivers/serial/serial_ns16550.c | 23 ++++-------------------
 include/ns16550.h               | 13 +++++++++++++
 3 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index f26979d..056210e 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -4,7 +4,7 @@
  * modified to use CONFIG_SYS_ISA_MEM and new defines
  */
 
-#include <config.h>
+#include <common.h>
 #include <ns16550.h>
 #include <watchdog.h>
 #include <linux/types.h>
@@ -45,6 +45,22 @@
 #define CONFIG_SYS_NS16550_IER  0x00
 #endif /* CONFIG_SYS_NS16550_IER */
 
+int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate)
+{
+	const unsigned int mode_x_div = 16;
+
+#ifdef CONFIG_OMAP1510
+	/* If can't cleanly clock 115200 set div to 1 */
+	if ((clock == 12000000) && (baudrate == 115200)) {
+		port->osc_12m_sel = OSC_12M_SEL;  /* enable 6.5 * divisor */
+		return 1;			/* return 1 for base divisor */
+	}
+	port->osc_12m_sel = 0;			/* clear if previsouly set */
+#endif
+
+	return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate);
+}
+
 void NS16550_init(NS16550_t com_port, int baud_divisor)
 {
 #if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
diff --git a/drivers/serial/serial_ns16550.c b/drivers/serial/serial_ns16550.c
index 4413e69..632da4c 100644
--- a/drivers/serial/serial_ns16550.c
+++ b/drivers/serial/serial_ns16550.c
@@ -81,7 +81,8 @@ static NS16550_t serial_ports[6] = {
 	static int  eserial##port##_init(void) \
 	{ \
 		int clock_divisor; \
-		clock_divisor = calc_divisor(serial_ports[port-1]); \
+		clock_divisor = ns16550_calc_divisor(serial_ports[port-1], \
+				CONFIG_SYS_NS16550_CLK, gd->baudrate); \
 		NS16550_init(serial_ports[port-1], clock_divisor); \
 		return 0 ; \
 	} \
@@ -118,23 +119,6 @@ static NS16550_t serial_ports[6] = {
 	.puts	= eserial##port##_puts,		\
 }
 
-static int calc_divisor (NS16550_t port)
-{
-	const unsigned int mode_x_div = 16;
-
-#ifdef CONFIG_OMAP1510
-	/* If can't cleanly clock 115200 set div to 1 */
-	if ((CONFIG_SYS_NS16550_CLK == 12000000) && (gd->baudrate == 115200)) {
-		port->osc_12m_sel = OSC_12M_SEL;	/* enable 6.5 * divisor */
-		return (1);				/* return 1 for base divisor */
-	}
-	port->osc_12m_sel = 0;			/* clear if previsouly set */
-#endif
-
-	return DIV_ROUND_CLOSEST(CONFIG_SYS_NS16550_CLK,
-						mode_x_div * gd->baudrate);
-}
-
 void
 _serial_putc(const char c,const int port)
 {
@@ -176,7 +160,8 @@ _serial_setbrg (const int port)
 {
 	int clock_divisor;
 
-	clock_divisor = calc_divisor(PORT);
+	clock_divisor = ns16550_calc_divisor(PORT, CONFIG_SYS_NS16550_CLK,
+					     gd->baudrate);
 	NS16550_reinit(PORT, clock_divisor);
 }
 
diff --git a/include/ns16550.h b/include/ns16550.h
index 17f829f..b106f8a 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -177,3 +177,16 @@ void NS16550_putc(NS16550_t com_port, char c);
 char NS16550_getc(NS16550_t com_port);
 int NS16550_tstc(NS16550_t com_port);
 void NS16550_reinit(NS16550_t com_port, int baud_divisor);
+
+/**
+ * ns16550_calc_divisor() - calculate the divisor given clock and baud rate
+ *
+ * Given the UART input clock and required baudrate, calculate the divisor
+ * that should be used.
+ *
+ * @port:	UART port
+ * @clock:	UART input clock speed in Hz
+ * @baudrate:	Required baud rate
+ * @return baud rate divisor that should be used
+ */
+int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate);
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 12/16] dm: serial: Collect common baud rate code in ns16550

[Simon Glass]

The same sequence is used in several places, so move it into a function.
Note that UART_LCR_BKSE is an alias for UART_LCR_DLAB.

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

Changes in v3:
- Add new patch to collect common baud rate code in ns16550

Changes in v2: None

 drivers/serial/ns16550.c | 33 +++++++++++++--------------------
 1 file changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 056210e..9373c8d 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -61,6 +61,14 @@ int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate)
 	return DIV_ROUND_CLOSEST(clock, mode_x_div * baudrate);
 }
 
+static void NS16550_setbrg(NS16550_t com_port, int baud_divisor)
+{
+	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
+	serial_out(baud_divisor & 0xff, &com_port->dll);
+	serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
+	serial_out(UART_LCRVAL, &com_port->lcr);
+}
+
 void NS16550_init(NS16550_t com_port, int baud_divisor)
 {
 #if (defined(CONFIG_SPL_BUILD) && defined(CONFIG_OMAP34XX))
@@ -71,10 +79,7 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 	 */
 	if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
 	     == UART_LSR_THRE) {
-		serial_out(UART_LCR_DLAB, &com_port->lcr);
-		serial_out(baud_divisor & 0xff, &com_port->dll);
-		serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
-		serial_out(UART_LCRVAL, &com_port->lcr);
+		NS16550_setbrg(com_port, baud_divisor);
 		serial_out(0, &com_port->mdr1);
 	}
 #endif
@@ -87,16 +92,10 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 			defined(CONFIG_TI81XX) || defined(CONFIG_AM43XX)
 	serial_out(0x7, &com_port->mdr1);	/* mode select reset TL16C750*/
 #endif
-	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
-	serial_out(0, &com_port->dll);
-	serial_out(0, &com_port->dlm);
-	serial_out(UART_LCRVAL, &com_port->lcr);
+	NS16550_setbrg(com_port, 0);
 	serial_out(UART_MCRVAL, &com_port->mcr);
 	serial_out(UART_FCRVAL, &com_port->fcr);
-	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
-	serial_out(baud_divisor & 0xff, &com_port->dll);
-	serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
-	serial_out(UART_LCRVAL, &com_port->lcr);
+	NS16550_setbrg(com_port, baud_divisor);
 #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
 	defined(CONFIG_AM33XX) || defined(CONFIG_SOC_DA8XX) || \
 	defined(CONFIG_TI81XX) || defined(CONFIG_AM43XX)
@@ -113,16 +112,10 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 void NS16550_reinit(NS16550_t com_port, int baud_divisor)
 {
 	serial_out(CONFIG_SYS_NS16550_IER, &com_port->ier);
-	serial_out(UART_LCR_BKSE | UART_LCRVAL, &com_port->lcr);
-	serial_out(0, &com_port->dll);
-	serial_out(0, &com_port->dlm);
-	serial_out(UART_LCRVAL, &com_port->lcr);
+	NS16550_setbrg(com_port, 0);
 	serial_out(UART_MCRVAL, &com_port->mcr);
 	serial_out(UART_FCRVAL, &com_port->fcr);
-	serial_out(UART_LCR_BKSE, &com_port->lcr);
-	serial_out(baud_divisor & 0xff, &com_port->dll);
-	serial_out((baud_divisor >> 8) & 0xff, &com_port->dlm);
-	serial_out(UART_LCRVAL, &com_port->lcr);
+	NS16550_setbrg(com_port, baud_divisor);
 }
 #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
 
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 13/16] dm: serial: Add driver model support for ns16550

[Simon Glass]

Add driver model support so that ns16550 can support operation both with
and without driver model.

Add a suitable device tree binding also.

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

Changes in v3:
- Add new patch to support driver model in ns16550

Changes in v2: None

 doc/device-tree-bindings/serial/ns16550.txt |  10 ++
 drivers/serial/Makefile                     |   2 +-
 drivers/serial/ns16550.c                    | 177 +++++++++++++++++++++++++++-
 include/ns16550.h                           |  24 ++++
 4 files changed, 210 insertions(+), 3 deletions(-)
 create mode 100644 doc/device-tree-bindings/serial/ns16550.txt

diff --git a/doc/device-tree-bindings/serial/ns16550.txt b/doc/device-tree-bindings/serial/ns16550.txt
new file mode 100644
index 0000000..ef0b9ae
--- /dev/null
+++ b/doc/device-tree-bindings/serial/ns16550.txt
@@ -0,0 +1,10 @@
+NS16550 UART
+
+This UART driver supports many chip variants and is used in mamy SoCs.
+
+Required properties:
+- compatible: "ns16550" or "nvidia,tegra20-uart"
+- reg: start address and size of registers
+- reg-shift: shift value indicating register size: 0=byte, 1=16bit,2=32bit etc.
+- clock-frequency: input clock frequency for the UART (used to calculate the
+    baud rate divisor)
diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
index 4720e1d..5ae6416 100644
--- a/drivers/serial/Makefile
+++ b/drivers/serial/Makefile
@@ -9,6 +9,7 @@ ifdef CONFIG_DM_SERIAL
 obj-y += serial-uclass.o
 else
 obj-y += serial.o
+obj-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 endif
 
 obj-$(CONFIG_ALTERA_UART) += altera_uart.o
@@ -20,7 +21,6 @@ obj-$(CONFIG_MCFUART) += mcfuart.o
 obj-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 obj-$(CONFIG_SYS_NS16550) += ns16550.o
 obj-$(CONFIG_S5P) += serial_s5p.o
-obj-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 obj-$(CONFIG_IMX_SERIAL) += serial_imx.o
 obj-$(CONFIG_KS8695_SERIAL) += serial_ks8695.o
 obj-$(CONFIG_MAX3100_SERIAL) += serial_max3100.o
diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c
index 9373c8d..c07a58c 100644
--- a/drivers/serial/ns16550.c
+++ b/drivers/serial/ns16550.c
@@ -5,17 +5,25 @@
  */
 
 #include <common.h>
+#include <dm.h>
+#include <errno.h>
+#include <fdtdec.h>
 #include <ns16550.h>
+#include <serial.h>
 #include <watchdog.h>
 #include <linux/types.h>
 #include <asm/io.h>
 
+DECLARE_GLOBAL_DATA_PTR;
+
 #define UART_LCRVAL UART_LCR_8N1		/* 8 data, 1 stop, no parity */
 #define UART_MCRVAL (UART_MCR_DTR | \
 		     UART_MCR_RTS)		/* RTS/DTR */
 #define UART_FCRVAL (UART_FCR_FIFO_EN |	\
 		     UART_FCR_RXSR |	\
 		     UART_FCR_TXSR)		/* Clear & enable FIFOs */
+
+#ifndef CONFIG_DM_SERIAL
 #ifdef CONFIG_SYS_NS16550_PORT_MAPPED
 #define serial_out(x, y)	outb(x, (ulong)y)
 #define serial_in(y)		inb((ulong)y)
@@ -29,6 +37,7 @@
 #define serial_out(x, y)	writeb(x, y)
 #define serial_in(y)		readb(y)
 #endif
+#endif /* !CONFIG_DM_SERIAL */
 
 #if defined(CONFIG_K2HK_EVM)
 #define UART_REG_VAL_PWREMU_MGMT_UART_DISABLE   0
@@ -45,6 +54,58 @@
 #define CONFIG_SYS_NS16550_IER  0x00
 #endif /* CONFIG_SYS_NS16550_IER */
 
+#ifdef CONFIG_DM_SERIAL
+void ns16550_writeb(NS16550_t port, int offset, int value)
+{
+	struct ns16550_platdata *plat = port->plat;
+	unsigned char *addr;
+
+	offset *= 1 << plat->reg_shift;
+	addr = plat->base + offset;
+	/*
+	 * As far as we know it doesn't make sense to support selection of
+	 * these options at run-time, so use the existing CONFIG options.
+	 */
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+	outb(value, addr);
+#elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
+	out_le32(addr, value);
+#elif defined(CONFIG_SYS_NS16550_MEM32) && defined(CONFIG_SYS_BIG_ENDIAN)
+	out_be32(addr, value);
+#elif defined(CONFIG_SYS_BIG_ENDIAN)
+	writeb(value, addr + (1 << plat->reg_shift) - 1);
+#else
+	writeb(value, addr);
+#endif
+}
+
+int ns16550_readb(NS16550_t port, int offset)
+{
+	struct ns16550_platdata *plat = port->plat;
+	unsigned char *addr;
+
+	offset *= 1 << plat->reg_shift;
+	addr = plat->base + offset;
+#ifdef CONFIG_SYS_NS16550_PORT_MAPPED
+	return inb(addr);
+#elif defined(CONFIG_SYS_NS16550_MEM32) && !defined(CONFIG_SYS_BIG_ENDIAN)
+	return in_le32(addr);
+#elif defined(CONFIG_SYS_NS16550_MEM32) && defined(CONFIG_SYS_BIG_ENDIAN)
+	return in_be32(addr);
+#elif defined(CONFIG_SYS_BIG_ENDIAN)
+	return readb(addr + (1 << plat->reg_shift) - 1);
+#else
+	return readb(addr);
+#endif
+}
+
+/* We can clean these up once everything is moved to driver model */
+#define serial_out(value, addr)	\
+	ns16550_writeb(com_port, addr - (unsigned char *)com_port, value)
+#define serial_in(addr) \
+	ns16550_readb(com_port, addr - (unsigned char *)com_port)
+#endif
+
 int ns16550_calc_divisor(NS16550_t port, int clock, int baudrate)
 {
 	const unsigned int mode_x_div = 16;
@@ -79,7 +140,8 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 	 */
 	if ((serial_in(&com_port->lsr) & (UART_LSR_TEMT | UART_LSR_THRE))
 	     == UART_LSR_THRE) {
-		NS16550_setbrg(com_port, baud_divisor);
+		if (baud_divisor != -1)
+			NS16550_setbrg(com_port, baud_divisor);
 		serial_out(0, &com_port->mdr1);
 	}
 #endif
@@ -95,7 +157,8 @@ void NS16550_init(NS16550_t com_port, int baud_divisor)
 	NS16550_setbrg(com_port, 0);
 	serial_out(UART_MCRVAL, &com_port->mcr);
 	serial_out(UART_FCRVAL, &com_port->fcr);
-	NS16550_setbrg(com_port, baud_divisor);
+	if (baud_divisor != -1)
+		NS16550_setbrg(com_port, baud_divisor);
 #if (defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)) || \
 	defined(CONFIG_AM33XX) || defined(CONFIG_SOC_DA8XX) || \
 	defined(CONFIG_TI81XX) || defined(CONFIG_AM43XX)
@@ -154,3 +217,113 @@ int NS16550_tstc(NS16550_t com_port)
 }
 
 #endif /* CONFIG_NS16550_MIN_FUNCTIONS */
+
+#ifdef CONFIG_DM_SERIAL
+static int ns16550_serial_putc(struct udevice *dev, const char ch)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+
+	if (!(serial_in(&com_port->lsr) & UART_LSR_THRE))
+		return -EAGAIN;
+	serial_out(ch, &com_port->thr);
+
+	/*
+	 * Call watchdog_reset() upon newline. This is done here in putc
+	 * since the environment code uses a single puts() to print the complete
+	 * environment upon "printenv". So we can't put this watchdog call
+	 * in puts().
+	 */
+	if (ch == '\n')
+		WATCHDOG_RESET();
+
+	return 0;
+}
+
+static int ns16550_serial_pending(struct udevice *dev, bool input)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+
+	if (input)
+		return serial_in(&com_port->lsr) & UART_LSR_DR ? 1 : 0;
+	else
+		return serial_in(&com_port->lsr) & UART_LSR_THRE ? 0 : 1;
+}
+
+static int ns16550_serial_getc(struct udevice *dev)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+
+	if (!serial_in(&com_port->lsr) & UART_LSR_DR)
+		return -EAGAIN;
+
+	return serial_in(&com_port->rbr);
+}
+
+int ns16550_serial_setbrg(struct udevice *dev, int baudrate)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+	struct ns16550_platdata *plat = com_port->plat;
+	int clock_divisor;
+
+	clock_divisor = ns16550_calc_divisor(com_port, plat->clock, baudrate);
+
+	NS16550_setbrg(com_port, clock_divisor);
+
+	return 0;
+}
+
+static int ns16550_serial_probe(struct udevice *dev)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+
+	NS16550_init(com_port, -1);
+
+	return 0;
+}
+
+static int ns16550_serial_ofdata_to_platdata(struct udevice *dev)
+{
+	struct NS16550 *const com_port = dev_get_priv(dev);
+	struct ns16550_platdata *plat = dev->platdata;
+	fdt_addr_t addr;
+
+	addr = fdtdec_get_addr(gd->fdt_blob, dev->of_offset, "reg");
+	if (addr == FDT_ADDR_T_NONE)
+		return -EINVAL;
+
+	plat->base = (unsigned char *)addr;
+	plat->reg_shift = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "reg-shift", 1);
+	plat->clock = fdtdec_get_int(gd->fdt_blob, dev->of_offset,
+					 "clock-frequency", -1);
+	if (plat->clock == -1)
+		return -EINVAL;
+	com_port->plat = plat;
+
+	return 0;
+}
+
+static const struct dm_serial_ops ns16550_serial_ops = {
+	.putc = ns16550_serial_putc,
+	.pending = ns16550_serial_pending,
+	.getc = ns16550_serial_getc,
+	.setbrg = ns16550_serial_setbrg,
+};
+
+static const struct udevice_id ns16550_serial_ids[] = {
+	{ .compatible = "ns16550" },
+	{ }
+};
+
+U_BOOT_DRIVER(serial_ns16550) = {
+	.name	= "serial_ns16550",
+	.id	= UCLASS_SERIAL,
+	.of_match = ns16550_serial_ids,
+	.ofdata_to_platdata = ns16550_serial_ofdata_to_platdata,
+	.platdata_auto_alloc_size = sizeof(struct ns16550_platdata),
+	.priv_auto_alloc_size = sizeof(struct NS16550),
+	.probe = ns16550_serial_probe,
+	.ops	= &ns16550_serial_ops,
+	.flags = DM_FLAG_PRE_RELOC,
+};
+#endif /* CONFIG_DM_SERIAL */
diff --git a/include/ns16550.h b/include/ns16550.h
index b106f8a..46ced93 100644
--- a/include/ns16550.h
+++ b/include/ns16550.h
@@ -23,6 +23,14 @@
 
 #include <linux/types.h>
 
+#ifdef CONFIG_DM_SERIAL
+/*
+ * For driver model we always use one byte per register, and sort out the
+ * differences in the driver
+ */
+#define CONFIG_SYS_NS16550_REG_SIZE (-1)
+#endif
+
 #if !defined(CONFIG_SYS_NS16550_REG_SIZE) || (CONFIG_SYS_NS16550_REG_SIZE == 0)
 #error "Please define NS16550 registers size."
 #elif defined(CONFIG_SYS_NS16550_MEM32)
@@ -37,6 +45,19 @@
 	unsigned char postpad_##x[-CONFIG_SYS_NS16550_REG_SIZE - 1];
 #endif
 
+/**
+ * struct ns16550_platdata - information about a NS16550 port
+ *
+ * @base:		Base register address
+ * @reg_shift:		Shift size of registers (0=byte, 1=16bit, 2=32bit...)
+ * @clock:		UART base clock speed in Hz
+ */
+struct ns16550_platdata {
+	unsigned char *base;
+	int reg_shift;
+	int clock;
+};
+
 struct NS16550 {
 	UART_REG(rbr);		/* 0 */
 	UART_REG(ier);		/* 1 */
@@ -67,6 +88,9 @@ struct NS16550 {
 	UART_REG(reg12);	/* 12*/
 	UART_REG(osc_12m_sel);	/* 13*/
 #endif
+#ifdef CONFIG_DM_SERIAL
+	struct ns16550_platdata *plat;
+#endif
 };
 
 #define thr rbr
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Simon Glass]

Some Tegra device tree files do not include information about the serial
ports. Add this and also add information about the input clock speed.

The console alias needs to be set up to indicate which port is used for
the console.

Also add a binding file since this is missing.

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

Changes in v3:
- Add new patch for tegra serial port details

Changes in v2: None

 arch/arm/dts/tegra114-dalmore.dts         |  1 +
 arch/arm/dts/tegra114.dtsi                | 40 +++++++++++++++++++++++++++++++
 arch/arm/dts/tegra124-jetson-tk1.dts      |  1 +
 arch/arm/dts/tegra124-venice2.dts         |  1 +
 arch/arm/dts/tegra124.dtsi                | 40 +++++++++++++++++++++++++++++++
 arch/arm/dts/tegra20-colibri_t20_iris.dts |  1 +
 arch/arm/dts/tegra20-harmony.dts          |  1 +
 arch/arm/dts/tegra20-medcom-wide.dts      |  1 +
 arch/arm/dts/tegra20-paz00.dts            |  1 +
 arch/arm/dts/tegra20-plutux.dts           |  1 +
 arch/arm/dts/tegra20-seaboard.dts         |  1 +
 arch/arm/dts/tegra20-tec.dts              |  1 +
 arch/arm/dts/tegra20-trimslice.dts        |  1 +
 arch/arm/dts/tegra20-ventana.dts          |  1 +
 arch/arm/dts/tegra20-whistler.dts         |  1 +
 arch/arm/dts/tegra20.dtsi                 | 15 ++++++++----
 arch/arm/dts/tegra30-beaver.dts           |  1 +
 arch/arm/dts/tegra30-cardhu.dts           |  1 +
 arch/arm/dts/tegra30-tamonten.dtsi        |  1 +
 arch/arm/dts/tegra30.dtsi                 | 40 +++++++++++++++++++++++++++++++
 20 files changed, 146 insertions(+), 5 deletions(-)

diff --git a/arch/arm/dts/tegra114-dalmore.dts b/arch/arm/dts/tegra114-dalmore.dts
index 435c01e..e2426ef 100644
--- a/arch/arm/dts/tegra114-dalmore.dts
+++ b/arch/arm/dts/tegra114-dalmore.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,dalmore", "nvidia,tegra114";
 
 	aliases {
+		console = &uart_d;
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
index 59434e0..f8d740b 100644
--- a/arch/arm/dts/tegra114.dtsi
+++ b/arch/arm/dts/tegra114.dtsi
@@ -116,6 +116,46 @@
 		status = "disabled";
 	};
 
+	uart_a: serial at 70006000 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006000 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 68 >;
+	};
+
+	uart_b: serial at 70006040 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006040 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 69 >;
+	};
+
+	uart_c: serial at 70006200 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006200 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 78 >;
+	};
+
+	uart_d: serial at 70006300 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006300 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 122 >;
+	};
+
+	uart_e: serial at 70006400 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006400 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 123 >;
+	};
+
 	spi at 7000d400 {
 		compatible = "nvidia,tegra114-spi";
 		reg = <0x7000d400 0x200>;
diff --git a/arch/arm/dts/tegra124-jetson-tk1.dts b/arch/arm/dts/tegra124-jetson-tk1.dts
index 464287e..368caaa 100644
--- a/arch/arm/dts/tegra124-jetson-tk1.dts
+++ b/arch/arm/dts/tegra124-jetson-tk1.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,jetson-tk1", "nvidia,tegra124";
 
 	aliases {
+		console = &uart_d;
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra124-venice2.dts b/arch/arm/dts/tegra124-venice2.dts
index f003413..4444155 100644
--- a/arch/arm/dts/tegra124-venice2.dts
+++ b/arch/arm/dts/tegra124-venice2.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,venice2", "nvidia,tegra124";
 
 	aliases {
+		console = &uart_a;
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra124.dtsi b/arch/arm/dts/tegra124.dtsi
index 4561c5f..5037036 100644
--- a/arch/arm/dts/tegra124.dtsi
+++ b/arch/arm/dts/tegra124.dtsi
@@ -126,6 +126,46 @@
 		status = "disabled";
 	};
 
+	uart_a: serial at 70006000 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006000 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 68 >;
+	};
+
+	uart_b: serial at 70006040 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006040 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 69 >;
+	};
+
+	uart_c: serial at 70006200 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006200 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 78 >;
+	};
+
+	uart_d: serial at 70006300 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006300 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 122 >;
+	};
+
+	uart_e: serial at 70006400 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006400 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 123 >;
+	};
+
 	spi at 7000d400 {
 		compatible = "nvidia,tegra124-spi", "nvidia,tegra114-spi";
 		reg = <0x7000d400 0x200>;
diff --git a/arch/arm/dts/tegra20-colibri_t20_iris.dts b/arch/arm/dts/tegra20-colibri_t20_iris.dts
index c0e54af..fdf966b 100644
--- a/arch/arm/dts/tegra20-colibri_t20_iris.dts
+++ b/arch/arm/dts/tegra20-colibri_t20_iris.dts
@@ -7,6 +7,7 @@
 	compatible = "toradex,t20", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_a;
 		usb0 = "/usb at c5008000";
 		usb1 = "/usb at c5000000";
 		usb2 = "/usb at c5004000";
diff --git a/arch/arm/dts/tegra20-harmony.dts b/arch/arm/dts/tegra20-harmony.dts
index b115f87..565045b 100644
--- a/arch/arm/dts/tegra20-harmony.dts
+++ b/arch/arm/dts/tegra20-harmony.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,harmony", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_d;
 		usb0 = "/usb at c5008000";
 		usb1 = "/usb at c5004000";
 		sdhci0 = "/sdhci at c8000600";
diff --git a/arch/arm/dts/tegra20-medcom-wide.dts b/arch/arm/dts/tegra20-medcom-wide.dts
index a9a07f9..b400ce8 100644
--- a/arch/arm/dts/tegra20-medcom-wide.dts
+++ b/arch/arm/dts/tegra20-medcom-wide.dts
@@ -7,6 +7,7 @@
 	compatible = "ad,medcom-wide", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_d;
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
 	};
diff --git a/arch/arm/dts/tegra20-paz00.dts b/arch/arm/dts/tegra20-paz00.dts
index 780203c..95b1ed4 100644
--- a/arch/arm/dts/tegra20-paz00.dts
+++ b/arch/arm/dts/tegra20-paz00.dts
@@ -7,6 +7,7 @@
 	compatible = "compal,paz00", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_a;
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
 		sdhci1 = "/sdhci at c8000000";
diff --git a/arch/arm/dts/tegra20-plutux.dts b/arch/arm/dts/tegra20-plutux.dts
index 20016f2..a4ec20f 100644
--- a/arch/arm/dts/tegra20-plutux.dts
+++ b/arch/arm/dts/tegra20-plutux.dts
@@ -7,6 +7,7 @@
 	compatible = "ad,plutux", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_d;
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
 	};
diff --git a/arch/arm/dts/tegra20-seaboard.dts b/arch/arm/dts/tegra20-seaboard.dts
index c0e2e1e..85d485a 100644
--- a/arch/arm/dts/tegra20-seaboard.dts
+++ b/arch/arm/dts/tegra20-seaboard.dts
@@ -11,6 +11,7 @@
 	};
 
 	aliases {
+		console = &uart_d;
 		/* This defines the order of our ports */
 		usb0 = "/usb at c5008000";
 		usb1 = "/usb at c5000000";
diff --git a/arch/arm/dts/tegra20-tec.dts b/arch/arm/dts/tegra20-tec.dts
index 4c1b08d..0e06100 100644
--- a/arch/arm/dts/tegra20-tec.dts
+++ b/arch/arm/dts/tegra20-tec.dts
@@ -7,6 +7,7 @@
 	compatible = "ad,tec", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_d;
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
 	};
diff --git a/arch/arm/dts/tegra20-trimslice.dts b/arch/arm/dts/tegra20-trimslice.dts
index ee31476..4506e8a 100644
--- a/arch/arm/dts/tegra20-trimslice.dts
+++ b/arch/arm/dts/tegra20-trimslice.dts
@@ -7,6 +7,7 @@
 	compatible = "compulab,trimslice", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_a;
 		usb0 = "/usb at c5008000";
 		usb1 = "/usb at c5000000";
 		sdhci0 = "/sdhci at c8000600";
diff --git a/arch/arm/dts/tegra20-ventana.dts b/arch/arm/dts/tegra20-ventana.dts
index 1a526ba..61b45f3 100644
--- a/arch/arm/dts/tegra20-ventana.dts
+++ b/arch/arm/dts/tegra20-ventana.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,ventana", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_d;
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
 		sdhci1 = "/sdhci at c8000400";
diff --git a/arch/arm/dts/tegra20-whistler.dts b/arch/arm/dts/tegra20-whistler.dts
index eb92264..6eb68b5 100644
--- a/arch/arm/dts/tegra20-whistler.dts
+++ b/arch/arm/dts/tegra20-whistler.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,whistler", "nvidia,tegra20";
 
 	aliases {
+		console = &uart_a;
 		i2c0 = "/i2c at 7000d000";
 		usb0 = "/usb at c5008000";
 		sdhci0 = "/sdhci at c8000600";
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index a524f6e..e3e2fbd 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -189,38 +189,43 @@
 		dma-channel = < 1 >;
 	};
 
-	serial at 70006000 {
+	uart_a: serial at 70006000 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006000 0x40>;
 		reg-shift = <2>;
+		clock-frequency = <216000000>;
 		interrupts = < 68 >;
 	};
 
-	serial at 70006040 {
+	uart_b: serial at 70006040 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006040 0x40>;
 		reg-shift = <2>;
+		clock-frequency = <216000000>;
 		interrupts = < 69 >;
 	};
 
-	serial at 70006200 {
+	uart_c: serial at 70006200 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006200 0x100>;
 		reg-shift = <2>;
+		clock-frequency = <216000000>;
 		interrupts = < 78 >;
 	};
 
-	serial at 70006300 {
+	uart_d: serial at 70006300 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006300 0x100>;
 		reg-shift = <2>;
+		clock-frequency = <216000000>;
 		interrupts = < 122 >;
 	};
 
-	serial at 70006400 {
+	uart_e: serial at 70006400 {
 		compatible = "nvidia,tegra20-uart";
 		reg = <0x70006400 0x100>;
 		reg-shift = <2>;
+		clock-frequency = <216000000>;
 		interrupts = < 123 >;
 	};
 
diff --git a/arch/arm/dts/tegra30-beaver.dts b/arch/arm/dts/tegra30-beaver.dts
index 85e62e9..79a9fff 100644
--- a/arch/arm/dts/tegra30-beaver.dts
+++ b/arch/arm/dts/tegra30-beaver.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,beaver", "nvidia,tegra30";
 
 	aliases {
+		console = &uart_a;
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra30-cardhu.dts b/arch/arm/dts/tegra30-cardhu.dts
index ea2cf76..3f829ba 100644
--- a/arch/arm/dts/tegra30-cardhu.dts
+++ b/arch/arm/dts/tegra30-cardhu.dts
@@ -7,6 +7,7 @@
 	compatible = "nvidia,cardhu", "nvidia,tegra30";
 
 	aliases {
+		console = &uart_a;
 		i2c0 = "/i2c at 7000d000";
 		i2c1 = "/i2c at 7000c000";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra30-tamonten.dtsi b/arch/arm/dts/tegra30-tamonten.dtsi
index 50d5762..e9ed61c 100644
--- a/arch/arm/dts/tegra30-tamonten.dtsi
+++ b/arch/arm/dts/tegra30-tamonten.dtsi
@@ -9,6 +9,7 @@
 	};
 
 	aliases {
+		console = &uart_d;
 		i2c0 = "/i2c at 7000c000";
 		i2c1 = "/i2c at 7000c700";
 		i2c2 = "/i2c at 7000c400";
diff --git a/arch/arm/dts/tegra30.dtsi b/arch/arm/dts/tegra30.dtsi
index 7be3791..ddd7f4a 100644
--- a/arch/arm/dts/tegra30.dtsi
+++ b/arch/arm/dts/tegra30.dtsi
@@ -122,6 +122,46 @@
 		status = "disabled";
 	};
 
+	uart_a: serial at 70006000 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006000 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 68 >;
+	};
+
+	uart_b: serial at 70006040 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006040 0x40>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 69 >;
+	};
+
+	uart_c: serial at 70006200 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006200 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 78 >;
+	};
+
+	uart_d: serial at 70006300 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006300 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 122 >;
+	};
+
+	uart_e: serial at 70006400 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006400 0x100>;
+		reg-shift = <2>;
+		clock-frequency = <408000000>;
+		interrupts = < 123 >;
+	};
+
 	spi at 7000d400 {
 		compatible = "nvidia,tegra30-slink", "nvidia,tegra20-slink";
 		reg = <0x7000d400 0x200>;
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren]

On 07/30/2014 03:49 AM, Simon Glass wrote:
> Some Tegra device tree files do not include information about the serial
> ports. Add this and also add information about the input clock speed.
>
> The console alias needs to be set up to indicate which port is used for
> the console.
>
> Also add a binding file since this is missing.

> diff --git a/arch/arm/dts/tegra114-dalmore.dts b/arch/arm/dts/tegra114-dalmore.dts
> index 435c01e..e2426ef 100644
> --- a/arch/arm/dts/tegra114-dalmore.dts
> +++ b/arch/arm/dts/tegra114-dalmore.dts
> @@ -7,6 +7,7 @@
>   	compatible = "nvidia,dalmore", "nvidia,tegra114";
>
>   	aliases {
> +		console = &uart_d;

I don't think that's a standard alias name. There was some recent 
discussion in the devicetree mailing list re: using some property in 
/chosen for this purpose instead. U-Boot and the kernel should use the 
same representation here.

> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi

> +	uart_a: serial at 70006000 {
> +		compatible = "nvidia,tegra20-uart";

This property needs to include both the specific HW (i.e. Tegra114) and 
any HW it's compatible with (i.e. Tegra20).

> +		reg = <0x70006000 0x40>;
> +		reg-shift = <2>;
> +		clock-frequency = <408000000>;

This isn't a property that's defined by the Tegra serial binding. This 
information should be obtained by looking up the relevant clock, and 
querying its rate.

> +		interrupts = < 68 >;
> +	};

For reference, here's the DT node for this UART in the kernel DT, which 
complies with the relevant binding document:

         uarta: serial at 70006000 {
                 compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
                 reg = <0x70006000 0x40>;
                 reg-shift = <2>;
                 interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
                 clocks = <&tegra_car TEGRA114_CLK_UARTA>;
                 resets = <&tegra_car 6>;
                 reset-names = "serial";
                 dmas = <&apbdma 8>, <&apbdma 8>;
                 dma-names = "rx", "tx";
                 status = "disabled";
         };

All the comment above apply to all the files in this patch.

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Simon Glass]

Hi Stephen,

On 31 July 2014 21:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>
>> Some Tegra device tree files do not include information about the serial
>> ports. Add this and also add information about the input clock speed.
>>
>> The console alias needs to be set up to indicate which port is used for
>> the console.
>>
>> Also add a binding file since this is missing.
>
>
>> diff --git a/arch/arm/dts/tegra114-dalmore.dts
>> b/arch/arm/dts/tegra114-dalmore.dts
>> index 435c01e..e2426ef 100644
>> --- a/arch/arm/dts/tegra114-dalmore.dts
>> +++ b/arch/arm/dts/tegra114-dalmore.dts
>> @@ -7,6 +7,7 @@
>>         compatible = "nvidia,dalmore", "nvidia,tegra114";
>>
>>         aliases {
>> +               console = &uart_d;
>
>
> I don't think that's a standard alias name. There was some recent discussion
> in the devicetree mailing list re: using some property in /chosen for this
> purpose instead. U-Boot and the kernel should use the same representation
> here.

This is U-Boot's approach at present, if we change it then we should
change it everywhere. I worry that 'chosen' is for Linux rather than
U-Boot and we might get very confused about what chosen is for?

>
>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>
>
>> +       uart_a: serial at 70006000 {
>> +               compatible = "nvidia,tegra20-uart";
>
>
> This property needs to include both the specific HW (i.e. Tegra114) and any
> HW it's compatible with (i.e. Tegra20).

So something like this?

compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";

>
>
>> +               reg = <0x70006000 0x40>;
>> +               reg-shift = <2>;
>> +               clock-frequency = <408000000>;
>
>
> This isn't a property that's defined by the Tegra serial binding. This
> information should be obtained by looking up the relevant clock, and
> querying its rate.

We can't do that in the ns16550 driver as yet since there is no
generic U-Boot clock infrastructure. I suspect that will come with
time.

>
>
>> +               interrupts = < 68 >;
>> +       };
>
>
> For reference, here's the DT node for this UART in the kernel DT, which
> complies with the relevant binding document:
>
>         uarta: serial at 70006000 {
>                 compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
>
>                 reg = <0x70006000 0x40>;
>                 reg-shift = <2>;
>                 interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>                 clocks = <&tegra_car TEGRA114_CLK_UARTA>;
>                 resets = <&tegra_car 6>;
>                 reset-names = "serial";
>                 dmas = <&apbdma 8>, <&apbdma 8>;
>                 dma-names = "rx", "tx";
>                 status = "disabled";
>         };
>
> All the comment above apply to all the files in this patch.

My intent was to make this work with a more generic binding for now -
ns16550 is a pretty standard thing and I thought I could avoid making
the driver Tegra-specific. Then we could allow many SoCs to use it.
Why does Tegra have its own binding in the kernel for this standard
UART?

Regards,
Simon

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren]

On 07/31/2014 04:10 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 31 July 2014 21:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>
>>> Some Tegra device tree files do not include information about the serial
>>> ports. Add this and also add information about the input clock speed.
>>>
>>> The console alias needs to be set up to indicate which port is used for
>>> the console.
>>>
>>> Also add a binding file since this is missing.
>>
>>
>>> diff --git a/arch/arm/dts/tegra114-dalmore.dts
>>> b/arch/arm/dts/tegra114-dalmore.dts
>>> index 435c01e..e2426ef 100644
>>> --- a/arch/arm/dts/tegra114-dalmore.dts
>>> +++ b/arch/arm/dts/tegra114-dalmore.dts
>>> @@ -7,6 +7,7 @@
>>>          compatible = "nvidia,dalmore", "nvidia,tegra114";
>>>
>>>          aliases {
>>> +               console = &uart_d;
>>
>>
>> I don't think that's a standard alias name. There was some recent discussion
>> in the devicetree mailing list re: using some property in /chosen for this
>> purpose instead. U-Boot and the kernel should use the same representation
>> here.
>
> This is U-Boot's approach at present,

That's not the U-Boot approach on Tegra boards before this patch. I do 
not want Tegra U-Boot do adopt any more U-Boot-specific 
not-really-DT-but-pretending-to-be bindings.

 > if we change it then we should
> change it everywhere. I worry that 'chosen' is for Linux rather than
> U-Boot and we might get very confused about what chosen is for?

That discussion should be had on the devicetree mailing list.

>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>>
>>
>>> +       uart_a: serial at 70006000 {
>>> +               compatible = "nvidia,tegra20-uart";
>>
>>
>> This property needs to include both the specific HW (i.e. Tegra114) and any
>> HW it's compatible with (i.e. Tegra20).
>
> So something like this?
>
> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";

Yes.

>>> +               reg = <0x70006000 0x40>;
>>> +               reg-shift = <2>;
>>> +               clock-frequency = <408000000>;
>>
>>
>> This isn't a property that's defined by the Tegra serial binding. This
>> information should be obtained by looking up the relevant clock, and
>> querying its rate.
>
> We can't do that in the ns16550 driver as yet since there is no
> generic U-Boot clock infrastructure. I suspect that will come with
> time.

The solution here is to put the clock infra-structure in place first. 
One thing I've learned from the kernel DT experience is that a lot of 
time would have been saved by putting the correct infra-structure in 
place first, then using it, rather than hacking around things the wrong 
way, then putting the infra-structure in place, then converting to it. 
That's a lot more work, and rather painful. Equally, if we don't just do 
the infra-structure right, there's really little guarantee that we'll 
ever convert to the correct approach. Just look at all the DT content in 
use in U-Boot that don't match the real DT bindings, even after it's 
been around years.

>> For reference, here's the DT node for this UART in the kernel DT, which
>> complies with the relevant binding document:
>>
>>          uarta: serial at 70006000 {
>>                  compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
>>
>>                  reg = <0x70006000 0x40>;
>>                  reg-shift = <2>;
>>                  interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>                  clocks = <&tegra_car TEGRA114_CLK_UARTA>;
>>                  resets = <&tegra_car 6>;
>>                  reset-names = "serial";
>>                  dmas = <&apbdma 8>, <&apbdma 8>;
>>                  dma-names = "rx", "tx";
>>                  status = "disabled";
>>          };
>>
>> All the comment above apply to all the files in this patch.
>
> My intent was to make this work with a more generic binding for now -
> ns16550 is a pretty standard thing and I thought I could avoid making
> the driver Tegra-specific. Then we could allow many SoCs to use it.
> Why does Tegra have its own binding in the kernel for this standard
> UART?

The HW is not a PC-style UART where all you care about is the 16550 
registers, and clocks/resets/DMA/... can be ignored or deferred to 
firmware to set up before DT-driven SW runs.

As an aside, /almost/ all reviewed DT bindings use DT properties of the 
form:

clocks = <&provider parameters>;

rather than:

clock-rate = <number>;

So, that aspect of the Tegra UART binding isn't anything remotely 
unusual/non-standard.

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Simon Glass]

Hi Stephen,

On 1 August 2014 00:06, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/31/2014 04:10 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 31 July 2014 21:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>>
>>>>
>>>> Some Tegra device tree files do not include information about the serial
>>>> ports. Add this and also add information about the input clock speed.
>>>>
>>>> The console alias needs to be set up to indicate which port is used for
>>>> the console.
>>>>
>>>> Also add a binding file since this is missing.
>>>
>>>
>>>
>>>> diff --git a/arch/arm/dts/tegra114-dalmore.dts
>>>> b/arch/arm/dts/tegra114-dalmore.dts
>>>> index 435c01e..e2426ef 100644
>>>> --- a/arch/arm/dts/tegra114-dalmore.dts
>>>> +++ b/arch/arm/dts/tegra114-dalmore.dts
>>>> @@ -7,6 +7,7 @@
>>>>          compatible = "nvidia,dalmore", "nvidia,tegra114";
>>>>
>>>>          aliases {
>>>> +               console = &uart_d;
>>>
>>>
>>>
>>> I don't think that's a standard alias name. There was some recent
>>> discussion
>>> in the devicetree mailing list re: using some property in /chosen for
>>> this
>>> purpose instead. U-Boot and the kernel should use the same representation
>>> here.
>>
>>
>> This is U-Boot's approach at present,
>
>
> That's not the U-Boot approach on Tegra boards before this patch. I do not
> want Tegra U-Boot do adopt any more U-Boot-specific
> not-really-DT-but-pretending-to-be bindings.
>
>
>> if we change it then we should
>>
>> change it everywhere. I worry that 'chosen' is for Linux rather than
>> U-Boot and we might get very confused about what chosen is for?
>
>
> That discussion should be had on the devicetree mailing list.

Please go ahead if you wish, but this is not a Linux issue. The
aliases are used for U-Boot and have been for some time.

>
>
>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>>>
>>>
>>>
>>>> +       uart_a: serial at 70006000 {
>>>> +               compatible = "nvidia,tegra20-uart";
>>>
>>>
>>>
>>> This property needs to include both the specific HW (i.e. Tegra114) and
>>> any
>>> HW it's compatible with (i.e. Tegra20).
>>
>>
>> So something like this?
>>
>> compatible = "nvidia,tegra114-uart", "nvidia,tegra20-uart";
>
>
> Yes.
>
>
>>>> +               reg = <0x70006000 0x40>;
>>>> +               reg-shift = <2>;
>>>> +               clock-frequency = <408000000>;
>>>
>>>
>>>
>>> This isn't a property that's defined by the Tegra serial binding. This
>>> information should be obtained by looking up the relevant clock, and
>>> querying its rate.
>>
>>
>> We can't do that in the ns16550 driver as yet since there is no
>> generic U-Boot clock infrastructure. I suspect that will come with
>> time.
>
>
> The solution here is to put the clock infra-structure in place first. One
> thing I've learned from the kernel DT experience is that a lot of time would
> have been saved by putting the correct infra-structure in place first, then
> using it, rather than hacking around things the wrong way, then putting the
> infra-structure in place, then converting to it. That's a lot more work, and
> rather painful. Equally, if we don't just do the infra-structure right,
> there's really little guarantee that we'll ever convert to the correct
> approach. Just look at all the DT content in use in U-Boot that don't match
> the real DT bindings, even after it's been around years.

OK, is there a plan to put this in place? Who is working on it?

>
>
>>> For reference, here's the DT node for this UART in the kernel DT, which
>>> complies with the relevant binding document:
>>>
>>>          uarta: serial at 70006000 {
>>>                  compatible = "nvidia,tegra114-uart",
>>> "nvidia,tegra20-uart";
>>>
>>>                  reg = <0x70006000 0x40>;
>>>                  reg-shift = <2>;
>>>                  interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>>                  clocks = <&tegra_car TEGRA114_CLK_UARTA>;
>>>                  resets = <&tegra_car 6>;
>>>                  reset-names = "serial";
>>>                  dmas = <&apbdma 8>, <&apbdma 8>;
>>>                  dma-names = "rx", "tx";
>>>                  status = "disabled";
>>>          };
>>>
>>> All the comment above apply to all the files in this patch.
>>
>>
>> My intent was to make this work with a more generic binding for now -
>> ns16550 is a pretty standard thing and I thought I could avoid making
>> the driver Tegra-specific. Then we could allow many SoCs to use it.
>> Why does Tegra have its own binding in the kernel for this standard
>> UART?
>
>
> The HW is not a PC-style UART where all you care about is the 16550
> registers, and clocks/resets/DMA/... can be ignored or deferred to firmware
> to set up before DT-driven SW runs.

I'm not sure what you are referring to here. Are you saying that
U-Boot needs to support these things? I agree it would be great to add
a generic clock/reset framework and have made considerable efforts in
Tegra towards this myself. But we don't have it yet.

> As an aside, /almost/ all reviewed DT bindings use DT properties of the
> form:
>
> clocks = <&provider parameters>;
>
> rather than:
>
> clock-rate = <number>;
>
> So, that aspect of the Tegra UART binding isn't anything remotely
> unusual/non-standard.

The great is the enemy of the good. In this case, I think I might just
leave the clock as a #define.

Regards,
Simon

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren]

On 08/01/2014 03:32 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 1 August 2014 00:06, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 07/31/2014 04:10 PM, Simon Glass wrote:
>>>
>>> Hi Stephen,
>>>
>>> On 31 July 2014 21:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>
>>>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>>>
>>>>>
>>>>> Some Tegra device tree files do not include information about the serial
>>>>> ports. Add this and also add information about the input clock speed.
>>>>>
>>>>> The console alias needs to be set up to indicate which port is used for
>>>>> the console.
>>>>>
>>>>> Also add a binding file since this is missing.
>>>>
>>>>
>>>>
>>>>> diff --git a/arch/arm/dts/tegra114-dalmore.dts
>>>>> b/arch/arm/dts/tegra114-dalmore.dts
>>>>> index 435c01e..e2426ef 100644
>>>>> --- a/arch/arm/dts/tegra114-dalmore.dts
>>>>> +++ b/arch/arm/dts/tegra114-dalmore.dts
>>>>> @@ -7,6 +7,7 @@
>>>>>           compatible = "nvidia,dalmore", "nvidia,tegra114";
>>>>>
>>>>>           aliases {
>>>>> +               console = &uart_d;
>>>>
>>>>
>>>>
>>>> I don't think that's a standard alias name. There was some recent
>>>> discussion
>>>> in the devicetree mailing list re: using some property in /chosen for
>>>> this
>>>> purpose instead. U-Boot and the kernel should use the same representation
>>>> here.
>>>
>>>
>>> This is U-Boot's approach at present,
>>
>>
>> That's not the U-Boot approach on Tegra boards before this patch. I do not
>> want Tegra U-Boot do adopt any more U-Boot-specific
>> not-really-DT-but-pretending-to-be bindings.
>>
>>
>>> if we change it then we should
>>>
>>> change it everywhere. I worry that 'chosen' is for Linux rather than
>>> U-Boot and we might get very confused about what chosen is for?
>>
>>
>> That discussion should be had on the devicetree mailing list.
>
> Please go ahead if you wish, but this is not a Linux issue. The
> aliases are used for U-Boot and have been for some time.

No, it's not a Linux issue, it's DT issue.

DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD, 
Barebox, ... (all of which use DT). As such, all the DT bindings MUST be 
discussed on the devicetree mailing list.

Since you're the author of the patch, it's your responsibility to have 
that discussion.

>>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>>>>> +       uart_a: serial at 70006000 {
>>>>> +               compatible = "nvidia,tegra20-uart";
>>>>> +               reg = <0x70006000 0x40>;
>>>>> +               reg-shift = <2>;
>>>>> +               clock-frequency = <408000000>;
>>>>
>>>> This isn't a property that's defined by the Tegra serial binding. This
>>>> information should be obtained by looking up the relevant clock, and
>>>> querying its rate.
>>>
>>> We can't do that in the ns16550 driver as yet since there is no
>>> generic U-Boot clock infrastructure. I suspect that will come with
>>> time.
>>
>> The solution here is to put the clock infra-structure in place first. One
>> thing I've learned from the kernel DT experience is that a lot of time would
>> have been saved by putting the correct infra-structure in place first, then
>> using it, rather than hacking around things the wrong way, then putting the
>> infra-structure in place, then converting to it. That's a lot more work, and
>> rather painful. Equally, if we don't just do the infra-structure right,
>> there's really little guarantee that we'll ever convert to the correct
>> approach. Just look at all the DT content in use in U-Boot that don't match
>> the real DT bindings, even after it's been around years.
>
> OK, is there a plan to put this in place? Who is working on it?

I don't know whether anyone is or not. However, the fact that nobody is 
working on a clock driver is no excuse for using the incorrect DT 
bindings in order to hack around its lack of existence.

>>>> For reference, here's the DT node for this UART in the kernel DT, which
>>>> complies with the relevant binding document:
>>>>
>>>>           uarta: serial at 70006000 {
>>>>                   compatible = "nvidia,tegra114-uart",
>>>> "nvidia,tegra20-uart";
>>>>
>>>>                   reg = <0x70006000 0x40>;
>>>>                   reg-shift = <2>;
>>>>                   interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>>>                   clocks = <&tegra_car TEGRA114_CLK_UARTA>;
>>>>                   resets = <&tegra_car 6>;
>>>>                   reset-names = "serial";
>>>>                   dmas = <&apbdma 8>, <&apbdma 8>;
>>>>                   dma-names = "rx", "tx";
>>>>                   status = "disabled";
>>>>           };
>>>>
>>>> All the comment above apply to all the files in this patch.
>>>
>>>
>>> My intent was to make this work with a more generic binding for now -
>>> ns16550 is a pretty standard thing and I thought I could avoid making
>>> the driver Tegra-specific. Then we could allow many SoCs to use it.
>>> Why does Tegra have its own binding in the kernel for this standard
>>> UART?
>>
>> The HW is not a PC-style UART where all you care about is the 16550
>> registers, and clocks/resets/DMA/... can be ignored or deferred to firmware
>> to set up before DT-driven SW runs.
>
> I'm not sure what you are referring to here. Are you saying that
> U-Boot needs to support these things? I agree it would be great to add
> a generic clock/reset framework and have made considerable efforts in
> Tegra towards this myself. But we don't have it yet.

Yes.

Part of the decision to use DT is to use *the* DT bindings, not 
something that looks like DT.

If you want to move the serial port information into DT, and part of 
doing so requires extracting clock-related information from DT, and that 
in turn requires a clock driver to do so, then yes, U-Boot needs to have 
that clock driver implemented before you can move the serial port 
information into DT.

>> As an aside, /almost/ all reviewed DT bindings use DT properties of the
>> form:
>>
>> clocks = <&provider parameters>;
>>
>> rather than:
>>
>> clock-rate = <number>;
>>
>> So, that aspect of the Tegra UART binding isn't anything remotely
>> unusual/non-standard.
>
> The great is the enemy of the good. In this case, I think I might just
> leave the clock as a #define.

That probably makes sense.

To be honest, I'm not sure that using DT is going to buy U-Boot much, or 
even the kernel in retrospect.

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Simon Glass]

Hi Stephen,

On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/01/2014 03:32 PM, Simon Glass wrote:
>>
>> Hi Stephen,
>>
>> On 1 August 2014 00:06, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>
>>> On 07/31/2014 04:10 PM, Simon Glass wrote:
>>>>
>>>>
>>>> Hi Stephen,
>>>>
>>>> On 31 July 2014 21:16, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>>>>
>>>>>
>>>>> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> Some Tegra device tree files do not include information about the
>>>>>> serial
>>>>>> ports. Add this and also add information about the input clock speed.
>>>>>>
>>>>>> The console alias needs to be set up to indicate which port is used
>>>>>> for
>>>>>> the console.
>>>>>>
>>>>>> Also add a binding file since this is missing.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> diff --git a/arch/arm/dts/tegra114-dalmore.dts
>>>>>> b/arch/arm/dts/tegra114-dalmore.dts
>>>>>> index 435c01e..e2426ef 100644
>>>>>> --- a/arch/arm/dts/tegra114-dalmore.dts
>>>>>> +++ b/arch/arm/dts/tegra114-dalmore.dts
>>>>>> @@ -7,6 +7,7 @@
>>>>>>           compatible = "nvidia,dalmore", "nvidia,tegra114";
>>>>>>
>>>>>>           aliases {
>>>>>> +               console = &uart_d;
>>>>>
>>>>>
>>>>>
>>>>>
>>>>> I don't think that's a standard alias name. There was some recent
>>>>> discussion
>>>>> in the devicetree mailing list re: using some property in /chosen for
>>>>> this
>>>>> purpose instead. U-Boot and the kernel should use the same
>>>>> representation
>>>>> here.
>>>>
>>>>
>>>>
>>>> This is U-Boot's approach at present,
>>>
>>>
>>>
>>> That's not the U-Boot approach on Tegra boards before this patch. I do
>>> not
>>> want Tegra U-Boot do adopt any more U-Boot-specific
>>> not-really-DT-but-pretending-to-be bindings.
>>>
>>>
>>>> if we change it then we should
>>>>
>>>> change it everywhere. I worry that 'chosen' is for Linux rather than
>>>> U-Boot and we might get very confused about what chosen is for?
>>>
>>>
>>>
>>> That discussion should be had on the devicetree mailing list.
>>
>>
>> Please go ahead if you wish, but this is not a Linux issue. The
>> aliases are used for U-Boot and have been for some time.
>
>
> No, it's not a Linux issue, it's DT issue.

OK agreed on that then.

>
> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
> discussed on the devicetree mailing list.
>
> Since you're the author of the patch, it's your responsibility to have that
> discussion.

Are you referring to the linux,stdout-path discussion, or something
more DT-generic?I suppose we could have a 'u-boot,console' for our
part. But in any case you are talking about code and a convention that
is already in mainline U-Boot. While I accept that we might change to
something DT-generic if Linux points the way to something better, I
don't want to stop using it just because Linux hasn't decided yet. The
early console stuff and early debug UART stuff in Linux is not yet a
shining example of perfection.

That said, it's a good time to adopt 'u-boot,console' if that's what we need?

>
>>>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>>>>>> +       uart_a: serial at 70006000 {
>>>>>> +               compatible = "nvidia,tegra20-uart";
>>>>>> +               reg = <0x70006000 0x40>;
>>>>>> +               reg-shift = <2>;
>>>>>> +               clock-frequency = <408000000>;
>>>>>
>>>>>
>>>>> This isn't a property that's defined by the Tegra serial binding. This
>>>>> information should be obtained by looking up the relevant clock, and
>>>>> querying its rate.
>>>>
>>>>
>>>> We can't do that in the ns16550 driver as yet since there is no
>>>> generic U-Boot clock infrastructure. I suspect that will come with
>>>> time.
>>>
>>>
>>> The solution here is to put the clock infra-structure in place first. One
>>> thing I've learned from the kernel DT experience is that a lot of time
>>> would
>>> have been saved by putting the correct infra-structure in place first,
>>> then
>>> using it, rather than hacking around things the wrong way, then putting
>>> the
>>> infra-structure in place, then converting to it. That's a lot more work,
>>> and
>>> rather painful. Equally, if we don't just do the infra-structure right,
>>> there's really little guarantee that we'll ever convert to the correct
>>> approach. Just look at all the DT content in use in U-Boot that don't
>>> match
>>> the real DT bindings, even after it's been around years.
>>
>>
>> OK, is there a plan to put this in place? Who is working on it?
>
>
> I don't know whether anyone is or not. However, the fact that nobody is
> working on a clock driver is no excuse for using the incorrect DT bindings
> in order to hack around its lack of existence.

Actually the bindings are correct, you are referring to a single new
addition which is the input clock speed of the UART. This information
is needed in U-Boot because it does not have generic clock
infrastructure (and since no one is working on clock infrastructure I
guess we can ignore it for this discussion). See below.

>
>
>>>>> For reference, here's the DT node for this UART in the kernel DT, which
>>>>> complies with the relevant binding document:
>>>>>
>>>>>           uarta: serial at 70006000 {
>>>>>                   compatible = "nvidia,tegra114-uart",
>>>>> "nvidia,tegra20-uart";
>>>>>
>>>>>                   reg = <0x70006000 0x40>;
>>>>>                   reg-shift = <2>;
>>>>>                   interrupts = <GIC_SPI 36 IRQ_TYPE_LEVEL_HIGH>;
>>>>>                   clocks = <&tegra_car TEGRA114_CLK_UARTA>;
>>>>>                   resets = <&tegra_car 6>;
>>>>>                   reset-names = "serial";
>>>>>                   dmas = <&apbdma 8>, <&apbdma 8>;
>>>>>                   dma-names = "rx", "tx";
>>>>>                   status = "disabled";
>>>>>           };
>>>>>
>>>>> All the comment above apply to all the files in this patch.
>>>>
>>>>
>>>>
>>>> My intent was to make this work with a more generic binding for now -
>>>> ns16550 is a pretty standard thing and I thought I could avoid making
>>>> the driver Tegra-specific. Then we could allow many SoCs to use it.
>>>> Why does Tegra have its own binding in the kernel for this standard
>>>> UART?
>>>
>>>
>>> The HW is not a PC-style UART where all you care about is the 16550
>>> registers, and clocks/resets/DMA/... can be ignored or deferred to
>>> firmware
>>> to set up before DT-driven SW runs.
>>
>>
>> I'm not sure what you are referring to here. Are you saying that
>> U-Boot needs to support these things? I agree it would be great to add
>> a generic clock/reset framework and have made considerable efforts in
>> Tegra towards this myself. But we don't have it yet.
>
>
> Yes.
>
> Part of the decision to use DT is to use *the* DT bindings, not something
> that looks like DT.
>
> If you want to move the serial port information into DT, and part of doing
> so requires extracting clock-related information from DT, and that in turn
> requires a clock driver to do so, then yes, U-Boot needs to have that clock
> driver implemented before you can move the serial port information into DT.

Again, your objection is purely about the clock-frequency property I
think. The binding is otherwise correct, right?

>
>
>>> As an aside, /almost/ all reviewed DT bindings use DT properties of the
>>> form:
>>>
>>> clocks = <&provider parameters>;
>>>
>>> rather than:
>>>
>>> clock-rate = <number>;
>>>
>>> So, that aspect of the Tegra UART binding isn't anything remotely
>>> unusual/non-standard.
>>
>>
>> The great is the enemy of the good. In this case, I think I might just
>> leave the clock as a #define.
>
>
> That probably makes sense.
>
> To be honest, I'm not sure that using DT is going to buy U-Boot much, or
> even the kernel in retrospect.

It's not that bad :-)

The only option I can think of given your objection to the
clock-frequency property is to come up with a tegra driver which
mostly uses ns16550 but provides the clock frequency from a CONFIG. I
think that will work. I'll update and resend.

Regards,
Simon

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren]

On 08/04/2014 04:43 AM, Simon Glass wrote:
> On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
...
>> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
>> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
>> discussed on the devicetree mailing list.
>>
>> Since you're the author of the patch, it's your responsibility to have that
>> discussion.
>
> Are you referring to the linux,stdout-path discussion, or something
> more DT-generic?I suppose we could have a 'u-boot,console' for our
> part. But in any case you are talking about code and a convention that
> is already in mainline U-Boot.

I'm saying that any and all additions or changes to DT schemas/bindings 
must be discussed on the devicetree mailing list, not made/reviewed in 
isolation on only the U-Boot mailing list.

 > While I accept that we might change to
> something DT-generic if Linux points the way to something better, I
> don't want to stop using it just because Linux hasn't decided yet. The
> early console stuff and early debug UART stuff in Linux is not yet a
> shining example of perfection.

I strongly believe that if U-Boot continues to use DT, the current DT 
usage in U-Boot needs to be actively moved in line with the bindings 
that the Linux kernel, Barebox, FreeBSD, ... use. I'd prefer this to 
happen even before U-Boot starts making additional use of DT, so the 
conversion doesn't get forgotten. However, I suppose it's a bit 
draconian to prevent further usage until the existing usage is cleaned 
up, except where new usage introduces additional dependencies on any 
current usage that's inconsistent with the standard bindings.

> That said, it's a good time to adopt 'u-boot,console' if that's what we need?

It's certainly a good time to start that discussion on the devicetree 
mailing list, and get such a new property reviewed/ack'd there.

>>>>>>> diff --git a/arch/arm/dts/tegra114.dtsi b/arch/arm/dts/tegra114.dtsi
>>>>>>> +       uart_a: serial at 70006000 {
>>>>>>> +               compatible = "nvidia,tegra20-uart";
>>>>>>> +               reg = <0x70006000 0x40>;
>>>>>>> +               reg-shift = <2>;
>>>>>>> +               clock-frequency = <408000000>;
>>>>>>
>>>>>>
>>>>>> This isn't a property that's defined by the Tegra serial binding. This
>>>>>> information should be obtained by looking up the relevant clock, and
>>>>>> querying its rate.
>>>>>
>>>>>
>>>>> We can't do that in the ns16550 driver as yet since there is no
>>>>> generic U-Boot clock infrastructure. I suspect that will come with
>>>>> time.
>>>>
>>>>
>>>> The solution here is to put the clock infra-structure in place first. One
>>>> thing I've learned from the kernel DT experience is that a lot of time
>>>> would
>>>> have been saved by putting the correct infra-structure in place first,
>>>> then
>>>> using it, rather than hacking around things the wrong way, then putting
>>>> the
>>>> infra-structure in place, then converting to it. That's a lot more work,
>>>> and
>>>> rather painful. Equally, if we don't just do the infra-structure right,
>>>> there's really little guarantee that we'll ever convert to the correct
>>>> approach. Just look at all the DT content in use in U-Boot that don't
>>>> match
>>>> the real DT bindings, even after it's been around years.
>>>
>>>
>>> OK, is there a plan to put this in place? Who is working on it?
>>
>>
>> I don't know whether anyone is or not. However, the fact that nobody is
>> working on a clock driver is no excuse for using the incorrect DT bindings
>> in order to hack around its lack of existence.
>
> Actually the bindings are correct, you are referring to a single new
> addition which is the input clock speed of the UART.

That property (or the definition-of/schema-for it) is part of the 
bindings. Unless that addition is reviewed in the appropriate place, I 
think it's legitimate to refer the such a change to the binding as 
incorrect.

>> If you want to move the serial port information into DT, and part of doing
>> so requires extracting clock-related information from DT, and that in turn
>> requires a clock driver to do so, then yes, U-Boot needs to have that clock
>> driver implemented before you can move the serial port information into DT.
>
> Again, your objection is purely about the clock-frequency property I
> think. The binding is otherwise correct, right?

I think that was the primary issue, yes. If the U-Boot usage is 
consistent with Documentation/devicetree/bindings/serial/ of-serial.txt 
and/or nvidia,tegra20-hsuart.txt (as currently hosted in the Linux 
kernel source tree), then it's fine.

...
>> To be honest, I'm not sure that using DT is going to buy U-Boot much, or
>> even the kernel in retrospect.
>
> It's not that bad :-)

My experience working on DT support in the kernel says otherwise, but 
I'm happy to disagree on that point for now:-)

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Tom Rini]

On Mon, Aug 04, 2014 at 11:47:56AM -0600, Stephen Warren wrote:
> On 08/04/2014 04:43 AM, Simon Glass wrote:
> >On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
> ...
> >>DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
> >>Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
> >>discussed on the devicetree mailing list.
> >>
> >>Since you're the author of the patch, it's your responsibility to have that
> >>discussion.
> >
> >Are you referring to the linux,stdout-path discussion, or something
> >more DT-generic?I suppose we could have a 'u-boot,console' for our
> >part. But in any case you are talking about code and a convention that
> >is already in mainline U-Boot.
> 
> I'm saying that any and all additions or changes to DT
> schemas/bindings must be discussed on the devicetree mailing list,
> not made/reviewed in isolation on only the U-Boot mailing list.
> 
> > While I accept that we might change to
> >something DT-generic if Linux points the way to something better, I
> >don't want to stop using it just because Linux hasn't decided yet. The
> >early console stuff and early debug UART stuff in Linux is not yet a
> >shining example of perfection.
> 
> I strongly believe that if U-Boot continues to use DT, the current
> DT usage in U-Boot needs to be actively moved in line with the
> bindings that the Linux kernel, Barebox, FreeBSD, ... use. I'd
> prefer this to happen even before U-Boot starts making additional
> use of DT, so the conversion doesn't get forgotten. However, I
> suppose it's a bit draconian to prevent further usage until the
> existing usage is cleaned up, except where new usage introduces
> additional dependencies on any current usage that's inconsistent
> with the standard bindings.

I don't disagree (and at ELC I was trying to see who, if anyone, had
been reaching out to the FreeBSD folks since their DT files were very
far from what Linux uses afaict for am33xx), but I'm also not seeing
anything from non-Linux kernel folks on the devicetrees ML, or at least
my quick search failed.

-- 
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/20140804/8a41703d/attachment.pgp>

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Jeroen Hofstee]

Hello Tom, +Warner, +Julien Grall,

On 04-08-14 20:11, Tom Rini wrote:
> On Mon, Aug 04, 2014 at 11:47:56AM -0600, Stephen Warren wrote:
>> On 08/04/2014 04:43 AM, Simon Glass wrote:
>>> On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> ...
>>>> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
>>>> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
>>>> discussed on the devicetree mailing list.
>>>>
>>>> Since you're the author of the patch, it's your responsibility to have that
>>>> discussion.
>>> Are you referring to the linux,stdout-path discussion, or something
>>> more DT-generic?I suppose we could have a 'u-boot,console' for our
>>> part. But in any case you are talking about code and a convention that
>>> is already in mainline U-Boot.
>> I'm saying that any and all additions or changes to DT
>> schemas/bindings must be discussed on the devicetree mailing list,
>> not made/reviewed in isolation on only the U-Boot mailing list.
>>
>>> While I accept that we might change to
>>> something DT-generic if Linux points the way to something better, I
>>> don't want to stop using it just because Linux hasn't decided yet. The
>>> early console stuff and early debug UART stuff in Linux is not yet a
>>> shining example of perfection.
>> I strongly believe that if U-Boot continues to use DT, the current
>> DT usage in U-Boot needs to be actively moved in line with the
>> bindings that the Linux kernel, Barebox, FreeBSD, ... use. I'd
>> prefer this to happen even before U-Boot starts making additional
>> use of DT, so the conversion doesn't get forgotten. However, I
>> suppose it's a bit draconian to prevent further usage until the
>> existing usage is cleaned up, except where new usage introduces
>> additional dependencies on any current usage that's inconsistent
>> with the standard bindings.
> I don't disagree (and at ELC I was trying to see who, if anyone, had
> been reaching out to the FreeBSD folks since their DT files were very
> far from what Linux uses afaict for am33xx), but I'm also not seeing
> anything from non-Linux kernel folks on the devicetrees ML, or at least
> my quick search failed.

I am not actually following this topic, but FreeBSD folks do
try to support unmodified dft as far as possible afaik, see e.g. [1].

Regards,
Jeroen

[1] http://lists.freebsd.org/pipermail/freebsd-arm/2014-March/007634.html

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Simon Glass]

Hi Stephen,

On 4 August 2014 11:47, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> On 08/04/2014 04:43 AM, Simon Glass wrote:
>>
>> On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>
> ...
>
>>> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
>>> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
>>> discussed on the devicetree mailing list.
>>>
>>> Since you're the author of the patch, it's your responsibility to have that
>>> discussion.
>>
>>
>> Are you referring to the linux,stdout-path discussion, or something
>> more DT-generic?I suppose we could have a 'u-boot,console' for our
>> part. But in any case you are talking about code and a convention that
>> is already in mainline U-Boot.
>
>
> I'm saying that any and all additions or changes to DT schemas/bindings must be discussed on the devicetree mailing list, not made/reviewed in isolation on only the U-Boot mailing list.
>
>
> > While I accept that we might change to
>>
>> something DT-generic if Linux points the way to something better, I
>> don't want to stop using it just because Linux hasn't decided yet. The
>> early console stuff and early debug UART stuff in Linux is not yet a
>> shining example of perfection.
>
>
> I strongly believe that if U-Boot continues to use DT, the current DT usage in U-Boot needs to be actively moved in line with the bindings that the Linux kernel, Barebox, FreeBSD, ... use. I'd prefer this to happen even before U-Boot starts making additional use of DT, so the conversion doesn't get forgotten. However, I suppose it's a bit draconian to prevent further usage until the existing usage is cleaned up, except where new usage introduces additional dependencies on any current usage that's inconsistent with the standard bindings.
>
>
>> That said, it's a good time to adopt 'u-boot,console' if that's what we need?
>
>
> It's certainly a good time to start that discussion on the devicetree mailing list, and get such a new property reviewed/ack'd there.

[side note: You will be aware that I have expended considerable effort
getting agreement on bindings. I used to copy all DT patches to that
mailing list, but I can't recall getting a reply that often. Also note
that U-Boot's use of DT pre-dated the kernel with many subsystems
(e.g. the request to retrofit clock bindings after the code was
already written). Yes, DT bindings should be common across all
platforms, but where subsystems don't exist in U-Boot I feel the
approach of 'do nothing until someone writes a new subsystem' might
just be a recipe for inaction/no progress. Better to iterate towards
perfection than never move]

So to be clear, with the clock-frequency property moved back to being
hard-coded in the CONFIG, your remaining objection is that the console
alias, which is already used in U-Boot, should be agreed with the
devicetree mailing list? Is that right?

Regards,
Simon

[U-Boot] [PATCH v3 14/16] tegra: dts: Add serial port details

[Stephen Warren]

On 08/04/2014 02:14 PM, Simon Glass wrote:
> Hi Stephen,
>
> On 4 August 2014 11:47, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> On 08/04/2014 04:43 AM, Simon Glass wrote:
>>>
>>> On 1 August 2014 15:50, Stephen Warren <swarren@wwwdotorg.org> wrote:
>>
>> ...
>>
>>>> DT schemas/bindings MUST be identical between U-Boot, Linux, FreeBSD,
>>>> Barebox, ... (all of which use DT). As such, all the DT bindings MUST be
>>>> discussed on the devicetree mailing list.
>>>>
>>>> Since you're the author of the patch, it's your responsibility to have that
>>>> discussion.
>>>
>>>
>>> Are you referring to the linux,stdout-path discussion, or something
>>> more DT-generic?I suppose we could have a 'u-boot,console' for our
>>> part. But in any case you are talking about code and a convention that
>>> is already in mainline U-Boot.
>>
>>
>> I'm saying that any and all additions or changes to DT schemas/bindings must be discussed on the devicetree mailing list, not made/reviewed in isolation on only the U-Boot mailing list.
>>
>>
>>> While I accept that we might change to
>>>
>>> something DT-generic if Linux points the way to something better, I
>>> don't want to stop using it just because Linux hasn't decided yet. The
>>> early console stuff and early debug UART stuff in Linux is not yet a
>>> shining example of perfection.
>>
>>
>> I strongly believe that if U-Boot continues to use DT, the current DT usage in U-Boot needs to be actively moved in line with the bindings that the Linux kernel, Barebox, FreeBSD, ... use. I'd prefer this to happen even before U-Boot starts making additional use of DT, so the conversion doesn't get forgotten. However, I suppose it's a bit draconian to prevent further usage until the existing usage is cleaned up, except where new usage introduces additional dependencies on any current usage that's inconsistent with the standard bindings.
>>
>>
>>> That said, it's a good time to adopt 'u-boot,console' if that's what we need?
>>
>>
>> It's certainly a good time to start that discussion on the devicetree mailing list, and get such a new property reviewed/ack'd there.
>
> [side note: You will be aware that I have expended considerable effort
> getting agreement on bindings. I used to copy all DT patches to that
> mailing list, but I can't recall getting a reply that often. Also note
> that U-Boot's use of DT pre-dated the kernel with many subsystems
> (e.g. the request to retrofit clock bindings after the code was
> already written). Yes, DT bindings should be common across all
> platforms, but where subsystems don't exist in U-Boot I feel the
> approach of 'do nothing until someone writes a new subsystem' might
> just be a recipe for inaction/no progress. Better to iterate towards
> perfection than never move]

Do nothing isn't the right approach.

Invent something else without co-ordinating it isn't the right approach.

Proposing the DT bindings for that subsystem (even if the kernel doesn't 
implement code that handles the binding yet) and getting that merged 
into the DT binding documentation repository (which just happens to be 
part of the Linux kernel tree right now) is the only approach that makes 
sense.

> So to be clear, with the clock-frequency property moved back to being
> hard-coded in the CONFIG, your remaining objection is that the console
> alias, which is already used in U-Boot, should be agreed with the
> devicetree mailing list? Is that right?

IIRC, yes.

[U-Boot] [PATCH v3 15/16] RFC: dm: tegra: Enable driver model for serial

[Simon Glass]

Use driver model for serial ports (for test and comment only).

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

Changes in v3:
- Add new patch to enable driver model for seial on tegra

Changes in v2: None

 include/configs/tegra-common.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h
index 6eb0dd9..e0779f1 100644
--- a/include/configs/tegra-common.h
+++ b/include/configs/tegra-common.h
@@ -22,6 +22,9 @@
 #define CONFIG_DM
 #define CONFIG_CMD_DM
 #define CONFIG_DM_GPIO
+#ifndef CONFIG_SPL_BUILD
+#define CONFIG_DM_SERIAL
+#endif
 
 #define CONFIG_SYS_TIMER_RATE		1000000
 #define CONFIG_SYS_TIMER_COUNTER	NV_PA_TMRUS_BASE
@@ -42,14 +45,17 @@
  * Size of malloc() pool
  */
 #define CONFIG_SYS_MALLOC_LEN		(4 << 20)	/* 4MB  */
+#define CONFIG_SYS_MALLOC_F_LEN	(1 << 10)
 
 /*
  * NS16550 Configuration
  */
-#define CONFIG_SYS_NS16550
+#ifdef CONFIG_SPL_BUILD
 #define CONFIG_SYS_NS16550_SERIAL
 #define CONFIG_SYS_NS16550_REG_SIZE	(-4)
 #define CONFIG_SYS_NS16550_CLK		V_NS16550_CLK
+#endif
+#define CONFIG_SYS_NS16550
 
 /*
  * Common HW configuration.
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 15/16] RFC: dm: tegra: Enable driver model for serial

[Stephen Warren]

On 07/30/2014 03:49 AM, Simon Glass wrote:
> Use driver model for serial ports (for test and comment only).

A couple of nits:

> diff --git a/include/configs/tegra-common.h b/include/configs/tegra-common.h

>   #define CONFIG_SYS_MALLOC_LEN		(4 << 20)	/* 4MB  */
> +#define CONFIG_SYS_MALLOC_F_LEN	(1 << 10)

README says:

                 Pre-relocation malloc() is only supported on sandbox
                @present but is fairly easy to enable for other archs.

I assume that should be deleted now.

>
>   /*
>    * NS16550 Configuration
>    */
> -#define CONFIG_SYS_NS16550
> +#ifdef CONFIG_SPL_BUILD
>   #define CONFIG_SYS_NS16550_SERIAL
>   #define CONFIG_SYS_NS16550_REG_SIZE	(-4)
>   #define CONFIG_SYS_NS16550_CLK		V_NS16550_CLK
> +#endif
> +#define CONFIG_SYS_NS16550

Leaving that define in the same place would simplify the diff.

[U-Boot] [PATCH v3 15/16] RFC: dm: tegra: Enable driver model for serial

[Simon Glass]

Hi Stephen,

On 31 July 2014 21:18, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>
>> Use driver model for serial ports (for test and comment only).
>
>
> A couple of nits:
>
>> diff --git a/include/configs/tegra-common.h
>> b/include/configs/tegra-common.h
>
>
>>   #define CONFIG_SYS_MALLOC_LEN         (4 << 20)       /* 4MB  */
>> +#define CONFIG_SYS_MALLOC_F_LEN        (1 << 10)
>
>
> README says:
>
>                 Pre-relocation malloc() is only supported on sandbox
>                 at present but is fairly easy to enable for other archs.
>
> I assume that should be deleted now.

There's another series ahead of this patch which changes this message.

>
>
>>
>>   /*
>>    * NS16550 Configuration
>>    */
>> -#define CONFIG_SYS_NS16550
>> +#ifdef CONFIG_SPL_BUILD
>>   #define CONFIG_SYS_NS16550_SERIAL
>>   #define CONFIG_SYS_NS16550_REG_SIZE   (-4)
>>   #define CONFIG_SYS_NS16550_CLK                V_NS16550_CLK
>> +#endif
>> +#define CONFIG_SYS_NS16550
>
>
> Leaving that define in the same place would simplify the diff.

OK, will do.

Regards,
Simon

[U-Boot] [PATCH v3 16/16] dm: tegra: Use V_NS16550_CLK only in SPL builds

[Simon Glass]

Since Tegra now uses driver model for serial, adjust the definition of
V_NS16550_CLK so that it is clear that this is only used for SPL.

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

Changes in v3:
- Add new patch to use V_NS16550_CLK only in SPL builds

Changes in v2: None

 include/configs/tegra124-common.h | 2 ++
 include/configs/tegra20-common.h  | 2 ++
 include/configs/tegra30-common.h  | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/include/configs/tegra124-common.h b/include/configs/tegra124-common.h
index 61e5026..8c5842e 100644
--- a/include/configs/tegra124-common.h
+++ b/include/configs/tegra124-common.h
@@ -16,7 +16,9 @@
 /*
  * NS16550 Configuration
  */
+#ifdef CONFIG_SPL_BUILD
 #define V_NS16550_CLK		408000000	/* 408MHz (pllp_out0) */
+#endif
 
 /* Environment information, boards can override if required */
 #define CONFIG_LOADADDR		0x80408000	/* def. location for kernel */
diff --git a/include/configs/tegra20-common.h b/include/configs/tegra20-common.h
index 21bf977..4f8706a 100644
--- a/include/configs/tegra20-common.h
+++ b/include/configs/tegra20-common.h
@@ -22,7 +22,9 @@
 /*
  * NS16550 Configuration
  */
+#ifdef CONFIG_SPL_BUILD
 #define V_NS16550_CLK		216000000	/* 216MHz (pllp_out0) */
+#endif
 
 /* Environment information, boards can override if required */
 #define CONFIG_LOADADDR		0x00408000	/* def. location for kernel */
diff --git a/include/configs/tegra30-common.h b/include/configs/tegra30-common.h
index 443c842..6d2558c 100644
--- a/include/configs/tegra30-common.h
+++ b/include/configs/tegra30-common.h
@@ -21,7 +21,9 @@
 /*
  * NS16550 Configuration
  */
+#ifdef CONFIG_SPL_BUILD
 #define V_NS16550_CLK		408000000	/* 408MHz (pllp_out0) */
+#endif
 
 /* Environment information, boards can override if required */
 #define CONFIG_LOADADDR		0x80408000	/* def. location for kernel */
-- 
2.0.0.526.g5318336

[U-Boot] [PATCH v3 16/16] dm: tegra: Use V_NS16550_CLK only in SPL builds

[Stephen Warren]

On 07/30/2014 03:49 AM, Simon Glass wrote:
> Since Tegra now uses driver model for serial, adjust the definition of
> V_NS16550_CLK so that it is clear that this is only used for SPL.

May as well squash this into the previous patch.

[U-Boot] [PATCH v3 16/16] dm: tegra: Use V_NS16550_CLK only in SPL builds

[Simon Glass]

Hi Stephen,

On 31 July 2014 21:22, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 07/30/2014 03:49 AM, Simon Glass wrote:
>>
>> Since Tegra now uses driver model for serial, adjust the definition of
>> V_NS16550_CLK so that it is clear that this is only used for SPL.
>
>
> May as well squash this into the previous patch.

OK, will do. I wasn't sure what people would think of this patch so
left it separate.

Regards,
Simon