Thanks for taking it up :)

On 23-06-21, 17:07, Rob Herring wrote:
> diff --git a/Documentation/devicetree/bindings/opp/opp-v2-base.yaml 
> b/Documentation/devicetree/bindings/opp/opp-v2-base.yaml
> +$id: http://devicetree.org/schemas/opp/opp-v2-base.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic OPP (Operating Performance Points) Common Binding
> +
> +maintainers:
> +  - Viresh Kumar <[email protected]>
> +
> +description: |
> +  Devices work at voltage-current-frequency combinations and some 
> implementations
> +  have the liberty of choosing these. These combinations are called Operating
> +  Performance Points aka OPPs. This document defines bindings for these OPPs
> +  applicable across wide range of devices. For illustration purpose, this 
> document
> +  uses CPU as a device.
> +
> +  This describes the OPPs belonging to a device.
> +
> +select: false
> +
> +properties:
> +  $nodename:
> +    pattern: '^opp-table(-[a-z0-9]+)?$'
> +
> +  opp-shared:
> +    description:
> +      Indicates that device nodes using this OPP Table Node's phandle switch
> +      their DVFS state together, i.e. they share clock/voltage/current lines.
> +      Missing property means devices have independent clock/voltage/current
> +      lines, but they share OPP tables.
> +    type: boolean
> +
> +patternProperties:
> +  '^opp-?[0-9]+$':
> +    type: object
> +    description:
> +      One or more OPP nodes describing voltage-current-frequency 
> combinations.
> +      Their name isn't significant but their phandle can be used to 
> reference an
> +      OPP. These are mandatory except for the case where the OPP table is
> +      present only to indicate dependency between devices using the 
> opp-shared
> +      property.
> +
> +    properties:
> +      opp-hz:
> +        description:
> +          Frequency in Hz, expressed as a 64-bit big-endian integer. This is 
> a
> +          required property for all device nodes, unless another "required"
> +          property to uniquely identify the OPP nodes exists. Devices like 
> power
> +          domains must have another (implementation dependent) property.
> +
> +      opp-peak-kBps:
> +        description:
> +          Peak bandwidth in kilobytes per second, expressed as an array of
> +          32-bit big-endian integers. Each element of the array represents 
> the
> +          peak bandwidth value of each interconnect path. The number of 
> elements
> +          should match the number of interconnect paths.
> +        minItems: 1
> +        maxItems: 32  # Should be enough

Can we move this down, closer to opp-avg-kBps ?

> +
> +      opp-microvolt:
> +        description: |
> +          Voltage for the OPP
> +
> +          A single regulator's voltage is specified with an array of size 
> one or three.
> +          Single entry is for target voltage and three entries are for 
> <target min max>
> +          voltages.
> +
> +          Entries for multiple regulators shall be provided in the same 
> field separated
> +          by angular brackets <>. The OPP binding doesn't provide any 
> provisions to
> +          relate the values to their power supplies or the order in which 
> the supplies
> +          need to be configured and that is left for the implementation 
> specific
> +          binding.
> +
> +          Entries for all regulators shall be of the same size, i.e. either 
> all use a
> +          single value or triplets.
> +        minItems: 1
> +        maxItems: 8

For consistency with rest of the doc, maybe add

# Should be enough regulators

> +        items:
> +          minItems: 1
> +          maxItems: 3
> +
> +      opp-microamp:
> +        description: |
> +          The maximum current drawn by the device in microamperes considering
> +          system specific parameters (such as transients, process, aging,
> +          maximum operating temperature range etc.) as necessary. This may be
> +          used to set the most efficient regulator operating mode.
> +
> +          Should only be set if opp-microvolt(-name)? is set for the OPP.

What is the significance of '?' here ?

> +
> +          Entries for multiple regulators shall be provided in the same field
> +          separated by angular brackets <>. If current values aren't required
> +          for a regulator, then it shall be filled with 0. If current values
> +          aren't required for any of the regulators, then this field is not
> +          required. The OPP binding doesn't provide any provisions to relate 
> the
> +          values to their power supplies or the order in which the supplies 
> need
> +          to be configured and that is left for the implementation specific
> +          binding.
> +        minItems: 1
> +        maxItems: 8   # Should be enough regulators
> +        items:
> +          minItems: 1
> +          maxItems: 3

Acked-by: Viresh Kumar <[email protected]>

-- 
viresh

Reply via email to