linux-kernel - Re: [PATCH] clk: rockchip: Fix PLL bandwidth

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 linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 21 Jul 2015 15:37:38 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Stephen Boyd <sboyd@...eaurora.org>
Cc:	Heiko Stuebner <heiko@...ech.de>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Yakir Yang <ykk@...k-chips.com>,
	Alexandru Stan <amstan@...omium.org>, mturquette@...libre.com,
	linux-clk@...r.kernel.org,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] clk: rockchip: Fix PLL bandwidth

Stephen,

On Tue, Jul 21, 2015 at 2:16 PM, Stephen Boyd <sboyd@...eaurora.org> wrote:
> On 07/21/2015 01:41 PM, Douglas Anderson wrote:
>>
>> In the TRM we see that BWADJ is "a 12-bit bus that selects the values
>> 1-4096 for the bandwidth divider (NB)":
>>   NB = BWADJ[11:0] + 1
>> The recommended setting of NB: NB = NF / 2.
>>
>> So:
>>    NB = NF / 2
>>    BWADJ[11:0] + 1 = NF / 2
>>    BWADJ[11:0] = NF / 2 - 1
>>
>> Right now, we have:
>>
>> {                                               \
>>          .rate   = _rate##U,                     \
>>          .nr = _nr,                              \
>>          .nf = _nf,                              \
>>          .no = _no,                              \
>>          .bwadj = (_nf >> 1),                    \
>> }
>>
>> That means we set bwadj to NF / 2, not NF / 2 - 1
>>
>> All of this is a bit confusing because we specify "NR" (the 1-based
>> value), "NF" (the 1-based value), "NO" (the 1-based value), but
>> "BWADJ" (the 0-based value) instead of "NB" (the 1-based value).
>>
>> Let's change to working with "NB" and fix the off by one error.  This
>> may affect PLL jitter in a small way (hopefully for the better).
>>
>> Signed-off-by: Douglas Anderson <dianders@...omium.org>
>>
>
> There's no Fixes tag or stable Cc so I take it this isn't fixing any
> manifesting regression, more of a visual inspection bug find?

There is no known problem fixed.  I've been looking at HDMI and
controlling PLL jitter is an important part of supporting HDMI clock
rates.  That got me to looking at this parameter and deciding that we
should set it correctly.  Apparently it doesn't help in any hugely
significant way...  I just got done re-testing a whole lot of rates
and if it helped or hurt my jitter it's in the noise (AKA there's
enough variance run-to-run that it's hard to tell if this made any
difference).

-Doug
--
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/