linux-kernel - Re: [PATCH v8 1/4] dt-bindings: clock: Document external clocks for MSM8998 gcc

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]

Date:   Tue, 12 Nov 2019 12:37:04 -0600
From:   Rob Herring <robh@...nel.org>
To:     Jeffrey Hugo <jhugo@...eaurora.org>
Cc:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Marc Gonzalez <marc.w.gonzalez@...e.fr>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        linux-clk <linux-clk@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        devicetree@...r.kernel.org
Subject: Re: [PATCH v8 1/4] dt-bindings: clock: Document external clocks for
 MSM8998 gcc

On Tue, Nov 12, 2019 at 10:25 AM Jeffrey Hugo <jhugo@...eaurora.org> wrote:
>
> On 11/11/2019 5:44 PM, Rob Herring wrote:
> > On Fri, Nov 08, 2019 at 04:17:16PM -0700, Jeffrey Hugo wrote:
> >> The global clock controller on MSM8998 can consume a number of external
> >> clocks.  Document them.
> >>
> >> Signed-off-by: Jeffrey Hugo <jhugo@...eaurora.org>
> >> ---
> >>   .../devicetree/bindings/clock/qcom,gcc.yaml        | 47 +++++++++++++++-------
> >>   1 file changed, 33 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> >> index e73a56f..2f3512b 100644
> >> --- a/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> >> +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.yaml
> >> @@ -40,20 +40,38 @@ properties:
> >>          - qcom,gcc-sm8150
> >>
> >>     clocks:
> >> -    minItems: 1
> >
> > 1 or 2 clocks are no longer allowed?
>
> Correct.
>
> The primary reason is that Stephen indicated in previous discussions
> that if the hardware exists, it should be indicated in DT, regardless if
> the driver uses it.  In the 7180 and 8150 case, the hardware exists, so
> these should not be optional.

Agreed. The commit message should mention this though.

>
> The secondary reason is I found that the schema was broken anyways.  In
> the way it was written, if you implemented sleep, you could not skip
> xo_ao, however there is a dts that did exactly that.

If a dts can be updated in a compatible way, we should do that rather
than carry inconsistencies into the schema.

> The third reason was that I couldn't find a way to write valid yaml to
> preserve the original meaning.  when you have an "items" as a subnode of
> "oneOf", you no longer have control over the minItems/maxItems, so all 3
> became required anyways.

That would be a bug. You're saying something like this doesn't work?:

oneOf:
  - minItems: 1
    maxItems: 3
    items:
      - const: a
      - const: b
      - const: c

>  I find it disappointing that the "version" of
> Yaml used for DT bindings is not documented,

Not sure which part you mean? json-schema is the vocabulary which has
a spec. The meta-schema then constrains what the json-schema structure
should look like. That's still evolving a bit as I try to improve it
based on mistakes people make. Then there's the intermediate .dt.yaml
format used internally. That's supposed to stay internal and may go
away when/if we integrate the validation into dtc.

> so after several hours of
> trial and error, I just gave up since I found this to work (failed cases
> just gave me an error with no indication of what was wrong, not even a
> line number).

Schema failures or dts failures? It is possible to get line numbers
for either, but that makes validation much slower. In the latter case,
the line numbers aren't too useful either given they are for the
.dt.yaml file and not the .dts source file (dtc integration would
solve that). Adding '-n' to dt-doc-validate or dt-validate will turn
them on though.

Yes, error messages need work. I have some idea how to improve them,
but haven't had time to implement. Too many binding reviews... You can
get more detail with '-v' option. It's *way* more verbose, but not
necessarily more useful.

Rob