From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hanjun Guo Subject: Re: [PATCH v20 08/17] clocksource/drivers/arm_arch_timer: Rework counter frequency detection. Date: Thu, 19 Jan 2017 20:41:14 +0800 Message-ID: <74160889-5eb0-ceb2-2727-61e9d538cf2f@linaro.org> References: <20170118132541.8989-1-fu.wei@linaro.org> <20170118132541.8989-9-fu.wei@linaro.org> 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-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Fu Wei Cc: Mark Rutland , Linaro ACPI Mailman List , Catalin Marinas , Will Deacon , rruigrok@codeaurora.org, Wim Van Sebroeck , Wei Huang , Lorenzo Pieralisi , Al Stone , Tomasz Nowicki , Timur Tabi , Daniel Lezcano , ACPI Devel Maling List , Guenter Roeck , Len Brown , "Abdulhamid, Harb" , Julien Grall , linux-watchdog@vger.kernel.org, Arnd Bergmann , Marc Zyngier , Jon Masters , Christopher Covington , Thomas Gleixner , linux-arm-kernel@lists List-Id: linux-acpi@vger.kernel.org T24gMjAxNy8xLzE5IDE3OjQ0LCBGdSBXZWkgd3JvdGU6Cj4gSGkgSGFuanVuLAo+Cj4gT24gMTkg SmFudWFyeSAyMDE3IGF0IDE2OjAyLCBIYW5qdW4gR3VvIDxoYW5qdW4uZ3VvQGxpbmFyby5vcmc+ IHdyb3RlOgo+PiBIaSBGdXdlaSwKPj4KPj4gT25lIGNvbW1lbnRzIGJlbG93Lgo+Pgo+Pgo+PiBP biAyMDE3LzEvMTggMjE6MjUsIGZ1LndlaUBsaW5hcm8ub3JnIHdyb3RlOgo+Pj4KPj4+IEZyb206 IEZ1IFdlaSA8ZnUud2VpQGxpbmFyby5vcmc+Cj4+Pgo+Pj4gVGhlIGNvdW50ZXIgZnJlcXVlbmN5 IGRldGVjdGlvbiBjYWxsKGFyY2hfdGltZXJfZGV0ZWN0X3JhdGUpIGNvbWJpbmVzIHR3bwo+Pj4g d2F5cyB0byBnZXQgY291bnRlciBmcmVxdWVuY3k6IHN5c3RlbSBjb3Byb2Nlc3NvciByZWdpc3Rl ciBhbmQgTU1JTyB0aW1lci4KPj4+IEJ1dCBpbiBhIHNwZWNpZmljIHRpbWVyIGluaXQgY29kZSwg d2Ugb25seSBuZWVkIG9uZSB3YXkgdG8gdHJ5Ogo+Pj4gZ2V0dGluZyBmcmVxdWVuY3kgZnJvbSBN TUlPIHRpbWVyIHJlZ2lzdGVyIHdpbGwgYmUgbmVlZGVkIG9ubHkgd2hlbiB3ZQo+Pj4gaW5pdCBN TUlPIHRpbWVyOyBnZXR0aW5nIGZyZXF1ZW5jeSBmcm9tIHN5c3RlbSBjb3Byb2Nlc3NvciByZWdp c3RlciB3aWxsCj4+PiBiZSBuZWVkZWQgb25seSB3aGVuIHdlIGluaXQgYXJjaCB0aW1lci4KPj4+ Cj4+PiBUaGlzIHBhdGNoIHNlcGFyYXRlcyBwYXRocyB0byBkZXRlcm1pbmUgZnJlcXVlbmN5Ogo+ Pj4gU2VwYXJhdGUgb3V0IHRoZSBNTUlPIGZyZXF1ZW5jeSBhbmQgdGhlIHN5c3JlZyBmcmVxdWVu Y3kgZGV0ZWN0aW9uIGNhbGwsCj4+PiBhbmQgdXNlIHRoZSBhcHByb3ByaWF0ZSBvbmUgZm9yIHRo ZSBjb3VudGVyLgo+Pj4KPj4+IFNpZ25lZC1vZmYtYnk6IEZ1IFdlaSA8ZnUud2VpQGxpbmFyby5v cmc+Cj4+PiAtLS0KPj4+ICBkcml2ZXJzL2Nsb2Nrc291cmNlL2FybV9hcmNoX3RpbWVyLmMgfCA0 MAo+Pj4gKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLS0tCj4+PiAgMSBmaWxlIGNo YW5nZWQsIDI1IGluc2VydGlvbnMoKyksIDE1IGRlbGV0aW9ucygtKQo+Pj4KPj4+IGRpZmYgLS1n aXQgYS9kcml2ZXJzL2Nsb2Nrc291cmNlL2FybV9hcmNoX3RpbWVyLmMKPj4+IGIvZHJpdmVycy9j bG9ja3NvdXJjZS9hcm1fYXJjaF90aW1lci5jCj4+PiBpbmRleCA2NDg0Zjg0Li45NDgyNDgxIDEw MDY0NAo+Pj4gLS0tIGEvZHJpdmVycy9jbG9ja3NvdXJjZS9hcm1fYXJjaF90aW1lci5jCj4+PiAr KysgYi9kcml2ZXJzL2Nsb2Nrc291cmNlL2FybV9hcmNoX3RpbWVyLmMKPj4+IEBAIC00ODgsMjMg KzQ4OCwzMyBAQCBzdGF0aWMgaW50IGFyY2hfdGltZXJfc3RhcnRpbmdfY3B1KHVuc2lnbmVkIGlu dCBjcHUpCj4+PiAgICAgICAgIHJldHVybiAwOwo+Pj4gIH0KPj4+Cj4+PiAtc3RhdGljIHZvaWQg YXJjaF90aW1lcl9kZXRlY3RfcmF0ZSh2b2lkIF9faW9tZW0gKmNudGJhc2UpCj4+PiArc3RhdGlj IHZvaWQgX19hcmNoX3RpbWVyX2RldGVybWluZV9yYXRlKHUzMiByYXRlKQo+Pj4gIHsKPj4+IC0g ICAgICAgLyogV2hvIGhhcyBtb3JlIHRoYW4gb25lIGluZGVwZW5kZW50IHN5c3RlbSBjb3VudGVy PyAqLwo+Pj4gLSAgICAgICBpZiAoYXJjaF90aW1lcl9yYXRlKQo+Pj4gLSAgICAgICAgICAgICAg IHJldHVybjsKPj4+ICsgICAgICAgLyogQ2hlY2sgdGhlIHRpbWVyIGZyZXF1ZW5jeS4gKi8KPj4+ ICsgICAgICAgaWYgKCFhcmNoX3RpbWVyX3JhdGUpIHsKPj4+ICsgICAgICAgICAgICAgICBpZiAo cmF0ZSkKPj4+ICsgICAgICAgICAgICAgICAgICAgICAgIGFyY2hfdGltZXJfcmF0ZSA9IHJhdGU7 Cj4+PiArICAgICAgICAgICAgICAgZWxzZQo+Pj4gKyAgICAgICAgICAgICAgICAgICAgICAgcHJf d2FybigiZnJlcXVlbmN5IG5vdCBhdmFpbGFibGVcbiIpOwo+Pj4gKyAgICAgICB9IGVsc2UgaWYg KHJhdGUgJiYgYXJjaF90aW1lcl9yYXRlICE9IHJhdGUpIHsKPj4KPj4gICAgICAgICAgICAgICAg ICAgICAgICAgXgo+PiBUeXBvPyBJIHRoaW5rIGl0J3MgIiYiIGhlcmUuCj4KPiBOb3QgYSB0eXBv LCBJdCdzIGRlZmluaXRlbHkgYSDigJwmJuKAnSAgOi0pCj4KPiBIZXJlIGFyY2hfdGltZXJfcmF0 ZSBpcyBub3QgemVyby4KPgo+IElmIHJhdGUgaXMgbm90IHplcm8odGhhdCBtZWFucyB3ZSBnb3Qg YSB2YWxpZCByYXRlKSwgYW5kCj4gYXJjaF90aW1lcl9yYXRlICE9IHJhdGUgLAo+IHdlIHdpbGwg cHJpbnQgd2FybmluZyBtZXNzYWdlLgoKQWgsIG1pc3JlYWRpbmcgdGhlIGNvZGUsIHRoYW5rcyBm b3IgY2xhcmlmeSA6KQoKSGFuanVuCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2Vy bmVsQGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1h bi9saXN0aW5mby9saW51eC1hcm0ta2VybmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752599AbdASMrk (ORCPT ); Thu, 19 Jan 2017 07:47:40 -0500 Received: from mail-pf0-f181.google.com ([209.85.192.181]:34996 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752364AbdASMrj (ORCPT ); Thu, 19 Jan 2017 07:47:39 -0500 Subject: Re: [PATCH v20 08/17] clocksource/drivers/arm_arch_timer: Rework counter frequency detection. To: Fu Wei References: <20170118132541.8989-1-fu.wei@linaro.org> <20170118132541.8989-9-fu.wei@linaro.org> Cc: "Rafael J. Wysocki" , Len Brown , Daniel Lezcano , Thomas Gleixner , Marc Zyngier , Mark Rutland , Lorenzo Pieralisi , Sudeep Holla , linux-arm-kernel@lists.infradead.org, Linaro ACPI Mailman List , Linux Kernel Mailing List , ACPI Devel Maling List , rruigrok@codeaurora.org, "Abdulhamid, Harb" , Christopher Covington , Timur Tabi , G Gregory , Al Stone , Jon Masters , Wei Huang , Arnd Bergmann , Catalin Marinas , Will Deacon , Suravee Suthikulpanit , Leo Duran , Wim Van Sebroeck , Guenter Roeck , linux-watchdog@vger.kernel.org, Tomasz Nowicki , Christoffer Dall , Julien Grall From: Hanjun Guo Message-ID: <74160889-5eb0-ceb2-2727-61e9d538cf2f@linaro.org> Date: Thu, 19 Jan 2017 20:41:14 +0800 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2017/1/19 17:44, Fu Wei wrote: > Hi Hanjun, > > On 19 January 2017 at 16:02, Hanjun Guo wrote: >> Hi Fuwei, >> >> One comments below. >> >> >> On 2017/1/18 21:25, fu.wei@linaro.org wrote: >>> >>> From: Fu Wei >>> >>> The counter frequency detection call(arch_timer_detect_rate) combines two >>> ways to get counter frequency: system coprocessor register and MMIO timer. >>> But in a specific timer init code, we only need one way to try: >>> getting frequency from MMIO timer register will be needed only when we >>> init MMIO timer; getting frequency from system coprocessor register will >>> be needed only when we init arch timer. >>> >>> This patch separates paths to determine frequency: >>> Separate out the MMIO frequency and the sysreg frequency detection call, >>> and use the appropriate one for the counter. >>> >>> Signed-off-by: Fu Wei >>> --- >>> drivers/clocksource/arm_arch_timer.c | 40 >>> ++++++++++++++++++++++-------------- >>> 1 file changed, 25 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/clocksource/arm_arch_timer.c >>> b/drivers/clocksource/arm_arch_timer.c >>> index 6484f84..9482481 100644 >>> --- a/drivers/clocksource/arm_arch_timer.c >>> +++ b/drivers/clocksource/arm_arch_timer.c >>> @@ -488,23 +488,33 @@ static int arch_timer_starting_cpu(unsigned int cpu) >>> return 0; >>> } >>> >>> -static void arch_timer_detect_rate(void __iomem *cntbase) >>> +static void __arch_timer_determine_rate(u32 rate) >>> { >>> - /* Who has more than one independent system counter? */ >>> - if (arch_timer_rate) >>> - return; >>> + /* Check the timer frequency. */ >>> + if (!arch_timer_rate) { >>> + if (rate) >>> + arch_timer_rate = rate; >>> + else >>> + pr_warn("frequency not available\n"); >>> + } else if (rate && arch_timer_rate != rate) { >> >> ^ >> Typo? I think it's "&" here. > > Not a typo, It's definitely a “&&” :-) > > Here arch_timer_rate is not zero. > > If rate is not zero(that means we got a valid rate), and > arch_timer_rate != rate , > we will print warning message. Ah, misreading the code, thanks for clarify :) Hanjun From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-pf0-f179.google.com ([209.85.192.179]:36671 "EHLO mail-pf0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752364AbdASMty (ORCPT ); Thu, 19 Jan 2017 07:49:54 -0500 Received: by mail-pf0-f179.google.com with SMTP id 189so13506916pfu.3 for ; Thu, 19 Jan 2017 04:49:02 -0800 (PST) Subject: Re: [PATCH v20 08/17] clocksource/drivers/arm_arch_timer: Rework counter frequency detection. To: Fu Wei References: <20170118132541.8989-1-fu.wei@linaro.org> <20170118132541.8989-9-fu.wei@linaro.org> Cc: "Rafael J. Wysocki" , Len Brown , Daniel Lezcano , Thomas Gleixner , Marc Zyngier , Mark Rutland , Lorenzo Pieralisi , Sudeep Holla , linux-arm-kernel@lists.infradead.org, Linaro ACPI Mailman List , Linux Kernel Mailing List , ACPI Devel Maling List , rruigrok@codeaurora.org, "Abdulhamid, Harb" , Christopher Covington , Timur Tabi , G Gregory , Al Stone , Jon Masters , Wei Huang , Arnd Bergmann , Catalin Marinas , Will Deacon , Suravee Suthikulpanit , Leo Duran , Wim Van Sebroeck , Guenter Roeck , linux-watchdog@vger.kernel.org, Tomasz Nowicki , Christoffer Dall , Julien Grall From: Hanjun Guo Message-ID: <74160889-5eb0-ceb2-2727-61e9d538cf2f@linaro.org> Date: Thu, 19 Jan 2017 20:41:14 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Sender: linux-watchdog-owner@vger.kernel.org List-Id: linux-watchdog@vger.kernel.org Content-Transfer-Encoding: quoted-printable On 2017/1/19 17:44, Fu Wei wrote: > Hi Hanjun, > > On 19 January 2017 at 16:02, Hanjun Guo wrote: >> Hi Fuwei, >> >> One comments below. >> >> >> On 2017/1/18 21:25, fu.wei@linaro.org wrote: >>> >>> From: Fu Wei >>> >>> The counter frequency detection call(arch_timer_detect_rate) combines= two >>> ways to get counter frequency: system coprocessor register and MMIO t= imer. >>> But in a specific timer init code, we only need one way to try: >>> getting frequency from MMIO timer register will be needed only when w= e >>> init MMIO timer; getting frequency from system coprocessor register w= ill >>> be needed only when we init arch timer. >>> >>> This patch separates paths to determine frequency: >>> Separate out the MMIO frequency and the sysreg frequency detection ca= ll, >>> and use the appropriate one for the counter. >>> >>> Signed-off-by: Fu Wei >>> --- >>> drivers/clocksource/arm_arch_timer.c | 40 >>> ++++++++++++++++++++++-------------- >>> 1 file changed, 25 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/clocksource/arm_arch_timer.c >>> b/drivers/clocksource/arm_arch_timer.c >>> index 6484f84..9482481 100644 >>> --- a/drivers/clocksource/arm_arch_timer.c >>> +++ b/drivers/clocksource/arm_arch_timer.c >>> @@ -488,23 +488,33 @@ static int arch_timer_starting_cpu(unsigned int= cpu) >>> return 0; >>> } >>> >>> -static void arch_timer_detect_rate(void __iomem *cntbase) >>> +static void __arch_timer_determine_rate(u32 rate) >>> { >>> - /* Who has more than one independent system counter? */ >>> - if (arch_timer_rate) >>> - return; >>> + /* Check the timer frequency. */ >>> + if (!arch_timer_rate) { >>> + if (rate) >>> + arch_timer_rate =3D rate; >>> + else >>> + pr_warn("frequency not available\n"); >>> + } else if (rate && arch_timer_rate !=3D rate) { >> >> ^ >> Typo? I think it's "&" here. > > Not a typo, It's definitely a =E2=80=9C&&=E2=80=9D :-) > > Here arch_timer_rate is not zero. > > If rate is not zero(that means we got a valid rate), and > arch_timer_rate !=3D rate , > we will print warning message. Ah, misreading the code, thanks for clarify :) Hanjun -- To unsubscribe from this list: send the line "unsubscribe linux-watchdog"= in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html From mboxrd@z Thu Jan 1 00:00:00 1970 From: hanjun.guo@linaro.org (Hanjun Guo) Date: Thu, 19 Jan 2017 20:41:14 +0800 Subject: [PATCH v20 08/17] clocksource/drivers/arm_arch_timer: Rework counter frequency detection. In-Reply-To: References: <20170118132541.8989-1-fu.wei@linaro.org> <20170118132541.8989-9-fu.wei@linaro.org> Message-ID: <74160889-5eb0-ceb2-2727-61e9d538cf2f@linaro.org> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2017/1/19 17:44, Fu Wei wrote: > Hi Hanjun, > > On 19 January 2017 at 16:02, Hanjun Guo wrote: >> Hi Fuwei, >> >> One comments below. >> >> >> On 2017/1/18 21:25, fu.wei at linaro.org wrote: >>> >>> From: Fu Wei >>> >>> The counter frequency detection call(arch_timer_detect_rate) combines two >>> ways to get counter frequency: system coprocessor register and MMIO timer. >>> But in a specific timer init code, we only need one way to try: >>> getting frequency from MMIO timer register will be needed only when we >>> init MMIO timer; getting frequency from system coprocessor register will >>> be needed only when we init arch timer. >>> >>> This patch separates paths to determine frequency: >>> Separate out the MMIO frequency and the sysreg frequency detection call, >>> and use the appropriate one for the counter. >>> >>> Signed-off-by: Fu Wei >>> --- >>> drivers/clocksource/arm_arch_timer.c | 40 >>> ++++++++++++++++++++++-------------- >>> 1 file changed, 25 insertions(+), 15 deletions(-) >>> >>> diff --git a/drivers/clocksource/arm_arch_timer.c >>> b/drivers/clocksource/arm_arch_timer.c >>> index 6484f84..9482481 100644 >>> --- a/drivers/clocksource/arm_arch_timer.c >>> +++ b/drivers/clocksource/arm_arch_timer.c >>> @@ -488,23 +488,33 @@ static int arch_timer_starting_cpu(unsigned int cpu) >>> return 0; >>> } >>> >>> -static void arch_timer_detect_rate(void __iomem *cntbase) >>> +static void __arch_timer_determine_rate(u32 rate) >>> { >>> - /* Who has more than one independent system counter? */ >>> - if (arch_timer_rate) >>> - return; >>> + /* Check the timer frequency. */ >>> + if (!arch_timer_rate) { >>> + if (rate) >>> + arch_timer_rate = rate; >>> + else >>> + pr_warn("frequency not available\n"); >>> + } else if (rate && arch_timer_rate != rate) { >> >> ^ >> Typo? I think it's "&" here. > > Not a typo, It's definitely a ?&&? :-) > > Here arch_timer_rate is not zero. > > If rate is not zero(that means we got a valid rate), and > arch_timer_rate != rate , > we will print warning message. Ah, misreading the code, thanks for clarify :) Hanjun