Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-06-10 Thread Kumar Gala


On May 23, 2008, at 11:38 AM, Anton Vorontsov wrote:


GTM stands for General-purpose Timers Module and able to generate
timer{1,2,3,4} interrupts. These timers are used by the drivers that
need time precise interrupts (like for USB transactions scheduling for
the Freescale USB Host controller as found in some QE and CPM chips),
or these timers could be used as wakeup events from the CPU deep-sleep
mode.

Things unimplemented:
1. Cascaded (32 bit) timers (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 (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 [EMAIL PROTECTED]
---
Documentation/powerpc/booting-without-of.txt |   32 ++
arch/powerpc/Kconfig |6 +
arch/powerpc/sysdev/Makefile |1 +
arch/powerpc/sysdev/fsl_gtm.c|  434 + 
+

include/asm-powerpc/fsl_gtm.h|   47 +++
5 files changed, 520 insertions(+), 0 deletions(-)
create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
create mode 100644 include/asm-powerpc/fsl_gtm.h



applied.

- k

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Kumar Gala


On May 19, 2008, at 12:46 PM, Anton Vorontsov wrote:


GTM stands for General-purpose Timers Module and able to generate
timer{1,2,3,4} interrupts. These timers are used by the drivers that
need time precise interrupts (like for USB transactions scheduling for
the Freescale USB Host controller as found in some QE and CPM chips),
or these timers could be used as wakeup events from the CPU deep-sleep
mode.

Things unimplemented:
1. Cascaded (32 bit) timers (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 (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 [EMAIL PROTECTED]
---
Documentation/powerpc/booting-without-of.txt |   37 +++-
arch/powerpc/Kconfig |5 +
arch/powerpc/sysdev/Makefile |1 +
arch/powerpc/sysdev/fsl_gtm.c|  424 + 
+

include/asm-powerpc/fsl_gtm.h|   47 +++
5 files changed, 513 insertions(+), 1 deletions(-)
create mode 100644 arch/powerpc/sysdev/fsl_gtm.c
create mode 100644 include/asm-powerpc/fsl_gtm.h




diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 3934e26..e5d3366 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -538,6 +538,11 @@ config FSL_LBC
help
  Freescale Localbus support

+config FSL_GTM
+   bool


I'd prefer something like:

depends on PPC_83xx || QUICC_ENGINE || CPM2



+   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/fsl_gtm.c b/arch/powerpc/sysdev/ 
fsl_gtm.c

new file mode 100644
index 000..8b35cc4
--- /dev/null
+++ b/arch/powerpc/sysdev/fsl_gtm.c
@@ -0,0 +1,424 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *   Shlomi Gridish [EMAIL PROTECTED]
+ *   Jerry Huang [EMAIL PROTECTED]
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *   Anton Vorontsov [EMAIL PROTECTED]
+ *
+ * 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/list.h
+#include linux/io.h
+#include linux/of.h
+#include linux/spinlock.h
+#include linux/bitops.h
+#include asm/fsl_gtm.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_FRR  (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 {
+   unsigned int clock;
+   struct 

Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Anton Vorontsov
On Tue, May 20, 2008 at 01:04:55AM -0500, Kumar Gala wrote:
[...]
 diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
 index 3934e26..e5d3366 100644
 --- a/arch/powerpc/Kconfig
 +++ b/arch/powerpc/Kconfig
 @@ -538,6 +538,11 @@ config FSL_LBC
  help
Freescale Localbus support

 +config FSL_GTM
 +bool

 I'd prefer something like:

 depends on PPC_83xx || QUICC_ENGINE || CPM2

Ok.

 +/**
 + * 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);

 should we not range check timer (since it can only be 0..3)?

I thought that caller should be smart enough to not ask for bogus
timers. Ok, will range check.

 +
 +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;

 should we not stop the timer as well?

Sounds reasonable. I recalling that I intended to implement that,
but obviously forgot. ;-)


 +
 +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;
 +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). Though, for CPM2 GTMs we losing psr.
 + */
 +if (prescaler  max_prescaler)
 +return -EINVAL;
 +
 +if (prescaler  max_prescaler / 16) {
 +iclk = GTMDR_ICLK_SLGO;
 +prescaler /= 16;
 +}
 +
 +if (prescaler = 256) {
 +psr = 0;
 +sps = prescaler - 1;
 +} else {
 +psr = 256 - 1;
 +sps = prescaler / 256 - 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));
 +
 +if (tmr-gtpsr)
 +out_be16(tmr-gtpsr, psr);
 +clrsetbits_be16(tmr-gtmdr, 0x, iclk | GTMDR_SPS(sps) |
 +GTMDR_ORI | (free_run ? GTMDR_FRR : 0));
 +out_be16(tmr-gtcnr, 0);
 +out_be16(tmr-gtrfr, reference_value);
 +out_be16(tmr-gtevr, 0x);
 +
 +/* Let it be. */
 +clrbits8(tmr-gtcfr, GTCFR_STP(num));
 +
 +spin_unlock_irqrestore(gtm-lock, flags);
 +
 +return 0;
 +}
 +/**
 + * 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.

 is this save to call in interrupt context?

Yes. Will document.


 + */
 +int gtm_reset_timer16(struct gtm_timer *tmr, unsigned long usec, bool 
 reload)
 +{
 +/* quite obvious, frequency which is enough for µSec precision */
 +int freq = 100;
 +unsigned int bit;
 +
 +bit = fls_long(usec);
 +if (bit  15) {
 

Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Kumar Gala

+void __init fsl_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;
+   }


why bother with making this a dynamic alloc?


Because different platforms have different number of GTMs
blocks. For QE machines this could be up to three GTMs, and QE-less
usually implement two GTMs. Not sure about CPM2.


ok, that makes sense.


+
+   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(np, gtm-timers, gtm-regs);
+   list_add(gtm-list_node, gtms);
+
+   /* We don't want to lose the node and its -data */
+   np-data = gtm;
+   of_node_get(np);
+
+   continue;
+err:
+   kfree(gtm);
+   }
+}


Shouldn't we have an arch_initcall(fsl_gtm_init);


There (and in the QE GPIO) was an arch_initcall, but based on
Grant Likely's review it was removed in favour of platform-specific
machine_initcalls.

See http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg16469.html
There I was trying to argue, but quickly gave up. ;-) I don't have any
strong preference for this anyway. I can do either way, just tell  
which

