| Message ID | 20150416134931.30238.90240.stgit@brunhilda (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/16/2015 03:49 PM, Don Brace wrote: > From: Robert Elliott <elliott@hp.com> > > Despite the fact that PCI devices are enabled in this order: > 1. pci_enable_device > 2. pci_request_regions > > Documentation/PCI/pci.txt specifies that they be undone > in this order > 1. pci_disable_device > 2. pci_release_regions > > Tested by injecting error in the call to pci_enable_device > in hpsa_init_one -> hpsa_pci_init: > [ 9.095001] hpsa 0000:04:00.0: failed to enable PCI device > [ 9.095005] hpsa: probe of 0000:04:00.0 failed with error -22 > (-22 is -EINVAL) > and then in the call pci_request_regions: > [ 9.178623] hpsa 0000:04:00.0: failed to obtain PCI resources > [ 9.178671] hpsa: probe of 0000:04:00.0 failed with error -16 > (-16 is -EBUSY) > > and then by adding > reset_devices > to the kernel command line and inject errors into the two > calls to pci_enable_device and the call to pci_request_regions > in hpsa_init_one -> hpsa_init_reset_devices. > > (inject on 6th call, 1st to hpsa2) > [ 62.413750] hpsa 0000:04:00.0: Failed to enable PCI device > > (inject on 7th call, 2nd to hpsa2) > [ 62.807571] hpsa 0000:04:00.0: failed to enable device. > > (inject on 8th call, 3rd to hpsa2) > [ 62.697198] hpsa 0000:04:00.0: failed to obtain PCI resources > [ 62.697234] hpsa: probe of 0000:04:00.0 failed with error -16 > > The reset_devices path calls return -ENODEV on failure > rather than passing the result, which apparently doesn't > cause the pci driver to print anything. > > Reviewed-by: Scott Teel <scott.teel@pmcs.com> > Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com> > Signed-off-by: Robert Elliott <elliott@hp.com> > Signed-off-by: Don Brace < don.brace@pmcs.com> > --- Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
On 04/16/2015 03:49 PM, Don Brace wrote: > From: Robert Elliott <elliott@hp.com> > > Despite the fact that PCI devices are enabled in this order: > 1. pci_enable_device > 2. pci_request_regions > > Documentation/PCI/pci.txt specifies that they be undone > in this order > 1. pci_disable_device > 2. pci_release_regions > > Tested by injecting error in the call to pci_enable_device > in hpsa_init_one -> hpsa_pci_init: > [ 9.095001] hpsa 0000:04:00.0: failed to enable PCI device > [ 9.095005] hpsa: probe of 0000:04:00.0 failed with error -22 > (-22 is -EINVAL) > and then in the call pci_request_regions: > [ 9.178623] hpsa 0000:04:00.0: failed to obtain PCI resources > [ 9.178671] hpsa: probe of 0000:04:00.0 failed with error -16 > (-16 is -EBUSY) > > and then by adding > reset_devices > to the kernel command line and inject errors into the two > calls to pci_enable_device and the call to pci_request_regions > in hpsa_init_one -> hpsa_init_reset_devices. > > (inject on 6th call, 1st to hpsa2) > [ 62.413750] hpsa 0000:04:00.0: Failed to enable PCI device > > (inject on 7th call, 2nd to hpsa2) > [ 62.807571] hpsa 0000:04:00.0: failed to enable device. > > (inject on 8th call, 3rd to hpsa2) > [ 62.697198] hpsa 0000:04:00.0: failed to obtain PCI resources > [ 62.697234] hpsa: probe of 0000:04:00.0 failed with error -16 > > The reset_devices path calls return -ENODEV on failure > rather than passing the result, which apparently doesn't > cause the pci driver to print anything. > > Reviewed-by: Scott Teel <scott.teel@pmcs.com> > Reviewed-by: Kevin Barnett <kevin.barnett@pmcs.com> > Signed-off-by: Robert Elliott <elliott@hp.com> > Signed-off-by: Don Brace < don.brace@pmcs.com> Reviewed-by: Tomas Henzl <thenzl@redhat.com> Tomas -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index 5e12520..8aeef06 100644 --- a/drivers/scsi/hpsa.c +++ b/drivers/scsi/hpsa.c @@ -7048,8 +7048,12 @@ static void hpsa_free_pci_init(struct ctlr_info *h) iounmap(h->vaddr); /* pci_init 3 */ h->vaddr = NULL; hpsa_disable_interrupt_mode(h); /* pci_init 2 */ - pci_release_regions(h->pdev); /* pci_init 2 */ + /* + * call pci_disable_device before pci_release_regions per + * Documentation/PCI/pci.txt + */ pci_disable_device(h->pdev); /* pci_init 1 */ + pci_release_regions(h->pdev); /* pci_init 2 */ } /* several items must be freed later */ @@ -7072,6 +7076,7 @@ static int hpsa_pci_init(struct ctlr_info *h) err = pci_enable_device(h->pdev); if (err) { dev_err(&h->pdev->dev, "failed to enable PCI device\n"); + pci_disable_device(h->pdev); return err; } @@ -7079,7 +7084,8 @@ static int hpsa_pci_init(struct ctlr_info *h) if (err) { dev_err(&h->pdev->dev, "failed to obtain PCI resources\n"); - goto clean1; /* pci */ + pci_disable_device(h->pdev); + return err; } pci_set_master(h->pdev); @@ -7120,9 +7126,12 @@ clean3: /* vaddr, intmode+region, pci */ h->vaddr = NULL; clean2: /* intmode+region, pci */ hpsa_disable_interrupt_mode(h); - pci_release_regions(h->pdev); -clean1: /* pci */ + /* + * call pci_disable_device before pci_release_regions per + * Documentation/PCI/pci.txt + */ pci_disable_device(h->pdev); + pci_release_regions(h->pdev); return err; }