[PATCH] Use platform device for 8250 registration

[PATCH] Use platform device for 8250 registration

[David Woodhouse]

My dual G4 PowerMac crashes sometimes when it probes for the (absent)
serial ports. Theoretically it's supposed to take a machine check and
recover -- but it doesn't always work like that.

This patch removes the defaults from asm/pc_serial.h and uses the code
which already existed for ppc64 to register any ports which are found in
the device tree. It's slightly updated to work in 32-bit and also on the
Pegasos which claims the input frequency is 0Hz.

We could probably remove all the rest of the crap from asm/serial.h and
let platforms register their own serial8250 platform devices. And this
probably breaks serial ports on PReP, which needs to do likewise.

--- linux-2.6.12/drivers/serial/Makefile~	2005-08-11 13:51:50.000000000 +0100
+++ linux-2.6.12/drivers/serial/Makefile	2005-08-15 21:08:49.000000000 +0100
@@ -22,6 +22,7 @@ obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250
 obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
 obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_MCA) += 8250_mca.o
+obj-$(CONFIG_SERIAL_8250_OF) += 8250_of.o
 obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
 obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
 obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
--- linux-2.6.12/drivers/serial/8250_of.c~	2005-08-15 21:14:27.000000000 +0100
+++ linux-2.6.12/drivers/serial/8250_of.c	2005-08-15 21:20:59.000000000 +0100
@@ -0,0 +1,199 @@
+#include <linux/kernel.h>
+#include <linux/serial.h>
+#include <linux/serial_8250.h>
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <asm/serial.h>
+#include <asm/prom.h>
+
+#if 0
+#define DBG(fmt...) printk(KERN_DEBUG fmt)
+#else
+#define DBG(fmt...) do { } while (0)
+#endif
+
+/*
+ * This function can be used by platforms to "find" legacy serial ports.
+ * It works for "serial" nodes under an "isa" node, and will try to
+ * respect the "ibm,aix-loc" property if any. It works with up to 8
+ * ports.
+ */
+
+#define MAX_LEGACY_SERIAL_PORTS	8
+static int ports_probed = 0;
+
+static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
+static unsigned int old_serial_count;
+
+void __init generic_find_legacy_serial_ports(u64 *physport,
+		unsigned int *default_speed)
+{
+	struct device_node *np;
+	u32 *sizeprop;
+
+	struct isa_reg_property {
+		u32 space;
+		u32 address;
+		u32 size;
+	};
+
+	DBG(" -> generic_find_legacy_serial_port()\n");
+	ports_probed = 1;
+
+	*physport = 0;
+	if (default_speed)
+		*default_speed = 0;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return;
+
+	/* First fill our array */
+	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
+		struct device_node *isa, *pci;
+		struct isa_reg_property *reg;
+		unsigned long phys_size, addr_size;
+		u64 io_base;
+		u32 *rangesp;
+		u32 *interrupts, *clk, *spd;
+		char *typep;
+		int index, rlen, rentsize;
+
+		/* Ok, first check if it's under an "isa" parent */
+		isa = of_get_parent(np);
+		if (!isa || strcmp(isa->name, "isa")) {
+			DBG("%s: no isa parent found\n", np->full_name);
+			continue;
+		}
+		
+		/* Now look for an "ibm,aix-loc" property that gives us ordering
+		 * if any...
+		 */
+	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
+
+		/* Get the ISA port number */
+		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);	
+		if (reg == NULL)
+			goto next_port;
+		/* We assume the interrupt number isn't translated ... */
+		interrupts = (u32 *)get_property(np, "interrupts", NULL);
+		/* get clock freq. if present */
+		clk = (u32 *)get_property(np, "clock-frequency", NULL);
+		/* get default speed if present */
+		spd = (u32 *)get_property(np, "current-speed", NULL);
+		/* Default to locate at end of array */
+		index = old_serial_count; /* end of the array by default */
+
+		/* If we have a location index, then use it */
+		if (typep && *typep == 'S') {
+			index = simple_strtol(typep+1, NULL, 0) - 1;
+			/* if index is out of range, use end of array instead */
+			if (index >= MAX_LEGACY_SERIAL_PORTS)
+				index = old_serial_count;
+			/* if our index is still out of range, that mean that
+			 * array is full, we could scan for a free slot but that
+			 * make little sense to bother, just skip the port
+			 */
+			if (index >= MAX_LEGACY_SERIAL_PORTS)
+				goto next_port;
+			if (index >= old_serial_count)
+				old_serial_count = index + 1;
+			/* Check if there is a port who already claimed our slot */
+			if (serial_ports[index].iobase != 0) {
+				/* if we still have some room, move it, else override */
+				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
+					DBG("Moved legacy port %d -> %d\n", index,
+					    old_serial_count);
+					serial_ports[old_serial_count++] =
+						serial_ports[index];
+				} else {
+					DBG("Replacing legacy port %d\n", index);
+				}
+			}
+		}
+		if (index >= MAX_LEGACY_SERIAL_PORTS)
+			goto next_port;
+		if (index >= old_serial_count)
+			old_serial_count = index + 1;
+
+		/* Now fill the entry */
+		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
+		serial_ports[index].uartclk = (clk && *clk) ? *clk : BASE_BAUD * 16;
+		serial_ports[index].iobase = reg->address;
+		serial_ports[index].irq = interrupts ? interrupts[0] : 0;
+		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
+
+		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
+		    index,
+		    serial_ports[index].iobase,
+		    serial_ports[index].irq,
+		    serial_ports[index].uartclk);
+
+		/* Get phys address of IO reg for port 1 */
+		if (index != 0)
+			goto next_port;
+
+		pci = of_get_parent(isa);
+		if (!pci) {
+			DBG("%s: no pci parent found\n", np->full_name);
+			goto next_port;
+		}
+
+		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
+		if (rangesp == NULL) {
+			of_node_put(pci);
+			goto next_port;
+		}
+		rlen /= 4;
+
+		/* we need the #size-cells of the PCI bridge node itself */
+		phys_size = 1;
+		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
+		if (sizeprop != NULL)
+			phys_size = *sizeprop;
+		/* we need the parent #addr-cells */
+		addr_size = prom_n_addr_cells(pci);
+		rentsize = 3 + addr_size + phys_size;
+		io_base = 0;
+		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
+			if (((rangesp[0] >> 24) & 0x3) != 1)
+				continue; /* not IO space */
+			io_base = rangesp[3];
+			if (addr_size == 2)
+				io_base = (io_base << 32) | rangesp[4];
+		}
+		if (io_base != 0) {
+			*physport = io_base + reg->address;
+			if (default_speed && spd)
+				*default_speed = *spd;
+		}
+		of_node_put(pci);
+	next_port:
+		of_node_put(isa);
+	}
+
+	DBG(" <- generic_find_legacy_serial_port()\n");
+}
+
+static struct platform_device serial_device = {
+	.name	= "serial8250",
+	.id	= 0,
+	.dev	= {
+		.platform_data = serial_ports,
+	},
+};
+
+static int __init serial_dev_init(void)
+{
+	u64 phys;
+	unsigned int spd;
+
+	if (!ports_probed)
+		generic_find_legacy_serial_ports(&phys, &spd);
+	return platform_device_register(&serial_device);
+}
+arch_initcall(serial_dev_init);
+
+
--- linux-2.6.12/drivers/serial/Kconfig~	2005-08-11 13:51:50.000000000 +0100
+++ linux-2.6.12/drivers/serial/Kconfig	2005-08-15 21:13:41.000000000 +0100
@@ -77,6 +77,11 @@ config SERIAL_8250_CS
 
 	  If unsure, say N.
 
+config SERIAL_8250_OF
+       bool
+       default y
+       depends on PPC_OF && SERIAL_8250 != n
+
 config SERIAL_8250_ACPI
 	bool "8250/16550 device discovery via ACPI namespace"
 	default y if IA64
--- linux-2.6.12/arch/ppc64/kernel/setup.c~	2005-08-11 13:52:04.000000000 +0100
+++ linux-2.6.12/arch/ppc64/kernel/setup.c	2005-08-15 20:27:25.000000000 +0100
@@ -1147,186 +1147,6 @@ void __init setup_default_decr(void)
 	lpaca->next_jiffy_update_tb = get_tb() + tb_ticks_per_jiffy;
 }
 
