| Message ID | 20200203183149.73842-1-dianders@chromium.org |
|---|---|
| Headers | show |
| Series | clk: qcom: Fix parenting for dispcc/gpucc/videocc | expand |
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
Quoting Douglas Anderson (2020-02-03 10:31:40) > The qcom,gpucc bindings had a few problems with them: > > 1. When things were converted to yaml the name of the "gpll0 main" > clock got changed from "gpll0" to "gpll0_main". Change it back for > msm8998. > > 2. Apparently there is a push not to use purist aliases for clocks but > instead to just use the internal Qualcomm names. For sdm845 and > sc7180 (where the drivers haven't already been changed) move in > this direction. > > Things were also getting complicated harder to deal with by jamming > several SoCs into one file. Splitting simplifies things. > > Fixes: 5c6f3a36b913 ("dt-bindings: clock: Add YAML schemas for the QCOM GPUCC clock bindings") > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Applied to clk-next
Quoting Douglas Anderson (2020-02-03 10:31:43) > It's nicer to use ARRAY_SIZE instead of hardcoding. Had we always > been doing this it would have prevented a previous bug. See commit > 74c31ff9c84a ("clk: qcom: gpu_cc_gmu_clk_src has 5 parents, not 6"). > > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Applied to clk-next
Quoting Douglas Anderson (2020-02-03 10:31:45) > The test clock isn't in the bindings and apparently it's not used by > anyone upstream. Remove it. > > Suggested-by: Stephen Boyd <swboyd@chromium.org> > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- Applied to clk-next
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.