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  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:	Mon, 01 Sep 2014 09:17:36 +0200
From:	Gregory CLEMENT <gregory.clement@...e-electrons.com>
To:	Leigh Brown <leigh@...inno.co.uk>
CC:	Mike Turquette <mturquette@...aro.org>,
	linux-kernel@...r.kernel.org,
	Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>,
	Andrew Lunn <andrew@...n.ch>,
	Jason Cooper <jason@...edaemon.net>,
	Tawfik Bayouk <tawfik@...vell.com>,
	Simon Boulay <simon.boulay@...ec.com>,
	Arnaud Ebalard <arno@...isbad.org>,
	Nadav Haklai <nadavh@...vell.com>,
	Lior Amsalem <alior@...vell.com>,
	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>,
	Raphael Rigo <ml-arm@...call.eu>,
	linux-arm-kernel@...ts.infradead.org,
	Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH 1/4] clk: mvebu: Fix clk frequency value if SSCG is enabled

Hi Leigh,

On 01/09/2014 00:25, Leigh Brown wrote:
> Hi Gregory,
> 
> On 2014-08-29 12:43, 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.
> 
> Thank you so much for this series.  I'm running 3.16 and without these 
> patches ntpd is all over the place, so it is a huge improvement  I do 
> have a comment further down though..
> 
>> 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.
>>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@...e-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
> 
> In the datasheet it says the percentage is 0.96 * (H - L) / H.  1/96 is 
> close but not the same as 0.0096.

Actually the public datasheet is wrong (even the NDA datasheet).

> 
>> +	 * 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));
> 
> So this would be become :-
> 
> 	/* NB: 0.96% = 0.0048 or 3/625 */
> 	freq_swing_half = (((u64)high_bound - (u64)low_bound)
> 			* (u64)system_clk * 3);
> 	do_div(freq_swing_half, (625 * high_bound));

It was the first implementation I tried, but with this one
the drift was about 200ppm instead of 50ppm. After having
checked this with the Marvell engineers, they confirmed me
that the datasheet was erroneous.


Thanks for your feedback: I will add a comment in the code
about the fact that the datasheet is erroneous.



-- 
Gregory Clement, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists