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

Viresh Kumar
2021-06-24T04:32:56Z

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 ?

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

--
viresh