On 27/01/2023 16:10, Jon Hunter wrote:
> From: Wayne Chang <waynec@xxxxxxxxxx>
>
> Add the device-tree binding documentation for Cypress cypd4226 dual
> Type-C controller.
>
> Signed-off-by: Wayne Chang <waynec@xxxxxxxxxx>
> Signed-off-by: Jon Hunter <jonathanh@xxxxxxxxxx>
> ---
> V9: added 'unevaluatedProperties' and 'additionalProperties'
> V8: removed 'additionalProperties'
> V7: updated example to use 'typec' for the node name
> V6: no changes
> V5: updated subject and updated binding to use 'firmware-name'.
> V4: no changes
> V3: fix additionalProperties warning on new schema
> V2: based on the review comments. Fix some addressed issues on
Thanks, this is looking good, although few more questions popped up
while comparing it with other bindings.
(...)
> + firmware-name:
> + enum:
> + - nvidia,gpu
> + - nvidia,jetson-agx-xavier
> + description: |
> + The name of the CCGx firmware built for product series.
> + should be set one of following:
> + - "nvidia,gpu" for the NVIDIA RTX product series
> + - "nvidia,jetson-agx-xavier" for the NVIDIA Jetson product series
> +
> +patternProperties:
> + '^connector@[0-1]+$':
How many connectors do you expect/support? 1111 is valid? I guess you
wanted only [01]?
> + $ref: /schemas/connector/usb-connector.yaml#
> + unevaluatedProperties: false
> + properties:
> + reg:
> + maxItems: 1
> +
> +required:
> + - compatible
> + - reg
> + - interrupts
I would assume that at least one connector is required (oneOf: required:).
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/gpio/tegra194-gpio.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + #interrupt-cells = <2>;
> +
> + typec@8 {
> + compatible = "cypress,cypd4226";
> + reg = <0x08>;
> + interrupt-parent = <&gpio_aon>;
> + interrupts = <TEGRA194_AON_GPIO(BB, 2) IRQ_TYPE_LEVEL_LOW>;
> + firmware-name = "nvidia,jetson-agx-xavier";
> + #address-cells = <1>;
> + #size-cells = <0>;
> + connector@0 {
> + compatible = "usb-c-connector";
> + reg = <0>;
> + label = "USB-C";
> + data-role = "dual";
> + ports {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + port@0 {
> + reg = <0>;
> + endpoint {
> + remote-endpoint = <&usb_role_switch0>;
> + };
> + };
> + };
> + };
> + };
> + };
Best regards,
Krzysztof
