Linux-SCSI Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v2 00/15] scsi: use generic power management
@ 2020-07-20 13:34 Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
                   ` (16 more replies)
  0 siblings, 17 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Linux Kernel Mentee: Remove Legacy Power Management.

The purpose of this patch series is to upgrade power management in scsi
drivers. This has been done by upgrading .suspend() and .resume() callbacks.

The upgrade makes sure that the involvement of PCI Core does not change the
order of operations executed in a driver. Thus, does not change its behavior.

In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
helper functions like pci_request/release_regions(), pci_set_power_state(),
pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
their job.

The conversion requires the removal of those function calls, change the
callbacks' definition accordingly and make use of dev_pm_ops structure.

v2: kbuild error in v1.

All patches are compile-tested only.

Test tools:
    - Compiler: gcc (GCC) 10.1.0
    - allmodconfig build: make -j$(nproc) W=1 all

Vaibhav Gupta (15):
  scsi: megaraid_sas: use generic power management
  scsi: aacraid: use generic power management
  scsi: aic7xxx: use generic power management
  scsi: aic79xx: use generic power management
  scsi: arcmsr: use generic power management
  scsi: esas2r: use generic power management
  scsi: hisi_sas_v3_hw: use generic power management
  scsi: mpt3sas_scsih: use generic power management
  scsi: lpfc: use generic power management
  scsi: pm_8001: use generic power management
  scsi: hpsa: use generic power management
  scsi: 3w-9xxx: use generic power management
  scsi: 3w-sas: use generic power management
  scsi: mvumi: use generic power management
  scsi: pmcraid: use generic power management

 drivers/scsi/3w-9xxx.c                    |  30 ++-----
 drivers/scsi/3w-sas.c                     |  31 ++-----
 drivers/scsi/aacraid/linit.c              |  34 ++------
 drivers/scsi/aic7xxx/aic79xx.h            |  12 +--
 drivers/scsi/aic7xxx/aic79xx_core.c       |   8 +-
 drivers/scsi/aic7xxx/aic79xx_osm_pci.c    |  43 +++-------
 drivers/scsi/aic7xxx/aic79xx_pci.c        |   6 +-
 drivers/scsi/aic7xxx/aic7xxx.h            |  10 +--
 drivers/scsi/aic7xxx/aic7xxx_core.c       |   6 +-
 drivers/scsi/aic7xxx/aic7xxx_osm_pci.c    |  46 +++-------
 drivers/scsi/aic7xxx/aic7xxx_pci.c        |   4 +-
 drivers/scsi/arcmsr/arcmsr_hba.c          |  35 +++-----
 drivers/scsi/esas2r/esas2r.h              |   5 +-
 drivers/scsi/esas2r/esas2r_init.c         |  48 +++--------
 drivers/scsi/esas2r/esas2r_main.c         |   3 +-
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c    |  32 +++----
 drivers/scsi/hpsa.c                       |  12 +--
 drivers/scsi/lpfc/lpfc_init.c             | 100 +++++++---------------
 drivers/scsi/megaraid/megaraid_sas_base.c |  61 ++++---------
 drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  36 +++-----
 drivers/scsi/mvumi.c                      |  49 +++--------
 drivers/scsi/pm8001/pm8001_init.c         |  46 ++++------
 drivers/scsi/pmcraid.c                    |  44 +++-------
 23 files changed, 212 insertions(+), 489 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 16:57   ` Vaibhav Gupta
  2020-09-08 17:32   ` Bjorn Helgaas
  2020-07-20 13:34 ` [PATCH v2 02/15] scsi: aacraid: " Vaibhav Gupta
                   ` (15 subsequent siblings)
  16 siblings, 2 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM hooks, it was the responsibility of a driver to manage PCI
states and also the device's power state. The generic approach is to let
the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks. As the .suspend() work with "struct instance*",
extract it from "struct device*" using dev_get_drv_data().

Driver was also using PCI helper functions like pci_save/restore_state(),
pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
 1 file changed, 16 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
index 00668335c2af..4a6ee7778977 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
 	megasas_return_cmd(instance, cmd);
 }
 
-#ifdef CONFIG_PM
 /**
  * megasas_suspend -	driver suspend entry point
- * @pdev:		PCI device structure
- * @state:		PCI power state to suspend routine
+ * @dev:		Device structure
  */
-static int
-megasas_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused
+megasas_suspend(struct device *dev)
 {
-	struct megasas_instance *instance;
-
-	instance = pci_get_drvdata(pdev);
+	struct megasas_instance *instance = dev_get_drvdata(dev);
 
 	if (!instance)
 		return 0;
 
 	instance->unload = 1;
 
-	dev_info(&pdev->dev, "%s is called\n", __func__);
+	dev_info(dev, "%s is called\n", __func__);
 
 	/* Shutdown SR-IOV heartbeat timer */
 	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
@@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
 
 	tasklet_kill(&instance->isr_tasklet);
 
-	pci_set_drvdata(instance->pdev, instance);
+	dev_set_drvdata(dev, instance);
 	instance->instancet->disable_intr(instance);
 
 	megasas_destroy_irqs(instance);
@@ -7587,48 +7583,28 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
 	if (instance->msix_vectors)
 		pci_free_irq_vectors(instance->pdev);
 
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
 	return 0;
 }
 
 /**
  * megasas_resume-      driver resume entry point
- * @pdev:               PCI device structure
+ * @dev:              Device structure
  */
-static int
-megasas_resume(struct pci_dev *pdev)
+static int __maybe_unused
+megasas_resume(struct device *dev)
 {
 	int rval;
 	struct Scsi_Host *host;
-	struct megasas_instance *instance;
+	struct megasas_instance *instance = dev_get_drvdata(dev);
 	u32 status_reg;
 
-	instance = pci_get_drvdata(pdev);
-
 	if (!instance)
 		return 0;
 
 	host = instance->host;
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
+	device_wakeup_disable(dev);
 
-	dev_info(&pdev->dev, "%s is called\n", __func__);
-	/*
-	 * PCI prepping: enable device set bus mastering and dma mask
-	 */
-	rval = pci_enable_device_mem(pdev);
-
-	if (rval) {
-		dev_err(&pdev->dev, "Enable device failed\n");
-		return rval;
-	}
-
-	pci_set_master(pdev);
+	dev_info(dev, "%s is called\n", __func__);
 
 	/*
 	 * We expect the FW state to be READY
@@ -7754,14 +7730,8 @@ megasas_resume(struct pci_dev *pdev)
 fail_set_dma_mask:
 fail_ready_state:
 
-	pci_disable_device(pdev);
-
 	return -ENODEV;
 }
-#else
-#define megasas_suspend	NULL
-#define megasas_resume	NULL
-#endif
 
 static inline int
 megasas_wait_for_adapter_operational(struct megasas_instance *instance)
@@ -7931,7 +7901,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
 
 /**
  * megasas_shutdown -	Shutdown entry point
- * @device:		Generic device structure
+ * @pdev:		PCI device structure
  */
 static void megasas_shutdown(struct pci_dev *pdev)
 {
@@ -8508,6 +8478,8 @@ static const struct file_operations megasas_mgmt_fops = {
 	.llseek = noop_llseek,
 };
 
+static SIMPLE_DEV_PM_OPS(megasas_pm_ops, megasas_suspend, megasas_resume);
+
 /*
  * PCI hotplug support registration structure
  */
@@ -8517,8 +8489,7 @@ static struct pci_driver megasas_pci_driver = {
 	.id_table = megasas_pci_table,
 	.probe = megasas_probe_one,
 	.remove = megasas_detach_one,
-	.suspend = megasas_suspend,
-	.resume = megasas_resume,
+	.driver.pm = &megasas_pm_ops,
 	.shutdown = megasas_shutdown,
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 02/15] scsi: aacraid: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-07-23  6:58   ` Balsundar.P
  2020-09-08 16:58   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 03/15] scsi: aic7xxx: " Vaibhav Gupta
                   ` (14 subsequent siblings)
  16 siblings, 2 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM hooks, it was the responsibility of a driver to manage PCI
states and also the device's power state. The generic approach is to let
the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks.

Driver was using PCI helper functions like pci_save/restore_state(),
pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/aacraid/linit.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
index a308e86a97f1..1e44868ee953 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1910,11 +1910,9 @@ static int aac_acquire_resources(struct aac_dev *dev)
 
 }
 
-#if (defined(CONFIG_PM))
-static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused aac_suspend(struct device *dev)
 {
-
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev);
 	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
 
 	scsi_host_block(shost);
@@ -1923,29 +1921,16 @@ static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
 
 	aac_release_resources(aac);
 
-	pci_set_drvdata(pdev, shost);
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
 	return 0;
 }
 
-static int aac_resume(struct pci_dev *pdev)
+static int __maybe_unused aac_resume(struct device *dev)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev);
 	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
-	int r;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-	r = pci_enable_device(pdev);
+	device_wakeup_disable(dev);
 
-	if (r)
-		goto fail_device;
-
-	pci_set_master(pdev);
 	if (aac_acquire_resources(aac))
 		goto fail_device;
 	/*
@@ -1960,10 +1945,8 @@ static int aac_resume(struct pci_dev *pdev)
 fail_device:
 	printk(KERN_INFO "%s%d: resume failed.\n", aac->name, aac->id);
 	scsi_host_put(shost);
-	pci_disable_device(pdev);
 	return -ENODEV;
 }
-#endif
 
 static void aac_shutdown(struct pci_dev *dev)
 {
@@ -2108,15 +2091,14 @@ static struct pci_error_handlers aac_pci_err_handler = {
 	.resume			= aac_pci_resume,
 };
 
+static SIMPLE_DEV_PM_OPS(aac_pm_ops, aac_suspend, aac_resume);
+
 static struct pci_driver aac_pci_driver = {
 	.name		= AAC_DRIVERNAME,
 	.id_table	= aac_pci_tbl,
 	.probe		= aac_probe_one,
 	.remove		= aac_remove_one,
-#if (defined(CONFIG_PM))
-	.suspend	= aac_suspend,
-	.resume		= aac_resume,
-#endif
+	.driver.pm	= &aac_pm_ops,
 	.shutdown	= aac_shutdown,
 	.err_handler    = &aac_pci_err_handler,
 };
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 03/15] scsi: aic7xxx: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 02/15] scsi: aacraid: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 16:59   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 04/15] scsi: aic79xx: " Vaibhav Gupta
                   ` (13 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM hooks, it was the responsibility of a driver to manage PCI
states and also the device's power state. The generic approach is to let
the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks.

Driver was using PCI helper functions like pci_save/restore_state(),
pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Also, .suspend() and .resume() are invoking other functions for PM, which
were againg bounded by "#ifdef CONFIG_PM" directive. Remove the directive
and mark those functions as "__maybe_unused".

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/aic7xxx/aic7xxx.h         | 10 ++----
 drivers/scsi/aic7xxx/aic7xxx_core.c    |  6 ++--
 drivers/scsi/aic7xxx/aic7xxx_osm_pci.c | 46 ++++++--------------------
 drivers/scsi/aic7xxx/aic7xxx_pci.c     |  4 +--
 4 files changed, 17 insertions(+), 49 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic7xxx.h b/drivers/scsi/aic7xxx/aic7xxx.h
index 88b90f9806c9..11a09798e6b5 100644
--- a/drivers/scsi/aic7xxx/aic7xxx.h
+++ b/drivers/scsi/aic7xxx/aic7xxx.h
@@ -1134,9 +1134,7 @@ const struct ahc_pci_identity	*ahc_find_pci_device(ahc_dev_softc_t);
 int			 ahc_pci_config(struct ahc_softc *,
 					const struct ahc_pci_identity *);
 int			 ahc_pci_test_register_access(struct ahc_softc *);
-#ifdef CONFIG_PM
-void			 ahc_pci_resume(struct ahc_softc *ahc);
-#endif
+void __maybe_unused	 ahc_pci_resume(struct ahc_softc *ahc);
 
 /*************************** EISA/VL Front End ********************************/
 struct aic7770_identity *aic7770_find_device(uint32_t);
@@ -1160,10 +1158,8 @@ int			 ahc_chip_init(struct ahc_softc *ahc);
 int			 ahc_init(struct ahc_softc *ahc);
 void			 ahc_intr_enable(struct ahc_softc *ahc, int enable);
 void			 ahc_pause_and_flushwork(struct ahc_softc *ahc);
-#ifdef CONFIG_PM
-int			 ahc_suspend(struct ahc_softc *ahc); 
-int			 ahc_resume(struct ahc_softc *ahc);
-#endif
+int __maybe_unused	 ahc_suspend(struct ahc_softc *ahc);
+int __maybe_unused	 ahc_resume(struct ahc_softc *ahc);
 void			 ahc_set_unit(struct ahc_softc *, int);
 void			 ahc_set_name(struct ahc_softc *, char *);
 void			 ahc_free(struct ahc_softc *ahc);
diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c b/drivers/scsi/aic7xxx/aic7xxx_core.c
index 3d4df906fa4f..c7eb238a9599 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_core.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
@@ -5590,8 +5590,7 @@ ahc_pause_and_flushwork(struct ahc_softc *ahc)
 	ahc->flags &= ~AHC_ALL_INTERRUPTS;
 }
 
-#ifdef CONFIG_PM
-int
+int __maybe_unused
 ahc_suspend(struct ahc_softc *ahc)
 {
 
@@ -5617,7 +5616,7 @@ ahc_suspend(struct ahc_softc *ahc)
 	return (0);
 }
 
-int
+int __maybe_unused
 ahc_resume(struct ahc_softc *ahc)
 {
 
@@ -5626,7 +5625,6 @@ ahc_resume(struct ahc_softc *ahc)
 	ahc_restart(ahc);
 	return (0);
 }
-#endif
 /************************** Busy Target Table *********************************/
 /*
  * Return the untagged transaction id for a given target/channel lun.
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
index 9b293b1f0b71..a07e94fac673 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
@@ -121,47 +121,23 @@ static const struct pci_device_id ahc_linux_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, ahc_linux_pci_id_table);
 
-#ifdef CONFIG_PM
-static int
-ahc_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
+static int __maybe_unused
+ahc_linux_pci_dev_suspend(struct device *dev)
 {
-	struct ahc_softc *ahc = pci_get_drvdata(pdev);
-	int rc;
-
-	if ((rc = ahc_suspend(ahc)))
-		return rc;
+	struct ahc_softc *ahc = dev_get_drvdata(dev);
 
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-
-	if (mesg.event & PM_EVENT_SLEEP)
-		pci_set_power_state(pdev, PCI_D3hot);
-
-	return rc;
+	return ahc_suspend(ahc);
 }
 
-static int
-ahc_linux_pci_dev_resume(struct pci_dev *pdev)
+static int __maybe_unused
+ahc_linux_pci_dev_resume(struct device *dev)
 {
-	struct ahc_softc *ahc = pci_get_drvdata(pdev);
-	int rc;
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
-	if ((rc = pci_enable_device(pdev))) {
-		dev_printk(KERN_ERR, &pdev->dev,
-			   "failed to enable device after resume (%d)\n", rc);
-		return rc;
-	}
-
-	pci_set_master(pdev);
+	struct ahc_softc *ahc = dev_get_drvdata(dev);
 
 	ahc_pci_resume(ahc);
 
 	return (ahc_resume(ahc));
 }
-#endif
 
 static void
 ahc_linux_pci_dev_remove(struct pci_dev *pdev)
@@ -319,14 +295,14 @@ ahc_pci_write_config(ahc_dev_softc_t pci, int reg, uint32_t value, int width)
 	}
 }
 
+static SIMPLE_DEV_PM_OPS(ahc_linux_pci_dev_pm_ops,
+			 ahc_linux_pci_dev_suspend,
+			 ahc_linux_pci_dev_resume);
 
 static struct pci_driver aic7xxx_pci_driver = {
 	.name		= "aic7xxx",
 	.probe		= ahc_linux_pci_dev_probe,
-#ifdef CONFIG_PM
-	.suspend	= ahc_linux_pci_dev_suspend,
-	.resume		= ahc_linux_pci_dev_resume,
-#endif
+	.driver.pm	= &ahc_linux_pci_dev_pm_ops,
 	.remove		= ahc_linux_pci_dev_remove,
 	.id_table	= ahc_linux_pci_id_table
 };
diff --git a/drivers/scsi/aic7xxx/aic7xxx_pci.c b/drivers/scsi/aic7xxx/aic7xxx_pci.c
index 656f680c7802..dab3a6d12c4d 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_pci.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_pci.c
@@ -2008,8 +2008,7 @@ ahc_pci_chip_init(struct ahc_softc *ahc)
 	return (ahc_chip_init(ahc));
 }
 
-#ifdef CONFIG_PM
-void
+void __maybe_unused
 ahc_pci_resume(struct ahc_softc *ahc)
 {
 	/*
@@ -2040,7 +2039,6 @@ ahc_pci_resume(struct ahc_softc *ahc)
 		ahc_release_seeprom(&sd);
 	}
 }
-#endif
 
 static int
 ahc_aic785X_setup(struct ahc_softc *ahc)
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 04/15] scsi: aic79xx: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (2 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 03/15] scsi: aic7xxx: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 16:59   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 05/15] scsi: arcmsr: " Vaibhav Gupta
                   ` (12 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM hooks, it was the responsibility of a driver to manage PCI
states and also the device's power state. The generic approach is to let
the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks.

Driver was using PCI helper functions like pci_save/restore_state(),
pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Also, .suspend() and .resume() were invoking other functions for PM, which
were againg bounded by "#ifdef CONFIG_PM" directive. Remove them and mark
them as "__maybe_unused".

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/aic7xxx/aic79xx.h         | 12 +++----
 drivers/scsi/aic7xxx/aic79xx_core.c    |  8 ++---
 drivers/scsi/aic7xxx/aic79xx_osm_pci.c | 43 +++++++-------------------
 drivers/scsi/aic7xxx/aic79xx_pci.c     |  6 ++--
 4 files changed, 20 insertions(+), 49 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx.h b/drivers/scsi/aic7xxx/aic79xx.h
index 9a515551641c..dd5dfd4f30a5 100644
--- a/drivers/scsi/aic7xxx/aic79xx.h
+++ b/drivers/scsi/aic7xxx/aic79xx.h
@@ -1330,10 +1330,8 @@ const struct	ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
 int			  ahd_pci_config(struct ahd_softc *,
 					 const struct ahd_pci_identity *);
 int	ahd_pci_test_register_access(struct ahd_softc *);
-#ifdef CONFIG_PM
-void	ahd_pci_suspend(struct ahd_softc *);
-void	ahd_pci_resume(struct ahd_softc *);
-#endif
+void __maybe_unused	ahd_pci_suspend(struct ahd_softc *);
+void __maybe_unused	ahd_pci_resume(struct ahd_softc *);
 
 /************************** SCB and SCB queue management **********************/
 void		ahd_qinfifo_requeue_tail(struct ahd_softc *ahd,
@@ -1344,10 +1342,8 @@ struct ahd_softc	*ahd_alloc(void *platform_arg, char *name);
 int			 ahd_softc_init(struct ahd_softc *);
 void			 ahd_controller_info(struct ahd_softc *ahd, char *buf);
 int			 ahd_init(struct ahd_softc *ahd);
-#ifdef CONFIG_PM
-int			 ahd_suspend(struct ahd_softc *ahd);
-void			 ahd_resume(struct ahd_softc *ahd);
-#endif
+int __maybe_unused	 ahd_suspend(struct ahd_softc *ahd);
+void __maybe_unused	 ahd_resume(struct ahd_softc *ahd);
 int			 ahd_default_config(struct ahd_softc *ahd);
 int			 ahd_parse_vpddata(struct ahd_softc *ahd,
 					   struct vpd_config *vpd);
diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c b/drivers/scsi/aic7xxx/aic79xx_core.c
index e4a09b93d00c..06ee7abd0f8f 100644
--- a/drivers/scsi/aic7xxx/aic79xx_core.c
+++ b/drivers/scsi/aic7xxx/aic79xx_core.c
@@ -7880,11 +7880,9 @@ ahd_pause_and_flushwork(struct ahd_softc *ahd)
 	ahd->flags &= ~AHD_ALL_INTERRUPTS;
 }
 
-#ifdef CONFIG_PM
-int
+int __maybe_unused
 ahd_suspend(struct ahd_softc *ahd)
 {
-
 	ahd_pause_and_flushwork(ahd);
 
 	if (LIST_FIRST(&ahd->pending_scbs) != NULL) {
@@ -7895,15 +7893,13 @@ ahd_suspend(struct ahd_softc *ahd)
 	return (0);
 }
 
-void
+void __maybe_unused
 ahd_resume(struct ahd_softc *ahd)
 {
-
 	ahd_reset(ahd, /*reinit*/TRUE);
 	ahd_intr_enable(ahd, TRUE); 
 	ahd_restart(ahd);
 }
-#endif
 
 /************************** Busy Target Table *********************************/
 /*
diff --git a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
index 8b891a05d9e7..07b670b80f1b 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
@@ -74,11 +74,10 @@ static const struct pci_device_id ahd_linux_pci_id_table[] = {
 
 MODULE_DEVICE_TABLE(pci, ahd_linux_pci_id_table);
 
-#ifdef CONFIG_PM
-static int
-ahd_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
+static int __maybe_unused
+ahd_linux_pci_dev_suspend(struct device *dev)
 {
-	struct ahd_softc *ahd = pci_get_drvdata(pdev);
+	struct ahd_softc *ahd = dev_get_drvdata(dev);
 	int rc;
 
 	if ((rc = ahd_suspend(ahd)))
@@ -86,39 +85,20 @@ ahd_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
 
 	ahd_pci_suspend(ahd);
 
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-
-	if (mesg.event & PM_EVENT_SLEEP)
-		pci_set_power_state(pdev, PCI_D3hot);
-
 	return rc;
 }
 
-static int
-ahd_linux_pci_dev_resume(struct pci_dev *pdev)
+static int __maybe_unused
+ahd_linux_pci_dev_resume(struct device *dev)
 {
-	struct ahd_softc *ahd = pci_get_drvdata(pdev);
-	int rc;
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
-	if ((rc = pci_enable_device(pdev))) {
-		dev_printk(KERN_ERR, &pdev->dev,
-			   "failed to enable device after resume (%d)\n", rc);
-		return rc;
-	}
-
-	pci_set_master(pdev);
+	struct ahd_softc *ahd = dev_get_drvdata(dev);
 
 	ahd_pci_resume(ahd);
 
 	ahd_resume(ahd);
 
-	return rc;
+	return 0;
 }
-#endif
 
 static void
 ahd_linux_pci_dev_remove(struct pci_dev *pdev)
@@ -224,13 +204,14 @@ ahd_linux_pci_dev_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return (0);
 }
 
+static SIMPLE_DEV_PM_OPS(ahd_linux_pci_dev_pm_ops,
+			 ahd_linux_pci_dev_suspend,
+			 ahd_linux_pci_dev_resume);
+
 static struct pci_driver aic79xx_pci_driver = {
 	.name		= "aic79xx",
 	.probe		= ahd_linux_pci_dev_probe,
-#ifdef CONFIG_PM
-	.suspend	= ahd_linux_pci_dev_suspend,
-	.resume		= ahd_linux_pci_dev_resume,
-#endif
+	.driver.pm	= &ahd_linux_pci_dev_pm_ops,
 	.remove		= ahd_linux_pci_dev_remove,
 	.id_table	= ahd_linux_pci_id_table
 };
diff --git a/drivers/scsi/aic7xxx/aic79xx_pci.c b/drivers/scsi/aic7xxx/aic79xx_pci.c
index 8397ae93f7dd..2f0bdb9225a4 100644
--- a/drivers/scsi/aic7xxx/aic79xx_pci.c
+++ b/drivers/scsi/aic7xxx/aic79xx_pci.c
@@ -377,8 +377,7 @@ ahd_pci_config(struct ahd_softc *ahd, const struct ahd_pci_identity *entry)
 	return ahd_pci_map_int(ahd);
 }
 
-#ifdef CONFIG_PM
-void
+void __maybe_unused
 ahd_pci_suspend(struct ahd_softc *ahd)
 {
 	/*
@@ -394,7 +393,7 @@ ahd_pci_suspend(struct ahd_softc *ahd)
 
 }
 
-void
+void __maybe_unused
 ahd_pci_resume(struct ahd_softc *ahd)
 {
 	ahd_pci_write_config(ahd->dev_softc, DEVCONFIG,
@@ -404,7 +403,6 @@ ahd_pci_resume(struct ahd_softc *ahd)
 	ahd_pci_write_config(ahd->dev_softc, CSIZE_LATTIME,
 			     ahd->suspend_state.pci_state.csize_lattime, /*bytes*/1);
 }
-#endif
 
 /*
  * Perform some simple tests that should catch situations where
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 05/15] scsi: arcmsr: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (3 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 04/15] scsi: aic79xx: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:00   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 06/15] scsi: esas2r: " Vaibhav Gupta
                   ` (11 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM hooks, it was the responsibility of a driver to manage PCI
states and also the device's power state. The generic approach is to let
the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks.

Driver was also using PCI helper functions like pci_save/restore_state(),
pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/arcmsr/arcmsr_hba.c | 35 ++++++++++++--------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
index 30914c8f29cc..7e098ddcc4f5 100644
--- a/drivers/scsi/arcmsr/arcmsr_hba.c
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c
@@ -113,8 +113,8 @@ static int arcmsr_bios_param(struct scsi_device *sdev,
 static int arcmsr_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd);
 static int arcmsr_probe(struct pci_dev *pdev,
 				const struct pci_device_id *id);
-static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state);
-static int arcmsr_resume(struct pci_dev *pdev);
+static int __maybe_unused arcmsr_suspend(struct device *dev);
+static int __maybe_unused arcmsr_resume(struct device *dev);
 static void arcmsr_remove(struct pci_dev *pdev);
 static void arcmsr_shutdown(struct pci_dev *pdev);
 static void arcmsr_iop_init(struct AdapterControlBlock *acb);
@@ -213,13 +213,14 @@ static struct pci_device_id arcmsr_device_id_table[] = {
 };
 MODULE_DEVICE_TABLE(pci, arcmsr_device_id_table);
 
+static SIMPLE_DEV_PM_OPS(arcmsr_pm_ops, arcmsr_suspend, arcmsr_resume);
+
 static struct pci_driver arcmsr_pci_driver = {
 	.name			= "arcmsr",
 	.id_table		= arcmsr_device_id_table,
 	.probe			= arcmsr_probe,
 	.remove			= arcmsr_remove,
-	.suspend		= arcmsr_suspend,
-	.resume			= arcmsr_resume,
+	.driver.pm		= &arcmsr_pm_ops,
 	.shutdown		= arcmsr_shutdown,
 };
 /*
@@ -1065,14 +1066,14 @@ static void arcmsr_free_irq(struct pci_dev *pdev,
 	pci_free_irq_vectors(pdev);
 }
 
-static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused arcmsr_suspend(struct device *dev)
 {
-	uint32_t intmask_org;
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 	struct AdapterControlBlock *acb =
 		(struct AdapterControlBlock *)host->hostdata;
 
-	intmask_org = arcmsr_disable_outbound_ints(acb);
+	arcmsr_disable_outbound_ints(acb);
 	arcmsr_free_irq(pdev, acb);
 	del_timer_sync(&acb->eternal_timer);
 	if (set_date_time)
@@ -1080,29 +1081,21 @@ static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state)
 	flush_work(&acb->arcmsr_do_message_isr_bh);
 	arcmsr_stop_adapter_bgrb(acb);
 	arcmsr_flush_adapter_cache(acb);
-	pci_set_drvdata(pdev, host);
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
 	return 0;
 }
 
-static int arcmsr_resume(struct pci_dev *pdev)
+static int __maybe_unused arcmsr_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 	struct AdapterControlBlock *acb =
 		(struct AdapterControlBlock *)host->hostdata;
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-	if (pci_enable_device(pdev)) {
-		pr_warn("%s: pci_enable_device error\n", __func__);
-		return -ENODEV;
-	}
+	device_wakeup_disable(dev);
+
 	if (arcmsr_set_dma_mask(acb))
 		goto controller_unregister;
-	pci_set_master(pdev);
+
 	if (arcmsr_request_irq(pdev, acb) == FAILED)
 		goto controller_stop;
 	switch (acb->adapter_type) {
@@ -1137,9 +1130,7 @@ static int arcmsr_resume(struct pci_dev *pdev)
 	scsi_remove_host(host);
 	arcmsr_free_ccb_pool(acb);
 	arcmsr_unmap_pciregion(acb);
-	pci_release_regions(pdev);
 	scsi_host_put(host);
-	pci_disable_device(pdev);
 	return -ENODEV;
 }
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 06/15] scsi: esas2r: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (4 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 05/15] scsi: arcmsr: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:01   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 07/15] scsi: hisi_sas_v3_hw: " Vaibhav Gupta
                   ` (10 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/esas2r/esas2r.h      |  5 ++--
 drivers/scsi/esas2r/esas2r_init.c | 48 +++++++++----------------------
 drivers/scsi/esas2r/esas2r_main.c |  3 +-
 3 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index 7f43b95f4e94..6ad3e0871ef0 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -996,8 +996,9 @@ void esas2r_adapter_tasklet(unsigned long context);
 irqreturn_t esas2r_interrupt(int irq, void *dev_id);
 irqreturn_t esas2r_msi_interrupt(int irq, void *dev_id);
 void esas2r_kickoff_timer(struct esas2r_adapter *a);
-int esas2r_suspend(struct pci_dev *pcid, pm_message_t state);
-int esas2r_resume(struct pci_dev *pcid);
+
+extern const struct dev_pm_ops esas2r_pm_ops;
+
 void esas2r_fw_event_off(struct esas2r_adapter *a);
 void esas2r_fw_event_on(struct esas2r_adapter *a);
 bool esas2r_nvram_write(struct esas2r_adapter *a, struct esas2r_request *rq,
diff --git a/drivers/scsi/esas2r/esas2r_init.c b/drivers/scsi/esas2r/esas2r_init.c
index eb7d139ffc00..6c5854969791 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -640,53 +640,31 @@ void esas2r_kill_adapter(int i)
 	}
 }
 
-int esas2r_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused esas2r_suspend(struct device *dev)
 {
-	struct Scsi_Host *host = pci_get_drvdata(pdev);
-	u32 device_state;
+	struct Scsi_Host *host = dev_get_drvdata(dev);
 	struct esas2r_adapter *a = (struct esas2r_adapter *)host->hostdata;
 
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "suspending adapter()");
+	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "suspending adapter()");
 	if (!a)
 		return -ENODEV;
 
 	esas2r_adapter_power_down(a, 1);
-	device_state = pci_choose_state(pdev, state);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_save_state() called");
-	pci_save_state(pdev);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_disable_device() called");
-	pci_disable_device(pdev);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_set_power_state() called");
-	pci_set_power_state(pdev, device_state);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "esas2r_suspend(): 0");
+	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "esas2r_suspend(): 0");
 	return 0;
 }
 
-int esas2r_resume(struct pci_dev *pdev)
+static int __maybe_unused esas2r_resume(struct device *dev)
 {
-	struct Scsi_Host *host = pci_get_drvdata(pdev);
+	struct Scsi_Host *host = dev_get_drvdata(dev);
 	struct esas2r_adapter *a = (struct esas2r_adapter *)host->hostdata;
-	int rez;
+	int rez = 0;
 
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "resuming adapter()");
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_set_power_state(PCI_D0) "
+	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "resuming adapter()");
+	esas2r_log_dev(ESAS2R_LOG_INFO, dev,
+		       "device_wakeup_disable() "
 		       "called");
-	pci_set_power_state(pdev, PCI_D0);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_enable_wake(PCI_D0, 0) "
-		       "called");
-	pci_enable_wake(pdev, PCI_D0, 0);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_restore_state() called");
-	pci_restore_state(pdev);
-	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
-		       "pci_enable_device() called");
-	rez = pci_enable_device(pdev);
-	pci_set_master(pdev);
+	device_wakeup_disable(dev);
 
 	if (!a) {
 		rez = -ENODEV;
@@ -730,11 +708,13 @@ int esas2r_resume(struct pci_dev *pdev)
 	}
 
 error_exit:
-	esas2r_log_dev(ESAS2R_LOG_CRIT, &(pdev->dev), "esas2r_resume(): %d",
+	esas2r_log_dev(ESAS2R_LOG_CRIT, dev, "esas2r_resume(): %d",
 		       rez);
 	return rez;
 }
 
+SIMPLE_DEV_PM_OPS(esas2r_pm_ops, esas2r_suspend, esas2r_resume);
+
 bool esas2r_set_degraded_mode(struct esas2r_adapter *a, char *error_str)
 {
 	set_bit(AF_DEGRADED_MODE, &a->flags);
diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
index 7b49e2e9fcde..aab3ea580e6b 100644
--- a/drivers/scsi/esas2r/esas2r_main.c
+++ b/drivers/scsi/esas2r/esas2r_main.c
@@ -346,8 +346,7 @@ static struct pci_driver
 	.id_table	= esas2r_pci_table,
 	.probe		= esas2r_probe,
 	.remove		= esas2r_remove,
-	.suspend	= esas2r_suspend,
-	.resume		= esas2r_resume,
+	.driver.pm	= &esas2r_pm_ops,
 };
 
 static int esas2r_probe(struct pci_dev *pcid,
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 07/15] scsi: hisi_sas_v3_hw: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (5 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 06/15] scsi: esas2r: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-07-21  1:36   ` chenxiang (M)
  2020-09-08 17:02   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 08/15] scsi: mpt3sas_scsih: " Vaibhav Gupta
                   ` (9 subsequent siblings)
  16 siblings, 2 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 32 +++++++++-----------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index 55e2321a65bc..824bfbe1abbb 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,16 +3426,8 @@ 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_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-	rc = pci_enable_device(pdev);
-	if (rc) {
-		dev_err(dev, "enable device failed during resume (%d)\n", rc);
-		return rc;
-	}
+	device_wakeup_disable(dev_d);
 
-	pci_set_master(pdev);
 	scsi_unblock_requests(shost);
 	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 
@@ -3447,7 +3435,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);
@@ -3468,13 +3455,16 @@ static const struct pci_error_handlers hisi_sas_err_handler = {
 	.reset_done	= hisi_sas_reset_done_v3_hw,
 };
 
+static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,
+			 hisi_sas_v3_suspend,
+			 hisi_sas_v3_resume);
+
 static struct pci_driver sas_v3_pci_driver = {
 	.name		= DRV_NAME,
 	.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.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 08/15] scsi: mpt3sas_scsih: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (6 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 07/15] scsi: hisi_sas_v3_hw: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:03   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 09/15] scsi: lpfc: " Vaibhav Gupta
                   ` (8 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/mpt3sas/mpt3sas_scsih.c | 36 +++++++++++-----------------
 1 file changed, 14 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
index 08fc4b381056..f3c6e68b2921 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
@@ -10829,44 +10829,40 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	return rv;
 }
 
-#ifdef CONFIG_PM
 /**
  * scsih_suspend - power management suspend main entry point
- * @pdev: PCI device struct
- * @state: PM state change to (usually PCI_D3)
+ * @dev: Device struct
  *
  * Return: 0 success, anything else error.
  */
-static int
-scsih_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused
+scsih_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
-	pci_power_t device_state;
 
 	mpt3sas_base_stop_watchdog(ioc);
 	flush_scheduled_work();
 	scsi_block_requests(shost);
 	_scsih_nvme_shutdown(ioc);
-	device_state = pci_choose_state(pdev, state);
-	ioc_info(ioc, "pdev=0x%p, slot=%s, entering operating state [D%d]\n",
-		 pdev, pci_name(pdev), device_state);
+	ioc_info(ioc, "pdev=0x%p, slot=%s, entering suspended state\n",
+		 pdev, pci_name(pdev));
 
-	pci_save_state(pdev);
 	mpt3sas_base_free_resources(ioc);
-	pci_set_power_state(pdev, device_state);
 	return 0;
 }
 
 /**
  * scsih_resume - power management resume main entry point
- * @pdev: PCI device struct
+ * @dev: Device struct
  *
  * Return: 0 success, anything else error.
  */
-static int
-scsih_resume(struct pci_dev *pdev)
+static int __maybe_unused
+scsih_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *shost = pci_get_drvdata(pdev);
 	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
 	pci_power_t device_state = pdev->current_state;
@@ -10875,9 +10871,7 @@ scsih_resume(struct pci_dev *pdev)
 	ioc_info(ioc, "pdev=0x%p, slot=%s, previous operating state [D%d]\n",
 		 pdev, pci_name(pdev), device_state);
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
+	device_wakeup_disable(dev);
 	ioc->pdev = pdev;
 	r = mpt3sas_base_map_resources(ioc);
 	if (r)
@@ -10888,7 +10882,6 @@ scsih_resume(struct pci_dev *pdev)
 	mpt3sas_base_start_watchdog(ioc);
 	return 0;
 }
-#endif /* CONFIG_PM */
 
 /**
  * scsih_pci_error_detected - Called when a PCI error is detected.
@@ -11162,6 +11155,8 @@ static struct pci_error_handlers _mpt3sas_err_handler = {
 	.resume		= scsih_pci_resume,
 };
 
+static SIMPLE_DEV_PM_OPS(scsih_pm_ops, scsih_suspend, scsih_resume);
+
 static struct pci_driver mpt3sas_driver = {
 	.name		= MPT3SAS_DRIVER_NAME,
 	.id_table	= mpt3sas_pci_table,
@@ -11169,10 +11164,7 @@ static struct pci_driver mpt3sas_driver = {
 	.remove		= scsih_remove,
 	.shutdown	= scsih_shutdown,
 	.err_handler	= &_mpt3sas_err_handler,
-#ifdef CONFIG_PM
-	.suspend	= scsih_suspend,
-	.resume		= scsih_resume,
-#endif
+	.driver.pm	= &scsih_pm_ops,
 };
 
 /**
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 09/15] scsi: lpfc: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (7 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 08/15] scsi: mpt3sas_scsih: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:03   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 10/15] scsi: pm_8001: " Vaibhav Gupta
                   ` (7 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state() and
pci_set_power_state() which is no more needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/lpfc/lpfc_init.c | 100 +++++++++++-----------------------
 1 file changed, 33 insertions(+), 67 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 6637f84a3d1b..a36309b48144 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -12452,8 +12452,7 @@ lpfc_pci_remove_one_s3(struct pci_dev *pdev)
 
 /**
  * lpfc_pci_suspend_one_s3 - PCI func to suspend SLI-3 device for power mgmnt
- * @pdev: pointer to PCI device
- * @msg: power management message
+ * @dev_d: pointer to device
  *
  * This routine is to be called from the kernel's PCI subsystem to support
  * system Power Management (PM) to device with SLI-3 interface spec. When
@@ -12471,10 +12470,10 @@ lpfc_pci_remove_one_s3(struct pci_dev *pdev)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
+static int __maybe_unused
+lpfc_pci_suspend_one_s3(struct device *dev_d)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 
 	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
@@ -12488,16 +12487,12 @@ lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
 	/* Disable interrupt from device */
 	lpfc_sli_disable_intr(phba);
 
-	/* Save device state to PCI config space */
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
-
 	return 0;
 }
 
 /**
  * lpfc_pci_resume_one_s3 - PCI func to resume SLI-3 device for power mgmnt
- * @pdev: pointer to PCI device
+ * @dev_d: pointer to device
  *
  * This routine is to be called from the kernel's PCI subsystem to support
  * system Power Management (PM) to device with SLI-3 interface spec. When PM
@@ -12514,10 +12509,10 @@ lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_resume_one_s3(struct pci_dev *pdev)
+static int __maybe_unused
+lpfc_pci_resume_one_s3(struct device *dev_d)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 	uint32_t intr_mode;
 	int error;
@@ -12525,19 +12520,6 @@ lpfc_pci_resume_one_s3(struct pci_dev *pdev)
 	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
 			"0452 PCI device Power Management resume.\n");
 
-	/* Restore device state from PCI config space */
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
-	/*
-	 * As the new kernel behavior of pci_restore_state() API call clears
-	 * device saved_state flag, need to save the restored state again.
-	 */
-	pci_save_state(pdev);
-
-	if (pdev->is_busmaster)
-		pci_set_master(pdev);
-
 	/* Startup the kernel thread for this host adapter. */
 	phba->worker_thread = kthread_run(lpfc_do_work, phba,
 					"lpfc_worker_%d", phba->brd_no);
@@ -13294,8 +13276,7 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
 
 /**
  * lpfc_pci_suspend_one_s4 - PCI func to suspend SLI-4 device for power mgmnt
- * @pdev: pointer to PCI device
- * @msg: power management message
+ * @dev_d: pointer to device
  *
  * This routine is called from the kernel's PCI subsystem to support system
  * Power Management (PM) to device with SLI-4 interface spec. When PM invokes
@@ -13313,10 +13294,10 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
+static int __maybe_unused
+lpfc_pci_suspend_one_s4(struct device *dev_d)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 
 	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
@@ -13331,16 +13312,12 @@ lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
 	lpfc_sli4_disable_intr(phba);
 	lpfc_sli4_queue_destroy(phba);
 
-	/* Save device state to PCI config space */
-	pci_save_state(pdev);
-	pci_set_power_state(pdev, PCI_D3hot);
-
 	return 0;
 }
 
 /**
  * lpfc_pci_resume_one_s4 - PCI func to resume SLI-4 device for power mgmnt
- * @pdev: pointer to PCI device
+ * @dev_d: pointer to device
  *
  * This routine is called from the kernel's PCI subsystem to support system
  * Power Management (PM) to device with SLI-4 interface spac. When PM invokes
@@ -13357,10 +13334,10 @@ lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_resume_one_s4(struct pci_dev *pdev)
+static int __maybe_unused
+lpfc_pci_resume_one_s4(struct device *dev_d)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 	uint32_t intr_mode;
 	int error;
@@ -13368,19 +13345,6 @@ lpfc_pci_resume_one_s4(struct pci_dev *pdev)
 	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
 			"0292 PCI device Power Management resume.\n");
 
-	/* Restore device state from PCI config space */
-	pci_set_power_state(pdev, PCI_D0);
-	pci_restore_state(pdev);
-
-	/*
-	 * As the new kernel behavior of pci_restore_state() API call clears
-	 * device saved_state flag, need to save the restored state again.
-	 */
-	pci_save_state(pdev);
-
-	if (pdev->is_busmaster)
-		pci_set_master(pdev);
-
 	 /* Startup the kernel thread for this host adapter. */
 	phba->worker_thread = kthread_run(lpfc_do_work, phba,
 					"lpfc_worker_%d", phba->brd_no);
@@ -13696,8 +13660,7 @@ lpfc_pci_remove_one(struct pci_dev *pdev)
 
 /**
  * lpfc_pci_suspend_one - lpfc PCI func to suspend dev for power management
- * @pdev: pointer to PCI device
- * @msg: power management message
+ * @dev: pointer to device
  *
  * This routine is to be registered to the kernel's PCI subsystem to support
  * system Power Management (PM). When PM invokes this method, it dispatches
@@ -13708,19 +13671,19 @@ lpfc_pci_remove_one(struct pci_dev *pdev)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
+static int __maybe_unused
+lpfc_pci_suspend_one(struct device *dev)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 	int rc = -ENODEV;
 
 	switch (phba->pci_dev_grp) {
 	case LPFC_PCI_DEV_LP:
-		rc = lpfc_pci_suspend_one_s3(pdev, msg);
+		rc = lpfc_pci_suspend_one_s3(dev);
 		break;
 	case LPFC_PCI_DEV_OC:
-		rc = lpfc_pci_suspend_one_s4(pdev, msg);
+		rc = lpfc_pci_suspend_one_s4(dev);
 		break;
 	default:
 		lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
@@ -13733,7 +13696,7 @@ lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
 
 /**
  * lpfc_pci_resume_one - lpfc PCI func to resume dev for power management
- * @pdev: pointer to PCI device
+ * @dev: pointer to device
  *
  * This routine is to be registered to the kernel's PCI subsystem to support
  * system Power Management (PM). When PM invokes this method, it dispatches
@@ -13744,19 +13707,19 @@ lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
  * 	0 - driver suspended the device
  * 	Error otherwise
  **/
-static int
-lpfc_pci_resume_one(struct pci_dev *pdev)
+static int __maybe_unused
+lpfc_pci_resume_one(struct device *dev)
 {
-	struct Scsi_Host *shost = pci_get_drvdata(pdev);
+	struct Scsi_Host *shost = dev_get_drvdata(dev);
 	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
 	int rc = -ENODEV;
 
 	switch (phba->pci_dev_grp) {
 	case LPFC_PCI_DEV_LP:
-		rc = lpfc_pci_resume_one_s3(pdev);
+		rc = lpfc_pci_resume_one_s3(dev);
 		break;
 	case LPFC_PCI_DEV_OC:
-		rc = lpfc_pci_resume_one_s4(pdev);
+		rc = lpfc_pci_resume_one_s4(dev);
 		break;
 	default:
 		lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
@@ -13936,14 +13899,17 @@ static const struct pci_error_handlers lpfc_err_handler = {
 	.resume = lpfc_io_resume,
 };
 
+static SIMPLE_DEV_PM_OPS(lpfc_pci_pm_ops_one,
+			 lpfc_pci_suspend_one,
+			 lpfc_pci_resume_one);
+
 static struct pci_driver lpfc_driver = {
 	.name		= LPFC_DRIVER_NAME,
 	.id_table	= lpfc_id_table,
 	.probe		= lpfc_pci_probe_one,
 	.remove		= lpfc_pci_remove_one,
 	.shutdown	= lpfc_pci_remove_one,
-	.suspend        = lpfc_pci_suspend_one,
-	.resume		= lpfc_pci_resume_one,
+	.driver.pm	= &lpfc_pci_pm_ops_one,
 	.err_handler    = &lpfc_err_handler,
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 10/15] scsi: pm_8001: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (8 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 09/15] scsi: lpfc: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-07-23  7:02   ` Jinpu Wang
  2020-09-08 17:04   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 11/15] scsi: hpsa: " Vaibhav Gupta
                   ` (6 subsequent siblings)
  16 siblings, 2 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

With legacy PM, drivers themselves were responsible for managing the
device's power states and takes care of register states.

After upgrading to the generic structure, PCI core will take care of
required tasks and drivers should do only device-specific operations.

The driver was calling pci_save/restore_state(), pci_choose_state(),
pci_enable/disable_device() and pci_set_power_state() which is no more
needed.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/pm8001/pm8001_init.c | 46 ++++++++++++-------------------
 1 file changed, 17 insertions(+), 29 deletions(-)

diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
index 9e99262a2b9d..d7d664b87720 100644
--- a/drivers/scsi/pm8001/pm8001_init.c
+++ b/drivers/scsi/pm8001/pm8001_init.c
@@ -1178,23 +1178,21 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
 
 /**
  * pm8001_pci_suspend - power management suspend main entry point
- * @pdev: PCI device struct
- * @state: PM state change to (usually PCI_D3)
+ * @dev: Device struct
  *
  * Returns 0 success, anything else error.
  */
-static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused pm8001_pci_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
-	struct pm8001_hba_info *pm8001_ha;
+	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
 	int  i, j;
-	u32 device_state;
-	pm8001_ha = sha->lldd_ha;
 	sas_suspend_ha(sha);
 	flush_workqueue(pm8001_wq);
 	scsi_block_requests(pm8001_ha->shost);
 	if (!pdev->pm_cap) {
-		dev_err(&pdev->dev, " PCI PM not supported\n");
+		dev_err(dev, " PCI PM not supported\n");
 		return -ENODEV;
 	}
 	PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
@@ -1217,24 +1215,21 @@ static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state)
 		for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
 			tasklet_kill(&pm8001_ha->tasklet[j]);
 #endif
-	device_state = pci_choose_state(pdev, state);
 	pm8001_printk("pdev=0x%p, slot=%s, entering "
-		      "operating state [D%d]\n", pdev,
-		      pm8001_ha->name, device_state);
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, device_state);
+		      "suspended state\n", pdev,
+		      pm8001_ha->name);
 	return 0;
 }
 
 /**
  * pm8001_pci_resume - power management resume main entry point
- * @pdev: PCI device struct
+ * @dev: Device struct
  *
  * Returns 0 success, anything else error.
  */
-static int pm8001_pci_resume(struct pci_dev *pdev)
+static int __maybe_unused pm8001_pci_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
 	struct pm8001_hba_info *pm8001_ha;
 	int rc;
@@ -1247,17 +1242,8 @@ static int pm8001_pci_resume(struct pci_dev *pdev)
 	pm8001_printk("pdev=0x%p, slot=%s, resuming from previous "
 		"operating state [D%d]\n", pdev, pm8001_ha->name, device_state);
 
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-	rc = pci_enable_device(pdev);
-	if (rc) {
-		pm8001_printk("slot=%s Enable device failed during resume\n",
-			      pm8001_ha->name);
-		goto err_out_enable;
-	}
+	device_wakeup_disable(dev);
 
-	pci_set_master(pdev);
 	rc = pci_go_44(pdev);
 	if (rc)
 		goto err_out_disable;
@@ -1318,8 +1304,7 @@ static int pm8001_pci_resume(struct pci_dev *pdev)
 
 err_out_disable:
 	scsi_remove_host(pm8001_ha->shost);
-	pci_disable_device(pdev);
-err_out_enable:
+
 	return rc;
 }
 
@@ -1402,13 +1387,16 @@ static struct pci_device_id pm8001_pci_table[] = {
 	{} /* terminate list */
 };
 
+static SIMPLE_DEV_PM_OPS(pm8001_pci_pm_ops,
+			 pm8001_pci_suspend,
+			 pm8001_pci_resume);
+
 static struct pci_driver pm8001_pci_driver = {
 	.name		= DRV_NAME,
 	.id_table	= pm8001_pci_table,
 	.probe		= pm8001_pci_probe,
 	.remove		= pm8001_pci_remove,
-	.suspend	= pm8001_pci_suspend,
-	.resume		= pm8001_pci_resume,
+	.driver.pm	= &pm8001_pci_pm_ops,
 };
 
 /**
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 11/15] scsi: hpsa: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (9 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 10/15] scsi: pm_8001: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-07-20 22:23   ` Don.Brace
  2020-09-08 17:05   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 12/15] scsi: 3w-9xxx: " Vaibhav Gupta
                   ` (5 subsequent siblings)
  16 siblings, 2 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

Change function parameter in both .suspend() and .resume() to
"struct device*" type. The function body remains unchanged as it was empty.
Also, bind callbacks with "static const struct dev_pm_ops" variable.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/hpsa.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 81d0414e2117..70bdd6fe91ee 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -9070,25 +9070,27 @@ static void hpsa_remove_one(struct pci_dev *pdev)
 	hpda_free_ctlr_info(h);				/* init_one 1 */
 }
 
-static int hpsa_suspend(__attribute__((unused)) struct pci_dev *pdev,
-	__attribute__((unused)) pm_message_t state)
+static int __maybe_unused hpsa_suspend(
+	__attribute__((unused)) struct device *dev)
 {
 	return -ENOSYS;
 }
 
-static int hpsa_resume(__attribute__((unused)) struct pci_dev *pdev)
+static int __maybe_unused hpsa_resume
+	(__attribute__((unused)) struct device *dev)
 {
 	return -ENOSYS;
 }
 
+static SIMPLE_DEV_PM_OPS(hpsa_pm_ops, hpsa_suspend, hpsa_resume);
+
 static struct pci_driver hpsa_pci_driver = {
 	.name = HPSA,
 	.probe = hpsa_init_one,
 	.remove = hpsa_remove_one,
 	.id_table = hpsa_pci_device_id,	/* id_table */
 	.shutdown = hpsa_shutdown,
-	.suspend = hpsa_suspend,
-	.resume = hpsa_resume,
+	.driver.pm = &hpsa_pm_ops,
 };
 
 /* Fill in bucket_map[], given nsgs (the max number of
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 12/15] scsi: 3w-9xxx: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (10 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 11/15] scsi: hpsa: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:05   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 13/15] scsi: 3w-sas: " Vaibhav Gupta
                   ` (4 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(),
pci_set_power_state() and pci_set_master() to do required operations. In
generic mode, they are no longer needed.

Change function parameter in both .suspend() and .resume() to
"struct device*" type. Use to_pci_dev() and dev_get_drvdata() to get
"struct pci_dev*" variable and drv data.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/3w-9xxx.c | 30 ++++++++----------------------
 1 file changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index 3337b1e80412..c15dcfda3957 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -2191,10 +2191,10 @@ static void twa_remove(struct pci_dev *pdev)
 	twa_device_extension_count--;
 } /* End twa_remove() */
 
-#ifdef CONFIG_PM
 /* This function is called on PCI suspend */
-static int twa_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused twa_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
 
@@ -2214,32 +2214,21 @@ static int twa_suspend(struct pci_dev *pdev, pm_message_t state)
 	}
 	TW_CLEAR_ALL_INTERRUPTS(tw_dev);
 
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
 	return 0;
 } /* End twa_suspend() */
 
 /* This function is called on PCI resume */
-static int twa_resume(struct pci_dev *pdev)
+static int __maybe_unused twa_resume(struct device *dev)
 {
 	int retval = 0;
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
 
 	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
 
-	retval = pci_enable_device(pdev);
-	if (retval) {
-		TW_PRINTK(tw_dev->host, TW_DRIVER, 0x39, "Enable device failed during resume");
-		return retval;
-	}
+	device_wakeup_disable(dev);
 
-	pci_set_master(pdev);
 	pci_try_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
@@ -2277,11 +2266,9 @@ static int twa_resume(struct pci_dev *pdev)
 
 out_disable_device:
 	scsi_remove_host(host);
-	pci_disable_device(pdev);
 
 	return retval;
 } /* End twa_resume() */
-#endif
 
 /* PCI Devices supported by this driver */
 static struct pci_device_id twa_pci_tbl[] = {
@@ -2297,16 +2284,15 @@ static struct pci_device_id twa_pci_tbl[] = {
 };
 MODULE_DEVICE_TABLE(pci, twa_pci_tbl);
 
+static SIMPLE_DEV_PM_OPS(twa_pm_ops, twa_suspend, twa_resume);
+
 /* pci_driver initializer */
 static struct pci_driver twa_driver = {
 	.name		= "3w-9xxx",
 	.id_table	= twa_pci_tbl,
 	.probe		= twa_probe,
 	.remove		= twa_remove,
-#ifdef CONFIG_PM
-	.suspend	= twa_suspend,
-	.resume		= twa_resume,
-#endif
+	.driver.pm	= &twa_pm_ops,
 	.shutdown	= twa_shutdown
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 13/15] scsi: 3w-sas: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (11 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 12/15] scsi: 3w-9xxx: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:06   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 14/15] scsi: mvumi: " Vaibhav Gupta
                   ` (3 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(),
pci_set_power_state() and pci_set_master() to do required operations. In
generic mode, they are no longer needed.

Change function parameter in both .suspend() and .resume() to
"struct device*" type. Use to_pci_dev() and dev_get_drvdata() to get
"struct pci_dev*" variable and drv data.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/3w-sas.c | 31 ++++++++-----------------------
 1 file changed, 8 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
index dda6fa857709..efaba30b0ca8 100644
--- a/drivers/scsi/3w-sas.c
+++ b/drivers/scsi/3w-sas.c
@@ -1756,11 +1756,10 @@ static void twl_remove(struct pci_dev *pdev)
 	twl_device_extension_count--;
 } /* End twl_remove() */
 
-#ifdef CONFIG_PM
 /* This function is called on PCI suspend */
-static int twl_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused twl_suspend(struct device *dev)
 {
-	struct Scsi_Host *host = pci_get_drvdata(pdev);
+	struct Scsi_Host *host = dev_get_drvdata(dev);
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
 
 	printk(KERN_WARNING "3w-sas: Suspending host %d.\n", tw_dev->host->host_no);
@@ -1779,32 +1778,21 @@ static int twl_suspend(struct pci_dev *pdev, pm_message_t state)
 	/* Clear doorbell interrupt */
 	TWL_CLEAR_DB_INTERRUPT(tw_dev);
 
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
 	return 0;
 } /* End twl_suspend() */
 
 /* This function is called on PCI resume */
-static int twl_resume(struct pci_dev *pdev)
+static int __maybe_unused twl_resume(struct device *dev)
 {
 	int retval = 0;
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct Scsi_Host *host = pci_get_drvdata(pdev);
 	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
 
 	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
 
-	retval = pci_enable_device(pdev);
-	if (retval) {
-		TW_PRINTK(tw_dev->host, TW_DRIVER, 0x24, "Enable device failed during resume");
-		return retval;
-	}
+	device_wakeup_disable(dev);
 
-	pci_set_master(pdev);
 	pci_try_set_mwi(pdev);
 
 	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
@@ -1842,11 +1830,9 @@ static int twl_resume(struct pci_dev *pdev)
 
 out_disable_device:
 	scsi_remove_host(host);
-	pci_disable_device(pdev);
 
 	return retval;
 } /* End twl_resume() */
-#endif
 
 /* PCI Devices supported by this driver */
 static struct pci_device_id twl_pci_tbl[] = {
@@ -1855,16 +1841,15 @@ static struct pci_device_id twl_pci_tbl[] = {
 };
 MODULE_DEVICE_TABLE(pci, twl_pci_tbl);
 
+static SIMPLE_DEV_PM_OPS(twl_pm_ops, twl_suspend, twl_resume);
+
 /* pci_driver initializer */
 static struct pci_driver twl_driver = {
 	.name		= "3w-sas",
 	.id_table	= twl_pci_tbl,
 	.probe		= twl_probe,
 	.remove		= twl_remove,
-#ifdef CONFIG_PM
-	.suspend	= twl_suspend,
-	.resume		= twl_resume,
-#endif
+	.driver.pm	= &twl_pm_ops,
 	.shutdown	= twl_shutdown
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 14/15] scsi: mvumi: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (12 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 13/15] scsi: 3w-sas: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:07   ` Vaibhav Gupta
  2020-07-20 13:34 ` [PATCH v2 15/15] scsi: pmcraid: " Vaibhav Gupta
                   ` (2 subsequent siblings)
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(),
pci_set_power_state() and pci_set_master() to do required operations. In
generic mode, they are no longer needed.

Change function parameter in both .suspend() and .resume() to
"struct device*" type. Use dev_get_drvdata() to get drv data.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/mvumi.c | 49 ++++++++++----------------------------------
 1 file changed, 11 insertions(+), 38 deletions(-)

diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
index 8906aceda4c4..7a6ef8264e47 100644
--- a/drivers/scsi/mvumi.c
+++ b/drivers/scsi/mvumi.c
@@ -2558,7 +2558,7 @@ static void mvumi_detach_one(struct pci_dev *pdev)
 
 /**
  * mvumi_shutdown -	Shutdown entry point
- * @device:		Generic device structure
+ * @pdev:		PCI device structure
  */
 static void mvumi_shutdown(struct pci_dev *pdev)
 {
@@ -2567,47 +2567,28 @@ static void mvumi_shutdown(struct pci_dev *pdev)
 	mvumi_flush_cache(mhba);
 }
 
-static int __maybe_unused mvumi_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused mvumi_suspend(struct device *dev)
 {
-	struct mvumi_hba *mhba = NULL;
+	struct pci_dev *pdev = to_pci_dev(dev);
 
-	mhba = pci_get_drvdata(pdev);
+	struct mvumi_hba *mhba = pci_get_drvdata(pdev);
 	mvumi_flush_cache(mhba);
 
-	pci_set_drvdata(pdev, mhba);
 	mhba->instancet->disable_intr(mhba);
-	free_irq(mhba->pdev->irq, mhba);
 	mvumi_unmap_pci_addr(pdev, mhba->base_addr);
-	pci_release_regions(pdev);
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
 
 	return 0;
 }
 
-static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
+static int __maybe_unused mvumi_resume(struct device *dev)
 {
 	int ret;
-	struct mvumi_hba *mhba = NULL;
-
-	mhba = pci_get_drvdata(pdev);
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
+	struct pci_dev *pdev = to_pci_dev(dev);
+	struct mvumi_hba *mhba = pci_get_drvdata(pdev);
 
-	ret = pci_enable_device(pdev);
-	if (ret) {
-		dev_err(&pdev->dev, "enable device failed\n");
-		return ret;
-	}
+	device_wakeup_disable(dev);
 
-	ret = mvumi_pci_set_master(pdev);
 	ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
-	if (ret)
-		goto fail;
-	ret = pci_request_regions(mhba->pdev, MV_DRIVER_NAME);
 	if (ret)
 		goto fail;
 	ret = mvumi_map_pci_addr(mhba->pdev, mhba->base_addr);
@@ -2627,12 +2608,6 @@ static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
 		goto unmap_pci_addr;
 	}
 
-	ret = request_irq(mhba->pdev->irq, mvumi_isr_handler, IRQF_SHARED,
-				"mvumi", mhba);
-	if (ret) {
-		dev_err(&pdev->dev, "failed to register IRQ\n");
-		goto unmap_pci_addr;
-	}
 	mhba->instancet->enable_intr(mhba);
 
 	return 0;
@@ -2642,11 +2617,12 @@ static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
 release_regions:
 	pci_release_regions(pdev);
 fail:
-	pci_disable_device(pdev);
 
 	return ret;
 }
 
+static SIMPLE_DEV_PM_OPS(mvumi_pm_ops, mvumi_suspend, mvumi_resume);
+
 static struct pci_driver mvumi_pci_driver = {
 
 	.name = MV_DRIVER_NAME,
@@ -2654,10 +2630,7 @@ static struct pci_driver mvumi_pci_driver = {
 	.probe = mvumi_probe_one,
 	.remove = mvumi_detach_one,
 	.shutdown = mvumi_shutdown,
-#ifdef CONFIG_PM
-	.suspend = mvumi_suspend,
-	.resume = mvumi_resume,
-#endif
+	.driver.pm = &mvumi_pm_ops,
 };
 
 module_pci_driver(mvumi_pci_driver);
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* [PATCH v2 15/15] scsi: pmcraid: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (13 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 14/15] scsi: mvumi: " Vaibhav Gupta
@ 2020-07-20 13:34 ` Vaibhav Gupta
  2020-09-08 17:08   ` Vaibhav Gupta
  2020-08-17  8:16 ` [PATCH v2 00/15] scsi: " Vaibhav Gupta
  2020-09-08 16:54 ` Vaibhav Gupta
  16 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-07-20 13:34 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Vaibhav Gupta, Shuah Khan, linux-kernel, linux-kernel-mentees,
	linux-scsi, esc.storagedev, megaraidlinux.pdl,
	MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states
themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of
above mentioned, device-independent, jobs.

This driver makes use of PCI helper functions like
pci_save/restore_state(), pci_enable/disable_device(),
pci_set_power_state() and to do required operations. In generic mode, they
are no longer needed.

Change function parameter in both .suspend() and .resume() to
"struct device*" type. Use to_pci_dev() to get "struct pci_dev*" variable.

In function pmcraid_resume(), earlier, the variable "rc" was set by
pci_enable_device() which is now removed. Since PCI core does the required
job, initialize "rc" with 0 value when declaring it.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/pmcraid.c | 44 +++++++++++-------------------------------
 1 file changed, 11 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
index aa9ae2ae8579..b6b70ac2e2ee 100644
--- a/drivers/scsi/pmcraid.c
+++ b/drivers/scsi/pmcraid.c
@@ -5237,54 +5237,39 @@ static void pmcraid_remove(struct pci_dev *pdev)
 	return;
 }
 
-#ifdef CONFIG_PM
 /**
  * pmcraid_suspend - driver suspend entry point for power management
- * @pdev:   PCI device structure
- * @state:  PCI power state to suspend routine
+ * @dev:   Device structure
  *
  * Return Value - 0 always
  */
-static int pmcraid_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused pmcraid_suspend(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pmcraid_instance *pinstance = pci_get_drvdata(pdev);
 
 	pmcraid_shutdown(pdev);
 	pmcraid_disable_interrupts(pinstance, ~0);
 	pmcraid_kill_tasklets(pinstance);
-	pci_set_drvdata(pinstance->pdev, pinstance);
 	pmcraid_unregister_interrupt_handler(pinstance);
-	pci_save_state(pdev);
-	pci_disable_device(pdev);
-	pci_set_power_state(pdev, pci_choose_state(pdev, state));
 
 	return 0;
 }
 
 /**
  * pmcraid_resume - driver resume entry point PCI power management
- * @pdev: PCI device structure
+ * @dev: Device structure
  *
  * Return Value - 0 in case of success. Error code in case of any failure
  */
-static int pmcraid_resume(struct pci_dev *pdev)
+static int __maybe_unused pmcraid_resume(struct device *dev)
 {
+	struct pci_dev *pdev = to_pci_dev(dev);
 	struct pmcraid_instance *pinstance = pci_get_drvdata(pdev);
 	struct Scsi_Host *host = pinstance->host;
-	int rc;
-
-	pci_set_power_state(pdev, PCI_D0);
-	pci_enable_wake(pdev, PCI_D0, 0);
-	pci_restore_state(pdev);
-
-	rc = pci_enable_device(pdev);
-
-	if (rc) {
-		dev_err(&pdev->dev, "resume: Enable device failed\n");
-		return rc;
-	}
+	int rc = 0;
 
-	pci_set_master(pdev);
+	device_wakeup_disable(dev);
 
 	if (sizeof(dma_addr_t) == 4 ||
 	    dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)))
@@ -5337,18 +5322,10 @@ static int pmcraid_resume(struct pci_dev *pdev)
 	scsi_host_put(host);
 
 disable_device:
-	pci_disable_device(pdev);
 
 	return rc;
 }
 
-#else
-
-#define pmcraid_suspend NULL
-#define pmcraid_resume  NULL
-
-#endif /* CONFIG_PM */
-
 /**
  * pmcraid_complete_ioa_reset - Called by either timer or tasklet during
  *				completion of the ioa reset
@@ -5836,6 +5813,8 @@ static int pmcraid_probe(struct pci_dev *pdev,
 	return -ENODEV;
 }
 
+static SIMPLE_DEV_PM_OPS(pmcraid_pm_ops, pmcraid_suspend, pmcraid_resume);
+
 /*
  * PCI driver structure of pmcraid driver
  */
@@ -5844,8 +5823,7 @@ static struct pci_driver pmcraid_driver = {
 	.id_table = pmcraid_pci_table,
 	.probe = pmcraid_probe,
 	.remove = pmcraid_remove,
-	.suspend = pmcraid_suspend,
-	.resume = pmcraid_resume,
+	.driver.pm = &pmcraid_pm_ops,
 	.shutdown = pmcraid_shutdown
 };
 
-- 
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 11/15] scsi: hpsa: use generic power management
  2020-07-20 13:34 ` [PATCH v2 11/15] scsi: hpsa: " Vaibhav Gupta
@ 2020-07-20 22:23   ` Don.Brace
  2020-09-08 17:05   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Don.Brace @ 2020-07-20 22:23 UTC (permalink / raw)
  To: vaibhavgupta40, helgaas, bhelgaas, bjorn, vaibhav.varodek,
	aradford, jejb, martin.petersen, aacraid, hare, linuxdrivers,
	john.garry, don.brace, james.smart, dick.kennedy, kashyap.desai,
	sumit.saxena, shivasharan.srikanteshwara, sathya.prakash,
	sreekanth.reddy, suganath-prabu.subramani, jinpu.wang
  Cc: skhan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

Drivers using legacy PM have to manage PCI states and device's PM states themselves. They also need to take care of configuration registers.

With improved and powerful support of generic PM, PCI Core takes care of above mentioned, device-independent, jobs.

Change function parameter in both .suspend() and .resume() to "struct device*" type. The function body remains unchanged as it was empty.
Also, bind callbacks with "static const struct dev_pm_ops" variable.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>

Acked-by: Don Brace <don.brace@microsemi.com>
Compile-tested: no compile/sparse issues found.
Tested-by: Don Brace <don.brace@microsemi.com>
   (for hpsa driver)


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 07/15] scsi: hisi_sas_v3_hw: use generic power management
  2020-07-20 13:34 ` [PATCH v2 07/15] scsi: hisi_sas_v3_hw: " Vaibhav Gupta
@ 2020-07-21  1:36   ` chenxiang (M)
  2020-09-08 17:02   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: chenxiang (M) @ 2020-07-21  1:36 UTC (permalink / raw)
  To: Vaibhav Gupta, Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas,
	Vaibhav Gupta, Adam Radford, James E.J. Bottomley,
	Martin K. Petersen, Adaptec OEM Raid Solutions, Hannes Reinecke,
	Bradley Grove, John Garry, Don Brace, James Smart, Dick Kennedy,
	Kashyap Desai, Sumit Saxena, Shivasharan S, Sathya Prakash,
	Sreekanth Reddy, Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl



在 2020/7/20 21:34, Vaibhav Gupta 写道:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
>
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
>
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
>
> Compile-tested only.
>
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>

Reviewed-by: Xiang Chen <chenxiang66@hisilicon.com>

> ---
>   drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 32 +++++++++-----------------
>   1 file changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 55e2321a65bc..824bfbe1abbb 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,16 +3426,8 @@ 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_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -	rc = pci_enable_device(pdev);
> -	if (rc) {
> -		dev_err(dev, "enable device failed during resume (%d)\n", rc);
> -		return rc;
> -	}
> +	device_wakeup_disable(dev_d);
>   
> -	pci_set_master(pdev);
>   	scsi_unblock_requests(shost);
>   	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
>   
> @@ -3447,7 +3435,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);
> @@ -3468,13 +3455,16 @@ static const struct pci_error_handlers hisi_sas_err_handler = {
>   	.reset_done	= hisi_sas_reset_done_v3_hw,
>   };
>   
> +static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,
> +			 hisi_sas_v3_suspend,
> +			 hisi_sas_v3_resume);
> +
>   static struct pci_driver sas_v3_pci_driver = {
>   	.name		= DRV_NAME,
>   	.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,
>   };
>   



^ permalink raw reply	[flat|nested] 41+ messages in thread

* RE: [PATCH v2 02/15] scsi: aacraid: use generic power management
  2020-07-20 13:34 ` [PATCH v2 02/15] scsi: aacraid: " Vaibhav Gupta
@ 2020-07-23  6:58   ` Balsundar.P
  2020-09-08 16:58   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Balsundar.P @ 2020-07-23  6:58 UTC (permalink / raw)
  To: vaibhavgupta40, helgaas, bhelgaas, bjorn, vaibhav.varodek,
	aradford, jejb, martin.petersen, aacraid, hare, linuxdrivers,
	john.garry, don.brace, james.smart, dick.kennedy, kashyap.desai,
	sumit.saxena, shivasharan.srikanteshwara, sathya.prakash,
	sreekanth.reddy, suganath-prabu.subramani, jinpu.wang
  Cc: skhan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

Acked-by: Balsundar P < balsundar.p@microchip.com>

-----Original Message-----
From: Vaibhav Gupta <vaibhavgupta40@gmail.com> 
Sent: Monday, July 20, 2020 19:04
To: Bjorn Helgaas <helgaas@kernel.org>; Bjorn Helgaas <bhelgaas@google.com>; Bjorn Helgaas <bjorn@helgaas.com>; Vaibhav Gupta <vaibhav.varodek@gmail.com>; Adam Radford <aradford@gmail.com>; James E.J. Bottomley <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>; Adaptec OEM Raid Solutions <aacraid@microsemi.com>; Hannes Reinecke <hare@suse.com>; Bradley Grove <linuxdrivers@attotech.com>; John Garry <john.garry@huawei.com>; Don Brace <don.brace@microsemi.com>; James Smart <james.smart@broadcom.com>; Dick Kennedy <dick.kennedy@broadcom.com>; Kashyap Desai <kashyap.desai@broadcom.com>; Sumit Saxena <sumit.saxena@broadcom.com>; Shivasharan S <shivasharan.srikanteshwara@broadcom.com>; Sathya Prakash <sathya.prakash@broadcom.com>; Sreekanth Reddy <sreekanth.reddy@broadcom.com>; Suganath Prabu Subramani <suganath-prabu.subramani@broadcom.com>; Jack Wang <jinpu.wang@cloud.ionos.com>
Cc: Vaibhav Gupta <vaibhavgupta40@gmail.com>; Shuah Khan <skhan@linuxfoundation.org>; linux-kernel@vger.kernel.org; linux-kernel-mentees@lists.linuxfoundation.org; linux-scsi@vger.kernel.org; esc.storagedev@microsemi.com; megaraidlinux.pdl@broadcom.com; MPT-FusionLinux.pdl@broadcom.com
Subject: [PATCH v2 02/15] scsi: aacraid: use generic power management

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

With legacy PM hooks, it was the responsibility of a driver to manage PCI states and also the device's power state. The generic approach is to let the PCI core handle the work.

PCI core passes "struct device*" as an argument to the .suspend() and
.resume() callbacks.

Driver was using PCI helper functions like pci_save/restore_state(), pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
They should not be invoked by the driver.

Compile-tested only.

Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
---
 drivers/scsi/aacraid/linit.c | 34 ++++++++--------------------------
 1 file changed, 8 insertions(+), 26 deletions(-)

diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c index a308e86a97f1..1e44868ee953 100644
--- a/drivers/scsi/aacraid/linit.c
+++ b/drivers/scsi/aacraid/linit.c
@@ -1910,11 +1910,9 @@ static int aac_acquire_resources(struct aac_dev *dev)

 }

-#if (defined(CONFIG_PM))
-static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused aac_suspend(struct device *dev)
 {
-
-       struct Scsi_Host *shost = pci_get_drvdata(pdev);
+       struct Scsi_Host *shost = dev_get_drvdata(dev);
        struct aac_dev *aac = (struct aac_dev *)shost->hostdata;

        scsi_host_block(shost);
@@ -1923,29 +1921,16 @@ static int aac_suspend(struct pci_dev *pdev, pm_message_t state)

        aac_release_resources(aac);

-       pci_set_drvdata(pdev, shost);
-       pci_save_state(pdev);
-       pci_disable_device(pdev);
-       pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
        return 0;
 }

