[PATCH 0/3] PReP support

[PATCH 0/3] PReP support

[David Gibson]

Here is a string of three patches which make a first cut at supporting
PReP machines under arch/powerpc.  The first two are, I think, ready
for merge.  The third, well, up to you whether we ought to do the
numerous necessary cleanups before or after merge.

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

[PATCH 2/3] Make more OF-related bootwrapper functions available to non-OF platforms

[David Gibson]

Commit 2e6016133755eb3cc44e8efab92573d23ed75888 split up
arch/powerpc/boot/of.c so that some OF functions can be used on
platforms that don't want to use the overall OF platform boot code.
This is useful on things like PReP which can have an OF implementation
which is useful for debugging output, but inadequate for booting.

However, that commit didn't export quite enough things to make a
usable OF console on a non-OF system.  In particular, the device tree
manipulation performed to initialize the OF console code must
explicitly use the OF device tree, rather than the flattened device
tree, even if the system is otherwise booting using a flattened device
tree.  This patch makes it so.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---

 arch/powerpc/boot/of.c        |   30 ------------------------------
 arch/powerpc/boot/of.h        |    6 ++++++
 arch/powerpc/boot/ofconsole.c |    8 ++++----
 arch/powerpc/boot/oflib.c     |   40 +++++++++++++++++++++++++++++++++++-----
 4 files changed, 45 insertions(+), 39 deletions(-)

Index: working-2.6/arch/powerpc/boot/of.c
===================================================================
--- working-2.6.orig/arch/powerpc/boot/of.c	2007-06-15 13:24:11.000000000 +1000
+++ working-2.6/arch/powerpc/boot/of.c	2007-06-21 13:21:46.000000000 +1000
@@ -63,36 +63,6 @@ static void of_image_hdr(const void *hdr
 	}
 }
 
-static void *of_vmlinux_alloc(unsigned long size)
-{
-	void *p = malloc(size);
-
-	if (!p)
-		fatal("Can't allocate memory for kernel image!\n\r");
-
-	return p;
-}
-
-/*
- * OF device tree routines
- */
-static void *of_finddevice(const char *name)
-{
-	return (phandle) of_call_prom("finddevice", 1, 1, name);
-}
-
-static int of_getprop(const void *phandle, const char *name, void *buf,
-		const int buflen)
-{
-	return of_call_prom("getprop", 4, 1, phandle, name, buf, buflen);
-}
-
-static int of_setprop(const void *phandle, const char *name, const void *buf,
-		const int buflen)
-{
-	return of_call_prom("setprop", 4, 1, phandle, name, buf, buflen);
-}
-
 void platform_init(unsigned long a1, unsigned long a2, void *promptr)
 {
 	platform_ops.image_hdr = of_image_hdr;
Index: working-2.6/arch/powerpc/boot/of.h
===================================================================
--- working-2.6.orig/arch/powerpc/boot/of.h	2007-06-15 13:24:11.000000000 +1000
+++ working-2.6/arch/powerpc/boot/of.h	2007-06-21 13:22:14.000000000 +1000
@@ -7,7 +7,13 @@ typedef void *ihandle;
 void of_init(void *promptr);
 int of_call_prom(const char *service, int nargs, int nret, ...);
 void *of_claim(unsigned long virt, unsigned long size, unsigned long align);
+void *of_vmlinux_alloc(unsigned long size);
 void of_exit(void);
+void *of_finddevice(const char *name);
+int of_getprop(const void *phandle, const char *name, void *buf,
+	       const int buflen);
+int of_setprop(const void *phandle, const char *name, const void *buf,
+	       const int buflen);
 
 /* Console functions */
 void of_console_init(void);
Index: working-2.6/arch/powerpc/boot/oflib.c
===================================================================
--- working-2.6.orig/arch/powerpc/boot/oflib.c	2007-06-15 13:24:11.000000000 +1000
+++ working-2.6/arch/powerpc/boot/oflib.c	2007-06-21 13:21:58.000000000 +1000
@@ -110,25 +110,25 @@ static int check_of_version(void)
 	phandle oprom, chosen;
 	char version[64];
 
-	oprom = finddevice("/openprom");
+	oprom = of_finddevice("/openprom");
 	if (oprom == (phandle) -1)
 		return 0;
-	if (getprop(oprom, "model", version, sizeof(version)) <= 0)
+	if (of_getprop(oprom, "model", version, sizeof(version)) <= 0)
 		return 0;
 	version[sizeof(version)-1] = 0;
 	printf("OF version = '%s'\r\n", version);
 	if (!string_match(version, "Open Firmware, 1.")
 	    && !string_match(version, "FirmWorks,3."))
 		return 0;
-	chosen = finddevice("/chosen");
+	chosen = of_finddevice("/chosen");
 	if (chosen == (phandle) -1) {
-		chosen = finddevice("/chosen@0");
+		chosen = of_finddevice("/chosen@0");
 		if (chosen == (phandle) -1) {
 			printf("no chosen\n");
 			return 0;
 		}
 	}
-	if (getprop(chosen, "mmu", &chosen_mmu, sizeof(chosen_mmu)) <= 0) {
+	if (of_getprop(chosen, "mmu", &chosen_mmu, sizeof(chosen_mmu)) <= 0) {
 		printf("no mmu\n");
 		return 0;
 	}
@@ -166,7 +166,37 @@ void *of_claim(unsigned long virt, unsig
 	return (void *) virt;
 }
 
+void *of_vmlinux_alloc(unsigned long size)
+{
+	void *p = malloc(size);
+
+	if (!p)
+		fatal("Can't allocate memory for kernel image!\n\r");
+
+	return p;
+}
+
 void of_exit(void)
 {
 	of_call_prom("exit", 0, 0);
 }
+
+/*
+ * OF device tree routines
+ */
+void *of_finddevice(const char *name)
+{
+	return (phandle) of_call_prom("finddevice", 1, 1, name);
+}
+
+int of_getprop(const void *phandle, const char *name, void *buf,
+	       const int buflen)
+{
+	return of_call_prom("getprop", 4, 1, phandle, name, buf, buflen);
+}
+
+int of_setprop(const void *phandle, const char *name, const void *buf,
+	       const int buflen)
+{
+	return of_call_prom("setprop", 4, 1, phandle, name, buf, buflen);
+}
Index: working-2.6/arch/powerpc/boot/ofconsole.c
===================================================================
--- working-2.6.orig/arch/powerpc/boot/ofconsole.c	2007-06-15 13:24:11.000000000 +1000
+++ working-2.6/arch/powerpc/boot/ofconsole.c	2007-06-21 10:30:18.000000000 +1000
@@ -24,10 +24,10 @@ static int of_console_open(void)
 {
 	void *devp;
 
-	if (((devp = finddevice("/chosen")) != NULL)
-			&& (getprop(devp, "stdout", &of_stdout_handle,
-				sizeof(of_stdout_handle))
-				== sizeof(of_stdout_handle)))
+	if (((devp = of_finddevice("/chosen")) != NULL)
+	    && (of_getprop(devp, "stdout", &of_stdout_handle,
+			   sizeof(of_stdout_handle))
+		== sizeof(of_stdout_handle)))
 		return 0;
 
 	return -1;

Re: [PATCH 2/3] Make more OF-related bootwrapper functions available to non-OF platforms

[Segher Boessenkool]

> Commit 2e6016133755eb3cc44e8efab92573d23ed75888 split up
> arch/powerpc/boot/of.c so that some OF functions can be used on
> platforms that don't want to use the overall OF platform boot code.
> This is useful on things like PReP which can have an OF implementation
> which is useful for debugging output, but inadequate for booting.
>
> However, that commit didn't export quite enough things to make a
> usable OF console on a non-OF system.  In particular, the device tree
> manipulation performed to initialize the OF console code must
> explicitly use the OF device tree, rather than the flattened device
> tree, even if the system is otherwise booting using a flattened device
> tree.  This patch makes it so.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Looks good to me, I assume you tested it?  You didn't
mention on what hardware...


Segher

[PATCH 1/3] Abolish unused ucBoardRev variables

[David Gibson]

asm-powerpc/processor.h declares, and arch/ppc/platforms/prep_setup.c
defines variables ucBoardRev, ucBoardRevMaj and ucBoardRevMin which
are used nowhere in the current kernel (neither in arch/ppc nor
arch/powerpc).  This patch removes them.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---

 arch/ppc/platforms/prep_setup.c |    3 ---
 include/asm-powerpc/processor.h |    8 --------
 2 files changed, 11 deletions(-)

Index: working-2.6/arch/ppc/platforms/prep_setup.c
===================================================================
--- working-2.6.orig/arch/ppc/platforms/prep_setup.c	2007-06-19 16:38:20.000000000 +1000
+++ working-2.6/arch/ppc/platforms/prep_setup.c	2007-06-19 16:38:23.000000000 +1000
@@ -69,9 +69,6 @@
 
 TODC_ALLOC();
 
-unsigned char ucBoardRev;
-unsigned char ucBoardRevMaj, ucBoardRevMin;
-
 extern unsigned char prep_nvram_read_val(int addr);
 extern void prep_nvram_write_val(int addr,
 				 unsigned char val);
Index: working-2.6/include/asm-powerpc/processor.h
===================================================================
--- working-2.6.orig/include/asm-powerpc/processor.h	2007-06-19 16:38:03.000000000 +1000
+++ working-2.6/include/asm-powerpc/processor.h	2007-06-19 16:38:09.000000000 +1000
@@ -43,14 +43,6 @@ extern int _chrp_type;
 /* what kind of prep workstation we are */
 extern int _prep_type;
 
-/*
- * This is used to identify the board type from a given PReP board
- * vendor. Board revision is also made available. This will be moved
- * elsewhere soon
- */
-extern unsigned char ucBoardRev;
-extern unsigned char ucBoardRevMaj, ucBoardRevMin;
-
 #endif /* CONFIG_PPC_PREP */
 
 #endif /* defined(__KERNEL__) && defined(CONFIG_PPC32) */

[PATCH 3/3] First cut at PReP support for arch/powerpc

[David Gibson]

Here is an implementation to allow PReP systems to boot under the
arch/powerpc codebase, one of the few remaining platforms supported in
arch/ppc but not so far in arch/powerpc.

This support is still fairly incomplete - it needs to obtain a lot
more device information from the residual data and put it into the
device tree, rather than relying on hardcoded values (both in the code
and in prep.dts).  Likewise many peripherals will not be supported -
in particular the RTC is not yet supported, so the time will generally
be wrong.

Still, it should do enough to boot on a number of PReP machines, and
makes a reasonable start.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Too big for the list, the patch is at:
	http://ozlabs.org/~dgibson/home/prep-support

 arch/powerpc/Kconfig.debug           |    7 
 arch/powerpc/boot/Makefile           |    8 
 arch/powerpc/boot/dts/prep.dts       |  130 ++++
 arch/powerpc/boot/mkprep.c           |  178 ++++++
 arch/powerpc/boot/pnp.h              |  640 ++++++++++++++++++++++
 arch/powerpc/boot/prep.c             |  150 +++++
 arch/powerpc/boot/residual.h         |  313 ++++++++++
 arch/powerpc/boot/wrapper            |    7 
 arch/powerpc/configs/prep_defconfig  | 1006 +++++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/misc_32.S        |   27 
 arch/powerpc/kernel/setup_32.c       |    1 
 arch/powerpc/kernel/udbg.c           |    2 
 arch/powerpc/kernel/udbg_16550.c     |   28 
 arch/powerpc/platforms/Makefile      |    1 
 arch/powerpc/platforms/prep/Kconfig  |    3 
 arch/powerpc/platforms/prep/Makefile |    4 
 arch/powerpc/platforms/prep/nvram.c  |  135 ++++
 arch/powerpc/platforms/prep/pci.c    |   66 ++
 arch/powerpc/platforms/prep/setup.c  |  720 +++++++++++++++++++++++++
 include/asm-powerpc/residual.h       |  350 ++++++++++++
 include/asm-powerpc/udbg.h           |    1 
 21 files changed, 3772 insertions(+), 5 deletions(-)

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Milton Miller]

On Wed Jun 27 17:10:08 EST 2007, David Gibson wrote:
> Here is an implementation to allow PReP systems to boot under the
> arch/powerpc codebase, one of the few remaining platforms supported in
> arch/ppc but not so far in arch/powerpc.
>
> This support is still fairly incomplete - it needs to obtain a lot
> more device information from the residual data and put it into the
> device tree, rather than relying on hardcoded values (both in the code
> and in prep.dts).  Likewise many peripherals will not be supported -
> in particular the RTC is not yet supported, so the time will generally
> be wrong.


Hi David.

A few things I noticed:

(1) Your cleaned up mkprep.c seems to always put on a floppy header, 
where as before that was optional with the -php flag.  Is that header 
right for a hard disk partition?  Or would that just have the 1-sector 
header?  The comments in the old mkprep impiled the raw elf contents 
are used for netboot.  What if the zImage does't fit on the 1.44MB disk 
given in the header?

(2) Uour wrapper rule can leave behind zImage.dts.prep.elf, but there 
is no clean rule for that

(3) Is this the right platform to create the device tree from scratch 
by traversing fw data?  A significiant number of the properties are to 
be filled in or say IBM, which is incorrect.

(4) Adding a 5th udbg_putc method that calls a byte read to 
udbg_comport->lsr masked with THRE until 0 and performs a byte write to 
->thr shows we have the wrong abstraction.   Please make the inb/outb a 
method with the comport.  It would seem the actual init_xx could be in 
the platforms, passing inb, outb, and comport to a helper in 
udbg_16550.c.   I'm fine with declaring all the early inits in udbg.h

(5) the printk PReP architecture is in the middle of prep_find_bridges?

(6) #if 0 prep_request_io will need platform check

(7) do you expect both the kernel and the boot code will continue to 
need residual.h?  If so, can we have the boot dir make a copy like the 
zlib headers?  Or is this only temporary, the kernel only needs a copy 
of the raw blob to export to userspace in the same spot and the 
include/asm-powerpc copy will go away?

milton

udbg_16550.c and legacy_serial.c

[Milton Miller]

On Jun 27, 2007, at 6:22 AM, Milton Miller wrote:
> (4) Adding a 5th udbg_putc method that calls a byte read to 
> udbg_comport->lsr masked with THRE until 0 and performs a byte write 
> to ->thr shows we have the wrong abstraction.   Please make the 
> inb/outb a method with the comport.  It would seem the actual init_xx 
> could be in the platforms, passing inb, outb, and comport to a helper 
> in udbg_16550.c.   I'm fine with declaring all the early inits in 
> udbg.h
>


I'll repost that with a different subject in case someone else wants to 
work on that.


Also, while researching this, I found that check_legacy_serial_console 
has a check for legacy_serial_console < 0 in it.  Since that is set to 
the legacy_ports array we pass to the serial layer, it would seem all 
the other parsing to find the console is redundant; we can get the np 
that we already found.   However, it also means that the code to notice 
the stdout path is ch-a or ch-b will never be executed because of that 
check.

The comment about stdout being a wierd phandle should read 
"/chosen/stdout is a ihandle.  use linux,stdout-path instead".

no patch at this hour.

milton

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[David Gibson]

On Wed, Jun 27, 2007 at 06:22:27AM -0500, Milton Miller wrote:
> On Wed Jun 27 17:10:08 EST 2007, David Gibson wrote:
> > Here is an implementation to allow PReP systems to boot under the
> > arch/powerpc codebase, one of the few remaining platforms supported in
> > arch/ppc but not so far in arch/powerpc.
> >
> > This support is still fairly incomplete - it needs to obtain a lot
> > more device information from the residual data and put it into the
> > device tree, rather than relying on hardcoded values (both in the code
> > and in prep.dts).  Likewise many peripherals will not be supported -
> > in particular the RTC is not yet supported, so the time will generally
> > be wrong.
> 
> 
> Hi David.
> 
> A few things I noticed:
> 
> (1) Your cleaned up mkprep.c seems to always put on a floppy header, 
> where as before that was optional with the -php flag.  Is that header 
> right for a hard disk partition?  Or would that just have the 1-sector 
> header?

Well, the arch/ppc code always invokes mkprep with the -php option to
make the zImage.prep.  That's what I was basing my change on..

> The comments in the old mkprep impiled the raw elf contents 
> are used for netboot.  What if the zImage does't fit on the 1.44MB disk 
> given in the header?

Um... dunno.

> (2) Uour wrapper rule can leave behind zImage.dts.prep.elf, but there 
> is no clean rule for that

Hrm, yes.  And this is true for a number of intermediares that the
wrapper can make.  Fix later...

> (3) Is this the right platform to create the device tree from scratch 
> by traversing fw data?  A significiant number of the properties are to 
> be filled in or say IBM, which is incorrect.

Maybe, I'm certainly considering it.  For a first cut tweaking a fixed
tree was easier, but by the time we read all the information from the
residual data that we ought to, we may well be more-or-less making a
tree from scratch.

I'm hoping to merge libfdt into the kernel tree before attempting this
path, though.

> (4) Adding a 5th udbg_putc method that calls a byte read to 
> udbg_comport->lsr masked with THRE until 0 and performs a byte write to 
> ->thr shows we have the wrong abstraction.   Please make the inb/outb a 
> method with the comport.  It would seem the actual init_xx could be in 
> the platforms, passing inb, outb, and comport to a helper in 
> udbg_16550.c.   I'm fine with declaring all the early inits in udbg.h

Hrm.. I'm not entirely convinced.  The duplication of the serial
polling stuff is duplication, certainly, but it's not that complex a
bit of code:  I'm not sure it's worth inventing a second level of
indirection in the udbg stuff to get rid of it.  We'd still need the
putc/getc callbacks, to deal with non-8250 like debug methods
(e.g. OF, HV).

> (5) the printk PReP architecture is in the middle of prep_find_bridges?

Huh..?  I have no idea what you mean.

> (6) #if 0 prep_request_io will need platform check

If it stays.  I'm pretty unconvinced we need it at all - if we
actually need to use those devices, they can have their own entries in
the device tree and of_platform drivers which can do the
OF_request_ranges thing.

> (7) do you expect both the kernel and the boot code will continue to 
> need residual.h?  If so, can we have the boot dir make a copy like the 
> zlib headers?  Or is this only temporary, the kernel only needs a copy 
> of the raw blob to export to userspace in the same spot and the 
> include/asm-powerpc copy will go away?

For now, only the boot code needs it, and I'm hoping it can be kept
that way.  Anything the kernel proper needs from the residual should
probably be copied to a relevant place in the device tree.

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

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

> Here is an implementation to allow PReP systems to boot under the
> arch/powerpc codebase, one of the few remaining platforms supported in
> arch/ppc but not so far in arch/powerpc.

> Too big for the list, the patch is at:
> 	http://ozlabs.org/~dgibson/home/prep-support

Too lazy to split the patch into bite-size chunks, you mean ;-)

Anyway, here goes the DTS bits:

+/*
+ * PReP skeleton device tree
+ *
+ * Paul Mackerras <paulus@samba.org>
+ */
+
+/ {
+	device_type = "prep";
+	model = "IBM,PReP";

Not specific enough, leave it out or fill it in in the bootwrapper.

+	compatible = "prep";

Maybe fill this in, too.

+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {

Do all (supported) PReP boards have one CPU only?

+			device_type = "cpu";
+			reg = <0>;
+			clock-frequency = <0>; // filled in by bootwrapper
+			bus-frequency = <0>; // filled in by bootwrapper
+			timebase-frequency = <0>; // filled in by bootwrapper
+			i-cache-line-size = <0>; // filled in by bootwrapper
+			d-cache-line-size = <0>; // filled in by bootwrapper
+			d-cache-size = <0>; // filled in by bootwrapper
+			i-cache-size = <0>; // filled in by bootwrapper
+			external-control;

Really?

+			graphics;
+			performance-monitor;
+
+			l2-cache {
+				device_type = "cache";
+				i-cache-size = <00100000>;
+				d-cache-size = <00100000>;
+				i-cache-sets = <00008000>;
+				d-cache-sets = <00008000>;
+				i-cache-line-size = <00000020>;
+				d-cache-line-size = <00000020>;

Drop the leading zeroes, they make my head spin :-)

+				cache-unified;
+			};
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		// dummy range here, zImage wrapper will fill in the actual
+		// amount of memory from the residual data
+		reg = <00000000 00000000>;
+	};
+
+	pci@80000000 {
+		device_type = "pci";
+		compatible = "prep";

Is that specific enough?

+		clock-frequency = <01fca055>;
+		reg = <80000000 7effffff>;
+		8259-interrupt-acknowledge = <bffffff0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges=<01000000 00000000 00000000 80000000 00000000 00800000
+			01000000 00000000 00800000 81000000 00000000 3e800000
+			02000000 00000000 00000000 c0000000 00000000 01000000
+			02000000 00000000 01000000 c1000000 00000000 3e000000>;
+		interrupt-map-mask = <f800 0 0 7>;
+		interrupt-map = <6000 0 0 1	&MPIC 6 0
+				 8000 0 0 1	&MPIC 7 0
+				 9000 0 0 1	&MPIC 2 0
+				 b000 0 0 1	&MPIC 1 0>;

I can't believe this "ranges" and interrupt mapping will
work on all PReP systems...

+		isa {
+			device_type = "isa";
+			#address-cells = <2>;
+			#size-cells = <1>;
+			#interrupt-cells = <2>;
+			ranges = <00000001 00000000
+				  01005800 00000000 00000000  00010000
+				  00000000 00000000
+				  02005800 00000000 00000000  01000000>;
+
+			parallel {
+				device_type = "parallel";
+				compatible = "ecp", "pnpPNP,400";

"pnpPNP,401", "pnpPNP,400"

+				reg =  <00000001 000003bc  00000008
+					00000001 000007bc  00000006>;
+				interrupts = <00000007 00000003>;
+				interrupt-parent = <&PIC8259>;
+			};
+
+			serial@3f8 {
+				device_type = "serial";
+				compatible = "pnpPNP,501";

"pnpPNP,501", "pnpPNP,500" I'd say.  Many/some device
tree users will only care it is _some_ 8250 family thing.

+				clock-frequency = <001c2000>;
+				reg =  <00000001 000003f8  00000008>;
+				interrupts = <00000004 00000003>;
+				interrupt-parent = <&PIC8259>;
+			};
+			serial@2f8 {
+				device_type = "serial";
+				compatible = "pnpPNP,501";
+				clock-frequency = <001c2000>;
+				reg =  <00000001 000002f8  00000008>;
+				interrupts = <00000003 00000003>;
+				interrupt-parent = <&PIC8259>;
+			};
+			PIC8259: interrupt-controller {
+				device_type = "i8259";

device_type = "interrupt-controller".

+				compatible = "prep,iic";
+				reg = <	00000001 00000020  00000002
+					00000001 000000a0  00000002
+					00000001 000004d0  00000002>;
+				interrupts = <00000000 00000003
+					      00000002 00000003>;
+				interrupt-parent = <&MPIC>;
+			};
+		};
+
+		MPIC: interrupt-controller@d {
+			device_type = "open-pic";

device_type = "interrupt-controller".

+			compatible = "mpic";
+			reg = <	00006800 00000000 00000000  00000000 00000000
+				02006810 00000000 00000000  00000000 00040000>;
+			assigned-addresses = <
+				82006810 00000000 3afc0000  00000000 00040000>;
+		};
+	};
+
+	chosen {
+		linux,stdout-path = "/pci/isa/serial@3f8";
+	};
+};

