diff mbox

[v9,18/21] ARM64 / ACPI: Select ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64

Message ID 1424853601-6675-19-git-send-email-hanjun.guo@linaro.org (mailing list archive)
State New, archived
Headers show

Commit Message

Hanjun Guo Feb. 25, 2015, 8:39 a.m. UTC
From: Al Stone <al.stone@linaro.org>

ACPI reduced hardware mode is disabled by default, but ARM64
can only run properly in ACPI hardware reduced mode, so select
ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64.

CC: Catalin Marinas <catalin.marinas@arm.com>
CC: Will Deacon <will.deacon@arm.com>
Reviewed-by: Grant Likely <grant.likely@linaro.org>
Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
Tested-by: Yijing Wang <wangyijing@huawei.com>
Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
Tested-by: Jon Masters <jcm@redhat.com>
Tested-by: Timur Tabi <timur@codeaurora.org>
Tested-by: Robert Richter <rrichter@cavium.com>
Acked-by: Robert Richter <rrichter@cavium.com>
Signed-off-by: Al Stone <al.stone@linaro.org>
Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
---
 arch/arm64/Kconfig | 1 +
 1 file changed, 1 insertion(+)

Comments

Lorenzo Pieralisi March 6, 2015, 5:47 p.m. UTC | #1
On Wed, Feb 25, 2015 at 08:39:58AM +0000, Hanjun Guo wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> ACPI reduced hardware mode is disabled by default, but ARM64
> can only run properly in ACPI hardware reduced mode, so select
> ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64.

Agreed. This will remove unneeded acpica code. I think though,
you should also check the FADT flag HW_REDUCED_ACPI at boot to
make sure that the tables are HW reduced compliant. I am flagging
this up because I noticed code in core code (not guarded by
ifdef) that checks:

acpi_gbl_reduced_hardware

and if it is not set it goes on instantiating fixed HW devices.

drivers/acpi/scan.c:2567

It is just a heads up, I think it is harmless, but one thing is
removing acpica code on ACPI HW reduced only, the other is to make
sure the kernel does not try to use ACPI HW features that acpica
can't support.

Lorenzo

> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> Reviewed-by: Grant Likely <grant.likely@linaro.org>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Tested-by: Yijing Wang <wangyijing@huawei.com>
> Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
> Tested-by: Jon Masters <jcm@redhat.com>
> Tested-by: Timur Tabi <timur@codeaurora.org>
> Tested-by: Robert Richter <rrichter@cavium.com>
> Acked-by: Robert Richter <rrichter@cavium.com>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>
> ---
>  arch/arm64/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1b8e973..d00ab9a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM64
>  	def_bool y
> +	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>  	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> -- 
> 1.9.1
> 
>
Grant Likely March 7, 2015, 11:16 p.m. UTC | #2
On Wed, 25 Feb 2015 16:39:58 +0800
, Hanjun Guo <hanjun.guo@linaro.org>
 wrote:
> From: Al Stone <al.stone@linaro.org>
> 
> ACPI reduced hardware mode is disabled by default, but ARM64
> can only run properly in ACPI hardware reduced mode, so select
> ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64.
> 
> CC: Catalin Marinas <catalin.marinas@arm.com>
> CC: Will Deacon <will.deacon@arm.com>
> Reviewed-by: Grant Likely <grant.likely@linaro.org>
> Tested-by: Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com>
> Tested-by: Yijing Wang <wangyijing@huawei.com>
> Tested-by: Mark Langsdorf <mlangsdo@redhat.com>
> Tested-by: Jon Masters <jcm@redhat.com>
> Tested-by: Timur Tabi <timur@codeaurora.org>
> Tested-by: Robert Richter <rrichter@cavium.com>
> Acked-by: Robert Richter <rrichter@cavium.com>
> Signed-off-by: Al Stone <al.stone@linaro.org>
> Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org>

Acked-by: Grant Likely <grant.likely@linaro.org>