-static int aac_resume(struct pci_dev *pdev)
+static int __maybe_unused aac_resume(struct device *dev)
 {
-       struct Scsi_Host *shost = pci_get_drvdata(pdev);
+       struct Scsi_Host *shost = dev_get_drvdata(dev);
        struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
-       int r;

-       pci_set_power_state(pdev, PCI_D0);
-       pci_enable_wake(pdev, PCI_D0, 0);
-       pci_restore_state(pdev);
-       r = pci_enable_device(pdev);
+       device_wakeup_disable(dev);

-       if (r)
-               goto fail_device;
-
-       pci_set_master(pdev);
        if (aac_acquire_resources(aac))
                goto fail_device;
        /*
@@ -1960,10 +1945,8 @@ static int aac_resume(struct pci_dev *pdev)
 fail_device:
        printk(KERN_INFO "%s%d: resume failed.\n", aac->name, aac->id);
        scsi_host_put(shost);
-       pci_disable_device(pdev);
        return -ENODEV;
 }
-#endif

 static void aac_shutdown(struct pci_dev *dev)  { @@ -2108,15 +2091,14 @@ static struct pci_error_handlers aac_pci_err_handler = {
        .resume                 = aac_pci_resume,
 };

+static SIMPLE_DEV_PM_OPS(aac_pm_ops, aac_suspend, aac_resume);
+
 static struct pci_driver aac_pci_driver = {
        .name           = AAC_DRIVERNAME,
        .id_table       = aac_pci_tbl,
        .probe          = aac_probe_one,
        .remove         = aac_remove_one,
-#if (defined(CONFIG_PM))
-       .suspend        = aac_suspend,
-       .resume         = aac_resume,
-#endif
+       .driver.pm      = &aac_pm_ops,
        .shutdown       = aac_shutdown,
        .err_handler    = &aac_pci_err_handler,
 };
--
2.27.0


^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 10/15] scsi: pm_8001: use generic power management
  2020-07-20 13:34 ` [PATCH v2 10/15] scsi: pm_8001: " Vaibhav Gupta
@ 2020-07-23  7:02   ` Jinpu Wang
  2020-09-08 17:04   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Jinpu Wang @ 2020-07-23  7:02 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Shuah Khan, linux-kernel,
	linux-kernel-mentees, Linux SCSI Mailinglist, esc.storagedev,
	megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 3:38 PM Vaibhav Gupta <vaibhavgupta40@gmail.com> wrote:
>
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
>
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
>
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
>
> Compile-tested only.
>
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
nit: subject should be pm8001
Thanks!
Acked-by: Jack Wang <jinpu.wang@cloud.ionos.com>

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 00/15] scsi: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (14 preceding siblings ...)
  2020-07-20 13:34 ` [PATCH v2 15/15] scsi: pmcraid: " Vaibhav Gupta
@ 2020-08-17  8:16 ` Vaibhav Gupta
  2020-09-08 16:54 ` Vaibhav Gupta
  16 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-08-17  8:16 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:13PM +0530, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
> 
> The purpose of this patch series is to upgrade power management in scsi
> drivers. This has been done by upgrading .suspend() and .resume() callbacks.
> 
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
> 
> In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
> helper functions like pci_request/release_regions(), pci_set_power_state(),
> pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
> their job.
> 
> The conversion requires the removal of those function calls, change the
> callbacks' definition accordingly and make use of dev_pm_ops structure.
> 
> v2: kbuild error in v1.
> 
> All patches are compile-tested only.
> 
> Test tools:
>     - Compiler: gcc (GCC) 10.1.0
>     - allmodconfig build: make -j$(nproc) W=1 all
> 
> Vaibhav Gupta (15):
>   scsi: megaraid_sas: use generic power management
>   scsi: aacraid: use generic power management
>   scsi: aic7xxx: use generic power management
>   scsi: aic79xx: use generic power management
>   scsi: arcmsr: use generic power management
>   scsi: esas2r: use generic power management
>   scsi: hisi_sas_v3_hw: use generic power management
>   scsi: mpt3sas_scsih: use generic power management
>   scsi: lpfc: use generic power management
>   scsi: pm_8001: use generic power management
>   scsi: hpsa: use generic power management
>   scsi: 3w-9xxx: use generic power management
>   scsi: 3w-sas: use generic power management
>   scsi: mvumi: use generic power management
>   scsi: pmcraid: use generic power management
> 
>  drivers/scsi/3w-9xxx.c                    |  30 ++-----
>  drivers/scsi/3w-sas.c                     |  31 ++-----
>  drivers/scsi/aacraid/linit.c              |  34 ++------
>  drivers/scsi/aic7xxx/aic79xx.h            |  12 +--
>  drivers/scsi/aic7xxx/aic79xx_core.c       |   8 +-
>  drivers/scsi/aic7xxx/aic79xx_osm_pci.c    |  43 +++-------
>  drivers/scsi/aic7xxx/aic79xx_pci.c        |   6 +-
>  drivers/scsi/aic7xxx/aic7xxx.h            |  10 +--
>  drivers/scsi/aic7xxx/aic7xxx_core.c       |   6 +-
>  drivers/scsi/aic7xxx/aic7xxx_osm_pci.c    |  46 +++-------
>  drivers/scsi/aic7xxx/aic7xxx_pci.c        |   4 +-
>  drivers/scsi/arcmsr/arcmsr_hba.c          |  35 +++-----
>  drivers/scsi/esas2r/esas2r.h              |   5 +-
>  drivers/scsi/esas2r/esas2r_init.c         |  48 +++--------
>  drivers/scsi/esas2r/esas2r_main.c         |   3 +-
>  drivers/scsi/hisi_sas/hisi_sas_v3_hw.c    |  32 +++----
>  drivers/scsi/hpsa.c                       |  12 +--
>  drivers/scsi/lpfc/lpfc_init.c             | 100 +++++++---------------
>  drivers/scsi/megaraid/megaraid_sas_base.c |  61 ++++---------
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  36 +++-----
>  drivers/scsi/mvumi.c                      |  49 +++--------
>  drivers/scsi/pm8001/pm8001_init.c         |  46 ++++------
>  drivers/scsi/pmcraid.c                    |  44 +++-------
>  23 files changed, 212 insertions(+), 489 deletions(-)
> 
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 00/15] scsi: use generic power management
  2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
                   ` (15 preceding siblings ...)
  2020-08-17  8:16 ` [PATCH v2 00/15] scsi: " Vaibhav Gupta
@ 2020-09-08 16:54 ` Vaibhav Gupta
  16 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 16:54 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:13PM +0530, Vaibhav Gupta wrote:
> Linux Kernel Mentee: Remove Legacy Power Management.
> 
> The purpose of this patch series is to upgrade power management in scsi
> drivers. This has been done by upgrading .suspend() and .resume() callbacks.
> 
> The upgrade makes sure that the involvement of PCI Core does not change the
> order of operations executed in a driver. Thus, does not change its behavior.
> 
> In general, drivers with legacy PM, .suspend() and .resume() make use of PCI
> helper functions like pci_request/release_regions(), pci_set_power_state(),
> pci_save/restore_state(), pci_enable/disable_device(), etc. to complete
> their job.
> 
> The conversion requires the removal of those function calls, change the
> callbacks' definition accordingly and make use of dev_pm_ops structure.
> 
> v2: kbuild error in v1.
> 
> All patches are compile-tested only.
> 
> Test tools:
>     - Compiler: gcc (GCC) 10.1.0
>     - allmodconfig build: make -j$(nproc) W=1 all
> 
> Vaibhav Gupta (15):
>   scsi: megaraid_sas: use generic power management
>   scsi: aacraid: use generic power management
>   scsi: aic7xxx: use generic power management
>   scsi: aic79xx: use generic power management
>   scsi: arcmsr: use generic power management
>   scsi: esas2r: use generic power management
>   scsi: hisi_sas_v3_hw: use generic power management
>   scsi: mpt3sas_scsih: use generic power management
>   scsi: lpfc: use generic power management
>   scsi: pm_8001: use generic power management
>   scsi: hpsa: use generic power management
>   scsi: 3w-9xxx: use generic power management
>   scsi: 3w-sas: use generic power management
>   scsi: mvumi: use generic power management
>   scsi: pmcraid: use generic power management
> 
>  drivers/scsi/3w-9xxx.c                    |  30 ++-----
>  drivers/scsi/3w-sas.c                     |  31 ++-----
>  drivers/scsi/aacraid/linit.c              |  34 ++------
>  drivers/scsi/aic7xxx/aic79xx.h            |  12 +--
>  drivers/scsi/aic7xxx/aic79xx_core.c       |   8 +-
>  drivers/scsi/aic7xxx/aic79xx_osm_pci.c    |  43 +++-------
>  drivers/scsi/aic7xxx/aic79xx_pci.c        |   6 +-
>  drivers/scsi/aic7xxx/aic7xxx.h            |  10 +--
>  drivers/scsi/aic7xxx/aic7xxx_core.c       |   6 +-
>  drivers/scsi/aic7xxx/aic7xxx_osm_pci.c    |  46 +++-------
>  drivers/scsi/aic7xxx/aic7xxx_pci.c        |   4 +-
>  drivers/scsi/arcmsr/arcmsr_hba.c          |  35 +++-----
>  drivers/scsi/esas2r/esas2r.h              |   5 +-
>  drivers/scsi/esas2r/esas2r_init.c         |  48 +++--------
>  drivers/scsi/esas2r/esas2r_main.c         |   3 +-
>  drivers/scsi/hisi_sas/hisi_sas_v3_hw.c    |  32 +++----
>  drivers/scsi/hpsa.c                       |  12 +--
>  drivers/scsi/lpfc/lpfc_init.c             | 100 +++++++---------------
>  drivers/scsi/megaraid/megaraid_sas_base.c |  61 ++++---------
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c      |  36 +++-----
>  drivers/scsi/mvumi.c                      |  49 +++--------
>  drivers/scsi/pm8001/pm8001_init.c         |  46 ++++------
>  drivers/scsi/pmcraid.c                    |  44 +++-------
>  23 files changed, 212 insertions(+), 489 deletions(-)
> 
> -- 
> 2.27.0
> 
Please review the patch-series.

Thanks
Vaibhav Gupta

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
@ 2020-09-08 16:57   ` Vaibhav Gupta
  2020-09-08 17:32   ` Bjorn Helgaas
  1 sibling, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 16:57 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:14PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks. As the .suspend() work with "struct instance*",
> extract it from "struct device*" using dev_get_drv_data().
> 
> Driver was also using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
>  1 file changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 00668335c2af..4a6ee7778977 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
>  	megasas_return_cmd(instance, cmd);
>  }
>  
> -#ifdef CONFIG_PM
>  /**
>   * megasas_suspend -	driver suspend entry point
> - * @pdev:		PCI device structure
> - * @state:		PCI power state to suspend routine
> + * @dev:		Device structure
>   */
> -static int
> -megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused
> +megasas_suspend(struct device *dev)
>  {
> -	struct megasas_instance *instance;
> -
> -	instance = pci_get_drvdata(pdev);
> +	struct megasas_instance *instance = dev_get_drvdata(dev);
>  
>  	if (!instance)
>  		return 0;
>  
>  	instance->unload = 1;
>  
> -	dev_info(&pdev->dev, "%s is called\n", __func__);
> +	dev_info(dev, "%s is called\n", __func__);
>  
>  	/* Shutdown SR-IOV heartbeat timer */
>  	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
> @@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
>  
>  	tasklet_kill(&instance->isr_tasklet);
>  
> -	pci_set_drvdata(instance->pdev, instance);
> +	dev_set_drvdata(dev, instance);
>  	instance->instancet->disable_intr(instance);
>  
>  	megasas_destroy_irqs(instance);
> @@ -7587,48 +7583,28 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
>  	if (instance->msix_vectors)
>  		pci_free_irq_vectors(instance->pdev);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	return 0;
>  }
>  
>  /**
>   * megasas_resume-      driver resume entry point
> - * @pdev:               PCI device structure
> + * @dev:              Device structure
>   */
> -static int
> -megasas_resume(struct pci_dev *pdev)
> +static int __maybe_unused
> +megasas_resume(struct device *dev)
>  {
>  	int rval;
>  	struct Scsi_Host *host;
> -	struct megasas_instance *instance;
> +	struct megasas_instance *instance = dev_get_drvdata(dev);
>  	u32 status_reg;
>  
> -	instance = pci_get_drvdata(pdev);
> -
>  	if (!instance)
>  		return 0;
>  
>  	host = instance->host;
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> +	device_wakeup_disable(dev);
>  
> -	dev_info(&pdev->dev, "%s is called\n", __func__);
> -	/*
> -	 * PCI prepping: enable device set bus mastering and dma mask
> -	 */
> -	rval = pci_enable_device_mem(pdev);
> -
> -	if (rval) {
> -		dev_err(&pdev->dev, "Enable device failed\n");
> -		return rval;
> -	}
> -
> -	pci_set_master(pdev);
> +	dev_info(dev, "%s is called\n", __func__);
>  
>  	/*
>  	 * We expect the FW state to be READY
> @@ -7754,14 +7730,8 @@ megasas_resume(struct pci_dev *pdev)
>  fail_set_dma_mask:
>  fail_ready_state:
>  
> -	pci_disable_device(pdev);
> -
>  	return -ENODEV;
>  }
> -#else
> -#define megasas_suspend	NULL
> -#define megasas_resume	NULL
> -#endif
>  
>  static inline int
>  megasas_wait_for_adapter_operational(struct megasas_instance *instance)
> @@ -7931,7 +7901,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
>  
>  /**
>   * megasas_shutdown -	Shutdown entry point
> - * @device:		Generic device structure
> + * @pdev:		PCI device structure
>   */
>  static void megasas_shutdown(struct pci_dev *pdev)
>  {
> @@ -8508,6 +8478,8 @@ static const struct file_operations megasas_mgmt_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(megasas_pm_ops, megasas_suspend, megasas_resume);
> +
>  /*
>   * PCI hotplug support registration structure
>   */
> @@ -8517,8 +8489,7 @@ static struct pci_driver megasas_pci_driver = {
>  	.id_table = megasas_pci_table,
>  	.probe = megasas_probe_one,
>  	.remove = megasas_detach_one,
> -	.suspend = megasas_suspend,
> -	.resume = megasas_resume,
> +	.driver.pm = &megasas_pm_ops,
>  	.shutdown = megasas_shutdown,
>  };
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 02/15] scsi: aacraid: use generic power management
  2020-07-20 13:34 ` [PATCH v2 02/15] scsi: aacraid: " Vaibhav Gupta
  2020-07-23  6:58   ` Balsundar.P
@ 2020-09-08 16:58   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 16:58 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:15PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks.
> 
> Driver was using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/aacraid/linit.c | 34 ++++++++--------------------------
>  1 file changed, 8 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/scsi/aacraid/linit.c b/drivers/scsi/aacraid/linit.c
> index a308e86a97f1..1e44868ee953 100644
> --- a/drivers/scsi/aacraid/linit.c
> +++ b/drivers/scsi/aacraid/linit.c
> @@ -1910,11 +1910,9 @@ static int aac_acquire_resources(struct aac_dev *dev)
>  
>  }
>  
> -#if (defined(CONFIG_PM))
> -static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused aac_suspend(struct device *dev)
>  {
> -
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev);
>  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
>  
>  	scsi_host_block(shost);
> @@ -1923,29 +1921,16 @@ static int aac_suspend(struct pci_dev *pdev, pm_message_t state)
>  
>  	aac_release_resources(aac);
>  
> -	pci_set_drvdata(pdev, shost);
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	return 0;
>  }
>  
> -static int aac_resume(struct pci_dev *pdev)
> +static int __maybe_unused aac_resume(struct device *dev)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev);
>  	struct aac_dev *aac = (struct aac_dev *)shost->hostdata;
> -	int r;
>  
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -	r = pci_enable_device(pdev);
> +	device_wakeup_disable(dev);
>  
> -	if (r)
> -		goto fail_device;
> -
> -	pci_set_master(pdev);
>  	if (aac_acquire_resources(aac))
>  		goto fail_device;
>  	/*
> @@ -1960,10 +1945,8 @@ static int aac_resume(struct pci_dev *pdev)
>  fail_device:
>  	printk(KERN_INFO "%s%d: resume failed.\n", aac->name, aac->id);
>  	scsi_host_put(shost);
> -	pci_disable_device(pdev);
>  	return -ENODEV;
>  }
> -#endif
>  
>  static void aac_shutdown(struct pci_dev *dev)
>  {
> @@ -2108,15 +2091,14 @@ static struct pci_error_handlers aac_pci_err_handler = {
>  	.resume			= aac_pci_resume,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(aac_pm_ops, aac_suspend, aac_resume);
> +
>  static struct pci_driver aac_pci_driver = {
>  	.name		= AAC_DRIVERNAME,
>  	.id_table	= aac_pci_tbl,
>  	.probe		= aac_probe_one,
>  	.remove		= aac_remove_one,
> -#if (defined(CONFIG_PM))
> -	.suspend	= aac_suspend,
> -	.resume		= aac_resume,
> -#endif
> +	.driver.pm	= &aac_pm_ops,
>  	.shutdown	= aac_shutdown,
>  	.err_handler    = &aac_pci_err_handler,
>  };
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 03/15] scsi: aic7xxx: use generic power management
  2020-07-20 13:34 ` [PATCH v2 03/15] scsi: aic7xxx: " Vaibhav Gupta
@ 2020-09-08 16:59   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 16:59 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:16PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks.
> 
> Driver was using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Also, .suspend() and .resume() are invoking other functions for PM, which
> were againg bounded by "#ifdef CONFIG_PM" directive. Remove the directive
> and mark those functions as "__maybe_unused".
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/aic7xxx/aic7xxx.h         | 10 ++----
>  drivers/scsi/aic7xxx/aic7xxx_core.c    |  6 ++--
>  drivers/scsi/aic7xxx/aic7xxx_osm_pci.c | 46 ++++++--------------------
>  drivers/scsi/aic7xxx/aic7xxx_pci.c     |  4 +--
>  4 files changed, 17 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/scsi/aic7xxx/aic7xxx.h b/drivers/scsi/aic7xxx/aic7xxx.h
> index 88b90f9806c9..11a09798e6b5 100644
> --- a/drivers/scsi/aic7xxx/aic7xxx.h
> +++ b/drivers/scsi/aic7xxx/aic7xxx.h
> @@ -1134,9 +1134,7 @@ const struct ahc_pci_identity	*ahc_find_pci_device(ahc_dev_softc_t);
>  int			 ahc_pci_config(struct ahc_softc *,
>  					const struct ahc_pci_identity *);
>  int			 ahc_pci_test_register_access(struct ahc_softc *);
> -#ifdef CONFIG_PM
> -void			 ahc_pci_resume(struct ahc_softc *ahc);
> -#endif
> +void __maybe_unused	 ahc_pci_resume(struct ahc_softc *ahc);
>  
>  /*************************** EISA/VL Front End ********************************/
>  struct aic7770_identity *aic7770_find_device(uint32_t);
> @@ -1160,10 +1158,8 @@ int			 ahc_chip_init(struct ahc_softc *ahc);
>  int			 ahc_init(struct ahc_softc *ahc);
>  void			 ahc_intr_enable(struct ahc_softc *ahc, int enable);
>  void			 ahc_pause_and_flushwork(struct ahc_softc *ahc);
> -#ifdef CONFIG_PM
> -int			 ahc_suspend(struct ahc_softc *ahc); 
> -int			 ahc_resume(struct ahc_softc *ahc);
> -#endif
> +int __maybe_unused	 ahc_suspend(struct ahc_softc *ahc);
> +int __maybe_unused	 ahc_resume(struct ahc_softc *ahc);
>  void			 ahc_set_unit(struct ahc_softc *, int);
>  void			 ahc_set_name(struct ahc_softc *, char *);
>  void			 ahc_free(struct ahc_softc *ahc);
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_core.c b/drivers/scsi/aic7xxx/aic7xxx_core.c
> index 3d4df906fa4f..c7eb238a9599 100644
> --- a/drivers/scsi/aic7xxx/aic7xxx_core.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_core.c
> @@ -5590,8 +5590,7 @@ ahc_pause_and_flushwork(struct ahc_softc *ahc)
>  	ahc->flags &= ~AHC_ALL_INTERRUPTS;
>  }
>  
> -#ifdef CONFIG_PM
> -int
> +int __maybe_unused
>  ahc_suspend(struct ahc_softc *ahc)
>  {
>  
> @@ -5617,7 +5616,7 @@ ahc_suspend(struct ahc_softc *ahc)
>  	return (0);
>  }
>  
> -int
> +int __maybe_unused
>  ahc_resume(struct ahc_softc *ahc)
>  {
>  
> @@ -5626,7 +5625,6 @@ ahc_resume(struct ahc_softc *ahc)
>  	ahc_restart(ahc);
>  	return (0);
>  }
> -#endif
>  /************************** Busy Target Table *********************************/
>  /*
>   * Return the untagged transaction id for a given target/channel lun.
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> index 9b293b1f0b71..a07e94fac673 100644
> --- a/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_osm_pci.c
> @@ -121,47 +121,23 @@ static const struct pci_device_id ahc_linux_pci_id_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, ahc_linux_pci_id_table);
>  
> -#ifdef CONFIG_PM
> -static int
> -ahc_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +static int __maybe_unused
> +ahc_linux_pci_dev_suspend(struct device *dev)
>  {
> -	struct ahc_softc *ahc = pci_get_drvdata(pdev);
> -	int rc;
> -
> -	if ((rc = ahc_suspend(ahc)))
> -		return rc;
> +	struct ahc_softc *ahc = dev_get_drvdata(dev);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -
> -	if (mesg.event & PM_EVENT_SLEEP)
> -		pci_set_power_state(pdev, PCI_D3hot);
> -
> -	return rc;
> +	return ahc_suspend(ahc);
>  }
>  
> -static int
> -ahc_linux_pci_dev_resume(struct pci_dev *pdev)
> +static int __maybe_unused
> +ahc_linux_pci_dev_resume(struct device *dev)
>  {
> -	struct ahc_softc *ahc = pci_get_drvdata(pdev);
> -	int rc;
> -
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_restore_state(pdev);
> -
> -	if ((rc = pci_enable_device(pdev))) {
> -		dev_printk(KERN_ERR, &pdev->dev,
> -			   "failed to enable device after resume (%d)\n", rc);
> -		return rc;
> -	}
> -
> -	pci_set_master(pdev);
> +	struct ahc_softc *ahc = dev_get_drvdata(dev);
>  
>  	ahc_pci_resume(ahc);
>  
>  	return (ahc_resume(ahc));
>  }
> -#endif
>  
>  static void
>  ahc_linux_pci_dev_remove(struct pci_dev *pdev)
> @@ -319,14 +295,14 @@ ahc_pci_write_config(ahc_dev_softc_t pci, int reg, uint32_t value, int width)
>  	}
>  }
>  
> +static SIMPLE_DEV_PM_OPS(ahc_linux_pci_dev_pm_ops,
> +			 ahc_linux_pci_dev_suspend,
> +			 ahc_linux_pci_dev_resume);
>  
>  static struct pci_driver aic7xxx_pci_driver = {
>  	.name		= "aic7xxx",
>  	.probe		= ahc_linux_pci_dev_probe,
> -#ifdef CONFIG_PM
> -	.suspend	= ahc_linux_pci_dev_suspend,
> -	.resume		= ahc_linux_pci_dev_resume,
> -#endif
> +	.driver.pm	= &ahc_linux_pci_dev_pm_ops,
>  	.remove		= ahc_linux_pci_dev_remove,
>  	.id_table	= ahc_linux_pci_id_table
>  };
> diff --git a/drivers/scsi/aic7xxx/aic7xxx_pci.c b/drivers/scsi/aic7xxx/aic7xxx_pci.c
> index 656f680c7802..dab3a6d12c4d 100644
> --- a/drivers/scsi/aic7xxx/aic7xxx_pci.c
> +++ b/drivers/scsi/aic7xxx/aic7xxx_pci.c
> @@ -2008,8 +2008,7 @@ ahc_pci_chip_init(struct ahc_softc *ahc)
>  	return (ahc_chip_init(ahc));
>  }
>  
> -#ifdef CONFIG_PM
> -void
> +void __maybe_unused
>  ahc_pci_resume(struct ahc_softc *ahc)
>  {
>  	/*
> @@ -2040,7 +2039,6 @@ ahc_pci_resume(struct ahc_softc *ahc)
>  		ahc_release_seeprom(&sd);
>  	}
>  }
> -#endif
>  
>  static int
>  ahc_aic785X_setup(struct ahc_softc *ahc)
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 04/15] scsi: aic79xx: use generic power management
  2020-07-20 13:34 ` [PATCH v2 04/15] scsi: aic79xx: " Vaibhav Gupta
@ 2020-09-08 16:59   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 16:59 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:17PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks.
> 
> Driver was using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Also, .suspend() and .resume() were invoking other functions for PM, which
> were againg bounded by "#ifdef CONFIG_PM" directive. Remove them and mark
> them as "__maybe_unused".
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/aic7xxx/aic79xx.h         | 12 +++----
>  drivers/scsi/aic7xxx/aic79xx_core.c    |  8 ++---
>  drivers/scsi/aic7xxx/aic79xx_osm_pci.c | 43 +++++++-------------------
>  drivers/scsi/aic7xxx/aic79xx_pci.c     |  6 ++--
>  4 files changed, 20 insertions(+), 49 deletions(-)
> 
> diff --git a/drivers/scsi/aic7xxx/aic79xx.h b/drivers/scsi/aic7xxx/aic79xx.h
> index 9a515551641c..dd5dfd4f30a5 100644
> --- a/drivers/scsi/aic7xxx/aic79xx.h
> +++ b/drivers/scsi/aic7xxx/aic79xx.h
> @@ -1330,10 +1330,8 @@ const struct	ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
>  int			  ahd_pci_config(struct ahd_softc *,
>  					 const struct ahd_pci_identity *);
>  int	ahd_pci_test_register_access(struct ahd_softc *);
> -#ifdef CONFIG_PM
> -void	ahd_pci_suspend(struct ahd_softc *);
> -void	ahd_pci_resume(struct ahd_softc *);
> -#endif
> +void __maybe_unused	ahd_pci_suspend(struct ahd_softc *);
> +void __maybe_unused	ahd_pci_resume(struct ahd_softc *);
>  
>  /************************** SCB and SCB queue management **********************/
>  void		ahd_qinfifo_requeue_tail(struct ahd_softc *ahd,
> @@ -1344,10 +1342,8 @@ struct ahd_softc	*ahd_alloc(void *platform_arg, char *name);
>  int			 ahd_softc_init(struct ahd_softc *);
>  void			 ahd_controller_info(struct ahd_softc *ahd, char *buf);
>  int			 ahd_init(struct ahd_softc *ahd);
> -#ifdef CONFIG_PM
> -int			 ahd_suspend(struct ahd_softc *ahd);
> -void			 ahd_resume(struct ahd_softc *ahd);
> -#endif
> +int __maybe_unused	 ahd_suspend(struct ahd_softc *ahd);
> +void __maybe_unused	 ahd_resume(struct ahd_softc *ahd);
>  int			 ahd_default_config(struct ahd_softc *ahd);
>  int			 ahd_parse_vpddata(struct ahd_softc *ahd,
>  					   struct vpd_config *vpd);
> diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c b/drivers/scsi/aic7xxx/aic79xx_core.c
> index e4a09b93d00c..06ee7abd0f8f 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_core.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_core.c
> @@ -7880,11 +7880,9 @@ ahd_pause_and_flushwork(struct ahd_softc *ahd)
>  	ahd->flags &= ~AHD_ALL_INTERRUPTS;
>  }
>  
> -#ifdef CONFIG_PM
> -int
> +int __maybe_unused
>  ahd_suspend(struct ahd_softc *ahd)
>  {
> -
>  	ahd_pause_and_flushwork(ahd);
>  
>  	if (LIST_FIRST(&ahd->pending_scbs) != NULL) {
> @@ -7895,15 +7893,13 @@ ahd_suspend(struct ahd_softc *ahd)
>  	return (0);
>  }
>  
> -void
> +void __maybe_unused
>  ahd_resume(struct ahd_softc *ahd)
>  {
> -
>  	ahd_reset(ahd, /*reinit*/TRUE);
>  	ahd_intr_enable(ahd, TRUE); 
>  	ahd_restart(ahd);
>  }
> -#endif
>  
>  /************************** Busy Target Table *********************************/
>  /*
> diff --git a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
> index 8b891a05d9e7..07b670b80f1b 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_osm_pci.c
> @@ -74,11 +74,10 @@ static const struct pci_device_id ahd_linux_pci_id_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, ahd_linux_pci_id_table);
>  
> -#ifdef CONFIG_PM
> -static int
> -ahd_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
> +static int __maybe_unused
> +ahd_linux_pci_dev_suspend(struct device *dev)
>  {
> -	struct ahd_softc *ahd = pci_get_drvdata(pdev);
> +	struct ahd_softc *ahd = dev_get_drvdata(dev);
>  	int rc;
>  
>  	if ((rc = ahd_suspend(ahd)))
> @@ -86,39 +85,20 @@ ahd_linux_pci_dev_suspend(struct pci_dev *pdev, pm_message_t mesg)
>  
>  	ahd_pci_suspend(ahd);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -
> -	if (mesg.event & PM_EVENT_SLEEP)
> -		pci_set_power_state(pdev, PCI_D3hot);
> -
>  	return rc;
>  }
>  
> -static int
> -ahd_linux_pci_dev_resume(struct pci_dev *pdev)
> +static int __maybe_unused
> +ahd_linux_pci_dev_resume(struct device *dev)
>  {
> -	struct ahd_softc *ahd = pci_get_drvdata(pdev);
> -	int rc;
> -
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_restore_state(pdev);
> -
> -	if ((rc = pci_enable_device(pdev))) {
> -		dev_printk(KERN_ERR, &pdev->dev,
> -			   "failed to enable device after resume (%d)\n", rc);
> -		return rc;
> -	}
> -
> -	pci_set_master(pdev);
> +	struct ahd_softc *ahd = dev_get_drvdata(dev);
>  
>  	ahd_pci_resume(ahd);
>  
>  	ahd_resume(ahd);
>  
> -	return rc;
> +	return 0;
>  }
> -#endif
>  
>  static void
>  ahd_linux_pci_dev_remove(struct pci_dev *pdev)
> @@ -224,13 +204,14 @@ ahd_linux_pci_dev_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>  	return (0);
>  }
>  
> +static SIMPLE_DEV_PM_OPS(ahd_linux_pci_dev_pm_ops,
> +			 ahd_linux_pci_dev_suspend,
> +			 ahd_linux_pci_dev_resume);
> +
>  static struct pci_driver aic79xx_pci_driver = {
>  	.name		= "aic79xx",
>  	.probe		= ahd_linux_pci_dev_probe,
> -#ifdef CONFIG_PM
> -	.suspend	= ahd_linux_pci_dev_suspend,
> -	.resume		= ahd_linux_pci_dev_resume,
> -#endif
> +	.driver.pm	= &ahd_linux_pci_dev_pm_ops,
>  	.remove		= ahd_linux_pci_dev_remove,
>  	.id_table	= ahd_linux_pci_id_table
>  };
> diff --git a/drivers/scsi/aic7xxx/aic79xx_pci.c b/drivers/scsi/aic7xxx/aic79xx_pci.c
> index 8397ae93f7dd..2f0bdb9225a4 100644
> --- a/drivers/scsi/aic7xxx/aic79xx_pci.c
> +++ b/drivers/scsi/aic7xxx/aic79xx_pci.c
> @@ -377,8 +377,7 @@ ahd_pci_config(struct ahd_softc *ahd, const struct ahd_pci_identity *entry)
>  	return ahd_pci_map_int(ahd);
>  }
>  
> -#ifdef CONFIG_PM
> -void
> +void __maybe_unused
>  ahd_pci_suspend(struct ahd_softc *ahd)
>  {
>  	/*
> @@ -394,7 +393,7 @@ ahd_pci_suspend(struct ahd_softc *ahd)
>  
>  }
>  
> -void
> +void __maybe_unused
>  ahd_pci_resume(struct ahd_softc *ahd)
>  {
>  	ahd_pci_write_config(ahd->dev_softc, DEVCONFIG,
> @@ -404,7 +403,6 @@ ahd_pci_resume(struct ahd_softc *ahd)
>  	ahd_pci_write_config(ahd->dev_softc, CSIZE_LATTIME,
>  			     ahd->suspend_state.pci_state.csize_lattime, /*bytes*/1);
>  }
> -#endif
>  
>  /*
>   * Perform some simple tests that should catch situations where
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 05/15] scsi: arcmsr: use generic power management
  2020-07-20 13:34 ` [PATCH v2 05/15] scsi: arcmsr: " Vaibhav Gupta
@ 2020-09-08 17:00   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:00 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:18PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks.
> 
> Driver was also using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/arcmsr/arcmsr_hba.c | 35 ++++++++++++--------------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
> index 30914c8f29cc..7e098ddcc4f5 100644
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c
> @@ -113,8 +113,8 @@ static int arcmsr_bios_param(struct scsi_device *sdev,
>  static int arcmsr_queue_command(struct Scsi_Host *h, struct scsi_cmnd *cmd);
>  static int arcmsr_probe(struct pci_dev *pdev,
>  				const struct pci_device_id *id);
> -static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state);
> -static int arcmsr_resume(struct pci_dev *pdev);
> +static int __maybe_unused arcmsr_suspend(struct device *dev);
> +static int __maybe_unused arcmsr_resume(struct device *dev);
>  static void arcmsr_remove(struct pci_dev *pdev);
>  static void arcmsr_shutdown(struct pci_dev *pdev);
>  static void arcmsr_iop_init(struct AdapterControlBlock *acb);
> @@ -213,13 +213,14 @@ static struct pci_device_id arcmsr_device_id_table[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, arcmsr_device_id_table);
>  
> +static SIMPLE_DEV_PM_OPS(arcmsr_pm_ops, arcmsr_suspend, arcmsr_resume);
> +
>  static struct pci_driver arcmsr_pci_driver = {
>  	.name			= "arcmsr",
>  	.id_table		= arcmsr_device_id_table,
>  	.probe			= arcmsr_probe,
>  	.remove			= arcmsr_remove,
> -	.suspend		= arcmsr_suspend,
> -	.resume			= arcmsr_resume,
> +	.driver.pm		= &arcmsr_pm_ops,
>  	.shutdown		= arcmsr_shutdown,
>  };
>  /*
> @@ -1065,14 +1066,14 @@ static void arcmsr_free_irq(struct pci_dev *pdev,
>  	pci_free_irq_vectors(pdev);
>  }
>  
> -static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused arcmsr_suspend(struct device *dev)
>  {
> -	uint32_t intmask_org;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>  	struct AdapterControlBlock *acb =
>  		(struct AdapterControlBlock *)host->hostdata;
>  
> -	intmask_org = arcmsr_disable_outbound_ints(acb);
> +	arcmsr_disable_outbound_ints(acb);
>  	arcmsr_free_irq(pdev, acb);
>  	del_timer_sync(&acb->eternal_timer);
>  	if (set_date_time)
> @@ -1080,29 +1081,21 @@ static int arcmsr_suspend(struct pci_dev *pdev, pm_message_t state)
>  	flush_work(&acb->arcmsr_do_message_isr_bh);
>  	arcmsr_stop_adapter_bgrb(acb);
>  	arcmsr_flush_adapter_cache(acb);
> -	pci_set_drvdata(pdev, host);
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
>  	return 0;
>  }
>  
> -static int arcmsr_resume(struct pci_dev *pdev)
> +static int __maybe_unused arcmsr_resume(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>  	struct AdapterControlBlock *acb =
>  		(struct AdapterControlBlock *)host->hostdata;
>  
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -	if (pci_enable_device(pdev)) {
> -		pr_warn("%s: pci_enable_device error\n", __func__);
> -		return -ENODEV;
> -	}
> +	device_wakeup_disable(dev);
> +
>  	if (arcmsr_set_dma_mask(acb))
>  		goto controller_unregister;
> -	pci_set_master(pdev);
> +
>  	if (arcmsr_request_irq(pdev, acb) == FAILED)
>  		goto controller_stop;
>  	switch (acb->adapter_type) {
> @@ -1137,9 +1130,7 @@ static int arcmsr_resume(struct pci_dev *pdev)
>  	scsi_remove_host(host);
>  	arcmsr_free_ccb_pool(acb);
>  	arcmsr_unmap_pciregion(acb);
> -	pci_release_regions(pdev);
>  	scsi_host_put(host);
> -	pci_disable_device(pdev);
>  	return -ENODEV;
>  }
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 06/15] scsi: esas2r: use generic power management
  2020-07-20 13:34 ` [PATCH v2 06/15] scsi: esas2r: " Vaibhav Gupta
@ 2020-09-08 17:01   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:01 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:19PM +0530, Vaibhav Gupta wrote:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
> 
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
> 
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/esas2r/esas2r.h      |  5 ++--
>  drivers/scsi/esas2r/esas2r_init.c | 48 +++++++++----------------------
>  drivers/scsi/esas2r/esas2r_main.c |  3 +-
>  3 files changed, 18 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
> index 7f43b95f4e94..6ad3e0871ef0 100644
> --- a/drivers/scsi/esas2r/esas2r.h
> +++ b/drivers/scsi/esas2r/esas2r.h
> @@ -996,8 +996,9 @@ void esas2r_adapter_tasklet(unsigned long context);
>  irqreturn_t esas2r_interrupt(int irq, void *dev_id);
>  irqreturn_t esas2r_msi_interrupt(int irq, void *dev_id);
>  void esas2r_kickoff_timer(struct esas2r_adapter *a);
> -int esas2r_suspend(struct pci_dev *pcid, pm_message_t state);
> -int esas2r_resume(struct pci_dev *pcid);
> +
> +extern const struct dev_pm_ops esas2r_pm_ops;
> +
>  void esas2r_fw_event_off(struct esas2r_adapter *a);
>  void esas2r_fw_event_on(struct esas2r_adapter *a);
>  bool esas2r_nvram_write(struct esas2r_adapter *a, struct esas2r_request *rq,
> diff --git a/drivers/scsi/esas2r/esas2r_init.c b/drivers/scsi/esas2r/esas2r_init.c
> index eb7d139ffc00..6c5854969791 100644
> --- a/drivers/scsi/esas2r/esas2r_init.c
> +++ b/drivers/scsi/esas2r/esas2r_init.c
> @@ -640,53 +640,31 @@ void esas2r_kill_adapter(int i)
>  	}
>  }
>  
> -int esas2r_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused esas2r_suspend(struct device *dev)
>  {
> -	struct Scsi_Host *host = pci_get_drvdata(pdev);
> -	u32 device_state;
> +	struct Scsi_Host *host = dev_get_drvdata(dev);
>  	struct esas2r_adapter *a = (struct esas2r_adapter *)host->hostdata;
>  
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "suspending adapter()");
> +	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "suspending adapter()");
>  	if (!a)
>  		return -ENODEV;
>  
>  	esas2r_adapter_power_down(a, 1);
> -	device_state = pci_choose_state(pdev, state);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_save_state() called");
> -	pci_save_state(pdev);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_disable_device() called");
> -	pci_disable_device(pdev);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_set_power_state() called");
> -	pci_set_power_state(pdev, device_state);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "esas2r_suspend(): 0");
> +	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "esas2r_suspend(): 0");
>  	return 0;
>  }
>  
> -int esas2r_resume(struct pci_dev *pdev)
> +static int __maybe_unused esas2r_resume(struct device *dev)
>  {
> -	struct Scsi_Host *host = pci_get_drvdata(pdev);
> +	struct Scsi_Host *host = dev_get_drvdata(dev);
>  	struct esas2r_adapter *a = (struct esas2r_adapter *)host->hostdata;
> -	int rez;
> +	int rez = 0;
>  
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev), "resuming adapter()");
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_set_power_state(PCI_D0) "
> +	esas2r_log_dev(ESAS2R_LOG_INFO, dev, "resuming adapter()");
> +	esas2r_log_dev(ESAS2R_LOG_INFO, dev,
> +		       "device_wakeup_disable() "
>  		       "called");
> -	pci_set_power_state(pdev, PCI_D0);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_enable_wake(PCI_D0, 0) "
> -		       "called");
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_restore_state() called");
> -	pci_restore_state(pdev);
> -	esas2r_log_dev(ESAS2R_LOG_INFO, &(pdev->dev),
> -		       "pci_enable_device() called");
> -	rez = pci_enable_device(pdev);
> -	pci_set_master(pdev);
> +	device_wakeup_disable(dev);
>  
>  	if (!a) {
>  		rez = -ENODEV;
> @@ -730,11 +708,13 @@ int esas2r_resume(struct pci_dev *pdev)
>  	}
>  
>  error_exit:
> -	esas2r_log_dev(ESAS2R_LOG_CRIT, &(pdev->dev), "esas2r_resume(): %d",
> +	esas2r_log_dev(ESAS2R_LOG_CRIT, dev, "esas2r_resume(): %d",
>  		       rez);
>  	return rez;
>  }
>  
> +SIMPLE_DEV_PM_OPS(esas2r_pm_ops, esas2r_suspend, esas2r_resume);
> +
>  bool esas2r_set_degraded_mode(struct esas2r_adapter *a, char *error_str)
>  {
>  	set_bit(AF_DEGRADED_MODE, &a->flags);
> diff --git a/drivers/scsi/esas2r/esas2r_main.c b/drivers/scsi/esas2r/esas2r_main.c
> index 7b49e2e9fcde..aab3ea580e6b 100644
> --- a/drivers/scsi/esas2r/esas2r_main.c
> +++ b/drivers/scsi/esas2r/esas2r_main.c
> @@ -346,8 +346,7 @@ static struct pci_driver
>  	.id_table	= esas2r_pci_table,
>  	.probe		= esas2r_probe,
>  	.remove		= esas2r_remove,
> -	.suspend	= esas2r_suspend,
> -	.resume		= esas2r_resume,
> +	.driver.pm	= &esas2r_pm_ops,
>  };
>  
>  static int esas2r_probe(struct pci_dev *pcid,
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 07/15] scsi: hisi_sas_v3_hw: use generic power management
  2020-07-20 13:34 ` [PATCH v2 07/15] scsi: hisi_sas_v3_hw: " Vaibhav Gupta
  2020-07-21  1:36   ` chenxiang (M)
@ 2020-09-08 17:02   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:02 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:20PM +0530, Vaibhav Gupta wrote:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
> 
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
> 
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 32 +++++++++-----------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 55e2321a65bc..824bfbe1abbb 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,16 +3426,8 @@ 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_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -	rc = pci_enable_device(pdev);
> -	if (rc) {
> -		dev_err(dev, "enable device failed during resume (%d)\n", rc);
> -		return rc;
> -	}
> +	device_wakeup_disable(dev_d);
>  
> -	pci_set_master(pdev);
>  	scsi_unblock_requests(shost);
>  	clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
>  
> @@ -3447,7 +3435,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);
> @@ -3468,13 +3455,16 @@ static const struct pci_error_handlers hisi_sas_err_handler = {
>  	.reset_done	= hisi_sas_reset_done_v3_hw,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(hisi_sas_v3_pm_ops,
> +			 hisi_sas_v3_suspend,
> +			 hisi_sas_v3_resume);
> +
>  static struct pci_driver sas_v3_pci_driver = {
>  	.name		= DRV_NAME,
>  	.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.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 08/15] scsi: mpt3sas_scsih: use generic power management
  2020-07-20 13:34 ` [PATCH v2 08/15] scsi: mpt3sas_scsih: " Vaibhav Gupta
@ 2020-09-08 17:03   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:03 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:21PM +0530, Vaibhav Gupta wrote:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
> 
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
> 
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/mpt3sas/mpt3sas_scsih.c | 36 +++++++++++-----------------
>  1 file changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 08fc4b381056..f3c6e68b2921 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -10829,44 +10829,40 @@ _scsih_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  	return rv;
>  }
>  
> -#ifdef CONFIG_PM
>  /**
>   * scsih_suspend - power management suspend main entry point
> - * @pdev: PCI device struct
> - * @state: PM state change to (usually PCI_D3)
> + * @dev: Device struct
>   *
>   * Return: 0 success, anything else error.
>   */
> -static int
> -scsih_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused
> +scsih_suspend(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *shost = pci_get_drvdata(pdev);
>  	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
> -	pci_power_t device_state;
>  
>  	mpt3sas_base_stop_watchdog(ioc);
>  	flush_scheduled_work();
>  	scsi_block_requests(shost);
>  	_scsih_nvme_shutdown(ioc);
> -	device_state = pci_choose_state(pdev, state);
> -	ioc_info(ioc, "pdev=0x%p, slot=%s, entering operating state [D%d]\n",
> -		 pdev, pci_name(pdev), device_state);
> +	ioc_info(ioc, "pdev=0x%p, slot=%s, entering suspended state\n",
> +		 pdev, pci_name(pdev));
>  
> -	pci_save_state(pdev);
>  	mpt3sas_base_free_resources(ioc);
> -	pci_set_power_state(pdev, device_state);
>  	return 0;
>  }
>  
>  /**
>   * scsih_resume - power management resume main entry point
> - * @pdev: PCI device struct
> + * @dev: Device struct
>   *
>   * Return: 0 success, anything else error.
>   */
> -static int
> -scsih_resume(struct pci_dev *pdev)
> +static int __maybe_unused
> +scsih_resume(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *shost = pci_get_drvdata(pdev);
>  	struct MPT3SAS_ADAPTER *ioc = shost_priv(shost);
>  	pci_power_t device_state = pdev->current_state;
> @@ -10875,9 +10871,7 @@ scsih_resume(struct pci_dev *pdev)
>  	ioc_info(ioc, "pdev=0x%p, slot=%s, previous operating state [D%d]\n",
>  		 pdev, pci_name(pdev), device_state);
>  
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> +	device_wakeup_disable(dev);
>  	ioc->pdev = pdev;
>  	r = mpt3sas_base_map_resources(ioc);
>  	if (r)
> @@ -10888,7 +10882,6 @@ scsih_resume(struct pci_dev *pdev)
>  	mpt3sas_base_start_watchdog(ioc);
>  	return 0;
>  }
> -#endif /* CONFIG_PM */
>  
>  /**
>   * scsih_pci_error_detected - Called when a PCI error is detected.
> @@ -11162,6 +11155,8 @@ static struct pci_error_handlers _mpt3sas_err_handler = {
>  	.resume		= scsih_pci_resume,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(scsih_pm_ops, scsih_suspend, scsih_resume);
> +
>  static struct pci_driver mpt3sas_driver = {
>  	.name		= MPT3SAS_DRIVER_NAME,
>  	.id_table	= mpt3sas_pci_table,
> @@ -11169,10 +11164,7 @@ static struct pci_driver mpt3sas_driver = {
>  	.remove		= scsih_remove,
>  	.shutdown	= scsih_shutdown,
>  	.err_handler	= &_mpt3sas_err_handler,
> -#ifdef CONFIG_PM
> -	.suspend	= scsih_suspend,
> -	.resume		= scsih_resume,
> -#endif
> +	.driver.pm	= &scsih_pm_ops,
>  };
>  
>  /**
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 09/15] scsi: lpfc: use generic power management
  2020-07-20 13:34 ` [PATCH v2 09/15] scsi: lpfc: " Vaibhav Gupta
@ 2020-09-08 17:03   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:03 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:22PM +0530, Vaibhav Gupta wrote:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
> 
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
> 
> The driver was calling pci_save/restore_state(), pci_choose_state() and
> pci_set_power_state() which is no more needed.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/lpfc/lpfc_init.c | 100 +++++++++++-----------------------
>  1 file changed, 33 insertions(+), 67 deletions(-)
> 
> diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
> index 6637f84a3d1b..a36309b48144 100644
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -12452,8 +12452,7 @@ lpfc_pci_remove_one_s3(struct pci_dev *pdev)
>  
>  /**
>   * lpfc_pci_suspend_one_s3 - PCI func to suspend SLI-3 device for power mgmnt
> - * @pdev: pointer to PCI device
> - * @msg: power management message
> + * @dev_d: pointer to device
>   *
>   * This routine is to be called from the kernel's PCI subsystem to support
>   * system Power Management (PM) to device with SLI-3 interface spec. When
> @@ -12471,10 +12470,10 @@ lpfc_pci_remove_one_s3(struct pci_dev *pdev)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
> +static int __maybe_unused
> +lpfc_pci_suspend_one_s3(struct device *dev_d)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  
>  	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
> @@ -12488,16 +12487,12 @@ lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
>  	/* Disable interrupt from device */
>  	lpfc_sli_disable_intr(phba);
>  
> -	/* Save device state to PCI config space */
> -	pci_save_state(pdev);
> -	pci_set_power_state(pdev, PCI_D3hot);
> -
>  	return 0;
>  }
>  
>  /**
>   * lpfc_pci_resume_one_s3 - PCI func to resume SLI-3 device for power mgmnt
> - * @pdev: pointer to PCI device
> + * @dev_d: pointer to device
>   *
>   * This routine is to be called from the kernel's PCI subsystem to support
>   * system Power Management (PM) to device with SLI-3 interface spec. When PM
> @@ -12514,10 +12509,10 @@ lpfc_pci_suspend_one_s3(struct pci_dev *pdev, pm_message_t msg)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_resume_one_s3(struct pci_dev *pdev)
> +static int __maybe_unused
> +lpfc_pci_resume_one_s3(struct device *dev_d)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  	uint32_t intr_mode;
>  	int error;
> @@ -12525,19 +12520,6 @@ lpfc_pci_resume_one_s3(struct pci_dev *pdev)
>  	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
>  			"0452 PCI device Power Management resume.\n");
>  
> -	/* Restore device state from PCI config space */
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_restore_state(pdev);
> -
> -	/*
> -	 * As the new kernel behavior of pci_restore_state() API call clears
> -	 * device saved_state flag, need to save the restored state again.
> -	 */
> -	pci_save_state(pdev);
> -
> -	if (pdev->is_busmaster)
> -		pci_set_master(pdev);
> -
>  	/* Startup the kernel thread for this host adapter. */
>  	phba->worker_thread = kthread_run(lpfc_do_work, phba,
>  					"lpfc_worker_%d", phba->brd_no);
> @@ -13294,8 +13276,7 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
>  
>  /**
>   * lpfc_pci_suspend_one_s4 - PCI func to suspend SLI-4 device for power mgmnt
> - * @pdev: pointer to PCI device
> - * @msg: power management message
> + * @dev_d: pointer to device
>   *
>   * This routine is called from the kernel's PCI subsystem to support system
>   * Power Management (PM) to device with SLI-4 interface spec. When PM invokes
> @@ -13313,10 +13294,10 @@ lpfc_pci_remove_one_s4(struct pci_dev *pdev)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
> +static int __maybe_unused
> +lpfc_pci_suspend_one_s4(struct device *dev_d)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  
>  	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
> @@ -13331,16 +13312,12 @@ lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
>  	lpfc_sli4_disable_intr(phba);
>  	lpfc_sli4_queue_destroy(phba);
>  
> -	/* Save device state to PCI config space */
> -	pci_save_state(pdev);
> -	pci_set_power_state(pdev, PCI_D3hot);
> -
>  	return 0;
>  }
>  
>  /**
>   * lpfc_pci_resume_one_s4 - PCI func to resume SLI-4 device for power mgmnt
> - * @pdev: pointer to PCI device
> + * @dev_d: pointer to device
>   *
>   * This routine is called from the kernel's PCI subsystem to support system
>   * Power Management (PM) to device with SLI-4 interface spac. When PM invokes
> @@ -13357,10 +13334,10 @@ lpfc_pci_suspend_one_s4(struct pci_dev *pdev, pm_message_t msg)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_resume_one_s4(struct pci_dev *pdev)
> +static int __maybe_unused
> +lpfc_pci_resume_one_s4(struct device *dev_d)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev_d);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  	uint32_t intr_mode;
>  	int error;
> @@ -13368,19 +13345,6 @@ lpfc_pci_resume_one_s4(struct pci_dev *pdev)
>  	lpfc_printf_log(phba, KERN_INFO, LOG_INIT,
>  			"0292 PCI device Power Management resume.\n");
>  
> -	/* Restore device state from PCI config space */
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_restore_state(pdev);
> -
> -	/*
> -	 * As the new kernel behavior of pci_restore_state() API call clears
> -	 * device saved_state flag, need to save the restored state again.
> -	 */
> -	pci_save_state(pdev);
> -
> -	if (pdev->is_busmaster)
> -		pci_set_master(pdev);
> -
>  	 /* Startup the kernel thread for this host adapter. */
>  	phba->worker_thread = kthread_run(lpfc_do_work, phba,
>  					"lpfc_worker_%d", phba->brd_no);
> @@ -13696,8 +13660,7 @@ lpfc_pci_remove_one(struct pci_dev *pdev)
>  
>  /**
>   * lpfc_pci_suspend_one - lpfc PCI func to suspend dev for power management
> - * @pdev: pointer to PCI device
> - * @msg: power management message
> + * @dev: pointer to device
>   *
>   * This routine is to be registered to the kernel's PCI subsystem to support
>   * system Power Management (PM). When PM invokes this method, it dispatches
> @@ -13708,19 +13671,19 @@ lpfc_pci_remove_one(struct pci_dev *pdev)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
> +static int __maybe_unused
> +lpfc_pci_suspend_one(struct device *dev)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  	int rc = -ENODEV;
>  
>  	switch (phba->pci_dev_grp) {
>  	case LPFC_PCI_DEV_LP:
> -		rc = lpfc_pci_suspend_one_s3(pdev, msg);
> +		rc = lpfc_pci_suspend_one_s3(dev);
>  		break;
>  	case LPFC_PCI_DEV_OC:
> -		rc = lpfc_pci_suspend_one_s4(pdev, msg);
> +		rc = lpfc_pci_suspend_one_s4(dev);
>  		break;
>  	default:
>  		lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
> @@ -13733,7 +13696,7 @@ lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
>  
>  /**
>   * lpfc_pci_resume_one - lpfc PCI func to resume dev for power management
> - * @pdev: pointer to PCI device
> + * @dev: pointer to device
>   *
>   * This routine is to be registered to the kernel's PCI subsystem to support
>   * system Power Management (PM). When PM invokes this method, it dispatches
> @@ -13744,19 +13707,19 @@ lpfc_pci_suspend_one(struct pci_dev *pdev, pm_message_t msg)
>   * 	0 - driver suspended the device
>   * 	Error otherwise
>   **/
> -static int
> -lpfc_pci_resume_one(struct pci_dev *pdev)
> +static int __maybe_unused
> +lpfc_pci_resume_one(struct device *dev)
>  {
> -	struct Scsi_Host *shost = pci_get_drvdata(pdev);
> +	struct Scsi_Host *shost = dev_get_drvdata(dev);
>  	struct lpfc_hba *phba = ((struct lpfc_vport *)shost->hostdata)->phba;
>  	int rc = -ENODEV;
>  
>  	switch (phba->pci_dev_grp) {
>  	case LPFC_PCI_DEV_LP:
> -		rc = lpfc_pci_resume_one_s3(pdev);
> +		rc = lpfc_pci_resume_one_s3(dev);
>  		break;
>  	case LPFC_PCI_DEV_OC:
> -		rc = lpfc_pci_resume_one_s4(pdev);
> +		rc = lpfc_pci_resume_one_s4(dev);
>  		break;
>  	default:
>  		lpfc_printf_log(phba, KERN_ERR, LOG_INIT,
> @@ -13936,14 +13899,17 @@ static const struct pci_error_handlers lpfc_err_handler = {
>  	.resume = lpfc_io_resume,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(lpfc_pci_pm_ops_one,
> +			 lpfc_pci_suspend_one,
> +			 lpfc_pci_resume_one);
> +
>  static struct pci_driver lpfc_driver = {
>  	.name		= LPFC_DRIVER_NAME,
>  	.id_table	= lpfc_id_table,
>  	.probe		= lpfc_pci_probe_one,
>  	.remove		= lpfc_pci_remove_one,
>  	.shutdown	= lpfc_pci_remove_one,
> -	.suspend        = lpfc_pci_suspend_one,
> -	.resume		= lpfc_pci_resume_one,
> +	.driver.pm	= &lpfc_pci_pm_ops_one,
>  	.err_handler    = &lpfc_err_handler,
>  };
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 10/15] scsi: pm_8001: use generic power management
  2020-07-20 13:34 ` [PATCH v2 10/15] scsi: pm_8001: " Vaibhav Gupta
  2020-07-23  7:02   ` Jinpu Wang
@ 2020-09-08 17:04   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:04 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:23PM +0530, Vaibhav Gupta wrote:
> With legacy PM, drivers themselves were responsible for managing the
> device's power states and takes care of register states.
> 
> After upgrading to the generic structure, PCI core will take care of
> required tasks and drivers should do only device-specific operations.
> 
> The driver was calling pci_save/restore_state(), pci_choose_state(),
> pci_enable/disable_device() and pci_set_power_state() which is no more
> needed.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/pm8001/pm8001_init.c | 46 ++++++++++++-------------------
>  1 file changed, 17 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c
> index 9e99262a2b9d..d7d664b87720 100644
> --- a/drivers/scsi/pm8001/pm8001_init.c
> +++ b/drivers/scsi/pm8001/pm8001_init.c
> @@ -1178,23 +1178,21 @@ static void pm8001_pci_remove(struct pci_dev *pdev)
>  
>  /**
>   * pm8001_pci_suspend - power management suspend main entry point
> - * @pdev: PCI device struct
> - * @state: PM state change to (usually PCI_D3)
> + * @dev: Device struct
>   *
>   * Returns 0 success, anything else error.
>   */
> -static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused pm8001_pci_suspend(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
> -	struct pm8001_hba_info *pm8001_ha;
> +	struct pm8001_hba_info *pm8001_ha = sha->lldd_ha;
>  	int  i, j;
> -	u32 device_state;
> -	pm8001_ha = sha->lldd_ha;
>  	sas_suspend_ha(sha);
>  	flush_workqueue(pm8001_wq);
>  	scsi_block_requests(pm8001_ha->shost);
>  	if (!pdev->pm_cap) {
> -		dev_err(&pdev->dev, " PCI PM not supported\n");
> +		dev_err(dev, " PCI PM not supported\n");
>  		return -ENODEV;
>  	}
>  	PM8001_CHIP_DISP->interrupt_disable(pm8001_ha, 0xFF);
> @@ -1217,24 +1215,21 @@ static int pm8001_pci_suspend(struct pci_dev *pdev, pm_message_t state)
>  		for (j = 0; j < PM8001_MAX_MSIX_VEC; j++)
>  			tasklet_kill(&pm8001_ha->tasklet[j]);
>  #endif
> -	device_state = pci_choose_state(pdev, state);
>  	pm8001_printk("pdev=0x%p, slot=%s, entering "
> -		      "operating state [D%d]\n", pdev,
> -		      pm8001_ha->name, device_state);
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, device_state);
> +		      "suspended state\n", pdev,
> +		      pm8001_ha->name);
>  	return 0;
>  }
>  
>  /**
>   * pm8001_pci_resume - power management resume main entry point
> - * @pdev: PCI device struct
> + * @dev: Device struct
>   *
>   * Returns 0 success, anything else error.
>   */
> -static int pm8001_pci_resume(struct pci_dev *pdev)
> +static int __maybe_unused pm8001_pci_resume(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct sas_ha_struct *sha = pci_get_drvdata(pdev);
>  	struct pm8001_hba_info *pm8001_ha;
>  	int rc;
> @@ -1247,17 +1242,8 @@ static int pm8001_pci_resume(struct pci_dev *pdev)
>  	pm8001_printk("pdev=0x%p, slot=%s, resuming from previous "
>  		"operating state [D%d]\n", pdev, pm8001_ha->name, device_state);
>  
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -	rc = pci_enable_device(pdev);
> -	if (rc) {
> -		pm8001_printk("slot=%s Enable device failed during resume\n",
> -			      pm8001_ha->name);
> -		goto err_out_enable;
> -	}
> +	device_wakeup_disable(dev);
>  
> -	pci_set_master(pdev);
>  	rc = pci_go_44(pdev);
>  	if (rc)
>  		goto err_out_disable;
> @@ -1318,8 +1304,7 @@ static int pm8001_pci_resume(struct pci_dev *pdev)
>  
>  err_out_disable:
>  	scsi_remove_host(pm8001_ha->shost);
> -	pci_disable_device(pdev);
> -err_out_enable:
> +
>  	return rc;
>  }
>  
> @@ -1402,13 +1387,16 @@ static struct pci_device_id pm8001_pci_table[] = {
>  	{} /* terminate list */
>  };
>  
> +static SIMPLE_DEV_PM_OPS(pm8001_pci_pm_ops,
> +			 pm8001_pci_suspend,
> +			 pm8001_pci_resume);
> +
>  static struct pci_driver pm8001_pci_driver = {
>  	.name		= DRV_NAME,
>  	.id_table	= pm8001_pci_table,
>  	.probe		= pm8001_pci_probe,
>  	.remove		= pm8001_pci_remove,
> -	.suspend	= pm8001_pci_suspend,
> -	.resume		= pm8001_pci_resume,
> +	.driver.pm	= &pm8001_pci_pm_ops,
>  };
>  
>  /**
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 11/15] scsi: hpsa: use generic power management
  2020-07-20 13:34 ` [PATCH v2 11/15] scsi: hpsa: " Vaibhav Gupta
  2020-07-20 22:23   ` Don.Brace
@ 2020-09-08 17:05   ` Vaibhav Gupta
  1 sibling, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:05 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:24PM +0530, Vaibhav Gupta wrote:
> Drivers using legacy PM have to manage PCI states and device's PM states
> themselves. They also need to take care of configuration registers.
> 
> With improved and powerful support of generic PM, PCI Core takes care of
> above mentioned, device-independent, jobs.
> 
> Change function parameter in both .suspend() and .resume() to
> "struct device*" type. The function body remains unchanged as it was empty.
> Also, bind callbacks with "static const struct dev_pm_ops" variable.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/hpsa.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
> index 81d0414e2117..70bdd6fe91ee 100644
> --- a/drivers/scsi/hpsa.c
> +++ b/drivers/scsi/hpsa.c
> @@ -9070,25 +9070,27 @@ static void hpsa_remove_one(struct pci_dev *pdev)
>  	hpda_free_ctlr_info(h);				/* init_one 1 */
>  }
>  
> -static int hpsa_suspend(__attribute__((unused)) struct pci_dev *pdev,
> -	__attribute__((unused)) pm_message_t state)
> +static int __maybe_unused hpsa_suspend(
> +	__attribute__((unused)) struct device *dev)
>  {
>  	return -ENOSYS;
>  }
>  
> -static int hpsa_resume(__attribute__((unused)) struct pci_dev *pdev)
> +static int __maybe_unused hpsa_resume
> +	(__attribute__((unused)) struct device *dev)
>  {
>  	return -ENOSYS;
>  }
>  
> +static SIMPLE_DEV_PM_OPS(hpsa_pm_ops, hpsa_suspend, hpsa_resume);
> +
>  static struct pci_driver hpsa_pci_driver = {
>  	.name = HPSA,
>  	.probe = hpsa_init_one,
>  	.remove = hpsa_remove_one,
>  	.id_table = hpsa_pci_device_id,	/* id_table */
>  	.shutdown = hpsa_shutdown,
> -	.suspend = hpsa_suspend,
> -	.resume = hpsa_resume,
> +	.driver.pm = &hpsa_pm_ops,
>  };
>  
>  /* Fill in bucket_map[], given nsgs (the max number of
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 12/15] scsi: 3w-9xxx: use generic power management
  2020-07-20 13:34 ` [PATCH v2 12/15] scsi: 3w-9xxx: " Vaibhav Gupta
@ 2020-09-08 17:05   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:05 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:25PM +0530, Vaibhav Gupta wrote:
> Drivers using legacy PM have to manage PCI states and device's PM states
> themselves. They also need to take care of configuration registers.
> 
> With improved and powerful support of generic PM, PCI Core takes care of
> above mentioned, device-independent, jobs.
> 
> This driver makes use of PCI helper functions like
> pci_save/restore_state(), pci_enable/disable_device(),
> pci_set_power_state() and pci_set_master() to do required operations. In
> generic mode, they are no longer needed.
> 
> Change function parameter in both .suspend() and .resume() to
> "struct device*" type. Use to_pci_dev() and dev_get_drvdata() to get
> "struct pci_dev*" variable and drv data.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/3w-9xxx.c | 30 ++++++++----------------------
>  1 file changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index 3337b1e80412..c15dcfda3957 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -2191,10 +2191,10 @@ static void twa_remove(struct pci_dev *pdev)
>  	twa_device_extension_count--;
>  } /* End twa_remove() */
>  
> -#ifdef CONFIG_PM
>  /* This function is called on PCI suspend */
> -static int twa_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused twa_suspend(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
> @@ -2214,32 +2214,21 @@ static int twa_suspend(struct pci_dev *pdev, pm_message_t state)
>  	}
>  	TW_CLEAR_ALL_INTERRUPTS(tw_dev);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	return 0;
>  } /* End twa_suspend() */
>  
>  /* This function is called on PCI resume */
> -static int twa_resume(struct pci_dev *pdev)
> +static int __maybe_unused twa_resume(struct device *dev)
>  {
>  	int retval = 0;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
>  	printk(KERN_WARNING "3w-9xxx: Resuming host %d.\n", tw_dev->host->host_no);
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
>  
> -	retval = pci_enable_device(pdev);
> -	if (retval) {
> -		TW_PRINTK(tw_dev->host, TW_DRIVER, 0x39, "Enable device failed during resume");
> -		return retval;
> -	}
> +	device_wakeup_disable(dev);
>  
> -	pci_set_master(pdev);
>  	pci_try_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> @@ -2277,11 +2266,9 @@ static int twa_resume(struct pci_dev *pdev)
>  
>  out_disable_device:
>  	scsi_remove_host(host);
> -	pci_disable_device(pdev);
>  
>  	return retval;
>  } /* End twa_resume() */
> -#endif
>  
>  /* PCI Devices supported by this driver */
>  static struct pci_device_id twa_pci_tbl[] = {
> @@ -2297,16 +2284,15 @@ static struct pci_device_id twa_pci_tbl[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, twa_pci_tbl);
>  
> +static SIMPLE_DEV_PM_OPS(twa_pm_ops, twa_suspend, twa_resume);
> +
>  /* pci_driver initializer */
>  static struct pci_driver twa_driver = {
>  	.name		= "3w-9xxx",
>  	.id_table	= twa_pci_tbl,
>  	.probe		= twa_probe,
>  	.remove		= twa_remove,
> -#ifdef CONFIG_PM
> -	.suspend	= twa_suspend,
> -	.resume		= twa_resume,
> -#endif
> +	.driver.pm	= &twa_pm_ops,
>  	.shutdown	= twa_shutdown
>  };
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 13/15] scsi: 3w-sas: use generic power management
  2020-07-20 13:34 ` [PATCH v2 13/15] scsi: 3w-sas: " Vaibhav Gupta
@ 2020-09-08 17:06   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:06 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:26PM +0530, Vaibhav Gupta wrote:
> Drivers using legacy PM have to manage PCI states and device's PM states
> themselves. They also need to take care of configuration registers.
> 
> With improved and powerful support of generic PM, PCI Core takes care of
> above mentioned, device-independent, jobs.
> 
> This driver makes use of PCI helper functions like
> pci_save/restore_state(), pci_enable/disable_device(),
> pci_set_power_state() and pci_set_master() to do required operations. In
> generic mode, they are no longer needed.
> 
> Change function parameter in both .suspend() and .resume() to
> "struct device*" type. Use to_pci_dev() and dev_get_drvdata() to get
> "struct pci_dev*" variable and drv data.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/3w-sas.c | 31 ++++++++-----------------------
>  1 file changed, 8 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/scsi/3w-sas.c b/drivers/scsi/3w-sas.c
> index dda6fa857709..efaba30b0ca8 100644
> --- a/drivers/scsi/3w-sas.c
> +++ b/drivers/scsi/3w-sas.c
> @@ -1756,11 +1756,10 @@ static void twl_remove(struct pci_dev *pdev)
>  	twl_device_extension_count--;
>  } /* End twl_remove() */
>  
> -#ifdef CONFIG_PM
>  /* This function is called on PCI suspend */
> -static int twl_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused twl_suspend(struct device *dev)
>  {
> -	struct Scsi_Host *host = pci_get_drvdata(pdev);
> +	struct Scsi_Host *host = dev_get_drvdata(dev);
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
>  	printk(KERN_WARNING "3w-sas: Suspending host %d.\n", tw_dev->host->host_no);
> @@ -1779,32 +1778,21 @@ static int twl_suspend(struct pci_dev *pdev, pm_message_t state)
>  	/* Clear doorbell interrupt */
>  	TWL_CLEAR_DB_INTERRUPT(tw_dev);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	return 0;
>  } /* End twl_suspend() */
>  
>  /* This function is called on PCI resume */
> -static int twl_resume(struct pci_dev *pdev)
> +static int __maybe_unused twl_resume(struct device *dev)
>  {
>  	int retval = 0;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct Scsi_Host *host = pci_get_drvdata(pdev);
>  	TW_Device_Extension *tw_dev = (TW_Device_Extension *)host->hostdata;
>  
>  	printk(KERN_WARNING "3w-sas: Resuming host %d.\n", tw_dev->host->host_no);
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
>  
> -	retval = pci_enable_device(pdev);
> -	if (retval) {
> -		TW_PRINTK(tw_dev->host, TW_DRIVER, 0x24, "Enable device failed during resume");
> -		return retval;
> -	}
> +	device_wakeup_disable(dev);
>  
> -	pci_set_master(pdev);
>  	pci_try_set_mwi(pdev);
>  
>  	retval = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
> @@ -1842,11 +1830,9 @@ static int twl_resume(struct pci_dev *pdev)
>  
>  out_disable_device:
>  	scsi_remove_host(host);
> -	pci_disable_device(pdev);
>  
>  	return retval;
>  } /* End twl_resume() */
> -#endif
>  
>  /* PCI Devices supported by this driver */
>  static struct pci_device_id twl_pci_tbl[] = {
> @@ -1855,16 +1841,15 @@ static struct pci_device_id twl_pci_tbl[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, twl_pci_tbl);
>  
> +static SIMPLE_DEV_PM_OPS(twl_pm_ops, twl_suspend, twl_resume);
> +
>  /* pci_driver initializer */
>  static struct pci_driver twl_driver = {
>  	.name		= "3w-sas",
>  	.id_table	= twl_pci_tbl,
>  	.probe		= twl_probe,
>  	.remove		= twl_remove,
> -#ifdef CONFIG_PM
> -	.suspend	= twl_suspend,
> -	.resume		= twl_resume,
> -#endif
> +	.driver.pm	= &twl_pm_ops,
>  	.shutdown	= twl_shutdown
>  };
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 14/15] scsi: mvumi: use generic power management
  2020-07-20 13:34 ` [PATCH v2 14/15] scsi: mvumi: " Vaibhav Gupta
