linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [-next-20130204] usb/hcd:  irq 18: nobody cared
@ 2013-02-05 17:51 Peter Hurley
  2013-02-05 20:26 ` Alan Stern
  0 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-02-05 17:51 UTC (permalink / raw)
  To: Alan Stern, Lan Tianyu
  Cc: Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel, linux-next

With -next-20130204:

[   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
[   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
[   33.855582] Call Trace:
[   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
[   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
[   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
[   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
[   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
[   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
[   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
[   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
[   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
[   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
[   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
[   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
[   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
[   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
[   33.855660] handlers:
[   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
[   33.855667] Disabling IRQ #18

>From earlier in the boot log:
[    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
[    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
[    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
[    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
[    1.297213] hub 4-0:1.0: USB hub found
[    1.297221] hub 4-0:1.0: 2 ports detected

lsusb:
...
Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver
...

Didn't have this problem with -next-20120125. One of these commits, maybe?
next/drivers/usb/core$ git log --oneline next-20130125..next-20130204 -- *
2c2e865 usb: forbid memory allocation with I/O during bus reset
e9121a8 Merge remote-tracking branch 'usb/usb-next'
3b2ab2b Revert "usb: Register usb port's acpi power resources"
da0aa71 USB: add usb_hcd_{start,end}_port_resume
192fef1 usb: enable usb port device's async suspend.
f6cced1 usb: expose usb port's pm qos flags to user space
ad493e5 usb: add usb port auto power off mechanism
971fcd4 usb: add runtime pm support for usb port device
88bb965 usb: Register usb port's acpi power resources
54a3ac0 usb: Using correct way to clear usb3.0 device's remote wakeup feature.

I didn't see any changes in the drivers/hid/hid-logitech-dj.c or usbhid
but maybe it's doing something wrong anyway?

I'll open a bugzilla if a bunch more info is necessary.

Regards,
Peter Hurley




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

* Re: [-next-20130204] usb/hcd:  irq 18: nobody cared
  2013-02-05 17:51 [-next-20130204] usb/hcd: irq 18: nobody cared Peter Hurley
@ 2013-02-05 20:26 ` Alan Stern
  2013-02-08 11:57   ` Peter Hurley
  2013-02-10  3:14   ` [Bisected] " Peter Hurley
  0 siblings, 2 replies; 27+ messages in thread
From: Alan Stern @ 2013-02-05 20:26 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Lan Tianyu, Greg Kroah-Hartman, Jiri Kosina, linux-usb,
	linux-kernel, linux-next

On Tue, 5 Feb 2013, Peter Hurley wrote:

