linux-kernel - Re: [PATCH 13/13] dt-bindings: cpufreq: qcom-hw: Add bindings for 8998

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20201208181103.GA2795715@robh.at.kernel.org>
Date:   Tue, 8 Dec 2020 12:11:03 -0600
From:   Rob Herring <robh@...nel.org>
To:     AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...ainline.org>
Cc:     linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org, linux-pm@...r.kernel.org,
        ulf.hansson@...aro.org, jorge.ramirez-ortiz@...aro.org,
        broonie@...nel.org, lgirdwood@...il.com, daniel.lezcano@...aro.org,
        nks@...wful.org, bjorn.andersson@...aro.org, agross@...nel.org,
        viresh.kumar@...aro.org, rjw@...ysocki.net,
        konrad.dybcio@...ainline.org, martin.botka@...ainline.org,
        marijn.suijten@...ainline.org, phone-devel@...r.kernel.org
Subject: Re: [PATCH 13/13] dt-bindings: cpufreq: qcom-hw: Add bindings for
 8998

On Thu, Nov 26, 2020 at 07:45:59PM +0100, AngeloGioacchino Del Regno wrote:
> The OSM programming addition has been done under the
> qcom,cpufreq-hw-8998 compatible name: specify the requirement
> of two additional register spaces for this functionality.
> 
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...ainline.org>
> ---
>  .../bindings/cpufreq/qcom,cpufreq-hw.yaml     | 31 ++++++++++++++++---
>  1 file changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml
> index 94a56317b14b..f64cea73037e 100644
> --- a/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml
> +++ b/Documentation/devicetree/bindings/cpufreq/qcom,cpufreq-hw.yaml
> @@ -23,17 +23,21 @@ properties:
>            - qcom,cpufreq-epss
>  
>    reg:
> +    description: Base address and size of the RBCPR register region

That doesn't make sense given you have 2 regions.

>      minItems: 2
>      maxItems: 2

maxItems: 4

>  
>    reg-names:
>      description:
> -      Frequency domain register region for each domain.
> -    items:
> -      - const: "freq-domain0"
> -      - const: "freq-domain1"
> +      Frequency domain register region for each domain. If OSM programming
> +      does not happen in the bootloader and has to be done in this driver,
> +      then also the OSM domain region osm-domain[0-1] has to be provided.

Don't write free form text for what can be expressed as schema.

> +    minItems: 2
> +    maxItems: 2

You obviously haven't tried this change with 8998. It will fail with 
more than 2. What you need here is:

minItems: 2
maxItems: 4

items:
  - const: "freq-domain0"
  - const: "freq-domain1"
  - const: "osm-domain0"
  - const: "osm-domain1"

And then...

>  
>    clock-names:
> +    minItems: 2
> +    maxItems: 2
>      - const: xo
>      - const: ref
>  
> @@ -53,9 +57,28 @@ properties:
>        property with phandle to a cpufreq_hw followed by the Domain ID(0/1)
>        in the CPU DT node.
>  
> +allOf:
> + - if:
> +     properties:
> +       reg-names:
> +         contains:
> +           const: qcom,cpufreq-hw-8998
> +   then:
> +     properties:
> +       reg:
> +         minItems: 4
> +         maxItems: 4
> +       reg-names:

...here just:

minItems: 4

And you'll need an 'else' clause with 'maxItems: 2' for reg and 
reg-names.

> +         items:
> +           - const: "freq-domain0"
> +           - const: "freq-domain1"
> +           - const: "osm-domain0"
> +           - const: "osm-domain1"
> +
>  required:
>    - compatible
>    - reg
> +  - reg-names

You can't make something that was optional now required. (Unless it was 
a mistake and all existing users always had 'reg-names'.)

>    - clock-names
>    - clocks
>    - "#freq-domain-cells"
> -- 
> 2.29.2
>