Patchworkβ i2c-scmi: Quirk to work on IBM machines with broken BIOSes

login
register
about
Submitter Crane Cai
Date 2009-10-22 07:17:53
Message ID <20091022071748.GA17917@crane-desktop>
Download mbox | patch
Permalink /patch/55268/
State New
Headers show

Comments

Crane Cai - 2009-10-22 07:17:53
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(-)
Darrick J. Wong - 2009-10-22 17:43:04
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

--
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
Jean Delvare - 2009-10-22 18:37:46
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.

Patch

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);
---