Ok, now it works perfectly :) 2012/7/27 Corentin Chary <corentincj@xxxxxxxxxx>: > On Fri, Jul 27, 2012 at 4:20 AM, AceLan Kao <acelan.kao@xxxxxxxxxxxxx> wrote: >> Dear Corentin, >> >> 2012/7/26 Corentin Chary <corentincj@xxxxxxxxxx>: >>> On Thu, Jul 26, 2012 at 10:12 AM, AceLan Kao <acelan.kao@xxxxxxxxxxxxx> wrote: >>>> Dear Corentin, >>>> >>>> 2012/7/26 Corentin Chary <corentincj@xxxxxxxxxx>: >>>>> On Thu, Jul 26, 2012 at 5:34 AM, AceLan Kao <acelan.kao@xxxxxxxxxxxxx> wrote: >>>>>> For those machines with wapf=4, BIOS won't update the wireless LED, >>>>>> since wapf=4 means user application will take in chage of the wifi and bt. >>>>>> So, we have to update wlan LED status explicitly. >>>>>> >>>>>> But I found there is another wireless LED bug in launchpad and which is >>>>>> not in the wapf=4 quirk. >>>>>> So, it might be better to set wireless LED status explicitly for all >>>>>> machines. >>>>>> >>>>>> BugLink: https://launchpad.net/bugs/901105 >>>>>> >>>>>> Signed-off-by: AceLan Kao <acelan.kao@xxxxxxxxxxxxx> >>>>>> --- >>>>>> drivers/platform/x86/asus-wmi.c | 24 ++++++++++++++++++++++++ >>>>>> 1 file changed, 24 insertions(+) >>>>>> >>>>>> diff --git a/drivers/platform/x86/asus-wmi.c b/drivers/platform/x86/asus-wmi.c >>>>>> index a13eb45..6ddfb0b 100644 >>>>>> --- a/drivers/platform/x86/asus-wmi.c >>>>>> +++ b/drivers/platform/x86/asus-wmi.c >>>>>> @@ -183,6 +183,7 @@ struct asus_wmi { >>>>>> struct device *hwmon_device; >>>>>> struct platform_device *platform_device; >>>>>> >>>>>> + struct led_classdev wlan_led; >>>>>> struct led_classdev tpd_led; >>>>>> int tpd_led_wk; >>>>>> struct led_classdev kbd_led; >>>>>> @@ -452,12 +453,20 @@ static enum led_brightness kbd_led_get(struct led_classdev *led_cdev) >>>>>> return value; >>>>>> } >>>>>> >>>>>> +static void wlan_led_set(struct led_classdev *led_cdev, >>>>>> + enum led_brightness value) >>>>>> +{ >>>>>> + asus_wmi_set_devstate(ASUS_WMI_DEVID_WIRELESS_LED, value, NULL); >>>>>> +} >>>>>> + >>>>>> static void asus_wmi_led_exit(struct asus_wmi *asus) >>>>>> { >>>>>> if (!IS_ERR_OR_NULL(asus->kbd_led.dev)) >>>>>> led_classdev_unregister(&asus->kbd_led); >>>>>> if (!IS_ERR_OR_NULL(asus->tpd_led.dev)) >>>>>> led_classdev_unregister(&asus->tpd_led); >>>>>> + if (!IS_ERR_OR_NULL(asus->wlan_led.dev)) >>>>>> + led_classdev_unregister(&asus->wlan_led); >>>>>> if (asus->led_workqueue) >>>>>> destroy_workqueue(asus->led_workqueue); >>>>>> } >>>>>> @@ -465,6 +474,7 @@ static void asus_wmi_led_exit(struct asus_wmi *asus) >>>>>> static int asus_wmi_led_init(struct asus_wmi *asus) >>>>>> { >>>>>> int rv = 0; >>>>>> + u32 result; >>>>>> >>>>>> asus->led_workqueue = create_singlethread_workqueue("led_workqueue"); >>>>>> if (!asus->led_workqueue) >>>>>> @@ -498,6 +508,17 @@ static int asus_wmi_led_init(struct asus_wmi *asus) >>>>>> goto error; >>>>>> } >>>>>> >>>>>> + asus_wmi_get_devstate(asus, ASUS_WMI_DEVID_WIRELESS_LED, &result); >>>>>> + if (result & ASUS_WMI_DSTS_PRESENCE_BIT) { >>>>>> + asus->wlan_led.name = "asus-wmi::wlan"; > > Like you said, asus::wlan would be better. > >>>>>> + asus->wlan_led.brightness_set = wlan_led_set; >>>>> >>>>> Why no brightness_get ? >>>> The return value from querying ASUS_WMI_DEVID_WIRELESS_LED is not correct. >>>> In my machine, I'll get 0x00050003 when querying its status no matter >>>> the LED is on or off. >>> >>> Could I take a look to the DSDT ? >> Sure, here comes the DSDT > > It's 0x00050002 no ? > 0x2 means "status unknown" (ASUS_WMI_DSTS_UNKNOWN_BIT). > So maybe you could check this bit, and set brightness_get only when > it's not set. > >>> >>>> That's why I didn't provide a brightness_get() function. >>>> >>>>> (note that if you decide to add wl_led_get() function that returns >>>>> -ENODEV when not found (like for touchpad), please use directly >>>>> instead of checking devstate above). >>>> I'll wrap the devstate in a function, but won't provide a brightness_get() >>>> Is that okay? >>>> >>>>> >>>>>> + asus->wlan_led.flags = LED_CORE_SUSPENDRESUME; >>>>>> + asus->wlan_led.max_brightness = 1; >>>>>> + asus->wlan_led.default_trigger = "asus-wlan"; >>>>>> + rv = led_classdev_register(&asus->platform_device->dev, >>>>>> + &asus->wlan_led); >>>>>> + >>>>>> + } >>>>>> error: >>>>>> if (rv) >>>>>> asus_wmi_led_exit(asus); >>>>>> @@ -811,6 +832,9 @@ static int asus_new_rfkill(struct asus_wmi *asus, >>>>>> if (!*rfkill) >>>>>> return -EINVAL; >>>>>> >>>>>> + if (dev_id == ASUS_WMI_DEVID_WLAN) >>>>>> + rfkill_set_led_trigger_name(*rfkill, "asus-wlan"); >>>>>> + >>>>> >>>>> I think this is only needed because leds are created before rfkill >>>>> switchs. You may also simply move rfkill_init() above led_init() (take >>>>> care of fail_led, fail_rfkill too). >>>>> >>>>> Another solution is to call led_trigger_set_default() instead of >>>>> rfkill_set_led_trigger_name() >>>> I tried to exchange rfkill_init() and led_init() only in asus_wmi_add(), >>>> and the LED didn't work anymore. >>> >>> Hum, how it didn't work ? the order should not change anything, very strange... >> I re-test again, the LED still works when only exchange rfkill_init() >> and led_init(). >> Sorry for not test it carefully yesterday. > > Ok, good. > >> But when I remove rfkill_set_led_trigger_name() function, the LED stops working. >> The function key won't trigger the LED to change its status. >> But we still can turn it on/off through >> /sys/class/leds/asus-wmi::wlan/brightness >> >> (BTW, I should correct its name to asus::wlan, just like >> asus::touchpad and asus::kbd_backlight) >> >>> >>>> So, I think the current init sequence is correct and should not change it. >>>> >>>> And while calling led_classdev_register() in asus_wmi_led_init() >>>> it will call led_trigger_set_default() in the end of >>>> led_classdev_register() function. >>> >>> I know, but here you have to set the trigger by name because the led >>> is registered before the rfkill trigger. led_trigger_set_default() >>> will do exactly the same thing, but selecting the trigger specified in >>> .default_trigger instead of using a name (thus, avoiding the need to >>> revert the led trigger patch). >> I dig into the code, in net/rfkill/core.c +153 >> static int rfkill_led_trigger_register(struct rfkill *rfkill) >> { >> rfkill->led_trigger.name = rfkill->ledtrigname >> ? : dev_name(&rfkill->dev); >> rfkill->led_trigger.activate = rfkill_led_trigger_activate; >> return led_trigger_register(&rfkill->led_trigger); >> } >> This is the only place that set the led_trigger.name, >> it will assign the name from dev_name(&rfkill->dev) if we didn't >> assign the name to rfkill->ledtrigname. >> >> And in include/linux/device.h +695 >> static inline const char *dev_name(const struct device *dev) >> { >> /* Use the init name until the kobject becomes available */ >> if (dev->init_name) >> return dev->init_name; >> >> return kobject_name(&dev->kobj); >> } >> >> dev_name() will return the name dev->init_name or kobject_name(&dev->kobj) >> And while we register the rfkill by calling rfkill_alloc(), we will >> pass the name into the function. >> In net/rfkill/core.c +831 >> The name only assign to "rfkill->name = name", >> so neither dev->init_name nor kobject_name(&dev->kobj) be assigned the >> rfkill name. >> That means there is no led trigger name be assigned while calling >> rfkill_led_trigger_register() >> So, I don't know how it works if we don't set the led trigger name by >> rfkill_set_led_trigger_name() > > Oh, right, sorry for that, I didn't read that carrefully enought ! Yes > we need that ! (or a way to make all rfkill have a trigger correctly > named without calling that !). > Ok then :) > >> >>> >>>> >>>> We still need to set the led trigger name to rfkill, so that our led function >>>> will be called when rfkill status is changed. >>>> >>>>> >>>>>> rfkill_init_sw_state(*rfkill, !result); >>>>>> result = rfkill_register(*rfkill); >>>>>> if (result) { >>>>>> -- >>>>>> 1.7.9.5 >>>>>> >>>>> >>>>> >>>>> >>>>> -- >>>>> Corentin Chary >>>>> http://xf.iksaif.net >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in >>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>>> >>>> >>>> -- >>>> Chia-Lin Kao(AceLan) >>>> http://blog.acelan.idv.tw/ >>>> E-Mail: acelan.kaoATcanonical.com (s/AT/@/) >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in >>>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >>> >>> >>> -- >>> Corentin Chary >>> http://xf.iksaif.net >>> -- >>> To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in >>> the body of a message to majordomo@xxxxxxxxxxxxxxx >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> >> >> >> -- >> Chia-Lin Kao(AceLan) >> http://blog.acelan.idv.tw/ >> E-Mail: acelan.kaoATcanonical.com (s/AT/@/) > > > > -- > Corentin Chary > http://xf.iksaif.net > -- > To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Chia-Lin Kao(AceLan) http://blog.acelan.idv.tw/ E-Mail: acelan.kaoATcanonical.com (s/AT/@/) -- To unsubscribe from this list: send the line "unsubscribe platform-driver-x86" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
