ACPI patches for 2.6.28-rc3

ACPI patches for 2.6.28-rc3

[Len Brown]

please speak up if you see any reason these
pathces should not go into 2.6.28-rc3.

thanks,
-Len

[PATCH 01/21] i7300_idle: Kconfig, show menu only on x86_64

[Len Brown]

From: Venki Pallipadi <venkatesh.pallipadi@intel.com>

...since today it contains only a single driver
which is visible to just x86_64

Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/idle/Kconfig |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/idle/Kconfig b/drivers/idle/Kconfig
index 108264d..f15e90a 100644
--- a/drivers/idle/Kconfig
+++ b/drivers/idle/Kconfig
@@ -1,5 +1,6 @@
 
 menu "Memory power savings"
+depends on X86_64
 
 config I7300_IDLE_IOAT_CHANNEL
 	bool
@@ -7,7 +8,7 @@ config I7300_IDLE_IOAT_CHANNEL
 config I7300_IDLE
 	tristate "Intel chipset idle memory power saving driver"
 	select I7300_IDLE_IOAT_CHANNEL
-	depends on X86_64 && EXPERIMENTAL
+	depends on EXPERIMENTAL
 	help
 	  Enable memory power savings when idle with certain Intel server
 	  chipsets. The chipset must have I/O AT support, such as the
-- 
1.5.6.5

[PATCH 02/21] ACPI: EC: revert msleep patch

[Len Brown]

From: Alexey Starikovskiy <astarikovskiy@suse.de>

With the better solution for EC interrupt storm issue,
there is no need to use msleep over udelay.

References:
	http://bugzilla.kernel.org/show_bug.cgi?id=11810
	http://bugzilla.kernel.org/show_bug.cgi?id=10724

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/ec.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index ef42316..3ef5b79 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -239,10 +239,10 @@ static int ec_check_sci(struct acpi_ec *ec, u8 state)
 static int ec_poll(struct acpi_ec *ec)
 {
 	unsigned long delay = jiffies + msecs_to_jiffies(ACPI_EC_DELAY);
-	msleep(1);
+	udelay(ACPI_EC_UDELAY);
 	while (time_before(jiffies, delay)) {
 		gpe_transaction(ec, acpi_ec_read_status(ec));
-		msleep(1);
+		udelay(ACPI_EC_UDELAY);
 		if (ec_transaction_done(ec))
 			return 0;
 	}
-- 
1.5.6.5

[PATCH 03/21] sony-laptop: Ignore missing _DIS method on pic device

[Len Brown]

From: Matthew Garrett <mjg59@srcf.ucam.org>

At least the Vaio VGN-Z540N doesn't have this method, so let's not fail
to suspend just because it doesn't exist.

Signed-off-by: Adam Jackson <ajax@redhat.com>
Acked-by: Mattia Dongili <malattia@linux.it>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/misc/sony-laptop.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/sony-laptop.c b/drivers/misc/sony-laptop.c
index 5a97d3a..f483c42 100644
--- a/drivers/misc/sony-laptop.c
+++ b/drivers/misc/sony-laptop.c
@@ -2315,8 +2315,10 @@ end:
  */
 static int sony_pic_disable(struct acpi_device *device)
 {
-	if (ACPI_FAILURE(acpi_evaluate_object(device->handle,
-			"_DIS", NULL, NULL)))
+	acpi_status ret = acpi_evaluate_object(device->handle, "_DIS", NULL,
+					       NULL);
+
+	if (ACPI_FAILURE(ret) && ret != AE_NOT_FOUND)
 		return -ENXIO;
 
 	dprintk("Device disabled\n");
-- 
1.5.6.5

[PATCH 04/21] ACPI: fix de-reference bug in power resource driver

[Len Brown]

From: Zhao Yakui <yakui.zhao@intel.com>

change state to *state in the function of acpi_power_get_state()

Signed-off-by: yakui.zhao@intel.com
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/power.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index a1718e5..30d4a52 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -153,7 +153,8 @@ static int acpi_power_get_state(acpi_handle handle, int *state)
 			      ACPI_POWER_RESOURCE_STATE_OFF;
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Resource [%s] is %s\n",
-			  acpi_ut_get_node_name(handle), state ? "on" : "off"));
+			  acpi_ut_get_node_name(handle),
+				*state ? "on" : "off"));
 
 	return 0;
 }
-- 
1.5.6.5

[PATCH 05/21] ACPI: fan: Delete the strict check in power transition

[Len Brown]

From: Zhao Yakui <yakui.zhao@intel.com>

On some laptops the Fan device is turned on/off by controlling the
corresponding power resource. For example: If the power resource
defined in _PR0 object is turned off, it indicates that the FAN device
is in off state(the ACPI state is in D3 state).

Maybe the device is already in D3 state and expected to be transited to
D3 state. As there is no _PR3 object, the power transition can't be
finished and it will be switched to the Unknown state.

Maybe it is more reasonable that the strick check in power transistion
is deleted.

http://bugzilla.kernel.org/show_bug.cgi?id=9485

Signed-off-by: yakui.zhao@intel.com
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/power.c |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index 30d4a52..89111cd 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -517,11 +517,6 @@ int acpi_power_transition(struct acpi_device *device, int state)
 	cl = &device->power.states[device->power.state].resources;
 	tl = &device->power.states[state].resources;
 
-	if (!cl->count && !tl->count) {
-		result = -ENODEV;
-		goto end;
-	}
-
 	/* TBD: Resources must be ordered. */
 
 	/*
-- 
1.5.6.5

[PATCH 06/21] ACPI: bugfix reporting of event handler status

[Len Brown]

From: Zhang Rui <rui.zhang@intel.com>

Introduce a new flag showing whether the event has an event handler/method.

For all the GPEs and Fixed Events,
 1. ACPI_EVENT_FLAG_HANDLE is cleared, it's an "invalid" ACPI event.
 2. Both ACPI_EVENT_FLAG_HANDLE and ACPI_EVENT_FLAG_DISABLE are set,
    it's "disabled".
 3. Both ACPI_EVENT_FLAG_HANDLE and ACPI_EVENT_FLAG_ENABLE are set,
    it's "enabled".
 4. Both ACPI_EVENT_FLAG_HANDLE and ACPI_EVENT_FLAG_WAKE_ENABLE are set,
    it's "wake_enabled".

Among other things, this prevents incorrect reporting of ACPI events
as being "invalid" when it's really just (temporarily) "disabled".

Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: David Brownell <dbrownell@users.sourceforge.net>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 Documentation/ABI/testing/sysfs-firmware-acpi |   16 ++++++++--------
 drivers/acpi/events/evxfevnt.c                |    6 ++++++
 drivers/acpi/system.c                         |   19 +++++++------------
 include/acpi/actypes.h                        |    1 +
 4 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-firmware-acpi b/Documentation/ABI/testing/sysfs-firmware-acpi
index f27be7d..e8ffc70 100644
--- a/Documentation/ABI/testing/sysfs-firmware-acpi
+++ b/Documentation/ABI/testing/sysfs-firmware-acpi
@@ -89,7 +89,7 @@ Description:
 
 		error - an interrupt that can't be accounted for above.
 
-		invalid: it's either a wakeup GPE or a GPE/Fixed Event that
+		invalid: it's either a GPE or a Fixed Event that
 			doesn't have an event handler.
 
 		disable: the GPE/Fixed Event is valid but disabled.
@@ -117,30 +117,30 @@ Description:
 		and other user space applications so that the machine won't shutdown
 		when pressing the power button.
 		# cat ff_pwr_btn
-		0
+		0	enabled
 		# press the power button for 3 times;
 		# cat ff_pwr_btn
-		3
+		3	enabled
 		# echo disable > ff_pwr_btn
 		# cat ff_pwr_btn
-		disable
+		3	disabled
 		# press the power button for 3 times;
 		# cat ff_pwr_btn
-		disable
+		3	disabled
 		# echo enable > ff_pwr_btn
 		# cat ff_pwr_btn
-		4
+		4	enabled
 		/*
 		 * this is because the status bit is set even if the enable bit is cleared,
 		 * and it triggers an ACPI fixed event when the enable bit is set again
 		 */
 		# press the power button for 3 times;
 		# cat ff_pwr_btn
-		7
+		7	enabled
 		# echo disable > ff_pwr_btn
 		# press the power button for 3 times;
 		# echo clear > ff_pwr_btn	/* clear the status bit */
 		# echo disable > ff_pwr_btn
 		# cat ff_pwr_btn
-		7
+		7	enabled
 
diff --git a/drivers/acpi/events/evxfevnt.c b/drivers/acpi/events/evxfevnt.c
index 73bfd6b..211e93a 100644
--- a/drivers/acpi/events/evxfevnt.c
+++ b/drivers/acpi/events/evxfevnt.c
@@ -521,6 +521,9 @@ acpi_status acpi_get_event_status(u32 event, acpi_event_status * event_status)
 	if (value)
 		*event_status |= ACPI_EVENT_FLAG_SET;
 
+	if (acpi_gbl_fixed_event_handlers[event].handler)
+		*event_status |= ACPI_EVENT_FLAG_HANDLE;
+
 	return_ACPI_STATUS(status);
 }
 
@@ -571,6 +574,9 @@ acpi_get_gpe_status(acpi_handle gpe_device,
 
 	status = acpi_hw_get_gpe_status(gpe_event_info, event_status);
 
+	if (gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK)
+		*event_status |= ACPI_EVENT_FLAG_HANDLE;
+
       unlock_and_exit:
 	if (flags & ACPI_NOT_ISR) {
 		(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
diff --git a/drivers/acpi/system.c b/drivers/acpi/system.c
index 1d74171..6d348dc 100644
--- a/drivers/acpi/system.c
+++ b/drivers/acpi/system.c
@@ -167,7 +167,6 @@ static int acpi_system_sysfs_init(void)
 #define COUNT_ERROR 2	/* other */
 #define NUM_COUNTERS_EXTRA 3
 
-#define ACPI_EVENT_VALID	0x01
 struct event_counter {
 	u32 count;
 	u32 flags;
@@ -312,12 +311,6 @@ static int get_status(u32 index, acpi_event_status *status, acpi_handle *handle)
 	} else if (index < (num_gpes + ACPI_NUM_FIXED_EVENTS))
 		result = acpi_get_event_status(index - num_gpes, status);
 
-	/*
-	 * sleep/power button GPE/Fixed Event is enabled after acpi_system_init,
-	 * check the status at runtime and mark it as valid once it's enabled
-	 */
-	if (!result && (*status & ACPI_EVENT_FLAG_ENABLED))
-		all_counters[index].flags |= ACPI_EVENT_VALID;
 end:
 	return result;
 }
@@ -346,12 +339,14 @@ static ssize_t counter_show(struct kobject *kobj,
 	if (result)
 		goto end;
 
-	if (!(all_counters[index].flags & ACPI_EVENT_VALID))
-		size += sprintf(buf + size, "  invalid");
+	if (!(status & ACPI_EVENT_FLAG_HANDLE))
+		size += sprintf(buf + size, "	invalid");
 	else if (status & ACPI_EVENT_FLAG_ENABLED)
-		size += sprintf(buf + size, "	enable");
+		size += sprintf(buf + size, "	enabled");
+	else if (status & ACPI_EVENT_FLAG_WAKE_ENABLED)
+		size += sprintf(buf + size, "	wake_enabled");
 	else
-		size += sprintf(buf + size, "  disable");
+		size += sprintf(buf + size, "	disabled");
 
 end:
 	size += sprintf(buf + size, "\n");
@@ -385,7 +380,7 @@ static ssize_t counter_set(struct kobject *kobj,
 	if (result)
 		goto end;
 
-	if (!(all_counters[index].flags & ACPI_EVENT_VALID)) {
+	if (!(status & ACPI_EVENT_FLAG_HANDLE)) {
 		printk(KERN_WARNING PREFIX
 			"Can not change Invalid GPE/Fixed Event status\n");
 		return -EINVAL;
diff --git a/include/acpi/actypes.h b/include/acpi/actypes.h
index e8936ab..7220361 100644
--- a/include/acpi/actypes.h
+++ b/include/acpi/actypes.h
@@ -525,6 +525,7 @@ typedef u32 acpi_event_status;
 #define ACPI_EVENT_FLAG_ENABLED         (acpi_event_status) 0x01
 #define ACPI_EVENT_FLAG_WAKE_ENABLED    (acpi_event_status) 0x02
 #define ACPI_EVENT_FLAG_SET             (acpi_event_status) 0x04
+#define ACPI_EVENT_FLAG_HANDLE		(acpi_event_status) 0x08
 
 /*
  * General Purpose Events (GPE)
-- 
1.5.6.5

[PATCH 07/21] ACPI EC: Fix regression due to use of uninitialized variable

[Len Brown]

From: Alexey Starikovskiy <astarikovskiy@suse.de>

breakage introduced by following patch
commit 27663c5855b10af9ec67bc7dfba001426ba21222
Author: Matthew Wilcox <willy@linux.intel.com>
Date:   Fri Oct 10 02:22:59 2008 -0400

acpi_evaluate_integer() does not clear passed variable if
there is an error at evaluation.
So if we ignore error, we must supply initialized variable.

References: http://bugzilla.kernel.org/show_bug.cgi?id=11917
	    http://bugzilla.kernel.org/show_bug.cgi?id=11896

Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
Tested-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/ec.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index ef42316..523ac5b 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -736,7 +736,7 @@ static acpi_status
 ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
 {
 	acpi_status status;
-	unsigned long long tmp;
+	unsigned long long tmp = 0;
 
 	struct acpi_ec *ec = context;
 	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
@@ -751,6 +751,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
 		return status;
 	ec->gpe = tmp;
 	/* Use the global lock for all EC transactions? */
+	tmp = 0;
 	acpi_evaluate_integer(handle, "_GLK", NULL, &tmp);
 	ec->global_lock = tmp;
 	ec->handle = handle;
-- 
1.5.6.5

Re: [PATCH 07/21] ACPI EC: Fix regression due to use of uninitialized variable

[Zhao Yakui]

On Fri, 2008-11-07 at 12:39 +0800, Len Brown wrote:
> From: Alexey Starikovskiy <astarikovskiy@suse.de>
> 
> breakage introduced by following patch
> commit 27663c5855b10af9ec67bc7dfba001426ba21222
> Author: Matthew Wilcox <willy@linux.intel.com>
> Date:   Fri Oct 10 02:22:59 2008 -0400
> 
> acpi_evaluate_integer() does not clear passed variable if
> there is an error at evaluation.
> So if we ignore error, we must supply initialized variable.
> 
> References: http://bugzilla.kernel.org/show_bug.cgi?id=11917
> 	    http://bugzilla.kernel.org/show_bug.cgi?id=11896
The bug references for 11896 should be deleted.
    The bug for 11896 can't be resolved by this patch.
Thanks.
> 
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> Tested-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> Signed-off-by: Len Brown <len.brown@intel.com>
> ---
>  drivers/acpi/ec.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index ef42316..523ac5b 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -736,7 +736,7 @@ static acpi_status
>  ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
>  {
>  	acpi_status status;
> -	unsigned long long tmp;
> +	unsigned long long tmp = 0;
>  
>  	struct acpi_ec *ec = context;
>  	status = acpi_walk_resources(handle, METHOD_NAME__CRS,
> @@ -751,6 +751,7 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval)
>  		return status;
>  	ec->gpe = tmp;
>  	/* Use the global lock for all EC transactions? */
> +	tmp = 0;
>  	acpi_evaluate_integer(handle, "_GLK", NULL, &tmp);
>  	ec->global_lock = tmp;
>  	ec->handle = handle;

Re: [PATCH 07/21] ACPI EC: Fix regression due to use of uninitialized variable

[Len Brown]

> > From: Alexey Starikovskiy <astarikovskiy@suse.de>
> > 
> > breakage introduced by following patch
> > commit 27663c5855b10af9ec67bc7dfba001426ba21222
> > Author: Matthew Wilcox <willy@linux.intel.com>
> > Date:   Fri Oct 10 02:22:59 2008 -0400
> > 
> > acpi_evaluate_integer() does not clear passed variable if
> > there is an error at evaluation.
> > So if we ignore error, we must supply initialized variable.
> > 
> > References: http://bugzilla.kernel.org/show_bug.cgi?id=11917
> > 	    http://bugzilla.kernel.org/show_bug.cgi?id=11896
> The bug references for 11896 should be deleted.
>     The bug for 11896 can't be resolved by this patch.
> Thanks.

amended.

thanks,
-Len

The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Zhao, Yakui]

Hi, Alexey
     After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
     According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
     a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
     b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
     c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
   
   When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
     1. OS writes the EC command to EC CMD I/O port
     2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
     3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
      >status = acpi_ec_read_status(ec);
      >gpe_transaction(ec, status);
      > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
                wake_up(&ec->wait); 
     In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
     But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
　　 
　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
     
　　Welcome the comments.
     
　　Best regards
　　     Yakui

    

Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Alexey Starikovskiy]

Hi Yakui,

Just don't know how I would have lived without your valuable analisys...
Do you remember that ec_transaction starts by waiting for IBF to become 0?
Thus not any new transaction should begin before previous write or burst-disable command completes.
So, fatal logic error exists only in your head.

Now, let me explain why it is better to wait for last IBF=0 event at beginning of transaction, rather than at the end of previous one.
Look at the flow chart from different angle -- as fast transaction patch does it:
EC sends interrupt to OS then it is ready for us to proceed. It will send us interrupt marking IBF=0 transition if it expects us to write byte and it will send us interrupt marking OBF=1 transition if it expects us to read byte. It does not ever send us IBF=1 or OBF=0 interrupts just because there is nothing we could of should do in such a case.
After we completed write sequence, we don't need to wait for last IBF=0 transition, as we don't know if we will have anything else to write -- it might be last transaction for quite a while. But next transaction knows that it should not start before IBF is cleared, thus it waits 
for it (and expects possible interrupt).

Regards,
Alex.
 
Zhao, Yakui wrote:
> Hi, Alexey
>      After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
>      According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
>      a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
>      b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
>      c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
>    
>    When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
>      1. OS writes the EC command to EC CMD I/O port
>      2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
>      3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
>       >status = acpi_ec_read_status(ec);
>       >gpe_transaction(ec, status);
>       > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
>                 wake_up(&ec->wait); 
>      In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
>      But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
> 　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
> 　　 
> 　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
>      
> 　　Welcome the comments.
>      
> 　　Best regards
> 　　     Yakui
> 
>     
> 

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

RE: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Zhao, Yakui]

    a. When the host writes data to the command or data register of the embedded controller, the input buffer flag (IBF) in the status register is set within 1 microsecond.( Please see it in the section 12.7 of ACPI 3.0b. This should be paid attention to).  So If another EC transaction is started in 1 microsecond, maybe the EC status is not updated. In such case the problem will appear.
       At the same time we can't expect that all the EC follows the spec strictly. Maybe on some EC controllers more time is needed.
    b. Why not wait before the EC transaction is finished?  If it fails, it can't be caught in time.  From the programming viewpoint the logic is incorrect. If you insist that viewpoint is correct, IMO this issue can be discussed in the wider range. 
    
     In my email I raise the issue about your patch. If your patch can work well, no one is willing to raise any issue about your patch. 
     

-----Original Message-----
From: Alexey Starikovskiy [mailto:astarikovskiy@suse.de] 
Sent: 2008年11月8日 23:34
To: Zhao, Yakui
Cc: LenBrown; linux-acpi@vger.kernel.org; Brown, Len; Li, Shaohua; Zhang, Rui; Lin, Ming M
Subject: Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

Hi Yakui,

Just don't know how I would have lived without your valuable analisys...
Do you remember that ec_transaction starts by waiting for IBF to become 0?
Thus not any new transaction should begin before previous write or burst-disable command completes.
So, fatal logic error exists only in your head.

Now, let me explain why it is better to wait for last IBF=0 event at beginning of transaction, rather than at the end of previous one.
Look at the flow chart from different angle -- as fast transaction patch does it:
EC sends interrupt to OS then it is ready for us to proceed. It will send us interrupt marking IBF=0 transition if it expects us to write byte and it will send us interrupt marking OBF=1 transition if it expects us to read byte. It does not ever send us IBF=1 or OBF=0 interrupts just because there is nothing we could of should do in such a case.
After we completed write sequence, we don't need to wait for last IBF=0 transition, as we don't know if we will have anything else to write -- it might be last transaction for quite a while. But next transaction knows that it should not start before IBF is cleared, thus it waits 
for it (and expects possible interrupt).

Regards,
Alex.
 
Zhao, Yakui wrote:
> Hi, Alexey
>      After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
>      According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
>      a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
>      b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
>      c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
>    
>    When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
>      1. OS writes the EC command to EC CMD I/O port
>      2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
>      3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
>       >status = acpi_ec_read_status(ec);
>       >gpe_transaction(ec, status);
>       > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
>                 wake_up(&ec->wait); 
>      In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
>      But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
> 　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
> 　　 
> 　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
>      
> 　　Welcome the comments.
>      
> 　　Best regards
> 　　     Yakui
> 
>     
> 

Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Alexey Starikovskiy]

Yakui,
This is not about my patch, this is about your ego...
My patch did not introduce this behaviour, it was there since "burst mode" was introduced, and may be even earlier. You may ask Luming Yu, why he was so optimistic about 1 microsecond (is he still around? you did not include him into CC yet...).

Regards,
Alex.

Zhao, Yakui wrote:
>     a. When the host writes data to the command or data register of the embedded controller, the input buffer flag (IBF) in the status register is set within 1 microsecond.( Please see it in the section 12.7 of ACPI 3.0b. This should be paid attention to).  So If another EC transaction is started in 1 microsecond, maybe the EC status is not updated. In such case the problem will appear.
>        At the same time we can't expect that all the EC follows the spec strictly. Maybe on some EC controllers more time is needed.
>     b. Why not wait before the EC transaction is finished?  If it fails, it can't be caught in time.  From the programming viewpoint the logic is incorrect. If you insist that viewpoint is correct, IMO this issue can be discussed in the wider range. 
>     
>      In my email I raise the issue about your patch. If your patch can work well, no one is willing to raise any issue about your patch. 
>      
> 
> -----Original Message-----
> From: Alexey Starikovskiy [mailto:astarikovskiy@suse.de] 
> Sent: 2008年11月8日 23:34
> To: Zhao, Yakui
> Cc: LenBrown; linux-acpi@vger.kernel.org; Brown, Len; Li, Shaohua; Zhang, Rui; Lin, Ming M
> Subject: Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)
> 
> Hi Yakui,
> 
> Just don't know how I would have lived without your valuable analisys...
> Do you remember that ec_transaction starts by waiting for IBF to become 0?
> Thus not any new transaction should begin before previous write or burst-disable command completes.
> So, fatal logic error exists only in your head.
> 
> Now, let me explain why it is better to wait for last IBF=0 event at beginning of transaction, rather than at the end of previous one.
> Look at the flow chart from different angle -- as fast transaction patch does it:
> EC sends interrupt to OS then it is ready for us to proceed. It will send us interrupt marking IBF=0 transition if it expects us to write byte and it will send us interrupt marking OBF=1 transition if it expects us to read byte. It does not ever send us IBF=1 or OBF=0 interrupts just because there is nothing we could of should do in such a case.
> After we completed write sequence, we don't need to wait for last IBF=0 transition, as we don't know if we will have anything else to write -- it might be last transaction for quite a while. But next transaction knows that it should not start before IBF is cleared, thus it waits 
> for it (and expects possible interrupt).
> 
> Regards,
> Alex.
>  
> Zhao, Yakui wrote:
>> Hi, Alexey
>>      After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
>>      According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
>>      a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
>>      b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
>>      c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
>>    
>>    When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
>>      1. OS writes the EC command to EC CMD I/O port
>>      2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
>>      3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
>>       >status = acpi_ec_read_status(ec);
>>       >gpe_transaction(ec, status);
>>       > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
>>                 wake_up(&ec->wait); 
>>      In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
>>      But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
>> 　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
>> 　　 
>> 　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
>>      
>> 　　Welcome the comments.
>>      
>> 　　Best regards
>> 　　     Yakui
>>
>>     
>>
> 

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

[PATCH] ACPI: EC: wait for last write gpe

[Alexey Starikovskiy]

There is a possibility that EC might break if next command is
issued within 1 us after write or burst-disable command.
This "possibility" was in EC driver for 3.5 years, after
451566f45a2e6cd10ba56e7220a9dd84ba3ef550.

References: http://marc.info/?l=linux-acpi&m=122616284402886&w=4
Cc: Zhao Yakui <yakui.zhao@intel.com>
Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
---

 drivers/acpi/ec.c |   21 +++++++++++++--------
 1 files changed, 13 insertions(+), 8 deletions(-)


diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index e5dbe21..d6007ac 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -102,6 +102,7 @@ struct transaction {
 	u8 command;
 	u8 wlen;
 	u8 rlen;
+	bool done;
 };
 
 static struct acpi_ec {
@@ -178,7 +179,7 @@ static int ec_transaction_done(struct acpi_ec *ec)
 	unsigned long flags;
 	int ret = 0;
 	spin_lock_irqsave(&ec->curr_lock, flags);
-	if (!ec->curr || (!ec->curr->wlen && !ec->curr->rlen))
+	if (!ec->curr || ec->curr->done)
 		ret = 1;
 	spin_unlock_irqrestore(&ec->curr_lock, flags);
 	return ret;
@@ -195,17 +196,20 @@ static void gpe_transaction(struct acpi_ec *ec, u8 status)
 			acpi_ec_write_data(ec, *(ec->curr->wdata++));
 			--ec->curr->wlen;
 		} else
-			/* false interrupt, state didn't change */
-			++ec->curr->irq_count;
-
+			goto err;
 	} else if (ec->curr->rlen > 0) {
 		if ((status & ACPI_EC_FLAG_OBF) == 1) {
 			*(ec->curr->rdata++) = acpi_ec_read_data(ec);
-			--ec->curr->rlen;
+			if (--ec->curr->rlen == 0)
+				ec->curr->done = true;
 		} else
-			/* false interrupt, state didn't change */
-			++ec->curr->irq_count;
-	}
+			goto err;
+	} else if (ec->curr->wlen == 0 && (status & ACPI_EC_FLAG_IBF) == 0)
+		ec->curr->done = true;
+	goto unlock;
+err:
+	/* false interrupt, state didn't change */
+	++ec->curr->irq_count;
 unlock:
 	spin_unlock_irqrestore(&ec->curr_lock, flags);
 }
@@ -265,6 +269,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 	spin_lock_irqsave(&ec->curr_lock, tmp);
 	/* following two actions should be kept atomic */
 	t->irq_count = 0;
+	t->done = false;
 	ec->curr = t;
 	acpi_ec_write_cmd(ec, ec->curr->command);
 	if (ec->curr->command == ACPI_EC_COMMAND_QUERY)

Re: [PATCH] ACPI: EC: wait for last write gpe

[Zhao Yakui]

On Sun, 2008-11-09 at 02:42 +0800, Alexey Starikovskiy wrote:
> There is a possibility that EC might break if next command is
> issued within 1 us after write or burst-disable command.
> This "possibility" was in EC driver for 3.5 years, after
> 451566f45a2e6cd10ba56e7220a9dd84ba3ef550.
It seems OK. Now OS will wait until the EC transaction is really
finished. The logic error what I mentioned is eliminated.
Will you please attach the patch in the bug 11892/11896 and let the bug
reporter try it?
Thanks.
> 
> References: http://marc.info/?l=linux-acpi&m=122616284402886&w=4
> Cc: Zhao Yakui <yakui.zhao@intel.com>
> Signed-off-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> ---
> 
>  drivers/acpi/ec.c |   21 +++++++++++++--------
>  1 files changed, 13 insertions(+), 8 deletions(-)
> 
> 
> diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
> index e5dbe21..d6007ac 100644
> --- a/drivers/acpi/ec.c
> +++ b/drivers/acpi/ec.c
> @@ -102,6 +102,7 @@ struct transaction {
>  	u8 command;
>  	u8 wlen;
>  	u8 rlen;
> +	bool done;
>  };
>  
>  static struct acpi_ec {
> @@ -178,7 +179,7 @@ static int ec_transaction_done(struct acpi_ec *ec)
>  	unsigned long flags;
>  	int ret = 0;
>  	spin_lock_irqsave(&ec->curr_lock, flags);
> -	if (!ec->curr || (!ec->curr->wlen && !ec->curr->rlen))
> +	if (!ec->curr || ec->curr->done)
>  		ret = 1;
>  	spin_unlock_irqrestore(&ec->curr_lock, flags);
>  	return ret;
> @@ -195,17 +196,20 @@ static void gpe_transaction(struct acpi_ec *ec, u8 status)
>  			acpi_ec_write_data(ec, *(ec->curr->wdata++));
>  			--ec->curr->wlen;
>  		} else
> -			/* false interrupt, state didn't change */
> -			++ec->curr->irq_count;
> -
> +			goto err;
>  	} else if (ec->curr->rlen > 0) {
>  		if ((status & ACPI_EC_FLAG_OBF) == 1) {
>  			*(ec->curr->rdata++) = acpi_ec_read_data(ec);
> -			--ec->curr->rlen;
> +			if (--ec->curr->rlen == 0)
> +				ec->curr->done = true;
>  		} else
> -			/* false interrupt, state didn't change */
> -			++ec->curr->irq_count;
> -	}
> +			goto err;
> +	} else if (ec->curr->wlen == 0 && (status & ACPI_EC_FLAG_IBF) == 0)
> +		ec->curr->done = true;
> +	goto unlock;
> +err:
> +	/* false interrupt, state didn't change */
> +	++ec->curr->irq_count;
>  unlock:
>  	spin_unlock_irqrestore(&ec->curr_lock, flags);
>  }
> @@ -265,6 +269,7 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
>  	spin_lock_irqsave(&ec->curr_lock, tmp);
>  	/* following two actions should be kept atomic */
>  	t->irq_count = 0;
> +	t->done = false;
>  	ec->curr = t;
>  	acpi_ec_write_cmd(ec, ec->curr->command);
>  	if (ec->curr->command == ACPI_EC_COMMAND_QUERY)
> 
> --
> 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

Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)

[Zhao Yakui]

On Sun, 2008-11-09 at 00:47 +0800, Alexey Starikovskiy wrote:
> Yakui,
> This is not about my patch, this is about your ego...
Thanks for the reply. I know that this is not introduced by your patch.
In the previous kernel the EC transaction is explicitly divided into
several phases. Only when EC transaction is really finished, the EC
mutex lock is released.  In such case the previous EC transaction is
already  finished when OS begins another EC transaction. So the issue
about "1 microsecond" won't appear. But in your patch(EC transaction is
done in interrupt context) when EC mutex is released , maybe the
previous EC transaction is not really finished. In such case the  issue
of "1 microsecond"  will appear.
Is what I said right? 
> My patch did not introduce this behaviour, it was there since "burst mode" was introduced, and may be even earlier. You may ask Luming Yu, why he was so optimistic about 1 microsecond (is he still around? you did not include him into CC yet...).

In my email what I said is only to point out the logic error. Maybe
there is no problem that the EC mutex is released before EC transaction
is really finished. IMO this is still a logic error. The program  logic
states that EC transaction is already finished but the EC transaction is
not really finished. They are inconsistent..
   The more important is that the failure in EC transaction can't be
detected in time.  
   Do you agree with my analysis?
  
Best regards.
   Yakui
> R egards,
> Alex.
> 
> Zhao, Yakui wrote:
> >     a. When the host writes data to the command or data register of the embedded controller, the input buffer flag (IBF) in the status register is set within 1 microsecond.( Please see it in the section 12.7 of ACPI 3.0b. This should be paid attention to).  So If another EC transaction is started in 1 microsecond, maybe the EC status is not updated. In such case the problem will appear.
> >        At the same time we can't expect that all the EC follows the spec strictly. Maybe on some EC controllers more time is needed.
> >     b. Why not wait before the EC transaction is finished?  If it fails, it can't be caught in time.  From the programming viewpoint the logic is incorrect. If you insist that viewpoint is correct, IMO this issue can be discussed in the wider range. 
> >     
> >      In my email I raise the issue about your patch. If your patch can work well, no one is willing to raise any issue about your patch. 
> >      
> > 
> > -----Original Message-----
> > From: Alexey Starikovskiy [mailto:astarikovskiy@suse.de] 
> > Sent: 2008年11月8日 23:34
> > To: Zhao, Yakui
> > Cc: LenBrown; linux-acpi@vger.kernel.org; Brown, Len; Li, Shaohua; Zhang, Rui; Lin, Ming M
> > Subject: Re: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)
> > 
> > Hi Yakui,
> > 
> > Just don't know how I would have lived without your valuable analisys...
> > Do you remember that ec_transaction starts by waiting for IBF to become 0?
> > Thus not any new transaction should begin before previous write or burst-disable command completes.
> > So, fatal logic error exists only in your head.
> > 
> > Now, let me explain why it is better to wait for last IBF=0 event at beginning of transaction, rather than at the end of previous one.
> > Look at the flow chart from different angle -- as fast transaction patch does it:
> > EC sends interrupt to OS then it is ready for us to proceed. It will send us interrupt marking IBF=0 transition if it expects us to write byte and it will send us interrupt marking OBF=1 transition if it expects us to read byte. It does not ever send us IBF=1 or OBF=0 interrupts just because there is nothing we could of should do in such a case.
> > After we completed write sequence, we don't need to wait for last IBF=0 transition, as we don't know if we will have anything else to write -- it might be last transaction for quite a while. But next transaction knows that it should not start before IBF is cleared, thus it waits 
> > for it (and expects possible interrupt).
> > 
> > Regards,
> > Alex.
> >  
> > Zhao, Yakui wrote:
> >> Hi, Alexey
> >>      After checking the EC working flowchart I find a fatal logic error that exists in EC write transaction. The issue happens on most laptops. The following is the detailed explanation about this issue:
> >>      According to the ACPI the EC write transaction is divided into the following three phases: (Seen in the section 12.6.2 of ACPI 3.0b)
> >>      a. Byte #1: Host Writes the EC command to EC CMD I/O PORT. Interrupt is triggered after EC reads the command byte 
> >>      b. Byte #2: Host writes the address index to EC data I/O port. Interrupt is triggered after EC reads the address index byte
> >>      c. Byte #3: Host writes the Data to EC data I/O port. Interrupt is triggered after EC reads the data byte
> >>    
> >>    When the function of acpi_ec_transaction_unlocked is called for EC write transaction, the wlen variable is initialized as 2 and the rlen variable is initialized as zero. Then EC transaction follows the below three steps.
> >>      1. OS writes the EC command to EC CMD I/O port
> >>      2. EC GPE interrupt is triggered and then the address index is written into EC data I/O port. (The wlen is decreased to 1.This is executed in EC GPE handler)
> >>      3. EC GPE interrupt is triggered again and then the data is written into the EC data I/O port. The wlen is decreased to 0.(This is also executed in EC GPE handler). As the wlen is decreased to zero, it means that the EC transaction is finished(True is returned by the function of ec_transaction_done). 
> >>       >status = acpi_ec_read_status(ec);
> >>       >gpe_transaction(ec, status);
> >>       > if (ec_transaction_done(ec) && (status & ACPI_EC_FLAG_IBF) == 0)
> >>                 wake_up(&ec->wait); 
> >>      In this step the status variable only indicates that EC controller already reads the address index written by OS(It can’t indicate that EC controller already reads the data byte written by Host). As the IBF flag is zero, the waiting process is waked up and EC mutex lock is released. In such case it means that OS can begin another EC transaction. 
> >>      But in fact in such case EC transaction is not really finished. After the EC data is written into EC Data I/O port, no flag indicates whether EC transaction is finished. Only when the IBF bit becomes zero again, we can say that EC transaction is really finished. If OS begins another EC transaction before previous EC transaction is really finished, we can't imagine what will happen.
> >> 　　IMO Based on the above analysis there exists the fatal logic error in the EC write transaction after the patch from Alexey is shipped. Although now no one reports this issue, it is still an unstable factor. After all it is the fatal logic error. 
> >> 　　 
> >> 　　 At the same time there exists the similar logic error when OS begins an EC transaction to disable the EC burst mode.
> >>      
> >> 　　Welcome the comments.
> >>      
> >> 　　Best regards
> >> 　　     Yakui
> >>
> >>     
> >>
> > 
> 
> --
> 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

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

Spreading FUD [was: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)]

[Alexey Starikovskiy]

Yakui,

Please take a look at the subject of the thread -- notice words "fatal"
and "(EC transaction is done in interrupt context)".
Looks horrible, isn't it? Now we begin to get into details, and it appears
that it is not fatal, introduced 3.5 years ago, and has nothing to do with
interrupt context transaction patch.
So, the whole point of your message is to spread FUD about "fast 
transaction" patch,
and show how great you are. I would guess that you've failed in 
achieving both goals.

If you cry "wolf!" too often, you'll get ignored, when the real wolf 
appears.

Now, for a positive example, the same message without exclamation marks...
---------------------------------------------------------------------------------------------------------------------------------
Subject: difference in EC transaction protocol
Hi Alex,
I've noticed difference in EC transaction protocol for write and 
burst-disable commands,
namely all that end with writing byte to EC -- they don't wait for last 
confirmation interrupt and
next transaction might start before current one completes.
May be this is the "root cause" we are looking for.
Regards,
Yakui
---------------------------------------------------------------------------------------------------------------------------------
Note that this message is shorter, and is not offensive.
Smile, and people may start to like you...

Regards,
Alex.

P.S. And I don't agree with your analysis as you still believe that 
before the fast transaction mutex was
not released at the same time as it is now.

Zhao Yakui wrote:
> On Sun, 2008-11-09 at 00:47 +0800, Alexey Starikovskiy wrote:
>   
>> Yakui,
>> This is not about my patch, this is about your ego...
>>     
> Thanks for the reply. I know that this is not introduced by your patch.
> In the previous kernel the EC transaction is explicitly divided into
> several phases. Only when EC transaction is really finished, the EC
> mutex lock is released.  In such case the previous EC transaction is
> already  finished when OS begins another EC transaction. So the issue
> about "1 microsecond" won't appear.
No. No matter how many phases, transaction is done and EC mutex is released
as soon as last write happens. EC driver never waited for confirmation 
for last written byte.
>  But in your patch(EC transaction is
> done in interrupt context) when EC mutex is released , maybe the
> previous EC transaction is not really finished. In such case the  issue
> of "1 microsecond"  will appear.
> Is what I said right?
No. my patch has same "optimization" as appeared in EC driver since 
introduction of interrupt mode.
May be it is more visible now, and you managed to notice it, while still 
failing to see it in earlier EC driver versions?
I agree that there are some benefits in not doing this optimization, 
thus I made the patch to drop it.
>  
>   
>> My patch did not introduce this behaviour, it was there since "burst mode" was introduced, and may be even earlier. You may ask Luming Yu, why he was so optimistic about 1 microsecond (is he still around? you did not include him into CC yet...).
>>     
Actually, I gave too much credit to Shanghai office... Interrupt mode patch was written by Dmitry Torokhov, not Luming Yu.

> In my email what I said is only to point out the logic error. Maybe
>   
No, see above.
> there is no problem that the EC mutex is released before EC transaction
> is really finished. IMO this is still a logic error. The program  logic
> states that EC transaction is already finished but the EC transaction is
> not really finished. They are inconsistent..
>    The more important is that the failure in EC transaction can't be
> detected in time.
>   
>    Do you agree with my analysis?
>   
>   
No, see above.

Re: Spreading FUD [was: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)]

[Zhao Yakui]

On Sun, 2008-11-09 at 23:27 +0800, Alexey Starikovskiy wrote:
> Yakui,
> 
> Please take a look at the subject of the thread -- notice words "fatal"
> and "(EC transaction is done in interrupt context)".
> Looks horrible, isn't it? Now we begin to get into details, and it appears
> that it is not fatal, introduced 3.5 years ago, and has nothing to do with
> interrupt context transaction patch.
> So, the whole point of your message is to spread FUD about "fast 
> transaction" patch,
> and show how great you are. I would guess that you've failed in 
> achieving both goals.
I won't argue with you about this issue. 
     In your patch the program logic states that the EC transaction is
already finished. But in fact the EC transaction is not really finished.
They are inconsistent. Why can't call this a fatal logic error? 
     The more important is that the failure in EC transaction can't be
detected in time. 
     Why can't call this a fatal logic error?
    
If this is not a fatal logic error, what can be called fatal? Like that
Several commits are reverted each other as the following three commits.
    a.7c010de7506954e973abfab5c5999c5a97f7a73e
    b.4c611060660f0de3e9b8f02df207312bc6f5c331
    c.f9319f903f898dd4b15dbc386499725ce6c59776

If you still reply the subject so impolitely, Don't blame that I will
forward the similar error to wider range.
 
     
> 
> If you cry "wolf!" too often, you'll get ignored, when the real wolf 
> appears.
> 
> Now, for a positive example, the same message without exclamation marks...
> ---------------------------------------------------------------------------------------------------------------------------------
> Subject: difference in EC transaction protocol
> Hi Alex,
> I've noticed difference in EC transaction protocol for write and 
> burst-disable commands,
> namely all that end with writing byte to EC -- they don't wait for last 
> confirmation interrupt and
> next transaction might start before current one completes.
> May be this is the "root cause" we are looking for.
> Regards,
> Yakui
> ---------------------------------------------------------------------------------------------------------------------------------
> Note that this message is shorter, and is not offensive.
> Smile, and people may start to like you...
> 
> Regards,
> Alex.
> 
> P.S. And I don't agree with your analysis as you still believe that 
> before the fast transaction mutex was
> not released at the same time as it is now.
> 
> Zhao Yakui wrote:
> > On Sun, 2008-11-09 at 00:47 +0800, Alexey Starikovskiy wrote:
> >   
> >> Yakui,
> >> This is not about my patch, this is about your ego...
> >>     
> > Thanks for the reply. I know that this is not introduced by your patch.
> > In the previous kernel the EC transaction is explicitly divided into
> > several phases. Only when EC transaction is really finished, the EC
> > mutex lock is released.  In such case the previous EC transaction is
> > already  finished when OS begins another EC transaction. So the issue
> > about "1 microsecond" won't appear.
> No. No matter how many phases, transaction is done and EC mutex is released
> as soon as last write happens. EC driver never waited for confirmation 
> for last written byte.
> >  But in your patch(EC transaction is
> > done in interrupt context) when EC mutex is released , maybe the
> > previous EC transaction is not really finished. In such case the  issue
> > of "1 microsecond"  will appear.
> > Is what I said right?
> No. my patch has same "optimization" as appeared in EC driver since 
> introduction of interrupt mode.
> May be it is more visible now, and you managed to notice it, while still 
> failing to see it in earlier EC driver versions?
> I agree that there are some benefits in not doing this optimization, 
> thus I made the patch to drop it.
> >  
> >   
> >> My patch did not introduce this behaviour, it was there since "burst mode" was introduced, and may be even earlier. You may ask Luming Yu, why he was so optimistic about 1 microsecond (is he still around? you did not include him into CC yet...).
> >>     
> Actually, I gave too much credit to Shanghai office... Interrupt mode patch was written by Dmitry Torokhov, not Luming Yu.
> 
> > In my email what I said is only to point out the logic error. Maybe
> >   
> No, see above.
> > there is no problem that the EC mutex is released before EC transaction
> > is really finished. IMO this is still a logic error. The program  logic
> > states that EC transaction is already finished but the EC transaction is
> > not really finished. They are inconsistent..
> >    The more important is that the failure in EC transaction can't be
> > detected in time.
> >   
> >    Do you agree with my analysis?
> >   
> >   
> No, see above.
> 
> --
> 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

