| Message ID | 20200130211231.224656-1-dianders@chromium.org |
|---|---|
| Headers | show |
| Series | clk: qcom: Fix parenting for dispcc/gpucc/videocc | expand |
On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote: > > The qcom,dispcc bindings had a few problems with them: > > 1. They didn't specify all the clocks that dispcc is a client of. > Specifically on sc7180 there are two clocks from the DSI PHY and > two from the DP PHY. On sdm845 there are actually two DSI PHYs > (each of which has two clocks) and an extra clock from the gcc. > These all need to be specified. > > 2. The sdm845.dtsi has existed for quite some time without specifying > the clocks. The Linux driver was relying on global names to match > things up. While we should transition things, it should be noted > in the bindings. > > 3. The names used the bindings for "xo" and "gpll0" didn't match the > names that QC used for these clocks internally and this was causing > confusion / difficulty with their code generation tools. Switched > to the internal names to simplify everyone's lives. It's not quite > as clean in a purist sense but it should avoid headaches. This > officially changes the binding, but that seems OK in this case. > > Also note that I updated the example. > > Fixes: 5d28e44ba630 ("dt-bindings: clock: Add YAML schemas for the QCOM DISPCC clock bindings") > Signed-off-by: Douglas Anderson <dianders@chromium.org> > --- > > Changes in v3: > - Added include file to description. > - Discovered / added new gcc input clock on sdm845. > - Split sc7180 and sdm845 into two files. > - Switched names to internal QC names rather than logical ones. > - Updated commit description. > > Changes in v2: > - Patch ("dt-bindings: clock: Fix qcom,dispcc...") new for v2. > > .../bindings/clock/qcom,dispcc.yaml | 67 ------------- > .../bindings/clock/qcom,sc7180-dispcc.yaml | 84 ++++++++++++++++ > .../bindings/clock/qcom,sdm845-dispcc.yaml | 99 +++++++++++++++++++ > 3 files changed, 183 insertions(+), 67 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/clock/qcom,dispcc.yaml > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-dispcc.yaml > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-dispcc.yaml Other than the same $id problem, Reviewed-by: Rob Herring <robh@kernel.org>
Hi, On Fri, Jan 31, 2020 at 8:43 AM Rob Herring <robh@kernel.org> wrote: > > On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > 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> > > --- > > > > Changes in v3: > > - Added pointer to inlude file in description. > > - Everyone but msm8998 now uses internal QC names. > > - Fixed typo grpahics => graphics > > - Split bindings into 3 files. > > > > Changes in v2: > > - Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2. > > > > .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 ------------------- > > .../bindings/clock/qcom,msm8998-gpucc.yaml | 66 +++++++++++++++++ > > .../bindings/clock/qcom,sc7180-gpucc.yaml | 72 +++++++++++++++++++ > > .../bindings/clock/qcom,sdm845-gpucc.yaml | 72 +++++++++++++++++++ > > 4 files changed, 210 insertions(+), 72 deletions(-) > > delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml > > I'm not seeing any differences in sdm845 and sc7180. Do those really > need to be separate? It doesn't have to be all combined or all > separate. They are the same, other than pointing to a different #include file. I debated whether to put them in one file (arbitrarily named after one SoC or the other) or to put them in individual files. I got the impression from Stephen that he'd prefer them to be separate files even in the case that they were 99% identical, but I certainly could have misunderstood. I'll do whatever you guys agree to. If you want them in one file I'll probably name it "qcom,sdm845-gpucc.yaml" just because that SoC is earlier, unless someone tells me otherwise. -Doug
Hi, On Mon, Feb 3, 2020 at 8:29 AM Stephen Boyd <sboyd@kernel.org> wrote: > > Quoting Doug Anderson (2020-01-31 08:48:37) > > Hi, > > > > On Fri, Jan 31, 2020 at 8:43 AM Rob Herring <robh@kernel.org> wrote: > > > > > > On Thu, Jan 30, 2020 at 3:12 PM Douglas Anderson <dianders@chromium.org> wrote: > > > > > > > > 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> > > > > --- > > > > > > > > Changes in v3: > > > > - Added pointer to inlude file in description. > > > > - Everyone but msm8998 now uses internal QC names. > > > > - Fixed typo grpahics => graphics > > > > - Split bindings into 3 files. > > > > > > > > Changes in v2: > > > > - Patch ("dt-bindings: clock: Fix qcom,gpucc...") new for v2. > > > > > > > > .../devicetree/bindings/clock/qcom,gpucc.yaml | 72 ------------------- > > > > .../bindings/clock/qcom,msm8998-gpucc.yaml | 66 +++++++++++++++++ > > > > .../bindings/clock/qcom,sc7180-gpucc.yaml | 72 +++++++++++++++++++ > > > > .../bindings/clock/qcom,sdm845-gpucc.yaml | 72 +++++++++++++++++++ > > > > 4 files changed, 210 insertions(+), 72 deletions(-) > > > > delete mode 100644 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml > > > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,msm8998-gpucc.yaml > > > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sc7180-gpucc.yaml > > > > create mode 100644 Documentation/devicetree/bindings/clock/qcom,sdm845-gpucc.yaml > > > > > > I'm not seeing any differences in sdm845 and sc7180. Do those really > > > need to be separate? It doesn't have to be all combined or all > > > separate. > > > > They are the same, other than pointing to a different #include file. > > I debated whether to put them in one file (arbitrarily named after one > > SoC or the other) or to put them in individual files. I got the > > impression from Stephen that he'd prefer them to be separate files > > even in the case that they were 99% identical, but I certainly could > > have misunderstood. > > > > I'll do whatever you guys agree to. If you want them in one file I'll > > probably name it "qcom,sdm845-gpucc.yaml" just because that SoC is > > earlier, unless someone tells me otherwise. > > > > I'd prefer them to be split out and point at the include file so we know > what numbers are valid. It provides clarity and helps avoid the back and > forth of combining and splitting the files. We suffer the same problem > on the driver side, and we've long given up trying to combine SoCs when > they're otherwise fairly similar. Thanks for clarifying! Rob: I hope it's OK that I've gone ahead and sent out v4 leaving this alone. I knew you were interested in getting the other bindings patch out sooner rather than later and I was hoping to get both series out together so I could context switch to a few other things early this week. Apologies if this was moving too fast... -Doug
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. Compared to v2, 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. 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 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} (60%)