[PATCH v13 0/3] Firmware Support for USB-HID Devices and CP2112

[PATCH v13 0/3] Firmware Support for USB-HID Devices and CP2112

[Danny Kaehn]

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 v13:
- dt-binding changes:
  - drop scl-gpios and sda-gpios from binding, since they are included
      from the i2c-controller schena.
  - Set indentation to 4 spaces consistently for the DTS example
  - Fix alignment for gpio-line-names in the example
- Use dev_fwnode in hid-cp2112.c instead of directly accessing fwnode
- Immediately release the fwnode_handle from
    device_get_named_child_node() in hid-cp2112.c
- Link to v12: https://lore.kernel.org/r/20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com

Changes in v12:
- dt-binding changes:
  - Drop "on the host controller" from top-level description based on
      comment from Rob H.
  - Correct "Properties must precede subnodes" dt_binding_check error by
      moving gpio_chip-related properties above the i2c subnode in the
      binding and in the example.
  - Include `interrupt-controller` property in the example
- Modify hid-cp2112.c to support separate schemas for DT vs. ACPI - DT
  combines gpio subnode with the CP2112's node, but will have an I2C
  subnode; while ACPI will maintain separate child nodes for the GPIO
  I2C devices

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
   - Remove ngpios property
   - Constrain the hog pattern to a single naming scheme
   - Remove unneeded properties from the gpio hog which are provided by
       the parent schema
 - 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 (3):
      dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
      HID: cp2112: Fwnode Support
      HID: cp2112: Configure I2C Bus Speed from Firmware

 .../devicetree/bindings/i2c/silabs,cp2112.yaml     | 100 +++++++++++++++++++++
 drivers/hid/hid-cp2112.c                           |  37 ++++++++
 2 files changed, 137 insertions(+)
---
base-commit: 1c772200c9dcb23a304f84a9334fe2e0d9529ab0
change-id: 20240605-cp2112-dt-7cdc95448e8a

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

[PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Danny Kaehn]

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,
with the i2c controller being bound to a subnode named "i2c". 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     | 100 +++++++++++++++++++++
 1 file changed, 100 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..a204adfe57b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
@@ -0,0 +1,100 @@
+# 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
+
+  interrupt-controller: true
+  "#interrupt-cells":
+    const: 2
+
+  gpio-controller: true
+  "#gpio-cells":
+    const: 2
+
+  gpio-line-names:
+    minItems: 1
+    maxItems: 8
+
+  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
+
+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>;
+
+            gpio-controller;
+            interrupt-controller;
+            #interrupt-cells = <2>;
+            #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";
+            };
+
+            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>;
+                };
+            };
+        };
+    };

-- 
2.25.1

[PATCH v13 2/3] HID: cp2112: Fwnode Support

[Danny Kaehn]

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

Bindings between the firmware nodes and the functions of the device
are distinct between ACPI and DeviceTree.

For ACPI, the i2c_adapter will use the child with _ADR Zero and the
gpio_chip will use the child with _ADR One. For DeviceTree, the
i2c_adapter will use the child with name "i2c", but the gpio_chip
will share a firmware node with the CP2112.

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

diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 803b883ae875..ea19b5cb58f9 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -29,6 +29,16 @@
 #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
+ * @CP2112_GPIO_ADR: Address for GPIO node
+ */
+enum cp2112_child_acpi_cell_addrs {
+	CP2112_I2C_ADR = 0,
+	CP2112_GPIO_ADR = 1,
+};
+
 #define CP2112_REPORT_MAX_LENGTH		64
 #define CP2112_GPIO_CONFIG_LENGTH		5
 #define CP2112_GPIO_GET_LENGTH			2
@@ -1208,7 +1218,9 @@ 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;
+	u32 addr;
 	int ret;
 
 	dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1226,6 +1238,27 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
 		return ret;
 	}
 
+	if (is_acpi_device_node(dev_fwnode(&hdev->dev))) {
+		device_for_each_child_node(&hdev->dev, child) {
+			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
+			if (ret)
+				continue;
+
+			switch (addr) {
+			case CP2112_I2C_ADR:
+				device_set_node(&dev->adap.dev, child);
+				break;
+			case CP2112_GPIO_ADR:
+				dev->gc.fwnode = child;
+				break;
+			}
+		}
+	} else {
+		child = device_get_named_child_node(&hdev->dev, "i2c");
+		device_set_node(&dev->adap.dev, child);
+		fwnode_handle_put(child);
+	}
+
 	ret = hid_parse(hdev);
 	if (ret) {
 		hid_err(hdev, "parse failed\n");

-- 
2.25.1

[PATCH v13 3/3] HID: cp2112: Configure I2C Bus Speed from Firmware

[Danny Kaehn]

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 ea19b5cb58f9..4c5957e0a5cc 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1220,6 +1220,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;
 	u32 addr;
 	int ret;
 
@@ -1304,6 +1305,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

Re: [PATCH v13 3/3] HID: cp2112: Configure I2C Bus Speed from Firmware

[Danny Kaehn]

On Tue, Jan 27, 2026 at 08:47:50AM -0600, Danny Kaehn wrote:
> Now that the I2C adapter on the CP2112 can have an associated firmware
> node, set the bus speed based on firmware configuration
>

Apologies, realized post-send that I never addressed Andy's comment
asking for a period at the end here and expressing his sign-off. Will
address with the next send if there is one; else I'll re-send this one
after other approvals.

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Danny Kaehn]

