[PATCH] asus_atk0110: add support for Asus P7P55D

[PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
making the driver unable to read the data from the sensors.
Change the driver to use dynamic buffers (allocated by ACPI core); the
return value is cached, so the number of memory allocations is very low.

Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
Tested-by: Robert Hancock <hancockrwd@gmail.com>

---
 drivers/hwmon/asus_atk0110.c |   50 ++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..aed6e90 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -129,9 +129,15 @@ struct atk_sensor_data {
 	char const *acpi_name;
 };
 
-struct atk_acpi_buffer_u64 {
-	union acpi_object buf;
-	u64 value;
+/* Return buffer format:
+ * [0-3] "value" is valid flag
+ * [4-7] value
+ * [8- ] unknown stuff on newer mobos
+ */
+struct atk_acpi_ret_buffer {
+	u32 flags;
+	u32 value;
+	u8 data[];
 };
 
 static int atk_add(struct acpi_device *device);
@@ -446,8 +452,10 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 	struct acpi_object_list params;
 	struct acpi_buffer ret;
 	union acpi_object id;
-	struct atk_acpi_buffer_u64 tmp;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
 	acpi_status status;
+	int err = 0;
 
 	id.type = ACPI_TYPE_INTEGER;
 	id.integer.value = sensor->id;
@@ -455,11 +463,7 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 	params.count = 1;
 	params.pointer = &id;
 
-	tmp.buf.type = ACPI_TYPE_BUFFER;
-	tmp.buf.buffer.pointer = (u8 *)&tmp.value;
-	tmp.buf.buffer.length = sizeof(u64);
-	ret.length = sizeof(tmp);
-	ret.pointer = &tmp;
+	ret.length = ACPI_ALLOCATE_BUFFER;
 
 	status = acpi_evaluate_object_typed(data->read_handle, NULL, &params,
 			&ret, ACPI_TYPE_BUFFER);
@@ -468,23 +472,31 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 				acpi_format_exception(status));
 		return -EIO;
 	}
+	obj = ret.pointer;
 
-	/* Return buffer format:
-	 * [0-3] "value" is valid flag
-	 * [4-7] value
-	 */
-	if (!(tmp.value & 0xffffffff)) {
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		err = -EIO;
+		goto out;
+	}
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+
+	if (!buf->flags) {
 		/* The reading is not valid, possible causes:
 		 * - sensor failure
 		 * - enumeration was FUBAR (and we didn't notice)
 		 */
-		dev_info(dev, "Failure: %#llx\n", tmp.value);
-		return -EIO;
+		dev_warn(dev, "Failure: %#x\n", buf->flags);
+		err = -EIO;
+		goto out;
 	}
 
-	*value = (tmp.value & 0xffffffff00000000ULL) >> 32;
-
-	return 0;
+	*value = buf->value;
+out:
+	ACPI_FREE(ret.pointer);
+	return err;
 }
 
 static int atk_read_value(struct atk_sensor_data *sensor, u64 *value)


Luca
-- 
Al termine di un pranzo di nozze mi hanno dato un
amaro alle erbe cosi' schifoso che perfino sull'etichetta
c'era un frate che vomitava.

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
> making the driver unable to read the data from the sensors.
> Change the driver to use dynamic buffers (allocated by ACPI core); the
> return value is cached, so the number of memory allocations is very low.
>
> Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
> Tested-by: Robert Hancock <hancockrwd@gmail.com>

I just noticed a problem (either with this patch or some other issue
with the driver on this board): The readings don't seem to be
updating, I get the same values all the time. (Just now I started
compiling a kernel and coretemp reports temperatures in the 60 degree
plus range but atk0110-acpi still reports 35 degrees as it did
before..)

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Thu, Sep 24, 2009 at 5:18 AM, Robert Hancock <hancockrwd@gmail.com> wrote:
> On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
>> making the driver unable to read the data from the sensors.
>> Change the driver to use dynamic buffers (allocated by ACPI core); the
>> return value is cached, so the number of memory allocations is very low.
>>
>> Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
>> Tested-by: Robert Hancock <hancockrwd@gmail.com>
>
> I just noticed a problem (either with this patch or some other issue
> with the driver on this board): The readings don't seem to be
> updating, I get the same values all the time.

Jean just relayed me some information from Asus on this board, the EC
is disabled by default.
I'm working on a fix.

L

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Wed, Sep 23, 2009 at 09:18:45PM -0600, Robert Hancock wrote:
> On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> > With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
> > making the driver unable to read the data from the sensors.
> > Change the driver to use dynamic buffers (allocated by ACPI core); the
> > return value is cached, so the number of memory allocations is very low.
> >
> > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
> > Tested-by: Robert Hancock <hancockrwd@gmail.com>
> 
> I just noticed a problem (either with this patch or some other issue
> with the driver on this board): The readings don't seem to be
> updating, I get the same values all the time. (Just now I started
> compiling a kernel and coretemp reports temperatures in the 60 degree
> plus range but atk0110-acpi still reports 35 degrees as it did
> before..)

Hi Robert,
I have a new patch for you :)
It contains the previous changes to handle the bigger ASBF buffer plus a new
method to enable the EC as suggested by Asus. Be sure to compile with
HWMON_DEBUG_CHIP enabled.

diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..319618d 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -35,18 +35,21 @@
 #define METHOD_OLD_ENUM_FAN	"FSIF"
 
 #define ATK_MUX_HWMON		0x00000006ULL
+#define ATK_MUX_EC		0x00000011ULL
 
 #define ATK_CLASS_MASK		0xff000000ULL
 #define ATK_CLASS_FREQ_CTL	0x03000000ULL
 #define ATK_CLASS_FAN_CTL	0x04000000ULL
 #define ATK_CLASS_HWMON		0x06000000ULL
+#define ATK_CLASS_MGMT		0x11000000ULL
 
 #define ATK_TYPE_MASK		0x00ff0000ULL
 #define HWMON_TYPE_VOLT		0x00020000ULL
 #define HWMON_TYPE_TEMP		0x00030000ULL
 #define HWMON_TYPE_FAN		0x00040000ULL
 
-#define HWMON_SENSOR_ID_MASK	0x0000ffffULL
+#define ELEMENT_ID_MASK		0x0000ffffULL
+#define ATK_EC_ID		0x0004
 
 enum atk_pack_member {
 	HWMON_PACK_FLAGS,
@@ -89,6 +92,7 @@ struct atk_data {
 	/* new inteface */
 	acpi_handle enumerate_handle;
 	acpi_handle read_handle;
+	acpi_handle write_handle;
 
 	int voltage_count;
 	int temperature_count;
@@ -129,9 +133,21 @@ struct atk_sensor_data {
 	char const *acpi_name;
 };
 
-struct atk_acpi_buffer_u64 {
-	union acpi_object buf;
-	u64 value;
+/* Return buffer format:
+ * [0-3] "value" is valid flag
+ * [4-7] value
+ * [8- ] unknown stuff on newer mobos
+ */
+struct atk_acpi_ret_buffer {
+	u32 flags;
+	u32 value;
+	u8 data[];
+};
+
+struct atk_sitm_buffer {
+	u32 id;
+	u32 value1;
+	u32 value2;
 };
 
 static int atk_add(struct acpi_device *device);
@@ -446,8 +462,10 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 	struct acpi_object_list params;
 	struct acpi_buffer ret;
 	union acpi_object id;
-	struct atk_acpi_buffer_u64 tmp;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
 	acpi_status status;
+	int err = 0;
 
 	id.type = ACPI_TYPE_INTEGER;
 	id.integer.value = sensor->id;
@@ -455,11 +473,7 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 	params.count = 1;
 	params.pointer = &id;
 
-	tmp.buf.type = ACPI_TYPE_BUFFER;
-	tmp.buf.buffer.pointer = (u8 *)&tmp.value;
-	tmp.buf.buffer.length = sizeof(u64);
-	ret.length = sizeof(tmp);
-	ret.pointer = &tmp;
+	ret.length = ACPI_ALLOCATE_BUFFER;
 
 	status = acpi_evaluate_object_typed(data->read_handle, NULL, &params,
 			&ret, ACPI_TYPE_BUFFER);
@@ -468,23 +482,31 @@ static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
 				acpi_format_exception(status));
 		return -EIO;
 	}
+	obj = ret.pointer;
 
-	/* Return buffer format:
-	 * [0-3] "value" is valid flag
-	 * [4-7] value
-	 */
-	if (!(tmp.value & 0xffffffff)) {
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		err = -EIO;
+		goto out;
+	}
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+
+	if (!buf->flags) {
 		/* The reading is not valid, possible causes:
 		 * - sensor failure
 		 * - enumeration was FUBAR (and we didn't notice)
 		 */
-		dev_info(dev, "Failure: %#llx\n", tmp.value);
-		return -EIO;
+		dev_warn(dev, "Failure: %#x\n", buf->flags);
+		err = -EIO;
+		goto out;
 	}
 
-	*value = (tmp.value & 0xffffffff00000000ULL) >> 32;
-
-	return 0;
+	*value = buf->value;
+out:
+	ACPI_FREE(ret.pointer);
+	return err;
 }
 
 static int atk_read_value(struct atk_sensor_data *sensor, u64 *value)
@@ -713,6 +735,96 @@ cleanup:
 	return ret;
 }
 
