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 <ti...@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. > > > +. > > .LP > > A tap event happens when the finger is touched and released in a time > > interval shorter than MaxTapTime, and the touch and release > > diff --git a/src/eventcomm.c b/src/eventcomm.c > > index ca99f3a..292e64e 100644 > > --- a/src/eventcomm.c > > +++ b/src/eventcomm.c > > @@ -179,6 +179,39 @@ static void event_query_clickpad(LocalDevicePtr local) > > } > > } > > > > +/* LED support: the kernel driver should give EV_LED and LED_MUTE bits */ > > +static void event_query_led(LocalDevicePtr local) > > +{ > > + SynapticsPrivate *priv = (SynapticsPrivate *)local->private; > > + unsigned long evbits[NBITS(EV_MAX)] = {0}; > > + int rc; > > + > > + priv->has_led = FALSE; > > + > > + SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(0, sizeof(evbits)), evbits)); > > + if (TEST_BIT(EV_LED, evbits)) { > > + unsigned long ledbits[NBITS(LED_MAX)] = {0}; > > + SYSCALL(rc = ioctl(local->fd, EVIOCGBIT(EV_LED, sizeof(ledbits)), > > ledbits)); > > + if (TEST_BIT(LED_MUTE, ledbits)) { > > + xf86Msg(X_INFO, "%s: has LED control\n", local->name); > > + priv->has_led = TRUE; > > + } > > + } > > +} > > + > > +static void EventUpdateHardware(LocalDevicePtr local) > > +{ > > + SynapticsPrivate *priv = (SynapticsPrivate *)local->private; > > + if (priv->has_led) { > > + struct input_event ev; > > + gettimeofday(&ev.time, NULL); > > + ev.type = EV_LED; > > + ev.code = LED_MUTE; > > + ev.value = priv->synpara.touchpad_off != 0; > > + (void)write(local->fd, &ev, sizeof(ev)); > > skip the (void) here please. a return value check would be nice in case of > error. Heh, I'm a lazy guy :) > > + } > > +} > > + > > /* Query device for axis ranges */ > > static void > > event_query_axis_ranges(LocalDevicePtr local) > > @@ -268,6 +301,7 @@ event_query_axis_ranges(LocalDevicePtr local) > > } > > > > event_query_clickpad(local); > > + event_query_led(local); > > } > > > > static Bool > > @@ -421,6 +455,8 @@ EventReadHwState(LocalDevicePtr local, > > break; > > } > > break; > > + case EV_LED: > > + return TRUE; > > } > > } > > return FALSE; > > @@ -502,5 +538,6 @@ struct SynapticsProtocolOperations > > event_proto_operations = { > > EventQueryHardware, > > EventReadHwState, > > EventAutoDevProbe, > > - EventReadDevDimensions > > + EventReadDevDimensions, > > + EventUpdateHardware, > > }; > > diff --git a/src/properties.c b/src/properties.c > > index a579479..57f11f0 100644 > > --- a/src/properties.c > > +++ b/src/properties.c > > @@ -86,6 +86,7 @@ Atom prop_resolution = 0; > > Atom prop_area = 0; > > Atom prop_touch_button_area = 0; > > Atom prop_touch_button_sense = 0; > > +Atom prop_led_double_tap = 0; > > > > static Atom > > InitAtom(DeviceIntPtr dev, char *name, int format, int nvalues, int > > *values) > > @@ -280,6 +281,9 @@ InitDeviceProperties(LocalDevicePtr local) > > prop_touch_button_area = InitAtom(local->dev, > > SYNAPTICS_PROP_TOUCH_BUTTON_AREA, 8, 1, ¶->touch_button_area); > > > > prop_touch_button_sense = InitAtom(local->dev, > > SYNAPTICS_PROP_TOUCH_BUTTON_SENSE, 8, 1, ¶->touch_button_sense); > > + > > + prop_led_double_tap = InitAtom(local->dev, > > SYNAPTICS_PROP_LED_DOUBLE_TAP, 32, 1, ¶->led_double_tap); > > + > > } > > > > int > > @@ -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? > > } else if (property == prop_guestmouse) > > { > > @@ -661,10 +670,27 @@ SetProperty(DeviceIntPtr dev, Atom property, > > XIPropertyValuePtr prop, > > return BadMatch; > > > > para->touch_button_sense = *(BOOL*)prop->data; > > + } else if (property == prop_led_double_tap) > > + { > > + if (prop->size != 1 || prop->format != 32 || prop->type != > > XA_INTEGER) > > + return BadMatch; > > + > > + para->led_double_tap = *(INT32*)prop->data; > > } > > > > return Success; > > } > > > > +void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off) > > +{ > > + uint8_t val; > > please use CARD8 here for consistency with the rest of the code. OK. > > + > > + if (!prop_off) > > + return; > > + val = off; > > + XIChangeDeviceProperty(dev, prop_off, XA_INTEGER, 8, > > + PropModeReplace, 1, &val, FALSE); > > +} > > + > > #endif > > > > diff --git a/src/synaptics.c b/src/synaptics.c > > index f701074..86001e2 100644 > > --- a/src/synaptics.c > > +++ b/src/synaptics.c > > @@ -134,6 +134,7 @@ static void CalculateScalingCoeffs(SynapticsPrivate > > *priv); > > void InitDeviceProperties(LocalDevicePtr local); > > int SetProperty(DeviceIntPtr dev, Atom property, XIPropertyValuePtr prop, > > BOOL checkonly); > > +void SynapticsToggleOffProperty(DeviceIntPtr dev, Bool off); > > #endif > > > > InputDriverRec SYNAPTICS = { > > @@ -534,6 +535,7 @@ static void set_default_parameters(LocalDevicePtr local) > > pars->resolution_vert = xf86SetIntOption(opts, "VertResolution", > > vertResolution); > > pars->touch_button_area = xf86SetIntOption(opts, "TouchButtonArea", > > 20); > > pars->touch_button_sense = xf86SetBoolOption(opts, "TouchButtonSense", > > FALSE); > > + pars->led_double_tap = xf86SetIntOption(opts, "LEDDoubleTap", 400); > > > > /* Warn about (and fix) incorrectly configured TopEdge/BottomEdge > > parameters */ > > if (pars->top_edge > pars->bottom_edge) { > > @@ -766,6 +768,11 @@ DeviceOn(DeviceIntPtr dev) > > } > > > > xf86AddEnabledDevice(local); > > + > > + /* update LED */ > > + if (priv->proto_ops && priv->proto_ops->UpdateHardware) > > + priv->proto_ops->UpdateHardware(local); > > + > > dev->public.on = TRUE; > > > > return Success; > > @@ -2054,6 +2061,60 @@ HandleClickWithFingers(SynapticsParameters *para, > > struct SynapticsHwState *hw) > > } > > } > > > > +/* clicpad button toggle point: > > + * some devices have a LED at the upper-left corner, and double-tapping it > > + * toggles the touchpad enable/disable > > + */ > > +static int > > +HandleToggleLED(LocalDevicePtr local, struct SynapticsHwState *hw, int > > finger) > > +{ > > + SynapticsPrivate *priv = (SynapticsPrivate *) (local->private); > > + SynapticsParameters *para = &priv->synpara; > > + int click_led_x = (priv->maxx - priv->minx) * 1 / 10 + priv->minx; > > + int click_led_y = priv->maxy / 8 + priv->miny; > > please use defines for the 1/10 and 1/8 instead of the numbers being hidden > in the code. OK. > I guess this doesn't change that much so we might get by > without having configuration options for this but it should at least be a > define. Yeah, that's why I didn't give options or properties, too. thanks, Takashi _______________________________________________ xorg-devel@lists.x.org: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel