[PATCH v8 0/4] Introduce and plumb PMSG_POWEROFF

[PATCH v8 0/4] Introduce and plumb PMSG_POWEROFF

[Mario Limonciello (AMD)]

I've been working on a series that uses the hibernate flows (S4)
during shutdown (S5) [1], but it's a bit risky because it has changes
all around the kernel.  To mitigate risk Rafael suggested [2] to split
the series into 3 parts across different kernel cycles.

Here is the first part, which just introduces a PMSG_POWEROFF event
and uses it in any driver that manipulates PM events.

There are no functional changes for these changes and this series is
intended for 6.18.

Link: https://lore.kernel.org/linux-pm/20250909191619.2580169-1-superm1@kernel.org/ [1]
Link: https://lore.kernel.org/linux-pm/CAJZ5v0jHKp7c7dSQMZr5tmQOV6=fHOygTf-YG6Gx9YmurA9cTA@mail.gmail.com/ [2]
Mario Limonciello (AMD) (4):
  PM: Introduce new PMSG_POWEROFF event
  scsi: Add PM_EVENT_POWEROFF into suspend callbacks
  usb: sl811-hcd: Add PM_EVENT_POWEROFF into suspend callbacks
  USB: Pass PMSG_POWEROFF event to suspend_common()

 drivers/base/power/main.c    |  7 +++++++
 drivers/scsi/mesh.c          |  1 +
 drivers/scsi/stex.c          |  1 +
 drivers/usb/core/hcd-pci.c   | 11 ++++++++++-
 drivers/usb/host/sl811-hcd.c |  1 +
 include/linux/pm.h           |  3 +++
 include/trace/events/power.h |  3 ++-
 7 files changed, 25 insertions(+), 2 deletions(-)

-- 
2.51.0

[PATCH v8 1/4] PM: Introduce new PMSG_POWEROFF event

[Mario Limonciello (AMD)]

PMSG_POWEROFF will be used for the PM core to allow differentiating between
a hibernation or shutdown sequence when re-using callbacks.

Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v8:
 * Break series into 3 parts
 * Drop PMSG_NO_WAKEUP change
v7:
 * Reword commit
v5:
 * Re-order and split
 * Add tags
v4:
 * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
---
 drivers/base/power/main.c    | 7 +++++++
 include/linux/pm.h           | 3 +++
 include/trace/events/power.h | 3 ++-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index 2ea6e05e6ec9..86661c94e8ce 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -99,6 +99,8 @@ static const char *pm_verb(int event)
 		return "restore";
 	case PM_EVENT_RECOVER:
 		return "recover";
+	case PM_EVENT_POWEROFF:
+		return "poweroff";
 	default:
 		return "(unknown PM event)";
 	}
@@ -369,6 +371,7 @@ static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state)
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
 		return ops->freeze;
+	case PM_EVENT_POWEROFF:
 	case PM_EVENT_HIBERNATE:
 		return ops->poweroff;
 	case PM_EVENT_THAW:
@@ -403,6 +406,7 @@ static pm_callback_t pm_late_early_op(const struct dev_pm_ops *ops,
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
 		return ops->freeze_late;
+	case PM_EVENT_POWEROFF:
 	case PM_EVENT_HIBERNATE:
 		return ops->poweroff_late;
 	case PM_EVENT_THAW:
@@ -437,6 +441,7 @@ static pm_callback_t pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t stat
 	case PM_EVENT_FREEZE:
 	case PM_EVENT_QUIESCE:
 		return ops->freeze_noirq;
+	case PM_EVENT_POWEROFF:
 	case PM_EVENT_HIBERNATE:
 		return ops->poweroff_noirq;
 	case PM_EVENT_THAW:
@@ -1370,6 +1375,8 @@ static pm_message_t resume_event(pm_message_t sleep_state)
 		return PMSG_RECOVER;
 	case PM_EVENT_HIBERNATE:
 		return PMSG_RESTORE;
+	case PM_EVENT_POWEROFF:
+		return PMSG_ON;
 	}
 	return PMSG_ON;
 }
diff --git a/include/linux/pm.h b/include/linux/pm.h
index cc7b2dc28574..d001224c92fd 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -507,6 +507,7 @@ const struct dev_pm_ops name = { \
  * RECOVER	Creation of a hibernation image or restoration of the main
  *		memory contents from a hibernation image has failed, call
  *		->thaw() and ->complete() for all devices.
+ * POWEROFF	System will poweroff, call ->poweroff() for all devices.
  *
  * The following PM_EVENT_ messages are defined for internal use by
  * kernel subsystems.  They are never issued by the PM core.
@@ -537,6 +538,7 @@ const struct dev_pm_ops name = { \
 #define PM_EVENT_USER		0x0100
 #define PM_EVENT_REMOTE		0x0200
 #define PM_EVENT_AUTO		0x0400
+#define PM_EVENT_POWEROFF	0x0800
 
 #define PM_EVENT_SLEEP		(PM_EVENT_SUSPEND | PM_EVENT_HIBERNATE)
 #define PM_EVENT_USER_SUSPEND	(PM_EVENT_USER | PM_EVENT_SUSPEND)
@@ -551,6 +553,7 @@ const struct dev_pm_ops name = { \
 #define PMSG_QUIESCE	((struct pm_message){ .event = PM_EVENT_QUIESCE, })
 #define PMSG_SUSPEND	((struct pm_message){ .event = PM_EVENT_SUSPEND, })
 #define PMSG_HIBERNATE	((struct pm_message){ .event = PM_EVENT_HIBERNATE, })
+#define PMSG_POWEROFF	((struct pm_message){ .event = PM_EVENT_POWEROFF, })
 #define PMSG_RESUME	((struct pm_message){ .event = PM_EVENT_RESUME, })
 #define PMSG_THAW	((struct pm_message){ .event = PM_EVENT_THAW, })
 #define PMSG_RESTORE	((struct pm_message){ .event = PM_EVENT_RESTORE, })
diff --git a/include/trace/events/power.h b/include/trace/events/power.h
index 82904291c2b8..370f8df2fdb4 100644
--- a/include/trace/events/power.h
+++ b/include/trace/events/power.h
@@ -179,7 +179,8 @@ TRACE_EVENT(pstate_sample,
 		{ PM_EVENT_HIBERNATE, "hibernate" }, \
 		{ PM_EVENT_THAW, "thaw" }, \
 		{ PM_EVENT_RESTORE, "restore" }, \
-		{ PM_EVENT_RECOVER, "recover" })
+		{ PM_EVENT_RECOVER, "recover" }, \
+		{ PM_EVENT_POWEROFF, "poweroff" })
 
 DEFINE_EVENT(cpu, cpu_frequency,
 
-- 
2.51.0

Re: [PATCH v8 1/4] PM: Introduce new PMSG_POWEROFF event

[Bjorn Helgaas]

On Wed, Sep 17, 2025 at 10:44:24PM -0500, Mario Limonciello (AMD) wrote:
> PMSG_POWEROFF will be used for the PM core to allow differentiating between
> a hibernation or shutdown sequence when re-using callbacks.

I think it would be useful to say something here about how the
hibernation and shutdown sequences are entered, e.g., what user
commands and syscalls are related?

> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v8:
>  * Break series into 3 parts
>  * Drop PMSG_NO_WAKEUP change
> v7:
>  * Reword commit
> v5:
>  * Re-order and split
>  * Add tags
> v4:
>  * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
> ---
>  drivers/base/power/main.c    | 7 +++++++
>  include/linux/pm.h           | 3 +++
>  include/trace/events/power.h | 3 ++-
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index 2ea6e05e6ec9..86661c94e8ce 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -99,6 +99,8 @@ static const char *pm_verb(int event)
>  		return "restore";
>  	case PM_EVENT_RECOVER:
>  		return "recover";
> +	case PM_EVENT_POWEROFF:
> +		return "poweroff";
>  	default:
>  		return "(unknown PM event)";
>  	}
> @@ -369,6 +371,7 @@ static pm_callback_t pm_op(const struct dev_pm_ops *ops, pm_message_t state)
>  	case PM_EVENT_FREEZE:
>  	case PM_EVENT_QUIESCE:
>  		return ops->freeze;
> +	case PM_EVENT_POWEROFF:
>  	case PM_EVENT_HIBERNATE:
>  		return ops->poweroff;
>  	case PM_EVENT_THAW:
> @@ -403,6 +406,7 @@ static pm_callback_t pm_late_early_op(const struct dev_pm_ops *ops,
>  	case PM_EVENT_FREEZE:
>  	case PM_EVENT_QUIESCE:
>  		return ops->freeze_late;
> +	case PM_EVENT_POWEROFF:
>  	case PM_EVENT_HIBERNATE:
>  		return ops->poweroff_late;
>  	case PM_EVENT_THAW:
> @@ -437,6 +441,7 @@ static pm_callback_t pm_noirq_op(const struct dev_pm_ops *ops, pm_message_t stat
>  	case PM_EVENT_FREEZE:
>  	case PM_EVENT_QUIESCE:
>  		return ops->freeze_noirq;
> +	case PM_EVENT_POWEROFF:
>  	case PM_EVENT_HIBERNATE:
>  		return ops->poweroff_noirq;
>  	case PM_EVENT_THAW:
> @@ -1370,6 +1375,8 @@ static pm_message_t resume_event(pm_message_t sleep_state)
>  		return PMSG_RECOVER;
>  	case PM_EVENT_HIBERNATE:
>  		return PMSG_RESTORE;
> +	case PM_EVENT_POWEROFF:
> +		return PMSG_ON;
>  	}
>  	return PMSG_ON;
>  }
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index cc7b2dc28574..d001224c92fd 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -507,6 +507,7 @@ const struct dev_pm_ops name = { \
>   * RECOVER	Creation of a hibernation image or restoration of the main
>   *		memory contents from a hibernation image has failed, call
>   *		->thaw() and ->complete() for all devices.

Looks like there should be a blank line here to match formatting of
other messages.

> + * POWEROFF	System will poweroff, call ->poweroff() for all devices.
>   *
>   * The following PM_EVENT_ messages are defined for internal use by
>   * kernel subsystems.  They are never issued by the PM core.
> @@ -537,6 +538,7 @@ const struct dev_pm_ops name = { \
>  #define PM_EVENT_USER		0x0100
>  #define PM_EVENT_REMOTE		0x0200
>  #define PM_EVENT_AUTO		0x0400
> +#define PM_EVENT_POWEROFF	0x0800
>  
>  #define PM_EVENT_SLEEP		(PM_EVENT_SUSPEND | PM_EVENT_HIBERNATE)
>  #define PM_EVENT_USER_SUSPEND	(PM_EVENT_USER | PM_EVENT_SUSPEND)
> @@ -551,6 +553,7 @@ const struct dev_pm_ops name = { \
>  #define PMSG_QUIESCE	((struct pm_message){ .event = PM_EVENT_QUIESCE, })
>  #define PMSG_SUSPEND	((struct pm_message){ .event = PM_EVENT_SUSPEND, })
>  #define PMSG_HIBERNATE	((struct pm_message){ .event = PM_EVENT_HIBERNATE, })
> +#define PMSG_POWEROFF	((struct pm_message){ .event = PM_EVENT_POWEROFF, })
>  #define PMSG_RESUME	((struct pm_message){ .event = PM_EVENT_RESUME, })
>  #define PMSG_THAW	((struct pm_message){ .event = PM_EVENT_THAW, })
>  #define PMSG_RESTORE	((struct pm_message){ .event = PM_EVENT_RESTORE, })
> diff --git a/include/trace/events/power.h b/include/trace/events/power.h
> index 82904291c2b8..370f8df2fdb4 100644
> --- a/include/trace/events/power.h
> +++ b/include/trace/events/power.h
> @@ -179,7 +179,8 @@ TRACE_EVENT(pstate_sample,
>  		{ PM_EVENT_HIBERNATE, "hibernate" }, \
>  		{ PM_EVENT_THAW, "thaw" }, \
>  		{ PM_EVENT_RESTORE, "restore" }, \
> -		{ PM_EVENT_RECOVER, "recover" })
> +		{ PM_EVENT_RECOVER, "recover" }, \
> +		{ PM_EVENT_POWEROFF, "poweroff" })
>  
>  DEFINE_EVENT(cpu, cpu_frequency,
>  
> -- 
> 2.51.0
> 

[PATCH v8 2/4] scsi: Add PM_EVENT_POWEROFF into suspend callbacks

[Mario Limonciello (AMD)]

When the ACPI core uses hibernation callbacks for shutdown drivers
will receive PM_EVENT_POWEROFF and should handle it the same as
PM_EVENT_HIBERNATE would have been used.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v8:
 * Break up series to 3 parts
 * Pick up tag
v5:
 * Re-order
v4:
 * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
---
 drivers/scsi/mesh.c | 1 +
 drivers/scsi/stex.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
index 1c15cac41d80..768b85eecc8f 100644
--- a/drivers/scsi/mesh.c
+++ b/drivers/scsi/mesh.c
@@ -1762,6 +1762,7 @@ static int mesh_suspend(struct macio_dev *mdev, pm_message_t mesg)
 	case PM_EVENT_SUSPEND:
 	case PM_EVENT_HIBERNATE:
 	case PM_EVENT_FREEZE:
+	case PM_EVENT_POWEROFF:
 		break;
 	default:
 		return 0;
diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 63ed7f9aaa93..ee9372e1f7f0 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -1965,6 +1965,7 @@ static int stex_choice_sleep_mic(struct st_hba *hba, pm_message_t state)
 	case PM_EVENT_SUSPEND:
 		return ST_S3;
 	case PM_EVENT_HIBERNATE:
+	case PM_EVENT_POWEROFF:
 		hba->msi_lock = 0;
 		return ST_S4;
 	default:
-- 
2.51.0

Re: [PATCH v8 2/4] scsi: Add PM_EVENT_POWEROFF into suspend callbacks

[Bjorn Helgaas]

On Wed, Sep 17, 2025 at 10:44:25PM -0500, Mario Limonciello (AMD) wrote:
> When the ACPI core uses hibernation callbacks for shutdown drivers
> will receive PM_EVENT_POWEROFF and should handle it the same as
> PM_EVENT_HIBERNATE would have been used.

Apparently ACPI hibernation means PM_EVENT_POWEROFF, and something
else means PM_EVENT_HIBERNATE?  That's confusing to me at least.

> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Tested-by: Eric Naim <dnaim@cachyos.org>
> Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
> ---
> v8:
>  * Break up series to 3 parts
>  * Pick up tag
> v5:
>  * Re-order
> v4:
>  * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
> ---
>  drivers/scsi/mesh.c | 1 +
>  drivers/scsi/stex.c | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/scsi/mesh.c b/drivers/scsi/mesh.c
> index 1c15cac41d80..768b85eecc8f 100644
> --- a/drivers/scsi/mesh.c
> +++ b/drivers/scsi/mesh.c
> @@ -1762,6 +1762,7 @@ static int mesh_suspend(struct macio_dev *mdev, pm_message_t mesg)
>  	case PM_EVENT_SUSPEND:
>  	case PM_EVENT_HIBERNATE:
>  	case PM_EVENT_FREEZE:
> +	case PM_EVENT_POWEROFF:
>  		break;
>  	default:
>  		return 0;
> diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
> index 63ed7f9aaa93..ee9372e1f7f0 100644
> --- a/drivers/scsi/stex.c
> +++ b/drivers/scsi/stex.c
> @@ -1965,6 +1965,7 @@ static int stex_choice_sleep_mic(struct st_hba *hba, pm_message_t state)
>  	case PM_EVENT_SUSPEND:
>  		return ST_S3;
>  	case PM_EVENT_HIBERNATE:
> +	case PM_EVENT_POWEROFF:
>  		hba->msi_lock = 0;
>  		return ST_S4;
>  	default:
> -- 
> 2.51.0
> 

[PATCH v8 3/4] usb: sl811-hcd: Add PM_EVENT_POWEROFF into suspend callbacks

[Mario Limonciello (AMD)]

When the ACPI core uses hibernation callbacks for shutdown drivers
will receive PM_EVENT_POWEROFF and should handle it the same as
PM_EVENT_HIBERNATE would have been used.

Tested-by: Eric Naim <dnaim@cachyos.org>
Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v8:
 * Break up series to 3 parts
v5:
 * Re-order
 * Add tags
v4:
 * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
---
 drivers/usb/host/sl811-hcd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/host/sl811-hcd.c b/drivers/usb/host/sl811-hcd.c
index ea3cab99c5d4..5d6dba681e50 100644
--- a/drivers/usb/host/sl811-hcd.c
+++ b/drivers/usb/host/sl811-hcd.c
@@ -1748,6 +1748,7 @@ sl811h_suspend(struct platform_device *dev, pm_message_t state)
 		break;
 	case PM_EVENT_SUSPEND:
 	case PM_EVENT_HIBERNATE:
+	case PM_EVENT_POWEROFF:
 	case PM_EVENT_PRETHAW:		/* explicitly discard hw state */
 		port_power(sl811, 0);
 		break;
-- 
2.51.0

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

[Mario Limonciello (AMD)]

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.  There are no functional changes in this patch.

Signed-off-by: Mario Limonciello (AMD) <superm1@kernel.org>
---
v8:
 * Drop SYSTEM_HALT case
v7:
 * Reword commit mesasge
v6:
 * Fix LKP robot issue without CONFIG_PM_SLEEP
v5:
 * New patch
v4:
 * https://lore.kernel.org/linux-pci/20250616175019.3471583-1-superm1@kernel.org/
---
 drivers/usb/core/hcd-pci.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/hcd-pci.c b/drivers/usb/core/hcd-pci.c
index cd223475917e..959baccfb07d 100644
--- a/drivers/usb/core/hcd-pci.c
+++ b/drivers/usb/core/hcd-pci.c
@@ -6,6 +6,7 @@
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/pci.h>
+#include <linux/pm.h>
 #include <linux/usb.h>
 #include <linux/usb/hcd.h>
 
@@ -531,6 +532,13 @@ static int hcd_pci_freeze(struct device *dev)
 	return suspend_common(dev, PMSG_FREEZE);
 }
 
+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

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

[Bjorn Helgaas]

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
> 

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

[Mario Limonciello (AMD) (kernel.org)]

On 9/18/2025 2:55 PM, Bjorn Helgaas wrote:
> 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".

A wild 'a' appeared.
> 
>> 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.
> 

It's actually either a hibernate or a power off event.  The nomenclature 
can be a bit confusing.

I'll add a comment to the top of the function to explain it.

> 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?

Well right now it's not a possible path, but per Rafael's guidance of 
splitting the series into 3 parts across 3 cycles the path that leads 
here won't exist for a while.  It was just plumbing.

I'll adjust the commit message to allude to the future.

> 
>> +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
>>