ACPI-WMI patches for 2.6.25-rc3
Subsystems (3)
cover ACPI-WMI patches for 2.6.25-rc3
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
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.
---
Documentation/laptops/acer-wmi.txt | 2 +- drivers/misc/acer-wmi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
Documentation/laptops/acer-wmi.txt +1 -1
@@ -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.
drivers/misc/acer-wmi.c +1 -1
@@ -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
applied..
-thanks,
len
Quoted 47 lines · 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 <[email protected]>---
Documentation/laptops/acer-wmi.txt | 2 +- drivers/misc/acer-wmi.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)Documentation/laptops/acer-wmi.txt +1
-1@@ -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.drivers/misc/acer-wmi.c +1
-2@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 2/4 acer-wmi: Make device detection error messages more descriptive
In reply to Carlos CorbachoThe 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.
---
drivers/misc/acer-wmi.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)
drivers/misc/acer-wmi.c +5 -3
@@ -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 ; } }
Len Brown
reviewerRe: [PATCH 2/4] acer-wmi: Make device detection error messages more descriptive
Quoted 16 lines · 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 <[email protected]>---
drivers/misc/acer-wmi.c | 8 +++++--- 1 files changed, 5 insertions(+), 3 deletions(-)drivers/misc/acer-wmi.c +0
-0
drivers/misc/acer-wmi.c static int __init acer_wmi_init(void) · line 1055
Quoted 5 lines
drivers/misc/acer-wmi.c +1
-1@@ -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.
drivers/misc/acer-wmi.c static int __init acer_wmi_init(void) · line 1056
Quoted 1 line
drivers/misc/acer-wmi.c +1
-0+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
Quoted 24 lines
drivers/misc/acer-wmi.c +3
-3return -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/4] acer-wmi: Make device detection error messages more descriptive
Quoted 5 lines · 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
Signature · 3 lines
E-Mail: [email protected] Web: strangeworlds.co.uk GPG Key ID: 0x23EE722D
PATCH 3/4 acer-wmi: Don't warn if mail LED cannot be detected
In reply to Carlos CorbachoThis 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.
---
drivers/misc/acer-wmi.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
drivers/misc/acer-wmi.c +2 -4
@@ -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
applied
thanks,
-len
Quoted 33 lines · 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 <[email protected]>---
drivers/misc/acer-wmi.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)drivers/misc/acer-wmi.c +2
-5@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
PATCH 4/4 ACPI: WMI: Clean up handling of spec violating data blocks
In reply to Carlos CorbachoAcer 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.
---
drivers/acpi/wmi.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/acpi/wmi.c +6 -4
@@ -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
applied
thanks,
-len
Quoted 55 lines · 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 <[email protected]>---
drivers/acpi/wmi.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-)drivers/acpi/wmi.c +6
-5@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html