-#ifndef CONFIG_PPC_ISERIES
-/*
- * This function can be used by platforms to "find" legacy serial ports.
- * It works for "serial" nodes under an "isa" node, and will try to
- * respect the "ibm,aix-loc" property if any. It works with up to 8
- * ports.
- */
-
-#define MAX_LEGACY_SERIAL_PORTS	8
-static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
-static unsigned int old_serial_count;
-
-void __init generic_find_legacy_serial_ports(u64 *physport,
-		unsigned int *default_speed)
-{
-	struct device_node *np;
-	u32 *sizeprop;
-
-	struct isa_reg_property {
-		u32 space;
-		u32 address;
-		u32 size;
-	};
-	struct pci_reg_property {
-		struct pci_address addr;
-		u32 size_hi;
-		u32 size_lo;
-	};                                                                        
-
-	DBG(" -> generic_find_legacy_serial_port()\n");
-
-	*physport = 0;
-	if (default_speed)
-		*default_speed = 0;
-
-	np = of_find_node_by_path("/");
-	if (!np)
-		return;
-
-	/* First fill our array */
-	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
-		struct device_node *isa, *pci;
-		struct isa_reg_property *reg;
-		unsigned long phys_size, addr_size, io_base;
-		u32 *rangesp;
-		u32 *interrupts, *clk, *spd;
-		char *typep;
-		int index, rlen, rentsize;
-
-		/* Ok, first check if it's under an "isa" parent */
-		isa = of_get_parent(np);
-		if (!isa || strcmp(isa->name, "isa")) {
-			DBG("%s: no isa parent found\n", np->full_name);
-			continue;
-		}
-		
-		/* Now look for an "ibm,aix-loc" property that gives us ordering
-		 * if any...
-		 */
-	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
-
-		/* Get the ISA port number */
-		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);	
-		if (reg == NULL)
-			goto next_port;
-		/* We assume the interrupt number isn't translated ... */
-		interrupts = (u32 *)get_property(np, "interrupts", NULL);
-		/* get clock freq. if present */
-		clk = (u32 *)get_property(np, "clock-frequency", NULL);
-		/* get default speed if present */
-		spd = (u32 *)get_property(np, "current-speed", NULL);
-		/* Default to locate at end of array */
-		index = old_serial_count; /* end of the array by default */
-
-		/* If we have a location index, then use it */
-		if (typep && *typep == 'S') {
-			index = simple_strtol(typep+1, NULL, 0) - 1;
-			/* if index is out of range, use end of array instead */
-			if (index >= MAX_LEGACY_SERIAL_PORTS)
-				index = old_serial_count;
-			/* if our index is still out of range, that mean that
-			 * array is full, we could scan for a free slot but that
-			 * make little sense to bother, just skip the port
-			 */
-			if (index >= MAX_LEGACY_SERIAL_PORTS)
-				goto next_port;
-			if (index >= old_serial_count)
-				old_serial_count = index + 1;
-			/* Check if there is a port who already claimed our slot */
-			if (serial_ports[index].iobase != 0) {
-				/* if we still have some room, move it, else override */
-				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
-					DBG("Moved legacy port %d -> %d\n", index,
-					    old_serial_count);
-					serial_ports[old_serial_count++] =
-						serial_ports[index];
-				} else {
-					DBG("Replacing legacy port %d\n", index);
-				}
-			}
-		}
-		if (index >= MAX_LEGACY_SERIAL_PORTS)
-			goto next_port;
-		if (index >= old_serial_count)
-			old_serial_count = index + 1;
-
-		/* Now fill the entry */
-		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
-		serial_ports[index].uartclk = clk ? *clk : BASE_BAUD * 16;
-		serial_ports[index].iobase = reg->address;
-		serial_ports[index].irq = interrupts ? interrupts[0] : 0;
-		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
-
-		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
-		    index,
-		    serial_ports[index].iobase,
-		    serial_ports[index].irq,
-		    serial_ports[index].uartclk);
-
-		/* Get phys address of IO reg for port 1 */
-		if (index != 0)
-			goto next_port;
-
-		pci = of_get_parent(isa);
-		if (!pci) {
-			DBG("%s: no pci parent found\n", np->full_name);
-			goto next_port;
-		}
-
-		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
-		if (rangesp == NULL) {
-			of_node_put(pci);
-			goto next_port;
-		}
-		rlen /= 4;
-
-		/* we need the #size-cells of the PCI bridge node itself */
-		phys_size = 1;
-		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
-		if (sizeprop != NULL)
-			phys_size = *sizeprop;
-		/* we need the parent #addr-cells */
-		addr_size = prom_n_addr_cells(pci);
-		rentsize = 3 + addr_size + phys_size;
-		io_base = 0;
-		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
-			if (((rangesp[0] >> 24) & 0x3) != 1)
-				continue; /* not IO space */
-			io_base = rangesp[3];
-			if (addr_size == 2)
-				io_base = (io_base << 32) | rangesp[4];
-		}
-		if (io_base != 0) {
-			*physport = io_base + reg->address;
-			if (default_speed && spd)
-				*default_speed = *spd;
-		}
-		of_node_put(pci);
-	next_port:
-		of_node_put(isa);
-	}
-
-	DBG(" <- generic_find_legacy_serial_port()\n");
-}
-
-static struct platform_device serial_device = {
-	.name	= "serial8250",
-	.id	= 0,
-	.dev	= {
-		.platform_data = serial_ports,
-	},
-};
-
-static int __init serial_dev_init(void)
-{
-	return platform_device_register(&serial_device);
-}
-arch_initcall(serial_dev_init);
-
-#endif /* CONFIG_PPC_ISERIES */
 
 int check_legacy_ioport(unsigned long base_port)
 {
--- linux-2.6.12/include/asm-ppc/pc_serial.h~	2005-08-15 21:19:32.000000000 +0100
+++ linux-2.6.12/include/asm-ppc/pc_serial.h	2005-08-15 21:20:24.000000000 +0100
@@ -26,18 +26,3 @@
 #define RS_TABLE_SIZE  4
 #endif
 
-/* Standard COM flags (except for COM4, because of the 8514 problem) */
-#ifdef CONFIG_SERIAL_DETECT_IRQ
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
-#define STD_COM4_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
-#else
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
-#define STD_COM4_FLAGS ASYNC_BOOT_AUTOCONF
-#endif
-
-#define SERIAL_PORT_DFNS			\
-	/* UART CLK   PORT IRQ     FLAGS        */			\
-	{ 0, BASE_BAUD, 0x3F8, 4, STD_COM_FLAGS },	/* ttyS0 */	\
-	{ 0, BASE_BAUD, 0x2F8, 3, STD_COM_FLAGS },	/* ttyS1 */	\
-	{ 0, BASE_BAUD, 0x3E8, 4, STD_COM_FLAGS },	/* ttyS2 */	\
-	{ 0, BASE_BAUD, 0x2E8, 3, STD_COM4_FLAGS },	/* ttyS3 */



-- 
dwmw2

Re: [PATCH] Use platform device for 8250 registration

[Kumar Gala]

On Aug 16, 2005, at 11:21 AM, David Woodhouse wrote:

> My dual G4 PowerMac crashes sometimes when it probes for the (absent)
> serial ports. Theoretically it's supposed to take a machine check and
> recover -- but it doesn't always work like that.
>
> This patch removes the defaults from asm/pc_serial.h and uses the code
> which already existed for ppc64 to register any ports which are  
> found in
> the device tree. It's slightly updated to work in 32-bit and also  
> on the
> Pegasos which claims the input frequency is 0Hz.
>
> We could probably remove all the rest of the crap from asm/serial.h  
> and
> let platforms register their own serial8250 platform devices. And this
> probably breaks serial ports on PReP, which needs to do likewise.

Hmm, I wondering if we can provide some standard way of handling this  
for the embedded platforms as well.  It would be nice to drop the old  
style of initialization completely and move to using a platform  
device always.

- kumar

Re: [PATCH] Use platform device for 8250 registration

[Benjamin Herrenschmidt]

On Wed, 2005-08-17 at 01:30 -0500, Kumar Gala wrote:
> On Aug 16, 2005, at 11:21 AM, David Woodhouse wrote:
> 
> > My dual G4 PowerMac crashes sometimes when it probes for the (absent)
> > serial ports. Theoretically it's supposed to take a machine check and
> > recover -- but it doesn't always work like that.
> >
> > This patch removes the defaults from asm/pc_serial.h and uses the code
> > which already existed for ppc64 to register any ports which are  
> > found in
> > the device tree. It's slightly updated to work in 32-bit and also  
> > on the
> > Pegasos which claims the input frequency is 0Hz.
> >
> > We could probably remove all the rest of the crap from asm/serial.h  
> > and
> > let platforms register their own serial8250 platform devices. And this
> > probably breaks serial ports on PReP, which needs to do likewise.
> 
> Hmm, I wondering if we can provide some standard way of handling this  
> for the embedded platforms as well.  It would be nice to drop the old  
> style of initialization completely and move to using a platform  
> device always.

Sure, if they have a device-tree :) The code from ppc64 could be useable
there too. For others, they need to create their ports which whatever
mecanism they have to "know" where the port actually is ... little
common code here... but then, instanciating a platform device is easy.