you prefer.


I'd prefer the arch_initcall().  If its the board that is going to do  
the Kconfig select on this that seems sufficient to say do init for  
me w/o an explicit call to it.



diff --git a/include/asm-powerpc/fsl_gtm.h b/include/asm-powerpc/
fsl_gtm.h
new file mode 100644
index 000..49f1240
--- /dev/null
+++ b/include/asm-powerpc/fsl_gtm.h
@@ -0,0 +1,47 @@
+/*
+ * Freescale General-purpose Timers Module
+ *
+ * Copyright (c) Freescale Semicondutor, Inc. 2006.
+ *   Shlomi Gridish [EMAIL PROTECTED]
+ *   Jerry Huang [EMAIL PROTECTED]
+ * Copyright (c) MontaVista Software, Inc. 2008.
+ *   Anton Vorontsov [EMAIL PROTECTED]
+ *
+ * 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
+
+struct gtm;
+
+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;
+};
+
+extern void __init fsl_gtm_init(void);
+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_timer16(struct gtm_timer *tmr, unsigned long
usec,
+bool reload);
+extern int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool
reload);


can you explain the difference between these two.  I'm not sure I
understand the difference.


This is explained in the .c file with a kernel doc. Basically the
difference is that timer16 could silently crop the precision, while
utimer16 could not thus explicitly accepts u16 argument (max. timer
interval with usec precision fits in u16).


Maybe I'm confused what the utility is of cropping the precision in  
this way is.  I'd also say that _timer16 is poorly named to convey the  
behavior.  I'm not sure what to call it because I still dont get  
exactly 

Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Anton Vorontsov
On Tue, May 20, 2008 at 08:15:15AM -0500, Kumar Gala wrote:
[...]
 +  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;
 +  }

 why bother with making this a dynamic alloc?

 Because different platforms have different number of GTMs
 blocks. For QE machines this could be up to three GTMs, and QE-less
 usually implement two GTMs. Not sure about CPM2.

 ok, that makes sense.

 +
 +  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(np, gtm-timers, gtm-regs);
 +  list_add(gtm-list_node, gtms);
 +
 +  /* We don't want to lose the node and its -data */
 +  np-data = gtm;
 +  of_node_get(np);
 +
 +  continue;
 +err:
 +  kfree(gtm);
 +  }
 +}

 Shouldn't we have an arch_initcall(fsl_gtm_init);

 There (and in the QE GPIO) was an arch_initcall, but based on
 Grant Likely's review it was removed in favour of platform-specific
 machine_initcalls.

 See http://www.mail-archive.com/linuxppc-dev@ozlabs.org/msg16469.html
 There I was trying to argue, but quickly gave up. ;-) I don't have any
 strong preference for this anyway. I can do either way, just tell  
 which
 you prefer.

 I'd prefer the arch_initcall().  If its the board that is going to do  
 the Kconfig select on this that seems sufficient to say do init for me 
 w/o an explicit call to it.

IIRC, the argument was that we don't need unnecessary initcalls for the
multi-platform kernels. With arch_initcall() GTM/QE GPIOs will be probed
regardless of a board the kernel currently running at. With
machine_initcalls we only probe the GTMs/QE GPIOs on the boards which
actually use it.

