From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751273AbdGPPl1 (ORCPT ); Sun, 16 Jul 2017 11:41:27 -0400 Received: from mail-oi0-f68.google.com ([209.85.218.68]:35364 "EHLO mail-oi0-f68.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751211AbdGPPlZ (ORCPT ); Sun, 16 Jul 2017 11:41:25 -0400 Subject: Re: [PATCH 1/2] PM / suspend: Add platform_suspend_target_state() To: Pavel Machek Cc: linux-kernel@vger.kernel.org, "Rafael J. Wysocki" , Alexandre Belloni , "Rafael J. Wysocki" , Ulf Hansson , Daniel Lezcano , linux-pm , Thibaud Cornic , JB , Mason , Kevin Hilman , Linux ARM References: <20170623010837.11199-1-f.fainelli@gmail.com> <20170716023610.4658-1-f.fainelli@gmail.com> <20170716023610.4658-2-f.fainelli@gmail.com> <20170706031831.GE12954@xo-6d-61-c0.localdomain> From: Florian Fainelli Message-ID: <12d5fe53-089c-87e8-53fe-8461489efba3@gmail.com> Date: Sun, 16 Jul 2017 08:41:22 -0700 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170706031831.GE12954@xo-6d-61-c0.localdomain> Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 07/05/2017 08:18 PM, Pavel Machek wrote: > Hi! > >> Although this callback is optional and documented as such, it requires >> a platform_suspend_ops::begin callback to be implemented in order to >> provide an accurate suspend/resume state within the driver that >> implements this platform_suspend_ops. >> >> An enumeration: platform_target_state is defined which currently defines >> the standard ACPI_S[1-4] states and can be extended with platform >> specific suspend states. >> >> Signed-off-by: Florian Fainelli >> --- >> include/linux/suspend.h | 25 +++++++++++++++++++++++++ >> kernel/power/suspend.c | 15 +++++++++++++++ >> 2 files changed, 40 insertions(+) >> >> diff --git a/include/linux/suspend.h b/include/linux/suspend.h >> index 0b1cf32edfd7..6e6cc0778816 100644 >> --- a/include/linux/suspend.h >> +++ b/include/linux/suspend.h >> @@ -50,6 +50,16 @@ enum suspend_stat_step { >> SUSPEND_RESUME >> }; >> >> +enum platform_target_state { >> + PLATFORM_STATE_UNKNOWN = -1, >> + PLATFORM_STATE_WORKING = 0, >> + PLATFORM_STATE_ACPI_S1, >> + PLATFORM_STATE_ACPI_S2, >> + PLATFORM_STATE_ACPI_S3, >> + PLATFORM_STATE_ACPI_S4, >> + /* Add platform specific states here */ >> +}; >> + > > As I tried to explain in the email thread, having list with all the possible platform > states is no-go. We have about 1000 platforms supported... FYI, the recent (relatively recent) CPU hotplug conversion from notifiers to a state machine has a similar pattern whereby pieces of code needing to hook into the CPU hotplug state machine add their own enum values as they need. So far it's been working for them, and there were tons of CPU hotplug notifiers in the kernel. Anyhow, let me implement Rafael's suggestions and we can see how we move from there. -- Florian From mboxrd@z Thu Jan 1 00:00:00 1970 From: f.fainelli@gmail.com (Florian Fainelli) Date: Sun, 16 Jul 2017 08:41:22 -0700 Subject: [PATCH 1/2] PM / suspend: Add platform_suspend_target_state() In-Reply-To: <20170706031831.GE12954@xo-6d-61-c0.localdomain> References: <20170623010837.11199-1-f.fainelli@gmail.com> <20170716023610.4658-1-f.fainelli@gmail.com> <20170716023610.4658-2-f.fainelli@gmail.com> <20170706031831.GE12954@xo-6d-61-c0.localdomain> Message-ID: <12d5fe53-089c-87e8-53fe-8461489efba3@gmail.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 07/05/2017 08:18 PM, Pavel Machek wrote: > Hi! > >> Although this callback is optional and documented as such, it requires >> a platform_suspend_ops::begin callback to be implemented in order to >> provide an accurate suspend/resume state within the driver that >> implements this platform_suspend_ops. >> >> An enumeration: platform_target_state is defined which currently defines >> the standard ACPI_S[1-4] states and can be extended with platform >> specific suspend states. >> >> Signed-off-by: Florian Fainelli >> --- >> include/linux/suspend.h | 25 +++++++++++++++++++++++++ >> kernel/power/suspend.c | 15 +++++++++++++++ >> 2 files changed, 40 insertions(+) >> >> diff --git a/include/linux/suspend.h b/include/linux/suspend.h >> index 0b1cf32edfd7..6e6cc0778816 100644 >> --- a/include/linux/suspend.h >> +++ b/include/linux/suspend.h >> @@ -50,6 +50,16 @@ enum suspend_stat_step { >> SUSPEND_RESUME >> }; >> >> +enum platform_target_state { >> + PLATFORM_STATE_UNKNOWN = -1, >> + PLATFORM_STATE_WORKING = 0, >> + PLATFORM_STATE_ACPI_S1, >> + PLATFORM_STATE_ACPI_S2, >> + PLATFORM_STATE_ACPI_S3, >> + PLATFORM_STATE_ACPI_S4, >> + /* Add platform specific states here */ >> +}; >> + > > As I tried to explain in the email thread, having list with all the possible platform > states is no-go. We have about 1000 platforms supported... FYI, the recent (relatively recent) CPU hotplug conversion from notifiers to a state machine has a similar pattern whereby pieces of code needing to hook into the CPU hotplug state machine add their own enum values as they need. So far it's been working for them, and there were tons of CPU hotplug notifiers in the kernel. Anyhow, let me implement Rafael's suggestions and we can see how we move from there. -- Florian