From: sebastian.hesselbarth@gmail.com (Sebastian Hesselbarth)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/4] clk: mvebu: Fix clk frequency value if SSCG is enabled
Date: Fri, 29 Aug 2014 14:48:36 +0200 [thread overview]
Message-ID: <540076A4.5000303@gmail.com> (raw)
In-Reply-To: <1409312620-20631-2-git-send-email-gregory.clement@free-electrons.com>
On 08/29/2014 01:43 PM, Gregory CLEMENT wrote:
> When the SSCG (Spread Spectrum Clock Generator) is enabled, it shifts
> the frequency of the clock. The percentage is no more than 1% but when
> the clock is used for a timer it leads to a clock drift.
>
> This patch allows to correct the affected clock when the SSCG is
> enabled. The check is done in an new optional function related to each
> SoC: is_sscg_enabled(). If this function is not present then no
> correction is done on the clock frequency.
Gregory,
I think computing the SSCG inside the common clk part is a no-go. I
flipped through KW, Dove, Armada 370, and Armada XP FS looking at
SSCG config register layout. Guess what: Dove is different.
How about we have a .get_sscg_deviation() callback instead and move
the one below to some common place where KW, Armada 370, and XP can
set it for their callback struct?
Sebastian
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clk/mvebu/common.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/mvebu/common.h | 1 +
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> index 25ceccf939ad..834d36cf79b0 100644
> --- a/drivers/clk/mvebu/common.c
> +++ b/drivers/clk/mvebu/common.c
> @@ -26,8 +26,78 @@
> * Core Clocks
> */
>
> +#define SSCG_CONF_MODE(reg) (((reg) >> 16) & 0x3)
> +#define SSCG_SPREAD_DOWN 0x0
> +#define SSCG_SPREAD_UP 0x1
> +#define SSCG_SPREAD_CENTRAL 0x2
> +#define SSCG_CONF_LOW(reg) (((reg) >> 8) & 0xFF)
> +#define SSCG_CONF_HIGH(reg) ((reg) & 0xFF)
> +
> static struct clk_onecell_data clk_data;
>
> +static u32 fix_sscg_deviation(struct device_node *np, u32 system_clk)
> +{
> + struct device_node *sscg_np = NULL;
> + void __iomem *sscg_map;
> + u32 sscg_reg;
> + s32 low_bound, high_bound;
> + u64 freq_swing_half;
> +
> + sscg_np = of_find_node_by_name(np, "sscg");
> + if (sscg_np == NULL) {
> + pr_err("cannot get SSCG register node\n");
> + return system_clk;
> + }
> +
> + sscg_map = of_iomap(sscg_np, 0);
> + if (sscg_map == NULL) {
> + pr_err("cannot map SSCG register\n");
> + goto out;
> + }
> +
> + sscg_reg = readl(sscg_map);
> + high_bound = SSCG_CONF_HIGH(sscg_reg);
> + low_bound = SSCG_CONF_LOW(sscg_reg);
> +
> + if ((high_bound - low_bound) <= 0)
> + goto out;
> + /*
> + * From the datasheet we got the following formula
> + * Spread percentage = 1/96 * (H - L) / H
> + * H = SSCG_High_Boundary
> + * L = SSCG_Low_Boundary
> + *
> + * As the deviation is half of spread then it lead to the
> + * following formula in the code.
> + *
> + * To avoid an overflow and not lose any significant digit in
> + * the same time we have to use a 64 bit integer.
> + */
> +
> + freq_swing_half = (((u64)high_bound - (u64)low_bound)
> + * (u64)system_clk);
> + do_div(freq_swing_half, (2 * 96 * high_bound));
> +
> + switch (SSCG_CONF_MODE(sscg_reg)) {
> + case SSCG_SPREAD_DOWN:
> + system_clk -= freq_swing_half;
> + break;
> + case SSCG_SPREAD_UP:
> + system_clk += freq_swing_half;
> + break;
> + case SSCG_SPREAD_CENTRAL:
> + default:
> + break;
> + }
> +
> + iounmap(sscg_map);
> +
> +out:
> + of_node_put(sscg_np);
> +
> + return system_clk;
> +}
> +
> void __init mvebu_coreclk_setup(struct device_node *np,
> const struct coreclk_soc_desc *desc)
> {
> @@ -62,6 +132,10 @@ void __init mvebu_coreclk_setup(struct device_node *np,
> of_property_read_string_index(np, "clock-output-names", 1,
> &cpuclk_name);
> rate = desc->get_cpu_freq(base);
> +
> + if (desc->is_sscg_enabled && desc->is_sscg_enabled(base))
> + rate = fix_sscg_deviation(np, rate);
> +
> clk_data.clks[1] = clk_register_fixed_rate(NULL, cpuclk_name, NULL,
> CLK_IS_ROOT, rate);
> WARN_ON(IS_ERR(clk_data.clks[1]));
> diff --git a/drivers/clk/mvebu/common.h b/drivers/clk/mvebu/common.h
> index f968b4d9df92..495f94ff4c90 100644
> --- a/drivers/clk/mvebu/common.h
> +++ b/drivers/clk/mvebu/common.h
> @@ -28,6 +28,7 @@ struct coreclk_soc_desc {
> u32 (*get_tclk_freq)(void __iomem *sar);
> u32 (*get_cpu_freq)(void __iomem *sar);
> void (*get_clk_ratio)(void __iomem *sar, int id, int *mult, int *div);
> + bool (*is_sscg_enabled)(void __iomem *sar);
> const struct coreclk_ratio *ratios;
> int num_ratios;
> };
>
WARNING: multiple messages have this Message-ID (diff)
From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
To: Gregory CLEMENT <gregory.clement@free-electrons.com>,
Mike Turquette <mturquette@linaro.org>,
linux-kernel@vger.kernel.org
Cc: Jason Cooper <jason@lakedaemon.net>, Andrew Lunn <andrew@lunn.ch>,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Ezequiel Garcia <ezequiel.garcia@free-electrons.com>,
linux-arm-kernel@lists.infradead.org,
Lior Amsalem <alior@marvell.com>,
Tawfik Bayouk <tawfik@marvell.com>,
Nadav Haklai <nadavh@marvell.com>,
Raphael Rigo <ml-arm@syscall.eu>,
Arnaud Ebalard <arno@natisbad.org>,
Simon Boulay <simon.boulay@vitec.com>
Subject: Re: [PATCH 1/4] clk: mvebu: Fix clk frequency value if SSCG is enabled
Date: Fri, 29 Aug 2014 14:48:36 +0200 [thread overview]
Message-ID: <540076A4.5000303@gmail.com> (raw)
In-Reply-To: <1409312620-20631-2-git-send-email-gregory.clement@free-electrons.com>
On 08/29/2014 01:43 PM, Gregory CLEMENT wrote:
> When the SSCG (Spread Spectrum Clock Generator) is enabled, it shifts
> the frequency of the clock. The percentage is no more than 1% but when
> the clock is used for a timer it leads to a clock drift.
>
> This patch allows to correct the affected clock when the SSCG is
> enabled. The check is done in an new optional function related to each
> SoC: is_sscg_enabled(). If this function is not present then no
> correction is done on the clock frequency.
Gregory,
I think computing the SSCG inside the common clk part is a no-go. I
flipped through KW, Dove, Armada 370, and Armada XP FS looking at
SSCG config register layout. Guess what: Dove is different.
How about we have a .get_sscg_deviation() callback instead and move
the one below to some common place where KW, Armada 370, and XP can
set it for their callback struct?
Sebastian
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> drivers/clk/mvebu/common.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
> drivers/clk/mvebu/common.h | 1 +
> 2 files changed, 75 insertions(+)
>
> diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> index 25ceccf939ad..834d36cf79b0 100644
> --- a/drivers/clk/mvebu/common.c
> +++ b/drivers/clk/mvebu/common.c
> @@ -26,8 +26,78 @@
> * Core Clocks
> */
>
> +#define SSCG_CONF_MODE(reg) (((reg) >> 16) & 0x3)
> +#define SSCG_SPREAD_DOWN 0x0
> +#define SSCG_SPREAD_UP 0x1
> +#define SSCG_SPREAD_CENTRAL 0x2
> +#define SSCG_CONF_LOW(reg) (((reg) >> 8) & 0xFF)
> +#define SSCG_CONF_HIGH(reg) ((reg) & 0xFF)
> +
> static struct clk_onecell_data clk_data;
>
> +static u32 fix_sscg_deviation(struct device_node *np, u32 system_clk)
> +{
> + struct device_node *sscg_np = NULL;
> + void __iomem *sscg_map;
> + u32 sscg_reg;
> + s32 low_bound, high_bound;
> + u64 freq_swing_half;
> +
> + sscg_np = of_find_node_by_name(np, "sscg");
> + if (sscg_np == NULL) {
> + pr_err("cannot get SSCG register node\n");
> + return system_clk;
> + }
> +
> + sscg_map = of_iomap(sscg_np, 0);
> + if (sscg_map == NULL) {
> + pr_err("cannot map SSCG register\n");
> + goto out;
> + }
> +
> + sscg_reg = readl(sscg_map);
> + high_bound = SSCG_CONF_HIGH(sscg_reg);
> + low_bound = SSCG_CONF_LOW(sscg_reg);
> +
> + if ((high_bound - low_bound) <= 0)
> + goto out;
> + /*
> + * From the datasheet we got the following formula
> + * Spread percentage = 1/96 * (H - L) / H
> + * H = SSCG_High_Boundary
> + * L = SSCG_Low_Boundary
> + *
> + * As the deviation is half of spread then it lead to the
> + * following formula in the code.
> + *
> + * To avoid an overflow and not lose any significant digit in
> + * the same time we have to use a 64 bit integer.
> + */
> +
> + freq_swing_half = (((u64)high_bound - (u64)low_bound)
> + * (u64)system_clk);
> + do_div(freq_swing_half, (2 * 96 * high_bound));
> +
> + switch (SSCG_CONF_MODE(sscg_reg)) {
> + case SSCG_SPREAD_DOWN:
> + system_clk -= freq_swing_half;
> + break;
> + case SSCG_SPREAD_UP:
> + system_clk += freq_swing_half;
> + break;
> + case SSCG_SPREAD_CENTRAL:
> + default:
> + break;
> + }
> +
> + iounmap(sscg_map);
> +
> +out:
> + of_node_put(sscg_np);
> +
> + return system_clk;
> +}
> +
> void __init mvebu_coreclk_setup(struct device_node *np,
> const struct coreclk_soc_desc *desc)
> {
> @@ -62,6 +132,10 @@ void __init mvebu_coreclk_setup(struct device_node *np,
> of_property_read_string_index(np, "clock-output-names", 1,
> &cpuclk_name);
> rate = desc->get_cpu_freq(base);
> +
> + if (desc->is_sscg_enabled && desc->is_sscg_enabled(base))
> + rate = fix_sscg_deviation(np, rate);
> +
> clk_data.clks[1] = clk_register_fixed_rate(NULL, cpuclk_name, NULL,
> CLK_IS_ROOT, rate);
> WARN_ON(IS_ERR(clk_data.clks[1]));
> diff --git a/drivers/clk/mvebu/common.h b/drivers/clk/mvebu/common.h
> index f968b4d9df92..495f94ff4c90 100644
> --- a/drivers/clk/mvebu/common.h
> +++ b/drivers/clk/mvebu/common.h
> @@ -28,6 +28,7 @@ struct coreclk_soc_desc {
> u32 (*get_tclk_freq)(void __iomem *sar);
> u32 (*get_cpu_freq)(void __iomem *sar);
> void (*get_clk_ratio)(void __iomem *sar, int id, int *mult, int *div);
> + bool (*is_sscg_enabled)(void __iomem *sar);
> const struct coreclk_ratio *ratios;
> int num_ratios;
> };
>
next prev parent reply other threads:[~2014-08-29 12:48 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-08-29 11:43 [PATCH 0/4] clk:mvebu: Improve clock drift Gregory CLEMENT
2014-08-29 11:43 ` Gregory CLEMENT
2014-08-29 11:43 ` [PATCH 1/4] clk: mvebu: Fix clk frequency value if SSCG is enabled Gregory CLEMENT
2014-08-29 11:43 ` Gregory CLEMENT
2014-08-29 12:48 ` Sebastian Hesselbarth [this message]
2014-08-29 12:48 ` Sebastian Hesselbarth
2014-08-29 13:35 ` Gregory CLEMENT
2014-08-29 13:35 ` Gregory CLEMENT
2014-08-31 22:25 ` Leigh Brown
2014-08-31 22:25 ` Leigh Brown
2014-08-31 22:30 ` Leigh Brown
2014-08-31 22:30 ` Leigh Brown
2014-09-01 7:17 ` Gregory CLEMENT
2014-09-01 7:17 ` Gregory CLEMENT
2014-08-29 11:43 ` [PATCH 2/4] clk: mvebu: armada-370: Fix timer drift caused by the SSCG deviation Gregory CLEMENT
2014-08-29 11:43 ` Gregory CLEMENT
2014-08-29 13:08 ` Thomas Petazzoni
2014-08-29 13:08 ` Thomas Petazzoni
2014-08-29 13:37 ` Gregory CLEMENT
2014-08-29 13:37 ` Gregory CLEMENT
2014-08-29 11:43 ` [PATCH 3/4] ARM: mvebu: add SSCG to Armada 370 Device Tree Gregory CLEMENT
2014-08-29 11:43 ` Gregory CLEMENT
2014-08-29 11:43 ` [PATCH 4/4] clk: mvebu: armada-375: Fix the description of the SAR in the comment Gregory CLEMENT
2014-08-29 11:43 ` Gregory CLEMENT
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=540076A4.5000303@gmail.com \
--to=sebastian.hesselbarth@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.