clk: qcom: Add support for RCG to register for DFS
Subsystems (3)
PATCH 1/1 clk: qcom: Add support for RCG to register for DFS
In the cases where a RCG requires a Dynamic Frequency switch support requires to register which would at runtime read the clock perf level registers to identify the frequencies supported and update the frequency table accordingly.
--- drivers/clk/qcom/clk-rcg.h | 7 +- drivers/clk/qcom/clk-rcg2.c | 172 ++++++++++++++++++++++++++++++++++++++++++++ drivers/clk/qcom/common.c | 23 +++++- drivers/clk/qcom/common.h | 14 +++- 4 files changed, 213 insertions(+), 3 deletions(-)
drivers/clk/qcom/clk-rcg.h +6 -1
@@ -1,5 +1,5 @@/* -* Copyright (c) 2013, The Linux Foundation. All rights reserved. +* Copyright (c) 2013, 2018, The Linux Foundation. All rights reserved. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and
@@ -144,6 +144,7 @@ struct clk_dyn_rcg {* @cmd_rcgr: corresponds to *_CMD_RCGR * @mnd_width: number of bits in m/n/d values * @hid_width: number of bits in half integer divider +* @flags: additional flag parameters for the RCG * @parent_map: map from software's parent index to hardware's src_sel field * @freq_tbl: frequency table * @clkr: regmap clock handle
@@ -153,6 +154,8 @@ struct clk_rcg2 {u32 cmd_rcgr;u8 mnd_width;u8 hid_width; +u8 flags; +#define DFS_ENABLE_RCG BIT(0)const struct parent_map *parent_map;const struct freq_tbl *freq_tbl;struct clk_regmap clkr;
@@ -168,4 +171,6 @@ struct clk_rcg2 {extern const struct clk_ops clk_pixel_ops;extern const struct clk_ops clk_gfx3d_ops; +extern int clk_rcg2_get_dfs_clock_rate (struct clk_rcg2 *clk, +struct device *dev);#endif
drivers/clk/qcom/clk-rcg2.c +172 -0
199 lines not on this page · raw message
drivers/clk/qcom/common.c +22 -1
@@ -1,5 +1,5 @@/* -* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved. +* Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights reserved. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and
@@ -288,4 +288,25 @@ int qcom_cc_probe(struct platform_device *pdev, const struct qcom_cc_desc *desc) }EXPORT_SYMBOL_GPL (qcom_cc_probe); +int qcom_cc_register_rcg_dfs (struct platform_device *pdev, +const struct qcom_cc_dfs_desc *desc) +{ +struct clk_dfs *clks = desc->clks; +size_t num_clks = desc->num_clks; +int i, ret =0 ; + +for (i =0 ; i < num_clks; i++) { + ret =clk_rcg2_get_dfs_clock_rate (clks[i].rcg, &pdev->dev); +if (ret) { +dev_err (&pdev->dev, +"Failed calculating DFS frequencies for %s\n" , +clk_hw_get_name (&(clks[i].rcg)->clkr.hw)); +break ; + } + } + +return ret; +} +EXPORT_SYMBOL (qcom_cc_register_rcg_dfs); +MODULE_LICENSE ("GPL v2" );
drivers/clk/qcom/common.h +13 -1
@@ -1,5 +1,5 @@/* -* Copyright (c) 2014, The Linux Foundation. All rights reserved. +* Copyright (c) 2014, 2018, The Linux Foundation. All rights reserved. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and
@@ -48,6 +48,15 @@ struct parent_map {u8 cfg; }; +struct clk_dfs { +struct clk_rcg2 *rcg; +}; + +struct qcom_cc_dfs_desc { +struct clk_dfs *clks; +size_t num_clks; +}; +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,
@@ -69,4 +78,7 @@ extern int qcom_cc_really_probe(struct platform_device *pdev,extern int qcom_cc_probe (struct platform_device *pdev,const struct qcom_cc_desc *desc); +extern int qcom_cc_register_rcg_dfs (struct platform_device *pdev, +const struct qcom_cc_dfs_desc *desc); +#endif
--
Qualcomm INDIA, on behalf of Qualcomm Innovation Center, Inc.is a member of the Code Aurora Forum, hosted by the Linux Foundation.
Hi Taniya,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on clk/clk-next]
[also build test WARNING on v4.17-rc3 next-20180503]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Taniya-Das/clk-qcom-Add-support-for-RCG-to-register-for-DFS/20180503-220112 base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
smatch warnings:drivers/clk/qcom/clk-rcg2.c:820 clk_parent_index_pre_div_and_mode() warn: signedness bug returning '(-22)'
# https://github.com/0day-ci/linux/commit/ac73368dea3e89ee63025d4da64dd908bcde367e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout ac73368dea3e89ee63025d4da64dd908bcde367e
vim +820 drivers/clk/qcom/clk-rcg2.c
ac73368d Taniya Das 2018-05-03 809
ac73368d Taniya Das 2018-05-03 810 /* Common APIs to be used for DFS based RCGR */
ac73368d Taniya Das 2018-05-03 811 static u8 clk_parent_index_pre_div_and_mode(struct clk_hw *hw, u32 offset,
ac73368d Taniya Das 2018-05-03 812 u32 *mode, u32 *pre_div)
ac73368d Taniya Das 2018-05-03 813 {
ac73368d Taniya Das 2018-05-03 814 struct clk_rcg2 *rcg;
ac73368d Taniya Das 2018-05-03 815 int num_parents;
ac73368d Taniya Das 2018-05-03 816 u32 cfg, mask;
ac73368d Taniya Das 2018-05-03 817 int i, ret;
ac73368d Taniya Das 2018-05-03 818
ac73368d Taniya Das 2018-05-03 819 if (!hw)
ac73368d Taniya Das 2018-05-03 @820 return -EINVAL;
ac73368d Taniya Das 2018-05-03 821
ac73368d Taniya Das 2018-05-03 822 num_parents = clk_hw_get_num_parents(hw);
ac73368d Taniya Das 2018-05-03 823
ac73368d Taniya Das 2018-05-03 824 rcg = to_clk_rcg2(hw);
ac73368d Taniya Das 2018-05-03 825
ac73368d Taniya Das 2018-05-03 826 ret = regmap_read(rcg->clkr.regmap, rcg->cmd_rcgr + offset, &cfg);
ac73368d Taniya Das 2018-05-03 827 if (ret)
ac73368d Taniya Das 2018-05-03 828 goto err;
ac73368d Taniya Das 2018-05-03 829
ac73368d Taniya Das 2018-05-03 830 mask = BIT(rcg->hid_width) - 1;
ac73368d Taniya Das 2018-05-03 831 *pre_div = cfg & mask ? (cfg & mask) : 1;
ac73368d Taniya Das 2018-05-03 832
ac73368d Taniya Das 2018-05-03 833 *mode = cfg & CFG_MODE_MASK;
ac73368d Taniya Das 2018-05-03 834 *mode >>= CFG_MODE_SHIFT;
ac73368d Taniya Das 2018-05-03 835
ac73368d Taniya Das 2018-05-03 836 cfg &= CFG_SRC_SEL_MASK;
ac73368d Taniya Das 2018-05-03 837 cfg >>= CFG_SRC_SEL_SHIFT;
ac73368d Taniya Das 2018-05-03 838
ac73368d Taniya Das 2018-05-03 839 for (i = 0; i < num_parents; i++)
ac73368d Taniya Das 2018-05-03 840 if (cfg == rcg->parent_map[i].cfg)
ac73368d Taniya Das 2018-05-03 841 return i;
ac73368d Taniya Das 2018-05-03 842 err:
ac73368d Taniya Das 2018-05-03 843 return 0;
ac73368d Taniya Das 2018-05-03 844 }
ac73368d Taniya Das 2018-05-03 845 Stephen Boyd
maintainerQuoting Taniya Das (2018-05-02 21:12:32)
Quoted 16 lines
drivers/clk/qcom/clk-rcg2.c +4
-0@@ -122,6 +131,10 @@ static int clk_rcg2_set_parent(struct clk_hw *hw, u8 index)int ret;u32 cfg = rcg->parent_map[index].cfg <<CFG_SRC_SEL_SHIFT ; +/* In DFS mode skip updating the RCG CFG */ +if (rcg->flags &DFS_ENABLE_RCG ) +return 0 ; + ret =regmap_update_bits (rcg->clkr.regmap, rcg->cmd_rcgr +CFG_REG ,CFG_SRC_SEL_MASK , cfg);if (ret)
drivers/clk/qcom/clk-rcg2.c static int __clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate, · line 314
Quoted 6 lines
drivers/clk/qcom/clk-rcg2.c +3
-0@@ -296,6 +309,9 @@ static int __clk_rcg2_set_rate(struct clk_hw *hw, unsigned long rate,struct clk_rcg2 *rcg =to_clk_rcg2 (hw);const struct freq_tbl *f; +if (rcg->flags &DFS_ENABLE_RCG ) +return -EPERM ; +
Please no.
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 811
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +3
-0switch (policy) {case FLOOR : f =qcom_find_freq_floor (rcg->freq_tbl, rate);@@ -790,3 +806,159 @@ static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, .determine_rate = clk_gfx3d_determine_rate, };EXPORT_SYMBOL_GPL (clk_gfx3d_ops); + +/* Common APIs to be used for DFS based RCGR */ +static u8 clk_parent_index_pre_div_and_mode(struct clk_hw *hw, u32 offset,
Fix return type please.
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 820
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c +9
-0+ u32 *mode, u32 *pre_div) +{ +struct clk_rcg2 *rcg; +int num_parents; +u32 cfg, mask; +int i, ret; + +if (!hw) +return -EINVAL ;
Why?
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 826
Quoted 6 lines
drivers/clk/qcom/clk-rcg2.c +6
-0+ + num_parents =clk_hw_get_num_parents (hw); + + rcg =to_clk_rcg2 (hw); + + ret =regmap_read (rcg->clkr.regmap, rcg->cmd_rcgr + offset, &cfg);
We may need to pass the regmap into here so this can be done before the clk is registered so that we can switch out the clk_ops at runtime. But that may not be the case if my other comment makes sense further down.
Quoted 11 lines
drivers/clk/qcom/clk-rcg2.c +11
-0+if (ret) + goto err; + + mask =BIT (rcg->hid_width) -1 ; + *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 ;
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 847
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+ +for (i =0 ; i < num_parents; i++) +if (cfg == rcg->parent_map[i].cfg) +return i; +err: +return 0 ; +} + +static int calculate_m_and_n (struct clk_hw *hw,u32 m_offset,u32 n_offset, +u32 mode,u32 *m,u32 *n)
Instead of returning pointers, please have these functions update a frequency table entry structure.
Quoted 40 lines
drivers/clk/qcom/clk-rcg2.c +40
-0+{ +struct clk_rcg2 *rcg =to_clk_rcg2 (hw); +u32 val, mask; +int ret =0 ; + +if (!hw) +return -EINVAL ; + + *m = *n =0 ; + +if (mode) { +/* Calculate M & N values */ + mask =BIT (rcg->mnd_width) -1 ; + ret =regmap_read (rcg->clkr.regmap, rcg->cmd_rcgr + m_offset, + &val); +if (ret) { +pr_err ("Failed to read M offset register\n" ); + goto err; + } + + val &= mask; + *m = val; + + ret =regmap_read (rcg->clkr.regmap, rcg->cmd_rcgr + n_offset, + &val); +if (ret) { +pr_err ("Failed to read N offset register\n" ); + goto err; + } + +/* val ~(N-M) */ + val = ~val; + val &= mask; + val += *m; + *n = val; + } +err: +return ret; +} +
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 897
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+int clk_rcg2_get_dfs_clock_rate (struct clk_rcg2 *clk,struct device *dev) +{ +int i, j, index, ret =0 ; +unsigned long calc_freq, prate; +u32 val, pre_div =0 , mode =0 , m =0 , n =0 ; +struct freq_tbl *dfs_freq_tbl; +struct clk_hw *phw; + +if (!clk) +return -EINVAL ;
Never happens?
Quoted 4 lines
drivers/clk/qcom/clk-rcg2.c +4
-0+ +/* Check for DFS_EN */ + ret =regmap_read (clk->clkr.regmap, clk->cmd_rcgr +SE_CMD_DFSR_OFFSET , + &val);
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 911
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+if (ret) { +dev_err (dev,"Failed to read DFS enable register\n" ); +return -EINVAL ; + } + +if (!(val &SE_CMD_DFS_EN )) +return ret; + + dfs_freq_tbl =devm_kzalloc (dev,MAX_PERF_LEVEL * +sizeof (struct freq_tbl ),GFP_KERNEL );
Use devm_kcalloc() for numbers of elements in an array.
Quoted 13 lines
drivers/clk/qcom/clk-rcg2.c +13
-0+if (!dfs_freq_tbl) +return -ENOMEM ; + +/* Populate the Perf Level */ +for (i =0 ; i <MAX_PERF_LEVEL ; i++) { +/* Get parent index and mode */ + index =clk_parent_index_pre_div_and_mode (&clk->clkr.hw, +SE_PERF_DFSR (i), &mode, + &pre_div); +if (index <0 ) { +pr_err ("Failed to get parent index & mode %d\n" , index); +return index; + }
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 934
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+ +/* clock pre_div */ + dfs_freq_tbl[i].pre_div = pre_div; + +/* Fill the parent src */ + dfs_freq_tbl[i].src = clk->parent_map[index].src; + +/* Get the parent clock and parent rate */ + phw =clk_hw_get_parent_by_index (&clk->clkr.hw, index); + prate =clk_hw_get_rate (phw);
Urgh this is sort of annoying. We'll need the parents to be registered when the frequency table is created, just so we can parse the parent rate out and calculate the final frequency? What do we do if the parent isn't registered yet? We may need to push this logic into the determine rate path, and calculate the frequency table lazily at that point and hope that the parent is registered with some frequency. Otherwise fail the determine rate call.
Quoted 14 lines
drivers/clk/qcom/clk-rcg2.c +14
-0+ + ret =calculate_m_and_n (&clk->clkr.hw,SE_PERF_M_DFSR (i), +SE_PERF_N_DFSR (i), mode, &m, &n); +if (ret) + goto err; + + dfs_freq_tbl[i].m = m; + dfs_freq_tbl[i].n = n; + +/* calculate the final frequency */ + calc_freq =calc_rate (prate, dfs_freq_tbl[i].m, + dfs_freq_tbl[i].n, mode, + dfs_freq_tbl[i].pre_div); +
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 958
Quoted 10 lines
drivers/clk/qcom/clk-rcg2.c +10
-0+/* Check for duplicate frequencies */ +for (j =0 ; j < i; j++) { +if (dfs_freq_tbl[j].freq == calc_freq) + goto done; + } + + dfs_freq_tbl[i].freq = calc_freq; + } +done: + j = i;
Why do we care about j = i? It isn't used now.
drivers/clk/qcom/clk-rcg2.c static int clk_gfx3d_set_rate(struct clk_hw *hw, unsigned long rate, · line 960
Quoted 2 lines
drivers/clk/qcom/clk-rcg2.c +2
-0+ + clk->flags |=DFS_ENABLE_RCG ;
I'd rather the ops are replaced with different clk_ops for DFS explicitly. The flag is not very nice.
Quoted 9 lines
drivers/clk/qcom/clk-rcg2.c +4
-0+ clk->freq_tbl = dfs_freq_tbl; +err: +return ret; +}drivers/clk/qcom/common.c +0
-0
drivers/clk/qcom/common.c line 5
Quoted 6 lines
drivers/clk/qcom/common.c +1
-1@@ -1,5 +1,5 @@/* -* Copyright (c) 2013-2014, The Linux Foundation. All rights reserved. +* Copyright (c) 2013-2014, 2018, The Linux Foundation. All rights reserved. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and
SPDX update?
Quoted 7 lines
drivers/clk/qcom/common.c +3
-0@@ -288,4 +288,25 @@ int qcom_cc_probe(struct platform_device *pdev, const struct qcom_cc_desc *desc) }EXPORT_SYMBOL_GPL (qcom_cc_probe); +int qcom_cc_register_rcg_dfs (struct platform_device *pdev, +const struct qcom_cc_dfs_desc *desc) +{
drivers/clk/qcom/common.c int qcom_cc_probe(struct platform_device *pdev, const struct qcom_cc_desc *desc) · line 303
Quoted 10 lines
drivers/clk/qcom/common.c +10
-0+struct clk_dfs *clks = desc->clks; +size_t num_clks = desc->num_clks; +int i, ret =0 ; + +for (i =0 ; i < num_clks; i++) { + ret =clk_rcg2_get_dfs_clock_rate (clks[i].rcg, &pdev->dev); +if (ret) { +dev_err (&pdev->dev, +"Failed calculating DFS frequencies for %s\n" , +clk_hw_get_name (&(clks[i].rcg)->clkr.hw));
That is a long line. Preferably iterate a local variable 'rcg' that is set to the start of the array and ++ through this loop.
Quoted 21 lines
drivers/clk/qcom/common.c +8
-0+break ; + } + } + +return ret; +} +EXPORT_SYMBOL (qcom_cc_register_rcg_dfs); +MODULE_LICENSE ("GPL v2" );drivers/clk/qcom/common.h +1
-1@@ -1,5 +1,5 @@/* -* Copyright (c) 2014, The Linux Foundation. All rights reserved. +* Copyright (c) 2014, 2018, The Linux Foundation. All rights reserved. * * This software is licensed under the terms of the GNU General Public * License version 2, as published by the Free Software Foundation, and
drivers/clk/qcom/common.h struct parent_map { · line 53
Quoted 6 lines
drivers/clk/qcom/common.h +3
-0@@ -48,6 +48,15 @@ struct parent_map {u8 cfg; }; +struct clk_dfs { +struct clk_rcg2 *rcg; +};
Just have an array of clk_rcg2's instead of this struct? I'd add it to the common descriptor structure too, because I'm going to guess this is common going forward.