[U-Boot] [PATCH v4 0/20] tegra: Add fdt definitions and USB driver

[U-Boot] [PATCH v4 0/20] tegra: Add fdt definitions and USB driver

[Simon Glass]

This series brings in the Linux kernel fdt file and provides a working
USB driver for Tegra2 Seaboard.

(I have done this in one series since otherwise most of the fdt additions
will just look like dead code.)

The driver requires CONFIG_OF_CONTROL and a device tree to operate.

Some enhancements to fdtdec are required to make this easier, and these
are included in the series also. I have had to bring in basic GPIO
support due to the request to put the USB VBUS into the fdt.

Since the kernel recently got a very minimal USB binding, I have started
with that and extended it where appropriate.

Tegra likes to have cache-aligned buffers. I have dropped the patch which
implements this since we will solve this problem by making callers align
their buffers (as we did with MMC).

Changes in v2:
- Use "okay" instead of "ok" for fdt node status
- Remove 0x from fdt aliases
- Rename params to timing
- Store entire fdt config in port list, not just register pointer
- Remove non-fdt operation of USB, since it is not needed
- Decode USB VBUS GPIO from the fdt
- Decode phy type differently (to match new kernel fdt)
- Rename tegra20-usb to tegra20-ehcui (to match new kernel fdt)
- Improve debug() printouts in case of failure to init USB
- Add setting of pinmux for USB VBUS GPIO
- Remove unneeded CONFIG_TEGRA_USBx defines

Changes in v3:
- Remove "okay" from nodes since this is the default anyway
- Fix device tree indenting with tabs instead of spaces
- Disable USB2 which is not used on Seaboard
- Remove usbparams properties from fdt and moved them to C code
- Drop Tegra USB alignment patch as we will deal with this another way

Changes in v4:
- Add staging area for device tree bindings used in U-Boot
- Add clock bindings for Tegra2x
- Add fdtdec function to return peripheral ID
- Use updated fdtdec alias functiona to get USB aliases
- Use peripheral clock node to obtain peripheral ID

Simon Glass (20):
  fdt: Tidy up a few fdtdec problems
  fdt: Add functions to access phandles, arrays and bools
  fdt: Add basic support for decoding GPIO definitions
  arm: fdt: Ensure that an embedded fdt is word-aligned
  arm: fdt: Add skeleton device tree file from kernel
  tegra: fdt: Add Tegra2x device tree file from kernel
  tegra: fdt: Add device tree file for Tegra2 Seaboard from kernel
  fdt: Add staging area for device tree binding documentation
  fdt: Add tegra-usb bindings file from linux
  tegra: fdt: Add additional USB binding
  tegra: fdt: Add clock bindings
  tegra: usb: fdt: Add additional device tree definitions for USB ports
  tegra: usb: fdt: Add USB definitions for Tegra2 Seaboard
  usb: Add support for txfifo threshold
  fdt: Add function to return peripheral/clock ID
  tegra: usb: Add support for Tegra USB peripheral
  tegra: usb: Add USB support to nvidia boards
  tegra: usb: Add common USB defines for tegra2 boards
  tegra: usb: Enable USB on Seaboard
  tegra: fdt: Enable FDT support for Seaboard

 README                                         |    3 +
 arch/arm/cpu/armv7/tegra2/Makefile             |    4 +-
 arch/arm/cpu/armv7/tegra2/config.mk            |    2 +
 arch/arm/cpu/armv7/tegra2/usb.c                |  426 ++++++++++++++++++++++++
 arch/arm/cpu/armv7/u-boot.lds                  |    5 +
 arch/arm/dts/skeleton.dtsi                     |   13 +
 arch/arm/dts/tegra20.dtsi                      |  199 +++++++++++
 arch/arm/include/asm/arch-tegra2/tegra2.h      |    2 +
 arch/arm/include/asm/arch-tegra2/usb.h         |  255 ++++++++++++++
 board/nvidia/common/board.c                    |    8 +
 board/nvidia/dts/tegra2-seaboard.dts           |   47 +++
 doc/device-tree-bindings/README                |   19 +
 doc/device-tree-bindings/clock/tegra-periphclk |   51 +++
 doc/device-tree-bindings/usb/tegra-usb.txt     |   17 +
 drivers/usb/host/Makefile                      |    1 +
 drivers/usb/host/ehci-hcd.c                    |    7 +
 drivers/usb/host/ehci-tegra.c                  |   63 ++++
 drivers/usb/host/ehci.h                        |    6 +-
 dts/Makefile                                   |    2 +-
 include/configs/seaboard.h                     |   12 +
 include/configs/tegra2-common.h                |   14 +
 include/fdtdec.h                               |  121 +++++++-
 lib/fdtdec.c                                   |  181 ++++++++++-
 23 files changed, 1445 insertions(+), 13 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/tegra2/usb.c
 create mode 100644 arch/arm/dts/skeleton.dtsi
 create mode 100644 arch/arm/dts/tegra20.dtsi
 create mode 100644 arch/arm/include/asm/arch-tegra2/usb.h
 create mode 100644 board/nvidia/dts/tegra2-seaboard.dts
 create mode 100644 doc/device-tree-bindings/README
 create mode 100644 doc/device-tree-bindings/clock/tegra-periphclk
 create mode 100644 doc/device-tree-bindings/usb/tegra-usb.txt
 create mode 100644 drivers/usb/host/ehci-tegra.c

-- 
1.7.7.3

[U-Boot] [PATCH v4 01/20] fdt: Tidy up a few fdtdec problems

[Simon Glass]

This fixes five trivial issues in fdtdec.c:
1. fdtdec_get_is_enabled() doesn't really need a default value
2. The fdt must be word-aligned, since otherwise it will fail on ARM
3. The compat_names[] array is missing its first element. This is needed
only because the first fdt_compat_id is defined to be invalid.
4. Added a header prototype for fdtdec_next_compatible()
5. Change fdtdec_next_alias() to only increment its 'upto' parameter
on success, to make the display error messages in the caller easier.

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

 include/fdtdec.h |   23 +++++++++++++++++++----
 lib/fdtdec.c     |   22 ++++++++++++++++------
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 3a15f55..01badd4 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -82,6 +82,21 @@ int fdtdec_next_alias(const void *blob, const char *name,
 		enum fdt_compat_id id, int *upto);
 
 /**
+ * Find the next compatible node for a peripheral.
+ *
+ * Do the first call with node = 0. This function will return a pointer to
+ * the next compatible node. Next time you call this function, pass the
+ * value returned, and the next node will be provided.
+ *
+ * @param blob		FDT blob to use
+ * @param node		Start node for search
+ * @param id		Compatible ID to look for (enum fdt_compat_id)
+ * @return offset of next compatible node, or -FDT_ERR_NOTFOUND if no more
+ */
+int fdtdec_next_compatible(const void *blob, int node,
+		enum fdt_compat_id id);
+
+/**
  * Look up an address property in a node and return it as an address.
  * The property must hold either one address with no trailing data or
  * one address with a length. This is only tested on 32-bit machines.
@@ -112,14 +127,14 @@ s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
  * Checks whether a node is enabled.
  * This looks for a 'status' property. If this exists, then returns 1 if
  * the status is 'ok' and 0 otherwise. If there is no status property,
- * it returns the default value.
+ * it returns 1 on the assumption that anything mentioned should be enabled
+ * by default.
  *
  * @param blob	FDT blob
  * @param node	node to examine
- * @param default_val	default value to return if no 'status' property exists
- * @return integer value 0/1, if found, or default_val if not
+ * @return integer value 0 (not enabled) or 1 (enabled)
  */
-int fdtdec_get_is_enabled(const void *blob, int node, int default_val);
+int fdtdec_get_is_enabled(const void *blob, int node);
 
 /**
  * Checks whether we have a valid fdt available to control U-Boot, and panic
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 3852038..846ec3f 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -33,6 +33,7 @@ DECLARE_GLOBAL_DATA_PTR;
  */
 #define COMPAT(id, name) name
 static const char * const compat_names[COMPAT_COUNT] = {
+	COMPAT(UNKNOWN, "<none>"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
@@ -89,14 +90,21 @@ s32 fdtdec_get_int(const void *blob, int node, const char *prop_name,
 	return default_val;
 }
 
-int fdtdec_get_is_enabled(const void *blob, int node, int default_val)
+int fdtdec_get_is_enabled(const void *blob, int node)
 {
 	const char *cell;
 
+	/*
+	 * It should say "okay", so only allow that. Some fdts use "ok" but
+	 * this is a bug. Please fix your device tree source file. See here
+	 * for discussion:
+	 *
+	 * http://www.mail-archive.com/u-boot at lists.denx.de/msg71598.html
+	 */
 	cell = fdt_getprop(blob, node, "status", NULL);
 	if (cell)
-		return 0 == strcmp(cell, "ok");
-	return default_val;
+		return 0 == strcmp(cell, "okay");
+	return 1;
 }
 
 enum fdt_compat_id fd_dec_lookup(const void *blob, int node)
@@ -127,14 +135,16 @@ int fdtdec_next_alias(const void *blob, const char *name,
 	/* snprintf() is not available */
 	assert(strlen(name) < MAX_STR_LEN);
 	sprintf(str, "%.*s%d", MAX_STR_LEN, name, *upto);
-	(*upto)++;
 	node = find_alias_node(blob, str);
 	if (node < 0)
 		return node;
 	err = fdt_node_check_compatible(blob, node, compat_names[id]);
 	if (err < 0)
 		return err;
-	return err ? -FDT_ERR_NOTFOUND : node;
+	if (err)
+		return -FDT_ERR_NOTFOUND;
+	(*upto)++;
+	return node;
 }
 
 /* TODO: Can we tighten this code up a little? */
@@ -254,7 +264,7 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
 int fdtdec_check_fdt(void)
 {
 	/* We must have an fdt */
-	if (fdt_check_header(gd->fdt_blob))
+	if (((uintptr_t)gd->fdt_blob & 3) || fdt_check_header(gd->fdt_blob))
 		panic("No valid fdt found - please append one to U-Boot\n"
 			"binary or define CONFIG_OF_EMBED\n");
 	return 0;
-- 
1.7.7.3

[U-Boot] [PATCH v4 01/20] fdt: Tidy up a few fdtdec problems

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> This fixes five trivial issues in fdtdec.c:
> 1. fdtdec_get_is_enabled() doesn't really need a default value
> 2. The fdt must be word-aligned, since otherwise it will fail on ARM
> 3. The compat_names[] array is missing its first element. This is needed
> only because the first fdt_compat_id is defined to be invalid.
> 4. Added a header prototype for fdtdec_next_compatible()
> 5. Change fdtdec_next_alias() to only increment its 'upto' parameter
> on success, to make the display error messages in the caller easier.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Stephen Warren <swarren@nvidia.com>
(only because I commented on this before; I'm not an FDT maintainer here)

-- 
nvpublic

[U-Boot] [PATCH v4 01/20] fdt: Tidy up a few fdtdec problems

[Jerry Van Baren]

On 01/11/2012 11:32 PM, Simon Glass wrote:
> This fixes five trivial issues in fdtdec.c:
> 1. fdtdec_get_is_enabled() doesn't really need a default value
> 2. The fdt must be word-aligned, since otherwise it will fail on ARM
> 3. The compat_names[] array is missing its first element. This is needed
> only because the first fdt_compat_id is defined to be invalid.
> 4. Added a header prototype for fdtdec_next_compatible()
> 5. Change fdtdec_next_alias() to only increment its 'upto' parameter
> on success, to make the display error messages in the caller easier.
> 
> Signed-off-by: Simon Glass<sjg@chromium.org>

Acked-by: Gerald Van Baren <vanbaren@cideas.com>

Thanks,
gvb

[U-Boot] [PATCH v4 02/20] fdt: Add functions to access phandles, arrays and bools

[Simon Glass]

Add a function to look up a property which is a phandle in a node, and
another to read a fixed-length integer array from an fdt property.
Also add a function to read boolean properties, although there is no
actual boolean type in U-Boot.

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

 include/fdtdec.h |   39 +++++++++++++++++++++++++++++++
 lib/fdtdec.c     |   67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+), 0 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 01badd4..047f603 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -188,3 +188,42 @@ int fdtdec_find_aliases_for_id(const void *blob, const char *name,
  * @return compatible string for that id
  */
 const char *fdtdec_get_compatible(enum fdt_compat_id id);
+
+/* Look up a phandle and follow it to its node. Then return the offset
+ * of that node.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param prop_name	name of property to find
+ * @return node offset if found, -ve error code on error
+ */
+int fdtdec_lookup_phandle(const void *blob, int node, const char *prop_name);
+
+/**
+ * Look up a property in a node and return its contents in an integer
+ * array of given length. The property must have at least enough data for
+ * the array (4*count bytes). It may have more, but this will be ignored.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param prop_name	name of property to find
+ * @param array		array to fill with data
+ * @param count		number of array elements
+ * @return 0 if ok, or -FDT_ERR_NOTFOUND if the property is not found,
+ *		or -FDT_ERR_BADLAYOUT if not enough data
+ */
+int fdtdec_get_int_array(const void *blob, int node, const char *prop_name,
+		u32 *array, int count);
+
+/**
+ * Look up a boolean property in a node and return it.
+ *
+ * A boolean properly is true if present in the device tree and false if not
+ * present, regardless of its value.
+ *
+ * @param blob	FDT blob
+ * @param node	node to examine
+ * @param prop_name	name of property to find
+ * @return 1 if the properly is present; 0 if it isn't present
+ */
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 846ec3f..977528b 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -269,3 +269,70 @@ int fdtdec_check_fdt(void)
 			"binary or define CONFIG_OF_EMBED\n");
 	return 0;
 }
+
+int fdtdec_lookup_phandle(const void *blob, int node, const char *prop_name)
+{
+	const u32 *phandle;
+	int lookup;
+
+	phandle = fdt_getprop(blob, node, prop_name, NULL);
+	if (!phandle)
+		return -FDT_ERR_NOTFOUND;
+
+	lookup = fdt_node_offset_by_phandle(blob, fdt32_to_cpu(*phandle));
+	return lookup;
+}
+
+/**
+ * Look up a property in a node and check that it has a minimum length.
+ *
+ * @param blob		FDT blob
+ * @param node		node to examine
+ * @param prop_name	name of property to find
+ * @param min_len	minimum property length in bytes
+ * @param err		0 if ok, or -FDT_ERR_NOTFOUND if the property is not
+			found, or -FDT_ERR_BADLAYOUT if not enough data
+ * @return pointer to cell, which is only valid if err == 0
+ */
+static const void *get_prop_check_min_len(const void *blob, int node,
+		const char *prop_name, int min_len, int *err)
+{
+	const void *cell;
+	int len;
+
+	debug("%s: %s\n", __func__, prop_name);
+	cell = fdt_getprop(blob, node, prop_name, &len);
+	if (!cell)
+		*err = -FDT_ERR_NOTFOUND;
+	else if (len < min_len)
+		*err = -FDT_ERR_BADLAYOUT;
+	else
+		*err = 0;
+	return cell;
+}
+
+int fdtdec_get_int_array(const void *blob, int node, const char *prop_name,
+		u32 *array, int count)
+{
+	const u32 *cell;
+	int i, err = 0;
+
+	debug("%s: %s\n", __func__, prop_name);
+	cell = get_prop_check_min_len(blob, node, prop_name,
+				      sizeof(u32) * count, &err);
+	if (!err) {
+		for (i = 0; i < count; i++)
+			array[i] = fdt32_to_cpu(cell[i]);
+	}
+	return err;
+}
+
+int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
+{
+	const s32 *cell;
+	int len;
+
+	debug("%s: %s\n", __func__, prop_name);
+	cell = fdt_getprop(blob, node, prop_name, &len);
+	return cell != NULL;
+}
-- 
1.7.7.3

[U-Boot] [PATCH v4 02/20] fdt: Add functions to access phandles, arrays and bools

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> Add a function to look up a property which is a phandle in a node, and
> another to read a fixed-length integer array from an fdt property.
> Also add a function to read boolean properties, although there is no
> actual boolean type in U-Boot.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

Acked-by: Stephen Warren <swarren@nvidia.com>
(only because I commented on this before; I'm not an FDT maintainer here)

-- 
nvpublic

[U-Boot] [PATCH v4 02/20] fdt: Add functions to access phandles, arrays and bools

[Jerry Van Baren]

On 01/11/2012 11:32 PM, Simon Glass wrote:
> Add a function to look up a property which is a phandle in a node, and
> another to read a fixed-length integer array from an fdt property.
> Also add a function to read boolean properties, although there is no
> actual boolean type in U-Boot.
> 
> Signed-off-by: Simon Glass<sjg@chromium.org>

Acked-by: Gerald Van Baren <vanbaren@cideas.com>

Thanks,
gvb

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Simon Glass]

This adds some support into fdtdec for reading GPIO definitions from
the fdt. We permit up to FDT_GPIO_MAX GPIOs in the system. Each GPIO
is of the form:

gpio-function-name = <phandle gpio_num flags>;

where:

phandle is a pointer to the GPIO node
gpio_num is the number of the GPIO (0 to 223)
flags is a flag, as follows:

   bit    meaning
   0      0=polarity normal, 1=active low (inverted)

An example is:

gpio-enable-propounder = <&gpio 43 0>;

which means that GPIO 43 is used to enable the propounder (setting the
GPIO high), or that you can detect that the propounder is enabled by
checking if the GPIO is high (the fdt does not indicate input/output).

Two main functions are provided:

fdtdec_decode_gpio() reads a GPIO property from an fdt node and decodes it
into a structure.

fdtdec_setup_gpio() sets up the GPIO by calling gpio_request for you.

Both functions can cope with the property being missing, which is taken to
mean that that GPIO function is not available or is not needed.

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

 include/fdtdec.h |   45 +++++++++++++++++++++++++++++++
 lib/fdtdec.c     |   78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 0 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 047f603..6c0a2d1 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -61,6 +61,23 @@ enum fdt_compat_id {
 	COMPAT_COUNT,
 };
 
+/* GPIOs are numbered from 0 */
+enum {
+	FDT_GPIO_NONE = -1U,	/* an invalid GPIO used to end our list */
+
+	FDT_GPIO_ACTIVE_LOW = 1 << 0,	/* input is active low (else high) */
+};
+
+/* This is the state of a GPIO pin as defined by the fdt */
+struct fdt_gpio_state {
+	const char *name;	/* name of the fdt property defining this */
+	uint gpio;		/* GPIO number, or FDT_GPIO_NONE if none */
+	u8 flags;		/* FDT_GPIO_... flags */
+};
+
+/* This tells us whether a fdt_gpio_state record is valid or not */
+#define fdt_gpio_isvalid(x) ((x)->gpio != FDT_GPIO_NONE)
+
 /**
  * Find the next numbered alias for a peripheral. This is used to enumerate
  * all the peripherals of a certain type.
@@ -227,3 +244,31 @@ int fdtdec_get_int_array(const void *blob, int node, const char *prop_name,
  * @return 1 if the properly is present; 0 if it isn't present
  */
 int fdtdec_get_bool(const void *blob, int node, const char *prop_name);
+
+/**
+ * Decode a single GPIOs from an FDT.
+ *
+ * If the property is not found, then the GPIO structure will still be
+ * initialised, with gpio set to FDT_GPIO_NONE. This makes it easy to
+ * provide optional GPIOs.
+ *
+ * @param blob		FDT blob to use
+ * @param node		Node to look at
+ * @param prop_name	Node property name
+ * @param gpio		gpio elements to fill from FDT
+ * @return 0 if ok, -FDT_ERR_NOTFOUND if the property is missing.
+ */
+int fdtdec_decode_gpio(const void *blob, int node, const char *prop_name,
+		struct fdt_gpio_state *gpio);
+
+/**
+ * Set up a GPIO pin according to the provided gpio information. At present this
+ * just requests the GPIO.
+ *
+ * If the gpio is FDT_GPIO_NONE, no action is taken. This makes it easy to
+ * deal with optional GPIOs.
+ *
+ * @param gpio		GPIO info to use for set up
+ * @return 0 if all ok or gpio was FDT_GPIO_NONE; -1 on error
+ */
+int fdtdec_setup_gpio(struct fdt_gpio_state *gpio);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 977528b..f0b2196 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -24,6 +24,9 @@
 #include <libfdt.h>
 #include <fdtdec.h>
 
+/* we need the generic GPIO interface here */
+#include <asm-generic/gpio.h>
+
 DECLARE_GLOBAL_DATA_PTR;
 
 /*
@@ -336,3 +339,78 @@ int fdtdec_get_bool(const void *blob, int node, const char *prop_name)
 	cell = fdt_getprop(blob, node, prop_name, &len);
 	return cell != NULL;
 }
+
+/**
+ * Decode a list of GPIOs from an FDT. This creates a list of GPIOs with no
+ * terminating item.
+ *
+ * @param blob		FDT blob to use
+ * @param node		Node to look at
+ * @param prop_name	Node property name
+ * @param gpio		Array of gpio elements to fill from FDT. This will be
+ *			untouched if either 0 or an error is returned
+ * @param max_count	Maximum number of elements allowed
+ * @return number of GPIOs read if ok, -FDT_ERR_BADLAYOUT if max_count would
+ * be exceeded, or -FDT_ERR_NOTFOUND if the property is missing.
+ */
+static int fdtdec_decode_gpios(const void *blob, int node,
+		const char *prop_name, struct fdt_gpio_state *gpio,
+		int max_count)
+{
+	const struct fdt_property *prop;
+	const u32 *cell;
+	const char *name;
+	int len, i;
+
+	debug("%s: %s\n", __func__, prop_name);
+	assert(max_count > 0);
+	prop = fdt_get_property(blob, node, prop_name, &len);
+	if (!prop) {
+		debug("FDT: %s: property '%s' missing\n", __func__, prop_name);
+		return -FDT_ERR_NOTFOUND;
+	}
+
+	/* We will use the name to tag the GPIO */
+	name = fdt_string(blob, prop->nameoff);
+	cell = (u32 *)prop->data;
+	len /= sizeof(u32) * 3;		/* 3 cells per GPIO record */
+	if (len > max_count) {
+		debug("FDT: %s: too many GPIOs / cells for "
+			"property '%s'\n", __func__, prop_name);
+		return -FDT_ERR_BADLAYOUT;
+	}
+
+	/* Read out the GPIO data from the cells */
+	for (i = 0; i < len; i++, cell += 3) {
+		gpio[i].gpio = fdt32_to_cpu(cell[1]);
+		gpio[i].flags = fdt32_to_cpu(cell[2]);
+		gpio[i].name = name;
+	}
+
+	return len;
+}
+
+int fdtdec_decode_gpio(const void *blob, int node, const char *prop_name,
+		struct fdt_gpio_state *gpio)
+{
+	int err;
+
+	debug("%s: %s\n", __func__, prop_name);
+	gpio->gpio = FDT_GPIO_NONE;
+	err = fdtdec_decode_gpios(blob, node, prop_name, gpio, 1);
+	return err == 1 ? 0 : err;
+}
+
+int fdtdec_setup_gpio(struct fdt_gpio_state *gpio)
+{
+	/*
+	 * Return success if there is no GPIO defined. This is used for
+	 * optional GPIOs)
+	 */
+	if (!fdt_gpio_isvalid(gpio))
+		return 0;
+
+	if (gpio_request(gpio->gpio, gpio->name))
+		return -1;
+	return 0;
+}
-- 
1.7.7.3

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> This adds some support into fdtdec for reading GPIO definitions from
> the fdt. We permit up to FDT_GPIO_MAX GPIOs in the system. Each GPIO
> is of the form:
> 
> gpio-function-name = <phandle gpio_num flags>;

That's not true in general.

The binding definition for the GPIO controller that provides the GPIO in
question defines the number and meaning of all cells after the phandle
cell. In practice, this usually does mean two cells with the meanings
above, but there's no need for this to be true in general.

For reference, the correct way to parse such a property is:

* Read the first cell.
* Find the node referenced by the phandle.
* Ensure property gpio-controller is present in the controller node.
* Read property #gpio-cells from the controller node.
* Extract #gpio-cells from the original property.
* Keep processing more cells from the original property; there may be
multiple GPIOs listed.

Still, I'll review this patch under the assumption that it's understood
that its applicability is limited to the subset of controllers that use
the same GPIO specifier as Tegra, i.e. that which you documented.

> where:
> 
> phandle is a pointer to the GPIO node
> gpio_num is the number of the GPIO (0 to 223)
> flags is a flag, as follows:
> 
>    bit    meaning
>    0      0=polarity normal, 1=active low (inverted)

For reference, according to the binding documentation in the Linux
kernel, Samsung Exynos4 doesn't use this format, and while all other
chips do have a flags cell, about 50% of the controllers indicate the
cell is unused.

> An example is:
> 
> gpio-enable-propounder = <&gpio 43 0>;

It appears to be idiomatic to name GPIO-related properties as plural
even when only one GPIO is expected, so e.g. "wp-gpios" for an SDHCI
controller's write-proptect GPIO, rather than simply "wp-gpio.

...
> diff --git a/include/fdtdec.h b/include/fdtdec.h
...
> +/* GPIOs are numbered from 0 */
> +enum {
> +	FDT_GPIO_NONE = -1U,	/* an invalid GPIO used to end our list */

Is this due to the way U-Boot works right now, or something defined by
this patch? It's been pointed out that the kernel's choice to use -1 as
"invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
fields being easily added to platform data structures, since you then
have to go and initialize every new instance to -1, rather than relying
on BSS initializing it to 0. I assume this is just the way U-Boot works,
so solving this is outside the scope of this patch.

Ignoring all the above, the code looks correct to perform as documented.

-- 
nvpublic

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 2:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> This adds some support into fdtdec for reading GPIO definitions from
>> the fdt. We permit up to FDT_GPIO_MAX GPIOs in the system. Each GPIO
>> is of the form:
>>
>> gpio-function-name = <phandle gpio_num flags>;
>
> That's not true in general.
>
> The binding definition for the GPIO controller that provides the GPIO in
> question defines the number and meaning of all cells after the phandle
> cell. In practice, this usually does mean two cells with the meanings
> above, but there's no need for this to be true in general.
>
> For reference, the correct way to parse such a property is:
>
> * Read the first cell.
> * Find the node referenced by the phandle.
> * Ensure property gpio-controller is present in the controller node.
> * Read property #gpio-cells from the controller node.
> * Extract #gpio-cells from the original property.
> * Keep processing more cells from the original property; there may be
> multiple GPIOs listed.
>
> Still, I'll review this patch under the assumption that it's understood
> that its applicability is limited to the subset of controllers that use
> the same GPIO specifier as Tegra, i.e. that which you documented.
>
>> where:
>>
>> phandle is a pointer to the GPIO node
>> gpio_num is the number of the GPIO (0 to 223)
>> flags is a flag, as follows:
>>
>> ? ?bit ? ?meaning
>> ? ?0 ? ? ?0=polarity normal, 1=active low (inverted)
>
> For reference, according to the binding documentation in the Linux
> kernel, Samsung Exynos4 doesn't use this format, and while all other
> chips do have a flags cell, about 50% of the controllers indicate the
> cell is unused.
>
>> An example is:
>>
>> gpio-enable-propounder = <&gpio 43 0>;
>
> It appears to be idiomatic to name GPIO-related properties as plural
> even when only one GPIO is expected, so e.g. "wp-gpios" for an SDHCI
> controller's write-proptect GPIO, rather than simply "wp-gpio.

OK, for both of those I will update the commit message. I have so far
been avoiding having the fdtdec library keep any state around. Looking
up a phandle and properties every time we need to decode a GPIO might
push me towards adding some sort of structure to speed decoding of the
device tree. For now we don't need it, and these functions only do
what they say they do, but it looks like at some point in the future
we might need more complexity here.

Regarding multiple GPIOs - there is a static function
fdtdec_decode_gpios() for decoding such a list which we can export as
needed later. But not everything is required to be a list. For many
GPIOs it makes no sense to have more than one, so a single GPIO is
convenient and conceptually simple for people, and easy to code.

>
> ...
>> diff --git a/include/fdtdec.h b/include/fdtdec.h
> ...
>> +/* GPIOs are numbered from 0 */
>> +enum {
>> + ? ? FDT_GPIO_NONE = -1U, ? ?/* an invalid GPIO used to end our list */
>
> Is this due to the way U-Boot works right now, or something defined by
> this patch? It's been pointed out that the kernel's choice to use -1 as
> "invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
> fields being easily added to platform data structures, since you then
> have to go and initialize every new instance to -1, rather than relying
> on BSS initializing it to 0. I assume this is just the way U-Boot works,
> so solving this is outside the scope of this patch.

It is nothing to do with U-Boot itself - we can choose any number.
What is Linux using now / planning to use?

The number -1U is convenient because it allows us to number GPIOs from
0, which is natural for people and easier to see what is going on when
debugging. Off-by-one situations can be very confusing for people.

At present you must call fdtdec_decode_gpio() to decode a gpio
property in the node. If you don't then you can't know if anything was
there. U-Boot doesn't really have a device model yet, but if it did,
and if you wanted to add a GPIO to a core structure that the driver
used, then indeed the driver (or the code that owns the structure)
would need to call fdtdec_decode_gpio() to set it up. If it did not
then the GPIO number would be 0, which is valid.

A few things do come to mind though. First ->name is set to the
property name - if that is NULL then fdtdec_decode_gpio() has not been
called. Also we could add a 'valid' flag to the flags byte perhaps.
Perhaps in Linux a GPIO is represented just with a number, but here we
are using a structure and can fairly easily add something else to
indicate validity.

Should we try to do something about this now, or later? I am
especially interested in what Linux plans to do here.

>
> Ignoring all the above, the code looks correct to perform as documented.

Thanks for looking at it.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Stephen Warren]

On 01/21/2012 10:08 AM, Simon Glass wrote:
> Hi Stephen,
> 
> On Wed, Jan 18, 2012 at 2:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> On 01/11/2012 09:32 PM, Simon Glass wrote:
>>> This adds some support into fdtdec for reading GPIO definitions from
>>> the fdt. ...
...
>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> ...
>>> +/* GPIOs are numbered from 0 */
>>> +enum {
>>> +     FDT_GPIO_NONE = -1U,    /* an invalid GPIO used to end our list */
>>
>> Is this due to the way U-Boot works right now, or something defined by
>> this patch? It's been pointed out that the kernel's choice to use -1 as
>> "invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
>> fields being easily added to platform data structures, since you then
>> have to go and initialize every new instance to -1, rather than relying
>> on BSS initializing it to 0. I assume this is just the way U-Boot works,
>> so solving this is outside the scope of this patch.
> 
> It is nothing to do with U-Boot itself - we can choose any number.

Surely the value you choose for DT parsing has to align with the value
that U-Boot's GPIO API chooses, so you can't just choose any number.

> What is Linux using now / planning to use?

Linux uses -1 right now, but should really have used 0. I don't think
there's an actual plan to change this now, since the numbering scheme is
entrenched.

What Linux does isn't relevant; the numbering scheme I'm talking about
is the internal numbering scheme within U-Boot or Linux, which are
independent from each-other and the values in the device tree.

So that said, I was wondering if U-Boot's GPIO support (covering both
non-DT and DT cases) was new enough that it could use 0 to represent
"invalid GPIO" rather than -1, and hence FDT_GPIO_NONE be 0 not -1. It
probably isn't though; I guess U-Boot's GPIO numbering scheme is also
already entrenched?

-- 
nvpublic

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Simon Glass]

Hi Stephen,

On Mon, Jan 23, 2012 at 10:18 AM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/21/2012 10:08 AM, Simon Glass wrote:
>> Hi Stephen,
>>
>> On Wed, Jan 18, 2012 at 2:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
>>> On 01/11/2012 09:32 PM, Simon Glass wrote:
>>>> This adds some support into fdtdec for reading GPIO definitions from
>>>> the fdt. ...
> ...
>>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>>> ...
>>>> +/* GPIOs are numbered from 0 */
>>>> +enum {
>>>> + ? ? FDT_GPIO_NONE = -1U, ? ?/* an invalid GPIO used to end our list */
>>>
>>> Is this due to the way U-Boot works right now, or something defined by
>>> this patch? It's been pointed out that the kernel's choice to use -1 as
>>> "invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
>>> fields being easily added to platform data structures, since you then
>>> have to go and initialize every new instance to -1, rather than relying
>>> on BSS initializing it to 0. I assume this is just the way U-Boot works,
>>> so solving this is outside the scope of this patch.
>>
>> It is nothing to do with U-Boot itself - we can choose any number.
>
> Surely the value you choose for DT parsing has to align with the value
> that U-Boot's GPIO API chooses, so you can't just choose any number.

No, in the fdt we just leave out the gpio if it is not required. The
FDT_GPIO_NONE is purely internal to U-Boot.

>
>> What is Linux using now / planning to use?
>
> Linux uses -1 right now, but should really have used 0. I don't think
> there's an actual plan to change this now, since the numbering scheme is
> entrenched.
>
> What Linux does isn't relevant; the numbering scheme I'm talking about
> is the internal numbering scheme within U-Boot or Linux, which are
> independent from each-other and the values in the device tree.
>
> So that said, I was wondering if U-Boot's GPIO support (covering both
> non-DT and DT cases) was new enough that it could use 0 to represent
> "invalid GPIO" rather than -1, and hence FDT_GPIO_NONE be 0 not -1. It
> probably isn't though; I guess U-Boot's GPIO numbering scheme is also
> already entrenched?

Yes it starts at zero. I'm not sure how to even begin such a
discussion on the list :-)

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Stephen Warren]

Simon Glass wrote at Tuesday, January 24, 2012 4:12 PM:
> On Mon, Jan 23, 2012 at 10:18 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > On 01/21/2012 10:08 AM, Simon Glass wrote:
> >> Hi Stephen,
> >>
> >> On Wed, Jan 18, 2012 at 2:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
> >>> On 01/11/2012 09:32 PM, Simon Glass wrote:
> >>>> This adds some support into fdtdec for reading GPIO definitions from
> >>>> the fdt. ...
> > ...
> >>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
> >>> ...
> >>>> +/* GPIOs are numbered from 0 */
> >>>> +enum {
> >>>> + ? ? FDT_GPIO_NONE = -1U, ? ?/* an invalid GPIO used to end our list */
> >>>
> >>> Is this due to the way U-Boot works right now, or something defined by
> >>> this patch? It's been pointed out that the kernel's choice to use -1 as
> >>> "invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
> >>> fields being easily added to platform data structures, since you then
> >>> have to go and initialize every new instance to -1, rather than relying
> >>> on BSS initializing it to 0. I assume this is just the way U-Boot works,
> >>> so solving this is outside the scope of this patch.
> >>
> >> It is nothing to do with U-Boot itself - we can choose any number.
> >
> > Surely the value you choose for DT parsing has to align with the value
> > that U-Boot's GPIO API chooses, so you can't just choose any number.
> 
> No, in the fdt we just leave out the gpio if it is not required. The
> FDT_GPIO_NONE is purely internal to U-Boot.

That's consistent with what I said; I'm talking about the value that a
missing DT property translates into within U-Boot.

-- 
nvpublic

[U-Boot] [PATCH v4 03/20] fdt: Add basic support for decoding GPIO definitions

[Simon Glass]

Hi Stephen,

On Tue, Jan 24, 2012 at 3:14 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Simon Glass wrote at Tuesday, January 24, 2012 4:12 PM:
>> On Mon, Jan 23, 2012 at 10:18 AM, Stephen Warren <swarren@nvidia.com> wrote:
>> > On 01/21/2012 10:08 AM, Simon Glass wrote:
>> >> Hi Stephen,
>> >>
>> >> On Wed, Jan 18, 2012 at 2:17 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> >>> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> >>>> This adds some support into fdtdec for reading GPIO definitions from
>> >>>> the fdt. ...
>> > ...
>> >>>> diff --git a/include/fdtdec.h b/include/fdtdec.h
>> >>> ...
>> >>>> +/* GPIOs are numbered from 0 */
>> >>>> +enum {
>> >>>> + ? ? FDT_GPIO_NONE = -1U, ? ?/* an invalid GPIO used to end our list */
>> >>>
>> >>> Is this due to the way U-Boot works right now, or something defined by
>> >>> this patch? It's been pointed out that the kernel's choice to use -1 as
>> >>> "invalid GPIO" rather than 0 was a mistake, since that prevents GPIO
>> >>> fields being easily added to platform data structures, since you then
>> >>> have to go and initialize every new instance to -1, rather than relying
>> >>> on BSS initializing it to 0. I assume this is just the way U-Boot works,
>> >>> so solving this is outside the scope of this patch.
>> >>
>> >> It is nothing to do with U-Boot itself - we can choose any number.
>> >
>> > Surely the value you choose for DT parsing has to align with the value
>> > that U-Boot's GPIO API chooses, so you can't just choose any number.
>>
>> No, in the fdt we just leave out the gpio if it is not required. The
>> FDT_GPIO_NONE is purely internal to U-Boot.
>
> That's consistent with what I said; I'm talking about the value that a
> missing DT property translates into within U-Boot.

OK I see. I'm not proposing to change U-Boot convention now (if in
fact it has one). At least FDT_GPIO_NONE is a constant that people
need to check against (but they should use fdt_gpio_isvalid()
instead). We can adjust this in the future if U-Boot people want to.

Regards,
Simon
>
> --
> nvpublic
>

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Simon Glass]

By putting the fdt blob into a distinctive area we can ensure that it appears
at the start of the data section and is word-aligned.

Note: It does not seem to be possible to get objcopy to honour its
--section-alignment flag, which would otherwise provide an easier fix
for this problem.

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

 arch/arm/cpu/armv7/u-boot.lds |    5 +++++
 dts/Makefile                  |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/arch/arm/cpu/armv7/u-boot.lds b/arch/arm/cpu/armv7/u-boot.lds
index 40ecf78..793e51b 100644
--- a/arch/arm/cpu/armv7/u-boot.lds
+++ b/arch/arm/cpu/armv7/u-boot.lds
@@ -43,6 +43,11 @@ SECTIONS
 
 	. = ALIGN(4);
 	.data : {
+		/*
+		 * Sadly objcopy seems to ignore --section-alignment.
+		 * Put any embedded device tree first so it is aligned.
+		 */
+		*(.dts.data)
 		*(.data)
 	}
 
diff --git a/dts/Makefile b/dts/Makefile
index 5792afd..83547d4 100644
--- a/dts/Makefile
+++ b/dts/Makefile
@@ -79,7 +79,7 @@ $(obj)dt.o: $(DT_BIN)
 	\
 	cd $(dir ${DT_BIN}) && \
 	$(OBJCOPY) -I binary -O $${oformat} -B $${oarch} \
-		$(notdir ${DT_BIN}) $@
+		--prefix-sections=.dts $(notdir ${DT_BIN}) $@
 	rm $(DT_BIN)
 
 OBJS-$(CONFIG_OF_EMBED)	:= dt.o
-- 
1.7.7.3

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Albert ARIBAUD]

Hi Simon,

Le 12/01/2012 05:32, Simon Glass a ?crit :
> By putting the fdt blob into a distinctive area we can ensure that it appears
> at the start of the data section and is word-aligned.
>
> Note: It does not seem to be possible to get objcopy to honour its
> --section-alignment flag, which would otherwise provide an easier fix
> for this problem.

Stop me if I am wrong, but objcopy obviously works with output sections 
of its input file, not with input ones that this file was linked from, 
and so it cannot act upon alignment of data input sections, right? And 
as, before your patch, there is no designated output section for DTS 
data, it ends up (possibly mis-aligned) in the .data output section 
(which is globally aligned however), so there is no chance anyway that 
objcopy can re-align DTS data if they were mis-aligned.

So I must be missing something. Can you clarify the scenario that gets 
fixed here?

Not that I am against the patch, mind you, quite the opposite. I am just 
wondering about the pros and cons of having a separated (and aligned) 
DTS data *output* section and placing that section after .code and .data 
rather than between them.

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Simon Glass]

Hi Albert,

On Feb 18, 2012 3:51 AM, "Albert ARIBAUD" <albert.u.boot@aribaud.net> wrote:
>
> Hi Simon,
>
> Le 12/01/2012 05:32, Simon Glass a ?crit :
>
>> By putting the fdt blob into a distinctive area we can ensure that it
appears
>> at the start of the data section and is word-aligned.
>>
>> Note: It does not seem to be possible to get objcopy to honour its
>> --section-alignment flag, which would otherwise provide an easier fix
>> for this problem.
>
>
> Stop me if I am wrong, but objcopy obviously works with output sections
of its input file, not with input ones that this file was linked from, and
so it cannot act upon alignment of data input sections, right? And as,
before your patch, there is no designated output section for DTS data, it
ends up (possibly mis-aligned) in the .data output section (which is
globally aligned however), so there is no chance anyway that objcopy can
re-align DTS data if they were mis-aligned.

Well that's a shame if true. I was rather hoping that it would respect
input section alignment when packing into an output section - it must do
that in at least some cases since otherise I don't see how code regions
could be collected together without one becoming misaligned if the one
before was an odd size.

>
> So I must be missing something. Can you clarify the scenario that gets
fixed here?

The fdt blob is in one of the object files. If during linking the file
before it has a data segment whose size is not a multiple of 4, then the
fdt object can be misaligned in the output.

>
> Not that I am against the patch, mind you, quite the opposite. I am just
wondering about the pros and cons of having a separated (and aligned) DTS
data *output* section and placing that section after .code and .data rather
than between them.

Bear in mind that embedding the fdt in the image is really only for
development.

We could have a separate output section if you like. Up to you.

Regards,
Simon

>
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Albert ARIBAUD]

Hi Simon,

Le 18/02/2012 19:09, Simon Glass a ?crit :
> Hi Albert,
>
> On Feb 18, 2012 3:51 AM, "Albert ARIBAUD"<albert.u.boot@aribaud.net>  wrote:
>>
>> Hi Simon,
>>
>> Le 12/01/2012 05:32, Simon Glass a ?crit :
>>
>>> By putting the fdt blob into a distinctive area we can ensure that it
> appears
>>> at the start of the data section and is word-aligned.
>>>
>>> Note: It does not seem to be possible to get objcopy to honour its
>>> --section-alignment flag, which would otherwise provide an easier fix
>>> for this problem.
>>
>>
>> Stop me if I am wrong, but objcopy obviously works with output sections
> of its input file, not with input ones that this file was linked from, and
> so it cannot act upon alignment of data input sections, right? And as,
> before your patch, there is no designated output section for DTS data, it
> ends up (possibly mis-aligned) in the .data output section (which is
> globally aligned however), so there is no chance anyway that objcopy can
> re-align DTS data if they were mis-aligned.
>
> Well that's a shame if true. I was rather hoping that it would respect
> input section alignment when packing into an output section - it must do
> that in at least some cases since otherise I don't see how code regions
> could be collected together without one becoming misaligned if the one
> before was an odd size.

Objcopy is only a utility to convert built binaries from one format to 
another, and rightly ignores any alignment considerations. Alignment 
constraints are solved at link time within the .lds file. This is why I 
suggest a separate output section in u-boot.lds, because then you can 
set that section's alignment where all alignment constraints are laid out.

>> So I must be missing something. Can you clarify the scenario that gets
> fixed here?
>
> The fdt blob is in one of the object files. If during linking the file
> before it has a data segment whose size is not a multiple of 4, then the
> fdt object can be misaligned in the output.

Understood. This example of an alignment failure scenario shows that the 
failure cause is indeed that the FDT is embedded in the .data output 
section at link time.

>> Not that I am against the patch, mind you, quite the opposite. I am just
> wondering about the pros and cons of having a separated (and aligned) DTS
> data *output* section and placing that section after .code and .data rather
> than between them.
>
> Bear in mind that embedding the fdt in the image is really only for
> development.

Understood, and I think that's one more reason to make sure that the act 
of FDT embedding does not alter .data input section ordering.

> We could have a separate output section if you like. Up to you.

I much prefer embedded FDT data to be explicitly mapped to a a dedicated 
.fdt output section in u-boot.lds, because it ensures that aligment 
constaints for .fdt and .data can be controlled independently.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Simon Glass]

Hi Albert,

On Feb 19, 2012 12:27 AM, "Albert ARIBAUD" <albert.u.boot@aribaud.net>
wrote:
>
> Hi Simon,
>
> Le 18/02/2012 19:09, Simon Glass a ?crit :
>
>> Hi Albert,
>>
>> On Feb 18, 2012 3:51 AM, "Albert ARIBAUD"<albert.u.boot@aribaud.net>
 wrote:
>>>
>>>
>>> Hi Simon,
>>>
>>> Le 12/01/2012 05:32, Simon Glass a ?crit :
>>>
>>>> By putting the fdt blob into a distinctive area we can ensure that it
>>
>> appears
>>>>
>>>> at the start of the data section and is word-aligned.
>>>>
>>>> Note: It does not seem to be possible to get objcopy to honour its
>>>> --section-alignment flag, which would otherwise provide an easier fix
>>>> for this problem.
>>>
>>>
>>>
>>> Stop me if I am wrong, but objcopy obviously works with output sections
>>
>> of its input file, not with input ones that this file was linked from,
and
>> so it cannot act upon alignment of data input sections, right? And as,
>> before your patch, there is no designated output section for DTS data, it
>> ends up (possibly mis-aligned) in the .data output section (which is
>> globally aligned however), so there is no chance anyway that objcopy can
>> re-align DTS data if they were mis-aligned.
>>
>> Well that's a shame if true. I was rather hoping that it would respect
>> input section alignment when packing into an output section - it must do
>> that in at least some cases since otherise I don't see how code regions
>> could be collected together without one becoming misaligned if the one
>> before was an odd size.
>
>
> Objcopy is only a utility to convert built binaries from one format to
another, and rightly ignores any alignment considerations. Alignment
constraints are solved at link time within the .lds file. This is why I
suggest a separate output section in u-boot.lds, because then you can set
that section's alignment where all alignment constraints are laid out.
>

Sorry I had that wrong - it's not the link step I am bothered by, it is the
objcopy step.

What I would like to do is mark the input section, created by objcopy, with
a particular alignment so that the linker does the right thing. This
feature appears to be present in objcopy but does not appear to work.
Perhaps I should debug that first to see what is going on.

>
>>> So I must be missing something. Can you clarify the scenario that gets
>>
>> fixed here?
>>
>> The fdt blob is in one of the object files. If during linking the file
>> before it has a data segment whose size is not a multiple of 4, then the
>> fdt object can be misaligned in the output.
>
>
> Understood. This example of an alignment failure scenario shows that the
failure cause is indeed that the FDT is embedded in the .data output
section at link time.
>
>
>>> Not that I am against the patch, mind you, quite the opposite. I am just
>>
>> wondering about the pros and cons of having a separated (and aligned) DTS
>> data *output* section and placing that section after .code and .data
rather
>> than between them.
>>
>> Bear in mind that embedding the fdt in the image is really only for
>> development.
>
>
> Understood, and I think that's one more reason to make sure that the act
of FDT embedding does not alter .data input section ordering.
>

Well it is just a data section really.

>
>> We could have a separate output section if you like. Up to you.
>
>
> I much prefer embedded FDT data to be explicitly mapped to a a dedicated
.fdt output section in u-boot.lds, because it ensures that aligment
constaints for .fdt and .data can be controlled independently.

I will take a look at this.

Regards,
Simon

>
>
>> Regards,
>> Simon
>
>
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Albert ARIBAUD]

Hi Simon,

Le 19/02/2012 17:23, Simon Glass a ?crit :

> Sorry I had that wrong - it's not the link step I am bothered by, it is the
> objcopy step.
>
> What I would like to do is mark the input section, created by objcopy, with
> a particular alignment so that the linker does the right thing. This
> feature appears to be present in objcopy but does not appear to work.
> Perhaps I should debug that first to see what is going on.

(Formally there are two "objcopy steps" in u-boot building. One happens 
after the link step, where from the u-boot ELF binary objcopy derives 
the u-boot.bin pure binary and u-boot.srec S-Record file. I suspect 
you're referring to the other one, which happens before the link stage 
and which turns the binary FDT data into an ELF-linkable object. Correct?)

Maybe one is allowed to specify alignment requirements when making an 
ELF object file from a pure binary with objcopy; but obviously it does 
not work as you want it to, whereas section alignment in linker scripts 
is a tried and proven method.

>>>> So I must be missing something. Can you clarify the scenario that gets
>>>
>>> fixed here?
>>>
>>> The fdt blob is in one of the object files. If during linking the file
>>> before it has a data segment whose size is not a multiple of 4, then the
>>> fdt object can be misaligned in the output.
>>
>>
>> Understood. This example of an alignment failure scenario shows that the
> failure cause is indeed that the FDT is embedded in the .data output
> section at link time.
>>
>>
>>>> Not that I am against the patch, mind you, quite the opposite. I am just
>>>
>>> wondering about the pros and cons of having a separated (and aligned) DTS
>>> data *output* section and placing that section after .code and .data
> rather
>>> than between them.
>>>
>>> Bear in mind that embedding the fdt in the image is really only for
>>> development.
>>
>>
>> Understood, and I think that's one more reason to make sure that the act
> of FDT embedding does not alter .data input section ordering.
>
> Well it is just a data section really.

So are the relocation tables in .rel.dyn and .dynsyms, but that does not 
mean they have to go to the "dot data" output section.

>>> We could have a separate output section if you like. Up to you.
>>
>>
>> I much prefer embedded FDT data to be explicitly mapped to a a dedicated
> .fdt output section in u-boot.lds, because it ensures that aligment
> constaints for .fdt and .data can be controlled independently.
>
> I will take a look at this.

Thanks. This should be fairly easy to do in the linker script by 
inserting an output section (e.g. ".fdt") after .data, aligning the 
start of that section to a word boundary, and explicitly mapping the 
.data section of the FDT object file into it.

> Regards,
> Simon

Amicalement,
-- 
Albert.

[U-Boot] [PATCH v4 04/20] arm: fdt: Ensure that an embedded fdt is word-aligned

[Simon Glass]

Hi Albert,

On Sun, Feb 19, 2012 at 10:33 AM, Albert ARIBAUD
<albert.u.boot@aribaud.net> wrote:
> Hi Simon,
>
> Le 19/02/2012 17:23, Simon Glass a ?crit :
>
>
>> Sorry I had that wrong - it's not the link step I am bothered by, it is
>> the
>> objcopy step.
>>
>> What I would like to do is mark the input section, created by objcopy,
>> with
>> a particular alignment so that the linker does the right thing. This
>> feature appears to be present in objcopy but does not appear to work.
>> Perhaps I should debug that first to see what is going on.
>
>
> (Formally there are two "objcopy steps" in u-boot building. One happens
> after the link step, where from the u-boot ELF binary objcopy derives the
> u-boot.bin pure binary and u-boot.srec S-Record file. I suspect you're
> referring to the other one, which happens before the link stage and which
> turns the binary FDT data into an ELF-linkable object. Correct?)
>
> Maybe one is allowed to specify alignment requirements when making an ELF
> object file from a pure binary with objcopy; but obviously it does not work
> as you want it to, whereas section alignment in linker scripts is a tried
> and proven method.
>
>
>>>>> So I must be missing something. Can you clarify the scenario that gets
>>>>
>>>>
>>>> fixed here?
>>>>
>>>> The fdt blob is in one of the object files. If during linking the file
>>>> before it has a data segment whose size is not a multiple of 4, then the
>>>> fdt object can be misaligned in the output.
>>>
>>>
>>>
>>> Understood. This example of an alignment failure scenario shows that the
>>
>> failure cause is indeed that the FDT is embedded in the .data output
>> section at link time.
>>>
>>>
>>>
>>>>> Not that I am against the patch, mind you, quite the opposite. I am
>>>>> just
>>>>
>>>>
>>>> wondering about the pros and cons of having a separated (and aligned)
>>>> DTS
>>>> data *output* section and placing that section after .code and .data
>>
>> rather
>>>>
>>>> than between them.
>>>>
>>>> Bear in mind that embedding the fdt in the image is really only for
>>>> development.
>>>
>>>
>>>
>>> Understood, and I think that's one more reason to make sure that the act
>>
>> of FDT embedding does not alter .data input section ordering.
>>
>> Well it is just a data section really.
>
>
> So are the relocation tables in .rel.dyn and .dynsyms, but that does not
> mean they have to go to the "dot data" output section.
>
>
>>>> We could have a separate output section if you like. Up to you.
>>>
>>>
>>>
>>> I much prefer embedded FDT data to be explicitly mapped to a a dedicated
>>
>> .fdt output section in u-boot.lds, because it ensures that aligment
>> constaints for .fdt and .data can be controlled independently.
>>
>> I will take a look at this.
>
>
> Thanks. This should be fairly easy to do in the linker script by inserting
> an output section (e.g. ".fdt") after .data, aligning the start of that
> section to a word boundary, and explicitly mapping the .data section of the
> FDT object file into it.

