linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* Problem with OF interrupt parsing code
@ 2007-10-01 21:00 Gerhard Pircher
  2007-10-01 21:11 ` Gerhard Pircher
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-01 21:00 UTC (permalink / raw)
  To: linuxppc-dev

Hi,

I think I found an issue in the OF interrupt parsing code, although I
have to admit that my device tree source doesn't really follow the
specification.

First some information about my target setup. I didn't specify an
interrupt-map and interrupt-map-mask property in the pci node, because
AFAIK there are three different AmigaOne models with different IRQ
routing. Secondly the AmigaOne is a desktop system with 4 PCI/AGP slots,
thus I can't specify device nodes for all possible devices. By looking at
pci_read_irq_line() in pci_common.c it should be possible for the kernel
to fall back to the interrupt settings in the PCI config space of every
device.

Unfortunately I couldn't capture the kernel log, because the serial
console is not yet working, so here comes a description of what I think
is going on.
Well, pci_read_irq_line() starts with PCI device 00:00.0 (PCI host bridge)
and invokes of_irq_map_pci() in prom_parse.c. It find the predefined
device node in the device tree. Since of_irq_map_one() can't find an
interrupt property in the device node, it returns to pci_read_irq_line()
and setups the interrupt mapping based on the settings in the PCI config
space. So far so good.
The problem occurs now, if there is no device node defined for another
PCI device. In this case, of_irq_map_pci() checks for an interrupt pin,
searches again for the host bridge node and calls of_irq_map_raw() with
the device node of the host bridge. The function finds the
#interrupt-cells, #address-cells, interrupt-controller properties, but
fails to find the interrupt-map property (because it's missing in the
device tree). Therefore the function then tries to find a new parent,
which leads to an endless loop (it always selects the PCI2ISA southbridge
in the device tree).

Can somebody confirm my explanation? It would help, if the kernel could
fall back to the PCI settings in this case, too.

Thanks!

regards,

Gerhard
-- 
GMX FreeMail: 1 GB Postfach, 5 E-Mail-Adressen, 10 Free SMS.
Alle Infos und kostenlose Anmeldung: http://www.gmx.net/de/go/freemail

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:00 Problem with OF interrupt parsing code Gerhard Pircher
@ 2007-10-01 21:11 ` Gerhard Pircher
  2007-10-01 21:39   ` Benjamin Herrenschmidt
  2007-10-01 22:33   ` Segher Boessenkool
  2007-10-01 21:26 ` Scott Wood
  2007-10-01 21:36 ` Benjamin Herrenschmidt
  2 siblings, 2 replies; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-01 21:11 UTC (permalink / raw)
  To: linuxppc-dev

And here comes the device tree source:

/*
 * AmigaOne Device Tree Source
 *
 * Copyright 2007 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.
 */

