| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 26 Jul 2018 13:11:10 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Michael Turquette <mturquette@...libre.com>,
Taniya Das <tdas@...eaurora.org>
Cc: Andy Gross <andy.gross@...aro.org>,
David Brown <david.brown@...aro.org>,
Rajendra Nayak <rnayak@...eaurora.org>,
Amit Nischal <anischal@...eaurora.org>,
linux-arm-msm@...r.kernel.org, linux-soc@...r.kernel.org,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
Taniya Das <tdas@...eaurora.org>
Subject: Re: [PATCH v3 1/2] clk: qcom: Add support for RCG to register for DFS
Quoting Taniya Das (2018-07-15 22:37:47)
> diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c
> index 52208d4..f8f1417 100644
> --- a/drivers/clk/qcom/clk-rcg2.c
> +++ b/drivers/clk/qcom/clk-rcg2.c
> @@ -929,3 +938,167 @@ static void clk_rcg2_shared_disable(struct clk_hw *hw)
> .set_rate_and_parent = clk_rcg2_shared_set_rate_and_parent,
> };
> EXPORT_SYMBOL_GPL(clk_rcg2_shared_ops);
> +
> +/* Common APIs to be used for DFS based RCGR */
> +static unsigned long clk_rcg2_calculate_freq(struct clk_hw *hw,
> + int level, struct freq_tbl *f)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + struct clk_hw *p;
> + unsigned long prate = 0;
> + u32 val, mask, cfg, m_off, n_off, offset, mode;
> + int i, ret, num_parents;
> +
> + offset = SE_PERF_DFSR(level);
> + ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + offset, &cfg);
> + if (ret)
> + return ret;
> +
> + mask = BIT(rcg->hid_width) - 1;
> + f->pre_div = cfg & mask ? (cfg & mask) : 1;
> +
> + mode = cfg & CFG_MODE_MASK;
> + mode >>= CFG_MODE_SHIFT;
> +
> + cfg &= CFG_SRC_SEL_MASK;
> + cfg >>= CFG_SRC_SEL_SHIFT;
> +
> + num_parents = clk_hw_get_num_parents(hw);
> + for (i = 0; i < num_parents; i++) {
> + if (cfg == rcg->parent_map[i].cfg) {
> + f->src = rcg->parent_map[i].src;
> + p = clk_hw_get_parent_by_index(&rcg->clkr.hw, i);
> + prate = clk_hw_get_rate(p);
> + }
> + }
> +
> + if (!mode)
> + goto done;
Please remove the goto and indent all the below code when we need to do
the mode check.
> +
> + /* Calculate M & N values */
> + m_off = SE_PERF_M_DFSR(level);
> + n_off = SE_PERF_N_DFSR(level);
> +
> + mask = BIT(rcg->mnd_width) - 1;
> + ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + m_off, &val);
> + if (ret) {
> + pr_err("Failed to read M offset register\n");
> + return ret;
> + }
> + val &= mask;
> + f->m = val;
> +
> + ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + n_off, &val);
> + if (ret) {
> + pr_err("Failed to read N offset register\n");
> + return ret;
> + }
> + /* val ~(N-M) */
> + val = ~val;
> + val &= mask;
> + val += f->m;
> + f->n = val;
> +done:
> + return calc_rate(prate, f->m, f->n, mode, f->pre_div);
> +}
> +
> +static int clk_rcg2_dfs_populate_freq_table(struct clk_rcg2 *rcg)
> +{
> + struct freq_tbl *freq_tbl;
> + unsigned long calc_freq;
> + int i;
> +
> + freq_tbl = kcalloc(MAX_PERF_LEVEL, sizeof(*freq_tbl),
> + GFP_KERNEL);
> + if (!freq_tbl)
> + return -ENOMEM;
> +
> + for (i = 0; i < MAX_PERF_LEVEL; i++) {
> + calc_freq = clk_rcg2_calculate_freq(&rcg->clkr.hw,
> + i, &freq_tbl[i]);
> + if (calc_freq < 0) {
> + kfree(freq_tbl);
> + return calc_freq;
> + }
> +
> + freq_tbl[i].freq = calc_freq;
> + }
> + rcg->freq_tbl = freq_tbl;
> +
> + return 0;
> +}
> +
> +static int clk_rcg2_dfs_determine_rate(struct clk_hw *hw,
> + struct clk_rate_request *req)
> +{
> + struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> + int ret;
> +
> + if (!rcg->freq_tbl) {
> + ret = clk_rcg2_dfs_populate_freq_table(rcg);
> + if (ret) {
> + pr_err("Failed to update DFS tables for %s\n",
> + clk_hw_get_name(hw));
> + return ret;
> + }
> + }
> +
> + return clk_rcg2_shared_ops.determine_rate(hw, req);
> +}
> +
> +static const struct clk_ops clk_rcg2_dfs_ops = {
> + .is_enabled = clk_rcg2_is_enabled,
> + .get_parent = clk_rcg2_get_parent,
> + .determine_rate = clk_rcg2_dfs_determine_rate,
It seems very risky to do this without being able to read the frequency
out of the hardware. Taking a peek at the DFS registers I see that there
is some sort of 'CURR_PERF_STATE' field in the DFSR register. Wouldn't
that correspond to the index in the table that the clk is currently
using? Why can't we mark these clks as CLK_GET_RATE_NOCACHE and then
read this field when we're using DFS?
In fact, I just tested this on my device and I see that changing the DFS
bit on the QUP register that controls the DFS used causes the DFSR
register in GCC to update the CURR_PERF_STATE field to match what is
chosen in the QUP. So there is definitely a way to implement
recalc_rate() for this clk. Please do so.
> +};
> +
> +static int clk_rcg2_enable_dfs(struct clk_rcg2 *rcg, struct regmap *regmap)
> +{
> + struct clk_init_data *init;
> + u32 val;
> + int ret;
> +
> + ret = regmap_read(regmap, rcg->cmd_rcgr + SE_CMD_DFSR_OFFSET,
> + &val);
> + if (ret)
> + return -EINVAL;
> +
> + if (!(val & SE_CMD_DFS_EN))
> + return 0;
> +
> + init = kzalloc(sizeof(*init), GFP_KERNEL);
> + if (!init)
> + return -ENOMEM;
> +
> + init->name = rcg->clkr.hw.init->name;
> + init->flags = rcg->clkr.hw.init->flags;
> + init->parent_names = rcg->clkr.hw.init->parent_names;
> + init->num_parents = rcg->clkr.hw.init->num_parents;
> + init->ops = &clk_rcg2_dfs_ops;
> +
> + rcg->clkr.hw.init = init;
> + rcg->freq_tbl = NULL;
> +
> + pr_debug("DFS registered for clk %s\n", init->name);
Ok
> +
> + return 0;
> +}
> +
> +int qcom_cc_register_rcg_dfs(struct regmap *regmap,
> + struct clk_rcg2 **rcgs, int num_clks)
> +{
> + int i, ret = 0;
> +
> + for (i = 0; i < num_clks; i++) {
> + ret = clk_rcg2_enable_dfs(rcgs[i], regmap);
> + if (ret) {
> + const char *name = rcgs[i]->clkr.hw.init->name;
> +
> + pr_err("DFS register failed for clk %s\n", name);
> + break;
Just return ret please.
> + }
> + }
> +
> + return ret;
And return 0 here.
> +}
> +EXPORT_SYMBOL_GPL(qcom_cc_register_rcg_dfs);
Powered by blists - more mailing lists