[PATCH 0/4] ACPI-WMI patches for 2.6.25-rc3

[PATCH 0/4] ACPI-WMI patches for 2.6.25-rc3

[Carlos Corbacho]

Len,

The following patchset is a fix and some minor cleanups for 2.6.25-rc3.

The acer-wmi patches are minor cleanups to rename some unhelpful error
messages, remove a superflous warning (a user complained about this one),
and rename the mail LED in line with current reccomendations. There is no
functional change to the acer-wmi driver.

The WMI patch fixes a real bug observed on an Acer laptop, and has been
tested as working on the relevant hardware (yes, Acer can even manage to
violate ACPI-WMI :)

-Carlos
---

Carlos Corbacho (4):
      ACPI: WMI: Clean up handling of spec violating data blocks
      acer-wmi: Don't warn if mail LED cannot be detected
      acer-wmi: Make device detection error messages more descriptive
      acer-wmi: Rename mail LED correctly & remove hardcoded colour


 Documentation/laptops/acer-wmi.txt |    2 +-
 drivers/acpi/wmi.c                 |   10 ++++++----
 drivers/misc/acer-wmi.c            |   16 ++++++++--------
 3 files changed, 15 insertions(+), 13 deletions(-)

[PATCH 1/4] acer-wmi: Rename mail LED correctly & remove hardcoded colour

[Carlos Corbacho]

The mail LED name for acer-wmi currently hardcodes in the colour as green.
This is wrong, since many of the newer laptops now come with an orange
LED, and we have no way of telling what colour is used on a given system.

Also, rename the mail LED to be inline with the current recommendations of
the LED class documentation.

Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
---

 Documentation/laptops/acer-wmi.txt |    2 +-
 drivers/misc/acer-wmi.c            |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)


diff --git a/Documentation/laptops/acer-wmi.txt b/Documentation/laptops/acer-wmi.txt
index b066963..a346c86 100644
--- a/Documentation/laptops/acer-wmi.txt
+++ b/Documentation/laptops/acer-wmi.txt
@@ -169,7 +169,7 @@ can be added to acer-wmi.
 
 The LED is exposed through the LED subsystem, and can be found in:
 
-/sys/devices/platform/acer-wmi/leds/acer-mail:green/
+/sys/devices/platform/acer-wmi/leds/acer-wmi::mail/
 
 The mail LED is autodetected, so if you don't have one, the LED device won't
 be registered.
diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index cf71e31..d1d64db 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -761,7 +761,7 @@ enum led_brightness value)
 }
 
 static struct led_classdev mail_led = {
-	.name = "acer-mail:green",
+	.name = "acer-wmi::mail",
 	.brightness_set = mail_led_set,
 };
 

Re: [PATCH 1/4] acer-wmi: Rename mail LED correctly & remove hardcoded colour

[Len Brown]

applied..
-thanks,
len

On Sunday 24 February 2008, Carlos Corbacho wrote:
> The mail LED name for acer-wmi currently hardcodes in the colour as green.
> This is wrong, since many of the newer laptops now come with an orange
> LED, and we have no way of telling what colour is used on a given system.
> 
> Also, rename the mail LED to be inline with the current recommendations of
> the LED class documentation.
> 
> Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
> ---
> 
>  Documentation/laptops/acer-wmi.txt |    2 +-
>  drivers/misc/acer-wmi.c            |    2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/Documentation/laptops/acer-wmi.txt b/Documentation/laptops/acer-wmi.txt
> index b066963..a346c86 100644
> --- a/Documentation/laptops/acer-wmi.txt
> +++ b/Documentation/laptops/acer-wmi.txt
> @@ -169,7 +169,7 @@ can be added to acer-wmi.
>  
>  The LED is exposed through the LED subsystem, and can be found in:
>  
> -/sys/devices/platform/acer-wmi/leds/acer-mail:green/
> +/sys/devices/platform/acer-wmi/leds/acer-wmi::mail/
>  
>  The mail LED is autodetected, so if you don't have one, the LED device won't
>  be registered.
> diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
> index cf71e31..d1d64db 100644
> --- a/drivers/misc/acer-wmi.c
> +++ b/drivers/misc/acer-wmi.c
> @@ -761,7 +761,7 @@ enum led_brightness value)
>  }
>  
>  static struct led_classdev mail_led = {
> -	.name = "acer-mail:green",
> +	.name = "acer-wmi::mail",
>  	.brightness_set = mail_led_set,
>  };
>  
> 
> -
> 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 2/4] acer-wmi: Make device detection error messages more descriptive

