linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-11 17:21 [PATCH 0/8] A bit of new code and sparse cleanups along the way Anton Vorontsov
@ 2008-03-11 17:24 ` Anton Vorontsov
  2008-03-18 17:43   ` Scott Wood
  2008-04-08  9:01   ` Laurent Pinchart
  0 siblings, 2 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-03-11 17:24 UTC (permalink / raw)
  To: linuxppc-dev

GTM stands for General-purpose Timers Module and able to generate
timer{1,2,3,4} interrupts.

There are several limitations in this support:
1. Cascaded (32 bit) timers unimplemented (1-2, 3-4).
   This is straightforward to implement when needed, two timers should
   be marked as "requested" and configured as appropriate.
2. Super-cascaded (64 bit) timers unimplemented (1-2-3-4).
   This is also straightforward to implement when needed, all timers
   should be marked as "requested" and configured as appropriate.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 Documentation/powerpc/booting-without-of.txt |   27 +++-
 arch/powerpc/Kconfig                         |    5 +
 arch/powerpc/sysdev/Makefile                 |    1 +
 arch/powerpc/sysdev/fsl_gtm.c                |  263 ++++++++++++++++++++++++++
 arch/powerpc/sysdev/qe_lib/Kconfig           |    5 +
 arch/powerpc/sysdev/qe_lib/Makefile          |    1 +
 arch/powerpc/sysdev/qe_lib/gtm.c             |   47 +++++
 include/asm-powerpc/fsl_gtm.h                |  138 ++++++++++++++
 8 files changed, 486 insertions(+), 1 deletions(-)
 create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
 create mode 100644 arch/powerpc/sysdev/qe_lib/gtm.c
 create mode 100644 include/asm-powerpc/fsl_gtm.h

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index 8ae57f2..b506245 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -57,7 +57,8 @@ Table of Contents
       n) 4xx/Axon EMAC ethernet nodes
       o) Xilinx IP cores
       p) Freescale Synchronous Serial Interface
-	  q) USB EHCI controllers
+      q) USB EHCI controllers
+      r) Freescale General-purpose Timers Module
 
   VII - Specifying interrupt information for devices
     1) interrupts property
@@ -2811,6 +2812,30 @@ platforms are moved over to use the flattened-device-tree model.
 		   big-endian;
 	   };
 
+    r) Freescale General-purpose Timers Module
+
+    Required properties:
+      - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for QE
+                     GTMs).
+      - reg : should contain gtm registers location and length (0x40).
+      - interrupts : should contain four interrupts.
+      - interrupt-parent : interrupt source phandle.
+
+    Example:
+
+    gtm@500 {
+    	compatible = "fsl,gtm";
+    	reg = <0x500 0x40>;
+    	interrupts = <90 8 78 8 84 8 72 8>;
+    	interrupt-parent = <&ipic>;
+    };
+
+    gtm@440 {
+    	compatible = "fsl,qe-gtm", "fsl,gtm";
+    	reg = <0x440 0x40>;
+    	interrupts = <12 13 14 15>;
+    	interrupt-parent = <&qeic>;
+    };
 
    More devices will be defined as this spec matures.
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 9c68592..0b27cbd 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -496,6 +496,11 @@ config FSL_LBC
 	help
 	  Freescale Localbus support
 
+config FSL_GTM
+	bool
+	help
+	  Freescale General-purpose Timers support
+
 # Yes MCA RS/6000s exist but Linux-PPC does not currently support any
 config MCA
 	bool
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 62b6ef0..a7e8da4 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MMIO_NVRAM)	+= mmio_nvram.o
 obj-$(CONFIG_FSL_SOC)		+= fsl_soc.o
 obj-$(CONFIG_FSL_PCI)		+= fsl_pci.o
 obj-$(CONFIG_FSL_LBC)		+= fsl_lbc.o
+obj-$(CONFIG_FSL_GTM)		+= fsl_gtm.o
 obj-$(CONFIG_RAPIDIO)		+= fsl_rio.o
 obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
diff --git a/arch/powerpc/sysdev/fsl_gtm.c b/arch/powerpc/sysdev/fsl_gtm.c
new file mode 100644
index 0000000..975fe4e
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_gtm.c
@@ -0,0 +1,263 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *               Shlomi Gridish <gridish@freescale.com>
+ *               Jerry Huang <Chang-Ming.Huang@freescale.com>
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *               Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/spinlock.h>
+#include <asm/fsl_gtm.h>
+
+struct gtm_timer *gtm_get_timer(int width)
+{
+	struct device_node *np;
+	struct gtm *gtm = NULL;
+	int i;
+
+	if (width != 16)
+		return ERR_PTR(-ENOSYS);
+
+	for_each_compatible_node(np, NULL, "fsl,gtm") {
+		if (!np->data) {
+			WARN_ON(1);
+			continue;
+		}
+		gtm = np->data;
+
+		spin_lock_irq(&gtm->lock);
+
+		for (i = 0; i < ARRAY_SIZE(gtm->timers); i++) {
+			if (!gtm->timers[i].requested) {
+				gtm->timers[i].requested = true;
+				spin_unlock_irq(&gtm->lock);
+				of_node_put(np);
+				return &gtm->timers[i];
+			}
+		}
+
+		spin_unlock_irq(&gtm->lock);
+	}
+
+	if (gtm)
+		return ERR_PTR(-EBUSY);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(gtm_get_timer);
+
+void gtm_put_timer(struct gtm_timer *tmr)
+{
+	spin_lock_irq(&tmr->gtm->lock);
+
+	tmr->requested = false;
+
+	spin_unlock_irq(&tmr->gtm->lock);
+}
+EXPORT_SYMBOL(gtm_put_timer);
+
+int gtm_reset_ref_timer_16(struct gtm_timer *tmr, unsigned int hz, u16 ref,
+			   bool ffr)
+{
+	struct gtm *gtm = tmr->gtm;
+	int num = tmr - &gtm->timers[0];
+	unsigned long flags;
+	unsigned int prescaler;
+	u8 iclk = GTMDR_ICLK_ICLK;
+	u8 psr;
+	u8 sps;
+
+	prescaler = gtm->clock / hz;
+
+	/*
+	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
+	 * plus "slow go" mode (clk / 16). So, total prescale value is
+	 * 16 * (psr + 1) * (sps + 1).
+	 */
+	if (prescaler > 256 * 256 * 16)
+		return -EINVAL;
+
+	if (prescaler > 256 * 256) {
+		iclk = GTMDR_ICLK_SLGO;
+		prescaler /= 16;
+	}
+
+	if (prescaler > 256) {
+		psr = 256 - 1;
+		sps = prescaler / 256 - 1;
+	} else {
+		psr = prescaler - 1;
+		sps = 1 - 1;
+	}
+
+	spin_lock_irqsave(&gtm->lock, flags);
+
+	/*
+	 * Properly reset timers: stop, reset, set up prescalers, reference
+	 * value and clear event register.
+	 */
+	clrsetbits_8(tmr->gtcfr, ~(GTCFR_STP(num) | GTCFR_RST(num)),
+				 GTCFR_STP(num) | GTCFR_RST(num));
+
+	setbits8(tmr->gtcfr, GTCFR_STP(num));
+
+	out_be16(tmr->gtpsr, psr);
+	clrsetbits_be16(tmr->gtmdr, 0xFFFF, iclk | GTMDR_SPS(sps) |
+			GTMDR_ORI | (ffr ? GTMDR_FFR : 0));
+	out_be16(tmr->gtcnr, 0);
+	out_be16(tmr->gtrfr, ref);
+	out_be16(tmr->gtevr, 0xFFFF);
+
+	/* Let it be. */
+	clrbits8(tmr->gtcfr, GTCFR_STP(num));
+
+	spin_unlock_irqrestore(&gtm->lock, flags);
+
+	return 0;
+}
+EXPORT_SYMBOL(gtm_reset_ref_timer_16);
+
+void gtm_stop_timer_16(struct gtm_timer *tmr)
+{
+	struct gtm *gtm = tmr->gtm;
+	int num = tmr - &gtm->timers[0];
+	unsigned long flags;
+
+	spin_lock_irqsave(&gtm->lock, flags);
+
+	setbits8(tmr->gtcfr, GTCFR_STP(num));
+
+	spin_unlock_irqrestore(&gtm->lock, flags);
+}
+EXPORT_SYMBOL(gtm_stop_timer_16);
+
+static void __init gtm_set_shortcuts(struct gtm_timer *timers,
+				     struct gtm_timers_regs __iomem *regs)
+{
+	/*
+	 * Yeah, I don't like this either, but timers' registers a bit messed,
+	 * so we have to provide shortcuts to write timer independent code.
+	 * Alternative option is to create gt*() accessors, but that will be
+	 * even uglier and cryptic.
+	 */
+	timers[0].gtcfr = &regs->gtcfr1;
+	timers[0].gtmdr = &regs->gtmdr1;
+	timers[0].gtpsr = &regs->gtpsr1;
+	timers[0].gtcnr = &regs->gtcnr1;
+	timers[0].gtrfr = &regs->gtrfr1;
+	timers[0].gtevr = &regs->gtevr1;
+
+	timers[1].gtcfr = &regs->gtcfr1;
+	timers[1].gtmdr = &regs->gtmdr2;
+	timers[1].gtpsr = &regs->gtpsr2;
+	timers[1].gtcnr = &regs->gtcnr2;
+	timers[1].gtrfr = &regs->gtrfr2;
+	timers[1].gtevr = &regs->gtevr2;
+
+	timers[2].gtcfr = &regs->gtcfr2;
+	timers[2].gtmdr = &regs->gtmdr3;
+	timers[2].gtpsr = &regs->gtpsr3;
+	timers[2].gtcnr = &regs->gtcnr3;
+	timers[2].gtrfr = &regs->gtrfr3;
+	timers[2].gtevr = &regs->gtevr3;
+
+	timers[3].gtcfr = &regs->gtcfr2;
+	timers[3].gtmdr = &regs->gtmdr4;
+	timers[3].gtpsr = &regs->gtpsr4;
+	timers[3].gtcnr = &regs->gtcnr4;
+	timers[3].gtrfr = &regs->gtrfr4;
+	timers[3].gtevr = &regs->gtevr4;
+}
+
+static int __init gtm_get_clock(struct gtm *gtm, struct device_node *np)
+{
+	struct device_node *parent;
+	const u32 *clock;
+	int size;
+	int ret;
+
+	parent = of_get_parent(np);
+	if (!parent) {
+		pr_err("%s: no parent?\n", np->full_name);
+		return -EINVAL;
+	}
+
+	clock = of_get_property(parent, "clock-frequency", &size);
+	if (!clock || size != sizeof(*clock)) {
+		pr_err("%s: no clock-frequency for %s\n",
+		       np->full_name, parent->full_name);
+		ret = -EINVAL;
+		goto err;
+	}
+
+	ret = 0;
+	gtm->clock = *clock;
+err:
+	of_node_put(parent);
+	return ret;
+}
+
+static int __init gtm_init_gtm(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,gtm") {
+		int i;
+		struct gtm *gtm;
+
+		gtm = kzalloc(sizeof(*gtm), GFP_KERNEL);
+		if (!gtm) {
+			pr_err("%s: unable to allocate memory\n",
+				np->full_name);
+			continue;
+		}
+
+		spin_lock_init(&gtm->lock);
+
+		if (gtm_get_clock(gtm, np))
+			goto err;
+
+		for (i = 0; i < ARRAY_SIZE(gtm->timers); i++) {
+			int ret;
+			struct resource irq;
+
+			ret = of_irq_to_resource(np, i, &irq);
+			if (ret == NO_IRQ) {
+				pr_err("%s: not enough interrupts specified\n",
+				       np->full_name);
+				goto err;
+			}
+			gtm->timers[i].irq = irq.start;
+			gtm->timers[i].gtm = gtm;
+		}
+
+		gtm->regs = of_iomap(np, 0);
+		if (!gtm->regs) {
+			pr_err("%s: unable to iomap registers\n",
+			       np->full_name);
+			goto err;
+		}
+
+		gtm_set_shortcuts(gtm->timers, gtm->regs);
+
+		/* We don't want to lose the node and its ->data */
+		of_node_get(np);
+		np->data = gtm;
+
+		continue;
+err:
+		kfree(gtm);
+	}
+	return 0;
+}
+arch_initcall(gtm_init_gtm);
diff --git a/arch/powerpc/sysdev/qe_lib/Kconfig b/arch/powerpc/sysdev/qe_lib/Kconfig
index adc6621..c1f2849 100644
--- a/arch/powerpc/sysdev/qe_lib/Kconfig
+++ b/arch/powerpc/sysdev/qe_lib/Kconfig
@@ -20,3 +20,8 @@ config UCC
 	bool
 	default y if UCC_FAST || UCC_SLOW
 
+config QE_GTM
+	bool
+	default y if FSL_GTM
+	help
+	  QE General-purpose Timers Module support
diff --git a/arch/powerpc/sysdev/qe_lib/Makefile b/arch/powerpc/sysdev/qe_lib/Makefile
index 874fe1a..3297a52 100644
--- a/arch/powerpc/sysdev/qe_lib/Makefile
+++ b/arch/powerpc/sysdev/qe_lib/Makefile
@@ -6,3 +6,4 @@ obj-$(CONFIG_QUICC_ENGINE)+= qe.o qe_ic.o qe_io.o
 obj-$(CONFIG_UCC)	+= ucc.o
 obj-$(CONFIG_UCC_SLOW)	+= ucc_slow.o
 obj-$(CONFIG_UCC_FAST)	+= ucc_fast.o
+obj-$(CONFIG_QE_GTM)	+= gtm.o
diff --git a/arch/powerpc/sysdev/qe_lib/gtm.c b/arch/powerpc/sysdev/qe_lib/gtm.c
new file mode 100644
index 0000000..2ce9c25
--- /dev/null
+++ b/arch/powerpc/sysdev/qe_lib/gtm.c
@@ -0,0 +1,47 @@
+/*
+ * QE General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *               Shlomi Gridish <gridish@freescale.com>
+ *               Jerry Huang <Chang-Ming.Huang@freescale.com>
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *               Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <asm/qe.h>
+#include <asm/fsl_gtm.h>
+
+/*
+ * For now we just fixing up the clock -- it's brg-frequency for QE
+ * chips, generic code does not and should not know these details.
+ *
+ * Later we might want to set up BRGs, when QE will actually use
+ * them (there are TIMERCS bits in the CMXGCR register, but today
+ * these bits seem to be no-ops.
+ */
+static int __init qe_init_gtm(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,qe-gtm") {
+		struct gtm *gtm = np->data;
+
+		if (!gtm) {
+			/* fsl,qe-gtm without fsl,gtm compatible? */
+			WARN_ON(1);
+			continue;
+		}
+
+		gtm->clock = qe_get_brg_clk();
+	}
+
+	return 0;
+}
+arch_initcall(qe_init_gtm);
diff --git a/include/asm-powerpc/fsl_gtm.h b/include/asm-powerpc/fsl_gtm.h
new file mode 100644
index 0000000..bdb9d5a
--- /dev/null
+++ b/include/asm-powerpc/fsl_gtm.h
@@ -0,0 +1,138 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *               Shlomi Gridish <gridish@freescale.com>
+ *               Jerry Huang <Chang-Ming.Huang@freescale.com>
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *               Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#ifndef __ASM_FSL_GTM_H
+#define __ASM_FSL_GTM_H
+
+#include <linux/types.h>
+#include <linux/spinlock.h>
+
+#define GTCFR_STP(x)		((x) & 1 ? 1 << 5 : 1 << 1)
+#define GTCFR_RST(x)		((x) & 1 ? 1 << 4 : 1 << 0)
+
+#define GTMDR_ICLK_MASK		(3 << 1)
+#define GTMDR_ICLK_ICAS		(0 << 1)
+#define GTMDR_ICLK_ICLK		(1 << 1)
+#define GTMDR_ICLK_SLGO		(2 << 1)
+#define GTMDR_FFR		(1 << 3)
+#define GTMDR_ORI		(1 << 4)
+#define GTMDR_SPS(x)		((x) << 8)
+
+struct gtm_timers_regs {
+	u8	gtcfr1;		/* Timer 1, Timer 2 global config register */
+	u8	res0[0x3];
+	u8	gtcfr2;		/* Timer 3, timer 4 global config register */
+	u8	res1[0xB];
+	__be16	gtmdr1;		/* Timer 1 mode register */
+	__be16	gtmdr2;		/* Timer 2 mode register */
+	__be16	gtrfr1;		/* Timer 1 reference register */
+	__be16	gtrfr2;		/* Timer 2 reference register */
+	__be16	gtcpr1;		/* Timer 1 capture register */
+	__be16	gtcpr2;		/* Timer 2 capture register */
+	__be16	gtcnr1;		/* Timer 1 counter */
+	__be16	gtcnr2;		/* Timer 2 counter */
+	__be16	gtmdr3;		/* Timer 3 mode register */
+	__be16	gtmdr4;		/* Timer 4 mode register */
+	__be16	gtrfr3;		/* Timer 3 reference register */
+	__be16	gtrfr4;		/* Timer 4 reference register */
+	__be16	gtcpr3;		/* Timer 3 capture register */
+	__be16	gtcpr4;		/* Timer 4 capture register */
+	__be16	gtcnr3;		/* Timer 3 counter */
+	__be16	gtcnr4;		/* Timer 4 counter */
+	__be16	gtevr1;		/* Timer 1 event register */
+	__be16	gtevr2;		/* Timer 2 event register */
+	__be16	gtevr3;		/* Timer 3 event register */
+	__be16	gtevr4;		/* Timer 4 event register */
+	__be16	gtpsr1;		/* Timer 1 prescale register */
+	__be16	gtpsr2;		/* Timer 2 prescale register */
+	__be16	gtpsr3;		/* Timer 3 prescale register */
+	__be16	gtpsr4;		/* Timer 4 prescale register */
+	u8 res2[0x40];
+} __attribute__ ((packed));
+
+struct gtm_timer {
+	unsigned int irq;
+
+	struct gtm *gtm;
+	bool requested;
+	u8 __iomem *gtcfr;
+	__be16 __iomem *gtmdr;
+	__be16 __iomem *gtpsr;
+	__be16 __iomem *gtcnr;
+	__be16 __iomem *gtrfr;
+	__be16 __iomem *gtevr;
+};
+
+struct gtm {
+	unsigned int clock;
+	struct gtm_timers_regs __iomem *regs;
+	struct gtm_timer timers[4];
+	spinlock_t lock;
+};
+
+/**
+ * gtm_get_timer - request GTM timer for use with the rest of GTM API
+ * @width:	timer width (only 16 bits wide timers implemented so far)
+ *
+ * This function reserves GTM timer for later use. It returns gtm_timer
+ * structure to use with the rest of GTM API, you should use timer->irq
+ * to manage timer interrupt.
+ */
+extern struct gtm_timer *gtm_get_timer(int width);
+
+/**
+ * gtm_put_timer - release GTM timer
+ * @width:	timer width (only 16 bits wide timers implemented so far)
+ *
+ * This function releases GTM timer sp others might request it.
+ */
+extern void gtm_put_timer(struct gtm_timer *tmr);
+
+/**
+ * gtm_reset_ref_timer_16 - (re)set single (16 bits) timer in reference mode
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ * @hz:		timer rate in Hz
+ * @ref:	refernce value
+ * @ffr:	free run flag
+ *
+ * Thus function (re)sets GTM timer so it counts up to the reference value and
+ * fires the interrupt when the value is reached. If ffr flag is set, timer
+ * will also reset itself upon reference value, otherwise it continues to
+ * increment.
+ */
+extern int gtm_reset_ref_timer_16(struct gtm_timer *tmr, unsigned int hz,
+				  u16 ref, bool ffr);
+
+/**
+ * gtm_ack_ref_timer_16 - acknowledge timer event (free-run timers only)
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ *
+ * Thus function used to acknowledge timer interrupt event, use it inside the
+ * interrupt handler.
+ */
+static inline void gtm_ack_ref_timer_16(struct gtm_timer *tmr)
+{
+	out_be16(tmr->gtevr, 0xFFFF);
+}
+
+/**
+ * gtm_stop_timer_16 - stop single timer
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ *
+ * This function simply stops the GTM timer.
+ */
+extern void gtm_stop_timer_16(struct gtm_timer *tmr);
+
+#endif /* __ASM_FSL_GTM_H */
-- 
1.5.2.2

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-11 17:24 ` [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support Anton Vorontsov
@ 2008-03-18 17:43   ` Scott Wood
  2008-03-18 19:21     ` Anton Vorontsov
  2008-04-08  9:01   ` Laurent Pinchart
  1 sibling, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-03-18 17:43 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev

On Tue, Mar 11, 2008 at 08:24:29PM +0300, Anton Vorontsov wrote:
> +    Required properties:
> +      - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for QE
> +                     GTMs).
> +      - reg : should contain gtm registers location and length (0x40).
> +      - interrupts : should contain four interrupts.
> +      - interrupt-parent : interrupt source phandle.

interrupt-parent isn't required; it's perfectly valid to specify that in the
parent node instead.

> +    Example:
> +
> +    gtm@500 {
> +    	compatible = "fsl,gtm";
> +    	reg = <0x500 0x40>;
> +    	interrupts = <90 8 78 8 84 8 72 8>;
> +    	interrupt-parent = <&ipic>;
> +    };
> +
> +    gtm@440 {
> +    	compatible = "fsl,qe-gtm", "fsl,gtm";
> +    	reg = <0x440 0x40>;
> +    	interrupts = <12 13 14 15>;
> +    	interrupt-parent = <&qeic>;
> +    };

"timer" would be a better node name than "gtm".

> +static int __init gtm_init_gtm(void)

Name seems rather redundant... what's wrong with gtm_init()?

> +/*
> + * For now we just fixing up the clock -- it's brg-frequency for QE
> + * chips, generic code does not and should not know these details.
> + *
> + * Later we might want to set up BRGs, when QE will actually use
> + * them (there are TIMERCS bits in the CMXGCR register, but today
> + * these bits seem to be no-ops.
> + */
> +static int __init qe_init_gtm(void)
> +{
> +	struct device_node *np;
> +
> +	for_each_compatible_node(np, NULL, "fsl,qe-gtm") {
> +		struct gtm *gtm = np->data;
> +
> +		if (!gtm) {
> +			/* fsl,qe-gtm without fsl,gtm compatible? */
> +			WARN_ON(1);
> +			continue;
> +		}
> +
> +		gtm->clock = qe_get_brg_clk();
> +	}
> +
> +	return 0;
> +}
> +arch_initcall(qe_init_gtm);

If this happens before the gtm_init_gtm(), then np->data will not be set. 

If this happens after gtm_init_gtm(), then gtm_init_gtm() will fail in
gtm_get_clock(), if there's no clock-frequency -- and if there is, then why
do we need qe_init_gtm() at all?

> +/**
> + * gtm_get_timer - request GTM timer for use with the rest of GTM API
> + * @width:	timer width (only 16 bits wide timers implemented so far)
> + *
> + * This function reserves GTM timer for later use. It returns gtm_timer
> + * structure to use with the rest of GTM API, you should use timer->irq
> + * to manage timer interrupt.
> + */
> +extern struct gtm_timer *gtm_get_timer(int width);

To support using the GTM as a wakeup from deep sleep on 831x (which I've had
a patch pending for quite a while now), we'll need some way of reserving a
specific timer (only GTM1, timer 4 is supported).

> +/**
> + * gtm_put_timer - release GTM timer
> + * @width:	timer width (only 16 bits wide timers implemented so far)
> + *
> + * This function releases GTM timer sp others might request it.
> + */
> +extern void gtm_put_timer(struct gtm_timer *tmr);
> +
> +/**
> + * gtm_reset_ref_timer_16 - (re)set single (16 bits) timer in reference mode
> + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> + * @hz:		timer rate in Hz
> + * @ref:	refernce value

How about "period" or "expiry"?  And it'd be better to let the caller
request a time in some real unit (e.g. microseconds), and let the gtm driver
figure out how to divide that between prescaler and reference value,
especially in the absence of a way to ask for the allowable hz ranges.

> + * @ffr:	free run flag

Could we call it something more intuitive such as "freerun"?

> + * Thus function (re)sets GTM timer so it counts up to the reference value and
> + * fires the interrupt when the value is reached. If ffr flag is set, timer
> + * will also reset itself upon reference value, otherwise it continues to
> + * increment.
> + */
> +extern int gtm_reset_ref_timer_16(struct gtm_timer *tmr, unsigned int hz,
> +				  u16 ref, bool ffr);
> +
> +/**
> + * gtm_ack_ref_timer_16 - acknowledge timer event (free-run timers only)
> + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> + *
> + * Thus function used to acknowledge timer interrupt event, use it inside the
> + * interrupt handler.
> + */
> +static inline void gtm_ack_ref_timer_16(struct gtm_timer *tmr)

What does the "ref" mean in these names?

How about "gtm_arm_timer16" and "gtm_ack_timer16"?

> +{
> +	out_be16(tmr->gtevr, 0xFFFF);
> +}

You need to include <asm/io.h> for this.

Don't blindly clear all events, just the events that have been acted upon. 
Either take the events as an argument, or make the ack function specific to
REF, and only set that bit.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-18 17:43   ` Scott Wood
@ 2008-03-18 19:21     ` Anton Vorontsov
  2008-03-18 19:55       ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Anton Vorontsov @ 2008-03-18 19:21 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Tue, Mar 18, 2008 at 12:43:29PM -0500, Scott Wood wrote:
> On Tue, Mar 11, 2008 at 08:24:29PM +0300, Anton Vorontsov wrote:
> > +    Required properties:
> > +      - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for QE
> > +                     GTMs).
> > +      - reg : should contain gtm registers location and length (0x40).
> > +      - interrupts : should contain four interrupts.
> > +      - interrupt-parent : interrupt source phandle.
> 
> interrupt-parent isn't required; it's perfectly valid to specify that in the
> parent node instead.

Ok

> 
> > +    Example:
> > +
> > +    gtm@500 {
> > +    	compatible = "fsl,gtm";
> > +    	reg = <0x500 0x40>;
> > +    	interrupts = <90 8 78 8 84 8 72 8>;
> > +    	interrupt-parent = <&ipic>;
> > +    };
> > +
> > +    gtm@440 {
> > +    	compatible = "fsl,qe-gtm", "fsl,gtm";
> > +    	reg = <0x440 0x40>;
> > +    	interrupts = <12 13 14 15>;
> > +    	interrupt-parent = <&qeic>;
> > +    };
> 
> "timer" would be a better node name than "gtm".

Ok

> > +static int __init gtm_init_gtm(void)
> 
> Name seems rather redundant... what's wrong with gtm_init()?

Probably :%s/// effect. Will fix.

> > +/*
> > + * For now we just fixing up the clock -- it's brg-frequency for QE
> > + * chips, generic code does not and should not know these details.
> > + *
> > + * Later we might want to set up BRGs, when QE will actually use
> > + * them (there are TIMERCS bits in the CMXGCR register, but today
> > + * these bits seem to be no-ops.
> > + */
> > +static int __init qe_init_gtm(void)
> > +{
> > +	struct device_node *np;
> > +
> > +	for_each_compatible_node(np, NULL, "fsl,qe-gtm") {
> > +		struct gtm *gtm = np->data;
> > +
> > +		if (!gtm) {
> > +			/* fsl,qe-gtm without fsl,gtm compatible? */
> > +			WARN_ON(1);
> > +			continue;
> > +		}
> > +
> > +		gtm->clock = qe_get_brg_clk();
> > +	}
> > +
> > +	return 0;
> > +}
> > +arch_initcall(qe_init_gtm);
> 
> If this happens before the gtm_init_gtm(),

"If" isn't possible, order is guaranteed.

> then np->data will not be set. 

It's a bug in the device tree or in the Linux code then.

> If this happens after gtm_init_gtm(), then gtm_init_gtm() will fail in
> gtm_get_clock(), if there's no clock-frequency -- and if there is, then why
> do we need qe_init_gtm() at all?

Because for the QE clock-frequency != brg-frequency.

> > +/**
> > + * gtm_get_timer - request GTM timer for use with the rest of GTM API
> > + * @width:	timer width (only 16 bits wide timers implemented so far)
> > + *
> > + * This function reserves GTM timer for later use. It returns gtm_timer
> > + * structure to use with the rest of GTM API, you should use timer->irq
> > + * to manage timer interrupt.
> > + */
> > +extern struct gtm_timer *gtm_get_timer(int width);
> 
> To support using the GTM as a wakeup from deep sleep on 831x (which I've had
> a patch pending for quite a while now), we'll need some way of reserving a
> specific timer (only GTM1, timer 4 is supported).

You can add reserve function either in the PM driver (if any), or
you can do something in the device tree (wakeup-timer = <..>). I don't
see any problems if you want to implement it.

> > +/**
> > + * gtm_put_timer - release GTM timer
> > + * @width:	timer width (only 16 bits wide timers implemented so far)
> > + *
> > + * This function releases GTM timer sp others might request it.
> > + */
> > +extern void gtm_put_timer(struct gtm_timer *tmr);
> > +
> > +/**
> > + * gtm_reset_ref_timer_16 - (re)set single (16 bits) timer in reference mode
> > + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> > + * @hz:		timer rate in Hz
> > + * @ref:	refernce value
> 
> How about "period" or "expiry"?  And it'd be better to let the caller
> request a time in some real unit (e.g. microseconds), and let the gtm driver
> figure out how to divide that between prescaler and reference value,
> especially in the absence of a way to ask for the allowable hz ranges.

Will think about it.

> > + * @ffr:	free run flag
> 
> Could we call it something more intuitive such as "freerun"?

Easy.

> > + * Thus function (re)sets GTM timer so it counts up to the reference value and
> > + * fires the interrupt when the value is reached. If ffr flag is set, timer
> > + * will also reset itself upon reference value, otherwise it continues to
> > + * increment.
> > + */
> > +extern int gtm_reset_ref_timer_16(struct gtm_timer *tmr, unsigned int hz,
> > +				  u16 ref, bool ffr);
> > +
> > +/**
> > + * gtm_ack_ref_timer_16 - acknowledge timer event (free-run timers only)
> > + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> > + *
> > + * Thus function used to acknowledge timer interrupt event, use it inside the
> > + * interrupt handler.
> > + */
> > +static inline void gtm_ack_ref_timer_16(struct gtm_timer *tmr)
> 
> What does the "ref" mean in these names?
> 
> How about "gtm_arm_timer16" and "gtm_ack_timer16"?

Ok.

> 
> > +{
> > +	out_be16(tmr->gtevr, 0xFFFF);
> > +}
> 
> You need to include <asm/io.h> for this.

Ok.

> Don't blindly clear all events, just the events that have been acted upon. 
> Either take the events as an argument, or make the ack function specifi

Ok.


Thanks,

-- 
Anton Vorontsov
email: cboumailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-18 19:21     ` Anton Vorontsov
@ 2008-03-18 19:55       ` Scott Wood
  2008-03-18 20:27         ` Anton Vorontsov
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-03-18 19:55 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev

Anton Vorontsov wrote:
>>> +arch_initcall(qe_init_gtm);
>> If this happens before the gtm_init_gtm(),
> 
> "If" isn't possible, order is guaranteed.

You use arch_initcall for both, so you're relying on link order.  I 
think this at least merits a comment.

>> then np->data will not be set. 
> 
> It's a bug in the device tree or in the Linux code then.

Hmm?  It's set by gtm_init_gtm().  If this code runs before 
gtm_init_gtm(), what are you expecting to initialize np->data?

>> If this happens after gtm_init_gtm(), then gtm_init_gtm() will fail in
>> gtm_get_clock(), if there's no clock-frequency -- and if there is, then why
>> do we need qe_init_gtm() at all?
> 
> Because for the QE clock-frequency != brg-frequency.

Sorry, I missed that you were getting clock-frequency from the parent, 
rather than the gtm node.  If you do the latter, then you can just stick 
the relevant frequency in the gtm node and not worry about where it 
comes from.  This would be analogous to how UART clocks are specified.

Also, what if some arch_initcall runs between gtm_init_gtm and 
qe_init_gtm, that registers itself as a client of the gtm driver, and 
uses the wrong clock value?

>>> +extern struct gtm_timer *gtm_get_timer(int width);
>> To support using the GTM as a wakeup from deep sleep on 831x (which I've had
>> a patch pending for quite a while now), we'll need some way of reserving a
>> specific timer (only GTM1, timer 4 is supported).
> 
> You can add reserve function either in the PM driver (if any), or

What I meant was that there needs to be some way of telling this driver 
not to hand the reserved timer out to some other client.

> you can do something in the device tree (wakeup-timer = <..>). I don't
> see any problems if you want to implement it.

How about simply having optional arguments to gtm_get_timer() to specify 
the GTM device and timer number, which will fail if it's already in use? 
  Then, the PM driver simply needs to run early enough to grab the timer 
it needs.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-18 19:55       ` Scott Wood
@ 2008-03-18 20:27         ` Anton Vorontsov
  2008-03-18 20:48           ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Anton Vorontsov @ 2008-03-18 20:27 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Tue, Mar 18, 2008 at 02:55:14PM -0500, Scott Wood wrote:
> Anton Vorontsov wrote:
> >>>+arch_initcall(qe_init_gtm);
> >>If this happens before the gtm_init_gtm(),
> >
> >"If" isn't possible, order is guaranteed.
> 
> You use arch_initcall for both, so you're relying on link order.  I 
> think this at least merits a comment.
> >>then np->data will not be set. 
> >
> >It's a bug in the device tree or in the Linux code then.
> 
> Hmm?  It's set by gtm_init_gtm().  If this code runs before 
> gtm_init_gtm(), what are you expecting to initialize np->data?

What code exactly?

> >>If this happens after gtm_init_gtm(), then gtm_init_gtm() will fail in
> >>gtm_get_clock(), if there's no clock-frequency -- and if there is, then 
> >>why
> >>do we need qe_init_gtm() at all?
> >
> >Because for the QE clock-frequency != brg-frequency.
> 
> Sorry, I missed that you were getting clock-frequency from the parent, 
> rather than the gtm node.  If you do the latter, then you can just stick 
> the relevant frequency in the gtm node and not worry about where it 
> comes from.  This would be analogous to how UART clocks are specified.

Ok.

> Also, what if some arch_initcall runs between gtm_init_gtm and 
> qe_init_gtm, that registers itself as a client of the gtm driver, and 
> uses the wrong clock value?

Again, what code exactly? If it is a driver (for what this API is
created for), it hardly will run earlier than arch/ code. If this is
platform code (arch/powerpc/platform/), then it is hardly will run
earlier than arch/sysdev/. Inside the arch/sysdev/ fsl_gtm.c is
guaranteed to run earlier than qe_lib/gtm.c. So, where is the problem?

Since I'll implement clock-frequency inside the timer node, this
isn't relevant anymore...

> >>>+extern struct gtm_timer *gtm_get_timer(int width);
> >>To support using the GTM as a wakeup from deep sleep on 831x (which I've 
> >>had
> >>a patch pending for quite a while now), we'll need some way of reserving a
> >>specific timer (only GTM1, timer 4 is supported).
> >
> >You can add reserve function either in the PM driver (if any), or
> 
> What I meant was that there needs to be some way of telling this driver 
> not to hand the reserved timer out to some other client.
> 
> >you can do something in the device tree (wakeup-timer = <..>). I don't
> >see any problems if you want to implement it.
> 
> How about simply having optional arguments to gtm_get_timer() to specify 
> the GTM device and timer number, which will fail if it's already in use? 
>  Then, the PM driver simply needs to run early enough to grab the timer 
> it needs.

Ah. You need specific timer. No problem. I don't like idea of new arguments
to the gtm_get_timer() function (complicates things), but we can just
implement another one. gtm_get_timer_<name>, choice the name please.
_specific, _2, _for, __gtm_get_timer, ...

-- 
Anton Vorontsov
email: cboumailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-18 20:27         ` Anton Vorontsov
@ 2008-03-18 20:48           ` Scott Wood
  2008-04-16 18:39             ` Anton Vorontsov
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-03-18 20:48 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev

Anton Vorontsov wrote:
> On Tue, Mar 18, 2008 at 02:55:14PM -0500, Scott Wood wrote:
>> Anton Vorontsov wrote:
>>>>> +arch_initcall(qe_init_gtm);
>>>> If this happens before the gtm_init_gtm(),
>>> "If" isn't possible, order is guaranteed.
>> You use arch_initcall for both, so you're relying on link order.  I 
>> think this at least merits a comment.
>>>> then np->data will not be set. 
>>> It's a bug in the device tree or in the Linux code then.
>> Hmm?  It's set by gtm_init_gtm().  If this code runs before 
>> gtm_init_gtm(), what are you expecting to initialize np->data?
> 
> What code exactly?

Sorry, "this code" == qe_init_gtm().  Obviously, if you assume that 
gtm_init_gtm() will always be linked earlier, then it's not an issue.

>> Also, what if some arch_initcall runs between gtm_init_gtm and 
>> qe_init_gtm, that registers itself as a client of the gtm driver, and 
>> uses the wrong clock value?
> 
> Again, what code exactly?   If it is a driver (for what this API is
> created for), it hardly will run earlier than arch/ code.   If this is
> platform code (arch/powerpc/platform/), then it is hardly will run
> earlier than arch/sysdev/. Inside the arch/sysdev/ fsl_gtm.c is
> guaranteed to run earlier than qe_lib/gtm.c. So, where is the problem?

That's a lot of implicit, undocumented dependency on link order... 
Things can be moved around, and driver-ish code can pop up in surprising 
places.  All I meant was that having the gtm driver present itself as 
ready when it isn't, in a way which isn't readily apparent if it 
happens, is worrysome.

> Since I'll implement clock-frequency inside the timer node, this
> isn't relevant anymore...

OK, good.

> Ah. You need specific timer. No problem. I don't like idea of new arguments
> to the gtm_get_timer() function (complicates things), but we can just
> implement another one. gtm_get_timer_<name>, choice the name please.
> _specific, _2, _for, __gtm_get_timer, ...

How about:

struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer,
                                          int width);

...with np->data used by the caller to figure out which gtm pointer to 
pass in.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-11 17:24 ` [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support Anton Vorontsov
  2008-03-18 17:43   ` Scott Wood
@ 2008-04-08  9:01   ` Laurent Pinchart
  2008-04-08 11:48     ` Anton Vorontsov
  1 sibling, 1 reply; 21+ messages in thread
From: Laurent Pinchart @ 2008-04-08  9:01 UTC (permalink / raw)
  To: linuxppc-dev

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

On Tuesday 11 March 2008 18:24, Anton Vorontsov wrote:
> GTM stands for General-purpose Timers Module and able to generate
> timer{1,2,3,4} interrupts.
> 
> There are several limitations in this support:
> 1. Cascaded (32 bit) timers unimplemented (1-2, 3-4).
>    This is straightforward to implement when needed, two timers should
>    be marked as "requested" and configured as appropriate.
> 2. Super-cascaded (64 bit) timers unimplemented (1-2-3-4).
>    This is also straightforward to implement when needed, all timers
>    should be marked as "requested" and configured as appropriate.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>

[snip]

> +void gtm_stop_timer_16(struct gtm_timer *tmr)
> +{
> +	struct gtm *gtm = tmr->gtm;
> +	int num = tmr - &gtm->timers[0];
> +	unsigned long flags;
> +
> +	spin_lock_irqsave(&gtm->lock, flags);
> +
> +	setbits8(tmr->gtcfr, GTCFR_STP(num));

Shouldn't we clear the timer events with

out_be16(tmr->gtevr, 0xFFFF);

here ? Otherwise the timer interrupt could still fire after the timer is 
stopped. This introduces a race condition in drivers that blindly re-arm the 
timer in the interrupt handler. I've been bitten by this while porting your 
FHCI USB driver to a CPM2 platform.

> +
> +	spin_unlock_irqrestore(&gtm->lock, flags);
> +}
> +EXPORT_SYMBOL(gtm_stop_timer_16);

-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-08  9:01   ` Laurent Pinchart
@ 2008-04-08 11:48     ` Anton Vorontsov
  0 siblings, 0 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-08 11:48 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: linuxppc-dev

On Tue, Apr 08, 2008 at 11:01:53AM +0200, Laurent Pinchart wrote:
> On Tuesday 11 March 2008 18:24, Anton Vorontsov wrote:
> > GTM stands for General-purpose Timers Module and able to generate
> > timer{1,2,3,4} interrupts.
> > 
> > There are several limitations in this support:
> > 1. Cascaded (32 bit) timers unimplemented (1-2, 3-4).
> >    This is straightforward to implement when needed, two timers should
> >    be marked as "requested" and configured as appropriate.
> > 2. Super-cascaded (64 bit) timers unimplemented (1-2-3-4).
> >    This is also straightforward to implement when needed, all timers
> >    should be marked as "requested" and configured as appropriate.
> > 
> > Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> 
> [snip]
> 
> > +void gtm_stop_timer_16(struct gtm_timer *tmr)
> > +{
> > +	struct gtm *gtm = tmr->gtm;
> > +	int num = tmr - &gtm->timers[0];
> > +	unsigned long flags;
> > +
> > +	spin_lock_irqsave(&gtm->lock, flags);
> > +
> > +	setbits8(tmr->gtcfr, GTCFR_STP(num));
> 
> Shouldn't we clear the timer events with
> 
> out_be16(tmr->gtevr, 0xFFFF);

Yeah.

> here ? Otherwise the timer interrupt could still fire after the timer is 
> stopped. This introduces a race condition in drivers that blindly re-arm the 
> timer in the interrupt handler. I've been bitten by this while porting your 
> FHCI USB driver to a CPM2 platform.

Thanks, will fix.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-03-18 20:48           ` Scott Wood
@ 2008-04-16 18:39             ` Anton Vorontsov
  2008-04-16 18:44               ` Scott Wood
  2008-04-17 14:23               ` Laurent Pinchart
  0 siblings, 2 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-16 18:39 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Tue, Mar 18, 2008 at 03:48:12PM -0500, Scott Wood wrote:
[...]
> How about:
>
> struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer,
>                                          int width);
>
> ...with np->data used by the caller to figure out which gtm pointer to  
> pass in.

Thanks for the comments, I've tried to address them all.

Updated patch below (not for applying, still waiting for further
comments, if any).

- - - -
From: Anton Vorontsov <avorontsov@ru.mvista.com>
Subject: [POWERPC] sysdev,qe_lib: implement FSL GTM support

GTM stands for General-purpose Timers Module and able to generate
timer{1,2,3,4} interrupts.

There are several limitations in this support:
1. Cascaded (32 bit) timers unimplemented (1-2, 3-4).
   This is straightforward to implement when needed, two timers should
   be marked as "requested" and configured as appropriate.
2. Super-cascaded (64 bit) timers unimplemented (1-2-3-4).
   This is also straightforward to implement when needed, all timers
   should be marked as "requested" and configured as appropriate.

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 Documentation/powerpc/booting-without-of.txt |   32 +++-
 arch/powerpc/Kconfig                         |    5 +
 arch/powerpc/sysdev/Makefile                 |    1 +
 arch/powerpc/sysdev/fsl_gtm.c                |  322 ++++++++++++++++++++++++++
 include/asm-powerpc/fsl_gtm.h                |  106 +++++++++
 5 files changed, 465 insertions(+), 1 deletions(-)
 create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
 create mode 100644 include/asm-powerpc/fsl_gtm.h

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index f19fe9f..ee14ecd 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -57,7 +57,8 @@ Table of Contents
       n) 4xx/Axon EMAC ethernet nodes
       o) Xilinx IP cores
       p) Freescale Synchronous Serial Interface
-	  q) USB EHCI controllers
+      q) USB EHCI controllers
+      r) Freescale General-purpose Timers Module
 
   VII - Specifying interrupt information for devices
     1) interrupts property
@@ -2795,6 +2796,35 @@ platforms are moved over to use the flattened-device-tree model.
 		   big-endian;
 	   };
 
+    r) Freescale General-purpose Timers Module
+
+    Required properties:
+      - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for QE
+                     GTMs).
+      - reg : should contain gtm registers location and length (0x40).
+      - interrupts : should contain four interrupts.
+      - interrupt-parent : interrupt source phandle.
+      - clock-frequency : specifies the frequency driving the timer.
+
+    Example:
+
+    timer@500 {
+    	compatible = "fsl,gtm";
+    	reg = <0x500 0x40>;
+    	interrupts = <90 8 78 8 84 8 72 8>;
+    	interrupt-parent = <&ipic>;
+    	/* filled by u-boot */
+    	clock-frequency = <0>;
+    };
+
+    timer@440 {
+    	compatible = "fsl,qe-gtm", "fsl,gtm";
+    	reg = <0x440 0x40>;
+    	interrupts = <12 13 14 15>;
+    	interrupt-parent = <&qeic>;
+    	/* filled by u-boot */
+    	clock-frequency = <0>;
+    };
 
    More devices will be defined as this spec matures.
 
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 00c5e79..dd30ea4 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -520,6 +520,11 @@ config FSL_LBC
 	help
 	  Freescale Localbus support
 
+config FSL_GTM
+	bool
+	help
+	  Freescale General-purpose Timers support
+
 # Yes MCA RS/6000s exist but Linux-PPC does not currently support any
 config MCA
 	bool
diff --git a/arch/powerpc/sysdev/Makefile b/arch/powerpc/sysdev/Makefile
index 42b44a1..3974412 100644
--- a/arch/powerpc/sysdev/Makefile
+++ b/arch/powerpc/sysdev/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_MMIO_NVRAM)	+= mmio_nvram.o
 obj-$(CONFIG_FSL_SOC)		+= fsl_soc.o
 obj-$(CONFIG_FSL_PCI)		+= fsl_pci.o
 obj-$(CONFIG_FSL_LBC)		+= fsl_lbc.o
+obj-$(CONFIG_FSL_GTM)		+= fsl_gtm.o
 obj-$(CONFIG_RAPIDIO)		+= fsl_rio.o
 obj-$(CONFIG_TSI108_BRIDGE)	+= tsi108_pci.o tsi108_dev.o
 obj-$(CONFIG_QUICC_ENGINE)	+= qe_lib/
diff --git a/arch/powerpc/sysdev/fsl_gtm.c b/arch/powerpc/sysdev/fsl_gtm.c
new file mode 100644
index 0000000..6d86983
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_gtm.c
@@ -0,0 +1,322 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *               Shlomi Gridish <gridish@freescale.com>
+ *               Jerry Huang <Chang-Ming.Huang@freescale.com>
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *               Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#include <linux/kernel.h>
+#include <linux/errno.h>
+#include <linux/io.h>
+#include <linux/of.h>
+#include <linux/spinlock.h>
+#include <asm/fsl_gtm.h>
+
+/**
+ * gtm_get_timer - request GTM timer to use it with the rest of GTM API
+ * @width:	timer width (only 16 bits wide timers implemented so far)
+ *
+ * This function reserves GTM timer for later use. It returns gtm_timer
+ * structure to use with the rest of GTM API, you should use timer->irq
+ * to manage timer interrupt.
+ */
+struct gtm_timer *gtm_get_timer(int width)
+{
+	struct device_node *np;
+	struct gtm *gtm = NULL;
+	int i;
+
+	if (width != 16)
+		return ERR_PTR(-ENOSYS);
+
+	for_each_compatible_node(np, NULL, "fsl,gtm") {
+		if (!np->data) {
+			WARN_ON(1);
+			continue;
+		}
+		gtm = np->data;
+
+		spin_lock_irq(&gtm->lock);
+
+		for (i = 0; i < ARRAY_SIZE(gtm->timers); i++) {
+			if (!gtm->timers[i].requested) {
+				gtm->timers[i].requested = true;
+				spin_unlock_irq(&gtm->lock);
+				of_node_put(np);
+				return &gtm->timers[i];
+			}
+		}
+
+		spin_unlock_irq(&gtm->lock);
+	}
+
+	if (gtm)
+		return ERR_PTR(-EBUSY);
+	return ERR_PTR(-ENODEV);
+}
+EXPORT_SYMBOL(gtm_get_timer);
+
+/**
+ * gtm_get_specific_timer - request specific GTM timer
+ * @gtm:	specific GTM, pass here GTM's device_node->data
+ * @timer:	specific timer number, Timer1 is 0.
+ * @width:	timer width (only 16 bits wide timers implemented so far)
+ *
+ * This function reserves GTM timer for later use. It returns gtm_timer
+ * structure to use with the rest of GTM API, you should use timer->irq
+ * to manage timer interrupt.
+ */
+struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer, int width)
+{
+	struct gtm_timer *ret = ERR_PTR(-EBUSY);
+
+	if (width != 16)
+		return ERR_PTR(-ENOSYS);
+
+	spin_lock_irq(&gtm->lock);
+
+	if (gtm->timers[timer].requested)
+		goto out;
+
+	ret = &gtm->timers[timer];
+	ret->requested = true;
+
+out:
+	spin_unlock_irq(&gtm->lock);
+	return ret;
+}
+EXPORT_SYMBOL(gtm_get_specific_timer);
+
+/**
+ * gtm_put_timer - release GTM timer
+ * @width:	timer width (only 16 bits wide timers implemented so far)
+ *
+ * This function releases GTM timer so others may request it.
+ */
+void gtm_put_timer(struct gtm_timer *tmr)
+{
+	spin_lock_irq(&tmr->gtm->lock);
+
+	tmr->requested = false;
+
+	spin_unlock_irq(&tmr->gtm->lock);
+}
+EXPORT_SYMBOL(gtm_put_timer);
+
+/*
+ * This is back-end for the exported functions, it's used to reset single
+ * timer in reference mode.
+ */
+static int gtm_reset_ref_timer16(struct gtm_timer *tmr, int frequency,
+				 int reference_value, bool free_run)
+{
+	struct gtm *gtm = tmr->gtm;
+	int num = tmr - &gtm->timers[0];
+	unsigned int prescaler;
+	u8 iclk = GTMDR_ICLK_ICLK;
+	u8 psr;
+	u8 sps;
+	unsigned long flags;
+
+	prescaler = gtm->clock / frequency;
+	/*
+	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
+	 * plus "slow go" mode (clk / 16). So, total prescale value is
+	 * 16 * (psr + 1) * (sps + 1).
+	 */
+	if (prescaler > 256 * 256 * 16)
+		return -EINVAL;
+
+	if (prescaler > 256 * 256) {
+		iclk = GTMDR_ICLK_SLGO;
+		prescaler /= 16;
+	}
+
+	if (prescaler > 256) {
+		psr = 256 - 1;
+		sps = prescaler / 256 - 1;
+	} else {
+		psr = prescaler - 1;
+		sps = 1 - 1;
+	}
+
+	spin_lock_irqsave(&gtm->lock, flags);
+
+	/*
+	 * Properly reset timers: stop, reset, set up prescalers, reference
+	 * value and clear event register.
+	 */
+	clrsetbits_8(tmr->gtcfr, ~(GTCFR_STP(num) | GTCFR_RST(num)),
+				 GTCFR_STP(num) | GTCFR_RST(num));
+
+	setbits8(tmr->gtcfr, GTCFR_STP(num));
+
+	out_be16(tmr->gtpsr, psr);
+	clrsetbits_be16(tmr->gtmdr, 0xFFFF, iclk | GTMDR_SPS(sps) |
+			GTMDR_ORI | (free_run ? GTMDR_FFR : 0));
+	out_be16(tmr->gtcnr, 0);
+	out_be16(tmr->gtrfr, reference_value);
+	out_be16(tmr->gtevr, 0xFFFF);
+
+	/* Let it be. */
+	clrbits8(tmr->gtcfr, GTCFR_STP(num));
+
+	spin_unlock_irqrestore(&gtm->lock, flags);
+
+	return 0;
+}
+
+/**
+ * gtm_reset_utimer16 - reset 16 bits timer
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ * @usec:	timer interval in microseconds
+ * @free_run:	free run flag
+ *
+ * This function (re)sets GTM timer so it counts up to the interval value and
+ * fires the interrupt when the value is reached. If free_run flag was set,
+ * timer will also reset itself upon reference value, otherwise it continues to
+ * increment.
+ */
+int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool free_run)
+{
+	/* quite obvious, frequency which is enough for µSec precision */
+	const int freq = 1000000;
+
+	/*
+	 * We can lower the frequency (and probably power consumption) by
+	 * dividing both frequency and usec by 2 until there is no remainder.
+	 * But we won't bother with this unless savings are measured, so just
+	 * run the timer as is.
+	 */
+
+	return gtm_reset_ref_timer16(tmr, freq, usec, free_run);
+}
+EXPORT_SYMBOL(gtm_reset_utimer16);
+
+/**
+ * gtm_stop_timer16 - stop single timer
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ *
+ * This function simply stops the GTM timer.
+ */
+void gtm_stop_timer16(struct gtm_timer *tmr)
+{
+	struct gtm *gtm = tmr->gtm;
+	int num = tmr - &gtm->timers[0];
+	unsigned long flags;
+
+	spin_lock_irqsave(&gtm->lock, flags);
+
+	setbits8(tmr->gtcfr, GTCFR_STP(num));
+	out_be16(tmr->gtevr, 0xFFFF);
+
+	spin_unlock_irqrestore(&gtm->lock, flags);
+}
+EXPORT_SYMBOL(gtm_stop_timer16);
+
+static void __init gtm_set_shortcuts(struct gtm_timer *timers,
+				     struct gtm_timers_regs __iomem *regs)
+{
+	/*
+	 * Yeah, I don't like this either, but timers' registers a bit messed,
+	 * so we have to provide shortcuts to write timer independent code.
+	 * Alternative option is to create gt*() accessors, but that will be
+	 * even uglier and cryptic.
+	 */
+	timers[0].gtcfr = &regs->gtcfr1;
+	timers[0].gtmdr = &regs->gtmdr1;
+	timers[0].gtpsr = &regs->gtpsr1;
+	timers[0].gtcnr = &regs->gtcnr1;
+	timers[0].gtrfr = &regs->gtrfr1;
+	timers[0].gtevr = &regs->gtevr1;
+
+	timers[1].gtcfr = &regs->gtcfr1;
+	timers[1].gtmdr = &regs->gtmdr2;
+	timers[1].gtpsr = &regs->gtpsr2;
+	timers[1].gtcnr = &regs->gtcnr2;
+	timers[1].gtrfr = &regs->gtrfr2;
+	timers[1].gtevr = &regs->gtevr2;
+
+	timers[2].gtcfr = &regs->gtcfr2;
+	timers[2].gtmdr = &regs->gtmdr3;
+	timers[2].gtpsr = &regs->gtpsr3;
+	timers[2].gtcnr = &regs->gtcnr3;
+	timers[2].gtrfr = &regs->gtrfr3;
+	timers[2].gtevr = &regs->gtevr3;
+
+	timers[3].gtcfr = &regs->gtcfr2;
+	timers[3].gtmdr = &regs->gtmdr4;
+	timers[3].gtpsr = &regs->gtpsr4;
+	timers[3].gtcnr = &regs->gtcnr4;
+	timers[3].gtrfr = &regs->gtrfr4;
+	timers[3].gtevr = &regs->gtevr4;
+}
+
+static int __init gtm_init(void)
+{
+	struct device_node *np;
+
+	for_each_compatible_node(np, NULL, "fsl,gtm") {
+		int i;
+		struct gtm *gtm;
+		const u32 *clock;
+		int size;
+
+		gtm = kzalloc(sizeof(*gtm), GFP_KERNEL);
+		if (!gtm) {
+			pr_err("%s: unable to allocate memory\n",
+				np->full_name);
+			continue;
+		}
+
+		spin_lock_init(&gtm->lock);
+
+		clock = of_get_property(np, "clock-frequency", &size);
+		if (!clock || size != sizeof(*clock)) {
+			pr_err("%s: no clock-frequency\n", np->full_name);
+			goto err;
+		}
+		gtm->clock = *clock;
+
+		for (i = 0; i < ARRAY_SIZE(gtm->timers); i++) {
+			int ret;
+			struct resource irq;
+
+			ret = of_irq_to_resource(np, i, &irq);
+			if (ret == NO_IRQ) {
+				pr_err("%s: not enough interrupts specified\n",
+				       np->full_name);
+				goto err;
+			}
+			gtm->timers[i].irq = irq.start;
+			gtm->timers[i].gtm = gtm;
+		}
+
+		gtm->regs = of_iomap(np, 0);
+		if (!gtm->regs) {
+			pr_err("%s: unable to iomap registers\n",
+			       np->full_name);
+			goto err;
+		}
+
+		gtm_set_shortcuts(gtm->timers, gtm->regs);
+
+		/* We don't want to lose the node and its ->data */
+		of_node_get(np);
+		np->data = gtm;
+
+		continue;
+err:
+		kfree(gtm);
+	}
+	return 0;
+}
+arch_initcall(gtm_init);
diff --git a/include/asm-powerpc/fsl_gtm.h b/include/asm-powerpc/fsl_gtm.h
new file mode 100644
index 0000000..46c02e7
--- /dev/null
+++ b/include/asm-powerpc/fsl_gtm.h
@@ -0,0 +1,106 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *               Shlomi Gridish <gridish@freescale.com>
+ *               Jerry Huang <Chang-Ming.Huang@freescale.com>
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *               Anton Vorontsov <avorontsov@ru.mvista.com>
+ *
+ * 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.
+ */
+
+#ifndef __ASM_FSL_GTM_H
+#define __ASM_FSL_GTM_H
+
+#include <linux/types.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+
+#define GTCFR_STP(x)		((x) & 1 ? 1 << 5 : 1 << 1)
+#define GTCFR_RST(x)		((x) & 1 ? 1 << 4 : 1 << 0)
+
+#define GTMDR_ICLK_MASK		(3 << 1)
+#define GTMDR_ICLK_ICAS		(0 << 1)
+#define GTMDR_ICLK_ICLK		(1 << 1)
+#define GTMDR_ICLK_SLGO		(2 << 1)
+#define GTMDR_FFR		(1 << 3)
+#define GTMDR_ORI		(1 << 4)
+#define GTMDR_SPS(x)		((x) << 8)
+
+struct gtm_timers_regs {
+	u8	gtcfr1;		/* Timer 1, Timer 2 global config register */
+	u8	res0[0x3];
+	u8	gtcfr2;		/* Timer 3, timer 4 global config register */
+	u8	res1[0xB];
+	__be16	gtmdr1;		/* Timer 1 mode register */
+	__be16	gtmdr2;		/* Timer 2 mode register */
+	__be16	gtrfr1;		/* Timer 1 reference register */
+	__be16	gtrfr2;		/* Timer 2 reference register */
+	__be16	gtcpr1;		/* Timer 1 capture register */
+	__be16	gtcpr2;		/* Timer 2 capture register */
+	__be16	gtcnr1;		/* Timer 1 counter */
+	__be16	gtcnr2;		/* Timer 2 counter */
+	__be16	gtmdr3;		/* Timer 3 mode register */
+	__be16	gtmdr4;		/* Timer 4 mode register */
+	__be16	gtrfr3;		/* Timer 3 reference register */
+	__be16	gtrfr4;		/* Timer 4 reference register */
+	__be16	gtcpr3;		/* Timer 3 capture register */
+	__be16	gtcpr4;		/* Timer 4 capture register */
+	__be16	gtcnr3;		/* Timer 3 counter */
+	__be16	gtcnr4;		/* Timer 4 counter */
+	__be16	gtevr1;		/* Timer 1 event register */
+	__be16	gtevr2;		/* Timer 2 event register */
+	__be16	gtevr3;		/* Timer 3 event register */
+	__be16	gtevr4;		/* Timer 4 event register */
+	__be16	gtpsr1;		/* Timer 1 prescale register */
+	__be16	gtpsr2;		/* Timer 2 prescale register */
+	__be16	gtpsr3;		/* Timer 3 prescale register */
+	__be16	gtpsr4;		/* Timer 4 prescale register */
+	u8 res2[0x40];
+} __attribute__ ((packed));
+
+struct gtm_timer {
+	unsigned int irq;
+
+	struct gtm *gtm;
+	bool requested;
+	u8 __iomem *gtcfr;
+	__be16 __iomem *gtmdr;
+	__be16 __iomem *gtpsr;
+	__be16 __iomem *gtcnr;
+	__be16 __iomem *gtrfr;
+	__be16 __iomem *gtevr;
+};
+
+struct gtm {
+	unsigned int clock;
+	struct gtm_timers_regs __iomem *regs;
+	struct gtm_timer timers[4];
+	spinlock_t lock;
+};
+
+extern struct gtm_timer *gtm_get_timer(int width);
+extern struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer,
+						int width);
+extern void gtm_put_timer(struct gtm_timer *tmr);
+extern int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool free_run);
+extern void gtm_stop_timer16(struct gtm_timer *tmr);
+
+/**
+ * gtm_ack_timer16 - acknowledge timer event (free-run timers only)
+ * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
+ * @events:	events mask to ack
+ *
+ * Thus function used to acknowledge timer interrupt event, use it inside the
+ * interrupt handler.
+ */
+static inline void gtm_ack_timer16(struct gtm_timer *tmr, u16 events)
+{
+	out_be16(tmr->gtevr, events);
+}
+
+#endif /* __ASM_FSL_GTM_H */
-- 
1.5.5

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-16 18:39             ` Anton Vorontsov
@ 2008-04-16 18:44               ` Scott Wood
  2008-04-16 21:00                 ` Anton Vorontsov
  2008-04-17 14:23               ` Laurent Pinchart
  1 sibling, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-04-16 18:44 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev

On Wed, Apr 16, 2008 at 10:39:04PM +0400, Anton Vorontsov wrote:
> +/**
> + * gtm_reset_utimer16 - reset 16 bits timer
> + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> + * @usec:	timer interval in microseconds
> + * @free_run:	free run flag
> + *
> + * This function (re)sets GTM timer so it counts up to the interval value and
> + * fires the interrupt when the value is reached. If free_run flag was set,
> + * timer will also reset itself upon reference value, otherwise it continues to
> + * increment.
> + */
> +int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool free_run)

A maximal timeout of ~65 ms is a little low...  For use as a wakeup from
sleep mode, I'd like to be able to request timeouts as large as the
hardware allows.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-16 18:44               ` Scott Wood
@ 2008-04-16 21:00                 ` Anton Vorontsov
  2008-04-16 21:58                   ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-16 21:00 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Wed, Apr 16, 2008 at 01:44:42PM -0500, Scott Wood wrote:
> On Wed, Apr 16, 2008 at 10:39:04PM +0400, Anton Vorontsov wrote:
> > +/**
> > + * gtm_reset_utimer16 - reset 16 bits timer
> > + * @tmr:	pointer to the gtm_timer structure obtained from gtm_get_timer
> > + * @usec:	timer interval in microseconds
> > + * @free_run:	free run flag
> > + *
> > + * This function (re)sets GTM timer so it counts up to the interval value and
> > + * fires the interrupt when the value is reached. If free_run flag was set,
> > + * timer will also reset itself upon reference value, otherwise it continues to
> > + * increment.
> > + */
> > +int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool free_run)
> 
> A maximal timeout of ~65 ms is a little low...  For use as a wakeup from
> sleep mode, I'd like to be able to request timeouts as large as the
> hardware allows.

That is about precision. You just need to implement gtm_reset_stimer()
is you want precision with a seconds, this will run a timer at 1 Hz.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-16 21:00                 ` Anton Vorontsov
@ 2008-04-16 21:58                   ` Scott Wood
  2008-04-17 12:52                     ` Anton Vorontsov
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-04-16 21:58 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev

On Thu, Apr 17, 2008 at 01:00:42AM +0400, Anton Vorontsov wrote:
> On Wed, Apr 16, 2008 at 01:44:42PM -0500, Scott Wood wrote:
> > A maximal timeout of ~65 ms is a little low...  For use as a wakeup from
> > sleep mode, I'd like to be able to request timeouts as large as the
> > hardware allows.
> 
> That is about precision. You just need to implement gtm_reset_stimer()
> is you want precision with a seconds, this will run a timer at 1 Hz.

Enh.  I'm not crazy about having to call separately named functions,
rather than have the timer code set the reference clock to the highest
precision that has the needed range.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-16 21:58                   ` Scott Wood
@ 2008-04-17 12:52                     ` Anton Vorontsov
  2008-04-17 14:19                       ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-17 12:52 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Wed, Apr 16, 2008 at 04:58:21PM -0500, Scott Wood wrote:
> On Thu, Apr 17, 2008 at 01:00:42AM +0400, Anton Vorontsov wrote:
> > On Wed, Apr 16, 2008 at 01:44:42PM -0500, Scott Wood wrote:
> > > A maximal timeout of ~65 ms is a little low...  For use as a wakeup from
> > > sleep mode, I'd like to be able to request timeouts as large as the
> > > hardware allows.
> > 
> > That is about precision. You just need to implement gtm_reset_stimer()
> > is you want precision with a seconds, this will run a timer at 1 Hz.
> 
> Enh.  I'm not crazy about having to call separately named functions,
> rather than have the timer code set the reference clock to the highest
> precision that has the needed range.

Heh. Scott, think about it. You have single 16bit timer with variable
frequency. To use it, you'd better know what exactly precision you need.
Then you limited to u16 for the interval for this chosen precision.

Yes, you can implement this:

#define MAX_PRESCALER (256 * 256 * 16)

int gtm_reset_weird_behaving_utimer16(struct gtm_timer *tmr,
				      unsigned long long usec,
				      bool free_run)
{
	int freq = 1000000;
	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;

	while (!(freq & 1) && !(usec & 1) && freq >= min_hz2) {
		freq >>= 1;
		usec >>= 1;
	}

	if (usec > 0xffff)
		return -EINVAL;

	return gtm_reset_ref_timer16(tmr, freq, (u16)usec, free_run);
}

This function (depending on clock-frequency) will work for 1001 usecs,
but will return -EINVAL for 1000001 usecs, thought it will work for
1000000 usecs, and for 333000000 it will work too, but will again
return -EINVAL for 333000010 usecs. I hope this pattern is obvious.
This is really weird behaving, isn't it?

Oh, and later you can drop the 16 suffix and implement the dynamically
chosen cascading, depending if usec argument fits into single/double/quad
timer.

Though, I don't need all this stuff in the FHCI irq handlers, I do know
the precision I want. Like the most of kernel code does.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 12:52                     ` Anton Vorontsov
@ 2008-04-17 14:19                       ` Scott Wood
  2008-04-17 15:07                         ` Anton Vorontsov
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-04-17 14:19 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev

On Thu, Apr 17, 2008 at 04:52:35PM +0400, Anton Vorontsov wrote:
> Heh. Scott, think about it. You have single 16bit timer with variable
> frequency. To use it, you'd better know what exactly precision you need.

Why?  I know the timeout I need.

> Then you limited to u16 for the interval for this chosen precision.
> 
> Yes, you can implement this:
> 
> #define MAX_PRESCALER (256 * 256 * 16)
> 
> int gtm_reset_weird_behaving_utimer16(struct gtm_timer *tmr,
> 				      unsigned long long usec,
> 				      bool free_run)
> {
> 	int freq = 1000000;
> 	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;
> 
> 	while (!(freq & 1) && !(usec & 1) && freq >= min_hz2) {
> 		freq >>= 1;
> 		usec >>= 1;
> 	}
> 
> 	if (usec > 0xffff)
> 		return -EINVAL;
> 
> 	return gtm_reset_ref_timer16(tmr, freq, (u16)usec, free_run);
> }

Try something like this:

int gtm_reset_sane_behaving_timer(struct gtm_timer *tmr,
                                  u64 usec, bool free_run)
{
	int freq = 1000000;
	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;

	while (usec > 0xffff && freq >= min_hz2) {
		freq >>= 1;
		usec >>= 1;
	}

	if (usec > 0xffff)
		return -EINVAL;

	return gtm_reset_ref_timer16(tmr, freq, usec, free_run);
}

It could be made faster using cntlzw.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-16 18:39             ` Anton Vorontsov
  2008-04-16 18:44               ` Scott Wood
@ 2008-04-17 14:23               ` Laurent Pinchart
  2008-04-17 15:13                 ` Anton Vorontsov
  2008-04-17 16:12                 ` Anton Vorontsov
  1 sibling, 2 replies; 21+ messages in thread
From: Laurent Pinchart @ 2008-04-17 14:23 UTC (permalink / raw)
  To: linuxppc-dev, avorontsov; +Cc: Scott Wood

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

On Wednesday 16 April 2008 20:39, Anton Vorontsov wrote:
> On Tue, Mar 18, 2008 at 03:48:12PM -0500, Scott Wood wrote:
> [...]
> > How about:
> >
> > struct gtm_timer *gtm_get_specific_timer(struct gtm *gtm, int timer,
> >                                          int width);
> >
> > ...with np->data used by the caller to figure out which gtm pointer to  
> > pass in.
> 
> Thanks for the comments, I've tried to address them all.
> 
> Updated patch below (not for applying, still waiting for further
> comments, if any).
> 
> - - - -
> From: Anton Vorontsov <avorontsov@ru.mvista.com>
> Subject: [POWERPC] sysdev,qe_lib: implement FSL GTM support
> 
> GTM stands for General-purpose Timers Module and able to generate
> timer{1,2,3,4} interrupts.
> 
> There are several limitations in this support:
> 1. Cascaded (32 bit) timers unimplemented (1-2, 3-4).
>    This is straightforward to implement when needed, two timers should
>    be marked as "requested" and configured as appropriate.
> 2. Super-cascaded (64 bit) timers unimplemented (1-2-3-4).
>    This is also straightforward to implement when needed, all timers
>    should be marked as "requested" and configured as appropriate.
> 
> Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
> ---
>  Documentation/powerpc/booting-without-of.txt |   32 +++-
>  arch/powerpc/Kconfig                         |    5 +
>  arch/powerpc/sysdev/Makefile                 |    1 +
>  arch/powerpc/sysdev/fsl_gtm.c                |  322 ++++++++++++++++++++++++++
>  include/asm-powerpc/fsl_gtm.h                |  106 +++++++++
>  5 files changed, 465 insertions(+), 1 deletions(-)
>  create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
>  create mode 100644 include/asm-powerpc/fsl_gtm.h

[snip]

> +/*
> + * This is back-end for the exported functions, it's used to reset single
> + * timer in reference mode.
> + */
> +static int gtm_reset_ref_timer16(struct gtm_timer *tmr, int frequency,
> +				 int reference_value, bool free_run)
> +{
> +	struct gtm *gtm = tmr->gtm;
> +	int num = tmr - &gtm->timers[0];
> +	unsigned int prescaler;
> +	u8 iclk = GTMDR_ICLK_ICLK;
> +	u8 psr;
> +	u8 sps;
> +	unsigned long flags;
> +
> +	prescaler = gtm->clock / frequency;
> +	/*
> +	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
> +	 * plus "slow go" mode (clk / 16). So, total prescale value is
> +	 * 16 * (psr + 1) * (sps + 1).
> +	 */
> +	if (prescaler > 256 * 256 * 16)
> +		return -EINVAL;
> +
> +	if (prescaler > 256 * 256) {
> +		iclk = GTMDR_ICLK_SLGO;
> +		prescaler /= 16;
> +	}
> +
> +	if (prescaler > 256) {
> +		psr = 256 - 1;
> +		sps = prescaler / 256 - 1;
> +	} else {
> +		psr = prescaler - 1;
> +		sps = 1 - 1;
> +	}

Don't forget that the CPM2 doesn't support the primary prescaler.

> +	spin_lock_irqsave(&gtm->lock, flags);
> +
> +	/*
> +	 * Properly reset timers: stop, reset, set up prescalers, reference
> +	 * value and clear event register.
> +	 */
> +	clrsetbits_8(tmr->gtcfr, ~(GTCFR_STP(num) | GTCFR_RST(num)),
> +				 GTCFR_STP(num) | GTCFR_RST(num));
> +
> +	setbits8(tmr->gtcfr, GTCFR_STP(num));
> +
> +	out_be16(tmr->gtpsr, psr);
> +	clrsetbits_be16(tmr->gtmdr, 0xFFFF, iclk | GTMDR_SPS(sps) |
> +			GTMDR_ORI | (free_run ? GTMDR_FFR : 0));
> +	out_be16(tmr->gtcnr, 0);
> +	out_be16(tmr->gtrfr, reference_value);
> +	out_be16(tmr->gtevr, 0xFFFF);
> +
> +	/* Let it be. */
> +	clrbits8(tmr->gtcfr, GTCFR_STP(num));
> +
> +	spin_unlock_irqrestore(&gtm->lock, flags);
> +
> +	return 0;
> +}


-- 
Laurent Pinchart
CSE Semaphore Belgium

Chaussee de Bruxelles, 732A
B-1410 Waterloo
Belgium

T +32 (2) 387 42 59
F +32 (2) 387 42 75

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

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 14:19                       ` Scott Wood
@ 2008-04-17 15:07                         ` Anton Vorontsov
  2008-04-17 16:14                           ` Scott Wood
  0 siblings, 1 reply; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-17 15:07 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Thu, Apr 17, 2008 at 09:19:03AM -0500, Scott Wood wrote:
> On Thu, Apr 17, 2008 at 04:52:35PM +0400, Anton Vorontsov wrote:
> > Heh. Scott, think about it. You have single 16bit timer with variable
> > frequency. To use it, you'd better know what exactly precision you need.
> 
> Why?  I know the timeout I need.
> 
> > Then you limited to u16 for the interval for this chosen precision.
> > 
> > Yes, you can implement this:
> > 
> > #define MAX_PRESCALER (256 * 256 * 16)
> > 
> > int gtm_reset_weird_behaving_utimer16(struct gtm_timer *tmr,
> > 				      unsigned long long usec,
> > 				      bool free_run)
> > {
> > 	int freq = 1000000;
> > 	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;
> > 
> > 	while (!(freq & 1) && !(usec & 1) && freq >= min_hz2) {
> > 		freq >>= 1;
> > 		usec >>= 1;
> > 	}
> > 
> > 	if (usec > 0xffff)
> > 		return -EINVAL;
> > 
> > 	return gtm_reset_ref_timer16(tmr, freq, (u16)usec, free_run);
> > }
> 
> Try something like this:
> 
> int gtm_reset_sane_behaving_timer(struct gtm_timer *tmr,
>                                   u64 usec, bool free_run)
> {
> 	int freq = 1000000;
> 	int min_hz2 = (tmr->gtm->freq / MAX_PRESCALER) << 1;
> 
> 	while (usec > 0xffff && freq >= min_hz2) {

This isn't a timer with usec precision! This is a timer that silently
crops precision as it wants to. Ahh, I see you dropped "u" prefix.

Well. I'm not going to use it anyway, so just give it some name you
prefer and I'll wrap it into the patch. Preferably, drop a line here with
kerneldoc for it, so I'll not have to document its drawbacks. :-)

> 		freq >>= 1;
> 		usec >>= 1;
> 	}
> 
> 	if (usec > 0xffff)
> 		return -EINVAL;
> 
> 	return gtm_reset_ref_timer16(tmr, freq, usec, free_run);
> }
> 
> It could be made faster using cntlzw.

No need to cntlzw, there is fls() already. Though, here you'll need
two because of u64.

Btw, I hope you aware that single GTM timer running at 166MHz will give you
6 minutes of sleep, maximum. With cascaded timer you'll get much better
result of 310 days. Is that possible to use cascaded timer as a wakeup
event on 8313? If so, I'd suggest you to implement cascading firstly.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 14:23               ` Laurent Pinchart
@ 2008-04-17 15:13                 ` Anton Vorontsov
  2008-04-17 16:12                 ` Anton Vorontsov
  1 sibling, 0 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-17 15:13 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Scott Wood, linuxppc-dev

On Thu, Apr 17, 2008 at 04:23:56PM +0200, Laurent Pinchart wrote:
[...]
> > +	/*
> > +	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
> > +	 * plus "slow go" mode (clk / 16). So, total prescale value is
> > +	 * 16 * (psr + 1) * (sps + 1).
> > +	 */
> > +	if (prescaler > 256 * 256 * 16)
> > +		return -EINVAL;
> > +
> > +	if (prescaler > 256 * 256) {
> > +		iclk = GTMDR_ICLK_SLGO;
> > +		prescaler /= 16;
> > +	}
> > +
> > +	if (prescaler > 256) {
> > +		psr = 256 - 1;
> > +		sps = prescaler / 256 - 1;
> > +	} else {
> > +		psr = prescaler - 1;
> > +		sps = 1 - 1;
> > +	}
> 
> Don't forget that the CPM2 doesn't support the primary prescaler.

I didn't know that, how can I possibly forget it? Oh, now I can.
Thanks for the info. :-)

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 14:23               ` Laurent Pinchart
  2008-04-17 15:13                 ` Anton Vorontsov
@ 2008-04-17 16:12                 ` Anton Vorontsov
  1 sibling, 0 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-17 16:12 UTC (permalink / raw)
  To: Laurent Pinchart; +Cc: Scott Wood, linuxppc-dev

On Thu, Apr 17, 2008 at 04:23:56PM +0200, Laurent Pinchart wrote:
> > +	/*
> > +	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
> > +	 * plus "slow go" mode (clk / 16). So, total prescale value is
> > +	 * 16 * (psr + 1) * (sps + 1).
> > +	 */
> > +	if (prescaler > 256 * 256 * 16)
> > +		return -EINVAL;
> > +
> > +	if (prescaler > 256 * 256) {
> > +		iclk = GTMDR_ICLK_SLGO;
> > +		prescaler /= 16;
> > +	}
> > +
> > +	if (prescaler > 256) {
> > +		psr = 256 - 1;
> > +		sps = prescaler / 256 - 1;
> > +	} else {
> > +		psr = prescaler - 1;
> > +		sps = 1 - 1;
> > +	}
> 
> Don't forget that the CPM2 doesn't support the primary prescaler.

Here is incremental diff of how this is solved. I guess this should work.

diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt
index b0ddd54..b89c56d 100644
--- a/Documentation/powerpc/booting-without-of.txt
+++ b/Documentation/powerpc/booting-without-of.txt
@@ -2835,7 +2835,7 @@ platforms are moved over to use the flattened-device-tree model.
 
     Required properties:
       - compatible : should be "fsl,gtm" ("fsl,qe-gtm" in addition for QE
-                     GTMs).
+                     GTMs or "fsl,cpm2-gtm" for CPM2 GTMs).
       - reg : should contain gtm registers location and length (0x40).
       - interrupts : should contain four interrupts.
       - interrupt-parent : interrupt source phandle.
diff --git a/arch/powerpc/sysdev/fsl_gtm.c b/arch/powerpc/sysdev/fsl_gtm.c
index 6d86983..105c633 100644
--- a/arch/powerpc/sysdev/fsl_gtm.c
+++ b/arch/powerpc/sysdev/fsl_gtm.c
@@ -125,27 +125,32 @@ static int gtm_reset_ref_timer16(struct gtm_timer *tmr, int frequency,
 	u8 psr;
 	u8 sps;
 	unsigned long flags;
+	int max_prescaler = 256 * 256 * 16;
+
+	/* CPM2 doesn't have primary prescaler */
+	if (!tmr->gtpsr)
+		max_prescaler /= 256;
 
 	prescaler = gtm->clock / frequency;
 	/*
 	 * We have two 8 bit prescalers -- primary and secondary (psr, sps),
 	 * plus "slow go" mode (clk / 16). So, total prescale value is
-	 * 16 * (psr + 1) * (sps + 1).
+	 * 16 * (psr + 1) * (sps + 1). Though, for CPM2 GTMs we losing psr.
 	 */
-	if (prescaler > 256 * 256 * 16)
+	if (prescaler > max_prescaler)
 		return -EINVAL;
 
-	if (prescaler > 256 * 256) {
+	if (prescaler > max_prescaler / 16) {
 		iclk = GTMDR_ICLK_SLGO;
 		prescaler /= 16;
 	}
 
-	if (prescaler > 256) {
+	if (prescaler <= 256) {
+		psr = 0;
+		sps = prescaler - 1;
+	} else {
 		psr = 256 - 1;
 		sps = prescaler / 256 - 1;
-	} else {
-		psr = prescaler - 1;
-		sps = 1 - 1;
 	}
 
 	spin_lock_irqsave(&gtm->lock, flags);
@@ -159,7 +164,8 @@ static int gtm_reset_ref_timer16(struct gtm_timer *tmr, int frequency,
 
 	setbits8(tmr->gtcfr, GTCFR_STP(num));
 
-	out_be16(tmr->gtpsr, psr);
+	if (tmr->gtpsr)
+		out_be16(tmr->gtpsr, psr);
 	clrsetbits_be16(tmr->gtmdr, 0xFFFF, iclk | GTMDR_SPS(sps) |
 			GTMDR_ORI | (free_run ? GTMDR_FFR : 0));
 	out_be16(tmr->gtcnr, 0);
@@ -222,7 +228,8 @@ void gtm_stop_timer16(struct gtm_timer *tmr)
 }
 EXPORT_SYMBOL(gtm_stop_timer16);
 
-static void __init gtm_set_shortcuts(struct gtm_timer *timers,
+static void __init gtm_set_shortcuts(struct device_node *np,
+				     struct gtm_timer *timers,
 				     struct gtm_timers_regs __iomem *regs)
 {
 	/*
@@ -233,31 +240,35 @@ static void __init gtm_set_shortcuts(struct gtm_timer *timers,
 	 */
 	timers[0].gtcfr = &regs->gtcfr1;
 	timers[0].gtmdr = &regs->gtmdr1;
-	timers[0].gtpsr = &regs->gtpsr1;
 	timers[0].gtcnr = &regs->gtcnr1;
 	timers[0].gtrfr = &regs->gtrfr1;
 	timers[0].gtevr = &regs->gtevr1;
 
 	timers[1].gtcfr = &regs->gtcfr1;
 	timers[1].gtmdr = &regs->gtmdr2;
-	timers[1].gtpsr = &regs->gtpsr2;
 	timers[1].gtcnr = &regs->gtcnr2;
 	timers[1].gtrfr = &regs->gtrfr2;
 	timers[1].gtevr = &regs->gtevr2;
 
 	timers[2].gtcfr = &regs->gtcfr2;
 	timers[2].gtmdr = &regs->gtmdr3;
-	timers[2].gtpsr = &regs->gtpsr3;
 	timers[2].gtcnr = &regs->gtcnr3;
 	timers[2].gtrfr = &regs->gtrfr3;
 	timers[2].gtevr = &regs->gtevr3;
 
 	timers[3].gtcfr = &regs->gtcfr2;
 	timers[3].gtmdr = &regs->gtmdr4;
-	timers[3].gtpsr = &regs->gtpsr4;
 	timers[3].gtcnr = &regs->gtcnr4;
 	timers[3].gtrfr = &regs->gtrfr4;
 	timers[3].gtevr = &regs->gtevr4;
+
+	/* CPM2 doesn't have primary prescaler */
+	if (!of_device_is_compatible(np, "fsl,cpm2-gtm")) {
+		timers[0].gtpsr = &regs->gtpsr1;
+		timers[1].gtpsr = &regs->gtpsr2;
+		timers[2].gtpsr = &regs->gtpsr3;
+		timers[3].gtpsr = &regs->gtpsr4;
+	}
 }
 
 static int __init gtm_init(void)
@@ -307,7 +318,7 @@ static int __init gtm_init(void)
 			goto err;
 		}
 
-		gtm_set_shortcuts(gtm->timers, gtm->regs);
+		gtm_set_shortcuts(np, gtm->timers, gtm->regs);
 
 		/* We don't want to lose the node and its ->data */
 		of_node_get(np);

^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 15:07                         ` Anton Vorontsov
@ 2008-04-17 16:14                           ` Scott Wood
  2008-04-17 16:43                             ` Anton Vorontsov
  0 siblings, 1 reply; 21+ messages in thread
From: Scott Wood @ 2008-04-17 16:14 UTC (permalink / raw)
  To: avorontsov; +Cc: linuxppc-dev

Anton Vorontsov wrote:
> This isn't a timer with usec precision! This is a timer that silently
> crops precision as it wants to. Ahh, I see you dropped "u" prefix.

It is a timer with usec precision, unless you ask for a timeout of more 
than 65535 usec -- at which point the hardware can't provide usec precision.

And s/as it wants to/as it needs to/.

> Well. I'm not going to use it anyway, so just give it some name you
> prefer and I'll wrap it into the patch. Preferably, drop a line here with
> kerneldoc for it, so I'll not have to document its drawbacks. :-)

/**
  * gtm_reset_timer16 - reset 16 bit timer with arbitrary precision
  * @tmr: pointer to the gtm_timer structure obtained from gtm_get_timer
  * @usec: timer interval in microseconds
  * @reload: if set, the timer will reset upon expiry rather than
  * continue running free.
  *
  * This function (re)sets the GTM timer so that it counts up to the
  * requested interval value, and fires the interrupt when the value is
  * reached.  This function will reduce the precision of the timer as
  * needed in order for the requested timeout to fit in a 16-bit
  * register.
  */
int gtm_reset_timer16(struct gtm_timer *tmr, unsigned long usec,
                       bool reload)
{
	...
}

>> It could be made faster using cntlzw.
> 
> No need to cntlzw, there is fls() already.

fls() uses cntlzw, does it not?  I was just too lazy to look up what 
Linux calls it. :-)

> Though, here you'll need two because of u64.

We can probably get away with 32 bits.

> Btw, I hope you aware that single GTM timer running at 166MHz will give you
> 6 minutes of sleep, maximum.

Yes, but it's all we have on-chip that can do the job.

> With cascaded timer you'll get much better
> result of 310 days. Is that possible to use cascaded timer as a wakeup
> event on 8313? 

No, unfortunately.  Only timer4 can be a wakeup source, and when 
cascaded, timer4 is the input to timer3, rather than the other way around.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
@ 2008-04-17 16:22 Scott Wood
  0 siblings, 0 replies; 21+ messages in thread
From: Scott Wood @ 2008-04-17 16:22 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: linuxppc-dev

On Thu, Apr 17, 2008 at 04:39:04AM +1000, Anton Vorontsov wrote:
> +#define GTMDR_FFR		(1 << 3)

This should be GTMDR_FRR according to the chip docs.

-Scott

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support
  2008-04-17 16:14                           ` Scott Wood
@ 2008-04-17 16:43                             ` Anton Vorontsov
  0 siblings, 0 replies; 21+ messages in thread
From: Anton Vorontsov @ 2008-04-17 16:43 UTC (permalink / raw)
  To: Scott Wood; +Cc: linuxppc-dev

On Thu, Apr 17, 2008 at 11:14:00AM -0500, Scott Wood wrote:
> Anton Vorontsov wrote:
>> This isn't a timer with usec precision! This is a timer that silently
>> crops precision as it wants to. Ahh, I see you dropped "u" prefix.
>
> It is a timer with usec precision, unless you ask for a timeout of more  
> than 65535 usec -- at which point the hardware can't provide usec 
> precision.
>
> And s/as it wants to/as it needs to/.
>
>> Well. I'm not going to use it anyway, so just give it some name you
>> prefer and I'll wrap it into the patch. Preferably, drop a line here with
>> kerneldoc for it, so I'll not have to document its drawbacks. :-)
>
> /**
>  * gtm_reset_timer16 - reset 16 bit timer with arbitrary precision
>  * @tmr: pointer to the gtm_timer structure obtained from gtm_get_timer
>  * @usec: timer interval in microseconds
>  * @reload: if set, the timer will reset upon expiry rather than
>  * continue running free.
>  *
>  * This function (re)sets the GTM timer so that it counts up to the
>  * requested interval value, and fires the interrupt when the value is
>  * reached.  This function will reduce the precision of the timer as
>  * needed in order for the requested timeout to fit in a 16-bit
>  * register.
>  */
> int gtm_reset_timer16(struct gtm_timer *tmr, unsigned long usec,
>                       bool reload)
> {
> 	...
> }

Thanks!

>>> It could be made faster using cntlzw.
>>
>> No need to cntlzw, there is fls() already.
>
> fls() uses cntlzw, does it not?  I was just too lazy to look up what  
> Linux calls it. :-)

Yup, I looked it up. ;-)

>> Though, here you'll need two because of u64.
>
> We can probably get away with 32 bits.
>
>> Btw, I hope you aware that single GTM timer running at 166MHz will give you
>> 6 minutes of sleep, maximum.
>
> Yes, but it's all we have on-chip that can do the job.
>
>> With cascaded timer you'll get much better
>> result of 310 days. Is that possible to use cascaded timer as a wakeup
>> event on 8313? 
>
> No, unfortunately.  Only timer4 can be a wakeup source, and when  
> cascaded, timer4 is the input to timer3, rather than the other way 
> around.

Ok, very well.

-- 
Anton Vorontsov
email: cbouatmailru@gmail.com
irc://irc.freenode.net/bd2

^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2008-04-17 16:43 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-04-17 16:22 [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support Scott Wood
  -- strict thread matches above, loose matches on Subject: below --
2008-03-11 17:21 [PATCH 0/8] A bit of new code and sparse cleanups along the way Anton Vorontsov
2008-03-11 17:24 ` [PATCH 6/8] [POWERPC] sysdev,qe_lib: implement FSL GTM support Anton Vorontsov
2008-03-18 17:43   ` Scott Wood
2008-03-18 19:21     ` Anton Vorontsov
2008-03-18 19:55       ` Scott Wood
2008-03-18 20:27         ` Anton Vorontsov
2008-03-18 20:48           ` Scott Wood
2008-04-16 18:39             ` Anton Vorontsov
2008-04-16 18:44               ` Scott Wood
2008-04-16 21:00                 ` Anton Vorontsov
2008-04-16 21:58                   ` Scott Wood
2008-04-17 12:52                     ` Anton Vorontsov
2008-04-17 14:19                       ` Scott Wood
2008-04-17 15:07                         ` Anton Vorontsov
2008-04-17 16:14                           ` Scott Wood
2008-04-17 16:43                             ` Anton Vorontsov
2008-04-17 14:23               ` Laurent Pinchart
2008-04-17 15:13                 ` Anton Vorontsov
2008-04-17 16:12                 ` Anton Vorontsov
2008-04-08  9:01   ` Laurent Pinchart
2008-04-08 11:48     ` Anton Vorontsov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).