[PATCH 0/5] Allwinner SoCs High Speed Timer support

[PATCH 0/5] Allwinner SoCs High Speed Timer support

[Maxime Ripard]

Hi everyone,

Here is a few patches adding support for the High Speed Timers running on the
Allwinner SoCs.

These timers are 64 bits timers running at a much higher speed than the timers
used for now on these SoCs, since they are no longer wired to the 24MHz
oscillator, but to the AHB clock.

This HS timers are actually found in all the supported SoCs but the A10.
However, the A20 and A31 come with 4 of these high speed timers, while the A10s
and A13 only have two, hence why we introduce two different compatibles.

The A31 is not using these for now, as its timers are asserted in reset by a
reset controller that first need to gain some support in the kernel first, but
that's for another patchset.

Thanks,
Maxime

Maxime Ripard (5):
  clocksource: sun4i: Select CLKSRC_MMIO
  clocksource: Add Allwinner SoCs HS timers driver
  ARM: sun5i: a10s: Add support for the High Speed Timers
  ARM: sun5i: a13: Add support for the High Speed Timers
  ARM: sun7i: a20: Add support for the High Speed Timers

 .../bindings/timer/allwinner,sun5i-a13-hstimer.txt |  22 +++
 arch/arm/boot/dts/sun5i-a10s.dtsi                  |   7 +
 arch/arm/boot/dts/sun5i-a13.dtsi                   |   7 +
 arch/arm/boot/dts/sun7i-a20.dtsi                   |  10 ++
 arch/arm/mach-sunxi/Kconfig                        |   1 +
 drivers/clocksource/Kconfig                        |   5 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-sun5i.c                  | 189 +++++++++++++++++++++
 8 files changed, 242 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
 create mode 100644 drivers/clocksource/timer-sun5i.c

-- 
1.8.4

[PATCH 1/5] clocksource: sun4i: Select CLKSRC_MMIO

[Maxime Ripard]

The Allwinner SoCs timer use the clocksource MMIO functions. We thus
need to select them in Kconfig.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 drivers/clocksource/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 41c6946..cea50f0 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -33,6 +33,7 @@ config ORION_TIMER
 	bool
 
 config SUN4I_TIMER
+	select CLKSRC_MMIO
 	bool
 
 config VT8500_TIMER
-- 
1.8.4

[PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

Most of the Allwinner SoCs (at this time, all but the A10) also have a
High Speed timers that are not using the 24MHz oscillator as a source
but rather the AHB clock running much faster.

The IP is slightly different between the A10s/A13 and the one used in
the A20/A31, since the latter have 4 timers available, while the former
have only 2 of them.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 .../bindings/timer/allwinner,sun5i-a13-hstimer.txt |  22 +++
 arch/arm/mach-sunxi/Kconfig                        |   1 +
 drivers/clocksource/Kconfig                        |   4 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-sun5i.c                  | 189 +++++++++++++++++++++
 5 files changed, 217 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
 create mode 100644 drivers/clocksource/timer-sun5i.c

diff --git a/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
new file mode 100644
index 0000000..b1f81e9
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
@@ -0,0 +1,22 @@
+Allwinner SoCs High Speed Timer Controller
+
+Required properties:
+
+- compatible :	should be "allwinner,sun5i-a13-hstimer" or
+		"allwinner,sun7i-a20-hstimer"
+- reg : Specifies base physical address and size of the registers.
+- interrupts :	The interrupts of these timers (2 for the sun5i IP, 4 for the sun7i
+		one)
+- clocks: phandle to the source clock (usually the AHB clock)
+
+Example:
+
+hstimer@01c60000 {
+	compatible = "allwinner,sun7i-a20-hstimer";
+        reg = <0x01c60000 0x1000>;
+        interrupts = <0 51 1>,
+                     <0 52 1>,
+                     <0 53 1>,
+                     <0 54 1>;
+        clocks = <&ahb1_gates 19>;
+};
diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
index 3ab2f65..b78931d 100644
--- a/arch/arm/mach-sunxi/Kconfig
+++ b/arch/arm/mach-sunxi/Kconfig
@@ -9,6 +9,7 @@ config ARCH_SUNXI
 	select PINCTRL
 	select SPARSE_IRQ
 	select SUN4I_TIMER
+	select SUN5I_HSTIMER
 	select PINCTRL_SUNXI
 	select ARM_GIC
 	select HAVE_SMP
diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cea50f0..4f53439 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -36,6 +36,10 @@ config SUN4I_TIMER
 	select CLKSRC_MMIO
 	bool
 
+config SUN5I_HSTIMER
+	select CLKSRC_MMIO
+	bool
+
 config VT8500_TIMER
 	bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 704d6d3..30488d6 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
 obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
+obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra20_timer.o
 obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
 obj-$(CONFIG_ARCH_NSPIRE)	+= zevio-timer.o
diff --git a/drivers/clocksource/timer-sun5i.c b/drivers/clocksource/timer-sun5i.c
new file mode 100644
index 0000000..dc39a9d
--- /dev/null
+++ b/drivers/clocksource/timer-sun5i.c
@@ -0,0 +1,189 @@
+/*
+ * Allwinner SoCs hstimer driver.
+ *
+ * Copyright (C) 2013 Maxime Ripard
+ *
+ * Maxime Ripard <maxime.ripard@free-electrons.com>
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2.  This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+
+#include <linux/clk.h>
+#include <linux/clockchips.h>
+#include <linux/delay.h>
+#include <linux/interrupt.h>
+#include <linux/irq.h>
+#include <linux/irqreturn.h>
+#include <linux/sched_clock.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+#define TIMER_IRQ_EN_REG		0x00
+#define TIMER_IRQ_EN(val)			BIT(val)
+#define TIMER_IRQ_ST_REG		0x04
+#define TIMER_CTL_REG(val)		(0x20 * (val) + 0x10)
+#define TIMER_CTL_ENABLE			BIT(0)
+#define TIMER_CTL_RELOAD			BIT(1)
+#define TIMER_CTL_CLK_PRES(val)			(((val) & 0x7) << 4)
+#define TIMER_CTL_ONESHOT			BIT(7)
+#define TIMER_INTVAL_LO_REG(val)	(0x20 * (val) + 0x14)
+#define TIMER_INTVAL_HI_REG(val)	(0x20 * (val) + 0x18)
+#define TIMER_CNTVAL_LO_REG(val)	(0x20 * (val) + 0x1c)
+#define TIMER_CNTVAL_HI_REG(val)	(0x20 * (val) + 0x20)
+
+static void __iomem *timer_base;
+static u32 ticks_per_jiffy;
+
+/*
+ * When we disable a timer, we need to wait at least for 2 cycles of
+ * the timer source clock. We will use for that the clocksource timer
+ * that is already setup and runs at the same frequency than the other
+ * timers, and we never will be disabled.
+ */
+static void sun5i_clkevt_sync(void)
+{
+	u32 old = readl(timer_base + TIMER_CNTVAL_LO_REG(1));
+
+	while ((old - readl(timer_base + TIMER_CNTVAL_LO_REG(1))) < 3)
+		cpu_relax();
+}
+
+static void sun5i_clkevt_time_stop(u8 timer)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+	writel(val & ~TIMER_CTL_ENABLE, timer_base + TIMER_CTL_REG(timer));
+
+	sun5i_clkevt_sync();
+}
+
+static void sun5i_clkevt_time_setup(u8 timer, u32 delay)
+{
+	writel(delay, timer_base + TIMER_INTVAL_LO_REG(timer));
+}
+
+static void sun5i_clkevt_time_start(u8 timer, bool periodic)
+{
+	u32 val = readl(timer_base + TIMER_CTL_REG(timer));
+
+	if (periodic)
+		val &= ~TIMER_CTL_ONESHOT;
+	else
+		val |= TIMER_CTL_ONESHOT;
+
+	writel(val | TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(timer));
+}
+
+static void sun5i_clkevt_mode(enum clock_event_mode mode,
+			      struct clock_event_device *clk)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_PERIODIC:
+		sun5i_clkevt_time_stop(0);
+		sun5i_clkevt_time_setup(0, ticks_per_jiffy);
+		sun5i_clkevt_time_start(0, true);
+		break;
+	case CLOCK_EVT_MODE_ONESHOT:
+		sun5i_clkevt_time_stop(0);
+		sun5i_clkevt_time_start(0, false);
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	default:
+		sun5i_clkevt_time_stop(0);
+		break;
+	}
+}
+
+static int sun5i_clkevt_next_event(unsigned long evt,
+				   struct clock_event_device *unused)
+{
+	sun5i_clkevt_time_stop(0);
+	sun5i_clkevt_time_setup(0, evt);
+	sun5i_clkevt_time_start(0, false);
+
+	return 0;
+}
+
+static struct clock_event_device sun5i_clockevent = {
+	.name = "sun5i_tick",
+	.rating = 350,
+	.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
+	.set_mode = sun5i_clkevt_mode,
+	.set_next_event = sun5i_clkevt_next_event,
+};
+
+
+static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
+
+	writel(0x1, timer_base + TIMER_IRQ_ST_REG);
+	evt->event_handler(evt);
+
+	return IRQ_HANDLED;
+}
+
+static struct irqaction sun5i_timer_irq = {
+	.name = "sun5i_timer0",
+	.flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
+	.handler = sun5i_timer_interrupt,
+	.dev_id = &sun5i_clockevent,
+};
+
+static u32 sun5i_timer_sched_read(void)
+{
+	return ~readl(timer_base + TIMER_CNTVAL_LO_REG(1));
+}
+
+static void __init sun5i_timer_init(struct device_node *node)
+{
+	unsigned long rate;
+	struct clk *clk;
+	int ret, irq;
+	u32 val;
+
+	timer_base = of_iomap(node, 0);
+	if (!timer_base)
+		panic("Can't map registers");
+
+	irq = irq_of_parse_and_map(node, 0);
+	if (irq <= 0)
+		panic("Can't parse IRQ");
+
+	clk = of_clk_get(node, 0);
+	if (IS_ERR(clk))
+		panic("Can't get timer clock");
+	clk_prepare_enable(clk);
+	rate = clk_get_rate(clk);
+
+	writel(~0, timer_base + TIMER_INTVAL_LO_REG(1));
+	writel(TIMER_CTL_ENABLE | TIMER_CTL_RELOAD,
+	       timer_base + TIMER_CTL_REG(1));
+
+	setup_sched_clock(sun5i_timer_sched_read, 32, rate);
+	clocksource_mmio_init(timer_base + TIMER_CNTVAL_LO_REG(1), node->name,
+			      rate, 350, 32, clocksource_mmio_readl_down);
+
+	ticks_per_jiffy = DIV_ROUND_UP(rate, HZ);
+
+	ret = setup_irq(irq, &sun5i_timer_irq);
+	if (ret)
+		pr_warn("failed to setup irq %d\n", irq);
+
+	/* Enable timer0 interrupt */
+	val = readl(timer_base + TIMER_IRQ_EN_REG);
+	writel(val | TIMER_IRQ_EN(0), timer_base + TIMER_IRQ_EN_REG);
+
+	sun5i_clockevent.cpumask = cpumask_of(0);
+
+	clockevents_config_and_register(&sun5i_clockevent, rate, 0x1,
+					0xffffffff);
+}
+CLOCKSOURCE_OF_DECLARE(sun5i_a13, "allwinner,sun5i-a13-hstimer",
+		       sun5i_timer_init);
+CLOCKSOURCE_OF_DECLARE(sun7i_a20, "allwinner,sun7i-a20-hstimer",
+		       sun5i_timer_init);
-- 
1.8.4

[PATCH 3/5] ARM: sun5i: a10s: Add support for the High Speed Timers

[Maxime Ripard]

The Allwinner A10s has support for two high speed timers. Now that we
have a driver to support it, we can enable them in the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-a10s.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a10s.dtsi b/arch/arm/boot/dts/sun5i-a10s.dtsi
index 5247674..317490c 100644
--- a/arch/arm/boot/dts/sun5i-a10s.dtsi
+++ b/arch/arm/boot/dts/sun5i-a10s.dtsi
@@ -332,5 +332,12 @@
 			clock-frequency = <100000>;
 			status = "disabled";
 		};
+
+		hstimer@01c60000 {
+			compatible = "allwinner,sun5i-a13-hstimer";
+			reg = <0x01c60000 0x1000>;
+			interrupts = <82>, <83>;
+			clocks = <&ahb_gates 28>;
+		};
 	};
 };
-- 
1.8.4

[PATCH 4/5] ARM: sun5i: a13: Add support for the High Speed Timers

[Maxime Ripard]

The Allwinner A13 has support for two high speed timers. Now that we
have a driver to support it, we can enable them in the device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun5i-a13.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/sun5i-a13.dtsi b/arch/arm/boot/dts/sun5i-a13.dtsi
index ce8ef2a..a1c5af2 100644
--- a/arch/arm/boot/dts/sun5i-a13.dtsi
+++ b/arch/arm/boot/dts/sun5i-a13.dtsi
@@ -273,5 +273,12 @@
 			clock-frequency = <100000>;
 			status = "disabled";
 		};
+
+		hstimer@01c60000 {
+			compatible = "allwinner,sun5i-a13-hstimer";
+			reg = <0x01c60000 0x1000>;
+			interrupts = <82>, <83>;
+			clocks = <&ahb_gates 28>;
+		};
 	};
 };
-- 
1.8.4

[PATCH 5/5] ARM: sun7i: a20: Add support for the High Speed Timers

[Maxime Ripard]

The Allwinner A20 has support for four high speed timers. Apart for the
number of timers (4 vs 2), it's basically the same logic than the high
speed timers found in the sun5i chips.

Now that we have a driver to support it, we can enable them in the
device tree.

Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
---
 arch/arm/boot/dts/sun7i-a20.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/sun7i-a20.dtsi b/arch/arm/boot/dts/sun7i-a20.dtsi
index e46cfed..ee6cec7 100644
--- a/arch/arm/boot/dts/sun7i-a20.dtsi
+++ b/arch/arm/boot/dts/sun7i-a20.dtsi
@@ -395,6 +395,16 @@
 			status = "disabled";
 		};
 
+		hstimer@01c60000 {
+			compatible = "allwinner,sun7i-a20-hstimer";
+			reg = <0x01c60000 0x1000>;
+			interrupts = <0 81 1>,
+				     <0 82 1>,
+				     <0 83 1>,
+				     <0 84 1>;
+			clocks = <&ahb_gates 28>;
+		};
+
 		gic: interrupt-controller@01c81000 {
 			compatible = "arm,cortex-a7-gic", "arm,cortex-a15-gic";
 			reg = <0x01c81000 0x1000>,
-- 
1.8.4

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Kevin Hilman]

Maxime Ripard <maxime.ripard@free-electrons.com> writes:

> Hi everyone,
>
> Here is a few patches adding support for the High Speed Timers running on the
> Allwinner SoCs.
>
> These timers are 64 bits timers running at a much higher speed than the timers
> used for now on these SoCs, since they are no longer wired to the 24MHz
> oscillator, but to the AHB clock.
>
> This HS timers are actually found in all the supported SoCs but the A10.

hmm, timers not found in the A10...

> However, the A20 and A31 come with 4 of these high speed timers, while the A10s
> and A13 only have two, hence why we introduce two different compatibles.

...but the A10 has 2?

-ECONFUSED

Kevin

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Olof Johansson]

On Wed, Sep 25, 2013 at 11:13 AM, Kevin Hilman <khilman@linaro.org> wrote:

>> However, the A20 and A31 come with 4 of these high speed timers, while the A10s
>> and A13 only have two, hence why we introduce two different compatibles.
>
> ...but the A10 has 2?
>
> -ECONFUSED

A10s != A10. Yes, confusing. :)


-Olof

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 963 bytes --]

On Wed, Sep 25, 2013 at 11:14:46AM -0700, Olof Johansson wrote:
> On Wed, Sep 25, 2013 at 11:13 AM, Kevin Hilman <khilman@linaro.org> wrote:
> 
> >> However, the A20 and A31 come with 4 of these high speed timers, while the A10s
> >> and A13 only have two, hence why we introduce two different compatibles.
> >
> > ...but the A10 has 2?
> >
> > -ECONFUSED
> 
> A10s != A10. Yes, confusing. :)

Yeah, I didn't get to chose the names :)

For all we know, so far, allwinner has released, by family:
  - sun3i (ARM926)
    * F20 (not supported)
  - sun4i (Cortex A8)
    * A10
  - sun5i (Cortex A8)
    * A10s
    * A13
  - sun6i (4 * Cortex A7)
    * A31
    * A31s (not supported)
  - sun7i (2 * Cortex A7)
    * A20
    * A23? (that was just announced)

So yes, even though they are quite close, A10 != A10s :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Kevin Hilman]

Olof Johansson <olof@lixom.net> writes:

> On Wed, Sep 25, 2013 at 11:13 AM, Kevin Hilman <khilman@linaro.org> wrote:
>
>>> However, the A20 and A31 come with 4 of these high speed timers,
>>> while the A10s
>>> and A13 only have two, hence why we introduce two different compatibles.
>>
>> ...but the A10 has 2?
>>
>> -ECONFUSED
>
> A10s != A10. Yes, confusing. :)

Aaaaaah.  Silly me for thinking that the trailing 's' meant plural.

Thanks for clarifying.

Kevin

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Emilio López]

Hi Maxime,

El 25/09/13 11:03, Maxime Ripard escribió:
> Hi everyone,
>
> Here is a few patches adding support for the High Speed Timers running on the
> Allwinner SoCs.
>
> These timers are 64 bits timers running at a much higher speed than the timers
> used for now on these SoCs, since they are no longer wired to the 24MHz
> oscillator, but to the AHB clock.
>
> This HS timers are actually found in all the supported SoCs but the A10.
> However, the A20 and A31 come with 4 of these high speed timers, while the A10s
> and A13 only have two, hence why we introduce two different compatibles.
>
> The A31 is not using these for now, as its timers are asserted in reset by a
> reset controller that first need to gain some support in the kernel first, but
> that's for another patchset.
>
> Thanks,
> Maxime
>
> Maxime Ripard (5):
>    clocksource: sun4i: Select CLKSRC_MMIO
>    clocksource: Add Allwinner SoCs HS timers driver
>    ARM: sun5i: a10s: Add support for the High Speed Timers
>    ARM: sun5i: a13: Add support for the High Speed Timers
>    ARM: sun7i: a20: Add support for the High Speed Timers

I tested these 5 patches on my Cubieboard2 (A20) and it boots and seems 
to work fine, so

Tested-by: Emilio López <emilio@elopez.com.ar>

# uptime && cat /proc/interrupts
  01:45:01 up  1:45,  load average: 0.01, 0.03, 0.04
            CPU0
  33:      56223       GIC  33  serial
  54:          0       GIC  54  sun4i_timer0
  87:        522       GIC  87  eth0
113:      66980       GIC 113  sun5i_timer0
IPI0:          0  CPU wakeup interrupts
IPI1:          0  Timer broadcast interrupts
IPI2:          0  Rescheduling interrupts
IPI3:          0  Function call interrupts
IPI4:          0  Single function call interrupts
IPI5:          0  CPU stop interrupts
Err:          0

Cheers,

Emilio

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Stephen Boyd]

On 09/25/13 07:03, Maxime Ripard wrote:
> diff --git a/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> new file mode 100644
> index 0000000..b1f81e9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> @@ -0,0 +1,22 @@
> +Allwinner SoCs High Speed Timer Controller
> +
> +Required properties:
> +
> +- compatible :	should be "allwinner,sun5i-a13-hstimer" or
> +		"allwinner,sun7i-a20-hstimer"
> +- reg : Specifies base physical address and size of the registers.
> +- interrupts :	The interrupts of these timers (2 for the sun5i IP, 4 for the sun7i
> +		one)
> +- clocks: phandle to the source clock (usually the AHB clock)
> +
> +Example:
> +
> +hstimer@01c60000 {

This should just be 'timer@1c60000'

> +	compatible = "allwinner,sun7i-a20-hstimer";
> +        reg = <0x01c60000 0x1000>;
> +        interrupts = <0 51 1>,
> +                     <0 52 1>,
> +                     <0 53 1>,
> +                     <0 54 1>;
> +        clocks = <&ahb1_gates 19>;
> +};

Weird mix of tabs and spaces here.

> +
> +static void __iomem *timer_base;
> +static u32 ticks_per_jiffy;
> +
> +/*
> + * When we disable a timer, we need to wait at least for 2 cycles of
> + * the timer source clock. We will use for that the clocksource timer
> + * that is already setup and runs at the same frequency than the other
> + * timers, and we never will be disabled.
> + */
> +static void sun5i_clkevt_sync(void)
> +{
> +	u32 old = readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> +
> +	while ((old - readl(timer_base + TIMER_CNTVAL_LO_REG(1))) < 3)
> +		cpu_relax();
> +}
> +
[...]
> +
> +static int sun5i_clkevt_next_event(unsigned long evt,
> +				   struct clock_event_device *unused)
> +{
> +	sun5i_clkevt_time_stop(0);
> +	sun5i_clkevt_time_setup(0, evt);
> +	sun5i_clkevt_time_start(0, false);

I suppose the min delta wants to be 3 instead of 1 because if we program
an evt one tick in the future first we'll wait for two ticks (or is that
three?) while we stop the timer and then program the timer to fire one
tick after that. Perhaps we should subtract two ticks from the evt as
well when we program it here?

> +
> +	return 0;
> +}
> +
> +static struct clock_event_device sun5i_clockevent = {
> +	.name = "sun5i_tick",
> +	.rating = 350,
> +	.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> +	.set_mode = sun5i_clkevt_mode,
> +	.set_next_event = sun5i_clkevt_next_event,
> +};
> +
> +
> +static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
> +{
> +	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
> +
> +	writel(0x1, timer_base + TIMER_IRQ_ST_REG);
> +	evt->event_handler(evt);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static struct irqaction sun5i_timer_irq = {
> +	.name = "sun5i_timer0",
> +	.flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,

IRQF_DISABLED is a no-op and can be dropped.

> +
> +static u32 sun5i_timer_sched_read(void)
> +{
> +	return ~readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> +}
> +
> +static void __init sun5i_timer_init(struct device_node *node)
> +{
[...]
> +
> +	sun5i_clockevent.cpumask = cpumask_of(0);

Can this timer interrupt any CPU or is it hardwired to CPU0? If the
interrupt can go to any CPU this should be cpu_possible_mask instead.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Emilio López]

Hi Maxime,

El 25/09/13 11:03, Maxime Ripard escribió:
> Most of the Allwinner SoCs (at this time, all but the A10) also have a
> High Speed timers that are not using the 24MHz oscillator as a source
> but rather the AHB clock running much faster.
>
> The IP is slightly different between the A10s/A13 and the one used in
> the A20/A31, since the latter have 4 timers available, while the former
> have only 2 of them.
>
> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> ---

[...]

> +static void __init sun5i_timer_init(struct device_node *node)
> +{
> +	unsigned long rate;
> +	struct clk *clk;
> +	int ret, irq;
> +	u32 val;
> +
> +	timer_base = of_iomap(node, 0);
> +	if (!timer_base)
> +		panic("Can't map registers");
> +
> +	irq = irq_of_parse_and_map(node, 0);
> +	if (irq <= 0)
> +		panic("Can't parse IRQ");
> +
> +	clk = of_clk_get(node, 0);
> +	if (IS_ERR(clk))
> +		panic("Can't get timer clock");

I'm not familiar with clocksources, but does this have to be as fatal as 
it is considering the kernel also supports the slower sun4i timer?

Also, would any special considerations be needed when adjusting the ahb 
clock? A future cpufreq driver will most likely need to.

Cheers,

Emilio

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 4062 bytes --]

Hi Stephen,

On Wed, Sep 25, 2013 at 04:16:14PM -0700, Stephen Boyd wrote:
> On 09/25/13 07:03, Maxime Ripard wrote:
> > diff --git a/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> > new file mode 100644
> > index 0000000..b1f81e9
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/allwinner,sun5i-a13-hstimer.txt
> > @@ -0,0 +1,22 @@
> > +Allwinner SoCs High Speed Timer Controller
> > +
> > +Required properties:
> > +
> > +- compatible :	should be "allwinner,sun5i-a13-hstimer" or
> > +		"allwinner,sun7i-a20-hstimer"
> > +- reg : Specifies base physical address and size of the registers.
> > +- interrupts :	The interrupts of these timers (2 for the sun5i IP, 4 for the sun7i
> > +		one)
> > +- clocks: phandle to the source clock (usually the AHB clock)
> > +
> > +Example:
> > +
> > +hstimer@01c60000 {
> 
> This should just be 'timer@1c60000'

Ok.

> > +	compatible = "allwinner,sun7i-a20-hstimer";
> > +        reg = <0x01c60000 0x1000>;
> > +        interrupts = <0 51 1>,
> > +                     <0 52 1>,
> > +                     <0 53 1>,
> > +                     <0 54 1>;
> > +        clocks = <&ahb1_gates 19>;
> > +};
> 
> Weird mix of tabs and spaces here.

Right.

> > +
> > +static void __iomem *timer_base;
> > +static u32 ticks_per_jiffy;
> > +
> > +/*
> > + * When we disable a timer, we need to wait at least for 2 cycles of
> > + * the timer source clock. We will use for that the clocksource timer
> > + * that is already setup and runs at the same frequency than the other
> > + * timers, and we never will be disabled.
> > + */
> > +static void sun5i_clkevt_sync(void)
> > +{
> > +	u32 old = readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> > +
> > +	while ((old - readl(timer_base + TIMER_CNTVAL_LO_REG(1))) < 3)
> > +		cpu_relax();
> > +}
> > +
> [...]
> > +
> > +static int sun5i_clkevt_next_event(unsigned long evt,
> > +				   struct clock_event_device *unused)
> > +{
> > +	sun5i_clkevt_time_stop(0);
> > +	sun5i_clkevt_time_setup(0, evt);
> > +	sun5i_clkevt_time_start(0, false);
> 
> I suppose the min delta wants to be 3 instead of 1 because if we program
> an evt one tick in the future first we'll wait for two ticks (or is that
> three?) while we stop the timer and then program the timer to fire one
> tick after that. Perhaps we should subtract two ticks from the evt as
> well when we program it here?

Hmmm, indeed.

> > +
> > +	return 0;
> > +}
> > +
> > +static struct clock_event_device sun5i_clockevent = {
> > +	.name = "sun5i_tick",
> > +	.rating = 350,
> > +	.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT,
> > +	.set_mode = sun5i_clkevt_mode,
> > +	.set_next_event = sun5i_clkevt_next_event,
> > +};
> > +
> > +
> > +static irqreturn_t sun5i_timer_interrupt(int irq, void *dev_id)
> > +{
> > +	struct clock_event_device *evt = (struct clock_event_device *)dev_id;
> > +
> > +	writel(0x1, timer_base + TIMER_IRQ_ST_REG);
> > +	evt->event_handler(evt);
> > +
> > +	return IRQ_HANDLED;
> > +}
> > +
> > +static struct irqaction sun5i_timer_irq = {
> > +	.name = "sun5i_timer0",
> > +	.flags = IRQF_DISABLED | IRQF_TIMER | IRQF_IRQPOLL,
> 
> IRQF_DISABLED is a no-op and can be dropped.

Right.

> > +
> > +static u32 sun5i_timer_sched_read(void)
> > +{
> > +	return ~readl(timer_base + TIMER_CNTVAL_LO_REG(1));
> > +}
> > +
> > +static void __init sun5i_timer_init(struct device_node *node)
> > +{
> [...]
> > +
> > +	sun5i_clockevent.cpumask = cpumask_of(0);
> 
> Can this timer interrupt any CPU or is it hardwired to CPU0? If the
> interrupt can go to any CPU this should be cpu_possible_mask instead.

This is an interrupt that can interrupt any CPU available on the system,
so yes, cpu_possible_mask would make sense.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]

On Wed, Sep 25, 2013 at 08:23:14PM -0300, Emilio López wrote:
> El 25/09/13 11:03, Maxime Ripard escribió:
> >Most of the Allwinner SoCs (at this time, all but the A10) also have a
> >High Speed timers that are not using the 24MHz oscillator as a source
> >but rather the AHB clock running much faster.
> >
> >The IP is slightly different between the A10s/A13 and the one used in
> >the A20/A31, since the latter have 4 timers available, while the former
> >have only 2 of them.
> >
> >Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
> >---
> 
> [...]
> 
> >+static void __init sun5i_timer_init(struct device_node *node)
> >+{
> >+	unsigned long rate;
> >+	struct clk *clk;
> >+	int ret, irq;
> >+	u32 val;
> >+
> >+	timer_base = of_iomap(node, 0);
> >+	if (!timer_base)
> >+		panic("Can't map registers");
> >+
> >+	irq = irq_of_parse_and_map(node, 0);
> >+	if (irq <= 0)
> >+		panic("Can't parse IRQ");
> >+
> >+	clk = of_clk_get(node, 0);
> >+	if (IS_ERR(clk))
> >+		panic("Can't get timer clock");
> 
> I'm not familiar with clocksources, but does this have to be as
> fatal as it is considering the kernel also supports the slower sun4i
> timer?

Hmmm, I don't know, one might choose to enable only this timer, in that
case that would make sense to panic, since it would be the only timer in
that case.

> Also, would any special considerations be needed when adjusting the
> ahb clock? A future cpufreq driver will most likely need to.

While this will be needed at some point, I don't really see how to
handle that properly. The clock framework doesn't seem to have any
callback when it comes to reconfiguring a clock that a device might
use.

This will also creates trouble for IPs such as the I2C that have to
setup internal dividers, and use clk_get_rate to do so.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Thomas Petazzoni]

Dear Maxime Ripard,

On Wed, 25 Sep 2013 22:50:12 +0300, Maxime Ripard wrote:

> For all we know, so far, allwinner has released, by family:
>   - sun3i (ARM926)
>     * F20 (not supported)
>   - sun4i (Cortex A8)
>     * A10
>   - sun5i (Cortex A8)
>     * A10s
>     * A13
>   - sun6i (4 * Cortex A7)
>     * A31
>     * A31s (not supported)
>   - sun7i (2 * Cortex A7)
>     * A20
>     * A23? (that was just announced)

A small update to Documentation/arm/sunxi/README would be nice :)

Thomas
-- 
Thomas Petazzoni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 873 bytes --]