[Carlos Corbacho]

The current device detection error messages are all copy & pasted - make
them more descriptive so it's easier to see where in the code a problem
occurs.

Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
---

 drivers/misc/acer-wmi.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)


diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index d1d64db..99beeb8 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -1052,11 +1052,12 @@ static int __init acer_wmi_init(void)
 
 	if (wmi_has_guid(WMID_GUID2) && interface) {
 		if (ACPI_FAILURE(WMID_set_capabilities())) {
-			printk(ACER_ERR "Unable to detect available devices\n");
+			printk(ACER_ERR "Unable to detect available WMID ");
+			printk(KERN_CONT "devices\n");
 			return -ENODEV;
 		}
 	} else if (!wmi_has_guid(WMID_GUID2) && interface) {
-		printk(ACER_ERR "Unable to detect available devices\n");
+		printk(ACER_ERR "No WMID device detection method found\n");
 		return -ENODEV;
 	}
 
@@ -1064,7 +1065,8 @@ static int __init acer_wmi_init(void)
 		interface = &AMW0_interface;
 
 		if (ACPI_FAILURE(AMW0_set_capabilities())) {
-			printk(ACER_ERR "Unable to detect available devices\n");
+			printk(ACER_ERR "Unable to detect available AMW0 ");
+			printk(KERN_CONT "devices\n");
 			return -ENODEV;
 		}
 	}

Re: [PATCH 2/4] acer-wmi: Make device detection error messages more descriptive

[Len Brown]

On Sunday 24 February 2008, Carlos Corbacho wrote:
> The current device detection error messages are all copy & pasted - make
> them more descriptive so it's easier to see where in the code a problem
> occurs.
> 
> Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
> ---
> 
>  drivers/misc/acer-wmi.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
> index d1d64db..99beeb8 100644
> --- a/drivers/misc/acer-wmi.c
> +++ b/drivers/misc/acer-wmi.c
> @@ -1052,11 +1052,12 @@ static int __init acer_wmi_init(void)
>  
>  	if (wmi_has_guid(WMID_GUID2) && interface) {
>  		if (ACPI_FAILURE(WMID_set_capabilities())) {
> -			printk(ACER_ERR "Unable to detect available devices\n");
> +			printk(ACER_ERR "Unable to detect available WMID ");

While KERN_CONT will shut up checkpatch, I don't think
it isn't what you want here
since acer_wmi is loaded well afer we've gone SMP.

> +			printk(KERN_CONT "devices\n");

Instead, to get under 80 columns, you simpmly want to do something like this:

printk(ACER_ERR "Unable to detect available WMID "
	"devices");

as the pre-processor will concatenate adjacent strings for you at build time.

-Len

>  			return -ENODEV;
>  		}
>  	} else if (!wmi_has_guid(WMID_GUID2) && interface) {
> -		printk(ACER_ERR "Unable to detect available devices\n");
> +		printk(ACER_ERR "No WMID device detection method found\n");
>  		return -ENODEV;
>  	}
>  
> @@ -1064,7 +1065,8 @@ static int __init acer_wmi_init(void)
>  		interface = &AMW0_interface;
>  
>  		if (ACPI_FAILURE(AMW0_set_capabilities())) {
> -			printk(ACER_ERR "Unable to detect available devices\n");
> +			printk(ACER_ERR "Unable to detect available AMW0 ");
> +			printk(KERN_CONT "devices\n");
>  			return -ENODEV;
>  		}
>  	}
> 
> -
> 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: [PATCH 2/4] acer-wmi: Make device detection error messages more descriptive

[Carlos Corbacho]

On Tuesday 11 March 2008 21:57:37 Len Brown wrote:
> Instead, to get under 80 columns, you simpmly want to do something like
> this:
>
> printk(ACER_ERR "Unable to detect available WMID "
> 	"devices");

Will do.

-Carlos
-- 
E-Mail: carlos@strangeworlds.co.uk
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

[PATCH 3/4] acer-wmi: Don't warn if mail LED cannot be detected

[Carlos Corbacho]

This warning confuses users, who think it is an error. Not detecting the
mail LED simply means it isn't there, so let's not unduly panic users.

Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
---

 drivers/misc/acer-wmi.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)


diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
index 99beeb8..68b12d6 100644
--- a/drivers/misc/acer-wmi.c
+++ b/drivers/misc/acer-wmi.c
@@ -1071,10 +1071,8 @@ static int __init acer_wmi_init(void)
 		}
 	}
 
-	if (wmi_has_guid(AMW0_GUID1)) {
-		if (ACPI_FAILURE(AMW0_find_mailled()))
-			printk(ACER_ERR "Unable to detect mail LED\n");
-	}
+	if (wmi_has_guid(AMW0_GUID1))
+		AMW0_find_mailled();
 
 	find_quirks();
 

Re: [PATCH 3/4] acer-wmi: Don't warn if mail LED cannot be detected

[Len Brown]

applied

thanks,
-len

On Sunday 24 February 2008, Carlos Corbacho wrote:
> This warning confuses users, who think it is an error. Not detecting the
> mail LED simply means it isn't there, so let's not unduly panic users.
> 
> Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
> ---
> 
>  drivers/misc/acer-wmi.c |    6 ++----
>  1 files changed, 2 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/drivers/misc/acer-wmi.c b/drivers/misc/acer-wmi.c
> index 99beeb8..68b12d6 100644
> --- a/drivers/misc/acer-wmi.c
> +++ b/drivers/misc/acer-wmi.c
> @@ -1071,10 +1071,8 @@ static int __init acer_wmi_init(void)
>  		}
>  	}
>  
> -	if (wmi_has_guid(AMW0_GUID1)) {
> -		if (ACPI_FAILURE(AMW0_find_mailled()))
> -			printk(ACER_ERR "Unable to detect mail LED\n");
> -	}
> +	if (wmi_has_guid(AMW0_GUID1))
> +		AMW0_find_mailled();
>  
>  	find_quirks();
>  
> 
> -
> 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 4/4] ACPI: WMI: Clean up handling of spec violating data blocks

[Carlos Corbacho]

Acer violate the ACPI-WMI spec by declaring some of their data blocks as
expensive, but with no corresponding WCxx method. There is already some
workaround code in to handle the initial WCxx call (we just ignore a
failure here); but we need to properly check if the second, "clean up",
WCxx call is actually needed or not, rather than fail simply because it
isn't there.

Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
---

 drivers/acpi/wmi.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)


diff --git a/drivers/acpi/wmi.c b/drivers/acpi/wmi.c
index efacc9f..c33b1c6 100644
--- a/drivers/acpi/wmi.c
+++ b/drivers/acpi/wmi.c
@@ -293,7 +293,7 @@ struct acpi_buffer *out)
 {
 	struct guid_block *block = NULL;
 	struct wmi_block *wblock = NULL;
-	acpi_handle handle;
+	acpi_handle handle, wc_handle;
 	acpi_status status, wc_status = AE_ERROR;
 	struct acpi_object_list input, wc_input;
 	union acpi_object wc_params[1], wq_params[1];
@@ -338,8 +338,10 @@ struct acpi_buffer *out)
 		 * expensive, but have no corresponding WCxx method. So we
 		 * should not fail if this happens.
 		 */
