This small series fix a current problem with ipq8074 where the 2 uniphy
port doesn't work in some corner case with some clk configuration. The
port to correctly work require a specific frequency, using the wrong one
results in the port not transmitting data.
With the current code with a requested freq of 125MHz, the frequency is
set to 105MHz. This is caused by the fact that there are 2 different
configuration to set 125MHz and it's always selected the first one that
results in 105MHz.
In the original QSDK code, the frequency configuration selection is
different and the CEIL FLOOR logic is not present. Instead it's used a
BEST approach where the frequency table is checked and then it's checked
if there are duplicate entry.
This proposed implementation is more specific and introduce an entire new
set of ops and a specific freq table to support this special configuration.
A union is introduced in rcg2 struct to not duplicate the struct.
A new set of ops clk_rcg2_fm_ops are introduced to support this new kind
of frequency table.
Changes v6:
- Small rework of best_conf selection to mute Sparse warn.
Changes v5:
- Rework selection logic with suggestion from Konrad
- Return -EINVAL and WARN if we fail to find a correct conf
Changes v4:
- Drop suggested but wrong re-search patch
- Move everything to separate ops and struct to not affect current rcg2
users.
Changes v3:
- Add qcom_find_freq_exact
- Drop re-search on rcg2_set_rate
- Rework multiple conf patch to follow new implementation
Changes v2:
- Out of RFC
- Fix compile warning from buildbot related to F redefinition
Christian Marangi (3):
clk: qcom: clk-rcg: introduce support for multiple conf for same freq
clk: qcom: clk-rcg2: add support for rcg2 freq multi ops
clk: qcom: gcc-ipq8074: rework nss_port5/6 clock to multiple conf
drivers/clk/qcom/clk-rcg.h | 24 ++++-
drivers/clk/qcom/clk-rcg2.c | 167 +++++++++++++++++++++++++++++++++
drivers/clk/qcom/common.c | 18 ++++
drivers/clk/qcom/common.h | 2 +
drivers/clk/qcom/gcc-ipq8074.c | 120 ++++++++++++++---------
5 files changed, 286 insertions(+), 45 deletions(-)
--
2.40.1
Rework nss_port5/6 to use the new multiple configuration implementation
and correctly fix the clocks for these port under some corner case.
This is particularly relevant for device that have 2.5G or 10G port
connected to port5 or port 6 on ipq8074. As the parent are shared
across multiple port it may be required to select the correct
configuration to accomplish the desired clock. Without this patch such
port doesn't work in some specific ethernet speed as the clock will be
set to the wrong frequency as we just select the first configuration for
the related frequency instead of selecting the best one.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/clk/qcom/gcc-ipq8074.c | 120 +++++++++++++++++++++------------
1 file changed, 76 insertions(+), 44 deletions(-)
diff --git a/drivers/clk/qcom/gcc-ipq8074.c b/drivers/clk/qcom/gcc-ipq8074.c
index 63ac2ced76bb..595cb7bd4834 100644
--- a/drivers/clk/qcom/gcc-ipq8074.c
+++ b/drivers/clk/qcom/gcc-ipq8074.c
@@ -1681,15 +1681,23 @@ static struct clk_regmap_div nss_port4_tx_div_clk_src = {
},
};
-static const struct freq_tbl ftbl_nss_port5_rx_clk_src[] = {
- F(19200000, P_XO, 1, 0, 0),
- F(25000000, P_UNIPHY1_RX, 12.5, 0, 0),
- F(25000000, P_UNIPHY0_RX, 5, 0, 0),
- F(78125000, P_UNIPHY1_RX, 4, 0, 0),
- F(125000000, P_UNIPHY1_RX, 2.5, 0, 0),
- F(125000000, P_UNIPHY0_RX, 1, 0, 0),
- F(156250000, P_UNIPHY1_RX, 2, 0, 0),
- F(312500000, P_UNIPHY1_RX, 1, 0, 0),
+static const struct freq_conf ftbl_nss_port5_rx_clk_src_25[] = {
+ C(P_UNIPHY1_RX, 12.5, 0, 0),
+ C(P_UNIPHY0_RX, 5, 0, 0),
+};
+
+static const struct freq_conf ftbl_nss_port5_rx_clk_src_125[] = {
+ C(P_UNIPHY1_RX, 2.5, 0, 0),
+ C(P_UNIPHY0_RX, 1, 0, 0),
+};
+
+static const struct freq_multi_tbl ftbl_nss_port5_rx_clk_src[] = {
+ FMS(19200000, P_XO, 1, 0, 0),
+ FM(25000000, ftbl_nss_port5_rx_clk_src_25),
+ FMS(78125000, P_UNIPHY1_RX, 4, 0, 0),
+ FM(125000000, ftbl_nss_port5_rx_clk_src_125),
+ FMS(156250000, P_UNIPHY1_RX, 2, 0, 0),
+ FMS(312500000, P_UNIPHY1_RX, 1, 0, 0),
{ }
};
@@ -1716,14 +1724,14 @@ gcc_xo_uniphy0_rx_tx_uniphy1_rx_tx_ubi32_bias_map[] = {
static struct clk_rcg2 nss_port5_rx_clk_src = {
.cmd_rcgr = 0x68060,
- .freq_tbl = ftbl_nss_port5_rx_clk_src,
+ .freq_multi_tbl = ftbl_nss_port5_rx_clk_src,
.hid_width = 5,
.parent_map = gcc_xo_uniphy0_rx_tx_uniphy1_rx_tx_ubi32_bias_map,
.clkr.hw.init = &(struct clk_init_data){
.name = "nss_port5_rx_clk_src",
.parent_data = gcc_xo_uniphy0_rx_tx_uniphy1_rx_tx_ubi32_bias,
.num_parents = ARRAY_SIZE(gcc_xo_uniphy0_rx_tx_uniphy1_rx_tx_ubi32_bias),
- .ops = &clk_rcg2_ops,
+ .ops = &clk_rcg2_fm_ops,
},
};
@@ -1743,15 +1751,23 @@ static struct clk_regmap_div nss_port5_rx_div_clk_src = {
},
};
-static const struct freq_tbl ftbl_nss_port5_tx_clk_src[] = {
- F(19200000, P_XO, 1, 0, 0),
- F(25000000, P_UNIPHY1_TX, 12.5, 0, 0),
- F(25000000, P_UNIPHY0_TX, 5, 0, 0),
- F(78125000, P_UNIPHY1_TX, 4, 0, 0),
- F(125000000, P_UNIPHY1_TX, 2.5, 0, 0),
- F(125000000, P_UNIPHY0_TX, 1, 0, 0),
- F(156250000, P_UNIPHY1_TX, 2, 0, 0),
- F(312500000, P_UNIPHY1_TX, 1, 0, 0),
+static const struct freq_conf ftbl_nss_port5_tx_clk_src_25[] = {
+ C(P_UNIPHY1_TX, 12.5, 0, 0),
+ C(P_UNIPHY0_TX, 5, 0, 0),
+};
+
+static const struct freq_conf ftbl_nss_port5_tx_clk_src_125[] = {
+ C(P_UNIPHY1_TX, 2.5, 0, 0),
+ C(P_UNIPHY0_TX, 1, 0, 0),
+};
+
+static const struct freq_multi_tbl ftbl_nss_port5_tx_clk_src[] = {
+ FMS(19200000, P_XO, 1, 0, 0),
+ FM(25000000, ftbl_nss_port5_tx_clk_src_25),
+ FMS(78125000, P_UNIPHY1_TX, 4, 0, 0),
+ FM(125000000, ftbl_nss_port5_tx_clk_src_125),
+ FMS(156250000, P_UNIPHY1_TX, 2, 0, 0),
+ FMS(312500000, P_UNIPHY1_TX, 1, 0, 0),
{ }
};
@@ -1778,14 +1794,14 @@ gcc_xo_uniphy0_tx_rx_uniphy1_tx_rx_ubi32_bias_map[] = {
static struct clk_rcg2 nss_port5_tx_clk_src = {
.cmd_rcgr = 0x68068,
- .freq_tbl = ftbl_nss_port5_tx_clk_src,
+ .freq_multi_tbl = ftbl_nss_port5_tx_clk_src,
.hid_width = 5,
.parent_map = gcc_xo_uniphy0_tx_rx_uniphy1_tx_rx_ubi32_bias_map,
.clkr.hw.init = &(struct clk_init_data){
.name = "nss_port5_tx_clk_src",
.parent_data = gcc_xo_uniphy0_tx_rx_uniphy1_tx_rx_ubi32_bias,
.num_parents = ARRAY_SIZE(gcc_xo_uniphy0_tx_rx_uniphy1_tx_rx_ubi32_bias),
- .ops = &clk_rcg2_ops,
+ .ops = &clk_rcg2_fm_ops,
},
};
@@ -1805,15 +1821,23 @@ static struct clk_regmap_div nss_port5_tx_div_clk_src = {
},
};
-static const struct freq_tbl ftbl_nss_port6_rx_clk_src[] = {
- F(19200000, P_XO, 1, 0, 0),
- F(25000000, P_UNIPHY2_RX, 5, 0, 0),
- F(25000000, P_UNIPHY2_RX, 12.5, 0, 0),
- F(78125000, P_UNIPHY2_RX, 4, 0, 0),
- F(125000000, P_UNIPHY2_RX, 1, 0, 0),
- F(125000000, P_UNIPHY2_RX, 2.5, 0, 0),
- F(156250000, P_UNIPHY2_RX, 2, 0, 0),
- F(312500000, P_UNIPHY2_RX, 1, 0, 0),
+static const struct freq_conf ftbl_nss_port6_rx_clk_src_25[] = {
+ C(P_UNIPHY2_RX, 5, 0, 0),
+ C(P_UNIPHY2_RX, 12.5, 0, 0),
+};
+
+static const struct freq_conf ftbl_nss_port6_rx_clk_src_125[] = {
+ C(P_UNIPHY2_RX, 1, 0, 0),
+ C(P_UNIPHY2_RX, 2.5, 0, 0),
+};
+
+static const struct freq_multi_tbl ftbl_nss_port6_rx_clk_src[] = {
+ FMS(19200000, P_XO, 1, 0, 0),
+ FM(25000000, ftbl_nss_port6_rx_clk_src_25),
+ FMS(78125000, P_UNIPHY2_RX, 4, 0, 0),
+ FM(125000000, ftbl_nss_port6_rx_clk_src_125),
+ FMS(156250000, P_UNIPHY2_RX, 2, 0, 0),
+ FMS(312500000, P_UNIPHY2_RX, 1, 0, 0),
{ }
};
@@ -1835,14 +1859,14 @@ static const struct parent_map gcc_xo_uniphy2_rx_tx_ubi32_bias_map[] = {
static struct clk_rcg2 nss_port6_rx_clk_src = {
.cmd_rcgr = 0x68070,
- .freq_tbl = ftbl_nss_port6_rx_clk_src,
+ .freq_multi_tbl = ftbl_nss_port6_rx_clk_src,
.hid_width = 5,
.parent_map = gcc_xo_uniphy2_rx_tx_ubi32_bias_map,
.clkr.hw.init = &(struct clk_init_data){
.name = "nss_port6_rx_clk_src",
.parent_data = gcc_xo_uniphy2_rx_tx_ubi32_bias,
.num_parents = ARRAY_SIZE(gcc_xo_uniphy2_rx_tx_ubi32_bias),
- .ops = &clk_rcg2_ops,
+ .ops = &clk_rcg2_fm_ops,
},
};
@@ -1862,15 +1886,23 @@ static struct clk_regmap_div nss_port6_rx_div_clk_src = {
},
};
-static const struct freq_tbl ftbl_nss_port6_tx_clk_src[] = {
- F(19200000, P_XO, 1, 0, 0),
- F(25000000, P_UNIPHY2_TX, 5, 0, 0),
- F(25000000, P_UNIPHY2_TX, 12.5, 0, 0),
- F(78125000, P_UNIPHY2_TX, 4, 0, 0),
- F(125000000, P_UNIPHY2_TX, 1, 0, 0),
- F(125000000, P_UNIPHY2_TX, 2.5, 0, 0),
- F(156250000, P_UNIPHY2_TX, 2, 0, 0),
- F(312500000, P_UNIPHY2_TX, 1, 0, 0),
+static const struct freq_conf ftbl_nss_port6_tx_clk_src_25[] = {
+ C(P_UNIPHY2_TX, 5, 0, 0),
+ C(P_UNIPHY2_TX, 12.5, 0, 0),
+};
+
+static const struct freq_conf ftbl_nss_port6_tx_clk_src_125[] = {
+ C(P_UNIPHY2_TX, 1, 0, 0),
+ C(P_UNIPHY2_TX, 2.5, 0, 0),
+};
+
+static const struct freq_multi_tbl ftbl_nss_port6_tx_clk_src[] = {
+ FMS(19200000, P_XO, 1, 0, 0),
+ FM(25000000, ftbl_nss_port6_tx_clk_src_25),
+ FMS(78125000, P_UNIPHY1_RX, 4, 0, 0),
+ FM(125000000, ftbl_nss_port6_tx_clk_src_125),
+ FMS(156250000, P_UNIPHY1_RX, 2, 0, 0),
+ FMS(312500000, P_UNIPHY1_RX, 1, 0, 0),
{ }
};
@@ -1892,14 +1924,14 @@ static const struct parent_map gcc_xo_uniphy2_tx_rx_ubi32_bias_map[] = {
static struct clk_rcg2 nss_port6_tx_clk_src = {
.cmd_rcgr = 0x68078,
- .freq_tbl = ftbl_nss_port6_tx_clk_src,
+ .freq_multi_tbl = ftbl_nss_port6_tx_clk_src,
.hid_width = 5,
.parent_map = gcc_xo_uniphy2_tx_rx_ubi32_bias_map,
.clkr.hw.init = &(struct clk_init_data){
.name = "nss_port6_tx_clk_src",
.parent_data = gcc_xo_uniphy2_tx_rx_ubi32_bias,
.num_parents = ARRAY_SIZE(gcc_xo_uniphy2_tx_rx_ubi32_bias),
- .ops = &clk_rcg2_ops,
+ .ops = &clk_rcg2_fm_ops,
},
};
--
2.40.1
Some RCG frequency can be reached by multiple configuration.
We currently declare multiple configuration for the same frequency but
that is not supported and always the first configuration will be taken.
These multiple configuration are needed as based on the current parent
configuration, it may be needed to use a different configuration to
reach the same frequency.
To handle this introduce 3 new macro, C, FM and FMS:
- C is used to declare a freq_conf where src, pre_div, m and n are
provided.
- FM is used to declare a freq_multi_tbl with the frequency and an
array of confs to insert all the config for the provided frequency.
- FMS is used to declare a freq_multi_tbl with the frequency and an
array of a single conf with the provided src, pre_div, m and n.
Struct clk_rcg2 is changed to add a union type to reference a simple
freq_tbl or a complex freq_multi_tbl.
Signed-off-by: Christian Marangi <[email protected]>
---
drivers/clk/qcom/clk-rcg.h | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/qcom/clk-rcg.h b/drivers/clk/qcom/clk-rcg.h
index e6d84c8c7989..c81458db6ce4 100644
--- a/drivers/clk/qcom/clk-rcg.h
+++ b/drivers/clk/qcom/clk-rcg.h
@@ -17,6 +17,23 @@ struct freq_tbl {
u16 n;
};
+#define C(s, h, m, n) { (s), (2 * (h) - 1), (m), (n) }
+#define FM(f, confs) { (f), ARRAY_SIZE(confs), (confs) }
+#define FMS(f, s, h, m, n) { (f), 1, (const struct freq_conf []){ C(s, h, m, n) } }
+
+struct freq_conf {
+ u8 src;
+ u8 pre_div;
+ u16 m;
+ u16 n;
+};
+
+struct freq_multi_tbl {
+ unsigned long freq;
+ int num_confs;
+ const struct freq_conf *confs;
+};
+
/**
* struct mn - M/N:D counter
* @mnctr_en_bit: bit to enable mn counter
@@ -138,6 +155,7 @@ extern const struct clk_ops clk_dyn_rcg_ops;
* @safe_src_index: safe src index value
* @parent_map: map from software's parent index to hardware's src_sel field
* @freq_tbl: frequency table
+ * @freq_multi_tbl: frequency table for clocks reachable with multiple RCGs conf
* @clkr: regmap clock handle
* @cfg_off: defines the cfg register offset from the CMD_RCGR + CFG_REG
* @parked_cfg: cached value of the CFG register for parked RCGs
@@ -149,7 +167,10 @@ struct clk_rcg2 {
u8 hid_width;
u8 safe_src_index;
const struct parent_map *parent_map;
- const struct freq_tbl *freq_tbl;
+ union {
+ const struct freq_tbl *freq_tbl;
+ const struct freq_multi_tbl *freq_multi_tbl;
+ };
struct clk_regmap clkr;
u8 cfg_off;
u32 parked_cfg;
--
2.40.1
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 <[email protected]>
---
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);
+ }
+
+exit:
+ return best_conf;
+}
+
+static int _freq_tbl_fm_determine_rate(struct clk_hw *hw, const struct freq_multi_tbl *f,
+ struct clk_rate_request *req)
+{
+ unsigned long clk_flags, rate = req->rate;
+ struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+ const struct freq_conf *conf;
+ struct clk_hw *p;
+ int index;
+
+ f = qcom_find_freq_multi(f, rate);
+ if (!f || !f->confs)
+ return -EINVAL;
+
+ conf = __clk_rcg2_select_conf(hw, f, rate);
+ if (IS_ERR(conf))
+ return PTR_ERR(conf);
+ index = qcom_find_src_index(hw, rcg->parent_map, conf->src);
+ if (index < 0)
+ return index;
+
+ clk_flags = clk_hw_get_flags(hw);
+ p = clk_hw_get_parent_by_index(hw, index);
+ if (!p)
+ return -EINVAL;
+
+ if (clk_flags & CLK_SET_RATE_PARENT) {
+ rate = f->freq;
+ if (conf->pre_div) {
+ if (!rate)
+ rate = req->rate;
+ rate /= 2;
+ rate *= conf->pre_div + 1;
+ }
+
+ if (conf->n) {
+ u64 tmp = rate;
+
+ tmp = tmp * conf->n;
+ do_div(tmp, conf->m);
+ rate = tmp;
+ }
+ } else {
+ rate = clk_hw_get_rate(p);
+ }
+
+ req->best_parent_hw = p;
+ req->best_parent_rate = rate;
+ req->rate = f->freq;
+
+ return 0;
+}
+
static int clk_rcg2_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
@@ -282,6 +392,14 @@ static int clk_rcg2_determine_floor_rate(struct clk_hw *hw,
return _freq_tbl_determine_rate(hw, rcg->freq_tbl, req, FLOOR);
}
+static int clk_rcg2_fm_determine_rate(struct clk_hw *hw,
+ struct clk_rate_request *req)
+{
+ struct clk_rcg2 *rcg = to_clk_rcg2(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)
{
@@ -377,6 +495,30 @@ static int __clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
return clk_rcg2_configure(rcg, f);
}
+static int __clk_rcg2_fm_set_rate(struct clk_hw *hw, unsigned long rate)
+{
+ struct clk_rcg2 *rcg = to_clk_rcg2(hw);
+ const struct freq_multi_tbl *f;
+ const struct freq_conf *conf;
+ struct freq_tbl f_tbl;
+
+ f = qcom_find_freq_multi(rcg->freq_multi_tbl, rate);
+ if (!f || !f->confs)
+ return -EINVAL;
+
+ conf = __clk_rcg2_select_conf(hw, f, rate);
+ if (IS_ERR(conf))
+ return PTR_ERR(conf);
+
+ f_tbl.freq = f->freq;
+ f_tbl.src = conf->src;
+ f_tbl.pre_div = conf->pre_div;
+ f_tbl.m = conf->m;
+ f_tbl.n = conf->n;
+
+ return clk_rcg2_configure(rcg, &f_tbl);
+}
+
static int clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
unsigned long parent_rate)
{
@@ -389,6 +531,12 @@ static int clk_rcg2_set_floor_rate(struct clk_hw *hw, unsigned long rate,
return __clk_rcg2_set_rate(hw, rate, FLOOR);
}
+static int clk_rcg2_fm_set_rate(struct clk_hw *hw, unsigned long rate,
+ unsigned long parent_rate)
+{
+ return __clk_rcg2_fm_set_rate(hw, rate);
+}
+
static int clk_rcg2_set_rate_and_parent(struct clk_hw *hw,
unsigned long rate, unsigned long parent_rate, u8 index)
{
@@ -401,6 +549,12 @@ static int clk_rcg2_set_floor_rate_and_parent(struct clk_hw *hw,
return __clk_rcg2_set_rate(hw, rate, FLOOR);
}
+static int clk_rcg2_fm_set_rate_and_parent(struct clk_hw *hw,
+ unsigned long rate, unsigned long parent_rate, u8 index)
+{
+ return __clk_rcg2_fm_set_rate(hw, rate);
+}
+
static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
{
struct clk_rcg2 *rcg = to_clk_rcg2(hw);
@@ -511,6 +665,19 @@ const struct clk_ops clk_rcg2_floor_ops = {
};
EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
+const struct clk_ops clk_rcg2_fm_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_fm_determine_rate,
+ .set_rate = clk_rcg2_fm_set_rate,
+ .set_rate_and_parent = clk_rcg2_fm_set_rate_and_parent,
+ .get_duty_cycle = clk_rcg2_get_duty_cycle,
+ .set_duty_cycle = clk_rcg2_set_duty_cycle,
+};
+EXPORT_SYMBOL_GPL(clk_rcg2_fm_ops);
+
const struct clk_ops clk_rcg2_mux_closest_ops = {
.determine_rate = __clk_mux_determine_rate_closest,
.get_parent = clk_rcg2_get_parent,
diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
index 75f09e6e057e..48f81e3a5e80 100644
--- a/drivers/clk/qcom/common.c
+++ b/drivers/clk/qcom/common.c
@@ -41,6 +41,24 @@ struct freq_tbl *qcom_find_freq(const struct freq_tbl *f, unsigned long rate)
}
EXPORT_SYMBOL_GPL(qcom_find_freq);
+const struct freq_multi_tbl *qcom_find_freq_multi(const struct freq_multi_tbl *f,
+ unsigned long rate)
+{
+ if (!f)
+ return NULL;
+
+ if (!f->freq)
+ return f;
+
+ for (; f->freq; f++)
+ if (rate <= f->freq)
+ return f;
+
+ /* Default to our fastest rate */
+ return f - 1;
+}
+EXPORT_SYMBOL_GPL(qcom_find_freq_multi);
+
const struct freq_tbl *qcom_find_freq_floor(const struct freq_tbl *f,
unsigned long rate)
{
diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
index 9c8f7b798d9f..2d4a8a837e6c 100644
--- a/drivers/clk/qcom/common.h
+++ b/drivers/clk/qcom/common.h
@@ -45,6 +45,8 @@ extern const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f,
unsigned long rate);
extern const struct freq_tbl *qcom_find_freq_floor(const struct freq_tbl *f,
unsigned long rate);
+extern const struct freq_multi_tbl *qcom_find_freq_multi(const struct freq_multi_tbl *f,
+ unsigned long rate);
extern void
qcom_pll_set_fsm_mode(struct regmap *m, u32 reg, u8 bias_count, u8 lock_count);
extern int qcom_find_src_index(struct clk_hw *hw, const struct parent_map *map,
--
2.40.1
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 <[email protected]>
> ---
> 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!
Thanks,
Devi Priya
> +
> +exit:
> + return best_conf;
> +}
> +
> +static int _freq_tbl_fm_determine_rate(struct clk_hw *hw, const struct freq_multi_tbl *f,
> + struct clk_rate_request *req)
> +{
> + unsigned long clk_flags, rate = req->rate;
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + const struct freq_conf *conf;
> + struct clk_hw *p;
> + int index;
> +
> + f = qcom_find_freq_multi(f, rate);
> + if (!f || !f->confs)
> + return -EINVAL;
> +
> + conf = __clk_rcg2_select_conf(hw, f, rate);
> + if (IS_ERR(conf))
> + return PTR_ERR(conf);
> + index = qcom_find_src_index(hw, rcg->parent_map, conf->src);
> + if (index < 0)
> + return index;
> +
> + clk_flags = clk_hw_get_flags(hw);
> + p = clk_hw_get_parent_by_index(hw, index);
> + if (!p)
> + return -EINVAL;
> +
> + if (clk_flags & CLK_SET_RATE_PARENT) {
> + rate = f->freq;
> + if (conf->pre_div) {
> + if (!rate)
> + rate = req->rate;
> + rate /= 2;
> + rate *= conf->pre_div + 1;
> + }
> +
> + if (conf->n) {
> + u64 tmp = rate;
> +
> + tmp = tmp * conf->n;
> + do_div(tmp, conf->m);
> + rate = tmp;
> + }
> + } else {
> + rate = clk_hw_get_rate(p);
> + }
> +
> + req->best_parent_hw = p;
> + req->best_parent_rate = rate;
> + req->rate = f->freq;
> +
> + return 0;
> +}
> +
> static int clk_rcg2_determine_rate(struct clk_hw *hw,
> struct clk_rate_request *req)
> {
> @@ -282,6 +392,14 @@ static int clk_rcg2_determine_floor_rate(struct clk_hw *hw,
> return _freq_tbl_determine_rate(hw, rcg->freq_tbl, req, FLOOR);
> }
>
> +static int clk_rcg2_fm_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(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)
> {
> @@ -377,6 +495,30 @@ static int __clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
> return clk_rcg2_configure(rcg, f);
> }
>
> +static int __clk_rcg2_fm_set_rate(struct clk_hw *hw, unsigned long rate)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + const struct freq_multi_tbl *f;
> + const struct freq_conf *conf;
> + struct freq_tbl f_tbl;
> +
> + f = qcom_find_freq_multi(rcg->freq_multi_tbl, rate);
> + if (!f || !f->confs)
> + return -EINVAL;
> +
> + conf = __clk_rcg2_select_conf(hw, f, rate);
> + if (IS_ERR(conf))
> + return PTR_ERR(conf);
> +
> + f_tbl.freq = f->freq;
> + f_tbl.src = conf->src;
> + f_tbl.pre_div = conf->pre_div;
> + f_tbl.m = conf->m;
> + f_tbl.n = conf->n;
> +
> + return clk_rcg2_configure(rcg, &f_tbl);
> +}
> +
> static int clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,
> unsigned long parent_rate)
> {
> @@ -389,6 +531,12 @@ static int clk_rcg2_set_floor_rate(struct clk_hw *hw, unsigned long rate,
> return __clk_rcg2_set_rate(hw, rate, FLOOR);
> }
>
> +static int clk_rcg2_fm_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + return __clk_rcg2_fm_set_rate(hw, rate);
> +}
> +
> static int clk_rcg2_set_rate_and_parent(struct clk_hw *hw,
> unsigned long rate, unsigned long parent_rate, u8 index)
> {
> @@ -401,6 +549,12 @@ static int clk_rcg2_set_floor_rate_and_parent(struct clk_hw *hw,
> return __clk_rcg2_set_rate(hw, rate, FLOOR);
> }
>
> +static int clk_rcg2_fm_set_rate_and_parent(struct clk_hw *hw,
> + unsigned long rate, unsigned long parent_rate, u8 index)
> +{
> + return __clk_rcg2_fm_set_rate(hw, rate);
> +}
> +
> static int clk_rcg2_get_duty_cycle(struct clk_hw *hw, struct clk_duty *duty)
> {
> struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> @@ -511,6 +665,19 @@ const struct clk_ops clk_rcg2_floor_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_rcg2_floor_ops);
>
> +const struct clk_ops clk_rcg2_fm_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_fm_determine_rate,
> + .set_rate = clk_rcg2_fm_set_rate,
> + .set_rate_and_parent = clk_rcg2_fm_set_rate_and_parent,
> + .get_duty_cycle = clk_rcg2_get_duty_cycle,
> + .set_duty_cycle = clk_rcg2_set_duty_cycle,
> +};
> +EXPORT_SYMBOL_GPL(clk_rcg2_fm_ops);
> +
> const struct clk_ops clk_rcg2_mux_closest_ops = {
> .determine_rate = __clk_mux_determine_rate_closest,
> .get_parent = clk_rcg2_get_parent,
> diff --git a/drivers/clk/qcom/common.c b/drivers/clk/qcom/common.c
> index 75f09e6e057e..48f81e3a5e80 100644
> --- a/drivers/clk/qcom/common.c
> +++ b/drivers/clk/qcom/common.c
> @@ -41,6 +41,24 @@ struct freq_tbl *qcom_find_freq(const struct freq_tbl *f, unsigned long rate)
> }
> EXPORT_SYMBOL_GPL(qcom_find_freq);
>
> +const struct freq_multi_tbl *qcom_find_freq_multi(const struct freq_multi_tbl *f,
> + unsigned long rate)
> +{
> + if (!f)
> + return NULL;
> +
> + if (!f->freq)
> + return f;
> +
> + for (; f->freq; f++)
> + if (rate <= f->freq)
> + return f;
> +
> + /* Default to our fastest rate */
> + return f - 1;
> +}
> +EXPORT_SYMBOL_GPL(qcom_find_freq_multi);
> +
> const struct freq_tbl *qcom_find_freq_floor(const struct freq_tbl *f,
> unsigned long rate)
> {
> diff --git a/drivers/clk/qcom/common.h b/drivers/clk/qcom/common.h
> index 9c8f7b798d9f..2d4a8a837e6c 100644
> --- a/drivers/clk/qcom/common.h
> +++ b/drivers/clk/qcom/common.h
> @@ -45,6 +45,8 @@ extern const struct freq_tbl *qcom_find_freq(const struct freq_tbl *f,
> unsigned long rate);
> extern const struct freq_tbl *qcom_find_freq_floor(const struct freq_tbl *f,
> unsigned long rate);
> +extern const struct freq_multi_tbl *qcom_find_freq_multi(const struct freq_multi_tbl *f,
> + unsigned long rate);
> extern void
> qcom_pll_set_fsm_mode(struct regmap *m, u32 reg, u8 bias_count, u8 lock_count);
> extern int qcom_find_src_index(struct clk_hw *hw, const struct parent_map *map,
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 <[email protected]>
> > ---
> > 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)
--
Ansuel
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 <[email protected]>
>>> ---
>>> 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
On Tue, Nov 21, 2023 at 12:01:15PM +0530, Devi Priya wrote:
>
>
> 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 <[email protected]>
> > > > ---
> > > > 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!
>
Can you also confirm that the code correctly works with your div
scenario?
Would love to have some Tested-by tag to move this further!
(since it seems newer SoC after ipq807x will use this implementation
even more)
--
Ansuel
On 11/21/2023 7:25 PM, Christian Marangi wrote:
>> Hi Christian,
>>
>> WARN does not get printed with the attached patchset.
>> Thanks much!
>>
> Can you also confirm that the code correctly works with your div
> scenario?
>
> Would love to have some Tested-by tag to move this further!
>
> (since it seems newer SoC after ipq807x will use this implementation
> even more)
Sorry was not available for a couple of days.
Sure, will validate and add tested-by tag.
Thanks,
Devi Priya
>
> --