+static int atk_enable_ec(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_buffer buf;
+	acpi_status ret;
+	struct acpi_object_list params;
+	union acpi_object id;
+	union acpi_object *pack;
+	union acpi_object *ec;
+	struct atk_sitm_buffer sitm;
+	int err = 0;
+	u32 ec_ret;
+	int i;
+
+	id.type = ACPI_TYPE_INTEGER;
+	id.integer.value = ATK_MUX_EC;
+	params.count = 1;
+	params.pointer = &id;
+
+	buf.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object(data->enumerate_handle, NULL, &params, &buf);
+	if (ret != AE_OK) {
+		dev_warn(dev, METHOD_ENUMERATE ": ACPI exception: %s\n",
+				acpi_format_exception(ret));
+		return -ENODEV;
+	}
+
+	pack = buf.pointer;
+	if (pack->type != ACPI_TYPE_PACKAGE) {
+		/* EC not present in the enumeration - that's ok */
+		dev_dbg(dev, "GGRP: %#llx not found\n", ATK_MUX_EC);
+		goto out;
+	}
+
+	/* Search the EC */
+	ec = NULL;
+	for (i = 0; i < pack->package.count; i++) {
+		union acpi_object *id;
+		union acpi_object *obj = &pack->package.elements[i];
+		if (obj->type != ACPI_TYPE_PACKAGE)
+			continue;
+
+		id = &obj->package.elements[0];
+		if (id->type != ACPI_TYPE_INTEGER)
+			continue;
+
+		if ((id->integer.value & ELEMENT_ID_MASK) == ATK_EC_ID) {
+			ec = obj;
+			break;
+		}
+	}
+	if (ec == NULL) {
+		/* EC not present */
+		dev_dbg(dev, "EC not found\n");
+		goto out;
+	}
+
+	ACPI_FREE(buf.pointer);
+
+	/* Enable */
+	sitm.id = ec->package.elements[0].integer.value & 0xffffffff;
+	sitm.value1 = 1;
+	sitm.value2 = 0;
+	id.type = ACPI_TYPE_BUFFER;
+	id.buffer.pointer = (u8 *)&sitm;
+	id.buffer.length = sizeof(sitm);
+
+	buf.length = ACPI_ALLOCATE_BUFFER;
+
+	ret = acpi_evaluate_object_typed(data->write_handle, NULL, &params,
+			&buf, ACPI_TYPE_BUFFER);
+	if (ret != AE_OK) {
+		/* Failed to enable the EC */
+		dev_warn(dev, "Failed to enable EC: %s\n",
+				acpi_format_exception(ret));
+		err = -ENODEV;
+		goto out;
+	}
+	ec_ret = *(u32 *)(((union acpi_object *)buf.pointer)->buffer.pointer);
+	if (ec_ret == 0) {
+		dev_warn(dev, "Failed to enable EC\n");
+		err = -ENODEV;
+	} else {
+		dev_info(dev, "EC enabled\n");
+	}
+out:
+	ACPI_FREE(buf.pointer);
+	return err;
+}
+
 static int atk_enumerate_new_hwmon(struct atk_data *data)
 {
 	struct device *dev = &data->acpi_dev->dev;
@@ -724,6 +836,10 @@ static int atk_enumerate_new_hwmon(struct atk_data *data)
 	int err;
 	int i;
 
+	err = atk_enable_ec(data);
+	if (err)
+		return err;
+
 	dev_dbg(dev, "Enumerating hwmon sensors\n");
 
 	id.type = ACPI_TYPE_INTEGER;
@@ -895,6 +1011,15 @@ static int atk_check_new_if(struct atk_data *data)
 	}
 	data->read_handle = ret;
 
+	/* De-multiplexer (write) */
+	status = acpi_get_handle(data->atk_handle, METHOD_WRITE, &ret);
+	if (status != AE_OK) {
+		dev_dbg(dev, "method " METHOD_READ " not found: %s\n",
+				acpi_format_exception(status));
+		return -ENODEV;
+	}
+	data->write_handle = ret;
+
 	return 0;
 }
 

Luca

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Jean Delvare]

Hi Lucas,

On Mon, 28 Sep 2009 15:17:00 +0200, Luca Tettamanti wrote:
> I have a new patch for you :)
> It contains the previous changes to handle the bigger ASBF buffer plus a new
> method to enable the EC as suggested by Asus. Be sure to compile with
> HWMON_DEBUG_CHIP enabled.

Wow, that's a lot of code to just enable the EC, I hoped it would be
more simple than this :(

Wouldn't it make sense to also disable the EC when the asus_atk0110
driver is removed? To restore the chip to its initial state.

-- 
Jean Delvare

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Mon, Sep 28, 2009 at 3:22 PM, Jean Delvare <khali@linux-fr.org> wrote:
> Hi Lucas,
>
> On Mon, 28 Sep 2009 15:17:00 +0200, Luca Tettamanti wrote:
>> I have a new patch for you :)
>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>> method to enable the EC as suggested by Asus. Be sure to compile with
>> HWMON_DEBUG_CHIP enabled.
>
> Wow, that's a lot of code to just enable the EC, I hoped it would be
> more simple than this :(

A lot of that code is error checking ;-)

> Wouldn't it make sense to also disable the EC when the asus_atk0110
> driver is removed? To restore the chip to its initial state.

Yes, it makes sense.

L

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Mon, Sep 28, 2009 at 7:17 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
> On Wed, Sep 23, 2009 at 09:18:45PM -0600, Robert Hancock wrote:
>> On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> > With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
>> > making the driver unable to read the data from the sensors.
>> > Change the driver to use dynamic buffers (allocated by ACPI core); the
>> > return value is cached, so the number of memory allocations is very low.
>> >
>> > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
>> > Tested-by: Robert Hancock <hancockrwd@gmail.com>
>>
>> I just noticed a problem (either with this patch or some other issue
>> with the driver on this board): The readings don't seem to be
>> updating, I get the same values all the time. (Just now I started
>> compiling a kernel and coretemp reports temperatures in the 60 degree
>> plus range but atk0110-acpi still reports 35 degrees as it did
>> before..)
>
> Hi Robert,
> I have a new patch for you :)
> It contains the previous changes to handle the bigger ASBF buffer plus a new
> method to enable the EC as suggested by Asus. Be sure to compile with
> HWMON_DEBUG_CHIP enabled.

Excellent.. seems to work now and give actually updating sensor readings :-)

By the way, if you have any firmware-type contacts at Asus that know
about these boards, you might want to point them at this problem, the
BIOS DMAR tables point to invalid locations when Intel VT-d is
enabled. So far haven't gotten any useful response from tech support..

http://www.gossamer-threads.com/lists/linux/kernel/1131574

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Mon, Sep 28, 2009 at 8:20 PM, Robert Hancock <hancockrwd@gmail.com> wrote:
> On Mon, Sep 28, 2009 at 7:17 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>> On Wed, Sep 23, 2009 at 09:18:45PM -0600, Robert Hancock wrote:
>>> On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>> > With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
>>> > making the driver unable to read the data from the sensors.
>>> > Change the driver to use dynamic buffers (allocated by ACPI core); the
>>> > return value is cached, so the number of memory allocations is very low.
>>> >
>>> > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
>>> > Tested-by: Robert Hancock <hancockrwd@gmail.com>
>>>
>>> I just noticed a problem (either with this patch or some other issue
>>> with the driver on this board): The readings don't seem to be
>>> updating, I get the same values all the time. (Just now I started
>>> compiling a kernel and coretemp reports temperatures in the 60 degree
>>> plus range but atk0110-acpi still reports 35 degrees as it did
>>> before..)
>>
>> Hi Robert,
>> I have a new patch for you :)
>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>> method to enable the EC as suggested by Asus. Be sure to compile with
>> HWMON_DEBUG_CHIP enabled.
>
> Excellent.. seems to work now and give actually updating sensor readings :-)

Have seen a couple of these though, looks like about once an hour:

ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
(20090903/evregion-424)
ACPI Error (psparse-0537): Method parse/execution failed
[\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
ACPI Error (psparse-0537): Method parse/execution failed
[\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME

Maybe sometimes the embedded controller takes longer than the timeout
to process, or something?

>
> By the way, if you have any firmware-type contacts at Asus that know
> about these boards, you might want to point them at this problem, the
> BIOS DMAR tables point to invalid locations when Intel VT-d is
> enabled. So far haven't gotten any useful response from tech support..
>
> http://www.gossamer-threads.com/lists/linux/kernel/1131574
>

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Tue, Sep 29, 2009 at 6:34 AM, Robert Hancock <hancockrwd@gmail.com> wrote:
> On Mon, Sep 28, 2009 at 8:20 PM, Robert Hancock <hancockrwd@gmail.com> wrote:
>> On Mon, Sep 28, 2009 at 7:17 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>> On Wed, Sep 23, 2009 at 09:18:45PM -0600, Robert Hancock wrote:
>>>> On Wed, Sep 23, 2009 at 1:12 PM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>>> > With P7P55D (and newer) boards Asus extended the output buffer (ASBF)
>>>> > making the driver unable to read the data from the sensors.
>>>> > Change the driver to use dynamic buffers (allocated by ACPI core); the
>>>> > return value is cached, so the number of memory allocations is very low.
>>>> >
>>>> > Signed-off-by: Luca Tettamanti <kronos.it@gmail.com>
>>>> > Tested-by: Robert Hancock <hancockrwd@gmail.com>
>>>>
>>>> I just noticed a problem (either with this patch or some other issue
>>>> with the driver on this board): The readings don't seem to be
>>>> updating, I get the same values all the time. (Just now I started
>>>> compiling a kernel and coretemp reports temperatures in the 60 degree
>>>> plus range but atk0110-acpi still reports 35 degrees as it did
>>>> before..)
>>>
>>> Hi Robert,
>>> I have a new patch for you :)
>>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>> HWMON_DEBUG_CHIP enabled.
>>
>> Excellent.. seems to work now and give actually updating sensor readings :-)
>
> Have seen a couple of these though, looks like about once an hour:
>
> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
> (20090903/evregion-424)
> ACPI Error (psparse-0537): Method parse/execution failed
> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
> ACPI Error (psparse-0537): Method parse/execution failed
> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>
> Maybe sometimes the embedded controller takes longer than the timeout
> to process, or something?

Is there a message like "input buffer is not empty" before that?

>> By the way, if you have any firmware-type contacts at Asus that know
>> about these boards, you might want to point them at this problem, the
>> BIOS DMAR tables point to invalid locations when Intel VT-d is
>> enabled. So far haven't gotten any useful response from tech support..

I don't have any inside contact, sorry (and I agree with dwmw2 :P)

L

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Tue, Sep 29, 2009 at 8:07 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>>> Hi Robert,
>>>> I have a new patch for you :)
>>>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>>> HWMON_DEBUG_CHIP enabled.
>>>
>>> Excellent.. seems to work now and give actually updating sensor readings :-)
>>
>> Have seen a couple of these though, looks like about once an hour:
>>
>> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
>> (20090903/evregion-424)
>> ACPI Error (psparse-0537): Method parse/execution failed
>> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
>> ACPI Error (psparse-0537): Method parse/execution failed
>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
>> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>>
>> Maybe sometimes the embedded controller takes longer than the timeout
>> to process, or something?
>
> Is there a message like "input buffer is not empty" before that?

Nope, that's all I'm getting each time it happens. Suggestions/debug
patches welcome..

>
>>> By the way, if you have any firmware-type contacts at Asus that know
>>> about these boards, you might want to point them at this problem, the
>>> BIOS DMAR tables point to invalid locations when Intel VT-d is
>>> enabled. So far haven't gotten any useful response from tech support..
>
> I don't have any inside contact, sorry (and I agree with dwmw2 :P)
>
> L
>

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Robert Hancock skrev:
> On Tue, Sep 29, 2009 at 8:07 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>>>> Hi Robert,
>>>>> I have a new patch for you :)
>>>>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>>>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>>>> HWMON_DEBUG_CHIP enabled.
>>>> Excellent.. seems to work now and give actually updating sensor readings :-)
>>> Have seen a couple of these though, looks like about once an hour:
>>>
>>> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
>>> (20090903/evregion-424)
>>> ACPI Error (psparse-0537): Method parse/execution failed
>>> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
>>> ACPI Error (psparse-0537): Method parse/execution failed
>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
>>> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>>>
>>> Maybe sometimes the embedded controller takes longer than the timeout
>>> to process, or something?
>> Is there a message like "input buffer is not empty" before that?
> 
> Nope, that's all I'm getting each time it happens. Suggestions/debug
> patches welcome..
> 

