* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: kernel test robot @ 2023-06-08 7:14 UTC (permalink / raw)
To: Douglas Anderson, Jiri Kosina, Benjamin Tissoires,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Frank Rowand,
Krzysztof Kozlowski, Conor Dooley, Neil Armstrong, Sam Ravnborg,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann
Cc: llvm, oe-kbuild-all, Douglas Anderson, devicetree,
cros-qcom-dts-watchers, linux-arm-msm, yangcong5, Dmitry Torokhov,
linux-kernel, dri-devel, Chris Morgan, linux-input, hsinyi
In-Reply-To: <20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid>
Hi Douglas,
kernel test robot noticed the following build errors:
[auto build test ERROR on robh/for-next]
[also build test ERROR on hid/for-next dtor-input/next dtor-input/for-linus drm-misc/drm-misc-next linus/master v6.4-rc5 next-20230607]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Douglas-Anderson/dt-bindings-HID-i2c-hid-Add-panel-property-to-i2c-hid-backed-touchscreens/20230608-055515
base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link: https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15%40changeid
patch subject: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
config: i386-randconfig-i003-20230607 (https://download.01.org/0day-ci/archive/20230608/202306081419.Dzz0T4iW-lkp@intel.com/config)
compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add robh https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
git fetch robh for-next
git checkout robh/for-next
b4 shazam https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306081419.Dzz0T4iW-lkp@intel.com/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: drm_panel_add_follower
>>> referenced by i2c-hid-core.c:1159 (drivers/hid/i2c-hid/i2c-hid-core.c:1159)
>>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_probe) in archive vmlinux.a
--
>> ld.lld: error: undefined symbol: drm_panel_remove_follower
>>> referenced by i2c-hid-core.c:1218 (drivers/hid/i2c-hid/i2c-hid-core.c:1218)
>>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_remove) in archive vmlinux.a
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v2 00/10] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Maxime Ripard @ 2023-06-08 7:17 UTC (permalink / raw)
To: Douglas Anderson
Cc: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst,
Thomas Zimmermann, dri-devel, Dmitry Torokhov, linux-input,
Daniel Vetter, linux-kernel, hsinyi, cros-qcom-dts-watchers,
devicetree, yangcong5, linux-arm-msm, Chris Morgan
In-Reply-To: <20230607215224.2067679-1-dianders@chromium.org>
[-- Attachment #1: Type: text/plain, Size: 2137 bytes --]
Hi Douglas,
On Wed, Jun 07, 2023 at 02:49:22PM -0700, Douglas Anderson wrote:
>
> The big motivation for this patch series is mostly described in the patch
> ("drm/panel: Add a way for other devices to follow panel state"), but to
> quickly summarize here: for touchscreens that are connected to a panel we
> need the ability to power sequence the two device together. This is not a
> new need, but so far we've managed to get by through a combination of
> inefficiency, added costs, or perhaps just a little bit of brokenness.
> It's time to do better. This patch series allows us to do better.
>
> Assuming that people think this patch series looks OK, we'll have to
> figure out the right way to land it. The panel patches and i2c-hid
> patches will go through very different trees and so either we'll need
> an Ack from one side or the other or someone to create a tag for the
> other tree to pull in. This will _probably_ require the true drm-misc
> maintainers to get involved, not a lowly committer. ;-)
>
> Version 2 of this patch series doesn't change too much. At a high level:
> * I added all the forgotten "static" to functions.
> * I've hopefully made the bindings better.
> * I've integrated into fw_devlink.
> * I cleaned up a few descriptions / comments.
>
> This still needs someone to say that the idea looks OK or to suggest
> an alternative that solves the problems. ;-)
Thanks for working on this.
I haven't seen in any of your commit messages how the panels were
actually "packaged" together?
Do a panel model typically come together with the i2c-hid support, or is
it added at manufacture time?
If it's the latter, it's indeed a fairly loose connection and we need
your work.
If it's the former though and we don't expect a given panel reference to
always (or never) come with a touchscreen attached, I guess we can have
something much simpler with a bunch of helpers that would register a
i2c-hid device and would be called by the panel driver itself.
And then, since everything is self-contained managing the power state
becomes easier as well.
Maxime
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v4 0/2] Add ili9882t bindings and timing
From: Cong Yang @ 2023-06-08 13:01 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel, Cong Yang
Add bindings for Ilitek. The ili9882t touch screen chip same as
Elan eKTH6915 controller has a reset gpio. The difference is that
ilitek9882 needs to use vccio-supply instead of vcc33-supply.
From Dmitry suggestion, it would make more sense to distinguish the
binging of ili9882 and eKTH6915.
From The datasheet specifies there should be 60ms between touch SDA
sleep and panel RESX. so we can add the 65 ms delay in i2c_hid_core_suspend.
Changes in v4:
- PATCH 1/2: Remove compatible items and add reset maxItems.
- PATCH 1/2: Refer to the panel description in Doug serias[1].
[1] https://lore.kernel.org/all/20230607144931.v2.1.Id68e30343bb1e11470582a9078b086176cfec46b@changeid/
- PATCH 2/2: Set a "null" to ili9882t_chip_data for vcc33-supply, then using dummy regulator.
- Link to v3: https://lore.kernel.org/all/20230607133458.4075667-1-yangcong5@huaqin.corp-partner.google.com/
Changes in v3:
- PATCH 1/2: Introduce bindings for Ilitek.
- Link to v2: https://lore.kernel.org/all/20230605060524.1178164-1-yangcong5@huaqin.corp-partner.google.com/
Changes in v2:
- PATCH 1/2: fix ran make dt_binding_check warnings/errors.
- PATCH 1/2: remove oneOf,just enum.
- Link to v1: https://lore.kernel.org/all/20230602140948.2138668-1-yangcong5@huaqin.corp-partner.google.com/
Cong Yang (2):
dt-bindings: HID: i2c-hid: ilitek: Introduce bindings for Ilitek
ili9882t
HID: i2c-hid: elan: Add ili9882t timing
.../bindings/input/ilitek,ili9882t.yaml | 66 +++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 33 ++++++++--
2 files changed, 92 insertions(+), 7 deletions(-)
create mode 100644 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
--
2.25.1
^ permalink raw reply
* [PATCH v4 1/2] dt-bindings: HID: i2c-hid: ilitek: Introduce bindings for Ilitek ili9882t
From: Cong Yang @ 2023-06-08 13:01 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230608130147.2835818-1-yangcong5@huaqin.corp-partner.google.com>
The ili9882t touch screen chip same as Elan eKTH6915 controller
has a reset gpio. The difference is that ili9882t needs to use
vccio-supply instead of vcc33-supply. Doug's series[1] allows panels
and touchscreens to power on/off together, let's add a phandle for this.
[1]: https://lore.kernel.org/r/20230607215224.2067679-1-dianders@chromium.org
Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
.../bindings/input/ilitek,ili9882t.yaml | 66 +++++++++++++++++++
1 file changed, 66 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
diff --git a/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
new file mode 100644
index 000000000000..89584264d6a2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/ilitek,ili9882t.yaml
@@ -0,0 +1,66 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/ilitek,ili9882t.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ilitek ili9882t touchscreen controller
+
+maintainers:
+ - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+description:
+ Supports the Ilitek ili9882t touchscreen controller.
+ This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
+
+allOf:
+ - $ref: /schemas/input/touchscreen/touchscreen.yaml#
+
+properties:
+ compatible:
+ const: ilitek,ili9882t
+
+ reg:
+ const: 0x41
+
+ interrupts:
+ maxItems: 1
+
+ panel: true
+
+ reset-gpios:
+ maxItems: 1
+ description: Reset GPIO.
+
+
+ vccio-supply:
+ description: The 1.8V supply to the touchscreen.
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - vccio-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen: touchscreen@41 {
+ compatible = "ilitek,ili9882t";
+ reg = <0x41>;
+
+ interrupt-parent = <&pio>;
+ interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
+
+ reset-gpios = <&pio 60 GPIO_ACTIVE_LOW>;
+ vccio-supply = <&mt6366_vio18_reg>;
+ };
+ };
--
2.25.1
^ permalink raw reply related
* [PATCH v4 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Cong Yang @ 2023-06-08 13:01 UTC (permalink / raw)
To: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, dianders, hsinyi
Cc: linux-input, devicetree, linux-kernel, Cong Yang
In-Reply-To: <20230608130147.2835818-1-yangcong5@huaqin.corp-partner.google.com>
The ili9882t is a TDDI IC (Touch with Display Driver). The
datasheet specifies there should be 60ms between touch SDA
sleep and panel RESX. Doug's series[1] allows panels and
touchscreens to power on/off together, so we can add the 65 ms
delay in i2c_hid_core_suspend before panel_unprepare.
Beacuse ilitek9882 touchscrgeen is a panel follower, and
needs to use vccio-supply instead of vcc33-supply, so set
it "null" to ili9882t_chip_data, then using dummy regulator.
[1]: https://lore.kernel.org/r/20230607215224.2067679-1-dianders@chromium.org
Signed-off-by: Cong Yang <yangcong5@huaqin.corp-partner.google.com>
---
drivers/hid/i2c-hid/i2c-hid-of-elan.c | 33 +++++++++++++++++++++------
1 file changed, 26 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/i2c-hid/i2c-hid-of-elan.c b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
index 76ddc8be1cbb..40e6b8ebe8d1 100644
--- a/drivers/hid/i2c-hid/i2c-hid-of-elan.c
+++ b/drivers/hid/i2c-hid/i2c-hid-of-elan.c
@@ -18,9 +18,11 @@
#include "i2c-hid.h"
struct elan_i2c_hid_chip_data {
- unsigned int post_gpio_reset_delay_ms;
+ unsigned int post_gpio_reset_on_delay_ms;
+ unsigned int post_gpio_reset_off_delay_ms;
unsigned int post_power_delay_ms;
u16 hid_descriptor_address;
+ const char *main_supply_name;
};
struct i2c_hid_of_elan {
@@ -52,8 +54,8 @@ static int elan_i2c_hid_power_up(struct i2chid_ops *ops)
msleep(ihid_elan->chip_data->post_power_delay_ms);
gpiod_set_value_cansleep(ihid_elan->reset_gpio, 0);
- if (ihid_elan->chip_data->post_gpio_reset_delay_ms)
- msleep(ihid_elan->chip_data->post_gpio_reset_delay_ms);
+ if (ihid_elan->chip_data->post_gpio_reset_on_delay_ms)
+ msleep(ihid_elan->chip_data->post_gpio_reset_on_delay_ms);
return 0;
}
@@ -64,6 +66,9 @@ static void elan_i2c_hid_power_down(struct i2chid_ops *ops)
container_of(ops, struct i2c_hid_of_elan, ops);
gpiod_set_value_cansleep(ihid_elan->reset_gpio, 1);
+ if (ihid_elan->chip_data->post_gpio_reset_off_delay_ms)
+ msleep(ihid_elan->chip_data->post_gpio_reset_off_delay_ms);
+
regulator_disable(ihid_elan->vccio);
regulator_disable(ihid_elan->vcc33);
}
@@ -89,24 +94,38 @@ static int i2c_hid_of_elan_probe(struct i2c_client *client)
if (IS_ERR(ihid_elan->vccio))
return PTR_ERR(ihid_elan->vccio);
- ihid_elan->vcc33 = devm_regulator_get(&client->dev, "vcc33");
+ ihid_elan->chip_data = device_get_match_data(&client->dev);
+
+ ihid_elan->vcc33 = devm_regulator_get(&client->dev, ihid_elan->chip_data->main_supply_name);
if (IS_ERR(ihid_elan->vcc33))
return PTR_ERR(ihid_elan->vcc33);
- ihid_elan->chip_data = device_get_match_data(&client->dev);
-
return i2c_hid_core_probe(client, &ihid_elan->ops,
ihid_elan->chip_data->hid_descriptor_address, 0);
}
static const struct elan_i2c_hid_chip_data elan_ekth6915_chip_data = {
.post_power_delay_ms = 1,
- .post_gpio_reset_delay_ms = 300,
+ .post_gpio_reset_on_delay_ms = 300,
+ .hid_descriptor_address = 0x0001,
+ .main_supply_name = "vcc33",
+};
+
+static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
+ .post_power_delay_ms = 1,
+ .post_gpio_reset_on_delay_ms = 200,
+ .post_gpio_reset_off_delay_ms = 65,
.hid_descriptor_address = 0x0001,
+ /* this touchscreen is tightly integrated with the panel and assumes
+ * that the relevant power rails (other than the IO rail) have already
+ * been turned on by the panel driver because we're a panel follower.
+ */
+ .main_supply_name = "null",
};
static const struct of_device_id elan_i2c_hid_of_match[] = {
{ .compatible = "elan,ekth6915", .data = &elan_ekth6915_chip_data },
+ { .compatible = "ilitek,ili9882t", .data = &ilitek_ili9882t_chip_data },
{ }
};
MODULE_DEVICE_TABLE(of, elan_i2c_hid_of_match);
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v4 1/2] dt-bindings: HID: i2c-hid: ilitek: Introduce bindings for Ilitek ili9882t
From: Doug Anderson @ 2023-06-08 13:37 UTC (permalink / raw)
To: Cong Yang
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, hsinyi, linux-input, devicetree, linux-kernel
In-Reply-To: <20230608130147.2835818-2-yangcong5@huaqin.corp-partner.google.com>
Hi,
On Thu, Jun 8, 2023 at 6:02 AM Cong Yang
<yangcong5@huaqin.corp-partner.google.com> wrote:
>
> @@ -0,0 +1,66 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/ilitek,ili9882t.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ilitek ili9882t touchscreen controller
> +
> +maintainers:
> + - Dmitry Torokhov <dmitry.torokhov@gmail.com>
Has Dmitry agreed to be the maintainer here? Traditionally when adding
a new file one lists themselves.
> +description:
> + Supports the Ilitek ili9882t touchscreen controller.
> + This touchscreen controller uses the i2c-hid protocol with a reset GPIO.
> +
> +allOf:
> + - $ref: /schemas/input/touchscreen/touchscreen.yaml#
> +
> +properties:
> + compatible:
> + const: ilitek,ili9882t
> +
> + reg:
> + const: 0x41
> +
> + interrupts:
> + maxItems: 1
> +
> + panel: true
> +
> + reset-gpios:
> + maxItems: 1
> + description: Reset GPIO.
> +
> +
> + vccio-supply:
> + description: The 1.8V supply to the touchscreen.
Why two blank lines between reset-gpios and vccio-supply?
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - vccio-supply
I think you want "panel" as a required property too, right?
> +examples:
> + - |
> + #include <dt-bindings/gpio/gpio.h>
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + touchscreen: touchscreen@41 {
> + compatible = "ilitek,ili9882t";
> + reg = <0x41>;
> +
> + interrupt-parent = <&pio>;
> + interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
> +
> + reset-gpios = <&pio 60 GPIO_ACTIVE_LOW>;
> + vccio-supply = <&mt6366_vio18_reg>;
...and "panel" would also need to go in the above example.
^ permalink raw reply
* Re: [PATCH v4 2/2] HID: i2c-hid: elan: Add ili9882t timing
From: Doug Anderson @ 2023-06-08 13:38 UTC (permalink / raw)
To: Cong Yang
Cc: robh+dt, krzysztof.kozlowski+dt, conor+dt, dmitry.torokhov, jikos,
benjamin.tissoires, hsinyi, linux-input, devicetree, linux-kernel
In-Reply-To: <20230608130147.2835818-3-yangcong5@huaqin.corp-partner.google.com>
Hi,
On Thu, Jun 8, 2023 at 6:02 AM Cong Yang
<yangcong5@huaqin.corp-partner.google.com> wrote:
>
> +static const struct elan_i2c_hid_chip_data ilitek_ili9882t_chip_data = {
> + .post_power_delay_ms = 1,
> + .post_gpio_reset_on_delay_ms = 200,
> + .post_gpio_reset_off_delay_ms = 65,
> .hid_descriptor_address = 0x0001,
> + /* this touchscreen is tightly integrated with the panel and assumes
> + * that the relevant power rails (other than the IO rail) have already
> + * been turned on by the panel driver because we're a panel follower.
> + */
I don't think the above is the right block comment style for this
file. The first "/*" should be on a line on its own.
> + .main_supply_name = "null",
This should actually be NULL without the quotes. ...then you should
change the code not to try to grab the regulator in that case.
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: add a module parameter to keep firmware gestures
From: Лежанкин Иван @ 2023-06-08 13:50 UTC (permalink / raw)
To: Jiri Kosina
Cc: Benjamin Tissoires, Nestor Lopez Casado, linux-input,
linux-kernel
In-Reply-To: <alpine.LNX.2.00.1504021432310.843@pobox.suse.cz>
Hello again!
I recently rebooted my laptop just to find out that my touchpad
stopped working: no more gestures, no more button clicks supported,
tap-to-click is very laggy.
Just checked the module history and found commit
https://github.com/torvalds/linux/commit/cae253d6033da885e71c29c1591b22838a52de76
with description about "desktop environments can and
should support touchpad gestures through libinput". Unfortunately my
environment (Fedora 39 KDE) doesn't support it out-of-the-box, and I
don't know how to return all the handful features that my touchpad
provided.
Right now it's slightly better than useless.
Is there a way to return this option back? Or provide any guides on
how to properly setup all the features?
P.S. Sorry for the duplicate - the mail-lists rejected my previous
message because of an accidental HTML subpart.
On Thu, 2 Apr 2015 at 15:32, Jiri Kosina <jkosina@suse.cz> wrote:
>
> On Thu, 26 Mar 2015, Benjamin Tissoires wrote:
>
> > The Logitech T650 used to report 3 fingers swipes to the up as a press on
> > the Super key. When we switched the touchpad to the raw mode, we also
> > disable such firmware gesture and some users may rely on it.
> >
> > Unfortunately, 3 finger swipes are still not supported in most of the
> > Linux environments, which means that we disabled a feature of the touchpad.
> >
> > Allow users to revert the raw reporting mode and keep going with the
> > firmware gestures by providing a new module parameter.
> >
> > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com>
>
> Applied to for-4.1/logitech.
>
> --
> Jiri Kosina
> SUSE Labs
^ permalink raw reply
* Re: [PATCH] selftests: hid: Add touch tests for Wacom devices
From: Jiri Kosina @ 2023-06-08 13:56 UTC (permalink / raw)
To: Joshua Dickens
Cc: linux-input, Benjamin Tissoires, Ping Cheng,
Aaron Armstrong Skomra, Jason Gerecke, Joshua Dickens,
Jason Gerecke
In-Reply-To: <20230511194754.129571-1-Joshua@Joshua-Dickens.com>
On Thu, 11 May 2023, Joshua Dickens wrote:
> Adding a wacom touch device to use the test_multitouch tests.
> Adding a 2 additional tests.
> - A test to check if a touch event is sent when the contact_id of the event is 0.
> - A test to check if a touch event is not sent when confidence is set to 0.
>
> Signed-off-by: Joshua Dickens <joshua.dickens@wacom.com>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
Applied, sorry for the delay.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
From: kernel test robot @ 2023-06-08 13:35 UTC (permalink / raw)
To: Jason Gerecke, linux-input, linux-kernel, Benjamin Tissoires,
Jiri Kosina
Cc: oe-kbuild-all, Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens,
Jason Gerecke
In-Reply-To: <20230607214102.2113-1-jason.gerecke@wacom.com>
Hi Jason,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arc-randconfig-c003-20230608 (https://download.01.org/0day-ci/archive/20230608/202306082127.6r7qAALi-lkp@intel.com/config)
compiler: arc-elf-gcc (GCC) 12.3.0
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke@wacom.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.3.0 ~/bin/make.cross W=1 O=build_dir ARCH=arc SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306082127.6r7qAALi-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "__divdi3" [drivers/hid/wacom.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4] Add rumble support to latest xbox controllers
From: Jiri Kosina @ 2023-06-08 14:10 UTC (permalink / raw)
To: Siarhei Vishniakou
Cc: Benjamin Tissoires, linux-input, linux-kernel, Bastien Nocera
In-Reply-To: <20230425163844.3711710-1-svv@google.com>
On Tue, 25 Apr 2023, Siarhei Vishniakou wrote:
> Currently, rumble is only supported via bluetooth on a single xbox
> controller, called 'model 1708'. On the back of the device, it's named
> 'wireless controller for xbox one'. However, in 2021, Microsoft released
> a firmware update for this controller. As part of this update, the HID
> descriptor of the device changed. The product ID was also changed from
> 0x02fd to 0x0b20. On this controller, rumble was supported via
> hid-microsoft, which matched against the old product id (0x02fd). As a
> result, the firmware update broke rumble support on this controller.
>
> See:
> https://news.xbox.com/en-us/2021/09/08/xbox-controller-firmware-update-rolling-out-to-insiders-starting-today/
>
> The hid-microsoft driver actually supports rumble on the new firmware,
> as well. So simply adding new product id is sufficient to bring back
> this support.
>
> After discussing further with the xbox team, it was pointed out that
> another xbox controller, xbox elite series 2, can be supported in a
> similar way.
>
> Add rumble support for all of these devices in this patch. Two of the
> devices have received firmware updates that caused their product id's to
> change. Both old and new firmware versions of these devices were tested.
>
> The tested controllers are:
>
> 1. 'wireless controller for xbox one', model 1708
> 2. 'xbox wireless controller', model 1914. This is also sometimes
> referred to as 'xbox series S|X'.
> 3. 'elite series 2', model 1797.
>
> The tested configurations are:
> 1. model 1708, pid 0x02fd (old firmware)
> 2. model 1708, pid 0x0b20 (new firmware)
> 3. model 1914, pid 0x0b13
> 4. model 1797, pid 0x0b05 (old firmware)
> 5. model 1797, pid 0x0b22 (new firmware)
>
> I verified rumble support on both bluetooth and usb.
>
> Reviewed-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Siarhei Vishniakou <svv@google.com>
Applied, sorry for the delay.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH RESEND v4 0/2] Fix Goodix touchscreen power leakage for MT8186 boards
From: Jiri Kosina @ 2023-06-08 14:20 UTC (permalink / raw)
To: Fei Shao
Cc: Rob Herring, Matthias Brugger, Jeff LaBundy, Douglas Anderson,
Benjamin Tissoires, Conor Dooley, Dmitry Torokhov,
Krzysztof Kozlowski, Matthias Kaehlcke, Stephen Kitt, devicetree,
linux-input, linux-kernel
In-Reply-To: <20230524114233.RESEND.v4.2.I424e840ae6de3cdbd67394cf4efd24534f6434ba@changeid>
On Wed, 24 May 2023, Fei Shao wrote:
> 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.
Please apologize the delay. Now applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Switch i2c drivers back to use .probe()
From: Jiri Kosina @ 2023-06-08 14:22 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Benjamin Tissoires, Andy Shevchenko, Raul E Rangel,
Dmitry Torokhov, Matthias Kaehlcke, Stephen Kitt,
Douglas Anderson, Hans de Goede, linux-input, kernel
In-Reply-To: <20230525203202.646669-1-u.kleine-koenig@pengutronix.de>
On Thu, 25 May 2023, Uwe Kleine-König wrote:
> After commit b8a1a4cd5a98 ("i2c: Provide a temporary .probe_new()
> call-back type"), all drivers being converted to .probe_new() and then
> 03c835f498b5 ("i2c: Switch .probe() to not take an id parameter") convert
> back to (the new) .probe() to be able to eventually drop .probe_new() from
> struct i2c_driver.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Applied, thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2] HID: logitech-hidpp: Add USB and Bluetooth IDs for the Logitech G915 TKL Keyboard
From: Jiri Kosina @ 2023-06-08 14:32 UTC (permalink / raw)
To: Stuart Hayhurst
Cc: Filipe Laíns, Benjamin Tissoires, linux-input, linux-kernel,
Bastien Nocera
In-Reply-To: <20230530144427.24732-1-stuart.a.hayhurst@gmail.com>
On Tue, 30 May 2023, Stuart Hayhurst wrote:
> From: stuarthayhurst <stuart.a.hayhurst@gmail.com>
>
> Adds the USB and Bluetooth IDs for the Logitech G915 TKL keyboard, for device detection
> For this device, this provides battery reporting on top of hid-generic
>
> Reviewed-by: Bastien Nocera <hadess@hadess.net>
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
Applied. thanks.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
From: kernel test robot @ 2023-06-08 14:26 UTC (permalink / raw)
To: Jason Gerecke, linux-input, linux-kernel, Benjamin Tissoires,
Jiri Kosina
Cc: llvm, oe-kbuild-all, Ping Cheng, Aaron Armstrong Skomra,
Joshua Dickens, Jason Gerecke
In-Reply-To: <20230607214102.2113-1-jason.gerecke@wacom.com>
Hi Jason,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on linus/master v6.4-rc5 next-20230608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Jason-Gerecke/HID-wacom-Use-ktime_t-rather-than-int-when-dealing-with-timestamps/20230608-054255
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke%40wacom.com
patch subject: [PATCH] HID: wacom: Use ktime_t rather than int when dealing with timestamps
config: arm-randconfig-r036-20230608 (https://download.01.org/0day-ci/archive/20230608/202306082249.cMqfu24Y-lkp@intel.com/config)
compiler: clang version 17.0.0 (https://github.com/llvm/llvm-project.git 4a5ac14ee968ff0ad5d2cc1ffa0299048db4c88a)
reproduce (this is a W=1 build):
mkdir -p ~/bin
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm cross compiling tool for clang build
# apt-get install binutils-arm-linux-gnueabi
git remote add hid https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git
git fetch hid for-next
git checkout hid/for-next
b4 shazam https://lore.kernel.org/r/20230607214102.2113-1-jason.gerecke@wacom.com
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202306082249.cMqfu24Y-lkp@intel.com/
All errors (new ones prefixed by >>):
>> ld.lld: error: undefined symbol: __aeabi_ldivmod
>>> referenced by wacom_wac.c
>>> drivers/hid/wacom_wac.o:(wacom_wac_irq) in archive vmlinux.a
>>> did you mean: __aeabi_idivmod
>>> defined in: arch/arm/lib/lib.a(lib1funcs.o)
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH RESEND v4 0/1] HID: nvidia-shield
From: Rahul Rameshbabu @ 2023-06-08 14:34 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/20230418203747.24000-1-rrameshbabu@nvidia.com/
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.5
^ permalink raw reply
* [PATCH RESEND v4 1/1] HID: nvidia-shield: Initial driver implementation with Thunderstrike support
From: Rahul Rameshbabu @ 2023-06-08 14:34 UTC (permalink / raw)
To: Benjamin Tissoires, Jiri Kosina
Cc: linux-input, linux-kernel, Randy Dunlap, Rahul Rameshbabu
In-Reply-To: <20230608143450.12283-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..4182ad3a7b93
--- /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.5
^ permalink raw reply related
* Re: [PATCH v2 00/10] drm/panel and i2c-hid: Allow panels and touchscreens to power sequence together
From: Doug Anderson @ 2023-06-08 14:38 UTC (permalink / raw)
To: Maxime Ripard
Cc: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst,
Thomas Zimmermann, dri-devel, Dmitry Torokhov, linux-input,
Daniel Vetter, linux-kernel, hsinyi, cros-qcom-dts-watchers,
devicetree, yangcong5, linux-arm-msm, Chris Morgan
In-Reply-To: <jehxiy3z4aieop5qgzmlon4u76n7gvt3kc6knxhb5yqkiz3rsp@mx27m75sx43r>
Hi,
On Thu, Jun 8, 2023 at 12:17 AM Maxime Ripard <mripard@kernel.org> wrote:
>
> Hi Douglas,
>
> On Wed, Jun 07, 2023 at 02:49:22PM -0700, Douglas Anderson wrote:
> >
> > The big motivation for this patch series is mostly described in the patch
> > ("drm/panel: Add a way for other devices to follow panel state"), but to
> > quickly summarize here: for touchscreens that are connected to a panel we
> > need the ability to power sequence the two device together. This is not a
> > new need, but so far we've managed to get by through a combination of
> > inefficiency, added costs, or perhaps just a little bit of brokenness.
> > It's time to do better. This patch series allows us to do better.
> >
> > Assuming that people think this patch series looks OK, we'll have to
> > figure out the right way to land it. The panel patches and i2c-hid
> > patches will go through very different trees and so either we'll need
> > an Ack from one side or the other or someone to create a tag for the
> > other tree to pull in. This will _probably_ require the true drm-misc
> > maintainers to get involved, not a lowly committer. ;-)
> >
> > Version 2 of this patch series doesn't change too much. At a high level:
> > * I added all the forgotten "static" to functions.
> > * I've hopefully made the bindings better.
> > * I've integrated into fw_devlink.
> > * I cleaned up a few descriptions / comments.
> >
> > This still needs someone to say that the idea looks OK or to suggest
> > an alternative that solves the problems. ;-)
>
> Thanks for working on this.
>
> I haven't seen in any of your commit messages how the panels were
> actually "packaged" together?
>
> Do a panel model typically come together with the i2c-hid support, or is
> it added at manufacture time?
>
> If it's the latter, it's indeed a fairly loose connection and we need
> your work.
>
> If it's the former though and we don't expect a given panel reference to
> always (or never) come with a touchscreen attached,
Thanks for your reply. Let me see what I can do to bring clarity.
In at least some of the cases, I believe that the panel and the
touchscreen _are_ logically distinct components, even if they've been
glued together at some stage in manufacturing. Even on one of the
"poster child" boards that I talk about in patch #3, the early
versions of "homestar", I believe this to be the case. However, even
if the panel and touchscreen are separate components then they still
could be connected to the main board in a way that they share power
and/or reset signals. In my experience, in every case where they do
the EEs expect that the panel is power sequenced first and then the
touchscreen is power sequenced second. The EEs look at the power
sequencing requirements of the panel and touchscreen, see that there
is a valid power sequence protocol where they can share rails, and
design the board that way. Even if the touchscreen and panel are
logically separate, the moment the board designers hook them up to the
same power rails and/or reset signals they become tied. This is well
supported by my patch series.
The case that really motivated my patch series, though, is the case
that Cong Yang recently has been working on. I think most of the
discussion is in his original patch series [1]. Cong Yang's patch
series is largely focused on supporting the "ILI9882T" chip and some
panels that it's used with. I found a datasheet for that, and the
title from the first page is illustrative: "In-cell IC Integrates TFT
LCD Driver and Capacitive Touch Controller into a Two Chip Cascade".
This is an integrated solution that's designed to handle both the LCD
and the touchscreen.
[1] https://lore.kernel.org/lkml/20230519032316.3464732-1-yangcong5@huaqin.corp-partner.google.com/
> I guess we can have
> something much simpler with a bunch of helpers that would register a
> i2c-hid device and would be called by the panel driver itself.
>
> And then, since everything is self-contained managing the power state
> becomes easier as well.
Can you give me more details about how you think this would work?
When you say that the panel would register an i2c-hid device itself,
do you mean that we'd do something like give a phandle to the i2c bus
to the panel and then the panel would manually instantiate the i2c-hid
device on it? ...and I guess it would need to be a "subclass" of
i2c-hid that knew about the connection to the panel code? This
subclass and the panel code would communicate with each other about
power sequencing needs through some private API (like MFD devices
usually do?). Assuming I'm understanding correctly, I think that could
work. Is it cleaner than my current approach, though?
I guess, alternatively, we could put the "panel" directly under the
i2c bus in this case. That would probably work for Cong Yang's current
needs, but we'd end up in trouble if we ever had a similar situation
with an eDP panel since eDP panels need to be under the DP-AUX bus.
I guess overall, though, while I think this approach could solve Cong
Yang's needs, I still feel like it's worth solving the case where
board designers have made panel and touchscreens "coupled" by having
them rely on the same power rails and/or reset signals.
-Doug
^ permalink raw reply
* Re: [PATCH v1 27/43] dt-bindings: input: Add Cirrus EP93xx keypad
From: Rob Herring @ 2023-06-08 15:01 UTC (permalink / raw)
To: Nikita Shubin
Cc: Alexander Sverdlin, Arnd Bergmann, Linus Walleij, Dmitry Torokhov,
Krzysztof Kozlowski, Michael Peters, Kris Bahnsen, linux-input,
devicetree, linux-kernel
In-Reply-To: <20230601054549.10843-9-nikita.shubin@maquefel.me>
On Thu, Jun 01, 2023 at 08:45:32AM +0300, Nikita Shubin wrote:
> Add YAML bindings for ep93xx SoC keypad.
>
> Signed-off-by: Nikita Shubin <nikita.shubin@maquefel.me>
> ---
>
> Notes:
> v0 -> v1:
>
> - remove almost all but debounce-delay-ms and prescale
> - s/ep9301-keypad/ep9307-keypad/ it's actually only for
> ep9307, ep9312, ep9315
>
> Krzysztof Kozlowski:
> - renamed file
> - changed maintainers
> - dropped quotes
> - dropped clock-names
> - use fallback compatible and list all possible compatibles
> - fix ident
>
> .../bindings/input/cirrus,ep9307-keypad.yaml | 86 +++++++++++++++++++
> 1 file changed, 86 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
>
> diff --git a/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> new file mode 100644
> index 000000000000..c7eb10a84a6b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/cirrus,ep9307-keypad.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/cirrus,ep9307-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Cirrus ep93xx keypad
> +
> +maintainers:
> + - Alexander Sverdlin <alexander.sverdlin@gmail.com>
> +
> +allOf:
> + - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +description: |
Don't need '|'.
> + The KPP is designed to interface with a keypad matrix with 2-point contact
> + or 3-point contact keys. The KPP is designed to simplify the software task
> + of scanning a keypad matrix. The KPP is capable of detecting, debouncing,
> + and decoding one or multiple keys pressed simultaneously on a keypad.
> +
> +properties:
> + compatible:
> + oneOf:
> + - const: cirrus,ep9307-keypad
> + - items:
> + - enum:
> + - cirrus,ep9312-keypad
> + - cirrus,ep9315-keypad
> + - const: cirrus,ep9307-keypad
> +
> + reg:
> + maxItems: 1
> +
> + interrupts:
> + maxItems: 1
> +
> + clocks:
> + maxItems: 1
> +
> + cirrus,debounce-delay-ms:
Use the somewhat standard 'debounce-delay-ms'.
> + description: |
> + Time in microseconds that key must be pressed or
> + released for state change interrupt to trigger.
> +
> + cirrus,prescale:
> + description: row/column counter pre-scaler load value
> + $ref: /schemas/types.yaml#/definitions/uint32
Constraints?
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
> + - clocks
> + - linux,keymap
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/cirrus,ep93xx-clock.h>
> + #include <dt-bindings/input/input.h>
> + keypad@800f0000 {
> + compatible = "cirrus,ep9301-keypad";
> + reg = <0x800f0000 0x0c>;
> + interrupt-parent = <&vic0>;
> + interrupts = <29>;
> + clocks = <&syscon EP93XX_CLK_KEYPAD>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&keypad_default_pins>;
> + linux,keymap = <KEY_UP>,
> + <KEY_DOWN>,
> + <KEY_VOLUMEDOWN>,
> + <KEY_HOME>,
> + <KEY_RIGHT>,
> + <KEY_LEFT>,
> + <KEY_ENTER>,
> + <KEY_VOLUMEUP>,
> + <KEY_F6>,
> + <KEY_F8>,
> + <KEY_F9>,
> + <KEY_F10>,
> + <KEY_F1>,
> + <KEY_F2>,
> + <KEY_F3>,
> + <KEY_POWER>;
> + };
> --
> 2.37.4
>
^ permalink raw reply
* Re: [PATCH] HID: fix an error code in hid_check_device_match()
From: Jiri Kosina @ 2023-06-08 15:01 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dmitry Torokhov, Benjamin Tissoires, linux-input, kernel-janitors
In-Reply-To: <ZH7sZv4PEovkMxNP@moroto>
On Tue, 6 Jun 2023, Dan Carpenter wrote:
> The hid_check_device_match() returns true if we find a match and false
> if we don't. But here it returns -ENODEV which becomes true instead
> of false.
>
> Fixes: 207733f92661 ("HID: split apart hid_device_probe to make logic more apparent")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/hid/hid-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index 4b7062dcefec..8992e3c1e769 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -2593,7 +2593,7 @@ static bool hid_check_device_match(struct hid_device *hdev,
> {
> *id = hid_match_device(hdev, hdrv);
> if (!*id)
> - return -ENODEV;
> + return false;
>
> if (hdrv->match)
> return hdrv->match(hdev, hid_ignore_special_drivers);
Thanks for checking this! Applied.
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: Doug Anderson @ 2023-06-08 15:10 UTC (permalink / raw)
To: kernel test robot
Cc: Jiri Kosina, Benjamin Tissoires, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Frank Rowand, Krzysztof Kozlowski, Conor Dooley,
Neil Armstrong, Sam Ravnborg, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, llvm, oe-kbuild-all, devicetree,
cros-qcom-dts-watchers, linux-arm-msm, yangcong5, Dmitry Torokhov,
linux-kernel, dri-devel, Chris Morgan, linux-input, hsinyi
In-Reply-To: <202306081419.Dzz0T4iW-lkp@intel.com>
Hi,
On Thu, Jun 8, 2023 at 12:15 AM kernel test robot <lkp@intel.com> wrote:
>
> Hi Douglas,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on robh/for-next]
> [also build test ERROR on hid/for-next dtor-input/next dtor-input/for-linus drm-misc/drm-misc-next linus/master v6.4-rc5 next-20230607]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Douglas-Anderson/dt-bindings-HID-i2c-hid-Add-panel-property-to-i2c-hid-backed-touchscreens/20230608-055515
> base: https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
> patch link: https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15%40changeid
> patch subject: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
> config: i386-randconfig-i003-20230607 (https://download.01.org/0day-ci/archive/20230608/202306081419.Dzz0T4iW-lkp@intel.com/config)
> compiler: clang version 15.0.7 (https://github.com/llvm/llvm-project.git 8dfdcc7b7bf66834a761bd8de445840ef68e4d1a)
> reproduce (this is a W=1 build):
> mkdir -p ~/bin
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> git remote add robh https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git
> git fetch robh for-next
> git checkout robh/for-next
> b4 shazam https://lore.kernel.org/r/20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid
> # save the config file
> mkdir build_dir && cp config build_dir/.config
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 olddefconfig
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang ~/bin/make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202306081419.Dzz0T4iW-lkp@intel.com/
>
> All errors (new ones prefixed by >>):
>
> >> ld.lld: error: undefined symbol: drm_panel_add_follower
> >>> referenced by i2c-hid-core.c:1159 (drivers/hid/i2c-hid/i2c-hid-core.c:1159)
> >>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_probe) in archive vmlinux.a
> --
> >> ld.lld: error: undefined symbol: drm_panel_remove_follower
> >>> referenced by i2c-hid-core.c:1218 (drivers/hid/i2c-hid/i2c-hid-core.c:1218)
> >>> drivers/hid/i2c-hid/i2c-hid-core.o:(i2c_hid_core_remove) in archive vmlinux.a
Thanks for the report! Ugh, I guess I forgot that even though
DRM_PANEL is bool, it gets bundled up into all of DRM which can be a
module. Assuming that this series looks mostly the same in the next
version, I'll plan to add this:
depends on DRM || !DRM # if DRM=m, this can't be 'y'
...to each of the i2c-hid subclasses.
-Doug
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: Benjamin Tissoires @ 2023-06-08 15:36 UTC (permalink / raw)
To: Douglas Anderson
Cc: Jiri Kosina, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Frank Rowand, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan
In-Reply-To: <20230607144931.v2.8.Ib1a98309c455cd7e26b931c69993d4fba33bbe15@changeid>
On Jun 07 2023, Douglas Anderson wrote:
>
> As talked about in the patch ("drm/panel: Add a way for other devices
> to follow panel state"), we really want to keep the power states of a
> touchscreen and the panel it's attached to in sync with each other. In
> that spirit, add support to i2c-hid to be a panel follower. This will
> let the i2c-hid driver get informed when the panel is powered on and
> off. From there we can match the i2c-hid device's power state to that
> of the panel.
>
> NOTE: this patch specifically _doesn't_ use pm_runtime to keep track
> of / manage the power state of the i2c-hid device, even though my
> first instinct said that would be the way to go. Specific problems
> with using pm_runtime():
> * The initial power up couldn't happen in a runtime resume function
> since it create sub-devices and, apparently, that's not good to do
> in your resume function.
> * Managing our power state with pm_runtime meant fighting to make the
> right thing happen at system suspend to prevent the system from
> trying to resume us only to suspend us again. While this might be
> able to be solved, it added complexity.
> Overall the code without pm_runtime() ended up being smaller and
> easier to understand.
Generally speaking, I'm not that happy when we need to coordinate with
other subsystems for bringing up resources...
Anyway, a remark inlined (at least):
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> Changes in v2:
> - i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
>
> drivers/hid/i2c-hid/i2c-hid-core.c | 82 +++++++++++++++++++++++++++++-
> 1 file changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> index fa8a1ca43d7f..368db3ae612f 100644
> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> @@ -38,6 +38,8 @@
> #include <linux/mutex.h>
> #include <asm/unaligned.h>
>
> +#include <drm/drm_panel.h>
> +
> #include "../hid-ids.h"
> #include "i2c-hid.h"
>
> @@ -107,6 +109,8 @@ struct i2c_hid {
> struct mutex reset_lock;
>
> struct i2chid_ops *ops;
> + struct drm_panel_follower panel_follower;
> + bool is_panel_follower;
> };
>
> static const struct i2c_hid_quirks {
> @@ -1058,6 +1062,34 @@ static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> return ret;
> }
>
> +static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
> +{
> + struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
> + struct hid_device *hid = ihid->hid;
> +
> + /*
> + * hid->version is set on the first power up. If it's still zero then
> + * this is the first power on so we should perform initial power up
> + * steps.
> + */
> + if (!hid->version)
> + return i2c_hid_core_initial_power_up(ihid);
> +
> + return i2c_hid_core_resume(ihid);
> +}
> +
> +static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
> +{
> + struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
> +
> + return i2c_hid_core_suspend(ihid);
> +}
> +
> +static const struct drm_panel_follower_funcs i2c_hid_core_panel_follower_funcs = {
> + .panel_prepared = i2c_hid_core_panel_prepared,
> + .panel_unpreparing = i2c_hid_core_panel_unpreparing,
> +};
Can we make that above block at least behind a Kconfig?
i2c-hid is often used for touchpads, and the notion of drm panel has
nothing to do with them. So I'd be more confident if we could disable
that code if not required.
Actually, I'd be even more happier if it were in a different compilation
unit. Not necessary a different module, but at least a different file.
Cheers,
Benjamin
> +
> int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> u16 hid_descriptor_address, u32 quirks)
> {
> @@ -1119,6 +1151,41 @@ int i2c_hid_core_probe(struct i2c_client *client, struct i2chid_ops *ops,
> hid->bus = BUS_I2C;
> hid->initial_quirks = quirks;
>
> + /*
> + * See if we're following a panel. If drm_panel_add_follower()
> + * returns no error then we are.
> + */
> + ihid->panel_follower.funcs = &i2c_hid_core_panel_follower_funcs;
> + ret = drm_panel_add_follower(&client->dev, &ihid->panel_follower);
> + if (!ret) {
> + /* We're a follower. That means we'll power things up later. */
> + ihid->is_panel_follower = true;
> +
> + /*
> + * If we're not in control of our own power up/power down then
> + * we can't do the logic to manage wakeups. Give a warning if
> + * a user thought that was possible then force the capability
> + * off.
> + */
> + if (device_can_wakeup(&client->dev)) {
> + dev_warn(&client->dev, "Can't wakeup if following panel\n");
> + device_set_wakeup_capable(&client->dev, false);
> + }
> +
> + return 0;
> + }
> +
> + /*
> + * -ENODEV means that we're not following a panel, so any other error
> + * is a real problem (like -EPROBE_DEFER, -ENOMEM, ...).
> + */
> + if (ret != -ENODEV)
> + goto err_mem_free;
> +
> + /*
> + * We're not following a panel. That's fine and means that we
> + * can power up right away.
> + */
> ret = i2c_hid_core_initial_power_up(ihid);
> if (ret)
> goto err_mem_free;
> @@ -1143,7 +1210,14 @@ void i2c_hid_core_remove(struct i2c_client *client)
> struct i2c_hid *ihid = i2c_get_clientdata(client);
> struct hid_device *hid;
>
> - i2c_hid_core_power_down(ihid);
> + /*
> + * If we're a follower, the act of unfollowing will cause us to be
> + * powered down. Otherwise we need to manually do it.
> + */
> + if (ihid->is_panel_follower)
> + drm_panel_remove_follower(&ihid->panel_follower);
> + else
> + i2c_hid_core_power_down(ihid);
>
> hid = ihid->hid;
> hid_destroy_device(hid);
> @@ -1171,6 +1245,9 @@ static int i2c_hid_core_pm_suspend(struct device *dev)
> struct i2c_client *client = to_i2c_client(dev);
> struct i2c_hid *ihid = i2c_get_clientdata(client);
>
> + if (ihid->is_panel_follower)
> + return 0;
> +
> return i2c_hid_core_suspend(ihid);
> }
>
> @@ -1179,6 +1256,9 @@ static int i2c_hid_core_pm_resume(struct device *dev)
> struct i2c_client *client = to_i2c_client(dev);
> struct i2c_hid *ihid = i2c_get_clientdata(client);
>
> + if (ihid->is_panel_follower)
> + return 0;
> +
> return i2c_hid_core_resume(ihid);
> }
>
> --
> 2.41.0.162.gfafddb0af9-goog
>
^ permalink raw reply
* Re: [PATCH v2 08/10] HID: i2c-hid: Support being a panel follower
From: Doug Anderson @ 2023-06-08 16:42 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Jiri Kosina, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Frank Rowand, Krzysztof Kozlowski, Conor Dooley, Neil Armstrong,
Sam Ravnborg, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann,
dri-devel, Dmitry Torokhov, linux-input, Daniel Vetter,
linux-kernel, hsinyi, cros-qcom-dts-watchers, devicetree,
yangcong5, linux-arm-msm, Chris Morgan
In-Reply-To: <y3l4x3kv7jgog3miexati5wbveaynnryzqvj6sc4ul6625f2if@w7nqgojfavfw>
Hi,
On Thu, Jun 8, 2023 at 8:37 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
>
> On Jun 07 2023, Douglas Anderson wrote:
> >
> > As talked about in the patch ("drm/panel: Add a way for other devices
> > to follow panel state"), we really want to keep the power states of a
> > touchscreen and the panel it's attached to in sync with each other. In
> > that spirit, add support to i2c-hid to be a panel follower. This will
> > let the i2c-hid driver get informed when the panel is powered on and
> > off. From there we can match the i2c-hid device's power state to that
> > of the panel.
> >
> > NOTE: this patch specifically _doesn't_ use pm_runtime to keep track
> > of / manage the power state of the i2c-hid device, even though my
> > first instinct said that would be the way to go. Specific problems
> > with using pm_runtime():
> > * The initial power up couldn't happen in a runtime resume function
> > since it create sub-devices and, apparently, that's not good to do
> > in your resume function.
> > * Managing our power state with pm_runtime meant fighting to make the
> > right thing happen at system suspend to prevent the system from
> > trying to resume us only to suspend us again. While this might be
> > able to be solved, it added complexity.
> > Overall the code without pm_runtime() ended up being smaller and
> > easier to understand.
>
> Generally speaking, I'm not that happy when we need to coordinate with
> other subsystems for bringing up resources...
Yeah, I'd agree that it's not amazingly elegant. Unfortunately, I
couldn't find any existing clean frameworks that would do what was
needed, which is (presumably) why this problem hasn't been solved
before. I could try to come up with a grand abstraction / new
framework, but that doesn't seem like a great choice either unless we
expect more users...
> Anyway, a remark inlined (at least):
>
> >
> > Signed-off-by: Douglas Anderson <dianders@chromium.org>
> > ---
> >
> > Changes in v2:
> > - i2c_hid_core_panel_prepared() and ..._unpreparing() are now static.
> >
> > drivers/hid/i2c-hid/i2c-hid-core.c | 82 +++++++++++++++++++++++++++++-
> > 1 file changed, 81 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
> > index fa8a1ca43d7f..368db3ae612f 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid-core.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
> > @@ -38,6 +38,8 @@
> > #include <linux/mutex.h>
> > #include <asm/unaligned.h>
> >
> > +#include <drm/drm_panel.h>
> > +
> > #include "../hid-ids.h"
> > #include "i2c-hid.h"
> >
> > @@ -107,6 +109,8 @@ struct i2c_hid {
> > struct mutex reset_lock;
> >
> > struct i2chid_ops *ops;
> > + struct drm_panel_follower panel_follower;
> > + bool is_panel_follower;
> > };
> >
> > static const struct i2c_hid_quirks {
> > @@ -1058,6 +1062,34 @@ static int i2c_hid_core_initial_power_up(struct i2c_hid *ihid)
> > return ret;
> > }
> >
> > +static int i2c_hid_core_panel_prepared(struct drm_panel_follower *follower)
> > +{
> > + struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
> > + struct hid_device *hid = ihid->hid;
> > +
> > + /*
> > + * hid->version is set on the first power up. If it's still zero then
> > + * this is the first power on so we should perform initial power up
> > + * steps.
> > + */
> > + if (!hid->version)
> > + return i2c_hid_core_initial_power_up(ihid);
> > +
> > + return i2c_hid_core_resume(ihid);
> > +}
> > +
> > +static int i2c_hid_core_panel_unpreparing(struct drm_panel_follower *follower)
> > +{
> > + struct i2c_hid *ihid = container_of(follower, struct i2c_hid, panel_follower);
> > +
> > + return i2c_hid_core_suspend(ihid);
> > +}
> > +
> > +static const struct drm_panel_follower_funcs i2c_hid_core_panel_follower_funcs = {
> > + .panel_prepared = i2c_hid_core_panel_prepared,
> > + .panel_unpreparing = i2c_hid_core_panel_unpreparing,
> > +};
>
> Can we make that above block at least behind a Kconfig?
>
> i2c-hid is often used for touchpads, and the notion of drm panel has
> nothing to do with them. So I'd be more confident if we could disable
> that code if not required.
Happy to put it behind a Kconfig. I'll plan on that for v3. I'll stub
the functions out if there is no Kconfig, but plan to still leave
structure members just to avoid uglifying the sources too much.
> Actually, I'd be even more happier if it were in a different compilation
> unit. Not necessary a different module, but at least a different file.
I suspect that it's not worth it, but I'll do this if you feel
strongly about it.
I guess the simplest way I can think of to move this to its own file
would be to put the whole private data structure (struct i2c_hid) in a
private header file and then add prototypes for i2c_hid_core_resume()
and i2c_hid_core_suspend() there. Then I could add something like
i2c_hid_core_handle_panel_follower() that would have all the
registration logic. I'd still need special cases in the core
suspend/resume/remove code unless I add a level of abstraction. While
the level of abstraction is more "pure", it also would make the code
harder to follow.
Unless I hear a strong opinion (or if this series changes
significantly), I'll plan to keep things in the same file and just use
a Kconfig.
^ permalink raw reply
* [PATCH v3] Input: synaptics-rmi4 - retry reading SMBus version on resume
From: Jeffery Miller @ 2023-06-08 21:04 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Jonathan Denose, jdenose, Lyude Paul, benjamin.tissoires,
Andrew Duggan, loic.poulain, Andrew Duggan, Jeffery Miller,
Javier Martinez Canillas, Jeremy Kerr, Jonathan Cameron,
Uwe Kleine-König, linux-input, linux-kernel
On resume there can be a period of time after the
preceding serio_resume -> psmouse_deactivate call
where calls to rmi_smb_get_version fail with
-ENXIO.
The call path in rmi_smb_resume is rmi_smb_resume -> rmi_smb_reset ->
rmi_smb_enable_smbus_mode -> rmi_smb_get_version where
this failure would occur.
Add a 30ms delay and retry in the ENXIO case to ensure the following
rmi_driver_resume calls in rmi_smbus_resume can succeed.
This behavior was seen on a Lenovo T440p machine that required
a delay of approximately 7-12ms.
The 30ms delay was chosen based on the linux-input message:
Link: https://lore.kernel.org/all/BYAPR03MB47572F2C65E52ED673238D41B2439@BYAPR03MB4757.namprd03.prod.outlook.com/
With this patch the trimmed resume logs look similar to:
```
psmouse serio1: PM: calling serio_resume+0x0/0x8c @ 5399, parent: i8042
[5399] libps2:__ps2_command:316: psmouse serio1: f5 [] - 0/00000000 []
psmouse serio1: PM: serio_resume+0x0/0x8c returned 0 after 3259 usecs
...
rmi4_smbus 0-002c: PM: calling rmi_smb_resume ... @ 5454, parent: i2c-0
...
[5454] i2c_i801:i801_check_post:414: i801_smbus 0000:00:1f.3: No response
smbus_result: i2c-0 a=02c f=0000 c=fd BYTE_DATA rd res=-6
rmi4_smbus 0-002c: failed to get SMBus version number!
rmi4_smbus 0-002c: sleeping to retry getting the SMBus version number
...
rmi4_smbus 0-002c: PM: rmi_smb_resume ... returned 0 after 41351 usecs
```
Signed-off-by: Jeffery Miller <jefferymiller@google.com>
---
Early boot dmesg include:
```
rmi4_smbus 0-002c: registering SMbus-connected sensor
rmi4_f01 rmi4-00.fn01: found RMI device, manufacturer: Synaptics, product: TM2722-001, fw id: 0
```
The resume order looks correct. The `psmouse serio1` resume returns
before the rmi_smb_resume is called showing the patch from
https://lore.kernel.org/all/89456fcd-a113-4c82-4b10-a9bcaefac68f@google.com/
is applied and working for that ordering.
I attempted to try to rule out some interaction between the concurrent
input resume calls for other i8042 devices.
Adding a 7ms delay after psmouse_deactivate which is called in the
preceding psmouse serio1 serio_resume function also allows
this version call to succeed.
If the rmi_smb_probe device_disable_async_suspend patch is applied
it will also avoid this issue. However the time between
the psmouse_deactivate call for serio_resume and rmi_smb_resume
was over 60ms on my test machine. This would naturally be long
enough to avoid this particular delay.
Changes in v3:
- Tagged mail message Link to resolve checkpatch warning.
Changes in v2:
- Changed to a single retry of 30ms based on previous feedback.
drivers/input/rmi4/rmi_smbus.c | 22 +++++++++++++++++-----
1 file changed, 17 insertions(+), 5 deletions(-)
diff --git a/drivers/input/rmi4/rmi_smbus.c b/drivers/input/rmi4/rmi_smbus.c
index 4bf0e1df6a4a..b6d90c92e8a2 100644
--- a/drivers/input/rmi4/rmi_smbus.c
+++ b/drivers/input/rmi4/rmi_smbus.c
@@ -45,13 +45,25 @@ static int rmi_smb_get_version(struct rmi_smb_xport *rmi_smb)
int retval;
/* Check if for SMBus new version device by reading version byte. */
- retval = i2c_smbus_read_byte_data(client, SMB_PROTOCOL_VERSION_ADDRESS);
- if (retval < 0) {
+ for (int i = 0; i < 2; i++) {
+ if (i > 0) {
+ dev_warn(&client->dev, "sleeping to retry getting the SMBus version number\n");
+ fsleep(30000);
+ }
+ retval = i2c_smbus_read_byte_data(client,
+ SMB_PROTOCOL_VERSION_ADDRESS);
+ if (retval >= 0)
+ return retval + 1;
+
dev_err(&client->dev, "failed to get SMBus version number!\n");
- return retval;
+ /* There can be a delay on resume where the read returns
+ * -ENXIO. Retry to allow additional time for the read
+ * to become responsive.
+ */
+ if (retval != -ENXIO)
+ break;
}
-
- return retval + 1;
+ return retval;
}
/* SMB block write - wrapper over ic2_smb_write_block */
--
2.41.0.162.gfafddb0af9-goog
^ permalink raw reply related
* [PATCH v2] HID: wacom: Use ktime_t rather than int when dealing with timestamps
From: Jason Gerecke @ 2023-06-08 21:38 UTC (permalink / raw)
To: linux-input, linux-kernel, Benjamin Tissoires, Jiri Kosina
Cc: Ping Cheng, Aaron Armstrong Skomra, Joshua Dickens,
Peter Hutterer, Jason Gerecke, stable
In-Reply-To: <20230607214102.2113-1-jason.gerecke@wacom.com>
Code which interacts with timestamps needs to use the ktime_t type
returned by functions like ktime_get. The int type does not offer
enough space to store these values, and attempting to use it is a
recipe for problems. In this particular case, overflows would occur
when calculating/storing timestamps leading to incorrect values being
reported to userspace. In some cases these bad timestamps cause input
handling in userspace to appear hung.
Link: https://gitlab.freedesktop.org/libinput/libinput/-/issues/901
Fixes: 17d793f3ed53 ("HID: wacom: insert timestamp to packed Bluetooth (BT) events")
CC: stable@vger.kernel.org
Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
v2: Use div_u64 to perform division to deal with ARC and ARM architectures
(as found by the kernel test robot)
drivers/hid/wacom_wac.c | 6 +++---
drivers/hid/wacom_wac.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 2ccf838371343..174bf03908d7c 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1314,7 +1314,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
struct input_dev *pen_input = wacom->pen_input;
unsigned char *data = wacom->data;
int number_of_valid_frames = 0;
- int time_interval = 15000000;
+ ktime_t time_interval = 15000000;
ktime_t time_packet_received = ktime_get();
int i;
@@ -1348,7 +1348,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
if (number_of_valid_frames) {
if (wacom->hid_data.time_delayed)
time_interval = ktime_get() - wacom->hid_data.time_delayed;
- time_interval /= number_of_valid_frames;
+ time_interval = div_u64(time_interval, number_of_valid_frames);
wacom->hid_data.time_delayed = time_packet_received;
}
@@ -1359,7 +1359,7 @@ static void wacom_intuos_pro2_bt_pen(struct wacom_wac *wacom)
bool range = frame[0] & 0x20;
bool invert = frame[0] & 0x10;
int frames_number_reversed = number_of_valid_frames - i - 1;
- int event_timestamp = time_packet_received - frames_number_reversed * time_interval;
+ ktime_t event_timestamp = time_packet_received - frames_number_reversed * time_interval;
if (!valid)
continue;
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 1a40bb8c5810c..ee21bb260f22f 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -324,7 +324,7 @@ struct hid_data {
int ps_connected;
bool pad_input_event_flag;
unsigned short sequence_number;
- int time_delayed;
+ ktime_t time_delayed;
};
struct wacom_remote_data {
--
2.41.0
^ permalink raw reply related
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