| Message ID | 1506941567-10762-1-git-send-email-lorenzo.pieralisi@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On 10/02/2017 03:52 AM, Lorenzo Pieralisi wrote: > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now > possible to define IRQ mapping functions on a per PCI host bridge basis. > > Actual IRQ allocation is carried out by the pci_assign_irq() function in > pci_device_probe() - to make sure a device is assigned an IRQ only if it > is probed (ie match a driver); it retrieves a struct pci_host_bridge* > for a given PCI device and through {map/swizzle}_irq() it carries out > the PCI IRQ allocation. > > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks > and pci_assign_irq() to deal with legacy IRQs in > > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in > pci_device_probe()") > > did not take into account that the IDE subsystem relies on a special > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force > devices probe ordering by sidestepping the core PCI bus layer entirely > (and therefore pci_device_probe()) by executing the registered IDE PCI > drivers probe routines (ie registered with ide_pci_register_driver()) > through ide_scan_pcidev(). > > Since this IDE PCI specific probe mechanism bypasses the PCI core > code generic probing (ie pci_device_probe()) it also misses the > pci_assign_irq() call (among other PCI initialization functions); > this triggers IDE PCI devices initialization failures caused > by the missing IRQ allocation: > > ide0: disabled, no IRQ > ide0: failed to initialize IDE interface > ide0: disabling port > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] > cmd64x 0000:00:02.0: can't reserve resources > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 > ide_generic: please use "probe_mask=0x3f" module parameter for probing > all legacy ISA IDE ports > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 > sysfs: cannot create duplicate filename '/class/ide_port/ide0' > ... > > Trace: > [<fffffc00003308a0>] __warn+0x160/0x190 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 > [<fffffc00005ba690>] device_create+0x50/0x70 > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005ba2a0>] device_register+0x20/0x50 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005df944>] ide_host_add+0x64/0xe0 > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 > [<fffffc0000310288>] do_one_initcall+0x68/0x260 > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > ---[ end trace 24a70433c3e4d374 ]--- > ide0: disabling port > > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the > ide_scan_pcidev() function before probing the IDE PCI drivers, so that > IRQs for a given PCI device are allocated for the IDE PCI drivers to > use them for device configuration. > > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> > Cc: David S. Miller <davem@davemloft.net> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Matt Turner <mattst88@gmail.com> Tested-by: Guenter Roeck <linux@roeck-us.net> Failure in -next is different and worse. From the -next build log: ide0: failed to initialize IDE interface ide0: disabling port cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] cmd64x 0000:00:02.0: can't reserve resources CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at lib/kobject.c:244 kobject_add_internal+0x17c/0x490 kobject_add_internal failed for ide0 (error: -2 parent: ide_port) Modules linked in: CPU: 0 PID: 1 Comm: swapper Not tainted 4.14.0-rc2-next-20170929 #1 ... and various other kobject related errors. The test then hangs on reboot. This patch fixes that problem as well. Guenter > --- > v1->v2 > > - Moved fix from PCI core code to IDE subsystem following further > debugging and mailing list discussions > - Rebased against v4.14-rc3 > > drivers/ide/ide-scan-pci.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c > index 86aa88a..acf8748 100644 > --- a/drivers/ide/ide-scan-pci.c > +++ b/drivers/ide/ide-scan-pci.c > @@ -56,6 +56,7 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > { > struct list_head *l; > struct pci_driver *d; > + int ret; > > list_for_each(l, &ide_pci_drivers) { > d = list_entry(l, struct pci_driver, node); > @@ -63,10 +64,14 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > const struct pci_device_id *id = > pci_match_id(d->id_table, dev); > > - if (id != NULL && d->probe(dev, id) >= 0) { > - dev->driver = d; > - pci_dev_get(dev); > - return 1; > + if (id != NULL) { > + pci_assign_irq(dev); > + ret = d->probe(dev, id); > + if (ret >= 0) { > + dev->driver = d; > + pci_dev_get(dev); > + return 1; > + } > } > } > } >
The patch below now only touches drivers/ide/ide-scan-pci.c. It fixes a problem introduced via the PCI tree, so I'd be happy to merge the fix as well, given an ack from you, Dave. Or you can merge it yourself with my ack: Acked-by: Bjorn Helgaas <bhelgaas@google.com> It mentions that it fixes 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()"), which appeared in v4.13. But the problem wasn't actually exposed until 0e4c2eeb758a ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks"), which appeared in v4.14-rc1. So as long as we get this into v4.14, I don't think we need a stable backport to v4.13. Note that there are other potential problems with the ide_scan_pcidev() path. For example, the normal PCI probe path in pci_device_probe() calls pcibios_alloc_irq(), which may do ACPI IRQ setup and potentially change dev->irq. ide_scan_pcidev() does not do that, so there may be cases where the driver gets the wrong IRQ. On Mon, Oct 02, 2017 at 11:52:47AM +0100, Lorenzo Pieralisi wrote: > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now > possible to define IRQ mapping functions on a per PCI host bridge basis. > > Actual IRQ allocation is carried out by the pci_assign_irq() function in > pci_device_probe() - to make sure a device is assigned an IRQ only if it > is probed (ie match a driver); it retrieves a struct pci_host_bridge* > for a given PCI device and through {map/swizzle}_irq() it carries out > the PCI IRQ allocation. > > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks > and pci_assign_irq() to deal with legacy IRQs in > > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in > pci_device_probe()") > > did not take into account that the IDE subsystem relies on a special > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force > devices probe ordering by sidestepping the core PCI bus layer entirely > (and therefore pci_device_probe()) by executing the registered IDE PCI > drivers probe routines (ie registered with ide_pci_register_driver()) > through ide_scan_pcidev(). > > Since this IDE PCI specific probe mechanism bypasses the PCI core > code generic probing (ie pci_device_probe()) it also misses the > pci_assign_irq() call (among other PCI initialization functions); > this triggers IDE PCI devices initialization failures caused > by the missing IRQ allocation: > > ide0: disabled, no IRQ > ide0: failed to initialize IDE interface > ide0: disabling port > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] > cmd64x 0000:00:02.0: can't reserve resources > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 > ide_generic: please use "probe_mask=0x3f" module parameter for probing > all legacy ISA IDE ports > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 > sysfs: cannot create duplicate filename '/class/ide_port/ide0' > ... > > Trace: > [<fffffc00003308a0>] __warn+0x160/0x190 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 > [<fffffc00005ba690>] device_create+0x50/0x70 > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005ba2a0>] device_register+0x20/0x50 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005df944>] ide_host_add+0x64/0xe0 > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 > [<fffffc0000310288>] do_one_initcall+0x68/0x260 > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > ---[ end trace 24a70433c3e4d374 ]--- > ide0: disabling port > > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the > ide_scan_pcidev() function before probing the IDE PCI drivers, so that > IRQs for a given PCI device are allocated for the IDE PCI drivers to > use them for device configuration. > > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> > Cc: David S. Miller <davem@davemloft.net> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Matt Turner <mattst88@gmail.com> > --- > v1->v2 > > - Moved fix from PCI core code to IDE subsystem following further > debugging and mailing list discussions > - Rebased against v4.14-rc3 > > drivers/ide/ide-scan-pci.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c > index 86aa88a..acf8748 100644 > --- a/drivers/ide/ide-scan-pci.c > +++ b/drivers/ide/ide-scan-pci.c > @@ -56,6 +56,7 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > { > struct list_head *l; > struct pci_driver *d; > + int ret; > > list_for_each(l, &ide_pci_drivers) { > d = list_entry(l, struct pci_driver, node); > @@ -63,10 +64,14 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > const struct pci_device_id *id = > pci_match_id(d->id_table, dev); > > - if (id != NULL && d->probe(dev, id) >= 0) { > - dev->driver = d; > - pci_dev_get(dev); > - return 1; > + if (id != NULL) { > + pci_assign_irq(dev); > + ret = d->probe(dev, id); > + if (ret >= 0) { > + dev->driver = d; > + pci_dev_get(dev); > + return 1; > + } > } > } > } > -- > 2.4.12 >
From: Bjorn Helgaas <helgaas@kernel.org> Date: Mon, 2 Oct 2017 18:53:20 -0500 > The patch below now only touches drivers/ide/ide-scan-pci.c. > > It fixes a problem introduced via the PCI tree, so I'd be happy to > merge the fix as well, given an ack from you, Dave. Or you can merge > it yourself with my ack: > > Acked-by: Bjorn Helgaas <bhelgaas@google.com> Bjorn, feel free to merge this via the PCI tree with my ACK: Acked-by: David S. Miller <davem@davemloft.net>
On Monday, October 02, 2017 11:52:47 AM Lorenzo Pieralisi wrote: > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now > possible to define IRQ mapping functions on a per PCI host bridge basis. > > Actual IRQ allocation is carried out by the pci_assign_irq() function in > pci_device_probe() - to make sure a device is assigned an IRQ only if it > is probed (ie match a driver); it retrieves a struct pci_host_bridge* > for a given PCI device and through {map/swizzle}_irq() it carries out > the PCI IRQ allocation. > > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks > and pci_assign_irq() to deal with legacy IRQs in > > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in > pci_device_probe()") > > did not take into account that the IDE subsystem relies on a special > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force > devices probe ordering by sidestepping the core PCI bus layer entirely > (and therefore pci_device_probe()) by executing the registered IDE PCI > drivers probe routines (ie registered with ide_pci_register_driver()) > through ide_scan_pcidev(). > > Since this IDE PCI specific probe mechanism bypasses the PCI core > code generic probing (ie pci_device_probe()) it also misses the > pci_assign_irq() call (among other PCI initialization functions); > this triggers IDE PCI devices initialization failures caused > by the missing IRQ allocation: > > ide0: disabled, no IRQ > ide0: failed to initialize IDE interface > ide0: disabling port > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] > cmd64x 0000:00:02.0: can't reserve resources > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 > ide_generic: please use "probe_mask=0x3f" module parameter for probing > all legacy ISA IDE ports > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 > sysfs: cannot create duplicate filename '/class/ide_port/ide0' > ... > > Trace: > [<fffffc00003308a0>] __warn+0x160/0x190 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 > [<fffffc00005ba690>] device_create+0x50/0x70 > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005ba2a0>] device_register+0x20/0x50 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005df944>] ide_host_add+0x64/0xe0 > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 > [<fffffc0000310288>] do_one_initcall+0x68/0x260 > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > ---[ end trace 24a70433c3e4d374 ]--- > ide0: disabling port > > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the > ide_scan_pcidev() function before probing the IDE PCI drivers, so that > IRQs for a given PCI device are allocated for the IDE PCI drivers to > use them for device configuration. > > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> > Cc: David S. Miller <davem@davemloft.net> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Matt Turner <mattst88@gmail.com> Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > v1->v2 > > - Moved fix from PCI core code to IDE subsystem following further > debugging and mailing list discussions > - Rebased against v4.14-rc3 > > drivers/ide/ide-scan-pci.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c > index 86aa88a..acf8748 100644 > --- a/drivers/ide/ide-scan-pci.c > +++ b/drivers/ide/ide-scan-pci.c > @@ -56,6 +56,7 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > { > struct list_head *l; > struct pci_driver *d; > + int ret; > > list_for_each(l, &ide_pci_drivers) { > d = list_entry(l, struct pci_driver, node); > @@ -63,10 +64,14 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > const struct pci_device_id *id = > pci_match_id(d->id_table, dev); > > - if (id != NULL && d->probe(dev, id) >= 0) { > - dev->driver = d; > - pci_dev_get(dev); > - return 1; > + if (id != NULL) { > + pci_assign_irq(dev); > + ret = d->probe(dev, id); > + if (ret >= 0) { > + dev->driver = d; > + pci_dev_get(dev); > + return 1; > + } > } > } > } Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Tuesday, October 03, 2017 11:39:55 AM Bartlomiej Zolnierkiewicz wrote: > On Monday, October 02, 2017 11:52:47 AM Lorenzo Pieralisi wrote: > > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now > > possible to define IRQ mapping functions on a per PCI host bridge basis. > > > > Actual IRQ allocation is carried out by the pci_assign_irq() function in > > pci_device_probe() - to make sure a device is assigned an IRQ only if it > > is probed (ie match a driver); it retrieves a struct pci_host_bridge* > > for a given PCI device and through {map/swizzle}_irq() it carries out > > the PCI IRQ allocation. > > > > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks > > and pci_assign_irq() to deal with legacy IRQs in > > > > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in > > pci_device_probe()") > > > > did not take into account that the IDE subsystem relies on a special > > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force > > devices probe ordering by sidestepping the core PCI bus layer entirely > > (and therefore pci_device_probe()) by executing the registered IDE PCI > > drivers probe routines (ie registered with ide_pci_register_driver()) > > through ide_scan_pcidev(). > > > > Since this IDE PCI specific probe mechanism bypasses the PCI core > > code generic probing (ie pci_device_probe()) it also misses the > > pci_assign_irq() call (among other PCI initialization functions); > > this triggers IDE PCI devices initialization failures caused > > by the missing IRQ allocation: > > > > ide0: disabled, no IRQ > > ide0: failed to initialize IDE interface > > ide0: disabling port > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) > > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] > > cmd64x 0000:00:02.0: can't reserve resources > > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 > > ide_generic: please use "probe_mask=0x3f" module parameter for probing > > all legacy ISA IDE ports > > ------------[ cut here ]------------ > > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 > > sysfs: cannot create duplicate filename '/class/ide_port/ide0' > > ... > > > > Trace: > > [<fffffc00003308a0>] __warn+0x160/0x190 > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 > > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 > > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 > > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 > > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 > > [<fffffc00005ba690>] device_create+0x50/0x70 > > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 > > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > > [<fffffc00005ba2a0>] device_register+0x20/0x50 > > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > > [<fffffc00005df944>] ide_host_add+0x64/0xe0 > > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 > > [<fffffc0000310288>] do_one_initcall+0x68/0x260 > > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 > > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > > > ---[ end trace 24a70433c3e4d374 ]--- > > ide0: disabling port > > > > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the > > ide_scan_pcidev() function before probing the IDE PCI drivers, so that > > IRQs for a given PCI device are allocated for the IDE PCI drivers to > > use them for device configuration. > > > > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") > > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net > > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > > Reported-by: Guenter Roeck <linux@roeck-us.net> > > Cc: Bjorn Helgaas <bhelgaas@google.com> > > Cc: Richard Henderson <rth@twiddle.net> > > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> > > Cc: David S. Miller <davem@davemloft.net> > > Cc: Guenter Roeck <linux@roeck-us.net> > > Cc: Matt Turner <mattst88@gmail.com> > > Reviewed-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> IDE specific problems uncovered by pci_assign_irq() change have been addressed in separate patches, please see: http://patchwork.ozlabs.org/patch/820859/ http://patchwork.ozlabs.org/patch/820870/ Guenter, could you please verify them (I have only compile tested them)? [ Just revert Lorenzo's fix and apply both my patches. ] Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Tuesday, October 03, 2017 02:10:11 PM Bartlomiej Zolnierkiewicz wrote: > IDE specific problems uncovered by pci_assign_irq() change have been > addressed in separate patches, please see: > > http://patchwork.ozlabs.org/patch/820859/ > http://patchwork.ozlabs.org/patch/820870/ v2 of the 2nd patch: http://patchwork.ozlabs.org/patch/820875/ > Guenter, could you please verify them (I have only compile tested > them)? [ Just revert Lorenzo's fix and apply both my patches. ] Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
On Mon, Oct 02, 2017 at 11:52:47AM +0100, Lorenzo Pieralisi wrote: > Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now > possible to define IRQ mapping functions on a per PCI host bridge basis. > > Actual IRQ allocation is carried out by the pci_assign_irq() function in > pci_device_probe() - to make sure a device is assigned an IRQ only if it > is probed (ie match a driver); it retrieves a struct pci_host_bridge* > for a given PCI device and through {map/swizzle}_irq() it carries out > the PCI IRQ allocation. > > The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks > and pci_assign_irq() to deal with legacy IRQs in > > commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in > pci_device_probe()") > > did not take into account that the IDE subsystem relies on a special > purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force > devices probe ordering by sidestepping the core PCI bus layer entirely > (and therefore pci_device_probe()) by executing the registered IDE PCI > drivers probe routines (ie registered with ide_pci_register_driver()) > through ide_scan_pcidev(). > > Since this IDE PCI specific probe mechanism bypasses the PCI core > code generic probing (ie pci_device_probe()) it also misses the > pci_assign_irq() call (among other PCI initialization functions); > this triggers IDE PCI devices initialization failures caused > by the missing IRQ allocation: > > ide0: disabled, no IRQ > ide0: failed to initialize IDE interface > ide0: disabling port > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) > CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] > cmd64x 0000:00:02.0: can't reserve resources > CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 > ide_generic: please use "probe_mask=0x3f" module parameter for probing > all legacy ISA IDE ports > ------------[ cut here ]------------ > WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 > sysfs: cannot create duplicate filename '/class/ide_port/ide0' > ... > > Trace: > [<fffffc00003308a0>] __warn+0x160/0x190 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 > [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 > [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 > [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 > [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 > [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 > [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 > [<fffffc00005ba690>] device_create+0x50/0x70 > [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005ba2a0>] device_register+0x20/0x50 > [<fffffc00005df330>] ide_host_register+0x450/0xa00 > [<fffffc00005df944>] ide_host_add+0x64/0xe0 > [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 > [<fffffc0000310288>] do_one_initcall+0x68/0x260 > [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 > [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 > > ---[ end trace 24a70433c3e4d374 ]--- > ide0: disabling port > > Fix the IRQ allocation issue by adding a pci_assign_irq() call in the > ide_scan_pcidev() function before probing the IDE PCI drivers, so that > IRQs for a given PCI device are allocated for the IDE PCI drivers to > use them for device configuration. > > Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") > Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net > Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Cc: Bjorn Helgaas <bhelgaas@google.com> > Cc: Richard Henderson <rth@twiddle.net> > Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> > Cc: David S. Miller <davem@davemloft.net> > Cc: Guenter Roeck <linux@roeck-us.net> > Cc: Matt Turner <mattst88@gmail.com> Applied with Guenter's tested-by, Bartlomiej's reviewed-by, and David's ack to for-linus for v4.14, thanks! I edited the changelog as follows (so Google can find this discussion): ide: fix IRQ assignment for PCI bus order probing We used to assign IRQs for all devices at boot-time, before any drivers claimed devices. The following commits: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") 0e4c2eeb758a ("alpha/PCI: Replace pci_fixup_irqs() call with host bridge IRQ mapping hooks") changed this so we now call pci_assign_irq() from pci_device_probe() when we call a driver's probe method. The ide_scan_pcibus() path (enabled by CONFIG_IDEPCI_PCIBUS_ORDER) bypasses pci_device_probe() so it can guarantee devices are claimed in order of PCI bus address. It calls the driver's probe method directly, so it misses the pci_assign_irq() call (and other PCI initialization functions), which causes failures like this: > --- > v1->v2 > > - Moved fix from PCI core code to IDE subsystem following further > debugging and mailing list discussions > - Rebased against v4.14-rc3 > > drivers/ide/ide-scan-pci.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c > index 86aa88a..acf8748 100644 > --- a/drivers/ide/ide-scan-pci.c > +++ b/drivers/ide/ide-scan-pci.c > @@ -56,6 +56,7 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > { > struct list_head *l; > struct pci_driver *d; > + int ret; > > list_for_each(l, &ide_pci_drivers) { > d = list_entry(l, struct pci_driver, node); > @@ -63,10 +64,14 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) > const struct pci_device_id *id = > pci_match_id(d->id_table, dev); > > - if (id != NULL && d->probe(dev, id) >= 0) { > - dev->driver = d; > - pci_dev_get(dev); > - return 1; > + if (id != NULL) { > + pci_assign_irq(dev); > + ret = d->probe(dev, id); > + if (ret >= 0) { > + dev->driver = d; > + pci_dev_get(dev); > + return 1; > + } > } > } > } > -- > 2.4.12 >
diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c index 86aa88a..acf8748 100644 --- a/drivers/ide/ide-scan-pci.c +++ b/drivers/ide/ide-scan-pci.c @@ -56,6 +56,7 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) { struct list_head *l; struct pci_driver *d; + int ret; list_for_each(l, &ide_pci_drivers) { d = list_entry(l, struct pci_driver, node); @@ -63,10 +64,14 @@ static int __init ide_scan_pcidev(struct pci_dev *dev) const struct pci_device_id *id = pci_match_id(d->id_table, dev); - if (id != NULL && d->probe(dev, id) >= 0) { - dev->driver = d; - pci_dev_get(dev); - return 1; + if (id != NULL) { + pci_assign_irq(dev); + ret = d->probe(dev, id); + if (ret >= 0) { + dev->driver = d; + pci_dev_get(dev); + return 1; + } } } }
Through struct pci_host_bridge->{map/swizzle}_irq() hooks is now possible to define IRQ mapping functions on a per PCI host bridge basis. Actual IRQ allocation is carried out by the pci_assign_irq() function in pci_device_probe() - to make sure a device is assigned an IRQ only if it is probed (ie match a driver); it retrieves a struct pci_host_bridge* for a given PCI device and through {map/swizzle}_irq() it carries out the PCI IRQ allocation. The code conversion to struct pci_host_bridge {map/swizzle}_irq() hooks and pci_assign_irq() to deal with legacy IRQs in commit 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") did not take into account that the IDE subsystem relies on a special purpose IDE PCI layer, configured by CONFIG_IDEPCI_PCIBUS_ORDER to force devices probe ordering by sidestepping the core PCI bus layer entirely (and therefore pci_device_probe()) by executing the registered IDE PCI drivers probe routines (ie registered with ide_pci_register_driver()) through ide_scan_pcidev(). Since this IDE PCI specific probe mechanism bypasses the PCI core code generic probing (ie pci_device_probe()) it also misses the pci_assign_irq() call (among other PCI initialization functions); this triggers IDE PCI devices initialization failures caused by the missing IRQ allocation: ide0: disabled, no IRQ ide0: failed to initialize IDE interface ide0: disabling port cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07) CMD64x_IDE 0000:00:02.0: BAR 0: can't reserve [io 0x8050-0x8057] cmd64x 0000:00:02.0: can't reserve resources CMD64x_IDE: probe of 0000:00:02.0 failed with error -16 ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports ------------[ cut here ]------------ WARNING: CPU: 0 PID: 1 at fs/sysfs/dir.c:31 sysfs_warn_dup+0x94/0xd0 sysfs: cannot create duplicate filename '/class/ide_port/ide0' ... Trace: [<fffffc00003308a0>] __warn+0x160/0x190 [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 [<fffffc0000330928>] warn_slowpath_fmt+0x58/0x70 [<fffffc000048c9f4>] sysfs_warn_dup+0x94/0xd0 [<fffffc0000486d40>] kernfs_path_from_node+0x30/0x60 [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 [<fffffc00004874ac>] kernfs_put+0x16c/0x2c0 [<fffffc000048d010>] sysfs_do_create_link_sd.isra.2+0x100/0x120 [<fffffc00005b9d64>] device_add+0x2a4/0x7c0 [<fffffc00005ba5cc>] device_create_groups_vargs+0x14c/0x170 [<fffffc00005ba518>] device_create_groups_vargs+0x98/0x170 [<fffffc00005ba690>] device_create+0x50/0x70 [<fffffc00005df36c>] ide_host_register+0x48c/0xa00 [<fffffc00005df330>] ide_host_register+0x450/0xa00 [<fffffc00005ba2a0>] device_register+0x20/0x50 [<fffffc00005df330>] ide_host_register+0x450/0xa00 [<fffffc00005df944>] ide_host_add+0x64/0xe0 [<fffffc000079b41c>] kobject_uevent_env+0x16c/0x710 [<fffffc0000310288>] do_one_initcall+0x68/0x260 [<fffffc00007b13bc>] kernel_init+0x1c/0x1a0 [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 [<fffffc0000311868>] ret_from_kernel_thread+0x18/0x20 [<fffffc00007b13a0>] kernel_init+0x0/0x1a0 ---[ end trace 24a70433c3e4d374 ]--- ide0: disabling port Fix the IRQ allocation issue by adding a pci_assign_irq() call in the ide_scan_pcidev() function before probing the IDE PCI drivers, so that IRQs for a given PCI device are allocated for the IDE PCI drivers to use them for device configuration. Fixes: 30fdfb929e82 ("PCI: Add a call to pci_assign_irq() in pci_device_probe()") Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Reported-by: Guenter Roeck <linux@roeck-us.net> Cc: Bjorn Helgaas <bhelgaas@google.com> Cc: Richard Henderson <rth@twiddle.net> Cc: Ivan Kokshaysky <ink@jurassic.park.msu.ru> Cc: David S. Miller <davem@davemloft.net> Cc: Guenter Roeck <linux@roeck-us.net> Cc: Matt Turner <mattst88@gmail.com> --- v1->v2 - Moved fix from PCI core code to IDE subsystem following further debugging and mailing list discussions - Rebased against v4.14-rc3 drivers/ide/ide-scan-pci.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)