mbox series

[v2,00/10] clk: qcom: Fix parenting for dispcc/gpucc/videocc

Message ID 20200124224225.22547-1-dianders@chromium.org
Headers show
Series clk: qcom: Fix parenting for dispcc/gpucc/videocc | expand

Message

Doug Anderson Jan. 24, 2020, 10:42 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) 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:

  ARCH=arm64 make dtbs_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
  ARCH=arm64 make dtbs_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,dispcc.yaml
  ARCH=arm64 make dtbs_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,videocc.yaml
  ARCH=arm64 make dt_binding_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,videocc.yaml
  ARCH=arm64 make dt_binding_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
  ARCH=arm64 make dt_binding_check \
    DT_SCHEMA_FILES=Documentation/devicetree/bindings/clock/qcom,dispcc.yaml

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/3

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

This series is marked as 'v2' because in it I have snarfed up Taniya's
dts patch adding the clock controller nodes to sc7180.dtsi and this is
"v2" of that patch.  Everything else is brand new.

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

Douglas Anderson (9):
  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
  dt-bindings: clock: Fix qcom,gpucc bindings for sdm845/sc7180/msm8998
  clk: qcom: Fix sc7180 dispcc parent data
  arm64: dts: qcom: sdm845: Add the missing clocks on the gpucc
  clk: qcom: Fix sc7180 gpucc parent data
  dt-bindings: clock: Cleanup qcom,videocc bindings for sdm845/sc7180
  arm64: dts: qcom: sdm845: Add the missing clock on the videocc

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

 .../bindings/clock/qcom,dispcc.yaml           | 87 +++++++++++++++----
 .../devicetree/bindings/clock/qcom,gpucc.yaml | 42 ++++++---
 .../bindings/clock/qcom,videocc.yaml          | 10 ++-
 arch/arm64/boot/dts/qcom/sc7180.dtsi          | 41 +++++++++
 arch/arm64/boot/dts/qcom/sdm845.dtsi          | 20 ++++-
 drivers/clk/qcom/clk-rcg2.c                   |  3 +
 drivers/clk/qcom/dispcc-sc7180.c              | 63 +++++---------
 drivers/clk/qcom/gpucc-sc7180.c               | 11 ++-
 8 files changed, 199 insertions(+), 78 deletions(-)

Comments

Taniya Das Jan. 28, 2020, 5:55 a.m. UTC | #1
Hi Doug,

Thanks for your patch. But as mentioned earlier we really want to avoid 
updating the auto generated code.

On 1/25/2020 4:12 AM, Douglas Anderson wrote:
> The bindings file (qcom,gpucc.yaml) does not agree with the names we
> use for input clocks.  Fix us.  This takes into account the changes in
> the recent patch ("dt-bindings: clock: Fix qcom,gpucc bindings for
> sdm845/sc7180/msm8998"), but even without that patch the names in the
> driver were still not right.
> 
> Since we didn't add the "test" clock to the bindings (apparently it's
> never used), kill it from the driver.  If someone has a use for it we
> should add it to the bindings and bring it back.
> 
> Instead of updating the size of the array now that the test clock is
> gone, switch to using the less error-prone ARRAY_SIZE.  Not sure why
> it didn't always use that.
> 
> Fixes: 745ff069a49c ("clk: qcom: Add graphics clock controller driver for SC7180")
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
> 
> Changes in v2:
> - Patch ("clk: qcom: Fix sc7180 gpucc parent data") new for v2.
> 
>   drivers/clk/qcom/gpucc-sc7180.c | 11 +++++------
>   1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/clk/qcom/gpucc-sc7180.c b/drivers/clk/qcom/gpucc-sc7180.c
> index ec61194cceaf..da56506036e2 100644
> --- a/drivers/clk/qcom/gpucc-sc7180.c
> +++ b/drivers/clk/qcom/gpucc-sc7180.c
> @@ -47,7 +47,7 @@ static struct clk_alpha_pll gpu_cc_pll1 = {
>   		.hw.init = &(struct clk_init_data){
>   			.name = "gpu_cc_pll1",
>   			.parent_data =  &(const struct clk_parent_data){
> -				.fw_name = "bi_tcxo",
> +				.fw_name = "xo",
>   			},
>   			.num_parents = 1,
>   			.ops = &clk_alpha_pll_fabia_ops,
> @@ -64,11 +64,10 @@ static const struct parent_map gpu_cc_parent_map_0[] = {
>   };
>   
>   static const struct clk_parent_data gpu_cc_parent_data_0[] = {
> -	{ .fw_name = "bi_tcxo" },
> +	{ .fw_name = "xo" },
>   	{ .hw = &gpu_cc_pll1.clkr.hw },
> -	{ .fw_name = "gcc_gpu_gpll0_clk_src" },
> -	{ .fw_name = "gcc_gpu_gpll0_div_clk_src" },
> -	{ .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" },
> +	{ .fw_name = "gpll0" },
> +	{ .fw_name = "gpll0_div" },
>   };
>   
>   static const struct freq_tbl ftbl_gpu_cc_gmu_clk_src[] = {
> @@ -86,7 +85,7 @@ static struct clk_rcg2 gpu_cc_gmu_clk_src = {
>   	.clkr.hw.init = &(struct clk_init_data){
>   		.name = "gpu_cc_gmu_clk_src",
>   		.parent_data = gpu_cc_parent_data_0,
> -		.num_parents = 5,
> +		.num_parents = ARRAY_SIZE(gpu_cc_parent_data_0),
>   		.flags = CLK_SET_RATE_PARENT,
>   		.ops = &clk_rcg2_shared_ops,
>   	},
>
Doug Anderson Jan. 28, 2020, 4:37 p.m. UTC | #2
Hi,

On Mon, Jan 27, 2020 at 9:55 PM Taniya Das <tdas@codeaurora.org> wrote:
>
> Hi Doug,
>
> Thanks for your patch. But as mentioned earlier we really want to avoid
> updating the auto generated code.

As per my reply [1], I think you need to update your auto-generation
tools to generate the code that results from my patch.  The existing
code either requires using global clock names to match or requires to
pass clocks in the dts that aren't documented in the bindings.  That
needs to be fixed and my patch fixes it.

[1] https://lore.kernel.org/r/CAD=FV=XFFCPj8S7-WPjPLFe=iygpkYiyMqbneY0DMXsMz+j73w@mail.gmail.com

-Doug