I'll take a look when I get a chance, Steve's out on vacation all week. At first glance it looks good, but like you said...gotta test it. Joe -----Original Message----- From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] Sent: Monday, July 07, 2014 9:13 AM To: 'linux-scsi@xxxxxxxxxxxxxxx' Cc: stephenmcameron@xxxxxxxxx; michael.miller@xxxxxxxxxxxxx; Handzik, Joe Subject: Re: [PATCH] hpsa: refine the pci enble/disable handling Steve, Joe, any chance you could review this patch and verify the sw reset case? Thanks, Tomas On 06/12/2014 05:29 PM, Tomas Henzl wrote: > When a second(kdump) kernel starts and the hard reset method is used > the driver calls pci_disable_device without previously enabling it, so > the kernel shows a warning - > [ 16.876248] WARNING: at drivers/pci/pci.c:1431 pci_disable_device+0x84/0x90() > [ 16.882686] Device hpsa > disabling already-disabled device > ... > This patch fixes it, in addition to this I tried to balance also some > other pairs of enable/disable device in the driver. > Unfortunately I wasn't able to verify the functionality for the case > of a sw reset, because of a lack of proper hw. > > Signed-off-by: Tomas Henzl <thenzl@xxxxxxxxxx> > --- > diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c index > 5858600..67c41b9 100644 > --- a/drivers/scsi/hpsa.c > +++ b/drivers/scsi/hpsa.c > @@ -5983,7 +5983,6 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) > /* Turn the board off. This is so that later pci_restore_state() > * won't turn the board on before the rest of config space is ready. > */ > - pci_disable_device(pdev); > pci_save_state(pdev); > > /* find the first memory BAR, so we can find the cfg table */ @@ > -6031,11 +6030,6 @@ static int hpsa_kdump_hard_reset_controller(struct pci_dev *pdev) > goto unmap_cfgtable; > > pci_restore_state(pdev); > - rc = pci_enable_device(pdev); > - if (rc) { > - dev_warn(&pdev->dev, "failed to enable device.\n"); > - goto unmap_cfgtable; > - } > pci_write_config_word(pdev, 4, command_register); > > /* Some devices (notably the HP Smart Array 5i Controller) @@ > -6548,6 +6542,12 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) > if (!reset_devices) > return 0; > > + rc = pci_enable_device(pdev); > + if (rc) { > + dev_warn(&pdev->dev, "failed to enable device.\n"); > + return -ENODEV; > + } > + > /* Reset the controller with a PCI power-cycle or via doorbell */ > rc = hpsa_kdump_hard_reset_controller(pdev); > > @@ -6556,10 +6556,11 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) > * "performant mode". Or, it might be 640x, which can't reset > * due to concerns about shared bbwc between 6402/6404 pair. > */ > - if (rc == -ENOTSUPP) > - return rc; /* just try to do the kdump anyhow. */ > - if (rc) > - return -ENODEV; > + if (rc) { > + if (rc != -ENOTSUPP) /* just try to do the kdump anyhow. */ > + rc = -ENODEV; > + goto out_disable; > + } > > /* Now try to get the controller to respond to a no-op */ > dev_warn(&pdev->dev, "Waiting for controller to respond to > no-op\n"); @@ -6570,7 +6571,11 @@ static int hpsa_init_reset_devices(struct pci_dev *pdev) > dev_warn(&pdev->dev, "no-op failed%s\n", > (i < 11 ? "; re-trying" : "")); > } > - return 0; > + > +out_disable: > + > + pci_disable_device(pdev); > + return rc; > } > > static int hpsa_allocate_cmd_pool(struct ctlr_info *h) @@ -6722,6 > +6727,7 @@ static void hpsa_undo_allocations_after_kdump_soft_reset(struct ctlr_info *h) > iounmap(h->transtable); > if (h->cfgtable) > iounmap(h->cfgtable); > + pci_disable_device(h->pdev); > pci_release_regions(h->pdev); > kfree(h); > } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
