[PATCH 0/2] clocksource: add rockchip rk3288 specific armv7-timer

[PATCH 0/2] clocksource: add rockchip rk3288 specific armv7-timer

[Heiko Stübner]

The architected timer on Rockchip's RK3288 SoCs needs a bit of special handling
to start.

This seems to be similar to a recent discussion about the Exynos arch-timer
that also seems to need similar special handling (and mct timer running).

And, the arch-timer on recent Mediatek SoCs also uses this pattern of a
supplying timer below the arch-timer.

Therefore follow a suggestion from Arnd Bergmann and implement a separate
clocksource for it that does the setup and then initializes the arch-timer.

This series uses "of: Provide function to request and map memory" from
Matthias Brugger.

Heiko Stuebner (2):
  clocksource: arm_arch_timer: remove static from arch_timer_init
  clocksource: add rockchip-specific armv7-timer setup

 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/arm_arch_timer.c               |  2 +-
 drivers/clocksource/arm_arch_timer.h               |  7 +++
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 5 files changed, 88 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/arm_arch_timer.h
 create mode 100644 drivers/clocksource/rockchip_timer.c

-- 
1.9.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] clocksource: arm_arch_timer: remove static from arch_timer_init

[Heiko Stübner]

There exist SoCs that need some special setup procedure for their architected-timer
to work. Therefore allow arch_timer_init to be called from such specialized
clocksources.

Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
---
 drivers/clocksource/arm_arch_timer.c | 2 +-
 drivers/clocksource/arm_arch_timer.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clocksource/arm_arch_timer.h

diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
index 5163ec1..55116d6 100644
--- a/drivers/clocksource/arm_arch_timer.c
+++ b/drivers/clocksource/arm_arch_timer.c
@@ -635,7 +635,7 @@ static void __init arch_timer_common_init(void)
 	arch_timer_arch_init();
 }
 
