mbox series

[v10,0/3] Firmware Support for USB-HID Devices and CP2112

Message ID 20240205170920.93499-1-danny.kaehn@plexus.com
Headers show
Series Firmware Support for USB-HID Devices and CP2112 | expand

Message

Danny Kaehn Feb. 5, 2024, 5:09 p.m. UTC
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 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

Comments

Andy Shevchenko Feb. 6, 2024, 2:05 p.m. UTC | #1
On Mon, Feb 05, 2024 at 11:09:22AM -0600, Danny Kaehn wrote:
> Support describing the CP2112's I2C and GPIO interfaces in firmware.
> 
> I2C and GPIO child nodes can either be children with names "i2c" and
> "gpio", or, for ACPI, device nodes with _ADR Zero and One,
> respectively.

> Additionally, support configuring the I2C bus speed from the
> clock-frequency device property.

This has to be in a separate patch, which may predecess this one.

...

> +		name = fwnode_get_name(child);
> +		if (name) {
> +			ret = match_string(cp2112_cell_names,
> +							ARRAY_SIZE(cp2112_cell_names), name);
> +			if (ret >= 0)
> +				addr = ret;
> +		}
> +		if (!name || ret < 0)
> +			ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> +
> +		if (ret < 0)
> +			continue;

I don't like this piece (esp. due to possible matching with node name which
may not be so reliable), but I have no better solution right now.

Maybe this way (this doesn't particularly solve the issue but seems better
to me)?

		ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
		if (ret) {
			/* If no ACPI given or compiled, fallback to matching names */
			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;
		}