| Message ID | 1409583475-6978-5-git-send-email-hanjun.guo@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Mon, Sep 01, 2014 at 03:57:42PM +0100, Hanjun Guo wrote: > --- a/arch/arm64/kernel/acpi.c > +++ b/arch/arm64/kernel/acpi.c > @@ -74,3 +74,18 @@ void __init acpi_boot_table_init(void) > * TBD when ARM/ARM64 starts to support suspend... > */ > int (*acpi_suspend_lowlevel)(void) = NULL; > + > +static int __init parse_acpi(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + /* "acpi=off" disables both ACPI table parsing and interpreter */ > + if (strcmp(arg, "off") == 0) > + disable_acpi(); > + else > + return -EINVAL; /* Core will print when we return error */ > + > + return 0; > +} > +early_param("acpi", parse_acpi); I forgot about early param, so there is a way to set acpi_disabled to 1 before populating the tables.
On Mon, Sep 1, 2014 at 8:57 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: > From: Al Stone <al.stone@linaro.org> > > Introduce one early parameters "off" for "acpi" to disable ACPI on > ARM64. It might be worth mentioning the purpose of this in the changelog (and maybe the documentation). I don't think this parameter is supported on ia64, and on x86 it seems like it's mostly used as a "fix" by Ubuntu users who consider the problem resolved when they find this parameter. That just means we don't get a chance to fix the real underlying problem, so I'm not sure it's a real benefit to have the parameter. > Signed-off-by: Al Stone <al.stone@linaro.org> > Signed-off-by: Graeme Gregory <graeme.gregory@linaro.org> > Signed-off-by: Hanjun Guo <hanjun.guo@linaro.org> > --- > Documentation/kernel-parameters.txt | 3 ++- > arch/arm64/kernel/acpi.c | 15 +++++++++++++++ > 2 files changed, 17 insertions(+), 1 deletion(-) > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index 5ae8608..9dfb1d8 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -165,7 +165,7 @@ multipliers 'Kilo', 'Mega', and 'Giga', equalling 2^10, 2^20, and 2^30 > bytes respectively. Such letter suffixes can also be entirely omitted. > > > - acpi= [HW,ACPI,X86] > + acpi= [HW,ACPI,X86,ARM] > Advanced Configuration and Power Interface > Format: { force | off | strict | noirq | rsdt } > force -- enable ACPI if default was off > @@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. > strictly ACPI specification compliant. > rsdt -- prefer RSDT over (default) XSDT > copy_dsdt -- copy DSDT to memory > + For ARM64, ONLY "acpi=off" is available. > > See also Documentation/power/runtime_pm.txt, pci=noacpi > > diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c > index b6940a0..9547275 100644 > --- a/arch/arm64/kernel/acpi.c > +++ b/arch/arm64/kernel/acpi.c > @@ -74,3 +74,18 @@ void __init acpi_boot_table_init(void) > * TBD when ARM/ARM64 starts to support suspend... > */ > int (*acpi_suspend_lowlevel)(void) = NULL; > + > +static int __init parse_acpi(char *arg) > +{ > + if (!arg) > + return -EINVAL; > + > + /* "acpi=off" disables both ACPI table parsing and interpreter */ > + if (strcmp(arg, "off") == 0) > + disable_acpi(); > + else > + return -EINVAL; /* Core will print when we return error */ > + > + return 0; > +} > +early_param("acpi", parse_acpi); > -- > 1.7.9.5 >
On 09/09/2014 01:17 PM, Bjorn Helgaas wrote: > On Mon, Sep 1, 2014 at 8:57 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: >> From: Al Stone <al.stone@linaro.org> >> >> Introduce one early parameters "off" for "acpi" to disable ACPI on >> ARM64. > > It might be worth mentioning the purpose of this in the changelog (and > maybe the documentation). I don't think this parameter is supported > on ia64, and on x86 it seems like it's mostly used as a "fix" by > Ubuntu users who consider the problem resolved when they find this > parameter. That just means we don't get a chance to fix the real > underlying problem, so I'm not sure it's a real benefit to have the > parameter. >> - acpi= [HW,ACPI,X86] >> + acpi= [HW,ACPI,X86,ARM] >> Advanced Configuration and Power Interface >> Format: { force | off | strict | noirq | rsdt } >> force -- enable ACPI if default was off >> @@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. >> strictly ACPI specification compliant. >> rsdt -- prefer RSDT over (default) XSDT >> copy_dsdt -- copy DSDT to memory >> + For ARM64, ONLY "acpi=off" is available. Something along the lines of: "The purpose of disabling ACPI on 64-bit ARM server platforms is to allow for an orderly adoption of ACPI on early systems that may also provide a DeviceTree based boot option initially. The acpi= option disables both parsing of any tables passed in through the EFI System Table RSDP and also disables the runtime ACPI interpreter on arm64". On our internal systems, we've both ACPI and DeviceTree working with a simple switch (acpi=) to determine the behavior, which is facilitating an orderly total migration to ACPI that is in currently in flight. Jon.
On Tue, Sep 09, 2014 at 11:14:58PM +0100, Jon Masters wrote: > On 09/09/2014 01:17 PM, Bjorn Helgaas wrote: > > On Mon, Sep 1, 2014 at 8:57 AM, Hanjun Guo <hanjun.guo@linaro.org> wrote: > >> From: Al Stone <al.stone@linaro.org> > >> > >> Introduce one early parameters "off" for "acpi" to disable ACPI on > >> ARM64. > > > > It might be worth mentioning the purpose of this in the changelog (and > > maybe the documentation). I don't think this parameter is supported > > on ia64, and on x86 it seems like it's mostly used as a "fix" by > > Ubuntu users who consider the problem resolved when they find this > > parameter. That just means we don't get a chance to fix the real > > underlying problem, so I'm not sure it's a real benefit to have the > > parameter. > > >> - acpi= [HW,ACPI,X86] > >> + acpi= [HW,ACPI,X86,ARM] > >> Advanced Configuration and Power Interface > >> Format: { force | off | strict | noirq | rsdt } > >> force -- enable ACPI if default was off > >> @@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. > >> strictly ACPI specification compliant. > >> rsdt -- prefer RSDT over (default) XSDT > >> copy_dsdt -- copy DSDT to memory > >> + For ARM64, ONLY "acpi=off" is available. > > Something along the lines of: > > "The purpose of disabling ACPI on 64-bit ARM server platforms is to > allow for an orderly adoption of ACPI on early systems that may also > provide a DeviceTree based boot option initially. The acpi= option > disables both parsing of any tables passed in through the EFI System > Table RSDP and also disables the runtime ACPI interpreter on arm64". Please, let's keep this polarised rhetoric out of the Linux kernel. If ACPI support gets merged for arm64, then the community has a commitment to support it alongside devicetree. There isn't a migration path because nothing is being replaced and we shouldn't dictate to users in which circumstances they are allowed to use one firmware interface over another. Other entities (server vendors, distributions, firmware guys, ...) can make up their own rules, but attempting to peddle their agenda in the upstream kernel is completely inappropriate. It's blindingly obvious that acpi=off is there to disable ACPI at boot. We either support that option or we don't -- none of this `oh, well you can use it in this specific case I suppose' rubbish. I'm not questioning your use-case, but there's really no need to talk about an `orderly adoption' when all you need to say is that your ACPI is busted and passing acpi=off lets you boot with a devicetree. Yes, I'm being pedantic, but it's really important not to get the upstream messaging wrong about ACPI. Devicetree is absolutely *not* going away and people can choose to use whatever they prefer, however they prefer to use it. Will
On Wed, Sep 10, 2014 at 7:04 AM, Will Deacon <will.deacon@arm.com> wrote: > It's blindingly obvious that acpi=off is there to disable ACPI at boot. > We either support that option or we don't -- none of this `oh, well you > can use it in this specific case I suppose' rubbish. I'm not questioning > your use-case, but there's really no need to talk about an `orderly > adoption' when all you need to say is that your ACPI is busted and passing > acpi=off lets you boot with a devicetree. Maybe we should set a taint bit or give some other indication that we're using a flag to work around breakage. There's a big difference between parameters like "root=", "console=", "quiet", etc., and the ones like "pci=nocrs", "pci=realloc", "acpi=off". The latter are basically workarounds for deficiencies in Linux or the platform, and we should try really hard to minimize their use. We might need some of them as interim workarounds, but I don't think we should regard their use as acceptable standard practice for end users. Bjorn
On Wed, Sep 10, 2014 at 02:21:59PM +0100, Bjorn Helgaas wrote: > On Wed, Sep 10, 2014 at 7:04 AM, Will Deacon <will.deacon@arm.com> wrote: > > > It's blindingly obvious that acpi=off is there to disable ACPI at boot. > > We either support that option or we don't -- none of this `oh, well you > > can use it in this specific case I suppose' rubbish. I'm not questioning > > your use-case, but there's really no need to talk about an `orderly > > adoption' when all you need to say is that your ACPI is busted and passing > > acpi=off lets you boot with a devicetree. > > Maybe we should set a taint bit or give some other indication that > we're using a flag to work around breakage. That sounds like a good idea. Working around breakage is a fact of life, but the taint indicates that it's not the ideal behaviour. Will
On Wed, 10 Sep 2014 07:21:59 -0600, Bjorn Helgaas <bhelgaas@google.com> wrote: > On Wed, Sep 10, 2014 at 7:04 AM, Will Deacon <will.deacon@arm.com> wrote: > > > It's blindingly obvious that acpi=off is there to disable ACPI at boot. > > We either support that option or we don't -- none of this `oh, well you > > can use it in this specific case I suppose' rubbish. I'm not questioning > > your use-case, but there's really no need to talk about an `orderly > > adoption' when all you need to say is that your ACPI is busted and passing > > acpi=off lets you boot with a devicetree. > > Maybe we should set a taint bit or give some other indication that > we're using a flag to work around breakage. Nope. No taint. Maybe a log message, but there are perfectly valid reasons to use acpi=off, such as the user has a DT for the hardware that moves all the PM operations into the kernel-proper to tune for a very specific use case. This moves outside the support envelope, but that doesn't make it a bad thing or the wrong thing to do. g.
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index 5ae8608..9dfb1d8 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -165,7 +165,7 @@ multipliers 'Kilo', 'Mega', and 'Giga', equalling 2^10, 2^20, and 2^30 bytes respectively. Such letter suffixes can also be entirely omitted. - acpi= [HW,ACPI,X86] + acpi= [HW,ACPI,X86,ARM] Advanced Configuration and Power Interface Format: { force | off | strict | noirq | rsdt } force -- enable ACPI if default was off @@ -175,6 +175,7 @@ bytes respectively. Such letter suffixes can also be entirely omitted. strictly ACPI specification compliant. rsdt -- prefer RSDT over (default) XSDT copy_dsdt -- copy DSDT to memory + For ARM64, ONLY "acpi=off" is available. See also Documentation/power/runtime_pm.txt, pci=noacpi diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c index b6940a0..9547275 100644 --- a/arch/arm64/kernel/acpi.c +++ b/arch/arm64/kernel/acpi.c @@ -74,3 +74,18 @@ void __init acpi_boot_table_init(void) * TBD when ARM/ARM64 starts to support suspend... */ int (*acpi_suspend_lowlevel)(void) = NULL; + +static int __init parse_acpi(char *arg) +{ + if (!arg) + return -EINVAL; + + /* "acpi=off" disables both ACPI table parsing and interpreter */ + if (strcmp(arg, "off") == 0) + disable_acpi(); + else + return -EINVAL; /* Core will print when we return error */ + + return 0; +} +early_param("acpi", parse_acpi);