[v4,00/15] clk: qcom: Fix parenting for dispcc/gpucc/videocc
mbox series

Message ID 20200203183149.73842-1-dianders@chromium.org
Headers show
Series
  • clk: qcom: Fix parenting for dispcc/gpucc/videocc
Related show

Message

Doug Anderson Feb. 3, 2020, 6:31 p.m. UTC
The aim of this series is to get the dispcc and gpucc in a workable
shape upstream for sc7180.  I personally wasn't focusing on (and
didn't test much) videocc but pulled it along for the ride.

Most of the work in this series deals with the fact that the parenting
info for these clock controllers was in a bad shape.  It looks like it
was half transitioned from the old way of doing things (relying on
global names) to the new way of doing things (putting the linkage in
the device tree).  This should fully transition us.

As part of this transition I update the sdm845.dtsi file to specify
the info as per the new way of doing things.  Although I've now put
the linkage info in the sdm845.dtsi file, though, I haven't updated
the sdm845 clock drivers in Linux so they still work via the global
name matching.  It's left as an exercise to the reader to update the
sdm845 clock drivers in Linux.

This series passes these things for me on linux-next (next-20200129)
after picking the recent gcc fix I posted [1]:

  for f in \
    Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sc7180-dispcc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sc7180-videocc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml \
    Documentation/devicetree/bindings/clock/qcom,sdm845-videocc.yaml; do \
        ARCH=arm64 make dtbs_check DT_SCHEMA_FILES=$f; \
    done

  I also tried this:
    # 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.

  I also tried this (make sure you don't run w/ -j64 or diff is hard):
    # Delete broken yaml:
    rm Documentation/devicetree/bindings/pci/intel-gw-pcie.yaml
    git checkout beforeMyCode
    ARCH=arm64 make dt_binding_check > old.txt 2>&1
    git checkout myCode
    ARCH=arm64 make dt_binding_check > new.txt 2>&1
    diff old.txt new.txt
  ...and that didn't seem to indicate problems.

I have confirmed that (with extra patches) the display/gpu come up on
sc7180 and sdm845-cheza.  You can find the top of my downstream tree at:
  https://crrev.com/c/2017976/4

I have confirmed that sdm845-cheza display / GPU come up atop
next-20200129, which is what this series is posted against.

From v2 to v3, this series has quite a few changes.  Mostly it's:
- Always split into multiple files (Stephen).
- Use internal names, not purist names (Taniya).
- I realized that I forgot to update the sc7180 video clock controller
  driver in v2.
- A few other misc cleanups / fixes, see each patch for details.

From v3 to v4 this just removes the "bindings/" from the schema ID and
adds Rob's tag to two of the bindings patches.  The third binding
patch didn't get Rob's Ack because he suggested combining two files
might be a good idea.  I'm assuming he'll be OK with leaving them
split since Stephen prefers it that way, but if I have to do a v5 I
can do that.

It feels like with this many patches there's very little chance I
didn't do something stupid like make a tpyo or a paste-o paste-o,
though I tried to cross-check as much as I could.  I apologize in
advance for the stupid things I did that I should have known better
about.

[1] https://lore.kernel.org/r/20200129152458.v2.1.I4452dc951d7556ede422835268742b25a18b356b@changeid

Changes in v4:
- (non-change): Didn't combine sdm845 & sc7180 gpucc as per Stephen.
- Added Rob's review tag.
- Fixed schema id to not have "bindings/" as per Rob.

Changes in v3:
- Add Matthias tag.
- Added include file to description.
- Added pointer to inlude file in description.
- Added videocc include file.
- Discovered / added new gcc input clock on sdm845.
- Everyone but msm8998 now uses internal QC names.
- Fixed typo grpahics => graphics
- Newly discovered gcc_disp_gpll0_div_clk_src added.
- Patch ("clk: qcom: Get rid of fallback...dispcc-sc7180") split out for v3.
- Patch ("clk: qcom: Get rid of the test...dispcc-sc7180") split out for v3.
- Patch ("clk: qcom: Get rid of the test...gpucc-sc7180") split out for v3.
- Patch ("clk: qcom: Get rid of the test...videocc-sc7180") new for v3.
- Patch ("clk: qcom: Use ARRAY_SIZE in dispcc-sc7180...") split out for v3.
- Patch ("clk: qcom: Use ARRAY_SIZE in gpucc-sc7180...") split out for v3.
- Patch ("clk: qcom: Use ARRAY_SIZE in videocc-sc7180...") new for v3.
- Split bindings into 3 files.
- Split sc7180 and sdm845 into two files.
- Split videocc bindings into 2 files.
- Switched names to internal QC names rather than logical ones.
- Unlike in v2, use internal name instead of purist name.
- Updated commit description.

Changes in v2:
- Added includes
- Changed various parent names to match bindings / driver
- Patch ("arm64: dts: qcom: sdm845: Add...dispcc") new for v2.
- Patch ("arm64: dts: qcom: sdm845: Add...gpucc") new for v2.
- Patch ("arm64: dts: qcom: sdm845: Add...videocc") new for v2.
- Patch ("clk: qcom: rcg2: Don't crash...") new for v2.
- Patch ("dt-bindings: clock: Cleanup qcom,videocc") new for v2.
- Patch ("dt-bindings: clock: Fix qcom,dispcc...") new for v2.
- Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2.

Douglas Anderson (14):
  clk: qcom: rcg2: Don't crash if our parent can't be found; return an
    error
  dt-bindings: clock: Fix qcom,dispcc bindings for sdm845/sc7180
  arm64: dts: qcom: sdm845: Add the missing clocks on the dispcc
  clk: qcom: Get rid of fallback global names for dispcc-sc7180
  clk: qcom: Get rid of the test clock for dispcc-sc7180
  clk: qcom: Use ARRAY_SIZE in dispcc-sc7180 for parent clocks
  dt-bindings: clock: Fix qcom,gpucc bindings for sdm845/sc7180/msm8998
  arm64: dts: qcom: sdm845: Add missing clocks / fix names on the gpucc
  clk: qcom: Get rid of the test clock for gpucc-sc7180
  clk: qcom: Use ARRAY_SIZE in gpucc-sc7180 for parent clocks
  dt-bindings: clock: Cleanup qcom,videocc bindings for sdm845/sc7180
  clk: qcom: Get rid of the test clock for videocc-sc7180
  clk: qcom: Use ARRAY_SIZE in videocc-sc7180 for parent clocks
  arm64: dts: qcom: sdm845: Add the missing clock on the videocc

Taniya Das (1):
  arm64: dts: sc7180: Add clock controller nodes

 .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 --------------
 ...om,dispcc.yaml => qcom,msm8998-gpucc.yaml} | 33 +++----
 .../bindings/clock/qcom,sc7180-dispcc.yaml    | 84 ++++++++++++++++
 .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 ++++++++++++++
 .../bindings/clock/qcom,sc7180-videocc.yaml   | 63 ++++++++++++
 .../bindings/clock/qcom,sdm845-dispcc.yaml    | 99 +++++++++++++++++++
 .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 ++++++++++++++
 ...,videocc.yaml => qcom,sdm845-videocc.yaml} | 27 ++---
 arch/arm64/boot/dts/qcom/sc7180.dtsi          | 47 +++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 28 +++++-
 drivers/clk/qcom/clk-rcg2.c                   |  3 +
 drivers/clk/qcom/dispcc-sc7180.c              | 45 +++------
 drivers/clk/qcom/gpucc-sc7180.c               |  4 +-
 drivers/clk/qcom/videocc-sc7180.c             |  4 +-
 14 files changed, 513 insertions(+), 140 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
 rename Documentation/devicetree/bindings/clock/{qcom,dispcc.yaml => qcom,msm8998-gpucc.yaml} (51%)
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-dispcc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-videocc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml
 create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml
 rename Documentation/devicetree/bindings/clock/{qcom,videocc.yaml => qcom,sdm845-videocc.yaml} (61%)

Comments

Stephen Boyd Feb. 3, 2020, 7:30 p.m. UTC | #1
Quoting Douglas Anderson (2020-02-03 10:31:33)
> 
>  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 --------------
>  ...om,dispcc.yaml => qcom,msm8998-gpucc.yaml} | 33 +++----
>  .../bindings/clock/qcom,sc7180-dispcc.yaml    | 84 ++++++++++++++++
>  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 ++++++++++++++
>  .../bindings/clock/qcom,sc7180-videocc.yaml   | 63 ++++++++++++
>  .../bindings/clock/qcom,sdm845-dispcc.yaml    | 99 +++++++++++++++++++
>  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 ++++++++++++++
>  ...,videocc.yaml => qcom,sdm845-videocc.yaml} | 27 ++---
>  arch/arm64/boot/dts/qcom/sc7180.dtsi          | 47 +++++++++
>  arch/arm64/boot/dts/qcom/sdm845.dtsi          | 28 +++++-

I don't want to take patches touching dts/qcom/. These aren't necessary
to merge right now, correct? Or at least, they can go via arm-soc tree?
Doug Anderson Feb. 3, 2020, 7:41 p.m. UTC | #2
Hi,

On Mon, Feb 3, 2020 at 11:30 AM Stephen Boyd <sboyd@kernel.org> wrote:
>
> Quoting Douglas Anderson (2020-02-03 10:31:33)
> >
> >  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 --------------
> >  ...om,dispcc.yaml => qcom,msm8998-gpucc.yaml} | 33 +++----
> >  .../bindings/clock/qcom,sc7180-dispcc.yaml    | 84 ++++++++++++++++
> >  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 ++++++++++++++
> >  .../bindings/clock/qcom,sc7180-videocc.yaml   | 63 ++++++++++++
> >  .../bindings/clock/qcom,sdm845-dispcc.yaml    | 99 +++++++++++++++++++
> >  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 ++++++++++++++
> >  ...,videocc.yaml => qcom,sdm845-videocc.yaml} | 27 ++---
> >  arch/arm64/boot/dts/qcom/sc7180.dtsi          | 47 +++++++++
> >  arch/arm64/boot/dts/qcom/sdm845.dtsi          | 28 +++++-
>
> I don't want to take patches touching dts/qcom/. These aren't necessary
> to merge right now, correct? Or at least, they can go via arm-soc tree?

