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

[Bjorn Helgaas <helgaas@kernel.org>]

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
>