What is the plan here -- have the bootwrapper build the
device tree / fill in the details from the residual data?


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Gabriel Paubert]

On Thu, Jun 28, 2007 at 10:59:35AM +0200, Segher Boessenkool wrote:
> > Here is an implementation to allow PReP systems to boot under the
> > arch/powerpc codebase, one of the few remaining platforms supported in
> > arch/ppc but not so far in arch/powerpc.
> 
> > Too big for the list, the patch is at:
> > 	http://ozlabs.org/~dgibson/home/prep-support
> 
> Too lazy to split the patch into bite-size chunks, you mean ;-)
> 
> Anyway, here goes the DTS bits:
> 
> +/*
> + * PReP skeleton device tree
> + *
> + * Paul Mackerras <paulus@samba.org>
> + */
> +
> +/ {
> +	device_type = "prep";
> +	model = "IBM,PReP";
> 
> Not specific enough, leave it out or fill it in in the bootwrapper.

Motorola also provided PreP boards (MTX and MVME at least).

> 
> +	compatible = "prep";
> 
> Maybe fill this in, too.
> 
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> 
> Do all (supported) PReP boards have one CPU only?

Not sure, but I don't have any. I believe that there were
dual processors MTX boards, and dual 604 MVME boards were
offered (but probably not very popular).

> 
> +			device_type = "cpu";
> +			reg = <0>;
> +			clock-frequency = <0>; // filled in by bootwrapper
> +			bus-frequency = <0>; // filled in by bootwrapper
> +			timebase-frequency = <0>; // filled in by bootwrapper
> +			i-cache-line-size = <0>; // filled in by bootwrapper
> +			d-cache-line-size = <0>; // filled in by bootwrapper
> +			d-cache-size = <0>; // filled in by bootwrapper
> +			i-cache-size = <0>; // filled in by bootwrapper
> +			external-control;
> 
> Really?

Well, is anybody actually using eciwx/ecowx?


> 
> +			graphics;
> +			performance-monitor;
> +
> +			l2-cache {
> +				device_type = "cache";
> +				i-cache-size = <00100000>;
> +				d-cache-size = <00100000>;
> +				i-cache-sets = <00008000>;
> +				d-cache-sets = <00008000>;
> +				i-cache-line-size = <00000020>;
> +				d-cache-line-size = <00000020>;
> 
> Drop the leading zeroes, they make my head spin :-)

It's also wrong, my boards have 256kB of L2 cache.


> 
> +				cache-unified;
> +			};
> +		};
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		// dummy range here, zImage wrapper will fill in the actual
> +		// amount of memory from the residual data
> +		reg = <00000000 00000000>;
> +	};
> +
> +	pci@80000000 {
> +		device_type = "pci";
> +		compatible = "prep";
> 
> Is that specific enough?

On the MVME5100, actually the mapping is more CHRP like, and PCI I/O
space is smaller and at a higher address. Actually for
MVME2400/2600/2700, I wrote a bootloader that reprograms the bridge
in a CHRP like mode since nobody needs almost 1GB of PCI I/O space
but having 1.5GB of PCI memory space is very useful.

> 
> +		clock-frequency = <01fca055>;
> +		reg = <80000000 7effffff>;
> +		8259-interrupt-acknowledge = <bffffff0>;

Not always. It is at feff0030 on Raven/Falcon/Hawk boards (given
by a system address assigned to the 8259 PIC in the residual data).

> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges=<01000000 00000000 00000000 80000000 00000000 00800000
> +			01000000 00000000 00800000 81000000 00000000 3e800000
> +			02000000 00000000 00000000 c0000000 00000000 01000000
> +			02000000 00000000 01000000 c1000000 00000000 3e000000>;
> +		interrupt-map-mask = <f800 0 0 7>;
> +		interrupt-map = <6000 0 0 1	&MPIC 6 0
> +				 8000 0 0 1	&MPIC 7 0
> +				 9000 0 0 1	&MPIC 2 0
> +				 b000 0 0 1	&MPIC 1 0>;
> 
> I can't believe this "ranges" and interrupt mapping will
> work on all PReP systems...

Neither do I.

> +		isa {
> +			device_type = "isa";
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +			#interrupt-cells = <2>;
> +			ranges = <00000001 00000000
> +				  01005800 00000000 00000000  00010000
> +				  00000000 00000000
> +				  02005800 00000000 00000000  01000000>;
> +
> +			parallel {
> +				device_type = "parallel";
> +				compatible = "ecp", "pnpPNP,400";
> 
> "pnpPNP,401", "pnpPNP,400"
> 
> +				reg =  <00000001 000003bc  00000008
> +					00000001 000007bc  00000006>;
> +				interrupts = <00000007 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};
> +
> +			serial@3f8 {
> +				device_type = "serial";
> +				compatible = "pnpPNP,501";
> 
> "pnpPNP,501", "pnpPNP,500" I'd say.  Many/some device
> tree users will only care it is _some_ 8250 family thing.
> 
> +				clock-frequency = <001c2000>;
> +				reg =  <00000001 000003f8  00000008>;
> +				interrupts = <00000004 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};
> +			serial@2f8 {
> +				device_type = "serial";
> +				compatible = "pnpPNP,501";
> +				clock-frequency = <001c2000>;
> +				reg =  <00000001 000002f8  00000008>;
> +				interrupts = <00000003 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};

Some of my boards have only one serial port (and also only 1 in the residual data).

> +			PIC8259: interrupt-controller {
> +				device_type = "i8259";
> 
> device_type = "interrupt-controller".
> 
> +				compatible = "prep,iic";
> +				reg = <	00000001 00000020  00000002
> +					00000001 000000a0  00000002
> +					00000001 000004d0  00000002>;
> +				interrupts = <00000000 00000003
> +					      00000002 00000003>;
> +				interrupt-parent = <&MPIC>;
> +			};
> +		};
> +
> +		MPIC: interrupt-controller@d {
> +			device_type = "open-pic";
> 
> device_type = "interrupt-controller".
> 
> +			compatible = "mpic";
> +			reg = <	00006800 00000000 00000000  00000000 00000000
> +				02006810 00000000 00000000  00000000 00040000>;
> +			assigned-addresses = <
> +				82006810 00000000 3afc0000  00000000 00040000>;
> +		};
> +	};
> +
> +	chosen {
> +		linux,stdout-path = "/pci/isa/serial@3f8";
> +	};
> +};
> 
> What is the plan here -- have the bootwrapper build the
> device tree / fill in the details from the residual data?