Ben.

Re: [PATCH] Use platform device for 8250 registration

[David Woodhouse]

On Wed, 2005-08-17 at 01:30 -0500, Kumar Gala wrote:
> > We could probably remove all the rest of the crap from asm/serial.h and
> > let platforms register their own serial8250 platform devices...

> Hmm, I wondering if we can provide some standard way of handling this  
> for the embedded platforms as well.  It would be nice to drop the old  
> style of initialization completely and move to using a platform  
> device always.

Yes, that's precisely what I meant. Just remove it from the list in
serial.h and as Ben says, instantiating a platform device is easy.

static struct plat_serial8250_port my_serial_ports[] = {
	{
		.uartclk = 115200*16,
		.iobase = 0x2f8,
		.irq = 3,
		.flags = ASYNC_BOOT_AUTOCONF;
	},
};

static struct platform_device my_serial_device = {
	.name = "serial8250",
	.dev.platform_data = my_serial_ports,
};

  ... platform_device_register(&my_serial_device);

-- 
dwmw2

Re: [PATCH] Use platform device for 8250 registration

[Russell King]

On Wed, Aug 17, 2005 at 11:44:38AM +0100, David Woodhouse wrote:
> This patch removes the defaults from asm/pc_serial.h and uses the code
> which already existed for ppc64 to register any ports which are found in
> the device tree.

Great.  See comments below - not many of them though, most of them
trivial.

> --- linux-2.6.12/drivers/serial/8250_of.c~	2005-08-15 21:14:27.000000000 +0100
> +++ linux-2.6.12/drivers/serial/8250_of.c	2005-08-15 21:20:59.000000000 +0100
> @@ -0,0 +1,199 @@
> +#include <linux/kernel.h>
> +#include <linux/serial.h>
> +#include <linux/serial_8250.h>
> +#include <linux/config.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/pci.h>
> +#include <asm/serial.h>
> +#include <asm/prom.h>
> +
> +#if 0
> +#define DBG(fmt...) printk(KERN_DEBUG fmt)
> +#else
> +#define DBG(fmt...) do { } while (0)
> +#endif

pr_debug() ?

> +
> +/*
> + * This function can be used by platforms to "find" legacy serial ports.
> + * It works for "serial" nodes under an "isa" node, and will try to
> + * respect the "ibm,aix-loc" property if any. It works with up to 8
> + * ports.
> + */
> +
> +#define MAX_LEGACY_SERIAL_PORTS	8
> +static int ports_probed = 0;
> +
> +static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
> +static unsigned int old_serial_count;
> +
> +void __init generic_find_legacy_serial_ports(u64 *physport,
> +		unsigned int *default_speed)
> +{
> +	struct device_node *np;
> +	u32 *sizeprop;
> +
> +	struct isa_reg_property {
> +		u32 space;
> +		u32 address;
> +		u32 size;
> +	};
> +
> +	DBG(" -> generic_find_legacy_serial_port()\n");
> +	ports_probed = 1;
> +
> +	*physport = 0;
> +	if (default_speed)
> +		*default_speed = 0;
> +
> +	np = of_find_node_by_path("/");
> +	if (!np)
> +		return;
> +
> +	/* First fill our array */
> +	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
> +		struct device_node *isa, *pci;
> +		struct isa_reg_property *reg;
> +		unsigned long phys_size, addr_size;
> +		u64 io_base;
> +		u32 *rangesp;
> +		u32 *interrupts, *clk, *spd;
> +		char *typep;
> +		int index, rlen, rentsize;
> +
> +		/* Ok, first check if it's under an "isa" parent */
> +		isa = of_get_parent(np);
> +		if (!isa || strcmp(isa->name, "isa")) {
> +			DBG("%s: no isa parent found\n", np->full_name);
> +			continue;
> +		}
> +		

Extraneous two tabs

> +		/* Now look for an "ibm,aix-loc" property that gives us ordering
> +		 * if any...
> +		 */
> +	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
> +
> +		/* Get the ISA port number */
> +		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);	

Extraneous tab at the end of this line

> +		if (reg == NULL)
> +			goto next_port;
> +		/* We assume the interrupt number isn't translated ... */
> +		interrupts = (u32 *)get_property(np, "interrupts", NULL);
> +		/* get clock freq. if present */
> +		clk = (u32 *)get_property(np, "clock-frequency", NULL);
> +		/* get default speed if present */
> +		spd = (u32 *)get_property(np, "current-speed", NULL);
> +		/* Default to locate at end of array */
> +		index = old_serial_count; /* end of the array by default */
> +
> +		/* If we have a location index, then use it */
> +		if (typep && *typep == 'S') {
> +			index = simple_strtol(typep+1, NULL, 0) - 1;
> +			/* if index is out of range, use end of array instead */
> +			if (index >= MAX_LEGACY_SERIAL_PORTS)
> +				index = old_serial_count;
> +			/* if our index is still out of range, that mean that
> +			 * array is full, we could scan for a free slot but that
> +			 * make little sense to bother, just skip the port
> +			 */
> +			if (index >= MAX_LEGACY_SERIAL_PORTS)
> +				goto next_port;
> +			if (index >= old_serial_count)
> +				old_serial_count = index + 1;
> +			/* Check if there is a port who already claimed our slot */
> +			if (serial_ports[index].iobase != 0) {
> +				/* if we still have some room, move it, else override */
> +				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
> +					DBG("Moved legacy port %d -> %d\n", index,
> +					    old_serial_count);
> +					serial_ports[old_serial_count++] =
> +						serial_ports[index];
> +				} else {
> +					DBG("Replacing legacy port %d\n", index);
> +				}
> +			}
> +		}
> +		if (index >= MAX_LEGACY_SERIAL_PORTS)
> +			goto next_port;
> +		if (index >= old_serial_count)
> +			old_serial_count = index + 1;
> +
> +		/* Now fill the entry */
> +		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
> +		serial_ports[index].uartclk = (clk && *clk) ? *clk : BASE_BAUD * 16;
> +		serial_ports[index].iobase = reg->address;
> +		serial_ports[index].irq = interrupts ? interrupts[0] : 0;

Doesn't PPC64 have NO_IRQ ?

> +		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
> +
> +		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
> +		    index,
> +		    serial_ports[index].iobase,
> +		    serial_ports[index].irq,
> +		    serial_ports[index].uartclk);
> +
> +		/* Get phys address of IO reg for port 1 */
> +		if (index != 0)
> +			goto next_port;
> +
> +		pci = of_get_parent(isa);
> +		if (!pci) {
> +			DBG("%s: no pci parent found\n", np->full_name);
> +			goto next_port;
> +		}
> +
> +		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
> +		if (rangesp == NULL) {
> +			of_node_put(pci);
> +			goto next_port;
> +		}
> +		rlen /= 4;
> +
> +		/* we need the #size-cells of the PCI bridge node itself */
> +		phys_size = 1;
> +		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
> +		if (sizeprop != NULL)
> +			phys_size = *sizeprop;
> +		/* we need the parent #addr-cells */
> +		addr_size = prom_n_addr_cells(pci);
> +		rentsize = 3 + addr_size + phys_size;
> +		io_base = 0;
> +		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
> +			if (((rangesp[0] >> 24) & 0x3) != 1)
> +				continue; /* not IO space */
> +			io_base = rangesp[3];
> +			if (addr_size == 2)
> +				io_base = (io_base << 32) | rangesp[4];
> +		}
> +		if (io_base != 0) {
> +			*physport = io_base + reg->address;
> +			if (default_speed && spd)
> +				*default_speed = *spd;
> +		}
> +		of_node_put(pci);
> +	next_port:
> +		of_node_put(isa);
> +	}
> +
> +	DBG(" <- generic_find_legacy_serial_port()\n");
> +}
> +
> +static struct platform_device serial_device = {
> +	.name	= "serial8250",
> +	.id	= 0,
> +	.dev	= {
> +		.platform_data = serial_ports,
> +	},
> +};
> +
> +static int __init serial_dev_init(void)
> +{
> +	u64 phys;
> +	unsigned int spd;
> +
> +	if (!ports_probed)
> +		generic_find_legacy_serial_ports(&phys, &spd);
> +	return platform_device_register(&serial_device);
> +}
> +arch_initcall(serial_dev_init);
> +
> +

