[PATCH 00/18] Virtex support in arch/powerpc

[PATCH 00/18] Virtex support in arch/powerpc

[Grant Likely]

This series adds Xilinx Virtex support to arch/powerpc.  Please review
and comment.  It includes support for the uartlite and SystemACE devices

Cheers,
g.

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

[PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Grant Likely]

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

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

 arch/powerpc/boot/Makefile   |    2 +
 arch/powerpc/boot/ops.h      |    1 +
 arch/powerpc/boot/serial.c   |    2 +
 arch/powerpc/boot/uartlite.c |   64 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 68 insertions(+), 1 deletions(-)

diff --git a/arch/powerpc/boot/Makefile b/arch/powerpc/boot/Makefile
index ca469e5..ac488ab 100644
--- a/arch/powerpc/boot/Makefile
+++ b/arch/powerpc/boot/Makefile
@@ -51,7 +51,7 @@ src-wlib := string.S crt0.S stdio.c main.c flatdevtree.c flatdevtree_misc.c \
 		ns16550.c serial.c simple_alloc.c div64.S util.S \
 		gunzip_util.c elf_util.c $(zlib) devtree.c oflib.c ofconsole.c \
 		4xx.c ebony.c mv64x60.c mpsc.c mv64x60_i2c.c cuboot.c bamboo.c \
-		cpm-serial.c stdlib.c
+		cpm-serial.c stdlib.c uartlite.c
 src-plat := of.c cuboot-83xx.c cuboot-85xx.c holly.c \
 		cuboot-ebony.c treeboot-ebony.c prpmc2800.c \
 		ps3-head.S ps3-hvcall.S ps3.c treeboot-bamboo.c cuboot-8xx.c \
diff --git a/arch/powerpc/boot/ops.h b/arch/powerpc/boot/ops.h
index 703255b..4ef30e4 100644
--- a/arch/powerpc/boot/ops.h
+++ b/arch/powerpc/boot/ops.h
@@ -84,6 +84,7 @@ int serial_console_init(void);
 int ns16550_console_init(void *devp, struct serial_console_data *scdp);
 int mpsc_console_init(void *devp, struct serial_console_data *scdp);
 int cpm_console_init(void *devp, struct serial_console_data *scdp);
+int uartlite_console_init(void *devp, struct serial_console_data *scdp);
 void *simple_alloc_init(char *base, unsigned long heap_size,
 			unsigned long granularity, unsigned long max_allocs);
 extern void flush_cache(void *, unsigned long);
diff --git a/arch/powerpc/boot/serial.c b/arch/powerpc/boot/serial.c
index d47f8e0..70f36bf 100644
--- a/arch/powerpc/boot/serial.c
+++ b/arch/powerpc/boot/serial.c
@@ -126,6 +126,8 @@ int serial_console_init(void)
 	         dt_is_compatible(devp, "fsl,cpm2-scc-uart") ||
 	         dt_is_compatible(devp, "fsl,cpm2-smc-uart"))
 		rc = cpm_console_init(devp, &serial_cd);
+	else if (dt_is_compatible(devp, "xilinx,uartlite"))
+		rc = uartlite_console_init(devp, &serial_cd);
 
 	/* Add other serial console driver calls here */
 
diff --git a/arch/powerpc/boot/uartlite.c b/arch/powerpc/boot/uartlite.c
new file mode 100644
index 0000000..f4249a7
--- /dev/null
+++ b/arch/powerpc/boot/uartlite.c
@@ -0,0 +1,64 @@
+/*
+ * Xilinx UARTLITE bootloader driver
+ *
+ * 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
+ * kind, whether express or implied.
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include "types.h"
+#include "string.h"
+#include "stdio.h"
+#include "io.h"
+#include "ops.h"
+
+static void * reg_base;
+
+static int uartlite_open(void)
+{
+	/* Clear the RX FIFO */
+	out_be32(reg_base + 0x0C, 0x2);
+	return 0;
+}
+
+static void uartlite_putc(unsigned char c)
+{
+	while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
+	out_be32(reg_base + 0x4, c);
+}
+
+static unsigned char uartlite_getc(void)
+{
+	while ((in_be32(reg_base + 0x8) & 0x01) == 0); /* spin */
+	return in_be32(reg_base);
+}
+
+static u8 uartlite_tstc(void)
+{
+	return ((in_be32(reg_base + 0x8) & 0x01) != 0);
+}
+
+int uartlite_console_init(void *devp, struct serial_console_data *scdp)
+{
+	int n;
+	unsigned long reg_phys;
+
+	n = getprop(devp, "virtual-reg", &reg_base, sizeof(reg_base));
+	if (n != sizeof(reg_base)) {
+		if (!dt_xlate_reg(devp, 0, &reg_phys, NULL))
+			return -1;
+
+		reg_base = (void *)reg_phys;
+	}
+
+	scdp->open = uartlite_open;
+	scdp->putc = uartlite_putc;
+	scdp->getc = uartlite_getc;
+	scdp->tstc = uartlite_tstc;
+	scdp->close = NULL;
+	return 0;
+}

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Arnd Bergmann]

On Friday 28 September 2007, Grant Likely wrote:
> +static void uartlite_putc(unsigned char c)
> +{
> +=A0=A0=A0=A0=A0=A0=A0while ((in_be32(reg_base + 0x8) & 0x08) !=3D 0); /*=
 spin */
> +=A0=A0=A0=A0=A0=A0=A0out_be32(reg_base + 0x4, c);
> +}

When coding a spin-loop, it's better to do a cpu_relax() between
each attempt.

	Arnd <><

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Grant Likely]

On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +static void uartlite_putc(unsigned char c)
> > +{
> > +while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
> > +out_be32(reg_base + 0x4, c);
> > +}
>
> When coding a spin-loop, it's better to do a cpu_relax() between
> each attempt.

Is cpu_relax even implemented in the bootwrapper?

g.

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

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Josh Boyer]

On Fri, 28 Sep 2007 14:04:04 -0600
"Grant Likely" <grant.likely@secretlab.ca> wrote:

> On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> > On Friday 28 September 2007, Grant Likely wrote:
> > > +static void uartlite_putc(unsigned char c)
> > > +{
> > > +while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
> > > +out_be32(reg_base + 0x4, c);
> > > +}
> >
> > When coding a spin-loop, it's better to do a cpu_relax() between
> > each attempt.
> 
> Is cpu_relax even implemented in the bootwrapper?

No.  And it doesn't need to be :)

josh

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Arnd Bergmann]

On Friday 28 September 2007, Josh Boyer wrote:
>=20
> > Is cpu_relax even implemented in the bootwrapper?
>=20
> No. =A0And it doesn't need to be :)

I think I should learn to read subject lines. You are of course
both right.

	Arnd <><

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Peter Korsgaard]

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

Hi,

 Grant> +static int uartlite_open(void)
 Grant> +{
 Grant> +	/* Clear the RX FIFO */
 Grant> +	out_be32(reg_base + 0x0C, 0x2);
 Grant> +	return 0;
 Grant> +}
 Grant> +
 Grant> +static void uartlite_putc(unsigned char c)
 Grant> +{
 Grant> +	while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
 Grant> +	out_be32(reg_base + 0x4, c);

RX, TX, STATUS and CONTROL defines for the registers would be nice for
readability.

-- 
Bye, Peter Korsgaard

Re: [PATCH 01/18] Virtex: Add uartlite bootwrapper driver

[Grant Likely]

On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>
> Hi,
>
>  Grant> +static int uartlite_open(void)
>  Grant> +{
>  Grant> +       /* Clear the RX FIFO */
>  Grant> +       out_be32(reg_base + 0x0C, 0x2);
>  Grant> +       return 0;
>  Grant> +}
>  Grant> +
>  Grant> +static void uartlite_putc(unsigned char c)
>  Grant> +{
>  Grant> +       while ((in_be32(reg_base + 0x8) & 0x08) != 0); /* spin */
>  Grant> +       out_be32(reg_base + 0x4, c);
>
> RX, TX, STATUS and CONTROL defines for the registers would be nice for
> readability.

Good idea, I'll add a patch for that

g.


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

[PATCH 02/18] Add Kconfig macros for Xilinx Virtex support

[Grant Likely]

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

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

 arch/powerpc/platforms/40x/Kconfig |   30 +++++++++++++++++-------------
 1 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index c3dce3b..1aae0e6 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -61,13 +61,14 @@ config WALNUT
 	help
 	  This option enables support for the IBM PPC405GP evaluation board.
 
-#config XILINX_ML300
-#	bool "Xilinx-ML300"
-#	depends on 40x
-#	default y
-#	select VIRTEX_II_PRO
-#	help
-#	  This option enables support for the Xilinx ML300 evaluation board.
+config XILINX_VIRTEX_GENERIC_BOARD
+	bool "Generic Xilinx Virtex board"
+	depends on 40x
+	default y
+	select VIRTEX_II_PRO
+	select VIRTEX_4_FX
+	help
+	  This option enables generic support for Xilinx Virtex based boards.
 
 # 40x specific CPU modules, selected based on the board above.
 config NP405H
@@ -91,11 +92,19 @@ config 405EP
 config 405GPR
 	bool
 
-config VIRTEX_II_PRO
+config XILINX_VIRTEX
 	bool
+
+config XILINX_VIRTEX_II_PRO
+	bool
+	select XILINX_VIRTEX
 	select IBM405_ERR77
 	select IBM405_ERR51
 
+config XILINX_VIRTEX_4_FX
+	bool
+	select XILINX_VIRTEX
+
 config STB03xxx
 	bool
 	select IBM405_ERR77
@@ -111,11 +120,6 @@ config IBM405_ERR77
 config IBM405_ERR51
 	bool
 
-#config XILINX_OCP
-#	bool
-#	depends on XILINX_ML300
-#	default y
-
 #config BIOS_FIXUP
 #	bool
 #	depends on BUBINGA || EP405 || SYCAMORE || WALNUT

Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support

[Scott Wood]

Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  arch/powerpc/platforms/40x/Kconfig |   30 +++++++++++++++++-------------
>  1 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> index c3dce3b..1aae0e6 100644
> --- a/arch/powerpc/platforms/40x/Kconfig
> +++ b/arch/powerpc/platforms/40x/Kconfig
> @@ -61,13 +61,14 @@ config WALNUT
>  	help
>  	  This option enables support for the IBM PPC405GP evaluation board.
>  
> -#config XILINX_ML300
> -#	bool "Xilinx-ML300"
> -#	depends on 40x
> -#	default y
> -#	select VIRTEX_II_PRO
> -#	help
> -#	  This option enables support for the Xilinx ML300 evaluation board.
> +config XILINX_VIRTEX_GENERIC_BOARD
> +	bool "Generic Xilinx Virtex board"
> +	depends on 40x
> +	default y
> +	select VIRTEX_II_PRO
> +	select VIRTEX_4_FX
> +	help
> +	  This option enables generic support for Xilinx Virtex based boards.

I don't think we want default y here.

-Scott

Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support

[Grant Likely]

On 9/28/07, Scott Wood <scottwood@freescale.com> wrote:
> Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> >  arch/powerpc/platforms/40x/Kconfig |   30 +++++++++++++++++-------------
> >  1 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> > index c3dce3b..1aae0e6 100644
> > --- a/arch/powerpc/platforms/40x/Kconfig
> > +++ b/arch/powerpc/platforms/40x/Kconfig
> > @@ -61,13 +61,14 @@ config WALNUT
> >       help
> >         This option enables support for the IBM PPC405GP evaluation board.
> >
> > -#config XILINX_ML300
> > -#    bool "Xilinx-ML300"
> > -#    depends on 40x
> > -#    default y
> > -#    select VIRTEX_II_PRO
> > -#    help
> > -#      This option enables support for the Xilinx ML300 evaluation board.
> > +config XILINX_VIRTEX_GENERIC_BOARD
> > +     bool "Generic Xilinx Virtex board"
> > +     depends on 40x
> > +     default y
> > +     select VIRTEX_II_PRO
> > +     select VIRTEX_4_FX
> > +     help
> > +       This option enables generic support for Xilinx Virtex based boards.
>
> I don't think we want default y here.

I just followed the lead of Walnut here.  Perhaps for the embedded
targets all of them should be 'default n'.  Josh, thoughts?

g.

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

Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support

[Olof Johansson]

On Fri, Sep 28, 2007 at 12:16:01PM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
> 
>  arch/powerpc/platforms/40x/Kconfig |   30 +++++++++++++++++-------------
>  1 files changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> index c3dce3b..1aae0e6 100644
> --- a/arch/powerpc/platforms/40x/Kconfig
> +++ b/arch/powerpc/platforms/40x/Kconfig
> @@ -61,13 +61,14 @@ config WALNUT
>  	help
>  	  This option enables support for the IBM PPC405GP evaluation board.
>  
> -#config XILINX_ML300
> -#	bool "Xilinx-ML300"
> -#	depends on 40x
> -#	default y
> -#	select VIRTEX_II_PRO
> -#	help
> -#	  This option enables support for the Xilinx ML300 evaluation board.
> +config XILINX_VIRTEX_GENERIC_BOARD
> +	bool "Generic Xilinx Virtex board"
> +	depends on 40x
> +	default y
> +	select VIRTEX_II_PRO
> +	select VIRTEX_4_FX
> +	help
> +	  This option enables generic support for Xilinx Virtex based boards.

I would appreciate a bit verboser help text here, i.e. including what
boards are considered generic. Maybe something like "...including ML403,
<x>, <y>, and other 4FX/IIPro-based boards"?


-Olof

Re: [PATCH 02/18] Add Kconfig macros for Xilinx Virtex support

[Grant Likely]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:16:01PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> >  arch/powerpc/platforms/40x/Kconfig |   30 +++++++++++++++++-------------
> >  1 files changed, 17 insertions(+), 13 deletions(-)
> >
> > diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
> > index c3dce3b..1aae0e6 100644
> > --- a/arch/powerpc/platforms/40x/Kconfig
> > +++ b/arch/powerpc/platforms/40x/Kconfig
> > @@ -61,13 +61,14 @@ config WALNUT
> >       help
> >         This option enables support for the IBM PPC405GP evaluation board.
> >
> > -#config XILINX_ML300
> > -#    bool "Xilinx-ML300"
> > -#    depends on 40x
> > -#    default y
> > -#    select VIRTEX_II_PRO
> > -#    help
> > -#      This option enables support for the Xilinx ML300 evaluation board.
> > +config XILINX_VIRTEX_GENERIC_BOARD
> > +     bool "Generic Xilinx Virtex board"
> > +     depends on 40x
> > +     default y
> > +     select VIRTEX_II_PRO
> > +     select VIRTEX_4_FX
> > +     help
> > +       This option enables generic support for Xilinx Virtex based boards.
>
> I would appreciate a bit verboser help text here, i.e. including what
> boards are considered generic. Maybe something like "...including ML403,
> <x>, <y>, and other 4FX/IIPro-based boards"?

Done.

          This option enables generic support for Xilinx Virtex based boards.

+         The generic virtex board support matches any device tree which
+         specifies 'xilinx,virtex' in its compatible field.  This includes
+         the Xilinx ML3xx and ML4xx reference designs using the powerpc
+         core.
+
+         Most Virtex designs should use this unless it needs to do some
+         special configuration at board probe time.
+

Cheers,
g.

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

[PATCH 03/18] Virtex: add xilinx interrupt controller driver

[Grant Likely]

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

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

 arch/powerpc/sysdev/Makefile      |    1 
 arch/powerpc/sysdev/xilinx_intc.c |  151 +++++++++++++++++++++++++++++++++++++
 include/asm-powerpc/xilinx_intc.h |   20 +++++
 3 files changed, 172 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 08ce31e..0457117 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_PPC_INDIRECT_PCI)	+= indirect_pci.o
 obj-$(CONFIG_PPC_I8259)		+= i8259.o
 obj-$(CONFIG_PPC_83xx)		+= ipic.o
 obj-$(CONFIG_4xx)		+= uic.o
+obj-$(CONFIG_XILINX_VIRTEX)	+= xilinx_intc.o
 endif
 
 # Temporary hack until we have migrated to asm-powerpc
diff --git a/arch/powerpc/sysdev/xilinx_intc.c b/arch/powerpc/sysdev/xilinx_intc.c
new file mode 100644
index 0000000..fe24f0f
--- /dev/null
+++ b/arch/powerpc/sysdev/xilinx_intc.c
@@ -0,0 +1,151 @@
+/*
+ * Interrupt controller driver for Xilinx Virtex FPGAs
+ *
+ * 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
+ * kind, whether express or implied.
+ *
+ */
+
+/*
+ * This is a driver for the interrupt controller typically found in
+ * Xilinx Virtex FPGA designs.
+ *
+ * The interrupt sense levels are hard coded into the FPGA design with
+ * typically a 1:1 relationship between irq lines and devices (no shared
+ * irq lines).  Therefore, this driver does not attempt to handle edge
+ * and level interrupts differently.
+ */
+#undef DEBUG
+
+#include <linux/kernel.h>
+#include <linux/irq.h>
+#include <asm/io.h>
+#include <asm/processor.h>
+#include <asm/prom.h>
+#include <asm/irq.h>
+
+/*
+ * INTC Registers
+ */
+#define ISR	0	/* Interrupt Status */
+#define IPR	4	/* Interrupt Pending */
+#define IER	8	/* Interrupt Enable */
+#define IAR	12	/* Interrupt Acknowledge */
+#define SIE	16	/* Set Interrupt Enable bits */
+#define CIE	20	/* Clear Interrupt Enable bits */
+#define IVR	24	/* Interrupt Vector */
+#define MER	28	/* Master Enable */
+
+static struct irq_host *master_irqhost;
+
+/*
+ * IRQ Chip operations
+ */
+static void xilinx_intc_mask(unsigned int virq)
+{
+	int irq = irq_map[virq].hwirq;
+	void * regs = get_irq_chip_data(virq);
+	pr_debug("mask: %d\n", irq);
+	out_be32(regs + CIE, 1 << irq);
+}
+
+static void xilinx_intc_unmask(unsigned int virq)
+{
+	int irq = irq_map[virq].hwirq;
+	void * regs = get_irq_chip_data(virq);
+	pr_debug("unmask: %d\n", irq);
+	out_be32(regs + SIE, 1 << irq);
+}
+
+static void xilinx_intc_ack(unsigned int virq)
+{
+	int irq = irq_map[virq].hwirq;
+	void * regs = get_irq_chip_data(virq);
+	pr_debug("ack: %d\n", irq);
+	out_be32(regs + IAR, 1 << irq);
+}
+
+static struct irq_chip xilinx_intc_irqchip = {
+	.typename = "Xilinx INTC",
+	.mask = xilinx_intc_mask,
+	.unmask = xilinx_intc_unmask,
+	.ack = xilinx_intc_ack,
+};
+
+/*
+ * IRQ Host operations
+ */
+static int xilinx_intc_map(struct irq_host *h, unsigned int virq,
+				  irq_hw_number_t irq)
+{
+	set_irq_chip_data(virq, h->host_data);
+	set_irq_chip_and_handler(virq, &xilinx_intc_irqchip, handle_level_irq);
+	set_irq_type(virq, IRQ_TYPE_NONE);
+	return 0;
+}
+
+static struct irq_host_ops xilinx_intc_ops = {
+	.map = xilinx_intc_map,
+};
+
+struct irq_host * __init
+xilinx_intc_init(struct device_node *np)
+{
+	struct irq_host * irq;
+	struct resource res;
+	void * regs;
+	int rc;
+
+	/* Find and map the intc registers */
+	rc = of_address_to_resource(np, 0, &res);
+	if (rc) {
+		printk(KERN_ERR __FILE__ ": of_address_to_resource() failed\n");
+		return NULL;
+	}
+	regs = ioremap(res.start, 32);
+
+	printk(KERN_INFO "Xilinx intc at 0x%08X mapped to 0x%p\n",
+		res.start, regs);
+
+	/* Setup interrupt controller */
+	out_be32(regs + IER, 0); /* disable all irqs */
+	out_be32(regs + IAR, ~(u32) 0); /* Acknowledge pending irqs */
+	out_be32(regs + MER, 0x3UL); /* Turn on the Master Enable. */
+
+	/* Allocate and initialize an irq_host structure. */
+	irq = irq_alloc_host(np, IRQ_HOST_MAP_LINEAR, 32, &xilinx_intc_ops, -1);
+	if (!irq)
+		panic(__FILE__ ": Cannot allocate IRQ host\n");
+	irq->host_data = regs;
+	return irq;
+}
+
+int xilinx_intc_get_irq(void)
+{
+	void * regs = master_irqhost->host_data;
+	pr_debug("get_irq:\n");
+	return irq_linear_revmap(master_irqhost, in_be32(regs + IVR));
+}
+
+void __init xilinx_intc_init_tree(void)
+{
+	struct device_node *np;
+
+	/* find top level interrupt controller */
+	for_each_compatible_node(np, NULL, "xilinx,intc") {
+		if (!of_get_property(np, "interrupts", NULL))
+			break;
+	}
+
+	/* xilinx interrupt controller needs to be top level */
+	BUG_ON(!np);
+
+	master_irqhost = xilinx_intc_init(np);
+	BUG_ON(!master_irqhost);
+
+	irq_set_default_host(master_irqhost);
+	of_node_put(np);
+}
diff --git a/include/asm-powerpc/xilinx_intc.h b/include/asm-powerpc/xilinx_intc.h
new file mode 100644
index 0000000..343612f
--- /dev/null
+++ b/include/asm-powerpc/xilinx_intc.h
@@ -0,0 +1,20 @@
+/*
+ * Xilinx intc external definitions
+ *
+ * Copyright 2007 Secret Lab Technologies Ltd.
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+#ifndef _ASM_POWERPC_XILINX_INTC_H
+#define _ASM_POWERPC_XILINX_INTC_H
+
+#ifdef __KERNEL__
+
+extern void __init xilinx_intc_init_tree(void);
+extern unsigned int xilinx_intc_get_irq(void);
+
+#endif /* __KERNEL__ */
+#endif /* _ASM_POWERPC_XILINX_INTC_H */

Re: [PATCH 03/18] Virtex: add xilinx interrupt controller driver

[Olof Johansson]

On Fri, Sep 28, 2007 at 12:16:07PM -0600, Grant Likely wrote:

> +/*
> + * INTC Registers
> + */
> +#define ISR	0	/* Interrupt Status */
> +#define IPR	4	/* Interrupt Pending */
> +#define IER	8	/* Interrupt Enable */
> +#define IAR	12	/* Interrupt Acknowledge */
> +#define SIE	16	/* Set Interrupt Enable bits */
> +#define CIE	20	/* Clear Interrupt Enable bits */
> +#define IVR	24	/* Interrupt Vector */
> +#define MER	28	/* Master Enable */

The defines are fairly generic, I guess you haven't ran across cases
where there's naming conflicts, but you might want to prefix them with
something more unique just in case.

> +static struct irq_host *master_irqhost;
> +
> +/*
> + * IRQ Chip operations
> + */
> +static void xilinx_intc_mask(unsigned int virq)
> +{
> +	int irq = irq_map[virq].hwirq;
> +	void * regs = get_irq_chip_data(virq);
> +	pr_debug("mask: %d\n", irq);
> +	out_be32(regs + CIE, 1 << irq);
> +}
> +
> +static void xilinx_intc_unmask(unsigned int virq)
> +{
> +	int irq = irq_map[virq].hwirq;
> +	void * regs = get_irq_chip_data(virq);
> +	pr_debug("unmask: %d\n", irq);
> +	out_be32(regs + SIE, 1 << irq);
> +}
> +
> +static void xilinx_intc_ack(unsigned int virq)
> +{
> +	int irq = irq_map[virq].hwirq;
> +	void * regs = get_irq_chip_data(virq);
> +	pr_debug("ack: %d\n", irq);
> +	out_be32(regs + IAR, 1 << irq);
> +}

I guess some of the above are open-coded instead of using virq_to_hw()
for performance reasons, it could be useful to have comments regarding
this so they aren't changed by some janitor down the road. Or, in case
they're not performance-critical, change them to use virq_to_hw.


-Olof

Re: [PATCH 03/18] Virtex: add xilinx interrupt controller driver

[Grant Likely]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:16:07PM -0600, Grant Likely wrote:
>
> > +/*
> > + * INTC Registers
> > + */
> > +#define ISR  0       /* Interrupt Status */
> > +#define IPR  4       /* Interrupt Pending */
> > +#define IER  8       /* Interrupt Enable */
> > +#define IAR  12      /* Interrupt Acknowledge */
> > +#define SIE  16      /* Set Interrupt Enable bits */
> > +#define CIE  20      /* Clear Interrupt Enable bits */
> > +#define IVR  24      /* Interrupt Vector */
> > +#define MER  28      /* Master Enable */
>
> The defines are fairly generic, I guess you haven't ran across cases
> where there's naming conflicts, but you might want to prefix them with
> something more unique just in case.

Will do

>
> > +static void xilinx_intc_ack(unsigned int virq)
> > +{
> > +     int irq = irq_map[virq].hwirq;
> > +     void * regs = get_irq_chip_data(virq);
> > +     pr_debug("ack: %d\n", irq);
> > +     out_be32(regs + IAR, 1 << irq);
> > +}
>
> I guess some of the above are open-coded instead of using virq_to_hw()
> for performance reasons, it could be useful to have comments regarding
> this so they aren't changed by some janitor down the road. Or, in case
> they're not performance-critical, change them to use virq_to_hw.

Or it was just that my example code from another driver wasn't using
virq_to_hw() either.  I'll fix this.

g.

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

[PATCH 04/18] Xilinx Virtex: Add generic virtex board support

[Grant Likely]

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

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

 arch/powerpc/platforms/40x/Makefile |    1 +
 arch/powerpc/platforms/40x/virtex.c |   50 +++++++++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Makefile b/arch/powerpc/platforms/40x/Makefile
index e6c0bbd..0a3cfe9 100644
--- a/arch/powerpc/platforms/40x/Makefile
+++ b/arch/powerpc/platforms/40x/Makefile
@@ -1 +1,2 @@
 obj-$(CONFIG_WALNUT) += walnut.o
+obj-$(CONFIG_XILINX_VIRTEX_GENERIC_BOARD) += virtex.o
diff --git a/arch/powerpc/platforms/40x/virtex.c b/arch/powerpc/platforms/40x/virtex.c
new file mode 100644
index 0000000..ede982c
--- /dev/null
+++ b/arch/powerpc/platforms/40x/virtex.c
@@ -0,0 +1,50 @@
+/*
+ * Xilinx Virtex (IIpro & 4FX) based board support
+ *
+ * Copyright 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
+ * kind, whether express or implied.
+ */
+
+#include <linux/init.h>
+#include <asm/machdep.h>
+#include <asm/prom.h>
+#include <asm/time.h>
+#include <asm/xilinx_intc.h>
+#include <asm/of_platform.h>
+
+static int __init virtex_device_probe(void)
+{
+	if (!machine_is(virtex))
+		return 0;
+
+	of_platform_bus_probe(NULL, NULL, NULL);
+
+	return 0;
+}
+device_initcall(virtex_device_probe);
+
+static int __init virtex_probe(void)
+{
+	unsigned long root = of_get_flat_dt_root();
+
+	if (!of_flat_dt_is_compatible(root, "xilinx,virtex"))
+		return 0;
+
+	return 1;
+}
+
+static void __init virtex_setup_arch(void)
+{
+}
+
+define_machine(virtex) {
+	.name			= "Xilinx Virtex",
+	.probe			= virtex_probe,
+	.setup_arch		= virtex_setup_arch,
+	.init_IRQ		= xilinx_intc_init_tree,
+	.get_irq		= xilinx_intc_get_irq,
+	.calibrate_decr		= generic_calibrate_decr,
+};

[PATCH 05/18] Add PowerPC Xilinx Virtex entry to maintainers

[Grant Likely]

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

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

Paul, is this okay by you?  Josh has already okayed it.

Cheers,
g.

 MAINTAINERS |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8f80068..ea4ff15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2304,6 +2304,13 @@ L:	linuxppc-embedded@ozlabs.org
 T:	git kernel.org:/pub/scm/linux/kernel/git/jwboyer/powerpc.git
 S:	Maintained
 
+LINUX FOR POWERPC EMBEDDED XILINX VIRTEX
+P:	Grant Likely
+M:	grant.likely@secretlab.ca
+W:	http://www.secretlab.ca/
+L:	linuxppc-embedded@ozlabs.org
+S:	Maintained
+
 LINUX FOR POWERPC BOOT CODE
 P:	Tom Rini
 M:	trini@kernel.crashing.org

Re: [PATCH 05/18] Add PowerPC Xilinx Virtex entry to maintainers

[Grant Likely]

On 9/28/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
>
> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> ---
>
> Paul, is this okay by you?  Josh has already okayed it.

Specifically, I'll collect the virtex changes and ask Josh to pull
them from me before requesting a pull from you.

Cheers,
g.

> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8f80068..ea4ff15 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2304,6 +2304,13 @@ L:       linuxppc-embedded@ozlabs.org
>  T:     git kernel.org:/pub/scm/linux/kernel/git/jwboyer/powerpc.git
>  S:     Maintained
>
> +LINUX FOR POWERPC EMBEDDED XILINX VIRTEX
> +P:     Grant Likely
> +M:     grant.likely@secretlab.ca
> +W:     http://www.secretlab.ca/
> +L:     linuxppc-embedded@ozlabs.org
> +S:     Maintained
> +
>  LINUX FOR POWERPC BOOT CODE
>  P:     Tom Rini
>  M:     trini@kernel.crashing.org
>
>


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

Re: [PATCH 05/18] Add PowerPC Xilinx Virtex entry to maintainers

[Paul Mackerras]

Grant Likely writes:
> On 9/28/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
> > ---
> >
> > Paul, is this okay by you?  Josh has already okayed it.
> 
> Specifically, I'll collect the virtex changes and ask Josh to pull
> them from me before requesting a pull from you.

Sounds fine to me.

Paul.

Re: [PATCH 05/18] Add PowerPC Xilinx Virtex entry to maintainers

[Peter Korsgaard]

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

Hi,
 
 Grant> +LINUX FOR POWERPC EMBEDDED XILINX VIRTEX
 Grant> +P:	Grant Likely
 Grant> +M:	grant.likely@secretlab.ca
 Grant> +W:	http://www.secretlab.ca/

That page doesn't have any Xilinx info. What about 
http://wiki.secretlab.ca/index.php/Linux_on_Xilinx_Virtex instead?

-- 
Bye, Peter Korsgaard

[PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Grant Likely]

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

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);

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Olof Johansson]

On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:
> From: Grant Likely <grant.likely@secretlab.ca>
> 
> 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);

Hmm, I see the start changed, and you're now reading/writing a full
32-bit word instead of individual bytes. Still, looks a little fishy to
me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
3 higher and still read/write bytes?

Or are the registers defined as 32-bit ones? (I don't remember, it was
so long since I touched uartlite myself. :-)

(Same for the other functions below, but the general principle applies.)

Also, I'm not sure you need to cast port->membase to void*, do you? The
math will still be right since it's declared as char *.


-Olof

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Grant Likely]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 12:17:13PM -0600, Grant Likely wrote:
> > From: Grant Likely <grant.likely@secretlab.ca>
> >
> > 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);
>
> Hmm, I see the start changed, and you're now reading/writing a full
> 32-bit word instead of individual bytes. Still, looks a little fishy to
> me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
> 3 higher and still read/write bytes?
>
> Or are the registers defined as 32-bit ones? (I don't remember, it was
> so long since I touched uartlite myself. :-)

All the registers are defined as 32 bit ones.  I think it makes more
sense to access the registers as they are documented, and it
eliminates the 'magic' +3 needed to make it work now.

>
> (Same for the other functions below, but the general principle applies.)
>
> Also, I'm not sure you need to cast port->membase to void*, do you? The
> math will still be right since it's declared as char *.

membase is now defined as u32*, so the cast is needed.

Cheers,
g.

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

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Olof Johansson]

On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
>
> > Hmm, I see the start changed, and you're now reading/writing a full
> > 32-bit word instead of individual bytes. Still, looks a little fishy to
> > me. Wouldn't it be more appropriate to change the ULITE_RX offset to be
> > 3 higher and still read/write bytes?
> >
> > Or are the registers defined as 32-bit ones? (I don't remember, it was
> > so long since I touched uartlite myself. :-)
> 
> All the registers are defined as 32 bit ones.  I think it makes more
> sense to access the registers as they are documented, and it
> eliminates the 'magic' +3 needed to make it work now.

Ok, thaks for the clarification. Feel free to add it as motivation in
the patch description. :)

> > (Same for the other functions below, but the general principle applies.)
> >
> > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > math will still be right since it's declared as char *.
> 
> membase is now defined as u32*, so the cast is needed.

Hm, I must have looked at a stale tree.


Thanks,

-Olof

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Grant Likely]

On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> > On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > > math will still be right since it's declared as char *.
> >
> > membase is now defined as u32*, so the cast is needed.
>
> Hm, I must have looked at a stale tree.

No, wait.  You're right.  It is a char*.  I'll drop the cast.

g.

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

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Grant Likely]

On 9/28/07, Grant Likely <grant.likely@secretlab.ca> wrote:
> On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > On Fri, Sep 28, 2007 at 02:42:32PM -0600, Grant Likely wrote:
> > > On 9/28/07, Olof Johansson <olof@lixom.net> wrote:
> > > > Also, I'm not sure you need to cast port->membase to void*, do you? The
> > > > math will still be right since it's declared as char *.
> > >
> > > membase is now defined as u32*, so the cast is needed.
> >
> > Hm, I must have looked at a stale tree.
>
> No, wait.  You're right.  It is a char*.  I'll drop the cast.

Wait, I'm wrong again... it's in/out_be32 that expects an (unsigned*).
 The compiler complains without the cast.

g.

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

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Peter Korsgaard]

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

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

Huh? This seems a bit confused. Microblaze is big endian and out_be32
is only defined in arch/p{,ower}pc.

This has been discussed before. The logical registers are 8bit, but
the OPB implementation (by virtue of it being a 32bit bus) uses
32bit spacing between the registers.

The fact is that the only portable access to non-pci registers in the
kernel is 8 bit.

The uartlite driver is ofcause primarily used to drive Xilinx
OPB_Uartlite IP blocks, but that's not the only use - E.G. we are
using another simple UART with the same hardware interface but sitting
on a 16bit bus. With the current driver this works fine, but won't
with the out_be32.

For a new design wi'll use an AT91 (arm). I don't see any reason why
we shouldn't be able to use the same UART block there.

 Grant> ---

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

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


-- 
Bye, Peter Korsgaard

Re: [PATCH 06/18] [POWERPC] Fix UARTLITE reg io for little-endian architectures (ie. microblaze)

[Grant Likely]

On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> The uartlite driver is ofcause primarily used to drive Xilinx
> OPB_Uartlite IP blocks, but that's not the only use - E.G. we are
> using another simple UART with the same hardware interface but sitting
> on a 16bit bus. With the current driver this works fine, but won't
> with the out_be32.

Ugh.  Alright I'll revert the patch.

Cheers,
g.

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

[PATCH 07/18] 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;

Re: [PATCH 07/18] 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

Ok.

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

-- 
Bye, Peter Korsgaard

[PATCH 08/18] Uartlite: Add macro for uartlite device name

[Grant Likely]

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

Changed to make the OF bus binding a wee bit cleaner

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

 arch/powerpc/platforms/40x/Kconfig |    4 ++--
 drivers/serial/uartlite.c          |    5 +++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
index 1aae0e6..44f08dd 100644
--- a/arch/powerpc/platforms/40x/Kconfig
+++ b/arch/powerpc/platforms/40x/Kconfig
@@ -65,8 +65,8 @@ config XILINX_VIRTEX_GENERIC_BOARD
 	bool "Generic Xilinx Virtex board"
 	depends on 40x
 	default y
-	select VIRTEX_II_PRO
-	select VIRTEX_4_FX
+	select XILINX_VIRTEX_II_PRO
+	select XILINX_VIRTEX_4_FX
 	help
 	  This option enables generic support for Xilinx Virtex based boards.
 
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,

Re: [PATCH 08/18] Uartlite: Add macro for uartlite device name

[Peter Korsgaard]

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

Hi,

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

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

 Grant>  arch/powerpc/platforms/40x/Kconfig |    4 ++--
 Grant>  drivers/serial/uartlite.c          |    5 +++--
 Grant>  2 files changed, 5 insertions(+), 4 deletions(-)

 Grant> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
 Grant> index 1aae0e6..44f08dd 100644
 Grant> --- a/arch/powerpc/platforms/40x/Kconfig
 Grant> +++ b/arch/powerpc/platforms/40x/Kconfig
 Grant> @@ -65,8 +65,8 @@ config XILINX_VIRTEX_GENERIC_BOARD
 Grant>  	bool "Generic Xilinx Virtex board"
 Grant>  	depends on 40x
 Grant>  	default y
 Grant> -	select VIRTEX_II_PRO
 Grant> -	select VIRTEX_4_FX
 Grant> +	select XILINX_VIRTEX_II_PRO
 Grant> +	select XILINX_VIRTEX_4_FX

Huh? What does this have to do with $SUBJ?

 Grant>  	help
 Grant>  	  This option enables generic support for Xilinx Virtex based boards.
 
 Grant> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
 Grant> index ae05a67..10e0da9 100644
 Grant> --- a/drivers/serial/uartlite.c
 Grant> +++ b/drivers/serial/uartlite.c
 Grant> @@ -18,6 +18,7 @@
 Grant>  #include <linux/interrupt.h>
 Grant>  #include <asm/io.h>
 
 Grant> +#define ULITE_NAME		"ttyUL"
 Grant>  #define ULITE_MAJOR		204
 Grant>  #define ULITE_MINOR		187
 Grant>  #define ULITE_NR_UARTS		4

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

-- 
Bye, Peter Korsgaard

Re: [PATCH 08/18] Uartlite: Add macro for uartlite device name

[Grant Likely]

On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>
> Hi,
>
>  Grant> From: Grant Likely <grant.likely@secretlab.ca>
>  Grant> Changed to make the OF bus binding a wee bit cleaner
>
>  Grant> Signed-off-by: Grant Likely <grant.likely@secretlab.ca>
>  Grant> ---
>
>  Grant>  arch/powerpc/platforms/40x/Kconfig |    4 ++--
>  Grant>  drivers/serial/uartlite.c          |    5 +++--
>  Grant>  2 files changed, 5 insertions(+), 4 deletions(-)
>
>  Grant> diff --git a/arch/powerpc/platforms/40x/Kconfig b/arch/powerpc/platforms/40x/Kconfig
>  Grant> index 1aae0e6..44f08dd 100644
>  Grant> --- a/arch/powerpc/platforms/40x/Kconfig
>  Grant> +++ b/arch/powerpc/platforms/40x/Kconfig
>  Grant> @@ -65,8 +65,8 @@ config XILINX_VIRTEX_GENERIC_BOARD
>  Grant>         bool "Generic Xilinx Virtex board"
>  Grant>         depends on 40x
>  Grant>         default y
>  Grant> -       select VIRTEX_II_PRO
>  Grant> -       select VIRTEX_4_FX
>  Grant> +       select XILINX_VIRTEX_II_PRO
>  Grant> +       select XILINX_VIRTEX_4_FX
>
> Huh? What does this have to do with $SUBJ?

This was a mess up on my part on this version of the patch set.  v3
has it fixed.

g.

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

[PATCH 09/18] Uartlite: Separate the bus binding from the driver proper

[Grant Likely]

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

In preparation of adding the of_platform_bus bindings

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 10/18] Uartlite: improve in-code comments

[Grant Likely]

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

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;

Re: [PATCH 10/18] Uartlite: improve in-code comments

[Arnd Bergmann]

On Friday 28 September 2007, Grant Likely wrote:
> +/* ---------------------------------------------------------------------
> + * Core UART driver operations
> + */
> +

This is a rather unusual style of commenting. IMHO it would be better if you
left out the ----- line.

	Arnd <><

Re: [PATCH 10/18] Uartlite: improve in-code comments

[Grant Likely]

On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +/* ---------------------------------------------------------------------
> > + * Core UART driver operations
> > + */
> > +
>
> This is a rather unusual style of commenting. IMHO it would be better if you
> left out the ----- line.

I find the horizontal breaks useful when parsing through the code.  If
others agree with you, then I'll happily remove them.

g.

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

Re: [PATCH 10/18] Uartlite: improve in-code comments

[Peter Korsgaard]

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

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

Fine by me.

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

-- 
Bye, Peter Korsgaard

[PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus

[Grant Likely]

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

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..8f742e0 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 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 = 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 __init 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 __init 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);
 }
 

