On 11/21/2013 02:36 PM, Rafael J. Wysocki wrote:
On Thursday, November 21, 2013 12:36:35 PM Al Stone wrote:
On 11/20/2013 05:27 PM, Rafael J. Wysocki wrote:
On Wednesday, November 20, 2013 02:24:29 PM Al Stone wrote:
On 11/17/2013 03:06 PM, Rafael J. Wysocki wrote:
On Saturday, November 09, 2013 06:36:14 PM al.stone@linaro.org wrote:
From: Al Stone ahs3@redhat.com
-ENOCHANGELOG
Yup. Will be added.
Signed-off-by: Al Stone al.stone@linaro.org
drivers/acpi/bus.c | 3 ++- drivers/acpi/osl.c | 10 ++++++---- drivers/acpi/pci_link.c | 14 ++++++++------ 3 files changed, 16 insertions(+), 11 deletions(-)diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c index b587ec8..6a54dd5 100644 --- a/drivers/acpi/bus.c +++ b/drivers/acpi/bus.c @@ -540,7 +540,8 @@ void __init acpi_early_init(void) goto error0; }
-#ifdef CONFIG_X86 +#if (!CONFIG_ACPI_REDUCED_HARDWARE)
Why don't you use #ifndef here?
No particular reason; I'll change it.
- /* NB: in HW reduced mode, FADT sci_interrupt has no meaning */
I'm not sure what the "NB" stands for, but it looks like that's what "NOTE:" is used for elsewhere.
Ah. Whups. "NB" == "Nota Bene" -- Latin for "note well" and a personal habit when writing. Yes, it should be "NOTE:".
if (!acpi_ioapic) { /* compatible (0) means level (3) */ if (!(acpi_sci_flags & ACPI_MADT_TRIGGER_MASK)) {diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 54a20ff..017b85c 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -84,6 +84,7 @@ static int (*__acpi_os_prepare_extended_sleep)(u8 sleep_state, u32 val_a,
static acpi_osd_handler acpi_irq_handler; static void *acpi_irq_context;+static u32 acpi_irq_number; static struct workqueue_struct *kacpid_wq; static struct workqueue_struct *kacpi_notify_wq; static struct workqueue_struct *kacpi_hotplug_wq; @@ -797,9 +798,9 @@ acpi_os_install_interrupt_handler(u32 gsi, acpi_osd_handler handler,
/* * ACPI interrupts different from the SCI in our copy of the FADT are
* not supported.
* not supported, except in HW reduced mode. */
- if (gsi != acpi_gbl_FADT.sci_interrupt)
- if (!acpi_gbl_reduced_hardware && (gsi != acpi_gbl_FADT.sci_interrupt))
The inner parens are not necessary.
Ack.
Also it seems that we may need to support gsi != acpi_gbl_FADT.sci_interrupt generically, because there may be GPE device objects with interrupts different from the SCI.
In reduced HW mode, there are no GPE blocks defined; all interrupts of that nature are required to use GPIO interrupts instead, afaict.
Well, I'm not sure about that. The GPE0/1 blocks in FADT are not supposed to be present, but does that apply to GPE block devices (Section 9.10 of ACPI 5.0)?
The spec unfortunately has this info scattered through out -- the earlier parts of the spec discussing the reduced HW mode and the discussion around the FADT go into some of the details.
Any more precise pointers?
Anyway, the point was that we may need to support interrupts different from acpi_gbl_FADT.sci_interrupt even if the HW reduced mode is *not* used, so making that depend on acpi_gbl_reduced_hardware doesn't seem quite right.
Yeah, sorry, I should have included the pointers earlier. I'm basing my thinking on my understanding of these sections:
-- Section 4.1 which defines HW reduced ACPI, and specifically on 4.1.1, Hardware-Reduced Events. -- Section 5.2.9 defining the FADT and the HW reduced restrictions -- Section 5.6.5, GPIO-signaled ACPI events -- Section 9.10, GPE block deviceThe way I read 9.10 in particular is why I'm thinking that any use of acpi_gbl_FADT.sci_interrupt needs to go away in HW reduced mode. The first sentence says:
The GPE Block device is an optional device that allows a system designer to describe GPE blocks beyond the two that are described in the FADT.Well, precisely. It doesn't say anything like "GPE block devices may only be used if the GPE blocks described in the FADT are present."
The way I am interpreting that is that a GPE block device only makes sense as an extension of the GPE0/1 blocks, not as an independent device.
It is an independent device and it may use a *different* interrupt (see the example in Section 9.10). [The paragraph right before that example even says: "To represent the GPE block associated with the FADT ..." and describes that in detail.]
So to my eyes the spec doesn't actually explicitly say anywhere that using GPE block devices in the HW reduced mode is invalid.
Valid point. I am making the interpretation based on implied statements, versus an explicit statement. Now that the ACPI spec is part of the UEFI forum, perhaps I can get some clarifying language inserted one way or the other. I'll start poking at that as a side project.
Since using the GPE0/1 blocks is not allowed in reduced HW mode (see 5.2.9), we cannot extend them with a GPE block device.
That being said, I agree we should be able to install interrupt handlers other than acpi_gbl_FADT.sci_interrupt regardless of whether we are in legacy or reduced HW mode. So, I'm thinking that this block:
if (gsi != acpi_gbl_FADT.sci_interrupt) return AE_BAD_PARAMETER;
should just be removed from acpi_os_install_interrupt_handler().
Does that make sense?
I think so. At least I don't recall any specific situation in which it will lead to problems.
I couldn't recall any either. I'll change the patch to remove this if test, then.
Thanks for the feedback.