Just an update on this: I am going to drop this patch from the series
now and deal with it later once the lds cleanup series lands. I don't
want to patch a file that is being removed.

Regards,
Simon

>
>
>> Regards,
>> Simon
>
>
> Amicalement,
> --
> Albert.

[U-Boot] [PATCH v4 05/20] arm: fdt: Add skeleton device tree file from kernel

[Simon Glass]

This was taken from commit b48c54e2 at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git

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

 arch/arm/dts/skeleton.dtsi |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/dts/skeleton.dtsi

diff --git a/arch/arm/dts/skeleton.dtsi b/arch/arm/dts/skeleton.dtsi
new file mode 100644
index 0000000..b41d241
--- /dev/null
+++ b/arch/arm/dts/skeleton.dtsi
@@ -0,0 +1,13 @@
+/*
+ * Skeleton device tree; the bare minimum needed to boot; just include and
+ * add a compatible value.  The bootloader will typically populate the memory
+ * node.
+ */
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+	chosen { };
+	aliases { };
+	memory { device_type = "memory"; reg = <0 0>; };
+};
-- 
1.7.7.3

[U-Boot] [PATCH v4 06/20] tegra: fdt: Add Tegra2x device tree file from kernel

[Simon Glass]

This was taken from commit b48c54e2 at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git

config.mk is updated to provide this file to boards through the
built-in mechanism:

/include/ ARCH_CPU_DTS

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

 arch/arm/cpu/armv7/tegra2/config.mk |    2 +
 arch/arm/dts/tegra20.dtsi           |  169 +++++++++++++++++++++++++++++++++++
 2 files changed, 171 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/dts/tegra20.dtsi

diff --git a/arch/arm/cpu/armv7/tegra2/config.mk b/arch/arm/cpu/armv7/tegra2/config.mk
index 2303dba..fe9ef5b 100644
--- a/arch/arm/cpu/armv7/tegra2/config.mk
+++ b/arch/arm/cpu/armv7/tegra2/config.mk
@@ -31,3 +31,5 @@ CFLAGS_arch/arm/lib/board.o += -march=armv4t
 endif
 
 USE_PRIVATE_LIBGCC = yes
+
+CONFIG_ARCH_DEVICE_TREE := tegra20
diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
new file mode 100644
index 0000000..6146d24
--- /dev/null
+++ b/arch/arm/dts/tegra20.dtsi
@@ -0,0 +1,169 @@
+/include/ "skeleton.dtsi"
+
+/ {
+	compatible = "nvidia,tegra20";
+	interrupt-parent = <&intc>;
+
+	intc: interrupt-controller at 50041000 {
+		compatible = "nvidia,tegra20-gic";
+		interrupt-controller;
+		#interrupt-cells = <1>;
+		reg = < 0x50041000 0x1000 >,
+		      < 0x50040100 0x0100 >;
+	};
+
+	i2c at 7000c000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2c";
+		reg = <0x7000C000 0x100>;
+		interrupts = < 70 >;
+	};
+
+	i2c at 7000c400 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2c";
+		reg = <0x7000C400 0x100>;
+		interrupts = < 116 >;
+	};
+
+	i2c at 7000c500 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2c";
+		reg = <0x7000C500 0x100>;
+		interrupts = < 124 >;
+	};
+
+	i2c at 7000d000 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2c";
+		reg = <0x7000D000 0x200>;
+		interrupts = < 85 >;
+	};
+
+	i2s at 70002800 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2s";
+		reg = <0x70002800 0x200>;
+		interrupts = < 45 >;
+		dma-channel = < 2 >;
+	};
+
+	i2s at 70002a00 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-i2s";
+		reg = <0x70002a00 0x200>;
+		interrupts = < 35 >;
+		dma-channel = < 1 >;
+	};
+
+	das at 70000c00 {
+		#address-cells = <1>;
+		#size-cells = <0>;
+		compatible = "nvidia,tegra20-das";
+		reg = <0x70000c00 0x80>;
+	};
+
+	gpio: gpio at 6000d000 {
+		compatible = "nvidia,tegra20-gpio";
+		reg = < 0x6000d000 0x1000 >;
+		interrupts = < 64 65 66 67 87 119 121 >;
+		#gpio-cells = <2>;
+		gpio-controller;
+	};
+
+	pinmux: pinmux at 70000000 {
+		compatible = "nvidia,tegra20-pinmux";
+		reg = < 0x70000014 0x10    /* Tri-state registers */
+			0x70000080 0x20    /* Mux registers */
+			0x700000a0 0x14    /* Pull-up/down registers */
+			0x70000868 0xa8 >; /* Pad control registers */
+	};
+
+	serial at 70006000 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006000 0x40>;
+		reg-shift = <2>;
+		interrupts = < 68 >;
+	};
+
+	serial at 70006040 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006040 0x40>;
+		reg-shift = <2>;
+		interrupts = < 69 >;
+	};
+
+	serial at 70006200 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006200 0x100>;
+		reg-shift = <2>;
+		interrupts = < 78 >;
+	};
+
+	serial at 70006300 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006300 0x100>;
+		reg-shift = <2>;
+		interrupts = < 122 >;
+	};
+
+	serial at 70006400 {
+		compatible = "nvidia,tegra20-uart";
+		reg = <0x70006400 0x100>;
+		reg-shift = <2>;
+		interrupts = < 123 >;
+	};
+
+	sdhci at c8000000 {
+		compatible = "nvidia,tegra20-sdhci";
+		reg = <0xc8000000 0x200>;
+		interrupts = < 46 >;
+	};
+
+	sdhci at c8000200 {
+		compatible = "nvidia,tegra20-sdhci";
+		reg = <0xc8000200 0x200>;
+		interrupts = < 47 >;
+	};
+
+	sdhci at c8000400 {
+		compatible = "nvidia,tegra20-sdhci";
+		reg = <0xc8000400 0x200>;
+		interrupts = < 51 >;
+	};
+
+	sdhci at c8000600 {
+		compatible = "nvidia,tegra20-sdhci";
+		reg = <0xc8000600 0x200>;
+		interrupts = < 63 >;
+	};
+
+	usb at c5000000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5000000 0x4000>;
+		interrupts = < 52 >;
+		phy_type = "utmi";
+	};
+
+	usb at c5004000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5004000 0x4000>;
+		interrupts = < 53 >;
+		phy_type = "ulpi";
+	};
+
+	usb at c5008000 {
+		compatible = "nvidia,tegra20-ehci", "usb-ehci";
+		reg = <0xc5008000 0x4000>;
+		interrupts = < 129 >;
+		phy_type = "utmi";
+	};
+
+};
+
-- 
1.7.7.3

[U-Boot] [PATCH v4 06/20] tegra: fdt: Add Tegra2x device tree file from kernel

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> This was taken from commit b48c54e2 at:
> git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
> 
> config.mk is updated to provide this file to boards through the
> built-in mechanism:
> 
> /include/ ARCH_CPU_DTS
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
...
> +	i2c at 7000d000 {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		compatible = "nvidia,tegra20-i2c";
> +		reg = <0x7000D000 0x200>;
> +		interrupts = < 85 >;
> +	};

This is slightly out-of-date now; in next-20120118, that node's
compatible flag is nvidia,tegra20-i2c-dvc. The HW register layout is
somewhat different for this I2C controller, and the different compatible
flag is how the driver this.

Still, you can always apply that fix in a later patch before you add I2C
support.

-- 
nvpublic

[U-Boot] [PATCH v4 06/20] tegra: fdt: Add Tegra2x device tree file from kernel

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 2:24 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> This was taken from commit b48c54e2 at:
>> git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
>>
>> config.mk is updated to provide this file to boards through the
>> built-in mechanism:
>>
>> /include/ ARCH_CPU_DTS
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
> ...
>> + ? ? i2c at 7000d000 {
>> + ? ? ? ? ? ? #address-cells = <1>;
>> + ? ? ? ? ? ? #size-cells = <0>;
>> + ? ? ? ? ? ? compatible = "nvidia,tegra20-i2c";
>> + ? ? ? ? ? ? reg = <0x7000D000 0x200>;
>> + ? ? ? ? ? ? interrupts = < 85 >;
>> + ? ? };
>
> This is slightly out-of-date now; in next-20120118, that node's
> compatible flag is nvidia,tegra20-i2c-dvc. The HW register layout is
> somewhat different for this I2C controller, and the different compatible
> flag is how the driver this.
>
> Still, you can always apply that fix in a later patch before you add I2C
> support.

This series was original submitted 24 November, so of course things
change. The changes you mention relate to I2C and I think I picked
these up in the later I2C patches.

But since we are chasing a moving target, I really don't want to get
too worried about redoing all the patches every time the kernel burps
:-) The changes you mention are not even in mainline yet and don't
relate to USB (this series).

Is it ok to adopt this sort of thinking throughout these series? As
you know it is easy to pick up a new fdt file later and make any
changes that are needed. I am well aware that things are still in
flux.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 06/20] tegra: fdt: Add Tegra2x device tree file from kernel

[Stephen Warren]

On 01/19/2012 04:51 PM, Simon Glass wrote:
> Hi Stephen,
> 
> On Wed, Jan 18, 2012 at 2:24 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> On 01/11/2012 09:32 PM, Simon Glass wrote:
>>> This was taken from commit b48c54e2 at:
>>> git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
>>>
>>> config.mk is updated to provide this file to boards through the
>>> built-in mechanism:
>>>
>>> /include/ ARCH_CPU_DTS
>>>
>>> Signed-off-by: Simon Glass <sjg@chromium.org>
>>
>>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>> ...
>>> +     i2c at 7000d000 {
>>> +             #address-cells = <1>;
>>> +             #size-cells = <0>;
>>> +             compatible = "nvidia,tegra20-i2c";
>>> +             reg = <0x7000D000 0x200>;
>>> +             interrupts = < 85 >;
>>> +     };
>>
>> This is slightly out-of-date now; in next-20120118, that node's
>> compatible flag is nvidia,tegra20-i2c-dvc. The HW register layout is
>> somewhat different for this I2C controller, and the different compatible
>> flag is how the driver this.
>>
>> Still, you can always apply that fix in a later patch before you add I2C
>> support.
> 
> This series was original submitted 24 November, so of course things
> change. The changes you mention relate to I2C and I think I picked
> these up in the later I2C patches.

As I said, as long as this is updated before the I2C support that relies
on it (as IIRC your I2C patch series did), that's fine.

-- 
nvpublic

[U-Boot] [PATCH v4 07/20] tegra: fdt: Add device tree file for Tegra2 Seaboard from kernel

[Simon Glass]

This was taken from commit b48c54e2 at:
git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git

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

 board/nvidia/dts/tegra2-seaboard.dts |   36 ++++++++++++++++++++++++++++++++++
 1 files changed, 36 insertions(+), 0 deletions(-)
 create mode 100644 board/nvidia/dts/tegra2-seaboard.dts

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
new file mode 100644
index 0000000..dde5d03
--- /dev/null
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -0,0 +1,36 @@
+/dts-v1/;
+
+/memreserve/ 0x1c000000 0x04000000;
+/include/ ARCH_CPU_DTS
+
+/ {
+	model = "NVIDIA Seaboard";
+	compatible = "nvidia,seaboard", "nvidia,tegra20";
+
+	chosen {
+		bootargs = "vmalloc=192M video=tegrafb console=ttyS0,115200n8 root=/dev/mmcblk1p3 rw rootwait";
+	};
+
+	memory {
+		device_type = "memory";
+		reg = < 0x00000000 0x40000000 >;
+	};
+
+	serial at 70006300 {
+		clock-frequency = < 216000000 >;
+	};
+
+	sdhci at c8000400 {
+		cd-gpios = <&gpio 69 0>; /* gpio PI5 */
+		wp-gpios = <&gpio 57 0>; /* gpio PH1 */
+		power-gpios = <&gpio 70 0>; /* gpio PI6 */
+	};
+
+	sdhci at c8000600 {
+		support-8bit;
+	};
+
+	usb at c5000000 {
+		nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
+	};
+};
-- 
1.7.7.3

[U-Boot] [PATCH v4 07/20] tegra: fdt: Add device tree file for Tegra2 Seaboard from kernel

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> This was taken from commit b48c54e2 at:
> git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts

This is also somewhat out-of-date w.r.t. the latest in kernel tag
next-20120118. It's probably not too much of an issue, so this is
probably fine to apply as-is. U-Boot may benefit from the addition of
many status="disable" properties, and clock-frequency properties for the
I2C nodes. It'd be good to keep U-Boot up-to-date with the kernel as
much as possible. Of course, there are ongoing changes being queued for
the kernel as work is done too...

-- 
nvpublic

[U-Boot] [PATCH v4 07/20] tegra: fdt: Add device tree file for Tegra2 Seaboard from kernel

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 2:28 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> This was taken from commit b48c54e2 at:
>> git://git.kernel.org/pub/scm/linux/kernel/git/olof/tegra.git
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>> diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
>
> This is also somewhat out-of-date w.r.t. the latest in kernel tag
> next-20120118. It's probably not too much of an issue, so this is
> probably fine to apply as-is. U-Boot may benefit from the addition of
> many status="disable" properties, and clock-frequency properties for the
> I2C nodes. It'd be good to keep U-Boot up-to-date with the kernel as
> much as possible. Of course, there are ongoing changes being queued for
> the kernel as work is done too...
>

OK once everything is in we can do any updates that then exist based
on what is finalised in the kernel.

Regards,
Simon

> --
> nvpublic

[U-Boot] [PATCH v4 08/20] fdt: Add staging area for device tree binding documentation

[Simon Glass]

Add a directory to hold device tree binding files, to permit easy review
of this material in U-Boot patches.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v4:
- Add staging area for device tree bindings used in U-Boot

 doc/device-tree-bindings/README |   19 +++++++++++++++++++
 1 files changed, 19 insertions(+), 0 deletions(-)
 create mode 100644 doc/device-tree-bindings/README

diff --git a/doc/device-tree-bindings/README b/doc/device-tree-bindings/README
new file mode 100644
index 0000000..84e8cba
--- /dev/null
+++ b/doc/device-tree-bindings/README
@@ -0,0 +1,19 @@
+Device Tree Bindings Staging Area
+=================================
+
+This directory contains device tree bindings for U-Boot.
+
+These follow along with Linux kernel bindings, with a few additions. By
+adding the files here, U-Boot patches can clearly show thees additions.
+This makes it easier for device tree people to review these additions in
+patches sent to the U-Boot mailing list.
+
+The intent is not to commit these files to U-Boot, or at least only
+temporarily, Rather we hope that the files will be stored in another
+repo (shared with Linux) which is brought in as needed. Failing that,
+changes here will be  commited to the Linux
+Documentation/devicetree/bindings/ directory.
+
+
+sjg at chromium.org
+11-Jan-12
-- 
1.7.7.3

[U-Boot] [PATCH v4 08/20] fdt: Add staging area for device tree binding documentation

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> Add a directory to hold device tree binding files, to permit easy review
> of this material in U-Boot patches.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>

> diff --git a/doc/device-tree-bindings/README b/doc/device-tree-bindings/README

> +The intent is not to commit these files to U-Boot, or at least only
> +temporarily, Rather we hope that the files will be stored in another
> +repo (shared with Linux) which is brought in as needed. Failing that,
> +changes here will be  commited to the Linux
> +Documentation/devicetree/bindings/ directory.

Given there is no shared repo right now, I'd assert we should be
specific and explicitly state that the intent /is/ to check in the
binding documentation here, and mirror the kernel tree.

But, the patch is fine to me as-is if you wish.

-- 
nvpublic

[U-Boot] [PATCH v4 08/20] fdt: Add staging area for device tree binding documentation

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 2:30 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> Add a directory to hold device tree binding files, to permit easy review
>> of this material in U-Boot patches.
>>
>> Signed-off-by: Simon Glass <sjg@chromium.org>
>
>> diff --git a/doc/device-tree-bindings/README b/doc/device-tree-bindings/README
>
>> +The intent is not to commit these files to U-Boot, or at least only
>> +temporarily, Rather we hope that the files will be stored in another
>> +repo (shared with Linux) which is brought in as needed. Failing that,
>> +changes here will be ?commited to the Linux
>> +Documentation/devicetree/bindings/ directory.
>
> Given there is no shared repo right now, I'd assert we should be
> specific and explicitly state that the intent /is/ to check in the
> binding documentation here, and mirror the kernel tree.
>
> But, the patch is fine to me as-is if you wish.

I'm fine with this too, it probably is more sensible if the U-Boot PTB
don't mind.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 09/20] fdt: Add tegra-usb bindings file from linux

[Simon Glass]

This file is taken from the Linux mailing list.

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

 doc/device-tree-bindings/usb/tegra-usb.txt |   13 +++++++++++++
 1 files changed, 13 insertions(+), 0 deletions(-)
 create mode 100644 doc/device-tree-bindings/usb/tegra-usb.txt

diff --git a/doc/device-tree-bindings/usb/tegra-usb.txt b/doc/device-tree-bindings/usb/tegra-usb.txt
new file mode 100644
index 0000000..035d63d
--- /dev/null
+++ b/doc/device-tree-bindings/usb/tegra-usb.txt
@@ -0,0 +1,13 @@
+Tegra SOC USB controllers
+
+The device node for a USB controller that is part of a Tegra
+SOC is as described in the document "Open Firmware Recommended
+Practice : Universal Serial Bus" with the following modifications
+and additions :
+
+Required properties :
+ - compatible : Should be "nvidia,tegra20-ehci" for USB controllers
+   used in host mode.
+ - phy_type : Should be one of "ulpi" or "utmi".
+ - nvidia,vbus-gpio : If present, specifies a gpio that needs to be
+   activated for the bus to be powered.
-- 
1.7.7.3

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Simon Glass]

This adds a property to indicate a port which can switch between host and device
mode.

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

 doc/device-tree-bindings/usb/tegra-usb.txt |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/doc/device-tree-bindings/usb/tegra-usb.txt b/doc/device-tree-bindings/usb/tegra-usb.txt
index 035d63d..96fd022 100644
--- a/doc/device-tree-bindings/usb/tegra-usb.txt
+++ b/doc/device-tree-bindings/usb/tegra-usb.txt
@@ -11,3 +11,7 @@ Required properties :
  - phy_type : Should be one of "ulpi" or "utmi".
  - nvidia,vbus-gpio : If present, specifies a gpio that needs to be
    activated for the bus to be powered.
+
+Optional properties:
+ - support-host-mode : If present then this peripheral can switch between
+   host and device mode
-- 
1.7.7.3

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Stephen Warren]

On 01/11/2012 09:32 PM, Simon Glass wrote:
> This adds a property to indicate a port which can switch between host and device
> mode.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  doc/device-tree-bindings/usb/tegra-usb.txt |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
> 
> diff --git a/doc/device-tree-bindings/usb/tegra-usb.txt b/doc/device-tree-bindings/usb/tegra-usb.txt
> index 035d63d..96fd022 100644
> --- a/doc/device-tree-bindings/usb/tegra-usb.txt
> +++ b/doc/device-tree-bindings/usb/tegra-usb.txt
> @@ -11,3 +11,7 @@ Required properties :
>   - phy_type : Should be one of "ulpi" or "utmi".
>   - nvidia,vbus-gpio : If present, specifies a gpio that needs to be
>     activated for the bus to be powered.
> +
> +Optional properties:
> + - support-host-mode : If present then this peripheral can switch between
> +   host and device mode

All of Tegra's USB ports support host mode, and it's the typical mode of
operation. The TRM also indicates that all USB ports support device mode
(possibly depending on the PHY type that's been selected for or attached
to port). Port 2 explicitly doesn't support run-time switching, but by
omission, ports 1 and 3 do.

Hence, "support-host-mode" is not an accurate name, and doesn't match
the description given either.

I think that it'd be better to:
* Add properties to explicitly specify whether the port is
intended/allowed to operate (per board design) in each of host or device
mode.
* Default to static host mode if no properties are present.

i.e.:

+Optional properties:
+ - enable-host-mode : Indicate that the port is intended to operate in
+   host mode.
+ - enable-device-mode : Indicate that the port is intended to operate
+   in device mode.

We should error out if both properties are set for USB 2, since it
doesn't support run-time switching. Perhaps you can skip this
error-checking for now since we aren't implementing device mode anywhere
anyway...

Then in the later patch in your series that was using support-host-mode,

* Do VBUS detection only if both enable-host-mode and enable-device-mode
are present.
* Do VBUS GPIO enable if enable-host-mode is present (or assumed by
default) and VBUS detection passed (if it ran).

-- 
nvpublic

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Olof Johansson]

On Wed, Jan 18, 2012 at 03:48:30PM -0700, Stephen Warren wrote:
> On 01/11/2012 09:32 PM, Simon Glass wrote:
> > This adds a property to indicate a port which can switch between host and device
> > mode.
> > 
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> > 
> >  doc/device-tree-bindings/usb/tegra-usb.txt |    4 ++++
> >  1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > diff --git a/doc/device-tree-bindings/usb/tegra-usb.txt b/doc/device-tree-bindings/usb/tegra-usb.txt
> > index 035d63d..96fd022 100644
> > --- a/doc/device-tree-bindings/usb/tegra-usb.txt
> > +++ b/doc/device-tree-bindings/usb/tegra-usb.txt
> > @@ -11,3 +11,7 @@ Required properties :
> >   - phy_type : Should be one of "ulpi" or "utmi".
> >   - nvidia,vbus-gpio : If present, specifies a gpio that needs to be
> >     activated for the bus to be powered.
> > +
> > +Optional properties:
> > + - support-host-mode : If present then this peripheral can switch between
> > +   host and device mode
> 
> All of Tegra's USB ports support host mode, and it's the typical mode of
> operation. The TRM also indicates that all USB ports support device mode
> (possibly depending on the PHY type that's been selected for or attached
> to port). Port 2 explicitly doesn't support run-time switching, but by
> omission, ports 1 and 3 do.
> 
> Hence, "support-host-mode" is not an accurate name, and doesn't match
> the description given either.
> 
> I think that it'd be better to:
> * Add properties to explicitly specify whether the port is
> intended/allowed to operate (per board design) in each of host or device
> mode.
> * Default to static host mode if no properties are present.
> 
> i.e.:
> 
> +Optional properties:
> + - enable-host-mode : Indicate that the port is intended to operate in
> +   host mode.
> + - enable-device-mode : Indicate that the port is intended to operate
> +   in device mode.

fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
the shared IP in question for device mode:

 - dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
   controllers.  Can be "host", "peripheral", or "otg".  Default to
   "host" if not defined for backward compatibility.


-Olof

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Simon Glass]

Hi Olof,

On Wed, Jan 18, 2012 at 9:35 PM, Olof Johansson <olof@lixom.net> wrote:
> On Wed, Jan 18, 2012 at 03:48:30PM -0700, Stephen Warren wrote:
>> On 01/11/2012 09:32 PM, Simon Glass wrote:
>> > This adds a property to indicate a port which can switch between host and device
>> > mode.
>> >
>> > Signed-off-by: Simon Glass <sjg@chromium.org>
>> > ---
>> >
>> > ?doc/device-tree-bindings/usb/tegra-usb.txt | ? ?4 ++++
>> > ?1 files changed, 4 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/doc/device-tree-bindings/usb/tegra-usb.txt b/doc/device-tree-bindings/usb/tegra-usb.txt
>> > index 035d63d..96fd022 100644
>> > --- a/doc/device-tree-bindings/usb/tegra-usb.txt
>> > +++ b/doc/device-tree-bindings/usb/tegra-usb.txt
>> > @@ -11,3 +11,7 @@ Required properties :
>> > ? - phy_type : Should be one of "ulpi" or "utmi".
>> > ? - nvidia,vbus-gpio : If present, specifies a gpio that needs to be
>> > ? ? activated for the bus to be powered.
>> > +
>> > +Optional properties:
>> > + - support-host-mode : If present then this peripheral can switch between
>> > + ? host and device mode
>>
>> All of Tegra's USB ports support host mode, and it's the typical mode of
>> operation. The TRM also indicates that all USB ports support device mode
>> (possibly depending on the PHY type that's been selected for or attached
>> to port). Port 2 explicitly doesn't support run-time switching, but by
>> omission, ports 1 and 3 do.
>>
>> Hence, "support-host-mode" is not an accurate name, and doesn't match
>> the description given either.
>>
>> I think that it'd be better to:
>> * Add properties to explicitly specify whether the port is
>> intended/allowed to operate (per board design) in each of host or device
>> mode.
>> * Default to static host mode if no properties are present.
>>
>> i.e.:
>>
>> +Optional properties:
>> + - enable-host-mode : Indicate that the port is intended to operate in
>> + ? host mode.
>> + - enable-device-mode : Indicate that the port is intended to operate
>> + ? in device mode.
>
> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
> the shared IP in question for device mode:
>
> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
> ? "host" if not defined for backward compatibility.