Can we kill the two blank lines please?

> --- linux-2.6.12/drivers/serial/Kconfig~	2005-08-11 13:51:50.000000000 +0100
> +++ linux-2.6.12/drivers/serial/Kconfig	2005-08-15 21:13:41.000000000 +0100
> @@ -77,6 +77,11 @@ config SERIAL_8250_CS
>  
>  	  If unsure, say N.
>  
> +config SERIAL_8250_OF
> +       bool
> +       default y
> +       depends on PPC_OF && SERIAL_8250 != n

Wouldn't "depends on PPC_OF && SERIAL_8250" be sufficient?  iirc "bool"
treats a dependency result of 'm' as 'y'.

> +
>  config SERIAL_8250_ACPI
>  	bool "8250/16550 device discovery via ACPI namespace"
>  	default y if IA64
> --- linux-2.6.12/arch/ppc64/kernel/setup.c~	2005-08-11 13:52:04.000000000 +0100
> +++ linux-2.6.12/arch/ppc64/kernel/setup.c	2005-08-15 20:27:25.000000000 +0100
> @@ -1147,186 +1147,6 @@ void __init setup_default_decr(void)
>  	lpaca->next_jiffy_update_tb = get_tb() + tb_ticks_per_jiffy;
>  }
>  
> -#ifndef CONFIG_PPC_ISERIES
> -/*
> - * This function can be used by platforms to "find" legacy serial ports.
> - * It works for "serial" nodes under an "isa" node, and will try to
> - * respect the "ibm,aix-loc" property if any. It works with up to 8
> - * ports.
> - */
> -
> -#define MAX_LEGACY_SERIAL_PORTS	8
> -static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
> -static unsigned int old_serial_count;
> -
> -void __init generic_find_legacy_serial_ports(u64 *physport,
> -		unsigned int *default_speed)
> -{
> -	struct device_node *np;
> -	u32 *sizeprop;
> -
> -	struct isa_reg_property {
> -		u32 space;
> -		u32 address;
> -		u32 size;
> -	};
> -	struct pci_reg_property {
> -		struct pci_address addr;
> -		u32 size_hi;
> -		u32 size_lo;
> -	};                                                                        

Lots a space at the end of this line.

> -
> -	DBG(" -> generic_find_legacy_serial_port()\n");
> -
> -	*physport = 0;
> -	if (default_speed)
> -		*default_speed = 0;
> -
> -	np = of_find_node_by_path("/");
> -	if (!np)
> -		return;
> -
> -	/* First fill our array */
> -	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
> -		struct device_node *isa, *pci;
> -		struct isa_reg_property *reg;
> -		unsigned long phys_size, addr_size, io_base;
> -		u32 *rangesp;
> -		u32 *interrupts, *clk, *spd;
> -		char *typep;
> -		int index, rlen, rentsize;
> -
> -		/* Ok, first check if it's under an "isa" parent */
> -		isa = of_get_parent(np);
> -		if (!isa || strcmp(isa->name, "isa")) {
> -			DBG("%s: no isa parent found\n", np->full_name);
> -			continue;
> -		}
> -		
> -		/* Now look for an "ibm,aix-loc" property that gives us ordering
> -		 * if any...
> -		 */
> -	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
> -
> -		/* Get the ISA port number */
> -		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);	
> -		if (reg == NULL)
> -			goto next_port;
> -		/* We assume the interrupt number isn't translated ... */
> -		interrupts = (u32 *)get_property(np, "interrupts", NULL);
> -		/* get clock freq. if present */
> -		clk = (u32 *)get_property(np, "clock-frequency", NULL);
> -		/* get default speed if present */
> -		spd = (u32 *)get_property(np, "current-speed", NULL);
> -		/* Default to locate at end of array */
> -		index = old_serial_count; /* end of the array by default */
> -
> -		/* If we have a location index, then use it */
> -		if (typep && *typep == 'S') {
> -			index = simple_strtol(typep+1, NULL, 0) - 1;
> -			/* if index is out of range, use end of array instead */
> -			if (index >= MAX_LEGACY_SERIAL_PORTS)
> -				index = old_serial_count;
> -			/* if our index is still out of range, that mean that
> -			 * array is full, we could scan for a free slot but that
> -			 * make little sense to bother, just skip the port
> -			 */
> -			if (index >= MAX_LEGACY_SERIAL_PORTS)
> -				goto next_port;
> -			if (index >= old_serial_count)
> -				old_serial_count = index + 1;
> -			/* Check if there is a port who already claimed our slot */
> -			if (serial_ports[index].iobase != 0) {
> -				/* if we still have some room, move it, else override */
> -				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
> -					DBG("Moved legacy port %d -> %d\n", index,
> -					    old_serial_count);
> -					serial_ports[old_serial_count++] =
> -						serial_ports[index];
> -				} else {
> -					DBG("Replacing legacy port %d\n", index);
> -				}
> -			}
> -		}
> -		if (index >= MAX_LEGACY_SERIAL_PORTS)
> -			goto next_port;
> -		if (index >= old_serial_count)
> -			old_serial_count = index + 1;
> -
> -		/* Now fill the entry */
> -		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
> -		serial_ports[index].uartclk = clk ? *clk : BASE_BAUD * 16;
> -		serial_ports[index].iobase = reg->address;
> -		serial_ports[index].irq = interrupts ? interrupts[0] : 0;
> -		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
> -
> -		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
> -		    index,
> -		    serial_ports[index].iobase,
> -		    serial_ports[index].irq,
> -		    serial_ports[index].uartclk);
> -
> -		/* Get phys address of IO reg for port 1 */
> -		if (index != 0)
> -			goto next_port;
> -
> -		pci = of_get_parent(isa);
> -		if (!pci) {
> -			DBG("%s: no pci parent found\n", np->full_name);
> -			goto next_port;
> -		}
> -
> -		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
> -		if (rangesp == NULL) {
> -			of_node_put(pci);
> -			goto next_port;
> -		}
> -		rlen /= 4;
> -
> -		/* we need the #size-cells of the PCI bridge node itself */
> -		phys_size = 1;
> -		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
> -		if (sizeprop != NULL)
> -			phys_size = *sizeprop;
> -		/* we need the parent #addr-cells */
> -		addr_size = prom_n_addr_cells(pci);
> -		rentsize = 3 + addr_size + phys_size;
> -		io_base = 0;
> -		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
> -			if (((rangesp[0] >> 24) & 0x3) != 1)
> -				continue; /* not IO space */
> -			io_base = rangesp[3];
> -			if (addr_size == 2)
> -				io_base = (io_base << 32) | rangesp[4];
> -		}
> -		if (io_base != 0) {
> -			*physport = io_base + reg->address;
> -			if (default_speed && spd)
> -				*default_speed = *spd;
> -		}
> -		of_node_put(pci);
> -	next_port:
> -		of_node_put(isa);
> -	}
> -
> -	DBG(" <- generic_find_legacy_serial_port()\n");
> -}
> -
> -static struct platform_device serial_device = {
> -	.name	= "serial8250",
> -	.id	= 0,
> -	.dev	= {
> -		.platform_data = serial_ports,
> -	},
> -};
> -
> -static int __init serial_dev_init(void)
> -{
> -	return platform_device_register(&serial_device);
> -}
> -arch_initcall(serial_dev_init);
> -
> -#endif /* CONFIG_PPC_ISERIES */
>  
>  int check_legacy_ioport(unsigned long base_port)
>  {
> --- linux-2.6.12/include/asm-ppc/pc_serial.h~	2005-08-15 21:19:32.000000000 +0100
> +++ linux-2.6.12/include/asm-ppc/pc_serial.h	2005-08-15 21:20:24.000000000 +0100
> @@ -26,18 +26,3 @@
>  #define RS_TABLE_SIZE  4
>  #endif
>  
> -/* Standard COM flags (except for COM4, because of the 8514 problem) */
> -#ifdef CONFIG_SERIAL_DETECT_IRQ
> -#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
> -#define STD_COM4_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
> -#else
> -#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
> -#define STD_COM4_FLAGS ASYNC_BOOT_AUTOCONF
> -#endif
> -
> -#define SERIAL_PORT_DFNS			\
> -	/* UART CLK   PORT IRQ     FLAGS        */			\
> -	{ 0, BASE_BAUD, 0x3F8, 4, STD_COM_FLAGS },	/* ttyS0 */	\
> -	{ 0, BASE_BAUD, 0x2F8, 3, STD_COM_FLAGS },	/* ttyS1 */	\
> -	{ 0, BASE_BAUD, 0x3E8, 4, STD_COM_FLAGS },	/* ttyS2 */	\
> -	{ 0, BASE_BAUD, 0x2E8, 3, STD_COM4_FLAGS },	/* ttyS3 */
> 

-- 
Russell King

Re: [PATCH] Use platform device for 8250 registration

[Kumar Gala]

> Yes, that's precisely what I meant. Just remove it from the list in
> serial.h and as Ben says, instantiating a platform device is easy.
>
> static struct plat_serial8250_port my_serial_ports[] = {
>     {
>         .uartclk = 115200*16,
>         .iobase = 0x2f8,
>         .irq = 3,
>         .flags = ASYNC_BOOT_AUTOCONF;
>     },
> };
>
> static struct platform_device my_serial_device = {
>     .name = "serial8250",
>     .dev.platform_data = my_serial_ports,
> };
>
>   ... platform_device_register(&my_serial_device);

If we are going forward with this change for OF, can we kill  
pc_serial.h and just move the default BASE_BAUD and RS_TABLE size  
into serial.h.

- kumar

When are machine checks suppose to be recoverable?

[Kumar Gala]

David's 8250 cleanup patch made me wondering when are machine checks  
suppose to recoverable?  General class of conditions is what I'm  
looking for here.

Is David's case due to some PCI master abort or something else?

- kumar

On Aug 16, 2005, at 11:21 AM, David Woodhouse wrote:

> My dual G4 PowerMac crashes sometimes when it probes for the (absent)
> serial ports. Theoretically it's supposed to take a machine check and
> recover -- but it doesn't always work like that.

Re: [PATCH] Use platform device for 8250 registration

[Tom Rini]

On Wed, Aug 17, 2005 at 08:31:11AM +0100, David Woodhouse wrote:
> On Wed, 2005-08-17 at 01:30 -0500, Kumar Gala wrote:
> > > We could probably remove all the rest of the crap from asm/serial.h and
> > > let platforms register their own serial8250 platform devices...
> 
> > Hmm, I wondering if we can provide some standard way of handling this  
> > for the embedded platforms as well.  It would be nice to drop the old  
> > style of initialization completely and move to using a platform  
> > device always.
> 
> Yes, that's precisely what I meant. Just remove it from the list in
> serial.h and as Ben says, instantiating a platform device is easy.
> 
> static struct plat_serial8250_port my_serial_ports[] = {
> 	{
> 		.uartclk = 115200*16,
> 		.iobase = 0x2f8,
> 		.irq = 3,
> 		.flags = ASYNC_BOOT_AUTOCONF;
> 	},
> };

So long as you convert arch/ppc/boot/ to this as well, why not (or at
least being able to grab the infos from these structs somehow).

Once everyone is on a flat tree, I don't object to killing all of the
old-style uart definitions steaming out of <asm-ppc/serial.h>.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH] Use platform device for 8250 registration

