* Re: [PATCH v3 1/4] Input: ts-overlay - Add touchscreen overlay object handling
From: Javier Carrasco @ 2023-06-29 7:53 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch, linux-kernel,
linux-input, devicetree
In-Reply-To: <ZJz6jnt9hDjuU3du@nixie71>
Hi Jeff,
On 29.06.23 05:29, Jeff LaBundy wrote:
> Hi Javier,
>
> On Wed, Jun 28, 2023 at 08:44:51AM +0200, Javier Carrasco wrote:
>
> [...]
>
>>>>>> +static const char *const ts_overlay_names[] = {
>>>>>> + [TOUCHSCREEN] = "overlay-touchscreen",
>>>>>
>>>>> I'm a little confused why we need new code for this particular function; it's
>>>>> what touchscreen-min-x/y and touchscreen-size-x/y were meant to define. Why
>>>>> can't we keep using those?
>>>>>
>>>> According to the bindings, touchscreen-min-x/y define the minimum
>>>> reported values, but the overlay-touchscreen is actually setting a new
>>>> origin. Therefore I might be misusing those properties. On the other
>>>> hand touchscreen-size-x/y would make more sense, but I also considered
>>>> the case where someone would like to describe the real size of the
>>>> touchscreen outside of the overlay node as well as the clipped size
>>>> inside the node. In that case using the same property twice would be
>>>> confusing.
>>>> So in the end I thought that the origin/size properties are more precise
>>>> and applicable for all objects and not only the overlay touchscreen.
>>>> These properties are needed for the buttons anyways and in the future
>>>> more overlay would use the same properties.
>>>
>>> Ah, I understand now. touchscreen-min-x/y define the lower limits of the axes
>>> reported to input but they don't move the origin. I'm aligned with the reason
>>> to introduce this function.
>>>
>>> This does beg the question as to whether we need two separate types of children
>>> and related parsing code. Can we not simply have one overlay definition, and
>>> make the decision as to whether we are dealing with a border or virtual button
>>> based on whether 'linux,code' is present?
>>>
>> A single overlay definition would be possible, but in case more objects
>> are added in the future, looking for single properties and then deciding
>> what object it is might get messy pretty fast. You could end up needing
>> a decision tree and the definition in the DT would get more complex.
>>
>> Now the decision tree is straightforward (linux,code -> button), but
>> that might not always be the case. In the current implementation there
>> are well-defined objects and adding a new one will never affect the
>> parsing of the rest.
>> Therefore I would like to keep it more readable and easily extendable.
>
> As a potential customer of this feature, I'm ultimately looking to describe
> the hardware as succinctly as possible. Currently we have two overlay types,
> a border and button(s). The former will never have linux,code defined, while
> the latter will. From my naive perspective, it seems redundant to define the
> overlay types differently when their properties imply the difference already.
>
> Ultimately it seems we are simply dealing with generic "segments" scattered
> throughout a larger touch surface. These segments start to look something
> like the following:
>
> struct touch_segment {
> unsigned int x_origin;
> unsigned int y_origin;
> unsigned int x_size;
> unsigned int y_size;
> unsigned int code;
> };
>
> You then have one exported function akin to touchscreen_parse_properties() that
> simply walks the parent device looking for children named "touch-segment-0",
> "touch-segment-1", etc. and parses the five properties, with the fifth (keycode)
> being optional.
>
> And then, you have one last exported function akin to touchscreen_report_pos()
> that processes the touch coordinates. If the coordinates are in a given segment
> and segment->code == KEY_RESERVED (i.e. linux,code was never given), then this
> function simply passes the shifted coordinates to touchscreen_report_pos().
>
> If however segment->code != KEY_RESERVED, it calls input_report_key() based on
> whether the coordinates are within the segment. If this simplified solution
> shrinks the code enough, it may even make sense to keep it in touchscreen.c
> which this new feature is so tightly coupled to anyway.
>
> I'm sure the devil is in the details however, and I understand the value in
> future-proofing. Can you help me understand a potential future case where this
> simplified view would break, and the existing definitions would be better?
>
> Kind regards,
> Jeff LaBundy
I agree that your approach would reduce the code and then moving this
feature to touchscreen.c would be reasonable. So if in the end that is
the desired solution, I will go for it. But there are some points where
I think the bit of extra code would be worth it.
From a DT perspective, I can imagine some scenarios where a bunch of
segments scattered around would be messy. An example would be a keypad
with let's say N=9 buttons. It could be described easily with a buttons
node and the keys inside. Understanding what the node describes would be
straightforward as well, let alone N being much bigger.
You could argue that the buttons node could have segments inside instead
of buttons, but in the case where a cropped touchscreen is also
described, you would end up with a segment outside the buttons node and
the rest inside. That would reduce the parsing savings. Some labeling
would help in that case, but that would be not as clear as the current
implementation.
There is another point that I will just touch upon because I have no
experience in the matter. I have seen that some keys use the
'linux,input-type' property to define themselves as keys, switches, etc.
If that property or any other that I do not know is necessary for other
implementations, the button object will cover them better than a generic
segment where half of the properties would be meaningless in some
scenarios. Buttons/keys are so ubiquitous that a dedicated object for
them does not look that bad imho.
But as I said, I do not want to make a strong statement here because I
have seen that you maintain several bindings where this properties are
present and I am not the right person to explain that to you... or
actually anyone else out there :)
Talking about the code itself, having a structure for buttons is handy
because you can keep track of the button status (e.g. pressed) and in
the end it is just a child of the base shape that is used for the
overlay touchscreen. The same applies to any function that handles
buttons: they just wrap around the shape functions and add the
button-specific management. So if the parsing is taken aside, the code
does not get much savings from that side and it is again much more
readable and comprehensible.
Thank you for your efforts to improve these patches and the constructive
discussion.
Best regards,
Javier Carrasco
^ permalink raw reply
* [PATCH v4 0/4] input: touchscreen: add initial support for Goodix Berlin touchscreen IC
From: Neil Armstrong @ 2023-06-29 9:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong, Rob Herring
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
Changes in v4:
- Core updates:
- drop kconfig depends, deps will be handled by _SPI and _I2C
- change power_on() error labels
- print errors on all dev_err() prints
- remove useless default variable initialization
- switch irq touch checksum error to dev_err()
- add Jeff's review tag
- I2C changes
- change REGMAP_I2C Kconfig from depends to select
- add Jeff's review tag
- SPI changes
- add select REGMAP to Kconfig
- added GOODIX_BERLIN_ prefix to defines
- switched from ret to error
- add Jeff's review tag
- Link to v3: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v3-0-f0577cead709@linaro.org
Changes in v3:
- Another guge cleanups after Jeff's review:
- appended goodix_berlin_ before all defines
- removed some unused defines
- removed retries on most of read functions, can be added back later
- added __le to ic_info structures
- reworked and simplified irq handling, dropped enum and ts_event structs
- added struct for touch data
- simplified and cleaned goodix_berlin_check_checksum & goodix_berlin_is_dummy_data
- moved touch_data_addr to the end of the main code_data
- reworked probe to get_irq last and right before setip input device
- cleaned probe by removing the "cd->dev"
- added short paragraph to justify new driver for berlin devices
- defined all offsets & masks
- Added bindings review tag
- Link to v2: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v2-0-26bc8fe1e90e@linaro.org
Changes in v2:
- Huge cleanups after Jeff's review:
- switch to error instead of ret
- drop dummy vendor/product ids
- drop unused defined/enums
- drop unused ic_info and only keep needes values
- cleanup namings and use goodix_berlin_ everywhere
- fix regulator setup
- fix default variables value when assigned afterwars
- removed indirections
- dropped debugfs
- cleaned input_dev setup
- dropped _remove()
- sync'ed i2c and spi drivers
- fixed yaml bindings
- Link to v1: https://lore.kernel.org/r/20230606-topic-goodix-berlin-upstream-initial-v1-0-4a0741b8aefd@linaro.org
---
Neil Armstrong (4):
dt-bindings: input: document Goodix Berlin Touchscreen IC
input: touchscreen: add core support for Goodix Berlin Touchscreen IC
input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++
drivers/input/touchscreen/Kconfig | 31 ++
drivers/input/touchscreen/Makefile | 3 +
drivers/input/touchscreen/goodix_berlin.h | 159 ++++++
drivers/input/touchscreen/goodix_berlin_core.c | 581 +++++++++++++++++++++
drivers/input/touchscreen/goodix_berlin_i2c.c | 69 +++
drivers/input/touchscreen/goodix_berlin_spi.c | 173 ++++++
7 files changed, 1111 insertions(+)
---
base-commit: 6db29e14f4fb7bce9eb5290288e71b05c2b0d118
change-id: 20230606-topic-goodix-berlin-upstream-initial-ba97e8ec8f4c
Best regards,
--
Neil Armstrong <neil.armstrong@linaro.org>
^ permalink raw reply
* [PATCH v4 1/4] dt-bindings: input: document Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-29 9:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong, Rob Herring
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org>
Document the Goodix GT9916 wich is part of the "Berlin" serie
of Touchscreen controllers IC from Goodix.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
.../bindings/input/touchscreen/goodix,gt9916.yaml | 95 ++++++++++++++++++++++
1 file changed, 95 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
new file mode 100644
index 000000000000..d90f045ac06c
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/touchscreen/goodix,gt9916.yaml
@@ -0,0 +1,95 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/touchscreen/goodix,gt9916.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Goodix Berlin series touchscreen controller
+
+description: The Goodix Berlin series of touchscreen controllers
+ be connected to either I2C or SPI buses.
+
+maintainers:
+ - Neil Armstrong <neil.armstrong@linaro.org>
+
+allOf:
+ - $ref: touchscreen.yaml#
+ - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+properties:
+ compatible:
+ enum:
+ - goodix,gt9916
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ reset-gpios:
+ maxItems: 1
+
+ avdd-supply:
+ description: Analog power supply regulator on AVDD pin
+
+ vddio-supply:
+ description: power supply regulator on VDDIO pin
+
+ spi-max-frequency: true
+ touchscreen-inverted-x: true
+ touchscreen-inverted-y: true
+ touchscreen-size-x: true
+ touchscreen-size-y: true
+ touchscreen-swapped-x-y: true
+
+additionalProperties: false
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - avdd-supply
+ - touchscreen-size-x
+ - touchscreen-size-y
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ touchscreen@5d {
+ compatible = "goodix,gt9916";
+ reg = <0x5d>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ num-cs = <1>;
+ cs-gpios = <&gpio 2 GPIO_ACTIVE_HIGH>;
+ touchscreen@0 {
+ compatible = "goodix,gt9916";
+ reg = <0>;
+ interrupt-parent = <&gpio>;
+ interrupts = <25 IRQ_TYPE_LEVEL_LOW>;
+ reset-gpios = <&gpio1 1 GPIO_ACTIVE_LOW>;
+ avdd-supply = <&ts_avdd>;
+ spi-max-frequency = <1000000>;
+ touchscreen-size-x = <1024>;
+ touchscreen-size-y = <768>;
+ };
+ };
+
+...
--
2.34.1
^ permalink raw reply related
* [PATCH v4 3/4] input: touchscreen: add I2C support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-29 9:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the I2C interface.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 ++++++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_i2c.c | 69 +++++++++++++++++++++++++++
3 files changed, 84 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index f6ed3f334447..d35169cbb499 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -419,6 +419,20 @@ config TOUCHSCREEN_GOODIX
config TOUCHSCREEN_GOODIX_BERLIN_CORE
tristate
+config TOUCHSCREEN_GOODIX_BERLIN_I2C
+ tristate "Goodix Berlin I2C touchscreen"
+ depends on I2C
+ select REGMAP_I2C
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via I2C.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_i2c.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 29cdb042e104..921a2da0c2be 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -48,6 +48,7 @@ obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_i2c.c b/drivers/input/touchscreen/goodix_berlin_i2c.c
new file mode 100644
index 000000000000..6407b2258eb1
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_i2c.c
@@ -0,0 +1,69 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <linux/i2c.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+#define I2C_MAX_TRANSFER_SIZE 256
+
+static const struct regmap_config goodix_berlin_i2c_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .max_raw_read = I2C_MAX_TRANSFER_SIZE,
+ .max_raw_write = I2C_MAX_TRANSFER_SIZE,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_i2c_input_id = {
+ .bustype = BUS_I2C,
+};
+
+static int goodix_berlin_i2c_probe(struct i2c_client *client)
+{
+ struct regmap *regmap;
+
+ regmap = devm_regmap_init_i2c(client, &goodix_berlin_i2c_regmap_conf);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return goodix_berlin_probe(&client->dev, client->irq,
+ &goodix_berlin_i2c_input_id, regmap);
+}
+
+static const struct i2c_device_id goodix_berlin_i2c_id[] = {
+ { "gt9916", 0 },
+ { }
+};
+
+MODULE_DEVICE_TABLE(i2c, goodix_berlin_i2c_id);
+
+static const struct of_device_id goodix_berlin_i2c_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_i2c_of_match);
+
+static struct i2c_driver goodix_berlin_i2c_driver = {
+ .driver = {
+ .name = "goodix-berlin-i2c",
+ .of_match_table = goodix_berlin_i2c_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_i2c_probe,
+ .id_table = goodix_berlin_i2c_id,
+};
+module_i2c_driver(goodix_berlin_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin I2C Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v4 2/4] input: touchscreen: add core support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-29 9:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs.
These touchscreen ICs support SPI, I2C and I3C interface, up to
10 finger touch, stylus and gestures events.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
Support for advanced features like:
- Firmware & config update
- Stylus events
- Gestures events
- Previous revisions support (BerlinA or BerlinB)
is not included in current version.
The current support will work with currently flashed firmware
and config, and bail out if firmware or config aren't flashed yet.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 3 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin.h | 159 +++++++
drivers/input/touchscreen/goodix_berlin_core.c | 581 +++++++++++++++++++++++++
4 files changed, 744 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index c2cbd332af1d..f6ed3f334447 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -416,6 +416,9 @@ config TOUCHSCREEN_GOODIX
To compile this driver as a module, choose M here: the
module will be called goodix.
+config TOUCHSCREEN_GOODIX_BERLIN_CORE
+ tristate
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 159cd5136fdb..29cdb042e104 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -47,6 +47,7 @@ obj-$(CONFIG_TOUCHSCREEN_EGALAX_SERIAL) += egalax_ts_serial.o
obj-$(CONFIG_TOUCHSCREEN_EXC3000) += exc3000.o
obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin.h b/drivers/input/touchscreen/goodix_berlin.h
new file mode 100644
index 000000000000..235f44947a28
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin.h
@@ -0,0 +1,159 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_berlin_berlin driver.
+ */
+
+#ifndef __GOODIX_BERLIN_H_
+#define __GOODIX_BERLIN_H_
+
+#include <linux/gpio/consumer.h>
+#include <linux/input.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regulator/consumer.h>
+#include <linux/sizes.h>
+
+#define GOODIX_BERLIN_MAX_TOUCH 10
+
+#define GOODIX_BERLIN_NORMAL_RESET_DELAY_MS 100
+
+#define GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN 8
+#define GOODIX_BERLIN_STATUS_OFFSET 0
+#define GOODIX_BERLIN_REQUEST_TYPE_OFFSET 2
+
+#define GOODIX_BERLIN_BYTES_PER_POINT 8
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE 2
+#define GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK GENMASK(15, 0)
+
+/* Read n finger events */
+#define GOODIX_BERLIN_IRQ_READ_LEN(n) (GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN + \
+ (GOODIX_BERLIN_BYTES_PER_POINT * (n)) + \
+ GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+
+#define GOODIX_BERLIN_TOUCH_EVENT BIT(7)
+#define GOODIX_BERLIN_REQUEST_EVENT BIT(6)
+#define GOODIX_BERLIN_TOUCH_COUNT_MASK GENMASK(3, 0)
+
+#define GOODIX_BERLIN_REQUEST_CODE_RESET 3
+
+#define GOODIX_BERLIN_POINT_TYPE_MASK GENMASK(3, 0)
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER 1
+#define GOODIX_BERLIN_POINT_TYPE_STYLUS 3
+
+#define GOODIX_BERLIN_TOUCH_ID_MASK GENMASK(7, 4)
+
+#define GOODIX_BERLIN_DEV_CONFIRM_VAL 0xAA
+#define GOODIX_BERLIN_BOOTOPTION_ADDR 0x10000
+#define GOODIX_BERLIN_FW_VERSION_INFO_ADDR 0x10014
+
+#define GOODIX_BERLIN_IC_INFO_MAX_LEN SZ_1K
+#define GOODIX_BERLIN_IC_INFO_ADDR 0x10070
+
+struct goodix_berlin_fw_version {
+ u8 rom_pid[6];
+ u8 rom_vid[3];
+ u8 rom_vid_reserved;
+ u8 patch_pid[8];
+ u8 patch_vid[4];
+ u8 patch_vid_reserved;
+ u8 sensor_id;
+ u8 reserved[2];
+ __le16 checksum;
+} __packed;
+
+struct goodix_berlin_ic_info_version {
+ u8 info_customer_id;
+ u8 info_version_id;
+ u8 ic_die_id;
+ u8 ic_version_id;
+ __le32 config_id;
+ u8 config_version;
+ u8 frame_data_customer_id;
+ u8 frame_data_version_id;
+ u8 touch_data_customer_id;
+ u8 touch_data_version_id;
+ u8 reserved[3];
+} __packed;
+
+struct goodix_berlin_ic_info_feature {
+ __le16 freqhop_feature;
+ __le16 calibration_feature;
+ __le16 gesture_feature;
+ __le16 side_touch_feature;
+ __le16 stylus_feature;
+} __packed;
+
+struct goodix_berlin_ic_info_misc {
+ __le32 cmd_addr;
+ __le16 cmd_max_len;
+ __le32 cmd_reply_addr;
+ __le16 cmd_reply_len;
+ __le32 fw_state_addr;
+ __le16 fw_state_len;
+ __le32 fw_buffer_addr;
+ __le16 fw_buffer_max_len;
+ __le32 frame_data_addr;
+ __le16 frame_data_head_len;
+ __le16 fw_attr_len;
+ __le16 fw_log_len;
+ u8 pack_max_num;
+ u8 pack_compress_version;
+ __le16 stylus_struct_len;
+ __le16 mutual_struct_len;
+ __le16 self_struct_len;
+ __le16 noise_struct_len;
+ __le32 touch_data_addr;
+ __le16 touch_data_head_len;
+ __le16 point_struct_len;
+ __le16 reserved1;
+ __le16 reserved2;
+ __le32 mutual_rawdata_addr;
+ __le32 mutual_diffdata_addr;
+ __le32 mutual_refdata_addr;
+ __le32 self_rawdata_addr;
+ __le32 self_diffdata_addr;
+ __le32 self_refdata_addr;
+ __le32 iq_rawdata_addr;
+ __le32 iq_refdata_addr;
+ __le32 im_rawdata_addr;
+ __le16 im_readata_len;
+ __le32 noise_rawdata_addr;
+ __le16 noise_rawdata_len;
+ __le32 stylus_rawdata_addr;
+ __le16 stylus_rawdata_len;
+ __le32 noise_data_addr;
+ __le32 esd_addr;
+} __packed;
+
+struct goodix_berlin_touch_data {
+ u8 id;
+ u8 unused;
+ __le16 x;
+ __le16 y;
+ __le16 w;
+} __packed;
+
+struct goodix_berlin_core {
+ struct device *dev;
+ struct regmap *regmap;
+ struct regulator *avdd;
+ struct regulator *iovdd;
+ struct gpio_desc *reset_gpio;
+ struct touchscreen_properties props;
+ struct goodix_berlin_fw_version fw_version;
+ struct input_dev *input_dev;
+ int irq;
+
+ /* Runtime parameters extracted from IC_INFO buffer */
+ u32 touch_data_addr;
+};
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap);
+
+extern const struct dev_pm_ops goodix_berlin_pm_ops;
+
+#endif
diff --git a/drivers/input/touchscreen/goodix_berlin_core.c b/drivers/input/touchscreen/goodix_berlin_core.c
new file mode 100644
index 000000000000..08c508677669
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_core.c
@@ -0,0 +1,581 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Touchscreen Driver
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/input/mt.h>
+#include <linux/input/touchscreen.h>
+#include <linux/regmap.h>
+
+#include "goodix_berlin.h"
+
+/*
+ * Goodix "Berlin" Touchscreen ID driver
+ *
+ * This driver is distinct from goodix.c since hardware interface
+ * is different enough to require a new driver.
+ * None of the register address or data structure are close enough
+ * to the previous generations.
+ *
+ * Currently only handles Multitouch events with already
+ * programmed firmware and "config" for "Revision D" Berlin IC.
+ *
+ * Support is missing for:
+ * - ESD Management
+ * - Firmware update/flashing
+ * - "Config" update/flashing
+ * - Stylus Events
+ * - Gesture Events
+ * - Support for older revisions (A & B)
+ */
+
+static bool goodix_berlin_checksum_valid(const u8 *data, int size)
+{
+ u32 cal_checksum = 0;
+ u16 r_checksum;
+ u32 i;
+
+ if (size < GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE)
+ return false;
+
+ for (i = 0; i < size - GOODIX_BERLIN_COOR_DATA_CHECKSUM_SIZE; i++)
+ cal_checksum += data[i];
+
+ r_checksum = get_unaligned_le16(&data[i]);
+
+ return FIELD_GET(GOODIX_BERLIN_COOR_DATA_CHECKSUM_MASK, cal_checksum) == r_checksum;
+}
+
+static bool goodix_berlin_is_dummy_data(struct goodix_berlin_core *cd,
+ const u8 *data, int size)
+{
+ int i;
+
+ /*
+ * If the device is missing or doesn't respond the buffer
+ * could be filled with bus default line state, 0x00 or 0xff,
+ * so declare success the first time we encounter neither.
+ */
+ for (i = 0; i < size; i++)
+ if (data[i] > 0 && data[i] < 0xff)
+ return false;
+
+ return true;
+}
+
+static int goodix_berlin_dev_confirm(struct goodix_berlin_core *cd)
+{
+ u8 tx_buf[8], rx_buf[8];
+ int retry = 3;
+ int error;
+
+ memset(tx_buf, GOODIX_BERLIN_DEV_CONFIRM_VAL, sizeof(tx_buf));
+ while (retry--) {
+ error = regmap_raw_write(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, tx_buf,
+ sizeof(tx_buf));
+ if (error)
+ return error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_BOOTOPTION_ADDR, rx_buf,
+ sizeof(rx_buf));
+ if (error)
+ return error;
+
+ if (!memcmp(tx_buf, rx_buf, sizeof(tx_buf)))
+ return 0;
+
+ usleep_range(5000, 5100);
+ }
+
+ dev_err(cd->dev, "device confirm failed, rx_buf: %*ph\n", 8, rx_buf);
+
+ return -EINVAL;
+}
+
+static int goodix_berlin_power_on(struct goodix_berlin_core *cd, bool on)
+{
+ int error = 0;
+
+ if (on) {
+ error = regulator_enable(cd->iovdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable iovdd: %d\n", error);
+ return error;
+ }
+
+ /* Vendor waits 3ms for IOVDD to settle */
+ usleep_range(3000, 3100);
+
+ error = regulator_enable(cd->avdd);
+ if (error) {
+ dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
+ goto err_iovdd_disable;
+ }
+
+ /* Vendor waits 15ms for IOVDD to settle */
+ usleep_range(15000, 15100);
+
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ /* Vendor waits 4ms for Firmware to initialize */
+ usleep_range(4000, 4100);
+
+ error = goodix_berlin_dev_confirm(cd);
+ if (error)
+ goto err_dev_reset;
+
+ /* Vendor waits 100ms for Firmware to fully boot */
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+ }
+
+err_dev_reset:
+ gpiod_set_value(cd->reset_gpio, 1);
+
+ regulator_disable(cd->avdd);
+
+err_iovdd_disable:
+ regulator_disable(cd->iovdd);
+
+ return error;
+}
+
+static int goodix_berlin_read_version(struct goodix_berlin_core *cd)
+{
+ u8 buf[sizeof(struct goodix_berlin_fw_version)];
+ int error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_FW_VERSION_INFO_ADDR, buf, sizeof(buf));
+ if (error) {
+ dev_err(cd->dev, "error reading fw version, %d\n", error);
+ return error;
+ }
+
+ if (!goodix_berlin_checksum_valid(buf, sizeof(buf))) {
+ dev_err(cd->dev, "invalid fw version: checksum error\n");
+ return -EINVAL;
+ }
+
+ memcpy(&cd->fw_version, buf, sizeof(cd->fw_version));
+
+ return 0;
+}
+
+/* Only extract necessary data for runtime */
+static int goodix_berlin_convert_ic_info(struct goodix_berlin_core *cd,
+ const u8 *data, u16 length)
+{
+ struct goodix_berlin_ic_info_misc misc;
+ unsigned int offset = 0;
+ u8 param_num;
+
+ offset += sizeof(__le16); /* length */
+ offset += sizeof(struct goodix_berlin_ic_info_version);
+ offset += sizeof(struct goodix_berlin_ic_info_feature);
+
+ /* IC_INFO Parameters, variable width structure */
+ offset += 4 * sizeof(u8); /* drv_num, sen_num, button_num, force_num */
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* active_scan_rate_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* mutual_freq_num*/
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_tx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* self_rx_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset >= length)
+ goto invalid_offset;
+
+ param_num = data[offset++]; /* stylus_freq_num */
+
+ offset += param_num * sizeof(__le16);
+
+ if (offset + sizeof(misc) > length)
+ goto invalid_offset;
+
+ /* goodix_berlin_ic_info_misc */
+ memcpy(&misc, &data[offset], sizeof(misc));
+
+ cd->touch_data_addr = le32_to_cpu(misc.touch_data_addr);
+
+ return 0;
+
+invalid_offset:
+ dev_err(cd->dev, "ic_info length is invalid (offset %d length %d)\n",
+ offset, length);
+ return -EINVAL;
+}
+
+static int goodix_berlin_get_ic_info(struct goodix_berlin_core *cd)
+{
+ u8 afe_data[GOODIX_BERLIN_IC_INFO_MAX_LEN];
+ __le16 length_raw;
+ u16 length;
+ int error;
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ &length_raw, sizeof(length_raw));
+ if (error) {
+ dev_info(cd->dev, "failed get ic info length, %d\n", error);
+ return error;
+ }
+
+ length = le16_to_cpu(length_raw);
+ if (length >= GOODIX_BERLIN_IC_INFO_MAX_LEN) {
+ dev_info(cd->dev, "invalid ic info length %d\n", length);
+ return -EINVAL;
+ }
+
+ error = regmap_raw_read(cd->regmap, GOODIX_BERLIN_IC_INFO_ADDR,
+ afe_data, length);
+ if (error) {
+ dev_info(cd->dev, "failed get ic info data, %d\n", error);
+ return error;
+ }
+
+ /* check whether the data is valid (ex. bus default values) */
+ if (goodix_berlin_is_dummy_data(cd, (const uint8_t *)afe_data, length)) {
+ dev_err(cd->dev, "fw info data invalid\n");
+ return -EINVAL;
+ }
+
+ if (!goodix_berlin_checksum_valid((const uint8_t *)afe_data, length)) {
+ dev_info(cd->dev, "fw info checksum error\n");
+ return -EINVAL;
+ }
+
+ error = goodix_berlin_convert_ic_info(cd, afe_data, length);
+ if (error)
+ return error;
+
+ /* check some key info */
+ if (!cd->touch_data_addr) {
+ dev_err(cd->dev, "touch_data_addr is null\n");
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static void goodix_berlin_parse_finger(struct goodix_berlin_core *cd,
+ const void *buf, int touch_num)
+{
+ const struct goodix_berlin_touch_data *touch_data = buf;
+ int i;
+
+ /* Check for data validity */
+ for (i = 0; i < touch_num; i++) {
+ unsigned int id;
+
+ id = FIELD_GET(GOODIX_BERLIN_TOUCH_ID_MASK, touch_data[i].id);
+
+ if (id >= GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid finger id %d\n", id);
+ return;
+ }
+ }
+
+ /* Report finger touches */
+ for (i = 0; i < touch_num; i++) {
+ input_mt_slot(cd->input_dev,
+ FIELD_GET(GOODIX_BERLIN_TOUCH_ID_MASK,
+ touch_data[i].id));
+ input_mt_report_slot_state(cd->input_dev, MT_TOOL_FINGER, true);
+
+ touchscreen_report_pos(cd->input_dev, &cd->props,
+ __le16_to_cpu(touch_data[i].x),
+ __le16_to_cpu(touch_data[i].y),
+ true);
+ input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
+ __le16_to_cpu(touch_data[i].w));
+ }
+
+ input_mt_sync_frame(cd->input_dev);
+ input_sync(cd->input_dev);
+}
+
+static void goodix_berlin_touch_handler(struct goodix_berlin_core *cd,
+ const void *pre_buf, u32 pre_buf_len)
+{
+ static u8 buffer[GOODIX_BERLIN_IRQ_READ_LEN(GOODIX_BERLIN_MAX_TOUCH)];
+ u8 point_type, touch_num;
+ int error;
+
+ /* copy pre-data to buffer */
+ memcpy(buffer, pre_buf, pre_buf_len);
+
+ touch_num = FIELD_GET(GOODIX_BERLIN_TOUCH_COUNT_MASK,
+ buffer[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+
+ if (touch_num > GOODIX_BERLIN_MAX_TOUCH) {
+ dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
+ return;
+ }
+
+ if (touch_num) {
+ /* read more data if more than 2 touch events */
+ if (unlikely(touch_num > 2)) {
+ error = regmap_raw_read(cd->regmap,
+ cd->touch_data_addr + pre_buf_len,
+ &buffer[pre_buf_len],
+ (touch_num - 2) * GOODIX_BERLIN_BYTES_PER_POINT);
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed to get touch data, %d\n",
+ error);
+ return;
+ }
+ }
+
+ point_type = FIELD_GET(GOODIX_BERLIN_POINT_TYPE_MASK,
+ buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+
+ if (point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS ||
+ point_type == GOODIX_BERLIN_POINT_TYPE_STYLUS_HOVER) {
+ dev_warn_once(cd->dev, "Stylus event type not handled\n");
+ return;
+ }
+
+ if (!goodix_berlin_checksum_valid(&buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2)) {
+ dev_err(cd->dev, "touch data checksum error, data: %*ph\n",
+ touch_num * GOODIX_BERLIN_BYTES_PER_POINT + 2,
+ &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN]);
+ return;
+ }
+ }
+
+ goodix_berlin_parse_finger(cd, &buffer[GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN],
+ touch_num);
+}
+
+static int goodix_berlin_request_handle_reset(struct goodix_berlin_core *cd)
+{
+ gpiod_set_value(cd->reset_gpio, 1);
+ usleep_range(2000, 2100);
+ gpiod_set_value(cd->reset_gpio, 0);
+
+ msleep(GOODIX_BERLIN_NORMAL_RESET_DELAY_MS);
+
+ return 0;
+}
+
+static irqreturn_t goodix_berlin_threadirq_func(int irq, void *data)
+{
+ struct goodix_berlin_core *cd = data;
+ u8 buf[GOODIX_BERLIN_IRQ_READ_LEN(2)];
+ u8 event_status;
+ int error;
+
+ /* First, read buffer with space for 2 touch events */
+ error = regmap_raw_read(cd->regmap, cd->touch_data_addr, buf,
+ GOODIX_BERLIN_IRQ_READ_LEN(2));
+ if (error) {
+ dev_err_ratelimited(cd->dev, "failed get event head data, %d\n", error);
+ return IRQ_HANDLED;
+ }
+
+ if (buf[GOODIX_BERLIN_STATUS_OFFSET] == 0)
+ return IRQ_HANDLED;
+
+ if (!goodix_berlin_checksum_valid(buf, GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN)) {
+ dev_warn_ratelimited(cd->dev, "touch head checksum err : %*ph\n",
+ GOODIX_BERLIN_IRQ_EVENT_HEAD_LEN, buf);
+ return IRQ_HANDLED;
+ }
+
+ event_status = buf[GOODIX_BERLIN_STATUS_OFFSET];
+
+ if (event_status & GOODIX_BERLIN_TOUCH_EVENT)
+ goodix_berlin_touch_handler(cd, buf, GOODIX_BERLIN_IRQ_READ_LEN(2));
+
+ if (event_status & GOODIX_BERLIN_REQUEST_EVENT) {
+ switch (buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]) {
+ case GOODIX_BERLIN_REQUEST_CODE_RESET:
+ goodix_berlin_request_handle_reset(cd);
+ break;
+
+ default:
+ dev_warn(cd->dev, "unsupported request code 0x%x\n",
+ buf[GOODIX_BERLIN_REQUEST_TYPE_OFFSET]);
+ }
+ }
+
+ /* Clear up status field */
+ regmap_write(cd->regmap, cd->touch_data_addr, 0);
+
+ return IRQ_HANDLED;
+}
+
+static int goodix_berlin_input_dev_config(struct goodix_berlin_core *cd,
+ const struct input_id *id)
+{
+ struct input_dev *input_dev;
+ int error;
+
+ input_dev = devm_input_allocate_device(cd->dev);
+ if (!input_dev)
+ return -ENOMEM;
+
+ cd->input_dev = input_dev;
+ input_set_drvdata(input_dev, cd);
+
+ input_dev->name = "Goodix Berlin Capacitive TouchScreen";
+ input_dev->phys = "input/ts";
+
+ input_dev->id = *id;
+
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_X, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_POSITION_Y, 0, SZ_64K - 1, 0, 0);
+ input_set_abs_params(cd->input_dev, ABS_MT_TOUCH_MAJOR, 0, 255, 0, 0);
+
+ touchscreen_parse_properties(cd->input_dev, true, &cd->props);
+
+ error = input_mt_init_slots(cd->input_dev, GOODIX_BERLIN_MAX_TOUCH,
+ INPUT_MT_DIRECT | INPUT_MT_DROP_UNUSED);
+ if (error)
+ return error;
+
+ return input_register_device(cd->input_dev);
+}
+
+static int goodix_berlin_pm_suspend(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+
+ disable_irq(cd->irq);
+
+ return goodix_berlin_power_on(cd, false);
+}
+
+static int goodix_berlin_pm_resume(struct device *dev)
+{
+ struct goodix_berlin_core *cd = dev_get_drvdata(dev);
+ int error;
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error)
+ return error;
+
+ enable_irq(cd->irq);
+
+ return 0;
+}
+
+EXPORT_GPL_SIMPLE_DEV_PM_OPS(goodix_berlin_pm_ops,
+ goodix_berlin_pm_suspend,
+ goodix_berlin_pm_resume);
+
+static void goodix_berlin_power_off(void *data)
+{
+ struct goodix_berlin_core *cd = data;
+
+ goodix_berlin_power_on(cd, false);
+}
+
+int goodix_berlin_probe(struct device *dev, int irq, const struct input_id *id,
+ struct regmap *regmap)
+{
+ struct goodix_berlin_core *cd;
+ int error;
+
+ if (irq <= 0) {
+ dev_err(dev, "Missing interrupt number\n");
+ return -EINVAL;
+ }
+
+ cd = devm_kzalloc(dev, sizeof(*cd), GFP_KERNEL);
+ if (!cd)
+ return -ENOMEM;
+
+ cd->dev = dev;
+ cd->regmap = regmap;
+ cd->irq = irq;
+
+ cd->reset_gpio = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH);
+ if (IS_ERR(cd->reset_gpio))
+ return dev_err_probe(dev, PTR_ERR(cd->reset_gpio),
+ "Failed to request reset gpio\n");
+
+ cd->avdd = devm_regulator_get(dev, "avdd");
+ if (IS_ERR(cd->avdd))
+ return dev_err_probe(dev, PTR_ERR(cd->avdd),
+ "Failed to request avdd regulator\n");
+
+ cd->iovdd = devm_regulator_get(dev, "iovdd");
+ if (IS_ERR(cd->iovdd))
+ return dev_err_probe(dev, PTR_ERR(cd->iovdd),
+ "Failed to request iovdd regulator\n");
+
+ error = goodix_berlin_power_on(cd, true);
+ if (error) {
+ dev_err(dev, "failed power on");
+ return error;
+ }
+
+ error = devm_add_action_or_reset(dev, goodix_berlin_power_off, cd);
+ if (error)
+ return error;
+
+ error = goodix_berlin_read_version(cd);
+ if (error) {
+ dev_err(dev, "failed to get version info");
+ return error;
+ }
+
+ error = goodix_berlin_get_ic_info(cd);
+ if (error) {
+ dev_err(dev, "invalid ic info, abort");
+ return error;
+ }
+
+ error = goodix_berlin_input_dev_config(cd, id);
+ if (error) {
+ dev_err(dev, "failed set input device");
+ return error;
+ }
+
+ error = devm_request_threaded_irq(dev, irq, NULL,
+ goodix_berlin_threadirq_func,
+ IRQF_ONESHOT, "goodix-berlin", cd);
+ if (error) {
+ dev_err(dev, "request threaded irq failed: %d\n", error);
+ return error;
+ }
+
+ dev_set_drvdata(dev, cd);
+
+ dev_dbg(dev, "Goodix Berlin %s Touchscreen Controller", cd->fw_version.patch_pid);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(goodix_berlin_probe);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin Core Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* [PATCH v4 4/4] input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-29 9:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, Jeff LaBundy
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v4-0-0947c489be17@linaro.org>
Add initial support for the new Goodix "Berlin" touchscreen ICs
over the SPI interface.
The driver doesn't use the regmap_spi code since the SPI messages
needs to be prefixed, thus this custom regmap code.
This initial driver is derived from the Goodix goodix_ts_berlin
available at [1] and [2] and only supports the GT9916 IC
present on the Qualcomm SM8550 MTP & QRD touch panel.
The current implementation only supports BerlinD, aka GT9916.
[1] https://github.com/goodix/goodix_ts_berlin
[2] https://git.codelinaro.org/clo/la/platform/vendor/opensource/touch-drivers
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 14 +++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_spi.c | 173 ++++++++++++++++++++++++++
3 files changed, 188 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index d35169cbb499..4f017555bd5b 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -433,6 +433,20 @@ config TOUCHSCREEN_GOODIX_BERLIN_I2C
To compile this driver as a module, choose M here: the
module will be called goodix_berlin_i2c.
+config TOUCHSCREEN_GOODIX_BERLIN_SPI
+ tristate "Goodix Berlin SPI touchscreen"
+ depends on SPI_MASTER
+ select REGMAP
+ select TOUCHSCREEN_GOODIX_BERLIN_CORE
+ help
+ Say Y here if you have a Goodix Berlin IC connected to
+ your system via SPI.
+
+ If unsure, say N.
+
+ To compile this driver as a module, choose M here: the
+ module will be called goodix_berlin_spi.
+
config TOUCHSCREEN_HIDEEP
tristate "HiDeep Touch IC"
depends on I2C
diff --git a/drivers/input/touchscreen/Makefile b/drivers/input/touchscreen/Makefile
index 921a2da0c2be..29524e8a83db 100644
--- a/drivers/input/touchscreen/Makefile
+++ b/drivers/input/touchscreen/Makefile
@@ -49,6 +49,7 @@ obj-$(CONFIG_TOUCHSCREEN_FUJITSU) += fujitsu_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX) += goodix_ts.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_CORE) += goodix_berlin_core.o
obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_I2C) += goodix_berlin_i2c.o
+obj-$(CONFIG_TOUCHSCREEN_GOODIX_BERLIN_SPI) += goodix_berlin_spi.o
obj-$(CONFIG_TOUCHSCREEN_HIDEEP) += hideep.o
obj-$(CONFIG_TOUCHSCREEN_HYNITRON_CSTXXX) += hynitron_cstxxx.o
obj-$(CONFIG_TOUCHSCREEN_ILI210X) += ili210x.o
diff --git a/drivers/input/touchscreen/goodix_berlin_spi.c b/drivers/input/touchscreen/goodix_berlin_spi.c
new file mode 100644
index 000000000000..1f57c3592918
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -0,0 +1,173 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Goodix Berlin Touchscreen Driver
+ *
+ * Copyright (C) 2020 - 2021 Goodix, Inc.
+ * Copyright (C) 2023 Linaro Ltd.
+ *
+ * Based on goodix_ts_berlin driver.
+ */
+#include <asm/unaligned.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+#include <linux/spi/spi.h>
+
+#include "goodix_berlin.h"
+
+#define GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN 1
+#define GOODIX_BERLIN_REGISTER_WIDTH 4
+#define GOODIX_BERLIN_SPI_READ_DUMMY_LEN 3
+#define GOODIX_BERLIN_SPI_READ_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH + \
+ GOODIX_BERLIN_SPI_READ_DUMMY_LEN)
+#define GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN (GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + \
+ GOODIX_BERLIN_REGISTER_WIDTH)
+
+#define GOODIX_BERLIN_SPI_WRITE_FLAG 0xF0
+#define GOODIX_BERLIN_SPI_READ_FLAG 0xF1
+
+static int goodix_berlin_spi_read(void *context, const void *reg_buf,
+ size_t reg_size, void *val_buf,
+ size_t val_size)
+{
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = reg_buf; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ if (reg_size != GOODIX_BERLIN_REGISTER_WIDTH)
+ return -EINVAL;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ /* buffer format: 0xF1 + addr(4bytes) + dummy(3bytes) + data */
+ buf[0] = GOODIX_BERLIN_SPI_READ_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memset(buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN + GOODIX_BERLIN_REGISTER_WIDTH,
+ 0xff, GOODIX_BERLIN_SPI_READ_DUMMY_LEN);
+
+ xfers.tx_buf = buf;
+ xfers.rx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_READ_PREFIX_LEN + val_size;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+ else
+ memcpy(val_buf, buf + GOODIX_BERLIN_SPI_READ_PREFIX_LEN, val_size);
+
+ kfree(buf);
+ return error;
+}
+
+static int goodix_berlin_spi_write(void *context, const void *data,
+ size_t count)
+{
+ unsigned int len = count - GOODIX_BERLIN_REGISTER_WIDTH;
+ struct spi_device *spi = context;
+ struct spi_transfer xfers;
+ struct spi_message spi_msg;
+ const u32 *reg = data; /* reg is stored as native u32 at start of buffer */
+ u8 *buf;
+ int error;
+
+ buf = kzalloc(GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ buf[0] = GOODIX_BERLIN_SPI_WRITE_FLAG;
+ put_unaligned_be32(*reg, buf + GOODIX_BERLIN_SPI_TRANS_PREFIX_LEN);
+ memcpy(buf + GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN,
+ data + GOODIX_BERLIN_REGISTER_WIDTH, len);
+
+ xfers.tx_buf = buf;
+ xfers.len = GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN + len;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ error = spi_sync(spi, &spi_msg);
+ if (error < 0)
+ dev_err(&spi->dev, "spi transfer error, %d", error);
+
+ kfree(buf);
+ return error;
+}
+
+static const struct regmap_config goodix_berlin_spi_regmap_conf = {
+ .reg_bits = 32,
+ .val_bits = 8,
+ .read = goodix_berlin_spi_read,
+ .write = goodix_berlin_spi_write,
+};
+
+/* vendor & product left unassigned here, should probably be updated from fw info */
+static const struct input_id goodix_berlin_spi_input_id = {
+ .bustype = BUS_SPI,
+};
+
+static int goodix_berlin_spi_probe(struct spi_device *spi)
+{
+ struct regmap_config regmap_config;
+ struct regmap *regmap;
+ size_t max_size;
+ int error = 0;
+
+ spi->mode = SPI_MODE_0;
+ spi->bits_per_word = 8;
+ error = spi_setup(spi);
+ if (error)
+ return error;
+
+ max_size = spi_max_transfer_size(spi);
+
+ regmap_config = goodix_berlin_spi_regmap_conf;
+ regmap_config.max_raw_read = max_size - GOODIX_BERLIN_SPI_READ_PREFIX_LEN;
+ regmap_config.max_raw_write = max_size - GOODIX_BERLIN_SPI_WRITE_PREFIX_LEN;
+
+ regmap = devm_regmap_init(&spi->dev, NULL, spi, ®map_config);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ return goodix_berlin_probe(&spi->dev, spi->irq,
+ &goodix_berlin_spi_input_id, regmap);
+}
+
+static const struct spi_device_id goodix_berlin_spi_ids[] = {
+ { "gt9916" },
+ { },
+};
+MODULE_DEVICE_TABLE(spi, goodix_berlin_spi_ids);
+
+static const struct of_device_id goodix_berlin_spi_of_match[] = {
+ { .compatible = "goodix,gt9916", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, goodix_berlin_spi_of_match);
+
+static struct spi_driver goodix_berlin_spi_driver = {
+ .driver = {
+ .name = "goodix-berlin-spi",
+ .of_match_table = goodix_berlin_spi_of_match,
+ .pm = pm_sleep_ptr(&goodix_berlin_pm_ops),
+ },
+ .probe = goodix_berlin_spi_probe,
+ .id_table = goodix_berlin_spi_ids,
+};
+module_spi_driver(goodix_berlin_spi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Goodix Berlin SPI Touchscreen driver");
+MODULE_AUTHOR("Neil Armstrong <neil.armstrong@linaro.org>");
--
2.34.1
^ permalink raw reply related
* Re: Touchpad lost sync at byte 6
From: Greg KH @ 2023-06-29 9:45 UTC (permalink / raw)
To: Jonathan Denose; +Cc: linux-input
In-Reply-To: <CALNJtpUN+DrWtudWdKZxYMR7oM-yt926fj7_Wi9dUPLZybw-vQ@mail.gmail.com>
On Wed, Jun 28, 2023 at 04:00:54PM -0500, Jonathan Denose wrote:
> I am on a Lenovo N24 laptop.
>
> When this device resumes after closing the lid, the mouse freezes and
> is unusable. After right-clicking multiple times the touchpad resumes
> working. I am seeing the following in the logs:
>
> [ 1188.690138] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.699636] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.709124] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.718557] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.728110] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.975964] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.985456] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1188.994954] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1189.004443] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
> [ 1189.013930] psmouse serio1: Touchpad at isa0060/serio1/input0 lost
> sync at byte 6
>
> This device has an ETPS/2 Elantech Touchpad. I am on kernel version:
> 5.15.117-19629-gf1eb3139d206
Why not try 6.4 and see if that solves the problem?
Has this ever worked on this hardware with any kernel version?
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: input: microchip,cap11xx: add advanced sensitivity settings
From: Rob Herring @ 2023-06-29 15:16 UTC (permalink / raw)
To: Jiri Valek - 2N
Cc: krzysztof.kozlowski+dt, dmitry.torokhov, devicetree, linux-input,
linux-kernel, u.kleine-koenig
In-Reply-To: <20230627065316.1065911-2-jiriv@axis.com>
On Tue, Jun 27, 2023 at 08:53:15AM +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 | 77 +++++++++++++++++--
> 1 file changed, 72 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> index 5fa625b5c5fb..b69dac1fba0e 100644
> --- a/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> +++ b/Documentation/devicetree/bindings/input/microchip,cap11xx.yaml
> @@ -45,13 +45,13 @@ properties:
> Enables the Linux input system's autorepeat feature on the input device.
>
> linux,keycodes:
> - minItems: 6
> - maxItems: 6
> + minItems: 3
> + maxItems: 8
> description: |
> Specifies an array of numeric keycode values to
> be used for the channels. If this property is
> omitted, KEY_A, KEY_B, etc are used as defaults.
> - The array must have exactly six entries.
> + The number of entries must correspond to the number of channels.
This change seems unrelated? Or maybe advanced sensitivity means more
channels? If so, explain this in the commit msg.
>
> microchip,sensor-gain:
> $ref: /schemas/types.yaml#/definitions/uint32
> @@ -70,6 +70,55 @@ properties:
> open drain. This property allows using the active
> high push-pull output.
>
> + microchip,sensitivity-delta-sense:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + default: 32
> + enum: [1, 2, 4, 8, 16, 32, 64, 128]
> + description:
> + Optional parameter. Controls the sensitivity multiplier of a touch detection.
> + At the more sensitive settings, touches are detected for a smaller delta
> + capacitance corresponding to a “lighter” touch.
> +
> + microchip,signal-guard:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 3
> + maxItems: 8
> + items:
> + minimum: 0
> + maximum: 1
> + description: |
> + Optional parameter supported only for CAP129x.
> + 0 - off
> + 1 - on
> + The signal guard isolates the signal from virtual grounds.
> + If enabled then the behavior of the channel is changed to signal guard.
> + The number of entries must correspond to the number of channels.
> +
> + microchip,input-treshold:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 3
> + maxItems: 8
> + items:
> + minimum: 0
> + maximum: 127
> + description:
> + Optional parameter. Specifies the delta threshold that is used to
> + determine if a touch has been detected.
> + The number of entries must correspond to the number of channels.
> +
> + microchip,calib-sensitivity:
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 3
> + maxItems: 8
> + items:
> + minimum: 1
> + maximum: 4
> + description:
> + Optional parameter supported only for CAP129x. Specifies an array of
> + numeric values that controls the gain used by the calibration routine to
> + enable sensor inputs to be more sensitive for proximity detection.
> + The number of entries must correspond to the number of channels.
> +
> patternProperties:
> "^led@[0-7]$":
> type: object
> @@ -98,10 +147,23 @@ allOf:
> compatible:
> contains:
> enum:
> - - microchip,cap1106
> + - microchip,cap1188
> then:
> patternProperties:
> - "^led@[0-7]$": false
> + "^led@[0-7]$": true
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - microchip,cap1293
> + - microchip,cap1298
> +
> + then:
> + properties:
> + microchip,signal-guard: true
> + microchip,calib-sensitivity: true
This 'if' doesn't do anything. These properties are already allowed. You
need an if for the cases they aren't allowed.
>
> required:
> - compatible
> @@ -122,6 +184,11 @@ examples:
> reg = <0x28>;
> autorepeat;
> microchip,sensor-gain = <2>;
> + microchip,sensitivity-delta-sense = <16>;
> +
> + microchip,signal-guard = <0>, <0>, <0>, <0>, <0>, <0>;
> + microchip,input-treshold = <21>, <18>, <46>, <46>, <46>, <21>;
> + microchip,calib-sensitivity = <1>, <2>, <2>, <1>, <1>, <2>;
>
> linux,keycodes = <103>, /* KEY_UP */
> <106>, /* KEY_RIGHT */
> --
> 2.25.1
>
^ permalink raw reply
* Re: [RESEND PATCH 2/2] HID: logitech-hidpp: Add support for the Pro X Superlight
From: Mavroudis Chatzilazaridis @ 2023-06-29 16:03 UTC (permalink / raw)
To: Bastien Nocera, jikos; +Cc: linux-input, benjamin.tissoires, lains
In-Reply-To: <51ab0acb9e903798dc10ef4fe11eb726df76517b.camel@hadess.net>
On 2023-06-28 13:50, Bastien Nocera wrote:
> On Sun, 2023-06-25 at 16:21 +0000, Mavroudis Chatzilazaridis wrote:
>> This patch adds support for the Pro X Superlight. Tested over USB.
>
> Could you please list which functionality this gains the device?
> Battery reporting? Hi-res scrolling? Something else?
Sure thing. The device gains battery reporting in both wired and
wireless mode. Also, without this it's not being recognized by something
like Piper in wireless mode, and thus can't be configured.
^ permalink raw reply
* [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-06-29 17:20 UTC (permalink / raw)
To: linux-input; +Cc: linux-kernel, Jiri Kosina, Benjamin Tissoires
Add support for the Steelseries Arctis 1 XBox headset. This driver
will export the battery information from the headset, as well as the
"wireless_status" property.
Signed-off-by: Bastien Nocera <hadess@hadess.net>
---
drivers/hid/hid-steelseries.c | 307 ++++++++++++++++++++++++++++++++--
1 file changed, 292 insertions(+), 15 deletions(-)
diff --git a/drivers/hid/hid-steelseries.c b/drivers/hid/hid-steelseries.c
index aae3afc4107a..a9300a4244aa 100644
--- a/drivers/hid/hid-steelseries.c
+++ b/drivers/hid/hid-steelseries.c
@@ -1,8 +1,9 @@
// SPDX-License-Identifier: GPL-2.0-or-later
/*
- * HID driver for Steelseries SRW-S1
+ * HID driver for Steelseries devices
*
* Copyright (c) 2013 Simon Wood
+ * Copyright (c) 2023 Bastien Nocera
*/
/*
@@ -11,10 +12,28 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/module.h>
+#include <linux/usb.h>
#include <linux/leds.h>
#include "hid-ids.h"
+#define STEELSERIES_SRWS1 BIT(0)
+#define STEELSERIES_ARCTIS_1 BIT(1)
+
+struct steelseries_device {
+ struct hid_device *hdev;
+ unsigned long quirks;
+
+ struct delayed_work battery_work;
+ spinlock_t lock;
+ bool removed;
+
+ struct power_supply_desc battery_desc;
+ struct power_supply *battery;
+ uint8_t battery_capacity;
+ bool headset_connected;
+};
+
#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
(IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
#define SRWS1_NUMBER_LEDS 15
@@ -353,9 +372,208 @@ static void steelseries_srws1_remove(struct hid_device *hdev)
}
#endif
+#define STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS 3000
+
+#define ARCTIS_1_BATTERY_RESPONSE_LEN 8
+
+static int steelseries_headset_arctis_1_fetch_battery(struct hid_device *hdev)
+{
+ u8 *write_buf;
+ int ret;
+ char battery_request[2] = { 0x06, 0x12 };
+
+ /* Request battery information */
+ write_buf = kmemdup(battery_request, sizeof(battery_request), GFP_KERNEL);
+ if (!write_buf)
+ return -ENOMEM;
+
+ ret = hid_hw_raw_request(hdev, battery_request[0],
+ write_buf, sizeof(battery_request),
+ HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ if (ret < sizeof(battery_request)) {
+ hid_err(hdev, "hid_hw_raw_request() failed with %d\n", ret);
+ ret = -ENODATA;
+ }
+ kfree(write_buf);
+ return ret;
+}
+
+static void steelseries_headset_fetch_battery(struct hid_device *hdev)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ int ret = 0;
+
+ if (sd->quirks & STEELSERIES_ARCTIS_1)
+ ret = steelseries_headset_arctis_1_fetch_battery(hdev);
+
+ if (ret < 0)
+ hid_dbg(hdev,
+ "Battery query failed (err: %d)\n", ret);
+}
+
+static void steelseries_headset_battery_timer_tick(struct work_struct *work)
+{
+ struct steelseries_device *sd = container_of(work,
+ struct steelseries_device, battery_work.work);
+ struct hid_device *hdev = sd->hdev;
+
+ steelseries_headset_fetch_battery(hdev);
+}
+
+static int steelseries_headset_battery_get_property(struct power_supply *psy,
+ enum power_supply_property psp,
+ union power_supply_propval *val)
+{
+ struct steelseries_device *sd = power_supply_get_drvdata(psy);
+ int ret = 0;
+
+ switch (psp) {
+ case POWER_SUPPLY_PROP_PRESENT:
+ val->intval = 1;
+ break;
+ case POWER_SUPPLY_PROP_STATUS:
+ val->intval = sd->headset_connected ?
+ POWER_SUPPLY_STATUS_DISCHARGING :
+ POWER_SUPPLY_STATUS_UNKNOWN;
+ break;
+ case POWER_SUPPLY_PROP_SCOPE:
+ val->intval = POWER_SUPPLY_SCOPE_DEVICE;
+ break;
+ case POWER_SUPPLY_PROP_CAPACITY:
+ val->intval = sd->battery_capacity;
+ break;
+ default:
+ ret = -EINVAL;
+ break;
+ }
+ return ret;
+}
+
+static void
+steelseries_headset_set_wireless_status(struct hid_device *hdev,
+ bool connected)
+{
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ usb_set_wireless_status(intf, connected ?
+ USB_WIRELESS_STATUS_CONNECTED :
+ USB_WIRELESS_STATUS_DISCONNECTED);
+}
+
+static enum power_supply_property steelseries_headset_battery_props[] = {
+ POWER_SUPPLY_PROP_PRESENT,
+ POWER_SUPPLY_PROP_STATUS,
+ POWER_SUPPLY_PROP_SCOPE,
+ POWER_SUPPLY_PROP_CAPACITY,
+};
+
+static int steelseries_headset_battery_register(struct steelseries_device *sd)
+{
+ static atomic_t battery_no = ATOMIC_INIT(0);
+ struct power_supply_config battery_cfg = { .drv_data = sd, };
+ unsigned long n;
+ int ret;
+
+ sd->battery_desc.type = POWER_SUPPLY_TYPE_BATTERY;
+ sd->battery_desc.properties = steelseries_headset_battery_props;
+ sd->battery_desc.num_properties = ARRAY_SIZE(steelseries_headset_battery_props);
+ sd->battery_desc.get_property = steelseries_headset_battery_get_property;
+ sd->battery_desc.use_for_apm = 0;
+ n = atomic_inc_return(&battery_no) - 1;
+ sd->battery_desc.name = devm_kasprintf(&sd->hdev->dev, GFP_KERNEL,
+ "steelseries_headset_battery_%ld", n);
+ if (!sd->battery_desc.name)
+ return -ENOMEM;
+
+ /* avoid the warning of 0% battery while waiting for the first info */
+ steelseries_headset_set_wireless_status(sd->hdev, false);
+ sd->battery_capacity = 100;
+
+ sd->battery = devm_power_supply_register(&sd->hdev->dev,
+ &sd->battery_desc, &battery_cfg);
+ if (IS_ERR(sd->battery)) {
+ ret = PTR_ERR(sd->battery);
+ hid_err(sd->hdev,
+ "%s:power_supply_register failed with error %d\n",
+ __func__, ret);
+ return ret;
+ }
+ power_supply_powers(sd->battery, &sd->hdev->dev);
+
+ INIT_DELAYED_WORK(&sd->battery_work, steelseries_headset_battery_timer_tick);
+ steelseries_headset_fetch_battery(sd->hdev);
+
+ return 0;
+}
+
+static int steelseries_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ struct steelseries_device *sd;
+ int ret;
+
+ sd = devm_kzalloc(&hdev->dev, sizeof(*sd), GFP_KERNEL);
+ if (!sd)
+ return -ENOMEM;
+ hid_set_drvdata(hdev, sd);
+ sd->hdev = hdev;
+ sd->quirks = id->driver_data;
+
+ if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+ return steelseries_srws1_probe(hdev, id);
+#else
+ return -ENODEV;
+#endif
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ return ret;
+
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ return ret;
+
+ if (steelseries_headset_battery_register(sd) < 0)
+ hid_err(sd->hdev,
+ "Failed to register battery for headset\n");
+
+ spin_lock_init(&sd->lock);
+
+ return ret;
+}
+
+static void steelseries_remove(struct hid_device *hdev)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ unsigned long flags;
+
+ if (sd->quirks & STEELSERIES_SRWS1) {
+#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
+ (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
+ steelseries_srws1_remove(hdev);
+#endif
+ return;
+ }
+
+ spin_lock_irqsave(&sd->lock, flags);
+ sd->removed = true;
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ cancel_delayed_work_sync(&sd->battery_work);
+
+ hid_hw_stop(hdev);
+}
+
static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc,
unsigned int *rsize)
{
+ if (hdev->vendor != USB_VENDOR_ID_STEELSERIES ||
+ hdev->product != USB_DEVICE_ID_STEELSERIES_SRWS1)
+ return rdesc;
+
if (*rsize >= 115 && rdesc[11] == 0x02 && rdesc[13] == 0xc8
&& rdesc[29] == 0xbb && rdesc[40] == 0xc5) {
hid_info(hdev, "Fixing up Steelseries SRW-S1 report descriptor\n");
@@ -365,22 +583,81 @@ static __u8 *steelseries_srws1_report_fixup(struct hid_device *hdev, __u8 *rdesc
return rdesc;
}
-static const struct hid_device_id steelseries_srws1_devices[] = {
- { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1) },
+static int steelseries_headset_raw_event(struct hid_device *hdev,
+ struct hid_report *report, u8 *read_buf,
+ int size)
+{
+ struct steelseries_device *sd = hid_get_drvdata(hdev);
+ int capacity = sd->battery_capacity;
+ bool connected = sd->headset_connected;
+ unsigned long flags;
+
+ /* Not a headset */
+ if (sd->quirks & STEELSERIES_SRWS1)
+ return 0;
+
+ if (sd->quirks & STEELSERIES_ARCTIS_1) {
+ hid_dbg(sd->hdev,
+ "Parsing raw event for Arctis 1 headset (len: %d)\n", size);
+ if (size < 8)
+ return 0;
+ if (read_buf[2] == 0x01) {
+ connected = false;
+ capacity = 100;
+ } else {
+ connected = true;
+ capacity = read_buf[3];
+ }
+ }
+
+ if (connected != sd->headset_connected) {
+ hid_dbg(sd->hdev,
+ "Connected status changed from %sconnected to %sconnected\n",
+ sd->headset_connected ? "" : "not ",
+ connected ? "" : "not ");
+ sd->headset_connected = connected;
+ steelseries_headset_set_wireless_status(hdev, connected);
+ }
+
+ if (capacity != sd->battery_capacity) {
+ hid_dbg(sd->hdev,
+ "Battery capacity changed from %d%% to %d%%\n",
+ sd->battery_capacity, capacity);
+ sd->battery_capacity = capacity;
+ power_supply_changed(sd->battery);
+ }
+
+ spin_lock_irqsave(&sd->lock, flags);
+ if (!sd->removed)
+ schedule_delayed_work(&sd->battery_work,
+ msecs_to_jiffies(STEELSERIES_HEADSET_BATTERY_TIMEOUT_MS));
+ spin_unlock_irqrestore(&sd->lock, flags);
+
+ return 0;
+}
+
+static const struct hid_device_id steelseries_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
+ .driver_data = STEELSERIES_SRWS1 },
+
+ { /* SteelSeries Arctis 1 Wireless for XBox */
+ HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
+ .driver_data = STEELSERIES_ARCTIS_1 },
+
{ }
};
-MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
-
-static struct hid_driver steelseries_srws1_driver = {
- .name = "steelseries_srws1",
- .id_table = steelseries_srws1_devices,
-#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
- (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
- .probe = steelseries_srws1_probe,
- .remove = steelseries_srws1_remove,
-#endif
- .report_fixup = steelseries_srws1_report_fixup
+MODULE_DEVICE_TABLE(hid, steelseries_devices);
+
+static struct hid_driver steelseries_driver = {
+ .name = "steelseries",
+ .id_table = steelseries_devices,
+ .probe = steelseries_probe,
+ .remove = steelseries_remove,
+ .report_fixup = steelseries_srws1_report_fixup,
+ .raw_event = steelseries_headset_raw_event,
};
-module_hid_driver(steelseries_srws1_driver);
+module_hid_driver(steelseries_driver);
MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Bastien Nocera <hadess@hadess.net>");
+MODULE_AUTHOR("Simon Wood <simon@mungewell.org>");
--
2.41.0
^ permalink raw reply related
* Re: [git pull] Input updates for v6.5-rc0
From: pr-tracker-bot @ 2023-06-29 17:34 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ZJtnNxQjJ2yrpwIV@google.com>
The pull request you sent on Tue, 27 Jun 2023 15:48:23 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v6.5-rc0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/86e203edf24bb327ce8fcd3c5c8c6bf530a846df
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [RESEND PATCH 2/2] HID: logitech-hidpp: Add support for the Pro X Superlight
From: Bastien Nocera @ 2023-06-29 18:36 UTC (permalink / raw)
To: Mavroudis Chatzilazaridis, jikos; +Cc: linux-input, benjamin.tissoires, lains
In-Reply-To: <5bdf5680-3b02-1408-330d-ddf0c573896c@protonmail.com>
On Thu, 2023-06-29 at 16:03 +0000, Mavroudis Chatzilazaridis wrote:
> On 2023-06-28 13:50, Bastien Nocera wrote:
> > On Sun, 2023-06-25 at 16:21 +0000, Mavroudis Chatzilazaridis wrote:
> > > This patch adds support for the Pro X Superlight. Tested over
> > > USB.
> >
> > Could you please list which functionality this gains the device?
> > Battery reporting? Hi-res scrolling? Something else?
>
> Sure thing. The device gains battery reporting in both wired and
> wireless mode. Also, without this it's not being recognized by
> something
> like Piper in wireless mode, and thus can't be configured.
Great. Could you add that information in the commit message for a v2?
^ permalink raw reply
* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: Rahul Rameshbabu @ 2023-06-29 18:50 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230629172041.3771-1-hadess@hadess.net>
On Thu, 29 Jun, 2023 19:20:27 +0200 Bastien Nocera <hadess@hadess.net> wrote:
> +static const struct hid_device_id steelseries_devices[] = {
> + { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, USB_DEVICE_ID_STEELSERIES_SRWS1),
> + .driver_data = STEELSERIES_SRWS1 },
> +
> + { /* SteelSeries Arctis 1 Wireless for XBox */
> + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
Shouldn't the USB device id be added to drivers/hid/hid-ids.h?
> + .driver_data = STEELSERIES_ARCTIS_1 },
> +
> { }
> };
> -MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
> -
> -static struct hid_driver steelseries_srws1_driver = {
> - .name = "steelseries_srws1",
> - .id_table = steelseries_srws1_devices,
> -#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
> - (IS_MODULE(CONFIG_LEDS_CLASS) && IS_MODULE(CONFIG_HID_STEELSERIES))
> - .probe = steelseries_srws1_probe,
> - .remove = steelseries_srws1_remove,
> -#endif
> - .report_fixup = steelseries_srws1_report_fixup
> +MODULE_DEVICE_TABLE(hid, steelseries_devices);
Thanks,
Rahul Rameshbabu
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Stuart Hayhurst @ 2023-06-29 19:24 UTC (permalink / raw)
Cc: Stuart Hayhurst, linux-kernel, linux-input, Benjamin Tissoires,
Jiri Kosina, Bastien Nocera, Filipe Laíns
Previously, support for the G502 had been attempted and reverted within this driver.
Since then, a new version of this mouse has been released, and seems to work fine.
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5e1a412fd28f..050bad0f9aca 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4598,6 +4598,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* Logitech G403 Wireless Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
+ { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
{ /* Logitech G703 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
{ /* Logitech G703 Hero Gaming Mouse over USB */
--
2.40.1.521.gf1e218fcd8
^ permalink raw reply related
* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: kernel test robot @ 2023-06-29 19:49 UTC (permalink / raw)
To: Bastien Nocera, linux-input
Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230629172041.3771-1-hadess@hadess.net>
Hi Bastien,
kernel test robot noticed the following build errors:
[auto build test ERROR on hid/for-next]
[also build test ERROR on v6.4 next-20230629]
[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/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230630-012252
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230629172041.3771-1-hadess%40hadess.net
patch subject: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
config: parisc-randconfig-r015-20230629 (https://download.01.org/0day-ci/archive/20230630/202306300309.WEpmiIaW-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230630/202306300309.WEpmiIaW-lkp@intel.com/reproduce)
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/202306300309.WEpmiIaW-lkp@intel.com/
All errors (new ones prefixed by >>, old ones prefixed by <<):
>> ERROR: modpost: "usb_set_wireless_status" [drivers/hid/hid-steelseries.ko] undefined!
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Bastien Nocera @ 2023-06-29 20:00 UTC (permalink / raw)
To: Stuart Hayhurst
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Filipe Laíns
In-Reply-To: <20230629192422.980071-1-stuart.a.hayhurst@gmail.com>
On Thu, 2023-06-29 at 20:24 +0100, Stuart Hayhurst wrote:
> Previously, support for the G502 had been attempted and reverted
> within this driver.
Can you please mention the commit(s) where this was done?
> Since then, a new version of this mouse has been released, and seems
> to work fine.
Can you please list the features that get enabled by adding this USB ID
to the list in the commit message?
Cheers
>
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 5e1a412fd28f..050bad0f9aca 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4598,6 +4598,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
>
> { /* Logitech G403 Wireless Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
> + { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB
> */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
> { /* Logitech G703 Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
> { /* Logitech G703 Hero Gaming Mouse over USB */
^ permalink raw reply
* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: Bastien Nocera @ 2023-06-29 20:03 UTC (permalink / raw)
To: Rahul Rameshbabu
Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <87wmzm9k7l.fsf@nvidia.com>
On Thu, 2023-06-29 at 11:50 -0700, Rahul Rameshbabu wrote:
> On Thu, 29 Jun, 2023 19:20:27 +0200 Bastien Nocera
> <hadess@hadess.net> wrote:
> > +static const struct hid_device_id steelseries_devices[] = {
> > + { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
> > USB_DEVICE_ID_STEELSERIES_SRWS1),
> > + .driver_data = STEELSERIES_SRWS1 },
> > +
> > + { /* SteelSeries Arctis 1 Wireless for XBox */
> > + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
>
> Shouldn't the USB device id be added to drivers/hid/hid-ids.h?
No, the hid-ids.h is only used if 2 files in the hid directory need to
reference the same device.
Since changes to the way hid drivers are loaded, there's no need to
blocklist the devices in the core to handle them in a driver, so
there's no need to reference the device in another place than the
driver itself, so no need to add it to hid-ids.h
>
> > + .driver_data = STEELSERIES_ARCTIS_1 },
> > +
> > { }
> > };
> > -MODULE_DEVICE_TABLE(hid, steelseries_srws1_devices);
> > -
> > -static struct hid_driver steelseries_srws1_driver = {
> > - .name = "steelseries_srws1",
> > - .id_table = steelseries_srws1_devices,
> > -#if IS_BUILTIN(CONFIG_LEDS_CLASS) || \
> > - (IS_MODULE(CONFIG_LEDS_CLASS) &&
> > IS_MODULE(CONFIG_HID_STEELSERIES))
> > - .probe = steelseries_srws1_probe,
> > - .remove = steelseries_srws1_remove,
> > -#endif
> > - .report_fixup = steelseries_srws1_report_fixup
> > +MODULE_DEVICE_TABLE(hid, steelseries_devices);
>
> Thanks,
>
> Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: Rahul Rameshbabu @ 2023-06-29 20:09 UTC (permalink / raw)
To: Bastien Nocera; +Cc: linux-input, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <8b8d8d78bb8856b933ee92ba2b9c8a256f90d29b.camel@hadess.net>
On Thu, 29 Jun, 2023 22:03:05 +0200 Bastien Nocera <hadess@hadess.net> wrote:
> On Thu, 2023-06-29 at 11:50 -0700, Rahul Rameshbabu wrote:
>> On Thu, 29 Jun, 2023 19:20:27 +0200 Bastien Nocera
>> <hadess@hadess.net> wrote:
>> > +static const struct hid_device_id steelseries_devices[] = {
>> > + { HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES,
>> > USB_DEVICE_ID_STEELSERIES_SRWS1),
>> > + .driver_data = STEELSERIES_SRWS1 },
>> > +
>> > + { /* SteelSeries Arctis 1 Wireless for XBox */
>> > + HID_USB_DEVICE(USB_VENDOR_ID_STEELSERIES, 0x12b6),
>>
>> Shouldn't the USB device id be added to drivers/hid/hid-ids.h?
>
> No, the hid-ids.h is only used if 2 files in the hid directory need to
> reference the same device.
>
> Since changes to the way hid drivers are loaded, there's no need to
> blocklist the devices in the core to handle them in a driver, so
> there's no need to reference the device in another place than the
> driver itself, so no need to add it to hid-ids.h
Makes sense with the fact that the device id is only needed in this
source file. For example, you don't need to recompile the steelcase
driver module when introduce some other unrelated device. Thanks for the
clarification.
-- Rahul Rameshbabu
^ permalink raw reply
* Re: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
From: kernel test robot @ 2023-06-29 20:32 UTC (permalink / raw)
To: Bastien Nocera, linux-input
Cc: oe-kbuild-all, linux-kernel, Jiri Kosina, Benjamin Tissoires
In-Reply-To: <20230629172041.3771-1-hadess@hadess.net>
Hi Bastien,
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 next-20230629]
[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/Bastien-Nocera/HID-steelseries-Add-support-for-Arctis-1-XBox/20230630-012252
base: https://git.kernel.org/pub/scm/linux/kernel/git/hid/hid.git for-next
patch link: https://lore.kernel.org/r/20230629172041.3771-1-hadess%40hadess.net
patch subject: [PATCH] HID: steelseries: Add support for Arctis 1 XBox
config: arc-randconfig-r035-20230629 (https://download.01.org/0day-ci/archive/20230630/202306300419.dmlkSoqD-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230630/202306300419.dmlkSoqD-lkp@intel.com/reproduce)
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/202306300419.dmlkSoqD-lkp@intel.com/
All errors (new ones prefixed by >>):
arceb-elf-ld: drivers/hid/hid-steelseries.o: in function `steelseries_headset_raw_event':
>> hid-steelseries.c:(.text+0x344): undefined reference to `usb_set_wireless_status'
>> arceb-elf-ld: hid-steelseries.c:(.text+0x344): undefined reference to `usb_set_wireless_status'
arceb-elf-ld: drivers/hid/hid-steelseries.o: in function `steelseries_probe':
hid-steelseries.c:(.text+0x790): undefined reference to `usb_set_wireless_status'
arceb-elf-ld: hid-steelseries.c:(.text+0x790): undefined reference to `usb_set_wireless_status'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Stuart Hayhurst @ 2023-06-29 21:40 UTC (permalink / raw)
Cc: Stuart Hayhurst, linux-kernel, linux-input, Benjamin Tissoires,
Jiri Kosina, Bastien Nocera, Filipe Laíns
In-Reply-To: <20230629192422.980071-1-stuart.a.hayhurst@gmail.com>
Previously, support for the G502 had been attempted in commit 27fc32f.
This caused some issues and was reverted by addf338.
Since then, a new version of this mouse has been released (Lightpseed Wireless), and works correctly.
This device has support for battery reporting with the driver
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5e1a412fd28f..94a045ef8e50 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4598,6 +4598,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* Logitech G403 Wireless Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
+ { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
{ /* Logitech G703 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
{ /* Logitech G703 Hero Gaming Mouse over USB */
--
2.40.1.521.gf1e218fcd8
^ permalink raw reply related
* [PATCH v2] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Stuart Hayhurst @ 2023-06-29 21:46 UTC (permalink / raw)
Cc: Stuart Hayhurst, linux-kernel, linux-input, Benjamin Tissoires,
Jiri Kosina, Bastien Nocera, Filipe Laíns
In-Reply-To: <20230629192422.980071-1-stuart.a.hayhurst@gmail.com>
Previously, support for the G502 had been attempted in commit 27fc32f.
This caused some issues and was reverted by addf338.
Since then, a new version of this mouse has been released (Lightpseed Wireless), and works correctly.
This device has support for battery reporting with the driver
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5e1a412fd28f..94a045ef8e50 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4598,6 +4598,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* Logitech G403 Wireless Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
+ { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
{ /* Logitech G703 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
{ /* Logitech G703 Hero Gaming Mouse over USB */
--
2.40.1.521.gf1e218fcd8
^ permalink raw reply related
* Re: [PATCH] Input: elan_i2c - Implement inhibit/uninhibit functions.
From: Dmitry Torokhov @ 2023-06-29 22:57 UTC (permalink / raw)
To: jingle.wu; +Cc: linux-kernel, linux-input, phoenix, josh.chen, dave.wang
In-Reply-To: <20230531090340.1035499-1-jingle.wu@emc.com.tw>
Hi Jingle,
On Wed, May 31, 2023 at 05:03:40PM +0800, jingle.wu wrote:
> Add inhibit/uninhibit functions.
You need to provide justification for this change, i.e. explain why you
need this/what is currently not working or working sub-optimally.
>
> Signed-off-by: Jingle.wu <jingle.wu@emc.com.tw>
> ---
> drivers/input/mouse/elan_i2c_core.c | 207 ++++++++++++++++++++++++++++
> 1 file changed, 207 insertions(+)
>
> diff --git a/drivers/input/mouse/elan_i2c_core.c b/drivers/input/mouse/elan_i2c_core.c
> index 5f0d75a45c80..4ea57f4c7bd4 100644
> --- a/drivers/input/mouse/elan_i2c_core.c
> +++ b/drivers/input/mouse/elan_i2c_core.c
> @@ -56,6 +56,7 @@ struct elan_tp_data {
> struct input_dev *input;
> struct input_dev *tp_input; /* trackpoint input node */
> struct regulator *vcc;
> + struct list_head list; /* for list of devices needing input handler */
>
> const struct elan_transport_ops *ops;
>
> @@ -63,6 +64,11 @@ struct elan_tp_data {
> struct completion fw_completion;
> bool in_fw_update;
>
> + struct work_struct lid_work;
> + bool lid_switch;
> + int lid_value;
> + bool in_inhibit;
> +
> struct mutex sysfs_mutex;
>
> unsigned int max_x;
> @@ -96,6 +102,9 @@ struct elan_tp_data {
> u32 quirks; /* Various quirks */
> };
>
> +static struct workqueue_struct *elan_mode_wq;
> +static LIST_HEAD(elan_devices_with_lid_handler);
> +
> static u32 elan_i2c_lookup_quirks(u16 ic_type, u16 product_id)
> {
> static const struct {
> @@ -329,6 +338,74 @@ static int elan_initialize(struct elan_tp_data *data, bool skip_reset)
> return error;
> }
>
> +static int elan_reactivate(struct elan_tp_data *data)
> +{
> + struct device *dev = &data->client->dev;
> + int error;
> +
> + error = elan_set_power(data, true);
> + if (error)
> + dev_err(dev, "failed to restore power: %d\n", error);
> +
> + error = data->ops->sleep_control(data->client, false);
> + if (error) {
> + dev_err(dev,
> + "failed to wake device up: %d\n", error);
> + return error;
> + }
> +
> + return error;
> +}
> +
> +static int elan_inhibit(struct input_dev *input_dev)
I would rather you did not call it inhibit/uninhibit because this is not
what it does.
Please split the logic for recalibration from logic of powering on and
off the device. I also hope in the future firmware revisions the
distinction will be more clear (i.e. have a separate method/command to
recalibrate baseline on demand).
> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "inhibiting\n");
> + /*
> + * We are taking the mutex to make sure sysfs operations are
> + * complete before we attempt to bring the device into low[er]
> + * power mode.
> + */
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + disable_irq(client->irq);
> +
> + error = elan_set_power(data, false);
> + if (error)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = true;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> +static int elan_uninhibit(struct input_dev *input_dev)
> +{
> + struct elan_tp_data *data = input_get_drvdata(input_dev);
> + struct i2c_client *client = data->client;
> + int error;
> +
> + dev_dbg(&client->dev, "uninhibiting\n");
> + error = mutex_lock_interruptible(&data->sysfs_mutex);
> + if (error)
> + return error;
> +
> + error = elan_reactivate(data);
> + if (error == 0)
> + enable_irq(client->irq);
> +
> + data->in_inhibit = false;
> + mutex_unlock(&data->sysfs_mutex);
> +
> + return error;
> +}
> +
> static int elan_query_device_info(struct elan_tp_data *data)
> {
> int error;
> @@ -1187,6 +1264,124 @@ static void elan_disable_regulator(void *_data)
> regulator_disable(data->vcc);
> }
>
> +static void lid_work_handler(struct work_struct *work)
> +{
> + struct elan_tp_data *data = container_of(work, struct elan_tp_data,
> + lid_work);
> +
> + if (data->lid_value)
> + elan_inhibit(data->input);
> + else
> + elan_uninhibit(data->input);
> +
> +}
> +
> +static void elan_input_lid_event(struct input_handle *handle, unsigned int type,
> + unsigned int code, int value)
> +{
> + struct elan_tp_data *data, *n;
> +
> + if (type == EV_SW && code == SW_LID) {
> + list_for_each_entry_safe(data, n, &elan_devices_with_lid_handler, list) {
Why do you need the "_safe()" variant here?
> + data->lid_value = value;
> + queue_work(elan_mode_wq, &data->lid_work);
> + }
> + }
> +
> +}
> +
> +struct elan_input_lid {
> + struct input_handle handle;
> +};
> +
> +static int elan_input_lid_connect(struct input_handler *handler,
> + struct input_dev *dev,
> + const struct input_device_id *id)
> +{
> + struct elan_input_lid *lid;
> + char *name;
> + int error;
> +
> + lid = kzalloc(sizeof(*lid), GFP_KERNEL);
> + if (!lid)
> + return -ENOMEM;
> + name = kasprintf(GFP_KERNEL, "elan-i2c-lid-%s", dev_name(&dev->dev));
> + if (!name) {
> + error = -ENOMEM;
> + goto err_free_lid;
> + }
> + lid->handle.dev = dev;
> + lid->handle.handler = handler;
> + lid->handle.name = name;
> + lid->handle.private = lid;
> + error = input_register_handle(&lid->handle);
> + if (error)
> + goto err_free_name;
> + error = input_open_device(&lid->handle);
> + if (error)
> + goto err_unregister_handle;
> + return 0;
> +err_unregister_handle:
> + input_unregister_handle(&lid->handle);
> +err_free_name:
> + kfree(name);
> +err_free_lid:
> + kfree(lid);
> + return error;
> +}
> +
> +static void elan_input_lid_disconnect(struct input_handle *handle)
> +{
> + struct elan_input_lid *lid = handle->private;
> +
> + input_close_device(handle);
> + input_unregister_handle(handle);
> + kfree(handle->name);
> + kfree(lid);
> +}
> +
> +static const struct input_device_id elan_input_lid_ids[] = {
> + {
> + .flags = INPUT_DEVICE_ID_MATCH_EVBIT | INPUT_DEVICE_ID_MATCH_SWBIT,
> + .evbit = { BIT_MASK(EV_SW) },
> + .swbit = { [BIT_WORD(SW_LID)] = BIT_MASK(SW_LID) },
> + },
> + { },
> +};
> +
> +static struct input_handler elan_input_lid_handler = {
> + .event = elan_input_lid_event,
> + .connect = elan_input_lid_connect,
> + .disconnect = elan_input_lid_disconnect,
> + .name = "elan-i2c-lid",
> + .id_table = elan_input_lid_ids,
> +};
> +
> +static int elan_create_lid_handler(struct elan_tp_data *data)
> +{
> + int error = 0;
> +
> + elan_mode_wq = create_singlethread_workqueue("elan-i2c-lid");
> + if (elan_mode_wq == NULL)
> + return -ENOMEM;
> + error = input_register_handler(&elan_input_lid_handler);
> + if (error)
> + goto remove_wq;
> +
> + data->lid_switch = true;
> + INIT_LIST_HEAD(&data->list);
> + INIT_WORK(&data->lid_work, lid_work_handler);
> + list_add_tail(&data->list, &elan_devices_with_lid_handler);
It looks like you call elan_create_lid_handler() from elan_probe() which
means it can be called several times (we should not assume there is only
one controller), I do not see it being destroyed in remove() either, so
it will break if you bind/unbind the driver.
I also not sure why you need the list of you have a handler per device.
> +
> + return 0;
> +
> +remove_wq:
> + data->lid_switch = false;
> + destroy_workqueue(elan_mode_wq);
> + elan_mode_wq = NULL;
> + return error;
> +}
> +
> static int elan_probe(struct i2c_client *client)
> {
> const struct elan_transport_ops *transport_ops;
> @@ -1325,6 +1520,10 @@ static int elan_probe(struct i2c_client *client)
> }
> }
>
> + error = elan_create_lid_handler(data);
> + if (error)
> + dev_err(dev, "failed to create lid handler: %d\n", error);
Do we need this on _ALL_ devices with ELan controllers, or just certain
ones? If we need this on all devices how did it work before?
> +
> return 0;
> }
>
> @@ -1334,6 +1533,10 @@ static int elan_suspend(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int ret;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> /*
> * We are taking the mutex to make sure sysfs operations are
> * complete before we attempt to bring the device into low[er]
> @@ -1371,6 +1574,10 @@ static int elan_resume(struct device *dev)
> struct elan_tp_data *data = i2c_get_clientdata(client);
> int error;
>
> + /* Wait for switch on completion */
> + if (data->lid_switch)
> + flush_workqueue(elan_mode_wq);
> +
> if (!device_may_wakeup(dev)) {
> error = regulator_enable(data->vcc);
> if (error) {
> --
> 2.34.1
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Bastien Nocera @ 2023-06-30 11:13 UTC (permalink / raw)
To: Stuart Hayhurst
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Filipe Laíns
In-Reply-To: <20230629214602.987600-1-stuart.a.hayhurst@gmail.com>
On Thu, 2023-06-29 at 22:46 +0100, Stuart Hayhurst wrote:
> Previously, support for the G502 had been attempted in commit
> 27fc32f.
> This caused some issues and was reverted by addf338.
> Since then, a new version of this mouse has been released (Lightpseed
> Wireless), and works correctly.
Sorry to do this to you, but you might need to rework the description
again. The kernel maintainers are pretty strict when it comes to commit
references.
"If you want to refer to a specific commit, don't just refer to the
SHA-1 ID of the commit. Please also include the oneline summary of the
commit, to make it easier for reviewers to know what it is about."
https://docs.kernel.org/process/submitting-patches.html#describe-your-changes
So 12-character short SHA-1 and the subject of the commit.
./scripts/checkpatch.pl should warn about that.
Should be:
27fc32fd9417 ("HID: logitech-hidpp: add USB PID for a few more
supported mice"
and
addf3382c47c ("Revert "HID: logitech-hidpp: add USB PID for a few more
supported mice"")
But you'll want to check those :)
>
> This device has support for battery reporting with the driver
>
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 5e1a412fd28f..94a045ef8e50 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4598,6 +4598,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
>
> { /* Logitech G403 Wireless Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
> + { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB
> */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
> { /* Logitech G703 Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
> { /* Logitech G703 Hero Gaming Mouse over USB */
^ permalink raw reply
* [PATCH v3] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Stuart Hayhurst @ 2023-06-30 11:38 UTC (permalink / raw)
Cc: Stuart Hayhurst, linux-kernel, linux-input, Benjamin Tissoires,
Jiri Kosina, Bastien Nocera, Filipe Laíns
Previously, support for the G502 had been attempted in commit
'27fc32fd9417 ("HID: logitech-hidpp: add USB PID for a few more supported mice")'
This caused some issues and was reverted by
'addf3382c47c ("Revert "HID: logitech-hidpp: add USB PID for a few more
supported mice"")'.
Since then, a new version of this mouse has been released (Lightpseed Wireless), and works correctly.
This device has support for battery reporting with the driver
Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
---
Changes:
v3:
- Add git commit titles
v2:
- Add commit references
---
drivers/hid/hid-logitech-hidpp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index 5e1a412fd28f..94a045ef8e50 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4598,6 +4598,8 @@ static const struct hid_device_id hidpp_devices[] = {
{ /* Logitech G403 Wireless Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
+ { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB */
+ HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
{ /* Logitech G703 Gaming Mouse over USB */
HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
{ /* Logitech G703 Hero Gaming Mouse over USB */
--
2.40.1.521.gf1e218fcd8
^ permalink raw reply related
* Re: [PATCH v3] HID: logitech-hidpp: Add wired USB id for Logitech G502 Lightspeed
From: Bastien Nocera @ 2023-06-30 11:51 UTC (permalink / raw)
To: Stuart Hayhurst
Cc: linux-kernel, linux-input, Benjamin Tissoires, Jiri Kosina,
Filipe Laíns
In-Reply-To: <20230630113818.13005-1-stuart.a.hayhurst@gmail.com>
On Fri, 2023-06-30 at 12:38 +0100, Stuart Hayhurst wrote:
> Previously, support for the G502 had been attempted in commit
> '27fc32fd9417 ("HID: logitech-hidpp: add USB PID for a few more
> supported mice")'
>
> This caused some issues and was reverted by
> 'addf3382c47c ("Revert "HID: logitech-hidpp: add USB PID for a few
> more
> supported mice"")'.
>
> Since then, a new version of this mouse has been released (Lightpseed
> Wireless), and works correctly.
>
> This device has support for battery reporting with the driver
>
> Signed-off-by: Stuart Hayhurst <stuart.a.hayhurst@gmail.com>
Reviewed-by: Bastien Nocera <hadess@hadess.net>
> ---
>
> Changes:
> v3:
> - Add git commit titles
> v2:
> - Add commit references
>
> ---
> drivers/hid/hid-logitech-hidpp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-
> logitech-hidpp.c
> index 5e1a412fd28f..94a045ef8e50 100644
> --- a/drivers/hid/hid-logitech-hidpp.c
> +++ b/drivers/hid/hid-logitech-hidpp.c
> @@ -4598,6 +4598,8 @@ static const struct hid_device_id
> hidpp_devices[] = {
>
> { /* Logitech G403 Wireless Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC082) },
> + { /* Logitech G502 Lightspeed Wireless Gaming Mouse over USB
> */
> + HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC08D) },
> { /* Logitech G703 Gaming Mouse over USB */
> HID_USB_DEVICE(USB_VENDOR_ID_LOGITECH, 0xC087) },
> { /* Logitech G703 Hero Gaming Mouse over USB */
^ 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