Re: Spreading FUD [was: The fatal logic error in EC write transaction(EC transaction is done in interrupt context)]

[Alexey Starikovskiy]

Zhao Yakui wrote:
> On Sun, 2008-11-09 at 23:27 +0800, Alexey Starikovskiy wrote:
>   
>> Yakui,
>>
>> Please take a look at the subject of the thread -- notice words "fatal"
>> and "(EC transaction is done in interrupt context)".
>> Looks horrible, isn't it? Now we begin to get into details, and it appears
>> that it is not fatal, introduced 3.5 years ago, and has nothing to do with
>> interrupt context transaction patch.
>> So, the whole point of your message is to spread FUD about "fast 
>> transaction" patch,
>> and show how great you are. I would guess that you've failed in 
>> achieving both goals.
>>     
> I won't argue with you about this issue. 
>   
Good.
>      In your patch the program logic states that the EC transaction is
> already finished. But in fact the EC transaction is not really finished.
> They are inconsistent. Why can't call this a fatal logic error? 
>      The more important is that the failure in EC transaction can't be
> detected in time. 
>      Why can't call this a fatal logic error?
>   
Here is Merriam-Webster' definition of "fatal", look at use case number 
4, which
is closest to your use of it:
<a href="http://www.merriam-webster.com/dictionary/fatal">fatal</a>
As we lived with the problem you decribe all these years, it can not be 
described as
"causing failure/death", thus it can not be called "fatal".

>     
> If this is not a fatal logic error, what can be called fatal? Like that
> Several commits are reverted each other as the following three commits.
>     a.7c010de7506954e973abfab5c5999c5a97f7a73e
>     b.4c611060660f0de3e9b8f02df207312bc6f5c331
>     c.f9319f903f898dd4b15dbc386499725ce6c59776
>
>   
If I had all these machines sitting on my desk side-by-side, I may have 
fixed them with one patch,
otherwise one need to make rounds. Do you know any other solution?
> If you still reply the subject so impolitely, Don't blame that I will
> forward the similar error to wider range.
>  
>   
You send false or bended statements not for the first time, do you 
expect any other treatment?
You certainly could do whatever you want, but prepare for others to 
include you in ignore/spam list.

Regards,
Alex.

[PATCH 08/21] intel_menlow: don't set max_state a negative value

[Len Brown]

From: Zhang Rui <rui.zhang@intel.com>

max_state is unsigned long.
don't set max_state a negative value

Cc : Thomas Sujith <sujith.thomas@intel.com>
Cc : Roel Kluin <roel.kluin@gmail.com>
Signed-off-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/misc/intel_menlow.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/intel_menlow.c b/drivers/misc/intel_menlow.c
index e00a275..124b37d 100644
--- a/drivers/misc/intel_menlow.c
+++ b/drivers/misc/intel_menlow.c
@@ -71,6 +71,9 @@ static int memory_get_int_max_bandwidth(struct thermal_cooling_device *cdev,
 	if (ACPI_FAILURE(status))
 		return -EFAULT;
 
+	if (!value)
+		return -EINVAL;
+
 	*max_state = value - 1;
 	return 0;
 }
