Re: [PATCH v1 4/5] misc/phantom.c: use generic power management

From: Jiri Slaby <jirislaby@kernel.org>
To: Vaibhav Gupta <vaibhavgupta40@gmail.com>,
	Bjorn Helgaas <helgaas@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>,
	bjorn@helgaas.com, Vaibhav Gupta <vaibhav.varodek@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Alex Dubov <oakad@yahoo.com>
Cc: linux-kernel@vger.kernel.org,
	linux-kernel-mentees@lists.linuxfoundation.org,
	skhan@linuxfoundation.org
Subject: Re: [PATCH v1 4/5] misc/phantom.c: use generic power management
Date: Tue, 30 Jun 2020 10:19:42 +0200	[thread overview]
Message-ID: <164d7366-27f5-c764-3122-5c4a0239e66f@kernel.org> (raw)
In-Reply-To: <20200629081531.214734-5-vaibhavgupta40@gmail.com>

On 29. 06. 20, 10:15, Vaibhav Gupta wrote:
> With the support of generic PM callbacks, drivers no longer need to use
> legacy .suspend() and .resume() in which they had to maintain PCI states
> changes and device's power state themselves. All required operations are
> done by PCI core.
> 
> Driver needs to do only device-specific operations.
> 
> Compile-tested only.
> 
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>

From the driver's POV:
Reviewed-by: Jiri Slaby <jirislaby@kernel.org>

> ---
>  drivers/misc/phantom.c | 20 ++++++++------------
>  1 file changed, 8 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/misc/phantom.c b/drivers/misc/phantom.c
> index 6a5ed0e25ff1..ce72e46a2e73 100644
> --- a/drivers/misc/phantom.c
> +++ b/drivers/misc/phantom.c
> @@ -457,31 +457,26 @@ static void phantom_remove(struct pci_dev *pdev)
>  	pci_disable_device(pdev);
>  }
>  
> -#ifdef CONFIG_PM
> -static int phantom_suspend(struct pci_dev *pdev, pm_message_t state)
> +static int __maybe_unused phantom_suspend(struct device *dev_d)
>  {
> -	struct phantom_device *dev = pci_get_drvdata(pdev);
> +	struct phantom_device *dev = dev_get_drvdata(dev_d);
>  
>  	iowrite32(0, dev->caddr + PHN_IRQCTL);
>  	ioread32(dev->caddr + PHN_IRQCTL); /* PCI posting */
>  
> -	synchronize_irq(pdev->irq);
> +	synchronize_irq(to_pci_dev(dev_d)->irq);
>  
>  	return 0;
>  }
>  
> -static int phantom_resume(struct pci_dev *pdev)
> +static int __maybe_unused phantom_resume(struct device *dev_d)
>  {
> -	struct phantom_device *dev = pci_get_drvdata(pdev);
> +	struct phantom_device *dev = dev_get_drvdata(dev_d);
>  
>  	iowrite32(0, dev->caddr + PHN_IRQCTL);
>  
>  	return 0;
>  }
> -#else
> -#define phantom_suspend	NULL
> -#define phantom_resume	NULL
> -#endif
>  
>  static struct pci_device_id phantom_pci_tbl[] = {
>  	{ .vendor = PCI_VENDOR_ID_PLX, .device = PCI_DEVICE_ID_PLX_9050,
> @@ -491,13 +486,14 @@ static struct pci_device_id phantom_pci_tbl[] = {
>  };
>  MODULE_DEVICE_TABLE(pci, phantom_pci_tbl);
>  
> +static SIMPLE_DEV_PM_OPS(phantom_pm_ops, phantom_suspend, phantom_resume);
> +
>  static struct pci_driver phantom_pci_driver = {
>  	.name = "phantom",
>  	.id_table = phantom_pci_tbl,
>  	.probe = phantom_probe,
>  	.remove = phantom_remove,
> -	.suspend = phantom_suspend,
> -	.resume = phantom_resume
> +	.driver.pm = &phantom_pm_ops,
>  };
>  
>  static CLASS_ATTR_STRING(version, 0444, PHANTOM_VERSION);
> 


-- 
js