wistron_btns - Is polling always required?

wistron_btns - Is polling always required?

[Carlos Corbacho]

I've been looking over the wistron_btns driver (again), and am rather
puzzled by some of the latest additions to it.

In particular, laptops such as the 3020 and 5020 series (both identical, bar
different AMD processors) are being polled to map their extra keys -
however, both these laptops (and, AFAIK, most modern Acer laptops since at
least 2005) do _not_ require this - all the extra keys are already emitting
scancodes.

>From what I can see, it would be surely better to drop the BIOS probing and
polling on the newer laptops, and then either:

A) Use the setkeycode ioctl for these particular laptops

Pro: 
For users with x86-64 Acer laptops that produce scancodes, wistron_btns
could be made to work, since the BIOS probing and polling code is now
unnecessary.

Con:
Should setkeycode really be used in a kernel driver? (I don't know what the
official policy is here)

B) Let userspace map the scancodes to keycodes (such as HAL 0.5.10 and above
is doing at the moment) (i.e. cut back wistron_btns to _just_ the older
models that really do need polling to generate keycodes).

Pro:
Don't have to update the kernel for every single new Acer model and keyboard
layout found (in the case of HAL, it would just be the hal-info package,
which contains all this kind of information).

Con:
Not everyone may be using HAL - potential for inconsistency in keyboard
mappings.

-Carlos
-- 
E-Mail: cathectic@gmail.com
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: wistron_btns - Is polling always required?

[Dmitry Torokhov]

Hi Carlos,

On Thursday 19 July 2007 13:03, Carlos Corbacho wrote:
> I've been looking over the wistron_btns driver (again), and am rather
> puzzled by some of the latest additions to it.
> 
> In particular, laptops such as the 3020 and 5020 series (both identical, bar
> different AMD processors) are being polled to map their extra keys -
> however, both these laptops (and, AFAIK, most modern Acer laptops since at
> least 2005) do _not_ require this - all the extra keys are already emitting
> scancodes.
> 
> From what I can see, it would be surely better to drop the BIOS probing and
> polling on the newer laptops, and then either:
> 
> A) Use the setkeycode ioctl for these particular laptops
> 
> Pro: 
> For users with x86-64 Acer laptops that produce scancodes, wistron_btns
> could be made to work, since the BIOS probing and polling code is now
> unnecessary.
> 
> Con:
> Should setkeycode really be used in a kernel driver? (I don't know what the
> official policy is here)
> 
> B) Let userspace map the scancodes to keycodes (such as HAL 0.5.10 and above
> is doing at the moment) (i.e. cut back wistron_btns to _just_ the older
> models that really do need polling to generate keycodes).
> 
> Pro:
> Don't have to update the kernel for every single new Acer model and keyboard
> layout found (in the case of HAL, it would just be the hal-info package,
> which contains all this kind of information).
> 
> Con:
> Not everyone may be using HAL - potential for inconsistency in keyboard
> mappings.
> 

I was not aware that these models can deliver events for hotkeys via
atkbd. I think option B is best since interrupt-driven mode is always
better than polling. I am CC-ing Eric Piel who did most of the work
on new models support in wistron_btns.

-- 
Dmitry

Re: wistron_btns - Is polling always required?

[Carlos Corbacho]

Dimitry, Éric,

On Friday 20 July 2007 05:06:35 Dmitry Torokhov wrote:
> I was not aware that these models can deliver events for hotkeys via
> atkbd. I think option B is best since interrupt-driven mode is always
> better than polling. I am CC-ing Eric Piel who did most of the work
> on new models support in wistron_btns.

Looking around a bit more, the following should definitely be removed from 
wistron_btns:

Acer Aspire 3020
Acer Aspire 5020

The following I'm not entirely sure on these, but they _probably_ should be 
removed:

Acer TravelMate 2100
Acer TravelMate 2410 (identical hardware to the Aspire 1610, which is known to 
generate scancodes)
Acer TravelMate 2424NWXCi (aka TravelMate 2420 series)

As a rule of thumb, if the laptop is based on Acer's "folio" design, which 
they've used for most of the TravelMate and Aspire laptops from 2004 to the 
present day (although some of the newer ones are now using a different 
design), and/ or the model/ series number is a four digit one (such as 5020), 
it shouldn't be in wistron_btns (the Aspire 1500 and Aspire 1600 I would make 
an exception for, as they don't appear to be based on the "folio" design, and 
therefore don't fit this pattern).

-Carlos
-- 
E-Mail: cathectic@gmail.com
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: wistron_btns - Is polling always required?

[Éric Piel]

