On Wed, Oct 13, 2010 at 08:57:37AM +0200, Takashi Iwai wrote: > At Wed, 13 Oct 2010 15:56:38 +1000, > Peter Hutterer wrote: > > > > On Wed, Oct 13, 2010 at 07:47:32AM +0200, Takashi Iwai wrote: > > > At Wed, 13 Oct 2010 13:29:21 +1000, > > > Peter Hutterer wrote: > > > > > > > > On Fri, Oct 08, 2010 at 07:22:27PM +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 a sysfs file /sys/class/leds/psmouse::synaptics exists, the > > > > > driver > > > > > assumes it supports the embeded LED control. > > > > > > > > > > The LED can be controlled via new properties, "Synaptics LED" and > > > > > "Synaptics LED Status". > > > > > > > > > > Signed-off-by: Takashi Iwai <[email protected]> > > > > > --- > > > > > include/synaptics-properties.h | 6 ++++++ > > > > > man/synaptics.man | 9 +++++++++ > > > > > src/eventcomm.c | 32 > > > > > +++++++++++++++++++++++++++++++- > > > > > src/properties.c | 15 +++++++++++++++ > > > > > src/synapticsstr.h | 2 ++ > > > > > src/synproto.h | 1 + > > > > > tools/synclient.c | 1 + > > > > > 7 files changed, 65 insertions(+), 1 deletions(-) > > > > > > > > > > diff --git a/include/synaptics-properties.h > > > > > b/include/synaptics-properties.h > > > > > index 9c6a2ee..dd7e259 100644 > > > > > --- a/include/synaptics-properties.h > > > > > +++ b/include/synaptics-properties.h > > > > > @@ -155,4 +155,10 @@ > > > > > /* 32 bit, 4 values, left, right, top, bottom */ > > > > > #define SYNAPTICS_PROP_AREA "Synaptics Area" > > > > > > > > > > +/* 8 bit (BOOL, read-only), has_led */ > > > > > +#define SYNAPTICS_PROP_LED "Synaptics LED" > > > > > + > > > > > > > > this should be added to the "Synaptics Capabilities" property, there's > > > > no > > > > need for a separate property. > > > > > > OK, sounds sensible. > > > > > > > I guess sooner or later we'll see non-clickpad > > > > devices come out with leds too, making this a generic capability like > > > > two-finger tapping and similar. > > > > > > There are already lots of laptops with an LED but also separate > > > buttons, indeed. > > > > > > > > @@ -278,6 +280,9 @@ InitDeviceProperties(InputInfoPtr pInfo) > > > > > values[2] = para->area_top_edge; > > > > > values[3] = para->area_bottom_edge; > > > > > prop_area = InitAtom(pInfo->dev, SYNAPTICS_PROP_AREA, 32, 4, > > > > > values); > > > > > + > > > > > + prop_led = InitAtom(local->dev, SYNAPTICS_PROP_LED, 8, 1, > > > > > ¶->has_led); > > > > > + prop_led_status = InitAtom(local->dev, > > > > > SYNAPTICS_PROP_LED_STATUS, 8, 1, ¶->led_status); > > > > > > > > the prop_led_status atom should only be initialized if the touchpad > > > > actually > > > > has a led, please make this conditional on the has_led bit. > > > > > > So, you mean that prop_led should be NULL when no LED exists, thus > > > the querying this property would lead to an error? > > > > yeah, the property prop_led_status simply won't be available and return a > > BadAtom to a client that tries to modify it. this is more explanatory than > > having the property but not doing anything with it if the device doesn't > > have a led. > > > > fwiw, note that all prop_* are of type Atom, which is just an int. And since > > 0 (None) isn't a property, we don't need any extra checks for it in the > > driver, the server filters for us. > > Alright. > > While we are at it. Any reason all these prop_* aren't static?
not that I know of, probably just because I forget in the original patch and then it just kept being the same since. Cheers, Peter _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
