Re: [PATCH 1/7] [POWERPC] sysdev: implement FSL GTM support
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
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
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
+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
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
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
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