From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH 2/2] clk: rockchip: fix incorrect parent for rk3399's {c,g}pll_aclk_perihp_src To: Doug Anderson , Brian Norris References: <1463164937-91089-1-git-send-email-briannorris@chromium.org> <1463164937-91089-2-git-send-email-briannorris@chromium.org> <57369D2E.3030002@rock-chips.com> Cc: Michael Turquette , Stephen Boyd , linux-clk , Heiko Stuebner , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Rockchip SoC..." , Brian Norris From: Xing Zheng Message-ID: <579F1F8A.8030704@rock-chips.com> Date: Mon, 1 Aug 2016 18:08:10 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed List-ID: Hi Doug, It is so sorry that our IC folks re-correct these bits, they need to be recovered like these: ---- In the definition of CRU_CLKGATE_CON5: bit 1 - shows aclk_perihp_cpll_src_en bit 0 - shows aclk_perihp_gpll_src_en ---- And the new TRM will be updated. FYI. Thanks. On 2016年05月16日 23:49, Doug Anderson wrote: > Hi, > > On Fri, May 13, 2016 at 8:36 PM, Xing Zheng wrote: >> Hi Doug, >> >> >> On 2016年05月14日 04:10, Doug Anderson wrote: >>> Hi, >>> >>> On Fri, May 13, 2016 at 11:42 AM, Brian Norris >>> wrote: >>>> From: Xing Zheng >>>> >>>> There was a typo, swapping 'c' <--> 'g'. >>>> >>>> Signed-off-by: Xing Zheng >>>> Signed-off-by: Brian Norris >>>> --- >>>> drivers/clk/rockchip/clk-rk3399.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/clk/rockchip/clk-rk3399.c >>>> b/drivers/clk/rockchip/clk-rk3399.c >>>> index 145756c4f3c8..9f86bfef70f7 100644 >>>> --- a/drivers/clk/rockchip/clk-rk3399.c >>>> +++ b/drivers/clk/rockchip/clk-rk3399.c >>>> @@ -832,9 +832,9 @@ static struct rockchip_clk_branch >>>> rk3399_clk_branches[] __initdata = { >>>> RK3399_CLKGATE_CON(13), 1, GFLAGS), >>>> >>>> /* perihp */ >>>> - GATE(0, "cpll_aclk_perihp_src", "gpll", CLK_IGNORE_UNUSED, >>>> + GATE(0, "cpll_aclk_perihp_src", "cpll", CLK_IGNORE_UNUSED, >>>> RK3399_CLKGATE_CON(5), 0, GFLAGS), >>>> - GATE(0, "gpll_aclk_perihp_src", "cpll", CLK_IGNORE_UNUSED, >>>> + GATE(0, "gpll_aclk_perihp_src", "gpll", CLK_IGNORE_UNUSED, >>>> RK3399_CLKGATE_CON(5), 1, GFLAGS), >>>> COMPOSITE(ACLK_PERIHP, "aclk_perihp", mux_aclk_perihp_p, >>>> CLK_IGNORE_UNUSED, >>>> RK3399_CLKSEL_CON(14), 7, 1, MFLAGS, 0, 5, >>>> DFLAGS, >>> Definitely there was a bug since this table itself was inconsistent. >>> ...and I _think_ this fix is correct, but I'll note that the TRM has >>> more inconsistency here. >>> >>> In the big clock table 'CRU Clock Architecture Diagram', I see: >>> CLK 4 is CPLL >>> CLK 5 is GPLL >>> CLK 125 is aclk_perihp_cpll_src and has 4 (CPLL) as source, with >>> g5[0] as the bit >>> CLK 126 is aclk_perihp_gpll_src and has 5 (GPLL) as source, with >>> g5[1] as the bit >>> >>> In the definition of CRU_CLKGATE_CON5: >>> bit 0 shows aclk_perihp_gpll_src_en >>> bit 1 shows aclk_perihp_cpll_src_en >>> >>> >>> Thus the table shows CPLL as gate5[0] and GPLL as gate5[1]. The >>> register definition shows the opposite. I'll tend to believe the >>> table over the register definition, but I figured I'd bring it up >>> anyway. >>> >>> >>> Xing Zheng: can you confirm that the table is correct and ask >>> documentation folks to fix the register definition for >>> CRU_CLKGATE_CON5? >> Yes, previously, our IC & DOC partner confirmed that the definition of >> CRU_CLKGATE_CON5 should be: >> bit 0 shows aclk_perihp_cpll_src_en >> bit 1 shows aclk_perihp_gpll_src_en >> >> Sorry to the incorrect register definition, we will fix them and review the >> TRM again. > Great! > > Since we now have extra confirmation that Brian's patch is indeed correct: > > Reviewed-by: Douglas Anderson > > > -- - Xing Zheng From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xing Zheng Subject: Re: [PATCH 2/2] clk: rockchip: fix incorrect parent for rk3399's {c,g}pll_aclk_perihp_src Date: Mon, 1 Aug 2016 18:08:10 +0800 Message-ID: <579F1F8A.8030704@rock-chips.com> References: <1463164937-91089-1-git-send-email-briannorris@chromium.org> <1463164937-91089-2-git-send-email-briannorris@chromium.org> <57369D2E.3030002@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-rockchip" Errors-To: linux-rockchip-bounces+glpar-linux-rockchip=m.gmane.org-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org To: Doug Anderson , Brian Norris Cc: Heiko Stuebner , Michael Turquette , Stephen Boyd , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "open list:ARM/Rockchip SoC..." , Brian Norris , linux-clk , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-rockchip.vger.kernel.org SGkgRG91ZywKICAgICBJdCBpcyBzbyBzb3JyeSB0aGF0IG91ciBJQyBmb2xrcyByZS1jb3JyZWN0 IHRoZXNlIGJpdHMsIHRoZXkgbmVlZCAKdG8gYmUgcmVjb3ZlcmVkIGxpa2UgdGhlc2U6Ci0tLS0K SW4gdGhlIGRlZmluaXRpb24gb2YgQ1JVX0NMS0dBVEVfQ09ONToKICAgICBiaXQgMSAtIHNob3dz IGFjbGtfcGVyaWhwX2NwbGxfc3JjX2VuCiAgICAgYml0IDAgLSBzaG93cyBhY2xrX3BlcmlocF9n cGxsX3NyY19lbgotLS0tCiAgICAgQW5kIHRoZSBuZXcgVFJNIHdpbGwgYmUgdXBkYXRlZC4KCiAg ICAgRllJLgoKVGhhbmtzLgoKT24gMjAxNuW5tDA15pyIMTbml6UgMjM6NDksIERvdWcgQW5kZXJz b24gd3JvdGU6Cj4gSGksCj4KPiBPbiBGcmksIE1heSAxMywgMjAxNiBhdCA4OjM2IFBNLCBYaW5n IFpoZW5nIDx6aGVuZ3hpbmdAcm9jay1jaGlwcy5jb20+IHdyb3RlOgo+PiBIaSBEb3VnLAo+Pgo+ Pgo+PiBPbiAyMDE25bm0MDXmnIgxNOaXpSAwNDoxMCwgRG91ZyBBbmRlcnNvbiB3cm90ZToKPj4+ IEhpLAo+Pj4KPj4+IE9uIEZyaSwgTWF5IDEzLCAyMDE2IGF0IDExOjQyIEFNLCBCcmlhbiBOb3Jy aXMgPGJyaWFubm9ycmlzQGNocm9taXVtLm9yZz4KPj4+IHdyb3RlOgo+Pj4+IEZyb206IFhpbmcg WmhlbmcgPHpoZW5neGluZ0Byb2NrLWNoaXBzLmNvbT4KPj4+Pgo+Pj4+IFRoZXJlIHdhcyBhIHR5 cG8sIHN3YXBwaW5nICdjJyA8LS0+ICdnJy4KPj4+Pgo+Pj4+IFNpZ25lZC1vZmYtYnk6IFhpbmcg WmhlbmcgPHpoZW5neGluZ0Byb2NrLWNoaXBzLmNvbT4KPj4+PiBTaWduZWQtb2ZmLWJ5OiBCcmlh biBOb3JyaXMgPGJyaWFubm9ycmlzQGNocm9taXVtLm9yZz4KPj4+PiAtLS0KPj4+PiAgICBkcml2 ZXJzL2Nsay9yb2NrY2hpcC9jbGstcmszMzk5LmMgfCA0ICsrLS0KPj4+PiAgICAxIGZpbGUgY2hh bmdlZCwgMiBpbnNlcnRpb25zKCspLCAyIGRlbGV0aW9ucygtKQo+Pj4+Cj4+Pj4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvY2xrL3JvY2tjaGlwL2Nsay1yazMzOTkuYwo+Pj4+IGIvZHJpdmVycy9jbGsv cm9ja2NoaXAvY2xrLXJrMzM5OS5jCj4+Pj4gaW5kZXggMTQ1NzU2YzRmM2M4Li45Zjg2YmZlZjcw ZjcgMTAwNjQ0Cj4+Pj4gLS0tIGEvZHJpdmVycy9jbGsvcm9ja2NoaXAvY2xrLXJrMzM5OS5jCj4+ Pj4gKysrIGIvZHJpdmVycy9jbGsvcm9ja2NoaXAvY2xrLXJrMzM5OS5jCj4+Pj4gQEAgLTgzMiw5 ICs4MzIsOSBAQCBzdGF0aWMgc3RydWN0IHJvY2tjaGlwX2Nsa19icmFuY2gKPj4+PiByazMzOTlf Y2xrX2JyYW5jaGVzW10gX19pbml0ZGF0YSA9IHsKPj4+PiAgICAgICAgICAgICAgICAgICAgICAg ICAgIFJLMzM5OV9DTEtHQVRFX0NPTigxMyksIDEsIEdGTEFHUyksCj4+Pj4KPj4+PiAgICAgICAg ICAgLyogcGVyaWhwICovCj4+Pj4gLSAgICAgICBHQVRFKDAsICJjcGxsX2FjbGtfcGVyaWhwX3Ny YyIsICJncGxsIiwgQ0xLX0lHTk9SRV9VTlVTRUQsCj4+Pj4gKyAgICAgICBHQVRFKDAsICJjcGxs X2FjbGtfcGVyaWhwX3NyYyIsICJjcGxsIiwgQ0xLX0lHTk9SRV9VTlVTRUQsCj4+Pj4gICAgICAg ICAgICAgICAgICAgICAgICAgICBSSzMzOTlfQ0xLR0FURV9DT04oNSksIDAsIEdGTEFHUyksCj4+ Pj4gLSAgICAgICBHQVRFKDAsICJncGxsX2FjbGtfcGVyaWhwX3NyYyIsICJjcGxsIiwgQ0xLX0lH Tk9SRV9VTlVTRUQsCj4+Pj4gKyAgICAgICBHQVRFKDAsICJncGxsX2FjbGtfcGVyaWhwX3NyYyIs ICJncGxsIiwgQ0xLX0lHTk9SRV9VTlVTRUQsCj4+Pj4gICAgICAgICAgICAgICAgICAgICAgICAg ICBSSzMzOTlfQ0xLR0FURV9DT04oNSksIDEsIEdGTEFHUyksCj4+Pj4gICAgICAgICAgIENPTVBP U0lURShBQ0xLX1BFUklIUCwgImFjbGtfcGVyaWhwIiwgbXV4X2FjbGtfcGVyaWhwX3AsCj4+Pj4g Q0xLX0lHTk9SRV9VTlVTRUQsCj4+Pj4gICAgICAgICAgICAgICAgICAgICAgICAgICBSSzMzOTlf Q0xLU0VMX0NPTigxNCksIDcsIDEsIE1GTEFHUywgMCwgNSwKPj4+PiBERkxBR1MsCj4+PiBEZWZp bml0ZWx5IHRoZXJlIHdhcyBhIGJ1ZyBzaW5jZSB0aGlzIHRhYmxlIGl0c2VsZiB3YXMgaW5jb25z aXN0ZW50Lgo+Pj4gLi4uYW5kIEkgX3RoaW5rXyB0aGlzIGZpeCBpcyBjb3JyZWN0LCBidXQgSSds bCBub3RlIHRoYXQgdGhlIFRSTSBoYXMKPj4+IG1vcmUgaW5jb25zaXN0ZW5jeSBoZXJlLgo+Pj4K Pj4+IEluIHRoZSBiaWcgY2xvY2sgdGFibGUgJ0NSVSBDbG9jayBBcmNoaXRlY3R1cmUgRGlhZ3Jh bScsIEkgc2VlOgo+Pj4gICAgIENMSyA0IGlzIENQTEwKPj4+ICAgICBDTEsgNSBpcyBHUExMCj4+ PiAgICAgQ0xLIDEyNSBpcyBhY2xrX3BlcmlocF9jcGxsX3NyYyBhbmQgaGFzIDQgKENQTEwpIGFz IHNvdXJjZSwgd2l0aAo+Pj4gZzVbMF0gYXMgdGhlIGJpdAo+Pj4gICAgIENMSyAxMjYgaXMgYWNs a19wZXJpaHBfZ3BsbF9zcmMgYW5kIGhhcyA1IChHUExMKSBhcyBzb3VyY2UsIHdpdGgKPj4+IGc1 WzFdIGFzIHRoZSBiaXQKPj4+Cj4+PiBJbiB0aGUgZGVmaW5pdGlvbiBvZiBDUlVfQ0xLR0FURV9D T041Ogo+Pj4gICAgIGJpdCAwIHNob3dzIGFjbGtfcGVyaWhwX2dwbGxfc3JjX2VuCj4+PiAgICAg Yml0IDEgc2hvd3MgYWNsa19wZXJpaHBfY3BsbF9zcmNfZW4KPj4+Cj4+Pgo+Pj4gVGh1cyB0aGUg dGFibGUgc2hvd3MgQ1BMTCBhcyBnYXRlNVswXSBhbmQgR1BMTCBhcyBnYXRlNVsxXS4gIFRoZQo+ Pj4gcmVnaXN0ZXIgZGVmaW5pdGlvbiBzaG93cyB0aGUgb3Bwb3NpdGUuICBJJ2xsIHRlbmQgdG8g YmVsaWV2ZSB0aGUKPj4+IHRhYmxlIG92ZXIgdGhlIHJlZ2lzdGVyIGRlZmluaXRpb24sIGJ1dCBJ IGZpZ3VyZWQgSSdkIGJyaW5nIGl0IHVwCj4+PiBhbnl3YXkuCj4+Pgo+Pj4KPj4+IFhpbmcgWmhl bmc6IGNhbiB5b3UgY29uZmlybSB0aGF0IHRoZSB0YWJsZSBpcyBjb3JyZWN0IGFuZCBhc2sKPj4+ IGRvY3VtZW50YXRpb24gZm9sa3MgdG8gZml4IHRoZSByZWdpc3RlciBkZWZpbml0aW9uIGZvcgo+ Pj4gQ1JVX0NMS0dBVEVfQ09ONT8KPj4gWWVzLCBwcmV2aW91c2x5LCBvdXIgSUMgJiBET0MgcGFy dG5lciBjb25maXJtZWQgdGhhdCB0aGUgZGVmaW5pdGlvbiBvZgo+PiBDUlVfQ0xLR0FURV9DT041 IHNob3VsZCBiZToKPj4gICAgYml0IDAgc2hvd3MgYWNsa19wZXJpaHBfY3BsbF9zcmNfZW4KPj4g ICAgYml0IDEgc2hvd3MgYWNsa19wZXJpaHBfZ3BsbF9zcmNfZW4KPj4KPj4gU29ycnkgdG8gdGhl IGluY29ycmVjdCByZWdpc3RlciBkZWZpbml0aW9uLCB3ZSB3aWxsIGZpeCB0aGVtIGFuZCByZXZp ZXcgdGhlCj4+IFRSTSBhZ2Fpbi4KPiBHcmVhdCEKPgo+IFNpbmNlIHdlIG5vdyBoYXZlIGV4dHJh IGNvbmZpcm1hdGlvbiB0aGF0IEJyaWFuJ3MgcGF0Y2ggaXMgaW5kZWVkIGNvcnJlY3Q6Cj4KPiBS ZXZpZXdlZC1ieTogRG91Z2xhcyBBbmRlcnNvbiA8ZGlhbmRlcnNAY2hyb21pdW0ub3JnPgo+Cj4K PgoKLS0gCi0gWGluZyBaaGVuZwoKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpMaW51eC1yb2NrY2hpcCBtYWlsaW5nIGxpc3QKTGludXgtcm9ja2NoaXBA bGlzdHMuaW5mcmFkZWFkLm9yZwpodHRwOi8vbGlzdHMuaW5mcmFkZWFkLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2xpbnV4LXJvY2tjaGlwCg== From mboxrd@z Thu Jan 1 00:00:00 1970 From: zhengxing@rock-chips.com (Xing Zheng) Date: Mon, 1 Aug 2016 18:08:10 +0800 Subject: [PATCH 2/2] clk: rockchip: fix incorrect parent for rk3399's {c,g}pll_aclk_perihp_src In-Reply-To: References: <1463164937-91089-1-git-send-email-briannorris@chromium.org> <1463164937-91089-2-git-send-email-briannorris@chromium.org> <57369D2E.3030002@rock-chips.com> Message-ID: <579F1F8A.8030704@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Doug, It is so sorry that our IC folks re-correct these bits, they need to be recovered like these: ---- In the definition of CRU_CLKGATE_CON5: bit 1 - shows aclk_perihp_cpll_src_en bit 0 - shows aclk_perihp_gpll_src_en ---- And the new TRM will be updated. FYI. Thanks. On 2016?05?16? 23:49, Doug Anderson wrote: > Hi, > > On Fri, May 13, 2016 at 8:36 PM, Xing Zheng wrote: >> Hi Doug, >> >> >> On 2016?05?14? 04:10, Doug Anderson wrote: >>> Hi, >>> >>> On Fri, May 13, 2016 at 11:42 AM, Brian Norris >>> wrote: >>>> From: Xing Zheng >>>> >>>> There was a typo, swapping 'c' <--> 'g'. >>>> >>>> Signed-off-by: Xing Zheng >>>> Signed-off-by: Brian Norris >>>> --- >>>> drivers/clk/rockchip/clk-rk3399.c | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/clk/rockchip/clk-rk3399.c >>>> b/drivers/clk/rockchip/clk-rk3399.c >>>> index 145756c4f3c8..9f86bfef70f7 100644 >>>> --- a/drivers/clk/rockchip/clk-rk3399.c >>>> +++ b/drivers/clk/rockchip/clk-rk3399.c >>>> @@ -832,9 +832,9 @@ static struct rockchip_clk_branch >>>> rk3399_clk_branches[] __initdata = { >>>> RK3399_CLKGATE_CON(13), 1, GFLAGS), >>>> >>>> /* perihp */ >>>> - GATE(0, "cpll_aclk_perihp_src", "gpll", CLK_IGNORE_UNUSED, >>>> + GATE(0, "cpll_aclk_perihp_src", "cpll", CLK_IGNORE_UNUSED, >>>> RK3399_CLKGATE_CON(5), 0, GFLAGS), >>>> - GATE(0, "gpll_aclk_perihp_src", "cpll", CLK_IGNORE_UNUSED, >>>> + GATE(0, "gpll_aclk_perihp_src", "gpll", CLK_IGNORE_UNUSED, >>>> RK3399_CLKGATE_CON(5), 1, GFLAGS), >>>> COMPOSITE(ACLK_PERIHP, "aclk_perihp", mux_aclk_perihp_p, >>>> CLK_IGNORE_UNUSED, >>>> RK3399_CLKSEL_CON(14), 7, 1, MFLAGS, 0, 5, >>>> DFLAGS, >>> Definitely there was a bug since this table itself was inconsistent. >>> ...and I _think_ this fix is correct, but I'll note that the TRM has >>> more inconsistency here. >>> >>> In the big clock table 'CRU Clock Architecture Diagram', I see: >>> CLK 4 is CPLL >>> CLK 5 is GPLL >>> CLK 125 is aclk_perihp_cpll_src and has 4 (CPLL) as source, with >>> g5[0] as the bit >>> CLK 126 is aclk_perihp_gpll_src and has 5 (GPLL) as source, with >>> g5[1] as the bit >>> >>> In the definition of CRU_CLKGATE_CON5: >>> bit 0 shows aclk_perihp_gpll_src_en >>> bit 1 shows aclk_perihp_cpll_src_en >>> >>> >>> Thus the table shows CPLL as gate5[0] and GPLL as gate5[1]. The >>> register definition shows the opposite. I'll tend to believe the >>> table over the register definition, but I figured I'd bring it up >>> anyway. >>> >>> >>> Xing Zheng: can you confirm that the table is correct and ask >>> documentation folks to fix the register definition for >>> CRU_CLKGATE_CON5? >> Yes, previously, our IC & DOC partner confirmed that the definition of >> CRU_CLKGATE_CON5 should be: >> bit 0 shows aclk_perihp_cpll_src_en >> bit 1 shows aclk_perihp_gpll_src_en >> >> Sorry to the incorrect register definition, we will fix them and review the >> TRM again. > Great! > > Since we now have extra confirmation that Brian's patch is indeed correct: > > Reviewed-by: Douglas Anderson > > > -- - Xing Zheng