> With -next-20130204:
> 
> [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
> [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
> [   33.855582] Call Trace:
> [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
> [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
> [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
> [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
> [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
> [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
> [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
> [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
> [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
> [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
> [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
> [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
> [   33.855660] handlers:
> [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
> [   33.855667] Disabling IRQ #18
> 
> From earlier in the boot log:
> [    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
> [    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
> [    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
> [    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
> [    1.297213] hub 4-0:1.0: USB hub found
> [    1.297221] hub 4-0:1.0: 2 ports detected
> 
> lsusb:
> ...
> Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
> Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver
> ...
> 
> Didn't have this problem with -next-20120125. One of these commits, maybe?
> next/drivers/usb/core$ git log --oneline next-20130125..next-20130204 -- *
> 2c2e865 usb: forbid memory allocation with I/O during bus reset
> e9121a8 Merge remote-tracking branch 'usb/usb-next'
> 3b2ab2b Revert "usb: Register usb port's acpi power resources"
> da0aa71 USB: add usb_hcd_{start,end}_port_resume
> 192fef1 usb: enable usb port device's async suspend.
> f6cced1 usb: expose usb port's pm qos flags to user space
> ad493e5 usb: add usb port auto power off mechanism
> 971fcd4 usb: add runtime pm support for usb port device
> 88bb965 usb: Register usb port's acpi power resources
> 54a3ac0 usb: Using correct way to clear usb3.0 device's remote wakeup feature.

None of those commits seems likely to have caused the problem.  The 
most likely is da0aa71, combined with one you didn't mention:

840008bb USB: UHCI: notify usbcore about port resumes

If you revert these two commits, does that make any difference?  What
if you revert all of these commits?

> I didn't see any changes in the drivers/hid/hid-logitech-dj.c or usbhid
> but maybe it's doing something wrong anyway?
> 
> I'll open a bugzilla if a bunch more info is necessary.

If the suggestion above doesn't work out, bisection may be the best way
to find the answer.

Alan Stern


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

* Re: [-next-20130204] usb/hcd:  irq 18: nobody cared
  2013-02-05 20:26 ` Alan Stern
@ 2013-02-08 11:57   ` Peter Hurley
  2013-02-10  3:14   ` [Bisected] " Peter Hurley
  1 sibling, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-02-08 11:57 UTC (permalink / raw)
  To: Alan Stern
  Cc: Lan Tianyu, Greg Kroah-Hartman, Jiri Kosina, linux-usb,
	linux-kernel, linux-next

On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
> On Tue, 5 Feb 2013, Peter Hurley wrote:
> 
> > With -next-20130204:
> > 
> > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
> > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
> > [   33.855582] Call Trace:
> > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
> > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
> > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
> > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
> > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
> > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
> > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
> > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
> > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
> > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
> > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
> > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
> > [   33.855660] handlers:
> > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
> > [   33.855667] Disabling IRQ #18
> > 
> > From earlier in the boot log:
> > [    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
> > [    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
> > [    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
> > [    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
> > [    1.297213] hub 4-0:1.0: USB hub found
> > [    1.297221] hub 4-0:1.0: 2 ports detected
> > 
> > lsusb:
> > ...
> > Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
> > Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver
> > ...
> > 
> > Didn't have this problem with -next-20120125. One of these commits, maybe?
> > next/drivers/usb/core$ git log --oneline next-20130125..next-20130204 -- *
> > 2c2e865 usb: forbid memory allocation with I/O during bus reset
> > e9121a8 Merge remote-tracking branch 'usb/usb-next'
> > 3b2ab2b Revert "usb: Register usb port's acpi power resources"
> > da0aa71 USB: add usb_hcd_{start,end}_port_resume
> > 192fef1 usb: enable usb port device's async suspend.
> > f6cced1 usb: expose usb port's pm qos flags to user space
> > ad493e5 usb: add usb port auto power off mechanism
> > 971fcd4 usb: add runtime pm support for usb port device
> > 88bb965 usb: Register usb port's acpi power resources
> > 54a3ac0 usb: Using correct way to clear usb3.0 device's remote wakeup feature.
> 
> None of those commits seems likely to have caused the problem.  The 
> most likely is da0aa71, combined with one you didn't mention:
> 
> 840008bb USB: UHCI: notify usbcore about port resumes
> 
> If you revert these two commits, does that make any difference?  What
> if you revert all of these commits?

None of these commits are directly causing this. I tested Greg's usb
master and usb-next and both run fine for me.

> > I didn't see any changes in the drivers/hid/hid-logitech-dj.c or usbhid
> > but maybe it's doing something wrong anyway?
> > 
> > I'll open a bugzilla if a bunch more info is necessary.
> 
> If the suggestion above doesn't work out, bisection may be the best way
> to find the answer.

Any pointers on how to one of) build a linear history for -next, get
bisect to work on an integration tree, or third option I haven't thought
of?

Thanks,
Peter Hurley



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

* [Bisected] [-next-20130204] usb/hcd:  irq 18: nobody cared
  2013-02-05 20:26 ` Alan Stern
  2013-02-08 11:57   ` Peter Hurley
@ 2013-02-10  3:14   ` Peter Hurley
  2013-02-10 14:23     ` Peter Hurley
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-02-10  3:14 UTC (permalink / raw)
  To: Yinghai Lu, Bjorn Helgaas, Rafael J. Wysocki
  Cc: Alan Stern, Lan Tianyu, Greg Kroah-Hartman, Jiri Kosina,
	linux-usb, linux-kernel, linux-next

On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
> On Tue, 5 Feb 2013, Peter Hurley wrote:
> 
> > With -next-20130204:
> > 
> > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
> > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
> > [   33.855582] Call Trace:
> > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
> > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
> > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
> > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
> > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
> > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
> > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
> > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
> > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
> > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
> > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
> > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
> > [   33.855660] handlers:
> > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
> > [   33.855667] Disabling IRQ #18
> > 
> > From earlier in the boot log:
> > [    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
> > [    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
> > [    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
> > [    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
> > [    1.297213] hub 4-0:1.0: USB hub found
> > [    1.297221] hub 4-0:1.0: 2 ports detected
> > 
> > lsusb:
> > ...
> > Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
> > Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver

[...]

> If the suggestion above doesn't work out, bisection may be the best way
> to find the answer.

This bad irq bisected to:

4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4 is the first bad commit
commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
Author: Yinghai Lu <yinghai@kernel.org>
Date:   Mon Jan 21 13:20:52 2013 -0800

    PCI: Put pci_dev in device tree as early as possible
    
    We want to put pci_dev structs in the device tree as soon as possible so
    for_each_pci_dev() iteration will not miss them, but driver attachment
    needs to be delayed until after pci_assign_unassigned_resources() to make
    sure all devices have resources assigned first.
    
    This patch moves device registering from pci_bus_add_devices() to
    pci_device_add(), which happens earlier, leaving driver attachment in
    pci_bus_add_devices().
    
    It also removes unattached child bus handling in pci_bus_add_devices().
    That's not needed because child bus via pci_add_new_bus() is already
    in parent bus children list.
    
    [bhelgaas: changelog]
    Signed-off-by: Yinghai Lu <yinghai@kernel.org>
    Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
    Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

:040000 040000 0540c98d04d00de24b4e12fa750f6cd26c5addd2 2e24946cb7165a4028b7efb0a622271cc3990005 M	drivers

All is fine if I revert these 2 commits:

40064ac PCI: Remove unused "rc" in virtfn_add_bus()
4f53509 (HEAD, refs/bisect/bad) PCI: Put pci_dev in device tree as early as possible

Any ideas how these are causing the USB HCD core / hid-logitech-dj
driver to drop interrupts?

Regards,
Peter Hurley


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

* Re: [Bisected] [-next-20130204] usb/hcd:  irq 18: nobody cared
  2013-02-10  3:14   ` [Bisected] " Peter Hurley
@ 2013-02-10 14:23     ` Peter Hurley
  2013-02-10 20:33       ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-02-10 14:23 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next

On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
> > On Tue, 5 Feb 2013, Peter Hurley wrote:
> > 
> > > With -next-20130204:
> > > 
> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
> > > [   33.855582] Call Trace:
> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
> > > [   33.855660] handlers:
> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
> > > [   33.855667] Disabling IRQ #18
> > > 
> > > From earlier in the boot log:
> > > [    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
> > > [    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
> > > [    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
> > > [    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
> > > [    1.297213] hub 4-0:1.0: USB hub found
> > > [    1.297221] hub 4-0:1.0: 2 ports detected
> > > 
> > > lsusb:
> > > ...
> > > Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
> > > Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver
> 
> [...]
> 
> > If the suggestion above doesn't work out, bisection may be the best way
> > to find the answer.
> 
> This bad irq bisected to:
> 
> 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4 is the first bad commit
> commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
> Author: Yinghai Lu <yinghai@kernel.org>
> Date:   Mon Jan 21 13:20:52 2013 -0800
> 
>     PCI: Put pci_dev in device tree as early as possible
>     
>     We want to put pci_dev structs in the device tree as soon as possible so
>     for_each_pci_dev() iteration will not miss them, but driver attachment
>     needs to be delayed until after pci_assign_unassigned_resources() to make
>     sure all devices have resources assigned first.
>     
>     This patch moves device registering from pci_bus_add_devices() to
>     pci_device_add(), which happens earlier, leaving driver attachment in
>     pci_bus_add_devices().
>     
>     It also removes unattached child bus handling in pci_bus_add_devices().
>     That's not needed because child bus via pci_add_new_bus() is already
>     in parent bus children list.
>     
>     [bhelgaas: changelog]
>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>     Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> :040000 040000 0540c98d04d00de24b4e12fa750f6cd26c5addd2 2e24946cb7165a4028b7efb0a622271cc3990005 M	drivers
> 
> All is fine if I revert these 2 commits:
> 
> 40064ac PCI: Remove unused "rc" in virtfn_add_bus()
> 4f53509 (HEAD, refs/bisect/bad) PCI: Put pci_dev in device tree as early as possible
> 
> Any ideas how these are causing the USB HCD core / hid-logitech-dj
> driver to drop interrupts?

https://bugzilla.kernel.org/show_bug.cgi?id=53561

Maybe this is some interaction with all the new ACPI code and fixes
written in those 8 days.

peter@thor:~/src/kernels/next$ git log --oneline next-20130125..next-20130204 | grep -i acpi
65d6ce7 Merge remote-tracking branch 'acpi/next'
0d50e8c Merge branch 'acpi-pm-next' into linux-next
be6d286 PCI: acpiphp: Remove dead code for PCI host bridge hotplug
4b794a0 Merge branch 'acpi-assorted' into linux-next
bd3e4a3 Merge branch 'acpi-cleanup' into linux-next
02df734 ACPI / dock: Fix acpi_bus_get_device() check in drivers/acpi/dock.c
7217dac Merge branch 'acpi-lpss' into linux-next
4bede3f Merge branch 'acpi-pm' into linux-next
cc0755b Merge branch 'acpica' into linux-next
308b10d Merge branch 'acpi-scan' into linux-next
2ca344e PCI: acpiphp: Create companion ACPI devices before creating PCI devices
741220e cpufreq: Make acpi-cpufreq link first
c40a451 acpi-cpufreq: Do not load on K8
b378549 ACPI / PM: Do not power manage devices in unknown initial states
cfc5755 PNPACPI: Fix acpi_bus_get_device() check in drivers/pnp/pnpacpi/core.c
dde3bb4 ACPI / PM: Fix acpi_bus_get_device() check in drivers/acpi/device_pm.c
456de89 ACPI / scan: Clean up acpi_bus_get_parent()
0613e1f ACPI / scan: Fix acpi_bus_get_device() check in acpi_match_device()
141a297 ACPI / platform: Use struct acpi_scan_handler for creating devices
4daeaf6 ACPI / PCI: Make PCI IRQ link driver use struct acpi_scan_handler
00c43b9 ACPI / PCI: Make PCI root driver use struct acpi_scan_handler
ca589f9 ACPI / scan: Introduce struct acpi_scan_handler
3b2ab2b Revert "usb: Register usb port's acpi power resources"
8b4e2fa Merge branch 'acpi-lpss' into acpi-cleanup
64e94e7 Merge branch 'acpi-scan' into acpi-cleanup
2c0d4fe ACPI / scan: Make scanning of fixed devices follow the general scheme
8a78cf7 Merge branch 'acpi-pm' into acpi-cleanup
0d1c28a gpiolib-acpi: Add ACPI5 event model support to gpio.
65ab96f ACPI / PM: Fix /proc/acpi/wakeup for devices w/o bus or parent
c511cc1 ACPI / scan: Make namespace scanning and trimming mutually exclusive
fb45579 Merge branch 'pci/acpi-scan2' into next
09212fd ACPI: Drop device start operation that is not used
51fac83 ACPI: Remove useless type argument of driver .remove() operation
cc38e51 Merge branch 'acpi-scan' into acpi-cleanup
5baa1be ACPI: fix obsolete comment in custom_method.c
ca4e713 ACPI / thermal: Use mode to enable/disable kernel thermal processing
7e3cf24 ACPI thermal: remove unnecessary newline from exception message
c628423 ACPI sysfs: remove unnecessary newline from exception
bf6787e ACPI video: remove unnecessary newline from error messages
208f6cc ACPI: SRAT: report non-volatile memory in debug
33f767d ACPI: Rework acpi_get_child() to be more efficient
bfee26d ACPI / scan: Make it clear that acpi_bus_trim() cannot fail
d59f53b PCI: acpiphp: Keep driver loaded even if no slots found
121b090 PCI/ACPI: Print info if host bridge notify handler installation fails
668192b PCI: acpiphp: Move host bridge hotplug to pci_root.c
92d8aff PCI/ACPI: acpiphp: Rename alloc_acpiphp_hp_work() to alloc_acpi_hp_work()
660b111 ACPI / PM: Fix consistency check for power resources during resume
18a3870 ACPI / PM: Expose lists of device power resources to user space
1f96a96 PCI: acpiphp: Add is_hotplug_bridge detection
a276660 Merge branch 'acpi-scan' of git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm into pci/acpi-scan2
88bb965 usb: Register usb port's acpi power resources
2171953 ACPICA: Update version to 20130117
378bb8b ACPICA: Update predefined info table for _MLS method
5e30a96 ACPICA: Remove some extraneous newlines in ACPI_ERROR type calls
d9652b4 ACPICA: iASL/Disassembler: Add option to ignore NOOP opcodes/operators
48ffb94 ACPICA: AcpiGetSleepTypeData: Allow \_Sx to return either 1 or 2 integers
25f044e ACPICA: Update ACPICA copyrights to 2013
586ce51 ACPICA: Update predefined info table
afd51a0 x86/acpi: Use __pa_symbol instead of __pa on C visible symbols

Regards,
Peter Hurley


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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-10 14:23     ` Peter Hurley
@ 2013-02-10 20:33       ` Yinghai Lu
  2013-02-11  6:40         ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-10 20:33 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next

On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>> >
>> > > With -next-20130204:
>> > >
>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>> > > [   33.855582] Call Trace:
>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>> > > [   33.855660] handlers:
>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>> > > [   33.855667] Disabling IRQ #18
>> > >
>> > > From earlier in the boot log:
>> > > [    1.297020] uhci_hcd 0000:00:1d.2: setting latency timer to 64
>> > > [    1.297032] uhci_hcd 0000:00:1d.2: UHCI Host Controller
>> > > [    1.297040] uhci_hcd 0000:00:1d.2: new USB bus registered, assigned bus number 4
>> > > [    1.297076] uhci_hcd 0000:00:1d.2: irq 18, io base 0x0000ff40
>> > > [    1.297213] hub 4-0:1.0: USB hub found
>> > > [    1.297221] hub 4-0:1.0: 2 ports detected
>> > >
>> > > lsusb:
>> > > ...
>> > > Bus 004 Device 002: ID 0764:0501 Cyber Power System, Inc. CP1500 AVR UPS
>> > > Bus 004 Device 003: ID 046d:c52b Logitech, Inc. Unifying Receiver
>>
>> [...]
>>
>> > If the suggestion above doesn't work out, bisection may be the best way
>> > to find the answer.
>>
>> This bad irq bisected to:
>>
>> 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4 is the first bad commit
>> commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>> Author: Yinghai Lu <yinghai@kernel.org>
>> Date:   Mon Jan 21 13:20:52 2013 -0800
>>
>>     PCI: Put pci_dev in device tree as early as possible
>>
>>     We want to put pci_dev structs in the device tree as soon as possible so
>>     for_each_pci_dev() iteration will not miss them, but driver attachment
>>     needs to be delayed until after pci_assign_unassigned_resources() to make
>>     sure all devices have resources assigned first.
>>
>>     This patch moves device registering from pci_bus_add_devices() to
>>     pci_device_add(), which happens earlier, leaving driver attachment in
>>     pci_bus_add_devices().
>>
>>     It also removes unattached child bus handling in pci_bus_add_devices().
>>     That's not needed because child bus via pci_add_new_bus() is already
>>     in parent bus children list.
>>
>>     [bhelgaas: changelog]
>>     Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>     Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>>     Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>> :040000 040000 0540c98d04d00de24b4e12fa750f6cd26c5addd2 2e24946cb7165a4028b7efb0a622271cc3990005 M    drivers
>>
>> All is fine if I revert these 2 commits:
>>
>> 40064ac PCI: Remove unused "rc" in virtfn_add_bus()
>> 4f53509 (HEAD, refs/bisect/bad) PCI: Put pci_dev in device tree as early as possible
>>
>> Any ideas how these are causing the USB HCD core / hid-logitech-dj
>> driver to drop interrupts?
>
> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>
> Maybe this is some interaction with all the new ACPI code and fixes
> written in those 8 days.

interrupt routing seems get changed:
next:
   5:          0          0          0          0          0
0          0          0   IO-APIC-fasteoi   snd_ctxfi
  18:      99970         13         16         20      99940
13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
v3.8-rc7:
  18:        424         15         11        112        420
16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi

These messages in the bad dmesg log are interesting since PCI INT A is routed
on
irq 18 with the kernels that work.
[    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
[    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
...

acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
that bridge is not scanned.

Will check if I can produce one patch for it.

Thanks

Yinghai

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-10 20:33       ` Yinghai Lu
@ 2013-02-11  6:40         ` Yinghai Lu
  2013-02-11 10:25           ` Jiri Slaby
  2013-02-11 13:02           ` Peter Hurley
  0 siblings, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2013-02-11  6:40 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next

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

On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>>> >
>>> > > With -next-20130204:
>>> > >
>>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>>> > > [   33.855582] Call Trace:
>>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>>> > > [   33.855660] handlers:
>>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>>> > > [   33.855667] Disabling IRQ #18
>>> > >
>> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>>
>> Maybe this is some interaction with all the new ACPI code and fixes
>> written in those 8 days.
>
> interrupt routing seems get changed:
> next:
>    5:          0          0          0          0          0
> 0          0          0   IO-APIC-fasteoi   snd_ctxfi
>   18:      99970         13         16         20      99940
> 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
> v3.8-rc7:
>   18:        424         15         11        112        420
> 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
>
> These messages in the bad dmesg log are interesting since PCI INT A is routed
> on
> irq 18 with the kernels that work.
> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
> ...
>
> acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
> that bridge is not scanned.
>
> Will check if I can produce one patch for it.

Hi Peter,

Can you try attached debug patch?

Thanks

Yinghai

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

diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
index 8647dc6..e901902 100644
--- a/drivers/pci/bus.c
+++ b/drivers/pci/bus.c
@@ -160,6 +160,8 @@ pci_bus_alloc_resource(struct pci_bus *bus, struct resource *res,
 
 void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
 
+void __weak pcibios_irq_setup(struct device *dev) { }
+
 /**
  * pci_bus_add_device - start driver for a single device
  * @dev: device to add
@@ -170,6 +172,9 @@ int pci_bus_add_device(struct pci_dev *dev)
 {
 	int retval;
 
+	/* need to after bridge is scanned */
+	pcibios_irq_setup(&dev->dev);
+
 	/*
 	 * Can not put in pci_device_add yet because resources
 	 * are not assigned yet for some devices.
diff --git a/drivers/pci/pci-acpi.c b/drivers/pci/pci-acpi.c
index ba4545f..87bd80b 100644
--- a/drivers/pci/pci-acpi.c
+++ b/drivers/pci/pci-acpi.c
@@ -302,14 +302,16 @@ static int acpi_pci_find_device(struct device *dev, acpi_handle *handle)
 	return 0;
 }
 
-static void pci_acpi_setup(struct device *dev)
+void pcibios_irq_setup(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
-	struct acpi_device *adev;
 	acpi_status status;
 	acpi_handle dummy;
 
+	if (acpi_disabled || acpi_pci_disabled)
+		return;
+
 	/*
 	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
 	 * _PRT objects within the scope of non-bridge devices.  Note that
@@ -326,6 +328,13 @@ static void pci_acpi_setup(struct device *dev)
 			pci_dev->subordinate->number : pci_dev->bus->number;
 		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
 	}
+}
+
+static void pci_acpi_setup(struct device *dev)
+{
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
+	struct acpi_device *adev;
 
 	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
 		return;

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11  6:40         ` Yinghai Lu
@ 2013-02-11 10:25           ` Jiri Slaby
  2013-02-11 13:02           ` Peter Hurley
  1 sibling, 0 replies; 27+ messages in thread
From: Jiri Slaby @ 2013-02-11 10:25 UTC (permalink / raw)
  To: Yinghai Lu, Peter Hurley
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next

On 02/11/2013 07:40 AM, Yinghai Lu wrote:
> Can you try attached debug patch?

FWIW this fixes it for me. No more 'derived' or 'nobody' in dmesg.

thanks,
-- 
js
suse labs

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11  6:40         ` Yinghai Lu
  2013-02-11 10:25           ` Jiri Slaby
@ 2013-02-11 13:02           ` Peter Hurley
  2013-02-11 19:19             ` Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-02-11 13:02 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next

On Sun, 2013-02-10 at 22:40 -0800, Yinghai Lu wrote:
> On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> > On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> >> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
> >>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
> >>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
> >>> >
> >>> > > With -next-20130204:
> >>> > >
> >>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
> >>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
> >>> > > [   33.855582] Call Trace:
> >>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
> >>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
> >>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> >>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
> >>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
> >>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
> >>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
> >>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
> >>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
> >>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
> >>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
> >>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
> >>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
> >>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
> >>> > > [   33.855660] handlers:
> >>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
> >>> > > [   33.855667] Disabling IRQ #18
> >>> > >
> >> https://bugzilla.kernel.org/show_bug.cgi?id=53561
> >>
> >> Maybe this is some interaction with all the new ACPI code and fixes
> >> written in those 8 days.
> >
> > interrupt routing seems get changed:
> > next:
> >    5:          0          0          0          0          0
> > 0          0          0   IO-APIC-fasteoi   snd_ctxfi
> >   18:      99970         13         16         20      99940
> > 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
> > v3.8-rc7:
> >   18:        424         15         11        112        420
> > 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
> >
> > These messages in the bad dmesg log are interesting since PCI INT A is routed
> > on
> > irq 18 with the kernels that work.
> > [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
> > [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
> > ...
> >
> > acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
> > that bridge is not scanned.
> >
> > Will check if I can produce one patch for it.
> 
> Hi Peter,
> 
> Can you try attached debug patch?

Fixed, thanks.

Regards,
Peter Hurley



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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 13:02           ` Peter Hurley
@ 2013-02-11 19:19             ` Yinghai Lu
  2013-02-11 19:45               ` Sedat Dilek
  2013-02-11 19:57               ` Yinghai Lu
  0 siblings, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2013-02-11 19:19 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Peter Hurley, Alan Stern, Lan Tianyu, Greg Kroah-Hartman,
	Jiri Kosina, linux-usb, linux-kernel, linux-next

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

On Mon, Feb 11, 2013 at 5:02 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On Sun, 2013-02-10 at 22:40 -0800, Yinghai Lu wrote:
>> On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> > On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> >> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>> >>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>> >>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>> >>> >
>> >>> > > With -next-20130204:
>> >>> > >
>> >>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>> >>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>> >>> > > [   33.855582] Call Trace:
>> >>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>> >>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>> >>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>> >>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>> >>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>> >>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>> >>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>> >>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>> >>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>> >>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>> >>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>> >>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>> >>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>> >>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>> >>> > > [   33.855660] handlers:
>> >>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>> >>> > > [   33.855667] Disabling IRQ #18
>> >>> > >
>> >> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>> >>
>> >> Maybe this is some interaction with all the new ACPI code and fixes
>> >> written in those 8 days.
>> >
>> > interrupt routing seems get changed:
>> > next:
>> >    5:          0          0          0          0          0
>> > 0          0          0   IO-APIC-fasteoi   snd_ctxfi
>> >   18:      99970         13         16         20      99940
>> > 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
>> > v3.8-rc7:
>> >   18:        424         15         11        112        420
>> > 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
>> >
>> > These messages in the bad dmesg log are interesting since PCI INT A is routed
>> > on
>> > irq 18 with the kernels that work.
>> > [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>> > [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>> > ...
>> >
>> > acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
>> > that bridge is not scanned.
>> >
>> > Will check if I can produce one patch for it.
>>
>> Hi Peter,
>>
>> Can you try attached debug patch?
>
> Fixed, thanks.

Bjorn, Rafael,

acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
so we can not call it from pci_acpi_setup, after we move dev_register
for pci_dev early.

The attached debug patch move down that calling into
pci_bus_add_devices and that will make prt works again.

Can acpi provide another hook after bridge get scanned?

Thanks

Yinghai

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

---
 drivers/pci/bus.c      |    5 +++++
 drivers/pci/pci-acpi.c |   13 +++++++++++--
 2 files changed, 16 insertions(+), 2 deletions(-)

Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -160,6 +160,8 @@ pci_bus_alloc_resource(struct pci_bus *b
 
 void __weak pcibios_resource_survey_bus(struct pci_bus *bus) { }
 
+void __weak pcibios_irq_setup(struct device *dev) { }
+
 /**
  * pci_bus_add_device - start driver for a single device
  * @dev: device to add
@@ -170,6 +172,9 @@ int pci_bus_add_device(struct pci_dev *d
 {
 	int retval;
 
+	/* need to after bridge is scanned */
+	pcibios_irq_setup(&dev->dev);
+
 	/*
 	 * Can not put in pci_device_add yet because resources
 	 * are not assigned yet for some devices.
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -302,14 +302,16 @@ static int acpi_pci_find_device(struct d
 	return 0;
 }
 
-static void pci_acpi_setup(struct device *dev)
+void pcibios_irq_setup(struct device *dev)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
-	struct acpi_device *adev;
 	acpi_status status;
 	acpi_handle dummy;
 
+	if (acpi_disabled || acpi_pci_disabled)
+		return;
+
 	/*
 	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
 	 * _PRT objects within the scope of non-bridge devices.  Note that
@@ -326,6 +328,13 @@ static void pci_acpi_setup(struct device
 			pci_dev->subordinate->number : pci_dev->bus->number;
 		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
 	}
+}
+
+static void pci_acpi_setup(struct device *dev)
+{
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
+	struct acpi_device *adev;
 
 	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
 		return;

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 19:19             ` Yinghai Lu
@ 2013-02-11 19:45               ` Sedat Dilek
  2013-02-11 19:53                 ` Yinghai Lu
  2013-02-11 19:57               ` Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Sedat Dilek @ 2013-02-11 19:45 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Peter Hurley, Alan Stern,
	Lan Tianyu, Greg Kroah-Hartman, Jiri Kosina, linux-usb,
	linux-kernel, linux-next

On Mon, Feb 11, 2013 at 8:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Feb 11, 2013 at 5:02 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On Sun, 2013-02-10 at 22:40 -0800, Yinghai Lu wrote:
>>> On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> > On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> >> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>>> >>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>>> >>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>>> >>> >
>>> >>> > > With -next-20130204:
>>> >>> > >
>>> >>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>>> >>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>>> >>> > > [   33.855582] Call Trace:
>>> >>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>>> >>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>>> >>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> >>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>>> >>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>>> >>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>>> >>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>>> >>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>>> >>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>>> >>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>>> >>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> >>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>>> >>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>>> >>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>>> >>> > > [   33.855660] handlers:
>>> >>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>>> >>> > > [   33.855667] Disabling IRQ #18
>>> >>> > >
>>> >> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>>> >>
>>> >> Maybe this is some interaction with all the new ACPI code and fixes
>>> >> written in those 8 days.
>>> >
>>> > interrupt routing seems get changed:
>>> > next:
>>> >    5:          0          0          0          0          0
>>> > 0          0          0   IO-APIC-fasteoi   snd_ctxfi
>>> >   18:      99970         13         16         20      99940
>>> > 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
>>> > v3.8-rc7:
>>> >   18:        424         15         11        112        420
>>> > 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
>>> >
>>> > These messages in the bad dmesg log are interesting since PCI INT A is routed
>>> > on
>>> > irq 18 with the kernels that work.
>>> > [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>> > [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>> > ...
>>> >
>>> > acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
>>> > that bridge is not scanned.
>>> >
>>> > Will check if I can produce one patch for it.
>>>
>>> Hi Peter,
>>>
>>> Can you try attached debug patch?
>>
>> Fixed, thanks.
>
> Bjorn, Rafael,
>
> acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
> so we can not call it from pci_acpi_setup, after we move dev_register
> for pci_dev early.
>
> The attached debug patch move down that calling into
> pci_bus_add_devices and that will make prt works again.
>
> Can acpi provide another hook after bridge get scanned?
>

23: +	/* need to after bridge is scanned */
24: +	pcibios_irq_setup(&dev->dev);

... need to *be called* after...

Even this is a temporary workaround, can you send a separate patch for this?

- Sedat -

P.S.: Still wonder why people use "linux-2.6" as prefix in their patches.

> Thanks
>
> Yinghai

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 19:45               ` Sedat Dilek
@ 2013-02-11 19:53                 ` Yinghai Lu
  0 siblings, 0 replies; 27+ messages in thread
From: Yinghai Lu @ 2013-02-11 19:53 UTC (permalink / raw)
  To: sedat.dilek
  Cc: Bjorn Helgaas, Rafael J. Wysocki, Peter Hurley, Alan Stern,
	Lan Tianyu, Greg Kroah-Hartman, Jiri Kosina, linux-usb,
	linux-kernel, linux-next

On Mon, Feb 11, 2013 at 11:45 AM, Sedat Dilek <sedat.dilek@gmail.com> wrote:
> On Mon, Feb 11, 2013 at 8:19 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Mon, Feb 11, 2013 at 5:02 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> On Sun, 2013-02-10 at 22:40 -0800, Yinghai Lu wrote:
>>>> On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> > On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>>> >> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>>>> >>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>>>> >>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>>>> >>> >
>>>> >>> > > With -next-20130204:
>>>> >>> > >
>>>> >>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>>>> >>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>>>> >>> > > [   33.855582] Call Trace:
>>>> >>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>>>> >>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>>>> >>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>>> >>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>>>> >>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>>>> >>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>>>> >>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>>>> >>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>>>> >>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>>>> >>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>>>> >>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>>> >>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>>>> >>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>>>> >>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>>>> >>> > > [   33.855660] handlers:
>>>> >>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>>>> >>> > > [   33.855667] Disabling IRQ #18
>>>> >>> > >
>>>> >> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>>>> >>
>>>> >> Maybe this is some interaction with all the new ACPI code and fixes
>>>> >> written in those 8 days.
>>>> >
>>>> > interrupt routing seems get changed:
>>>> > next:
>>>> >    5:          0          0          0          0          0
>>>> > 0          0          0   IO-APIC-fasteoi   snd_ctxfi
>>>> >   18:      99970         13         16         20      99940
>>>> > 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
>>>> > v3.8-rc7:
>>>> >   18:        424         15         11        112        420
>>>> > 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
>>>> >
>>>> > These messages in the bad dmesg log are interesting since PCI INT A is routed
>>>> > on
>>>> > irq 18 with the kernels that work.
>>>> > [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>>> > [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>>> > ...
>>>> >
>>>> > acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
>>>> > that bridge is not scanned.
>>>> >
>>>> > Will check if I can produce one patch for it.
>>>>
>>>> Hi Peter,
>>>>
>>>> Can you try attached debug patch?
>>>
>>> Fixed, thanks.
>>
>> Bjorn, Rafael,
>>
>> acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
>> so we can not call it from pci_acpi_setup, after we move dev_register
>> for pci_dev early.
>>
>> The attached debug patch move down that calling into
>> pci_bus_add_devices and that will make prt works again.
>>
>> Can acpi provide another hook after bridge get scanned?
>>
>
> 23: +   /* need to after bridge is scanned */
> 24: +   pcibios_irq_setup(&dev->dev);
>
> ... need to *be called* after...
>
> Even this is a temporary workaround, can you send a separate patch for this?

again, this is debug patch.

Thanks

Yinghai

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 19:19             ` Yinghai Lu
  2013-02-11 19:45               ` Sedat Dilek
@ 2013-02-11 19:57               ` Yinghai Lu
  2013-02-11 21:06                 ` Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-11 19:57 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Peter Hurley, Alan Stern, Lan Tianyu, Greg Kroah-Hartman,
	Jiri Kosina, linux-usb, linux-kernel, linux-next

On Mon, Feb 11, 2013 at 11:19 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Feb 11, 2013 at 5:02 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>> On Sun, 2013-02-10 at 22:40 -0800, Yinghai Lu wrote:
>>> On Sun, Feb 10, 2013 at 12:33 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> > On Sun, Feb 10, 2013 at 6:23 AM, Peter Hurley <peter@hurleysoftware.com> wrote:
>>> >> On Sat, 2013-02-09 at 22:14 -0500, Peter Hurley wrote:
>>> >>> On Tue, 2013-02-05 at 15:26 -0500, Alan Stern wrote:
>>> >>> > On Tue, 5 Feb 2013, Peter Hurley wrote:
>>> >>> >
>>> >>> > > With -next-20130204:
>>> >>> > >
>>> >>> > > [   33.855570] irq 18: nobody cared (try booting with the "irqpoll" option)
>>> >>> > > [   33.855580] Pid: 0, comm: swapper/4 Not tainted 3.8.0-next-20130204-xeon #20130204
>>> >>> > > [   33.855582] Call Trace:
>>> >>> > > [   33.855585]  <IRQ>  [<ffffffff810f1076>] __report_bad_irq+0x36/0xe0
>>> >>> > > [   33.855600]  [<ffffffff810f152a>] note_interrupt+0x1aa/0x200
>>> >>> > > [   33.855606]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> >>> > > [   33.855610]  [<ffffffff810eed89>] handle_irq_event_percpu+0xc9/0x260
>>> >>> > > [   33.855614]  [<ffffffff810eef68>] handle_irq_event+0x48/0x70
>>> >>> > > [   33.855618]  [<ffffffff810f20ba>] handle_fasteoi_irq+0x5a/0x100
>>> >>> > > [   33.855624]  [<ffffffff810182f2>] handle_irq+0x22/0x40
>>> >>> > > [   33.855630]  [<ffffffff816e2a9a>] do_IRQ+0x5a/0xd0
>>> >>> > > [   33.855636]  [<ffffffff816d97ad>] common_interrupt+0x6d/0x6d
>>> >>> > > [   33.855638]  <EOI>  [<ffffffff810f8e2a>] ? rcu_eqs_enter_common+0x4a/0x320
>>> >>> > > [   33.855646]  [<ffffffff8101edf2>] ? mwait_idle+0x82/0x1b0
>>> >>> > > [   33.855649]  [<ffffffff8101ed99>] ? mwait_idle+0x29/0x1b0
>>> >>> > > [   33.855653]  [<ffffffff8101f8a6>] cpu_idle+0x116/0x130
>>> >>> > > [   33.855658]  [<ffffffff816c0c0f>] start_secondary+0x251/0x258
>>> >>> > > [   33.855660] handlers:
>>> >>> > > [   33.855664] [<ffffffff814f98a0>] usb_hcd_irq
>>> >>> > > [   33.855667] Disabling IRQ #18
>>> >>> > >
>>> >> https://bugzilla.kernel.org/show_bug.cgi?id=53561
>>> >>
>>> >> Maybe this is some interaction with all the new ACPI code and fixes
>>> >> written in those 8 days.
>>> >
>>> > interrupt routing seems get changed:
>>> > next:
>>> >    5:          0          0          0          0          0
>>> > 0          0          0   IO-APIC-fasteoi   snd_ctxfi
>>> >   18:      99970         13         16         20      99940
>>> > 13         13         16   IO-APIC-fasteoi   uhci_hcd:usb4
>>> > v3.8-rc7:
>>> >   18:        424         15         11        112        420
>>> > 16         18        105   IO-APIC-fasteoi   uhci_hcd:usb4, snd_ctxfi
>>> >
>>> > These messages in the bad dmesg log are interesting since PCI INT A is routed
>>> > on
>>> > irq 18 with the kernels that work.
>>> > [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>> > [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>> > ...
>>> >
>>> > acpi_pci_irq_add_prt() add wrong bus for that bridge, because that
>>> > that bridge is not scanned.
>>> >
>>> > Will check if I can produce one patch for it.
>>>
>>> Hi Peter,
>>>
>>> Can you try attached debug patch?
>>
>> Fixed, thanks.
>
> Bjorn, Rafael,
>
> acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
> so we can not call it from pci_acpi_setup, after we move dev_register
> for pci_dev early.
>
> The attached debug patch move down that calling into
> pci_bus_add_devices and that will make prt works again.
>
> Can acpi provide another hook after bridge get scanned?
>

Can we extend platform_notify to be
platform_notify(struct device *dev, event) ?

and event could be _ADD, _SCAN, _REMOVE

Thanks

Yinghai

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 19:57               ` Yinghai Lu
@ 2013-02-11 21:06                 ` Yinghai Lu
  2013-02-11 22:41                   ` Bjorn Helgaas
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-11 21:06 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: Peter Hurley, Alan Stern, Lan Tianyu, Greg Kroah-Hartman,
	Jiri Kosina, linux-usb, linux-kernel, linux-next

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

On Mon, Feb 11, 2013 at 11:57 AM, Yinghai Lu <yinghai@kernel.org> wrote:
>>
>> Bjorn, Rafael,
>>
>> acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
>> so we can not call it from pci_acpi_setup, after we move dev_register
>> for pci_dev early.
>>
>> The attached debug patch move down that calling into
>> pci_bus_add_devices and that will make prt works again.
>>
>> Can acpi provide another hook after bridge get scanned?
>>

Rafael, Bjorn,

Can you check attached patch?

Thanks

Yinghai

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

Subject: [PATCH] ACPI, PCI: add acpi_platform_notify_scan()

Peter Hurley found irq nobody cared, and dmesg has

[    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
[    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5

bisect to
| commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
|     PCI: Put pci_dev in device tree as early as possible

It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
are scanned.

Add acpi_platform_notify_scan() to call acpi_pci_irq_add_prt later.

Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/glue.c     |   12 ++++++++++++
 drivers/base/core.c     |    1 +
 drivers/pci/bus.c       |    4 ++++
 drivers/pci/pci-acpi.c  |   27 +++++++++++++++++----------
 include/acpi/acpi_bus.h |    1 +
 include/linux/device.h  |    3 +--
 6 files changed, 36 insertions(+), 12 deletions(-)

Index: linux-2.6/drivers/acpi/glue.c
===================================================================
--- linux-2.6.orig/drivers/acpi/glue.c
+++ linux-2.6/drivers/acpi/glue.c
@@ -312,6 +312,17 @@ static int acpi_platform_notify(struct d
 	return ret;
 }
 
+static int acpi_platform_notify_scan(struct device *dev)
+{
+	struct acpi_bus_type *type;
+
+	type = acpi_get_bus_type(dev->bus);
+	if (type && type->setup_scan)
+		type->setup_scan(dev);
+
+	return 0;
+}
+
 static int acpi_platform_notify_remove(struct device *dev)
 {
 	struct acpi_bus_type *type;
@@ -331,6 +342,7 @@ int __init init_acpi_device_notify(void)
 		return 0;
 	}
 	platform_notify = acpi_platform_notify;
+	platform_notify_scan = acpi_platform_notify_scan;
 	platform_notify_remove = acpi_platform_notify_remove;
 	return 0;
 }
Index: linux-2.6/drivers/base/core.c
===================================================================
--- linux-2.6.orig/drivers/base/core.c
+++ linux-2.6/drivers/base/core.c
@@ -44,6 +44,7 @@ early_param("sysfs.deprecated", sysfs_de
 #endif
 
 int (*platform_notify)(struct device *dev) = NULL;
+int (*platform_notify_scan)(struct device *dev) = NULL;
 int (*platform_notify_remove)(struct device *dev) = NULL;
 static struct kobject *dev_kobj;
 struct kobject *sysfs_dev_char_kobj;
Index: linux-2.6/drivers/pci/bus.c
===================================================================
--- linux-2.6.orig/drivers/pci/bus.c
+++ linux-2.6/drivers/pci/bus.c
@@ -170,6 +170,10 @@ int pci_bus_add_device(struct pci_dev *d
 {
 	int retval;
 
+	/* need to be called after bridge is scanned */
+	if (platform_notify_scan)
+		platform_notify_scan(&dev->dev);
+
 	/*
 	 * Can not put in pci_device_add yet because resources
 	 * are not assigned yet for some devices.
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -307,6 +307,22 @@ static void pci_acpi_setup(struct device
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
+
+	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
+		return;
+
+	device_set_wakeup_capable(dev, true);
+	acpi_pci_sleep_wake(pci_dev, false);
+
+	pci_acpi_add_pm_notifier(adev, pci_dev);
+	if (adev->wakeup.flags.run_wake)
+		device_set_run_wake(dev, true);
+}
+
+static void pci_acpi_setup_scan(struct device *dev)
+{
+	struct pci_dev *pci_dev = to_pci_dev(dev);
+	acpi_handle handle = ACPI_HANDLE(dev);
 	acpi_status status;
 	acpi_handle dummy;
 
@@ -326,16 +342,6 @@ static void pci_acpi_setup(struct device
 			pci_dev->subordinate->number : pci_dev->bus->number;
 		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
 	}
-
-	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
-		return;
-
-	device_set_wakeup_capable(dev, true);
-	acpi_pci_sleep_wake(pci_dev, false);
-
-	pci_acpi_add_pm_notifier(adev, pci_dev);
-	if (adev->wakeup.flags.run_wake)
-		device_set_run_wake(dev, true);
 }
 
 static void pci_acpi_cleanup(struct device *dev)
@@ -359,6 +365,7 @@ static struct acpi_bus_type acpi_pci_bus
 	.bus = &pci_bus_type,
 	.find_device = acpi_pci_find_device,
 	.setup = pci_acpi_setup,
+	.setup_scan = pci_acpi_setup_scan,
 	.cleanup = pci_acpi_cleanup,
 };
 
Index: linux-2.6/include/acpi/acpi_bus.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_bus.h
+++ linux-2.6/include/acpi/acpi_bus.h
@@ -440,6 +440,7 @@ struct acpi_bus_type {
 	/* For bridges, such as PCI root bridge, IDE controller */
 	int (*find_bridge) (struct device *, acpi_handle *);
 	void (*setup)(struct device *);
+	void (*setup_scan)(struct device *);
 	void (*cleanup)(struct device *);
 };
 int register_acpi_bus_type(struct acpi_bus_type *);
Index: linux-2.6/include/linux/device.h
===================================================================
--- linux-2.6.orig/include/linux/device.h
+++ linux-2.6/include/linux/device.h
@@ -897,10 +897,9 @@ extern void device_destroy(struct class
  */
 /* Notify platform of device discovery */
 extern int (*platform_notify)(struct device *dev);
-
+extern int (*platform_notify_scan)(struct device *dev);
 extern int (*platform_notify_remove)(struct device *dev);
 
-
 /*
  * get_device - atomically increment the reference count for the device.
  *

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 21:06                 ` Yinghai Lu
@ 2013-02-11 22:41                   ` Bjorn Helgaas
  2013-02-12  0:08                     ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Bjorn Helgaas @ 2013-02-11 22:41 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Rafael J. Wysocki, Peter Hurley, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next, linux-acpi, linux-pci

[+cc linux-acpi, linux-pci]

On Mon, Feb 11, 2013 at 01:06:27PM -0800, Yinghai Lu wrote:
> On Mon, Feb 11, 2013 at 11:57 AM, Yinghai Lu <yinghai@kernel.org> wrote:
> >>
> >> Bjorn, Rafael,
> >>
> >> acpi_pci_irq_add_prt need to be called after pci bridge get scanned,
> >> so we can not call it from pci_acpi_setup, after we move dev_register
> >> for pci_dev early.
> >>
> >> The attached debug patch move down that calling into
> >> pci_bus_add_devices and that will make prt works again.
> >>
> >> Can acpi provide another hook after bridge get scanned?
> 
> Rafael, Bjorn,
> 
> Can you check attached patch?

[Yinghai's patch included below for ease of review]

> Subject: [PATCH] ACPI, PCI: add acpi_platform_notify_scan()
> 
> Peter Hurley found irq nobody cared, and dmesg has
> 
> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
> 
> bisect to
> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
> |     PCI: Put pci_dev in device tree as early as possible
> 
> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
> are scanned.
> 
> Add acpi_platform_notify_scan() to call acpi_pci_irq_add_prt later.
> 
> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
> 
> ---
>  drivers/acpi/glue.c     |   12 ++++++++++++
>  drivers/base/core.c     |    1 +
>  drivers/pci/bus.c       |    4 ++++
>  drivers/pci/pci-acpi.c  |   27 +++++++++++++++++----------
>  include/acpi/acpi_bus.h |    1 +
>  include/linux/device.h  |    3 +--
>  6 files changed, 36 insertions(+), 12 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/glue.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/glue.c
> +++ linux-2.6/drivers/acpi/glue.c
> @@ -312,6 +312,17 @@ static int acpi_platform_notify(struct d
>  	return ret;
>  }
>  
> +static int acpi_platform_notify_scan(struct device *dev)
> +{
> +	struct acpi_bus_type *type;
> +
> +	type = acpi_get_bus_type(dev->bus);
> +	if (type && type->setup_scan)
> +		type->setup_scan(dev);
> +
> +	return 0;
> +}
> +
>  static int acpi_platform_notify_remove(struct device *dev)
>  {
>  	struct acpi_bus_type *type;
> @@ -331,6 +342,7 @@ int __init init_acpi_device_notify(void)
>  		return 0;
>  	}
>  	platform_notify = acpi_platform_notify;
> +	platform_notify_scan = acpi_platform_notify_scan;
>  	platform_notify_remove = acpi_platform_notify_remove;
>  	return 0;
>  }
> Index: linux-2.6/drivers/base/core.c
> ===================================================================
> --- linux-2.6.orig/drivers/base/core.c
> +++ linux-2.6/drivers/base/core.c
> @@ -44,6 +44,7 @@ early_param("sysfs.deprecated", sysfs_de
>  #endif
>  
>  int (*platform_notify)(struct device *dev) = NULL;
> +int (*platform_notify_scan)(struct device *dev) = NULL;

I don't like the idea of adding another global function pointer just
for this.  That seems like overkill for a small, local, problem.

>  int (*platform_notify_remove)(struct device *dev) = NULL;
>  static struct kobject *dev_kobj;
>  struct kobject *sysfs_dev_char_kobj;
> Index: linux-2.6/drivers/pci/bus.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/bus.c
> +++ linux-2.6/drivers/pci/bus.c
> @@ -170,6 +170,10 @@ int pci_bus_add_device(struct pci_dev *d
>  {
>  	int retval;
>  
> +	/* need to be called after bridge is scanned */
> +	if (platform_notify_scan)
> +		platform_notify_scan(&dev->dev);
> +
>  	/*
>  	 * Can not put in pci_device_add yet because resources
>  	 * are not assigned yet for some devices.
> Index: linux-2.6/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-acpi.c
> +++ linux-2.6/drivers/pci/pci-acpi.c
> @@ -307,6 +307,22 @@ static void pci_acpi_setup(struct device
>  	struct pci_dev *pci_dev = to_pci_dev(dev);
>  	acpi_handle handle = ACPI_HANDLE(dev);
>  	struct acpi_device *adev;
> +
> +	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
> +		return;
> +
> +	device_set_wakeup_capable(dev, true);
> +	acpi_pci_sleep_wake(pci_dev, false);
> +
> +	pci_acpi_add_pm_notifier(adev, pci_dev);
> +	if (adev->wakeup.flags.run_wake)
> +		device_set_run_wake(dev, true);
> +}
> +
> +static void pci_acpi_setup_scan(struct device *dev)
> +{
> +	struct pci_dev *pci_dev = to_pci_dev(dev);
> +	acpi_handle handle = ACPI_HANDLE(dev);
>  	acpi_status status;
>  	acpi_handle dummy;
>  
> @@ -326,16 +342,6 @@ static void pci_acpi_setup(struct device
>  			pci_dev->subordinate->number : pci_dev->bus->number;
>  		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);

The _PRT describes motherboard interrupt wiring, which has nothing to
do with PCI bus numbers.  Our current drivers/acpi/pci_irq.c caches
the PCI bus number along with the _PRT, and I think that's a mistake.

The bus number binding means acpi_pci_irq_add_prt() has to happen
after enumerating everything below a bridge, and it will prevent us
from doing any bus number reassignment for hotplug.

I think we should remove the bus numbers from the cached _PRT (or
maybe even remove the _PRT caching completely).  When we enable a PCI
device's IRQ, we should search up the PCI device tree looking for a
_PRT associated with each node, and applying normal PCI bridge
swizzling when we don't find a _PRT.  I think this can be done without
using PCI bus numbers at all.

>  	}
> -
> -	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
> -		return;
> -
> -	device_set_wakeup_capable(dev, true);
> -	acpi_pci_sleep_wake(pci_dev, false);
> -
> -	pci_acpi_add_pm_notifier(adev, pci_dev);
> -	if (adev->wakeup.flags.run_wake)
> -		device_set_run_wake(dev, true);
>  }
>  
>  static void pci_acpi_cleanup(struct device *dev)
> @@ -359,6 +365,7 @@ static struct acpi_bus_type acpi_pci_bus
>  	.bus = &pci_bus_type,
>  	.find_device = acpi_pci_find_device,
>  	.setup = pci_acpi_setup,
> +	.setup_scan = pci_acpi_setup_scan,
>  	.cleanup = pci_acpi_cleanup,
>  };
>  
> Index: linux-2.6/include/acpi/acpi_bus.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpi_bus.h
> +++ linux-2.6/include/acpi/acpi_bus.h
> @@ -440,6 +440,7 @@ struct acpi_bus_type {
>  	/* For bridges, such as PCI root bridge, IDE controller */
>  	int (*find_bridge) (struct device *, acpi_handle *);
>  	void (*setup)(struct device *);
> +	void (*setup_scan)(struct device *);
>  	void (*cleanup)(struct device *);
>  };
>  int register_acpi_bus_type(struct acpi_bus_type *);
> Index: linux-2.6/include/linux/device.h
> ===================================================================
> --- linux-2.6.orig/include/linux/device.h
> +++ linux-2.6/include/linux/device.h
> @@ -897,10 +897,9 @@ extern void device_destroy(struct class
>   */
>  /* Notify platform of device discovery */
>  extern int (*platform_notify)(struct device *dev);
> -
> +extern int (*platform_notify_scan)(struct device *dev);
>  extern int (*platform_notify_remove)(struct device *dev);
>  
> -
>  /*
>   * get_device - atomically increment the reference count for the device.
>   *

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-11 22:41                   ` Bjorn Helgaas
@ 2013-02-12  0:08                     ` Yinghai Lu
  2013-02-12  3:21                       ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-12  0:08 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Peter Hurley, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next, linux-acpi, linux-pci

On Mon, Feb 11, 2013 at 2:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> [+cc linux-acpi, linux-pci]
>
> The _PRT describes motherboard interrupt wiring, which has nothing to
> do with PCI bus numbers.  Our current drivers/acpi/pci_irq.c caches
> the PCI bus number along with the _PRT, and I think that's a mistake.
>
> The bus number binding means acpi_pci_irq_add_prt() has to happen
> after enumerating everything below a bridge, and it will prevent us
> from doing any bus number reassignment for hotplug.
>
> I think we should remove the bus numbers from the cached _PRT (or
> maybe even remove the _PRT caching completely).  When we enable a PCI
> device's IRQ, we should search up the PCI device tree looking for a
> _PRT associated with each node, and applying normal PCI bridge
> swizzling when we don't find a _PRT.  I think this can be done without
> using PCI bus numbers at all.
>

Agreed, will give it try to remove the _PRT caching completely.

Thanks

Yinghai

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

* Re: [Bisected] [-next-20130204] usb/hcd: irq 18: nobody cared
  2013-02-12  0:08                     ` Yinghai Lu
@ 2013-02-12  3:21                       ` Yinghai Lu
  2013-02-12 19:11                         ` [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq() Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-12  3:21 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Rafael J. Wysocki, Peter Hurley, Alan Stern, Lan Tianyu,
	Greg Kroah-Hartman, Jiri Kosina, linux-usb, linux-kernel,
	linux-next, linux-acpi, linux-pci

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

On Mon, Feb 11, 2013 at 4:08 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Feb 11, 2013 at 2:41 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> [+cc linux-acpi, linux-pci]
>>
>> The _PRT describes motherboard interrupt wiring, which has nothing to
>> do with PCI bus numbers.  Our current drivers/acpi/pci_irq.c caches
>> the PCI bus number along with the _PRT, and I think that's a mistake.
>>
>> The bus number binding means acpi_pci_irq_add_prt() has to happen
>> after enumerating everything below a bridge, and it will prevent us
>> from doing any bus number reassignment for hotplug.
>>
>> I think we should remove the bus numbers from the cached _PRT (or
>> maybe even remove the _PRT caching completely).  When we enable a PCI
>> device's IRQ, we should search up the PCI device tree looking for a
>> _PRT associated with each node, and applying normal PCI bridge
>> swizzling when we don't find a _PRT.  I think this can be done without
>> using PCI bus numbers at all.
>>
>
> Agreed, will give it try to remove the _PRT caching completely.
>

Please check attached patch that removes _PRT caching.

Thanks

Yinghai

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

Subject: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()

Peter Hurley found irq nobody cared, and dmesg has

[    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
[    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5

bisect to
| commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
|     PCI: Put pci_dev in device tree as early as possible

It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
are scanned.

Bjorn said:
 The bus number binding means acpi_pci_irq_add_prt() has to happen
 after enumerating everything below a bridge, and it will prevent us
 from doing any bus number reassignment for hotplug.

 I think we should remove the bus numbers from the cached _PRT (or
 maybe even remove the _PRT caching completely).  When we enable a PCI
 device's IRQ, we should search up the PCI device tree looking for a
 _PRT associated with each node, and applying normal PCI bridge
 swizzling when we don't find a _PRT.  I think this can be done without
 using PCI bus numbers at all.

So here we try to remove _PRT caching completely.

Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/pci_irq.c      |   97 ++++++++++++++++++--------------------------
 drivers/acpi/pci_root.c     |   18 --------
 drivers/pci/pci-acpi.c      |   24 ----------
 include/acpi/acpi_drivers.h |    5 --
 4 files changed, 40 insertions(+), 104 deletions(-)

Index: linux-2.6/drivers/acpi/pci_irq.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_irq.c
+++ linux-2.6/drivers/acpi/pci_irq.c
@@ -53,9 +53,6 @@ struct acpi_prt_entry {
 	u32			index;		/* GSI, or link _CRS index */
 };
 
-static LIST_HEAD(acpi_prt_list);
-static DEFINE_SPINLOCK(acpi_prt_lock);
-
 static inline char pin_name(int pin)
 {
 	return 'A' + pin - 1;
@@ -65,28 +62,6 @@ static inline char pin_name(int pin)
                          PCI IRQ Routing Table (PRT) Support
    -------------------------------------------------------------------------- */
 
-static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
-							  int pin)
-{
-	struct acpi_prt_entry *entry;
-	int segment = pci_domain_nr(dev->bus);
-	int bus = dev->bus->number;
-	int device = PCI_SLOT(dev->devfn);
-
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry(entry, &acpi_prt_list, list) {
-		if ((segment == entry->id.segment)
-		    && (bus == entry->id.bus)
-		    && (device == entry->id.device)
-		    && (pin == entry->pin)) {
-			spin_unlock(&acpi_prt_lock);
-			return entry;
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-	return NULL;
-}
-
 /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
 static const struct dmi_system_id medion_md9580[] = {
 	{
@@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
 	}
 }
 
-static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
-				  struct acpi_pci_routing_table *prt)
+static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
+				  int pin, struct acpi_pci_routing_table *prt,
+				  struct acpi_prt_entry **prt_entry)
 {
+	int segment = pci_domain_nr(dev->bus);
+	int bus = dev->bus->number;
+	int device = PCI_SLOT(dev->devfn);
 	struct acpi_prt_entry *entry;
 
+	if (((prt->address >> 16) & 0xffff) != device ||
+	    prt->pin + 1 != pin)
+		return -ENODEV;
+
 	entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
@@ -237,26 +220,33 @@ static int acpi_pci_irq_add_entry(acpi_h
 			      entry->id.device, pin_name(entry->pin),
 			      prt->source, entry->index));
 
-	spin_lock(&acpi_prt_lock);
-	list_add_tail(&entry->list, &acpi_prt_list);
-	spin_unlock(&acpi_prt_lock);
+	*prt_entry = entry;
 
 	return 0;
 }
 
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
+static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
+			  int pin, struct acpi_prt_entry **entry_ptr)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	struct acpi_pci_routing_table *entry;
+	acpi_handle handle;
+	struct acpi_prt_entry *prt_entry = NULL;
+
+	if (dev->bus->bridge)
+		handle = ACPI_HANDLE(dev->bus->bridge);
+	else
+		return -ENODEV;
 
 	/* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
 	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
-	       (char *) buffer.pointer);
+	dev_printk(KERN_DEBUG, &dev->dev,
+			"ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
+		       (char *) buffer.pointer);
 
 	kfree(buffer.pointer);
 
@@ -273,7 +263,11 @@ int acpi_pci_irq_add_prt(acpi_handle han
 
 	entry = buffer.pointer;
 	while (entry && (entry->length > 0)) {
-		acpi_pci_irq_add_entry(handle, segment, bus, entry);
+		if (!acpi_pci_irq_check_entry(handle, dev, pin,
+						 entry, &prt_entry)) {
+			*entry_ptr = prt_entry;
+			break;
+		}
 		entry = (struct acpi_pci_routing_table *)
 		    ((unsigned long)entry + entry->length);
 	}
@@ -282,23 +276,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
 	return 0;
 }
 
-void acpi_pci_irq_del_prt(int segment, int bus)
-{
-	struct acpi_prt_entry *entry, *tmp;
-
-	printk(KERN_DEBUG
-	       "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
-	       segment, bus);
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
-		if (segment == entry->id.segment && bus == entry->id.bus) {
-			list_del(&entry->list);
-			kfree(entry);
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-}
-
 /* --------------------------------------------------------------------------
                           PCI Interrupt Routing Support
    -------------------------------------------------------------------------- */
@@ -359,12 +336,13 @@ static int acpi_reroute_boot_interrupt(s
 
 static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
 {
-	struct acpi_prt_entry *entry;
+	struct acpi_prt_entry *entry = NULL;
 	struct pci_dev *bridge;
 	u8 bridge_pin, orig_pin = pin;
+	int ret;
 
-	entry = acpi_pci_irq_find_prt_entry(dev, pin);
-	if (entry) {
+	ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
+	if (!ret && entry) {
 #ifdef CONFIG_X86_IO_APIC
 		acpi_reroute_boot_interrupt(dev, entry);
 #endif /* CONFIG_X86_IO_APIC */
@@ -373,7 +351,7 @@ static struct acpi_prt_entry *acpi_pci_i
 		return entry;
 	}
 
-	/* 
+	/*
 	 * Attempt to derive an IRQ for this device from a parent bridge's
 	 * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
 	 */
@@ -393,8 +371,8 @@ static struct acpi_prt_entry *acpi_pci_i
 			pin = bridge_pin;
 		}
 
-		entry = acpi_pci_irq_find_prt_entry(bridge, pin);
-		if (entry) {
+		ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
+		if (!ret && entry) {
 			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 					 "Derived GSI for %s INT %c from %s\n",
 					 pci_name(dev), pin_name(orig_pin),
@@ -470,6 +448,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));
 		}
+
 		return 0;
 	}
 
@@ -477,6 +456,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	if (rc < 0) {
 		dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
 			 pin_name(pin));
+		kfree(entry);
 		return rc;
 	}
 	dev->irq = rc;
@@ -491,6 +471,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 		(triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
 		(polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
 
+	kfree(entry);
 	return 0;
 }
 
@@ -513,6 +494,8 @@ void acpi_pci_irq_disable(struct pci_dev
 	else
 		gsi = entry->index;
 
+	kfree(entry);
+
 	/*
 	 * TBD: It might be worth clearing dev->irq by magic constant
 	 * (e.g. PCI_UNDEFINED_IRQ).
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -413,7 +413,6 @@ static int acpi_pci_root_add(struct acpi
 	acpi_status status;
 	int result;
 	struct acpi_pci_root *root;
-	acpi_handle handle;
 	struct acpi_pci_driver *driver;
 	u32 flags, base_flags;
 	bool is_osc_granted = false;
@@ -468,16 +467,6 @@ static int acpi_pci_root_add(struct acpi
 	       acpi_device_name(device), acpi_device_bid(device),
 	       root->segment, &root->secondary);
 
-	/*
-	 * PCI Routing Table
-	 * -----------------
-	 * Evaluate and parse _PRT, if exists.
-	 */
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		result = acpi_pci_irq_add_prt(device->handle, root->segment,
-					      root->secondary.start);
-
 	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
 
 	/*
@@ -602,7 +591,6 @@ out_del_root:
 	list_del(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
 
-	acpi_pci_irq_del_prt(root->segment, root->secondary.start);
 end:
 	kfree(root);
 	return result;
@@ -610,8 +598,6 @@ end:
 
 static void acpi_pci_root_remove(struct acpi_device *device)
 {
-	acpi_status status;
-	acpi_handle handle;
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
@@ -626,10 +612,6 @@ static void acpi_pci_root_remove(struct
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		acpi_pci_irq_del_prt(root->segment, root->secondary.start);
-
 	pci_remove_root_bus(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -307,25 +307,6 @@ static void pci_acpi_setup(struct device
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
-	acpi_status status;
-	acpi_handle dummy;
-
-	/*
-	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
-	 * _PRT objects within the scope of non-bridge devices.  Note that
-	 * _PRTs within the scope of a PCI bridge assume the bridge's
-	 * subordinate bus number.
-	 *
-	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
-	 */
-	status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
-	if (ACPI_SUCCESS(status)) {
-		unsigned char bus;
-
-		bus = pci_dev->subordinate ?
-			pci_dev->subordinate->number : pci_dev->bus->number;
-		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
-	}
 
 	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
 		return;
@@ -340,7 +321,6 @@ static void pci_acpi_setup(struct device
 
 static void pci_acpi_cleanup(struct device *dev)
 {
-	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
 
@@ -349,10 +329,6 @@ static void pci_acpi_cleanup(struct devi
 		device_set_run_wake(dev, false);
 		pci_acpi_remove_pm_notifier(adev);
 	}
-
-	if (pci_dev->subordinate)
-		acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
-				     pci_dev->subordinate->number);
 }
 
 static struct acpi_bus_type acpi_pci_bus = {
Index: linux-2.6/include/acpi/acpi_drivers.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_drivers.h
+++ linux-2.6/include/acpi/acpi_drivers.h
@@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
 			       int *polarity, char **name);
 int acpi_pci_link_free_irq(acpi_handle handle);
 
-/* ACPI PCI Interrupt Routing (pci_irq.c) */
-
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
-void acpi_pci_irq_del_prt(int segment, int bus);
-
 /* ACPI PCI Device Binding (pci_bind.c) */
 
 struct pci_bus;

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

* [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-12  3:21                       ` Yinghai Lu
@ 2013-02-12 19:11                         ` Yinghai Lu
  2013-02-12 20:22                           ` Rafael J. Wysocki
  2013-02-13  2:20                           ` Peter Hurley
  0 siblings, 2 replies; 27+ messages in thread
From: Yinghai Lu @ 2013-02-12 19:11 UTC (permalink / raw)
  To: Bjorn Helgaas, Rafael J. Wysocki
  Cc: linux-pci, linux-acpi, linux-kernel, Yinghai Lu

Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has

[    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
[    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5

bisect to
| commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
|     PCI: Put pci_dev in device tree as early as possible

It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
are scanned.

Bjorn said:
 The bus number binding means acpi_pci_irq_add_prt() has to happen
 after enumerating everything below a bridge, and it will prevent us
 from doing any bus number reassignment for hotplug.

 I think we should remove the bus numbers from the cached _PRT (or
 maybe even remove the _PRT caching completely).  When we enable a PCI
 device's IRQ, we should search up the PCI device tree looking for a
 _PRT associated with each node, and applying normal PCI bridge
 swizzling when we don't find a _PRT.  I think this can be done without
 using PCI bus numbers at all.

So here we try to remove _PRT caching completely.

-v2: check !handle early.

Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
 drivers/acpi/pci_root.c     |   18 --------
 drivers/pci/pci-acpi.c      |   24 -----------
 include/acpi/acpi_drivers.h |    5 --
 4 files changed, 38 insertions(+), 104 deletions(-)

Index: linux-2.6/drivers/acpi/pci_irq.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_irq.c
+++ linux-2.6/drivers/acpi/pci_irq.c
@@ -53,9 +53,6 @@ struct acpi_prt_entry {
 	u32			index;		/* GSI, or link _CRS index */
 };
 
-static LIST_HEAD(acpi_prt_list);
-static DEFINE_SPINLOCK(acpi_prt_lock);
-
 static inline char pin_name(int pin)
 {
 	return 'A' + pin - 1;
@@ -65,28 +62,6 @@ static inline char pin_name(int pin)
                          PCI IRQ Routing Table (PRT) Support
    -------------------------------------------------------------------------- */
 
-static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
-							  int pin)
-{
-	struct acpi_prt_entry *entry;
-	int segment = pci_domain_nr(dev->bus);
-	int bus = dev->bus->number;
-	int device = PCI_SLOT(dev->devfn);
-
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry(entry, &acpi_prt_list, list) {
-		if ((segment == entry->id.segment)
-		    && (bus == entry->id.bus)
-		    && (device == entry->id.device)
-		    && (pin == entry->pin)) {
-			spin_unlock(&acpi_prt_lock);
-			return entry;
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-	return NULL;
-}
-
 /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
 static const struct dmi_system_id medion_md9580[] = {
 	{
@@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
 	}
 }
 
-static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
-				  struct acpi_pci_routing_table *prt)
+static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
+				  int pin, struct acpi_pci_routing_table *prt,
+				  struct acpi_prt_entry **entry_ptr)
 {
+	int segment = pci_domain_nr(dev->bus);
+	int bus = dev->bus->number;
+	int device = PCI_SLOT(dev->devfn);
 	struct acpi_prt_entry *entry;
 
+	if (((prt->address >> 16) & 0xffff) != device ||
+	    prt->pin + 1 != pin)
+		return -ENODEV;
+
 	entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
@@ -237,26 +220,33 @@ static int acpi_pci_irq_add_entry(acpi_h
 			      entry->id.device, pin_name(entry->pin),
 			      prt->source, entry->index));
 
-	spin_lock(&acpi_prt_lock);
-	list_add_tail(&entry->list, &acpi_prt_list);
-	spin_unlock(&acpi_prt_lock);
+	*entry_ptr = entry;
 
 	return 0;
 }
 
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
+static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
+			  int pin, struct acpi_prt_entry **entry_ptr)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	struct acpi_pci_routing_table *entry;
+	acpi_handle handle = NULL;
+
+	if (dev->bus->bridge)
+		handle = ACPI_HANDLE(dev->bus->bridge);
+
+	if (!handle)
+		return -ENODEV;
 
 	/* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
 	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
 
-	printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
-	       (char *) buffer.pointer);
+	dev_printk(KERN_DEBUG, &dev->dev,
+			"ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
+		       (char *) buffer.pointer);
 
 	kfree(buffer.pointer);
 
@@ -273,7 +263,9 @@ int acpi_pci_irq_add_prt(acpi_handle han
 
 	entry = buffer.pointer;
 	while (entry && (entry->length > 0)) {
-		acpi_pci_irq_add_entry(handle, segment, bus, entry);
+		if (!acpi_pci_irq_check_entry(handle, dev, pin,
+						 entry, entry_ptr))
+			break;
 		entry = (struct acpi_pci_routing_table *)
 		    ((unsigned long)entry + entry->length);
 	}
@@ -282,23 +274,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
 	return 0;
 }
 
-void acpi_pci_irq_del_prt(int segment, int bus)
-{
-	struct acpi_prt_entry *entry, *tmp;
-
-	printk(KERN_DEBUG
-	       "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
-	       segment, bus);
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
-		if (segment == entry->id.segment && bus == entry->id.bus) {
-			list_del(&entry->list);
-			kfree(entry);
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-}
-
 /* --------------------------------------------------------------------------
                           PCI Interrupt Routing Support
    -------------------------------------------------------------------------- */
@@ -359,12 +334,13 @@ static int acpi_reroute_boot_interrupt(s
 
 static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
 {
-	struct acpi_prt_entry *entry;
+	struct acpi_prt_entry *entry = NULL;
 	struct pci_dev *bridge;
 	u8 bridge_pin, orig_pin = pin;
+	int ret;
 
-	entry = acpi_pci_irq_find_prt_entry(dev, pin);
-	if (entry) {
+	ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
+	if (!ret && entry) {
 #ifdef CONFIG_X86_IO_APIC
 		acpi_reroute_boot_interrupt(dev, entry);
 #endif /* CONFIG_X86_IO_APIC */
@@ -373,7 +349,7 @@ static struct acpi_prt_entry *acpi_pci_i
 		return entry;
 	}
 
-	/* 
+	/*
 	 * Attempt to derive an IRQ for this device from a parent bridge's
 	 * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
 	 */
@@ -393,8 +369,8 @@ static struct acpi_prt_entry *acpi_pci_i
 			pin = bridge_pin;
 		}
 
-		entry = acpi_pci_irq_find_prt_entry(bridge, pin);
-		if (entry) {
+		ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
+		if (!ret && entry) {
 			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 					 "Derived GSI for %s INT %c from %s\n",
 					 pci_name(dev), pin_name(orig_pin),
@@ -470,6 +446,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));
 		}
+
 		return 0;
 	}
 
@@ -477,6 +454,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	if (rc < 0) {
 		dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
 			 pin_name(pin));
+		kfree(entry);
 		return rc;
 	}
 	dev->irq = rc;
@@ -491,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 		(triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
 		(polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
 
+	kfree(entry);
 	return 0;
 }
 
@@ -513,6 +492,8 @@ void acpi_pci_irq_disable(struct pci_dev
 	else
 		gsi = entry->index;
 
+	kfree(entry);
+
 	/*
 	 * TBD: It might be worth clearing dev->irq by magic constant
 	 * (e.g. PCI_UNDEFINED_IRQ).
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -413,7 +413,6 @@ static int acpi_pci_root_add(struct acpi
 	acpi_status status;
 	int result;
 	struct acpi_pci_root *root;
-	acpi_handle handle;
 	struct acpi_pci_driver *driver;
 	u32 flags, base_flags;
 	bool is_osc_granted = false;
@@ -468,16 +467,6 @@ static int acpi_pci_root_add(struct acpi
 	       acpi_device_name(device), acpi_device_bid(device),
 	       root->segment, &root->secondary);
 
-	/*
-	 * PCI Routing Table
-	 * -----------------
-	 * Evaluate and parse _PRT, if exists.
-	 */
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		result = acpi_pci_irq_add_prt(device->handle, root->segment,
-					      root->secondary.start);
-
 	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
 
 	/*
@@ -602,7 +591,6 @@ out_del_root:
 	list_del(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
 
-	acpi_pci_irq_del_prt(root->segment, root->secondary.start);
 end:
 	kfree(root);
 	return result;
@@ -610,8 +598,6 @@ end:
 
 static void acpi_pci_root_remove(struct acpi_device *device)
 {
-	acpi_status status;
-	acpi_handle handle;
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
@@ -626,10 +612,6 @@ static void acpi_pci_root_remove(struct
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		acpi_pci_irq_del_prt(root->segment, root->secondary.start);
-
 	pci_remove_root_bus(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -307,25 +307,6 @@ static void pci_acpi_setup(struct device
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
-	acpi_status status;
-	acpi_handle dummy;
-
-	/*
-	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
-	 * _PRT objects within the scope of non-bridge devices.  Note that
-	 * _PRTs within the scope of a PCI bridge assume the bridge's
-	 * subordinate bus number.
-	 *
-	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
-	 */
-	status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
-	if (ACPI_SUCCESS(status)) {
-		unsigned char bus;
-
-		bus = pci_dev->subordinate ?
-			pci_dev->subordinate->number : pci_dev->bus->number;
-		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
-	}
 
 	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
 		return;
@@ -340,7 +321,6 @@ static void pci_acpi_setup(struct device
 
 static void pci_acpi_cleanup(struct device *dev)
 {
-	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
 
@@ -349,10 +329,6 @@ static void pci_acpi_cleanup(struct devi
 		device_set_run_wake(dev, false);
 		pci_acpi_remove_pm_notifier(adev);
 	}
-
-	if (pci_dev->subordinate)
-		acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
-				     pci_dev->subordinate->number);
 }
 
 static struct acpi_bus_type acpi_pci_bus = {
Index: linux-2.6/include/acpi/acpi_drivers.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_drivers.h
+++ linux-2.6/include/acpi/acpi_drivers.h
@@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
 			       int *polarity, char **name);
 int acpi_pci_link_free_irq(acpi_handle handle);
 
-/* ACPI PCI Interrupt Routing (pci_irq.c) */
-
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
-void acpi_pci_irq_del_prt(int segment, int bus);
-
 /* ACPI PCI Device Binding (pci_bind.c) */
 
 struct pci_bus;

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-12 19:11                         ` [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq() Yinghai Lu
@ 2013-02-12 20:22                           ` Rafael J. Wysocki
  2013-02-15  0:50                             ` Yinghai Lu
  2013-02-13  2:20                           ` Peter Hurley
  1 sibling, 1 reply; 27+ messages in thread
From: Rafael J. Wysocki @ 2013-02-12 20:22 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Bjorn Helgaas, linux-pci, linux-acpi, linux-kernel

On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
> 
> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
> 
> bisect to
> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
> |     PCI: Put pci_dev in device tree as early as possible
> 
> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
> are scanned.
> 
> Bjorn said:
>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>  after enumerating everything below a bridge, and it will prevent us
>  from doing any bus number reassignment for hotplug.
> 
>  I think we should remove the bus numbers from the cached _PRT (or
>  maybe even remove the _PRT caching completely).  When we enable a PCI
>  device's IRQ, we should search up the PCI device tree looking for a
>  _PRT associated with each node, and applying normal PCI bridge
>  swizzling when we don't find a _PRT.  I think this can be done without
>  using PCI bus numbers at all.
> 
> So here we try to remove _PRT caching completely.
> 
> -v2: check !handle early.
> 
> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Yinghai Lu <yinghai@kernel.org>

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

> ---
>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>  drivers/acpi/pci_root.c     |   18 --------
>  drivers/pci/pci-acpi.c      |   24 -----------
>  include/acpi/acpi_drivers.h |    5 --
>  4 files changed, 38 insertions(+), 104 deletions(-)
> 
> Index: linux-2.6/drivers/acpi/pci_irq.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/pci_irq.c
> +++ linux-2.6/drivers/acpi/pci_irq.c
> @@ -53,9 +53,6 @@ struct acpi_prt_entry {
>  	u32			index;		/* GSI, or link _CRS index */
>  };
>  
> -static LIST_HEAD(acpi_prt_list);
> -static DEFINE_SPINLOCK(acpi_prt_lock);
> -
>  static inline char pin_name(int pin)
>  {
>  	return 'A' + pin - 1;
> @@ -65,28 +62,6 @@ static inline char pin_name(int pin)
>                           PCI IRQ Routing Table (PRT) Support
>     -------------------------------------------------------------------------- */
>  
> -static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
> -							  int pin)
> -{
> -	struct acpi_prt_entry *entry;
> -	int segment = pci_domain_nr(dev->bus);
> -	int bus = dev->bus->number;
> -	int device = PCI_SLOT(dev->devfn);
> -
> -	spin_lock(&acpi_prt_lock);
> -	list_for_each_entry(entry, &acpi_prt_list, list) {
> -		if ((segment == entry->id.segment)
> -		    && (bus == entry->id.bus)
> -		    && (device == entry->id.device)
> -		    && (pin == entry->pin)) {
> -			spin_unlock(&acpi_prt_lock);
> -			return entry;
> -		}
> -	}
> -	spin_unlock(&acpi_prt_lock);
> -	return NULL;
> -}
> -
>  /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
>  static const struct dmi_system_id medion_md9580[] = {
>  	{
> @@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
>  	}
>  }
>  
> -static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
> -				  struct acpi_pci_routing_table *prt)
> +static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
> +				  int pin, struct acpi_pci_routing_table *prt,
> +				  struct acpi_prt_entry **entry_ptr)
>  {
> +	int segment = pci_domain_nr(dev->bus);
> +	int bus = dev->bus->number;
> +	int device = PCI_SLOT(dev->devfn);
>  	struct acpi_prt_entry *entry;
>  
> +	if (((prt->address >> 16) & 0xffff) != device ||
> +	    prt->pin + 1 != pin)
> +		return -ENODEV;
> +
>  	entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
>  	if (!entry)
>  		return -ENOMEM;
> @@ -237,26 +220,33 @@ static int acpi_pci_irq_add_entry(acpi_h
>  			      entry->id.device, pin_name(entry->pin),
>  			      prt->source, entry->index));
>  
> -	spin_lock(&acpi_prt_lock);
> -	list_add_tail(&entry->list, &acpi_prt_list);
> -	spin_unlock(&acpi_prt_lock);
> +	*entry_ptr = entry;
>  
>  	return 0;
>  }
>  
> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
> +static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
> +			  int pin, struct acpi_prt_entry **entry_ptr)
>  {
>  	acpi_status status;
>  	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>  	struct acpi_pci_routing_table *entry;
> +	acpi_handle handle = NULL;
> +
> +	if (dev->bus->bridge)
> +		handle = ACPI_HANDLE(dev->bus->bridge);
> +
> +	if (!handle)
> +		return -ENODEV;
>  
>  	/* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
>  	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>  	if (ACPI_FAILURE(status))
>  		return -ENODEV;
>  
> -	printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
> -	       (char *) buffer.pointer);
> +	dev_printk(KERN_DEBUG, &dev->dev,
> +			"ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
> +		       (char *) buffer.pointer);
>  
>  	kfree(buffer.pointer);
>  
> @@ -273,7 +263,9 @@ int acpi_pci_irq_add_prt(acpi_handle han
>  
>  	entry = buffer.pointer;
>  	while (entry && (entry->length > 0)) {
> -		acpi_pci_irq_add_entry(handle, segment, bus, entry);
> +		if (!acpi_pci_irq_check_entry(handle, dev, pin,
> +						 entry, entry_ptr))
> +			break;
>  		entry = (struct acpi_pci_routing_table *)
>  		    ((unsigned long)entry + entry->length);
>  	}
> @@ -282,23 +274,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
>  	return 0;
>  }
>  
> -void acpi_pci_irq_del_prt(int segment, int bus)
> -{
> -	struct acpi_prt_entry *entry, *tmp;
> -
> -	printk(KERN_DEBUG
> -	       "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
> -	       segment, bus);
> -	spin_lock(&acpi_prt_lock);
> -	list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
> -		if (segment == entry->id.segment && bus == entry->id.bus) {
> -			list_del(&entry->list);
> -			kfree(entry);
> -		}
> -	}
> -	spin_unlock(&acpi_prt_lock);
> -}
> -
>  /* --------------------------------------------------------------------------
>                            PCI Interrupt Routing Support
>     -------------------------------------------------------------------------- */
> @@ -359,12 +334,13 @@ static int acpi_reroute_boot_interrupt(s
>  
>  static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
>  {
> -	struct acpi_prt_entry *entry;
> +	struct acpi_prt_entry *entry = NULL;
>  	struct pci_dev *bridge;
>  	u8 bridge_pin, orig_pin = pin;
> +	int ret;
>  
> -	entry = acpi_pci_irq_find_prt_entry(dev, pin);
> -	if (entry) {
> +	ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
> +	if (!ret && entry) {
>  #ifdef CONFIG_X86_IO_APIC
>  		acpi_reroute_boot_interrupt(dev, entry);
>  #endif /* CONFIG_X86_IO_APIC */
> @@ -373,7 +349,7 @@ static struct acpi_prt_entry *acpi_pci_i
>  		return entry;
>  	}
>  
> -	/* 
> +	/*
>  	 * Attempt to derive an IRQ for this device from a parent bridge's
>  	 * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
>  	 */
> @@ -393,8 +369,8 @@ static struct acpi_prt_entry *acpi_pci_i
>  			pin = bridge_pin;
>  		}
>  
> -		entry = acpi_pci_irq_find_prt_entry(bridge, pin);
> -		if (entry) {
> +		ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
> +		if (!ret && entry) {
>  			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>  					 "Derived GSI for %s INT %c from %s\n",
>  					 pci_name(dev), pin_name(orig_pin),
> @@ -470,6 +446,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>  			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
>  				 pin_name(pin));
>  		}
> +
>  		return 0;
>  	}
>  
> @@ -477,6 +454,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>  	if (rc < 0) {
>  		dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
>  			 pin_name(pin));
> +		kfree(entry);
>  		return rc;
>  	}
>  	dev->irq = rc;
> @@ -491,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>  		(triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
>  		(polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
>  
> +	kfree(entry);
>  	return 0;
>  }
>  
> @@ -513,6 +492,8 @@ void acpi_pci_irq_disable(struct pci_dev
>  	else
>  		gsi = entry->index;
>  
> +	kfree(entry);
> +
>  	/*
>  	 * TBD: It might be worth clearing dev->irq by magic constant
>  	 * (e.g. PCI_UNDEFINED_IRQ).
> Index: linux-2.6/drivers/acpi/pci_root.c
> ===================================================================
> --- linux-2.6.orig/drivers/acpi/pci_root.c
> +++ linux-2.6/drivers/acpi/pci_root.c
> @@ -413,7 +413,6 @@ static int acpi_pci_root_add(struct acpi
>  	acpi_status status;
>  	int result;
>  	struct acpi_pci_root *root;
> -	acpi_handle handle;
>  	struct acpi_pci_driver *driver;
>  	u32 flags, base_flags;
>  	bool is_osc_granted = false;
> @@ -468,16 +467,6 @@ static int acpi_pci_root_add(struct acpi
>  	       acpi_device_name(device), acpi_device_bid(device),
>  	       root->segment, &root->secondary);
>  
> -	/*
> -	 * PCI Routing Table
> -	 * -----------------
> -	 * Evaluate and parse _PRT, if exists.
> -	 */
> -	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> -	if (ACPI_SUCCESS(status))
> -		result = acpi_pci_irq_add_prt(device->handle, root->segment,
> -					      root->secondary.start);
> -
>  	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
>  
>  	/*
> @@ -602,7 +591,6 @@ out_del_root:
>  	list_del(&root->node);
>  	mutex_unlock(&acpi_pci_root_lock);
>  
> -	acpi_pci_irq_del_prt(root->segment, root->secondary.start);
>  end:
>  	kfree(root);
>  	return result;
> @@ -610,8 +598,6 @@ end:
>  
>  static void acpi_pci_root_remove(struct acpi_device *device)
>  {
> -	acpi_status status;
> -	acpi_handle handle;
>  	struct acpi_pci_root *root = acpi_driver_data(device);
>  	struct acpi_pci_driver *driver;
>  
> @@ -626,10 +612,6 @@ static void acpi_pci_root_remove(struct
>  	device_set_run_wake(root->bus->bridge, false);
>  	pci_acpi_remove_bus_pm_notifier(device);
>  
> -	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
> -	if (ACPI_SUCCESS(status))
> -		acpi_pci_irq_del_prt(root->segment, root->secondary.start);
> -
>  	pci_remove_root_bus(root->bus);
>  
>  	mutex_lock(&acpi_pci_root_lock);
> Index: linux-2.6/drivers/pci/pci-acpi.c
> ===================================================================
> --- linux-2.6.orig/drivers/pci/pci-acpi.c
> +++ linux-2.6/drivers/pci/pci-acpi.c
> @@ -307,25 +307,6 @@ static void pci_acpi_setup(struct device
>  	struct pci_dev *pci_dev = to_pci_dev(dev);
>  	acpi_handle handle = ACPI_HANDLE(dev);
>  	struct acpi_device *adev;
> -	acpi_status status;
> -	acpi_handle dummy;
> -
> -	/*
> -	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
> -	 * _PRT objects within the scope of non-bridge devices.  Note that
> -	 * _PRTs within the scope of a PCI bridge assume the bridge's
> -	 * subordinate bus number.
> -	 *
> -	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
> -	 */
> -	status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
> -	if (ACPI_SUCCESS(status)) {
> -		unsigned char bus;
> -
> -		bus = pci_dev->subordinate ?
> -			pci_dev->subordinate->number : pci_dev->bus->number;
> -		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
> -	}
>  
>  	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
>  		return;
> @@ -340,7 +321,6 @@ static void pci_acpi_setup(struct device
>  
>  static void pci_acpi_cleanup(struct device *dev)
>  {
> -	struct pci_dev *pci_dev = to_pci_dev(dev);
>  	acpi_handle handle = ACPI_HANDLE(dev);
>  	struct acpi_device *adev;
>  
> @@ -349,10 +329,6 @@ static void pci_acpi_cleanup(struct devi
>  		device_set_run_wake(dev, false);
>  		pci_acpi_remove_pm_notifier(adev);
>  	}
> -
> -	if (pci_dev->subordinate)
> -		acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
> -				     pci_dev->subordinate->number);
>  }
>  
>  static struct acpi_bus_type acpi_pci_bus = {
> Index: linux-2.6/include/acpi/acpi_drivers.h
> ===================================================================
> --- linux-2.6.orig/include/acpi/acpi_drivers.h
> +++ linux-2.6/include/acpi/acpi_drivers.h
> @@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
>  			       int *polarity, char **name);
>  int acpi_pci_link_free_irq(acpi_handle handle);
>  
> -/* ACPI PCI Interrupt Routing (pci_irq.c) */
> -
> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
> -void acpi_pci_irq_del_prt(int segment, int bus);
> -
>  /* ACPI PCI Device Binding (pci_bind.c) */
>  
>  struct pci_bus;
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-12 19:11                         ` [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq() Yinghai Lu
  2013-02-12 20:22                           ` Rafael J. Wysocki
@ 2013-02-13  2:20                           ` Peter Hurley
  2013-02-13  2:42                             ` Yinghai Lu
  1 sibling, 1 reply; 27+ messages in thread
From: Peter Hurley @ 2013-02-13  2:20 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Tue, 2013-02-12 at 11:11 -0800, Yinghai Lu wrote:
> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
> 
> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
> 
> bisect to
> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
> |     PCI: Put pci_dev in device tree as early as possible
> 
> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
> are scanned.
> 
> Bjorn said:
>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>  after enumerating everything below a bridge, and it will prevent us
>  from doing any bus number reassignment for hotplug.
> 
>  I think we should remove the bus numbers from the cached _PRT (or
>  maybe even remove the _PRT caching completely).  When we enable a PCI
>  device's IRQ, we should search up the PCI device tree looking for a
>  _PRT associated with each node, and applying normal PCI bridge
>  swizzling when we don't find a _PRT.  I think this can be done without
>  using PCI bus numbers at all.
> 
> So here we try to remove _PRT caching completely.
> 
> -v2: check !handle early.
> 
> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>

Actually true now :)

Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>

Regards and thanks again,
Peter Hurley

PS - I just happened to see this version on the list. Would you please
cc me on any future versions?



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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-13  2:20                           ` Peter Hurley
@ 2013-02-13  2:42                             ` Yinghai Lu
  2013-02-13  3:18                               ` Peter Hurley
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-13  2:42 UTC (permalink / raw)
  To: Peter Hurley
  Cc: Bjorn Helgaas, Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Tue, Feb 12, 2013 at 6:20 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> On Tue, 2013-02-12 at 11:11 -0800, Yinghai Lu wrote:
>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>
>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>
>> bisect to
>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>> |     PCI: Put pci_dev in device tree as early as possible
>>
>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>> are scanned.
>>
>> Bjorn said:
>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>  after enumerating everything below a bridge, and it will prevent us
>>  from doing any bus number reassignment for hotplug.
>>
>>  I think we should remove the bus numbers from the cached _PRT (or
>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>  device's IRQ, we should search up the PCI device tree looking for a
>>  _PRT associated with each node, and applying normal PCI bridge
>>  swizzling when we don't find a _PRT.  I think this can be done without
>>  using PCI bus numbers at all.
>>
>> So here we try to remove _PRT caching completely.
>>
>> -v2: check !handle early.
>>
>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>
> Actually true now :)
>
> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>

Thanks.

> PS - I just happened to see this version on the list. Would you please
> cc me on any future versions?

oh, my fault. I take it for granted that "git send-email" will pick
that email from
patch just like Cc.

Yinghai

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-13  2:42                             ` Yinghai Lu
@ 2013-02-13  3:18                               ` Peter Hurley
  0 siblings, 0 replies; 27+ messages in thread
From: Peter Hurley @ 2013-02-13  3:18 UTC (permalink / raw)
  To: Yinghai Lu
  Cc: Bjorn Helgaas, Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Tue, 2013-02-12 at 18:42 -0800, Yinghai Lu wrote:
> On Tue, Feb 12, 2013 at 6:20 PM, Peter Hurley <peter@hurleysoftware.com> wrote:
> >
> > Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
> 
> Thanks.
> 
> > PS - I just happened to see this version on the list. Would you please
> > cc me on any future versions?
> 
> oh, my fault. I take it for granted that "git send-email" will pick
> that email from
> patch just like Cc.

No problem. Actually, I'm sorry it took this long for me to test. I was
stuck in an epic bisect that I eventually abandoned at the 41st test
with it still in merge bases.



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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-12 20:22                           ` Rafael J. Wysocki
@ 2013-02-15  0:50                             ` Yinghai Lu
  2013-02-16  0:39                               ` Bjorn Helgaas
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-15  0:50 UTC (permalink / raw)
  To: Rafael J. Wysocki, Bjorn Helgaas; +Cc: linux-pci, linux-acpi, linux-kernel

On Tue, Feb 12, 2013 at 12:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>
>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>
>> bisect to
>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>> |     PCI: Put pci_dev in device tree as early as possible
>>
>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>> are scanned.
>>
>> Bjorn said:
>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>  after enumerating everything below a bridge, and it will prevent us
>>  from doing any bus number reassignment for hotplug.
>>
>>  I think we should remove the bus numbers from the cached _PRT (or
>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>  device's IRQ, we should search up the PCI device tree looking for a
>>  _PRT associated with each node, and applying normal PCI bridge
>>  swizzling when we don't find a _PRT.  I think this can be done without
>>  using PCI bus numbers at all.
>>
>> So here we try to remove _PRT caching completely.
>>
>> -v2: check !handle early.
>>
>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
>> ---
>>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>>  drivers/acpi/pci_root.c     |   18 --------
>>  drivers/pci/pci-acpi.c      |   24 -----------
>>  include/acpi/acpi_drivers.h |    5 --
>>  4 files changed, 38 insertions(+), 104 deletions(-)

Bjorn,

Can you put this one into pci/next?

Thanks

Yinghai

>>
>> Index: linux-2.6/drivers/acpi/pci_irq.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/pci_irq.c
>> +++ linux-2.6/drivers/acpi/pci_irq.c
>> @@ -53,9 +53,6 @@ struct acpi_prt_entry {
>>       u32                     index;          /* GSI, or link _CRS index */
>>  };
>>
>> -static LIST_HEAD(acpi_prt_list);
>> -static DEFINE_SPINLOCK(acpi_prt_lock);
>> -
>>  static inline char pin_name(int pin)
>>  {
>>       return 'A' + pin - 1;
>> @@ -65,28 +62,6 @@ static inline char pin_name(int pin)
>>                           PCI IRQ Routing Table (PRT) Support
>>     -------------------------------------------------------------------------- */
>>
>> -static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
>> -                                                       int pin)
>> -{
>> -     struct acpi_prt_entry *entry;
>> -     int segment = pci_domain_nr(dev->bus);
>> -     int bus = dev->bus->number;
>> -     int device = PCI_SLOT(dev->devfn);
>> -
>> -     spin_lock(&acpi_prt_lock);
>> -     list_for_each_entry(entry, &acpi_prt_list, list) {
>> -             if ((segment == entry->id.segment)
>> -                 && (bus == entry->id.bus)
>> -                 && (device == entry->id.device)
>> -                 && (pin == entry->pin)) {
>> -                     spin_unlock(&acpi_prt_lock);
>> -                     return entry;
>> -             }
>> -     }
>> -     spin_unlock(&acpi_prt_lock);
>> -     return NULL;
>> -}
>> -
>>  /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
>>  static const struct dmi_system_id medion_md9580[] = {
>>       {
>> @@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
>>       }
>>  }
>>
>> -static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
>> -                               struct acpi_pci_routing_table *prt)
>> +static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
>> +                               int pin, struct acpi_pci_routing_table *prt,
>> +                               struct acpi_prt_entry **entry_ptr)
>>  {
>> +     int segment = pci_domain_nr(dev->bus);
>> +     int bus = dev->bus->number;
>> +     int device = PCI_SLOT(dev->devfn);
>>       struct acpi_prt_entry *entry;
>>
>> +     if (((prt->address >> 16) & 0xffff) != device ||
>> +         prt->pin + 1 != pin)
>> +             return -ENODEV;
>> +
>>       entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
>>       if (!entry)
>>               return -ENOMEM;
>> @@ -237,26 +220,33 @@ static int acpi_pci_irq_add_entry(acpi_h
>>                             entry->id.device, pin_name(entry->pin),
>>                             prt->source, entry->index));
>>
>> -     spin_lock(&acpi_prt_lock);
>> -     list_add_tail(&entry->list, &acpi_prt_list);
>> -     spin_unlock(&acpi_prt_lock);
>> +     *entry_ptr = entry;
>>
>>       return 0;
>>  }
>>
>> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
>> +static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
>> +                       int pin, struct acpi_prt_entry **entry_ptr)
>>  {
>>       acpi_status status;
>>       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>       struct acpi_pci_routing_table *entry;
>> +     acpi_handle handle = NULL;
>> +
>> +     if (dev->bus->bridge)
>> +             handle = ACPI_HANDLE(dev->bus->bridge);
>> +
>> +     if (!handle)
>> +             return -ENODEV;
>>
>>       /* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
>>       status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>       if (ACPI_FAILURE(status))
>>               return -ENODEV;
>>
>> -     printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
>> -            (char *) buffer.pointer);
>> +     dev_printk(KERN_DEBUG, &dev->dev,
>> +                     "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
>> +                    (char *) buffer.pointer);
>>
>>       kfree(buffer.pointer);
>>
>> @@ -273,7 +263,9 @@ int acpi_pci_irq_add_prt(acpi_handle han
>>
>>       entry = buffer.pointer;
>>       while (entry && (entry->length > 0)) {
>> -             acpi_pci_irq_add_entry(handle, segment, bus, entry);
>> +             if (!acpi_pci_irq_check_entry(handle, dev, pin,
>> +                                              entry, entry_ptr))
>> +                     break;
>>               entry = (struct acpi_pci_routing_table *)
>>                   ((unsigned long)entry + entry->length);
>>       }
>> @@ -282,23 +274,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
>>       return 0;
>>  }
>>
>> -void acpi_pci_irq_del_prt(int segment, int bus)
>> -{
>> -     struct acpi_prt_entry *entry, *tmp;
>> -
>> -     printk(KERN_DEBUG
>> -            "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
>> -            segment, bus);
>> -     spin_lock(&acpi_prt_lock);
>> -     list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
>> -             if (segment == entry->id.segment && bus == entry->id.bus) {
>> -                     list_del(&entry->list);
>> -                     kfree(entry);
>> -             }
>> -     }
>> -     spin_unlock(&acpi_prt_lock);
>> -}
>> -
>>  /* --------------------------------------------------------------------------
>>                            PCI Interrupt Routing Support
>>     -------------------------------------------------------------------------- */
>> @@ -359,12 +334,13 @@ static int acpi_reroute_boot_interrupt(s
>>
>>  static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
>>  {
>> -     struct acpi_prt_entry *entry;
>> +     struct acpi_prt_entry *entry = NULL;
>>       struct pci_dev *bridge;
>>       u8 bridge_pin, orig_pin = pin;
>> +     int ret;
>>
>> -     entry = acpi_pci_irq_find_prt_entry(dev, pin);
>> -     if (entry) {
>> +     ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
>> +     if (!ret && entry) {
>>  #ifdef CONFIG_X86_IO_APIC
>>               acpi_reroute_boot_interrupt(dev, entry);
>>  #endif /* CONFIG_X86_IO_APIC */
>> @@ -373,7 +349,7 @@ static struct acpi_prt_entry *acpi_pci_i
>>               return entry;
>>       }
>>
>> -     /*
>> +     /*
>>        * Attempt to derive an IRQ for this device from a parent bridge's
>>        * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
>>        */
>> @@ -393,8 +369,8 @@ static struct acpi_prt_entry *acpi_pci_i
>>                       pin = bridge_pin;
>>               }
>>
>> -             entry = acpi_pci_irq_find_prt_entry(bridge, pin);
>> -             if (entry) {
>> +             ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
>> +             if (!ret && entry) {
>>                       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>>                                        "Derived GSI for %s INT %c from %s\n",
>>                                        pci_name(dev), pin_name(orig_pin),
>> @@ -470,6 +446,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>                       dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
>>                                pin_name(pin));
>>               }
>> +
>>               return 0;
>>       }
>>
>> @@ -477,6 +454,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>       if (rc < 0) {
>>               dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
>>                        pin_name(pin));
>> +             kfree(entry);
>>               return rc;
>>       }
>>       dev->irq = rc;
>> @@ -491,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>               (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
>>               (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
>>
>> +     kfree(entry);
>>       return 0;
>>  }
>>
>> @@ -513,6 +492,8 @@ void acpi_pci_irq_disable(struct pci_dev
>>       else
>>               gsi = entry->index;
>>
>> +     kfree(entry);
>> +
>>       /*
>>        * TBD: It might be worth clearing dev->irq by magic constant
>>        * (e.g. PCI_UNDEFINED_IRQ).
>> Index: linux-2.6/drivers/acpi/pci_root.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/acpi/pci_root.c
>> +++ linux-2.6/drivers/acpi/pci_root.c
>> @@ -413,7 +413,6 @@ static int acpi_pci_root_add(struct acpi
>>       acpi_status status;
>>       int result;
>>       struct acpi_pci_root *root;
>> -     acpi_handle handle;
>>       struct acpi_pci_driver *driver;
>>       u32 flags, base_flags;
>>       bool is_osc_granted = false;
>> @@ -468,16 +467,6 @@ static int acpi_pci_root_add(struct acpi
>>              acpi_device_name(device), acpi_device_bid(device),
>>              root->segment, &root->secondary);
>>
>> -     /*
>> -      * PCI Routing Table
>> -      * -----------------
>> -      * Evaluate and parse _PRT, if exists.
>> -      */
>> -     status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
>> -     if (ACPI_SUCCESS(status))
>> -             result = acpi_pci_irq_add_prt(device->handle, root->segment,
>> -                                           root->secondary.start);
>> -
>>       root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
>>
>>       /*
>> @@ -602,7 +591,6 @@ out_del_root:
>>       list_del(&root->node);
>>       mutex_unlock(&acpi_pci_root_lock);
>>
>> -     acpi_pci_irq_del_prt(root->segment, root->secondary.start);
>>  end:
>>       kfree(root);
>>       return result;
>> @@ -610,8 +598,6 @@ end:
>>
>>  static void acpi_pci_root_remove(struct acpi_device *device)
>>  {
>> -     acpi_status status;
>> -     acpi_handle handle;
>>       struct acpi_pci_root *root = acpi_driver_data(device);
>>       struct acpi_pci_driver *driver;
>>
>> @@ -626,10 +612,6 @@ static void acpi_pci_root_remove(struct
>>       device_set_run_wake(root->bus->bridge, false);
>>       pci_acpi_remove_bus_pm_notifier(device);
>>
>> -     status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
>> -     if (ACPI_SUCCESS(status))
>> -             acpi_pci_irq_del_prt(root->segment, root->secondary.start);
>> -
>>       pci_remove_root_bus(root->bus);
>>
>>       mutex_lock(&acpi_pci_root_lock);
>> Index: linux-2.6/drivers/pci/pci-acpi.c
>> ===================================================================
>> --- linux-2.6.orig/drivers/pci/pci-acpi.c
>> +++ linux-2.6/drivers/pci/pci-acpi.c
>> @@ -307,25 +307,6 @@ static void pci_acpi_setup(struct device
>>       struct pci_dev *pci_dev = to_pci_dev(dev);
>>       acpi_handle handle = ACPI_HANDLE(dev);
>>       struct acpi_device *adev;
>> -     acpi_status status;
>> -     acpi_handle dummy;
>> -
>> -     /*
>> -      * Evaluate and parse _PRT, if exists.  This code allows parsing of
>> -      * _PRT objects within the scope of non-bridge devices.  Note that
>> -      * _PRTs within the scope of a PCI bridge assume the bridge's
>> -      * subordinate bus number.
>> -      *
>> -      * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
>> -      */
>> -     status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
>> -     if (ACPI_SUCCESS(status)) {
>> -             unsigned char bus;
>> -
>> -             bus = pci_dev->subordinate ?
>> -                     pci_dev->subordinate->number : pci_dev->bus->number;
>> -             acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
>> -     }
>>
>>       if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
>>               return;
>> @@ -340,7 +321,6 @@ static void pci_acpi_setup(struct device
>>
>>  static void pci_acpi_cleanup(struct device *dev)
>>  {
>> -     struct pci_dev *pci_dev = to_pci_dev(dev);
>>       acpi_handle handle = ACPI_HANDLE(dev);
>>       struct acpi_device *adev;
>>
>> @@ -349,10 +329,6 @@ static void pci_acpi_cleanup(struct devi
>>               device_set_run_wake(dev, false);
>>               pci_acpi_remove_pm_notifier(adev);
>>       }
>> -
>> -     if (pci_dev->subordinate)
>> -             acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
>> -                                  pci_dev->subordinate->number);
>>  }
>>
>>  static struct acpi_bus_type acpi_pci_bus = {
>> Index: linux-2.6/include/acpi/acpi_drivers.h
>> ===================================================================
>> --- linux-2.6.orig/include/acpi/acpi_drivers.h
>> +++ linux-2.6/include/acpi/acpi_drivers.h
>> @@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
>>                              int *polarity, char **name);
>>  int acpi_pci_link_free_irq(acpi_handle handle);
>>
>> -/* ACPI PCI Interrupt Routing (pci_irq.c) */
>> -
>> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
>> -void acpi_pci_irq_del_prt(int segment, int bus);
>> -
>>  /* ACPI PCI Device Binding (pci_bind.c) */
>>
>>  struct pci_bus;
> --
> I speak only for myself.
> Rafael J. Wysocki, Intel Open Source Technology Center.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-15  0:50                             ` Yinghai Lu
@ 2013-02-16  0:39                               ` Bjorn Helgaas
  2013-02-16  1:26                                 ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Bjorn Helgaas @ 2013-02-16  0:39 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Thu, Feb 14, 2013 at 5:50 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Tue, Feb 12, 2013 at 12:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
>>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>>
>>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>>
>>> bisect to
>>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>>> |     PCI: Put pci_dev in device tree as early as possible
>>>
>>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>>> are scanned.
>>>
>>> Bjorn said:
>>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>>  after enumerating everything below a bridge, and it will prevent us
>>>  from doing any bus number reassignment for hotplug.
>>>
>>>  I think we should remove the bus numbers from the cached _PRT (or
>>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>>  device's IRQ, we should search up the PCI device tree looking for a
>>>  _PRT associated with each node, and applying normal PCI bridge
>>>  swizzling when we don't find a _PRT.  I think this can be done without
>>>  using PCI bus numbers at all.
>>>
>>> So here we try to remove _PRT caching completely.
>>>
>>> -v2: check !handle early.
>>>
>>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>
>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>
>>> ---
>>>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>>>  drivers/acpi/pci_root.c     |   18 --------
>>>  drivers/pci/pci-acpi.c      |   24 -----------
>>>  include/acpi/acpi_drivers.h |    5 --
>>>  4 files changed, 38 insertions(+), 104 deletions(-)
>
> Bjorn,
>
> Can you put this one into pci/next?

I'm not sure what this patch is based on or what the best way to merge
it is.  It doesn't apply cleanly to my next or
pci/yinghai-root-bus-hotplug branches.

I did apply it manually on top of pci/yinghai-root-bus-hotplug to try
it out, but we need to tweak the messages a little bit.

Previously we printed "ACPI: PCI Interrupt Routing Table [%s._PRT]"
once when loading it, which was fine.  Now we print it every time we
look at a _PRT, which is too much because it isn't really adding any
information.

We also print "ACPI Exception: AE_NOT_FOUND, Evaluating _PRT
[AE_NOT_FOUND] (20121018/pci_irq-259)" if we find ACPI nodes without
_PRTs, which we shouldn't do, because that's a common and normal
situation.

Bjorn

>>> Index: linux-2.6/drivers/acpi/pci_irq.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/acpi/pci_irq.c
>>> +++ linux-2.6/drivers/acpi/pci_irq.c
>>> @@ -53,9 +53,6 @@ struct acpi_prt_entry {
>>>       u32                     index;          /* GSI, or link _CRS index */
>>>  };
>>>
>>> -static LIST_HEAD(acpi_prt_list);
>>> -static DEFINE_SPINLOCK(acpi_prt_lock);
>>> -
>>>  static inline char pin_name(int pin)
>>>  {
>>>       return 'A' + pin - 1;
>>> @@ -65,28 +62,6 @@ static inline char pin_name(int pin)
>>>                           PCI IRQ Routing Table (PRT) Support
>>>     -------------------------------------------------------------------------- */
>>>
>>> -static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
>>> -                                                       int pin)
>>> -{
>>> -     struct acpi_prt_entry *entry;
>>> -     int segment = pci_domain_nr(dev->bus);
>>> -     int bus = dev->bus->number;
>>> -     int device = PCI_SLOT(dev->devfn);
>>> -
>>> -     spin_lock(&acpi_prt_lock);
>>> -     list_for_each_entry(entry, &acpi_prt_list, list) {
>>> -             if ((segment == entry->id.segment)
>>> -                 && (bus == entry->id.bus)
>>> -                 && (device == entry->id.device)
>>> -                 && (pin == entry->pin)) {
>>> -                     spin_unlock(&acpi_prt_lock);
>>> -                     return entry;
>>> -             }
>>> -     }
>>> -     spin_unlock(&acpi_prt_lock);
>>> -     return NULL;
>>> -}
>>> -
>>>  /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
>>>  static const struct dmi_system_id medion_md9580[] = {
>>>       {
>>> @@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
>>>       }
>>>  }
>>>
>>> -static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
>>> -                               struct acpi_pci_routing_table *prt)
>>> +static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
>>> +                               int pin, struct acpi_pci_routing_table *prt,
>>> +                               struct acpi_prt_entry **entry_ptr)
>>>  {
>>> +     int segment = pci_domain_nr(dev->bus);
>>> +     int bus = dev->bus->number;
>>> +     int device = PCI_SLOT(dev->devfn);
>>>       struct acpi_prt_entry *entry;
>>>
>>> +     if (((prt->address >> 16) & 0xffff) != device ||
>>> +         prt->pin + 1 != pin)
>>> +             return -ENODEV;
>>> +
>>>       entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
>>>       if (!entry)
>>>               return -ENOMEM;
>>> @@ -237,26 +220,33 @@ static int acpi_pci_irq_add_entry(acpi_h
>>>                             entry->id.device, pin_name(entry->pin),
>>>                             prt->source, entry->index));
>>>
>>> -     spin_lock(&acpi_prt_lock);
>>> -     list_add_tail(&entry->list, &acpi_prt_list);
>>> -     spin_unlock(&acpi_prt_lock);
>>> +     *entry_ptr = entry;
>>>
>>>       return 0;
>>>  }
>>>
>>> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
>>> +static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
>>> +                       int pin, struct acpi_prt_entry **entry_ptr)
>>>  {
>>>       acpi_status status;
>>>       struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
>>>       struct acpi_pci_routing_table *entry;
>>> +     acpi_handle handle = NULL;
>>> +
>>> +     if (dev->bus->bridge)
>>> +             handle = ACPI_HANDLE(dev->bus->bridge);
>>> +
>>> +     if (!handle)
>>> +             return -ENODEV;
>>>
>>>       /* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
>>>       status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
>>>       if (ACPI_FAILURE(status))
>>>               return -ENODEV;
>>>
>>> -     printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
>>> -            (char *) buffer.pointer);
>>> +     dev_printk(KERN_DEBUG, &dev->dev,
>>> +                     "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
>>> +                    (char *) buffer.pointer);
>>>
>>>       kfree(buffer.pointer);
>>>
>>> @@ -273,7 +263,9 @@ int acpi_pci_irq_add_prt(acpi_handle han
>>>
>>>       entry = buffer.pointer;
>>>       while (entry && (entry->length > 0)) {
>>> -             acpi_pci_irq_add_entry(handle, segment, bus, entry);
>>> +             if (!acpi_pci_irq_check_entry(handle, dev, pin,
>>> +                                              entry, entry_ptr))
>>> +                     break;
>>>               entry = (struct acpi_pci_routing_table *)
>>>                   ((unsigned long)entry + entry->length);
>>>       }
>>> @@ -282,23 +274,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
>>>       return 0;
>>>  }
>>>
>>> -void acpi_pci_irq_del_prt(int segment, int bus)
>>> -{
>>> -     struct acpi_prt_entry *entry, *tmp;
>>> -
>>> -     printk(KERN_DEBUG
>>> -            "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
>>> -            segment, bus);
>>> -     spin_lock(&acpi_prt_lock);
>>> -     list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
>>> -             if (segment == entry->id.segment && bus == entry->id.bus) {
>>> -                     list_del(&entry->list);
>>> -                     kfree(entry);
>>> -             }
>>> -     }
>>> -     spin_unlock(&acpi_prt_lock);
>>> -}
>>> -
>>>  /* --------------------------------------------------------------------------
>>>                            PCI Interrupt Routing Support
>>>     -------------------------------------------------------------------------- */
>>> @@ -359,12 +334,13 @@ static int acpi_reroute_boot_interrupt(s
>>>
>>>  static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
>>>  {
>>> -     struct acpi_prt_entry *entry;
>>> +     struct acpi_prt_entry *entry = NULL;
>>>       struct pci_dev *bridge;
>>>       u8 bridge_pin, orig_pin = pin;
>>> +     int ret;
>>>
>>> -     entry = acpi_pci_irq_find_prt_entry(dev, pin);
>>> -     if (entry) {
>>> +     ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
>>> +     if (!ret && entry) {
>>>  #ifdef CONFIG_X86_IO_APIC
>>>               acpi_reroute_boot_interrupt(dev, entry);
>>>  #endif /* CONFIG_X86_IO_APIC */
>>> @@ -373,7 +349,7 @@ static struct acpi_prt_entry *acpi_pci_i
>>>               return entry;
>>>       }
>>>
>>> -     /*
>>> +     /*
>>>        * Attempt to derive an IRQ for this device from a parent bridge's
>>>        * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
>>>        */
>>> @@ -393,8 +369,8 @@ static struct acpi_prt_entry *acpi_pci_i
>>>                       pin = bridge_pin;
>>>               }
>>>
>>> -             entry = acpi_pci_irq_find_prt_entry(bridge, pin);
>>> -             if (entry) {
>>> +             ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
>>> +             if (!ret && entry) {
>>>                       ACPI_DEBUG_PRINT((ACPI_DB_INFO,
>>>                                        "Derived GSI for %s INT %c from %s\n",
>>>                                        pci_name(dev), pin_name(orig_pin),
>>> @@ -470,6 +446,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>>                       dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
>>>                                pin_name(pin));
>>>               }
>>> +
>>>               return 0;
>>>       }
>>>
>>> @@ -477,6 +454,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>>       if (rc < 0) {
>>>               dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
>>>                        pin_name(pin));
>>> +             kfree(entry);
>>>               return rc;
>>>       }
>>>       dev->irq = rc;
>>> @@ -491,6 +469,7 @@ int acpi_pci_irq_enable(struct pci_dev *
>>>               (triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
>>>               (polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
>>>
>>> +     kfree(entry);
>>>       return 0;
>>>  }
>>>
>>> @@ -513,6 +492,8 @@ void acpi_pci_irq_disable(struct pci_dev
>>>       else
>>>               gsi = entry->index;
>>>
>>> +     kfree(entry);
>>> +
>>>       /*
>>>        * TBD: It might be worth clearing dev->irq by magic constant
>>>        * (e.g. PCI_UNDEFINED_IRQ).
>>> Index: linux-2.6/drivers/acpi/pci_root.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/acpi/pci_root.c
>>> +++ linux-2.6/drivers/acpi/pci_root.c
>>> @@ -413,7 +413,6 @@ static int acpi_pci_root_add(struct acpi
>>>       acpi_status status;
>>>       int result;
>>>       struct acpi_pci_root *root;
>>> -     acpi_handle handle;
>>>       struct acpi_pci_driver *driver;
>>>       u32 flags, base_flags;
>>>       bool is_osc_granted = false;
>>> @@ -468,16 +467,6 @@ static int acpi_pci_root_add(struct acpi
>>>              acpi_device_name(device), acpi_device_bid(device),
>>>              root->segment, &root->secondary);
>>>
>>> -     /*
>>> -      * PCI Routing Table
>>> -      * -----------------
>>> -      * Evaluate and parse _PRT, if exists.
>>> -      */
>>> -     status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
>>> -     if (ACPI_SUCCESS(status))
>>> -             result = acpi_pci_irq_add_prt(device->handle, root->segment,
>>> -                                           root->secondary.start);
>>> -
>>>       root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
>>>
>>>       /*
>>> @@ -602,7 +591,6 @@ out_del_root:
>>>       list_del(&root->node);
>>>       mutex_unlock(&acpi_pci_root_lock);
>>>
>>> -     acpi_pci_irq_del_prt(root->segment, root->secondary.start);
>>>  end:
>>>       kfree(root);
>>>       return result;
>>> @@ -610,8 +598,6 @@ end:
>>>
>>>  static void acpi_pci_root_remove(struct acpi_device *device)
>>>  {
>>> -     acpi_status status;
>>> -     acpi_handle handle;
>>>       struct acpi_pci_root *root = acpi_driver_data(device);
>>>       struct acpi_pci_driver *driver;
>>>
>>> @@ -626,10 +612,6 @@ static void acpi_pci_root_remove(struct
>>>       device_set_run_wake(root->bus->bridge, false);
>>>       pci_acpi_remove_bus_pm_notifier(device);
>>>
>>> -     status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
>>> -     if (ACPI_SUCCESS(status))
>>> -             acpi_pci_irq_del_prt(root->segment, root->secondary.start);
>>> -
>>>       pci_remove_root_bus(root->bus);
>>>
>>>       mutex_lock(&acpi_pci_root_lock);
>>> Index: linux-2.6/drivers/pci/pci-acpi.c
>>> ===================================================================
>>> --- linux-2.6.orig/drivers/pci/pci-acpi.c
>>> +++ linux-2.6/drivers/pci/pci-acpi.c
>>> @@ -307,25 +307,6 @@ static void pci_acpi_setup(struct device
>>>       struct pci_dev *pci_dev = to_pci_dev(dev);
>>>       acpi_handle handle = ACPI_HANDLE(dev);
>>>       struct acpi_device *adev;
>>> -     acpi_status status;
>>> -     acpi_handle dummy;
>>> -
>>> -     /*
>>> -      * Evaluate and parse _PRT, if exists.  This code allows parsing of
>>> -      * _PRT objects within the scope of non-bridge devices.  Note that
>>> -      * _PRTs within the scope of a PCI bridge assume the bridge's
>>> -      * subordinate bus number.
>>> -      *
>>> -      * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
>>> -      */
>>> -     status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
>>> -     if (ACPI_SUCCESS(status)) {
>>> -             unsigned char bus;
>>> -
>>> -             bus = pci_dev->subordinate ?
>>> -                     pci_dev->subordinate->number : pci_dev->bus->number;
>>> -             acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
>>> -     }
>>>
>>>       if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
>>>               return;
>>> @@ -340,7 +321,6 @@ static void pci_acpi_setup(struct device
>>>
>>>  static void pci_acpi_cleanup(struct device *dev)
>>>  {
>>> -     struct pci_dev *pci_dev = to_pci_dev(dev);
>>>       acpi_handle handle = ACPI_HANDLE(dev);
>>>       struct acpi_device *adev;
>>>
>>> @@ -349,10 +329,6 @@ static void pci_acpi_cleanup(struct devi
>>>               device_set_run_wake(dev, false);
>>>               pci_acpi_remove_pm_notifier(adev);
>>>       }
>>> -
>>> -     if (pci_dev->subordinate)
>>> -             acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
>>> -                                  pci_dev->subordinate->number);
>>>  }
>>>
>>>  static struct acpi_bus_type acpi_pci_bus = {
>>> Index: linux-2.6/include/acpi/acpi_drivers.h
>>> ===================================================================
>>> --- linux-2.6.orig/include/acpi/acpi_drivers.h
>>> +++ linux-2.6/include/acpi/acpi_drivers.h
>>> @@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
>>>                              int *polarity, char **name);
>>>  int acpi_pci_link_free_irq(acpi_handle handle);
>>>
>>> -/* ACPI PCI Interrupt Routing (pci_irq.c) */
>>> -
>>> -int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
>>> -void acpi_pci_irq_del_prt(int segment, int bus);
>>> -
>>>  /* ACPI PCI Device Binding (pci_bind.c) */
>>>
>>>  struct pci_bus;
>> --
>> I speak only for myself.
>> Rafael J. Wysocki, Intel Open Source Technology Center.
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-16  0:39                               ` Bjorn Helgaas
@ 2013-02-16  1:26                                 ` Yinghai Lu
  2013-02-16  1:37                                   ` Yinghai Lu
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-16  1:26 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Fri, Feb 15, 2013 at 4:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> On Thu, Feb 14, 2013 at 5:50 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Tue, Feb 12, 2013 at 12:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>> On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
>>>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>>>
>>>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>>>
>>>> bisect to
>>>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>>>> |     PCI: Put pci_dev in device tree as early as possible
>>>>
>>>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>>>> are scanned.
>>>>
>>>> Bjorn said:
>>>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>>>  after enumerating everything below a bridge, and it will prevent us
>>>>  from doing any bus number reassignment for hotplug.
>>>>
>>>>  I think we should remove the bus numbers from the cached _PRT (or
>>>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>>>  device's IRQ, we should search up the PCI device tree looking for a
>>>>  _PRT associated with each node, and applying normal PCI bridge
>>>>  swizzling when we don't find a _PRT.  I think this can be done without
>>>>  using PCI bus numbers at all.
>>>>
>>>> So here we try to remove _PRT caching completely.
>>>>
>>>> -v2: check !handle early.
>>>>
>>>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>
>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>
>>>> ---
>>>>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>>>>  drivers/acpi/pci_root.c     |   18 --------
>>>>  drivers/pci/pci-acpi.c      |   24 -----------
>>>>  include/acpi/acpi_drivers.h |    5 --
>>>>  4 files changed, 38 insertions(+), 104 deletions(-)
>>
>> Bjorn,
>>
>> Can you put this one into pci/next?
>
> I'm not sure what this patch is based on or what the best way to merge
> it is.  It doesn't apply cleanly to my next or
> pci/yinghai-root-bus-hotplug branches.

My fault, that is based on pci/next + pm/linux-next

linux-next removed
acpi_power_resource_(un)register_device ...

>
> I did apply it manually on top of pci/yinghai-root-bus-hotplug to try
> it out, but we need to tweak the messages a little bit.
>
> Previously we printed "ACPI: PCI Interrupt Routing Table [%s._PRT]"
> once when loading it, which was fine.  Now we print it every time we
> look at a _PRT, which is too much because it isn't really adding any
> information.
>
> We also print "ACPI Exception: AE_NOT_FOUND, Evaluating _PRT
> [AE_NOT_FOUND] (20121018/pci_irq-259)" if we find ACPI nodes without
> _PRTs, which we shouldn't do, because that's a common and normal
> situation.

Sure. Can you have separated patch to do that ?

Or want me to resend the patch.

Thanks

Yinghai

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-16  1:26                                 ` Yinghai Lu
@ 2013-02-16  1:37                                   ` Yinghai Lu
  2013-02-19 18:51                                     ` Bjorn Helgaas
  0 siblings, 1 reply; 27+ messages in thread
From: Yinghai Lu @ 2013-02-16  1:37 UTC (permalink / raw)
  To: Bjorn Helgaas; +Cc: Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

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

On Fri, Feb 15, 2013 at 5:26 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Feb 15, 2013 at 4:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> On Thu, Feb 14, 2013 at 5:50 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>> On Tue, Feb 12, 2013 at 12:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>>> On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
>>>>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>>>>
>>>>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>>>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>>>>
>>>>> bisect to
>>>>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>>>>> |     PCI: Put pci_dev in device tree as early as possible
>>>>>
>>>>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>>>>> are scanned.
>>>>>
>>>>> Bjorn said:
>>>>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>>>>  after enumerating everything below a bridge, and it will prevent us
>>>>>  from doing any bus number reassignment for hotplug.
>>>>>
>>>>>  I think we should remove the bus numbers from the cached _PRT (or
>>>>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>>>>  device's IRQ, we should search up the PCI device tree looking for a
>>>>>  _PRT associated with each node, and applying normal PCI bridge
>>>>>  swizzling when we don't find a _PRT.  I think this can be done without
>>>>>  using PCI bus numbers at all.
>>>>>
>>>>> So here we try to remove _PRT caching completely.
>>>>>
>>>>> -v2: check !handle early.
>>>>>
>>>>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>>>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>>
>>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>
>>>>> ---
>>>>>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>>>>>  drivers/acpi/pci_root.c     |   18 --------
>>>>>  drivers/pci/pci-acpi.c      |   24 -----------
>>>>>  include/acpi/acpi_drivers.h |    5 --
>>>>>  4 files changed, 38 insertions(+), 104 deletions(-)
>>>
>>> Bjorn,
>>>
>>> Can you put this one into pci/next?
>>
>> I'm not sure what this patch is based on or what the best way to merge
>> it is.  It doesn't apply cleanly to my next or
>> pci/yinghai-root-bus-hotplug branches.
>
> My fault, that is based on pci/next + pm/linux-next
>
> linux-next removed
> acpi_power_resource_(un)register_device ...
>
>>
>> I did apply it manually on top of pci/yinghai-root-bus-hotplug to try
>> it out, but we need to tweak the messages a little bit.
>>
>> Previously we printed "ACPI: PCI Interrupt Routing Table [%s._PRT]"
>> once when loading it, which was fine.  Now we print it every time we
>> look at a _PRT, which is too much because it isn't really adding any
>> information.
>>
>> We also print "ACPI Exception: AE_NOT_FOUND, Evaluating _PRT
>> [AE_NOT_FOUND] (20121018/pci_irq-259)" if we find ACPI nodes without
>> _PRTs, which we shouldn't do, because that's a common and normal
>> situation.
>
> Sure. Can you have separated patch to do that ?
>
> Or want me to resend the patch.

Please check attached updated version that remove print out ...

and it could be applied cleanly on top of pci/yinghai-root-bus-hotplug

Thanks

Yinghai

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

From: Yinghai Lu <yinghai@kernel.org>
Subject: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()

Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has

[    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
[    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5

bisect to
| commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
|     PCI: Put pci_dev in device tree as early as possible

It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
are scanned.

Bjorn said:
 The bus number binding means acpi_pci_irq_add_prt() has to happen
 after enumerating everything below a bridge, and it will prevent us
 from doing any bus number reassignment for hotplug.

 I think we should remove the bus numbers from the cached _PRT (or
 maybe even remove the _PRT caching completely).  When we enable a PCI
 device's IRQ, we should search up the PCI device tree looking for a
 _PRT associated with each node, and applying normal PCI bridge
 swizzling when we don't find a _PRT.  I think this can be done without
 using PCI bus numbers at all.

So here we try to remove _PRT caching completely.

-v2: check !handle early.
-v3: remove not needed print out according to Bjorn.

Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 drivers/acpi/pci_irq.c      |  102 ++++++++++++++------------------------------
 drivers/acpi/pci_root.c     |   18 -------
 drivers/pci/pci-acpi.c      |   24 ----------
 include/acpi/acpi_drivers.h |    5 --
 4 files changed, 34 insertions(+), 115 deletions(-)

Index: linux-2.6/drivers/acpi/pci_irq.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_irq.c
+++ linux-2.6/drivers/acpi/pci_irq.c
@@ -53,9 +53,6 @@ struct acpi_prt_entry {
 	u32			index;		/* GSI, or link _CRS index */
 };
 
-static LIST_HEAD(acpi_prt_list);
-static DEFINE_SPINLOCK(acpi_prt_lock);
-
 static inline char pin_name(int pin)
 {
 	return 'A' + pin - 1;
@@ -65,28 +62,6 @@ static inline char pin_name(int pin)
                          PCI IRQ Routing Table (PRT) Support
    -------------------------------------------------------------------------- */
 
-static struct acpi_prt_entry *acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
-							  int pin)
-{
-	struct acpi_prt_entry *entry;
-	int segment = pci_domain_nr(dev->bus);
-	int bus = dev->bus->number;
-	int device = PCI_SLOT(dev->devfn);
-
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry(entry, &acpi_prt_list, list) {
-		if ((segment == entry->id.segment)
-		    && (bus == entry->id.bus)
-		    && (device == entry->id.device)
-		    && (pin == entry->pin)) {
-			spin_unlock(&acpi_prt_lock);
-			return entry;
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-	return NULL;
-}
-
 /* http://bugzilla.kernel.org/show_bug.cgi?id=4773 */
 static const struct dmi_system_id medion_md9580[] = {
 	{
@@ -184,11 +159,19 @@ static void do_prt_fixups(struct acpi_pr
 	}
 }
 
-static int acpi_pci_irq_add_entry(acpi_handle handle, int segment, int bus,
-				  struct acpi_pci_routing_table *prt)
+static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev,
+				  int pin, struct acpi_pci_routing_table *prt,
+				  struct acpi_prt_entry **entry_ptr)
 {
+	int segment = pci_domain_nr(dev->bus);
+	int bus = dev->bus->number;
+	int device = PCI_SLOT(dev->devfn);
 	struct acpi_prt_entry *entry;
 
+	if (((prt->address >> 16) & 0xffff) != device ||
+	    prt->pin + 1 != pin)
+		return -ENODEV;
+
 	entry = kzalloc(sizeof(struct acpi_prt_entry), GFP_KERNEL);
 	if (!entry)
 		return -ENOMEM;
@@ -237,43 +220,37 @@ static int acpi_pci_irq_add_entry(acpi_h
 			      entry->id.device, pin_name(entry->pin),
 			      prt->source, entry->index));
 
-	spin_lock(&acpi_prt_lock);
-	list_add_tail(&entry->list, &acpi_prt_list);
-	spin_unlock(&acpi_prt_lock);
+	*entry_ptr = entry;
 
 	return 0;
 }
 
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus)
+static int acpi_pci_irq_find_prt_entry(struct pci_dev *dev,
+			  int pin, struct acpi_prt_entry **entry_ptr)
 {
 	acpi_status status;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	struct acpi_pci_routing_table *entry;
+	acpi_handle handle = NULL;
 
-	/* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
-	status = acpi_get_name(handle, ACPI_FULL_PATHNAME, &buffer);
-	if (ACPI_FAILURE(status))
-		return -ENODEV;
-
-	printk(KERN_DEBUG "ACPI: PCI Interrupt Routing Table [%s._PRT]\n",
-	       (char *) buffer.pointer);
-
-	kfree(buffer.pointer);
+	if (dev->bus->bridge)
+		handle = ACPI_HANDLE(dev->bus->bridge);
 
-	buffer.length = ACPI_ALLOCATE_BUFFER;
-	buffer.pointer = NULL;
+	if (!handle)
+		return -ENODEV;
 
+	/* 'handle' is the _PRT's parent (root bridge or PCI-PCI bridge) */
 	status = acpi_get_irq_routing_table(handle, &buffer);
 	if (ACPI_FAILURE(status)) {
-		ACPI_EXCEPTION((AE_INFO, status, "Evaluating _PRT [%s]",
-				acpi_format_exception(status)));
 		kfree(buffer.pointer);
 		return -ENODEV;
 	}
 
 	entry = buffer.pointer;
 	while (entry && (entry->length > 0)) {
-		acpi_pci_irq_add_entry(handle, segment, bus, entry);
+		if (!acpi_pci_irq_check_entry(handle, dev, pin,
+						 entry, entry_ptr))
+			break;
 		entry = (struct acpi_pci_routing_table *)
 		    ((unsigned long)entry + entry->length);
 	}
@@ -282,23 +259,6 @@ int acpi_pci_irq_add_prt(acpi_handle han
 	return 0;
 }
 
-void acpi_pci_irq_del_prt(int segment, int bus)
-{
-	struct acpi_prt_entry *entry, *tmp;
-
-	printk(KERN_DEBUG
-	       "ACPI: Delete PCI Interrupt Routing Table for %04x:%02x\n",
-	       segment, bus);
-	spin_lock(&acpi_prt_lock);
-	list_for_each_entry_safe(entry, tmp, &acpi_prt_list, list) {
-		if (segment == entry->id.segment && bus == entry->id.bus) {
-			list_del(&entry->list);
-			kfree(entry);
-		}
-	}
-	spin_unlock(&acpi_prt_lock);
-}
-
 /* --------------------------------------------------------------------------
                           PCI Interrupt Routing Support
    -------------------------------------------------------------------------- */
@@ -359,12 +319,13 @@ static int acpi_reroute_boot_interrupt(s
 
 static struct acpi_prt_entry *acpi_pci_irq_lookup(struct pci_dev *dev, int pin)
 {
-	struct acpi_prt_entry *entry;
+	struct acpi_prt_entry *entry = NULL;
 	struct pci_dev *bridge;
 	u8 bridge_pin, orig_pin = pin;
+	int ret;
 
-	entry = acpi_pci_irq_find_prt_entry(dev, pin);
-	if (entry) {
+	ret = acpi_pci_irq_find_prt_entry(dev, pin, &entry);
+	if (!ret && entry) {
 #ifdef CONFIG_X86_IO_APIC
 		acpi_reroute_boot_interrupt(dev, entry);
 #endif /* CONFIG_X86_IO_APIC */
@@ -373,7 +334,7 @@ static struct acpi_prt_entry *acpi_pci_i
 		return entry;
 	}
 
-	/* 
+	/*
 	 * Attempt to derive an IRQ for this device from a parent bridge's
 	 * PCI interrupt routing entry (eg. yenta bridge and add-in card bridge).
 	 */
@@ -393,8 +354,8 @@ static struct acpi_prt_entry *acpi_pci_i
 			pin = bridge_pin;
 		}
 
-		entry = acpi_pci_irq_find_prt_entry(bridge, pin);
-		if (entry) {
+		ret = acpi_pci_irq_find_prt_entry(bridge, pin, &entry);
+		if (!ret && entry) {
 			ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 					 "Derived GSI for %s INT %c from %s\n",
 					 pci_name(dev), pin_name(orig_pin),
@@ -470,6 +431,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 			dev_warn(&dev->dev, "PCI INT %c: no GSI\n",
 				 pin_name(pin));
 		}
+
 		return 0;
 	}
 
@@ -477,6 +439,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 	if (rc < 0) {
 		dev_warn(&dev->dev, "PCI INT %c: failed to register GSI\n",
 			 pin_name(pin));
+		kfree(entry);
 		return rc;
 	}
 	dev->irq = rc;
@@ -491,6 +454,7 @@ int acpi_pci_irq_enable(struct pci_dev *
 		(triggering == ACPI_LEVEL_SENSITIVE) ? "level" : "edge",
 		(polarity == ACPI_ACTIVE_LOW) ? "low" : "high", dev->irq);
 
+	kfree(entry);
 	return 0;
 }
 
@@ -513,6 +477,8 @@ void acpi_pci_irq_disable(struct pci_dev
 	else
 		gsi = entry->index;
 
+	kfree(entry);
+
 	/*
 	 * TBD: It might be worth clearing dev->irq by magic constant
 	 * (e.g. PCI_UNDEFINED_IRQ).
Index: linux-2.6/drivers/acpi/pci_root.c
===================================================================
--- linux-2.6.orig/drivers/acpi/pci_root.c
+++ linux-2.6/drivers/acpi/pci_root.c
@@ -434,7 +434,6 @@ static int acpi_pci_root_add(struct acpi
 	acpi_status status;
 	int result;
 	struct acpi_pci_root *root;
-	acpi_handle handle;
 	struct acpi_pci_driver *driver;
 	u32 flags, base_flags;
 	bool is_osc_granted = false;
@@ -489,16 +488,6 @@ static int acpi_pci_root_add(struct acpi
 	       acpi_device_name(device), acpi_device_bid(device),
 	       root->segment, &root->secondary);
 
-	/*
-	 * PCI Routing Table
-	 * -----------------
-	 * Evaluate and parse _PRT, if exists.
-	 */
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		result = acpi_pci_irq_add_prt(device->handle, root->segment,
-					      root->secondary.start);
-
 	root->mcfg_addr = acpi_pci_root_get_mcfg_addr(device->handle);
 
 	/*
@@ -623,7 +612,6 @@ out_del_root:
 	list_del(&root->node);
 	mutex_unlock(&acpi_pci_root_lock);
 
-	acpi_pci_irq_del_prt(root->segment, root->secondary.start);
 end:
 	kfree(root);
 	return result;
@@ -631,8 +619,6 @@ end:
 
 static int acpi_pci_root_remove(struct acpi_device *device, int type)
 {
-	acpi_status status;
-	acpi_handle handle;
 	struct acpi_pci_root *root = acpi_driver_data(device);
 	struct acpi_pci_driver *driver;
 
@@ -647,10 +633,6 @@ static int acpi_pci_root_remove(struct a
 	device_set_run_wake(root->bus->bridge, false);
 	pci_acpi_remove_bus_pm_notifier(device);
 
-	status = acpi_get_handle(device->handle, METHOD_NAME__PRT, &handle);
-	if (ACPI_SUCCESS(status))
-		acpi_pci_irq_del_prt(root->segment, root->secondary.start);
-
 	pci_remove_root_bus(root->bus);
 
 	mutex_lock(&acpi_pci_root_lock);
Index: linux-2.6/drivers/pci/pci-acpi.c
===================================================================
--- linux-2.6.orig/drivers/pci/pci-acpi.c
+++ linux-2.6/drivers/pci/pci-acpi.c
@@ -325,25 +325,6 @@ static void pci_acpi_setup(struct device
 	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
-	acpi_status status;
-	acpi_handle dummy;
-
-	/*
-	 * Evaluate and parse _PRT, if exists.  This code allows parsing of
-	 * _PRT objects within the scope of non-bridge devices.  Note that
-	 * _PRTs within the scope of a PCI bridge assume the bridge's
-	 * subordinate bus number.
-	 *
-	 * TBD: Can _PRTs exist within the scope of non-bridge PCI devices?
-	 */
-	status = acpi_get_handle(handle, METHOD_NAME__PRT, &dummy);
-	if (ACPI_SUCCESS(status)) {
-		unsigned char bus;
-
-		bus = pci_dev->subordinate ?
-			pci_dev->subordinate->number : pci_dev->bus->number;
-		acpi_pci_irq_add_prt(handle, pci_domain_nr(pci_dev->bus), bus);
-	}
 
 	acpi_power_resource_register_device(dev, handle);
 	if (acpi_bus_get_device(handle, &adev) || !adev->wakeup.flags.valid)
@@ -359,7 +340,6 @@ static void pci_acpi_setup(struct device
 
 static void pci_acpi_cleanup(struct device *dev)
 {
-	struct pci_dev *pci_dev = to_pci_dev(dev);
 	acpi_handle handle = ACPI_HANDLE(dev);
 	struct acpi_device *adev;
 
@@ -369,10 +349,6 @@ static void pci_acpi_cleanup(struct devi
 		pci_acpi_remove_pm_notifier(adev);
 	}
 	acpi_power_resource_unregister_device(dev, handle);
-
-	if (pci_dev->subordinate)
-		acpi_pci_irq_del_prt(pci_domain_nr(pci_dev->bus),
-				     pci_dev->subordinate->number);
 }
 
 static struct acpi_bus_type acpi_pci_bus = {
Index: linux-2.6/include/acpi/acpi_drivers.h
===================================================================
--- linux-2.6.orig/include/acpi/acpi_drivers.h
+++ linux-2.6/include/acpi/acpi_drivers.h
@@ -90,11 +90,6 @@ int acpi_pci_link_allocate_irq(acpi_hand
 			       int *polarity, char **name);
 int acpi_pci_link_free_irq(acpi_handle handle);
 
-/* ACPI PCI Interrupt Routing (pci_irq.c) */
-
-int acpi_pci_irq_add_prt(acpi_handle handle, int segment, int bus);
-void acpi_pci_irq_del_prt(int segment, int bus);
-
 /* ACPI PCI Device Binding (pci_bind.c) */
 
 struct pci_bus;

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

* Re: [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq()
  2013-02-16  1:37                                   ` Yinghai Lu
@ 2013-02-19 18:51                                     ` Bjorn Helgaas
  0 siblings, 0 replies; 27+ messages in thread
From: Bjorn Helgaas @ 2013-02-19 18:51 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Rafael J. Wysocki, linux-pci, linux-acpi, linux-kernel

On Fri, Feb 15, 2013 at 6:37 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Fri, Feb 15, 2013 at 5:26 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>> On Fri, Feb 15, 2013 at 4:39 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>>> On Thu, Feb 14, 2013 at 5:50 PM, Yinghai Lu <yinghai@kernel.org> wrote:
>>>> On Tue, Feb 12, 2013 at 12:22 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>>>>> On Tuesday, February 12, 2013 11:11:23 AM Yinghai Lu wrote:
>>>>>> Peter Hurley found "irq 18 nobody cared" with pci-next, and dmesg has
>>>>>>
>>>>>> [    8.983246] pci 0000:00:1e.0: can't derive routing for PCI INT A
>>>>>> [    8.983600] snd_ctxfi 0000:09:02.0: PCI INT A: no GSI - using ISA IRQ 5
>>>>>>
>>>>>> bisect to
>>>>>> | commit 4f535093cf8f6da8cfda7c36c2c1ecd2e9586ee4
>>>>>> |     PCI: Put pci_dev in device tree as early as possible
>>>>>>
>>>>>> It turns out we need to call acpi_pci_irq_add_prt() after the pci bridges
>>>>>> are scanned.
>>>>>>
>>>>>> Bjorn said:
>>>>>>  The bus number binding means acpi_pci_irq_add_prt() has to happen
>>>>>>  after enumerating everything below a bridge, and it will prevent us
>>>>>>  from doing any bus number reassignment for hotplug.
>>>>>>
>>>>>>  I think we should remove the bus numbers from the cached _PRT (or
>>>>>>  maybe even remove the _PRT caching completely).  When we enable a PCI
>>>>>>  device's IRQ, we should search up the PCI device tree looking for a
>>>>>>  _PRT associated with each node, and applying normal PCI bridge
>>>>>>  swizzling when we don't find a _PRT.  I think this can be done without
>>>>>>  using PCI bus numbers at all.
>>>>>>
>>>>>> So here we try to remove _PRT caching completely.
>>>>>>
>>>>>> -v2: check !handle early.
>>>>>>
>>>>>> Reported-and-tested-by: Peter Hurley <peter@hurleysoftware.com>
>>>>>> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
>>>>>> Signed-off-by: Yinghai Lu <yinghai@kernel.org>
>>>>>
>>>>> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>>>>>
>>>>>> ---
>>>>>>  drivers/acpi/pci_irq.c      |   95 +++++++++++++++++---------------------------
>>>>>>  drivers/acpi/pci_root.c     |   18 --------
>>>>>>  drivers/pci/pci-acpi.c      |   24 -----------
>>>>>>  include/acpi/acpi_drivers.h |    5 --
>>>>>>  4 files changed, 38 insertions(+), 104 deletions(-)
>>>>
>>>> Bjorn,
>>>>
>>>> Can you put this one into pci/next?
>>>
>>> I'm not sure what this patch is based on or what the best way to merge
>>> it is.  It doesn't apply cleanly to my next or
>>> pci/yinghai-root-bus-hotplug branches.
>>
>> My fault, that is based on pci/next + pm/linux-next
>>
>> linux-next removed
>> acpi_power_resource_(un)register_device ...
>>
>>>
>>> I did apply it manually on top of pci/yinghai-root-bus-hotplug to try
>>> it out, but we need to tweak the messages a little bit.
>>>
>>> Previously we printed "ACPI: PCI Interrupt Routing Table [%s._PRT]"
>>> once when loading it, which was fine.  Now we print it every time we
>>> look at a _PRT, which is too much because it isn't really adding any
>>> information.
>>>
>>> We also print "ACPI Exception: AE_NOT_FOUND, Evaluating _PRT
>>> [AE_NOT_FOUND] (20121018/pci_irq-259)" if we find ACPI nodes without
>>> _PRTs, which we shouldn't do, because that's a common and normal
>>> situation.
>>
>> Sure. Can you have separated patch to do that ?
>>
>> Or want me to resend the patch.
>
> Please check attached updated version that remove print out ...
>
> and it could be applied cleanly on top of pci/yinghai-root-bus-hotplug

Thanks, I applied this to pci/yinghai-root-bus-hotplug and merged it
into my next branch.

Bjorn

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

end of thread, other threads:[~2013-02-19 18:51 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-02-05 17:51 [-next-20130204] usb/hcd: irq 18: nobody cared Peter Hurley
2013-02-05 20:26 ` Alan Stern
2013-02-08 11:57   ` Peter Hurley
2013-02-10  3:14   ` [Bisected] " Peter Hurley
2013-02-10 14:23     ` Peter Hurley
2013-02-10 20:33       ` Yinghai Lu
2013-02-11  6:40         ` Yinghai Lu
2013-02-11 10:25           ` Jiri Slaby
2013-02-11 13:02           ` Peter Hurley
2013-02-11 19:19             ` Yinghai Lu
2013-02-11 19:45               ` Sedat Dilek
2013-02-11 19:53                 ` Yinghai Lu
2013-02-11 19:57               ` Yinghai Lu
2013-02-11 21:06                 ` Yinghai Lu
2013-02-11 22:41                   ` Bjorn Helgaas
2013-02-12  0:08                     ` Yinghai Lu
2013-02-12  3:21                       ` Yinghai Lu
2013-02-12 19:11                         ` [PATCH] ACPI, PCI: Get PRT entry during acpi_pci_enable_irq() Yinghai Lu
2013-02-12 20:22                           ` Rafael J. Wysocki
2013-02-15  0:50                             ` Yinghai Lu
2013-02-16  0:39                               ` Bjorn Helgaas
2013-02-16  1:26                                 ` Yinghai Lu
2013-02-16  1:37                                   ` Yinghai Lu
2013-02-19 18:51                                     ` Bjorn Helgaas
2013-02-13  2:20                           ` Peter Hurley
2013-02-13  2:42                             ` Yinghai Lu
2013-02-13  3:18                               ` Peter Hurley

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).