Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752641AbcDCTXp (ORCPT ); Sun, 3 Apr 2016 15:23:45 -0400 Received: from mail-oi0-f51.google.com ([209.85.218.51]:34928 "EHLO mail-oi0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752313AbcDCTXn (ORCPT ); Sun, 3 Apr 2016 15:23:43 -0400 MIME-Version: 1.0 In-Reply-To: <1459589383-16914-3-git-send-email-guodong.xu@linaro.org> References: <1459589383-16914-1-git-send-email-guodong.xu@linaro.org> <1459589383-16914-3-git-send-email-guodong.xu@linaro.org> Date: Sun, 3 Apr 2016 21:23:42 +0200 Message-ID: Subject: Re: [PATCH v2 02/16] arm64: dts: add sp804 timer node for Hi6220 From: Linus Walleij To: Guodong Xu Cc: Xu Wei , Mark Rutland , Rob Herring , Grant Likely , Arnd Bergmann , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , XinWei Kong , Leo Yan Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2233 Lines: 57 On Sat, Apr 2, 2016 at 11:29 AM, Guodong Xu wrote: > From: Leo Yan > > Add sp804 timer for hi6220, so it can be used as broadcast timer. > > Signed-off-by: Leo Yan > Signed-off-by: Wei Xu > --- > arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > index ad1f1eb..82c4756 100644 > --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi > @@ -209,5 +209,14 @@ > clock-names = "uartclk", "apb_pclk"; > status = "disabled"; > }; > + > + dual_timer0: dual_timer@f8008000 { > + compatible = "arm,sp804", "arm,primecell"; > + reg = <0x0 0xf8008000 0x0 0x1000>; > + interrupts = , > + ; > + clocks = <&ao_ctrl 27>; > + clock-names = "apb_pclk"; How can this work? You only give the apb_pclk for clocking the bus to the timer. Most platforms using this driver has something like this: timer01: timer@10104000 { compatible = "arm,sp804", "arm,primecell"; reg = <0x10104000 0x1000>; interrupt-parent = <&intc_dc1176>; interrupts = <0 8 IRQ_TYPE_LEVEL_HIGH>, <0 9 IRQ_TYPE_LEVEL_HIGH>; clocks = <&timclk>, <&timclk>, <&pclk>; clock-names = "timer1", "timer2", "apb_pclk"; }; It then reads the two clocks in the beginning of clocks() to determine the frequency of each timer. By chance the code in the driver will allow just one clock and will then assume that both the bus to the timer and the timer itself is clocked from the same clock. But I highly doubt that this is the case. Please verify what clocks actually goes into this timer, it should nominally be three of them. Yours, Linus Walleij