From: "Heiko Stübner" <heiko@sntech.de>
To: Douglas Anderson <dianders@chromium.org>,
Stephen Boyd <sboyd@codeaurora.org>,
mturquette@baylibre.com
Cc: linux-rockchip@lists.infradead.org, linux-kernel@vger.kernel.org,
ykk@rock-chips.com, linux-clk@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] clk: rockchip: Fix PLL bandwidth
Date: Tue, 21 Jul 2015 23:04:25 +0200 [thread overview]
Message-ID: <2227753.WV2ipXIsxy@diego> (raw)
In-Reply-To: <1437511283-14216-1-git-send-email-dianders@chromium.org>
Hi Doug,
Am Dienstag, 21. Juli 2015, 13:41:23 schrieb Douglas Anderson:
> 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@chromium.org>
we talked about this beforehand in the Chromeos bug and I verified this in the
manual for all currently supported Rockchip socs (rk3066 - rk3368), so
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Background: the original code stems from a time when I was still operating
without documentation and I took this from the upstream kernel from that time
and sadly forgot to double check once I had docs.
Thanks for fixing this
Heiko
WARNING: multiple messages have this Message-ID (diff)
From: heiko@sntech.de (Heiko Stübner)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] clk: rockchip: Fix PLL bandwidth
Date: Tue, 21 Jul 2015 23:04:25 +0200 [thread overview]
Message-ID: <2227753.WV2ipXIsxy@diego> (raw)
In-Reply-To: <1437511283-14216-1-git-send-email-dianders@chromium.org>
Hi Doug,
Am Dienstag, 21. Juli 2015, 13:41:23 schrieb Douglas Anderson:
> 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@chromium.org>
we talked about this beforehand in the Chromeos bug and I verified this in the
manual for all currently supported Rockchip socs (rk3066 - rk3368), so
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Background: the original code stems from a time when I was still operating
without documentation and I took this from the upstream kernel from that time
and sadly forgot to double check once I had docs.
Thanks for fixing this
Heiko
next prev parent reply other threads:[~2015-07-21 21:04 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-21 20:41 [PATCH] clk: rockchip: Fix PLL bandwidth Douglas Anderson
2015-07-21 20:41 ` Douglas Anderson
2015-07-21 21:04 ` Heiko Stübner [this message]
2015-07-21 21:04 ` Heiko Stübner
2015-07-21 21:16 ` Stephen Boyd
2015-07-21 21:16 ` Stephen Boyd
2015-07-21 22:14 ` Heiko Stübner
2015-07-21 22:14 ` Heiko Stübner
2015-07-21 22:37 ` Doug Anderson
2015-07-21 22:37 ` Doug Anderson
2015-07-21 22:37 ` Doug Anderson
2015-07-21 22:43 ` Stephen Boyd
2015-07-21 22:43 ` Stephen Boyd
2015-07-21 22:43 ` Stephen Boyd
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=2227753.WV2ipXIsxy@diego \
--to=heiko@sntech.de \
--cc=dianders@chromium.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-clk@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=mturquette@baylibre.com \
--cc=sboyd@codeaurora.org \
--cc=ykk@rock-chips.com \
/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.