| Message ID | 1579905147-12142-5-git-send-email-vnkgutta@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add device tree and clock drivers for SM8250 SoC | expand |
Quoting Venkata Narendra Kumar Gutta (2020-01-24 14:32:24) > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > index 1b073b2..4258ab0 100644 > --- a/drivers/clk/qcom/clk-alpha-pll.c > +++ b/drivers/clk/qcom/clk-alpha-pll.c > @@ -1367,3 +1388,172 @@ static int clk_alpha_pll_postdiv_fabia_set_rate(struct clk_hw *hw, > .set_rate = clk_alpha_pll_postdiv_fabia_set_rate, > }; > EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops); > + > +void clk_lucid_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, Can we get some kernel documentation for this function? > + const struct alpha_pll_config *config) > +{ > + if (config->l) > + regmap_write(regmap, PLL_L_VAL(pll), config->l); > + > + regmap_write(regmap, PLL_CAL_L_VAL(pll), LUCID_PLL_CAL_VAL); > + > + if (config->alpha) > + regmap_write(regmap, PLL_ALPHA_VAL(pll), config->alpha); > + > + if (config->config_ctl_val) > + regmap_write(regmap, PLL_CONFIG_CTL(pll), > + config->config_ctl_val); > + > + if (config->config_ctl_hi_val) > + regmap_write(regmap, PLL_CONFIG_CTL_U(pll), > + config->config_ctl_hi_val); > + > + if (config->config_ctl_hi1_val) > + regmap_write(regmap, PLL_CONFIG_CTL_U1(pll), > + config->config_ctl_hi1_val); > + > + if (config->user_ctl_val) > + regmap_write(regmap, PLL_USER_CTL(pll), > + config->user_ctl_val); > + > + if (config->user_ctl_hi_val) > + regmap_write(regmap, PLL_USER_CTL_U(pll), > + config->user_ctl_hi_val); > + > + if (config->user_ctl_hi1_val) > + regmap_write(regmap, PLL_USER_CTL_U1(pll), > + config->user_ctl_hi1_val); > + > + if (config->test_ctl_val) > + regmap_write(regmap, PLL_TEST_CTL(pll), > + config->test_ctl_val); > + > + if (config->test_ctl_hi_val) > + regmap_write(regmap, PLL_TEST_CTL_U(pll), > + config->test_ctl_hi_val); > + > + if (config->test_ctl_hi1_val) > + regmap_write(regmap, PLL_TEST_CTL_U1(pll), > + config->test_ctl_hi1_val); > + > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_UPDATE_BYPASS, > + PLL_UPDATE_BYPASS); > + > + /* Disable PLL output */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); > + > + /* Set operation mode to OFF */ > + regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY); > + > + /* PLL should be in OFF mode before continuing */ > + wmb(); How does the write above overtake the write below? This barrier looks wrong. > + > + /* Place the PLL in STANDBY mode */ > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); > +} > +EXPORT_SYMBOL_GPL(clk_lucid_pll_configure); > + > +/* > + * The Lucid PLL requires a power-on self-calibration which happens when the > + * PLL comes out of reset. Calibrate in case it is not completed. > + */ > +static int alpha_pll_lucid_prepare(struct clk_hw *hw) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + u32 regval; > + int ret; > + > + /* Return early if calibration is not needed. */ > + regmap_read(pll->clkr.regmap, PLL_STATUS(pll), ®val); > + if (regval & LUCID_PCAL_DONE) > + return 0; > + > + ret = clk_trion_pll_enable(hw); > + if (ret) > + return ret; > + > + clk_trion_pll_disable(hw); > + > + return 0; Can you write this like: /* On/off to calibrate */ ret = clk_trion_pll_enable(hw); if (!ret) clk_trion_pll_disable(hw); return ret; > +} > + > +static int alpha_pll_lucid_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long prate) > +{ > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > + unsigned long rrate; > + u32 regval, l, alpha_width = pll_alpha_width(pll); > + u64 a; > + int ret; > + > + rrate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width); > + > + /* > + * Due to a limited number of bits for fractional rate programming, the > + * rounded up rate could be marginally higher than the requested rate. > + */ > + if (rrate > (rate + PLL_RATE_MARGIN) || rrate < rate) { Any chance this can be pushed into the alpha_pll_round_rate() API? It's duplicated in this driver. > + pr_err("Call set rate on the PLL with rounded rates!\n"); > + return -EINVAL; > + } > + > + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l); > + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a); > + > + /* Latch the PLL input */ > + ret = regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), > + PLL_UPDATE, PLL_UPDATE); > + if (ret) > + return ret; > + > + /* Wait for 2 reference cycles before checking the ACK bit. */ Are reference cycles 2 * 1 / 19.2MHz? > + udelay(1); > + regmap_read(pll->clkr.regmap, PLL_MODE(pll), ®val); > + if (!(regval & ALPHA_PLL_ACK_LATCH)) { > + WARN(1, "PLL latch failed. Output may be unstable!\n"); Do we need a big WARN stack for this? How about pr_warn() instead? > + return -EINVAL; > + } > + > + /* Return the latch input to 0 */ > + ret = regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), > + PLL_UPDATE, 0); > + if (ret) > + return ret; > + > + if (clk_hw_is_enabled(hw)) { > + ret = wait_for_pll_enable_lock(pll); > + if (ret) > + return ret; > + } > + > + /* Wait for PLL output to stabilize */ > + udelay(100); > + return 0; > +}
On 05-02-20, 11:33, Stephen Boyd wrote: > Quoting Venkata Narendra Kumar Gutta (2020-01-24 14:32:24) > > diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c > > index 1b073b2..4258ab0 100644 > > --- a/drivers/clk/qcom/clk-alpha-pll.c > > +++ b/drivers/clk/qcom/clk-alpha-pll.c > > @@ -1367,3 +1388,172 @@ static int clk_alpha_pll_postdiv_fabia_set_rate(struct clk_hw *hw, > > .set_rate = clk_alpha_pll_postdiv_fabia_set_rate, > > }; > > EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops); > > + > > +void clk_lucid_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, > > Can we get some kernel documentation for this function? Okay adding > > +{ > > + if (config->l) > > + regmap_write(regmap, PLL_L_VAL(pll), config->l); > > + > > + regmap_write(regmap, PLL_CAL_L_VAL(pll), LUCID_PLL_CAL_VAL); > > + > > + if (config->alpha) > > + regmap_write(regmap, PLL_ALPHA_VAL(pll), config->alpha); > > + > > + if (config->config_ctl_val) > > + regmap_write(regmap, PLL_CONFIG_CTL(pll), > > + config->config_ctl_val); > > + > > + if (config->config_ctl_hi_val) > > + regmap_write(regmap, PLL_CONFIG_CTL_U(pll), > > + config->config_ctl_hi_val); > > + > > + if (config->config_ctl_hi1_val) > > + regmap_write(regmap, PLL_CONFIG_CTL_U1(pll), > > + config->config_ctl_hi1_val); > > + > > + if (config->user_ctl_val) > > + regmap_write(regmap, PLL_USER_CTL(pll), > > + config->user_ctl_val); > > + > > + if (config->user_ctl_hi_val) > > + regmap_write(regmap, PLL_USER_CTL_U(pll), > > + config->user_ctl_hi_val); > > + > > + if (config->user_ctl_hi1_val) > > + regmap_write(regmap, PLL_USER_CTL_U1(pll), > > + config->user_ctl_hi1_val); > > + > > + if (config->test_ctl_val) > > + regmap_write(regmap, PLL_TEST_CTL(pll), > > + config->test_ctl_val); > > + > > + if (config->test_ctl_hi_val) > > + regmap_write(regmap, PLL_TEST_CTL_U(pll), > > + config->test_ctl_hi_val); > > + > > + if (config->test_ctl_hi1_val) > > + regmap_write(regmap, PLL_TEST_CTL_U1(pll), > > + config->test_ctl_hi1_val); > > + > > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_UPDATE_BYPASS, > > + PLL_UPDATE_BYPASS); > > + > > + /* Disable PLL output */ > > + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); > > + > > + /* Set operation mode to OFF */ > > + regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY); > > + > > + /* PLL should be in OFF mode before continuing */ > > + wmb(); > > How does the write above overtake the write below? This barrier looks > wrong. I think you are correct, it doesnt :), so removing this > > +static int alpha_pll_lucid_prepare(struct clk_hw *hw) > > +{ > > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > > + u32 regval; > > + int ret; > > + > > + /* Return early if calibration is not needed. */ > > + regmap_read(pll->clkr.regmap, PLL_STATUS(pll), ®val); > > + if (regval & LUCID_PCAL_DONE) > > + return 0; > > + > > + ret = clk_trion_pll_enable(hw); > > + if (ret) > > + return ret; > > + > > + clk_trion_pll_disable(hw); > > + > > + return 0; > > Can you write this like: > > /* On/off to calibrate */ > ret = clk_trion_pll_enable(hw); > if (!ret) > clk_trion_pll_disable(hw); > > return ret; Looks better, updated now. > > +static int alpha_pll_lucid_set_rate(struct clk_hw *hw, unsigned long rate, > > + unsigned long prate) > > +{ > > + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); > > + unsigned long rrate; > > + u32 regval, l, alpha_width = pll_alpha_width(pll); > > + u64 a; > > + int ret; > > + > > + rrate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width); > > + > > + /* > > + * Due to a limited number of bits for fractional rate programming, the > > + * rounded up rate could be marginally higher than the requested rate. > > + */ > > + if (rrate > (rate + PLL_RATE_MARGIN) || rrate < rate) { > > Any chance this can be pushed into the alpha_pll_round_rate() API? It's > duplicated in this driver. Yes here and couple of fabia pll functions. Said that I see alpha_pll_round_rate() is also invoked two places, alpha_pll_fabia_set_rate() and __clk_alpha_pll_set_rate(), so should we let these two also be updated, if you are okay with that I will update this > > + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l); > > + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a); > > + > > + /* Latch the PLL input */ > > + ret = regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), > > + PLL_UPDATE, PLL_UPDATE); > > + if (ret) > > + return ret; > > + > > + /* Wait for 2 reference cycles before checking the ACK bit. */ > > Are reference cycles 2 * 1 / 19.2MHz? Will check and update on this > > > + udelay(1); > > + regmap_read(pll->clkr.regmap, PLL_MODE(pll), ®val); > > + if (!(regval & ALPHA_PLL_ACK_LATCH)) { > > + WARN(1, "PLL latch failed. Output may be unstable!\n"); > > Do we need a big WARN stack for this? How about pr_warn() instead? Nope :), will move to a warn print :)
diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 1b073b2..4258ab0 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -52,6 +52,7 @@ #define PLL_CONFIG_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_CONFIG_CTL_U1]) #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_TEST_CTL_U1(p) ((p)->offset + (p)->regs[PLL_OFF_TEST_CTL_U1]) #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]) @@ -116,6 +117,22 @@ [PLL_OFF_ALPHA_VAL] = 0x40, [PLL_OFF_CAL_VAL] = 0x44, }, + [CLK_ALPHA_PLL_TYPE_LUCID] = { + [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_TEST_CTL_U1] = 0x2c, + [PLL_OFF_STATUS] = 0x30, + [PLL_OFF_OPMODE] = 0x38, + [PLL_OFF_ALPHA_VAL] = 0x40, + }, }; EXPORT_SYMBOL_GPL(clk_alpha_pll_regs); @@ -139,6 +156,10 @@ #define PLL_OUT_MASK 0x7 #define PLL_RATE_MARGIN 500 +/* LUCID PLL specific settings and offsets */ +#define LUCID_PLL_CAL_VAL 0x44 +#define LUCID_PCAL_DONE BIT(26) + #define pll_alpha_width(p) \ ((PLL_ALPHA_VAL_U(p) - PLL_ALPHA_VAL(p) == 4) ? \ ALPHA_REG_BITWIDTH : ALPHA_REG_16BIT_WIDTH) @@ -1367,3 +1388,172 @@ static int clk_alpha_pll_postdiv_fabia_set_rate(struct clk_hw *hw, .set_rate = clk_alpha_pll_postdiv_fabia_set_rate, }; EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_fabia_ops); + +void clk_lucid_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap, + const struct alpha_pll_config *config) +{ + if (config->l) + regmap_write(regmap, PLL_L_VAL(pll), config->l); + + regmap_write(regmap, PLL_CAL_L_VAL(pll), LUCID_PLL_CAL_VAL); + + if (config->alpha) + regmap_write(regmap, PLL_ALPHA_VAL(pll), config->alpha); + + if (config->config_ctl_val) + regmap_write(regmap, PLL_CONFIG_CTL(pll), + config->config_ctl_val); + + if (config->config_ctl_hi_val) + regmap_write(regmap, PLL_CONFIG_CTL_U(pll), + config->config_ctl_hi_val); + + if (config->config_ctl_hi1_val) + regmap_write(regmap, PLL_CONFIG_CTL_U1(pll), + config->config_ctl_hi1_val); + + if (config->user_ctl_val) + regmap_write(regmap, PLL_USER_CTL(pll), + config->user_ctl_val); + + if (config->user_ctl_hi_val) + regmap_write(regmap, PLL_USER_CTL_U(pll), + config->user_ctl_hi_val); + + if (config->user_ctl_hi1_val) + regmap_write(regmap, PLL_USER_CTL_U1(pll), + config->user_ctl_hi1_val); + + if (config->test_ctl_val) + regmap_write(regmap, PLL_TEST_CTL(pll), + config->test_ctl_val); + + if (config->test_ctl_hi_val) + regmap_write(regmap, PLL_TEST_CTL_U(pll), + config->test_ctl_hi_val); + + if (config->test_ctl_hi1_val) + regmap_write(regmap, PLL_TEST_CTL_U1(pll), + config->test_ctl_hi1_val); + + regmap_update_bits(regmap, PLL_MODE(pll), PLL_UPDATE_BYPASS, + PLL_UPDATE_BYPASS); + + /* Disable PLL output */ + regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, 0); + + /* Set operation mode to OFF */ + regmap_write(regmap, PLL_OPMODE(pll), PLL_STANDBY); + + /* PLL should be in OFF mode before continuing */ + wmb(); + + /* Place the PLL in STANDBY mode */ + regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N); +} +EXPORT_SYMBOL_GPL(clk_lucid_pll_configure); + +/* + * The Lucid PLL requires a power-on self-calibration which happens when the + * PLL comes out of reset. Calibrate in case it is not completed. + */ +static int alpha_pll_lucid_prepare(struct clk_hw *hw) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + u32 regval; + int ret; + + /* Return early if calibration is not needed. */ + regmap_read(pll->clkr.regmap, PLL_STATUS(pll), ®val); + if (regval & LUCID_PCAL_DONE) + return 0; + + ret = clk_trion_pll_enable(hw); + if (ret) + return ret; + + clk_trion_pll_disable(hw); + + return 0; +} + +static int alpha_pll_lucid_set_rate(struct clk_hw *hw, unsigned long rate, + unsigned long prate) +{ + struct clk_alpha_pll *pll = to_clk_alpha_pll(hw); + unsigned long rrate; + u32 regval, l, alpha_width = pll_alpha_width(pll); + u64 a; + int ret; + + rrate = alpha_pll_round_rate(rate, prate, &l, &a, alpha_width); + + /* + * Due to a limited number of bits for fractional rate programming, the + * rounded up rate could be marginally higher than the requested rate. + */ + if (rrate > (rate + PLL_RATE_MARGIN) || rrate < rate) { + pr_err("Call set rate on the PLL with rounded rates!\n"); + return -EINVAL; + } + + regmap_write(pll->clkr.regmap, PLL_L_VAL(pll), l); + regmap_write(pll->clkr.regmap, PLL_ALPHA_VAL(pll), a); + + /* Latch the PLL input */ + ret = regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), + PLL_UPDATE, PLL_UPDATE); + if (ret) + return ret; + + /* Wait for 2 reference cycles before checking the ACK bit. */ + udelay(1); + regmap_read(pll->clkr.regmap, PLL_MODE(pll), ®val); + if (!(regval & ALPHA_PLL_ACK_LATCH)) { + WARN(1, "PLL latch failed. Output may be unstable!\n"); + return -EINVAL; + } + + /* Return the latch input to 0 */ + ret = regmap_update_bits(pll->clkr.regmap, PLL_MODE(pll), + PLL_UPDATE, 0); + if (ret) + return ret; + + if (clk_hw_is_enabled(hw)) { + ret = wait_for_pll_enable_lock(pll); + if (ret) + return ret; + } + + /* Wait for PLL output to stabilize */ + udelay(100); + return 0; +} + +const struct clk_ops clk_alpha_pll_lucid_ops = { + .prepare = alpha_pll_lucid_prepare, + .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_alpha_pll_round_rate, + .set_rate = alpha_pll_lucid_set_rate, +}; +EXPORT_SYMBOL_GPL(clk_alpha_pll_lucid_ops); + +const struct clk_ops clk_alpha_pll_fixed_lucid_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_alpha_pll_round_rate, +}; +EXPORT_SYMBOL_GPL(clk_alpha_pll_fixed_lucid_ops); + +const struct clk_ops clk_alpha_pll_postdiv_lucid_ops = { + .recalc_rate = clk_alpha_pll_postdiv_fabia_recalc_rate, + .round_rate = clk_alpha_pll_postdiv_fabia_round_rate, + .set_rate = clk_alpha_pll_postdiv_fabia_set_rate, +}; +EXPORT_SYMBOL_GPL(clk_alpha_pll_postdiv_lucid_ops); diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h index fbc1f67..704674a 100644 --- a/drivers/clk/qcom/clk-alpha-pll.h +++ b/drivers/clk/qcom/clk-alpha-pll.h @@ -14,6 +14,7 @@ enum { CLK_ALPHA_PLL_TYPE_BRAMMO, CLK_ALPHA_PLL_TYPE_FABIA, CLK_ALPHA_PLL_TYPE_TRION, + CLK_ALPHA_PLL_TYPE_LUCID, CLK_ALPHA_PLL_TYPE_MAX, }; @@ -30,6 +31,7 @@ enum { PLL_OFF_CONFIG_CTL_U1, PLL_OFF_TEST_CTL, PLL_OFF_TEST_CTL_U, + PLL_OFF_TEST_CTL_U1, PLL_OFF_STATUS, PLL_OFF_OPMODE, PLL_OFF_FRAC, @@ -94,10 +96,13 @@ struct alpha_pll_config { u32 alpha_hi; u32 config_ctl_val; u32 config_ctl_hi_val; + u32 config_ctl_hi1_val; u32 user_ctl_val; u32 user_ctl_hi_val; + u32 user_ctl_hi1_val; u32 test_ctl_val; u32 test_ctl_hi_val; + u32 test_ctl_hi1_val; u32 main_output_mask; u32 aux_output_mask; u32 aux2_output_mask; @@ -123,10 +128,17 @@ struct alpha_pll_config { extern const struct clk_ops clk_alpha_pll_fixed_fabia_ops; extern const struct clk_ops clk_alpha_pll_postdiv_fabia_ops; +extern const struct clk_ops clk_alpha_pll_lucid_ops; +extern const struct clk_ops clk_alpha_pll_fixed_lucid_ops; +extern const struct clk_ops clk_alpha_pll_postdiv_lucid_ops; + 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); +void clk_lucid_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;