[PATCH 03/18] Add the embedded LED support

[Peter Hutterer]

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 <tiwai at suse.de>
> > > ---
> > >  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, &para->has_led);
> > > +    prop_led_status = InitAtom(local->dev, SYNAPTICS_PROP_LED_STATUS, 8, 1, &para->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.

Cheers,
  Peter