mpparse.c APIC/ACPI support

mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

I'm glad you got your system working with ACPI again.  Regarding
Windows, I've heard that it may|will remap just about everything to its
liking, so it doesn't surprise me that it's different.

I'm not an expert on APICs, but it seems that the
mp_config_acpi_legacy_irqs() routine shouldn't blindly set IRQs 0..15 to
ISA conforming mode (edge triggered).  I know for a fact that many
systems will have PCI devices using/sharing these IRQs, which will
require the IRQ to be level-triggered.

Perhaps Paul, Ingo or Alan could comment?

Thanks,
John

--------------------------------
John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
Principal Member Technical Staff
   Industry Standard Servers
    Hewlett-Packard Company
    http://www.hp.com/linux

> -----Original Message-----
> From: DPR [mailto:info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org] 
> Sent: Friday, February 28, 2003 8:46 PM
> To: Cagle, John (ISS-Houston); acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Subject: RE: [ACPI] APIC, USB mouse and errors
> 
> 
> Hi John,
> 
> YOU GOT IT - GREAT JOB !!!
> I made an ugly but working hack in 
> "mp_config_acpi_legacy_irqs" - I simply forced the trigger 
> mode to "level" for my USB-IRQs. And now everything works 
> fine !!! If you find another way to fix it - let me know ;)
> 
> Besides, do you know why the high IRQs (>17) are not used at 
> all ? I'm just wondering why M$ Windows produces such a 
> different IRQ-mapping - take a look at the attached file 
> (IRQs 0,1,4,6,8,9,13,15-19,21,23 are used).
> 
> Thanks for your professional help,
> 
> HOLGER
> 
> 
> > Holger,
> > 
> > I think that your APIC IRQs (0..15) aren't getting programmed
> > properly.
> > 
> > PCI is using IRQs 10, 11, 12 & 14, but I think the kernel 
> has made an 
> > incorrect assumption that these are ISA IRQs and set them to be 
> > edge-triggered.  PCI IRQs are always level-triggered.
> > 
> > Take a look at arch/i386/kernel/mpparse.c in the
> > mp_config_acpi_legacy_irqs() routine.  It sets  interrupts
> > 0..15 to ISA
> > bus conforming behavior (edge).
> > 
> > You might try forcing the kernel to do the right thing and see what 
> > happens.
> > 
> > Regards,
> > John
> 


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Grover, Andrew]

> From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] 
> I'm glad you got your system working with ACPI again.  Regarding
> Windows, I've heard that it may|will remap just about 
> everything to its
> liking, so it doesn't surprise me that it's different.
> 
> I'm not an expert on APICs, but it seems that the
> mp_config_acpi_legacy_irqs() routine shouldn't blindly set 
> IRQs 0..15 to
> ISA conforming mode (edge triggered).  I know for a fact that many
> systems will have PCI devices using/sharing these IRQs, which will
> require the IRQ to be level-triggered.

In theory, ACPI comes through later and changes the irqs < 15 with
possible PCI devices by calling io_apic_set_pci_routing.

So *in theory* it should work ok. Obviously something is not right.

Regards -- Andy

PS Paul and Ingo seem to have moved on to greener pastures, so I am not
copying them.


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

> -----Original Message-----
> From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org] 
> 
> > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > I'm glad you got your system working with ACPI again.  Regarding
> > Windows, I've heard that it may|will remap just about 
> > everything to its
> > liking, so it doesn't surprise me that it's different.
> > 
> > I'm not an expert on APICs, but it seems that the
> > mp_config_acpi_legacy_irqs() routine shouldn't blindly set
> > IRQs 0..15 to
> > ISA conforming mode (edge triggered).  I know for a fact that many
> > systems will have PCI devices using/sharing these IRQs, which will
> > require the IRQ to be level-triggered.
> 
> In theory, ACPI comes through later and changes the irqs < 15 
> with possible PCI devices by calling io_apic_set_pci_routing.
>
> So *in theory* it should work ok. Obviously something is not right.

How does ACPI, when it comes through later, know which irq's < 15 are
for PCI devices?

Thanks,
John


-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger
for complex code. Debugging C/C++ programs can leave you feeling lost and
disoriented. TotalView can help you find your way. Available on major UNIX
and Linux platforms. Try it free. www.etnus.com

RE: mpparse.c APIC/ACPI support

[Grover, Andrew]

> From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] 
> > In theory, ACPI comes through later and changes the irqs < 15 
> > with possible PCI devices by calling io_apic_set_pci_routing.
> >
> > So *in theory* it should work ok. Obviously something is not right.
> 
> How does ACPI, when it comes through later, know which irq's < 15 are
> for PCI devices?

