On Mon, Oct 07, 2024 at 06:36:12PM GMT, Dzmitry Sankouski wrote:
> SDM845 has "General Purpose" clocks that can be muxed to
> SoC pins to clock various external devices.
> Those clocks may be used as e.g. PWM sources for external peripherals.
>
> GPCLK can in theory have arbitrary value depending on the use case, so
> the concept of frequency tables, used in rcg2 clock driver, is not
> efficient, because it allows only defined frequencies.
>
> Introduce clk_rcg2_gp_ops, which automatically calculate clock
> mnd values for arbitrary clock rate. The calculation done as follows:
> - upon determine rate request, we calculate m/n/pre_div as follows:
> - find parent(from our client's assigned-clock-parent) rate
> - find scaled rates by dividing rates on its greatest common divisor
> - assign requested scaled rate to m
> - factorize scaled parent rate, put multipliers to n till max value
> (determined by mnd_width)
> - validate calculated values with *_width:
> - if doesn't fit, delete divisor and multiplier by 2 until fit
> - return determined rate
>
> Limitations:
> - The driver doesn't select a parent clock (it may be selected by client
> in device tree with assigned-clocks, assigned-clock-parents properties)
>
> Signed-off-by: Dzmitry Sankouski <dsankouski@xxxxxxxxx>
>
> ---
> Changes in v6:
> - remove unused count variable
> - run sparse and smatch
>
> Changes in v5:
> - replace '/' to div64_u64 to fix 32 bit gcc error
> - fix empty scalar initializer
> ---
> drivers/clk/qcom/clk-rcg.h | 1 +
> drivers/clk/qcom/clk-rcg2.c | 191 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
> drivers/clk/qcom/gcc-sdm845.c | 11 +++-----
> 3 files changed, 189 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
> index 8e0f3372dc7a..8817d14bbda4 100644
> --- a/drivers/clk/qcom/clk-rcg.h
> +++ b/drivers/clk/qcom/clk-rcg.h
> @@ -189,6 +189,7 @@ struct clk_rcg2_gfx3d {
> container_of(to_clk_rcg2(_hw), struct clk_rcg2_gfx3d, rcg)
>
> extern const struct clk_ops clk_rcg2_ops;
> +extern const struct clk_ops clk_rcg2_gp_ops;
> extern const struct clk_ops clk_rcg2_floor_ops;
> extern const struct clk_ops clk_rcg2_fm_ops;
> extern const struct clk_ops clk_rcg2_mux_closest_ops;
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 0fc23a87b432..ba0cccdc73ec 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -8,11 +8,13 @@
> #include <linux/err.h>
> #include <linux/bug.h>
> #include <linux/export.h>
> +#include <linux/clk.h>
> #include <linux/clk-provider.h>
> #include <linux/delay.h>
> #include <linux/rational.h>
> #include <linux/regmap.h>
> #include <linux/math64.h>
> +#include <linux/gcd.h>
> #include <linux/minmax.h>
> #include <linux/slab.h>
>
> @@ -32,6 +34,7 @@
>
> #define CFG_REG 0x4
> #define CFG_SRC_DIV_SHIFT 0
> +#define CFG_SRC_DIV_LENGTH 8
> #define CFG_SRC_SEL_SHIFT 8
> #define CFG_SRC_SEL_MASK (0x7 << CFG_SRC_SEL_SHIFT)
> #define CFG_MODE_SHIFT 12
> @@ -148,6 +151,14 @@ static int clk_rcg2_set_parent(struct clk_hw *hw, u8 index)
> return update_config(rcg);
> }
>
> +// Converts divisors to corresponding clock register values.
> +// @param f - Frequency table with pure m/n/pre_div parameters.
Please use kernel-doc formatting for these.
> +static void convert_to_reg_val(struct freq_tbl *f)
> +{
> + f->pre_div *= 2;
> + f->pre_div -= 1;
> +}
> +
> /*
> * Calculate m/n:d rate
> *
> @@ -400,16 +411,115 @@ static int clk_rcg2_fm_determine_rate(struct clk_hw *hw,
> return _freq_tbl_fm_determine_rate(hw, rcg->freq_multi_tbl, req);
> }
>
> -static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f,
> - u32 *_cfg)
Could you split this patch into one that affects the non-gp rcg2
operations, and then one that adds and uses the gp-ops?
I'd expect that would make it easier to assess the risk to all other
rcg2 use cases.
> +// Split multiplier that doesn't fit in n neither in pre_div.
> +//
> +// @param multiplier - multiplier to split between n and pre_div
> +// @param pre_div - pointer to pre divisor value
> +// @param n - pointer to n divisor value
> +// @param pre_div_max - pre divisor maximum value
> +//
> +static inline void clk_rcg2_split_div(int multiplier, unsigned int *pre_div,
> + u16 *n, unsigned int pre_div_max)
> +{
> + *n = mult_frac(multiplier * *n, *pre_div, pre_div_max);
> + *pre_div = pre_div_max;
> +}
> +
> +static void clk_rcg2_calc_mnd(u64 parent_rate, u64 rate, struct freq_tbl *f,
> + unsigned int mnd_max, unsigned int pre_div_max)
> +{
> + int i = 2;
> + unsigned int pre_div = 1;
> + unsigned long rates_gcd, scaled_parent_rate;
> + u16 m, n = 1, n_candidate = 1, n_max;
> +
> + rates_gcd = gcd(parent_rate, rate);
> + m = div64_u64(rate, rates_gcd);
> + scaled_parent_rate = div64_u64(parent_rate, rates_gcd);
> + while (scaled_parent_rate > (mnd_max + m) * pre_div_max) {
> + // we're exceeding divisor's range, trying lower scale.
> + if (m > 1) {
> + m--;
> + scaled_parent_rate = mult_frac(scaled_parent_rate, m, (m + 1));
> + } else {
> + f->n = mnd_max + m;
> + f->pre_div = pre_div_max;
> + f->m = m;
> + }
> + }
> +
> + n_max = m + mnd_max;
> +
> + while (scaled_parent_rate > 1) {
> + while (scaled_parent_rate % i == 0) {
> + n_candidate *= i;
> + if (n_candidate < n_max)
> + n = n_candidate;
> + else if (pre_div * i < pre_div_max)
> + pre_div *= i;
> + else
> + clk_rcg2_split_div(i, &pre_div, &n, pre_div_max);
> +
> + scaled_parent_rate /= i;
> + }
> + i++;
> + }
> +
> + f->m = m;
> + f->n = n;
> + f->pre_div = pre_div > 1 ? pre_div : 0;
> +}
> +
> +static int clk_rcg2_determine_gp_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + struct freq_tbl *f;
> + int mnd_max = BIT(rcg->mnd_width) - 1;
> + int hid_max = BIT(rcg->hid_width) - 1;
> + struct clk_hw *parent;
> + u64 parent_rate;
> +
> + parent = clk_hw_get_parent(hw);
> + parent_rate = clk_get_rate(parent->clk);
> + if (!parent_rate)
> + return -EINVAL;
> +
> + f = kzalloc(sizeof(*f), GFP_KERNEL);
As far as I can tell sizeof(*f) is 16, so should be fine to just put it
on the stack?
Regards,
Bjorn
> +
> + if (!f)
> + return -ENOMEM;
> +
> + clk_rcg2_calc_mnd(parent_rate, req->rate, f, mnd_max, hid_max / 2);
> + convert_to_reg_val(f);
> + req->rate = calc_rate(parent_rate, f->m, f->n, f->n, f->pre_div);
> +
> + kfree(f);
> +
> + return 0;
> +}
> +
> +static int __clk_rcg2_configure_parent(struct clk_rcg2 *rcg, u8 src, u32 *_cfg)
> {
> - u32 cfg, mask, d_val, not2d_val, n_minus_m;
> struct clk_hw *hw = &rcg->clkr.hw;
> - int ret, index = qcom_find_src_index(hw, rcg->parent_map, f->src);
> + u32 mask = CFG_SRC_SEL_MASK;
> + int index = qcom_find_src_index(hw, rcg->parent_map, src);
>
> if (index < 0)
> return index;
>
> + *_cfg &= ~mask;
> + *_cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> +
> + return 0;
> +}
> +
> +static int __clk_rcg2_configure_mnd(struct clk_rcg2 *rcg, const struct freq_tbl *f,
> + u32 *_cfg)
> +{
> + u32 cfg, mask, d_val, not2d_val, n_minus_m;
> + int ret;
> +
> if (rcg->mnd_width && f->n) {
> mask = BIT(rcg->mnd_width) - 1;
> ret = regmap_update_bits(rcg->clkr.regmap,
> @@ -438,9 +548,8 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f,
> }
>
> mask = BIT(rcg->hid_width) - 1;
> - mask |= CFG_SRC_SEL_MASK | CFG_MODE_MASK | CFG_HW_CLK_CTRL_MASK;
> + mask |= CFG_MODE_MASK | CFG_HW_CLK_CTRL_MASK;
> cfg = f->pre_div << CFG_SRC_DIV_SHIFT;
> - cfg |= rcg->parent_map[index].cfg << CFG_SRC_SEL_SHIFT;
> if (rcg->mnd_width && f->n && (f->m != f->n))
> cfg |= CFG_MODE_DUAL_EDGE;
> if (rcg->hw_clk_ctrl)
> @@ -452,6 +561,22 @@ static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f,
> return 0;
> }
>
> +static int __clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f,
> + u32 *_cfg)
> +{
> + int ret;
> +
> + ret = __clk_rcg2_configure_parent(rcg, f->src, _cfg);
> + if (ret)
> + return ret;
> +
> + ret = __clk_rcg2_configure_mnd(rcg, f, _cfg);
> + if (ret)
> + return ret;
> +
> + return 0;
> +}
> +
> static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> {
> u32 cfg;
> @@ -472,6 +597,26 @@ static int clk_rcg2_configure(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> return update_config(rcg);
> }
>
> +static int clk_rcg2_configure_gp(struct clk_rcg2 *rcg, const struct freq_tbl *f)
> +{
> + u32 cfg;
> + int ret;
> +
> + ret = regmap_read(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), &cfg);
> + if (ret)
> + return ret;
> +
> + ret = __clk_rcg2_configure_mnd(rcg, f, &cfg);
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(rcg->clkr.regmap, RCG_CFG_OFFSET(rcg), cfg);
> + if (ret)
> + return ret;
> +
> + return update_config(rcg);
> +}
> +
> static int __clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
> enum freq_policy policy)
> {
> @@ -525,6 +670,28 @@ static int clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
> return __clk_rcg2_set_rate(hw, rate, CEIL);
> }
>
> +static int clk_rcg2_set_gp_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + int mnd_max = BIT(rcg->mnd_width) - 1;
> + int hid_max = BIT(rcg->hid_width) - 1;
> + struct freq_tbl *f;
> + int ret;
> +
> + f = kzalloc(sizeof(*f), GFP_KERNEL);
> +
> + if (!f)
> + return -ENOMEM;
> +
> + clk_rcg2_calc_mnd(parent_rate, rate, f, mnd_max, hid_max / 2);
> + convert_to_reg_val(f);
> + ret = clk_rcg2_configure_gp(rcg, f);
> + kfree(f);
> +
> + return ret;
> +}
> +
> static int clk_rcg2_set_floor_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long parent_rate)
> {
> @@ -652,6 +819,18 @@ const struct clk_ops clk_rcg2_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_rcg2_ops);
>
> +const struct clk_ops clk_rcg2_gp_ops = {
> + .is_enabled = clk_rcg2_is_enabled,
> + .get_parent = clk_rcg2_get_parent,
> + .set_parent = clk_rcg2_set_parent,
> + .recalc_rate = clk_rcg2_recalc_rate,
> + .determine_rate = clk_rcg2_determine_gp_rate,
> + .set_rate = clk_rcg2_set_gp_rate,
> + .get_duty_cycle = clk_rcg2_get_duty_cycle,
> + .set_duty_cycle = clk_rcg2_set_duty_cycle,
> +};
> +EXPORT_SYMBOL_GPL(clk_rcg2_gp_ops);
> +
> const struct clk_ops clk_rcg2_floor_ops = {
> .is_enabled = clk_rcg2_is_enabled,
> .get_parent = clk_rcg2_get_parent,
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index dc3aa7014c3e..0def0fc0e009 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -284,11 +284,6 @@ static struct clk_rcg2 gcc_sdm670_cpuss_rbcpr_clk_src = {
> };
>
> static const struct freq_tbl ftbl_gcc_gp1_clk_src[] = {
> - F(19200000, P_BI_TCXO, 1, 0, 0),
> - F(25000000, P_GPLL0_OUT_EVEN, 12, 0, 0),
> - F(50000000, P_GPLL0_OUT_EVEN, 6, 0, 0),
> - F(100000000, P_GPLL0_OUT_MAIN, 6, 0, 0),
> - F(200000000, P_GPLL0_OUT_MAIN, 3, 0, 0),
> { }
> };
>
> @@ -302,7 +297,7 @@ static struct clk_rcg2 gcc_gp1_clk_src = {
> .name = "gcc_gp1_clk_src",
> .parent_data = gcc_parent_data_1,
> .num_parents = ARRAY_SIZE(gcc_parent_data_1),
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_rcg2_gp_ops,
> },
> };
>
> @@ -316,7 +311,7 @@ static struct clk_rcg2 gcc_gp2_clk_src = {
> .name = "gcc_gp2_clk_src",
> .parent_data = gcc_parent_data_1,
> .num_parents = ARRAY_SIZE(gcc_parent_data_1),
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_rcg2_gp_ops,
> },
> };
>
> @@ -330,7 +325,7 @@ static struct clk_rcg2 gcc_gp3_clk_src = {
> .name = "gcc_gp3_clk_src",
> .parent_data = gcc_parent_data_1,
> .num_parents = ARRAY_SIZE(gcc_parent_data_1),
> - .ops = &clk_rcg2_ops,
> + .ops = &clk_rcg2_gp_ops,
> },
> };
>
>
> --
> 2.39.2
>
