Attached patch adds a kernel thread to do polling on Toshiba hardware.
Toshiba hardware is a little oddball, and does not provide ACPI events
on some key presses, typically Fn hotkey buttons. The key interface is
now polled, and events now matched to a list of toshiba specific
scancodes, and are squirted to userspace using the INPUT subsystem.
This means that toshiba laptops buttons "just work" without any
userspace daemon (using uinput) such as fnfx or bodges such as using a
userspace hal addon. Doing the polling in kernel is more efficient, and
makes stuff just work out of the box. You can assign the keys using
standard X keymaps, or using tools such as gnome-keybinding-properties.
This is similar to other patches sent for the thinkpad_acpi driver, and
is part of the "Unf*ck my keyboard" initiative to make multimedia keys
just work.
Changes from the first patch involve switching to a workqueue for the
polling, not breaking the spaces in "hotkeys_via_input" and also masking
out the fn key button up.
toshiba_acpi.c | 228 +++++++++++++++++++++++++++++++++++++++++++++++++++------
1 file changed, 206 insertions(+), 22 deletions(-)
Signed-off-by: Richard Hughes <[email protected]>
On Thu, 2007-05-31 at 13:53 +0100, Bastien Nocera wrote:
> On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
> > Attached patch adds a kernel thread to do polling on Toshiba hardware.
> >
> > Toshiba hardware is a little oddball, and does not provide ACPI events
> > on some key presses, typically Fn hotkey buttons. The key interface is
> > now polled, and events now matched to a list of toshiba specific
> > scancodes, and are squirted to userspace using the INPUT subsystem.
> >
> > This means that toshiba laptops buttons "just work" without any
> > userspace daemon (using uinput) such as fnfx or bodges such as using a
> > userspace hal addon. Doing the polling in kernel is more efficient, and
> > makes stuff just work out of the box. You can assign the keys using
> > standard X keymaps, or using tools such as gnome-keybinding-properties.
> >
> > This is similar to other patches sent for the thinkpad_acpi driver, and
> > is part of the "Unf*ck my keyboard" initiative to make multimedia keys
> > just work.
> >
> > Changes from the first patch involve switching to a workqueue for the
> > polling, not breaking the spaces in "hotkeys_via_input" and also masking
> > out the fn key button up.
>
> A couple of things:
> - use a switch statement in toshiba_deliver_button_event(), would look a
> bit cleaner.
Agree, done.
> - make the magic number #defines
Agree, done.
> - not sure if it's possible, but you should disable the workqueue
> altogether if nothing has the input device opened.
Do we get an event when the [input]->users value changes? If not, we
could do 1Hz (users > 0) checking when the input device is unclaimed,
and then switch to 4Hz polling when the input device is open.
> Hopefully we'll find a way to receive an interrupt, or some kind of
> event when the keys are pressed in the future, and completely avoid the
> polling. In the meantime, it should be minimised.
Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
toshiba supplied daemon that polls, so I think we have to just bite the
bullet.
New patch attached.
Richard.
On Thu, 2007-05-31 at 13:36 +0100, Richard Hughes wrote:
> Attached patch adds a kernel thread to do polling on Toshiba hardware.
>
> Toshiba hardware is a little oddball, and does not provide ACPI events
> on some key presses, typically Fn hotkey buttons. The key interface is
> now polled, and events now matched to a list of toshiba specific
> scancodes, and are squirted to userspace using the INPUT subsystem.
>
> This means that toshiba laptops buttons "just work" without any
> userspace daemon (using uinput) such as fnfx or bodges such as using a
> userspace hal addon. Doing the polling in kernel is more efficient, and
> makes stuff just work out of the box. You can assign the keys using
> standard X keymaps, or using tools such as gnome-keybinding-properties.
>
> This is similar to other patches sent for the thinkpad_acpi driver, and
> is part of the "Unf*ck my keyboard" initiative to make multimedia keys
> just work.
>
> Changes from the first patch involve switching to a workqueue for the
> polling, not breaking the spaces in "hotkeys_via_input" and also masking
> out the fn key button up.
A couple of things:
- use a switch statement in toshiba_deliver_button_event(), would look a
bit cleaner.
- make the magic number #defines
- not sure if it's possible, but you should disable the workqueue
altogether if nothing has the input device opened.
Hopefully we'll find a way to receive an interrupt, or some kind of
event when the keys are pressed in the future, and completely avoid the
polling. In the meantime, it should be minimised.
Cheers
--
Bastien Nocera <[email protected]>
On Thu, 2007-05-31 at 14:44 +0100, Richard Hughes wrote:
> Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
> toshiba supplied daemon that polls, so I think we have to just bite the
> bullet.
... adding in reply in different thread...
On Thu, 2007-05-31 at 10:37 -0400, Dmitry Torokhov wrote:
> Please use input-polldev to set up polled devices. It
> will create work queue for you and will make sure that polling is
> stopped when device is closed.
Okay, I had never heard of this. I've done the suggested changes in the
attached patch. Please can you have a look and make sure I'm being sane.
> Also I don't think you want to use
> KEY_BREAK. What is the expected function of that key?
It's a "lock" key, I really want KEY_LOCK added to input.h, but that
might prove difficult. For now I've used KEY_CLEAR, yell if you think
there's a better one to substitute or if you guys want me to add get a
constant added to input.h.
Thanks for the review.
Richard.
Hi,
On Thu, May 31, 2007 at 04:46:56PM +0100, Richard Hughes wrote:
+ if (!hotkeys_over_input) {
+ if (!key_event_valid) {
+ hci_read1(HCI_SYSTEM_EVENT, &value, &hci_result);
+ if (hci_result == HCI_SUCCESS) {
+ key_event_valid = 1;
+ last_key_event = value;
+ } else if (hci_result == HCI_EMPTY) {
+ /* better luck next time */
HCI_EMPTY is *by far* the most frequent state to occur I think
(users won't press keys all the time), thus it's probably better(?)
for branch prediction to have this placed first, right?
Not that it matters too much instruction-wise, but still...
Apart from that I'm very happy to see progress on this front
(speaking as a "proud" owner of an old Toshiba notebook requiring
this stuff).
Oh, and maybe merge the sprintf()s into a single one to reduce code size.
And I'd definitely move the multiple identical "Re-enabled hotkeys" parts
into one single non-inlined(!) function for the same reason.
Not to mention that it's BUTT UGLY to have the *same* fat
multi-line comment duplicated bazillion times.
Thanks a lot!
Andreas Mohr
On Thu, 2007-05-31 at 18:46 +0200, Andreas Mohr wrote:
> HCI_EMPTY is *by far* the most frequent state to occur I think
> (users won't press keys all the time), thus it's probably better(?)
> for branch prediction to have this placed first, right?
> Not that it matters too much instruction-wise, but still...
Sure.
> Apart from that I'm very happy to see progress on this front
> (speaking as a "proud" owner of an old Toshiba notebook requiring
> this stuff).
Good - this stuff should just work :-)
> And I'd definitely move the multiple identical "Re-enabled hotkeys" parts
> into one single non-inlined(!) function for the same reason.
> Not to mention that it's BUTT UGLY to have the *same* fat
> multi-line comment duplicated bazillion times.
Agree. New patch (untested) attached.
Thanks for review,
Richard.
Hi Richard,
On 5/31/07, Richard Hughes <[email protected]> wrote:
> On Thu, 2007-05-31 at 14:44 +0100, Richard Hughes wrote:
> > Nope, impossible AFAICS. The hardware is just broken. Windows XP has an
> > toshiba supplied daemon that polls, so I think we have to just bite the
> > bullet.
>
> ... adding in reply in different thread...
>
> On Thu, 2007-05-31 at 10:37 -0400, Dmitry Torokhov wrote:
> > Please use input-polldev to set up polled devices. It
> > will create work queue for you and will make sure that polling is
> > stopped when device is closed.
>
> Okay, I had never heard of this. I've done the suggested changes in the
> attached patch. Please can you have a look and make sure I'm being sane.
>
The patch was pretty good, I did not quite like the driver
registration code so I tried to clean it up. What do you think about
the attached patch (not tested due to the lack of hardware). If you
are OK with it please add your signed-off-by because version of the
patch I grabbed did not have it.
> > Also I don't think you want to use
> > KEY_BREAK. What is the expected function of that key?
>
> It's a "lock" key, I really want KEY_LOCK added to input.h, but that
> might prove difficult. For now I've used KEY_CLEAR, yell if you think
> there's a better one to substitute or if you guys want me to add get a
> constant added to input.h.
Iam still struggling with the purpose of the key. What would you
extect to happen when youser presses this key? Screen gets locked?
KEY_SCREENLOCK? KEY_SCREENSAVER?
--
Dmitry
On Fri, 2007-06-01 at 12:45 -0400, Dmitry Torokhov wrote:
> The patch was pretty good, I did not quite like the driver
> registration code so I tried to clean it up.
Cool, cheers.
> What do you think about
> the attached patch (not tested due to the lack of hardware). If you
> are OK with it please add your signed-off-by because version of the
> patch I grabbed did not have it.
Attached patch looks good - I can't test it until next week, but looks
logically correct to me. I would also like to add the oddball proc event
notification system to the removal-schedule document, but this can be a
fight for another day.
> > > Also I don't think you want to use
> > > KEY_BREAK. What is the expected function of that key?
To lock the screen. We probably want to replace HCI_HKEY_BREAK with
HCI_HKEY_LOCK, as the picture on the keys is a little padlock.
> > It's a "lock" key, I really want KEY_LOCK added to input.h, but that
> > might prove difficult. For now I've used KEY_CLEAR, yell if you think
> > there's a better one to substitute or if you guys want me to add get a
> > constant added to input.h.
>
> Iam still struggling with the purpose of the key. What would you
> extect to happen when youser presses this key? Screen gets locked?
> KEY_SCREENLOCK? KEY_SCREENSAVER?
Either of these keys would be good to add.
So yes, patch looks good, cheers for the improvements.
Signed-off-by: Richard Hughes <[email protected]>
Thanks again,
Richard.
On Sat, Jun 02, 2007 at 01:50:52PM +0100, Richard Hughes wrote:
> > KEY_SCREENLOCK? KEY_SCREENSAVER?
>
> Either of these keys would be good to add.
We've been interpreting "KEY_COFFEE" as screenlock (as in "I've gone for
a coffee break, lock the screen").
--
Matthew Garrett | [email protected]
On Saturday 02 June 2007 10:23, Matthew Garrett wrote:
> On Sat, Jun 02, 2007 at 01:50:52PM +0100, Richard Hughes wrote:
> > > KEY_SCREENLOCK? KEY_SCREENSAVER?
> >
> > Either of these keys would be good to add.
>
> We've been interpreting "KEY_COFFEE" as screenlock (as in "I've gone for
> a coffee break, lock the screen").
>
It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal Lock/Screensaver
so your interpretation is indeed correct. I guess I better add an alias to
input.h
--
Dmitry
On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote:
>
>
> It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal
> Lock/Screensaver
> so your interpretation is indeed correct. I guess I better add an
> alias to
> input.h
What's the status of this patch? Good for merging? Do you want me to
redo the current patch using input-polldev and the new setkeycode stuff?
Richard.
On 6/7/07, Richard Hughes <[email protected]> wrote:
> On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote:
> >
> >
> > It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal
> > Lock/Screensaver
> > so your interpretation is indeed correct. I guess I better add an
> > alias to
> > input.h
>
> What's the status of this patch? Good for merging?
Not my call - John is the official maintainer of the driver. However
from input layer POV it looks good.
> Do you want me to
> redo the current patch using input-polldev
The patch I send to you earlier already does this. I am attaching a
refreshed version that used KEY_COFFEE instead of KEY_BREAK.
> and the new setkeycode stuff?
And I have the patch doing this as well (attached). If you could
please give it a try I'd appreciate it.
--
Dmitry
On Fri, 2007-06-08 at 10:23 -0400, Dmitry Torokhov wrote:
> On 6/7/07, Richard Hughes <[email protected]> wrote:
> > On Sun, 2007-06-03 at 00:48 -0400, Dmitry Torokhov wrote:
> > >
> > >
> > > It looks like KEY_COFFE comes from 0x0c/0x19e - AL Terminal
> > > Lock/Screensaver
> > > so your interpretation is indeed correct. I guess I better add an
> > > alias to
> > > input.h
> >
> > What's the status of this patch? Good for merging?
>
> Not my call - John is the official maintainer of the driver. However
> from input layer POV it looks good.
Good, thanks.
> > Do you want me to
> > redo the current patch using input-polldev
>
> The patch I send to you earlier already does this. I am attaching a
> refreshed version that used KEY_COFFEE instead of KEY_BREAK.
Brilliant, these look great.
> > and the new setkeycode stuff?
>
> And I have the patch doing this as well (attached). If you could
> please give it a try I'd appreciate it.
You beat me to it {hastily removes item from TODO...} :-)
I'll give it a try as soon as possible, thanks.
Richard.
Richard Hughes wrote:
> What's the status of this patch? Good for merging? Do you want me to
> redo the current patch using input-polldev and the new setkeycode stuff?
A detail:
I receive error message (no such device) when I try load toshiba_acpi on
my Toshiba M45-S355 (BIOS is Toshiba and NOT Phoenix).
So, I can't change brightness of LCD.
To solve it, I use omnibook driver available in:
<http://omnibook.sourceforge.net>
Last SVN can get with:
svn export https://svn.sourceforge.net/svnroot/omnibook/omnibook/trunk
With omnibook driver I can change brightness writing in /proc/omnibook/lcd
Only one problem: multimedia keys don't is recognized by xev and I can't
configure it with omnibook driver.
Best regards,
Renato S. Yamane
Richard Hughes wrote:
> Attached patch adds a kernel thread to do polling on Toshiba hardware.
Is there something similar available to support other Toshiba laptops? I own a
A110-178 that is not supported by the driver but has a lot of keys that I
can't use currently:
Fn:
-screen zoom (I actually don't care about this one)
-S2RAM
-S2Disk
-Mute*
Multimedia:
-Browser
-Video player
-Play/pause
-Stop
-Forward
-Reverse
*Mute: it's a bit special. If I use it I can an on screen display (mute/sound)
but regardless of which setting is displayed the sound is still there.
None of the above keys generated a key event. Neither does "Brightness down",
but it still works. "Brightness up" generates an event and works. Kpowersave
tells me it can't do brightness switching in software (which works in
WinXtraPain).
If you need an acpidump or I can do some testing just ask.
Eike
On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote:
> None of the above keys generated a key event. Neither does "Brightness down",
> but it still works. "Brightness up" generates an event and works. Kpowersave
> tells me it can't do brightness switching in software (which works in
> WinXtraPain).
Do you know how windows does this? Do you have to load a special
system-try thing to make the keys work?
Richard.
Richard Hughes wrote:
> On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote:
> > None of the above keys generated a key event. Neither does "Brightness
> > down", but it still works. "Brightness up" generates an event and works.
> > Kpowersave tells me it can't do brightness switching in software (which
> > works in WinXtraPain).
>
> Do you know how windows does this? Do you have to load a special
> system-try thing to make the keys work?
I'll have a look once I find time to boot Windows.
Eike
Richard Hughes wrote:
> On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote:
> > None of the above keys generated a key event. Neither does "Brightness
> > down", but it still works. "Brightness up" generates an event and works.
> > Kpowersave tells me it can't do brightness switching in software (which
> > works in WinXtraPain).
>
> Do you know how windows does this? Do you have to load a special
> system-try thing to make the keys work?
Yes, I need a special process running for this. Would it be of some use if I
take a look with IRPtracker on it or is that all you need to know?
Eike
On Tue, 2007-06-26 at 07:03 +0200, Rolf Eike Beer wrote:
> Richard Hughes wrote:
> > On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote:
> > > None of the above keys generated a key event. Neither does
> "Brightness
> > > down", but it still works. "Brightness up" generates an event and
> works.
> > > Kpowersave tells me it can't do brightness switching in software
> (which
> > > works in WinXtraPain).
> Yes, I need a special process running for this. Would it be of some
> use if I take a look with IRPtracker on it or is that all you need to
> know?
Yes, although this is out of my area or expertise, sorry.
Richard.
Richard Hughes wrote:
> On Tue, 2007-06-26 at 07:03 +0200, Rolf Eike Beer wrote:
>> Richard Hughes wrote:
>>> On Sat, 2007-06-23 at 16:56 +0200, Rolf Eike Beer wrote:
>>>> None of the above keys generated a key event. Neither does
>>>> "Brightness down", but it still works. "Brightness up" generates an event
>>>> and works.
>>
>>>> Kpowersave tells me it can't do brightness switching in software
>>>> (which works in WinXtraPain).
>>
>> Yes, I need a special process running for this. Would it be of some
>> use if I take a look with IRPtracker on it or is that all you need to
>> know?
>
> Yes, although this is out of my area or expertise, sorry.
I've looked a bit but can't find any driver interaction of those programs. Any
further ideas welcome.
Eike
Rolf Eike Beer wrote:
> Richard Hughes wrote:
>> Yes, although this is out of my area or expertise, sorry.
>
> I've looked a bit but can't find any driver interaction of those programs. Any
> further ideas welcome.
Do you try omnibook driver?
svn export https://svn.sourceforge.net/svnroot/omnibook/omnibook/trunk
toshiba_acpi don't work on my Toshiba M45-S355 (Toshiba BIOS) and I try
this module above and now I can change brightness writing in
/proc/omnibook/lcd and kpowersave can change brightness too.
But, is impossible use multimedia keys (play, pause, browser, etc) and
couple keys (fn-fx), because Fn key and Multimedia keys don't is recognized.
Regards,
Renato S. Yamane