Re: [PATCH v3 13/28] scsi: hisi_sas_v3_hw: use generic power management

[John Garry]

On 01/10/2020 13:24, Vaibhav Gupta wrote:

Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.

Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.

Signed-off-by: Vaibhav Gupta 
---

Hi Vaibhav,

Can you please consider rebasing this series? This driver has had many 
changes in the area of PM for v5.10-rc1


Also please note that my colleague provided a reviewed-by tag for v2, 
so, if no big significant changes from v2->v3, please add that tag. 
Obviously, in light of what I say about recent changes, that is not 
applicable now, but just for future reference.


Thanks,
John

[PATCH v3 13/28] scsi: hisi_sas_v3_hw: use generic power management

[Vaibhav Gupta]

Drivers should do only device-specific jobs. But in general, drivers using
legacy PCI PM framework for .suspend()/.resume() have to manage many PCI
PM-related tasks themselves which can be done by PCI Core itself. This
brings extra load on the driver and it directly calls PCI helper functions
to handle them.

Switch to the new generic framework by updating function signatures and
define a "struct dev_pm_ops" variable to bind PM callbacks. Also, remove
unnecessary calls to the PCI Helper functions along with the legacy
.suspend & .resume bindings.

Signed-off-by: Vaibhav Gupta 
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 30 +-
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c 
b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 8f0f4084a054..98b1848aabe7 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -3374,13 +3374,13 @@ enum {
hip08,
 };
 
-static int hisi_sas_v3_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused hisi_sas_v3_suspend(struct device *dev_d)
 {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct device *dev = hisi_hba->dev;
struct Scsi_Host *shost = hisi_hba->shost;
-   pci_power_t device_state;
int rc;
 
if (!pdev->pm_cap) {
@@ -3406,12 +3406,7 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, 
pm_message_t state)
 
hisi_sas_init_mem(hisi_hba);
 
-   device_state = pci_choose_state(pdev, state);
-   dev_warn(dev, "entering operating state [D%d]\n",
-   device_state);
-   pci_save_state(pdev);
-   pci_disable_device(pdev);
-   pci_set_power_state(pdev, device_state);
+   dev_warn(dev, "entering suspend state\n");
 
hisi_sas_release_tasks(hisi_hba);
 
@@ -3419,8 +3414,9 @@ static int hisi_sas_v3_suspend(struct pci_dev *pdev, 
pm_message_t state)
return 0;
 }
 
-static int hisi_sas_v3_resume(struct pci_dev *pdev)
+static int __maybe_unused hisi_sas_v3_resume(struct device *dev_d)
 {
+   struct pci_dev *pdev = to_pci_dev(dev_d);
struct sas_ha_struct *sha = pci_get_drvdata(pdev);
struct hisi_hba *hisi_hba = sha->lldd_ha;
struct Scsi_Host *shost = hisi_hba->shost;
@@ -3430,15 +3426,7 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)
 
dev_warn(dev, "resuming from operating state [D%d]\n",
 device_state);
-   pci_set_power_state(pdev, PCI_D0);
-   pci_restore_state(pdev);
-   rc = pci_enable_device(pdev);
-   if (rc) {
-   dev_err(dev, "enable device failed during resume (%d)\n", rc);
-   return rc;
-   }
 
-   pci_set_master(pdev);
scsi_unblock_requests(shost);
clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 
@@ -3446,7 +3434,6 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)
rc = hw_init_v3_hw(hisi_hba);
if (rc) {
scsi_remove_host(shost);
-   pci_disable_device(pdev);
return rc;
}
hisi_hba->hw->phys_init(hisi_hba);
@@ -3456,6 +3443,10 @@ static int hisi_sas_v3_resume(struct pci_dev *pdev)
return 0;
 }
 
+static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,
+hisi_sas_v3_suspend,
+hisi_sas_v3_resume);
+
 static const struct pci_device_id sas_v3_pci_table[] = {
{ PCI_VDEVICE(HUAWEI, 0xa230), hip08 },
{}
@@ -3472,8 +3463,7 @@ static struct pci_driver sas_v3_pci_driver = {
.id_table   = sas_v3_pci_table,
.probe  = hisi_sas_v3_probe,
.remove = hisi_sas_v3_remove,
-   .suspend= hisi_sas_v3_suspend,
-   .resume = hisi_sas_v3_resume,
+   .driver.pm  = &hisi_sas_v3_pm_ops,
.err_handler= &hisi_sas_err_handler,
 };
 
-- 
2.28.0