On Thu, Aug 01, 2013 at 02:37:27PM +0200, Takashi Iwai wrote: > At Thu, 1 Aug 2013 14:12:58 +0200, > Egbert Eich wrote: > > > > On Thu, Aug 01, 2013 at 04:38:38PM +1000, Peter Hutterer wrote: > > > On Tue, Jul 30, 2013 at 05:27:06AM +0200, Egbert Eich wrote: > > > > From: Takashi Iwai <[email protected]> > > > > > > > > Under some circumstances the synaptics device may be wedged when > > > > coming back from a sleep state. Add some magic which tries to > > > > detect this and reconnect. > > > > > > > > Signed-off-by: Takashi Iwai <[email protected]> > > > > Signed-off-by: Egbert Eich <[email protected]> > > > > --- > > > > v2: Avoid '_' at beginning of function name. > > > > > > > > src/eventcomm.c | 3 ++- > > > > src/synaptics.c | 56 > > > > +++++++++++++++++++++++++++++++++++++++++++++++------- > > > > src/synapticsstr.h | 3 +++ > > > > 3 files changed, 54 insertions(+), 8 deletions(-) > > > > > > > > diff --git a/src/eventcomm.c b/src/eventcomm.c > > > > index 258a538..86b74cb 100644 > > > > --- a/src/eventcomm.c > > > > +++ b/src/eventcomm.c > > > > @@ -638,7 +638,8 @@ EventReadHwState(InputInfoPtr pInfo, > > > > } > > > > > > > > while (SynapticsReadEvent(pInfo, &ev)) { > > > > - switch (ev.type) { > > > > + priv->comm_read++; > > > > + switch (ev.type) { > > > > > > indentation, goes for the rest of the patch. > > > > > > comm_read is increased on every event so there's the (unlikely) chance of > > > an > > > overrun. why not just set this to 1? > > > > Yeah, for the purpose we use it for it shouldn't make a difference. > > > > > > > > > case EV_SYN: > > > > switch (ev.code) { > > > > case SYN_REPORT: > > > > diff --git a/src/synaptics.c b/src/synaptics.c > > > > index f0a8269..007d0bb 100644 > > > > --- a/src/synaptics.c > > > > +++ b/src/synaptics.c > > > > @@ -920,18 +920,30 @@ DeviceControl(DeviceIntPtr dev, int mode) > > > > } > > > > > > > > static int > > > > -DeviceOn(DeviceIntPtr dev) > > > > +doDeviceOn(InputInfoPtr pInfo) > > > > { > > > > - InputInfoPtr pInfo = dev->public.devicePrivate; > > > > SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private); > > > > + int n; > > > > > > > > DBG(3, "Synaptics DeviceOn called\n"); > > > > > > > > - pInfo->fd = xf86OpenSerial(pInfo->options); > > > > + for (n = priv->retries; n >= 0; n--) { > > > > > > I really want a comment here - I read over the >= 0 and was wondering how > > > we'd even enter the loop. > > > > I don't like comments for this purpose. So let's rewrite the code to > > make this more obvious. > > > > > > > > > + pInfo->fd = xf86OpenSerial(pInfo->options); > > > > + if (pInfo->fd != -1) > > > > + break; > > > > + if (n) > > > > + xf86Msg(X_WARNING, "%s: cannot open input device - " > > > > + "retrying %d more times\n", pInfo->name, n); > > > > + } > > > > if (pInfo->fd == -1) { > > > > xf86IDrvMsg(pInfo, X_WARNING, "cannot open input device\n"); > > > > return !Success; > > > > } > > > > + /* This has succeeded once, so chances are the hardware *really* > > > > is present > > > > + * - this is not a hotplug device after all. > > > > + * Without trying really hard on some machines with some kernels > > > > the device > > > > + * won't be found after S3/S4 again. */ > > > > + priv->retries = 4; > > > > > > I'd love to have some details here above "some machines with some > > > kernels". > > > I remember this being a problem around the Fedora 8 cycle (or so) but it's > > > hasn't been seen for ages here (evdev doesnt have the code anymore). > > > > > > what's the errno when this happens? > > > > I don't know the details here as I never looked into this bug. > > Takashi - do you remember this? Do we still need to bother? > > It's the time of SLE11-GA, i.e. 2.6.27 kernel, found on a few HP > laptops based on IronLake. There is no problem since then.
[...] > > IIRC, the device is still present in a stale state, so it's there > uselessly until explicitly closed. It was a kind of stale state while > the kernel synaptics driver does reset after resume or so. 2.6.27 sounds about right for Fedora 8.fwiw, see evdev commit 9930477cbeb4acfd0, and its removal in 59056e656c6475816. In that case the fd could be opened, but the first read would give ENODEV. I suspect you're seeing the same problem here. If you haven't seen this problem since we'll leave it out upstream though, please ship this as distro patch if you still need it. Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