Hi Thomas,

On Thu, Sep 26, 2013 at 04:39:42PM +0200, Thomas Petazzoni wrote:
> On Wed, 25 Sep 2013 22:50:12 +0300, Maxime Ripard wrote:
> 
> > For all we know, so far, allwinner has released, by family:
> >   - sun3i (ARM926)
> >     * F20 (not supported)
> >   - sun4i (Cortex A8)
> >     * A10
> >   - sun5i (Cortex A8)
> >     * A10s
> >     * A13
> >   - sun6i (4 * Cortex A7)
> >     * A31
> >     * A31s (not supported)
> >   - sun7i (2 * Cortex A7)
> >     * A20
> >     * A23? (that was just announced)
> 
> A small update to Documentation/arm/sunxi/README would be nice :)

Yep, right. Even though, for most of these SoCs (F20, A10s, A31*, A2*),
I don't think we have publicly available datasheet to point to there.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 0/5] Allwinner SoCs High Speed Timer support

[Thomas Petazzoni]

Dear Maxime Ripard,

On Fri, 27 Sep 2013 20:05:05 +0300, Maxime Ripard wrote:

> > A small update to Documentation/arm/sunxi/README would be nice :)
> 
> Yep, right. Even though, for most of these SoCs (F20, A10s, A31*,
> A2*), I don't think we have publicly available datasheet to point to
> there.

The Marvell file mentions Armada 370 and XP even though no public
datasheets are available. I think you should mention those SOCs, and
indicate that no datasheet is publicly available for the moment.

Best regards,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Emilio López]

Hi Maxime,

El 26/09/13 10:13, Maxime Ripard escribió:
> On Wed, Sep 25, 2013 at 08:23:14PM -0300, Emilio López wrote:
>> El 25/09/13 11:03, Maxime Ripard escribió:
>>> Most of the Allwinner SoCs (at this time, all but the A10) also have a
>>> High Speed timers that are not using the 24MHz oscillator as a source
>>> but rather the AHB clock running much faster.
>>>
>>> The IP is slightly different between the A10s/A13 and the one used in
>>> the A20/A31, since the latter have 4 timers available, while the former
>>> have only 2 of them.
>>>
>>> Signed-off-by: Maxime Ripard <maxime.ripard@free-electrons.com>
>>> ---
>>
>> [...]
>>
>>> +static void __init sun5i_timer_init(struct device_node *node)
>>> +{
>>> +	unsigned long rate;
>>> +	struct clk *clk;
>>> +	int ret, irq;
>>> +	u32 val;
>>> +
>>> +	timer_base = of_iomap(node, 0);
>>> +	if (!timer_base)
>>> +		panic("Can't map registers");
>>> +
>>> +	irq = irq_of_parse_and_map(node, 0);
>>> +	if (irq <= 0)
>>> +		panic("Can't parse IRQ");
>>> +
>>> +	clk = of_clk_get(node, 0);
>>> +	if (IS_ERR(clk))
>>> +		panic("Can't get timer clock");
>>
>> I'm not familiar with clocksources, but does this have to be as
>> fatal as it is considering the kernel also supports the slower sun4i
>> timer?
>
> Hmmm, I don't know, one might choose to enable only this timer, in that
> case that would make sense to panic, since it would be the only timer in
> that case.

Fair enough.

>> Also, would any special considerations be needed when adjusting the
>> ahb clock? A future cpufreq driver will most likely need to.
>
> While this will be needed at some point, I don't really see how to
> handle that properly. The clock framework doesn't seem to have any
> callback when it comes to reconfiguring a clock that a device might
> use.

Maybe we should consider using one of the other timers; from a quick 
look at the A20 user manual, it seems they can run at ~200MHz ("PLL6/6" 
as input)

> This will also creates trouble for IPs such as the I2C that have to
> setup internal dividers, and use clk_get_rate to do so.

Not really, because they use APB1 which is not scaled. The manual makes 
it explicit when describing APB1:

"This clock is used for some special module apbclk(twi,uart,
ps2, can, scr). Because these modules need special clock rate
even if the apbclk changed."