What does 'dr' mean?

Regards,
Simon

>
>
> -Olof

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Olof Johansson]

Hi,

On Wed, Jan 18, 2012 at 9:55 PM, Simon Glass <sjg@chromium.org> wrote:

>> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
>> the shared IP in question for device mode:
>>
>> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
>> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
>> ? "host" if not defined for backward compatibility.
>
> What does 'dr' mean?

After some searching, it looks like it means "dual-role", which seems
appropriate terminology for the tegra SoC as well.


-Olof

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Simon Glass]

Hi Olof,

On Wed, Jan 18, 2012 at 10:41 PM, Olof Johansson <olof@lixom.net> wrote:
> Hi,
>
> On Wed, Jan 18, 2012 at 9:55 PM, Simon Glass <sjg@chromium.org> wrote:
>
>>> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
>>> the shared IP in question for device mode:
>>>
>>> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
>>> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
>>> ? "host" if not defined for backward compatibility.
>>
>> What does 'dr' mean?
>
> After some searching, it looks like it means "dual-role", which seems
> appropriate terminology for the tegra SoC as well.

Thanks for looking that up.

Well maybe, but it could mean data reverse mode or distinctive
ring...I like Stephen's nice device-tree-friendly naming and
explanation :-) Are we stuck with this?

Regards,
Simon

>
>
> -Olof

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Olof Johansson]

On Wed, Jan 18, 2012 at 10:59 PM, Simon Glass <sjg@chromium.org> wrote:
> Hi Olof,
>
> On Wed, Jan 18, 2012 at 10:41 PM, Olof Johansson <olof@lixom.net> wrote:
>> Hi,
>>
>> On Wed, Jan 18, 2012 at 9:55 PM, Simon Glass <sjg@chromium.org> wrote:
>>
>>>> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
>>>> the shared IP in question for device mode:
>>>>
>>>> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
>>>> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
>>>> ? "host" if not defined for backward compatibility.
>>>
>>> What does 'dr' mean?
>>
>> After some searching, it looks like it means "dual-role", which seems
>> appropriate terminology for the tegra SoC as well.
>
> Thanks for looking that up.
>
> Well maybe, but it could mean data reverse mode or distinctive
> ring...I like Stephen's nice device-tree-friendly naming and
> explanation :-) Are we stuck with this?

I think there's value in staying common with similar bindings on other
platforms, yes. Sorry. :)


-Olof

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Stephen Warren]

Olof Johansson wrote at Thursday, January 19, 2012 12:13 AM:
> On Wed, Jan 18, 2012 at 10:59 PM, Simon Glass <sjg@chromium.org> wrote:
> > Hi Olof,
> >
> > On Wed, Jan 18, 2012 at 10:41 PM, Olof Johansson <olof@lixom.net> wrote:
> >> Hi,
> >>
> >> On Wed, Jan 18, 2012 at 9:55 PM, Simon Glass <sjg@chromium.org> wrote:
> >>
> >>>> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
> >>>> the shared IP in question for device mode:
> >>>>
> >>>> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
> >>>> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
> >>>> ? "host" if not defined for backward compatibility.
> >>>
> >>> What does 'dr' mean?
> >>
> >> After some searching, it looks like it means "dual-role", which seems
> >> appropriate terminology for the tegra SoC as well.
> >
> > Thanks for looking that up.
> >
> > Well maybe, but it could mean data reverse mode or distinctive
> > ring...I like Stephen's nice device-tree-friendly naming and
> > explanation :-) Are we stuck with this?
> 
> I think there's value in staying common with similar bindings on other
> platforms, yes. Sorry. :)

Ack from me too. If there's an existing convention, I'd prefer to stick
with it. Sorry, I should have looked this up before coming up with
something new.

-- 
nvpublic

[U-Boot] [PATCH v4 10/20] tegra: fdt: Add additional USB binding

[Simon Glass]

Hi,

On Thu, Jan 19, 2012 at 9:33 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Olof Johansson wrote at Thursday, January 19, 2012 12:13 AM:
>> On Wed, Jan 18, 2012 at 10:59 PM, Simon Glass <sjg@chromium.org> wrote:
>> > Hi Olof,
>> >
>> > On Wed, Jan 18, 2012 at 10:41 PM, Olof Johansson <olof@lixom.net> wrote:
>> >> Hi,
>> >>
>> >> On Wed, Jan 18, 2012 at 9:55 PM, Simon Glass <sjg@chromium.org> wrote:
>> >>
>> >>>> fsl-usb.txt uses "dr_mode" here, which might make sense to reuse given some of
>> >>>> the shared IP in question for device mode:
>> >>>>
>> >>>> ?- dr_mode : indicates the working mode for "fsl-usb2-dr" compatible
>> >>>> ? controllers. ?Can be "host", "peripheral", or "otg". ?Default to
>> >>>> ? "host" if not defined for backward compatibility.
>> >>>
>> >>> What does 'dr' mean?
>> >>
>> >> After some searching, it looks like it means "dual-role", which seems
>> >> appropriate terminology for the tegra SoC as well.
>> >
>> > Thanks for looking that up.
>> >
>> > Well maybe, but it could mean data reverse mode or distinctive
>> > ring...I like Stephen's nice device-tree-friendly naming and
>> > explanation :-) Are we stuck with this?
>>
>> I think there's value in staying common with similar bindings on other
>> platforms, yes. Sorry. :)
>
> Ack from me too. If there's an existing convention, I'd prefer to stick
> with it. Sorry, I should have looked this up before coming up with
> something new.

OK thanks for that. Will take a look.

Regards,
Simon

>
> --
> nvpublic
>

[U-Boot] [PATCH v4 11/20] tegra: fdt: Add clock bindings

[Simon Glass]

This adds a basic binding for the oscillator and peripheral clocks. The
second cell is the clock number, defined as the bit number within the clock
enable register if the peripheral clock.

This uses the RFC clock bindings from Grant Likely so may change later:

https://lkml.org/lkml/2011/12/12/498

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v4:
- Add clock bindings for Tegra2x

 arch/arm/dts/tegra20.dtsi                      |   27 ++++++++++++
 doc/device-tree-bindings/clock/tegra-periphclk |   51 ++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 0 deletions(-)
 create mode 100644 doc/device-tree-bindings/clock/tegra-periphclk

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index 6146d24..68eae12 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -4,6 +4,33 @@
 	compatible = "nvidia,tegra20";
 	interrupt-parent = <&intc>;
 
+	clocks = <&osc_clk>;
+	clock-names = "osc_clk";
+	clock-ranges;
+
+	clocks {
+		#address-cells = <1>;
+		#size-cells = <1>;
+
+		/* The frequency of this clock is board-specific */
+		osc_clk: oscclk {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+		};
+
+		/*
+		 * This node provides clocks to all peripherals. We don't
+		 * enumerate the clock names for now since there are no
+		 * users of this information.
+		 */
+		periph_clk: periphclk {
+			compatible = "tegra,periphclk";
+			#clock-cells = <1>;
+			clocks = <&osc_clk>;
+			reg = <0x60006000 400>;
+		};
+	};
+
 	intc: interrupt-controller at 50041000 {
 		compatible = "nvidia,tegra20-gic";
 		interrupt-controller;
diff --git a/doc/device-tree-bindings/clock/tegra-periphclk b/doc/device-tree-bindings/clock/tegra-periphclk
new file mode 100644
index 0000000..8d21e4d
--- /dev/null
+++ b/doc/device-tree-bindings/clock/tegra-periphclk
@@ -0,0 +1,51 @@
+Clock controllers
+
+(there isn't yet a binding in Linux, so this describes what is in U-Boot)
+
+The device node for a clock controller is as described in the document
+"Open Firmware Recommended Practice : Universal Serial Bus" with the
+following modifications and additions :
+
+This is based on Grant Likely's proposed patch for clock bindings.
+
+Required properties :
+ - compatible : Should be "tegra,periphclk" for peripheral clock controller
+ - clocks : Should contain a single phandle pointing to the oscillator clock
+
+Peripherals which refer to a clock should have a property called "clocks" with
+two cells: phandle of the peripheral clock and the clock ID number (which
+is the bit number in the peripheral clock controller enable register numbered
+from 0).
+
+Example:
+
+clocks {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	/* The frequency of this clock is board-specific */
+	osc_clk: oscclk {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+	};
+
+	/*
+	 * This node provides clocks to all peripherals. We don't
+	 * enumerate the clock names for now since there are no
+	 * users of this information.
+	 */
+	periph_clk: periphclk {
+		compatible = "tegra,periphclk";
+		#clock-cells = <1>;
+		clocks = <&osc_clk>;
+		reg = <0x60006000 400>;
+	};
+};
+
+usb at c5004000 {
+	compatible = "nvidia,tegra20-ehci", "usb-ehci";
+	reg = <0xc5004000 0x4000>;
+	interrupts = < 53 >;
+	phy_type = "ulpi";
+	clocks = <&periph_clk 58>;	// PERIPH_ID_USB2
+};
-- 
1.7.7.3

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Document a binding for the Tegra20 CAR (Clock And Reset) Controller,
add it to tegra20.dtsi, and configure it for the board in tegra-
seaboard.dts.

Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
All, Simon Glass is attempting to upstream Tegra USB support in U-Boot.
The driver is only instantiated from device tree, and needs to configure
clocks for the USB module. Hence, he proposed a clock binding for Tegra.
I've read through Grant's proposed common clock bindings and reworked
Simon's proposal a little, ending up with the patch below for the kernel.
I'd appreciate any comments you have.

Grant, can you confirm that it's reasonable to start making use of your
proposed common clock bindings in U-Boot, even though they're only an RFC?

Grant, there are a couple of FIXMEs in the binding documentation below.
Can you give any insight on these?

A comment on the shared enable issue:

When enabling/disabling clocks, Tegra requires you to reset the HW module
that the clock is routed to. Both reset and clock enable registers are
part of the CAR. U-Boot currently has an API to do the reset based on a
"peripheral ID", which simply means a bit number in a set of 3 "reset"
registers. The bits in those registers share the same numbering as the
"clock enable" registers. Hence, to make life easy for U-Boot, I've
defined the clock IDs in this binding to be equal to these bit numbers.
However, this breaks down where there isn't a 1:1 mapping between reset
and clock enable bits, and clocks. For example, there's a single reset
and clock enable bit for the SPDIF controller, yet this applies to two
clocks; spdif_in and spdif_out. Hence, this simplification for U-Boot
breaks down. I think the correct solution is to fix U-Boot not to
require this simplification, renumber the clocks in the CAR binding in
a completely arbitrary fashion, and require U-Boot to contain a mapping
table between CAR's DT clock IDs and any other information related to
those clocks. Do you see any alternative? We really need the two SPDIF
clocks to be different clock IDs in the binding, since each has a
completely independant mux for the clock source/parent, and the two
clocks obviously can't share the same clock ID (well, hmm, perhaps they
could if we used 2 cells for the specifier, 1 for the bit number, and
one more to differentiate clocks that use the same bit...). I'm still
inclined to simply push back on U-Boot to do it right.

 .../bindings/clock/nvidia,tegra20-car.txt          |  156 ++++++++++++++++++++
 arch/arm/boot/dts/tegra-seaboard.dts               |   18 +++
 arch/arm/boot/dts/tegra20.dtsi                     |    7 +
 3 files changed, 181 insertions(+), 0 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt

diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
new file mode 100644
index 0000000..71cfdd2
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
@@ -0,0 +1,156 @@
+* NVIDIA Tegra20 Clock And Reset Controller
+
+This binding uses the common clock binding:
+Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
+for muxing and gating Tegra's clocks, and setting their rates.
+
+Required properties :
+- compatible : Should be "nvidia,<chip>-car"
+- reg : Should contain CAR registers location and length
+- clocks : Should contain phandle and clock specifiers for two clocks:
+  the 32 KHz "32k_in", and the board-specific oscillator "osc".
+- clock-names : Should contain a list of strings, with values "32k_in",
+  and "osc".
+- #clock-cells : Should be 1.
+  In clock consumers, this cell represents the clock ID exposed by the CAR.
+  For a list of valid values, see the clock-output-names property.
+
+Optional properties :
+- clock-output-names : Should contain a list of strings defining the clocks
+  that the CAR provides. The list of names and clock IDs is below. The IDs
+  may be used in clock specifiers. The names should be listed in this clock-
+  output-names property, sorted in ID order, if this property is present.
+
+  The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
+  registers. Later, subsequent IDs will be assigned to all other clocks the
+  CAR controls.
+
+    0   "cpu"
+    1   "unassigned"
+    2   "unassigned"    FIXME: Is it OK to have 2 identical (unused) names?
+    3   "ac97"
+    4   "rtc"
+    5   "tmr"
+    6   "uart1"
+    7   "uart2"
+    8   "gpio"
+    9   "sdmmc2"
+    10  "spdif"         FIXME: One enable bit controls two clocks!!!
+    11  "i2s1"
+    12  "i2c1"
+    13  "ndflash"
+    14  "sdmmc1"
+    15  "sdmmc4"
+    16  "twc"
+    17  "pwm"
+    18  "i2s2"
+    19  "epp"
+    20  "vi"            FIXME: One enable bit controls two clocks!!!
+    21  "2d"
+    22  "usbd"
+    23  "isp"
+    24  "3d"
+    25  "ide"
+    26  "disp2"
+    27  "disp1"
+    28  "host1x"
+    29  "vcp"
+    30  "unassigned"
+    31  "cache2"
+
+    32  "mem"
+    33  "ahbdma"
+    34  "apbdma"
+    35  "unassigned"
+    36  "kbc"
+    37  "stat_mon"
+    38  "pmc"
+    39  "fuse"
+    40  "kfuse"
+    41  "sbc1"
+    42  "snor"
+    43  "spi"
+    44  "sbc2"
+    45  "xio"
+    46  "sbc3"
+    47  "dvc_i2c"
+    48  "dsi"
+    49  "tvo"
+    50  "mipi"
+    51  "hdmi"
+    52  "csi"
+    53  "tvdac"
+    54  "i2c2"
+    55  "uart3"
+    56  "unassigned"
+    57  "emc"
+    58  "usb2"
+    59  "usb3"
+    60  "mpe"
+    61  "vde"
+    62  "bsea"
+    63  "bsev"
+
+    64  "speedo"
+    65  "uart4"
+    66  "uart5"
+    67  "i2c3"
+    68  "sbc4"
+    69  "sdmmc3"
+    70  "pcie"
+    71  "owr"
+    72  "afi"
+    73  "csite"
+    74  "unassigned"
+    75  "avpucq"
+    76  "unassigned"
+    77  "unassigned"
+    78  "unassigned"
+    79  "unassigned"
+    80  "unassigned"
+    81  "unassigned"
+    82  "unassigned"
+    83  "unassigned"
+    84  "irama"
+    85  "iramb"
+    86  "iramc"
+    87  "iramd"
+    88  "cram2"
+    89  "audio_2x"
+    90  "clk_d"
+    91  "unassigned"
+    92  "sus"
+    93  "cdev1"
+    94  "cdev2"
+    95  "unassigned"
+
+Example:
+
+clocks {
+	clk_32k: oscillator at 0 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <32768>;
+	};
+
+	osc: oscillator at 1 {
+		compatible = "fixed-clock";
+		#clock-cells = <0>;
+		clock-frequency = <12000000>;
+	};
+};
+
+tegar_car: clock at 60006000 {
+	compatible = "tegra,periphclk";
+	reg = <0x60006000 1000>;
+	clocks = <&clk_32k> <&osc>;
+	clock-names = "32k_in", "osc";
+	#clock-cells = <1>;
+};
+
+usb at c5004000 {
+	...
+	clocks = <&tegra_car 58>; /* usb2 */
+};
diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
index b55a02e..f9347fe 100644
--- a/arch/arm/boot/dts/tegra-seaboard.dts
+++ b/arch/arm/boot/dts/tegra-seaboard.dts
@@ -11,6 +11,24 @@
 		reg = < 0x00000000 0x40000000 >;
 	};
 
+	clocks {
+		clk_32k: oscillator at 0 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <32768>;
+		};
+
+		osc: oscillator at 1 {
+			compatible = "fixed-clock";
+			#clock-cells = <0>;
+			clock-frequency = <12000000>;
+		};
+	};
+
+	clock at 60006000 {
+		clocks = <&clk_32k> <&osc>;
+	};
+
 	i2c at 7000c000 {
 		clock-frequency = <400000>;
 	};
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 3da7afd..be4abd2 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -4,6 +4,13 @@
 	compatible = "nvidia,tegra20";
 	interrupt-parent = <&intc>;
 
+	tegar_car: clock at 60006000 {
+		compatible = "tegra,periphclk";
+		reg = <0x60006000 1000>;
+		clock-names = "32k_in", "osc";
+		#clock-cells = <1>;
+	};
+
 	intc: interrupt-controller at 50041000 {
 		compatible = "arm,cortex-a9-gic";
 		interrupt-controller;
-- 
1.7.0.4

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Olof Johansson]

Hi,

On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:

>  .../bindings/clock/nvidia,tegra20-car.txt          |  156 ++++++++++++++++++++
>  arch/arm/boot/dts/tegra-seaboard.dts               |   18 +++
>  arch/arm/boot/dts/tegra20.dtsi                     |    7 +
>  3 files changed, 181 insertions(+), 0 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> new file mode 100644
> index 0000000..71cfdd2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> @@ -0,0 +1,156 @@
> +* NVIDIA Tegra20 Clock And Reset Controller
> +
> +This binding uses the common clock binding:
> +Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> +for muxing and gating Tegra's clocks, and setting their rates.
> +
> +Required properties :
> +- compatible : Should be "nvidia,<chip>-car"
> +- reg : Should contain CAR registers location and length
> +- clocks : Should contain phandle and clock specifiers for two clocks:
> +  the 32 KHz "32k_in", and the board-specific oscillator "osc".
> +- clock-names : Should contain a list of strings, with values "32k_in",
> +  and "osc".

Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
where it isn't strictly necessary. Just because some vendors don't want
to define an order between their clocks doesn't mean it's a good idea
for everybody to use that model. In this case, just declaring that the
two clocks refs have to be to those two clocks in that order should
be sufficient.

> +- #clock-cells : Should be 1.
> +  In clock consumers, this cell represents the clock ID exposed by the CAR.
> +  For a list of valid values, see the clock-output-names property.
> +
> +Optional properties :
> +- clock-output-names : Should contain a list of strings defining the clocks
> +  that the CAR provides. The list of names and clock IDs is below. The IDs
> +  may be used in clock specifiers. The names should be listed in this clock-
> +  output-names property, sorted in ID order, if this property is present.
> +
> +  The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
> +  registers. Later, subsequent IDs will be assigned to all other clocks the
> +  CAR controls.

Aren't these names hardcoded per SoC? If so, they can be derived from the
compatible field + output number without having a table in the device tree.

If anything, you might want to enumerate the allowed/expected values, but
actually putting them in a property seems overkill.

[...]

> +Example:
> +
> +clocks {
> +	clk_32k: oscillator at 0 {

If it has a unit addres (@<x>) it needs a reg property with the same base
address.

> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <32768>;
> +	};
> +
> +	osc: oscillator at 1 {
> +		compatible = "fixed-clock";
> +		#clock-cells = <0>;
> +		clock-frequency = <12000000>;
> +	};
> +};
> +
> +tegar_car: clock at 60006000 {

typo? tegra_car?

> +	compatible = "tegra,periphclk";
> +	reg = <0x60006000 1000>;
> +	clocks = <&clk_32k> <&osc>;
> +	clock-names = "32k_in", "osc";
> +	#clock-cells = <1>;
> +};
> +
> +usb at c5004000 {
> +	...
> +	clocks = <&tegra_car 58>; /* usb2 */
> +};

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> > +* NVIDIA Tegra20 Clock And Reset Controller
> > +
> > +This binding uses the common clock binding:
> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> > +for muxing and gating Tegra's clocks, and setting their rates.
> > +
> > +Required properties :
> > +- compatible : Should be "nvidia,<chip>-car"
> > +- reg : Should contain CAR registers location and length
> > +- clocks : Should contain phandle and clock specifiers for two clocks:
> > +  the 32 KHz "32k_in", and the board-specific oscillator "osc".
> > +- clock-names : Should contain a list of strings, with values "32k_in",
> > +  and "osc".
> 
> Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
> where it isn't strictly necessary. Just because some vendors don't want
> to define an order between their clocks doesn't mean it's a good idea
> for everybody to use that model. In this case, just declaring that the
> two clocks refs have to be to those two clocks in that order should
> be sufficient.

OK, that seems reasonable. I'm happy using of_clk_get() rather than
of_clk_get_by_name(). I guess that means we should just avoid any
discussion of clock-output-names too.

> > +- #clock-cells : Should be 1.
> > +  In clock consumers, this cell represents the clock ID exposed by the CAR.
> > +  For a list of valid values, see the clock-output-names property.
> > +
> > +Optional properties :
> > +- clock-output-names : Should contain a list of strings defining the clocks
> > +  that the CAR provides. The list of names and clock IDs is below. The IDs
> > +  may be used in clock specifiers. The names should be listed in this clock-
> > +  output-names property, sorted in ID order, if this property is present.
> > +
> > +  The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
> > +  registers. Later, subsequent IDs will be assigned to all other clocks the
> > +  CAR controls.
> 
> Aren't these names hardcoded per SoC? If so, they can be derived from the
> compatible field + output number without having a table in the device tree.
>
> If anything, you might want to enumerate the allowed/expected values, but
> actually putting them in a property seems overkill.

Yes, the set of clocks is hard-coded per SoC, and the clock is uniquely
identified by compatible+id.

clock-output-names doesn't actually serve any functional purpose in
Grant's common clock bindings patches; it's more of a documentation
thing. As such, yes, I can remove the suggestion to actually put the
table into the device tree, and rework the binding to simply define
what the mapping is independently.

...
> > +Example:
> > +
> > +clocks {
> > +	clk_32k: oscillator at 0 {
> 
> If it has a unit addres (@<x>) it needs a reg property with the same base
> address.

I thought everything needed a unit address period? Is the rule more like
the unit address is optional, but if present must match reg, so I can
simply remove @0 and @1 here? I guess that makes a lot more sense.

> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <32768>;
> > +	};
> > +
> > +	osc: oscillator at 1 {
> > +		compatible = "fixed-clock";
> > +		#clock-cells = <0>;
> > +		clock-frequency = <12000000>;
> > +	};
> > +};

-- 
nvpublic

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Olof Johansson]

Hi,

On Thu, Jan 19, 2012 at 9:17 AM, Stephen Warren <swarren@nvidia.com> wrote:
> Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
>> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
>> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>> > +* NVIDIA Tegra20 Clock And Reset Controller
>> > +
>> > +This binding uses the common clock binding:
>> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
>> > +
>> > +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
>> > +for muxing and gating Tegra's clocks, and setting their rates.
>> > +
>> > +Required properties :
>> > +- compatible : Should be "nvidia,<chip>-car"
>> > +- reg : Should contain CAR registers location and length
>> > +- clocks : Should contain phandle and clock specifiers for two clocks:
>> > + ?the 32 KHz "32k_in", and the board-specific oscillator "osc".
>> > +- clock-names : Should contain a list of strings, with values "32k_in",
>> > + ?and "osc".
>>
>> Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
>> where it isn't strictly necessary. Just because some vendors don't want
>> to define an order between their clocks doesn't mean it's a good idea
>> for everybody to use that model. In this case, just declaring that the
>> two clocks refs have to be to those two clocks in that order should
>> be sufficient.
>
> OK, that seems reasonable. I'm happy using of_clk_get() rather than
> of_clk_get_by_name(). I guess that means we should just avoid any
> discussion of clock-output-names too.

Sounds good to me. Let's make sure Grant is OK with it too though.

>> > +- #clock-cells : Should be 1.
>> > + ?In clock consumers, this cell represents the clock ID exposed by the CAR.
>> > + ?For a list of valid values, see the clock-output-names property.
>> > +
>> > +Optional properties :
>> > +- clock-output-names : Should contain a list of strings defining the clocks
>> > + ?that the CAR provides. The list of names and clock IDs is below. The IDs
>> > + ?may be used in clock specifiers. The names should be listed in this clock-
>> > + ?output-names property, sorted in ID order, if this property is present.
>> > +
>> > + ?The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
>> > + ?registers. Later, subsequent IDs will be assigned to all other clocks the
>> > + ?CAR controls.
>>
>> Aren't these names hardcoded per SoC? If so, they can be derived from the
>> compatible field + output number without having a table in the device tree.
>>
>> If anything, you might want to enumerate the allowed/expected values, but
>> actually putting them in a property seems overkill.
>
> Yes, the set of clocks is hard-coded per SoC, and the clock is uniquely
> identified by compatible+id.
>
> clock-output-names doesn't actually serve any functional purpose in
> Grant's common clock bindings patches; it's more of a documentation
> thing. As such, yes, I can remove the suggestion to actually put the
> table into the device tree, and rework the binding to simply define
> what the mapping is independently.

Again, sounds good to me.

>> > +Example:
>> > +
>> > +clocks {
>> > + ? clk_32k: oscillator at 0 {
>>
>> If it has a unit addres (@<x>) it needs a reg property with the same base
>> address.
>
> I thought everything needed a unit address period? Is the rule more like
> the unit address is optional, but if present must match reg, so I can
> simply remove @0 and @1 here? I guess that makes a lot more sense.

Nope, you only need a unit address to make a node unique, i.e. if you
have more than one with the same name. It's not something that's been
followed very well on ARM dts files, I have even seen review comments
asking for explicit unit IDs when none are needed.

So you can't remove @0 and @1 here, since there are two oscillator subnodes.

I'm not 100% sure if you have to have the unit address represented as
"reg" or not, but it should probably be represented by _something_ in
the properties of the node. Mitch? Segher? :)


>> > + ? ? ? ? ? compatible = "fixed-clock";
>> > + ? ? ? ? ? #clock-cells = <0>;
>> > + ? ? ? ? ? clock-frequency = <32768>;
>> > + ? };
>> > +
>> > + ? osc: oscillator at 1 {
>> > + ? ? ? ? ? compatible = "fixed-clock";
>> > + ? ? ? ? ? #clock-cells = <0>;
>> > + ? ? ? ? ? clock-frequency = <12000000>;
>> > + ? };
>> > +};


-Olof

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Mitch Bradley]

On 1/20/2012 9:32 PM, Olof Johansson wrote:
> Hi,
>
> On Thu, Jan 19, 2012 at 9:17 AM, Stephen Warren<swarren@nvidia.com>  wrote:
>> Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
>>> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
>>>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>>>> +* NVIDIA Tegra20 Clock And Reset Controller
>>>> +
>>>> +This binding uses the common clock binding:
>>>> +Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>> +
>>>> +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
>>>> +for muxing and gating Tegra's clocks, and setting their rates.
>>>> +
>>>> +Required properties :
>>>> +- compatible : Should be "nvidia,<chip>-car"
>>>> +- reg : Should contain CAR registers location and length
>>>> +- clocks : Should contain phandle and clock specifiers for two clocks:
>>>> +  the 32 KHz "32k_in", and the board-specific oscillator "osc".
>>>> +- clock-names : Should contain a list of strings, with values "32k_in",
>>>> +  and "osc".
>>>
>>> Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
>>> where it isn't strictly necessary. Just because some vendors don't want
>>> to define an order between their clocks doesn't mean it's a good idea
>>> for everybody to use that model. In this case, just declaring that the
>>> two clocks refs have to be to those two clocks in that order should
>>> be sufficient.
>>
>> OK, that seems reasonable. I'm happy using of_clk_get() rather than
>> of_clk_get_by_name(). I guess that means we should just avoid any
>> discussion of clock-output-names too.
>
> Sounds good to me. Let's make sure Grant is OK with it too though.
>
>>>> +- #clock-cells : Should be 1.
>>>> +  In clock consumers, this cell represents the clock ID exposed by the CAR.
>>>> +  For a list of valid values, see the clock-output-names property.
>>>> +
>>>> +Optional properties :
>>>> +- clock-output-names : Should contain a list of strings defining the clocks
>>>> +  that the CAR provides. The list of names and clock IDs is below. The IDs
>>>> +  may be used in clock specifiers. The names should be listed in this clock-
>>>> +  output-names property, sorted in ID order, if this property is present.
>>>> +
>>>> +  The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
>>>> +  registers. Later, subsequent IDs will be assigned to all other clocks the
>>>> +  CAR controls.
>>>
>>> Aren't these names hardcoded per SoC? If so, they can be derived from the
>>> compatible field + output number without having a table in the device tree.
>>>
>>> If anything, you might want to enumerate the allowed/expected values, but
>>> actually putting them in a property seems overkill.
>>
>> Yes, the set of clocks is hard-coded per SoC, and the clock is uniquely
>> identified by compatible+id.
>>
>> clock-output-names doesn't actually serve any functional purpose in
>> Grant's common clock bindings patches; it's more of a documentation
>> thing. As such, yes, I can remove the suggestion to actually put the
>> table into the device tree, and rework the binding to simply define
>> what the mapping is independently.
>
> Again, sounds good to me.
>
>>>> +Example:
>>>> +
>>>> +clocks {
>>>> +   clk_32k: oscillator at 0 {
>>>
>>> If it has a unit addres (@<x>) it needs a reg property with the same base
>>> address.
>>
>> I thought everything needed a unit address period? Is the rule more like
>> the unit address is optional, but if present must match reg, so I can
>> simply remove @0 and @1 here? I guess that makes a lot more sense.
>
> Nope, you only need a unit address to make a node unique, i.e. if you
> have more than one with the same name. It's not something that's been
> followed very well on ARM dts files, I have even seen review comments
> asking for explicit unit IDs when none are needed.
>
> So you can't remove @0 and @1 here, since there are two oscillator subnodes.
>
> I'm not 100% sure if you have to have the unit address represented as
> "reg" or not, but it should probably be represented by _something_ in
> the properties of the node. Mitch? Segher? :)

unit-address is, by definition, the first address component of reg

>
>
>>>> +           compatible = "fixed-clock";
>>>> +           #clock-cells =<0>;
>>>> +           clock-frequency =<32768>;
>>>> +   };
>>>> +
>>>> +   osc: oscillator at 1 {
>>>> +           compatible = "fixed-clock";
>>>> +           #clock-cells =<0>;
>>>> +           clock-frequency =<12000000>;
>>>> +   };
>>>> +};
>
>
> -Olof
>
>

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Olof Johansson wrote at Saturday, January 21, 2012 12:32 AM:
> On Thu, Jan 19, 2012 at 9:17 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
> >> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
> >> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> >> > +* NVIDIA Tegra20 Clock And Reset Controller
> >> > +
> >> > +This binding uses the common clock binding:
> >> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> > +
> >> > +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> >> > +for muxing and gating Tegra's clocks, and setting their rates.
> >> > +
> >> > +Required properties :
> >> > +- compatible : Should be "nvidia,<chip>-car"
> >> > +- reg : Should contain CAR registers location and length
> >> > +- clocks : Should contain phandle and clock specifiers for two clocks:
> >> > + ?the 32 KHz "32k_in", and the board-specific oscillator "osc".
> >> > +- clock-names : Should contain a list of strings, with values "32k_in",
> >> > + ?and "osc".
...
> >> > +Example:
> >> > +
> >> > +clocks {
> >> > + ? clk_32k: oscillator at 0 {
> >>
> >> If it has a unit addres (@<x>) it needs a reg property with the same base
> >> address.
> >
> > I thought everything needed a unit address period? Is the rule more like
> > the unit address is optional, but if present must match reg, so I can
> > simply remove @0 and @1 here? I guess that makes a lot more sense.
> 
> Nope, you only need a unit address to make a node unique, i.e. if you
> have more than one with the same name. It's not something that's been
> followed very well on ARM dts files, I have even seen review comments
> asking for explicit unit IDs when none are needed.
> 
> So you can't remove @0 and @1 here, since there are two oscillator subnodes.

If I keep the unit address, then I need to add a reg property per Mitch's
response. But, then I need #address-cells and #size-cells in the clock
node too. Yuck, since this isn't an addressable bus.

Instead, is it acceptable to simply rename the nodes to e.g.:

clk_32k: clock {...};
osc: crystal {...};

In the long term, I believe that osc/crystal is going to continue to
be a top-level object, but clk_32k is actually an output from the PMU
chip, and hence there won't be any naming conflict here anyway...

-- 
nvpublic

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Mitch Bradley]

On 1/23/2012 6:18 AM, Stephen Warren wrote:
> Olof Johansson wrote at Saturday, January 21, 2012 12:32 AM:
>> On Thu, Jan 19, 2012 at 9:17 AM, Stephen Warren<swarren@nvidia.com>  wrote:
>>> Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
>>>> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
>>>>> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>>>>> +* NVIDIA Tegra20 Clock And Reset Controller
>>>>> +
>>>>> +This binding uses the common clock binding:
>>>>> +Documentation/devicetree/bindings/clock/clock-bindings.txt
>>>>> +
>>>>> +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
>>>>> +for muxing and gating Tegra's clocks, and setting their rates.
>>>>> +
>>>>> +Required properties :
>>>>> +- compatible : Should be "nvidia,<chip>-car"
>>>>> +- reg : Should contain CAR registers location and length
>>>>> +- clocks : Should contain phandle and clock specifiers for two clocks:
>>>>> +  the 32 KHz "32k_in", and the board-specific oscillator "osc".
>>>>> +- clock-names : Should contain a list of strings, with values "32k_in",
>>>>> +  and "osc".
> ...
>>>>> +Example:
>>>>> +
>>>>> +clocks {
>>>>> +   clk_32k: oscillator at 0 {
>>>>
>>>> If it has a unit addres (@<x>) it needs a reg property with the same base
>>>> address.
>>>
>>> I thought everything needed a unit address period? Is the rule more like
>>> the unit address is optional, but if present must match reg, so I can
>>> simply remove @0 and @1 here? I guess that makes a lot more sense.
>>
>> Nope, you only need a unit address to make a node unique, i.e. if you
>> have more than one with the same name. It's not something that's been
>> followed very well on ARM dts files, I have even seen review comments
>> asking for explicit unit IDs when none are needed.
>>
>> So you can't remove @0 and @1 here, since there are two oscillator subnodes.
>
> If I keep the unit address, then I need to add a reg property per Mitch's
> response. But, then I need #address-cells and #size-cells in the clock
> node too. Yuck, since this isn't an addressable bus.
>
> Instead, is it acceptable to simply rename the nodes to e.g.:
>
> clk_32k: clock {...};
> osc: crystal {...};


One consideration:  These nodes are children of a parent, so that parent 
is implicitly a "bus node", even though there is no physical hardware 
bus.  The path resolution rules say that, in the absence of a 
"#address-cells" property in a bus node, the default value is 2.  So any 
code that implements that rule will think these nodes are missing a 
"reg" property, thus triggering the "wildcard node" rule, which says 
that you can match the node with any unit address.

Whether or not that would cause problems in practice is hard to say.  I 
think that my Open Firmware implementation would handle it okay, but I 
can't speak to the Linux device tree code.

I'm not sure why you thing "yuck" about synthesizing an address space 
for the subnodes.  It doesn't seem that bad to me.

>
> In the long term, I believe that osc/crystal is going to continue to
> be a top-level object, but clk_32k is actually an output from the PMU
> chip, and hence there won't be any naming conflict here anyway...
>

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Grant Likely]

On Fri, Jan 20, 2012 at 11:32:04PM -0800, Olof Johansson wrote:
> Hi,
> 
> On Thu, Jan 19, 2012 at 9:17 AM, Stephen Warren <swarren@nvidia.com> wrote:
> > Olof Johansson wrote at Wednesday, January 18, 2012 10:32 PM:
> >> On Wed, Jan 18, 2012 at 05:16:52PM -0700, Stephen Warren wrote:
> >> > diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> >> > +* NVIDIA Tegra20 Clock And Reset Controller
> >> > +
> >> > +This binding uses the common clock binding:
> >> > +Documentation/devicetree/bindings/clock/clock-bindings.txt
> >> > +
> >> > +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> >> > +for muxing and gating Tegra's clocks, and setting their rates.
> >> > +
> >> > +Required properties :
> >> > +- compatible : Should be "nvidia,<chip>-car"
> >> > +- reg : Should contain CAR registers location and length
> >> > +- clocks : Should contain phandle and clock specifiers for two clocks:
> >> > + ?the 32 KHz "32k_in", and the board-specific oscillator "osc".
> >> > +- clock-names : Should contain a list of strings, with values "32k_in",
> >> > + ?and "osc".
> >>
> >> Hmm. I'd prefer to just ditch the notion of "clock-names" in the cases
> >> where it isn't strictly necessary. Just because some vendors don't want
> >> to define an order between their clocks doesn't mean it's a good idea
> >> for everybody to use that model. In this case, just declaring that the
> >> two clocks refs have to be to those two clocks in that order should
> >> be sufficient.
> >
> > OK, that seems reasonable. I'm happy using of_clk_get() rather than
> > of_clk_get_by_name(). I guess that means we should just avoid any
> > discussion of clock-output-names too.
> 
> Sounds good to me. Let's make sure Grant is OK with it too though.

Yes, I agree.

g.

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 4:16 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Document a binding for the Tegra20 CAR (Clock And Reset) Controller,
> add it to tegra20.dtsi, and configure it for the board in tegra-
> seaboard.dts.
>
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> ---
> All, Simon Glass is attempting to upstream Tegra USB support in U-Boot.
> The driver is only instantiated from device tree, and needs to configure
> clocks for the USB module. Hence, he proposed a clock binding for Tegra.
> I've read through Grant's proposed common clock bindings and reworked
> Simon's proposal a little, ending up with the patch below for the kernel.
> I'd appreciate any comments you have.
>
> Grant, can you confirm that it's reasonable to start making use of your
> proposed common clock bindings in U-Boot, even though they're only an RFC?
>
> Grant, there are a couple of FIXMEs in the binding documentation below.
> Can you give any insight on these?
>
> A comment on the shared enable issue:
>
> When enabling/disabling clocks, Tegra requires you to reset the HW module
> that the clock is routed to. Both reset and clock enable registers are
> part of the CAR. U-Boot currently has an API to do the reset based on a
> "peripheral ID", which simply means a bit number in a set of 3 "reset"
> registers. The bits in those registers share the same numbering as the
> "clock enable" registers. Hence, to make life easy for U-Boot, I've
> defined the clock IDs in this binding to be equal to these bit numbers.
> However, this breaks down where there isn't a 1:1 mapping between reset
> and clock enable bits, and clocks. For example, there's a single reset
> and clock enable bit for the SPDIF controller, yet this applies to two
> clocks; spdif_in and spdif_out. Hence, this simplification for U-Boot
> breaks down. I think the correct solution is to fix U-Boot not to
> require this simplification, renumber the clocks in the CAR binding in
> a completely arbitrary fashion, and require U-Boot to contain a mapping
> table between CAR's DT clock IDs and any other information related to
> those clocks. Do you see any alternative? We really need the two SPDIF
> clocks to be different clock IDs in the binding, since each has a
> completely independant mux for the clock source/parent, and the two
> clocks obviously can't share the same clock ID (well, hmm, perhaps they
> could if we used 2 cells for the specifier, 1 for the bit number, and
> one more to differentiate clocks that use the same bit...). I'm still
> inclined to simply push back on U-Boot to do it right.

Are SPDIF and VI the only examples of this? If so, perhaps just having
a special extra clock ID for those two and mapping in a special way
would be more efficient. So two IDs would map to one clock/reset bit
but different clocks.

Also I note that one is an input and one is an output clock. So we
could name them this way, and use the separate ID for the input clocks
perhaps.

If you do add an arbitrary mapping, what would it be? It might as well
follow along with the registers so far as it can, since the device
tree should describe the hardware. Then the mapping becomes a few
lines of code in the driver instead of YAT (yet another table).

Poor U-Boot doesn't even use the SPDIF or vi controllers.

>
> ?.../bindings/clock/nvidia,tegra20-car.txt ? ? ? ? ?| ?156 ++++++++++++++++++++
> ?arch/arm/boot/dts/tegra-seaboard.dts ? ? ? ? ? ? ? | ? 18 +++
> ?arch/arm/boot/dts/tegra20.dtsi ? ? ? ? ? ? ? ? ? ? | ? ?7 +
> ?3 files changed, 181 insertions(+), 0 deletions(-)
> ?create mode 100644 Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
>
> diff --git a/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> new file mode 100644
> index 0000000..71cfdd2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/nvidia,tegra20-car.txt
> @@ -0,0 +1,156 @@
> +* NVIDIA Tegra20 Clock And Reset Controller
> +
> +This binding uses the common clock binding:
> +Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +The CAR (Clock And Reset) Controller on Tegra is the HW module responsible
> +for muxing and gating Tegra's clocks, and setting their rates.
> +
> +Required properties :
> +- compatible : Should be "nvidia,<chip>-car"
> +- reg : Should contain CAR registers location and length
> +- clocks : Should contain phandle and clock specifiers for two clocks:
> + ?the 32 KHz "32k_in", and the board-specific oscillator "osc".
> +- clock-names : Should contain a list of strings, with values "32k_in",
> + ?and "osc".
> +- #clock-cells : Should be 1.
> + ?In clock consumers, this cell represents the clock ID exposed by the CAR.
> + ?For a list of valid values, see the clock-output-names property.
> +
> +Optional properties :
> +- clock-output-names : Should contain a list of strings defining the clocks
> + ?that the CAR provides. The list of names and clock IDs is below. The IDs
> + ?may be used in clock specifiers. The names should be listed in this clock-
> + ?output-names property, sorted in ID order, if this property is present.
> +
> + ?The first 96 clocks are numbered to match the bits in the CAR's CLK_OUT_ENB
> + ?registers. Later, subsequent IDs will be assigned to all other clocks the
> + ?CAR controls.

I hope that at some point these would have symbolic names so that we
don't need to use open-coded integers in the device tree cells.

> +
> + ? ?0 ? "cpu"
> + ? ?1 ? "unassigned"
> + ? ?2 ? "unassigned" ? ?FIXME: Is it OK to have 2 identical (unused) names?

Or in fact just empty so save space?

> + ? ?3 ? "ac97"
> + ? ?4 ? "rtc"
> + ? ?5 ? "tmr"
> + ? ?6 ? "uart1"
> + ? ?7 ? "uart2"
> + ? ?8 ? "gpio"
> + ? ?9 ? "sdmmc2"
> + ? ?10 ?"spdif" ? ? ? ? FIXME: One enable bit controls two clocks!!!
> + ? ?11 ?"i2s1"
> + ? ?12 ?"i2c1"
> + ? ?13 ?"ndflash"
> + ? ?14 ?"sdmmc1"
> + ? ?15 ?"sdmmc4"
> + ? ?16 ?"twc"
> + ? ?17 ?"pwm"
> + ? ?18 ?"i2s2"
> + ? ?19 ?"epp"
> + ? ?20 ?"vi" ? ? ? ? ? ?FIXME: One enable bit controls two clocks!!!
> + ? ?21 ?"2d"
> + ? ?22 ?"usbd"
> + ? ?23 ?"isp"
> + ? ?24 ?"3d"
> + ? ?25 ?"ide"
> + ? ?26 ?"disp2"
> + ? ?27 ?"disp1"
> + ? ?28 ?"host1x"
> + ? ?29 ?"vcp"
> + ? ?30 ?"unassigned"
> + ? ?31 ?"cache2"
> +
> + ? ?32 ?"mem"
> + ? ?33 ?"ahbdma"
> + ? ?34 ?"apbdma"
> + ? ?35 ?"unassigned"
> + ? ?36 ?"kbc"
> + ? ?37 ?"stat_mon"
> + ? ?38 ?"pmc"
> + ? ?39 ?"fuse"
> + ? ?40 ?"kfuse"
> + ? ?41 ?"sbc1"
> + ? ?42 ?"snor"
> + ? ?43 ?"spi"
> + ? ?44 ?"sbc2"
> + ? ?45 ?"xio"
> + ? ?46 ?"sbc3"
> + ? ?47 ?"dvc_i2c"
> + ? ?48 ?"dsi"
> + ? ?49 ?"tvo"
> + ? ?50 ?"mipi"
> + ? ?51 ?"hdmi"
> + ? ?52 ?"csi"
> + ? ?53 ?"tvdac"
> + ? ?54 ?"i2c2"
> + ? ?55 ?"uart3"
> + ? ?56 ?"unassigned"
> + ? ?57 ?"emc"
> + ? ?58 ?"usb2"
> + ? ?59 ?"usb3"
> + ? ?60 ?"mpe"
> + ? ?61 ?"vde"
> + ? ?62 ?"bsea"
> + ? ?63 ?"bsev"
> +
> + ? ?64 ?"speedo"
> + ? ?65 ?"uart4"
> + ? ?66 ?"uart5"
> + ? ?67 ?"i2c3"
> + ? ?68 ?"sbc4"
> + ? ?69 ?"sdmmc3"
> + ? ?70 ?"pcie"
> + ? ?71 ?"owr"
> + ? ?72 ?"afi"
> + ? ?73 ?"csite"
> + ? ?74 ?"unassigned"
> + ? ?75 ?"avpucq"
> + ? ?76 ?"unassigned"
> + ? ?77 ?"unassigned"
> + ? ?78 ?"unassigned"
> + ? ?79 ?"unassigned"
> + ? ?80 ?"unassigned"
> + ? ?81 ?"unassigned"
> + ? ?82 ?"unassigned"
> + ? ?83 ?"unassigned"
> + ? ?84 ?"irama"
> + ? ?85 ?"iramb"
> + ? ?86 ?"iramc"
> + ? ?87 ?"iramd"
> + ? ?88 ?"cram2"
> + ? ?89 ?"audio_2x"
> + ? ?90 ?"clk_d"
> + ? ?91 ?"unassigned"
> + ? ?92 ?"sus"
> + ? ?93 ?"cdev1"
> + ? ?94 ?"cdev2"
> + ? ?95 ?"unassigned"
> +
> +Example:
> +
> +clocks {
> + ? ? ? clk_32k: oscillator at 0 {
> + ? ? ? ? ? ? ? compatible = "fixed-clock";
> + ? ? ? ? ? ? ? #clock-cells = <0>;
> + ? ? ? ? ? ? ? clock-frequency = <32768>;
> + ? ? ? };
> +
> + ? ? ? osc: oscillator at 1 {
> + ? ? ? ? ? ? ? compatible = "fixed-clock";
> + ? ? ? ? ? ? ? #clock-cells = <0>;
> + ? ? ? ? ? ? ? clock-frequency = <12000000>;
> + ? ? ? };
> +};
> +
> +tegar_car: clock at 60006000 {
> + ? ? ? compatible = "tegra,periphclk";
> + ? ? ? reg = <0x60006000 1000>;

0x1000 ?

> + ? ? ? clocks = <&clk_32k> <&osc>;
> + ? ? ? clock-names = "32k_in", "osc";
> + ? ? ? #clock-cells = <1>;
> +};
> +
> +usb at c5004000 {
> + ? ? ? ...
> + ? ? ? clocks = <&tegra_car 58>; /* usb2 */
> +};
> diff --git a/arch/arm/boot/dts/tegra-seaboard.dts b/arch/arm/boot/dts/tegra-seaboard.dts
> index b55a02e..f9347fe 100644
> --- a/arch/arm/boot/dts/tegra-seaboard.dts
> +++ b/arch/arm/boot/dts/tegra-seaboard.dts
> @@ -11,6 +11,24 @@
> ? ? ? ? ? ? ? ?reg = < 0x00000000 0x40000000 >;
> ? ? ? ?};
>
> + ? ? ? clocks {
> + ? ? ? ? ? ? ? clk_32k: oscillator at 0 {
> + ? ? ? ? ? ? ? ? ? ? ? compatible = "fixed-clock";
> + ? ? ? ? ? ? ? ? ? ? ? #clock-cells = <0>;
> + ? ? ? ? ? ? ? ? ? ? ? clock-frequency = <32768>;
> + ? ? ? ? ? ? ? };
> +
> + ? ? ? ? ? ? ? osc: oscillator at 1 {
> + ? ? ? ? ? ? ? ? ? ? ? compatible = "fixed-clock";
> + ? ? ? ? ? ? ? ? ? ? ? #clock-cells = <0>;
> + ? ? ? ? ? ? ? ? ? ? ? clock-frequency = <12000000>;
> + ? ? ? ? ? ? ? };

Is there a reason why these two nodes are in the tegra20.dtsi include
file? Is it because some boards don't have one of these clocks? Or
because the frequency is not know? If the former, perhaps used status
= "disabled" and if the latter perhaps define only the frequency in
this file? Just trying to reduce boilerplate and complexity for board
bring-up engineers.

Also the oscillator frequency seems to be detected at run-time. What
happens if it doesn't match the fdt? Should be not detect it at
run-time?

> + ? ? ? };
> +
> + ? ? ? clock at 60006000 {
> + ? ? ? ? ? ? ? clocks = <&clk_32k> <&osc>;

Why is the clocks property here, but the clock-names property in the
include file?

> + ? ? ? };
> +
> ? ? ? ?i2c at 7000c000 {
> ? ? ? ? ? ? ? ?clock-frequency = <400000>;
> ? ? ? ?};
> diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
> index 3da7afd..be4abd2 100644
> --- a/arch/arm/boot/dts/tegra20.dtsi
> +++ b/arch/arm/boot/dts/tegra20.dtsi
> @@ -4,6 +4,13 @@
> ? ? ? ?compatible = "nvidia,tegra20";
> ? ? ? ?interrupt-parent = <&intc>;
>
> + ? ? ? tegar_car: clock at 60006000 {

Is car a standard name? If not tegra_clk_rst would be an alternative.

> + ? ? ? ? ? ? ? compatible = "tegra,periphclk";
> + ? ? ? ? ? ? ? reg = <0x60006000 1000>;

0x1000 ?

> + ? ? ? ? ? ? ? clock-names = "32k_in", "osc";
> + ? ? ? ? ? ? ? #clock-cells = <1>;
> + ? ? ? };
> +
> ? ? ? ?intc: interrupt-controller at 50041000 {
> ? ? ? ? ? ? ? ?compatible = "arm,cortex-a9-gic";
> ? ? ? ? ? ? ? ?interrupt-controller;
> --
> 1.7.0.4
>

Regards,
Simon

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Simon Glass wrote at Sunday, January 22, 2012 11:03 AM:
> On Wed, Jan 18, 2012 at 4:16 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Document a binding for the Tegra20 CAR (Clock And Reset) Controller,
> > add it to tegra20.dtsi, and configure it for the board in tegra-
> > seaboard.dts.
...
> > A comment on the shared enable issue:
> >
> > When enabling/disabling clocks, Tegra requires you to reset the HW module
> > that the clock is routed to. Both reset and clock enable registers are
> > part of the CAR. U-Boot currently has an API to do the reset based on a
> > "peripheral ID", which simply means a bit number in a set of 3 "reset"
> > registers. The bits in those registers share the same numbering as the
> > "clock enable" registers. Hence, to make life easy for U-Boot, I've
> > defined the clock IDs in this binding to be equal to these bit numbers.
> > However, this breaks down where there isn't a 1:1 mapping between reset
> > and clock enable bits, and clocks. For example, there's a single reset
> > and clock enable bit for the SPDIF controller, yet this applies to two
> > clocks; spdif_in and spdif_out. Hence, this simplification for U-Boot
> > breaks down. I think the correct solution is to fix U-Boot not to
> > require this simplification, renumber the clocks in the CAR binding in
> > a completely arbitrary fashion, and require U-Boot to contain a mapping
> > table between CAR's DT clock IDs and any other information related to
> > those clocks. Do you see any alternative? We really need the two SPDIF
> > clocks to be different clock IDs in the binding, since each has a
> > completely independant mux for the clock source/parent, and the two
> > clocks obviously can't share the same clock ID (well, hmm, perhaps they
> > could if we used 2 cells for the specifier, 1 for the bit number, and
> > one more to differentiate clocks that use the same bit...). I'm still
> > inclined to simply push back on U-Boot to do it right.
> 
> Are SPDIF and VI the only examples of this?

I think so. I should double-check to be sure though before committing
to a final binding.

> If so, perhaps just having
> a special extra clock ID for those two and mapping in a special way
> would be more efficient. So two IDs would map to one clock/reset bit
> but different clocks.

I thought about that initially, but it doesn't make semantic sense;
there isn't a 1:1 mapping between clock ID and reset bit, so I don't
think we should pretend there is for some clocks, and special-case
others.

> Also I note that one is an input and one is an output clock. So we
> could name them this way, and use the separate ID for the input clocks
> perhaps.

I don't think that solves the problem; HW modules and drivers use both
clocks, so special-casing the input clocks doesn't make the problem
less relevant.

> If you do add an arbitrary mapping, what would it be? It might as well
> follow along with the registers so far as it can, since the device
> tree should describe the hardware. Then the mapping becomes a few
> lines of code in the driver instead of YAT (yet another table).

The mapping would be that the clock IDs are unrelated to the bit numbers
in the CAR's reset/clock-enable registers.

In practice, this means that to find the reset bit number, you need a
table indexed by the .dts's clock number, with the bit number being
retrieved from that table.

So instead of bit = of_get_u32(1), we'd have bit = table[of_get_u32(1)].

In practice, I believe we'll need such a table anyway, since each clock
has many more parameters than just the reset bit ID; some clocks have
no reset bit at all, some clocks have a mux but some don't, the inputs
to the mux are different for each clock, some have a divider but some
don't, where there's a divider or mux, you need to know the register
address to configure them, each clock has a different max rate, etc.

Having thought a little more about this, I'm definitely leaning towards
this way; making the clock ID and register bit completely unrelated just
to make it really obvious that you can't use the clock ID as a register
bit.

-- 
nvpublic

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Peter De Schrijver]

What about the peripheral resets which are also handled by CAR? Peripheral
clock nodes also offer assert and deassert methods for the reset signal
associated with them. Those methods are used when powergating domains for
example. Should we model this in the same binding?

Thanks,

Peter.

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Peter De Schrijver wrote at Tuesday, January 24, 2012 2:53 AM:
> What about the peripheral resets which are also handled by CAR? Peripheral
> clock nodes also offer assert and deassert methods for the reset signal
> associated with them. Those methods are used when powergating domains for
> example. Should we model this in the same binding?

In most cases, I think the resets are handled purely within clock enable
and disable, so there's no need to explicitly manage them or represent
them in the device tree. Do you agree here?

I recall that you mentioned some power-management code might need to
manage reset separately though. Can you explain why a module would be
reset separately from disabling the clocks though?

If we do need this, I think we can just add a property to the node of
each device that needs such explicit management. Something like:

tegra_car: clock at 60006000 {
    compatible = "nvidia,tegra20-car";
    ....
};

foo at 70007000 {
    compatible = "nvidia,tegra20-foo";
    regs = <...>;
    nvidia,car-reset-id = <&tegra_car 30>;
};

And the driver for "foo" can ignore cell 0, and extract the cell 1 and
pass the value to tegra_periph_reset_assert().

I'd expect to put the CAR phandle into the property in case we ever get
a more generalized reset subsystem, and need more general code to
interpret the property. I did the same for the Tegra APB DMA controller
client binding where clients need to know the "DMA select" value.

Does that all seem reasonable?

-- 
nvpublic

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Colin Cross]

On Tue, Jan 24, 2012 at 2:08 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Peter De Schrijver wrote at Tuesday, January 24, 2012 2:53 AM:
>> What about the peripheral resets which are also handled by CAR? Peripheral
>> clock nodes also offer assert and deassert methods for the reset signal
>> associated with them. Those methods are used when powergating domains for
>> example. Should we model this in the same binding?
>
> In most cases, I think the resets are handled purely within clock enable
> and disable, so there's no need to explicitly manage them or represent
> them in the device tree. Do you agree here?

clk_enable could force a deasserted reset, but clk_disable cannot
imply asserting reset.  The clocks need to be turned off for power
management without resetting the registers.

> I recall that you mentioned some power-management code might need to
> manage reset separately though. Can you explain why a module would be
> reset separately from disabling the clocks though?

The TRM lists a very specific sequence of clocks, resets, clamps, and
power for power domain gating.  Other than that, I think the only use
for directly calling reset that ended up in the Android Tegra2 tree
was for error recovery on HW blocks that could get locked up, probably
I2C.

> If we do need this, I think we can just add a property to the node of
> each device that needs such explicit management. Something like:
>
> tegra_car: clock at 60006000 {
> ? ?compatible = "nvidia,tegra20-car";
> ? ?....
> };
>
> foo at 70007000 {
> ? ?compatible = "nvidia,tegra20-foo";
> ? ?regs = <...>;
> ? ?nvidia,car-reset-id = <&tegra_car 30>;
> };
>
> And the driver for "foo" can ignore cell 0, and extract the cell 1 and
> pass the value to tegra_periph_reset_assert().
>
> I'd expect to put the CAR phandle into the property in case we ever get
> a more generalized reset subsystem, and need more general code to
> interpret the property. I did the same for the Tegra APB DMA controller
> client binding where clients need to know the "DMA select" value.
>
> Does that all seem reasonable?
>
> --
> nvpublic
>

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Stephen Warren]

Colin Cross wrote at Tuesday, January 24, 2012 3:33 PM:
> On Tue, Jan 24, 2012 at 2:08 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Peter De Schrijver wrote at Tuesday, January 24, 2012 2:53 AM:
> >> What about the peripheral resets which are also handled by CAR? Peripheral
> >> clock nodes also offer assert and deassert methods for the reset signal
> >> associated with them. Those methods are used when powergating domains for
> >> example. Should we model this in the same binding?
> >
> > In most cases, I think the resets are handled purely within clock enable
> > and disable, so there's no need to explicitly manage them or represent
> > them in the device tree. Do you agree here?
> 
> clk_enable could force a deasserted reset, but clk_disable cannot
> imply asserting reset.  The clocks need to be turned off for power
> management without resetting the registers.

Yes, I just spoke to someone off-list, and he said the same thing. He
went on to say that therefore even the reset removal with clk_enable
was questionable, and that drivers should explicitly manage reset
removal themselves. Does that seem a reasonable stance? It'd certainly
take away the somewhat asymmetric nature of reset removal just magically
working when you first enable the clocks. We'd presumably then want a
common reset infra-structure and binding to match that change?

(I know that'd make Simon happy, since then we'd be able to represent
the reset IDs directly everywhere, unrelated to the clock IDs, and
hence he could just use the reset IDs directly in the U-Boot code he's
writing and ignore the non 1:1 mapping between clock and reset IDs)

> > I recall that you mentioned some power-management code might need to
> > manage reset separately though. Can you explain why a module would be
> > reset separately from disabling the clocks though?
> 
> The TRM lists a very specific sequence of clocks, resets, clamps, and
> power for power domain gating.  Other than that, I think the only use
> for directly calling reset that ended up in the Android Tegra2 tree
> was for error recovery on HW blocks that could get locked up, probably
> I2C.

OK, those reasons make sense.

-- 
nvpublic

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Colin Cross]

On Tue, Jan 24, 2012 at 2:43 PM, Stephen Warren <swarren@nvidia.com> wrote:
> Colin Cross wrote at Tuesday, January 24, 2012 3:33 PM:
>> On Tue, Jan 24, 2012 at 2:08 PM, Stephen Warren <swarren@nvidia.com> wrote:
>> > Peter De Schrijver wrote at Tuesday, January 24, 2012 2:53 AM:
>> >> What about the peripheral resets which are also handled by CAR? Peripheral
>> >> clock nodes also offer assert and deassert methods for the reset signal
>> >> associated with them. Those methods are used when powergating domains for
>> >> example. Should we model this in the same binding?
>> >
>> > In most cases, I think the resets are handled purely within clock enable
>> > and disable, so there's no need to explicitly manage them or represent
>> > them in the device tree. Do you agree here?
>>
>> clk_enable could force a deasserted reset, but clk_disable cannot
>> imply asserting reset. ?The clocks need to be turned off for power
>> management without resetting the registers.
>
> Yes, I just spoke to someone off-list, and he said the same thing. He
> went on to say that therefore even the reset removal with clk_enable
> was questionable, and that drivers should explicitly manage reset
> removal themselves. Does that seem a reasonable stance? It'd certainly
> take away the somewhat asymmetric nature of reset removal just magically
> working when you first enable the clocks. We'd presumably then want a
> common reset infra-structure and binding to match that change?

In 99% of drivers reset is irrelevant, as long as the block is out of
reset by the time the driver starts writing to registers.  clk_enable
is a decent place to ensure that - it always has to be done before
writing to the registers, and it maps nicely to the reset bits on
Tegra.  It would be nice if those driver didn't need to deassert reset
explicitly, especially if that requires a Tegra-specific API.

> (I know that'd make Simon happy, since then we'd be able to represent
> the reset IDs directly everywhere, unrelated to the clock IDs, and
> hence he could just use the reset IDs directly in the U-Boot code he's
> writing and ignore the non 1:1 mapping between clock and reset IDs)
>
>> > I recall that you mentioned some power-management code might need to
>> > manage reset separately though. Can you explain why a module would be
>> > reset separately from disabling the clocks though?
>>
>> The TRM lists a very specific sequence of clocks, resets, clamps, and
>> power for power domain gating. ?Other than that, I think the only use
>> for directly calling reset that ended up in the Android Tegra2 tree
>> was for error recovery on HW blocks that could get locked up, probably
>> I2C.
>
> OK, those reasons make sense.
>
> --
> nvpublic
>

[U-Boot] [PATCH] ARM: tegra: Define Tegra20 CAR binding

[Peter De Schrijver]

On Tue, Jan 24, 2012 at 11:59:40PM +0100, Colin Cross wrote:
> On Tue, Jan 24, 2012 at 2:43 PM, Stephen Warren <swarren@nvidia.com> wrote:
> > Colin Cross wrote at Tuesday, January 24, 2012 3:33 PM:
> >> On Tue, Jan 24, 2012 at 2:08 PM, Stephen Warren <swarren@nvidia.com> wrote:
> >> > Peter De Schrijver wrote at Tuesday, January 24, 2012 2:53 AM:
> >> >> What about the peripheral resets which are also handled by CAR? Peripheral
> >> >> clock nodes also offer assert and deassert methods for the reset signal
> >> >> associated with them. Those methods are used when powergating domains for
> >> >> example. Should we model this in the same binding?
> >> >
> >> > In most cases, I think the resets are handled purely within clock enable
> >> > and disable, so there's no need to explicitly manage them or represent
> >> > them in the device tree. Do you agree here?
> >>
> >> clk_enable could force a deasserted reset, but clk_disable cannot
> >> imply asserting reset. ?The clocks need to be turned off for power
> >> management without resetting the registers.
> >
> > Yes, I just spoke to someone off-list, and he said the same thing. He
> > went on to say that therefore even the reset removal with clk_enable
> > was questionable, and that drivers should explicitly manage reset
> > removal themselves. Does that seem a reasonable stance? It'd certainly
> > take away the somewhat asymmetric nature of reset removal just magically
> > working when you first enable the clocks. We'd presumably then want a
> > common reset infra-structure and binding to match that change?
> 
> In 99% of drivers reset is irrelevant, as long as the block is out of
> reset by the time the driver starts writing to registers.  clk_enable
> is a decent place to ensure that - it always has to be done before
> writing to the registers, and it maps nicely to the reset bits on
> Tegra.  It would be nice if those driver didn't need to deassert reset
> explicitly, especially if that requires a Tegra-specific API.

I think this could be solved by moving the drivers to runtime PM and handle the
clock and reset bits in tegra platform device specific code. iirc that's the
way it's handled in OMAP?

Cheers,

Peter. 

[U-Boot] [PATCH v4 12/20] tegra: usb: fdt: Add additional device tree definitions for USB ports

[Simon Glass]

This adds clock references to the USB part of the device tree for U-Boot.

The USB timing information may vary between boards sometimes, but for
now we hard-code it in C. This is because all current T2x boards use
the same values, we will deal with T3x later and we first need to agree
on the format for this timing information in the fdt and may in fact
decide that it has no place there.

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

 arch/arm/dts/tegra20.dtsi |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
index 68eae12..ca7b523 100644
--- a/arch/arm/dts/tegra20.dtsi
+++ b/arch/arm/dts/tegra20.dtsi
@@ -176,6 +176,7 @@
 		reg = <0xc5000000 0x4000>;
 		interrupts = < 52 >;
 		phy_type = "utmi";
+		clocks = <&periph_clk 22>;	// PERIPH_ID_USBD
 	};
 
 	usb at c5004000 {
@@ -183,6 +184,7 @@
 		reg = <0xc5004000 0x4000>;
 		interrupts = < 53 >;
 		phy_type = "ulpi";
+		clocks = <&periph_clk 58>;	// PERIPH_ID_USB2
 	};
 
 	usb at c5008000 {
@@ -190,6 +192,7 @@
 		reg = <0xc5008000 0x4000>;
 		interrupts = < 129 >;
 		phy_type = "utmi";
+		clocks = <&periph_clk 59>;	// PERIPH_ID_USB3
 	};
 
 };
-- 
1.7.7.3

[U-Boot] [PATCH v4 12/20] tegra: usb: fdt: Add additional device tree definitions for USB ports

[Stephen Warren]

On 01/11/2012 09:33 PM, Simon Glass wrote:
> This adds clock references to the USB part of the device tree for U-Boot.
> 
> The USB timing information may vary between boards sometimes, but for
> now we hard-code it in C. This is because all current T2x boards use
> the same values, we will deal with T3x later and we first need to agree
> on the format for this timing information in the fdt and may in fact
> decide that it has no place there.

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

> +		clocks = <&periph_clk 22>;	// PERIPH_ID_USBD

Typically /* */ rather than // I think

-- 
nvpublic

[U-Boot] [PATCH v4 12/20] tegra: usb: fdt: Add additional device tree definitions for USB ports

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 4:19 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:33 PM, Simon Glass wrote:
>> This adds clock references to the USB part of the device tree for U-Boot.
>>
>> The USB timing information may vary between boards sometimes, but for
>> now we hard-code it in C. This is because all current T2x boards use
>> the same values, we will deal with T3x later and we first need to agree
>> on the format for this timing information in the fdt and may in fact
>> decide that it has no place there.
>
>> diff --git a/arch/arm/dts/tegra20.dtsi b/arch/arm/dts/tegra20.dtsi
>
>> + ? ? ? ? ? ? clocks = <&periph_clk 22>; ? ? ?// PERIPH_ID_USBD
>
> Typically /* */ rather than // I think

Yes will fix. Looking forward to your symbolic stuff so that I can
remove the comment.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 13/20] tegra: usb: fdt: Add USB definitions for Tegra2 Seaboard

[Simon Glass]

We set up two USB ports, one of which can be host or device.
For some reason the kernel version does enable both ports.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Use "okay" instead of "ok" for fdt node status
- Remove 0x from fdt aliases

Changes in v3:
- Remove "okay" from nodes since this is the default anyway
- Fix device tree indenting with tabs instead of spaces
- Disable USB2 which is not used on Seaboard

 board/nvidia/dts/tegra2-seaboard.dts |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/dts/tegra2-seaboard.dts b/board/nvidia/dts/tegra2-seaboard.dts
index dde5d03..7a4ecae 100644
--- a/board/nvidia/dts/tegra2-seaboard.dts
+++ b/board/nvidia/dts/tegra2-seaboard.dts
@@ -11,6 +11,12 @@
 		bootargs = "vmalloc=192M video=tegrafb console=ttyS0,115200n8 root=/dev/mmcblk1p3 rw rootwait";
 	};
 
+	aliases {
+		/* This defines the order of our USB ports */
+		usb0 = "/usb at c5008000";
+		usb1 = "/usb at c5000000";
+	};
+
 	memory {
 		device_type = "memory";
 		reg = < 0x00000000 0x40000000 >;
@@ -32,5 +38,10 @@
 
 	usb at c5000000 {
 		nvidia,vbus-gpio = <&gpio 24 0>; /* PD0 */
+		support-host-mode;
+	};
+
+	usb at c5004000 {
+		status = "disabled";
 	};
 };
-- 
1.7.7.3

[U-Boot] [PATCH v4 14/20] usb: Add support for txfifo threshold

[Simon Glass]

CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the txfilltuning
field in the EHCI controller on reset.

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

 README                      |    3 +++
 drivers/usb/host/ehci-hcd.c |    7 +++++++
 drivers/usb/host/ehci.h     |    6 +++++-
 3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/README b/README
index 9d713e8..7adf7c7 100644
--- a/README
+++ b/README
@@ -1135,6 +1135,9 @@ The following options need to be configured:
 				May be defined to allow interrupt polling
 				instead of using asynchronous interrupts
 
+		CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the
+		txfilltuning field in the EHCI controller on reset.
+
 - USB Device:
 		Define the below if you wish to use the USB console.
 		Once firmware is rebuilt from a serial console issue the
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 3f7bc2c..d893b2a 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -259,6 +259,13 @@ static int ehci_reset(void)
 #endif
 		ehci_writel(reg_ptr, tmp);
 	}
+
+#ifdef CONFIG_USB_EHCI_TXFIFO_THRESH
+	cmd = ehci_readl(&hcor->or_txfilltuning);
+	cmd &= ~TXFIFO_THRESH(0x3f);
+	cmd |= TXFIFO_THRESH(CONFIG_USB_EHCI_TXFIFO_THRESH);
+	ehci_writel(&hcor->or_txfilltuning, cmd);
+#endif
 out:
 	return ret;
 }
diff --git a/drivers/usb/host/ehci.h b/drivers/usb/host/ehci.h
index 3d0ad0c..cc00ce4 100644
--- a/drivers/usb/host/ehci.h
+++ b/drivers/usb/host/ehci.h
@@ -80,7 +80,11 @@ struct ehci_hcor {
 	uint32_t or_ctrldssegment;
 	uint32_t or_periodiclistbase;
 	uint32_t or_asynclistaddr;
-	uint32_t _reserved_[9];
+	uint32_t _reserved_0_;
+	uint32_t or_burstsize;
+	uint32_t or_txfilltuning;
+#define TXFIFO_THRESH(p)		((p & 0x3f) << 16)
+	uint32_t _reserved_1_[6];
 	uint32_t or_configflag;
 #define FLAG_CF		(1 << 0)	/* true:  we'll support "high speed" */
 	uint32_t or_portsc[CONFIG_SYS_USB_EHCI_MAX_ROOT_PORTS];
-- 
1.7.7.3

[U-Boot] [PATCH v4 14/20] usb: Add support for txfifo threshold

[Remy Bohmer]

Hi,

2012/1/12 Simon Glass <sjg@chromium.org>:
> CONFIG_USB_EHCI_TXFIFO_THRESH enables setting of the txfilltuning
> field in the EHCI controller on reset.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
> ?README ? ? ? ? ? ? ? ? ? ? ?| ? ?3 +++
> ?drivers/usb/host/ehci-hcd.c | ? ?7 +++++++
> ?drivers/usb/host/ehci.h ? ? | ? ?6 +++++-
> ?3 files changed, 15 insertions(+), 1 deletions(-)

Acked-by: Remy Bohmer <linux@bohmer.net>

[U-Boot] [PATCH v4 15/20] fdt: Add function to return peripheral/clock ID

[Simon Glass]

A common requirement is to find the clock ID for a peripheral. This is the
second cell of the 'clocks' property (the first being the phandle itself).

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v4:
- Add fdtdec function to return peripheral ID

 include/fdtdec.h |   13 +++++++++++++
 lib/fdtdec.c     |   13 +++++++++++++
 2 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/include/fdtdec.h b/include/fdtdec.h
index 6c0a2d1..f3115a6 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -272,3 +272,16 @@ int fdtdec_decode_gpio(const void *blob, int node, const char *prop_name,
  * @return 0 if all ok or gpio was FDT_GPIO_NONE; -1 on error
  */
 int fdtdec_setup_gpio(struct fdt_gpio_state *gpio);
+
+/**
+ * Decode a peripheral ID from a device tree node. This may be a temporary
+ * function depending on what happens with clocks in the Linux fdt.
+ *
+ * This works by looking up the peripheral's 'clocks' node and reading out
+ * the second cell, which is the clock number / peripheral ID.
+ *
+ * @param blob		FDT blob to use
+ * @param node		Node to look at
+ * @return
+ */
+int fdtdec_decode_periph_id(const void *blob, int node);
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index f0b2196..780948c 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -414,3 +414,16 @@ int fdtdec_setup_gpio(struct fdt_gpio_state *gpio)
 		return -1;
 	return 0;
 }
+
+int fdtdec_decode_periph_id(const void *blob, int node)
+{
+	u32 cell[2];
+	int err;
+
+	err = fdtdec_get_int_array(blob, node, "clocks", cell,
+				   ARRAY_SIZE(cell));
+	if (err)
+		return -1;
+
+	return cell[1];
+}
-- 
1.7.7.3

[U-Boot] [PATCH v4 16/20] tegra: usb: Add support for Tegra USB peripheral

[Simon Glass]

This adds basic support for the Tegra2 USB controller. Board files should
call board_usb_init() to set things up.

Configuration is performed through the FDT, with aliases used to set the
order of the ports, like this fragment:

        aliases {
		/* This defines the order of our USB ports */
                usb0 = "/usb at 0xc5008000";
                usb1 = "/usb at 0xc5000000";
        };

drivers/usb/host files ONLY: Acked-by: Remy Bohmer <linux@bohmer.net>
Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Rename params to timing
- Store entire fdt config in port list, not just register pointer
- Remove non-fdt operation of USB, since it is not needed
- Decode USB VBUS GPIO from the fdt
- Decode phy type differently (to match new kernel fdt)
- Rename tegra20-usb to tegra20-ehcui (to match new kernel fdt)
- Improve debug() printouts in case of failure to init USB

Changes in v3:
- Remove usbparams properties from fdt and moved them to C code

Changes in v4:
- Use updated fdtdec alias functiona to get USB aliases
- Use peripheral clock node to obtain peripheral ID

 arch/arm/cpu/armv7/tegra2/Makefile        |    4 +-
 arch/arm/cpu/armv7/tegra2/usb.c           |  426 +++++++++++++++++++++++++++++
 arch/arm/include/asm/arch-tegra2/tegra2.h |    2 +
 arch/arm/include/asm/arch-tegra2/usb.h    |  255 +++++++++++++++++
 drivers/usb/host/Makefile                 |    1 +
 drivers/usb/host/ehci-tegra.c             |   63 +++++
 include/fdtdec.h                          |    1 +
 lib/fdtdec.c                              |    1 +
 8 files changed, 752 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/cpu/armv7/tegra2/usb.c
 create mode 100644 arch/arm/include/asm/arch-tegra2/usb.h
 create mode 100644 drivers/usb/host/ehci-tegra.c

diff --git a/arch/arm/cpu/armv7/tegra2/Makefile b/arch/arm/cpu/armv7/tegra2/Makefile
index f668a81..e9ac6c9 100644
--- a/arch/arm/cpu/armv7/tegra2/Makefile
+++ b/arch/arm/cpu/armv7/tegra2/Makefile
@@ -33,8 +33,10 @@ include $(TOPDIR)/config.mk
 LIB	=  $(obj)lib$(SOC).o
 
 SOBJS	:= lowlevel_init.o
-COBJS	:= ap20.o board.o clock.o funcmux.o pinmux.o sys_info.o timer.o
+COBJS-y	:= ap20.o board.o clock.o funcmux.o pinmux.o sys_info.o timer.o
+COBJS-$(CONFIG_USB_EHCI_TEGRA) += usb.o
 
+COBJS	:= $(COBJS-y)
 SRCS	:= $(SOBJS:.o=.S) $(COBJS:.o=.c)
 OBJS	:= $(addprefix $(obj),$(COBJS) $(SOBJS))
 
diff --git a/arch/arm/cpu/armv7/tegra2/usb.c b/arch/arm/cpu/armv7/tegra2/usb.c
new file mode 100644
index 0000000..62d32b4
--- /dev/null
+++ b/arch/arm/cpu/armv7/tegra2/usb.c
@@ -0,0 +1,426 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * (C) Copyright 2010,2011 NVIDIA Corporation <www.nvidia.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+#include <asm-generic/gpio.h>
+#include <asm/arch/tegra2.h>
+#include <asm/arch/clk_rst.h>
+#include <asm/arch/clock.h>
+#include <asm/arch/gpio.h>
+#include <asm/arch/pinmux.h>
+#include <asm/arch/sys_proto.h>
+#include <asm/arch/uart.h>
+#include <asm/arch/usb.h>
+#include <libfdt.h>
+#include <fdtdec.h>
+
+enum {
+	USB_PORTS_MAX	= 4,			/* Maximum ports we allow */
+};
+
+/* Parameters we need for USB */
+enum {
+	PARAM_DIVN,                     /* PLL FEEDBACK DIVIDer */
+	PARAM_DIVM,                     /* PLL INPUT DIVIDER */
+	PARAM_DIVP,                     /* POST DIVIDER (2^N) */
+	PARAM_CPCON,                    /* BASE PLLC CHARGE Pump setup ctrl */
+	PARAM_LFCON,                    /* BASE PLLC LOOP FILter setup ctrl */
+	PARAM_ENABLE_DELAY_COUNT,       /* PLL-U Enable Delay Count */
+	PARAM_STABLE_COUNT,             /* PLL-U STABLE count */
+	PARAM_ACTIVE_DELAY_COUNT,       /* PLL-U Active delay count */
+	PARAM_XTAL_FREQ_COUNT,          /* PLL-U XTAL frequency count */
+	PARAM_DEBOUNCE_A_TIME,          /* 10MS DELAY for BIAS_DEBOUNCE_A */
+	PARAM_BIAS_TIME,                /* 20US DELAY AFter bias cell op */
+
+	PARAM_COUNT
+};
+
+/* Information about a USB port */
+struct fdt_usb {
+	struct usb_ctlr *reg;   /* address of registers in physical memory */
+	int host_mode;          /* 1 if port has host mode, else 0 */
+	int utmi;               /* 1 if port has external tranceiver, else 0 */
+	int enabled;            /* 1 to enable, 0 to disable */
+	enum periph_id periph_id;/* peripheral id */
+	struct fdt_gpio_state vbus_gpio;	/* GPIO for vbus enable */
+};
+
+static struct fdt_usb port[USB_PORTS_MAX];	/* List of valid USB ports */
+static unsigned port_count;			/* Number of available ports */
+static int port_current;			/* Current port (-1 = none) */
+
+/* Record which controller can switch from host to device mode */
+static struct fdt_usb *host_dev_ctlr;
+
+/*
+ * This table has USB timing parameters for each Oscillator frequency we
+ * support. There are four sets of values:
+ *
+ * 1. PLLU configuration information (reference clock is osc/clk_m and
+ * PLLU-FOs are fixed at 12MHz/60MHz/480MHz).
+ *
+ *  Reference frequency     13.0MHz      19.2MHz      12.0MHz      26.0MHz
+ *  ----------------------------------------------------------------------
+ *      DIVN                960 (0x3c0)  200 (0c8)    960 (3c0h)   960 (3c0)
+ *      DIVM                13 (0d)      4 (04)       12 (0c)      26 (1a)
+ * Filter frequency (MHz)   1            4.8          6            2
+ * CPCON                    1100b        0011b        1100b        1100b
+ * LFCON0                   0            0            0            0
+ *
+ * 2. PLL CONFIGURATION & PARAMETERS for different clock generators:
+ *
+ * Reference frequency     13.0MHz         19.2MHz         12.0MHz     26.0MHz
+ * ---------------------------------------------------------------------------
+ * PLLU_ENABLE_DLY_COUNT   02 (0x02)       03 (03)         02 (02)     04 (04)
+ * PLLU_STABLE_COUNT       51 (33)         75 (4B)         47 (2F)    102 (66)
+ * PLL_ACTIVE_DLY_COUNT    05 (05)         06 (06)         04 (04)     09 (09)
+ * XTAL_FREQ_COUNT        127 (7F)        187 (BB)        118 (76)    254 (FE)
+ *
+ * 3. Debounce values IdDig, Avalid, Bvalid, VbusValid, VbusWakeUp, and
+ * SessEnd. Each of these signals have their own debouncer and for each of
+ * those one out of two debouncing times can be chosen (BIAS_DEBOUNCE_A or
+ * BIAS_DEBOUNCE_B).
+ *
+ * The values of DEBOUNCE_A and DEBOUNCE_B are calculated as follows:
+ *    0xffff -> No debouncing at all
+ *    <n> ms = <n> *1000 / (1/19.2MHz) / 4
+ *
+ * So to program a 1 ms debounce for BIAS_DEBOUNCE_A, we have:
+ * BIAS_DEBOUNCE_A[15:0] = 1000 * 19.2 / 4  = 4800 = 0x12c0
+ *
+ * We need to use only DebounceA for BOOTROM. We don't need the DebounceB
+ * values, so we can keep those to default.
+ *
+ * 4. The 20 microsecond delay after bias cell operation.
+ */
+static const unsigned usb_pll[CLOCK_OSC_FREQ_COUNT][PARAM_COUNT] = {
+	/* DivN, DivM, DivP, CPCON, LFCON, Delays             Debounce, Bias */
+	{ 0x3C0, 0x0D, 0x00, 0xC,   0,  0x02, 0x33, 0x05, 0x7F, 0x7EF4, 5 },
+	{ 0x0C8, 0x04, 0x00, 0x3,   0,  0x03, 0x4B, 0x06, 0xBB, 0xBB80, 7 },
+	{ 0x3C0, 0x0C, 0x00, 0xC,   0,  0x02, 0x2F, 0x04, 0x76, 0x7530, 5 },
+	{ 0x3C0, 0x1A, 0x00, 0xC,   0,  0x04, 0x66, 0x09, 0xFE, 0xFDE8, 9 }
+};
+
+/* UTMIP Idle Wait Delay */
+static const u8 utmip_idle_wait_delay = 17;
+
+/* UTMIP Elastic limit */
+static const u8 utmip_elastic_limit = 16;
+
+/* UTMIP High Speed Sync Start Delay */
+static const u8 utmip_hs_sync_start_delay = 9;
+
+/* Put the port into host mode (this only works for USB1) */
+static void set_host_mode(struct fdt_usb *config)
+{
+	/* Check whether remote host from USB1 is driving VBus */
+	if (readl(&config->reg->phy_vbus_sensors) & VBUS_VLD_STS)
+		return;
+
+	/*
+	 * If not driving, we set GPIO USB1_VBus_En. We assume that the
+	 * pinmux is set up correctly for this.
+	 */
+	if (fdt_gpio_isvalid(&config->vbus_gpio)) {
+		fdtdec_setup_gpio(&config->vbus_gpio);
+		gpio_direction_output(config->vbus_gpio.gpio, 1);
+	}
+}
+
+/* Put our ports into host mode */
+void usb_set_host_mode(void)
+{
+	if (host_dev_ctlr)
+		set_host_mode(host_dev_ctlr);
+}
+
+void usbf_reset_controller(enum periph_id id, struct usb_ctlr *usbctlr)
+{
+	/* Reset the USB controller with 2us delay */
+	reset_periph(id, 2);
+
+	/*
+	 * Set USB1_NO_LEGACY_MODE to 1, Registers are accessible under
+	 * base address
+	 */
+	if (id == PERIPH_ID_USBD)
+		setbits_le32(&usbctlr->usb1_legacy_ctrl, USB1_NO_LEGACY_MODE);
+
+	/* Put UTMIP1/3 in reset */
+	setbits_le32(&usbctlr->susp_ctrl, UTMIP_RESET);
+
+	/* Set USB3 to use UTMIP PHY */
+	if (id == PERIPH_ID_USB3)
+		setbits_le32(&usbctlr->susp_ctrl, UTMIP_PHY_ENB);
+
+	/*
+	 * TODO: where do we take the USB1 out of reset? The old code would
+	 * take USB3 out of reset, but not USB1. This code doesn't do either.
+	 */
+}
+
+/* set up the USB controller with the parameters provided */
+static void init_usb_controller(enum periph_id id, struct usb_ctlr *usbctlr,
+		const u32 timing[])
+{
+	u32 val;
+	int loop_count;
+
+	clock_enable(id);
+
+	/* Reset the usb controller */
+	usbf_reset_controller(id, usbctlr);
+
+	/* Stop crystal clock by setting UTMIP_PHY_XTAL_CLOCKEN low */
+	clrbits_le32(&usbctlr->utmip_misc_cfg1, UTMIP_PHY_XTAL_CLOCKEN);
+
+	/* Follow the crystal clock disable by >100ns delay */
+	udelay(1);
+
+	/*
+	 * To Use the A Session Valid for cable detection logic, VBUS_WAKEUP
+	 * mux must be switched to actually use a_sess_vld threshold.
+	 */
+	if (id == PERIPH_ID_USBD) {
+		clrsetbits_le32(&usbctlr->usb1_legacy_ctrl,
+			VBUS_SENSE_CTL_MASK, VBUS_SENSE_CTL_A_SESS_VLD);
+	}
+
+	/*
+	 * PLL Delay CONFIGURATION settings. The following parameters control
+	 * the bring up of the plls.
+	 */
+	val = readl(&usbctlr->utmip_misc_cfg1);
+	clrsetbits_le32(&val, UTMIP_PLLU_STABLE_COUNT_MASK,
+		timing[PARAM_STABLE_COUNT] << UTMIP_PLLU_STABLE_COUNT_SHIFT);
+	clrsetbits_le32(&val, UTMIP_PLL_ACTIVE_DLY_COUNT_MASK,
+		timing[PARAM_ACTIVE_DELAY_COUNT] <<
+			UTMIP_PLL_ACTIVE_DLY_COUNT_SHIFT);
+	writel(val, &usbctlr->utmip_misc_cfg1);
+
+	/* Set PLL enable delay count and crystal frequency count */
+	val = readl(&usbctlr->utmip_pll_cfg1);
+	clrsetbits_le32(&val, UTMIP_PLLU_ENABLE_DLY_COUNT_MASK,
+		timing[PARAM_ENABLE_DELAY_COUNT] <<
+			UTMIP_PLLU_ENABLE_DLY_COUNT_SHIFT);
+	clrsetbits_le32(&val, UTMIP_XTAL_FREQ_COUNT_MASK,
+		timing[PARAM_XTAL_FREQ_COUNT] <<
+			UTMIP_XTAL_FREQ_COUNT_SHIFT);
+	writel(val, &usbctlr->utmip_pll_cfg1);
+
+	/* Setting the tracking length time */
+	clrsetbits_le32(&usbctlr->utmip_bias_cfg1,
+		UTMIP_BIAS_PDTRK_COUNT_MASK,
+		timing[PARAM_BIAS_TIME] << UTMIP_BIAS_PDTRK_COUNT_SHIFT);
+
+	/* Program debounce time for VBUS to become valid */
+	clrsetbits_le32(&usbctlr->utmip_debounce_cfg0,
+		UTMIP_DEBOUNCE_CFG0_MASK,
+		timing[PARAM_DEBOUNCE_A_TIME] << UTMIP_DEBOUNCE_CFG0_SHIFT);
+
+	setbits_le32(&usbctlr->utmip_tx_cfg0, UTMIP_FS_PREAMBLE_J);
+
+	/* Disable battery charge enabling bit */
+	setbits_le32(&usbctlr->utmip_bat_chrg_cfg0, UTMIP_PD_CHRG);
+
+	clrbits_le32(&usbctlr->utmip_xcvr_cfg0, UTMIP_XCVR_LSBIAS_SE);
+	setbits_le32(&usbctlr->utmip_spare_cfg0, FUSE_SETUP_SEL);
+
+	/*
+	 * Configure the UTMIP_IDLE_WAIT and UTMIP_ELASTIC_LIMIT
+	 * Setting these fields, together with default values of the
+	 * other fields, results in programming the registers below as
+	 * follows:
+	 *         UTMIP_HSRX_CFG0 = 0x9168c000
+	 *         UTMIP_HSRX_CFG1 = 0x13
+	 */
+
+	/* Set PLL enable delay count and Crystal frequency count */
+	val = readl(&usbctlr->utmip_hsrx_cfg0);
+	clrsetbits_le32(&val, UTMIP_IDLE_WAIT_MASK,
+		utmip_idle_wait_delay << UTMIP_IDLE_WAIT_SHIFT);
+	clrsetbits_le32(&val, UTMIP_ELASTIC_LIMIT_MASK,
+		utmip_elastic_limit << UTMIP_ELASTIC_LIMIT_SHIFT);
+	writel(val, &usbctlr->utmip_hsrx_cfg0);
+
+	/* Configure the UTMIP_HS_SYNC_START_DLY */
+	clrsetbits_le32(&usbctlr->utmip_hsrx_cfg1,
+		UTMIP_HS_SYNC_START_DLY_MASK,
+		utmip_hs_sync_start_delay << UTMIP_HS_SYNC_START_DLY_SHIFT);
+
+	/* Preceed the crystal clock disable by >100ns delay. */
+	udelay(1);
+
+	/* Resuscitate crystal clock by setting UTMIP_PHY_XTAL_CLOCKEN */
+	setbits_le32(&usbctlr->utmip_misc_cfg1, UTMIP_PHY_XTAL_CLOCKEN);
+
+	/* Finished the per-controller init. */
+
+	/* De-assert UTMIP_RESET to bring out of reset. */
+	clrbits_le32(&usbctlr->susp_ctrl, UTMIP_RESET);
+
+	/* Wait for the phy clock to become valid in 100 ms */
+	for (loop_count = 100000; loop_count != 0; loop_count--) {
+		if (readl(&usbctlr->susp_ctrl) & USB_PHY_CLK_VALID)
+			break;
+		udelay(1);
+	}
+}
+
+static void power_up_port(struct usb_ctlr *usbctlr)
+{
+	/* Deassert power down state */
+	clrbits_le32(&usbctlr->utmip_xcvr_cfg0, UTMIP_FORCE_PD_POWERDOWN |
+		UTMIP_FORCE_PD2_POWERDOWN | UTMIP_FORCE_PDZI_POWERDOWN);
+	clrbits_le32(&usbctlr->utmip_xcvr_cfg1, UTMIP_FORCE_PDDISC_POWERDOWN |
+		UTMIP_FORCE_PDCHRP_POWERDOWN | UTMIP_FORCE_PDDR_POWERDOWN);
+}
+
+static void config_clock(const u32 timing[])
+{
+	clock_start_pll(CLOCK_ID_USB,
+		timing[PARAM_DIVM], timing[PARAM_DIVN], timing[PARAM_DIVP],
+		timing[PARAM_CPCON], timing[PARAM_LFCON]);
+}
+
+/**
+ * Add a new USB port to the list of available ports.
+ *
+ * @param config	USB port configuration
+ * @return 0 if ok, -1 if error (too many ports)
+ */
+static int add_port(struct fdt_usb *config, const u32 timing[])
+{
+	struct usb_ctlr *usbctlr = config->reg;
+
+	if (port_count == USB_PORTS_MAX) {
+		debug("tegrausb: Cannot register more than %d ports\n",
+		      USB_PORTS_MAX);
+		return -1;
+	}
+	init_usb_controller(config->periph_id, usbctlr, timing);
+	if (config->utmi) {
+		/* Disable ICUSB FS/LS transceiver */
+		clrbits_le32(&usbctlr->icusb_ctrl, IC_ENB1);
+
+		/* Select UTMI parallel interface */
+		clrsetbits_le32(&usbctlr->port_sc1, PTS_MASK,
+				PTS_UTMI << PTS_SHIFT);
+		clrbits_le32(&usbctlr->port_sc1, STS);
+		power_up_port(usbctlr);
+	}
+	if (config->host_mode) {
+		/* Only one host-dev port is supported */
+		if (host_dev_ctlr)
+			return -1;
+		host_dev_ctlr = &port[port_count];
+	}
+	port[port_count++] = *config;
+	return 0;
+}
+
+int tegrausb_start_port(unsigned portnum, u32 *hccr, u32 *hcor)
+{
+	struct usb_ctlr *usbctlr;
+
+	if (portnum >= port_count)
+		return -1;
+	tegrausb_stop_port();
+
+	usbctlr = port[portnum].reg;
+	*hccr = (u32)&usbctlr->cap_length;
+	*hcor = (u32)&usbctlr->usb_cmd;
+	port_current = portnum;
+	return 0;
+}
+
+int tegrausb_stop_port(void)
+{
+	struct usb_ctlr *usbctlr;
+
+	if (port_current == -1)
+		return -1;
+
+	usbctlr = port[port_current].reg;
+
+	/* Stop controller */
+	writel(0, &usbctlr->usb_cmd);
+	udelay(1000);
+
+	/* Initiate controller reset */
+	writel(2, &usbctlr->usb_cmd);
+	udelay(1000);
+	port_current = -1;
+	return 0;
+}
+
+int fdt_decode_usb(const void *blob, int node, unsigned osc_frequency_mhz,
+		   struct fdt_usb *config)
+{
+	const char *phy;
+
+	config->reg = (struct usb_ctlr *)fdtdec_get_addr(blob, node, "reg");
+	config->host_mode = fdtdec_get_bool(blob, node, "support-host-mode");
+	phy = fdt_getprop(blob, node, "phy_type", NULL);
+	config->utmi = phy && 0 == strcmp("utmi", phy);
+	config->enabled = fdtdec_get_is_enabled(blob, node);
+	config->periph_id = fdtdec_decode_periph_id(blob, node);
+	if (config->periph_id == -1)
+		return -FDT_ERR_NOTFOUND;
+	fdtdec_decode_gpio(blob, node, "nvidia,vbus-gpio", &config->vbus_gpio);
+	return 0;
+}
+
+int board_usb_init(const void *blob)
+{
+	struct fdt_usb config;
+	unsigned osc_freq = clock_get_rate(CLOCK_ID_OSC);
+	enum clock_osc_freq freq;
+	int node_list[USB_PORTS_MAX];
+	int node, count, i;
+
+	/* Set up the USB clocks correctly based on our oscillator frequency */
+	freq = clock_get_osc_freq();
+	config_clock(usb_pll[freq]);
+
+	/* count may return <0 on error */
+	count = fdtdec_find_aliases_for_id(blob, "usb",
+			COMPAT_NVIDIA_TEGRA20_USB, node_list, USB_PORTS_MAX);
+	for (i = 0; i < count; i++) {
+		node = node_list[i];
+		if (!node)
+			continue;
+		if (fdt_decode_usb(blob, node, osc_freq, &config)) {
+			debug("Cannot decode USB node %s\n",
+			      fdt_get_name(blob, node, NULL));
+			return -1;
+		}
+
+		if (add_port(&config, usb_pll[freq]))
+			return -1;
+	}
+	usb_set_host_mode();
+	port_current = -1;
+	return 0;
+}
diff --git a/arch/arm/include/asm/arch-tegra2/tegra2.h b/arch/arm/include/asm/arch-tegra2/tegra2.h
index 8941443..baae2eb 100644
--- a/arch/arm/include/asm/arch-tegra2/tegra2.h
+++ b/arch/arm/include/asm/arch-tegra2/tegra2.h
@@ -41,6 +41,8 @@
 #define TEGRA2_SPI_BASE		(NV_PA_APB_MISC_BASE + 0xC380)
 #define NV_PA_PMC_BASE		0x7000E400
 #define NV_PA_CSITE_BASE	0x70040000
+#define TEGRA_USB1_BASE		0xC5000000
+#define TEGRA_USB3_BASE		0xC5008000
 
 #define TEGRA2_SDRC_CS0		NV_PA_SDRAM_BASE
 #define LOW_LEVEL_SRAM_STACK	0x4000FFFC
diff --git a/arch/arm/include/asm/arch-tegra2/usb.h b/arch/arm/include/asm/arch-tegra2/usb.h
new file mode 100644
index 0000000..0a3056d
--- /dev/null
+++ b/arch/arm/include/asm/arch-tegra2/usb.h
@@ -0,0 +1,255 @@
+/*
+ * Copyright (c) 2011 The Chromium OS Authors.
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _TEGRA_USB_H_
+#define _TEGRA_USB_H_
+
+
+/* USB Controller (USBx_CONTROLLER_) regs */
+struct usb_ctlr {
+	/* 0x000 */
+	uint id;
+	uint reserved0;
+	uint host;
+	uint device;
+
+	/* 0x010 */
+	uint txbuf;
+	uint rxbuf;
+	uint reserved1[2];
+
+	/* 0x020 */
+	uint reserved2[56];
+
+	/* 0x100 */
+	u16 cap_length;
+	u16 hci_version;
+	uint hcs_params;
+	uint hcc_params;
+	uint reserved3[5];
+
+	/* 0x120 */
+	uint dci_version;
+	uint dcc_params;
+	uint reserved4[6];
+
+	/* 0x140 */
+	uint usb_cmd;
+	uint usb_sts;
+	uint usb_intr;
+	uint frindex;
+
+	/* 0x150 */
+	uint reserved5;
+	uint periodic_list_base;
+	uint async_list_addr;
+	uint async_tt_sts;
+
+	/* 0x160 */
+	uint burst_size;
+	uint tx_fill_tuning;
+	uint reserved6;   /* is this port_sc1 on some controllers? */
+	uint icusb_ctrl;
+
+	/* 0x170 */
+	uint ulpi_viewport;
+	uint reserved7;
+	uint endpt_nak;
+	uint endpt_nak_enable;
+
+	/* 0x180 */
+	uint reserved;
+	uint port_sc1;
+	uint reserved8[6];
+
+	/* 0x1a0 */
+	uint reserved9;
+	uint otgsc;
+	uint usb_mode;
+	uint endpt_setup_stat;
+
+	/* 0x1b0 */
+	uint reserved10[20];
+
+	/* 0x200 */
+	uint reserved11[0x80];
+
+	/* 0x400 */
+	uint susp_ctrl;
+	uint phy_vbus_sensors;
+	uint phy_vbus_wakeup_id;
+	uint phy_alt_vbus_sys;
+
+	/* 0x410 */
+	uint usb1_legacy_ctrl;
+	uint reserved12[3];
+
+	/* 0x420 */
+	uint reserved13[56];
+
+	/* 0x500 */
+	uint reserved14[64 * 3];
+
+	/* 0x800 */
+	uint utmip_pll_cfg0;
+	uint utmip_pll_cfg1;
+	uint utmip_xcvr_cfg0;
+	uint utmip_bias_cfg0;
+
+	/* 0x810 */
+	uint utmip_hsrx_cfg0;
+	uint utmip_hsrx_cfg1;
+	uint utmip_fslsrx_cfg0;
+	uint utmip_fslsrx_cfg1;
+
+	/* 0x820 */
+	uint utmip_tx_cfg0;
+	uint utmip_misc_cfg0;
+	uint utmip_misc_cfg1;
+	uint utmip_debounce_cfg0;
+
+	/* 0x830 */
+	uint utmip_bat_chrg_cfg0;
+	uint utmip_spare_cfg0;
+	uint utmip_xcvr_cfg1;
+	uint utmip_bias_cfg1;
+};
+
+
+/* USB1_LEGACY_CTRL */
+#define USB1_NO_LEGACY_MODE		1
+
+#define VBUS_SENSE_CTL_SHIFT			1
+#define VBUS_SENSE_CTL_MASK			(3 << VBUS_SENSE_CTL_SHIFT)
+#define VBUS_SENSE_CTL_VBUS_WAKEUP		0
+#define VBUS_SENSE_CTL_AB_SESS_VLD_OR_VBUS_WAKEUP	1
+#define VBUS_SENSE_CTL_AB_SESS_VLD		2
+#define VBUS_SENSE_CTL_A_SESS_VLD		3
+
+/* USBx_IF_USB_SUSP_CTRL_0 */
+#define UTMIP_PHY_ENB			        (1 << 12)
+#define UTMIP_RESET			        (1 << 11)
+#define USB_PHY_CLK_VALID			(1 << 7)
+
+/* USBx_UTMIP_MISC_CFG1 */
+#define UTMIP_PLLU_STABLE_COUNT_SHIFT		6
+#define UTMIP_PLLU_STABLE_COUNT_MASK		\
+				(0xfff << UTMIP_PLLU_STABLE_COUNT_SHIFT)
+#define UTMIP_PLL_ACTIVE_DLY_COUNT_SHIFT	18
+#define UTMIP_PLL_ACTIVE_DLY_COUNT_MASK		\
+				(0x1f << UTMIP_PLL_ACTIVE_DLY_COUNT_SHIFT)
+#define UTMIP_PHY_XTAL_CLOCKEN			(1 << 30)
+
+/* USBx_UTMIP_PLL_CFG1_0 */
+#define UTMIP_PLLU_ENABLE_DLY_COUNT_SHIFT	27
+#define UTMIP_PLLU_ENABLE_DLY_COUNT_MASK	\
+				(0xf << UTMIP_PLLU_ENABLE_DLY_COUNT_SHIFT)
+#define UTMIP_XTAL_FREQ_COUNT_SHIFT		0
+#define UTMIP_XTAL_FREQ_COUNT_MASK		0xfff
+
+/* USBx_UTMIP_BIAS_CFG1_0 */
+#define UTMIP_BIAS_PDTRK_COUNT_SHIFT		3
+#define UTMIP_BIAS_PDTRK_COUNT_MASK		\
+				(0x1f << UTMIP_BIAS_PDTRK_COUNT_SHIFT)
+#define UTMIP_BIAS_PDTRK_COUNT_SHIFT		3
+#define UTMIP_BIAS_PDTRK_COUNT_MASK		\
+				(0x1f << UTMIP_BIAS_PDTRK_COUNT_SHIFT)
+
+#define UTMIP_DEBOUNCE_CFG0_SHIFT		0
+#define UTMIP_DEBOUNCE_CFG0_MASK		0xffff
+
+/* USBx_UTMIP_TX_CFG0_0 */
+#define UTMIP_FS_PREAMBLE_J			(1 << 19)
+
+/* USBx_UTMIP_BAT_CHRG_CFG0_0 */
+#define UTMIP_PD_CHRG				1
+
+/* USBx_UTMIP_XCVR_CFG0_0 */
+#define UTMIP_XCVR_LSBIAS_SE			(1 << 21)
+
+/* USBx_UTMIP_SPARE_CFG0_0 */
+#define FUSE_SETUP_SEL				(1 << 3)
+
+/* USBx_UTMIP_HSRX_CFG0_0 */
+#define UTMIP_IDLE_WAIT_SHIFT			15
+#define UTMIP_IDLE_WAIT_MASK			(0x1f << UTMIP_IDLE_WAIT_SHIFT)
+#define UTMIP_ELASTIC_LIMIT_SHIFT		10
+#define UTMIP_ELASTIC_LIMIT_MASK		\
+				(0x1f << UTMIP_ELASTIC_LIMIT_SHIFT)
+
+/* USBx_UTMIP_HSRX_CFG0_1 */
+#define UTMIP_HS_SYNC_START_DLY_SHIFT		1
+#define UTMIP_HS_SYNC_START_DLY_MASK		\
+				(0xf << UTMIP_HS_SYNC_START_DLY_SHIFT)
+
+/* USBx_CONTROLLER_2_USB2D_ICUSB_CTRL_0 */
+#define IC_ENB1					(1 << 3)
+
+/* SB2_CONTROLLER_2_USB2D_PORTSC1_0 */
+#define PTS_SHIFT				30
+#define PTS_MASK				(3 << PTS_SHIFT)
+#define PTS_UTMI	0
+#define PTS_RESERVED	1
+#define PTS_ULP		2
+#define PTS_ICUSB_SER	3
+
+#define STS					(1 << 29)
+
+/* USBx_UTMIP_XCVR_CFG0_0 */
+#define UTMIP_FORCE_PD_POWERDOWN		(1 << 14)
+#define UTMIP_FORCE_PD2_POWERDOWN		(1 << 16)
+#define UTMIP_FORCE_PDZI_POWERDOWN		(1 << 18)
+
+/* USBx_UTMIP_XCVR_CFG1_0 */
+#define UTMIP_FORCE_PDDISC_POWERDOWN		(1 << 0)
+#define UTMIP_FORCE_PDCHRP_POWERDOWN		(1 << 2)
+#define UTMIP_FORCE_PDDR_POWERDOWN		(1 << 4)
+
+/* USB3_IF_USB_PHY_VBUS_SENSORS_0 */
+#define VBUS_VLD_STS			(1 << 26)
+
+
+/* Change the USB host port into host mode */
+void usb_set_host_mode(void);
+
+/* Setup USB on the board */
+int board_usb_init(const void *blob);
+
+/**
+ * Start up the given port number (ports are numbered from 0 on each board).
+ * This returns values for the appropriate hccr and hcor addresses to use for
+ * USB EHCI operations.
+ *
+ * @param portnum	port number to start
+ * @param hccr		returns start address of EHCI HCCR registers
+ * @param hcor		returns start address of EHCI HCOR registers
+ * @return 0 if ok, -1 on error (generally invalid port number)
+ */
+int tegrausb_start_port(unsigned portnum, u32 *hccr, u32 *hcor);
+
+/**
+ * Stop the current port
+ *
+ * @return 0 if ok, -1 if no port was active
+ */
+int tegrausb_stop_port(void);
+
+#endif	/* _TEGRA_USB_H_ */
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 77e217f..a11cebb 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -47,6 +47,7 @@ COBJS-$(CONFIG_USB_EHCI_PPC4XX) += ehci-ppc4xx.o
 COBJS-$(CONFIG_USB_EHCI_IXP4XX) += ehci-ixp.o
 COBJS-$(CONFIG_USB_EHCI_KIRKWOOD) += ehci-kirkwood.o
 COBJS-$(CONFIG_USB_EHCI_PCI) += ehci-pci.o
+COBJS-$(CONFIG_USB_EHCI_TEGRA) += ehci-tegra.o
 COBJS-$(CONFIG_USB_EHCI_VCT) += ehci-vct.o
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/usb/host/ehci-tegra.c b/drivers/usb/host/ehci-tegra.c
new file mode 100644
index 0000000..3a48c26
--- /dev/null
+++ b/drivers/usb/host/ehci-tegra.c
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2009 NVIDIA Corporation
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <usb.h>
+
+#include "ehci.h"
+#include "ehci-core.h"
+
+#include <asm/errno.h>
+#include <asm/arch/usb.h>
+
+
+/*
+ * Create the appropriate control structures to manage
+ * a new EHCI host controller.
+ */
+int ehci_hcd_init(void)
+{
+	u32 our_hccr, our_hcor;
+
+	/*
+	 * Select the first port, as we don't have a way of selecting others
+	 * yet
+	 */
+	if (tegrausb_start_port(0, &our_hccr, &our_hcor))
+		return -1;
+
+	hccr = (struct ehci_hccr *)our_hccr;
+	hcor = (struct ehci_hcor *)our_hcor;
+
+	return 0;
+}
+
+/*
+ * Destroy the appropriate control structures corresponding
+ * the the EHCI host controller.
+ */
+int ehci_hcd_stop(void)
+{
+	usb_set_host_mode();
+	tegrausb_stop_port();
+	return 0;
+}
diff --git a/include/fdtdec.h b/include/fdtdec.h
index f3115a6..bd2222c 100644
--- a/include/fdtdec.h
+++ b/include/fdtdec.h
@@ -57,6 +57,7 @@ struct fdt_memory {
  */
 enum fdt_compat_id {
 	COMPAT_UNKNOWN,
+	COMPAT_NVIDIA_TEGRA20_USB,	/* Tegra2 USB port */
 
 	COMPAT_COUNT,
 };
diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index 780948c..daf2c7e 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -37,6 +37,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define COMPAT(id, name) name
 static const char * const compat_names[COMPAT_COUNT] = {
 	COMPAT(UNKNOWN, "<none>"),
+	COMPAT(NVIDIA_TEGRA20_USB, "nvidia,tegra20-ehci"),
 };
 
 const char *fdtdec_get_compatible(enum fdt_compat_id id)
-- 
1.7.7.3

[U-Boot] [PATCH v4 16/20] tegra: usb: Add support for Tegra USB peripheral

[Remy Bohmer]

Hi,

2012/1/12 Simon Glass <sjg@chromium.org>:
> This adds basic support for the Tegra2 USB controller. Board files should
> call board_usb_init() to set things up.
>
> Configuration is performed through the FDT, with aliases used to set the
> order of the ports, like this fragment:
>
> ? ? ? ?aliases {
> ? ? ? ? ? ? ? ?/* This defines the order of our USB ports */
> ? ? ? ? ? ? ? ?usb0 = "/usb at 0xc5008000";
> ? ? ? ? ? ? ? ?usb1 = "/usb at 0xc5000000";
> ? ? ? ?};
>
> drivers/usb/host files ONLY: Acked-by: Remy Bohmer <linux@bohmer.net>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> Changes in v2:
> - Rename params to timing
> - Store entire fdt config in port list, not just register pointer
> - Remove non-fdt operation of USB, since it is not needed
> - Decode USB VBUS GPIO from the fdt
> - Decode phy type differently (to match new kernel fdt)
> - Rename tegra20-usb to tegra20-ehcui (to match new kernel fdt)
> - Improve debug() printouts in case of failure to init USB
>
> Changes in v3:
> - Remove usbparams properties from fdt and moved them to C code
>
> Changes in v4:
> - Use updated fdtdec alias functiona to get USB aliases
> - Use peripheral clock node to obtain peripheral ID
>
> ?arch/arm/cpu/armv7/tegra2/Makefile ? ? ? ?| ? ?4 +-
> ?arch/arm/cpu/armv7/tegra2/usb.c ? ? ? ? ? | ?426 +++++++++++++++++++++++++++++
> ?arch/arm/include/asm/arch-tegra2/tegra2.h | ? ?2 +
> ?arch/arm/include/asm/arch-tegra2/usb.h ? ?| ?255 +++++++++++++++++
> ?drivers/usb/host/Makefile ? ? ? ? ? ? ? ? | ? ?1 +
> ?drivers/usb/host/ehci-tegra.c ? ? ? ? ? ? | ? 63 +++++

As for the drivers/usb files:
Acked-by: Remy Bohmer <linux@bohmer.net>

Kind regards,

Remy

[U-Boot] [PATCH v4 16/20] tegra: usb: Add support for Tegra USB peripheral

[Stephen Warren]

On 01/11/2012 09:33 PM, Simon Glass wrote:
> This adds basic support for the Tegra2 USB controller. Board files should
> call board_usb_init() to set things up.

> diff --git a/arch/arm/cpu/armv7/tegra2/usb.c b/arch/arm/cpu/armv7/tegra2/usb.c

> +/* Record which controller can switch from host to device mode */
> +static struct fdt_usb *host_dev_ctlr;

As you'll note from my comments on the DT bindings, it doesn't make
sense to have a single global variable for that; both USB1 and USB3 can
switch at run-time apparently, and USB2 at initialization time.

> +void usbf_reset_controller(enum periph_id id, struct usb_ctlr *usbctlr)
...
> +       /*
> +        * Set USB1_NO_LEGACY_MODE to 1, Registers are accessible under
> +        * base address
> +        */
> +       if (id == PERIPH_ID_USBD)
> +               setbits_le32(&usbctlr->usb1_legacy_ctrl, USB1_NO_LEGACY_MODE);
...
> +       /* Set USB3 to use UTMIP PHY */
> +       if (id == PERIPH_ID_USB3)
> +               setbits_le32(&usbctlr->susp_ctrl, UTMIP_PHY_ENB);
...
> +static void init_usb_controller(enum periph_id id, struct usb_ctlr *usbctlr,
> +               const u32 timing[])
...
> +       /*
> +        * To Use the A Session Valid for cable detection logic, VBUS_WAKEUP
> +        * mux must be switched to actually use a_sess_vld threshold.
> +        */
> +       if (id == PERIPH_ID_USBD) {
> +               clrsetbits_le32(&usbctlr->usb1_legacy_ctrl,
> +                       VBUS_SENSE_CTL_MASK, VBUS_SENSE_CTL_A_SESS_VLD);
> +       }

Uggh. The driver shouldn't really be altering its behaviour based on
knowledge of the instance number. Can those conditions be modified to
test something else instead?

For the first (USBD) condition, it seems like if the registers are
different, we should really have either different compatible values, or
a flag in the DT to indicate this.

For the second (USB3) condition, shouldn't this be testing the phy type
field, not the instance ID?

For the third (USBD) condition, can it test whether there's a VBUS GPIO
defined, or whether an appropriate combination of my proposed
enable-host-mode/enable-device-mode properties are present?

-- 
nvpublic

[U-Boot] [PATCH v4 16/20] tegra: usb: Add support for Tegra USB peripheral

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 4:43 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:33 PM, Simon Glass wrote:
>> This adds basic support for the Tegra2 USB controller. Board files should
>> call board_usb_init() to set things up.
>
>> diff --git a/arch/arm/cpu/armv7/tegra2/usb.c b/arch/arm/cpu/armv7/tegra2/usb.c
>
>> +/* Record which controller can switch from host to device mode */
>> +static struct fdt_usb *host_dev_ctlr;
>
> As you'll note from my comments on the DT bindings, it doesn't make
> sense to have a single global variable for that; both USB1 and USB3 can
> switch at run-time apparently, and USB2 at initialization time.
>
>> +void usbf_reset_controller(enum periph_id id, struct usb_ctlr *usbctlr)
> ...
>> + ? ? ? /*
>> + ? ? ? ?* Set USB1_NO_LEGACY_MODE to 1, Registers are accessible under
>> + ? ? ? ?* base address
>> + ? ? ? ?*/
>> + ? ? ? if (id == PERIPH_ID_USBD)
>> + ? ? ? ? ? ? ? setbits_le32(&usbctlr->usb1_legacy_ctrl, USB1_NO_LEGACY_MODE);
> ...
>> + ? ? ? /* Set USB3 to use UTMIP PHY */
>> + ? ? ? if (id == PERIPH_ID_USB3)
>> + ? ? ? ? ? ? ? setbits_le32(&usbctlr->susp_ctrl, UTMIP_PHY_ENB);
> ...
>> +static void init_usb_controller(enum periph_id id, struct usb_ctlr *usbctlr,
>> + ? ? ? ? ? ? ? const u32 timing[])
> ...
>> + ? ? ? /*
>> + ? ? ? ?* To Use the A Session Valid for cable detection logic, VBUS_WAKEUP
>> + ? ? ? ?* mux must be switched to actually use a_sess_vld threshold.
>> + ? ? ? ?*/
>> + ? ? ? if (id == PERIPH_ID_USBD) {
>> + ? ? ? ? ? ? ? clrsetbits_le32(&usbctlr->usb1_legacy_ctrl,
>> + ? ? ? ? ? ? ? ? ? ? ? VBUS_SENSE_CTL_MASK, VBUS_SENSE_CTL_A_SESS_VLD);
>> + ? ? ? }
>
> Uggh. The driver shouldn't really be altering its behaviour based on
> knowledge of the instance number. Can those conditions be modified to
> test something else instead?
>
> For the first (USBD) condition, it seems like if the registers are
> different, we should really have either different compatible values, or
> a flag in the DT to indicate this.
>
> For the second (USB3) condition, shouldn't this be testing the phy type
> field, not the instance ID?
>
> For the third (USBD) condition, can it test whether there's a VBUS GPIO
> defined, or whether an appropriate combination of my proposed
> enable-host-mode/enable-device-mode properties are present?

Yes I'm pretty sure I can clean this up to make it more fdt-dependent.
Will have a look.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 17/20] tegra: usb: Add USB support to nvidia boards

[Simon Glass]

This adds basic USB support for port 0. The other port is not supported
yet.

Tegra2 (SeaBoard) # usb start
(Re)start USB...
USB:   Register 10011 NbrPorts 1
USB EHCI 1.00
scanning bus for devices... 5 USB Device(s) found
       scanning bus for storage devices... 1 Storage Device(s) found
Tegra2 (SeaBoard) # ext2load usb 0:3 10000000 /boot/vmlinuz
Loading file "/boot/vmlinuz" from usb device 0:3 (ROOT-A)
2932976 bytes read

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Add setting of pinmux for USB VBUS GPIO

 board/nvidia/common/board.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
index e8253a0..a7c566d 100644
--- a/board/nvidia/common/board.c
+++ b/board/nvidia/common/board.c
@@ -33,6 +33,7 @@
 #include <asm/arch/pinmux.h>
 #include <asm/arch/uart.h>
 #include <spi.h>
+#include <asm/arch/usb.h>
 #include "board.h"
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -69,6 +70,13 @@ int board_init(void)
 	/* boot param addr */
 	gd->bd->bi_boot_params = (NV_PA_SDRAM_BASE + 0x100);
 
+#ifdef CONFIG_USB_EHCI_TEGRA
+	/* For USB GPIO PD0. for now, since we have no pinmux in fdt */
+	pinmux_tristate_disable(PINGRP_SLXK);
+
+	board_usb_init(gd->fdt_blob);
+#endif
+
 	return 0;
 }
 
-- 
1.7.7.3

[U-Boot] [PATCH v4 17/20] tegra: usb: Add USB support to nvidia boards

[Stephen Warren]

On 01/11/2012 09:33 PM, Simon Glass wrote:
> This adds basic USB support for port 0. The other port is not supported
> yet.

> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c

> +#ifdef CONFIG_USB_EHCI_TEGRA
> +	/* For USB GPIO PD0. for now, since we have no pinmux in fdt */
> +	pinmux_tristate_disable(PINGRP_SLXK);

Can we ifdef this so it only happens on Seaboard? Who knows what other
boards use that pin group for. Unfortunately, I checked and this really
does appear required for Seaboard, since there's no pull on the
USB1_VBUS_EN GPIO, so Tegra must drive it.

-- 
nvpublic

[U-Boot] [PATCH v4 17/20] tegra: usb: Add USB support to nvidia boards

[Simon Glass]

Hi Stephen,

On Wed, Jan 18, 2012 at 4:30 PM, Stephen Warren <swarren@nvidia.com> wrote:
> On 01/11/2012 09:33 PM, Simon Glass wrote:
>> This adds basic USB support for port 0. The other port is not supported
>> yet.
>
>> diff --git a/board/nvidia/common/board.c b/board/nvidia/common/board.c
>
>> +#ifdef CONFIG_USB_EHCI_TEGRA
>> + ? ? /* For USB GPIO PD0. for now, since we have no pinmux in fdt */
>> + ? ? pinmux_tristate_disable(PINGRP_SLXK);
>
> Can we ifdef this so it only happens on Seaboard? Who knows what other
> boards use that pin group for. Unfortunately, I checked and this really
> does appear required for Seaboard, since there's no pull on the
> USB1_VBUS_EN GPIO, so Tegra must drive it.

Yes will do. The pinmux is the hole in our fdt armour at present as you know.

Regards,
Simon

>
> --
> nvpublic

[U-Boot] [PATCH v4 18/20] tegra: usb: Add common USB defines for tegra2 boards

[Simon Glass]

All Tegra2 boards should include tegra2-common. This adds the required
USB config to that file.

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

 include/configs/tegra2-common.h |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/include/configs/tegra2-common.h b/include/configs/tegra2-common.h
index e6f385f..6549d00 100644
--- a/include/configs/tegra2-common.h
+++ b/include/configs/tegra2-common.h
@@ -84,6 +84,20 @@
 #define CONFIG_SYS_BAUDRATE_TABLE	{4800, 9600, 19200, 38400, 57600,\
 					115200}
 
+/*
+ * USB Host. Tegra2 requires USB buffers to be aligned to a word boundary
+ */
+#define CONFIG_USB_EHCI_DATA_ALIGN	4
+
+/*
+ * This parameter affects a TXFILLTUNING field that controls how much data is
+ * sent to the latency fifo before it is sent to the wire. Without this
+ * parameter, the default (2) causes occasional Data Buffer Errors in OUT
+ * packets depending on the buffer address and size.
+ */
+#define CONFIG_USB_EHCI_TXFIFO_THRESH	10
+
+#define CONFIG_EHCI_IS_TDI
 /* include default commands */
 #include <config_cmd_default.h>
 
-- 
1.7.7.3

[U-Boot] [PATCH v4 19/20] tegra: usb: Enable USB on Seaboard

[Simon Glass]

Seaboard has a top port which is USB host or device, and a side port which
is host only.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v2:
- Remove unneeded CONFIG_TEGRA_USBx defines

 include/configs/seaboard.h |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index 261f952..b6d9f7a 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -72,4 +72,11 @@
 
 #define CONFIG_ENV_SECT_SIZE    CONFIG_ENV_SIZE
 #define CONFIG_ENV_OFFSET       (CONFIG_SPI_FLASH_SIZE - CONFIG_ENV_SECT_SIZE)
+
+/* USB Host support */
+#define CONFIG_USB_EHCI
+#define CONFIG_USB_EHCI_TEGRA
+#define CONFIG_USB_STORAGE
+#define CONFIG_CMD_USB
+
 #endif /* __CONFIG_H */
-- 
1.7.7.3

[U-Boot] [PATCH v4 20/20] tegra: fdt: Enable FDT support for Seaboard

[Simon Glass]

This switches Seaboard over to use FDT for run-time config instead of
CONFIG options. USB is the only user at present.

Signed-off-by: Simon Glass <sjg@chromium.org>
---
Changes in v3:
- Drop Tegra USB alignment patch as we will deal with this another way

 include/configs/seaboard.h |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/include/configs/seaboard.h b/include/configs/seaboard.h
index b6d9f7a..1dc775a 100644
--- a/include/configs/seaboard.h
+++ b/include/configs/seaboard.h
@@ -27,6 +27,11 @@
 #include <asm/sizes.h>
 #include "tegra2-common.h"
 
+/* Enable fdt support for Seaboard. Flash the image in u-boot-dtb.bin */
+#define CONFIG_DEFAULT_DEVICE_TREE	tegra2-seaboard
+#define CONFIG_OF_CONTROL
+#define CONFIG_OF_SEPARATE
+
 /* High-level configuration options */
 #define TEGRA2_SYSMEM		"mem=384M at 0M nvmem=128M at 384M mem=512M at 512M"
 #define V_PROMPT		"Tegra2 (SeaBoard) # "
-- 
1.7.7.3

[U-Boot] [PATCH v4 0/20] tegra: Add fdt definitions and USB driver

[Simon Glass]

Hi,

On Wed, Jan 11, 2012 at 8:32 PM, Simon Glass <sjg@chromium.org> wrote:
> This series brings in the Linux kernel fdt file and provides a working
> USB driver for Tegra2 Seaboard.
>
> (I have done this in one series since otherwise most of the fdt additions
> will just look like dead code.)
>
> The driver requires CONFIG_OF_CONTROL and a device tree to operate.
>
> Some enhancements to fdtdec are required to make this easier, and these
> are included in the series also. I have had to bring in basic GPIO
> support due to the request to put the USB VBUS into the fdt.
>
> Since the kernel recently got a very minimal USB binding, I have started
> with that and extended it where appropriate.
>
> Tegra likes to have cache-aligned buffers. I have dropped the patch which
> implements this since we will solve this problem by making callers align
> their buffers (as we did with MMC).
>
> Changes in v2:
> - Use "okay" instead of "ok" for fdt node status
> - Remove 0x from fdt aliases
> - Rename params to timing
> - Store entire fdt config in port list, not just register pointer
> - Remove non-fdt operation of USB, since it is not needed
> - Decode USB VBUS GPIO from the fdt
> - Decode phy type differently (to match new kernel fdt)
> - Rename tegra20-usb to tegra20-ehcui (to match new kernel fdt)
> - Improve debug() printouts in case of failure to init USB
> - Add setting of pinmux for USB VBUS GPIO
> - Remove unneeded CONFIG_TEGRA_USBx defines
>
> Changes in v3:
> - Remove "okay" from nodes since this is the default anyway
> - Fix device tree indenting with tabs instead of spaces
> - Disable USB2 which is not used on Seaboard
> - Remove usbparams properties from fdt and moved them to C code
> - Drop Tegra USB alignment patch as we will deal with this another way
>
> Changes in v4:
> - Add staging area for device tree bindings used in U-Boot
> - Add clock bindings for Tegra2x
> - Add fdtdec function to return peripheral ID
> - Use updated fdtdec alias functiona to get USB aliases
> - Use peripheral clock node to obtain peripheral ID

There are quite a number of comments for v5 due to the fdt binding
changes. So I am sending the entire series again to avoid confusion.
It is now down to only 18 patches though, so some light showing
through.

Regards,
Simon

>
> Simon Glass (20):
> ?fdt: Tidy up a few fdtdec problems
> ?fdt: Add functions to access phandles, arrays and bools
> ?fdt: Add basic support for decoding GPIO definitions
> ?arm: fdt: Ensure that an embedded fdt is word-aligned
> ?arm: fdt: Add skeleton device tree file from kernel
> ?tegra: fdt: Add Tegra2x device tree file from kernel
> ?tegra: fdt: Add device tree file for Tegra2 Seaboard from kernel
> ?fdt: Add staging area for device tree binding documentation
> ?fdt: Add tegra-usb bindings file from linux
> ?tegra: fdt: Add additional USB binding
> ?tegra: fdt: Add clock bindings
> ?tegra: usb: fdt: Add additional device tree definitions for USB ports
> ?tegra: usb: fdt: Add USB definitions for Tegra2 Seaboard
> ?usb: Add support for txfifo threshold
> ?fdt: Add function to return peripheral/clock ID
> ?tegra: usb: Add support for Tegra USB peripheral
> ?tegra: usb: Add USB support to nvidia boards
> ?tegra: usb: Add common USB defines for tegra2 boards
> ?tegra: usb: Enable USB on Seaboard
> ?tegra: fdt: Enable FDT support for Seaboard
>
> ?README ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?3 +
> ?arch/arm/cpu/armv7/tegra2/Makefile ? ? ? ? ? ? | ? ?4 +-
> ?arch/arm/cpu/armv7/tegra2/config.mk ? ? ? ? ? ?| ? ?2 +
> ?arch/arm/cpu/armv7/tegra2/usb.c ? ? ? ? ? ? ? ?| ?426 ++++++++++++++++++++++++
> ?arch/arm/cpu/armv7/u-boot.lds ? ? ? ? ? ? ? ? ?| ? ?5 +
> ?arch/arm/dts/skeleton.dtsi ? ? ? ? ? ? ? ? ? ? | ? 13 +
> ?arch/arm/dts/tegra20.dtsi ? ? ? ? ? ? ? ? ? ? ?| ?199 +++++++++++
> ?arch/arm/include/asm/arch-tegra2/tegra2.h ? ? ?| ? ?2 +
> ?arch/arm/include/asm/arch-tegra2/usb.h ? ? ? ? | ?255 ++++++++++++++
> ?board/nvidia/common/board.c ? ? ? ? ? ? ? ? ? ?| ? ?8 +
> ?board/nvidia/dts/tegra2-seaboard.dts ? ? ? ? ? | ? 47 +++
> ?doc/device-tree-bindings/README ? ? ? ? ? ? ? ?| ? 19 +
> ?doc/device-tree-bindings/clock/tegra-periphclk | ? 51 +++
> ?doc/device-tree-bindings/usb/tegra-usb.txt ? ? | ? 17 +
> ?drivers/usb/host/Makefile ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
> ?drivers/usb/host/ehci-hcd.c ? ? ? ? ? ? ? ? ? ?| ? ?7 +
> ?drivers/usb/host/ehci-tegra.c ? ? ? ? ? ? ? ? ?| ? 63 ++++
> ?drivers/usb/host/ehci.h ? ? ? ? ? ? ? ? ? ? ? ?| ? ?6 +-
> ?dts/Makefile ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ? ?2 +-
> ?include/configs/seaboard.h ? ? ? ? ? ? ? ? ? ? | ? 12 +
> ?include/configs/tegra2-common.h ? ? ? ? ? ? ? ?| ? 14 +
> ?include/fdtdec.h ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ?121 +++++++-
> ?lib/fdtdec.c ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ?181 ++++++++++-
> ?23 files changed, 1445 insertions(+), 13 deletions(-)
> ?create mode 100644 arch/arm/cpu/armv7/tegra2/usb.c
> ?create mode 100644 arch/arm/dts/skeleton.dtsi
> ?create mode 100644 arch/arm/dts/tegra20.dtsi
> ?create mode 100644 arch/arm/include/asm/arch-tegra2/usb.h
> ?create mode 100644 board/nvidia/dts/tegra2-seaboard.dts
> ?create mode 100644 doc/device-tree-bindings/README
> ?create mode 100644 doc/device-tree-bindings/clock/tegra-periphclk
> ?create mode 100644 doc/device-tree-bindings/usb/tegra-usb.txt
> ?create mode 100644 drivers/usb/host/ehci-tegra.c
>
> --
> 1.7.7.3
>