* Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: Bartlomiej Zolnierkiewicz @ 2017-10-03 12:10 UTC (permalink / raw)
To: David S. Miller, Guenter Roeck
Cc: Lorenzo Pieralisi, linux-pci, linux-kernel, linux-ide,
Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky, Matt Turner
In-Reply-To: <4605844.iOhxHctWQT@amdc3058>
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
^ permalink raw reply
* [PATCH] ide: pci: free PCI BARs on initialization failure
From: Bartlomiej Zolnierkiewicz @ 2017-10-03 11:59 UTC (permalink / raw)
To: David S. Miller
Cc: Guenter Roeck, Lorenzo Pieralisi, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, Matt Turner, linux-ide,
linux-kernel
In-Reply-To: <CGME20171003115909epcas2p21c25b1c67678df6fd6c26386345abf9a@epcas2p2.samsung.com>
Recent pci_assign_irq() changes uncovered a problem with missing
freeing of PCI BARs on PCI IDE host initialization failure:
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
Fix the problem by adding missing freeing of PCI BARs to
ide_setup_pci_controller() and ide_pci_init_two().
Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net
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: Guenter Roeck <linux@roeck-us.net>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
This should fix problem with reserving PCI resources on a secondary
PCI device probe attempt (please test when pci_assign_irq() fix is
not present and "[PATCH] ide: add missing hwif->portdev freeing on
hwif_init() failure" is applied).
drivers/ide/setup-pci.c | 61 +++++++++++++++++++++++++++++-------------------
1 file changed, 38 insertions(+), 23 deletions(-)
Index: b/drivers/ide/setup-pci.c
===================================================================
--- a/drivers/ide/setup-pci.c 2017-10-03 13:43:29.567778114 +0200
+++ b/drivers/ide/setup-pci.c 2017-10-03 13:43:54.131778138 +0200
@@ -179,6 +179,7 @@ EXPORT_SYMBOL_GPL(ide_setup_pci_noise);
/**
* ide_pci_enable - do PCI enables
* @dev: PCI device
+ * @bars: PCI BARs mask
* @d: IDE port info
*
* Enable the IDE PCI device. We attempt to enable the device in full
@@ -189,9 +190,10 @@ EXPORT_SYMBOL_GPL(ide_setup_pci_noise);
* Returns zero on success or an error code
*/
-static int ide_pci_enable(struct pci_dev *dev, const struct ide_port_info *d)
+static int ide_pci_enable(struct pci_dev *dev, int bars,
+ const struct ide_port_info *d)
{
- int ret, bars;
+ int ret;
if (pci_enable_device(dev)) {
ret = pci_enable_device_io(dev);
@@ -216,18 +218,6 @@ static int ide_pci_enable(struct pci_dev
goto out;
}
- if (d->host_flags & IDE_HFLAG_SINGLE)
- bars = (1 << 2) - 1;
- else
- bars = (1 << 4) - 1;
-
- if ((d->host_flags & IDE_HFLAG_NO_DMA) == 0) {
- if (d->host_flags & IDE_HFLAG_CS5520)
- bars |= (1 << 2);
- else
- bars |= (1 << 4);
- }
-
ret = pci_request_selected_regions(dev, bars, d->name);
if (ret < 0)
printk(KERN_ERR "%s %s: can't reserve resources\n",
@@ -403,6 +393,7 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
/**
* ide_setup_pci_controller - set up IDE PCI
* @dev: PCI device
+ * @bars: PCI BARs mask
* @d: IDE port info
* @noisy: verbose flag
*
@@ -411,7 +402,7 @@ int ide_hwif_setup_dma(ide_hwif_t *hwif,
* and enables it if need be
*/
-static int ide_setup_pci_controller(struct pci_dev *dev,
+static int ide_setup_pci_controller(struct pci_dev *dev, int bars,
const struct ide_port_info *d, int noisy)
{
int ret;
@@ -420,7 +411,7 @@ static int ide_setup_pci_controller(stru
if (noisy)
ide_setup_pci_noise(dev, d);
- ret = ide_pci_enable(dev, d);
+ ret = ide_pci_enable(dev, bars, d);
if (ret < 0)
goto out;
@@ -428,16 +419,18 @@ static int ide_setup_pci_controller(stru
if (ret < 0) {
printk(KERN_ERR "%s %s: error accessing PCI regs\n",
d->name, pci_name(dev));
- goto out;
+ goto out_free_bars;
}
if (!(pcicmd & PCI_COMMAND_IO)) { /* is device disabled? */
ret = ide_pci_configure(dev, d);
if (ret < 0)
- goto out;
+ goto out_free_bars;
printk(KERN_INFO "%s %s: device enabled (Linux)\n",
d->name, pci_name(dev));
}
+out_free_bars:
+ pci_release_selected_regions(dev, bars);
out:
return ret;
}
@@ -540,13 +533,28 @@ int ide_pci_init_two(struct pci_dev *dev
{
struct pci_dev *pdev[] = { dev1, dev2 };
struct ide_host *host;
- int ret, i, n_ports = dev2 ? 4 : 2;
+ int ret, i, n_ports = dev2 ? 4 : 2, bars;
struct ide_hw hw[4], *hws[] = { NULL, NULL, NULL, NULL };
+ if (d->host_flags & IDE_HFLAG_SINGLE)
+ bars = (1 << 2) - 1;
+ else
+ bars = (1 << 4) - 1;
+
+ if ((d->host_flags & IDE_HFLAG_NO_DMA) == 0) {
+ if (d->host_flags & IDE_HFLAG_CS5520)
+ bars |= (1 << 2);
+ else
+ bars |= (1 << 4);
+ }
+
for (i = 0; i < n_ports / 2; i++) {
- ret = ide_setup_pci_controller(pdev[i], d, !i);
- if (ret < 0)
+ ret = ide_setup_pci_controller(pdev[i], bars, d, !i);
+ if (ret < 0) {
+ if (i == 1)
+ pci_release_selected_regions(pdev[0], bars);
goto out;
+ }
ide_pci_setup_ports(pdev[i], d, &hw[i*2], &hws[i*2]);
}
@@ -554,7 +562,7 @@ int ide_pci_init_two(struct pci_dev *dev
host = ide_host_alloc(d, hws, n_ports);
if (host == NULL) {
ret = -ENOMEM;
- goto out;
+ goto out_free_bars;
}
host->dev[0] = &dev1->dev;
@@ -576,7 +584,7 @@ int ide_pci_init_two(struct pci_dev *dev
* do_ide_setup_pci_device() on the first device!
*/
if (ret < 0)
- goto out;
+ goto out_free_bars;
/* fixup IRQ */
if (ide_pci_is_in_compatibility_mode(pdev[i])) {
@@ -589,6 +597,13 @@ int ide_pci_init_two(struct pci_dev *dev
ret = ide_host_register(host, d, hws);
if (ret)
ide_host_free(host);
+ else
+ goto out;
+
+out_free_bars:
+ i = n_ports / 2;
+ while (i--)
+ pci_release_selected_regions(pdev[i], bars);
out:
return ret;
}
^ permalink raw reply
* [PATCH v2] ide: add missing hwif->portdev freeing on hwif_init() failure
From: Bartlomiej Zolnierkiewicz @ 2017-10-03 11:18 UTC (permalink / raw)
To: David S. Miller
Cc: Guenter Roeck, Lorenzo Pieralisi, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, Matt Turner, linux-ide,
linux-kernel
In-Reply-To: <CGME20171003111849epcas1p3335e2bc56e5aafdeadc340301284b2b8@epcas1p3.samsung.com>
Recent pci_assign_irq() changes uncovered a problem with missing
freeing of ide_port class instance on hwif_init() failure in
ide_host_register():
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 problem by adding missing code to ide_host_register().
Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net
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: Guenter Roeck <linux@roeck-us.net>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
This should fix sysfs warning and probing of ide_generic driver
driven ports in case of hwif_init() failure (i.e. no IRQ issue
when pci_assign_irq() fix is not present).
v2:
- Fixed typo in the patch description: pci_assing_irq() -> pci_assign_irq().
drivers/ide/ide-probe.c | 1 +
1 file changed, 1 insertion(+)
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c 2017-10-02 15:08:16.658662459 +0200
+++ b/drivers/ide/ide-probe.c 2017-10-03 11:59:36.867189557 +0200
@@ -1451,6 +1451,7 @@ int ide_host_register(struct ide_host *h
if (hwif_init(hwif) == 0) {
printk(KERN_INFO "%s: failed to initialize IDE "
"interface\n", hwif->name);
+ device_unregister(hwif->portdev);
device_unregister(&hwif->gendev);
ide_disable_port(hwif);
continue;
^ permalink raw reply
* [PATCH] ide: add missing hwif->portdev freeing on hwif_init() failure
From: Bartlomiej Zolnierkiewicz @ 2017-10-03 10:20 UTC (permalink / raw)
To: David S. Miller
Cc: Guenter Roeck, Lorenzo Pieralisi, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, Matt Turner, linux-ide,
linux-kernel
In-Reply-To: <CGME20171003102037epcas2p1ed603f6c3a335ac34438ce32b5421a00@epcas2p1.samsung.com>
Recent pci_assing_irq() changes uncovered a problem with missing
freeing of ide_port class instance on hwif_init() failure in
ide_host_register():
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 problem by adding missing code to ide_host_register().
Link: http://lkml.kernel.org/r/32ec730f-c1b0-5584-cd35-f8a809122b96@roeck-us.net
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: Guenter Roeck <linux@roeck-us.net>
Cc: Matt Turner <mattst88@gmail.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
---
This should fix sysfs warning and probing of ide_generic driver
driven ports in case of hwif_init() failure (i.e. no IRQ issue
when pci_assing_irq() fix is not present).
drivers/ide/ide-probe.c | 1 +
1 file changed, 1 insertion(+)
Index: b/drivers/ide/ide-probe.c
===================================================================
--- a/drivers/ide/ide-probe.c 2017-10-02 15:08:16.658662459 +0200
+++ b/drivers/ide/ide-probe.c 2017-10-03 11:59:36.867189557 +0200
@@ -1451,6 +1451,7 @@ int ide_host_register(struct ide_host *h
if (hwif_init(hwif) == 0) {
printk(KERN_INFO "%s: failed to initialize IDE "
"interface\n", hwif->name);
+ device_unregister(hwif->portdev);
device_unregister(&hwif->gendev);
ide_disable_port(hwif);
continue;
^ permalink raw reply
* Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: Bartlomiej Zolnierkiewicz @ 2017-10-03 9:39 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, linux-kernel, linux-ide, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, David S. Miller,
Guenter Roeck, Matt Turner
In-Reply-To: <1506941567-10762-1-git-send-email-lorenzo.pieralisi@arm.com>
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
^ permalink raw reply
* Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: David Miller @ 2017-10-03 3:09 UTC (permalink / raw)
To: helgaas
Cc: lorenzo.pieralisi, linux-pci, linux-kernel, linux-ide, bhelgaas,
rth, ink, linux, mattst88
In-Reply-To: <20171002235320.GA27400@bhelgaas-glaptop.roam.corp.google.com>
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>
^ permalink raw reply
* downloads for SAS2008 based HBAs
From: Eyal Lebedinsky @ 2017-10-03 0:37 UTC (permalink / raw)
To: list linux-ide
First, is this the correct list? I looked at the archives and see many SATA posts.
I am about to acquire an LSI 9211-8i, and then flash it to the IT fw.
However, I find the official sites no longer provide the necessary files.
Is there an official (or otherwise trusted) repository for these files?
TIA
--
Eyal Lebedinsky (eyal@eyal.emu.id.au)
^ permalink raw reply
* Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: Bjorn Helgaas @ 2017-10-02 23:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, linux-kernel, linux-ide, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, David S. Miller,
Guenter Roeck, Matt Turner
In-Reply-To: <1506941567-10762-1-git-send-email-lorenzo.pieralisi@arm.com>
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
>
^ permalink raw reply
* Re: [PATCH] ahci: don't ignore result code of ahci_reset_controller()
From: Tejun Heo @ 2017-10-02 19:21 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-ide, graeme.gregory, leif.lindholm, daniel.thompson
In-Reply-To: <20171002183124.17003-1-ard.biesheuvel@linaro.org>
On Mon, Oct 02, 2017 at 07:31:24PM +0100, Ard Biesheuvel wrote:
> ahci_pci_reset_controller() calls ahci_reset_controller(), which may
> fail, but ignores the result code and always returns success. This
> may result in failures like below
Applied to libata/for-4.14-fixes.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH] ahci: don't ignore result code of ahci_reset_controller()
From: Ard Biesheuvel @ 2017-10-02 18:59 UTC (permalink / raw)
To: Tejun Heo; +Cc: IDE-ML, Graeme Gregory, Leif Lindholm, Daniel Thompson
In-Reply-To: <20171002185450.GC3301751@devbig577.frc2.facebook.com>
On 2 October 2017 at 19:54, Tejun Heo <tj@kernel.org> wrote:
> Hello, Ard.
>
> On Mon, Oct 02, 2017 at 07:31:24PM +0100, Ard Biesheuvel wrote:
>> ahci_pci_reset_controller() calls ahci_reset_controller(), which may
>> fail, but ignores the result code and always returns success. This
>> may result in failures like below
>>
>> ahci 0000:02:00.0: version 3.0
>> ahci 0000:02:00.0: enabling device (0000 -> 0003)
>> ahci 0000:02:00.0: SSS flag set, parallel bus scan disabled
>> ahci 0000:02:00.0: controller reset failed (0xffffffff)
>> ahci 0000:02:00.0: failed to stop engine (-5)
>> ... repeated many times ...
>> ahci 0000:02:00.0: failed to stop engine (-5)
>> Unable to handle kernel paging request at virtual address ffff0000093f9018
>> ...
>> PC is at ahci_stop_engine+0x5c/0xd8 [libahci]
>> LR is at ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
>> ...
>> [<ffff000000a17014>] ahci_stop_engine+0x5c/0xd8 [libahci]
>> [<ffff000000a196b4>] ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
>> [<ffff000000a197d8>] ahci_init_controller+0x80/0x168 [libahci]
>> [<ffff000000a260f8>] ahci_pci_init_controller+0x60/0x68 [ahci]
>> [<ffff000000a26f94>] ahci_init_one+0x75c/0xd88 [ahci]
>> [<ffff000008430324>] local_pci_probe+0x3c/0xb8
>> [<ffff000008431728>] pci_device_probe+0x138/0x170
>> [<ffff000008585e54>] driver_probe_device+0x2dc/0x458
>> [<ffff0000085860e4>] __driver_attach+0x114/0x118
>> [<ffff000008583ca8>] bus_for_each_dev+0x60/0xa0
>> [<ffff000008585638>] driver_attach+0x20/0x28
>> [<ffff0000085850b0>] bus_add_driver+0x1f0/0x2a8
>> [<ffff000008586ae0>] driver_register+0x60/0xf8
>> [<ffff00000842f9b4>] __pci_register_driver+0x3c/0x48
>> [<ffff000000a3001c>] ahci_pci_driver_init+0x1c/0x1000 [ahci]
>> [<ffff000008083918>] do_one_initcall+0x38/0x120
>>
>> where an obvious hardware level failure results in an unnecessary 15 second
>> delay and a subsequent crash.
>
> I'm not sure the retries are necessarily bad and am hesitant to change
> that part; however, we definitely wanna fix the crash.
It is not retrying anything. It is repeatedly trying to stop the
engine as part of the 'start engine' sequence, with which it proceeds
because the code in ahci_init_one() does not realize the reset has
failed.
> How does
> forwarding the error make the crash go away? That sounds like we
> aren't clearing something we should have cleared while offlining the
> controller.
>
ahci_init_one() bails out if ahci_pci_reset_controller() returns a
failure. But as you can see, that code never does return a failure,
because it ignores the result code of ahci_reset_controller(). So the
'if (rc) return rc;' that follows in ahci_init_one() is essentially
dead code.
^ permalink raw reply
* Re: [PATCH] ahci: don't ignore result code of ahci_reset_controller()
From: Tejun Heo @ 2017-10-02 18:54 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-ide, graeme.gregory, leif.lindholm, daniel.thompson
In-Reply-To: <20171002183124.17003-1-ard.biesheuvel@linaro.org>
Hello, Ard.
On Mon, Oct 02, 2017 at 07:31:24PM +0100, Ard Biesheuvel wrote:
> ahci_pci_reset_controller() calls ahci_reset_controller(), which may
> fail, but ignores the result code and always returns success. This
> may result in failures like below
>
> ahci 0000:02:00.0: version 3.0
> ahci 0000:02:00.0: enabling device (0000 -> 0003)
> ahci 0000:02:00.0: SSS flag set, parallel bus scan disabled
> ahci 0000:02:00.0: controller reset failed (0xffffffff)
> ahci 0000:02:00.0: failed to stop engine (-5)
> ... repeated many times ...
> ahci 0000:02:00.0: failed to stop engine (-5)
> Unable to handle kernel paging request at virtual address ffff0000093f9018
> ...
> PC is at ahci_stop_engine+0x5c/0xd8 [libahci]
> LR is at ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
> ...
> [<ffff000000a17014>] ahci_stop_engine+0x5c/0xd8 [libahci]
> [<ffff000000a196b4>] ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
> [<ffff000000a197d8>] ahci_init_controller+0x80/0x168 [libahci]
> [<ffff000000a260f8>] ahci_pci_init_controller+0x60/0x68 [ahci]
> [<ffff000000a26f94>] ahci_init_one+0x75c/0xd88 [ahci]
> [<ffff000008430324>] local_pci_probe+0x3c/0xb8
> [<ffff000008431728>] pci_device_probe+0x138/0x170
> [<ffff000008585e54>] driver_probe_device+0x2dc/0x458
> [<ffff0000085860e4>] __driver_attach+0x114/0x118
> [<ffff000008583ca8>] bus_for_each_dev+0x60/0xa0
> [<ffff000008585638>] driver_attach+0x20/0x28
> [<ffff0000085850b0>] bus_add_driver+0x1f0/0x2a8
> [<ffff000008586ae0>] driver_register+0x60/0xf8
> [<ffff00000842f9b4>] __pci_register_driver+0x3c/0x48
> [<ffff000000a3001c>] ahci_pci_driver_init+0x1c/0x1000 [ahci]
> [<ffff000008083918>] do_one_initcall+0x38/0x120
>
> where an obvious hardware level failure results in an unnecessary 15 second
> delay and a subsequent crash.
I'm not sure the retries are necessarily bad and am hesitant to change
that part; however, we definitely wanna fix the crash. How does
forwarding the error make the crash go away? That sounds like we
aren't clearing something we should have cleared while offlining the
controller.
Thanks.
--
tejun
^ permalink raw reply
* [PATCH] ahci: don't ignore result code of ahci_reset_controller()
From: Ard Biesheuvel @ 2017-10-02 18:31 UTC (permalink / raw)
To: linux-ide, tj
Cc: graeme.gregory, leif.lindholm, daniel.thompson, Ard Biesheuvel
ahci_pci_reset_controller() calls ahci_reset_controller(), which may
fail, but ignores the result code and always returns success. This
may result in failures like below
ahci 0000:02:00.0: version 3.0
ahci 0000:02:00.0: enabling device (0000 -> 0003)
ahci 0000:02:00.0: SSS flag set, parallel bus scan disabled
ahci 0000:02:00.0: controller reset failed (0xffffffff)
ahci 0000:02:00.0: failed to stop engine (-5)
... repeated many times ...
ahci 0000:02:00.0: failed to stop engine (-5)
Unable to handle kernel paging request at virtual address ffff0000093f9018
...
PC is at ahci_stop_engine+0x5c/0xd8 [libahci]
LR is at ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
...
[<ffff000000a17014>] ahci_stop_engine+0x5c/0xd8 [libahci]
[<ffff000000a196b4>] ahci_deinit_port.constprop.12+0x1c/0xc0 [libahci]
[<ffff000000a197d8>] ahci_init_controller+0x80/0x168 [libahci]
[<ffff000000a260f8>] ahci_pci_init_controller+0x60/0x68 [ahci]
[<ffff000000a26f94>] ahci_init_one+0x75c/0xd88 [ahci]
[<ffff000008430324>] local_pci_probe+0x3c/0xb8
[<ffff000008431728>] pci_device_probe+0x138/0x170
[<ffff000008585e54>] driver_probe_device+0x2dc/0x458
[<ffff0000085860e4>] __driver_attach+0x114/0x118
[<ffff000008583ca8>] bus_for_each_dev+0x60/0xa0
[<ffff000008585638>] driver_attach+0x20/0x28
[<ffff0000085850b0>] bus_add_driver+0x1f0/0x2a8
[<ffff000008586ae0>] driver_register+0x60/0xf8
[<ffff00000842f9b4>] __pci_register_driver+0x3c/0x48
[<ffff000000a3001c>] ahci_pci_driver_init+0x1c/0x1000 [ahci]
[<ffff000008083918>] do_one_initcall+0x38/0x120
where an obvious hardware level failure results in an unnecessary 15 second
delay and a subsequent crash.
So record the result code of ahci_reset_controller() and relay it, rather
than ignoring it.
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
drivers/ata/ahci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 5a5fd0b404eb..649e799df9c1 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -621,8 +621,11 @@ static void ahci_pci_save_initial_config(struct pci_dev *pdev,
static int ahci_pci_reset_controller(struct ata_host *host)
{
struct pci_dev *pdev = to_pci_dev(host->dev);
+ int rc;
- ahci_reset_controller(host);
+ rc = ahci_reset_controller(host);
+ if (rc)
+ return rc;
if (pdev->vendor == PCI_VENDOR_ID_INTEL) {
struct ahci_host_priv *hpriv = host->private_data;
--
2.11.0
^ permalink raw reply related
* (unknown),
From: dengx @ 2017-10-02 18:06 UTC (permalink / raw)
To: linux-ide
[-- Attachment #1: 64703085.zip --]
[-- Type: application/zip, Size: 7218 bytes --]
^ permalink raw reply
* Re: [PATCH] libata: make ata_port_type const
From: Tejun Heo @ 2017-10-02 16:57 UTC (permalink / raw)
To: Bhumika Goyal; +Cc: julia.lawall, linux-ide, linux-kernel
In-Reply-To: <1506789640-10748-1-git-send-email-bhumirks@gmail.com>
On Sat, Sep 30, 2017 at 10:10:40PM +0530, Bhumika Goyal wrote:
> Make this const as it is only stored in the const field of a device
> structure. Make the declaration in header const too.
>
> Structure found using Coccinelle and changes done by hand.
>
> Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
Applied to libata/for-4.15.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: Guenter Roeck @ 2017-10-02 13:24 UTC (permalink / raw)
To: Lorenzo Pieralisi, linux-pci
Cc: linux-kernel, linux-ide, Bjorn Helgaas, Richard Henderson,
Ivan Kokshaysky, David S. Miller, Matt Turner
In-Reply-To: <1506941567-10762-1-git-send-email-lorenzo.pieralisi@arm.com>
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;
> + }
> }
> }
> }
>
^ permalink raw reply
* [PATCH v2] drivers/ide/pci: Fix legacy IRQ assignment
From: Lorenzo Pieralisi @ 2017-10-02 10:52 UTC (permalink / raw)
To: linux-pci
Cc: linux-kernel, linux-ide, Lorenzo Pieralisi, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, David S. Miller,
Guenter Roeck, Matt Turner
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
^ permalink raw reply related
* [PATCH] libata: make ata_port_type const
From: Bhumika Goyal @ 2017-09-30 16:40 UTC (permalink / raw)
To: julia.lawall, tj, linux-ide, linux-kernel; +Cc: Bhumika Goyal
Make this const as it is only stored in the const field of a device
structure. Make the declaration in header const too.
Structure found using Coccinelle and changes done by hand.
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/ata/libata-core.c | 2 +-
drivers/ata/libata.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 65f7574..29e3516 100644
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -5824,7 +5824,7 @@ void ata_host_resume(struct ata_host *host)
}
#endif
-struct device_type ata_port_type = {
+const struct device_type ata_port_type = {
.name = "ata_port",
#ifdef CONFIG_PM
.pm = &ata_port_pm_ops,
diff --git a/drivers/ata/libata.h b/drivers/ata/libata.h
index 839d487..18bf1e9 100644
--- a/drivers/ata/libata.h
+++ b/drivers/ata/libata.h
@@ -51,7 +51,7 @@ enum {
extern int libata_fua;
extern int libata_noacpi;
extern int libata_allow_tpm;
-extern struct device_type ata_port_type;
+extern const struct device_type ata_port_type;
extern struct ata_link *ata_dev_phys_link(struct ata_device *dev);
extern void ata_force_cbl(struct ata_port *ap);
extern u64 ata_tf_to_lba(const struct ata_taskfile *tf);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Guenter Roeck @ 2017-09-30 1:27 UTC (permalink / raw)
To: Lorenzo Pieralisi, linux-pci
Cc: linux-kernel, linux-arm-kernel, linux-ide, Bjorn Helgaas,
Richard Henderson, Ivan Kokshaysky, David S. Miller, Matt Turner
In-Reply-To: <1506598627-8985-1-git-send-email-lorenzo.pieralisi@arm.com>
On 09/28/2017 04:37 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() hooks it carries
> out the PCI IRQ allocation.
>
> As it turned out, some legacy drivers (eg IDE layer) require that a
> device allocates IRQ as soon as it is added so that its actual IRQ
> settings are available early in the boot process. With current code
> calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> for some drivers:
>
> 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 moving the pci_assign_irq() call from
> pci_device_probe() to pci_device_add(), so that IRQs for a given PCI
> device are allocated as soon as it is scanned by the PCI enumeration.
>
> This has the drawback of allocating an IRQ even for PCI devices that
> do not have a matching kernel driver but it should be safe to carry
> out in all configurations.
>
> 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>
> ---
> drivers/pci/pci-driver.c | 2 --
> drivers/pci/probe.c | 3 +++
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 11bd267..750d688 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -415,8 +415,6 @@ static int pci_device_probe(struct device *dev)
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct pci_driver *drv = to_pci_driver(dev->driver);
>
> - pci_assign_irq(pci_dev);
> -
> error = pcibios_alloc_irq(pci_dev);
> if (error < 0)
> return error;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ff94b69..ba4e466 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2103,6 +2103,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> /* Initialize various capabilities */
> pci_init_capabilities(dev);
>
> + /* Assign device IRQ number */
> + pci_assign_irq(dev);
> +
> /*
> * Add the device to our list of discovered devices
> * and the bus list for fixup functions, etc.
>
^ permalink raw reply
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Bjorn Helgaas @ 2017-09-29 20:19 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, linux-kernel, linux-ide, Ivan Kokshaysky,
Guenter Roeck, Bjorn Helgaas, Matt Turner, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20170929171958.GA18114@red-moon>
On Fri, Sep 29, 2017 at 06:19:58PM +0100, Lorenzo Pieralisi wrote:
> On Fri, Sep 29, 2017 at 05:02:26PM +0100, Lorenzo Pieralisi wrote:
> > On Thu, Sep 28, 2017 at 05:37:19PM -0500, Bjorn Helgaas wrote:
> > > On Thu, Sep 28, 2017 at 12:37:07PM +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() hooks it carries
> > > > out the PCI IRQ allocation.
> > > >
> > > > As it turned out, some legacy drivers (eg IDE layer) require that a
> > > > device allocates IRQ as soon as it is added so that its actual IRQ
> > > > settings are available early in the boot process. With current code
> > > > calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> > > > for some drivers:
> > >
> > > I think the patch is fine, but I don't understand the changelog. I
> > > want to know specifically what the dependency on dev->irq is. "Early
> > > in the boot process" is pretty vague.
> > >
> > > I *thought* we were doing something like this:
> > >
> > > pci_device_probe(dev1)
> > > pci_assign_irq(dev1)
> > > ...
> > > ide_pci_init_two(dev1, dev2, ...)
> > > do_ide_setup_pci_device(dev1)
> > > pciirq = dev1->irq # this one is fine
> > > do_ide_setup_pci_device(dev2)
> > > pciirq = dev2->irq # not fine
> > >
> > > where the problem is that we haven't called pci_assign_irq(dev2), so
> > > dev2->irq hasn't been set.
> > >
> > > But that doesn't match the data because we should be coming through
> > > cmd64x_init_one(), which calls ide_pci_init_one(), so we shouldn't
> > > have a dev2 in this path.
> >
> > I *think* I understand what's going on here, the key is:
> >
> > ide_scan_pcibus()
> >
> > and CONFIG_IDEPCI_PCIBUS_ORDER
> >
> > I still have to replicate it but I suspect that
> > do_ide_setup_pci_device() for dev1 finds an unallocated IRQ (ie dev->irq
> > == 0) because the probing did NOT happen via pci_device_probe(), ie
> > pci_device_probe() was not called for the dev1, the cmd64x probe
> > routine is called straight from ide_scan_pcidev().
> >
> > I am struggling to understand the logic behind:
> >
> > ide_pci_register_driver() and ide_scan_pcibus()
> >
> > and the sequence wrt PCI bus probing but I think that's the problem
> > and that's why moving pci_assign_irq() to pci_device_add() will
> > sort this out, adding pci_assign_irq() in ide_scan_pcidev() will
> > solve the problem too (patch below).
> >
> > Needless to say, ide_scan_pcibus() relies on pre_init global variable
> > to make sure ide_pci_register_driver() chooses the "right" way of
> > registering a driver, see:
> >
> > __ide_pci_register_driver()
> >
> > Patch here to verify my assumption in case Guenter has a chance to
> > run it if I do not beat him to it:
>
> That's what's happening unfortunately.
>
> We end up probing twice (both fails):
>
> (1)
>
> ->ide_scan_pcidev()
> ->d->probe()
> ->cmd64x_init_one()
> ->ide_pci_init_one()
> ->ide_pci_init_two()
> [...]
> -> ide_host_register() !! Fails in hwif_init(), no IRQ
>
> (2) ->pci_device_probe()
> ->cmd64x_init_one()
> ->ide_pci_init_one()
> ->ide_pci_init_two()
> [...]
> -> ide_pci_enable() !! Fails with -EBUSY,
> pci_request_selected_regions() can't reserve already reserved
> regions (ie step (1) did not unwind resource reservation)
>
> That's my reading and patch patch below fixes it and given that
> IDE created its own PCI bus probing layer may be a more appropriate
> kludge than forcing us to move pci_assign_irq() to pci_device_add()
> for all PCI devices, please let me know what's your preferred solution.
Oh, this is lovely :( I wonder what other things this breaks.
This IDE probing path misses the IRQ assignment, the "initialize the
driver on the device's node" stuff, the PM code, the IOV autoprobe
checks, the ACPI IRQ init in pcibios_alloc_irq(), etc. Ugh.
I think your solution below is the best one so far. It's still hacky
because the whole ide_scan_pcibus() is a hack, but at least this fixes
it at the point where we need it, so it's obvious why it's there and
it's clear that if we could ever get rid of ide_scan_pcibus(), we
could get rid of this pci_assign_irq() call as well.
> With fix below (1) still tries to re-probe cmd64x through
> pci_device_probe() but the device has already a driver attached
> to it so second probe stops before calling the cmd64x probe routine.
>
> I hope Guenter can give it a go too to confirm my findings.
>
> Lorenzo
>
> > -- >8 --
> > diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c
> > index 86aa88a..86b570a 100644
> > --- a/drivers/ide/ide-scan-pci.c
> > +++ b/drivers/ide/ide-scan-pci.c
> > @@ -56,6 +56,8 @@ 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 +65,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;
> > + }
> > }
> > }
> > }
^ permalink raw reply
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Lorenzo Pieralisi @ 2017-09-29 17:19 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, linux-ide, Ivan Kokshaysky,
Guenter Roeck, Bjorn Helgaas, Matt Turner, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20170929160226.GA17398@red-moon>
On Fri, Sep 29, 2017 at 05:02:26PM +0100, Lorenzo Pieralisi wrote:
> On Thu, Sep 28, 2017 at 05:37:19PM -0500, Bjorn Helgaas wrote:
> > On Thu, Sep 28, 2017 at 12:37:07PM +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() hooks it carries
> > > out the PCI IRQ allocation.
> > >
> > > As it turned out, some legacy drivers (eg IDE layer) require that a
> > > device allocates IRQ as soon as it is added so that its actual IRQ
> > > settings are available early in the boot process. With current code
> > > calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> > > for some drivers:
> >
> > I think the patch is fine, but I don't understand the changelog. I
> > want to know specifically what the dependency on dev->irq is. "Early
> > in the boot process" is pretty vague.
> >
> > I *thought* we were doing something like this:
> >
> > pci_device_probe(dev1)
> > pci_assign_irq(dev1)
> > ...
> > ide_pci_init_two(dev1, dev2, ...)
> > do_ide_setup_pci_device(dev1)
> > pciirq = dev1->irq # this one is fine
> > do_ide_setup_pci_device(dev2)
> > pciirq = dev2->irq # not fine
> >
> > where the problem is that we haven't called pci_assign_irq(dev2), so
> > dev2->irq hasn't been set.
> >
> > But that doesn't match the data because we should be coming through
> > cmd64x_init_one(), which calls ide_pci_init_one(), so we shouldn't
> > have a dev2 in this path.
>
> I *think* I understand what's going on here, the key is:
>
> ide_scan_pcibus()
>
> and CONFIG_IDEPCI_PCIBUS_ORDER
>
> I still have to replicate it but I suspect that
> do_ide_setup_pci_device() for dev1 finds an unallocated IRQ (ie dev->irq
> == 0) because the probing did NOT happen via pci_device_probe(), ie
> pci_device_probe() was not called for the dev1, the cmd64x probe
> routine is called straight from ide_scan_pcidev().
>
> I am struggling to understand the logic behind:
>
> ide_pci_register_driver() and ide_scan_pcibus()
>
> and the sequence wrt PCI bus probing but I think that's the problem
> and that's why moving pci_assign_irq() to pci_device_add() will
> sort this out, adding pci_assign_irq() in ide_scan_pcidev() will
> solve the problem too (patch below).
>
> Needless to say, ide_scan_pcibus() relies on pre_init global variable
> to make sure ide_pci_register_driver() chooses the "right" way of
> registering a driver, see:
>
> __ide_pci_register_driver()
>
> Patch here to verify my assumption in case Guenter has a chance to
> run it if I do not beat him to it:
That's what's happening unfortunately.
We end up probing twice (both fails):
(1)
->ide_scan_pcidev()
->d->probe()
->cmd64x_init_one()
->ide_pci_init_one()
->ide_pci_init_two()
[...]
-> ide_host_register() !! Fails in hwif_init(), no IRQ
(2) ->pci_device_probe()
->cmd64x_init_one()
->ide_pci_init_one()
->ide_pci_init_two()
[...]
-> ide_pci_enable() !! Fails with -EBUSY,
pci_request_selected_regions() can't reserve already reserved
regions (ie step (1) did not unwind resource reservation)
That's my reading and patch patch below fixes it and given that
IDE created its own PCI bus probing layer may be a more appropriate
kludge than forcing us to move pci_assign_irq() to pci_device_add()
for all PCI devices, please let me know what's your preferred solution.
With fix below (1) still tries to re-probe cmd64x through
pci_device_probe() but the device has already a driver attached
to it so second probe stops before calling the cmd64x probe routine.
I hope Guenter can give it a go too to confirm my findings.
Lorenzo
> -- >8 --
> diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c
> index 86aa88a..86b570a 100644
> --- a/drivers/ide/ide-scan-pci.c
> +++ b/drivers/ide/ide-scan-pci.c
> @@ -56,6 +56,8 @@ 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 +65,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;
> + }
> }
> }
> }
^ permalink raw reply
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Lorenzo Pieralisi @ 2017-09-29 16:02 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, linux-ide, Ivan Kokshaysky,
Guenter Roeck, Bjorn Helgaas, Matt Turner, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20170928223719.GY15970@bhelgaas-glaptop.roam.corp.google.com>
On Thu, Sep 28, 2017 at 05:37:19PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 28, 2017 at 12:37:07PM +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() hooks it carries
> > out the PCI IRQ allocation.
> >
> > As it turned out, some legacy drivers (eg IDE layer) require that a
> > device allocates IRQ as soon as it is added so that its actual IRQ
> > settings are available early in the boot process. With current code
> > calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> > for some drivers:
>
> I think the patch is fine, but I don't understand the changelog. I
> want to know specifically what the dependency on dev->irq is. "Early
> in the boot process" is pretty vague.
>
> I *thought* we were doing something like this:
>
> pci_device_probe(dev1)
> pci_assign_irq(dev1)
> ...
> ide_pci_init_two(dev1, dev2, ...)
> do_ide_setup_pci_device(dev1)
> pciirq = dev1->irq # this one is fine
> do_ide_setup_pci_device(dev2)
> pciirq = dev2->irq # not fine
>
> where the problem is that we haven't called pci_assign_irq(dev2), so
> dev2->irq hasn't been set.
>
> But that doesn't match the data because we should be coming through
> cmd64x_init_one(), which calls ide_pci_init_one(), so we shouldn't
> have a dev2 in this path.
I *think* I understand what's going on here, the key is:
ide_scan_pcibus()
and CONFIG_IDEPCI_PCIBUS_ORDER
I still have to replicate it but I suspect that
do_ide_setup_pci_device() for dev1 finds an unallocated IRQ (ie dev->irq
== 0) because the probing did NOT happen via pci_device_probe(), ie
pci_device_probe() was not called for the dev1, the cmd64x probe
routine is called straight from ide_scan_pcidev().
I am struggling to understand the logic behind:
ide_pci_register_driver() and ide_scan_pcibus()
and the sequence wrt PCI bus probing but I think that's the problem
and that's why moving pci_assign_irq() to pci_device_add() will
sort this out, adding pci_assign_irq() in ide_scan_pcidev() will
solve the problem too (patch below).
Needless to say, ide_scan_pcibus() relies on pre_init global variable
to make sure ide_pci_register_driver() chooses the "right" way of
registering a driver, see:
__ide_pci_register_driver()
Patch here to verify my assumption in case Guenter has a chance to
run it if I do not beat him to it:
-- >8 --
diff --git a/drivers/ide/ide-scan-pci.c b/drivers/ide/ide-scan-pci.c
index 86aa88a..86b570a 100644
--- a/drivers/ide/ide-scan-pci.c
+++ b/drivers/ide/ide-scan-pci.c
@@ -56,6 +56,8 @@ 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 +65,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;
+ }
}
}
}
^ permalink raw reply related
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Lorenzo Pieralisi @ 2017-09-29 13:25 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, linux-ide, Ivan Kokshaysky,
Guenter Roeck, Bjorn Helgaas, Matt Turner, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <20170928223719.GY15970@bhelgaas-glaptop.roam.corp.google.com>
On Thu, Sep 28, 2017 at 05:37:19PM -0500, Bjorn Helgaas wrote:
> On Thu, Sep 28, 2017 at 12:37:07PM +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() hooks it carries
> > out the PCI IRQ allocation.
> >
> > As it turned out, some legacy drivers (eg IDE layer) require that a
> > device allocates IRQ as soon as it is added so that its actual IRQ
> > settings are available early in the boot process. With current code
> > calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> > for some drivers:
>
> I think the patch is fine, but I don't understand the changelog. I
> want to know specifically what the dependency on dev->irq is. "Early
> in the boot process" is pretty vague.
I agree with you Bjorn and I also agree that this fix does not
explain what the problem really is so it is not actually fixing
anything.
> I *thought* we were doing something like this:
>
> pci_device_probe(dev1)
> pci_assign_irq(dev1)
> ...
> ide_pci_init_two(dev1, dev2, ...)
> do_ide_setup_pci_device(dev1)
> pciirq = dev1->irq # this one is fine
> do_ide_setup_pci_device(dev2)
> pciirq = dev2->irq # not fine
>
> where the problem is that we haven't called pci_assign_irq(dev2), so
> dev2->irq hasn't been set.
>
> But that doesn't match the data because we should be coming through
> cmd64x_init_one(), which calls ide_pci_init_one(), so we shouldn't
> have a dev2 in this path.
That's my understanding too and pci_assign_irq() should have been called
for dev1 (and failed) by then, why calling it again succeeds is not
clear to me.
There is something I am missing here which is not obvious, I need
more data to debug this, I will try to replicate Guenter's setup.
Thanks,
Lorenzo
> > 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 moving the pci_assign_irq() call from
> > pci_device_probe() to pci_device_add(), so that IRQs for a given PCI
> > device are allocated as soon as it is scanned by the PCI enumeration.
> >
> > This has the drawback of allocating an IRQ even for PCI devices that
> > do not have a matching kernel driver but it should be safe to carry
> > out in all configurations.
> >
> > 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>
> > ---
> > drivers/pci/pci-driver.c | 2 --
> > drivers/pci/probe.c | 3 +++
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> > index 11bd267..750d688 100644
> > --- a/drivers/pci/pci-driver.c
> > +++ b/drivers/pci/pci-driver.c
> > @@ -415,8 +415,6 @@ static int pci_device_probe(struct device *dev)
> > struct pci_dev *pci_dev = to_pci_dev(dev);
> > struct pci_driver *drv = to_pci_driver(dev->driver);
> >
> > - pci_assign_irq(pci_dev);
> > -
> > error = pcibios_alloc_irq(pci_dev);
> > if (error < 0)
> > return error;
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index ff94b69..ba4e466 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -2103,6 +2103,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> > /* Initialize various capabilities */
> > pci_init_capabilities(dev);
> >
> > + /* Assign device IRQ number */
>
> Pointless comment (as is the existing capabilities one above).
>
> > + pci_assign_irq(dev);
> > +
> > /*
> > * Add the device to our list of discovered devices
> > * and the bus list for fixup functions, etc.
> > --
> > 2.4.12
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Bjorn Helgaas @ 2017-09-28 22:37 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: linux-pci, linux-kernel, linux-ide, Ivan Kokshaysky,
Guenter Roeck, Bjorn Helgaas, Matt Turner, David S. Miller,
linux-arm-kernel, Richard Henderson
In-Reply-To: <1506598627-8985-1-git-send-email-lorenzo.pieralisi@arm.com>
On Thu, Sep 28, 2017 at 12:37:07PM +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() hooks it carries
> out the PCI IRQ allocation.
>
> As it turned out, some legacy drivers (eg IDE layer) require that a
> device allocates IRQ as soon as it is added so that its actual IRQ
> settings are available early in the boot process. With current code
> calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
> for some drivers:
I think the patch is fine, but I don't understand the changelog. I
want to know specifically what the dependency on dev->irq is. "Early
in the boot process" is pretty vague.
I *thought* we were doing something like this:
pci_device_probe(dev1)
pci_assign_irq(dev1)
...
ide_pci_init_two(dev1, dev2, ...)
do_ide_setup_pci_device(dev1)
pciirq = dev1->irq # this one is fine
do_ide_setup_pci_device(dev2)
pciirq = dev2->irq # not fine
where the problem is that we haven't called pci_assign_irq(dev2), so
dev2->irq hasn't been set.
But that doesn't match the data because we should be coming through
cmd64x_init_one(), which calls ide_pci_init_one(), so we shouldn't
have a dev2 in this path.
> 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 moving the pci_assign_irq() call from
> pci_device_probe() to pci_device_add(), so that IRQs for a given PCI
> device are allocated as soon as it is scanned by the PCI enumeration.
>
> This has the drawback of allocating an IRQ even for PCI devices that
> do not have a matching kernel driver but it should be safe to carry
> out in all configurations.
>
> 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>
> ---
> drivers/pci/pci-driver.c | 2 --
> drivers/pci/probe.c | 3 +++
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 11bd267..750d688 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -415,8 +415,6 @@ static int pci_device_probe(struct device *dev)
> struct pci_dev *pci_dev = to_pci_dev(dev);
> struct pci_driver *drv = to_pci_driver(dev->driver);
>
> - pci_assign_irq(pci_dev);
> -
> error = pcibios_alloc_irq(pci_dev);
> if (error < 0)
> return error;
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index ff94b69..ba4e466 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2103,6 +2103,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
> /* Initialize various capabilities */
> pci_init_capabilities(dev);
>
> + /* Assign device IRQ number */
Pointless comment (as is the existing capabilities one above).
> + pci_assign_irq(dev);
> +
> /*
> * Add the device to our list of discovered devices
> * and the bus list for fixup functions, etc.
> --
> 2.4.12
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: alpha runtime warnings due to commit 0e4c2eeb758 ("alpha/PCI: Replace pci_fixup_irqs() call with host ...")
From: Bjorn Helgaas @ 2017-09-28 17:53 UTC (permalink / raw)
To: Lorenzo Pieralisi
Cc: Guenter Roeck, Bjorn Helgaas, linux-kernel@vger.kernel.org,
Richard Henderson, Ivan Kokshaysky, Matt Turner, alpha, linux-pci,
David S. Miller, linux-ide
In-Reply-To: <20170928092532.GA23071@red-moon>
On Thu, Sep 28, 2017 at 10:25:33AM +0100, Lorenzo Pieralisi wrote:
> On Wed, Sep 27, 2017 at 02:55:02PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 27, 2017 at 11:30:34AM +0100, Lorenzo Pieralisi wrote:
> > > On Wed, Sep 20, 2017 at 12:31:04PM +0100, Lorenzo Pieralisi wrote:
> > > > On Tue, Sep 19, 2017 at 11:02:33AM -0700, Guenter Roeck wrote:
> > > > > On Tue, Sep 19, 2017 at 10:12:32AM +0100, Lorenzo Pieralisi wrote:
> > > > > > On Mon, Sep 18, 2017 at 07:00:55PM -0700, Guenter Roeck wrote:
> > > > > > > I see the following runtime warnings in mainline when running alpha images in qemu.
> > > > > > >
> > > > > > >
> > > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > > 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
> > > > > > >
> > > > > > > [ multiple times ]
> > > > > > >
> > > > > > > A complete log is available at http://kerneltests.org/builders/qemu-alpha-master.
> > > > > > >
> > > > > > > Prior to the offending commit, the kernel log looks as follows.
> > > > > > >
> > > > > > > ...
> > > > > > > Uniform Multi-Platform E-IDE driver
> > > > > > > cmd64x 0000:00:02.0: IDE controller (0x1095:0x0646 rev 0x07)
> > > > > > > cmd64x 0000:00:02.0: IDE port disabled
> > > > > > > cmd64x 0000:00:02.0: 100% native mode on irq 28
> > > > > > > PCI: Setting latency timer of device 0000:00:02.0 to 64
> > > > > > > ide0: BM-DMA at 0x8040-0x8047
> > > > > > > Floppy drive(s): fd0 is 2.88M
> > > > > > > ide0 at 0x8050-0x8057,0x8062 on irq 28 (serialized)
> > > > > > > ide_generic: please use "probe_mask=0x3f" module parameter for probing all legacy ISA IDE ports
> > > > > > > ide1 at 0x1f0-0x1f7,0x3f6 on irq 14
> > > > > > > ide2 at 0x170-0x177,0x376 on irq 15
> > > > > > > ide-gd driver 1.18
> > > > > > > ide-cd driver 5.00
> > > > > > > ...
> > > > > > >
> > > > > > > Reverting the commit is not possible due to context changes.
> > > > > > >
> > > > > > > Bisect log is attached.
> > > > > >
> > > > > > Ok thanks. Can you please check if the diff below fixes the issue ?
> > > > > >
> > > > > It does. Feel free to add
> > > > >
> > > > > Tested-by: Guenter Roeck <linux@roeck-us.net>
> > > > >
> > > > > to the actual patch.
> > > > >
> > > > > > I think the problem is that now we allocate the IRQ at device_probe
> > > > > > instead of device_add time, if the patch below fixes the issue the
> > > > > > question is whether we want to move pci_assign_irq() to pci_device_add()
> > > > > > for ALL PCI devices or just fix this for IDE subsytem.
> > > > > >
> > > > >
> > > > > That I can not answer ...
> > > >
> > > > It is not a clean-cut answer. Moving pci_assign_irq() from
> > > >
> > > > pci_device_probe()
> > > >
> > > > to
> > > >
> > > > pci_device_add()
> > > >
> > > > would fix this issue too and just map/swizzle() irqs for ALL PCI devices
> > > > earlier (even ones that are not even probed); it should have no side
> > > > effects (famous last words) and would be a cleaner fix.
> >
> > Hmmm, this is ugly. I *think* this is related to ide_pci_init_two().
> > We already call pci_assign_irq() from pci_device_probe(), where we try
> > to bind one device to a driver. But ide_pci_init_two() deals with
> > *two* devices: the one pci_device_probe() is trying to bind, and
> > another random one found via pci_get_slot(). We have not called
> > pci_assign_irq() for this second device.
> >
> > That breaks the model the PCI core expects, where we call the driver
> > probe method for device X, and that method deals only with device X.
> >
> > We can fix this by doing the pci_assign_irq() either in the PCI core's
> > device add path or in the driver's probe path (ide_pci_init_two(), as
> > in the patch below). Doing it in the PCI core is sort of ugly because
> > there's no obvious reason why we should map/swizzle the IRQ if there's
> > no driver.
> >
> > Doing it in the driver is also sort of ugly because this is something
> > that should be confined to the core (pci_assign_irq() probably should
> > be declared in drivers/pci/pci.h so it's not available outside the
> > core).
> >
> > I think the patch below means we call pci_assign_irq() *twice* on the
> > device we're binding: once from pci_device_probe() and again from
> > do_ide_setup_pci_device(). And I guess we might call it twice for the
> > second device, too: once from do_ide_setup_pci_device() and again if
> > pci_device_probe() tries to bind it. It's probably idempotent, but it
> > does seem a little ugly.
> >
> > I guess I agree that calling pci_assign_irq() from pci_device_add() is
> > the lesser evil. That will do it unnecessarily in cases where a
> > device doesn't have a driver, but it should be safe. It sets
> > pdev->irq but probably doesn't touch the device itself.
>
> I will do that, actually this would reinstate the exact boot stage at
> which pci_fixup_irqs() was called (except that pci_assign_irq() does
> that on a per host-bridge basis and in core code) because
> pci_fixup_irqs() was called after enumeration before PCI devices were
> added (ie before calling pci_bus_add_devices()), it *should* be safe.
>
> > We do more IRQ setup at pci_enable_device()-time (or probe-time for
> > arm64) via acpi_pci_irq_enable(). This also updates pdev->irq. I'd
> > rather not do that part at device add-time, because it does things
> > like allocate and enable interrupt links, and we shouldn't do that
> > until we know we have a driver for the device.
> >
> > But this is potentially another problem for this "second-device" thing
> > IDE does. We've called pci_assign_irq() but not acpi_pci_irq_enable()
> > for the second device, so we may still get the wrong IRQ for it.
>
> I have not touched the ACPI irq assignment path with my patches on
> purpose. Put it differently, PNP0A03 host bridges do not have any
> {map/swizzle}_irq() hook (yet) so pci_assign_irq() does nothing on
> them. ACPI based platforms never used pci_fixup_irqs() so I changed
> nothing on them (and I doubt I can given how complicated x86 legacy
> IRQ assignment code is in arch code).
Right. This isn't a problem with your patches; it's just a problem
with the ide_pci_init_two() strategy of looking at dev->irq for a
device for which the core hasn't called the probe path.
Bjorn
^ permalink raw reply
* [PATCH] PCI: Fix legacy IRQ assignment execution stage
From: Lorenzo Pieralisi @ 2017-09-28 11:37 UTC (permalink / raw)
To: linux-pci
Cc: linux-kernel, linux-arm-kernel, linux-ide, Lorenzo Pieralisi,
Bjorn Helgaas, Richard Henderson, Ivan Kokshaysky,
David S. Miller, Guenter Roeck, Matt Turner
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() hooks it carries
out the PCI IRQ allocation.
As it turned out, some legacy drivers (eg IDE layer) require that a
device allocates IRQ as soon as it is added so that its actual IRQ
settings are available early in the boot process. With current code
calling pci_assign_irq() in pci_device_probe() IDE IRQ probing fails
for some drivers:
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 moving the pci_assign_irq() call from
pci_device_probe() to pci_device_add(), so that IRQs for a given PCI
device are allocated as soon as it is scanned by the PCI enumeration.
This has the drawback of allocating an IRQ even for PCI devices that
do not have a matching kernel driver but it should be safe to carry
out in all configurations.
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>
---
drivers/pci/pci-driver.c | 2 --
drivers/pci/probe.c | 3 +++
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index 11bd267..750d688 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -415,8 +415,6 @@ static int pci_device_probe(struct device *dev)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *drv = to_pci_driver(dev->driver);
- pci_assign_irq(pci_dev);
-
error = pcibios_alloc_irq(pci_dev);
if (error < 0)
return error;
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index ff94b69..ba4e466 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2103,6 +2103,9 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
/* Initialize various capabilities */
pci_init_capabilities(dev);
+ /* Assign device IRQ number */
+ pci_assign_irq(dev);
+
/*
* Add the device to our list of discovered devices
* and the bus list for fixup functions, etc.
--
2.4.12
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox