Dear Mr. Vojtech,
I learn that you are the author for PS/2 mouse driver on Linux 2.6.x
kernel.
I just want to share a little change that I've did to
psmouse_pm_callback() which without this, my synaptics touchpad would
prevent my laptop (IBM Thinkpad T30) from suspending.
I don't remember how the functions read at first. But this is how it
become. This is not exactly a patch, huh? Sorry, but I hope it helps.
I also not sure if this patch already done the right thing and I haven't
tested it on other machine. It works for me, but I can't promise if it
blows up others. Maybe you could give it a test?
The kernel version was 2.6.0-test9 from Debian (not vanilla, but I'm
sure the psmouse part is clean).
This is the function:
static int psmouse_pm_callback(struct pm_dev *dev, pm_request_t request,
void *data)
{
struct psmouse *psmouse = dev->data;
struct serio_dev *ser_dev = psmouse->serio->dev;
switch (request) {
case PM_RESUME:
psmouse->state = PSMOUSE_IGNORE;
serio_rescan(psmouse->serio);
default:
return 0;
}
}
Sorry if this just bothers you.
I'm cc-ing also to [linux-kernel], but I'm not subscribed to the list yet.
Best Regards,
--
arief_mulya <http://www.geocities.com/naida_s_rasha/>
Peace is beautiful.
arief_mulya <[email protected]> wrote:
>
> static int psmouse_pm_callback(struct pm_dev *dev, pm_request_t request,
> void *data)
> {
> struct psmouse *psmouse = dev->data;
> struct serio_dev *ser_dev = psmouse->serio->dev;
>
>
> switch (request) {
> case PM_RESUME:
> psmouse->state = PSMOUSE_IGNORE;
> serio_rescan(psmouse->serio);
> default:
> return 0;
> }
> }
What does the driver do without this change? ie: what problem is this
fixing?
Why is it calling serio_rescan() rather than serio_reconnect()?
On Sunday 09 November 2003 11:12 pm, Andrew Morton wrote:
> arief_mulya <[email protected]> wrote:
> > static int psmouse_pm_callback(struct pm_dev *dev, pm_request_t
> > request, void *data)
> > {
> > struct psmouse *psmouse = dev->data;
> > struct serio_dev *ser_dev = psmouse->serio->dev;
> >
> >
> > switch (request) {
> > case PM_RESUME:
> > psmouse->state = PSMOUSE_IGNORE;
> > serio_rescan(psmouse->serio);
> > default:
> > return 0;
> > }
> > }
>
> What does the driver do without this change? ie: what problem is this
> fixing?
>
> Why is it calling serio_rescan() rather than serio_reconnect()?
>
serio_reconnect() is only in your tree (-mm), it has not been pushed to
Linus yet... Unfortunately using rescan can cause input devices be shifted
if some program has them open while suspending.
Dmitry
Dmitry Torokhov <[email protected]> wrote:
>
> On Sunday 09 November 2003 11:12 pm, Andrew Morton wrote:
> > arief_mulya <[email protected]> wrote:
> > > static int psmouse_pm_callback(struct pm_dev *dev, pm_request_t
> > > request, void *data)
> > > {
> > > struct psmouse *psmouse = dev->data;
> > > struct serio_dev *ser_dev = psmouse->serio->dev;
> > >
> > >
> > > switch (request) {
> > > case PM_RESUME:
> > > psmouse->state = PSMOUSE_IGNORE;
> > > serio_rescan(psmouse->serio);
> > > default:
> > > return 0;
> > > }
> > > }
> >
> > What does the driver do without this change? ie: what problem is this
> > fixing?
> >
> > Why is it calling serio_rescan() rather than serio_reconnect()?
> >
>
> serio_reconnect() is only in your tree (-mm), it has not been pushed to
> Linus yet... Unfortunately using rescan can cause input devices be shifted
> if some program has them open while suspending.
Ah, I see. So would you say that reconnect is the correct thing to use
here?
That would mean that the appropriate patch against -mm is
--- 25/drivers/input/mouse/psmouse-base.c~serio-pm-fix 2003-11-09 20:12:27.000000000 -0800
+++ 25-akpm/drivers/input/mouse/psmouse-base.c 2003-11-09 20:12:27.000000000 -0800
@@ -533,9 +533,10 @@ static int psmouse_pm_callback(struct pm
{
struct psmouse *psmouse = dev->data;
- psmouse->state = PSMOUSE_IGNORE;
- serio_reconnect(psmouse->serio);
-
+ if (request == PM_RESUME) {
+ psmouse->state = PSMOUSE_IGNORE;
+ serio_reconnect(psmouse->serio);
+ }
return 0;
}
_
Those serio patches have been in -mm for six weeks btw. Was there some
problem with them?
Andrew Morton wrote:
>Dmitry Torokhov <[email protected]> wrote:
>
>
>>On Sunday 09 November 2003 11:12 pm, Andrew Morton wrote:
>>
>>
>>>arief_mulya <[email protected]> wrote:
>>>
>>>
>>>>static int psmouse_pm_callback(struct pm_dev *dev, pm_request_t
>>>>request, void *data)
>>>> {
>>>> struct psmouse *psmouse = dev->data;
>>>> struct serio_dev *ser_dev = psmouse->serio->dev;
>>>>
>>>>
>>>> switch (request) {
>>>> case PM_RESUME:
>>>> psmouse->state = PSMOUSE_IGNORE;
>>>> serio_rescan(psmouse->serio);
>>>> default:
>>>> return 0;
>>>> }
>>>> }
>>>>
>>>>
>>>What does the driver do without this change? ie: what problem is this
>>>fixing?
>>>
>>>Why is it calling serio_rescan() rather than serio_reconnect()?
>>>
>>>
>>>
>>serio_reconnect() is only in your tree (-mm), it has not been pushed to
>>Linus yet... Unfortunately using rescan can cause input devices be shifted
>>if some program has them open while suspending.
>>
>>
>
>
>
Well, I don't know about this.
I was just trying to get my synaptics mouse working nicely through
suspend-resume. And I haven't learn about linux kernel structure before,
so I just spend all my sunshiny Sunday, researching everywhere with my
lazy brain.
And finally, those (ugly?) patch come out ;-)
>Ah, I see. So would you say that reconnect is the correct thing to use
>here?
>
>That would mean that the appropriate patch against -mm is
>
>--- 25/drivers/input/mouse/psmouse-base.c~serio-pm-fix 2003-11-09 20:12:27.000000000 -0800
>+++ 25-akpm/drivers/input/mouse/psmouse-base.c 2003-11-09 20:12:27.000000000 -0800
>@@ -533,9 +533,10 @@ static int psmouse_pm_callback(struct pm
> {
> struct psmouse *psmouse = dev->data;
>
>- psmouse->state = PSMOUSE_IGNORE;
>- serio_reconnect(psmouse->serio);
>-
>+ if (request == PM_RESUME) {
>+ psmouse->state = PSMOUSE_IGNORE;
>+ serio_reconnect(psmouse->serio);
>+ }
> return 0;
> }
>
>
>_
>
>
>
I guess I gotta try your tree now.
>Those serio patches have been in -mm for six weeks btw. Was there some
>problem with them?
>
>
I don't know. Has anybody tested it using an IBM T30 with Synaptics inside?
Best Regards.
--
arief_mulya
On Monday 10 November 2003 01:56 am, Andrew Morton wrote:
> Dmitry Torokhov <[email protected]> wrote:
> >
> > serio_reconnect() is only in your tree (-mm), it has not been pushed
> > to Linus yet... Unfortunately using rescan can cause input devices be
> > shifted if some program has them open while suspending.
>
> Ah, I see. So would you say that reconnect is the correct thing to use
> here?
>
> That would mean that the appropriate patch against -mm is
>
> --- 25/drivers/input/mouse/psmouse-base.c~serio-pm-fix 2003-11-09
> 20:12:27.000000000 -0800 +++
> 25-akpm/drivers/input/mouse/psmouse-base.c 2003-11-09
> 20:12:27.000000000 -0800 @@ -533,9 +533,10 @@ static int
> psmouse_pm_callback(struct pm
> {
> struct psmouse *psmouse = dev->data;
>
> - psmouse->state = PSMOUSE_IGNORE;
> - serio_reconnect(psmouse->serio);
> -
> + if (request == PM_RESUME) {
> + psmouse->state = PSMOUSE_IGNORE;
> + serio_reconnect(psmouse->serio);
> + }
> return 0;
> }
>
Yes, I believe this will work. And for vanilla 2.6 the patch below should
do the trick. As you can see vanilla 2.6 has custom reconnect logic in PM
handler but it does not work very well for devices connected to Synaptics
pass-through port - it will unregister it and register again potentially
creating a new input device like serio does. The "main" mouse device will
retain its device though.
===================================================================
[email protected], 2003-11-11 00:06:11-05:00, [email protected]
Re-initialize mouse hardware on resume only.
psmouse-base.c | 20 +++++++++++---------
1 files changed, 11 insertions(+), 9 deletions(-)
diff -Nru a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
--- a/drivers/input/mouse/psmouse-base.c Tue Nov 11 00:07:50 2003
+++ b/drivers/input/mouse/psmouse-base.c Tue Nov 11 00:07:50 2003
@@ -528,17 +528,19 @@
struct psmouse *psmouse = dev->data;
struct serio_dev *ser_dev = psmouse->serio->dev;
- synaptics_disconnect(psmouse);
+ if (request == PM_RESUME) {
+ synaptics_disconnect(psmouse);
- /* We need to reopen the serio port to reinitialize the i8042 controller */
- serio_close(psmouse->serio);
- serio_open(psmouse->serio, ser_dev);
+ /* We need to reopen the serio port to reinitialize the i8042 controller */
+ serio_close(psmouse->serio);
+ serio_open(psmouse->serio, ser_dev);
- /* Probe and re-initialize the mouse */
- psmouse_probe(psmouse);
- psmouse_initialize(psmouse);
- synaptics_pt_init(psmouse);
- psmouse_activate(psmouse);
+ /* Probe and re-initialize the mouse */
+ psmouse_probe(psmouse);
+ psmouse_initialize(psmouse);
+ synaptics_pt_init(psmouse);
+ psmouse_activate(psmouse);
+ }
return 0;
}
===================================================================
Unfortunately I do not suspend my laptop so I did not run it, just
made sure it compiles. Arief? could you give this patch a try?
Dmitry
Dmitry Torokhov wrote:
>On Monday 10 November 2003 01:56 am, Andrew Morton wrote:
>
>
>>Dmitry Torokhov <[email protected]> wrote:
>>
>>
>
>
>
>>>serio_reconnect() is only in your tree (-mm), it has not been pushed
>>>to Linus yet... Unfortunately using rescan can cause input devices be
>>>shifted if some program has them open while suspending.
>>>
>>>
>>Ah, I see. So would you say that reconnect is the correct thing to use
>>here?
>>
>>That would mean that the appropriate patch against -mm is
>>
>>--- 25/drivers/input/mouse/psmouse-base.c~serio-pm-fix 2003-11-09
>>20:12:27.000000000 -0800 +++
>>25-akpm/drivers/input/mouse/psmouse-base.c 2003-11-09
>>20:12:27.000000000 -0800 @@ -533,9 +533,10 @@ static int
>>psmouse_pm_callback(struct pm
>> {
>> struct psmouse *psmouse = dev->data;
>>
>>- psmouse->state = PSMOUSE_IGNORE;
>>- serio_reconnect(psmouse->serio);
>>-
>>+ if (request == PM_RESUME) {
>>+ psmouse->state = PSMOUSE_IGNORE;
>>+ serio_reconnect(psmouse->serio);
>>+ }
>> return 0;
>> }
>>
>>
>>
>
>Yes, I believe this will work. And for vanilla 2.6 the patch below should
>do the trick. As you can see vanilla 2.6 has custom reconnect logic in PM
>handler but it does not work very well for devices connected to Synaptics
>pass-through port - it will unregister it and register again potentially
>creating a new input device like serio does. The "main" mouse device will
>retain its device though.
>
>===================================================================
>[email protected], 2003-11-11 00:06:11-05:00, [email protected]
> Re-initialize mouse hardware on resume only.
>
>
> psmouse-base.c | 20 +++++++++++---------
> 1 files changed, 11 insertions(+), 9 deletions(-)
>
>
>diff -Nru a/drivers/input/mouse/psmouse-base.c b/drivers/input/mouse/psmouse-base.c
>--- a/drivers/input/mouse/psmouse-base.c Tue Nov 11 00:07:50 2003
>+++ b/drivers/input/mouse/psmouse-base.c Tue Nov 11 00:07:50 2003
>@@ -528,17 +528,19 @@
> struct psmouse *psmouse = dev->data;
> struct serio_dev *ser_dev = psmouse->serio->dev;
>
>- synaptics_disconnect(psmouse);
>+ if (request == PM_RESUME) {
>+ synaptics_disconnect(psmouse);
>
>- /* We need to reopen the serio port to reinitialize the i8042 controller */
>- serio_close(psmouse->serio);
>- serio_open(psmouse->serio, ser_dev);
>+ /* We need to reopen the serio port to reinitialize the i8042 controller */
>+ serio_close(psmouse->serio);
>+ serio_open(psmouse->serio, ser_dev);
>
>- /* Probe and re-initialize the mouse */
>- psmouse_probe(psmouse);
>- psmouse_initialize(psmouse);
>- synaptics_pt_init(psmouse);
>- psmouse_activate(psmouse);
>+ /* Probe and re-initialize the mouse */
>+ psmouse_probe(psmouse);
>+ psmouse_initialize(psmouse);
>+ synaptics_pt_init(psmouse);
>+ psmouse_activate(psmouse);
>+ }
>
> return 0;
> }
>
>===================================================================
>
>Unfortunately I do not suspend my laptop so I did not run it, just
>made sure it compiles. Arief? could you give this patch a try?
>
>
>
>
I have tested it before.
My first attempts looked quite just like that.
It didn't work quite nicely.
Especially with gpm, after resume, you cannot do Tap-to-Click behaviour
with that patch. You can still move it, use left and right button, but
no tap-to-click. I don't know why. That's why, finally, I use
serio_rescan().
I haven't tested it with X, though, as I use gpm as a repeater, I
thought this was unnecessary.
But I have try Andrew's tree. And it works flawlessly with the patch
(case PM_RESUME: serio_reconnect()). I think I'm going to stick with mm
tree, and dump my vanilla kernel.
One more think, I also sets "psmouse_resetafter" to 1 at the
declaration. Without that, I get too many ugly message saying "Synaptics
lost sync at 1 byte..." or something like that. As it is a module
parameter, but on menuconfig synaptics does not available as module, so
I set it directly on the source. I don't know if I can set it on boot
time, can it?
Best Regards
--
arief_mulya
Peace is Beautiful.
On Tuesday 11 November 2003 01:54 am, arief_mulya wrote:
> Dmitry Torokhov wrote:
> >
> >Unfortunately I do not suspend my laptop so I did not run it, just
> >made sure it compiles. Arief? could you give this patch a try?
>
> I have tested it before.
> My first attempts looked quite just like that.
>
> It didn't work quite nicely.
> Especially with gpm, after resume, you cannot do Tap-to-Click behaviour
> with that patch. You can still move it, use left and right button, but
> no tap-to-click. I don't know why. That's why, finally, I use
> serio_rescan().
>
> I haven't tested it with X, though, as I use gpm as a repeater, I
> thought this was unnecessary.
>
> But I have try Andrew's tree. And it works flawlessly with the patch
> (case PM_RESUME: serio_reconnect()). I think I'm going to stick with mm
> tree, and dump my vanilla kernel.
>
> One more think, I also sets "psmouse_resetafter" to 1 at the
> declaration. Without that, I get too many ugly message saying
> "Synaptics lost sync at 1 byte..." or something like that. As it is a
> module parameter, but on menuconfig synaptics does not available as
> module, so I set it directly on the source. I don't know if I can set
> it on boot time, can it?
>
Ok, I somewhat confused as the sequence in psmouse_pm_callback is pretty
much the same as in rescan/reconnect. Wait...
Does suspend/resume work at all if you don't set psmouse_resetafter to 1??
The reason I am asking is that we have alot of different PM interfaces and
this one is marked as deprecated. If it is not called during resume it would
leave the touchpad in relative mode while kernel expects absolute and spews
"lost sync" messages... Until Synaptics decides that it's time to reset
after X bad packets. Does it make any sense?
Btw, Synaptics is intergated into psmouse module so you don't really need
to edit the source to set resetafter parameter. Either pass
"psmouse_resetafter=X" to the kernel on boot if psmouse compiled in or
pass it to modprobe.
Dmitry
Dmitry Torokhov wrote:
>On Tuesday 11 November 2003 01:54 am, arief_mulya wrote:
>
>
>>Dmitry Torokhov wrote:
>>
>>
>
>
>
>>>Unfortunately I do not suspend my laptop so I did not run it, just
>>>made sure it compiles. Arief? could you give this patch a try?
>>>
>>>
>>I have tested it before.
>>My first attempts looked quite just like that.
>>
>>It didn't work quite nicely.
>>Especially with gpm, after resume, you cannot do Tap-to-Click behaviour
>>with that patch. You can still move it, use left and right button, but
>>no tap-to-click. I don't know why. That's why, finally, I use
>>serio_rescan().
>>
>>I haven't tested it with X, though, as I use gpm as a repeater, I
>>thought this was unnecessary.
>>
>>But I have try Andrew's tree. And it works flawlessly with the patch
>>(case PM_RESUME: serio_reconnect()). I think I'm going to stick with mm
>>tree, and dump my vanilla kernel.
>>
>>One more think, I also sets "psmouse_resetafter" to 1 at the
>>declaration. Without that, I get too many ugly message saying
>>"Synaptics lost sync at 1 byte..." or something like that. As it is a
>>module parameter, but on menuconfig synaptics does not available as
>>module, so I set it directly on the source. I don't know if I can set
>>it on boot time, can it?
>>
>>
>>
>
>Ok, I somewhat confused as the sequence in psmouse_pm_callback is pretty
>much the same as in rescan/reconnect. Wait...
>
>
>
This is what I thought, too.
Why did it work with serio_rescan() or serio_reconnect() but not with
the one above?
But on second thought, I didn't know how the kernel works, not then. Not
now. So when it works for me, my lazy brain, said. That's enough. ;-)
>Does suspend/resume work at all if you don't set psmouse_resetafter to 1??
>
>
Nope. At least, I didn't wait that long. (I hate seeing lots of "lost
sync" messages in my console forever, so I always reboot. And in X, you
just get un-moveable mouse.)
>The reason I am asking is that we have alot of different PM interfaces and
>this one is marked as deprecated. If it is not called during resume it would
>leave the touchpad in relative mode while kernel expects absolute and spews
>"lost sync" messages... Until Synaptics decides that it's time to reset
>after X bad packets. Does it make any sense?
>
>
>
Personally, I don't think it makes any sense.
If something is broken. Why do I need to see lots of funny messages
(which I didn't even know what it means) if then it could be fixed. Or
is it has something to do with other serio devices?
>Btw, Synaptics is intergated into psmouse module so you don't really need
>to edit the source to set resetafter parameter. Either pass
>"psmouse_resetafter=X" to the kernel on boot if psmouse compiled in or
>pass it to modprobe.
>
>
>
I thought so, too.
But how do you pass to modprobe something that does not available as
Module in menuconfig?
Best Regards.
--
arief_mulya
Peace is Beautiful.
>Dmitry
>
>
>
On Tuesday 11 November 2003 09:28 pm, arief_mulya wrote:
> Dmitry Torokhov wrote:
>
> >Does suspend/resume work at all if you don't set psmouse_resetafter to
> > 1??
>
> Nope. At least, I didn't wait that long. (I hate seeing lots of "lost
> sync" messages in my console forever, so I always reboot. And in X, you
> just get un-moveable mouse.)
>
OK, thank you for confirming it.
> >The reason I am asking is that we have alot of different PM interfaces
> > and this one is marked as deprecated. If it is not called during
> > resume it would leave the touchpad in relative mode while kernel
> > expects absolute and spews "lost sync" messages... Until Synaptics
> > decides that it's time to reset after X bad packets. Does it make any
> > sense?
>
> Personally, I don't think it makes any sense.
> If something is broken. Why do I need to see lots of funny messages
> (which I didn't even know what it means) if then it could be fixed. Or
> is it has something to do with other serio devices?
>
These messages tell you that something bad happened and data stream that
is coming from the mouse hardware does not match selected protocol. In your
case it turns out that PM callback isn't get called at all. This leaves
the touchpad in relative mode (after BIOS init) while Synaptics driver
expects so called absolute protocol. Normally you should not see these
messages, they signal that something went wrong.
Now, you activated psmouse_resetafter option. I coded it to take care of
my docking station resetting the touchpad back into relative mode without
telling anyone. If it is active then Synaptics tries to re-initialize
itself after so many bad packets (1 in your case).
What I am trying to tell is that pm_callback does not affect anything in
your case, its psmouse_resetafter that does re-initialization.
Andrew, the patch, although probably not needed should be still valid as
there is no point of trying to reconnect when suspending.
> >Btw, Synaptics is intergated into psmouse module so you don't really
> > need to edit the source to set resetafter parameter. Either pass
> >"psmouse_resetafter=X" to the kernel on boot if psmouse compiled in or
> >pass it to modprobe.
>
> I thought so, too.
> But how do you pass to modprobe something that does not available as
> Module in menuconfig?
>
There is only one module called psmouse that supports different flavors
of PS/2 protocol along with (optional) Synaptics protocol. You should try
to "modprobe psmouse psmouse_resetafter=X".
Dmitry