From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751198AbdEaJT7 (ORCPT ); Wed, 31 May 2017 05:19:59 -0400 Received: from mout.kundenserver.de ([212.227.126.134]:51223 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751056AbdEaJT6 (ORCPT ); Wed, 31 May 2017 05:19:58 -0400 Subject: Re: [PATCH 2/2] clk: bcm2835: Minimise clock jitter for PCM clock To: Phil Elwell , linux-clk@vger.kernel.org, Eric Anholt , linux-kernel@vger.kernel.org, Michael Turquette , Stephen Boyd , Florian Fainelli , linux-rpi-kernel@lists.infradead.org References: <1412052002.45738.1496171081779@email.1und1.de> <7aa4fb5f-65e3-56ed-9792-e38a947346b9@raspberrypi.org> From: Stefan Wahren Message-ID: <461fec8c-f8ee-e6d6-ab72-0ac735fa4605@i2se.com> Date: Wed, 31 May 2017 11:19:11 +0200 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <7aa4fb5f-65e3-56ed-9792-e38a947346b9@raspberrypi.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: de-DE X-Provags-ID: V03:K0:vfYZXjnkYQFqLXN9UFcibVCo5X1+FlxHF6KJXAEpl0ZRZjdQj+G rxyaJLFEESX5pArHWymTUTwukru+8smRCcUREzxVAzO7HAJFR76kxn+ynuWF75CMaMN71C3 HMfDSbZ5yBnczW4l5rPtLBkvygK/rP2o9tibgcKQeUTiDcNAWcYMa7JscJ3CJLNiVJc8UmR Ju1ICTv6/9kUivmWYB3Yw== X-UI-Out-Filterresults: notjunk:1;V01:K0:MJjM/wYvrX0=:TJJFFYGnYb2gnI9+DdjJtQ TAkKb61ph/WOXqRWNguOOyUEd1RDyJzWQKX+A760sDbR8v6l6RP/I4bMsHy+vGJVwk7PS36R+ WZGEdGh/Iv2g4zpAKM+WAZlBPM63nE02XcG4FCzfYB7KQVHbbpSIhq1wgr+XjbOydOlA0FFgL 6kVC6yS5OCmoQ209Pn2wJ70qcy2HytW9HZcEFF70PvP5ZKxkrftICKDhIf4cUgqd9wBEanhtH ABW5ZSeNDPKnZMmAq+2soc0VSZTL1hs2nqOqxe3iZiA7bNhPdVgjae5VMUpIBoYAMclLafc4v /DX9cVQ0uoezqZ+TSOE5g1kKa9PYC/Krute8qzOHQP0Rsd54q0s6rzJHuMPJLIJjEdYda7Tq+ Cvfz2A4VHOTgkbVKnBbrmaPWe1JxUj45FkxF7F7zUzyQnSHkuc/xkX3/K7BIRh9kdV08gm1hO f31fuNmv+pxHXLy9jJm//0YXtTDfs24vMCTGdiRWdZlej+izNVO0FgnsmjGuPDFd73E0oKnLD VM23d/nKogfXknAR8KkEcle8dKZ24omkTfpzNCGWJzgDbeXaS6ErbGmf28NfvEvxkEXTI0hXh q5Z06efSTOB2r7mzr44fyd6iHVSEooTA9WVTTNqL05Rn9iabrLXh8BwGjcv2hFrlSYbB4WGyh R+78wEyL7npDHaT6UHRMb0fInX/6I2aEsKWwrIsHo9Q/uSxZLFNLoZGL+LNHhXezq1heaXee5 3rE8ueGKkI/H2yeN Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Am 31.05.2017 um 10:33 schrieb Phil Elwell: > Hi Stefan, > > On 30/05/2017 20:04, Stefan Wahren wrote: >> Hi Phil, >> >>> Phil Elwell hat am 30. Mai 2017 um 18:28 geschrieben: >>> >>> >>> Fractional clock dividers generate accurate average frequencies but >>> with jitter, particularly when the integer divisor is small. >>> >>> Introduce a new metric of clock accuracy to penalise clocks with a good >>> average but worse jitter compared to clocks with an average which is no >>> better but with lower jitter. The metric is the ideal rate minus the >>> worse deviation from that ideal using the nearest integer divisors. >>> >>> Use this metric for parent selection for clocks requiring low jitter >>> (currently just PCM). >>> >>> Signed-off-by: Phil Elwell >>> --- >>> drivers/clk/bcm/clk-bcm2835.c | 39 ++++++++++++++++++++++++++++++++++----- >>> 1 file changed, 34 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c >>> index facc346..e0ce5e7 100644 >>> --- a/drivers/clk/bcm/clk-bcm2835.c >>> +++ b/drivers/clk/bcm/clk-bcm2835.c >>> @@ -530,6 +530,7 @@ struct bcm2835_clock_data { >>> >>> bool is_vpu_clock; >>> bool is_mash_clock; >>> + bool low_jitter; >>> >>> u32 tcnt_mux; >>> }; >>> @@ -1124,7 +1125,8 @@ static unsigned long bcm2835_clock_choose_div_and_prate(struct clk_hw *hw, >>> int parent_idx, >>> unsigned long rate, >>> u32 *div, >>> - unsigned long *prate) >>> + unsigned long *prate, >>> + unsigned long *avgrate) >>> { >>> struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw); >>> struct bcm2835_cprman *cprman = clock->cprman; >>> @@ -1136,11 +1138,33 @@ static unsigned long bcm2835_clock_choose_div_and_prate(struct clk_hw *hw, >>> parent = clk_hw_get_parent_by_index(hw, parent_idx); >>> >>> if (!(BIT(parent_idx) & data->set_rate_parent)) { >>> + unsigned long tmp_rate; >>> + >>> *prate = clk_hw_get_rate(parent); >>> *div = bcm2835_clock_choose_div(hw, rate, *prate, true); >>> >>> - return bcm2835_clock_rate_from_divisor(clock, *prate, >>> - *div); >>> + tmp_rate = bcm2835_clock_rate_from_divisor(clock, *prate, *div); >>> + *avgrate = tmp_rate; >>> + >>> + if (data->low_jitter && (*div & CM_DIV_FRAC_MASK)) { >>> + unsigned long high, low; >>> + u32 idiv = *div & ~CM_DIV_FRAC_MASK; >>> + >>> + high = bcm2835_clock_rate_from_divisor(clock, *prate, >>> + idiv); >>> + idiv += CM_DIV_FRAC_MASK + 1; >>> + low = bcm2835_clock_rate_from_divisor(clock, *prate, >>> + idiv); >> What about int_div or intdiv instead of idiv as variable name? > OK - int_div matches tmp_rate. > >>> + >>> + /* Return a value which is the maximum deviation >>> + * below the ideal rate, for use as a metric. >>> + */ >> Please fix coding style of the comment. > Oops. I'm surprised checkpatch didn't complain about that. The style above is only used for netdev. > > Thanks, > > Phil