linux-kernel - Re: [PATCH v2 1/2] dt-bindings: connector: Add child nodes for multiple PD capabilities

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230821162517.GA1832382-robh@kernel.org>
Date:   Mon, 21 Aug 2023 11:25:17 -0500
From:   Rob Herring <robh@...nel.org>
To:     Kyle Tso <kyletso@...gle.com>
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 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.

> 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).

> 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.

> +  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
>