linux-kernel - Re: [PATCH v2 02/16] arm64: dts: add sp804 timer node for Hi6220

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 3 Apr 2016 21:23:42 +0200
From:	Linus Walleij <linus.walleij@...aro.org>
To:	Guodong Xu <guodong.xu@...aro.org>
Cc:	Xu Wei <xuwei5@...ilicon.com>, Mark Rutland <mark.rutland@....com>,
	Rob Herring <robh@...nel.org>,
	Grant Likely <grant.likely@...retlab.ca>,
	Arnd Bergmann <arnd.bergmann@...aro.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	XinWei Kong <kong.kongxinwei@...ilicon.com>,
	Leo Yan <leo.yan@...aro.org>
Subject: Re: [PATCH v2 02/16] arm64: dts: add sp804 timer node for Hi6220

On Sat, Apr 2, 2016 at 11:29 AM, Guodong Xu <guodong.xu@...aro.org> wrote:

> From: Leo Yan <leo.yan@...aro.org>
>
> Add sp804 timer for hi6220, so it can be used as broadcast timer.
>
> Signed-off-by: Leo Yan <leo.yan@...aro.org>
> Signed-off-by: Wei Xu <xuwei5@...ilicon.com>
> ---
>  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@...08000 {
> +                       compatible = "arm,sp804", "arm,primecell";
> +                       reg = <0x0 0xf8008000 0x0 0x1000>;
> +                       interrupts = <GIC_SPI 14 IRQ_TYPE_LEVEL_HIGH>,
> +                                    <GIC_SPI 15 IRQ_TYPE_LEVEL_HIGH>;
> +                       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@...04000 {
                        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