07/22/2007 04:14 PM, Carlos Corbacho wrote/a écrit:
> Dimitry, Éric,
> 
> On Friday 20 July 2007 05:06:35 Dmitry Torokhov wrote:
>> I was not aware that these models can deliver events for hotkeys via
>> atkbd. I think option B is best since interrupt-driven mode is always
>> better than polling. I am CC-ing Eric Piel who did most of the work
>> on new models support in wistron_btns.
> 
> Looking around a bit more, the following should definitely be removed from 
> wistron_btns:
> 
> Acer Aspire 3020
> Acer Aspire 5020
> 
> The following I'm not entirely sure on these, but they _probably_ should be 
> removed:
> 
> Acer TravelMate 2100
> Acer TravelMate 2410 (identical hardware to the Aspire 1610, which is known to 
> generate scancodes)
> Acer TravelMate 2424NWXCi (aka TravelMate 2420 series)
> 
> As a rule of thumb, if the laptop is based on Acer's "folio" design, which 
> they've used for most of the TravelMate and Aspire laptops from 2004 to the 
> present day (although some of the newer ones are now using a different 
> design), and/ or the model/ series number is a four digit one (such as 5020), 
> it shouldn't be in wistron_btns (the Aspire 1500 and Aspire 1600 I would make 
> an exception for, as they don't appear to be based on the "folio" design, and 
> therefore don't fit this pattern).

Hi,

I finally got a hand on an old 3020, a 32 bit-only one. I can confirm 
that even on this old version the keys are also passed through atkdb 
(including wifi and bluetooth keys). I had put all the one you mentioned 
only because they were described in the acerhk driver as using the 
wistron interface. I'm not attached to them at all ;-)

So I'm fine with your suggestion to remove their entries, at least for: 
3020, 5020, 2100, 2410. For the 2420, it came earlier from Ashutosh Naik 
(cc'ed now), asking him first if it's also working without the driver 
would be much better!

By removing the entries, we should keep in mind that with the vanilla 
kernel, there is still a small functionality loss: wireless and led 
activations are lost. Those functionalities are provided by the 
acer_acpi driver (with a much cleaner approach), but it's unlikely to 
reach the vanilla kernel within less than three or four releases. So 
Dimitry should decide if we want to only remove the key part from those 
entries, leaving the led and wireless part until acer_acpi gets merged 
or if we should delete them completely right now.

See you,
Eric

Re: wistron_btns - Is polling always required?

[Carlos Corbacho]

On Wednesday 25 July 2007 22:41:09 Éric Piel wrote:
> So I'm fine with your suggestion to remove their entries, at least for:
> 3020, 5020, 2100, 2410. For the 2420, it came earlier from Ashutosh Naik
> (cc'ed now), asking him first if it's also working without the driver
> would be much better!

Fair enough. :-)

> By removing the entries, we should keep in mind that with the vanilla
> kernel, there is still a small functionality loss: wireless and led
> activations are lost. 

True, but then again, this still really shouldn't be done by polling (perhaps 
some sort of userspace application would do-the-right-thing?)

> Those functionalities are provided by the 
> acer_acpi driver (with a much cleaner approach), 

For the 3020/ 5020 yes. For the other machines I do not have DSDTs for them 
yet, so I don't know if acer_acpi will work with them or not (unfortunately 
acer_acpi is limited to a small subset of Acer's 2005 and newer laptops - I 
just don't know all of them yet).

> but it's unlikely to 
> reach the vanilla kernel within less than three or four releases. 

Possibly - it won't go in as is though (I'm currently waiting on Len Brown to 
release a generic WMI ACPI implementation that acer_acpi can then build on, 
and hopefully _that_ will then get submitted upstream; acer_acpi in it's 
current form has been NAKed).

> So 
> Dimitry should decide if we want to only remove the key part from those
> entries, leaving the led and wireless part until acer_acpi gets merged
> or if we should delete them completely right now.

I would say not to base this decision on acer_acpi - I can't yet guarantee it 
will support anything other than the 3020/5020 of the other listed laptops. I 
would say at the very least remove the 5020 from wistron_btns, otherwise it's 
left with inconsistent support between x86 and x86-64.

-Carlos
-- 
E-Mail: cathectic@gmail.com
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D

Re: wistron_btns - Is polling always required?

[Éric Piel]

07/27/2007 01:20 PM, Carlos Corbacho wrote/a écrit:
> On Wednesday 25 July 2007 22:41:09 Éric Piel wrote:
>> By removing the entries, we should keep in mind that with the vanilla
>> kernel, there is still a small functionality loss: wireless and led
>> activations are lost. 
> 
> True, but then again, this still really shouldn't be done by polling (perhaps 
> some sort of userspace application would do-the-right-thing?)
Yes, that's right, I hadn't thought this way. What we could do is that 
if an entry doesn't contain any key related info, we don't activate the 
polling. That would require adding a user interface to control the 
bluetooth and wifi cards...