@@ -121,7 +124,7 @@ static int memory_set_cur_bandwidth(struct thermal_cooling_device *cdev,
 	if (memory_get_int_max_bandwidth(cdev, &max_state))
 		return -EFAULT;
 
-	if (max_state < 0 || state > max_state)
+	if (state > max_state)
 		return -EINVAL;
 
 	arg_list.count = 1;
-- 
1.5.6.5

[PATCH 09/21] ACPI: remove comments about debug layer/level to use

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

I don't think there's any point in cluttering the code with these.
Better to improve the documentation so *anybody* can figure out
what layer & level to use.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/pci_root.c |    6 ------
 drivers/acpi/video.c    |    6 ------
 2 files changed, 0 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/pci_root.c b/drivers/acpi/pci_root.c
index 1b8f67d..642554b 100644
--- a/drivers/acpi/pci_root.c
+++ b/drivers/acpi/pci_root.c
@@ -376,15 +376,9 @@ static int acpi_pci_root_remove(struct acpi_device *device, int type)
 
 static int __init acpi_pci_root_init(void)
 {
-
 	if (acpi_pci_disabled)
 		return 0;
 
-	/* DEBUG:
-	   acpi_dbg_layer = ACPI_PCI_COMPONENT;
-	   acpi_dbg_level = 0xFFFFFFFF;
-	 */
-
 	if (acpi_bus_register_driver(&acpi_pci_root_driver) < 0)
 		return -ENODEV;
 
diff --git a/drivers/acpi/video.c b/drivers/acpi/video.c
index a29b0cc..bf0c26a 100644
--- a/drivers/acpi/video.c
+++ b/drivers/acpi/video.c
@@ -2094,12 +2094,6 @@ static int __init acpi_video_init(void)
 {
 	int result = 0;
 
-
-	/*
-	   acpi_dbg_level = 0xFFFFFFFF;
-	   acpi_dbg_layer = 0x08000000;
-	 */
-
 	acpi_video_dir = proc_mkdir(ACPI_VIDEO_CLASS, acpi_root_dir);
 	if (!acpi_video_dir)
 		return -ENODEV;
-- 
1.5.6.5

[PATCH 10/21] ACPI: SBS: remove useless acpi_cm_sbs_init() initcall

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

acpi_cm_sbs_init() doesn't do anything, so we can just remove it.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/cm_sbs.c |    6 ------
 1 files changed, 0 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/cm_sbs.c b/drivers/acpi/cm_sbs.c
index 4441e84..80d5c88 100644
--- a/drivers/acpi/cm_sbs.c
+++ b/drivers/acpi/cm_sbs.c
@@ -105,9 +105,3 @@ void acpi_unlock_battery_dir(struct proc_dir_entry *acpi_battery_dir_param)
 	return;
 }
 EXPORT_SYMBOL(acpi_unlock_battery_dir);
-
-static int __init acpi_cm_sbs_init(void)
-{
-	return 0;
-}
-subsys_initcall(acpi_cm_sbs_init);
-- 
1.5.6.5

[PATCH 11/21] ACPI: remove CONFIG_ACPI_POWER

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

Remove CONFIG_ACPI_POWER.  It was always set the same as CONFIG_ACPI,
and it had no menu label, so there was no way to set it to anything
other than "y".

The interfaces under CONFIG_ACPI_POWER (acpi_device_sleep_wake(),
acpi_power_transition(), etc) are called unconditionally from the
ACPI core, so we already depend on it always being present.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/Kconfig        |    4 ----
 drivers/acpi/Makefile       |    2 +-
 include/acpi/acpi_drivers.h |    2 --
 3 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index f4f6329..4fa7866 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -341,10 +341,6 @@ config ACPI_PCI_SLOT
 	  help you correlate PCI bus addresses with the physical geography
 	  of your slots. If you are unsure, say N.
 
-config ACPI_POWER
-	bool
-	default y
-
 config ACPI_SYSTEM
 	bool
 	default y
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index d91c027..8017f63 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -51,7 +51,7 @@ obj-$(CONFIG_ACPI_PCI_SLOT)	+= pci_slot.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_CONTAINER)	+= container.o
 obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
-obj-$(CONFIG_ACPI_POWER)	+= power.o
+obj-y				+= power.o
 obj-$(CONFIG_ACPI_SYSTEM)	+= system.o event.o
 obj-$(CONFIG_ACPI_DEBUG)	+= debug.o
 obj-$(CONFIG_ACPI_NUMA)		+= numa.o
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index cf04c60..818215f 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -86,7 +86,6 @@ struct pci_bus *pci_acpi_scan_root(struct acpi_device *device, int domain,
                                   Power Resource
    -------------------------------------------------------------------------- */
 
-#ifdef CONFIG_ACPI_POWER
 int acpi_device_sleep_wake(struct acpi_device *dev,
                            int enable, int sleep_state, int dev_state);
 int acpi_enable_wakeup_device_power(struct acpi_device *dev, int sleep_state);
@@ -94,7 +93,6 @@ int acpi_disable_wakeup_device_power(struct acpi_device *dev);
 int acpi_power_get_inferred_state(struct acpi_device *device);
 int acpi_power_transition(struct acpi_device *device, int state);
 extern int acpi_power_nocheck;
-#endif
 
 /* --------------------------------------------------------------------------
                                   Embedded Controller
-- 
1.5.6.5

[PATCH 12/21] ACPI: remove CONFIG_ACPI_EC

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

Remove CONFIG_ACPI_EC.  It was always set the same as CONFIG_ACPI,
and it had no menu label, so there was no way to set it to anything
other than "y".

Per section 6.5.4 of the ACPI 3.0b specification,

    OSPM must make Embedded Controller operation regions, accessed
    via the Embedded Controllers described in ECDT, available before
    executing any control method.

The ECDT table is optional, but if it is present, the above text
means that the EC it describes is a required part of the ACPI
subsystem, so CONFIG_ACPI_EC=n wouldn't make sense.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/Kconfig        |    8 --------
 drivers/acpi/Makefile       |    2 +-
 drivers/acpi/bus.c          |    3 +--
 drivers/char/sonypi.c       |    4 ++--
 drivers/misc/Kconfig        |    4 ++--
 include/acpi/acpi_drivers.h |    2 --
 include/linux/acpi.h        |    4 ----
 7 files changed, 6 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 4fa7866..90cb2a8 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -324,14 +324,6 @@ config ACPI_DEBUG_FUNC_TRACE
 	  ACPI Debug Statements slow down ACPI processing. Function trace
 	  is about half of the penalty and is rarely useful.
 
-config ACPI_EC
-	bool
-	default y
-	help
-	  This driver is required on some systems for the proper operation of
-	  the battery and thermal drivers.  If you are compiling for a 
-	  mobile system, say Y.
-
 config ACPI_PCI_SLOT
 	tristate "PCI slot detection driver"
 	default n
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 8017f63..fc62231 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -39,7 +39,7 @@ obj-y				+= sleep/
 obj-y				+= bus.o glue.o
 obj-y				+= scan.o
 # Keep EC driver first. Initialization of others depend on it.
-obj-$(CONFIG_ACPI_EC)		+= ec.o
+obj-y				+= ec.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index c797c64..765fd1c 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -774,7 +774,7 @@ static int __init acpi_bus_init(void)
 		       "Unable to initialize ACPI OS objects\n");
 		goto error1;
 	}
-#ifdef CONFIG_ACPI_EC
+
 	/*
 	 * ACPI 2.0 requires the EC driver to be loaded and work before
 	 * the EC device is found in the namespace (i.e. before acpi_initialize_objects()
@@ -785,7 +785,6 @@ static int __init acpi_bus_init(void)
 	 */
 	status = acpi_ec_ecdt_probe();
 	/* Ignore result. Not having an ECDT is not fatal. */
-#endif
 
 	status = acpi_initialize_objects(ACPI_FULL_INITIALIZATION);
 	if (ACPI_FAILURE(status)) {
diff --git a/drivers/char/sonypi.c b/drivers/char/sonypi.c
index 85e0eb7..1b128d1 100644
--- a/drivers/char/sonypi.c
+++ b/drivers/char/sonypi.c
@@ -523,7 +523,7 @@ static int acpi_driver_registered;
 
 static int sonypi_ec_write(u8 addr, u8 value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_write(addr, value);
 #endif
@@ -539,7 +539,7 @@ static int sonypi_ec_write(u8 addr, u8 value)
 
 static int sonypi_ec_read(u8 addr, u8 *value)
 {
-#ifdef CONFIG_ACPI_EC
+#ifdef CONFIG_ACPI
 	if (SONYPI_ACPI_ACTIVE)
 		return ec_read(addr, value);
 #endif
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 9494400..4494ad2 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -230,7 +230,7 @@ config HP_WMI
 config MSI_LAPTOP
         tristate "MSI Laptop Extras"
         depends on X86
-        depends on ACPI_EC
+        depends on ACPI
         depends on BACKLIGHT_CLASS_DEVICE
         ---help---
 	  This is a driver for laptops built by MSI (MICRO-STAR
@@ -260,7 +260,7 @@ config PANASONIC_LAPTOP
 config COMPAL_LAPTOP
 	tristate "Compal Laptop Extras"
 	depends on X86
-	depends on ACPI_EC
+	depends on ACPI
 	depends on BACKLIGHT_CLASS_DEVICE
 	---help---
 	  This is a driver for laptops built by Compal:
diff --git a/include/acpi/acpi_drivers.h b/include/acpi/acpi_drivers.h
index 818215f..b3c40dc 100644
--- a/include/acpi/acpi_drivers.h
+++ b/include/acpi/acpi_drivers.h
@@ -97,10 +97,8 @@ extern int acpi_power_nocheck;
 /* --------------------------------------------------------------------------
                                   Embedded Controller
    -------------------------------------------------------------------------- */
-#ifdef CONFIG_ACPI_EC
 int acpi_ec_ecdt_probe(void);
 int acpi_boot_ec_enable(void);
-#endif
 
 /* --------------------------------------------------------------------------
                                     Processor
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index fd6a452..d7846bd 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -163,8 +163,6 @@ struct acpi_pci_driver {
 int acpi_pci_register_driver(struct acpi_pci_driver *driver);
 void acpi_pci_unregister_driver(struct acpi_pci_driver *driver);
 
-#ifdef CONFIG_ACPI_EC
-
 extern int ec_read(u8 addr, u8 *val);
 extern int ec_write(u8 addr, u8 val);
 extern int ec_transaction(u8 command,
@@ -172,8 +170,6 @@ extern int ec_transaction(u8 command,
                           u8 *rdata, unsigned rdata_len,
 			  int force_poll);
 
-#endif /*CONFIG_ACPI_EC*/
-
 #if defined(CONFIG_ACPI_WMI) || defined(CONFIG_ACPI_WMI_MODULE)
 
 typedef void (*wmi_notify_handler) (u32 value, void *context);
-- 
1.5.6.5

[PATCH 13/21] intel_menlow: Add comment documenting legal GTHS values

[Len Brown]

From: Thomas, Sujith <sujith.thomas@intel.com>

Signed-off-by: Sujith Thomas <sujith.thomas@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/misc/intel_menlow.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/drivers/misc/intel_menlow.c b/drivers/misc/intel_menlow.c
index 124b37d..27b7662 100644
--- a/drivers/misc/intel_menlow.c
+++ b/drivers/misc/intel_menlow.c
@@ -52,6 +52,11 @@ MODULE_LICENSE("GPL");
 #define MEMORY_ARG_CUR_BANDWIDTH 1
 #define MEMORY_ARG_MAX_BANDWIDTH 0
 
+/*
+ * GTHS returning 'n' would mean that [0,n-1] states are supported
+ * In that case max_cstate would be n-1
+ * GTHS returning '0' would mean that no bandwidth control states are supported
+ */
 static int memory_get_int_max_bandwidth(struct thermal_cooling_device *cdev,
 					unsigned long *max_state)
 {
-- 
1.5.6.5

[PATCH 14/21] intel_menlow: MAINTAINERS

[Len Brown]

From: Thomas, Sujith <sujith.thomas@intel.com>

Signed-off-by: Sujith Thomas <sujith.thomas@intel.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 MAINTAINERS |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 16202c8..808bb85 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2178,6 +2178,13 @@ M:	adaplas@gmail.com
 L:	linux-fbdev-devel@lists.sourceforge.net (moderated for non-subscribers)
 S:	Maintained
 
+INTEL MENLOW THERMAL DRIVER
+P:	Sujith Thomas
+M:	sujith.thomas@intel.com
+L:	linux-acpi@vger.kernel.org
+W:	http://www.lesswatts.org/projects/acpi/
+S:	Supported
+
 INTEL IA32 MICROCODE UPDATE SUPPORT
 P:	Tigran Aivazian
 M:	tigran@aivazian.fsnet.co.uk
-- 
1.5.6.5

[PATCH 15/21] Revert "ACPI: Ingore the RESET_REG_SUP bit when using ACPI reset mechanism"

[Len Brown]

From: Len Brown <len.brown@intel.com>

This reverts commit 8fd145917fb62368a9b80db59562c20576238f5a.

http://bugzilla.kernel.org/show_bug.cgi?id=11942

Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/reboot.c |   25 +++----------------------
 1 files changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/acpi/reboot.c b/drivers/acpi/reboot.c
index 755baf2..a6b662c 100644
--- a/drivers/acpi/reboot.c
+++ b/drivers/acpi/reboot.c
@@ -15,28 +15,9 @@ void acpi_reboot(void)
 
 	rr = &acpi_gbl_FADT.reset_register;
 
-	/*
-	 * Is the ACPI reset register supported?
-	 *
-	 * According to ACPI 3.0, FADT.flags.RESET_REG_SUP indicates
-	 * whether the ACPI reset mechanism is supported.
-	 *
-	 * However, some boxes have this bit clear, yet a valid
-	 * ACPI_RESET_REG & RESET_VALUE, and ACPI reboot is the only
-	 * mechanism that works for them after S3.
-	 *
-	 * This suggests that other operating systems may not be checking
-	 * the RESET_REG_SUP bit, and are using other means to decide
-	 * whether to use the ACPI reboot mechanism or not.
-	 *
-	 * So when acpi reboot is requested,
-	 * only the reset_register is checked. If the following
-	 * conditions are met, it indicates that the reset register is supported.
-	 * 	a. reset_register is not zero
-	 * 	b. the access width is eight
-	 * 	c. the bit_offset is zero
-	 */
-	if (!(rr->address) || rr->bit_width != 8 || rr->bit_offset != 0)
+	/* Is the reset register supported? */
+	if (!(acpi_gbl_FADT.flags & ACPI_FADT_RESET_REGISTER) ||
+	    rr->bit_width != 8 || rr->bit_offset != 0)
 		return;
 
 	reset_value = acpi_gbl_FADT.reset_value;
-- 
1.5.6.5

[PATCH 16/21] PNP: add Bjorn Helgaas as PNP co-maintainer

[Len Brown]

From: Bjorn Helgaas <bjorn.helgaas@hp.com>

Update Adam's email address and add myself as PNP co-maintainer.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 MAINTAINERS |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 808bb85..5cf7492 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3336,7 +3336,9 @@ S:	Maintained
 
 PNP SUPPORT
 P:	Adam Belay
-M:	ambx1@neo.rr.com
+M:	abelay@mit.edu
+P:	Bjorn Helgaas
+M:	bjorn.helgaas@hp.com
 S:	Maintained
 
 PNXxxxx I2C DRIVER
-- 
1.5.6.5

[PATCH 17/21] ACPI: struct device - replace bus_id with dev_name(), dev_set_name()

[Len Brown]

From: Kay Sievers <kay.sievers@vrfy.org>

This patch is part of a larger patch series which will remove
the "char bus_id[20]" name string from struct device. The device
name is managed in the kobject anyway, and without any size
limitation, and just needlessly copied into "struct device".

To set and read the device name dev_name(dev) and dev_set_name(dev)
must be used. If your code uses static kobjects, which it shouldn't
do, "const char *init_name" can be used to statically provide the
name the registered device should have. At registration time, the
init_name field is cleared, to enforce the use of dev_name(dev) to
access the device name at a later time.

We need to get rid of all occurrences of bus_id in the entire tree
to be able to enable the new interface. Please apply this patch,
and possibly convert any remaining remaining occurrences of bus_id.

We want to submit a patch to -next, which will remove bus_id from
"struct device", to find the remaining pieces to convert, and finally
switch over to the new api, which will remove the 20 bytes array
and does no longer have a size limitation.

Acked-by: Greg Kroah-Hartman <gregkh@suse.de>
Signed-Off-By: Kay Sievers <kay.sievers@vrfy.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/ac.c             |    2 +-
 drivers/acpi/battery.c        |    2 +-
 drivers/acpi/processor_core.c |    6 +++---
 drivers/acpi/scan.c           |    8 ++++----
 drivers/acpi/sleep/proc.c     |    4 ++--
 drivers/acpi/thermal.c        |    8 ++++----
 drivers/acpi/wmi.c            |    2 +-
 7 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/ac.c b/drivers/acpi/ac.c
index d72a1b6..2d46732 100644
--- a/drivers/acpi/ac.c
+++ b/drivers/acpi/ac.c
@@ -242,7 +242,7 @@ static void acpi_ac_notify(acpi_handle handle, u32 event, void *data)
 		acpi_ac_get_state(ac);
 		acpi_bus_generate_proc_event(device, event, (u32) ac->state);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event,
+						  dev_name(&device->dev), event,
 						  (u32) ac->state);
 #ifdef CONFIG_ACPI_SYSFS_POWER
 		kobject_uevent(&ac->charger.dev->kobj, KOBJ_CHANGE);
diff --git a/drivers/acpi/battery.c b/drivers/acpi/battery.c
index b2133e8..e68f218 100644
--- a/drivers/acpi/battery.c
+++ b/drivers/acpi/battery.c
@@ -782,7 +782,7 @@ static void acpi_battery_notify(acpi_handle handle, u32 event, void *data)
 	acpi_bus_generate_proc_event(device, event,
 				     acpi_battery_present(battery));
 	acpi_bus_generate_netlink_event(device->pnp.device_class,
-					device->dev.bus_id, event,
+					dev_name(&device->dev), event,
 					acpi_battery_present(battery));
 #ifdef CONFIG_ACPI_SYSFS_POWER
 	/* acpi_batter_update could remove power_supply object */
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 24a362f..cf7e885 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -761,20 +761,20 @@ static void acpi_processor_notify(acpi_handle handle, u32 event, void *data)
 		acpi_bus_generate_proc_event(device, event,
 					pr->performance_platform_limit);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event,
+						  dev_name(&device->dev), event,
 						  pr->performance_platform_limit);
 		break;
 	case ACPI_PROCESSOR_NOTIFY_POWER:
 		acpi_processor_cst_has_changed(pr);
 		acpi_bus_generate_proc_event(device, event, 0);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event, 0);
+						  dev_name(&device->dev), event, 0);
 		break;
 	case ACPI_PROCESSOR_NOTIFY_THROTTLING:
 		acpi_processor_tstate_has_changed(pr);
 		acpi_bus_generate_proc_event(device, event, 0);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event, 0);
+						  dev_name(&device->dev), event, 0);
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
 				  "Unsupported event [0x%x]\n", event));
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a9dda8e..4dd1f31 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -109,8 +109,7 @@ static int acpi_bus_hot_remove_device(void *context)
 		return 0;
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO,
-		"Hot-removing device %s...\n", device->dev.bus_id));
-
+		"Hot-removing device %s...\n", dev_name(&device->dev)));
 
 	if (acpi_bus_trim(device, 1)) {
 		printk(KERN_ERR PREFIX
@@ -460,7 +459,7 @@ static int acpi_device_register(struct acpi_device *device,
 		acpi_device_bus_id->instance_no = 0;
 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
 	}
-	sprintf(device->dev.bus_id, "%s:%02x", acpi_device_bus_id->bus_id, acpi_device_bus_id->instance_no);
+	dev_set_name(&device->dev, "%s:%02x", acpi_device_bus_id->bus_id, acpi_device_bus_id->instance_no);
 
 	if (device->parent) {
 		list_add_tail(&device->node, &device->parent->children);
@@ -484,7 +483,8 @@ static int acpi_device_register(struct acpi_device *device,
 
 	result = acpi_device_setup_files(device);
 	if(result)
-		printk(KERN_ERR PREFIX "Error creating sysfs interface for device %s\n", device->dev.bus_id);
+		printk(KERN_ERR PREFIX "Error creating sysfs interface for device %s\n",
+		       dev_name(&device->dev));
 
 	device->removal_type = ACPI_BUS_REMOVAL_NORMAL;
 	return 0;
diff --git a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
index 631ee2e..64e591b 100644
--- a/drivers/acpi/sleep/proc.c
+++ b/drivers/acpi/sleep/proc.c
@@ -366,8 +366,8 @@ acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 			   dev->wakeup.state.enabled ? "enabled" : "disabled");
 		if (ldev)
 			seq_printf(seq, "%s:%s",
-				   ldev->bus ? ldev->bus->name : "no-bus",
-				   ldev->bus_id);
+				   dev_name(ldev) ? ldev->bus->name : "no-bus",
+				   dev_name(ldev));
 		seq_printf(seq, "\n");
 		put_device(ldev);
 
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index ad6cae9..462b168 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -576,7 +576,7 @@ static int acpi_thermal_critical(struct acpi_thermal *tz)
 	acpi_bus_generate_proc_event(tz->device, ACPI_THERMAL_NOTIFY_CRITICAL,
 				tz->trips.critical.flags.enabled);
 	acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
-					  tz->device->dev.bus_id,
+					  dev_name(&tz->device->dev),
 					  ACPI_THERMAL_NOTIFY_CRITICAL,
 					  tz->trips.critical.flags.enabled);
 
@@ -605,7 +605,7 @@ static int acpi_thermal_hot(struct acpi_thermal *tz)
 	acpi_bus_generate_proc_event(tz->device, ACPI_THERMAL_NOTIFY_HOT,
 				tz->trips.hot.flags.enabled);
 	acpi_bus_generate_netlink_event(tz->device->pnp.device_class,
-					  tz->device->dev.bus_id,
+					  dev_name(&tz->device->dev),
 					  ACPI_THERMAL_NOTIFY_HOT,
 					  tz->trips.hot.flags.enabled);
 
@@ -1592,14 +1592,14 @@ static void acpi_thermal_notify(acpi_handle handle, u32 event, void *data)
 		acpi_thermal_check(tz);
 		acpi_bus_generate_proc_event(device, event, 0);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event, 0);
