On 27/06/2022 16:21, Fabrice Gasnier wrote: > On 6/24/22 18:16, Krzysztof Kozlowski wrote: >> On 24/06/2022 17:54, Fabrice Gasnier wrote: >>> This patch adds DT schema documentation for the STM32G0 Type-C controller. >> >> No "This patch" > > Hi Krzysztof, > > ack, > >> >> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95 >> >>> 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@xxxxxxxxxxx> >>> --- >>> .../bindings/usb/st,typec-stm32g0.yaml | 83 +++++++++++++++++++ >>> 1 file changed, 83 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..b2729bd015a1a >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/usb/st,typec-stm32g0.yaml >>> @@ -0,0 +1,83 @@ >>> +# 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#"; >> >> No quotes. > > ack, > >> >>> + >>> +title: STMicroelectronics STM32G0 Type-C controller bindings >> >> s/bindings// > > ack, > >> >>> + >>> +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@xxxxxxxxxxx> >>> + >>> +properties: >>> + compatible: >>> + const: st,stm32g0-typec >>> + >>> + reg: >>> + maxItems: 1 >>> + >>> + interrupts: >>> + maxItems: 1 >>> + >>> + connector: >>> + type: object> + allOf: >>> + - $ref: ../connector/usb-connector.yaml# >> >> Full path, so /schemas/connector/... >> >> unevaluatedProperties: false > > ack, > >> >>> + >>> + firmware-name: >>> + description: | >>> + Should contain the name of the default firmware image >>> + file located on the firmware search path >>> + >>> + wakeup-source: true >>> + power-domains: true >> >> maxItems > > Do you mean maxItems regarding the "power-domains" property ? Yes. > This will depend on the user platform, where it's used as an I2C device. > So I'm not sure this can / should be specified here. > Could please you clarify ? Then maybe this property is not valid here. Power domains usually are used for blocks of a SoC, having common power source and power gating. In your case it looks much more like a regulator supply. > >> >>> + >>> +required: >>> + - compatible >>> + - reg >>> + - interrupts >>> + >>> +additionalProperties: false >>> + >>> +examples: >>> + - | >>> + #include <dt-bindings/interrupt-controller/irq.h> >>> + i2c5 { >> >> Just "i2c" > > ack, > >> >>> + #address-cells = <1>; >>> + #size-cells = <0>; >>> + >>> + stm32g0@53 { >> >> Generic node name describing class of the device. > > > I wasn't aware of generic node name for an I2C device (not talking of > the controller). I may have missed it. > > Could you please clarify ? The class of a device is not a I2C device. I2C is just a bus. For example the generic name for Power Management IC connected over I2C (quite common case) is "pmic". For USB HCD controllers the generic name is "usb". For USB ports/connectors this is "connector". So what is your hardware? "interface" is a bit too unspecific to figure it out. > >> >>> + 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"; >>> + >>> + port { >> >> This does not look like proper schema of connector.yaml. > > This refers to graph.yaml [1], where similar example is seen [2]. > > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/graph.yaml#L79 > > https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/graph.yaml#L207 Just look at the usb-conector schema. It's different. You miss ports. Maybe other properties as well. > > device-1 { > port { > device_1_output: endpoint { > remote-endpoint = <&device_2_input>; > }; > }; > }; > device-2 { > port { > device_2_input: endpoint { > remote-endpoint = <&device_1_output>; > }; > }; > }; > > > Could you please clarify this point too ? > >> >>> + con_usb_c_ep: endpoint { >>> + remote-endpoint = <&usbotg_hs_ep>; >>> + }; >>> + }; >>> + }; >>> + }; >>> + }; >>> + >>> + usbotg_hs { >> >> Generic node names, no underscores in node names. > > ack, I guess you'd recommend "usb" here. I'll update it. Yes, looks like usb. Best regards, Krzysztof