Try this one:
http://marc.info/?l=linux-kernel&m=125421276407283&w=2

--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
> Robert Hancock skrev:
>>
>> On Tue, Sep 29, 2009 at 8:07 AM, Luca Tettamanti <kronos.it@gmail.com>
>> wrote:
>>>>>>
>>>>>> Hi Robert,
>>>>>> I have a new patch for you :)
>>>>>> It contains the previous changes to handle the bigger ASBF buffer plus
>>>>>> a new
>>>>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>>>>> HWMON_DEBUG_CHIP enabled.
>>>>>
>>>>> Excellent.. seems to work now and give actually updating sensor
>>>>> readings :-)
>>>>
>>>> Have seen a couple of these though, looks like about once an hour:
>>>>
>>>> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
>>>> (20090903/evregion-424)
>>>> ACPI Error (psparse-0537): Method parse/execution failed
>>>> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
>>>> ACPI Error (psparse-0537): Method parse/execution failed
>>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
>>>> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>>>>
>>>> Maybe sometimes the embedded controller takes longer than the timeout
>>>> to process, or something?
>>>
>>> Is there a message like "input buffer is not empty" before that?
>>
>> Nope, that's all I'm getting each time it happens. Suggestions/debug
>> patches welcome..
>>
>
> Try this one:
> http://marc.info/?l=linux-kernel&m=125421276407283&w=2

I added this patch, no timeouts after 2 hours. Could be just chance
though, if it lasts till tomorrow evening it will be pretty
conclusive..

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
> Robert Hancock skrev:
>>
>> On Tue, Sep 29, 2009 at 8:07 AM, Luca Tettamanti <kronos.it@gmail.com>
>> wrote:
>>>>>>
>>>>>> Hi Robert,
>>>>>> I have a new patch for you :)
>>>>>> It contains the previous changes to handle the bigger ASBF buffer plus
>>>>>> a new
>>>>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>>>>> HWMON_DEBUG_CHIP enabled.
>>>>>
>>>>> Excellent.. seems to work now and give actually updating sensor
>>>>> readings :-)
>>>>
>>>> Have seen a couple of these though, looks like about once an hour:
>>>>
>>>> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
>>>> (20090903/evregion-424)
>>>> ACPI Error (psparse-0537): Method parse/execution failed
>>>> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
>>>> ACPI Error (psparse-0537): Method parse/execution failed
>>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
>>>> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>>>>
>>>> Maybe sometimes the embedded controller takes longer than the timeout
>>>> to process, or something?
>>>
>>> Is there a message like "input buffer is not empty" before that?
>>
>> Nope, that's all I'm getting each time it happens. Suggestions/debug
>> patches welcome..
>>
>
> Try this one:
> http://marc.info/?l=linux-kernel&m=125421276407283&w=2

With that patch it's lasted almost a day with no timeouts, previously
I was getting about one every hour. Has that patch been submitted?

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Jean Delvare]

On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
> > Try this one:
> > http://marc.info/?l=linux-kernel&m=125421276407283&w=2
> 
> With that patch it's lasted almost a day with no timeouts, previously
> I was getting about one every hour. Has that patch been submitted?

Do you mean you're still getting timeouts but they are less frequent,
or you did not get any timeout at all so far?

-- 
Jean Delvare

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Thu, Oct 1, 2009 at 12:50 AM, Jean Delvare <khali@linux-fr.org> wrote:
> On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
>> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
>> > Try this one:
>> > http://marc.info/?l=linux-kernel&m=125421276407283&w=2
>>
>> With that patch it's lasted almost a day with no timeouts, previously
>> I was getting about one every hour. Has that patch been submitted?
>
> Do you mean you're still getting timeouts but they are less frequent,
> or you did not get any timeout at all so far?

So far no timeout at all (after about 36 hours).

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Thu, Oct 01, 2009 at 08:40:59AM -0600, Robert Hancock wrote:
> On Thu, Oct 1, 2009 at 12:50 AM, Jean Delvare <khali@linux-fr.org> wrote:
> > On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
> >> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
> >> > Try this one:
> >> > http://marc.info/?l=linux-kernel&m=125421276407283&w=2
> >>
> >> With that patch it's lasted almost a day with no timeouts, previously
> >> I was getting about one every hour. Has that patch been submitted?
> >
> > Do you mean you're still getting timeouts but they are less frequent,
> > or you did not get any timeout at all so far?
> 
> So far no timeout at all (after about 36 hours).

Good. I have a new patch for you then :P

This version checks whether the EC is already enabled or not before touching it
and also restore the state when the module is unloaded.
You should check that the driver keeps working when is unloaded and reloaded.

diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..b70840e 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -35,18 +35,22 @@
 #define METHOD_OLD_ENUM_FAN	"FSIF"
 
 #define ATK_MUX_HWMON		0x00000006ULL
+#define ATK_MUX_MGMT		0x00000011ULL
 
 #define ATK_CLASS_MASK		0xff000000ULL
 #define ATK_CLASS_FREQ_CTL	0x03000000ULL
 #define ATK_CLASS_FAN_CTL	0x04000000ULL
 #define ATK_CLASS_HWMON		0x06000000ULL
+#define ATK_CLASS_MGMT		0x11000000ULL
 
 #define ATK_TYPE_MASK		0x00ff0000ULL
 #define HWMON_TYPE_VOLT		0x00020000ULL
 #define HWMON_TYPE_TEMP		0x00030000ULL
 #define HWMON_TYPE_FAN		0x00040000ULL
 