@ 2020-09-08 17:07   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:07 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:27PM +0530, Vaibhav Gupta wrote:
> Drivers using legacy PM have to manage PCI states and device's PM states
> themselves. They also need to take care of configuration registers.
> 
> With improved and powerful support of generic PM, PCI Core takes care of
> above mentioned, device-independent, jobs.
> 
> This driver makes use of PCI helper functions like
> pci_save/restore_state(), pci_enable/disable_device(),
> pci_set_power_state() and pci_set_master() to do required operations. In
> generic mode, they are no longer needed.
> 
> Change function parameter in both .suspend() and .resume() to
> "struct device*" type. Use dev_get_drvdata() to get drv data.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/mvumi.c | 49 ++++++++++----------------------------------
>  1 file changed, 11 insertions(+), 38 deletions(-)
> 
> diff --git a/drivers/scsi/mvumi.c b/drivers/scsi/mvumi.c
> index 8906aceda4c4..7a6ef8264e47 100644
> --- a/drivers/scsi/mvumi.c
> +++ b/drivers/scsi/mvumi.c
> @@ -2558,7 +2558,7 @@ static void mvumi_detach_one(struct pci_dev *pdev)
>  
>  /**
>   * mvumi_shutdown -	Shutdown entry point
> - * @device:		Generic device structure
> + * @pdev:		PCI device structure
>   */
>  static void mvumi_shutdown(struct pci_dev *pdev)
>  {
> @@ -2567,47 +2567,28 @@ static void mvumi_shutdown(struct pci_dev *pdev)
>  	mvumi_flush_cache(mhba);
>  }
>  
> -static int __maybe_unused mvumi_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused mvumi_suspend(struct device *dev)
>  {
> -	struct mvumi_hba *mhba = NULL;
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  
> -	mhba = pci_get_drvdata(pdev);
> +	struct mvumi_hba *mhba = pci_get_drvdata(pdev);
>  	mvumi_flush_cache(mhba);
>  
> -	pci_set_drvdata(pdev, mhba);
>  	mhba->instancet->disable_intr(mhba);
> -	free_irq(mhba->pdev->irq, mhba);
>  	mvumi_unmap_pci_addr(pdev, mhba->base_addr);
> -	pci_release_regions(pdev);
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
>  
>  	return 0;
>  }
>  
> -static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
> +static int __maybe_unused mvumi_resume(struct device *dev)
>  {
>  	int ret;
> -	struct mvumi_hba *mhba = NULL;
> -
> -	mhba = pci_get_drvdata(pdev);
> -
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct mvumi_hba *mhba = pci_get_drvdata(pdev);
>  
> -	ret = pci_enable_device(pdev);
> -	if (ret) {
> -		dev_err(&pdev->dev, "enable device failed\n");
> -		return ret;
> -	}
> +	device_wakeup_disable(dev);
>  
> -	ret = mvumi_pci_set_master(pdev);
>  	ret = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
> -	if (ret)
> -		goto fail;
> -	ret = pci_request_regions(mhba->pdev, MV_DRIVER_NAME);
>  	if (ret)
>  		goto fail;
>  	ret = mvumi_map_pci_addr(mhba->pdev, mhba->base_addr);
> @@ -2627,12 +2608,6 @@ static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
>  		goto unmap_pci_addr;
>  	}
>  
> -	ret = request_irq(mhba->pdev->irq, mvumi_isr_handler, IRQF_SHARED,
> -				"mvumi", mhba);
> -	if (ret) {
> -		dev_err(&pdev->dev, "failed to register IRQ\n");
> -		goto unmap_pci_addr;
> -	}
>  	mhba->instancet->enable_intr(mhba);
>  
>  	return 0;
> @@ -2642,11 +2617,12 @@ static int __maybe_unused mvumi_resume(struct pci_dev *pdev)
>  release_regions:
>  	pci_release_regions(pdev);
>  fail:
> -	pci_disable_device(pdev);
>  
>  	return ret;
>  }
>  
> +static SIMPLE_DEV_PM_OPS(mvumi_pm_ops, mvumi_suspend, mvumi_resume);
> +
>  static struct pci_driver mvumi_pci_driver = {
>  
>  	.name = MV_DRIVER_NAME,
> @@ -2654,10 +2630,7 @@ static struct pci_driver mvumi_pci_driver = {
>  	.probe = mvumi_probe_one,
>  	.remove = mvumi_detach_one,
>  	.shutdown = mvumi_shutdown,
> -#ifdef CONFIG_PM
> -	.suspend = mvumi_suspend,
> -	.resume = mvumi_resume,
> -#endif
> +	.driver.pm = &mvumi_pm_ops,
>  };
>  
>  module_pci_driver(mvumi_pci_driver);
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 15/15] scsi: pmcraid: use generic power management
  2020-07-20 13:34 ` [PATCH v2 15/15] scsi: pmcraid: " Vaibhav Gupta
@ 2020-09-08 17:08   ` Vaibhav Gupta
  0 siblings, 0 replies; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-08 17:08 UTC (permalink / raw)
  To: Bjorn Helgaas, Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta,
	Adam Radford, James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang
  Cc: Shuah Khan, linux-kernel, linux-kernel-mentees, linux-scsi,
	esc.storagedev, megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:28PM +0530, Vaibhav Gupta wrote:
> Drivers using legacy PM have to manage PCI states and device's PM states
> themselves. They also need to take care of configuration registers.
> 
> With improved and powerful support of generic PM, PCI Core takes care of
> above mentioned, device-independent, jobs.
> 
> This driver makes use of PCI helper functions like
> pci_save/restore_state(), pci_enable/disable_device(),
> pci_set_power_state() and to do required operations. In generic mode, they
> are no longer needed.
> 
> Change function parameter in both .suspend() and .resume() to
> "struct device*" type. Use to_pci_dev() to get "struct pci_dev*" variable.
> 
> In function pmcraid_resume(), earlier, the variable "rc" was set by
> pci_enable_device() which is now removed. Since PCI core does the required
> job, initialize "rc" with 0 value when declaring it.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/pmcraid.c | 44 +++++++++++-------------------------------
>  1 file changed, 11 insertions(+), 33 deletions(-)
> 
> diff --git a/drivers/scsi/pmcraid.c b/drivers/scsi/pmcraid.c
> index aa9ae2ae8579..b6b70ac2e2ee 100644
> --- a/drivers/scsi/pmcraid.c
> +++ b/drivers/scsi/pmcraid.c
> @@ -5237,54 +5237,39 @@ static void pmcraid_remove(struct pci_dev *pdev)
>  	return;
>  }
>  
> -#ifdef CONFIG_PM
>  /**
>   * pmcraid_suspend - driver suspend entry point for power management
> - * @pdev:   PCI device structure
> - * @state:  PCI power state to suspend routine
> + * @dev:   Device structure
>   *
>   * Return Value - 0 always
>   */
> -static int pmcraid_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused pmcraid_suspend(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct pmcraid_instance *pinstance = pci_get_drvdata(pdev);
>  
>  	pmcraid_shutdown(pdev);
>  	pmcraid_disable_interrupts(pinstance, ~0);
>  	pmcraid_kill_tasklets(pinstance);
> -	pci_set_drvdata(pinstance->pdev, pinstance);
>  	pmcraid_unregister_interrupt_handler(pinstance);
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
>  
>  	return 0;
>  }
>  
>  /**
>   * pmcraid_resume - driver resume entry point PCI power management
> - * @pdev: PCI device structure
> + * @dev: Device structure
>   *
>   * Return Value - 0 in case of success. Error code in case of any failure
>   */
> -static int pmcraid_resume(struct pci_dev *pdev)
> +static int __maybe_unused pmcraid_resume(struct device *dev)
>  {
> +	struct pci_dev *pdev = to_pci_dev(dev);
>  	struct pmcraid_instance *pinstance = pci_get_drvdata(pdev);
>  	struct Scsi_Host *host = pinstance->host;
> -	int rc;
> -
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> -
> -	rc = pci_enable_device(pdev);
> -
> -	if (rc) {
> -		dev_err(&pdev->dev, "resume: Enable device failed\n");
> -		return rc;
> -	}
> +	int rc = 0;
>  
> -	pci_set_master(pdev);
> +	device_wakeup_disable(dev);
>  
>  	if (sizeof(dma_addr_t) == 4 ||
>  	    dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)))
> @@ -5337,18 +5322,10 @@ static int pmcraid_resume(struct pci_dev *pdev)
>  	scsi_host_put(host);
>  
>  disable_device:
> -	pci_disable_device(pdev);
>  
>  	return rc;
>  }
>  
> -#else
> -
> -#define pmcraid_suspend NULL
> -#define pmcraid_resume  NULL
> -
> -#endif /* CONFIG_PM */
> -
>  /**
>   * pmcraid_complete_ioa_reset - Called by either timer or tasklet during
>   *				completion of the ioa reset
> @@ -5836,6 +5813,8 @@ static int pmcraid_probe(struct pci_dev *pdev,
>  	return -ENODEV;
>  }
>  
> +static SIMPLE_DEV_PM_OPS(pmcraid_pm_ops, pmcraid_suspend, pmcraid_resume);
> +
>  /*
>   * PCI driver structure of pmcraid driver
>   */
> @@ -5844,8 +5823,7 @@ static struct pci_driver pmcraid_driver = {
>  	.id_table = pmcraid_pci_table,
>  	.probe = pmcraid_probe,
>  	.remove = pmcraid_remove,
> -	.suspend = pmcraid_suspend,
> -	.resume = pmcraid_resume,
> +	.driver.pm = &pmcraid_pm_ops,
>  	.shutdown = pmcraid_shutdown
>  };
>  
> -- 
> 2.27.0
> 
.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
  2020-09-08 16:57   ` Vaibhav Gupta
@ 2020-09-08 17:32   ` Bjorn Helgaas
  2020-09-09 10:03     ` Vaibhav Gupta
  1 sibling, 1 reply; 41+ messages in thread
From: Bjorn Helgaas @ 2020-09-08 17:32 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang, Shuah Khan, linux-kernel,
	linux-kernel-mentees, linux-scsi, esc.storagedev,
	megaraidlinux.pdl, MPT-FusionLinux.pdl

On Mon, Jul 20, 2020 at 07:04:14PM +0530, Vaibhav Gupta wrote:
> With legacy PM hooks, it was the responsibility of a driver to manage PCI
> states and also the device's power state. The generic approach is to let
> the PCI core handle the work.
> 
> PCI core passes "struct device*" as an argument to the .suspend() and
> .resume() callbacks. As the .suspend() work with "struct instance*",
> extract it from "struct device*" using dev_get_drv_data().
> 
> Driver was also using PCI helper functions like pci_save/restore_state(),
> pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> They should not be invoked by the driver.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> ---
>  drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
>  1 file changed, 16 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index 00668335c2af..4a6ee7778977 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
>  	megasas_return_cmd(instance, cmd);
>  }
>  
> -#ifdef CONFIG_PM
>  /**
>   * megasas_suspend -	driver suspend entry point
> - * @pdev:		PCI device structure
> - * @state:		PCI power state to suspend routine
> + * @dev:		Device structure
>   */
> -static int
> -megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused
> +megasas_suspend(struct device *dev)
>  {
> -	struct megasas_instance *instance;
> -
> -	instance = pci_get_drvdata(pdev);
> +	struct megasas_instance *instance = dev_get_drvdata(dev);
>  
>  	if (!instance)
>  		return 0;
>  
>  	instance->unload = 1;
>  
> -	dev_info(&pdev->dev, "%s is called\n", __func__);
> +	dev_info(dev, "%s is called\n", __func__);
>  
>  	/* Shutdown SR-IOV heartbeat timer */
>  	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
> @@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
>  
>  	tasklet_kill(&instance->isr_tasklet);
>  
> -	pci_set_drvdata(instance->pdev, instance);
> +	dev_set_drvdata(dev, instance);

It *might* be correct to replace "instance->pdev" with "dev", but it's
not obvious and deserves some explanation.  It's true that you can
replace &pdev->dev with dev, but I don't know anything about
instance->dev.

I don't think this change is actually necessary, is it?
"instance->pdev" is still a pci_dev pointer, so pci_set_drvdata()
should work fine.

It looks goofy to use pci_set_drvdata() or dev_set_drvdata() in a
suspend routine, but I didn't bother trying to figure out what's going
on here.

>  	instance->instancet->disable_intr(instance);
>  
>  	megasas_destroy_irqs(instance);
> @@ -7587,48 +7583,28 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
>  	if (instance->msix_vectors)
>  		pci_free_irq_vectors(instance->pdev);
>  
> -	pci_save_state(pdev);
> -	pci_disable_device(pdev);
> -
> -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> -
>  	return 0;
>  }
>  
>  /**
>   * megasas_resume-      driver resume entry point
> - * @pdev:               PCI device structure
> + * @dev:              Device structure
>   */
> -static int
> -megasas_resume(struct pci_dev *pdev)
> +static int __maybe_unused
> +megasas_resume(struct device *dev)
>  {
>  	int rval;
>  	struct Scsi_Host *host;
> -	struct megasas_instance *instance;
> +	struct megasas_instance *instance = dev_get_drvdata(dev);
>  	u32 status_reg;
>  
> -	instance = pci_get_drvdata(pdev);
> -
>  	if (!instance)
>  		return 0;
>  
>  	host = instance->host;
> -	pci_set_power_state(pdev, PCI_D0);
> -	pci_enable_wake(pdev, PCI_D0, 0);
> -	pci_restore_state(pdev);
> +	device_wakeup_disable(dev);

Shouldn't there be a corresponding device_wakeup_enable() or similar
elsewhere?

Maybe the fact that megasas disables wakeup but never enables it is a
latent bug?

> -	dev_info(&pdev->dev, "%s is called\n", __func__);
> -	/*
> -	 * PCI prepping: enable device set bus mastering and dma mask
> -	 */
> -	rval = pci_enable_device_mem(pdev);
> -
> -	if (rval) {
> -		dev_err(&pdev->dev, "Enable device failed\n");
> -		return rval;
> -	}
> -
> -	pci_set_master(pdev);
> +	dev_info(dev, "%s is called\n", __func__);
>  
>  	/*
>  	 * We expect the FW state to be READY
> @@ -7754,14 +7730,8 @@ megasas_resume(struct pci_dev *pdev)
>  fail_set_dma_mask:
>  fail_ready_state:
>  
> -	pci_disable_device(pdev);
> -
>  	return -ENODEV;
>  }
> -#else
> -#define megasas_suspend	NULL
> -#define megasas_resume	NULL
> -#endif
>  
>  static inline int
>  megasas_wait_for_adapter_operational(struct megasas_instance *instance)
> @@ -7931,7 +7901,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
>  
>  /**
>   * megasas_shutdown -	Shutdown entry point
> - * @device:		Generic device structure
> + * @pdev:		PCI device structure

Looks like an unrelated typo fix?  I would put this in a separate
patch.

>   */
>  static void megasas_shutdown(struct pci_dev *pdev)
>  {
> @@ -8508,6 +8478,8 @@ static const struct file_operations megasas_mgmt_fops = {
>  	.llseek = noop_llseek,
>  };
>  
> +static SIMPLE_DEV_PM_OPS(megasas_pm_ops, megasas_suspend, megasas_resume);
> +
>  /*
>   * PCI hotplug support registration structure
>   */
> @@ -8517,8 +8489,7 @@ static struct pci_driver megasas_pci_driver = {
>  	.id_table = megasas_pci_table,
>  	.probe = megasas_probe_one,
>  	.remove = megasas_detach_one,
> -	.suspend = megasas_suspend,
> -	.resume = megasas_resume,
> +	.driver.pm = &megasas_pm_ops,
>  	.shutdown = megasas_shutdown,
>  };
>  
> -- 
> 2.27.0
> 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-09-08 17:32   ` Bjorn Helgaas
@ 2020-09-09 10:03     ` Vaibhav Gupta
  2020-09-09 13:23       ` Bjorn Helgaas
  0 siblings, 1 reply; 41+ messages in thread
