diff mbox series

[1/3] dt-bindings: connector: Add pd-revision property

Message ID 20241205-get_rev_upstream-v1-1-90158ee7d75f@google.com (mailing list archive)
State Superseded
Headers show
Series Add support for responding to Get_Revision request | expand

Commit Message

Amit Sunil Dhamne via B4 Relay Dec. 6, 2024, 7:46 a.m. UTC
From: Amit Sunil Dhamne <amitsd@google.com>

Add pd-revision property definition, to specify the maximum Power
Delivery Revision and Version supported by the connector.

Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
---
 Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
 Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
 2 files changed, 7 insertions(+)

Comments

Conor Dooley Dec. 6, 2024, 4:52 p.m. UTC | #1
On Thu, Dec 05, 2024 at 11:46:08PM -0800, Amit Sunil Dhamne via B4 Relay wrote:
> From: Amit Sunil Dhamne <amitsd@google.com>
> 
> Add pd-revision property definition, to specify the maximum Power
> Delivery Revision and Version supported by the connector.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -293,6 +293,12 @@ properties:
>        PD negotiation till BC1.2 detection completes.
>      default: 0
>  
> +  pd-revision:
> +    description: Specifies the maximum USB PD revision and version supported by
> +      the connector. This property is specified in the following order;
> +      <revision_major, revision_minor, version_major, version_minor>.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> +
>  dependencies:
>    sink-vdos-v1: [ sink-vdos ]
>    sink-vdos: [ sink-vdos-v1 ]
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -70,6 +70,7 @@ examples:
>                                         PDO_FIXED_DUAL_ROLE)
>                                         PDO_FIXED(9000, 2000, 0)>;
>                  sink-bc12-completion-time-ms = <500>;
> +                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;

Why do you need this? Doesn't the compatible already give you this
information?

>              };
>          };
>      };
> 
> -- 
> 2.47.0.338.g60cca15819-goog
> 
>
Amit Sunil Dhamne Dec. 7, 2024, 12:43 a.m. UTC | #2
Hi Conor,

On 12/6/24 8:52 AM, Conor Dooley wrote:
> On Thu, Dec 05, 2024 at 11:46:08PM -0800, Amit Sunil Dhamne via B4 Relay wrote:
>> From: Amit Sunil Dhamne<amitsd@google.com>
>>
>> Add pd-revision property definition, to specify the maximum Power
>> Delivery Revision and Version supported by the connector.
>>
>> Signed-off-by: Amit Sunil Dhamne<amitsd@google.com>
>> ---
>>   Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
>>   Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -293,6 +293,12 @@ properties:
>>         PD negotiation till BC1.2 detection completes.
>>       default: 0
>>   
>> +  pd-revision:
>> +    description: Specifies the maximum USB PD revision and version supported by
>> +      the connector. This property is specified in the following order;
>> +      <revision_major, revision_minor, version_major, version_minor>.
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
>> +
>>   dependencies:
>>     sink-vdos-v1: [ sink-vdos ]
>>     sink-vdos: [ sink-vdos-v1 ]
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> @@ -70,6 +70,7 @@ examples:
>>                                          PDO_FIXED_DUAL_ROLE)
>>                                          PDO_FIXED(9000, 2000, 0)>;
>>                   sink-bc12-completion-time-ms = <500>;
>> +                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
> Why do you need this?

This DT property helps Type-C Port Manager (TCPM, consumer of the 
connector class properties) fetch the exact Power Delivery (PD) revision 
& version information of the Type-C port controller (TCPC)'s connector. 
In turn, we require it to be able to support "Revision_Information" 
Atomic Message Sequence (AMS) in TCPM to be USB PD spec compliant for 
all revision & versions after PD3.1 v1.x.

> Doesn't the compatible already give you this
> information?

Compatible property does not give information regarding the PD revision 
& version but only gives info on the type of connector (usb a, b or c). 
Also, connector class is used by several TCPCs like maxim,max33359, 
ptn5110, etc. and each of them may be compliant to  different 
combinations of revision & version. This feature would help users set 
these values based on the revision/versions their TCPC supports.

Currently  TCPM driver hardcodes the Revision value to 3.0 and doesn't 
provide any info on version (undesirable).

It should be noted that:

1. There are multiple versions & revisions of the USB PD spec and they 
keep evolving frequently. A certain connector hardware may only be spec 
compliant for up to a certain version + version. Thus, this is the only 
way for TCPM to know what ver + rev the connector hardware supports. 
This will enable the TCPC system to present the exact rev & ver values 
when requested for revision info by the port partner.