The device's driver calls pci_enable_device, which calls:
pci_enable_device_bars, which calls:
pcibios_enable_device, which calls:
pcibios_enable_irq, which is a function pointer we set to
acpi_pci_irq_enable, which does an eisa_set_level_irq(), thus setting
the interrupt (if less than 16) to level-triggered.

So why isn't it working? Hmm.

Regards -- Andy


-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger 
for complex code. Debugging C/C++ programs can leave you feeling lost and 
disoriented. TotalView can help you find your way. Available on major UNIX 
and Linux platforms. Try it free. www.etnus.com

RE: mpparse.c APIC/ACPI support

[DPR]

> The device's driver calls pci_enable_device, which calls:
> pci_enable_device_bars, which calls:
> pcibios_enable_device, which calls:
> pcibios_enable_irq, which is a function pointer we set to
> acpi_pci_irq_enable, which does an eisa_set_level_irq(), thus setting
> the interrupt (if less than 16) to level-triggered.
> 
> So why isn't it working? Hmm.

Perhaps these messages from "pcibios_enable_irq" will help you finding the error ...

<4>PCI: No IRQ known for interrupt pin A of device 00:10.0
<4>PCI: No IRQ known for interrupt pin B of device 00:10.1
<4>PCI: No IRQ known for interrupt pin C of device 00:10.2
<4>PCI: No IRQ known for interrupt pin D of device 00:10.3
<4>PCI: No IRQ known for interrupt pin A of device 00:11.1 - using IRQ 255
<4>PCI: No IRQ known for interrupt pin A of device 00:12.0

The devices 00:10.0 up to 00:10.3 are my USB 1.1/2.0 controllers, device 00:11.1 is my EIDE
controller and device 00:12.0 is my ethernet adapter.

Regards,

HOLGER



-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger 
for complex code. Debugging C/C++ programs can leave you feeling lost and 
disoriented. TotalView can help you find your way. Available on major UNIX 
and Linux platforms. Try it free. www.etnus.com

Re: mpparse.c APIC/ACPI support

[Alan Cox]

> <4>PCI: No IRQ known for interrupt pin A of device 00:11.1 - using IRQ 255
> <4>PCI: No IRQ known for interrupt pin A of device 00:12.0
> 
> The devices 00:10.0 up to 00:10.3 are my USB 1.1/2.0 controllers, device 00:11.1 is my EIDE
> controller and device 00:12.0 is my ethernet adapter.

There is deep magic to beware of in the IDE case. I recently fixed this
for 2.4 (and sent Linus a patch for 2.5). If the IDE controller channels
are in legacy mode then the IRQ is 14/15 and ISA style. In that case we
shouldn't be looking up the IRQ line in the ACPI and PC BIOS cases at least.

For some non PC platforms it may be more complex


-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger 
for complex code. Debugging C/C++ programs can leave you feeling lost and 
disoriented. TotalView can help you find your way. Available on major UNIX 
and Linux platforms. Try it free. www.etnus.com

RE: mpparse.c APIC/ACPI support

[DPR]

> Perhaps these messages from "pcibios_enable_irq" will help 
> you finding the error ...

Sorry, the messages don't come from "pcibios_enable_irq" but from "acpi_pci_irq_init".
So this seems to be my problem because the following code snippet in "acpi_pci_irq_enable" will
never be executed (at least in my case)

	dev->irq = irq;

	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device %s using IRQ %d\n", dev->slot_name, dev->irq));

	/* 
	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
	 */
#ifdef CONFIG_X86
	if ((dev->irq < 16) &&  !((1 << dev->irq) & irq_mask)) {
		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d as level-triggered\n",
dev->irq));
		irq_mask |= (1 << dev->irq);
		eisa_set_level_irq(dev->irq);
	}
#endif

	return_VALUE(dev->irq);


Regards,

HOLGER



-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger 
for complex code. Debugging C/C++ programs can leave you feeling lost and 
disoriented. TotalView can help you find your way. Available on major UNIX 
and Linux platforms. Try it free. www.etnus.com

RE: mpparse.c APIC/ACPI support

[DPR]

And one more addition - "acpi_pci_irq_lookup" fails with "Link disabled", that's the reason why
"acpi_pci_irq_enable" returns without setting the PCI IRQs to level-triggered. I don't know what
"Link disabled" means but I hope one of the PCI/ACPI/APIC experts knows what to do and what's
going wrong :)

Thanks in advance,

