[PATCH driver/input/synaptics] For serial devices try reconnecting if device is wedged.

[Daniel Stone]

Hi,

On 29 July 2013 14:55, Mark Kettenis <mark.kettenis at xs4all.nl> wrote:
> 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.

Ah yes, you're right: I'd forgotten about the uppercase rule.  A dumb
rule, but a rule nonetheless.  The further provision is that you're
not allowed to use anything beginning with an underscore at all in the
global namespace.

So, _deviceOn would be fine here.

>> nightslugs:~/x/xorg/xserver(master)% g -r '^_' **/*.[ch] | grep -v _X_
>> | wc -l
>> 1857
>
> That doesn't make it right.

I'll happily ack patches to de-underscore XKB, which is the primary
offender here.

Cheers,
Daniel

>> >> 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
>>