All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/1] asus-wireless: Use the correct HSWC parameter for each HID
@ 2017-01-26 15:00 João Paulo Rechi Vita
  2017-01-26 15:00 ` [PATCH 1/1] " João Paulo Rechi Vita
  0 siblings, 1 reply; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-01-26 15:00 UTC (permalink / raw)
  To: Darren Hart
  Cc: platform-driver-x86, acpi4asus-user, linux-kernel, linux,
	João Paulo Rechi Vita

As stated in the commit message, different machines can use different values to
represent LED ON/OFF, and this commit makes sure we use the correct value pair
for each _HID.

I don't know if there is a better way to store these pairs than in a structure
matching the device_ids[] indexing that would avoid looping through
device_ids[] in asus_wireless_add(). I'm happy to try suggestions to improve
that.

João Paulo Rechi Vita (1):
  asus-wireless: Use the correct HSWC parameter for each HID

 drivers/platform/x86/asus-wireless.c | 46 ++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 13 deletions(-)

-- 
2.11.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-01-26 15:00 [PATCH 0/1] asus-wireless: Use the correct HSWC parameter for each HID João Paulo Rechi Vita
@ 2017-01-26 15:00 ` João Paulo Rechi Vita
  2017-01-27 15:26   ` Andy Shevchenko
  0 siblings, 1 reply; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-01-26 15:00 UTC (permalink / raw)
  To: Darren Hart
  Cc: platform-driver-x86, acpi4asus-user, linux-kernel, linux,
	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 <jprvita@endlessm.com>
---
 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 },
+};
+
+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;
 
 	data = container_of(led, struct asus_wireless_data, led);
-	data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
-					     ASUS_WIRELESS_LED_ON;
+	data->led_state = value == LED_OFF ? data->hswc_params->off :
+					     data->hswc_params->on;
 	queue_work(data->wq, &data->led_work);
 }
 
@@ -104,13 +121,22 @@ static void asus_wireless_notify(struct acpi_device *adev, u32 event)
 static int asus_wireless_add(struct acpi_device *adev)
 {
 	struct asus_wireless_data *data;
-	int err;
+	const char *hid;
+	int err, i;
 
 	data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 	adev->driver_data = data;
 
+	hid = acpi_device_hid(adev);
+	for (i = 0; strcmp(device_ids[i].id, ""); i++) {
+		if (!strcmp(device_ids[i].id, hid)) {
+			data->hswc_params = &id_params[i];
+			break;
+		}
+	}
+
 	data->idev = devm_input_allocate_device(&adev->dev);
 	if (!data->idev)
 		return -ENOMEM;
@@ -137,6 +163,7 @@ static int asus_wireless_add(struct acpi_device *adev)
 	err = devm_led_classdev_register(&adev->dev, &data->led);
 	if (err)
 		destroy_workqueue(data->wq);
+
 	return err;
 }
 
@@ -149,13 +176,6 @@ static int asus_wireless_remove(struct acpi_device *adev)
 	return 0;
 }
 
-static const struct acpi_device_id device_ids[] = {
-	{"ATK4001", 0},
-	{"ATK4002", 0},
-	{"", 0},
-};
-MODULE_DEVICE_TABLE(acpi, device_ids);
-
 static struct acpi_driver asus_wireless_driver = {
 	.name = "Asus Wireless Radio Control Driver",
 	.class = "hotkey",
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-01-26 15:00 ` [PATCH 1/1] " João Paulo Rechi Vita
@ 2017-01-27 15:26   ` Andy Shevchenko
  2017-02-01 12:20       ` João Paulo Rechi Vita
  0 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-01-27 15:26 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
<jprvita@gmail.com> 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 <jprvita@endlessm.com>
> ---
>  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.
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 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;
>
>         data = container_of(led, struct asus_wireless_data, led);
> -       data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
> -                                            ASUS_WIRELESS_LED_ON;
> +       data->led_state = value == LED_OFF ? data->hswc_params->off :
> +                                            data->hswc_params->on;
>         queue_work(data->wq, &data->led_work);
>  }
>
> @@ -104,13 +121,22 @@ static void asus_wireless_notify(struct acpi_device *adev, u32 event)
>  static int asus_wireless_add(struct acpi_device *adev)
>  {
>         struct asus_wireless_data *data;
> -       int err;
> +       const char *hid;
> +       int err, i;
>
>         data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
>         if (!data)
>                 return -ENOMEM;
>         adev->driver_data = data;
>

> +       hid = acpi_device_hid(adev);
> +       for (i = 0; strcmp(device_ids[i].id, ""); i++) {
> +               if (!strcmp(device_ids[i].id, hid)) {
> +                       data->hswc_params = &id_params[i];
> +                       break;
> +               }
> +       }
> +

No, see above.

>         data->idev = devm_input_allocate_device(&adev->dev);
>         if (!data->idev)
>                 return -ENOMEM;
> @@ -137,6 +163,7 @@ static int asus_wireless_add(struct acpi_device *adev)
>         err = devm_led_classdev_register(&adev->dev, &data->led);
>         if (err)
>                 destroy_workqueue(data->wq);
> +
>         return err;
>  }
>
> @@ -149,13 +176,6 @@ static int asus_wireless_remove(struct acpi_device *adev)
>         return 0;
>  }
>
> -static const struct acpi_device_id device_ids[] = {
> -       {"ATK4001", 0},
> -       {"ATK4002", 0},
> -       {"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, device_ids);
> -
>  static struct acpi_driver asus_wireless_driver = {
>         .name = "Asus Wireless Radio Control Driver",
>         .class = "hotkey",
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-01-27 15:26   ` Andy Shevchenko
@ 2017-02-01 12:20       ` João Paulo Rechi Vita
  0 siblings, 0 replies; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-01 12:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
> <jprvita@gmail.com> 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@endlessm.com>
>> ---
>>  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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
@ 2017-02-01 12:20       ` João Paulo Rechi Vita
  0 siblings, 0 replies; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-01 12:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
> <jprvita@gmail.com> 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@endlessm.com>
>> ---
>>  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,

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-02-01 12:20       ` João Paulo Rechi Vita
  (?)
@ 2017-02-04 15:02       ` Andy Shevchenko
  2017-02-07 21:44           ` João Paulo Rechi Vita
  -1 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-02-04 15:02 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>> <jprvita@gmail.com> 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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-02-04 15:02       ` Andy Shevchenko
@ 2017-02-07 21:44           ` João Paulo Rechi Vita
  0 siblings, 0 replies; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-07 21:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
>> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>> <jprvita@gmail.com> 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 (&atk4001_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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
@ 2017-02-07 21:44           ` João Paulo Rechi Vita
  0 siblings, 0 replies; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-07 21:44 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
>> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>> <jprvita@gmail.com> 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 (&atk4001_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.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters
  2017-02-07 21:44           ` João Paulo Rechi Vita
  (?)
@ 2017-02-07 21:45           ` João Paulo Rechi Vita
  2017-02-13 22:09             ` Andy Shevchenko
  -1 siblings, 1 reply; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-07 21:45 UTC (permalink / raw)
  To: Darren Hart, Andy Shevchenko
  Cc: platform-driver-x86, acpi4asus-user, linux-kernel, linux,
	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 excerpts below. Luckily it seems this
behavior is tied to different HIDs, after looking at 44 DSDTs from
different Asus models.

Another small difference is that a few of them call GWBL instead of
OWGS, and SWBL instead of OWGD. That does not seem to make a difference
for asus-wireless, and is additional reasoning 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@endlessm.com>
---
 drivers/platform/x86/asus-wireless.c | 57 ++++++++++++++++++++++++++----------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
index a7ee18383146..f3796164329e 100644
--- a/drivers/platform/x86/asus-wireless.c
+++ b/drivers/platform/x86/asus-wireless.c
@@ -17,19 +17,41 @@
 #include <linux/pci_ids.h>
 #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;
+	u8 status;
+};
 
 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;
 };
 
+static const struct hswc_params atk4001_id_params = {
+	.on = 0x0,
+	.off = 0x1,
+	.status = 0x2,
+};
+
+static const struct hswc_params atk4002_id_params = {
+	.on = 0x5,
+	.off = 0x4,
+	.status = 0x2,
+};
+
+static const struct acpi_device_id device_ids[] = {
+	{"ATK4001", (kernel_ulong_t)&atk4001_id_params},
+	{"ATK4002", (kernel_ulong_t)&atk4002_id_params},
+	{"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, device_ids);
+
 static u64 asus_wireless_method(acpi_handle handle, const char *method,
 				int param)
 {
@@ -61,8 +83,8 @@ 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)
+				 data->hswc_params->status);
+	if (s == data->hswc_params->on)
 		return LED_FULL;
 	return LED_OFF;
 }
@@ -81,8 +103,8 @@ static void led_state_set(struct led_classdev *led, enum led_brightness value)
 	struct asus_wireless_data *data;
 
 	data = container_of(led, struct asus_wireless_data, led);
-	data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
-					     ASUS_WIRELESS_LED_ON;
+	data->led_state = value == LED_OFF ? data->hswc_params->off :
+					     data->hswc_params->on;
 	queue_work(data->wq, &data->led_work);
 }
 
@@ -103,12 +125,14 @@ static void asus_wireless_notify(struct acpi_device *adev, u32 event)
 static int asus_wireless_add(struct acpi_device *adev)
 {
 	struct asus_wireless_data *data;
+	const struct acpi_device_id *id;
 	int err;
 
 	data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
 	adev->driver_data = data;
+	data->adev = adev;
 
 	data->idev = devm_input_allocate_device(&adev->dev);
 	if (!data->idev)
@@ -123,7 +147,16 @@ static int asus_wireless_add(struct acpi_device *adev)
 	if (err)
 		return err;
 
-	data->adev = adev;
+	for (id = device_ids; id->id[0]; id++) {
+		if (!strcmp((char *) id->id, acpi_device_hid(adev))) {
+			data->hswc_params =
+				(const struct hswc_params *)id->driver_data;
+			break;
+		}
+	}
+	if (!data->hswc_params)
+		return 0;
+
 	data->wq = create_singlethread_workqueue("asus_wireless_workqueue");
 	if (!data->wq)
 		return -ENOMEM;
@@ -136,6 +169,7 @@ static int asus_wireless_add(struct acpi_device *adev)
 	err = devm_led_classdev_register(&adev->dev, &data->led);
 	if (err)
 		destroy_workqueue(data->wq);
+
 	return err;
 }
 
@@ -148,13 +182,6 @@ static int asus_wireless_remove(struct acpi_device *adev)
 	return 0;
 }
 
-static const struct acpi_device_id device_ids[] = {
-	{"ATK4001", 0},
-	{"ATK4002", 0},
-	{"", 0},
-};
-MODULE_DEVICE_TABLE(acpi, device_ids);
-
 static struct acpi_driver asus_wireless_driver = {
 	.name = "Asus Wireless Radio Control Driver",
 	.class = "hotkey",
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-02-07 21:44           ` João Paulo Rechi Vita
  (?)
  (?)
@ 2017-02-07 23:37           ` Andy Shevchenko
  2017-02-08 17:04             ` João Paulo Rechi Vita
  -1 siblings, 1 reply; 12+ messages in thread
From: Andy Shevchenko @ 2017-02-07 23:37 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
>>> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>>> <jprvita@gmail.com> 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 (&atk4001_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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID
  2017-02-07 23:37           ` [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID Andy Shevchenko
@ 2017-02-08 17:04             ` João Paulo Rechi Vita
  0 siblings, 0 replies; 12+ messages in thread
From: João Paulo Rechi Vita @ 2017-02-08 17:04 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On 7 February 2017 at 18:37, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> On Tue, Feb 7, 2017 at 11:44 PM, João Paulo Rechi Vita
> <jprvita@gmail.com> wrote:
>> On 4 February 2017 at 10:02, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>> On Wed, Feb 1, 2017 at 2:20 PM, João Paulo Rechi Vita <jprvita@gmail.com> wrote:
>>>> On 27 January 2017 at 10:26, Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
>>>>> On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
>>>>> <jprvita@gmail.com> 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 (&atk4001_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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters
  2017-02-07 21:45           ` [PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters João Paulo Rechi Vita
@ 2017-02-13 22:09             ` Andy Shevchenko
  0 siblings, 0 replies; 12+ messages in thread
From: Andy Shevchenko @ 2017-02-13 22:09 UTC (permalink / raw)
  To: João Paulo Rechi Vita
  Cc: Darren Hart, Platform Driver, acpi4asus-user, linux-kernel,
	linux, João Paulo Rechi Vita

On Tue, Feb 7, 2017 at 11:45 PM, João Paulo Rechi Vita
<jprvita@gmail.com> wrote:
> Some Asus machines use 0x4/0x5 as their LED on/off values, while others
> use 0x0/0x1, as shown in the DSDT excerpts below. Luckily it seems this
> behavior is tied to different HIDs, after looking at 44 DSDTs from
> different Asus models.
>
> Another small difference is that a few of them call GWBL instead of
> OWGS, and SWBL instead of OWGD. That does not seem to make a difference
> for asus-wireless, and is additional reasoning 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)           |
>         }                           |
>     }                               |
> }                                   |
>

Thanks, applied to testing.

> Signed-off-by: João Paulo Rechi Vita <jprvita@endlessm.com>
> ---
>  drivers/platform/x86/asus-wireless.c | 57 ++++++++++++++++++++++++++----------
>  1 file changed, 42 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
> index a7ee18383146..f3796164329e 100644
> --- a/drivers/platform/x86/asus-wireless.c
> +++ b/drivers/platform/x86/asus-wireless.c
> @@ -17,19 +17,41 @@
>  #include <linux/pci_ids.h>
>  #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;
> +       u8 status;
> +};
>
>  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;
>  };
>
> +static const struct hswc_params atk4001_id_params = {
> +       .on = 0x0,
> +       .off = 0x1,
> +       .status = 0x2,
> +};
> +
> +static const struct hswc_params atk4002_id_params = {
> +       .on = 0x5,
> +       .off = 0x4,
> +       .status = 0x2,
> +};
> +
> +static const struct acpi_device_id device_ids[] = {
> +       {"ATK4001", (kernel_ulong_t)&atk4001_id_params},
> +       {"ATK4002", (kernel_ulong_t)&atk4002_id_params},
> +       {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, device_ids);
> +
>  static u64 asus_wireless_method(acpi_handle handle, const char *method,
>                                 int param)
>  {
> @@ -61,8 +83,8 @@ 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)
> +                                data->hswc_params->status);
> +       if (s == data->hswc_params->on)
>                 return LED_FULL;
>         return LED_OFF;
>  }
> @@ -81,8 +103,8 @@ static void led_state_set(struct led_classdev *led, enum led_brightness value)
>         struct asus_wireless_data *data;
>
>         data = container_of(led, struct asus_wireless_data, led);
> -       data->led_state = value == LED_OFF ? ASUS_WIRELESS_LED_OFF :
> -                                            ASUS_WIRELESS_LED_ON;
> +       data->led_state = value == LED_OFF ? data->hswc_params->off :
> +                                            data->hswc_params->on;
>         queue_work(data->wq, &data->led_work);
>  }
>
> @@ -103,12 +125,14 @@ static void asus_wireless_notify(struct acpi_device *adev, u32 event)
>  static int asus_wireless_add(struct acpi_device *adev)
>  {
>         struct asus_wireless_data *data;
> +       const struct acpi_device_id *id;
>         int err;
>
>         data = devm_kzalloc(&adev->dev, sizeof(*data), GFP_KERNEL);
>         if (!data)
>                 return -ENOMEM;
>         adev->driver_data = data;
> +       data->adev = adev;
>
>         data->idev = devm_input_allocate_device(&adev->dev);
>         if (!data->idev)
> @@ -123,7 +147,16 @@ static int asus_wireless_add(struct acpi_device *adev)
>         if (err)
>                 return err;
>
> -       data->adev = adev;
> +       for (id = device_ids; id->id[0]; id++) {
> +               if (!strcmp((char *) id->id, acpi_device_hid(adev))) {
> +                       data->hswc_params =
> +                               (const struct hswc_params *)id->driver_data;
> +                       break;
> +               }
> +       }
> +       if (!data->hswc_params)
> +               return 0;
> +
>         data->wq = create_singlethread_workqueue("asus_wireless_workqueue");
>         if (!data->wq)
>                 return -ENOMEM;
> @@ -136,6 +169,7 @@ static int asus_wireless_add(struct acpi_device *adev)
>         err = devm_led_classdev_register(&adev->dev, &data->led);
>         if (err)
>                 destroy_workqueue(data->wq);
> +
>         return err;
>  }
>
> @@ -148,13 +182,6 @@ static int asus_wireless_remove(struct acpi_device *adev)
>         return 0;
>  }
>
> -static const struct acpi_device_id device_ids[] = {
> -       {"ATK4001", 0},
> -       {"ATK4002", 0},
> -       {"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, device_ids);
> -
>  static struct acpi_driver asus_wireless_driver = {
>         .name = "Asus Wireless Radio Control Driver",
>         .class = "hotkey",
> --
> 2.11.0
>



-- 
With Best Regards,
Andy Shevchenko

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-02-13 22:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-26 15:00 [PATCH 0/1] asus-wireless: Use the correct HSWC parameter for each HID João Paulo Rechi Vita
2017-01-26 15:00 ` [PATCH 1/1] " João Paulo Rechi Vita
2017-01-27 15:26   ` Andy Shevchenko
2017-02-01 12:20     ` João Paulo Rechi Vita
2017-02-01 12:20       ` João Paulo Rechi Vita
2017-02-04 15:02       ` Andy Shevchenko
2017-02-07 21:44         ` João Paulo Rechi Vita
2017-02-07 21:44           ` João Paulo Rechi Vita
2017-02-07 21:45           ` [PATCHv2] platform/x86: asus-wireless: Use per-HID HSWC parameters João Paulo Rechi Vita
2017-02-13 22:09             ` Andy Shevchenko
2017-02-07 23:37           ` [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for each HID Andy Shevchenko
2017-02-08 17:04             ` João Paulo Rechi Vita

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.