From: Vaibhav Gupta @ 2020-09-09 10:03 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang, Shuah Khan, linux-kernel,
	linux-kernel-mentees, linux-scsi, esc.storagedev,
	megaraidlinux.pdl, MPT-FusionLinux.pdl

On Tue, Sep 08, 2020 at 12:32:09PM -0500, Bjorn Helgaas wrote:
> On Mon, Jul 20, 2020 at 07:04:14PM +0530, Vaibhav Gupta wrote:
> > With legacy PM hooks, it was the responsibility of a driver to manage PCI
> > states and also the device's power state. The generic approach is to let
> > the PCI core handle the work.
> > 
> > PCI core passes "struct device*" as an argument to the .suspend() and
> > .resume() callbacks. As the .suspend() work with "struct instance*",
> > extract it from "struct device*" using dev_get_drv_data().
> > 
> > Driver was also using PCI helper functions like pci_save/restore_state(),
> > pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> > They should not be invoked by the driver.
> > 
> > Compile-tested only.
> > 
> > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> > ---
> >  drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
> >  1 file changed, 16 insertions(+), 45 deletions(-)
> > 
> > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> > index 00668335c2af..4a6ee7778977 100644
> > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > @@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
> >  	megasas_return_cmd(instance, cmd);
> >  }
> >  
> > -#ifdef CONFIG_PM
> >  /**
> >   * megasas_suspend -	driver suspend entry point
> > - * @pdev:		PCI device structure
> > - * @state:		PCI power state to suspend routine
> > + * @dev:		Device structure
> >   */
> > -static int
> > -megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> > +static int __maybe_unused
> > +megasas_suspend(struct device *dev)
> >  {
> > -	struct megasas_instance *instance;
> > -
> > -	instance = pci_get_drvdata(pdev);
> > +	struct megasas_instance *instance = dev_get_drvdata(dev);
> >  
> >  	if (!instance)
> >  		return 0;
> >  
> >  	instance->unload = 1;
> >  
> > -	dev_info(&pdev->dev, "%s is called\n", __func__);
> > +	dev_info(dev, "%s is called\n", __func__);
> >  
> >  	/* Shutdown SR-IOV heartbeat timer */
> >  	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
> > @@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> >  
> >  	tasklet_kill(&instance->isr_tasklet);
> >  
> > -	pci_set_drvdata(instance->pdev, instance);
> > +	dev_set_drvdata(dev, instance);
> 
> It *might* be correct to replace "instance->pdev" with "dev", but it's
> not obvious and deserves some explanation.  It's true that you can
> replace &pdev->dev with dev, but I don't know anything about
> instance->dev.
> 
> I don't think this change is actually necessary, is it?
> "instance->pdev" is still a pci_dev pointer, so pci_set_drvdata()
> should work fine.
> 
> It looks goofy to use pci_set_drvdata() or dev_set_drvdata() in a
> suspend routine, but I didn't bother trying to figure out what's going
> on here.
> 
There is no instance->dev . The 'dev' passed dev_set_drvdata() is same
&pdev->dev. The dev pointer used here, points to same value.

pci_get_drvdata() and pci_set_drvdata() invoke dev_get_drvdata() and
dev_set_drvdata() respectively. And they do nothing else. Seems like additional
unnecessary function calls and operations.

We can get rid of these PCI helper functions too.
> >  	instance->instancet->disable_intr(instance);
> >  
> >  	megasas_destroy_irqs(instance);
> > @@ -7587,48 +7583,28 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> >  	if (instance->msix_vectors)
> >  		pci_free_irq_vectors(instance->pdev);
> >  
> > -	pci_save_state(pdev);
> > -	pci_disable_device(pdev);
> > -
> > -	pci_set_power_state(pdev, pci_choose_state(pdev, state));
> > -
> >  	return 0;
> >  }
> >  
> >  /**
> >   * megasas_resume-      driver resume entry point
> > - * @pdev:               PCI device structure
> > + * @dev:              Device structure
> >   */
> > -static int
> > -megasas_resume(struct pci_dev *pdev)
> > +static int __maybe_unused
> > +megasas_resume(struct device *dev)
> >  {
> >  	int rval;
> >  	struct Scsi_Host *host;
> > -	struct megasas_instance *instance;
> > +	struct megasas_instance *instance = dev_get_drvdata(dev);
> >  	u32 status_reg;
> >  
> > -	instance = pci_get_drvdata(pdev);
> > -
> >  	if (!instance)
> >  		return 0;
> >  
> >  	host = instance->host;
> > -	pci_set_power_state(pdev, PCI_D0);
> > -	pci_enable_wake(pdev, PCI_D0, 0);
> > -	pci_restore_state(pdev);
> > +	device_wakeup_disable(dev);
> 
> Shouldn't there be a corresponding device_wakeup_enable() or similar
> elsewhere?
> 
> Maybe the fact that megasas disables wakeup but never enables it is a
> latent bug?
> 
Yeah, I guess I sent this patch a lot earlier than we figured this out and
didn't notice it later. I will send v3 for it.
> > -	dev_info(&pdev->dev, "%s is called\n", __func__);
> > -	/*
> > -	 * PCI prepping: enable device set bus mastering and dma mask
> > -	 */
> > -	rval = pci_enable_device_mem(pdev);
> > -
> > -	if (rval) {
> > -		dev_err(&pdev->dev, "Enable device failed\n");
> > -		return rval;
> > -	}
> > -
> > -	pci_set_master(pdev);
> > +	dev_info(dev, "%s is called\n", __func__);
> >  
> >  	/*
> >  	 * We expect the FW state to be READY
> > @@ -7754,14 +7730,8 @@ megasas_resume(struct pci_dev *pdev)
> >  fail_set_dma_mask:
> >  fail_ready_state:
> >  
> > -	pci_disable_device(pdev);
> > -
> >  	return -ENODEV;
> >  }
> > -#else
> > -#define megasas_suspend	NULL
> > -#define megasas_resume	NULL
> > -#endif
> >  
> >  static inline int
> >  megasas_wait_for_adapter_operational(struct megasas_instance *instance)
> > @@ -7931,7 +7901,7 @@ static void megasas_detach_one(struct pci_dev *pdev)
> >  
> >  /**
> >   * megasas_shutdown -	Shutdown entry point
> > - * @device:		Generic device structure
> > + * @pdev:		PCI device structure
> 
> Looks like an unrelated typo fix?  I would put this in a separate
> patch.
> 
Okay.
> >   */
> >  static void megasas_shutdown(struct pci_dev *pdev)
> >  {
> > @@ -8508,6 +8478,8 @@ static const struct file_operations megasas_mgmt_fops = {
> >  	.llseek = noop_llseek,
> >  };
> >  
> > +static SIMPLE_DEV_PM_OPS(megasas_pm_ops, megasas_suspend, megasas_resume);
> > +
> >  /*
> >   * PCI hotplug support registration structure
> >   */
> > @@ -8517,8 +8489,7 @@ static struct pci_driver megasas_pci_driver = {
> >  	.id_table = megasas_pci_table,
> >  	.probe = megasas_probe_one,
> >  	.remove = megasas_detach_one,
> > -	.suspend = megasas_suspend,
> > -	.resume = megasas_resume,
> > +	.driver.pm = &megasas_pm_ops,
> >  	.shutdown = megasas_shutdown,
> >  };
> >  
> > -- 
> > 2.27.0
> > 

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-09-09 10:03     ` Vaibhav Gupta
@ 2020-09-09 13:23       ` Bjorn Helgaas
  0 siblings, 0 replies; 41+ messages in thread
From: Bjorn Helgaas @ 2020-09-09 13:23 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang, Shuah Khan, linux-kernel,
	linux-kernel-mentees, linux-scsi, esc.storagedev,
	megaraidlinux.pdl, MPT-FusionLinux.pdl

On Wed, Sep 09, 2020 at 03:33:15PM +0530, Vaibhav Gupta wrote:
> On Tue, Sep 08, 2020 at 12:32:09PM -0500, Bjorn Helgaas wrote:
> > On Mon, Jul 20, 2020 at 07:04:14PM +0530, Vaibhav Gupta wrote:
> > > With legacy PM hooks, it was the responsibility of a driver to manage PCI
> > > states and also the device's power state. The generic approach is to let
> > > the PCI core handle the work.
> > > 
> > > PCI core passes "struct device*" as an argument to the .suspend() and
> > > .resume() callbacks. As the .suspend() work with "struct instance*",
> > > extract it from "struct device*" using dev_get_drv_data().
> > > 
> > > Driver was also using PCI helper functions like pci_save/restore_state(),
> > > pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> > > They should not be invoked by the driver.
> > > 
> > > Compile-tested only.
> > > 
> > > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> > > ---
> > >  drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
> > >  1 file changed, 16 insertions(+), 45 deletions(-)
> > > 
> > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > index 00668335c2af..4a6ee7778977 100644
> > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > @@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
> > >  	megasas_return_cmd(instance, cmd);
> > >  }
> > >  
> > > -#ifdef CONFIG_PM
> > >  /**
> > >   * megasas_suspend -	driver suspend entry point
> > > - * @pdev:		PCI device structure
> > > - * @state:		PCI power state to suspend routine
> > > + * @dev:		Device structure
> > >   */
> > > -static int
> > > -megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> > > +static int __maybe_unused
> > > +megasas_suspend(struct device *dev)
> > >  {
> > > -	struct megasas_instance *instance;
> > > -
> > > -	instance = pci_get_drvdata(pdev);
> > > +	struct megasas_instance *instance = dev_get_drvdata(dev);
> > >  
> > >  	if (!instance)
> > >  		return 0;
> > >  
> > >  	instance->unload = 1;
> > >  
> > > -	dev_info(&pdev->dev, "%s is called\n", __func__);
> > > +	dev_info(dev, "%s is called\n", __func__);
> > >  
> > >  	/* Shutdown SR-IOV heartbeat timer */
> > >  	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
> > > @@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> > >  
> > >  	tasklet_kill(&instance->isr_tasklet);
> > >  
> > > -	pci_set_drvdata(instance->pdev, instance);
> > > +	dev_set_drvdata(dev, instance);
> > 
> > It *might* be correct to replace "instance->pdev" with "dev", but it's
> > not obvious and deserves some explanation.  It's true that you can
> > replace &pdev->dev with dev, but I don't know anything about
> > instance->dev.

Sorry, I meant "instance->pdev" here.

> > I don't think this change is actually necessary, is it?
> > "instance->pdev" is still a pci_dev pointer, so pci_set_drvdata()
> > should work fine. ...
> > 
> There is no instance->dev . The 'dev' passed dev_set_drvdata() is
> same &pdev->dev. 

Yes, it's true that "dev" here is the same as the "&pdev->dev" we had
previously.  But we passed "instance->pdev" (not "pdev") to
pci_set_drvdata().  So the question is whether instance->pdev->dev ==
dev.

They *might* be the same, but I don't think it's obvious.

> The dev pointer used here, points to same value.
> 
> pci_get_drvdata() and pci_set_drvdata() invoke dev_get_drvdata() and
> dev_set_drvdata() respectively. And they do nothing else. Seems like
> additional unnecessary function calls and operations.

^ permalink raw reply	[flat|nested] 41+ messages in thread

* Re: [PATCH v2 01/15] scsi: megaraid_sas: use generic power management
  2020-09-09 15:20 [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
@ 2020-09-09 16:21 ` Bjorn Helgaas
  0 siblings, 0 replies; 41+ messages in thread
From: Bjorn Helgaas @ 2020-09-09 16:21 UTC (permalink / raw)
  To: Vaibhav Gupta
  Cc: Bjorn Helgaas, Bjorn Helgaas, Vaibhav Gupta, Adam Radford,
	James E.J. Bottomley, Martin K. Petersen,
	Adaptec OEM Raid Solutions, Hannes Reinecke, Bradley Grove,
	John Garry, Don Brace, James Smart, Dick Kennedy, Kashyap Desai,
	Sumit Saxena, Shivasharan S, Sathya Prakash, Sreekanth Reddy,
	Suganath Prabu Subramani, Jack Wang, Shuah Khan, linux-kernel,
	linux-kernel-mentees, linux-scsi, esc.storagedev,
	megaraidlinux.pdl, MPT-FusionLinux.pdl

On Wed, Sep 09, 2020 at 08:50:58PM +0530, Vaibhav Gupta wrote:
> On Wed, Sep 09, 2020 at 08:23:32AM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 09, 2020 at 03:33:15PM +0530, Vaibhav Gupta wrote:
> > > On Tue, Sep 08, 2020 at 12:32:09PM -0500, Bjorn Helgaas wrote:
> > > > On Mon, Jul 20, 2020 at 07:04:14PM +0530, Vaibhav Gupta wrote:
> > > > > With legacy PM hooks, it was the responsibility of a driver to manage PCI
> > > > > states and also the device's power state. The generic approach is to let
> > > > > the PCI core handle the work.
> > > > > 
> > > > > PCI core passes "struct device*" as an argument to the .suspend() and
> > > > > .resume() callbacks. As the .suspend() work with "struct instance*",
> > > > > extract it from "struct device*" using dev_get_drv_data().
> > > > > 
> > > > > Driver was also using PCI helper functions like pci_save/restore_state(),
> > > > > pci_disable/enable_device(), pci_set_power_state() and pci_enable_wake().
> > > > > They should not be invoked by the driver.
> > > > > 
> > > > > Compile-tested only.
> > > > > 
> > > > > Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> > > > > ---
> > > > >  drivers/scsi/megaraid/megaraid_sas_base.c | 61 ++++++-----------------
> > > > >  1 file changed, 16 insertions(+), 45 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > > > index 00668335c2af..4a6ee7778977 100644
> > > > > --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> > > > > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> > > > > @@ -7539,25 +7539,21 @@ static void megasas_shutdown_controller(struct megasas_instance *instance,
> > > > >  	megasas_return_cmd(instance, cmd);
> > > > >  }
> > > > >  
> > > > > -#ifdef CONFIG_PM
> > > > >  /**
> > > > >   * megasas_suspend -	driver suspend entry point
> > > > > - * @pdev:		PCI device structure
> > > > > - * @state:		PCI power state to suspend routine
> > > > > + * @dev:		Device structure
> > > > >   */
> > > > > -static int
> > > > > -megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> > > > > +static int __maybe_unused
> > > > > +megasas_suspend(struct device *dev)
> > > > >  {
> > > > > -	struct megasas_instance *instance;
> > > > > -
> > > > > -	instance = pci_get_drvdata(pdev);
> > > > > +	struct megasas_instance *instance = dev_get_drvdata(dev);
> > > > >  
> > > > >  	if (!instance)
> > > > >  		return 0;
> > > > >  
> > > > >  	instance->unload = 1;
> > > > >  
> > > > > -	dev_info(&pdev->dev, "%s is called\n", __func__);
> > > > > +	dev_info(dev, "%s is called\n", __func__);
> > > > >  
> > > > >  	/* Shutdown SR-IOV heartbeat timer */
> > > > >  	if (instance->requestorId && !instance->skip_heartbeat_timer_del)
> > > > > @@ -7579,7 +7575,7 @@ megasas_suspend(struct pci_dev *pdev, pm_message_t state)
> > > > >  
> > > > >  	tasklet_kill(&instance->isr_tasklet);
> > > > >  
> > > > > -	pci_set_drvdata(instance->pdev, instance);
> > > > > +	dev_set_drvdata(dev, instance);
> > > > 
> > > > It *might* be correct to replace "instance->pdev" with "dev", but it's
> > > > not obvious and deserves some explanation.  It's true that you can
> > > > replace &pdev->dev with dev, but I don't know anything about
> > > > instance->dev.
> > 
> > Sorry, I meant "instance->pdev" here.
> > 
> > > > I don't think this change is actually necessary, is it?
> > > > "instance->pdev" is still a pci_dev pointer, so pci_set_drvdata()
> > > > should work fine. ...
> > > > 
> > > There is no instance->dev . The 'dev' passed dev_set_drvdata() is
> > > same &pdev->dev. 
> > 
> > Yes, it's true that "dev" here is the same as the "&pdev->dev" we had
> > previously.  But we passed "instance->pdev" (not "pdev") to
> > pci_set_drvdata().  So the question is whether instance->pdev->dev ==
> > dev.
> > 
> > They *might* be the same, but I don't think it's obvious.
> > 
> Yes, they are same.
> driver/pci/pci-driver.c :
> 'dev' is passed as parameter to both pci_device_probe() and pci_pm_suspend()
> From 'dev', pci_device_probe() extracts "struct pci_dev*" and passes it to the
> probe callback of this driver.
> 
> In the proble function - megasas_probe_one()
> :7347	instance->pdev = pdev;
> :7386	pci_set_drvdata(pdev, instance);
> 
> The proble function is using "struct pci_dev*" variable "pdev" provided by core
> and same we replaced &pdev->dev with "struct device *dev".
> 
> So the instance->pdev->dev and 'dev' can only differ if 'dev' passed to
> pci_device_probe() and pci_pm_suspend() are different.

OK.  I think this requires too much analysis for this particular
patch, which is really about deprecating pci_driver.suspend and
.resume.  We want patches to be easily verifiable with a minimum of
extra research.

I would completely support a separate patch to clean this up, though.
The *ideal* thing would be to get rid of the set_drvdata() completely
from the suspend routine.  Doing it in the probe routine should be
enough.

> > > The dev pointer used here, points to same value.
> > > 
> > > pci_get_drvdata() and pci_set_drvdata() invoke dev_get_drvdata() and
> > > dev_set_drvdata() respectively. And they do nothing else. Seems like
> > > additional unnecessary function calls and operations.

^ permalink raw reply	[flat|nested] 41+ messages in thread

end of thread, back to index

Thread overview: 41+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 13:34 [PATCH v2 00/15] scsi: use generic power management Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
2020-09-08 16:57   ` Vaibhav Gupta
2020-09-08 17:32   ` Bjorn Helgaas
2020-09-09 10:03     ` Vaibhav Gupta
2020-09-09 13:23       ` Bjorn Helgaas
2020-07-20 13:34 ` [PATCH v2 02/15] scsi: aacraid: " Vaibhav Gupta
2020-07-23  6:58   ` Balsundar.P
2020-09-08 16:58   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 03/15] scsi: aic7xxx: " Vaibhav Gupta
2020-09-08 16:59   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 04/15] scsi: aic79xx: " Vaibhav Gupta
2020-09-08 16:59   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 05/15] scsi: arcmsr: " Vaibhav Gupta
2020-09-08 17:00   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 06/15] scsi: esas2r: " Vaibhav Gupta
2020-09-08 17:01   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 07/15] scsi: hisi_sas_v3_hw: " Vaibhav Gupta
2020-07-21  1:36   ` chenxiang (M)
2020-09-08 17:02   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 08/15] scsi: mpt3sas_scsih: " Vaibhav Gupta
2020-09-08 17:03   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 09/15] scsi: lpfc: " Vaibhav Gupta
2020-09-08 17:03   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 10/15] scsi: pm_8001: " Vaibhav Gupta
2020-07-23  7:02   ` Jinpu Wang
2020-09-08 17:04   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 11/15] scsi: hpsa: " Vaibhav Gupta
2020-07-20 22:23   ` Don.Brace
2020-09-08 17:05   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 12/15] scsi: 3w-9xxx: " Vaibhav Gupta
2020-09-08 17:05   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 13/15] scsi: 3w-sas: " Vaibhav Gupta
2020-09-08 17:06   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 14/15] scsi: mvumi: " Vaibhav Gupta
2020-09-08 17:07   ` Vaibhav Gupta
2020-07-20 13:34 ` [PATCH v2 15/15] scsi: pmcraid: " Vaibhav Gupta
2020-09-08 17:08   ` Vaibhav Gupta
2020-08-17  8:16 ` [PATCH v2 00/15] scsi: " Vaibhav Gupta
2020-09-08 16:54 ` Vaibhav Gupta
2020-09-09 15:20 [PATCH v2 01/15] scsi: megaraid_sas: " Vaibhav Gupta
2020-09-09 16:21 ` Bjorn Helgaas

Linux-SCSI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-scsi/0 linux-scsi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-scsi linux-scsi/ https://lore.kernel.org/linux-scsi \
		linux-scsi@vger.kernel.org
	public-inbox-index linux-scsi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-scsi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git