From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko =?ISO-8859-1?Q?St=FCbner?= Subject: Re: [PATCH v4 1/3] ARM: rockchip: fix the CPU soft reset Date: Sun, 07 Jun 2015 11:02:18 +0200 Message-ID: <3887846.0ppL0Y56At@diego> References: <1433523923-4755-1-git-send-email-wxt@rock-chips.com> <5573DBDC.2010204@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <5573DBDC.2010204-TNX95d0MmH7DzftRWevZcw@public.gmane.org> 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: Caesar Wang Cc: Russell King , Dmitry Torokhov , Doug Anderson , "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org" List-Id: linux-rockchip.vger.kernel.org SGkgQ2Flc2FyLCBEb3VnLAoKQW0gU29ubnRhZywgNy4gSnVuaSAyMDE1LCAxMzo1MToyNCBzY2hy aWViIENhZXNhciBXYW5nOgo+IOWcqCAyMDE15bm0MDbmnIgwN+aXpSAxMTo0MywgRG91ZyBBbmRl cnNvbiDlhpnpgZM6Cj4gPiBDYWVzYXIsCj4gPiAKPiA+IE9uIFNhdCwgSnVuIDYsIDIwMTUgYXQg Nzo1MSBQTSwgQ2Flc2FyIFdhbmcgPHd4dEByb2NrLWNoaXBzLmNvbT4gd3JvdGU6Cj4gPj4gQEAg LTE1MCwxMyArMTU5LDE1IEBAIHN0YXRpYyBpbnQgX19jcHVpbml0Cj4gPj4gcm9ja2NoaXBfYm9v dF9zZWNvbmRhcnkodW5zaWduZWQKPiA+PiBpbnQgY3B1LAo+ID4+IAo+ID4+ICAgICAgICAgICAg ICAgICAgICogc3JhbV9iYXNlX2FkZHIgKyA0OiAweGRlYWRiZWFmCj4gPj4gICAgICAgICAgICAg ICAgICAgKiBzcmFtX2Jhc2VfYWRkciArIDg6IHN0YXJ0IGFkZHJlc3MgZm9yIHBjCj4gPj4gICAg ICAgICAgICAgICAgICAgKiAqLwo+ID4+IAo+ID4+IC0gICAgICAgICAgICAgICB1ZGVsYXkoMTAp Owo+ID4+ICsgICAgICAgICAgICAgICB1ZGVsYXkoMjApOwo+ID4+IAo+ID4+IEkgaW5jcmVhc2Vk IHRoZSAndWRlbGF5KDIwKScgb3IgJ3VkZWxheSg1MCknIGluCj4gPj4gcm9ja2NoaXBfYm9vdF9z ZWNvbmRhcnkoKS4KPiA+PiBTZXQjMiBhbHNvIGNhbiByZXBybyB0aGlzIGlzc3VlIG92ZXIgMjI2 MDAgY3ljbGVzIHdpdGggdGVzdGluZyBzY3JpcHRzLgo+ID4+IChhYm91dCAxIGhvdXJzKQo+ID4+ IAo+ID4+IGxvZzoKPiA+PiA9PT09PT09PT09PT09PT09PSAyMjYgPT09PT09PT09PT09Cj4gPj4g WyA0MDY5LjEzNDQxOV0gQ1BVMTogc2h1dGRvd24KPiA+PiBbIDQwNjkuMTY0NDMxXSBDUFUyOiBz aHV0ZG93bgo+ID4+IFsgNDA2OS4yMDQ0NzVdIENQVTM6IHNodXRkb3duCj4gPj4gLi4uLi4uCj4g Pj4gWyA0MDcyLjQ1NDQ1M10gQ1BVMTogc2h1dGRvd24KPiA+PiBbIDQwNzIuNTA0NDM2XSBDUFUy OiBzaHV0ZG93bgo+ID4+IFsgNDA3Mi41NTQ0MjZdIENQVTM6IHNodXRkb3duCj4gPj4gWyA0MDcy LjU3NzgyN10gQ1BVMTogQm9vdGVkIHNlY29uZGFyeSBwcm9jZXNzb3IKPiA+PiBbIDQwNzIuNTgy NjExXSBDUFUyOiBCb290ZWQgc2Vjb25kYXJ5IHByb2Nlc3Nvcgo+ID4+IFsgNDA3Mi41ODc0MjZd IENQVTM6IEJvb3RlZCBzZWNvbmRhcnkgcHJvY2Vzc29yCj4gPj4gPGhhbmc+Cj4gPj4gCj4gPj4g VGhlIHNldCAjNCB3aWxsIGJlIGJldHRlciB3b3JrLgo+ID4gCj4gPiBPSywgSSdtIE9LIHdpdGgg dGhpcywgYnV0IEknZCBsaWtlIHRvIGdldCBIZWlrbydzIG9waW5pb24uCj4gPiAKPiA+IEFsc286 Cj4gPiAqIEp1c3QgZm9yIGtpY2tzLCBkb2VzIG1kZWxheSgxKSB3b3JrPyAgSSBrbm93IHRoYXQn cyAxMDB4IG1vcmUgdGhhbgo+IAo+IE9LLCBpdCBzaG91bGQgZGVsYXkgbW9yZSB0aW1lLgo+IAo+ IHRoZSBtZGVsYXkoMSkgY2FuIGJlIHdvcmsgb3ZlciA1MDAwMCBjeWNsZXMsIHNvIHRoYXQgc2hv dWxkIGJlIHdvcmsuCj4gCj4gCj4gUGVyaGFwcywgY2FuIHdlIHVzZSAndXNsZWVwX3JhbmdlKDUw MCwgMTAwMCknIHRvIHdvcmsuCj4gSGVpa28sIGRvIHlvdSBhZ3JlZSB3aXRoIGl0PwoKeWVwIDot KQoKQXMgSSBzYWlkIGJlZm9yZSwgZG9pbmcKCglwb3dlcnVwLCBkZWFzc2VydF9yZXNldCwgd2Fp dF9mb3JfcG93ZXJkb21haW4KCmZlZWxzIGxpa2UgaXQgaXMgb25seSBtb3ZpbmcgdGhlIHByb2Js ZW0gYSBiaXQgYnV0IGlzIGFjdHVhbGx5IG9ubHkgd29ya2luZyBieSAKY2hhbmNlLCBhcyBteSBb bGl0dGxlIGJpdCBvZiA6LSkgXSBjb21tb24gc2Vuc2UgdGVsbHMgbWUsIHRoYXQgd2UgcmVhbGx5 IG9ubHkgCnNob3VsZCBkZWFzc2VydCB0aGUgcmVzZXQgd2hlbiB3ZSdyZSBzdXJlIHRoYXQgdGhl IGNvcmUgaGFzIHBvd2VyLCBpLmUuCgoJcG93ZXJ1cCwgd2FpdF9mb3JfcG93ZXJkb21haW4sIGRl YXNzZXJ0X3Jlc2V0CgpBbHNvLCB3aGVuIGdvaW5nIGRvd24gdGhpcyBwYXRoLCBwbGVhc2UgdGFr ZSBhIGxvb2sgYXQgdGhlIHNsaWdodGx5IGRpZmZlcmVudCAKdmFyaWFudCBJIHBvc3RlZCBhcyBy ZXNwb25zZSB0byB2MywgYXMgaXQgbWFrZXMgdGhlIGRpZmYgYSBiaXQgc21hbGxlciA6LSkKCgpB cyBmb3Ige3UvbX1kZWxheSB2cy4geW91ciB1c2xlZXBfcmFuZ2VzLCBJIGRvbid0IGtub3cgaWYg eW91J3JlIGFsbG93ZWQgdG8gCnNsZWVwIGluIHRoaXMgYXJlYS4gT3RoZXIgYXJjaGl0ZWN0dXJl cyBvbmx5IHNlZW0gdG8gdXNlIHVkZWxheSBpbiBfX2NwdV91cCAKd2hpY2ggY2FsbHMgdGhlIHNt cF9zZWNvbmRhcnlfc3RhcnR1cCBjYWxsYmFjaywgbGlrZToKCi0gYXJjaC9zaC9rZXJuZWwvc21w LmMKLSBhcmNoL20zMnIva2VybmVsL3NtcGJvb3QuYyBbaXMgZXZlbiBhIHVkZWxheSgxMDAwKQph bmQgbW9yZQoKCkhlaWtvCgo+ID4gdWRlbGF5KDEwKSwgYnV0IHByZXZpb3VzbHkgd2Ugd2VyZSBh Y3R1YWxseSBsb29waW5nIHdhaXRpbmcgZm9yIHRoZQo+ID4gcG93ZXIgZG9tYWluLCByaWdodD8g IC4uLnNvIG1heWJlIHRoZSBvbGQgY29kZSB1c2VkIHRvIGludHJvZHVjZSBhCj4gPiBwcmV0dHkg YmlnIGRlbGF5Lgo+ID4gCj4gPiAqIERvZXMgYW55b25lIGZyb20gdGhlIGNoaXAgZGVzaWduIHRl YW0gaGF2ZSBhbnkgaWRlYSB3aHkgcGF0Y2ggc2V0ICM0Cj4gPiB3b3JrcyBidXQgcGF0Y2ggc2V0 ICMyIGRvZXNuJ3Q/ICBJIGtub3cgaXQncyBTdW5kYXkgbW9ybmluZyBpbiBDaGluYQo+ID4gcmln aHQgbm93LCBidXQgbWF5YmUgeW91IGNvdWxkIGFzayBNb25kYXk/Cj4gPiAKPiA+IAo+ID4gVGhh bmtzIQo+ID4gCj4gPiAtRG91ZwoKCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fCkxpbnV4LXJvY2tjaGlwIG1haWxpbmcgbGlzdApMaW51eC1yb2NrY2hpcEBs aXN0cy5pbmZyYWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlz dGluZm8vbGludXgtcm9ja2NoaXAK From mboxrd@z Thu Jan 1 00:00:00 1970 From: heiko@sntech.de (Heiko =?ISO-8859-1?Q?St=FCbner?=) Date: Sun, 07 Jun 2015 11:02:18 +0200 Subject: [PATCH v4 1/3] ARM: rockchip: fix the CPU soft reset In-Reply-To: <5573DBDC.2010204@rock-chips.com> References: <1433523923-4755-1-git-send-email-wxt@rock-chips.com> <5573DBDC.2010204@rock-chips.com> Message-ID: <3887846.0ppL0Y56At@diego> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Caesar, Doug, Am Sonntag, 7. Juni 2015, 13:51:24 schrieb Caesar Wang: > ? 2015?06?07? 11:43, Doug Anderson ??: > > Caesar, > > > > On Sat, Jun 6, 2015 at 7:51 PM, Caesar Wang wrote: > >> @@ -150,13 +159,15 @@ static int __cpuinit > >> rockchip_boot_secondary(unsigned > >> int cpu, > >> > >> * sram_base_addr + 4: 0xdeadbeaf > >> * sram_base_addr + 8: start address for pc > >> * */ > >> > >> - udelay(10); > >> + udelay(20); > >> > >> I increased the 'udelay(20)' or 'udelay(50)' in > >> rockchip_boot_secondary(). > >> Set#2 also can repro this issue over 22600 cycles with testing scripts. > >> (about 1 hours) > >> > >> log: > >> ================= 226 ============ > >> [ 4069.134419] CPU1: shutdown > >> [ 4069.164431] CPU2: shutdown > >> [ 4069.204475] CPU3: shutdown > >> ...... > >> [ 4072.454453] CPU1: shutdown > >> [ 4072.504436] CPU2: shutdown > >> [ 4072.554426] CPU3: shutdown > >> [ 4072.577827] CPU1: Booted secondary processor > >> [ 4072.582611] CPU2: Booted secondary processor > >> [ 4072.587426] CPU3: Booted secondary processor > >> > >> > >> The set #4 will be better work. > > > > OK, I'm OK with this, but I'd like to get Heiko's opinion. > > > > Also: > > * Just for kicks, does mdelay(1) work? I know that's 100x more than > > OK, it should delay more time. > > the mdelay(1) can be work over 50000 cycles, so that should be work. > > > Perhaps, can we use 'usleep_range(500, 1000)' to work. > Heiko, do you agree with it? yep :-) As I said before, doing powerup, deassert_reset, wait_for_powerdomain feels like it is only moving the problem a bit but is actually only working by chance, as my [little bit of :-) ] common sense tells me, that we really only should deassert the reset when we're sure that the core has power, i.e. powerup, wait_for_powerdomain, deassert_reset Also, when going down this path, please take a look at the slightly different variant I posted as response to v3, as it makes the diff a bit smaller :-) As for {u/m}delay vs. your usleep_ranges, I don't know if you're allowed to sleep in this area. Other architectures only seem to use udelay in __cpu_up which calls the smp_secondary_startup callback, like: - arch/sh/kernel/smp.c - arch/m32r/kernel/smpboot.c [is even a udelay(1000) and more Heiko > > udelay(10), but previously we were actually looping waiting for the > > power domain, right? ...so maybe the old code used to introduce a > > pretty big delay. > > > > * Does anyone from the chip design team have any idea why patch set #4 > > works but patch set #2 doesn't? I know it's Sunday morning in China > > right now, but maybe you could ask Monday? > > > > > > Thanks! > > > > -Doug From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752763AbbFGJjv (ORCPT ); Sun, 7 Jun 2015 05:39:51 -0400 Received: from gloria.sntech.de ([95.129.55.99]:58770 "EHLO gloria.sntech.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751004AbbFGJjn convert rfc822-to-8bit (ORCPT ); Sun, 7 Jun 2015 05:39:43 -0400 From: Heiko =?ISO-8859-1?Q?St=FCbner?= To: Caesar Wang Cc: Doug Anderson , Dmitry Torokhov , "open list:ARM/Rockchip SoC..." , Russell King , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH v4 1/3] ARM: rockchip: fix the CPU soft reset Date: Sun, 07 Jun 2015 11:02:18 +0200 Message-ID: <3887846.0ppL0Y56At@diego> User-Agent: KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; ) In-Reply-To: <5573DBDC.2010204@rock-chips.com> References: <1433523923-4755-1-git-send-email-wxt@rock-chips.com> <5573DBDC.2010204@rock-chips.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8BIT Content-Type: text/plain; charset="utf-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Caesar, Doug, Am Sonntag, 7. Juni 2015, 13:51:24 schrieb Caesar Wang: > 在 2015年06月07日 11:43, Doug Anderson 写道: > > Caesar, > > > > On Sat, Jun 6, 2015 at 7:51 PM, Caesar Wang wrote: > >> @@ -150,13 +159,15 @@ static int __cpuinit > >> rockchip_boot_secondary(unsigned > >> int cpu, > >> > >> * sram_base_addr + 4: 0xdeadbeaf > >> * sram_base_addr + 8: start address for pc > >> * */ > >> > >> - udelay(10); > >> + udelay(20); > >> > >> I increased the 'udelay(20)' or 'udelay(50)' in > >> rockchip_boot_secondary(). > >> Set#2 also can repro this issue over 22600 cycles with testing scripts. > >> (about 1 hours) > >> > >> log: > >> ================= 226 ============ > >> [ 4069.134419] CPU1: shutdown > >> [ 4069.164431] CPU2: shutdown > >> [ 4069.204475] CPU3: shutdown > >> ...... > >> [ 4072.454453] CPU1: shutdown > >> [ 4072.504436] CPU2: shutdown > >> [ 4072.554426] CPU3: shutdown > >> [ 4072.577827] CPU1: Booted secondary processor > >> [ 4072.582611] CPU2: Booted secondary processor > >> [ 4072.587426] CPU3: Booted secondary processor > >> > >> > >> The set #4 will be better work. > > > > OK, I'm OK with this, but I'd like to get Heiko's opinion. > > > > Also: > > * Just for kicks, does mdelay(1) work? I know that's 100x more than > > OK, it should delay more time. > > the mdelay(1) can be work over 50000 cycles, so that should be work. > > > Perhaps, can we use 'usleep_range(500, 1000)' to work. > Heiko, do you agree with it? yep :-) As I said before, doing powerup, deassert_reset, wait_for_powerdomain feels like it is only moving the problem a bit but is actually only working by chance, as my [little bit of :-) ] common sense tells me, that we really only should deassert the reset when we're sure that the core has power, i.e. powerup, wait_for_powerdomain, deassert_reset Also, when going down this path, please take a look at the slightly different variant I posted as response to v3, as it makes the diff a bit smaller :-) As for {u/m}delay vs. your usleep_ranges, I don't know if you're allowed to sleep in this area. Other architectures only seem to use udelay in __cpu_up which calls the smp_secondary_startup callback, like: - arch/sh/kernel/smp.c - arch/m32r/kernel/smpboot.c [is even a udelay(1000) and more Heiko > > udelay(10), but previously we were actually looping waiting for the > > power domain, right? ...so maybe the old code used to introduce a > > pretty big delay. > > > > * Does anyone from the chip design team have any idea why patch set #4 > > works but patch set #2 doesn't? I know it's Sunday morning in China > > right now, but maybe you could ask Monday? > > > > > > Thanks! > > > > -Doug