[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[Christian Daudt]

This adds support for the Broadcom timer, used in the following SoCs:
BCM11130, BCM11140, BCM11351, BCM28145, BCM28155

Updates from V6:
- Split DT portion into a separate patch

Updates from V5:
- Rebase to latest arm-soc/for-next

Updates from V4:
- Switch code to use CLOCKSOURCE_OF_DECLARE

Updates from V3:
- Migrate to 3.9 timer framework updates

Updates from V2:
- prepend static fns + fields with kona_

Updates from V1:
- Rename bcm_timer.c to bcm_kona_timer.c
- Pull .h into bcm_kona_timer.c
- Make timers static
- Clean up comment block
- Switched to using clockevents_config_and_register
- Added an error to the get_timer loop if it repeats too much
- Added to Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
- Added missing readl to timer_disable_and_clear

Note: bcm,kona-timer was kept as the 'compatible' field to make it
specific enough for when there are multiple bcm timers (bcm,timer is
too generic).

Signed-off-by: Christian Daudt <csd@broadcom.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>

diff --git a/arch/arm/mach-bcm/Kconfig b/arch/arm/mach-bcm/Kconfig
index bf02471..f112895 100644
--- a/arch/arm/mach-bcm/Kconfig
+++ b/arch/arm/mach-bcm/Kconfig
@@ -6,6 +6,7 @@ config ARCH_BCM
 	select ARM_ERRATA_764369 if SMP
 	select ARM_GIC
 	select CPU_V7
+	select CLKSRC_OF
 	select GENERIC_CLOCKEVENTS
 	select GENERIC_TIME
 	select GPIO_BCM
diff --git a/arch/arm/mach-bcm/board_bcm.c b/arch/arm/mach-bcm/board_bcm.c
index f0f9aba..2595935 100644
--- a/arch/arm/mach-bcm/board_bcm.c
+++ b/arch/arm/mach-bcm/board_bcm.c
@@ -16,14 +16,11 @@
 #include <linux/device.h>
 #include <linux/platform_device.h>
 #include <linux/irqchip.h>
+#include <linux/clocksource.h>
 
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
 
-static void timer_init(void)
-{
-}
-
 
 static void __init board_init(void)
 {
@@ -35,7 +32,7 @@ static const char * const bcm11351_dt_compat[] = { "bcm,bcm11351", NULL, };
 
 DT_MACHINE_START(BCM11351_DT, "Broadcom Application Processor")
 	.init_irq = irqchip_init,
-	.init_time = timer_init,
+	.init_time = clocksource_of_init,
 	.init_machine = board_init,
 	.dt_compat = bcm11351_dt_compat,
 MACHINE_END
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 4d8283a..96e2531 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -19,6 +19,7 @@ obj-$(CONFIG_ARCH_BCM2835)	+= bcm2835_timer.o
 obj-$(CONFIG_SUNXI_TIMER)	+= sunxi_timer.o
 obj-$(CONFIG_ARCH_TEGRA)	+= tegra20_timer.o
 obj-$(CONFIG_VT8500_TIMER)	+= vt8500_timer.o
+obj-$(CONFIG_ARCH_BCM)		+= bcm_kona_timer.o
 
 obj-$(CONFIG_ARM_ARCH_TIMER)		+= arm_arch_timer.o
 obj-$(CONFIG_CLKSRC_METAG_GENERIC)	+= metag_generic.o
diff --git a/drivers/clocksource/bcm_kona_timer.c b/drivers/clocksource/bcm_kona_timer.c
new file mode 100644
index 0000000..350f493
--- /dev/null
+++ b/drivers/clocksource/bcm_kona_timer.c
@@ -0,0 +1,211 @@
+/*
+ * Copyright (C) 2012 Broadcom Corporation
+ *
+ * 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 version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; 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/irq.h>
+#include <linux/interrupt.h>
+#include <linux/jiffies.h>
+#include <linux/clockchips.h>
+#include <linux/types.h>
+
+#include <linux/io.h>
+#include <asm/mach/time.h>
+
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+
+
+#define KONA_GPTIMER_STCS_OFFSET			0x00000000
+#define KONA_GPTIMER_STCLO_OFFSET			0x00000004
+#define KONA_GPTIMER_STCHI_OFFSET			0x00000008
+#define KONA_GPTIMER_STCM0_OFFSET			0x0000000C
+
+#define KONA_GPTIMER_STCS_TIMER_MATCH_SHIFT		0
+#define KONA_GPTIMER_STCS_COMPARE_ENABLE_SHIFT		4
+
+struct kona_bcm_timers {
+	int tmr_irq;
+	void __iomem *tmr_regs;
+};
+
+static struct kona_bcm_timers timers;
+
+static u32 arch_timer_rate;
+
+/*
+ * We use the peripheral timers for system tick, the cpu global timer for
+ * profile tick
+ */
+static void kona_timer_disable_and_clear(void __iomem *base)
+{
+	uint32_t reg;
+
+	/*
+	 * clear and disable interrupts
+	 * We are using compare/match register 0 for our system interrupts
+	 */
+	reg = readl(base + KONA_GPTIMER_STCS_OFFSET);
+
+	/* Clear compare (0) interrupt */
+	reg |= 1 << KONA_GPTIMER_STCS_TIMER_MATCH_SHIFT;
+	/* disable compare */
+	reg &= ~(1 << KONA_GPTIMER_STCS_COMPARE_ENABLE_SHIFT);
+
+	writel(reg, base + KONA_GPTIMER_STCS_OFFSET);
+
+}
+
+static void
+kona_timer_get_counter(void *timer_base, uint32_t *msw, uint32_t *lsw)
+{
+	void __iomem *base = IOMEM(timer_base);
+	int loop_limit = 4;
+
+	/*
+	 * Read 64-bit free running counter
+	 * 1. Read hi-word
+	 * 2. Read low-word
+	 * 3. Read hi-word again
+	 * 4.1
+	 *      if new hi-word is not equal to previously read hi-word, then
+	 *      start from #1
+	 * 4.2
+	 *      if new hi-word is equal to previously read hi-word then stop.
+	 */
+
+	while (--loop_limit) {
+		*msw = readl(base + KONA_GPTIMER_STCHI_OFFSET);
+		*lsw = readl(base + KONA_GPTIMER_STCLO_OFFSET);
+		if (*msw == readl(base + KONA_GPTIMER_STCHI_OFFSET))
+			break;
+	}
+	if (!loop_limit) {
+		pr_err("bcm_kona_timer: getting counter failed.\n");
+		pr_err(" Timer will be impacted\n");
+	}
+
+	return;
+}
+
+static const struct of_device_id bcm_timer_ids[] __initconst = {
+	{.compatible = "bcm,kona-timer"},
+	{},
+};
+
+static void __init kona_timers_init(void)
+{
+	struct device_node *node;
+	u32 freq;
+
+	node = of_find_matching_node(NULL, bcm_timer_ids);
+
+	if (!node)
+		panic("No timer");
+
+	if (!of_property_read_u32(node, "clock-frequency", &freq))
+		arch_timer_rate = freq;
+	else
+		panic("clock-frequency not set in the .dts file");
+
+	/* Setup IRQ numbers */
+	timers.tmr_irq = irq_of_parse_and_map(node, 0);
+
+	/* Setup IO addresses */
+	timers.tmr_regs = of_iomap(node, 0);
+
+	kona_timer_disable_and_clear(timers.tmr_regs);
+}
+
+static int kona_timer_set_next_event(unsigned long clc,
+				  struct clock_event_device *unused)
+{
+	/*
+	 * timer (0) is disabled by the timer interrupt already
+	 * so, here we reload the next event value and re-enable
+	 * the timer.
+	 *
+	 * This way, we are potentially losing the time between
+	 * timer-interrupt->set_next_event. CPU local timers, when
+	 * they come in should get rid of skew.
+	 */
+
+	uint32_t lsw, msw;
+	uint32_t reg;
+
+	kona_timer_get_counter(timers.tmr_regs, &msw, &lsw);
+
+	/* Load the "next" event tick value */
+	writel(lsw + clc, timers.tmr_regs + KONA_GPTIMER_STCM0_OFFSET);
+
+	/* Enable compare */
+	reg = readl(timers.tmr_regs + KONA_GPTIMER_STCS_OFFSET);
+	reg |= (1 << KONA_GPTIMER_STCS_COMPARE_ENABLE_SHIFT);
+	writel(reg, timers.tmr_regs + KONA_GPTIMER_STCS_OFFSET);
+
+	return 0;
+}
+
+static void kona_timer_set_mode(enum clock_event_mode mode,
+			     struct clock_event_device *unused)
+{
+	switch (mode) {
+	case CLOCK_EVT_MODE_ONESHOT:
+		/* by default mode is one shot don't do any thing */
+		break;
+	case CLOCK_EVT_MODE_UNUSED:
+	case CLOCK_EVT_MODE_SHUTDOWN:
+	default:
+		kona_timer_disable_and_clear(timers.tmr_regs);
+	}
+}
+
+static struct clock_event_device kona_clockevent_timer = {
+	.name = "timer 1",
+	.features = CLOCK_EVT_FEAT_ONESHOT,
+	.set_next_event = kona_timer_set_next_event,
+	.set_mode = kona_timer_set_mode
+};
+
+static void __init kona_timer_clockevents_init(void)
+{
+	kona_clockevent_timer.cpumask = cpumask_of(0);
+	clockevents_config_and_register(&kona_clockevent_timer,
+		arch_timer_rate, 6, 0xffffffff);
+}
+
+static irqreturn_t kona_timer_interrupt(int irq, void *dev_id)
+{
+	struct clock_event_device *evt = &kona_clockevent_timer;
+
+	kona_timer_disable_and_clear(timers.tmr_regs);
+	evt->event_handler(evt);
+	return IRQ_HANDLED;
+}
+
+static struct irqaction kona_timer_irq = {
+	.name = "Kona Timer Tick",
+	.flags = IRQF_TIMER,
+	.handler = kona_timer_interrupt,
+};
+
+static void __init kona_timer_init(void)
+{
+	kona_timers_init();
+	kona_timer_clockevents_init();
+	setup_irq(timers.tmr_irq, &kona_timer_irq);
+	kona_timer_set_next_event((arch_timer_rate / HZ), NULL);
+}
+
+CLOCKSOURCE_OF_DECLARE(bcm_kona, "bcm,kona-timer",
+	kona_timer_init);
-- 
1.7.10.4

[PATCH V7 2/2] ARM: bcm281xx: Add timer driver (DT portion)

[Christian Daudt]

This adds support for the Broadcom timer, used in the following SoCs:
BCM11130, BCM11140, BCM11351, BCM28145, BCM28155

Updates from V6:
- Split DT portion into a separate patch

Updates from V5:
- Rebase to latest arm-soc/for-next

Updates from V4:
- Switch code to use CLOCKSOURCE_OF_DECLARE

Updates from V3:
- Migrate to 3.9 timer framework updates

Updates from V2:
- prepend static fns + fields with kona_

Updates from V1:
- Rename bcm_timer.c to bcm_kona_timer.c
- Pull .h into bcm_kona_timer.c
- Make timers static
- Clean up comment block
- Switched to using clockevents_config_and_register
- Added an error to the get_timer loop if it repeats too much
- Added to Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
- Added missing readl to timer_disable_and_clear

Note: bcm,kona-timer was kept as the 'compatible' field to make it
specific enough for when there are multiple bcm timers (bcm,timer is
too generic).

Signed-off-by: Christian Daudt <csd@broadcom.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: John Stultz <john.stultz@linaro.org>
Reviewed-by: Stephen Warren <swarren@nvidia.com>

diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt b/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
new file mode 100644
index 0000000..59fa6e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
@@ -0,0 +1,19 @@
+Broadcom Kona Family timer
+-----------------------------------------------------
+This timer is used in the following Broadcom SoCs:
+ BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
+
+Required properties:
+- compatible : "bcm,kona-timer"
+- reg : Register range for the timer
+- interrupts : interrupt for the timer
+- clock-frequency: frequency that the clock operates
+
+Example:
+	timer at 35006000 {
+		compatible = "bcm,kona-timer";
+		reg = <0x35006000 0x1000>;
+		interrupts = <0x0 7 0x4>;
+		clock-frequency = <32768>;
+	};
+
diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
index ad13588..8f71f40 100644
--- a/arch/arm/boot/dts/bcm11351.dtsi
+++ b/arch/arm/boot/dts/bcm11351.dtsi
@@ -47,4 +47,12 @@
 		    cache-unified;
 		    cache-level = <2>;
 	};
+
+	timer at 35006000 {
+		compatible = "bcm,kona-timer";
+		reg = <0x35006000 0x1000>;
+		interrupts = <0x0 7 0x4>;
+		clock-frequency = <32768>;
+	};
+
 };
