Re: [PATCH v8 4/4] USB: Pass PMSG_POWEROFF event to suspend_common()

Bjorn Helgaas
2025-09-18T19:55:10Z

On Wed, Sep 17, 2025 at 10:44:27PM -0500, Mario Limonciello (AMD) wrote:
> suspend_common() passes the a PM message indicating what type of event
> is occurring. Add a new callback hcd_pci_poweroff() which will
> determine if target state is power off and pass PMSG_POWEROFF as the
> message.

Something is missing in "passes the a".

> There are no functional changes in this patch.

Maybe so, but the .poweroff() path previously called
"suspend_common(dev, PMSG_SUSPEND)" and now may call
"suspend_common(dev, PMSG_POWEROFF)", so it's not completely obvious
that this is a functional no-op.

It seems sort of weird that apparently we can call .poweroff() for
either a "suspend" or a "power-off" event.

Maybe it would be helpful to explain how we get to .poweroff() when
system_state is something other than SYSTEM_POWER_OFF, and what that
means?

> +static int hcd_pci_poweroff(struct device *dev)
> +{
> + if (system_state == SYSTEM_POWER_OFF)
> + return suspend_common(dev, PMSG_POWEROFF);
> + return suspend_common(dev, PMSG_SUSPEND);
> +}
> +
> static int hcd_pci_suspend_noirq(struct device *dev)
> {
> struct pci_dev *pci_dev = to_pci_dev(dev);
> @@ -602,6 +610,7 @@ static int hcd_pci_restore(struct device *dev)
> #define hcd_pci_suspend NULL
> #define hcd_pci_freeze NULL
> #define hcd_pci_suspend_noirq NULL
> +#define hcd_pci_poweroff NULL
> #define hcd_pci_poweroff_late NULL
> #define hcd_pci_resume_noirq NULL
> #define hcd_pci_resume NULL
> @@ -639,7 +648,7 @@ const struct dev_pm_ops usb_hcd_pci_pm_ops = {
> .freeze_noirq = check_root_hub_suspended,
> .thaw_noirq = NULL,
> .thaw = hcd_pci_resume,
> - .poweroff = hcd_pci_suspend,
> + .poweroff = hcd_pci_poweroff,
> .poweroff_late = hcd_pci_poweroff_late,
> .poweroff_noirq = hcd_pci_suspend_noirq,
> .restore_noirq = hcd_pci_resume_noirq,
> --
> 2.51.0
>