On Wed 10 Aug 06:35 PDT 2022, Krzysztof Kozlowski wrote:
> 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?
>
Correct, while it's only possible to do Displayport on the second
connector, the EC is involved in both the connectors - based on the
events received from it.
> > +
> > + '#size-cells':
> > + const: 0
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > +patternProperties:
> > + '^connector@\d$':
> > + $ref: /schemas/connector/usb-connector.yaml#
>
> unevaluatedProperties:false inside connector (on its level)
>
Okay, will update accordingly.
> > +
> > +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.
>
Odd, will fix.
Thanks,
Bjorn
> Best regards,
> Krzysztof