-- 
1.7.10.4

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[John Stultz]

On 03/13/2013 02:27 PM, Christian Daudt wrote:
> This adds support for the Broadcom timer, used in the following SoCs:
> BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
[snip]
> Signed-off-by: Christian Daudt <csd@broadcom.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Hey Olof,
     So Christian mentioned you were hoping I'd pull these in through my 
tree, but I'd really rather these go via the arch-soc tree, since that 
is more likely where they will be tested and more intelligently reviewed.

I've mentioned before my distaste for the drivers/clocksource directory 
becoming a dumping ground for arch specific (for the most part arm) 
clocksource, and more recently clockevent, drivers. I initially wanted 
the drivers/clocksource directory to be only for cross-arch 
clocksources, and I fear we'll end up with something like the 
drivers/rtc directory where its not entirely clear what hardware you 
actually need for a given driver (might as well be rtc-<sha1>.c ;).

That said, I realize I've been overruled on this. :)

But I'd really rather the patch flow for arch specific clocksource and 
clockevent drivers go through the arch tree, since there's a better 
chance folks will be building and testing against other arch specific 
changes that might cause problems. There's just no way for me to be able 
to intelligently review these sorts of changes.

Now, if there are changes to the clocksource core code, then I 
definitely want to be in the path there. Additionally meta-drivers like 
mmio, I should probably be more involved with, so they can be more 
properly integrated into the clocksource core. Also for drivers that are 
actually arch shared (ie: things like hpet/acpi_pm where ia64 and x86 
share them).

