linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
@ 2024-06-05 23:12 Danny Kaehn
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
                   ` (6 more replies)
  0 siblings, 7 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-05 23:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

This patchset allows USB-HID devices to have Firmware bindings through sharing
the USB fwnode 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 ACPI and interoperate with other drivers.

Changes in v11:
- Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
    from Rob H.
- Edit hid-cp2112.c to match for ACPI index and fall back to matching by
    name (instead of the other way around)
- Separate CP2112 I2C bus speed configuration into a separate patch

Changes in v10:
- Define an enumeration and mapping for CP2112 ACPI _ADRs and devicetree
    child node names, and use these in the scanning of child nodes
- Address other miscellaneous

Changes in v9:
- Add _ADR-based ACPI binding of child nodes (I2C is _ADR Zero, GPIO is _ADR One)
- Use a loop-based approach for assigning child nodes within probe().
    As a consequence, hid-cp2112.c no longer maintains references to the
    child fwnodes during the lifetime of the device. (plese correct if this
    is actually needed for this use-case)

Changes in v8:
- Apply Review tags retroactively to patches previously reviewed

Changes in v7:
- Use dev_fwnode when calling fwnod_handle_put in i2c_adapter in hid-cp2112.c
- Capitalize I2C and GPIO in commit message for patch 0003

Changes in v6:
- Fix fwnode_handle reference leaks in hid-cp21112.c
- Simplify hog node pattern in silabs,cp2112.yaml

Changes in v5:
 - Use fwnode API instead of of_node api in hid-core.c and hid-cp2112.c
 - Include sda-gpios and scl-gpios in silabs,cp2112.yaml
 - Additional fixups to silabs,cp2112.yaml to address comments
 - Submit threaded interrupt bugfix separately from this patchset, as requested

Changes in v4:
 - Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c

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 (3):
  dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  HID: usbhid: Share USB device firmware node with child HID device
  HID: cp2112: Fwnode Support

 .../bindings/i2c/silabs,cp2112.yaml           | 113 ++++++++++++++++++
 drivers/hid/hid-cp2112.c                      |  50 ++++++++
 drivers/hid/usbhid/hid-core.c                 |   2 +
 3 files changed, 165 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml

--
2.25.1

---
Danny Kaehn (4):
      dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
      HID: usbhid: Share USB device firmware node with child HID device
      HID: cp2112: Fwnode Support
      HID: cp2112: Configure I2C Bus Speed from Firmware

 .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
 drivers/hid/hid-cp2112.c                           |  43 +++++++++
 drivers/hid/usbhid/hid-core.c                      |   2 +
 3 files changed, 150 insertions(+)
---
base-commit: 4f54308c970692e66a2a354ac2bde32f228cedeb
change-id: 20240605-cp2112-dt-7cdc95448e8a

Best regards,
-- 
Danny Kaehn <danny.kaehn@plexus.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
@ 2024-06-05 23:12 ` Danny Kaehn
  2024-06-06  0:18   ` Rob Herring (Arm)
                     ` (2 more replies)
  2024-06-05 23:12 ` [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device Danny Kaehn
                   ` (5 subsequent siblings)
  6 siblings, 3 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-05 23:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

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 <danny.kaehn@plexus.com>
---
 .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
 1 file changed, 105 insertions(+)

diff --git a/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
new file mode 100644
index 000000000000..0108f2e43c8c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/silabs,cp2112.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: CP2112 HID USB to SMBus/I2C Bridge
+
+maintainers:
+  - Danny Kaehn <danny.kaehn@plexus.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:
+      sda-gpios:
+        maxItems: 1
+
+      scl-gpios:
+        maxItems: 1
+
+      clock-frequency:
+        minimum: 10000
+        default: 100000
+        maximum: 400000
+
+  interrupt-controller: true
+  "#interrupt-cells":
+    const: 2
+
+  gpio-controller: true
+  "#gpio-cells":
+    const: 2
+
+  gpio-line-names:
+    minItems: 1
+    maxItems: 8
+
+patternProperties:
+  "-hog(-[0-9]+)?$":
+    type: object
+
+    required:
+      - gpio-hog
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+
+    usb {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      cp2112: device@1 {
+        compatible = "usb10c4,ea90";
+        reg = <1>;
+
+        i2c {
+          #address-cells = <1>;
+          #size-cells = <0>;
+          sda-gpios = <&cp2112 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+          scl-gpios = <&cp2112 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+
+          temp@48 {
+            compatible = "national,lm75";
+            reg = <0x48>;
+          };
+        };
+
+        gpio-controller;
+        interrupt-controller;
+        #gpio-cells = <2>;
+        gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
+          "TEST3","TEST4", "TEST5", "TEST6";
+
+        fan-rst-hog {
+            gpio-hog;
+            gpios = <7 GPIO_ACTIVE_HIGH>;
+            output-high;
+            line-name = "FAN_RST";
+        };
+      };
+    };

-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
@ 2024-06-05 23:12 ` Danny Kaehn
  2024-06-05 23:43   ` Andy Shevchenko
  2024-06-05 23:12 ` [PATCH v11 3/4] HID: cp2112: Fwnode Support Danny Kaehn
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Danny Kaehn @ 2024-06-05 23:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

USB HID core now shares its fwnode 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. This allows usb HID device
drivers to be described in firmware and make use of device properties.

Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.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 a90ed2ceae84..cb687ea7325c 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -19,6 +19,7 @@
 #include <linux/list.h>
 #include <linux/mm.h>
 #include <linux/mutex.h>
+#include <linux/property.h>
 #include <linux/spinlock.h>
 #include <asm/unaligned.h>
 #include <asm/byteorder.h>
@@ -1374,6 +1375,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_node(&hid->dev, dev_fwnode(&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	[flat|nested] 24+ messages in thread

* [PATCH v11 3/4] HID: cp2112: Fwnode Support
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
  2024-06-05 23:12 ` [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device Danny Kaehn
@ 2024-06-05 23:12 ` Danny Kaehn
  2024-06-05 23:39   ` Andy Shevchenko
  2024-06-05 23:12 ` [PATCH v11 4/4] HID: cp2112: Configure I2C Bus Speed from Firmware Danny Kaehn
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 24+ messages in thread
From: Danny Kaehn @ 2024-06-05 23:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

Support describing the CP2112's I2C and GPIO interfaces in firmware.

The GPIO chip shares a firmware node with the CP2112. The I2C child
node can either be a child with the name "i2c" or, in ACPI, a device
node with _ADR Zero.

Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
 drivers/hid/hid-cp2112.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 20a0d1315d90..b78d81275065 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -27,6 +27,22 @@
 #include <linux/usb/ch9.h>
 #include "hid-ids.h"
 
+/**
+ * enum cp2112_child_acpi_cell_addrs - Child ACPI addresses for CP2112 sub-functions
+ * @CP2112_I2C_ADR: Address for I2C node
+ */
+enum cp2112_child_acpi_cell_addrs {
+	CP2112_I2C_ADR = 0,
+};
+
+/*
+ * CP2112 Fwnode child names.
+ * CP2112 sub-functions can be described by named fwnode children or by ACPI _ADR
+ */
+static const char * const cp2112_cell_names[] = {
+	[CP2112_I2C_ADR]	= "i2c",
+};
+
 #define CP2112_REPORT_MAX_LENGTH		64
 #define CP2112_GPIO_CONFIG_LENGTH		5
 #define CP2112_GPIO_GET_LENGTH			2
@@ -1195,7 +1211,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	struct cp2112_device *dev;
 	u8 buf[3];
 	struct cp2112_smbus_config_report config;
+	struct fwnode_handle *child;
 	struct gpio_irq_chip *girq;
+	const char *name;
+	u32 addr;
 	int ret;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1209,6 +1228,26 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 
 	mutex_init(&dev->lock);
 
+	device_for_each_child_node(&hdev->dev, child) {
+		ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
+		if (ret) {
+			name = fwnode_get_name(child);
+			if (!name)
+				continue;
+			ret = match_string(cp2112_cell_names,
+					ARRAY_SIZE(cp2112_cell_names), name);
+			if (ret < 0)
+				continue;
+			addr = ret;
+		}
+
+		switch (addr) {
+		case CP2112_I2C_ADR:
+			device_set_node(&dev->adap.dev, child);
+			break;
+		}
+	}
+
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "parse failed\n");

-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH v11 4/4] HID: cp2112: Configure I2C Bus Speed from Firmware
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
                   ` (2 preceding siblings ...)
  2024-06-05 23:12 ` [PATCH v11 3/4] HID: cp2112: Fwnode Support Danny Kaehn
@ 2024-06-05 23:12 ` Danny Kaehn
  2024-06-05 23:42 ` [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Andy Shevchenko
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-05 23:12 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

Now that the I2C adapter on the CP2112 can have an associated firmware
node, set the bus speed based on firmware configuration

Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
 drivers/hid/hid-cp2112.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index b78d81275065..547c2cbd410f 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1213,6 +1213,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 	struct cp2112_smbus_config_report config;
 	struct fwnode_handle *child;
 	struct gpio_irq_chip *girq;
+	struct i2c_timings timings;
 	const char *name;
 	u32 addr;
 	int ret;
@@ -1293,6 +1294,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		goto err_power_normal;
 	}
 
+	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),

-- 
2.25.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 3/4] HID: cp2112: Fwnode Support
  2024-06-05 23:12 ` [PATCH v11 3/4] HID: cp2112: Fwnode Support Danny Kaehn
@ 2024-06-05 23:39   ` Andy Shevchenko
  0 siblings, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2024-06-05 23:39 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Wed, Jun 05, 2024 at 06:12:46PM -0500, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
> 
> The GPIO chip shares a firmware node with the CP2112. The I2C child
> node can either be a child with the name "i2c" or, in ACPI, a device
> node with _ADR Zero.

...

> +	device_for_each_child_node(&hdev->dev, child) {
> +		ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +		if (ret) {
> +			name = fwnode_get_name(child);
> +			if (!name)
> +				continue;
> +			ret = match_string(cp2112_cell_names,
> +					ARRAY_SIZE(cp2112_cell_names), name);
> +			if (ret < 0)
> +				continue;
> +			addr = ret;
> +		}
> +
> +		switch (addr) {
> +		case CP2112_I2C_ADR:
> +			device_set_node(&dev->adap.dev, child);
> +			break;

		default:
			break;

?

> +		}
> +	}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
                   ` (3 preceding siblings ...)
  2024-06-05 23:12 ` [PATCH v11 4/4] HID: cp2112: Configure I2C Bus Speed from Firmware Danny Kaehn
@ 2024-06-05 23:42 ` Andy Shevchenko
  2024-06-06 15:54   ` Danny Kaehn
  2024-06-06  7:30 ` (subset) " Benjamin Tissoires
  2025-07-29 14:53 ` Re " Willie Thai
  6 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2024-06-05 23:42 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Wed, Jun 05, 2024 at 06:12:43PM -0500, Danny Kaehn wrote:
> This patchset allows USB-HID devices to have Firmware bindings through sharing
> the USB fwnode 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 ACPI and interoperate with other drivers.

...

> Changes in v11:
> - Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
>     from Rob H.

Hmm... I don't know much about DT, but how is this supposed to work in ACPI?
I mean if we want to refer to the GPIO in GpioIo() or GpioInt() resources,
what should we put there as ACPI path?

> - Edit hid-cp2112.c to match for ACPI index and fall back to matching by
>     name (instead of the other way around)
> - Separate CP2112 I2C bus speed configuration into a separate patch


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device
  2024-06-05 23:12 ` [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device Danny Kaehn
@ 2024-06-05 23:43   ` Andy Shevchenko
  2024-06-06  7:31     ` Benjamin Tissoires
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2024-06-05 23:43 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Wed, Jun 05, 2024 at 06:12:45PM -0500, Danny Kaehn wrote:
> USB HID core now shares its fwnode 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. This allows usb HID device
> drivers to be described in firmware and make use of device properties.

Can this patch be applied already, so we don't drag it again and again?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
@ 2024-06-06  0:18   ` Rob Herring (Arm)
  2024-06-06  6:28   ` Krzysztof Kozlowski
  2024-06-06 15:18   ` Rob Herring
  2 siblings, 0 replies; 24+ messages in thread
From: Rob Herring (Arm) @ 2024-06-06  0:18 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: devicetree, Bartosz Golaszewski, Ethan Twardy, Jiri Kosina,
	Krzysztof Kozlowski, Andy Shevchenko, Dmitry Torokhov,
	linux-input, Benjamin Tissoires


On Wed, 05 Jun 2024 18:12:44 -0500, 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 <danny.kaehn@plexus.com>
> ---
>  .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/i2c/silabs,cp2112.example.dts:41.13-29 Properties must precede subnodes
FATAL ERROR: Unable to parse input tree
make[2]: *** [scripts/Makefile.lib:427: Documentation/devicetree/bindings/i2c/silabs,cp2112.example.dtb] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240605-cp2112-dt-v11-1-d55f0f945a62@plexus.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
  2024-06-06  0:18   ` Rob Herring (Arm)
@ 2024-06-06  6:28   ` Krzysztof Kozlowski
  2024-06-06 15:12     ` Danny Kaehn
  2024-06-06 15:18   ` Rob Herring
  2 siblings, 1 reply; 24+ messages in thread
From: Krzysztof Kozlowski @ 2024-06-06  6:28 UTC (permalink / raw)
  To: Danny Kaehn, Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires,
	Andy Shevchenko
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

On 06/06/2024 01:12, 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 <danny.kaehn@plexus.com>
> ---
>  .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
>  1 file changed, 105 insertions(+)

So this is v11 but was never tested?

Changelog does not help me understanding what happened with this binding...

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: (subset) [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
                   ` (4 preceding siblings ...)
  2024-06-05 23:42 ` [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Andy Shevchenko
@ 2024-06-06  7:30 ` Benjamin Tissoires
  2025-07-29 14:53 ` Re " Willie Thai
  6 siblings, 0 replies; 24+ messages in thread
From: Benjamin Tissoires @ 2024-06-06  7:30 UTC (permalink / raw)
  To: Rob Herring, Krzysztof Kozlowski, Andy Shevchenko, Danny Kaehn
  Cc: Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

On Wed, 05 Jun 2024 18:12:43 -0500, Danny Kaehn wrote:
> This patchset allows USB-HID devices to have Firmware bindings through sharing
> the USB fwnode 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 ACPI and interoperate with other drivers.
> 
> Changes in v11:
> - Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
>     from Rob H.
> - Edit hid-cp2112.c to match for ACPI index and fall back to matching by
>     name (instead of the other way around)
> - Separate CP2112 I2C bus speed configuration into a separate patch
> 
> [...]

Applied to hid/hid.git (for-6.11/core), thanks!

[2/4] HID: usbhid: Share USB device firmware node with child HID device
      https://git.kernel.org/hid/hid/c/b81881b9c10e

Cheers,
-- 
Benjamin Tissoires <bentiss@kernel.org>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device
  2024-06-05 23:43   ` Andy Shevchenko
@ 2024-06-06  7:31     ` Benjamin Tissoires
  0 siblings, 0 replies; 24+ messages in thread
From: Benjamin Tissoires @ 2024-06-06  7:31 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Danny Kaehn, Rob Herring, Krzysztof Kozlowski, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Jun 06 2024, Andy Shevchenko wrote:
> On Wed, Jun 05, 2024 at 06:12:45PM -0500, Danny Kaehn wrote:
> > USB HID core now shares its fwnode 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. This allows usb HID device
> > drivers to be described in firmware and make use of device properties.
> 
> Can this patch be applied already, so we don't drag it again and again?

done :)

Cheers,
Benjamin

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-06  6:28   ` Krzysztof Kozlowski
@ 2024-06-06 15:12     ` Danny Kaehn
  0 siblings, 0 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-06 15:12 UTC (permalink / raw)
  To: robh@kernel.org, krzk+dt@kernel.org,
	andriy.shevchenko@linux.intel.com, krzk@kernel.org,
	bentiss@kernel.org
  Cc: jikos@kernel.org, linux-input@vger.kernel.org,
	bartosz.golaszewski@linaro.org, devicetree@vger.kernel.org,
	dmitry.torokhov@gmail.com, Ethan Twardy

On Thu, 2024-06-06 at 08:28 +0200, Krzysztof Kozlowski wrote:
> PLEXUS SECURITY WARNING
> You have not previously corresponded with this sender.       
> On 06/06/2024 01:12, 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 <danny.kaehn@plexus.com>
> > ---
> >  .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105
> +++++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> 
> So this is v11 but was never tested?

My apologies -- initially `DT_SCHEMA_FILES=silabs,cp2112.yaml make
dt_binding_check` was completing without any output (and I assumed
success), but after a clean, I get make errors, either relating to "no
rule to make ... *.example.dtb", or yamllint usage errors. Have tried
updating dtschema and/or reinstalling, and also started from scratch on
a different system with the same issues. I will get this working and
run this before submitting additional revisions, apologies again!
(and I will of course fix the issue reported by the bot)
> 
> Changelog does not help me understanding what happened with this
> binding...

I'll keep in mind better specifying changes to the binding in the
changelog!

Since v6, where Rob added his review tag [1], the only changes were
eliminating the gpio subnode and combining it with the parent, and
updating my email address.

Since v4, where you had added your review tag [2], I addressed Rob's
comments in [3], including:
- Removing the ngpios property
- Constraining the hog pattern more to a single naming scheme
- Removing unneeded properties from the hog which are provided by the
parent schema
- Adding an example of the hog to test the schema

Additionally, I added sda-gpios and scl-gpios to the i2c node as well
as usage of it in the example dts. This is intended to be used for bus
recovery GPIOs (not yet included in the kernel drivers)

[1]: 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230217184904.1290-2-kaehndan@gmail.com/

[2]: 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230206135016.6737-2-kaehndan@gmail.com/

[3]: 
https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230205145450.3396-2-kaehndan@gmail.com/#3051932

Thanks,

Danny Kaehn

> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
  2024-06-06  0:18   ` Rob Herring (Arm)
  2024-06-06  6:28   ` Krzysztof Kozlowski
@ 2024-06-06 15:18   ` Rob Herring
  2024-06-06 16:24     ` Danny Kaehn
  2 siblings, 1 reply; 24+ messages in thread
From: Rob Herring @ 2024-06-06 15:18 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Krzysztof Kozlowski, Benjamin Tissoires, Andy Shevchenko,
	Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

On Wed, Jun 05, 2024 at 06:12:44PM -0500, 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

There's no more gpio subnode.

> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.
> 
> Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
> ---
>  .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
>  1 file changed, 105 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
> new file mode 100644
> index 000000000000..0108f2e43c8c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
> @@ -0,0 +1,105 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i2c/silabs,cp2112.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CP2112 HID USB to SMBus/I2C Bridge
> +
> +maintainers:
> +  - Danny Kaehn <danny.kaehn@plexus.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

Or hub ports. Just drop 'on the host controller'.

> +
> +  i2c:
> +    description: The SMBus/I2C controller node for the CP2112
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +
> +    properties:
> +      sda-gpios:
> +        maxItems: 1
> +
> +      scl-gpios:
> +        maxItems: 1

These are because I2C can be on any of the pins? It's a bit odd if they 
aren't used as gpios. Probably should be pinmux, but that's overkill for 
2 pins.

> +
> +      clock-frequency:
> +        minimum: 10000
> +        default: 100000
> +        maximum: 400000
> +
> +  interrupt-controller: true
> +  "#interrupt-cells":
> +    const: 2

Where does the 
> +
> +  gpio-controller: true
> +  "#gpio-cells":
> +    const: 2
> +
> +  gpio-line-names:
> +    minItems: 1
> +    maxItems: 8
> +
> +patternProperties:
> +  "-hog(-[0-9]+)?$":
> +    type: object
> +
> +    required:
> +      - gpio-hog
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    usb {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      cp2112: device@1 {
> +        compatible = "usb10c4,ea90";
> +        reg = <1>;
> +
> +        i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +          sda-gpios = <&cp2112 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +          scl-gpios = <&cp2112 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> +
> +          temp@48 {
> +            compatible = "national,lm75";
> +            reg = <0x48>;
> +          };
> +        };
> +
> +        gpio-controller;
> +        interrupt-controller;
> +        #gpio-cells = <2>;
> +        gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
> +          "TEST3","TEST4", "TEST5", "TEST6";
> +
> +        fan-rst-hog {
> +            gpio-hog;
> +            gpios = <7 GPIO_ACTIVE_HIGH>;
> +            output-high;
> +            line-name = "FAN_RST";
> +        };
> +      };
> +    };
> 
> -- 
> 2.25.1
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-05 23:42 ` [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Andy Shevchenko
@ 2024-06-06 15:54   ` Danny Kaehn
  2024-06-06 19:47     ` Andy Shevchenko
  0 siblings, 1 reply; 24+ messages in thread
From: Danny Kaehn @ 2024-06-06 15:54 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Thu, Jun 06, 2024 at 02:42:42AM +0300, Andy Shevchenko wrote:
> On Wed, Jun 05, 2024 at 06:12:43PM -0500, Danny Kaehn wrote:
> > This patchset allows USB-HID devices to have Firmware bindings through sharing
> > the USB fwnode 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 ACPI and interoperate with other drivers.
> 
> ...
> 
> > Changes in v11:
> > - Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
> >     from Rob H.
> 
> Hmm... I don't know much about DT, but how is this supposed to work in ACPI?
> I mean if we want to refer to the GPIO in GpioIo() or GpioInt() resources,
> what should we put there as ACPI path?

What I tested was essentially taking what Benjamin had done in [1], just
removing the "GPIO" device and combining it with the parent device (the
CP2112 itself). So for the example below, I believe the path would be
"\_SB_.PCI0.SE9_.RHUB.CP2_". If I get the chance (and can figure out how
to do it using ACPI) I'll try to add a "gpio-keys" or something to the
system using this path and make sure that works.

[1]: https://patchwork.kernel.org/project/linux-input/patch/20230227140758.1575-4-kaehndan@gmail.com/#25242036

Thanks,

Danny Kaehn

---

Full example within context:

Device (SE9)
{
    Name (_ADR, 0x001D0001)  // _ADR: Address
    Device (RHUB)
    {
        Name (_ADR, Zero)
        Device (CP2) // the USB-hid & CP2112 shared node
        {
            Name (_ADR, One)
            Name (_STA, 0x0F)
            
            Name (_DSD, Package () {
                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                Package () {
                    Package () { "gpio-hog", 1 },
                    Package () { "gpios", Package () { 4, 0 } },
                    Package () { "output-high", 1 },
                    Package () { "line-name", "gpio4-pullup" },
                },
                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                Package () {
                    Package () { "gpio-line-names", Package () {
                                "",
                                "",
                                "irq-rmi4",
                                "",
                                "power", // set to 1 with gpio-hog above
                                "",
                                "",
                                "",
                                ""}},
                }
            })
            Device (I2C0)
            {
                Name (_ADR, Zero)
                Name (_STA, 0x0F)
            }
        }
    }
}

> 
> > - Edit hid-cp2112.c to match for ACPI index and fall back to matching by
> >     name (instead of the other way around)
> > - Separate CP2112 I2C bus speed configuration into a separate patch
> 
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-06 15:18   ` Rob Herring
@ 2024-06-06 16:24     ` Danny Kaehn
  2024-06-06 19:49       ` Andy Shevchenko
  0 siblings, 1 reply; 24+ messages in thread
From: Danny Kaehn @ 2024-06-06 16:24 UTC (permalink / raw)
  To: Rob Herring
  Cc: Krzysztof Kozlowski, Benjamin Tissoires, Andy Shevchenko,
	Jiri Kosina, devicetree, linux-input, Dmitry Torokhov,
	Bartosz Golaszewski, Ethan Twardy

I appreciate you taking a look (even in spite of the dt_binding_check
failure).

On Thu, Jun 06, 2024 at 09:18:59AM -0600, Rob Herring wrote:
> On Wed, Jun 05, 2024 at 06:12:44PM -0500, 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
> 
> There's no more gpio subnode.
>
Will fix.

> > intended to be used in configurations where the CP2112 is permanently
> > connected in hardware.
> > 
> > Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
> > ---
> >  .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 105 +++++++++++++++++++++
> >  1 file changed, 105 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
> > new file mode 100644
> > index 000000000000..0108f2e43c8c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
> > @@ -0,0 +1,105 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/i2c/silabs,cp2112.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: CP2112 HID USB to SMBus/I2C Bridge
> > +
> > +maintainers:
> > +  - Danny Kaehn <danny.kaehn@plexus.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
> 
> Or hub ports. Just drop 'on the host controller'.
>

Will fix.

> > +
> > +  i2c:
> > +    description: The SMBus/I2C controller node for the CP2112
> > +    $ref: /schemas/i2c/i2c-controller.yaml#
> > +    unevaluatedProperties: false
> > +
> > +    properties:
> > +      sda-gpios:
> > +        maxItems: 1
> > +
> > +      scl-gpios:
> > +        maxItems: 1
> 
> These are because I2C can be on any of the pins? It's a bit odd if they 
> aren't used as gpios. Probably should be pinmux, but that's overkill for 
> 2 pins.
>

I'm beginning to realize now that this may be a bit non-standard, but it
did solve a stuck bus issue under some conditions.

The CP2112's I2C controller is self-contained and can only be on the
specific pins it is attached to (no pinmux, etc..).

In this case, these properties are ment to specify additional gpio pins
which are connected to the SCL and SDA lines (this then also assumes those
are configured to be open drain / inputs to not interfere with the bus
during normal operation). This was inspired by what is done ini2c-imx.yaml,
but I realize this is a bit different due to using external pins rather
than pinmuxing to the GPIOs.

How I used this was to actually connect some of the CP2112's own GPIO pins
to the SDA and SCL lines to be able to use those pins to recover the
bus. This was able to solve a stuck bus under some real-world cases with
the v2 of the CP2112 containing an errata which caused this
semi-frequently.

> > +
> > +      clock-frequency:
> > +        minimum: 10000
> > +        default: 100000
> > +        maximum: 400000
> > +
> > +  interrupt-controller: true
> > +  "#interrupt-cells":
> > +    const: 2
> 
> Where does the 

Unsure what you intended to ask here -- but the interrupt comes from the
GPIO chip. It is unfortunately not a hardware interrupt, but is
accomplished through polling.

> > +
> > +  gpio-controller: true
> > +  "#gpio-cells":
> > +    const: 2
> > +
> > +  gpio-line-names:
> > +    minItems: 1
> > +    maxItems: 8
> > +
> > +patternProperties:
> > +  "-hog(-[0-9]+)?$":
> > +    type: object
> > +
> > +    required:
> > +      - gpio-hog
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    #include <dt-bindings/gpio/gpio.h>
> > +
> > +    usb {
> > +      #address-cells = <1>;
> > +      #size-cells = <0>;
> > +
> > +      cp2112: device@1 {
> > +        compatible = "usb10c4,ea90";
> > +        reg = <1>;
> > +
> > +        i2c {
> > +          #address-cells = <1>;
> > +          #size-cells = <0>;
> > +          sda-gpios = <&cp2112 0 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +          scl-gpios = <&cp2112 1 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
> > +
> > +          temp@48 {
> > +            compatible = "national,lm75";
> > +            reg = <0x48>;
> > +          };
> > +        };
> > +
> > +        gpio-controller;
> > +        interrupt-controller;
> > +        #gpio-cells = <2>;
> > +        gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
> > +          "TEST3","TEST4", "TEST5", "TEST6";
> > +
> > +        fan-rst-hog {
> > +            gpio-hog;
> > +            gpios = <7 GPIO_ACTIVE_HIGH>;
> > +            output-high;
> > +            line-name = "FAN_RST";
> > +        };
> > +      };
> > +    };
> > 
> > -- 
> > 2.25.1
> > 

Thanks,

Danny Kaehn


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-06 15:54   ` Danny Kaehn
@ 2024-06-06 19:47     ` Andy Shevchenko
  2024-06-07 17:20       ` Danny Kaehn
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2024-06-06 19:47 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Thu, Jun 06, 2024 at 10:54:53AM -0500, Danny Kaehn wrote:
> On Thu, Jun 06, 2024 at 02:42:42AM +0300, Andy Shevchenko wrote:
> > On Wed, Jun 05, 2024 at 06:12:43PM -0500, Danny Kaehn wrote:

...

> > > Changes in v11:
> > > - Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
> > >     from Rob H.
> > 
> > Hmm... I don't know much about DT, but how is this supposed to work in ACPI?
> > I mean if we want to refer to the GPIO in GpioIo() or GpioInt() resources,
> > what should we put there as ACPI path?
> 
> What I tested was essentially taking what Benjamin had done in [1], just
> removing the "GPIO" device and combining it with the parent device (the
> CP2112 itself). So for the example below, I believe the path would be
> "\_SB_.PCI0.SE9_.RHUB.CP2_". If I get the chance (and can figure out how
> to do it using ACPI) I'll try to add a "gpio-keys" or something to the
> system using this path and make sure that works.

This is counter intuitive and doesn't follow other (ACPI) devices with GPIO.
So whatever you do for DT I don't care much, but let's not remove subnode
for ACPI case.

...

> Full example within context:

Thank you!

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-06 16:24     ` Danny Kaehn
@ 2024-06-06 19:49       ` Andy Shevchenko
  2024-06-07 18:38         ` Danny Kaehn
  0 siblings, 1 reply; 24+ messages in thread
From: Andy Shevchenko @ 2024-06-06 19:49 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Thu, Jun 06, 2024 at 11:24:38AM -0500, Danny Kaehn wrote:
> On Thu, Jun 06, 2024 at 09:18:59AM -0600, Rob Herring wrote:
> > On Wed, Jun 05, 2024 at 06:12:44PM -0500, Danny Kaehn wrote:

...

> > > +  i2c:
> > > +    description: The SMBus/I2C controller node for the CP2112
> > > +    $ref: /schemas/i2c/i2c-controller.yaml#
> > > +    unevaluatedProperties: false
> > > +
> > > +    properties:
> > > +      sda-gpios:
> > > +        maxItems: 1
> > > +
> > > +      scl-gpios:
> > > +        maxItems: 1
> > 
> > These are because I2C can be on any of the pins? It's a bit odd if they 
> > aren't used as gpios. Probably should be pinmux, but that's overkill for 
> > 2 pins.
> >
> 
> I'm beginning to realize now that this may be a bit non-standard, but it
> did solve a stuck bus issue under some conditions.
> 
> The CP2112's I2C controller is self-contained and can only be on the
> specific pins it is attached to (no pinmux, etc..).
> 
> In this case, these properties are ment to specify additional gpio pins
> which are connected to the SCL and SDA lines (this then also assumes those
> are configured to be open drain / inputs to not interfere with the bus
> during normal operation). This was inspired by what is done ini2c-imx.yaml,
> but I realize this is a bit different due to using external pins rather
> than pinmuxing to the GPIOs.
> 
> How I used this was to actually connect some of the CP2112's own GPIO pins
> to the SDA and SCL lines to be able to use those pins to recover the
> bus. This was able to solve a stuck bus under some real-world cases with
> the v2 of the CP2112 containing an errata which caused this
> semi-frequently.

Aren't they just for I²C recovery?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-06 19:47     ` Andy Shevchenko
@ 2024-06-07 17:20       ` Danny Kaehn
  0 siblings, 0 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-07 17:20 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Thu, Jun 06, 2024 at 10:47:52PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 06, 2024 at 10:54:53AM -0500, Danny Kaehn wrote:
> > On Thu, Jun 06, 2024 at 02:42:42AM +0300, Andy Shevchenko wrote:
> > > On Wed, Jun 05, 2024 at 06:12:43PM -0500, Danny Kaehn wrote:
> 
> ...
> 
> > > > Changes in v11:
> > > > - Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
> > > >     from Rob H.
> > > 
> > > Hmm... I don't know much about DT, but how is this supposed to work in ACPI?
> > > I mean if we want to refer to the GPIO in GpioIo() or GpioInt() resources,
> > > what should we put there as ACPI path?
> > 
> > What I tested was essentially taking what Benjamin had done in [1], just
> > removing the "GPIO" device and combining it with the parent device (the
> > CP2112 itself). So for the example below, I believe the path would be
> > "\_SB_.PCI0.SE9_.RHUB.CP2_". If I get the chance (and can figure out how
> > to do it using ACPI) I'll try to add a "gpio-keys" or something to the
> > system using this path and make sure that works.
> 
> This is counter intuitive and doesn't follow other (ACPI) devices with GPIO.
> So whatever you do for DT I don't care much, but let's not remove subnode
> for ACPI case.
>

Fair enough, will let this sit for a moment to see if there are comments
from Rob/Krzysztof, and otherwise will rework the driver to support the
different models for DT and ACPI. For what it's worth, combining the
GPIO chip and CP2112 nodes in DT also seems unintuitive to me, and it
seems there's other bindings for multi-function devices which define a
separate child "gpio" node, so I'm not sure why it's not desired here.

Thanks,

Danny Kaehn


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
  2024-06-06 19:49       ` Andy Shevchenko
@ 2024-06-07 18:38         ` Danny Kaehn
  0 siblings, 0 replies; 24+ messages in thread
From: Danny Kaehn @ 2024-06-07 18:38 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rob Herring, Krzysztof Kozlowski, Benjamin Tissoires, Jiri Kosina,
	devicetree, linux-input, Dmitry Torokhov, Bartosz Golaszewski,
	Ethan Twardy

On Thu, Jun 06, 2024 at 10:49:00PM +0300, Andy Shevchenko wrote:
> On Thu, Jun 06, 2024 at 11:24:38AM -0500, Danny Kaehn wrote:
> > On Thu, Jun 06, 2024 at 09:18:59AM -0600, Rob Herring wrote:
> > > On Wed, Jun 05, 2024 at 06:12:44PM -0500, Danny Kaehn wrote:
> 
> ...
> 
> > > > +  i2c:
> > > > +    description: The SMBus/I2C controller node for the CP2112
> > > > +    $ref: /schemas/i2c/i2c-controller.yaml#
> > > > +    unevaluatedProperties: false
> > > > +
> > > > +    properties:
> > > > +      sda-gpios:
> > > > +        maxItems: 1
> > > > +
> > > > +      scl-gpios:
> > > > +        maxItems: 1
> > > 
> > > These are because I2C can be on any of the pins? It's a bit odd if they 
> > > aren't used as gpios. Probably should be pinmux, but that's overkill for 
> > > 2 pins.
> > >
> > 
> > I'm beginning to realize now that this may be a bit non-standard, but it
> > did solve a stuck bus issue under some conditions.
> > 
> > The CP2112's I2C controller is self-contained and can only be on the
> > specific pins it is attached to (no pinmux, etc..).
> > 
> > In this case, these properties are ment to specify additional gpio pins
> > which are connected to the SCL and SDA lines (this then also assumes those
> > are configured to be open drain / inputs to not interfere with the bus
> > during normal operation). This was inspired by what is done ini2c-imx.yaml,
> > but I realize this is a bit different due to using external pins rather
> > than pinmuxing to the GPIOs.
> > 
> > How I used this was to actually connect some of the CP2112's own GPIO pins
> > to the SDA and SCL lines to be able to use those pins to recover the
> > bus. This was able to solve a stuck bus under some real-world cases with
> > the v2 of the CP2112 containing an errata which caused this
> > semi-frequently.
> 
> Aren't they just for I²C recovery?

Not sure if you were looking for my reply here, but yes. I guess the
only "non-standard" thing really here is the idea of using
external/separate GPIO pins to do the recovery, rather than pinmuxing the
I2C pins to GPIO which most current implementations seem to do.

Thanks,

Danny Kaehn

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
                   ` (5 preceding siblings ...)
  2024-06-06  7:30 ` (subset) " Benjamin Tissoires
@ 2025-07-29 14:53 ` Willie Thai
  2025-07-29 17:49   ` Danny Kaehn
  6 siblings, 1 reply; 24+ messages in thread
From: Willie Thai @ 2025-07-29 14:53 UTC (permalink / raw)
  To: danny.kaehn
  Cc: andriy.shevchenko, bartosz.golaszewski, bentiss, devicetree,
	dmitry.torokhov, ethan.twardy, jikos, krzk+dt, linux-input, robh,
	tingkaic, rastekar, dkodihalli, mhn, arundp

Hi Danny,

I hope this message finds you well.
Thank you for the patch set — it’s exactly what we need for the I2C-over-USB feature in our new products.
Could you please let us know when we can expect the next version of the patch set?
If you've paused work on it, we're happy to take over and continue from where you left off.

Thanks!

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Re [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2025-07-29 14:53 ` Re " Willie Thai
@ 2025-07-29 17:49   ` Danny Kaehn
  2025-08-05 21:32     ` Andy Shevchenko
  2025-08-06 11:07     ` Willie Thai
  0 siblings, 2 replies; 24+ messages in thread
From: Danny Kaehn @ 2025-07-29 17:49 UTC (permalink / raw)
  To: Willie Thai
  Cc: andriy.shevchenko, bartosz.golaszewski, bentiss, devicetree,
	dmitry.torokhov, ethan.twardy, jikos, krzk+dt, linux-input, robh,
	tingkaic, rastekar, dkodihalli, mhn, arundp

On Tue, Jul 29, 2025 at 02:53:50PM +0000, Willie Thai wrote:
> Hi Danny,
> 
> I hope this message finds you well.
> Thank you for the patch set — it’s exactly what we need for the I2C-over-USB feature in our new products.
> Could you please let us know when we can expect the next version of the patch set?
> If you've paused work on it, we're happy to take over and continue from where you left off.
> 
> Thanks!

Thanks for reaching out!

Apologies, I haven't been working on this in a while, and have only been able
to intermittently return to attempt to bring it forward.

Feel free to take over and move this forward! I'm not sure what the protocol
is for that, as far as changelogs and versions and whatnot. If your product's
timeline for needing this mainlined is not urgent; however, I can prioritize
coming back to this and having a v12 submitted, likely by the end of next
week, to remove the overhead needed for you to assume ownership of the
patchset.

The last several versions of this patchset have all revolved around trying
to get this change working for ACPI as well as DeviceTree in such a way which
make the ACPI and DeviceTree interface/binding acceptable to their respective
maintainers. With this latest version, it seemed that there was not going to
be any consensus between the two firmware languages, so it seemed an entirely
different binding/interface and corresponding logic in the device driver
would be needed. This seems unfortunate, as it seemed the whole purpose of
the fwnode / device_*() functions was to unify the driver interface to the
firmware language used... but this is presumably a special case, being almost
exclusively a device composed of different generic device functions...

Let me know if you plan to take this over and if there's any
documentation/context/test procedures you would need from me; else I would be
happy to start moving this forward again now that there is someone waiting
on it.

Thanks

Danny Kaehn



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Re [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2025-07-29 17:49   ` Danny Kaehn
@ 2025-08-05 21:32     ` Andy Shevchenko
  2025-08-06 11:07     ` Willie Thai
  1 sibling, 0 replies; 24+ messages in thread
From: Andy Shevchenko @ 2025-08-05 21:32 UTC (permalink / raw)
  To: Danny Kaehn
  Cc: Willie Thai, bartosz.golaszewski, bentiss, devicetree,
	dmitry.torokhov, ethan.twardy, jikos, krzk+dt, linux-input, robh,
	tingkaic, rastekar, dkodihalli, mhn, arundp

On Tue, Jul 29, 2025 at 12:49:51PM -0500, Danny Kaehn wrote:
> On Tue, Jul 29, 2025 at 02:53:50PM +0000, Willie Thai wrote:
> > Hi Danny,
> > 
> > I hope this message finds you well.
> > Thank you for the patch set — it’s exactly what we need for the I2C-over-USB feature in our new products.
> > Could you please let us know when we can expect the next version of the patch set?
> > If you've paused work on it, we're happy to take over and continue from where you left off.
> > 
> > Thanks!
> 
> Thanks for reaching out!
> 
> Apologies, I haven't been working on this in a while, and have only been able
> to intermittently return to attempt to bring it forward.
> 
> Feel free to take over and move this forward! I'm not sure what the protocol
> is for that, as far as changelogs and versions and whatnot. If your product's
> timeline for needing this mainlined is not urgent; however, I can prioritize
> coming back to this and having a v12 submitted, likely by the end of next
> week, to remove the overhead needed for you to assume ownership of the
> patchset.
> 
> The last several versions of this patchset have all revolved around trying
> to get this change working for ACPI as well as DeviceTree in such a way which
> make the ACPI and DeviceTree interface/binding acceptable to their respective
> maintainers. With this latest version, it seemed that there was not going to
> be any consensus between the two firmware languages, so it seemed an entirely
> different binding/interface and corresponding logic in the device driver
> would be needed. This seems unfortunate, as it seemed the whole purpose of
> the fwnode / device_*() functions was to unify the driver interface to the
> firmware language used... but this is presumably a special case, being almost
> exclusively a device composed of different generic device functions...
> 
> Let me know if you plan to take this over and if there's any
> documentation/context/test procedures you would need from me; else I would be
> happy to start moving this forward again now that there is someone waiting
> on it.

Right and I'm, for instance, lost the context long time ago, so please Cc me on
a new version to have a fresh look at it.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: Re [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112
  2025-07-29 17:49   ` Danny Kaehn
  2025-08-05 21:32     ` Andy Shevchenko
@ 2025-08-06 11:07     ` Willie Thai
  1 sibling, 0 replies; 24+ messages in thread
From: Willie Thai @ 2025-08-06 11:07 UTC (permalink / raw)
  To: danny.kaehn
  Cc: andriy.shevchenko, arundp, bartosz.golaszewski, bentiss,
	devicetree, dkodihalli, dmitry.torokhov, ethan.twardy, jikos,
	krzk+dt, linux-input, mhn, rastekar, robh, tingkaic, wthai

>> Hi Danny,
>> 
>> I hope this message finds you well.
>> Thank you for the patch set — it’s exactly what we need for the I2C-over-USB feature in our new products.
>> Could you please let us know when we can expect the next version of the patch set?
>> If you've paused work on it, we're happy to take over and continue from where you left off.
>> 
>> Thanks!
>
> Thanks for reaching out!
>
> Apologies, I haven't been working on this in a while, and have only been able
> to intermittently return to attempt to bring it forward.
>
> Feel free to take over and move this forward! I'm not sure what the protocol
> is for that, as far as changelogs and versions and whatnot. If your product's
> timeline for needing this mainlined is not urgent; however, I can prioritize
> coming back to this and having a v12 submitted, likely by the end of next
> week, to remove the overhead needed for you to assume ownership of the
> patchset.
>
> The last several versions of this patchset have all revolved around trying
> to get this change working for ACPI as well as DeviceTree in such a way which
> make the ACPI and DeviceTree interface/binding acceptable to their respective
> maintainers. With this latest version, it seemed that there was not going to
> be any consensus between the two firmware languages, so it seemed an entirely
> different binding/interface and corresponding logic in the device driver
> would be needed. This seems unfortunate, as it seemed the whole purpose of
> the fwnode / device_*() functions was to unify the driver interface to the
> firmware language used... but this is presumably a special case, being almost
> exclusively a device composed of different generic device functions...
>
> Let me know if you plan to take this over and if there's any
> documentation/context/test procedures you would need from me; else I would be
> happy to start moving this forward again now that there is someone waiting
> on it.
>
> Thanks
> 
> Danny Kaehn

Hi Danny,

Thanks for your response !
Currently, your patch is working fine with our feature without ACPI.
We can use it downstream first.
Please let me know if you need any support !

Thanks !

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2025-08-06 11:07 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-05 23:12 [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Danny Kaehn
2024-06-05 23:12 ` [PATCH v11 1/4] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge Danny Kaehn
2024-06-06  0:18   ` Rob Herring (Arm)
2024-06-06  6:28   ` Krzysztof Kozlowski
2024-06-06 15:12     ` Danny Kaehn
2024-06-06 15:18   ` Rob Herring
2024-06-06 16:24     ` Danny Kaehn
2024-06-06 19:49       ` Andy Shevchenko
2024-06-07 18:38         ` Danny Kaehn
2024-06-05 23:12 ` [PATCH v11 2/4] HID: usbhid: Share USB device firmware node with child HID device Danny Kaehn
2024-06-05 23:43   ` Andy Shevchenko
2024-06-06  7:31     ` Benjamin Tissoires
2024-06-05 23:12 ` [PATCH v11 3/4] HID: cp2112: Fwnode Support Danny Kaehn
2024-06-05 23:39   ` Andy Shevchenko
2024-06-05 23:12 ` [PATCH v11 4/4] HID: cp2112: Configure I2C Bus Speed from Firmware Danny Kaehn
2024-06-05 23:42 ` [PATCH v11 0/4] Firmware Support for USB-HID Devices and CP2112 Andy Shevchenko
2024-06-06 15:54   ` Danny Kaehn
2024-06-06 19:47     ` Andy Shevchenko
2024-06-07 17:20       ` Danny Kaehn
2024-06-06  7:30 ` (subset) " Benjamin Tissoires
2025-07-29 14:53 ` Re " Willie Thai
2025-07-29 17:49   ` Danny Kaehn
2025-08-05 21:32     ` Andy Shevchenko
2025-08-06 11:07     ` Willie Thai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).