From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 856D12BE629; Wed, 10 Sep 2025 17:48:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757526503; cv=none; b=T3eJTRwzecPStV8VMtHyEFTDWWl2ONhkFI6RzMhZm8sufrWTuVK0/FiNUelZ/oLXS74OwQS6ktnflZvyIu8/LwEC8sg5qe/sBFTkUbXWsUNcJd7mpYRGurFH5pTxcGBLbwyZuNOAtPiM03T2RkQdvsLHPPrW7NId5VCc3XU7ee8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1757526503; c=relaxed/simple; bh=0ztWwTmDi2ZzwzQNUAOEIl2hbr5ojTLDzSuY2PO0wsk=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=e/JgHzBNyAkoq/i20uR72o2kG1S9QbSuUzXuaoxdSqBIUf7rKPkN/WlZGFIevXN8WqaRrVbEi0JtG0DZSB5LSL2OGDCCAOjsYwYLghShfaXu4hBQK+Zg+bZ4CYZDu40UEBtSGNn0UBMbYJC4kz8Aor8FcRRReGZYVLhwRNOPUe8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=IH4SbEcd; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="IH4SbEcd" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7CF5CC4CEF7; Wed, 10 Sep 2025 17:48:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1757526502; bh=0ztWwTmDi2ZzwzQNUAOEIl2hbr5ojTLDzSuY2PO0wsk=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=IH4SbEcdWEveaRhNvmgsmIMFByRp6rVXdwbu5/4JZNVyZRuSBFNkPE9iYzP8mVCch yusciDeKotqoa7woikSAdDXU1fddbSkewujMT+vE7wekHh3peOaJXK3pc5H5xMwKXh wYOWC2mTTbUCx+qY+D+jiLKqRuZ/J6zmZq2xN3t+ISmu00BrBcgAYKG2+CpTZ0I8T0 O/Bx6Zp/hAWrUgFhXZHjWQhDhgbqlHNIfUzucWPfHnwEehmQpMQmptpAUzpAGsConZ f/Gs8x+aBLlJ6xE2mXPxxka8AwqC/koFc/4EkzPZ56Q08lxfe8xMLVKUfAAz+jn13U cng9KhikfgNKQ== Message-ID: <9332e17e-380b-42cc-b078-7c5d9d09d798@kernel.org> Date: Wed, 10 Sep 2025 12:48:19 -0500 Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 01/12] PM: Introduce new PMSG_POWEROFF event To: "Rafael J. Wysocki" Cc: Greg Kroah-Hartman , Danilo Krummrich , Bjorn Helgaas , Pavel Machek , Len Brown , =?UTF-8?Q?Christian_K=C3=B6nig?= , "James E . J . Bottomley" , "Martin K . Petersen" , Steven Rostedt , "open list:HIBERNATION (aka Software Suspend, aka swsusp)" , "open list:RADEON and AMDGPU DRM DRIVERS" , "open list:DRM DRIVERS" , "open list:PCI SUBSYSTEM" , "open list:SCSI SUBSYSTEM" , "open list:USB SUBSYSTEM" , "open list:TRACING" , AceLan Kao , Kai-Heng Feng , Mark Pearson , =?UTF-8?Q?Merthan_Karaka=C5=9F?= , Eric Naim , "Guilherme G . Piccoli" References: <20250909191619.2580169-1-superm1@kernel.org> <20250909191619.2580169-2-superm1@kernel.org> Content-Language: en-US From: Mario Limonciello In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 9/10/25 8:58 AM, Rafael J. Wysocki wrote: > On Tue, Sep 9, 2025 at 9:16 PM 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. >> >> This event should not have wakeups enabled > > Why? > > It surely is valid to wake up the system while it is being powered > off, especially in the hibernation case. In the hibernation case - yes you want wakeups. But in what is perceived as powering off the machine it's not expected that you would have wakeups. If I have a USB mouse connected and set as a wakeup source, I can click the mouse and the machine wakes up. A user wouldn't expect that happens with a powered off machine. That's certainly not how it works today at least. > > The "poweroff" transition is generally not recoverable, however, so it > may be better to complete it and trigger a reboot if wakeup has been > signaled. Hmm, I'm not sure about that. Back to hypothesizing on the USB mouse case: If I'm entering suspend, the mouse is an enabled wakeup source and I'm moving the mouse the suspend should be aborted. But if I requested the machine to be powered off and and I clicked the mouse while powering off that would be an "aborted power off?. Wouldn't that be really counterintuitive to reboot instead? IE as a user you expect that pressing the power button gets you an off machine, not a different result based on other activity. > >> so update PMSG_NO_WAKEUP() to match it as well. > > No, please. > >> Tested-by: Eric Naim >> Signed-off-by: Mario Limonciello (AMD) >> --- >> 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 | 5 ++++- >> include/trace/events/power.h | 3 ++- >> 3 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c >> index 2ea6e05e6ec90..86661c94e8cef 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 cc7b2dc28574c..892bd93f13dad 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, }) >> @@ -568,7 +571,7 @@ const struct dev_pm_ops name = { \ >> >> #define PMSG_IS_AUTO(msg) (((msg).event & PM_EVENT_AUTO) != 0) >> #define PMSG_NO_WAKEUP(msg) (((msg).event & \ >> - (PM_EVENT_FREEZE | PM_EVENT_QUIESCE)) != 0) >> + (PM_EVENT_FREEZE | PM_EVENT_QUIESCE | PM_EVENT_POWEROFF)) != 0) >> /* >> * Device run-time power management status. >> * >> diff --git a/include/trace/events/power.h b/include/trace/events/power.h >> index 82904291c2b81..370f8df2fdb4b 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.43.0 >>