[v3,0/7] ufs: set power domain performance state when scaling gears

Hi,

Changes since v2
================
1. PM: Split PM OPP patch into two - getting clocks and rates. (Viresh)
2. PM: Do not set clock rates from PM OPPs but rely on set_opp helper. (Viresh)
3. PM: Use clk bulk operations in PM OPP for getting/releasing the clocks. (Bjorn)
4. UFS: Rework clock scalling to be called in the same place as old method, so
   pre/post changes notification will work. (Mani)
5. UFS: Bail out if both freq-table-hz and operating-points are provided. (Mani)
6. Add review tags.

Changes since v1
================
1. Patch #1 qcom,gcc-sdm845: fix typo (Stephen).
2. Patch #2 ufs dt-bindings: not adding Rob's review because patch
   changed significantly.
3. PM: add new code for handling multiple clocks.
4. UFS: deprecate freq-table-hz property and use PM opps instead.

Dependencies
============
The UFS patch depends on PM OPP patches adding multiple clocks/rates support.

Best regards,
Krzysztof

Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Krzysztof Kozlowski (7):
  dt-bindings: clock: qcom,gcc-sdm845: add parent power domain
  dt-bindings: opp: accept array of frequencies
  dt-bindings: ufs: common: add OPP table
  arm64: dts: qcom: sdm845: control RPMHPD performance states with UFS
  PM: opp: allow control of multiple clocks
  PM: opp: parse multiple frequencies in each OPP
  ufs: use PM OPP when scaling gears

 .../bindings/clock/qcom,gcc-sdm845.yaml       |   3 +
 .../devicetree/bindings/opp/opp-v2-base.yaml  |  10 +
 .../devicetree/bindings/ufs/ufs-common.yaml   |  34 ++-
 arch/arm64/boot/dts/qcom/sdm845.dtsi          |  43 +++-
 drivers/opp/core.c                            | 207 +++++++++++++-----
 drivers/opp/of.c                              |  47 ++++
 drivers/opp/opp.h                             |   9 +-
 drivers/opp/ti-opp-supply.c                   |   6 +-
 drivers/scsi/ufs/ufshcd-pltfrm.c              |  73 ++++++
 drivers/scsi/ufs/ufshcd.c                     | 150 ++++++++++---
 drivers/scsi/ufs/ufshcd.h                     |   6 +
 include/linux/pm_opp.h                        |  32 ++-
 12 files changed, 518 insertions(+), 102 deletions(-)

On Fri, May 13, 2022 at 08:13:41AM +0200, Krzysztof Kozlowski wrote:
> Allow Qualcomm GCC to register its parent power domain (e.g. RPMHPD) to
> properly pass performance state from children.
> 
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

Thanks,
Mani

> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> Acked-by: Rob Herring <robh@kernel.org>
> 
> ---
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml
> index d902f137ab17..daf7906ebc40 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-sdm845.yaml
> @@ -43,6 +43,9 @@ properties:
>    '#reset-cells':
>      const: 1
>  
> +  power-domains:
> +    maxItems: 1
> +
>    '#power-domain-cells':
>      const: 1
>  
> -- 
> 2.32.0
>

On Fri, May 13, 2022 at 08:13:43AM +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>
> Reviewed-by: Rob Herring <robh@kernel.org>
> 
> ---
> 
> Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> ---
>  .../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.

This description mentions only the clocks and not voltages. But in theory, the
OPP table can contain other parameters like current, bandwidth, etc,... So to
avoid confusion, I'd suggest to get rid of the description.

> +
> +  opp-table: true
>  
>    interrupts:
>      maxItems: 1
> @@ -75,8 +88,23 @@ properties:
>  
>  dependencies:
>    freq-table-hz: [ 'clocks' ]
> +  operating-points-v2: [ 'clocks', 'clock-names' ]

What about voltage regulators if relevant opp property is present?

Thanks,
Mani

>  
>  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
> +
>  additionalProperties: true
> -- 
> 2.32.0
>

On Fri, May 13, 2022 at 11:10:20PM +0530, Manivannan Sadhasivam wrote:
> On Fri, May 13, 2022 at 08:13:43AM +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>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > 
> > ---
> > 
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>
> > ---
> >  .../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.
> 
> This description mentions only the clocks and not voltages. But in theory, the
> OPP table can contain other parameters like current, bandwidth, etc,... So to
> avoid confusion, I'd suggest to get rid of the description.
> 
> > +
> > +  opp-table: true
> >  
> >    interrupts:
> >      maxItems: 1
> > @@ -75,8 +88,23 @@ properties:
> >  
> >  dependencies:
> >    freq-table-hz: [ 'clocks' ]
> > +  operating-points-v2: [ 'clocks', 'clock-names' ]
> 
> What about voltage regulators if relevant opp property is present?
> 

Current UFS driver model won't allow us to change both voltage supplies and clks
using OPP implementation. So please ignore my above comment.

Thanks,
Mani

> Thanks,
> Mani
> 
> >  
> >  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
> > +
> >  additionalProperties: true
> > -- 
> > 2.32.0
> > 
> 
> -- 
> மணிவண்ணன் சதாசிவம்