I think so. I might have some time to try a more recent kernel
on MVME2400/2600 boards next week.

	Regards,
	Gabriel

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>> +			external-control;
>>
>> Really?
>
> Well, is anybody actually using eciwx/ecowx?

That's not the point -- the device tree should only
say "external-control" if the CPU actually supports
it; AFAIK, that's 601 only.

>> +	pci@80000000 {
>> +		device_type = "pci";
>> +		compatible = "prep";
>>
>> Is that specific enough?
>
> On the MVME5100, actually the mapping is more CHRP like, and PCI I/O
> space is smaller and at a higher address.

Right, so it's not; "compatible" should specify the
model of PCI host bridge, instead.

>> What is the plan here -- have the bootwrapper build the
>> device tree / fill in the details from the residual data?
>
> I think so.

Right, good.  What's the way forward then -- David, rename
this DTS file to <board name>.dts so you have something to
boot, and at the same time work on the bootwrapper?

>  I might have some time to try a more recent kernel
> on MVME2400/2600 boards next week.

Great!


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Gabriel Paubert]

On Mon, Jul 02, 2007 at 01:51:42PM +0200, Segher Boessenkool wrote:
> >>+			external-control;
> >>
> >>Really?
> >
> >Well, is anybody actually using eciwx/ecowx?
> 
> That's not the point -- the device tree should only
> say "external-control" if the CPU actually supports
> it; AFAIK, that's 601 only.

I wonder whether you are mixing up external control
and direct-store segments (the "T" bit in segment registers).

>From my docs, external control has been supported by many 
processors, up to many variants of the 750 and even 7440/7450. 
Now I've never seen any hardware which could make use of these 
instructions (you'd need a device on the processor bus that 
reacts to the special bus cycles generated by the ec[io]wx 
instructions, since no host bridge I've ever met handles them. 
I've not seen support for external control in the Linux kernel 
either (you'd need to setup the EAR SPR and probablly to modify
it on context switches).

OTOH, the direct-store segments were only implemented in the 
original 601, 603 (not 603e) and 604 processors (can't remember 
about the 604e) and even then the 601 had special features
in this area.



> 
> >>+	pci@80000000 {
> >>+		device_type = "pci";
> >>+		compatible = "prep";
> >>
> >>Is that specific enough?
> >
> >On the MVME5100, actually the mapping is more CHRP like, and PCI I/O
> >space is smaller and at a higher address.
> 
> Right, so it's not; "compatible" should specify the
> model of PCI host bridge, instead.

Well, the PCI host bridge is reprogrammable in a very flexible way
and  and I actually wrote a boot loader that reprograms it the
way I wanted address space to look like. I shall send it to
you in a PM (it is quite big, it includes an x86 emulator
which is able to initialize the VGA PMC modules I have by
running the BIOS).

	Regards,
	Gabriel

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>>>> +			external-control;
>>>>
>>>> Really?
>>>
>>> Well, is anybody actually using eciwx/ecowx?
>>
>> That's not the point -- the device tree should only
>> say "external-control" if the CPU actually supports
>> it; AFAIK, that's 601 only.
>
> I wonder whether you are mixing up external control
> and direct-store segments (the "T" bit in segment registers).

=93external-control=94 prop-encoded-array: <none>
	This property, if present, indicates that the PowerPC
	microprocessor defined by this CPU node implements the
	External Control Facility as described in the =93Optional
	Facilities and Instructions=94 appendix of Book II of [2].
	The absence of his property indicates that the PowerPC
	microprocessor defined by this CPU node does not support
	the External Control Facility.

No, I don't think I'm mixing up anything.  G2 CPUs don't
support this stuff, do they?

>> =46rom my docs, external control has been supported by many
> processors, up to many variants of the 750 and even 7440/7450.

Hrm I don't think so...  </me looks>...  erm yeah.  It's just
that it isn't usable on any of those systems ;-)

> Now I've never seen any hardware which could make use of these
> instructions (you'd need a device on the processor bus that
> reacts to the special bus cycles generated by the ec[io]wx
> instructions, since no host bridge I've ever met handles them.

Yeah exactly.

> I've not seen support for external control in the Linux kernel
> either (you'd need to setup the EAR SPR and probablly to modify
> it on context switches).
>
> OTOH, the direct-store segments were only implemented in the
> original 601, 603 (not 603e) and 604 processors (can't remember
> about the 604e) and even then the 601 had special features
> in this area.

Yah.

>>>> +	pci@80000000 {
>>>> +		device_type =3D "pci";
>>>> +		compatible =3D "prep";
>>>>
>>>> Is that specific enough?
>>>
>>> On the MVME5100, actually the mapping is more CHRP like, and PCI I/O
>>> space is smaller and at a higher address.
>>
>> Right, so it's not; "compatible" should specify the
>> model of PCI host bridge, instead.
>
> Well, the PCI host bridge is reprogrammable in a very flexible way
> and  and I actually wrote a boot loader that reprograms it the
> way I wanted address space to look like.

"compatible" should just say what exactly the hardware is;
"prep" isn't good enough to describe a certain PCI host
bridge (it doesn't say it is a PHB, to start with!)

> I shall send it to
> you in a PM (it is quite big, it includes an x86 emulator
> which is able to initialize the VGA PMC modules I have by
> running the BIOS).

Hrm I'm not sure what I should do with that?


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Tom Gall]

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

On 6/28/07, Gabriel Paubert <paubert@iram.es> wrote:
>
> On Thu, Jun 28, 2007 at 10:59:35AM +0200, Segher Boessenkool wrote:
> <snip>
> > Do all (supported) PReP boards have one CPU only?
>
> Not sure, but I don't have any. I believe that there were
> dual processors MTX boards, and dual 604 MVME boards were
> offered (but probably not very popular).


There were several IBM RS/6000 models that  were PReP and were at least dual
systems. The 240 I recall was a dual 604. There were some Fxx and Exx models
I think might have gone all the way up to 4 ways but my memory is getting
fuzzy.

One thing for sure ... not alot of this hardware exists anymore.

Regards,

Tom

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

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Ulrich Teichert]

Hi,

>> On Thu, Jun 28, 2007 at 10:59:35AM +0200, Segher Boessenkool wrote:
>> <snip>
>> > Do all (supported) PReP boards have one CPU only?
>>
>> Not sure, but I don't have any. I believe that there were
>> dual processors MTX boards, and dual 604 MVME boards were
>> offered (but probably not very popular).
>
>
>There were several IBM RS/6000 models that  were PReP and were at least dual
>systems. The 240 I recall was a dual 604. There were some Fxx and Exx models
>I think might have gone all the way up to 4 ways but my memory is getting
>fuzzy.

I have a 240 at home and it's definitely a dual PReP box. I think that the H70
is PReP, too - that would be a 4 CPU box.

>One thing for sure ... not alot of this hardware exists anymore.
[del]

Oh, the 43p 140 shows up ebay quite often - for reasonable prices in Germany.
Even the multi CPU boxes are available from time to time.

HTH,
Uli
-- 
Dipl. Inf. Ulrich Teichert|e-mail: Ulrich.Teichert@gmx.de
Stormweg 24               |listening to: Channel 13 Is Haunted (Hex Dispensers)
24539 Neumuenster, Germany|Adrenalin (Supabond), Ne Me Touch Pas (Opération S)

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[David Gibson]

On Thu, Jun 28, 2007 at 12:00:20PM +0200, Gabriel Paubert wrote:
> On Thu, Jun 28, 2007 at 10:59:35AM +0200, Segher Boessenkool wrote:
> > > Here is an implementation to allow PReP systems to boot under the
> > > arch/powerpc codebase, one of the few remaining platforms supported in
> > > arch/ppc but not so far in arch/powerpc.
> > 
> > > Too big for the list, the patch is at:
> > > 	http://ozlabs.org/~dgibson/home/prep-support
> > 
> > Too lazy to split the patch into bite-size chunks, you mean ;-)
> > 
> > Anyway, here goes the DTS bits:
> > 
> > +/*
> > + * PReP skeleton device tree
> > + *
> > + * Paul Mackerras <paulus@samba.org>
> > + */
> > +
> > +/ {
> > +	device_type = "prep";
> > +	model = "IBM,PReP";
> > 
> > Not specific enough, leave it out or fill it in in the bootwrapper.
> 
> Motorola also provided PreP boards (MTX and MVME at least).
> 
> > 
> > +	compatible = "prep";
> > 
> > Maybe fill this in, too.
> > 
> > +	#address-cells = <1>;
> > +	#size-cells = <1>;
> > +
> > +	cpus {
> > +		#address-cells = <1>;
> > +		#size-cells = <0>;
> > +
> > +		cpu@0 {
> > 
> > Do all (supported) PReP boards have one CPU only?
> 
> Not sure, but I don't have any. I believe that there were
> dual processors MTX boards, and dual 604 MVME boards were
> offered (but probably not very popular).
> 
> > 
> > +			device_type = "cpu";
> > +			reg = <0>;
> > +			clock-frequency = <0>; // filled in by bootwrapper
> > +			bus-frequency = <0>; // filled in by bootwrapper
> > +			timebase-frequency = <0>; // filled in by bootwrapper
> > +			i-cache-line-size = <0>; // filled in by bootwrapper
> > +			d-cache-line-size = <0>; // filled in by bootwrapper
> > +			d-cache-size = <0>; // filled in by bootwrapper
> > +			i-cache-size = <0>; // filled in by bootwrapper
> > +			external-control;
> > 
> > Really?
> 
> Well, is anybody actually using eciwx/ecowx?

Eh, I've removed those properties anyway, the kernel never looks at
them anyway.

> > +			graphics;
> > +			performance-monitor;
> > +
> > +			l2-cache {
> > +				device_type = "cache";
> > +				i-cache-size = <00100000>;
> > +				d-cache-size = <00100000>;
> > +				i-cache-sets = <00008000>;
> > +				d-cache-sets = <00008000>;
> > +				i-cache-line-size = <00000020>;
> > +				d-cache-line-size = <00000020>;
> > 
> > Drop the leading zeroes, they make my head spin :-)
> 
> It's also wrong, my boards have 256kB of L2 cache.

Yeah, I need to fill in the L2 cache information from the residual as
well.

> > +				cache-unified;
> > +			};
> > +		};
> > +	};
> > +
> > +	memory {
> > +		device_type = "memory";
> > +		// dummy range here, zImage wrapper will fill in the actual
> > +		// amount of memory from the residual data
> > +		reg = <00000000 00000000>;
> > +	};
> > +
> > +	pci@80000000 {
> > +		device_type = "pci";
> > +		compatible = "prep";
> > 
> > Is that specific enough?
> 
> On the MVME5100, actually the mapping is more CHRP like, and PCI I/O
> space is smaller and at a higher address. Actually for
> MVME2400/2600/2700, I wrote a bootloader that reprograms the bridge
> in a CHRP like mode since nobody needs almost 1GB of PCI I/O space
> but having 1.5GB of PCI memory space is very useful.

Hrm... ok.  Now I'm a bit confused, because I thought what I'd put in
the device tree for PCI was all based on what was already hardcoded /
assumed in prep_pci.c anyway.

> > +		clock-frequency = <01fca055>;
> > +		reg = <80000000 7effffff>;
> > +		8259-interrupt-acknowledge = <bffffff0>;
> 
> Not always. It is at feff0030 on Raven/Falcon/Hawk boards (given
> by a system address assigned to the 8259 PIC in the residual data).
> 
> > +		#address-cells = <3>;
> > +		#size-cells = <2>;
> > +		ranges=<01000000 00000000 00000000 80000000 00000000 00800000
> > +			01000000 00000000 00800000 81000000 00000000 3e800000
> > +			02000000 00000000 00000000 c0000000 00000000 01000000
> > +			02000000 00000000 01000000 c1000000 00000000 3e000000>;
> > +		interrupt-map-mask = <f800 0 0 7>;
> > +		interrupt-map = <6000 0 0 1	&MPIC 6 0
> > +				 8000 0 0 1	&MPIC 7 0
> > +				 9000 0 0 1	&MPIC 2 0
> > +				 b000 0 0 1	&MPIC 1 0>;
> > 
> > I can't believe this "ranges" and interrupt mapping will
> > work on all PReP systems...
> 
> Neither do I.
> 
> > +		isa {
> > +			device_type = "isa";
> > +			#address-cells = <2>;
> > +			#size-cells = <1>;
> > +			#interrupt-cells = <2>;
> > +			ranges = <00000001 00000000
> > +				  01005800 00000000 00000000  00010000
> > +				  00000000 00000000
> > +				  02005800 00000000 00000000  01000000>;
> > +
> > +			parallel {
> > +				device_type = "parallel";
> > +				compatible = "ecp", "pnpPNP,400";
> > 
> > "pnpPNP,401", "pnpPNP,400"

Adjusted.

> > +				reg =  <00000001 000003bc  00000008
> > +					00000001 000007bc  00000006>;
> > +				interrupts = <00000007 00000003>;
> > +				interrupt-parent = <&PIC8259>;
> > +			};
> > +
> > +			serial@3f8 {
> > +				device_type = "serial";
> > +				compatible = "pnpPNP,501";
> > 
> > "pnpPNP,501", "pnpPNP,500" I'd say.  Many/some device
> > tree users will only care it is _some_ 8250 family thing.

Ok.

> > +				clock-frequency = <001c2000>;
> > +				reg =  <00000001 000003f8  00000008>;
> > +				interrupts = <00000004 00000003>;
> > +				interrupt-parent = <&PIC8259>;
> > +			};
> > +			serial@2f8 {
> > +				device_type = "serial";
> > +				compatible = "pnpPNP,501";
> > +				clock-frequency = <001c2000>;
> > +				reg =  <00000001 000002f8  00000008>;
> > +				interrupts = <00000003 00000003>;
> > +				interrupt-parent = <&PIC8259>;
> > +			};
> 
> Some of my boards have only one serial port (and also only 1 in the residual data).

Ok, I'll have to think about how to implement that.  Probably wants to
wait until we've integrated libfdt for more flexible manipulation of
the device tree.

> > +			PIC8259: interrupt-controller {
> > +				device_type = "i8259";
> > 
> > device_type = "interrupt-controller".

Is that really right?  The MPIC binding, at least, has device_type =
"open-pic" rather than "interrupt-controller".

> > +				compatible = "prep,iic";
> > +				reg = <	00000001 00000020  00000002
> > +					00000001 000000a0  00000002
> > +					00000001 000004d0  00000002>;
> > +				interrupts = <00000000 00000003
> > +					      00000002 00000003>;
> > +				interrupt-parent = <&MPIC>;
> > +			};
> > +		};
> > +
> > +		MPIC: interrupt-controller@d {
> > +			device_type = "open-pic";
> > 
> > device_type = "interrupt-controller".

Not according to the binding in booting-without-of.txt

> > +			compatible = "mpic";
> > +			reg = <	00006800 00000000 00000000  00000000 00000000
> > +				02006810 00000000 00000000  00000000 00040000>;
> > +			assigned-addresses = <
> > +				82006810 00000000 3afc0000  00000000 00040000>;
> > +		};
> > +	};
> > +
> > +	chosen {
> > +		linux,stdout-path = "/pci/isa/serial@3f8";
> > +	};
> > +};
> > 
> > What is the plan here -- have the bootwrapper build the
> > device tree / fill in the details from the residual data?
> 
> I think so. I might have some time to try a more recent kernel
> on MVME2400/2600 boards next week.
> 
> 	Regards,
> 	Gabriel
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
> 

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

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Jon Loeliger]

On Fri, 2007-08-03 at 01:35, David Gibson wrote:


> > > +			PIC8259: interrupt-controller {
> > > +				device_type = "i8259";
> > > 
> > > device_type = "interrupt-controller".
> 
> Is that really right?  The MPIC binding, at least, has device_type =
> "open-pic" rather than "interrupt-controller".
> 
> > > +				compatible = "prep,iic";
> > > +				reg = <	00000001 00000020  00000002
> > > +					00000001 000000a0  00000002
> > > +					00000001 000004d0  00000002>;
> > > +				interrupts = <00000000 00000003
> > > +					      00000002 00000003>;
> > > +				interrupt-parent = <&MPIC>;
> > > +			};
> > > +		};
> > > +
> > > +		MPIC: interrupt-controller@d {
> > > +			device_type = "open-pic";
> > > 
> > > device_type = "interrupt-controller".
> 
> Not according to the binding in booting-without-of.txt

My understanding here, though possibly flawed, is that the current
implementation has "open-pic" but _should_ have "interrupt-controller"
as that is the officially correct name.

I _think_ this means we need a transitional period where we update
the code to look for "interrupt-controller", and obsoletedly, looks
for the "open-pic", while we transition to the new, correct name.

I'm just betting that if I'm wrong Segher will tell me! :-)