HOLGER



-------------------------------------------------------
This SF.net email is sponsored by: Etnus, makers of TotalView, The debugger
for complex code. Debugging C/C++ programs can leave you feeling lost and
disoriented. TotalView can help you find your way. Available on major UNIX
and Linux platforms. Try it free. www.etnus.com

RE: mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

Sorry to revive an old thread, but it took us a while to root-cause
this.

What we have found is that the Linux ACPI code (from bitkeeper) is not
properly configuring all PCI interrupts:

In particular, mp_parse_prt() only calls io_apic_set_pci_routing() for
*static* entries in the PRT.  It doesn't do anything for dynamic entries
in the PRT.  To be correct, mp_parse_prt() needs to call
io_apic_set_pci_routing() for dynamic entries as well.

For example, a PCI-USB interrupt which is routed to IRQ 7 by the BIOS,
and which has a dynamic entry in the PRT would be left as edge-triggered
by default (since IRQ 7 is one of the "legacy" interrupts).  This breaks
USB interrupt handling (since it needs to be level triggered).

ACPI Gurus -- Does this make sense, and is there some reason that
dynamic PRT entries aren't being handled?

Thanks,
John
--------------------------------
John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
Principal Member Technical Staff
   Industry Standard Servers
    Hewlett-Packard Company
    http://www.hp.com/linux

> -----Original Message-----
> From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org] 
> Sent: Tuesday, March 04, 2003 12:21 PM
> To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> 
> 
> > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > In theory, ACPI comes through later and changes the irqs < 15
> > > with possible PCI devices by calling io_apic_set_pci_routing.
> > >
> > > So *in theory* it should work ok. Obviously something is 
> not right.
> > 
> > How does ACPI, when it comes through later, know which 
> irq's < 15 are 
> > for PCI devices?
> 
> The device's driver calls pci_enable_device, which calls: 
> pci_enable_device_bars, which calls: pcibios_enable_device, 
> which calls: pcibios_enable_irq, which is a function pointer 
> we set to acpi_pci_irq_enable, which does an 
> eisa_set_level_irq(), thus setting the interrupt (if less 
> than 16) to level-triggered.
> 
> So why isn't it working? Hmm.
> 
> Regards -- Andy
> 


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Grover, Andrew]

John,

Take a look at pci_irq.c acpi_pci_irq_enable(). We should be calling
this for all PCI devices, when they are enabled. We appear to be doing
the right thing in the PIC case (calling eisa_set_level_irq) but not in
the IO APIC case. I would think something like:

	/* 
	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
	 */
	if (acpi_ioapic)
	{
		int ioapic = mp_find_ioapic(irq);
		int ioapic_pin = irq -
mp_ioapic_routing[ioapic].irq_start;
		io_apic_set_pci_routing(ioapic, ioapic_pin, irq);
	}
#ifdef CONFIG_X86
	else if ((dev->irq < 16) &&  !((1 << dev->irq) & irq_mask)) {
		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d as
level-triggered\n", dev->irq));
		irq_mask |= (1 << dev->irq);
		eisa_set_level_irq(dev->irq);
	}
#endif

Totally untested. Thoughts? Do you have a system that we can verify a
fix on?

Regards -- Andy


> From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] 
> What we have found is that the Linux ACPI code (from 
> bitkeeper) is not properly configuring all PCI interrupts:
> 
> In particular, mp_parse_prt() only calls 
> io_apic_set_pci_routing() for *static* entries in the PRT.  
> It doesn't do anything for dynamic entries in the PRT.  To be 
> correct, mp_parse_prt() needs to call 
> io_apic_set_pci_routing() for dynamic entries as well.
> 
> For example, a PCI-USB interrupt which is routed to IRQ 7 by 
> the BIOS, and which has a dynamic entry in the PRT would be 
> left as edge-triggered by default (since IRQ 7 is one of the 
> "legacy" interrupts).  This breaks USB interrupt handling 
> (since it needs to be level triggered).
> 
> ACPI Gurus -- Does this make sense, and is there some reason 
> that dynamic PRT entries aren't being handled?
> 
> Thanks,
> John
> --------------------------------
> John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
> Principal Member Technical Staff
>    Industry Standard Servers
>     Hewlett-Packard Company
>     http://www.hp.com/linux
> 
> > -----Original Message-----
> > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > Sent: Tuesday, March 04, 2003 12:21 PM
> > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > 
> > 
> > > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > > In theory, ACPI comes through later and changes the 
> irqs < 15 with 
> > > > possible PCI devices by calling io_apic_set_pci_routing.
> > > >
> > > > So *in theory* it should work ok. Obviously something is
> > not right.
> > > 
> > > How does ACPI, when it comes through later, know which
> > irq's < 15 are
> > > for PCI devices?
> > 
> > The device's driver calls pci_enable_device, which calls:
> > pci_enable_device_bars, which calls: pcibios_enable_device, 
> > which calls: pcibios_enable_irq, which is a function pointer 
> > we set to acpi_pci_irq_enable, which does an 
> > eisa_set_level_irq(), thus setting the interrupt (if less 
> > than 16) to level-triggered.
> > 
> > So why isn't it working? Hmm.
> > 
> > Regards -- Andy
> > 
> 


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

