Re: [PATCH v6 01/14] dt-bindings: usb: tps6598x: Add tps25750

From: Krzysztof Kozlowski
Date: Sat Sep 23 2023 - 07:49:32 EST


On 23/09/2023 09:39, Abdel Alkuor wrote:
> From: Abdel Alkuor <abdelalkuor@xxxxxxxxxx>
>
> TPS25750 is USB TypeC PD controller which is a subset of TPS6598x.
>
> Signed-off-by: Abdel Alkuor <abdelalkuor@xxxxxxxxxx>
> ---
> Changes in v6:
> - Use reg property for patch address
> Changes in v5:
> - Add tps25750 bindings
>
> .../devicetree/bindings/usb/ti,tps6598x.yaml | 80 ++++++++++++++++++-
> 1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> index 5497a60cddbc..da299a2bb19e 100644
> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> @@ -20,8 +20,7 @@ properties:
> enum:
> - ti,tps6598x
> - apple,cd321x
> - reg:
> - maxItems: 1

reg must be defined in top-level.

https://elixir.bootlin.com/linux/v5.19-rc6/source/Documentation/devicetree/bindings/clock/samsung,exynos7-clock.yaml#L57

> + - ti,tps25750
>
> wakeup-source: true
>
> @@ -32,10 +31,55 @@ properties:
> items:
> - const: irq
>
> + firmware-name:
> + description: |
> + Should contain the name of the default patch binary
> + file located on the firmware search path which is
> + used to switch the controller into APP mode.
> + This is used when tps25750 doesn't have an EEPROM
> + connected to it.
> + maxItems: 1
> +
> required:
> - compatible
> - reg
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,tps25750
> + then:
> + properties:
> + reg:
> + maxItems: 2
> +
> + reg-names:
> + description: |
> + The first reg is PD device address and the second
> + reg is I2C slave address field in PBMs input data
> + which is used as the device address when writing the
> + patch for TPS25750.
> + The patch address can be any value except 0x00, 0x20,
> + 0x21, 0x22, and 0x23

Entire description is not suitable here, but should be used as
description of items in reg: (instead of maxItems).

> + items:
> + - const: main
> + - const: patch-address
> +
> + connector:
> + required:
> + - data-role
> +
> + required:
> + - connector
> + - reg-names
> + else:
> + properties:
> + reg:
> + maxItems: 1
> +
> +

Only one blank line.

Best regards,
Krzysztof