2. I also considered incrementing the revision & version information 
values in the TCPM code. However, that won't be backward compatible for 
connectors whose hardware doesn't support features in the latest spec. 
In this case we would be presenting incorrect revision & version values 
(higher than what is actually supported by the hardware).

Regards,

Amit

>>               };
>>           };
>>       };
>>
>> -- 
>> 2.47.0.338.g60cca15819-goog
>>
>>
Rob Herring (Arm) Dec. 10, 2024, 11:06 p.m. UTC | #3
On Thu, Dec 05, 2024 at 11:46:08PM -0800, Amit Sunil Dhamne wrote:
> Add pd-revision property definition, to specify the maximum Power
> Delivery Revision and Version supported by the connector.
> 
> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
> ---
>  Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
>  Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
>  2 files changed, 7 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -293,6 +293,12 @@ properties:
>        PD negotiation till BC1.2 detection completes.
>      default: 0
>  
> +  pd-revision:
> +    description: Specifies the maximum USB PD revision and version supported by
> +      the connector. This property is specified in the following order;
> +      <revision_major, revision_minor, version_major, version_minor>.
> +    $ref: /schemas/types.yaml#/definitions/uint8-array

Always exactly 4 entries? Then:

maxItems: 4

> +
>  dependencies:
>    sink-vdos-v1: [ sink-vdos ]
>    sink-vdos: [ sink-vdos-v1 ]
> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> @@ -70,6 +70,7 @@ examples:
>                                         PDO_FIXED_DUAL_ROLE)
>                                         PDO_FIXED(9000, 2000, 0)>;
>                  sink-bc12-completion-time-ms = <500>;
> +                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
>              };
>          };
>      };
> 
> -- 
> 2.47.0.338.g60cca15819-goog
>
Amit Sunil Dhamne Dec. 11, 2024, 2:55 a.m. UTC | #4
Hi Rob,

On 12/10/24 3:06 PM, Rob Herring wrote:
> On Thu, Dec 05, 2024 at 11:46:08PM -0800, Amit Sunil Dhamne wrote:
>> Add pd-revision property definition, to specify the maximum Power
>> Delivery Revision and Version supported by the connector.
>>
>> Signed-off-by: Amit Sunil Dhamne <amitsd@google.com>
>> ---
>>   Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
>>   Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
>>   2 files changed, 7 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
>> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
>> @@ -293,6 +293,12 @@ properties:
>>         PD negotiation till BC1.2 detection completes.
>>       default: 0
>>   
>> +  pd-revision:
>> +    description: Specifies the maximum USB PD revision and version supported by
>> +      the connector. This property is specified in the following order;
>> +      <revision_major, revision_minor, version_major, version_minor>.
>> +    $ref: /schemas/types.yaml#/definitions/uint8-array
> Always exactly 4 entries? Then:
>
> maxItems: 4

Thanks for the catch! Updating it in the next revision.

Regards,

Amit

>> +
>>   dependencies:
>>     sink-vdos-v1: [ sink-vdos ]
>>     sink-vdos: [ sink-vdos-v1 ]
>> diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
>> --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
>> @@ -70,6 +70,7 @@ examples:
>>                                          PDO_FIXED_DUAL_ROLE)
>>                                          PDO_FIXED(9000, 2000, 0)>;
>>                   sink-bc12-completion-time-ms = <500>;
>> +                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
>>               };
>>           };
>>       };
>>
>> -- 
>> 2.47.0.338.g60cca15819-goog
>>
Conor Dooley Dec. 15, 2024, 3:37 p.m. UTC | #5
On Fri, Dec 06, 2024 at 04:43:44PM -0800, Amit Sunil Dhamne wrote:
> Hi Conor,
> 
> On 12/6/24 8:52 AM, Conor Dooley wrote:
> > On Thu, Dec 05, 2024 at 11:46:08PM -0800, Amit Sunil Dhamne via B4 Relay wrote:
> > > From: Amit Sunil Dhamne<amitsd@google.com>
> > > 
> > > Add pd-revision property definition, to specify the maximum Power
> > > Delivery Revision and Version supported by the connector.
> > > 
> > > Signed-off-by: Amit Sunil Dhamne<amitsd@google.com>
> > > ---
> > >   Documentation/devicetree/bindings/connector/usb-connector.yaml | 6 ++++++
> > >   Documentation/devicetree/bindings/usb/maxim,max33359.yaml      | 1 +
> > >   2 files changed, 7 insertions(+)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
> > > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > > @@ -293,6 +293,12 @@ properties:
> > >         PD negotiation till BC1.2 detection completes.
> > >       default: 0
> > > +  pd-revision:
> > > +    description: Specifies the maximum USB PD revision and version supported by
> > > +      the connector. This property is specified in the following order;
> > > +      <revision_major, revision_minor, version_major, version_minor>.
> > > +    $ref: /schemas/types.yaml#/definitions/uint8-array
> > > +
> > >   dependencies:
> > >     sink-vdos-v1: [ sink-vdos ]
> > >     sink-vdos: [ sink-vdos-v1 ]
> > > diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
> > > --- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > +++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
> > > @@ -70,6 +70,7 @@ examples:
> > >                                          PDO_FIXED_DUAL_ROLE)
> > >                                          PDO_FIXED(9000, 2000, 0)>;
> > >                   sink-bc12-completion-time-ms = <500>;
> > > +                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
> > Why do you need this?
> 
> This DT property helps Type-C Port Manager (TCPM, consumer of the connector
> class properties) fetch the exact Power Delivery (PD) revision & version
> information of the Type-C port controller (TCPC)'s connector. In turn, we
> require it to be able to support "Revision_Information" Atomic Message
> Sequence (AMS) in TCPM to be USB PD spec compliant for all revision &
> versions after PD3.1 v1.x.

This information should be in hte commit message.

> 
> > Doesn't the compatible already give you this
> > information?
> 
> Compatible property does not give information regarding the PD revision &
> version but only gives info on the type of connector (usb a, b or c). Also,
> connector class is used by several TCPCs like maxim,max33359, ptn5110, etc.
> and each of them may be compliant to  different combinations of revision &
> version. This feature would help users set these values based on the
> revision/versions their TCPC supports.

Is the version fixed for a given TCPC? If so, the driver would be able
to determine the correct revision based on the compatible. If not, then
you commit message needs to mention that this is variable.

> Currently  TCPM driver hardcodes the Revision value to 3.0 and doesn't
> provide any info on version (undesirable).
> 
> It should be noted that:
> 
> 1. There are multiple versions & revisions of the USB PD spec and they keep
> evolving frequently. A certain connector hardware may only be spec compliant
> for up to a certain version + version. Thus, this is the only way for TCPM
> to know what ver + rev the connector hardware supports. This will enable the
> TCPC system to present the exact rev & ver values when requested for
> revision info by the port partner.
> 
> 2. I also considered incrementing the revision & version information values
> in the TCPM code. However, that won't be backward compatible for connectors
> whose hardware doesn't support features in the latest spec. In this case we
> would be presenting incorrect revision & version values (higher than what is
> actually supported by the hardware).
> 
> Regards,
> 
> Amit
> 
> > >               };
> > >           };
> > >       };
> > > 
> > > -- 
> > > 2.47.0.338.g60cca15819-goog
> > > 
> > >
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 67700440e23b5b7ca0db2c395c8a455bcf650864..341d2872e8d43450d219b7b72d48790051dc4e2b 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -293,6 +293,12 @@  properties:
       PD negotiation till BC1.2 detection completes.
     default: 0
 
+  pd-revision:
+    description: Specifies the maximum USB PD revision and version supported by
+      the connector. This property is specified in the following order;
+      <revision_major, revision_minor, version_major, version_minor>.
+    $ref: /schemas/types.yaml#/definitions/uint8-array
+
 dependencies:
   sink-vdos-v1: [ sink-vdos ]
   sink-vdos: [ sink-vdos-v1 ]
diff --git a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
index 20b62228371bdedf2fe92767ffe443bec87babc5..350d39fbf2dcd4d99db07cb8f099467e6fc653ee 100644
--- a/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
+++ b/Documentation/devicetree/bindings/usb/maxim,max33359.yaml
@@ -70,6 +70,7 @@  examples:
                                        PDO_FIXED_DUAL_ROLE)
                                        PDO_FIXED(9000, 2000, 0)>;
                 sink-bc12-completion-time-ms = <500>;
+                pd-revision = /bits/ 8 <0x03 0x01 0x01 0x08>;
             };
         };
     };