[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAD=FV=UsURCk4WD9uhvw1LJoTNMZ0anxgEfFGXRMk1Ps3HWxeQ@mail.gmail.com>
Date: Mon, 3 Feb 2020 09:09:02 -0800
From: Doug Anderson <dianders@...omium.org>
To: Stephen Boyd <sboyd@...nel.org>
Cc: Rob Herring <robh+dt@...nel.org>, Taniya Das <tdas@...eaurora.org>,
Jeffrey Hugo <jhugo@...eaurora.org>,
Abhishek Sahu <absahu@...eaurora.org>, sivaprak@...eaurora.org,
anusharao@...eaurora.org, Sricharan <sricharan@...eaurora.org>,
Rob Herring <robh@...nel.org>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Michael Turquette <mturquette@...libre.com>,
"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS"
<devicetree@...r.kernel.org>,
linux-arm-msm <linux-arm-msm@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Andy Gross <agross@...nel.org>,
Mark Rutland <mark.rutland@....com>,
linux-clk <linux-clk@...r.kernel.org>
Subject: Re: [PATCH v2] dt-bindings: clk: qcom: Fix self-validation, split,
and clean cruft
Hi,
On Thu, Jan 30, 2020 at 10:04 AM Stephen Boyd <sboyd@...nel.org> wrote:
>
> Quoting Douglas Anderson (2020-01-29 15:25:06)
> > The 'qcom,gcc.yaml' file failed self-validation (dt_binding_check)
> > because it required a property to be either (3 entries big),
> > (3 entries big), or (7 entries big), but not more than one of those
> > things. That didn't make a ton of sense.
> >
> > This patch splits all of the exceptional device trees (AKA those that
> > would have needed if/then/else rules) from qcom,gcc.yaml. It also
> > cleans up some cruft found while doing that.
> >
> > After this lands, this worked for me atop clk-next with just the known
> > error about msm8998:
> > for f in \
> > Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-ipq8074.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-msm8996.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-msm8998.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-qcs404.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc-sm8150.yaml \
> > Documentation/devicetree/bindings/clock/qcom,gcc.yaml; do \
> > ARCH=arm64 make dtbs_check DT_SCHEMA_FILES=$f; \
> > done
> >
> > I then picked this patch atop linux-next (next-20200129) and ran:
> > # Delete broken yaml:
> > rm Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
> > ARCH=arm64 make dt_binding_check | grep 'clock/qcom'
> > ...and that didn't seem to indicate problems.
> >
> > Arbitrary decisions made (yell if you want changed):
> > - Left all the older devices (where clocks / clock-names weren't
> > specified) in a single file.
> > - Didn't make clocks "required" for msm8996 but left them as listed.
> > This seems a little weird but it matches the old binding.
> >
> > Misc cleanups as part of this patch:
> > - sm8150 was claimed to be same set of clocks as sc7180, but driver
> > and dts appear to say that "bi_tcxo_ao" doesn't exist. Fixed.
>
> Someone will probably want to change this at some point.
I haven't heard anything from Qualcomm, so leaving it the way it was
in v2 for the v3 I'm prepping. Someone can always do a follow-up
patch.
> > - In "apq8064", "#thermal-sensor-cells" was missing the "#".
> > - Got rid of "|" at the end of top description since spacing doesn't
> > matter.
> > - Changed indentation to consistently 2 spaces (it was 3 in some
> > places).
> > - Added period at the end of protected-clocks description.
> > - No space before ":".
> > - Updated sc7180/sm8150 example to use the 'qcom,rpmh.h' include.
> > - Updated sc7180/sm8150 example to use larger address/size cells as
> > per reality.
> > - Updated sc7180/sm8150 example to point to the sleep_clk rather than
> > <0>.
> > - Made it so that gcc-ipq8074 didn't require #power-domain-cells since
> > actual dts didn't have it and I got no hits from:
> > git grep _GDSC include/dt-bindings/clock/qcom,gcc-ipq8074.h
> > - Made it so that gcc-qcs404 didn't require #power-domain-cells since
> > actual dts didn't have it and I got no hits from:
> > git grep _GDSC include/dt-bindings/clock/qcom,gcc-qcs404.h
> >
> > Noticed, but not done in this patch (volunteers needed):
> > - Add "aud_ref_clk" to sm8150 bindings / dts even though I found a
> > reference to it in "gcc-sm8150.c".
> > - Fix node name in actual ipq8074 to be "clock-controller" (it's gcc).
> > - Since the example doesn't need phandes to exist, in msm8998 could
> > just make up places providing some of the clocks currently bogused
> > out with <0>.
> > - On msm8998 clocks are listed as required but current dts doesn't
> > have them.
> >
> > Fixes: ab91f72e018a ("clk: qcom: gcc-msm8996: Fix parent for CLKREF clocks")
> > Signed-off-by: Douglas Anderson <dianders@...omium.org>
> > Reviewed-by: Rob Herring <robh@...nel.org>
> > ---
> >
> > Changes in v2:
> > - Clocks are required for msm8998; note that current dts is broken.
> > - Drop description for 'gcc-apq8064' nvmem-cell-names.
> > - Commit message now describes running dt_binding_check differently.
> > - Added Rob's review tag.
> >
> > .../bindings/clock/qcom,gcc-apq8064.yaml | 79 +++++++
> > .../bindings/clock/qcom,gcc-ipq8074.yaml | 48 ++++
> > .../bindings/clock/qcom,gcc-msm8996.yaml | 65 ++++++
> > .../bindings/clock/qcom,gcc-msm8998.yaml | 90 ++++++++
> > .../bindings/clock/qcom,gcc-qcs404.yaml | 48 ++++
> > .../bindings/clock/qcom,gcc-sc7180.yaml | 72 ++++++
> > .../bindings/clock/qcom,gcc-sm8150.yaml | 69 ++++++
> > .../devicetree/bindings/clock/qcom,gcc.yaml | 212 ++----------------
> > 8 files changed, 489 insertions(+), 194 deletions(-)
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-ipq8074.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-msm8996.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-msm8998.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-qcs404.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-sc7180.yaml
> > create mode 100644 Documentation/devicetree/bindings/clock/qcom,gcc-sm8150.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> > new file mode 100644
> > index 000000000000..a386cfd27793
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc-apq8064.yaml
> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: GPL-2.0-only
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/bindings/clock/qcom,gcc-apq8064.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Global Clock & Reset Controller Binding for APQ8064
> > +
> > +maintainers:
> > + - Stephen Boyd <sboyd@...nel.org>
> > + - Taniya Das <tdas@...eaurora.org>
> > +
> > +description:
> > + Qualcomm global clock control module which supports the clocks, resets and
> > + power domains on APQ8064.
>
> It would be great if this could also point to the
> include/dt-bindings/clock/qcom,apq8064.h file here. If you don't resend
> this patch then I will try to remember to make this addition to the
> binding docs.
Funny that you should pick this example. There is no 'qcom,apq8064.h'
and it appears that this driver and device tree files use:
- dt-bindings/clock/qcom,gcc-msm8960.h
- dt-bindings/reset/qcom,gcc-msm8960.h
I'll point to those.
-Doug
Powered by blists - more mailing lists