public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* bug in acer-wmi.c, wmid3_set_device_status(), initialization of params.
@ 2011-05-31  3:03 Joern Heissler
  0 siblings, 0 replies; 5+ messages in thread
From: Joern Heissler @ 2011-05-31  3:03 UTC (permalink / raw)
  To: linux-acpi

Hello,

I think I have found a bug in acer-wmi.c,
commit 987dfbaa65b2c3568b85e29d2598da08a011ee09:

The function wmid3_set_device_status contains:

    struct wmid3_gds_input_param params = {
        .function_num = 0x1,
        .hotkey_number = 0x01,
        .devices = ACER_WMID3_GDS_WIRELESS &
                ACER_WMID3_GDS_THREEG &
                ACER_WMID3_GDS_WIMAX &
                ACER_WMID3_GDS_BLUETOOTH,
    };

(1<<0) & (1<<6) & (1<<7) & (1<<11) is zero. Meant bitwise or?

And could someone please update the driver to support my laptop?
See https://bugzilla.kernel.org/show_bug.cgi?id=36322 for details :-)

Cheery
Joern Heissler

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: bug in acer-wmi.c, wmid3_set_device_status(), initialization of params.
@ 2011-05-31  3:39 Joey Lee
  0 siblings, 0 replies; 5+ messages in thread
From: Joey Lee @ 2011-05-31  3:39 UTC (permalink / raw)
  To: linux-acpi; +Cc: linux-acpi

Hi Joern, 

於 二，2011-05-31 於 15:03 +1200，Joern Heissler 提到：
> Hello,
> 
> I think I have found a bug in acer-wmi.c,
> commit 987dfbaa65b2c3568b85e29d2598da08a011ee09:
> 
> The function wmid3_set_device_status contains:
> 
>     struct wmid3_gds_input_param params = {
>         .function_num = 0x1,
>         .hotkey_number = 0x01,
>         .devices = ACER_WMID3_GDS_WIRELESS &
>                 ACER_WMID3_GDS_THREEG &
>                 ACER_WMID3_GDS_WIMAX &
>                 ACER_WMID3_GDS_BLUETOOTH,
>     };
> 
> (1<<0) & (1<<6) & (1<<7) & (1<<11) is zero. Meant bitwise or?
> 

OK, it's my mistake, must be OR but not AND.
I will generate a fix patch, I will test it again, please kindly help
test it later.

> And could someone please update the driver to support my laptop?
> See https://bugzilla.kernel.org/show_bug.cgi?id=36322 for details :-)
> 
> Cheery
> Joern Heissler

I will take look at.


Thank's
Joey Lee

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: bug in acer-wmi.c, wmid3_set_device_status(), initialization of params.
@ 2011-05-31  4:46 Joey Lee
  2011-05-31  5:10 ` Joern Heissler
  0 siblings, 1 reply; 5+ messages in thread
From: Joey Lee @ 2011-05-31  4:46 UTC (permalink / raw)
  To: linux-acpi; +Cc: Joey Lee, linux-acpi

Hi Joern, 

於 一，2011-05-30 於 21:39 -0600，Joey Lee 提到：
> Hi Joern, 
> 
> 於 二，2011-05-31 於 15:03 +1200，Joern Heissler 提到：
> > Hello,
> > 
> > I think I have found a bug in acer-wmi.c,
> > commit 987dfbaa65b2c3568b85e29d2598da08a011ee09:
> > 
> > The function wmid3_set_device_status contains:
> > 
> >     struct wmid3_gds_input_param params = {
> >         .function_num = 0x1,
> >         .hotkey_number = 0x01,
> >         .devices = ACER_WMID3_GDS_WIRELESS &
> >                 ACER_WMID3_GDS_THREEG &
> >                 ACER_WMID3_GDS_WIMAX &
> >                 ACER_WMID3_GDS_BLUETOOTH,
> >     };
> > 
> > (1<<0) & (1<<6) & (1<<7) & (1<<11) is zero. Meant bitwise or?
> > 
> 
> OK, it's my mistake, must be OR but not AND.
> I will generate a fix patch, I will test it again, please kindly help
> test it later.
> 

Please kindly help to review this patch for bitwise bug:


>From 57756e3f0c727b07314485833e4b9b557bf158ce Mon Sep 17 00:00:00 2001
From: Lee, Chun-Yi <jlee@novell.com>
Date: Tue, 31 May 2011 12:36:09 +0800
Subject: [PATCH] acer-wmi: fix bitwise bug when set device state

Fix a bitwise bug that was found by Joern Heissler, it must be OR
but not AND when we query current device state.

Cc: Joern Heissler <linux-acpi@joern.heissler.de>
Signed-off-by: Lee, Chun-Yi <jlee@novell.com>
---
 drivers/platform/x86/acer-wmi.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
index 005417b..7d4675a 100644
--- a/drivers/platform/x86/acer-wmi.c
+++ b/drivers/platform/x86/acer-wmi.c
@@ -1156,9 +1156,9 @@ static acpi_status wmid3_set_device_status(u32 value, u16 device)
 	struct wmid3_gds_input_param params = {
 		.function_num = 0x1,
 		.hotkey_number = 0x01,
-		.devices = ACER_WMID3_GDS_WIRELESS &
-				ACER_WMID3_GDS_THREEG &
-				ACER_WMID3_GDS_WIMAX &
+		.devices = ACER_WMID3_GDS_WIRELESS |
+				ACER_WMID3_GDS_THREEG |
+				ACER_WMID3_GDS_WIMAX |
 				ACER_WMID3_GDS_BLUETOOTH,
 	};
 	struct acpi_buffer input = {
-- 
1.6.0.2



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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: bug in acer-wmi.c, wmid3_set_device_status(), initialization of params.
  2011-05-31  4:46 bug in acer-wmi.c, wmid3_set_device_status(), initialization of params Joey Lee
@ 2011-05-31  5:10 ` Joern Heissler
  2011-05-31  6:12   ` Joey Lee
  0 siblings, 1 reply; 5+ messages in thread
From: Joern Heissler @ 2011-05-31  5:10 UTC (permalink / raw)
  To: Joey Lee; +Cc: linux-acpi

On Mon, May 30, 2011 at 10:46:03PM -0600, Joey Lee wrote:
> Hi Joern, 
> 
> 於 一，2011-05-30 於 21:39 -0600，Joey Lee 提到：
> > Hi Joern, 
> > 
> > 於 二，2011-05-31 於 15:03 +1200，Joern Heissler 提到：
> > > Hello,
> > > 
> > > I think I have found a bug in acer-wmi.c,
> > > commit 987dfbaa65b2c3568b85e29d2598da08a011ee09:
> > > 
> > > The function wmid3_set_device_status contains:
> > > 
> > >     struct wmid3_gds_input_param params = {
> > >         .function_num = 0x1,
> > >         .hotkey_number = 0x01,
> > >         .devices = ACER_WMID3_GDS_WIRELESS &
> > >                 ACER_WMID3_GDS_THREEG &
> > >                 ACER_WMID3_GDS_WIMAX &
> > >                 ACER_WMID3_GDS_BLUETOOTH,
> > >     };
> > > 
> > > (1<<0) & (1<<6) & (1<<7) & (1<<11) is zero. Meant bitwise or?
> > > 
> > 
> > OK, it's my mistake, must be OR but not AND.
> > I will generate a fix patch, I will test it again, please kindly help
> > test it later.
> > 
> 
> Please kindly help to review this patch for bitwise bug:
> 
> 
> >From 57756e3f0c727b07314485833e4b9b557bf158ce Mon Sep 17 00:00:00 2001
> From: Lee, Chun-Yi <jlee@novell.com>
> Date: Tue, 31 May 2011 12:36:09 +0800
> Subject: [PATCH] acer-wmi: fix bitwise bug when set device state
> 
> Fix a bitwise bug that was found by Joern Heissler, it must be OR
> but not AND when we query current device state.
> 
> Cc: Joern Heissler <linux-acpi@joern.heissler.de>
> Signed-off-by: Lee, Chun-Yi <jlee@novell.com>
> ---
>  drivers/platform/x86/acer-wmi.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> index 005417b..7d4675a 100644
> --- a/drivers/platform/x86/acer-wmi.c
> +++ b/drivers/platform/x86/acer-wmi.c
> @@ -1156,9 +1156,9 @@ static acpi_status wmid3_set_device_status(u32 value, u16 device)
>  	struct wmid3_gds_input_param params = {
>  		.function_num = 0x1,
>  		.hotkey_number = 0x01,
> -		.devices = ACER_WMID3_GDS_WIRELESS &
> -				ACER_WMID3_GDS_THREEG &
> -				ACER_WMID3_GDS_WIMAX &
> +		.devices = ACER_WMID3_GDS_WIRELESS |
> +				ACER_WMID3_GDS_THREEG |
> +				ACER_WMID3_GDS_WIMAX |
>  				ACER_WMID3_GDS_BLUETOOTH,
>  	};
>  	struct acpi_buffer input = {
> -- 
> 1.6.0.2

The patch looks good and works in my heavily stripped-down version of
acer-wmi.c.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: bug in acer-wmi.c, wmid3_set_device_status(), initialization of params.
  2011-05-31  5:10 ` Joern Heissler
@ 2011-05-31  6:12   ` Joey Lee
  0 siblings, 0 replies; 5+ messages in thread
From: Joey Lee @ 2011-05-31  6:12 UTC (permalink / raw)
  To: linux-acpi; +Cc: Joey Lee, linux-acpi

於 二，2011-05-31 於 17:10 +1200，Joern Heissler 提到：
> On Mon, May 30, 2011 at 10:46:03PM -0600, Joey Lee wrote:
> > Hi Joern, 
> > 
> > 於 一，2011-05-30 於 21:39 -0600，Joey Lee 提到：
> > > Hi Joern, 
> > > 
> > > 於 二，2011-05-31 於 15:03 +1200，Joern Heissler 提到：
> > > > Hello,
> > > > 
> > > > I think I have found a bug in acer-wmi.c,
> > > > commit 987dfbaa65b2c3568b85e29d2598da08a011ee09:
> > > > 
> > > > The function wmid3_set_device_status contains:
> > > > 
> > > >     struct wmid3_gds_input_param params = {
> > > >         .function_num = 0x1,
> > > >         .hotkey_number = 0x01,
> > > >         .devices = ACER_WMID3_GDS_WIRELESS &
> > > >                 ACER_WMID3_GDS_THREEG &
> > > >                 ACER_WMID3_GDS_WIMAX &
> > > >                 ACER_WMID3_GDS_BLUETOOTH,
> > > >     };
> > > > 
> > > > (1<<0) & (1<<6) & (1<<7) & (1<<11) is zero. Meant bitwise or?
> > > > 
> > > 
> > > OK, it's my mistake, must be OR but not AND.
> > > I will generate a fix patch, I will test it again, please kindly help
> > > test it later.
> > > 
> > 
> > Please kindly help to review this patch for bitwise bug:
> > 
> > 
> > >From 57756e3f0c727b07314485833e4b9b557bf158ce Mon Sep 17 00:00:00 2001
> > From: Lee, Chun-Yi <jlee@novell.com>
> > Date: Tue, 31 May 2011 12:36:09 +0800
> > Subject: [PATCH] acer-wmi: fix bitwise bug when set device state
> > 
> > Fix a bitwise bug that was found by Joern Heissler, it must be OR
> > but not AND when we query current device state.
> > 
> > Cc: Joern Heissler <linux-acpi@joern.heissler.de>
> > Signed-off-by: Lee, Chun-Yi <jlee@novell.com>
> > ---
> >  drivers/platform/x86/acer-wmi.c |    6 +++---
> >  1 files changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/acer-wmi.c b/drivers/platform/x86/acer-wmi.c
> > index 005417b..7d4675a 100644
> > --- a/drivers/platform/x86/acer-wmi.c
> > +++ b/drivers/platform/x86/acer-wmi.c
> > @@ -1156,9 +1156,9 @@ static acpi_status wmid3_set_device_status(u32 value, u16 device)
> >  	struct wmid3_gds_input_param params = {
> >  		.function_num = 0x1,
> >  		.hotkey_number = 0x01,
> > -		.devices = ACER_WMID3_GDS_WIRELESS &
> > -				ACER_WMID3_GDS_THREEG &
> > -				ACER_WMID3_GDS_WIMAX &
> > +		.devices = ACER_WMID3_GDS_WIRELESS |
> > +				ACER_WMID3_GDS_THREEG |
> > +				ACER_WMID3_GDS_WIMAX |
> >  				ACER_WMID3_GDS_BLUETOOTH,
> >  	};
> >  	struct acpi_buffer input = {
> > -- 
> > 1.6.0.2
> 
> The patch looks good and works in my heavily stripped-down version of
> acer-wmi.c.
> 

Thank's for your review, will send out patch to 3.0-rc2

Joey Lee

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

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-31  6:12 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-05-31  4:46 bug in acer-wmi.c, wmid3_set_device_status(), initialization of params Joey Lee
2011-05-31  5:10 ` Joern Heissler
2011-05-31  6:12   ` Joey Lee
  -- strict thread matches above, loose matches on Subject: below --
2011-05-31  3:39 Joey Lee
2011-05-31  3:03 Joern Heissler

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox