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? Takashi _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
