[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGZ6i=0ey+94YYgc=os5iQ_pNmNVB=qZQD0=jB4fRTKggZPSWA@mail.gmail.com>
Date: Thu, 19 Oct 2023 02:19:01 +0800
From: Kyle Tso <kyletso@...gle.com>
To: Rob Herring <robh@...nel.org>
Cc: krzysztof.kozlowski+dt@...aro.org, conor+dt@...nel.org,
linux@...ck-us.net, heikki.krogerus@...ux.intel.com,
gregkh@...uxfoundation.org, badhri@...gle.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-usb@...r.kernel.org
Subject: Re: [PATCH v2 1/2] dt-bindings: connector: Add child nodes for
multiple PD capabilities
On Tue, Aug 22, 2023 at 12:25 AM Rob Herring <robh@...nel.org> wrote:
>
Sorry for the late response...
> On Tue, Aug 08, 2023 at 01:41:58AM +0800, Kyle Tso wrote:
> > Define a new optional property "capabilities" which is a child node
> > under connector to contain multiple USB Power Delivery capabilities.
> >
> > Define a new property with pattern (e.g. caps-0, caps-1) which is a
>
> A property in json-schema terms, but for DT it's a node not a property.
> 'Define a child node ...' would be clearer.
>
Will do in the future patch
> > child node under "capabilities". Each node contains PDO data of a
> > selectable Power Delivery capability.
> >
> > Also define common properties for source-pdos, sink-pdos, and
> > op-sink-microwatt that can be referenced.
>
> Why do we need this? What issue does this solve? The commit message
> should answer those questions (always).
>
Will state the rationale in the future patch
> > Signed-off-by: Kyle Tso <kyletso@...gle.com>
> > ---
> > v1 -> v2:
> > - move source/sink-pdos to $defs and reference them in properties
> >
> > .../bindings/connector/usb-connector.yaml | 80 +++++++++++++------
> > 1 file changed, 57 insertions(+), 23 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 1c4d3eb87763..c6b02dbda83f 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -117,28 +117,10 @@ properties:
> > # The following are optional properties for "usb-c-connector" with power
> > # delivery support.
> > source-pdos:
> > - description: An array of u32 with each entry providing supported power
> > - source data object(PDO), the detailed bit definitions of PDO can be found
> > - in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> > - Source_Capabilities Message, the order of each entry(PDO) should follow
> > - the PD spec chapter 6.4.1. Required for power source and power dual role.
> > - User can specify the source PDO array via PDO_FIXED/BATT/VAR/PPS_APDO()
> > - defined in dt-bindings/usb/pd.h.
> > - minItems: 1
> > - maxItems: 7
> > - $ref: /schemas/types.yaml#/definitions/uint32-array
> > + $ref: "#/$defs/source-pdos"
> >
> > sink-pdos:
> > - description: An array of u32 with each entry providing supported power sink
> > - data object(PDO), the detailed bit definitions of PDO can be found in
> > - "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> > - Sink Capabilities Message, the order of each entry(PDO) should follow the
> > - PD spec chapter 6.4.1. Required for power sink and power dual role. User
> > - can specify the sink PDO array via PDO_FIXED/BATT/VAR/PPS_APDO() defined
> > - in dt-bindings/usb/pd.h.
> > - minItems: 1
> > - maxItems: 7
> > - $ref: /schemas/types.yaml#/definitions/uint32-array
> > + $ref: "#/$defs/sink-pdos"
> >
> > sink-vdos:
> > description: An array of u32 with each entry, a Vendor Defined Message Object (VDO),
> > @@ -164,9 +146,7 @@ properties:
> > $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > op-sink-microwatt:
> > - description: Sink required operating power in microwatt, if source can't
> > - offer the power, Capability Mismatch is set. Required for power sink and
> > - power dual role.
> > + $ref: "#/$defs/op-sink-microwatt"
> >
> > port:
> > $ref: /schemas/graph.yaml#/properties/port
> > @@ -228,6 +208,30 @@ properties:
> > SNK_READY for non-pd link.
> > type: boolean
> >
> > + capabilities:
> > + description: A child node to contain all the selectable USB Power Delivery capabilities.
> > + type: object
> > +
> > + patternProperties:
> > + "^caps-[0-9]+$":
> > + description: Child nodes under "capabilities" node. Each node contains a selectable USB
> > + Power Delivery capability.
> > + type: object
> > +
> > + properties:
> > + source-pdos:
> > + $ref: "#/$defs/source-pdos"
> > +
> > + sink-pdos:
> > + $ref: "#/$defs/sink-pdos"
> > +
> > + op-sink-microwatt:
> > + $ref: "#/$defs/op-sink-microwatt"
> > +
> > + additionalProperties: false
> > +
> > + additionalProperties: false
> > +
> > dependencies:
> > sink-vdos-v1: [ 'sink-vdos' ]
> > sink-vdos: [ 'sink-vdos-v1' ]
> > @@ -235,6 +239,36 @@ dependencies:
> > required:
> > - compatible
> >
> > +$defs:
>
> Make this:
>
> $defs:
> capabilities:
> properties:
> ...
>
> And then just reference "#/$defs/capabilities" at the top-level and in
> caps-[0-9] node schema.
>
> You'll need to use unevaluatedProperties instead of additionalProperties
> as well.
>
I am sorry I don't understand this part. Could you explain more?
In $defs, define a whole structure of "capabilities" which has
patternProperties "caps-[0-9]+", and inside "caps-[0-9]+" there are
properties "source/sink-pdos" ?
Reference it in the property "capabilities" under connector node?
Reference it in the property "source/sink-pdos" under connector node as well?
And I don't know the usage of unevaluatedProperties. Where should it
be inserted and what is the value of it?
> > + source-pdos:
> > + description: An array of u32 with each entry providing supported power
> > + source data object(PDO), the detailed bit definitions of PDO can be found
> > + in "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.2
> > + Source_Capabilities Message, the order of each entry(PDO) should follow
> > + the PD spec chapter 6.4.1. Required for power source and power dual role.
> > + User can specify the source PDO array via PDO_FIXED/BATT/VAR/PPS_APDO()
> > + defined in dt-bindings/usb/pd.h.
> > + minItems: 1
> > + maxItems: 7
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + sink-pdos:
> > + description: An array of u32 with each entry providing supported power sink
> > + data object(PDO), the detailed bit definitions of PDO can be found in
> > + "Universal Serial Bus Power Delivery Specification" chapter 6.4.1.3
> > + Sink Capabilities Message, the order of each entry(PDO) should follow the
> > + PD spec chapter 6.4.1. Required for power sink and power dual role. User
> > + can specify the sink PDO array via PDO_FIXED/BATT/VAR/PPS_APDO() defined
> > + in dt-bindings/usb/pd.h.
> > + minItems: 1
> > + maxItems: 7
> > + $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> > + op-sink-microwatt:
> > + description: Sink required operating power in microwatt, if source can't
> > + offer the power, Capability Mismatch is set. Required for power sink and
> > + power dual role.
> > +
> > allOf:
> > - if:
> > properties:
> > --
> > 2.41.0.585.gd2178a4bd4-goog
> >
Powered by blists - more mailing lists