/ {
	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 = <20>;	// 32 bytes
			i-cache-line-size = <20>;	// 32 bytes
			d-cache-size = <8000>;		// L1, 32K
			i-cache-size = <8000>;		// L1, 32K
			timebase-frequency = <0>;	// 33.3 MHz, from U-boot
			clock-frequency = <0>;		// From U-boot
			bus-frequency = <0>;		// From U-boot
			32-bit;
		};
	};

	memory {
		device_type = "memory";
		reg = <0 0>;				// From U-boot
	};

  	pci@80000000 {
		device_type = "pci";
		compatible = "mai-logic,articia-s";
		bus-frequency = <01fca055>;		// 33.3MHz
		bus-range = <0 ff>;
		ranges = <01000000 0 00000000 fe000000 0 00c00000	// PCI I/O
			  02000000 0 80000000 80000000 0 7d000000	// PCI memory
			  02000000 0 fd000000 fd000000 0 01000000>;	// PCI alias memory
		8259-interrupt-acknowledge = <fef00000>;
		interrupt-parent = <&i8259>;
		#interrupt-cells = <1>;
		#address-cells = <3>;
		#size-cells = <2>;

		host@0 {
			compatible = "pciclass,0600";
			vendor-id = <000010cc>;
			device-id = <00000660>;
			revision-id = <00000001>;
			class-code = <00060000>;
			subsystem-id = <0>;
			subsystem-vendor-id = <0>;
			devsel-speed = <00000001>;
			66mhz-capable;
			min-grant = <0>;
			max-latency = <0>;
			// AGP aperture is unset.
			reg = <42000010 0 00000000 0 00400000>;
			assigned-addresses = <42000010 0 00000000 0 00400000>;
		};

		isa@7 {
			device_type = "isa";
			compatible = "pciclass,0601";
			vendor-id = <00001106>;
			device-id = <00000686>;
			revision-id = <00000010>;
			class-code = <00060100>;
			subsystem-id = <0>;
			subsystem-vendor-id = <0>;
			devsel-speed = <00000001>;
			min-grant = <0>;
			max-latency = <0>;
			/* First 64k for I/O at 0x0 on PCI mapped to 0x0 on ISA. */
			ranges = <00000001 0 01000000 0 00000000 00010000>;
			interrupt-parent = <&i8259>;
			#interrupt-cells = <2>;
			#address-cells = <2>;
			#size-cells = <1>;

			dma-controller@0 {
				device_type = "dma-controller";
				compatible = "pnpPNP,200";
				reg = <00000001 00000000 00000020
				       00000001 00000080 00000010
				       00000001 000000c0 00000020>;
				/* Channel 4 reserverd, cascade mode, 2x32k transfer/counter
				 * widths and bus master capability. Is this really necessary?
				 */
/*				dma = <4 4 20 20 1>; */
			};

		  	i8259: interrupt-controller@20 {
				device_type = "interrupt-controller";
				compatible = "pnpPNP,000";
				interrupt-controller;
				reg = <00000001 00000020 00000002
				       00000001 000000a0 00000002
				       00000001 000004d0 00000002>;
				reserved-interrupts = <2>;
			};

			timer@40 {
/*				device_type = "timer"; */		// No device type binding for now.
				compatibe = "pnpPNP,100";		// Also add pcspkr to platform devices.
				reg = <00000001 00000040 00000020>;
			};

			8042@60 {
				device_type = "8042";
				reg = <00000001 00000060 00000001
				       00000001 00000064 00000001>;
				interrupts = <1 3 c 3>;			// IRQ1, IRQ12 (rising edge)
				#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 {
				device_type = "rtc";
				compatible = "pnpPNP,b00";
				reg = <00000001 00000070 00000002>;
				interrupts = <8 3>;
			};

			serial@2f8 {
				device_type = "serial";
				compatible = "pnpPNP,501","pnpPNP,500";	// "ns16550"; add property check to OF serial code.
				reg = <00000001 000002f8 00000008>;
				interrupts = <3 3>;			// IRQ3 (rising edge)
				clock-frequency = <001C2000>;		// Not necessary?
			};

			serial@3f8 {
				device_type = "serial";
				compatible = "pnpPNP,501","pnpPNP,500";	// "ns16550"; add property check to OF serial code.
				reg = <00000001 000003f8 00000008>;
				interrupts = <4 3>;			// IRQ4 (rising edge)
				clock-frequency = <001C2000>;		// Not necessary?
			};

			parallel@378 {
				device_type = "parallel";
				compatible = "pnpPNP,400"; 		// "pnpPNP,401"	// No ECP support for now.
				reg = <00000001 00000378 00000003
				       00000001 00000778 00000003>;
/*				interrupts = <7>; */
/*				dma = <3 0 0 0>; */			// Parallel port DMA mode?
			};

			fdc@3f0 {
				device_type = "fdc";
				compatible = "pnpPNP,700";
				reg = <00000001 000003f0 00000008>;
				interrupts = <6 3>;			// IRQ6 (rising edge)
/*				dma = < >; */				// Floppy DMA mode?
				#address-cells = <1>;
				#size-cells = <0>;

				disk@0 {
					device_type = "block";
					reg = <0>;
				};
			};
		};

		ide@7,1 {
			compatible = "pciclass,01018f";
			vendor-id = <00001106>;
			device-id = <00000571>;
			revision-id = <00000006>;
			// Class code with PCI IDE programming interface indicator.
			class-code = <0001018f>;
			subsystem-id = <0>;
			subsystem-vendor-id = <0>;
			devsel-speed = <00000001>;
			min-grant = <0>;
			max-latency = <0>;
			fast-back-to-back;
			// Assume base addresses are relocateable, even if
			// controller operates in compatibility mode.
			reg = <21003910 0 00000000 0 00000000
			       21003914 0 00000000 0 00000000
			       21003918 0 00000000 0 00000000
			       2100391c 0 00000000 0 00000000
			       21003920 0 00000000 0 00000000>;
			assigned-addresses = <01003910 0 000001f0 0 00000008
					      01003914 0 000003f4 0 00000004
					      01003918 0 00000170 0 00000008
					      0100391c 0 00000374 0 00000004
					      01003920 0 0000cc00 0 00000010>;
			interrupt-parent = <&i8259>;
			interrupts = <e 3 f 3>;
			#interrupt-cells = <2>;
		};
	};

	chosen {
		linux,stdout-path = "/pci@80000000/isa@7/serial@2f8";
	};
};

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:00 Problem with OF interrupt parsing code Gerhard Pircher
  2007-10-01 21:11 ` Gerhard Pircher
@ 2007-10-01 21:26 ` Scott Wood
  2007-10-01 21:37   ` Scott Wood
  2007-10-02 12:46   ` Gerhard Pircher
  2007-10-01 21:36 ` Benjamin Herrenschmidt
  2 siblings, 2 replies; 18+ messages in thread
From: Scott Wood @ 2007-10-01 21:26 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev

Gerhard Pircher wrote:
> First some information about my target setup. I didn't specify an
> interrupt-map and interrupt-map-mask property in the pci node, because
> AFAIK there are three different AmigaOne models with different IRQ
> routing.

So detect which one you're running on in the bootwrapper, and fix up (or 
select) the device tree appropriately.

> Secondly the AmigaOne is a desktop system with 4 PCI/AGP slots,
> thus I can't specify device nodes for all possible devices. By looking at
> pci_read_irq_line() in pci_common.c it should be possible for the kernel
> to fall back to the interrupt settings in the PCI config space of every
> device.

Those interrupt settings are purely a communication vector from firmware 
to OS.  Is your firmware putting i8259 interrupt numbers in there, and 
did you set the default interrupt controller?

> The problem occurs now, if there is no device node defined for another
> PCI device. In this case, of_irq_map_pci() checks for an interrupt pin,
> searches again for the host bridge node and calls of_irq_map_raw() with
> the device node of the host bridge. The function finds the
> #interrupt-cells, #address-cells, interrupt-controller properties, but
> fails to find the interrupt-map property (because it's missing in the
> device tree). Therefore the function then tries to find a new parent,
> which leads to an endless loop (it always selects the PCI2ISA southbridge
> in the device tree).

That seems likely... there should probably be some loop detection.

-Scott

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:00 Problem with OF interrupt parsing code Gerhard Pircher
  2007-10-01 21:11 ` Gerhard Pircher
  2007-10-01 21:26 ` Scott Wood
@ 2007-10-01 21:36 ` Benjamin Herrenschmidt
  2 siblings, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 21:36 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev


On Mon, 2007-10-01 at 23:00 +0200, Gerhard Pircher wrote:
> Hi,
> 
> I think I found an issue in the OF interrupt parsing code, although I
> have to admit that my device tree source doesn't really follow the
> specification.
> 
> First some information about my target setup. I didn't specify an
> interrupt-map and interrupt-map-mask property in the pci node, because
> AFAIK there are three different AmigaOne models with different IRQ
> routing. Secondly the AmigaOne is a desktop system with 4 PCI/AGP slots,
> thus I can't specify device nodes for all possible devices. By looking at
> pci_read_irq_line() in pci_common.c it should be possible for the kernel
> to fall back to the interrupt settings in the PCI config space of every
> device.

That's deprecated and only support for legacy stuff that predates the
current device-tree support & old/crappy firmware.

You should be able to generate the appropriate interrupt-map from your
loader or wrapper to provide the routing for your slots. It should be
easy enough to have 3 static arrays in your wrapper and use the right
one to "set" the property value.

> Unfortunately I couldn't capture the kernel log, because the serial
> console is not yet working, so here comes a description of what I think
> is going on.
> Well, pci_read_irq_line() starts with PCI device 00:00.0 (PCI host bridge)
> and invokes of_irq_map_pci() in prom_parse.c. It find the predefined
> device node in the device tree. Since of_irq_map_one() can't find an
> interrupt property in the device node, it returns to pci_read_irq_line()
> and setups the interrupt mapping based on the settings in the PCI config
> space. So far so good.
> The problem occurs now, if there is no device node defined for another
> PCI device. In this case, of_irq_map_pci() checks for an interrupt pin,
> searches again for the host bridge node and calls of_irq_map_raw() with
> the device node of the host bridge. The function finds the
> #interrupt-cells, #address-cells, interrupt-controller properties, but
> fails to find the interrupt-map property (because it's missing in the
> device tree). Therefore the function then tries to find a new parent,
> which leads to an endless loop (it always selects the PCI2ISA southbridge
> in the device tree).
> 
> Can somebody confirm my explanation? It would help, if the kernel could
> fall back to the PCI settings in this case, too.

I'm not sure what you are trying to explain above... It looks like your
tree layout is somewhat broken though I think I see how... 

One option would be to make the PCI specific IRQ parsing stop & fail if
the host bridge doesn't provide a map instead of following up an
interrupt-parent link (which is I think what you have in there).

But I don't like that. Just fixup your maps properly and everything
should work just fine.

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:26 ` Scott Wood
@ 2007-10-01 21:37   ` Scott Wood
  2007-10-01 21:43     ` Benjamin Herrenschmidt
  2007-10-02 12:46   ` Gerhard Pircher
  1 sibling, 1 reply; 18+ messages in thread
From: Scott Wood @ 2007-10-01 21:37 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev

Scott Wood wrote:
>> The problem occurs now, if there is no device node defined for another
>> PCI device. In this case, of_irq_map_pci() checks for an interrupt pin,
>> searches again for the host bridge node and calls of_irq_map_raw() with
>> the device node of the host bridge. The function finds the
>> #interrupt-cells, #address-cells, interrupt-controller properties, but
>> fails to find the interrupt-map property (because it's missing in the
>> device tree). Therefore the function then tries to find a new parent,
>> which leads to an endless loop (it always selects the PCI2ISA southbridge
>> in the device tree).
> 
> That seems likely... there should probably be some loop detection.

Actually, it doesn't -- it should stop when it sees the 
interrupt-controller property in the i8259 node, at which point it'll be 
trying to use the raw PCI IRQ pin number as an i8259 IRQ.  This is 
Unlikely To Work(tm).

-Scott

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:11 ` Gerhard Pircher
@ 2007-10-01 21:39   ` Benjamin Herrenschmidt
  2007-10-01 22:33   ` Segher Boessenkool
  1 sibling, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 21:39 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev


On Mon, 2007-10-01 at 23:11 +0200, Gerhard Pircher wrote:
>         pci@80000000 {
>                 device_type = "pci";
>                 compatible = "mai-logic,articia-s";
>                 bus-frequency = <01fca055>;             // 33.3MHz
>                 bus-range = <0 ff>;
>                 ranges = <01000000 0 00000000 fe000000 0
> 00c00000       // PCI I/O
>                           02000000 0 80000000 80000000 0
> 7d000000       // PCI memory
>                           02000000 0 fd000000 fd000000 0
> 01000000>;     // PCI alias memory
>                 8259-interrupt-acknowledge = <fef00000>;
>                 interrupt-parent = <&i8259>;
>                 #interrupt-cells = <1>;
>                 #address-cells = <3>;
>                 #size-cells = <2>;
> 

Part of your problem is that interrupt-parent property. You shouldn't
have such a property in a PCI host bridge. It's not technically illegal,
but it's triggering the "loop" you've been experiencing.

If you want the parsing to fail for PCI devices (to get the fallback to
config space values), you need to make sure it does fail. 

Another option is to put an empty interrupt-map in there. That will
guarantee failure.

But that's all very ugly. I don't understand why you don't setup a
proper map either from your bootloader, zImage wrapper or even prom_init
or platform code.

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:37   ` Scott Wood
@ 2007-10-01 21:43     ` Benjamin Herrenschmidt
  2007-10-01 21:48       ` Scott Wood
  0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 21:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev


On Mon, 2007-10-01 at 16:37 -0500, Scott Wood wrote:
> Scott Wood wrote:
> >> The problem occurs now, if there is no device node defined for another
> >> PCI device. In this case, of_irq_map_pci() checks for an interrupt pin,
> >> searches again for the host bridge node and calls of_irq_map_raw() with
> >> the device node of the host bridge. The function finds the
> >> #interrupt-cells, #address-cells, interrupt-controller properties, but
> >> fails to find the interrupt-map property (because it's missing in the
> >> device tree). Therefore the function then tries to find a new parent,
> >> which leads to an endless loop (it always selects the PCI2ISA southbridge
> >> in the device tree).
> > 
> > That seems likely... there should probably be some loop detection.
> 
> Actually, it doesn't -- it should stop when it sees the 
> interrupt-controller property in the i8259 node, at which point it'll be 
> trying to use the raw PCI IRQ pin number as an i8259 IRQ.  This is 
> Unlikely To Work(tm).

It will work in the specific 8259 case I suppose since it gets the
legacy 1:1 mapping... but it sucks :-)

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:43     ` Benjamin Herrenschmidt
@ 2007-10-01 21:48       ` Scott Wood
  2007-10-01 22:07         ` Benjamin Herrenschmidt
  0 siblings, 1 reply; 18+ messages in thread
From: Scott Wood @ 2007-10-01 21:48 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

Benjamin Herrenschmidt wrote:
> On Mon, 2007-10-01 at 16:37 -0500, Scott Wood wrote:
>> Scott Wood wrote:
>> Actually, it doesn't -- it should stop when it sees the 
>> interrupt-controller property in the i8259 node, at which point it'll be 
>> trying to use the raw PCI IRQ pin number as an i8259 IRQ.  This is 
>> Unlikely To Work(tm).
> 
> It will work in the specific 8259 case I suppose since it gets the
> legacy 1:1 mapping... but it sucks :-)

The mapping between INTA-D and i8259 numbers isn't generally 1:1, and it 
looked as if it'd try using the former... though the code is 
sufficiently complicated that I could easily be missing something.

-Scott

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:48       ` Scott Wood
@ 2007-10-01 22:07         ` Benjamin Herrenschmidt
  0 siblings, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 22:07 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev


On Mon, 2007-10-01 at 16:48 -0500, Scott Wood wrote:
> Benjamin Herrenschmidt wrote:
> > On Mon, 2007-10-01 at 16:37 -0500, Scott Wood wrote:
> >> Scott Wood wrote:
> >> Actually, it doesn't -- it should stop when it sees the 
> >> interrupt-controller property in the i8259 node, at which point it'll be 
> >> trying to use the raw PCI IRQ pin number as an i8259 IRQ.  This is 
> >> Unlikely To Work(tm).
> > 
> > It will work in the specific 8259 case I suppose since it gets the
> > legacy 1:1 mapping... but it sucks :-)
> 
> The mapping between INTA-D and i8259 numbers isn't generally 1:1, and it 
> looked as if it'd try using the former... though the code is 
> sufficiently complicated that I could easily be missing something.

If the whole of_* thing totally fails, pci_read_irq_line should pickup
the value in the config space PCI_INTERRUPT_LINE. That will work on
things like Pegasos and possibly on this AmigaOne but it sucks.

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:11 ` Gerhard Pircher
  2007-10-01 21:39   ` Benjamin Herrenschmidt
@ 2007-10-01 22:33   ` Segher Boessenkool
  2007-10-01 22:54     ` Benjamin Herrenschmidt
  1 sibling, 1 reply; 18+ messages in thread
From: Segher Boessenkool @ 2007-10-01 22:33 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev

> 		  	i8259: interrupt-controller@20 {
> 				device_type = "interrupt-controller";
> 				compatible = "pnpPNP,000";
> 				interrupt-controller;
> 				reg = <00000001 00000020 00000002
> 				       00000001 000000a0 00000002
> 				       00000001 000004d0 00000002>;
> 				reserved-interrupts = <2>;
> 			};

This is an interrupt controller (it has an "interrupt-controller"
property, and it has no interrupt parent (there is no "interrupt-parent"
property, for interrupt controllers you do not follow the "normal" tree
parent), so it is the root interrupt controller and there is no loop.

It seems from your description that the Linux code is using the tree
parent as interrupt parent even for interrupt controller nodes.  This
is wrong behaviour.


Segher

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 22:33   ` Segher Boessenkool
@ 2007-10-01 22:54     ` Benjamin Herrenschmidt
  2007-10-01 22:55       ` Benjamin Herrenschmidt
                         ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 22:54 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev


On Tue, 2007-10-02 at 00:33 +0200, Segher Boessenkool wrote:
> > 		  	i8259: interrupt-controller@20 {
> > 				device_type = "interrupt-controller";
> > 				compatible = "pnpPNP,000";
> > 				interrupt-controller;
> > 				reg = <00000001 00000020 00000002
> > 				       00000001 000000a0 00000002
> > 				       00000001 000004d0 00000002>;
> > 				reserved-interrupts = <2>;
> > 			};
> 
> This is an interrupt controller (it has an "interrupt-controller"
> property, and it has no interrupt parent (there is no "interrupt-parent"
> property, for interrupt controllers you do not follow the "normal" tree
> parent), so it is the root interrupt controller and there is no loop.
> 
> It seems from your description that the Linux code is using the tree
> parent as interrupt parent even for interrupt controller nodes.  This
> is wrong behaviour.

It shoudn't normally happen. The reason it -does- happen in fact is that
the above node is also missing the #interrupt-cells property, which
cause the parent-lookup routine to skip it before it gets a chance to
see that there's an "interrupt-controller" property in there.

I'm not sure whether linux behaviour is a bug or not since I believe we
are clearly in undefined-land as an interrupt controller should always
have a #interrupt-cells property.

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 22:54     ` Benjamin Herrenschmidt
@ 2007-10-01 22:55       ` Benjamin Herrenschmidt
  2007-10-01 23:36       ` Segher Boessenkool
  2007-10-02 12:38       ` Gerhard Pircher
  2 siblings, 0 replies; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-01 22:55 UTC (permalink / raw)
  To: Segher Boessenkool; +Cc: linuxppc-dev, David Gibson


On Tue, 2007-10-02 at 08:54 +1000, Benjamin Herrenschmidt wrote:
> It shoudn't normally happen. The reason it -does- happen in fact is
> that
> the above node is also missing the #interrupt-cells property, which
> cause the parent-lookup routine to skip it before it gets a chance to
> see that there's an "interrupt-controller" property in there.
> 
> I'm not sure whether linux behaviour is a bug or not since I believe
> we
> are clearly in undefined-land as an interrupt controller should always
> have a #interrupt-cells property.

That's btw something we could add as a warning to dtc...

A node with "interrupt-controller" or "interrupt-map" in it should
always have a #interrupt-cells property.

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 22:54     ` Benjamin Herrenschmidt
  2007-10-01 22:55       ` Benjamin Herrenschmidt
@ 2007-10-01 23:36       ` Segher Boessenkool
  2007-10-02 12:38       ` Gerhard Pircher
  2 siblings, 0 replies; 18+ messages in thread
From: Segher Boessenkool @ 2007-10-01 23:36 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev

>> This is an interrupt controller (it has an "interrupt-controller"
>> property, and it has no interrupt parent (there is no 
>> "interrupt-parent"
>> property, for interrupt controllers you do not follow the "normal" 
>> tree
>> parent), so it is the root interrupt controller and there is no loop.
>>
>> It seems from your description that the Linux code is using the tree
>> parent as interrupt parent even for interrupt controller nodes.  This
>> is wrong behaviour.
>
> It shoudn't normally happen. The reason it -does- happen in fact is 
> that
> the above node is also missing the #interrupt-cells property, which
> cause the parent-lookup routine to skip it before it gets a chance to
> see that there's an "interrupt-controller" property in there.
>
> I'm not sure whether linux behaviour is a bug or not since I believe we
> are clearly in undefined-land as an interrupt controller should always
> have a #interrupt-cells property.

That isn't required for legacy (pieces of) trees that don't go
through an interrupt-map, actually (but strongly recommended of
course) -- and I'm not sure how valid a tree that uses the imap
recommended practice for only part of the tree is really ;-)

Either way, the Linux code should use the explicit information (the
lack of "interrupt-parent" in a node with "interrupt-controller")
to determine whether a node is the root interrupt controller, instead
of some implicit information.  You can then more clearly do sanity
checking on "#interrupt-cells" etc.


Segher

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 22:54     ` Benjamin Herrenschmidt
  2007-10-01 22:55       ` Benjamin Herrenschmidt
  2007-10-01 23:36       ` Segher Boessenkool
@ 2007-10-02 12:38       ` Gerhard Pircher
  2007-10-02 22:03         ` Benjamin Herrenschmidt
  2 siblings, 1 reply; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-02 12:38 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev


-------- Original-Nachricht --------
> Datum: Tue, 02 Oct 2007 07:39:47 +1000
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@ozlabs.org
> Betreff: Re: Problem with OF interrupt parsing code

> Part of your problem is that interrupt-parent property. You shouldn't
> have such a property in a PCI host bridge. It's not technically illegal,
> but it's triggering the "loop" you've been experiencing.
Okay, I'll remove this one.

> If you want the parsing to fail for PCI devices (to get the fallback to
> config space values), you need to make sure it does fail. 
> 
> Another option is to put an empty interrupt-map in there. That will
> guarantee failure.
> 
> But that's all very ugly. I don't understand why you don't setup a
> proper map either from your bootloader, zImage wrapper or even prom_init
> or platform code.
I know that it's ugly, but the problem is how to distinguish the boards.
The only real difference I know of is the PCI interrupt mapping. The
northbridges chip revision for example is always the same, but CPU type,
amount of memory and PCI devices can appear in all possible combinations.
The firmware doesn't tell me, which board the kernel is runnning on, so I
would like to rely on this fall back here until I get the chance to
update the firmware (which is beyond my control).

Gerhard
-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
@ 2007-10-02 12:40 Gerhard Pircher
  0 siblings, 0 replies; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-02 12:40 UTC (permalink / raw)
  To: benh, segher; +Cc: linuxppc-dev


-------- Original-Nachricht --------
> Datum: Tue, 02 Oct 2007 08:54:04 +1000
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Segher Boessenkool <segher@kernel.crashing.org>
> CC: Gerhard Pircher <gerhard_pircher@gmx.net>, linuxppc-dev@ozlabs.org
> Betreff: Re: Problem with OF interrupt parsing code

> It shoudn't normally happen. The reason it -does- happen in fact is that
> the above node is also missing the #interrupt-cells property, which
> cause the parent-lookup routine to skip it before it gets a chance to
> see that there's an "interrupt-controller" property in there.
> 
> I'm not sure whether linux behaviour is a bug or not since I believe we
> are clearly in undefined-land as an interrupt controller should always
> have a #interrupt-cells property.
I think these properties weren't specified in the OF CHRP ISA PIC device
binding document for the PIC node, thus I may have forgotten about them
(CHRP also defines a parent interrupt controller for the PIC, right?).
But the AmigaOne is not a CHRP platform, so I'll add them to the device
tree and then I will see how it works.

Thanks!

Gerhard

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-01 21:26 ` Scott Wood
  2007-10-01 21:37   ` Scott Wood
@ 2007-10-02 12:46   ` Gerhard Pircher
  1 sibling, 0 replies; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-02 12:46 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev


-------- Original-Nachricht --------
> Datum: Mon, 01 Oct 2007 16:26:14 -0500
> Von: Scott Wood <scottwood@freescale.com>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@ozlabs.org
> Betreff: Re: Problem with OF interrupt parsing code

> > Secondly the AmigaOne is a desktop system with 4 PCI/AGP slots,
> > thus I can't specify device nodes for all possible devices. By looking
> > at pci_read_irq_line() in pci_common.c it should be possible for the
> > kernel to fall back to the interrupt settings in the PCI config space
> > of every device.
> 
> Those interrupt settings are purely a communication vector from firmware 
> to OS.  Is your firmware putting i8259 interrupt numbers in there, and 
> did you set the default interrupt controller?
Yes, the firmware puts the i8259 interrupt numbers in the PCI interrupt
line register and the PIC is setup as default interrupt controller.

Gerhard

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-02 12:38       ` Gerhard Pircher
@ 2007-10-02 22:03         ` Benjamin Herrenschmidt
  2007-10-03  7:43           ` Gerhard Pircher
  0 siblings, 1 reply; 18+ messages in thread
From: Benjamin Herrenschmidt @ 2007-10-02 22:03 UTC (permalink / raw)
  To: Gerhard Pircher; +Cc: linuxppc-dev


On Tue, 2007-10-02 at 14:38 +0200, Gerhard Pircher wrote:
> I know that it's ugly, but the problem is how to distinguish the
> boards.
> The only real difference I know of is the PCI interrupt mapping. The
> northbridges chip revision for example is always the same, but CPU
> type,
> amount of memory and PCI devices can appear in all possible
> combinations.
> The firmware doesn't tell me, which board the kernel is runnning on,
> so I
> would like to rely on this fall back here until I get the chance to
> update the firmware (which is beyond my control).

And how does the firmware know ? There must be a strap somewhere...

Ben.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: Problem with OF interrupt parsing code
  2007-10-02 22:03         ` Benjamin Herrenschmidt
@ 2007-10-03  7:43           ` Gerhard Pircher
  0 siblings, 0 replies; 18+ messages in thread
From: Gerhard Pircher @ 2007-10-03  7:43 UTC (permalink / raw)
  To: benh; +Cc: linuxppc-dev


-------- Original-Nachricht --------
> Datum: Wed, 03 Oct 2007 08:03:27 +1000
> Von: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> An: Gerhard Pircher <gerhard_pircher@gmx.net>
> CC: linuxppc-dev@ozlabs.org
> Betreff: Re: Problem with OF interrupt parsing code

> 
> On Tue, 2007-10-02 at 14:38 +0200, Gerhard Pircher wrote:
> > I know that it's ugly, but the problem is how to distinguish the
> > boards. The only real difference I know of is the PCI interrupt
> > mapping. The northbridges chip revision for example is always the
> > same, but CPU type, amount of memory and PCI devices can appear in
> > all possible combinations. The firmware doesn't tell me, which board
> > the kernel is runnning on, so I would like to rely on this fall back
> > here until I get the chance to update the firmware (which is beyond
> > my control).
> 
> And how does the firmware know ? There must be a strap somewhere...
> 
> Ben.
Good question. I don't know. Anyway I changed the device tree based on
your comments and now it works (at least until the kernel deadlocks
somewhere else). :(

Gerhard

-- 
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen! 
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2007-10-03  7:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-10-01 21:00 Problem with OF interrupt parsing code Gerhard Pircher
2007-10-01 21:11 ` Gerhard Pircher
2007-10-01 21:39   ` Benjamin Herrenschmidt
2007-10-01 22:33   ` Segher Boessenkool
2007-10-01 22:54     ` Benjamin Herrenschmidt
2007-10-01 22:55       ` Benjamin Herrenschmidt
2007-10-01 23:36       ` Segher Boessenkool
2007-10-02 12:38       ` Gerhard Pircher
2007-10-02 22:03         ` Benjamin Herrenschmidt
2007-10-03  7:43           ` Gerhard Pircher
2007-10-01 21:26 ` Scott Wood
2007-10-01 21:37   ` Scott Wood
2007-10-01 21:43     ` Benjamin Herrenschmidt
2007-10-01 21:48       ` Scott Wood
2007-10-01 22:07         ` Benjamin Herrenschmidt
2007-10-02 12:46   ` Gerhard Pircher
2007-10-01 21:36 ` Benjamin Herrenschmidt
  -- strict thread matches above, loose matches on Subject: below --
2007-10-02 12:40 Gerhard Pircher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).