diff mbox

: ACPI: Add the dmi check to make acpi_enforce_resources strict

Message ID 1236687873.7060.31.camel@localhost.localdomain (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Zhao, Yakui March 10, 2009, 12:24 p.m. UTC
Subject: ACPI: Add the dmi check to make acpi_enforce_resources strict
From: Zhao Yakui <yakui.zhao@intel.com>

   On some boxes the SMBus PCI controller is not hidden. The SMbus I/O port
will be accessed in AML code. If the i2c driver is still loaded for the SMbus
PCI device, there is no synchronization between OS and BIOS. And the conflict
will happen.
   So the dmi check is added so that the acpi_enforce_resources is strict when
the box falls into the dmi check table. In such case the i2c driver won't be
loaded for the SMbus pci device.

http://bugzilla.kernel.org/show_bug.cgi?id=12376
http://bugzilla.kernel.org/show_bug.cgi?id=12541
  
Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Acked-by: Jean  Delvare <khali@linux-fr.org>
---
 drivers/acpi/osl.c |   85 +++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 63 insertions(+), 22 deletions(-)



--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Matthew Garrett March 10, 2009, 12:39 p.m. UTC | #1
On Tue, Mar 10, 2009 at 08:24:33PM +0800, yakui_zhao wrote:
> Subject: ACPI: Add the dmi check to make acpi_enforce_resources strict
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
>    On some boxes the SMBus PCI controller is not hidden. The SMbus I/O port
> will be accessed in AML code. If the i2c driver is still loaded for the SMbus
> PCI device, there is no synchronization between OS and BIOS. And the conflict
> will happen.
>    So the dmi check is added so that the acpi_enforce_resources is strict when
> the box falls into the dmi check table. In such case the i2c driver won't be
> loaded for the SMbus pci device.

I think the only safe behaviour here is to default to strict on all 
hardware.
Zhang Rui March 11, 2009, 2:34 a.m. UTC | #2
On Tue, 2009-03-10 at 20:39 +0800, Matthew Garrett wrote:
> On Tue, Mar 10, 2009 at 08:24:33PM +0800, yakui_zhao wrote:
> > Subject: ACPI: Add the dmi check to make acpi_enforce_resources strict
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> >    On some boxes the SMBus PCI controller is not hidden. The SMbus I/O port
> > will be accessed in AML code. If the i2c driver is still loaded for the SMbus
> > PCI device, there is no synchronization between OS and BIOS. And the conflict
> > will happen.
> >    So the dmi check is added so that the acpi_enforce_resources is strict when
> > the box falls into the dmi check table. In such case the i2c driver won't be
> > loaded for the SMbus pci device.
> 
> I think the only safe behaviour here is to default to strict on all 
> hardware.
> 
we've discussed this with Len,
he thinks that we should first generate a DMI patch to fix the eeepc901
regression, which can be shipped upstream soon.
then we'll cook up another patch, to set acpi_enforce_resources=strict
by default, which should be shipped to len's test tree first, and see if
there are any objections from the SMBus driver side.

Yakui will send the second patch out soon.

thanks,
rui.

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Len Brown April 2, 2009, 9:18 p.m. UTC | #3
On Wed, 11 Mar 2009, Zhang Rui wrote:

> On Tue, 2009-03-10 at 20:39 +0800, Matthew Garrett wrote:
> > On Tue, Mar 10, 2009 at 08:24:33PM +0800, yakui_zhao wrote:
> > > Subject: ACPI: Add the dmi check to make acpi_enforce_resources strict
> > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > 
> > >    On some boxes the SMBus PCI controller is not hidden. The SMbus I/O port
> > > will be accessed in AML code. If the i2c driver is still loaded for the SMbus
> > > PCI device, there is no synchronization between OS and BIOS. And the conflict
> > > will happen.
> > >    So the dmi check is added so that the acpi_enforce_resources is strict when
> > > the box falls into the dmi check table. In such case the i2c driver won't be
> > > loaded for the SMbus pci device.
> > 
> > I think the only safe behaviour here is to default to strict on all 
> > hardware.
> > 
> we've discussed this with Len,
> he thinks that we should first generate a DMI patch to fix the eeepc901
> regression, which can be shipped upstream soon.
> then we'll cook up another patch, to set acpi_enforce_resources=strict
> by default, which should be shipped to len's test tree first, and see if
> there are any objections from the SMBus driver side.
> 
> Yakui will send the second patch out soon.

I agree with Matthew.

What I asked for what the DMI patch first -- something we could
send to .29 and .stable w/ minimum risk.

Then (later) on top of that...
a patch to make the DMI unnecessary and switch the 
default -- something we can revert upstream if it turns into a disaster.

at this point, "later" is "now":-)

So I'm going to dump the DMI patch and apply Luca's patch for 2.6.30.

for 2.6.29 we can either go low risk (dmi patch) or change the default
after it cooks in 2.6.30 for a bit.

thanks,
-Len

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

Index: linux-2.6/drivers/acpi/osl.c
===================================================================
--- linux-2.6.orig/drivers/acpi/osl.c	2009-03-10 11:35:01.000000000 +0800
+++ linux-2.6/drivers/acpi/osl.c	2009-03-10 20:21:12.000000000 +0800
@@ -134,6 +134,61 @@ 
 	unsigned int	cmdline:1;
 	unsigned int	known:1;
 } osi_linux = { 0, 0, 0, 0};
+/* Check of resource interference between native drivers and ACPI
+ * OperationRegions (SystemIO and System Memory only).
+ * IO ports and memory declared in ACPI might be used by the ACPI subsystem
+ * in arbitrary AML code and can interfere with legacy drivers.
+ * acpi_enforce_resources= can be set to:
+ *
+ *   - strict           (2)
+ *     -> further driver trying to access the resources will not load
+ *   - lax (default)    (1)
+ *     -> further driver trying to access the resources will load, but you
+ *     get a system message that something might go wrong...
+ *
+ *   - no               (0)
+ *     -> ACPI Operation Region resources will not be registered
+ *
+ */
+
+#define ENFORCE_RESOURCES_STRICT 2
+#define ENFORCE_RESOURCES_LAX    1
+#define ENFORCE_RESOURCES_NO     0
+
+static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
+static struct dmi_system_id __initdata acpi_resources_dmi_table[] = {
+	{
+	.ident = "Asus EEEPC-901",
+	.matches = {
+		DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK Computer INC."),
+		DMI_MATCH(DMI_BOARD_NAME, "901"),
+		},
+	},
+	{
+	.ident = "Asus P6T DELUXE",
+	.matches = {
+		DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK Computer INC."),
+		DMI_MATCH(DMI_BOARD_NAME, "P6T DELUXE"),
+		},
+	},
+	{
+	.ident = "Asus EEEPC-702",
+	.matches = {
+		DMI_MATCH(DMI_PRODUCT_NAME, "Eee PC"),
+		DMI_MATCH(DMI_BOARD_VENDOR, "ASUSTeK Computer INC."),
+		DMI_MATCH(DMI_BOARD_NAME, "702"),
+		},
+	},
+	{
+	.ident = "Dell 1537",
+	.matches = {
+		DMI_MATCH(DMI_PRODUCT_NAME, "Studio 1537"),
+		DMI_MATCH(DMI_BOARD_VENDOR, "Dell Inc."),
+		DMI_MATCH(DMI_BOARD_NAME, "0P132H"),
+		},
+	},
+	{},
+};
 
 static void __init acpi_request_region (struct acpi_generic_address *addr,
 	unsigned int length, char *desc)
@@ -179,6 +234,14 @@ 
 		acpi_request_region(&acpi_gbl_FADT.xgpe1_block,
 			       acpi_gbl_FADT.gpe1_block_length, "ACPI GPE1_BLK");
 
+	/*
+	 * Only when the ACPI is enabled, the dmi table will be checked. If
+	 * the system falls into the dmi check table, the strict resource
+	 * check will be used.
+	 */
+	if (!acpi_disabled && dmi_check_system(acpi_resources_dmi_table))
+		acpi_enforce_resources = ENFORCE_RESOURCES_STRICT;
+
 	return 0;
 }
 device_initcall(acpi_reserve_resources);
@@ -1057,28 +1120,6 @@ 
 
 __setup("acpi_wake_gpes_always_on", acpi_wake_gpes_always_on_setup);
 
-/* Check of resource interference between native drivers and ACPI
- * OperationRegions (SystemIO and System Memory only).
- * IO ports and memory declared in ACPI might be used by the ACPI subsystem
- * in arbitrary AML code and can interfere with legacy drivers.
- * acpi_enforce_resources= can be set to:
- *
- *   - strict           (2)
- *     -> further driver trying to access the resources will not load
- *   - lax (default)    (1)
- *     -> further driver trying to access the resources will load, but you
- *     get a system message that something might go wrong...
- *
- *   - no               (0)
- *     -> ACPI Operation Region resources will not be registered
- *
- */
-#define ENFORCE_RESOURCES_STRICT 2
-#define ENFORCE_RESOURCES_LAX    1
-#define ENFORCE_RESOURCES_NO     0
-
-static unsigned int acpi_enforce_resources = ENFORCE_RESOURCES_LAX;
-
 static int __init acpi_enforce_resources_setup(char *str)
 {
 	if (str == NULL || *str == '\0')