diff mbox series

[v3,1/4] dt-bindings: input: Add CP2112 HID USB to SMBus Bridge

Message ID 20230205145450.3396-2-kaehndan@gmail.com (mailing list archive)
State Superseded
Headers show
Series DeviceTree Support for USB-HID Devices and CP2112 | expand

Commit Message

Daniel Kaehn Feb. 5, 2023, 2:54 p.m. UTC
This is a USB HID device which includes an I2C controller and 8 GPIO pins.

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

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

Comments

Krzysztof Kozlowski Feb. 6, 2023, 7:59 a.m. UTC | #1
On 05/02/2023 15:54, Danny Kaehn wrote:
> This is a USB HID device which includes an I2C controller and 8 GPIO pins.

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

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


Best regards,
Krzysztof
Daniel Kaehn Feb. 6, 2023, 1:15 p.m. UTC | #2
On Mon, Feb 6, 2023 at 1:59 AM Krzysztof Kozlowski
<krzysztof.kozlowski@linaro.org> wrote:
>
> On 05/02/2023 15:54, Danny Kaehn wrote:
> > This is a USB HID device which includes an I2C controller and 8 GPIO pins.
>
> I actually wonder - which part of CP2112 is input or HID related? The
> manufacturer advertises it as USB to SMBus bridge, so it is an I2C
> controller, thus should be in i2c directory.
>

That's a great point - - the device is technically a USB HID device,
and since HID is usually used for input devices, it's lumped in with
hid and input devices on the driver tree. Though, since dt bindings
and Linux are separate, I see how it would make sense to classify it
differently on the bindings side. Though I wonder, since it has both
an i2c controller and gpio controller, should it go under mfd? Or,
since i2c is its "primary" use, going under i2c would be fine?

Thanks,

Danny Kaehn
Krzysztof Kozlowski Feb. 6, 2023, 1:37 p.m. UTC | #3
On 06/02/2023 14:15, Daniel Kaehn wrote:
> On Mon, Feb 6, 2023 at 1:59 AM Krzysztof Kozlowski
> <krzysztof.kozlowski@linaro.org> wrote:
>>
>> On 05/02/2023 15:54, Danny Kaehn wrote:
>>> This is a USB HID device which includes an I2C controller and 8 GPIO pins.
>>
>> I actually wonder - which part of CP2112 is input or HID related? The
>> manufacturer advertises it as USB to SMBus bridge, so it is an I2C
>> controller, thus should be in i2c directory.
>>
> 
> That's a great point - - the device is technically a USB HID device,
> and since HID is usually used for input devices, it's lumped in with
> hid and input devices on the driver tree. Though, since dt bindings
> and Linux are separate, I see how it would make sense to classify it
> differently on the bindings side. Though I wonder, since it has both
> an i2c controller and gpio controller, should it go under mfd? Or,
> since i2c is its "primary" use, going under i2c would be fine?

mfd directory is rather Linuxism and we use for strictly multi-function
(not dual-function) devices like PMICs. Manufacturer calls it I2C
bridge, so this looks like the main function of the device.

Best regards,
Krzysztof
Rob Herring (Arm) Feb. 6, 2023, 4:51 p.m. UTC | #4
On Sun, Feb 05, 2023 at 08:54:47AM -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
> using the subnodes named "gpio" and "i2c", respectively. This is
> intended to be used in configurations where the CP2112 is permanently
> connected in hardware.
> 
> Signed-off-by: Danny Kaehn <kaehndan@gmail.com>
> ---
>  .../bindings/input/silabs,cp2112.yaml         | 112 ++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/silabs,cp2112.yaml b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> new file mode 100644
> index 000000000000..eb2e89edb80a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/silabs,cp2112.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/silabs,cp2112.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: CP2112 HID USB to SMBus/I2C Bridge
> +
> +maintainers:
> +  - Danny Kaehn <kaehndan@gmail.com>
> +
> +description:
> +  The CP2112 is a USB HID device which includes an integrated I2C controller
> +  and 8 GPIO pins. Its GPIO pins can each be configured as inputs, open-drain
> +  outputs, or push-pull outputs.
> +
> +properties:
> +  compatible:
> +    const: usb10c4,ea90
> +
> +  reg:
> +    maxItems: 1
> +    description: The USB port number on the host controller
> +
> +  i2c:
> +    description: The SMBus/I2C controller node for the CP2112
> +    $ref: /schemas/i2c/i2c-controller.yaml#
> +    unevaluatedProperties: false
> +    properties:
> +      clock-frequency:
> +        minimum: 10000
> +        default: 100000
> +        maximum: 400000
> +
> +  gpio:
> +    description: The GPIO controller node for the CP2112
> +    type: object
> +    properties:
> +      interrupt-controller: true
> +      "#interrupt-cells":
> +        const: 2
> +
> +      gpio-controller: true
> +      "#gpio-cells":
> +        const: 2
> +
> +      ngpios:
> +        const: 8

If this can only be 1 value, then it doesn't need to be in DT.

> +
> +      gpio-line-names:
> +        minItems: 1
> +        maxItems: 8
> +
> +    patternProperties:
> +      "^(hog-[0-9]+|.+-hog(-[0-9]+)?)$":

Pick one naming scheme, not everything we allow.

> +        type: object
> +        properties:
> +          gpio-hog: true
> +          input: true
> +          output-high: true
> +          output-low: true
> +          line-name: true
> +          gpios:
> +            minItems: 1
> +            maxItems: 8
> +
> +        required:
> +          - gpio-hog
> +          - gpios
> +
> +        additionalProperties: false

You shouldn't need all this for the hog nodes, just need the following 
and the common schema will check the rest:

required:
  - gpio-hog 

> +
> +    unevaluatedProperties: false

Move this above 'properties'. Easier to read rather than after 
a long indented block.

> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +
> +    usb {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      device@1 {
> +        compatible = "usb10c4,ea90";
> +        reg = <1>;
> +
> +        i2c {
> +          #address-cells = <1>;
> +          #size-cells = <0>;
> +
> +          temp@48 {
> +            compatible = "national,lm75";
> +            reg = <0x48>;
> +          };
> +        };
> +
> +        gpio {
> +          gpio-controller;
> +          interrupt-controller;
> +          #gpio-cells = <2>;
> +          gpio-line-names = "TEST0", "TEST1", "TEST2",
> +            "TEST3", "TEST4", "TEST5", "TEST6", "TEST7";

Put a hog to test the schema.

> +        };
> +      };
> +    };
> -- 
> 2.25.1
>
diff mbox series

Patch

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