Once again, I see pros and cons of both ways, and I don't have preference,
so.. ok, I will revert the arch_initcall() for GTM and QE GPIO.

 diff --git a/include/asm-powerpc/fsl_gtm.h b/include/asm-powerpc/
 fsl_gtm.h
 new file mode 100644
 index 000..49f1240
 --- /dev/null
 +++ b/include/asm-powerpc/fsl_gtm.h
 @@ -0,0 +1,47 @@
 +/*
 + * Freescale General-purpose Timers Module
 + *
 + * Copyright (c) Freescale Semicondutor, Inc. 2006.
 + *   Shlomi Gridish [EMAIL PROTECTED]
 + *   Jerry Huang [EMAIL PROTECTED]
 + * Copyright (c) MontaVista Software, Inc. 2008.
 + *   Anton Vorontsov [EMAIL PROTECTED]
 + *
 + * 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
 +
 +struct gtm;
 +
 +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;
 +};
 +
 +extern void __init fsl_gtm_init(void);
 +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_timer16(struct gtm_timer *tmr, unsigned long
 usec,
 +   bool reload);
 +extern int gtm_reset_utimer16(struct gtm_timer *tmr, u16 usec, bool
 reload);

 can you explain the difference between these two.  I'm not sure I
 understand the difference.

 This is explained in the .c file with a kernel doc. Basically the
 difference is that timer16 could silently crop the precision, while
 utimer16 could not thus explicitly accepts u16 argument 

Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Anton Vorontsov
On Tue, May 20, 2008 at 09:20:50AM -0500, Kumar Gala wrote:
 This is explained in the .c file with a kernel doc. Basically the
 difference is that timer16 could silently crop the precision, while
 utimer16 could not thus explicitly accepts u16 argument (max. timer
 interval with usec precision fits in u16).

 Maybe I'm confused what the utility is of cropping the precision in  
 this
 way is.  I'd also say that _timer16 is poorly named to convey the
 behavior. I'm not sure what to call it because I still dont get  
 exactly
 why you'd want the precision cropped.

 Precision matters for FHCI-like drivers, when driver, for example,
 schedule transactions via the GTM timers, and there timings matters
 a lot.

 Though, timer16 crops the precision _only_ if usecs  65535, so FHCI
 _can_ still use the _timer16 (because FHCI does not request intervals
 65535). But I implemented two function because:

 1. I think we don't need unnecessary stuff in the ISRs (this is weak
   argument since I didn't measure the impact).
 2. I wanted to make the API clear (seem to fail this undertaking :-),
   which functions will behave exactly the way you asked it (utimer16),
   and which functions will _silently_ crop the precision (timer16)
   (if asked for 1001000 usecs, it will give you ~~1001000, depending
   on the GTM frequency).

 I'm fine w/having both. I think they are poorly named.  I'd also call  
 them _set_timer but that's just me.

 Maybe something w/the term _exact_ in the name.  Is it the case w/the  
 precise form we'd have no prescaling (if so maybe a comment in the API  
 about that would help clarity)?

We're always prescale [to 100 MHz for usec precision]. Otherwise
these functions would be non-deterministic (will purely depend on the GTM
frequency, so usec argument would not have defined boundaries).

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev


Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support

2008-05-20 Thread Anton Vorontsov
On Tue, May 20, 2008 at 06:32:34PM +0400, Anton Vorontsov wrote:
 On Tue, May 20, 2008 at 09:20:50AM -0500, Kumar Gala wrote:
  This is explained in the .c file with a kernel doc. Basically the
  difference is that timer16 could silently crop the precision, while
  utimer16 could not thus explicitly accepts u16 argument (max. timer
  interval with usec precision fits in u16).
 
  Maybe I'm confused what the utility is of cropping the precision in  
  this
  way is.  I'd also say that _timer16 is poorly named to convey the
  behavior. I'm not sure what to call it because I still dont get  
  exactly
  why you'd want the precision cropped.
 
  Precision matters for FHCI-like drivers, when driver, for example,
  schedule transactions via the GTM timers, and there timings matters
  a lot.
 
  Though, timer16 crops the precision _only_ if usecs  65535, so FHCI
  _can_ still use the _timer16 (because FHCI does not request intervals
  65535). But I implemented two function because:
 
  1. I think we don't need unnecessary stuff in the ISRs (this is weak
argument since I didn't measure the impact).
  2. I wanted to make the API clear (seem to fail this undertaking :-),
which functions will behave exactly the way you asked it (utimer16),
and which functions will _silently_ crop the precision (timer16)
(if asked for 1001000 usecs, it will give you ~~1001000, depending
on the GTM frequency).
 
  I'm fine w/having both. I think they are poorly named.  I'd also call  
  them _set_timer but that's just me.
 
  Maybe something w/the term _exact_ in the name.  Is it the case w/the  
  precise form we'd have no prescaling (if so maybe a comment in the API  
  about that would help clarity)?
 
 We're always prescale [to 100 MHz for usec precision].

-MHz
+Hz

-- 
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev