On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
> <jprvita@xxxxxxxxx> 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 <jprvita@xxxxxxxxxxxx>
>> ---
>> 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 <linux/leds.h>
>>
>> #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 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?
Thanks for the review,
--
João Paulo Rechi Vita
http://about.me/jprvita
