[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200908170333.GJ9948@gmail.com>
Date: Tue, 8 Sep 2020 22:33:33 +0530
From: Vaibhav Gupta <vaibhavgupta40@...il.com>
To: Bjorn Helgaas <helgaas@...nel.org>,
Bjorn Helgaas <bhelgaas@...gle.com>,
Bjorn Helgaas <bjorn@...gaas.com>,
Vaibhav Gupta <vaibhav.varodek@...il.com>,
Adam Radford <aradford@...il.com>,
"James E.J. Bottomley" <jejb@...ux.ibm.com>,
"Martin K. Petersen" <martin.petersen@...cle.com>,
Adaptec OEM Raid Solutions <aacraid@...rosemi.com>,
Hannes Reinecke <hare@...e.com>,
Bradley Grove <linuxdrivers@...otech.com>,
John Garry <john.garry@...wei.com>,
Don Brace <don.brace@...rosemi.com>,
James Smart <james.smart@...adcom.com>,
Dick Kennedy <dick.kennedy@...adcom.com>,
Kashyap Desai <kashyap.desai@...adcom.com>,
Sumit Saxena <sumit.saxena@...adcom.com>,
Shivasharan S <shivasharan.srikanteshwara@...adcom.com>,
Sathya Prakash <sathya.prakash@...adcom.com>,
Sreekanth Reddy <sreekanth.reddy@...adcom.com>,
Suganath Prabu Subramani
<suganath-prabu.subramani@...adcom.com>,
Jack Wang <jinpu.wang@...ud.ionos.com>
Cc: Shuah Khan <skhan@...uxfoundation.org>,
linux-kernel@...r.kernel.org,
linux-kernel-mentees@...ts.linuxfoundation.org,
linux-scsi@...r.kernel.org, esc.storagedev@...rosemi.com,
megaraidlinux.pdl@...adcom.com, MPT-FusionLinux.pdl@...adcom.com
Subject: Re: [PATCH v2 09/15] scsi: lpfc: use generic power management
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@...il.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
>
.
Powered by blists - more mailing lists