From 8fff5a3f7a3f85752a99a616aaae9cfad7fb8267 Mon Sep 17 00:00:00 2001 From: Mike Tipton Date: Tue, 3 May 2022 11:20:22 -0700 Subject: [PATCH] clk: qcom: clk-alpha-pll: Don't round up alpha value in HW PLLs with a narrow ALPHA can't hit all frequencies precisely and may be under by a few hundred Hz. When this happens we currently increment ALPHA by 1 in round_rate() to push the frequency slightly above the requested rate. However, when clk_set_rate() is called on a child of the PLL, then the final ALPHA will be ALPHA+N where N depends on the number of children in the chain. This is because clk_calc_new_rates() calls round_rate() recursively for each parent in the chain, each time using the previous iteration's ALPHA+1 based frequency. This results in a higher frequency than intended. The overall frequency delta is small and generally not a functional issue, but it's different than the HW recommendations and also causes certain frequency validation tests to fail. Additionally, this causes clk_propagate_rate_change() to short-circuit before notifying all children in some cases. If clk_set_rate() on a child results in a PLL frequency of ALPHA+N but the PLL is already configured to this rate, then clk_calc_new_rates() will return the PLL as the top clock, even though pll->rate == pll->new_rate. This causes clk_propagate_rate_change() to return immediately. This happens because clk_calc_new_rates() is recursively called for the parent whenever best_parent_rate != parent->rate. And since ALPHA isn't incremented to its final ALPHA+N value until the final parent iteration, then best_parent_rate is slightly lower than parent->rate. Returning early from clk_propagate_rate_change() can result in undervoltage, since we vote for increased voltage in the pre_rate_change() callback. To fix these issues, don't round up by incrementing the HW ALPHA value. Instead, keep the PLL configured to the slightly lower frequency and report its frequency as the rounded up version. Change-Id: I9aa7c1269735c07ef2524b08306edb6427af3629 Signed-off-by: Mike Tipton --- drivers/clk/qcom/clk-alpha-pll.c | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c index 3df042b60a51..9b12b174da48 100644 --- a/drivers/clk/qcom/clk-alpha-pll.c +++ b/drivers/clk/qcom/clk-alpha-pll.c @@ -542,7 +542,17 @@ static void clk_alpha_pll_disable(struct clk_hw *hw) static unsigned long alpha_pll_calc_rate(u64 prate, u32 l, u32 a, u32 alpha_width) { - return (prate * l) + ((prate * a) >> ALPHA_SHIFT(alpha_width)); + unsigned long rate; + + rate = (prate * l) + ((prate * a) >> ALPHA_SHIFT(alpha_width)); + + /* + * PLLs with narrow ALPHA (e.g. 16 bits) aren't able to hit all + * frequencies precisely and may be under by a few hundred Hz. Round to + * the nearest KHz to avoid reporting strange, slightly lower than + * requested frequencies. The small delta has no functional impact. + */ + return roundup(rate, 1000); } static unsigned long @@ -564,10 +574,7 @@ alpha_pll_round_rate(unsigned long rate, unsigned long prate, u32 *l, u64 *a, /* Upper ALPHA_BITWIDTH bits of Alpha */ quotient = remainder << ALPHA_SHIFT(alpha_width); - remainder = do_div(quotient, prate); - - if (remainder) - quotient++; + do_div(quotient, prate); *a = quotient; return alpha_pll_calc_rate(prate, *l, *a, alpha_width); @@ -749,7 +756,7 @@ alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a) if (a >= BIT(PLL_HUAYRA_ALPHA_WIDTH - 1)) l -= 1; - return (prate * l) + (prate * a >> PLL_HUAYRA_ALPHA_WIDTH); + return alpha_pll_calc_rate(prate, l, a, PLL_HUAYRA_ALPHA_WIDTH); } static unsigned long @@ -769,10 +776,7 @@ alpha_huayra_pll_round_rate(unsigned long rate, unsigned long prate, } quotient = remainder << PLL_HUAYRA_ALPHA_WIDTH; - remainder = do_div(quotient, prate); - - if (remainder) - quotient++; + do_div(quotient, prate); /* * alpha_val should be in two’s complement number in the range