hpsa: refine the pci enble/disable handling
Subsystems (2)
PATCH 1/1 hpsa: refine the pci enble/disable handling
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.
---
drivers/scsi/hpsa.c +17 -11
@@ -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); }
Signature · 1 line
1.8.3.1
Steve, Joe,
any chance you could review this patch and verify the sw reset case? Thanks, Tomas
Quoted 88 lines · Tomas Henzl
When a second(kdump) kernel starts and the hard reset method is used the driver calls
pci_disable_devicewithout 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 hpsadisabling 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 <[email protected]>---
drivers/scsi/hpsa.c +17
-11@@ -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); }
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:[email protected]]
Sent: Monday, July 07, 2014 9:13 AM
To: '[email protected]'
Cc: [email protected]; [email protected]; 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
Quoted 88 lines · Tomas Henzl
When a second(kdump) kernel starts and the hard reset method is used the driver calls
pci_disable_devicewithout 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 hpsadisabling 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 <[email protected]>---
drivers/scsi/hpsa.c index renamed from drivers/scsi/hpsa.c +0
-05858600..67c41b9 100644
drivers/scsi/hpsa.c +11
-12@@ -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 tono-op\n"); @@ -6570,7 +6571,11 @@ static int
hpsa_init_reset_devices(struct pci_dev *pdev)drivers/scsi/hpsa.c +7
-1dev_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); }