[PATCH 2/2] Support LED and tap-on-LED feature
Takashi Iwai
tiwai at suse.de
Wed Apr 21 00:18:12 PDT 2010
At Wed, 21 Apr 2010 16:53:14 +1000,
Peter Hutterer wrote:
>
> On Wed, Apr 21, 2010 at 08:26:41AM +0200, Takashi Iwai wrote:
> > At Wed, 21 Apr 2010 14:16:29 +1000,
> > Peter Hutterer wrote:
> > >
> > > On Thu, Apr 15, 2010 at 06:12:11PM +0200, Takashi Iwai wrote:
> > > > This patch adds the control of the embedded LED on the top-left corner
> > > > of new Synaptics devices. For LED control, it requires the patch to
> > > > Linux synaptics input driver,
> > > > https://patchwork.kernel.org/patch/92434/
> > > >
> > > > When evdev reports the presense of LED and LED_MUTE bits, the driver
> > > > assumes it supports the embeded LED control. This corresponds to the
> > > > touchpad-off state. When touchpad is disabled, LED is turned on.
> > > >
> > > > For linking between the touchpad state and the LED state, a new callback
> > > > UpdateHardware is introduced. Only eventcomm.c supports this (naturally).
> > > >
> > > > A new feature for the LED-equipped device is that user can double-tap
> > > > on the LED to toggle the touchpad state on the fly. This is also linked
> > > > with the touchpad-off state.
> > > >
> > > > There is a new parameter for controlling the LED double-tap behavior, too.
> > > > It specifies the double-tap time. Passing zero disables the double-tap
> > > > feature.
> > > >
> > > > Signed-off-by: Takashi Iwai <tiwai at suse.de>
> > > > ---
> > > > include/synaptics-properties.h | 3 ++
> > > > man/synaptics.man | 13 +++++++
> > > > src/eventcomm.c | 39 +++++++++++++++++++++-
> > > > src/properties.c | 26 ++++++++++++++
> > > > src/synaptics.c | 73 ++++++++++++++++++++++++++++++++++++++-
> > > > src/synapticsstr.h | 6 +++
> > > > src/synproto.h | 1 +
> > > > tools/synclient.c | 1 +
> > > > 8 files changed, 159 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/synaptics-properties.h b/include/synaptics-properties.h
> > > > index c77afd3..56c3e1d 100644
> > > > --- a/include/synaptics-properties.h
> > > > +++ b/include/synaptics-properties.h
> > > > @@ -161,4 +161,7 @@
> > > > /* 8 bit (BOOL) */
> > > > #define SYNAPTICS_PROP_TOUCH_BUTTON_SENSE "Synaptics Touch Button Sense"
> > > >
> > > > +/* 32 bit */
> > > > +#define SYNAPTICS_PROP_LED_DOUBLE_TAP "Synaptics LED Double Tap"
> > > > +
> > > > #endif /* _SYNAPTICS_PROPERTIES_H_ */
> > > > diff --git a/man/synaptics.man b/man/synaptics.man
> > > > index f92ea0c..baa7e8f 100644
> > > > --- a/man/synaptics.man
> > > > +++ b/man/synaptics.man
> > > > @@ -530,6 +530,19 @@ clicking the button. On the other hand, this reduces the available
> > > > space on the touchpad. The default is on.
> > > > Property: "Synaptics Touch Button Sense"
> > > > .
> > > > +.TP
> > > > +.BI "Option \*qLEDDoubleTap\*q \*q" integer \*q
> > > > +.
> > > > +The double-tap time for toggling the touchpad-control on the top-left
> > > > +corner LED.
> > > > +.
> > > > +Some devices have an LED on the top-left corner to indicate the
> > > > +touchpad state. User can double-tap on the LED to toggle the touchpad
> > > > +state. This option controls the double-tap time in milli-seconds.
> > > > +When it's zero, the LED-tapping feature is disabled. The default
> > > > +value is 400.
> > > > +Property: "Synaptics LED Double Tap"
> > >
> > > a few questions to the principle:
> > > - any specific reason this is a double-tap?
> >
> > A single-tap is often too sensitive. User may toggle it mistakenly,
> > and wonders what happens suddenly. Also, Windows driver uses the
> > double-tap, too.
> >
> > > - the corner might be used for the same feature even if there is no LED
> > > present, right?
> >
> > The check is done only when has_led flag is set, so it's exclusive for
> > the device with LED control.
> >
> > > as I read this patch, the LED updating could be split out
> > > from the actual gesture of disabling the touchpad by clicking into the
> > > corner.
> >
> > In general yes, the double-tapping to toggle touchpad is an individual
> > feature. But this becomes intuitive when coupled with LED, thus
> > rather somewhat special for LED-equipped device, I suppose.
>
> gnome has a feature that allows you to disable the touchpad hitting Fn+F8,
> and it then displays a popup box to notify the user of this. The same could
> be done for this feature by monitoring the off property, even if there's no
> LED present. So I think there are use-cases that warrant this feature
> without the tight LED integration.
Right, but the positioned double-tapping (that is featured on Windows)
seems coupled with LED, I suppose.
> > > > @@ -496,6 +500,11 @@ SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop,
> > > > return BadValue;
> > > >
> > > > para->touchpad_off = off;
> > > > + if (!checkonly) {
> > > > + if (priv->proto_ops && priv->proto_ops->UpdateHardware)
> > > > + priv->proto_ops->UpdateHardware(local);
> > > > + }
> > > > +
> > >
> > > indentation.
> >
> > What is wrong here?
>
> the property code is one of the few places where the indentation is
> consistent (mainly because I wrote most of it :). Please use spaces here
> instead of tabs, which generally is my preference wherever in doubt.
OK. I'm a kernel guy, so I prefer differently, but hey, it's just a
personal preference :)
thanks,
Takashi
More information about the xorg-devel
mailing list