* Re: [regression] Bug 216946 - Toshiba satellite click mini l9w-b: touchscreen: no touch events with kernel 6.1.4
From: Linux regression tracking (Thorsten Leemhuis) @ 2023-04-17 11:55 UTC (permalink / raw)
To: Hans de Goede, Benjamin Tissoires, Linux regressions mailing list
Cc: Jiri Kosina, Dmitry Torokhov, open list:HID CORE LAYER, LKML,
Gé Koerkamp
In-Reply-To: <13c5e60d-d9ba-f840-f43e-40db957a6617@redhat.com>
On 01.03.23 12:41, Hans de Goede wrote:
> On 2/28/23 14:26, Benjamin Tissoires wrote:
>> On Tue, Feb 28, 2023 at 12:32 PM Thorsten Leemhuis
>> <regressions@leemhuis.info> wrote:
>>>
>>> On 19.01.23 16:06, Linux kernel regression tracking (Thorsten Leemhuis)
>>> wrote:
>>>> Hi, this is your Linux kernel regression tracker.
>>>>
>>>> I noticed a regression report in bugzilla.kernel.org. As many (most?)
>>>> kernel developer don't keep an eye on it, I decided to forward it by
>>>> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=216946 :
>>>
>>> The reporter recently confirmed in the ticket that the issue still
>>> happens with 6.2.
>>>
>>> There wasn't any reply from any of the input developers here or in
>>> bugzilla afaics. :-/ Hmmm. Could someone from that camp maybe please
>>> take a minute and at least briefly look into this as answer something
>>> like "that might be due to a problem in subsystem 'foo'", "maybe ask bar
>>> for an option", or "we have no idea what might cause this, this needs to
>>> be bisected"? That would help a lot.
>>
>> The working dmesg shows a line with:
>> hid-generic 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00 Device
>> [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>> and then
>> hid-multitouch 0018:0457:10FB.0002: input,hidraw1: I2C HID v1.00
>> Device [SIS0817:00 0457:10FB] on i2c-SIS0817:00
>>
>> But these 2 lines do not appear on the 6.1.4 logs.
>>
>> So the device is not properly enumerated by ACPI or I2C. Hans might
>> have an idea on how to debug/solve that issue.
>
> I actually have a Toshiba satellite click mini l9w-b lying around
> myself. I already made a note to try and reproduce this
>
> But I'm very much swamped with too much kernel work, so no promises
> when I will get around to this ...
Has anyone made any progress with this? Doesn't look like it, hence I
wondered if this fall through the cracks; but maybe I'm just missing
something.
Gé Koerkamp: with a bit of luck Hans sooner or later will have time to
look into this, but it might speed things up if you could try to bisect
this.
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
If I did something stupid, please tell me, as explained on that page.
#regzbot poke
>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>> --
>>> Everything you wanna know about Linux kernel regression tracking:
>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>> If I did something stupid, please tell me, as explained on that page.
>>>
>>> #regzbot poke
>>>>> Gé Koerkamp 2023-01-17 20:21:51 UTC
>>>>>
>>>>> Created attachment 303619 [details]
>>>>> Kernel configuration for v6.1.4/ journalctl (dmesg)/ ACPIdump/lsmod
>>>>>
>>>>> Overview:
>>>>> The touchscreen does not react on touch events.
>>>>> Touchscreen display and touchpad are working.
>>>>>
>>>>> Step to reproduce:
>>>>> Open any UI page
>>>>> Try to use touch on relevant UI controls (buttons etc.)
>>>>>
>>>>> Result:
>>>>> No reaction on screen touches
>>>>>
>>>>> Expected result:
>>>>> Reaction on touched control, same as when using the touch pad or connected mouse (which do work).
>>>>>
>>>>> Build information:
>>>>> The error happens with kernel version 6.1.4
>>>>> After rebuilding with different kernel versions, it appears that it first fails with kernel 5.16
>>>>>
>>>>> Additional builds:
>>>>> The click mini l9w-b still works with kernel 5.10.y LTS and 5.15.y LTS.
>>>>>
>>>>> Important remark:
>>>>> Touchscreen still works fine with kernel 6.1.4 using
>>>>> - an HP x2 detachable 10-p0xx or
>>>>> - a Lenovo yoga 520-14ikb
>>>>>
>>>>> So it could be a hardware dependent issue
>>>>
>>>> See the ticket for more details.
>>>>
>>>>
>>>> [TLDR for the rest of this mail: I'm adding this report to the list of
>>>> tracked Linux kernel regressions; the text you find below is based on a
>>>> few templates paragraphs you might have encountered already in similar
>>>> form.]
>>>>
>>>> BTW, let me use this mail to also add the report to the list of tracked
>>>> regressions to ensure it's doesn't fall through the cracks:
>>>>
>>>> #regzbot introduced: v5.15..v5.16
>>>> https://bugzilla.kernel.org/show_bug.cgi?id=216946
>>>> #regzbot title: hid: touchscreen broken with Toshiba satellite click
>>>> mini l9w-b
>>>> #regzbot ignore-activity
>>>>
>>>> This isn't a regression? This issue or a fix for it are already
>>>> discussed somewhere else? It was fixed already? You want to clarify when
>>>> the regression started to happen? Or point out I got the title or
>>>> something else totally wrong? Then just reply and tell me -- ideally
>>>> while also telling regzbot about it, as explained by the page listed in
>>>> the footer of this mail.
>>>>
>>>> Developers: When fixing the issue, remember to add 'Link:' tags pointing
>>>> to the report (e.g. the buzgzilla ticket and maybe this mail as well, if
>>>> this thread sees some discussion). See page linked in footer for details.
>>>>
>>>> Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
>>>> --
>>>> Everything you wanna know about Linux kernel regression tracking:
>>>> https://linux-regtracking.leemhuis.info/about/#tldr
>>>> If I did something stupid, please tell me, as explained on that page.
>>>
>>
>
>
>
^ permalink raw reply
* Re: [regression] Bug 217182 - Dell Latitude E7450 Trackpoint not working as expected
From: Linux regression tracking #update (Thorsten Leemhuis) @ 2023-04-17 11:46 UTC (permalink / raw)
To: Linux kernel regressions list; +Cc: open list:HID CORE LAYER, LKML
In-Reply-To: <6625ee0a-31a7-9fef-d299-457c0f98f5a0@leemhuis.info>
[TLDR: This mail in primarily relevant for Linux regression tracking. A
change or fix related to the regression discussed in this thread was
posted or applied, but it did not use a Link: tag to point to the
report, as Linus and the documentation call for. Things happen, no
worries -- but now the regression tracking bot needs to be told manually
about the fix. See link in footer if these mails annoy you.]
On 13.03.23 11:20, Linux regression tracking (Thorsten Leemhuis) wrote:
>
> Hi, Thorsten here, the Linux kernel's regression tracker.
>
> I noticed a regression report in bugzilla.kernel.org. As many (most?)
> kernel developer don't keep an eye on it, I decided to forward it by
> mail. Quoting from https://bugzilla.kernel.org/show_bug.cgi?id=217182 :
Tell regzbot about the fix for this:
#regzbot fix: 754ff5060daf5a1cf4474eff9b4edeb6c17ef7ab
#regzbot ignore-activity
Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings
From: Rob Herring @ 2023-04-17 13:10 UTC (permalink / raw)
To: Jiri Valek - 2N
Cc: devicetree, dmitry.torokhov, linux-kernel, u.kleine-koenig,
krzysztof.kozlowski+dt, linux-input, robh+dt
In-Reply-To: <20230414233815.4004526-2-jiriv@axis.com>
On Sat, 15 Apr 2023 01:38:14 +0200, Jiri Valek - 2N wrote:
> Add support for advanced sensitivity settings and signal guard feature.
>
> Signed-off-by: Jiri Valek - 2N <jiriv@axis.com>
> ---
> .../bindings/input/microchip,cap11xx.yaml | 64 ++++++++++++++++++-
> 1 file changed, 61 insertions(+), 3 deletions(-)
>
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,signal-guard: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,signal-guard: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'maximum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'minimum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'maximum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,calib-sensitivity: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,calib-sensitivity: 'enum' should not be valid under {'enum': ['const', 'enum', 'exclusiveMaximum', 'exclusiveMinimum', 'minimum', 'maximum', 'multipleOf', 'pattern']}
hint: Scalar and array keywords cannot be mixed
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,signal-guard: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('default', 'enum', 'maxItems', 'minItems' were unexpected)
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('default', 'maxItems', 'minItems' were unexpected)
hint: A vendor string property with exact values has an implicit type
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,signal-guard: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
0 is not of type 'string'
hint: A vendor string property with exact values has an implicit type
1 is not of type 'string'
hint: A vendor string property with exact values has an implicit type
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('default', 'maxItems', 'maximum', 'minItems', 'minimum' were unexpected)
hint: A vendor boolean property can use "type: boolean"
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'oneOf' conditional failed, one must be fixed:
'enum' is a required property
'const' is a required property
hint: A vendor string property with exact values has an implicit type
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,input-treshold: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,calib-sensitivity: 'oneOf' conditional failed, one must be fixed:
'type' is a required property
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('default', 'enum', 'maxItems', 'minItems' were unexpected)
hint: A vendor boolean property can use "type: boolean"
Additional properties are not allowed ('default', 'maxItems', 'minItems' were unexpected)
hint: A vendor string property with exact values has an implicit type
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml: properties:microchip,calib-sensitivity: 'oneOf' conditional failed, one must be fixed:
'$ref' is a required property
'allOf' is a required property
hint: A vendor property needs a $ref to types.yaml
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
1 is not of type 'string'
hint: A vendor string property with exact values has an implicit type
2 is not of type 'string'
hint: A vendor string property with exact values has an implicit type
4 is not of type 'string'
hint: A vendor string property with exact values has an implicit type
hint: Vendor specific properties must have a type and description unless they have a defined, common suffix.
from schema $id: http://devicetree.org/meta-schemas/vendor-props.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.example.dtb: touch@28: microchip,signal-guard: [[0, 0, 0, 0, 0, 0]] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.example.dtb: touch@28: microchip,signal-guard:0: [0, 0, 0, 0, 0, 0] is too long
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.example.dtb: touch@28: microchip,input-treshold: [[21, 18, 46, 46, 46, 21]] is too short
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.example.dtb: touch@28: microchip,input-treshold:0: [21, 18, 46, 46, 46, 21] is too long
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.example.dtb: touch@28: 'microchip,calib-sensitivityj' does not match any of the regexes: '^led@[0-7]$', 'pinctrl-[0-9]+'
From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230414233815.4004526-2-jiriv@axis.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()
From: Nikita Zhandarovich @ 2023-04-17 16:01 UTC (permalink / raw)
To: Ping Cheng
Cc: Nikita Zhandarovich, Jason Gerecke, Jiri Kosina,
Benjamin Tissoires, linux-input, linux-kernel, lvc-project
If high bit is set to 1 in ((data[3] & 0x0f << 28), after all arithmetic
operations and integer promotions are done, high bits in
wacom->serial[idx] will be filled with 1s as well.
Avoid this, albeit unlikely, issue by specifying left operand's __u64
type for the right operand.
Found by Linux Verification Center (linuxtesting.org) with static
analysis tool SVACE.
Fixes: 3bea733ab212 ("USB: wacom tablet driver reorganization")
Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
---
drivers/hid/wacom_wac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 9312d611db8e..0e4404f3801e 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -826,7 +826,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
/* Enter report */
if ((data[1] & 0xfc) == 0xc0) {
/* serial number of the tool */
- wacom->serial[idx] = ((data[3] & 0x0f) << 28) +
+ wacom->serial[idx] = ((__u64)(data[3] & 0x0f) << 28) +
(data[4] << 20) + (data[5] << 12) +
(data[6] << 4) + (data[7] >> 4);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH] HID: wacom: avoid integer overflow in wacom_intuos_inout()
From: Ping Cheng @ 2023-04-17 17:13 UTC (permalink / raw)
To: Nikita Zhandarovich
Cc: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
linux-input, linux-kernel, lvc-project
In-Reply-To: <20230417160148.60011-1-n.zhandarovich@fintech.ru>
On Mon, Apr 17, 2023 at 9:03 AM Nikita Zhandarovich
<n.zhandarovich@fintech.ru> wrote:
>
> If high bit is set to 1 in ((data[3] & 0x0f << 28), after all arithmetic
> operations and integer promotions are done, high bits in
> wacom->serial[idx] will be filled with 1s as well.
> Avoid this, albeit unlikely, issue by specifying left operand's __u64
> type for the right operand.
Thank you Nikita for the fix! It's good to avoid potential issues.
> Found by Linux Verification Center (linuxtesting.org) with static
> analysis tool SVACE.
>
> Fixes: 3bea733ab212 ("USB: wacom tablet driver reorganization")
> Signed-off-by: Nikita Zhandarovich <n.zhandarovich@fintech.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Cc: stable@vger.kernel.org
Cheers,
Ping
> ---
> drivers/hid/wacom_wac.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 9312d611db8e..0e4404f3801e 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -826,7 +826,7 @@ static int wacom_intuos_inout(struct wacom_wac *wacom)
> /* Enter report */
> if ((data[1] & 0xfc) == 0xc0) {
> /* serial number of the tool */
> - wacom->serial[idx] = ((data[3] & 0x0f) << 28) +
> + wacom->serial[idx] = ((__u64)(data[3] & 0x0f) << 28) +
> (data[4] << 20) + (data[5] << 12) +
> (data[6] << 4) + (data[7] >> 4);
>
> --
> 2.25.1
>
^ permalink raw reply
* [PATCH v3 0/1] HID: shield
From: Rahul Rameshbabu @ 2023-04-17 18:29 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Rahul Rameshbabu
Hi.
This is an initial driver implementation for supporting NVIDIA SHIELD
peripherals. Currently supports the following functionality for the
THUNDERSTRIKE (SHIELD 2017) controller.
- Haptics (ff_memless)
- Serial number query (sysfs)
- Hardware information query (sysfs)
- Firmware version query (sysfs)
Changes:
v2 -> v3:
* Renamed drivers/hid/hid-shield.c -> drivers/hid/hid-nvidia-shield.c.
* Added space between braces for terminating element in shield_devices
array.
v1 -> v2:
* Declared thunderstrike_hostcmd_req_work_handler as static.
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202304011342.6Bh3cWhA-lkp@intel.com/
Link: https://lore.kernel.org/oe-kbuild-all/202304020922.vsngJnBT-lkp@intel.com/
Rahul Rameshbabu (1):
HID: shield: Initial driver implementation with Thunderstrike support
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-nvidia-shield.c | 587 ++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-nvidia-shield.c
Link: https://lore.kernel.org/linux-input/20230410170840.16119-1-rrameshbabu@nvidia.com/
Link: https://lore.kernel.org/linux-input/20230401032150.7424-1-rrameshbabu@nvidia.com/
--
2.38.4
^ permalink raw reply
* [PATCH v3 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-17 18:29 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, Rahul Rameshbabu
In-Reply-To: <20230417182909.33833-1-rrameshbabu@nvidia.com>
Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
SHIELD device and introduces haptics support for controllers.
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-nvidia-shield.c | 587 ++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-nvidia-shield.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2b073facf399..2390b6427e07 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9122,6 +9122,12 @@ F: drivers/hid/hid-sensor-*
F: drivers/iio/*/hid-*
F: include/linux/hid-sensor-*
+HID SHIELD DRIVER
+M: Rahul Rameshbabu <rrameshbabu@nvidia.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-nvidia-shield.c
+
HID VRC-2 CAR CONTROLLER DRIVER
M: Marcus Folkesson <marcus.folkesson@gmail.com>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 82f64fb31fda..eb19debaa1f5 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -990,6 +990,24 @@ config HID_SEMITEK
- Woo-dy
- X-Bows Nature/Knight
+config HID_SHIELD
+ tristate "SHIELD HID Driver"
+ depends on USB_HID
+ depends on BT_HIDP
+ help
+ Support for NVIDIA SHIELD accessories.
+
+ Supported devices:
+ - Thunderstrike (NVIDIA SHIELD Controller 2017)
+
+config SHIELD_FF
+ bool "SHIELD force feedback support"
+ depends on HID_SHIELD
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here if you would like to enable force feedback support for
+ NVIDIA SHIELD accessories with haptics capabilities.
+
config HID_SIGMAMICRO
tristate "SiGma Micro-based keyboards"
depends on USB_HID
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..a37487b470c8 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -116,6 +116,7 @@ obj-$(CONFIG_HID_RMI) += hid-rmi.o
obj-$(CONFIG_HID_SAITEK) += hid-saitek.o
obj-$(CONFIG_HID_SAMSUNG) += hid-samsung.o
obj-$(CONFIG_HID_SEMITEK) += hid-semitek.o
+obj-$(CONFIG_HID_SHIELD) += hid-nvidia-shield.o
obj-$(CONFIG_HID_SIGMAMICRO) += hid-sigmamicro.o
obj-$(CONFIG_HID_SMARTJOYPLUS) += hid-sjoy.o
obj-$(CONFIG_HID_SONY) += hid-sony.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 63545cd307e5..4000d53b1bac 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -993,6 +993,9 @@
#define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18 0x0014
#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
+#define USB_VENDOR_ID_NVIDIA 0x0955
+#define USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER 0x7214
+
#define USB_VENDOR_ID_ONTRAK 0x0a07
#define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
new file mode 100644
index 000000000000..32173cf7f8c9
--- /dev/null
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -0,0 +1,587 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * HID driver for NVIDIA SHIELD peripherals.
+ */
+
+#include <linux/hid.h>
+#include <linux/input-event-codes.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define NOT_INIT_STR "NOT INITIALIZED"
+
+enum {
+ SHIELD_FW_VERSION_INITIALIZED = 0,
+ SHIELD_BOARD_INFO_INITIALIZED,
+};
+
+enum {
+ THUNDERSTRIKE_FW_VERSION_UPDATE = 0,
+ THUNDERSTRIKE_BOARD_INFO_UPDATE,
+ THUNDERSTRIKE_HAPTICS_UPDATE,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE = 33,
+ THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID = 0x4,
+ THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID = 0x3,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION = 1,
+ THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO = 16,
+ THUNDERSTRIKE_HOSTCMD_ID_USB_INIT = 53,
+ THUNDERSTRIKE_HOSTCMD_ID_HAPTICS = 57,
+ THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT = 58,
+};
+
+struct thunderstrike_hostcmd_board_info {
+ __le16 revision;
+ __le16 serial[7];
+};
+
+struct thunderstrike_hostcmd_haptics {
+ u8 motor_left;
+ u8 motor_right;
+};
+
+struct thunderstrike_hostcmd_resp_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ union {
+ struct thunderstrike_hostcmd_board_info board_info;
+ struct thunderstrike_hostcmd_haptics motors;
+ __le16 fw_version;
+ u8 payload[30];
+ };
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_resp_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+struct thunderstrike_hostcmd_req_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ struct {
+ u8 update;
+ struct thunderstrike_hostcmd_haptics motors;
+ } haptics;
+ u8 reserved_at_30[27];
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_req_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+/* Common struct for shield accessories. */
+struct shield_device {
+ struct hid_device *hdev;
+
+ unsigned long initialized_flags;
+ const char *codename;
+ u16 fw_version;
+ struct {
+ u16 revision;
+ char serial_number[15];
+ } board_info;
+};
+
+struct thunderstrike {
+ struct shield_device base;
+
+ /* Sub-devices */
+ struct input_dev *haptics_dev;
+
+ /* Resources */
+ void *req_report_dmabuf;
+ unsigned long update_flags;
+ struct thunderstrike_hostcmd_haptics haptics_val;
+ spinlock_t haptics_update_lock;
+ struct work_struct hostcmd_req_work;
+};
+
+static inline void thunderstrike_hostcmd_req_report_init(
+ struct thunderstrike_hostcmd_req_report *report, u8 cmd_id)
+{
+ memset(report, 0, sizeof(*report));
+ report->report_id = THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID;
+ report->cmd_id = cmd_id;
+}
+
+static inline void shield_strrev(char *dest, size_t len, u16 rev)
+{
+ dest[0] = ('A' - 1) + (rev >> 8);
+ snprintf(&dest[1], len - 1, "%02X", 0xff & rev);
+}
+
+static struct input_dev *shield_allocate_input_dev(struct hid_device *hdev,
+ const char *name_suffix)
+{
+ struct input_dev *idev;
+
+ idev = input_allocate_device();
+ if (!idev)
+ goto err_device;
+
+ idev->id.bustype = hdev->bus;
+ idev->id.vendor = hdev->vendor;
+ idev->id.product = hdev->product;
+ idev->id.version = hdev->version;
+ idev->uniq = hdev->uniq;
+ idev->name = devm_kasprintf(&idev->dev, GFP_KERNEL, "%s %s", hdev->name,
+ name_suffix);
+ if (!idev->name)
+ goto err_name;
+
+ input_set_drvdata(idev, hdev);
+
+ return idev;
+
+err_name:
+ input_free_device(idev);
+err_device:
+ return ERR_PTR(-ENOMEM);
+}
+
+static struct input_dev *shield_haptics_create(
+ struct shield_device *dev,
+ int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
+{
+ struct input_dev *haptics;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_SHIELD_FF))
+ return NULL;
+
+ haptics = shield_allocate_input_dev(dev->hdev, "Haptics");
+ if (IS_ERR(haptics))
+ return haptics;
+
+ input_set_capability(haptics, EV_FF, FF_RUMBLE);
+ input_ff_create_memless(haptics, NULL, play_effect);
+
+ ret = input_register_device(haptics);
+ if (ret)
+ goto err;
+
+ return haptics;
+
+ err:
+ input_free_device(haptics);
+ return ERR_PTR(ret);
+}
+
+static inline void thunderstrike_send_hostcmd_request(struct thunderstrike *ts)
+{
+ struct thunderstrike_hostcmd_req_report *report = ts->req_report_dmabuf;
+ struct shield_device *shield_dev = &ts->base;
+ int ret;
+
+ ret = hid_hw_raw_request(shield_dev->hdev, report->report_id,
+ ts->req_report_dmabuf,
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+ if (ret < 0) {
+ hid_err(shield_dev->hdev,
+ "Failed to output Thunderstrike HOSTCMD request HID report due to %pe\n",
+ ERR_PTR(ret));
+ }
+}
+
+static void thunderstrike_hostcmd_req_work_handler(struct work_struct *work)
+{
+ struct thunderstrike *ts =
+ container_of(work, struct thunderstrike, hostcmd_req_work);
+ struct thunderstrike_hostcmd_req_report *report;
+ unsigned long flags;
+
+ report = ts->req_report_dmabuf;
+
+ if (test_and_clear_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_HAPTICS);
+
+ report->haptics.update = 1;
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ report->haptics.motors = ts->haptics_val;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ thunderstrike_send_hostcmd_request(ts);
+ }
+}
+
+static inline void thunderstrike_request_firmware_version(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline void thunderstrike_request_board_info(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline int
+thunderstrike_update_haptics(struct thunderstrike *ts,
+ struct thunderstrike_hostcmd_haptics *motors)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ ts->haptics_val = *motors;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ set_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+
+ return 0;
+}
+
+static int thunderstrike_play_effect(struct input_dev *idev, void *data,
+ struct ff_effect *effect)
+{
+ struct hid_device *hdev = input_get_drvdata(idev);
+ struct thunderstrike_hostcmd_haptics motors;
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ shield_dev = hid_get_drvdata(hdev);
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ /* Thunderstrike motor values range from 0 to 32 inclusively */
+ motors.motor_left = effect->u.rumble.strong_magnitude / 2047;
+ motors.motor_right = effect->u.rumble.weak_magnitude / 2047;
+
+ hid_dbg(hdev, "Thunderstrike FF_RUMBLE request, left: %u right: %u\n",
+ motors.motor_left, motors.motor_right);
+
+ return thunderstrike_update_haptics(ts, &motors);
+}
+
+static void
+thunderstrike_parse_fw_version_payload(struct shield_device *shield_dev,
+ __le16 fw_version)
+{
+ shield_dev->fw_version = le16_to_cpu(fw_version);
+
+ set_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags);
+
+ hid_dbg(shield_dev->hdev, "Thunderstrike firmware version 0x%04X\n",
+ shield_dev->fw_version);
+}
+
+static void
+thunderstrike_parse_board_info_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_board_info *board_info)
+{
+ char board_revision_str[4];
+ int i;
+
+ shield_dev->board_info.revision = le16_to_cpu(board_info->revision);
+ for (i = 0; i < 7; ++i) {
+ u16 val = le16_to_cpu(board_info->serial[i]);
+
+ shield_dev->board_info.serial_number[2 * i] = val & 0xFF;
+ shield_dev->board_info.serial_number[2 * i + 1] = val >> 8;
+ }
+ shield_dev->board_info.serial_number[14] = '\0';
+
+ set_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags);
+
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike BOARD_REVISION_%s (0x%04X) S/N: %s\n",
+ board_revision_str, shield_dev->board_info.revision,
+ shield_dev->board_info.serial_number);
+}
+
+static inline void
+thunderstrike_parse_haptics_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_haptics *haptics)
+{
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike haptics HOSTCMD response, left: %u right: %u\n",
+ haptics->motor_left, haptics->motor_right);
+}
+
+static int thunderstrike_parse_report(struct shield_device *shield_dev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct thunderstrike_hostcmd_resp_report *hostcmd_resp_report;
+ struct thunderstrike *ts =
+ container_of(shield_dev, struct thunderstrike, base);
+ struct hid_device *hdev = shield_dev->hdev;
+
+ switch (report->id) {
+ case THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID:
+ if (size != THUNDERSTRIKE_HOSTCMD_REPORT_SIZE) {
+ hid_err(hdev,
+ "Encountered Thunderstrike HOSTCMD HID report with unexpected size %d\n",
+ size);
+ return -EINVAL;
+ }
+
+ hostcmd_resp_report =
+ (struct thunderstrike_hostcmd_resp_report *)data;
+
+ switch (hostcmd_resp_report->cmd_id) {
+ case THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION:
+ thunderstrike_parse_fw_version_payload(
+ shield_dev, hostcmd_resp_report->fw_version);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO:
+ thunderstrike_parse_board_info_payload(
+ shield_dev, &hostcmd_resp_report->board_info);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_HAPTICS:
+ thunderstrike_parse_haptics_payload(
+ shield_dev, &hostcmd_resp_report->motors);
+ break;
+
+ case THUNDERSTRIKE_HOSTCMD_ID_USB_INIT:
+ case THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT:
+ /* May block HOSTCMD requests till received initially */
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+ /* Only HOSTCMD that can be triggered without a request */
+ return 0;
+ default:
+ hid_warn(hdev,
+ "Unhandled Thunderstrike HOSTCMD id %d\n",
+ hostcmd_resp_report->cmd_id);
+ return -ENOENT;
+ }
+
+ break;
+ default:
+ return 0;
+ }
+
+ return 0;
+}
+
+static struct shield_device *thunderstrike_create(struct hid_device *hdev)
+{
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ ts = devm_kzalloc(&hdev->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return ERR_PTR(-ENOMEM);
+
+ ts->req_report_dmabuf = devm_kzalloc(
+ &hdev->dev, THUNDERSTRIKE_HOSTCMD_REPORT_SIZE, GFP_KERNEL);
+ if (!ts->req_report_dmabuf)
+ return ERR_PTR(-ENOMEM);
+
+ shield_dev = &ts->base;
+ shield_dev->hdev = hdev;
+ shield_dev->codename = "Thunderstrike";
+
+ spin_lock_init(&ts->haptics_update_lock);
+ INIT_WORK(&ts->hostcmd_req_work, thunderstrike_hostcmd_req_work_handler);
+
+ hid_set_drvdata(hdev, shield_dev);
+
+ ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
+ if (IS_ERR(ts->haptics_dev))
+ return ERR_CAST(ts->haptics_dev);
+
+ hid_info(hdev, "Registered Thunderstrike controller\n");
+ return shield_dev;
+}
+
+static ssize_t firmware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "0x%04X\n", shield_dev->fw_version);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(firmware_version);
+
+static ssize_t hardware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ char board_revision_str[4];
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags)) {
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ ret = sysfs_emit(buf, "%s BOARD_REVISION_%s (0x%04X)\n",
+ shield_dev->codename, board_revision_str,
+ shield_dev->board_info.revision);
+ } else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(hardware_version);
+
+static ssize_t serial_number_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "%s\n", shield_dev->board_info.serial_number);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(serial_number);
+
+static struct attribute *shield_device_attrs[] = {
+ &dev_attr_firmware_version.attr,
+ &dev_attr_hardware_version.attr,
+ &dev_attr_serial_number.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(shield_device);
+
+static int shield_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+
+ return thunderstrike_parse_report(dev, report, data, size);
+}
+
+static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct shield_device *shield_dev = NULL;
+ struct thunderstrike *ts;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "Parse failed\n");
+ return ret;
+ }
+
+ switch (id->product) {
+ case USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER:
+ shield_dev = thunderstrike_create(hdev);
+ break;
+ }
+
+ if (unlikely(!shield_dev)) {
+ hid_err(hdev, "Failed to identify SHIELD device\n");
+ return -ENODEV;
+ }
+ if (IS_ERR(shield_dev)) {
+ hid_err(hdev, "Failed to create SHIELD device\n");
+ return PTR_ERR(shield_dev);
+ }
+
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
+ if (ret) {
+ hid_err(hdev, "Failed to start HID device\n");
+ goto err_haptics;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "Failed to open HID device\n");
+ goto err_stop;
+ }
+
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+
+ return ret;
+
+err_stop:
+ hid_hw_stop(hdev);
+err_haptics:
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ return ret;
+}
+
+static void shield_remove(struct hid_device *hdev)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+ struct thunderstrike *ts;
+
+ ts = container_of(dev, struct thunderstrike, base);
+
+ hid_hw_close(hdev);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ cancel_work_sync(&ts->hostcmd_req_work);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id shield_devices[] = {
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, shield_devices);
+
+static struct hid_driver shield_driver = {
+ .name = "shield",
+ .id_table = shield_devices,
+ .probe = shield_probe,
+ .remove = shield_remove,
+ .raw_event = shield_raw_event,
+ .driver = {
+ .dev_groups = shield_device_groups,
+ },
+};
+module_hid_driver(shield_driver);
+
+MODULE_AUTHOR("Rahul Rameshbabu <rrameshbabu@nvidia.com>");
+MODULE_DESCRIPTION("HID Driver for NVIDIA SHIELD peripherals.");
+MODULE_LICENSE("GPL");
--
2.38.4
^ permalink raw reply related
* Re: [PATCH v3 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Randy Dunlap @ 2023-04-17 19:20 UTC (permalink / raw)
To: Rahul Rameshbabu, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel
In-Reply-To: <20230417182909.33833-2-rrameshbabu@nvidia.com>
Hi,
On 4/17/23 11:29, Rahul Rameshbabu wrote:
> diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
> index 82f64fb31fda..eb19debaa1f5 100644
> --- a/drivers/hid/Kconfig
> +++ b/drivers/hid/Kconfig
> @@ -990,6 +990,24 @@ config HID_SEMITEK
> - Woo-dy
> - X-Bows Nature/Knight
>
> +config HID_SHIELD
> + tristate "SHIELD HID Driver"
> + depends on USB_HID
> + depends on BT_HIDP
> + help
> + Support for NVIDIA SHIELD accessories.
> +
> + Supported devices:
> + - Thunderstrike (NVIDIA SHIELD Controller 2017)
> +
> +config SHIELD_FF
> + bool "SHIELD force feedback support"
> + depends on HID_SHIELD
> + select INPUT_FF_MEMLESS
> + help
> + Say Y here if you would like to enable force feedback support for
> + NVIDIA SHIELD accessories with haptics capabilities.
According to Documentation/process/coding-style.rst, Kconfig help text should
be indented by 2 spaces after the "help" line:
10) Kconfig configuration files
-------------------------------
For all of the Kconfig* configuration files throughout the source tree,
the indentation is somewhat different. Lines under a ``config`` definition
are indented with one tab, while help text is indented an additional two
spaces. Example::
config AUDIT
bool "Auditing support"
depends on NET
help
Enable auditing infrastructure that can be used with another
kernel subsystem, such as SELinux (which requires this for
logging of avc messages output). Does not do system-call
auditing without CONFIG_AUDITSYSCALL.
--
~Randy
^ permalink raw reply
* Re: [PATCH v3 1/1] HID: shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-17 19:33 UTC (permalink / raw)
To: Randy Dunlap; +Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <1a754a3e-2a2b-07d5-c94a-0373a1677e4e@infradead.org>
Hi Randy,
Thanks for the feedback. You are correct about the styling issue in my
patch. I will fix this. I do want to bring to attention that there seem
to be a couple of kernel config options in the Kconfig file for the hid
subsystem that currently violate the styling rules.
It probably would make sense to have a clean-up patch for this issue
overall (separate from this patch, which I will correct in my next
version).
-- Rahul Rameshbabu
On Mon, 17 Apr, 2023 12:20:51 -0700 Randy Dunlap <rdunlap@infradead.org> wrote:
> According to Documentation/process/coding-style.rst, Kconfig help text should
> be indented by 2 spaces after the "help" line:
>
> 10) Kconfig configuration files
> -------------------------------
>
> For all of the Kconfig* configuration files throughout the source tree,
> the indentation is somewhat different. Lines under a ``config`` definition
> are indented with one tab, while help text is indented an additional two
> spaces. Example::
>
> config AUDIT
> bool "Auditing support"
> depends on NET
> help
> Enable auditing infrastructure that can be used with another
> kernel subsystem, such as SELinux (which requires this for
> logging of avc messages output). Does not do system-call
> auditing without CONFIG_AUDITSYSCALL.
^ permalink raw reply
* Re: [PATCH v2] Input: synaptics - disable intertouch for Lenovo L440
From: Andrew Duggan @ 2023-04-17 20:14 UTC (permalink / raw)
To: Jonathan Denose
Cc: Lyude Paul, Andrew Duggan, amandhoot12@gmail.com, Dmitry Torokhov,
jdenose@google.com, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org, markpearson@lenovo.com,
wsa+renesas@sang-engineering.com, benjamin.tissoires
In-Reply-To: <CALNJtpV4WsknSSfBBer-MM0y_V=O5Fv2Lc3ei3heEyZwvR2rzQ@mail.gmail.com>
Hi Lyude and Jonathan,
I was just about to reply and suggest that we look into this issue a little more since the touchpad in the L440 would benefit from the additional data from the intertouch interface. Especially, since it has a large area and several buttons. PS/2 only reports position data for two fingers so three finger gestures is another example.
Generally, these types of suspend / resume issues are the result of the touchpad resetting and the firmware expecting commands from the PS/2 interface. On resume, the PS/2 driver should send a command over the PS/2 interface to switch the touchpad firmware back into intertouch (SMBus) mode. The logs you provided look like that's what is happening here. The SMBus driver is sending commands, but the touchpad firmware won't respond until it is switch back into intertouch mode. It has been a while since I have worked on these touchpads, but from what I remember I think there is code in the psmouse-smbus driver to handle these situations. I added Benjamin Tissoires to CC since I think he worked on that handling. I thought suspend / resume was tested on with these "top button" touchpads when support for them was added. I don't know if the L440 specifically included in the testing. I'm curious if this is a regression or not.
Regarding the patch, I do have one comment below:
> On Apr 17, 2023, at 11:52, Jonathan Denose <jdenose@chromium.org> wrote:
>
> CAUTION: Email originated externally, do not click links or open attachments unless you recognize the sender and know the content is safe.
>
>
> Sorry, I thought I sent this as plain text but I think maybe not.
> Trying once more, the message was:
>
> I think that disabling synaptics_intertouch would resolve the issue
> mentioned in the commit, but cause a regression in the functionality
> that intertouch is supposed to bring, like three-finger gestures. I'm
> attaching some of the logs that I captured when the touchpad was
> failing on resume. I think the main culprit is
> i2c_smbus_read_byte_data where the driver is unable to get the SMBus
> version number. I get the following lines in dmesg:
>
> [ 2869.745860] rmi4_smbus 0-002c: failed to get SMBus version number!
> [ 2869.746060] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed
> to read current IRQ mask.
> [ 2869.746260] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
> [ 2869.746262] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
> [ 2869.746264] rmi4_physical rmi4-00: Failed to suspend functions: -6
> [ 2869.746265] rmi4_smbus 0-002c: Failed to resume device: -6
> [ 2869.746268] rmi4_smbus 0-002c: rmi_smb_resume+0x0/0x6b [rmi_smbus]
> returned 0 after 549 usecs
> [ 2869.746446] rmi4_physical rmi4-00: Failed to read irqs, code=-6
>
> Any ideas on what might be causing this, only on resume from deep sleep?
>
>
> On Mon, Apr 17, 2023 at 1:47 PM Jonathan Denose <jdenose@chromium.org> wrote:
>>
>> I think that disabling synaptics_intertouch would resolve the issue mentioned in the commit, but cause a regression in the functionality that intertouch is supposed to bring, like three-finger gestures. I'm attaching some of the logs that I captured when the touchpad was failing on resume. I think the main culprit is i2c_smbus_read_byte_data where the driver is unable to get the SMBus version number. I get the following lines in dmesg:
>>
>> [ 2869.745860] rmi4_smbus 0-002c: failed to get SMBus version number!
>> [ 2869.746060] rmi4_physical rmi4-00: rmi_driver_reset_handler: Failed to read current IRQ mask.
>> [ 2869.746260] rmi4_f01 rmi4-00.fn01: Failed to restore normal operation: -6.
>> [ 2869.746262] rmi4_f01 rmi4-00.fn01: Resume failed with code -6.
>> [ 2869.746264] rmi4_physical rmi4-00: Failed to suspend functions: -6
>> [ 2869.746265] rmi4_smbus 0-002c: Failed to resume device: -6
>> [ 2869.746268] rmi4_smbus 0-002c: rmi_smb_resume+0x0/0x6b [rmi_smbus] returned 0 after 549 usecs
>> [ 2869.746446] rmi4_physical rmi4-00: Failed to read irqs, code=-6
>>
>> Any ideas on what might be causing this, only on resume from deep sleep?
>>
>>
>> On Fri, Apr 14, 2023 at 11:41 AM Jonathan Denose <jdenose@chromium.org> wrote:
>>>
>>> When intertouch is enabled for the L440 a (deep)sleep/resume
>>> cycle causes the touchpad driver to hang which causes the
>>> touchpad to become unresponsive. Disable intertouch resolves
>>> this issue and the touchpad is fine after resume from sleep.
>>>
>>> Additionally, when the PNP id for the L440 is only removed
>>> from the topbuttonpad_pnp_ids list, a message is logged to
>>> enable psmouse.synaptics_intertouch, which would cause the
>>> sleep/resume issue again. By removing the PNP id from
>>> topbutton_pnp_ids and then adding it to the
>>> forcepad_pnp_ids array, intertouch is disabled and the
>>> message is not logged.
>>>
>>> Signed-off-by: Jonathan Denose <jdenose@google.com>
>>> ---
>>>
>>> Changes in v2:
>>> - remove debug statement
>>>
>>> drivers/input/mouse/synaptics.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/input/mouse/synaptics.c b/drivers/input/mouse/synaptics.c
>>> index fa021af8506e4..b7218b7652c20 100644
>>> --- a/drivers/input/mouse/synaptics.c
>>> +++ b/drivers/input/mouse/synaptics.c
>>> @@ -150,7 +150,6 @@ static const char * const topbuttonpad_pnp_ids[] = {
>>> "LEN2001", /* Edge E431 */
>>> "LEN2002", /* Edge E531 */
>>> "LEN2003",
>>> - "LEN2004", /* L440 */
>>> "LEN2005",
>>> "LEN2006", /* Edge E440/E540 */
>>> "LEN2007",
>>> @@ -198,6 +197,7 @@ static const char * const smbus_pnp_ids[] = {
>>> static const char * const forcepad_pnp_ids[] = {
>>> "SYN300D",
>>> "SYN3014",
>>> + "LEN2004", /* L440 */
While this does seem to elliminate the message, the touchpad in the L440 is not a forcepad. Adding the L440 PnP ID here implies that it is one of these special forcepads which reports "force" data for contacts and that is not the case here.
>>> NULL
>>> };
>>>
>>> —
>>> 2.39.2
>>>
Andrew
^ permalink raw reply
* Re: [PATCH 0/2] Input: st1232: wakeup in Suspend to idle
From: Javier Carrasco Cruz @ 2023-04-18 4:12 UTC (permalink / raw)
To: linux-input, linux-kernel
Cc: Dmitry Torokhov, Jonathan Cameron, Linus Walleij, Uwe Kleine-g,
Michael Riesch
In-Reply-To: <20230403124707.102986-1-javier.carrasco@wolfvision.net>
On 03.04.23 14:47, Javier Carrasco wrote:
> The st1232 touchscreen provides an interrupt line that can be configured
> as a wakeup source, and currently it is possible to use this mechanism in
> "Suspend to RAM" and "Hibernate" power states.
>
> Unfortunately, that does not work in "Suspend to idle" (freeze) because
> the device driver disables the interrupts in its suspend callback.
> Given that the interrupt handling prior to entering the mentioned power
> state modes is managed by the power subsystem, the irq enabling/disabling
> can be removed from the touchscreen driver, allowing the device to work
> as a wakeup source in "Suspend to idle".
>
> Given that the st1232 device driver does not reflect its wakeup events
> in sysfs, this series also adds pm_wakeup_event to the interrupt
> handler.
>
> These changes have been successfully tested with an ST1624-N32C and a
> Rockchip-based platform.
>
> Javier Carrasco (2):
> Input: st1232 - remove enable/disable irq in resume/suspend callbacks
> Input: st1232 - add wake up event counting
>
> drivers/input/touchscreen/st1232.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
>
Hi, I submitted this series 2 weeks ago and given that I did not get any
feedback yet, I would like to make sure that the patches arrived in the
right lists and if so, that they are "queued for review" and I just have
to wait, which would be indeed alright.
Thanks and best regards.
^ permalink raw reply
* Re: [PATCH 2/2] input: imx_sc_key: add wakeup support
From: Ulf Hansson @ 2023-04-18 8:32 UTC (permalink / raw)
To: Peng Fan
Cc: Dmitry Torokhov, robh+dt@kernel.org,
krzysztof.kozlowski+dt@linaro.org, shawnguo@kernel.org,
Aisheng Dong, linux-input@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, s.hauer@pengutronix.de,
kernel@pengutronix.de, festevam@gmail.com, dl-linux-imx
In-Reply-To: <DU0PR04MB9417185EB8243ED2D60A6E43889B9@DU0PR04MB9417.eurprd04.prod.outlook.com>
On Wed, 12 Apr 2023 at 17:58, Peng Fan <peng.fan@nxp.com> wrote:
>
> +Ulf
>
> > Subject: RE: [PATCH 2/2] input: imx_sc_key: add wakeup support
> >
> > > Subject: Re: [PATCH 2/2] input: imx_sc_key: add wakeup support
> > >
> > > On Thu, Mar 23, 2023 at 05:31:41PM +0800, Peng Fan (OSS) wrote:
> > > > From: Peng Fan <peng.fan@nxp.com>
> > > >
> > > > Add support for waking up from system wide suspend.
> > > >
> > > > Signed-off-by: Peng Fan <peng.fan@nxp.com>
> > > > ---
> > > > drivers/input/keyboard/imx_sc_key.c | 2 ++
> > > > 1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/drivers/input/keyboard/imx_sc_key.c
> > > b/drivers/input/keyboard/imx_sc_key.c
> > > > index d18839f1f4f6..234f23cf9990 100644
> > > > --- a/drivers/input/keyboard/imx_sc_key.c
> > > > +++ b/drivers/input/keyboard/imx_sc_key.c
> > > > @@ -151,6 +151,8 @@ static int imx_sc_key_probe(struct
> > > platform_device *pdev)
> > > > priv->input = input;
> > > > platform_set_drvdata(pdev, priv);
> > > >
> > > > + device_init_wakeup(&pdev->dev,
> > > device_property_read_bool(&pdev->dev, "wakeup-source"));
> > > > +
> > >
> > > I wonder - could we move this to the device core?
> >
> > I see lots device drivers parse wakeup-source, so I also follow That. Not sure
> > whether could move this feature to device core, but anyway I could give a
> > try.
>
> Do you think it is feasible to move device_init_wakeup into device core
> part?
Not sure it would really improve things that much. Subsystems/drivers
need to make additional configurations based upon whether this DT
property is set anyway.
Perhaps an option is to make this a part of the common input subsystem
helper functions instead? Other subsystems do this, but I am not sure
how feasible that would be in the input case.
Kind regards
Uffe
^ permalink raw reply
* [PATCH 0/2] Fix Goodix touchscreen power leakage for MT8186 boards
From: Fei Shao @ 2023-04-18 12:49 UTC (permalink / raw)
To: Benjamin Tissoires, Douglas Anderson, Rob Herring
Cc: linux-mediatek, Fei Shao, Dmitry Torokhov, Jiri Kosina,
Krzysztof Kozlowski, Matthias Kaehlcke, Stephen Kitt, devicetree,
linux-input, linux-kernel
These changes are based on the series in [1], which modified the
i2c-hid-of-goodix driver and removed the workaround for a power leakage
issue, so the issue revisits on Mediatek MT8186 boards (Steelix).
The root cause is that the touchscreen can be powered in different ways
depending on the hardware designs, and it's not as easy to come up with
a solution that is both simple and elegant for all the known designs.
To address the issue, I ended up adding a new boolean property for the
driver so that we can control the power up/down sequence depending on
that.
Adding a new property might not be the cleanest approach for this, but
at least the intention would be easy enough to understand, and it
introduces relatively small change to the code and fully preserves the
original control flow.
I hope this is something acceptable, and I'm open to any better
approaches.
[1] https://lore.kernel.org/all/20230207024816.525938-1-dianders@chromium.org/
Fei Shao (2):
dt-bindings: input: goodix: Add powered-in-suspend property
HID: i2c-hid: goodix: Add support for powered-in-suspend property
.../bindings/input/goodix,gt7375p.yaml | 6 +++
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 46 ++++++++++++++++---
2 files changed, 45 insertions(+), 7 deletions(-)
--
2.40.0.634.g4ca3ef3211-goog
^ permalink raw reply
* [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Fei Shao @ 2023-04-18 12:49 UTC (permalink / raw)
To: Benjamin Tissoires, Douglas Anderson, Rob Herring
Cc: linux-mediatek, Fei Shao, Dmitry Torokhov, Krzysztof Kozlowski,
devicetree, linux-input, linux-kernel
In-Reply-To: <20230418124953.3170028-1-fshao@chromium.org>
We observed that on Chromebook device Steelix, if Goodix GT7375P
touchscreen is powered in suspend (because, for example, it connects to
an always-on regulator) and with the reset GPIO asserted, it will
introduce about 14mW power leakage.
This property is used to indicate that the touchscreen is powered in
suspend. If it's set, the driver will stop asserting the reset GPIO in
power-down, and it will do it in power-up instead to ensure that the
state is always reset after resuming.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
Documentation/devicetree/bindings/input/goodix,gt7375p.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
index ce18d7dadae2..942acb286d77 100644
--- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
+++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
@@ -43,6 +43,12 @@ properties:
itself as long as it allows the main board to make signals compatible
with what the touchscreen is expecting for its IO rails.
+ powered-in-suspend:
+ description:
+ This indicates that the touchscreen is powered in suspend, so the driver
+ will not assert the reset GPIO in power-down to prevent power leakage.
+ type: boolean
+
required:
- compatible
- reg
--
2.40.0.634.g4ca3ef3211-goog
^ permalink raw reply related
* [PATCH 2/2] HID: i2c-hid: goodix: Add support for powered-in-suspend property
From: Fei Shao @ 2023-04-18 12:49 UTC (permalink / raw)
To: Benjamin Tissoires, Douglas Anderson, Rob Herring
Cc: linux-mediatek, Fei Shao, Dmitry Torokhov, Jiri Kosina,
Matthias Kaehlcke, Stephen Kitt, linux-input, linux-kernel
In-Reply-To: <20230418124953.3170028-1-fshao@chromium.org>
In the beginning, commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the
reset line to true state of the regulator") introduced a change to tie
the reset line of the Goodix touchscreen to the state of the regulator
to fix a power leakage issue in suspend.
After some time, the change was deemed unnecessary and was reverted in
commit 557e05fa9fdd ("HID: i2c-hid: goodix: Stop tying the reset line to
the regulator") due to difficulties in managing regulator notifiers for
designs like Evoker, which provides a second power rail to touchscreen.
However, the revert caused a power regression on another Chromebook
device Steelix in the field, which has a dedicated always-on regulator
for touchscreen and was covered by the workaround in the first commit.
To address both cases, this patch adds the support for the
`powered-in-suspend` property in the driver that allows the driver to
determine whether the touchscreen is still powered in suspend, and
handle the reset GPIO accordingly as below:
- When set to true, the driver does not assert the reset GPIO in power
down. To ensure a clean start and the consistent behavior, it does the
assertion in power up instead.
This is for designs with a dedicated always-on regulator.
- When set to false, the driver uses the original control flow and
asserts GPIO and disable regulators normally.
This is for the two-regulator and shared-regulator designs.
Signed-off-by: Fei Shao <fshao@chromium.org>
---
drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 46 +++++++++++++++++++++----
1 file changed, 39 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
index 0060e3dcd775..b438db8ca6f4 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-goodix.c
@@ -28,6 +28,7 @@ struct i2c_hid_of_goodix {
struct regulator *vdd;
struct regulator *vddio;
struct gpio_desc *reset_gpio;
+ bool powered_in_suspend;
const struct goodix_i2c_hid_timing_data *timings;
};
@@ -37,13 +38,34 @@ static int goodix_i2c_hid_power_up(struct i2chid_ops *ops)
container_of(ops, struct i2c_hid_of_goodix, ops);
int ret;
- ret = regulator_enable(ihid_goodix->vdd);
- if (ret)
- return ret;
-
- ret = regulator_enable(ihid_goodix->vddio);
- if (ret)
- return ret;
+ /*
+ * This is to ensure that the reset GPIO will be asserted and the
+ * regulators will be enabled for all cases.
+ */
+ if (ihid_goodix->powered_in_suspend) {
+ /*
+ * This is not mandatory, but we assert reset here (instead of
+ * in power-down) to ensure that the device will have a clean
+ * state later on just like the normal scenarios would have.
+ *
+ * Also, since the regulators were not disabled in power-down,
+ * we don't need to enable them here.
+ */
+ gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
+ } else {
+ /*
+ * In this case, the reset is already asserted (either in
+ * probe or power-down).
+ * All we need is to enable the regulators.
+ */
+ ret = regulator_enable(ihid_goodix->vdd);
+ if (ret)
+ return ret;
+
+ ret = regulator_enable(ihid_goodix->vddio);
+ if (ret)
+ return ret;
+ }
if (ihid_goodix->timings->post_power_delay_ms)
msleep(ihid_goodix->timings->post_power_delay_ms);
@@ -60,6 +82,13 @@ static void goodix_i2c_hid_power_down(struct i2chid_ops *ops)
struct i2c_hid_of_goodix *ihid_goodix =
container_of(ops, struct i2c_hid_of_goodix, ops);
+ /*
+ * Don't assert reset GPIO or disable regulators if we're keeping the
+ * device powered in suspend.
+ */
+ if (ihid_goodix->powered_in_suspend)
+ return;
+
gpiod_set_value_cansleep(ihid_goodix->reset_gpio, 1);
regulator_disable(ihid_goodix->vddio);
regulator_disable(ihid_goodix->vdd);
@@ -91,6 +120,9 @@ static int i2c_hid_of_goodix_probe(struct i2c_client *client)
if (IS_ERR(ihid_goodix->vddio))
return PTR_ERR(ihid_goodix->vddio);
+ ihid_goodix->powered_in_suspend =
+ of_property_read_bool(client->dev.of_node, "powered-in-suspend");
+
ihid_goodix->timings = device_get_match_data(&client->dev);
return i2c_hid_core_probe(client, &ihid_goodix->ops, 0x0001, 0);
--
2.40.0.634.g4ca3ef3211-goog
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Doug Anderson @ 2023-04-18 14:18 UTC (permalink / raw)
To: Fei Shao
Cc: Benjamin Tissoires, Rob Herring, linux-mediatek, Dmitry Torokhov,
Krzysztof Kozlowski, devicetree, linux-input, linux-kernel
In-Reply-To: <20230418124953.3170028-2-fshao@chromium.org>
Hi,
On Tue, Apr 18, 2023 at 5:50 AM Fei Shao <fshao@chromium.org> wrote:
>
> We observed that on Chromebook device Steelix, if Goodix GT7375P
> touchscreen is powered in suspend (because, for example, it connects to
> an always-on regulator) and with the reset GPIO asserted, it will
> introduce about 14mW power leakage.
>
> This property is used to indicate that the touchscreen is powered in
> suspend. If it's set, the driver will stop asserting the reset GPIO in
> power-down, and it will do it in power-up instead to ensure that the
> state is always reset after resuming.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
>
> Documentation/devicetree/bindings/input/goodix,gt7375p.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> index ce18d7dadae2..942acb286d77 100644
> --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> @@ -43,6 +43,12 @@ properties:
> itself as long as it allows the main board to make signals compatible
> with what the touchscreen is expecting for its IO rails.
>
> + powered-in-suspend:
> + description:
> + This indicates that the touchscreen is powered in suspend, so the driver
> + will not assert the reset GPIO in power-down to prevent power leakage.
> + type: boolean
This seems OK to me. The overall idea is that if we ever turn off the
power to the touchscreen we have to assert reset (active low) before
doing so, but we don't want to hold reset if we're not actually going
to turn the power off because it causes the touchscreen to go into a
high power state. This gets complicated if the power rail is always-on
or shared with another device.
Alternatives I could think of:
1. In the OS, monitor the regulator and see when it's about to go off
and then assert reset. This is what I tried to do in previous patches
but it got too messy in Linux. It also wasn't perfect since it's
(theoretically) possible for a regulator to turn off outside of the
scope of the OS (some PMICs will auto turn off rails when the main
processor says it's off).
2. In the OS, peek to see if our regulator is marked "always on". This
doesn't handle the shared rail case, of course. Also, regulators that
are "always on" from the OS perspective could still (theoretically)
get turned off at suspend time by the PMIC.
3. Don't even hook up the reset line and just leave the touchscreen
out of reset all the time. This has the disadvantage that we can't
reset the touchscreen if it gets into a bad state, which could be even
more important if the touchscreen is on an always-on or shared rail.
Given the above, the solution in ${SUBJECT} patch seems reasonable.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [PATCH 2/2] HID: i2c-hid: goodix: Add support for powered-in-suspend property
From: Doug Anderson @ 2023-04-18 14:22 UTC (permalink / raw)
To: Fei Shao
Cc: Benjamin Tissoires, Rob Herring, linux-mediatek, Dmitry Torokhov,
Jiri Kosina, Matthias Kaehlcke, Stephen Kitt, linux-input,
linux-kernel
In-Reply-To: <20230418124953.3170028-3-fshao@chromium.org>
Hi,
On Tue, Apr 18, 2023 at 5:51 AM Fei Shao <fshao@chromium.org> wrote:
>
> In the beginning, commit 18eeef46d359 ("HID: i2c-hid: goodix: Tie the
> reset line to true state of the regulator") introduced a change to tie
> the reset line of the Goodix touchscreen to the state of the regulator
> to fix a power leakage issue in suspend.
>
> After some time, the change was deemed unnecessary and was reverted in
> commit 557e05fa9fdd ("HID: i2c-hid: goodix: Stop tying the reset line to
> the regulator") due to difficulties in managing regulator notifiers for
> designs like Evoker, which provides a second power rail to touchscreen.
>
> However, the revert caused a power regression on another Chromebook
> device Steelix in the field, which has a dedicated always-on regulator
> for touchscreen and was covered by the workaround in the first commit.
>
> To address both cases, this patch adds the support for the
> `powered-in-suspend` property in the driver that allows the driver to
> determine whether the touchscreen is still powered in suspend, and
> handle the reset GPIO accordingly as below:
> - When set to true, the driver does not assert the reset GPIO in power
> down. To ensure a clean start and the consistent behavior, it does the
> assertion in power up instead.
> This is for designs with a dedicated always-on regulator.
> - When set to false, the driver uses the original control flow and
> asserts GPIO and disable regulators normally.
> This is for the two-regulator and shared-regulator designs.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
>
> ---
>
> drivers/hid/i2c-hid/i2c-hid-of-goodix.c | 46 +++++++++++++++++++++----
> 1 file changed, 39 insertions(+), 7 deletions(-)
I privately reviewed earlier versions of this patch, so it's
unsurprising that I have no comments. Assuming that the DT folks don't
have any objections to the bindings change, this LGTM.
Reviewed-by: Douglas Anderson <dianders@chromium.org>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Matthias Brugger @ 2023-04-18 18:02 UTC (permalink / raw)
To: Fei Shao, Benjamin Tissoires, Douglas Anderson, Rob Herring
Cc: devicetree, Dmitry Torokhov, linux-kernel, linux-mediatek,
Krzysztof Kozlowski, linux-input
In-Reply-To: <20230418124953.3170028-2-fshao@chromium.org>
On 18/04/2023 14:49, Fei Shao wrote:
> We observed that on Chromebook device Steelix, if Goodix GT7375P
> touchscreen is powered in suspend (because, for example, it connects to
> an always-on regulator) and with the reset GPIO asserted, it will
> introduce about 14mW power leakage.
>
> This property is used to indicate that the touchscreen is powered in
> suspend. If it's set, the driver will stop asserting the reset GPIO in
> power-down, and it will do it in power-up instead to ensure that the
> state is always reset after resuming.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
Reviewed-by: Matthias Brugger <matthias.bgg@gmail.com>
> ---
>
> Documentation/devicetree/bindings/input/goodix,gt7375p.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> index ce18d7dadae2..942acb286d77 100644
> --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> @@ -43,6 +43,12 @@ properties:
> itself as long as it allows the main board to make signals compatible
> with what the touchscreen is expecting for its IO rails.
>
> + powered-in-suspend:
> + description:
> + This indicates that the touchscreen is powered in suspend, so the driver
> + will not assert the reset GPIO in power-down to prevent power leakage.
> + type: boolean
> +
> required:
> - compatible
> - reg
^ permalink raw reply
* [PATCH v4 0/1] HID: nvidia-shield
From: Rahul Rameshbabu @ 2023-04-18 20:37 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina
Cc: linux-input, linux-kernel, Randy Dunlap, Rahul Rameshbabu
Hi.
This is an initial driver implementation for supporting NVIDIA SHIELD
peripherals. Currently supports the following functionality for the
THUNDERSTRIKE (SHIELD 2017) controller.
- Haptics (ff_memless)
- Serial number query (sysfs)
- Hardware information query (sysfs)
- Firmware version query (sysfs)
Changes:
v3 -> v4:
* Follow correct formatting rules for help section in drivers/hid/Kconfig.
* Refactor configuration names to indicate Kconfig options are for NVIDIA
devices.
v2 -> v3:
* Renamed drivers/hid/hid-shield.c -> drivers/hid/hid-nvidia-shield.c.
* Added space between braces for terminating element in shield_devices
array.
v1 -> v2:
* Declared thunderstrike_hostcmd_req_work_handler as static.
Reported-by: kernel test robot <lkp@intel.com>
Link: https://lore.kernel.org/oe-kbuild-all/202304011342.6Bh3cWhA-lkp@intel.com/
Link: https://lore.kernel.org/oe-kbuild-all/202304020922.vsngJnBT-lkp@intel.com/
Rahul Rameshbabu (1):
HID: nvidia-shield: Initial driver implementation with Thunderstrike
support
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-nvidia-shield.c | 587 ++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-nvidia-shield.c
Link: https://lore.kernel.org/linux-input/20230417182909.33833-1-rrameshbabu@nvidia.com/
Link: https://lore.kernel.org/linux-input/20230410170840.16119-1-rrameshbabu@nvidia.com/
Link: https://lore.kernel.org/linux-input/20230401032150.7424-1-rrameshbabu@nvidia.com/
--
2.38.4
^ permalink raw reply
* [PATCH v4 1/1] HID: nvidia-shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-04-18 20:37 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina
Cc: linux-input, linux-kernel, Randy Dunlap, Rahul Rameshbabu
In-Reply-To: <20230418203747.24000-1-rrameshbabu@nvidia.com>
Supports the Thunderstrike (SHIELD 2017) controller. Implements support for
the Thunderstrike HOSTCMD firmware interface. Adds sysfs attributes about a
SHIELD device and introduces haptics support for controllers.
Signed-off-by: Rahul Rameshbabu <rrameshbabu@nvidia.com>
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 18 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 3 +
drivers/hid/hid-nvidia-shield.c | 587 ++++++++++++++++++++++++++++++++
5 files changed, 615 insertions(+)
create mode 100644 drivers/hid/hid-nvidia-shield.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2b073facf399..2e8badb87da8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9098,6 +9098,12 @@ L: linux-input@vger.kernel.org
S: Maintained
F: drivers/hid/hid-logitech-hidpp.c
+HID NVIDIA SHIELD DRIVER
+M: Rahul Rameshbabu <rrameshbabu@nvidia.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-nvidia-shield.c
+
HID PLAYSTATION DRIVER
M: Roderick Colenbrander <roderick.colenbrander@sony.com>
L: linux-input@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 82f64fb31fda..b1790dc20d2d 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -788,6 +788,24 @@ config HID_NTRIG
help
Support for N-Trig touch screen.
+config HID_NVIDIA_SHIELD
+ tristate "NVIDIA SHIELD devices"
+ depends on USB_HID
+ depends on BT_HIDP
+ help
+ Support for NVIDIA SHIELD accessories.
+
+ Supported devices:
+ - Thunderstrike (NVIDIA SHIELD Controller 2017)
+
+config NVIDIA_SHIELD_FF
+ bool "NVIDIA SHIELD force feedback support"
+ depends on HID_NVIDIA_SHIELD
+ select INPUT_FF_MEMLESS
+ help
+ Say Y here if you would like to enable force feedback support for
+ NVIDIA SHIELD accessories with haptics capabilities.
+
config HID_ORTEK
tristate "Ortek PKB-1700/WKB-2000/Skycable wireless keyboard and mouse trackpad"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..7a9e160158f7 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -87,6 +87,7 @@ obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
obj-$(CONFIG_HID_NTRIG) += hid-ntrig.o
+obj-$(CONFIG_HID_NVIDIA_SHIELD) += hid-nvidia-shield.o
obj-$(CONFIG_HID_ORTEK) += hid-ortek.o
obj-$(CONFIG_HID_PRODIKEYS) += hid-prodikeys.o
obj-$(CONFIG_HID_PANTHERLORD) += hid-pl.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 63545cd307e5..4000d53b1bac 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -993,6 +993,9 @@
#define USB_DEVICE_ID_NTRIG_TOUCH_SCREEN_18 0x0014
#define USB_DEVICE_ID_NTRIG_DUOSENSE 0x1500
+#define USB_VENDOR_ID_NVIDIA 0x0955
+#define USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER 0x7214
+
#define USB_VENDOR_ID_ONTRAK 0x0a07
#define USB_DEVICE_ID_ONTRAK_ADU100 0x0064
diff --git a/drivers/hid/hid-nvidia-shield.c b/drivers/hid/hid-nvidia-shield.c
new file mode 100644
index 000000000000..bcf345e365c0
--- /dev/null
+++ b/drivers/hid/hid-nvidia-shield.c
@@ -0,0 +1,587 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+ *
+ * HID driver for NVIDIA SHIELD peripherals.
+ */
+
+#include <linux/hid.h>
+#include <linux/input-event-codes.h>
+#include <linux/input.h>
+#include <linux/module.h>
+#include <linux/spinlock.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define NOT_INIT_STR "NOT INITIALIZED"
+
+enum {
+ SHIELD_FW_VERSION_INITIALIZED = 0,
+ SHIELD_BOARD_INFO_INITIALIZED,
+};
+
+enum {
+ THUNDERSTRIKE_FW_VERSION_UPDATE = 0,
+ THUNDERSTRIKE_BOARD_INFO_UPDATE,
+ THUNDERSTRIKE_HAPTICS_UPDATE,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE = 33,
+ THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID = 0x4,
+ THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID = 0x3,
+};
+
+enum {
+ THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION = 1,
+ THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO = 16,
+ THUNDERSTRIKE_HOSTCMD_ID_USB_INIT = 53,
+ THUNDERSTRIKE_HOSTCMD_ID_HAPTICS = 57,
+ THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT = 58,
+};
+
+struct thunderstrike_hostcmd_board_info {
+ __le16 revision;
+ __le16 serial[7];
+};
+
+struct thunderstrike_hostcmd_haptics {
+ u8 motor_left;
+ u8 motor_right;
+};
+
+struct thunderstrike_hostcmd_resp_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ union {
+ struct thunderstrike_hostcmd_board_info board_info;
+ struct thunderstrike_hostcmd_haptics motors;
+ __le16 fw_version;
+ u8 payload[30];
+ };
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_resp_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+struct thunderstrike_hostcmd_req_report {
+ u8 report_id; /* THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID */
+ u8 cmd_id;
+ u8 reserved_at_10;
+
+ struct {
+ u8 update;
+ struct thunderstrike_hostcmd_haptics motors;
+ } haptics;
+ u8 reserved_at_30[27];
+} __packed;
+static_assert(sizeof(struct thunderstrike_hostcmd_req_report) ==
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE);
+
+/* Common struct for shield accessories. */
+struct shield_device {
+ struct hid_device *hdev;
+
+ unsigned long initialized_flags;
+ const char *codename;
+ u16 fw_version;
+ struct {
+ u16 revision;
+ char serial_number[15];
+ } board_info;
+};
+
+struct thunderstrike {
+ struct shield_device base;
+
+ /* Sub-devices */
+ struct input_dev *haptics_dev;
+
+ /* Resources */
+ void *req_report_dmabuf;
+ unsigned long update_flags;
+ struct thunderstrike_hostcmd_haptics haptics_val;
+ spinlock_t haptics_update_lock;
+ struct work_struct hostcmd_req_work;
+};
+
+static inline void thunderstrike_hostcmd_req_report_init(
+ struct thunderstrike_hostcmd_req_report *report, u8 cmd_id)
+{
+ memset(report, 0, sizeof(*report));
+ report->report_id = THUNDERSTRIKE_HOSTCMD_REQ_REPORT_ID;
+ report->cmd_id = cmd_id;
+}
+
+static inline void shield_strrev(char *dest, size_t len, u16 rev)
+{
+ dest[0] = ('A' - 1) + (rev >> 8);
+ snprintf(&dest[1], len - 1, "%02X", 0xff & rev);
+}
+
+static struct input_dev *shield_allocate_input_dev(struct hid_device *hdev,
+ const char *name_suffix)
+{
+ struct input_dev *idev;
+
+ idev = input_allocate_device();
+ if (!idev)
+ goto err_device;
+
+ idev->id.bustype = hdev->bus;
+ idev->id.vendor = hdev->vendor;
+ idev->id.product = hdev->product;
+ idev->id.version = hdev->version;
+ idev->uniq = hdev->uniq;
+ idev->name = devm_kasprintf(&idev->dev, GFP_KERNEL, "%s %s", hdev->name,
+ name_suffix);
+ if (!idev->name)
+ goto err_name;
+
+ input_set_drvdata(idev, hdev);
+
+ return idev;
+
+err_name:
+ input_free_device(idev);
+err_device:
+ return ERR_PTR(-ENOMEM);
+}
+
+static struct input_dev *shield_haptics_create(
+ struct shield_device *dev,
+ int (*play_effect)(struct input_dev *, void *, struct ff_effect *))
+{
+ struct input_dev *haptics;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_NVIDIA_SHIELD_FF))
+ return NULL;
+
+ haptics = shield_allocate_input_dev(dev->hdev, "Haptics");
+ if (IS_ERR(haptics))
+ return haptics;
+
+ input_set_capability(haptics, EV_FF, FF_RUMBLE);
+ input_ff_create_memless(haptics, NULL, play_effect);
+
+ ret = input_register_device(haptics);
+ if (ret)
+ goto err;
+
+ return haptics;
+
+ err:
+ input_free_device(haptics);
+ return ERR_PTR(ret);
+}
+
+static inline void thunderstrike_send_hostcmd_request(struct thunderstrike *ts)
+{
+ struct thunderstrike_hostcmd_req_report *report = ts->req_report_dmabuf;
+ struct shield_device *shield_dev = &ts->base;
+ int ret;
+
+ ret = hid_hw_raw_request(shield_dev->hdev, report->report_id,
+ ts->req_report_dmabuf,
+ THUNDERSTRIKE_HOSTCMD_REPORT_SIZE,
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+
+ if (ret < 0) {
+ hid_err(shield_dev->hdev,
+ "Failed to output Thunderstrike HOSTCMD request HID report due to %pe\n",
+ ERR_PTR(ret));
+ }
+}
+
+static void thunderstrike_hostcmd_req_work_handler(struct work_struct *work)
+{
+ struct thunderstrike *ts =
+ container_of(work, struct thunderstrike, hostcmd_req_work);
+ struct thunderstrike_hostcmd_req_report *report;
+ unsigned long flags;
+
+ report = ts->req_report_dmabuf;
+
+ if (test_and_clear_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO);
+ thunderstrike_send_hostcmd_request(ts);
+ }
+
+ if (test_and_clear_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags)) {
+ thunderstrike_hostcmd_req_report_init(
+ report, THUNDERSTRIKE_HOSTCMD_ID_HAPTICS);
+
+ report->haptics.update = 1;
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ report->haptics.motors = ts->haptics_val;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ thunderstrike_send_hostcmd_request(ts);
+ }
+}
+
+static inline void thunderstrike_request_firmware_version(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_FW_VERSION_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline void thunderstrike_request_board_info(struct thunderstrike *ts)
+{
+ set_bit(THUNDERSTRIKE_BOARD_INFO_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+}
+
+static inline int
+thunderstrike_update_haptics(struct thunderstrike *ts,
+ struct thunderstrike_hostcmd_haptics *motors)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ts->haptics_update_lock, flags);
+ ts->haptics_val = *motors;
+ spin_unlock_irqrestore(&ts->haptics_update_lock, flags);
+
+ set_bit(THUNDERSTRIKE_HAPTICS_UPDATE, &ts->update_flags);
+ schedule_work(&ts->hostcmd_req_work);
+
+ return 0;
+}
+
+static int thunderstrike_play_effect(struct input_dev *idev, void *data,
+ struct ff_effect *effect)
+{
+ struct hid_device *hdev = input_get_drvdata(idev);
+ struct thunderstrike_hostcmd_haptics motors;
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ if (effect->type != FF_RUMBLE)
+ return 0;
+
+ shield_dev = hid_get_drvdata(hdev);
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ /* Thunderstrike motor values range from 0 to 32 inclusively */
+ motors.motor_left = effect->u.rumble.strong_magnitude / 2047;
+ motors.motor_right = effect->u.rumble.weak_magnitude / 2047;
+
+ hid_dbg(hdev, "Thunderstrike FF_RUMBLE request, left: %u right: %u\n",
+ motors.motor_left, motors.motor_right);
+
+ return thunderstrike_update_haptics(ts, &motors);
+}
+
+static void
+thunderstrike_parse_fw_version_payload(struct shield_device *shield_dev,
+ __le16 fw_version)
+{
+ shield_dev->fw_version = le16_to_cpu(fw_version);
+
+ set_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags);
+
+ hid_dbg(shield_dev->hdev, "Thunderstrike firmware version 0x%04X\n",
+ shield_dev->fw_version);
+}
+
+static void
+thunderstrike_parse_board_info_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_board_info *board_info)
+{
+ char board_revision_str[4];
+ int i;
+
+ shield_dev->board_info.revision = le16_to_cpu(board_info->revision);
+ for (i = 0; i < 7; ++i) {
+ u16 val = le16_to_cpu(board_info->serial[i]);
+
+ shield_dev->board_info.serial_number[2 * i] = val & 0xFF;
+ shield_dev->board_info.serial_number[2 * i + 1] = val >> 8;
+ }
+ shield_dev->board_info.serial_number[14] = '\0';
+
+ set_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags);
+
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike BOARD_REVISION_%s (0x%04X) S/N: %s\n",
+ board_revision_str, shield_dev->board_info.revision,
+ shield_dev->board_info.serial_number);
+}
+
+static inline void
+thunderstrike_parse_haptics_payload(struct shield_device *shield_dev,
+ struct thunderstrike_hostcmd_haptics *haptics)
+{
+ hid_dbg(shield_dev->hdev,
+ "Thunderstrike haptics HOSTCMD response, left: %u right: %u\n",
+ haptics->motor_left, haptics->motor_right);
+}
+
+static int thunderstrike_parse_report(struct shield_device *shield_dev,
+ struct hid_report *report, u8 *data,
+ int size)
+{
+ struct thunderstrike_hostcmd_resp_report *hostcmd_resp_report;
+ struct thunderstrike *ts =
+ container_of(shield_dev, struct thunderstrike, base);
+ struct hid_device *hdev = shield_dev->hdev;
+
+ switch (report->id) {
+ case THUNDERSTRIKE_HOSTCMD_RESP_REPORT_ID:
+ if (size != THUNDERSTRIKE_HOSTCMD_REPORT_SIZE) {
+ hid_err(hdev,
+ "Encountered Thunderstrike HOSTCMD HID report with unexpected size %d\n",
+ size);
+ return -EINVAL;
+ }
+
+ hostcmd_resp_report =
+ (struct thunderstrike_hostcmd_resp_report *)data;
+
+ switch (hostcmd_resp_report->cmd_id) {
+ case THUNDERSTRIKE_HOSTCMD_ID_FW_VERSION:
+ thunderstrike_parse_fw_version_payload(
+ shield_dev, hostcmd_resp_report->fw_version);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_BOARD_INFO:
+ thunderstrike_parse_board_info_payload(
+ shield_dev, &hostcmd_resp_report->board_info);
+ break;
+ case THUNDERSTRIKE_HOSTCMD_ID_HAPTICS:
+ thunderstrike_parse_haptics_payload(
+ shield_dev, &hostcmd_resp_report->motors);
+ break;
+
+ case THUNDERSTRIKE_HOSTCMD_ID_USB_INIT:
+ case THUNDERSTRIKE_HOSTCMD_ID_BLUETOOTH_INIT:
+ /* May block HOSTCMD requests till received initially */
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+ /* Only HOSTCMD that can be triggered without a request */
+ return 0;
+ default:
+ hid_warn(hdev,
+ "Unhandled Thunderstrike HOSTCMD id %d\n",
+ hostcmd_resp_report->cmd_id);
+ return -ENOENT;
+ }
+
+ break;
+ default:
+ return 0;
+ }
+
+ return 0;
+}
+
+static struct shield_device *thunderstrike_create(struct hid_device *hdev)
+{
+ struct shield_device *shield_dev;
+ struct thunderstrike *ts;
+
+ ts = devm_kzalloc(&hdev->dev, sizeof(*ts), GFP_KERNEL);
+ if (!ts)
+ return ERR_PTR(-ENOMEM);
+
+ ts->req_report_dmabuf = devm_kzalloc(
+ &hdev->dev, THUNDERSTRIKE_HOSTCMD_REPORT_SIZE, GFP_KERNEL);
+ if (!ts->req_report_dmabuf)
+ return ERR_PTR(-ENOMEM);
+
+ shield_dev = &ts->base;
+ shield_dev->hdev = hdev;
+ shield_dev->codename = "Thunderstrike";
+
+ spin_lock_init(&ts->haptics_update_lock);
+ INIT_WORK(&ts->hostcmd_req_work, thunderstrike_hostcmd_req_work_handler);
+
+ hid_set_drvdata(hdev, shield_dev);
+
+ ts->haptics_dev = shield_haptics_create(shield_dev, thunderstrike_play_effect);
+ if (IS_ERR(ts->haptics_dev))
+ return ERR_CAST(ts->haptics_dev);
+
+ hid_info(hdev, "Registered Thunderstrike controller\n");
+ return shield_dev;
+}
+
+static ssize_t firmware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_FW_VERSION_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "0x%04X\n", shield_dev->fw_version);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(firmware_version);
+
+static ssize_t hardware_version_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ char board_revision_str[4];
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags)) {
+ shield_strrev(board_revision_str, 4, shield_dev->board_info.revision);
+ ret = sysfs_emit(buf, "%s BOARD_REVISION_%s (0x%04X)\n",
+ shield_dev->codename, board_revision_str,
+ shield_dev->board_info.revision);
+ } else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(hardware_version);
+
+static ssize_t serial_number_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct shield_device *shield_dev;
+ int ret;
+
+ shield_dev = hid_get_drvdata(hdev);
+
+ if (test_bit(SHIELD_BOARD_INFO_INITIALIZED, &shield_dev->initialized_flags))
+ ret = sysfs_emit(buf, "%s\n", shield_dev->board_info.serial_number);
+ else
+ ret = sysfs_emit(buf, NOT_INIT_STR "\n");
+
+ return ret;
+}
+
+static DEVICE_ATTR_RO(serial_number);
+
+static struct attribute *shield_device_attrs[] = {
+ &dev_attr_firmware_version.attr,
+ &dev_attr_hardware_version.attr,
+ &dev_attr_serial_number.attr,
+ NULL,
+};
+ATTRIBUTE_GROUPS(shield_device);
+
+static int shield_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+
+ return thunderstrike_parse_report(dev, report, data, size);
+}
+
+static int shield_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct shield_device *shield_dev = NULL;
+ struct thunderstrike *ts;
+ int ret;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "Parse failed\n");
+ return ret;
+ }
+
+ switch (id->product) {
+ case USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER:
+ shield_dev = thunderstrike_create(hdev);
+ break;
+ }
+
+ if (unlikely(!shield_dev)) {
+ hid_err(hdev, "Failed to identify SHIELD device\n");
+ return -ENODEV;
+ }
+ if (IS_ERR(shield_dev)) {
+ hid_err(hdev, "Failed to create SHIELD device\n");
+ return PTR_ERR(shield_dev);
+ }
+
+ ts = container_of(shield_dev, struct thunderstrike, base);
+
+ ret = hid_hw_start(hdev, HID_CONNECT_HIDINPUT);
+ if (ret) {
+ hid_err(hdev, "Failed to start HID device\n");
+ goto err_haptics;
+ }
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "Failed to open HID device\n");
+ goto err_stop;
+ }
+
+ thunderstrike_request_firmware_version(ts);
+ thunderstrike_request_board_info(ts);
+
+ return ret;
+
+err_stop:
+ hid_hw_stop(hdev);
+err_haptics:
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ return ret;
+}
+
+static void shield_remove(struct hid_device *hdev)
+{
+ struct shield_device *dev = hid_get_drvdata(hdev);
+ struct thunderstrike *ts;
+
+ ts = container_of(dev, struct thunderstrike, base);
+
+ hid_hw_close(hdev);
+ if (ts->haptics_dev)
+ input_unregister_device(ts->haptics_dev);
+ cancel_work_sync(&ts->hostcmd_req_work);
+ hid_hw_stop(hdev);
+}
+
+static const struct hid_device_id shield_devices[] = {
+ { HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_NVIDIA,
+ USB_DEVICE_ID_NVIDIA_THUNDERSTRIKE_CONTROLLER) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, shield_devices);
+
+static struct hid_driver shield_driver = {
+ .name = "shield",
+ .id_table = shield_devices,
+ .probe = shield_probe,
+ .remove = shield_remove,
+ .raw_event = shield_raw_event,
+ .driver = {
+ .dev_groups = shield_device_groups,
+ },
+};
+module_hid_driver(shield_driver);
+
+MODULE_AUTHOR("Rahul Rameshbabu <rrameshbabu@nvidia.com>");
+MODULE_DESCRIPTION("HID Driver for NVIDIA SHIELD peripherals.");
+MODULE_LICENSE("GPL");
--
2.38.4
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: input: goodix: Add powered-in-suspend property
From: Jeff LaBundy @ 2023-04-19 0:20 UTC (permalink / raw)
To: Fei Shao
Cc: Benjamin Tissoires, Douglas Anderson, Rob Herring, linux-mediatek,
Dmitry Torokhov, Krzysztof Kozlowski, devicetree, linux-input,
linux-kernel
In-Reply-To: <20230418124953.3170028-2-fshao@chromium.org>
Hi Fei,
On Tue, Apr 18, 2023 at 08:49:51PM +0800, Fei Shao wrote:
> We observed that on Chromebook device Steelix, if Goodix GT7375P
> touchscreen is powered in suspend (because, for example, it connects to
> an always-on regulator) and with the reset GPIO asserted, it will
> introduce about 14mW power leakage.
>
> This property is used to indicate that the touchscreen is powered in
> suspend. If it's set, the driver will stop asserting the reset GPIO in
> power-down, and it will do it in power-up instead to ensure that the
> state is always reset after resuming.
>
> Signed-off-by: Fei Shao <fshao@chromium.org>
> ---
This is an interesting problem; were you able to root-cause why the silicon
exhibits this behavior? Simply asserting reset should not cause it to draw
additional power, let alone 14 mW. This almost sounds like a back-powering
problem during suspend.
If this is truly expected behavior, is it sufficient to use the always_on
constraint of the relevant regulator(s) to make this decision as opposed to
introducing a new property?
>
> Documentation/devicetree/bindings/input/goodix,gt7375p.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> index ce18d7dadae2..942acb286d77 100644
> --- a/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> +++ b/Documentation/devicetree/bindings/input/goodix,gt7375p.yaml
> @@ -43,6 +43,12 @@ properties:
> itself as long as it allows the main board to make signals compatible
> with what the touchscreen is expecting for its IO rails.
>
> + powered-in-suspend:
> + description:
> + This indicates that the touchscreen is powered in suspend, so the driver
> + will not assert the reset GPIO in power-down to prevent power leakage.
> + type: boolean
> +
> required:
> - compatible
> - reg
> --
> 2.40.0.634.g4ca3ef3211-goog
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [PATCH] Input: xpad - Move Xbox 360 magic packet sending
From: Vicki Pfau @ 2023-04-19 2:24 UTC (permalink / raw)
To: Dmitry Torokhov, Dongliang Mu, linux-input; +Cc: Dan Carpenter, Vicki Pfau
This moves the sending of the magic packet introduced in db7220c48d8d from
xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
event that an error occurs. This should also fix issues with suspend that may
occur with some controllers.
Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
Signed-off-by: Vicki Pfau <vi@endrift.com>
---
drivers/input/joystick/xpad.c | 43 +++++++++++++++++------------------
1 file changed, 21 insertions(+), 22 deletions(-)
diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
index 138e4a9f341f..3eab76b2c8fc 100644
--- a/drivers/input/joystick/xpad.c
+++ b/drivers/input/joystick/xpad.c
@@ -1721,6 +1721,27 @@ static int xpad_start_input(struct usb_xpad *xpad)
return error;
}
}
+ if (xpad->xtype == XTYPE_XBOX360) {
+ /*
+ * Some third-party controllers Xbox 360-style controllers
+ * require this message to finish initialization.
+ */
+ u8 dummy[20];
+
+ error = usb_control_msg_recv(udev, 0,
+ /* bRequest */ 0x01,
+ /* bmRequestType */
+ USB_TYPE_VENDOR | USB_DIR_IN |
+ USB_RECIP_INTERFACE,
+ /* wValue */ 0x100,
+ /* wIndex */ 0x00,
+ dummy, sizeof(dummy),
+ 25, GFP_KERNEL);
+ if (error)
+ dev_warn(&xpad->dev->dev,
+ "unable to receive magic message: %d\n",
+ error);
+ }
return 0;
}
@@ -2032,28 +2053,6 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
goto err_free_in_urb;
}
- if (xpad->xtype == XTYPE_XBOX360) {
- /*
- * Some third-party controllers Xbox 360-style controllers
- * require this message to finish initialization.
- */
- u8 dummy[20];
-
- error = usb_control_msg_recv(udev, 0,
- /* bRequest */ 0x01,
- /* bmRequestType */
- USB_TYPE_VENDOR | USB_DIR_IN |
- USB_RECIP_INTERFACE,
- /* wValue */ 0x100,
- /* wIndex */ 0x00,
- dummy, sizeof(dummy),
- 25, GFP_KERNEL);
- if (error)
- dev_warn(&xpad->dev->dev,
- "unable to receive magic message: %d\n",
- error);
- }
-
ep_irq_in = ep_irq_out = NULL;
for (i = 0; i < 2; i++) {
--
2.40.0
^ permalink raw reply related
* Re: [PATCH] Input: xpad - Move Xbox 360 magic packet sending
From: Dan Carpenter @ 2023-04-19 4:28 UTC (permalink / raw)
To: Vicki Pfau; +Cc: Dmitry Torokhov, Dongliang Mu, linux-input
In-Reply-To: <20230419022414.1790979-1-vi@endrift.com>
On Tue, Apr 18, 2023 at 07:24:14PM -0700, Vicki Pfau wrote:
> This moves the sending of the magic packet introduced in db7220c48d8d from
> xpad_probe to xpad_start_input to ensure that xpad->dev->dev exists in the
> event that an error occurs. This should also fix issues with suspend that may
> occur with some controllers.
>
> Fixes: db7220c48d8d ("Input: xpad - fix support for some third-party controllers")
> Signed-off-by: Vicki Pfau <vi@endrift.com>
Can you add the syzbot stuff as well and a Reported-by tag for
Dongliang Mu as well.
Reported-by: syzbot+a3f758b8d8cb7e49afec@syzkaller.appspotmail.com
Reported-by: Dongliang Mu <dzm91@hust.edu.cn>
Link: https://groups.google.com/g/syzkaller-bugs/c/iMhTgpGuIbM
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH v3] staging: HID: Add ShanWan USB WirelessGamepad driver
From: Mubashshir @ 2023-04-19 6:36 UTC (permalink / raw)
To: Benjamin Tissoires; +Cc: Jiri Kosina, Huseyin BIYIK, linux-kernel, linux-input
In-Reply-To: <20230413161305.ywthwbu3ta3mv66b@mail.corp.redhat.com>
On Thu, Apr 13 2023 at 06:13:05 PM +02:00:00, Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
> What's the point of keeping it opened for the lifetime of the device?
> Do
> you really need this?
>
>> + if (error) {
>> + dev_err(&hdev->dev, "hw open failed\n");
>> + hid_hw_stop(hdev);
>> + return error;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static void shanwan_remove(struct hid_device *hdev)
>> +{
>> + hid_hw_close(hdev);
>
> If you can drop the last hid_hw_open/close, then you can entirely skip
> the ->remove().
Would hid_hw_request work without open device?
^ permalink raw reply
* W sprawie samochodu
From: Krystian Wieczorek @ 2023-04-19 8:45 UTC (permalink / raw)
To: linux-input
Dzień dobry,
chcielibyśmy zapewnić Państwu kompleksowe rozwiązania, jeśli chodzi o system monitoringu GPS.
Precyzyjne monitorowanie pojazdów na mapach cyfrowych, śledzenie ich parametrów eksploatacyjnych w czasie rzeczywistym oraz kontrola paliwa to kluczowe funkcjonalności naszego systemu.
Organizowanie pracy pracowników jest dzięki temu prostsze i bardziej efektywne, a oszczędności i optymalizacja w zakresie ponoszonych kosztów, mają dla każdego przedsiębiorcy ogromne znaczenie.
Dopasujemy naszą ofertę do Państwa oczekiwań i potrzeb organizacji. Czy moglibyśmy porozmawiać o naszej propozycji?
Pozdrawiam
Krystian Wieczorek
^ 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