[Kumar Gala]

> So long as you convert arch/ppc/boot/ to this as well, why not (or at
> least being able to grab the infos from these structs somehow).
>
> Once everyone is on a flat tree, I don't object to killing all of the
> old-style uart definitions steaming out of <asm-ppc/serial.h>.

Tom, do have a test system that I can look at converting that you  
would be willing to test the changes to arch/ppc/boot for me.

(everything I boot doesn't use it)

- kumar

Re: [PATCH] Use platform device for 8250 registration

[Tom Rini]

On Wed, Aug 17, 2005 at 10:22:54AM -0500, Kumar Gala wrote:

> >So long as you convert arch/ppc/boot/ to this as well, why not (or at
> >least being able to grab the infos from these structs somehow).
> >
> >Once everyone is on a flat tree, I don't object to killing all of the
> >old-style uart definitions steaming out of <asm-ppc/serial.h>.
> 
> Tom, do have a test system that I can look at converting that you  
> would be willing to test the changes to arch/ppc/boot for me.

With Matt's patch, you could boot a PReP kernel (no VGA con) :)
But if you convert LoPEC, I'll throw the kernel at it.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH] Use platform device for 8250 registration

[Kumar Gala]

On Aug 17, 2005, at 10:30 AM, Tom Rini wrote:

> On Wed, Aug 17, 2005 at 10:22:54AM -0500, Kumar Gala wrote:
>
>
>>> So long as you convert arch/ppc/boot/ to this as well, why not  
>>> (or at
>>> least being able to grab the infos from these structs somehow).
>>>
>>> Once everyone is on a flat tree, I don't object to killing all of  
>>> the
>>> old-style uart definitions steaming out of <asm-ppc/serial.h>.
>>>
>>
>> Tom, do have a test system that I can look at converting that you
>> would be willing to test the changes to arch/ppc/boot for me.
>>
>
> With Matt's patch, you could boot a PReP kernel (no VGA con) :)
> But if you convert LoPEC, I'll throw the kernel at it.

Does PReP use bootcode?  I thought it was OF based, but what do I know.

- kumar

Re: [PATCH] Use platform device for 8250 registration

[Matt Porter]

On Wed, Aug 17, 2005 at 11:16:39AM -0500, Kumar Gala wrote:
> 
> On Aug 17, 2005, at 10:30 AM, Tom Rini wrote:
> 
> > On Wed, Aug 17, 2005 at 10:22:54AM -0500, Kumar Gala wrote:
> >
> >
> >>> So long as you convert arch/ppc/boot/ to this as well, why not  
> >>> (or at
> >>> least being able to grab the infos from these structs somehow).
> >>>
> >>> Once everyone is on a flat tree, I don't object to killing all of  
> >>> the
> >>> old-style uart definitions steaming out of <asm-ppc/serial.h>.
> >>>
> >>
> >> Tom, do have a test system that I can look at converting that you
> >> would be willing to test the changes to arch/ppc/boot for me.
> >>
> >
> > With Matt's patch, you could boot a PReP kernel (no VGA con) :)
> > But if you convert LoPEC, I'll throw the kernel at it.
> 
> Does PReP use bootcode?  I thought it was OF based, but what do I know.

Are you asking on a real machine or on qemu?  On real PReP hardwrae
you can find both OF and other firmware like PPCBUG...in both cases
they dump residual data.  On qemu, jmayer wrote the openhackware bios
with is supposed to be some minimal OF-like thing...it dumps
residual data too.
 
-Matt

Re: [PATCH] Use platform device for 8250 registration

[Tom Rini]

On Wed, Aug 17, 2005 at 11:16:39AM -0500, Kumar Gala wrote:
> 
> On Aug 17, 2005, at 10:30 AM, Tom Rini wrote:
> 
> >On Wed, Aug 17, 2005 at 10:22:54AM -0500, Kumar Gala wrote:
> >
> >
> >>>So long as you convert arch/ppc/boot/ to this as well, why not  
> >>>(or at
> >>>least being able to grab the infos from these structs somehow).
> >>>
> >>>Once everyone is on a flat tree, I don't object to killing all of  
> >>>the
> >>>old-style uart definitions steaming out of <asm-ppc/serial.h>.
> >>>
> >>
> >>Tom, do have a test system that I can look at converting that you
> >>would be willing to test the changes to arch/ppc/boot for me.
> >>
> >
> >With Matt's patch, you could boot a PReP kernel (no VGA con) :)
> >But if you convert LoPEC, I'll throw the kernel at it.
> 
> Does PReP use bootcode?  I thought it was OF based, but what do I know.

In qemu?  Yes, it does.

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH] Use platform device for 8250 registration

[Tom Rini]