Right.  They can go later.

Specifically for sdm845 until the sdm845 patches lands the old dts
trees will yell about the missing clocks, but it's not like they will
compile fail.  Also the bindings themselves will validate fine.
There's no other way forward, though, and the old bindings caused
similar yells.

For sc7180 there's no usage of any of these clocks and this adds the
first usage, so definitely no problem there.

Once you've landed then Bjorn or Andy can pick up the dts.

-Doug
Bjorn Andersson Feb. 3, 2020, 8:04 p.m. UTC | #3
On Mon 03 Feb 11:41 PST 2020, Doug Anderson wrote:

> Hi,
> 
> On Mon, Feb 3, 2020 at 11:30 AM Stephen Boyd <sboyd@kernel.org> wrote:
> >
> > Quoting Douglas Anderson (2020-02-03 10:31:33)
> > >
> > >  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 --------------
> > >  ...om,dispcc.yaml => qcom,msm8998-gpucc.yaml} | 33 +++----
> > >  .../bindings/clock/qcom,sc7180-dispcc.yaml    | 84 ++++++++++++++++
> > >  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 ++++++++++++++
> > >  .../bindings/clock/qcom,sc7180-videocc.yaml   | 63 ++++++++++++
> > >  .../bindings/clock/qcom,sdm845-dispcc.yaml    | 99 +++++++++++++++++++
> > >  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 ++++++++++++++
> > >  ...,videocc.yaml => qcom,sdm845-videocc.yaml} | 27 ++---
> > >  arch/arm64/boot/dts/qcom/sc7180.dtsi          | 47 +++++++++
> > >  arch/arm64/boot/dts/qcom/sdm845.dtsi          | 28 +++++-
> >
> > I don't want to take patches touching dts/qcom/. These aren't necessary
> > to merge right now, correct? Or at least, they can go via arm-soc tree?
> 
> Right.  They can go later.
> 
> Specifically for sdm845 until the sdm845 patches lands the old dts
> trees will yell about the missing clocks, but it's not like they will
> compile fail.  Also the bindings themselves will validate fine.
> There's no other way forward, though, and the old bindings caused
> similar yells.
> 

Can you please help me parse this, will old DT cause complaints or will
it fail to boot?

> For sc7180 there's no usage of any of these clocks and this adds the
> first usage, so definitely no problem there.
> 
> Once you've landed then Bjorn or Andy can pick up the dts.
> 

Do I need to apply these after Stephen picks the driver patches? Or are
they simply nop until the driver patches lands?

Regards,
Bjorn
Doug Anderson Feb. 3, 2020, 8:48 p.m. UTC | #4
Hi,

