[PATCH 2 1/7] Uartlite: Fix reg io to access documented register size

[PATCH 2 1/7] Uartlite: Fix reg io to access documented register size

[Grant Likely]

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

The Uartlite data sheet defines the registers as 32 bit wide.  This
patch changes the register access to use 32 bit transfers and eliminates
the magic +3 offset which is currently required to make the device
work.

Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
Acked-by: John Williams <jwilliams@itee.uq.edu.au>
---

 arch/ppc/syslib/virtex_devices.c |    2 +-
 drivers/serial/uartlite.c        |   32 ++++++++++++++++----------------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/arch/ppc/syslib/virtex_devices.c b/arch/ppc/syslib/virtex_devices.c
index ace4ec0..270ad3a 100644
--- a/arch/ppc/syslib/virtex_devices.c
+++ b/arch/ppc/syslib/virtex_devices.c
@@ -28,7 +28,7 @@
 	.num_resources = 2, \
 	.resource = (struct resource[]) { \
 		{ \
-			.start = XPAR_UARTLITE_##num##_BASEADDR + 3, \
+			.start = XPAR_UARTLITE_##num##_BASEADDR, \
 			.end = XPAR_UARTLITE_##num##_HIGHADDR, \
 			.flags = IORESOURCE_MEM, \
 		}, \
diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index f5051cf..59b674a 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -61,7 +61,7 @@ static int ulite_receive(struct uart_port *port, int stat)
 	/* stats */
 	if (stat & ULITE_STATUS_RXVALID) {
 		port->icount.rx++;
-		ch = readb(port->membase + ULITE_RX);
+		ch = in_be32((void*)port->membase + ULITE_RX);
 
 		if (stat & ULITE_STATUS_PARITY)
 			port->icount.parity++;
@@ -106,7 +106,7 @@ static int ulite_transmit(struct uart_port *port, int stat)
 		return 0;
 
 	if (port->x_char) {
-		writeb(port->x_char, port->membase + ULITE_TX);
+		out_be32((void*)port->membase + ULITE_TX, port->x_char);
 		port->x_char = 0;
 		port->icount.tx++;
 		return 1;
@@ -115,7 +115,7 @@ static int ulite_transmit(struct uart_port *port, int stat)
 	if (uart_circ_empty(xmit) || uart_tx_stopped(port))
 		return 0;
 
-	writeb(xmit->buf[xmit->tail], port->membase + ULITE_TX);
+	out_be32((void*)port->membase + ULITE_TX, xmit->buf[xmit->tail]);
 	xmit->tail = (xmit->tail + 1) & (UART_XMIT_SIZE-1);
 	port->icount.tx++;
 
@@ -132,7 +132,7 @@ static irqreturn_t ulite_isr(int irq, void *dev_id)
 	int busy;
 
 	do {
-		int stat = readb(port->membase + ULITE_STATUS);
+		int stat = in_be32((void*)port->membase + ULITE_STATUS);
 		busy  = ulite_receive(port, stat);
 		busy |= ulite_transmit(port, stat);
 	} while (busy);
@@ -148,7 +148,7 @@ static unsigned int ulite_tx_empty(struct uart_port *port)
 	unsigned int ret;
 
 	spin_lock_irqsave(&port->lock, flags);
-	ret = readb(port->membase + ULITE_STATUS);
+	ret = in_be32((void*)port->membase + ULITE_STATUS);
 	spin_unlock_irqrestore(&port->lock, flags);
 
 	return ret & ULITE_STATUS_TXEMPTY ? TIOCSER_TEMT : 0;
@@ -171,7 +171,7 @@ static void ulite_stop_tx(struct uart_port *port)
 
 static void ulite_start_tx(struct uart_port *port)
 {
-	ulite_transmit(port, readb(port->membase + ULITE_STATUS));
+	ulite_transmit(port, in_be32((void*)port->membase + ULITE_STATUS));
 }
 
 static void ulite_stop_rx(struct uart_port *port)
@@ -200,17 +200,17 @@ static int ulite_startup(struct uart_port *port)
 	if (ret)
 		return ret;
 
-	writeb(ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX,
-	       port->membase + ULITE_CONTROL);
-	writeb(ULITE_CONTROL_IE, port->membase + ULITE_CONTROL);
+	out_be32((void*)port->membase + ULITE_CONTROL,
+	         ULITE_CONTROL_RST_RX | ULITE_CONTROL_RST_TX);
+	out_be32((void*)port->membase + ULITE_CONTROL, ULITE_CONTROL_IE);
 
 	return 0;
 }
 
 static void ulite_shutdown(struct uart_port *port)
 {
-	writeb(0, port->membase + ULITE_CONTROL);
-	readb(port->membase + ULITE_CONTROL); /* dummy */
+	out_be32((void*)port->membase + ULITE_CONTROL, 0);
+	in_be32((void*)port->membase + ULITE_CONTROL); /* dummy */
 	free_irq(port->irq, port);
 }
 
@@ -314,7 +314,7 @@ static void ulite_console_wait_tx(struct uart_port *port)
 
 	/* wait up to 10ms for the character(s) to be sent */
 	for (i = 0; i < 10000; i++) {
-		if (readb(port->membase + ULITE_STATUS) & ULITE_STATUS_TXEMPTY)
+		if (in_be32((void*)port->membase + ULITE_STATUS) & ULITE_STATUS_TXEMPTY)
 			break;
 		udelay(1);
 	}
@@ -323,7 +323,7 @@ static void ulite_console_wait_tx(struct uart_port *port)
 static void ulite_console_putchar(struct uart_port *port, int ch)
 {
 	ulite_console_wait_tx(port);
-	writeb(ch, port->membase + ULITE_TX);
+	out_be32((void*)port->membase + ULITE_TX, ch);
 }
 
 static void ulite_console_write(struct console *co, const char *s,
@@ -340,8 +340,8 @@ static void ulite_console_write(struct console *co, const char *s,
 		spin_lock_irqsave(&port->lock, flags);
 
 	/* save and disable interrupt */
-	ier = readb(port->membase + ULITE_STATUS) & ULITE_STATUS_IE;
-	writeb(0, port->membase + ULITE_CONTROL);
+	ier = in_be32((void*)port->membase + ULITE_STATUS) & ULITE_STATUS_IE;
+	out_be32((void*)port->membase + ULITE_CONTROL, 0);
 
 	uart_console_write(port, s, count, ulite_console_putchar);
 
@@ -349,7 +349,7 @@ static void ulite_console_write(struct console *co, const char *s,
 
 	/* restore interrupt state */
 	if (ier)
-		writeb(ULITE_CONTROL_IE, port->membase + ULITE_CONTROL);
+		out_be32((void*)port->membase + ULITE_CONTROL, ULITE_CONTROL_IE);
 
 	if (locked)
 		spin_unlock_irqrestore(&port->lock, flags);

[PATCH 2 2/7] Uartlite: change name of ports to ulite_ports

[Grant Likely]

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

Changed to match naming convention used in the rest of the module

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

 drivers/serial/uartlite.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index 59b674a..ae05a67 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -46,7 +46,7 @@
 #define ULITE_CONTROL_IE	0x10
 
 
-static struct uart_port ports[ULITE_NR_UARTS];
+static struct uart_port ulite_ports[ULITE_NR_UARTS];
 
 static int ulite_receive(struct uart_port *port, int stat)
 {
@@ -329,7 +329,7 @@ static void ulite_console_putchar(struct uart_port *port, int ch)
 static void ulite_console_write(struct console *co, const char *s,
 				unsigned int count)
 {
-	struct uart_port *port = &ports[co->index];
+	struct uart_port *port = &ulite_ports[co->index];
 	unsigned long flags;
 	unsigned int ier;
 	int locked = 1;
@@ -366,7 +366,7 @@ static int __init ulite_console_setup(struct console *co, char *options)
 	if (co->index < 0 || co->index >= ULITE_NR_UARTS)
 		return -EINVAL;
 
-	port = &ports[co->index];
+	port = &ulite_ports[co->index];
 
 	/* not initialized yet? */
 	if (!port->membase)
@@ -420,7 +420,7 @@ static int __devinit ulite_probe(struct platform_device *pdev)
 	if (pdev->id < 0 || pdev->id >= ULITE_NR_UARTS)
 		return -EINVAL;
 
-	if (ports[pdev->id].membase)
+	if (ulite_ports[pdev->id].membase)
 		return -EBUSY;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -431,7 +431,7 @@ static int __devinit ulite_probe(struct platform_device *pdev)
 	if (!res2)
 		return -ENODEV;
 
-	port = &ports[pdev->id];
+	port = &ulite_ports[pdev->id];
 
 	port->fifosize	= 16;
 	port->regshift	= 2;

[PATCH 2 3/7] Uartlite: Add macro for uartlite device name

[Grant Likely]

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

Changed to make the following OF_platform bus binding patch a wee bit cleaner

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

 drivers/serial/uartlite.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index ae05a67..10e0da9 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -18,6 +18,7 @@
 #include <linux/interrupt.h>
 #include <asm/io.h>
 
+#define ULITE_NAME		"ttyUL"
 #define ULITE_MAJOR		204
 #define ULITE_MINOR		187
 #define ULITE_NR_UARTS		4
@@ -381,7 +382,7 @@ static int __init ulite_console_setup(struct console *co, char *options)
 static struct uart_driver ulite_uart_driver;
 
 static struct console ulite_console = {
-	.name	= "ttyUL",
+	.name	= ULITE_NAME,
 	.write	= ulite_console_write,
 	.device	= uart_console_device,
 	.setup	= ulite_console_setup,
@@ -403,7 +404,7 @@ console_initcall(ulite_console_init);
 static struct uart_driver ulite_uart_driver = {
 	.owner		= THIS_MODULE,
 	.driver_name	= "uartlite",
-	.dev_name	= "ttyUL",
+	.dev_name	= ULITE_NAME,
 	.major		= ULITE_MAJOR,
 	.minor		= ULITE_MINOR,
 	.nr		= ULITE_NR_UARTS,

[PATCH 2 4/7] Uartlite: Separate the bus binding from the driver proper

[Grant Likely]

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

Separate the bus binding code from the driver structure allocation code in
preparation for adding the of_platform_bus bindings needed by arch/powerpc

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

 drivers/serial/uartlite.c |   99 ++++++++++++++++++++++++++++++---------------
 1 files changed, 65 insertions(+), 34 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index 10e0da9..c00a627 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -413,59 +413,90 @@ static struct uart_driver ulite_uart_driver = {
 #endif
 };
 
-static int __devinit ulite_probe(struct platform_device *pdev)
+static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 {
-	struct resource *res, *res2;
 	struct uart_port *port;
+	int rc;
 
-	if (pdev->id < 0 || pdev->id >= ULITE_NR_UARTS)
+	/* 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;
+	}
 
-	if (ulite_ports[pdev->id].membase)
+	if (ulite_ports[id].mapbase) {
+		dev_err(dev, "cannot assign to %s%i; it is already in use\n",
+			ULITE_NAME, id);
 		return -EBUSY;
+	}
 
-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	if (!res)
-		return -ENODEV;
+	port = &ulite_ports[id];
 
-	res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
-	if (!res2)
-		return -ENODEV;
+	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 */
+	rc = uart_add_one_port(&ulite_uart_driver, port);
+	if (rc) {
+		dev_err(dev, "uart_add_one_port() failed; err=%i\n", rc);
+		port->mapbase = 0;
+		dev_set_drvdata(dev, NULL);
+		return rc;
+	}
 
-	port = &ulite_ports[pdev->id];
+	return 0;
+}
 
-	port->fifosize	= 16;
-	port->regshift	= 2;
-	port->iotype	= UPIO_MEM;
-	port->iobase	= 1; /* mark port in use */
-	port->mapbase	= res->start;
-	port->membase	= NULL;
-	port->ops	= &ulite_ops;
-	port->irq	= res2->start;
-	port->flags	= UPF_BOOT_AUTOCONF;
-	port->dev	= &pdev->dev;
-	port->type	= PORT_UNKNOWN;
-	port->line	= pdev->id;
+static int __devinit ulite_release(struct device *dev)
+{
+	struct uart_port *port = dev_get_drvdata(dev);
+	int rc = 0;
 
-	uart_add_one_port(&ulite_uart_driver, port);
-	platform_set_drvdata(pdev, port);
+	if (port) {
+		rc = uart_remove_one_port(&ulite_uart_driver, port);
+		dev_set_drvdata(dev, NULL);
+		port->mapbase = 0;
+	}
 
-	return 0;
+	return rc;
 }
 
-static int ulite_remove(struct platform_device *pdev)
+static int __devinit ulite_probe(struct platform_device *pdev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct resource *res, *res2;
 
-	platform_set_drvdata(pdev, NULL);
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
 
-	if (port)
-		uart_remove_one_port(&ulite_uart_driver, port);
+	res2 = platform_get_resource(pdev, IORESOURCE_IRQ, 0);
+	if (!res2)
+		return -ENODEV;
 
-	/* mark port as free */
-	port->membase = NULL;
+	return ulite_assign(&pdev->dev, pdev->id, res->start, res2->start);
+}
 
-	return 0;
+static int ulite_remove(struct platform_device *pdev)
+{
+	return ulite_release(&pdev->dev);
 }
 
 static struct platform_driver ulite_platform_driver = {

[PATCH 2 5/7] Uartlite: Comment block tidy

[Grant Likely]

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

Tidy the comments to split the driver into logical section; the main driver,
the console driver, the platform bus binding, and module initialization
and teardown.

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

 drivers/serial/uartlite.c |   43 ++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index c00a627..ed13b9f 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -23,9 +23,13 @@
 #define ULITE_MINOR		187
 #define ULITE_NR_UARTS		4
 
-/* For register details see datasheet:
-   http://www.xilinx.com/bvdocs/ipcenter/data_sheet/opb_uartlite.pdf
-*/
+/* ---------------------------------------------------------------------
+ * Register definitions
+ *
+ * For register details see datasheet:
+ * http://www.xilinx.com/bvdocs/ipcenter/data_sheet/opb_uartlite.pdf
+ */
+
 #define ULITE_RX		0x00
 #define ULITE_TX		0x04
 #define ULITE_STATUS		0x08
@@ -49,6 +53,10 @@
 
 static struct uart_port ulite_ports[ULITE_NR_UARTS];
 
+/* ---------------------------------------------------------------------
+ * Core UART driver operations
+ */
+
 static int ulite_receive(struct uart_port *port, int stat)
 {
 	struct tty_struct *tty = port->info->tty;
@@ -308,6 +316,10 @@ static struct uart_ops ulite_ops = {
 	.verify_port	= ulite_verify_port
 };
 
+/* ---------------------------------------------------------------------
+ * Console driver operations
+ */
+
 #ifdef CONFIG_SERIAL_UARTLITE_CONSOLE
 static void ulite_console_wait_tx(struct uart_port *port)
 {
@@ -413,6 +425,19 @@ static struct uart_driver ulite_uart_driver = {
 #endif
 };
 
+/* ---------------------------------------------------------------------
+ * Port assignment functions (mapping devices to uart_port structures)
+ */
+
+/** ulite_assign: register a uartlite device with the driver
+ *
+ * @dev: pointer to device structure
+ * @id: requested id number.  Pass -1 for automatic port assignment
+ * @base: base address of uartlite registers
+ * @irq: irq number for uartlite
+ *
+ * Returns: 0 on success, <0 otherwise
+ */
 static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 {
 	struct uart_port *port;
@@ -465,6 +490,10 @@ static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 	return 0;
 }
 
+/** ulite_release: register a uartlite device with the driver
+ *
+ * @dev: pointer to device structure
+ */
 static int __devinit ulite_release(struct device *dev)
 {
 	struct uart_port *port = dev_get_drvdata(dev);
@@ -479,6 +508,10 @@ static int __devinit ulite_release(struct device *dev)
 	return rc;
 }
 
+/* ---------------------------------------------------------------------
+ * Platform bus binding
+ */
+
 static int __devinit ulite_probe(struct platform_device *pdev)
 {
 	struct resource *res, *res2;
@@ -508,6 +541,10 @@ static struct platform_driver ulite_platform_driver = {
 		   },
 };
 
+/* ---------------------------------------------------------------------
+ * Module setup/teardown
+ */
+
 int __init ulite_init(void)
 {
 	int ret;

[PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

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

Add of_platform bus binding so this driver can be used with arch/powerpc

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

 drivers/serial/uartlite.c |  101 +++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index ed13b9f..8752fac 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -1,7 +1,8 @@
 /*
  * uartlite.c: Serial driver for Xilinx uartlite serial controller
  *
- * Peter Korsgaard <jacmet@sunsite.dk>
+ * Copyright (C) 2006 Peter Korsgaard <jacmet@sunsite.dk>
+ * Copyright (C) 2007 Secret Lab Technologies Ltd.
  *
  * This file is licensed under the terms of the GNU General Public License
  * version 2.  This program is licensed "as is" without any warranty of any
@@ -17,6 +18,10 @@
 #include <linux/delay.h>
 #include <linux/interrupt.h>
 #include <asm/io.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#endif
 
 #define ULITE_NAME		"ttyUL"
 #define ULITE_MAJOR		204
@@ -382,8 +387,10 @@ static int __init ulite_console_setup(struct console *co, char *options)
 	port = &ulite_ports[co->index];
 
 	/* not initialized yet? */
-	if (!port->membase)
+	if (!port->membase) {
+		pr_debug("console on ttyUL%i not initialized\n", co->index);
 		return -ENODEV;
+	}
 
 	if (options)
 		uart_parse_options(options, &baud, &parity, &bits, &flow);
@@ -542,6 +549,72 @@ static struct platform_driver ulite_platform_driver = {
 };
 
 /* ---------------------------------------------------------------------
+ * OF bus bindings
+ */
+#if defined(CONFIG_OF)
+static int __devinit
+ulite_of_probe(struct of_device *op, const struct of_device_id *match)
+{
+	struct resource res;
+	const unsigned int *id;
+	int irq, rc;
+
+	dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
+
+	rc = of_address_to_resource(op->node, 0, &res);
+	if (rc) {
+		dev_err(&op->dev, "invalide address\n");
+		return rc;
+	}
+
+	irq = irq_of_parse_and_map(op->node, 0);
+
+	id = of_get_property(op->node, "port-number", NULL);
+
+	return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
+}
+
+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",
+	.match_table = ulite_of_match,
+	.probe = ulite_of_probe,
+	.remove = __devexit_p(ulite_of_remove),
+	.driver = {
+		.name = "uartlite",
+	},
+};
+
+/* Registration helpers to keep the number of #ifdefs to a minimum */
+static inline int __init ulite_of_register(void)
+{
+	pr_debug("uartlite: calling of_register_platform_driver()\n");
+	return of_register_platform_driver(&ulite_of_driver);
+}
+
+static inline void __exit ulite_of_unregister(void)
+{
+	of_unregister_platform_driver(&ulite_of_driver);
+}
+#else /* CONFIG_OF */
+/* CONFIG_OF not enabled; do nothing helpers */
+static inline int __init ulite_of_register(void) { return 0; }
+static inline void __exit ulite_of_unregister(void) { }
+#endif /* CONFIG_OF */
+
+/* ---------------------------------------------------------------------
  * Module setup/teardown
  */
 
@@ -549,20 +622,32 @@ int __init ulite_init(void)
 {
 	int ret;
 
-	ret = uart_register_driver(&ulite_uart_driver);
-	if (ret)
-		return ret;
+	pr_debug("uartlite: calling uart_register_driver()\n");
+	if ((ret = uart_register_driver(&ulite_uart_driver)) != 0)
+		goto err_uart;
 
-	ret = platform_driver_register(&ulite_platform_driver);
-	if (ret)
-		uart_unregister_driver(&ulite_uart_driver);
+	if ((ret = ulite_of_register()) != 0)
+		goto err_of;
 
+	pr_debug("uartlite: calling platform_driver_register()\n");
+	if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
+		goto err_plat;
+
+	return 0;
+
+err_plat:
+	ulite_of_unregister();
+err_of:
+	uart_unregister_driver(&ulite_uart_driver);
+err_uart:
+	printk(KERN_ERR "registering uartlite driver failed: err=%i", ret);
 	return ret;
 }
 
 void __exit ulite_exit(void)
 {
 	platform_driver_unregister(&ulite_platform_driver);
+	ulite_of_unregister();
 	uart_unregister_driver(&ulite_uart_driver);
 }
 

[PATCH 2 7/7] Uartlite: Let the console be initialized earlier

[Grant Likely]

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

By configuring it earlier we get console output sooner which is helpful
for debugging when the kernel crashes before the serial drivers are
initialized.

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

 drivers/serial/uartlite.c |   41 ++++++++++++++++++++++++++++++++++++++---
 1 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
index 8752fac..a1891d9 100644
--- a/drivers/serial/uartlite.c
+++ b/drivers/serial/uartlite.c
@@ -373,6 +373,31 @@ static void ulite_console_write(struct console *co, const char *s,
 		spin_unlock_irqrestore(&port->lock, flags);
 }
 
+#if defined(CONFIG_OF)
+static inline void __init ulite_console_of_find_device(int id)
+{
+	struct device_node *np;
+	struct resource res;
+	const unsigned int *of_id;
+	int rc;
+
+	for_each_compatible_node(np, NULL, "xilinx,uartlite") {
+		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;
+
+		ulite_ports[id].mapbase = res.start;
+		return;
+	}
+}
+#else /* CONFIG_OF */
+static inline void __init ulite_console_of_find_device(int id) { /* do nothing */ }
+#endif /* CONFIG_OF */
+
 static int __init ulite_console_setup(struct console *co, char *options)
 {
 	struct uart_port *port;
@@ -386,10 +411,20 @@ static int __init ulite_console_setup(struct console *co, char *options)
 
 	port = &ulite_ports[co->index];
 
+	/* Check if it is an OF device */
+	if (!port->mapbase)
+		ulite_console_of_find_device(co->index);
+
+	/* Do we have a device now? */
+	if (!port->mapbase) {
+		pr_debug("console on ttyUL%i not present\n", co->index);
+		return -ENODEV;
+	}
+
 	/* not initialized yet? */
 	if (!port->membase) {
-		pr_debug("console on ttyUL%i not initialized\n", co->index);
-		return -ENODEV;
+		if (ulite_request_port(port))
+			return -ENODEV;
 	}
 
 	if (options)
@@ -461,7 +496,7 @@ static int __devinit ulite_assign(struct device *dev, int id, u32 base, int irq)
 		return -EINVAL;
 	}
 
-	if (ulite_ports[id].mapbase) {
+	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;

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Benjamin Herrenschmidt]

On Sun, 2007-09-30 at 16:42 -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Add of_platform bus binding so this driver can be used with arch/powerpc

Another option is to have a "constructor" in the platform code that
generates the platform device from the DT. It might even become the
preferred way one of these days, I'm not too sure at this stage. Anyway,
do what you prefer.

> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  drivers/serial/uartlite.c |  101 +++++++++++++++++++++++++++++++++++++++++----
>  1 files changed, 93 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
> index ed13b9f..8752fac 100644
> --- a/drivers/serial/uartlite.c
> +++ b/drivers/serial/uartlite.c
> @@ -1,7 +1,8 @@
>  /*
>   * uartlite.c: Serial driver for Xilinx uartlite serial controller
>   *
> - * Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2006 Peter Korsgaard <jacmet@sunsite.dk>
> + * Copyright (C) 2007 Secret Lab Technologies Ltd.
>   *
>   * This file is licensed under the terms of the GNU General Public License
>   * version 2.  This program is licensed "as is" without any warranty of any
> @@ -17,6 +18,10 @@
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <asm/io.h>
> +#if defined(CONFIG_OF)
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#endif
>  
>  #define ULITE_NAME		"ttyUL"
>  #define ULITE_MAJOR		204
> @@ -382,8 +387,10 @@ static int __init ulite_console_setup(struct console *co, char *options)
>  	port = &ulite_ports[co->index];
>  
>  	/* not initialized yet? */
> -	if (!port->membase)
> +	if (!port->membase) {
> +		pr_debug("console on ttyUL%i not initialized\n", co->index);
>  		return -ENODEV;
> +	}
>  
>  	if (options)
>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
> @@ -542,6 +549,72 @@ static struct platform_driver ulite_platform_driver = {
>  };
>  
>  /* ---------------------------------------------------------------------
> + * OF bus bindings
> + */
> +#if defined(CONFIG_OF)
> +static int __devinit
> +ulite_of_probe(struct of_device *op, const struct of_device_id *match)
> +{
> +	struct resource res;
> +	const unsigned int *id;
> +	int irq, rc;
> +
> +	dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
> +
> +	rc = of_address_to_resource(op->node, 0, &res);
> +	if (rc) {
> +		dev_err(&op->dev, "invalide address\n");
> +		return rc;
> +	}
> +
> +	irq = irq_of_parse_and_map(op->node, 0);
> +
> +	id = of_get_property(op->node, "port-number", NULL);
> +
> +	return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
> +}
> +
> +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",
> +	.match_table = ulite_of_match,
> +	.probe = ulite_of_probe,
> +	.remove = __devexit_p(ulite_of_remove),
> +	.driver = {
> +		.name = "uartlite",
> +	},
> +};
> +
> +/* Registration helpers to keep the number of #ifdefs to a minimum */
> +static inline int __init ulite_of_register(void)
> +{
> +	pr_debug("uartlite: calling of_register_platform_driver()\n");
> +	return of_register_platform_driver(&ulite_of_driver);
> +}
> +
> +static inline void __exit ulite_of_unregister(void)
> +{
> +	of_unregister_platform_driver(&ulite_of_driver);
> +}
> +#else /* CONFIG_OF */
> +/* CONFIG_OF not enabled; do nothing helpers */
> +static inline int __init ulite_of_register(void) { return 0; }
> +static inline void __exit ulite_of_unregister(void) { }
> +#endif /* CONFIG_OF */
> +
> +/* ---------------------------------------------------------------------
>   * Module setup/teardown
>   */
>  
> @@ -549,20 +622,32 @@ int __init ulite_init(void)
>  {
>  	int ret;
>  
> -	ret = uart_register_driver(&ulite_uart_driver);
> -	if (ret)
> -		return ret;
> +	pr_debug("uartlite: calling uart_register_driver()\n");
> +	if ((ret = uart_register_driver(&ulite_uart_driver)) != 0)
> +		goto err_uart;
>  
> -	ret = platform_driver_register(&ulite_platform_driver);
> -	if (ret)
> -		uart_unregister_driver(&ulite_uart_driver);
> +	if ((ret = ulite_of_register()) != 0)
> +		goto err_of;
>  
> +	pr_debug("uartlite: calling platform_driver_register()\n");
> +	if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
> +		goto err_plat;
> +
> +	return 0;
> +
> +err_plat:
> +	ulite_of_unregister();
> +err_of:
> +	uart_unregister_driver(&ulite_uart_driver);
> +err_uart:
> +	printk(KERN_ERR "registering uartlite driver failed: err=%i", ret);
>  	return ret;
>  }
>  
>  void __exit ulite_exit(void)
>  {
>  	platform_driver_unregister(&ulite_platform_driver);
> +	ulite_of_unregister();
>  	uart_unregister_driver(&ulite_uart_driver);
>  }
>  
> 
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

On 10/1/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Sun, 2007-09-30 at 16:42 -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Add of_platform bus binding so this driver can be used with arch/powerpc
>
> Another option is to have a "constructor" in the platform code that
> generates the platform device from the DT. It might even become the
> preferred way one of these days, I'm not too sure at this stage. Anyway,
> do what you prefer.

I've looked at both, and I'm not to fond of the 'constructor'
approach.  Here are my reasons:

- Separate constructor code must be written for each driver anyway
(except perhaps for very simple cases which only require 'regs' and
'interrupts' properties).  The device tree data must then be mapped
onto a pdata structure; which the driver immediately turns around and
unmaps from pdata to populate it's driver state structure.  It ends up
being a lot of indirection to follow.
- Many (but perhaps not all) of these devices will be registered as an
of_device anyway.  By having a constructor to generate a
platform_device from that means each device will get probed twice;
once by the of_platform bus and once by the platform bus.
- Writing device drivers with multiple binding is easy and probable
results in smaller simpler code than the two step process of using a
constructor.

For example, here is my of_device binding for the systemace driver:

static int __devinit
ulite_of_probe(struct of_device *op, const struct of_device_id *match)
{
        struct resource res;
        const unsigned int *id;
        int irq, rc;
        dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
        rc = of_address_to_resource(op->node, 0, &res);
        if (rc) {
                dev_err(&op->dev, "invalide address\n");
                return rc;
        }
        irq = irq_of_parse_and_map(op->node, 0);
        id = of_get_property(op->node, "port-number", NULL);
        return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
}

That's 3 calls to get data out of the device tree and one call to
setup the device.  The final 'ulite_assign()' call is called by both
the of_device and platform_device bindings.

To do the same thing with a constructor requires an additional
allocation and registration of a platform device structure and a pdata
structure.  Plus it requires the platform code to explicitly call a
set of helper functions to find and create platform devices for nodes
in the device tree.  (Unless you're thinking about leveraging the
of_platform probe code and have the .probe hook to the 'constructor'
bit)

What advantages do you see with the constructor approach?

Cheers,
g.

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

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Peter Korsgaard]

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

Hi,

 Grant> static int __devinit
 Grant> ulite_of_probe(struct of_device *op, const struct of_device_id *match)

This looks like uartlite code to me ;)

 Grant> {
 Grant>         struct resource res;
 Grant>         const unsigned int *id;
 Grant>         int irq, rc;
 Grant>         dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
 Grant>         rc = of_address_to_resource(op->node, 0, &res);
 Grant>         if (rc) {
 Grant>                 dev_err(&op->dev, "invalide address\n");
 Grant>                 return rc;
 Grant>         }
 Grant>         irq = irq_of_parse_and_map(op->node, 0);
 Grant>         id = of_get_property(op->node, "port-number", NULL);
 Grant>         return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
 Grant> }

 Grant> What advantages do you see with the constructor approach?

One advantage is that it keeps the of stuff out of the drivers. There
already is one bus for platform stuff in the kernel, so from a device
driver writer POV the of stuff is just extra fluff. Imagine the ARM or
MIPS people coming up with 2 other incompatible ways of doing this and
you'll see the drivers bloat.

E.G. I use the smsc911x.c network driver on powerpc which is written
by an ARM guy. Why should he need to care about of stuff in his driver?

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Scott Wood]

Peter Korsgaard wrote:
>  Grant> What advantages do you see with the constructor approach?
> 
> One advantage is that it keeps the of stuff out of the drivers. There
> already is one bus for platform stuff in the kernel, so from a device
> driver writer POV the of stuff is just extra fluff. Imagine the ARM or
> MIPS people coming up with 2 other incompatible ways of doing this and
> you'll see the drivers bloat.

OTOH, it's nice to keep everything relating to a certain device in one 
spot, rather than scattering some bits in arch.  Plus, the device tree 
is not just powerpc; it's also used on sparc ond OLPC.

> E.G. I use the smsc911x.c network driver on powerpc which is written
> by an ARM guy. Why should he need to care about of stuff in his driver?

Just because an ARM guy wrote it doesn't mean powerpc guys can't stuff 
things in there. :-)

It would be nice, though, to merge platform and of_platform to some 
extent, so that things which don't need to check "special" device tree 
properties wouldn't have to make any changes other than maybe adding an 
extra match table entry.

-Scott

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>
> Hi,
>
>  Grant> static int __devinit
>  Grant> ulite_of_probe(struct of_device *op, const struct of_device_id *match)
>
> This looks like uartlite code to me ;)
>
>  Grant> {
>  Grant>         struct resource res;
>  Grant>         const unsigned int *id;
>  Grant>         int irq, rc;
>  Grant>         dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
>  Grant>         rc = of_address_to_resource(op->node, 0, &res);
>  Grant>         if (rc) {
>  Grant>                 dev_err(&op->dev, "invalide address\n");
>  Grant>                 return rc;
>  Grant>         }
>  Grant>         irq = irq_of_parse_and_map(op->node, 0);
>  Grant>         id = of_get_property(op->node, "port-number", NULL);
>  Grant>         return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
>  Grant> }
>
>  Grant> What advantages do you see with the constructor approach?
>
> One advantage is that it keeps the of stuff out of the drivers. There
> already is one bus for platform stuff in the kernel, so from a device
> driver writer POV the of stuff is just extra fluff. Imagine the ARM or
> MIPS people coming up with 2 other incompatible ways of doing this and
> you'll see the drivers bloat.
>
> E.G. I use the smsc911x.c network driver on powerpc which is written
> by an ARM guy. Why should he need to care about of stuff in his driver?

The problem is that driver specific constructor code needs to be
written regardless.  Where should it live?  With the driver or in the
arch code?  Actually, with the arch code doesn't work well either
because multiple archs will be using of_platform (microblaze for
example), so they need to live somewhere common.

My opinion is that since it is driver-specific code anyway, then it
belongs with the driver.  Plus a driver writer for ARM doesn't need to
write them.  It's the powerpc or microblaze developer who will do it.
If the driver maintainer doesn't want the binding in the main driver
.c file, then the binding can easily be in an additional .c file
without needing to add a constructor.  (Kind of like how many USB host
controllers are managed)

g.

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

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

On 10/2/07, Scott Wood <scottwood@freescale.com> wrote:
> It would be nice, though, to merge platform and of_platform to some
> extent, so that things which don't need to check "special" device tree
> properties wouldn't have to make any changes other than maybe adding an
> extra match table entry.

yes, that would be really nice.

On that note, is it possible to modify the device tree (and of_device
registrations) at run time?  The one feature I do really like about
using the platform bus is that you can register/unregister devices at
run time.

Cheers,
g.

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

Re: [PATCH 2 3/7] Uartlite: Add macro for uartlite device name

[Peter Korsgaard]

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

 Grant> From: Grant Likely <grant.likely@secretlab.ca>
 Grant> Changed to make the following OF_platform bus binding patch a wee bit cleaner

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

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 5/7] Uartlite: Comment block tidy

[Peter Korsgaard]

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

 Grant> From: Grant Likely <grant.likely@secretlab.ca> Tidy the
 Grant> comments to split the driver into logical section; the main
 Grant> driver, the console driver, the platform bus binding, and
 Grant> module initialization and teardown.

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

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 2/7] Uartlite: change name of ports to ulite_ports

[Peter Korsgaard]

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

 Grant> From: Grant Likely <grant.likely@secretlab.ca>
 Grant> Changed to match naming convention used in the rest of the module

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

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 7/7] Uartlite: Let the console be initialized earlier

[Peter Korsgaard]

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

 Grant> From: Grant Likely <grant.likely@secretlab.ca> By configuring
 Grant> it earlier we get console output sooner which is helpful for
 Grant> debugging when the kernel crashes before the serial drivers
 Grant> are initialized.

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

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Peter Korsgaard]

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

 Grant> From: Grant Likely <grant.likely@secretlab.ca> Add of_platform
 Grant> bus binding so this driver can be used with arch/powerpc

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

Acked-by: Peter Korsgaard <jacmet@sunsite.dk>

-- 
Bye, Peter Korsgaard

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Benjamin Herrenschmidt]

> My opinion is that since it is driver-specific code anyway, then it
> belongs with the driver.  Plus a driver writer for ARM doesn't need to
> write them.  It's the powerpc or microblaze developer who will do it.
> If the driver maintainer doesn't want the binding in the main driver
> .c file, then the binding can easily be in an additional .c file
> without needing to add a constructor.  (Kind of like how many USB host
> controllers are managed)

The main advantage is that it keeps the OF specific code localized to a
single function, whether that function lives in the driver or the arch
code, it makes it self contained and easier to deal with by the driver
author.

Having multiple device types on which the driver can attach is a pain
from a driver standpoint. It needs multiple
probe/remove/suspend/resume/shutdown hooks etc... it's a bigger
maintainance burden in the long run.

The important thing however, with the constructor approach is to try as
much as possible to keep the proper tree structure, and thus, try to
find a way to instanciate the devices with proper parent/child
relationship so that ordering for things like suspend/resume operations
is maintained.

Ben.

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

On 10/2/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> > My opinion is that since it is driver-specific code anyway, then it
> > belongs with the driver.  Plus a driver writer for ARM doesn't need to
> > write them.  It's the powerpc or microblaze developer who will do it.
> > If the driver maintainer doesn't want the binding in the main driver
> > .c file, then the binding can easily be in an additional .c file
> > without needing to add a constructor.  (Kind of like how many USB host
> > controllers are managed)
>
> The main advantage is that it keeps the OF specific code localized to a
> single function, whether that function lives in the driver or the arch
> code, it makes it self contained and easier to deal with by the driver
> author.
>
> Having multiple device types on which the driver can attach is a pain
> from a driver standpoint. It needs multiple
> probe/remove/suspend/resume/shutdown hooks etc... it's a bigger
> maintainance burden in the long run.

For many drivers, I think that is already the case.  USB OHCI is a
prime example where there are both PCI and platform_bus bindings among
others.  It seems to me that the bus binding effectively translates
down to "where do I go to get the needed information".  I think it
results in less of a maintenance burden to explicitly separate bus
binding from device setup as opposed to adding constructor code.

> The important thing however, with the constructor approach is to try as
> much as possible to keep the proper tree structure, and thus, try to
> find a way to instanciate the devices with proper parent/child
> relationship so that ordering for things like suspend/resume operations
> is maintained.

I'm not sure I follow.  Example?

Thanks,
g.

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

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Benjamin Herrenschmidt]

On Tue, 2007-10-02 at 22:18 -0600, Grant Likely wrote:

> For many drivers, I think that is already the case.  USB OHCI is a
> prime example where there are both PCI and platform_bus bindings among
> others.  It seems to me that the bus binding effectively translates
> down to "where do I go to get the needed information".  I think it
> results in less of a maintenance burden to explicitly separate bus
> binding from device setup as opposed to adding constructor code.

I think nobody consider the mess that is USB in that are to be something
we want to reproduce.

> > The important thing however, with the constructor approach is to try as
> > much as possible to keep the proper tree structure, and thus, try to
> > find a way to instanciate the devices with proper parent/child
> > relationship so that ordering for things like suspend/resume operations
> > is maintained.
> 
> I'm not sure I follow.  Example?

Well, make sure that if 2 platform devices repreesnt respectively a bus
and a device on that bus, they properly get instanciated as parent &
child in sysfs as well.

Ben.

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Grant Likely]