-		wc_status = acpi_evaluate_object(handle, wc_method,
-			&wc_input, NULL);
+		wc_status = acpi_get_handle(handle, wc_method, &wc_handle);
+		if (ACPI_SUCCESS(wc_status))
+			wc_status = acpi_evaluate_object(handle, wc_method,
+				&wc_input, NULL);
 	}
 
 	strcpy(method, "WQ");
@@ -351,7 +353,7 @@ struct acpi_buffer *out)
 	 * If ACPI_WMI_EXPENSIVE, call the relevant WCxx method, even if
 	 * the WQxx method failed - we should disable collection anyway.
 	 */
-	if ((block->flags & ACPI_WMI_EXPENSIVE) && wc_status) {
+	if ((block->flags & ACPI_WMI_EXPENSIVE) && ACPI_SUCCESS(wc_status)) {
 		wc_params[0].integer.value = 0;
 		status = acpi_evaluate_object(handle,
 		wc_method, &wc_input, NULL);

Re: [PATCH 4/4] ACPI: WMI: Clean up handling of spec violating data blocks

[Len Brown]

applied

thanks,
-len

On Sunday 24 February 2008, Carlos Corbacho wrote:
> Acer violate the ACPI-WMI spec by declaring some of their data blocks as
> expensive, but with no corresponding WCxx method. There is already some
> workaround code in to handle the initial WCxx call (we just ignore a
> failure here); but we need to properly check if the second, "clean up",
> WCxx call is actually needed or not, rather than fail simply because it
> isn't there.
> 
> Signed-off-by: Carlos Corbacho <carlos@strangeworlds.co.uk>
> ---
> 
>  drivers/acpi/wmi.c |   10 ++++++----
>  1 files changed, 6 insertions(+), 4 deletions(-)
> 
> 
> diff --git a/drivers/acpi/wmi.c b/drivers/acpi/wmi.c
> index efacc9f..c33b1c6 100644
> --- a/drivers/acpi/wmi.c
> +++ b/drivers/acpi/wmi.c
> @@ -293,7 +293,7 @@ struct acpi_buffer *out)
>  {
>  	struct guid_block *block = NULL;
>  	struct wmi_block *wblock = NULL;
> -	acpi_handle handle;
> +	acpi_handle handle, wc_handle;
>  	acpi_status status, wc_status = AE_ERROR;
>  	struct acpi_object_list input, wc_input;
>  	union acpi_object wc_params[1], wq_params[1];
> @@ -338,8 +338,10 @@ struct acpi_buffer *out)
>  		 * expensive, but have no corresponding WCxx method. So we
>  		 * should not fail if this happens.
>  		 */
> -		wc_status = acpi_evaluate_object(handle, wc_method,
> -			&wc_input, NULL);
> +		wc_status = acpi_get_handle(handle, wc_method, &wc_handle);
> +		if (ACPI_SUCCESS(wc_status))
> +			wc_status = acpi_evaluate_object(handle, wc_method,
> +				&wc_input, NULL);
>  	}
>  
>  	strcpy(method, "WQ");
> @@ -351,7 +353,7 @@ struct acpi_buffer *out)
>  	 * If ACPI_WMI_EXPENSIVE, call the relevant WCxx method, even if
>  	 * the WQxx method failed - we should disable collection anyway.
>  	 */
> -	if ((block->flags & ACPI_WMI_EXPENSIVE) && wc_status) {
> +	if ((block->flags & ACPI_WMI_EXPENSIVE) && ACPI_SUCCESS(wc_status)) {
>  		wc_params[0].integer.value = 0;
>  		status = acpi_evaluate_object(handle,
>  		wc_method, &wc_input, NULL);
> 
> -
> 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
>