* Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()
From: Kees Cook @ 2018-03-22 15:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Al Viro, Florian Weimer, Andrew Morton, Josh Poimboeuf,
Rasmus Villemoes, Randy Dunlap, Miguel Ojeda, Ingo Molnar,
David Laight, Ian Abbott, linux-input, linux-btrfs,
Network Development, Linux Kernel Mailing List, Kernel Hardening
In-Reply-To: <CA+55aFwxk=tUECYQkd4cog08qW4ZT=r2K7FQXzGnc-zuMc7JQA@mail.gmail.com>
On Tue, Mar 20, 2018 at 4:23 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Sat, Mar 17, 2018 at 1:07 PM, Kees Cook <keescook@chromium.org> wrote:
>>
>> No luck! :( gcc 4.4 refuses to play along. And, hilariously, not only
>> does it not change the complaint about __builtin_choose_expr(), it
>> also thinks that's a VLA now.
>
> Hmm. So thanks to the diseased mind of Martin Uecker, there's a better
> test for "__is_constant()":
>
> /* Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de> */
> #define __is_constant(a) \
> (sizeof(int) == sizeof(*(1 ? ((void*)((a) * 0l)) : (int*)1)))
>
> that is actually *specified* by the C standard to work, and doesn't
> even depend on any gcc extensions.
I feel we risk awakening Cthulhu with this. :)
> The reason is some really subtle pointer conversion rules, where the
> type of the ternary operator will depend on whether one of the
> pointers is NULL or not.
>
> And the definition of NULL, in turn, very much depends on "integer
> constant expression that has the value 0".
>
> Are you willing to do one final try on a generic min/max? Same as my
> last patch, but using the above __is_constant() test instead of
> __builtin_constant_p?
So, this time it's not a catastrophic failure with gcc 4.4. Instead it
fails in 11 distinct places:
$ grep "first argument to ‘__builtin_choose_expr’ not a constant" log
| cut -d: -f1-2
crypto/ablkcipher.c:71
crypto/blkcipher.c:70
crypto/skcipher.c:95
mm/percpu.c:2453
net/ceph/osdmap.c:1545
net/ceph/osdmap.c:1756
net/ceph/osdmap.c:1763
mm/kmemleak.c:1371
mm/kmemleak.c:1403
drivers/infiniband/hw/hfi1/pio_copy.c:421
drivers/infiniband/hw/hfi1/pio_copy.c:547
Seems like it doesn't like void * arguments:
mm/percpu.c:
void *ptr;
...
base = min(ptr, base);
mm/kmemleak.c:
static void scan_large_block(void *start, void *end)
...
next = min(start + MAX_SCAN_SIZE, end);
I'll poke a bit more...
-Kees
--
Kees Cook
Pixel Security
^ permalink raw reply
* RE: [PATCH v5 0/2] Remove false-positive VLAs when using max()
From: David Laight @ 2018-03-22 15:13 UTC (permalink / raw)
To: 'Kees Cook', Linus Torvalds
Cc: Al Viro, Florian Weimer, Andrew Morton, Josh Poimboeuf,
Rasmus Villemoes, Randy Dunlap, Miguel Ojeda, Ingo Molnar,
Ian Abbott, linux-input, linux-btrfs, Network Development,
Linux Kernel Mailing List, Kernel Hardening
In-Reply-To: <CAGXu5j+5i+56R0KDLMDA=+_DRW5w9aUGCEo0dq6PZvHPBWkM1g@mail.gmail.com>
From: Kees Cook
> Sent: 22 March 2018 15:01
...
> > /* Glory to Martin Uecker <Martin.Uecker@med.uni-goettingen.de> */
> > #define __is_constant(a) \
> > (sizeof(int) == sizeof(*(1 ? ((void*)((a) * 0l)) : (int*)1)))
...
> So, this time it's not a catastrophic failure with gcc 4.4. Instead it
> fails in 11 distinct places:
...
> Seems like it doesn't like void * arguments:
>
> mm/percpu.c:
> void *ptr;
> ...
> base = min(ptr, base);
Try adding (unsigned long) before the (a).
David
^ permalink raw reply
* [PATCHv1] Input: atmel_mxt_ts - fix the firmware update
From: Sebastian Reichel @ 2018-03-22 16:43 UTC (permalink / raw)
To: Sebastian Reichel, Nick Dyer, Dmitry Torokhov, linux-input
Cc: Henrik Rydberg, linux-kernel, kernel, Nandor Han,
Sebastian Reichel
From: Nandor Han <nandor.han@ge.com>
The automatic update mechanism will trigger an update if the
info block CRCs are different between maxtouch configuration
file (maxtouch.cfg) and chip.
The driver compared the CRCs without retrieving the chip CRC,
resulting always in a failure and firmware flashing action
triggered. The patch will fix this issue by retrieving the
chip info block CRC before the check.
Suggested-by: Todd Weyenberg <Todd.Weyenberg@med.ge.com>
Signed-off-by: Nandor Han <nandor.han@ge.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7659bc48f1db..ab936e6b0286 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -1728,6 +1728,25 @@ static int mxt_get_object_table(struct mxt_data *data)
return error;
}
+static int mxt_get_info_block_crc(struct mxt_data *data)
+{
+ size_t table_size;
+ int error;
+ u8 info_block_crc[MXT_INFO_CHECKSUM_SIZE];
+
+ table_size = data->info.object_num * sizeof(struct mxt_object);
+
+ /* Read the info block CRC */
+ error = __mxt_read_reg(data->client, MXT_OBJECT_START + table_size,
+ sizeof(info_block_crc), info_block_crc);
+ if (!error) {
+ data->info_crc = info_block_crc[0] | (info_block_crc[1] << 8) |
+ (info_block_crc[2] << 16);
+ }
+
+ return error;
+}
+
static int mxt_read_t9_resolution(struct mxt_data *data)
{
struct i2c_client *client = data->client;
@@ -2077,6 +2096,14 @@ static int mxt_initialize(struct mxt_data *data)
return error;
}
+ /* Get info block CRC */
+ error = mxt_get_info_block_crc(data);
+ if (error) {
+ dev_err(&client->dev, "Error %d reading info block CRC\n",
+ error);
+ return error;
+ }
+
error = mxt_acquire_irq(data);
if (error)
goto err_free_object_table;
--
2.16.2
^ permalink raw reply related
* Re: [PATCH v5 0/2] Remove false-positive VLAs when using max()
From: Linus Torvalds @ 2018-03-22 17:04 UTC (permalink / raw)
To: Kees Cook
Cc: Al Viro, Florian Weimer, Andrew Morton, Josh Poimboeuf,
Rasmus Villemoes, Randy Dunlap, Miguel Ojeda, Ingo Molnar,
David Laight, Ian Abbott, linux-input, linux-btrfs,
Network Development, Linux Kernel Mailing List, Kernel Hardening
In-Reply-To: <CAGXu5j+5i+56R0KDLMDA=+_DRW5w9aUGCEo0dq6PZvHPBWkM1g@mail.gmail.com>
On Thu, Mar 22, 2018 at 8:01 AM, Kees Cook <keescook@chromium.org> wrote:
>
> Seems like it doesn't like void * arguments:
Yeah, that was discussed separately, I just didn't realize we had any
such users.
As David said, just adding a (long) cast to it should be fine, ie
#define __is_constant(a) \
(sizeof(int) == sizeof(*(1 ? ((void*)((long)(a) * 0l)) : (int*)1)))
and is probably a good idea even outside of pointers (because "long
long" constants could cause warnings too due to the cast to (void *)).
Linus
^ permalink raw reply
* Re: [PATCH v2 05/14] platform/chrome: chromeos_laptop - stop setting suspend mode for Atmel devices
From: Benson Leung @ 2018-03-22 23:34 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Benson Leung, Nick Dyer, Olof Johansson,
linux-kernel
In-Reply-To: <20180320223138.234724-6-dmitry.torokhov@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]
Hi Dmitry,
On Tue, Mar 20, 2018 at 03:31:29PM -0700, Dmitry Torokhov wrote:
> Atmel touch controller driver no longer respects suspend mode specified in
> platform data, so let's stop setting it.
>
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Applied to my working branch for atmel_mxt_ts + chromeos_laptop.c for
v4.17. Thanks!
> ---
> drivers/platform/chrome/chromeos_laptop.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/platform/chrome/chromeos_laptop.c b/drivers/platform/chrome/chromeos_laptop.c
> index 54a13c70e1d8f..0a43f1833de3f 100644
> --- a/drivers/platform/chrome/chromeos_laptop.c
> +++ b/drivers/platform/chrome/chromeos_laptop.c
> @@ -100,7 +100,6 @@ static struct mxt_platform_data atmel_224s_tp_platform_data = {
> .irqflags = IRQF_TRIGGER_FALLING,
> .t19_num_keys = ARRAY_SIZE(mxt_t19_keys),
> .t19_keymap = mxt_t19_keys,
> - .suspend_mode = MXT_SUSPEND_T9_CTRL,
> };
>
> static struct i2c_board_info atmel_224s_tp_device = {
> @@ -111,7 +110,6 @@ static struct i2c_board_info atmel_224s_tp_device = {
>
> static struct mxt_platform_data atmel_1664s_platform_data = {
> .irqflags = IRQF_TRIGGER_FALLING,
> - .suspend_mode = MXT_SUSPEND_T9_CTRL,
> };
>
> static struct i2c_board_info atmel_1664s_device = {
> --
> 2.16.2.804.g6dcf76e118-goog
>
--
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Tirupathi Reddy @ 2018-03-23 6:23 UTC (permalink / raw)
To: dmitry.torokhov, bjorn.andersson, robh+dt, mark.rutland
Cc: tirupath, linux-input, linux-arm-msm, devicetree, linux-kernel
Add resin key support to handle different types of key events
defined in different platforms.
Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
---
.../bindings/input/qcom,pm8941-pwrkey.txt | 32 +++++++++
drivers/input/misc/pm8941-pwrkey.c | 81 ++++++++++++++++++++++
2 files changed, 113 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
index 07bf55f..c671636 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
+++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt
@@ -32,6 +32,32 @@ PROPERTIES
Definition: presence of this property indicates that the KPDPWR_N pin
should be configured for pull up.
+RESIN SUBNODE
+
+The HW module can generate other optional key events like RESIN(reset-in pin).
+The RESIN pin can be configured to support different key events on different
+platforms. The resin key is described by the following properties.
+
+- interrupts:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: key change interrupt; The format of the specifier is
+ defined by the binding document describing the node's
+ interrupt parent.
+
+- linux,code:
+ Usage: required
+ Value type: <u32>
+ Definition: The input key-code associated with the resin key.
+ Use the linux event codes defined in
+ include/dt-bindings/input/linux-event-codes.h
+
+- bias-pull-up:
+ Usage: optional
+ Value type: <empty>
+ Definition: presence of this property indicates that the RESIN pin
+ should be configured for pull up.
+
EXAMPLE
pwrkey@800 {
@@ -40,4 +66,10 @@ EXAMPLE
interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>;
debounce = <15625>;
bias-pull-up;
+
+ resin {
+ interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>;
+ linux,code = <KEY_VOLUMEDOWN>;
+ bias-pull-up;
+ };
};
diff --git a/drivers/input/misc/pm8941-pwrkey.c b/drivers/input/misc/pm8941-pwrkey.c
index 18ad956..6e45d01 100644
--- a/drivers/input/misc/pm8941-pwrkey.c
+++ b/drivers/input/misc/pm8941-pwrkey.c
@@ -20,6 +20,7 @@
#include <linux/log2.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_irq.h>
#include <linux/platform_device.h>
#include <linux/reboot.h>
#include <linux/regmap.h>
@@ -28,6 +29,7 @@
#define PON_RT_STS 0x10
#define PON_KPDPWR_N_SET BIT(0)
+#define PON_RESIN_N_SET BIT(1)
#define PON_PS_HOLD_RST_CTL 0x5a
#define PON_PS_HOLD_RST_CTL2 0x5b
@@ -37,6 +39,7 @@
#define PON_PS_HOLD_TYPE_HARD_RESET 7
#define PON_PULL_CTL 0x70
+#define PON_RESIN_PULL_UP BIT(0)
#define PON_KPDPWR_PULL_UP BIT(1)
#define PON_DBC_CTL 0x71
@@ -46,6 +49,7 @@
struct pm8941_pwrkey {
struct device *dev;
int irq;
+ u32 resin_key_code;
u32 baseaddr;
struct regmap *regmap;
struct input_dev *input;
@@ -130,6 +134,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void *_data)
return IRQ_HANDLED;
}
+static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
+{
+ struct pm8941_pwrkey *pwrkey = _data;
+ unsigned int sts;
+ int error;
+ u32 key_code = pwrkey->resin_key_code;
+
+ error = regmap_read(pwrkey->regmap,
+ pwrkey->baseaddr + PON_RT_STS, &sts);
+ if (error)
+ return IRQ_HANDLED;
+
+ input_report_key(pwrkey->input, key_code, !!(sts & PON_RESIN_N_SET));
+ input_sync(pwrkey->input);
+
+ return IRQ_HANDLED;
+}
+
static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
{
struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
@@ -153,9 +175,56 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
+static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey,
+ struct device_node *np)
+{
+ int error, irq;
+ bool pull_up;
+
+ /*
+ * Get the standard-key parameters. This might not be
+ * specified if there is no key mapping on the reset line.
+ */
+ error = of_property_read_u32(np, "linux,code", &pwrkey->resin_key_code);
+ if (error) {
+ dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
+ return error;
+ }
+
+ pull_up = of_property_read_bool(np, "bias-pull-up");
+
+ irq = irq_of_parse_and_map(np, 0);
+ if (irq < 0) {
+ dev_err(pwrkey->dev, "failed to get resin irq\n");
+ return -EINVAL;
+ }
+
+ /* Register key configuration */
+ input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
+
+ error = regmap_update_bits(pwrkey->regmap,
+ pwrkey->baseaddr + PON_PULL_CTL,
+ PON_RESIN_PULL_UP,
+ pull_up ? PON_RESIN_PULL_UP : 0);
+ if (error) {
+ dev_err(pwrkey->dev, "failed to set resin pull: %d\n", error);
+ return error;
+ }
+
+ error = devm_request_threaded_irq(pwrkey->dev, irq, NULL,
+ pm8941_resinkey_irq, IRQF_ONESHOT,
+ "pm8941_resinkey", pwrkey);
+ if (error)
+ dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
+ error);
+
+ return error;
+}
+
static int pm8941_pwrkey_probe(struct platform_device *pdev)
{
struct pm8941_pwrkey *pwrkey;
+ struct device_node *np = NULL;
bool pull_up;
u32 req_delay;
int error;
@@ -241,6 +310,18 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
return error;
}
+ np = of_find_node_by_name(pdev->dev.of_node, "resin");
+ if (np) {
+ /* resin key capabilities are defined in device node */
+ error = pm8941_resin_key_init(pwrkey, np);
+ of_node_put(np);
+ if (error) {
+ dev_err(&pdev->dev, "failed resin key initialization: %d\n",
+ error);
+ return error;
+ }
+ }
+
error = input_register_device(pwrkey->input);
if (error) {
dev_err(&pdev->dev, "failed to register input device: %d\n",
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply related
* Re: [PATCH 0/7] HID core and multitouch fixups
From: Jiri Kosina @ 2018-03-23 13:55 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20180320110451.16582-1-benjamin.tissoires@redhat.com>
On Tue, 20 Mar 2018, Benjamin Tissoires wrote:
> Patches 1 and 2 are related to the Razer Blade Stealth that has some dead zone
> near the edges of the touchpad.
> Patches 3 was previously sent and reviewed by Dmitry and he suggested patch 4
> at the time.
> Patches 5..7 are cleanups I realized while trying to merge hid-multitouch
> into hid-core, so that other drivers could reuse the hid-mt logic (but it's
> not that easy I must confess).
I've split this into two logical sets, and applied. Thanks!
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)
From: Dennis Wassenberg @ 2018-03-23 14:23 UTC (permalink / raw)
To: Pali Rohár, Dmitry Torokhov, Masaki Ota, Takashi Iwai,
Kees Cook, Nir Perry
Cc: linux-input, linux-kernel
The Lenovo Thinkpad L570 uses V8 protocol.
Add 0x73 0x03 0x28 devices to use V8 protovol which makes
trackstick and mouse buttons work with Lenovo Thinkpad L570.
Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
---
drivers/input/mouse/alps.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
index dbe57da..5523d4e 100644
--- a/drivers/input/mouse/alps.c
+++ b/drivers/input/mouse/alps.c
@@ -136,6 +136,8 @@
{ { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
{ { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } }, /* Ahtec Laptop */
{ { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } }, /* Dell Vostro 1400 */
+ { { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
+ ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } }, /* Lenovo L570 */
};
static const struct alps_protocol_info alps_v3_protocol_data = {
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] Input: ALPS - add support for 73 03 28 devices (Thinkpad L570)
From: Pali Rohár @ 2018-03-23 14:33 UTC (permalink / raw)
To: Dennis Wassenberg
Cc: Dmitry Torokhov, Masaki Ota, Takashi Iwai, Kees Cook, Nir Perry,
linux-input, linux-kernel
In-Reply-To: <7d5077ef-c185-c6b1-d602-df8d9f7dade3@secunet.com>
On Friday 23 March 2018 15:23:55 Dennis Wassenberg wrote:
> The Lenovo Thinkpad L570 uses V8 protocol.
> Add 0x73 0x03 0x28 devices to use V8 protovol which makes
> trackstick and mouse buttons work with Lenovo Thinkpad L570.
>
> Signed-off-by: Dennis Wassenberg <dennis.wassenberg@secunet.com>
> ---
> drivers/input/mouse/alps.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> index dbe57da..5523d4e 100644
> --- a/drivers/input/mouse/alps.c
> +++ b/drivers/input/mouse/alps.c
> @@ -136,6 +136,8 @@
> { { 0x73, 0x02, 0x0a }, { ALPS_PROTO_V2, 0xf8, 0xf8, 0 } },
> { { 0x73, 0x02, 0x14 }, { ALPS_PROTO_V2, 0xf8, 0xf8, ALPS_FW_BK_2 } }, /* Ahtec Laptop */
> { { 0x73, 0x02, 0x50 }, { ALPS_PROTO_V2, 0xcf, 0xcf, ALPS_FOUR_BUTTONS } }, /* Dell Vostro 1400 */
> + { { 0x73, 0x03, 0x28 }, { ALPS_PROTO_V8, 0x18, 0x18,
> + ALPS_DUALPOINT | ALPS_DUALPOINT_WITH_PRESSURE | ALPS_BUTTONPAD } }, /* Lenovo L570 */
> };
>
> static const struct alps_protocol_info alps_v3_protocol_data = {
Hi! alps_model_data table is used for fixed identification of v1 and v2
protocols. Why you need to add there v8 protocol which autodetection is
already done in alps_identify() function? There is already code:
} else if (e7[0] == 0x73 && e7[1] == 0x03 &&
(e7[2] == 0x14 || e7[2] == 0x28)) {
protocol = &alps_v8_protocol_data;
Which matches above your E7 detection 0x73, 0x03, 0x28.
Also you patch matches basically all v8 device and therefore has
potential to break proper v8 autodetection for other v8 devices...
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* Re: [PATCH v2 1/4] dt-bindings: mfd: Add Gateworks System Controller bindings
From: Tim Harvey @ 2018-03-23 17:11 UTC (permalink / raw)
To: Rob Herring
Cc: Lee Jones, Mark Rutland, Mark Brown, Dmitry Torokhov,
linux-kernel, devicetree, linux-arm-kernel, linux-hwmon,
linux-input
In-Reply-To: <20180309231433.tmlfy6vwbkvopmwd@rob-hp-laptop>
On Fri, Mar 9, 2018 at 3:14 PM, Rob Herring <robh@kernel.org> wrote:
> On Mon, Mar 05, 2018 at 02:02:38PM -0800, Tim Harvey wrote:
>> This patch adds documentation of device-tree bindings for the
>> Gateworks System Controller (GSC).
>>
>> Signed-off-by: Tim Harvey <tharvey@gateworks.com>
>> ---
>> Documentation/devicetree/bindings/mfd/gsc.txt | 159 ++++++++++++++++++++++++++
>> 1 file changed, 159 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/mfd/gsc.txt
>>
<snip>
>> +
>> +* hwmon:
>> +The GSC provides a set of Analog to Digitcal Converter (ADC) pins used for
>> +temperature and/or voltage monitoring.
>> +
>> +Required properties:
>> +- compatible: must be "gw,gsc-hwmon"
>> +
<snip>
>> +
>> + gsc_hwmon {
>
> hwmon {
>
>> + compatible = "gw,gsc-hwmon";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + hwmon@0 { /* A0: Board Temperature */
>> + type = <0>;
>
> Not documented.
>
>> + reg = <0x00>;
>> + label = "temp";
>> + };
>> +
>> + hwmon@1 { /* A1: Input Voltage */
>> + type = <1>;
>> + reg = <0x02>;
>> + label = "Vin";
>> + };
>> +
<snip>
>> +
>> + hwmon@15 { /* fan0 */
>> + type = <2>;
>> + reg = <0x2c>;
>> + label = "fan_50p";
>> + };
>> +
<snip>
>> + };
>> + };
Hi Rob,
Thanks for the review. I will roll in your changes.
I'm looking for suggestions on how to better define the child nodes of
the hwmon sub-device:
I have 4 types of hwmon child nodes supported by this device:
1. temperature in C/10 (is that called deci-celcuis?): this is a 2byte value
2. input voltage in mV: this is a 3 byte value pre-scaled by the GSC
firmware from a 10bit ADC where the
ref/bit-resolution/resistor-divider info is baked into the per-board
firmware
3. fan setpoint in C/10
4. input voltage in mV: raw 2-byte value of a 12bit ADC that needs to
be scaled based on a voltage-devider (pre-scaler), a ref voltage, and
resolution (or bit width)
The example I posted above shows use of the first three types but not
the more complicated fourth which requires the driver know more
details in order to present a cooked milivolt value. Note that the two
input types above would not both be present in any single board dts as
they represent a change in the way ADC's are reported in the overall
GSC version. A GSC typically has up to 16 different input ADC's and
the voltage rails monitored vary per board.
You mention that I don't document 'type' and your correct. I'm
thinking this is best done with a string using compatible within the
child node.
Something like:
Required properties:
...
- hwmon: This is the list of child nodes that specify the hwmon nodes.
...
hwmon required properties:
- hwmon-compatible: Must be "fan-setpoint", "temperature",
"input-raw", "input-cooked"
- reg: register offset of the node
- label: name of the input node
hwmon optional properties:
- gw,voltage-divider: an array of two integers containing the resistor
values R1 and R2 of the voltage divider in ohms
- gw,reference-voltage: the internal reference voltage of the ADC
input in milivolts
- gw,resolution: the resolution of the ADC input (ie 4096 for 12bit resolution)
...
Example:
...
hwmon {
compatible = "gw,gsc-hwmon";
#address-cells = <1>;
#size-cells = <0>;
hwmon@0 {
compatible = "temperature";
reg <0x00>;
label = "temp";
};
hwmon@2 {
compatible = "fan-setpoint";
reg <0x02>;
label = "fan_setpoint0";
};
/* this one represents the 3-byte ADC value that has been
'pre-scaled' by the GSC from a raw 10bit ADC (older GSC
version/firmware) */
hwmon@4 {
compatible = "input-cooked";
reg <0x02>;
label = "voltage1";
};
/* this one represents the 2-byte 12-bit ADC value that is 'raw'
from the GSC and needs to be scaled by driver (newer GSC
version/firmware) */
hwmon@4 {
compatible = "input-raw";
reg <0x04>;
label = "voltage2";
gw,voltage-divider = <10000 10000>; /* R1=10k R2=10k div-by-2 pre-scaler */
gw,referance-voltage = <2500>; /* 2.5V reference */
gw,resolution = <4096>; /* 12bit resolution */
};
};
If I add a byte-size (2|3) or bit-width (16|24) I could essentially
combine the two input types.
Suggestions?
Regards,
Tim
^ permalink raw reply
* Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update
From: Dmitry Torokhov @ 2018-03-23 18:25 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, Nick Dyer, linux-input, Henrik Rydberg,
linux-kernel, kernel, Nandor Han
In-Reply-To: <20180322164330.24809-1-sebastian.reichel@collabora.co.uk>
Hi Sebastian, Nandor,
On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:
> From: Nandor Han <nandor.han@ge.com>
>
> The automatic update mechanism will trigger an update if the
> info block CRCs are different between maxtouch configuration
> file (maxtouch.cfg) and chip.
>
> The driver compared the CRCs without retrieving the chip CRC,
> resulting always in a failure and firmware flashing action
> triggered. The patch will fix this issue by retrieving the
> chip info block CRC before the check.
>
> Suggested-by: Todd Weyenberg <Todd.Weyenberg@med.ge.com>
> Signed-off-by: Nandor Han <nandor.han@ge.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
> ---
> drivers/input/touchscreen/atmel_mxt_ts.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
> index 7659bc48f1db..ab936e6b0286 100644
> --- a/drivers/input/touchscreen/atmel_mxt_ts.c
> +++ b/drivers/input/touchscreen/atmel_mxt_ts.c
> @@ -1728,6 +1728,25 @@ static int mxt_get_object_table(struct mxt_data *data)
> return error;
> }
>
> +static int mxt_get_info_block_crc(struct mxt_data *data)
> +{
> + size_t table_size;
> + int error;
> + u8 info_block_crc[MXT_INFO_CHECKSUM_SIZE];
> +
> + table_size = data->info.object_num * sizeof(struct mxt_object);
> +
> + /* Read the info block CRC */
> + error = __mxt_read_reg(data->client, MXT_OBJECT_START + table_size,
> + sizeof(info_block_crc), info_block_crc);
> + if (!error) {
> + data->info_crc = info_block_crc[0] | (info_block_crc[1] << 8) |
> + (info_block_crc[2] << 16);
> + }
> +
> + return error;
> +}
> +
> static int mxt_read_t9_resolution(struct mxt_data *data)
> {
> struct i2c_client *client = data->client;
> @@ -2077,6 +2096,14 @@ static int mxt_initialize(struct mxt_data *data)
> return error;
> }
>
> + /* Get info block CRC */
> + error = mxt_get_info_block_crc(data);
> + if (error) {
> + dev_err(&client->dev, "Error %d reading info block CRC\n",
> + error);
> + return error;
You are leaking object table memory here.
By the way, why do we do this here, and not when we actually have config
file and is about to apply it?
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v6 0/6] Add MediaTek PMIC keys support
From: Dmitry Torokhov @ 2018-03-23 18:54 UTC (permalink / raw)
To: Sean Wang
Cc: Rob Herring, Lee Jones, Alexandre Belloni, Mark Rutland, a.zummo,
devicetree, linus.walleij, jcsing.lee, linux-kernel, krzk, javier,
linux-mediatek, linux-arm-kernel, linux-input, matthias.bgg,
eddie.huang, Chen.Zhong, beomho.seo, linux-rtc
In-Reply-To: <1521685073.28226.10.camel@mtkswgap22>
Hi Sean,
On Thu, Mar 22, 2018 at 10:17:53AM +0800, Sean Wang wrote:
> Hi, Dmitry and Lee
>
> The series seems not being got merged. Are they good enough to be ready
> into the your tree?
>
> Recently I've tested the series with focusing on pwrkey event generated
> through interrupt when push and release the key on bpi-r2 board and then
> finally it's working fine. but for homekey it cannot be found on the
> board and thus I cannot have more tests more about it.
>
> Tested-by: Sean Wang <sean.wang@mediatek.com>
You have my Ack on the input patch; I expect it go through Lee's tree as
there are some dependencies on mfd core piece.
Thanks.
>
> Sean
>
>
> On Thu, 2018-03-22 at 10:03 +0800, Sean Wang (王志亘) wrote:
> >
> > > -----Original Message-----
> > > From: Linux-mediatek [mailto:linux-mediatek-bounces@lists.infradead.org] On
> > > Behalf Of Chen Zhong
> > > Sent: Wednesday, October 25, 2017 9:16 PM
> > > To: Dmitry Torokhov <dmitry.torokhov@gmail.com>; Rob Herring
> > > <robh+dt@kernel.org>; Lee Jones <lee.jones@linaro.org>; Alexandre Belloni
> > > <alexandre.belloni@free-electrons.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>; Alessandro Zummo
> > > <a.zummo@towertech.it>; devicetree@vger.kernel.org; Linus Walleij
> > > <linus.walleij@linaro.org>; Jaechul Lee <jcsing.lee@samsung.com>; linux-
> > > kernel@vger.kernel.org; Krzysztof Kozlowski <krzk@kernel.org>; Javier
> > > Martinez Canillas <javier@osg.samsung.com>; linux-
> > > mediatek@lists.infradead.org; linux-arm-kernel@lists.infradead.org; linux-
> > > input@vger.kernel.org; Matthias Brugger <matthias.bgg@gmail.com>; Eddie
> > > Huang (S智) <eddie.huang@mediatek.com>; Chen Zhong (钟辰)
> > > <Chen.Zhong@mediatek.com>; Beomho Seo <beomho.seo@samsung.com>;
> > > linux-rtc@vger.kernel.org
> > > Subject: [PATCH v6 0/6] Add MediaTek PMIC keys support
> > >
> > > MediaTek PMIC are multi-function devices that can handle key interrupts,
> > > typically there are two keys attached to PMIC, which called pwrkey and
> > > homekey. PWRKEY usually used to wake up system from sleep. Homekey can
> > > used as volume down key due to board design. Long press keys can shutdown
> > > PMIC, the mode can be choose to be one key only or two keys together.
> > > This series add support for key functions for MediaTek PMIC MT6397/MT6323.
> > >
> > > Changes since v5:
> > > - use __maybe_unused annotation instead of #ifdef guard
> > > - use of_* API instead of device_* API
> > >
> > > Changes since v4:
> > > - rebase to Linux 4.14-rc1
> > > - add a common keyboard binding document
> > > - use child device tree node to define each key
> > >
> > > Changes since v3:
> > > - make the naming to be consistent as mtk_pmic or MTK_PMIC
> > > - add suspend/resume functions to enable/disable irq
> > > - change binding properties to define wakeup sources
> > >
> > > Changes since v2:
> > > - use standard properties for keycodes and debounce time
> > > - change to use platform_get_irq in leaf drivers
> > > - use better ways to define IRQ resources
> > >
> > > Changes since v1:
> > > - create irq mappings in mfd core driver instead of leaf drivers
> > > - remove some unused parts in mtk-pmic-keys driver
> > >
> > > Chen Zhong (6):
> > > mfd: mt6397: create irq mappings in mfd core driver
> > > dt-bindings: input: Add common keyboard document bindings
> > > dt-bindings: input: Add document bindings for mtk-pmic-keys
> > > dt-bindings: mfd: Add bindings for the keys as subnode of PMIC
> > > input: Add MediaTek PMIC keys support
> > > mfd: mt6397: Add PMIC keys support to MT6397 driver
> > >
> > > Documentation/devicetree/bindings/input/keys.txt | 8 +
> > > .../devicetree/bindings/input/mtk-pmic-keys.txt | 43 +++
> > > Documentation/devicetree/bindings/mfd/mt6397.txt | 6 +
> > > drivers/input/keyboard/Kconfig | 9 +
> > > drivers/input/keyboard/Makefile | 1 +
> > > drivers/input/keyboard/mtk-pmic-keys.c | 339
> > > +++++++++++++++++++++
> > > drivers/mfd/mt6397-core.c | 26 +-
> > > drivers/rtc/rtc-mt6397.c | 7 +-
> > > 8 files changed, 432 insertions(+), 7 deletions(-) create mode 100644
> > > Documentation/devicetree/bindings/input/keys.txt
> > > create mode 100644 Documentation/devicetree/bindings/input/mtk-pmic-
> > > keys.txt
> > > create mode 100644 drivers/input/keyboard/mtk-pmic-keys.c
> > >
> > > --
> > > 1.9.1
> > >
> > >
> > >
> > > _______________________________________________
> > > Linux-mediatek mailing list
> > > Linux-mediatek@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-mediatek
>
>
--
Dmitry
^ permalink raw reply
* Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Trilok Soni @ 2018-03-23 19:14 UTC (permalink / raw)
To: Tirupathi Reddy, dmitry.torokhov, bjorn.andersson, robh+dt,
mark.rutland
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <1521786192-19487-1-git-send-email-tirupath@codeaurora.org>
Hi Tirupathi,
>
> +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> +{
> + struct pm8941_pwrkey *pwrkey = _data;
> + unsigned int sts;
> + int error;
> + u32 key_code = pwrkey->resin_key_code;
> +
> + error = regmap_read(pwrkey->regmap,
> + pwrkey->baseaddr + PON_RT_STS, &sts);
> + if (error)
> + return IRQ_HANDLED;
> +
> + input_report_key(pwrkey->input, key_code, !!(sts & PON_RESIN_N_SET));
> + input_sync(pwrkey->input);
> +
> + return IRQ_HANDLED;
> +}
> +
> static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
> {
> struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
> @@ -153,9 +175,56 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
> static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
> pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
>
> +static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey,
> + struct device_node *np)
> +{
> + int error, irq;
> + bool pull_up;
> +
> + /*
> + * Get the standard-key parameters. This might not be
> + * specified if there is no key mapping on the reset line.
> + */
> + error = of_property_read_u32(np, "linux,code", &pwrkey->resin_key_code);
> + if (error) {
> + dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
> + return error;
> + }
> +
> + pull_up = of_property_read_bool(np, "bias-pull-up");
> +
> + irq = irq_of_parse_and_map(np, 0);
> + if (irq < 0) {
> + dev_err(pwrkey->dev, "failed to get resin irq\n");
> + return -EINVAL;
> + }
> +
> + /* Register key configuration */
> + input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
> +
> + error = regmap_update_bits(pwrkey->regmap,
> + pwrkey->baseaddr + PON_PULL_CTL,
> + PON_RESIN_PULL_UP,
> + pull_up ? PON_RESIN_PULL_UP : 0);
> + if (error) {
> + dev_err(pwrkey->dev, "failed to set resin pull: %d\n", error);
> + return error;
> + }
> +
> + error = devm_request_threaded_irq(pwrkey->dev, irq, NULL,
> + pm8941_resinkey_irq, IRQF_ONESHOT,
> + "pm8941_resinkey", pwrkey);
> + if (error)
> + dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
> + error);
> +
> + return error;
> +}
> +
> static int pm8941_pwrkey_probe(struct platform_device *pdev)
> {
> struct pm8941_pwrkey *pwrkey;
> + struct device_node *np = NULL;
> bool pull_up;
> u32 req_delay;
> int error;
> @@ -241,6 +310,18 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
> return error;
> }
>
> + np = of_find_node_by_name(pdev->dev.of_node, "resin");
> + if (np) {
> + /* resin key capabilities are defined in device node */
> + error = pm8941_resin_key_init(pwrkey, np);
What happens if interrupts comes as soon as you have done the
key_init(..) here? I see that you are registering the input device after
it, right? Looks like you should do this at the end of probe?
> + of_node_put(np);
> + if (error) {
> + dev_err(&pdev->dev, "failed resin key initialization: %d\n",
> + error);
> + return error;
> + }
> + }
> +
> error = input_register_device(pwrkey->input);
> if (error) {
> dev_err(&pdev->dev, "failed to register input device: %d\n",
>
--
---Trilok Soni
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Dmitry Torokhov @ 2018-03-23 19:22 UTC (permalink / raw)
To: Trilok Soni
Cc: Tirupathi Reddy, bjorn.andersson, robh+dt, mark.rutland,
linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <240c6cbc-886f-94c6-cba4-3c4eb3ed0b45@codeaurora.org>
On Fri, Mar 23, 2018 at 12:14:22PM -0700, Trilok Soni wrote:
> Hi Tirupathi,
>
> > +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data)
> > +{
> > + struct pm8941_pwrkey *pwrkey = _data;
> > + unsigned int sts;
> > + int error;
> > + u32 key_code = pwrkey->resin_key_code;
> > +
> > + error = regmap_read(pwrkey->regmap,
> > + pwrkey->baseaddr + PON_RT_STS, &sts);
> > + if (error)
> > + return IRQ_HANDLED;
> > +
> > + input_report_key(pwrkey->input, key_code, !!(sts & PON_RESIN_N_SET));
> > + input_sync(pwrkey->input);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev)
> > {
> > struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev);
> > @@ -153,9 +175,56 @@ static int __maybe_unused pm8941_pwrkey_resume(struct device *dev)
> > static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops,
> > pm8941_pwrkey_suspend, pm8941_pwrkey_resume);
> > +static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey,
> > + struct device_node *np)
> > +{
> > + int error, irq;
> > + bool pull_up;
> > +
> > + /*
> > + * Get the standard-key parameters. This might not be
> > + * specified if there is no key mapping on the reset line.
> > + */
> > + error = of_property_read_u32(np, "linux,code", &pwrkey->resin_key_code);
> > + if (error) {
> > + dev_err(pwrkey->dev, "failed to read key-code for resin key\n");
> > + return error;
> > + }
> > +
> > + pull_up = of_property_read_bool(np, "bias-pull-up");
> > +
> > + irq = irq_of_parse_and_map(np, 0);
> > + if (irq < 0) {
> > + dev_err(pwrkey->dev, "failed to get resin irq\n");
> > + return -EINVAL;
> > + }
> > +
> > + /* Register key configuration */
> > + input_set_capability(pwrkey->input, EV_KEY, pwrkey->resin_key_code);
> > +
> > + error = regmap_update_bits(pwrkey->regmap,
> > + pwrkey->baseaddr + PON_PULL_CTL,
> > + PON_RESIN_PULL_UP,
> > + pull_up ? PON_RESIN_PULL_UP : 0);
> > + if (error) {
> > + dev_err(pwrkey->dev, "failed to set resin pull: %d\n", error);
> > + return error;
> > + }
> > +
> > + error = devm_request_threaded_irq(pwrkey->dev, irq, NULL,
> > + pm8941_resinkey_irq, IRQF_ONESHOT,
> > + "pm8941_resinkey", pwrkey);
> > + if (error)
> > + dev_err(pwrkey->dev, "failed requesting resin key IRQ: %d\n",
> > + error);
> > +
> > + return error;
> > +}
> > +
> > static int pm8941_pwrkey_probe(struct platform_device *pdev)
> > {
> > struct pm8941_pwrkey *pwrkey;
> > + struct device_node *np = NULL;
> > bool pull_up;
> > u32 req_delay;
> > int error;
> > @@ -241,6 +310,18 @@ static int pm8941_pwrkey_probe(struct platform_device *pdev)
> > return error;
> > }
> > + np = of_find_node_by_name(pdev->dev.of_node, "resin");
> > + if (np) {
> > + /* resin key capabilities are defined in device node */
> > + error = pm8941_resin_key_init(pwrkey, np);
>
>
> What happens if interrupts comes as soon as you have done the key_init(..)
> here? I see that you are registering the input device after it, right? Looks
> like you should do this at the end of probe?
It should be fine, input devices properly allocated (with
input_allocate_device() or devm- version) are able to accept events even
if input device has not been registered yet. It is and advertised
feature:
drivers/input/input.c:
"
* input_event() - report new input event
...
* NOTE: input_event() may be safely used right after input device was
* allocated with input_allocate_device(), even before it is registered
* with input_register_device(), but the event will not reach any of the
* input handlers. Such early invocation of input_event() may be used
* to 'seed' initial state of a switch or initial position of absolute
* axis, etc.
"
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Trilok Soni @ 2018-03-23 19:34 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Tirupathi Reddy, bjorn.andersson, robh+dt, mark.rutland,
linux-input, linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20180323192210.GE255819@dtor-ws>
Hi Dmitry,
>>
>>
>> What happens if interrupts comes as soon as you have done the key_init(..)
>> here? I see that you are registering the input device after it, right? Looks
>> like you should do this at the end of probe?
>
> It should be fine, input devices properly allocated (with
> input_allocate_device() or devm- version) are able to accept events even
> if input device has not been registered yet. It is and advertised
> feature:
>
> drivers/input/input.c:
>
> "
> * input_event() - report new input event
> ...
> * NOTE: input_event() may be safely used right after input device was
> * allocated with input_allocate_device(), even before it is registered
> * with input_register_device(), but the event will not reach any of the
> * input handlers. Such early invocation of input_event() may be used
> * to 'seed' initial state of a switch or initial position of absolute
> * axis, etc.
Thanks. Looks good since input_allocate_device(...) is called before the
init.
--
---Trilok Soni
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply
* Re: [PATCHv1] Input: atmel_mxt_ts - fix the firmware update
From: Nick Dyer @ 2018-03-23 19:47 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Sebastian Reichel, Dmitry Torokhov, linux-input, Henrik Rydberg,
linux-kernel, kernel, Nandor Han
In-Reply-To: <20180322164330.24809-1-sebastian.reichel@collabora.co.uk>
On Thu, Mar 22, 2018 at 05:43:30PM +0100, Sebastian Reichel wrote:
> The automatic update mechanism will trigger an update if the
> info block CRCs are different between maxtouch configuration
> file (maxtouch.cfg) and chip.
>
> The driver compared the CRCs without retrieving the chip CRC,
> resulting always in a failure and firmware flashing action
> triggered. The patch will fix this issue by retrieving the
> chip info block CRC before the check.
Thanks for raising this, I agree it's definitely something we want to
fix.
However, I'm not convinced you're solving the problem in the best way.
You've attached it to the read_t9_resolution() code path, whereas the
info block is common between T9 and T100 and works in the same way.
Would you mind trying the below patch? I've dusted it off from some
work that I did back in 2012 and it should solve your issue.
It also has the benefit that by reading the information block and the
object table into a contiguous region of memory, we can verify the
checksum at probe time. This means we make sure that we are indeed
talking to a chip that supports object protocol correctly.
Signed-off-by: Nick Dyer <nick.dyer@shmanahar.org>
Acked-by: Benson Leung <bleung@chromium.org>
---
drivers/input/touchscreen/atmel_mxt_ts.c | 193 +++++++++++++++++++------------
1 file changed, 117 insertions(+), 76 deletions(-)
diff --git a/drivers/input/touchscreen/atmel_mxt_ts.c b/drivers/input/touchscreen/atmel_mxt_ts.c
index 7659bc48f1db..8a60d91d49a6 100644
--- a/drivers/input/touchscreen/atmel_mxt_ts.c
+++ b/drivers/input/touchscreen/atmel_mxt_ts.c
@@ -275,7 +275,8 @@ struct mxt_data {
char phys[64]; /* device physical location */
const struct mxt_platform_data *pdata;
struct mxt_object *object_table;
- struct mxt_info info;
+ struct mxt_info *info;
+ void *raw_info_block;
unsigned int irq;
unsigned int max_x;
unsigned int max_y;
@@ -450,12 +451,13 @@ static int mxt_lookup_bootloader_address(struct mxt_data *data, bool retry)
{
u8 appmode = data->client->addr;
u8 bootloader;
+ u8 family_id = data->info ? data->info->family_id : 0;
switch (appmode) {
case 0x4a:
case 0x4b:
/* Chips after 1664S use different scheme */
- if (retry || data->info.family_id >= 0xa2) {
+ if (retry || family_id >= 0xa2) {
bootloader = appmode - 0x24;
break;
}
@@ -682,7 +684,7 @@ mxt_get_object(struct mxt_data *data, u8 type)
struct mxt_object *object;
int i;
- for (i = 0; i < data->info.object_num; i++) {
+ for (i = 0; i < data->info->object_num; i++) {
object = data->object_table + i;
if (object->type == type)
return object;
@@ -1453,12 +1455,12 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
data_pos += offset;
}
- if (cfg_info.family_id != data->info.family_id) {
+ if (cfg_info.family_id != data->info->family_id) {
dev_err(dev, "Family ID mismatch!\n");
return -EINVAL;
}
- if (cfg_info.variant_id != data->info.variant_id) {
+ if (cfg_info.variant_id != data->info->variant_id) {
dev_err(dev, "Variant ID mismatch!\n");
return -EINVAL;
}
@@ -1503,7 +1505,7 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
/* Malloc memory to store configuration */
cfg_start_ofs = MXT_OBJECT_START +
- data->info.object_num * sizeof(struct mxt_object) +
+ data->info->object_num * sizeof(struct mxt_object) +
MXT_INFO_CHECKSUM_SIZE;
config_mem_size = data->mem_size - cfg_start_ofs;
config_mem = kzalloc(config_mem_size, GFP_KERNEL);
@@ -1554,20 +1556,6 @@ static int mxt_update_cfg(struct mxt_data *data, const struct firmware *cfg)
return ret;
}
-static int mxt_get_info(struct mxt_data *data)
-{
- struct i2c_client *client = data->client;
- struct mxt_info *info = &data->info;
- int error;
-
- /* Read 7-byte info block starting at address 0 */
- error = __mxt_read_reg(client, 0, sizeof(*info), info);
- if (error)
- return error;
-
- return 0;
-}
-
static void mxt_free_input_device(struct mxt_data *data)
{
if (data->input_dev) {
@@ -1582,9 +1570,10 @@ static void mxt_free_object_table(struct mxt_data *data)
video_unregister_device(&data->dbg.vdev);
v4l2_device_unregister(&data->dbg.v4l2);
#endif
-
- kfree(data->object_table);
data->object_table = NULL;
+ data->info = NULL;
+ kfree(data->raw_info_block);
+ data->raw_info_block = NULL;
kfree(data->msg_buf);
data->msg_buf = NULL;
data->T5_address = 0;
@@ -1600,34 +1589,18 @@ static void mxt_free_object_table(struct mxt_data *data)
data->max_reportid = 0;
}
-static int mxt_get_object_table(struct mxt_data *data)
+static int mxt_parse_object_table(struct mxt_data *data,
+ struct mxt_object *object_table)
{
struct i2c_client *client = data->client;
- size_t table_size;
- struct mxt_object *object_table;
- int error;
int i;
u8 reportid;
u16 end_address;
- table_size = data->info.object_num * sizeof(struct mxt_object);
- object_table = kzalloc(table_size, GFP_KERNEL);
- if (!object_table) {
- dev_err(&data->client->dev, "Failed to allocate memory\n");
- return -ENOMEM;
- }
-
- error = __mxt_read_reg(client, MXT_OBJECT_START, table_size,
- object_table);
- if (error) {
- kfree(object_table);
- return error;
- }
-
/* Valid Report IDs start counting from 1 */
reportid = 1;
data->mem_size = 0;
- for (i = 0; i < data->info.object_num; i++) {
+ for (i = 0; i < data->info->object_num; i++) {
struct mxt_object *object = object_table + i;
u8 min_id, max_id;
@@ -1651,8 +1624,8 @@ static int mxt_get_object_table(struct mxt_data *data)
switch (object->type) {
case MXT_GEN_MESSAGE_T5:
- if (data->info.family_id == 0x80 &&
- data->info.version < 0x20) {
+ if (data->info->family_id == 0x80 &&
+ data->info->version < 0x20) {
/*
* On mXT224 firmware versions prior to V2.0
* read and discard unused CRC byte otherwise
@@ -1707,24 +1680,108 @@ static int mxt_get_object_table(struct mxt_data *data)
/* If T44 exists, T5 position has to be directly after */
if (data->T44_address && (data->T5_address != data->T44_address + 1)) {
dev_err(&client->dev, "Invalid T44 position\n");
- error = -EINVAL;
- goto free_object_table;
+ return -EINVAL;
}
data->msg_buf = kcalloc(data->max_reportid,
data->T5_msg_size, GFP_KERNEL);
if (!data->msg_buf) {
dev_err(&client->dev, "Failed to allocate message buffer\n");
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
+static int mxt_read_info_block(struct mxt_data *data)
+{
+ struct i2c_client *client = data->client;
+ int error;
+ size_t size;
+ void *id_buf, *buf;
+ uint8_t num_objects;
+ u32 calculated_crc;
+ u8 *crc_ptr;
+
+ /* If info block already allocated, free it */
+ if (data->raw_info_block != NULL)
+ mxt_free_object_table(data);
+
+ /* Read 7-byte ID information block starting at address 0 */
+ size = sizeof(struct mxt_info);
+ id_buf = kzalloc(size, GFP_KERNEL);
+ if (!id_buf) {
+ dev_err(&client->dev, "Failed to allocate memory\n");
+ return -ENOMEM;
+ }
+
+ error = __mxt_read_reg(client, 0, size, id_buf);
+ if (error) {
+ kfree(id_buf);
+ return error;
+ }
+
+ /* Resize buffer to give space for rest of info block */
+ num_objects = ((struct mxt_info *)id_buf)->object_num;
+ size += (num_objects * sizeof(struct mxt_object))
+ + MXT_INFO_CHECKSUM_SIZE;
+
+ buf = krealloc(id_buf, size, GFP_KERNEL);
+ if (!buf) {
+ dev_err(&client->dev, "Failed to allocate memory\n");
error = -ENOMEM;
- goto free_object_table;
+ goto err_free_mem;
+ }
+
+ /* Read rest of info block */
+ error = __mxt_read_reg(client, MXT_OBJECT_START,
+ size - MXT_OBJECT_START,
+ buf + MXT_OBJECT_START);
+ if (error)
+ goto err_free_mem;
+
+ /* Extract & calculate checksum */
+ crc_ptr = buf + size - MXT_INFO_CHECKSUM_SIZE;
+ data->info_crc = crc_ptr[0] | (crc_ptr[1] << 8) | (crc_ptr[2] << 16);
+
+ calculated_crc = mxt_calculate_crc(buf, 0,
+ size - MXT_INFO_CHECKSUM_SIZE);
+
+ /*
+ * CRC mismatch can be caused by data corruption due to I2C comms
+ * issue or else device is not using Object Based Protocol (eg i2c-hid)
+ */
+ if ((data->info_crc == 0) || (data->info_crc != calculated_crc)) {
+ dev_err(&client->dev,
+ "Info Block CRC error calculated=0x%06X read=0x%06X\n",
+ calculated_crc, data->info_crc);
+ error = -EIO;
+ goto err_free_mem;
}
- data->object_table = object_table;
+ data->raw_info_block = buf;
+ data->info = (struct mxt_info *)buf;
+
+ dev_info(&client->dev,
+ "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
+ data->info->family_id, data->info->variant_id,
+ data->info->version >> 4, data->info->version & 0xf,
+ data->info->build, data->info->object_num);
+
+ /* Parse object table information */
+ error = mxt_parse_object_table(data, buf + MXT_OBJECT_START);
+ if (error) {
+ dev_err(&client->dev, "Error %d parsing object table\n", error);
+ mxt_free_object_table(data);
+ return error;
+ }
+
+ data->object_table = (struct mxt_object *)(buf + MXT_OBJECT_START);
return 0;
-free_object_table:
- mxt_free_object_table(data);
+err_free_mem:
+ kfree(buf);
return error;
}
@@ -2039,7 +2096,7 @@ static int mxt_initialize(struct mxt_data *data)
int error;
while (1) {
- error = mxt_get_info(data);
+ error = mxt_read_info_block(data);
if (!error)
break;
@@ -2070,13 +2127,6 @@ static int mxt_initialize(struct mxt_data *data)
msleep(MXT_FW_RESET_TIME);
}
- /* Get object table information */
- error = mxt_get_object_table(data);
- if (error) {
- dev_err(&client->dev, "Error %d reading object table\n", error);
- return error;
- }
-
error = mxt_acquire_irq(data);
if (error)
goto err_free_object_table;
@@ -2155,25 +2205,24 @@ static int mxt_init_t7_power_cfg(struct mxt_data *data)
static u16 mxt_get_debug_value(struct mxt_data *data, unsigned int x,
unsigned int y)
{
- struct mxt_info *info = &data->info;
struct mxt_dbg *dbg = &data->dbg;
unsigned int ofs, page;
unsigned int col = 0;
unsigned int col_width;
- if (info->family_id == MXT_FAMILY_1386) {
- col_width = info->matrix_ysize / MXT1386_COLUMNS;
+ if (data->info->family_id == MXT_FAMILY_1386) {
+ col_width = data->info->matrix_ysize / MXT1386_COLUMNS;
col = y / col_width;
y = y % col_width;
} else {
- col_width = info->matrix_ysize;
+ col_width = data->info->matrix_ysize;
}
ofs = (y + (x * col_width)) * sizeof(u16);
page = ofs / MXT_DIAGNOSTIC_SIZE;
ofs %= MXT_DIAGNOSTIC_SIZE;
- if (info->family_id == MXT_FAMILY_1386)
+ if (data->info->family_id == MXT_FAMILY_1386)
page += col * MXT1386_PAGES_PER_COLUMN;
return get_unaligned_le16(&dbg->t37_buf[page].data[ofs]);
@@ -2483,7 +2532,6 @@ static const struct video_device mxt_video_device = {
static void mxt_debug_init(struct mxt_data *data)
{
- struct mxt_info *info = &data->info;
struct mxt_dbg *dbg = &data->dbg;
struct mxt_object *object;
int error;
@@ -2508,11 +2556,11 @@ static void mxt_debug_init(struct mxt_data *data)
/* Calculate size of data and allocate buffer */
dbg->t37_nodes = data->xsize * data->ysize;
- if (info->family_id == MXT_FAMILY_1386)
+ if (data->info->family_id == MXT_FAMILY_1386)
dbg->t37_pages = MXT1386_COLUMNS * MXT1386_PAGES_PER_COLUMN;
else
dbg->t37_pages = DIV_ROUND_UP(data->xsize *
- info->matrix_ysize *
+ data->info->matrix_ysize *
sizeof(u16),
sizeof(dbg->t37_buf->data));
@@ -2569,7 +2617,6 @@ static int mxt_configure_objects(struct mxt_data *data,
const struct firmware *cfg)
{
struct device *dev = &data->client->dev;
- struct mxt_info *info = &data->info;
int error;
error = mxt_init_t7_power_cfg(data);
@@ -2594,11 +2641,6 @@ static int mxt_configure_objects(struct mxt_data *data,
mxt_debug_init(data);
- dev_info(dev,
- "Family: %u Variant: %u Firmware V%u.%u.%02X Objects: %u\n",
- info->family_id, info->variant_id, info->version >> 4,
- info->version & 0xf, info->build, info->object_num);
-
return 0;
}
@@ -2607,9 +2649,9 @@ static ssize_t mxt_fw_version_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct mxt_data *data = dev_get_drvdata(dev);
- struct mxt_info *info = &data->info;
return scnprintf(buf, PAGE_SIZE, "%u.%u.%02X\n",
- info->version >> 4, info->version & 0xf, info->build);
+ data->info->version >> 4, data->info->version & 0xf,
+ data->info->build);
}
/* Hardware Version is returned as FamilyID.VariantID */
@@ -2617,9 +2659,8 @@ static ssize_t mxt_hw_version_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
struct mxt_data *data = dev_get_drvdata(dev);
- struct mxt_info *info = &data->info;
return scnprintf(buf, PAGE_SIZE, "%u.%u\n",
- info->family_id, info->variant_id);
+ data->info->family_id, data->info->variant_id);
}
static ssize_t mxt_show_instance(char *buf, int count,
@@ -2656,7 +2697,7 @@ static ssize_t mxt_object_show(struct device *dev,
return -ENOMEM;
error = 0;
- for (i = 0; i < data->info.object_num; i++) {
+ for (i = 0; i < data->info->object_num; i++) {
object = data->object_table + i;
if (!mxt_object_readable(object->type))
--
2.14.1
^ permalink raw reply related
* [PATCH v7 0/2] hid-steam driver with user mode client dection
From: Rodrigo Rivas Costa @ 2018-03-25 16:07 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
This is a reroll of the Steam Controller driver.
This time the client usage is detected by using exposing a custom hidraw
device (hid_ll_driver) to userland and forwarding that to the real one.
About how the lizard-mode/hidraw-client issue is handled, I have added a
module parameter (I don't know if that is the best option, but it helps me
illustrate different approaches to the problem):
Independently of the lizard_mode parameter value:
1. When a hidraw client is running:
a. I will not send any lizard-mode command to the device, so the client is
in full control.
b. The input device will not send any input message. However it will not
dissappear nor return any error message. Maybe it could return ENODEV if
they try to open the input device when hidraw client is running?
If lizard_mode == 0 ('disabled'):
2.0. When a hidraw client is not running, lizard_mode is disabled.
If lizard_mode == 1, ('auto', the default):
2.1. When a hidraw client is not running:
a. When the input device is not in use, lizard mode is enabled.
b. When the input device is in use, lizard mode is disabled.
If lizard_mode == 2 ('usermode'):
2.2. This driver does not send any lizard_mode-related command. So it is up
to user mode to configure it, with steamctrl or whatever.
Note that when Steam Client opens it always disables lizard-mode (it creates
keyboard/mouse XTest inputs with the same function, though). And when it closed
(but not when it crashes, I think) it always re-enables the lizard mode.
About the input buttons/axes mapping, as per Clément suggestion, I tried to
conform to Documentation/gamepad.rst, but with a few caveats and doubts:
* BTN_NORTH/BTN_WEST are alias of BTN_X/BTN_Y, but those buttons in this
controller have the labels changed (BTN_NORTH is actually Y). I don't know
the best option, so for now I'm mapping 'Y' to BTN_Y and 'X' to BTN_X.
* I'm mapping the lpad clicks to BTN_DPAD_{UP,RIGHT,DOWN,LEFT} but I'm not
sure if that is such a good idea: it cannot do diagonals, for example.
Maybe we could fake the whole dpad from the touch position?
* I'm mapping pressing the joystick to BTN_THUMBL and clicking the rpad to
BTN_THUMBR. Clicking the lpad is unmapped because that is used for the
dpad, depending on where it is clicked.
* Currently I'm mapping the lpad-touch to BTN_THUMB and rpad-touch to
BTN_THUMB2, but I don't know if that is so useful. lpad-touch will overlap
with any use of the dpad. And rpad-touch will overlap with rpad-click...
Changes in v7:
* All the automatic lizard_mode stuff.
* Added the lizard_mode parameter.
* The patchset is reduced to 2 commits. The separation of the
steam_get_serial command no longer makes sense, since I need the
steam_send_cmd in the first commit to implement the lizard mode.
* Change the input mapping to conform to Documentation/gamepad.rst.
(v6 was a RFC, it does not count).
Changes in v5:
* Fix license SPDX to GPL-2.0+.
* Minor stylistic changes (BIT(3) instead 0x08 and so on).
Changes in v4:
* Add command to check the wireless connection status on probe, without
waiting for a message (thanks to Clément Vuchener for the tip).
* Removed the error code on redundant connection/disconnection messages. That
was harmless but polluted dmesg.
* Added buttons for touching the left-pad and right-pad.
* Fixed a misplaced #include from 2/4 to 1/4.
Changes in v3:
* Use RCU to do the dynamic connec/disconnect of wireless devices.
* Remove entries in hid-quirks.c as they are no longer needed. This allows
this module to be blacklisted without side effects.
* Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
existing use cases (lizard mode). A user-space tool to do that is
linked.
* Fully separated axes for joystick and left-pad. As it happens.
* Add fuzz values for left/right pad axes, they are a little wiggly.
Changes in v2:
* Remove references to USB. Now the interesting interfaces are selected by
looking for the ones with feature reports.
* Feature reports buffers are allocated with hid_alloc_report_buf().
* Feature report length is checked, to avoid overflows in case of
corrupt/malicius USB devices.
* Resolution added to the ABS axes.
* A lot of minor cleanups.
Rodrigo Rivas Costa (2):
HID: add driver for Valve Steam Controller
HID: steam: add battery device.
drivers/hid/Kconfig | 8 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 4 +
drivers/hid/hid-steam.c | 978 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
5 files changed, 992 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
--
2.16.2
^ permalink raw reply
* [PATCH v7 1/2] HID: add driver for Valve Steam Controller
From: Rodrigo Rivas Costa @ 2018-03-25 16:07 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180325160703.15997-1-rodrigorivascosta@gmail.com>
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 hidraw and a
creates a uinput virtual gamepad and XTest keyboard/mouse.
This driver intercepts the hidraw usage, so it can get out of the way
when the Steam Client is in use.
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-steam.c | 840 ++++++++++++++++++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
5 files changed, 854 insertions(+)
create mode 100644 drivers/hid/hid-steam.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 779c5ae47f36..de5f4849bfe4 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -811,6 +811,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 235bd2a7b333..e146c257285a 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -94,6 +94,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 a0baa5ba5b84..3014991e5d4b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -987,6 +987,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-steam.c b/drivers/hid/hid-steam.c
new file mode 100644
index 000000000000..3504d2e2d0e5
--- /dev/null
+++ b/drivers/hid/hid-steam.c
@@ -0,0 +1,840 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * HID driver for Valve Steam Controller
+ *
+ * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
+ *
+ * Supports both the wired and wireless interfaces.
+ *
+ * This controller has a builtin emulation of mouse and keyboard: the right pad
+ * can be used as a mouse, the shoulder buttons are mouse buttons, A and B
+ * buttons are ENTER and ESCAPE, and so on. This is implemented as additional
+ * HID interfaces.
+ *
+ * This is known as the "lizard mode", because apparently lizards like to use
+ * the computer from the coach, without a proper mouse and keyboard.
+ *
+ * This driver will disable the lizard mode when the input device is opened
+ * and re-enable it when the input device is closed, so as not to break user
+ * mode behaviour. The lizard_mode parameter can be used to change that.
+ *
+ * There are a few user space applications (notably Steam Client) that use
+ * the hidraw interface directly to create input devices (XTest, uinput...).
+ * In order to avoid breaking them this driver creates a layered hidraw device,
+ * so it can detect when the client is running and then:
+ * - it will not send any command to the controller.
+ * - this input device will be disabled, to avoid double input of the same
+ * user action.
+ *
+ * For additional functions, such as changing the right-pad margin or switching
+ * the led, you can use the user-space tool at:
+ *
+ * https://github.com/rodrigorc/steamctrl
+ */
+
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/hid.h>
+#include <linux/module.h>
+#include <linux/workqueue.h>
+#include <linux/mutex.h>
+#include <linux/rcupdate.h>
+#include <linux/delay.h>
+#include <linux/power_supply.h>
+#include "hid-ids.h"
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
+
+static int lizard_mode = 1;
+module_param(lizard_mode, int, 0644);
+MODULE_PARM_DESC(lizard_mode,
+ "Mouse and keyboard emulation (0 = always disabled; "
+ "1 (default): enabled when gamepad is not in use; "
+ "2: let userspace decide)");
+
+#define STEAM_QUIRK_WIRELESS BIT(0)
+
+#define STEAM_SERIAL_LEN 10
+/* Touch pads are 40 mm in diameter and 65535 units */
+#define STEAM_PAD_RESOLUTION 1638
+/* Trigger runs are about 5 mm and 256 units */
+#define STEAM_TRIGGER_RESOLUTION 51
+/* Joystick runs are about 5 mm and 256 units */
+#define STEAM_JOYSTICK_RESOLUTION 51
+
+#define STEAM_PAD_FUZZ 256
+
+struct steam_device {
+ spinlock_t lock;
+ struct hid_device *hdev, *client_hdev;
+ struct mutex mutex;
+ bool client_opened, input_opened;
+ struct input_dev __rcu *input;
+ unsigned long quirks;
+ struct work_struct work_connect;
+ bool connected;
+ char serial_no[STEAM_SERIAL_LEN + 1];
+};
+
+static int steam_recv_report(struct steam_device *steam,
+ u8 *data, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ /*
+ * The report ID is always 0, so strip the first byte from the output.
+ * hid_report_len() is not counting the report ID, so +1 to the length
+ * or else we get a EOVERFLOW. We are safe from a buffer overflow
+ * because hid_alloc_report_buf() allocates +7 bytes.
+ */
+ ret = hid_hw_raw_request(steam->hdev, 0x00,
+ buf, hid_report_len(r) + 1,
+ HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
+ if (ret > 0)
+ memcpy(data, buf + 1, min(size, ret - 1));
+ kfree(buf);
+ return ret;
+}
+
+static int steam_send_report(struct steam_device *steam,
+ u8 *cmd, int size)
+{
+ struct hid_report *r;
+ u8 *buf;
+ unsigned int retries = 10;
+ int ret;
+
+ r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
+ if (hid_report_len(r) < 64)
+ return -EINVAL;
+
+ buf = hid_alloc_report_buf(r, 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.
+ */
+ do {
+ ret = hid_hw_raw_request(steam->hdev, 0,
+ buf, size + 1,
+ HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+ if (ret != -EPIPE)
+ break;
+ steam_recv_report(steam, NULL, 0);
+ msleep(50);
+ } while (--retries);
+
+ kfree(buf);
+ if (ret < 0)
+ hid_err(steam->hdev, "%s: error %d (%*ph)\n", __func__,
+ ret, size, cmd);
+ return ret;
+}
+
+static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
+{
+ return steam_send_report(steam, &cmd, 1);
+}
+
+static inline int steam_write_register(struct steam_device *steam,
+ u8 reg, u16 value)
+{
+ u8 cmd[] = {0x87, 0x03, reg, value & 0xFF, value >> 8};
+
+ return steam_send_report(steam, cmd, sizeof(cmd));
+}
+
+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[3 + STEAM_SERIAL_LEN + 1];
+
+ 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[3 + STEAM_SERIAL_LEN] = 0;
+ strlcpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
+ return 0;
+}
+
+/*
+ * This command requests the wireless adaptor to post an event
+ * with the connection status. Useful if this driver is loaded when
+ * the controller is already connected.
+ */
+static inline int steam_request_conn_status(struct steam_device *steam)
+{
+ return steam_send_report_byte(steam, 0xb4);
+}
+
+static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
+{
+ if (enable) {
+ /* enable esc, enter, cursors */
+ steam_send_report_byte(steam, 0x85);
+ /* enable mouse */
+ steam_send_report_byte(steam, 0x8e);
+ } else {
+ /* disable esc, enter, cursor */
+ steam_send_report_byte(steam, 0x81);
+ /* disable mouse */
+ steam_write_register(steam, 0x08, 0x07);
+ }
+}
+
+static int steam_input_open(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+ int ret;
+
+ ret = hid_hw_open(steam->hdev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&steam->mutex);
+ steam->input_opened = true;
+ if (!steam->client_opened && lizard_mode == 1)
+ steam_set_lizard_mode(steam, false);
+ mutex_unlock(&steam->mutex);
+ return 0;
+}
+
+static void steam_input_close(struct input_dev *dev)
+{
+ struct steam_device *steam = input_get_drvdata(dev);
+
+ hid_hw_close(steam->hdev);
+
+ mutex_lock(&steam->mutex);
+ steam->input_opened = false;
+ if (!steam->client_opened && lizard_mode == 1)
+ steam_set_lizard_mode(steam, true);
+ mutex_unlock(&steam->mutex);
+}
+
+static int steam_register(struct steam_device *steam)
+{
+ struct hid_device *hdev = steam->hdev;
+ struct input_dev *input;
+ int ret;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+ if (input) {
+ dbg_hid("%s: already connected\n", __func__);
+ return 0;
+ }
+
+ ret = steam_get_serial(steam);
+ if (ret)
+ return ret;
+
+ hid_info(hdev, "Steam Controller '%s' connected",
+ steam->serial_no);
+
+ 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->quirks & STEAM_QUIRK_WIRELESS) ?
+ "Wireless Steam Controller" :
+ "Steam Controller";
+ input->phys = hdev->phys;
+ input->uniq = steam->serial_no;
+ 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_DPAD_UP);
+ input_set_capability(input, EV_KEY, BTN_DPAD_RIGHT);
+ input_set_capability(input, EV_KEY, BTN_DPAD_LEFT);
+ input_set_capability(input, EV_KEY, BTN_DPAD_DOWN);
+ 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_capability(input, EV_KEY, BTN_THUMB);
+ input_set_capability(input, EV_KEY, BTN_THUMB2);
+
+ input_set_abs_params(input, ABS_HAT2Y, 0, 255, 0, 0);
+ input_set_abs_params(input, ABS_HAT2X, 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,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_RY, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT0X, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_set_abs_params(input, ABS_HAT0Y, -32767, 32767,
+ STEAM_PAD_FUZZ, 0);
+ input_abs_set_res(input, ABS_X, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_Y, STEAM_JOYSTICK_RESOLUTION);
+ input_abs_set_res(input, ABS_RX, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_RY, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT2Y, STEAM_TRIGGER_RESOLUTION);
+ input_abs_set_res(input, ABS_HAT2X, STEAM_TRIGGER_RESOLUTION);
+
+ ret = input_register_device(input);
+ if (ret)
+ goto input_register_fail;
+
+ rcu_assign_pointer(steam->input, input);
+
+ return 0;
+
+input_register_fail:
+ input_free_device(input);
+ return ret;
+}
+
+static void steam_unregister(struct steam_device *steam)
+{
+ struct input_dev *input;
+
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ rcu_read_unlock();
+
+ if (input) {
+ RCU_INIT_POINTER(steam->input, NULL);
+ synchronize_rcu();
+ hid_info(steam->hdev, "Steam Controller '%s' disconnected",
+ steam->serial_no);
+ input_unregister_device(input);
+ }
+}
+
+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;
+
+ spin_lock_irqsave(&steam->lock, flags);
+ connected = steam->connected;
+ spin_unlock_irqrestore(&steam->lock, flags);
+
+ if (connected) {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(steam->hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ }
+ } else {
+ steam_unregister(steam);
+ }
+}
+
+static bool steam_is_valve_interface(struct hid_device *hdev)
+{
+ struct hid_report_enum *rep_enum;
+
+ /*
+ * 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.
+ * We know which one is the real gamepad interface because they are the
+ * only ones with a feature report.
+ */
+ rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
+ return !list_empty(&rep_enum->report_list);
+}
+
+static int steam_client_ll_parse(struct hid_device *hdev)
+{
+ return 0;
+}
+
+static int steam_client_ll_start(struct hid_device *hdev)
+{
+ return 0;
+}
+
+static void steam_client_ll_stop(struct hid_device *hdev)
+{
+}
+
+static int steam_client_ll_open(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+ int ret;
+
+ ret = hid_hw_open(steam->hdev);
+ if (ret)
+ return ret;
+
+ mutex_lock(&steam->mutex);
+ steam->client_opened = true;
+ mutex_unlock(&steam->mutex);
+ return ret;
+}
+
+static void steam_client_ll_close(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ hid_hw_close(steam->hdev);
+
+ mutex_lock(&steam->mutex);
+ steam->client_opened = false;
+ if (lizard_mode != 2) {
+ if (steam->input_opened)
+ steam_set_lizard_mode(steam, false);
+ else
+ steam_set_lizard_mode(steam, lizard_mode);
+ }
+ mutex_unlock(&steam->mutex);
+}
+
+static int steam_client_ll_raw_request(struct hid_device *hdev,
+ unsigned char reportnum, u8 *buf,
+ size_t count, unsigned char report_type,
+ int reqtype)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ return hid_hw_raw_request(steam->hdev, reportnum, buf, count,
+ report_type, reqtype);
+}
+
+static struct hid_ll_driver steam_client_ll_driver = {
+ .parse = steam_client_ll_parse,
+ .start = steam_client_ll_start,
+ .stop = steam_client_ll_stop,
+ .open = steam_client_ll_open,
+ .close = steam_client_ll_close,
+ .raw_request = steam_client_ll_raw_request,
+};
+
+static int steam_probe(struct hid_device *hdev,
+ const struct hid_device_id *id)
+{
+ struct steam_device *steam;
+ struct hid_device *client_hdev;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev,
+ "%s:parse of hid interface failed\n", __func__);
+ return ret;
+ }
+
+ /*
+ * The non-valve interfaces (mouse and keyboard emulation) and
+ * the client_hid are connected without changes.
+ */
+ if (hdev->group == HID_GROUP_STEAM ||
+ !steam_is_valve_interface(hdev)) {
+ return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ }
+
+ /*
+ * With the real steam controller interface, do not connect hidraw.
+ * Instead, create the client_hid and connect that.
+ */
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
+ if (ret)
+ return ret;
+
+ client_hdev = hid_allocate_device();
+ if (IS_ERR(client_hdev)) {
+ ret = PTR_ERR(client_hdev);
+ goto client_hdev_fail;
+ }
+
+ client_hdev->ll_driver = &steam_client_ll_driver;
+ client_hdev->dev.parent = hdev->dev.parent;
+ client_hdev->bus = hdev->bus;
+ client_hdev->vendor = hdev->vendor;
+ client_hdev->product = hdev->product;
+ strlcpy(client_hdev->name, hdev->name,
+ sizeof(client_hdev->name));
+ strlcpy(client_hdev->phys, hdev->phys,
+ sizeof(client_hdev->phys));
+ client_hdev->dev_rdesc = kmemdup(hdev->dev_rdesc,
+ hdev->dev_rsize, GFP_KERNEL);
+ client_hdev->dev_rsize = hdev->dev_rsize;
+ /*
+ * Since we use the same device info than the real interface to
+ * trick userspace, we will be calling steam_probe recursively.
+ * We need to recognize the client interface somehow.
+ */
+ client_hdev->group = HID_GROUP_STEAM;
+
+ ret = hid_add_device(client_hdev);
+ if (ret)
+ goto client_hdev_add_fail;
+
+ steam = devm_kzalloc(&hdev->dev, sizeof(*steam), GFP_KERNEL);
+ if (!steam) {
+ ret = -ENOMEM;
+ goto steam_alloc_fail;
+ }
+
+ steam->client_hdev = client_hdev;
+ hid_set_drvdata(client_hdev, steam);
+
+ spin_lock_init(&steam->lock);
+ mutex_init(&steam->mutex);
+ steam->hdev = hdev;
+ hid_set_drvdata(hdev, steam);
+ steam->quirks = id->driver_data;
+ INIT_WORK(&steam->work_connect, steam_work_connect_cb);
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ 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");
+ steam_request_conn_status(steam);
+ } else {
+ ret = steam_register(steam);
+ if (ret) {
+ hid_err(hdev,
+ "%s:steam_register failed with error %d\n",
+ __func__, ret);
+ goto input_register_fail;
+ }
+ }
+ if (lizard_mode != 2)
+ steam_set_lizard_mode(steam, lizard_mode);
+
+ return 0;
+
+input_register_fail:
+hid_hw_open_fail:
+ cancel_work_sync(&steam->work_connect);
+ hid_set_drvdata(hdev, NULL);
+steam_alloc_fail:
+ hid_hw_stop(client_hdev);
+client_hdev_add_fail:
+ hid_destroy_device(client_hdev);
+client_hdev_fail:
+ hid_err(hdev, "%s: failed with error %d\n",
+ __func__, ret);
+ return 0;
+}
+
+static void steam_remove(struct hid_device *hdev)
+{
+ struct steam_device *steam = hid_get_drvdata(hdev);
+
+ if (hdev->group == HID_GROUP_STEAM)
+ return;
+
+ if (!steam) {
+ hid_hw_stop(hdev);
+ return;
+ }
+
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ hid_info(hdev, "Steam wireless receiver disconnected");
+ hid_hw_close(hdev);
+ }
+ hid_hw_stop(hdev);
+ cancel_work_sync(&steam->work_connect);
+ hid_hw_stop(steam->client_hdev);
+ hid_destroy_device(steam->client_hdev);
+ steam_unregister(steam);
+}
+
+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_HAT2Y | left trigger
+ * 12 | u8 | ABS_HAT2X | right trigger
+ * 13-15 | -- | -- | always 0
+ * 16-17 | s16 | ABS_X/ABS_HAT0X | X value
+ * 18-19 | s16 | ABS_Y/ABS_HAT0Y | 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 | BTN_DPAD_UP | lef-pad up
+ * 9.1 | BTN_DPAD_RIGHT | lef-pad right
+ * 9.2 | BTN_DPAD_LEFT | lef-pad left
+ * 9.3 | BTN_DPAD_DOWN | 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 | BTN_THUMB | left-pad touched (but see explanation below)
+ * 10.4 | BTN_THUMB2 | 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,
+ struct input_dev *input, u8 *data)
+{
+ /* 24 bits of buttons */
+ u8 b8, b9, b10;
+ bool lpad_touched, lpad_and_joy;
+
+ b8 = data[8];
+ b9 = data[9];
+ b10 = data[10];
+
+ input_report_abs(input, ABS_HAT2Y, data[11]);
+ input_report_abs(input, ABS_HAT2X, data[12]);
+
+ /*
+ * These two bits tells how to interpret the values X and Y.
+ * lpad_and_joy tells that the joystick and the lpad are used at the
+ * same time.
+ * lpad_touched tells whether X/Y are to be read as lpad coord or
+ * joystick values.
+ * (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
+ */
+ lpad_touched = b10 & BIT(3);
+ lpad_and_joy = b10 & BIT(7);
+ input_report_abs(input, lpad_touched ? ABS_HAT0X : ABS_X,
+ (s16) le16_to_cpup((__le16 *)(data + 16)));
+ input_report_abs(input, lpad_touched ? ABS_HAT0Y : ABS_Y,
+ -(s16) le16_to_cpup((__le16 *)(data + 18)));
+ /* Check if joystick is centered */
+ if (lpad_touched && !lpad_and_joy) {
+ input_report_abs(input, ABS_X, 0);
+ input_report_abs(input, ABS_Y, 0);
+ }
+ /* Check if lpad is untouched */
+ if (!(lpad_touched || lpad_and_joy)) {
+ input_report_abs(input, ABS_HAT0X, 0);
+ input_report_abs(input, ABS_HAT0Y, 0);
+ }
+
+ 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)));
+
+ input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
+ input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
+ input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
+ input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
+ input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
+ input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
+ input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
+ input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
+ input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
+ input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
+ input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
+ input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
+ input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
+ input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
+ input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
+ input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & BIT(7)));
+ input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & BIT(0)));
+ input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
+ input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
+ input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
+ input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
+
+ input_sync(input);
+}
+
+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);
+ struct input_dev *input;
+
+ if (!steam)
+ return 0;
+
+ if (steam->client_opened)
+ hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
+ data, size, 0);
+ /*
+ * 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:
+ if (steam->client_opened)
+ return 0;
+ rcu_read_lock();
+ input = rcu_dereference(steam->input);
+ if (likely(input)) {
+ steam_do_input_event(steam, input, data);
+ } else {
+ dbg_hid("%s: input data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ 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 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);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index d491027a7c22..5e5d76589954 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -364,6 +364,7 @@ struct hid_item {
#define HID_GROUP_RMI 0x0100
#define HID_GROUP_WACOM 0x0101
#define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102
+#define HID_GROUP_STEAM 0x0103
/*
* HID protocol status
--
2.16.2
^ permalink raw reply related
* [PATCH v7 2/2] HID: steam: add battery device.
From: Rodrigo Rivas Costa @ 2018-03-25 16:07 UTC (permalink / raw)
To: Benjamin Tissoires, Pierre-Loup A. Griffais,
Clément VUCHENER, Jiri Kosina, Cameron Gutman, lkml,
linux-input
Cc: Rodrigo Rivas Costa
In-Reply-To: <20180325160703.15997-1-rodrigorivascosta@gmail.com>
The wireless Steam Controller is battery operated, so add the battery
device and power information.
---
drivers/hid/hid-steam.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 139 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c
index 3504d2e2d0e5..28dd5ebe2b9a 100644
--- a/drivers/hid/hid-steam.c
+++ b/drivers/hid/hid-steam.c
@@ -75,6 +75,10 @@ struct steam_device {
struct work_struct work_connect;
bool connected;
char serial_no[STEAM_SERIAL_LEN + 1];
+ struct power_supply_desc battery_desc;
+ struct power_supply __rcu *battery;
+ u8 battery_charge;
+ u16 voltage;
};
static int steam_recv_report(struct steam_device *steam,
@@ -238,6 +242,85 @@ static void steam_input_close(struct input_dev *dev)
mutex_unlock(&steam->mutex);
}
+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;
+
+ 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 = devm_kasprintf(&steam->hdev->dev,
+ GFP_KERNEL, "steam-controller-%s-battery",
+ steam->serial_no);
+ if (!steam->battery_desc.name)
+ return -ENOMEM;
+
+ /* 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->hdev->dev,
+ &steam->battery_desc, &battery_cfg);
+ if (IS_ERR(battery)) {
+ ret = PTR_ERR(battery);
+ hid_err(steam->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ rcu_assign_pointer(steam->battery, battery);
+ power_supply_powers(battery, &steam->hdev->dev);
+ return 0;
+}
+
static int steam_register(struct steam_device *steam)
{
struct hid_device *hdev = steam->hdev;
@@ -327,6 +410,10 @@ static int steam_register(struct steam_device *steam)
rcu_assign_pointer(steam->input, input);
+ /* ignore battery errors, we can live without it */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS)
+ steam_battery_register(steam);
+
return 0;
input_register_fail:
@@ -337,11 +424,18 @@ static int steam_register(struct steam_device *steam)
static void steam_unregister(struct steam_device *steam)
{
struct input_dev *input;
+ struct power_supply *battery;
rcu_read_lock();
input = rcu_dereference(steam->input);
+ battery = rcu_dereference(steam->battery);
rcu_read_unlock();
+ if (battery) {
+ RCU_INIT_POINTER(steam->battery, NULL);
+ synchronize_rcu();
+ power_supply_unregister(battery);
+ }
if (input) {
RCU_INIT_POINTER(steam->input, NULL);
synchronize_rcu();
@@ -745,12 +839,44 @@ static void steam_do_input_event(struct steam_device *steam,
input_sync(input);
}
+/*
+ * 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,
+ struct power_supply *battery, u8 *data)
+{
+ unsigned long flags;
+
+ s16 volts = le16_to_cpup((__le16 *)(data + 12));
+ u8 batt = data[14];
+
+ /* Creating the battery may have failed */
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ spin_lock_irqsave(&steam->lock, flags);
+ steam->voltage = volts;
+ steam->battery_charge = batt;
+ spin_unlock_irqrestore(&steam->lock, flags);
+ power_supply_changed(battery);
+ }
+ rcu_read_unlock();
+}
+
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);
struct input_dev *input;
+ struct power_supply *battery;
if (!steam)
return 0;
@@ -808,7 +934,19 @@ static int steam_raw_event(struct hid_device *hdev,
}
break;
case 0x04:
- /* TODO: battery status */
+ if (steam->quirks & STEAM_QUIRK_WIRELESS) {
+ rcu_read_lock();
+ battery = rcu_dereference(steam->battery);
+ if (likely(battery)) {
+ steam_do_battery_event(steam, battery, data);
+ } else {
+ dbg_hid(
+ "%s: battery data without connect event\n",
+ __func__);
+ steam_do_connect_event(steam, true);
+ }
+ rcu_read_unlock();
+ }
break;
}
return 0;
--
2.16.2
^ permalink raw reply related
* Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
From: Pali Rohár @ 2018-03-25 21:25 UTC (permalink / raw)
To: Masaki Ota
Cc: Dmitry Torokhov, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, Aaron Ma
In-Reply-To: <CAKdAkRRjMrpdjuaivh-5bKCCgYpcySqDCKzbuvso1_x_+Qkp1w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3503 bytes --]
On Tuesday 20 March 2018 11:47:26 Dmitry Torokhov wrote:
> On Mon, Jan 29, 2018 at 2:51 PM, dmitry.torokhov@gmail.com
> <dmitry.torokhov@gmail.com> wrote:
> > Hi,
> >
> > On Thu, Nov 16, 2017 at 07:27:02AM +0000, Masaki Ota wrote:
> >> Hi, Pali, Aaron,
> >>
> >> Current code is correct device setting, previous code is wrong.
> >> If the trackstick does not work(DUALPOINT flag disable), Device Firmware setting is wrong.
> >>
> >> But recently I received the same report from Thinkpad L570 user, and I checked this device and found this device Firmware setting is wrong. Sorry for our mistake.
> >> Is your laptop L570 ?
> >>
> >> I will add code that supports the trackstick for this device.
> >
> > Sorry for resurrecting this old thread, I am just trying to understand
> > what went wrong here. Is the sequence of "f0 f0 e9" and "ea ea e9" is
> > important in getting the correct OTP data and we originally got this
> > order wrong? It is not clear from the original patch and discussion that
> > this change was intentional.
>
> Could I please get an answer to my question?
>
> Thanks!
Masaki, this question is for you ↑↑↑
> >
> > Thanks.
> >
> >>
> >> Best Regards,
> >> Masaki Ota
> >> -----Original Message-----
> >> From: Pali Rohár [mailto:pali.rohar@gmail.com]
> >> Sent: Wednesday, November 15, 2017 5:35 PM
> >> To: 太田 真喜 Masaki Ota <masaki.ota@jp.alps.com>
> >> Cc: linux-input@vger.kernel.org; linux-kernel@vger.kernel.org; dmitry.torokhov@gmail.com; Aaron Ma <aaron.ma@canonical.com>
> >> Subject: Re: [PATCH] Input: ALPS - fix DualPoint flag for 74 03 28 devices
> >>
> >> On Wednesday 15 November 2017 14:34:04 Aaron Ma wrote:
> >> > There is a regression of commit 4a646580f793 ("Input: ALPS - fix
> >> > two-finger scroll breakage"), ALPS device fails with log:
> >> >
> >> > psmouse serio1: alps: Rejected trackstick packet from non DualPoint
> >> > device
> >> >
> >> > ALPS device with id "74 03 28" report OTP[0] data 0xCE after commit
> >> > 4a646580f793, after restore the OTP reading order, it becomes to 0x10
> >> > as before and reports the right flag.
> >> >
> >> > Fixes: 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage")
> >> > Cc: <stable@vger.kernel.org>
> >> > Signed-off-by: Aaron Ma <aaron.ma@canonical.com>
> >> > ---
> >> > drivers/input/mouse/alps.c | 4 ++--
> >> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >> >
> >> > diff --git a/drivers/input/mouse/alps.c b/drivers/input/mouse/alps.c
> >> > index 579b899add26..c59b8f7ca2fc 100644
> >> > --- a/drivers/input/mouse/alps.c
> >> > +++ b/drivers/input/mouse/alps.c
> >> > @@ -2562,8 +2562,8 @@ static int alps_set_defaults_ss4_v2(struct
> >> > psmouse *psmouse,
> >> >
> >> > memset(otp, 0, sizeof(otp));
> >> >
> >> > - if (alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]) ||
> >> > - alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]))
> >> > + if (alps_get_otp_values_ss4_v2(psmouse, 0, &otp[0][0]) ||
> >> > + alps_get_otp_values_ss4_v2(psmouse, 1, &otp[1][0]))
> >> > return -1;
> >> >
> >> > alps_update_device_area_ss4_v2(otp, priv);
> >>
> >> Masaki Ota, please look at this patch as it partially revert your commit
> >> 4a646580f793 ("Input: ALPS - fix two-finger scroll breakage"). Something smells here.
> >>
> >> --
> >> Pali Rohár
> >> pali.rohar@gmail.com
> >
> > --
> > Dmitry
>
>
>
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCH v7 0/2] hid-steam driver with user mode client dection
From: Benjamin Tissoires @ 2018-03-26 8:12 UTC (permalink / raw)
To: Rodrigo Rivas Costa
Cc: Pierre-Loup A. Griffais, Clément VUCHENER, Jiri Kosina,
Cameron Gutman, lkml, linux-input
In-Reply-To: <20180325160703.15997-1-rodrigorivascosta@gmail.com>
Hi Rodrigo,
On Sun, Mar 25, 2018 at 6:07 PM, Rodrigo Rivas Costa
<rodrigorivascosta@gmail.com> wrote:
> This is a reroll of the Steam Controller driver.
>
> This time the client usage is detected by using exposing a custom hidraw
> device (hid_ll_driver) to userland and forwarding that to the real one.
That is actually more clever than what I had in mind.
This way you reliably know if the hidraw node has been opened without
touching hid-core.c or hidraw.c, well played :)
>
> About how the lizard-mode/hidraw-client issue is handled, I have added a
> module parameter (I don't know if that is the best option, but it helps me
> illustrate different approaches to the problem):
>
> Independently of the lizard_mode parameter value:
I find the values of the lizard_mode parameter hard to understand. And
honestly, I do not think there is much a difference between 0 and 1.
AFAICT, the difference between these two values is that in the first
case you are disabling the lizard mode whether or not the joystick is
in used, and in the latter, you disable it only when the joystick is
in use. Does that gives any benefits to users?
I would think having a boolean "let the kernel handle the lizard mode
with some magic inside" would be simpler to understand. You do not
really want to support too many configurations and I think it would be
wiser to say either disable the new mode or just enable it.
Also, I think there will be races if a user changes the value of the
parameter while running the system. You might want to add an
additional patch that would trigger the mode change on module
parameter change.
As far as I can tell, I am happy with such hack. There are a few
points I'd like to raise in the patch 1, but I'll inline my comments
there.
Cheers,
Benjamin
>
> 1. When a hidraw client is running:
> a. I will not send any lizard-mode command to the device, so the client is
> in full control.
> b. The input device will not send any input message. However it will not
> dissappear nor return any error message. Maybe it could return ENODEV if
> they try to open the input device when hidraw client is running?
>
> If lizard_mode == 0 ('disabled'):
> 2.0. When a hidraw client is not running, lizard_mode is disabled.
>
> If lizard_mode == 1, ('auto', the default):
> 2.1. When a hidraw client is not running:
> a. When the input device is not in use, lizard mode is enabled.
> b. When the input device is in use, lizard mode is disabled.
>
> If lizard_mode == 2 ('usermode'):
> 2.2. This driver does not send any lizard_mode-related command. So it is up
> to user mode to configure it, with steamctrl or whatever.
>
> Note that when Steam Client opens it always disables lizard-mode (it creates
> keyboard/mouse XTest inputs with the same function, though). And when it closed
> (but not when it crashes, I think) it always re-enables the lizard mode.
>
> About the input buttons/axes mapping, as per Clément suggestion, I tried to
> conform to Documentation/gamepad.rst, but with a few caveats and doubts:
> * BTN_NORTH/BTN_WEST are alias of BTN_X/BTN_Y, but those buttons in this
> controller have the labels changed (BTN_NORTH is actually Y). I don't know
> the best option, so for now I'm mapping 'Y' to BTN_Y and 'X' to BTN_X.
> * I'm mapping the lpad clicks to BTN_DPAD_{UP,RIGHT,DOWN,LEFT} but I'm not
> sure if that is such a good idea: it cannot do diagonals, for example.
> Maybe we could fake the whole dpad from the touch position?
> * I'm mapping pressing the joystick to BTN_THUMBL and clicking the rpad to
> BTN_THUMBR. Clicking the lpad is unmapped because that is used for the
> dpad, depending on where it is clicked.
> * Currently I'm mapping the lpad-touch to BTN_THUMB and rpad-touch to
> BTN_THUMB2, but I don't know if that is so useful. lpad-touch will overlap
> with any use of the dpad. And rpad-touch will overlap with rpad-click...
>
> Changes in v7:
> * All the automatic lizard_mode stuff.
> * Added the lizard_mode parameter.
> * The patchset is reduced to 2 commits. The separation of the
> steam_get_serial command no longer makes sense, since I need the
> steam_send_cmd in the first commit to implement the lizard mode.
> * Change the input mapping to conform to Documentation/gamepad.rst.
>
> (v6 was a RFC, it does not count).
>
> Changes in v5:
> * Fix license SPDX to GPL-2.0+.
> * Minor stylistic changes (BIT(3) instead 0x08 and so on).
>
> Changes in v4:
> * Add command to check the wireless connection status on probe, without
> waiting for a message (thanks to Clément Vuchener for the tip).
> * Removed the error code on redundant connection/disconnection messages. That
> was harmless but polluted dmesg.
> * Added buttons for touching the left-pad and right-pad.
> * Fixed a misplaced #include from 2/4 to 1/4.
>
> Changes in v3:
> * Use RCU to do the dynamic connec/disconnect of wireless devices.
> * Remove entries in hid-quirks.c as they are no longer needed. This allows
> this module to be blacklisted without side effects.
> * Do not bypass the virtual keyboard/mouse HID devices to avoid breaking
> existing use cases (lizard mode). A user-space tool to do that is
> linked.
> * Fully separated axes for joystick and left-pad. As it happens.
> * Add fuzz values for left/right pad axes, they are a little wiggly.
>
> Changes in v2:
> * Remove references to USB. Now the interesting interfaces are selected by
> looking for the ones with feature reports.
> * Feature reports buffers are allocated with hid_alloc_report_buf().
> * Feature report length is checked, to avoid overflows in case of
> corrupt/malicius USB devices.
> * Resolution added to the ABS axes.
> * A lot of minor cleanups.
>
> Rodrigo Rivas Costa (2):
> HID: add driver for Valve Steam Controller
> HID: steam: add battery device.
>
> drivers/hid/Kconfig | 8 +
> drivers/hid/Makefile | 1 +
> drivers/hid/hid-ids.h | 4 +
> drivers/hid/hid-steam.c | 978 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/hid.h | 1 +
> 5 files changed, 992 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
> --
> 2.16.2
>
^ permalink raw reply
* Re: [PATCH v7 1/2] HID: add driver for Valve Steam Controller
From: Benjamin Tissoires @ 2018-03-26 9:20 UTC (permalink / raw)
To: Rodrigo Rivas Costa
Cc: Pierre-Loup A. Griffais, Clément VUCHENER, Jiri Kosina,
Cameron Gutman, lkml, linux-input
In-Reply-To: <20180325160703.15997-2-rodrigorivascosta@gmail.com>
Hi Rodrigo,
few comments inlined.
On Sun, Mar 25, 2018 at 6:07 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 hidraw and a
> creates a uinput virtual gamepad and XTest keyboard/mouse.
>
> This driver intercepts the hidraw usage, so it can get out of the way
> when the Steam Client is in use.
>
> 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-steam.c | 840 ++++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/hid.h | 1 +
> 5 files changed, 854 insertions(+)
> create mode 100644 drivers/hid/hid-steam.c
>
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 779c5ae47f36..de5f4849bfe4 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -811,6 +811,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 235bd2a7b333..e146c257285a 100644
> --- a/drivers/hid/Makefile
> +++ b/drivers/hid/Makefile
> @@ -94,6 +94,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 a0baa5ba5b84..3014991e5d4b 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -987,6 +987,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-steam.c b/drivers/hid/hid-steam.c
> new file mode 100644
> index 000000000000..3504d2e2d0e5
> --- /dev/null
> +++ b/drivers/hid/hid-steam.c
> @@ -0,0 +1,840 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * HID driver for Valve Steam Controller
> + *
> + * Copyright (c) 2018 Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>
> + *
> + * Supports both the wired and wireless interfaces.
> + *
> + * This controller has a builtin emulation of mouse and keyboard: the right pad
> + * can be used as a mouse, the shoulder buttons are mouse buttons, A and B
> + * buttons are ENTER and ESCAPE, and so on. This is implemented as additional
> + * HID interfaces.
> + *
> + * This is known as the "lizard mode", because apparently lizards like to use
> + * the computer from the coach, without a proper mouse and keyboard.
> + *
> + * This driver will disable the lizard mode when the input device is opened
> + * and re-enable it when the input device is closed, so as not to break user
> + * mode behaviour. The lizard_mode parameter can be used to change that.
> + *
> + * There are a few user space applications (notably Steam Client) that use
> + * the hidraw interface directly to create input devices (XTest, uinput...).
> + * In order to avoid breaking them this driver creates a layered hidraw device,
> + * so it can detect when the client is running and then:
> + * - it will not send any command to the controller.
> + * - this input device will be disabled, to avoid double input of the same
> + * user action.
> + *
> + * For additional functions, such as changing the right-pad margin or switching
> + * the led, you can use the user-space tool at:
> + *
> + * https://github.com/rodrigorc/steamctrl
> + */
> +
> +#include <linux/device.h>
> +#include <linux/input.h>
> +#include <linux/hid.h>
> +#include <linux/module.h>
> +#include <linux/workqueue.h>
> +#include <linux/mutex.h>
> +#include <linux/rcupdate.h>
> +#include <linux/delay.h>
> +#include <linux/power_supply.h>
> +#include "hid-ids.h"
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Rodrigo Rivas Costa <rodrigorivascosta@gmail.com>");
> +
> +static int lizard_mode = 1;
> +module_param(lizard_mode, int, 0644);
> +MODULE_PARM_DESC(lizard_mode,
> + "Mouse and keyboard emulation (0 = always disabled; "
> + "1 (default): enabled when gamepad is not in use; "
> + "2: let userspace decide)");
As mentioned in 0/2, I think a boolean might be better. Probably
rename the parameter to something else more explicit too (like
'control_lizard_mode').
Also you might want to hook up to changes to this values so users can
control it better. But this can be added in a later patch.
> +
> +#define STEAM_QUIRK_WIRELESS BIT(0)
> +
> +#define STEAM_SERIAL_LEN 10
> +/* Touch pads are 40 mm in diameter and 65535 units */
> +#define STEAM_PAD_RESOLUTION 1638
> +/* Trigger runs are about 5 mm and 256 units */
> +#define STEAM_TRIGGER_RESOLUTION 51
> +/* Joystick runs are about 5 mm and 256 units */
> +#define STEAM_JOYSTICK_RESOLUTION 51
> +
> +#define STEAM_PAD_FUZZ 256
> +
> +struct steam_device {
> + spinlock_t lock;
> + struct hid_device *hdev, *client_hdev;
> + struct mutex mutex;
> + bool client_opened, input_opened;
> + struct input_dev __rcu *input;
> + unsigned long quirks;
> + struct work_struct work_connect;
> + bool connected;
> + char serial_no[STEAM_SERIAL_LEN + 1];
> +};
> +
> +static int steam_recv_report(struct steam_device *steam,
> + u8 *data, int size)
> +{
> + struct hid_report *r;
> + u8 *buf;
> + int ret;
> +
> + r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
> + if (hid_report_len(r) < 64)
> + return -EINVAL;
> +
> + buf = hid_alloc_report_buf(r, GFP_KERNEL);
> + if (!buf)
> + return -ENOMEM;
> +
> + /*
> + * The report ID is always 0, so strip the first byte from the output.
> + * hid_report_len() is not counting the report ID, so +1 to the length
> + * or else we get a EOVERFLOW. We are safe from a buffer overflow
> + * because hid_alloc_report_buf() allocates +7 bytes.
> + */
> + ret = hid_hw_raw_request(steam->hdev, 0x00,
> + buf, hid_report_len(r) + 1,
> + HID_FEATURE_REPORT, HID_REQ_GET_REPORT);
> + if (ret > 0)
> + memcpy(data, buf + 1, min(size, ret - 1));
> + kfree(buf);
> + return ret;
> +}
> +
> +static int steam_send_report(struct steam_device *steam,
> + u8 *cmd, int size)
> +{
> + struct hid_report *r;
> + u8 *buf;
> + unsigned int retries = 10;
> + int ret;
> +
> + r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0];
> + if (hid_report_len(r) < 64)
> + return -EINVAL;
> +
> + buf = hid_alloc_report_buf(r, 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.
> + */
> + do {
> + ret = hid_hw_raw_request(steam->hdev, 0,
> + buf, size + 1,
> + HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
> + if (ret != -EPIPE)
> + break;
> + steam_recv_report(steam, NULL, 0);
> + msleep(50);
> + } while (--retries);
> +
> + kfree(buf);
> + if (ret < 0)
> + hid_err(steam->hdev, "%s: error %d (%*ph)\n", __func__,
> + ret, size, cmd);
> + return ret;
> +}
> +
> +static inline int steam_send_report_byte(struct steam_device *steam, u8 cmd)
> +{
> + return steam_send_report(steam, &cmd, 1);
> +}
> +
> +static inline int steam_write_register(struct steam_device *steam,
> + u8 reg, u16 value)
> +{
> + u8 cmd[] = {0x87, 0x03, reg, value & 0xFF, value >> 8};
> +
> + return steam_send_report(steam, cmd, sizeof(cmd));
> +}
> +
> +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[3 + STEAM_SERIAL_LEN + 1];
> +
> + 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[3 + STEAM_SERIAL_LEN] = 0;
> + strlcpy(steam->serial_no, reply + 3, sizeof(steam->serial_no));
> + return 0;
> +}
> +
> +/*
> + * This command requests the wireless adaptor to post an event
> + * with the connection status. Useful if this driver is loaded when
> + * the controller is already connected.
> + */
> +static inline int steam_request_conn_status(struct steam_device *steam)
> +{
> + return steam_send_report_byte(steam, 0xb4);
> +}
> +
> +static void steam_set_lizard_mode(struct steam_device *steam, bool enable)
> +{
> + if (enable) {
> + /* enable esc, enter, cursors */
> + steam_send_report_byte(steam, 0x85);
> + /* enable mouse */
> + steam_send_report_byte(steam, 0x8e);
> + } else {
> + /* disable esc, enter, cursor */
> + steam_send_report_byte(steam, 0x81);
> + /* disable mouse */
> + steam_write_register(steam, 0x08, 0x07);
> + }
> +}
> +
> +static int steam_input_open(struct input_dev *dev)
> +{
> + struct steam_device *steam = input_get_drvdata(dev);
> + int ret;
> +
> + ret = hid_hw_open(steam->hdev);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&steam->mutex);
> + steam->input_opened = true;
> + if (!steam->client_opened && lizard_mode == 1)
> + steam_set_lizard_mode(steam, false);
> + mutex_unlock(&steam->mutex);
> + return 0;
> +}
> +
> +static void steam_input_close(struct input_dev *dev)
> +{
> + struct steam_device *steam = input_get_drvdata(dev);
> +
> + hid_hw_close(steam->hdev);
> +
> + mutex_lock(&steam->mutex);
> + steam->input_opened = false;
> + if (!steam->client_opened && lizard_mode == 1)
> + steam_set_lizard_mode(steam, true);
> + mutex_unlock(&steam->mutex);
hid_hw_close() should be called after setting the lizard mode.
> +}
> +
> +static int steam_register(struct steam_device *steam)
> +{
> + struct hid_device *hdev = steam->hdev;
> + struct input_dev *input;
> + int ret;
> +
> + rcu_read_lock();
> + input = rcu_dereference(steam->input);
> + rcu_read_unlock();
> + if (input) {
> + dbg_hid("%s: already connected\n", __func__);
> + return 0;
> + }
> +
> + ret = steam_get_serial(steam);
> + if (ret)
> + return ret;
> +
> + hid_info(hdev, "Steam Controller '%s' connected",
> + steam->serial_no);
> +
> + 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->quirks & STEAM_QUIRK_WIRELESS) ?
> + "Wireless Steam Controller" :
> + "Steam Controller";
> + input->phys = hdev->phys;
> + input->uniq = steam->serial_no;
> + 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_DPAD_UP);
> + input_set_capability(input, EV_KEY, BTN_DPAD_RIGHT);
> + input_set_capability(input, EV_KEY, BTN_DPAD_LEFT);
> + input_set_capability(input, EV_KEY, BTN_DPAD_DOWN);
> + 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_capability(input, EV_KEY, BTN_THUMB);
> + input_set_capability(input, EV_KEY, BTN_THUMB2);
> +
> + input_set_abs_params(input, ABS_HAT2Y, 0, 255, 0, 0);
> + input_set_abs_params(input, ABS_HAT2X, 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,
> + STEAM_PAD_FUZZ, 0);
> + input_set_abs_params(input, ABS_RY, -32767, 32767,
> + STEAM_PAD_FUZZ, 0);
> + input_set_abs_params(input, ABS_HAT0X, -32767, 32767,
> + STEAM_PAD_FUZZ, 0);
> + input_set_abs_params(input, ABS_HAT0Y, -32767, 32767,
> + STEAM_PAD_FUZZ, 0);
> + input_abs_set_res(input, ABS_X, STEAM_JOYSTICK_RESOLUTION);
> + input_abs_set_res(input, ABS_Y, STEAM_JOYSTICK_RESOLUTION);
> + input_abs_set_res(input, ABS_RX, STEAM_PAD_RESOLUTION);
> + input_abs_set_res(input, ABS_RY, STEAM_PAD_RESOLUTION);
> + input_abs_set_res(input, ABS_HAT0X, STEAM_PAD_RESOLUTION);
> + input_abs_set_res(input, ABS_HAT0Y, STEAM_PAD_RESOLUTION);
> + input_abs_set_res(input, ABS_HAT2Y, STEAM_TRIGGER_RESOLUTION);
> + input_abs_set_res(input, ABS_HAT2X, STEAM_TRIGGER_RESOLUTION);
> +
> + ret = input_register_device(input);
> + if (ret)
> + goto input_register_fail;
> +
> + rcu_assign_pointer(steam->input, input);
> +
> + return 0;
> +
> +input_register_fail:
> + input_free_device(input);
> + return ret;
> +}
> +
> +static void steam_unregister(struct steam_device *steam)
> +{
> + struct input_dev *input;
> +
> + rcu_read_lock();
> + input = rcu_dereference(steam->input);
> + rcu_read_unlock();
> +
> + if (input) {
> + RCU_INIT_POINTER(steam->input, NULL);
> + synchronize_rcu();
> + hid_info(steam->hdev, "Steam Controller '%s' disconnected",
> + steam->serial_no);
> + input_unregister_device(input);
> + }
> +}
> +
> +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;
> +
> + spin_lock_irqsave(&steam->lock, flags);
> + connected = steam->connected;
> + spin_unlock_irqrestore(&steam->lock, flags);
> +
> + if (connected) {
> + ret = steam_register(steam);
> + if (ret) {
> + hid_err(steam->hdev,
> + "%s:steam_register failed with error %d\n",
> + __func__, ret);
> + }
> + } else {
> + steam_unregister(steam);
> + }
> +}
> +
> +static bool steam_is_valve_interface(struct hid_device *hdev)
> +{
> + struct hid_report_enum *rep_enum;
> +
> + /*
> + * 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.
> + * We know which one is the real gamepad interface because they are the
> + * only ones with a feature report.
> + */
> + rep_enum = &hdev->report_enum[HID_FEATURE_REPORT];
> + return !list_empty(&rep_enum->report_list);
> +}
> +
> +static int steam_client_ll_parse(struct hid_device *hdev)
> +{
> + return 0;
Instead of returning 0 here, you should probably call
hid_parse_report() on the report descriptors from the parent node.
Some clients might want to have a look at them or might even rely on
them.
> +}
> +
> +static int steam_client_ll_start(struct hid_device *hdev)
> +{
> + return 0;
> +}
> +
> +static void steam_client_ll_stop(struct hid_device *hdev)
> +{
> +}
> +
> +static int steam_client_ll_open(struct hid_device *hdev)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
You probably want to check if steam is not null here. Given the
ordering of the initialization, you might have someone attempting to
open the hidraw node before steam is created.
> + int ret;
> +
> + ret = hid_hw_open(steam->hdev);
> + if (ret)
> + return ret;
> +
> + mutex_lock(&steam->mutex);
> + steam->client_opened = true;
> + mutex_unlock(&steam->mutex);
> + return ret;
> +}
> +
> +static void steam_client_ll_close(struct hid_device *hdev)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
Same here, you might want to check on the validity of steam.
> +
> + hid_hw_close(steam->hdev);
> +
> + mutex_lock(&steam->mutex);
> + steam->client_opened = false;
> + if (lizard_mode != 2) {
> + if (steam->input_opened)
> + steam_set_lizard_mode(steam, false);
> + else
> + steam_set_lizard_mode(steam, lizard_mode);
> + }
> + mutex_unlock(&steam->mutex);
You should call hid_hw_close(steam->hdev); after sending the commands
and not before.
> +}
> +
> +static int steam_client_ll_raw_request(struct hid_device *hdev,
> + unsigned char reportnum, u8 *buf,
> + size_t count, unsigned char report_type,
> + int reqtype)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
> +
> + return hid_hw_raw_request(steam->hdev, reportnum, buf, count,
> + report_type, reqtype);
> +}
I wish we could reuse directly the pointer in
hdev->ll_driver->raw_request to avoid adding an indirection.
OTOH, the raw_requests are not happening that often, so we should be good.
> +
> +static struct hid_ll_driver steam_client_ll_driver = {
> + .parse = steam_client_ll_parse,
> + .start = steam_client_ll_start,
> + .stop = steam_client_ll_stop,
> + .open = steam_client_ll_open,
> + .close = steam_client_ll_close,
> + .raw_request = steam_client_ll_raw_request,
> +};
> +
> +static int steam_probe(struct hid_device *hdev,
> + const struct hid_device_id *id)
> +{
> + struct steam_device *steam;
> + struct hid_device *client_hdev;
> + int ret;
> +
> + ret = hid_parse(hdev);
> + if (ret) {
> + hid_err(hdev,
> + "%s:parse of hid interface failed\n", __func__);
> + return ret;
> + }
> +
> + /*
> + * The non-valve interfaces (mouse and keyboard emulation) and
> + * the client_hid are connected without changes.
> + */
> + if (hdev->group == HID_GROUP_STEAM ||
> + !steam_is_valve_interface(hdev)) {
> + return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
> + }
To make thinks clearer, I would split these calls in 2:
- if (!steam_is_valve_interface(hdev)) return hid_hw_start(hdev,
HID_CONNECT_DEFAULT);
and
- if (hdev->group == HID_GROUP_STEAM) return hid_hw_start(hdev,
HID_CONNECT_HIDRAW);
Explicitly having HID_CONNECT_HIDRAW would also make it clearer you
are just exporting the hidraw interface here. It'll prevent any other
interface to mess your detection of the hidraw usage.
> +
> + /*
> + * With the real steam controller interface, do not connect hidraw.
> + * Instead, create the client_hid and connect that.
> + */
> + ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT & ~HID_CONNECT_HIDRAW);
> + if (ret)
> + return ret;
this should likely be at the end of the probe, when you are done
allocating your data.
> +
> + client_hdev = hid_allocate_device();
> + if (IS_ERR(client_hdev)) {
> + ret = PTR_ERR(client_hdev);
> + goto client_hdev_fail;
> + }
> +
> + client_hdev->ll_driver = &steam_client_ll_driver;
> + client_hdev->dev.parent = hdev->dev.parent;
> + client_hdev->bus = hdev->bus;
> + client_hdev->vendor = hdev->vendor;
> + client_hdev->product = hdev->product;
> + strlcpy(client_hdev->name, hdev->name,
> + sizeof(client_hdev->name));
> + strlcpy(client_hdev->phys, hdev->phys,
> + sizeof(client_hdev->phys));
> + client_hdev->dev_rdesc = kmemdup(hdev->dev_rdesc,
> + hdev->dev_rsize, GFP_KERNEL);
> + client_hdev->dev_rsize = hdev->dev_rsize;
> + /*
> + * Since we use the same device info than the real interface to
> + * trick userspace, we will be calling steam_probe recursively.
> + * We need to recognize the client interface somehow.
> + */
> + client_hdev->group = HID_GROUP_STEAM;
I'd extract out the client_hdev initialization in its own function to
keep .probe() clean.
> +
> + ret = hid_add_device(client_hdev);
> + if (ret)
> + goto client_hdev_add_fail;
This should likely be called after initializing steam. It'll keep the
cleanup path cleaner and make sure all fields are properly initialized
before they are used.
> +
> + steam = devm_kzalloc(&hdev->dev, sizeof(*steam), GFP_KERNEL);
> + if (!steam) {
> + ret = -ENOMEM;
> + goto steam_alloc_fail;
> + }
> +
> + steam->client_hdev = client_hdev;
> + hid_set_drvdata(client_hdev, steam);
> +
> + spin_lock_init(&steam->lock);
> + mutex_init(&steam->mutex);
> + steam->hdev = hdev;
> + hid_set_drvdata(hdev, steam);
> + steam->quirks = id->driver_data;
> + INIT_WORK(&steam->work_connect, steam_work_connect_cb);
I'd call hid_hw_start(hdev, ...) here, and then hid_add_device(client_hdev);
To have a better cleanup path, you porbably should allocate
client_hdev here too (before hid_add_device, of course)
> +
> + if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> + 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");
> + steam_request_conn_status(steam);
> + } else {
> + ret = steam_register(steam);
> + if (ret) {
> + hid_err(hdev,
> + "%s:steam_register failed with error %d\n",
> + __func__, ret);
> + goto input_register_fail;
> + }
> + }
> + if (lizard_mode != 2)
> + steam_set_lizard_mode(steam, lizard_mode);
> +
> + return 0;
> +
> +input_register_fail:
> +hid_hw_open_fail:
> + cancel_work_sync(&steam->work_connect);
> + hid_set_drvdata(hdev, NULL);
> +steam_alloc_fail:
> + hid_hw_stop(client_hdev);
> +client_hdev_add_fail:
> + hid_destroy_device(client_hdev);
> +client_hdev_fail:
> + hid_err(hdev, "%s: failed with error %d\n",
> + __func__, ret);
> + return 0;
> +}
> +
> +static void steam_remove(struct hid_device *hdev)
> +{
> + struct steam_device *steam = hid_get_drvdata(hdev);
> +
> + if (hdev->group == HID_GROUP_STEAM)
Why don't you call hid_hw_stop() here instead of calling it later in
the parent device?
> + return;
> +
> + if (!steam) {
> + hid_hw_stop(hdev);
> + return;
> + }
> +
You should reorder these cleanup calls:
- you first call hid_destroy_device(), this will clean up properly
everything related to the hidraw node and should set client_opened to
false (just to be on the safe side, you might want to overwrite it
after to be sure not to forward events to the destoryed hid node).
- then you take care of the rest of the hid device:
- cancel_work_sync should happen before calling hid_hw_close() and
hid_hw_stop() on the hdev.
- steam_unregister(steam);
- if needed call hid_hw_close()
- hid_hw_stop()
> + if (steam->quirks & STEAM_QUIRK_WIRELESS) {
> + hid_info(hdev, "Steam wireless receiver disconnected");
> + hid_hw_close(hdev);
> + }
> + hid_hw_stop(hdev);
> + cancel_work_sync(&steam->work_connect);
> + hid_hw_stop(steam->client_hdev);
> + hid_destroy_device(steam->client_hdev);
> +
> +}
> +
> +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_HAT2Y | left trigger
> + * 12 | u8 | ABS_HAT2X | right trigger
> + * 13-15 | -- | -- | always 0
> + * 16-17 | s16 | ABS_X/ABS_HAT0X | X value
> + * 18-19 | s16 | ABS_Y/ABS_HAT0Y | 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 | BTN_DPAD_UP | lef-pad up
> + * 9.1 | BTN_DPAD_RIGHT | lef-pad right
> + * 9.2 | BTN_DPAD_LEFT | lef-pad left
> + * 9.3 | BTN_DPAD_DOWN | 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 | BTN_THUMB | left-pad touched (but see explanation below)
> + * 10.4 | BTN_THUMB2 | 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,
> + struct input_dev *input, u8 *data)
> +{
> + /* 24 bits of buttons */
> + u8 b8, b9, b10;
> + bool lpad_touched, lpad_and_joy;
> +
> + b8 = data[8];
> + b9 = data[9];
> + b10 = data[10];
> +
> + input_report_abs(input, ABS_HAT2Y, data[11]);
> + input_report_abs(input, ABS_HAT2X, data[12]);
> +
> + /*
> + * These two bits tells how to interpret the values X and Y.
> + * lpad_and_joy tells that the joystick and the lpad are used at the
> + * same time.
> + * lpad_touched tells whether X/Y are to be read as lpad coord or
> + * joystick values.
> + * (lpad_touched || lpad_and_joy) tells if the lpad is really touched.
> + */
> + lpad_touched = b10 & BIT(3);
> + lpad_and_joy = b10 & BIT(7);
> + input_report_abs(input, lpad_touched ? ABS_HAT0X : ABS_X,
> + (s16) le16_to_cpup((__le16 *)(data + 16)));
> + input_report_abs(input, lpad_touched ? ABS_HAT0Y : ABS_Y,
> + -(s16) le16_to_cpup((__le16 *)(data + 18)));
> + /* Check if joystick is centered */
> + if (lpad_touched && !lpad_and_joy) {
> + input_report_abs(input, ABS_X, 0);
> + input_report_abs(input, ABS_Y, 0);
> + }
> + /* Check if lpad is untouched */
> + if (!(lpad_touched || lpad_and_joy)) {
> + input_report_abs(input, ABS_HAT0X, 0);
> + input_report_abs(input, ABS_HAT0Y, 0);
> + }
> +
> + 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)));
> +
> + input_event(input, EV_KEY, BTN_TR2, !!(b8 & BIT(0)));
> + input_event(input, EV_KEY, BTN_TL2, !!(b8 & BIT(1)));
> + input_event(input, EV_KEY, BTN_TR, !!(b8 & BIT(2)));
> + input_event(input, EV_KEY, BTN_TL, !!(b8 & BIT(3)));
> + input_event(input, EV_KEY, BTN_Y, !!(b8 & BIT(4)));
> + input_event(input, EV_KEY, BTN_B, !!(b8 & BIT(5)));
> + input_event(input, EV_KEY, BTN_X, !!(b8 & BIT(6)));
> + input_event(input, EV_KEY, BTN_A, !!(b8 & BIT(7)));
> + input_event(input, EV_KEY, BTN_DPAD_UP, !!(b9 & BIT(0)));
> + input_event(input, EV_KEY, BTN_DPAD_RIGHT, !!(b9 & BIT(1)));
> + input_event(input, EV_KEY, BTN_DPAD_LEFT, !!(b9 & BIT(2)));
> + input_event(input, EV_KEY, BTN_DPAD_DOWN, !!(b9 & BIT(3)));
> + input_event(input, EV_KEY, BTN_SELECT, !!(b9 & BIT(4)));
> + input_event(input, EV_KEY, BTN_MODE, !!(b9 & BIT(5)));
> + input_event(input, EV_KEY, BTN_START, !!(b9 & BIT(6)));
> + input_event(input, EV_KEY, BTN_GEAR_DOWN, !!(b9 & BIT(7)));
> + input_event(input, EV_KEY, BTN_GEAR_UP, !!(b10 & BIT(0)));
> + input_event(input, EV_KEY, BTN_THUMBR, !!(b10 & BIT(2)));
> + input_event(input, EV_KEY, BTN_THUMBL, !!(b10 & BIT(6)));
> + input_event(input, EV_KEY, BTN_THUMB, lpad_touched || lpad_and_joy);
> + input_event(input, EV_KEY, BTN_THUMB2, !!(b10 & BIT(4)));
> +
> + input_sync(input);
> +}
> +
> +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);
> + struct input_dev *input;
> +
> + if (!steam)
> + return 0;
> +
> + if (steam->client_opened)
> + hid_input_report(steam->client_hdev, HID_FEATURE_REPORT,
> + data, size, 0);
> + /*
> + * 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:
> + if (steam->client_opened)
> + return 0;
> + rcu_read_lock();
> + input = rcu_dereference(steam->input);
> + if (likely(input)) {
> + steam_do_input_event(steam, input, data);
> + } else {
> + dbg_hid("%s: input data without connect event\n",
> + __func__);
> + steam_do_connect_event(steam, true);
> + }
> + rcu_read_unlock();
> + 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 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);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index d491027a7c22..5e5d76589954 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -364,6 +364,7 @@ struct hid_item {
> #define HID_GROUP_RMI 0x0100
> #define HID_GROUP_WACOM 0x0101
> #define HID_GROUP_LOGITECH_DJ_DEVICE 0x0102
> +#define HID_GROUP_STEAM 0x0103
>
> /*
> * HID protocol status
> --
> 2.16.2
>
Cheers,
Benjamin
^ permalink raw reply
* Re: [PATCH] Input: alps - Update documentation for trackstick v3 format
From: Jonathan Corbet @ 2018-03-26 14:41 UTC (permalink / raw)
To: Pali Rohár
Cc: Dmitry Torokhov, Masaki Ota, linux-input, linux-doc, linux-kernel
In-Reply-To: <20180321200336.32035-1-pali.rohar@gmail.com>
On Wed, 21 Mar 2018 21:03:36 +0100
Pali Rohár <pali.rohar@gmail.com> wrote:
> Bits for M, R and L buttons are already processed in alps. Other newly
> documented bits not yet.
>
> Signed-off-by: Pali Rohár <pali.rohar@gmail.com>
> ---
> This is based on information which Masaki Ota provided to us.
In the absence of screams of agony I'll assume that the change is
correct. Applied to the docs tree, thanks.
jon
^ permalink raw reply
* Re: [PATCH] Input: add bu21029 touch driver
From: Rob Herring @ 2018-03-26 22:24 UTC (permalink / raw)
To: Mark Jonas
Cc: Dmitry Torokhov, Mark Rutland, linux-input, devicetree,
linux-kernel, hs, Zhu Yi
In-Reply-To: <1521651874-15379-1-git-send-email-mark.jonas@de.bosch.com>
On Wed, Mar 21, 2018 at 06:04:34PM +0100, Mark Jonas wrote:
> From: Zhu Yi <yi.zhu5@cn.bosch.com>
>
> Add the ROHM BU21029 resistive touch panel controller
> support with i2c interface.
>
> Signed-off-by: Zhu Yi <yi.zhu5@cn.bosch.com>
> Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com>
> Reviewed-by: Heiko Schocher <hs@denx.de>
> ---
> .../bindings/input/touchscreen/bu21029.txt | 30 ++
> drivers/input/touchscreen/Kconfig | 12 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/bu21029_ts.c | 456 +++++++++++++++++++++
> 4 files changed, 499 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> create mode 100644 drivers/input/touchscreen/bu21029_ts.c
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> new file mode 100644
> index 0000000..7b61602
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/touchscreen/bu21029.txt
> @@ -0,0 +1,30 @@
> +* Rohm BU21029 Touch Screen Controller
> +
> +Required properties:
> + - compatible : must be "rohm,bu21029"
> + - reg : i2c device address of the chip
What the valid value(s)?
> + - interrupt-parent : the phandle for the gpio controller
> + - interrupts : (gpio) interrupt to which the chip is connected
> + - reset-gpios : gpio pin to reset the chip
Active high or low?
> + - rohm,x-plate-ohms : x-plate resistance in ohms
IIRC, we have a standard touchscreen property for this?
> +
> +Optional properties:
> + - touchscreen-max-pressure: maximum pressure value
> +
> +Example:
> +
> + &i2c1 {
> + /* ... */
> +
> + bu21029: bu21029@40 {
> + compatible = "rohm,bu21029";
> + reg = <0x40>;
> + interrupt-parent = <&gpio1>;
> + interrupts = <4 IRQ_TYPE_EDGE_FALLING>;
> + reset-gpios = <&gpio6 16 GPIO_ACTIVE_LOW>;
> + rohm,x-plate-ohms = <600>;
> + touchscreen-max-pressure = <4095>;
> + };
> +
> + /* ... */
> + };
^ permalink raw reply
* Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
From: Rob Herring @ 2018-03-26 22:24 UTC (permalink / raw)
To: Tirupathi Reddy
Cc: dmitry.torokhov, bjorn.andersson, mark.rutland, linux-input,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <1521786192-19487-1-git-send-email-tirupath@codeaurora.org>
On Fri, Mar 23, 2018 at 11:53:12AM +0530, Tirupathi Reddy wrote:
> Add resin key support to handle different types of key events
> defined in different platforms.
>
> Signed-off-by: Tirupathi Reddy <tirupath@codeaurora.org>
> ---
> .../bindings/input/qcom,pm8941-pwrkey.txt | 32 +++++++++
Reviewed-by: Rob Herring <robh@kernel.org>
> drivers/input/misc/pm8941-pwrkey.c | 81 ++++++++++++++++++++++
> 2 files changed, 113 insertions(+)
^ 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