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@xxxxxxxxxx wrote:
From: Al Stone <ahs3@xxxxxxxxxx>
-ENOCHANGELOG
Yup. Will be added.
Signed-off-by: Al Stone <al.stone@xxxxxxxxxx>
---
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 device
The 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.
--
ciao,
al
-----------------------------------
Al Stone
Software Engineer
Linaro Enterprise Group
al.stone@xxxxxxxxxx
-----------------------------------
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