On Wed, Aug 17, 2005 at 09:27:03AM -0700, Matt Porter wrote:
> On Wed, Aug 17, 2005 at 11:16:39AM -0500, Kumar Gala wrote:
> > 
> > On Aug 17, 2005, at 10:30 AM, Tom Rini wrote:
> > 
> > > On Wed, Aug 17, 2005 at 10:22:54AM -0500, Kumar Gala wrote:
> > >
> > >
> > >>> So long as you convert arch/ppc/boot/ to this as well, why not  
> > >>> (or at
> > >>> least being able to grab the infos from these structs somehow).
> > >>>
> > >>> Once everyone is on a flat tree, I don't object to killing all of  
> > >>> the
> > >>> old-style uart definitions steaming out of <asm-ppc/serial.h>.
> > >>>
> > >>
> > >> Tom, do have a test system that I can look at converting that you
> > >> would be willing to test the changes to arch/ppc/boot for me.
> > >>
> > >
> > > With Matt's patch, you could boot a PReP kernel (no VGA con) :)
> > > But if you convert LoPEC, I'll throw the kernel at it.
> > 
> > Does PReP use bootcode?  I thought it was OF based, but what do I know.
> 
> Are you asking on a real machine or on qemu?  On real PReP hardwrae
> you can find both OF and other firmware like PPCBUG...in both cases
> they dump residual data.  On qemu, jmayer wrote the openhackware bios
> with is supposed to be some minimal OF-like thing...it dumps
> residual data too.

But either way, the kernel boots using the code in arch/ppc/boot/simple/
which uses serial values from <asm-ppc/pc_serial.h>

-- 
Tom Rini
http://gate.crashing.org/~trini/

Re: [PATCH] Use platform device for 8250 registration

[David Woodhouse]

On Wed, 2005-08-17 at 12:11 +0100, Russell King wrote:
> Doesn't PPC64 have NO_IRQ ?

PPC64 does but I find no definition of is_real_interrupt() other than
the bogus one in 8250.c which checks for zero. PPC32 does not -- let's
clean that one up separately.

--- linux-2.6.12/drivers/serial/Makefile~	2005-08-11 13:51:50.000000000 +0100
+++ linux-2.6.12/drivers/serial/Makefile	2005-08-15 21:08:49.000000000 +0100
@@ -22,6 +22,7 @@ obj-$(CONFIG_SERIAL_8250_ACCENT) += 8250
 obj-$(CONFIG_SERIAL_8250_BOCA) += 8250_boca.o
 obj-$(CONFIG_SERIAL_8250_HUB6) += 8250_hub6.o
 obj-$(CONFIG_SERIAL_8250_MCA) += 8250_mca.o
+obj-$(CONFIG_SERIAL_8250_OF) += 8250_of.o
 obj-$(CONFIG_SERIAL_AMBA_PL010) += amba-pl010.o
 obj-$(CONFIG_SERIAL_AMBA_PL011) += amba-pl011.o
 obj-$(CONFIG_SERIAL_CLPS711X) += clps711x.o
--- linux-2.6.12/drivers/serial/8250_of.c~	2005-08-15 21:14:27.000000000 +0100
+++ linux-2.6.12/drivers/serial/8250_of.c	2005-08-15 21:20:59.000000000 +0100
@@ -0,0 +1,197 @@
+#include <linux/kernel.h>
+#include <linux/serial.h>
+#include <linux/serial_8250.h>
+#include <linux/config.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/pci.h>
+#include <asm/serial.h>
+#include <asm/prom.h>
+
+#if 0
+#define DBG(fmt...) printk(KERN_DEBUG fmt)
+#else
+#define DBG(fmt...) do { } while (0)
+#endif
+
+/*
+ * This function can be used by platforms to "find" legacy serial ports.
+ * It works for "serial" nodes under an "isa" node, and will try to
+ * respect the "ibm,aix-loc" property if any. It works with up to 8
+ * ports.
+ */
+
+#define MAX_LEGACY_SERIAL_PORTS	8
+static int ports_probed = 0;
+
+static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
+static unsigned int old_serial_count;
+
+void __init generic_find_legacy_serial_ports(u64 *physport,
+		unsigned int *default_speed)
+{
+	struct device_node *np;
+	u32 *sizeprop;
+
+	struct isa_reg_property {
+		u32 space;
+		u32 address;
+		u32 size;
+	};
+
+	DBG(" -> generic_find_legacy_serial_port()\n");
+	ports_probed = 1;
+
+	*physport = 0;
+	if (default_speed)
+		*default_speed = 0;
+
+	np = of_find_node_by_path("/");
+	if (!np)
+		return;
+
+	/* First fill our array */
+	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
+		struct device_node *isa, *pci;
+		struct isa_reg_property *reg;
+		unsigned long phys_size, addr_size;
+		u64 io_base;
+		u32 *rangesp;
+		u32 *interrupts, *clk, *spd;
+		char *typep;
+		int index, rlen, rentsize;
+
+		/* Ok, first check if it's under an "isa" parent */
+		isa = of_get_parent(np);
+		if (!isa || strcmp(isa->name, "isa")) {
+			DBG("%s: no isa parent found\n", np->full_name);
+			continue;
+		}
+
+		/* Now look for an "ibm,aix-loc" property that gives us ordering
+		 * if any...
+		 */
+	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
+
+		/* Get the ISA port number */
+		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);
+		if (reg == NULL)
+			goto next_port;
+		/* We assume the interrupt number isn't translated ... */
+		interrupts = (u32 *)get_property(np, "interrupts", NULL);
+		/* get clock freq. if present */
+		clk = (u32 *)get_property(np, "clock-frequency", NULL);
+		/* get default speed if present */
+		spd = (u32 *)get_property(np, "current-speed", NULL);
+		/* Default to locate at end of array */
+		index = old_serial_count; /* end of the array by default */
+
+		/* If we have a location index, then use it */
+		if (typep && *typep == 'S') {
+			index = simple_strtol(typep+1, NULL, 0) - 1;
+			/* if index is out of range, use end of array instead */
+			if (index >= MAX_LEGACY_SERIAL_PORTS)
+				index = old_serial_count;
+			/* if our index is still out of range, that mean that
+			 * array is full, we could scan for a free slot but that
+			 * make little sense to bother, just skip the port
+			 */
+			if (index >= MAX_LEGACY_SERIAL_PORTS)
+				goto next_port;
+			if (index >= old_serial_count)
+				old_serial_count = index + 1;
+			/* Check if there is a port who already claimed our slot */
+			if (serial_ports[index].iobase != 0) {
+				/* if we still have some room, move it, else override */
+				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
+					DBG("Moved legacy port %d -> %d\n", index,
+					    old_serial_count);
+					serial_ports[old_serial_count++] =
+						serial_ports[index];
+				} else {
+					DBG("Replacing legacy port %d\n", index);
+				}
+			}
+		}
+		if (index >= MAX_LEGACY_SERIAL_PORTS)
+			goto next_port;
+		if (index >= old_serial_count)
+			old_serial_count = index + 1;
+
+		/* Now fill the entry */
+		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
+		serial_ports[index].uartclk = (clk && *clk) ? *clk : BASE_BAUD * 16;
+		serial_ports[index].iobase = reg->address;
+		serial_ports[index].irq = interrupts ? interrupts[0] : 0;
+		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
+
+		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
+		    index,
+		    serial_ports[index].iobase,
+		    serial_ports[index].irq,
+		    serial_ports[index].uartclk);
+
+		/* Get phys address of IO reg for port 1 */
+		if (index != 0)
+			goto next_port;
+
+		pci = of_get_parent(isa);
+		if (!pci) {
+			DBG("%s: no pci parent found\n", np->full_name);
+			goto next_port;
+		}
+
+		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
+		if (rangesp == NULL) {
+			of_node_put(pci);
+			goto next_port;
+		}
+		rlen /= 4;
+
+		/* we need the #size-cells of the PCI bridge node itself */
+		phys_size = 1;
+		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
+		if (sizeprop != NULL)
+			phys_size = *sizeprop;
+		/* we need the parent #addr-cells */
+		addr_size = prom_n_addr_cells(pci);
+		rentsize = 3 + addr_size + phys_size;
+		io_base = 0;
+		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
+			if (((rangesp[0] >> 24) & 0x3) != 1)
+				continue; /* not IO space */
+			io_base = rangesp[3];
+			if (addr_size == 2)
+				io_base = (io_base << 32) | rangesp[4];
+		}
+		if (io_base != 0) {
+			*physport = io_base + reg->address;
+			if (default_speed && spd)
+				*default_speed = *spd;
+		}
+		of_node_put(pci);
+	next_port:
+		of_node_put(isa);
+	}
+
+	DBG(" <- generic_find_legacy_serial_port()\n");
+}
+
+static struct platform_device serial_device = {
+	.name	= "serial8250",
+	.id	= 0,
+	.dev	= {
+		.platform_data = serial_ports,
+	},
+};
+
+static int __init serial_dev_init(void)
+{
+	u64 phys;
+	unsigned int spd;
+
+	if (!ports_probed)
+		generic_find_legacy_serial_ports(&phys, &spd);
+	return platform_device_register(&serial_device);
+}
+arch_initcall(serial_dev_init);
--- linux-2.6.12/drivers/serial/Kconfig~	2005-08-11 13:51:50.000000000 +0100
+++ linux-2.6.12/drivers/serial/Kconfig	2005-08-15 21:13:41.000000000 +0100
@@ -77,6 +77,11 @@ config SERIAL_8250_CS
 
 	  If unsure, say N.
 
