Re: [PATCH v3 7/7] clk: qcom: Add display clock controller driver for SM8250

[Jonathan Marek <jonathan@xxxxxxxx>]

On 9/22/20 3:00 PM, Stephen Boyd wrote:
> Quoting Jonathan Marek (2020-09-11 08:34:07)
> > diff --git a/drivers/clk/qcom/dispcc-sm8250.c b/drivers/clk/qcom/dispcc-sm8250.c
> > new file mode 100644
> > index 000000000000..7c0f384a3a42
> > --- /dev/null
> > +++ b/drivers/clk/qcom/dispcc-sm8250.c
> > @@ -0,0 +1,1100 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018-2020, The Linux Foundation. All rights reserved.
> > + */
> > +
> [...]
> > +
> > +static const struct clk_parent_data disp_cc_parent_data_6[] = {
> > +       { .fw_name = "bi_tcxo" },
> > +       { .fw_name = "dsi0_phy_pll_out_dsiclk" },
> > +       { .fw_name = "dsi1_phy_pll_out_dsiclk" },
>
> Can we remove clk postfix on these clk names?

This is consistent with the names used in both sdm845 and sc7180 
drivers. If this should change then those should be changed too?

> > +};
> > +
> > +static const struct freq_tbl ftbl_disp_cc_mdss_ahb_clk_src[] = {
> > +       F(19200000, P_BI_TCXO, 1, 0, 0),
> > +       F(37500000, P_DISP_CC_PLL1_OUT_MAIN, 16, 0, 0),
> > +       F(75000000, P_DISP_CC_PLL1_OUT_MAIN, 8, 0, 0),
> > +       { }
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_ahb_clk_src = {
> > +       .cmd_rcgr = 0x22bc,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_3,
> > +       .freq_tbl = ftbl_disp_cc_mdss_ahb_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_ahb_clk_src",
> > +               .parent_data = disp_cc_parent_data_3,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_3),
> > +               .flags = CLK_SET_RATE_PARENT,
> > +               .ops = &clk_rcg2_shared_ops,
> > +       },
> > +};
> > +
> > +static const struct freq_tbl ftbl_disp_cc_mdss_byte0_clk_src[] = {
> > +       F(19200000, P_BI_TCXO, 1, 0, 0),
> > +       { }
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_byte0_clk_src = {
> > +       .cmd_rcgr = 0x2110,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_2,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_byte0_clk_src",
> > +               .parent_data = disp_cc_parent_data_2,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
>
> Why do we need CLK_GET_RATE_NOCACHE? Please remove it.
>
> > +               .ops = &clk_byte2_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_byte1_clk_src = {
> > +       .cmd_rcgr = 0x212c,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_2,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_byte1_clk_src",
> > +               .parent_data = disp_cc_parent_data_2,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_2),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > +               .ops = &clk_byte2_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_aux1_clk_src = {
> > +       .cmd_rcgr = 0x2240,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_1,
> > +       .freq_tbl = ftbl_disp_cc_mdss_byte0_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_aux1_clk_src",
> > +               .parent_data = disp_cc_parent_data_1,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> > +               .flags = CLK_SET_RATE_PARENT,
> > +               .ops = &clk_rcg2_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_aux_clk_src = {
> > +       .cmd_rcgr = 0x21dc,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_1,
> > +       .freq_tbl = ftbl_disp_cc_mdss_byte0_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_aux_clk_src",
> > +               .parent_data = disp_cc_parent_data_1,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_1),
> > +               .flags = CLK_SET_RATE_PARENT,
> > +               .ops = &clk_rcg2_ops,
> > +       },
> > +};
> > +
> > +static const struct freq_tbl ftbl_disp_cc_mdss_dp_link1_clk_src[] = {
> > +       F(162000000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> > +       F(270000000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> > +       F(540000000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> > +       F(810000000, P_DP_PHY_PLL_LINK_CLK, 1, 0, 0),
> > +       { }
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_link1_clk_src = {
> > +       .cmd_rcgr = 0x220c,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_0,
> > +       .freq_tbl = ftbl_disp_cc_mdss_dp_link1_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_link1_clk_src",
> > +               .parent_data = disp_cc_parent_data_0,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > +               .ops = &clk_rcg2_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_link_clk_src = {
> > +       .cmd_rcgr = 0x2178,
> > +       .mnd_width = 0,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_0,
> > +       .freq_tbl = ftbl_disp_cc_mdss_dp_link1_clk_src,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_link_clk_src",
> > +               .parent_data = disp_cc_parent_data_0,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > +               .ops = &clk_rcg2_ops,
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_pixel1_clk_src = {
> > +       .cmd_rcgr = 0x21c4,
> > +       .mnd_width = 16,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_0,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_pixel1_clk_src",
> > +               .parent_data = disp_cc_parent_data_0,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > +               .ops = &clk_dp_ops,
>
> This is affected by the patch I just applied 355a7d754b92 ("clk: qcom:
> dispcc: Update DP clk ops for phy design"). Please resend.
>
> > +       },
> > +};
> > +
> > +static struct clk_rcg2 disp_cc_mdss_dp_pixel2_clk_src = {
> > +       .cmd_rcgr = 0x21f4,
> > +       .mnd_width = 16,
> > +       .hid_width = 5,
> > +       .parent_map = disp_cc_parent_map_0,
> > +       .clkr.hw.init = &(struct clk_init_data){
> > +               .name = "disp_cc_mdss_dp_pixel2_clk_src",
> > +               .parent_data = disp_cc_parent_data_0,
> > +               .num_parents = ARRAY_SIZE(disp_cc_parent_data_0),
> > +               .flags = CLK_SET_RATE_PARENT | CLK_GET_RATE_NOCACHE,
> > +               .ops = &clk_dp_ops,
> > +       },
> > +};
> > +