Re: [PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus

[Arnd Bergmann]

T24gRnJpZGF5IDI4IFNlcHRlbWJlciAyMDA3LCBHcmFudCBMaWtlbHkgd3JvdGU6Cj4gLaCgoKCg
oKByZXQgPSB1YXJ0X3JlZ2lzdGVyX2RyaXZlcigmdWxpdGVfdWFydF9kcml2ZXIpOwo+IC2goKCg
oKCgaWYgKHJldCkKPiAtoKCgoKCgoKCgoKCgoKCgcmV0dXJuIHJldDsKPiAroKCgoKCgoHByX2Rl
YnVnKCJ1YXJ0bGl0ZTogY2FsbGluZyB1YXJ0X3JlZ2lzdGVyX2RyaXZlcigpXG4iKTsKPiAroKCg
oKCgoGlmICgocmV0ID0gdWFydF9yZWdpc3Rlcl9kcml2ZXIoJnVsaXRlX3VhcnRfZHJpdmVyKSkg
IT0gMCkKPiAroKCgoKCgoKCgoKCgoKCgZ290byBlcnJfdWFydDsKPiCgCj4gLaCgoKCgoKByZXQg
PSBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoJnVsaXRlX3BsYXRmb3JtX2RyaXZlcik7Cj4gLaCg
oKCgoKBpZiAocmV0KQo+IC2goKCgoKCgoKCgoKCgoKB1YXJ0X3VucmVnaXN0ZXJfZHJpdmVyKCZ1
bGl0ZV91YXJ0X2RyaXZlcik7Cj4gK6CgoKCgoKBpZiAoKHJldCA9IHVsaXRlX29mX3JlZ2lzdGVy
KCkpICE9IDApCj4gK6CgoKCgoKCgoKCgoKCgoGdvdG8gZXJyX29mOwo+IKAKPiAroKCgoKCgoHBy
X2RlYnVnKCJ1YXJ0bGl0ZTogY2FsbGluZyBwbGF0Zm9ybV9kcml2ZXJfcmVnaXN0ZXIoKVxuIik7
Cj4gK6CgoKCgoKBpZiAoKHJldCA9IHBsYXRmb3JtX2RyaXZlcl9yZWdpc3RlcigmdWxpdGVfcGxh
dGZvcm1fZHJpdmVyKSkgIT0gMCkKPiAroKCgoKCgoKCgoKCgoKCgZ290byBlcnJfcGxhdDsKPiAr
Cj4gK6CgoKCgoKByZXR1cm4gMDsKCkkgZG9uJ3Qgc2VlIHRoaXMgYXMgbXVjaCBvZiBhbiBpbXBy
b3ZlbWVudC4gRG9pbmcgYW4gYXNzaWdubWVudCBpbiB0aGUKY29uZGl0aW9uIG1ha2VzIHRoZSBj
b2RlIGxlc3MgcmVhZGFibGUgSU1ITywgc28gaXQgc2hvdWxkIHJlYWxseQpiZSB3cml0dGVuIGFz
CgoJcmV0ID0gdHJ5X3NvbWV0aGluZygpOwoJaWYgKHJldCkKCQlnb3RvIHNvbWV0aGluZ19mYWls
ZWQ7CgpUaGlzIGtlZXBzIHRoZSBub3JtYWwgY29kZSB0byB0aGUgbGVmdCwgYW5kIHRoZSBlcnJv
ciBoYW5kbGluZyBpbmRlbnRlZAp0byB0aGUgcmlnaHQuCgoKCUFybmQgPD48Cg==

Re: [PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus

[Peter Korsgaard]

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

Hi,

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

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

 Grant> diff --git a/drivers/serial/uartlite.c b/drivers/serial/uartlite.c
 Grant> index ed13b9f..8f742e0 100644
 Grant> --- a/drivers/serial/uartlite.c
 Grant> +++ b/drivers/serial/uartlite.c
 Grant> @@ -1,7 +1,8 @@
 Grant>  /*
 Grant>   * uartlite.c: Serial driver for Xilinx uartlite serial controller
 Grant>   *
 Grant> - * Peter Korsgaard <jacmet@sunsite.dk>
 Grant> + * Copyright (C) 2006 Peter Korsgaard <jacmet@sunsite.dk>
 Grant> + * Copyright (C) 2007 Secret Lab Technologies Ltd.
 Grant>   *
 Grant>   * This file is licensed under the terms of the GNU General Public License
 Grant>   * version 2.  This program is licensed "as is" without any warranty of any
 Grant> @@ -17,6 +18,10 @@
 Grant>  #include <linux/delay.h>
 Grant>  #include <linux/interrupt.h>
 Grant>  #include <asm/io.h>
 Grant> +#if defined(CONFIG_OF)
 Grant> +#include <linux/of_device.h>
 Grant> +#include <linux/of_platform.h>
 Grant> +#endif
 
 Grant>  #define ULITE_NAME		"ttyUL"
 Grant>  #define ULITE_MAJOR		204
 Grant> @@ -382,8 +387,10 @@ static int __init ulite_console_setup(struct console *co, char *options)
 Grant>  	port = &ulite_ports[co->index];
 
 Grant>  	/* not initialized yet? */
 Grant> -	if (!port->membase)
 Grant> +	if (!port->membase) {
 Grant> +		pr_debug("console on ttyUL%i not initialized\n", co->index);
 Grant>  		return -ENODEV;
 Grant> +	}

Unrelated change.
 
 Grant>  	if (options)
 Grant>  		uart_parse_options(options, &baud, &parity, &bits, &flow);
 Grant> @@ -542,6 +549,72 @@ static struct platform_driver ulite_platform_driver = {
 Grant>  };
 
 Grant>  /* ---------------------------------------------------------------------
 Grant> + * OF bus bindings
 Grant> + */
 Grant> +#if defined(CONFIG_OF)
 Grant> +static int __devinit
 Grant> +ulite_of_probe(struct of_device *op, const struct of_device_id *match)
 Grant> +{
 Grant> +	struct resource res;
 Grant> +	const unsigned int *id;
 Grant> +	int irq, rc;
 Grant> +
 Grant> +	dev_dbg(&op->dev, "%s(%p, %p)\n", __FUNCTION__, op, match);
 Grant> +
 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> +
 Grant> +	irq = irq_of_parse_and_map(op->node, 0);
 Grant> +
 Grant> +	id = of_get_property(op->node, "port-number", NULL);
 Grant> +
 Grant> +	return ulite_assign(&op->dev, id ? *id : -1, res.start, irq);
 Grant> +}
 Grant> +
 Grant> +static int ulite_of_remove(struct of_device *op)
 Grant> +{
 Grant> +	return ulite_release(&op->dev);
 Grant> +}
 Grant> +
 Grant> +/* Match table for of_platform binding */
 Grant> +static struct of_device_id __devinit ulite_of_match[] = {
 Grant> +	{ .type = "serial", .compatible = "xilinx,uartlite", },
 Grant> +	{},
 Grant> +};
 Grant> +MODULE_DEVICE_TABLE(of, ulite_of_match);
 Grant> +
 Grant> +static struct of_platform_driver ulite_of_driver = {
 Grant> +	.owner = THIS_MODULE,
 Grant> +	.name = "uartlite",
 Grant> +	.match_table = ulite_of_match,
 Grant> +	.probe = ulite_of_probe,
 Grant> +	.remove = ulite_of_remove,
 Grant> +	.driver = {
 Grant> +		.name = "uartlite",
 Grant> +	},
 Grant> +};
 Grant> +
 Grant> +/* Registration helpers to keep the number of #ifdefs to a minimum */
 Grant> +static inline int __init ulite_of_register(void)
 Grant> +{
 Grant> +	pr_debug("uartlite: calling of_register_platform_driver()\n");
 Grant> +	return of_register_platform_driver(&ulite_of_driver);
 Grant> +}
 Grant> +
 Grant> +static inline void __init ulite_of_unregister(void)
 Grant> +{
 Grant> +	of_unregister_platform_driver(&ulite_of_driver);
 Grant> +}
 Grant> +#else /* CONFIG_OF */
 Grant> +/* CONFIG_OF not enabled; do nothing helpers */
 Grant> +static inline int __init ulite_of_register(void) { return 0; }
 Grant> +static inline void __init ulite_of_unregister(void) { }
 Grant> +#endif /* CONFIG_OF */
 Grant> +
 Grant> +/* ---------------------------------------------------------------------
 Grant>   * Module setup/teardown
 Grant>   */
 
 Grant> @@ -549,20 +622,32 @@ int __init ulite_init(void)
 Grant>  {
 Grant>  	int ret;
 
 Grant> -	ret = uart_register_driver(&ulite_uart_driver);
 Grant> -	if (ret)
 Grant> -		return ret;
 Grant> +	pr_debug("uartlite: calling uart_register_driver()\n");
 Grant> +	if ((ret = uart_register_driver(&ulite_uart_driver)) != 0)
 Grant> +		goto err_uart;
 
 Grant> -	ret = platform_driver_register(&ulite_platform_driver);
 Grant> -	if (ret)
 Grant> -		uart_unregister_driver(&ulite_uart_driver);
 Grant> +	if ((ret = ulite_of_register()) != 0)
 Grant> +		goto err_of;
 
 Grant> +	pr_debug("uartlite: calling platform_driver_register()\n");
 Grant> +	if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)

I prefer to not have assignments in the if ().
Are all the pr_debug necessary? It looks quite messy.

 Grant> +		goto err_plat;
 Grant> +

 Grant> +	return 0;
 Grant> +
 Grant> +err_plat:
 Grant> +	ulite_of_unregister();
 Grant> +err_of:
 Grant> +	uart_unregister_driver(&ulite_uart_driver);
 Grant> +err_uart:
 Grant> +	printk(KERN_ERR "registering uartlite driver failed: err=%i", ret);
 Grant>  	return ret;
 Grant>  }
 
 Grant>  void __exit ulite_exit(void)
 Grant>  {
 Grant>  	platform_driver_unregister(&ulite_platform_driver);
 Grant> +	ulite_of_unregister();
 Grant>  	uart_unregister_driver(&ulite_uart_driver);
 Grant>  }
 


-- 
Bye, Peter Korsgaard

Re: [PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus

[Grant Likely]

On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
>  Grant> +       pr_debug("uartlite: calling platform_driver_register()\n");
>  Grant> +       if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
>
> I prefer to not have assignments in the if ().

Already fixed in v3

> Are all the pr_debug necessary? It looks quite messy.

Maybe messy, but *very* useful.  Looks prettier in the v3 version with
the assignment and if() on separate lines.

Cheers,
g.

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

Re: [PATCH 11/18] Virtex: Port UARTLITE driver to of-platform-bus

[Peter Korsgaard]

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

Hi,

 Grant> On 10/2/07, Peter Korsgaard <jacmet@sunsite.dk> wrote:
 >> >>>>> "Grant" == Grant Likely <grant.likely@secretlab.ca> writes:
 Grant> +       pr_debug("uartlite: calling platform_driver_register()\n");
 Grant> +       if ((ret = platform_driver_register(&ulite_platform_driver)) != 0)
 >> 
 >> I prefer to not have assignments in the if ().

 Grant> Already fixed in v3

Ok, thanks.

 >> Are all the pr_debug necessary? It looks quite messy.

 Grant> Maybe messy, but *very* useful.  Looks prettier in the v3 version with
 Grant> the assignment and if() on separate lines.

Ok, I'll take a look (sorry, I'm behind on mails..)

-- 
Bye, Peter Korsgaard

[PATCH 12/18] Uartlite: Let the console be initialized earlier

[Grant Likely]

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

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 8f742e0..4b0bf0e 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 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 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 12/18] Uartlite: Let the console be initialized earlier

[Arnd Bergmann]

On Friday 28 September 2007, Grant Likely wrote:
> +#else /* CONFIG_OF */
> +static void __init ulite_console_of_find_device(int id) { /* do nothing */ }
> +#endif /* CONFIG_OF */

Shouldn't this be inline? It shouldn't matter much since most of the time
gcc -funit-at-a-time takes care of this, but it's common to make the inlining
explicit.

	Arnd <><

Re: [PATCH 12/18] Uartlite: Let the console be initialized earlier

[Grant Likely]

On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> On Friday 28 September 2007, Grant Likely wrote:
> > +#else /* CONFIG_OF */
> > +static void __init ulite_console_of_find_device(int id) { /* do nothing */ }
> > +#endif /* CONFIG_OF */
>
> Shouldn't this be inline? It shouldn't matter much since most of the time
> gcc -funit-at-a-time takes care of this, but it's common to make the inlining
> explicit.

heh, I even had the inline in there on an earlier version of the
patch.  I can add it back it.

g.

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

[PATCH 13/18] Add Xilinx SystemACE entry to maintainers

[Grant Likely]

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

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

 MAINTAINERS |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index ea4ff15..759cc40 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4186,6 +4186,13 @@ W:	http://oss.sgi.com/projects/xfs
 T:	git git://oss.sgi.com:8090/xfs/xfs-2.6.git
 S:	Supported
 
+XILINX SYSTEMACE DRIVER
+P:	Grant Likely
+M:	grant.likely@secretlab.ca
+W:	http://www.secretlab.ca/
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+
 XILINX UARTLITE SERIAL DRIVER
 P:	Peter Korsgaard
 M:	jacmet@sunsite.dk

[PATCH 14/18] Sysace: Use the established platform bus api

[Grant Likely]

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