-static void __init arch_timer_init(struct device_node *np)
+void __init arch_timer_init(struct device_node *np)
 {
 	int i;
 
diff --git a/drivers/clocksource/arm_arch_timer.h b/drivers/clocksource/arm_arch_timer.h
new file mode 100644
index 0000000..af3974d
--- /dev/null
+++ b/drivers/clocksource/arm_arch_timer.h
@@ -0,0 +1,7 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+void __init arch_timer_init(struct device_node *np);
-- 
1.9.0


--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

[Heiko Stübner]

The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run.
Therefore the special rockchip,rk3288-armv7-timer does this setup and
then initializes the architected timer using the new locally exposed
arch_timer_init.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Heiko Stuebner <heiko@sntech.de>
---
 .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
 drivers/clocksource/Makefile                       |  1 +
 drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
 3 files changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
 create mode 100644 drivers/clocksource/rockchip_timer.c

diff --git a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
new file mode 100644
index 0000000..4c1950a
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
@@ -0,0 +1,22 @@
+Rockchip armv7-timer:
+---------------------
+
+The architected timer on rk3288 SoCs has special setup requirements, as
+the cpu-timer block needs to supply the architected timer.
+
+Required node properties:
+- compatible value : = "rockchip,rk3288-armv7-timer";
+- reg : physical base address and the size of the registers window
+        of the supplying timer block
+- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
+
+Example:
+
+architected-timer {
+	compatible = "rockchip,rk3288-armv7-timer";
+	reg = <0xff810020 0x20>;
+	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
+		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
+	clock-frequency = <24000000>;
+};
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 800b130..cbad225 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
 obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
 obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
 obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
+obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
 obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
 obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
 obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
new file mode 100644
index 0000000..46c2146
--- /dev/null
+++ b/drivers/clocksource/rockchip_timer.c
@@ -0,0 +1,57 @@
+/*
+ * Copyright (c) 2014 MundoReader S.L.
+ * Author: Heiko Stuebner <heiko@sntech.de>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/init.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/clockchips.h>
+#include "arm_arch_timer.h"
+
+#define TIMER_LOAD_COUNT0		0x00
+#define TIMER_LOAD_COUNT1		0x04
+#define TIMER_CURRENT_VALUE0		0x08
+#define TIMER_CURRENT_VALUE1		0x0c
+#define TIMER_CONTROL_REG		0x10
+#define TIMER_INT_STATUS		0x18
+
+#define TIMER_DISABLE			(0 << 0)
+#define TIMER_ENABLE			(1 << 0)
+#define TIMER_MODE_FREE_RUNNING		(0 << 1)
+#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
+#define TIMER_INT_MASK			(0 << 2)
+#define TIMER_INT_UNMASK		(1 << 2)
+
+static __init void rk3288_arch_timer_init(struct device_node *np)
+{
+	void __iomem *reg_base;
+
+	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
+	if (!reg_base) {
+		pr_warn("%s: Can't get resource\n", __func__);
+		return;
+	}
+
+	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
+
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
+	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
+
+	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
+	       reg_base + TIMER_CONTROL_REG);
+
+	arch_timer_init(np);
+}
+CLOCKSOURCE_OF_DECLARE(rk3288_arch_timer, "rockchip,rk3288-armv7-timer",
+		       rk3288_arch_timer_init);
-- 
1.9.0

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

[Mark Rutland]

On Fri, Jun 20, 2014 at 11:44:49AM +0100, Heiko Stübner wrote:
> The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run.
> Therefore the special rockchip,rk3288-armv7-timer does this setup and
> then initializes the architected timer using the new locally exposed
> arch_timer_init.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
>  drivers/clocksource/Makefile                       |  1 +
>  drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
>  3 files changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
>  create mode 100644 drivers/clocksource/rockchip_timer.c
> 
> diff --git a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> new file mode 100644
> index 0000000..4c1950a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> @@ -0,0 +1,22 @@
> +Rockchip armv7-timer:
> +---------------------
> +
> +The architected timer on rk3288 SoCs has special setup requirements, as
> +the cpu-timer block needs to supply the architected timer.
> +
> +Required node properties:
> +- compatible value : = "rockchip,rk3288-armv7-timer";
> +- reg : physical base address and the size of the registers window
> +        of the supplying timer block
> +- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
> +
> +Example:
> +
> +architected-timer {
> +	compatible = "rockchip,rk3288-armv7-timer";
> +	reg = <0xff810020 0x20>;
> +	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
> +		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
> +	clock-frequency = <24000000>;
> +};
> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> index 800b130..cbad225 100644
> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
>  obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
>  obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
>  obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
> +obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
>  obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
>  obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
>  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
> diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
> new file mode 100644
> index 0000000..46c2146
> --- /dev/null
> +++ b/drivers/clocksource/rockchip_timer.c
> @@ -0,0 +1,57 @@
> +/*
> + * Copyright (c) 2014 MundoReader S.L.
> + * Author: Heiko Stuebner <heiko@sntech.de>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/clockchips.h>
> +#include "arm_arch_timer.h"
> +
> +#define TIMER_LOAD_COUNT0		0x00
> +#define TIMER_LOAD_COUNT1		0x04
> +#define TIMER_CURRENT_VALUE0		0x08
> +#define TIMER_CURRENT_VALUE1		0x0c
> +#define TIMER_CONTROL_REG		0x10
> +#define TIMER_INT_STATUS		0x18
> +
> +#define TIMER_DISABLE			(0 << 0)
> +#define TIMER_ENABLE			(1 << 0)
> +#define TIMER_MODE_FREE_RUNNING		(0 << 1)
> +#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
> +#define TIMER_INT_MASK			(0 << 2)
> +#define TIMER_INT_UNMASK		(1 << 2)
> +
> +static __init void rk3288_arch_timer_init(struct device_node *np)
> +{
> +	void __iomem *reg_base;
> +
> +	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
> +	if (!reg_base) {
> +		pr_warn("%s: Can't get resource\n", __func__);
> +		return;
> +	}
> +
> +	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
> +
> +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
> +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
> +
> +	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
> +	       reg_base + TIMER_CONTROL_REG);

Given that this is unconditionally set up in this way, could this not be
done in the fimrware/bootloader?

That way you could also setup CNTFREQ correctly, and virtualisation can
just work out of the box.

This looks like a clock driver in disguise...

Mark.

Re: [PATCH 1/2] clocksource: arm_arch_timer: remove static from arch_timer_init

[Mark Rutland]

On Fri, Jun 20, 2014 at 11:44:11AM +0100, Heiko Stübner wrote:
> There exist SoCs that need some special setup procedure for their architected-timer
> to work. Therefore allow arch_timer_init to be called from such specialized
> clocksources.
> 
> Signed-off-by: Heiko Stuebner <heiko@sntech.de>
> ---
>  drivers/clocksource/arm_arch_timer.c | 2 +-
>  drivers/clocksource/arm_arch_timer.h | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/clocksource/arm_arch_timer.h
> 
> diff --git a/drivers/clocksource/arm_arch_timer.c b/drivers/clocksource/arm_arch_timer.c
> index 5163ec1..55116d6 100644
> --- a/drivers/clocksource/arm_arch_timer.c
> +++ b/drivers/clocksource/arm_arch_timer.c
> @@ -635,7 +635,7 @@ static void __init arch_timer_common_init(void)
>  	arch_timer_arch_init();
>  }
>  
> -static void __init arch_timer_init(struct device_node *np)
> +void __init arch_timer_init(struct device_node *np)
>  {
>  	int i;
>  
> diff --git a/drivers/clocksource/arm_arch_timer.h b/drivers/clocksource/arm_arch_timer.h
> new file mode 100644
> index 0000000..af3974d
> --- /dev/null
> +++ b/drivers/clocksource/arm_arch_timer.h
> @@ -0,0 +1,7 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +void __init arch_timer_init(struct device_node *np);

If we _really_ need this (which I hope we don't), we already have
include/clocksource/arm_arch_timer.h.

Mark.

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

[Heiko Stübner]

Am Freitag, 20. Juni 2014, 11:51:23 schrieb Mark Rutland:
> On Fri, Jun 20, 2014 at 11:44:49AM +0100, Heiko Stübner wrote:
> > The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run.
> > Therefore the special rockchip,rk3288-armv7-timer does this setup and
> > then initializes the architected timer using the new locally exposed
> > arch_timer_init.
> > 
> > Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > ---
> > 
> >  .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
> >  drivers/clocksource/Makefile                       |  1 +
> >  drivers/clocksource/rockchip_timer.c               | 57
> >  ++++++++++++++++++++++ 3 files changed, 80 insertions(+)
> >  create mode 100644
> >  Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt create
> >  mode 100644 drivers/clocksource/rockchip_timer.c
> > 
> > diff --git
> > a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> > b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file
> > mode 100644
> > index 0000000..4c1950a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> > @@ -0,0 +1,22 @@
> > +Rockchip armv7-timer:
> > +---------------------
> > +
> > +The architected timer on rk3288 SoCs has special setup requirements, as
> > +the cpu-timer block needs to supply the architected timer.
> > +
> > +Required node properties:
> > +- compatible value : = "rockchip,rk3288-armv7-timer";
> > +- reg : physical base address and the size of the registers window
> > +        of the supplying timer block
> > +- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
> > +
> > +Example:
> > +
> > +architected-timer {
> > +	compatible = "rockchip,rk3288-armv7-timer";
> > +	reg = <0xff810020 0x20>;
> > +	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> > IRQ_TYPE_LEVEL_HIGH)>,
> > +		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | 
IRQ_TYPE_LEVEL_HIGH)>,
> > +		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | 
IRQ_TYPE_LEVEL_HIGH)>;
> > +	clock-frequency = <24000000>;
> > +};
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index 800b130..cbad225 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
> > 
> >  obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
> >  obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
> >  obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
> > 
> > +obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
> > 
> >  obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
> >  obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
> >  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
> > 
> > diff --git a/drivers/clocksource/rockchip_timer.c
> > b/drivers/clocksource/rockchip_timer.c new file mode 100644
> > index 0000000..46c2146
> > --- /dev/null
> > +++ b/drivers/clocksource/rockchip_timer.c
> > @@ -0,0 +1,57 @@
> > +/*
> > + * Copyright (c) 2014 MundoReader S.L.
> > + * Author: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > + * GNU General Public License for more details.
> > + */
> > +
> > +#include <linux/init.h>
> > +#include <linux/of.h>
> > +#include <linux/of_address.h>
> > +#include <linux/clockchips.h>
> > +#include "arm_arch_timer.h"
> > +
> > +#define TIMER_LOAD_COUNT0		0x00
> > +#define TIMER_LOAD_COUNT1		0x04
> > +#define TIMER_CURRENT_VALUE0		0x08
> > +#define TIMER_CURRENT_VALUE1		0x0c
> > +#define TIMER_CONTROL_REG		0x10
> > +#define TIMER_INT_STATUS		0x18
> > +
> > +#define TIMER_DISABLE			(0 << 0)
> > +#define TIMER_ENABLE			(1 << 0)
> > +#define TIMER_MODE_FREE_RUNNING		(0 << 1)
> > +#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
> > +#define TIMER_INT_MASK			(0 << 2)
> > +#define TIMER_INT_UNMASK		(1 << 2)
> > +
> > +static __init void rk3288_arch_timer_init(struct device_node *np)
> > +{
> > +	void __iomem *reg_base;
> > +
> > +	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
> > +	if (!reg_base) {
> > +		pr_warn("%s: Can't get resource\n", __func__);
> > +		return;
> > +	}
> > +
> > +	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
> > +
> > +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
> > +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
> > +
> > +	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
> > +	       reg_base + TIMER_CONTROL_REG);
> 
> Given that this is unconditionally set up in this way, could this not be
> done in the fimrware/bootloader?
> 
> That way you could also setup CNTFREQ correctly, and virtualisation can
> just work out of the box.

It always comes back to this :-) . And as always the counter-argument would be 
that I cannot influence the current bootloader, there are already devices 
shipping in this constellation. And of course quite some more SoCs using this 
pattern.


