All of lore.kernel.org
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Dennis Wassenberg <dennis.wassenberg@secunet.com>
Cc: linux-input@vger.kernel.org, linux-sound@vger.kernel.org,
	alsa-devel@alsa-project.org, lukas@wunner.de,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Andrew Duggan <aduggan@synaptics.com>,
	perex@perex.cz, david.henningsson@canonical.com,
	vinod.koul@intel.com, hui.wang@canonical.com,
	rafael.j.wysocki@intel.com, jikos@kernel.org
Subject: Re: [PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control
Date: Wed, 14 Sep 2016 09:03:50 +0200	[thread overview]
Message-ID: <s5h7fafvu7d.wl-tiwai@suse.de> (raw)
In-Reply-To: <6d21581c-cbc6-f148-77fd-2b468335f481@secunet.com>

On Wed, 14 Sep 2016 08:22:45 +0200,
Dennis Wassenberg wrote:
> 
> Hi Takashi,
> 
> at my first shot published to github I had an approach which makes thinkpad_acpi and hid-lenovo mutual exclusive (https://github.com/dwassenberg/linux/tree/development/hid-rmi). This approach has some issues. For example in case of thinkpad_acpi and hid-lenovo module is loaded (e.g. at Lenovo ThinkPad X1 Tablet) the symbol request to tpacpi_led_set is successful so the thinkpad_helper will decide in favor of thinkpad_acpi. So I need to add something which checks the return code of tpacpi_led_set. If -ENODEV is returned it can try hid-lenovo.
> 
> But the reason why I made thinkpad_acpi and hid-lenovo work in parallel was that if it is mutual exclusive there is no possibility to control LEDs of external Lenovo USB keyboards which are connected to a Thinkpad. Currently (including the other patches) there is only the X1 Tablet Cover LEDs which is controllable from external code. But the hid-lenovo driver is able to handle different Lenovo USB Keyboards.
> 
> - ThinkPad USB Keyboard with TrackPoint (tpkbd)
> - ThinkPad Compact Bluetooth Keyboard with TrackPoint (cptkbd)
> - ThinkPad Compact USB Keyboard with TrackPoint (cptkbd)
> - ThinkPad X1 Cover USB Keyboard with TrackPoint and Touchpad (tpx1cover)
> 
> These keyboards have these LEDs too. For some of these keyboards a LED control is implemented inside hid-lenovo too. If thinkpad_acpi and hid-lenovo is mutual exclusive it is impossible to let the thinkpad_helper control the LEDs of these external USB keyboards if they are connected to a thinkpad. I don't know if anyone will do this, but I don't want to take the possibility to do such thinks :)
> 
> So.. if there is the decision (from your side) that this should be mutual exclusive I will prepare a v2 which the mutual exclusive approach I described at the top.

OK, now understood.  But still make sure that it works even with one
of kconfigs is disabled.


thanks,

Takashi

WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de>
To: Dennis Wassenberg <dennis.wassenberg@secunet.com>
Cc: linux-input@vger.kernel.org, linux-sound@vger.kernel.org,
	alsa-devel@alsa-project.org, lukas@wunner.de,
	Benjamin Tissoires <benjamin.tissoires@redhat.com>,
	Andrew Duggan <aduggan@synaptics.com>,
	perex@perex.cz, david.henningsson@canonical.com,
	vinod.koul@intel.com, hui.wang@canonical.com,
	rafael.j.wysocki@intel.com, jikos@kernel.org
Subject: Re: [PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control
Date: Wed, 14 Sep 2016 07:03:50 +0000	[thread overview]
Message-ID: <s5h7fafvu7d.wl-tiwai@suse.de> (raw)
In-Reply-To: <6d21581c-cbc6-f148-77fd-2b468335f481@secunet.com>

On Wed, 14 Sep 2016 08:22:45 +0200,
Dennis Wassenberg wrote:
> 
> Hi Takashi,
> 
> at my first shot published to github I had an approach which makes thinkpad_acpi and hid-lenovo mutual exclusive (https://github.com/dwassenberg/linux/tree/development/hid-rmi). This approach has some issues. For example in case of thinkpad_acpi and hid-lenovo module is loaded (e.g. at Lenovo ThinkPad X1 Tablet) the symbol request to tpacpi_led_set is successful so the thinkpad_helper will decide in favor of thinkpad_acpi. So I need to add something which checks the return code of tpacpi_led_set. If -ENODEV is returned it can try hid-lenovo.
> 
> But the reason why I made thinkpad_acpi and hid-lenovo work in parallel was that if it is mutual exclusive there is no possibility to control LEDs of external Lenovo USB keyboards which are connected to a Thinkpad. Currently (including the other patches) there is only the X1 Tablet Cover LEDs which is controllable from external code. But the hid-lenovo driver is able to handle different Lenovo USB Keyboards.
> 
> - ThinkPad USB Keyboard with TrackPoint (tpkbd)
> - ThinkPad Compact Bluetooth Keyboard with TrackPoint (cptkbd)
> - ThinkPad Compact USB Keyboard with TrackPoint (cptkbd)
> - ThinkPad X1 Cover USB Keyboard with TrackPoint and Touchpad (tpx1cover)
> 
> These keyboards have these LEDs too. For some of these keyboards a LED control is implemented inside hid-lenovo too. If thinkpad_acpi and hid-lenovo is mutual exclusive it is impossible to let the thinkpad_helper control the LEDs of these external USB keyboards if they are connected to a thinkpad. I don't know if anyone will do this, but I don't want to take the possibility to do such thinks :)
> 
> So.. if there is the decision (from your side) that this should be mutual exclusive I will prepare a v2 which the mutual exclusive approach I described at the top.

OK, now understood.  But still make sure that it works even with one
of kconfigs is disabled.


thanks,

Takashi

  reply	other threads:[~2016-09-14  7:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-12 10:47 [PATCH 4/4] hda: thinkpad_helper: Add support for hid-lenovo LED control Dennis Wassenberg
2016-09-12 10:47 ` Dennis Wassenberg
2016-09-12 12:38 ` Takashi Iwai
2016-09-12 12:38   ` Takashi Iwai
2016-09-14  6:22   ` Dennis Wassenberg
2016-09-14  6:22     ` Dennis Wassenberg
2016-09-14  7:03     ` Takashi Iwai [this message]
2016-09-14  7:03       ` Takashi Iwai
2016-12-09 15:06 Dennis Wassenberg
2016-12-09 15:06 ` Dennis Wassenberg
2016-12-19 10:09 ` Jiri Kosina
2016-12-19 10:09   ` Jiri Kosina
2016-12-19 10:44   ` Dennis Wassenberg
2016-12-19 10:44     ` Dennis Wassenberg
2016-12-19 10:53     ` Jiri Kosina
2016-12-19 10:53       ` Jiri Kosina
2016-12-19 11:05       ` Dennis Wassenberg
2016-12-19 11:05         ` Dennis Wassenberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5h7fafvu7d.wl-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=aduggan@synaptics.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=benjamin.tissoires@redhat.com \
    --cc=david.henningsson@canonical.com \
    --cc=dennis.wassenberg@secunet.com \
    --cc=hui.wang@canonical.com \
    --cc=jikos@kernel.org \
    --cc=linux-input@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=lukas@wunner.de \
    --cc=perex@perex.cz \
    --cc=rafael.j.wysocki@intel.com \
    --cc=vinod.koul@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.