Re: [RFC PATCH v2 3/6] dt-bindings: ufs: common: add OPP table

Rob Herring
2022-04-14T16:04:46Z

On Thu, Apr 14, 2022 at 10:25 AM Rob Herring <robh@kernel.org> wrote:
>
> On Mon, Apr 11, 2022 at 05:43:44PM +0200, Krzysztof Kozlowski wrote:
> > Except scaling UFS and bus clocks, it's necessary to scale also the
> > voltages of regulators or power domain performance state levels. Adding
> > Operating Performance Points table allows to adjust power domain
> > performance state, depending on the UFS clock speed.
> >
> > OPPv2 deprecates previous property limited to clock scaling:
> > freq-table-hz.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> >
> > ---
> >
> > Not adding Rob's review tag because patch changed significantly.
> > ---
> > .../devicetree/bindings/ufs/ufs-common.yaml | 34 +++++++++++++++++--
> > 1 file changed, 31 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/ufs/ufs-common.yaml b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> > index 47a4e9e1a775..d7d2c8a136bb 100644
> > --- a/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> > +++ b/Documentation/devicetree/bindings/ufs/ufs-common.yaml
> > @@ -20,11 +20,24 @@ properties:
> > items:
> > - description: Minimum frequency for given clock in Hz
> > - description: Maximum frequency for given clock in Hz
> > + deprecated: true
> > description: |
> > + Preferred is operating-points-v2.
> > +
> > Array of <min max> operating frequencies in Hz stored in the same order
> > - as the clocks property. If this property is not defined or a value in the
> > - array is "0" then it is assumed that the frequency is set by the parent
> > - clock or a fixed rate clock source.
> > + as the clocks property. If either this property or operating-points-v2 is
> > + not defined or a value in the array is "0" then it is assumed that the
> > + frequency is set by the parent clock or a fixed rate clock source.
> > +
> > + operating-points-v2:
> > + description:
> > + Preferred over freq-table-hz.
> > + If present, each OPP must contain array of frequencies stored in the same
> > + order for each clock. If clock frequency in the array is "0" then it is
> > + assumed that the frequency is set by the parent clock or a fixed rate
> > + clock source.
> > +
> > + opp-table: true
> >
> > interrupts:
> > maxItems: 1
> > @@ -75,8 +88,23 @@ properties:
> >
> > dependencies:
> > freq-table-hz: [ 'clocks' ]
> > + operating-points-v2: [ 'clocks', 'clock-names' ]
> >
> > required:
> > - interrupts
> >
> > +allOf:
> > + - if:
> > + required:
> > + - freq-table-hz
> > + then:
> > + properties:
> > + operating-points-v2: false
> > + - if:
> > + required:
> > + - operating-points-v2
> > + then:
> > + properties:
> > + freq-table-hz: false
>
> You could also express this as:
>
> oneOf:
> - required: [ freq-table-hz ]
> - required: [ operating-points-v2 ]
> - not:
> required: [ freq-table-hz, operating-points-v2 ]

Err, NM. That doesn't work...

Reviewed-by: Rob Herring <robh@kernel.org>