SystemACE uses the platform bus binding, but it doesn't use the
platform bus API.  Move to using the correct API for consistency
sake.

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

 drivers/block/xsysace.c |   48 +++++++++++++++++++++++++++++------------------
 1 files changed, 30 insertions(+), 18 deletions(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 3ede0b6..b104476 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -1060,13 +1060,12 @@ static void __devexit ace_teardown(struct ace_device *ace)
  * Platform Bus Support
  */
 
-static int __devinit ace_probe(struct device *device)
+static int __devinit ace_probe(struct platform_device *dev)
 {
-	struct platform_device *dev = to_platform_device(device);
 	struct ace_device *ace;
 	int i;
 
-	dev_dbg(device, "ace_probe(%p)\n", device);
+	dev_dbg(&dev->dev, "ace_probe(%p)\n", dev);
 
 	/*
 	 * Allocate the ace device structure
@@ -1075,7 +1074,7 @@ static int __devinit ace_probe(struct device *device)
 	if (!ace)
 		goto err_alloc;
 
-	ace->dev = device;
+	ace->dev = &dev->dev;
 	ace->id = dev->id;
 	ace->irq = NO_IRQ;
 
@@ -1089,7 +1088,7 @@ static int __devinit ace_probe(struct device *device)
 	/* FIXME: Should get bus_width from the platform_device struct */
 	ace->bus_width = 1;
 
-	dev_set_drvdata(&dev->dev, ace);
+	platform_set_drvdata(dev, ace);
 
 	/* Call the bus-independant setup code */
 	if (ace_setup(ace) != 0)
@@ -1098,7 +1097,7 @@ static int __devinit ace_probe(struct device *device)
 	return 0;
 
       err_setup:
-	dev_set_drvdata(&dev->dev, NULL);
+	platform_set_drvdata(dev, NULL);
 	kfree(ace);
       err_alloc:
 	printk(KERN_ERR "xsysace: could not initialize device\n");
@@ -1108,25 +1107,27 @@ static int __devinit ace_probe(struct device *device)
 /*
  * Platform bus remove() method
  */
-static int __devexit ace_remove(struct device *device)
+static int __devexit ace_remove(struct platform_device *dev)
 {
-	struct ace_device *ace = dev_get_drvdata(device);
-
-	dev_dbg(device, "ace_remove(%p)\n", device);
+	struct ace_device *ace =  platform_get_drvdata(dev);
+	dev_dbg(&dev->dev, "ace_remove(%p)\n", dev);
 
 	if (ace) {
 		ace_teardown(ace);
+		platform_set_drvdata(dev, NULL);
 		kfree(ace);
 	}
 
 	return 0;
 }
 
-static struct device_driver ace_driver = {
-	.name = "xsysace",
-	.bus = &platform_bus_type,
+static struct platform_driver ace_platform_driver = {
 	.probe = ace_probe,
 	.remove = __devexit_p(ace_remove),
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "xsysace",
+	},
 };
 
 /* ---------------------------------------------------------------------
@@ -1134,20 +1135,31 @@ static struct device_driver ace_driver = {
  */
 static int __init ace_init(void)
 {
+	int rc;
+
 	ace_major = register_blkdev(ace_major, "xsysace");
 	if (ace_major <= 0) {
-		printk(KERN_WARNING "xsysace: register_blkdev() failed\n");
-		return ace_major;
+		rc = -ENOMEM;
+		goto err_blk;
 	}
 
-	pr_debug("Registering Xilinx SystemACE driver, major=%i\n", ace_major);
-	return driver_register(&ace_driver);
+	if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
+		goto err_plat;
+
+	pr_info("Xilinx SystemACE device driver, major=%i\n", ace_major);
+	return 0;
+
+      err_plat:
+	unregister_blkdev(ace_major, "xsysace");
+      err_blk:
+	printk(KERN_ERR "xsysace: registration failed; err=%i\n", rc);
+	return rc;
 }
 
 static void __exit ace_exit(void)
 {
 	pr_debug("Unregistering Xilinx SystemACE driver\n");
-	driver_unregister(&ace_driver);
+	platform_driver_unregister(&ace_platform_driver);
 	unregister_blkdev(ace_major, "xsysace");
 }
 

[PATCH 15/18] Sysace: Move structure allocation from bus binding into common code

[Grant Likely]

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

Split the determination of device registers/irqs/etc from the actual
allocation and initialization of the device structure.  This cleans
up the code a bit in preparation to add an of_platform bus binding

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

 drivers/block/xsysace.c |   99 +++++++++++++++++++++++++++++------------------
 1 files changed, 61 insertions(+), 38 deletions(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index b104476..555939b 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -1033,7 +1033,7 @@ static int __devinit ace_setup(struct ace_device *ace)
 	if (ace->irq != NO_IRQ)
 		free_irq(ace->irq, ace);
       err_ioremap:
-	printk(KERN_INFO "xsysace: error initializing device at 0x%lx\n",
+	dev_info(ace->dev, "xsysace: error initializing device at 0x%lx\n",
 	       ace->physaddr);
 	return -ENOMEM;
 }
@@ -1056,68 +1056,91 @@ static void __devexit ace_teardown(struct ace_device *ace)
 	iounmap(ace->baseaddr);
 }
 
-/* ---------------------------------------------------------------------
- * Platform Bus Support
- */
-
-static int __devinit ace_probe(struct platform_device *dev)
+static int __devinit
+ace_alloc(struct device *dev, int id, unsigned long physaddr,
+	  int irq, int bus_width)
 {
 	struct ace_device *ace;
-	int i;
+	int rc;
+	dev_dbg(dev, "ace_alloc(%p)\n", dev);
 
-	dev_dbg(&dev->dev, "ace_probe(%p)\n", dev);
+	if (!physaddr) {
+		rc = -ENODEV;
+		goto err_noreg;
+	}
 
-	/*
-	 * Allocate the ace device structure
-	 */
+	/* Allocate and initialize the ace device structure */
 	ace = kzalloc(sizeof(struct ace_device), GFP_KERNEL);
-	if (!ace)
+	if (!ace) {
+		rc = -ENOMEM;
 		goto err_alloc;
-
-	ace->dev = &dev->dev;
-	ace->id = dev->id;
-	ace->irq = NO_IRQ;
-
-	for (i = 0; i < dev->num_resources; i++) {
-		if (dev->resource[i].flags & IORESOURCE_MEM)
-			ace->physaddr = dev->resource[i].start;
-		if (dev->resource[i].flags & IORESOURCE_IRQ)
-			ace->irq = dev->resource[i].start;
 	}
 
-	/* FIXME: Should get bus_width from the platform_device struct */
-	ace->bus_width = 1;
-
-	platform_set_drvdata(dev, ace);
+	ace->dev = dev;
+	ace->id = id;
+	ace->physaddr = physaddr;
+	ace->irq = irq;
+	ace->bus_width = bus_width;
 
-	/* Call the bus-independant setup code */
-	if (ace_setup(ace) != 0)
+	/* Call the setup code */
+	if ((rc = ace_setup(ace)) != 0)
 		goto err_setup;
 
+	dev_set_drvdata(dev, ace);
 	return 0;
 
       err_setup:
-	platform_set_drvdata(dev, NULL);
+	dev_set_drvdata(dev, NULL);
 	kfree(ace);
       err_alloc:
-	printk(KERN_ERR "xsysace: could not initialize device\n");
-	return -ENOMEM;
+      err_noreg:
+	dev_err(dev, "could not initialize device, err=%i\n", rc);
+	return rc;
 }
 
-/*
- * Platform bus remove() method
- */
-static int __devexit ace_remove(struct platform_device *dev)
+static void __devexit ace_free(struct device *dev)
 {
-	struct ace_device *ace =  platform_get_drvdata(dev);
-	dev_dbg(&dev->dev, "ace_remove(%p)\n", dev);
+	struct ace_device *ace = dev_get_drvdata(dev);
+	dev_dbg(dev, "ace_free(%p)\n", dev);
 
 	if (ace) {
 		ace_teardown(ace);
-		platform_set_drvdata(dev, NULL);
+		dev_set_drvdata(dev, NULL);
 		kfree(ace);
 	}
+}
+
+/* ---------------------------------------------------------------------
+ * Platform Bus Support
+ */
+
+static int __devinit ace_probe(struct platform_device *dev)
+{
+	unsigned long physaddr = 0;
+	int bus_width = 1; /* FIXME: should not be hard coded */
+	int id = dev->id;
+	int irq = NO_IRQ;
+	int i;
+
+	dev_dbg(&dev->dev, "ace_probe(%p)\n", dev);
+
+	for (i = 0; i < dev->num_resources; i++) {
+		if (dev->resource[i].flags & IORESOURCE_MEM)
+			physaddr = dev->resource[i].start;
+		if (dev->resource[i].flags & IORESOURCE_IRQ)
+			irq = dev->resource[i].start;
+	}
+
+	/* Call the bus-independant setup code */
+	return ace_alloc(&dev->dev, id, physaddr, irq, bus_width);
+}
 
+/*
+ * Platform bus remove() method
+ */
+static int __devexit ace_remove(struct platform_device *dev)
+{
+	ace_free(&dev->dev);
 	return 0;
 }
 

[PATCH 16/18] Sysace: minor rework and cleanup changes

[Grant Likely]

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

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

 drivers/block/xsysace.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 555939b..10bb4e5 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -158,6 +158,9 @@ MODULE_LICENSE("GPL");
 #define ACE_FIFO_SIZE (32)
 #define ACE_BUF_PER_SECTOR (ACE_SECTOR_SIZE / ACE_FIFO_SIZE)
 
+#define ACE_BUS_WIDTH_8  0
+#define ACE_BUS_WIDTH_16 1
+
 struct ace_reg_ops;
 
 struct ace_device {
@@ -931,9 +934,11 @@ static int __devinit ace_setup(struct ace_device *ace)
 {
 	u16 version;
 	u16 val;
-
 	int rc;
 
+	dev_dbg(ace->dev, "ace_setup(ace=0x%p)\n", ace);
+	dev_dbg(ace->dev, "physaddr=0x%lx irq=%i\n", ace->physaddr, ace->irq);
+
 	spin_lock_init(&ace->lock);
 	init_completion(&ace->id_completion);
 
@@ -982,7 +987,7 @@ static int __devinit ace_setup(struct ace_device *ace)
 	snprintf(ace->gd->disk_name, 32, "xs%c", ace->id + 'a');
 
 	/* set bus width */
-	if (ace->bus_width == 1) {
+	if (ace->bus_width == ACE_BUS_WIDTH_16) {
 		/* 0x0101 should work regardless of endianess */
 		ace_out_le16(ace, ACE_BUSMODE, 0x0101);
 
@@ -1117,7 +1122,7 @@ static void __devexit ace_free(struct device *dev)
 static int __devinit ace_probe(struct platform_device *dev)
 {
 	unsigned long physaddr = 0;
-	int bus_width = 1; /* FIXME: should not be hard coded */
+	int bus_width = ACE_BUS_WIDTH_16; /* FIXME: should not be hard coded */
 	int id = dev->id;
 	int irq = NO_IRQ;
 	int i;
@@ -1166,6 +1171,7 @@ static int __init ace_init(void)
 		goto err_blk;
 	}
 
+	pr_debug("xsysace: registering platform binding\n");
 	if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
 		goto err_plat;
 

[PATCH 17/18] Sysace: Move IRQ handler registration to occur after FSM is initialized

[Grant Likely]

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

The FSM needs to be initialized before it is safe to call the ISR

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

 drivers/block/xsysace.c |   21 ++++++++++-----------
 1 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 10bb4e5..296d567 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -949,15 +949,6 @@ static int __devinit ace_setup(struct ace_device *ace)
 	if (!ace->baseaddr)
 		goto err_ioremap;
 
-	if (ace->irq != NO_IRQ) {
-		rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace);
-		if (rc) {
-			/* Failure - fall back to polled mode */
-			dev_err(ace->dev, "request_irq failed\n");
-			ace->irq = NO_IRQ;
-		}
-	}
-
 	/*
 	 * Initialize the state machine tasklet and stall timer
 	 */
@@ -1015,6 +1006,16 @@ static int __devinit ace_setup(struct ace_device *ace)
 	val |= ACE_CTRL_DATABUFRDYIRQ | ACE_CTRL_ERRORIRQ;
 	ace_out(ace, ACE_CTRL, val);
 
+	/* Now we can hook up the irq handler */
+	if (ace->irq != NO_IRQ) {
+		rc = request_irq(ace->irq, ace_interrupt, 0, "systemace", ace);
+		if (rc) {
+			/* Failure - fall back to polled mode */
+			dev_err(ace->dev, "request_irq failed\n");
+			ace->irq = NO_IRQ;
+		}
+	}
+
 	/* Print the identification */
 	dev_info(ace->dev, "Xilinx SystemACE revision %i.%i.%i\n",
 		 (version >> 12) & 0xf, (version >> 8) & 0x0f, version & 0xff);
@@ -1035,8 +1036,6 @@ static int __devinit ace_setup(struct ace_device *ace)
 	blk_cleanup_queue(ace->queue);
       err_blk_initq:
 	iounmap(ace->baseaddr);
-	if (ace->irq != NO_IRQ)
-		free_irq(ace->irq, ace);
       err_ioremap:
 	dev_info(ace->dev, "xsysace: error initializing device at 0x%lx\n",
 	       ace->physaddr);

[PATCH 18/18] xsysace: Add of_platform_bus binding

[Grant Likely]

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

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

 drivers/block/xsysace.c |   89 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 89 insertions(+), 0 deletions(-)

diff --git a/drivers/block/xsysace.c b/drivers/block/xsysace.c
index 296d567..a68301a 100644
--- a/drivers/block/xsysace.c
+++ b/drivers/block/xsysace.c
@@ -91,6 +91,10 @@
 #include <linux/blkdev.h>
 #include <linux/hdreg.h>
 #include <linux/platform_device.h>
+#if defined(CONFIG_OF)
+#include <linux/of_device.h>
+#include <linux/of_platform.h>
+#endif
 
 MODULE_AUTHOR("Grant Likely <grant.likely@secretlab.ca>");
 MODULE_DESCRIPTION("Xilinx SystemACE device driver");
@@ -1158,6 +1162,85 @@ static struct platform_driver ace_platform_driver = {
 };
 
 /* ---------------------------------------------------------------------
+ * OF_Platform Bus Support
+ */
+
+#if defined(CONFIG_OF)
+static int __devinit
+ace_of_probe(struct of_device *op, const struct of_device_id *match)
+{
+	struct resource res;
+	unsigned long physaddr;
+	const u32 *id;
+	int irq, bus_width, rc;
+
+	dev_dbg(&op->dev, "ace_of_probe(%p, %p)\n", op, match);
+
+	/* device id */
+	id = of_get_property(op->node, "port-number", NULL);
+
+	/* physaddr */
+	rc = of_address_to_resource(op->node, 0, &res);
+	if (rc) {
+		dev_err(&op->dev, "invalid address\n");
+		return rc;
+	}
+	physaddr = res.start;
+
+	/* irq */
+	irq = irq_of_parse_and_map(op->node, 0);
+
+	/* bus width */
+	bus_width = ACE_BUS_WIDTH_16;
+	if (of_find_property(op->node, "8-bit", NULL))
+		bus_width = ACE_BUS_WIDTH_8;
+
+	/* Call the bus-independant setup code */
+	return ace_alloc(&op->dev, id ? *id : 0, physaddr, irq, bus_width);
+}
+
+static int __devexit ace_of_remove(struct of_device *op)
+{
+	ace_free(&op->dev);
+	return 0;
+}
+
+/* Match table for of_platform binding */
+static struct of_device_id __devinit ace_of_match[] = {
+	{ .compatible = "xilinx,xsysace", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, ace_of_match);
+
+static struct of_platform_driver ace_of_driver = {
+	.owner = THIS_MODULE,
+	.name = "xsysace",
+	.match_table = ace_of_match,
+	.probe = ace_of_probe,
+	.remove = ace_of_remove,
+	.driver = {
+		.name = "xsysace",
+	},
+};
+
+/* Registration helpers to keep the number of #ifdefs to a minimum */
+static int __init ace_of_register(void)
+{
+	pr_debug("xsysace: registering OF binding\n");
+	return of_register_platform_driver(&ace_of_driver);
+}
+
+static void __exit ace_of_unregister(void)
+{
+	of_unregister_platform_driver(&ace_of_driver);
+}
+#else /* CONFIG_OF */
+/* CONFIG_OF not enabled; do nothing helpers */
+static int __init ace_of_register(void) { return 0 }
+static void __exit ace_of_unregister(void) { }
+#endif /* CONFIG_OF */
+
+/* ---------------------------------------------------------------------
  * Module init/exit routines
  */
 static int __init ace_init(void)
@@ -1170,6 +1253,9 @@ static int __init ace_init(void)
 		goto err_blk;
 	}
 
+	if ((rc = ace_of_register()) != 0)
+		goto err_of;
+
 	pr_debug("xsysace: registering platform binding\n");
 	if ((rc = platform_driver_register(&ace_platform_driver)) != 0)
 		goto err_plat;
@@ -1178,6 +1264,8 @@ static int __init ace_init(void)
 	return 0;
 
       err_plat:
+	ace_of_unregister();
+      err_of:
 	unregister_blkdev(ace_major, "xsysace");
       err_blk:
 	printk(KERN_ERR "xsysace: registration failed; err=%i\n", rc);
@@ -1188,6 +1276,7 @@ static void __exit ace_exit(void)
 {
 	pr_debug("Unregistering Xilinx SystemACE driver\n");
 	platform_driver_unregister(&ace_platform_driver);
+	ace_of_unregister();
 	unregister_blkdev(ace_major, "xsysace");
 }
 

Re: [PATCH 00/18] Virtex support in arch/powerpc

[Arnd Bergmann]

Hi Grant!

On Friday 28 September 2007, Grant Likely wrote:
> This series adds Xilinx Virtex support to arch/powerpc. =A0Please review
> and comment. =A0It includes support for the uartlite and SystemACE devices

Very nice set of patches, I looked at all of them but couldn't find much
to complain about, just trivial details.

	Arnd <><

Re: [PATCH 00/18] Virtex support in arch/powerpc

[Grant Likely]

On 9/28/07, Arnd Bergmann <arnd@arndb.de> wrote:
> Hi Grant!
>
> On Friday 28 September 2007, Grant Likely wrote:
> > This series adds Xilinx Virtex support to arch/powerpc. Please review
> > and comment. It includes support for the uartlite and SystemACE devices
>
> Very nice set of patches, I looked at all of them but couldn't find much
> to complain about, just trivial details.

Thank you very much for the review!

Cheers,
g.

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