* [PATCH v3 0/3] clk: qcom: Add support for SM8150 GCC @ 2019-06-25 6:31 Vinod Koul 2019-06-25 6:31 ` [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks Vinod Koul ` (2 more replies) 0 siblings, 3 replies; 13+ messages in thread From: Vinod Koul @ 2019-06-25 6:31 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das This series adds suport for SM8150 GCC which requires Trion PLLs and adds that as well. Also fixed some minor nits seen in clk-alpha-pll code and module alias for qcs404 gcc driver. Changes in v3: - Drop the cast for DIV_ROUND_UP_ULL as that need s afix in macro, so patch it up once core change in merged - Drop module alias patch and drop it from sm8150 driver - Add review tag by Bjorn Deepak Katragadda (2): clk: qcom: clk-alpha-pll: Add support for Trion PLLs clk: qcom: gcc: Add global clock controller driver for SM8150 Vinod Koul (1): clk: qcom: clk-alpha-pll: Remove post_div_table checks .../devicetree/bindings/clock/qcom,gcc.txt | 1 + drivers/clk/qcom/Kconfig | 7 + drivers/clk/qcom/Makefile | 1 + drivers/clk/qcom/clk-alpha-pll.c | 243 +- drivers/clk/qcom/clk-alpha-pll.h | 7 + drivers/clk/qcom/gcc-sm8150.c | 3719 +++++++++++++++++ include/dt-bindings/clock/qcom,gcc-sm8150.h | 243 ++ 7 files changed, 4206 insertions(+), 15 deletions(-) create mode 100644 drivers/clk/qcom/gcc-sm8150.c create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8150.h -- 2.20.1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks 2019-06-25 6:31 [PATCH v3 0/3] clk: qcom: Add support for SM8150 GCC Vinod Koul @ 2019-06-25 6:31 ` Vinod Koul 2019-06-27 21:37 ` Stephen Boyd 2019-07-16 0:08 ` Bjorn Andersson 2019-06-25 6:31 ` [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs Vinod Koul [not found] ` <20190625063140.17106-4-vkoul@kernel.org> 2 siblings, 2 replies; 13+ messages in thread From: Vinod Koul @ 2019-06-25 6:31 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das We want users to code properly and fix the post_div_table missing and not reply on core to check. So remove the post_div_table check. Signed-off-by: Vinod Koul <vkoul@kernel.org> Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/clk/qcom/clk-alpha-pll.c | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index b48707693ffd..2c6773188761 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -1036,11 +1036,6 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, u32 i, div = 1, val; int ret; - if (!pll->post_div_table) { - pr_err("Missing the post_div_table for the PLL\n"); - return -EINVAL; - } - ret = regmap_read(pll->clkr.regmap, PLL_USER_CTL(pll), &val); if (ret) return ret; @@ -1063,11 +1058,6 @@ static long clk_alpha_pll_postdiv_fabia_round_rate(struct clk_hw *hw, { struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); - if (!pll->post_div_table) { - pr_err("Missing the post_div_table for the PLL\n"); - return -EINVAL; - } - return divider_round_rate(hw, rate, prate, pll->post_div_table, pll->width, CLK_DIVIDER_ROUND_CLOSEST); } @@ -1089,11 +1079,6 @@ static int clk_alpha_pll_postdiv_fabia_set_rate(struct clk_hw *hw, if (val & PLL_VOTE_FSM_ENA) return 0; - if (!pll->post_div_table) { - pr_err("Missing the post_div_table for the PLL\n"); - return -EINVAL; - } - div = DIV_ROUND_UP_ULL(parent_rate, rate); for (i = 0; i < pll->num_post_div; i++) { if (pll->post_div_table[i].div == div) { -- 2.20.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks 2019-06-25 6:31 ` [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks Vinod Koul @ 2019-06-27 21:37 ` Stephen Boyd 2019-06-29 11:14 ` Vinod Koul 2019-07-16 0:08 ` Bjorn Andersson 1 sibling, 1 reply; 13+ messages in thread From: Stephen Boyd @ 2019-06-27 21:37 UTC (permalink / raw) To: Vinod Koul Cc: linux-arm-msm, Bjorn Andersson, Vinod Koul, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das Quoting Vinod Koul (2019-06-24 23:31:38) > We want users to code properly and fix the post_div_table missing and > not reply on core to check. So remove the post_div_table check. > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- This doesn't apply. Not sure why. Can you please format-patch with --base= so I can know what baseline commit you've based your patches on? Helps me avoid needing to ask, like right now. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks 2019-06-27 21:37 ` Stephen Boyd @ 2019-06-29 11:14 ` Vinod Koul 0 siblings, 0 replies; 13+ messages in thread From: Vinod Koul @ 2019-06-29 11:14 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On 27-06-19, 14:37, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-24 23:31:38) > > We want users to code properly and fix the post_div_table missing and > > not reply on core to check. So remove the post_div_table check. > > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > --- > > This doesn't apply. Not sure why. Can you please format-patch with I had rebased it on clk-next, but that was v2, let me rebase > --base= so I can know what baseline commit you've based your patches on? > Helps me avoid needing to ask, like right now. Sure will do that and repost after rebase. Also the patch fix for DIV_ROUND_DOWN_ULL has been merged to mm tree, will add that as well. Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks 2019-06-25 6:31 ` [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks Vinod Koul 2019-06-27 21:37 ` Stephen Boyd @ 2019-07-16 0:08 ` Bjorn Andersson 2019-07-16 4:08 ` Vinod Koul 1 sibling, 1 reply; 13+ messages in thread From: Bjorn Andersson @ 2019-07-16 0:08 UTC (permalink / raw) To: Vinod Koul Cc: Stephen Boyd, linux-arm-msm, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On Mon 24 Jun 23:31 PDT 2019, Vinod Koul wrote: > We want users to code properly and fix the post_div_table missing and > not reply on core to check. So remove the post_div_table check. s/reply/rely/ > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org> > --- > drivers/clk/qcom/clk-alpha-pll.c | 15 --------------- > 1 file changed, 15 deletions(-) > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index b48707693ffd..2c6773188761 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -1036,11 +1036,6 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, > u32 i, div = 1, val; > int ret; > > - if (!pll->post_div_table) { > - pr_err("Missing the post_div_table for the PLL\n"); > - return -EINVAL; > - } > - > ret = regmap_read(pll->clkr.regmap, PLL_USER_CTL(pll), &val); > if (ret) > return ret; > @@ -1063,11 +1058,6 @@ static long clk_alpha_pll_postdiv_fabia_round_rate(struct clk_hw *hw, > { > struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > > - if (!pll->post_div_table) { > - pr_err("Missing the post_div_table for the PLL\n"); > - return -EINVAL; > - } > - > return divider_round_rate(hw, rate, prate, pll->post_div_table, > pll->width, CLK_DIVIDER_ROUND_CLOSEST); > } > @@ -1089,11 +1079,6 @@ static int clk_alpha_pll_postdiv_fabia_set_rate(struct clk_hw *hw, > if (val & PLL_VOTE_FSM_ENA) > return 0; > > - if (!pll->post_div_table) { > - pr_err("Missing the post_div_table for the PLL\n"); > - return -EINVAL; > - } > - > div = DIV_ROUND_UP_ULL(parent_rate, rate); > for (i = 0; i < pll->num_post_div; i++) { > if (pll->post_div_table[i].div == div) { > -- > 2.20.1 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks 2019-07-16 0:08 ` Bjorn Andersson @ 2019-07-16 4:08 ` Vinod Koul 0 siblings, 0 replies; 13+ messages in thread From: Vinod Koul @ 2019-07-16 4:08 UTC (permalink / raw) To: Bjorn Andersson Cc: Stephen Boyd, linux-arm-msm, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On 15-07-19, 17:08, Bjorn Andersson wrote: > On Mon 24 Jun 23:31 PDT 2019, Vinod Koul wrote: > > > We want users to code properly and fix the post_div_table missing and > > not reply on core to check. So remove the post_div_table check. > > s/reply/rely/ Thanks for pointing, will update -- ~Vinod ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs 2019-06-25 6:31 [PATCH v3 0/3] clk: qcom: Add support for SM8150 GCC Vinod Koul 2019-06-25 6:31 ` [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks Vinod Koul @ 2019-06-25 6:31 ` Vinod Koul 2019-06-27 21:50 ` Stephen Boyd [not found] ` <20190625063140.17106-4-vkoul@kernel.org> 2 siblings, 1 reply; 13+ messages in thread From: Vinod Koul @ 2019-06-25 6:31 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das, Vinod Koul From: Deepak Katragadda <dkatraga@codeaurora.org> Add programming sequence support for managing the Trion PLLs. Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> Signed-off-by: Taniya Das <tdas@codeaurora.org> [vkoul: port to upstream and tidy-up] Signed-off-by: Vinod Koul <vkoul@kernel.org> --- drivers/clk/qcom/clk-alpha-pll.c | 228 +++++++++++++++++++++++++++++++ drivers/clk/qcom/clk-alpha-pll.h | 7 + 2 files changed, 235 insertions(+) diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 2c6773188761..30210f5c6726 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -32,6 +32,7 @@ # define PLL_LOCK_DET BIT(31) #define PLL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_L_VAL]) +#define PLL_CAL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_L_VAL]) #define PLL_ALPHA_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL]) #define PLL_ALPHA_VAL_U(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL_U]) @@ -44,14 +45,17 @@ # define PLL_VCO_MASK 0x3 #define PLL_USER_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U]) +#define PLL_USER_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U1]) #define PLL_CONFIG_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL]) #define PLL_CONFIG_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U]) +#define PLL_CONFIG_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U11]) #define PLL_TEST_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL]) #define PLL_TEST_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL_U]) #define PLL_STATUS(p) ((p)->offset + (p)->regs[PLL_OFF_STATUS]) #define PLL_OPMODE(p) ((p)->offset + (p)->regs[PLL_OFF_OPMODE]) #define PLL_FRAC(p) ((p)->offset + (p)->regs[PLL_OFF_FRAC]) +#define PLL_CAL_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_VAL]) const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { [CLK_ALPHA_PLL_TYPE_DEFAULT] = { @@ -96,6 +100,22 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { [PLL_OFF_OPMODE] = 0x2c, [PLL_OFF_FRAC] = 0x38, }, + [CLK_ALPHA_PLL_TYPE_TRION] = { + [PLL_OFF_L_VAL] = 0x04, + [PLL_OFF_CAL_L_VAL] = 0x08, + [PLL_OFF_USER_CTL] = 0x0c, + [PLL_OFF_USER_CTL_U] = 0x10, + [PLL_OFF_USER_CTL_U1] = 0x14, + [PLL_OFF_CONFIG_CTL] = 0x18, + [PLL_OFF_CONFIG_CTL_U] = 0x1c, + [PLL_OFF_CONFIG_CTL_U1] = 0x20, + [PLL_OFF_TEST_CTL] = 0x24, + [PLL_OFF_TEST_CTL_U] = 0x28, + [PLL_OFF_STATUS] = 0x30, + [PLL_OFF_OPMODE] = 0x38, + [PLL_OFF_ALPHA_VAL] = 0x40, + [PLL_OFF_CAL_VAL] = 0x44, + }, }; EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); @@ -120,6 +140,10 @@ EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); #define FABIA_PLL_OUT_MASK 0x7 #define FABIA_PLL_RATE_MARGIN 500 +#define TRION_PLL_STANDBY 0x0 +#define TRION_PLL_RUN 0x1 +#define TRION_PLL_OUT_MASK 0x7 + #define pll_alpha_width(p) \ ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \ ALPHA_REG_BITWIDTH : ALPHA_REG_16BIT_WIDTH) @@ -730,6 +754,130 @@ static long alpha_pll_huayra_round_rate(struct clk_hw *hw, unsigned long rate, return alpha_huayra_pll_round_rate(rate, *prate, &l, &a); } +static int trion_pll_is_enabled(struct clk_alpha_pll *pll, + struct regmap *regmap) +{ + u32 mode_regval, opmode_regval; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &mode_regval); + ret |= regmap_read(regmap, PLL_OPMODE(pll), &opmode_regval); + if (ret) + return 0; + + return ((opmode_regval & TRION_PLL_RUN) && (mode_regval & PLL_OUTCTRL)); +} + +static int clk_trion_pll_is_enabled(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + + return trion_pll_is_enabled(pll, pll->clkr.regmap); +} + +static int clk_trion_pll_enable(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 val; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return ret; + + /* If in FSM mode, just vote for it */ + if (val & PLL_VOTE_FSM_ENA) { + ret = clk_enable_regmap(hw); + if (ret) + return ret; + return wait_for_pll_enable_active(pll); + } + + /* Set operation mode to RUN */ + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_RUN); + + ret = wait_for_pll_enable_lock(pll); + if (ret) + return ret; + + /* Enable the PLL outputs */ + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), + TRION_PLL_OUT_MASK, TRION_PLL_OUT_MASK); + if (ret) + return ret; + + /* Enable the global PLL outputs */ + return regmap_update_bits(regmap, PLL_MODE(pll), + PLL_OUTCTRL, PLL_OUTCTRL); +} + +static void clk_trion_pll_disable(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 val; + int ret; + + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return; + + /* If in FSM mode, just unvote it */ + if (val & PLL_VOTE_FSM_ENA) { + clk_disable_regmap(hw); + return; + } + + /* Disable the global PLL output */ + ret = regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); + if (ret) + return; + + /* Disable the PLL outputs */ + ret = regmap_update_bits(regmap, PLL_USER_CTL(pll), + TRION_PLL_OUT_MASK, 0); + if (ret) + return; + + /* Place the PLL mode in STANDBY */ + regmap_write(regmap, PLL_OPMODE(pll), TRION_PLL_STANDBY); + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); +} + +static unsigned long +clk_trion_pll_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 l, frac; + u64 prate = parent_rate; + + regmap_read(regmap, PLL_L_VAL(pll), &l); + regmap_read(regmap, PLL_ALPHA_VAL(pll), &frac); + + return alpha_pll_calc_rate(prate, l, frac, ALPHA_REG_16BIT_WIDTH); +} + +static long clk_trion_pll_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + unsigned long min_freq, max_freq; + u32 l; + u64 a; + + rate = alpha_pll_round_rate(rate, *prate, + &l, &a, ALPHA_REG_16BIT_WIDTH); + if (!pll->vco_table || alpha_pll_find_vco(pll, rate)) + return rate; + + min_freq = pll->vco_table[0].min_freq; + max_freq = pll->vco_table[pll->num_vco - 1].max_freq; + + return clamp(rate, min_freq, max_freq); +} + const struct clk_ops clk_alpha_pll_ops = { .enable = clk_alpha_pll_enable, .disable = clk_alpha_pll_disable, @@ -760,6 +908,15 @@ const struct clk_ops clk_alpha_pll_hwfsm_ops = { }; EXPORT_SYMBOL_GPL(clk_alpha_pll_hwfsm_ops); +const struct clk_ops clk_trion_fixed_pll_ops = { + .enable = clk_trion_pll_enable, + .disable = clk_trion_pll_disable, + .is_enabled = clk_trion_pll_is_enabled, + .recalc_rate = clk_trion_pll_recalc_rate, + .round_rate = clk_trion_pll_round_rate, +}; +EXPORT_SYMBOL_GPL(clk_trion_fixed_pll_ops); + static unsigned long clk_alpha_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) { @@ -1053,6 +1210,77 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, return (parent_rate / div); } +static unsigned long +clk_trion_pll_postdiv_recalc_rate(struct clk_hw *hw, unsigned long parent_rate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + struct regmap *regmap = pll->clkr.regmap; + u32 i, div = 1, val; + + regmap_read(regmap, PLL_USER_CTL(pll), &val); + + val >>= pll->post_div_shift; + val &= PLL_POST_DIV_MASK(pll); + + for (i = 0; i < pll->num_post_div; i++) { + if (pll->post_div_table[i].val == val) { + div = pll->post_div_table[i].div; + break; + } + } + + return (parent_rate / div); +} + +static long +clk_trion_pll_postdiv_round_rate(struct clk_hw *hw, unsigned long rate, + unsigned long *prate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + + return divider_round_rate(hw, rate, prate, pll->post_div_table, + pll->width, CLK_DIVIDER_ROUND_CLOSEST); +}; + +static int +clk_trion_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long parent_rate) +{ + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); + struct regmap *regmap = pll->clkr.regmap; + int i, val = 0, div, ret; + + /* + * If the PLL is in FSM mode, then treat the set_rate callback + * as a no-operation. + */ + ret = regmap_read(regmap, PLL_MODE(pll), &val); + if (ret) + return ret; + + if (val & PLL_VOTE_FSM_ENA) + return 0; + + div = DIV_ROUND_UP_ULL(parent_rate, rate); + for (i = 0; i < pll->num_post_div; i++) { + if (pll->post_div_table[i].div == div) { + val = pll->post_div_table[i].val; + break; + } + } + + return regmap_update_bits(regmap, PLL_USER_CTL(pll), + PLL_POST_DIV_MASK(pll) << PLL_POST_DIV_SHIFT, + val << PLL_POST_DIV_SHIFT); +} + +const struct clk_ops clk_trion_pll_postdiv_ops = { + .recalc_rate = clk_trion_pll_postdiv_recalc_rate, + .round_rate = clk_trion_pll_postdiv_round_rate, + .set_rate = clk_trion_pll_postdiv_set_rate, +}; +EXPORT_SYMBOL_GPL(clk_trion_pll_postdiv_ops); + static long clk_alpha_pll_postdiv_fabia_round_rate(struct clk_hw *hw, unsigned long rate, unsigned long *prate) { diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h index 66755f0f84fc..15f27f4b06df 100644 --- a/drivers/clk/qcom/clk-alpha-pll.h +++ b/drivers/clk/qcom/clk-alpha-pll.h @@ -13,22 +13,27 @@ enum { CLK_ALPHA_PLL_TYPE_HUAYRA, CLK_ALPHA_PLL_TYPE_BRAMMO, CLK_ALPHA_PLL_TYPE_FABIA, + CLK_ALPHA_PLL_TYPE_TRION, CLK_ALPHA_PLL_TYPE_MAX, }; enum { PLL_OFF_L_VAL, + PLL_OFF_CAL_L_VAL, PLL_OFF_ALPHA_VAL, PLL_OFF_ALPHA_VAL_U, PLL_OFF_USER_CTL, PLL_OFF_USER_CTL_U, + PLL_OFF_USER_CTL_U1, PLL_OFF_CONFIG_CTL, PLL_OFF_CONFIG_CTL_U, + PLL_OFF_CONFIG_CTL_U1, PLL_OFF_TEST_CTL, PLL_OFF_TEST_CTL_U, PLL_OFF_STATUS, PLL_OFF_OPMODE, PLL_OFF_FRAC, + PLL_OFF_CAL_VAL, PLL_OFF_MAX_REGS }; @@ -117,5 +122,7 @@ void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, const struct alpha_pll_config *config); void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, const struct alpha_pll_config *config); +extern const struct clk_ops clk_trion_fixed_pll_ops; +extern const struct clk_ops clk_trion_pll_postdiv_ops; #endif -- 2.20.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs 2019-06-25 6:31 ` [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs Vinod Koul @ 2019-06-27 21:50 ` Stephen Boyd 2019-06-29 12:28 ` Vinod Koul 0 siblings, 1 reply; 13+ messages in thread From: Stephen Boyd @ 2019-06-27 21:50 UTC (permalink / raw) To: Vinod Koul Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das, Vinod Koul Quoting Vinod Koul (2019-06-24 23:31:39) > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index 2c6773188761..30210f5c6726 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -32,6 +32,7 @@ > # define PLL_LOCK_DET BIT(31) > > #define PLL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_L_VAL]) > +#define PLL_CAL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_L_VAL]) > #define PLL_ALPHA_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL]) > #define PLL_ALPHA_VAL_U(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL_U]) > > @@ -44,14 +45,17 @@ > # define PLL_VCO_MASK 0x3 > > #define PLL_USER_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U]) > +#define PLL_USER_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U1]) > > #define PLL_CONFIG_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL]) > #define PLL_CONFIG_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U]) > +#define PLL_CONFIG_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U11]) This looks like a typo, U11 vs U1. So I don't think this has been compile tested.... > #define PLL_TEST_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL]) > #define PLL_TEST_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL_U]) > #define PLL_STATUS(p) ((p)->offset + (p)->regs[PLL_OFF_STATUS]) > #define PLL_OPMODE(p) ((p)->offset + (p)->regs[PLL_OFF_OPMODE]) > #define PLL_FRAC(p) ((p)->offset + (p)->regs[PLL_OFF_FRAC]) > +#define PLL_CAL_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_VAL]) > > const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { > [CLK_ALPHA_PLL_TYPE_DEFAULT] = { > const struct clk_ops clk_alpha_pll_ops = { > .enable = clk_alpha_pll_enable, > .disable = clk_alpha_pll_disable, > @@ -1053,6 +1210,77 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, [...] > + > +static int > +clk_trion_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > + struct regmap *regmap = pll->clkr.regmap; > + int i, val = 0, div, ret; > + > + /* > + * If the PLL is in FSM mode, then treat the set_rate callback > + * as a no-operation. And this is OK? Shouldn't we fail because we can't change to the rate that's desired? > + */ > + ret = regmap_read(regmap, PLL_MODE(pll), &val); > + if (ret) > + return ret; > + > + if (val & PLL_VOTE_FSM_ENA) > + return 0; > + > + div = DIV_ROUND_UP_ULL(parent_rate, rate); > + for (i = 0; i < pll->num_post_div; i++) { > + if (pll->post_div_table[i].div == div) { > + val = pll->post_div_table[i].val; > + break; > + } > + } > + > + return regmap_update_bits(regmap, PLL_USER_CTL(pll), > + PLL_POST_DIV_MASK(pll) << PLL_POST_DIV_SHIFT, > + val << PLL_POST_DIV_SHIFT); > +} ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs 2019-06-27 21:50 ` Stephen Boyd @ 2019-06-29 12:28 ` Vinod Koul 0 siblings, 0 replies; 13+ messages in thread From: Vinod Koul @ 2019-06-29 12:28 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On 27-06-19, 14:50, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-24 23:31:39) > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > > index 2c6773188761..30210f5c6726 100644 > > --- a/drivers/clk/qcom/clk-alpha-pll.c > > +++ b/drivers/clk/qcom/clk-alpha-pll.c > > @@ -32,6 +32,7 @@ > > # define PLL_LOCK_DET BIT(31) > > > > #define PLL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_L_VAL]) > > +#define PLL_CAL_L_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_L_VAL]) > > #define PLL_ALPHA_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL]) > > #define PLL_ALPHA_VAL_U(p) ((p)->offset + (p)->regs[PLL_OFF_ALPHA_VAL_U]) > > > > @@ -44,14 +45,17 @@ > > # define PLL_VCO_MASK 0x3 > > > > #define PLL_USER_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U]) > > +#define PLL_USER_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_USER_CTL_U1]) > > > > #define PLL_CONFIG_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL]) > > #define PLL_CONFIG_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U]) > > +#define PLL_CONFIG_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U11]) > > This looks like a typo, U11 vs U1. So I don't think this has been > compile tested.... Not sure how this has happened, I have this in my test br too. Will fix this > > #define PLL_TEST_CTL(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL]) > > #define PLL_TEST_CTL_U(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL_U]) > > #define PLL_STATUS(p) ((p)->offset + (p)->regs[PLL_OFF_STATUS]) > > #define PLL_OPMODE(p) ((p)->offset + (p)->regs[PLL_OFF_OPMODE]) > > #define PLL_FRAC(p) ((p)->offset + (p)->regs[PLL_OFF_FRAC]) > > +#define PLL_CAL_VAL(p) ((p)->offset + (p)->regs[PLL_OFF_CAL_VAL]) > > > > const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = { > > [CLK_ALPHA_PLL_TYPE_DEFAULT] = { > > const struct clk_ops clk_alpha_pll_ops = { > > .enable = clk_alpha_pll_enable, > > .disable = clk_alpha_pll_disable, > > @@ -1053,6 +1210,77 @@ static unsigned long clk_alpha_pll_postdiv_fabia_recalc_rate(struct clk_hw *hw, > [...] > > + > > +static int > > +clk_trion_pll_postdiv_set_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long parent_rate) > > +{ > > + struct clk_alpha_pll_postdiv *pll = to_clk_alpha_pll_postdiv(hw); > > + struct regmap *regmap = pll->clkr.regmap; > > + int i, val = 0, div, ret; > > + > > + /* > > + * If the PLL is in FSM mode, then treat the set_rate callback > > + * as a no-operation. > > And this is OK? Shouldn't we fail because we can't change to the rate > that's desired? Agreed, we should error out. Also looking at other PLLs I see we check for FSM in clk enabled and I guess we should do it there for this as well, will move -- ~Vinod ^ permalink raw reply [flat|nested] 13+ messages in thread
[parent not found: <20190625063140.17106-4-vkoul@kernel.org>]
* Re: [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 [not found] ` <20190625063140.17106-4-vkoul@kernel.org> @ 2019-06-27 21:31 ` Stephen Boyd 2019-06-29 13:51 ` Vinod Koul 0 siblings, 1 reply; 13+ messages in thread From: Stephen Boyd @ 2019-06-27 21:31 UTC (permalink / raw) To: Vinod Koul Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das, Vinod Koul Quoting Vinod Koul (2019-06-24 23:31:40) > From: Deepak Katragadda <dkatraga@codeaurora.org> > > Add the clocks supported in global clock controller which clock the > peripherals like BLSPs, SDCC, USB, MDSS etc. Register all the clocks > to the clock framework for the clients to be able to request for them. > > Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> > Signed-off-by: Taniya Das <tdas@codeaurora.org> > [vkoul: port to upstream and tidy-up] This can say "changed to parent_data scheme"? > Signed-off-by: Vinod Koul <vkoul@kernel.org> > --- > .../devicetree/bindings/clock/qcom,gcc.txt | 1 + > drivers/clk/qcom/Kconfig | 7 + > drivers/clk/qcom/Makefile | 1 + > drivers/clk/qcom/gcc-sm8150.c | 3719 +++++++++++++++++ > include/dt-bindings/clock/qcom,gcc-sm8150.h | 243 ++ > 5 files changed, 3971 insertions(+) > create mode 100644 drivers/clk/qcom/gcc-sm8150.c > create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8150.h > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt b/Documentation/devicetree/bindings/clock/qcom,gcc.txt > index 8661c3cd3ccf..1b39d949e63d 100644 > --- a/Documentation/devicetree/bindings/clock/qcom,gcc.txt > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt > @@ -23,6 +23,7 @@ Required properties : > "qcom,gcc-sdm630" > "qcom,gcc-sdm660" > "qcom,gcc-sdm845" > + "qcom,gcc-sm8150" > > - reg : shall contain base register location and length > - #clock-cells : shall contain 1 I'd expect the binding to get a larger update indicating which clks come from outside the clk controller and need to be specified as 'clocks' and 'clock-names'. > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > index 18bdf34d5e64..076872d195fd 100644 > --- a/drivers/clk/qcom/Kconfig > +++ b/drivers/clk/qcom/Kconfig > @@ -291,6 +291,13 @@ config SDM_LPASSCC_845 > Say Y if you want to use the LPASS branch clocks of the LPASS clock > controller to reset the LPASS subsystem. > > +config SM_GCC_8150 > + tristate "SM8150 Global Clock Controller" > + help > + Support for the global clock controller on SM8150 devices. > + Say Y if you want to use peripheral devices such as UART, > + SPI, I2C, USB, SD/eMMC, PCIe etc. Is there eMMC support? > + > config SPMI_PMIC_CLKDIV > tristate "SPMI PMIC clkdiv Support" > depends on SPMI || COMPILE_TEST > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > index f0768fb1f037..4a813b4055d0 100644 > --- a/drivers/clk/qcom/Makefile > +++ b/drivers/clk/qcom/Makefile > @@ -50,6 +50,7 @@ obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o > obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o > obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o > obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o > +obj-$(CONFIG_SM_GCC_8150) += gcc-sm8150.o > obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o > obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o > obj-$(CONFIG_QCOM_HFPLL) += hfpll.o > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > new file mode 100644 > index 000000000000..11cd9e19f18d > --- /dev/null > +++ b/drivers/clk/qcom/gcc-sm8150.c > @@ -0,0 +1,3719 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2017-19, The Linux Foundation. All rights reserved. Is it really 2017-19 and not 2017-2019? > + */ > + > +#include <linux/kernel.h> > +#include <linux/bitops.h> > +#include <linux/err.h> > +#include <linux/platform_device.h> > +#include <linux/module.h> > +#include <linux/of.h> > +#include <linux/of_device.h> > +#include <linux/clk.h> Is this include used? > +#include <linux/clk-provider.h> > +#include <linux/regmap.h> > +#include <linux/reset-controller.h> > + > +#include <dt-bindings/clock/qcom,gcc-sm8150.h> > + > +#include "common.h" > +#include "clk-alpha-pll.h" > +#include "clk-branch.h" > +#include "clk-pll.h" > +#include "clk-rcg.h" > +#include "clk-regmap.h" > +#include "reset.h" > + > +#define GCC_NPU_MISC 0x4d110 > +#define GCC_GPU_MISC 0x71028 Drop these defines please and inline the values. > + > +enum { > + P_BI_TCXO, > + P_AUD_REF_CLK, > + P_CORE_BI_PLL_TEST_SE, > + P_GPLL0_OUT_EVEN, > + P_GPLL0_OUT_MAIN, > + P_GPLL4_OUT_MAIN, > + P_GPLL7_OUT_MAIN, > + P_GPLL9_OUT_MAIN, > + P_SLEEP_CLK, > +}; > + > +static const struct parent_map gcc_parent_map_0[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_0[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct clk_parent_data gcc_parents_0_ao[] = { > + { .fw_name = "bi_tcxo_ao", .name = "bi_tcxo_ao" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_1[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_SLEEP_CLK, 5 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_1[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "sleep_clk", .name = "sleep_clk" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_2[] = { > + { P_BI_TCXO, 0 }, > + { P_SLEEP_CLK, 5 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_2[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "sleep_clk", .name = "sleep_clk" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_3[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_3[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_4[] = { > + { P_BI_TCXO, 0 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_4[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_5[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_GPLL7_OUT_MAIN, 3 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_5[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "gpll7", .name = "gpll7" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, Aren't these gplls all created in this file? They shouldn't be listed in DT so I'm confused why we have .fw_name for them. > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, If there isn't a legacy string name for core_bi_pll_test_se we should just leave it out and not have a .name field, just the .fw_name one. > +}; > + > +static const struct parent_map gcc_parent_map_6[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_GPLL9_OUT_MAIN, 2 }, > + { P_GPLL4_OUT_MAIN, 5 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_6[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "gpll9", .name = "gpll9" }, > + { .fw_name = "gpll4", .name = "gpll4" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static const struct parent_map gcc_parent_map_7[] = { > + { P_BI_TCXO, 0 }, > + { P_GPLL0_OUT_MAIN, 1 }, > + { P_AUD_REF_CLK, 2 }, > + { P_GPLL0_OUT_EVEN, 6 }, > + { P_CORE_BI_PLL_TEST_SE, 7 }, > +}; > + > +static const struct clk_parent_data gcc_parents_7[] = { > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > + { .fw_name = "gpll0", .name = "gpll0" }, > + { .fw_name = "aud_ref_clk", .name = "aud_ref_clk" }, > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > +}; > + > +static struct pll_vco trion_vco[] = { const? > + { 249600000, 2000000000, 0 }, > +}; > + > +static struct clk_alpha_pll gpll0 = { > + .offset = 0x0, > + .vco_table = trion_vco, > + .num_vco = ARRAY_SIZE(trion_vco), > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION], > + .clkr = { > + .enable_reg = 0x52000, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gpll0", > + .parent_data = &(const struct clk_parent_data){ > + .fw_name = "bi_tcxo", > + .name = "bi_tcxo", > + }, > + .num_parents = 1, > + .ops = &clk_trion_fixed_pll_ops, > + }, > + }, > +}; > + > +static const struct clk_div_table post_div_table_trion_even[] = { > + { 0x0, 1 }, > + { 0x1, 2 }, > + { 0x3, 4 }, > + { 0x7, 8 }, > + { } > +}; > + > +static struct clk_alpha_pll_postdiv gpll0_out_even = { > + .offset = 0x0, > + .post_div_shift = 8, > + .post_div_table = post_div_table_trion_even, > + .num_post_div = ARRAY_SIZE(post_div_table_trion_even), > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION], > + .width = 4, > + .clkr.hw.init = &(struct clk_init_data){ > + .name = "gpll0_out_even", > + .parent_data = &(const struct clk_parent_data){ > + .fw_name = "gpll0", > + .name = "gpll0", Use a direct pointer? This applies in many places in the code. > + }, > + .num_parents = 1, > + .ops = &clk_trion_pll_postdiv_ops, > + }, > +}; > + [...] > + > +static struct clk_branch gcc_video_ahb_clk = { > + .halt_reg = 0xb004, > + .halt_check = BRANCH_HALT_DELAY, > + .hwcg_reg = 0xb004, > + .hwcg_bit = 1, > + .clkr = { > + .enable_reg = 0xb004, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gcc_video_ahb_clk", > + .flags = CLK_IS_CRITICAL, Can you please add a comment around all CLK_IS_CRITICAL flags indicating why these clks can't be turned off? > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch gcc_video_axi0_clk = { > + .halt_reg = 0xb024, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0xb024, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gcc_video_axi0_clk", > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch gcc_video_axi1_clk = { > + .halt_reg = 0xb028, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0xb028, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gcc_video_axi1_clk", > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch gcc_video_axic_clk = { > + .halt_reg = 0xb02c, > + .halt_check = BRANCH_HALT, > + .clkr = { > + .enable_reg = 0xb02c, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gcc_video_axic_clk", > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + > +static struct clk_branch gcc_video_xo_clk = { > + .halt_reg = 0xb040, > + .halt_check = BRANCH_HALT_DELAY, Why doesn't the halt bit work? > + .clkr = { > + .enable_reg = 0xb040, > + .enable_mask = BIT(0), > + .hw.init = &(struct clk_init_data){ > + .name = "gcc_video_xo_clk", > + .flags = CLK_IS_CRITICAL, > + .ops = &clk_branch2_ops, > + }, > + }, > +}; > + ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 2019-06-27 21:31 ` [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 Stephen Boyd @ 2019-06-29 13:51 ` Vinod Koul 2019-07-15 23:01 ` Stephen Boyd 0 siblings, 1 reply; 13+ messages in thread From: Vinod Koul @ 2019-06-29 13:51 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On 27-06-19, 14:31, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-24 23:31:40) > > From: Deepak Katragadda <dkatraga@codeaurora.org> > > > > Add the clocks supported in global clock controller which clock the > > peripherals like BLSPs, SDCC, USB, MDSS etc. Register all the clocks > > to the clock framework for the clients to be able to request for them. > > > > Signed-off-by: Deepak Katragadda <dkatraga@codeaurora.org> > > Signed-off-by: Taniya Das <tdas@codeaurora.org> > > [vkoul: port to upstream and tidy-up] > > This can say "changed to parent_data scheme"? > > > Signed-off-by: Vinod Koul <vkoul@kernel.org> > > --- > > .../devicetree/bindings/clock/qcom,gcc.txt | 1 + > > drivers/clk/qcom/Kconfig | 7 + > > drivers/clk/qcom/Makefile | 1 + > > drivers/clk/qcom/gcc-sm8150.c | 3719 +++++++++++++++++ > > include/dt-bindings/clock/qcom,gcc-sm8150.h | 243 ++ > > 5 files changed, 3971 insertions(+) > > create mode 100644 drivers/clk/qcom/gcc-sm8150.c > > create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8150.h > > > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gcc.txt b/Documentation/devicetree/bindings/clock/qcom,gcc.txt > > index 8661c3cd3ccf..1b39d949e63d 100644 > > --- a/Documentation/devicetree/bindings/clock/qcom,gcc.txt > > +++ b/Documentation/devicetree/bindings/clock/qcom,gcc.txt > > @@ -23,6 +23,7 @@ Required properties : > > "qcom,gcc-sdm630" > > "qcom,gcc-sdm660" > > "qcom,gcc-sdm845" > > + "qcom,gcc-sm8150" > > > > - reg : shall contain base register location and length > > - #clock-cells : shall contain 1 > > I'd expect the binding to get a larger update indicating which clks come > from outside the clk controller and need to be specified as 'clocks' and > 'clock-names'. > > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > > index 18bdf34d5e64..076872d195fd 100644 > > --- a/drivers/clk/qcom/Kconfig > > +++ b/drivers/clk/qcom/Kconfig > > @@ -291,6 +291,13 @@ config SDM_LPASSCC_845 > > Say Y if you want to use the LPASS branch clocks of the LPASS clock > > controller to reset the LPASS subsystem. > > > > +config SM_GCC_8150 > > + tristate "SM8150 Global Clock Controller" > > + help > > + Support for the global clock controller on SM8150 devices. > > + Say Y if you want to use peripheral devices such as UART, > > + SPI, I2C, USB, SD/eMMC, PCIe etc. > > Is there eMMC support? > > > + > > config SPMI_PMIC_CLKDIV > > tristate "SPMI PMIC clkdiv Support" > > depends on SPMI || COMPILE_TEST > > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > > index f0768fb1f037..4a813b4055d0 100644 > > --- a/drivers/clk/qcom/Makefile > > +++ b/drivers/clk/qcom/Makefile > > @@ -50,6 +50,7 @@ obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o > > obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o > > obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o > > obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o > > +obj-$(CONFIG_SM_GCC_8150) += gcc-sm8150.o > > obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o > > obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o > > obj-$(CONFIG_QCOM_HFPLL) += hfpll.o > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > new file mode 100644 > > index 000000000000..11cd9e19f18d > > --- /dev/null > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > @@ -0,0 +1,3719 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (c) 2017-19, The Linux Foundation. All rights reserved. > > Is it really 2017-19 and not 2017-2019? will update > > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/bitops.h> > > +#include <linux/err.h> > > +#include <linux/platform_device.h> > > +#include <linux/module.h> > > +#include <linux/of.h> > > +#include <linux/of_device.h> > > +#include <linux/clk.h> > > Is this include used? will check and update > > > +#include <linux/clk-provider.h> > > +#include <linux/regmap.h> > > +#include <linux/reset-controller.h> > > + > > +#include <dt-bindings/clock/qcom,gcc-sm8150.h> > > + > > +#include "common.h" > > +#include "clk-alpha-pll.h" > > +#include "clk-branch.h" > > +#include "clk-pll.h" > > +#include "clk-rcg.h" > > +#include "clk-regmap.h" > > +#include "reset.h" > > + > > +#define GCC_NPU_MISC 0x4d110 > > +#define GCC_GPU_MISC 0x71028 > > Drop these defines please and inline the values. Sure > > +static const struct clk_parent_data gcc_parents_5[] = { > > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > > + { .fw_name = "gpll0", .name = "gpll0" }, > > + { .fw_name = "gpll7", .name = "gpll7" }, > > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > > Aren't these gplls all created in this file? They shouldn't be listed in > DT so I'm confused why we have .fw_name for them. Yes they are and the DT doesnt provide these clock. From what I understood from the name conversion to new schema was we should add it like above, let me know if I missed something > > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > > If there isn't a legacy string name for core_bi_pll_test_se we should > just leave it out and not have a .name field, just the .fw_name one. Okay > > +static const struct clk_parent_data gcc_parents_7[] = { > > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > > + { .fw_name = "gpll0", .name = "gpll0" }, > > + { .fw_name = "aud_ref_clk", .name = "aud_ref_clk" }, > > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > > + { .fw_name = "core_bi_pll_test_se", .name = "core_bi_pll_test_se" }, > > +}; > > + > > +static struct pll_vco trion_vco[] = { > > const? will add > > +static struct clk_alpha_pll_postdiv gpll0_out_even = { > > + .offset = 0x0, > > + .post_div_shift = 8, > > + .post_div_table = post_div_table_trion_even, > > + .num_post_div = ARRAY_SIZE(post_div_table_trion_even), > > + .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_TRION], > > + .width = 4, > > + .clkr.hw.init = &(struct clk_init_data){ > > + .name = "gpll0_out_even", > > + .parent_data = &(const struct clk_parent_data){ > > + .fw_name = "gpll0", > > + .name = "gpll0", > > Use a direct pointer? This applies in many places in the code. okay will check here and other places > > +static struct clk_branch gcc_video_ahb_clk = { > > + .halt_reg = 0xb004, > > + .halt_check = BRANCH_HALT_DELAY, > > + .hwcg_reg = 0xb004, > > + .hwcg_bit = 1, > > + .clkr = { > > + .enable_reg = 0xb004, > > + .enable_mask = BIT(0), > > + .hw.init = &(struct clk_init_data){ > > + .name = "gcc_video_ahb_clk", > > + .flags = CLK_IS_CRITICAL, > > Can you please add a comment around all CLK_IS_CRITICAL flags indicating > why these clks can't be turned off? okay will do > > +static struct clk_branch gcc_video_axic_clk = { > > + .halt_reg = 0xb02c, > > + .halt_check = BRANCH_HALT, > > + .clkr = { > > + .enable_reg = 0xb02c, > > + .enable_mask = BIT(0), > > + .hw.init = &(struct clk_init_data){ > > + .name = "gcc_video_axic_clk", > > + .ops = &clk_branch2_ops, > > + }, > > + }, > > +}; > > + > > +static struct clk_branch gcc_video_xo_clk = { > > + .halt_reg = 0xb040, > > + .halt_check = BRANCH_HALT_DELAY, > > Why doesn't the halt bit work? Not sure, will check and update -- ~Vinod ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 2019-06-29 13:51 ` Vinod Koul @ 2019-07-15 23:01 ` Stephen Boyd 2019-07-16 4:07 ` Vinod Koul 0 siblings, 1 reply; 13+ messages in thread From: Stephen Boyd @ 2019-07-15 23:01 UTC (permalink / raw) To: Vinod Koul Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das Quoting Vinod Koul (2019-06-29 06:51:19) > On 27-06-19, 14:31, Stephen Boyd wrote: > > Quoting Vinod Koul (2019-06-24 23:31:40) > > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > > > index 18bdf34d5e64..076872d195fd 100644 > > > --- a/drivers/clk/qcom/Kconfig > > > +++ b/drivers/clk/qcom/Kconfig > > > @@ -291,6 +291,13 @@ config SDM_LPASSCC_845 > > > Say Y if you want to use the LPASS branch clocks of the LPASS clock > > > controller to reset the LPASS subsystem. > > > > > > +config SM_GCC_8150 > > > + tristate "SM8150 Global Clock Controller" > > > + help > > > + Support for the global clock controller on SM8150 devices. > > > + Say Y if you want to use peripheral devices such as UART, > > > + SPI, I2C, USB, SD/eMMC, PCIe etc. > > > > Is there eMMC support? > > I guess no? > > > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > > > index f0768fb1f037..4a813b4055d0 100644 > > > --- a/drivers/clk/qcom/Makefile > > > +++ b/drivers/clk/qcom/Makefile > > > @@ -50,6 +50,7 @@ obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o > > > obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o > > > obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o > > > obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o > > > +obj-$(CONFIG_SM_GCC_8150) += gcc-sm8150.o > > > obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o > > > obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o > > > obj-$(CONFIG_QCOM_HFPLL) += hfpll.o > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > > new file mode 100644 > > > index 000000000000..11cd9e19f18d > > > --- /dev/null > > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > > +static const struct clk_parent_data gcc_parents_5[] = { > > > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > > > + { .fw_name = "gpll0", .name = "gpll0" }, > > > + { .fw_name = "gpll7", .name = "gpll7" }, > > > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > > > > Aren't these gplls all created in this file? They shouldn't be listed in > > DT so I'm confused why we have .fw_name for them. > > Yes they are and the DT doesnt provide these clock. From what I > understood from the name conversion to new schema was we should add it > like above, let me know if I missed something > Yes, you should use the direct clk_hw pointer part of the clk_parent_data structure instead of having a .fw_name or .name member in these rows. Any clk inside of the clk controller shouldn't be exposed into DT just so that we can find it again through string comparisons. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 2019-07-15 23:01 ` Stephen Boyd @ 2019-07-16 4:07 ` Vinod Koul 0 siblings, 0 replies; 13+ messages in thread From: Vinod Koul @ 2019-07-16 4:07 UTC (permalink / raw) To: Stephen Boyd Cc: linux-arm-msm, Bjorn Andersson, Deepak Katragadda, Andy Gross, David Brown, Michael Turquette, linux-clk, Taniya Das On 15-07-19, 16:01, Stephen Boyd wrote: > Quoting Vinod Koul (2019-06-29 06:51:19) > > On 27-06-19, 14:31, Stephen Boyd wrote: > > > Quoting Vinod Koul (2019-06-24 23:31:40) > > > > diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig > > > > index 18bdf34d5e64..076872d195fd 100644 > > > > --- a/drivers/clk/qcom/Kconfig > > > > +++ b/drivers/clk/qcom/Kconfig > > > > @@ -291,6 +291,13 @@ config SDM_LPASSCC_845 > > > > Say Y if you want to use the LPASS branch clocks of the LPASS clock > > > > controller to reset the LPASS subsystem. > > > > > > > > +config SM_GCC_8150 > > > > + tristate "SM8150 Global Clock Controller" > > > > + help > > > > + Support for the global clock controller on SM8150 devices. > > > > + Say Y if you want to use peripheral devices such as UART, > > > > + SPI, I2C, USB, SD/eMMC, PCIe etc. > > > > > > Is there eMMC support? > > I guess no? Nope, copy paste error, have fixed it up :) > > > > > diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile > > > > index f0768fb1f037..4a813b4055d0 100644 > > > > --- a/drivers/clk/qcom/Makefile > > > > +++ b/drivers/clk/qcom/Makefile > > > > @@ -50,6 +50,7 @@ obj-$(CONFIG_SDM_GCC_845) += gcc-sdm845.o > > > > obj-$(CONFIG_SDM_GPUCC_845) += gpucc-sdm845.o > > > > obj-$(CONFIG_SDM_LPASSCC_845) += lpasscc-sdm845.o > > > > obj-$(CONFIG_SDM_VIDEOCC_845) += videocc-sdm845.o > > > > +obj-$(CONFIG_SM_GCC_8150) += gcc-sm8150.o > > > > obj-$(CONFIG_SPMI_PMIC_CLKDIV) += clk-spmi-pmic-div.o > > > > obj-$(CONFIG_KPSS_XCC) += kpss-xcc.o > > > > obj-$(CONFIG_QCOM_HFPLL) += hfpll.o > > > > diff --git a/drivers/clk/qcom/gcc-sm8150.c b/drivers/clk/qcom/gcc-sm8150.c > > > > new file mode 100644 > > > > index 000000000000..11cd9e19f18d > > > > --- /dev/null > > > > +++ b/drivers/clk/qcom/gcc-sm8150.c > > > > +static const struct clk_parent_data gcc_parents_5[] = { > > > > + { .fw_name = "bi_tcxo", .name = "bi_tcxo" }, > > > > + { .fw_name = "gpll0", .name = "gpll0" }, > > > > + { .fw_name = "gpll7", .name = "gpll7" }, > > > > + { .fw_name = "gpll0_out_even", .name = "gpll0_out_even" }, > > > > > > Aren't these gplls all created in this file? They shouldn't be listed in > > > DT so I'm confused why we have .fw_name for them. > > > > Yes they are and the DT doesnt provide these clock. From what I > > understood from the name conversion to new schema was we should add it > > like above, let me know if I missed something > > > > Yes, you should use the direct clk_hw pointer part of the > clk_parent_data structure instead of having a .fw_name or .name member > in these rows. Any clk inside of the clk controller shouldn't be exposed > into DT just so that we can find it again through string comparisons. Thanks, I think I have fixed it up. The parent clk in DT would be cxo and sleep_clk and creating bi_tcxo, gpll0, gpll7 etc in driver I have updated the patchset and will post shortly Thanks -- ~Vinod ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2019-07-16 4:11 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-25 6:31 [PATCH v3 0/3] clk: qcom: Add support for SM8150 GCC Vinod Koul 2019-06-25 6:31 ` [PATCH v3 1/3] clk: qcom: clk-alpha-pll: Remove post_div_table checks Vinod Koul 2019-06-27 21:37 ` Stephen Boyd 2019-06-29 11:14 ` Vinod Koul 2019-07-16 0:08 ` Bjorn Andersson 2019-07-16 4:08 ` Vinod Koul 2019-06-25 6:31 ` [PATCH v3 2/3] clk: qcom: clk-alpha-pll: Add support for Trion PLLs Vinod Koul 2019-06-27 21:50 ` Stephen Boyd 2019-06-29 12:28 ` Vinod Koul [not found] ` <20190625063140.17106-4-vkoul@kernel.org> 2019-06-27 21:31 ` [PATCH v3 3/3] clk: qcom: gcc: Add global clock controller driver for SM8150 Stephen Boyd 2019-06-29 13:51 ` Vinod Koul 2019-07-15 23:01 ` Stephen Boyd 2019-07-16 4:07 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).