diff mbox series

[v2,1/4] dt-bindings: usb: typec: add bindings for stm32g0 controller

Message ID 20220711120122.25804-2-fabrice.gasnier@foss.st.com (mailing list archive)
State Superseded
Headers show
Series usb: typec: ucsi: add support for stm32g0 | expand

Commit Message

Fabrice Gasnier July 11, 2022, 12:01 p.m. UTC 
Add DT schema documentation for the STM32G0 Type-C PD (Power Delivery)
controller.
STM32G0 provides an integrated USB Type-C and power delivery interface.
It can be programmed with a firmware to handle UCSI protocol over I2C
interface. A GPIO is used as an interrupt line.
It may be used as a wakeup source, so use optional "wakeup-source" and
"power-domains" properties to support wakeup.

Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
---
Changes in v2:
- Krzysztof's review comments: update commit message, use ports, use
  unevaluatedProperties: false for usb-connector schema, define maxItems
  for power-domains, adopt generic node names, remove quotes
---
 .../bindings/usb/st,typec-stm32g0.yaml        | 90 +++++++++++++++++++
 1 file changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml

Comments

Krzysztof Kozlowski July 12, 2022, 8:42 a.m. UTC | #1 
On 11/07/2022 14:01, Fabrice Gasnier wrote:
> Add DT schema documentation for the STM32G0 Type-C PD (Power Delivery)
> controller.
> STM32G0 provides an integrated USB Type-C and power delivery interface.
> It can be programmed with a firmware to handle UCSI protocol over I2C
> interface. A GPIO is used as an interrupt line.
> It may be used as a wakeup source, so use optional "wakeup-source" and
> "power-domains" properties to support wakeup.
> 
> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>

Thank you for your patch. There is something to discuss/improve.

> ---
> Changes in v2:
> - Krzysztof's review comments: update commit message, use ports, use
>   unevaluatedProperties: false for usb-connector schema, define maxItems
>   for power-domains, adopt generic node names, remove quotes
> ---
>  .../bindings/usb/st,typec-stm32g0.yaml        | 90 +++++++++++++++++++
>  1 file changed, 90 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
> 
> diff --git a/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
> new file mode 100644
> index 0000000000000..7b3a2c2124e38
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
> @@ -0,0 +1,90 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/st,typec-stm32g0.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics STM32G0 USB Type-C PD controller
> +
> +description: |
> +  The STM32G0 MCU can be programmed to control Type-C connector(s) through I2C
> +  typically using the UCSI protocol over I2C, with a dedicated alert
> +  (interrupt) pin.
> +
> +maintainers:
> +  - Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> +
> +properties:
> +  compatible:
> +    const: st,stm32g0-typec
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  connector:
> +    type: object
> +    $ref: /schemas/connector/usb-connector.yaml#
> +    unevaluatedProperties: false
> +
> +  firmware-name:
> +    description: |
> +      Should contain the name of the default firmware image
> +      file located on the firmware search path
> +
> +  wakeup-source: true
> +
> +  power-domains:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

Isn't connector a required property? I would assume the device does not
make much sense to operate without it.

What about firmware-name? Do you expect hardware to work fine without it
(default firmware?)?


Best regards,
Krzysztof
Fabrice Gasnier July 12, 2022, 10:18 a.m. UTC | #2 
On 7/12/22 10:42, Krzysztof Kozlowski wrote:
> On 11/07/2022 14:01, Fabrice Gasnier wrote:
>> Add DT schema documentation for the STM32G0 Type-C PD (Power Delivery)
>> controller.
>> STM32G0 provides an integrated USB Type-C and power delivery interface.
>> It can be programmed with a firmware to handle UCSI protocol over I2C
>> interface. A GPIO is used as an interrupt line.
>> It may be used as a wakeup source, so use optional "wakeup-source" and
>> "power-domains" properties to support wakeup.
>>
>> Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
> 
> Thank you for your patch. There is something to discuss/improve.
> 
>> ---
>> Changes in v2:
>> - Krzysztof's review comments: update commit message, use ports, use
>>   unevaluatedProperties: false for usb-connector schema, define maxItems
>>   for power-domains, adopt generic node names, remove quotes
>> ---
>>  .../bindings/usb/st,typec-stm32g0.yaml        | 90 +++++++++++++++++++
>>  1 file changed, 90 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
>> new file mode 100644
>> index 0000000000000..7b3a2c2124e38
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
>> @@ -0,0 +1,90 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/usb/st,typec-stm32g0.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: STMicroelectronics STM32G0 USB Type-C PD controller
>> +
>> +description: |
>> +  The STM32G0 MCU can be programmed to control Type-C connector(s) through I2C
>> +  typically using the UCSI protocol over I2C, with a dedicated alert
>> +  (interrupt) pin.
>> +
>> +maintainers:
>> +  - Fabrice Gasnier <fabrice.gasnier@foss.st.com>
>> +
>> +properties:
>> +  compatible:
>> +    const: st,stm32g0-typec
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  connector:
>> +    type: object
>> +    $ref: /schemas/connector/usb-connector.yaml#
>> +    unevaluatedProperties: false
>> +
>> +  firmware-name:
>> +    description: |
>> +      Should contain the name of the default firmware image
>> +      file located on the firmware search path
>> +
>> +  wakeup-source: true
>> +
>> +  power-domains:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
> 
> Isn't connector a required property? I would assume the device does not
> make much sense to operate without it.

Hi Krzysztof,

Indeed, that's sensible. I'll add connector to the required properties.

> 
> What about firmware-name? Do you expect hardware to work fine without it
> (default firmware?)?


Basically, the default firmware could be loaded in production. The
firmware content itself may be customized to restrict the firmware
update feature. That's a pure application decision of the firmware.
There could be other means to update it too (like via CC lines, with
external tools), instead of I2C lines.

Hence, the firmware-name is made optional here.

I can update the commit message with this explanation if this clarifies it.

Thanks for reviewing,
Best Regards,
Fabrice

> 
> 
> Best regards,
> Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
new file mode 100644
index 0000000000000..7b3a2c2124e38
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml
@@ -0,0 +1,90 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/st,typec-stm32g0.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32G0 USB Type-C PD controller
+
+description: |
+  The STM32G0 MCU can be programmed to control Type-C connector(s) through I2C
+  typically using the UCSI protocol over I2C, with a dedicated alert
+  (interrupt) pin.
+
+maintainers:
+  - Fabrice Gasnier <fabrice.gasnier@foss.st.com>
+
+properties:
+  compatible:
+    const: st,stm32g0-typec
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  connector:
+    type: object
+    $ref: /schemas/connector/usb-connector.yaml#
+    unevaluatedProperties: false
+
+  firmware-name:
+    description: |
+      Should contain the name of the default firmware image
+      file located on the firmware search path
+
+  wakeup-source: true
+
+  power-domains:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+
+      typec@53 {
+        compatible = "st,stm32g0-typec";
+        reg = <0x53>;
+        /* Alert pin on GPIO PE12 */
+        interrupts = <12 IRQ_TYPE_EDGE_FALLING>;
+        interrupt-parent = <&gpioe>;
+
+        /* Example with one type-C connector */
+        connector {
+          compatible = "usb-c-connector";
+          label = "USB-C";
+
+          ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+            port@0 {
+              reg = <0>;
+              con_usb_c_ep: endpoint {
+                remote-endpoint = <&usb_ep>;
+              };
+            };
+          };
+        };
+      };
+    };
+
+    usb {
+      usb-role-switch;
+      port {
+        usb_ep: endpoint {
+          remote-endpoint = <&con_usb_c_ep>;
+        };
+      };
+    };
+...