Hi Felipe, On 09/29/2015 10:44 PM, Felipe Balbi wrote:
Introduce a new clocksource driver for Texas Instruments 32.768 Hz device which is available on most OMAP-like devices. Signed-off-by: Felipe Balbi <balbi@xxxxxx> ---
[ ... ]
+config CLKSRC_TI_32K + bool "Texas Instruments 32.768 Hz Clocksource" + depends on OF && ARCH_OMAP2PLUS + select CLKSRC_OF + help + This option enables support for Texas Instruments 32.768 Hz clocksource + available on many OMAP-like platforms. +
It is the omap's Kconfig which should select the timer, not the user to enable the timer.
It should be something like: config CLKSRC_TI_32K bool "Texas Instruments 32.768 Hz Clocksource" if COMPILE_TEST select CLKSRC_OF if OF help This option enables support for Texas Instruments 32.768 Hz clocksource available on many OMAP-like platforms. And in the omap's Kconfig: select CLKSRC_TI_32K
config CLKSRC_STM32 bool "Clocksource for STM32 SoCs" if !ARCH_STM32 depends on OF && ARM && (ARCH_STM32 || COMPILE_TEST) diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile index 5c00863c3e33..749abc3665b3 100644 --- a/drivers/clocksource/Makefile +++ b/drivers/clocksource/Makefile @@ -45,6 +45,7 @@ obj-$(CONFIG_VF_PIT_TIMER) += vf_pit_timer.o obj-$(CONFIG_CLKSRC_QCOM) += qcom-timer.o obj-$(CONFIG_MTK_TIMER) += mtk_timer.o obj-$(CONFIG_CLKSRC_PISTACHIO) += time-pistachio.o +obj-$(CONFIG_CLKSRC_TI_32K) += timer-ti-32k.o obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o obj-$(CONFIG_ARM_GLOBAL_TIMER) += arm_global_timer.o diff --git a/drivers/clocksource/timer-ti-32k.c b/drivers/clocksource/timer-ti-32k.c new file mode 100644 index 000000000000..10ccce2eb645 --- /dev/null +++ b/drivers/clocksource/timer-ti-32k.c @@ -0,0 +1,121 @@ +/** + * timer-ti-32k.c - OMAP2 32k Timer Support + * + * Copyright (C) 2015 Texas Instruments Incorporated - http://www.ti.com + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 of + * the License as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <linux/init.h> +#include <linux/time.h> +#include <linux/interrupt.h> +#include <linux/err.h> +#include <linux/irq.h> +#include <linux/sched_clock.h> +#include <linux/clocksource.h> +#include <linux/slab.h> +#include <linux/of.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> + +#include <asm/mach/time.h>
Can you check all these headers are needed ? I don't think interrupt.h, or slab.h or irq.h, etc ... are needed.
A few nits below:
+/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */
I am not sure this comment gives some interesting indication.
+#define OMAP2_32KSYNCNT_REV_OFF 0x0 +#define OMAP2_32KSYNCNT_REV_SCHEME (0x3 << 30) +#define OMAP2_32KSYNCNT_CR_OFF_LOW 0x10 +#define OMAP2_32KSYNCNT_CR_OFF_HIGH 0x30 + +/* + * 32KHz clocksource ... always available, on pretty most chips except + * OMAP 730 and 1510. Other timers could be used as clocksources, with + * higher resolution in free-running counter modes (e.g. 12 MHz xtal), + * but systems won't necessarily want to spend resources that way. + */ +static void __iomem *sync32k_cnt_reg; + +/** + * omap_read_persistent_clock64 - Return time from a persistent clock. + * + * Reads the time from a source which isn't disabled during PM, the + * 32k sync timer. Convert the cycles elapsed since last read into + * nsecs and adds to a monotonically increasing timespec64. + */
This comment above should be moved right before the function it describes and in order to comply with the kernel-doc format the parameters must be documented also:
... * @ts: .... ...
+static struct timespec64 persistent_ts; +static cycles_t cycles; +static unsigned int persistent_mult, persistent_shift; + +static u64 notrace omap_32k_read_sched_clock(void) +{ + return readl_relaxed(sync32k_cnt_reg); +} + +static void omap_read_persistent_clock64(struct timespec64 *ts) +{ + unsigned long long nsecs; + cycles_t last_cycles; + + last_cycles = cycles; + cycles = sync32k_cnt_reg ? readl_relaxed(sync32k_cnt_reg) : 0;
This test is pointless because 'sync32k_cnt_reg' is always different from zero regarding the init routine, no ?
+ + nsecs = clocksource_cyc2ns(cycles - last_cycles, + persistent_mult, persistent_shift); + + timespec64_add_ns(&persistent_ts, nsecs); + + *ts = persistent_ts; +} + +static void __init ti_32k_timer_init(struct device_node *np) +{ + void __iomem *vbase; + int ret; + + vbase = of_iomap(np, 0); + if (!vbase) { + pr_err("Can't ioremap 32k timer base\n"); + return; + } + + /* + * 32k sync Counter IP register offsets vary between the + * highlander version and the legacy ones. + * The 'SCHEME' bits(30-31) of the revision register is used + * to identify the version. + */
Please fix comment length.
+ if (readl_relaxed(vbase + OMAP2_32KSYNCNT_REV_OFF) & + OMAP2_32KSYNCNT_REV_SCHEME) + sync32k_cnt_reg = vbase + OMAP2_32KSYNCNT_CR_OFF_HIGH; + else + sync32k_cnt_reg = vbase + OMAP2_32KSYNCNT_CR_OFF_LOW; + + /* + * 120000 rough estimate from the calculations in + * __clocksource_update_freq_scale. + */
Fix comment length also.
+ clocks_calc_mult_shift(&persistent_mult, &persistent_shift, + 32768, NSEC_PER_SEC, 120000); + + ret = clocksource_mmio_init(sync32k_cnt_reg, "32k_counter", 32768, + 250, 32, clocksource_mmio_readl_up); + if (ret) { + pr_err("32k_counter: can't register clocksource\n"); + return; + } + + sched_clock_register(omap_32k_read_sched_clock, 32, 32768); + register_persistent_clock(NULL, omap_read_persistent_clock64);
I will let John Stultz to have a look at this part because I have doubt regarding the usage of the persistent clock.
-- Daniel -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html