On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> with the i2c controller being bound to a subnode named "i2c". 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>
> ---

Hi Folks (Intended for Rob or Krzysztof),

Wasn't sure the best way to go about this, but trying to see the best
way to get a message in front of you regarding an ask from Andy S.

In [1], Rob H initially directed that the gpio chip share a node with
the CP2112 itself, rather than having a subnode named 'gpio'.

Initially, I did the same thing for both DT and ACPI, but Andy S.
directed that ACPI should not have the node be shared in that way.

With the last revision of this patch, Andy S. asked that I try to get a
rationalle from Rob (or other DT expert presumably) on why the gpio node
should be combined with the parent, rather than being a named subnode
[2].

Any context you can provide would be extremely helpful. Apologies about
the age of this patch series and the amount of historical context; some
is due to my long delays between revisions, but other of it is due to
attempting to get the ACPI and DT folks to talk / agree.

[1]: https://lore.kernel.org/all/20240213152825.GA1223720-robh@kernel.org/
[2]: https://lore.kernel.org/all/aSdvv3Qss5oz_o6P@smile.fi.intel.com/


Thanks,

Danny Kaehn

Re: [PATCH v13 2/3] HID: cp2112: Fwnode Support

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 08:47:49AM -0600, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
> 
> Bindings between the firmware nodes and the functions of the device
> are distinct between ACPI and DeviceTree.
> 
> For ACPI, the i2c_adapter will use the child with _ADR Zero and the

_ADR equals to Zero

> gpio_chip will use the child with _ADR One. For DeviceTree, the

_ADR equals to One

> i2c_adapter will use the child with name "i2c", but the gpio_chip
> will share a firmware node with the CP2112.

...

Also it's interesting choice of capital letters in the Subject.

I would expect "...: Add fwnode support"

...

> +/**
> + * enum cp2112_child_acpi_cell_addrs - Child ACPI addresses for CP2112 sub-functions
> + * @CP2112_I2C_ADR: Address for I2C node
> + * @CP2112_GPIO_ADR: Address for GPIO node

Probably you want to mention in the description of the enum (here) that
the assigned values are explicit since this is basically a protocol between
FW and OS. That's why we may not change this values without breaking
older firmware descriptions.

> + */
> +enum cp2112_child_acpi_cell_addrs {
> +	CP2112_I2C_ADR = 0,
> +	CP2112_GPIO_ADR = 1,
> +};

...

> +	if (is_acpi_device_node(dev_fwnode(&hdev->dev))) {

I'm wondering if we can avoid this (additional) check and use the result of one
of the branches.

> +		device_for_each_child_node(&hdev->dev, child) {

If we are still use the above check it will be dev_fwnode() duplication call,
so perhaps a temporary variable to collect the device's fwnode and use it
there, below (see below), and here as for

		fwnode_for_each_child_node()

> +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +			if (ret)
> +				continue;
> +
> +			switch (addr) {
> +			case CP2112_I2C_ADR:
> +				device_set_node(&dev->adap.dev, child);
> +				break;
> +			case CP2112_GPIO_ADR:
> +				dev->gc.fwnode = child;
> +				break;
> +			}
> +		}
> +	} else {

I would still check if this is a proper (OF) node, in case we stick with the
ACPI check above. Because we might have swnode and if it triggers, it will be
really something unexpected.

	} else if (is_of_node(fwnode)) {


> +		child = device_get_named_child_node(&hdev->dev, "i2c");
> +		device_set_node(&dev->adap.dev, child);
> +		fwnode_handle_put(child);
> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > with the i2c controller being bound to a subnode named "i2c". This is
> > intended to be used in configurations where the CP2112 is permanently
> > connected in hardware.

> Hi Folks (Intended for Rob or Krzysztof),
> 
> Wasn't sure the best way to go about this, but trying to see the best
> way to get a message in front of you regarding an ask from Andy S.
> 
> In [1], Rob H initially directed that the gpio chip share a node with
> the CP2112 itself, rather than having a subnode named 'gpio'.
> 
> Initially, I did the same thing for both DT and ACPI, but Andy S.
> directed that ACPI should not have the node be shared in that way.
> 
> With the last revision of this patch, Andy S. asked that I try to get a
> rationalle from Rob (or other DT expert presumably) on why the gpio node
> should be combined with the parent, rather than being a named subnode
> [2].
> 
> Any context you can provide would be extremely helpful. Apologies about
> the age of this patch series and the amount of historical context; some
> is due to my long delays between revisions, but other of it is due to
> attempting to get the ACPI and DT folks to talk / agree.

I think this is about markers such as "gpio-controller" or
"interrupt-controller" in DT for the device in question.
With that it might not be required to have a separate child
node for the GPIO function.

> [1]: https://lore.kernel.org/all/20240213152825.GA1223720-robh@kernel.org/
> [2]: https://lore.kernel.org/all/aSdvv3Qss5oz_o6P@smile.fi.intel.com/

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Krzysztof Kozlowski]

On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > with the i2c controller being bound to a subnode named "i2c". 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>
> > ---
> 
> Hi Folks (Intended for Rob or Krzysztof),
> 
> Wasn't sure the best way to go about this, but trying to see the best
> way to get a message in front of you regarding an ask from Andy S.
> 
> In [1], Rob H initially directed that the gpio chip share a node with
> the CP2112 itself, rather than having a subnode named 'gpio'.
> 
> Initially, I did the same thing for both DT and ACPI, but Andy S.
> directed that ACPI should not have the node be shared in that way.
> 
> With the last revision of this patch, Andy S. asked that I try to get a
> rationalle from Rob (or other DT expert presumably) on why the gpio node
> should be combined with the parent, rather than being a named subnode
> [2].

Because it is explicitly asked in writing bindings. Please read it.

Because we do not want Linux driver model affecting design of bindings
and DTS, by subnodes present only to instantiate Linux drivers. I do not
care about driver model in this review and I do not see any reason it
should make DTS less obvious or readable.

That's actually rule communicated many times, also documented in writing
bindings and in recent talks.

Best regards,
Krzysztof

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> > On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > > with the i2c controller being bound to a subnode named "i2c". 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>
> > > ---
> > 
> > Hi Folks (Intended for Rob or Krzysztof),
> > 
> > Wasn't sure the best way to go about this, but trying to see the best
> > way to get a message in front of you regarding an ask from Andy S.
> > 
> > In [1], Rob H initially directed that the gpio chip share a node with
> > the CP2112 itself, rather than having a subnode named 'gpio'.
> > 
> > Initially, I did the same thing for both DT and ACPI, but Andy S.
> > directed that ACPI should not have the node be shared in that way.
> > 
> > With the last revision of this patch, Andy S. asked that I try to get a
> > rationalle from Rob (or other DT expert presumably) on why the gpio node
> > should be combined with the parent, rather than being a named subnode
> > [2].
> 
> Because it is explicitly asked in writing bindings. Please read it.
> 
> Because we do not want Linux driver model affecting design of bindings
> and DTS, by subnodes present only to instantiate Linux drivers. I do not
> care about driver model in this review and I do not see any reason it
> should make DTS less obvious or readable.
> 
> That's actually rule communicated many times, also documented in writing
> bindings and in recent talks.

Does DT represents HW in this case? Shouldn't I²C controller be the same node?
Why not? This is inconsistent for the device that is multi-functional. And from
my understanding the firmware description (DT, ACPI, you-name-it) must follow
the HW. I don't see how it's done in this case.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> > On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> > > On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > > > with the i2c controller being bound to a subnode named "i2c". 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>
> > > > ---
> > > 
> > > Hi Folks (Intended for Rob or Krzysztof),
> > > 
> > > Wasn't sure the best way to go about this, but trying to see the best
> > > way to get a message in front of you regarding an ask from Andy S.
> > > 
> > > In [1], Rob H initially directed that the gpio chip share a node with
> > > the CP2112 itself, rather than having a subnode named 'gpio'.
> > > 
> > > Initially, I did the same thing for both DT and ACPI, but Andy S.
> > > directed that ACPI should not have the node be shared in that way.
> > > 
> > > With the last revision of this patch, Andy S. asked that I try to get a
> > > rationalle from Rob (or other DT expert presumably) on why the gpio node
> > > should be combined with the parent, rather than being a named subnode
> > > [2].
> > 
> > Because it is explicitly asked in writing bindings. Please read it.
> > 
> > Because we do not want Linux driver model affecting design of bindings
> > and DTS, by subnodes present only to instantiate Linux drivers. I do not
> > care about driver model in this review and I do not see any reason it
> > should make DTS less obvious or readable.
> > 
> > That's actually rule communicated many times, also documented in writing
> > bindings and in recent talks.
> 
> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> Why not? This is inconsistent for the device that is multi-functional. And from
> my understanding the firmware description (DT, ACPI, you-name-it) must follow
> the HW. I don't see how it's done in this case.

The i2c controller should probably be in the same node too, unless it
would cause conflicts between function (e.g. inability to figure out if
a child is a hog or a i2c device). I would like a rationale provided for
why the i2c controller is in a subnode.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Krzysztof Kozlowski]

On 28/01/2026 13:49, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
>>> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
>>>> with the i2c controller being bound to a subnode named "i2c". 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>
>>>> ---
>>>
>>> Hi Folks (Intended for Rob or Krzysztof),
>>>
>>> Wasn't sure the best way to go about this, but trying to see the best
>>> way to get a message in front of you regarding an ask from Andy S.
>>>
>>> In [1], Rob H initially directed that the gpio chip share a node with
>>> the CP2112 itself, rather than having a subnode named 'gpio'.
>>>
>>> Initially, I did the same thing for both DT and ACPI, but Andy S.
>>> directed that ACPI should not have the node be shared in that way.
>>>
>>> With the last revision of this patch, Andy S. asked that I try to get a
>>> rationalle from Rob (or other DT expert presumably) on why the gpio node
>>> should be combined with the parent, rather than being a named subnode
>>> [2].
>>
>> Because it is explicitly asked in writing bindings. Please read it.
>>
>> Because we do not want Linux driver model affecting design of bindings
>> and DTS, by subnodes present only to instantiate Linux drivers. I do not
>> care about driver model in this review and I do not see any reason it
>> should make DTS less obvious or readable.
>>
>> That's actually rule communicated many times, also documented in writing
>> bindings and in recent talks.
> 
> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> Why not? This is inconsistent for the device that is multi-functional. And from
> my understanding the firmware description (DT, ACPI, you-name-it) must follow
> the HW. I don't see how it's done in this case.

What is inconsistent exactly? What sort of rule tells that every little
function needs a device node? It's first time I hear about any of such
rule and for all this time we already NAKed it so many times (node per
GPIO, node per clock, node per every little pin).

> 


Best regards,
Krzysztof

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 03:06:58PM +0000, Conor Dooley wrote:
> On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> > > On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:

...

> > > That's actually rule communicated many times, also documented in writing
> > > bindings and in recent talks.
> > 
> > Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> > Why not? This is inconsistent for the device that is multi-functional. And from
> > my understanding the firmware description (DT, ACPI, you-name-it) must follow
> > the HW. I don't see how it's done in this case.
> 
> The i2c controller should probably be in the same node too, unless it
> would cause conflicts between function (e.g. inability to figure out if
> a child is a hog or a i2c device). I would like a rationale provided for
> why the i2c controller is in a subnode.

I can expect a disaster with such a scheme, splitting multi-functional device
to the subdevices (children) sounds to me like the best approach. With this,
one may have the same (globally named) property to be different on subdevices.

But I will hold my breath to see the outcome of this discussion.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Krzysztof Kozlowski]

On 28/01/2026 16:06, Conor Dooley wrote:
> On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
>> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
>>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
>>>> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
>>>>> with the i2c controller being bound to a subnode named "i2c". 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>
>>>>> ---
>>>>
>>>> Hi Folks (Intended for Rob or Krzysztof),
>>>>
>>>> Wasn't sure the best way to go about this, but trying to see the best
>>>> way to get a message in front of you regarding an ask from Andy S.
>>>>
>>>> In [1], Rob H initially directed that the gpio chip share a node with
>>>> the CP2112 itself, rather than having a subnode named 'gpio'.
>>>>
>>>> Initially, I did the same thing for both DT and ACPI, but Andy S.
>>>> directed that ACPI should not have the node be shared in that way.
>>>>
>>>> With the last revision of this patch, Andy S. asked that I try to get a
>>>> rationalle from Rob (or other DT expert presumably) on why the gpio node
>>>> should be combined with the parent, rather than being a named subnode
>>>> [2].
>>>
>>> Because it is explicitly asked in writing bindings. Please read it.
>>>
>>> Because we do not want Linux driver model affecting design of bindings
>>> and DTS, by subnodes present only to instantiate Linux drivers. I do not
>>> care about driver model in this review and I do not see any reason it
>>> should make DTS less obvious or readable.
>>>
>>> That's actually rule communicated many times, also documented in writing
>>> bindings and in recent talks.
>>
>> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
>> Why not? This is inconsistent for the device that is multi-functional. And from
>> my understanding the firmware description (DT, ACPI, you-name-it) must follow
>> the HW. I don't see how it's done in this case.
> 
> The i2c controller should probably be in the same node too, unless it
> would cause conflicts between function (e.g. inability to figure out if

This one is the rationale.

> a child is a hog or a i2c device). I would like a rationale provided for
> why the i2c controller is in a subnode.

I2C controller will have children, because it is a bus, so moving it up
one level would make the entire node I2C bus and that's not only problem
for the kernel but actually for reading DT - we expect consistent choice
for children, instead of mixing nodes with and without bus-addressing.
What's more, if you have two buses you also need separate nodes to group
them (obviously).

Best regards,
Krzysztof

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Krzysztof Kozlowski]

On 28/01/2026 16:51, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 03:06:58PM +0000, Conor Dooley wrote:
>> On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
>>> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
>>>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> 
> ...
> 
>>>> That's actually rule communicated many times, also documented in writing
>>>> bindings and in recent talks.
>>>
>>> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
>>> Why not? This is inconsistent for the device that is multi-functional. And from
>>> my understanding the firmware description (DT, ACPI, you-name-it) must follow
>>> the HW. I don't see how it's done in this case.
>>
>> The i2c controller should probably be in the same node too, unless it
>> would cause conflicts between function (e.g. inability to figure out if
>> a child is a hog or a i2c device). I would like a rationale provided for
>> why the i2c controller is in a subnode.
> 
> I can expect a disaster with such a scheme, splitting multi-functional device

So for some years no disaster happened, at least nothing was reported to us.

Best regards,
Krzysztof

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 04:48:18PM +0100, Krzysztof Kozlowski wrote:
> On 28/01/2026 13:49, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> >> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:

...

> >> That's actually rule communicated many times, also documented in writing
> >> bindings and in recent talks.
> > 
> > Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> > Why not? This is inconsistent for the device that is multi-functional. And from
> > my understanding the firmware description (DT, ACPI, you-name-it) must follow
> > the HW. I don't see how it's done in this case.
> 
> What is inconsistent exactly? What sort of rule tells that every little
> function needs a device node? It's first time I hear about any of such
> rule and for all this time we already NAKed it so many times (node per
> GPIO, node per clock, node per every little pin).

That we should represent the HW as is. There is no "rule", there is a common
sense. Of course, it's possible to have all-in-one node, but this may lead
to a disaster when there are tons of devices in the Multi Functional HW
and some of them use the same properties. How would you distinguish HW
with two GPIO banks, two I²C controllers, et cetera? That's what my common
sense tells to me, putting all eggs into one bucket is just a mine field
for the future.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Conor Dooley]

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

On Wed, Jan 28, 2026 at 04:52:01PM +0100, Krzysztof Kozlowski wrote:
> On 28/01/2026 16:06, Conor Dooley wrote:
> > On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
> >> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> >>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> >>>> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> >>>>> with the i2c controller being bound to a subnode named "i2c". 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>
> >>>>> ---
> >>>>
> >>>> Hi Folks (Intended for Rob or Krzysztof),
> >>>>
> >>>> Wasn't sure the best way to go about this, but trying to see the best
> >>>> way to get a message in front of you regarding an ask from Andy S.
> >>>>
> >>>> In [1], Rob H initially directed that the gpio chip share a node with
> >>>> the CP2112 itself, rather than having a subnode named 'gpio'.
> >>>>
> >>>> Initially, I did the same thing for both DT and ACPI, but Andy S.
> >>>> directed that ACPI should not have the node be shared in that way.
> >>>>
> >>>> With the last revision of this patch, Andy S. asked that I try to get a
> >>>> rationalle from Rob (or other DT expert presumably) on why the gpio node
> >>>> should be combined with the parent, rather than being a named subnode
> >>>> [2].
> >>>
> >>> Because it is explicitly asked in writing bindings. Please read it.
> >>>
> >>> Because we do not want Linux driver model affecting design of bindings
> >>> and DTS, by subnodes present only to instantiate Linux drivers. I do not
> >>> care about driver model in this review and I do not see any reason it
> >>> should make DTS less obvious or readable.
> >>>
> >>> That's actually rule communicated many times, also documented in writing
> >>> bindings and in recent talks.
> >>
> >> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> >> Why not? This is inconsistent for the device that is multi-functional. And from
> >> my understanding the firmware description (DT, ACPI, you-name-it) must follow
> >> the HW. I don't see how it's done in this case.
> > 
> > The i2c controller should probably be in the same node too, unless it
> > would cause conflicts between function (e.g. inability to figure out if
> 
> This one is the rationale.
> 
> > a child is a hog or a i2c device). I would like a rationale provided for
> > why the i2c controller is in a subnode.

I guess it wasn't clear that I was trying to say that the rationale
should be provided by the submitter in their patch, and the first
portion of my comment was trying to mention what has to be considered.

> I2C controller will have children, because it is a bus, so moving it up
> one level would make the entire node I2C bus and that's not only problem
> for the kernel but actually for reading DT - we expect consistent choice
> for children, instead of mixing nodes with and without bus-addressing.
> What's more, if you have two buses you also need separate nodes to group
> them (obviously).



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Krzysztof Kozlowski]

On 28/01/2026 17:05, Andy Shevchenko wrote:
> On Wed, Jan 28, 2026 at 04:48:18PM +0100, Krzysztof Kozlowski wrote:
>> On 28/01/2026 13:49, Andy Shevchenko wrote:
>>> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
>>>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> 
> ...
> 
>>>> That's actually rule communicated many times, also documented in writing
>>>> bindings and in recent talks.
>>>
>>> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
>>> Why not? This is inconsistent for the device that is multi-functional. And from
>>> my understanding the firmware description (DT, ACPI, you-name-it) must follow
>>> the HW. I don't see how it's done in this case.
>>
>> What is inconsistent exactly? What sort of rule tells that every little
>> function needs a device node? It's first time I hear about any of such
>> rule and for all this time we already NAKed it so many times (node per
>> GPIO, node per clock, node per every little pin).
> 
> That we should represent the HW as is. There is no "rule", there is a common
> sense. Of course, it's possible to have all-in-one node, but this may lead
> to a disaster when there are tons of devices in the Multi Functional HW
> and some of them use the same properties. How would you distinguish HW
> with two GPIO banks, two I²C controllers, et cetera? That's what my common

I do not see problems in these examples. GPIO banks have gpio-cells for
that. i2c controllers are busses, so as I explained in other email, must
have their own node whenever any other node is expected.

And for everything which is more complex, e.g. regulators, we do expect
child nodes.

Still the "MFD" is not a reason itself, we consistently give such review
and we also documented it.


> sense tells to me, putting all eggs into one bucket is just a mine field
> for the future.

Some years passed and I do not remember any mine happening here.
Actually mines appeared when people DID create fake nodes, because then
when the actual true bus node was needed it was violating the rule we
have - not mixing bus and non-bus nodes on the same level.


Best regards,
Krzysztof

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Danny Kaehn]

