Re: [PATCH 2/2] input: Add LED support to Synaptics device

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Wed, Apr 21, 2010 at 08:31:15AM +0200, Takashi Iwai wrote:
> At Tue, 20 Apr 2010 22:43:03 -0700,
> Dmitry Torokhov wrote:
> > 
> > On Mon, Apr 19, 2010 at 12:44:08PM +0200, Takashi Iwai wrote:
> > > At Fri, 16 Apr 2010 10:00:20 +0200,
> > > I wrote:
> > > > 
> > > > At Thu, 15 Apr 2010 21:12:18 +0200,
> > > > Pavel Machek wrote:
> > > > > 
> > > > > Hi!
> > > > > 
> > > > > > The new Synaptics devices have an LED on the top-left corner.
> > > > > > This is controlled via the command 0x0a with parameters 0x88 or 0x10.
> > > > > > 
> > > > > > The detection of the LED isn't clear yet.  It should have been the new
> > > > > > capability bits that indicate the presence, but on real machines, it
> > > > > > doesn't fit.  So, for the time being, the driver checks the product id
> > > > > > in the ext capability bits and assumes that LED exists on the known
> > > > > > devices.
> > > > > > 
> > > > > > The support of LED is controlled via a normal input event with EV_LED
> > > > > > bit mask.  It supports LED_MUTE bit.  X driver can detect the LED
> > > > > > support by checking these bits.
> > > > > 
> > > > > Could we use generic LED API for this?
> > > > 
> > > > Yeah, actually I started implementing with LED ADI at first.
> > > > 
> > > > But, then it turned out to be that it's much easier to use the
> > > > existing LED input bits since this LED is really tightly coupled with
> > > > the synaptics input device.  An individual LED device makes hard to
> > > > find out the corresponding input device.
> > > > 
> > > > If we assume there is only one synaptics and only one synaptics-LED
> > > > device, then yes, the situation can be a bit easier, though.
> > > > 
> > > > > It is not really 'mute' led after all...
> > > > 
> > > > If the problem is the misuse of LED_MUTE bit, how about adding a new
> > > > LED bit, e.g. LED_TOUCHPAD?
> > > 
> > > The revised patch with an addition of LED_TOUCHPAD is below.
> > > 
> > 
> > Sorry Takashi, but I will not add any new LED types to input. Even
> > current input LEDs are going to be accessible thought standard LED
> > framework (even though I have not apploed Samuel's patch yet I do think
> > it would move kernel in the right direction).
> 
> Hrm, OK, we can live in other way, too.  For touchpad, it wouldn't be
> a big problem because it's likely a single device.
> 
> But, how can we link a led class device and another device, in
> general?

I think this question is to Richard, I am a bit fuzzy on LED subsystem.
But in general I'd expect LED have touchpad input device as parent.

> 
> Also, another remaining question is the lifetime of led device.
> The mouse device tends to be re-assigned often, e.g. at each time you
> suspend/hibernate.

We go to a great lengths to properly resume the device keeping the same
input_dev structure and the same event node (for historical reasons
really, nowadays X is hotplug aware so we could do without), do you see
it being recreated often?

>  Should led device also be removed and revived at
> each time, or should we keep it and just ignore event?  If we remove
> it, how can we avoid race?

I am not sure what race you see - psmouse protocol switch should not
race so if you remove LED there you shoudl be race-free. If you see
something racing I am definitely interested in hearing about it.

Thanks!

-- 
Dmitry