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