But if its arch specific for hardware I don't have, I'd really prefer 
the arch maintainer be the upstream path.

Thomas: Am I being too obstinate here?  If not, should we drop "F:      
drivers/clocksource" from the MAINTAINERS entry?

Christian: Sorry for the confusion on all this!

thanks
-john

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[Thomas Gleixner]

On Wed, 13 Mar 2013, John Stultz wrote:
> On 03/13/2013 02:27 PM, Christian Daudt wrote:
> > This adds support for the Broadcom timer, used in the following SoCs:
> > BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
> [snip]
> > Signed-off-by: Christian Daudt <csd@broadcom.com>
> > Acked-by: Arnd Bergmann <arnd@arndb.de>
> > Acked-by: John Stultz <john.stultz@linaro.org>
> > Reviewed-by: Stephen Warren <swarren@nvidia.com>
> 
> Hey Olof,
>     So Christian mentioned you were hoping I'd pull these in through my tree,
> but I'd really rather these go via the arch-soc tree, since that is more
> likely where they will be tested and more intelligently reviewed.
> 
> I've mentioned before my distaste for the drivers/clocksource directory
> becoming a dumping ground for arch specific (for the most part arm)
> clocksource, and more recently clockevent, drivers. I initially wanted the

OTOH, if they are all in one place we have a better chance to analyze
them in bulk, find similarities and patterns for consolidation. And
it's simpler for the developer of a new SoC support to find the
matching driver for his IP block instead of copying stuff from one
mach directory to the next. We've been there :)

> But I'd really rather the patch flow for arch specific clocksource and
> clockevent drivers go through the arch tree, since there's a better chance
> folks will be building and testing against other arch specific changes that
> might cause problems. There's just no way for me to be able to intelligently
> review these sorts of changes.

I agree with the build and test part, but you can still review the
code in general - correctness of the particular hw details aside. I
just opened a random one there and found:

static cycle_t vt8500_timer_read(struct clocksource *cs)
{
        int loops = msecs_to_loops(10);
        writel(3, regbase + TIMER_CTRL_VAL);
        while ((readl((regbase + TIMER_AS_VAL)) & TIMER_COUNT_R_ACTIVE)
                                                && --loops)
                cpu_relax();
        return readl(regbase + TIMER_COUNT_VAL);

You don't need particular hw knowledge to see that this looks more
than fishy at least without comments describing the hardware designers
brainfart.
 
> Now, if there are changes to the clocksource core code, then I definitely want
> to be in the path there. Additionally meta-drivers like mmio, I should
> probably be more involved with, so they can be more properly integrated into
> the clocksource core. Also for drivers that are actually arch shared (ie:
> things like hpet/acpi_pm where ia64 and x86 share them).
> 
> But if its arch specific for hardware I don't have, I'd really prefer the arch
> maintainer be the upstream path.
> 
> Thomas: Am I being too obstinate here?  If not, should we drop "F:
> drivers/clocksource" from the MAINTAINERS entry?

Maybe we should move the ARM specific ones into
drivers/clocksource/arm/ ?
 
Thanks,

	tglx

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[Arnd Bergmann]

On Thursday 14 March 2013, Thomas Gleixner wrote:
> > But if its arch specific for hardware I don't have, I'd really prefer the arch
> > maintainer be the upstream path.
> > 
> > Thomas: Am I being too obstinate here?  If not, should we drop "F:
> > drivers/clocksource" from the MAINTAINERS entry?

The idea of moving drivers out of arch/* into drivers/* is definitely to
have someone who understands the subsystem act as the gatekeeper. There
should be very little architecture specific knowledge in those drivers,
and I think it's more important to ensure that they are following a
sensible understanding of how timekeeping is done than that they
are following specific architecture maintainer's preferences.

> Maybe we should move the ARM specific ones into
> drivers/clocksource/arm/ ?

About half the IP blocks we use on ARM are also used on at least
one ARM64/AVR32/MIPS/PowerPC/x86/SH/Hexagon/c6x/etc part. Grouping them
by which CPU architecture first starts using them or happens to be
more popular at the time does not seem too helpful here.

Maybe it's better to have a subdirectory for those clock sources
that are used on any SoC, or have subdirectories based on the
company that created that part, as we do for ethernet drivers.
I wouldn't bother with that until there are a couple of dozen
different clock source drivers.

	Arnd

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[Christian Daudt]

On Thu, Mar 14, 2013 at 9:03 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 14 March 2013, Thomas Gleixner wrote:
>> > But if its arch specific for hardware I don't have, I'd really prefer the arch
>> > maintainer be the upstream path.
>> >
>> > Thomas: Am I being too obstinate here?  If not, should we drop "F:
>> > drivers/clocksource" from the MAINTAINERS entry?
>
> The idea of moving drivers out of arch/* into drivers/* is definitely to
> have someone who understands the subsystem act as the gatekeeper. There
> should be very little architecture specific knowledge in those drivers,
> and I think it's more important to ensure that they are following a
> sensible understanding of how timekeeping is done than that they
> are following specific architecture maintainer's preferences.
>
>> Maybe we should move the ARM specific ones into
>> drivers/clocksource/arm/ ?
>
> About half the IP blocks we use on ARM are also used on at least
> one ARM64/AVR32/MIPS/PowerPC/x86/SH/Hexagon/c6x/etc part. Grouping them
> by which CPU architecture first starts using them or happens to be
> more popular at the time does not seem too helpful here.
>
> Maybe it's better to have a subdirectory for those clock sources
> that are used on any SoC, or have subdirectories based on the
> company that created that part, as we do for ethernet drivers.
> I wouldn't bother with that until there are a couple of dozen
> different clock source drivers.

So it seems the (weak...) consensus is that it should go through the
clocksource tree. John, can you please apply the patch ?

 Thanks,
   csd

[PATCH V7 2/2] ARM: bcm281xx: Add timer driver (DT portion)

[Christian Daudt]

On Wed, Mar 13, 2013 at 2:27 PM, Christian Daudt <csd@broadcom.com> wrote:
> This adds support for the Broadcom timer, used in the following SoCs:
> BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
>
> Updates from V6:
> - Split DT portion into a separate patch
>
> Updates from V5:
> - Rebase to latest arm-soc/for-next
>
> Updates from V4:
> - Switch code to use CLOCKSOURCE_OF_DECLARE
>
> Updates from V3:
> - Migrate to 3.9 timer framework updates
>
> Updates from V2:
> - prepend static fns + fields with kona_
>
> Updates from V1:
> - Rename bcm_timer.c to bcm_kona_timer.c
> - Pull .h into bcm_kona_timer.c
> - Make timers static
> - Clean up comment block
> - Switched to using clockevents_config_and_register
> - Added an error to the get_timer loop if it repeats too much
> - Added to Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
> - Added missing readl to timer_disable_and_clear
>
> Note: bcm,kona-timer was kept as the 'compatible' field to make it
> specific enough for when there are multiple bcm timers (bcm,timer is
> too generic).
>
> Signed-off-by: Christian Daudt <csd@broadcom.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>
>
> diff --git a/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt b/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
> new file mode 100644
> index 0000000..59fa6e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
> @@ -0,0 +1,19 @@
> +Broadcom Kona Family timer
> +-----------------------------------------------------
> +This timer is used in the following Broadcom SoCs:
> + BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
> +
> +Required properties:
> +- compatible : "bcm,kona-timer"
> +- reg : Register range for the timer
> +- interrupts : interrupt for the timer
> +- clock-frequency: frequency that the clock operates
> +
> +Example:
> +       timer at 35006000 {
> +               compatible = "bcm,kona-timer";
> +               reg = <0x35006000 0x1000>;
> +               interrupts = <0x0 7 0x4>;
> +               clock-frequency = <32768>;
> +       };
> +
> diff --git a/arch/arm/boot/dts/bcm11351.dtsi b/arch/arm/boot/dts/bcm11351.dtsi
> index ad13588..8f71f40 100644
> --- a/arch/arm/boot/dts/bcm11351.dtsi
> +++ b/arch/arm/boot/dts/bcm11351.dtsi
> @@ -47,4 +47,12 @@
>                     cache-unified;
>                     cache-level = <2>;
>         };
> +
> +       timer at 35006000 {
> +               compatible = "bcm,kona-timer";
> +               reg = <0x35006000 0x1000>;
> +               interrupts = <0x0 7 0x4>;
> +               clock-frequency = <32768>;
> +       };
> +
>  };
> --
> 1.7.10.4
>
>
Hi Grant,
 In discussion with Olof @ Connect, he suggested that it would be best
to send dt changes as a standalone patch to devicetree-discuss list to
get it applied on its own. Can you pls apply this patch ?

 Thanks,
   csd

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[John Stultz]

On 03/28/2013 09:03 AM, Christian Daudt wrote:
> On Thu, Mar 14, 2013 at 9:03 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Thursday 14 March 2013, Thomas Gleixner wrote:
>>>> But if its arch specific for hardware I don't have, I'd really prefer the arch
>>>> maintainer be the upstream path.
>>>>
>>>> Thomas: Am I being too obstinate here?  If not, should we drop "F:
>>>> drivers/clocksource" from the MAINTAINERS entry?
>> The idea of moving drivers out of arch/* into drivers/* is definitely to
>> have someone who understands the subsystem act as the gatekeeper. There
>> should be very little architecture specific knowledge in those drivers,
>> and I think it's more important to ensure that they are following a
>> sensible understanding of how timekeeping is done than that they
>> are following specific architecture maintainer's preferences.

Sorry for the slow reply here.

Arnd: So I think I better understand the crux of the debate:

I'm unfamiliar with the hardware specifics, so I'm pushing the 
gatekeeping to the SoC maintainers, where as the SoC maintainer (dealing 
with tons of different SoC's), you're also unfamiliar with the hardware 
specifics, and would like to push the gatekeeping of the subsystem 
specific functionality to those maintainers.

Given that for each SoC there's tons of subsystem specific functionality 
that you're having to deal with, I can sympathize with your need to 
offload some of the gatekeeping.


>>> Maybe we should move the ARM specific ones into
>>> drivers/clocksource/arm/ ?
>> About half the IP blocks we use on ARM are also used on at least
>> one ARM64/AVR32/MIPS/PowerPC/x86/SH/Hexagon/c6x/etc part. Grouping them
>> by which CPU architecture first starts using them or happens to be
>> more popular at the time does not seem too helpful here.
>>
>> Maybe it's better to have a subdirectory for those clock sources
>> that are used on any SoC, or have subdirectories based on the
>> company that created that part, as we do for ethernet drivers.
>> I wouldn't bother with that until there are a couple of dozen
>> different clock source drivers.

So having had a few days to think about this, I think what usually rubs 
me the wrong way when I get driver/clocksource submissions, is that for 
99% of it, they *aren't clocksource drivers*.  Most of the code is 
*clockevent* driver logic, and then maybe 1-5 lines of actual 
clocksource code.

Now, I know the reason for this is often the clocksource and clockevent 
drivers are backed by the same hardware, and since there's no clockevent 
directory, might as well have it all in a single file somewhere. But 
mixing the different subsystem drivers together causes some of the 
maintenance confusion here.

So instead of creating drivers/clocksource/arch/ directories, what I'd 
propose is we create a drivers/clockevent directory to handle the actual 
clockevent code. I think this would better delineate the lines of 
responsibility on the gatekeeper side (that being Thomas or maybe 
someone else who has an interest in the subtleties of how various 
hardware timers are be broken-by-design ;), and I'd be much happier 
taking clocksource code where I felt I had a reasonable chance of 
noticing bugs.

Thomas: Not that you need more to maintain, but does this seem 
semi-reasonable? Do we need to find someone else to help here?

That said, at the end of the day, if I take a bad drivers/clocksource 
patch, what breaks won't be the timekeeping core, it will be an SoC 
board.  So I'll have to really rely on the original clocksource driver 
authors to help vet incoming patches. This is where I think having the 
SoC tree as a central point for SoC patches has and advantage, as its 
less likely breakage will sneak upstream via a subsystem tree. But 
understanding the need for review help, I think I'm ok with taking on 
more clocksource specific review.


> So it seems the (weak...) consensus is that it should go through the
> clocksource tree. John, can you please apply the patch ?

Christian: So thanks again for the ping here. So I think I'd prefer 
Thomas to be the one to apply these sorts (ie: clockevent) of patches in 
the future, but this time around I'll do it because its been unfair to 
make you the monkey-in-the-middle as we've tossed the responsibility 
around (and my stuff goes through Thomas anyway).

thanks
-john

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[John Stultz]

On 03/13/2013 02:27 PM, Christian Daudt wrote:
> This adds support for the Broadcom timer, used in the following SoCs:
> BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
>
> Updates from V6:
> - Split DT portion into a separate patch
>
> Updates from V5:
> - Rebase to latest arm-soc/for-next
>
> Updates from V4:
> - Switch code to use CLOCKSOURCE_OF_DECLARE
>
> Updates from V3:
> - Migrate to 3.9 timer framework updates
>
> Updates from V2:
> - prepend static fns + fields with kona_
>
> Updates from V1:
> - Rename bcm_timer.c to bcm_kona_timer.c
> - Pull .h into bcm_kona_timer.c
> - Make timers static
> - Clean up comment block
> - Switched to using clockevents_config_and_register
> - Added an error to the get_timer loop if it repeats too much
> - Added to Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
> - Added missing readl to timer_disable_and_clear
>
> Note: bcm,kona-timer was kept as the 'compatible' field to make it
> specific enough for when there are multiple bcm timers (bcm,timer is
> too generic).
>
> Signed-off-by: Christian Daudt <csd@broadcom.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Applied to my fortglx/3.10/time branch.

thanks
-john

[PATCH V7 1/2] ARM: bcm281xx: Add timer driver (driver portion)

[Arnd Bergmann]

On Thursday 28 March 2013, John Stultz wrote:
> On 03/28/2013 09:03 AM, Christian Daudt wrote:

> >>> Maybe we should move the ARM specific ones into
> >>> drivers/clocksource/arm/ ?
> >> About half the IP blocks we use on ARM are also used on at least
> >> one ARM64/AVR32/MIPS/PowerPC/x86/SH/Hexagon/c6x/etc part. Grouping them
> >> by which CPU architecture first starts using them or happens to be
> >> more popular at the time does not seem too helpful here.
> >>
> >> Maybe it's better to have a subdirectory for those clock sources
> >> that are used on any SoC, or have subdirectories based on the
> >> company that created that part, as we do for ethernet drivers.
> >> I wouldn't bother with that until there are a couple of dozen
> >> different clock source drivers.
> 
> So having had a few days to think about this, I think what usually rubs 
> me the wrong way when I get driver/clocksource submissions, is that for 
> 99% of it, they *aren't clocksource drivers*.  Most of the code is 
> *clockevent* driver logic, and then maybe 1-5 lines of actual 
> clocksource code.
> 
> Now, I know the reason for this is often the clocksource and clockevent 
> drivers are backed by the same hardware, and since there's no clockevent 
> directory, might as well have it all in a single file somewhere. But 
> mixing the different subsystem drivers together causes some of the 
> maintenance confusion here.
> 
> So instead of creating drivers/clocksource/arch/ directories, what I'd 
> propose is we create a drivers/clockevent directory to handle the actual 
> clockevent code. I think this would better delineate the lines of 
> responsibility on the gatekeeper side (that being Thomas or maybe 
> someone else who has an interest in the subtleties of how various 
> hardware timers are be broken-by-design ;), and I'd be much happier 
> taking clocksource code where I felt I had a reasonable chance of 
> noticing bugs.

Yes, this sounds like a good idea.

> Thomas: Not that you need more to maintain, but does this seem 
> semi-reasonable? Do we need to find someone else to help here?
> 
> That said, at the end of the day, if I take a bad drivers/clocksource 
> patch, what breaks won't be the timekeeping core, it will be an SoC 
> board.  So I'll have to really rely on the original clocksource driver 
> authors to help vet incoming patches. This is where I think having the 
> SoC tree as a central point for SoC patches has and advantage, as its 
> less likely breakage will sneak upstream via a subsystem tree. But 
> understanding the need for review help, I think I'm ok with taking on 
> more clocksource specific review.

I think the important point for you to realize is that you don't need to
worry about breaking a specific SoC. The patches will come from someone
who is using that hardware in the end. What we need from clock{source,event}
subsystem maintainers is perspective of how things fit into the subsystem
and to provide review like:
- this driver is not using CLOCKSOURCE_OF_DECLARE when it should
- that driver only handles clockevent, don't put it into drivers/clocksource
- I don't like your coding style, it doesn't fit in with the other
  drivers in this subsystem.
- what you do can be implemented much simpler using the generic infrastructure
  introduced by that earlier driver.

	Arnd

[PATCH V7 2/2] ARM: bcm281xx: Add timer driver (DT portion)

[Olof Johansson]

On Wed, Mar 13, 2013 at 02:27:28PM -0700, Christian Daudt wrote:
> This adds support for the Broadcom timer, used in the following SoCs:
> BCM11130, BCM11140, BCM11351, BCM28145, BCM28155
> 
> Updates from V6:
> - Split DT portion into a separate patch
> 
> Updates from V5:
> - Rebase to latest arm-soc/for-next
> 
> Updates from V4:
> - Switch code to use CLOCKSOURCE_OF_DECLARE
> 
> Updates from V3:
> - Migrate to 3.9 timer framework updates
> 
> Updates from V2:
> - prepend static fns + fields with kona_
> 
> Updates from V1:
> - Rename bcm_timer.c to bcm_kona_timer.c
> - Pull .h into bcm_kona_timer.c
> - Make timers static
> - Clean up comment block
> - Switched to using clockevents_config_and_register
> - Added an error to the get_timer loop if it repeats too much
> - Added to Documentation/devicetree/bindings/arm/bcm/bcm,kona-timer.txt
> - Added missing readl to timer_disable_and_clear
> 
> Note: bcm,kona-timer was kept as the 'compatible' field to make it
> specific enough for when there are multiple bcm timers (bcm,timer is
> too generic).
> 
> Signed-off-by: Christian Daudt <csd@broadcom.com>
> Acked-by: Arnd Bergmann <arnd@arndb.de>
> Acked-by: John Stultz <john.stultz@linaro.org>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

Thanks, applied to next/soc.


-Olof

[PATCH V7 2/2] ARM: bcm281xx: Add timer driver (DT portion)

[Olof Johansson]

On Thu, Mar 28, 2013 at 09:07:41AM -0700, Christian Daudt wrote:
> Hi Grant,
>  In discussion with Olof @ Connect, he suggested that it would be best
> to send dt changes as a standalone patch to devicetree-discuss list to
> get it applied on its own. Can you pls apply this patch ?

Actually, I wanted them split but since these change the SoC dtsi it should
probably still go through arm-soc. I've applied it to our next/soc branch now.


Based on the thread, it looks like the main driver is picked up by John and
will go in that through his tree.

Thanks for your patience on dealing with this!



-Olof