On Thursday 27 November 2014 12:41:19 Alex Hung wrote: > On Sun, Nov 23, 2014 at 8:52 AM, Pali Rohár <pali.rohar@xxxxxxxxx> wrote: > > On Saturday 22 November 2014 03:09:06 Darren Hart wrote: > >> On Sat, Nov 22, 2014 at 11:45:08PM +0100, Pali Rohár wrote: > >> > Hello, > >> > > >> > I saw dell-wireless driver on platform-driver-x86 > >> > mailinglist [1] which using DELLABCE acpi device and I do > >> > not like some parts in this driver. > >> > >> Hi Pali, > >> > >> Thanks for reviewing and speaking up :) > >> > >> > First is that this driver export rfkill event as keypress > >> > which is also reported to userspace by keyboard controller. > >> > So then userspace receive two rfkill keypresses. > >> > >> Alex, can you comment? Does the keyboard controller also see > >> this event? > > > > Yes on my laptop E6440. But it looks like it does not have to be > > truth for all laptops... > > > >> > Second is that DELLABCE acpi device can also control "soft" > >> > rfkill status and this driver does not enable it because it > >> > use input class instead rfkill. > >> > > >> > Anyway I have unfinished my version of DELLABCE acpi driver > >> > which will use rfkill interface and plus allow to use hw > >> > switch events in dell-laptop.ko driver. > >> > >> Is this something that could be applied incrementally fo > >> Alex's driver, or is it something we'd be best starting over > >> with? > > > > Alex's driver is different. It registers input device. My > > approach register rfkill device plus add exported functions for > > registering atomic notifier (so other drivers like dell-laptop > > can use events too). > > > > First we need to know if input driver is really needed. And if > > yes determinate in which conditions and for which laptops. Really > > duplicate key press are not good. > > > > In case when input driver is really needed I can just copy > > relevant input code and add it into my driver (in case when my > > driver will be used instead Alex's). This could be no problem, > > because my and Alex code doing different things and so could > > coexist in one driver (but cannot be split into more because only > > one acpi driver can handle one acpi device). > > > >> We have some precedent for input drivers (there is one nearly > >> identical to the dell driver for hp, also by Alex). Using > >> rfkill does seem like the better approach without digging > >> into it. > > > > It is different from HP. Dell ACPI device on some machines can > > also control wifi switches (it can enable/disable it!). > > > > So it make sense to use rfkill. But on some machines that ACPI > > device can only receive events that HW switch was switched, but > > it is possible to ask for state (if is enabled or not). HP driver > > just report switch was changed, but does not report if is enabled > > or disabled. > > > > So I think HP is not identical to this Dell one. > > I can provide a little more information on HP and Dell's design. > > Dell's design is more complex than HP's indeed. > > HP BIOS will send ACPI notification when hotkey is pressed; especially > HP uses buttons instead of hardware slider on their systems. > > Dell has two design > 1. Button similar to HP. My patch targeted this type. > 2. Hardware slider. This handled will handled by Wireless device > drivers (ex. WLAN, BT and so on) by their rfkill hard-block. There is > no need to handle this case. > > This can be distinguished by calling CRBT. I checked Pali's patch and > it was used but the two types are not distinguished. You may want to > use it as hard-block can be out-of-sync with soft-block on corner > cases for Type 2. Hi, my laptop (XPS13 9333) supports both the switch types taken into account in your patch. I can switch between them by calling a method named ARBT. I can see that in your patch CRBT is called to determine the switch type. On my system, that method always returns 0, independently on the current mode. I have to verify this, but I think that would be a problem on my system as by default the BIOS uses what you called "design 2" and there are currently no ways to change it. That means that with your driver KEY_RFKILL would be sent along with the rfkill event. To make things worse, dell-wmi is currently sending KEY_WLAN when I press the Fn key that toggles the state of WiFi and Bluetooth. I think that calling ARBT on driver init would solve all the problems: the correct mode would get selected, the BIOS would stop sending the WMI events that make dell-wmi send KEY_WLAN and it would also no longer hard block the rfkill, letting userspace applications take care of everything. As I said, I have to look into this better and I'll do it as soon as I can. Sorry for being late. Gabriele > >> > Currently dell-laptop.ko driver is using i8042 hook function > >> > for detecting hw switch key press event. It is needed to > >> > detect if rfkill state was changed or not. > >> > > >> > My prepared patches for dell-laptop.ko allows to use acpi > >> > event from DELLABCE driver, so i8042 hook function can be > >> > dropped. Really it is not good idea to pass every PS/2 data > >> > from both keyboard, touchpad and trackpoint to dell-laptop > >> > driver and if there is alternative (DELLABCE) it is better > >> > to use it. > >> > > >> > But now I would like to hear what do you think about it. > >> > > >> > Because only one kernel driver can attach to DELLABCE acpi > >> > device, I cannot use new dell-wireless driver. And I think > >> > only one driver can hit mainline kernel. > >> > >> I would like to see your patch, it sounds like it might be a > >> better option. > > > > Ok, I will sent patches. There are some problems which I'm trying > > to fix together with Gabriele. Do you need to see patches now, or > > can you wait some time until we fix it? > > > > -- > > Pali Rohár > > pali.rohar@xxxxxxxxx -- 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