-#define HWMON_SENSOR_ID_MASK	0x0000ffffULL
+#define ATK_ELEMENT_ID_MASK	0x0000ffffULL
+
+#define ATK_EC_ID		0x11060004ULL
 
 enum atk_pack_member {
 	HWMON_PACK_FLAGS,
@@ -89,6 +93,9 @@ struct atk_data {
 	/* new inteface */
 	acpi_handle enumerate_handle;
 	acpi_handle read_handle;
+	acpi_handle write_handle;
+
+	bool disable_ec;
 
 	int voltage_count;
 	int temperature_count;
@@ -129,9 +136,21 @@ struct atk_sensor_data {
 	char const *acpi_name;
 };
 
-struct atk_acpi_buffer_u64 {
-	union acpi_object buf;
-	u64 value;
+/* Return buffer format:
+ * [0-3] "value" is valid flag
+ * [4-7] value
+ * [8- ] unknown stuff on newer mobos
+ */
+struct atk_acpi_ret_buffer {
+	u32 flags;
+	u32 value;
+	u8 data[];
+};
+
+struct atk_sitm_buffer {
+	u32 id;
+	u32 value1;
+	u32 value2;
 };
 
 static int atk_add(struct acpi_device *device);
@@ -439,52 +458,140 @@ static int atk_read_value_old(struct atk_sensor_data *sensor, u64 *value)
 	return 0;
 }
 
-static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+static union acpi_object *atk_ggrp(struct atk_data *data, u16 mux)
 {
-	struct atk_data *data = sensor->data;
 	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_buffer buf;
+	acpi_status ret;
 	struct acpi_object_list params;
-	struct acpi_buffer ret;
 	union acpi_object id;
-	struct atk_acpi_buffer_u64 tmp;
-	acpi_status status;
+	union acpi_object *pack;
 
 	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = sensor->id;
-
+	id.integer.value = mux;
 	params.count = 1;
 	params.pointer = &id;
 
-	tmp.buf.type = ACPI_TYPE_BUFFER;
-	tmp.buf.buffer.pointer = (u8 *)&tmp.value;
-	tmp.buf.buffer.length = sizeof(u64);
-	ret.length = sizeof(tmp);
-	ret.pointer = &tmp;
+	buf.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object(data->enumerate_handle, NULL, &params, &buf);
+	if (ret != AE_OK) {
+		dev_err(dev, "GGRP[%#x] ACPI exception: %s\n", mux,
+				acpi_format_exception(ret));
+		return ERR_PTR(-EIO);
+	}
+	pack = buf.pointer;
+	if (pack->type != ACPI_TYPE_PACKAGE) {
+		/* Execution was successful, but the id was not found */
+		ACPI_FREE(pack);
+		return ERR_PTR(-ENOENT);
+	}
+
+	if (pack->package.count < 1) {
+		dev_err(dev, "GGRP[%#x] package is too small\n", mux);
+		ACPI_FREE(pack);
+		return ERR_PTR(-EIO);
+	}
+	return pack;
+}
+
+static union acpi_object *atk_gitm(struct atk_data *data, u64 id)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object tmp;
+	struct acpi_object_list params;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	tmp.type = ACPI_TYPE_INTEGER;
+	tmp.integer.value = id;
+
+	params.count = 1;
+	params.pointer = (void *)&tmp;
 
+	ret.length = ACPI_ALLOCATE_BUFFER;
 	status = acpi_evaluate_object_typed(data->read_handle, NULL, &params,
 			&ret, ACPI_TYPE_BUFFER);
 	if (status != AE_OK) {
-		dev_warn(dev, "%s: ACPI exception: %s\n", __func__,
+		dev_warn(dev, "GITM[%#llx] ACPI exception: %s\n", id,
 				acpi_format_exception(status));
-		return -EIO;
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
 	}
+	return obj;
+}
 
-	/* Return buffer format:
-	 * [0-3] "value" is valid flag
-	 * [4-7] value
-	 */
-	if (!(tmp.value & 0xffffffff)) {
+static union acpi_object *atk_sitm(struct atk_data *data,
+		struct atk_sitm_buffer *buf)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_object_list params;
+	union acpi_object tmp;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	tmp.type = ACPI_TYPE_BUFFER;
+	tmp.buffer.pointer = (u8 *)buf;
+	tmp.buffer.length = sizeof(*buf);
+
+	params.count = 1;
+	params.pointer = &tmp;
+
+	status = acpi_evaluate_object_typed(data->write_handle, NULL, &params,
+			&ret, ACPI_TYPE_BUFFER);
+	if (status != AE_OK) {
+		dev_warn(dev, "SITM[%#x] ACPI exception: %s\n", buf->id,
+				acpi_format_exception(status));
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
+	}
+	return obj;
+}
+
+static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+{
+	struct atk_data *data = sensor->data;
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err = 0;
+
+	obj = atk_gitm(data, sensor->id);
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
+
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (buf->flags == 0) {
 		/* The reading is not valid, possible causes:
 		 * - sensor failure
 		 * - enumeration was FUBAR (and we didn't notice)
 		 */
-		dev_info(dev, "Failure: %#llx\n", tmp.value);
-		return -EIO;
+		dev_warn(dev, "Read failed, sensor = %#llx\n", sensor->id);
+		err = -EIO;
+		goto out;
 	}
 
-	*value = (tmp.value & 0xffffffff00000000ULL) >> 32;
-
-	return 0;
+	*value = buf->value;
+out:
+	ACPI_FREE(obj);
+	return err;
 }
 
 static int atk_read_value(struct atk_sensor_data *sensor, u64 *value)
@@ -713,43 +820,141 @@ cleanup:
 	return ret;
 }
 
-static int atk_enumerate_new_hwmon(struct atk_data *data)
+static int atk_ec_present(struct atk_data *data)
 {
 	struct device *dev = &data->acpi_dev->dev;
-	struct acpi_buffer buf;
-	acpi_status ret;
-	struct acpi_object_list params;
-	union acpi_object id;
 	union acpi_object *pack;
-	int err;
+	union acpi_object *ec;
+	int ret;
 	int i;
 
-	dev_dbg(dev, "Enumerating hwmon sensors\n");
+	pack = atk_ggrp(data, ATK_MUX_MGMT);
+	if (IS_ERR(pack)) {
+		if (PTR_ERR(pack) == -ENOENT) {
+			/* The MGMT class does not exists - that's ok */
+			dev_dbg(dev, "Class %#llx not found\n", ATK_MUX_MGMT);
+			return 0;
+		}
+		return PTR_ERR(pack);
+	}
 
-	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = ATK_MUX_HWMON;
-	params.count = 1;
-	params.pointer = &id;
+	/* Search the EC */
+	ec = NULL;
+	for (i = 0; i < pack->package.count; i++) {
+		union acpi_object *obj = &pack->package.elements[i];
+		union acpi_object *id;
 
-	buf.length = ACPI_ALLOCATE_BUFFER;
-	ret = acpi_evaluate_object_typed(data->enumerate_handle, NULL, &params,
-			&buf, ACPI_TYPE_PACKAGE);
-	if (ret != AE_OK) {
-		dev_warn(dev, METHOD_ENUMERATE ": ACPI exception: %s\n",
-				acpi_format_exception(ret));
-		return -ENODEV;
+		if (obj->type != ACPI_TYPE_PACKAGE)
+			continue;
+
+		id = &obj->package.elements[0];
+		if (id->type != ACPI_TYPE_INTEGER)
+			continue;
+
+		if (id->integer.value == ATK_EC_ID) {
+			ec = obj;
+			break;
+		}
 	}
 
-	/* Result must be a package */
-	pack = buf.pointer;
+	ret = (ec != NULL);
+	if (!ret)
+		/* The system has no EC */
+		dev_dbg(dev, "EC not found\n");
 
-	if (pack->package.count < 1) {
-		dev_dbg(dev, "%s: hwmon package is too small: %d\n", __func__,
-				pack->package.count);
-		err = -EINVAL;
-		goto out;
+	ACPI_FREE(pack);
+	return ret;
+}
+
+static int atk_ec_enabled(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err;
+
+	obj = atk_gitm(data, ATK_EC_ID);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Unable to query EC status\n");
+		return PTR_ERR(obj);
+	}
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+
+	if (buf->flags == 0) {
+		dev_err(dev, "Unable to query EC status\n");
+		err = -EIO;
+	} else {
+		err = (buf->value != 0);
+		dev_dbg(dev, "EC is %sabled\n",
+				err ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_ec_ctl(struct atk_data *data, int enable)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_sitm_buffer sitm;
+	struct atk_acpi_ret_buffer *ec_ret;
+	int err = 0;
+
+	sitm.id = ATK_EC_ID;
+	sitm.value1 = enable;
+	sitm.value2 = 0;
+
+	obj = atk_sitm(data, &sitm);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		return PTR_ERR(obj);
+	}
+	ec_ret = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (ec_ret->flags == 0) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		err = -EIO;
+	} else {
+		dev_info(dev, "EC %sabled\n",
+				enable ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_enumerate_new_hwmon(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *pack;
+	int err;
+	int i;
+
+	err = atk_ec_present(data);
+	if (err < 0)
+		return err;
+	if (err) {
+		err = atk_ec_enabled(data);
+		if (err < 0)
+			return err;
+		/* If the EC was disabled we will disable it again on unload */
+		data->disable_ec = err;
+
+		err = atk_ec_ctl(data, 1);
+		if (err) {
+			data->disable_ec = false;
+			return err;
+		}
 	}
 
+	dev_dbg(dev, "Enumerating hwmon sensors\n");
+
+	pack = atk_ggrp(data, ATK_MUX_HWMON);
+	if (IS_ERR(pack))
+		return PTR_ERR(pack);
+
 	for (i = 0; i < pack->package.count; i++) {
 		union acpi_object *obj = &pack->package.elements[i];
 
@@ -758,8 +963,7 @@ static int atk_enumerate_new_hwmon(struct atk_data *data)
 
 	err = data->voltage_count + data->temperature_count + data->fan_count;
 
-out:
-	ACPI_FREE(buf.pointer);
+	ACPI_FREE(pack);
 	return err;
 }
 
@@ -895,6 +1099,15 @@ static int atk_check_new_if(struct atk_data *data)
 	}
 	data->read_handle = ret;
 
+	/* De-multiplexer (write) */
+	status = acpi_get_handle(data->atk_handle, METHOD_WRITE, &ret);
+	if (status != AE_OK) {
+		dev_dbg(dev, "method " METHOD_READ " not found: %s\n",
+				 acpi_format_exception(status));
+		return -ENODEV;
+	}
+	data->write_handle = ret;
+
 	return 0;
 }
 
@@ -915,6 +1128,7 @@ static int atk_add(struct acpi_device *device)
 	data->acpi_dev = device;
 	data->atk_handle = device->handle;
 	INIT_LIST_HEAD(&data->sensor_list);
+	data->disable_ec = false;
 
 	buf.length = ACPI_ALLOCATE_BUFFER;
 	ret = acpi_evaluate_object_typed(data->atk_handle, BOARD_ID, NULL,
@@ -973,6 +1187,8 @@ static int atk_add(struct acpi_device *device)
 cleanup:
 	atk_free_sensors(data);
 out:
+	if (data->disable_ec)
+		atk_ec_ctl(data, 0);
 	kfree(data);
 	return err;
 }
@@ -988,6 +1204,11 @@ static int atk_remove(struct acpi_device *device, int type)
 	atk_free_sensors(data);
 	hwmon_device_unregister(data->hwmon_dev);
 
+	if (data->disable_ec) {
+		if (atk_ec_ctl(data, 0))
+			dev_err(&device->dev, "Failed to disable EC\n");
+	}
+
 	kfree(data);
 
 	return 0;

thanks,
Luca

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Luca Tettamanti wrote:
> On Thu, Oct 01, 2009 at 08:40:59AM -0600, Robert Hancock wrote:
>> On Thu, Oct 1, 2009 at 12:50 AM, Jean Delvare <khali@linux-fr.org> wrote:
>>> On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
>>>> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
>>>>> Try this one:
>>>>> http://marc.info/?l=linux-kernel&m=125421276407283&w=2
>>>> With that patch it's lasted almost a day with no timeouts, previously
>>>> I was getting about one every hour. Has that patch been submitted?
>>> Do you mean you're still getting timeouts but they are less frequent,
>>> or you did not get any timeout at all so far?
>> So far no timeout at all (after about 36 hours).
> 
> Good. I have a new patch for you then :P
> 
> This version checks whether the EC is already enabled or not before touching it
> and also restore the state when the module is unloaded.
> You should check that the driver keeps working when is unloaded and reloaded.
> 

I tried your latest patch on a P7P55D Deluxe and get this:
> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size 32 (bits) (20090903/dsopcode-596)
> [   10.419125] ACPI Error (psparse-0537): Method parse/execution failed [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception: AE_AML_BUFFER_LIMIT
> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5

Your earlier patch did only report this:
[   10.632195] ATK0110 ATK0110:00: EC enabled


--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> wrote:
> Luca Tettamanti wrote:
>>
>> On Thu, Oct 01, 2009 at 08:40:59AM -0600, Robert Hancock wrote:
>>>
>>> On Thu, Oct 1, 2009 at 12:50 AM, Jean Delvare <khali@linux-fr.org> wrote:
>>>>
>>>> On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
>>>>>
>>>>> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org>
>>>>> wrote:
>>>>>>
>>>>>> Try this one:
>>>>>> http://marc.info/?l=linux-kernel&m=125421276407283&w=2
>>>>>
>>>>> With that patch it's lasted almost a day with no timeouts, previously
>>>>> I was getting about one every hour. Has that patch been submitted?
>>>>
>>>> Do you mean you're still getting timeouts but they are less frequent,
>>>> or you did not get any timeout at all so far?
>>>
>>> So far no timeout at all (after about 36 hours).
>>
>> Good. I have a new patch for you then :P
>>
>> This version checks whether the EC is already enabled or not before
>> touching it
>> and also restore the state when the module is unloaded.
>> You should check that the driver keeps working when is unloaded and
>> reloaded.
>>
>
> I tried your latest patch on a P7P55D Deluxe and get this:
>>
>> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size
>> 32 (bits) (20090903/dsopcode-596)
>> [   10.419125] ACPI Error (psparse-0537): Method parse/execution failed
>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
>> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
>> AE_AML_BUFFER_LIMIT
>> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
>> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5

I see. GITM probably expects the same buffer structure as SITM, though
in older models the upper bits were never used (hence I never noticed
the problem). Working on a patch.

L

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Thu, Oct 01, 2009 at 09:05:36PM +0200, Luca Tettamanti wrote:
> On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> wrote:
> > Luca Tettamanti wrote:
> >>
> >> On Thu, Oct 01, 2009 at 08:40:59AM -0600, Robert Hancock wrote:
> >>>
> >>> On Thu, Oct 1, 2009 at 12:50 AM, Jean Delvare <khali@linux-fr.org> wrote:
> >>>>
> >>>> On Wed, 30 Sep 2009 17:38:21 -0600, Robert Hancock wrote:
> >>>>>
> >>>>> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org>
> >>>>> wrote:
> >>>>>>
> >>>>>> Try this one:
> >>>>>> http://marc.info/?l=linux-kernel&m=125421276407283&w=2
> >>>>>
> >>>>> With that patch it's lasted almost a day with no timeouts, previously
> >>>>> I was getting about one every hour. Has that patch been submitted?
> >>>>
> >>>> Do you mean you're still getting timeouts but they are less frequent,
> >>>> or you did not get any timeout at all so far?
> >>>
> >>> So far no timeout at all (after about 36 hours).
> >>
> >> Good. I have a new patch for you then :P
> >>
> >> This version checks whether the EC is already enabled or not before
> >> touching it
> >> and also restore the state when the module is unloaded.
> >> You should check that the driver keeps working when is unloaded and
> >> reloaded.
> >>
> >
> > I tried your latest patch on a P7P55D Deluxe and get this:
> >>
> >> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size
> >> 32 (bits) (20090903/dsopcode-596)
> >> [   10.419125] ACPI Error (psparse-0537): Method parse/execution failed
> >> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
> >> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
> >> AE_AML_BUFFER_LIMIT
> >> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
> >> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5
> 
> I see. GITM probably expects the same buffer structure as SITM, though
> in older models the upper bits were never used (hence I never noticed
> the problem). Working on a patch.

Ok, here it is:

diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..6cda109 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -35,18 +35,22 @@
 #define METHOD_OLD_ENUM_FAN	"FSIF"
 
 #define ATK_MUX_HWMON		0x00000006ULL
+#define ATK_MUX_MGMT		0x00000011ULL
 
 #define ATK_CLASS_MASK		0xff000000ULL
 #define ATK_CLASS_FREQ_CTL	0x03000000ULL
 #define ATK_CLASS_FAN_CTL	0x04000000ULL
 #define ATK_CLASS_HWMON		0x06000000ULL
+#define ATK_CLASS_MGMT		0x11000000ULL
 
 #define ATK_TYPE_MASK		0x00ff0000ULL
 #define HWMON_TYPE_VOLT		0x00020000ULL
 #define HWMON_TYPE_TEMP		0x00030000ULL
 #define HWMON_TYPE_FAN		0x00040000ULL
 
-#define HWMON_SENSOR_ID_MASK	0x0000ffffULL
+#define ATK_ELEMENT_ID_MASK	0x0000ffffULL
+
+#define ATK_EC_ID		0x11060004ULL
 
 enum atk_pack_member {
 	HWMON_PACK_FLAGS,
@@ -89,6 +93,9 @@ struct atk_data {
 	/* new inteface */
 	acpi_handle enumerate_handle;
 	acpi_handle read_handle;
+	acpi_handle write_handle;
+
+	bool disable_ec;
 
 	int voltage_count;
 	int temperature_count;
@@ -129,9 +136,22 @@ struct atk_sensor_data {
 	char const *acpi_name;
 };
 
-struct atk_acpi_buffer_u64 {
-	union acpi_object buf;
-	u64 value;
+/* Return buffer format:
+ * [0-3] "value" is valid flag
+ * [4-7] value
+ * [8- ] unknown stuff on newer mobos
+ */
+struct atk_acpi_ret_buffer {
+	u32 flags;
+	u32 value;
+	u8 data[];
+};
+
+/* Input buffer used for GITM and SITM methods */
+struct atk_acpi_input_buf {
+	u32 id;
+	u32 param1;
+	u32 param2;
 };
 
 static int atk_add(struct acpi_device *device);
@@ -439,52 +459,146 @@ static int atk_read_value_old(struct atk_sensor_data *sensor, u64 *value)
 	return 0;
 }
 
-static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+static union acpi_object *atk_ggrp(struct atk_data *data, u16 mux)
 {
-	struct atk_data *data = sensor->data;
 	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_buffer buf;
+	acpi_status ret;
 	struct acpi_object_list params;
-	struct acpi_buffer ret;
 	union acpi_object id;
-	struct atk_acpi_buffer_u64 tmp;
-	acpi_status status;
+	union acpi_object *pack;
 
 	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = sensor->id;
-
+	id.integer.value = mux;
 	params.count = 1;
 	params.pointer = &id;
 
-	tmp.buf.type = ACPI_TYPE_BUFFER;
-	tmp.buf.buffer.pointer = (u8 *)&tmp.value;
-	tmp.buf.buffer.length = sizeof(u64);
-	ret.length = sizeof(tmp);
-	ret.pointer = &tmp;
+	buf.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object(data->enumerate_handle, NULL, &params, &buf);
+	if (ret != AE_OK) {
+		dev_err(dev, "GGRP[%#x] ACPI exception: %s\n", mux,
+				acpi_format_exception(ret));
+		return ERR_PTR(-EIO);
+	}
+	pack = buf.pointer;
+	if (pack->type != ACPI_TYPE_PACKAGE) {
+		/* Execution was successful, but the id was not found */
+		ACPI_FREE(pack);
+		return ERR_PTR(-ENOENT);
+	}
+
+	if (pack->package.count < 1) {
+		dev_err(dev, "GGRP[%#x] package is too small\n", mux);
+		ACPI_FREE(pack);
+		return ERR_PTR(-EIO);
+	}
+	return pack;
+}
+
+static union acpi_object *atk_gitm(struct atk_data *data, u64 id)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct atk_acpi_input_buf buf;
+	union acpi_object tmp;
+	struct acpi_object_list params;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	buf.id = id;
+	buf.param1 = 0;
+	buf.param2 = 0;
 
+	tmp.type = ACPI_TYPE_BUFFER;
+	tmp.buffer.pointer = (u8 *)&buf;
+	tmp.buffer.length = sizeof(buf);
+
+	params.count = 1;
+	params.pointer = (void *)&tmp;
+
+	ret.length = ACPI_ALLOCATE_BUFFER;
 	status = acpi_evaluate_object_typed(data->read_handle, NULL, &params,
 			&ret, ACPI_TYPE_BUFFER);
 	if (status != AE_OK) {
-		dev_warn(dev, "%s: ACPI exception: %s\n", __func__,
+		dev_warn(dev, "GITM[%#llx] ACPI exception: %s\n", id,
 				acpi_format_exception(status));
-		return -EIO;
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
 	}
+	return obj;
+}
 
-	/* Return buffer format:
-	 * [0-3] "value" is valid flag
-	 * [4-7] value
-	 */
-	if (!(tmp.value & 0xffffffff)) {
+static union acpi_object *atk_sitm(struct atk_data *data,
+		struct atk_acpi_input_buf *buf)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_object_list params;
+	union acpi_object tmp;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	tmp.type = ACPI_TYPE_BUFFER;
+	tmp.buffer.pointer = (u8 *)buf;
+	tmp.buffer.length = sizeof(*buf);
+
+	params.count = 1;
+	params.pointer = &tmp;
+
+	status = acpi_evaluate_object_typed(data->write_handle, NULL, &params,
+			&ret, ACPI_TYPE_BUFFER);
+	if (status != AE_OK) {
+		dev_warn(dev, "SITM[%#x] ACPI exception: %s\n", buf->id,
+				acpi_format_exception(status));
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
+	}
+	return obj;
+}
+
+static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+{
+	struct atk_data *data = sensor->data;
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err = 0;
+
+	obj = atk_gitm(data, sensor->id);
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
+
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (buf->flags == 0) {
 		/* The reading is not valid, possible causes:
 		 * - sensor failure
 		 * - enumeration was FUBAR (and we didn't notice)
 		 */
-		dev_info(dev, "Failure: %#llx\n", tmp.value);
-		return -EIO;
+		dev_warn(dev, "Read failed, sensor = %#llx\n", sensor->id);
+		err = -EIO;
+		goto out;
 	}
 
-	*value = (tmp.value & 0xffffffff00000000ULL) >> 32;
-
-	return 0;
+	*value = buf->value;
+out:
+	ACPI_FREE(obj);
+	return err;
 }
 
 static int atk_read_value(struct atk_sensor_data *sensor, u64 *value)
@@ -713,43 +827,141 @@ cleanup:
 	return ret;
 }
 
-static int atk_enumerate_new_hwmon(struct atk_data *data)
+static int atk_ec_present(struct atk_data *data)
 {
 	struct device *dev = &data->acpi_dev->dev;
-	struct acpi_buffer buf;
-	acpi_status ret;
-	struct acpi_object_list params;
-	union acpi_object id;
 	union acpi_object *pack;
-	int err;
+	union acpi_object *ec;
+	int ret;
 	int i;
 
-	dev_dbg(dev, "Enumerating hwmon sensors\n");
+	pack = atk_ggrp(data, ATK_MUX_MGMT);
+	if (IS_ERR(pack)) {
+		if (PTR_ERR(pack) == -ENOENT) {
+			/* The MGMT class does not exists - that's ok */
+			dev_dbg(dev, "Class %#llx not found\n", ATK_MUX_MGMT);
+			return 0;
+		}
+		return PTR_ERR(pack);
+	}
 
-	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = ATK_MUX_HWMON;
-	params.count = 1;
-	params.pointer = &id;
+	/* Search the EC */
+	ec = NULL;
+	for (i = 0; i < pack->package.count; i++) {
+		union acpi_object *obj = &pack->package.elements[i];
+		union acpi_object *id;
 
-	buf.length = ACPI_ALLOCATE_BUFFER;
-	ret = acpi_evaluate_object_typed(data->enumerate_handle, NULL, &params,
-			&buf, ACPI_TYPE_PACKAGE);
-	if (ret != AE_OK) {
-		dev_warn(dev, METHOD_ENUMERATE ": ACPI exception: %s\n",
-				acpi_format_exception(ret));
-		return -ENODEV;
+		if (obj->type != ACPI_TYPE_PACKAGE)
+			continue;
+
+		id = &obj->package.elements[0];
+		if (id->type != ACPI_TYPE_INTEGER)
+			continue;
+
+		if (id->integer.value == ATK_EC_ID) {
+			ec = obj;
+			break;
+		}
 	}
 
-	/* Result must be a package */
-	pack = buf.pointer;
+	ret = (ec != NULL);
+	if (!ret)
+		/* The system has no EC */
+		dev_dbg(dev, "EC not found\n");
 
-	if (pack->package.count < 1) {
-		dev_dbg(dev, "%s: hwmon package is too small: %d\n", __func__,
-				pack->package.count);
-		err = -EINVAL;
-		goto out;
+	ACPI_FREE(pack);
+	return ret;
+}
+
+static int atk_ec_enabled(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err;
+
+	obj = atk_gitm(data, ATK_EC_ID);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Unable to query EC status\n");
+		return PTR_ERR(obj);
+	}
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+
+	if (buf->flags == 0) {
+		dev_err(dev, "Unable to query EC status\n");
+		err = -EIO;
+	} else {
+		err = (buf->value != 0);
+		dev_dbg(dev, "EC is %sabled\n",
+				err ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_ec_ctl(struct atk_data *data, int enable)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_input_buf sitm;
+	struct atk_acpi_ret_buffer *ec_ret;
+	int err = 0;
+
+	sitm.id = ATK_EC_ID;
+	sitm.param1 = enable;
+	sitm.param2 = 0;
+
+	obj = atk_sitm(data, &sitm);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		return PTR_ERR(obj);
+	}
+	ec_ret = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (ec_ret->flags == 0) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		err = -EIO;
+	} else {
+		dev_info(dev, "EC %sabled\n",
+				enable ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_enumerate_new_hwmon(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *pack;
+	int err;
+	int i;
+
+	err = atk_ec_present(data);
+	if (err < 0)
+		return err;
+	if (err) {
+		err = atk_ec_enabled(data);
+		if (err < 0)
+			return err;
+		/* If the EC was disabled we will disable it again on unload */
+		data->disable_ec = err;
+
+		err = atk_ec_ctl(data, 1);
+		if (err) {
+			data->disable_ec = false;
+			return err;
+		}
 	}
 
+	dev_dbg(dev, "Enumerating hwmon sensors\n");
+
+	pack = atk_ggrp(data, ATK_MUX_HWMON);
+	if (IS_ERR(pack))
+		return PTR_ERR(pack);
+
 	for (i = 0; i < pack->package.count; i++) {
 		union acpi_object *obj = &pack->package.elements[i];
 
@@ -758,8 +970,7 @@ static int atk_enumerate_new_hwmon(struct atk_data *data)
 
 	err = data->voltage_count + data->temperature_count + data->fan_count;
 
-out:
-	ACPI_FREE(buf.pointer);
+	ACPI_FREE(pack);
 	return err;
 }
 
@@ -895,6 +1106,15 @@ static int atk_check_new_if(struct atk_data *data)
 	}
 	data->read_handle = ret;
 
+	/* De-multiplexer (write) */
+	status = acpi_get_handle(data->atk_handle, METHOD_WRITE, &ret);
+	if (status != AE_OK) {
+		dev_dbg(dev, "method " METHOD_READ " not found: %s\n",
+				 acpi_format_exception(status));
+		return -ENODEV;
+	}
+	data->write_handle = ret;
+
 	return 0;
 }
 
@@ -915,6 +1135,7 @@ static int atk_add(struct acpi_device *device)
 	data->acpi_dev = device;
 	data->atk_handle = device->handle;
 	INIT_LIST_HEAD(&data->sensor_list);
+	data->disable_ec = false;
 
 	buf.length = ACPI_ALLOCATE_BUFFER;
 	ret = acpi_evaluate_object_typed(data->atk_handle, BOARD_ID, NULL,
@@ -973,6 +1194,8 @@ static int atk_add(struct acpi_device *device)
 cleanup:
 	atk_free_sensors(data);
 out:
+	if (data->disable_ec)
+		atk_ec_ctl(data, 0);
 	kfree(data);
 	return err;
 }
@@ -988,6 +1211,11 @@ static int atk_remove(struct acpi_device *device, int type)
 	atk_free_sensors(data);
 	hwmon_device_unregister(data->hwmon_dev);
 
+	if (data->disable_ec) {
+		if (atk_ec_ctl(data, 0))
+			dev_err(&device->dev, "Failed to disable EC\n");
+	}
+
 	kfree(data);
 
 	return 0;

Luca

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Luca Tettamanti wrote:
> On Thu, Oct 01, 2009 at 09:05:36PM +0200, Luca Tettamanti wrote:
>> On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> wrote:
>>>>
>>> I tried your latest patch on a P7P55D Deluxe and get this:
>>>> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size
>>>> 32 (bits) (20090903/dsopcode-596)
>>>> [   10.419125] ACPI Error (psparse-0537): Method parse/execution failed
>>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
>>>> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
>>>> AE_AML_BUFFER_LIMIT
>>>> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
>>>> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5
>> I see. GITM probably expects the same buffer structure as SITM, though
>> in older models the upper bits were never used (hence I never noticed
>> the problem). Working on a patch.
> 
> Ok, here it is:
> 

no go...

[root@thunder ~]# modprobe asus_atk0110
Killed
[root@thunder ~]#
Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697739] Oops: 0002 [#1] PREEMPT SMP

Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697741] last sysfs file: /sys/module/ipv6/initstate

Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697792] Stack:

Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697799] Call Trace:

Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697851] Code: eb 0e 4c 89 e7 e8 a3 ff ff ff 48 89 
43 08 eb 05 48 39 f0 72 1c 48 8b 53 08 b8 04 00 00 00 48 85 d2 74 13 31 
c0 48 89 d7 4c 89 e1 <f3> aa 31 c0 eb 05 b8 0b 00 00 00 5b 41 5c c9 c3 
55 48 8b 3d 27

Message from syslogd@thunder at Fri Oct  2 23:39:34 2009 ...
thunder klogd: [ 2270.697870] CR2: 0000000000000011

--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Thomas Backlund wrote:
> Luca Tettamanti wrote:
>> On Thu, Oct 01, 2009 at 09:05:36PM +0200, Luca Tettamanti wrote:
>>> On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> 
>>> wrote:
>>>>>
>>>> I tried your latest patch on a P7P55D Deluxe and get this:
>>>>> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] 
>>>>> size
>>>>> 32 (bits) (20090903/dsopcode-596)
>>>>> [   10.419125] ACPI Error (psparse-0537): Method parse/execution 
>>>>> failed
>>>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), 
>>>>> AE_AML_BUFFER_LIMIT
>>>>> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
>>>>> AE_AML_BUFFER_LIMIT
>>>>> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
>>>>> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5
>>> I see. GITM probably expects the same buffer structure as SITM, though
>>> in older models the upper bits were never used (hence I never noticed
>>> the problem). Working on a patch.
>>
>> Ok, here it is:
>>
> 
> no go...
> 

Sorry, forgot to post the full bug:

> [ 2270.697725] BUG: unable to handle kernel NULL pointer dereference at 0000000000000011
> [ 2270.697728] IP: [<ffffffff811cc2f1>] acpi_ut_initialize_buffer+0x5c/0x6c
> [ 2270.697734] PGD 2300a7067 PUD 23641e067 PMD 0 
> [ 2270.697739] Oops: 0002 [#1] PREEMPT SMP 
> [ 2270.697741] last sysfs file: /sys/module/ipv6/initstate
> [ 2270.697742] CPU 3 
> [ 2270.697742] Modules linked in: asus_atk0110(+) af_packet bonding ipv6 binfmt_misc loop xfs exportfs usblp joydev hid_logitech ff_memless usbhid cpufreq_ondemand hid cpufreq_conservative cpufreq_powersave acpi_cpufreq freq_table radeon ttm drm_kms_helper drm i2c_algo_bit snd_hda_codec_atihdmi snd_hda_codec_via snd_hda_intel firewire_ohci snd_hda_codec firewire_core crc_itu_t snd_seq_dummy snd_hwdep snd_seq_oss snd_seq_midi_event snd_seq snd_seq_device snd_pcm_oss snd_pcm snd_timer snd_mixer_oss snd ohci1394 soundcore ieee1394 r8169 snd_page_alloc i2c_i801 ehci_hcd i2c_core pcspkr mii serio_raw sr_mod sg wmi rtc_cmos processor button thermal evdev usbcore pata_jmicron dm_snapshot dm_zero dm_mirror dm_region_hash dm_log dm_mod ata_piix ahci libata sd_mod scsi_mod crc_t10dif ext4 jbd2 crc16 [last unloaded: scsi_wait_scan]
> [ 2270.697773] Pid: 20267, comm: modprobe Tainted: G        W  2.6.31.1-tmb-desktop-9mdv #1 System Product Name
> [ 2270.697775] RIP: 0010:[<ffffffff811cc2f1>]  [<ffffffff811cc2f1>] acpi_ut_initialize_buffer+0x5c/0x6c
> [ 2270.697777] RSP: 0018:ffff880232d47ba8  EFLAGS: 00010246
> [ 2270.697778] RAX: 0000000000000000 RBX: ffff880232d47c88 RCX: 0000000000000218
> [ 2270.697779] RDX: 0000000000000011 RSI: 0000000000000218 RDI: 0000000000000011
> [ 2270.697780] RBP: ffff880232d47bb8 R08: 0000000000000000 R09: ffff88023f8512d0
> [ 2270.697782] R10: 0000000000000002 R11: 0000000000000000 R12: 0000000000000218
> [ 2270.697783] R13: ffff880232d47c98 R14: ffff880236c42640 R15: ffff880232d47c88
> [ 2270.697784] FS:  00007fb0a9b876f0(0000) GS:ffff88002807f000(0000) knlGS:0000000000000000
> [ 2270.697786] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 2270.697787] CR2: 0000000000000011 CR3: 0000000231479000 CR4: 00000000000006e0
> [ 2270.697788] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 2270.697789] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 2270.697791] Process modprobe (pid: 20267, threadinfo ffff880232d46000, task ffff88023e7b4980)
> [ 2270.697792] Stack:
> [ 2270.697792]  0000000000000000 ffff88023f8147c0 ffff880232d47c18 ffffffff811c4a71
> [ 2270.697794] <0> ffff880232d47c28 ffffffff00000000 ffff880238843dd8 0000000000000218
> [ 2270.697796] <0> ffff880232d47c08 ffff880232d47c88 ffff880200000001 ffff88023917f2e0
> [ 2270.697799] Call Trace:
> [ 2270.697802]  [<ffffffff811c4a71>] acpi_evaluate_object+0x191/0x1f2
> [ 2270.697804]  [<ffffffff811c4afa>] acpi_evaluate_object_typed+0x28/0xd2
> [ 2270.697807]  [<ffffffffa04bb30c>] atk_ec_ctl+0x7c/0x220 [asus_atk0110]
> [ 2270.697809]  [<ffffffffa04bb1e9>] ? atk_gitm+0x79/0x120 [asus_atk0110]
> [ 2270.697811]  [<ffffffffa04bc1b2>] atk_add+0x232/0x72c [asus_atk0110]
> [ 2270.697814]  [<ffffffff81128d0b>] ? sysfs_do_create_link+0xcb/0x170
> [ 2270.697818]  [<ffffffff811aee48>] acpi_device_probe+0x4b/0x11d
> [ 2270.697821]  [<ffffffff812070e6>] driver_probe_device+0x86/0x180
> [ 2270.697823]  [<ffffffff812071e0>] ? __driver_attach+0x0/0xa0
> [ 2270.697825]  [<ffffffff81207273>] __driver_attach+0x93/0xa0
> [ 2270.697826]  [<ffffffff812071e0>] ? __driver_attach+0x0/0xa0
> [ 2270.697828]  [<ffffffff81206658>] bus_for_each_dev+0x68/0x90
> [ 2270.697830]  [<ffffffff81206f49>] driver_attach+0x19/0x20
> [ 2270.697832]  [<ffffffff812068f8>] bus_add_driver+0xb8/0x250
> [ 2270.697833]  [<ffffffff81207568>] driver_register+0x78/0x140
> [ 2270.697835]  [<ffffffffa016c000>] ? atk0110_init+0x0/0x31 [asus_atk0110]
> [ 2270.697837]  [<ffffffff811af6bf>] acpi_bus_register_driver+0x3e/0x40
> [ 2270.697839]  [<ffffffffa016c015>] atk0110_init+0x15/0x31 [asus_atk0110]
> [ 2270.697845]  [<ffffffff81009047>] do_one_initcall+0x37/0x1a0
> [ 2270.697848]  [<ffffffff81071cc9>] sys_init_module+0xd9/0x230
> [ 2270.697850]  [<ffffffff8100bf2b>] system_call_fastpath+0x16/0x1b
> [ 2270.697851] Code: eb 0e 4c 89 e7 e8 a3 ff ff ff 48 89 43 08 eb 05 48 39 f0 72 1c 48 8b 53 08 b8 04 00 00 00 48 85 d2 74 13 31 c0 48 89 d7 4c 89 e1 <f3> aa 31 c0 eb 05 b8 0b 00 00 00 5b 41 5c c9 c3 55 48 8b 3d 27 
> [ 2270.697867] RIP  [<ffffffff811cc2f1>] acpi_ut_initialize_buffer+0x5c/0x6c
> [ 2270.697870]  RSP <ffff880232d47ba8>
> [ 2270.697870] CR2: 0000000000000011
> [ 2270.697957] ---[ end trace f03c8442c43e630d ]---

--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Fri, Oct 02, 2009 at 11:43:16PM +0300, Thomas Backlund wrote:
> Luca Tettamanti wrote:
> >On Thu, Oct 01, 2009 at 09:05:36PM +0200, Luca Tettamanti wrote:
> >>On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> wrote:
> >>>>
> >>>I tried your latest patch on a P7P55D Deluxe and get this:
> >>>>[   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size
> >>>>32 (bits) (20090903/dsopcode-596)
> >>>>[   10.419125] ACPI Error (psparse-0537): Method parse/execution failed
> >>>>[\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
> >>>>[   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
> >>>>AE_AML_BUFFER_LIMIT
> >>>>[   10.419158] ATK0110 ATK0110:00: Unable to query EC status
> >>>>[   10.419161] ATK0110: probe of ATK0110:00 failed with error -5
> >>I see. GITM probably expects the same buffer structure as SITM, though
> >>in older models the upper bits were never used (hence I never noticed
> >>the problem). Working on a patch.
> >
> >Ok, here it is:
> 
> no go...

Ok, I should have fixed the OOPS.

diff --git a/drivers/hwmon/asus_atk0110.c b/drivers/hwmon/asus_atk0110.c
index fe4fa29..5a3ee00 100644
--- a/drivers/hwmon/asus_atk0110.c
+++ b/drivers/hwmon/asus_atk0110.c
@@ -35,18 +35,22 @@
 #define METHOD_OLD_ENUM_FAN	"FSIF"
 
 #define ATK_MUX_HWMON		0x00000006ULL
+#define ATK_MUX_MGMT		0x00000011ULL
 
 #define ATK_CLASS_MASK		0xff000000ULL
 #define ATK_CLASS_FREQ_CTL	0x03000000ULL
 #define ATK_CLASS_FAN_CTL	0x04000000ULL
 #define ATK_CLASS_HWMON		0x06000000ULL
+#define ATK_CLASS_MGMT		0x11000000ULL
 
 #define ATK_TYPE_MASK		0x00ff0000ULL
 #define HWMON_TYPE_VOLT		0x00020000ULL
 #define HWMON_TYPE_TEMP		0x00030000ULL
 #define HWMON_TYPE_FAN		0x00040000ULL
 
-#define HWMON_SENSOR_ID_MASK	0x0000ffffULL
+#define ATK_ELEMENT_ID_MASK	0x0000ffffULL
+
+#define ATK_EC_ID		0x11060004ULL
 
 enum atk_pack_member {
 	HWMON_PACK_FLAGS,
@@ -89,6 +93,9 @@ struct atk_data {
 	/* new inteface */
 	acpi_handle enumerate_handle;
 	acpi_handle read_handle;
+	acpi_handle write_handle;
+
+	bool disable_ec;
 
 	int voltage_count;
 	int temperature_count;
@@ -129,9 +136,22 @@ struct atk_sensor_data {
 	char const *acpi_name;
 };
 
-struct atk_acpi_buffer_u64 {
-	union acpi_object buf;
-	u64 value;
+/* Return buffer format:
+ * [0-3] "value" is valid flag
+ * [4-7] value
+ * [8- ] unknown stuff on newer mobos
+ */
+struct atk_acpi_ret_buffer {
+	u32 flags;
+	u32 value;
+	u8 data[];
+};
+
+/* Input buffer used for GITM and SITM methods */
+struct atk_acpi_input_buf {
+	u32 id;
+	u32 param1;
+	u32 param2;
 };
 
 static int atk_add(struct acpi_device *device);
@@ -439,52 +459,147 @@ static int atk_read_value_old(struct atk_sensor_data *sensor, u64 *value)
 	return 0;
 }
 
-static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+static union acpi_object *atk_ggrp(struct atk_data *data, u16 mux)
 {
-	struct atk_data *data = sensor->data;
 	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_buffer buf;
+	acpi_status ret;
 	struct acpi_object_list params;
-	struct acpi_buffer ret;
 	union acpi_object id;
-	struct atk_acpi_buffer_u64 tmp;
-	acpi_status status;
+	union acpi_object *pack;
 
 	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = sensor->id;
-
+	id.integer.value = mux;
 	params.count = 1;
 	params.pointer = &id;
 
-	tmp.buf.type = ACPI_TYPE_BUFFER;
-	tmp.buf.buffer.pointer = (u8 *)&tmp.value;
-	tmp.buf.buffer.length = sizeof(u64);
-	ret.length = sizeof(tmp);
-	ret.pointer = &tmp;
+	buf.length = ACPI_ALLOCATE_BUFFER;
+	ret = acpi_evaluate_object(data->enumerate_handle, NULL, &params, &buf);
+	if (ret != AE_OK) {
+		dev_err(dev, "GGRP[%#x] ACPI exception: %s\n", mux,
+				acpi_format_exception(ret));
+		return ERR_PTR(-EIO);
+	}
+	pack = buf.pointer;
+	if (pack->type != ACPI_TYPE_PACKAGE) {
+		/* Execution was successful, but the id was not found */
+		ACPI_FREE(pack);
+		return ERR_PTR(-ENOENT);
+	}
+
+	if (pack->package.count < 1) {
+		dev_err(dev, "GGRP[%#x] package is too small\n", mux);
+		ACPI_FREE(pack);
+		return ERR_PTR(-EIO);
+	}
+	return pack;
+}
+
+static union acpi_object *atk_gitm(struct atk_data *data, u64 id)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct atk_acpi_input_buf buf;
+	union acpi_object tmp;
+	struct acpi_object_list params;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	buf.id = id;
+	buf.param1 = 0;
+	buf.param2 = 0;
 
+	tmp.type = ACPI_TYPE_BUFFER;
+	tmp.buffer.pointer = (u8 *)&buf;
+	tmp.buffer.length = sizeof(buf);
+
+	params.count = 1;
+	params.pointer = (void *)&tmp;
+
+	ret.length = ACPI_ALLOCATE_BUFFER;
 	status = acpi_evaluate_object_typed(data->read_handle, NULL, &params,
 			&ret, ACPI_TYPE_BUFFER);
 	if (status != AE_OK) {
-		dev_warn(dev, "%s: ACPI exception: %s\n", __func__,
+		dev_warn(dev, "GITM[%#llx] ACPI exception: %s\n", id,
 				acpi_format_exception(status));
-		return -EIO;
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
 	}
+	return obj;
+}
 
-	/* Return buffer format:
-	 * [0-3] "value" is valid flag
-	 * [4-7] value
-	 */
-	if (!(tmp.value & 0xffffffff)) {
+static union acpi_object *atk_sitm(struct atk_data *data,
+		struct atk_acpi_input_buf *buf)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	struct acpi_object_list params;
+	union acpi_object tmp;
+	struct acpi_buffer ret;
+	union acpi_object *obj;
+	acpi_status status;
+
+	tmp.type = ACPI_TYPE_BUFFER;
+	tmp.buffer.pointer = (u8 *)buf;
+	tmp.buffer.length = sizeof(*buf);
+
+	params.count = 1;
+	params.pointer = &tmp;
+
+	ret.length = ACPI_ALLOCATE_BUFFER;
+	status = acpi_evaluate_object_typed(data->write_handle, NULL, &params,
+			&ret, ACPI_TYPE_BUFFER);
+	if (status != AE_OK) {
+		dev_warn(dev, "SITM[%#x] ACPI exception: %s\n", buf->id,
+				acpi_format_exception(status));
+		return ERR_PTR(-EIO);
+	}
+	obj = ret.pointer;
+
+	/* Sanity check */
+	if (obj->buffer.length < 8) {
+		dev_warn(dev, "Unexpected ASBF length: %u\n",
+				obj->buffer.length);
+		ACPI_FREE(obj);
+		return ERR_PTR(-EIO);
+	}
+	return obj;
+}
+
+static int atk_read_value_new(struct atk_sensor_data *sensor, u64 *value)
+{
+	struct atk_data *data = sensor->data;
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err = 0;
+
+	obj = atk_gitm(data, sensor->id);
+	if (IS_ERR(obj))
+		return PTR_ERR(obj);
+
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (buf->flags == 0) {
 		/* The reading is not valid, possible causes:
 		 * - sensor failure
 		 * - enumeration was FUBAR (and we didn't notice)
 		 */
-		dev_info(dev, "Failure: %#llx\n", tmp.value);
-		return -EIO;
+		dev_warn(dev, "Read failed, sensor = %#llx\n", sensor->id);
+		err = -EIO;
+		goto out;
 	}
 
-	*value = (tmp.value & 0xffffffff00000000ULL) >> 32;
-
-	return 0;
+	*value = buf->value;
+out:
+	ACPI_FREE(obj);
+	return err;
 }
 
 static int atk_read_value(struct atk_sensor_data *sensor, u64 *value)
@@ -713,43 +828,141 @@ cleanup:
 	return ret;
 }
 
-static int atk_enumerate_new_hwmon(struct atk_data *data)
+static int atk_ec_present(struct atk_data *data)
 {
 	struct device *dev = &data->acpi_dev->dev;
-	struct acpi_buffer buf;
-	acpi_status ret;
-	struct acpi_object_list params;
-	union acpi_object id;
 	union acpi_object *pack;
-	int err;
+	union acpi_object *ec;
+	int ret;
 	int i;
 
-	dev_dbg(dev, "Enumerating hwmon sensors\n");
+	pack = atk_ggrp(data, ATK_MUX_MGMT);
+	if (IS_ERR(pack)) {
+		if (PTR_ERR(pack) == -ENOENT) {
+			/* The MGMT class does not exists - that's ok */
+			dev_dbg(dev, "Class %#llx not found\n", ATK_MUX_MGMT);
+			return 0;
+		}
+		return PTR_ERR(pack);
+	}
 
-	id.type = ACPI_TYPE_INTEGER;
-	id.integer.value = ATK_MUX_HWMON;
-	params.count = 1;
-	params.pointer = &id;
+	/* Search the EC */
+	ec = NULL;
+	for (i = 0; i < pack->package.count; i++) {
+		union acpi_object *obj = &pack->package.elements[i];
+		union acpi_object *id;
 
-	buf.length = ACPI_ALLOCATE_BUFFER;
-	ret = acpi_evaluate_object_typed(data->enumerate_handle, NULL, &params,
-			&buf, ACPI_TYPE_PACKAGE);
-	if (ret != AE_OK) {
-		dev_warn(dev, METHOD_ENUMERATE ": ACPI exception: %s\n",
-				acpi_format_exception(ret));
-		return -ENODEV;
+		if (obj->type != ACPI_TYPE_PACKAGE)
+			continue;
+
+		id = &obj->package.elements[0];
+		if (id->type != ACPI_TYPE_INTEGER)
+			continue;
+
+		if (id->integer.value == ATK_EC_ID) {
+			ec = obj;
+			break;
+		}
 	}
 
-	/* Result must be a package */
-	pack = buf.pointer;
+	ret = (ec != NULL);
+	if (!ret)
+		/* The system has no EC */
+		dev_dbg(dev, "EC not found\n");
 
-	if (pack->package.count < 1) {
-		dev_dbg(dev, "%s: hwmon package is too small: %d\n", __func__,
-				pack->package.count);
-		err = -EINVAL;
-		goto out;
+	ACPI_FREE(pack);
+	return ret;
+}
+
+static int atk_ec_enabled(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_ret_buffer *buf;
+	int err;
+
+	obj = atk_gitm(data, ATK_EC_ID);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Unable to query EC status\n");
+		return PTR_ERR(obj);
+	}
+	buf = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+
+	if (buf->flags == 0) {
+		dev_err(dev, "Unable to query EC status\n");
+		err = -EIO;
+	} else {
+		err = (buf->value != 0);
+		dev_dbg(dev, "EC is %sabled\n",
+				err ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_ec_ctl(struct atk_data *data, int enable)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *obj;
+	struct atk_acpi_input_buf sitm;
+	struct atk_acpi_ret_buffer *ec_ret;
+	int err = 0;
+
+	sitm.id = ATK_EC_ID;
+	sitm.param1 = enable;
+	sitm.param2 = 0;
+
+	obj = atk_sitm(data, &sitm);
+	if (IS_ERR(obj)) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		return PTR_ERR(obj);
+	}
+	ec_ret = (struct atk_acpi_ret_buffer *)obj->buffer.pointer;
+	if (ec_ret->flags == 0) {
+		dev_err(dev, "Failed to %sable the EC\n",
+				enable ? "en" : "dis");
+		err = -EIO;
+	} else {
+		dev_info(dev, "EC %sabled\n",
+				enable ? "en" : "dis");
+	}
+
+	ACPI_FREE(obj);
+	return err;
+}
+
+static int atk_enumerate_new_hwmon(struct atk_data *data)
+{
+	struct device *dev = &data->acpi_dev->dev;
+	union acpi_object *pack;
+	int err;
+	int i;
+
+	err = atk_ec_present(data);
+	if (err < 0)
+		return err;
+	if (err) {
+		err = atk_ec_enabled(data);
+		if (err < 0)
+			return err;
+		/* If the EC was disabled we will disable it again on unload */
+		data->disable_ec = err;
+
+		err = atk_ec_ctl(data, 1);
+		if (err) {
+			data->disable_ec = false;
+			return err;
+		}
 	}
 
+	dev_dbg(dev, "Enumerating hwmon sensors\n");
+
+	pack = atk_ggrp(data, ATK_MUX_HWMON);
+	if (IS_ERR(pack))
+		return PTR_ERR(pack);
+
 	for (i = 0; i < pack->package.count; i++) {
 		union acpi_object *obj = &pack->package.elements[i];
 
@@ -758,8 +971,7 @@ static int atk_enumerate_new_hwmon(struct atk_data *data)
 
 	err = data->voltage_count + data->temperature_count + data->fan_count;
 
-out:
-	ACPI_FREE(buf.pointer);
+	ACPI_FREE(pack);
 	return err;
 }
 
@@ -895,6 +1107,15 @@ static int atk_check_new_if(struct atk_data *data)
 	}
 	data->read_handle = ret;
 
+	/* De-multiplexer (write) */
+	status = acpi_get_handle(data->atk_handle, METHOD_WRITE, &ret);
+	if (status != AE_OK) {
+		dev_dbg(dev, "method " METHOD_READ " not found: %s\n",
+				 acpi_format_exception(status));
+		return -ENODEV;
+	}
+	data->write_handle = ret;
+
 	return 0;
 }
 
@@ -915,6 +1136,7 @@ static int atk_add(struct acpi_device *device)
 	data->acpi_dev = device;
 	data->atk_handle = device->handle;
 	INIT_LIST_HEAD(&data->sensor_list);
+	data->disable_ec = false;
 
 	buf.length = ACPI_ALLOCATE_BUFFER;
 	ret = acpi_evaluate_object_typed(data->atk_handle, BOARD_ID, NULL,
@@ -973,6 +1195,8 @@ static int atk_add(struct acpi_device *device)
 cleanup:
 	atk_free_sensors(data);
 out:
+	if (data->disable_ec)
+		atk_ec_ctl(data, 0);
 	kfree(data);
 	return err;
 }
@@ -988,6 +1212,11 @@ static int atk_remove(struct acpi_device *device, int type)
 	atk_free_sensors(data);
 	hwmon_device_unregister(data->hwmon_dev);
 
+	if (data->disable_ec) {
+		if (atk_ec_ctl(data, 0))
+			dev_err(&device->dev, "Failed to disable EC\n");
+	}
+
 	kfree(data);
 
 	return 0;


Luca

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Luca Tettamanti wrote:
> On Fri, Oct 02, 2009 at 11:43:16PM +0300, Thomas Backlund wrote:
>> Luca Tettamanti wrote:
>>> On Thu, Oct 01, 2009 at 09:05:36PM +0200, Luca Tettamanti wrote:
>>>> On Thu, Oct 1, 2009 at 6:21 PM, Thomas Backlund <tmb@mandriva.org> wrote:
>>>>> I tried your latest patch on a P7P55D Deluxe and get this:
>>>>>> [   10.419119] ACPI Error: Field [PR11] at 64 exceeds Buffer [NULL] size
>>>>>> 32 (bits) (20090903/dsopcode-596)
>>>>>> [   10.419125] ACPI Error (psparse-0537): Method parse/execution failed
>>>>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88023f8147a0), AE_AML_BUFFER_LIMIT
>>>>>> [   10.419157] ATK0110 ATK0110:00: GITM[0x11060004] ACPI exception:
>>>>>> AE_AML_BUFFER_LIMIT
>>>>>> [   10.419158] ATK0110 ATK0110:00: Unable to query EC status
>>>>>> [   10.419161] ATK0110: probe of ATK0110:00 failed with error -5
>>>> I see. GITM probably expects the same buffer structure as SITM, though
>>>> in older models the upper bits were never used (hence I never noticed
>>>> the problem). Working on a patch.
>>> Ok, here it is:
>> no go...
> 
> Ok, I should have fixed the OOPS.
> 

Yep.
This one works so...

Tested-by: Thomas Backlund <tmb@mandriva.org>


Thanks!

--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Robert Hancock]

On Mon, Oct 5, 2009 at 11:26 AM, Thomas Backlund <tmb@mandriva.org> wrote:
>>>>> I see. GITM probably expects the same buffer structure as SITM, though
>>>>> in older models the upper bits were never used (hence I never noticed
>>>>> the problem). Working on a patch.
>>>>
>>>> Ok, here it is:
>>>
>>> no go...
>>
>> Ok, I should have fixed the OOPS.
>>
>
> Yep.
> This one works so...
>
> Tested-by: Thomas Backlund <tmb@mandriva.org>

Works for me as well.

Tested-by: Robert Hancock <hancockrwd@gmail.com>

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Thomas Backlund]

Robert Hancock wrote:
> On Tue, Sep 29, 2009 at 8:49 AM, Thomas Backlund <tmb@mandriva.org> wrote:
>> Try this one:
>> http://marc.info/?l=linux-kernel&m=125421276407283&w=2
> 
> With that patch it's lasted almost a day with no timeouts, previously
> I was getting about one every hour. Has that patch been submitted?
> .

I assume it will come through the acpi tree as it's a fix to an (other) 
acpi ec regression...

--
Thomas

Re: [PATCH] asus_atk0110: add support for Asus P7P55D

[Luca Tettamanti]

On Tue, Sep 29, 2009 at 4:40 PM, Robert Hancock <hancockrwd@gmail.com> wrote:
> On Tue, Sep 29, 2009 at 8:07 AM, Luca Tettamanti <kronos.it@gmail.com> wrote:
>>>>> Hi Robert,
>>>>> I have a new patch for you :)
>>>>> It contains the previous changes to handle the bigger ASBF buffer plus a new
>>>>> method to enable the EC as suggested by Asus. Be sure to compile with
>>>>> HWMON_DEBUG_CHIP enabled.
>>>>
>>>> Excellent.. seems to work now and give actually updating sensor readings :-)
>>>
>>> Have seen a couple of these though, looks like about once an hour:
>>>
>>> ACPI Exception: AE_TIME, Returned by Handler for [EmbeddedControl]
>>> (20090903/evregion-424)
>>> ACPI Error (psparse-0537): Method parse/execution failed
>>> [\_SB_.PCI0.SBRG.ASOC.GIT6] (Node ffff88013ba778c0), AE_TIME
>>> ACPI Error (psparse-0537): Method parse/execution failed
>>> [\_SB_.PCI0.SBRG.ASOC.GITM] (Node ffff88013ba6ea40), AE_TIME
>>> ATK0110 ATK0110:00: atk_read_value_new: ACPI exception: AE_TIME
>>>
>>> Maybe sometimes the embedded controller takes longer than the timeout
>>> to process, or something?
>>
>> Is there a message like "input buffer is not empty" before that?
>
> Nope, that's all I'm getting each time it happens. Suggestions/debug
> patches welcome..

drivers/acpi/ec.c:30, uncomment "#define DEBUG"
Next step would be rising the timeouts and/or retry count in ec_poll
(will ask support from ACPI gurus...)

L