On 08-03-19, 17:47, Gregory CLEMENT wrote:
> From: Christian Neubert <christian.neubert.86@xxxxxxxxx>
>
> The clock parenting was not setup properly when DVFS was enabled. It was
> expected that the same clock source was used with and without DVFS which
> was not the case.
>
> This patch fixes this issue, allowing to make the cpufreq support work
> when the CPU clock source are not the default ones.
>
> Fixes: 92ce45fb875d ("cpufreq: Add DVFS support for Armada 37xx")
> Cc: <stable@xxxxxxxxxxxxxxx>
> [gregory: extract from a larger patch, modify comments and commit log]
> Signed-off-by: Christian Neubert <christian.neubert.86@xxxxxxxxx>
> Signed-off-by: Gregory CLEMENT <gregory.clement@xxxxxxxxxxx>
> ---
> drivers/cpufreq/armada-37xx-cpufreq.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/armada-37xx-cpufreq.c b/drivers/cpufreq/armada-37xx-cpufreq.c
> index 75491fc841a6..ad4463e4266e 100644
> --- a/drivers/cpufreq/armada-37xx-cpufreq.c
> +++ b/drivers/cpufreq/armada-37xx-cpufreq.c
> @@ -162,11 +162,25 @@ static void __init armada37xx_cpufreq_dvfs_setup(struct regmap *base,
> }
>
> /*
> - * Set cpu clock source, for all the level we keep the same
> - * clock source that the one already configured. For this one
> - * we need to use the clock framework
> + * Set CPU clock source, for all the level we keep the same
> + * clock source that the one already configured with DVS
DVFS ?
> + * disabled. For this one we need to use the clock framework
Please add a full-stop (.) at the end.
> */
> parent = clk_get_parent(clk);
> +
> + /*
> + * Unset parent clock to force the clock framework setting again
> + * the clock parent
> + */
> + clk_set_parent(clk, NULL);
> +
> + /*
> + * For the Armada 37xx CPU clocks, setting the parent will
> + * actually configure the parent when DVFS is enabled. At
> + * hardware level it will be a different register from the one
> + * read when doing clk_get_parent that will be set with
> + * clk_set_parent.
> + */
> clk_set_parent(clk, parent);
> }
Maybe this should be done right from the clock driver instead? As cpufreq may or
maynot be enabled by default (Surely most of the people will always enable it,
but I am just trying to find the right place for doing this). The way we are
setting the clock parent isn't that great, and looks a bit hacky just because of
the way clock framework is. Maybe doing it directly, without getting clock
framework in between, from the clock driver may look sane ?
--
viresh