> This looks like a clock driver in disguise...

Yeah, that was my other idea - doing this as a clock. So would you be ok, with 
the arch-timer getting the possibility to handle an optional clock?


Thanks
Heiko
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

[Mark Rutland]

On Fri, Jun 20, 2014 at 12:27:26PM +0100, Heiko Stübner wrote:
> Am Freitag, 20. Juni 2014, 11:51:23 schrieb Mark Rutland:
> > On Fri, Jun 20, 2014 at 11:44:49AM +0100, Heiko Stübner wrote:
> > > The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run.
> > > Therefore the special rockchip,rk3288-armv7-timer does this setup and
> > > then initializes the architected timer using the new locally exposed
> > > arch_timer_init.
> > > 
> > > Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
> > > Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > > ---
> > > 
> > >  .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
> > >  drivers/clocksource/Makefile                       |  1 +
> > >  drivers/clocksource/rockchip_timer.c               | 57
> > >  ++++++++++++++++++++++ 3 files changed, 80 insertions(+)
> > >  create mode 100644
> > >  Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt create
> > >  mode 100644 drivers/clocksource/rockchip_timer.c
> > > 
> > > diff --git
> > > a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> > > b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt new file
> > > mode 100644
> > > index 0000000..4c1950a
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
> > > @@ -0,0 +1,22 @@
> > > +Rockchip armv7-timer:
> > > +---------------------
> > > +
> > > +The architected timer on rk3288 SoCs has special setup requirements, as
> > > +the cpu-timer block needs to supply the architected timer.
> > > +
> > > +Required node properties:
> > > +- compatible value : = "rockchip,rk3288-armv7-timer";
> > > +- reg : physical base address and the size of the registers window
> > > +        of the supplying timer block
> > > +- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
> > > +
> > > +Example:
> > > +
> > > +architected-timer {
> > > +	compatible = "rockchip,rk3288-armv7-timer";
> > > +	reg = <0xff810020 0x20>;
> > > +	interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) |
> > > IRQ_TYPE_LEVEL_HIGH)>,
> > > +		     <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | 
> IRQ_TYPE_LEVEL_HIGH)>,
> > > +		     <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | 
> IRQ_TYPE_LEVEL_HIGH)>;
> > > +	clock-frequency = <24000000>;
> > > +};
> > > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > > index 800b130..cbad225 100644
> > > --- a/drivers/clocksource/Makefile
> > > +++ b/drivers/clocksource/Makefile
> > > @@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)	+= timer-marco.o
> > > 
> > >  obj-$(CONFIG_ARCH_MOXART)	+= moxart_timer.o
> > >  obj-$(CONFIG_ARCH_MXS)		+= mxs_timer.o
> > >  obj-$(CONFIG_ARCH_PRIMA2)	+= timer-prima2.o
> > > 
> > > +obj-$(CONFIG_ARCH_ROCKCHIP)	+= rockchip_timer.o
> > > 
> > >  obj-$(CONFIG_ARCH_U300)		+= timer-u300.o
> > >  obj-$(CONFIG_SUN4I_TIMER)	+= sun4i_timer.o
> > >  obj-$(CONFIG_SUN5I_HSTIMER)	+= timer-sun5i.o
> > > 
> > > diff --git a/drivers/clocksource/rockchip_timer.c
> > > b/drivers/clocksource/rockchip_timer.c new file mode 100644
> > > index 0000000..46c2146
> > > --- /dev/null
> > > +++ b/drivers/clocksource/rockchip_timer.c
> > > @@ -0,0 +1,57 @@
> > > +/*
> > > + * Copyright (c) 2014 MundoReader S.L.
> > > + * Author: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> > > + *
> > > + * This program is free software; you can redistribute it and/or modify
> > > + * it under the terms of the GNU General Public License as published by
> > > + * the Free Software Foundation; either version 2 of the License, or
> > > + * (at your option) any later version.
> > > + *
> > > + * This program is distributed in the hope that it will be useful,
> > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > > + * GNU General Public License for more details.
> > > + */
> > > +
> > > +#include <linux/init.h>
> > > +#include <linux/of.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/clockchips.h>
> > > +#include "arm_arch_timer.h"
> > > +
> > > +#define TIMER_LOAD_COUNT0		0x00
> > > +#define TIMER_LOAD_COUNT1		0x04
> > > +#define TIMER_CURRENT_VALUE0		0x08
> > > +#define TIMER_CURRENT_VALUE1		0x0c
> > > +#define TIMER_CONTROL_REG		0x10
> > > +#define TIMER_INT_STATUS		0x18
> > > +
> > > +#define TIMER_DISABLE			(0 << 0)
> > > +#define TIMER_ENABLE			(1 << 0)
> > > +#define TIMER_MODE_FREE_RUNNING		(0 << 1)
> > > +#define TIMER_MODE_USER_DEFINED_COUNT	(1 << 1)
> > > +#define TIMER_INT_MASK			(0 << 2)
> > > +#define TIMER_INT_UNMASK		(1 << 2)
> > > +
> > > +static __init void rk3288_arch_timer_init(struct device_node *np)
> > > +{
> > > +	void __iomem *reg_base;
> > > +
> > > +	reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
> > > +	if (!reg_base) {
> > > +		pr_warn("%s: Can't get resource\n", __func__);
> > > +		return;
> > > +	}
> > > +
> > > +	writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
> > > +
> > > +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
> > > +	writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
> > > +
> > > +	writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
> > > +	       reg_base + TIMER_CONTROL_REG);
> > 
> > Given that this is unconditionally set up in this way, could this not be
> > done in the fimrware/bootloader?
> > 
> > That way you could also setup CNTFREQ correctly, and virtualisation can
> > just work out of the box.
> 
> It always comes back to this :-) . And as always the counter-argument would be 
> that I cannot influence the current bootloader, there are already devices 
> shipping in this constellation. And of course quite some more SoCs using this 
> pattern.

