On 10/08/2022 06:04, Bjorn Andersson wrote: > Add binding for the Embedded Controller found in the Qualcomm > Snapdragon-based Lenovo Yoga C630. Thank you for your patch. There is something to discuss/improve. > + > +description: > + The Qualcomm Snapdragon-based Lenovo Yoga C630 has an Embedded Controller > + (EC) which handles things such as battery and USB Type-C. This binding > + describes the interface, on an I2C bus, to this EC. > + > +properties: > + compatible: > + const: lenovo,yoga-c630-ec > + > + reg: > + const: 0x70 > + > + '#address-cells': > + const: 1 Just to clarify: the EC have physically two USB connectors? > + > + '#size-cells': > + const: 0 > + > + interrupts: > + maxItems: 1 > + > +patternProperties: > + '^connector@\d$': > + $ref: /schemas/connector/usb-connector.yaml# unevaluatedProperties:false inside connector (on its level) > + > +required: > + - compatible > + - reg > + - interrupts > + > +additionalProperties: false > + > +examples: > + - |+ > + #include <dt-bindings/interrupt-controller/irq.h> > + i2c1 { > + clock-frequency = <400000>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + embedded-controller@70 { > + compatible = "lenovo,yoga-c630-ec"; > + reg = <0x70>; > + > + interrupts-extended = <&tlmm 20 IRQ_TYPE_LEVEL_HIGH>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + connector@0 { > + compatible = "usb-c-connector"; > + reg = <0>; > + power-role = "source"; > + data-role = "host"; > + }; > + > + connector@1 { > + compatible = "usb-c-connector"; > + reg = <1>; > + power-role = "source"; > + data-role = "host"; > + > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + port@1 { > + reg = <1>; > + lenovo_ec_dp_in: endpoint { > + remote-endpoint = <&mdss_dp_out>; You have inconsistent indentation. Use 4-spaces for entire DTS example. Best regards, Krzysztof