On 10/2/07, Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:
>
> On Tue, 2007-10-02 at 22:18 -0600, Grant Likely wrote:
>
> > For many drivers, I think that is already the case.  USB OHCI is a
> > prime example where there are both PCI and platform_bus bindings among
> > others.  It seems to me that the bus binding effectively translates
> > down to "where do I go to get the needed information".  I think it
> > results in less of a maintenance burden to explicitly separate bus
> > binding from device setup as opposed to adding constructor code.
>
> I think nobody consider the mess that is USB in that are to be something
> we want to reproduce.

Heh, true, but the structure of multiple bus bindings is probably not
something we can get away from.

>
> > > The important thing however, with the constructor approach is to try as
> > > much as possible to keep the proper tree structure, and thus, try to
> > > find a way to instanciate the devices with proper parent/child
> > > relationship so that ordering for things like suspend/resume operations
> > > is maintained.
> >
> > I'm not sure I follow.  Example?
>
> Well, make sure that if 2 platform devices repreesnt respectively a bus
> and a device on that bus, they properly get instanciated as parent &
> child in sysfs as well.

Right, okay.  Looking at platform_device_add(), the default parent is
platform_bus, but it can be overridden.  of_platform_bus devices get
the hierarchy of the device tree by default.  So in the platform bus
case, the constructor would need to explicitly set the parent device?
Correct?

Also, how do you see the constructor code getting executed?  Called
explicitly from the platform code, or some form of auto binding?  I
look at fsl_soc.c and I shudder as each constructor does a pass of the
whole tree looking for compatible nodes.

Cheers,
g.

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

Re: [PATCH 2 6/7] Uartlite: Add of-platform-bus binding

[Benjamin Herrenschmidt]

On Wed, 2007-10-03 at 08:39 -0600, Grant Likely wrote:
> Right, okay.  Looking at platform_device_add(), the default parent is
> platform_bus, but it can be overridden.  of_platform_bus devices get
> the hierarchy of the device tree by default.  So in the platform bus
> case, the constructor would need to explicitly set the parent device?
> Correct?
> 
> Also, how do you see the constructor code getting executed?  Called
> explicitly from the platform code, or some form of auto binding?  I
> look at fsl_soc.c and I shudder as each constructor does a pass of the
> whole tree looking for compatible nodes.

My idea was to have some platform code at boot walk the DT and call the
constructors on the way, passing them the parent. Constructors return a
struct device * so it would keep track of who is where.

Drivers or platform code can then register constructors along with of
match tables.

Ben.