[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAHp75VfFGziE4iENsJqy8-HESK=HA8EYdGqQQ26EBqC3yv0KFg@mail.gmail.com>
Date: Fri, 27 Jan 2017 17:26:51 +0200
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: João Paulo Rechi Vita <jprvita@...il.com>
Cc: Darren Hart <dvhart@...radead.org>,
Platform Driver <platform-driver-x86@...r.kernel.org>,
acpi4asus-user@...ts.sourceforge.net,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
linux@...lessm.com,
João Paulo Rechi Vita <jprvita@...lessm.com>
Subject: Re: [PATCH 1/1] asus-wireless: Use the correct HSWC parameter for
each HID
On Thu, Jan 26, 2017 at 5:00 PM, João Paulo Rechi Vita
<jprvita@...il.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@...lessm.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
Powered by blists - more mailing lists