* [PATCH] HID: corsair: Add support for the GLAIVE RGB gaming mouse
From: Guus Sliepen @ 2018-02-10 17:12 UTC (permalink / raw)
To: linux-input, linux-kernel, jikos; +Cc: Guus Sliepen
This mouse sold by Corsair as the GLAIVE RGB gaming mouse has the same
problem with its HID reports as the Scimitar PRO RGB, so reuse the
same fix for the GLAIVE RGB.
Signed-off-by: Guus Sliepen <guus@sliepen.org>
---
drivers/hid/hid-corsair.c | 16 ++++++++++------
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 2 ++
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/hid/hid-corsair.c b/drivers/hid/hid-corsair.c
index 9ba5d98a1..6c9c8fe1b 100644
--- a/drivers/hid/hid-corsair.c
+++ b/drivers/hid/hid-corsair.c
@@ -673,7 +673,7 @@ static int corsair_input_mapping(struct hid_device *dev,
}
/*
- * The report descriptor of Corsair Scimitar RGB Pro gaming mouse is
+ * The report descriptor of some of the Corsair gaming mice is
* non parseable as they define two consecutive Logical Minimum for
* the Usage Page (Consumer) in rdescs bytes 75 and 77 being 77 0x16
* that should be obviousy 0x26 for Logical Magimum of 16 bits. This
@@ -681,7 +681,8 @@ static int corsair_input_mapping(struct hid_device *dev,
* Minimum being larger than Logical Maximum.
*
* This driver fixes the report descriptor for:
- * - USB ID b1c:1b3e, sold as Scimitar RGB Pro Gaming mouse
+ * - USB ID 1b1c:1b34, sold as GLAIVE RGB Gaming mouse
+ * - USB ID 1b1c:1b3e, sold as Scimitar RGB Pro Gaming mouse
*/
static __u8 *corsair_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
@@ -691,13 +692,14 @@ static __u8 *corsair_mouse_report_fixup(struct hid_device *hdev, __u8 *rdesc,
if (intf->cur_altsetting->desc.bInterfaceNumber == 1) {
/*
- * Corsair Scimitar RGB Pro report descriptor is broken and
- * defines two different Logical Minimum for the Consumer
- * Application. The byte 77 should be a 0x26 defining a 16
- * bits integer for the Logical Maximum but it is a 0x16
+ * Corsair GLAIVE RGB and Scimitar RGB Pro report descriptor is
+ * broken and defines two different Logical Minimum for the
+ * Consumer Application. The byte 77 should be a 0x26 defining
+ * a 16 bits integer for the Logical Maximum but it is a 0x16
* instead (Logical Minimum)
*/
switch (hdev->product) {
+ case USB_DEVICE_ID_CORSAIR_GLAIVE_RGB:
case USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB:
if (*rsize >= 172 && rdesc[75] == 0x15 && rdesc[77] == 0x16
&& rdesc[78] == 0xff && rdesc[79] == 0x0f) {
@@ -715,6 +717,8 @@ static const struct hid_device_id corsair_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90),
.driver_data = CORSAIR_USE_K90_MACRO |
CORSAIR_USE_K90_BACKLIGHT },
+ { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR,
+ USB_DEVICE_ID_CORSAIR_GLAIVE_RGB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR,
USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB) },
{}
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd..a099e5194 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -291,6 +291,7 @@
#define USB_DEVICE_ID_CORSAIR_K70RGB 0x1b13
#define USB_DEVICE_ID_CORSAIR_STRAFE 0x1b15
#define USB_DEVICE_ID_CORSAIR_K65RGB 0x1b17
+#define USB_DEVICE_ID_CORSAIR_GLAIVE_RGB 0x1b34
#define USB_DEVICE_ID_CORSAIR_K70RGB_RAPIDFIRE 0x1b38
#define USB_DEVICE_ID_CORSAIR_K65RGB_RAPIDFIRE 0x1b39
#define USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB 0x1b3e
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5f6035a5c..7203eba3c 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -62,6 +62,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K70R), HID_QUIRK_NO_INIT_REPORTS },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K95RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_M65RGB), HID_QUIRK_NO_INIT_REPORTS },
+ { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_GLAIVE_RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
@@ -317,6 +318,7 @@ static const struct hid_device_id hid_have_special_driver[] = {
#endif
#if IS_ENABLED(CONFIG_HID_CORSAIR)
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_K90) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_GLAIVE_RGB) },
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_SCIMITAR_PRO_RGB) },
#endif
#if IS_ENABLED(CONFIG_HID_CP2112)
--
2.16.1
^ permalink raw reply related
* Re: Software evolution around “checkpatch.pl”?
From: Joe Perches @ 2018-02-10 17:42 UTC (permalink / raw)
To: Jonathan Cameron
Cc: SF Markus Elfring, linux-iio-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA, Jonathan Cameron,
Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20180210155734.708d01b6@archlinux>
On Sat, 2018-02-10 at 15:57 +0000, Jonathan Cameron wrote:
> On Sat, 10 Feb 2018 06:59:43 -0800
> Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org> wrote:
>
> > On Sat, 2018-02-10 at 14:53 +0000, Jonathan Cameron wrote:
> > > While it would be great to improve checkpatches false
> > > positive rate, it's very nature as a string matcher makes
> > > this hard.
> >
> > true.
> >
> > what are the false positives you see?
> >
>
> This particular case is only 'sort of' a false positive
> in the warning that a message printed on a memory allocation
> failure 'may' not add any information over the generic case.
Right. So it's not a 'false positive' at all.
Are there any actual 'false positives' you know of?
> Very hard to judge on whether it is useful to know more than
> an allocation failed somewhere or not.
>
> Message makes this clear:
> > “WARNING: Possible unnecessary 'out of memory' message”
> > (from the script “checkpatch.pl”)
>
> We also have the balance between any changes to existing code
> adding 'some' maintenance overhead vs changing this stuff
> in a new driver - which is what checkpatch is really intended
> for.
There's almost zero maintenance overhead here.
The time it takes for the back and forth
replies is likely larger.
> So I think checkpatch is striking the right balance here in
> how it warns. Obviously if it could assess the text
> and come to an informed decision that would be great but
> we are some way from that ;)
The 'informed' bit is difficult as it is mostly
a political problem.
This particular message really is unnecessary as
the generic dump_stack on any normal allocation
(ie: without __GFP_WARN) already emits location
specific information.
Removing these messages can help make the kernel
image smaller and thereby help make these OOM
messages a tiny bit less likely.
I just wish Markus would improve his consistently
terrible commit messages that just restate the
action being done and detail _why_ a particular
thing _should_ be done.
His acceptance rate would improve as many of these
back and forth replies for what trivialities he
posts as patches would be minimized.
^ permalink raw reply
* 4.10 regression: d79e141c1c6ea7cb70c169971d522b88c8d5b419 breaks PS/2 barcode readers
From: Nikola Ciprich @ 2018-02-10 18:04 UTC (permalink / raw)
To: linux-input; +Cc: nikola.ciprich, dmitry.torokhov
[-- Attachment #1: Type: text/plain, Size: 1191 bytes --]
Hi,
after switching to 4.14 from 4.4 we noticed LS2208 PS/2 barcode stopped
working for us (on Lenovo desktops equipped with USB keyboards). If I plug
PS/2 keyboard, reader works.. (it's cable works as PS/2 splitter).
Digging deeper into this, I noticed kernel stopped probing PS/2 ports directly,
which seems to be caused by d79e141c1c6ea7cb70c169971d522b88c8d5b419,
reverting
+ if (x86_platform.legacy.i8042 !=
+ X86_LEGACY_I8042_EXPECTED_PRESENT)
+ return -ENODEV;
part makes reader working again (without PS/2 keyboard plugged), as well
as using i8042.nopnp.
My question is, what might best solution for that be? Could this be
solved by some quirk? I haven't found any similar to get some idea..
If I could provide some more details, please let me know..
BR
nik
--
-------------------------------------
Ing. Nikola CIPRICH
LinuxBox.cz, s.r.o.
28. rijna 168, 709 00 Ostrava
tel.: +420 591 166 214
fax: +420 596 621 273
mobil: +420 777 093 799
www.linuxbox.cz
mobil servis: +420 737 238 656
email servis: servis@linuxbox.cz
-------------------------------------
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: Software evolution around “checkpatch.pl”?
From: SF Markus Elfring @ 2018-02-10 18:30 UTC (permalink / raw)
To: Joe Perches, Jonathan Cameron, linux-iio, linux-input
Cc: Jonathan Cameron, Hartmut Knaack, Jiri Kosina, Lars-Peter Clausen,
Peter Meerwald-Stadler, Srinivas Pandruvada, LKML,
kernel-janitors
In-Reply-To: <1518284577.16865.8.camel@perches.com>
>> So I think checkpatch is striking the right balance here in
>> how it warns. Obviously if it could assess the text
>> and come to an informed decision that would be great but
>> we are some way from that ;)
>
> The 'informed' bit is difficult as it is mostly a political problem.
I find such a view very interesting.
> I just wish Markus would improve his consistently terrible commit messages
I tried to achieve another clarification a few times.
> that just restate the action being done and detail
> _why_ a particular thing _should_ be done.
Unfortunately, it seems that no other contributors picked
corresponding opportunities up so far.
You indicated also special software development challenges in your commit
“checkpatch: attempt to find unnecessary 'out of memory' messages”.
https://lkml.org/lkml/2014/6/10/382
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ebfdc40969f24fc0cdd1349835d36e8ebae05374
> His acceptance rate would improve as many of these back and forth
> replies for what trivialities he posts as patches would be minimized.
My selection of change possibilities leads to mixed integration results.
I stumbled on variations for general change resistance.
Regards,
Markus
^ permalink raw reply
* Re: [PATCH] Input: gpio_keys: Add level trigger support for GPIO keys
From: Baolin Wang @ 2018-02-11 1:54 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dmitry Torokhov, Rob Herring, Mark Rutland, Greg Kroah-Hartman,
stephen lu, Arvind Yadav, Joseph Lo, Kate Stewart,
Philippe Ombredanne, Thomas Gleixner,
linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, DTML, lkml,
Mark Brown, Linus Walleij
In-Reply-To: <CAHp75VcJdYh8fKpqkRXxegRkESaN2Cgxybyei1NVwf_1C7rhwQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 9 February 2018 at 23:03, Andy Shevchenko <andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Feb 9, 2018 at 11:23 AM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>> On 9 February 2018 at 14:39, Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> On Thu, Feb 8, 2018 at 10:08 PM, Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
>>>> On some platforms (such as Spreadtrum platform), the GPIO keys can only
>>>> be triggered by level type.
>>>
>>> How do you stop the interrupt from re-triggering as long as the key
>>> stays pressed?
>>
>> We will set the level type irq handler as handle_level_irq(), in this
>> function, it will mask and ack the irq firstly.
>
> Wouldn't be ambiguous?
>
> 1. User presses the key ->
> a) we got edge followed by level signaling;
> b) IRQ core masks line, calls handler, ACKs, unmasks;
> c) somewhere here Press Event is sent;
> d) we still have level... We get IRQ fired again? But see 1. It
> obviously not the case.
> 2. User releases the key ->
> ...
>
> So, the main question if I understood Dmitry correctly is the period
> in time where IRQ line should be masked on one hand, and on the other
> it will guarantee that user didn't release-press cylcle.
Yes, you and Dmitry are right. I realized my problem. When I tested on
Spreadtrum platform, we use Spreadtrum-special EIC (external interrupt
controller) to trigger button events, if we acked the EIC irq, it will
drop the interrupt line and never trigger events until we set irq type
again. But this can not work for general GPIOs like you said.
So I think I need disable the irq until reversing the level type. Thanks.
--
Baolin.wang
Best Regards
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v2] Input: gpio_keys: Add level trigger support for GPIO keys
From: Baolin Wang @ 2018-02-11 6:55 UTC (permalink / raw)
To: dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
lumotuwe-Re5JQEeQqe8AvxtiuMwx3w,
arvind.yadav.cs-Re5JQEeQqe8AvxtiuMwx3w,
josephl-DDmLM1+adcrQT0dZR+AlfA,
kstewart-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
pombredanne-od1rfyK75/E, tglx-hfZtesqFncYOwBW4kG4KsQ,
linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
broonie-DgEjT+Ai2ygdnm+yROfE0A,
linus.walleij-QSEj5FYQhm4dnm+yROfE0A,
baolin.wang-QSEj5FYQhm4dnm+yROfE0A
On some platforms (such as Spreadtrum platform), the GPIO keys can only
be triggered by level type. So this patch introduces one property to
indicate if the GPIO trigger type is level trigger or edge trigger.
Signed-off-by: Baolin Wang <baolin.wang-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
Changes since v1:
- Diable the GPIO irq until reversing the GPIO level type.
---
.../devicetree/bindings/input/gpio-keys.txt | 2 ++
drivers/input/keyboard/gpio_keys.c | 26 +++++++++++++++++++-
include/linux/gpio_keys.h | 1 +
3 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/input/gpio-keys.txt b/Documentation/devicetree/bindings/input/gpio-keys.txt
index a949404..e3104bd 100644
--- a/Documentation/devicetree/bindings/input/gpio-keys.txt
+++ b/Documentation/devicetree/bindings/input/gpio-keys.txt
@@ -29,6 +29,8 @@ Optional subnode-properties:
- linux,can-disable: Boolean, indicates that button is connected
to dedicated (not shared) interrupt which can be disabled to
suppress events from the button.
+ - gpio-key,level-trigger: Boolean, indicates that button's interrupt
+ type is level trigger. Otherwise it is edge trigger as default.
Example nodes:
diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
index 87e613d..218698a 100644
--- a/drivers/input/keyboard/gpio_keys.c
+++ b/drivers/input/keyboard/gpio_keys.c
@@ -385,6 +385,20 @@ static void gpio_keys_gpio_work_func(struct work_struct *work)
struct gpio_button_data *bdata =
container_of(work, struct gpio_button_data, work.work);
+ if (bdata->button->level_trigger) {
+ unsigned int trigger =
+ irq_get_trigger_type(bdata->irq) & ~IRQF_TRIGGER_MASK;
+ int state = gpiod_get_raw_value_cansleep(bdata->gpiod);
+
+ if (state)
+ trigger |= IRQF_TRIGGER_LOW;
+ else
+ trigger |= IRQF_TRIGGER_HIGH;
+
+ irq_set_irq_type(bdata->irq, trigger);
+ enable_irq(bdata->irq);
+ }
+
gpio_keys_gpio_report_event(bdata);
if (bdata->button->wakeup)
@@ -397,6 +411,9 @@ static irqreturn_t gpio_keys_gpio_isr(int irq, void *dev_id)
BUG_ON(irq != bdata->irq);
+ if (bdata->button->level_trigger)
+ disable_irq_nosync(bdata->irq);
+
if (bdata->button->wakeup) {
const struct gpio_keys_button *button = bdata->button;
@@ -566,7 +583,11 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
INIT_DELAYED_WORK(&bdata->work, gpio_keys_gpio_work_func);
isr = gpio_keys_gpio_isr;
- irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
+ if (button->level_trigger)
+ irqflags = gpiod_is_active_low(bdata->gpiod) ?
+ IRQF_TRIGGER_LOW : IRQF_TRIGGER_HIGH;
+ else
+ irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
} else {
if (!button->irq) {
@@ -721,6 +742,9 @@ static void gpio_keys_close(struct input_dev *input)
button->can_disable =
fwnode_property_read_bool(child, "linux,can-disable");
+ button->level_trigger =
+ fwnode_property_read_bool(child, "gpio-key,level-trigger");
+
if (fwnode_property_read_u32(child, "debounce-interval",
&button->debounce_interval))
button->debounce_interval = 5;
diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
index d06bf77..5095645 100644
--- a/include/linux/gpio_keys.h
+++ b/include/linux/gpio_keys.h
@@ -28,6 +28,7 @@ struct gpio_keys_button {
int wakeup;
int debounce_interval;
bool can_disable;
+ bool level_trigger;
int value;
unsigned int irq;
};
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: UHID Device Reports Do Not Generate OS Activity
From: Roderick Colenbrander @ 2018-02-11 19:46 UTC (permalink / raw)
To: R0b0t1; +Cc: linux-input, linux-usb
In-Reply-To: <CAAD4mYj0pUUbwX8mKDP2qTn=QZM9ubk3f9FsabefK4xntY7bFA@mail.gmail.com>
It is hard to say what is wrong. I would start with reading the uhid /
hid code and see where you are getting stuck (and double check uhid
docs). Even for others we would need to do that as it is not easy to
see what is wrong.
I'm not sure what your goal is obvious you want to simulate a keyboard
for some purpose, but do you really need to operate at the HID layer?
If not it is much more convenient to use uinput, which will create a
evdev device for you.
Thanks,
Roderick
On Sat, Feb 10, 2018 at 1:33 AM, R0b0t1 <r030t1@gmail.com> wrote:
> Hello,
>
> I am trying to simulate a keyboard using the uhid interface. The fake
> device shows up in dmesg, but attempting to press the "a" key does
> nothing I can see. No input appears in the active terminal, nor in an
> xev window.
>
> The program receives UHID_OPEN, UHID_START, and UHID_CLOSE.
>
> Many different report descriptors were tried. The one currently
> uncommented is the one distributed with the USB implementor forum's
> HID report descriptor tool. The other is one found in a description of
> a boot device compatible keyboard.
>
> Thanks in advance,
> R0b0t1
^ permalink raw reply
* Re: UHID Device Reports Do Not Generate OS Activity
From: R0b0t1 @ 2018-02-11 20:22 UTC (permalink / raw)
To: Roderick Colenbrander; +Cc: linux-input, linux-usb-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAEc3jaAyQiNDLbOvNkT65n-Vju-LOHjoSuiOqDunBFna0YDaBQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sun, Feb 11, 2018 at 1:46 PM, Roderick Colenbrander
<thunderbird2k-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> It is hard to say what is wrong. I would start with reading the uhid /
> hid code and see where you are getting stuck (and double check uhid
> docs). Even for others we would need to do that as it is not easy to
> see what is wrong.
>
The issue was that the device is not immediately able to give input to
userspace programs after being opened by the kernel.
Waiting 32ms (2 timeslices?) gives a 100% success rate, with 16ms
having a 50% success rate. Checking modern kernel sources timeslices
may no longer be 16ms.
> I'm not sure what your goal is obvious you want to simulate a keyboard
> for some purpose, but do you really need to operate at the HID layer?
> If not it is much more convenient to use uinput, which will create a
> evdev device for you.
>
My kernel was not originally compiled with uinput. I used uhid to
avoid rebooting. (I have since rebooted and ended 199 days of uptime.)
There were other reasons for doing it this way, one of them being that
I am going to be prototyping an HID device. A software model will
help. I also wanted more control of the keystrokes initially to see if
it would be necessary to maintain that control.
It might be best to switch to uinput, but once you have USB
descriptors USB is actually very simple.
Cheers,
R0b0t1
> Thanks,
> Roderick
>
> On Sat, Feb 10, 2018 at 1:33 AM, R0b0t1 <r030t1-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Hello,
>>
>> I am trying to simulate a keyboard using the uhid interface. The fake
>> device shows up in dmesg, but attempting to press the "a" key does
>> nothing I can see. No input appears in the active terminal, nor in an
>> xev window.
>>
>> The program receives UHID_OPEN, UHID_START, and UHID_CLOSE.
>>
>> Many different report descriptors were tried. The one currently
>> uncommented is the one distributed with the USB implementor forum's
>> HID report descriptor tool. The other is one found in a description of
>> a boot device compatible keyboard.
>>
>> Thanks in advance,
>> R0b0t1
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: ALPS touchpad ot correctly recognized: GlidePoint vs DualPoint
From: Pali Rohár @ 2018-02-11 23:01 UTC (permalink / raw)
To: Juanito
Cc: Masaki Ota, Dmitry Torokhov, linux-input@vger.kernel.org,
Paul Donohue
In-Reply-To: <712c2657-5cf6-5fa0-1358-82bece1c02f8@posteo.net>
[-- Attachment #1: Type: text/plain, Size: 4602 bytes --]
On Monday 05 February 2018 12:19:48 Juanito wrote:
>
>
> On 02/05/2018 08:48 AM, Juanito wrote:
> > On 02/04/2018 09:21 PM, Pali Rohár wrote:
> >> On Sunday 04 February 2018 20:39:06 Juanito wrote:
> >>> On 02/04/2018 07:16 PM, Pali Rohár wrote:
> >>>> On Monday 11 September 2017 13:26:30 Juanito wrote:
> >>>>> Hi Masaki Ota,
> >>>>>
> >>>>> Thanks!
> >>>>>
> >>>>> On 11.09.2017 04:38, Masaki Ota wrote:
> >>>>>> Hi, Juanito,
> >>>>>>
> >>>>>> In my information, ALPS Touchpad is used on Thinkpad E series and L series.
> >>>>>> I don't know the device that is ALPS Touchpad + other vendor TrackStick.
> >>>>>> But Lenovo might use ALPS Touchpad on such a combination.
> >>>>>>
> >>>>>
> >>>>> Well, that is probably my fault, as it is not the original touchpad that
> >>>>> was delievered with the laptop. I bought the touchpad (that included
> >>>>> the three buttons) separately because I didn't like the clickpad that
> >>>>> came with my laptop.
> >>>>
> >>>> Hi Juanito,
> >>>>
> >>>
> >>> Hi Pali,
> >>>
> >>>> if you are still want to play with your touchpad hardware, I have a good
> >>>> news for your.
> >>>>
> >>>
> >>> Yeah! I'd love to get it to work! It's just I've been "working" on some
> >>> other projects and my last kernel builds didn't work at all so I gave up
> >>> and never got back to it.
> >>>
> >>> Thank you very much for that!
> >>>
> >>>> It looks like that at least ALPS rushmore touchpads allow to receive RAW
> >>>> PS/2 packets from trackstick to host kernel, without modifying them by
> >>>> touchpad. Plus I was able to tell ALPS touchpad to start "mixing" those
> >>>> RAW trackstick PS/2 packets with native touchpad packets.
> >>>>
> >>>> On my configuration trackstick in RAW PS/2 mode by default talks with
> >>>> standard bare 3 byte PS/2 protocol and touchpad in 6 byte ALPS protocol.
> >>>> alps.c/psmouse.ko is already able to process and parse such mixed
> >>>> packets. And trackstick can be switched to some extended 4 byte
> >>>> protocol...
> >>>>
> >>>> All this happen when passthrough mode is enabled.
> >>>>
> >>>> From my understanding it seems that in normal mode, touchpad and
> >>>> trackstick communicate with that 4 byte protocol and touchpad converts
> >>>> it into 6 byte ALPS protocol and then send to kernel.
> >>>>
> >>>> On thinkpads trackstick communicate with TPPS/2 protcol and above 4
> >>>> byte. So in my opinion ALPS touchpad by default cannot understand it.
> >>>> But you should be able to enter passthrough mode and then you would
> >>>> receive that TPPS/2 in alps kernel code.
> >>>>
> >>>
> >>> I don't really understand what you mean. Do I "just" have to set it to
> >>> passthrough mode? How exactly can I do that?
> >>
> >> Look at function alps_passthrough_mode_v3(). And try to call it after
> >> touchpad is initialized.
> >>
> >
> > Cool, I'll give this a try!
> >
> >> You can also look which alps_command_mode_write_reg() functions are
> >> called for your touchpad and maybe try to figure out what those the
> >> registers can enabled/disable.
> >>
> >> On http://www.cirque.com/gen4-dev-resources you can find document named
> >> GP-AN- 130823 INTERFACING TO GEN4 OVER I2C (PDF) which contains
> >> some description of those registers (in section 7).
> >>
> >> Register C2C8, bit 0 has description "PS2AuxControl.CommandPassThruEnabled"
> >>
> >> If trackstick is not detected, it seems you can "force" enable it by
> >> setting bit 7 "PS2AuxControl.AuxDevicePresent" in same register.
> >>
> >>> Again, thank you very much!
> >>
> >> Have you tried to read from that register which Masaki Ota talked in
> >> previous emails?
> >>
> >
> > I tried but I just didn't manage to get it to work, at all and gave up
> > (I really am a newbie in this whole kernel world). Maybe a fresh start
> > helps here.
> >
>
> Oh, oh! Currently, my touchpad is just being recognized as a PS/2
> Generic Mouse.
>
> On both debian stretch kernel (4.9.0-5-amd64) and on a 4.15.0+ compiled
> by me.
Maybe bug in kernel driver? Can you install older kernel version and
find which worked?
> I'll try to see if I can find out why this is happening.
The best way is to enable debug output and look into dmesg log why
alps.c refused to register your touchpad.
> Do have any idea why this might be the case (it used to be recognized as
> ALPS)?
There were some bugs with ALPS touchpads and IIRC all of them (expect
one) should be fixed in git. So maybe you hit it.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* (unknown),
From: Alfred Cheuk Chow @ 2018-02-12 1:39 UTC (permalink / raw)
Good Day,
I am Mr. Alfred Cheuk Yu Chow, the Director for Credit & Marketing Chong
Hing Bank, Hong Kong, Chong Hing Bank Center, 24 Des Voeux Road Central,
Hong Kong. I have a business proposal of $ 38,980,369.00.
All confirmable documents to back up the claims will be made available
to you prior to your acceptance and as soon as I receive your return
mail.
Best Regards,
Alfred Chow.
^ permalink raw reply
* Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action
From: Brian Norris @ 2018-02-12 22:13 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel, briannorris, dtor, dianders, Enric Balletbo i Serra,
Thomas Gleixner, Joseph Lo, stephen lu, Dmitry Torokhov,
Kate Stewart, linux-input, Greg Kroah-Hartman,
Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180210110907.5504-2-jeffy.chen@rock-chips.com>
Hi Jeffy,
On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> Add support for specifying event actions to trigger wakeup when using
> the gpio-keys input device as a wakeup source.
>
> This would allow the device to configure when to wakeup the system. For
> example a gpio-keys input device for pen insert, may only want to wakeup
> the system when ejecting the pen.
>
> Suggested-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> ---
>
> Changes in v2:
> Specify wakeup event action instead of irq trigger type as Brian
> suggested.
>
> drivers/input/keyboard/gpio_keys.c | 27 +++++++++++++++++++++++++++
> include/linux/gpio_keys.h | 2 ++
> include/uapi/linux/input-event-codes.h | 9 +++++++++
> 3 files changed, 38 insertions(+)
>
> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
> index 87e613dc33b8..5c57339d3999 100644
> --- a/drivers/input/keyboard/gpio_keys.c
> +++ b/drivers/input/keyboard/gpio_keys.c
> @@ -45,6 +45,8 @@ struct gpio_button_data {
> unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>
> unsigned int irq;
> + unsigned int irq_trigger_type;
> + unsigned int wakeup_trigger_type;
> spinlock_t lock;
> bool disabled;
> bool key_pressed;
> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> }
>
> if (bdata->gpiod) {
> + int active_low = gpiod_is_active_low(bdata->gpiod);
> +
> if (button->debounce_interval) {
> error = gpiod_set_debounce(bdata->gpiod,
> button->debounce_interval * 1000);
> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> isr = gpio_keys_gpio_isr;
> irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>
> + switch (button->wakeup_event_action) {
> + case EV_ACT_ASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
> + break;
> + case EV_ACT_DEASSERTED:
> + bdata->wakeup_trigger_type = active_low ?
> + IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
> + break;
What about EV_ACT_ANY? And default case? I think for ANY, we're OK
letting suspend/resume not reconfigure the trigger type, but maybe a
comment here to note that?
> + }
> } else {
What about the 'else' case? Shouldn't we try to handle that?
Brian
> if (!button->irq) {
> dev_err(dev, "Found button without gpio or irq\n");
> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
> return error;
> }
>
> + bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
> +
> return 0;
> }
>
> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
> /* legacy name */
> fwnode_property_read_bool(child, "gpio-key,wakeup");
>
> + fwnode_property_read_u32(child, "wakeup-event-action",
> + &button->wakeup_event_action);
> +
> button->can_disable =
> fwnode_property_read_bool(child, "linux,can-disable");
>
> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
> if (device_may_wakeup(dev)) {
> for (i = 0; i < ddata->pdata->nbuttons; i++) {
> struct gpio_button_data *bdata = &ddata->data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> + bdata->wakeup_trigger_type);
> if (bdata->button->wakeup)
> enable_irq_wake(bdata->irq);
> bdata->suspended = true;
> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
> if (device_may_wakeup(dev)) {
> for (i = 0; i < ddata->pdata->nbuttons; i++) {
> struct gpio_button_data *bdata = &ddata->data[i];
> +
> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
> + irq_set_irq_type(bdata->irq,
> + bdata->irq_trigger_type);
> if (bdata->button->wakeup)
> disable_irq_wake(bdata->irq);
> bdata->suspended = false;
> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
> index d06bf77400f1..7160df54a6fe 100644
> --- a/include/linux/gpio_keys.h
> +++ b/include/linux/gpio_keys.h
> @@ -13,6 +13,7 @@ struct device;
> * @desc: label that will be attached to button's gpio
> * @type: input event type (%EV_KEY, %EV_SW, %EV_ABS)
> * @wakeup: configure the button as a wake-up source
> + * @wakeup_event_action: event action to trigger wakeup
> * @debounce_interval: debounce ticks interval in msecs
> * @can_disable: %true indicates that userspace is allowed to
> * disable button via sysfs
> @@ -26,6 +27,7 @@ struct gpio_keys_button {
> const char *desc;
> unsigned int type;
> int wakeup;
> + int wakeup_event_action;
> int debounce_interval;
> bool can_disable;
> int value;
> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
> index 53fbae27b280..d7917b0bd438 100644
> --- a/include/uapi/linux/input-event-codes.h
> +++ b/include/uapi/linux/input-event-codes.h
> @@ -32,6 +32,15 @@
> #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1)
>
> /*
> + * Event action types
> + */
> +#define EV_ACT_ANY 0x00 /* asserted or deasserted */
> +#define EV_ACT_ASSERTED 0x01 /* asserted */
> +#define EV_ACT_DEASSERTED 0x02 /* deasserted */
> +#define EV_ACT_MAX 0x02
> +#define EV_ACT_CNT (EV_ACT_MAX+1)
> +
> +/*
> * Event types
> */
>
> --
> 2.11.0
>
>
^ permalink raw reply
* Re: [PATCH] Support TrackStick of Thinkpad L570
From: Pali Rohár @ 2018-02-12 22:25 UTC (permalink / raw)
To: Masaki Ota
Cc: Masaki Ota, dmitry.torokhov@gmail.com,
benjamin.tissoires@redhat.com, aaron.ma@canonical.com,
jaak@ristioja.ee, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, richard
In-Reply-To: <TYXPR01MB0719CB1198852D3B3231DC3EC7220@TYXPR01MB0719.jpnprd01.prod.outlook.com>
[-- Attachment #1: Type: text/plain, Size: 4051 bytes --]
Masaki: About firmware update, there is Linux Vendor Firmware Service
project at https://fwupd.org/ exactly for purpose to update firmwares
from Linux distributions. Maintainer of this project, Richard Hughes
(CCed), is more times willing to write support for new devices. So
maybe this can be a solution for future.
On Monday 20 November 2017 08:43:45 Masaki Ota wrote:
> Hi, Pali,
>
> 1) Yes, but we don't have FW update plan.
> And this device works as SMBus in Windows, so this issue happens on only Linux.
>
> 2) Yes.
>
> Best Regards,
> Masaki Ota
> -----Original Message-----
> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> Sent: Monday, November 20, 2017 5:30 PM
> To: Masaki Ota <012nexus@gmail.com>; dmitry.torokhov@gmail.com
> Cc: benjamin.tissoires@redhat.com; aaron.ma@canonical.com; jaak@ristioja.ee; 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>; linux-input@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] Support TrackStick of Thinkpad L570
>
> On Monday 20 November 2017 16:55:30 Masaki Ota wrote:
> > From: Masaki Ota <masaki.ota@jp.alps.com>
> > - The issue is that Thinkpad L570 TrackStick does not work. Because the main interface of Thinkpad L570 device is SMBus, so ALPS overlooked PS2 interface Firmware setting of TrackStick. The detail is that TrackStick otp bit is disabled.
> > - Add the code that checks 0xD7 address value. This value is device number information, so we can identify the device by checking this value.
> > - If we check 0xD7 value, we need to enable Command mode and after check the value we need to disable Command mode, then we have to enable the device(0xF4 command).
> > - Thinkpad L570 device number is 0x0C or 0x1D. If it is TRUE, enable ALPS_DUALPOINT flag.
>
> So, the root of this problem is in ALPS firmware which provides wrong information to kernel?
>
> Masaki, I have two questions:
>
> 1) Can ALPS or Lenovo release a new firmware update for this Thinkpad to
> fix this issue?
>
> 2) Have all Thinkpad L570 machines trackpoint?
>
> Dmitry, as a workaround for firmware bug on particular notebook, would not be better to check DMI information and DMI based hook?
>
> > Signed-off-by: Masaki Ota <masaki.ota@jp.alps.com>
> > ---
> > drivers/input/mouse/alps.c | 21 ++++++++++++++++++---
> > 1 file changed, 18 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> > index 850b00e3ad8e..cce52104ed5a 100644
> > --- a/drivers/input/mouse/alps.c
> > +++ b/drivers/input/mouse/alps.c
> > @@ -2541,13 +2541,28 @@ static int
> > alps_update_btn_info_ss4_v2(unsigned char otp[][4], }
> >
> > static int alps_update_dual_info_ss4_v2(unsigned char otp[][4],
> > - struct alps_data *priv)
> > + struct alps_data *priv,
> > + struct psmouse *psmouse)
> > {
> > bool is_dual = false;
> > + int reg_val = 0;
> > + struct ps2dev *ps2dev = &psmouse->ps2dev;
> >
> > - if (IS_SS4PLUS_DEV(priv->dev_id))
> > + if (IS_SS4PLUS_DEV(priv->dev_id)) {
> > is_dual = (otp[0][0] >> 4) & 0x01;
> >
> > + /* For support TrackStick of Thinkpad L570 device */
> > + if (alps_exit_command_mode(psmouse) == 0 &&
> > + alps_enter_command_mode(psmouse) == 0) {
> > + reg_val = alps_command_mode_read_reg(psmouse, 0xD7);
> > + }
> > + alps_exit_command_mode(psmouse);
> > + ps2_command(ps2dev, NULL, PSMOUSE_CMD_ENABLE);
> > +
> > + if (reg_val == 0x0C || reg_val == 0x1D)
> > + is_dual = true;
> > + }
> > +
> > if (is_dual)
> > priv->flags |= ALPS_DUALPOINT |
> > ALPS_DUALPOINT_WITH_PRESSURE;
> > @@ -2570,7 +2585,7 @@ static int alps_set_defaults_ss4_v2(struct
> > psmouse *psmouse,
> >
> > alps_update_btn_info_ss4_v2(otp, priv);
> >
> > - alps_update_dual_info_ss4_v2(otp, priv);
> > + alps_update_dual_info_ss4_v2(otp, priv, psmouse);
> >
> > return 0;
> > }
>
> --
> Pali Rohár
> pali.rohar@gmail.com
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* rmi4 defunct on T460p [was: [git pull] Input updates for v4.15-rc8]
From: Jiri Slaby @ 2018-02-13 8:22 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-kernel, linux-input
In-Reply-To: <20180209013916.GA42766@dtor-ws>
On 02/09/2018, 02:39 AM, Dmitry Torokhov wrote:
> On Thu, Feb 08, 2018 at 11:04:22AM +0100, Jiri Slaby wrote:
>> On 01/19/2018, 06:42 PM, Dmitry Torokhov wrote:
>>> Please pull from:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
>>>
>>> to receive updates for the input subsystem. You will get:
>>>
>> ...
>>> - touchpad on Lenovo T640p is switched over to SMbus/RMI
>> ...
>>
>>> 王振杰 (1):
>>> Input: synaptics - Lenovo Thinkpad T460p devices should use RMI
>>
>> Hi,
>>
>> one of openSUSE users with T460p reports that the touchpad is defunct
>> after resume in 4.15. psmouse.synaptics_intertouch=0 works around the
>> problem.
>>
>> The problem is:
>> kernel: rmi4_smbus 8-002c: failed to get SMBus version number!
>> kernel: rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read
>> current IRQ mask.
>> kernel: rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -16.
>> kernel: rmi4_f01 rmi4-00.fn01: Resume failed with code -16.
>> kernel: rmi4_physical rmi4-00: Failed to suspend functions: -16
>> kernel: rmi4_smbus 8-002c: Failed to resume device: -16
>> kernel: rmi4_f03 rmi4-00.fn03: rmi_f03_pt_write: Failed to write to F03
>> TX register (-16).
>>
>> The downstream bug is at:
>> https://bugzilla.suse.com/show_bug.cgi?id=1079862
>>
>> Any ideas?
>
> Hmm, maybe the seuence between psmouse and rmi/smbus is not quite right
> on resume on that box. Can you ask the reporter to:
>
> echo 1 > /sys/power/pm_print_times
> suspend/resume
He uploaded it here:
https://bugzilla.suse.com/attachment.cgi?id=759933
thanks,
--
js
suse labs
^ permalink raw reply
* Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action
From: Enric Balletbo i Serra @ 2018-02-13 10:40 UTC (permalink / raw)
To: Brian Norris, Jeffy Chen
Cc: linux-kernel, briannorris, dtor, dianders, Thomas Gleixner,
Joseph Lo, stephen lu, Dmitry Torokhov, Kate Stewart, linux-input,
Greg Kroah-Hartman, Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180212221309.GA66974@ban.mtv.corp.google.com>
Hi Jeffy,
On 12/02/18 23:13, Brian Norris wrote:
> Hi Jeffy,
>
> On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> Add support for specifying event actions to trigger wakeup when using
>> the gpio-keys input device as a wakeup source.
>>
>> This would allow the device to configure when to wakeup the system. For
>> example a gpio-keys input device for pen insert, may only want to wakeup
>> the system when ejecting the pen.
>>
>> Suggested-by: Brian Norris <briannorris@chromium.org>
>> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> ---
>>
>> Changes in v2:
>> Specify wakeup event action instead of irq trigger type as Brian
>> suggested.
>>
>> drivers/input/keyboard/gpio_keys.c | 27 +++++++++++++++++++++++++++
>> include/linux/gpio_keys.h | 2 ++
>> include/uapi/linux/input-event-codes.h | 9 +++++++++
>> 3 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/input/keyboard/gpio_keys.c b/drivers/input/keyboard/gpio_keys.c
>> index 87e613dc33b8..5c57339d3999 100644
>> --- a/drivers/input/keyboard/gpio_keys.c
>> +++ b/drivers/input/keyboard/gpio_keys.c
>> @@ -45,6 +45,8 @@ struct gpio_button_data {
>> unsigned int software_debounce; /* in msecs, for GPIO-driven buttons */
>>
>> unsigned int irq;
>> + unsigned int irq_trigger_type;
>> + unsigned int wakeup_trigger_type;
>> spinlock_t lock;
>> bool disabled;
>> bool key_pressed;
>> @@ -540,6 +542,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>> }
>>
>> if (bdata->gpiod) {
>> + int active_low = gpiod_is_active_low(bdata->gpiod);
>> +
>> if (button->debounce_interval) {
>> error = gpiod_set_debounce(bdata->gpiod,
>> button->debounce_interval * 1000);
>> @@ -568,6 +572,16 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>> isr = gpio_keys_gpio_isr;
>> irqflags = IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING;
>>
>> + switch (button->wakeup_event_action) {
>> + case EV_ACT_ASSERTED:
>> + bdata->wakeup_trigger_type = active_low ?
>> + IRQF_TRIGGER_FALLING : IRQF_TRIGGER_RISING;
>> + break;
>> + case EV_ACT_DEASSERTED:
>> + bdata->wakeup_trigger_type = active_low ?
>> + IRQF_TRIGGER_RISING : IRQF_TRIGGER_FALLING;
>> + break;
>
> What about EV_ACT_ANY? And default case? I think for ANY, we're OK
> letting suspend/resume not reconfigure the trigger type, but maybe a
> comment here to note that?
>
>> + }
>> } else {
>
> What about the 'else' case? Shouldn't we try to handle that?
>
> Brian
>
>> if (!button->irq) {
>> dev_err(dev, "Found button without gpio or irq\n");
>> @@ -618,6 +632,8 @@ static int gpio_keys_setup_key(struct platform_device *pdev,
>> return error;
>> }
>>
>> + bdata->irq_trigger_type = irq_get_trigger_type(bdata->irq);
>> +
>> return 0;
>> }
>>
>> @@ -718,6 +734,9 @@ gpio_keys_get_devtree_pdata(struct device *dev)
>> /* legacy name */
>> fwnode_property_read_bool(child, "gpio-key,wakeup");
>>
>> + fwnode_property_read_u32(child, "wakeup-event-action",
>> + &button->wakeup_event_action);
>> +
>> button->can_disable =
>> fwnode_property_read_bool(child, "linux,can-disable");
>>
>> @@ -854,6 +873,10 @@ static int __maybe_unused gpio_keys_suspend(struct device *dev)
>> if (device_may_wakeup(dev)) {
>> for (i = 0; i < ddata->pdata->nbuttons; i++) {
>> struct gpio_button_data *bdata = &ddata->data[i];
>> +
>> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> + irq_set_irq_type(bdata->irq,
>> + bdata->wakeup_trigger_type);
>> if (bdata->button->wakeup)
>> enable_irq_wake(bdata->irq);
>> bdata->suspended = true;
>> @@ -878,6 +901,10 @@ static int __maybe_unused gpio_keys_resume(struct device *dev)
>> if (device_may_wakeup(dev)) {
>> for (i = 0; i < ddata->pdata->nbuttons; i++) {
>> struct gpio_button_data *bdata = &ddata->data[i];
>> +
>> + if (bdata->button->wakeup && bdata->wakeup_trigger_type)
>> + irq_set_irq_type(bdata->irq,
>> + bdata->irq_trigger_type);
>> if (bdata->button->wakeup)
>> disable_irq_wake(bdata->irq);
>> bdata->suspended = false;
>> diff --git a/include/linux/gpio_keys.h b/include/linux/gpio_keys.h
>> index d06bf77400f1..7160df54a6fe 100644
>> --- a/include/linux/gpio_keys.h
>> +++ b/include/linux/gpio_keys.h
>> @@ -13,6 +13,7 @@ struct device;
>> * @desc: label that will be attached to button's gpio
>> * @type: input event type (%EV_KEY, %EV_SW, %EV_ABS)
>> * @wakeup: configure the button as a wake-up source
>> + * @wakeup_event_action: event action to trigger wakeup
>> * @debounce_interval: debounce ticks interval in msecs
>> * @can_disable: %true indicates that userspace is allowed to
>> * disable button via sysfs
>> @@ -26,6 +27,7 @@ struct gpio_keys_button {
>> const char *desc;
>> unsigned int type;
>> int wakeup;
>> + int wakeup_event_action;
>> int debounce_interval;
>> bool can_disable;
>> int value;
>> diff --git a/include/uapi/linux/input-event-codes.h b/include/uapi/linux/input-event-codes.h
>> index 53fbae27b280..d7917b0bd438 100644
>> --- a/include/uapi/linux/input-event-codes.h
>> +++ b/include/uapi/linux/input-event-codes.h
>> @@ -32,6 +32,15 @@
>> #define INPUT_PROP_CNT (INPUT_PROP_MAX + 1)
>>
>> /*
>> + * Event action types
>> + */
>> +#define EV_ACT_ANY 0x00 /* asserted or deasserted */
>> +#define EV_ACT_ASSERTED 0x01 /* asserted */
>> +#define EV_ACT_DEASSERTED 0x02 /* deasserted */
>> +#define EV_ACT_MAX 0x02
>> +#define EV_ACT_CNT (EV_ACT_MAX+1)
>> +
>> +/*
>> * Event types
>> */
>>
>> --
>> 2.11.0
>>
>>
Not sure if you were aware but there is also some discussion related to this,
maybe we can join the efforts?
v1: https://patchwork.kernel.org/patch/10208261/
v2: https://patchwork.kernel.org/patch/10211147/
Best regards,
Enric
^ permalink raw reply
* [PATCH 2/3] HID: steam: add serial number information.
From: Rodrigo Rivas Costa @ 2018-02-13 12:03 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180213120308.23879-1-rodrigorivascosta@gmail.com>
This device has a feature report to send and receive commands.
Use it to get the serial number and set the device's uniq value.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/hid-steam.c | 89 +++++++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 86 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 03f912ab5484..f269a986e2be 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -34,12 +34,17 @@ struct steam_device {
unsigned long quirks;
struct work_struct work_connect;
bool connected;
+ char serial_no[11];
};
static int steam_register(struct steam_device *steam);
static void steam_unregister(struct steam_device *steam);
static void steam_do_connect_event(struct steam_device *steam, bool connected);
static void steam_do_input_event(struct steam_device *steam, u8 *data);
+static int steam_send_report(struct steam_device *steam,
+ u8 *cmd, int size);
+static int steam_recv_report(struct steam_device *steam,
+ u8 *data, int size);
static int steam_input_open(struct input_dev *dev)
{
@@ -55,6 +60,78 @@ static void steam_input_close(struct input_dev *dev)
hid_hw_close(steam->hid_dev);
}
+#define STEAM_FEATURE_REPORT_SIZE 65
+
+static int steam_send_report(struct steam_device *steam,
+ u8 *cmd, int size)
+{
+ int retry;
+ int ret;
+ u8 *buf = kzalloc(STEAM_FEATURE_REPORT_SIZE, GFP_KERNEL);
+
+ if (!buf)
+ return -ENOMEM;
+
+ /* The report ID is always 0 */
+ memcpy(buf + 1, cmd, size);
+
+ /* Sometimes the wireless controller fails with EPIPE
+ * when sending a feature report.
+ * Doing a HID_REQ_GET_REPORT and waiting for a while
+ * seems to fix that.
+ */
+ for (retry = 0; retry < 10; ++retry) {
+ ret = hid_hw_raw_request(steam->hid_dev, 0,
+ buf, size + 1,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != -EPIPE)
+ break;
+ dbg_hid("%s: failed, retrying (%d times)\n", __func__, retry+1);
+ steam_recv_report(steam, NULL, 0);
+ msleep(50);
+ }
+ kfree(buf);
+ return ret;
+}
+
+static int steam_recv_report(struct steam_device *steam,
+ u8 *data, int size)
+{
+ int ret;
+ u8 *buf = kzalloc(STEAM_FEATURE_REPORT_SIZE, GFP_KERNEL);
+
+ if (!buf)
+ return -ENOMEM;
+
+ /* The report ID is always 0 */
+ ret = hid_hw_raw_request(steam->hid_dev, 0x00,
+ buf, STEAM_FEATURE_REPORT_SIZE,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ memcpy(data, buf + 1, size);
+ kfree(buf);
+ return ret;
+}
+
+static int steam_get_serial(struct steam_device *steam)
+{
+ /* Send: 0xae 0x15 0x01
+ * Recv: 0xae 0x15 0x01 serialnumber (10 chars)
+ */
+ int ret;
+ u8 cmd[] = {0xae, 0x15, 0x01};
+ u8 reply[14];
+
+ ret = steam_send_report(steam, cmd, sizeof(cmd));
+ if (ret < 0)
+ return ret;
+ ret = steam_recv_report(steam, reply, sizeof(reply));
+ if (ret < 0)
+ return ret;
+ reply[13] = 0;
+ strcpy(steam->serial_no, reply + 3);
+ return 0;
+}
+
static void steam_work_connect_cb(struct work_struct *work)
{
struct steam_device *steam = container_of(work, struct steam_device,
@@ -385,7 +462,12 @@ static int steam_register(struct steam_device *steam)
dbg_hid("%s\n", __func__);
- hid_info(hdev, "Steam Controller connected");
+ ret = steam_get_serial(steam);
+ if (ret)
+ return ret;
+
+ hid_info(hdev, "Steam Controller SN: '%s' connected",
+ steam->serial_no);
input = input_allocate_device();
if (!input)
@@ -398,7 +480,7 @@ static int steam_register(struct steam_device *steam)
input->name = "Steam Controller";
input->phys = hdev->phys;
- input->uniq = hdev->uniq;
+ input->uniq = steam->serial_no;
input->id.bustype = hdev->bus;
input->id.vendor = hdev->vendor;
input->id.product = hdev->product;
@@ -447,7 +529,8 @@ static void steam_unregister(struct steam_device *steam)
dbg_hid("%s\n", __func__);
if (steam->input_dev) {
- hid_info(steam->hid_dev, "Steam Controller disconnected");
+ hid_info(steam->hid_dev, "Steam Controller SN: '%s' disconnected",
+ steam->serial_no);
input_unregister_device(steam->input_dev);
steam->input_dev = NULL;
}
--
2.16.1
^ permalink raw reply related
* [PATCH 1/3] HID: add driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-02-13 12:03 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
There are two ways to connect the Steam Controller: directly to the USB
or with the USB wireless adapter. Both methods are similar, but the
wireless adapter can connect up to 4 devices at the same time.
The wired device will appear as 3 interfaces: a virtual mouse, a virtual
keyboard and a custom HID device.
The wireless device will appear as 5 interfaces: a virtual keyboard and
4 custom HID devices, that will remain silent until a device is actually
connected.
The custom HID device has a report descriptor with all vendor specific
usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
Steam Client provices a software translation by using direct USB access
and a creates a uinput virtual gamepad.
This driver was reverse engineered to provide direct kernel support in
case you cannot, or do not want to, use Valve Steam Client. It disables
the virtual keyboard and mouse, as they are not so useful when you have
a working gamepad.
Working: buttons, axes, pads, wireless connect/disconnect.
TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-quirks.c | 4 +
drivers/hid/hid-steam.c | 480 +++++++++++++++++++++++++++++++++++++++++++++++
5 files changed, 497 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 19c499f5623d..6e80fbf04e03 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -823,6 +823,14 @@ config HID_SPEEDLINK
---help---
Support for Speedlink Vicious and Divine Cezanne mouse.
+config HID_STEAM
+ tristate "Steam Controller support"
+ depends on HID
+ ---help---
+ Say Y here if you have a Steam Controller if you want to use it
+ without running the Steam Client. It supports both the wired and
+ the wireless adaptor.
+
config HID_STEELSERIES
tristate "Steelseries SRW-S1 steering wheel support"
depends on HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index eb13b9e92d85..60a8abf84682 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
+obj-$(CONFIG_HID_STEAM) += hid-steam.o
obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 43ddcdfbd0da..be31a3c20818 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -988,6 +988,10 @@
#define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
#define USB_DEVICE_ID_MTP_SITRONIX 0x5001
+#define USB_VENDOR_ID_VALVE 0x28de
+#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
+#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS 0x1142
+
#define USB_VENDOR_ID_STEELSERIES 0x1038
#define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 5f6035a5ce36..72ac972dc00b 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -629,6 +629,10 @@ static const struct hid_device_id hid_have_special_driver[] = {
#if IS_ENABLED(CONFIG_HID_SPEEDLINK)
{ HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
#endif
+#if IS_ENABLED(CONFIG_HID_STEAM)
+ { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
+#endif
#if IS_ENABLED(CONFIG_HID_STEELSERIES)
{ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
#endif
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index 000000000000..03f912ab5484
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,480 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/usb.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+
+#define STEAM_QUIRK_WIRELESS BIT(0)
+
+struct steam_device {
+ spinlock_t lock;
+ struct hid_device *hid_dev;
+ struct input_dev *input_dev;
+ unsigned long quirks;
+ struct work_struct work_connect;
+ bool connected;
+};
+
+static int steam_register(struct steam_device *steam);
+static void steam_unregister(struct steam_device *steam);
+static void steam_do_connect_event(struct steam_device *steam, bool connected);
+static void steam_do_input_event(struct steam_device *steam, u8 *data);
+
+static int steam_input_open(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ return hid_hw_open(steam->hid_dev);
+}
+
+static void steam_input_close(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ hid_hw_close(steam->hid_dev);
+}
+
+static void steam_work_connect_cb(struct work_struct *work)
+{
+ struct steam_device *steam = container_of(work, struct steam_device,
+ work_connect);
+ unsigned long flags;
+ bool connected;
+ int ret;
+
+ dbg_hid("%s\n", __func__);
+
+ spin_lock_irqsave(&steam->lock, flags);
+ connected = steam->connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (connected) {
+ if (steam->input_dev) {
+ dbg_hid("%s: already connected\n", __func__);
+ return;
+ }
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(steam->hid_dev,
+ "%s:steam_register returned error %d\n",
+ __func__, ret);
+ return;
+ }
+ } else {
+ steam_unregister(steam);
+ }
+}
+
+static int steam_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct steam_device *steam;
+ int ret;
+
+ dbg_hid("%s called for ifnum %d protocol %d\n", __func__,
+ intf->cur_altsetting->desc.bInterfaceNumber,
+ intf->cur_altsetting->desc.bInterfaceProtocol
+ );
+
+ /*
+ * The wired device creates 3 interfaces:
+ * 0: emulated mouse.
+ * 1: emulated keyboard.
+ * 2: the real game pad.
+ * The wireless device creates 5 interfaces:
+ * 0: emulated keyboard.
+ * 1-4: slots where up to 4 real game pads will be connected to.
+ * Instead of the interface index we use the protocol, it is 0
+ * for the real game pad.
+ * Since we have a real game pad now, we can ignore the virtual
+ * mouse and keyboard.
+ */
+ if (intf->cur_altsetting->desc.bInterfaceProtocol != 0) {
+ dbg_hid("%s: interface ignored\n", __func__);
+ return -ENODEV;
+ }
+
+ steam = kzalloc(sizeof(struct steam_device), GFP_KERNEL);
+ if (!steam)
+ return -ENOMEM;
+
+ spin_lock_init(&steam->lock);
+ steam->hid_dev = hdev;
+ hid_set_drvdata(hdev, steam);
+ steam->quirks = id->driver_data;
+ INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:parse of hid interface failed\n", __func__);
+ goto hid_parse_fail;
+ }
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_start returned error\n", __func__);
+ goto hid_hw_start_fail;
+ }
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ steam->input_dev = NULL;
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:hid_hw_open for wireless\n",
+ __func__);
+ goto hid_hw_open_fail;
+ }
+ hid_info(hdev, "Steam wireless receiver connected");
+ } else {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(hdev,
+ "%s:steam_register returned error\n",
+ __func__);
+ goto input_register_fail;
+ }
+ }
+
+ return 0;
+
+input_register_fail:
+hid_hw_open_fail:
+ hid_hw_stop(hdev);
+hid_hw_start_fail:
+hid_parse_fail:
+ cancel_work_sync(&steam->work_connect);
+ kfree(steam);
+ hid_set_drvdata(hdev, NULL);
+ return ret;
+}
+
+static void steam_remove(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ dbg_hid("%s\n", __func__);
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ hid_info(hdev, "Steam wireless receiver disconnected");
+ hid_hw_close(hdev);
+ }
+ hid_hw_stop(hdev);
+ steam_unregister(steam);
+ cancel_work_sync(&steam->work_connect);
+ kfree(steam);
+ hid_set_drvdata(hdev, NULL);
+}
+
+static int steam_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ /*
+ * All messages are size=64, all values little-endian.
+ * The format is:
+ * Offset| Meaning
+ * -------+--------------------------------------------
+ * 0-1 | always 0x01, 0x00, maybe protocol version?
+ * 2 | type of message
+ * 3 | length of the real payload (not checked)
+ * 4-n | payload data, depends on the type
+ *
+ * There are these known types of message:
+ * 0x01: input data (60 bytes)
+ * 0x03: wireless connect/disconnect (1 byte)
+ * 0x04: battery status (11 bytes)
+ */
+
+ if (size != 64 || data[0] != 1 || data[1] != 0)
+ return 0;
+
+ switch (data[2]) {
+ case 0x01:
+ steam_do_input_event(steam, data);
+ break;
+ case 0x03:
+ /*
+ * The payload of this event is a single byte:
+ * 0x01: disconnected.
+ * 0x02: connected.
+ */
+ switch (data[4]) {
+ case 0x01:
+ steam_do_connect_event(steam, false);
+ break;
+ case 0x02:
+ steam_do_connect_event(steam, true);
+ break;
+ }
+ break;
+ case 0x04:
+ /* TODO battery status */
+ break;
+ }
+ return 0;
+}
+
+static void steam_do_connect_event(struct steam_device *steam, bool connected)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->connected = connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (schedule_work(&steam->work_connect) == 0)
+ dbg_hid("%s: connected=%d event already queued\n",
+ __func__, connected);
+}
+
+/* The size for this message payload is 60.
+ * The known values are:
+ * (* values are not sent through wireless)
+ * (* accelerator/gyro is disabled by default)
+ * Offset| Type | Mapped to |Meaning
+ * -------+-------+-----------+--------------------------
+ * 4-7 | u32 | -- | sequence number
+ * 8-10 | 24bit | see below | buttons
+ * 11 | u8 | ABS_Z | left trigger
+ * 12 | u8 | ABS_RZ | right trigger
+ * 13-15 | -- | -- | always 0
+ * 16-17 | s16 | ABS_X | X value
+ * 18-19 | s16 | ABS_Y | Y value
+ * 20-21 | s16 | ABS_RX | right-pad X value
+ * 22-23 | s16 | ABS_RY | right-pad Y value
+ * 24-25 | s16 | -- | * left trigger
+ * 26-27 | s16 | -- | * right trigger
+ * 28-29 | s16 | -- | * accelerometer X value
+ * 30-31 | s16 | -- | * accelerometer Y value
+ * 32-33 | s16 | -- | * accelerometer Z value
+ * 34-35 | s16 | -- | gyro X value
+ * 36-36 | s16 | -- | gyro Y value
+ * 38-39 | s16 | -- | gyro Z value
+ * 40-41 | s16 | -- | quaternion W value
+ * 42-43 | s16 | -- | quaternion X value
+ * 44-45 | s16 | -- | quaternion Y value
+ * 46-47 | s16 | -- | quaternion Z value
+ * 48-49 | -- | -- | always 0
+ * 50-51 | s16 | -- | * left trigger (uncalibrated)
+ * 52-53 | s16 | -- | * right trigger (uncalibrated)
+ * 54-55 | s16 | -- | * joystick X value (uncalibrated)
+ * 56-57 | s16 | -- | * joystick Y value (uncalibrated)
+ * 58-59 | s16 | -- | * left-pad X value
+ * 60-61 | s16 | -- | * left-pad Y value
+ * 62-63 | u16 | -- | * battery voltage
+ *
+ * The buttons are:
+ * Bit | Mapped to | Description
+ * ------+------------+--------------------------------
+ * 8.0 | BTN_TR2 | right trigger fully pressed
+ * 8.1 | BTN_TL2 | left trigger fully pressed
+ * 8.2 | BTN_TR | right shoulder
+ * 8.3 | BTN_TL | left shoulder
+ * 8.4 | BTN_Y | button Y
+ * 8.5 | BTN_B | button B
+ * 8.6 | BTN_X | button X
+ * 8.7 | BTN_A | button A
+ * 9.0 | -ABS_HAT0Y | lef-pad up
+ * 9.1 | +ABS_HAT0X | lef-pad right
+ * 9.2 | -ABS_HAT0X | lef-pad left
+ * 9.3 | +ABS_HAT0Y | lef-pad down
+ * 9.4 | BTN_SELECT | menu left
+ * 9.5 | BTN_MODE | steam logo
+ * 9.6 | BTN_START | menu right
+ * 9.7 | BTN_GEAR_DOWN | left back lever
+ * 10.0 | BTN_GEAR_UP | right back lever
+ * 10.1 | -- | left-pad clicked
+ * 10.2 | BTN_THUMBR | right-pad clicked
+ * 10.3 | -- | left-pad touched
+ * 10.4 | -- | right-pad touched
+ * 10.5 | -- | unknown
+ * 10.6 | BTN_THUMBL | joystick clicked
+ * 10.7 | -- | lpad_and_joy
+ */
+
+static void steam_do_input_event(struct steam_device *steam, u8 *data)
+{
+ struct input_dev *input = steam->input_dev;
+
+ /* 24 bits of buttons */
+ u8 b8, b9, b10;
+
+ /*
+ * If we get input events from the wireless without a 'connected'
+ * event, just connect it now.
+ * This can happen, for example, if we bind the HID device with
+ * the controller already paired.
+ */
+ if (unlikely(!input)) {
+ dbg_hid("%s: input data without connect event\n", __func__);
+ steam_do_connect_event(steam, true);
+ return;
+ }
+
+ input_report_abs(input, ABS_Z, data[11]);
+ input_report_abs(input, ABS_RZ, data[12]);
+
+ input_report_abs(input, ABS_X,
+ (s16) le16_to_cpup((__le16 *)(data + 16)));
+ input_report_abs(input, ABS_Y,
+ -(s16) le16_to_cpup((__le16 *)(data + 18)));
+ input_report_abs(input, ABS_RX,
+ (s16) le16_to_cpup((__le16 *)(data + 20)));
+ input_report_abs(input, ABS_RY,
+ -(s16) le16_to_cpup((__le16 *)(data + 22)));
+
+ b8 = data[8];
+ b9 = data[9];
+ b10 = data[10];
+
+ input_event(input, EV_KEY, BTN_TR2, !!(b8 & 0x01));
+ input_event(input, EV_KEY, BTN_TL2, !!(b8 & 0x02));
+ input_event(input, EV_KEY, BTN_TR, !!(b8 & 0x04));
+ input_event(input, EV_KEY, BTN_TL, !!(b8 & 0x08));
+ input_event(input, EV_KEY, BTN_Y, !!(b8 & 0x10));
+ input_event(input, EV_KEY, BTN_B, !!(b8 & 0x20));
+ input_event(input, EV_KEY, BTN_X, !!(b8 & 0x40));
+ input_event(input, EV_KEY, BTN_A, !!(b8 & 0x80));
+ input_event(input, EV_KEY, BTN_SELECT, !!(b9 & 0x10));
+ input_event(input, EV_KEY, BTN_MODE, !!(b9 & 0x20));
+ input_event(input, EV_KEY, BTN_START, !!(b9 & 0x40));
+ input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & 0x80));
+ input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & 0x01));
+ input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & 0x04));
+ input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & 0x40));
+
+ input_report_abs(input, ABS_HAT0X,
+ !!(b9 & 0x02) - !!(b9 & 0x04));
+ input_report_abs(input, ABS_HAT0Y,
+ !!(b9 & 0x08) - !!(b9 & 0x01));
+
+ input_sync(input);
+}
+
+static int steam_register(struct steam_device *steam)
+{
+ struct hid_device *hdev = steam->hid_dev;
+ struct input_dev *input;
+ int ret;
+
+ dbg_hid("%s\n", __func__);
+
+ hid_info(hdev, "Steam Controller connected");
+
+ input = input_allocate_device();
+ if (!input)
+ return -ENOMEM;
+
+ input_set_drvdata(input, steam);
+ input->dev.parent = &hdev->dev;
+ input->open = steam_input_open;
+ input->close = steam_input_close;
+
+ input->name = "Steam Controller";
+ input->phys = hdev->phys;
+ input->uniq = hdev->uniq;
+ input->id.bustype = hdev->bus;
+ input->id.vendor = hdev->vendor;
+ input->id.product = hdev->product;
+ input->id.version = hdev->version;
+
+ input_set_capability(input, EV_KEY, BTN_TR2);
+ input_set_capability(input, EV_KEY, BTN_TL2);
+ input_set_capability(input, EV_KEY, BTN_TR);
+ input_set_capability(input, EV_KEY, BTN_TL);
+ input_set_capability(input, EV_KEY, BTN_Y);
+ input_set_capability(input, EV_KEY, BTN_B);
+ input_set_capability(input, EV_KEY, BTN_X);
+ input_set_capability(input, EV_KEY, BTN_A);
+ input_set_capability(input, EV_KEY, BTN_SELECT);
+ input_set_capability(input, EV_KEY, BTN_MODE);
+ input_set_capability(input, EV_KEY, BTN_START);
+ input_set_capability(input, EV_KEY, BTN_GEAR_DOWN);
+ input_set_capability(input, EV_KEY, BTN_GEAR_UP);
+ input_set_capability(input, EV_KEY, BTN_THUMBR);
+ input_set_capability(input, EV_KEY, BTN_THUMBL);
+
+ input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);
+ input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
+
+ ret = input_register_device(input);
+ if (ret)
+ goto input_register_fail;
+
+ steam->input_dev = input;
+
+ return 0;
+
+input_register_fail:
+ input_free_device(input);
+ return ret;
+}
+
+static void steam_unregister(struct steam_device *steam)
+{
+ dbg_hid("%s\n", __func__);
+
+ if (steam->input_dev) {
+ hid_info(steam->hid_dev, "Steam Controller disconnected");
+ input_unregister_device(steam->input_dev);
+ steam->input_dev = NULL;
+ }
+}
+
+static const struct hid_device_id steam_controllers[] = {
+ { /* Wired Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER)
+ },
+ { /* Wireless Steam Controller */
+ HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
+ USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
+ .driver_data = STEAM_QUIRK_WIRELESS
+ },
+ {}
+};
+
+MODULE_DEVICE_TABLE(hid, steam_controllers);
+
+static struct hid_driver steam_controller_driver = {
+ .name = "hid-steam",
+ .id_table = steam_controllers,
+ .probe = steam_probe,
+ .remove = steam_remove,
+ .raw_event = steam_raw_event,
+};
+
+module_hid_driver(steam_controller_driver);
+/* vi: set softtabstop=8 shiftwidth=8 noexpandtab tabstop=8: */
--
2.16.1
^ permalink raw reply related
* [PATCH 3/3] HID: steam: add battery device.
From: Rodrigo Rivas Costa @ 2018-02-13 12:03 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-kernel, linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180213120308.23879-1-rodrigorivascosta@gmail.com>
The wireless Steam Controller is battery operated, so add the battery
device and power information.
Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
---
drivers/hid/hid-steam.c | 139 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 138 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index f269a986e2be..eb2424688092 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -20,6 +20,7 @@
#include <linux/hid.h>
#include <linux/module.h>
#include <linux/workqueue.h>
+#include <linux/power_supply.h>
#include "hid-ids.h"
MODULE_LICENSE("GPL");
@@ -35,6 +36,10 @@ struct steam_device {
struct work_struct work_connect;
bool connected;
char serial_no[11];
+ struct power_supply_desc battery_desc;
+ struct power_supply *battery;
+ u8 battery_charge;
+ u16 voltage;
};
static int steam_register(struct steam_device *steam);
@@ -45,6 +50,8 @@ static int steam_send_report(struct steam_device *steam,
u8 *cmd, int size);
static int steam_recv_report(struct steam_device *steam,
u8 *data, int size);
+static int steam_battery_register(struct steam_device *steam);
+static void steam_do_battery_event(struct steam_device *steam, u8 *data);
static int steam_input_open(struct input_dev *dev)
{
@@ -311,7 +318,8 @@ static int steam_raw_event(struct hid_device *hdev,
}
break;
case 0x04:
- /* TODO battery status */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS)
+ steam_do_battery_event(steam, data);
break;
}
return 0;
@@ -517,6 +525,10 @@ static int steam_register(struct steam_device *steam)
steam->input_dev = input;
+ /* ignore battery errors, we can live without it */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS)
+ steam_battery_register(steam);
+
return 0;
input_register_fail:
@@ -528,6 +540,12 @@ static void steam_unregister(struct steam_device *steam)
{
dbg_hid("%s\n", __func__);
+ if (steam->battery) {
+ power_supply_unregister(steam->battery);
+ steam->battery = NULL;
+ kfree(steam->battery_desc.name);
+ steam->battery_desc.name = NULL;
+ }
if (steam->input_dev) {
hid_info(steam->hid_dev, "Steam Controller SN: '%s' disconnected",
steam->serial_no);
@@ -536,6 +554,125 @@ static void steam_unregister(struct steam_device *steam)
}
}
+/* The size for this message payload is 11.
+ * The known values are:
+ * Offset| Type | Meaning
+ * -------+-------+---------------------------
+ * 4-7 | u32 | sequence number
+ * 8-11 | -- | always 0
+ * 12-13 | u16 | voltage (mV)
+ * 14 | u8 | battery percent
+ */
+static void steam_do_battery_event(struct steam_device *steam, u8 *data)
+{
+ unsigned long flags;
+ s16 volts = le16_to_cpup((__le16 *)(data + 12));
+ u8 batt = data[14];
+
+ dbg_hid("%s: %d %d\n", __func__, volts, batt);
+
+ if (unlikely(!steam->battery)) {
+ dbg_hid("%s: battery data without connect event\n", __func__);
+ steam_do_connect_event(steam, true);
+ return;
+ }
+
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = volts;
+ steam->battery_charge = batt;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ power_supply_changed(steam->battery);
+}
+
+static enum power_supply_property steam_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_VOLTAGE_NOW,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steam_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steam_device *steam = power_supply_get_drvdata(psy);
+ unsigned long flags;
+ s16 volts;
+ u8 batt;
+ int ret = 0;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ volts = steam->voltage;
+ batt = steam->battery_charge;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_VOLTAGE_NOW:
+ val->intval = volts * 1000; /* mV -> uV */
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = batt;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static int steam_battery_register(struct steam_device *steam)
+{
+ struct power_supply *battery;
+ struct power_supply_config battery_cfg = { .drv_data = steam, };
+ unsigned long flags;
+ int ret;
+
+ dbg_hid("%s\n", __func__);
+
+ steam->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ steam->battery_desc.properties = steam_battery_props;
+ steam->battery_desc.num_properties = ARRAY_SIZE(steam_battery_props);
+ steam->battery_desc.get_property = steam_battery_get_property;
+ steam->battery_desc.name = kasprintf(GFP_KERNEL,
+ "steam-controller-%s-battery", steam->serial_no);
+ if (!steam->battery_desc.name) {
+ ret = -ENOMEM;
+ goto print_name_fail;
+ }
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = 3000;
+ steam->battery_charge = 100;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ battery = power_supply_register(&steam->hid_dev->dev,
+ &steam->battery_desc, &battery_cfg);
+ if (IS_ERR(battery)) {
+ ret = PTR_ERR(battery);
+ hid_err(steam->hid_dev,
+ "%s:power_supply_register returned error %d\n",
+ __func__, ret);
+ goto power_supply_reg_fail;
+ }
+ steam->battery = battery;
+ power_supply_powers(steam->battery, &steam->hid_dev->dev);
+ return 0;
+
+power_supply_reg_fail:
+ kfree(steam->battery_desc.name);
+ steam->battery_desc.name = NULL;
+print_name_fail:
+ return ret;
+}
+
static const struct hid_device_id steam_controllers[] = {
{ /* Wired Steam Controller */
HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
--
2.16.1
^ permalink raw reply related
* [PATCH resend 0/1] Input: silead - add Chuwi Hi8 support
From: Hans de Goede @ 2018-02-13 15:42 UTC (permalink / raw)
To: Dmitry Torokhov, Benjamin Tissoires; +Cc: Hans de Goede, linux-input
Hi Dmitry,
This one seems to have fallen through the cracks, hence this
resend.
Regards,
Hans
^ permalink raw reply
* [PATCH resend] Input: silead - add Chuwi Hi8 support
From: Hans de Goede @ 2018-02-13 15:42 UTC (permalink / raw)
To: Dmitry Torokhov, Benjamin Tissoires
Cc: Hans de Goede, linux-input, Maruyama Shohei
In-Reply-To: <20180213154238.19181-1-hdegoede@redhat.com>
From: Maruyama Shohei <cheat.sc.linux@outlook.com>
This commit add ACPI support for Chuwi Hi8 tablet. On the ACPI table of
the tablet, GSL1680 is registered as MSSL0001, so the driver does not
recognize the device. This commit resolve it.
Signed-off-by: Shohei Maruyama <cheat.sc.linux@outlook.com>
Reviewed-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
drivers/input/touchscreen/silead.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/touchscreen/silead.c b/drivers/input/touchscreen/silead.c
index 646b1e768e6b..ff7043f74a3d 100644
--- a/drivers/input/touchscreen/silead.c
+++ b/drivers/input/touchscreen/silead.c
@@ -602,6 +602,7 @@ static const struct acpi_device_id silead_ts_acpi_match[] = {
{ "GSL3675", 0 },
{ "GSL3692", 0 },
{ "MSSL1680", 0 },
+ { "MSSL0001", 0 },
{ }
};
MODULE_DEVICE_TABLE(acpi, silead_ts_acpi_match);
--
2.14.3
^ permalink raw reply related
* Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action
From: Brian Norris @ 2018-02-13 18:25 UTC (permalink / raw)
To: Enric Balletbo i Serra
Cc: Jeffy Chen, linux-kernel, briannorris, dtor, dianders,
Thomas Gleixner, Joseph Lo, stephen lu, Dmitry Torokhov,
Kate Stewart, linux-input, Greg Kroah-Hartman,
Philippe Ombredanne, Arvind Yadav
In-Reply-To: <059e2aa9-bf94-517d-a132-abe851ec69f7@collabora.com>
Hi Enric,
On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
> On 12/02/18 23:13, Brian Norris wrote:
> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
> >> Add support for specifying event actions to trigger wakeup when using
> >> the gpio-keys input device as a wakeup source.
> >>
> >> This would allow the device to configure when to wakeup the system. For
> >> example a gpio-keys input device for pen insert, may only want to wakeup
> >> the system when ejecting the pen.
> >>
> >> Suggested-by: Brian Norris <briannorris@chromium.org>
> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
> >> ---
> >>
> >> Changes in v2:
> >> Specify wakeup event action instead of irq trigger type as Brian
> >> suggested.
[...]
> Not sure if you were aware but there is also some discussion related to this,
> maybe we can join the efforts?
>
> v1: https://patchwork.kernel.org/patch/10208261/
> v2: https://patchwork.kernel.org/patch/10211147/
Thanks for the pointers. IIUC, that's talking about a different problem:
how to utilize a GPIO key in level-triggered mode. That touches similar
code, but it doesn't really have anything to do with configuring a
different wakeup trigger type.
The two patches would need to be reconciled, if they both are going to
be merged. But otherwise, I think they're perfectly fine to be separate.
Brian
^ permalink raw reply
* Re: [PATCH v2 1/3] Input: gpio-keys - add support for wakeup event action
From: Enric Balletbo Serra @ 2018-02-13 22:25 UTC (permalink / raw)
To: Brian Norris
Cc: Enric Balletbo i Serra, Jeffy Chen, linux-kernel, Brian Norris,
dtor, Doug Anderson, Thomas Gleixner, Joseph Lo, stephen lu,
Dmitry Torokhov, Kate Stewart, linux-input, Greg Kroah-Hartman,
Philippe Ombredanne, Arvind Yadav
In-Reply-To: <20180213182521.GA51247@rodete-desktop-imager.corp.google.com>
Hi,
2018-02-13 19:25 GMT+01:00 Brian Norris <briannorris@chromium.org>:
> Hi Enric,
>
> On Tue, Feb 13, 2018 at 11:40:44AM +0100, Enric Balletbo i Serra wrote:
>> On 12/02/18 23:13, Brian Norris wrote:
>> > On Sat, Feb 10, 2018 at 07:09:05PM +0800, Jeffy Chen wrote:
>> >> Add support for specifying event actions to trigger wakeup when using
>> >> the gpio-keys input device as a wakeup source.
>> >>
>> >> This would allow the device to configure when to wakeup the system. For
>> >> example a gpio-keys input device for pen insert, may only want to wakeup
>> >> the system when ejecting the pen.
>> >>
>> >> Suggested-by: Brian Norris <briannorris@chromium.org>
>> >> Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
>> >> ---
>> >>
>> >> Changes in v2:
>> >> Specify wakeup event action instead of irq trigger type as Brian
>> >> suggested.
> [...]
>> Not sure if you were aware but there is also some discussion related to this,
>> maybe we can join the efforts?
>>
>> v1: https://patchwork.kernel.org/patch/10208261/
>> v2: https://patchwork.kernel.org/patch/10211147/
>
> Thanks for the pointers. IIUC, that's talking about a different problem:
> how to utilize a GPIO key in level-triggered mode. That touches similar
> code, but it doesn't really have anything to do with configuring a
> different wakeup trigger type.
>
Right, sorry. I see now what you are doing.
> The two patches would need to be reconciled, if they both are going to
> be merged. But otherwise, I think they're perfectly fine to be separate.
>
Yes, that's why I got confused, I had those patches applied on my tree
and when I tried to apply these failed and I wrongly assumed that were
doing the same. Waiting to test the third version ;)
Thanks,
Enric
> Brian
^ permalink raw reply
* Re: [PATCH] HID: sony: Add touchpad support for NSG-MR5U and NSG-MR7U remotes
From: Roderick Colenbrander @ 2018-02-13 23:48 UTC (permalink / raw)
To: Todd Kelner; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input
In-Reply-To: <20180201180904.x3gwrrtur7uioeoo@todd-HP-Compaq-Elite-8300-USDT>
On Thu, Feb 1, 2018 at 10:09 AM, Todd Kelner <tsopdump@gmail.com> wrote:
> Sony's NSG-MR5U and NSG-MR7U remote controls have a full keyboard and a
> touchpad. The keyboard is already supported by the existing Linux
> kernel and drivers but the touchpad is not recognized. This patch adds
> the coded needed to bring full functionality to the touchpad.
>
> Note that these remotes use the vendor code for SMK even though they are
> Sony branded.
>
> Known limitations
> - The built-in accelerometers are not supported by these changes
> - When the Drag (Fn) key is used as a mouse button, the button is
> automatically released when the key begins repeating. There are two
> workarounds for this 1) Use the button behind the touchpad instead of
> the Drag (Fn) key or 2) Disable the key repeat functionality or
> increase the key repeat delay.
>
> Signed-off-by: Todd Kelner <tsopdump@gmail.com>
> ---
> drivers/hid/hid-core.c | 2 +
> drivers/hid/hid-ids.h | 2 +
> drivers/hid/hid-sony.c | 118 +++++++++++++++++++++++++++++++++++++++++++++++--
> 3 files changed, 119 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 0c3f608131cf..4b6be344466b 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2301,6 +2301,8 @@ static const struct hid_device_id hid_have_special_driver[] = {
> #if IS_ENABLED(CONFIG_HID_SONY)
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, USB_DEVICE_ID_LOGITECH_HARMONY_PS3) },
> { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_PS3_BDREMOTE) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_NSG_MR5U_REMOTE) },
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_NSG_MR7U_REMOTE) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_BUZZ_CONTROLLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_WIRELESS_BUZZ_CONTROLLER) },
> { HID_USB_DEVICE(USB_VENDOR_ID_SONY, USB_DEVICE_ID_SONY_MOTION_CONTROLLER) },
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 5da3d6256d25..793d80bdd301 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -954,6 +954,8 @@
>
> #define USB_VENDOR_ID_SMK 0x0609
> #define USB_DEVICE_ID_SMK_PS3_BDREMOTE 0x0306
> +#define USB_DEVICE_ID_SMK_NSG_MR5U_REMOTE 0x0368
> +#define USB_DEVICE_ID_SMK_NSG_MR7U_REMOTE 0x0369
>
> #define USB_VENDOR_ID_SONY 0x054c
> #define USB_DEVICE_ID_SONY_VAIO_VGX_MOUSE 0x024b
> diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
> index b9dc3ac4d4aa..8504f4fae6af 100644
> --- a/drivers/hid/hid-sony.c
> +++ b/drivers/hid/hid-sony.c
> @@ -9,6 +9,7 @@
> * Copyright (c) 2006-2013 Jiri Kosina
> * Copyright (c) 2013 Colin Leitner <colin.leitner@gmail.com>
> * Copyright (c) 2014-2016 Frank Praznik <frank.praznik@gmail.com>
> + * Copyright (c) 2018 Todd Kelner
> */
>
> /*
> @@ -55,6 +56,8 @@
> #define NAVIGATION_CONTROLLER_BT BIT(11)
> #define SINO_LITE_CONTROLLER BIT(12)
> #define FUTUREMAX_DANCE_MAT BIT(13)
> +#define NSG_MR5U_REMOTE_BT BIT(14)
> +#define NSG_MR7U_REMOTE_BT BIT(15)
>
> #define SIXAXIS_CONTROLLER (SIXAXIS_CONTROLLER_USB | SIXAXIS_CONTROLLER_BT)
> #define MOTION_CONTROLLER (MOTION_CONTROLLER_USB | MOTION_CONTROLLER_BT)
> @@ -72,8 +75,11 @@
> MOTION_CONTROLLER)
> #define SONY_BT_DEVICE (SIXAXIS_CONTROLLER_BT | DUALSHOCK4_CONTROLLER_BT |\
> MOTION_CONTROLLER_BT | NAVIGATION_CONTROLLER_BT)
> +#define NSG_MRXU_REMOTE (NSG_MR5U_REMOTE_BT | NSG_MR7U_REMOTE_BT)
>
> #define MAX_LEDS 4
> +#define NSG_MRXU_MAX_X 1667
> +#define NSG_MRXU_MAX_Y 1868
>
>
> /* PS/3 Motion controller */
> @@ -1072,6 +1078,75 @@ static void dualshock4_parse_report(struct sony_sc *sc, u8 *rd, int size)
> }
> }
>
> +static void nsg_mrxu_parse_report(struct sony_sc *sc, u8 *rd, int size)
> +{
> + int n, offset;
> + u8 active;
> +
> + /*
> + * The NSG-MRxU multi-touch trackpad data starts at offset 1 and
> + * the touch-related data starts at offset 2.
> + * For the first byte, bit 0 is set when touchpad button is pressed.
> + * Bit 3 is set when a touch is active and the drag (Fn) key is pressed.
> + * This drag key is mapped to BTN_LEFT.
> + * Bit 4 is set when only the first touch point is active.
> + * Bit 6 is set when only the second touch point is active.
> + * Bits 5 and 7 are set when both touch points are active.
> + * The next 3 bytes are two 12 bit X/Y coordinates for the first touch.
> + * The following byte, offset 5, has the touch width and length.
> + * Bits 0-4=X (width), bits 5-7=Y (length).
> + * A signed relative X coordinate is at offset 6.
> + * The bytes at offset 7-9 are the second touch X/Y coordinates.
> + * Offset 10 has the second touch width and length.
> + * Offset 11 has the relative Y coordinate.
> + */
> + offset = 1;
> +
> + input_report_key(sc->touchpad, BTN_LEFT, rd[offset] & 0x0F);
> + active = (rd[offset] >> 4);
> +
> + offset++;
> +
> + for (n = 0; n < 2; n++) {
> + u16 x, y;
> + u8 contactx, contacty;
> + unsigned int rel_axis;
> +
> + x = rd[offset] | ((rd[offset+1] & 0x0F) << 8);
> + y = ((rd[offset+1] & 0xF0) >> 4) | (rd[offset+2] << 4);
> +
> + input_mt_slot(sc->touchpad, n);
> + input_mt_report_slot_state(sc->touchpad, MT_TOOL_FINGER, active & 0x03);
> +
> + if (active & 0x03) {
> + contactx = rd[offset+3] & 0x0F;
> + contacty = rd[offset+3] >> 4;
> + input_report_abs(sc->touchpad, ABS_MT_TOUCH_MAJOR,
> + max(contactx, contacty));
> + input_report_abs(sc->touchpad, ABS_MT_TOUCH_MINOR,
> + max(contactx, contacty));
> + input_report_abs(sc->touchpad, ABS_MT_ORIENTATION,
> + (bool) (contactx > contacty));
> + input_report_abs(sc->touchpad, ABS_MT_POSITION_X, x);
> + input_report_abs(sc->touchpad, ABS_MT_POSITION_Y,
> + NSG_MRXU_MAX_Y - y);
> + if (n == 0)
> + rel_axis = REL_X;
> + else
> + rel_axis = REL_Y;
> +
> + input_report_rel(sc->touchpad, rel_axis, rd[offset+4]);
> + }
> +
> + offset += 5;
> + active >>= 2;
> + }
> +
> + input_mt_sync_frame(sc->touchpad);
> +
> + input_sync(sc->touchpad);
> +}
> +
> static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
> u8 *rd, int size)
> {
> @@ -1180,6 +1255,10 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
> }
>
> dualshock4_parse_report(sc, rd, size);
> +
> + } else if ((sc->quirks & NSG_MRXU_REMOTE) && rd[0] == 0x02) {
> + nsg_mrxu_parse_report(sc, rd, size);
> + return 1;
> }
>
> if (sc->defer_initialization) {
> @@ -1268,9 +1347,11 @@ static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
> snprintf(name, name_sz, "%s" DS4_TOUCHPAD_SUFFIX, sc->hdev->name);
> sc->touchpad->name = name;
>
> - ret = input_mt_init_slots(sc->touchpad, touch_count, INPUT_MT_POINTER);
> - if (ret < 0)
> - goto err;
> + if (!(sc->quirks & NSG_MRXU_REMOTE)) {
> + ret = input_mt_init_slots(sc->touchpad, touch_count, INPUT_MT_POINTER);
> + if (ret < 0)
> + goto err;
> + }
Why do you need to split input_mt_init_slots between non-NSG and NSG
later? It looks like it is to set some parameters on the touchpad
before input_mt_init_slots. I have no problem moving the general
input_init_mt_slots until later, so we only need it once.
> /* We map the button underneath the touchpad to BTN_LEFT. */
> __set_bit(EV_KEY, sc->touchpad->evbit);
> @@ -1280,6 +1361,17 @@ static int sony_register_touchpad(struct sony_sc *sc, int touch_count,
> input_set_abs_params(sc->touchpad, ABS_MT_POSITION_X, 0, w, 0, 0);
> input_set_abs_params(sc->touchpad, ABS_MT_POSITION_Y, 0, h, 0, 0);
>
> + if (sc->quirks & NSG_MRXU_REMOTE) {
> + __set_bit(EV_REL, sc->touchpad->evbit);
> + input_set_abs_params(sc->touchpad, ABS_MT_TOUCH_MAJOR, 0, 15, 0, 0);
> + input_set_abs_params(sc->touchpad, ABS_MT_TOUCH_MINOR, 0, 15, 0, 0);
> + input_set_abs_params(sc->touchpad, ABS_MT_ORIENTATION, 0, 1, 0, 0);
> +
I'm a little thorn on the NSG quirk check. On one hand
sony_register_touchpad is a helper function, which receives width and
height, but then there is this special logic.
Maybe it would be best to introduce some new parameters (not sure
which) to trigger this other mode, so we don't need special case.
> + ret = input_mt_init_slots(sc->touchpad, touch_count, INPUT_MT_POINTER);
> + if (ret < 0)
> + goto err;
> + }
> +
> ret = input_register_device(sc->touchpad);
> if (ret < 0)
> goto err;
> @@ -2654,6 +2746,20 @@ static int sony_input_configured(struct hid_device *hdev,
> }
>
> sony_init_output_report(sc, dualshock4_send_output_report);
> + } else if (sc->quirks & NSG_MRXU_REMOTE) {
> + /*
> + * The NSG-MRxU touchpad supports 2 touches and has a
> + * resolution of 1667x1868
> + */
> + ret = sony_register_touchpad(sc, 2,
> + NSG_MRXU_MAX_X, NSG_MRXU_MAX_Y);
> + if (ret) {
> + hid_err(sc->hdev,
> + "Unable to initialize multi-touch slots: %d\n",
> + ret);
> + goto err_stop;
> + }
> +
> } else if (sc->quirks & MOTION_CONTROLLER) {
> sony_init_output_report(sc, motion_send_output_report);
> } else {
> @@ -2892,6 +2998,12 @@ static const struct hid_device_id sony_devices[] = {
> /* Nyko Core Controller for PS3 */
> { HID_USB_DEVICE(USB_VENDOR_ID_SINO_LITE, USB_DEVICE_ID_SINO_LITE_CONTROLLER),
> .driver_data = SIXAXIS_CONTROLLER_USB | SINO_LITE_CONTROLLER },
> + /* SMK-Link NSG-MR5U Remote Control */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_NSG_MR5U_REMOTE),
> + .driver_data = NSG_MR5U_REMOTE_BT },
> + /* SMK-Link NSG-MR7U Remote Control */
> + { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_SMK, USB_DEVICE_ID_SMK_NSG_MR7U_REMOTE),
> + .driver_data = NSG_MR7U_REMOTE_BT },
> { }
> };
> MODULE_DEVICE_TABLE(hid, sony_devices);
> --
> 2.14.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-input" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* (unknown),
From: Alfred Cheuk Chow @ 2018-02-13 22:57 UTC (permalink / raw)
Good Day,
I am Mr. Alfred Cheuk Yu Chow, the Director for Credit & Marketing Chong
Hing Bank, Hong Kong, Chong Hing Bank Center, 24 Des Voeux Road Central,
Hong Kong. I have a business proposal of $ 38,980,369.00.
All confirmable documents to back up the claims will be made available
to you prior to your acceptance and as soon as I receive your return
mail.
Best Regards,
Alfred Chow.
^ permalink raw reply
* Re: [PATCH v2 0/3] gpio-keys: Add support for specifying wakeup event action
From: Heiko Stübner @ 2018-02-14 13:39 UTC (permalink / raw)
To: Jeffy Chen
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
briannorris-hpIqsD4AKlfQT0dZR+AlfA, dtor-hpIqsD4AKlfQT0dZR+AlfA,
dianders-hpIqsD4AKlfQT0dZR+AlfA, Arnd Bergmann, Joseph Lo,
Rob Herring, Catalin Marinas, Emil Renner Berthing,
Enric Balletbo i Serra, Brian Norris, Thomas Gleixner,
Philippe Ombredanne,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Kate Stewart,
linux-input-u79uwXL29TY76Z2rM5mHXA, Will Deacon,
Matthias Kaehlcke, devicetree-u79uwXL29TY76Z2rM5mHXA, stephen lu,
Greg Kroah-Hartman, Arvind Yadav <arvind.yada>
In-Reply-To: <20180210110907.5504-1-jeffy.chen-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Hi Jeffy,
Am Samstag, 10. Februar 2018, 12:09:04 CET schrieb Jeffy Chen:
> On chromebook kevin, we are using gpio-keys for pen insert event. But
> we only want it to wakeup the system when ejecting the pen.
>
> So we may need to change the interrupt trigger type during suspending.
>
> Changes in v2:
> Specify wakeup event action instead of irq trigger type as Brian
> suggested.
>
> Jeffy Chen (3):
> Input: gpio-keys - add support for wakeup event action
> Input: gpio-keys - allow setting wakeup event action in DT
> arm64: dts: rockchip: Avoid wakeup when inserting the pen
The recipient-selection for the mail thread seems to be a bit off and
I only got the cover-letter and patch 3/3.
Therefore I won't really see if and when the Input-changes get
accepted. So please ping me once that has happened so I can pick up
the rockchip dts change for it.
Thanks
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/3] HID: add driver for Valve Steam Controller
From: Benjamin Tissoires @ 2018-02-14 14:45 UTC (permalink / raw)
To: Rodrigo Rivas Costa; +Cc: Jiri Kosina, lkml, linux-input
In-Reply-To: <20180213120308.23879-1-rodrigorivascosta@gmail.com>
Hi Rodrigo,
On Tue, Feb 13, 2018 at 1:03 PM, Rodrigo Rivas Costa
<rodrigorivascosta@gmail.com> wrote:
> There are two ways to connect the Steam Controller: directly to the USB
> or with the USB wireless adapter. Both methods are similar, but the
> wireless adapter can connect up to 4 devices at the same time.
>
> The wired device will appear as 3 interfaces: a virtual mouse, a virtual
> keyboard and a custom HID device.
>
> The wireless device will appear as 5 interfaces: a virtual keyboard and
> 4 custom HID devices, that will remain silent until a device is actually
> connected.
>
> The custom HID device has a report descriptor with all vendor specific
> usages, so the hid-generic is not very useful. In a PC/SteamBox Valve
> Steam Client provices a software translation by using direct USB access
> and a creates a uinput virtual gamepad.
I think I had a look at this a while ago, and didn't want to interfere
with SteamOS regarding this. I think your patch should be fine in that
regard, but have you tried SteamOS on a kernel patched with your
series? Does it behave properly or will it break?
See more comments inlined.
>
> This driver was reverse engineered to provide direct kernel support in
> case you cannot, or do not want to, use Valve Steam Client. It disables
> the virtual keyboard and mouse, as they are not so useful when you have
> a working gamepad.
>
> Working: buttons, axes, pads, wireless connect/disconnect.
>
> TO-DO: Battery, force-feedback, accelerometer/gyro, led, beeper...
>
> Signed-off-by: Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
> ---
> drivers/hid/Kconfig | 8 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-quirks.c | 4 +
> drivers/hid/hid-steam.c | 480 +++++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 497 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 19c499f5623d..6e80fbf04e03 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -823,6 +823,14 @@ config HID_SPEEDLINK
> ---help---
> Support for Speedlink Vicious and Divine Cezanne mouse.
>
> +config HID_STEAM
> + tristate "Steam Controller support"
> + depends on HID
> + ---help---
> + Say Y here if you have a Steam Controller if you want to use it
> + without running the Steam Client. It supports both the wired and
> + the wireless adaptor.
> +
> config HID_STEELSERIES
> tristate "Steelseries SRW-S1 steering wheel support"
> depends on HID
> diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
> index eb13b9e92d85..60a8abf84682 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -95,6 +95,7 @@ obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
> obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
> obj-$(CONFIG_HID_SONY) += hid-sony.o
> obj-$(CONFIG_HID_SPEEDLINK) += hid-speedlink.o
> +obj-$(CONFIG_HID_STEAM) += hid-steam.o
> obj-$(CONFIG_HID_STEELSERIES) += hid-steelseries.o
> obj-$(CONFIG_HID_SUNPLUS) += hid-sunplus.o
> obj-$(CONFIG_HID_GREENASIA) += hid-gaff.o
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 43ddcdfbd0da..be31a3c20818 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -988,6 +988,10 @@
> #define USB_VENDOR_ID_STANTUM_SITRONIX 0x1403
> #define USB_DEVICE_ID_MTP_SITRONIX 0x5001
>
> +#define USB_VENDOR_ID_VALVE 0x28de
> +#define USB_DEVICE_ID_STEAM_CONTROLLER 0x1102
> +#define USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS 0x1142
> +
> #define USB_VENDOR_ID_STEELSERIES 0x1038
> #define USB_DEVICE_ID_STEELSERIES_SRWS1 0x1410
>
> diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
> index 5f6035a5ce36..72ac972dc00b 100644
> --- a/drivers/hid/hid-quirks.c
> +++ b/drivers/hid/hid-quirks.c
> @@ -629,6 +629,10 @@ static const struct hid_device_id hid_have_special_driver[] = {
> #if IS_ENABLED(CONFIG_HID_SPEEDLINK)
> { HID_USB_DEVICE(USB_VENDOR_ID_X_TENSIONS, USB_DEVICE_ID_SPEEDLINK_VAD_CEZANNE) },
> #endif
> +#if IS_ENABLED(CONFIG_HID_STEAM)
> + { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER) },
> + { HID_USB_DEVICE(USB_VENDOR_ID_VALVE, USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS) },
> +#endif
> #if IS_ENABLED(CONFIG_HID_STEELSERIES)
> { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
> #endif
> diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
> new file mode 100644
> index 000000000000..03f912ab5484
> --- /dev/null
> +++ b/drivers/hid/hid-steam.c
> @@ -0,0 +1,480 @@
> +// SPDX-License-Identifier: GPL-2.0
Non standard header
> +/*
> + * HID driver for Valve Steam Controller
> + *
> + * Supports both the wired and wireless interfaces.
> + *
> + * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
> + */
> +
> +/*
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/usb.h>
Generally, this raises a red flag from my side. HID should be
transport agnostic and depending on usb.h kills this. The advantage of
not depending on USB is that we can replay the devices with uhid in a
way it still works.
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/workqueue.h>
> +#include "hid-ids.h"
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
> +
> +#define STEAM_QUIRK_WIRELESS BIT(0)
> +
> +struct steam_device {
> + spinlock_t lock;
> + struct hid_device *hid_dev;
> + struct input_dev *input_dev;
> + unsigned long quirks;
> + struct work_struct work_connect;
> + bool connected;
> +};
> +
> +static int steam_register(struct steam_device *steam);
> +static void steam_unregister(struct steam_device *steam);
> +static void steam_do_connect_event(struct steam_device *steam, bool connected);
> +static void steam_do_input_event(struct steam_device *steam, u8 *data);
We tend to generally not do forward declaration of functions in the
kernel. Unless really necessary.
> +
> +static int steam_input_open(struct input_dev *dev)
> +{
> + struct steam_device *steam = input_get_drvdata(dev);
> +
> + return hid_hw_open(steam->hid_dev);
> +}
> +
> +static void steam_input_close(struct input_dev *dev)
> +{
> + struct steam_device *steam = input_get_drvdata(dev);
> +
> + hid_hw_close(steam->hid_dev);
> +}
> +
> +static void steam_work_connect_cb(struct work_struct *work)
> +{
> + struct steam_device *steam = container_of(work, struct steam_device,
> + work_connect);
> + unsigned long flags;
> + bool connected;
> + int ret;
> +
> + dbg_hid("%s\n", __func__);
This is pure debugging, and should be stripped out of the code (you
can use ftrace to see if your code is called BTW).
> +
> + spin_lock_irqsave(&steam->lock, flags);
> + connected = steam->connected;
> + spin_unlock_irqrestore(&steam->lock, flags);
> +
> + if (connected) {
> + if (steam->input_dev) {
> + dbg_hid("%s: already connected\n", __func__);
> + return;
> + }
> + ret = steam_register(steam);
> + if (ret) {
> + hid_err(steam->hid_dev,
> + "%s:steam_register returned error %d\n",
> + __func__, ret);
> + return;
> + }
> + } else {
> + steam_unregister(steam);
> + }
> +}
> +
> +static int steam_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
> + struct steam_device *steam;
> + int ret;
> +
> + dbg_hid("%s called for ifnum %d protocol %d\n", __func__,
> + intf->cur_altsetting->desc.bInterfaceNumber,
> + intf->cur_altsetting->desc.bInterfaceProtocol
> + );
> +
> + /*
> + * The wired device creates 3 interfaces:
> + * 0: emulated mouse.
> + * 1: emulated keyboard.
> + * 2: the real game pad.
> + * The wireless device creates 5 interfaces:
> + * 0: emulated keyboard.
> + * 1-4: slots where up to 4 real game pads will be connected to.
> + * Instead of the interface index we use the protocol, it is 0
> + * for the real game pad.
> + * Since we have a real game pad now, we can ignore the virtual
> + * mouse and keyboard.
> + */
> + if (intf->cur_altsetting->desc.bInterfaceProtocol != 0) {
> + dbg_hid("%s: interface ignored\n", __func__);
> + return -ENODEV;
As mentioned above, I'd rather you to decide on whether ignoring or
not the interface based on the actual reports, not the place in the
USB device. Valve can also decide to change their USB stack design,
and you'll have to amend this.
If you check that the reports ID you are expecting are not in the HID
device, you should be fine (hopefully)
> + }
> +
> + steam = kzalloc(sizeof(struct steam_device), GFP_KERNEL);
Please use devres to not have to free the memory on fail or on
disconnect (look for devm_kzalloc in the hid tree for examples).
> + if (!steam)
> + return -ENOMEM;
> +
> + spin_lock_init(&steam->lock);
> + steam->hid_dev = hdev;
> + hid_set_drvdata(hdev, steam);
> + steam->quirks = id->driver_data;
> + INIT_WORK(&steam->work_connect, steam_work_connect_cb);
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev,
> + "%s:parse of hid interface failed\n", __func__);
> + goto hid_parse_fail;
> + }
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + if (ret) {
> + hid_err(hdev,
> + "%s:hid_hw_start returned error\n", __func__);
> + goto hid_hw_start_fail;
> + }
> +
> + if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> + steam->input_dev = NULL;
This seems superfluous
> + ret = hid_hw_open(hdev);
> + if (ret) {
> + hid_err(hdev,
> + "%s:hid_hw_open for wireless\n",
> + __func__);
> + goto hid_hw_open_fail;
> + }
> + hid_info(hdev, "Steam wireless receiver connected");
> + } else {
> + ret = steam_register(steam);
> + if (ret) {
> + hid_err(hdev,
> + "%s:steam_register returned error\n",
> + __func__);
> + goto input_register_fail;
> + }
> + }
> +
> + return 0;
> +
> +input_register_fail:
> +hid_hw_open_fail:
> + hid_hw_stop(hdev);
> +hid_hw_start_fail:
> +hid_parse_fail:
> + cancel_work_sync(&steam->work_connect);
> + kfree(steam);
> + hid_set_drvdata(hdev, NULL);
> + return ret;
> +}
> +
> +static void steam_remove(struct hid_device *hdev)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
> +
> + dbg_hid("%s\n", __func__);
please remove
> +
> + if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> + hid_info(hdev, "Steam wireless receiver disconnected");
> + hid_hw_close(hdev);
> + }
> + hid_hw_stop(hdev);
> + steam_unregister(steam);
shouldn't you unregister the input nodes *after* cancelling the work
that might use it (haven't fully read the code, so it might be fine)
> + cancel_work_sync(&steam->work_connect);
> + kfree(steam);
> + hid_set_drvdata(hdev, NULL);
> +}
> +
> +static int steam_raw_event(struct hid_device *hdev,
> + struct hid_report *report, u8 *data,
> + int size)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
> +
> + /*
> + * All messages are size=64, all values little-endian.
> + * The format is:
> + * Offset| Meaning
> + * -------+--------------------------------------------
> + * 0-1 | always 0x01, 0x00, maybe protocol version?
Offset 0 is usually the report ID.
> + * 2 | type of message
> + * 3 | length of the real payload (not checked)
> + * 4-n | payload data, depends on the type
> + *
> + * There are these known types of message:
> + * 0x01: input data (60 bytes)
> + * 0x03: wireless connect/disconnect (1 byte)
> + * 0x04: battery status (11 bytes)
> + */
> +
> + if (size != 64 || data[0] != 1 || data[1] != 0)
> + return 0;
> +
> + switch (data[2]) {
> + case 0x01:
> + steam_do_input_event(steam, data);
> + break;
> + case 0x03:
> + /*
> + * The payload of this event is a single byte:
> + * 0x01: disconnected.
> + * 0x02: connected.
> + */
> + switch (data[4]) {
> + case 0x01:
> + steam_do_connect_event(steam, false);
> + break;
> + case 0x02:
> + steam_do_connect_event(steam, true);
> + break;
> + }
> + break;
> + case 0x04:
> + /* TODO battery status */
> + break;
> + }
> + return 0;
> +}
> +
> +static void steam_do_connect_event(struct steam_device *steam, bool connected)
> +{
> + unsigned long flags;
> +
> + spin_lock_irqsave(&steam->lock, flags);
> + steam->connected = connected;
> + spin_unlock_irqrestore(&steam->lock, flags);
> +
> + if (schedule_work(&steam->work_connect) == 0)
> + dbg_hid("%s: connected=%d event already queued\n",
> + __func__, connected);
> +}
> +
> +/* The size for this message payload is 60.
The beginning of the comment should be on its own line:
/*
* The size ...
> + * The known values are:
> + * (* values are not sent through wireless)
> + * (* accelerator/gyro is disabled by default)
> + * Offset| Type | Mapped to |Meaning
> + * -------+-------+-----------+--------------------------
> + * 4-7 | u32 | -- | sequence number
> + * 8-10 | 24bit | see below | buttons
> + * 11 | u8 | ABS_Z | left trigger
> + * 12 | u8 | ABS_RZ | right trigger
> + * 13-15 | -- | -- | always 0
> + * 16-17 | s16 | ABS_X | X value
> + * 18-19 | s16 | ABS_Y | Y value
> + * 20-21 | s16 | ABS_RX | right-pad X value
> + * 22-23 | s16 | ABS_RY | right-pad Y value
> + * 24-25 | s16 | -- | * left trigger
> + * 26-27 | s16 | -- | * right trigger
> + * 28-29 | s16 | -- | * accelerometer X value
> + * 30-31 | s16 | -- | * accelerometer Y value
> + * 32-33 | s16 | -- | * accelerometer Z value
> + * 34-35 | s16 | -- | gyro X value
> + * 36-36 | s16 | -- | gyro Y value
> + * 38-39 | s16 | -- | gyro Z value
> + * 40-41 | s16 | -- | quaternion W value
> + * 42-43 | s16 | -- | quaternion X value
> + * 44-45 | s16 | -- | quaternion Y value
> + * 46-47 | s16 | -- | quaternion Z value
> + * 48-49 | -- | -- | always 0
> + * 50-51 | s16 | -- | * left trigger (uncalibrated)
> + * 52-53 | s16 | -- | * right trigger (uncalibrated)
> + * 54-55 | s16 | -- | * joystick X value (uncalibrated)
> + * 56-57 | s16 | -- | * joystick Y value (uncalibrated)
> + * 58-59 | s16 | -- | * left-pad X value
> + * 60-61 | s16 | -- | * left-pad Y value
> + * 62-63 | u16 | -- | * battery voltage
> + *
> + * The buttons are:
> + * Bit | Mapped to | Description
> + * ------+------------+--------------------------------
> + * 8.0 | BTN_TR2 | right trigger fully pressed
> + * 8.1 | BTN_TL2 | left trigger fully pressed
> + * 8.2 | BTN_TR | right shoulder
> + * 8.3 | BTN_TL | left shoulder
> + * 8.4 | BTN_Y | button Y
> + * 8.5 | BTN_B | button B
> + * 8.6 | BTN_X | button X
> + * 8.7 | BTN_A | button A
> + * 9.0 | -ABS_HAT0Y | lef-pad up
> + * 9.1 | +ABS_HAT0X | lef-pad right
> + * 9.2 | -ABS_HAT0X | lef-pad left
> + * 9.3 | +ABS_HAT0Y | lef-pad down
> + * 9.4 | BTN_SELECT | menu left
> + * 9.5 | BTN_MODE | steam logo
> + * 9.6 | BTN_START | menu right
> + * 9.7 | BTN_GEAR_DOWN | left back lever
> + * 10.0 | BTN_GEAR_UP | right back lever
> + * 10.1 | -- | left-pad clicked
> + * 10.2 | BTN_THUMBR | right-pad clicked
> + * 10.3 | -- | left-pad touched
> + * 10.4 | -- | right-pad touched
> + * 10.5 | -- | unknown
> + * 10.6 | BTN_THUMBL | joystick clicked
> + * 10.7 | -- | lpad_and_joy
> + */
> +
> +static void steam_do_input_event(struct steam_device *steam, u8 *data)
> +{
> + struct input_dev *input = steam->input_dev;
> +
> + /* 24 bits of buttons */
> + u8 b8, b9, b10;
> +
> + /*
> + * If we get input events from the wireless without a 'connected'
> + * event, just connect it now.
> + * This can happen, for example, if we bind the HID device with
> + * the controller already paired.
> + */
> + if (unlikely(!input)) {
> + dbg_hid("%s: input data without connect event\n", __func__);
> + steam_do_connect_event(steam, true);
> + return;
> + }
> +
> + input_report_abs(input, ABS_Z, data[11]);
> + input_report_abs(input, ABS_RZ, data[12]);
> +
> + input_report_abs(input, ABS_X,
> + (s16) le16_to_cpup((__le16 *)(data + 16)));
> + input_report_abs(input, ABS_Y,
> + -(s16) le16_to_cpup((__le16 *)(data + 18)));
> + input_report_abs(input, ABS_RX,
> + (s16) le16_to_cpup((__le16 *)(data + 20)));
> + input_report_abs(input, ABS_RY,
> + -(s16) le16_to_cpup((__le16 *)(data + 22)));
> +
> + b8 = data[8];
> + b9 = data[9];
> + b10 = data[10];
> +
> + input_event(input, EV_KEY, BTN_TR2, !!(b8 & 0x01));
> + input_event(input, EV_KEY, BTN_TL2, !!(b8 & 0x02));
> + input_event(input, EV_KEY, BTN_TR, !!(b8 & 0x04));
> + input_event(input, EV_KEY, BTN_TL, !!(b8 & 0x08));
> + input_event(input, EV_KEY, BTN_Y, !!(b8 & 0x10));
> + input_event(input, EV_KEY, BTN_B, !!(b8 & 0x20));
> + input_event(input, EV_KEY, BTN_X, !!(b8 & 0x40));
> + input_event(input, EV_KEY, BTN_A, !!(b8 & 0x80));
> + input_event(input, EV_KEY, BTN_SELECT, !!(b9 & 0x10));
> + input_event(input, EV_KEY, BTN_MODE, !!(b9 & 0x20));
> + input_event(input, EV_KEY, BTN_START, !!(b9 & 0x40));
> + input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & 0x80));
> + input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & 0x01));
> + input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & 0x04));
> + input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & 0x40));
> +
> + input_report_abs(input, ABS_HAT0X,
> + !!(b9 & 0x02) - !!(b9 & 0x04));
> + input_report_abs(input, ABS_HAT0Y,
> + !!(b9 & 0x08) - !!(b9 & 0x01));
> +
> + input_sync(input);
> +}
> +
> +static int steam_register(struct steam_device *steam)
> +{
> + struct hid_device *hdev = steam->hid_dev;
> + struct input_dev *input;
> + int ret;
> +
> + dbg_hid("%s\n", __func__);
please remove
> +
> + hid_info(hdev, "Steam Controller connected");
> +
> + input = input_allocate_device();
Don't you need one input node per wireless gamepad too?
> + if (!input)
> + return -ENOMEM;
> +
> + input_set_drvdata(input, steam);
> + input->dev.parent = &hdev->dev;
> + input->open = steam_input_open;
> + input->close = steam_input_close;
> +
> + input->name = "Steam Controller";
In case of the wireless controllers, you might want to personalize this a bit.
> + input->phys = hdev->phys;
> + input->uniq = hdev->uniq;
> + input->id.bustype = hdev->bus;
> + input->id.vendor = hdev->vendor;
> + input->id.product = hdev->product;
> + input->id.version = hdev->version;
> +
> + input_set_capability(input, EV_KEY, BTN_TR2);
> + input_set_capability(input, EV_KEY, BTN_TL2);
> + input_set_capability(input, EV_KEY, BTN_TR);
> + input_set_capability(input, EV_KEY, BTN_TL);
> + input_set_capability(input, EV_KEY, BTN_Y);
> + input_set_capability(input, EV_KEY, BTN_B);
> + input_set_capability(input, EV_KEY, BTN_X);
> + input_set_capability(input, EV_KEY, BTN_A);
> + input_set_capability(input, EV_KEY, BTN_SELECT);
> + input_set_capability(input, EV_KEY, BTN_MODE);
> + input_set_capability(input, EV_KEY, BTN_START);
> + input_set_capability(input, EV_KEY, BTN_GEAR_DOWN);
> + input_set_capability(input, EV_KEY, BTN_GEAR_UP);
> + input_set_capability(input, EV_KEY, BTN_THUMBR);
> + input_set_capability(input, EV_KEY, BTN_THUMBL);
> +
> + input_set_abs_params(input, ABS_Z, 0, 255, 0, 0);
> + input_set_abs_params(input, ABS_RZ, 0, 255, 0, 0);
> + input_set_abs_params(input, ABS_X, -32767, 32767, 0, 0);
> + input_set_abs_params(input, ABS_Y, -32767, 32767, 0, 0);
You are also probably missing the resolution bits. We need to
accurately report the physical dimensions to the user space (thanks to
the resolution)
> + input_set_abs_params(input, ABS_RX, -32767, 32767, 0, 0);
> + input_set_abs_params(input, ABS_RY, -32767, 32767, 0, 0);
What do RX/RY correspond to?
> + input_set_abs_params(input, ABS_HAT0X, -1, 1, 0, 0);
> + input_set_abs_params(input, ABS_HAT0Y, -1, 1, 0, 0);
> +
> + ret = input_register_device(input);
> + if (ret)
> + goto input_register_fail;
> +
> + steam->input_dev = input;
> +
> + return 0;
> +
> +input_register_fail:
> + input_free_device(input);
> + return ret;
> +}
> +
> +static void steam_unregister(struct steam_device *steam)
> +{
> + dbg_hid("%s\n", __func__);
please remove
> +
> + if (steam->input_dev) {
> + hid_info(steam->hid_dev, "Steam Controller disconnected");
> + input_unregister_device(steam->input_dev);
> + steam->input_dev = NULL;
> + }
> +}
> +
> +static const struct hid_device_id steam_controllers[] = {
> + { /* Wired Steam Controller */
> + HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
> + USB_DEVICE_ID_STEAM_CONTROLLER)
> + },
> + { /* Wireless Steam Controller */
> + HID_USB_DEVICE(USB_VENDOR_ID_VALVE,
> + USB_DEVICE_ID_STEAM_CONTROLLER_WIRELESS),
> + .driver_data = STEAM_QUIRK_WIRELESS
> + },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(hid, steam_controllers);
> +
> +static struct hid_driver steam_controller_driver = {
> + .name = "hid-steam",
> + .id_table = steam_controllers,
> + .probe = steam_probe,
> + .remove = steam_remove,
> + .raw_event = steam_raw_event,
> +};
> +
> +module_hid_driver(steam_controller_driver);
> +/* vi: set softtabstop=8 shiftwidth=8 noexpandtab tabstop=8: */
Non standard comment, useful, but we strip out these in upstream
Anyway. Thanks for the driver, there are a few bits to fix, nothing
scary though.
Cheers,
Benjamin
> --
> 2.16.1
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox