Re: [PATCH v2] firewire: ohci: convert to generic power management

[Bjorn Helgaas]

On Tue, Oct 25, 2022 at 04:25:21PM -0500, Bjorn Helgaas wrote:
> From: Vaibhav Gupta <vaibhavgupta40@xxxxxxxxx>
> 
> Convert ohci from legacy PCI power management to the generic power
> management framework.

> v1 was posted at https://lore.kernel.org/r/20200720150715.624520-1-vaibhavgupta40@xxxxxxxxx
> 
> Changes from v1 to v2:
>   - Convert from SIMPLE_DEV_PM_OPS() (which is deprecated) to
>     DEFINE_SIMPLE_DEV_PM_OPS() and remove __maybe_unused annotations.
>   - Expand commit log.

My mistake: This should have been marked *v3*.  v1 was posted as
listed above.

I posted a v2 in June at https://lore.kernel.org/all/20220607212157.343033-1-helgaas@xxxxxxxxxx/,
which had an expanded commit log and trivial parameter renames.

So this should have been marked v3 and added the
DEFINE_SIMPLE_DEV_PM_OPS() conversion.

Bjorn

>  drivers/firewire/ohci.c | 41 +++++++++++------------------------------
>  1 file changed, 11 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index 17c9d825188b..f3cceca51e59 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -3165,7 +3165,6 @@ static int ohci_set_iso_channels(struct fw_iso_context *base, u64 *channels)
>  	return ret;
>  }
>  
> -#ifdef CONFIG_PM
>  static void ohci_resume_iso_dma(struct fw_ohci *ohci)
>  {
>  	int i;
> @@ -3183,7 +3182,6 @@ static void ohci_resume_iso_dma(struct fw_ohci *ohci)
>  			ohci_start_iso(&ctx->base, 0, ctx->sync, ctx->tags);
>  	}
>  }
> -#endif
>  
>  static int queue_iso_transmit(struct iso_context *ctx,
>  			      struct fw_iso_packet *packet,
> @@ -3789,39 +3787,24 @@ static void pci_remove(struct pci_dev *dev)
>  	dev_notice(&dev->dev, "removed fw-ohci device\n");
>  }
>  
> -#ifdef CONFIG_PM
> -static int pci_suspend(struct pci_dev *dev, pm_message_t state)
> +static int pci_suspend(struct device *dev)
>  {
> -	struct fw_ohci *ohci = pci_get_drvdata(dev);
> -	int err;
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct fw_ohci *ohci = pci_get_drvdata(pdev);
>  
>  	software_reset(ohci);
> -	err = pci_save_state(dev);
> -	if (err) {
> -		ohci_err(ohci, "pci_save_state failed\n");
> -		return err;
> -	}
> -	err = pci_set_power_state(dev, pci_choose_state(dev, state));
> -	if (err)
> -		ohci_err(ohci, "pci_set_power_state failed with %d\n", err);
> -	pmac_ohci_off(dev);
> +	pmac_ohci_off(pdev);
>  
>  	return 0;
>  }
>  
> -static int pci_resume(struct pci_dev *dev)
> +static int pci_resume(struct device *dev)
>  {
> -	struct fw_ohci *ohci = pci_get_drvdata(dev);
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	struct fw_ohci *ohci = pci_get_drvdata(pdev);
>  	int err;
>  
> -	pmac_ohci_on(dev);
> -	pci_set_power_state(dev, PCI_D0);
> -	pci_restore_state(dev);
> -	err = pci_enable_device(dev);
> -	if (err) {
> -		ohci_err(ohci, "pci_enable_device failed\n");
> -		return err;
> -	}
> +	pmac_ohci_on(pdev);
>  
>  	/* Some systems don't setup GUID register on resume from ram  */
>  	if (!reg_read(ohci, OHCI1394_GUIDLo) &&
> @@ -3838,7 +3821,6 @@ static int pci_resume(struct pci_dev *dev)
>  
>  	return 0;
>  }
> -#endif
>  
>  static const struct pci_device_id pci_table[] = {
>  	{ PCI_DEVICE_CLASS(PCI_CLASS_SERIAL_FIREWIRE_OHCI, ~0) },
> @@ -3847,15 +3829,14 @@ static const struct pci_device_id pci_table[] = {
>  
>  MODULE_DEVICE_TABLE(pci, pci_table);
>  
> +static DEFINE_SIMPLE_DEV_PM_OPS(pci_pm_ops, pci_suspend, pci_resume);
> +
>  static struct pci_driver fw_ohci_pci_driver = {
>  	.name		= ohci_driver_name,
>  	.id_table	= pci_table,
>  	.probe		= pci_probe,
>  	.remove		= pci_remove,
> -#ifdef CONFIG_PM
> -	.resume		= pci_resume,
> -	.suspend	= pci_suspend,
> -#endif
> +	.driver.pm	= &pci_pm_ops,
>  };
>  
>  static int __init fw_ohci_init(void)
> -- 
> 2.25.1
>