diff mbox series

[v3,02/23] gcc-sdm845: Add rates to the GP clocks

Message ID 20240618-starqltechn_integration_upstream-v3-2-e3f6662017ac@gmail.com (mailing list archive)
State Superseded
Headers show
Series This is continued work on Samsung S9(SM-9600) starqltechn | expand

Commit Message

Dzmitry Sankouski June 18, 2024, 1:59 p.m. UTC 
sdm845 has "General Purpose" clocks that can be muxed to
SoC pins.

Those clocks may be used as e.g. PWM sources for external peripherals.
Add more frequencies to the table for those clocks so it's possible
for arbitrary peripherals to make use of them.

See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---
 drivers/clk/qcom/gcc-sdm845.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

Comments

Dmitry Baryshkov June 18, 2024, 5:50 p.m. UTC | #1 
On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> sdm845 has "General Purpose" clocks that can be muxed to
> SoC pins.
> 
> Those clocks may be used as e.g. PWM sources for external peripherals.
> Add more frequencies to the table for those clocks so it's possible
> for arbitrary peripherals to make use of them.
> 
> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)

Each time I look at the table attached to the GP CLK, I feel that it's
plain wrong. In the end the GPCLK can in theory have arbitrary value
depending on the usecase.

Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
allow more flexibility than a default clk_rcg2_ops?

> 
> Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
> ---
>  drivers/clk/qcom/gcc-sdm845.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
> index ea4c3bf4fb9b..0efd3364e8f5 100644
> --- a/drivers/clk/qcom/gcc-sdm845.c
> +++ b/drivers/clk/qcom/gcc-sdm845.c
> @@ -283,7 +283,21 @@ static struct clk_rcg2 gcc_sdm670_cpuss_rbcpr_clk_src = {
>  	},
>  };
>  
> +/*
> + * This is a frequency table for "General Purpose" clocks.
> + * These clocks can be muxed to the SoC pins and may be used by
> + * external devices. They're often used as PWM source.
> + *
> + * See comment in gcc-mam8916.c at ftbl_gcc_gp1_3_clk.
> + */
>  static const struct freq_tbl ftbl_gcc_gp1_clk_src[] = {
> +	F(10000,   P_BI_TCXO,    16,  1, 120),
> +	F(20000,   P_BI_TCXO,    16,  1, 60),
> +	F(100000,  P_BI_TCXO,    16,  1,  12),
> +	F(500000,  P_GPLL0_OUT_EVEN, 12, 1, 100),
> +	F(1000000, P_GPLL0_OUT_EVEN, 12, 1, 50),
> +	F(2500000, P_GPLL0_OUT_EVEN, 12, 1, 10),
> +	F(5000000, P_GPLL0_OUT_EVEN, 12, 1, 5),
>  	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),
> 
> -- 
> 2.39.2
>
Konrad Dybcio June 18, 2024, 6:50 p.m. UTC | #2 
On 6/18/24 19:50, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
>> sdm845 has "General Purpose" clocks that can be muxed to
>> SoC pins.
>>
>> Those clocks may be used as e.g. PWM sources for external peripherals.
>> Add more frequencies to the table for those clocks so it's possible
>> for arbitrary peripherals to make use of them.
>>
>> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> 
> Each time I look at the table attached to the GP CLK, I feel that it's
> plain wrong. In the end the GPCLK can in theory have arbitrary value
> depending on the usecase.
> 
> Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> allow more flexibility than a default clk_rcg2_ops?

If we can somehow get max m/n/d values for all possible parents, sure

Konrad
Dmitry Baryshkov June 18, 2024, 6:55 p.m. UTC | #3 
On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
> 
> 
> On 6/18/24 19:50, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> > > sdm845 has "General Purpose" clocks that can be muxed to
> > > SoC pins.
> > > 
> > > Those clocks may be used as e.g. PWM sources for external peripherals.
> > > Add more frequencies to the table for those clocks so it's possible
> > > for arbitrary peripherals to make use of them.
> > > 
> > > See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> > 
> > Each time I look at the table attached to the GP CLK, I feel that it's
> > plain wrong. In the end the GPCLK can in theory have arbitrary value
> > depending on the usecase.
> > 
> > Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> > allow more flexibility than a default clk_rcg2_ops?
> 
> If we can somehow get max m/n/d values for all possible parents, sure

Calculate them at runtime?
Konrad Dybcio June 18, 2024, 7:11 p.m. UTC | #4 
On 6/18/24 20:55, Dmitry Baryshkov wrote:
> On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
>>
>>
>> On 6/18/24 19:50, Dmitry Baryshkov wrote:
>>> On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
>>>> sdm845 has "General Purpose" clocks that can be muxed to
>>>> SoC pins.
>>>>
>>>> Those clocks may be used as e.g. PWM sources for external peripherals.
>>>> Add more frequencies to the table for those clocks so it's possible
>>>> for arbitrary peripherals to make use of them.
>>>>
>>>> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
>>>
>>> Each time I look at the table attached to the GP CLK, I feel that it's
>>> plain wrong. In the end the GPCLK can in theory have arbitrary value
>>> depending on the usecase.
>>>
>>> Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
>>> allow more flexibility than a default clk_rcg2_ops?
>>
>> If we can somehow get max m/n/d values for all possible parents, sure
> 
> Calculate them at runtime?

We'd be calculating the mnd values for a frequency that's either equal or
reasonably close to the one requested. My worry is that we somehow need
to get the maximum values they can take (unless they're well-known)

Konrad
Dmitry Baryshkov June 19, 2024, 6:31 a.m. UTC | #5 
On Tue, Jun 18, 2024 at 09:11:58PM GMT, Konrad Dybcio wrote:
> 
> 
> On 6/18/24 20:55, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
> > > 
> > > 
> > > On 6/18/24 19:50, Dmitry Baryshkov wrote:
> > > > On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> > > > > sdm845 has "General Purpose" clocks that can be muxed to
> > > > > SoC pins.
> > > > > 
> > > > > Those clocks may be used as e.g. PWM sources for external peripherals.
> > > > > Add more frequencies to the table for those clocks so it's possible
> > > > > for arbitrary peripherals to make use of them.
> > > > > 
> > > > > See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> > > > 
> > > > Each time I look at the table attached to the GP CLK, I feel that it's
> > > > plain wrong. In the end the GPCLK can in theory have arbitrary value
> > > > depending on the usecase.
> > > > 
> > > > Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> > > > allow more flexibility than a default clk_rcg2_ops?
> > > 
> > > If we can somehow get max m/n/d values for all possible parents, sure
> > 
> > Calculate them at runtime?
> 
> We'd be calculating the mnd values for a frequency that's either equal or
> reasonably close to the one requested. My worry is that we somehow need
> to get the maximum values they can take (unless they're well-known)

One of the options might be to force devices to use
assigned-clock-parent to set GP CLK sorource and pwm-clk as an actual
device using the clock.
Dzmitry Sankouski July 19, 2024, 9:01 a.m. UTC | #6 
Why cannot max values be defined as ((2 ^ mnd_width) - 1) and ((2 ^
hid_width) - 1)?

вт, 18 июн. 2024 г. в 22:12, Konrad Dybcio <konrad.dybcio@linaro.org>:
>
>
>
> On 6/18/24 20:55, Dmitry Baryshkov wrote:
> > On Tue, Jun 18, 2024 at 08:50:52PM GMT, Konrad Dybcio wrote:
> >>
> >>
> >> On 6/18/24 19:50, Dmitry Baryshkov wrote:
> >>> On Tue, Jun 18, 2024 at 04:59:36PM GMT, Dzmitry Sankouski wrote:
> >>>> sdm845 has "General Purpose" clocks that can be muxed to
> >>>> SoC pins.
> >>>>
> >>>> Those clocks may be used as e.g. PWM sources for external peripherals.
> >>>> Add more frequencies to the table for those clocks so it's possible
> >>>> for arbitrary peripherals to make use of them.
> >>>>
> >>>> See also: bf8bb8eaccf(clk: qcom: gcc-msm8916: Add rates to the GP clocks)
> >>>
> >>> Each time I look at the table attached to the GP CLK, I feel that it's
> >>> plain wrong. In the end the GPCLK can in theory have arbitrary value
> >>> depending on the usecase.
> >>>
> >>> Bjorn, Konrad, maybe we should add special clk_ops for GP CLK which
> >>> allow more flexibility than a default clk_rcg2_ops?
> >>
> >> If we can somehow get max m/n/d values for all possible parents, sure
> >
> > Calculate them at runtime?
>
> We'd be calculating the mnd values for a frequency that's either equal or
> reasonably close to the one requested. My worry is that we somehow need
> to get the maximum values they can take (unless they're well-known)
>
> Konrad
Dzmitry Sankouski Aug. 12, 2024, 3:16 p.m. UTC | #7 
This is Proof-of-concept for general purpose clock ops. It's purpose is to
eliminate the need of freq_tbl for general purpose clock, by runtime
m/n/hid_div(pre-divisor) calculation. The calculation done as follows:
- upon determine rate request, we calculate m/n/hid_div as follows:
  - find parent(from our client's assigned-clock-parent) rate
  - find requested rate - parent rate highest common factor
  - find scaled rates by dividing rates on highest common factor
  - assign requested scaled rate to m
  - factorize scaled parent rate, put even multipliers to hid_div,
    odd to n (to maintain approximately same hid_div and n width)
- validate calculated values with *_width:
  - if doesn't fit, shift right accordingly
- return determined rate

In this POC, pwm-clk driver requests assigned-clock-parent with the call of
assigned-clock-parent function, so GP clocks don't need to configure parent.
__clk_rcg2_configure is therefore divided to __clk_rcg2_configure_parent
and __clk_rcg2_configure_mnd to reuse code.

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.

Signed-off-by: Dzmitry Sankouski <dsankouski@gmail.com>
---

 drivers/clk/qcom/clk-rcg.h    |   1 +
 drivers/clk/qcom/clk-rcg2.c   | 162 ++++++++++++++++++++++++++++++++--
 drivers/clk/qcom/gcc-sdm845.c |  12 ++-
 drivers/pwm/pwm-clk.c         |   5 ++
 4 files changed, 167 insertions(+), 13 deletions(-)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index d7414361e432..5bd86bce0c4d 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 30b19bd39d08..44b257481556 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -32,6 +32,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
@@ -393,16 +394,103 @@ 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)
+static inline u64 find_hcf(u64 a, u64 b)
+{
+	while (a != 0 && b != 0) {
+		if (a > b)
+			a %= b;
+		else
+			b %= a;
+	}
+	return a + b;
+}
+
+static int clk_calc_mnd(u64 parent_rate, u64 rate, struct freq_tbl *f)
+{
+	u64 hcf;
+	u64 hid_div = 1, n = 1;
+	int i = 2, count = 0;
+
+	hcf = find_hcf(parent_rate, rate);
+	u64 scaled_rate = rate / hcf;
+	u64 scaled_parent_rate = parent_rate / hcf;
+
+	while (scaled_parent_rate > 1) {
+		while (scaled_parent_rate % i == 0) {
+			scaled_parent_rate /= i;
+			if (count % 2 == 0)
+				hid_div *= i;
+			else
+				n *= i;
+		}
+		i++;
+		count++;
+	}
+
+	f->m = scaled_rate;
+	f->n = n;
+	f->pre_div = hid_div;
+
+	return 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 src = clk_rcg2_get_parent(hw);
+	int mnd_max = BIT(rcg->mnd_width) - 1;
+	int hid_max = BIT(rcg->hid_width) - 1;
+	u64 parent_rate;
+	int ret;
+
+	parent_rate = rcg->freq_tbl[src].freq;
+	f = kcalloc(MAX_PERF_LEVEL + 1, sizeof(f), GFP_KERNEL);
+
+	if (!f)
+		return 0;
+
+	ret = clk_calc_mnd(parent_rate, req->rate, f);
+	if (ret)
+		return 0;
+
+
+	while (f->n - f->m >= mnd_max) {
+		f->m = f->m >> 1;
+		f->n = f->n >> 1;
+	}
+	while (f->pre_div >= hid_max) {
+		f->pre_div = f->pre_div >> 1;
+		f->m = f->m >> 1;
+	}
+
+	req->rate = calc_rate(parent_rate, f->m, f->n, f->n, f->pre_div);
+
+	return 0;
+}
+
+static int __clk_rcg2_configure_parent(struct clk_rcg2 *rcg, int 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,
@@ -431,9 +519,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)
@@ -445,6 +532,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;
@@ -465,6 +568,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)
 {
@@ -518,6 +641,22 @@ 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);
+	struct freq_tbl *f;
+
+	f = kcalloc(MAX_PERF_LEVEL + 1, sizeof(*f), GFP_KERNEL);
+
+	if (!f)
+		return -ENOMEM;
+
+	clk_calc_mnd(parent_rate, rate, f);
+
+	return clk_rcg2_configure_gp(rcg, f);
+}
+
 static int clk_rcg2_set_floor_rate(struct clk_hw *hw, unsigned long rate,
 				   unsigned long parent_rate)
 {
@@ -645,6 +784,17 @@ 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,
+	.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..4567f405683b 100644
--- a/drivers/clk/qcom/gcc-sdm845.c
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -285,10 +285,8 @@ 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),
+	F(300000000, P_GPLL0_OUT_EVEN, 1, 0, 0),
+	F(600000000, P_GPLL0_OUT_MAIN, 1, 0, 0),
 	{ }
 };
 
@@ -302,7 +300,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 +314,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 +328,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,
 	},
 };
 
diff --git a/drivers/pwm/pwm-clk.c b/drivers/pwm/pwm-clk.c
index c19a482d7e28..1bfc7870e3aa 100644
--- a/drivers/pwm/pwm-clk.c
+++ b/drivers/pwm/pwm-clk.c
@@ -25,6 +25,7 @@
 #include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
+#include <linux/clk/clk-conf.h>
 #include <linux/pwm.h>
 
 struct pwm_clk_chip {
@@ -87,6 +88,10 @@ static int pwm_clk_probe(struct platform_device *pdev)
 	struct pwm_clk_chip *pcchip;
 	int ret;
 
+	ret = of_clk_set_defaults(pdev->dev.of_node, false);
+	if (ret < 0)
+		return -EINVAL;
+
 	chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*pcchip));
 	if (IS_ERR(chip))
 		return PTR_ERR(chip);
Stephen Boyd Aug. 12, 2024, 6:08 p.m. UTC | #8 
Quoting Dzmitry Sankouski (2024-08-12 08:16:06)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 30b19bd39d08..44b257481556 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -32,6 +32,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
> @@ -393,16 +394,103 @@ 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)
> +static inline u64 find_hcf(u64 a, u64 b)
> +{
> +       while (a != 0 && b != 0) {
> +               if (a > b)
> +                       a %= b;
> +               else
> +                       b %= a;
> +       }
> +       return a + b;

Is this gcd()?

> +}
> +
> +static int clk_calc_mnd(u64 parent_rate, u64 rate, struct freq_tbl *f)
> +{
> +       u64 hcf;
> +       u64 hid_div = 1, n = 1;
> +       int i = 2, count = 0;
> +
> +       hcf = find_hcf(parent_rate, rate);
> +       u64 scaled_rate = rate / hcf;
> +       u64 scaled_parent_rate = parent_rate / hcf;
> +
> +       while (scaled_parent_rate > 1) {
> +               while (scaled_parent_rate % i == 0) {
> +                       scaled_parent_rate /= i;
> +                       if (count % 2 == 0)
> +                               hid_div *= i;
> +                       else
> +                               n *= i;
> +               }
> +               i++;
> +               count++;
> +       }
> +
> +       f->m = scaled_rate;
> +       f->n = n;
> +       f->pre_div = hid_div;
> +
> +       return 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 src = clk_rcg2_get_parent(hw);
> +       int mnd_max = BIT(rcg->mnd_width) - 1;
> +       int hid_max = BIT(rcg->hid_width) - 1;
> +       u64 parent_rate;
> +       int ret;
> +
> +       parent_rate = rcg->freq_tbl[src].freq;
> +       f = kcalloc(MAX_PERF_LEVEL + 1, sizeof(f), GFP_KERNEL);

When is this freed? Determine rate can be called many times. Is that
supposed to be sizeof(*f)? Why so many frequency entries?


> +
> +       if (!f)
> +               return 0;
> +
> +       ret = clk_calc_mnd(parent_rate, req->rate, f);
> +       if (ret)
> +               return 0;
> +
> +
> +       while (f->n - f->m >= mnd_max) {
> +               f->m = f->m >> 1;
> +               f->n = f->n >> 1;
> +       }
> +       while (f->pre_div >= hid_max) {
> +               f->pre_div = f->pre_div >> 1;
> +               f->m = f->m >> 1;
> +       }
> +
> +       req->rate = calc_rate(parent_rate, f->m, f->n, f->n, f->pre_div);
> +
> +       return 0;
> +}
> +
> +static int __clk_rcg2_configure_parent(struct clk_rcg2 *rcg, int src, u32 *_cfg)

u8 src? Good to keep types consistent.

>  {
> -       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,
> @@ -431,9 +519,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)
> @@ -445,6 +532,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;
> @@ -465,6 +568,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)
>  {
> @@ -518,6 +641,22 @@ 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);
> +       struct freq_tbl *f;
> +
> +       f = kcalloc(MAX_PERF_LEVEL + 1, sizeof(*f), GFP_KERNEL);

When is this freed?

> +
> +       if (!f)
> +               return -ENOMEM;
> +
> +       clk_calc_mnd(parent_rate, rate, f);
> +
> +       return clk_rcg2_configure_gp(rcg, f);
> +}
> +
>  static int clk_rcg2_set_floor_rate(struct clk_hw *hw, unsigned long rate,
>                                    unsigned long parent_rate)
>  {
> @@ -645,6 +784,17 @@ 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,
> +       .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/pwm/pwm-clk.c b/drivers/pwm/pwm-clk.c
> index c19a482d7e28..1bfc7870e3aa 100644
> --- a/drivers/pwm/pwm-clk.c
> +++ b/drivers/pwm/pwm-clk.c
> @@ -25,6 +25,7 @@
>  #include <linux/of.h>
>  #include <linux/platform_device.h>
>  #include <linux/clk.h>
> +#include <linux/clk/clk-conf.h>
>  #include <linux/pwm.h>
>  
>  struct pwm_clk_chip {
> @@ -87,6 +88,10 @@ static int pwm_clk_probe(struct platform_device *pdev)
>         struct pwm_clk_chip *pcchip;
>         int ret;
>  
> +       ret = of_clk_set_defaults(pdev->dev.of_node, false);
> +       if (ret < 0)
> +               return -EINVAL;
> +

What is this? Debug code?

>         chip = devm_pwmchip_alloc(&pdev->dev, 1, sizeof(*pcchip));
>         if (IS_ERR(chip))
>                 return PTR_ERR(chip);
> -- 
> 2.39.2
> 
>
diff mbox series

Patch

diff --git a/drivers/clk/qcom/gcc-sdm845.c b/drivers/clk/qcom/gcc-sdm845.c
index ea4c3bf4fb9b..0efd3364e8f5 100644
--- a/drivers/clk/qcom/gcc-sdm845.c
+++ b/drivers/clk/qcom/gcc-sdm845.c
@@ -283,7 +283,21 @@  static struct clk_rcg2 gcc_sdm670_cpuss_rbcpr_clk_src = {
 	},
 };
 
+/*
+ * This is a frequency table for "General Purpose" clocks.
+ * These clocks can be muxed to the SoC pins and may be used by
+ * external devices. They're often used as PWM source.
+ *
+ * See comment in gcc-mam8916.c at ftbl_gcc_gp1_3_clk.
+ */
 static const struct freq_tbl ftbl_gcc_gp1_clk_src[] = {
+	F(10000,   P_BI_TCXO,    16,  1, 120),
+	F(20000,   P_BI_TCXO,    16,  1, 60),
+	F(100000,  P_BI_TCXO,    16,  1,  12),
+	F(500000,  P_GPLL0_OUT_EVEN, 12, 1, 100),
+	F(1000000, P_GPLL0_OUT_EVEN, 12, 1, 50),
+	F(2500000, P_GPLL0_OUT_EVEN, 12, 1, 10),
+	F(5000000, P_GPLL0_OUT_EVEN, 12, 1, 5),
 	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),