Sure, but we aren't yet supporting the architected timer on those
systems. :)

This isn't so much a pattern as an unholy tapestry of braindead.

> > This looks like a clock driver in disguise...
> 
> Yeah, that was my other idea - doing this as a clock. So would you be ok, with 
> the arch-timer getting the possibility to handle an optional clock?

I would very much not like to do this, but it is better than a driver
whose sole purpose is to not be a clock driver.

Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] clocksource: add rockchip-specific armv7-timer setup

[Matthias Brugger]

2014-06-20 12:51 GMT+02:00 Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>:
> On Fri, Jun 20, 2014 at 11:44:49AM +0100, Heiko Stübner wrote:
>> The armv7-timer on Rockchip RK3288 SoCs needs an underlying timer to run.
>> Therefore the special rockchip,rk3288-armv7-timer does this setup and
>> then initializes the architected timer using the new locally exposed
>> arch_timer_init.
>>
>> Suggested-by: Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org>
>> Signed-off-by: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>> ---
>>  .../bindings/arm/rockchip/armv7-timer.txt          | 22 +++++++++
>>  drivers/clocksource/Makefile                       |  1 +
>>  drivers/clocksource/rockchip_timer.c               | 57 ++++++++++++++++++++++
>>  3 files changed, 80 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
>>  create mode 100644 drivers/clocksource/rockchip_timer.c
>>
>> diff --git a/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
>> new file mode 100644
>> index 0000000..4c1950a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/rockchip/armv7-timer.txt
>> @@ -0,0 +1,22 @@
>> +Rockchip armv7-timer:
>> +---------------------
>> +
>> +The architected timer on rk3288 SoCs has special setup requirements, as
>> +the cpu-timer block needs to supply the architected timer.
>> +
>> +Required node properties:
>> +- compatible value : = "rockchip,rk3288-armv7-timer";
>> +- reg : physical base address and the size of the registers window
>> +        of the supplying timer block
>> +- CP15 Timer node properties as described in bindings/arm/arch_timer.txt
>> +
>> +Example:
>> +
>> +architected-timer {
>> +     compatible = "rockchip,rk3288-armv7-timer";
>> +     reg = <0xff810020 0x20>;
>> +     interrupts = <GIC_PPI 13 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
>> +                  <GIC_PPI 14 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>,
>> +                  <GIC_PPI 11 (GIC_CPU_MASK_SIMPLE(4) | IRQ_TYPE_LEVEL_HIGH)>;
>> +     clock-frequency = <24000000>;
>> +};
>> diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
>> index 800b130..cbad225 100644
>> --- a/drivers/clocksource/Makefile
>> +++ b/drivers/clocksource/Makefile
>> @@ -20,6 +20,7 @@ obj-$(CONFIG_ARCH_MARCO)    += timer-marco.o
>>  obj-$(CONFIG_ARCH_MOXART)    += moxart_timer.o
>>  obj-$(CONFIG_ARCH_MXS)               += mxs_timer.o
>>  obj-$(CONFIG_ARCH_PRIMA2)    += timer-prima2.o
>> +obj-$(CONFIG_ARCH_ROCKCHIP)  += rockchip_timer.o
>>  obj-$(CONFIG_ARCH_U300)              += timer-u300.o
>>  obj-$(CONFIG_SUN4I_TIMER)    += sun4i_timer.o
>>  obj-$(CONFIG_SUN5I_HSTIMER)  += timer-sun5i.o
>> diff --git a/drivers/clocksource/rockchip_timer.c b/drivers/clocksource/rockchip_timer.c
>> new file mode 100644
>> index 0000000..46c2146
>> --- /dev/null
>> +++ b/drivers/clocksource/rockchip_timer.c
>> @@ -0,0 +1,57 @@
>> +/*
>> + * Copyright (c) 2014 MundoReader S.L.
>> + * Author: Heiko Stuebner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/init.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/clockchips.h>
>> +#include "arm_arch_timer.h"
>> +
>> +#define TIMER_LOAD_COUNT0            0x00
>> +#define TIMER_LOAD_COUNT1            0x04
>> +#define TIMER_CURRENT_VALUE0         0x08
>> +#define TIMER_CURRENT_VALUE1         0x0c
>> +#define TIMER_CONTROL_REG            0x10
>> +#define TIMER_INT_STATUS             0x18
>> +
>> +#define TIMER_DISABLE                        (0 << 0)
>> +#define TIMER_ENABLE                 (1 << 0)
>> +#define TIMER_MODE_FREE_RUNNING              (0 << 1)
>> +#define TIMER_MODE_USER_DEFINED_COUNT        (1 << 1)
>> +#define TIMER_INT_MASK                       (0 << 2)
>> +#define TIMER_INT_UNMASK             (1 << 2)
>> +
>> +static __init void rk3288_arch_timer_init(struct device_node *np)
>> +{
>> +     void __iomem *reg_base;
>> +
>> +     reg_base = of_io_request_and_map(np, 0, "rk3288-armv7-timer");
>> +     if (!reg_base) {
>> +             pr_warn("%s: Can't get resource\n", __func__);
>> +             return;
>> +     }
>> +
>> +     writel(TIMER_DISABLE, reg_base + TIMER_CONTROL_REG);
>> +
>> +     writel(0xffffffff, reg_base + TIMER_LOAD_COUNT0);
>> +     writel(0xffffffff, reg_base + TIMER_LOAD_COUNT1);
>> +
>> +     writel(TIMER_ENABLE | TIMER_MODE_FREE_RUNNING,
>> +            reg_base + TIMER_CONTROL_REG);
>
> Given that this is unconditionally set up in this way, could this not be
> done in the fimrware/bootloader?

IMHO this is a candidate for the init_time function of the machine
description file until we have a bootloader that enables the arm arch
timer properly.

>
> That way you could also setup CNTFREQ correctly, and virtualisation can
> just work out of the box.

Setting CNTFREQ as done in mach-shmobile [0] does not help here, right?

Regards,
Matthias

[0] http://lxr.free-electrons.com/source/arch/arm/mach-shmobile/setup-rcar-gen2.c#L77

>
> This looks like a clock driver in disguise...
>
> Mark.



-- 
motzblog.wordpress.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html