Andy,

I couldn't easily test that code snippet, since it requires a lot of
kernel changes to export mp_find_ioapic() and mp_ioapic_routing[].
Also, mp_find_ioapic() has the __init attribute, and that would have to
change too.

Wouldn't this be better fixed when parsing the PRT in mpparse.c --
handle the dynamic irqs along with the static irqs?

Thanks,
John

> -----Original Message-----
> From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org] 
> Sent: Monday, April 14, 2003 7:32 PM
> To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> 
> John,
> 
> Take a look at pci_irq.c acpi_pci_irq_enable(). We should be 
> calling this for all PCI devices, when they are enabled. We 
> appear to be doing the right thing in the PIC case (calling 
> eisa_set_level_irq) but not in the IO APIC case. I would 
> think something like:
> 
> 	/* 
> 	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
> 	 */
> 	if (acpi_ioapic)
> 	{
> 		int ioapic = mp_find_ioapic(irq);
> 		int ioapic_pin = irq -
> mp_ioapic_routing[ioapic].irq_start;
> 		io_apic_set_pci_routing(ioapic, ioapic_pin, irq);
> 	}
> #ifdef CONFIG_X86
> 	else if ((dev->irq < 16) &&  !((1 << dev->irq) & irq_mask)) {
> 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d 
> as level-triggered\n", dev->irq));
> 		irq_mask |= (1 << dev->irq);
> 		eisa_set_level_irq(dev->irq);
> 	}
> #endif
> 
> Totally untested. Thoughts? Do you have a system that we can 
> verify a fix on?
> 
> Regards -- Andy
> 
> 
> > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > What we have found is that the Linux ACPI code (from 
> > bitkeeper) is not properly configuring all PCI interrupts:
> > 
> > In particular, mp_parse_prt() only calls
> > io_apic_set_pci_routing() for *static* entries in the PRT.  
> > It doesn't do anything for dynamic entries in the PRT.  To be 
> > correct, mp_parse_prt() needs to call 
> > io_apic_set_pci_routing() for dynamic entries as well.
> > 
> > For example, a PCI-USB interrupt which is routed to IRQ 7 by
> > the BIOS, and which has a dynamic entry in the PRT would be 
> > left as edge-triggered by default (since IRQ 7 is one of the 
> > "legacy" interrupts).  This breaks USB interrupt handling 
> > (since it needs to be level triggered).
> > 
> > ACPI Gurus -- Does this make sense, and is there some reason
> > that dynamic PRT entries aren't being handled?
> > 
> > Thanks,
> > John
> > --------------------------------
> > John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
> > Principal Member Technical Staff
> >    Industry Standard Servers
> >     Hewlett-Packard Company
> >     http://www.hp.com/linux
> > 
> > > -----Original Message-----
> > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > Sent: Tuesday, March 04, 2003 12:21 PM
> > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org;
> > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > 
> > > 
> > > > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > > > In theory, ACPI comes through later and changes the
> > irqs < 15 with
> > > > > possible PCI devices by calling io_apic_set_pci_routing.
> > > > >
> > > > > So *in theory* it should work ok. Obviously something is
> > > not right.
> > > > 
> > > > How does ACPI, when it comes through later, know which
> > > irq's < 15 are
> > > > for PCI devices?
> > > 
> > > The device's driver calls pci_enable_device, which calls: 
> > > pci_enable_device_bars, which calls: pcibios_enable_device, which 
> > > calls: pcibios_enable_irq, which is a function pointer we set to 
> > > acpi_pci_irq_enable, which does an eisa_set_level_irq(), thus 
> > > setting the interrupt (if less than 16) to level-triggered.
> > > 
> > > So why isn't it working? Hmm.
> > > 
> > > Regards -- Andy


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

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

Andy,

Attached is a small patch that fixes the problem for dynamic PRT
entries.

