[alsa-devel] Questions about thinkpad_helper

[Takashi Iwai]

[Changed Cc to proper alsa-devel ML]

On Tue, 30 Aug 2016 12:02:56 +0200,
Dennis Wassenberg wrote:
> 
> Hi all,
> 
> I wrote a driver which handles the function / special keys for
> ThinKeyboard of Lenovo ThinkPad X1 Tablet 2016 including LED control.
> This Keyboard is connected via USB to the Tablet. I added the driver to
> hid-lenovo.c file because there are HID drivers included for different
> Lenovo devices.
> 
> For example this driver is able to handle LED control for mute and mic
> mute keys. Now I want to make the other direction working. If someone
> sets mute or mic mute at amixer, I want to control the mute and mic mute
> LEDs. At other Thinkpad devices this is done by thinkpad_helper.c. The
> thinkpad_helper let the thinkpad_apci driver to control the LEDs.
> 
> Now I think about how to make this working for Lenovo Thinkpad X1 Tablet
> 2016. There it is not possible to use the thinkpad_acpi driver. Instead
> we have to use the hid-lenovo driver to make this working.
> 
> Should I add this to thinkpad_helper because this is a thinkpad device
> too or should I make a new helper? Can I use the same API than
> thinkpad_acpi (static int (*led_set_func)(int, bool)) or should I make a
> new one (e.g. in combination with LED triggers)?
> 
> For the first shot I made something at
> https://github.com/dwassenberg/linux/tree/development/hid-rmi
> (dc05de0add955775434d18d4fe096ea78d1e63bd..5a88c66f2ae8db04147bb75ecf84a4f52e0da1df)
> this shot is based on using the thinkpad_helper and the same API. Please
> tell me your opinion.
> 
> Maybe one additional remark. In case of using the USB connected
> ThinKeyboard there is one additional effort to make this working at the
> initial state if the Tablet is finished booting and after that the
> ThinKeyboard is connected. So either there is an additional trigger
> needed which tells the helper to publish the current mute and mic mute
> state when the keyboard is connected. Or an other approach is to update
> the hid-lenovo driver even when this keyboard is not connected such that
> the hid-lenovo driver always knows the current state and is able to set
> this if the keyboard is connected. Or is there there an other solution?

I think your changes are OK.  A slight concern I found while looking
quickly through the git commits is the cosmetic build issues, either
only one of CONFIG_THINKPAD_ACPI and CONFIG_HID_LENOVO is enabled.
But other than that, it looks straightforward.

Of course, another option would be leave everything to user-space.
But I don't think it would work fluently on the current desktop, even
in near future.  So considering practically, we'll likely go to the
kernel hook way, I suppose.


thanks,

Takashi