From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fu Wei Subject: Re: [PATCH v21 01/13] clocksource: arm_arch_timer: introduce two functions to get the frequency from mmio and sysreg. Date: Mon, 20 Mar 2017 17:43:29 +0800 Message-ID: References: <20170206185015.12296-1-fu.wei@linaro.org> <20170206185015.12296-2-fu.wei@linaro.org> <20170317180513.GB15909@leverpostej> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 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: Mark Rutland Cc: 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.infradead.org, G Gregory List-Id: linux-acpi@vger.kernel.org Hi Mark, On 20 March 2017 at 15:36, Fu Wei wrote: > Hi Mark, > > On 18 March 2017 at 02:05, Mark Rutland wrote: >> On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei@linaro.org wrote: >>> +static u32 arch_timer_get_sysreg_freq(void) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of sysreg. >>> + */ >>> + return arch_timer_get_cntfrq(); >>> +} >> >> We already have arch_timer_get_cntfrq(), so I don't see the point in >> this wrapper. >> >>> +static u32 arch_timer_get_mmio_freq(void __iomem *cntbase) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of timer frame registers. >>> + * Note: please verify cntbase in caller. >>> + */ >>> + return readl_relaxed(cntbase + CNTFRQ); >>> +} >> >> Wrapping the MMIO read makes sense if we're going to do this in more >> than one place, so I'm happy with this wrapper. >> >> If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop > > sorry, May I guess that is > "s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/" > or > "s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/" > > which one do you prefer? :-) keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then +static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase) +{ + return readl_relaxed(cntbase + CNTFRQ); +} + Is that OK for you? > >> the comments, then this looks fine to me. >> >> Thanks, >> Mark. > > > > -- > Best regards, > > Fu Wei > Software Engineer > Red Hat -- Best regards, Fu Wei Software Engineer Red Hat From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753342AbdCTJpM (ORCPT ); Mon, 20 Mar 2017 05:45:12 -0400 Received: from mail-it0-f52.google.com ([209.85.214.52]:33665 "EHLO mail-it0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753170AbdCTJnn (ORCPT ); Mon, 20 Mar 2017 05:43:43 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170206185015.12296-1-fu.wei@linaro.org> <20170206185015.12296-2-fu.wei@linaro.org> <20170317180513.GB15909@leverpostej> From: Fu Wei Date: Mon, 20 Mar 2017 17:43:29 +0800 Message-ID: Subject: Re: [PATCH v21 01/13] clocksource: arm_arch_timer: introduce two functions to get the frequency from mmio and sysreg. To: Mark Rutland Cc: "Rafael J. Wysocki" , Len Brown , Daniel Lezcano , Thomas Gleixner , Marc Zyngier , Lorenzo Pieralisi , Sudeep Holla , Hanjun Guo , 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 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 Mark, On 20 March 2017 at 15:36, Fu Wei wrote: > Hi Mark, > > On 18 March 2017 at 02:05, Mark Rutland wrote: >> On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei@linaro.org wrote: >>> +static u32 arch_timer_get_sysreg_freq(void) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of sysreg. >>> + */ >>> + return arch_timer_get_cntfrq(); >>> +} >> >> We already have arch_timer_get_cntfrq(), so I don't see the point in >> this wrapper. >> >>> +static u32 arch_timer_get_mmio_freq(void __iomem *cntbase) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of timer frame registers. >>> + * Note: please verify cntbase in caller. >>> + */ >>> + return readl_relaxed(cntbase + CNTFRQ); >>> +} >> >> Wrapping the MMIO read makes sense if we're going to do this in more >> than one place, so I'm happy with this wrapper. >> >> If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop > > sorry, May I guess that is > "s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/" > or > "s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/" > > which one do you prefer? :-) keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then +static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase) +{ + return readl_relaxed(cntbase + CNTFRQ); +} + Is that OK for you? > >> the comments, then this looks fine to me. >> >> Thanks, >> Mark. > > > > -- > Best regards, > > Fu Wei > Software Engineer > Red Hat -- Best regards, Fu Wei Software Engineer Red Hat From mboxrd@z Thu Jan 1 00:00:00 1970 From: fu.wei@linaro.org (Fu Wei) Date: Mon, 20 Mar 2017 17:43:29 +0800 Subject: [PATCH v21 01/13] clocksource: arm_arch_timer: introduce two functions to get the frequency from mmio and sysreg. In-Reply-To: References: <20170206185015.12296-1-fu.wei@linaro.org> <20170206185015.12296-2-fu.wei@linaro.org> <20170317180513.GB15909@leverpostej> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org Hi Mark, On 20 March 2017 at 15:36, Fu Wei wrote: > Hi Mark, > > On 18 March 2017 at 02:05, Mark Rutland wrote: >> On Tue, Feb 07, 2017 at 02:50:03AM +0800, fu.wei at linaro.org wrote: >>> +static u32 arch_timer_get_sysreg_freq(void) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of sysreg. >>> + */ >>> + return arch_timer_get_cntfrq(); >>> +} >> >> We already have arch_timer_get_cntfrq(), so I don't see the point in >> this wrapper. >> >>> +static u32 arch_timer_get_mmio_freq(void __iomem *cntbase) >>> +{ >>> + /* >>> + * Try to get the frequency from the CNTFRQ of timer frame registers. >>> + * Note: please verify cntbase in caller. >>> + */ >>> + return readl_relaxed(cntbase + CNTFRQ); >>> +} >> >> Wrapping the MMIO read makes sense if we're going to do this in more >> than one place, so I'm happy with this wrapper. >> >> If you can s/arch_timer_get_mmio_freq/arch_timer_get_cntfrq/, and drop > > sorry, May I guess that is > "s/arch_timer_get_mmio_freq/arch_timer_get_mmio_cntfrq/" > or > "s/arch_timer_get_mmio_freq/arch_timer_mem_get_cntfrq/" > > which one do you prefer? :-) keeping using arch_timer_get_cntfrq(); for per-CPU arch timer, then +static u32 arch_timer_mem_get_cntfrq(void __iomem *cntbase) +{ + return readl_relaxed(cntbase + CNTFRQ); +} + Is that OK for you? > >> the comments, then this looks fine to me. >> >> Thanks, >> Mark. > > > > -- > Best regards, > > Fu Wei > Software Engineer > Red Hat -- Best regards, Fu Wei Software Engineer Red Hat