+config SERIAL_8250_OF
+       bool
+       default y
+       depends on PPC_OF && SERIAL_8250 != n
+
 config SERIAL_8250_ACPI
 	bool "8250/16550 device discovery via ACPI namespace"
 	default y if IA64
--- linux-2.6.12/arch/ppc64/kernel/setup.c~	2005-08-11 13:52:04.000000000 +0100
+++ linux-2.6.12/arch/ppc64/kernel/setup.c	2005-08-15 20:27:25.000000000 +0100
@@ -1147,186 +1147,6 @@ void __init setup_default_decr(void)
 	lpaca->next_jiffy_update_tb = get_tb() + tb_ticks_per_jiffy;
 }
 
-#ifndef CONFIG_PPC_ISERIES
-/*
- * This function can be used by platforms to "find" legacy serial ports.
- * It works for "serial" nodes under an "isa" node, and will try to
- * respect the "ibm,aix-loc" property if any. It works with up to 8
- * ports.
- */
-
-#define MAX_LEGACY_SERIAL_PORTS	8
-static struct plat_serial8250_port serial_ports[MAX_LEGACY_SERIAL_PORTS+1];
-static unsigned int old_serial_count;
-
-void __init generic_find_legacy_serial_ports(u64 *physport,
-		unsigned int *default_speed)
-{
-	struct device_node *np;
-	u32 *sizeprop;
-
-	struct isa_reg_property {
-		u32 space;
-		u32 address;
-		u32 size;
-	};
-	struct pci_reg_property {
-		struct pci_address addr;
-		u32 size_hi;
-		u32 size_lo;
-	};                                                                        
-
-	DBG(" -> generic_find_legacy_serial_port()\n");
-
-	*physport = 0;
-	if (default_speed)
-		*default_speed = 0;
-
-	np = of_find_node_by_path("/");
-	if (!np)
-		return;
-
-	/* First fill our array */
-	for (np = NULL; (np = of_find_node_by_type(np, "serial"));) {
-		struct device_node *isa, *pci;
-		struct isa_reg_property *reg;
-		unsigned long phys_size, addr_size, io_base;
-		u32 *rangesp;
-		u32 *interrupts, *clk, *spd;
-		char *typep;
-		int index, rlen, rentsize;
-
-		/* Ok, first check if it's under an "isa" parent */
-		isa = of_get_parent(np);
-		if (!isa || strcmp(isa->name, "isa")) {
-			DBG("%s: no isa parent found\n", np->full_name);
-			continue;
-		}
-		
-		/* Now look for an "ibm,aix-loc" property that gives us ordering
-		 * if any...
-		 */
-	 	typep = (char *)get_property(np, "ibm,aix-loc", NULL);
-
-		/* Get the ISA port number */
-		reg = (struct isa_reg_property *)get_property(np, "reg", NULL);	
-		if (reg == NULL)
-			goto next_port;
-		/* We assume the interrupt number isn't translated ... */
-		interrupts = (u32 *)get_property(np, "interrupts", NULL);
-		/* get clock freq. if present */
-		clk = (u32 *)get_property(np, "clock-frequency", NULL);
-		/* get default speed if present */
-		spd = (u32 *)get_property(np, "current-speed", NULL);
-		/* Default to locate at end of array */
-		index = old_serial_count; /* end of the array by default */
-
-		/* If we have a location index, then use it */
-		if (typep && *typep == 'S') {
-			index = simple_strtol(typep+1, NULL, 0) - 1;
-			/* if index is out of range, use end of array instead */
-			if (index >= MAX_LEGACY_SERIAL_PORTS)
-				index = old_serial_count;
-			/* if our index is still out of range, that mean that
-			 * array is full, we could scan for a free slot but that
-			 * make little sense to bother, just skip the port
-			 */
-			if (index >= MAX_LEGACY_SERIAL_PORTS)
-				goto next_port;
-			if (index >= old_serial_count)
-				old_serial_count = index + 1;
-			/* Check if there is a port who already claimed our slot */
-			if (serial_ports[index].iobase != 0) {
-				/* if we still have some room, move it, else override */
-				if (old_serial_count < MAX_LEGACY_SERIAL_PORTS) {
-					DBG("Moved legacy port %d -> %d\n", index,
-					    old_serial_count);
-					serial_ports[old_serial_count++] =
-						serial_ports[index];
-				} else {
-					DBG("Replacing legacy port %d\n", index);
-				}
-			}
-		}
-		if (index >= MAX_LEGACY_SERIAL_PORTS)
-			goto next_port;
-		if (index >= old_serial_count)
-			old_serial_count = index + 1;
-
-		/* Now fill the entry */
-		memset(&serial_ports[index], 0, sizeof(struct plat_serial8250_port));
-		serial_ports[index].uartclk = clk ? *clk : BASE_BAUD * 16;
-		serial_ports[index].iobase = reg->address;
-		serial_ports[index].irq = interrupts ? interrupts[0] : 0;
-		serial_ports[index].flags = ASYNC_BOOT_AUTOCONF;
-
-		DBG("Added legacy port, index: %d, port: %x, irq: %d, clk: %d\n",
-		    index,
-		    serial_ports[index].iobase,
-		    serial_ports[index].irq,
-		    serial_ports[index].uartclk);
-
-		/* Get phys address of IO reg for port 1 */
-		if (index != 0)
-			goto next_port;
-
-		pci = of_get_parent(isa);
-		if (!pci) {
-			DBG("%s: no pci parent found\n", np->full_name);
-			goto next_port;
-		}
-
-		rangesp = (u32 *)get_property(pci, "ranges", &rlen);
-		if (rangesp == NULL) {
-			of_node_put(pci);
-			goto next_port;
-		}
-		rlen /= 4;
-
-		/* we need the #size-cells of the PCI bridge node itself */
-		phys_size = 1;
-		sizeprop = (u32 *)get_property(pci, "#size-cells", NULL);
-		if (sizeprop != NULL)
-			phys_size = *sizeprop;
-		/* we need the parent #addr-cells */
-		addr_size = prom_n_addr_cells(pci);
-		rentsize = 3 + addr_size + phys_size;
-		io_base = 0;
-		for (;rlen >= rentsize; rlen -= rentsize,rangesp += rentsize) {
-			if (((rangesp[0] >> 24) & 0x3) != 1)
-				continue; /* not IO space */
-			io_base = rangesp[3];
-			if (addr_size == 2)
-				io_base = (io_base << 32) | rangesp[4];
-		}
-		if (io_base != 0) {
-			*physport = io_base + reg->address;
-			if (default_speed && spd)
-				*default_speed = *spd;
-		}
-		of_node_put(pci);
-	next_port:
-		of_node_put(isa);
-	}
-
-	DBG(" <- generic_find_legacy_serial_port()\n");
-}
-
-static struct platform_device serial_device = {
-	.name	= "serial8250",
-	.id	= 0,
-	.dev	= {
-		.platform_data = serial_ports,
-	},
-};
-
-static int __init serial_dev_init(void)
-{
-	return platform_device_register(&serial_device);
-}
-arch_initcall(serial_dev_init);
-
-#endif /* CONFIG_PPC_ISERIES */
 
 int check_legacy_ioport(unsigned long base_port)
 {
--- linux-2.6.12/include/asm-ppc/pc_serial.h~	2005-08-15 21:19:32.000000000 +0100
+++ linux-2.6.12/include/asm-ppc/pc_serial.h	2005-08-15 21:20:24.000000000 +0100
@@ -26,18 +26,4 @@
 #define RS_TABLE_SIZE  4
 #endif
 
-/* Standard COM flags (except for COM4, because of the 8514 problem) */
-#ifdef CONFIG_SERIAL_DETECT_IRQ
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST | ASYNC_AUTO_IRQ)
-#define STD_COM4_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_AUTO_IRQ)
-#else
-#define STD_COM_FLAGS (ASYNC_BOOT_AUTOCONF | ASYNC_SKIP_TEST)
-#define STD_COM4_FLAGS ASYNC_BOOT_AUTOCONF
-#endif
-
+#define SERIAL_PORT_DFNS /* */
-#define SERIAL_PORT_DFNS			\
-	/* UART CLK   PORT IRQ     FLAGS        */			\
-	{ 0, BASE_BAUD, 0x3F8, 4, STD_COM_FLAGS },	/* ttyS0 */	\
-	{ 0, BASE_BAUD, 0x2F8, 3, STD_COM_FLAGS },	/* ttyS1 */	\
-	{ 0, BASE_BAUD, 0x3E8, 4, STD_COM_FLAGS },	/* ttyS2 */	\
-	{ 0, BASE_BAUD, 0x2E8, 3, STD_COM4_FLAGS },	/* ttyS3 */


