[PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Darrick J. Wong]

On some old IBM workstations and desktop computers, the BIOS presents in the
DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
driver looks for.  It also omits the leading "_" in the method names (it should
be _SBR, not SBR_).  Modify the ACPI device scan code to insert the missing HID
if it finds an IBM system with such an object, and modify the i2c-scmi driver
to handle the odd method names.

Affected machines: IntelliStation Z20/Z30.  Note that the i2c-i801 driver no
longer works on these machines because of ACPI resource conflicts.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/acpi/scan.c           |   38 ++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-scmi.c |   49 ++++++++++++++++++++++++++++++++---------
 include/acpi/acpi_drivers.h   |    1 +
 3 files changed, 77 insertions(+), 11 deletions(-)


diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 14a7481..58cb324 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -8,6 +8,7 @@
 #include <linux/acpi.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
+#include <linux/dmi.h>
 
 #include <acpi/acpi_drivers.h>
 
@@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
 	list_add_tail(&id->list, &device->pnp.ids);
 }
 
+/*
+ * Old IBM workstations have a DSDT bug wherein the SMBus object
+ * lacks the SMBUS01 HID and the methods do not have the necessary "_"
+ * prefix.  Work around this.
+ */
+static int probe_ibm_smbus_device(struct acpi_device *device)
+{
+	acpi_handle h_dummy;
+	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
+	int result;
+
+	if (!dmi_name_in_vendors("IBM"))
+		return -ENODEV;
+
+	/* Look for SMBS object */
+	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
+	if (result)
+		return result;
+
+	if (strcmp("SMBS", path.pointer)) {
+		result = -ENODEV;
+		goto out;
+	}
+
+	/* Does it have the necessary (but misnamed) methods? */
+	result = -ENODEV;
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
+		result = 0;
+out:
+	kfree(path.pointer);
+	return result;
+}
+
 static void acpi_device_set_id(struct acpi_device *device)
 {
 	acpi_status status;
@@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 			acpi_add_id(device, ACPI_BAY_HID);
 		else if (ACPI_SUCCESS(acpi_dock_match(device)))
 			acpi_add_id(device, ACPI_DOCK_HID);
+		else if (!probe_ibm_smbus_device(device))
+			acpi_add_id(device, ACPI_SMBUS_HID);
 
 		break;
 	case ACPI_BUS_TYPE_POWER:
diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..f0ad03d 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,15 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{ACPI_SMBUS_HID, 0},
 	{"", 0}
 };
 
@@ -150,11 +158,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods.mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods.mt_sbw;
 		input.count = 5;
 	}
 
@@ -283,20 +291,21 @@ static const struct i2c_algorithm acpi_smbus_cmi_algorithm = {
 };
 
 
-static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
-				  const char *name)
+static int acpi_smbus_cmi_probe_cap(struct acpi_smbus_cmi *smbus_cmi,
+				    const char *name,
+				    const struct smbus_methods_t *methods)
 {
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,17 +328,35 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+		smbus_cmi->methods.mt_info = methods->mt_info;
+	} else if (!strcmp(name, methods->mt_sbr)) {
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+		smbus_cmi->methods.mt_sbr = methods->mt_sbr;
+	} else if (!strcmp(name, methods->mt_sbw)) {
 		smbus_cmi->cap_write = 1;
-	else
+		smbus_cmi->methods.mt_sbw = methods->mt_sbw;
+	} else {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
 				 name));
+		return -ENODEV;
+	}
 
 	return 0;
 }
 
+static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
+				  const char *name)
+{
+	int res;
+
+	res = acpi_smbus_cmi_probe_cap(smbus_cmi, name, &smbus_methods);
+	if (!res)
+		return 0;
+
+	res = acpi_smbus_cmi_probe_cap(smbus_cmi, name, &ibm_smbus_methods);
+	return res;
+}
+
 static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 			void *context, void **return_value)
 {
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index f4906f6..c724a08 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -65,6 +65,7 @@
 #define ACPI_VIDEO_HID			"LNXVIDEO"
 #define ACPI_BAY_HID			"LNXIOBAY"
 #define ACPI_DOCK_HID			"LNXDOCK"
+#define ACPI_SMBUS_HID			"SMBUS01"
 
 /*
  * For fixed hardware buttons, we fabricate acpi_devices with HID

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Crane Cai]

Hi Darrick,

On Tue, Oct 20, 2009 at 04:11:49PM -0700, Darrick J. Wong wrote:
> On some old IBM workstations and desktop computers, the BIOS presents in the
> DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> driver looks for.  It also omits the leading "_" in the method names (it should
> be _SBR, not SBR_).  Modify the ACPI device scan code to insert the missing HID
> if it finds an IBM system with such an object, and modify the i2c-scmi driver
> to handle the odd method names.
I have a suggestion:
You can need not to add quirk in acpi part, instead you can add your ACPI device
HID in i2c-scmi with your specificied methods set.

-- 
Best Regards,
- Crane

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Bjorn Helgaas]

On Tuesday 20 October 2009 08:30:16 pm Crane Cai wrote:
> Hi Darrick,
> 
> On Tue, Oct 20, 2009 at 04:11:49PM -0700, Darrick J. Wong wrote:
> > On some old IBM workstations and desktop computers, the BIOS presents in the
> > DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> > driver looks for.  It also omits the leading "_" in the method names (it should
> > be _SBR, not SBR_).  Modify the ACPI device scan code to insert the missing HID
> > if it finds an IBM system with such an object, and modify the i2c-scmi driver
> > to handle the odd method names.
> I have a suggestion:
> You can need not to add quirk in acpi part, instead you can add your ACPI device
> HID in i2c-scmi with your specificied methods set.

Maybe Darrick understands your suggestion, but I don't.  The only way
i2c-scmi ever learns about a device is when acpi_smbus_cmi_add() is
called, and that's only called when the Linux/ACPI core has found a
device that matches "SMBUS01".

Can you elaborate on your suggestion?

The i2c-scmi driver *could* walk the whole namespace itself, looking
for devices with the SBI/SBR/SBW methods, but I like Darrick's quirk
approach better because it allows i2c-scmi to continue to use the
normal ACPI driver model.

Bjorn

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Darrick J. Wong]

On Wed, Oct 21, 2009 at 08:57:13AM -0600, Bjorn Helgaas wrote:
> On Tuesday 20 October 2009 08:30:16 pm Crane Cai wrote:
> > Hi Darrick,
> > 
> > On Tue, Oct 20, 2009 at 04:11:49PM -0700, Darrick J. Wong wrote:
> > > On some old IBM workstations and desktop computers, the BIOS presents in
> > > the DSDT an SMBus object that is missing the HID identifier that the
> > > i2c-scmi driver looks for.  It also omits the leading "_" in the method
> > > names (it should be _SBR, not SBR_).  Modify the ACPI device scan code to
> > > insert the missing HID if it finds an IBM system with such an object, and
> > > modify the i2c-scmi driver to handle the odd method names.
> > I have a suggestion: You can need not to add quirk in acpi part, instead
> > you can add your ACPI device HID in i2c-scmi with your specificied methods
> > set.

The problem is that the BIOS does not provide an HID for the SMBus object at
all...

> Maybe Darrick understands your suggestion, but I don't.  The only way
> i2c-scmi ever learns about a device is when acpi_smbus_cmi_add() is
> called, and that's only called when the Linux/ACPI core has found a
> device that matches "SMBUS01".
> 
> Can you elaborate on your suggestion?
> 
> The i2c-scmi driver *could* walk the whole namespace itself, looking
> for devices with the SBI/SBR/SBW methods, but I like Darrick's quirk
> approach better because it allows i2c-scmi to continue to use the
> normal ACPI driver model.

...which is why I need the quirk to stuff one in at the OS level.  Were it as
simple as recognizing a different HID, I could have written a less intrusive
patch. :/

--D

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Crane Cai]

Hi Darrick,

On Wed, Oct 21, 2009 at 10:37:33AM -0700, Darrick J. Wong wrote:
> The problem is that the BIOS does not provide an HID for the SMBus object at
> all...
Yes, I now indeed understand. Orignal I guess you manually change IBM SMBus HID,
now I know you add a HID for it.
> > 
> > Can you elaborate on your suggestion?
> > 
This patch below represents my meanings:
*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices as you did
*) hook different HID with different control methods set
It may be more smooth for i2c-scmi, please consider.

---
 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..2c5b629 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	struct acpi_device_id *id = (struct acpi_device_id *)acpi_smbus_cmi_ids;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (; id->id[0]; id++)
+		if (!strcmp((char *) id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
---



-- 
Best Regards,
- Crane

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Darrick J. Wong]

On Thu, Oct 22, 2009 at 03:17:53PM +0800, Crane Cai wrote:

> This patch below represents my meanings:
> *) add a new HID for IBM SMBus CMI devices
> *) add methods for IBM SMBus CMI devices as you did
> *) hook different HID with different control methods set
> It may be more smooth for i2c-scmi, please consider.

Looks fine to me.

I still need the changes to drivers/acpi/scan.c, but if you push this patch
upstream then I'll reroll my patch as a follow-on to yours...

>  static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
> -	{"SMBUS01", 0},
> +	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
> +	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},

...with the custom HID string #define'd in a header file someplace.

Actually, if you'll add a Signed-off-by line to your patch, I'll submit both of
them as a patchset and save you a little work. :)

> +	for (; id->id[0]; id++)

Stylistic nit--would it be clearer to initialize id in the for loop instead of
at the beginning of the function?

--D

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Jean Delvare]

On Thu, 22 Oct 2009 10:43:04 -0700, Darrick J. Wong wrote:
> On Thu, Oct 22, 2009 at 03:17:53PM +0800, Crane Cai wrote:
> 
> > This patch below represents my meanings:
> > *) add a new HID for IBM SMBus CMI devices
> > *) add methods for IBM SMBus CMI devices as you did
> > *) hook different HID with different control methods set
> > It may be more smooth for i2c-scmi, please consider.
> 
> Looks fine to me.
> 
> I still need the changes to drivers/acpi/scan.c, but if you push this patch
> upstream then I'll reroll my patch as a follow-on to yours...
> 
> >  static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
> > -	{"SMBUS01", 0},
> > +	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
> > +	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
> 
> ...with the custom HID string #define'd in a header file someplace.
> 
> Actually, if you'll add a Signed-off-by line to your patch, I'll submit both of
> them as a patchset and save you a little work. :)

As soon as there is an agreement on how the problem would better be
addressed, I'll be happy to pick the patches.

> > +	for (; id->id[0]; id++)
> 
> Stylistic nit--would it be clearer to initialize id in the for loop instead of
> at the beginning of the function?

Definitely, yes.

-- 
Jean Delvare

Re: [PATCH] i2c-scmi: Quirk to work on IBM machines with broken BIOSes

[Crane Cai]

Hi Darrick,

Here is the formal one. Please apply.
IBM SMBus CMI device HID definition depends on you.

-- 
Best Regards,
- Crane

---
>From 7d6d23c944bb1ce29ad75347c867d323288dc2e2 Mon Sep 17 00:00:00 2001
From: Crane Cai <crane.cai@amd.com>
Date: Fri, 23 Oct 2009 10:13:58 +0800
Subject: [PATCH] i2c-scmi: support IBM SMBus CMI devices

*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices
*) hook different HID with different control methods set

Signed-off-by: Crane Cai <crane.cai@amd.com>
---
 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..a15aaf4 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	struct acpi_device_id *id;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (id = (struct acpi_device_id *)acpi_smbus_cmi_ids; id->id[0]; id++)
+		if (!strcmp((char *) id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
 
-- 
1.6.0.4

[PATCH 1/2] i2c-scmi: support IBM SMBus CMI devices

[Darrick J. Wong]

*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices
*) hook different HID with different control methods set

Signed-off-by: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>
---

 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)


diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..a15aaf4 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	struct acpi_device_id *id;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (id = (struct acpi_device_id *)acpi_smbus_cmi_ids; id->id[0]; id++)
+		if (!strcmp((char *) id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
 

Re: [PATCH 1/2] i2c-scmi: support IBM SMBus CMI devices

[Jean Delvare]

Hi Darrick,

On Fri, 23 Oct 2009 10:03:07 -0700, Darrick J. Wong wrote:
> *) add a new HID for IBM SMBus CMI devices
> *) add methods for IBM SMBus CMI devices
> *) hook different HID with different control methods set
> 
> Signed-off-by: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>

When sending patches not written by you, please add as the first line
of the comment a "From:" line with the author. Please also add your own
Signed-off-by line.

> ---
> 
>  drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
> index b4a55d4..a15aaf4 100644
> --- a/drivers/i2c/busses/i2c-scmi.c
> +++ b/drivers/i2c/busses/i2c-scmi.c
> @@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
>  	u8 cap_info:1;
>  	u8 cap_read:1;
>  	u8 cap_write:1;
> +	struct smbus_methods_t *methods;
>  };
>  
>  static const struct smbus_methods_t smbus_methods = {
> @@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
>  	.mt_sbw  = "_SBW",
>  };
>  
> +/* Some IBM BIOSes omit the leading underscore */
> +static const struct smbus_methods_t ibm_smbus_methods = {
> +	.mt_info = "SBI_",
> +	.mt_sbr  = "SBR_",
> +	.mt_sbw  = "SBW_",
> +};
> +
>  static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
> -	{"SMBUS01", 0},
> +	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
> +	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
>  	{"", 0}
>  };

I don't like this method, but as I am neither the author of the driver
not its maintainer... so be it.

>  
> @@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
>  
>  	if (read_write == I2C_SMBUS_READ) {
>  		protocol |= ACPI_SMBUS_PRTCL_READ;
> -		method = smbus_methods.mt_sbr;
> +		method = smbus_cmi->methods->mt_sbr;
>  		input.count = 3;
>  	} else {
>  		protocol |= ACPI_SMBUS_PRTCL_WRITE;
> -		method = smbus_methods.mt_sbw;
> +		method = smbus_cmi->methods->mt_sbw;
>  		input.count = 5;
>  	}
>  
> @@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
>  	union acpi_object *obj;
>  	acpi_status status;
>  
> -	if (!strcmp(name, smbus_methods.mt_info)) {
> +	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
>  		status = acpi_evaluate_object(smbus_cmi->handle,
> -					smbus_methods.mt_info,
> +					smbus_cmi->methods->mt_info,
>  					NULL, &buffer);
>  		if (ACPI_FAILURE(status)) {
>  			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
> -				   smbus_methods.mt_info, status));
> +				   smbus_cmi->methods->mt_info, status));
>  			return -EIO;
>  		}
>  
> @@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
>  
>  		kfree(buffer.pointer);
>  		smbus_cmi->cap_info = 1;
> -	} else if (!strcmp(name, smbus_methods.mt_sbr))
> +	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
>  		smbus_cmi->cap_read = 1;
> -	else if (!strcmp(name, smbus_methods.mt_sbw))
> +	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
>  		smbus_cmi->cap_write = 1;
>  	else
>  		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
> @@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
>  static int acpi_smbus_cmi_add(struct acpi_device *device)
>  {
>  	struct acpi_smbus_cmi *smbus_cmi;
> +	struct acpi_device_id *id;
>  
>  	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
>  	if (!smbus_cmi)
> @@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
>  	smbus_cmi->cap_read = 0;
>  	smbus_cmi->cap_write = 0;
>  
> +	for (id = (struct acpi_device_id *)acpi_smbus_cmi_ids; id->id[0]; id++)

This cast shouldn't be needed. The problem is that you should have
declared "id" const in the first place.

> +		if (!strcmp((char *) id->id, acpi_device_hid(device)))

This cast doesn't seem needed either. At least I don't get any warning
without it, do you?

> +			smbus_cmi->methods =
> +				(struct smbus_methods_t *) id->driver_data;
> +

As a side note it's really too bad that the acpi subsystem doesn't
provide the id as a parameter as all other subsystems do. This would
save individual drivers from having to look it up again.

>  	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
>  			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
>  


-- 
Jean Delvare

Re: [PATCH 1/2] i2c-scmi: support IBM SMBus CMI devices

[Crane Cai]

Hi Darrick,

I have modified the source as Jean suggested. Please apply.

---
>From 5b745774be8fe3f9670fe296b56a0c2964a362f8 Mon Sep 17 00:00:00 2001
From: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>
Date: Fri, 23 Oct 2009 10:13:58 +0800
Subject: [PATCH] i2c-scmi: support IBM SMBus CMI devices

*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices
*) hook different HID with different control methods set

Signed-off-by: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>
---
 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..d68b44f 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	const struct acpi_device_id *id;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
+		if (!strcmp(id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
 
-- 
1.6.0.4


On Sun, Oct 25, 2009 at 10:39:32AM +0100, Jean Delvare wrote:
> > +	for (id = (struct acpi_device_id *)acpi_smbus_cmi_ids; id->id[0]; id++)
> 
> This cast shouldn't be needed. The problem is that you should have
> declared "id" const in the first place.
> 
> > +		if (!strcmp((char *) id->id, acpi_device_hid(device)))
> 
> This cast doesn't seem needed either. At least I don't get any warning
> without it, do you?
> 

-- 
Best Regards,
- Crane

[PATCH 2/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On some old IBM workstations and desktop computers, the BIOS presents in the
DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
driver looks for.  Modify the ACPI device scan code to insert the missing HID
if it finds an IBM system with such an object.  This patch requires Crane Cai's
update to i2c-scmi to work around incorrectly named methods within the SMBus
control object.

Affected machines: IntelliStation Z20/Z30.  Note that the i2c-i801 driver no
longer works on these machines because of ACPI resource conflicts.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/acpi/scan.c           |   38 ++++++++++++++++++++++++++++++++++++++
 drivers/i2c/busses/i2c-scmi.c |    2 +-
 include/acpi/acpi_drivers.h   |    2 ++
 3 files changed, 41 insertions(+), 1 deletions(-)


diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 14a7481..89f8992 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -8,6 +8,7 @@
 #include <linux/acpi.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
+#include <linux/dmi.h>
 
 #include <acpi/acpi_drivers.h>
 
@@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
 	list_add_tail(&id->list, &device->pnp.ids);
 }
 
+/*
+ * Old IBM workstations have a DSDT bug wherein the SMBus object
+ * lacks the SMBUS01 HID and the methods do not have the necessary "_"
+ * prefix.  Work around this.
+ */
+static int probe_ibm_smbus_device(struct acpi_device *device)
+{
+	acpi_handle h_dummy;
+	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
+	int result;
+
+	if (!dmi_name_in_vendors("IBM"))
+		return -ENODEV;
+
+	/* Look for SMBS object */
+	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
+	if (result)
+		return result;
+
+	if (strcmp("SMBS", path.pointer)) {
+		result = -ENODEV;
+		goto out;
+	}
+
+	/* Does it have the necessary (but misnamed) methods? */
+	result = -ENODEV;
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
+		result = 0;
+out:
+	kfree(path.pointer);
+	return result;
+}
+
 static void acpi_device_set_id(struct acpi_device *device)
 {
 	acpi_status status;
@@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 			acpi_add_id(device, ACPI_BAY_HID);
 		else if (ACPI_SUCCESS(acpi_dock_match(device)))
 			acpi_add_id(device, ACPI_DOCK_HID);
+		else if (!probe_ibm_smbus_device(device))
+			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
 
 		break;
 	case ACPI_BUS_TYPE_POWER:
diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index a15aaf4..22c26c2 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -51,7 +51,7 @@ static const struct smbus_methods_t ibm_smbus_methods = {
 
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
 	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
-	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
+	{ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index f4906f6..83a2960 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -65,6 +65,8 @@
 #define ACPI_VIDEO_HID			"LNXVIDEO"
 #define ACPI_BAY_HID			"LNXIOBAY"
 #define ACPI_DOCK_HID			"LNXDOCK"
+/* Quirk for broken IBM BIOSes */
+#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
 
 /*
  * For fixed hardware buttons, we fabricate acpi_devices with HID

Re: [PATCH 2/2] acpi: support IBM SMBus CMI devices

[Jean Delvare]

Hi Darrick,

On Fri, 23 Oct 2009 10:03:11 -0700, Darrick J. Wong wrote:
> On some old IBM workstations and desktop computers, the BIOS presents in the
> DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> driver looks for.  Modify the ACPI device scan code to insert the missing HID
> if it finds an IBM system with such an object.  This patch requires Crane Cai's
> update to i2c-scmi to work around incorrectly named methods within the SMBus
> control object.
> 
> Affected machines: IntelliStation Z20/Z30.  Note that the i2c-i801 driver no
> longer works on these machines because of ACPI resource conflicts.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/acpi/scan.c           |   38 ++++++++++++++++++++++++++++++++++++++
>  drivers/i2c/busses/i2c-scmi.c |    2 +-
>  include/acpi/acpi_drivers.h   |    2 ++
>  3 files changed, 41 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 14a7481..89f8992 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi.h>
>  #include <linux/signal.h>
>  #include <linux/kthread.h>
> +#include <linux/dmi.h>
>  
>  #include <acpi/acpi_drivers.h>
>  
> @@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
>  	list_add_tail(&id->list, &device->pnp.ids);
>  }
>  
> +/*
> + * Old IBM workstations have a DSDT bug wherein the SMBus object
> + * lacks the SMBUS01 HID and the methods do not have the necessary "_"
> + * prefix.  Work around this.
> + */
> +static int probe_ibm_smbus_device(struct acpi_device *device)
> +{
> +	acpi_handle h_dummy;
> +	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
> +	int result;
> +
> +	if (!dmi_name_in_vendors("IBM"))
> +		return -ENODEV;
> +
> +	/* Look for SMBS object */
> +	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
> +	if (result)
> +		return result;
> +
> +	if (strcmp("SMBS", path.pointer)) {
> +		result = -ENODEV;
> +		goto out;
> +	}
> +
> +	/* Does it have the necessary (but misnamed) methods? */
> +	result = -ENODEV;
> +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
> +		result = 0;
> +out:
> +	kfree(path.pointer);
> +	return result;
> +}
> +
>  static void acpi_device_set_id(struct acpi_device *device)
>  {
>  	acpi_status status;
> @@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
>  			acpi_add_id(device, ACPI_BAY_HID);
>  		else if (ACPI_SUCCESS(acpi_dock_match(device)))
>  			acpi_add_id(device, ACPI_DOCK_HID);
> +		else if (!probe_ibm_smbus_device(device))
> +			acpi_add_id(device, ACPI_SMBUS_IBM_HID);

I am not responsible for the ACPI code, but... wouldn't it make sense
to rename probe_ibm_smbus_device() to acpi_ibm_smbus_match() and have
it follow the same convention as acpi_dock_match() and
acpi_bay_match()? To make the code more consistent.

>  
>  		break;
>  	case ACPI_BUS_TYPE_POWER:
> diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
> index a15aaf4..22c26c2 100644
> --- a/drivers/i2c/busses/i2c-scmi.c
> +++ b/drivers/i2c/busses/i2c-scmi.c
> @@ -51,7 +51,7 @@ static const struct smbus_methods_t ibm_smbus_methods = {
>  
>  static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
>  	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
> -	{"SMBUSIBM", (kernel_ulong_t)&ibm_smbus_methods},
> +	{ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods},

As both patches depend on each other and one is useless without the
other, you might as well sequence them the other way around, so that
you touch this line only once.

>  	{"", 0}
>  };
>  
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index f4906f6..83a2960 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -65,6 +65,8 @@
>  #define ACPI_VIDEO_HID			"LNXVIDEO"
>  #define ACPI_BAY_HID			"LNXIOBAY"
>  #define ACPI_DOCK_HID			"LNXDOCK"
> +/* Quirk for broken IBM BIOSes */
> +#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
>  
>  /*
>   * For fixed hardware buttons, we fabricate acpi_devices with HID

Other that these details, I like the patch. If this helps with ACPI
resource conflicts, even better :)

-- 
Jean Delvare

Re: [PATCH 2/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On Sun, Oct 25, 2009 at 12:53:53PM +0100, Jean Delvare wrote:
> > +		else if (!probe_ibm_smbus_device(device))
> > +			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
> 
> I am not responsible for the ACPI code, but... wouldn't it make sense
> to rename probe_ibm_smbus_device() to acpi_ibm_smbus_match() and have
> it follow the same convention as acpi_dock_match() and
> acpi_bay_match()? To make the code more consistent.

Sure.

> > +	{ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods},
> 
> As both patches depend on each other and one is useless without the
> other, you might as well sequence them the other way around, so that
> you touch this line only once.

Will do.

> Other that these details, I like the patch. If this helps with ACPI
> resource conflicts, even better :)

It should. :)  I'll reroll the patch set shortly.

--D

[PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On some old IBM workstations and desktop computers, the BIOS presents in the
DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
driver looks for.  Modify the ACPI device scan code to insert the missing HID
if it finds an IBM system with such an object.  This patch needs Crane Cai's
update to i2c-scmi to work around incorrectly named methods within the SMBus
control object.

Affected machines: IntelliStation Z20/Z30.  Note that the i2c-i801 driver no
longer works on these machines because of ACPI resource conflicts.

v2 contains minor tweaks suggested by Jean Delvare.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
---

 drivers/acpi/scan.c         |   38 ++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_drivers.h |    2 ++
 2 files changed, 40 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 14a7481..5a24429 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -8,6 +8,7 @@
 #include <linux/acpi.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
+#include <linux/dmi.h>
 
 #include <acpi/acpi_drivers.h>
 
@@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
 	list_add_tail(&id->list, &device->pnp.ids);
 }
 
+/*
+ * Old IBM workstations have a DSDT bug wherein the SMBus object
+ * lacks the SMBUS01 HID and the methods do not have the necessary "_"
+ * prefix.  Work around this.
+ */
+static int acpi_ibm_smbus_match(struct acpi_device *device)
+{
+	acpi_handle h_dummy;
+	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
+	int result;
+
+	if (!dmi_name_in_vendors("IBM"))
+		return -ENODEV;
+
+	/* Look for SMBS object */
+	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
+	if (result)
+		return result;
+
+	if (strcmp("SMBS", path.pointer)) {
+		result = -ENODEV;
+		goto out;
+	}
+
+	/* Does it have the necessary (but misnamed) methods? */
+	result = -ENODEV;
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
+		result = 0;
+out:
+	kfree(path.pointer);
+	return result;
+}
+
 static void acpi_device_set_id(struct acpi_device *device)
 {
 	acpi_status status;
@@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 			acpi_add_id(device, ACPI_BAY_HID);
 		else if (ACPI_SUCCESS(acpi_dock_match(device)))
 			acpi_add_id(device, ACPI_DOCK_HID);
+		else if (!acpi_ibm_smbus_match(device))
+			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
 
 		break;
 	case ACPI_BUS_TYPE_POWER:
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index f4906f6..83a2960 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -65,6 +65,8 @@
 #define ACPI_VIDEO_HID			"LNXVIDEO"
 #define ACPI_BAY_HID			"LNXIOBAY"
 #define ACPI_DOCK_HID			"LNXDOCK"
+/* Quirk for broken IBM BIOSes */
+#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
 
 /*
  * For fixed hardware buttons, we fabricate acpi_devices with HID

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Jean Delvare]

On Mon, 26 Oct 2009 13:58:19 -0700, Darrick J. Wong wrote:
> On some old IBM workstations and desktop computers, the BIOS presents in the
> DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> driver looks for.  Modify the ACPI device scan code to insert the missing HID
> if it finds an IBM system with such an object.  This patch needs Crane Cai's
> update to i2c-scmi to work around incorrectly named methods within the SMBus
> control object.
> 
> Affected machines: IntelliStation Z20/Z30.  Note that the i2c-i801 driver no
> longer works on these machines because of ACPI resource conflicts.
> 
> v2 contains minor tweaks suggested by Jean Delvare.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/acpi/scan.c         |   38 ++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_drivers.h |    2 ++
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 14a7481..5a24429 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi.h>
>  #include <linux/signal.h>
>  #include <linux/kthread.h>
> +#include <linux/dmi.h>
>  
>  #include <acpi/acpi_drivers.h>
>  
> @@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
>  	list_add_tail(&id->list, &device->pnp.ids);
>  }
>  
> +/*
> + * Old IBM workstations have a DSDT bug wherein the SMBus object
> + * lacks the SMBUS01 HID and the methods do not have the necessary "_"
> + * prefix.  Work around this.
> + */
> +static int acpi_ibm_smbus_match(struct acpi_device *device)
> +{
> +	acpi_handle h_dummy;
> +	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
> +	int result;
> +
> +	if (!dmi_name_in_vendors("IBM"))
> +		return -ENODEV;
> +
> +	/* Look for SMBS object */
> +	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
> +	if (result)
> +		return result;
> +
> +	if (strcmp("SMBS", path.pointer)) {
> +		result = -ENODEV;
> +		goto out;
> +	}
> +
> +	/* Does it have the necessary (but misnamed) methods? */
> +	result = -ENODEV;
> +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
> +		result = 0;
> +out:
> +	kfree(path.pointer);
> +	return result;
> +}

I'm only half please with this. You change the function named, but it
doesn't follow the calling convention of acpi_dock_match(), which is a
little confusing.

Anyway, I will need an ack from the ACPI people before I can pick this
patch. Or maybe they should even push it upstream themselves.

> +
>  static void acpi_device_set_id(struct acpi_device *device)
>  {
>  	acpi_status status;
> @@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
>  			acpi_add_id(device, ACPI_BAY_HID);
>  		else if (ACPI_SUCCESS(acpi_dock_match(device)))
>  			acpi_add_id(device, ACPI_DOCK_HID);
> +		else if (!acpi_ibm_smbus_match(device))
> +			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
>  
>  		break;
>  	case ACPI_BUS_TYPE_POWER:
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index f4906f6..83a2960 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -65,6 +65,8 @@
>  #define ACPI_VIDEO_HID			"LNXVIDEO"
>  #define ACPI_BAY_HID			"LNXIOBAY"
>  #define ACPI_DOCK_HID			"LNXDOCK"
> +/* Quirk for broken IBM BIOSes */
> +#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
>  
>  /*
>   * For fixed hardware buttons, we fabricate acpi_devices with HID


-- 
Jean Delvare

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On Tue, Oct 27, 2009 at 06:03:32PM +0100, Jean Delvare wrote:

> I'm only half please with this. You change the function named, but it
> doesn't follow the calling convention of acpi_dock_match(), which is a
> little confusing.
> 
> Anyway, I will need an ack from the ACPI people before I can pick this
> patch. Or maybe they should even push it upstream themselves.

I am confused.  Looking at that bunch of ifs, acpi_is_video_device returns 1
for a match and 0 for no match.  acpi_bay_match returns 0 for a match and
-ENODEV for no match, which just happens to work with the ACPI_SUCCESS macro.
acpi_dock_match returns ACPI error codes.  Each of the three existing tests has
different return value semantics, so it is not clear to me which one I should
use.

I didn't think it was correct for my probe function to use the ACPI_STATUS
macro unless it returned ACPI error codes... which it does not.  -ENODEV seemed
appropriate for "no device found".

Is it desirable to clean them all up to follow the same convention?

--D

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Jean Delvare]

On Tue, 27 Oct 2009 10:30:01 -0700, Darrick J. Wong wrote:
> On Tue, Oct 27, 2009 at 06:03:32PM +0100, Jean Delvare wrote:
> 
> > I'm only half please with this. You change the function named, but it
> > doesn't follow the calling convention of acpi_dock_match(), which is a
> > little confusing.
> > 
> > Anyway, I will need an ack from the ACPI people before I can pick this
> > patch. Or maybe they should even push it upstream themselves.
> 
> I am confused.  Looking at that bunch of ifs, acpi_is_video_device returns 1
> for a match and 0 for no match.  acpi_bay_match returns 0 for a match and
> -ENODEV for no match, which just happens to work with the ACPI_SUCCESS macro.
> acpi_dock_match returns ACPI error codes.  Each of the three existing tests has
> different return value semantics, so it is not clear to me which one I should
> use.

Ah, sorry, I looked at one (don't remember which one) and assumed the
other ones followed the same convention.

> I didn't think it was correct for my probe function to use the ACPI_STATUS
> macro unless it returned ACPI error codes... which it does not.  -ENODEV seemed
> appropriate for "no device found".
> 
> Is it desirable to clean them all up to follow the same convention?

Ideally, yes, but that would be a separate task from what you're up to
at the moment. And anyway this is not in my realm an I am not going to
work on it myself, so my opinion probably doesn't matter that much.
Sorry for bothering you with this in the first place.

-- 
Jean Delvare

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On Tue, Oct 27, 2009 at 06:36:19PM +0100, Jean Delvare wrote:
> On Tue, 27 Oct 2009 10:30:01 -0700, Darrick J. Wong wrote:
> > On Tue, Oct 27, 2009 at 06:03:32PM +0100, Jean Delvare wrote:
> > 
> > > I'm only half please with this. You change the function named, but it
> > > doesn't follow the calling convention of acpi_dock_match(), which is a
> > > little confusing.
> > > 
> > > Anyway, I will need an ack from the ACPI people before I can pick this
> > > patch. Or maybe they should even push it upstream themselves.

Len/Bjorn: Any opinion on this ACPI quirk patch to make i2c-scmi work on IBM
systems?  I think the i2c-scmi part is in Jean's tree, but I still need this
second patch to the ACPI code itself.  Did you pick it up, or should I resend?

--D

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Bjorn Helgaas]

On Friday 04 December 2009 10:06:21 am Darrick J. Wong wrote:
> On Tue, Oct 27, 2009 at 06:36:19PM +0100, Jean Delvare wrote:
> > On Tue, 27 Oct 2009 10:30:01 -0700, Darrick J. Wong wrote:
> > > On Tue, Oct 27, 2009 at 06:03:32PM +0100, Jean Delvare wrote:
> > > 
> > > > I'm only half please with this. You change the function named, but it
> > > > doesn't follow the calling convention of acpi_dock_match(), which is a
> > > > little confusing.
> > > > 
> > > > Anyway, I will need an ack from the ACPI people before I can pick this
> > > > patch. Or maybe they should even push it upstream themselves.
> 
> Len/Bjorn: Any opinion on this ACPI quirk patch to make i2c-scmi work on IBM
> systems?  I think the i2c-scmi part is in Jean's tree, but I still need this
> second patch to the ACPI code itself.  Did you pick it up, or should I resend?

I'm a little confused as to where we left things.  Can you repost
the current series in a new thread?  It looks like there might be
places that use "SMBUSIBM" when they should use ACPI_SMBUS_IBM_HID.

Bjorn

Re: [PATCH v2 1/2] acpi: support IBM SMBus CMI devices

[Darrick J. Wong]

On Fri, Dec 04, 2009 at 10:36:35AM -0700, Bjorn Helgaas wrote:
> On Friday 04 December 2009 10:06:21 am Darrick J. Wong wrote:
> > On Tue, Oct 27, 2009 at 06:36:19PM +0100, Jean Delvare wrote:
> > > On Tue, 27 Oct 2009 10:30:01 -0700, Darrick J. Wong wrote:
> > > > On Tue, Oct 27, 2009 at 06:03:32PM +0100, Jean Delvare wrote:
> > > > 
> > > > > I'm only half please with this. You change the function named, but it
> > > > > doesn't follow the calling convention of acpi_dock_match(), which is a
> > > > > little confusing.
> > > > > 
> > > > > Anyway, I will need an ack from the ACPI people before I can pick this
> > > > > patch. Or maybe they should even push it upstream themselves.
> > 
> > Len/Bjorn: Any opinion on this ACPI quirk patch to make i2c-scmi work on IBM
> > systems?  I think the i2c-scmi part is in Jean's tree, but I still need this
> > second patch to the ACPI code itself.  Did you pick it up, or should I resend?
> 
> I'm a little confused as to where we left things.  Can you repost
> the current series in a new thread?  It looks like there might be
> places that use "SMBUSIBM" when they should use ACPI_SMBUS_IBM_HID.

That was fixed in the patch that Jean picked up, which is why I especially need
the ACPI portion which actually #defines ACPI_SMBUS_IBM_HID. :)  I will repost
both patches soon.

--D

[RESEND PATCH v2 1/2] ACPI: Quirk to make SMBus objects work on IBM machines with broken BIOSes

[Darrick J. Wong]

On some old IBM workstations and desktop computers, the BIOS presents in the
DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
driver looks for.  Modify the ACPI device scan code to insert the missing HID
if it finds an IBM system with such an object.  This patch requires Crane Cai's
update to i2c-scmi to work around incorrectly named methods within the SMBus
control object.

Affected machines: ThinkCenter M52, IntelliStation Z20/Z30.  Note that the
i2c-i801 driver no longer works on these machines because of ACPI resource
conflicts.

Signed-off-by: Darrick J. Wong <djwong-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---

 drivers/acpi/scan.c         |   38 ++++++++++++++++++++++++++++++++++++++
 include/acpi/acpi_drivers.h |    2 ++
 2 files changed, 40 insertions(+), 0 deletions(-)


diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 14a7481..5a24429 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -8,6 +8,7 @@
 #include <linux/acpi.h>
 #include <linux/signal.h>
 #include <linux/kthread.h>
+#include <linux/dmi.h>
 
 #include <acpi/acpi_drivers.h>
 
@@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
 	list_add_tail(&id->list, &device->pnp.ids);
 }
 
+/*
+ * Old IBM workstations have a DSDT bug wherein the SMBus object
+ * lacks the SMBUS01 HID and the methods do not have the necessary "_"
+ * prefix.  Work around this.
+ */
+static int acpi_ibm_smbus_match(struct acpi_device *device)
+{
+	acpi_handle h_dummy;
+	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
+	int result;
+
+	if (!dmi_name_in_vendors("IBM"))
+		return -ENODEV;
+
+	/* Look for SMBS object */
+	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
+	if (result)
+		return result;
+
+	if (strcmp("SMBS", path.pointer)) {
+		result = -ENODEV;
+		goto out;
+	}
+
+	/* Does it have the necessary (but misnamed) methods? */
+	result = -ENODEV;
+	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
+	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
+		result = 0;
+out:
+	kfree(path.pointer);
+	return result;
+}
+
 static void acpi_device_set_id(struct acpi_device *device)
 {
 	acpi_status status;
@@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 			acpi_add_id(device, ACPI_BAY_HID);
 		else if (ACPI_SUCCESS(acpi_dock_match(device)))
 			acpi_add_id(device, ACPI_DOCK_HID);
+		else if (!acpi_ibm_smbus_match(device))
+			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
 
 		break;
 	case ACPI_BUS_TYPE_POWER:
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index f4906f6..83a2960 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -65,6 +65,8 @@
 #define ACPI_VIDEO_HID			"LNXVIDEO"
 #define ACPI_BAY_HID			"LNXIOBAY"
 #define ACPI_DOCK_HID			"LNXDOCK"
+/* Quirk for broken IBM BIOSes */
+#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
 
 /*
  * For fixed hardware buttons, we fabricate acpi_devices with HID

Re: [RESEND PATCH v2 1/2] ACPI: Quirk to make SMBus objects work on IBM machines with broken BIOSes

[Jean Delvare]

On Fri, 4 Dec 2009 10:11:03 -0800, Darrick J. Wong wrote:
> On some old IBM workstations and desktop computers, the BIOS presents in the
> DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> driver looks for.  Modify the ACPI device scan code to insert the missing HID
> if it finds an IBM system with such an object.  This patch requires Crane Cai's
> update to i2c-scmi to work around incorrectly named methods within the SMBus
> control object.
> 
> Affected machines: ThinkCenter M52, IntelliStation Z20/Z30.  Note that the
> i2c-i801 driver no longer works on these machines because of ACPI resource
> conflicts.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> ---
> 
>  drivers/acpi/scan.c         |   38 ++++++++++++++++++++++++++++++++++++++
>  include/acpi/acpi_drivers.h |    2 ++
>  2 files changed, 40 insertions(+), 0 deletions(-)
> 
> 
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 14a7481..5a24429 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -8,6 +8,7 @@
>  #include <linux/acpi.h>
>  #include <linux/signal.h>
>  #include <linux/kthread.h>
> +#include <linux/dmi.h>
>  
>  #include <acpi/acpi_drivers.h>
>  
> @@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
>  	list_add_tail(&id->list, &device->pnp.ids);
>  }
>  
> +/*
> + * Old IBM workstations have a DSDT bug wherein the SMBus object
> + * lacks the SMBUS01 HID and the methods do not have the necessary "_"
> + * prefix.  Work around this.
> + */
> +static int acpi_ibm_smbus_match(struct acpi_device *device)
> +{
> +	acpi_handle h_dummy;
> +	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
> +	int result;
> +
> +	if (!dmi_name_in_vendors("IBM"))
> +		return -ENODEV;
> +
> +	/* Look for SMBS object */
> +	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
> +	if (result)
> +		return result;
> +
> +	if (strcmp("SMBS", path.pointer)) {
> +		result = -ENODEV;
> +		goto out;
> +	}
> +
> +	/* Does it have the necessary (but misnamed) methods? */
> +	result = -ENODEV;
> +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
> +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
> +		result = 0;
> +out:
> +	kfree(path.pointer);
> +	return result;
> +}
> +
>  static void acpi_device_set_id(struct acpi_device *device)
>  {
>  	acpi_status status;
> @@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
>  			acpi_add_id(device, ACPI_BAY_HID);
>  		else if (ACPI_SUCCESS(acpi_dock_match(device)))
>  			acpi_add_id(device, ACPI_DOCK_HID);
> +		else if (!acpi_ibm_smbus_match(device))
> +			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
>  
>  		break;
>  	case ACPI_BUS_TYPE_POWER:
> diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> index f4906f6..83a2960 100644
> --- a/include/acpi/acpi_drivers.h
> +++ b/include/acpi/acpi_drivers.h
> @@ -65,6 +65,8 @@
>  #define ACPI_VIDEO_HID			"LNXVIDEO"
>  #define ACPI_BAY_HID			"LNXIOBAY"
>  #define ACPI_DOCK_HID			"LNXDOCK"
> +/* Quirk for broken IBM BIOSes */
> +#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
>  
>  /*
>   * For fixed hardware buttons, we fabricate acpi_devices with HID

Bjorn, Len, can you please apply this quickly? The i2c-scmi side of the
changes is pending for weeks now, but I can't apply it as long as the
ACPI parts aren't in.

Thanks,
-- 
Jean Delvare

Re: [RESEND PATCH v2 1/2] ACPI: Quirk to make SMBus objects work on IBM machines with broken BIOSes

[Jean Delvare]

On Thu, 17 Dec 2009 15:02:43 +0100, Jean Delvare wrote:
> On Fri, 4 Dec 2009 10:11:03 -0800, Darrick J. Wong wrote:
> > On some old IBM workstations and desktop computers, the BIOS presents in the
> > DSDT an SMBus object that is missing the HID identifier that the i2c-scmi
> > driver looks for.  Modify the ACPI device scan code to insert the missing HID
> > if it finds an IBM system with such an object.  This patch requires Crane Cai's
> > update to i2c-scmi to work around incorrectly named methods within the SMBus
> > control object.
> > 
> > Affected machines: ThinkCenter M52, IntelliStation Z20/Z30.  Note that the
> > i2c-i801 driver no longer works on these machines because of ACPI resource
> > conflicts.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> > ---
> > 
> >  drivers/acpi/scan.c         |   38 ++++++++++++++++++++++++++++++++++++++
> >  include/acpi/acpi_drivers.h |    2 ++
> >  2 files changed, 40 insertions(+), 0 deletions(-)
> > 
> > 
> > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> > index 14a7481..5a24429 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/acpi.h>
> >  #include <linux/signal.h>
> >  #include <linux/kthread.h>
> > +#include <linux/dmi.h>
> >  
> >  #include <acpi/acpi_drivers.h>
> >  
> > @@ -1014,6 +1015,41 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
> >  	list_add_tail(&id->list, &device->pnp.ids);
> >  }
> >  
> > +/*
> > + * Old IBM workstations have a DSDT bug wherein the SMBus object
> > + * lacks the SMBUS01 HID and the methods do not have the necessary "_"
> > + * prefix.  Work around this.
> > + */
> > +static int acpi_ibm_smbus_match(struct acpi_device *device)
> > +{
> > +	acpi_handle h_dummy;
> > +	struct acpi_buffer path = {ACPI_ALLOCATE_BUFFER, NULL};
> > +	int result;
> > +
> > +	if (!dmi_name_in_vendors("IBM"))
> > +		return -ENODEV;
> > +
> > +	/* Look for SMBS object */
> > +	result = acpi_get_name(device->handle, ACPI_SINGLE_NAME, &path);
> > +	if (result)
> > +		return result;
> > +
> > +	if (strcmp("SMBS", path.pointer)) {
> > +		result = -ENODEV;
> > +		goto out;
> > +	}
> > +
> > +	/* Does it have the necessary (but misnamed) methods? */
> > +	result = -ENODEV;
> > +	if (ACPI_SUCCESS(acpi_get_handle(device->handle, "SBI", &h_dummy)) &&
> > +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBR", &h_dummy)) &&
> > +	    ACPI_SUCCESS(acpi_get_handle(device->handle, "SBW", &h_dummy)))
> > +		result = 0;
> > +out:
> > +	kfree(path.pointer);
> > +	return result;
> > +}
> > +
> >  static void acpi_device_set_id(struct acpi_device *device)
> >  {
> >  	acpi_status status;
> > @@ -1064,6 +1100,8 @@ static void acpi_device_set_id(struct acpi_device *device)
> >  			acpi_add_id(device, ACPI_BAY_HID);
> >  		else if (ACPI_SUCCESS(acpi_dock_match(device)))
> >  			acpi_add_id(device, ACPI_DOCK_HID);
> > +		else if (!acpi_ibm_smbus_match(device))
> > +			acpi_add_id(device, ACPI_SMBUS_IBM_HID);
> >  
> >  		break;
> >  	case ACPI_BUS_TYPE_POWER:
> > diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
> > index f4906f6..83a2960 100644
> > --- a/include/acpi/acpi_drivers.h
> > +++ b/include/acpi/acpi_drivers.h
> > @@ -65,6 +65,8 @@
> >  #define ACPI_VIDEO_HID			"LNXVIDEO"
> >  #define ACPI_BAY_HID			"LNXIOBAY"
> >  #define ACPI_DOCK_HID			"LNXDOCK"
> > +/* Quirk for broken IBM BIOSes */
> > +#define ACPI_SMBUS_IBM_HID		"SMBUSIBM"
> >  
> >  /*
> >   * For fixed hardware buttons, we fabricate acpi_devices with HID
> 
> Bjorn, Len, can you please apply this quickly? The i2c-scmi side of the
> changes is pending for weeks now, but I can't apply it as long as the
> ACPI parts aren't in.

Guys, I am sorry to insist but I still can't see this patch upstream
while (I guess) rc3 is about to be released. Is there anything wrong
with Darrick's patch? If not, please apply it and push it to Linus
quickly.

Thanks,
-- 
Jean Delvare

[RESEND PATCH v2 2/2] i2c-scmi: support IBM SMBus CMI devices

[Darrick J. Wong]

From: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>

*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices
*) hook different HID with different control methods set
*) minor tweaks as suggested by Jean Delvare

Slightly modified to use #define'd HID from Darrick's ACPI scan quirk patch.

Signed-off-by: Crane Cai <crane.cai-5C7GfCeVMHo@public.gmane.org>
Signed-off-by: Darrick J. Wong <djwong-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
---

 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)


diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..7860cbb 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	const struct acpi_device_id *id;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
+		if (!strcmp(id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
 

[PATCH v2 2/2] i2c-scmi: support IBM SMBus CMI devices

[Darrick J. Wong]

From: Crane Cai <crane.cai@amd.com>

*) add a new HID for IBM SMBus CMI devices
*) add methods for IBM SMBus CMI devices
*) hook different HID with different control methods set
*) minor tweaks as suggested by Jean Delvare

Slightly modified by Darrick to use #define'd IBM SMBUS HID from Darrick's ACPI
scan quirk patch.

Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
Signed-off-by: Crane Cai <crane.cai@amd.com>
---

 drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
 1 files changed, 23 insertions(+), 8 deletions(-)


diff --git a/drivers/i2c/busses/i2c-scmi.c b/drivers/i2c/busses/i2c-scmi.c
index b4a55d4..7860cbb 100644
--- a/drivers/i2c/busses/i2c-scmi.c
+++ b/drivers/i2c/busses/i2c-scmi.c
@@ -33,6 +33,7 @@ struct acpi_smbus_cmi {
 	u8 cap_info:1;
 	u8 cap_read:1;
 	u8 cap_write:1;
+	struct smbus_methods_t *methods;
 };
 
 static const struct smbus_methods_t smbus_methods = {
@@ -41,8 +42,16 @@ static const struct smbus_methods_t smbus_methods = {
 	.mt_sbw  = "_SBW",
 };
 
+/* Some IBM BIOSes omit the leading underscore */
+static const struct smbus_methods_t ibm_smbus_methods = {
+	.mt_info = "SBI_",
+	.mt_sbr  = "SBR_",
+	.mt_sbw  = "SBW_",
+};
+
 static const struct acpi_device_id acpi_smbus_cmi_ids[] = {
-	{"SMBUS01", 0},
+	{"SMBUS01", (kernel_ulong_t)&smbus_methods},
+	{ACPI_SMBUS_IBM_HID, (kernel_ulong_t)&ibm_smbus_methods},
 	{"", 0}
 };
 
@@ -150,11 +159,11 @@ acpi_smbus_cmi_access(struct i2c_adapter *adap, u16 addr, unsigned short flags,
 
 	if (read_write == I2C_SMBUS_READ) {
 		protocol |= ACPI_SMBUS_PRTCL_READ;
-		method = smbus_methods.mt_sbr;
+		method = smbus_cmi->methods->mt_sbr;
 		input.count = 3;
 	} else {
 		protocol |= ACPI_SMBUS_PRTCL_WRITE;
-		method = smbus_methods.mt_sbw;
+		method = smbus_cmi->methods->mt_sbw;
 		input.count = 5;
 	}
 
@@ -290,13 +299,13 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 	union acpi_object *obj;
 	acpi_status status;
 
-	if (!strcmp(name, smbus_methods.mt_info)) {
+	if (!strcmp(name, smbus_cmi->methods->mt_info)) {
 		status = acpi_evaluate_object(smbus_cmi->handle,
-					smbus_methods.mt_info,
+					smbus_cmi->methods->mt_info,
 					NULL, &buffer);
 		if (ACPI_FAILURE(status)) {
 			ACPI_ERROR((AE_INFO, "Evaluating %s: %i",
-				   smbus_methods.mt_info, status));
+				   smbus_cmi->methods->mt_info, status));
 			return -EIO;
 		}
 
@@ -319,9 +328,9 @@ static int acpi_smbus_cmi_add_cap(struct acpi_smbus_cmi *smbus_cmi,
 
 		kfree(buffer.pointer);
 		smbus_cmi->cap_info = 1;
-	} else if (!strcmp(name, smbus_methods.mt_sbr))
+	} else if (!strcmp(name, smbus_cmi->methods->mt_sbr))
 		smbus_cmi->cap_read = 1;
-	else if (!strcmp(name, smbus_methods.mt_sbw))
+	else if (!strcmp(name, smbus_cmi->methods->mt_sbw))
 		smbus_cmi->cap_write = 1;
 	else
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Unsupported CMI method: %s\n",
@@ -349,6 +358,7 @@ static acpi_status acpi_smbus_cmi_query_methods(acpi_handle handle, u32 level,
 static int acpi_smbus_cmi_add(struct acpi_device *device)
 {
 	struct acpi_smbus_cmi *smbus_cmi;
+	const struct acpi_device_id *id;
 
 	smbus_cmi = kzalloc(sizeof(struct acpi_smbus_cmi), GFP_KERNEL);
 	if (!smbus_cmi)
@@ -362,6 +372,11 @@ static int acpi_smbus_cmi_add(struct acpi_device *device)
 	smbus_cmi->cap_read = 0;
 	smbus_cmi->cap_write = 0;
 
+	for (id = acpi_smbus_cmi_ids; id->id[0]; id++)
+		if (!strcmp(id->id, acpi_device_hid(device)))
+			smbus_cmi->methods =
+				(struct smbus_methods_t *) id->driver_data;
+
 	acpi_walk_namespace(ACPI_TYPE_METHOD, smbus_cmi->handle, 1,
 			    acpi_smbus_cmi_query_methods, smbus_cmi, NULL);
 

Re: [PATCH v2 2/2] i2c-scmi: support IBM SMBus CMI devices

[Jean Delvare]

On Mon, 26 Oct 2009 14:00:24 -0700, Darrick J. Wong wrote:
> From: Crane Cai <crane.cai@amd.com>
> 
> *) add a new HID for IBM SMBus CMI devices
> *) add methods for IBM SMBus CMI devices
> *) hook different HID with different control methods set
> *) minor tweaks as suggested by Jean Delvare
> 
> Slightly modified by Darrick to use #define'd IBM SMBUS HID from Darrick's ACPI
> scan quirk patch.
> 
> Signed-off-by: Darrick J. Wong <djwong@us.ibm.com>
> Signed-off-by: Crane Cai <crane.cai@amd.com>
> ---
> 
>  drivers/i2c/busses/i2c-scmi.c |   31 +++++++++++++++++++++++--------
>  1 files changed, 23 insertions(+), 8 deletions(-)

Applied, thanks.

-- 
Jean Delvare