* [PATCH v2] firewire: ohci: convert to generic power management
@ 2022-10-25 21:25 Bjorn Helgaas
2022-10-25 21:41 ` Bjorn Helgaas
2022-10-27 6:03 ` Lukas Wunner
0 siblings, 2 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2022-10-25 21:25 UTC (permalink / raw)
To: Stefan Richter
Cc: Vaibhav Gupta, Rafael J . Wysocki, linux1394-devel, linux-pci,
linux-pm, linux-kernel, Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert ohci from legacy PCI power management to the generic power
management framework.
Previously, ohci used legacy PCI power management, and pci_suspend() and
pci_resume() were responsible for both device-specific things and generic
PCI things:
pci_suspend
software_reset() <-- device-specific
pci_save_state(pdev) <-- generic PCI
pci_set_power_state(pdev, pci_choose_state(pdev, state)) <-- generic PCI
pmac_ohci_off(pdev) <-- device-specific
pci_resume
pmac_ohci_on(pdev) <-- device-specific
pci_set_power_state(pdev, PCI_D0) <-- generic PCI
pci_restore_state(pdev) <-- generic PCI
ohci_enable() <-- device-specific
...
With generic power management, the PCI bus PM methods do the generic PCI
things, and the driver needs only the device-specific part, i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
pci_suspend # dev->driver->pm->suspend
software_reset() <-- device-specific
pmac_ohci_off(pdev) <-- device-specific (DIFFERENT ORDER)
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
pci_resume # dev->driver->pm->resume
pmac_ohci_on(pdev) <-- device-specific (DIFFERENT ORDER)
ohci_enable() <-- device-specific
...
N.B. This changes the order of pmac_ohci_off() and pmac_ohci_on().
Previously, pmac_ohci_off() was called *after* pci_save_state() and
pci_set_power_state(), and this change calls it *before*.
Similarly, pmac_ohci_on() was previously called *before*
pci_set_power_state() and pci_restore_state() and this change calls it
*after*.
The code in pmac_ohci_on() and pmac_ohci_off() was added by ea8d006b91ac
("firewire: fw-ohci: PPC PMac platform code").
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
v1 was posted at https://lore.kernel.org/r/20200720150715.624520-1-vaibhavgupta40@gmail.com
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.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] firewire: ohci: convert to generic power management
2022-10-25 21:25 [PATCH v2] firewire: ohci: convert to generic power management Bjorn Helgaas
@ 2022-10-25 21:41 ` Bjorn Helgaas
2022-10-27 6:03 ` Lukas Wunner
1 sibling, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2022-10-25 21:41 UTC (permalink / raw)
To: Stefan Richter
Cc: Vaibhav Gupta, Rafael J . Wysocki, linux1394-devel, linux-pci,
linux-pm, linux-kernel, Bjorn Helgaas
On Tue, Oct 25, 2022 at 04:25:21PM -0500, Bjorn Helgaas wrote:
> From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
>
> 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@gmail.com
>
> 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@kernel.org/,
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] firewire: ohci: convert to generic power management
2022-10-25 21:25 [PATCH v2] firewire: ohci: convert to generic power management Bjorn Helgaas
2022-10-25 21:41 ` Bjorn Helgaas
@ 2022-10-27 6:03 ` Lukas Wunner
2022-10-27 21:36 ` Bjorn Helgaas
1 sibling, 1 reply; 6+ messages in thread
From: Lukas Wunner @ 2022-10-27 6:03 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Stefan Richter, Vaibhav Gupta, Rafael J . Wysocki,
linux1394-devel, linux-pci, linux-pm, linux-kernel, Bjorn Helgaas
On Tue, Oct 25, 2022 at 04:25:21PM -0500, Bjorn Helgaas wrote:
> N.B. This changes the order of pmac_ohci_off() and pmac_ohci_on().
> Previously, pmac_ohci_off() was called *after* pci_save_state() and
> pci_set_power_state(), and this change calls it *before*.
>
> Similarly, pmac_ohci_on() was previously called *before*
> pci_set_power_state() and pci_restore_state() and this change calls it
> *after*.
Seems likely the ordering change may break things.
pmac_ohci_on/off() toggles PMAC_FTR_1394_ENABLE, which is defined as:
* enable/disable the firewire cell of an uninorth ASIC.
It sounds like it will cut power to the firewire controller and I'd
expect that pci_save_state() will then not be able to access config
space.
The only way to make this work is to define a struct dev_pm_domain
whose ->suspend_noirq callback first invokes the pci_bus_type
->suspend_noirq callback and then cuts power to the firewire cell
by calling pmac_ohci_off().
I've done something like this for Thunderbolt power management on
x86 Macs a few years back but didn't get around to upstream it so far:
https://github.com/l1k/linux/commit/4db7f0b1f5c9
Thanks,
Lukas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] firewire: ohci: convert to generic power management
2022-10-27 6:03 ` Lukas Wunner
@ 2022-10-27 21:36 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2022-10-27 21:36 UTC (permalink / raw)
To: Lukas Wunner
Cc: Stefan Richter, Vaibhav Gupta, Rafael J . Wysocki,
linux1394-devel, linux-pci, linux-pm, linux-kernel, Bjorn Helgaas
On Thu, Oct 27, 2022 at 08:03:42AM +0200, Lukas Wunner wrote:
> On Tue, Oct 25, 2022 at 04:25:21PM -0500, Bjorn Helgaas wrote:
> > N.B. This changes the order of pmac_ohci_off() and pmac_ohci_on().
> > Previously, pmac_ohci_off() was called *after* pci_save_state() and
> > pci_set_power_state(), and this change calls it *before*.
> >
> > Similarly, pmac_ohci_on() was previously called *before*
> > pci_set_power_state() and pci_restore_state() and this change calls it
> > *after*.
>
> Seems likely the ordering change may break things.
>
> pmac_ohci_on/off() toggles PMAC_FTR_1394_ENABLE, which is defined as:
>
> * enable/disable the firewire cell of an uninorth ASIC.
>
> It sounds like it will cut power to the firewire controller and I'd
> expect that pci_save_state() will then not be able to access config
> space.
Yeah, definitely a risk, so I won't merge this myself since I have no
insight or way to test it.
> The only way to make this work is to define a struct dev_pm_domain
> whose ->suspend_noirq callback first invokes the pci_bus_type
> ->suspend_noirq callback and then cuts power to the firewire cell
> by calling pmac_ohci_off().
>
> I've done something like this for Thunderbolt power management on
> x86 Macs a few years back but didn't get around to upstream it so far:
>
> https://github.com/l1k/linux/commit/4db7f0b1f5c9
Wow, that's some impressive reverse engineering and work!
We're not quite there yet, but if we ever get to the point where this
driver is the only thing preventing us from removing the PCI legacy PM
hooks, I think I'd be inclined to just sacrifice PM completely for
this driver. I can't really see putting in the kind of work you did
for Thunderbolt.
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] firewire: ohci: Convert to generic power management
@ 2022-06-07 21:21 Bjorn Helgaas
2022-06-08 14:24 ` Bjorn Helgaas
0 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2022-06-07 21:21 UTC (permalink / raw)
To: Stefan Richter
Cc: Vaibhav Gupta, Rafael J . Wysocki, linux1394-devel, linux-pm,
linux-pci, linux-kernel, Bjorn Helgaas
From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Convert firewire-ohci from legacy PCI power management to the generic power
management framework.
Previously firewire-ohci used legacy PCI power management, and
pci_suspend() and pci_resume() were responsible for both device-specific
things and generic PCI things like saving and restoring config space and
managing power state:
pci_suspend
software_reset <-- device-specific
pci_save_state <-- generic PCI
pci_set_power_state <-- generic PCI
pmac_ohci_off <-- device-specific
pci_resume
pmac_ohci_on <-- device-specific
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
pci_enable_device <-- generic PCI
ohci_enable <-- device-specific
Convert to generic power management where the PCI bus PM methods do the
generic PCI things, and the driver needs only the device-specific part,
i.e.,
suspend_devices_and_enter
dpm_suspend_start(PMSG_SUSPEND)
pci_pm_suspend # PCI bus .suspend() method
pci_suspend # driver->pm->suspend
software_reset <-- device-specific
pmac_ohci_off <-- device-specific
suspend_enter
dpm_suspend_noirq(PMSG_SUSPEND)
pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
pci_save_state <-- generic PCI
pci_prepare_to_sleep <-- generic PCI
pci_set_power_state
...
dpm_resume_end(PMSG_RESUME)
pci_pm_resume # PCI bus .resume() method
pci_restore_standard_config
pci_set_power_state(PCI_D0) <-- generic PCI
pci_restore_state <-- generic PCI
pci_resume # driver->pm->resume
pmac_ohci_on <-- device-specific
ohci_enable <-- device-specific
[bhelgaas: commit log]
Link: https://lore.kernel.org/r/20200720150715.624520-1-vaibhavgupta40@gmail.com
Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
---
drivers/firewire/ohci.c | 35 ++++++++---------------------------
1 file changed, 8 insertions(+), 27 deletions(-)
diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
index 17c9d825188b..aee705132330 100644
--- a/drivers/firewire/ohci.c
+++ b/drivers/firewire/ohci.c
@@ -3165,8 +3165,7 @@ 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)
+static void __maybe_unused ohci_resume_iso_dma(struct fw_ohci *ohci)
{
int i;
struct iso_context *ctx;
@@ -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 __maybe_unused pci_suspend(struct device *devp)
{
+ struct pci_dev *dev = to_pci_dev(devp);
struct fw_ohci *ohci = pci_get_drvdata(dev);
- int err;
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);
return 0;
}
-static int pci_resume(struct pci_dev *dev)
+static int __maybe_unused pci_resume(struct device *devp)
{
+ struct pci_dev *dev = to_pci_dev(devp);
struct fw_ohci *ohci = pci_get_drvdata(dev);
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;
- }
/* 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 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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2] firewire: ohci: Convert to generic power management
2022-06-07 21:21 [PATCH v2] firewire: ohci: Convert " Bjorn Helgaas
@ 2022-06-08 14:24 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2022-06-08 14:24 UTC (permalink / raw)
To: Stefan Richter
Cc: Vaibhav Gupta, Rafael J . Wysocki, linux1394-devel, linux-pm,
linux-pci, linux-kernel, Bjorn Helgaas
On Tue, Jun 07, 2022 at 04:21:57PM -0500, Bjorn Helgaas wrote:
> From: Vaibhav Gupta <vaibhavgupta40@gmail.com>
>
> Convert firewire-ohci from legacy PCI power management to the generic power
> management framework.
>
> Previously firewire-ohci used legacy PCI power management, and
> pci_suspend() and pci_resume() were responsible for both device-specific
> things and generic PCI things like saving and restoring config space and
> managing power state:
>
> pci_suspend
> software_reset <-- device-specific
> pci_save_state <-- generic PCI
> pci_set_power_state <-- generic PCI
> pmac_ohci_off <-- device-specific
>
> pci_resume
> pmac_ohci_on <-- device-specific
> pci_set_power_state(PCI_D0) <-- generic PCI
> pci_restore_state <-- generic PCI
> pci_enable_device <-- generic PCI
> ohci_enable <-- device-specific
>
> Convert to generic power management where the PCI bus PM methods do the
> generic PCI things, and the driver needs only the device-specific part,
> i.e.,
>
> suspend_devices_and_enter
> dpm_suspend_start(PMSG_SUSPEND)
> pci_pm_suspend # PCI bus .suspend() method
> pci_suspend # driver->pm->suspend
> software_reset <-- device-specific
> pmac_ohci_off <-- device-specific
> suspend_enter
> dpm_suspend_noirq(PMSG_SUSPEND)
> pci_pm_suspend_noirq # PCI bus .suspend_noirq() method
> pci_save_state <-- generic PCI
> pci_prepare_to_sleep <-- generic PCI
> pci_set_power_state
> ...
> dpm_resume_end(PMSG_RESUME)
> pci_pm_resume # PCI bus .resume() method
> pci_restore_standard_config
> pci_set_power_state(PCI_D0) <-- generic PCI
> pci_restore_state <-- generic PCI
> pci_resume # driver->pm->resume
> pmac_ohci_on <-- device-specific
> ohci_enable <-- device-specific
I want to point out that this changes the ordering of pmac_ohci_off()
with respect to pci_save_state() and pci_set_power_state().
Previously we did those as:
pci_save_state
pci_set_power_state
pmac_ohci_off
and after this patch it would be:
pmac_ohci_off
pci_save_state
pci_set_power_state
The ordering of pmac_ohci_on() in the resume path is similarly
changed.
I don't know what pmac_ohci_off() and pmac_ohci_on() do, so I don't
know whether this is an issue. In general, drivers don't need to do
anything after their device is put in D3hot, but maybe firewire-ohci
is unique in this way.
> [bhelgaas: commit log]
> Link: https://lore.kernel.org/r/20200720150715.624520-1-vaibhavgupta40@gmail.com
> Signed-off-by: Vaibhav Gupta <vaibhavgupta40@gmail.com>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> ---
> drivers/firewire/ohci.c | 35 ++++++++---------------------------
> 1 file changed, 8 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/firewire/ohci.c b/drivers/firewire/ohci.c
> index 17c9d825188b..aee705132330 100644
> --- a/drivers/firewire/ohci.c
> +++ b/drivers/firewire/ohci.c
> @@ -3165,8 +3165,7 @@ 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)
> +static void __maybe_unused ohci_resume_iso_dma(struct fw_ohci *ohci)
> {
> int i;
> struct iso_context *ctx;
> @@ -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 __maybe_unused pci_suspend(struct device *devp)
> {
> + struct pci_dev *dev = to_pci_dev(devp);
> struct fw_ohci *ohci = pci_get_drvdata(dev);
> - int err;
>
> 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);
>
> return 0;
> }
>
> -static int pci_resume(struct pci_dev *dev)
> +static int __maybe_unused pci_resume(struct device *devp)
> {
> + struct pci_dev *dev = to_pci_dev(devp);
> struct fw_ohci *ohci = pci_get_drvdata(dev);
> 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;
> - }
>
> /* 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 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-10-27 21:36 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-25 21:25 [PATCH v2] firewire: ohci: convert to generic power management Bjorn Helgaas
2022-10-25 21:41 ` Bjorn Helgaas
2022-10-27 6:03 ` Lukas Wunner
2022-10-27 21:36 ` Bjorn Helgaas
-- strict thread matches above, loose matches on Subject: below --
2022-06-07 21:21 [PATCH v2] firewire: ohci: Convert " Bjorn Helgaas
2022-06-08 14:24 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).