Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2] HID: amd_sfh: if no sensors are enabled, clean up
From: Mario Limonciello @ 2023-02-03 22:08 UTC (permalink / raw)
  To: Basavaraj Natikar, Jiri Kosina
  Cc: Mario Limonciello, Xaver Hugl, Jiri Kosina, Benjamin Tissoires,
	linux-input, linux-kernel

It was reported that commit b300667b33b2 ("HID: amd_sfh: Disable the
interrupt for all command") had caused increased resume time on HP Envy
x360.

Before this commit 3 sensors were reported, but they were not actually
functional.  After this commit the sensors are no longer reported, but
also the resume time increased.

To avoid this problem explicitly look for the number of disabled sensors.
If all the sensors are disabled, clean everything up.

Fixes: b300667b33b2 ("HID: amd_sfh: Disable the interrupt for all command")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2115
Reported-by: Xaver Hugl <xaver.hugl@gmail.com>
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
v1->v2:
 * Use a boolean parameter instead
---
 drivers/hid/amd-sfh-hid/amd_sfh_client.c | 13 +++++++++++--
 drivers/hid/amd-sfh-hid/amd_sfh_hid.h    |  1 +
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
index 1fb0f7105fb21..c751d12f5df89 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
@@ -227,6 +227,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
 	cl_data->num_hid_devices = amd_mp2_get_sensor_num(privdata, &cl_data->sensor_idx[0]);
 	if (cl_data->num_hid_devices == 0)
 		return -ENODEV;
+	cl_data->is_any_sensor_enabled = false;
 
 	INIT_DELAYED_WORK(&cl_data->work, amd_sfh_work);
 	INIT_DELAYED_WORK(&cl_data->work_buffer, amd_sfh_work_buffer);
@@ -287,6 +288,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
 		status = amd_sfh_wait_for_response
 				(privdata, cl_data->sensor_idx[i], SENSOR_ENABLED);
 		if (status == SENSOR_ENABLED) {
+			cl_data->is_any_sensor_enabled = true;
 			cl_data->sensor_sts[i] = SENSOR_ENABLED;
 			rc = amdtp_hid_probe(cl_data->cur_hid_dev, cl_data);
 			if (rc) {
@@ -301,19 +303,26 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
 					cl_data->sensor_sts[i]);
 				goto cleanup;
 			}
+		} else {
+			cl_data->sensor_sts[i] = SENSOR_DISABLED;
+			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
+				cl_data->sensor_idx[i],
+				get_sensor_name(cl_data->sensor_idx[i]),
+				cl_data->sensor_sts[i]);
 		}
 		dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
 			cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
 			cl_data->sensor_sts[i]);
 	}
-	if (mp2_ops->discovery_status && mp2_ops->discovery_status(privdata) == 0) {
+	if (!cl_data->is_any_sensor_enabled ||
+	   (mp2_ops->discovery_status && mp2_ops->discovery_status(privdata) == 0)) {
 		amd_sfh_hid_client_deinit(privdata);
 		for (i = 0; i < cl_data->num_hid_devices; i++) {
 			devm_kfree(dev, cl_data->feature_report[i]);
 			devm_kfree(dev, in_data->input_report[i]);
 			devm_kfree(dev, cl_data->report_descr[i]);
 		}
-		dev_warn(dev, "Failed to discover, sensors not enabled\n");
+		dev_warn(dev, "Failed to discover, sensors not enabled is %d\n", cl_data->is_any_sensor_enabled);
 		return -EOPNOTSUPP;
 	}
 	schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP));
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
index 3754fb423e3ae..528036892c9d2 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
@@ -32,6 +32,7 @@ struct amd_input_data {
 struct amdtp_cl_data {
 	u8 init_done;
 	u32 cur_hid_dev;
+	bool is_any_sensor_enabled;
 	u32 hid_dev_count;
 	u32 num_hid_devices;
 	struct device_info *hid_devices;
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH] Input: synaptics-rmi4: Fix SPI device ID
From: Dmitry Torokhov @ 2023-02-04  0:55 UTC (permalink / raw)
  To: Rayyan Ansari; +Cc: linux-input, ~postmarketos/upstreaming, linux-kernel
In-Reply-To: <20221219133717.1638496-1-rayyan@ansari.sh>

On Mon, Dec 19, 2022 at 01:37:15PM +0000, Rayyan Ansari wrote:
> Currently, the ID being set to "rmi4_spi" causes this warning:
> "SPI driver rmi4_spi has no spi_device_id for syna,rmi4-spi"
> 
> Change the ID to rmi4-spi to stop this warning.
> 
> Signed-off-by: Rayyan Ansari <rayyan@ansari.sh>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] input: cyttsp5: Fix bitmask for touch buttons
From: Dmitry Torokhov @ 2023-02-04  0:59 UTC (permalink / raw)
  To: Richard Kjerstadius
  Cc: linux-input, Richard Kjerstadius, linus.walleij, alistair,
	linux-kernel
In-Reply-To: <20230127102903.3317089-1-richard.kjerstadius@teledyne.com>

On Fri, Jan 27, 2023 at 11:29:03AM +0100, Richard Kjerstadius wrote:
> Prior to this patch, the bitmask ends up being 0x3, as opposed to 0x1
> which likely was the intention. The erroneous bit results in the driver
> reporting 2 different button activations in designs with 2 or more
> buttons.
> 
> To detect which button has been pressed, cyttsp5_btn_attention() uses a
> for loop to iterate through the input buffer, while shifting and
> applying a bitmask to determine the state for each button.
> Unfortunately, when the bitmask is 0x3 and there are multiple buttons,
> this procedure falls apart.
> 
> Consider a design with 3 buttons. Pressing the third button will result
> in a call to cyttsp5_btn_attention() with the input buffer containing
> 0x4 (binary 0100). In the first iteration of the for loop cur_btn_state
> will be:
> 
> (0x4 >> 0 * 1) & 0x3 = 0x4 & 0x3 = 0x0
> 
> This is correct. However, in the next iteration this happens:
> 
> (0x4 >> 1 * 1) & 0x3 = 0x2 & 0x3 = 0x2
> 
> Which means that a key event for key 1 is generated, even though it's
> not really active. In the third iteration, the loop detects the button
> that was actually pressed:
> 
> (0x4 >> 2 * 1) & 0x3 = 0x1 & 0x3 = 0x1
> 
> This key event is the only one that should have been detected, but it is
> accompanied by the preceding key. Ensuring the applied mask is 0x1
> solves this problem.
> 
> Signed-off-by: Richard Kjerstadius <richard.kjerstadius@teledyne.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH] dt-bindings: input: touchscreen: st,stmfts: convert to dtschema
From: Dmitry Torokhov @ 2023-02-04  1:00 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Rob Herring, Krzysztof Kozlowski, Maxime Coquelin,
	Alexandre Torgue, linux-input, devicetree, linux-stm32,
	linux-arm-kernel, linux-kernel
In-Reply-To: <20230127202040.196411-1-krzysztof.kozlowski@linaro.org>

On Fri, Jan 27, 2023 at 09:20:40PM +0100, Krzysztof Kozlowski wrote:
> Convert the ST-Microelectronics FingerTip touchscreen controller
> bindings to DT schema.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 1/2] Input: iqs626a - drop unused device node references
From: Dmitry Torokhov @ 2023-02-04  1:21 UTC (permalink / raw)
  To: Jeff LaBundy; +Cc: robh+dt, linux-input, devicetree
In-Reply-To: <Y9RQVe/V1Hnw1oly@nixie71>

On Fri, Jan 27, 2023 at 04:29:41PM -0600, Jeff LaBundy wrote:
> Each call to device/fwnode_get_named_child_node() must be matched
> with a call to fwnode_handle_put() once the corresponding node is
> no longer in use. This ensures a reference count remains balanced
> in the case of dynamic device tree support.
> 
> Currently, the driver never calls fwnode_handle_put(); this patch
> adds the missing calls. Because fwnode_handle_put() does not take
> a const *fwnode_handle, the const qualifier is removed across all
> corresponding *fwnode_handle instances.
> 
> As part of this change, trackpad channel touch thresholds and ATI
> base values are now specified under single trackpad channel child
> nodes. This enhancement moves both properties to scalar values as
> opposed to arrays, making their types consistent across bindings.
> 
> Fixes: f1d2809de97a ("Input: Add support for Azoteq IQS626A")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* Re: [PATCH v2 2/2] dt-bindings: input: iqs626a: Redefine trackpad property types
From: Dmitry Torokhov @ 2023-02-04  1:22 UTC (permalink / raw)
  To: Jeff LaBundy; +Cc: robh+dt, linux-input, devicetree
In-Reply-To: <Y9RQcddToBr1rrnJ@nixie71>