>> Those functionalities are provided by the 
>> acer_acpi driver (with a much cleaner approach), 
> 
> For the 3020/ 5020 yes. For the other machines I do not have DSDTs for them 
> yet, so I don't know if acer_acpi will work with them or not (unfortunately 
> acer_acpi is limited to a small subset of Acer's 2005 and newer laptops - I 
> just don't know all of them yet).
[Ah! I hadn't realized that Carlos and Cathectic were the same person 
;-) So _you_ are writing acer_acpi :-) ]
So there might be some laptop which do report keys directly via atkbd 
but do not have the ACPI interface to control the led/wireless? What a 
mess... Well, then this solution of keeping the support in wistron_btns 
is necessary.

BTW, do you know you'll find plenty of acer DSDTs at this address: 
http://acpi.sourceforge.net/dsdt/view.php?manufacturer=ACER . At least, 
there is 3020 and 2410 that you are looking for :-)

>> So 
>> Dimitry should decide if we want to only remove the key part from those
>> entries, leaving the led and wireless part until acer_acpi gets merged
>> or if we should delete them completely right now.
> 
> I would say not to base this decision on acer_acpi - I can't yet guarantee it 
> will support anything other than the 3020/5020 of the other listed laptops. I 
> would say at the very least remove the 5020 from wistron_btns, otherwise it's 
> left with inconsistent support between x86 and x86-64.
Well, let's just remove the key part for every hardware which does 
report keys via atkbd. Once acer_acpi is merged we can also drop 
completely the laptops which it supports. It doesn't really matter much 
if x86 has slightly more hardware support than x86-64.

One very nice thing would be to agree on a common interface to control 
the wireless interfaces between wistron_btns and acer_acpi (IIRC, led 
interface is already mostly identical). So that users don't have the 
interface changing depending on which acer laptop or which driver they 
are using. That's basically two files called wifi and bluetooth, taking 
1 for activation and 0 for deactivation. Anyone can suggest a good path 
to put them in? (It will be /sys/... definitely as nothing should be 
added to /proc anymore)

/sys/devices/platform/acer/ ?

Eric

Re: wistron_btns - Is polling always required?

[Carlos Corbacho]

On Friday 27 July 2007 14:07:25 Éric Piel wrote:
> [Ah! I hadn't realized that Carlos and Cathectic were the same person
> ;-) So _you_ are writing acer_acpi :-) ]

[Yes (I really don't like Google obfuscating my name with my user name - I'm 
quite happy for people to know who I really am).]

> So there might be some laptop which do report keys directly via atkbd
> but do not have the ACPI interface to control the led/wireless? 

Yes. AFAIK, it seems the "newer" Acer laptops (2006 and newer) do all come 
with a useable ACPI interface (at least, no one has reported to me one that 
doesn't). The 2004 to 2005 are hit and miss. I don't know of any Acer laptop 
before 2004 that could be supported by acer_acpi (which in turn tend to be 
the ones more likely to require wistron_btns anyway).

> What a 
> mess... Well, then this solution of keeping the support in wistron_btns
> is necessary.

For enabling the hardware on many of these laptops, yes.

> BTW, do you know you'll find plenty of acer DSDTs at this address:
> http://acpi.sourceforge.net/dsdt/view.php?manufacturer=ACER . At least,
> there is 3020 and 2410 that you are looking for :-)

I've mostly found that a bit of a pain to sift through (since I really want 
the original DSDTs, not the user modified ones, and not everyone submits 
their originals, or marks properly which one they uploaded. Although I will 
get around to poking about there and update acer_acpi's supported hardware 
list soon).

> Well, let's just remove the key part for every hardware which does
> report keys via atkbd. Once acer_acpi is merged we can also drop
> completely the laptops which it supports. It doesn't really matter much
> if x86 has slightly more hardware support than x86-64.

Agreed.

> One very nice thing would be to agree on a common interface to control
> the wireless interfaces between wistron_btns and acer_acpi (IIRC, led
> interface is already mostly identical). So that users don't have the
> interface changing depending on which acer laptop or which driver they
> are using. That's basically two files called wifi and bluetooth, taking
> 1 for activation and 0 for deactivation. Anyone can suggest a good path
> to put them in? (It will be /sys/... definitely as nothing should be
> added to /proc anymore)
>
> /sys/devices/platform/acer/ ?

That sounds fine - my original upstream proposal for acer_acpi had something 
like that:

1) /sys/devices/platform/acer_acpi for wireless and bluetooth
2) Register Mail LED with LED subsystem

(I experimented with rfkill for wireless and bluetooth, but I didn't like the 
results - I ended up with devices named 'rfkill0', 'rfkill1', etc, which 
wasn't very helpful).

-Carlos
-- 
E-Mail: cathectic@gmail.com
Web: strangeworlds.co.uk
GPG Key ID: 0x23EE722D