2023-09-17 19:42:05

by Abdel Alkuor

[permalink] [raw]
Subject: [PATCH v5 01/15] dt-bindings: usb: tps6598x: Add tps25750

From: Abdel Alkuor <[email protected]>

TPS25750 is USB TypeC PD controller which is a subset of TPS6598x.

Signed-off-by: Abdel Alkuor <[email protected]>
---
.../devicetree/bindings/usb/ti,tps6598x.yaml | 70 +++++++++++++++++++
1 file changed, 70 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
index 5497a60cddbc..e49bd92b5276 100644
--- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
+++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
@@ -20,6 +20,8 @@ properties:
enum:
- ti,tps6598x
- apple,cd321x
+ - ti,tps25750
+
reg:
maxItems: 1

@@ -32,10 +34,45 @@ 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
+
+ ti,patch-address:
+ description: |
+ One of PBMs command data field is I2C slave address
+ which is used when writing the patch for TPS25750.
+ The slave address can be any value except 0x00, 0x20,
+ 0x21, 0x22, and 0x23
+ $ref: /schemas/types.yaml#/definitions/uint8
+ minimum: 1
+ maximum: 0x7e
+
required:
- compatible
- reg

+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ const: ti,tps25750
+ then:
+ required:
+ - ti,patch-address
+ - connector
+
+ properties:
+ connector:
+ required:
+ - data-role
+
additionalProperties: true

examples:
@@ -68,4 +105,37 @@ examples:
};
};
};
+
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ typec@21 {
+ compatible = "ti,tps25750";
+ reg = <0x21>;
+
+ interrupt-parent = <&msmgpio>;
+ interrupts = <100 IRQ_TYPE_LEVEL_LOW>;
+ interrupt-names = "irq";
+ firmware-name = "tps25750.bin";
+
+ pinctrl-names = "default";
+ pinctrl-0 = <&typec_pins>;
+
+ ti,patch-address = /bits/ 8 <0x0f>;
+
+ typec_con0: connector {
+ compatible = "usb-c-connector";
+ label = "USB-C";
+ data-role = "dual";
+ port {
+ typec_ep0: endpoint {
+ remote-endpoint = <&otg_ep>;
+ };
+ };
+ };
+ };
+ };
...
--
2.34.1


2023-09-17 23:33:56

by Abdel Alkuor

[permalink] [raw]
Subject: Re: [PATCH v5 01/15] dt-bindings: usb: tps6598x: Add tps25750

On Sun, Sep 17, 2023 at 07:30:52PM +0200, Krzysztof Kozlowski wrote:
> On 17/09/2023 17:26, Abdel Alkuor wrote:
> > From: Abdel Alkuor <[email protected]>
> >
> > TPS25750 is USB TypeC PD controller which is a subset of TPS6598x.
> >
> > Signed-off-by: Abdel Alkuor <[email protected]>
> > ---
> > .../devicetree/bindings/usb/ti,tps6598x.yaml | 70 +++++++++++++++++++
> > 1 file changed, 70 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > index 5497a60cddbc..e49bd92b5276 100644
> > --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> > @@ -20,6 +20,8 @@ properties:
> > enum:
> > - ti,tps6598x
> > - apple,cd321x
> > + - ti,tps25750
> > +
> > reg:
> > maxItems: 1
> >
> > @@ -32,10 +34,45 @@ 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
> > +
> > + ti,patch-address:
> > + description: |
> > + One of PBMs command data field is I2C slave address
> > + which is used when writing the patch for TPS25750.
> > + The slave address can be any value except 0x00, 0x20,
> > + 0x21, 0x22, and 0x23
>
> Why this cannot be another entry in the reg?
>
This address is different than the physical address of PD controller.
The patch address will be used instead of PD controller address when
writing the patch. I thought reg proprity is only for a device physical
address, should I add another entry in the reg property in this case?
> > + $ref: /schemas/types.yaml#/definitions/uint8
> > + minimum: 1
> > + maximum: 0x7e
> > +
> > required:
> > - compatible
> > - reg
> >
> > +allOf:
> > + - if:
> > + properties:
> > + compatible:
> > + contains:
> > + const: ti,tps25750
> > + then:
> > + required:
> > + - ti,patch-address
> > + - connector
>
>
> Why? Connector should be required or not required for both devices. What
> is different between them?
>
The data-role for tps6598x can be extracted from system config register
which it doesn't exist in tps25750, so the only way to extract
this information is by using data-role property from the Connector for
tps25750, hence Connector and data-role are set as required for
tps25750.
>
> Best regards,
> Krzysztof
>
Thanks,
Abdel

2023-09-18 00:05:00

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v5 01/15] dt-bindings: usb: tps6598x: Add tps25750

On 17/09/2023 17:26, Abdel Alkuor wrote:
> From: Abdel Alkuor <[email protected]>
>
> TPS25750 is USB TypeC PD controller which is a subset of TPS6598x.
>
> Signed-off-by: Abdel Alkuor <[email protected]>
> ---
> .../devicetree/bindings/usb/ti,tps6598x.yaml | 70 +++++++++++++++++++
> 1 file changed, 70 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> index 5497a60cddbc..e49bd92b5276 100644
> --- a/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> +++ b/Documentation/devicetree/bindings/usb/ti,tps6598x.yaml
> @@ -20,6 +20,8 @@ properties:
> enum:
> - ti,tps6598x
> - apple,cd321x
> + - ti,tps25750
> +
> reg:
> maxItems: 1
>
> @@ -32,10 +34,45 @@ 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
> +
> + ti,patch-address:
> + description: |
> + One of PBMs command data field is I2C slave address
> + which is used when writing the patch for TPS25750.
> + The slave address can be any value except 0x00, 0x20,
> + 0x21, 0x22, and 0x23

Why this cannot be another entry in the reg?

> + $ref: /schemas/types.yaml#/definitions/uint8
> + minimum: 1
> + maximum: 0x7e
> +
> required:
> - compatible
> - reg
>
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + const: ti,tps25750
> + then:
> + required:
> + - ti,patch-address
> + - connector


Why? Connector should be required or not required for both devices. What
is different between them?


Best regards,
Krzysztof