> ---
>  arch/arm64/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1b8e973..d00ab9a 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -1,5 +1,6 @@
>  config ARM64
>  	def_bool y
> +	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
>  	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
>  	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
>  	select ARCH_HAS_GCOV_PROFILE_ALL
> -- 
> 1.9.1
>
Hanjun Guo March 10, 2015, 12:23 p.m. UTC | #3
On 2015?03?07? 01:47, Lorenzo Pieralisi wrote:
> On Wed, Feb 25, 2015 at 08:39:58AM +0000, Hanjun Guo wrote:
>> From: Al Stone <al.stone@linaro.org>
>>
>> ACPI reduced hardware mode is disabled by default, but ARM64
>> can only run properly in ACPI hardware reduced mode, so select
>> ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64.
>
> Agreed. This will remove unneeded acpica code. I think though,
> you should also check the FADT flag HW_REDUCED_ACPI at boot to
> make sure that the tables are HW reduced compliant. I am flagging

Agreed, I will add that check on top of this patch set. We explicitly
stated that we are using hardware reduced mode on ARM64 in the doc, so, 
how about print some warning message and go on with the boot if we
got the wrong flag HW_REDUCED_ACPI at boot?

> this up because I noticed code in core code (not guarded by
> ifdef) that checks:
>
> acpi_gbl_reduced_hardware
>
> and if it is not set it goes on instantiating fixed HW devices.
>
> drivers/acpi/scan.c:2567
>
> It is just a heads up, I think it is harmless, but one thing is
> removing acpica code on ACPI HW reduced only, the other is to make
> sure the kernel does not try to use ACPI HW features that acpica
> can't support.

I agree, need some cleanups for the ACPICA code I think, and this
was raised about 1 and half years ago and Intel folks need more
consideration on that [1].

[1]: http://comments.gmane.org/gmane.linux.acpi.devel/63844

Thanks
Hanjun
Lorenzo Pieralisi March 10, 2015, 2:16 p.m. UTC | #4
On Tue, Mar 10, 2015 at 12:23:03PM +0000, Hanjun Guo wrote:
> On 2015?03?07? 01:47, Lorenzo Pieralisi wrote:
> > On Wed, Feb 25, 2015 at 08:39:58AM +0000, Hanjun Guo wrote:
> >> From: Al Stone <al.stone@linaro.org>
> >>
> >> ACPI reduced hardware mode is disabled by default, but ARM64
> >> can only run properly in ACPI hardware reduced mode, so select
> >> ACPI_REDUCED_HARDWARE_ONLY if ACPI is enabled on ARM64.
> >
> > Agreed. This will remove unneeded acpica code. I think though,
> > you should also check the FADT flag HW_REDUCED_ACPI at boot to
> > make sure that the tables are HW reduced compliant. I am flagging
> 
> Agreed, I will add that check on top of this patch set. We explicitly
> stated that we are using hardware reduced mode on ARM64 in the doc, so, 
> how about print some warning message and go on with the boot if we
> got the wrong flag HW_REDUCED_ACPI at boot?

Why would be boot if the ACPI tables are not HW_REDUCED_ACPI compliant ?

To look for trouble :) ? arm64 does not support any other ACPI model,
the missing flag should disable ACPI IMO on arm64.

You could add a check in this patch same place where you check
the FADT version and be done with this.

Thank you !
Lorenzo

> > this up because I noticed code in core code (not guarded by
> > ifdef) that checks:
> >
> > acpi_gbl_reduced_hardware
> >
> > and if it is not set it goes on instantiating fixed HW devices.
> >
> > drivers/acpi/scan.c:2567
> >
> > It is just a heads up, I think it is harmless, but one thing is
> > removing acpica code on ACPI HW reduced only, the other is to make
> > sure the kernel does not try to use ACPI HW features that acpica
> > can't support.
> 
> I agree, need some cleanups for the ACPICA code I think, and this
> was raised about 1 and half years ago and Intel folks need more
> consideration on that [1].
> 
> [1]: http://comments.gmane.org/gmane.linux.acpi.devel/63844
> 
> Thanks
> Hanjun
>
diff mbox

Patch

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1b8e973..d00ab9a 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1,5 +1,6 @@ 
 config ARM64
 	def_bool y
+	select ACPI_REDUCED_HARDWARE_ONLY if ACPI
 	select ARCH_BINFMT_ELF_RANDOMIZE_PIE
 	select ARCH_HAS_ATOMIC64_DEC_IF_POSITIVE
 	select ARCH_HAS_GCOV_PROFILE_ALL