diff mbox

[RFC/PATCH,09/11] clocksource: add TI 32.768 Hz counter driver

Message ID 1443559446-26969-10-git-send-email-balbi@ti.com
State New
Headers show

Commit Message

Felipe Balbi Sept. 29, 2015, 8:44 p.m. UTC
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@ti.com>
---
 drivers/clocksource/Kconfig        |   8 +++
 drivers/clocksource/Makefile       |   1 +
 drivers/clocksource/timer-ti-32k.c | 121 +++++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+)
 create mode 100644 drivers/clocksource/timer-ti-32k.c

Comments

Daniel Lezcano Oct. 1, 2015, 9:59 p.m. UTC | #1
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@ti.com>
> ---

[ ... ]

> +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
Tony Lindgren Oct. 5, 2015, 10:50 a.m. UTC | #2
* Daniel Lezcano <daniel.lezcano@linaro.org> [151001 15:04]:
> >+	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.

The persistent clock we want to keep for deeper idle states..
Especially when we start changing clocksource for the duration of
deeper idle states. I'll post some updated patches on that when I
get a chance.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a7726db13abb..0ccfd12a00a3 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -115,6 +115,14 @@  config CLKSRC_PISTACHIO
 	bool
 	select CLKSRC_OF
 
+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.
+
 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>
+
+/* OMAP2_32KSYNCNT_CR_OFF: offset of 32ksync counter register */
+#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.
+ */
+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;
+
+	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.
+	 */
+	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.
+	 */
+	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);
+	pr_info("OMAP clocksource: 32k_counter at 32768 Hz\n");
+}
+CLOCKSOURCE_OF_DECLARE(ti_32k_timer, "ti,omap-counter32k",
+		ti_32k_timer_init);