Re: [PATCH 4/4] asus-wireless: Toggle airplane mode LED

[JoÃo Paulo Rechi Vita]

On 27 December 2015 at 08:32, Andy Shevchenko <andy.shevchenko@xxxxxxxxx> wrote:
> On Sat, Dec 26, 2015 at 4:56 PM, JoÃo Paulo Rechi Vita
> <jprvita@xxxxxxxxx> wrote:
>> In the ASHS device we have the HSWC method, which basically calls either
>> OWGD or OWGS, depending on its parameter:
>>
>>         Device (ASHS)
>>         {
>>                 Name (_HID, "ATK4002")  // _HID: Hardware ID
>>                 Method (HSWC, 1, Serialized)
>>                 {
>>                         If ((Arg0 < 0x02))
>>                         {
>>                                 OWGD (Arg0)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x02))
>>                         {
>>                                 Local0 = OWGS ()
>>                                 If (Local0)
>>                                 {
>>                                         Return (0x05)
>>                                 }
>>                                 Else
>>                                 {
>>                                         Return (0x04)
>>                                 }
>>                         }
>>                         If ((Arg0 == 0x03))
>>                         {
>>                                 Return (0xFF)
>>                         }
>>                         If ((Arg0 == 0x04))
>>                         {
>>                                 OWGD (Zero)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x05))
>>                         {
>>                                 OWGD (One)
>>                                 Return (One)
>>                         }
>>                         If ((Arg0 == 0x80))
>>                         {
>>                                 Return (One)
>>                         }
>>                 }
>>                 Method (_STA, 0, NotSerialized)  // _STA: Status
>>                 {
>>                         If ((MSOS () >= OSW8))
>>                         {
>>                                 Return (0x0F)
>>                         }
>>                         Else
>>                         {
>>                                 Return (Zero)
>>                         }
>>                 }
>>         }
>>
>> On the Asus E202SA laptop, which does not have an airplane mode LED,
>> OWGD has an empty implementation and OWGS simply returns 0. On the Asus
>> X555UB these methods have the following implementation:
>>
>>         Method (OWGD, 1, Serialized)
>>         {
>>                 SGPL (0x0203000F, Arg0)
>>                 SGPL (0x0203000F, Arg0)
>>         }
>>
>>         Method (OWGS, 0, Serialized)
>>         {
>>                 Store (RGPL (0x0203000F), Local0)
>>                 Return (Local0)
>>         }
>>
>> Where OWGD(1) sets the airplane mode LED ON, OWGD(0) set it off, and
>> OWGS() returns its state.
>>
>> This commit makes use of a newly implemented RFKill LED trigger to
>> trigger the LED when the system enters or exits "Airplane Mode", there
>> is, when all radios are blocked.
>>
>> Signed-off-by: JoÃo Paulo Rechi Vita <jprvita@xxxxxxxxxxxx>
>> ---
>>  drivers/platform/x86/Kconfig         |  2 +
>>  drivers/platform/x86/asus-wireless.c | 81 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 83 insertions(+)
>>
>> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
>> index d3a088b..3d8dc0b 100644
>> --- a/drivers/platform/x86/Kconfig
>> +++ b/drivers/platform/x86/Kconfig
>> @@ -592,6 +592,8 @@ config ASUS_WIRELESS
>>         depends on ACPI
>>         depends on INPUT
>>         default m
>> +       select NEW_LEDS
>> +       select LEDS_CLASS
>>         ---help---
>>           The Asus Wireless Radio Control handles the airplane mode hotkey
>>           present on some Asus laptops.
>> diff --git a/drivers/platform/x86/asus-wireless.c b/drivers/platform/x86/asus-wireless.c
>> index 7928efd..489ef83 100644
>> --- a/drivers/platform/x86/asus-wireless.c
>> +++ b/drivers/platform/x86/asus-wireless.c
>> @@ -17,13 +17,76 @@
>>  #include <linux/acpi.h>
>>  #include <linux/input.h>
>>  #include <linux/pci_ids.h>
>> +#include <linux/leds.h>
>>
>>  #define ASUS_WIRELESS_MODULE_NAME "Asus Wireless Radio Control Driver"
>> +#define ASUS_WIRELESS_LED_STATUS 0x2
>> +#define ASUS_WIRELESS_LED_OFF 0x4
>> +#define ASUS_WIRELESS_LED_ON 0x5
>>
>>  struct asus_wireless_data {
>>         struct input_dev *inputdev;
>> +       struct acpi_device *acpidev;
>
> You can get this easily from struct device.
>

If I'm understanding it correctly that only works when the acpi device
is a companion to a physical device, which is not the case here
(ACPI_COMPANION(acpi_device->dev)==NULL). I'm keeping this for now,
please let me know if I'm missing something.

>> +       struct workqueue_struct *wq;
>> +       struct work_struct led_work;
>> +       struct led_classdev led;
>> +       int led_state;
>>  };
>>
>> +static u64 asus_wireless_method(acpi_handle handle, const char *method,
>> +                               int param)
>> +{
>> +       union acpi_object obj;
>> +       struct acpi_object_list p;
>> +       acpi_status s;
>> +       u64 ret;
>> +
>> +       pr_debug("Evaluating method %s, parameter 0x%X\n", method, param);
>
> acpi_handle_* in such cases.
>

Ok.

>> +       obj.type = ACPI_TYPE_INTEGER;
>> +       obj.integer.value = param;
>> +       p.count = 1;
>> +       p.pointer = &obj;
>> +
>> +       s = acpi_evaluate_integer(handle, (acpi_string) method, &p, &ret);
>> +       if (!ACPI_SUCCESS(s))
>
> ACPI_FAILURE()
>

Ok.

>> +               pr_err("Failed to evaluate method %s, parameter 0x%X (%d)\n",
>> +                      method, param, s);
>> +       pr_debug("%s returned 0x%X\n", method, (uint) ret);
>> +       return ret;
>> +}
>> +
>> +static enum led_brightness asus_wireless_led_get(struct led_classdev *led)
>> +{
>> +       struct asus_wireless_data *data;
>> +       int s;
>> +
>> +       data = container_of(led, struct asus_wireless_data, led);
>> +       s = asus_wireless_method(data->acpidev->handle, "HSWC",
>
> Usually we get a handle through specific macro ACPI_HANDLE from a
> struct device (see above).
>
>> +                                ASUS_WIRELESS_LED_STATUS);
>> +       if (s == ASUS_WIRELESS_LED_ON)
>> +               return LED_FULL;
>> +       return LED_OFF;
>> +}
>> +
>> +static void asus_wireless_led_update(struct work_struct *work)
>> +{
>> +       struct asus_wireless_data *data;
>> +
>> +       data = container_of(work, struct asus_wireless_data, led_work);
>> +       asus_wireless_method(data->acpidev->handle, "HSWC", data->led_state);
>
> Ditto.
>

These last two depend on ACPI_COMPANION as well. I'm changing this to
call acpi_device_handle() instead of accessing the pointer directly.

I will send an updated version shortly. Thanks again for your feedback.

--
JoÃo Paulo Rechi Vita
http://about.me/jprvita
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/