* [PATCH 1/2] clk: qcom: clk-krait: switch to .determine_rate
2023-02-12 14:11 [PATCH 0/2] Switch hfpll & krait clock drivers to .determine_rate Luca Weiss
@ 2023-02-12 14:11 ` Luca Weiss
2023-02-12 9:09 ` Christian Marangi
2023-02-12 14:11 ` [PATCH 2/2] clk: qcom: clk-hfpll: " Luca Weiss
2023-03-15 23:35 ` (subset) [PATCH 0/2] Switch hfpll & krait clock drivers " Bjorn Andersson
2 siblings, 1 reply; 6+ messages in thread
From: Luca Weiss @ 2023-02-12 14:11 UTC (permalink / raw)
To: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Christian Marangi
Cc: linux-arm-msm, linux-clk, linux-kernel, Luca Weiss
.determine_rate is meant to replace .round_rate. The former comes with a
benefit which is especially relevant on 32-bit systems: since
.determine_rate uses an "unsigned long" (compared to a "signed long"
which is used by .round_rate) the maximum value on 32-bit systems
increases from 2^31 (or approx. 2.14GHz) to 2^32 (or approx. 4.29GHz).
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
drivers/clk/qcom/clk-krait.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
index 293a9dfa7151..f5ce403e1e27 100644
--- a/drivers/clk/qcom/clk-krait.c
+++ b/drivers/clk/qcom/clk-krait.c
@@ -97,11 +97,11 @@ const struct clk_ops krait_mux_clk_ops = {
EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
/* The divider can divide by 2, 4, 6 and 8. But we only really need div-2. */
-static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long *parent_rate)
+static int krait_div2_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
{
- *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
- return DIV_ROUND_UP(*parent_rate, 2);
+ req->best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), req->rate * 2);
+ req->rate = DIV_ROUND_UP(req->best_parent_rate, 2);
+ return 0;
}
static int krait_div2_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -142,7 +142,7 @@ krait_div2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
}
const struct clk_ops krait_div2_clk_ops = {
- .round_rate = krait_div2_round_rate,
+ .determine_rate = krait_div2_determine_rate,
.set_rate = krait_div2_set_rate,
.recalc_rate = krait_div2_recalc_rate,
};
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] clk: qcom: clk-krait: switch to .determine_rate
2023-02-12 14:11 ` [PATCH 1/2] clk: qcom: clk-krait: switch " Luca Weiss
@ 2023-02-12 9:09 ` Christian Marangi
0 siblings, 0 replies; 6+ messages in thread
From: Christian Marangi @ 2023-02-12 9:09 UTC (permalink / raw)
To: Luca Weiss
Cc: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd,
linux-arm-msm, linux-clk, linux-kernel
On Sun, Feb 12, 2023 at 03:11:08PM +0100, Luca Weiss wrote:
> .determine_rate is meant to replace .round_rate. The former comes with a
> benefit which is especially relevant on 32-bit systems: since
> .determine_rate uses an "unsigned long" (compared to a "signed long"
> which is used by .round_rate) the maximum value on 32-bit systems
> increases from 2^31 (or approx. 2.14GHz) to 2^32 (or approx. 4.29GHz).
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Hi, found time to test this on ipq806x and seems to work correctly by
scaling to each freq and checking the hfpll reported freq.
Tested-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/clk/qcom/clk-krait.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-krait.c b/drivers/clk/qcom/clk-krait.c
> index 293a9dfa7151..f5ce403e1e27 100644
> --- a/drivers/clk/qcom/clk-krait.c
> +++ b/drivers/clk/qcom/clk-krait.c
> @@ -97,11 +97,11 @@ const struct clk_ops krait_mux_clk_ops = {
> EXPORT_SYMBOL_GPL(krait_mux_clk_ops);
>
> /* The divider can divide by 2, 4, 6 and 8. But we only really need div-2. */
> -static long krait_div2_round_rate(struct clk_hw *hw, unsigned long rate,
> - unsigned long *parent_rate)
> +static int krait_div2_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
> {
> - *parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), rate * 2);
> - return DIV_ROUND_UP(*parent_rate, 2);
> + req->best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), req->rate * 2);
> + req->rate = DIV_ROUND_UP(req->best_parent_rate, 2);
> + return 0;
> }
>
> static int krait_div2_set_rate(struct clk_hw *hw, unsigned long rate,
> @@ -142,7 +142,7 @@ krait_div2_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> }
>
> const struct clk_ops krait_div2_clk_ops = {
> - .round_rate = krait_div2_round_rate,
> + .determine_rate = krait_div2_determine_rate,
> .set_rate = krait_div2_set_rate,
> .recalc_rate = krait_div2_recalc_rate,
> };
>
> --
> 2.39.1
>
--
Ansuel
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] clk: qcom: clk-hfpll: switch to .determine_rate
2023-02-12 14:11 [PATCH 0/2] Switch hfpll & krait clock drivers to .determine_rate Luca Weiss
2023-02-12 14:11 ` [PATCH 1/2] clk: qcom: clk-krait: switch " Luca Weiss
@ 2023-02-12 14:11 ` Luca Weiss
2023-02-12 9:09 ` Christian Marangi
2023-03-15 23:35 ` (subset) [PATCH 0/2] Switch hfpll & krait clock drivers " Bjorn Andersson
2 siblings, 1 reply; 6+ messages in thread
From: Luca Weiss @ 2023-02-12 14:11 UTC (permalink / raw)
To: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Christian Marangi
Cc: linux-arm-msm, linux-clk, linux-kernel, Luca Weiss
.determine_rate is meant to replace .round_rate. The former comes with a
benefit which is especially relevant on 32-bit systems: since
.determine_rate uses an "unsigned long" (compared to a "signed long"
which is used by .round_rate) the maximum value on 32-bit systems
increases from 2^31 (or approx. 2.14GHz) to 2^32 (or approx. 4.29GHz).
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
drivers/clk/qcom/clk-hfpll.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
index 7dd17c184b69..86f728dc69e5 100644
--- a/drivers/clk/qcom/clk-hfpll.c
+++ b/drivers/clk/qcom/clk-hfpll.c
@@ -128,20 +128,20 @@ static void clk_hfpll_disable(struct clk_hw *hw)
spin_unlock_irqrestore(&h->lock, flags);
}
-static long clk_hfpll_round_rate(struct clk_hw *hw, unsigned long rate,
- unsigned long *parent_rate)
+static int clk_hfpll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
{
struct clk_hfpll *h = to_clk_hfpll(hw);
struct hfpll_data const *hd = h->d;
unsigned long rrate;
- rate = clamp(rate, hd->min_rate, hd->max_rate);
+ req->rate = clamp(req->rate, hd->min_rate, hd->max_rate);
- rrate = DIV_ROUND_UP(rate, *parent_rate) * *parent_rate;
+ rrate = DIV_ROUND_UP(req->rate, req->best_parent_rate) * req->best_parent_rate;
if (rrate > hd->max_rate)
- rrate -= *parent_rate;
+ rrate -= req->best_parent_rate;
- return rrate;
+ req->rate = rrate;
+ return 0;
}
/*
@@ -241,7 +241,7 @@ const struct clk_ops clk_ops_hfpll = {
.enable = clk_hfpll_enable,
.disable = clk_hfpll_disable,
.is_enabled = hfpll_is_enabled,
- .round_rate = clk_hfpll_round_rate,
+ .determine_rate = clk_hfpll_determine_rate,
.set_rate = clk_hfpll_set_rate,
.recalc_rate = clk_hfpll_recalc_rate,
.init = clk_hfpll_init,
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] clk: qcom: clk-hfpll: switch to .determine_rate
2023-02-12 14:11 ` [PATCH 2/2] clk: qcom: clk-hfpll: " Luca Weiss
@ 2023-02-12 9:09 ` Christian Marangi
0 siblings, 0 replies; 6+ messages in thread
From: Christian Marangi @ 2023-02-12 9:09 UTC (permalink / raw)
To: Luca Weiss
Cc: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Andy Gross, Konrad Dybcio, Michael Turquette, Stephen Boyd,
linux-arm-msm, linux-clk, linux-kernel
On Sun, Feb 12, 2023 at 03:11:09PM +0100, Luca Weiss wrote:
> .determine_rate is meant to replace .round_rate. The former comes with a
> benefit which is especially relevant on 32-bit systems: since
> .determine_rate uses an "unsigned long" (compared to a "signed long"
> which is used by .round_rate) the maximum value on 32-bit systems
> increases from 2^31 (or approx. 2.14GHz) to 2^32 (or approx. 4.29GHz).
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
Tested-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
> drivers/clk/qcom/clk-hfpll.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clk/qcom/clk-hfpll.c b/drivers/clk/qcom/clk-hfpll.c
> index 7dd17c184b69..86f728dc69e5 100644
> --- a/drivers/clk/qcom/clk-hfpll.c
> +++ b/drivers/clk/qcom/clk-hfpll.c
> @@ -128,20 +128,20 @@ static void clk_hfpll_disable(struct clk_hw *hw)
> spin_unlock_irqrestore(&h->lock, flags);
> }
>
> -static long clk_hfpll_round_rate(struct clk_hw *hw, unsigned long rate,
> - unsigned long *parent_rate)
> +static int clk_hfpll_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
> {
> struct clk_hfpll *h = to_clk_hfpll(hw);
> struct hfpll_data const *hd = h->d;
> unsigned long rrate;
>
> - rate = clamp(rate, hd->min_rate, hd->max_rate);
> + req->rate = clamp(req->rate, hd->min_rate, hd->max_rate);
>
> - rrate = DIV_ROUND_UP(rate, *parent_rate) * *parent_rate;
> + rrate = DIV_ROUND_UP(req->rate, req->best_parent_rate) * req->best_parent_rate;
> if (rrate > hd->max_rate)
> - rrate -= *parent_rate;
> + rrate -= req->best_parent_rate;
>
> - return rrate;
> + req->rate = rrate;
> + return 0;
> }
>
> /*
> @@ -241,7 +241,7 @@ const struct clk_ops clk_ops_hfpll = {
> .enable = clk_hfpll_enable,
> .disable = clk_hfpll_disable,
> .is_enabled = hfpll_is_enabled,
> - .round_rate = clk_hfpll_round_rate,
> + .determine_rate = clk_hfpll_determine_rate,
> .set_rate = clk_hfpll_set_rate,
> .recalc_rate = clk_hfpll_recalc_rate,
> .init = clk_hfpll_init,
>
> --
> 2.39.1
>
--
Ansuel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: (subset) [PATCH 0/2] Switch hfpll & krait clock drivers to .determine_rate
2023-02-12 14:11 [PATCH 0/2] Switch hfpll & krait clock drivers to .determine_rate Luca Weiss
2023-02-12 14:11 ` [PATCH 1/2] clk: qcom: clk-krait: switch " Luca Weiss
2023-02-12 14:11 ` [PATCH 2/2] clk: qcom: clk-hfpll: " Luca Weiss
@ 2023-03-15 23:35 ` Bjorn Andersson
2 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2023-03-15 23:35 UTC (permalink / raw)
To: Konrad Dybcio, phone-devel, Luca Weiss, Michael Turquette,
Stephen Boyd, ~postmarketos/upstreaming, Christian Marangi,
Andy Gross
Cc: linux-clk, linux-arm-msm, linux-kernel
On Sun, 12 Feb 2023 15:11:07 +0100, Luca Weiss wrote:
> While trying to get cpufreq working on msm8974 I've found an issue with
> clock rates above 2.11GHz (e.g. 2.15GHz). When a rate above this
> threshold gets requested the lowest possible frequency will be selected.
>
> This is caused by an overflow of the "long" return type of .round_rate
> which has a maximum value of 2147483647 (2.14GHz) on 32-bit systems,
> which msm8974 is.
>
> [...]
Applied, thanks!
[1/2] clk: qcom: clk-krait: switch to .determine_rate
commit: a7074c3eb26e0193f2c6ed79987e633b7578024e
[2/2] clk: qcom: clk-hfpll: switch to .determine_rate
commit: 04648b8fad219599ccc9b103188a38e72d339a3d
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
^ permalink raw reply [flat|nested] 6+ messages in thread