On Fri, Jan 27, 2023 at 04:30:09PM -0600, Jeff LaBundy wrote:
> Following a recent refactor of the driver to properly drop unused
> device nodes, the driver expects trackpad channel touch thresholds
> and ATI base values to be specified under single trackpad channel
> child nodes.
> 
> This enhancement moves both properties to scalar values as opposed
> to arrays, making their types consistent across bindings.
> 
> Fixes: a8f1f0dc865c ("dt-bindings: input: Add bindings for Azoteq IQS626A")
> Signed-off-by: Jeff LaBundy <jeff@labundy.com>

Applied, thank you.

-- 
Dmitry

^ permalink raw reply

* [PATCH v2 0/4] DeviceTree Support for USB-HID Devices and CP2112
From: Danny Kaehn @ 2023-02-04  4:43 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy

This patchset allows USB-HID devices to have DeviceTree bindings through sharing
the USB of_node with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and interoperate with other drivers, and exposed the threaded
interrupt bug fixed in patch 0003.

Changes in v2:
 - Added more detail to silabs,cp2112.yaml dt-binding
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
 - Added support for setting smbus clock-frequency from DT in hid-cp2112.c
 - Added freeing of of_nodes on error paths of _probe in hid-cp2112.c

Danny Kaehn (4):
  dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
  HID: usbhid: Share USB device devicetree node with child HID device
  HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
  HID: cp2112: Devicetree Support

 .../bindings/input/silabs,cp2112.yaml         | 114 ++++++++++++++++++
 drivers/hid/hid-cp2112.c                      |  23 +++-
 drivers/hid/usbhid/hid-core.c                 |   2 +
 3 files changed, 137 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml

-- 
2.25.1


^ permalink raw reply

* [PATCH v2 1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
From: Danny Kaehn @ 2023-02-04  4:43 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230204044313.364-1-kaehndan@gmail.com>

This is a USB HID device which includes an I2C controller and 8 GPIO pins.

The binding allows describing the chip's gpio and i2c controller in DT
using the subnodes named "gpio" and "i2c", respectively. This is
intended to be used in configurations where the CP2112 is permanently
connected in hardware.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 .../bindings/input/silabs,cp2112.yaml         | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml

diff --git a/Documentation/devicetree/bindings/input/silabs,cp2112.yaml b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
new file mode 100644
index 000000000000..a8d7f8aea6c2
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
@@ -0,0 +1,114 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/silabs,cp2112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CP2112 HID USB to SMBus/I2C Bridge
+
+maintainers:
+  - Danny Kaehn <kaehndan@gmail.com>
+
+description:
+  The CP2112 is a USB HID device which includes an integrated I2C controller
+  and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
+  outputs, or push-pull outputs.
+
+properties:
+  compatible:
+    const: usb10c4,ea90
+
+  reg:
+    maxItems: 1
+    description: The USB port number on the host controller
+
+  i2c:
+    description: The SMBus/I2C controller node for the CP2112
+    $ref: /schemas/i2c/i2c-controller.yaml#
+
+    properties:
+      clock-frequency:
+        minimum: 10000
+        default: 100000
+        maximum: 400000
+
+  gpio:
+    description: The GPIO controller node for the CP2112
+    $ref: /schemas/gpio/gpio.yaml#
+
+    properties:
+      interrupt-controller: true
+      '#interrupt-cells':
+        const: 2
+
+      gpio-controller: true
+      "#gpio-cells":
+        const: 2
+
+      ngpios:
+        const: 8
+
+    patternProperties:
+      "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
+        type: object
+        properties:
+          gpio-hog: true
+          gpios: true
+          input: true
+          output-high: true
+          output-low: true
+          line-name: true
+
+        required:
+          - gpio-hog
+          - gpios
+
+        additionalProperties: false
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    usb {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      device@1 {
+        compatible = "usb10c4,ea90";
+        reg = <1>;
+
+        i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          temp@48 {
+            compatible = "national,lm75";
+            reg = <0x48>;
+          };
+        };
+
+        gpio {
+          gpio-controller;
+          interrupt-controller;
+          #gpio-cells = <2>;
+          gpio-line-names =
+            "TEST0",
+            "TEST1",
+            "TEST2",
+            "TEST3",
+            "TEST4",
+            "TEST5",
+            "TEST6",
+            "TEST7";
+        };
+      };
+    };
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 3/4] HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
From: Danny Kaehn @ 2023-02-04  4:43 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230204044313.364-1-kaehndan@gmail.com>

The CP2112 generates interrupts from a polling routine on a thread,
and can only support threaded interrupts. This patch configures the
gpiochip irq chip with this flag, disallowing consumers to request
a hard IRQ from this driver, which resulted in a segfault previously.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/hid-cp2112.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 1e16b0fa310d..27cadadda7c9 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1354,6 +1354,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	girq->parents = NULL;
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_simple_irq;
+	girq->threaded = true;
 
 	ret = gpiochip_add_data(&dev->gc, dev);
 	if (ret < 0) {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 2/4] HID: usbhid: Share USB device devicetree node with child HID device
From: Danny Kaehn @ 2023-02-04  4:43 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230204044313.364-1-kaehndan@gmail.com>

