[PATCH driver/input/synaptics] For serial devices try reconnecting if device is wedged.
Mark Kettenis
mark.kettenis at xs4all.nl
Mon Jul 29 06:55:54 PDT 2013
> Date: Mon, 29 Jul 2013 14:02:55 +0100
> From: Daniel Stone <daniel at fooishbar.org>
>
> On 29 July 2013 12:31, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
> >> From: Egbert Eich <eich at freedesktop.org>
> >> Date: Mon, 29 Jul 2013 13:15:13 +0200
> >>
> >> From: Takashi Iwai <tiwai at suse.de>
> >>
> >> 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.
> >
> > You shouldn't use function names that start with an underscore.
>
> Says who?
C99, 7.1.3 Reserved identifiers:
-- All identifiers that begin with an underscore and either an
uppercase letter or another underscore are always reserved for
any use.
-- All identifiers that begin with an underscore are always reserved
for use as identifiers with file scope in both the ordinary and
tag name spaces.
... If the program declares or defines an identifier in a context
in which it is reserved (other than as allowed by 7.1.4), or
defines a reserved identifier as a macro name, the behavior is
undefined.
POSIX has similar provisions.
> nightslugs:~/x/xorg/xserver(master)% g -r '^_' **/*.[ch] | grep -v _X_
> | wc -l
> 1857
That doesn't make it right.
> >> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> >> Signed-off-by: Egbert Eich <eich at freedesktop.org>
> >> ---
> >> 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) {
> >> case EV_SYN:
> >> switch (ev.code) {
> >> case SYN_REPORT:
> >> diff --git a/src/synaptics.c b/src/synaptics.c
> >> index f0a8269..31e147c 100644
> >> --- a/src/synaptics.c
> >> +++ b/src/synaptics.c
> >> @@ -920,18 +920,30 @@ DeviceControl(DeviceIntPtr dev, int mode)
> >> }
> >>
> >> static int
> >> -DeviceOn(DeviceIntPtr dev)
> >> +_DeviceOn(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--) {
> >> + 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;
> >>
> >> if (priv->proto_ops->DeviceOnHook &&
> >> !priv->proto_ops->DeviceOnHook(pInfo, &priv->synpara))
> >> @@ -956,11 +968,21 @@ DeviceOn(DeviceIntPtr dev)
> >> }
> >>
> >> xf86AddEnabledDevice(pInfo);
> >> - dev->public.on = TRUE;
> >>
> >> return Success;
> >> }
> >>
> >> +static int
> >> +DeviceOn(DeviceIntPtr dev)
> >> +{
> >> + int ret = _DeviceOn(dev->public.devicePrivate);
> >> +
> >> + if (ret == Success)
> >> + dev->public.on = TRUE;
> >> +
> >> + return ret;
> >> +}
> >> +
> >> static void
> >> SynapticsReset(SynapticsPrivate * priv)
> >> {
> >> @@ -995,9 +1017,8 @@ SynapticsReset(SynapticsPrivate * priv)
> >> }
> >>
> >> static int
> >> -DeviceOff(DeviceIntPtr dev)
> >> +_DeviceOff(InputInfoPtr pInfo)
> >> {
> >> - InputInfoPtr pInfo = dev->public.devicePrivate;
> >> SynapticsPrivate *priv = (SynapticsPrivate *) (pInfo->private);
> >> Bool rc = Success;
> >>
> >> @@ -1018,11 +1039,18 @@ DeviceOff(DeviceIntPtr dev)
> >> xf86CloseSerial(pInfo->fd);
> >> pInfo->fd = -1;
> >> }
> >> - dev->public.on = FALSE;
> >> return rc;
> >> }
> >>
> >> static int
> >> +DeviceOff(DeviceIntPtr dev)
> >> +{
> >> + int ret = _DeviceOff(dev->public.devicePrivate);
> >> + dev->public.on = FALSE;
> >> + return ret;
> >> +}
> >> +
> >> +static int
> >> DeviceClose(DeviceIntPtr dev)
> >> {
> >> Bool RetValue;
> >> @@ -1469,6 +1497,7 @@ ReadInput(InputInfoPtr pInfo)
> >>
> >> SynapticsResetTouchHwState(hw, FALSE);
> >>
> >> + priv->comm_read = 0;
> >> while (SynapticsGetHwState(pInfo, priv, hw)) {
> >> /* Semi-mt device touch slots do not track touches. When there is a
> >> * change in the number of touches, we must disregard the temporary
> >> @@ -1487,6 +1516,19 @@ ReadInput(InputInfoPtr pInfo)
> >> newDelay = TRUE;
> >> }
> >>
> >> + if (!priv->comm_read) {
> >> + /* strange callback, check the device and reconnect if needed */
> >> + if (!priv->reconnecting) {
> >> + priv->reconnecting = 1;
> >> + xf86Msg(X_WARNING, "%s: reconnecting device...\n", pInfo->name);
> >> + _DeviceOff(pInfo);
> >> + usleep(100*1000);
> >> + _DeviceOn(pInfo);
> >> + xf86Msg(X_WARNING, "%s: reconnection done\n", pInfo->name);
> >> + } else
> >> + priv->reconnecting = 0;
> >> + }
> >> +
> >> if (newDelay) {
> >> priv->timer_time = GetTimeInMillis();
> >> priv->timer = TimerSet(priv->timer, 0, delay, timerFunc, pInfo);
> >> diff --git a/src/synapticsstr.h b/src/synapticsstr.h
> >> index 428befa..98d6552 100644
> >> --- a/src/synapticsstr.h
> >> +++ b/src/synapticsstr.h
> >> @@ -204,6 +204,9 @@ struct _SynapticsPrivateRec {
> >> OsTimerPtr timer; /* for tap processing, etc */
> >>
> >> struct CommData comm;
> >> + int comm_read; /* for reconnection check */
> >> + int reconnecting; /* for reconnection check */
> >> + int retries;
> >>
> >> struct SynapticsHwState *local_hw_state; /* used in place of local hw state variables */
> >>
> >> --
> >> 1.8.1.4
> >>
> >> _______________________________________________
> >> xorg-devel at lists.x.org: X.Org development
> >> Archives: http://lists.x.org/archives/xorg-devel
> >> Info: http://lists.x.org/mailman/listinfo/xorg-devel
> >>
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
>
More information about the xorg-devel
mailing list