[PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Gerhard Pircher]

This device tree does not provide the correct CPU name, as various CPU
models and revisions are used in AmigaOnes. Also the PCI root node does
not contain a interrupt mapping property, as all boards have different
interrupt routing. However the kernel can do a 1:1 mapping of all PCI
interrupts, as only i8259 legacy interrupts are used.

Signed-off-by: Gerhard Pircher <gerhard_pircher@gmx.net>
---
 arch/powerpc/boot/dts/amigaone.dts |  233 ++++++++++++++++++++++++++++++++++++
 1 files changed, 233 insertions(+), 0 deletions(-)
 create mode 100644 arch/powerpc/boot/dts/amigaone.dts

diff --git a/arch/powerpc/boot/dts/amigaone.dts b/arch/powerpc/boot/dts/amigaone.dts
new file mode 100644
index 0000000..9794bbc
--- /dev/null
+++ b/arch/powerpc/boot/dts/amigaone.dts
@@ -0,0 +1,233 @@
+/*
+ * AmigaOne Device Tree Source
+ *
+ * Copyright 2008 Gerhard Pircher (gerhard_pircher@gmx.net)
+ *
+ * This program is free software; you can redistribute  it and/or modify it
+ * under  the terms of  the GNU General  Public License as published by the
+ * Free Software Foundation;  either version 2 of the  License, or (at your
+ * option) any later version.
+ */
+
+/dts-v1/;
+
+/ {
+	model = "AmigaOne";
+	compatible = "eyetech,amigaone","mai-logic,teron";
+	coherency-off;
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	cpus {
+		#cpus = <1>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+
+		cpu@0 {
+			device_type = "cpu";
+			reg = <0>;
+			d-cache-line-size = <32>;	// 32 bytes
+			i-cache-line-size = <32>;	// 32 bytes
+			d-cache-size = <32768>;		// L1, 32K
+			i-cache-size = <32768>;		// L1, 32K
+			timebase-frequency = <0>;	// 33.3 MHz, from U-boot
+			clock-frequency = <0>;		// From U-boot
+			bus-frequency = <0>;		// From U-boot
+		};
+	};
+
+	memory {
+		device_type = "memory";
+		reg = <0 0>;				// From U-boot
+	};
+
+  	pci@80000000 {
+		device_type = "pci";
+		compatible = "mai-logic,articia-s";
+		bus-frequency = <33333333>;
+		bus-range = <0 0xff>;
+		ranges = <0x01000000 0 0x00000000 0xfe000000 0 0x00c00000	// PCI I/O
+			  0x02000000 0 0x80000000 0x80000000 0 0x7d000000	// PCI memory
+			  0x02000000 0 0x00000000 0xfd000000 0 0x01000000>;	// PCI alias memory (ISA)
+		8259-interrupt-acknowledge = <0xfef00000>;
+		/* Do not define a interrupt-parent here, if there is no interrupt-map property. */
+		#address-cells = <3>;
+		#size-cells = <2>;
+
+		host@0 {
+			compatible = "pciclass,0600";
+			vendor-id = <0x000010cc>;
+			device-id = <0x00000660>;
+			revision-id = <0x00000001>;
+			class-code = <0x00060000>;
+			subsystem-id = <0>;
+			subsystem-vendor-id = <0>;
+			devsel-speed = <0x00000001>;
+			66mhz-capable;
+			min-grant = <0>;
+			max-latency = <0>;
+			// AGP aperture is unset.
+//			reg = <0x42000010 0 0x00000000 0 0x00400000>;
+//			assigned-addresses = <0x42000010 0 0x00000000 0 0x00400000>;
+		};
+
+		isa@7 {
+			device_type = "isa";
+			compatible = "pciclass,0601";
+			vendor-id = <0x00001106>;
+			device-id = <0x00000686>;
+			revision-id = <0x00000010>;
+			class-code = <0x00060100>;
+			subsystem-id = <0>;
+			subsystem-vendor-id = <0>;
+			devsel-speed = <0x00000001>;
+			min-grant = <0>;
+			max-latency = <0>;
+			/* First 64k for I/O at 0x0 on PCI mapped to 0x0 on ISA. */
+			ranges = <0x00000001 0 0x01000000 0 0x00000000 0x00010000>;
+			interrupt-parent = <&i8259>;
+			#interrupt-cells = <2>;
+			#address-cells = <2>;
+			#size-cells = <1>;
+
+			dma-controller@0 {
+				device_type = "dma-controller";
+				compatible = "pnpPNP,200";
+				reg = <1 0x00000000 0x00000020
+				       1 0x00000080 0x00000010
+				       1 0x000000c0 0x00000020>;
+				/* Channel 4 reserverd, cascade mode, 2x32k transfer/counter
+				 * widths and bus master capability.
+				 */
+/*				dma = <0x4 0x4 0x20 0x20 0x1>; */
+			};
+
+		  	i8259: interrupt-controller@20 {
+				device_type = "interrupt-controller";
+				compatible = "pnpPNP,000";
+				interrupt-controller;
+				reg = <1 0x00000020 0x00000002
+				       1 0x000000a0 0x00000002
+				       1 0x000004d0 0x00000002>;
+				reserved-interrupts = <2>;
+				#interrupt-cells = <2>;
+			};
+
+			timer@40 {
+				// Also adds pcspkr to platform devices.
+				compatible = "pnpPNP,100";
+				reg = <1 0x00000040 0x00000020>;
+			};
+
+			8042@60 {
+				device_type = "8042";
+				reg = <1 0x00000060 0x00000001
+				       1 0x00000064 0x00000001>;
+				// IRQ1, IRQ12 (rising edge)
+				interrupts = <1 3 12 3>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				keyboard@0 {
+					device_type = "keyboard";
+					compatible = "pnpPNP,303";
+					reg = <0>;
+				};
+
+				mouse@1 {
+					device_type = "mouse";
+					compatible = "pnpPNP,f03";
+					reg = <1>;
+				};
+			};
+
+			rtc@70 {
+				compatible = "pnpPNP,b00";
+				reg = <1 0x00000070 0x00000002>;
+				interrupts = <8 3>;
+			};
+
+			serial@3f8 {
+				device_type = "serial";
+				compatible = "pnpPNP,501","pnpPNP,500";
+				reg = <1 0x000003f8 0x00000008>;
+				// IRQ4 (rising edge)
+				interrupts = <4 3>;
+				clock-frequency = <1843200>;
+				current-speed = <115200>;
+			};
+
+			serial@2f8 {
+				device_type = "serial";
+				compatible = "pnpPNP,501","pnpPNP,500";
+				reg = <1 0x000002f8 0x00000008>;
+				// IRQ3 (rising edge)
+				interrupts = <3 3>;
+				clock-frequency = <1843200>;
+				current-speed = <115200>;
+			};
+
+			parallel@378 {
+				device_type = "parallel";
+				// No ECP support for now, otherwise add "pnpPNP,401".
+				compatible = "pnpPNP,400";
+				reg = <1 0x00000378 0x00000003
+				       1 0x00000778 0x00000003>;
+/*				interrupts = <7 0>; */
+				// Parallel port DMA mode unknown.
+/*				dma = <0x3 0x0 0x0 0x0>; */
+			};
+
+			fdc@3f0 {
+				device_type = "fdc";
+				compatible = "pnpPNP,700";
+				reg = <1 0x000003f0 0x00000008>;
+				// IRQ6 (rising edge)
+				interrupts = <6 3>;
+				// Floppy DMA mode unknown.
+/*				dma = < >; */
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				disk@0 {
+					device_type = "block";
+					reg = <0>;
+				};
+			};
+		};
+
+		ide@7,1 {
+			compatible = "pciclass,01018a";
+			vendor-id = <0x00001106>;
+			device-id = <0x00000571>;
+			revision-id = <0x00000006>;
+			// Class code with PCI IDE programming interface indicator.
+			class-code = <0x0001018a>;
+			subsystem-id = <0>;
+			subsystem-vendor-id = <0>;
+			devsel-speed = <0x00000001>;
+			min-grant = <0>;
+			max-latency = <0>;
+			fast-back-to-back;
+			// Assume base addresses are relocateable, even if
+			// controller operates in compatibility mode.
+			reg = <0x21003910 0 0x00000000 0 0x00000000
+			       0x21003914 0 0x00000000 0 0x00000000
+			       0x21003918 0 0x00000000 0 0x00000000
+			       0x2100391c 0 0x00000000 0 0x00000000
+			       0x21003920 0 0x00000000 0 0x00000000>;
+			assigned-addresses = <0x01003910 0 0x000001f0 0 0x00000008
+					      0x01003914 0 0x000003f4 0 0x00000004
+					      0x01003918 0 0x00000170 0 0x00000008
+					      0x0100391c 0 0x00000374 0 0x00000004
+					      0x01003920 0 0x0000cc00 0 0x00000010>;
+/*			interrupt-parent = <&i8259>;
+			interrupts = <14 3 15 3>;
+			#interrupt-cells = <2>; */
+		};
+	};
+
+	chosen {
+		linux,stdout-path = "/pci@80000000/isa@7/serial@3f8";
+	};
+};
-- 
1.5.6.5


-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Grant Likely]

On Wed, Jan 7, 2009 at 7:01 AM, Gerhard Pircher <gerhard_pircher@gmx.net> wrote:
> This device tree does not provide the correct CPU name, as various CPU
> models and revisions are used in AmigaOnes.  Also the PCI root node does
> not contain a interrupt mapping property, as all boards have different
> interrupt routing. However the kernel can do a 1:1 mapping of all PCI
> interrupts, as only i8259 legacy interrupts are used.

Sounds to me like you need different device tree variants for each of
the AmigaOne boards.  Do any of the boards have physical PCI slots?
If so, then the lack of an interrupt map will break them.

>
> Signed-off-by: Gerhard Pircher <gerhard_pircher@gmx.net>
> ---
>  arch/powerpc/boot/dts/amigaone.dts |  233 ++++++++++++++++++++++++++++++++++++
>  1 files changed, 233 insertions(+), 0 deletions(-)
>  create mode 100644 arch/powerpc/boot/dts/amigaone.dts
>
> diff --git a/arch/powerpc/boot/dts/amigaone.dts b/arch/powerpc/boot/dts/amigaone.dts
> new file mode 100644
> index 0000000..9794bbc
> --- /dev/null
> +++ b/arch/powerpc/boot/dts/amigaone.dts
> @@ -0,0 +1,233 @@
> +/*
> + * AmigaOne Device Tree Source
> + *
> + * Copyright 2008 Gerhard Pircher (gerhard_pircher@gmx.net)
> + *
> + * This program is free software; you can redistribute  it and/or modify it
> + * under  the terms of  the GNU General  Public License as published by the
> + * Free Software Foundation;  either version 2 of the  License, or (at your
> + * option) any later version.
> + */
> +
> +/dts-v1/;
> +
> +/ {
> +       model = "AmigaOne";
> +       compatible = "eyetech,amigaone","mai-logic,teron";

Experience has shown that trying to claim one board to be compatible
with another is too ambiguous.  It is better to make the board level
compatible property have a single value specifying the exact board
model and have the platform support code include a list of supported
board models.  Otherwise you end up with odd heuristic code to try and
differentiate between the models (for bug fixes and such).

> +       coherency-off;
> +       #address-cells = <1>;
> +       #size-cells = <1>;
> +
> +       cpus {
> +               #cpus = <1>;
> +               #address-cells = <1>;
> +               #size-cells = <0>;
> +
> +               cpu@0 {
> +                       device_type = "cpu";
> +                       reg = <0>;
> +                       d-cache-line-size = <32>;       // 32 bytes
> +                       i-cache-line-size = <32>;       // 32 bytes
> +                       d-cache-size = <32768>;         // L1, 32K
> +                       i-cache-size = <32768>;         // L1, 32K
> +                       timebase-frequency = <0>;       // 33.3 MHz, from U-boot
> +                       clock-frequency = <0>;          // From U-boot
> +                       bus-frequency = <0>;            // From U-boot
> +               };
> +       };
> +
> +       memory {
> +               device_type = "memory";
> +               reg = <0 0>;                            // From U-boot
> +       };
> +
> +       pci@80000000 {
> +               device_type = "pci";
> +               compatible = "mai-logic,articia-s";
> +               bus-frequency = <33333333>;
> +               bus-range = <0 0xff>;
> +               ranges = <0x01000000 0 0x00000000 0xfe000000 0 0x00c00000       // PCI I/O
> +                         0x02000000 0 0x80000000 0x80000000 0 0x7d000000       // PCI memory
> +                         0x02000000 0 0x00000000 0xfd000000 0 0x01000000>;     // PCI alias memory (ISA)
> +               8259-interrupt-acknowledge = <0xfef00000>;
> +               /* Do not define a interrupt-parent here, if there is no interrupt-map property. */
> +               #address-cells = <3>;
> +               #size-cells = <2>;
> +
> +               host@0 {
> +                       compatible = "pciclass,0600";
> +                       vendor-id = <0x000010cc>;
> +                       device-id = <0x00000660>;
> +                       revision-id = <0x00000001>;
> +                       class-code = <0x00060000>;
> +                       subsystem-id = <0>;
> +                       subsystem-vendor-id = <0>;
> +                       devsel-speed = <0x00000001>;
> +                       66mhz-capable;
> +                       min-grant = <0>;
> +                       max-latency = <0>;
> +                       // AGP aperture is unset.
> +//                     reg = <0x42000010 0 0x00000000 0 0x00400000>;
> +//                     assigned-addresses = <0x42000010 0 0x00000000 0 0x00400000>;
> +               };

What does this node describe?  Is it something that isn't probeable?

> +                       8042@60 {
> +                               device_type = "8042";
> +                               reg = <1 0x00000060 0x00000001
> +                                      1 0x00000064 0x00000001>;
> +                               // IRQ1, IRQ12 (rising edge)
> +                               interrupts = <1 3 12 3>;

For the flattened device tree, I think we've settled on the convention
that every node with an IRQ connection should have both the
interrupt-parent and interrupts properties.  (ie. don't rely on the
parent node's interrupt-parent property.)

> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               keyboard@0 {
> +                                       device_type = "keyboard";

Drop device type in the flattened tree.  There are a few places where
you still need to have it for Linux to work; but it Linux doesn't look
for it, then don't add it.

> +                       serial@3f8 {
> +                               device_type = "serial";
> +                               compatible = "pnpPNP,501","pnpPNP,500";
> +                               reg = <1 0x000003f8 0x00000008>;
> +                               // IRQ4 (rising edge)
> +                               interrupts = <4 3>;
> +                               clock-frequency = <1843200>;
> +                               current-speed = <115200>;
> +                       };
> +
> +                       serial@2f8 {
> +                               device_type = "serial";
> +                               compatible = "pnpPNP,501","pnpPNP,500";
> +                               reg = <1 0x000002f8 0x00000008>;
> +                               // IRQ3 (rising edge)
> +                               interrupts = <3 3>;
> +                               clock-frequency = <1843200>;
> +                               current-speed = <115200>;
> +                       };
> +
> +                       parallel@378 {
> +                               device_type = "parallel";
> +                               // No ECP support for now, otherwise add "pnpPNP,401".
> +                               compatible = "pnpPNP,400";
> +                               reg = <1 0x00000378 0x00000003
> +                                      1 0x00000778 0x00000003>;
> +/*                             interrupts = <7 0>; */
> +                               // Parallel port DMA mode unknown.
> +/*                             dma = <0x3 0x0 0x0 0x0>; */
> +                       };
> +
> +                       fdc@3f0 {
> +                               device_type = "fdc";
> +                               compatible = "pnpPNP,700";
> +                               reg = <1 0x000003f0 0x00000008>;
> +                               // IRQ6 (rising edge)
> +                               interrupts = <6 3>;
> +                               // Floppy DMA mode unknown.
> +/*                             dma = < >; */
> +                               #address-cells = <1>;
> +                               #size-cells = <0>;
> +
> +                               disk@0 {
> +                                       device_type = "block";
> +                                       reg = <0>;
> +                               };
> +                       };
> +               };
> +
> +               ide@7,1 {
> +                       compatible = "pciclass,01018a";
> +                       vendor-id = <0x00001106>;
> +                       device-id = <0x00000571>;
> +                       revision-id = <0x00000006>;

Can this PCI device be probed?  Typically PCI devices don't get added
to the flattened device tree because PCI is a probeable bus.

> +       chosen {
> +               linux,stdout-path = "/pci@80000000/isa@7/serial@3f8";

If you put a 'serial-console:' label on the serial port node, then you
can use the simpler form of:
        linux,stdout-path = &serial-console;

Cheers,
g.

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

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Scott Wood]

On Wed, Jan 07, 2009 at 09:41:14AM -0700, Grant Likely wrote:
> > +                       8042@60 {
> > +                               device_type = "8042";
> > +                               reg = <1 0x00000060 0x00000001
> > +                                      1 0x00000064 0x00000001>;
> > +                               // IRQ1, IRQ12 (rising edge)
> > +                               interrupts = <1 3 12 3>;
> 
> For the flattened device tree, I think we've settled on the convention
> that every node with an IRQ connection should have both the
> interrupt-parent and interrupts properties.  (ie. don't rely on the
> parent node's interrupt-parent property.)

Why?

-Scott

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Grant Likely]

On Wed, Jan 7, 2009 at 3:10 PM, Scott Wood <scottwood@freescale.com> wrote:
> On Wed, Jan 07, 2009 at 09:41:14AM -0700, Grant Likely wrote:
>> > +                       8042@60 {
>> > +                               device_type = "8042";
>> > +                               reg = <1 0x00000060 0x00000001
>> > +                                      1 0x00000064 0x00000001>;
>> > +                               // IRQ1, IRQ12 (rising edge)
>> > +                               interrupts = <1 3 12 3>;
>>
>> For the flattened device tree, I think we've settled on the convention
>> that every node with an IRQ connection should have both the
>> interrupt-parent and interrupts properties.  (ie. don't rely on the
>> parent node's interrupt-parent property.)
>
> Why?

Defensive programming.  To not rely on implicit relationships

g.

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

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Scott Wood]

Grant Likely wrote:
> On Wed, Jan 7, 2009 at 3:10 PM, Scott Wood <scottwood@freescale.com> wrote:
>> On Wed, Jan 07, 2009 at 09:41:14AM -0700, Grant Likely wrote:
>>>> +                       8042@60 {
>>>> +                               device_type = "8042";
>>>> +                               reg = <1 0x00000060 0x00000001
>>>> +                                      1 0x00000064 0x00000001>;
>>>> +                               // IRQ1, IRQ12 (rising edge)
>>>> +                               interrupts = <1 3 12 3>;
>>> For the flattened device tree, I think we've settled on the convention
>>> that every node with an IRQ connection should have both the
>>> interrupt-parent and interrupts properties.  (ie. don't rely on the
>>> parent node's interrupt-parent property.)
>> Why?
> 
> Defensive programming.  To not rely on implicit relationships

It doesn't seem any more likely to introduce a fault by specifying the 
interrupt controller in one place than in many places.

-Scott

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Grant Likely]

On Wed, Jan 7, 2009 at 3:23 PM, Scott Wood <scottwood@freescale.com> wrote:
> Grant Likely wrote:
>>
>> On Wed, Jan 7, 2009 at 3:10 PM, Scott Wood <scottwood@freescale.com>
>> wrote:
>>>
>>> Why?
>>
>> Defensive programming.  To not rely on implicit relationships
>
> It doesn't seem any more likely to introduce a fault by specifying the
> interrupt controller in one place than in many places.

We do the same thing with #address-cells and #size-cells for the same
reason.  You can get away with leaving them out; but being explicit is
more clear.

g.

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

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Scott Wood]

Grant Likely wrote:
> On Wed, Jan 7, 2009 at 3:23 PM, Scott Wood <scottwood@freescale.com> wrote:
>> It doesn't seem any more likely to introduce a fault by specifying the
>> interrupt controller in one place than in many places.
> 
> We do the same thing with #address-cells and #size-cells for the same
> reason.  You can get away with leaving them out; but being explicit is
> more clear.

That's different -- the defaults for #address-cells and #size-cells are 
abitrary constants (2 and 1, respectively), not merely something that 
was defined higher in the hierarchy.

-Scott

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Wed, 7 Jan 2009 09:41:14 -0700
> Von: "Grant Likely" <grant.likely@secretlab.ca>
> An: "Gerhard Pircher" <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@ozlabs.org
> Betreff: Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

> Sounds to me like you need different device tree variants for each of
> the AmigaOne boards.  Do any of the boards have physical PCI slots?
> If so, then the lack of an interrupt map will break them.
Yes, all AmigaOne boards have physical PCI slots (at least 1). The different
interrupt routing wasn't a problem so far, as the firmware writes the IRQ number
to the interrupt line register of every PCI device.
Currently the kernel reads the IRQ number from this register, if there is no
interrupt mapping property. I know that it's not a good idea to rely on kernel
fallback behavior, but it makes a lot of things easier in this case.

> > +/ {
> > +       model = "AmigaOne";
> > +       compatible = "eyetech,amigaone","mai-logic,teron";
> 
> Experience has shown that trying to claim one board to be compatible
> with another is too ambiguous.  It is better to make the board level
> compatible property have a single value specifying the exact board
> model and have the platform support code include a list of supported
> board models.  Otherwise you end up with odd heuristic code to try and
> differentiate between the models (for bug fixes and such).
Okay, I'll remove the "mai-logic,teron" entry. I have never seen a MAI Teron
in the wild, so I can't even tell if it uses a U-boot firmware.

> > +               host@0 {
> > +                       compatible = "pciclass,0600";
> > +                       vendor-id = <0x000010cc>;
> > +                       device-id = <0x00000660>;
> > +                       revision-id = <0x00000001>;
> > +                       class-code = <0x00060000>;
> > +                       subsystem-id = <0>;
> > +                       subsystem-vendor-id = <0>;
> > +                       devsel-speed = <0x00000001>;
> > +                       66mhz-capable;
> > +                       min-grant = <0>;
> > +                       max-latency = <0>;
> > +                       // AGP aperture is unset.
> > +//                     reg = <0x42000010 0 0x00000000 0 0x00400000>;
> > +//                     assigned-addresses = <0x42000010 0 0x00000000 0
> 0x00400000>;
> > +               };
> 
> What does this node describe?  Is it something that isn't probeable?
Yes, the information in this node is probeable. Originally I planned to add
some child nodes to this node, as the PCI host bridge contains other PCI
functions (e.g. power management) which are not probeable. The host bridge
has an index register, which switches between the host bridge config space
and the config space of the other PCI functions. I don't know yet how this is
described correctly in a device tree, so I'll probably remove this node for now.

> > +                       8042@60 {
> > +                               device_type = "8042";
> > +                               reg = <1 0x00000060 0x00000001
> > +                                      1 0x00000064 0x00000001>;
> > +                               // IRQ1, IRQ12 (rising edge)
> > +                               interrupts = <1 3 12 3>;
> 
> For the flattened device tree, I think we've settled on the convention
> that every node with an IRQ connection should have both the
> interrupt-parent and interrupts properties.  (ie. don't rely on the
> parent node's interrupt-parent property.)
Even for ISA devices?

> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               keyboard@0 {
> > +                                       device_type = "keyboard";
> 
> Drop device type in the flattened tree.  There are a few places where
> you still need to have it for Linux to work; but it Linux doesn't look
> for it, then don't add it.
Okay.

> > +                       serial@3f8 {
> > +                               device_type = "serial";
> > +                               compatible = "pnpPNP,501","pnpPNP,500";
> > +                               reg = <1 0x000003f8 0x00000008>;
> > +                               // IRQ4 (rising edge)
> > +                               interrupts = <4 3>;
> > +                               clock-frequency = <1843200>;
> > +                               current-speed = <115200>;
> > +                       };
> > +
> > +                       serial@2f8 {
> > +                               device_type = "serial";
> > +                               compatible = "pnpPNP,501","pnpPNP,500";
> > +                               reg = <1 0x000002f8 0x00000008>;
> > +                               // IRQ3 (rising edge)
> > +                               interrupts = <3 3>;
> > +                               clock-frequency = <1843200>;
> > +                               current-speed = <115200>;
> > +                       };
> > +
> > +                       parallel@378 {
> > +                               device_type = "parallel";
> > +                               // No ECP support for now, otherwise add
> "pnpPNP,401".
> > +                               compatible = "pnpPNP,400";
> > +                               reg = <1 0x00000378 0x00000003
> > +                                      1 0x00000778 0x00000003>;
> > +/*                             interrupts = <7 0>; */
> > +                               // Parallel port DMA mode unknown.
> > +/*                             dma = <0x3 0x0 0x0 0x0>; */
> > +                       };
> > +
> > +                       fdc@3f0 {
> > +                               device_type = "fdc";
> > +                               compatible = "pnpPNP,700";
> > +                               reg = <1 0x000003f0 0x00000008>;
> > +                               // IRQ6 (rising edge)
> > +                               interrupts = <6 3>;
> > +                               // Floppy DMA mode unknown.
> > +/*                             dma = < >; */
> > +                               #address-cells = <1>;
> > +                               #size-cells = <0>;
> > +
> > +                               disk@0 {
> > +                                       device_type = "block";
> > +                                       reg = <0>;
> > +                               };
> > +                       };
> > +               };
> > +
> > +               ide@7,1 {
> > +                       compatible = "pciclass,01018a";
> > +                       vendor-id = <0x00001106>;
> > +                       device-id = <0x00000571>;
> > +                       revision-id = <0x00000006>;
> 
> Can this PCI device be probed?  Typically PCI devices don't get added
> to the flattened device tree because PCI is a probeable bus.
Yes, it can be probed. I thought it would be a good idea to include it,
because the IDE controller operates in legacy mode. I planned to specify the
two legacy interrupts in this node (as you can see), but the kernel didn't like
them.

Thanks!

Gerhard

-- 
Sensationsangebot verlängert: GMX FreeDSL - Telefonanschluss + DSL 
für nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K1308T4569a

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Benjamin Herrenschmidt]

On Wed, 2009-01-07 at 09:41 -0700, Grant Likely wrote:
> For the flattened device tree, I think we've settled on the convention
> that every node with an IRQ connection should have both the
> interrupt-parent and interrupts properties.  (ie. don't rely on the
> parent node's interrupt-parent property.)

Ah ? I use the trick of relying on the parent node regulary :-) Where
did we discuss that ?

Ben.

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Benjamin Herrenschmidt]

> Yes, all AmigaOne boards have physical PCI slots (at least 1). The different
> interrupt routing wasn't a problem so far, as the firmware writes the IRQ number
> to the interrupt line register of every PCI device.

The code in the kernel that retreives the interrupt that way is clearly
marked as a fishy workaround for bogus firmwares :-)

But I'm not going to reject things based on that, it will work for
simple board using really only legacy interrupts like yours...

> Currently the kernel reads the IRQ number from this register, if there is no
> interrupt mapping property. I know that it's not a good idea to rely on kernel
> fallback behavior, but it makes a lot of things easier in this case.

Sort-of. As long as it's really 8259 interrupts, I suppose it's
acceptable.

> > For the flattened device tree, I think we've settled on the convention
> > that every node with an IRQ connection should have both the
> > interrupt-parent and interrupts properties.  (ie. don't rely on the
> > parent node's interrupt-parent property.)
> Even for ISA devices?

I disagree with Grant here. Especially in simple ISA cases like that,
there's really no point in bloating the device-tree.

> > Can this PCI device be probed?  Typically PCI devices don't get added
> > to the flattened device tree because PCI is a probeable bus.
> Yes, it can be probed. I thought it would be a good idea to include it,
> because the IDE controller operates in legacy mode. I planned to specify the
> two legacy interrupts in this node (as you can see), but the kernel didn't like
> them.

Well, the kernel just didn't make use of them I'd say :-) But that can
probably be fixed with the appropriate hacks. 

Cheers,
Ben.

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Grant Likely]

On Sun, Jan 11, 2009 at 10:07 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Wed, 2009-01-07 at 09:41 -0700, Grant Likely wrote:
>> For the flattened device tree, I think we've settled on the convention
>> that every node with an IRQ connection should have both the
>> interrupt-parent and interrupts properties.  (ie. don't rely on the
>> parent node's interrupt-parent property.)
>
> Ah ? I use the trick of relying on the parent node regulary :-) Where
> did we discuss that ?

It is also entirely possible that I'm smoking something exotic.

g.

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

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Mon, 12 Jan 2009 16:12:18 +1100
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: Grant Likely <grant.likely@secretlab.ca>, linuxppc-dev@ozlabs.org
> Betreff: Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

> The code in the kernel that retreives the interrupt that way is clearly
> marked as a fishy workaround for bogus firmwares :-)
"bogus" applies a little bit to the PCI init code of the A1 firmware. ;-)

> But I'm not going to reject things based on that, it will work for
> simple board using really only legacy interrupts like yours...
Thanks!

> > > For the flattened device tree, I think we've settled on the convention
> > > that every node with an IRQ connection should have both the
> > > interrupt-parent and interrupts properties.  (ie. don't rely on the
> > > parent node's interrupt-parent property.)
> > Even for ISA devices?
> 
> I disagree with Grant here. Especially in simple ISA cases like that,
> there's really no point in bloating the device-tree.
Okay, so I leave it as it is.

> > > Can this PCI device be probed?  Typically PCI devices don't get added
> > > to the flattened device tree because PCI is a probeable bus.
> > Yes, it can be probed. I thought it would be a good idea to include it,
> > because the IDE controller operates in legacy mode. I planned to specify
> > the two legacy interrupts in this node (as you can see), but the kernel
> > didn't like them.
> 
> Well, the kernel just didn't make use of them I'd say :-) But that can
> probably be fixed with the appropriate hacks. 
I think I throw away the IDE controller node for now, as libata just reads
the PCI register settings (progif) and I guess the IDE subsystem will do
the same in the future.

Gerhard

-- 
Sensationsangebot verlängert: GMX FreeDSL - Telefonanschluss + DSL 
für nur 16,37 Euro/mtl.!* http://dsl.gmx.de/?ac=OM.AD.PD003K1308T4569a

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Benjamin Herrenschmidt]

> I think I throw away the IDE controller node for now, as libata just reads
> the PCI register settings (progif) and I guess the IDE subsystem will do
> the same in the future.

Well... if all AmigaOne use a 8259, they probably use the same interrupt
routing except for PCI slots. In which case, I would -still- prefer if
you had a proper interrupt tree, and at runtime or boot-wrapper time,
fixed up the PCI host "interrupt-map" property to contain the right
values for a given board.

I'm not going to include the new platform in .29, it's way too late
anyway (it should have been published a couple of weeks before the merge
window at least I'd say) so we have some time til .30 to polish things a
bit.

Another area to look at is to cleanup the non-coherent DMA thingy. The
config option should just enable a set of non-coherent backend ops, but
we should still be able to decide which ones to use (coherent vs.
non-coherent) at runtime.

Cheers,
Ben.

Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

[Gerhard Pircher]

-------- Original-Nachricht --------
> Datum: Tue, 13 Jan 2009 16:01:42 +1100
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@ozlabs.org, grant.likely@secretlab.ca
> Betreff: Re: [PATCH 2/5] powerpc: Generic device tree for all AmigaOne boards

> > I think I throw away the IDE controller node for now, as libata just
> > reads the PCI register settings (progif) and I guess the IDE subsystem
> > will do the same in the future.
> 
> Well... if all AmigaOne use a 8259, they probably use the same interrupt
> routing except for PCI slots. In which case, I would -still- prefer if
> you had a proper interrupt tree, and at runtime or boot-wrapper time,
> fixed up the PCI host "interrupt-map" property to contain the right
> values for a given board.
That would be the ideal solution. But there is no reliable way to identify,
if the board is an AmigaOneSE, AmigaOneXE or an uA1. AFAIK the firmware
checks for a specifc onboard PCI sound chip to differentiate between an A1XE
and the uA1, but this sound chip is also used on various PCI sound cards.
I don't really like that approach.

> I'm not going to include the new platform in .29, it's way too late
> anyway (it should have been published a couple of weeks before the merge
> window at least I'd say) so we have some time til .30 to polish things a
> bit.
Okay. Some months more don't matter after all these years. :-)

> Another area to look at is to cleanup the non-coherent DMA thingy. The
> config option should just enable a set of non-coherent backend ops, but
> we should still be able to decide which ones to use (coherent vs.
> non-coherent) at runtime.
What about drivers that need to handle non coherent DMA differently
(e.g. because they map DMA memory to userspace)? Will the
"#ifdef CONFIG_NOT_COHERENT_CACHE" be replaced with calls to
dma_is_consistent()? (for example in the radeon driver, that you fixed for
non coherent DMA platforms in 2.6.25).
If I may express a wish: a reworked CPU feature fixup code for PPC32 will
avoid bigger problems with the AmigaOne platform in the future.

Thanks a lot!

Gerhard

-- 
Psssst! Schon vom neuen GMX MultiMessenger gehört? Der kann`s mit allen: http://www.gmx.net/de/go/multimessenger