All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: New dell-wireless driver
  2014-11-22 22:45 New dell-wireless driver Pali Rohár
@ 2014-11-22  2:09 ` Darren Hart
  2014-11-23  0:52   ` Pali Rohár
  2014-11-27  4:27   ` Alex Hung
  0 siblings, 2 replies; 15+ messages in thread
From: Darren Hart @ 2014-11-22  2:09 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alex Hung, Matthew Garrett, platform-driver-x86,
	Gabriele Mazzotta

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?

> 
> 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?

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.

> 
> 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.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-23  0:52   ` Pali Rohár
@ 2014-11-22  3:44     ` Darren Hart
  2014-11-23 15:11       ` Pali Rohár
  2014-11-27  4:41     ` Alex Hung
  1 sibling, 1 reply; 15+ messages in thread
From: Darren Hart @ 2014-11-22  3:44 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Alex Hung, Matthew Garrett, platform-driver-x86,
	Gabriele Mazzotta

On Sun, Nov 23, 2014 at 01:52:46AM +0100, Pali Rohár wrote:
> 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?

We can definitely wait some time. A week is perfectly fine, a couple weeks
starts to be a stretch (relative to the upcoming merge window). Still, I'd
rather get this right. So yes, we can wait.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* New dell-wireless driver
@ 2014-11-22 22:45 Pali Rohár
  2014-11-22  2:09 ` Darren Hart
  0 siblings, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2014-11-22 22:45 UTC (permalink / raw)
  To: Alex Hung, Darren Hart, Matthew Garrett
  Cc: platform-driver-x86, Gabriele Mazzotta

[-- Attachment #1: Type: Text/Plain, Size: 1487 bytes --]

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.

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.

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.

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.

[1] - 
http://thread.gmane.org/gmane.linux.drivers.platform.x86.devel/6036

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-22  2:09 ` Darren Hart
@ 2014-11-23  0:52   ` Pali Rohár
  2014-11-22  3:44     ` Darren Hart
  2014-11-27  4:41     ` Alex Hung
  2014-11-27  4:27   ` Alex Hung
  1 sibling, 2 replies; 15+ messages in thread
From: Pali Rohár @ 2014-11-23  0:52 UTC (permalink / raw)
  To: Darren Hart
  Cc: Alex Hung, Matthew Garrett, platform-driver-x86,
	Gabriele Mazzotta

[-- Attachment #1: Type: Text/Plain, Size: 3711 bytes --]

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.

> > 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@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-22  3:44     ` Darren Hart
@ 2014-11-23 15:11       ` Pali Rohár
  0 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2014-11-23 15:11 UTC (permalink / raw)
  To: Darren Hart
  Cc: Alex Hung, Matthew Garrett, platform-driver-x86,
	Gabriele Mazzotta

[-- Attachment #1: Type: Text/Plain, Size: 682 bytes --]

On Saturday 22 November 2014 04:44:45 Darren Hart wrote:
> On Sun, Nov 23, 2014 at 01:52:46AM +0100, Pali Rohár wrote:
> > 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?
> 
> We can definitely wait some time. A week is perfectly fine, a
> couple weeks starts to be a stretch (relative to the upcoming
> merge window). Still, I'd rather get this right. So yes, we
> can wait.

Driver sent to ML. Setting soft rfkill has problems, so code is 
disabled and driver reports only hard rfkill state.

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-22  2:09 ` Darren Hart
  2014-11-23  0:52   ` Pali Rohár
@ 2014-11-27  4:27   ` Alex Hung
  2014-11-27 11:43     ` Pali Rohár
  1 sibling, 1 reply; 15+ messages in thread
From: Alex Hung @ 2014-11-27  4:27 UTC (permalink / raw)
  To: Darren Hart
  Cc: Pali Rohár, Matthew Garrett,
	platform-driver-x86@vger.kernel.org, Gabriele Mazzotta

On Sat, Nov 22, 2014 at 10:09 AM, Darren Hart <dvhart@infradead.org> 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?
>
Hi Darrent and Pali,

The internal KBC needs not to send any events in addition to the ACPI
event Notify(RBTN, 0x80) according to the information I received from
Dell. I did not received the same event on the Dell systems I tested.

Can I have more detailed information of the system and I can ask Dell
for clarifications.

>>
>> 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.

Yes DELLABCE could have a rkfkill device that providing soft-block
interface; however, I did not see the benefit of introducing the extra
interfaces when one is sufficient.

Prior to Windows 8, each OEM has wireless interfaces of its own.
Microsoft introduces standard wireless interfaces and OEM starts to
drop OEM's interfaces.

I  used the same philosophy and remove rfkill devices if they do not
add extra benefits, especially inconsistency between rfkill devices
can and does causes problems according to past experiences.

>>
>> 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?
>
> 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.
>
>>
>> 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.
>
> --
> Darren Hart
> Intel Open Source Technology Center



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-23  0:52   ` Pali Rohár
  2014-11-22  3:44     ` Darren Hart
@ 2014-11-27  4:41     ` Alex Hung
  2014-12-02 11:06       ` Gabriele Mazzotta
  1 sibling, 1 reply; 15+ messages in thread
From: Alex Hung @ 2014-11-27  4:41 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Darren Hart, Matthew Garrett, platform-driver-x86@vger.kernel.org,
	Gabriele Mazzotta

On Sun, Nov 23, 2014 at 8:52 AM, Pali Rohár <pali.rohar@gmail.com> 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.

>
>> > 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@gmail.com



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-27  4:27   ` Alex Hung
@ 2014-11-27 11:43     ` Pali Rohár
  2014-11-28  5:33       ` Alex Hung
  0 siblings, 1 reply; 15+ messages in thread
From: Pali Rohár @ 2014-11-27 11:43 UTC (permalink / raw)
  To: Alex Hung
  Cc: Darren Hart, Matthew Garrett, platform-driver-x86@vger.kernel.org,
	Gabriele Mazzotta

[-- Attachment #1: Type: Text/Plain, Size: 4453 bytes --]

On Thursday 27 November 2014 05:27:01 Alex Hung wrote:
> On Sat, Nov 22, 2014 at 10:09 AM, Darren Hart 
<dvhart@infradead.org> 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?
> 
> Hi Darrent and Pali,
> 
> The internal KBC needs not to send any events in addition to
> the ACPI event Notify(RBTN, 0x80) according to the
> information I received from Dell. I did not received the same
> event on the Dell systems I tested.
> 
> Can I have more detailed information of the system and I can
> ask Dell for clarifications.
> 

Hello,

I have Dell Latitude E6440.

Do you have some documentation for Dell Embedded Controller (or 
Keyboard controller) which receiving touchpad, trackstick and 
keyboard data and how it modify them?

There are couple of problems with EC on Latitude Exx40 models 
(invalid PS/2 packets from trackstick, keyboard repeating keys, 
keyboard not releasing keys, duplicate key events). You can find 
that these problems are discussed on internet by lot of users and 
only occur on Linux (not Windows!). I would like to fix these 
problems, but without some information from Dell it is 
impossible. I was told that these problems does not come from 
ALPS (touchpad+trackstick) device directly.

Do you have some contacts in Dell (BIOS/firmware/ACPI team) who 
can look at it or provide some information?

> >> 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.
> 
> Yes DELLABCE could have a rkfkill device that providing
> soft-block interface; however, I did not see the benefit of
> introducing the extra interfaces when one is sufficient.
> 

Kernel module dell-laptop.ko (which using Dell SMM, not ACPI!) 
can provides rfkill interface but only on some laptops 
(Prevision, Latitude). For other models there is no rfkill 
interface, so if ACPI provides one it should be exported to 
userspace.

> Prior to Windows 8, each OEM has wireless interfaces of its
> own. Microsoft introduces standard wireless interfaces and
> OEM starts to drop OEM's interfaces.
> 
> I  used the same philosophy and remove rfkill devices if they
> do not add extra benefits, especially inconsistency between
> rfkill devices can and does causes problems according to past
> experiences.
> 
> >> 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?
> > 
> > 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.
> > 
> >> 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.
> > 
> > --
> > Darren Hart
> > Intel Open Source Technology Center

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-27 11:43     ` Pali Rohár
@ 2014-11-28  5:33       ` Alex Hung
  0 siblings, 0 replies; 15+ messages in thread
From: Alex Hung @ 2014-11-28  5:33 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Darren Hart, Matthew Garrett, platform-driver-x86@vger.kernel.org,
	Gabriele Mazzotta

On Thu, Nov 27, 2014 at 7:43 PM, Pali Rohár <pali.rohar@gmail.com> wrote:
> On Thursday 27 November 2014 05:27:01 Alex Hung wrote:
>> On Sat, Nov 22, 2014 at 10:09 AM, Darren Hart
> <dvhart@infradead.org> 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?
>>
>> Hi Darrent and Pali,
>>
>> The internal KBC needs not to send any events in addition to
>> the ACPI event Notify(RBTN, 0x80) according to the
>> information I received from Dell. I did not received the same
>> event on the Dell systems I tested.
>>
>> Can I have more detailed information of the system and I can
>> ask Dell for clarifications.
>>
>
> Hello,
>
> I have Dell Latitude E6440.
>
> Do you have some documentation for Dell Embedded Controller (or
> Keyboard controller) which receiving touchpad, trackstick and
> keyboard data and how it modify them?
>
> There are couple of problems with EC on Latitude Exx40 models
> (invalid PS/2 packets from trackstick, keyboard repeating keys,
> keyboard not releasing keys, duplicate key events). You can find
> that these problems are discussed on internet by lot of users and
> only occur on Linux (not Windows!). I would like to fix these
> problems, but without some information from Dell it is
> impossible. I was told that these problems does not come from
> ALPS (touchpad+trackstick) device directly.

I do not have any Dell's KBC/EC document and I do not have direct
contacts with Dell's Latitude team, but I can try to get to them.

Will you be able to provide dumpfile from "dmidecode,> dmi.log" (you
can send the attachment directly to me). I did some online searches
but am not sure which problems you are referring to. Will you provide
more details of the problems you encountered. I will check with ALPS
to see what they know about them, too.

>
> Do you have some contacts in Dell (BIOS/firmware/ACPI team) who
> can look at it or provide some information?
>
>> >> 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.
>>
>> Yes DELLABCE could have a rkfkill device that providing
>> soft-block interface; however, I did not see the benefit of
>> introducing the extra interfaces when one is sufficient.
>>
>
> Kernel module dell-laptop.ko (which using Dell SMM, not ACPI!)
> can provides rfkill interface but only on some laptops
> (Prevision, Latitude). For other models there is no rfkill
> interface, so if ACPI provides one it should be exported to
> userspace.
>
>> Prior to Windows 8, each OEM has wireless interfaces of its
>> own. Microsoft introduces standard wireless interfaces and
>> OEM starts to drop OEM's interfaces.
>>
>> I  used the same philosophy and remove rfkill devices if they
>> do not add extra benefits, especially inconsistency between
>> rfkill devices can and does causes problems according to past
>> experiences.
>>
>> >> 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?
>> >
>> > 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.
>> >
>> >> 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.
>> >
>> > --
>> > Darren Hart
>> > Intel Open Source Technology Center
>
> --
> Pali Rohár
> pali.rohar@gmail.com



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-11-27  4:41     ` Alex Hung
@ 2014-12-02 11:06       ` Gabriele Mazzotta
  2014-12-02 12:01         ` Pali Rohár
  2014-12-02 15:08         ` Gabriele Mazzotta
  0 siblings, 2 replies; 15+ messages in thread
From: Gabriele Mazzotta @ 2014-12-02 11:06 UTC (permalink / raw)
  To: Alex Hung
  Cc: Pali Rohár, Darren Hart, Matthew Garrett,
	platform-driver-x86@vger.kernel.org

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@gmail.com> 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@gmail.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-12-02 11:06       ` Gabriele Mazzotta
@ 2014-12-02 12:01         ` Pali Rohár
  2014-12-02 15:08         ` Gabriele Mazzotta
  1 sibling, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2014-12-02 12:01 UTC (permalink / raw)
  To: Gabriele Mazzotta, Alex Hung
  Cc: Darren Hart, Matthew Garrett, platform-driver-x86@vger.kernel.org

[-- Attachment #1: Type: Text/Plain, Size: 6631 bytes --]

On Tuesday 02 December 2014 12:06:45 Gabriele Mazzotta wrote:
> 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@gmail.com> 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
> 

Ok, thanks for info.

Alex, do you have documentation what ARBT acpi function exactly 
doing on machines with HW switch and on machines with Fn wifi 
key?

> > >> > 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@gmail.com

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-12-02 11:06       ` Gabriele Mazzotta
  2014-12-02 12:01         ` Pali Rohár
@ 2014-12-02 15:08         ` Gabriele Mazzotta
  2014-12-03  8:50           ` Darren Hart
  1 sibling, 1 reply; 15+ messages in thread
From: Gabriele Mazzotta @ 2014-12-02 15:08 UTC (permalink / raw)
  To: Alex Hung
  Cc: Pali Rohár, Darren Hart, Matthew Garrett,
	platform-driver-x86@vger.kernel.org

On Tuesday 02 December 2014 12:06:45 Gabriele Mazzotta wrote:
> 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@gmail.com> 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

I confirm what I wrote in my previous mail.

I tried Alex's patch and every time I pressed Fn+WiFi I had an rfkill
event, a KEY_WLAN keypress and a KEY_RFKILL keypresses, all together.

Adding something like the following on module load:

	acpi_execute_simple_method(device->handle, "ARBT", 1);

and something like the following on module exit:

	acpi_execute_simple_method(device->handle, "ARBT", 0);

to his code solves the problem. Only KEY_RFKILL is sent to userspace
and nothing is done by the BIOS.

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@gmail.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-12-02 15:08         ` Gabriele Mazzotta
@ 2014-12-03  8:50           ` Darren Hart
  2014-12-04  6:52             ` Alex Hung
  0 siblings, 1 reply; 15+ messages in thread
From: Darren Hart @ 2014-12-03  8:50 UTC (permalink / raw)
  To: Gabriele Mazzotta
  Cc: Alex Hung, Pali Rohár, Matthew Garrett,
	platform-driver-x86@vger.kernel.org

On Tue, Dec 02, 2014 at 04:08:58PM +0100, Gabriele Mazzotta wrote:
> On Tuesday 02 December 2014 12:06:45 Gabriele Mazzotta wrote:
> > 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@gmail.com> 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
> 
> I confirm what I wrote in my previous mail.
> 
> I tried Alex's patch and every time I pressed Fn+WiFi I had an rfkill
> event, a KEY_WLAN keypress and a KEY_RFKILL keypresses, all together.
> 
> Adding something like the following on module load:
> 
> 	acpi_execute_simple_method(device->handle, "ARBT", 1);
> 
> and something like the following on module exit:
> 
> 	acpi_execute_simple_method(device->handle, "ARBT", 0);
> 
> to his code solves the problem. Only KEY_RFKILL is sent to userspace
> and nothing is done by the BIOS.

Will someone be sending a follow-up patch?

With Gabriele's patch above, is there any objection to merging Alex's
dell-wireless patch for 3.19? If Pali's driver adds broader support, we can
always merge them.

-- 
Darren Hart
Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-12-03  8:50           ` Darren Hart
@ 2014-12-04  6:52             ` Alex Hung
  2014-12-04  9:57               ` Pali Rohár
  0 siblings, 1 reply; 15+ messages in thread
From: Alex Hung @ 2014-12-04  6:52 UTC (permalink / raw)
  To: Darren Hart
  Cc: Gabriele Mazzotta, Pali Rohár, Matthew Garrett,
	platform-driver-x86@vger.kernel.org

Sorry for late reply. A few other tasks occupied my time.

Method(ARBT) is defined to enable/disable BIOS handling the radio,
where 1 is enable and 0 is disable, for your information.

However, I checked Latitude E5440, and its ARBT is empty such as

Method (ARBT, 1, NotSerialized)
{
}

This matches to what I remembered when I was working on other systems,
and that's why it is not used in my implementation, but it seems some
systems do implement it. It is good to know.


On Wed, Dec 3, 2014 at 4:50 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Dec 02, 2014 at 04:08:58PM +0100, Gabriele Mazzotta wrote:
>> On Tuesday 02 December 2014 12:06:45 Gabriele Mazzotta wrote:
>> > 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@gmail.com> 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
>>
>> I confirm what I wrote in my previous mail.
>>
>> I tried Alex's patch and every time I pressed Fn+WiFi I had an rfkill
>> event, a KEY_WLAN keypress and a KEY_RFKILL keypresses, all together.
>>
>> Adding something like the following on module load:
>>
>>       acpi_execute_simple_method(device->handle, "ARBT", 1);
>>
>> and something like the following on module exit:
>>
>>       acpi_execute_simple_method(device->handle, "ARBT", 0);
>>
>> to his code solves the problem. Only KEY_RFKILL is sent to userspace
>> and nothing is done by the BIOS.
>
> Will someone be sending a follow-up patch?
>
> With Gabriele's patch above, is there any objection to merging Alex's
> dell-wireless patch for 3.19? If Pali's driver adds broader support, we can
> always merge them.
>
> --
> Darren Hart
> Intel Open Source Technology Center



-- 
Cheers,
Alex Hung

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: New dell-wireless driver
  2014-12-04  6:52             ` Alex Hung
@ 2014-12-04  9:57               ` Pali Rohár
  0 siblings, 0 replies; 15+ messages in thread
From: Pali Rohár @ 2014-12-04  9:57 UTC (permalink / raw)
  To: Alex Hung
  Cc: Darren Hart, Gabriele Mazzotta, Matthew Garrett,
	platform-driver-x86@vger.kernel.org

[-- Attachment #1: Type: Text/Plain, Size: 7770 bytes --]

Thanks for info!

Do you know if Latitude E5440 has Fn wifi key or HW switch?

My Latitude E6440 has only HW switch and ARBT is empty too.

On Thursday 04 December 2014 07:52:21 Alex Hung wrote:
> Sorry for late reply. A few other tasks occupied my time.
> 
> Method(ARBT) is defined to enable/disable BIOS handling the
> radio, where 1 is enable and 0 is disable, for your
> information.
> 
> However, I checked Latitude E5440, and its ARBT is empty such
> as
> 
> Method (ARBT, 1, NotSerialized)
> {
> }
> 
> This matches to what I remembered when I was working on other
> systems, and that's why it is not used in my implementation,
> but it seems some systems do implement it. It is good to
> know.
> 
> On Wed, Dec 3, 2014 at 4:50 PM, Darren Hart 
<dvhart@infradead.org> wrote:
> > On Tue, Dec 02, 2014 at 04:08:58PM +0100, Gabriele Mazzotta 
wrote:
> >> On Tuesday 02 December 2014 12:06:45 Gabriele Mazzotta 
wrote:
> >> > 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@gmail.com> 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
> >> 
> >> I confirm what I wrote in my previous mail.
> >> 
> >> I tried Alex's patch and every time I pressed Fn+WiFi I had
> >> an rfkill event, a KEY_WLAN keypress and a KEY_RFKILL
> >> keypresses, all together.
> >> 
> >> Adding something like the following on module load:
> >>       acpi_execute_simple_method(device->handle, "ARBT",
> >>       1);
> >> 
> >> and something like the following on module exit:
> >>       acpi_execute_simple_method(device->handle, "ARBT",
> >>       0);
> >> 
> >> to his code solves the problem. Only KEY_RFKILL is sent to
> >> userspace and nothing is done by the BIOS.
> > 
> > Will someone be sending a follow-up patch?
> > 
> > With Gabriele's patch above, is there any objection to
> > merging Alex's dell-wireless patch for 3.19? If Pali's
> > driver adds broader support, we can always merge them.
> > 
> > --
> > Darren Hart
> > Intel Open Source Technology Center

-- 
Pali Rohár
pali.rohar@gmail.com

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2014-12-04  9:57 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-22 22:45 New dell-wireless driver Pali Rohár
2014-11-22  2:09 ` Darren Hart
2014-11-23  0:52   ` Pali Rohár
2014-11-22  3:44     ` Darren Hart
2014-11-23 15:11       ` Pali Rohár
2014-11-27  4:41     ` Alex Hung
2014-12-02 11:06       ` Gabriele Mazzotta
2014-12-02 12:01         ` Pali Rohár
2014-12-02 15:08         ` Gabriele Mazzotta
2014-12-03  8:50           ` Darren Hart
2014-12-04  6:52             ` Alex Hung
2014-12-04  9:57               ` Pali Rohár
2014-11-27  4:27   ` Alex Hung
2014-11-27 11:43     ` Pali Rohár
2014-11-28  5:33       ` Alex Hung

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.