Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-08 Thread João Paulo Rechi Vita 
On 7 February 2017 at 18:37, Andy Shevchenko  wrote:
> On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
>  wrote:
>> On 4 February 2017 at 10:02, Andy Shevchenko  
>> wrote:
>>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
>>> wrote:
 On 27 January 2017 at 10:26, Andy Shevchenko  
 wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>  wrote:
>>>
>> +static const struct acpi_device_id device_ids[] = {
>> +   {"ATK4001", 0},
>> +   {"ATK4002", 0},
>
> ...and use it as a parameter here.
>

 I'm not exactly sure how to do that, as driver_data is a
 kernel_ulong_t. Can you please elaborate a bit more?
>>>
>>> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>>
>>
>> The code you suggested:
>>
>> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
>> static const struct acpi_device_id device_ids[] = {
>>{"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>{"", 0},
>> };
>>
>> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
>> aggregate value used where an integer was expected", so I guess you
>> meant the address of that struct (_id_params), right? I don't
>> see any way how we could have the actual data in .driver_data.
>
> Yes, you are right. I kept in mind array when was suggesting this.
>
>> Even after moving the parameters in the driver_data field of
>> device_ids[], I'm not able retrieve them with acpi_match_device(),
>> because the "struct device" from the "struct acpi_device" that comes
>> as an input parameter in asus_wireless_add() does not have a acpi
>> companion device associated with it, so acpi_match_device() returns
>> NULL. I still need to manually loop through device_ids[] in order to
>> retrieve the .driver_data associated with the HID, and I see the same
>> pattern in the i2c-scmi driver.
>
> And what prevents us to set companion device?
>

Assuming this in the scope of a platform driver (genuine question, as
I don't see anything under drivers/platform/ doing so), nothing
prevents us to set it. But when doing so, acpi_match_device() still
fails, because acpi_get_first_physical_node() returns a different
"struct dev *" in acpi_primary_dev_companion().

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-08 Thread João Paulo Rechi Vita 
On 7 February 2017 at 18:37, Andy Shevchenko  wrote:
> On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
>  wrote:
>> On 4 February 2017 at 10:02, Andy Shevchenko  
>> wrote:
>>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
>>> wrote:
 On 27 January 2017 at 10:26, Andy Shevchenko  
 wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>  wrote:
>>>
>> +static const struct acpi_device_id device_ids[] = {
>> +   {"ATK4001", 0},
>> +   {"ATK4002", 0},
>
> ...and use it as a parameter here.
>

 I'm not exactly sure how to do that, as driver_data is a
 kernel_ulong_t. Can you please elaborate a bit more?
>>>
>>> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>>
>>
>> The code you suggested:
>>
>> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
>> static const struct acpi_device_id device_ids[] = {
>>{"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>{"", 0},
>> };
>>
>> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
>> aggregate value used where an integer was expected", so I guess you
>> meant the address of that struct (_id_params), right? I don't
>> see any way how we could have the actual data in .driver_data.
>
> Yes, you are right. I kept in mind array when was suggesting this.
>
>> Even after moving the parameters in the driver_data field of
>> device_ids[], I'm not able retrieve them with acpi_match_device(),
>> because the "struct device" from the "struct acpi_device" that comes
>> as an input parameter in asus_wireless_add() does not have a acpi
>> companion device associated with it, so acpi_match_device() returns
>> NULL. I still need to manually loop through device_ids[] in order to
>> retrieve the .driver_data associated with the HID, and I see the same
>> pattern in the i2c-scmi driver.
>
> And what prevents us to set companion device?
>

Assuming this in the scope of a platform driver (genuine question, as
I don't see anything under drivers/platform/ doing so), nothing
prevents us to set it. But when doing so, acpi_match_device() still
fails, because acpi_get_first_physical_node() returns a different
"struct dev *" in acpi_primary_dev_companion().

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-07 Thread Andy Shevchenko 
On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
 wrote:
> On 4 February 2017 at 10:02, Andy Shevchenko  
> wrote:
>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
>> wrote:
>>> On 27 January 2017 at 10:26, Andy Shevchenko  
>>> wrote:
 On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
  wrote:
>>
> +static const struct acpi_device_id device_ids[] = {
> +   {"ATK4001", 0},
> +   {"ATK4002", 0},

 ...and use it as a parameter here.

>>>
>>> I'm not exactly sure how to do that, as driver_data is a
>>> kernel_ulong_t. Can you please elaborate a bit more?
>>
>> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>
>
> The code you suggested:
>
> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
> static const struct acpi_device_id device_ids[] = {
>{"ATK4001", (kernel_ulong_t)atk4001_id_params},
>{"", 0},
> };
>
> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
> aggregate value used where an integer was expected", so I guess you
> meant the address of that struct (_id_params), right? I don't
> see any way how we could have the actual data in .driver_data.

Yes, you are right. I kept in mind array when was suggesting this.

> Even after moving the parameters in the driver_data field of
> device_ids[], I'm not able retrieve them with acpi_match_device(),
> because the "struct device" from the "struct acpi_device" that comes
> as an input parameter in asus_wireless_add() does not have a acpi
> companion device associated with it, so acpi_match_device() returns
> NULL. I still need to manually loop through device_ids[] in order to
> retrieve the .driver_data associated with the HID, and I see the same
> pattern in the i2c-scmi driver.

And what prevents us to set companion device?

>
> I'm sending the next revision for feedback, please advise if I'm
> missing any detail, or if you want this implemented differently.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-07 Thread Andy Shevchenko 
On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
 wrote:
> On 4 February 2017 at 10:02, Andy Shevchenko  
> wrote:
>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
>> wrote:
>>> On 27 January 2017 at 10:26, Andy Shevchenko  
>>> wrote:
 On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
  wrote:
>>
> +static const struct acpi_device_id device_ids[] = {
> +   {"ATK4001", 0},
> +   {"ATK4002", 0},

 ...and use it as a parameter here.

>>>
>>> I'm not exactly sure how to do that, as driver_data is a
>>> kernel_ulong_t. Can you please elaborate a bit more?
>>
>> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>>
>
> The code you suggested:
>
> static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
> static const struct acpi_device_id device_ids[] = {
>{"ATK4001", (kernel_ulong_t)atk4001_id_params},
>{"", 0},
> };
>
> does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
> aggregate value used where an integer was expected", so I guess you
> meant the address of that struct (_id_params), right? I don't
> see any way how we could have the actual data in .driver_data.

Yes, you are right. I kept in mind array when was suggesting this.

> Even after moving the parameters in the driver_data field of
> device_ids[], I'm not able retrieve them with acpi_match_device(),
> because the "struct device" from the "struct acpi_device" that comes
> as an input parameter in asus_wireless_add() does not have a acpi
> companion device associated with it, so acpi_match_device() returns
> NULL. I still need to manually loop through device_ids[] in order to
> retrieve the .driver_data associated with the HID, and I see the same
> pattern in the i2c-scmi driver.

And what prevents us to set companion device?

>
> I'm sending the next revision for feedback, please advise if I'm
> missing any detail, or if you want this implemented differently.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-07 Thread João Paulo Rechi Vita 
On 4 February 2017 at 10:02, Andy Shevchenko  wrote:
> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
> wrote:
>> On 27 January 2017 at 10:26, Andy Shevchenko  
>> wrote:
>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>>  wrote:
>
 +static const struct acpi_device_id device_ids[] = {
 +   {"ATK4001", 0},
 +   {"ATK4002", 0},
>>>
>>> ...and use it as a parameter here.
>>>
>>
>> I'm not exactly sure how to do that, as driver_data is a
>> kernel_ulong_t. Can you please elaborate a bit more?
>
> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>

The code you suggested:

static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
static const struct acpi_device_id device_ids[] = {
   {"ATK4001", (kernel_ulong_t)atk4001_id_params},
   {"", 0},
};

does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
aggregate value used where an integer was expected", so I guess you
meant the address of that struct (_id_params), right? I don't
see any way how we could have the actual data in .driver_data.

Even after moving the parameters in the driver_data field of
device_ids[], I'm not able retrieve them with acpi_match_device(),
because the "struct device" from the "struct acpi_device" that comes
as an input parameter in asus_wireless_add() does not have a acpi
companion device associated with it, so acpi_match_device() returns
NULL. I still need to manually loop through device_ids[] in order to
retrieve the .driver_data associated with the HID, and I see the same
pattern in the i2c-scmi driver.

I'm sending the next revision for feedback, please advise if I'm
missing any detail, or if you want this implemented differently.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-07 Thread João Paulo Rechi Vita 
On 4 February 2017 at 10:02, Andy Shevchenko  wrote:
> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  
> wrote:
>> On 27 January 2017 at 10:26, Andy Shevchenko  
>> wrote:
>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>>  wrote:
>
 +static const struct acpi_device_id device_ids[] = {
 +   {"ATK4001", 0},
 +   {"ATK4002", 0},
>>>
>>> ...and use it as a parameter here.
>>>
>>
>> I'm not exactly sure how to do that, as driver_data is a
>> kernel_ulong_t. Can you please elaborate a bit more?
>
> {"ATK4001", (kernel_ulong_t)atk4001_id_params},
>

The code you suggested:

static const struct hswc_params atk4001_id_params = {0x0, 0x1, 0x2};
static const struct acpi_device_id device_ids[] = {
   {"ATK4001", (kernel_ulong_t)atk4001_id_params},
   {"", 0},
};

does not compile: "drivers/platform/x86/asus-wireless.c:44:2: error:
aggregate value used where an integer was expected", so I guess you
meant the address of that struct (_id_params), right? I don't
see any way how we could have the actual data in .driver_data.

Even after moving the parameters in the driver_data field of
device_ids[], I'm not able retrieve them with acpi_match_device(),
because the "struct device" from the "struct acpi_device" that comes
as an input parameter in asus_wireless_add() does not have a acpi
companion device associated with it, so acpi_match_device() returns
NULL. I still need to manually loop through device_ids[] in order to
retrieve the .driver_data associated with the HID, and I see the same
pattern in the i2c-scmi driver.

I'm sending the next revision for feedback, please advise if I'm
missing any detail, or if you want this implemented differently.

--
João Paulo Rechi Vita
http://about.me/jprvita

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-04 Thread Andy Shevchenko 
On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  wrote:
> On 27 January 2017 at 10:26, Andy Shevchenko  
> wrote:
>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>  wrote:

>>> +static const struct acpi_device_id device_ids[] = {
>>> +   {"ATK4001", 0},
>>> +   {"ATK4002", 0},
>>
>> ...and use it as a parameter here.
>>
>
> I'm not exactly sure how to do that, as driver_data is a
> kernel_ulong_t. Can you please elaborate a bit more?

{"ATK4001", (kernel_ulong_t)atk4001_id_params},

Similar for the other one.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-04 Thread Andy Shevchenko 
On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita  wrote:
> On 27 January 2017 at 10:26, Andy Shevchenko  
> wrote:
>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>  wrote:

>>> +static const struct acpi_device_id device_ids[] = {
>>> +   {"ATK4001", 0},
>>> +   {"ATK4002", 0},
>>
>> ...and use it as a parameter here.
>>
>
> I'm not exactly sure how to do that, as driver_data is a
> kernel_ulong_t. Can you please elaborate a bit more?

{"ATK4001", (kernel_ulong_t)atk4001_id_params},

Similar for the other one.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-01 Thread João Paulo Rechi Vita 
On 27 January 2017 at 10:26, Andy Shevchenko  wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>  wrote:
>> Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
>> 0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get
>
> excerpts
>

Thanks for catching that.

>> the LED status). Luckly it seems this behavior is tied to different HIDs, 
>> after
>
> Luckily
>

Ditto.

>> looking at 44 DSDTs from different Asus models.
>
>
>
>>
>> Another small difference (not shown here) is that a few machines call GWBL
>> instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
>> difference for asus-wireless, and is a reason to not try to call these 
>> methods
>> directly.
>>
>> Device (ASHS)   | Device (ASHS)
>> {   | {
>> Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
>> Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
>> {   | {
>> If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
>> {   | {
>> OWGD (Arg0) | OWGD (Arg0)
>> Return (One)| Return (One)
>> }   | }
>> If ((Arg0 == 0x02)) |
>> {   | If ((Arg0 == 0x02))
>> Local0 = OWGS ()| {
>> If (Local0) | Return (OWGS ())
>> {   | }
>> Return (0x05)   |
>> }   | If ((Arg0 == 0x03))
>> Else| {
>> {   | Return (0xFF)
>> Return (0x04)   | }
>> }   |
>> }   | If ((Arg0 == 0x80))
>> If ((Arg0 == 0x03)) | {
>> {   |Return (One)
>> Return (0xFF)   | }
>> }   | }
>> If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
>> {   | {
>> OWGD (Zero) | If ((MSOS () >= OSW8))
>> Return (One)| {
>> }   | Return (0x0F)
>> If ((Arg0 == 0x05)) | }
>> {   | Else
>> OWGD (One)  | {
>> Return (One)| Return (Zero)
>> }   | }
>> If ((Arg0 == 0x80)) | }
>> {   | }
>> Return (One)|
>> }   |
>> }   |
>> Method (_STA, 0, NotSerialized) |
>> {   |
>> If ((MSOS () >= OSW8))  |
>> {   |
>> Return (0x0F)   |
>> }   |
>> Else|
>> {   |
>> Return (Zero)   |
>> }   |
>> }   |
>> }   |
>>
>> Signed-off-by: João Paulo Rechi Vita 
>> ---
>>  drivers/platform/x86/asus-wireless.c | 46 
>> ++--
>>  1 file changed, 33 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/platform/x86/asus-wireless.c 
>> b/drivers/platform/x86/asus-wireless.c
>> index c9b5ac152cf1..5a238fad35fb 100644
>> --- a/drivers/platform/x86/asus-wireless.c
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -18,18 +18,35 @@
>>  #include 
>>
>>  #define ASUS_WIRELESS_LED_STATUS 0x2
>> -#define ASUS_WIRELESS_LED_OFF 0x4
>> -#define ASUS_WIRELESS_LED_ON 0x5
>> +
>> +struct hswc_params {
>> +   u8 on;
>> +   u8 off;
>> +};
>>
>>  struct asus_wireless_data {
>> struct input_dev *idev;
>> struct acpi_device *adev;
>> +   const struct hswc_params *hswc_params;
>> struct workqueue_struct *wq;
>> struct work_struct led_work;
>> struct led_classdev led;
>> int led_state;
>>  };
>>
>> +/* LED ON/OFF values for different HIDs. Please update when adding new 
>> HIDs. */
>> +static const struct hswc_params id_params[] = {
>> +   { 0x0, 0x1 },
>> +   { 0x5, 0x4 },
>> +};
>
> Add status here as well.

Not really necessary, but who knows if that may also change in the
future. I'm fixing it for the next revision.

> Split to one struct per set.
>
>> +
>> +static const struct acpi_device_id device_ids[] = {
>> +

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-02-01 Thread João Paulo Rechi Vita 
On 27 January 2017 at 10:26, Andy Shevchenko  wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>  wrote:
>> Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
>> 0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get
>
> excerpts
>

Thanks for catching that.

>> the LED status). Luckly it seems this behavior is tied to different HIDs, 
>> after
>
> Luckily
>

Ditto.

>> looking at 44 DSDTs from different Asus models.
>
>
>
>>
>> Another small difference (not shown here) is that a few machines call GWBL
>> instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
>> difference for asus-wireless, and is a reason to not try to call these 
>> methods
>> directly.
>>
>> Device (ASHS)   | Device (ASHS)
>> {   | {
>> Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
>> Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
>> {   | {
>> If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
>> {   | {
>> OWGD (Arg0) | OWGD (Arg0)
>> Return (One)| Return (One)
>> }   | }
>> If ((Arg0 == 0x02)) |
>> {   | If ((Arg0 == 0x02))
>> Local0 = OWGS ()| {
>> If (Local0) | Return (OWGS ())
>> {   | }
>> Return (0x05)   |
>> }   | If ((Arg0 == 0x03))
>> Else| {
>> {   | Return (0xFF)
>> Return (0x04)   | }
>> }   |
>> }   | If ((Arg0 == 0x80))
>> If ((Arg0 == 0x03)) | {
>> {   |Return (One)
>> Return (0xFF)   | }
>> }   | }
>> If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
>> {   | {
>> OWGD (Zero) | If ((MSOS () >= OSW8))
>> Return (One)| {
>> }   | Return (0x0F)
>> If ((Arg0 == 0x05)) | }
>> {   | Else
>> OWGD (One)  | {
>> Return (One)| Return (Zero)
>> }   | }
>> If ((Arg0 == 0x80)) | }
>> {   | }
>> Return (One)|
>> }   |
>> }   |
>> Method (_STA, 0, NotSerialized) |
>> {   |
>> If ((MSOS () >= OSW8))  |
>> {   |
>> Return (0x0F)   |
>> }   |
>> Else|
>> {   |
>> Return (Zero)   |
>> }   |
>> }   |
>> }   |
>>
>> Signed-off-by: João Paulo Rechi Vita 
>> ---
>>  drivers/platform/x86/asus-wireless.c | 46 
>> ++--
>>  1 file changed, 33 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/platform/x86/asus-wireless.c 
>> b/drivers/platform/x86/asus-wireless.c
>> index c9b5ac152cf1..5a238fad35fb 100644
>> --- a/drivers/platform/x86/asus-wireless.c
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -18,18 +18,35 @@
>>  #include 
>>
>>  #define ASUS_WIRELESS_LED_STATUS 0x2
>> -#define ASUS_WIRELESS_LED_OFF 0x4
>> -#define ASUS_WIRELESS_LED_ON 0x5
>> +
>> +struct hswc_params {
>> +   u8 on;
>> +   u8 off;
>> +};
>>
>>  struct asus_wireless_data {
>> struct input_dev *idev;
>> struct acpi_device *adev;
>> +   const struct hswc_params *hswc_params;
>> struct workqueue_struct *wq;
>> struct work_struct led_work;
>> struct led_classdev led;
>> int led_state;
>>  };
>>
>> +/* LED ON/OFF values for different HIDs. Please update when adding new 
>> HIDs. */
>> +static const struct hswc_params id_params[] = {
>> +   { 0x0, 0x1 },
>> +   { 0x5, 0x4 },
>> +};
>
> Add status here as well.

Not really necessary, but who knows if that may also change in the
future. I'm fixing it for the next revision.

> Split to one struct per set.
>
>> +
>> +static const struct acpi_device_id device_ids[] = {
>> +   {"ATK4001", 0},
>> +   {"ATK4002", 0},
>
> ...and use it as a

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-01-27 Thread Andy Shevchenko 
On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
 wrote:
> Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
> 0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get

excerpts

> the LED status). Luckly it seems this behavior is tied to different HIDs, 
> after

Luckily

> looking at 44 DSDTs from different Asus models.



>
> Another small difference (not shown here) is that a few machines call GWBL
> instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
> difference for asus-wireless, and is a reason to not try to call these methods
> directly.
>
> Device (ASHS)   | Device (ASHS)
> {   | {
> Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
> Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
> {   | {
> If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
> {   | {
> OWGD (Arg0) | OWGD (Arg0)
> Return (One)| Return (One)
> }   | }
> If ((Arg0 == 0x02)) |
> {   | If ((Arg0 == 0x02))
> Local0 = OWGS ()| {
> If (Local0) | Return (OWGS ())
> {   | }
> Return (0x05)   |
> }   | If ((Arg0 == 0x03))
> Else| {
> {   | Return (0xFF)
> Return (0x04)   | }
> }   |
> }   | If ((Arg0 == 0x80))
> If ((Arg0 == 0x03)) | {
> {   |Return (One)
> Return (0xFF)   | }
> }   | }
> If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
> {   | {
> OWGD (Zero) | If ((MSOS () >= OSW8))
> Return (One)| {
> }   | Return (0x0F)
> If ((Arg0 == 0x05)) | }
> {   | Else
> OWGD (One)  | {
> Return (One)| Return (Zero)
> }   | }
> If ((Arg0 == 0x80)) | }
> {   | }
> Return (One)|
> }   |
> }   |
> Method (_STA, 0, NotSerialized) |
> {   |
> If ((MSOS () >= OSW8))  |
> {   |
> Return (0x0F)   |
> }   |
> Else|
> {   |
> Return (Zero)   |
> }   |
> }   |
> }   |
>
> Signed-off-by: João Paulo Rechi Vita 
> ---
>  drivers/platform/x86/asus-wireless.c | 46 
> ++--
>  1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wireless.c 
> b/drivers/platform/x86/asus-wireless.c
> index c9b5ac152cf1..5a238fad35fb 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -18,18 +18,35 @@
>  #include 
>
>  #define ASUS_WIRELESS_LED_STATUS 0x2
> -#define ASUS_WIRELESS_LED_OFF 0x4
> -#define ASUS_WIRELESS_LED_ON 0x5
> +
> +struct hswc_params {
> +   u8 on;
> +   u8 off;
> +};
>
>  struct asus_wireless_data {
> struct input_dev *idev;
> struct acpi_device *adev;
> +   const struct hswc_params *hswc_params;
> struct workqueue_struct *wq;
> struct work_struct led_work;
> struct led_classdev led;
> int led_state;
>  };
>
> +/* LED ON/OFF values for different HIDs. Please update when adding new HIDs. 
> */
> +static const struct hswc_params id_params[] = {
> +   { 0x0, 0x1 },
> +   { 0x5, 0x4 },
> +};

Add status here as well.
Split to one struct per set.

> +
> +static const struct acpi_device_id device_ids[] = {
> +   {"ATK4001", 0},
> +   {"ATK4002", 0},

...and use it as a parameter here.

> +   {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
>  static u64 asus_wireless_method(acpi_handle handle, const char *method,
> int param)
>  {
> @@ -62,7 +79,7 @@ static enum led_brightness led_state_get(struct 
> led_classdev *led)
>

Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-01-27 Thread Andy Shevchenko 
On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
 wrote:
> Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
> 0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get

excerpts

> the LED status). Luckly it seems this behavior is tied to different HIDs, 
> after

Luckily

> looking at 44 DSDTs from different Asus models.



>
> Another small difference (not shown here) is that a few machines call GWBL
> instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
> difference for asus-wireless, and is a reason to not try to call these methods
> directly.
>
> Device (ASHS)   | Device (ASHS)
> {   | {
> Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
> Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
> {   | {
> If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
> {   | {
> OWGD (Arg0) | OWGD (Arg0)
> Return (One)| Return (One)
> }   | }
> If ((Arg0 == 0x02)) |
> {   | If ((Arg0 == 0x02))
> Local0 = OWGS ()| {
> If (Local0) | Return (OWGS ())
> {   | }
> Return (0x05)   |
> }   | If ((Arg0 == 0x03))
> Else| {
> {   | Return (0xFF)
> Return (0x04)   | }
> }   |
> }   | If ((Arg0 == 0x80))
> If ((Arg0 == 0x03)) | {
> {   |Return (One)
> Return (0xFF)   | }
> }   | }
> If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
> {   | {
> OWGD (Zero) | If ((MSOS () >= OSW8))
> Return (One)| {
> }   | Return (0x0F)
> If ((Arg0 == 0x05)) | }
> {   | Else
> OWGD (One)  | {
> Return (One)| Return (Zero)
> }   | }
> If ((Arg0 == 0x80)) | }
> {   | }
> Return (One)|
> }   |
> }   |
> Method (_STA, 0, NotSerialized) |
> {   |
> If ((MSOS () >= OSW8))  |
> {   |
> Return (0x0F)   |
> }   |
> Else|
> {   |
> Return (Zero)   |
> }   |
> }   |
> }   |
>
> Signed-off-by: João Paulo Rechi Vita 
> ---
>  drivers/platform/x86/asus-wireless.c | 46 
> ++--
>  1 file changed, 33 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wireless.c 
> b/drivers/platform/x86/asus-wireless.c
> index c9b5ac152cf1..5a238fad35fb 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -18,18 +18,35 @@
>  #include 
>
>  #define ASUS_WIRELESS_LED_STATUS 0x2
> -#define ASUS_WIRELESS_LED_OFF 0x4
> -#define ASUS_WIRELESS_LED_ON 0x5
> +
> +struct hswc_params {
> +   u8 on;
> +   u8 off;
> +};
>
>  struct asus_wireless_data {
> struct input_dev *idev;
> struct acpi_device *adev;
> +   const struct hswc_params *hswc_params;
> struct workqueue_struct *wq;
> struct work_struct led_work;
> struct led_classdev led;
> int led_state;
>  };
>
> +/* LED ON/OFF values for different HIDs. Please update when adding new HIDs. 
> */
> +static const struct hswc_params id_params[] = {
> +   { 0x0, 0x1 },
> +   { 0x5, 0x4 },
> +};

Add status here as well.
Split to one struct per set.

> +
> +static const struct acpi_device_id device_ids[] = {
> +   {"ATK4001", 0},
> +   {"ATK4002", 0},

...and use it as a parameter here.

> +   {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
>  static u64 asus_wireless_method(acpi_handle handle, const char *method,
> int param)
>  {
> @@ -62,7 +79,7 @@ static enum led_brightness led_state_get(struct 
> led_classdev *led)
> data = container_of(led, struct

[PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-01-26 Thread João Paulo Rechi Vita 
Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get
the LED status). Luckly it seems this behavior is tied to different HIDs, after
looking at 44 DSDTs from different Asus models.

Another small difference (not shown here) is that a few machines call GWBL
instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
difference for asus-wireless, and is a reason to not try to call these methods
directly.

Device (ASHS)   | Device (ASHS)
{   | {
Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
{   | {
If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
{   | {
OWGD (Arg0) | OWGD (Arg0)
Return (One)| Return (One)
}   | }
If ((Arg0 == 0x02)) |
{   | If ((Arg0 == 0x02))
Local0 = OWGS ()| {
If (Local0) | Return (OWGS ())
{   | }
Return (0x05)   |
}   | If ((Arg0 == 0x03))
Else| {
{   | Return (0xFF)
Return (0x04)   | }
}   |
}   | If ((Arg0 == 0x80))
If ((Arg0 == 0x03)) | {
{   |Return (One)
Return (0xFF)   | }
}   | }
If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
{   | {
OWGD (Zero) | If ((MSOS () >= OSW8))
Return (One)| {
}   | Return (0x0F)
If ((Arg0 == 0x05)) | }
{   | Else
OWGD (One)  | {
Return (One)| Return (Zero)
}   | }
If ((Arg0 == 0x80)) | }
{   | }
Return (One)|
}   |
}   |
Method (_STA, 0, NotSerialized) |
{   |
If ((MSOS () >= OSW8))  |
{   |
Return (0x0F)   |
}   |
Else|
{   |
Return (Zero)   |
}   |
}   |
}   |

Signed-off-by: João Paulo Rechi Vita 
---
 drivers/platform/x86/asus-wireless.c | 46 ++--
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/asus-wireless.c 
b/drivers/platform/x86/asus-wireless.c
index c9b5ac152cf1..5a238fad35fb 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -18,18 +18,35 @@
 #include 
 
 #define ASUS_WIRELESS_LED_STATUS 0x2
-#define ASUS_WIRELESS_LED_OFF 0x4
-#define ASUS_WIRELESS_LED_ON 0x5
+
+struct hswc_params {
+   u8 on;
+   u8 off;
+};
 
 struct asus_wireless_data {
struct input_dev *idev;
struct acpi_device *adev;
+   const struct hswc_params *hswc_params;
struct workqueue_struct *wq;
struct work_struct led_work;
struct led_classdev led;
int led_state;
 };
 
+/* LED ON/OFF values for different HIDs. Please update when adding new HIDs. */
+static const struct hswc_params id_params[] = {
+   { 0x0, 0x1 },
+   { 0x5, 0x4 },
+};
+
+static const struct acpi_device_id device_ids[] = {
+   {"ATK4001", 0},
+   {"ATK4002", 0},
+   {"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, device_ids);
+
 static u64 asus_wireless_method(acpi_handle handle, const char *method,
int param)
 {
@@ -62,7 +79,7 @@ static enum led_brightness led_state_get(struct led_classdev 
*led)
data = container_of(led, struct asus_wireless_data, led);
s = asus_wireless_method(acpi_device_handle(data->adev), "HSWC",
 ASUS_WIRELESS_LED_STATUS);
-   if (s == ASUS_WIRELESS_LED_ON)
+   if (s == data->hswc_params->on)
return LED_FULL;
return LED_OFF;
 }
@@ -82,8 +99,8 @@ static void led_state_set(struct led_classdev *led,
struct

[PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID

2017-01-26 Thread João Paulo Rechi Vita 
Some Asus machines use 0x4/0x5 as their LED on/off values, while others use
0x0/0x1, as shown in the DSDT exerpts below (note "Arg0 == 0x02", used to get
the LED status). Luckly it seems this behavior is tied to different HIDs, after
looking at 44 DSDTs from different Asus models.

Another small difference (not shown here) is that a few machines call GWBL
instead of OWGS, and SWBL instead of OWGD. That does not seem to make a
difference for asus-wireless, and is a reason to not try to call these methods
directly.

Device (ASHS)   | Device (ASHS)
{   | {
Name (_HID, "ATK4002")  | Name (_HID, "ATK4001")
Method (HSWC, 1, Serialized)| Method (HSWC, 1, Serialized)
{   | {
If ((Arg0 < 0x02))  | If ((Arg0 < 0x02))
{   | {
OWGD (Arg0) | OWGD (Arg0)
Return (One)| Return (One)
}   | }
If ((Arg0 == 0x02)) |
{   | If ((Arg0 == 0x02))
Local0 = OWGS ()| {
If (Local0) | Return (OWGS ())
{   | }
Return (0x05)   |
}   | If ((Arg0 == 0x03))
Else| {
{   | Return (0xFF)
Return (0x04)   | }
}   |
}   | If ((Arg0 == 0x80))
If ((Arg0 == 0x03)) | {
{   |Return (One)
Return (0xFF)   | }
}   | }
If ((Arg0 == 0x04)) | Method (_STA, 0, NotSerialized)
{   | {
OWGD (Zero) | If ((MSOS () >= OSW8))
Return (One)| {
}   | Return (0x0F)
If ((Arg0 == 0x05)) | }
{   | Else
OWGD (One)  | {
Return (One)| Return (Zero)
}   | }
If ((Arg0 == 0x80)) | }
{   | }
Return (One)|
}   |
}   |
Method (_STA, 0, NotSerialized) |
{   |
If ((MSOS () >= OSW8))  |
{   |
Return (0x0F)   |
}   |
Else|
{   |
Return (Zero)   |
}   |
}   |
}   |

Signed-off-by: João Paulo Rechi Vita 
---
 drivers/platform/x86/asus-wireless.c | 46 ++--
 1 file changed, 33 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/asus-wireless.c 
b/drivers/platform/x86/asus-wireless.c
index c9b5ac152cf1..5a238fad35fb 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -18,18 +18,35 @@
 #include 
 
 #define ASUS_WIRELESS_LED_STATUS 0x2
-#define ASUS_WIRELESS_LED_OFF 0x4
-#define ASUS_WIRELESS_LED_ON 0x5
+
+struct hswc_params {
+   u8 on;
+   u8 off;
+};
 
 struct asus_wireless_data {
struct input_dev *idev;
struct acpi_device *adev;
+   const struct hswc_params *hswc_params;
struct workqueue_struct *wq;
struct work_struct led_work;
struct led_classdev led;
int led_state;
 };
 
+/* LED ON/OFF values for different HIDs. Please update when adding new HIDs. */
+static const struct hswc_params id_params[] = {
+   { 0x0, 0x1 },
+   { 0x5, 0x4 },
+};
+
+static const struct acpi_device_id device_ids[] = {
+   {"ATK4001", 0},
+   {"ATK4002", 0},
+   {"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, device_ids);
+
 static u64 asus_wireless_method(acpi_handle handle, const char *method,
int param)
 {
@@ -62,7 +79,7 @@ static enum led_brightness led_state_get(struct led_classdev 
*led)
data = container_of(led, struct asus_wireless_data, led);
s = asus_wireless_method(acpi_device_handle(data->adev), "HSWC",
 ASUS_WIRELESS_LED_STATUS);
-   if (s == ASUS_WIRELESS_LED_ON)
+   if (s == data->hswc_params->on)
return LED_FULL;
return LED_OFF;
 }
@@ -82,8 +99,8 @@ static void led_state_set(struct led_classdev *led,
struct asus_wireless_data *data;