[PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[Stephen Neuendorffer]

This code is needed to boot without a boot loader.

Grant:  I'm not sure where the right place to put this is.  I'm assuming we'll actually need some boot code that is not generic?  Also, note that there is a V4FX errata workaround in arch/ppc/boot/head.S, which probably also needs to get pulled to powerpc.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/boot/raw-platform.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/boot/raw-platform.c b/arch/powerpc/boot/raw-platform.c
index b9caeee..2a5e493 100644
--- a/arch/powerpc/boot/raw-platform.c
+++ b/arch/powerpc/boot/raw-platform.c
@@ -24,6 +24,28 @@ void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
                    unsigned long r6, unsigned long r7)
 {
 	u64 memsize64 = memsize[0];
+	static const unsigned long line_size = 32;
+	static const unsigned long congruence_classes = 256;
+	unsigned long addr;
+	unsigned long dccr;
+
+	/*
+	 * Invalidate the data cache if the data cache is turned off.
+	 * - The 405 core does not invalidate the data cache on power-up
+	 *   or reset but does turn off the data cache. We cannot assume
+	 *   that the cache contents are valid.
+	 * - If the data cache is turned on this must have been done by
+	 *   a bootloader and we assume that the cache contents are
+	 *   valid.
+	 */
+	__asm__("mfdccr %0": "=r" (dccr));
+	if (dccr == 0) {
+		for (addr = 0;
+		     addr < (congruence_classes * line_size);
+		     addr += line_size) {
+			__asm__("dccci 0,%0": :"b"(addr));
+		}
+	}
 
 	if (mem_size_cells == 2) {
 		memsize64 <<= 32;
-- 
1.5.3.4

Re: [PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[Benjamin Herrenschmidt]

On Thu, 2007-12-13 at 15:43 -0800, Stephen Neuendorffer wrote:
> This code is needed to boot without a boot loader.
> 
> Grant:  I'm not sure where the right place to put this is.  I'm assuming we'll actually need some boot code that is not generic?  Also, note that there is a V4FX errata workaround in arch/ppc/boot/head.S, which probably also needs to get pulled to powerpc.
> 
> Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> ---
>  arch/powerpc/boot/raw-platform.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)

This raw-platform.c file doesn't seem like a good place for code that is
totally platform specific ...

Ben.

> diff --git a/arch/powerpc/boot/raw-platform.c b/arch/powerpc/boot/raw-platform.c
> index b9caeee..2a5e493 100644
> --- a/arch/powerpc/boot/raw-platform.c
> +++ b/arch/powerpc/boot/raw-platform.c
> @@ -24,6 +24,28 @@ void platform_init(unsigned long r3, unsigned long r4, unsigned long r5,
>                     unsigned long r6, unsigned long r7)
>  {
>  	u64 memsize64 = memsize[0];
> +	static const unsigned long line_size = 32;
> +	static const unsigned long congruence_classes = 256;
> +	unsigned long addr;
> +	unsigned long dccr;
> +
> +	/*
> +	 * Invalidate the data cache if the data cache is turned off.
> +	 * - The 405 core does not invalidate the data cache on power-up
> +	 *   or reset but does turn off the data cache. We cannot assume
> +	 *   that the cache contents are valid.
> +	 * - If the data cache is turned on this must have been done by
> +	 *   a bootloader and we assume that the cache contents are
> +	 *   valid.
> +	 */
> +	__asm__("mfdccr %0": "=r" (dccr));
> +	if (dccr == 0) {
> +		for (addr = 0;
> +		     addr < (congruence_classes * line_size);
> +		     addr += line_size) {
> +			__asm__("dccci 0,%0": :"b"(addr));
> +		}
> +	}
>  
>  	if (mem_size_cells == 2) {
>  		memsize64 <<= 32;

Re: [PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[Benjamin Herrenschmidt]

On Fri, 2007-12-14 at 11:07 +1100, Benjamin Herrenschmidt wrote:
> On Thu, 2007-12-13 at 15:43 -0800, Stephen Neuendorffer wrote:
> > This code is needed to boot without a boot loader.
> > 
> > Grant:  I'm not sure where the right place to put this is.  I'm assuming we'll actually need some boot code that is not generic?  Also, note that there is a V4FX errata workaround in arch/ppc/boot/head.S, which probably also needs to get pulled to powerpc.
> > 
> > Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
> > ---
> >  arch/powerpc/boot/raw-platform.c |   22 ++++++++++++++++++++++
> >  1 files changed, 22 insertions(+), 0 deletions(-)
> 
> This raw-platform.c file doesn't seem like a good place for code that is
> totally platform specific ...

Maybe best is to do that in asm ? The problem when you are in C code is
that you may already have hit stale cache entries. "raw" platforms
probably need to perform various CPU-specific initializations anyway,
prior to entering C code, in a custom crt0.S, so that's probably the
best place to do the cache clearing.

Ben.

RE: [PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[Stephen Neuendorffer]

Unfortunately, I think your right... The asm code seems so clean at the
moment, I don't really want be the one to start tarnishing it with a
bunch of little fixups.. :)

Steve

> -----Original Message-----
> From: Benjamin Herrenschmidt [mailto:benh@kernel.crashing.org]=20
> Sent: Thursday, December 13, 2007 4:10 PM
> To: Stephen Neuendorffer
> Cc: grant.likely@secretlab.ca; simekm2@fel.cvut.cz;=20
> jwilliams@itee.uq.edu.au; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH 2/7] [POWERPC] Xilinx: clear data caches.
>=20
>=20
> On Fri, 2007-12-14 at 11:07 +1100, Benjamin Herrenschmidt wrote:
> > On Thu, 2007-12-13 at 15:43 -0800, Stephen Neuendorffer wrote:
> > > This code is needed to boot without a boot loader.
> > >=20
> > > Grant:  I'm not sure where the right place to put this=20
> is.  I'm assuming we'll actually need some boot code that is=20
> not generic?  Also, note that there is a V4FX errata=20
> workaround in arch/ppc/boot/head.S, which probably also needs=20
> to get pulled to powerpc.
> > >=20
> > > Signed-off-by: Stephen Neuendorffer=20
> <stephen.neuendorffer@xilinx.com>
> > > ---
> > >  arch/powerpc/boot/raw-platform.c |   22 ++++++++++++++++++++++
> > >  1 files changed, 22 insertions(+), 0 deletions(-)
> >=20
> > This raw-platform.c file doesn't seem like a good place for=20
> code that is
> > totally platform specific ...
>=20
> Maybe best is to do that in asm ? The problem when you are in=20
> C code is
> that you may already have hit stale cache entries. "raw" platforms
> probably need to perform various CPU-specific initializations anyway,
> prior to entering C code, in a custom crt0.S, so that's probably the
> best place to do the cache clearing.
>=20
> Ben.
>=20
>=20
>=20

Re: [PATCH 2/7] [POWERPC] Xilinx: clear data caches.

[Grant Likely]

On 12/13/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
> Unfortunately, I think your right... The asm code seems so clean at the
> moment, I don't really want be the one to start tarnishing it with a
> bunch of little fixups.. :)

No problem, we can use a little shim that executes this stuff before
going into crt0.S.  I'm working on this right now.

g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.

[PATCH 3/7] [POWERPC] Xilinx: Uartlite: Make console output actually work.

[Stephen Neuendorffer]

From: Grant Likely <grant.likely@secretlab.ca>

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>

Fixed to apply against 2.6.24-rc5, and remove DEBUG information.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 drivers/serial/uartlite.c |  121 +++++++++++++++++++++++++++++----------------
 1 files changed, 79 insertions(+), 42 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index 3f59324..5ec42f3 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -9,6 +9,8 @@
  * kind, whether express or implied.
  */
 
+#undef DEBUG
+
 #include <linux/platform_device.h>
 #include <linux/module.h>
 #include <linux/console.h>
@@ -321,6 +323,49 @@ static struct uart_ops ulite_ops = {
 	.verify_port	= ulite_verify_port
 };
 
+/**
+ * ulite_get_port: Get the uart_port for a given port number and base addr
+ */
+static struct uart_port * ulite_get_port(int id)
+{
+	struct uart_port *port;
+
+	/* if id = -1; then scan for a free id and use that */
+	if (id < 0) {
+		for (id = 0; id < ULITE_NR_UARTS; id++)
+			if (ulite_ports[id].mapbase == 0)
+				break;
+	}
+
+	if ((id < 0) || (id >= ULITE_NR_UARTS)) {
+		printk(KERN_WARNING "uartlite: invalid id: %i\n", id);
+		return NULL;
+	}
+
+	/* The ID is valid, so get the address of the uart_port structure */
+	port = &ulite_ports[id];
+
+	/* Is the structure is already initialized? */
+	if (port->mapbase)
+		return port;
+
+	/* At this point, we've got an empty uart_port struct, initialize it */
+	spin_lock_init(&port->lock);
+	port->membase = NULL;
+	port->fifosize = 16;
+	port->regshift = 2;
+	port->iotype = UPIO_MEM;
+	port->iobase = 1; /* mark port in use */
+	port->ops = &ulite_ops;
+	port->irq = NO_IRQ;
+	port->flags = UPF_BOOT_AUTOCONF;
+	port->dev = NULL;
+	port->type = PORT_UNKNOWN;
+	port->line = id;
+
+	return port;
+}
+
 /* ---------------------------------------------------------------------
  * Console driver operations
  */
@@ -376,7 +421,7 @@ static void ulite_console_write(struct console *co, const char *s,
 }
 
 #if defined(CONFIG_OF)
-static inline void __init ulite_console_of_find_device(int id)
+static inline u32 __init ulite_console_of_find_device(int id)
 {
 	struct device_node *np;
 	struct resource res;
@@ -392,13 +437,14 @@ static inline void __init ulite_console_of_find_device(int id)
 		if (rc)
 			continue;
 
-		ulite_ports[id].mapbase = res.start;
 		of_node_put(np);
-		return;
+		return res.start+3;
 	}
+
+	return 0;
 }
 #else /* CONFIG_OF */
-static inline void __init ulite_console_of_find_device(int id) { /* do nothing */ }
+static inline u32 __init ulite_console_of_find_device(int id) { return 0; }
 #endif /* CONFIG_OF */
 
 static int __init ulite_console_setup(struct console *co, char *options)
@@ -408,25 +454,33 @@ static int __init ulite_console_setup(struct console *co, char *options)
 	int bits = 8;
 	int parity = 'n';
 	int flow = 'n';
+	u32 base;
 
-	if (co->index < 0 || co->index >= ULITE_NR_UARTS)
-		return -EINVAL;
+	/* Find a matching uart port in the device tree */
+	base = ulite_console_of_find_device(co->index);
 
-	port = &ulite_ports[co->index];
+	/* Get the port structure */
+	port = ulite_get_port(co->index);
+	if (!port)
+		return -ENODEV;
 
-	/* Check if it is an OF device */
-	if (!port->mapbase)
-		ulite_console_of_find_device(co->index);
+	/* was it initialized for this device? */
+	if (base) {
+		if ((port->mapbase) && (port->mapbase != base)) {
+			pr_debug(KERN_DEBUG "ulite: addr mismatch; %x != %x\n",
+				 port->mapbase, base);
+			return -ENODEV; /* port used by another device; bail */
+		}
+		port->mapbase = base;
+	}
 
-	/* Do we have a device now? */
-	if (!port->mapbase) {
-		pr_debug("console on ttyUL%i not present\n", co->index);
+	if (!port->mapbase)
 		return -ENODEV;
-	}
 
-	/* not initialized yet? */
+	/* registers mapped yet? */
 	if (!port->membase) {
-		if (ulite_request_port(port))
+		port->membase = ioremap(port->mapbase, ULITE_REGION);
+		if (!port->membase)
 			return -ENODEV;
 	}
 
@@ -488,39 +542,22 @@ static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 	struct uart_port *port;
 	int rc;
 
-	/* if id = -1; then scan for a free id and use that */
-	if (id < 0) {
-		for (id = 0; id < ULITE_NR_UARTS; id++)
-			if (ulite_ports[id].mapbase == 0)
-				break;
-	}
-	if (id < 0 || id >= ULITE_NR_UARTS) {
-		dev_err(dev, "%s%i too large\n", ULITE_NAME, id);
-		return -EINVAL;
+	port = ulite_get_port(id);
+	if (!port) {
+		dev_err(dev, "Cannot get uart_port structure\n");
+		return -ENODEV;
 	}
 
-	if ((ulite_ports[id].mapbase) && (ulite_ports[id].mapbase != base)) {
-		dev_err(dev, "cannot assign to %s%i; it is already in use\n",
-			ULITE_NAME, id);
-		return -EBUSY;
+	/* was it initialized for this device? */
+	if ((port->mapbase) && (port->mapbase != base)) {
+		pr_debug(KERN_DEBUG "ulite: addr mismatch; %x != %x\n",
+			 port->mapbase, base);
+		return -ENODEV;
 	}
 
-	port = &ulite_ports[id];
-
-	spin_lock_init(&port->lock);
-	port->fifosize = 16;
-	port->regshift = 2;
-	port->iotype = UPIO_MEM;
-	port->iobase = 1; /* mark port in use */
 	port->mapbase = base;
-	port->membase = NULL;
-	port->ops = &ulite_ops;
 	port->irq = irq;
-	port->flags = UPF_BOOT_AUTOCONF;
 	port->dev = dev;
-	port->type = PORT_UNKNOWN;
-	port->line = id;
-
 	dev_set_drvdata(dev, port);
 
 	/* Register the port */
-- 
1.5.3.4

[PATCH 4/7] [POWERPC] Xilinx: update compatible list for interrupt controller

[Stephen Neuendorffer]

These values now match what is generated by the uboot BSP generator.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/sysdev/xilinx_intc.c |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/sysdev/xilinx_intc.c b/arch/powerpc/sysdev/xilinx_intc.c
index c2f17cc..10345dd 100644
--- a/arch/powerpc/sysdev/xilinx_intc.c
+++ b/arch/powerpc/sysdev/xilinx_intc.c
@@ -135,10 +135,16 @@ void __init xilinx_intc_init_tree(void)
 	struct device_node *np;
 
 	/* find top level interrupt controller */
-	for_each_compatible_node(np, NULL, "xilinx,intc") {
+	for_each_compatible_node(np, NULL, "xlnx,opb-intc-1.00.c") {
 		if (!of_get_property(np, "interrupts", NULL))
 			break;
 	}
+        if(!np) {
+		for_each_compatible_node(np, NULL, "xlnx,xps-intc-1.00.a") {
+			if (!of_get_property(np, "interrupts", NULL))
+				break;
+		}
+        }
 
 	/* xilinx interrupt controller needs to be top level */
 	BUG_ON(!np);
-- 
1.5.3.4

[PATCH 5/7] [POWERPC] Xilinx: Update compatible to use values generated by BSP generator.

[Stephen Neuendorffer]

Mainly, this involves two changes:
1) xilinx->xlnx (recognized standard is to use the stock ticker)
2) In order to have the device tree focus on describing what the hardware is as exactly as possible, the compatible strings contain the full IP name and IP version.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/virtex.c |    2 +-
 drivers/block/xsysace.c             |    4 ++-
 drivers/serial/uartlite.c           |   42 +++++++++++++++++++++-------------
 drivers/video/xilinxfb.c            |    2 +-
 4 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/arch/powerpc/platforms/40x/virtex.c b/arch/powerpc/platforms/40x/virtex.c
index 14bbc32..859ba1d 100644
--- a/arch/powerpc/platforms/40x/virtex.c
+++ b/arch/powerpc/platforms/40x/virtex.c
@@ -30,7 +30,7 @@ static int __init virtex_probe(void)
 {
 	unsigned long root = of_get_flat_dt_root();
 
-	if (!of_flat_dt_is_compatible(root, "xilinx,virtex"))
+	if (!of_flat_dt_is_compatible(root, "xlnx,virtex"))
 		return 0;
 
 	return 1;
diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 82effce..45bc51b 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -1208,7 +1208,9 @@ static int __devexit ace_of_remove(struct of_device *op)
 
 /* Match table for of_platform binding */
 static struct of_device_id __devinit ace_of_match[] = {
-	{ .compatible = "xilinx,xsysace", },
+	{ .compatible = "xlnx,opb-sysace-1.00.b", },
+	{ .compatible = "xlnx,opb-sysace-1.00.c", },
+	{ .compatible = "xlnx,xps-sysace-1.00.a", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, ace_of_match);
diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index 5ec42f3..6536cc7 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -21,8 +21,18 @@
 #include <linux/interrupt.h>
 #include <asm/io.h>
 #if defined(CONFIG_OF)
+#include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/of_platform.h>
+
+/* Match table for of_platform binding */
+static struct of_device_id __devinit ulite_of_match[] = {
+	{ .type = "serial", .compatible = "xlnx,opb-uartlite-1.00.b", },
+	{ .type = "serial", .compatible = "xlnx,xps-uartlite-1.00.a", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ulite_of_match);
+
 #endif
 
 #define ULITE_NAME		"ttyUL"
@@ -427,18 +437,25 @@ static inline u32 __init ulite_console_of_find_device(int id)
 	struct resource res;
 	const unsigned int *of_id;
 	int rc;
+        const struct of_device_id *matches = ulite_of_match;
+
+	while (matches->compatible[0]) {
+		for_each_compatible_node(np, NULL, matches->compatible) {
+			if (!of_match_node(matches, np))
+				continue;
 
-	for_each_compatible_node(np, NULL, "xilinx,uartlite") {
-		of_id = of_get_property(np, "port-number", NULL);
-		if ((!of_id) || (*of_id != id))
-			continue;
+			of_id = of_get_property(np, "port-number", NULL);
+			if ((!of_id) || (*of_id != id))
+				continue;
 
-		rc = of_address_to_resource(np, 0, &res);
-		if (rc)
-			continue;
+			rc = of_address_to_resource(np, 0, &res);
+			if (rc)
+				continue;
 
-		of_node_put(np);
-		return res.start+3;
+                        of_node_put(np);
+                        return res.start+3;
+		}
+		matches++;
 	}
 
 	return 0;
@@ -654,13 +671,6 @@ static int __devexit ulite_of_remove(struct of_device *op)
 	return ulite_release(&op->dev);
 }
 
-/* Match table for of_platform binding */
-static struct of_device_id __devinit ulite_of_match[] = {
-	{ .type = "serial", .compatible = "xilinx,uartlite", },
-	{},
-};
-MODULE_DEVICE_TABLE(of, ulite_of_match);
-
 static struct of_platform_driver ulite_of_driver = {
 	.owner = THIS_MODULE,
 	.name = "uartlite",
diff --git a/drivers/video/xilinxfb.c b/drivers/video/xilinxfb.c
index e38d3b7..9b426d3 100644
--- a/drivers/video/xilinxfb.c
+++ b/drivers/video/xilinxfb.c
@@ -460,7 +460,7 @@ static int __devexit xilinxfb_of_remove(struct of_device *op)
 
 /* Match table for of_platform binding */
 static struct of_device_id __devinit xilinxfb_of_match[] = {
-	{ .compatible = "xilinx,ml300-fb", },
+	{ .compatible = "xlnx,plb-tft-cntlr-ref-1.00.a", },
 	{},
 };
 MODULE_DEVICE_TABLE(of, xilinxfb_of_match);
-- 
1.5.3.4

[PATCH 6/7] [POWERPC] Xilinx: Add correct compatible list for device tree bus bindings.

[Stephen Neuendorffer]

Includes both flavors of plb, opb, dcr, and a pseudo 'compound' bus
for representing compound peripherals containing more than one logical
device.

Signed-off-by: Stephen Neuendorffer <stephen.neuendorffer@xilinx.com>
---
 arch/powerpc/platforms/40x/virtex.c |   12 +++++++++++-
 1 files changed, 11 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/platforms/40x/virtex.c b/arch/powerpc/platforms/40x/virtex.c
index 859ba1d..7590fa5 100644
--- a/arch/powerpc/platforms/40x/virtex.c
+++ b/arch/powerpc/platforms/40x/virtex.c
@@ -15,12 +15,22 @@
 #include <asm/time.h>
 #include <asm/xilinx_intc.h>
 
+static struct of_device_id xilinx_of_bus_ids[] = {
+	{ .compatible = "xlnx,plb-v46-1.00.a", },
+	{ .compatible = "xlnx,plb-v34-1.01.a", },
+	{ .compatible = "xlnx,plb-v34-1.02.a", },
+	{ .compatible = "xlnx,opb-v20-1.10.c", },
+	{ .compatible = "xlnx,dcr-v29-1.00.a", },
+	{ .compatible = "xlnx,compound", },
+	{},
+};
+
 static int __init virtex_device_probe(void)
 {
 	if (!machine_is(virtex))
 		return 0;
 
-	of_platform_bus_probe(NULL, NULL, NULL);
+	of_platform_bus_probe(NULL, xilinx_of_bus_ids, NULL);
 
 	return 0;
 }
-- 
1.5.3.4

[PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Stephen Neuendorffer]

This now better describes what the UBoot device tree generator actually does.  In particular:

1) Nodes have a label derived from the device name, and a node name
derived from the device type.
2) Usage of compound nodes (representing more than one device in the same IP) which actually works.  This requires having a valid compatible node, and all the other things that a bus normally has.  I've chosen 'xlnx,compound' as the bus name to describe these compound nodes.
3) Uartlite requires a port-number property for the console to work.

In addition, I've clarified some of the language relating to how mhs
nodes should be represent in the device tree.
---
 Documentation/powerpc/booting-without-of.txt |   61 +++++++++++++++-----------
 1 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index e9a3cb1..5e2b85a 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2276,7 +2276,7 @@ platforms are moved over to use the flattened-device-tree model.
    properties of the device node.  In general, device nodes for IP-cores
    will take the following form:
 
-	(name)@(base-address) {
+	(name): (ip-core-name)@(base-address) {
 		compatible = "xlnx,(ip-core-name)-(HW_VER)"
 			     [, (list of compatible devices), ...];
 		reg = <(baseaddr) (size)>;
@@ -2294,9 +2294,9 @@ platforms are moved over to use the flattened-device-tree model.
 			dropped from the parameter name, the name is converted
 			to lowercase and all underscore '_' characters are
 			converted to dashes '-'.
-	(baseaddr):	the C_BASEADDR parameter.
+	(baseaddr):	the baseaddr parameter value (often named C_BASEADDR).
 	(HW_VER):	from the HW_VER parameter.
-	(size):		equals C_HIGHADDR - C_BASEADDR + 1
+	(size):		the address range size (often C_HIGHADDR - C_BASEADDR + 1).
 
    Typically, the compatible list will include the exact IP core version
    followed by an older IP core version which implements the same
@@ -2326,12 +2326,13 @@ platforms are moved over to use the flattened-device-tree model.
 
    becomes the following device tree node:
 
-	opb-uartlite-0@ec100000 {
+	opb_uartlite_0: serial@ec100000 {
 		device_type = "serial";
 		compatible = "xlnx,opb-uartlite-1.00.b";
 		reg = <ec100000 10000>;
-		interrupt-parent = <&opb-intc>;
+		interrupt-parent = <&opb_intc_0>;
 		interrupts = <1 0>; // got this from the opb_intc parameters
+		port-number = <0>;
 		current-speed = <d#115200>;	// standard serial device prop
 		clock-frequency = <d#50000000>;	// standard serial device prop
 		xlnx,data-bits = <8>;
@@ -2339,16 +2340,19 @@ platforms are moved over to use the flattened-device-tree model.
 		xlnx,use-parity = <0>;
 	};
 
-   Some IP cores actually implement 2 or more logical devices.  In this case,
-   the device should still describe the whole IP core with a single node
-   and add a child node for each logical device.  The ranges property can
-   be used to translate from parent IP-core to the registers of each device.
-   (Note: this makes the assumption that both logical devices have the same
-   bus binding.  If this is not true, then separate nodes should be used for
-   each logical device).  The 'cell-index' property can be used to enumerate
-   logical devices within an IP core.  For example, the following is the
-   system.mhs entry for the dual ps2 controller found on the ml403 reference
-   design.
+   Some IP cores actually implement 2 or more logical devices.  In
+   this case, the device should still describe the whole IP core with
+   a single node and add a child node for each logical device.  The
+   ranges property can be used to translate from parent IP-core to the
+   registers of each device.  In addition, the parent node should be
+   compatible with the bus type 'xlnx,compound', and should contain
+   #address-cells and #size-cells, as with any other bus.  (Note: this
+   makes the assumption that both logical devices have the same bus
+   binding.  If this is not true, then separate nodes should be used
+   for each logical device).  The 'cell-index' property can be used to
+   enumerate logical devices within an IP core.  For example, the
+   following is the system.mhs entry for the dual ps2 controller found
+   on the ml403 reference design.
 
 	BEGIN opb_ps2_dual_ref
 		PARAMETER INSTANCE = opb_ps2_dual_ref_0
@@ -2370,21 +2374,24 @@ platforms are moved over to use the flattened-device-tree model.
 
    It would result in the following device tree nodes:
 
-	opb_ps2_dual_ref_0@a9000000 {
+	opb_ps2_dual_ref_0: opb-ps2-dual-ref@a9000000 {
+		#address-cells = <1>;
+		#size-cells = <1>;
+		compatible = "xlnx,compound";
 		ranges = <0 a9000000 2000>;
 		// If this device had extra parameters, then they would
 		// go here.
-		ps2@0 {
+		opb-ps2-dual-ref@0 {
 			compatible = "xlnx,opb-ps2-dual-ref-1.00.a";
 			reg = <0 40>;
-			interrupt-parent = <&opb-intc>;
+			interrupt-parent = <&opb_intc_0>;
 			interrupts = <3 0>;
 			cell-index = <0>;
 		};
-		ps2@1000 {
+		opb-ps2-dual-ref@1000 {
 			compatible = "xlnx,opb-ps2-dual-ref-1.00.a";
 			reg = <1000 40>;
-			interrupt-parent = <&opb-intc>;
+			interrupt-parent = <&opb_intc_0>;
 			interrupts = <3 0>;
 			cell-index = <0>;
 		};
@@ -2447,17 +2454,18 @@ platforms are moved over to use the flattened-device-tree model.
 
    Gives this device tree (some properties removed for clarity):
 
-	plb-v34-0 {
+	plb_v34 {
 		#address-cells = <1>;
 		#size-cells = <1>;
+		compatible = "xlnx,plb-v34-1.02.a";
 		device_type = "ibm,plb";
 		ranges; // 1:1 translation
 
-		plb-bram-if-cntrl-0@ffff0000 {
+		plb_bram_if_cntrl_0: plb-bram-if-cntrl@ffff0000 {
 			reg = <ffff0000 10000>;
 		}
 
-		opb-v20-0 {
+		opb_v20 {
 			#address-cells = <1>;
 			#size-cells = <1>;
 			ranges = <20000000 20000000 20000000
@@ -2465,11 +2473,11 @@ platforms are moved over to use the flattened-device-tree model.
 				  80000000 80000000 40000000
 				  c0000000 c0000000 20000000>;
 
-			opb-uart16550-0@a0000000 {
+			opb_uart16550_0: opb-uart16550@a0000000 {
 				reg = <a00000000 2000>;
 			};
 
-			opb-intc-0@d1000fc0 {
+			opb_intc_0: opb-intc@d1000fc0 {
 				reg = <d1000fc0 20>;
 			};
 		};
@@ -2513,6 +2521,9 @@ platforms are moved over to use the flattened-device-tree model.
 
       Requred properties:
        - current-speed : Baud rate of uartlite
+		Optional properties:
+       - port-number : unique ordinal index of the device. This
+         property is required for a console on uartlite.
 
    More devices will be defined as this spec matures.
 
-- 
1.5.3.4

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[David Gibson]

On Thu, Dec 13, 2007 at 03:43:33PM -0800, Stephen Neuendorffer wrote:
> This now better describes what the UBoot device tree generator actually does.  In particular:
> 
> 1) Nodes have a label derived from the device name, and a node name
> derived from the device type.
> 2) Usage of compound nodes (representing more than one device in the same IP) which actually works.  This requires having a valid compatible node, and all the other things that a bus normally has.  I've chosen 'xlnx,compound' as the bus name to describe these compound nodes.

I'm not sure I like this xlnx,compound business, although maybe it's
the best you can do.

> 3) Uartlite requires a port-number property for the console to work.

Why?  In general we try to avoid magical sequence numbers - cell-index
should *only* be used when it's needed to index or program some shared
register.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Grant Likely]

On 12/16/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Thu, Dec 13, 2007 at 03:43:33PM -0800, Stephen Neuendorffer wrote:
> > This now better describes what the UBoot device tree generator actually does.  In particular:
> >
> > 1) Nodes have a label derived from the device name, and a node name
> > derived from the device type.
> > 2) Usage of compound nodes (representing more than one device in the same IP) which actually works.  This requires having a valid compatible node, and all the other things that a bus normally has.  I've chosen 'xlnx,compound' as the bus name to describe these compound nodes.
>
> I'm not sure I like this xlnx,compound business, although maybe it's
> the best you can do.

I'd prefer something like "xlnx,<ip-name>-group".  "xlnx,compound" is
a very bad idea because it will be reused for very different types of
devices.  What happens when a device appears that has both
per-instance properties and 'top level' registers accessed by both
devices?

>
> > 3) Uartlite requires a port-number property for the console to work.
>
> Why?  In general we try to avoid magical sequence numbers - cell-index
> should *only* be used when it's needed to index or program some shared
> register.

Aliases is probably the correct construct for this.  port-number was a
bad idea and I never should have gone that direction.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Stephen Neuendorffer]

[-- Attachment #1: Type: text/plain, Size: 1659 bytes --]



Compound devices that actually have their own shared registers should do something different.  Likely they'll need some sort of data synchronization anyway.  It seems that by far the common case here is to going to be to just have aggregation, and I don't see any reason that can't be handled by a common 'pseudo-bus'.


Steve

-----Original Message-----
From: glikely@secretlab.ca on behalf of Grant Likely
Sent: Sun 12/16/2007 8:30 PM
To: Stephen Neuendorffer; grant.likely@secretlab.ca; simekm2@fel.cvut.cz; jwilliams@itee.uq.edu.au; linuxppc-dev@ozlabs.org
Subject: Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.
 
On 12/16/07, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Thu, Dec 13, 2007 at 03:43:33PM -0800, Stephen Neuendorffer wrote:
> > This now better describes what the UBoot device tree generator actually does.  In particular:
> >
> > 1) Nodes have a label derived from the device name, and a node name
> > derived from the device type.
> > 2) Usage of compound nodes (representing more than one device in the same IP) which actually works.  This requires having a valid compatible node, and all the other things that a bus normally has.  I've chosen 'xlnx,compound' as the bus name to describe these compound nodes.
>
> I'm not sure I like this xlnx,compound business, although maybe it's
> the best you can do.

I'd prefer something like "xlnx,<ip-name>-group".  "xlnx,compound" is
a very bad idea because it will be reused for very different types of
devices.  What happens when a device appears that has both
per-instance properties and 'top level' registers accessed by both
devices?

[-- Attachment #2: Type: text/html, Size: 2286 bytes --]

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Peter Korsgaard]

>>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:

Hi,

 >> > 3) Uartlite requires a port-number property for the console to work.
 >> 
 >> Why?  In general we try to avoid magical sequence numbers - cell-index
 >> should *only* be used when it's needed to index or program some shared
 >> register.

 Grant> Aliases is probably the correct construct for this.
 Grant> port-number was a bad idea and I never should have gone that
 Grant> direction.

Perhaps it would make more sense to add a _find_match_or_unused(port)
like 8250.c and make the initialization order define the index instead
of explicitly setting it?

Now we're at it - Why isn't the uartlite of glue handled by of_serial.c?

-- 
Bye, Peter Korsgaard

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Grant Likely]

On 12/17/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>
> Hi,
>
>  >> > 3) Uartlite requires a port-number property for the console to work.
>  >>
>  >> Why?  In general we try to avoid magical sequence numbers - cell-index
>  >> should *only* be used when it's needed to index or program some shared
>  >> register.
>
>  Grant> Aliases is probably the correct construct for this.
>  Grant> port-number was a bad idea and I never should have gone that
>  Grant> direction.
>
> Perhaps it would make more sense to add a _find_match_or_unused(port)
> like 8250.c and make the initialization order define the index instead
> of explicitly setting it?

aliases I think is the better approach as it is an already established
convention.

enumeration based on init order is fragile and there is no guarantee
that order will not change in the future.

>
> Now we're at it - Why isn't the uartlite of glue handled by of_serial.c?

The of_serial.c glue is IMHO too complex for the uartlite case.  I saw
no advantage provided by the added complexity.  (ulite_of_probe() is
all of 12 lines of code).

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Grant Likely]

On 12/13/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
> This now better describes what the UBoot device tree generator actually does.  In particular:
>
> 1) Nodes have a label derived from the device name, and a node name
> derived from the device type.
> 2) Usage of compound nodes (representing more than one device in the same IP) which actually works.  This requires having a valid compatible node, and all the other things that a bus normally has.  I've chosen 'xlnx,compound' as the bus name to describe these compound nodes.
> 3) Uartlite requires a port-number property for the console to work.
>
> In addition, I've clarified some of the language relating to how mhs
> nodes should be represent in the device tree.

Thanks for the updates.  Comments below.

> ---
>  Documentation/powerpc/booting-without-of.txt |   61 +++++++++++++++-----------
>  1 files changed, 36 insertions(+), 25 deletions(-)
>
> diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
> index e9a3cb1..5e2b85a 100644
> --- a/Documentation/powerpc/booting-without-of.txt
> +++ b/Documentation/powerpc/booting-without-of.txt
> @@ -2276,7 +2276,7 @@ platforms are moved over to use the flattened-device-tree model.
>     properties of the device node.  In general, device nodes for IP-cores
>     will take the following form:
>
> -       (name)@(base-address) {
> +       (name): (ip-core-name)@(base-address) {

(name): (generic-name)@(base-address) {

>                 compatible = "xlnx,(ip-core-name)-(HW_VER)"
>                              [, (list of compatible devices), ...];
>                 reg = <(baseaddr) (size)>;
> @@ -2294,9 +2294,9 @@ platforms are moved over to use the flattened-device-tree model.
>                         dropped from the parameter name, the name is converted
>                         to lowercase and all underscore '_' characters are
>                         converted to dashes '-'.
> -       (baseaddr):     the C_BASEADDR parameter.
> +       (baseaddr):     the baseaddr parameter value (often named C_BASEADDR).
>         (HW_VER):       from the HW_VER parameter.
> -       (size):         equals C_HIGHADDR - C_BASEADDR + 1
> +       (size):         the address range size (often C_HIGHADDR - C_BASEADDR + 1).

Ah, yes.  Good clarification.

>
>     Typically, the compatible list will include the exact IP core version
>     followed by an older IP core version which implements the same
> @@ -2326,12 +2326,13 @@ platforms are moved over to use the flattened-device-tree model.
>
>     becomes the following device tree node:
>
> -       opb-uartlite-0@ec100000 {
> +       opb_uartlite_0: serial@ec100000 {
>                 device_type = "serial";
>                 compatible = "xlnx,opb-uartlite-1.00.b";
>                 reg = <ec100000 10000>;
> -               interrupt-parent = <&opb-intc>;
> +               interrupt-parent = <&opb_intc_0>;
>                 interrupts = <1 0>; // got this from the opb_intc parameters
> +               port-number = <0>;
>                 current-speed = <d#115200>;     // standard serial device prop
>                 clock-frequency = <d#50000000>; // standard serial device prop
>                 xlnx,data-bits = <8>;
> @@ -2339,16 +2340,19 @@ platforms are moved over to use the flattened-device-tree model.
>                 xlnx,use-parity = <0>;
>         };
>
> -   Some IP cores actually implement 2 or more logical devices.  In this case,
> -   the device should still describe the whole IP core with a single node
> -   and add a child node for each logical device.  The ranges property can
> -   be used to translate from parent IP-core to the registers of each device.
> -   (Note: this makes the assumption that both logical devices have the same
> -   bus binding.  If this is not true, then separate nodes should be used for
> -   each logical device).  The 'cell-index' property can be used to enumerate
> -   logical devices within an IP core.  For example, the following is the
> -   system.mhs entry for the dual ps2 controller found on the ml403 reference
> -   design.
> +   Some IP cores actually implement 2 or more logical devices.  In
> +   this case, the device should still describe the whole IP core with
> +   a single node and add a child node for each logical device.  The
> +   ranges property can be used to translate from parent IP-core to the
> +   registers of each device.  In addition, the parent node should be
> +   compatible with the bus type 'xlnx,compound', and should contain
> +   #address-cells and #size-cells, as with any other bus.  (Note: this
> +   makes the assumption that both logical devices have the same bus
> +   binding.  If this is not true, then separate nodes should be used
> +   for each logical device).  The 'cell-index' property can be used to
> +   enumerate logical devices within an IP core.  For example, the
> +   following is the system.mhs entry for the dual ps2 controller found
> +   on the ml403 reference design.
>
>         BEGIN opb_ps2_dual_ref
>                 PARAMETER INSTANCE = opb_ps2_dual_ref_0
> @@ -2370,21 +2374,24 @@ platforms are moved over to use the flattened-device-tree model.
>
>     It would result in the following device tree nodes:
>
> -       opb_ps2_dual_ref_0@a9000000 {
> +       opb_ps2_dual_ref_0: opb-ps2-dual-ref@a9000000 {
> +               #address-cells = <1>;
> +               #size-cells = <1>;
> +               compatible = "xlnx,compound";
>                 ranges = <0 a9000000 2000>;
>                 // If this device had extra parameters, then they would
>                 // go here.
> -               ps2@0 {
> +               opb-ps2-dual-ref@0 {

According to the generic names recommendation, this should be either
"keyboard@0" or "mouse@0", but of course the two interfaces are
identical and EDK doesn't have any information about how they are
used.  Perhaps the node name should be: "ps2@0".  David, thoughts?

>                         compatible = "xlnx,opb-ps2-dual-ref-1.00.a";
>                         reg = <0 40>;
> -                       interrupt-parent = <&opb-intc>;
> +                       interrupt-parent = <&opb_intc_0>;
>                         interrupts = <3 0>;
>                         cell-index = <0>;
>                 };
> -               ps2@1000 {
> +               opb-ps2-dual-ref@1000 {
>                         compatible = "xlnx,opb-ps2-dual-ref-1.00.a";
>                         reg = <1000 40>;
> -                       interrupt-parent = <&opb-intc>;
> +                       interrupt-parent = <&opb_intc_0>;
>                         interrupts = <3 0>;
>                         cell-index = <0>;
>                 };
> @@ -2447,17 +2454,18 @@ platforms are moved over to use the flattened-device-tree model.
>
>     Gives this device tree (some properties removed for clarity):
>
> -       plb-v34-0 {
> +       plb_v34 {

Steve, when I wrote this I was lazy and I didn't add the bus address.
However, if we don't have the base address I think we'll end up with
name collisions (especially in the MPMC case).  So, based on generic
names convention, this should probably simply be "plb@<baseaddr>".

>                 #address-cells = <1>;
>                 #size-cells = <1>;
> +               compatible = "xlnx,plb-v34-1.02.a";
>                 device_type = "ibm,plb";
>                 ranges; // 1:1 translation
>
> -               plb-bram-if-cntrl-0@ffff0000 {
> +               plb_bram_if_cntrl_0: plb-bram-if-cntrl@ffff0000 {

Node name should probably just be "bram@ffff0000" here.

>                         reg = <ffff0000 10000>;
>                 }
>
> -               opb-v20-0 {
> +               opb_v20 {

opb@<baseaddr>

>                         #address-cells = <1>;
>                         #size-cells = <1>;
>                         ranges = <20000000 20000000 20000000
> @@ -2465,11 +2473,11 @@ platforms are moved over to use the flattened-device-tree model.
>                                   80000000 80000000 40000000
>                                   c0000000 c0000000 20000000>;
>
> -                       opb-uart16550-0@a0000000 {
> +                       opb_uart16550_0: opb-uart16550@a0000000 {

serial@a0000000

>                                 reg = <a00000000 2000>;
>                         };
>
> -                       opb-intc-0@d1000fc0 {
> +                       opb_intc_0: opb-intc@d1000fc0 {

interrupt-controller@d1000fc0

>                                 reg = <d1000fc0 20>;
>                         };
>                 };
> @@ -2513,6 +2521,9 @@ platforms are moved over to use the flattened-device-tree model.
>
>        Requred properties:
>         - current-speed : Baud rate of uartlite
> +               Optional properties:
> +       - port-number : unique ordinal index of the device. This
> +         property is required for a console on uartlite.

And has already been discussed, drop the port-number property.  I'll
rework the uartlite driver to use aliases instead.

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Stephen Neuendorffer]

[-- Attachment #1: Type: text/plain, Size: 1882 bytes --]

When the driver no longer requires the port number, it's easy to drop.  Until then, I'll keep it in.
 
Also, I'm not so sure that moving to completely generic names is really worth the effort...  All the 'semantically' interesting' information is already in the device tree somewhere else.  In the limit, the node name could just be a randomly generated string.  So now we have a matter of taste: what is the right amount of detail to put in so that someone who looks at the tree can easily understand what's going on, but not be overwhelmed?  The xilinx ip name seems to usually do that almost as well as a 'generic name'.  Anyway, you proved me wrong last time after a bunch of mulling it over, so maybe I'll just take your word for it and do it that way. :)
 
In other news, my computer seems to have died this morning, so productivity may be low. :)
 
Steve

________________________________

From: glikely@secretlab.ca on behalf of Grant Likely
Sent: Mon 12/17/2007 7:19 AM
To: Stephen Neuendorffer
Cc: simekm2@fel.cvut.cz; jwilliams@itee.uq.edu.au; linuxppc-dev@ozlabs.org; David Gibson
Subject: Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

>                                 reg = <d1000fc0 20>;
>                         };
>                 };
> @@ -2513,6 +2521,9 @@ platforms are moved over to use the flattened-device-tree model.
>
>        Requred properties:
>         - current-speed : Baud rate of uartlite
> +               Optional properties:
> +       - port-number : unique ordinal index of the device. This
> +         property is required for a console on uartlite.

And has already been discussed, drop the port-number property.  I'll
rework the uartlite driver to use aliases instead.

Cheers,
g.

--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195



[-- Attachment #2: Type: text/html, Size: 3445 bytes --]

Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Grant Likely]

On 12/17/07, Stephen Neuendorffer <stephen.neuendorffer@xilinx.com> wrote:
>
> When the driver no longer requires the port number, it's easy to drop.
> Until then, I'll keep it in.
>
> Also, I'm not so sure that moving to completely generic names is really
> worth the effort...  All the 'semantically' interesting' information is
> already in the device tree somewhere else.  In the limit, the node name
> could just be a randomly generated string.  So now we have a matter of
> taste: what is the right amount of detail to put in so that someone who
> looks at the tree can easily understand what's going on, but not be
> overwhelmed?

True, but part of 'taste' is following the established OF conventions
such as the generic names  ( http://playground.sun.com/1275/practice/
).  Those conventions come directly from the lessons learned by real
open firmware over the years.  It's okay to break convention; but only
if you've got a *damn* *good* reason for doing so.  :-)

>  The xilinx ip name seems to usually do that almost as well as
> a 'generic name'.  Anyway, you proved me wrong last time after a bunch of
> mulling it over, so maybe I'll just take your word for it and do it that
> way. :)

heh,   And a big reason I'm arguing it is David, Segher and others
took me to task for making the same mistakes.  :-)

> In other news, my computer seems to have died this morning, so productivity
> may be low. :)

Fun.  :-(

Cheers,
g.

-- 
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
grant.likely@secretlab.ca
(403) 399-0195

RE: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.

[Stephen Neuendorffer]

I've updated the generator based on the below feedback.  I'll also send
around the updated patch for this section briefly.  Comments below.

> -----Original Message-----
> From: glikely@secretlab.ca [mailto:glikely@secretlab.ca] On Behalf Of
> Grant Likely
> Sent: Monday, December 17, 2007 7:20 AM
> To: Stephen Neuendorffer
> Cc: simekm2@fel.cvut.cz; jwilliams@itee.uq.edu.au; linuxppc-
> dev@ozlabs.org; David Gibson
> Subject: Re: [PATCH 7/7] [POWERPC] Xilinx: Update booting-without-of.
>=20
> > -       (name)@(base-address) {
> > +       (name): (ip-core-name)@(base-address) {
>=20
> (name): (generic-name)@(base-address) {

In most cases it now does this.  In the default case for ip which is not
recognized, it falls back to the ip-core-name.

> > -       opb_ps2_dual_ref_0@a9000000 {
> > +       opb_ps2_dual_ref_0: opb-ps2-dual-ref@a9000000 {
> > +               #address-cells =3D <1>;
> > +               #size-cells =3D <1>;
> > +               compatible =3D "xlnx,compound";
> >                 ranges =3D <0 a9000000 2000>;
> >                 // If this device had extra parameters, then they
would
> >                 // go here.
> > -               ps2@0 {
> > +               opb-ps2-dual-ref@0 {
>=20
> According to the generic names recommendation, this should be either
> "keyboard@0" or "mouse@0", but of course the two interfaces are
> identical and EDK doesn't have any information about how they are
> used.  Perhaps the node name should be: "ps2@0".  David, thoughts?

I don't think keyboard or mouse really makes sense here.  Went with ps2.

> > -       plb-v34-0 {
> > +       plb_v34 {
>=20
> Steve, when I wrote this I was lazy and I didn't add the bus address.
> However, if we don't have the base address I think we'll end up with
> name collisions (especially in the MPMC case).  So, based on generic
> names convention, this should probably simply be "plb@<baseaddr>".

I wondered at one point if this would be a problem... in any event, you
now get the baseaddress.

> >                 #address-cells =3D <1>;
> >                 #size-cells =3D <1>;
> > +               compatible =3D "xlnx,plb-v34-1.02.a";
> >                 device_type =3D "ibm,plb";
> >                 ranges; // 1:1 translation
> >
> > -               plb-bram-if-cntrl-0@ffff0000 {
> > +               plb_bram_if_cntrl_0: plb-bram-if-cntrl@ffff0000 {
>=20
> Node name should probably just be "bram@ffff0000" here.

What actually gets generated is a memory node at the toplevel, which is
currently suppressed, since it appears that having disjoint memory
locations doesn't work.  If you have more than one 'real' memory
controller, such as an plb_emc connected to flash, then you currently
get the warning:

"Warning!: More than one memory found.  Note that most platforms don't
support non-contiguous memory maps!"

So, there is some divergence here, but I'm not too concerned about it
since the way that such memory might get used is still up for
discussion.

Also note that memory nodes which are not at the toplevel don't appear
to be detected.  Is this by intention or omission?

Obviously there is room for improvement here.  Having a some way to
leverage brams, in particular the dummy bram that you have to have at
high memory in order to get the ppc to boot would be nice!

Steve