-- 
dwmw2

Re: [PATCH] Use platform device for 8250 registration

[Russell King]

On Wed, Aug 17, 2005 at 05:39:30PM +0100, David Woodhouse wrote:
> On Wed, 2005-08-17 at 12:11 +0100, Russell King wrote:
> > Doesn't PPC64 have NO_IRQ ?
> 
> PPC64 does but I find no definition of is_real_interrupt() other than
> the bogus one in 8250.c which checks for zero. PPC32 does not -- let's
> clean that one up separately.

Ok.  I'll give you this line now for this patch:

Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

so you're free to push it upstream whenever you deem is the right time.
Thanks.

-- 
Russell King

Re: When are machine checks suppose to be recoverable?

[Benjamin Herrenschmidt]

On Wed, 2005-08-17 at 09:00 -0500, Kumar Gala wrote:
> David's 8250 cleanup patch made me wondering when are machine checks  
> suppose to recoverable?  General class of conditions is what I'm  
> looking for here.
> 
> Is David's case due to some PCI master abort or something else?

Might be some issue on SMP machines... 

Ben.

Re: When are machine checks suppose to be recoverable?

[David Woodhouse]

On Thu, 2005-08-18 at 07:44 +1000, Benjamin Herrenschmidt wrote:
> On Wed, 2005-08-17 at 09:00 -0500, Kumar Gala wrote:
> > David's 8250 cleanup patch made me wondering when are machine checks  
> > suppose to recoverable?  General class of conditions is what I'm  
> > looking for here.
> > 
> > Is David's case due to some PCI master abort or something else?
> 
> Might be some issue on SMP machines... 

Yeah, that'll probably be the reason it turns out _not_ to be
recoverable despite our expectations. But that wasn't Kumar's question.

-- 
dwmw2

Re: When are machine checks suppose to be recoverable?

[Kumar Gala]

On Aug 17, 2005, at 5:30 PM, David Woodhouse wrote:

> On Thu, 2005-08-18 at 07:44 +1000, Benjamin Herrenschmidt wrote:
>
>> On Wed, 2005-08-17 at 09:00 -0500, Kumar Gala wrote:
>>
>>> David's 8250 cleanup patch made me wondering when are machine checks
>>>
>
>
>>> suppose to recoverable?  General class of conditions is what I'm
>>> looking for here.
>>>
>>> Is David's case due to some PCI master abort or something else?
>>>
>>
>> Might be some issue on SMP machines...
>>
>
> Yeah, that'll probably be the reason it turns out _not_ to be
> recoverable despite our expectations. But that wasn't Kumar's  
> question.

David's right, that wasn't my question :)  I was asking more about  
what cases do we actually recover and that is considered correct  
behavior.

- kumar

Re: When are machine checks suppose to be recoverable?

[Marcelo Tosatti]

On Wed, Aug 17, 2005 at 10:42:12PM -0500, Kumar Gala wrote:
> 
> On Aug 17, 2005, at 5:30 PM, David Woodhouse wrote:
> 
> >On Thu, 2005-08-18 at 07:44 +1000, Benjamin Herrenschmidt wrote:
> >
> >>On Wed, 2005-08-17 at 09:00 -0500, Kumar Gala wrote:
> >>
> >>>David's 8250 cleanup patch made me wondering when are machine checks
> >>>
> >
> >
> >>>suppose to recoverable?  General class of conditions is what I'm
> >>>looking for here.
> >>>
> >>>Is David's case due to some PCI master abort or something else?
> >>>
> >>
> >>Might be some issue on SMP machines...
> >>
> >
> >Yeah, that'll probably be the reason it turns out _not_ to be
> >recoverable despite our expectations. But that wasn't Kumar's  
> >question.
> 
> David's right, that wasn't my question :)  I was asking more about  
> what cases do we actually recover and that is considered correct  
> behavior.

Quoting David:

"My dual G4 PowerMac crashes sometimes when it probes for the (absent)
serial ports. Theoretically it's supposed to take a machine check and
recover -- but it doesn't always work like that."

Maybe you just need the proper entry in the exception table for IO
inb/outb?

Is there a stacktrace, David?

arch/ppc/kernel/traps.c:

/*
 * I/O accesses can cause machine checks on powermacs.
 * Check if the NIP corresponds to the address of a sync
 * instruction for which there is an entry in the exception
 * table.
 * Note that the 601 only takes a machine check on TEA
 * (transfer error ack) signal assertion, and does not
 * set any of the top 16 bits of SRR1.
 *  -- paulus.
 */
static inline int check_io_access(struct pt_regs *regs)
{
#ifdef CONFIG_PPC_PMAC
        unsigned long msr = regs->msr;
        const struct exception_table_entry *entry;
        unsigned int *nip = (unsigned int *)regs->nip;

        if (((msr & 0xffff0000) == 0 || (msr & (0x80000 | 0x40000)))
            && (entry = search_exception_tables(regs->nip)) != NULL) {

Re: When are machine checks suppose to be recoverable?

[David Woodhouse]

On Mon, 2005-08-22 at 23:56 -0300, Marcelo Tosatti wrote:
> "My dual G4 PowerMac crashes sometimes when it probes for the (absent)
> serial ports. Theoretically it's supposed to take a machine check and
> recover -- but it doesn't always work like that."
> 
> Maybe you just need the proper entry in the exception table for IO
> inb/outb?
> 
> Is there a stacktrace, David?

No, just a silent death when probing for one of the serial ports;
usually not the first. It doesn't seem to even make it into the machine
check handler. Adding printks into serial_inb and serial_outb makes it
much less likely to happen -- could be something bizarre about alignment
of the instructions which cause the machine check, or time elapsed from
one machine check to the next.

-- 
dwmw2

Re: When are machine checks suppose to be recoverable?

[Kumar Gala]

On Aug 23, 2005, at 11:43 AM, David Woodhouse wrote:

> On Mon, 2005-08-22 at 23:56 -0300, Marcelo Tosatti wrote:
>
>> "My dual G4 PowerMac crashes sometimes when it probes for the  
>> (absent)
>> serial ports. Theoretically it's supposed to take a machine check and
>> recover -- but it doesn't always work like that."
>>
>> Maybe you just need the proper entry in the exception table for IO
>> inb/outb?
>>
>> Is there a stacktrace, David?
>>
>
> No, just a silent death when probing for one of the serial ports;
> usually not the first. It doesn't seem to even make it into the  
> machine
> check handler. Adding printks into serial_inb and serial_outb makes it
> much less likely to happen -- could be something bizarre about  
> alignment
> of the instructions which cause the machine check, or time elapsed  
> from
> one machine check to the next.

If the machine is locking hard it could be check stopping due to  
getting a second machine check.

- kumar

Re: When are machine checks suppose to be recoverable?

[David Woodhouse]

On Tue, 2005-08-23 at 12:04 -0500, Kumar Gala wrote:
> If the machine is locking hard it could be check stopping due to  
> getting a second machine check.

Perhaps... but it's not the _second_ machine check. We already take 33
of them, and recover, while the i8042 driver is poking for hardware. We
also tend to survive a few while the 8250 driver is poking... but not
all of them.

Some timing issue with recovering and then causing another machine check
in quick succession?

-- 
dwmw2