* [PATCH v3 3/4] Input: st1232 - add overlay touchscreen and buttons handling
From: Javier Carrasco @ 2023-06-16 7:28 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v3-0-b4fb7fc4bab7@wolfvision.net>
Use ts-overlay to support overlay objects such as buttons and resized
frames defined in the device tree.
If a overlay-touchscreen is provided, ignore touch events outside of
the area defined by its properties. Otherwise, transform the event
coordinates to the overlay-touchscreen x and y-axis if required.
If buttons are provided, register an additional device to report key
events separately. A key event will be generated if the coordinates
of a touch event are within the area defined by the button properties.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
drivers/input/touchscreen/Kconfig | 1 +
drivers/input/touchscreen/st1232.c | 87 ++++++++++++++++++++++++++++++--------
2 files changed, 70 insertions(+), 18 deletions(-)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 8382a4d68326..202e559371e8 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -1215,6 +1215,7 @@ config TOUCHSCREEN_SIS_I2C
config TOUCHSCREEN_ST1232
tristate "Sitronix ST1232 or ST1633 touchscreen controllers"
depends on I2C
+ select TOUCHSCREEN_TS_OVERLAY
help
Say Y here if you want to support the Sitronix ST1232
or ST1633 touchscreen controller.
diff --git a/drivers/input/touchscreen/st1232.c b/drivers/input/touchscreen/st1232.c
index f49566dc96f8..7d8a69e4831b 100644
--- a/drivers/input/touchscreen/st1232.c
+++ b/drivers/input/touchscreen/st1232.c
@@ -22,6 +22,7 @@
#include <linux/pm_qos.h>
#include <linux/slab.h>
#include <linux/types.h>
+#include <linux/input/ts-overlay.h>
#define ST1232_TS_NAME "st1232-ts"
#define ST1633_TS_NAME "st1633-ts"
@@ -56,6 +57,8 @@ struct st1232_ts_data {
struct touchscreen_properties prop;
struct dev_pm_qos_request low_latency_req;
struct gpio_desc *reset_gpio;
+ struct input_dev *overlay_keypad;
+ struct ts_overlay_map *map;
const struct st_chip_info *chip_info;
int read_buf_len;
u8 *read_buf;
@@ -129,10 +132,12 @@ static int st1232_ts_read_resolution(struct st1232_ts_data *ts, u16 *max_x,
static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
{
- struct input_dev *input = ts->input_dev;
+ struct input_dev *tscreen = ts->input_dev;
+ __maybe_unused struct input_dev *keypad = ts->overlay_keypad;
struct input_mt_pos pos[ST_TS_MAX_FINGERS];
u8 z[ST_TS_MAX_FINGERS];
int slots[ST_TS_MAX_FINGERS];
+ __maybe_unused bool button_pressed[ST_TS_MAX_FINGERS];
int n_contacts = 0;
int i;
@@ -143,6 +148,15 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
unsigned int x = ((buf[0] & 0x70) << 4) | buf[1];
unsigned int y = ((buf[0] & 0x07) << 8) | buf[2];
+ /* forward button presses to the keypad input device */
+ if (ts_overlay_is_button_slot(ts->map, i) ||
+ ts_overlay_button_press(ts->map, keypad, x, y, i)) {
+ button_pressed[i] = true;
+ continue;
+ }
+ /* Ignore events out of the overlay screen if defined */
+ if (!ts_overlay_mt_on_touchscreen(ts->map, &x, &y))
+ continue;
touchscreen_set_mt_pos(&pos[n_contacts],
&ts->prop, x, y);
@@ -154,18 +168,25 @@ static int st1232_ts_parse_and_report(struct st1232_ts_data *ts)
}
}
- input_mt_assign_slots(input, slots, pos, n_contacts, 0);
+ input_mt_assign_slots(tscreen, slots, pos, n_contacts, 0);
for (i = 0; i < n_contacts; i++) {
- input_mt_slot(input, slots[i]);
- input_mt_report_slot_state(input, MT_TOOL_FINGER, true);
- input_report_abs(input, ABS_MT_POSITION_X, pos[i].x);
- input_report_abs(input, ABS_MT_POSITION_Y, pos[i].y);
+ input_mt_slot(tscreen, slots[i]);
+ input_mt_report_slot_state(tscreen, MT_TOOL_FINGER, true);
+ input_report_abs(tscreen, ABS_MT_POSITION_X, pos[i].x);
+ input_report_abs(tscreen, ABS_MT_POSITION_Y, pos[i].y);
if (ts->chip_info->have_z)
- input_report_abs(input, ABS_MT_TOUCH_MAJOR, z[i]);
+ input_report_abs(tscreen, ABS_MT_TOUCH_MAJOR, z[i]);
+ }
+ input_mt_sync_frame(tscreen);
+ input_sync(tscreen);
+
+ if (ts_overlay_mapped_buttons(ts->map)) {
+ for (i = 0; i < ts->chip_info->max_fingers; i++)
+ if (ts_overlay_is_button_slot(ts->map, i) &&
+ !button_pressed[i])
+ ts_overlay_button_release(ts->map, keypad, i);
+ input_sync(keypad);
}
-
- input_mt_sync_frame(input);
- input_sync(input);
return n_contacts;
}
@@ -226,6 +247,7 @@ static int st1232_ts_probe(struct i2c_client *client)
const struct st_chip_info *match;
struct st1232_ts_data *ts;
struct input_dev *input_dev;
+ struct input_dev __maybe_unused *overlay_keypad;
u16 max_x, max_y;
int error;
@@ -292,18 +314,28 @@ static int st1232_ts_probe(struct i2c_client *client)
if (error)
return error;
- /* Read resolution from the chip */
- error = st1232_ts_read_resolution(ts, &max_x, &max_y);
- if (error) {
- dev_err(&client->dev,
- "Failed to read resolution: %d\n", error);
- return error;
- }
-
if (ts->chip_info->have_z)
input_set_abs_params(input_dev, ABS_MT_TOUCH_MAJOR, 0,
ts->chip_info->max_area, 0, 0);
+ /* map overlay objects if defined in the device tree */
+ ts->map = ts_overlay_map_objects(&ts->client->dev, ts->input_dev);
+ if (IS_ERR(ts->map))
+ return PTR_ERR(ts->map);
+
+ if (ts_overlay_mapped_touchscreen(ts->map)) {
+ /* Read resolution from the overlay touchscreen if defined*/
+ ts_overlay_get_touchscreen_abs(ts->map, &max_x, &max_y);
+ } else {
+ /* Read resolution from the chip */
+ error = st1232_ts_read_resolution(ts, &max_x, &max_y);
+ if (error) {
+ dev_err(&client->dev,
+ "Failed to read resolution: %d\n", error);
+ return error;
+ }
+ }
+
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, max_x, 0, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -335,6 +367,25 @@ static int st1232_ts_probe(struct i2c_client *client)
return error;
}
+ /* Register keypad input device if overlay buttons were defined */
+ if (ts_overlay_mapped_buttons(ts->map)) {
+ overlay_keypad = devm_input_allocate_device(&client->dev);
+ if (!overlay_keypad)
+ return -ENOMEM;
+
+ ts->overlay_keypad = overlay_keypad;
+ overlay_keypad->name = "st1232-keypad";
+ overlay_keypad->id.bustype = BUS_I2C;
+ ts_overlay_set_button_caps(ts->map, overlay_keypad);
+ error = input_register_device(ts->overlay_keypad);
+ if (error) {
+ dev_err(&client->dev,
+ "Unable to register %s input device\n",
+ overlay_keypad->name);
+ return error;
+ }
+ }
+
i2c_set_clientdata(client, ts);
return 0;
--
2.39.2
^ permalink raw reply related
* [PATCH v3 0/4] Input: support overlay objects on touchscreens
From: Javier Carrasco @ 2023-06-16 7:28 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
Some touchscreens are shipped with a physical layer on top of them where
a number of buttons and a resized touchscreen surface might be available.
In order to generate proper key events by overlay buttons and adjust the
touch events to a clipped surface, these patches offer a documented,
device-tree-based solution by means of helper functions.
An implementation for a specific touchscreen driver is also included.
The functions in ts-overlay provide a simple workflow to acquire
physical objects from the device tree, map them into the device driver
structures as overlay objects and generate events according to
the object descriptions.
This feature has been tested with a JT240MHQS-E3 display, which consists
of an st1624 as the base touchscreen and an overlay with two buttons and
a frame that clips its effective surface mounted on it.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
Changes in v3:
- General: rename "virtobj" and "virtual" to "overlay"
- PATCH 1/4: Make feature bool instead of tristate (selected by
supported touchscreens)
- Link to v2: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v2-0-f68a6bfe7a0f@wolfvision.net
Changes in v2:
- PATCH 1/4: remove preprocessor directives (the module is selected by
the drivers that support the feature). Typo in the commit message.
- PATCH 2/4: more detailed documentation. Images and examples were added.
- PATCH 3/4: select ts-virtobj automatically.
- Link to v1: https://lore.kernel.org/r/20230510-feature-ts_virtobj_patch-v1-0-5ae5e81bc264@wolfvision.net
---
Javier Carrasco (4):
Input: ts-overlay - Add touchscreen overlay object handling
dt-bindings: touchscreen: add overlay-touchscreen and overlay-buttons properties
Input: st1232 - add overlay touchscreen and buttons handling
dt-bindings: input: touchscreen: st1232: add example with ts-overlay
.../input/touchscreen/sitronix,st1232.yaml | 40 +++
.../bindings/input/touchscreen/touchscreen.yaml | 139 ++++++++
MAINTAINERS | 7 +
drivers/input/touchscreen/Kconfig | 10 +
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/st1232.c | 87 +++--
drivers/input/touchscreen/ts-overlay.c | 356 +++++++++++++++++++++
include/linux/input/ts-overlay.h | 43 +++
8 files changed, 665 insertions(+), 18 deletions(-)
---
base-commit: ac9a78681b921877518763ba0e89202254349d1b
change-id: 20230510-feature-ts_virtobj_patch-e267540aae74
Best regards,
--
Javier Carrasco <javier.carrasco@wolfvision.net>
^ permalink raw reply
* [PATCH v3 4/4] dt-bindings: input: touchscreen: st1232: add example with ts-overlay
From: Javier Carrasco @ 2023-06-16 7:28 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Henrik Rydberg, Bastian Hecht, Michael Riesch
Cc: linux-kernel, linux-input, devicetree, Javier Carrasco
In-Reply-To: <20230510-feature-ts_virtobj_patch-v3-0-b4fb7fc4bab7@wolfvision.net>
The st1232 driver supports the overlay-touchscreen and overlay-buttons
objects defined in the generic touchscreen bindings and implemented in
the ts-overlay module. Add an example where nodes for a overlay
touchscreen and overlay buttons are defined.
Signed-off-by: Javier Carrasco <javier.carrasco@wolfvision.net>
---
.../input/touchscreen/sitronix,st1232.yaml | 40 ++++++++++++++++++++++
1 file changed, 40 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
index 1d8ca19fd37a..6cc34e4c6c87 100644
--- a/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
+++ b/Documentation/devicetree/bindings/input/touchscreen/sitronix,st1232.yaml
@@ -48,3 +48,43 @@ examples:
gpios = <&gpio1 166 0>;
};
};
+ - |
+ #include <dt-bindings/input/linux-event-codes.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ touchscreen@55 {
+ compatible = "sitronix,st1232";
+ reg = <0x55>;
+ interrupts = <2 0>;
+ gpios = <&gpio1 166 0>;
+
+ overlay-touchscreen {
+ x-origin = <0>;
+ x-size = <240>;
+ y-origin = <40>;
+ y-size = <280>;
+ };
+
+ overlay-buttons {
+ button-light {
+ label = "Camera light";
+ linux,code = <KEY_LIGHTS_TOGGLE>;
+ x-origin = <40>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+
+ button-suspend {
+ label = "Suspend";
+ linux,code = <KEY_SUSPEND>;
+ x-origin = <160>;
+ x-size = <40>;
+ y-origin = <0>;
+ y-size = <40>;
+ };
+ };
+ };
+ };
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v2 2/4] input: touchscreen: add core support for Goodix Berlin Touchscreen IC
From: Jeff LaBundy @ 2023-06-16 3:14 UTC (permalink / raw)
To: Neil Armstrong
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg, linux-input,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v2-2-26bc8fe1e90e@linaro.org>
Hi Neil,
Great work; this driver cleaned up quite nicely. With the previous
noise out of the way, I have left some more detailed comments which
are relatively minor for the most part.
On Thu, Jun 15, 2023 at 12:27:01PM +0200, Neil Armstrong wrote:
> 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
>
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> drivers/input/touchscreen/Kconfig | 5 +
> drivers/input/touchscreen/Makefile | 1 +
> drivers/input/touchscreen/goodix_berlin.h | 178 +++++++
> drivers/input/touchscreen/goodix_berlin_core.c | 681 +++++++++++++++++++++++++
> 4 files changed, 865 insertions(+)
>
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index c2cbd332af1d..1a6f6f6da991 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -416,6 +416,11 @@ config TOUCHSCREEN_GOODIX
> To compile this driver as a module, choose M here: the
> module will be called goodix.
>
> +config TOUCHSCREEN_GOODIX_BERLIN_CORE
> + depends on GPIOLIB || COMPILE_TEST
No need to depend on GPIOLIB; all gpiod calls used in this driver define dummy
functions for the !CONFIG_GPIOLIB case.
> + depends on REGMAP
> + 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..56c110d94dff
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix_berlin.h
> @@ -0,0 +1,178 @@
> +/* 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/input.h>
> +#include <linux/of_gpio.h>
I believe this should have been <linux/gpio/consumer.h>.
> +#include <linux/input/touchscreen.h>
> +#include <linux/regulator/consumer.h>
> +
> +#define GOODIX_MAX_TOUCH 10
> +
> +#define GOODIX_NORMAL_RESET_DELAY_MS 100
> +
> +#define MAX_SCAN_FREQ_NUM 8
> +#define MAX_SCAN_RATE_NUM 8
> +#define MAX_FREQ_NUM_STYLUS 8
> +
> +#define IRQ_EVENT_HEAD_LEN 8
> +#define BYTES_PER_POINT 8
> +#define COOR_DATA_CHECKSUM_SIZE 2
> +
> +#define GOODIX_TOUCH_EVENT BIT(7)
> +#define GOODIX_REQUEST_EVENT BIT(6)
> +
> +#define GOODIX_REQUEST_CODE_RESET 3
> +
> +#define POINT_TYPE_STYLUS_HOVER 0x01
> +#define POINT_TYPE_STYLUS 0x03
> +
> +#define DEV_CONFIRM_VAL 0xAA
> +#define BOOTOPTION_ADDR 0x10000
> +#define FW_VERSION_INFO_ADDR 0x10014
> +
> +#define GOODIX_IC_INFO_MAX_LEN 1024
> +#define GOODIX_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];
> + u16 checksum;
> +} __packed;
> +
> +struct goodix_berlin_ic_info_version {
> + u8 info_customer_id;
> + u8 info_version_id;
> + u8 ic_die_id;
> + u8 ic_version_id;
> + u32 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 {
> + u16 freqhop_feature;
> + u16 calibration_feature;
> + u16 gesture_feature;
> + u16 side_touch_feature;
> + u16 stylus_feature;
> +} __packed;
> +
> +struct goodix_berlin_ic_info_misc {
> + u32 cmd_addr;
> + u16 cmd_max_len;
> + u32 cmd_reply_addr;
> + u16 cmd_reply_len;
> + u32 fw_state_addr;
> + u16 fw_state_len;
> + u32 fw_buffer_addr;
> + u16 fw_buffer_max_len;
> + u32 frame_data_addr;
> + u16 frame_data_head_len;
> + u16 fw_attr_len;
> + u16 fw_log_len;
> + u8 pack_max_num;
> + u8 pack_compress_version;
> + u16 stylus_struct_len;
> + u16 mutual_struct_len;
> + u16 self_struct_len;
> + u16 noise_struct_len;
> + u32 touch_data_addr;
> + u16 touch_data_head_len;
> + u16 point_struct_len;
> + u16 reserved1;
> + u16 reserved2;
> + u32 mutual_rawdata_addr;
> + u32 mutual_diffdata_addr;
> + u32 mutual_refdata_addr;
> + u32 self_rawdata_addr;
> + u32 self_diffdata_addr;
> + u32 self_refdata_addr;
> + u32 iq_rawdata_addr;
> + u32 iq_refdata_addr;
> + u32 im_rawdata_addr;
> + u16 im_readata_len;
> + u32 noise_rawdata_addr;
> + u16 noise_rawdata_len;
> + u32 stylus_rawdata_addr;
> + u16 stylus_rawdata_len;
> + u32 noise_data_addr;
> + u32 esd_addr;
> +} __packed;
> +
> +enum goodix_berlin_ts_event_type {
> + GOODIX_BERLIN_EVENT_INVALID,
> + GOODIX_BERLIN_EVENT_TOUCH,
> + GOODIX_BERLIN_EVENT_REQUEST,
> +};
> +
> +enum goodix_berlin_ts_request_type {
> + GOODIX_BERLIN_REQUEST_TYPE_INVALID,
> + GOODIX_BERLIN_REQUEST_TYPE_RESET,
> +};
> +
> +enum goodix_berlin_touch_point_status {
> + GOODIX_BERLIN_TS_NONE,
> + GOODIX_BERLIN_TS_TOUCH,
> +};
> +
> +struct goodix_berlin_coords {
> + enum goodix_berlin_touch_point_status status;
> + unsigned int x, y, w, p;
> +};
> +
> +struct goodix_berlin_touch_data {
> + int touch_num;
> + struct goodix_berlin_coords coords[GOODIX_MAX_TOUCH];
> +};
> +
> +struct goodix_berlin_event {
> + enum goodix_berlin_ts_event_type event_type;
> + enum goodix_berlin_ts_request_type request_code;
> + struct goodix_berlin_touch_data touch_data;
> +};
> +
> +/* Runtime parameters extracted from goodix_berlin_ic_info */
> +struct goodix_berlin_params {
> + u32 touch_data_addr;
> +};
Is there any reason to wrap this single member in a struct? It seems like
touch_data_addr can simply be a member of struct goodix_berlin_core; this
would shorten references to it throughout as well.
> +
> +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 goodix_berlin_params params;
> + struct input_dev *input_dev;
> + struct goodix_berlin_event ts_event;
> + int irq;
> + struct dentry *debugfs_root;
This last member appears to be unused.
> +};
> +
> +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..11788662722a
> --- /dev/null
> +++ b/drivers/input/touchscreen/goodix_berlin_core.c
> @@ -0,0 +1,681 @@
> +// 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 <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> +#include <linux/regmap.h>
> +
> +#include "goodix_berlin.h"
> +
> +/*
> + * Goodix "Berlin" Touchscreen ID driver
> + *
> + * 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 & C)
> + */
> +
> +static bool goodix_berlin_check_checksum(const u8 *data, int size)
Based on how this function is used later, something along the lines of
goodix_berlin_checksum_valid() may be more descriptive.
> +{
> + u32 cal_checksum = 0;
> + u32 r_checksum = 0;
This can probably just be:
u16 r_checksum;
> + u32 i;
> +
> + if (size < COOR_DATA_CHECKSUM_SIZE)
> + return false;
> +
> + for (i = 0; i < size - COOR_DATA_CHECKSUM_SIZE; i++)
> + cal_checksum += data[i];
> +
> + r_checksum += data[i++];
> + r_checksum += (data[i] << 8);
And then:
r_checksum = put_unaligned_le16(data[i]);
In which case you must #include <asm/unaligned.h>.
> +
> + return (cal_checksum & 0xFFFF) == r_checksum;
> +}
> +
> +static bool goodix_berlin_is_dummy_data(struct goodix_berlin_core *cd,
> + const u8 *data, int size)
> +{
> + int zero_count = 0;
> + int ff_count = 0;
'zero_count' and 'ff_count' seem like odd variable names; the following
seems cleaner:
static bool goodix_berlin_is_dummy_data(...)
{
int i;
/*
* 0 and 0xff represent ____, 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;
}
...with the comment filled in to clarify what is the significance of dummy
data, if possible. Note that the caller already prints a message when this
fails.
> + int i;
> +
> + for (i = 0; i < size; i++) {
> + if (data[i] == 0)
> + zero_count++;
> + else if (data[i] == 0xff)
> + ff_count++;
> + }
> + if (zero_count == size || ff_count == size) {
> + dev_warn(cd->dev, "warning data is all %s\n",
> + zero_count == size ? "zero" : "0xff");
> + return true;
> + }
> +
> + return false;
> +}
> +
> +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, DEV_CONFIRM_VAL, sizeof(tx_buf));
Please namespace this #define as well as the one below.
> + while (retry--) {
> + error = regmap_raw_write(cd->regmap, BOOTOPTION_ADDR, tx_buf,
> + sizeof(tx_buf));
> + if (error < 0)
> + return error;
This should just be:
if (error)
return error;
> +
> + error = regmap_raw_read(cd->regmap, BOOTOPTION_ADDR, rx_buf,
> + sizeof(rx_buf));
> + if (error < 0)
> + return error;
And here, as well as a few other places throughout.
> +
> + 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;
No need to initialize 'error' here.
> +
> + if (on) {
> + error = regulator_enable(cd->iovdd);
> + if (error < 0) {
> + 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 < 0) {
> + dev_err(cd->dev, "Failed to enable avdd: %d\n", error);
> + goto power_off_iovdd;
> + }
> +
> + /* 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 < 0)
> + goto power_off_gpio;
All of this cleaned up nicely. The following comment is idiomatic, but I feel
the goto can be easily eliminated as follows:
error = goodix_berlin_dev_confirm(cd);
if (error)
break;
If you feel strongly otherwise, please consider a different label name beside
'power_off_gpio' as it is a bit confusing.
> +
> + /* Vendor waits 100ms for Firmware to fully boot */
> + msleep(GOODIX_NORMAL_RESET_DELAY_MS);
> +
> + return 0;
> + }
> +
> +power_off_gpio:
> + gpiod_set_value(cd->reset_gpio, 1);
> +
> + regulator_disable(cd->avdd);
> +
> +power_off_iovdd:
> + 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 retry = 2;
> + int error;
> +
> + while (retry--) {
> + error = regmap_raw_read(cd->regmap, FW_VERSION_INFO_ADDR, buf, sizeof(buf));
> + if (error) {
> + dev_dbg(cd->dev, "read fw version: %d, retry %d\n", error, retry);
> + usleep_range(5000, 5100);
> + continue;
> + }
> +
> + if (goodix_berlin_check_checksum(buf, sizeof(buf)))
> + break;
> +
> + dev_dbg(cd->dev, "invalid fw version: checksum error\n");
> +
> + error = -EINVAL;
> +
> + /* Do not sleep on the last try */
> + if (retry)
> + usleep_range(10000, 11000);
This works, but do you reasonably expect to continue if the checksum is bad?
Perhaps the device can still respond over I2C/SPI, but returns garbage data
if the communication happens in the process of the device waking up?
If so, this may be even cleaner:
if (retry)
usleep_range(...);
else
error = -EINVAL;
> + }
> +
> + if (error) {
> + dev_err(cd->dev, "failed to get fw version");
> + return error;
> + }
Again the following comment is idiomatic, but this seems cleaner:
if (error)
dev_err(...);
else
memcpy(...);
return error;
I do not feel strongly about it if you prefer the existing method.
> +
> + 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)
Nit: these arguments do not seem aligned as below:
+static int goodix_berlin_convert_ic_info(...,
...)
> +{
> + 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);
> +
> + /* goodix_berlin_ic_info_param, variable width structure */
I don't see any need to make up this name 'goodix_berlin_ic_info_param' which
is not defined anywhere else; a generic text description of what this area of
memory represents seems fine.
> + 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;
Do you actually need this check after every operation? It seems the error
would be cumulative such that you could perform one check at the end, and
potentially avoid a goto statement like below:
offset += ...; /* foo */
offset += ---; /* bar */
if (offset > length) {
dev_err(...);
return -EINVAL);
}
return 0;
In case I have misunderstood, please let me know.
> +
> + 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->params.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)
> +{
> + int error, i;
> + u16 length = 0;
> + u32 ic_addr;
> + u8 afe_data[GOODIX_IC_INFO_MAX_LEN] = { 0 };
No need to initialize this array.
> +
> + ic_addr = GOODIX_IC_INFO_ADDR;
> +
> + for (i = 0; i < 3; i++) {
> + error = regmap_raw_read(cd->regmap, ic_addr, (u8 *)&length, sizeof(length));
> + if (error) {
> + dev_info(cd->dev, "failed get ic info length, %d\n", error);
> + usleep_range(5000, 5100);
> + continue;
> + }
> +
> + length = le16_to_cpu(length);
This seems incorrect; it seems like you mean to initialize the following:
__le16 length_raw;
u16 length;
And then this becomes:
length = le16_to_cpu(length_raw);
> + if (length >= GOODIX_IC_INFO_MAX_LEN) {
> + dev_info(cd->dev, "invalid ic info length %d, retry %d\n", length, i);
> + continue;
> + }
> +
> + error = regmap_raw_read(cd->regmap, ic_addr, afe_data, length);
> + if (error) {
> + dev_info(cd->dev, "failed get ic info data, %d\n", error);
> + usleep_range(5000, 5100);
> + continue;
> + }
> +
> + /* check whether the data is valid (ex. bus default values) */
> + if (goodix_berlin_is_dummy_data(cd, (const uint8_t *)afe_data, length)) {
> + dev_info(cd->dev, "fw info data invalid\n");
> + usleep_range(5000, 5100);
> + continue;
> + }
> +
> + if (!goodix_berlin_check_checksum((const uint8_t *)afe_data, length)) {
> + dev_info(cd->dev, "fw info checksum error\n");
> + usleep_range(5000, 5100);
> + continue;
> + }
> +
> + break;
> + }
Even after the loop gives up, we will still sleep for 5 ms; it seems we can
simplify this. One option may be to offload the function calls into a helper
like below:
static int __goodix_berlin_get_ic_info(...);
{
/* ... */
error = regmap_raw_read(...);
if (error)
return error;
length = le16_to_cpu(...);
if (...)
return -EINVAL;
if (goodix_berlin_is_dummy_data(...))
return -EAGAIN;
if (!goodix_berlin_check_checksum(...));
return -EAGAIN;
return 0;
}
...and then:
static int goodix_berlin_get_ic_info(...)
{
/* ... */
for (i = 0; i = GOODIX_BERLIN_INFO_RETRIES; i++) {
error = __goodix_berlin_get_ic_info(...);
if (!error)
break;
elseif (error == -EINVAL)
return error;
usleep_range(...);
}
if (i == GOODIX_BERLIN_INFO_RETRIES) {
dev_err(...);
return -ETIMEDOUT;
}
/* ... */
}
> + if (i == 3) {
> + dev_err(cd->dev, "failed get ic info\n");
> + return -EINVAL;
> + }
> +
> + error = goodix_berlin_convert_ic_info(cd, afe_data, length);
> + if (error) {
> + dev_err(cd->dev, "error converting ic info\n");
> + return error;
> + }
> +
> + /* check some key info */
> + if (!cd->params.touch_data_addr) {
> + dev_err(cd->dev, "touch_data_addr is null\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int goodix_berlin_after_event_handler(struct goodix_berlin_core *cd)
> +{
> + u8 sync_clean = 0;
> +
> + return regmap_raw_write(cd->regmap, cd->params.touch_data_addr, &sync_clean, 1);
Since your register data width is 8 bits, can this not simply be:
regmap_write(cd->regmap, params.touch_data_addr, 0);
> +}
> +
> +static void goodix_berlin_parse_finger(struct goodix_berlin_core *cd,
> + struct goodix_berlin_touch_data *touch_data,
> + u8 *buf, int touch_num)
> +{
> + unsigned int id = 0, x = 0, y = 0, w = 0;
No need to initialize these.
> + u8 *coor_data;
> + int i;
> +
> + coor_data = &buf[IRQ_EVENT_HEAD_LEN];
> +
> + for (i = 0; i < touch_num; i++) {
> + id = (coor_data[0] >> 4) & 0x0F;
> +
> + if (id >= GOODIX_MAX_TOUCH) {
> + dev_warn(cd->dev, "invalid finger id %d\n", id);
> +
> + touch_data->touch_num = 0;
> + return;
> + }
It does not seem this check needs to happen in the loop.
> +
> + x = le16_to_cpup((__le16 *)(coor_data + 2));
> + y = le16_to_cpup((__le16 *)(coor_data + 4));
> + w = le16_to_cpup((__le16 *)(coor_data + 6));
I got a bit lost here; this almost seems like there is an opportunity
to define the coordinate layout as a __packed struct.
It's also a bit confusing that all of this information is stored in
the driver's private data and then passed around multiple functions.
Please consider whether this logic can be simplified by consolidating
it into one homogenous interrupt handler.
> +
> + touch_data->coords[id].status = GOODIX_BERLIN_TS_TOUCH;
> + touch_data->coords[id].x = x;
> + touch_data->coords[id].y = y;
> + touch_data->coords[id].w = w;
> +
> + coor_data += BYTES_PER_POINT;
> + }
> +
> + touch_data->touch_num = touch_num;
> +}
> +
> +static int goodix_berlin_touch_handler(struct goodix_berlin_core *cd,
> + u8 *pre_buf, u32 pre_buf_len)
> +{
> + static u8 buffer[IRQ_EVENT_HEAD_LEN + BYTES_PER_POINT * GOODIX_MAX_TOUCH + 2];
> + struct goodix_berlin_touch_data *touch_data = &cd->ts_event.touch_data;
> + u8 point_type = 0;
> + u8 touch_num = 0;
> + int error = 0;
> +
> + memset(&cd->ts_event, 0, sizeof(cd->ts_event));
> +
> + /* copy pre-data to buffer */
> + memcpy(buffer, pre_buf, pre_buf_len);
> +
> + touch_num = buffer[2] & 0x0F;
> +
> + if (touch_num > GOODIX_MAX_TOUCH) {
> + dev_warn(cd->dev, "invalid touch num %d\n", touch_num);
> + return -EINVAL;
> + }
> +
> + /* read more data if more than 2 touch events */
> + if (unlikely(touch_num > 2)) {
> + error = regmap_raw_read(cd->regmap,
> + cd->params.touch_data_addr + pre_buf_len,
> + &buffer[pre_buf_len],
> + (touch_num - 2) * BYTES_PER_POINT);
> + if (error) {
> + dev_err(cd->dev, "failed get touch data\n");
> + return error;
> + }
> + }
> +
> + goodix_berlin_after_event_handler(cd);
> +
> + if (!touch_num)
> + goto out_touch_handler;
> +
> + point_type = buffer[IRQ_EVENT_HEAD_LEN] & 0x0F;
> +
> + if (point_type == POINT_TYPE_STYLUS || point_type == POINT_TYPE_STYLUS_HOVER) {
> + dev_warn_once(cd->dev, "Stylus event type not handled\n");
> + return 0;
> + }
> +
> + if (!goodix_berlin_check_checksum(&buffer[IRQ_EVENT_HEAD_LEN],
> + touch_num * BYTES_PER_POINT + 2)) {
> + dev_dbg(cd->dev, "touch data checksum error\n");
> + dev_dbg(cd->dev, "data: %*ph\n",
> + touch_num * BYTES_PER_POINT + 2, &buffer[IRQ_EVENT_HEAD_LEN]);
> + return -EINVAL;
> + }
> +
> +out_touch_handler:
> + cd->ts_event.event_type = GOODIX_BERLIN_EVENT_TOUCH;
> + goodix_berlin_parse_finger(cd, touch_data, buffer, touch_num);
> +
> + return 0;
> +}
> +
> +static int goodix_berlin_event_handler(struct goodix_berlin_core *cd)
> +{
> + int pre_read_len;
> + u8 pre_buf[32];
> + u8 event_status;
> + int error;
> +
> + pre_read_len = IRQ_EVENT_HEAD_LEN + BYTES_PER_POINT * 2 +
> + COOR_DATA_CHECKSUM_SIZE;
> +
> + error = regmap_raw_read(cd->regmap, cd->params.touch_data_addr, pre_buf,
> + pre_read_len);
> + if (error) {
> + dev_err(cd->dev, "failed get event head data\n");
> + return error;
> + }
> +
> + if (pre_buf[0] == 0x00)
> + return -EINVAL;
> +
> + if (!goodix_berlin_check_checksum(pre_buf, IRQ_EVENT_HEAD_LEN)) {
> + dev_warn(cd->dev, "touch head checksum err : %*ph\n",
> + IRQ_EVENT_HEAD_LEN, pre_buf);
> + return -EINVAL;
> + }
> +
> + event_status = pre_buf[0];
> + if (event_status & GOODIX_TOUCH_EVENT)
> + return goodix_berlin_touch_handler(cd, pre_buf, pre_read_len);
> +
> + if (event_status & GOODIX_REQUEST_EVENT) {
> + cd->ts_event.event_type = GOODIX_BERLIN_EVENT_REQUEST;
> + if (pre_buf[2] == GOODIX_REQUEST_CODE_RESET)
> + cd->ts_event.request_code = GOODIX_BERLIN_REQUEST_TYPE_RESET;
> + else
> + dev_warn(cd->dev, "unsupported request code 0x%x\n", pre_buf[2]);
> + }
> +
> + goodix_berlin_after_event_handler(cd);
> +
> + return 0;
> +}
> +
> +static void goodix_berlin_report_finger(struct goodix_berlin_core *cd)
> +{
> + struct goodix_berlin_touch_data *touch_data = &cd->ts_event.touch_data;
> + int i;
> +
> + mutex_lock(&cd->input_dev->mutex);
I do not see any need for a mutex here; this function is only ever called from
a threaded handler declared with IRQF_ONESHOT; it cannot become re-entrant. In
case I have misunderstood, please let me know.
> +
> + for (i = 0; i < GOODIX_MAX_TOUCH; i++) {
> + bool pressed = touch_data->coords[i].status == GOODIX_BERLIN_TS_TOUCH;
> +
> + input_mt_slot(cd->input_dev, i);
> + input_mt_report_slot_state(cd->input_dev, MT_TOOL_FINGER, pressed);
> +
> + if (touch_data->coords[i].status == GOODIX_BERLIN_TS_TOUCH) {
> + dev_dbg(cd->dev, "report: id[%d], x %d, y %d, w %d\n", i,
> + touch_data->coords[i].x,
> + touch_data->coords[i].y,
> + touch_data->coords[i].w);
> +
> + touchscreen_report_pos(cd->input_dev, &cd->props,
> + touch_data->coords[i].x,
> + touch_data->coords[i].y, true);
> + input_report_abs(cd->input_dev, ABS_MT_TOUCH_MAJOR,
> + touch_data->coords[i].w);
> + }
> + }
> +
> + input_mt_sync_frame(cd->input_dev);
> + input_sync(cd->input_dev);
> +
> + mutex_unlock(&cd->input_dev->mutex);
> +}
> +
> +static int goodix_berlin_request_handle(struct goodix_berlin_core *cd)
> +{
> + /* TOFIX: Handle more request codes */
> + if (cd->ts_event.request_code != GOODIX_BERLIN_REQUEST_TYPE_RESET) {
> + dev_info(cd->dev, "can't handle request type 0x%x\n",
> + cd->ts_event.request_code);
> + return -EINVAL;
> + }
> +
> + gpiod_set_value(cd->reset_gpio, 1);
> + usleep_range(2000, 2100);
> + gpiod_set_value(cd->reset_gpio, 0);
> +
> + msleep(GOODIX_NORMAL_RESET_DELAY_MS);
> +
> + return 0;
> +}
> +
> +static irqreturn_t goodix_berlin_threadirq_func(int irq, void *data)
> +{
> + struct goodix_berlin_core *cd = data;
> + int error;
> +
> + error = goodix_berlin_event_handler(cd);
> + if (likely(!error)) {
> + switch (cd->ts_event.event_type) {
> + case GOODIX_BERLIN_EVENT_TOUCH:
> + goodix_berlin_report_finger(cd);
> + break;
> +
> + case GOODIX_BERLIN_EVENT_REQUEST:
> + goodix_berlin_request_handle(cd);
> + break;
> +
> + /* TOFIX: Handle more request types */
> + case GOODIX_BERLIN_EVENT_INVALID:
> + break;
> + }
> + }
> +
> + 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->dev.parent = cd->dev;
This last line is not necessary as devm_input_allocate_device() already
does it for us.
> +
> + memcpy(&input_dev->id, id, sizeof(*id));
I do not think deep copy is necessary here; please let me know in case I
have misunderstood.
> +
> + /* Set input parameters */
This comment is not necessary.
> + 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_MAX_TOUCH, INPUT_MT_DIRECT);
> + 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)
> + return -EINVAL;
Please include a dev_err() here.
> +
> + 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(cd->dev, "reset", GPIOF_OUT_INIT_HIGH);
You can simply use 'dev' instead of 'cd->dev' here and throughout.
> + if (IS_ERR(cd->reset_gpio))
> + return dev_err_probe(cd->dev, PTR_ERR(cd->reset_gpio),
> + "Failed to request reset gpio\n");
> +
> + cd->avdd = devm_regulator_get(cd->dev, "avdd");
> + if (IS_ERR(cd->avdd))
> + return dev_err_probe(cd->dev, PTR_ERR(cd->avdd),
> + "Failed to request avdd regulator\n");
> +
> + cd->iovdd = devm_regulator_get(cd->dev, "iovdd");
> + if (IS_ERR(cd->iovdd))
> + return dev_err_probe(cd->dev, PTR_ERR(cd->iovdd),
> + "Failed to request iovdd regulator\n");
> +
> + error = goodix_berlin_input_dev_config(cd, id);
> + if (error < 0) {
> + dev_err(cd->dev, "failed set input device");
> + return error;
> + }
I recommend calling this after goodix_berlin_get_ic_info(); no need to go
through all this trouble if the device is not there.
> +
> + 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;
> + }
Typically, the interrupt should be the last resource to be registered. At
this stage the device is not powered and the IRQ pin may be in an invalid
state.
> +
> + dev_set_drvdata(dev, cd);
> +
> + error = goodix_berlin_power_on(cd, true);
> + if (error) {
> + dev_err(cd->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 < 0) {
if (error)
> + dev_err(cd->dev, "failed to get version info");
> + return error;
> + }
> +
> + error = goodix_berlin_get_ic_info(cd);
> + if (error) {
> + dev_err(cd->dev, "invalid ic info, abort");
> + return error;
> + }
> +
> + dev_dbg(cd->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
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [PATCH RFC 1/1] HID: allow specifying quirks params for hid-core
From: Marco Morandini @ 2023-06-15 21:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Before this patch it was possible to specify quirks
using run-time parameters only for the usbhid module.
This patch introduces a quirks parameter for hid-core
allowing to specify quirks as
hid.quirks=BUS:vendorID:productID:quirks
Signed-off-by: Marco Morandini <marco.morandini@polimi.it>
---
drivers/hid/hid-core.c | 13 +++++++++++++
drivers/hid/hid-quirks.c | 33 +++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
3 files changed, 47 insertions(+)
diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 22623eb4f72f..e1bdba978dbc 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -45,6 +45,13 @@ static int hid_ignore_special_drivers = 0;
module_param_named(ignore_special_drivers, hid_ignore_special_drivers, int, 0600);
MODULE_PARM_DESC(ignore_special_drivers, "Ignore any special drivers and handle all devices by generic driver");
+/* Quirks specified at module load time */
+static char *quirks_param[MAX_USBHID_BOOT_QUIRKS];
+module_param_array_named(quirks, quirks_param, charp, NULL, 0444);
+MODULE_PARM_DESC(quirks, "Add/modify USB HID quirks by specifying "
+ " quirks=BUS:vendorID:productID:quirks"
+ " where BUS, vendorID, productID, and quirks are all in"
+ " 0x-prefixed hex");
/*
* Register a new report for a device.
*/
@@ -2951,6 +2958,10 @@ static int __init hid_init(void)
{
int ret;
+ ret = hid_quirks_bus_init(quirks_param, MAX_USBHID_BOOT_QUIRKS);
+ if (ret)
+ goto usbhid_quirks_init_fail;
+
ret = bus_register(&hid_bus_type);
if (ret) {
pr_err("can't register hid bus\n");
@@ -2972,6 +2983,8 @@ static int __init hid_init(void)
bus_unregister(&hid_bus_type);
err:
return ret;
+usbhid_quirks_init_fail:
+ return ret;
}
static void __exit hid_exit(void)
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 3983b4f282f8..40d3ba78ff73 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -1182,6 +1182,39 @@ static void hid_remove_all_dquirks(__u16 bus)
}
+
+/**
+ * hid_quirks_bus_init - apply HID quirks specified at module load time
+ * @quirks_param: array of quirks strings (bus:vendor:product:quirks)
+ * @count: number of quirks to check
+ */
+int hid_quirks_bus_init(char **quirks_param, int count)
+{
+ struct hid_device_id id = { 0 };
+ int n = 0, m;
+ unsigned short int bus, vendor, product;
+ u32 quirks;
+
+ for (; n < count && quirks_param[n]; n++) {
+
+ m = sscanf(quirks_param[n], "0x%hx:0x%hx:0x%hx:0x%x",
+ &bus, &vendor, &product, &quirks);
+
+ id.bus = (__u16)bus;
+ id.vendor = (__u16)vendor;
+ id.product = (__u16)product;
+
+ if (m != 4 ||
+ hid_modify_dquirk(&id, quirks) != 0) {
+ pr_warn("Could not parse HID quirk module param %s\n",
+ quirks_param[n]);
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hid_quirks_bus_init);
+
/**
* hid_quirks_init - apply HID quirks specified at module load time
* @quirks_param: array of quirks strings (vendor:product:quirks)
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 4e4c4fe36911..7f2e8ba7d783 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -1178,6 +1178,7 @@ int hid_report_raw_event(struct hid_device *hid, enum hid_report_type type, u8 *
/* HID quirks API */
unsigned long hid_lookup_quirk(const struct hid_device *hdev);
+int hid_quirks_bus_init(char **quirks_param, int count);
int hid_quirks_init(char **quirks_param, __u16 bus, int count);
void hid_quirks_exit(__u16 bus);
--
2.41.0
^ permalink raw reply related
* [PATCH RFC 0/1] HID: allow specifying quirks params for hid-core
From: Marco Morandini @ 2023-06-15 21:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, linux-input
Right now, only usbhid accepts parameters
specifying quirks.
Following Benjamin's explanations,
this patch allows specifying them for hid
in the form hid.quirks=BUS:vendorID:productID:quirks
I don't know if this is a good idea or not, thus the RFC.
If it was for me, I'd get rid of the usbhid parameters
and of the duplicated hid_quirks_init,
forcing everyone to specify the bus,
but I fear this could be a regression for user space.
Marco Morandini (1)
HID: allow specifying quirks params for hid-core
drivers/hid/hid-core.c | 13 +++++++++++++
drivers/hid/hid-quirks.c | 33 +++++++++++++++++++++++++++++++++
include/linux/hid.h | 1 +
3 files changed, 47 insertions(+)
--
2.41.0
^ permalink raw reply
* Re: [RESEND] Input: support pre-stored effects
From: Jeff LaBundy @ 2023-06-15 19:49 UTC (permalink / raw)
To: James Ogletree
Cc: Dmitry Torokhov, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <5CD75646-02A6-4175-9176-A8DCD059085F@cirrus.com>
Hi James,
On Thu, Jun 15, 2023 at 06:12:20PM +0000, James Ogletree wrote:
>
>
> > On Jun 12, 2023, at 8:25 PM, Jeff LaBundy <jeff@labundy.com> wrote:
> >
> > Hi James,
> >
> > On Mon, Jun 12, 2023 at 07:43:57PM +0000, James Ogletree wrote:
> >> At present, the best way to define effects that
> >> pre-exist in device memory is by utilizing
> >> the custom_data field, which it was not intended
> >> for, and requires arbitrary interpretation by
> >> the driver to make meaningful.
> >>
> >> Provide option for defining pre-stored effects in
> >> device memory.
> >>
> >> Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
> >> ---
> >> include/uapi/linux/input.h | 32 ++++++++++++++++++++++----------
> >> 1 file changed, 22 insertions(+), 10 deletions(-)
> >>
> >> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
> >> index 2557eb7b0561..689e5fa10647 100644
> >> --- a/include/uapi/linux/input.h
> >> +++ b/include/uapi/linux/input.h
> >> @@ -428,17 +428,27 @@ struct ff_rumble_effect {
> >> __u16 weak_magnitude;
> >> };
> >>
> >> +/**
> >> + * struct ff_prestored_effect - defines parameters of a pre-stored force-feedback effect
> >> + * @index: index of effect
> >> + * @bank: memory bank of effect
> >> + */
> >> +struct ff_prestored_effect {
> >> + __u16 index;
> >> + __u16 bank;
> >> +};
> >
> > This seems like a good start; I do wonder if it's useful to review recent
> > customer vibrator HAL implementations and decide whether you want to pack
> > any additional members here such as magnitude, etc. as is done for periodic
> > effects?
> >
> > Back in L25 days, some customers would assign click or tap effects to one
> > or more entries in the wavetable and then use a separate digital volume
> > control (at that time exposed through sysfs) to create a few discrete
> > amplitude levels. Perhaps it would be handy to bundle this information as
> > part of the same call?
> >
> > It's just a suggestion; I'll defer to your much more recent expertise.
> >
>
> My thinking is that ff_prestored_effect ought to be for effects being used
> “off-the-shelf”, and in such cases it would seem appropriate to defer to
> firmware for the effect design. I think this fits nicely as-is with the other
> structures as it serves a clear and distinct use-case. Otherwise one might
> just add these two members to ff_periodic_effect (or every kind of effect).
>
> I think the current predominant method for setting "magnitude" for these
> pre-stored effects is by using the FF_GAIN event code as a separate write
> call, so I think adding a magnitude member would go unused, if I understand
> you correctly.
All great points. In that case:
Reviewed-by: Jeff LaBundy <jeff@labundy.com>
>
> Thanks,
> James
>
>
>
Kind regards,
Jeff LaBundy
^ permalink raw reply
* [PATCH] hid-mcp2200: added driver for GPIOs of MCP2200
From: Johannes Roith @ 2023-06-15 18:30 UTC (permalink / raw)
To: jikos
Cc: linux-kernel, linux-input, christophe.jaillet, andi.shyti,
Johannes Roith
Added a gpiochip compatible driver to control the 8 GPIOs of the MCP2200
by using the HID interface.
Using GPIOs with alternative functions (GP0<->SSPND, GP1<->USBCFG,
GP6<->RXLED, GP7<->TXLED) will reset the functions, if set (unset by
default).
The driver was tested while also using the UART of the chip. Setting
and reading the GPIOs has no effect on the UART communication. However,
a reset is triggered after the CONFIGURE command. If the GPIO Direction
is constantly changed, this will affect the communication at low baud
rates. This is a hardware problem of the MCP2200 and is not caused by
the driver.
Feedback from reviewers Christophe JAILLET <christophe.jaillet@wanadoo.fr>
and Andi Shyti <andi.shyti@kernel.org> was added.
Signed-off-by: Johannes Roith <johannes@gnu-linux.rocks>
---
drivers/hid/Kconfig | 10 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-mcp2200.c | 416 ++++++++++++++++++++++++++++++++++++++
4 files changed, 428 insertions(+)
create mode 100644 drivers/hid/hid-mcp2200.c
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 4ce012f83253..ca7927d22c23 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -1283,6 +1283,16 @@ config HID_MCP2221
To compile this driver as a module, choose M here: the module
will be called hid-mcp2221.ko.
+config HID_MCP2200
+ tristate "Microchip MCP2200 HID USB-to-GPIO bridge"
+ depends on USB_HID
+ imply GPIOLIB
+ help
+ Provides GPIO functionality over USB-HID through MCP2200 device.
+
+ To compile this driver as a module, choose M here: the module
+ will be called hid-mcp2200.ko.
+
config HID_KUNIT_TEST
tristate "KUnit tests for HID" if !KUNIT_ALL_TESTS
depends on KUNIT=y
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 5d37cacbde33..d593fb982f7d 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_HID_MACALLY) += hid-macally.o
obj-$(CONFIG_HID_MAGICMOUSE) += hid-magicmouse.o
obj-$(CONFIG_HID_MALTRON) += hid-maltron.o
obj-$(CONFIG_HID_MCP2221) += hid-mcp2221.o
+obj-$(CONFIG_HID_MCP2200) += hid-mcp2200.o
obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5d29abac2300..017e37a171a8 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -912,6 +912,7 @@
#define USB_DEVICE_ID_PICK16F1454_V2 0xf2f7
#define USB_DEVICE_ID_LUXAFOR 0xf372
#define USB_DEVICE_ID_MCP2221 0x00dd
+#define USB_DEVICE_ID_MCP2200 0x00df
#define USB_VENDOR_ID_MICROSOFT 0x045e
#define USB_DEVICE_ID_SIDEWINDER_GV 0x003b
diff --git a/drivers/hid/hid-mcp2200.c b/drivers/hid/hid-mcp2200.c
new file mode 100644
index 000000000000..3a950365c0cd
--- /dev/null
+++ b/drivers/hid/hid-mcp2200.c
@@ -0,0 +1,416 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * MCP2200 - Microchip USB to GPIO bridge
+ *
+ * Copyright (c) 2023, Johannes Roith <johannes@gnu-linux.rocks>
+ *
+ * Datasheet: https://ww1.microchip.com/downloads/en/DeviceDoc/22228A.pdf
+ * App Note for HID: https://ww1.microchip.com/downloads/en/DeviceDoc/93066A.pdf
+ */
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio/driver.h>
+#include <linux/hid.h>
+#include <linux/hidraw.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include "hid-ids.h"
+
+/* Commands codes in a raw output report */
+#define SET_CLEAR_OUTPUTS 0x08
+#define CONFIGURE 0x10
+#define READ_EE 0x20
+#define WRITE_EE 0x40
+#define READ_ALL 0x80
+
+/* MCP GPIO direction encoding */
+enum MCP_IO_DIR {
+ MCP2200_DIR_OUT = 0x00,
+ MCP2200_DIR_IN = 0x01,
+};
+
+/* Altternative pin assignments */
+#define TXLED 2
+#define RXLED 3
+#define USBCFG 6
+#define SSPND 7
+#define MCP_NGPIO 8
+
+/* CMD to set or clear a GPIO output */
+struct mcp_set_clear_outputs {
+ u8 cmd;
+ u8 dummys1[10];
+ u8 set_bmap;
+ u8 clear_bmap;
+ u8 dummys2[3];
+} __packed;
+
+/* CMD to configure the IOs */
+struct mcp_configure {
+ u8 cmd;
+ u8 dummys1[3];
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 dummys2[6];
+} __packed;
+
+/* CMD to read all parameters */
+struct mcp_read_all {
+ u8 cmd;
+ u8 dummys[15];
+} __packed;
+
+/* Response to the read all cmd */
+struct mcp_read_all_resp {
+ u8 cmd;
+ u8 eep_addr;
+ u8 dummy;
+ u8 eep_val;
+ u8 io_bmap;
+ u8 config_alt_pins;
+ u8 io_default_val_bmap;
+ u8 config_alt_options;
+ u8 baud_h;
+ u8 baud_l;
+ u8 io_port_val_bmap;
+ u8 dummys[5];
+} __packed;
+
+struct mcp2200 {
+ struct hid_device *hdev;
+ struct mutex lock;
+ struct completion wait_in_report;
+ u8 gpio_dir;
+ u8 gpio_val;
+ u8 gpio_inval;
+ u8 baud_h;
+ u8 baud_l;
+ u8 config_alt_pins;
+ u8 gpio_reset_val;
+ u8 config_alt_options;
+ int status;
+ struct gpio_chip gc;
+};
+
+/* this executes the READ_ALL cmd */
+static int mcp_cmd_read_all(struct mcp2200 *mcp)
+{
+ struct mcp_read_all *read_all;
+ int len, t;
+
+ reinit_completion(&mcp->wait_in_report);
+ read_all = kzalloc(sizeof(struct mcp_read_all), GFP_KERNEL);
+ if (!read_all)
+ return -ENOMEM;
+
+ read_all->cmd = READ_ALL;
+
+ mutex_lock(&mcp->lock);
+ len = hid_hw_output_report(mcp->hdev, (u8 *) read_all,
+ sizeof(struct mcp_read_all));
+
+ mutex_unlock(&mcp->lock);
+ kfree(read_all);
+
+ if (len != sizeof(struct mcp_read_all))
+ return -EINVAL;
+
+ t = wait_for_completion_timeout(&mcp->wait_in_report, msecs_to_jiffies(4000));
+ if (!t)
+ return -ETIMEDOUT;
+
+ /* return status, negative value if wrong response was received */
+ return mcp->status;
+}
+
+static void mcp_set_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ u8 value;
+ int status;
+ struct mcp_set_clear_outputs *cmd;
+
+ cmd = kzalloc(sizeof(struct mcp_set_clear_outputs), GFP_KERNEL);
+ if (!cmd)
+ return;
+
+ mutex_lock(&mcp->lock);
+
+ value = mcp->gpio_val & ~*mask;
+ value |= (*mask & *bits);
+
+ cmd->cmd = SET_CLEAR_OUTPUTS;
+ cmd->set_bmap = value;
+ cmd->clear_bmap = ~(value);
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) cmd,
+ sizeof(struct mcp_set_clear_outputs));
+
+ mutex_unlock(&mcp->lock);
+ kfree(cmd);
+
+ if (status == sizeof(struct mcp_set_clear_outputs))
+ mcp->gpio_val = value;
+}
+
+static void mcp_set(struct gpio_chip *gc, unsigned int gpio_nr, int value)
+{
+ unsigned long mask = 1 << gpio_nr;
+ unsigned long bmap_value = value << gpio_nr;
+
+ mcp_set_multiple(gc, &mask, &bmap_value);
+}
+
+static int mcp_get_multiple(struct gpio_chip *gc, unsigned long *mask,
+ unsigned long *bits)
+{
+ u32 val;
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ int status;
+
+ status = mcp_cmd_read_all(mcp);
+ if (status != 0)
+ return status;
+
+ val = mcp->gpio_inval;
+ *bits = (val & *mask);
+ return 0;
+}
+
+static int mcp_get(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ unsigned long mask = 0, bits = 0;
+
+ mask = (1 << gpio_nr);
+ mcp_get_multiple(gc, &mask, &bits);
+ return (bits > 0) ? 1 : 0;
+}
+
+static int mcp_get_direction(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+
+ return (mcp->gpio_dir & (MCP2200_DIR_IN << gpio_nr))
+ ? GPIO_LINE_DIRECTION_IN : GPIO_LINE_DIRECTION_OUT;
+}
+
+static int mcp_set_direction(struct gpio_chip *gc, unsigned int gpio_nr,
+ enum MCP_IO_DIR io_direction)
+{
+ struct mcp2200 *mcp = gpiochip_get_data(gc);
+ struct mcp_configure *conf;
+ int status;
+ /* after the configure cmd we will need to set the outputs again */
+ unsigned long mask = ~(mcp->gpio_dir); /* only set outputs */
+ unsigned long bits = mcp->gpio_val;
+ /* Offsets of alternative pins in config_alt_pins, 0 is not used */
+ u8 alt_pin_conf[8] = {SSPND, USBCFG, 0, 0, 0, 0, RXLED, TXLED};
+ u8 config_alt_pins = mcp->config_alt_pins;
+
+ /* Read in the reset baudrate first, we need it later */
+ status = mcp_cmd_read_all(mcp);
+ if (status != 0)
+ return status;
+
+ conf = kzalloc(sizeof(struct mcp_configure), GFP_KERNEL);
+ if (!conf)
+ return -ENOMEM;
+ mutex_lock(&mcp->lock);
+
+ /* configure will reset the chip! */
+ conf->cmd = CONFIGURE;
+ conf->io_bmap = (mcp->gpio_dir & ~(1 << gpio_nr))
+ | (io_direction << gpio_nr);
+ /* Don't overwrite the reset parameters */
+ conf->baud_h = mcp->baud_h;
+ conf->baud_l = mcp->baud_l;
+ conf->config_alt_options = mcp->config_alt_options;
+ conf->io_default_val_bmap = mcp->gpio_reset_val;
+ /* Adjust alt. func if necessary */
+ if (alt_pin_conf[gpio_nr])
+ config_alt_pins &= ~(1 << alt_pin_conf[gpio_nr]);
+ conf->config_alt_pins = config_alt_pins;
+
+ status = hid_hw_output_report(mcp->hdev, (u8 *) conf,
+ sizeof(struct mcp_set_clear_outputs));
+
+ mutex_unlock(&mcp->lock);
+
+ if (status == sizeof(struct mcp_set_clear_outputs)) {
+ mcp->gpio_dir &= ~(1 << gpio_nr);
+ mcp->config_alt_pins = config_alt_pins;
+ } else {
+ return -EIO;
+ }
+
+ kfree(conf);
+ /* Configure CMD will clear all IOs -> rewrite them */
+ mcp_set_multiple(gc, &mask, &bits);
+ return 0;
+}
+
+static int mcp_direction_input(struct gpio_chip *gc, unsigned int gpio_nr)
+{
+ return mcp_set_direction(gc, gpio_nr, MCP2200_DIR_IN);
+}
+
+static int mcp_direction_output(struct gpio_chip *gc, unsigned int gpio_nr,
+ int value)
+{
+ int ret;
+ unsigned long mask, bmap_value;
+
+ mask = 1 << gpio_nr;
+ bmap_value = value << gpio_nr;
+
+ ret = mcp_set_direction(gc, gpio_nr, MCP2200_DIR_OUT);
+ if (ret == 0)
+ mcp_set_multiple(gc, &mask, &bmap_value);
+ return ret;
+}
+
+static const struct gpio_chip template_chip = {
+ .label = "mcp2200",
+ .owner = THIS_MODULE,
+ .get_direction = mcp_get_direction,
+ .direction_input = mcp_direction_input,
+ .direction_output = mcp_direction_output,
+ .set = mcp_set,
+ .set_multiple = mcp_set_multiple,
+ .get = mcp_get,
+ .get_multiple = mcp_get_multiple,
+ .base = -1,
+ .ngpio = MCP_NGPIO,
+ .can_sleep = true,
+};
+
+/*
+ * MCP2200 uses interrupt endpoint for input reports. This function
+ * is called by HID layer when it receives i/p report from mcp2200,
+ * which is actually a response to the previously sent command.
+ */
+static int mcp2200_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct mcp2200 *mcp = hid_get_drvdata(hdev);
+ struct mcp_read_all_resp *all_resp;
+
+ switch (data[0]) {
+ case READ_ALL:
+ all_resp = (struct mcp_read_all_resp *) data;
+ mcp->status = 0;
+ mcp->gpio_inval = all_resp->io_port_val_bmap;
+ mcp->baud_h = all_resp->baud_h;
+ mcp->baud_l = all_resp->baud_l;
+ mcp->gpio_reset_val = all_resp->io_default_val_bmap;
+ mcp->config_alt_pins = all_resp->config_alt_pins;
+ mcp->config_alt_options = all_resp->config_alt_options;
+ break;
+ default:
+ mcp->status = -EIO;
+ break;
+ }
+
+ complete(&mcp->wait_in_report);
+ return 1;
+}
+
+static void mcp2200_hid_unregister(void *ptr)
+{
+ struct hid_device *hdev = ptr;
+
+ hid_hw_close(hdev);
+ hid_hw_stop(hdev);
+}
+
+static int mcp2200_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ struct mcp2200 *mcp;
+
+ mcp = devm_kzalloc(&hdev->dev, sizeof(*mcp), GFP_KERNEL);
+ if (!mcp)
+ return -ENOMEM;
+
+ ret = hid_parse(hdev);
+ if (ret) {
+ hid_err(hdev, "can't parse reports\n");
+ return ret;
+ }
+
+ /*
+ * This driver uses the .raw_event callback and therefore does not need any
+ * HID_CONNECT_xxx flags.
+ */
+ ret = hid_hw_start(hdev, 0);
+ if (ret) {
+ hid_err(hdev, "can't start hardware\n");
+ return ret;
+ }
+
+ hid_info(hdev, "USB HID v%x.%02x Device [%s] on %s\n", hdev->version >> 8,
+ hdev->version & 0xff, hdev->name, hdev->phys);
+
+ ret = hid_hw_open(hdev);
+ if (ret) {
+ hid_err(hdev, "can't open device\n");
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ mutex_init(&mcp->lock);
+ init_completion(&mcp->wait_in_report);
+ hid_set_drvdata(hdev, mcp);
+ mcp->hdev = hdev;
+
+ ret = devm_add_action_or_reset(&hdev->dev, mcp2200_hid_unregister, hdev);
+ if (ret)
+ return ret;
+
+ mcp->gc = template_chip;
+ mcp->gc.parent = &hdev->dev;
+
+ ret = gpiochip_add_data(&mcp->gc, mcp);
+ if (ret < 0) {
+ dev_err(&hdev->dev, "Unable to register gpiochip\n");
+ hid_hw_stop(hdev);
+ return ret;
+ }
+
+ return 0;
+}
+
+static void mcp2200_remove(struct hid_device *hdev)
+{
+ struct mcp2200 *mcp;
+
+ mcp = hid_get_drvdata(hdev);
+ gpiochip_remove(&mcp->gc);
+}
+
+static const struct hid_device_id mcp2200_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MICROCHIP, USB_DEVICE_ID_MCP2200) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, mcp2200_devices);
+
+static struct hid_driver mcp2200_driver = {
+ .name = "mcp2200",
+ .id_table = mcp2200_devices,
+ .probe = mcp2200_probe,
+ .remove = mcp2200_remove,
+ .raw_event = mcp2200_raw_event,
+};
+
+/* Register with HID core */
+module_hid_driver(mcp2200_driver);
+
+MODULE_AUTHOR("Johannes Roith <johannes@gnu-linux.rocks>");
+MODULE_DESCRIPTION("MCP2200 Microchip HID USB to GPIO bridge");
+MODULE_LICENSE("GPL");
+
--
2.41.0
^ permalink raw reply related
* Re: [RESEND] Input: support pre-stored effects
From: James Ogletree @ 2023-06-15 18:12 UTC (permalink / raw)
To: Jeff LaBundy
Cc: Dmitry Torokhov, linux-input@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <ZIfFgJJtGCfyIne4@nixie71>
> On Jun 12, 2023, at 8:25 PM, Jeff LaBundy <jeff@labundy.com> wrote:
>
> Hi James,
>
> On Mon, Jun 12, 2023 at 07:43:57PM +0000, James Ogletree wrote:
>> At present, the best way to define effects that
>> pre-exist in device memory is by utilizing
>> the custom_data field, which it was not intended
>> for, and requires arbitrary interpretation by
>> the driver to make meaningful.
>>
>> Provide option for defining pre-stored effects in
>> device memory.
>>
>> Signed-off-by: James Ogletree <james.ogletree@cirrus.com>
>> ---
>> include/uapi/linux/input.h | 32 ++++++++++++++++++++++----------
>> 1 file changed, 22 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/uapi/linux/input.h b/include/uapi/linux/input.h
>> index 2557eb7b0561..689e5fa10647 100644
>> --- a/include/uapi/linux/input.h
>> +++ b/include/uapi/linux/input.h
>> @@ -428,17 +428,27 @@ struct ff_rumble_effect {
>> __u16 weak_magnitude;
>> };
>>
>> +/**
>> + * struct ff_prestored_effect - defines parameters of a pre-stored force-feedback effect
>> + * @index: index of effect
>> + * @bank: memory bank of effect
>> + */
>> +struct ff_prestored_effect {
>> + __u16 index;
>> + __u16 bank;
>> +};
>
> This seems like a good start; I do wonder if it's useful to review recent
> customer vibrator HAL implementations and decide whether you want to pack
> any additional members here such as magnitude, etc. as is done for periodic
> effects?
>
> Back in L25 days, some customers would assign click or tap effects to one
> or more entries in the wavetable and then use a separate digital volume
> control (at that time exposed through sysfs) to create a few discrete
> amplitude levels. Perhaps it would be handy to bundle this information as
> part of the same call?
>
> It's just a suggestion; I'll defer to your much more recent expertise.
>
My thinking is that ff_prestored_effect ought to be for effects being used
“off-the-shelf”, and in such cases it would seem appropriate to defer to
firmware for the effect design. I think this fits nicely as-is with the other
structures as it serves a clear and distinct use-case. Otherwise one might
just add these two members to ff_periodic_effect (or every kind of effect).
I think the current predominant method for setting "magnitude" for these
pre-stored effects is by using the FF_GAIN event code as a separate write
call, so I think adding a magnitude member would go unused, if I understand
you correctly.
Thanks,
James
^ permalink raw reply
* Re: [PATCH] HID: i2c-hid: Block a rogue device on ASUS TUF A16
From: Friedrich Vock @ 2023-06-15 12:41 UTC (permalink / raw)
To: Limonciello, Mario, linux-input, Natikar, Basavaraj,
S-k, Shyam-sundar
Cc: Jiri Kosina, Benjamin Tissoires
In-Reply-To: <dc4da9cf-a66f-6e09-165d-f16a405f2a38@amd.com>
Hi,
sorry for taking so long to reply.
On 02.06.23 20:43, Limonciello, Mario wrote:
> + some AMD guys
>
> On 5/30/2023 10:40 AM, Friedrich Vock wrote:
>> On these laptops, there seems to be a device that, when probed by
>> i2c-hid, constantly sends bogus interrupts and interferes with the
>> keyboard controller. When the device is enabled, it takes the keyboard
>> around 8 seconds to register that keys are being pressed or released.
>
> Do you know what interrupt is firing constantly?
> Presumably it is the GPIO controller master interrupt, right?
> And it's for GPIO 7 (guessed from acpidump on one of the bug
> reports).
>
> To confirm check /proc/interrupts.
Seems likely that you guessed correctly. The corresponsing line in
/proc/interrupts (with the interrupts counts omitted):
71: amd_gpio 7 ITE5570:00
>
> If it's not obvious which GPIO is firing there is also a dynamic
> debug statement in pinctrl-amd.c that may be helpful to figure
> this out.
>
> I would also suspect in Windows this doesn't happen. If possible
> can you confirm that? Check in Device Manager what driver is bound
> to this device. Is it "inbox" from Microsoft or is it an ASUS
> specific driver?
>
> I wonder if the GPIO controller got programmed differently in
> Windows for some reason. We may want to confirm the values for
> GPIO registers from /sys/kernel/debug/gpio in Linux against those
> that are programmed in Windows.
>
> This can be accomplished using R/W everything in Windows.
Unfortunately I don't have Windows installed on this system. I know some
people with the Ryzen 9 7940HS model which might, I'll ask them if they
can give me the configuration on Windows and Linux.
>
>>
>> Nothing I tried seemed to make the device work, and preventing the
>> device from being probed doesn't seem to break any functionality of
>> the laptop.
>>
>> Signed-off-by: Friedrich Vock <friedrich.vock@gmx.de>
>
> There are a few bug reports that popped up around this issue that should
> probably also be tagged.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217336
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=217493
>
>> ---
>> drivers/hid/i2c-hid/i2c-hid-core.c | 5 +++
>> drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c | 48 ++++++++++++++++++++++++
>> drivers/hid/i2c-hid/i2c-hid.h | 3 ++
>> 3 files changed, 56 insertions(+)
>>
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c
>> b/drivers/hid/i2c-hid/i2c-hid-core.c
>> index efbba0465eef..5f0686d058df 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c
>> @@ -1035,6 +1035,11 @@ int i2c_hid_core_probe(struct i2c_client
>> *client, struct i2chid_ops *ops,
>>
>> ihid->quirks = i2c_hid_lookup_quirk(hid->vendor, hid->product);
>>
>> + if (i2c_hid_device_blocked(hid->vendor, hid->product)) {
>> + ret = -ENODEV;
>> + goto err_irq;
>> + }
>> +
> The thing I worry about here is that an unserviced interrupt can
> prevent the
> hardware from going into proper low power states; particularly at
> runtime.
>
> I think we should better understand what's going on before going down
> this
> path of just ignoring it.
Yeah, I guess I should've searched more for a proper explanation/fix
before submitting hacks like this. Let's see if this can be fixed in a
cleaner manner than preemptively disabling parts of the system.
>> ret = hid_add_device(hid);
>> if (ret) {
>> if (ret != -ENODEV)
>> diff --git a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>> b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>> index 210f17c3a0be..d2c2806b64b4 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>> +++ b/drivers/hid/i2c-hid/i2c-hid-dmi-quirks.c
>> @@ -440,6 +440,38 @@ static const struct dmi_system_id
>> i2c_hid_dmi_quirk_table[] = {
>> { } /* Terminate list */
>> };
>>
>> +static const struct hid_device_id i2c_hid_blocked_ite_device = {
>> + HID_DEVICE(BUS_I2C, HID_GROUP_GENERIC, USB_VENDOR_ID_ITE, 0x8051)
>> +};
>> +
>> +/*
>> + * This list contains devices that can't be activated at all, for
>> example
>> + * because activating them breaks other important parts of the system.
>> + */
>> +static const struct dmi_system_id i2c_hid_dmi_block_table[] = {
>> + /*
>> + * On ASUS TUF Gaming A16 laptops, there is a device that will
>> make the
>> + * keyboard controller stop working correctly and flood the CPU
>> with bogus
>> + * interrupts.
>> + */
>> + {
>> + .ident = "ASUS TUF Gaming A16 (Ryzen 7 7735HS)",
>> + .matches = {
>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617NS"),
>> + },
>> + .driver_data = (void *)&i2c_hid_blocked_ite_device,
>> + },
>> + {
>> + .ident = "ASUS TUF Gaming A16 (Ryzen 9 7940HS)",
>> + .matches = {
>> + DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
>> + DMI_MATCH(DMI_PRODUCT_NAME, "FA617XS"),
>> + },
>> + .driver_data = (void *)&i2c_hid_blocked_ite_device,
>> + },
>> + { } /* Terminate list */
> If this *does* end up being the best solution, I think it's better
> to patch in the DMI to gpiolib-acpi.c similar to other quirks for
> floating
> GPIOs. Example:
>
> https://github.com/torvalds/linux/blob/master/drivers/gpio/gpiolib-acpi.c#L1654
>
>
>> +};
>>
>> struct i2c_hid_desc *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t
>> *i2c_name)
>> {
>> @@ -492,3 +524,19 @@ u32 i2c_hid_get_dmi_quirks(const u16 vendor,
>> const u16 product)
>>
>> return quirks;
>> }
>> +
>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product)
>> +{
>> + const struct dmi_system_id *system_id =
>> + dmi_first_match(i2c_hid_dmi_block_table);
>> +
>> + if (system_id) {
>> + const struct hid_device_id *device_id =
>> + (struct hid_device_id *)(system_id->driver_data);
>> +
>> + if (device_id && device_id->vendor == vendor &&
>> + device_id->product == product)
>> + return true;
>> + }
>> + return false;
>> +}
>> diff --git a/drivers/hid/i2c-hid/i2c-hid.h
>> b/drivers/hid/i2c-hid/i2c-hid.h
>> index 2c7b66d5caa0..e17bdd758f39 100644
>> --- a/drivers/hid/i2c-hid/i2c-hid.h
>> +++ b/drivers/hid/i2c-hid/i2c-hid.h
>> @@ -10,6 +10,7 @@ struct i2c_hid_desc
>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name);
>> char *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>> unsigned int *size);
>> u32 i2c_hid_get_dmi_quirks(const u16 vendor, const u16 product);
>> +bool i2c_hid_device_blocked(const u16 vendor, const u16 product);
>> #else
>> static inline struct i2c_hid_desc
>> *i2c_hid_get_dmi_i2c_hid_desc_override(uint8_t *i2c_name)
>> @@ -19,6 +20,8 @@ static inline char
>> *i2c_hid_get_dmi_hid_report_desc_override(uint8_t *i2c_name,
>> { return NULL; }
>> static inline u32 i2c_hid_get_dmi_quirks(const u16 vendor, const
>> u16 product)
>> { return 0; }
>> +static inline bool i2c_hid_device_blocked(const u16 vendor, const
>> u16 product)
>> +{ return false; }
>> #endif
>>
>> /**
>> --
>> 2.40.1
>>
>>
^ permalink raw reply
* [PATCH AUTOSEL 4.14 2/6] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:40 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615114016.649846-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 3d521f289984a..28e7a4950b74a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2251,8 +2251,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 4/8] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113956.649736-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 4e4a3424c1f9f..c50b26a9bc445 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2390,8 +2390,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 3/8] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113938.649627-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index b6947d7573473..2ebad3ed4e3af 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -473,6 +473,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b883423a89c5d..479516bbb61bf 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -490,6 +490,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 3/8] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113956.649736-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 51a827470157b..b36bcc26bbfe3 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -124,6 +124,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 2c9597c8ac92d..c0ba8d6f4978f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -480,6 +480,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 4/8] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113938.649627-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a93070f5b214c..36cb456709ed7 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2419,8 +2419,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 4/9] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113917.649505-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index a93070f5b214c..36cb456709ed7 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2419,8 +2419,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 3/9] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:39 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113917.649505-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 0476301983964..2f4c5b45d4096 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -532,6 +532,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 1d1306a6004e6..2b658d820b800 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -491,6 +491,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 04/10] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113854.649370-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index d29773a799b4f..33e763e746a0b 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2425,8 +2425,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 03/10] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113854.649370-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 0403beb3104b9..6a227e07f8943 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -589,6 +589,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b153ddc3319e8..5daec769df7ae 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -502,6 +502,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 04/16] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113816.649135-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 7ae5f27df54dd..c6bdb9c4ef3e0 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -586,6 +586,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 53c6692d77714..653db6cdab579 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -523,6 +523,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 05/16] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:38 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113816.649135-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index fb538a6c4add8..aff4a21a46b6a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2417,8 +2417,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.3 07/19] HID: wacom: Add error check to wacom_parse_and_register()
From: Sasha Levin @ 2023-06-15 11:37 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Denis Arefev, Ping Cheng, Jiri Kosina, Sasha Levin, jason.gerecke,
jikos, benjamin.tissoires, linux-input
In-Reply-To: <20230615113719.648862-1-sashal@kernel.org>
From: Denis Arefev <arefev@swemel.ru>
[ Upstream commit 16a9c24f24fbe4564284eb575b18cc20586b9270 ]
Added a variable check and
transition in case of an error
Found by Linux Verification Center (linuxtesting.org) with SVACE.
Signed-off-by: Denis Arefev <arefev@swemel.ru>
Reviewed-by: Ping Cheng <ping.cheng@wacom.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/wacom_sys.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index fb538a6c4add8..aff4a21a46b6a 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -2417,8 +2417,13 @@ static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
goto fail_quirks;
}
- if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR)
+ if (features->device_type & WACOM_DEVICETYPE_WL_MONITOR) {
error = hid_hw_open(hdev);
+ if (error) {
+ hid_err(hdev, "hw open failed\n");
+ goto fail_quirks;
+ }
+ }
wacom_set_shared_values(wacom_wac);
devres_close_group(&hdev->dev, wacom);
--
2.39.2
^ permalink raw reply related
* [PATCH AUTOSEL 6.3 06/19] HID: google: add jewel USB id
From: Sasha Levin @ 2023-06-15 11:37 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Sung-Chi Li, Jiri Kosina, Sasha Levin, jikos, benjamin.tissoires,
linux-input
In-Reply-To: <20230615113719.648862-1-sashal@kernel.org>
From: Sung-Chi Li <lschyi@chromium.org>
[ Upstream commit ed84c4517a5bc536e8572a01dfa11bc22a280d06 ]
Add 1 additional hammer-like device.
Signed-off-by: Sung-Chi Li <lschyi@chromium.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-google-hammer.c | 2 ++
drivers/hid/hid-ids.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 7ae5f27df54dd..c6bdb9c4ef3e0 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -586,6 +586,8 @@ static const struct hid_device_id hammer_devices[] = {
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_EEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_HAMMER) },
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_JEWEL) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_GOOGLE, USB_DEVICE_ID_GOOGLE_MAGNEMITE) },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 8f3e0a5d5f834..2e23018c9f23d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -529,6 +529,7 @@
#define USB_DEVICE_ID_GOOGLE_MOONBALL 0x5044
#define USB_DEVICE_ID_GOOGLE_DON 0x5050
#define USB_DEVICE_ID_GOOGLE_EEL 0x5057
+#define USB_DEVICE_ID_GOOGLE_JEWEL 0x5061
#define USB_VENDOR_ID_GOTOP 0x08f2
#define USB_DEVICE_ID_SUPER_Q2 0x007f
--
2.39.2
^ permalink raw reply related
* Re: [PATCH] HID: logitech-hidpp: Handle timeout differently from busy
From: Mark Lord @ 2023-06-15 11:24 UTC (permalink / raw)
To: Linux regressions mailing list, Jiri Kosina, Bastien Nocera,
Benjamin Tissoires
Cc: linux-input, linux-kernel, Peter F . Patel-Schneider,
Filipe Laíns, Nestor Lopez Casado
In-Reply-To: <53903462-2552-b707-3831-cad3ef873b0d@leemhuis.info>
On 2023-06-15 03:24 AM, Linux regression tracking (Thorsten Leemhuis) wrote:
>
...
> https://bugzilla.kernel.org/show_bug.cgi?id=217412
>
> --- Comment #47 from Mark Blakeney ---
> @Juha, kernel 6.3.7 adds the 2 patches intended to fix this bug and the
> startup delay is now gone. However, I have had 2 cases over the last 5
> days in which I have been running 6.3.7 where my mouse fails to be
> detected at all after startup. I have to pull the Logitech receiver
> out/in to get the mouse working. Never seen this issue before so I
> suspect the patches are not right.
> ```
I too have had that happen with recent kernels, but have not yet put
a finger to a specific version or cause.
Just toggling the power button on the wireless mouse is enough for
it to "re-appear".
The 5.4.xx kernels never had this issue. I went straight from those
to the 6.3.xx ones, where it does happen sometimes, both with and without
the recent "delay" fixes.
--
Mark Lord
^ permalink raw reply
* [PATCH v2 4/4] input: touchscreen: add SPI support for Goodix Berlin Touchscreen IC
From: Neil Armstrong @ 2023-06-15 10:27 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Bastien Nocera, Hans de Goede, Henrik Rydberg
Cc: linux-input, linux-arm-msm, devicetree, linux-kernel,
Neil Armstrong
In-Reply-To: <20230606-topic-goodix-berlin-upstream-initial-v2-0-26bc8fe1e90e@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
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
drivers/input/touchscreen/Kconfig | 13 ++
drivers/input/touchscreen/Makefile | 1 +
drivers/input/touchscreen/goodix_berlin_spi.c | 172 ++++++++++++++++++++++++++
3 files changed, 186 insertions(+)
diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
index 5e21cca6025d..2d86615e5090 100644
--- a/drivers/input/touchscreen/Kconfig
+++ b/drivers/input/touchscreen/Kconfig
@@ -435,6 +435,19 @@ 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 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..3a1bc251b32d
--- /dev/null
+++ b/drivers/input/touchscreen/goodix_berlin_spi.c
@@ -0,0 +1,172 @@
+// 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 SPI_TRANS_PREFIX_LEN 1
+#define REGISTER_WIDTH 4
+#define SPI_READ_DUMMY_LEN 3
+#define SPI_READ_PREFIX_LEN (SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH + SPI_READ_DUMMY_LEN)
+#define SPI_WRITE_PREFIX_LEN (SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH)
+
+#define SPI_WRITE_FLAG 0xF0
+#define 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 ret;
+
+ if (reg_size != REGISTER_WIDTH)
+ return -EINVAL;
+
+ buf = kzalloc(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] = SPI_READ_FLAG;
+ put_unaligned_be32(*reg, buf + SPI_TRANS_PREFIX_LEN);
+ memset(buf + SPI_TRANS_PREFIX_LEN + REGISTER_WIDTH, 0xff,
+ SPI_READ_DUMMY_LEN);
+
+ xfers.tx_buf = buf;
+ xfers.rx_buf = buf;
+ xfers.len = SPI_READ_PREFIX_LEN + val_size;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ ret = spi_sync(spi, &spi_msg);
+ if (ret < 0)
+ dev_err(&spi->dev, "transfer error:%d", ret);
+ else
+ memcpy(val_buf, buf + SPI_READ_PREFIX_LEN, val_size);
+
+ kfree(buf);
+ return ret;
+}
+
+static int goodix_berlin_spi_write(void *context, const void *data,
+ size_t count)
+{
+ unsigned int len = count - 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 ret;
+
+ buf = kzalloc(SPI_WRITE_PREFIX_LEN + len, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ spi_message_init(&spi_msg);
+ memset(&xfers, 0, sizeof(xfers));
+
+ buf[0] = SPI_WRITE_FLAG;
+ put_unaligned_be32(*reg, buf + SPI_TRANS_PREFIX_LEN);
+ memcpy(buf + SPI_WRITE_PREFIX_LEN, data + REGISTER_WIDTH, len);
+
+ xfers.tx_buf = buf;
+ xfers.len = SPI_WRITE_PREFIX_LEN + len;
+ xfers.cs_change = 0;
+ spi_message_add_tail(&xfers, &spi_msg);
+
+ ret = spi_sync(spi, &spi_msg);
+ if (ret < 0)
+ dev_err(&spi->dev, "transfer error:%d", ret);
+
+ kfree(buf);
+ return ret;
+}
+
+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;
+
+ regmap_config = devm_kmemdup(&spi->dev, &goodix_berlin_spi_regmap_conf,
+ sizeof(*regmap_config), GFP_KERNEL);
+ if (!regmap_config)
+ return -ENOMEM;
+
+ 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->max_raw_read = max_size - SPI_READ_PREFIX_LEN;
+ regmap_config->max_raw_write = max_size - SPI_WRITE_PREFIX_LEN;
+
+ regmap = devm_regmap_init(&spi->dev, NULL, spi, regmap_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
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