We have tested this (with your 2.4 bk kernel) and it works perfectly.
Please consider including this in your next update to Marcelo & Linus
for 2.4 and 2.5.

Credit goes to Dan Zink for this patch.

Thanks,
John

> -----Original Message-----
> From: Cagle, John (ISS-Houston) 
> Sent: Tuesday, April 15, 2003 2:41 AM
> To: Grover, Andrew; info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; acpi-devel@lists.sourceforge.net
> Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> 
> 
> Andy,
> 
> I couldn't easily test that code snippet, since it requires a 
> lot of kernel changes to export mp_find_ioapic() and 
> mp_ioapic_routing[]. Also, mp_find_ioapic() has the __init 
> attribute, and that would have to change too.
> 
> Wouldn't this be better fixed when parsing the PRT in 
> mpparse.c -- handle the dynamic irqs along with the static irqs?
> 
> Thanks,
> John
> 
> > -----Original Message-----
> > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > Sent: Monday, April 14, 2003 7:32 PM
> > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > 
> > John,
> > 
> > Take a look at pci_irq.c acpi_pci_irq_enable(). We should be
> > calling this for all PCI devices, when they are enabled. We 
> > appear to be doing the right thing in the PIC case (calling 
> > eisa_set_level_irq) but not in the IO APIC case. I would 
> > think something like:
> > 
> > 	/* 
> > 	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
> > 	 */
> > 	if (acpi_ioapic)
> > 	{
> > 		int ioapic = mp_find_ioapic(irq);
> > 		int ioapic_pin = irq -
> > mp_ioapic_routing[ioapic].irq_start;
> > 		io_apic_set_pci_routing(ioapic, ioapic_pin, irq);
> > 	}
> > #ifdef CONFIG_X86
> > 	else if ((dev->irq < 16) &&  !((1 << dev->irq) & irq_mask)) {
> > 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d
> > as level-triggered\n", dev->irq));
> > 		irq_mask |= (1 << dev->irq);
> > 		eisa_set_level_irq(dev->irq);
> > 	}
> > #endif
> > 
> > Totally untested. Thoughts? Do you have a system that we can
> > verify a fix on?
> > 
> > Regards -- Andy
> > 
> > 
> > > From: Cagle, John (ISS-Houston) 
> [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] What we 
> > > have found is that the Linux ACPI code (from
> > > bitkeeper) is not properly configuring all PCI interrupts:
> > > 
> > > In particular, mp_parse_prt() only calls
> > > io_apic_set_pci_routing() for *static* entries in the PRT.
> > > It doesn't do anything for dynamic entries in the PRT.  To be 
> > > correct, mp_parse_prt() needs to call 
> > > io_apic_set_pci_routing() for dynamic entries as well.
> > > 
> > > For example, a PCI-USB interrupt which is routed to IRQ 7 by the 
> > > BIOS, and which has a dynamic entry in the PRT would be left as 
> > > edge-triggered by default (since IRQ 7 is one of the "legacy" 
> > > interrupts).  This breaks USB interrupt handling (since 
> it needs to 
> > > be level triggered).
> > > 
> > > ACPI Gurus -- Does this make sense, and is there some reason that 
> > > dynamic PRT entries aren't being handled?
> > > 
> > > Thanks,
> > > John
> > > --------------------------------
> > > John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
> > > Principal Member Technical Staff
> > >    Industry Standard Servers
> > >     Hewlett-Packard Company
> > >     http://www.hp.com/linux
> > > 
> > > > -----Original Message-----
> > > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > > Sent: Tuesday, March 04, 2003 12:21 PM
> > > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> > > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > > 
> > > > 
> > > > > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > > > > In theory, ACPI comes through later and changes the
> > > irqs < 15 with
> > > > > > possible PCI devices by calling io_apic_set_pci_routing.
> > > > > >
> > > > > > So *in theory* it should work ok. Obviously something is
> > > > not right.
> > > > > 
> > > > > How does ACPI, when it comes through later, know which
> > > > irq's < 15 are
> > > > > for PCI devices?
> > > > 
> > > > The device's driver calls pci_enable_device, which calls:
> > > > pci_enable_device_bars, which calls: 
> pcibios_enable_device, which 
> > > > calls: pcibios_enable_irq, which is a function pointer 
> we set to 
> > > > acpi_pci_irq_enable, which does an eisa_set_level_irq(), thus 
> > > > setting the interrupt (if less than 16) to level-triggered.
> > > > 
> > > > So why isn't it working? Hmm.
> > > > 
> > > > Regards -- Andy

[-- Attachment #2: dynamic_irq.patch --]
[-- Type: application/octet-stream, Size: 806 bytes --]

diff -uar linux-2.4-acpi/arch/i386/kernel/mpparse.c linux-2.4-acpi_fixed/arch/i386/kernel/mpparse.c
--- linux-2.4-acpi/arch/i386/kernel/mpparse.c	2003-04-08 14:24:16.000000000 -0700
+++ linux-2.4-acpi_fixed/arch/i386/kernel/mpparse.c	2003-04-15 08:07:25.000000000 -0700
@@ -1274,11 +1275,14 @@
 	list_for_each(node, &acpi_prt.entries) {
 		entry = list_entry(node, struct acpi_prt_entry, node);
 
-		/* We're only interested in static (non-link) entries. */
-		if (entry->link.handle)
-			continue;
+		/* Need to get irq for dynamic entry */
+		if (entry->link.handle) {
+			irq = acpi_pci_link_get_irq(entry->link.handle, entry->link.index);
+			if (!irq)
+				continue;
+		}
+		else irq = entry->link.index;
 
-		irq = entry->link.index;
 		ioapic = mp_find_ioapic(irq);
 		if (ioapic < 0)
 			continue;

RE: mpparse.c APIC/ACPI support

[Grover, Andrew]

Super cool, thanks to you and Dan.

I'm too jaded to think that this will fix all the various PCI routing
problems on SMP systems, but I am optimistic that it should fix a
sizeable percentage.

Regards -- Andy

> -----Original Message-----
> From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] 
> Sent: Tuesday, April 15, 2003 12:54 PM
> To: Grover, Andrew; info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; acpi-devel@lists.sourceforge.net
> Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> 
> 
> Andy,
> 
> Attached is a small patch that fixes the problem for dynamic 
> PRT entries.
> 
> We have tested this (with your 2.4 bk kernel) and it works 
> perfectly. Please consider including this in your next update 
> to Marcelo & Linus for 2.4 and 2.5.
> 
> Credit goes to Dan Zink for this patch.
> 
> Thanks,
> John
> 
> > -----Original Message-----
> > From: Cagle, John (ISS-Houston)
> > Sent: Tuesday, April 15, 2003 2:41 AM
> > To: Grover, Andrew; info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > 
> > 
> > Andy,
> > 
> > I couldn't easily test that code snippet, since it requires a
> > lot of kernel changes to export mp_find_ioapic() and 
> > mp_ioapic_routing[]. Also, mp_find_ioapic() has the __init 
> > attribute, and that would have to change too.
> > 
> > Wouldn't this be better fixed when parsing the PRT in
> > mpparse.c -- handle the dynamic irqs along with the static irqs?
> > 
> > Thanks,
> > John
> > 
> > > -----Original Message-----
> > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > Sent: Monday, April 14, 2003 7:32 PM
> > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org;
> > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > 
> > > John,
> > > 
> > > Take a look at pci_irq.c acpi_pci_irq_enable(). We should 
> be calling 
> > > this for all PCI devices, when they are enabled. We appear to be 
> > > doing the right thing in the PIC case (calling
> > > eisa_set_level_irq) but not in the IO APIC case. I would
> > > think something like:
> > > 
> > > 	/* 
> > > 	 * Make sure all (legacy) PCI IRQs are set as level-triggered.
> > > 	 */
> > > 	if (acpi_ioapic)
> > > 	{
> > > 		int ioapic = mp_find_ioapic(irq);
> > > 		int ioapic_pin = irq - 
> mp_ioapic_routing[ioapic].irq_start;
> > > 		io_apic_set_pci_routing(ioapic, ioapic_pin, irq);
> > > 	}
> > > #ifdef CONFIG_X86
> > > 	else if ((dev->irq < 16) &&  !((1 << dev->irq) & irq_mask)) {
> > > 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d
> > > as level-triggered\n", dev->irq));
> > > 		irq_mask |= (1 << dev->irq);
> > > 		eisa_set_level_irq(dev->irq);
> > > 	}
> > > #endif
> > > 
> > > Totally untested. Thoughts? Do you have a system that we 
> can verify 
> > > a fix on?
> > > 
> > > Regards -- Andy
> > > 
> > > 
> > > > From: Cagle, John (ISS-Houston)
> > [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] What we
> > > > have found is that the Linux ACPI code (from
> > > > bitkeeper) is not properly configuring all PCI interrupts:
> > > > 
> > > > In particular, mp_parse_prt() only calls
> > > > io_apic_set_pci_routing() for *static* entries in the PRT. It 
> > > > doesn't do anything for dynamic entries in the PRT.  To be 
> > > > correct, mp_parse_prt() needs to call
> > > > io_apic_set_pci_routing() for dynamic entries as well.
> > > > 
> > > > For example, a PCI-USB interrupt which is routed to IRQ 7 by the
> > > > BIOS, and which has a dynamic entry in the PRT would be left as 
> > > > edge-triggered by default (since IRQ 7 is one of the "legacy" 
> > > > interrupts).  This breaks USB interrupt handling (since 
> > it needs to
> > > > be level triggered).
> > > > 
> > > > ACPI Gurus -- Does this make sense, and is there some 
> reason that
> > > > dynamic PRT entries aren't being handled?
> > > > 
> > > > Thanks,
> > > > John
> > > > --------------------------------
> > > > John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
> > > > Principal Member Technical Staff
> > > >    Industry Standard Servers
> > > >     Hewlett-Packard Company
> > > >     http://www.hp.com/linux
> > > > 
> > > > > -----Original Message-----
> > > > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > > > Sent: Tuesday, March 04, 2003 12:21 PM
> > > > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org;
> > > > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > > > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > > > 
> > > > > 
> > > > > > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > > > > > In theory, ACPI comes through later and changes the
> > > > irqs < 15 with
> > > > > > > possible PCI devices by calling io_apic_set_pci_routing.
> > > > > > >
> > > > > > > So *in theory* it should work ok. Obviously something is
> > > > > not right.
> > > > > > 
> > > > > > How does ACPI, when it comes through later, know which
> > > > > irq's < 15 are
> > > > > > for PCI devices?
> > > > > 
> > > > > The device's driver calls pci_enable_device, which calls: 
> > > > > pci_enable_device_bars, which calls:
> > pcibios_enable_device, which
> > > > > calls: pcibios_enable_irq, which is a function pointer
> > we set to
> > > > > acpi_pci_irq_enable, which does an eisa_set_level_irq(), thus
> > > > > setting the interrupt (if less than 16) to level-triggered.
> > > > > 
> > > > > So why isn't it working? Hmm.
> > > > > 
> > > > > Regards -- Andy
> 


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

RE: mpparse.c APIC/ACPI support

[Cagle, John (ISS-Houston)]

Andy,

We're glad to be able to help.  On our implementations, this problem
showed up with ServerWorks CSB5 & CSB6 chipsets that have the USB
interrupt internally routed.  That design led to the use of dynamic PRT
entries for the USB (and maybe SCI) interrupts.

Cheers,
John

> -----Original Message-----
> From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org] 
> Sent: Friday, April 18, 2003 10:57 AM
> To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> 
> 
> Super cool, thanks to you and Dan.
> 
> I'm too jaded to think that this will fix all the various PCI 
> routing problems on SMP systems, but I am optimistic that it 
> should fix a sizeable percentage.
> 
> Regards -- Andy
> 
> > -----Original Message-----
> > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > Sent: Tuesday, April 15, 2003 12:54 PM
> > To: Grover, Andrew; info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > 
> > 
> > Andy,
> > 
> > Attached is a small patch that fixes the problem for dynamic
> > PRT entries.
> > 
> > We have tested this (with your 2.4 bk kernel) and it works
> > perfectly. Please consider including this in your next update 
> > to Marcelo & Linus for 2.4 and 2.5.
> > 
> > Credit goes to Dan Zink for this patch.
> > 
> > Thanks,
> > John
> > 
> > > -----Original Message-----
> > > From: Cagle, John (ISS-Houston)
> > > Sent: Tuesday, April 15, 2003 2:41 AM
> > > To: Grover, Andrew; info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org;
> > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > 
> > > 
> > > Andy,
> > > 
> > > I couldn't easily test that code snippet, since it 
> requires a lot of 
> > > kernel changes to export mp_find_ioapic() and 
> mp_ioapic_routing[]. 
> > > Also, mp_find_ioapic() has the __init attribute, and that 
> would have 
> > > to change too.
> > > 
> > > Wouldn't this be better fixed when parsing the PRT in 
> mpparse.c -- 
> > > handle the dynamic irqs along with the static irqs?
> > > 
> > > Thanks,
> > > John
> > > 
> > > > -----Original Message-----
> > > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > > Sent: Monday, April 14, 2003 7:32 PM
> > > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> > > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org; Zink, Dan; Mathiasen, Torben
> > > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > > 
> > > > John,
> > > > 
> > > > Take a look at pci_irq.c acpi_pci_irq_enable(). We should
> > be calling
> > > > this for all PCI devices, when they are enabled. We appear to be
> > > > doing the right thing in the PIC case (calling
> > > > eisa_set_level_irq) but not in the IO APIC case. I would
> > > > think something like:
> > > > 
> > > > 	/* 
> > > > 	 * Make sure all (legacy) PCI IRQs are set as 
> level-triggered.
> > > > 	 */
> > > > 	if (acpi_ioapic)
> > > > 	{
> > > > 		int ioapic = mp_find_ioapic(irq);
> > > > 		int ioapic_pin = irq -
> > mp_ioapic_routing[ioapic].irq_start;
> > > > 		io_apic_set_pci_routing(ioapic, 
> ioapic_pin, irq);
> > > > 	}
> > > > #ifdef CONFIG_X86
> > > > 	else if ((dev->irq < 16) &&  !((1 << dev->irq) 
> & irq_mask)) {
> > > > 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Setting IRQ %d
> > > > as level-triggered\n", dev->irq));
> > > > 		irq_mask |= (1 << dev->irq);
> > > > 		eisa_set_level_irq(dev->irq);
> > > > 	}
> > > > #endif
> > > > 
> > > > Totally untested. Thoughts? Do you have a system that we
> > can verify
> > > > a fix on?
> > > > 
> > > > Regards -- Andy
> > > > 
> > > > 
> > > > > From: Cagle, John (ISS-Houston)
> > > [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org] What we
> > > > > have found is that the Linux ACPI code (from
> > > > > bitkeeper) is not properly configuring all PCI interrupts:
> > > > > 
> > > > > In particular, mp_parse_prt() only calls
> > > > > io_apic_set_pci_routing() for *static* entries in the PRT. It
> > > > > doesn't do anything for dynamic entries in the PRT.  To be 
> > > > > correct, mp_parse_prt() needs to call
> > > > > io_apic_set_pci_routing() for dynamic entries as well.
> > > > > 
> > > > > For example, a PCI-USB interrupt which is routed to 
> IRQ 7 by the 
> > > > > BIOS, and which has a dynamic entry in the PRT would 
> be left as 
> > > > > edge-triggered by default (since IRQ 7 is one of the "legacy" 
> > > > > interrupts).  This breaks USB interrupt handling (since
> > > it needs to
> > > > > be level triggered).
> > > > > 
> > > > > ACPI Gurus -- Does this make sense, and is there some
> > reason that
> > > > > dynamic PRT entries aren't being handled?
> > > > > 
> > > > > Thanks,
> > > > > John
> > > > > --------------------------------
> > > > > John Cagle     john.cagle-VXdhtT5mjnY@public.gmane.org
> > > > > Principal Member Technical Staff
> > > > >    Industry Standard Servers
> > > > >     Hewlett-Packard Company
> > > > >     http://www.hp.com/linux
> > > > > 
> > > > > > -----Original Message-----
> > > > > > From: Grover, Andrew [mailto:andrew.grover-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org]
> > > > > > Sent: Tuesday, March 04, 2003 12:21 PM
> > > > > > To: Cagle, John (ISS-Houston); info-IGm5bqYn3xwb1SvskN2V4Q@public.gmane.org; 
> > > > > > acpi-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
> > > > > > Cc: alan-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org
> > > > > > Subject: RE: [ACPI] mpparse.c APIC/ACPI support
> > > > > > 
> > > > > > 
> > > > > > > From: Cagle, John (ISS-Houston) [mailto:john.cagle-VXdhtT5mjnY@public.gmane.org]
> > > > > > > > In theory, ACPI comes through later and changes the
> > > > > irqs < 15 with
> > > > > > > > possible PCI devices by calling io_apic_set_pci_routing.
> > > > > > > >
> > > > > > > > So *in theory* it should work ok. Obviously something is
> > > > > > not right.
> > > > > > > 
> > > > > > > How does ACPI, when it comes through later, know which
> > > > > > irq's < 15 are
> > > > > > > for PCI devices?
> > > > > > 
> > > > > > The device's driver calls pci_enable_device, which calls:
> > > > > > pci_enable_device_bars, which calls:
> > > pcibios_enable_device, which
> > > > > > calls: pcibios_enable_irq, which is a function pointer
> > > we set to
> > > > > > acpi_pci_irq_enable, which does an 
> eisa_set_level_irq(), thus 
> > > > > > setting the interrupt (if less than 16) to level-triggered.
> > > > > > 
> > > > > > So why isn't it working? Hmm.
> > > > > > 
> > > > > > Regards -- Andy
> > 
> 


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf