From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hanjun Guo Subject: Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP Date: Tue, 17 Mar 2015 12:10:02 +0800 Message-ID: <5507A91A.1090206@huawei.com> References: <1426234469-6434-1-git-send-email-hanjun.guo@linaro.org> <3737028.Ib0fKlgSkd@vostro.rjw.lan> <5507933F.2020203@huawei.com> <10995588.scpoK3KRg2@vostro.rjw.lan> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <10995588.scpoK3KRg2@vostro.rjw.lan> Sender: linux-kernel-owner@vger.kernel.org To: "Rafael J. Wysocki" Cc: Hanjun Guo , Catalin Marinas , Will Deacon , Olof Johansson , Grant Likely , Lorenzo Pieralisi , Arnd Bergmann , Mark Rutland , Graeme Gregory , Sudeep Holla , Jon Masters , Marc Zyngier , Mark Brown , Robert Richter , Timur Tabi , Ashwin Chaugule , suravee.suthikulpanit@amd.com, linux-acpi@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, linaro-acpi@lists.linaro.org, Tomasz Nowicki , Zhangdianfang List-Id: linux-acpi@vger.kernel.org On 2015/3/17 11:23, Rafael J. Wysocki wrote: > On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote: >> On 2015/3/17 10:28, Rafael J. Wysocki wrote: >>> On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote: >>>> On 2015/3/17 7:15, Rafael J. Wysocki wrote: >>>>> On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote: >>>>>> On 2015=E5=B9=B403=E6=9C=8814=E6=97=A5 05:49, Rafael J. Wysocki = wrote: >>>>>>> On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote: >>>> [...] >>>>>>>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >>>>>>>> index 074e52b..e8728d7 100644 >>>>>>>> --- a/arch/ia64/Kconfig >>>>>>>> +++ b/arch/ia64/Kconfig >>>>>>>> @@ -10,6 +10,7 @@ config IA64 >>>>>>>> select ARCH_MIGHT_HAVE_PC_SERIO >>>>>>>> select PCI if (!IA64_HP_SIM) >>>>>>>> select ACPI if (!IA64_HP_SIM) >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >>>>>>>> select HAVE_UNSTABLE_SCHED_CLOCK >>>>>>>> select HAVE_IDE >>>>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>>>>>>> index b7d31ca..9804431 100644 >>>>>>>> --- a/arch/x86/Kconfig >>>>>>>> +++ b/arch/x86/Kconfig >>>>>>>> @@ -22,6 +22,7 @@ config X86_64 >>>>>>>> ### Arch settings >>>>>>>> config X86 >>>>>>>> def_bool y >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>> One more nit. If you did >>>>>>> >>>>>>> + select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>>> >>>>>>> here (and above for ia64), you'd avoid having to make ACPI_SLEE= P >>>>>>> depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. >>>>>> In sleep.c, >>>>>> >>>>>> #ifdef CONFIG_ACPI_SLEEP >>>>>> acpi_target_system_state() >>>>>> { >>>>>> } >>>>>> #endif >>>>>> >>>>>> and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, >>>>>> which one of them will be enabled on ARM64 so ACPI_SLEEP >>>>>> will also enabled too. >>>>>> >>>>>> So if we >>>>>> >>>>>> +select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>> >>>>>> and >>>>>> >>>>>> +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) +=3D sleep.o >>>>>> >>>>>> it will lead to errors for acpi_target_system_state() that >>>>>> is declared but not defined, so I will keep the code as >>>>>> it is, what do you think? >>>>> No, we need to hash this out. Having two different Kconfig optio= ns meaning >>>>> almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyo= nd ugly. >>>>> >>>>> Do you need ACPI_SLEEP on ARM64 at all? >>>> No, at least for now we don't need it, the spec for sleep is not r= eady for >>>> ARM64 arch, so ACPI_SLEEP will not work at all on ARM64. >>> Well, so what about selecting ACPI_SLEEP from the architectures tha= t use it? >> Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and >> >> +acpi-$(CONFIG_ACPI_SLEEP) +=3D sleep.o >> >> as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in s= leep.c if >> we doing so)? > Well, almost. There is one problem with that, becuase sleep.c contai= ns code > outside of the ACPI_SLEEP-dependent blocks. That code is used for po= wering > off ACPI platforms. > > I guess you don't want that code on ARM too, right? Yes, you are right. > > Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will b= e the Sorry, I can't fully understand your intention here, could you please explain it more? Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY f= or powering off ACPI platforms? if so, I guess it's not a good idea, ACPI = spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know. > only arch setting it at least for the time being, is that correct? That's pretty sure for now. Thanks Hanjun From mboxrd@z Thu Jan 1 00:00:00 1970 From: guohanjun@huawei.com (Hanjun Guo) Date: Tue, 17 Mar 2015 12:10:02 +0800 Subject: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP In-Reply-To: <10995588.scpoK3KRg2@vostro.rjw.lan> References: <1426234469-6434-1-git-send-email-hanjun.guo@linaro.org> <3737028.Ib0fKlgSkd@vostro.rjw.lan> <5507933F.2020203@huawei.com> <10995588.scpoK3KRg2@vostro.rjw.lan> Message-ID: <5507A91A.1090206@huawei.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2015/3/17 11:23, Rafael J. Wysocki wrote: > On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote: >> On 2015/3/17 10:28, Rafael J. Wysocki wrote: >>> On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote: >>>> On 2015/3/17 7:15, Rafael J. Wysocki wrote: >>>>> On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote: >>>>>> On 2015?03?14? 05:49, Rafael J. Wysocki wrote: >>>>>>> On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote: >>>> [...] >>>>>>>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >>>>>>>> index 074e52b..e8728d7 100644 >>>>>>>> --- a/arch/ia64/Kconfig >>>>>>>> +++ b/arch/ia64/Kconfig >>>>>>>> @@ -10,6 +10,7 @@ config IA64 >>>>>>>> select ARCH_MIGHT_HAVE_PC_SERIO >>>>>>>> select PCI if (!IA64_HP_SIM) >>>>>>>> select ACPI if (!IA64_HP_SIM) >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >>>>>>>> select HAVE_UNSTABLE_SCHED_CLOCK >>>>>>>> select HAVE_IDE >>>>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>>>>>>> index b7d31ca..9804431 100644 >>>>>>>> --- a/arch/x86/Kconfig >>>>>>>> +++ b/arch/x86/Kconfig >>>>>>>> @@ -22,6 +22,7 @@ config X86_64 >>>>>>>> ### Arch settings >>>>>>>> config X86 >>>>>>>> def_bool y >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>> One more nit. If you did >>>>>>> >>>>>>> + select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>>> >>>>>>> here (and above for ia64), you'd avoid having to make ACPI_SLEEP >>>>>>> depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. >>>>>> In sleep.c, >>>>>> >>>>>> #ifdef CONFIG_ACPI_SLEEP >>>>>> acpi_target_system_state() >>>>>> { >>>>>> } >>>>>> #endif >>>>>> >>>>>> and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, >>>>>> which one of them will be enabled on ARM64 so ACPI_SLEEP >>>>>> will also enabled too. >>>>>> >>>>>> So if we >>>>>> >>>>>> +select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>> >>>>>> and >>>>>> >>>>>> +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o >>>>>> >>>>>> it will lead to errors for acpi_target_system_state() that >>>>>> is declared but not defined, so I will keep the code as >>>>>> it is, what do you think? >>>>> No, we need to hash this out. Having two different Kconfig options meaning >>>>> almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly. >>>>> >>>>> Do you need ACPI_SLEEP on ARM64 at all? >>>> No, at least for now we don't need it, the spec for sleep is not ready for >>>> ARM64 arch, so ACPI_SLEEP will not work at all on ARM64. >>> Well, so what about selecting ACPI_SLEEP from the architectures that use it? >> Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and >> >> +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o >> >> as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if >> we doing so)? > Well, almost. There is one problem with that, becuase sleep.c contains code > outside of the ACPI_SLEEP-dependent blocks. That code is used for powering > off ACPI platforms. > > I guess you don't want that code on ARM too, right? Yes, you are right. > > Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the Sorry, I can't fully understand your intention here, could you please explain it more? Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know. > only arch setting it at least for the time being, is that correct? That's pretty sure for now. Thanks Hanjun From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751850AbbCQEKk (ORCPT ); Tue, 17 Mar 2015 00:10:40 -0400 Received: from szxga03-in.huawei.com ([119.145.14.66]:43916 "EHLO szxga03-in.huawei.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751307AbbCQEKg (ORCPT ); Tue, 17 Mar 2015 00:10:36 -0400 Message-ID: <5507A91A.1090206@huawei.com> Date: Tue, 17 Mar 2015 12:10:02 +0800 From: Hanjun Guo User-Agent: Mozilla/5.0 (Windows NT 6.1; rv:24.0) Gecko/20100101 Thunderbird/24.0.1 MIME-Version: 1.0 To: "Rafael J. Wysocki" CC: Hanjun Guo , Catalin Marinas , Will Deacon , Olof Johansson , Grant Likely , Lorenzo Pieralisi , Arnd Bergmann , Mark Rutland , Graeme Gregory , "Sudeep Holla" , Jon Masters , Marc Zyngier , Mark Brown , Robert Richter , Timur Tabi , Ashwin Chaugule , , , , , , "Tomasz Nowicki" , Zhangdianfang Subject: Re: [update][PATCH v10 06/21] ACPI / sleep: Introduce CONFIG_ACPI_GENERIC_SLEEP References: <1426234469-6434-1-git-send-email-hanjun.guo@linaro.org> <3737028.Ib0fKlgSkd@vostro.rjw.lan> <5507933F.2020203@huawei.com> <10995588.scpoK3KRg2@vostro.rjw.lan> In-Reply-To: <10995588.scpoK3KRg2@vostro.rjw.lan> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.177.17.188] X-CFilter-Loop: Reflected X-Mirapoint-Virus-RAPID-Raw: score=unknown(0), refid=str=0001.0A0C0201.5507A931.0029,ss=1,re=0.001,recu=0.000,reip=0.000,cl=1,cld=1,fgs=0, ip=0.0.0.0, so=2013-05-26 15:14:31, dmn=2013-03-21 17:37:32 X-Mirapoint-Loop-Id: 5a02e736bf9fcee31d57be519ad21948 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2015/3/17 11:23, Rafael J. Wysocki wrote: > On Tuesday, March 17, 2015 10:36:47 AM Hanjun Guo wrote: >> On 2015/3/17 10:28, Rafael J. Wysocki wrote: >>> On Tuesday, March 17, 2015 09:08:45 AM Hanjun Guo wrote: >>>> On 2015/3/17 7:15, Rafael J. Wysocki wrote: >>>>> On Monday, March 16, 2015 08:14:52 PM Hanjun Guo wrote: >>>>>> On 2015年03月14日 05:49, Rafael J. Wysocki wrote: >>>>>>> On Friday, March 13, 2015 04:14:29 PM Hanjun Guo wrote: >>>> [...] >>>>>>>> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig >>>>>>>> index 074e52b..e8728d7 100644 >>>>>>>> --- a/arch/ia64/Kconfig >>>>>>>> +++ b/arch/ia64/Kconfig >>>>>>>> @@ -10,6 +10,7 @@ config IA64 >>>>>>>> select ARCH_MIGHT_HAVE_PC_SERIO >>>>>>>> select PCI if (!IA64_HP_SIM) >>>>>>>> select ACPI if (!IA64_HP_SIM) >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>>> select ARCH_MIGHT_HAVE_ACPI_PDC if ACPI >>>>>>>> select HAVE_UNSTABLE_SCHED_CLOCK >>>>>>>> select HAVE_IDE >>>>>>>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig >>>>>>>> index b7d31ca..9804431 100644 >>>>>>>> --- a/arch/x86/Kconfig >>>>>>>> +++ b/arch/x86/Kconfig >>>>>>>> @@ -22,6 +22,7 @@ config X86_64 >>>>>>>> ### Arch settings >>>>>>>> config X86 >>>>>>>> def_bool y >>>>>>>> + select ACPI_GENERIC_SLEEP if ACPI >>>>>>> One more nit. If you did >>>>>>> >>>>>>> + select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>>> >>>>>>> here (and above for ia64), you'd avoid having to make ACPI_SLEEP >>>>>>> depend on ACPI_GENERIC_SLEEP which goes somewhat backwards. >>>>>> In sleep.c, >>>>>> >>>>>> #ifdef CONFIG_ACPI_SLEEP >>>>>> acpi_target_system_state() >>>>>> { >>>>>> } >>>>>> #endif >>>>>> >>>>>> and CONFIG_ACPI_SLEEP depends on SUSPEND || HIBERNATION, >>>>>> which one of them will be enabled on ARM64 so ACPI_SLEEP >>>>>> will also enabled too. >>>>>> >>>>>> So if we >>>>>> >>>>>> +select ACPI_GENERIC_SLEEP if ACPI_SLEEP >>>>>> >>>>>> and >>>>>> >>>>>> +acpi-$(CONFIG_ACPI_GENERIC_SLEEP) += sleep.o >>>>>> >>>>>> it will lead to errors for acpi_target_system_state() that >>>>>> is declared but not defined, so I will keep the code as >>>>>> it is, what do you think? >>>>> No, we need to hash this out. Having two different Kconfig options meaning >>>>> almost the same thing (ACPI_SLEEP and ACPI_GENERIC_SLEEP) is beyond ugly. >>>>> >>>>> Do you need ACPI_SLEEP on ARM64 at all? >>>> No, at least for now we don't need it, the spec for sleep is not ready for >>>> ARM64 arch, so ACPI_SLEEP will not work at all on ARM64. >>> Well, so what about selecting ACPI_SLEEP from the architectures that use it? >> Do you mean remove CONFIG_ACPI_GENERIC_SLEEP and >> >> +acpi-$(CONFIG_ACPI_SLEEP) += sleep.o >> >> as well (also need to remove duplicate #ifdef CONFIG_ACPI_SLEEP in sleep.c if >> we doing so)? > Well, almost. There is one problem with that, becuase sleep.c contains code > outside of the ACPI_SLEEP-dependent blocks. That code is used for powering > off ACPI platforms. > > I guess you don't want that code on ARM too, right? Yes, you are right. > > Perhaps we can use ACPI_REDUCED_HARDWARE_ONLY for that? ARM64 will be the Sorry, I can't fully understand your intention here, could you please explain it more? Let me guess a little bit. Do you mean use ACPI_REDUCED_HARDWARE_ONLY for powering off ACPI platforms? if so, I guess it's not a good idea, ACPI spec only says that S4BIOS is not supported on HW-reduced ACPI platforms, S5 has no such limitation, if I miss something here, please let me know. > only arch setting it at least for the time being, is that correct? That's pretty sure for now. Thanks Hanjun