On Mon, Feb 3, 2020 at 12:04 PM Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
>
> On Mon 03 Feb 11:41 PST 2020, Doug Anderson wrote:
>
> > Hi,
> >
> > On Mon, Feb 3, 2020 at 11:30 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > >
> > > Quoting Douglas Anderson (2020-02-03 10:31:33)
> > > >
> > > >  .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 --------------
> > > >  ...om,dispcc.yaml => qcom,msm8998-gpucc.yaml} | 33 +++----
> > > >  .../bindings/clock/qcom,sc7180-dispcc.yaml    | 84 ++++++++++++++++
> > > >  .../bindings/clock/qcom,sc7180-gpucc.yaml     | 72 ++++++++++++++
> > > >  .../bindings/clock/qcom,sc7180-videocc.yaml   | 63 ++++++++++++
> > > >  .../bindings/clock/qcom,sdm845-dispcc.yaml    | 99 +++++++++++++++++++
> > > >  .../bindings/clock/qcom,sdm845-gpucc.yaml     | 72 ++++++++++++++
> > > >  ...,videocc.yaml => qcom,sdm845-videocc.yaml} | 27 ++---
> > > >  arch/arm64/boot/dts/qcom/sc7180.dtsi          | 47 +++++++++
> > > >  arch/arm64/boot/dts/qcom/sdm845.dtsi          | 28 +++++-
> > >
> > > I don't want to take patches touching dts/qcom/. These aren't necessary
> > > to merge right now, correct? Or at least, they can go via arm-soc tree?
> >
> > Right.  They can go later.
> >
> > Specifically for sdm845 until the sdm845 patches lands the old dts
> > trees will yell about the missing clocks, but it's not like they will
> > compile fail.  Also the bindings themselves will validate fine.
> > There's no other way forward, though, and the old bindings caused
> > similar yells.
> >
>
> Can you please help me parse this, will old DT cause complaints or will
> it fail to boot?

Sorry, let me try to be more clear:

a) Without my series: "make dtbs_check" limited to the bindings files
I'm touching yells.  Examples:

.../msm8998-mtp.dt.yaml: clock-controller@5065000: clock-names:1:
'gpll0_main' was expected

.../sdm845-mtp.dt.yaml: clock-controller@af00000: 'clocks' is a
required property
.../sdm845-mtp.dt.yaml: clock-controller@af00000: 'clock-names' is a
required property

.../sdm845-mtp.dt.yaml: clock-controller@ab00000: 'clocks' is a
required property
.../sdm845-mtp.dt.yaml: clock-controller@ab00000: 'clock-names' is a
required property


b) With just the bindings from my series, "make dtbs_check" yells, but
yells about different things.  The "gpll0_main" one goes away but this
one is new:

.../sdm845-mtp.dt.yaml: clock-controller@5090000: clock-names:0:
'bi_tcxo' was expected
.../sdm845-mtp.dt.yaml: clock-controller@5090000: clock-names: ['xo']
is too short
.../sdm845-mtp.dt.yaml: clock-controller@5090000: clocks: [[26, 0]] is too short


c) With just the "dts" from my series, you'll again get different
yells from "make dtbs_check".  I won't bother listing them, but they
are similar to the above.


d) With everything from my series, the "make dtbs_check" limited to
the bindings files I'm touching is clean.

---

* sdm845's ability to boot is unaffected by this series.  I have
tested booting sdm845 after this series and it's fine.  Since nothing
in this series touches the sdm845 clock drivers (only the bindings and
the dts) that means that the new dts must work with the existing
drivers.  You can land the sdm845 dts any time after Stephen and Rob
are happy with the bindings.

* It should be fine to land the sc7180 dts file without waiting for
the driver change.  The dts here will work with both the
dispcc/gpucc/videocc that's in clk-next and also the one that results
from applying all of my patches.


> > For sc7180 there's no usage of any of these clocks and this adds the
> > first usage, so definitely no problem there.
> >
> > Once you've landed then Bjorn or Andy can pick up the dts.
> >
>
> Do I need to apply these after Stephen picks the driver patches? Or are
> they simply nop until the driver patches lands?

The sdm845 patches are a nop until some future patch changes the Linux
driver to start using them.  I don't know of anyone planning to do
that.  The only real result from an sdm845 perspective will be making
things "more correct" from a dts standpoint and keeping "make
dtbs_check" happier.

The sc7180 patches are OK to land even without the driver.  They're
not a nop and I've actually validated that I can bring the display/gpu
up with them (even without the driver changes) on my downstream
kernel.

---

Sorry it's so confusing.  Happy to try to clarify more if the above is
still too hard to follow.

-Doug
Bjorn Andersson Feb. 3, 2020, 11:17 p.m. UTC | #5
On Mon 03 Feb 12:48 PST 2020, Doug Anderson wrote:
[..]
> Sorry it's so confusing.  Happy to try to clarify more if the above is
> still too hard to follow.
> 

Thanks for the clarification! I will pick up the dts patches

Regards,
Bjorn