USB HID core now shares its devicetree of_node with its child HID device.
Since there can only be one HID device on a USB interface, it is redundant
to specify a hid node under the USB device (and further, binding this way
isn't currently possible, as hid_driver does not support of_match_table).

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/usbhid/hid-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index be4c731aaa65..b6c968af258f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -33,6 +33,7 @@
 #include <linux/hiddev.h>
 #include <linux/hid-debug.h>
 #include <linux/hidraw.h>
+#include <linux/device.h>
 #include "usbhid.h"
 
 /*
@@ -1369,6 +1370,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
 	hid->dev.parent = &intf->dev;
+	device_set_of_node_from_dev(&hid->dev, &intf->dev);
 	hid->bus = BUS_USB;
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v2 4/4] HID: cp2112: Devicetree Support
From: Danny Kaehn @ 2023-02-04  4:43 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230204044313.364-1-kaehndan@gmail.com>

Bind i2c and gpio interfaces to subnodes with names
"i2c" and "gpio" if they exist, respectively. This
allows the gpio and i2c controllers to be described
in DT as usual. Additionally, support configuring the
i2c bus speed from the clock-frequency property.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/hid-cp2112.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 27cadadda7c9..aa634accdfb0 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	u8 buf[3];
 	struct cp2112_smbus_config_report config;
 	struct gpio_irq_chip *girq;
+	struct i2c_timings timings;
 	int ret;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_power_normal;
 	}
 
+	dev->adap.dev.of_node   = of_get_child_by_name(hdev->dev.of_node, "i2c");
+	i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+	config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
 	config.retry_time = cpu_to_be16(1);
 
 	ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
@@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hid_err(hdev, "error setting SMBus config\n");
 		if (ret >= 0)
 			ret = -EIO;
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_set_drvdata(hdev, (void *)dev);
@@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (ret) {
 		hid_err(hdev, "error registering i2c adapter\n");
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_dbg(hdev, "adapter registered\n");
@@ -1336,6 +1341,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->gc.ngpio			= 8;
 	dev->gc.can_sleep		= 1;
 	dev->gc.parent			= &hdev->dev;
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	dev->gc.of_node			= of_get_child_by_name(hdev->dev.of_node, "gpio");
+#endif
 
 	dev->irq.name = "cp2112-gpio";
 	dev->irq.irq_startup = cp2112_gpio_irq_startup;
@@ -1376,7 +1384,12 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 err_gpiochip_remove:
 	gpiochip_remove(&dev->gc);
 err_free_i2c:
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	of_node_put(dev->gc.of_node);
+#endif
 	i2c_del_adapter(&dev->adap);
+err_free_i2c_of:
+	of_node_put(dev->adap.dev.of_node);
 err_power_normal:
 	hid_hw_power(hdev, PM_HINT_NORMAL);
 err_hid_close:
@@ -1391,6 +1404,11 @@ static void cp2112_remove(struct hid_device *hdev)
 	struct cp2112_device *dev = hid_get_drvdata(hdev);
 	int i;
 
+	of_node_put(dev->adap.dev.of_node);
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	of_node_put(dev->gc.of_node);
+#endif
+
 	sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
 	i2c_del_adapter(&dev->adap);
 
-- 
2.25.1


^ permalink raw reply related

* Re: [PATCH v2 1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
From: Krzysztof Kozlowski @ 2023-02-04 11:19 UTC (permalink / raw)
  To: Danny Kaehn, robh+dt, krzysztof.kozlowski+dt, jikos,
	benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230204044313.364-2-kaehndan@gmail.com>

On 04/02/2023 05:43, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.
> 
> The binding allows describing the chip's gpio and i2c controller in DT
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.
> 
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
>  .../bindings/input/silabs,cp2112.yaml         | 114 ++++++++++++++++++
>  1 file changed, 114 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/silabs,cp2112.yaml b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> new file mode 100644
> index 000000000000..a8d7f8aea6c2
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> @@ -0,0 +1,114 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/silabs,cp2112.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CP2112 HID USB to SMBus/I2C Bridge
> +
> +maintainers:
> +  - Danny Kaehn <kaehndan@gmail.com>
> +
> +description:
> +  The CP2112 is a USB HID device which includes an integrated I2C controller
> +  and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
> +  outputs, or push-pull outputs.
> +
> +properties:
> +  compatible:
> +    const: usb10c4,ea90
> +
> +  reg:
> +    maxItems: 1
> +    description: The USB port number on the host controller
> +
> +  i2c:
> +    description: The SMBus/I2C controller node for the CP2112
> +    $ref: /schemas/i2c/i2c-controller.yaml#

on this level: unevaluatedProperties: false

> +
> +    properties:
> +      clock-frequency:
> +        minimum: 10000
> +        default: 100000
> +        maximum: 400000
> +
> +  gpio:
> +    description: The GPIO controller node for the CP2112
> +    $ref: /schemas/gpio/gpio.yaml#

Drop it, it's always selected.

> +
> +    properties:
> +      interrupt-controller: true
> +      '#interrupt-cells':
> +        const: 2
> +
> +      gpio-controller: true
> +      "#gpio-cells":

Keep consistent quotes - either ' or "

> +        const: 2
> +
> +      ngpios:
> +        const: 8
> +
> +    patternProperties:
> +      "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
> +        type: object
> +        properties:
> +          gpio-hog: true
> +          gpios: true

minItems: 1
maxItems: 8

> +          input: true
> +          output-high: true
> +          output-low: true
> +          line-name: true
> +
> +        required:
> +          - gpio-hog
> +          - gpios
> +
> +        additionalProperties: false
> +
> +    unevaluatedProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    usb {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      device@1 {
> +        compatible = "usb10c4,ea90";
> +        reg = <1>;
> +
> +        i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          temp@48 {
> +            compatible = "national,lm75";
> +            reg = <0x48>;
> +          };
> +        };
> +
> +        gpio {
> +          gpio-controller;
> +          interrupt-controller;
> +          #gpio-cells = <2>;
> +          gpio-line-names =
> +            "TEST0",

Don't break it - should be with gpio-line-names. Align the rest of entries.

> +            "TEST1",
> +            "TEST2",
> +            "TEST3",
> +            "TEST4",
> +            "TEST5",
> +            "TEST6",
> +            "TEST7";
> +        };
> +      };
> +    };

Best regards,
Krzysztof


^ permalink raw reply

* [PATCH] Use HID descriptor for razer devices
From: Dick Marinus @ 2023-02-04 13:33 UTC (permalink / raw)
  To: linux-input

[-- Attachment #1: Type: text/plain, Size: 686 bytes --]

hid-razer currently makes some guesses which device should be used to
send control messages and the size of it.

This patch derives this information from the HID descriptors, the
Windows Drivers from Razer also do this.

The crc in the blackwidow_init packet is now calculated (copied from
openrazer GPL code).

--

This is some groundwork to support more Razer devices and features.

I don't own a Razer Black Widow keyboard and used an emulator to test
this module:

https://github.com/meeuw/razer-emulator

Any feedback on this patch is appreciated, I'd also like to ask someone
to test this with real hardware.

This patch is based on v6.1.8, please let me know if I should rebase.

[-- Attachment #2: 0001-Use-HID-descriptor-for-razer-devices.patch --]
[-- Type: text/plain, Size: 5955 bytes --]

From 68ac13b153698413118c255ff9c3abc3175f6bf3 Mon Sep 17 00:00:00 2001
From: Dick Marinus <dick@mrns.nl>
Date: Sat, 4 Feb 2023 14:14:53 +0100
Subject: [PATCH] Use HID descriptor for razer devices

hid-razer currently makes some guesses which device should be used to
send control messages and the size of it.

This patch derives this information from the HID descriptors, the
Windows Drivers from Razer also do this.

The crc in the blackwidow_init packet is now calculated (copied from
openrazer GPL code).

Signed-off-by: Dick Marinus <dick@mrns.nl>
---
 drivers/hid/hid-razer.c | 134 ++++++++++++++++++++++++++++++----------
 drivers/hid/hid-razer.h |   6 ++
 2 files changed, 107 insertions(+), 33 deletions(-)
 create mode 100644 drivers/hid/hid-razer.h

diff --git a/drivers/hid/hid-razer.c b/drivers/hid/hid-razer.c
index 740df148b..93fef4918 100644
--- a/drivers/hid/hid-razer.c
+++ b/drivers/hid/hid-razer.c
@@ -15,31 +15,15 @@
 #include <linux/wait.h>
 
 #include "hid-ids.h"
+#include "hid-razer.h"
 
 #define map_key_clear(c) hid_map_usage_clear(hi, usage, bit, max, EV_KEY, (c))
 
-#define RAZER_BLACKWIDOW_TRANSFER_BUF_SIZE	91
-
 static bool macro_key_remapping = 1;
 module_param(macro_key_remapping, bool, 0644);
 MODULE_PARM_DESC(macro_key_remapping, " on (Y) off (N)");
 
-
-static unsigned char blackwidow_init[RAZER_BLACKWIDOW_TRANSFER_BUF_SIZE] = {
-	0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x04,
-	0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-	0x04, 0x00
-};
+static unsigned char set_device_mode[] = {0x00, 0x04, 0x02, 0x00};
 
 static int razer_input_mapping(struct hid_device *hdev,
 		struct hid_input *hi, struct hid_field *field,
@@ -73,35 +57,118 @@ static int razer_input_mapping(struct hid_device *hdev,
 	return 0;
 }
 
+static bool razer_check_control_interface(struct hid_device *hdev)
+{
+	int i;
+	unsigned int hid;
+	struct hid_report *report;
+	struct hid_razer *hid_razer_drvdata;
+
+	hid_razer_drvdata = hid_get_drvdata(hdev);
+
+	list_for_each_entry(report, &hdev->report_enum[HID_FEATURE_REPORT].report_list, list) {
+		for (i = 0; i < report->maxfield; i++) {
+			hid = report->field[i]->usage->hid;
+
+			if ((hid & HID_USAGE_PAGE) == HID_UP_MSVENDOR && (hid & HID_USAGE) == 0x2) {
+				hid_razer_drvdata->report_count = report->field[i]->report_count;
+				return true;
+			}
+		}
+	}
+
+	return false;
+}
+
+static int razer_control_message(struct hid_device *hdev, unsigned char data_len, unsigned char *data)
+{
+	struct hid_razer *hid_razer_drvdata;
+	unsigned char* full_control_message;
+	unsigned char crc = 0;
+	unsigned int i;
+	unsigned report_count;
+	int ret;
+
+	if (data_len < 2) {
+		ret = -EINVAL;
+		goto cleanup_and_exit;
+	}
+
+	hid_razer_drvdata = hid_get_drvdata(hdev);
+
+	report_count = hid_razer_drvdata->report_count;
+
+	if (report_count < 2) {
+		ret = -EINVAL;
+		goto cleanup_and_exit;
+	}
+
+	printk("%i", report_count);
+	full_control_message = kzalloc(report_count + 1, GFP_KERNEL);
+
+	if (full_control_message == NULL) {
+		ret = -ENOMEM;
+		goto cleanup_and_exit;
+	}
+
+	full_control_message[6] = data_len - 2;
+	memcpy(full_control_message + 7, data, data_len);
+
+	for(i = 2; i < report_count - 2; i++) {
+		crc ^= full_control_message[i];
+	}
+	full_control_message[report_count - 1] = crc;
+
+	ret = hid_hw_raw_request(hdev, 0, full_control_message, report_count + 1, HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
+
+	if (ret != report_count + 1) {
+		ret = -EIO;
+		goto cleanup_and_exit;
+	}
+
+cleanup_and_exit:
+	kfree(full_control_message);
+
+	return 0;
+}
+
+
 static int razer_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
-	char *buf;
 	int ret = 0;
+	struct hid_razer *hid_razer_drvdata;
+
+	hid_razer_drvdata = kzalloc(sizeof(struct hid_razer), GFP_KERNEL);
+	if (hid_razer_drvdata == NULL) {
+		return -ENOMEM;
+	}
+	hid_set_drvdata(hdev, hid_razer_drvdata);
 
 	ret = hid_parse(hdev);
+
 	if (ret)
 		return ret;
 
-	/*
-	 * Only send the enable macro keys command for the third device
-	 * identified as mouse input.
-	 */
-	if (hdev->type == HID_TYPE_USBMOUSE) {
-		buf = kmemdup(blackwidow_init, RAZER_BLACKWIDOW_TRANSFER_BUF_SIZE, GFP_KERNEL);
-		if (buf == NULL)
-			return -ENOMEM;
-
-		ret = hid_hw_raw_request(hdev, 0, buf, RAZER_BLACKWIDOW_TRANSFER_BUF_SIZE,
-				HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
-		if (ret != RAZER_BLACKWIDOW_TRANSFER_BUF_SIZE)
+	if (razer_check_control_interface(hdev)) {
+		ret = razer_control_message(hdev, sizeof(set_device_mode), set_device_mode);
+		if (ret) {
 			hid_err(hdev, "failed to enable macro keys: %d\n", ret);
-
-		kfree(buf);
+			return ret;
+		}
 	}
 
 	return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
 }
 
+static void razer_remove(struct hid_device *hdev)
+{
+	struct hid_razer *hid_razer_drvdata;
+
+	hid_razer_drvdata = hid_get_drvdata(hdev);
+
+	kfree(hid_razer_drvdata);
+}
+
 static const struct hid_device_id razer_devices[] = {
 	{ HID_USB_DEVICE(USB_VENDOR_ID_RAZER,
 		USB_DEVICE_ID_RAZER_BLACKWIDOW) },
@@ -118,6 +185,7 @@ static struct hid_driver razer_driver = {
 	.id_table = razer_devices,
 	.input_mapping = razer_input_mapping,
 	.probe = razer_probe,
+	.remove = razer_remove,
 };
 module_hid_driver(razer_driver);
 
diff --git a/drivers/hid/hid-razer.h b/drivers/hid/hid-razer.h
new file mode 100644
index 000000000..ef9bb81cb
--- /dev/null
+++ b/drivers/hid/hid-razer.h
@@ -0,0 +1,6 @@
+#ifndef __HID_RAZER_H
+#define __HID_RAZER_H
+struct hid_razer {
+	unsigned report_count;
+};
+#endif
-- 
2.39.1


^ permalink raw reply related

* [PATCH v3 0/2] HID: uclogic: Add support for XP-PEN Artist 22R Pro
From: Joshua Goins @ 2023-02-04 19:20 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, Joshua Goins,
	kernel test robot, Dan Carpenter, José Expósito

Adds support for the XP-PEN Artist 22R Pro, including stylus, tablet frame
and pen pressure.

The tablet has 20 buttons, but need to be remapped in order since the
device reports invalid keycodes. Existing tablet functionality should
not be inhibited, as BTN0-8 is still used.

New initialization functions are added since the device differs slightly
from other UGEE v2 devices.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Reported-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Joshua Goins <josh@redstrate.com>
---
v3 changes:
  - Removed duplicate frame initialization code, overriding the num of frame
  buttons instead.
  - Removed unnessecary key > 0 check.
  - Renamed init function to something more specific to the device.
  - Cherry-picked probe magic data patch.
  - Rebased onto 6.2-rc6
v2 changes:
  - rewrote initialization code to be more concise, and less error-prone, thanks
  Dan Carpenter & José Expósito for pointers!
  - less struct duplication, it now uses compatiable UGEE v2 ones.
  - uclogic_extra_input_mapping is now static, thanks José again!
  - straightened out dial transformation, now there's just the two cases and
  they're decimal.
  - fixed the patch formatting, it should be easier to apply now.
  - rebased onto 6.2-rc2

Joshua Goins (1):
  HID: uclogic: Add support for XP-PEN Artist 22R Pro

José Expósito (1):
  HID: uclogic: Refactor UGEEv2 probe magic data

 drivers/hid/hid-ids.h            |   1 +
 drivers/hid/hid-uclogic-core.c   |  66 +++++++++++++-
 drivers/hid/hid-uclogic-params.c | 143 +++++++++++++++++++++++++++++--
 drivers/hid/hid-uclogic-params.h |   5 ++
 drivers/hid/hid-uclogic-rdesc.c  |  50 +++++++++++
 drivers/hid/hid-uclogic-rdesc.h  |   9 ++
 6 files changed, 264 insertions(+), 10 deletions(-)


base-commit: 0136d86b78522bbd5755f8194c97a987f0586ba5
-- 
2.39.1


^ permalink raw reply

* [PATCH v3 1/2] HID: uclogic: Refactor UGEEv2 probe magic data
From: Joshua Goins @ 2023-02-04 19:20 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel,
	José Expósito, Mia Kanashi, Andreas Grosse,
	Joshua Goins
In-Reply-To: <20230204192051.1287369-1-josh@redstrate.com>

From: José Expósito <jose.exposito89@gmail.com>

A fututure patch will need to use the array of magic data that the
UGEEv2 devices expect on probe and the endpoint number. Move them to a
common place.

Refactor, no functional changes.

Tested-by: Mia Kanashi <chad@redpilled.dev>
Tested-by: Andreas Grosse <andig.mail@t-online.de>
Signed-off-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Joshua Goins <josh@redstrate.com>
---
 drivers/hid/hid-uclogic-params.c | 11 +++++------
 drivers/hid/hid-uclogic-rdesc.c  |  6 ++++++
 drivers/hid/hid-uclogic-rdesc.h  |  5 +++++
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index 3c5eea3df328..d68cb99a03de 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -1021,8 +1021,8 @@ static int uclogic_params_huion_init(struct uclogic_params *params,
  * Returns:
  *	Zero, if successful. A negative errno code on error.
  */
-static int uclogic_probe_interface(struct hid_device *hdev, u8 *magic_arr,
-				   int magic_size, int endpoint)
+static int uclogic_probe_interface(struct hid_device *hdev, const u8 *magic_arr,
+				   size_t magic_size, int endpoint)
 {
 	struct usb_device *udev;
 	unsigned int pipe = 0;
@@ -1305,9 +1305,6 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
 	__u8 *rdesc_pen = NULL;
 	s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
 	enum uclogic_params_frame_type frame_type;
-	__u8 magic_arr[] = {
-		0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
-	};
 	/* The resulting parameters (noop) */
 	struct uclogic_params p = {0, };
 
@@ -1337,7 +1334,9 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
 	 * The specific data was discovered by sniffing the Windows driver
 	 * traffic.
 	 */
-	rc = uclogic_probe_interface(hdev, magic_arr, sizeof(magic_arr), 0x03);
+	rc = uclogic_probe_interface(hdev, uclogic_ugee_v2_probe_arr,
+				     uclogic_ugee_v2_probe_size,
+				     uclogic_ugee_v2_probe_endpoint);
 	if (rc) {
 		uclogic_params_init_invalid(&p);
 		goto output;
diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index fb40775f5f5b..b6dfdf6356a6 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c
@@ -859,6 +859,12 @@ const __u8 uclogic_rdesc_v2_frame_dial_arr[] = {
 const size_t uclogic_rdesc_v2_frame_dial_size =
 			sizeof(uclogic_rdesc_v2_frame_dial_arr);
 
+const __u8 uclogic_ugee_v2_probe_arr[] = {
+	0x02, 0xb0, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+};
+const size_t uclogic_ugee_v2_probe_size = sizeof(uclogic_ugee_v2_probe_arr);
+const int uclogic_ugee_v2_probe_endpoint = 0x03;
+
 /* Fixed report descriptor template for UGEE v2 pen reports */
 const __u8 uclogic_rdesc_ugee_v2_pen_template_arr[] = {
 	0x05, 0x0d,         /*  Usage Page (Digitizers),                */
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h
index a1f78c07293f..906d068f50a9 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h
@@ -164,6 +164,11 @@ extern const size_t uclogic_rdesc_v2_frame_dial_size;
 /* Report ID for tweaked UGEE v2 battery reports */
 #define UCLOGIC_RDESC_UGEE_V2_BATTERY_ID 0xba
 
+/* Magic data expected by UGEEv2 devices on probe */
+extern const __u8 uclogic_ugee_v2_probe_arr[];
+extern const size_t uclogic_ugee_v2_probe_size;
+extern const int uclogic_ugee_v2_probe_endpoint;
+
 /* Fixed report descriptor template for UGEE v2 pen reports */
 extern const __u8 uclogic_rdesc_ugee_v2_pen_template_arr[];
 extern const size_t uclogic_rdesc_ugee_v2_pen_template_size;
-- 
2.39.1


^ permalink raw reply related

* [PATCH v3 2/2] HID: uclogic: Add support for XP-PEN Artist 22R Pro
From: Joshua Goins @ 2023-02-04 19:20 UTC (permalink / raw)
  To: linux-input
  Cc: Jiri Kosina, Benjamin Tissoires, linux-kernel, Joshua Goins,
	kernel test robot, Dan Carpenter, José Expósito
In-Reply-To: <20230204192051.1287369-1-josh@redstrate.com>

Adds support for the XP-PEN Artist 22R Pro, including stylus, tablet frame
and pen pressure.

The tablet has 20 buttons, but need to be remapped in order since the
device reports invalid keycodes. Existing tablet functionality should
not be inhibited, as BTN0-8 is still used.

New initialization functions are added since the device differs slightly
from other UGEE v2 devices.

Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <error27@gmail.com>
Reported-by: José Expósito <jose.exposito89@gmail.com>
Signed-off-by: Joshua Goins <josh@redstrate.com>
---
 drivers/hid/hid-ids.h            |   1 +
 drivers/hid/hid-uclogic-core.c   |  66 +++++++++++++++-
 drivers/hid/hid-uclogic-params.c | 132 +++++++++++++++++++++++++++++++
 drivers/hid/hid-uclogic-params.h |   5 ++
 drivers/hid/hid-uclogic-rdesc.c  |  44 +++++++++++
 drivers/hid/hid-uclogic-rdesc.h  |   4 +
 6 files changed, 248 insertions(+), 4 deletions(-)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 0f8c11842a3a..ac8b06fed870 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1298,6 +1298,7 @@
 #define USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_L	0x0935
 #define USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_PRO_S	0x0909
 #define USB_DEVICE_ID_UGEE_XPPEN_TABLET_STAR06	0x0078
+#define USB_DEVICE_ID_UGEE_XPPEN_TABLET_22R_PRO	0x091b
 #define USB_DEVICE_ID_UGEE_TABLET_G5		0x0074
 #define USB_DEVICE_ID_UGEE_TABLET_EX07S		0x0071
 #define USB_DEVICE_ID_UGEE_TABLET_RAINBOW_CV720	0x0055
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index cfbbc39807a6..75d1fea0764c 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -81,6 +81,30 @@ static __u8 *uclogic_report_fixup(struct hid_device *hdev, __u8 *rdesc,
 	return rdesc;
 }
 
+/* Buttons considered valid tablet pad inputs. */
+static const unsigned int uclogic_extra_input_mapping[] = {
+	BTN_0,
+	BTN_1,
+	BTN_2,
+	BTN_3,
+	BTN_4,
+	BTN_5,
+	BTN_6,
+	BTN_7,
+	BTN_8,
+	BTN_RIGHT,
+	BTN_MIDDLE,
+	BTN_SIDE,
+	BTN_EXTRA,
+	BTN_FORWARD,
+	BTN_BACK,
+	BTN_B,
+	BTN_A,
+	BTN_BASE,
+	BTN_BASE2,
+	BTN_X
+};
+
 static int uclogic_input_mapping(struct hid_device *hdev,
 				 struct hid_input *hi,
 				 struct hid_field *field,
@@ -91,9 +115,27 @@ static int uclogic_input_mapping(struct hid_device *hdev,
 	struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
 	struct uclogic_params *params = &drvdata->params;
 
-	/* Discard invalid pen usages */
-	if (params->pen.usage_invalid && (field->application == HID_DG_PEN))
-		return -1;
+	if (field->application == HID_GD_KEYPAD) {
+		/*
+		 * Remap input buttons to sensible ones that are not invalid.
+		 * This only affects previous behavior for devices with more than ten or so buttons.
+		 */
+		const int key = (usage->hid & HID_USAGE) - 1;
+
+		if (key < ARRAY_SIZE(uclogic_extra_input_mapping)) {
+			hid_map_usage(hi,
+				      usage,
+				      bit,
+				      max,
+				      EV_KEY,
+				      uclogic_extra_input_mapping[key]);
+			return 1;
+		}
+	} else if (field->application == HID_DG_PEN) {
+		/* Discard invalid pen usages */
+		if (params->pen.usage_invalid)
+			return -1;
+	}
 
 	/* Let hid-core decide what to do */
 	return 0;
@@ -403,8 +445,22 @@ static int uclogic_raw_event_frame(
 
 	/* If need to, and can, transform the bitmap dial reports */
 	if (frame->bitmap_dial_byte > 0 && frame->bitmap_dial_byte < size) {
-		if (data[frame->bitmap_dial_byte] == 2)
+		switch (data[frame->bitmap_dial_byte]) {
+		case 2:
 			data[frame->bitmap_dial_byte] = -1;
+			break;
+
+		/* Everything below here is for tablets that shove multiple dials into 1 byte */
+		case 16:
+			data[frame->bitmap_dial_byte] = 0;
+			data[frame->bitmap_second_dial_destination_byte] = 1;
+			break;
+
+		case 32:
+			data[frame->bitmap_dial_byte] = 0;
+			data[frame->bitmap_second_dial_destination_byte] = -1;
+			break;
+		}
 	}
 
 	return 0;
@@ -533,6 +589,8 @@ static const struct hid_device_id uclogic_devices[] = {
 				USB_DEVICE_ID_UGEE_XPPEN_TABLET_DECO_PRO_S) },
 	{ HID_USB_DEVICE(USB_VENDOR_ID_UGEE,
 				USB_DEVICE_ID_UGEE_XPPEN_TABLET_STAR06) },
+	{ HID_USB_DEVICE(USB_VENDOR_ID_UGEE,
+				USB_DEVICE_ID_UGEE_XPPEN_TABLET_22R_PRO) },
 	{ }
 };
 MODULE_DEVICE_TABLE(hid, uclogic_devices);
diff --git a/drivers/hid/hid-uclogic-params.c b/drivers/hid/hid-uclogic-params.c
index d68cb99a03de..0f94715bb627 100644
--- a/drivers/hid/hid-uclogic-params.c
+++ b/drivers/hid/hid-uclogic-params.c
@@ -103,6 +103,8 @@ static void uclogic_params_frame_hid_dbg(
 		frame->touch_flip_at);
 	hid_dbg(hdev, "\t\t.bitmap_dial_byte = %u\n",
 		frame->bitmap_dial_byte);
+	hid_dbg(hdev, "\t\t.bitmap_second_dial_destination_byte = %u\n",
+			frame->bitmap_second_dial_destination_byte);
 }
 
 /**
@@ -1417,6 +1419,126 @@ static int uclogic_params_ugee_v2_init(struct uclogic_params *params,
 	return rc;
 }
 
+/*
+ * uclogic_params_init_ugee_xppen_pro_22r() - Initializes a UGEE XP-Pen Pro 22R tablet device.
+ *
+ * @hdev:	The HID device of the tablet interface to initialize and get
+ *		parameters from. Cannot be NULL.
+ * @params:	Parameters to fill in (to be cleaned with
+ *		uclogic_params_cleanup()). Not modified in case of error.
+ *		Cannot be NULL.
+ *
+ * Returns:
+ *	Zero, if successful. A negative errno code on error.
+ */
+static int uclogic_params_init_ugee_xppen_pro_22r(struct uclogic_params *params,
+						  struct hid_device *hdev,
+						  const u8 rdesc_frame_arr[],
+						  const size_t rdesc_frame_size)
+{
+	int rc = 0;
+	struct usb_interface *iface;
+	__u8 bInterfaceNumber;
+	const int str_desc_len = 12;
+	u8 *str_desc = NULL;
+	__u8 *rdesc_pen = NULL;
+	s32 desc_params[UCLOGIC_RDESC_PH_ID_NUM];
+	enum uclogic_params_frame_type frame_type;
+	/* The resulting parameters (noop) */
+	struct uclogic_params p = {0, };
+
+	if (!hdev || !params) {
+		rc = -EINVAL;
+		goto cleanup;
+	}
+
+	iface = to_usb_interface(hdev->dev.parent);
+	bInterfaceNumber = iface->cur_altsetting->desc.bInterfaceNumber;
+
+	/* Ignore non-pen interfaces */
+	if (bInterfaceNumber != 2) {
+		uclogic_params_init_invalid(&p);
+		goto output;
+	}
+
+	/*
+	 * Initialize the interface by sending magic data.
+	 * This magic data is the same as other UGEE v2 tablets.
+	 */
+	rc = uclogic_probe_interface(hdev,
+				     uclogic_ugee_v2_probe_arr,
+				     uclogic_ugee_v2_probe_size,
+				     uclogic_ugee_v2_probe_endpoint);
+	if (rc) {
+		uclogic_params_init_invalid(&p);
+		goto output;
+	}
+
+	/**
+	 * Read the string descriptor containing pen and frame parameters.
+	 * These are slightly different than typical UGEE v2 devices.
+	 */
+	rc = uclogic_params_get_str_desc(&str_desc, hdev, 100, str_desc_len);
+	if (rc != str_desc_len) {
+		hid_err(hdev, "failed retrieving pen and frame parameters: %d\n", rc);
+		uclogic_params_init_invalid(&p);
+		goto output;
+	}
+
+	rc = uclogic_params_parse_ugee_v2_desc(str_desc, str_desc_len,
+					       desc_params,
+					       ARRAY_SIZE(desc_params),
+					       &frame_type);
+	if (rc)
+		goto cleanup;
+
+	// str_desc doesn't report the correct amount of buttons, so manually fix it
+	desc_params[UCLOGIC_RDESC_FRAME_PH_ID_UM] = 20;
+
+	kfree(str_desc);
+	str_desc = NULL;
+
+	/* Initialize the pen interface */
+	rdesc_pen = uclogic_rdesc_template_apply(
+				uclogic_rdesc_ugee_v2_pen_template_arr,
+				uclogic_rdesc_ugee_v2_pen_template_size,
+				desc_params, ARRAY_SIZE(desc_params));
+	if (!rdesc_pen) {
+		rc = -ENOMEM;
+		goto cleanup;
+	}
+
+	p.pen.desc_ptr = rdesc_pen;
+	p.pen.desc_size = uclogic_rdesc_ugee_v2_pen_template_size;
+	p.pen.id = 0x02;
+	p.pen.subreport_list[0].value = 0xf0;
+	p.pen.subreport_list[0].id = UCLOGIC_RDESC_V1_FRAME_ID;
+
+	/* Initialize the frame interface */
+	rc = uclogic_params_frame_init_with_desc(
+		&p.frame_list[0],
+		rdesc_frame_arr,
+		rdesc_frame_size,
+		UCLOGIC_RDESC_V1_FRAME_ID);
+	if (rc < 0) {
+		hid_err(hdev, "initializing frame params failed: %d\n", rc);
+		goto output;
+	}
+
+	p.frame_list[0].bitmap_dial_byte = 7;
+	p.frame_list[0].bitmap_second_dial_destination_byte = 8;
+
+output:
+	/* Output parameters */
+	memcpy(params, &p, sizeof(*params));
+	memset(&p, 0, sizeof(p));
+	rc = 0;
+cleanup:
+	kfree(str_desc);
+	uclogic_params_cleanup(&p);
+	return rc;
+}
+
 /**
  * uclogic_params_init() - initialize a tablet interface and discover its
  * parameters.
@@ -1729,6 +1851,16 @@ int uclogic_params_init(struct uclogic_params *params,
 			uclogic_params_init_invalid(&p);
 		}
 
+		break;
+	case VID_PID(USB_VENDOR_ID_UGEE,
+			USB_DEVICE_ID_UGEE_XPPEN_TABLET_22R_PRO):
+		rc = uclogic_params_init_ugee_xppen_pro_22r(&p,
+			hdev,
+			uclogic_rdesc_xppen_artist_22r_pro_frame_arr,
+			uclogic_rdesc_xppen_artist_22r_pro_frame_size);
+		if (rc != 0)
+			goto cleanup;
+
 		break;
 	}
 
diff --git a/drivers/hid/hid-uclogic-params.h b/drivers/hid/hid-uclogic-params.h
index a97477c02ff8..6621a75a4b1a 100644
--- a/drivers/hid/hid-uclogic-params.h
+++ b/drivers/hid/hid-uclogic-params.h
@@ -171,6 +171,11 @@ struct uclogic_params_frame {
 	 * counterclockwise, as opposed to the normal 1 and -1.
 	 */
 	unsigned int bitmap_dial_byte;
+	/*
+	 * Destination offset for the second bitmap dial byte, if the tablet
+	 * supports a second dial at all.
+	 */
+	unsigned int bitmap_second_dial_destination_byte;
 };
 
 /*
diff --git a/drivers/hid/hid-uclogic-rdesc.c b/drivers/hid/hid-uclogic-rdesc.c
index b6dfdf6356a6..86293ae8c995 100644
--- a/drivers/hid/hid-uclogic-rdesc.c
+++ b/drivers/hid/hid-uclogic-rdesc.c
@@ -1191,6 +1191,50 @@ const __u8 uclogic_rdesc_xppen_deco01_frame_arr[] = {
 const size_t uclogic_rdesc_xppen_deco01_frame_size =
 			sizeof(uclogic_rdesc_xppen_deco01_frame_arr);
 
+/* Fixed report descriptor for XP-Pen Arist 22R Pro frame */
+const __u8 uclogic_rdesc_xppen_artist_22r_pro_frame_arr[] = {
+	0x05, 0x01,         /*  Usage Page (Desktop),                       */
+	0x09, 0x07,         /*  Usage (Keypad),                             */
+	0xA1, 0x01,         /*  Collection (Application),                   */
+	0x85, UCLOGIC_RDESC_V1_FRAME_ID,
+	/*      Report ID (Virtual report),             */
+	0x05, 0x0D,         /*     Usage Page (Digitizer),                  */
+	0x09, 0x39,         /*      Usage (Tablet Function Keys),           */
+	0xA0,               /*      Collection (Physical),                  */
+	0x14,               /*          Logical Minimum (0),                */
+	0x25, 0x01,         /*          Logical Maximum (1),                */
+	0x75, 0x01,         /*          Report Size (1),                    */
+	0x95, 0x08,         /*          Report Count (8),                   */
+	0x81, 0x01,         /*          Input (Constant),                   */
+	0x05, 0x09,         /*          Usage Page (Button),                */
+	0x19, 0x01,         /*          Usage Minimum (01h),                */
+	0x29, 0x14,         /*          Usage Maximum (14h),                */
+	0x95, 0x14,         /*          Report Count (20),                  */
+	0x81, 0x02,         /*          Input (Variable),                   */
+	0x95, 0x14,         /*          Report Count (20),                  */
+	0x81, 0x01,         /*          Input (Constant),                   */
+	0x05, 0x01,         /*          Usage Page (Desktop),               */
+	0x09, 0x38,         /*          Usage (Wheel),                      */
+	0x75, 0x08,         /*          Report Size (8),                    */
+	0x95, 0x01,         /*          Report Count (1),                   */
+	0x15, 0xFF,         /*          Logical Minimum (-1),               */
+	0x25, 0x08,         /*          Logical Maximum (8),                */
+	0x81, 0x06,         /*          Input (Variable, Relative),         */
+	0x05, 0x0C,         /*          Usage Page (Consumer Devices),      */
+	0x0A, 0x38, 0x02,   /*          Usage (AC PAN),                     */
+	0x95, 0x01,         /*          Report Count (1),                   */
+	0x81, 0x06,         /*          Input (Variable, Relative),         */
+	0x26, 0xFF, 0x00,   /*          Logical Maximum (255),              */
+	0x75, 0x08,         /*          Report Size (8),                    */
+	0x95, 0x01,         /*          Report Count (1),                   */
+	0x81, 0x02,         /*          Input (Variable),                   */
+	0xC0,               /*      End Collection                          */
+	0xC0,               /*  End Collection                              */
+};
+
+const size_t uclogic_rdesc_xppen_artist_22r_pro_frame_size =
+				sizeof(uclogic_rdesc_xppen_artist_22r_pro_frame_arr);
+
 /**
  * uclogic_rdesc_template_apply() - apply report descriptor parameters to a
  * report descriptor template, creating a report descriptor. Copies the
diff --git a/drivers/hid/hid-uclogic-rdesc.h b/drivers/hid/hid-uclogic-rdesc.h
index 906d068f50a9..c3cb2c75dda5 100644
--- a/drivers/hid/hid-uclogic-rdesc.h
+++ b/drivers/hid/hid-uclogic-rdesc.h
@@ -210,4 +210,8 @@ extern const size_t uclogic_rdesc_ugee_g5_frame_size;
 /* Least-significant bit of Ugee G5 frame rotary encoder state */
 #define UCLOGIC_RDESC_UGEE_G5_FRAME_RE_LSB 38
 
+/* Fixed report descriptor for XP-Pen Arist 22R Pro frame */
+extern const __u8 uclogic_rdesc_xppen_artist_22r_pro_frame_arr[];
+extern const size_t uclogic_rdesc_xppen_artist_22r_pro_frame_size;
+
 #endif /* _HID_UCLOGIC_RDESC_H */
-- 
2.39.1


^ permalink raw reply related

* [PATCH v3 0/4] DeviceTree Support for USB-HID Devices and CP2112
From: Danny Kaehn @ 2023-02-05 14:54 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy

This patchset allows USB-HID devices to have DeviceTree bindings through sharing
the USB of_node with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and interoperate with other drivers, and exposed the threaded
interrupt bug fixed in patch 0003.

Changes in v3:
 - Additional fixups to silabs,cp2112.yaml to address comments

Changes in v2:
 - Added more detail to silabs,cp2112.yaml dt-binding
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
 - Added support for setting smbus clock-frequency from DT in hid-cp2112.c
 - Added freeing of of_nodes on error paths of _probe in hid-cp2112.c

Danny Kaehn (4):
  dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
  HID: usbhid: Share USB device devicetree node with child HID device
  HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
  HID: cp2112: Devicetree Support

 .../bindings/input/silabs,cp2112.yaml         | 112 ++++++++++++++++++
 drivers/hid/hid-cp2112.c                      |  23 +++-
 drivers/hid/usbhid/hid-core.c                 |   2 +
 3 files changed, 135 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml

-- 
2.25.1


^ permalink raw reply

* [PATCH v3 1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
From: Danny Kaehn @ 2023-02-05 14:54 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-1-kaehndan@gmail.com>

This is a USB HID device which includes an I2C controller and 8 GPIO pins.

The binding allows describing the chip's gpio and i2c controller in DT
using the subnodes named "gpio" and "i2c", respectively. This is
intended to be used in configurations where the CP2112 is permanently
connected in hardware.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 .../bindings/input/silabs,cp2112.yaml         | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml

diff --git a/Documentation/devicetree/bindings/input/silabs,cp2112.yaml b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
new file mode 100644
index 000000000000..eb2e89edb80a
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
@@ -0,0 +1,112 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/silabs,cp2112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CP2112 HID USB to SMBus/I2C Bridge
+
+maintainers:
+  - Danny Kaehn <kaehndan@gmail.com>
+
+description:
+  The CP2112 is a USB HID device which includes an integrated I2C controller
+  and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
+  outputs, or push-pull outputs.
+
+properties:
+  compatible:
+    const: usb10c4,ea90
+
+  reg:
+    maxItems: 1
+    description: The USB port number on the host controller
+
+  i2c:
+    description: The SMBus/I2C controller node for the CP2112
+    $ref: /schemas/i2c/i2c-controller.yaml#
+    unevaluatedProperties: false
+    properties:
+      clock-frequency:
+        minimum: 10000
+        default: 100000
+        maximum: 400000
+
+  gpio:
+    description: The GPIO controller node for the CP2112
+    type: object
+    properties:
+      interrupt-controller: true
+      "#interrupt-cells":
+        const: 2
+
+      gpio-controller: true
+      "#gpio-cells":
+        const: 2
+
+      ngpios:
+        const: 8
+
+      gpio-line-names:
+        minItems: 1
+        maxItems: 8
+
+    patternProperties:
+      "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":
+        type: object
+        properties:
+          gpio-hog: true
+          input: true
+          output-high: true
+          output-low: true
+          line-name: true
+          gpios:
+            minItems: 1
+            maxItems: 8
+
+        required:
+          - gpio-hog
+          - gpios
+
+        additionalProperties: false
+
+    unevaluatedProperties: false
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    usb {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      device@1 {
+        compatible = "usb10c4,ea90";
+        reg = <1>;
+
+        i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+
+          temp@48 {
+            compatible = "national,lm75";
+            reg = <0x48>;
+          };
+        };
+
+        gpio {
+          gpio-controller;
+          interrupt-controller;
+          #gpio-cells = <2>;
+          gpio-line-names = "TEST0", "TEST1", "TEST2",
+            "TEST3", "TEST4", "TEST5", "TEST6", "TEST7";
+        };
+      };
+    };
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 2/4] HID: usbhid: Share USB device devicetree node with child HID device
From: Danny Kaehn @ 2023-02-05 14:54 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-1-kaehndan@gmail.com>

USB HID core now shares its devicetree of_node with its child HID device.
Since there can only be one HID device on a USB interface, it is redundant
to specify a hid node under the USB device (and further, binding this way
isn't currently possible, as hid_driver does not support of_match_table).

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/usbhid/hid-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index be4c731aaa65..b6c968af258f 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -33,6 +33,7 @@
 #include <linux/hiddev.h>
 #include <linux/hid-debug.h>
 #include <linux/hidraw.h>
+#include <linux/device.h>
 #include "usbhid.h"
 
 /*
@@ -1369,6 +1370,7 @@ static int usbhid_probe(struct usb_interface *intf, const struct usb_device_id *
 	hid->hiddev_report_event = hiddev_report_event;
 #endif
 	hid->dev.parent = &intf->dev;
+	device_set_of_node_from_dev(&hid->dev, &intf->dev);
 	hid->bus = BUS_USB;
 	hid->vendor = le16_to_cpu(dev->descriptor.idVendor);
 	hid->product = le16_to_cpu(dev->descriptor.idProduct);
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 3/4] HID: cp2112: Fix driver not registering GPIO IRQ chip as threaded
From: Danny Kaehn @ 2023-02-05 14:54 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-1-kaehndan@gmail.com>

The CP2112 generates interrupts from a polling routine on a thread,
and can only support threaded interrupts. This patch configures the
gpiochip irq chip with this flag, disallowing consumers to request
a hard IRQ from this driver, which resulted in a segfault previously.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/hid-cp2112.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 1e16b0fa310d..27cadadda7c9 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1354,6 +1354,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	girq->parents = NULL;
 	girq->default_type = IRQ_TYPE_NONE;
 	girq->handler = handle_simple_irq;
+	girq->threaded = true;
 
 	ret = gpiochip_add_data(&dev->gc, dev);
 	if (ret < 0) {
-- 
2.25.1


^ permalink raw reply related

* [PATCH v3 4/4] HID: cp2112: Devicetree Support
From: Danny Kaehn @ 2023-02-05 14:54 UTC (permalink / raw)
  To: robh+dt, krzysztof.kozlowski+dt, jikos, benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-1-kaehndan@gmail.com>

Bind i2c and gpio interfaces to subnodes with names
"i2c" and "gpio" if they exist, respectively. This
allows the gpio and i2c controllers to be described
in DT as usual. Additionally, support configuring the
i2c bus speed from the clock-frequency property.

Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
---
 drivers/hid/hid-cp2112.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 27cadadda7c9..aa634accdfb0 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1234,6 +1234,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	u8 buf[3];
 	struct cp2112_smbus_config_report config;
 	struct gpio_irq_chip *girq;
+	struct i2c_timings timings;
 	int ret;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1292,6 +1293,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_power_normal;
 	}
 
+	dev->adap.dev.of_node   = of_get_child_by_name(hdev->dev.of_node, "i2c");
+	i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+	config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
 	config.retry_time = cpu_to_be16(1);
 
 	ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
@@ -1300,7 +1305,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		hid_err(hdev, "error setting SMBus config\n");
 		if (ret >= 0)
 			ret = -EIO;
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_set_drvdata(hdev, (void *)dev);
@@ -1322,7 +1327,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	if (ret) {
 		hid_err(hdev, "error registering i2c adapter\n");
-		goto err_power_normal;
+		goto err_free_i2c_of;
 	}
 
 	hid_dbg(hdev, "adapter registered\n");
@@ -1336,6 +1341,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	dev->gc.ngpio			= 8;
 	dev->gc.can_sleep		= 1;
 	dev->gc.parent			= &hdev->dev;
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	dev->gc.of_node			= of_get_child_by_name(hdev->dev.of_node, "gpio");
+#endif
 
 	dev->irq.name = "cp2112-gpio";
 	dev->irq.irq_startup = cp2112_gpio_irq_startup;
@@ -1376,7 +1384,12 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 err_gpiochip_remove:
 	gpiochip_remove(&dev->gc);
 err_free_i2c:
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	of_node_put(dev->gc.of_node);
+#endif
 	i2c_del_adapter(&dev->adap);
+err_free_i2c_of:
+	of_node_put(dev->adap.dev.of_node);
 err_power_normal:
 	hid_hw_power(hdev, PM_HINT_NORMAL);
 err_hid_close:
@@ -1391,6 +1404,11 @@ static void cp2112_remove(struct hid_device *hdev)
 	struct cp2112_device *dev = hid_get_drvdata(hdev);
 	int i;
 
+	of_node_put(dev->adap.dev.of_node);
+#if IS_ENABLED(CONFIG_OF_GPIO)
+	of_node_put(dev->gc.of_node);
+#endif
+
 	sysfs_remove_group(&hdev->dev.kobj, &cp2112_attr_group);
 	i2c_del_adapter(&dev->adap);
 
-- 
2.25.1


^ permalink raw reply related

* Re: remove arch/sh
From: Stephen Rothwell @ 2023-02-05 23:08 UTC (permalink / raw)
  To: John Paul Adrian Glaubitz
  Cc: Christoph Hellwig, Yoshinori Sato, Rich Felker, Arnd Bergmann,
	Greg Kroah-Hartman, Laurent Pinchart, Kieran Bingham,
	Geert Uytterhoeven, linux-kernel, linux-watchdog, devicetree,
	linux-arch, dmaengine, dri-devel, linux-renesas-soc, linux-i2c,
	linux-input, linux-media, linux-mmc, linux-mtd, netdev,
	linux-gpio, linux-rtc, linux-spi, linux-serial, linux-usb,
	linux-fbdev, alsa-devel, linux-sh
In-Reply-To: <20230203083037.GA30738@lst.de>

[-- Attachment #1: Type: text/plain, Size: 1305 bytes --]

Hi,

On Fri, 3 Feb 2023 09:30:37 +0100 Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Feb 03, 2023 at 09:24:46AM +0100, John Paul Adrian Glaubitz wrote:
> > Since this is my very first time stepping up as a kernel maintainer, I was hoping
> > to get some pointers on what to do to make this happen.
> > 
> > So far, we have set up a new kernel tree and I have set up a local development and
> > test environment for SH kernels using my SH7785LCR board as the target platform.
> > 
> > Do I just need to send a patch asking to change the corresponding entry in the
> > MAINTAINERS file?  
> 
> I'm not sure a there is a document, but:
> 
>  - add the MAINTAINERS change to your tree
>  - ask Stephen to get your tree included in linux-next

And by "Stephen", Christoph means me.  When you are ready, please send
me a request to include your tree/branch in linux-next (usually the
branch is called something like "for-next" or just "next") telling me
the git URL, and the contacts I should send email to if there are
conflicts/build issues with the branch.  I will then fetch the branch
every time I create a new linux-next release (most work days), so all
you need to do is update that branch each time you are ready to publish
more commits.

-- 
Cheers,
Stephen Rothwell

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: remove arch/sh
From: John Paul Adrian Glaubitz @ 2023-02-05 23:20 UTC (permalink / raw)
  To: Stephen Rothwell
  Cc: Christoph Hellwig, Yoshinori Sato, Rich Felker, Arnd Bergmann,
	Greg Kroah-Hartman, Laurent Pinchart, Kieran Bingham,
	Geert Uytterhoeven, linux-kernel, linux-watchdog, devicetree,
	linux-arch, dmaengine, dri-devel, linux-renesas-soc, linux-i2c,
	linux-input, linux-media, linux-mmc, linux-mtd, netdev,
	linux-gpio, linux-rtc, linux-spi, linux-serial, linux-usb,
	linux-fbdev, alsa-devel, linux-sh
In-Reply-To: <20230206100856.603a0f8f@canb.auug.org.au>

Hi Stephen!

On Mon, 2023-02-06 at 10:08 +1100, Stephen Rothwell wrote:
> Hi,
> 
> On Fri, 3 Feb 2023 09:30:37 +0100 Christoph Hellwig <hch@lst.de> wrote:
> > 
> > On Fri, Feb 03, 2023 at 09:24:46AM +0100, John Paul Adrian Glaubitz wrote:
> > > Since this is my very first time stepping up as a kernel maintainer, I was hoping
> > > to get some pointers on what to do to make this happen.
> > > 
> > > So far, we have set up a new kernel tree and I have set up a local development and
> > > test environment for SH kernels using my SH7785LCR board as the target platform.
> > > 
> > > Do I just need to send a patch asking to change the corresponding entry in the
> > > MAINTAINERS file?  
> > 
> > I'm not sure a there is a document, but:
> > 
> >  - add the MAINTAINERS change to your tree
> >  - ask Stephen to get your tree included in linux-next
> 
> And by "Stephen", Christoph means me.  When you are ready, please send
> me a request to include your tree/branch in linux-next (usually the
> branch is called something like "for-next" or just "next") telling me
> the git URL, and the contacts I should send email to if there are
> conflicts/build issues with the branch.  I will then fetch the branch
> every time I create a new linux-next release (most work days), so all
> you need to do is update that branch each time you are ready to publish
> more commits.

Thanks a lot! I will start with that tomorrow with Geert giving me some guidance.

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

^ permalink raw reply

* Re: [PATCH v2] HID: amd_sfh: if no sensors are enabled, clean up
From: Basavaraj Natikar @ 2023-02-06  5:52 UTC (permalink / raw)
  To: Mario Limonciello, Basavaraj Natikar, Jiri Kosina
  Cc: Xaver Hugl, Jiri Kosina, Benjamin Tissoires, linux-input,
	linux-kernel
In-Reply-To: <20230203220850.13924-1-mario.limonciello@amd.com>


On 2/4/2023 3:38 AM, Mario Limonciello wrote:
> It was reported that commit b300667b33b2 ("HID: amd_sfh: Disable the
> interrupt for all command") had caused increased resume time on HP Envy
> x360.
>
> Before this commit 3 sensors were reported, but they were not actually
> functional.  After this commit the sensors are no longer reported, but
> also the resume time increased.
>
> To avoid this problem explicitly look for the number of disabled sensors.
> If all the sensors are disabled, clean everything up.
>
> Fixes: b300667b33b2 ("HID: amd_sfh: Disable the interrupt for all command")
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2115
> Reported-by: Xaver Hugl <xaver.hugl@gmail.com>
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
> v1->v2:
>  * Use a boolean parameter instead
> ---
>  drivers/hid/amd-sfh-hid/amd_sfh_client.c | 13 +++++++++++--
>  drivers/hid/amd-sfh-hid/amd_sfh_hid.h    |  1 +
>  2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_client.c b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> index 1fb0f7105fb21..c751d12f5df89 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_client.c
> @@ -227,6 +227,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
>  	cl_data->num_hid_devices = amd_mp2_get_sensor_num(privdata, &cl_data->sensor_idx[0]);
>  	if (cl_data->num_hid_devices == 0)
>  		return -ENODEV;
> +	cl_data->is_any_sensor_enabled = false;
>  
>  	INIT_DELAYED_WORK(&cl_data->work, amd_sfh_work);
>  	INIT_DELAYED_WORK(&cl_data->work_buffer, amd_sfh_work_buffer);
> @@ -287,6 +288,7 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
>  		status = amd_sfh_wait_for_response
>  				(privdata, cl_data->sensor_idx[i], SENSOR_ENABLED);
>  		if (status == SENSOR_ENABLED) {
> +			cl_data->is_any_sensor_enabled = true;
>  			cl_data->sensor_sts[i] = SENSOR_ENABLED;
>  			rc = amdtp_hid_probe(cl_data->cur_hid_dev, cl_data);
>  			if (rc) {
> @@ -301,19 +303,26 @@ int amd_sfh_hid_client_init(struct amd_mp2_dev *privdata)
>  					cl_data->sensor_sts[i]);
>  				goto cleanup;
>  			}
> +		} else {
> +			cl_data->sensor_sts[i] = SENSOR_DISABLED;
> +			dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
> +				cl_data->sensor_idx[i],
> +				get_sensor_name(cl_data->sensor_idx[i]),
> +				cl_data->sensor_sts[i]);
>  		}
>  		dev_dbg(dev, "sid 0x%x (%s) status 0x%x\n",
>  			cl_data->sensor_idx[i], get_sensor_name(cl_data->sensor_idx[i]),
>  			cl_data->sensor_sts[i]);
>  	}
> -	if (mp2_ops->discovery_status && mp2_ops->discovery_status(privdata) == 0) {
> +	if (!cl_data->is_any_sensor_enabled ||
> +	   (mp2_ops->discovery_status && mp2_ops->discovery_status(privdata) == 0)) {
>  		amd_sfh_hid_client_deinit(privdata);
>  		for (i = 0; i < cl_data->num_hid_devices; i++) {
>  			devm_kfree(dev, cl_data->feature_report[i]);
>  			devm_kfree(dev, in_data->input_report[i]);
>  			devm_kfree(dev, cl_data->report_descr[i]);
>  		}
> -		dev_warn(dev, "Failed to discover, sensors not enabled\n");
> +		dev_warn(dev, "Failed to discover, sensors not enabled is %d\n", cl_data->is_any_sensor_enabled);
>  		return -EOPNOTSUPP;
>  	}
>  	schedule_delayed_work(&cl_data->work_buffer, msecs_to_jiffies(AMD_SFH_IDLE_LOOP));
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> index 3754fb423e3ae..528036892c9d2 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.h
> @@ -32,6 +32,7 @@ struct amd_input_data {
>  struct amdtp_cl_data {
>  	u8 init_done;
>  	u32 cur_hid_dev;
> +	bool is_any_sensor_enabled;
>  	u32 hid_dev_count;
>  	u32 num_hid_devices;
>  	struct device_info *hid_devices;

Changes looks good to me

Acked-by: Basavaraj Natikar <Basavaraj.Natikar@amd.com>

Thanks,
--
Basavaraj



^ permalink raw reply

* Re: [PATCH v3 1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge
From: Krzysztof Kozlowski @ 2023-02-06  7:59 UTC (permalink / raw)
  To: Danny Kaehn, robh+dt, krzysztof.kozlowski+dt, jikos,
	benjamin.tissoires
  Cc: devicetree, linux-input, ethan.twardy
In-Reply-To: <20230205145450.3396-2-kaehndan@gmail.com>

On 05/02/2023 15:54, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.

I actually wonder - which part of CP2112 is input or HID related? The
manufacturer advertises it as USB to SMBus bridge, so it is an I2C
controller, thus should be in i2c directory.

> 
> The binding allows describing the chip's gpio and i2c controller in DT
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.


Best regards,
Krzysztof


^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox