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

[Conor Dooley]

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@xxxxxxxxxx>
> > > 
> > > 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@xxxxxxxxxx>
> > > ---
> > >   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
> > > 
> > >

Attachment: signature.asc
Description: PGP signature