+						  dev_name(&device->dev), event, 0);
 		break;
 	case ACPI_THERMAL_NOTIFY_DEVICES:
 		acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
 		acpi_thermal_check(tz);
 		acpi_bus_generate_proc_event(device, event, 0);
 		acpi_bus_generate_netlink_event(device->pnp.device_class,
-						  device->dev.bus_id, event, 0);
+						  dev_name(&device->dev), event, 0);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
diff --git a/drivers/acpi/wmi.c b/drivers/acpi/wmi.c
index 47cd7ba..8a8b377 100644
--- a/drivers/acpi/wmi.c
+++ b/drivers/acpi/wmi.c
@@ -660,7 +660,7 @@ static void acpi_wmi_notify(acpi_handle handle, u32 event, void *data)
 				wblock->handler(event, wblock->handler_data);
 
 			acpi_bus_generate_netlink_event(
-				device->pnp.device_class, device->dev.bus_id,
+				device->pnp.device_class, dev_name(&device->dev),
 				event, 0);
 			break;
 		}
-- 
1.5.6.5

[PATCH 18/21] fujitsu-laptop: fix section mismatch warning

[Len Brown]

From: Randy Dunlap <randy.dunlap@oracle.com>

Could fix a bug in a hotplug add scenario.

WARNING: drivers/misc/fujitsu-laptop.o(.text+0xbde): Section mismatch in reference from the function acpi_fujitsu_add() to the variable .init.data:fujitsu_dmi_table
The function acpi_fujitsu_add() references
the variable __initdata fujitsu_dmi_table.
This is often because acpi_fujitsu_add lacks a __initdata
annotation or the annotation of fujitsu_dmi_table is wrong.

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
Acked-by: Jonathan Woithe <jwoithe@physics.adelaide.edu.au>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/misc/fujitsu-laptop.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/misc/fujitsu-laptop.c b/drivers/misc/fujitsu-laptop.c
index d2cf0bf..9124fcd 100644
--- a/drivers/misc/fujitsu-laptop.c
+++ b/drivers/misc/fujitsu-laptop.c
@@ -473,7 +473,7 @@ static int dmi_check_cb_p8010(const struct dmi_system_id *id)
 	return 0;
 }
 
-static struct dmi_system_id __initdata fujitsu_dmi_table[] = {
+static struct dmi_system_id fujitsu_dmi_table[] = {
 	{
 	 .ident = "Fujitsu Siemens S6410",
 	 .matches = {
-- 
1.5.6.5

[PATCH 19/21] ACPI: use macro to replace hard number

[Len Brown]

From: Yinghai Lu <yinghai@kernel.org>

Impact: cleanup

Use MACRO for rev 3 fadt id instead of 3 directly.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/tables/tbfadt.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/acpi/tables/tbfadt.c b/drivers/acpi/tables/tbfadt.c
index 2c7885e..2817158 100644
--- a/drivers/acpi/tables/tbfadt.c
+++ b/drivers/acpi/tables/tbfadt.c
@@ -304,7 +304,7 @@ static void acpi_tb_convert_fadt(void)
 	 * The ACPI 1.0 reserved fields that will be zeroed are the bytes located at
 	 * offset 45, 55, 95, and the word located at offset 109, 110.
 	 */
-	if (acpi_gbl_FADT.header.revision < 3) {
+	if (acpi_gbl_FADT.header.revision < FADT2_REVISION_ID) {
 		acpi_gbl_FADT.preferred_profile = 0;
 		acpi_gbl_FADT.pstate_control = 0;
 		acpi_gbl_FADT.cst_control = 0;
-- 
1.5.6.5

[PATCH 20/21] ACPI: avoid empty file name in sysfs

[Len Brown]

From: Peter Gruber <nokos@gmx.net>

Since commit bc45b1d39a925b56796bebf8a397a0491489d85c acpi tables are
allowed to have an empty signature and /sys/firmware/acpi/tables uses the
signature as filename.  Applications using naive recursion through /sys
loop forever.  A possible solution would be: (replacing the zero length
filename with the string "NULL")

http://bugzilla.kernel.org/show_bug.cgi?id=11539

Acked-by: Zhang Rui <rui.zhang@intel.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/system.c |   25 +++++++++++++++++--------
 1 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/acpi/system.c b/drivers/acpi/system.c
index 1d74171..62ec75e 100644
--- a/drivers/acpi/system.c
+++ b/drivers/acpi/system.c
@@ -78,9 +78,15 @@ static ssize_t acpi_table_show(struct kobject *kobj,
 	    container_of(bin_attr, struct acpi_table_attr, attr);
 	struct acpi_table_header *table_header = NULL;
 	acpi_status status;
+	char name[ACPI_NAME_SIZE];
+
+	if (strncmp(table_attr->name, "NULL", 4))
+		memcpy(name, table_attr->name, ACPI_NAME_SIZE);
+	else
+		memcpy(name, "\0\0\0\0", 4);
 
 	status =
-	    acpi_get_table(table_attr->name, table_attr->instance,
+	    acpi_get_table(name, table_attr->instance,
 			   &table_header);
 	if (ACPI_FAILURE(status))
 		return -ENODEV;
@@ -95,21 +101,24 @@ static void acpi_table_attr_init(struct acpi_table_attr *table_attr,
 	struct acpi_table_header *header = NULL;
 	struct acpi_table_attr *attr = NULL;
 
-	memcpy(table_attr->name, table_header->signature, ACPI_NAME_SIZE);
+	if (table_header->signature[0] != '\0')
+		memcpy(table_attr->name, table_header->signature,
+			ACPI_NAME_SIZE);
+	else
+		memcpy(table_attr->name, "NULL", 4);
 
 	list_for_each_entry(attr, &acpi_table_attr_list, node) {
-		if (!memcmp(table_header->signature, attr->name,
-			    ACPI_NAME_SIZE))
+		if (!memcmp(table_attr->name, attr->name, ACPI_NAME_SIZE))
 			if (table_attr->instance < attr->instance)
 				table_attr->instance = attr->instance;
 	}
 	table_attr->instance++;
 
 	if (table_attr->instance > 1 || (table_attr->instance == 1 &&
-					 !acpi_get_table(table_header->
-							 signature, 2,
-							 &header)))
-		sprintf(table_attr->name + 4, "%d", table_attr->instance);
+					!acpi_get_table
+					(table_header->signature, 2, &header)))
+		sprintf(table_attr->name + ACPI_NAME_SIZE, "%d",
+			table_attr->instance);
 
 	table_attr->attr.size = 0;
 	table_attr->attr.read = acpi_table_show;
-- 
1.5.6.5

[PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Len Brown]

From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>

Make sure we can tell if the GPE storm workaround gets activated,
and avoid flooding the logs afterwards.

http://bugzilla.kernel.org/show_bug.cgi?id=11841
"plenty of line "ACPI: EC: non-query interrupt received,
 switching to interrupt mode" in dmesg"

Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
Signed-off-by: Len Brown <len.brown@intel.com>
---
 drivers/acpi/ec.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 3ef5b79..b340e08 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -286,7 +286,8 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
 		acpi_enable_gpe(NULL, ec->gpe, ACPI_NOT_ISR);
 	} else if (test_bit(EC_FLAGS_GPE_MODE, &ec->flags) &&
 		   t->irq_count > ACPI_EC_STORM_THRESHOLD) {
-		pr_debug(PREFIX "GPE storm detected\n");
+		pr_info(PREFIX "GPE storm detected, "
+			"transactions will use polling mode\n");
 		set_bit(EC_FLAGS_GPE_STORM, &ec->flags);
 	}
 	return ret;
@@ -566,9 +567,15 @@ static u32 acpi_ec_gpe_handler(void *data)
 	if (!test_bit(EC_FLAGS_GPE_MODE, &ec->flags) &&
 	    !test_bit(EC_FLAGS_NO_GPE, &ec->flags)) {
 		/* this is non-query, must be confirmation */
-		if (printk_ratelimit())
-			pr_info(PREFIX "non-query interrupt received,"
+		if (!test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
+			if (printk_ratelimit())
+				pr_info(PREFIX "non-query interrupt received,"
+					" switching to interrupt mode\n");
+		} else {
+			/* hush, STORM switches the mode every transaction */
+			pr_debug(PREFIX "non-query interrupt received,"
 				" switching to interrupt mode\n");
+		}
 		set_bit(EC_FLAGS_GPE_MODE, &ec->flags);
 	}
 	return ACPI_INTERRUPT_HANDLED;
-- 
1.5.6.5

Re: [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Alan Jenkins]

Len Brown wrote:
> From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
>
> Make sure we can tell if the GPE storm workaround gets activated,
> and avoid flooding the logs afterwards.
>
> http://bugzilla.kernel.org/show_bug.cgi?id=11841
> "plenty of line "ACPI: EC: non-query interrupt received,
>  switching to interrupt mode" in dmesg"
>
> Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> Signed-off-by: Len Brown <len.brown@intel.com>
>   
I see the EC patches that cause this issue are part of 2.6.27-stable. 
So I think this patch should also be applied to the -stable tree.

Thanks
Alan

Re: [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Rafael J. Wysocki]

On Friday, 7 of November 2008, Alan Jenkins wrote:
> Len Brown wrote:
> > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> >
> > Make sure we can tell if the GPE storm workaround gets activated,
> > and avoid flooding the logs afterwards.
> >
> > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > "plenty of line "ACPI: EC: non-query interrupt received,
> >  switching to interrupt mode" in dmesg"
> >
> > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > Signed-off-by: Len Brown <len.brown@intel.com>
> >   
> I see the EC patches that cause this issue are part of 2.6.27-stable. 
> So I think this patch should also be applied to the -stable tree.

Good point, it should.

Thanks,
Rafael

Re: [stable] [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Greg KH]

On Fri, Nov 07, 2008 at 02:54:24PM +0100, Rafael J. Wysocki wrote:
> On Friday, 7 of November 2008, Alan Jenkins wrote:
> > Len Brown wrote:
> > > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > >
> > > Make sure we can tell if the GPE storm workaround gets activated,
> > > and avoid flooding the logs afterwards.
> > >
> > > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > > "plenty of line "ACPI: EC: non-query interrupt received,
> > >  switching to interrupt mode" in dmesg"
> > >
> > > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > > Signed-off-by: Len Brown <len.brown@intel.com>
> > >   
> > I see the EC patches that cause this issue are part of 2.6.27-stable. 
> > So I think this patch should also be applied to the -stable tree.
> 
> Good point, it should.

Ok, what is the git commit id that should be added to the .27-stable
tree?

thanks,

greg k-h

Re: [stable] [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Rafael J. Wysocki]

On Friday, 7 of November 2008, Greg KH wrote:
> On Fri, Nov 07, 2008 at 02:54:24PM +0100, Rafael J. Wysocki wrote:
> > On Friday, 7 of November 2008, Alan Jenkins wrote:
> > > Len Brown wrote:
> > > > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > >
> > > > Make sure we can tell if the GPE storm workaround gets activated,
> > > > and avoid flooding the logs afterwards.
> > > >
> > > > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > > > "plenty of line "ACPI: EC: non-query interrupt received,
> > > >  switching to interrupt mode" in dmesg"
> > > >
> > > > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > > > Signed-off-by: Len Brown <len.brown@intel.com>
> > > >   
> > > I see the EC patches that cause this issue are part of 2.6.27-stable. 
> > > So I think this patch should also be applied to the -stable tree.
> > 
> > Good point, it should.
> 
> Ok, what is the git commit id that should be added to the .27-stable
> tree?

Not yet in the Linus' tree, I'll let you know when it's merged.

Thanks,
Rafael

Re: [stable] [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Greg KH]

On Fri, Nov 07, 2008 at 11:15:24PM +0100, Rafael J. Wysocki wrote:
> On Friday, 7 of November 2008, Greg KH wrote:
> > On Fri, Nov 07, 2008 at 02:54:24PM +0100, Rafael J. Wysocki wrote:
> > > On Friday, 7 of November 2008, Alan Jenkins wrote:
> > > > Len Brown wrote:
> > > > > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > >
> > > > > Make sure we can tell if the GPE storm workaround gets activated,
> > > > > and avoid flooding the logs afterwards.
> > > > >
> > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > > > > "plenty of line "ACPI: EC: non-query interrupt received,
> > > > >  switching to interrupt mode" in dmesg"
> > > > >
> > > > > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > > > > Signed-off-by: Len Brown <len.brown@intel.com>
> > > > >   
> > > > I see the EC patches that cause this issue are part of 2.6.27-stable. 
> > > > So I think this patch should also be applied to the -stable tree.
> > > 
> > > Good point, it should.
> > 
> > Ok, what is the git commit id that should be added to the .27-stable
> > tree?
> 
> Not yet in the Linus' tree, I'll let you know when it's merged.

Thank you for doing this.

greg k-h

Re: [stable] [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Rafael J. Wysocki]

On Tuesday, 11 of November 2008, Greg KH wrote:
> On Fri, Nov 07, 2008 at 11:15:24PM +0100, Rafael J. Wysocki wrote:
> > On Friday, 7 of November 2008, Greg KH wrote:
> > > On Fri, Nov 07, 2008 at 02:54:24PM +0100, Rafael J. Wysocki wrote:
> > > > On Friday, 7 of November 2008, Alan Jenkins wrote:
> > > > > Len Brown wrote:
> > > > > > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > > >
> > > > > > Make sure we can tell if the GPE storm workaround gets activated,
> > > > > > and avoid flooding the logs afterwards.
> > > > > >
> > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > > > > > "plenty of line "ACPI: EC: non-query interrupt received,
> > > > > >  switching to interrupt mode" in dmesg"
> > > > > >
> > > > > > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > > > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > > > > > Signed-off-by: Len Brown <len.brown@intel.com>
> > > > > >   
> > > > > I see the EC patches that cause this issue are part of 2.6.27-stable. 
> > > > > So I think this patch should also be applied to the -stable tree.
> > > > 
> > > > Good point, it should.
> > > 
> > > Ok, what is the git commit id that should be added to the .27-stable
> > > tree?
> > 
> > Not yet in the Linus' tree, I'll let you know when it's merged.
> 
> Thank you for doing this.

Here:

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f8248434e6a11d7cd314281be3b39bbcf82fc243

The other EC patches in this merge are also regarded as -stable material,
but let's wait a couple of days to avoid possible regressions (although Alex
thinks there sould be none ;-)).

Thanks,
Rafael

Re: [stable] [PATCH 21/21] ACPI: EC: make kernel messages more useful when GPE storm is detected

[Greg KH]

On Thu, Nov 13, 2008 at 12:34:52AM +0100, Rafael J. Wysocki wrote:
> On Tuesday, 11 of November 2008, Greg KH wrote:
> > On Fri, Nov 07, 2008 at 11:15:24PM +0100, Rafael J. Wysocki wrote:
> > > On Friday, 7 of November 2008, Greg KH wrote:
> > > > On Fri, Nov 07, 2008 at 02:54:24PM +0100, Rafael J. Wysocki wrote:
> > > > > On Friday, 7 of November 2008, Alan Jenkins wrote:
> > > > > > Len Brown wrote:
> > > > > > > From: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > > > >
> > > > > > > Make sure we can tell if the GPE storm workaround gets activated,
> > > > > > > and avoid flooding the logs afterwards.
> > > > > > >
> > > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=11841
> > > > > > > "plenty of line "ACPI: EC: non-query interrupt received,
> > > > > > >  switching to interrupt mode" in dmesg"
> > > > > > >
> > > > > > > Signed-off-by: Alan Jenkins <alan-jenkins@tuffmail.co.uk>
> > > > > > > Acked-by: Alexey Starikovskiy <astarikovskiy@suse.de>
> > > > > > > Signed-off-by: Len Brown <len.brown@intel.com>
> > > > > > >   
> > > > > > I see the EC patches that cause this issue are part of 2.6.27-stable. 
> > > > > > So I think this patch should also be applied to the -stable tree.
> > > > > 
> > > > > Good point, it should.
> > > > 
> > > > Ok, what is the git commit id that should be added to the .27-stable
> > > > tree?
> > > 
> > > Not yet in the Linus' tree, I'll let you know when it's merged.
> > 
> > Thank you for doing this.
> 
> Here:
> 
> http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=f8248434e6a11d7cd314281be3b39bbcf82fc243
> 
> The other EC patches in this merge are also regarded as -stable material,
> but let's wait a couple of days to avoid possible regressions (although Alex
> thinks there sould be none ;-)).

Now applied.

If there's other git ids that should also be added, please let me know
what they are.

thanks,

greg k-h

Re: ACPI patches for 2.6.28-rc3

[Andrey Borzenkov]

Len Brown wrote:

> please speak up if you see any reason these
> pathces should not go into 2.6.28-rc3.
> 

You still did not include video brightness patches from Thomas; nor are they
in acpi-test. Are they delayed for -29 or rejected? If they are rejected it
would be nice to know why so we could fix them ...

Re: ACPI patches for 2.6.28-rc3

[Len Brown]

> You still did not include video brightness patches from Thomas; nor are they
> in acpi-test. Are they delayed for -29 or rejected? If they are rejected it
> would be nice to know why so we could fix them ...

hmmm, yeah, several folks have pointed this out,
not sure what happened there, but will follow up.

thanks,
-Len