Re: [PATCH 2/2] dt-bindings: opp: Convert to DT schema

On Wed, Jun 23, 2021 at 10:32 PM Viresh Kumar viresh.kumar@linaro.org wrote:

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 viresh.kumar@linaro.org

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

Sure.

...

•  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 ?

regex. '?' means optional.

...

•      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
  

Actually, I need to drop these 3 lines as opp-microamp doesn't have a range like opp-microvolt.

Acked-by: Viresh Kumar viresh.kumar@linaro.org

-- viresh