jdl

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>>>> +		MPIC: interrupt-controller@d {
>>>> +			device_type = "open-pic";
>>>>
>>>> device_type = "interrupt-controller".
>>
>> Not according to the binding in booting-without-of.txt
>
> My understanding here, though possibly flawed, is that the current
> implementation has "open-pic" but _should_ have "interrupt-controller"
> as that is the officially correct name.
>
> I _think_ this means we need a transitional period where we update
> the code to look for "interrupt-controller", and obsoletedly, looks
> for the "open-pic", while we transition to the new, correct name.
>
> I'm just betting that if I'm wrong Segher will tell me! :-)

You're wrong :-)

The kernel shouldn't look at device_type at all.


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>>> +			PIC8259: interrupt-controller {
>>> +				device_type = "i8259";
>>>
>>> device_type = "interrupt-controller".
>
> Is that really right?  The MPIC binding, at least, has device_type =
> "open-pic" rather than "interrupt-controller".

I got confused.  "open-pic" as device_type is nonsense, because
it doesn't define anything an OF can work with itself.  However,
since some older interrupt controller bindings (pre-imap) define
a device_type, the interrupt mapping recommended practice had to
work around this, so it specified an empty "interrupt-controller"
property for all interrupt controller nodes.

I don't know what the specified device_type for 8259s is; it
seems to me you don't need it in a flat device tree at all.

>>> +		MPIC: interrupt-controller@d {
>>> +			device_type = "open-pic";
>>>
>>> device_type = "interrupt-controller".
>
> Not according to the binding in booting-without-of.txt

Yeah, mea culpa.  Again, you can just leave it out I think.


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[David Gibson]

On Thu, Jun 28, 2007 at 10:59:35AM +0200, Segher Boessenkool wrote:
> > Here is an implementation to allow PReP systems to boot under the
> > arch/powerpc codebase, one of the few remaining platforms supported in
> > arch/ppc but not so far in arch/powerpc.
> 
> > Too big for the list, the patch is at:
> > 	http://ozlabs.org/~dgibson/home/prep-support
> 
> Too lazy to split the patch into bite-size chunks, you mean ;-)

Well... much as I like small patches, I don't really like having a big
string of patches, each of which does basically nothing on its own,
i.e. split up just for the sake of making smaller, rather than into
separate logically separate changes.

> Anyway, here goes the DTS bits:
> 
> +/*
> + * PReP skeleton device tree
> + *
> + * Paul Mackerras <paulus@samba.org>
> + */
> +
> +/ {
> +	device_type = "prep";
> +	model = "IBM,PReP";
> 
> Not specific enough, leave it out or fill it in in the bootwrapper.

Yeah, I should fill that in with the string from the residual data.
Haven't gotten around to it.

> +	compatible = "prep";
> 
> Maybe fill this in, too.
> 
> +	#address-cells = <1>;
> +	#size-cells = <1>;
> +
> +	cpus {
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +
> +		cpu@0 {
> 
> Do all (supported) PReP boards have one CPU only?

Well, depends what you mean by "supported", really.  My arch/powerpc
port only support UP boards so far...  I realise this will need to
change at some point.

> 
> +			device_type = "cpu";
> +			reg = <0>;
> +			clock-frequency = <0>; // filled in by bootwrapper
> +			bus-frequency = <0>; // filled in by bootwrapper
> +			timebase-frequency = <0>; // filled in by bootwrapper
> +			i-cache-line-size = <0>; // filled in by bootwrapper
> +			d-cache-line-size = <0>; // filled in by bootwrapper
> +			d-cache-size = <0>; // filled in by bootwrapper
> +			i-cache-size = <0>; // filled in by bootwrapper
> +			external-control;
> 
> Really?

No idea, just copied that from earlier work of Paulus'.  Don't even
know what the property means.

> +			graphics;
> +			performance-monitor;
> +
> +			l2-cache {
> +				device_type = "cache";
> +				i-cache-size = <00100000>;
> +				d-cache-size = <00100000>;
> +				i-cache-sets = <00008000>;
> +				d-cache-sets = <00008000>;
> +				i-cache-line-size = <00000020>;
> +				d-cache-line-size = <00000020>;
> 
> Drop the leading zeroes, they make my head spin :-)
> 
> +				cache-unified;
> +			};
> +		};
> +	};
> +
> +	memory {
> +		device_type = "memory";
> +		// dummy range here, zImage wrapper will fill in the actual
> +		// amount of memory from the residual data
> +		reg = <00000000 00000000>;
> +	};
> +
> +	pci@80000000 {
> +		device_type = "pci";
> +		compatible = "prep";
> 
> Is that specific enough?

Well, AFAICT, the prep PCI code doesn't need any more info.

> +		clock-frequency = <01fca055>;
> +		reg = <80000000 7effffff>;
> +		8259-interrupt-acknowledge = <bffffff0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges=<01000000 00000000 00000000 80000000 00000000 00800000
> +			01000000 00000000 00800000 81000000 00000000 3e800000
> +			02000000 00000000 00000000 c0000000 00000000 01000000
> +			02000000 00000000 01000000 c1000000 00000000 3e000000>;
> +		interrupt-map-mask = <f800 0 0 7>;
> +		interrupt-map = <6000 0 0 1	&MPIC 6 0
> +				 8000 0 0 1	&MPIC 7 0
> +				 9000 0 0 1	&MPIC 2 0
> +				 b000 0 0 1	&MPIC 1 0>;
> 
> I can't believe this "ranges" and interrupt mapping will
> work on all PReP systems...

Probably not, but it should work on a chunk of them.  Like I say,
there's still a good deal more that needs to be filled in from
residual data or wherever.

> +		isa {
> +			device_type = "isa";
> +			#address-cells = <2>;
> +			#size-cells = <1>;
> +			#interrupt-cells = <2>;
> +			ranges = <00000001 00000000
> +				  01005800 00000000 00000000  00010000
> +				  00000000 00000000
> +				  02005800 00000000 00000000  01000000>;
> +
> +			parallel {
> +				device_type = "parallel";
> +				compatible = "ecp", "pnpPNP,400";
> 
> "pnpPNP,401", "pnpPNP,400"
> 
> +				reg =  <00000001 000003bc  00000008
> +					00000001 000007bc  00000006>;
> +				interrupts = <00000007 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};
> +
> +			serial@3f8 {
> +				device_type = "serial";
> +				compatible = "pnpPNP,501";
> 
> "pnpPNP,501", "pnpPNP,500" I'd say.  Many/some device
> tree users will only care it is _some_ 8250 family thing.
> 
> +				clock-frequency = <001c2000>;
> +				reg =  <00000001 000003f8  00000008>;
> +				interrupts = <00000004 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};
> +			serial@2f8 {
> +				device_type = "serial";
> +				compatible = "pnpPNP,501";
> +				clock-frequency = <001c2000>;
> +				reg =  <00000001 000002f8  00000008>;
> +				interrupts = <00000003 00000003>;
> +				interrupt-parent = <&PIC8259>;
> +			};
> +			PIC8259: interrupt-controller {
> +				device_type = "i8259";
> 
> device_type = "interrupt-controller".
> 
> +				compatible = "prep,iic";
> +				reg = <	00000001 00000020  00000002
> +					00000001 000000a0  00000002
> +					00000001 000004d0  00000002>;
> +				interrupts = <00000000 00000003
> +					      00000002 00000003>;
> +				interrupt-parent = <&MPIC>;
> +			};
> +		};
> +
> +		MPIC: interrupt-controller@d {
> +			device_type = "open-pic";
> 
> device_type = "interrupt-controller".
> 
> +			compatible = "mpic";
> +			reg = <	00006800 00000000 00000000  00000000 00000000
> +				02006810 00000000 00000000  00000000 00040000>;
> +			assigned-addresses = <
> +				82006810 00000000 3afc0000  00000000 00040000>;
> +		};
> +	};
> +
> +	chosen {
> +		linux,stdout-path = "/pci/isa/serial@3f8";
> +	};
> +};
> 
> What is the plan here -- have the bootwrapper build the
> device tree / fill in the details from the residual data?

Not sure at this stage if it will be best for the bootwrapper to build
a complete tree from residual, or to have a dts skeleton with
substantial chunks filled in by bootwrapper from residual.  I was
intending to merge libfdt into the kernel for more flexible device
tree manipulation before investigating that further.


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

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>>> Too big for the list, the patch is at:
>>> 	http://ozlabs.org/~dgibson/home/prep-support
>>
>> Too lazy to split the patch into bite-size chunks, you mean ;-)
>
> Well... much as I like small patches, I don't really like having a big
> string of patches, each of which does basically nothing on its own,
> i.e. split up just for the sake of making smaller, rather than into
> separate logically separate changes.

Yeah I understand.  It's just that I had to dig out the DTS
part :-)

>> +			external-control;
>>
>> Really?
>
> No idea, just copied that from earlier work of Paulus'.  Don't even
> know what the property means.

It seems to me a flat device tree could leave out all those
properties that can be derived from the PVR (except for the
few that are really useful for bootloaders and such -- cache
line size, 64-bit-or-not, what kind of MMU).  Linux doesn't
use it anyway.  Existing DTSs already leave away most.

>> +	pci@80000000 {
>> +		device_type = "pci";
>> +		compatible = "prep";
>>
>> Is that specific enough?
>
> Well, AFAICT, the prep PCI code doesn't need any more info.

If PReP requires a specific programming model for the PCI
host bridge, that would be fine.  But then compatible =
"prep-pci-bridge" or such, not just "prep"; everything on
your board is "prep", it would make matching a bit hard ;-)

>> I can't believe this "ranges" and interrupt mapping will
>> work on all PReP systems...
>
> Probably not, but it should work on a chunk of them.  Like I say,
> there's still a good deal more that needs to be filled in from
> residual data or wherever.

Sure, I'm just pointing out things that seem problematic, I'm
not saying your code can't be merged because of that -- esp.
since it is a new port anyway (for arch/powerpc, that is).

>> What is the plan here -- have the bootwrapper build the
>> device tree / fill in the details from the residual data?
>
> Not sure at this stage if it will be best for the bootwrapper to build
> a complete tree from residual, or to have a dts skeleton with
> substantial chunks filled in by bootwrapper from residual.

Conceptually those two options are pretty much the same thing;
just try them out, see what is nicer for the implementation.

> I was
> intending to merge libfdt into the kernel for more flexible device
> tree manipulation before investigating that further.

Into the kernel wrapper, I think you mean?


Segher

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[David Gibson]

On Wed, Jul 18, 2007 at 05:55:16PM +0200, Segher Boessenkool wrote:
> >>> Too big for the list, the patch is at:
> >>> 	http://ozlabs.org/~dgibson/home/prep-support
> >>
> >> Too lazy to split the patch into bite-size chunks, you mean ;-)
> >
> > Well... much as I like small patches, I don't really like having a big
> > string of patches, each of which does basically nothing on its own,
> > i.e. split up just for the sake of making smaller, rather than into
> > separate logically separate changes.
> 
> Yeah I understand.  It's just that I had to dig out the DTS
> part :-)

Diddums :-p.

> >> +			external-control;
> >>
> >> Really?
> >
> > No idea, just copied that from earlier work of Paulus'.  Don't even
> > know what the property means.
> 
> It seems to me a flat device tree could leave out all those
> properties that can be derived from the PVR (except for the
> few that are really useful for bootloaders and such -- cache
> line size, 64-bit-or-not, what kind of MMU).  Linux doesn't
> use it anyway.  Existing DTSs already leave away most.

I agree, ditched them from my dts.

> >> +	pci@80000000 {
> >> +		device_type = "pci";
> >> +		compatible = "prep";
> >>
> >> Is that specific enough?
> >
> > Well, AFAICT, the prep PCI code doesn't need any more info.
> 
> If PReP requires a specific programming model for the PCI
> host bridge, that would be fine.  But then compatible =
> "prep-pci-bridge" or such, not just "prep"; everything on
> your board is "prep", it would make matching a bit hard ;-)

Well... I'm rather unclear on how much PReP requires of the PCI
bridge.  I thought what I'd coded was based on what appeared to be
hard assumptions in prep_pci.c, but now I'm hearing that this won't
cover all PReP machines, so I'm really not sure.

> >> I can't believe this "ranges" and interrupt mapping will
> >> work on all PReP systems...
> >
> > Probably not, but it should work on a chunk of them.  Like I say,
> > there's still a good deal more that needs to be filled in from
> > residual data or wherever.
> 
> Sure, I'm just pointing out things that seem problematic, I'm
> not saying your code can't be merged because of that -- esp.
> since it is a new port anyway (for arch/powerpc, that is).
> 
> >> What is the plan here -- have the bootwrapper build the
> >> device tree / fill in the details from the residual data?
> >
> > Not sure at this stage if it will be best for the bootwrapper to build
> > a complete tree from residual, or to have a dts skeleton with
> > substantial chunks filled in by bootwrapper from residual.
> 
> Conceptually those two options are pretty much the same thing;
> just try them out, see what is nicer for the implementation.
> 
> > I was
> > intending to merge libfdt into the kernel for more flexible device
> > tree manipulation before investigating that further.
> 
> Into the kernel wrapper, I think you mean?

Yes, of course.

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

Re: [PATCH 3/3] First cut at PReP support for arch/powerpc

[Segher Boessenkool]

>> It seems to me a flat device tree could leave out all those
>> properties that can be derived from the PVR (except for the
>> few that are really useful for bootloaders and such -- cache
>> line size, 64-bit-or-not, what kind of MMU).  Linux doesn't
>> use it anyway.  Existing DTSs already leave away most.
>
> I agree, ditched them from my dts.

It's good to have consensus :-)

Is this documented already?

>> If PReP requires a specific programming model for the PCI
>> host bridge, that would be fine.  But then compatible =
>> "prep-pci-bridge" or such, not just "prep"; everything on
>> your board is "prep", it would make matching a bit hard ;-)
>
> Well... I'm rather unclear on how much PReP requires of the PCI
> bridge.  I thought what I'd coded was based on what appeared to be
> hard assumptions in prep_pci.c, but now I'm hearing that this won't
> cover all PReP machines, so I'm really not sure.

Also, even if the current kernels assume certain hardware, that
doesn't mean you can do the same in the device tree.


Segher