| Message ID | 20200124144154.v2.7.I3bf44e33f4dc7ecca10a50dbccb7dc082894fa59@changeid (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | clk: qcom: Fix parenting for dispcc/gpucc/videocc | expand |
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, > }, >
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
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, },
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(-)