| 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
| ||
|
Message-ID: <8b2c1a23fd55c52e56cc875660d1fba9.sboyd@kernel.org>
Date: Mon, 12 Aug 2024 11:08:14 -0700
From: Stephen Boyd <sboyd@...nel.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, Dzmitry Sankouski <dsankouski@...il.com>
Cc: dsankouski@...il.com, Konrad Dybcio <konrad.dybcio@...aro.org>, Bjorn Andersson <andersson@...nel.org>, Michael Turquette <mturquette@...libre.com>, linux-arm-msm@...r.kernel.org, linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 02/23] gcc-sdm845: Add rates to the GP clocks
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
>
>
Powered by blists - more mailing lists