On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> > On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > > with the i2c controller being bound to a subnode named "i2c". 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>
> > > ---
> > 
> > Hi Folks (Intended for Rob or Krzysztof),
> > 
> > Wasn't sure the best way to go about this, but trying to see the best
> > way to get a message in front of you regarding an ask from Andy S.
> > 
> > In [1], Rob H initially directed that the gpio chip share a node with
> > the CP2112 itself, rather than having a subnode named 'gpio'.
> > 
> > Initially, I did the same thing for both DT and ACPI, but Andy S.
> > directed that ACPI should not have the node be shared in that way.
> > 
> > With the last revision of this patch, Andy S. asked that I try to get a
> > rationalle from Rob (or other DT expert presumably) on why the gpio node
> > should be combined with the parent, rather than being a named subnode
> > [2].
> 
> Because it is explicitly asked in writing bindings. Please read it.
> 
> Because we do not want Linux driver model affecting design of bindings
> and DTS, by subnodes present only to instantiate Linux drivers. I do not
> care about driver model in this review and I do not see any reason it
> should make DTS less obvious or readable.
> 
> That's actually rule communicated many times, also documented in writing
> bindings and in recent talks.
> 

Hi Krzysztof,

Thanks for all of the replies. It's never my intent to waste
maintainers' time, so apologies if due-diligence was missed here on my
part.

When initially writing this binding, I did search around for any kernel
doc or binding that might provide guidance on how the nodes could be    
split, but failed to find anything particularly relevent, aside from
general principles which can be applied to come to the same conclusion
you have about why the gpio and i2c nodes are necessarily different
because of i2c representing a true bus. This is likely a failing on my
part, but I'm not sure exactly where I'd go to find rules like this
which, as you say, have been communicated many times, aside from
querying the mailing lists.

I've started to go through some recent talks to see context there, and
came across "How to Get Your DT Schema Bindings Accepted in Less Than
10 Iterations"... clearly I've failed on that here :)

Thanks,

Danny Kaehn

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Danny Kaehn]

On Wed, Jan 28, 2026 at 05:24:24PM +0000, Conor Dooley wrote:
> On Wed, Jan 28, 2026 at 04:52:01PM +0100, Krzysztof Kozlowski wrote:
> > On 28/01/2026 16:06, Conor Dooley wrote:
> > > On Wed, Jan 28, 2026 at 02:49:39PM +0200, Andy Shevchenko wrote:
> > >> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> > >>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:
> > >>>> On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> > >>>>> with the i2c controller being bound to a subnode named "i2c". 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>
> > >>>>> ---
> > >>>>
> > >>>> Hi Folks (Intended for Rob or Krzysztof),
> > >>>>
> > >>>> Wasn't sure the best way to go about this, but trying to see the best
> > >>>> way to get a message in front of you regarding an ask from Andy S.
> > >>>>
> > >>>> In [1], Rob H initially directed that the gpio chip share a node with
> > >>>> the CP2112 itself, rather than having a subnode named 'gpio'.
> > >>>>
> > >>>> Initially, I did the same thing for both DT and ACPI, but Andy S.
> > >>>> directed that ACPI should not have the node be shared in that way.
> > >>>>
> > >>>> With the last revision of this patch, Andy S. asked that I try to get a
> > >>>> rationalle from Rob (or other DT expert presumably) on why the gpio node
> > >>>> should be combined with the parent, rather than being a named subnode
> > >>>> [2].
> > >>>
> > >>> Because it is explicitly asked in writing bindings. Please read it.
> > >>>
> > >>> Because we do not want Linux driver model affecting design of bindings
> > >>> and DTS, by subnodes present only to instantiate Linux drivers. I do not
> > >>> care about driver model in this review and I do not see any reason it
> > >>> should make DTS less obvious or readable.
> > >>>
> > >>> That's actually rule communicated many times, also documented in writing
> > >>> bindings and in recent talks.
> > >>
> > >> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> > >> Why not? This is inconsistent for the device that is multi-functional. And from
> > >> my understanding the firmware description (DT, ACPI, you-name-it) must follow
> > >> the HW. I don't see how it's done in this case.
> > > 
> > > The i2c controller should probably be in the same node too, unless it
> > > would cause conflicts between function (e.g. inability to figure out if
> > 
> > This one is the rationale.
> > 
> > > a child is a hog or a i2c device). I would like a rationale provided for
> > > why the i2c controller is in a subnode.
> 
> I guess it wasn't clear that I was trying to say that the rationale
> should be provided by the submitter in their patch, and the first
> portion of my comment was trying to mention what has to be considered.
>

Hello Conor,

Thanks for the comment -- as this binding was created, it was noted that 
it is somewhat of an oddball compared to existing bindings, so rationale
should have been provided somewhere, to justify whether those oddities
are needed because of this hardware being odd, or whether I was going
out of bounds. Would that have belonged in the initial patch series
message?

My rationale was this -- as has been mentioned by others, the i2c node
seems to be normalized as a separated submode because it does represent
a distinct bus, and it make sense to group the devices on that bus such
that the context of `reg` is aparrent (i.e. if there were multiple i2c
busses, they would necessarily need their own nodes). Initially when
creating this binding, I applied the same logic to the gpio chip,
observing the presence of the "hog" child nodes, and that their `gpios`
property also acts like `reg` on a bus, relating to the parent node.
But, now, seeing that that is already something that has been ruled not
to be the case, it makes sense to me why i2c busses might need their own
nodes while gpio chips might not.

Thanks,

Danny Kaehn

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Wed, Jan 28, 2026 at 08:52:50PM +0100, Krzysztof Kozlowski wrote:
> On 28/01/2026 17:05, Andy Shevchenko wrote:
> > On Wed, Jan 28, 2026 at 04:48:18PM +0100, Krzysztof Kozlowski wrote:
> >> On 28/01/2026 13:49, Andy Shevchenko wrote:
> >>> On Wed, Jan 28, 2026 at 11:35:25AM +0100, Krzysztof Kozlowski wrote:
> >>>> On Tue, Jan 27, 2026 at 10:02:17AM -0600, Danny Kaehn wrote:

...

> >>>> That's actually rule communicated many times, also documented in writing
> >>>> bindings and in recent talks.
> >>>
> >>> Does DT represents HW in this case? Shouldn't I²C controller be the same node?
> >>> Why not? This is inconsistent for the device that is multi-functional. And from
> >>> my understanding the firmware description (DT, ACPI, you-name-it) must follow
> >>> the HW. I don't see how it's done in this case.
> >>
> >> What is inconsistent exactly? What sort of rule tells that every little
> >> function needs a device node? It's first time I hear about any of such
> >> rule and for all this time we already NAKed it so many times (node per
> >> GPIO, node per clock, node per every little pin).
> > 
> > That we should represent the HW as is. There is no "rule", there is a common
> > sense. Of course, it's possible to have all-in-one node, but this may lead
> > to a disaster when there are tons of devices in the Multi Functional HW
> > and some of them use the same properties. How would you distinguish HW
> > with two GPIO banks, two I²C controllers, et cetera? That's what my common
> 
> I do not see problems in these examples. GPIO banks have gpio-cells for
> that. i2c controllers are busses, so as I explained in other email, must
> have their own node whenever any other node is expected.
> 
> And for everything which is more complex, e.g. regulators, we do expect
> child nodes.
> 
> Still the "MFD" is not a reason itself, we consistently give such review
> and we also documented it.
> 
> > sense tells to me, putting all eggs into one bucket is just a mine field
> > for the future.
> 
> Some years passed and I do not remember any mine happening here.
> Actually mines appeared when people DID create fake nodes, because then
> when the actual true bus node was needed it was violating the rule we
> have - not mixing bus and non-bus nodes on the same level.

Okay, thanks for elaboration. I definitely learnt something new about DT.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Rob Herring (Arm)]

On Tue, 27 Jan 2026 08:47:48 -0600, 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,
> with the i2c controller being bound to a subnode named "i2c". 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     | 100 +++++++++++++++++++++
>  1 file changed, 100 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v13 2/3] HID: cp2112: Fwnode Support

[Danny Kaehn]

On Tue, Jan 27, 2026 at 10:06:27PM +0200, Andy Shevchenko wrote:
> On Tue, Jan 27, 2026 at 08:47:49AM -0600, Danny Kaehn wrote:
> > Support describing the CP2112's I2C and GPIO interfaces in firmware.
> > 
> > Bindings between the firmware nodes and the functions of the device
> > are distinct between ACPI and DeviceTree.
> > 
> > For ACPI, the i2c_adapter will use the child with _ADR Zero and the
> 
> _ADR equals to Zero
>
Ack, will change.

> > gpio_chip will use the child with _ADR One. For DeviceTree, the
> 
> _ADR equals to One
> 

Ack, will change.

> > i2c_adapter will use the child with name "i2c", but the gpio_chip
> > will share a firmware node with the CP2112.
> 
> ...
> 
> Also it's interesting choice of capital letters in the Subject.
> 
> I would expect "...: Add fwnode support"
> 

Ack, will change.

> ...
> 
> > +/**
> > + * enum cp2112_child_acpi_cell_addrs - Child ACPI addresses for CP2112 sub-functions
> > + * @CP2112_I2C_ADR: Address for I2C node
> > + * @CP2112_GPIO_ADR: Address for GPIO node
> 
> Probably you want to mention in the description of the enum (here) that
> the assigned values are explicit since this is basically a protocol between
> FW and OS. That's why we may not change this values without breaking
> older firmware descriptions.
> 
> > + */
> > +enum cp2112_child_acpi_cell_addrs {
> > +	CP2112_I2C_ADR = 0,
> > +	CP2112_GPIO_ADR = 1,
> > +};
> 
> ...
> 
> > +	if (is_acpi_device_node(dev_fwnode(&hdev->dev))) {
> 
> I'm wondering if we can avoid this (additional) check and use the result of one
> of the branches.
>

Meaning something like using the result of acpi_get_local_address() to
determine whether the node is ACPI vs. not? That is what it used to do,
before I needed to switch to different schemas for DT vs. ACPI. Now, it
doesn't really make sense to use the child node types to determine
whether the GPIO node is shared, but still possible if we store a bool
result from the *_for_each_child_node() loop, but needs more complex
logic to store that based on each child's type (and the loop is fully
unnecessary for the non-ACPI case anyways).


Following the discussion on the DT binding thread, do you still want
ACPI to follow this different schema with the separate GPIO child node,
or would you prefer to unify them?

> > +		device_for_each_child_node(&hdev->dev, child) {
> 
> If we are still use the above check it will be dev_fwnode() duplication call,
> so perhaps a temporary variable to collect the device's fwnode and use it
> there, below (see below), and here as for
> 
> 		fwnode_for_each_child_node()
>

Makes sense, will update. I initially assumed we wanted to use the
"device_*" API wherever possible.

> > +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > +			if (ret)
> > +				continue;
> > +
> > +			switch (addr) {
> > +			case CP2112_I2C_ADR:
> > +				device_set_node(&dev->adap.dev, child);
> > +				break;
> > +			case CP2112_GPIO_ADR:
> > +				dev->gc.fwnode = child;
> > +				break;
> > +			}
> > +		}
> > +	} else {
> 
> I would still check if this is a proper (OF) node, in case we stick with the
> ACPI check above. Because we might have swnode and if it triggers, it will be
> really something unexpected.
>
> 	} else if (is_of_node(fwnode)) {
>

Wouldn't it be valid to use software nodes to describe the
CP2112's functions? Is there any reason to intentionally prevent that?

>
> > +		child = device_get_named_child_node(&hdev->dev, "i2c");
> > +		device_set_node(&dev->adap.dev, child);
> > +		fwnode_handle_put(child);
> > +	}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>

Thanks,

Danny Kaehn

Re: [PATCH v13 2/3] HID: cp2112: Fwnode Support

[Andy Shevchenko]

On Thu, Jan 29, 2026 at 12:36:50PM -0600, Danny Kaehn wrote:
> On Tue, Jan 27, 2026 at 10:06:27PM +0200, Andy Shevchenko wrote:
> > On Tue, Jan 27, 2026 at 08:47:49AM -0600, Danny Kaehn wrote:

...

> > I'm wondering if we can avoid this (additional) check and use the result of one
> > of the branches.
> 
> Meaning something like using the result of acpi_get_local_address() to
> determine whether the node is ACPI vs. not? That is what it used to do,
> before I needed to switch to different schemas for DT vs. ACPI. Now, it
> doesn't really make sense to use the child node types to determine
> whether the GPIO node is shared, but still possible if we store a bool
> result from the *_for_each_child_node() loop, but needs more complex
> logic to store that based on each child's type (and the loop is fully
> unnecessary for the non-ACPI case anyways).
> 
> Following the discussion on the DT binding thread, do you still want
> ACPI to follow this different schema with the separate GPIO child node,
> or would you prefer to unify them?

Wouldn't it be a bit messy if we combine main Device object with the GPIO
and leave I²C as a separate node? Besides that it seems already established
practice to have GPIO + I²C controllers separated based on _ADR (see Intel
Galileo case, drivers/mfd/intel_quark_i2c_gpio.c). Even if we can combine
I prefer to use the existing schema to have less animals in the zoo, for
the consistency's sake.

...

> > > +		device_for_each_child_node(&hdev->dev, child) {
> > 
> > If we are still use the above check it will be dev_fwnode() duplication call,
> > so perhaps a temporary variable to collect the device's fwnode and use it
> > there, below (see below), and here as for
> > 
> > 		fwnode_for_each_child_node()
> 
> 
> Makes sense, will update. I initially assumed we wanted to use the
> "device_*" API wherever possible.

Yes, but use a common sense. If we have fwnode already available, why should we
still use device_*()?

> > > +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> > > +			if (ret)
> > > +				continue;
> > > +
> > > +			switch (addr) {
> > > +			case CP2112_I2C_ADR:
> > > +				device_set_node(&dev->adap.dev, child);
> > > +				break;
> > > +			case CP2112_GPIO_ADR:
> > > +				dev->gc.fwnode = child;
> > > +				break;
> > > +			}
> > > +		}
> > > +	} else {
> > 
> > I would still check if this is a proper (OF) node, in case we stick with the
> > ACPI check above. Because we might have swnode and if it triggers, it will be
> > really something unexpected.
> >
> > 	} else if (is_of_node(fwnode)) {
> 
> Wouldn't it be valid to use software nodes to describe the
> CP2112's functions? Is there any reason to intentionally prevent that?

swnode:s are for quirks. I hope in this case we won't see them IRL.
In any case, let's enable them when we will have the case.

> > > +		child = device_get_named_child_node(&hdev->dev, "i2c");
> > > +		device_set_node(&dev->adap.dev, child);
> > > +		fwnode_handle_put(child);
> > > +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v13 1/3] dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge

[Andy Shevchenko]

On Tue, Jan 27, 2026 at 08:47:48AM -0600, 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,
> with the i2c controller being bound to a subnode named "i2c". This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.

...

> +            gpio-line-names = "CP2112_SDA", "CP2112_SCL", "TEST2",
> +                              "TEST3","TEST4", "TEST5", "TEST6";


Seems like missing space. Also I would resplit this logically, id est:

            gpio-line-names = "CP2112_SDA", "CP2112_SCL",
                              "TEST2", "TEST3", "TEST4", "TEST5", "TEST6";

-- 
With Best Regards,
Andy Shevchenko