Re: [PATCH v6 2/3] clk: qcom: clk-rcg2: add support for rcg2 freq multi ops

From: Devi Priya
Date: Tue Nov 21 2023 - 01:33:14 EST




On 11/20/2023 11:44 PM, Christian Marangi wrote:
On Mon, Nov 20, 2023 at 03:51:50PM +0530, Devi Priya wrote:


On 9/16/2023 7:30 PM, Christian Marangi wrote:
Some RCG frequency can be reached by multiple configuration.

Add clk_rcg2_fm_ops ops to support these special RCG configurations.

These alternative ops will select the frequency using a CEIL policy.

When the correct frequency is found, the correct config is selected by
calculating the final rate (by checking the defined parent and values
in the config that is being checked) and deciding based on the one that
is less different than the requested one.

These check are skipped if there is just on config for the requested
freq.

qcom_find_freq_multi is added to search the freq with the new struct
freq_multi_tbl.
__clk_rcg2_select_conf is used to select the correct conf by simulating
the final clock.
If a conf can't be found due to parent not reachable, a WARN is printed
and -EINVAL is returned.

Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
---
drivers/clk/qcom/clk-rcg.h | 1 +
drivers/clk/qcom/clk-rcg2.c | 167 ++++++++++++++++++++++++++++++++++++
drivers/clk/qcom/common.c | 18 ++++
drivers/clk/qcom/common.h | 2 +
4 files changed, 188 insertions(+)

diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index c81458db6ce4..dc9a77965e68 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -190,6 +190,7 @@ struct clk_rcg2_gfx3d {
extern const struct clk_ops clk_rcg2_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;
extern const struct clk_ops clk_edp_pixel_ops;
extern const struct clk_ops clk_byte_ops;
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
index e22baf3a7112..617e7ff0f6a3 100644
--- a/drivers/clk/qcom/clk-rcg2.c
+++ b/drivers/clk/qcom/clk-rcg2.c
@@ -266,6 +266,116 @@ static int _freq_tbl_determine_rate(struct clk_hw *hw, const struct freq_tbl *f,
return 0;
}
+static const struct freq_conf *
+__clk_rcg2_select_conf(struct clk_hw *hw, const struct freq_multi_tbl *f,
+ unsigned long req_rate)
+{
+ unsigned long rate_diff, best_rate_diff = ULONG_MAX;
+ const struct freq_conf *conf, *best_conf;
+ struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+ const char *name = clk_hw_get_name(hw);
+ unsigned long parent_rate, rate;
+ struct clk_hw *p;
+ int index, i;
+
+ /* Init best_conf to the first conf */
+ best_conf = f->confs;
+
+ /* Exit early if only one config is defined */
+ if (f->num_confs == 1)
+ goto exit;
+
+ /* Search in each provided config the one that is near the wanted rate */
+ for (i = 0, conf = f->confs; i < f->num_confs; i++, conf++) {
+ index = qcom_find_src_index(hw, rcg->parent_map, conf->src);
+ if (index < 0)
+ continue;
+
+ p = clk_hw_get_parent_by_index(hw, index);
+ if (!p)
+ continue;
+
+ parent_rate = clk_hw_get_rate(p);
+ rate = calc_rate(parent_rate, conf->n, conf->m, conf->n, conf->pre_div);
+
+ if (rate == req_rate) {
+ best_conf = conf;
+ goto exit;
+ }
+
+ rate_diff = abs(req_rate - rate);
+ if (rate_diff < best_rate_diff) {
+ best_rate_diff = rate_diff;
+ best_conf = conf;
+ }
+ }
+
+ /*
+ * Very unlikely. Warn if we couldn't find a correct config
+ * due to parent not found in every config.
+ */
+ if (unlikely(i == f->num_confs)) {
+ WARN(1, "%s: can't find a configuration for rate %lu.",
+ name, req_rate);
+ return ERR_PTR(-EINVAL);
+ }
Hi Christian,

Thanks a lot for the patch!
We have incorporated these changes along with the corresponding clock driver
changes & tested it on IPQ9574 & IPQ5332 targets.

When setting the clk rate for the nss port clocks, for the requested
frequency the correct config gets selected and the
clk rate is set properly.
We see the WARN getting printed for other frequencies (rate * i where
i=2 to maxdiv) that is requested by the clk_hw_round_rate function.

Upon analysis, we see that the for loop in clk_divider_bestdiv iterates
until the maxdiv value and requests (rate*i) via the clk_hw_round_rate
API to find the bestdiv and best_parent_rate. For frequencies which are
multiples of the requested frequency (rate*i where i=2 to maxdiv), it
seems unlikely to see the WARN being printed.

Can you please help us understand when the WARN is likely to be printed
& Looking forward to your suggestions on how this WARN could
be suppressed in the afore mentioned scenario!


Hi,

thanks a lot for testing this. Maybe was a small oversight by me.

I attached an alternative patch. Can you test it and tell me if the WARN
is still printed? (the WARN must be printed only when the parent is not
found, I don't think it's your case)

Hi Christian,

WARN does not get printed with the attached patchset.
Thanks much!

Regards,
Devi Priya