Hi!
I've reimplemented the Lifebook touchscreen driver using libps2 and
input, to make it short and fitting into the kernel drivers.
Please comment on code and test for functionality!
PS.: The driver should register two input devices. It doesn't yet,
since that isn't very straightforward in the psmouse framework.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
> Hi!
>
> I've reimplemented the Lifebook touchscreen driver using libps2 and
> input, to make it short and fitting into the kernel drivers.
>
> Please comment on code and test for functionality!
>
> PS.: The driver should register two input devices. It doesn't yet,
> since that isn't very straightforward in the psmouse framework.
First of all it's good to see lifebook-support integrated.
I have tested your driver on my box but the initialization fails. Do you
have hardware available for testing? As far as I can see the
init-sequence is the one from Haralds xfree 3.3.6-driver. There is a
reason why this sequence is not like that anymore in my driver. This
sequence does not always work and there is not something like a "magic
knock sequence". The lifebook-touchscreen hardware is a little bit slow
and it happens quite often that it does not understand a command that
you send. This is the reason why you often have to send a command
several times until the hardware understands...
Probably this was what was seen by Harald on the USB-bus and he just
used this sequence.
Second thing is that I am not shure that it is a good idea to integrate
the lbtouch-support into the psmouse-driver since there is no real way
of deciding if the device you are talking to is REALLY a
lifebook-touchsreen or not.
You have put the init-sequence for the hw into the
lifebook_detect-function which is not correct since this not really a
"detect" but a HW-init.
IMHO the driver should be standalone and the user should decide which
driver he wants to use. As default the touchscreen-functionality will be
disabled and only the quick-point device will work like a normal
PS2-mouse.
I also don't think that it is useful to have two devices for the
touchscreen. I assume you want to have one device for relative movements
and one for the absolute ones!? But for the implementor in userspace (X,
SDL,...) it will be easier if ALL events from this HW-device come via
one device-node.
I attached the current version of my driver here so people can also have
a look at it. I didn't release this version on my homepage yet and the
only difference between the released version an this one is that the
y-axis is swapped. I did this since all input devices seem to have a
common idea of where y=0 is and my driver was the only one where y was
just the other way round.
One more thing I observed by inspecting your code is that your
untouch-event will never happen.
I think my driver works and is clean enough for integration into the
kernel. We can talk about integrating calls to libps2 instead of doing
the stuff myself (I am a big fan of preventing code-duplication) but
don't you think it would be more wise to use a driver which works (since
the very early 2.6-days) and build upon that instead of trying to
implement your own one from the scratch and using snippets from very old
and obsolete code?
And as far as I can see you don't even have access to the hardware!?!?
Greetings
Kenan
On Sat, 2005-02-12 at 18:01 +0100, Kenan Esau wrote:
> Second thing is that I am not shure that it is a good idea to integrate
> the lbtouch-support into the psmouse-driver since there is no real way
> of deciding if the device you are talking to is REALLY a
> lifebook-touchsreen or not.
...
> IMHO the driver should be standalone and the user should decide which
> driver he wants to use. As default the touchscreen-functionality will be
> disabled and only the quick-point device will work like a normal
> PS2-mouse.
I just want to point out that this is a problem for distributions and
for not-so-technical users.
Is there *really* no way to know if you're on a lifebook? Can't you use
say the DMI identification mechanism to find this out ? If so, I think
integrating into the regular driver very much is the right thing to
do... it makes things JustWork(tm) for users without any need for manual
configuration (which also makes distribution makers happy).
On Saturday 12 February 2005 12:01, Kenan Esau wrote:
> Second thing is that I am not shure that it is a good idea to integrate
> the lbtouch-support into the psmouse-driver since there is no real way
> of deciding if the device you are talking to is REALLY a
> lifebook-touchsreen or not.
>
I think Arjan's idea about using DMI could be very effective for this
particular touchscreen.
> I also don't think that it is useful to have two devices for the
> touchscreen. I assume you want to have one device for relative movements
> and one for the absolute ones!? But for the implementor in userspace (X,
> SDL,...) it will be easier if ALL events from this HW-device come via
> one device-node.
>
Why? These are separate, one might want to disable touchscreen and keep
quick-point or other way around.
> I think my driver works and is clean enough for integration into the
> kernel. We can talk about integrating calls to libps2 instead of doing
> the stuff myself (I am a big fan of preventing code-duplication) but
> don't you think it would be more wise to use a driver which works (since
> the very early 2.6-days) and build upon that instead of trying to
> implement your own one from the scratch and using snippets from very old
> and obsolete code?
>
We need to get it integrated right away I think. There just too much stuff
in psmouse that shoudl be used (rate and resolution exported through sysfs,
timeout and resend handling, etc.)
Plus, in your current driver:
- MODULE_PARM is obsolete
- data_lock spinlock is either unneeded or may cause deadlocks since it is
used in interrupt handler and the other places that are using it do not
disable interrupts
- it is not necessary to suppress duplicate button events in the driver
as the driver core will do that for you and it will make interrupt
handler almost the same as Vojtech's. Btw, I don't think his handler
has a problem - I would expect touchscreen to send anoter packet when
finger leaves its surface so untouch will not be lost.
- using interruptible_sleep_on for delays is somewhat inelegant I think.
- it needs some changes to compile with the latest version of serio.
--
Dmitry
On Sat, Feb 12, 2005 at 06:01:19PM +0100, Kenan Esau wrote:
> Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
> > Hi!
> >
> > I've reimplemented the Lifebook touchscreen driver using libps2 and
> > input, to make it short and fitting into the kernel drivers.
> >
> > Please comment on code and test for functionality!
> >
> > PS.: The driver should register two input devices. It doesn't yet,
> > since that isn't very straightforward in the psmouse framework.
>
> First of all it's good to see lifebook-support integrated.
>
> I have tested your driver on my box but the initialization fails. Do you
> have hardware available for testing?
No, none at all.
> As far as I can see the
> init-sequence is the one from Haralds xfree 3.3.6-driver. There is a
> reason why this sequence is not like that anymore in my driver.
OK.
> This
> sequence does not always work and there is not something like a "magic
> knock sequence".
You mean that the only needed bit is setting the resolution to '7'?
> The lifebook-touchscreen hardware is a little bit slow
> and it happens quite often that it does not understand a command that
> you send.
I don't believe this - the PS/2 protocol has handshakes for both sides,
so each side can slow down as much as it wants. PLUS, the clock is
driven by the device.
> This is the reason why you often have to send a command
> several times until the hardware understands...
> Probably this was what was seen by Harald on the USB-bus and he just
> used this sequence.
USB?!
> Second thing is that I am not shure that it is a good idea to integrate
> the lbtouch-support into the psmouse-driver since there is no real way
> of deciding if the device you are talking to is REALLY a
> lifebook-touchsreen or not.
All normal PS/2 mice will fail on the SETRES 7 command. That's an
obligation given in the spec.
> You have put the init-sequence for the hw into the
> lifebook_detect-function which is not correct since this not really a
> "detect" but a HW-init.
Since based on the result of the commands it decides whether there is
something (first command would fail), and whether it is a Fujitsu
TouchScreen (the SETRES 7 command would fail), it is a detection.
> IMHO the driver should be standalone and the user should decide which
> driver he wants to use. As default the touchscreen-functionality will be
> disabled and only the quick-point device will work like a normal
> PS2-mouse.
If it cannot be probed for, I agree.
But I still hope it can be probed for. Almost every PS/2 device has a
method. Maybe it's not known yet, maybe the SETRES 7 command is enough,
but I believe there is one.
> I also don't think that it is useful to have two devices for the
> touchscreen. I assume you want to have one device for relative movements
> and one for the absolute ones!?
Definitely.
> But for the implementor in userspace (X,
> SDL,...) it will be easier if ALL events from this HW-device come via
> one device-node.
I believe it's much easier the other way around. X now has a event-based
mouse driver, and it might be a good idea to use its options on the
touchpoint on the lifebook.
> I attached the current version of my driver here so people can also have
> a look at it. I didn't release this version on my homepage yet and the
> only difference between the released version an this one is that the
> y-axis is swapped. I did this since all input devices seem to have a
> common idea of where y=0 is and my driver was the only one where y was
> just the other way round.
I think the driver I sent has the same problem.
> One more thing I observed by inspecting your code is that your
> untouch-event will never happen.
I'll take a look.
> I think my driver works and is clean enough for integration into the
> kernel. We can talk about integrating calls to libps2 instead of doing
> the stuff myself (I am a big fan of preventing code-duplication) but
> don't you think it would be more wise to use a driver which works (since
> the very early 2.6-days) and build upon that instead of trying to
> implement your own one from the scratch and using snippets from very old
> and obsolete code?
I did the rewrite mainly because the code I had from you was doing
everything in its own way and I remember it was quite hard to
persuade back then to use the already existing helpers for that.
I also didn't at all like the 'make every little check a function of its
own' way it was done.
Similarly I don't see a reason why to filter all the touch/button data
changes in the driver, when the input layer does that for you.
If those things get fixed (which is not the case in your attached
driver), I'll gladly accept it.
But then, it'll be pretty similar to what I sent you. You can take that
as my vision of how the driver should look, even if it doesn't work as
it is.
> And as far as I can see you don't even have access to the hardware!?!?
You're right. But I simply can't have all the hardware I've written
drivers for - it wouldn't fit into my office.
But I may get some ... the lifebooks are pretty cheap these days.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Sun, Feb 13, 2005 at 01:49:10AM -0500, Dmitry Torokhov wrote:
> On Friday 11 February 2005 15:10, Vojtech Pavlik wrote:
> > +???????if (set_properties) {
> > +???????????????psmouse->vendor = "Fujitsu Lifebook";
> > +???????????????psmouse->name = "TouchScreen";
> > +???????}
> > +
> > +???????psmouse->dev.evbit[0] = BIT(EV_ABS) | BIT(EV_KEY) | BIT(EV_REL);
> > +???????psmouse->dev.keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
> > +???????psmouse->dev.keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
> > +???????psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
> > +???????input_set_abs_params(&psmouse->dev, ABS_X, 130, 885, 0, 0);
> > +???????input_set_abs_params(&psmouse->dev, ABS_Y, 272, 830, 0, 0);
> > +
> > +???????psmouse->protocol_handler = lifebook_process_byte;
> > +???????psmouse->disconnect = lifebook_disconnect;
> > +???????psmouse->reconnect = lifebook_reconnect;
> > +???????psmouse->pktsize = 3;
> >
>
> Hi Vojtech,
>
> I have been trying not alter kernel state in probe routines (still has
> to touch hardware for mice that do not have separate detection routine)
> unless set_properties is set. So all of the above should be in
> "if (set_properties)" statement.
Ok, I updated the patch.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Samstag, den 12.02.2005, 19:34 +0100 schrieb Vojtech Pavlik:
> On Sat, Feb 12, 2005 at 06:01:19PM +0100, Kenan Esau wrote:
> > Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
> > > Hi!
[...]
> > As far as I can see the
> > init-sequence is the one from Haralds xfree 3.3.6-driver. There is a
> > reason why this sequence is not like that anymore in my driver.
>
> OK.
>
> > This
> > sequence does not always work and there is not something like a "magic
> > knock sequence".
>
> You mean that the only needed bit is setting the resolution to '7'?
The lifebook touchscreen has some extensions to the standard protocol:
0xe8 0x06: Stop absolute coordinate output
0xe8 0x07: Start absolute coordinate outpout (3-byte packets)
0xe8 0x08: Start absolute coord. output with 6-byte packets
> > The lifebook-touchscreen hardware is a little bit slow
> > and it happens quite often that it does not understand a command that
> > you send.
>
> I don't believe this - the PS/2 protocol has handshakes for both sides,
> so each side can slow down as much as it wants. PLUS, the clock is
> driven by the device.
I dont't know the PS2-specs. But I know the lifebook hardware quite
well. While implementing my driver (for xfree 4.0 at that time) I
noticed that it happens often that the device came back with an error or
resend. I fixed this by just waiting a short time and then retry.
If you agree I will take your patch as the basis and make it work. Now I
know how you want it to look like.
I think this was the kick in the ass I needed ;-)
> > This is the reason why you often have to send a command
> > several times until the hardware understands...
> > Probably this was what was seen by Harald on the USB-bus and he just
> > used this sequence.
>
> USB?!
Yeah -- OK. PS2...
[...]
Greetings
Kenan
Am Samstag, den 12.02.2005, 12:46 -0500 schrieb Arjan van de Ven:
> On Sat, 2005-02-12 at 18:01 +0100, Kenan Esau wrote:
>
> > Second thing is that I am not shure that it is a good idea to integrate
> > the lbtouch-support into the psmouse-driver since there is no real way
> > of deciding if the device you are talking to is REALLY a
> > lifebook-touchsreen or not.
> ...
> > IMHO the driver should be standalone and the user should decide which
> > driver he wants to use. As default the touchscreen-functionality will be
> > disabled and only the quick-point device will work like a normal
> > PS2-mouse.
>
> I just want to point out that this is a problem for distributions and
> for not-so-technical users.
>
> Is there *really* no way to know if you're on a lifebook? Can't you use
> say the DMI identification mechanism to find this out ? If so, I think
> integrating into the regular driver very much is the right thing to
> do... it makes things JustWork(tm) for users without any need for manual
> configuration (which also makes distribution makers happy).
Yes that would be nice. But the lifebook-touchscreen hardware is also
used in other notebooks. For example the Panasonic Toughbook CF28. But
we could still use DMI to check whether we are on a lifebook b-series
and then initialize the hardware. This would still get 95% of all cases.
For all the other ones we would have to provide some kind of
force-switch.
On Sun, Feb 13, 2005 at 10:39:46AM +0100, Kenan Esau wrote:
> Am Samstag, den 12.02.2005, 12:46 -0500 schrieb Arjan van de Ven:
> > On Sat, 2005-02-12 at 18:01 +0100, Kenan Esau wrote:
> >
> > > Second thing is that I am not shure that it is a good idea to integrate
> > > the lbtouch-support into the psmouse-driver since there is no real way
> > > of deciding if the device you are talking to is REALLY a
> > > lifebook-touchsreen or not.
> > ...
> > > IMHO the driver should be standalone and the user should decide which
> > > driver he wants to use. As default the touchscreen-functionality will be
> > > disabled and only the quick-point device will work like a normal
> > > PS2-mouse.
> >
> > I just want to point out that this is a problem for distributions and
> > for not-so-technical users.
> >
> > Is there *really* no way to know if you're on a lifebook? Can't you use
> > say the DMI identification mechanism to find this out ? If so, I think
> > integrating into the regular driver very much is the right thing to
> > do... it makes things JustWork(tm) for users without any need for manual
> > configuration (which also makes distribution makers happy).
>
> Yes that would be nice. But the lifebook-touchscreen hardware is also
> used in other notebooks. For example the Panasonic Toughbook CF28. But
> we could still use DMI to check whether we are on a lifebook b-series
> and then initialize the hardware. This would still get 95% of all cases.
> For all the other ones we would have to provide some kind of
> force-switch.
Or simply another entry in the DMI table for the Toughbook, and for the
few others who use this kind of touchscreen controller.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Sun, Feb 13, 2005 at 11:05:00AM +0100, Kenan Esau wrote:
> > > This
> > > sequence does not always work and there is not something like a "magic
> > > knock sequence".
> >
> > You mean that the only needed bit is setting the resolution to '7'?
>
> The lifebook touchscreen has some extensions to the standard protocol:
>
> 0xe8 0x06: Stop absolute coordinate output
> 0xe8 0x07: Start absolute coordinate outpout (3-byte packets)
> 0xe8 0x08: Start absolute coord. output with 6-byte packets
Are the 6-byte packets carrying any more information than the 3-byte
packets do, for example pressure? Would it be useful to go for the
6-byte mode instead in the driver?
Have you tried whether the controller responds to the GETID (f2),
GETINFO (e9) and POLL (eb) commands? Maybe we could detect it that way.
Did you try to test all possible commands (from 0xe0 to 0xff) to check
what commands the lifebook hw accepts?
> > > The lifebook-touchscreen hardware is a little bit slow
> > > and it happens quite often that it does not understand a command that
> > > you send.
> >
> > I don't believe this - the PS/2 protocol has handshakes for both sides,
> > so each side can slow down as much as it wants. PLUS, the clock is
> > driven by the device.
>
> I dont't know the PS2-specs. But I know the lifebook hardware quite
> well. While implementing my driver (for xfree 4.0 at that time) I
> noticed that it happens often that the device came back with an error or
> resend. I fixed this by just waiting a short time and then retry.
serio with libps2 now is a very solid foundation that makes sure the
commands are issued and waited for correctly. It is however possible
that the lifebook hardware will still need some workarounds.
> If you agree I will take your patch as the basis and make it work. Now I
> know how you want it to look like.
That would be very much appreciated.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Sonntag, den 13.02.2005, 13:01 +0100 schrieb Vojtech Pavlik:
> On Sun, Feb 13, 2005 at 11:05:00AM +0100, Kenan Esau wrote:
>
> > > > This
> > > > sequence does not always work and there is not something like a "magic
> > > > knock sequence".
> > >
> > > You mean that the only needed bit is setting the resolution to '7'?
> >
> > The lifebook touchscreen has some extensions to the standard protocol:
> >
> > 0xe8 0x06: Stop absolute coordinate output
> > 0xe8 0x07: Start absolute coordinate outpout (3-byte packets)
> > 0xe8 0x08: Start absolute coord. output with 6-byte packets
>
> Are the 6-byte packets carrying any more information than the 3-byte
> packets do, for example pressure? Would it be useful to go for the
> 6-byte mode instead in the driver?
No the 6-byte mode does not carry any more information. Sorry but no
pressure-info...
> Have you tried whether the controller responds to the GETID (f2),
> GETINFO (e9) and POLL (eb) commands? Maybe we could detect it that way.
I have to try those commands and check the response. I know that they
are supported but I have never tried them.
[...]
> > If you agree I will take your patch as the basis and make it work. Now I
> > know how you want it to look like.
>
> That would be very much appreciated.
OK. I'll send you a new patch within the next week.
On Sun, Feb 13, 2005 at 07:14:44PM +0100, Kenan Esau wrote:
> Am Sonntag, den 13.02.2005, 13:01 +0100 schrieb Vojtech Pavlik:
> > On Sun, Feb 13, 2005 at 11:05:00AM +0100, Kenan Esau wrote:
> >
> > > > > This
> > > > > sequence does not always work and there is not something like a "magic
> > > > > knock sequence".
> > > >
> > > > You mean that the only needed bit is setting the resolution to '7'?
> > >
> > > The lifebook touchscreen has some extensions to the standard protocol:
> > >
> > > 0xe8 0x06: Stop absolute coordinate output
> > > 0xe8 0x07: Start absolute coordinate outpout (3-byte packets)
> > > 0xe8 0x08: Start absolute coord. output with 6-byte packets
> >
> > Are the 6-byte packets carrying any more information than the 3-byte
> > packets do, for example pressure? Would it be useful to go for the
> > 6-byte mode instead in the driver?
>
> No the 6-byte mode does not carry any more information. Sorry but no
> pressure-info...
I wonder what it's good for then - there must be a reason.
> > Have you tried whether the controller responds to the GETID (f2),
> > GETINFO (e9) and POLL (eb) commands? Maybe we could detect it that way.
>
> I have to try those commands and check the response. I know that they
> are supported but I have never tried them.
>
> [...]
>
> > > If you agree I will take your patch as the basis and make it work. Now I
> > > know how you want it to look like.
> >
> > That would be very much appreciated.
>
> OK. I'll send you a new patch within the next week.
Thanks.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Vojtech Pavlik wrote:
> Hi!
>
> I've reimplemented the Lifebook touchscreen driver using libps2 and
> input, to make it short and fitting into the kernel drivers.
>
> Please comment on code and test for functionality!
>
> PS.: The driver should register two input devices. It doesn't yet,
> since that isn't very straightforward in the psmouse framework.
Sorry, I have no more Lifebook available to test :-/
Have fun!
Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
> Hi!
>
> I've reimplemented the Lifebook touchscreen driver using libps2 and
> input, to make it short and fitting into the kernel drivers.
>
> Please comment on code and test for functionality!
>
> PS.: The driver should register two input devices. It doesn't yet,
> since that isn't very straightforward in the psmouse framework.
Here are my changes. I have tested everything on my lifebook B2175 and
it works fine for me. I have used DMI for probing. Does anyone have an
Idea what devices we have to add to the DMI-probing?
Please comment on the code.
On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
> Here are my changes. I have tested everything on my lifebook B2175 and
> it works fine for me. I have used DMI for probing. Does anyone have an
> Idea what devices we have to add to the DMI-probing?
>
> Please comment on the code.
> diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.c linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.c
> --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.c 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.c 2005-02-14 19:09:37.000000000 +0100
> @@ -0,0 +1,150 @@
> +/*
> + * Fujitsu B-series Lifebook PS/2 TouchScreen driver
> + *
> + * Copyright (c) 2005 Vojtech Pavlik <[email protected]>
> + *
> + * Copyright (c) 2005 Kenan Esau <[email protected]>
> + *
> + * TouchScreen detection, absolute mode setting and packet layout is taken from
> + * Harald Hoyer's description of the device.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#include <linux/input.h>
> +#include <linux/serio.h>
> +#include <linux/libps2.h>
> +#include <linux/dmi.h>
> +
> +#include "psmouse.h"
> +#include "lifebook.h"
> +
> +#define LBTOUCH_TOUCHED 0x04
> +#define LBTOUCH_X_HIGH 0x30
> +#define LBTOUCH_Y_HIGH 0xC0
> +#define LBTOUCH_LB 0x01
> +#define LBTOUCH_RB 0x02
> +
> +static int max_y = 937;
This doesn't look correct. I think the correct value here is 1024,
because that's what is the maximum possible value transfered in the
packet. With 937 you can get negative values in your code.
> +static struct dmi_system_id lifebook_dmi_table[] = {
> + {
> + .ident = "Fujitsu Siemens Lifebook B-Sereis",
> + .matches = {
> + DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
> + },
> + },
> + { }
> +};
This might be a bit too much generic. Are you sure there are no B Series
lifebooks without a touchscreen?
> +static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> +{
> + unsigned char *packet = psmouse->packet;
> + struct input_dev *dev = &psmouse->dev;
> +
> + unsigned long x = 0;
> + unsigned long y = 0;
> + static uint8_t pkt_lst_touch = 0;
> + static uint8_t pkt_cur_touch = 0;
> + uint8_t pkt_lb = packet[0] & LBTOUCH_LB;
> + uint8_t pkt_rb = packet[0] & LBTOUCH_RB;
Tab/space damage here. Do we really need constants for everything? They
don't carry any information value, because we already know what the mask
means from the left side of the assignment.
Another use for constants is where the value would possibly change,
which again isn't the case with masks.
Also, input_regs() is missing here.
> + pkt_cur_touch = packet[0] & LBTOUCH_TOUCHED;
> +
> + if ( psmouse->pktcnt != 3 )
> + return PSMOUSE_GOOD_DATA;
> +
> + /* calculate X and Y */
> + if (pkt_cur_touch) {
> + x = (packet[1] | ((packet[0] & LBTOUCH_X_HIGH) << 4 ));
> + y = max_y -
> + (packet[2] | ((packet[0] & LBTOUCH_Y_HIGH) << 2 ));
> + } else {
> + x = ((packet[0] & 0x10) ? packet[1]-256 : packet[1]);
> + y = - ((packet[0] & 0x20) ? packet[2]-256 : packet[2]);
> + }
This doesn't make sense. As far as I know, there is bit 3 in byte 0
which signifies a relative packet. We don't need to take the decision
how to interpret the axis values based on the touch bit!
> + input_report_key(dev, BTN_LEFT, pkt_lb);
> + input_report_key(dev, BTN_RIGHT, pkt_rb);
> + input_report_key(dev, BTN_TOUCH, pkt_cur_touch);
> +
> + /* currently touched */
> + if (pkt_cur_touch) {
> + input_report_abs(dev, ABS_X, x);
> + input_report_abs(dev, ABS_Y, y);
> + }
> +
> + /* quickpoint move */
> + if ( !pkt_cur_touch && !pkt_lst_touch && (x || y ) ) {
> + input_report_rel(dev, REL_X, x);
> + input_report_rel(dev, REL_Y, y);
> + }
You don't need to check for x and y being nonzero here.
This looks like a stupid workaround for not using the relative/absolute
bit I refer to above properly.
Also, you can simply merge the reporting and computing of the x/y
values, making the use of the two variables completely unnecessary.
> + input_sync(dev);
> +
> + /* save the state for the currently received packet */
> + pkt_lst_touch = pkt_cur_touch;
> +
> + return PSMOUSE_FULL_PACKET;
> +}
> +
> +static int lifebook_initialize(struct psmouse *psmouse)
> +{
> + struct ps2dev *ps2dev = &psmouse->ps2dev;
> + unsigned char param;
> +
> + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
> + return -1;
> +
> + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_BAT))
> + return -1;
Do we need two resets here? I'd expect RESET_BAT to override completely
everything.
> +
> + /*
> + Enable absolute output -- ps2_command fails always but if
> + you leave this call out the touchsreen will never send
> + absolute coordinates
> + */
> + param = 0x07;
> + ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
Have you checked whether really the touchscreen sends a 0xfe error back,
or some other value, or timeout? i8042.debug=1 is your friend here.
> + psmouse->set_rate(psmouse, psmouse->rate);
> + psmouse->set_resolution(psmouse, psmouse->resolution);
> +
> + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE))
> + return -1;
> +
> + return 0;
> +}
> +
> +static void lifebook_disconnect(struct psmouse *psmouse)
> +{
> + psmouse_reset(psmouse);
> +}
> +
> +int lifebook_detect(struct psmouse *psmouse, int set_properties)
> +{
> + if (!dmi_check_system(lifebook_dmi_table))
> + return -1;
> +
> + if (set_properties) {
> + psmouse->vendor = "Fujitsu Lifebook";
> + psmouse->name = "TouchScreen";
> + psmouse->dev.evbit[0] = BIT(EV_ABS) | BIT(EV_KEY) | BIT(EV_REL);
> + psmouse->dev.keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
> + psmouse->dev.keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
> + psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
> + input_set_abs_params(&psmouse->dev, ABS_X, 130, 885, 0, 0);
> + input_set_abs_params(&psmouse->dev, ABS_Y, 272, 830, 0, 0);
> +
> + psmouse->protocol_handler = lifebook_process_byte;
> + psmouse->disconnect = lifebook_disconnect;
> + psmouse->reconnect = lifebook_initialize;
> + psmouse->initialize = lifebook_initialize;
> + psmouse->pktsize = 3;
> + }
> +
> + return 0;
> +}
The change to the psmouse interface I'm leaving to Dmitry to comment on.
> diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.h linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.h
> --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.h 1970-01-01 01:00:00.000000000 +0100
> +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.h 2005-02-14 13:40:19.000000000 +0100
> @@ -0,0 +1,16 @@
> +/*
> + * Fujitsu B-series Lifebook PS/2 TouchScreen driver
> + *
> + * Copyright (c) 2005 Vojtech Pavlik
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef _LIFEBOOK_H
> +#define _LIFEBOOK_H
> +
> +int lifebook_detect(struct psmouse *psmouse, int set_properties);
> +
> +#endif
> diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/Makefile linux-2.6.11-rc3-kenan/drivers/input/mouse/Makefile
> --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/Makefile 2005-02-15 09:33:36.000000000 +0100
> +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/Makefile 2005-02-13 10:46:45.000000000 +0100
> @@ -14,4 +14,4 @@
> obj-$(CONFIG_MOUSE_SERIAL) += sermouse.o
> obj-$(CONFIG_MOUSE_VSXXXAA) += vsxxxaa.o
>
> -psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o
> +psmouse-objs := psmouse-base.o alps.o logips2pp.o synaptics.o lifebook.o
> diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/psmouse-base.c linux-2.6.11-rc3-kenan/drivers/input/mouse/psmouse-base.c
> --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/psmouse-base.c 2005-02-15 09:33:36.000000000 +0100
> +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/psmouse-base.c 2005-02-15 08:54:45.000000000 +0100
> @@ -24,6 +24,7 @@
> #include "synaptics.h"
> #include "logips2pp.h"
> #include "alps.h"
> +#include "lifebook.h"
>
> #define DRIVER_DESC "PS/2 mouse driver"
>
> @@ -62,7 +63,7 @@
> __obsolete_setup("psmouse_resetafter=");
> __obsolete_setup("psmouse_rate=");
>
> -static char *psmouse_protocols[] = { "None", "PS/2", "PS2++", "ThinkPS/2", "GenPS/2", "ImPS/2", "ImExPS/2", "SynPS/2", "AlpsPS/2" };
> +static char *psmouse_protocols[] = { "None", "PS/2", "PS2++", "ThinkPS/2", "GenPS/2", "ImPS/2", "ImExPS/2", "SynPS/2", "AlpsPS/2", "LBPS/2" };
>
> /*
> * psmouse_process_byte() analyzes the PS/2 data stream and reports
> @@ -463,6 +464,9 @@
> }
> }
>
> + if (max_proto > PSMOUSE_IMEX && lifebook_detect(psmouse, set_properties) == 0)
> + return PSMOUSE_LIFEBOOK;
> +
> if (max_proto > PSMOUSE_IMEX && genius_detect(psmouse, set_properties) == 0)
> return PSMOUSE_GENPS;
>
> @@ -565,13 +569,13 @@
> * psmouse_initialize() initializes the mouse to a sane state.
> */
>
> -static void psmouse_initialize(struct psmouse *psmouse)
> +static int psmouse_initialize(struct psmouse *psmouse)
> {
> /*
> * We set the mouse into streaming mode.
> */
> -
> - ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSTREAM);
> + if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSTREAM))
> + return -1;
>
> /*
> * We set the mouse report rate, resolution and scaling.
> @@ -580,8 +584,11 @@
> if (psmouse_max_proto != PSMOUSE_PS2) {
> psmouse->set_rate(psmouse, psmouse->rate);
> psmouse->set_resolution(psmouse, psmouse->resolution);
> - ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11);
> + if (ps2_command(&psmouse->ps2dev, NULL, PSMOUSE_CMD_SETSCALE11))
> + return -1;
> }
> +
> + return 0;
> }
>
> /*
> @@ -719,6 +726,7 @@
> }
>
> psmouse->rate = psmouse_rate;
> + psmouse->initialize = psmouse_initialize;
> psmouse->resolution = psmouse_resolution;
> psmouse->resetafter = psmouse_resetafter;
> psmouse->smartscroll = psmouse_smartscroll;
> @@ -747,7 +755,8 @@
>
> psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
>
> - psmouse_initialize(psmouse);
> + if (psmouse->initialize(psmouse))
> + printk(KERN_ERR"input: %s initialization failed\n", psmouse->devname);
>
> if (parent && parent->pt_activate)
> parent->pt_activate(parent);
> @@ -804,7 +813,8 @@
> */
> psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
>
> - psmouse_initialize(psmouse);
> + if (psmouse->initialize(psmouse))
> + printk(KERN_ERR"input: %s initialization failed\n", psmouse->devname);
>
> if (parent && parent->pt_activate)
> parent->pt_activate(parent);
> diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/psmouse.h linux-2.6.11-rc3-kenan/drivers/input/mouse/psmouse.h
> --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/psmouse.h 2005-02-15 09:33:36.000000000 +0100
> +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/psmouse.h 2005-02-14 11:44:10.000000000 +0100
> @@ -60,6 +60,7 @@
> void (*set_rate)(struct psmouse *psmouse, unsigned int rate);
> void (*set_resolution)(struct psmouse *psmouse, unsigned int resolution);
>
> + int (*initialize)(struct psmouse *psmouse);
> int (*reconnect)(struct psmouse *psmouse);
> void (*disconnect)(struct psmouse *psmouse);
>
> @@ -77,6 +78,7 @@
> PSMOUSE_IMEX,
> PSMOUSE_SYNAPTICS,
> PSMOUSE_ALPS,
> + PSMOUSE_LIFEBOOK,
> };
>
> int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command);
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Tue, 15 Feb 2005 14:43:08 +0100, Vojtech Pavlik <[email protected]> wrote:
> On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> > +static struct dmi_system_id lifebook_dmi_table[] = {
> > + {
> > + .ident = "Fujitsu Siemens Lifebook B-Sereis",
> > + .matches = {
> > + DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
> > + },
> > + },
> > + { }
> > +};
>
> This might be a bit too much generic. Are you sure there are no B Series
> lifebooks without a touchscreen?
>
And another concern: does this notebook (or others using this
touchscreen) have an active MUX? We don't want to force LBTOUCH
protocol on an external mouse.
> > +static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> > +{
> > + unsigned char *packet = psmouse->packet;
> > + struct input_dev *dev = &psmouse->dev;
> > +
> > + unsigned long x = 0;
> > + unsigned long y = 0;
> > + static uint8_t pkt_lst_touch = 0;
> > + static uint8_t pkt_cur_touch = 0;
> > + uint8_t pkt_lb = packet[0] & LBTOUCH_LB;
> > + uint8_t pkt_rb = packet[0] & LBTOUCH_RB;
We usually don't use userspace types here. unsigned char or u8 for kernel.
> > +
> > + psmouse->protocol_handler = lifebook_process_byte;
> > + psmouse->disconnect = lifebook_disconnect;
> > + psmouse->reconnect = lifebook_initialize;
> > + psmouse->initialize = lifebook_initialize;
> > + psmouse->pktsize = 3;
> > + }
> > +
> > + return 0;
> > +}
>
> The change to the psmouse interface I'm leaving to Dmitry to comment on.
I don't think that we need a separate initialize handler simply
because it is called just once, at initialization time. Here we know
exactly what device (protocol) we are dealing with, no need for
indirection.
--
Dmitry
Am Dienstag, den 15.02.2005, 09:43 -0500 schrieb Dmitry Torokhov:
> On Tue, 15 Feb 2005 14:43:08 +0100, Vojtech Pavlik <[email protected]> wrote:
> > On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> > > +static struct dmi_system_id lifebook_dmi_table[] = {
> > > + {
> > > + .ident = "Fujitsu Siemens Lifebook B-Sereis",
> > > + .matches = {
> > > + DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
> > > + },
> > > + },
> > > + { }
> > > +};
> >
> > This might be a bit too much generic. Are you sure there are no B Series
> > lifebooks without a touchscreen?
> >
>
> And another concern: does this notebook (or others using this
> touchscreen) have an active MUX? We don't want to force LBTOUCH
> protocol on an external mouse.
All B-Series Lifebooks have the same touchscreen-hardware. But Dmitri's
concern is correct -- at the moment I would enforce the LBTOUCH-protocol
on external mice...
I have to fix this. I will additionally to the DMI stuff use "Status
Request". On a "Request ID"-Command the Device always answers with a
0x00 -- could this also be helpfull?
> > > +static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> > > +{
> > > + unsigned char *packet = psmouse->packet;
> > > + struct input_dev *dev = &psmouse->dev;
> > > +
> > > + unsigned long x = 0;
> > > + unsigned long y = 0;
> > > + static uint8_t pkt_lst_touch = 0;
> > > + static uint8_t pkt_cur_touch = 0;
> > > + uint8_t pkt_lb = packet[0] & LBTOUCH_LB;
> > > + uint8_t pkt_rb = packet[0] & LBTOUCH_RB;
>
> We usually don't use userspace types here. unsigned char or u8 for kernel.
>
> > > +
> > > + psmouse->protocol_handler = lifebook_process_byte;
> > > + psmouse->disconnect = lifebook_disconnect;
> > > + psmouse->reconnect = lifebook_initialize;
> > > + psmouse->initialize = lifebook_initialize;
> > > + psmouse->pktsize = 3;
> > > + }
> > > +
> > > + return 0;
> > > +}
> >
> > The change to the psmouse interface I'm leaving to Dmitry to comment on.
>
> I don't think that we need a separate initialize handler simply
> because it is called just once, at initialization time. Here we know
> exactly what device (protocol) we are dealing with, no need for
> indirection.
I introduced the new initialize-handler since psmouse->initialize() is
also used in psmouse-base.c. This is to prevent putting if-statements on
each place where the initialization-function for a certain protocol is
called in psmouse-base.c.
I admit since I am also using a different reconnect-function than
psmouse-base it's not such a huge benefit but think of a new
protocol/device which uses the same reconnect-function as psmouse-base
but a different init-function.
My goal was to have no dependency from psmouse-base to the
lifebook-handling (none but lifebook_detect()). Thus the indirection is
IMHO needed.
On Tue, Feb 15, 2005 at 06:03:28PM +0100, Kenan Esau wrote:
> All B-Series Lifebooks have the same touchscreen-hardware. But Dmitri's
> concern is correct -- at the moment I would enforce the LBTOUCH-protocol
> on external mice...
>
> I have to fix this. I will additionally to the DMI stuff use "Status
> Request". On a "Request ID"-Command the Device always answers with a
> 0x00 -- could this also be helpfull?
No. That's the standard response for common PS/2 mice.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Dienstag, den 15.02.2005, 14:43 +0100 schrieb Vojtech Pavlik:
> On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> > Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
>
> > Here are my changes. I have tested everything on my lifebook B2175 and
> > it works fine for me. I have used DMI for probing. Does anyone have an
> > Idea what devices we have to add to the DMI-probing?
> >
> > Please comment on the code.
>
> > diff -Naur -X dontdiff linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.c linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.c
> > --- linux-2.6.11-rc3-vanilla/drivers/input/mouse/lifebook.c 1970-01-01 01:00:00.000000000 +0100
> > +++ linux-2.6.11-rc3-kenan/drivers/input/mouse/lifebook.c 2005-02-14 19:09:37.000000000 +0100
> > @@ -0,0 +1,150 @@
> > +/*
> > + * Fujitsu B-series Lifebook PS/2 TouchScreen driver
> > + *
> > + * Copyright (c) 2005 Vojtech Pavlik <[email protected]>
> > + *
> > + * Copyright (c) 2005 Kenan Esau <[email protected]>
> > + *
> > + * TouchScreen detection, absolute mode setting and packet layout is taken from
> > + * Harald Hoyer's description of the device.
> > + *
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms of the GNU General Public License version 2 as published by
> > + * the Free Software Foundation.
> > + */
> > +
> > +#include <linux/input.h>
> > +#include <linux/serio.h>
> > +#include <linux/libps2.h>
> > +#include <linux/dmi.h>
> > +
> > +#include "psmouse.h"
> > +#include "lifebook.h"
> > +
> > +#define LBTOUCH_TOUCHED 0x04
> > +#define LBTOUCH_X_HIGH 0x30
> > +#define LBTOUCH_Y_HIGH 0xC0
> > +#define LBTOUCH_LB 0x01
> > +#define LBTOUCH_RB 0x02
> > +
> > +static int max_y = 937;
>
> This doesn't look correct. I think the correct value here is 1024,
> because that's what is the maximum possible value transfered in the
> packet. With 937 you can get negative values in your code.
Since the input_event-structure takes signed values that does not really
matter. But you are right it looks a little bit strange and I will
change it to 1024. It's 937 at the moment since this is the "ideal"
value for my touchscreen where y_max=937. ;-)
> > +static struct dmi_system_id lifebook_dmi_table[] = {
> > + {
> > + .ident = "Fujitsu Siemens Lifebook B-Sereis",
> > + .matches = {
> > + DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
> > + },
> > + },
> > + { }
> > +};
>
> This might be a bit too much generic. Are you sure there are no B Series
> lifebooks without a touchscreen?
>
> > +static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> > +{
> > + unsigned char *packet = psmouse->packet;
> > + struct input_dev *dev = &psmouse->dev;
> > +
> > + unsigned long x = 0;
> > + unsigned long y = 0;
> > + static uint8_t pkt_lst_touch = 0;
> > + static uint8_t pkt_cur_touch = 0;
> > + uint8_t pkt_lb = packet[0] & LBTOUCH_LB;
> > + uint8_t pkt_rb = packet[0] & LBTOUCH_RB;
>
> Tab/space damage here. Do we really need constants for everything? They
> don't carry any information value, because we already know what the mask
> means from the left side of the assignment.
>
> Another use for constants is where the value would possibly change,
> which again isn't the case with masks.
I put the constants there since I think it is more readable but if you
don't like them I'll throw em out.
> Also, input_regs() is missing here.
>
> > + pkt_cur_touch = packet[0] & LBTOUCH_TOUCHED;
> > +
> > + if ( psmouse->pktcnt != 3 )
> > + return PSMOUSE_GOOD_DATA;
> > +
> > + /* calculate X and Y */
> > + if (pkt_cur_touch) {
> > + x = (packet[1] | ((packet[0] & LBTOUCH_X_HIGH) << 4 ));
> > + y = max_y -
> > + (packet[2] | ((packet[0] & LBTOUCH_Y_HIGH) << 2 ));
> > + } else {
> > + x = ((packet[0] & 0x10) ? packet[1]-256 : packet[1]);
> > + y = - ((packet[0] & 0x20) ? packet[2]-256 : packet[2]);
> > + }
>
> This doesn't make sense. As far as I know, there is bit 3 in byte 0
> which signifies a relative packet. We don't need to take the decision
> how to interpret the axis values based on the touch bit!
I will check this.
>
> > + input_report_key(dev, BTN_LEFT, pkt_lb);
> > + input_report_key(dev, BTN_RIGHT, pkt_rb);
> > + input_report_key(dev, BTN_TOUCH, pkt_cur_touch);
> > +
> > + /* currently touched */
> > + if (pkt_cur_touch) {
> > + input_report_abs(dev, ABS_X, x);
> > + input_report_abs(dev, ABS_Y, y);
> > + }
> > +
> > + /* quickpoint move */
> > + if ( !pkt_cur_touch && !pkt_lst_touch && (x || y ) ) {
> > + input_report_rel(dev, REL_X, x);
> > + input_report_rel(dev, REL_Y, y);
> > + }
>
> You don't need to check for x and y being nonzero here.
>
> This looks like a stupid workaround for not using the relative/absolute
> bit I refer to above properly.
>
> Also, you can simply merge the reporting and computing of the x/y
> values, making the use of the two variables completely unnecessary.
OK
>
> > + input_sync(dev);
> > +
> > + /* save the state for the currently received packet */
> > + pkt_lst_touch = pkt_cur_touch;
> > +
> > + return PSMOUSE_FULL_PACKET;
> > +}
> > +
> > +static int lifebook_initialize(struct psmouse *psmouse)
> > +{
> > + struct ps2dev *ps2dev = &psmouse->ps2dev;
> > + unsigned char param;
> > +
> > + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
> > + return -1;
> > +
> > + if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_BAT))
> > + return -1;
>
> Do we need two resets here? I'd expect RESET_BAT to override completely
> everything.
It's quite a while ago when I developed the init-sequence of the
touchscreen. But if I remember correctly it needed the DISABLE and the
RESET_BAT.
>
> > +
> > + /*
> > + Enable absolute output -- ps2_command fails always but if
> > + you leave this call out the touchsreen will never send
> > + absolute coordinates
> > + */
> > + param = 0x07;
> > + ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
>
> Have you checked whether really the touchscreen sends a 0xfe error back,
> or some other value, or timeout? i8042.debug=1 is your friend here.
OK -- that's a good hint
[...]
On Tue, 15 Feb 2005 18:03:28 +0100, Kenan Esau <[email protected]> wrote:
> Am Dienstag, den 15.02.2005, 09:43 -0500 schrieb Dmitry Torokhov:
> > On Tue, 15 Feb 2005 14:43:08 +0100, Vojtech Pavlik <[email protected]> wrote:
> > > On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> > > > +static struct dmi_system_id lifebook_dmi_table[] = {
> > > > + {
> > > > + .ident = "Fujitsu Siemens Lifebook B-Sereis",
> > > > + .matches = {
> > > > + DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
> > > > + },
> > > > + },
> > > > + { }
> > > > +};
> > >
> > > This might be a bit too much generic. Are you sure there are no B Series
> > > lifebooks without a touchscreen?
> > >
> >
> > And another concern: does this notebook (or others using this
> > touchscreen) have an active MUX? We don't want to force LBTOUCH
> > protocol on an external mouse.
>
> All B-Series Lifebooks have the same touchscreen-hardware. But Dmitri's
> concern is correct -- at the moment I would enforce the LBTOUCH-protocol
> on external mice...
>
> I have to fix this. I will additionally to the DMI stuff use "Status
> Request". On a "Request ID"-Command the Device always answers with a
> 0x00 -- could this also be helpfull?
>
> > > > +static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
> > > > +{
> > > > + unsigned char *packet = psmouse->packet;
> > > > + struct input_dev *dev = &psmouse->dev;
> > > > +
> > > > + unsigned long x = 0;
> > > > + unsigned long y = 0;
> > > > + static uint8_t pkt_lst_touch = 0;
> > > > + static uint8_t pkt_cur_touch = 0;
> > > > + uint8_t pkt_lb = packet[0] & LBTOUCH_LB;
> > > > + uint8_t pkt_rb = packet[0] & LBTOUCH_RB;
> >
> > We usually don't use userspace types here. unsigned char or u8 for kernel.
> >
> > > > +
> > > > + psmouse->protocol_handler = lifebook_process_byte;
> > > > + psmouse->disconnect = lifebook_disconnect;
> > > > + psmouse->reconnect = lifebook_initialize;
> > > > + psmouse->initialize = lifebook_initialize;
> > > > + psmouse->pktsize = 3;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +}
> > >
> > > The change to the psmouse interface I'm leaving to Dmitry to comment on.
> >
> > I don't think that we need a separate initialize handler simply
> > because it is called just once, at initialization time. Here we know
> > exactly what device (protocol) we are dealing with, no need for
> > indirection.
>
> I introduced the new initialize-handler since psmouse->initialize() is
> also used in psmouse-base.c. This is to prevent putting if-statements on
> each place where the initialization-function for a certain protocol is
> called in psmouse-base.c.
It would be good idea if protocols were initialized in many difefrent
places, but the all are called from one place - psmouse_extensions.
Some protocols that can be detected without changing hardware state
have 2 functions - detect and init and the others have ony detect
which does initialization as well. But they all are called from the
same place.
psmouse_initialize has somewhat confising name, it is not "initialize
protocol", it is "enable streaming and initialize common parameters,
such as rate and resolution". Plus I think it is too late to do
protocol init in psmouse_initialize as this will not allow falling
back to "lesser" protocols if higher prtocol initialization fails.
> I admit since I am also using a different reconnect-function than
> psmouse-base it's not such a huge benefit but think of a new
> protocol/device which uses the same reconnect-function as psmouse-base
> but a different init-function.
I am not sure what difference does it make - if there is no reconnect
hamdler psmosue will end up calling psmouse_extensions which will
re-initialize hardware with proper function for the protocol. It still
does not require a handler in psmouse structure.
>
> My goal was to have no dependency from psmouse-base to the
> lifebook-handling (none but lifebook_detect()). Thus the indirection is
> IMHO needed.
You can hide all of it right in lifebook_detect or acknowledge that
you have 2 fucntions and call them from psmouse-base, like synaptics
and alps do. I don't think it exposes lbtouch internals too much.
--
Dmitry
Am Dienstag, den 15.02.2005, 14:43 +0100 schrieb Vojtech Pavlik:
> On Tue, Feb 15, 2005 at 09:57:59AM +0100, Kenan Esau wrote:
> > Am Freitag, den 11.02.2005, 21:10 +0100 schrieb Vojtech Pavlik:
[...]
> > +
> > + /*
> > + Enable absolute output -- ps2_command fails always but if
> > + you leave this call out the touchsreen will never send
> > + absolute coordinates
> > + */
> > + param = 0x07;
> > + ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
>
> Have you checked whether really the touchscreen sends a 0xfe error back,
> or some other value, or timeout? i8042.debug=1 is your friend here.
Yes the answer is 0xfe.
On Wed, Feb 16, 2005 at 07:34:52PM +0100, Kenan Esau wrote:
> > > +
> > > + /*
> > > + Enable absolute output -- ps2_command fails always but if
> > > + you leave this call out the touchsreen will never send
> > > + absolute coordinates
> > > + */
> > > + param = 0x07;
> > > + ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
> >
> > Have you checked whether really the touchscreen sends a 0xfe error back,
> > or some other value, or timeout? i8042.debug=1 is your friend here.
>
> Yes the answer is 0xfe.
Would you be so kind to post the 'dmesg' log?
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Mittwoch, den 16.02.2005, 22:35 +0100 schrieb Vojtech Pavlik:
> On Wed, Feb 16, 2005 at 07:34:52PM +0100, Kenan Esau wrote:
>
> > > > +
> > > > + /*
> > > > + Enable absolute output -- ps2_command fails always but if
> > > > + you leave this call out the touchsreen will never send
> > > > + absolute coordinates
> > > > + */
> > > > + param = 0x07;
> > > > + ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
> > >
> > > Have you checked whether really the touchscreen sends a 0xfe error back,
> > > or some other value, or timeout? i8042.debug=1 is your friend here.
> >
> > Yes the answer is 0xfe.
>
> Would you be so kind to post the 'dmesg' log?
Shure -- here you are...
...
input: LBPS/2 Fujitsu Lifebook TouchScreen on isa0060/serio1
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78488524]
drivers/input/serio/i8042.c: f5 -> i8042 (parameter) [78488524]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78488532]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78488753]
drivers/input/serio/i8042.c: ff -> i8042 (parameter) [78488753]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78488759]
drivers/input/serio/i8042.c: aa <- i8042 (interrupt, aux, 12) [78488822]
drivers/input/serio/i8042.c: 00 <- i8042 (interrupt, aux, 12) [78488823]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489004]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [78489004]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489009]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489009]
drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [78489009]
drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12) [78489014]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489014]
drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [78489014]
drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12) [78489018]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489216]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [78489216]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489218]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489219]
drivers/input/serio/i8042.c: 03 -> i8042 (parameter) [78489219]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489223]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489223]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [78489223]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489228]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489229]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [78489229]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489233]
drivers/input/serio/i8042.c: 20 <- i8042 (interrupt, kbd, 1) [78494505]
drivers/input/serio/i8042.c: a0 <- i8042 (interrupt, kbd, 1) [78494603]
drivers/input/serio/i8042.c: 32 <- i8042 (interrupt, kbd, 1) [78494680]
...
On Thu, Feb 17, 2005 at 03:19:53PM +0100, Kenan Esau wrote:
> > Would you be so kind to post the 'dmesg' log?
>
> Shure -- here you are...
>
> ...
> input: LBPS/2 Fujitsu Lifebook TouchScreen on isa0060/serio1
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78488524]
> drivers/input/serio/i8042.c: f5 -> i8042 (parameter) [78488524]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78488532]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78488753]
> drivers/input/serio/i8042.c: ff -> i8042 (parameter) [78488753]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78488759]
> drivers/input/serio/i8042.c: aa <- i8042 (interrupt, aux, 12) [78488822]
> drivers/input/serio/i8042.c: 00 <- i8042 (interrupt, aux, 12) [78488823]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489004]
> drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [78489004]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489009]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489009]
> drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [78489009]
> drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12) [78489014]
Ok, this is a regular 'I don't know what you mean' response from the
pad.
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489014]
> drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [78489014]
> drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12) [78489018]
But this return code is _very_ unusual. 0xfc means 'basic assurance test
failure' and should be reported only as a response to the 0xff command.
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489216]
> drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [78489216]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489218]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489219]
> drivers/input/serio/i8042.c: 03 -> i8042 (parameter) [78489219]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489223]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489223]
> drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [78489223]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489228]
> drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489229]
> drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [78489229]
> drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12) [78489233]
Nothing else important here.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Thu, Feb 17, 2005 at 04:04:55PM +0100, Vojtech Pavlik wrote:
> > drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489009]
> > drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [78489009]
> > drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12) [78489014]
>
> Ok, this is a regular 'I don't know what you mean' response from the
> pad.
>
> > drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489014]
> > drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [78489014]
> > drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12) [78489018]
>
> But this return code is _very_ unusual. 0xfc means 'basic assurance test
> failure' and should be reported only as a response to the 0xff command.
Kenan, can you check whether the 0xfc response is there even if you
don't do the setres 7 command before this one?
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Donnerstag, den 17.02.2005, 20:42 +0100 schrieb Vojtech Pavlik:
> On Thu, Feb 17, 2005 at 04:04:55PM +0100, Vojtech Pavlik wrote:
>
> > > drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489009]
> > > drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [78489009]
> > > drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12) [78489014]
> >
> > Ok, this is a regular 'I don't know what you mean' response from the
> > pad.
> >
> > > drivers/input/serio/i8042.c: d4 -> i8042 (command) [78489014]
> > > drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [78489014]
> > > drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12) [78489018]
> >
> > But this return code is _very_ unusual. 0xfc means 'basic assurance test
> > failure' and should be reported only as a response to the 0xff command.
>
> Kenan, can you check whether the 0xfc response is there even if you
> don't do the setres 7 command before this one?
Yes OK -- I will check. But as far as I know the 0xfe-answer from the
touchscreen means: "Please resend the last command". And 0xfc means:
"Error I didn't get that".
I also checked my original standalone-driver: Because of this behaviour
I always retried the last command 3 times if the responce from the
device was 0xfe or 0xfc.
On Sat, Feb 19, 2005 at 01:54:41PM +0100, Kenan Esau wrote:
> > > But this return code is _very_ unusual. 0xfc means 'basic assurance test
> > > failure' and should be reported only as a response to the 0xff command.
> >
> > Kenan, can you check whether the 0xfc response is there even if you
> > don't do the setres 7 command before this one?
>
> Yes OK -- I will check. But as far as I know the 0xfe-answer from the
> touchscreen means: "Please resend the last command".
In theory, it should mean that even with the PS/2 spec. But it only
works so in the other direction - when the PC doesn't get the bytes
right.
The devices use this error code for any problem, and this of course
would lead to infinite loops if the system did always resend.
> And 0xfc means:
> "Error I didn't get that".
Why wouldn't it?
> I also checked my original standalone-driver: Because of this behaviour
> I always retried the last command 3 times if the responce from the
> device was 0xfe or 0xfc.
And did it actually help? Did the touchscreen ever respond with a 0xfa
"ACK, OK" response to these commands?
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Samstag, den 19.02.2005, 14:16 +0100 schrieb Vojtech Pavlik:
[...]
>
> > I also checked my original standalone-driver: Because of this behaviour
> > I always retried the last command 3 times if the responce from the
> > device was 0xfe or 0xfc.
>
> And did it actually help? Did the touchscreen ever respond with a 0xfa
> "ACK, OK" response to these commands?
I played around a little bit last weekend. And obviously the touchscreen
always responds 0xfe to the 0xe8 0x07 command. Also repeating the
command does not really help. After the firxt 0x07 you get back the 0xfe
and the next byte you send to the device is ALWAYS answered by a
0xfc!?!?
At the end of this mail you'll find some traces I did.
I also wonder if it is possible at all to probe this device. I think
not. IMHO we should go for a module-parameter which enforces the
lifebook-protokoll. Something like "force_lb=1". Any Ideas /
suggestions? How does this work out with a second/external mouse?
##################################################
Without e8 07
input: LBPS/2 Fujitsu Lifebook TouchScreen on isa0060/serio1
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082637]
drivers/input/serio/i8042.c: f5 -> i8042 (parameter) [195082637]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082645]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082804]
drivers/input/serio/i8042.c: ff -> i8042 (parameter) [195082804]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082807]
drivers/input/serio/i8042.c: aa <- i8042 (interrupt, aux, 12)[195082869]
drivers/input/serio/i8042.c: 00 <- i8042 (interrupt, aux, 12)[195082871]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082871]
drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [195082871]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082874]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082874]
drivers/input/serio/i8042.c: 64 -> i8042 (parameter) [195082874]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082878]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082879]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [195082879]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082884]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082884]
drivers/input/serio/i8042.c: 03 -> i8042 (parameter) [195082884]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082888]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082889]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [195082889]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082894]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195082894]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [195082894]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195082899]
Repeating: e8 07 e8 07
input: LBPS/2 Fujitsu Lifebook TouchScreen on isa0060/serio1
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781502]
drivers/input/serio/i8042.c: f5 -> i8042 (parameter) [195781502]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781506]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781506]
drivers/input/serio/i8042.c: ff -> i8042 (parameter) [195781506]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781511]
drivers/input/serio/i8042.c: 2a <- i8042 (interrupt, kbd, 1) [195781569]
drivers/input/serio/i8042.c: aa <- i8042 (interrupt, aux, 12)[195781575]
drivers/input/serio/i8042.c: 00 <- i8042 (interrupt, aux, 12)[195781577]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781595]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [195781595]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781601]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781602]
drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [195781602]
drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12)[195781606]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781607]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [195781607]
drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12)[195781611]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781809]
drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [195781809]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781813]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781813]
drivers/input/serio/i8042.c: 64 -> i8042 (parameter) [195781813]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781817]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781818]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [195781818]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781822]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781823]
drivers/input/serio/i8042.c: 03 -> i8042 (parameter) [195781823]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781827]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781828]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [195781828]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[195781832]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [195781833]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [195781833]
Repeating: e8 07 e8 07 e8 07
input: LBPS/2 Fujitsu Lifebook TouchScreen on isa0060/serio1
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875239]
drivers/input/serio/i8042.c: f5 -> i8042 (parameter) [196875239]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875241]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875242]
drivers/input/serio/i8042.c: ff -> i8042 (parameter) [196875242]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875246]
drivers/input/serio/i8042.c: aa <- i8042 (interrupt, aux, 12)[196875311]
drivers/input/serio/i8042.c: 00 <- i8042 (interrupt, aux, 12)[196875312]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875319]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [196875319]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875327]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875327]
drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [196875327]
drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12)[196875332]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875332]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [196875332]
drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12)[196875336]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875534]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [196875534]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875538]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875538]
drivers/input/serio/i8042.c: 07 -> i8042 (parameter) [196875538]
drivers/input/serio/i8042.c: fe <- i8042 (interrupt, aux, 12)[196875543]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875543]
drivers/input/serio/i8042.c: f3 -> i8042 (parameter) [196875543]
drivers/input/serio/i8042.c: fc <- i8042 (interrupt, aux, 12)[196875547]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875746]
drivers/input/serio/i8042.c: e8 -> i8042 (parameter) [196875746]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875754]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875755]
drivers/input/serio/i8042.c: 03 -> i8042 (parameter) [196875755]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875759]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875759]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [196875759]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875764]
drivers/input/serio/i8042.c: d4 -> i8042 (command) [196875765]
drivers/input/serio/i8042.c: f4 -> i8042 (parameter) [196875765]
drivers/input/serio/i8042.c: fa <- i8042 (interrupt, aux, 12)[196875770]
On Mon, Feb 21, 2005 at 09:06:55AM +0100, Kenan Esau wrote:
> > > I also checked my original standalone-driver: Because of this behaviour
> > > I always retried the last command 3 times if the responce from the
> > > device was 0xfe or 0xfc.
> >
> > And did it actually help? Did the touchscreen ever respond with a 0xfa
> > "ACK, OK" response to these commands?
>
> I played around a little bit last weekend. And obviously the touchscreen
> always responds 0xfe to the 0xe8 0x07 command. Also repeating the
> command does not really help. After the firxt 0x07 you get back the 0xfe
> and the next byte you send to the device is ALWAYS answered by a
> 0xfc!?!?
This looks like it either expects some other data (like a second
parameter to the command?) or just wants the 0x07 again (and not the
whole command) to make sure you really mean it.
Could you try sending 0xe8 0x07 0x07?
> At the end of this mail you'll find some traces I did.
>
> I also wonder if it is possible at all to probe this device. I think
> not. IMHO we should go for a module-parameter which enforces the
> lifebook-protokoll. Something like "force_lb=1". Any Ideas /
> suggestions?
I'd suggest using psmouse.proto=lifebook
> How does this work out with a second/external mouse?
The external mouse has to be in bare PS/2 mode anyway, so we don't need
to care.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Am Donnerstag, den 24.02.2005, 10:03 +0100 schrieb Vojtech Pavlik:
> On Mon, Feb 21, 2005 at 09:06:55AM +0100, Kenan Esau wrote:
>
> > > > I also checked my original standalone-driver: Because of this behaviour
> > > > I always retried the last command 3 times if the responce from the
> > > > device was 0xfe or 0xfc.
> > >
> > > And did it actually help? Did the touchscreen ever respond with a 0xfa
> > > "ACK, OK" response to these commands?
> >
> > I played around a little bit last weekend. And obviously the touchscreen
> > always responds 0xfe to the 0xe8 0x07 command. Also repeating the
> > command does not really help. After the firxt 0x07 you get back the 0xfe
> > and the next byte you send to the device is ALWAYS answered by a
> > 0xfc!?!?
>
> This looks like it either expects some other data (like a second
> parameter to the command?) or just wants the 0x07 again (and not the
> whole command) to make sure you really mean it.
>
> Could you try sending 0xe8 0x07 0x07?
My old driver did that. But with the same result. It doesn't seem to
matter what the first and the second bytes are -- the answers from the
device are alway the same.
> > At the end of this mail you'll find some traces I did.
> >
> > I also wonder if it is possible at all to probe this device. I think
> > not. IMHO we should go for a module-parameter which enforces the
> > lifebook-protokoll. Something like "force_lb=1". Any Ideas /
> > suggestions?
>
> I'd suggest using psmouse.proto=lifebook
The current patch has implemented it that way. But the meaning is a
little bit different. With proto=lifebook you ENFORCE the lifebook
protocol. As far as I read the meaning of the other ones this does not
really enforce these protocols.
> > How does this work out with a second/external mouse?
>
> The external mouse has to be in bare PS/2 mode anyway, so we don't need
> to care.
Why that?
I have attached the newest version of the patch.
On Tue, Mar 01, 2005 at 09:11:49AM +0100, Kenan Esau wrote:
> > This looks like it either expects some other data (like a second
> > parameter to the command?) or just wants the 0x07 again (and not the
> > whole command) to make sure you really mean it.
> >
> > Could you try sending 0xe8 0x07 0x07?
>
> My old driver did that. But with the same result. It doesn't seem to
> matter what the first and the second bytes are -- the answers from the
> device are alway the same.
So even 0xe8 0x03 returns error?
Maybe we should send a command after this (any command), to make sure
the
psmouse->set_rate(psmouse, psmouse->rate);
call succeeds and is not confused by the 0xfc response.
> > > At the end of this mail you'll find some traces I did.
> > >
> > > I also wonder if it is possible at all to probe this device. I think
> > > not. IMHO we should go for a module-parameter which enforces the
> > > lifebook-protokoll. Something like "force_lb=1". Any Ideas /
> > > suggestions?
> >
> > I'd suggest using psmouse.proto=lifebook
>
> The current patch has implemented it that way. But the meaning is a
> little bit different. With proto=lifebook you ENFORCE the lifebook
> protocol. As far as I read the meaning of the other ones this does not
> really enforce these protocols.
That's OK. I'd like to keep the DMI probing as well, though, so it's not
absolutely necessary to provide the parameter.
> > > How does this work out with a second/external mouse?
> >
> > The external mouse has to be in bare PS/2 mode anyway, so we don't need
> > to care.
>
> Why that?
Can you send any commands to the external mouse? How the touchscreen
reacts when the mouse starts sending 4-byte responses? We process the
external mouse packets inside lifebook.c anyway and we don't have any
support for the enhanced protocols there.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Sorry for the late response.
Am Dienstag, den 01.03.2005, 13:08 +0100 schrieb Vojtech Pavlik:
> On Tue, Mar 01, 2005 at 09:11:49AM +0100, Kenan Esau wrote:
>
> > > This looks like it either expects some other data (like a second
> > > parameter to the command?) or just wants the 0x07 again (and not the
> > > whole command) to make sure you really mean it.
> > >
> > > Could you try sending 0xe8 0x07 0x07?
> >
> > My old driver did that. But with the same result. It doesn't seem to
> > matter what the first and the second bytes are -- the answers from the
> > device are alway the same.
>
> So even 0xe8 0x03 returns error?
No -- I meant only 0xe8 0x07 and 0xe8 0x06 . For those it doesn't matter
if you repeat the parameter or send something else. The answers from the
device for those command/parameters are always the same.
> Maybe we should send a command after this (any command), to make sure
> the
>
> psmouse->set_rate(psmouse, psmouse->rate);
>
> call succeeds and is not confused by the 0xfc response.
OK -- I will send the command after 0xe8 0x07 twice.
> > > > At the end of this mail you'll find some traces I did.
> > > >
> > > > I also wonder if it is possible at all to probe this device. I think
> > > > not. IMHO we should go for a module-parameter which enforces the
> > > > lifebook-protokoll. Something like "force_lb=1". Any Ideas /
> > > > suggestions?
> > >
> > > I'd suggest using psmouse.proto=lifebook
> >
> > The current patch has implemented it that way. But the meaning is a
> > little bit different. With proto=lifebook you ENFORCE the lifebook
> > protocol. As far as I read the meaning of the other ones this does not
> > really enforce these protocols.
>
> That's OK. I'd like to keep the DMI probing as well, though, so it's not
> absolutely necessary to provide the parameter.
You mean if the device is in the appropriate DMI-database use the
lifebook protocol and if the parameter is provided use it also (although
it might not be in the DMI database)?
> > > > How does this work out with a second/external mouse?
> > >
> > > The external mouse has to be in bare PS/2 mode anyway, so we don't need
> > > to care.
> >
> > Why that?
>
> Can you send any commands to the external mouse? How the touchscreen
> reacts when the mouse starts sending 4-byte responses?
No idea yet -- I will test this.
> We process the
> external mouse packets inside lifebook.c anyway and we don't have any
> support for the enhanced protocols there.
Ah OK.
I personally never used an external mouse. But last weekend I played
around a little bit and recognized that there are some BIOS-settings
which control the behavior of the touchscreen, quickpoint-device and
external mouse. I have to play around with those a little bit more. But
as far as I can see you can never have all three at the same time.
On Mon, Mar 07, 2005 at 08:27:16AM +0100, Kenan Esau wrote:
> > > > > At the end of this mail you'll find some traces I did.
> > > > >
> > > > > I also wonder if it is possible at all to probe this device. I think
> > > > > not. IMHO we should go for a module-parameter which enforces the
> > > > > lifebook-protokoll. Something like "force_lb=1". Any Ideas /
> > > > > suggestions?
> > > >
> > > > I'd suggest using psmouse.proto=lifebook
> > >
> > > The current patch has implemented it that way. But the meaning is a
> > > little bit different. With proto=lifebook you ENFORCE the lifebook
> > > protocol. As far as I read the meaning of the other ones this does not
> > > really enforce these protocols.
> >
> > That's OK. I'd like to keep the DMI probing as well, though, so it's not
> > absolutely necessary to provide the parameter.
>
> You mean if the device is in the appropriate DMI-database use the
> lifebook protocol and if the parameter is provided use it also (although
> it might not be in the DMI database)?
Yes.
> > > > > How does this work out with a second/external mouse?
> > > >
> > > > The external mouse has to be in bare PS/2 mode anyway, so we don't need
> > > > to care.
> > >
> > > Why that?
> >
> > Can you send any commands to the external mouse? How the touchscreen
> > reacts when the mouse starts sending 4-byte responses?
>
> No idea yet -- I will test this.
OK.
> > We process the external mouse packets inside lifebook.c anyway and
> > we don't have any support for the enhanced protocols there.
>
> Ah OK.
However, if it was possible to forward commands to the external mouse,
we could implement a full passthrough like synaptics.c has, and then
support any kind of external mouse (including wheels, bells and
whistles.)
> I personally never used an external mouse. But last weekend I played
> around a little bit and recognized that there are some BIOS-settings
> which control the behavior of the touchscreen, quickpoint-device and
> external mouse. I have to play around with those a little bit more. But
> as far as I can see you can never have all three at the same time.
That's probably because the quickpoint is connected in place of the
external mouse.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
Here is a new version of the patch:
- minimal changes
- reintroduced DMI-probing
I had a look at the synaptic-sources to see how the pass-through-mode is
implemented. We'll see if something similar to this also works with the
lifebook.
I received a request from a user who has a Panasonic CF-29. It also
seems to have the same Touchscreen hardware like the lifebook. But it
doesn't seem to work as expected. Can someone get hold of a CF-29 and
test the psmouse-patch with it?
On Tue, Mar 15, 2005 at 02:25:42PM +0100, Kenan Esau wrote:
> Here is a new version of the patch:
> - minimal changes
> - reintroduced DMI-probing
>
> I had a look at the synaptic-sources to see how the pass-through-mode is
> implemented. We'll see if something similar to this also works with the
> lifebook.
Thanks, I applied this version of the patch to my tree. It'll appear in
next -mm, and in 2.6.13.
> I received a request from a user who has a Panasonic CF-29. It also
> seems to have the same Touchscreen hardware like the lifebook. But it
> doesn't seem to work as expected. Can someone get hold of a CF-29 and
> test the psmouse-patch with it?
No, I don't have any ToughBooks nearby.
--
Vojtech Pavlik
SuSE Labs, SuSE CR
On Mon, 21 Mar 2005 13:44:07 +0100, Vojtech Pavlik <[email protected]> wrote:
> On Tue, Mar 15, 2005 at 02:25:42PM +0100, Kenan Esau wrote:
> > Here is a new version of the patch:
> > - minimal changes
> > - reintroduced DMI-probing
> >
> > I had a look at the synaptic-sources to see how the pass-through-mode is
> > implemented. We'll see if something similar to this also works with the
> > lifebook.
>
> Thanks, I applied this version of the patch to my tree. It'll appear in
> next -mm, and in 2.6.13.
>
There are couple of things that I an concerned with:
1. I don't like that it overrides meaning of max_proto parameter to be
exactly the protocol specified. However, if you take my psmouse
protocol switching through sysfs patch we can drop that change and
require that non auto-detectable protocols be activated through sysfs
after loading the driver.
2. It looks like it bypasses rate and resolution setting in
psmouse_initialize. What was the reason for it? Does setting rate or
resolution disturbs lifebook mode? If so the driver has to implement
it's own set_rate and set_resolution handlers so when one tries to
change rate from userspace (through sysfs) the request would be
ignored.
--
Dmitry
Am Montag, den 21.03.2005, 09:52 -0500 schrieb Dmitry Torokhov:
> On Mon, 21 Mar 2005 13:44:07 +0100, Vojtech Pavlik <[email protected]> wrote:
> > On Tue, Mar 15, 2005 at 02:25:42PM +0100, Kenan Esau wrote:
> > > Here is a new version of the patch:
> > > - minimal changes
> > > - reintroduced DMI-probing
> > >
> > > I had a look at the synaptic-sources to see how the pass-through-mode is
> > > implemented. We'll see if something similar to this also works with the
> > > lifebook.
> >
> > Thanks, I applied this version of the patch to my tree. It'll appear in
> > next -mm, and in 2.6.13.
> >
>
> There are couple of things that I an concerned with:
>
> 1. I don't like that it overrides meaning of max_proto parameter to be
> exactly the protocol specified.
Yeah -- I agree. I also don't like that double-meaning. That was the
reason why I originally proposed the use of a new parameter...
> However, if you take my psmouse
> protocol switching through sysfs patch we can drop that change and
> require that non auto-detectable protocols be activated through sysfs
> after loading the driver.
I think that would also be a good solution.
> 2. It looks like it bypasses rate and resolution setting in
> psmouse_initialize. What was the reason for it? Does setting rate or
> resolution disturbs lifebook mode? If so the driver has to implement
> it's own set_rate and set_resolution handlers so when one tries to
> change rate from userspace (through sysfs) the request would be
> ignored.
IMHO it does not make sense to call psmouse_initialize() although it
does not disturb lifebook-mode. But setting resolution is already done
during lifebook_initialize(). And there psmouse->set_resolution() and
psmouse->set_rate() are used so setting resolution and rate should also
work via sysfs.
My stomach feels strange when I call lifebook_initialize() and after
that call an xy_initialize() for some other protocol although it might
not disturb the lifebook-mode.
On Mon, 21 Mar 2005 16:31:08 +0100, Kenan Esau <[email protected]> wrote:
> Am Montag, den 21.03.2005, 09:52 -0500 schrieb Dmitry Torokhov:
> > On Mon, 21 Mar 2005 13:44:07 +0100, Vojtech Pavlik <[email protected]> wrote:
> > > On Tue, Mar 15, 2005 at 02:25:42PM +0100, Kenan Esau wrote:
> > > > Here is a new version of the patch:
> > > > - minimal changes
> > > > - reintroduced DMI-probing
> > > >
> > > > I had a look at the synaptic-sources to see how the pass-through-mode is
> > > > implemented. We'll see if something similar to this also works with the
> > > > lifebook.
> > >
> > > Thanks, I applied this version of the patch to my tree. It'll appear in
> > > next -mm, and in 2.6.13.
> > >
> >
> > There are couple of things that I an concerned with:
> >
> > 1. I don't like that it overrides meaning of max_proto parameter to be
> > exactly the protocol specified.
>
> Yeah -- I agree. I also don't like that double-meaning. That was the
> reason why I originally proposed the use of a new parameter...
>
> > However, if you take my psmouse
> > protocol switching through sysfs patch we can drop that change and
> > require that non auto-detectable protocols be activated through sysfs
> > after loading the driver.
>
> I think that would also be a good solution.
>
> > 2. It looks like it bypasses rate and resolution setting in
> > psmouse_initialize. What was the reason for it? Does setting rate or
> > resolution disturbs lifebook mode? If so the driver has to implement
> > it's own set_rate and set_resolution handlers so when one tries to
> > change rate from userspace (through sysfs) the request would be
> > ignored.
>
> IMHO it does not make sense to call psmouse_initialize() although it
> does not disturb lifebook-mode. But setting resolution is already done
> during lifebook_initialize(). And there psmouse->set_resolution() and
> psmouse->set_rate() are used so setting resolution and rate should also
> work via sysfs.
>
Oh, I see. But I think that setting rate and resolution as well as
enabling the device should be removed from lifebook_initialize.
*_initialize() functions should only be used to set up corresponding
protocol, the reset of the initializationis done by psmouse-base
independent of the protocol used.
--
Dmitry
On Monday 21 March 2005 10:31, Kenan Esau wrote:
> Am Montag, den 21.03.2005, 09:52 -0500 schrieb Dmitry Torokhov:
> >
> > There are couple of things that I an concerned with:
> >
> > 1. I don't like that it overrides meaning of max_proto parameter to be
> > exactly the protocol specified.
>
> Yeah -- I agree. I also don't like that double-meaning. That was the
> reason why I originally proposed the use of a new parameter...
>
Ok, I have some patches to lifebook that I would like to included (if
they work):
1. lifebook-dmi-x86-only - do not compile in DMI detection on anything
but x86.
2. lifebook-cleanup - do not set rate/resolution nor enable the device
in lifebook initialization routines, let psmouse code do it for us.
3. lifebook-init - rearrange initialization code to be more like other
protocols to ease dynamic protocol switching.
4. psmouse-proto-attr - expose protocol as a sysfs attribute and allow
changing it from userspace.
Please give it a try and let me know if it does/does not work.
Thanks!
--
Dmitry
===================================================================
Input: lifebook - DMI facility is only available on i386, do not
attempt to compile on anything else.
Signed-off-by: Dmitry Torokhov <[email protected]>
lifebook.c | 16 +++++++++++-----
1 files changed, 11 insertions(+), 5 deletions(-)
Index: dtor/drivers/input/mouse/lifebook.c
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.c
+++ dtor/drivers/input/mouse/lifebook.c
@@ -15,17 +15,17 @@
#include <linux/input.h>
#include <linux/serio.h>
#include <linux/libps2.h>
-#include <linux/dmi.h>
#include "psmouse.h"
#include "lifebook.h"
static int max_y = 1024;
-
+#if defined(__i386__)
+#include <linux/dmi.h>
static struct dmi_system_id lifebook_dmi_table[] = {
{
- .ident = "Fujitsu Siemens Lifebook B-Sereis",
+ .ident = "Lifebook",
.matches = {
DMI_MATCH(DMI_PRODUCT_NAME, "LIFEBOOK B Series"),
},
@@ -33,6 +33,13 @@ static struct dmi_system_id lifebook_dmi
{ }
};
+static inline int lifebook_check_dmi(void)
+{
+ return dmi_check_system(lifebook_dmi_table) ? 0 : -1;
+}
+#else
+static inline int lifebook_check_dmi(void) { return -1; }
+#endif
static psmouse_ret_t lifebook_process_byte(struct psmouse *psmouse, struct pt_regs *regs)
{
@@ -102,8 +109,7 @@ static void lifebook_disconnect(struct p
int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
int set_properties)
{
- if (!dmi_check_system(lifebook_dmi_table) &&
- (max_proto != PSMOUSE_LIFEBOOK) )
+ if (lifebook_check_dmi() && max_proto != PSMOUSE_LIFEBOOK)
return -1;
if (set_properties) {
===================================================================
Input: lifebook - various cleanups:
- do not try to set rate and resolution in init method, let
psmouse core do it for us. This also removes special quirks
from the core;
- do not disable mouse before doing full reset - meaningless;
- some formatting and whitespace cleanups.
Signed-off-by: Dmitry Torokhov <[email protected]>
lifebook.c | 31 ++++++++++---------------------
lifebook.h | 2 +-
psmouse-base.c | 2 --
3 files changed, 11 insertions(+), 24 deletions(-)
Index: dtor/drivers/input/mouse/lifebook.h
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.h
+++ dtor/drivers/input/mouse/lifebook.h
@@ -11,7 +11,7 @@
#ifndef _LIFEBOOK_H
#define _LIFEBOOK_H
-int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
+int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
int set_properties);
#endif
Index: dtor/drivers/input/mouse/psmouse-base.c
===================================================================
--- dtor.orig/drivers/input/mouse/psmouse-base.c
+++ dtor/drivers/input/mouse/psmouse-base.c
@@ -579,8 +579,6 @@ static void psmouse_set_rate(struct psmo
static void psmouse_initialize(struct psmouse *psmouse)
{
- if (psmouse->type==PSMOUSE_LIFEBOOK)
- return;
/*
* We set the mouse into streaming mode.
*/
Index: dtor/drivers/input/mouse/lifebook.c
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.c
+++ dtor/drivers/input/mouse/lifebook.c
@@ -19,8 +19,6 @@
#include "psmouse.h"
#include "lifebook.h"
-static int max_y = 1024;
-
#if defined(__i386__)
#include <linux/dmi.h>
static struct dmi_system_id lifebook_dmi_table[] = {
@@ -46,7 +44,7 @@ static psmouse_ret_t lifebook_process_by
unsigned char *packet = psmouse->packet;
struct input_dev *dev = &psmouse->dev;
- if ( psmouse->pktcnt != 3 )
+ if (psmouse->pktcnt != 3)
return PSMOUSE_GOOD_DATA;
input_regs(dev, regs);
@@ -56,12 +54,12 @@ static psmouse_ret_t lifebook_process_by
input_report_abs(dev, ABS_X,
(packet[1] | ((packet[0] & 0x30) << 4)));
input_report_abs(dev, ABS_Y,
- max_y - (packet[2] | ((packet[0] & 0xC0) << 2)));
+ 1024 - (packet[2] | ((packet[0] & 0xC0) << 2)));
} else {
- input_report_rel(dev, REL_X,
- ((packet[0] & 0x10) ? packet[1]-256 : packet[1]));
- input_report_rel(dev, REL_Y,
- (- (int)((packet[0] & 0x20) ? packet[2]-256 : packet[2])));
+ input_report_rel(dev, REL_X,
+ ((packet[0] & 0x10) ? packet[1] - 256 : packet[1]));
+ input_report_rel(dev, REL_Y,
+ -(int)((packet[0] & 0x20) ? packet[2] - 256 : packet[2]));
}
input_report_key(dev, BTN_LEFT, packet[0] & 0x01);
@@ -78,26 +76,17 @@ static int lifebook_initialize(struct ps
struct ps2dev *ps2dev = &psmouse->ps2dev;
unsigned char param;
- if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_DISABLE))
- return -1;
-
- if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_RESET_BAT))
+ if (psmouse_reset(psmouse))
return -1;
- /*
+ /*
Enable absolute output -- ps2_command fails always but if
you leave this call out the touchsreen will never send
absolute coordinates
- */
+ */
param = 0x07;
ps2_command(ps2dev, ¶m, PSMOUSE_CMD_SETRES);
- psmouse->set_rate(psmouse, psmouse->rate);
- psmouse->set_resolution(psmouse, psmouse->resolution);
-
- if (ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE))
- return -1;
-
return 0;
}
@@ -106,7 +95,7 @@ static void lifebook_disconnect(struct p
psmouse_reset(psmouse);
}
-int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
+int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
int set_properties)
{
if (lifebook_check_dmi() && max_proto != PSMOUSE_LIFEBOOK)
===================================================================
Input: lifebook - adjust initialization routines to be in line with
the rest of protocols in preparation to dynamic protocol
switching.
Signed-off-by: Dmitry Torokhov <[email protected]>
lifebook.c | 46 ++++++++++++++++++++++++++++------------------
lifebook.h | 4 ++--
psmouse-base.c | 14 ++++++++++++--
3 files changed, 42 insertions(+), 22 deletions(-)
Index: dtor/drivers/input/mouse/lifebook.h
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.h
+++ dtor/drivers/input/mouse/lifebook.h
@@ -11,7 +11,7 @@
#ifndef _LIFEBOOK_H
#define _LIFEBOOK_H
-int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
- int set_properties);
+int lifebook_detect(struct psmouse *psmouse, int set_properties);
+int lifebook_init(struct psmouse *psmouse);
#endif
Index: dtor/drivers/input/mouse/psmouse-base.c
===================================================================
--- dtor.orig/drivers/input/mouse/psmouse-base.c
+++ dtor/drivers/input/mouse/psmouse-base.c
@@ -424,8 +424,18 @@ static int psmouse_extensions(struct psm
{
int synaptics_hardware = 0;
- if (lifebook_detect(psmouse, max_proto, set_properties) == 0)
- return PSMOUSE_LIFEBOOK;
+/*
+ * We always check for lifebook because it does not disturb mouse
+ * (it only checks DMI information).
+ */
+ if (lifebook_detect(psmouse, set_properties) == 0 ||
+ max_proto == PSMOUSE_LIFEBOOK) {
+
+ if (max_proto > PSMOUSE_IMEX) {
+ if (!set_properties || lifebook_init(psmouse) == 0)
+ return PSMOUSE_LIFEBOOK;
+ }
+ }
/*
* Try Kensington ThinkingMouse (we try first, because synaptics probe
Index: dtor/drivers/input/mouse/lifebook.c
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.c
+++ dtor/drivers/input/mouse/lifebook.c
@@ -71,7 +71,7 @@ static psmouse_ret_t lifebook_process_by
return PSMOUSE_FULL_PACKET;
}
-static int lifebook_initialize(struct psmouse *psmouse)
+static int lifebook_absolute_mode(struct psmouse *psmouse)
{
struct ps2dev *ps2dev = &psmouse->ps2dev;
unsigned char param;
@@ -95,27 +95,37 @@ static void lifebook_disconnect(struct p
psmouse_reset(psmouse);
}
-int lifebook_detect(struct psmouse *psmouse, unsigned int max_proto,
- int set_properties)
+int lifebook_detect(struct psmouse *psmouse, int set_properties)
{
- if (lifebook_check_dmi() && max_proto != PSMOUSE_LIFEBOOK)
+ if (lifebook_check_dmi())
return -1;
if (set_properties) {
- psmouse->vendor = "Fujitsu Lifebook";
- psmouse->name = "TouchScreen";
- psmouse->dev.evbit[0] = BIT(EV_ABS) | BIT(EV_KEY) | BIT(EV_REL);
- psmouse->dev.keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
- psmouse->dev.keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
- psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
- input_set_abs_params(&psmouse->dev, ABS_X, 0, 1024, 0, 0);
- input_set_abs_params(&psmouse->dev, ABS_Y, 0, 1024, 0, 0);
-
- psmouse->protocol_handler = lifebook_process_byte;
- psmouse->disconnect = lifebook_disconnect;
- psmouse->reconnect = lifebook_initialize;
- psmouse->pktsize = 3;
+ psmouse->vendor = "Fujitsu";
+ psmouse->name = "Lifebook TouchScreen";
+
}
- return lifebook_initialize(psmouse);
+ return 0;
}
+
+int lifebook_init(struct psmouse *psmouse)
+{
+ if (lifebook_absolute_mode(psmouse))
+ return -1;
+
+ psmouse->dev.evbit[0] = BIT(EV_ABS) | BIT(EV_KEY) | BIT(EV_REL);
+ psmouse->dev.keybit[LONG(BTN_LEFT)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
+ psmouse->dev.keybit[LONG(BTN_TOUCH)] = BIT(BTN_TOUCH);
+ psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
+ input_set_abs_params(&psmouse->dev, ABS_X, 0, 1024, 0, 0);
+ input_set_abs_params(&psmouse->dev, ABS_Y, 0, 1024, 0, 0);
+
+ psmouse->protocol_handler = lifebook_process_byte;
+ psmouse->disconnect = lifebook_disconnect;
+ psmouse->reconnect = lifebook_absolute_mode;
+ psmouse->pktsize = 3;
+
+ return 0;
+}
+
===================================================================
Input: psmouse - export protocol as a sysfs per-device attribute.
Signed-off-by: Dmitry Torokhov <[email protected]>
drivers/input/mouse/psmouse-base.c | 291 ++++++++++++++++++++++++++++++-------
drivers/input/mouse/psmouse.h | 1
drivers/input/serio/serio.c | 44 +++++
include/linux/serio.h | 6
4 files changed, 290 insertions(+), 52 deletions(-)
Index: dtor/drivers/input/serio/serio.c
===================================================================
--- dtor.orig/drivers/input/serio/serio.c
+++ dtor/drivers/input/serio/serio.c
@@ -43,6 +43,7 @@ MODULE_LICENSE("GPL");
EXPORT_SYMBOL(serio_interrupt);
EXPORT_SYMBOL(__serio_register_port);
EXPORT_SYMBOL(serio_unregister_port);
+EXPORT_SYMBOL(serio_unregister_child_port);
EXPORT_SYMBOL(__serio_unregister_port_delayed);
EXPORT_SYMBOL(__serio_register_driver);
EXPORT_SYMBOL(serio_unregister_driver);
@@ -90,12 +91,19 @@ static void serio_bind_driver(struct ser
down_write(&serio_bus.subsys.rwsem);
if (serio_match_port(drv->id_table, serio)) {
+ /* make sure parent is not about to change */
+ if (serio->parent)
+ down(&serio->parent->drv_sem);
+
serio->dev.driver = &drv->driver;
if (drv->connect(serio, drv)) {
serio->dev.driver = NULL;
goto out;
}
device_bind_driver(&serio->dev);
+
+ if (serio->parent)
+ up(&serio->parent->drv_sem);
}
out:
up_write(&serio_bus.subsys.rwsem);
@@ -150,12 +158,12 @@ static void serio_queue_event(void *obje
spin_lock_irqsave(&serio_event_lock, flags);
/*
- * Scan event list for the other events for the same serio port,
+ * Scan event list for the other events for the same serio port,
* starting with the most recent one. If event is the same we
* do not need add new one. If event is of different type we
* need to add this event and should not look further because
* we need to preseve sequence of distinct events.
- */
+ */
list_for_each_entry_reverse(event, &serio_event_list, node) {
if (event->object == object) {
if (event->type == event_type)
@@ -614,6 +622,19 @@ void serio_unregister_port(struct serio
}
/*
+ * Safely unregisters child port if one is present.
+ */
+void serio_unregister_child_port(struct serio *serio)
+{
+ down(&serio_sem);
+ if (serio->child) {
+ serio_disconnect_port(serio->child);
+ serio_destroy_port(serio->child);
+ }
+ up(&serio_sem);
+}
+
+/*
* Submits register request to kseriod for subsequent execution.
* Can be used when it is not obvious whether the serio_sem is
* taken or not and when delayed execution is feasible.
@@ -669,8 +690,18 @@ static int serio_driver_probe(struct dev
{
struct serio *serio = to_serio_port(dev);
struct serio_driver *drv = to_serio_driver(dev->driver);
+ int result;
+
+ /* make sure parent is not about to change */
+ if (serio->parent)
+ down(&serio->parent->drv_sem);
+
+ result = drv->connect(serio, drv);
+
+ if (serio->parent)
+ up(&serio->parent->drv_sem);
- return drv->connect(serio, drv);
+ return result;
}
static int serio_driver_remove(struct device *dev)
@@ -678,7 +709,14 @@ static int serio_driver_remove(struct de
struct serio *serio = to_serio_port(dev);
struct serio_driver *drv = to_serio_driver(dev->driver);
+ if (serio->parent)
+ down(&serio->parent->drv_sem);
+
drv->disconnect(serio);
+
+ if (serio->parent)
+ up(&serio->parent->drv_sem);
+
return 0;
}
Index: dtor/drivers/input/mouse/psmouse.h
===================================================================
--- dtor.orig/drivers/input/mouse/psmouse.h
+++ dtor/drivers/input/mouse/psmouse.h
@@ -78,6 +78,7 @@ enum psmouse_type {
PSMOUSE_SYNAPTICS,
PSMOUSE_ALPS,
PSMOUSE_LIFEBOOK,
+ PSMOUSE_AUTO /* This one should always be last */
};
int psmouse_sliced_command(struct psmouse *psmouse, unsigned char command);
Index: dtor/include/linux/serio.h
===================================================================
--- dtor.orig/include/linux/serio.h
+++ dtor/include/linux/serio.h
@@ -83,6 +83,7 @@ static inline void serio_register_port(s
}
void serio_unregister_port(struct serio *serio);
+void serio_unregister_child_port(struct serio *serio);
void __serio_unregister_port_delayed(struct serio *serio, struct module *owner);
static inline void serio_unregister_port_delayed(struct serio *serio)
{
@@ -153,6 +154,11 @@ static inline int serio_pin_driver(struc
return down_interruptible(&serio->drv_sem);
}
+static inline void serio_pin_driver_uninterruptible(struct serio *serio)
+{
+ down(&serio->drv_sem);
+}
+
static inline void serio_unpin_driver(struct serio *serio)
{
up(&serio->drv_sem);
Index: dtor/drivers/input/mouse/psmouse-base.c
===================================================================
--- dtor.orig/drivers/input/mouse/psmouse-base.c
+++ dtor/drivers/input/mouse/psmouse-base.c
@@ -32,15 +32,14 @@ MODULE_AUTHOR("Vojtech Pavlik <vojtech@s
MODULE_DESCRIPTION(DRIVER_DESC);
MODULE_LICENSE("GPL");
-static unsigned int psmouse_max_proto = -1U;
+static unsigned int psmouse_max_proto = PSMOUSE_AUTO;
static int psmouse_set_maxproto(const char *val, struct kernel_param *kp);
static int psmouse_get_maxproto(char *buffer, struct kernel_param *kp);
-static char *psmouse_proto_abbrev[] = { NULL, "bare", NULL, NULL, NULL, "imps", "exps", NULL, NULL, "lifebook" };
#define param_check_proto_abbrev(name, p) __param_check(name, p, unsigned int)
#define param_set_proto_abbrev psmouse_set_maxproto
#define param_get_proto_abbrev psmouse_get_maxproto
module_param_named(proto, psmouse_max_proto, proto_abbrev, 0644);
-MODULE_PARM_DESC(proto, "Highest protocol extension to probe (bare, imps, exps, lifebook, any). Useful for KVM switches.");
+MODULE_PARM_DESC(proto, "Highest protocol extension to probe (bare, imps, exps, any). Useful for KVM switches.");
static unsigned int psmouse_resolution = 200;
module_param_named(resolution, psmouse_resolution, uint, 0644);
@@ -58,6 +57,7 @@ static unsigned int psmouse_resetafter;
module_param_named(resetafter, psmouse_resetafter, uint, 0644);
MODULE_PARM_DESC(resetafter, "Reset device after so many bad packets (0 = never).");
+PSMOUSE_DEFINE_ATTR(protocol);
PSMOUSE_DEFINE_ATTR(rate);
PSMOUSE_DEFINE_ATTR(resolution);
PSMOUSE_DEFINE_ATTR(resetafter);
@@ -68,7 +68,14 @@ __obsolete_setup("psmouse_smartscroll=")
__obsolete_setup("psmouse_resetafter=");
__obsolete_setup("psmouse_rate=");
-static char *psmouse_protocols[] = { "None", "PS/2", "PS2++", "ThinkPS/2", "GenPS/2", "ImPS/2", "ImExPS/2", "SynPS/2", "AlpsPS/2", "LBPS/2" };
+struct psmouse_protocol {
+ enum psmouse_type type;
+ char *name;
+ char *alias;
+ int maxproto;
+ int (*detect)(struct psmouse *, int);
+ int (*init)(struct psmouse *);
+};
/*
* psmouse_process_byte() analyzes the PS/2 data stream and reports
@@ -408,12 +415,15 @@ static int thinking_detect(struct psmous
*/
static int ps2bare_detect(struct psmouse *psmouse, int set_properties)
{
- if (!psmouse->vendor) psmouse->vendor = "Generic";
- if (!psmouse->name) psmouse->name = "Mouse";
+ if (set_properties) {
+ if (!psmouse->vendor) psmouse->vendor = "Generic";
+ if (!psmouse->name) psmouse->name = "Mouse";
+ }
return 0;
}
+
/*
* psmouse_extensions() probes for any extensions to the basic PS/2 protocol
* the mouse may have.
@@ -428,9 +438,7 @@ static int psmouse_extensions(struct psm
* We always check for lifebook because it does not disturb mouse
* (it only checks DMI information).
*/
- if (lifebook_detect(psmouse, set_properties) == 0 ||
- max_proto == PSMOUSE_LIFEBOOK) {
-
+ if (lifebook_detect(psmouse, set_properties) == 0) {
if (max_proto > PSMOUSE_IMEX) {
if (!set_properties || lifebook_init(psmouse) == 0)
return PSMOUSE_LIFEBOOK;
@@ -520,6 +528,103 @@ static int psmouse_extensions(struct psm
return PSMOUSE_PS2;
}
+static struct psmouse_protocol psmouse_protocols[] = {
+ {
+ .type = PSMOUSE_PS2,
+ .name = "PS/2",
+ .alias = "bare",
+ .maxproto = 1,
+ .detect = ps2bare_detect,
+ },
+ {
+ .type = PSMOUSE_PS2PP,
+ .name = "PS2++",
+ .alias = "logitech",
+ .detect = ps2pp_init,
+ },
+ {
+ .type = PSMOUSE_THINKPS,
+ .name = "ThinkPS/2",
+ .alias = "thinkps",
+ .detect = thinking_detect,
+ },
+ {
+ .type = PSMOUSE_GENPS,
+ .name = "GenPS/2",
+ .alias = "genius",
+ .detect = genius_detect,
+ },
+ {
+ .type = PSMOUSE_IMPS,
+ .name = "ImPS/2",
+ .alias = "imps",
+ .maxproto = 1,
+ .detect = intellimouse_detect,
+ },
+ {
+ .type = PSMOUSE_IMEX,
+ .name = "ImExPS/2",
+ .alias = "exps",
+ .maxproto = 1,
+ .detect = im_explorer_detect,
+ },
+ {
+ .type = PSMOUSE_SYNAPTICS,
+ .name = "SynPS/2",
+ .alias = "synaptics",
+ .detect = synaptics_detect,
+ .init = synaptics_init,
+ },
+ {
+ .type = PSMOUSE_ALPS,
+ .name = "AlpsPS/2",
+ .alias = "alps",
+ .detect = alps_detect,
+ .init = alps_init,
+ },
+ {
+ .type = PSMOUSE_LIFEBOOK,
+ .name = "LBPS/2",
+ .alias = "lifebook",
+ .init = lifebook_init,
+ },
+ {
+ .type = PSMOUSE_AUTO,
+ .name = "auto",
+ .alias = "any",
+ .maxproto = 1,
+ },
+};
+
+static struct psmouse_protocol *psmouse_protocol_by_type(enum psmouse_type type)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++)
+ if (psmouse_protocols[i].type == type)
+ return &psmouse_protocols[i];
+
+ WARN_ON(1);
+ return &psmouse_protocols[0];
+}
+
+static struct psmouse_protocol *psmouse_protocol_by_name(const char *name, size_t len)
+{
+ struct psmouse_protocol *p;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(psmouse_protocols); i++) {
+ p = &psmouse_protocols[i];
+
+ if ((strlen(p->name) == len && !strncmp(p->name, name, len)) ||
+ (strlen(p->alias) == len && !strncmp(p->alias, name, len)))
+ return &psmouse_protocols[i];
+ }
+
+ return NULL;
+}
+
+
/*
* psmouse_probe() probes for a PS/2 mouse.
*/
@@ -669,6 +774,7 @@ static void psmouse_disconnect(struct se
{
struct psmouse *psmouse, *parent;
+ device_remove_file(&serio->dev, &psmouse_attr_protocol);
device_remove_file(&serio->dev, &psmouse_attr_rate);
device_remove_file(&serio->dev, &psmouse_attr_resolution);
device_remove_file(&serio->dev, &psmouse_attr_resetafter);
@@ -693,6 +799,49 @@ static void psmouse_disconnect(struct se
kfree(psmouse);
}
+static int psmouse_switch_protocol(struct psmouse *psmouse, struct psmouse_protocol *proto)
+{
+ memset(&psmouse->dev, 0, sizeof(struct input_dev));
+
+ init_input_dev(&psmouse->dev);
+
+ psmouse->dev.private = psmouse;
+ psmouse->dev.dev = &psmouse->ps2dev.serio->dev;
+
+ psmouse->dev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
+ psmouse->dev.keybit[LONG(BTN_MOUSE)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
+ psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
+
+ psmouse->set_rate = psmouse_set_rate;
+ psmouse->set_resolution = psmouse_set_resolution;
+ psmouse->protocol_handler = psmouse_process_byte;
+ psmouse->pktsize = 3;
+
+ if (proto && (proto->detect || proto->init)) {
+ if (proto->detect && proto->detect(psmouse, 1) < 0)
+ return -1;
+
+ if (proto->init && proto->init(psmouse) < 0)
+ return -1;
+
+ psmouse->type = proto->type;
+ }
+ else
+ psmouse->type = psmouse_extensions(psmouse, psmouse_max_proto, 1);
+
+ sprintf(psmouse->devname, "%s %s %s",
+ psmouse_protocol_by_type(psmouse->type)->name, psmouse->vendor, psmouse->name);
+
+ psmouse->dev.name = psmouse->devname;
+ psmouse->dev.phys = psmouse->phys;
+ psmouse->dev.id.bustype = BUS_I8042;
+ psmouse->dev.id.vendor = 0x0002;
+ psmouse->dev.id.product = psmouse->type;
+ psmouse->dev.id.version = psmouse->model;
+
+ return 0;
+}
+
/*
* psmouse_connect() is a callback from the serio module when
* an unhandled serio port is found.
@@ -720,11 +869,7 @@ static int psmouse_connect(struct serio
ps2_init(&psmouse->ps2dev, serio);
sprintf(psmouse->phys, "%s/input0", serio->phys);
- psmouse->dev.evbit[0] = BIT(EV_KEY) | BIT(EV_REL);
- psmouse->dev.keybit[LONG(BTN_MOUSE)] = BIT(BTN_LEFT) | BIT(BTN_MIDDLE) | BIT(BTN_RIGHT);
- psmouse->dev.relbit[0] = BIT(REL_X) | BIT(REL_Y);
- psmouse->dev.private = psmouse;
- psmouse->dev.dev = &serio->dev;
+
psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
serio_set_drvdata(serio, psmouse);
@@ -748,25 +893,10 @@ static int psmouse_connect(struct serio
psmouse->resolution = psmouse_resolution;
psmouse->resetafter = psmouse_resetafter;
psmouse->smartscroll = psmouse_smartscroll;
- psmouse->set_rate = psmouse_set_rate;
- psmouse->set_resolution = psmouse_set_resolution;
- psmouse->protocol_handler = psmouse_process_byte;
- psmouse->pktsize = 3;
- psmouse->type = psmouse_extensions(psmouse, psmouse_max_proto, 1);
-
- sprintf(psmouse->devname, "%s %s %s",
- psmouse_protocols[psmouse->type], psmouse->vendor, psmouse->name);
-
- psmouse->dev.name = psmouse->devname;
- psmouse->dev.phys = psmouse->phys;
- psmouse->dev.id.bustype = BUS_I8042;
- psmouse->dev.id.vendor = 0x0002;
- psmouse->dev.id.product = psmouse->type;
- psmouse->dev.id.version = psmouse->model;
+ psmouse_switch_protocol(psmouse, NULL);
input_register_device(&psmouse->dev);
-
printk(KERN_INFO "input: %s on %s\n", psmouse->devname, serio->phys);
psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
@@ -776,6 +906,7 @@ static int psmouse_connect(struct serio
if (parent && parent->pt_activate)
parent->pt_activate(parent);
+ device_create_file(&serio->dev, &psmouse_attr_protocol);
device_create_file(&serio->dev, &psmouse_attr_rate);
device_create_file(&serio->dev, &psmouse_attr_resolution);
device_create_file(&serio->dev, &psmouse_attr_resetafter);
@@ -914,11 +1045,14 @@ ssize_t psmouse_attr_set_helper(struct d
parent = serio_get_drvdata(serio->parent);
psmouse_deactivate(parent);
}
+
psmouse_deactivate(psmouse);
retval = handler(psmouse, buf, count);
- psmouse_activate(psmouse);
+ if (retval != -ENODEV)
+ psmouse_activate(psmouse);
+
if (parent)
psmouse_activate(parent);
@@ -927,6 +1061,73 @@ out:
return retval;
}
+static ssize_t psmouse_attr_show_protocol(struct psmouse *psmouse, char *buf)
+{
+ return sprintf(buf, "%s\n", psmouse_protocol_by_type(psmouse->type)->name);
+}
+
+static ssize_t psmouse_attr_set_protocol(struct psmouse *psmouse, const char *buf, size_t count)
+{
+ struct serio *serio = psmouse->ps2dev.serio;
+ struct psmouse *parent = NULL;
+ struct psmouse_protocol *proto;
+ int retry = 0;
+
+ if (!(proto = psmouse_protocol_by_name(buf, count)))
+ return -EINVAL;
+
+ if (psmouse->type == proto->type)
+ return count;
+
+ while (serio->child) {
+ if (++retry > 3) {
+ printk(KERN_WARNING "psmouse: failed to destroy child port, protocol change aborted.\n");
+ return -EIO;
+ }
+
+ serio_unpin_driver(serio);
+ serio_unregister_child_port(serio);
+ serio_pin_driver_uninterruptible(serio);
+
+ if (serio->drv != &psmouse_drv)
+ return -ENODEV;
+
+ if (psmouse->type == proto->type)
+ return count; /* switched by other thread */
+ }
+
+ if (serio->parent && serio->id.type == SERIO_PS_PSTHRU) {
+ parent = serio_get_drvdata(serio->parent);
+ if (parent->pt_deactivate)
+ parent->pt_deactivate(parent);
+ }
+
+ if (psmouse->disconnect)
+ psmouse->disconnect(psmouse);
+
+ psmouse_set_state(psmouse, PSMOUSE_IGNORE);
+ input_unregister_device(&psmouse->dev);
+
+ psmouse_set_state(psmouse, PSMOUSE_INITIALIZING);
+
+ if (psmouse_switch_protocol(psmouse, proto) < 0) {
+ psmouse_reset(psmouse);
+ /* default to PSMOUSE_PS2 */
+ psmouse_switch_protocol(psmouse, &psmouse_protocols[0]);
+ }
+
+ psmouse_initialize(psmouse);
+ psmouse_set_state(psmouse, PSMOUSE_CMD_MODE);
+
+ input_register_device(&psmouse->dev);
+ printk(KERN_INFO "input: %s on %s\n", psmouse->devname, serio->phys);
+
+ if (parent && parent->pt_activate)
+ parent->pt_activate(parent);
+
+ return count;
+}
+
static ssize_t psmouse_attr_show_rate(struct psmouse *psmouse, char *buf)
{
return sprintf(buf, "%d\n", psmouse->rate);
@@ -983,34 +1184,26 @@ static ssize_t psmouse_attr_set_resetaft
static int psmouse_set_maxproto(const char *val, struct kernel_param *kp)
{
- int i;
+ struct psmouse_protocol *proto;
if (!val)
return -EINVAL;
- if (!strncmp(val, "any", 3)) {
- *((unsigned int *)kp->arg) = -1U;
- return 0;
- }
+ proto = psmouse_protocol_by_name(val, strlen(val));
- for (i = 0; i < ARRAY_SIZE(psmouse_proto_abbrev); i++) {
- if (!psmouse_proto_abbrev[i])
- continue;
-
- if (!strncmp(val, psmouse_proto_abbrev[i], strlen(psmouse_proto_abbrev[i]))) {
- *((unsigned int *)kp->arg) = i;
- return 0;
- }
- }
+ if (!proto || !proto->maxproto)
+ return -EINVAL;
- return -EINVAL; \
+ *((unsigned int *)kp->arg) = proto->type;
+
+ return 0; \
}
static int psmouse_get_maxproto(char *buffer, struct kernel_param *kp)
{
- return sprintf(buffer, "%s\n",
- psmouse_max_proto < ARRAY_SIZE(psmouse_proto_abbrev) ?
- psmouse_proto_abbrev[psmouse_max_proto] : "any");
+ int type = *((unsigned int *)kp->arg);
+
+ return sprintf(buffer, "%s\n", psmouse_protocol_by_type(type)->name);
}
static int __init psmouse_init(void)
On Tue, Mar 22, 2005 at 02:14:55AM -0500, Dmitry Torokhov wrote:
> ===================================================================
>
> Input: lifebook - DMI facility is only available on i386, do not
> attempt to compile on anything else.
Why would you want to build a driver for an x86 laptop on anything
but x86 ? Ie, why not just add a dependancy in the Kconfig
so you don't have to add ifdefs to the code ?
Dave
On Tuesday 22 March 2005 02:29, Dave Jones wrote:
> On Tue, Mar 22, 2005 at 02:14:55AM -0500, Dmitry Torokhov wrote:
> > ===================================================================
> >
> > Input: lifebook - DMI facility is only available on i386, do not
> > attempt to compile on anything else.
>
> Why would you want to build a driver for an x86 laptop on anything
> but x86 ? Ie, why not just add a dependancy in the Kconfig
> so you don't have to add ifdefs to the code ?
>
lifebook is a part of psmouse driver and is presenty not selectable on
its own.
--
Dmitry
On 081, 03 22, 2005 at 02:13:45AM -0500, Dmitry Torokhov wrote:
> On Monday 21 March 2005 10:31, Kenan Esau wrote:
> > Am Montag, den 21.03.2005, 09:52 -0500 schrieb Dmitry Torokhov:
> > >
> > > There are couple of things that I an concerned with:
> > >
> > > 1. I don't like that it overrides meaning of max_proto parameter to be
> > > exactly the protocol specified.
> >
> > Yeah -- I agree. I also don't like that double-meaning. That was the
> > reason why I originally proposed the use of a new parameter...
> >
>
> Ok, I have some patches to lifebook that I would like to included (if
> they work):
>
> 1. lifebook-dmi-x86-only - do not compile in DMI detection on anything
> but x86.
On !x86 machines DMI functions will be optimized away and so you'll save only
few bytes in .init.data section. IMHO it's not worth additional ugly #ifdef's.
--
Andrey Panin | Linux and UNIX system administrator
[email protected] | PGP key: wwwkeys.pgp.net
On Maw, 2005-03-22 at 07:14, Dmitry Torokhov wrote:
> ===================================================================
>
> Input: lifebook - DMI facility is only available on i386, do not
> attempt to compile on anything else.
DMI is also available on x86-64, and its present although not in Linux
on
iA64
On Tue, 22 Mar 2005 13:01:14 +0300, Andrey Panin <[email protected]> wrote:
> On 081, 03 22, 2005 at 02:13:45AM -0500, Dmitry Torokhov wrote:
> > On Monday 21 March 2005 10:31, Kenan Esau wrote:
> > > Am Montag, den 21.03.2005, 09:52 -0500 schrieb Dmitry Torokhov:
> > > >
> > > > There are couple of things that I an concerned with:
> > > >
> > > > 1. I don't like that it overrides meaning of max_proto parameter to be
> > > > exactly the protocol specified.
> > >
> > > Yeah -- I agree. I also don't like that double-meaning. That was the
> > > reason why I originally proposed the use of a new parameter...
> > >
> >
> > Ok, I have some patches to lifebook that I would like to included (if
> > they work):
> >
> > 1. lifebook-dmi-x86-only - do not compile in DMI detection on anything
> > but x86.
>
> On !x86 machines DMI functions will be optimized away and so you'll save only
> few bytes in .init.data section. IMHO it's not worth additional ugly #ifdef's.
>
It is not in .init.data, it is not discarded... Still probably OK to
keep. Not quite sure.
--
Dmitry
Patches 1-3 are fine.
Protocol switching via sysfs works too but if I switch from LBPS/2 to
PS/2 the device name changes from "/dev/event1" to "/dev/event2" -- is
this intended?
If I do "echo -n 50 > resolution" "0xe8 0x01" is sent. I don't know if
this is correct for "usual" PS/2-devices but for the lifebook it's
wrong.
For the lifebook the parameters are as following:
50cpi <=> 0x00
100cpi <=> 0x01
200cpi <=> 0x02
400cpi <=> 0x03
Hi Kenan,
On Sunday 03 April 2005 14:49, Kenan Esau wrote:
> Patches 1-3 are fine.
>
Thank you very much for testing the patches. Based on the feedback I
received I am goping to drop that DMI patch - does not save enough to
justify the ifdefs...
> Protocol switching via sysfs works too but if I switch from LBPS/2 to
> PS/2 the device name changes from "/dev/event1" to "/dev/event2" -- is
> this intended?
Yes - we in fact getting somewhat a "new" device with new capabilities so
the driver unregisters old input device and creates a new one. I strongly
believe that we should not change input device attributes "on fly".
> If I do "echo -n 50 > resolution" "0xe8 0x01" is sent. I don't know if
> this is correct for "usual" PS/2-devices but for the lifebook it's
> wrong.
>
> For the lifebook the parameters are as following:
>
> 50cpi <=> 0x00
> 100cpi <=> 0x01
> 200cpi <=> 0x02
> 400cpi <=> 0x03
>
"Classic" PS/2 protocol specifies available resolutions of 1, 2, 4 and 8
units per mm which gives you 25, 50, 100 and 200 cpi respectively. I am
surprised that Lifebook simply doubles the rates, but if it does I guess
the patch below will suffice.
--
Dmitry
===================================================================
Input: apparently Lifebook touchscreens have double resolution
compared to "classic" PS/2 mice, provide appropriate
resolution setting handler.
Signed-off-by: Dmitry Torokhov <[email protected]>
lifebook.c | 12 ++++++++++++
1 files changed, 12 insertions(+)
Index: dtor/drivers/input/mouse/lifebook.c
===================================================================
--- dtor.orig/drivers/input/mouse/lifebook.c
+++ dtor/drivers/input/mouse/lifebook.c
@@ -82,6 +82,17 @@ static int lifebook_absolute_mode(struct
return 0;
}
+static void lifebook_set_resolution(struct psmouse *psmouse, unsigned int resolution)
+{
+ unsigned char params[] = { 0, 1, 2, 2, 3 };
+
+ if (resolution == 0 || resolution > 400)
+ resolution = 400;
+
+ ps2_command(&psmouse->ps2dev, ¶ms[resolution / 100], PSMOUSE_CMD_SETRES);
+ psmouse->resolution = 50 << params[resolution / 100];
+}
+
static void lifebook_disconnect(struct psmouse *psmouse)
{
psmouse_reset(psmouse);
@@ -114,6 +125,7 @@ int lifebook_init(struct psmouse *psmous
input_set_abs_params(&psmouse->dev, ABS_Y, 0, 1024, 0, 0);
psmouse->protocol_handler = lifebook_process_byte;
+ psmouse->set_resolution = lifebook_set_resolution;
psmouse->disconnect = lifebook_disconnect;
psmouse->reconnect = lifebook_absolute_mode;
psmouse->pktsize = 3;
Am Montag, den 04.04.2005, 00:45 -0500 schrieb Dmitry Torokhov:
> Hi Kenan,
>
[..]
> > If I do "echo -n 50 > resolution" "0xe8 0x01" is sent. I don't know if
> > this is correct for "usual" PS/2-devices but for the lifebook it's
> > wrong.
> >
> > For the lifebook the parameters are as following:
> >
> > 50cpi <=> 0x00
> > 100cpi <=> 0x01
> > 200cpi <=> 0x02
> > 400cpi <=> 0x03
> >
>
> "Classic" PS/2 protocol specifies available resolutions of 1, 2, 4 and 8
> units per mm which gives you 25, 50, 100 and 200 cpi respectively. I am
> surprised that Lifebook simply doubles the rates, but if it does I guess
> the patch below will suffice.
Yes -- exactly. That was what I was thinking about. I was just not shure
whether it is lifebook specific or not.
I haven't tested the patch yet. But by inspecting the code it looks
good ;-)