Cheers,

Emilio

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1451 bytes --]

On Sun, Sep 29, 2013 at 01:34:08AM -0300, Emilio López wrote:
> >>Also, would any special considerations be needed when adjusting the
> >>ahb clock? A future cpufreq driver will most likely need to.
> >
> >While this will be needed at some point, I don't really see how to
> >handle that properly. The clock framework doesn't seem to have any
> >callback when it comes to reconfiguring a clock that a device might
> >use.
> 
> Maybe we should consider using one of the other timers; from a quick
> look at the A20 user manual, it seems they can run at ~200MHz
> ("PLL6/6" as input)

Which timers are you talking about? I only find the usual timers we
already have support for and the timers supported in this user manual.

> >This will also creates trouble for IPs such as the I2C that have to
> >setup internal dividers, and use clk_get_rate to do so.
> 
> Not really, because they use APB1 which is not scaled. The manual
> makes it explicit when describing APB1:
> 
> "This clock is used for some special module apbclk(twi,uart,
> ps2, can, scr). Because these modules need special clock rate
> even if the apbclk changed."

Hmmm, right. But still, we can have IPs that depend on this one that
requires to readjust their internal rate whenever AHB is recalculated.
These HS timers for example :)

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 926 bytes --]

Hi Stephen,

Just following back on this.

On Wed, Sep 25, 2013 at 04:16:14PM -0700, Stephen Boyd wrote:
> On 09/25/13 07:03, Maxime Ripard wrote:
> > +	sun5i_clockevent.cpumask = cpumask_of(0);
> 
> Can this timer interrupt any CPU or is it hardwired to CPU0? If the
> interrupt can go to any CPU this should be cpu_possible_mask instead.

I've changed the few other things you spotted, but this one making the
timer unusable.

I think what happens here is that we have the A31 I've tested these
patches on is a quad-core SoC. As such, the device tree has 4 CPUs
declared. However, we don't have any SMP support for it now. So we end
up having 4 cpus set as possible, and only one online (the boot cpu),
which isn't working.

Would using cpu_online_mask work in our case?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Stephen Boyd]

On 10/10/13 12:13, Maxime Ripard wrote:
> On Wed, Sep 25, 2013 at 04:16:14PM -0700, Stephen Boyd wrote:
>> On 09/25/13 07:03, Maxime Ripard wrote:
>>> +	sun5i_clockevent.cpumask = cpumask_of(0);
>> Can this timer interrupt any CPU or is it hardwired to CPU0? If the
>> interrupt can go to any CPU this should be cpu_possible_mask instead.
> I've changed the few other things you spotted, but this one making the
> timer unusable.
>
> I think what happens here is that we have the A31 I've tested these
> patches on is a quad-core SoC. As such, the device tree has 4 CPUs
> declared. However, we don't have any SMP support for it now. So we end
> up having 4 cpus set as possible, and only one online (the boot cpu),
> which isn't working.

Can you explain more why it isn't working? Is the timer being rejected
in favor of another timer?

>
> Would using cpu_online_mask work in our case?

It may work but it's probably hiding a problem with CONFIG_SMP=y

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH 2/5] clocksource: Add Allwinner SoCs HS timers driver

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

Hi Stephen,

On Thu, Oct 10, 2013 at 03:46:11PM -0700, Stephen Boyd wrote:
> On 10/10/13 12:13, Maxime Ripard wrote:
> > On Wed, Sep 25, 2013 at 04:16:14PM -0700, Stephen Boyd wrote:
> >> On 09/25/13 07:03, Maxime Ripard wrote:
> >>> +	sun5i_clockevent.cpumask = cpumask_of(0);
> >> Can this timer interrupt any CPU or is it hardwired to CPU0? If the
> >> interrupt can go to any CPU this should be cpu_possible_mask instead.
> > I've changed the few other things you spotted, but this one making the
> > timer unusable.
> >
> > I think what happens here is that we have the A31 I've tested these
> > patches on is a quad-core SoC. As such, the device tree has 4 CPUs
> > declared. However, we don't have any SMP support for it now. So we end
> > up having 4 cpus set as possible, and only one online (the boot cpu),
> > which isn't working.
> 
> Can you explain more why it isn't working? Is the timer being rejected
> in favor of another timer?

Hmm, right, I forgot it, sorry about that.

The timers actually seem to not be working at all. I get stuck at the
delay loop calibration.

I'm away from my hardware right now, so I couldn't debug it further, but
reverting to using cpumask_of(0) makes the kernel boot flawlessly.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]