devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Add st_lpc clocksource timer driver.
@ 2015-04-17 10:50 Peter Griffin
  2015-04-17 10:50 ` [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource Peter Griffin
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
	maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ
  Cc: peter.griffin-QSEj5FYQhm4dnm+yROfE0A,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi,

Following on from the discussion here
https://www.mail-archive.com/devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org/msg68857.html

This series adds the st_lpc clocksource driver found on stih407 family
silicon. Regardless of whether the change referenced above actually gets
merged, adding this alternative clocksource driver is useful so that we
can activate cpufreq upstream on stih407 family.

regards,

Peter.

Peter Griffin (5):
  clocksource: st_lpc: Add LPC timer as a clocksource.
  clocksource: st_lpc: Add DT bindings documentation for lpc timer
  MAINTAINERS: Add st_lpc.c to ARCH/STI section of maintainers
  ARM: sti: Always enable CLKSRC_ST_LPC_CLOCK
  ARM: DT: STi: STiH407: Add DT node for st-lpc timer.

 .../devicetree/bindings/timer/st,lpc-timer.txt     |  15 ++
 MAINTAINERS                                        |   1 +
 arch/arm/boot/dts/stih407-family.dtsi              |   7 +
 arch/arm/mach-sti/Kconfig                          |   7 +-
 drivers/clocksource/Kconfig                        |  16 +++
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/st_lpc.c                       | 154 +++++++++++++++++++++
 7 files changed, 198 insertions(+), 3 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/timer/st,lpc-timer.txt
 create mode 100644 drivers/clocksource/st_lpc.c

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
  2015-04-17 10:50 [PATCH 0/5] Add st_lpc clocksource timer driver Peter Griffin
@ 2015-04-17 10:50 ` Peter Griffin
  2015-04-17 21:16   ` Paul Bolle
       [not found]   ` <1429267823-8879-2-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2015-04-17 10:50 ` [PATCH 2/5] clocksource: st_lpc: Add DT bindings documentation for lpc timer Peter Griffin
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx
  Cc: peter.griffin, lee.jones, devicetree, Ajit Pal Singh

This patch adds support for the LPC timer as a clocksource
which is found on stih407 family SoCs.

We wish to use the LPC timer as a clocksource instead of
arm_global_timer, as the latter is tied to CPU frequency, and
that driver currently makes no account for frequency scaling.

Once this driver is merged cpufreq can be enabled for stih407
family SoCs without also effecting sched_clock.

Signed-off-by: Ajit Pal Singh <ajitpal.singh@st.com>
Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 drivers/clocksource/Kconfig  |  16 +++++
 drivers/clocksource/Makefile |   1 +
 drivers/clocksource/st_lpc.c | 154 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 171 insertions(+)
 create mode 100644 drivers/clocksource/st_lpc.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index a0b036c..29cd67d 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -253,4 +253,20 @@ config CLKSRC_PXA
 	help
 	  This enables OST0 support available on PXA and SA-11x0
 	  platforms.
+
+config CLKSRC_ST_LPC_CLOCK
+	bool
+	depends on ARCH_STI
+	select CLKSRC_OF if OF
+	help
+	  Enable this option to use the Low Power controller timer
+	  as clock source.
+
+config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
+	bool
+	depends on ST_LPC_CLOCK
+	default y
+	help
+	  Use Low Power controller timer clock source as sched_clock
+
 endmenu
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 752d5c7..356d331 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -51,3 +51,4 @@ obj-$(CONFIG_ARCH_INTEGRATOR_AP)	+= timer-integrator-ap.o
 obj-$(CONFIG_CLKSRC_VERSATILE)		+= versatile.o
 obj-$(CONFIG_CLKSRC_MIPS_GIC)		+= mips-gic-timer.o
 obj-$(CONFIG_ASM9260_TIMER)		+= asm9260_timer.o
+obj-$(CONFIG_CLKSRC_ST_LPC_CLOCK)	+= st_lpc.o
diff --git a/drivers/clocksource/st_lpc.c b/drivers/clocksource/st_lpc.c
new file mode 100644
index 0000000..f9abded
--- /dev/null
+++ b/drivers/clocksource/st_lpc.c
@@ -0,0 +1,154 @@
+/*
+ * This driver implements a Clocksource using the Low Power Timer in
+ * the Low Power Controller (LPC) in some STMicroelectronics
+ * STi series SoCs
+ *
+ * Copyright (C) 2015 STMicroelectronics Limited
+ * Author: Francesco Virlinzi <francesco.virlinzi@st.com>
+ * Author: Ajit Pal Singh <ajitpal.singh@st.com>
+ *
+ * May be copied or modified under the terms of the GNU General Public
+ * License.  See linux/COPYING for more information.
+ */
+
+#include <linux/clk.h>
+#include <linux/clocksource.h>
+#include <linux/init.h>
+#include <linux/of_address.h>
+#include <linux/sched_clock.h>
+#include <linux/slab.h>
+
+/* Low Power Timer */
+#define LPC_LPT_LSB_OFF		0x400
+#define LPC_LPT_MSB_OFF		0x404
+#define LPC_LPT_START_OFF	0x408
+
+struct st_lpc {
+	struct clk *clk;
+	void __iomem *iomem_cs;
+};
+
+static struct st_lpc *st_lpc;
+
+static u64 notrace st_lpc_counter_read(void)
+{
+	u64 counter;
+	u32 lower;
+	u32 upper, old_upper;
+
+	upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
+	do {
+		old_upper = upper;
+		lower = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_LSB_OFF);
+		upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
+	} while (upper != old_upper);
+
+	counter = upper;
+	counter <<= 32;
+	counter |= lower;
+	return counter;
+}
+
+static cycle_t st_lpc_clocksource_read(struct clocksource *cs)
+{
+	return st_lpc_counter_read();
+}
+
+static void st_lpc_clocksource_reset(struct clocksource *cs)
+{
+	writel_relaxed(0, st_lpc->iomem_cs + LPC_LPT_START_OFF);
+	writel_relaxed(0, st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
+	writel_relaxed(0, st_lpc->iomem_cs + LPC_LPT_LSB_OFF);
+	writel_relaxed(1, st_lpc->iomem_cs + LPC_LPT_START_OFF);
+}
+
+static struct clocksource st_lpc_clocksource = {
+	.name   = "st-lpc clocksource",
+	.rating = 301,
+	.read   = st_lpc_clocksource_read,
+	.mask   = CLOCKSOURCE_MASK(64),
+	.flags  = CLOCK_SOURCE_IS_CONTINUOUS,
+};
+
+#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK
+static u64 notrace st_lpc_sched_clock_read(void)
+{
+	return st_lpc_counter_read();
+}
+#endif
+
+static void __init st_lpc_clocksource_init(void)
+{
+	unsigned long rate;
+
+	st_lpc_clocksource_reset(&st_lpc_clocksource);
+
+	rate = clk_get_rate(st_lpc->clk);
+#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK
+	sched_clock_register(st_lpc_sched_clock_read, 64, rate);
+#endif
+	clocksource_register_hz(&st_lpc_clocksource, rate);
+
+}
+
+static int st_lpc_setup_clk(struct device_node *np)
+{
+	char *clk_name = "lpc_clk";
+	struct clk *clk;
+	int ret;
+
+	clk = of_clk_get_by_name(np, clk_name);
+	if (IS_ERR(clk)) {
+		pr_err("st-lpc: unable to get lpc clock\n");
+		ret = PTR_ERR(clk);
+		return ret;
+	}
+
+	if (clk_prepare_enable(clk)) {
+		pr_err("st-lpc: %s could not be enabled\n", clk_name);
+		return -EINVAL;
+	}
+
+	if (!clk_get_rate(clk)) {
+		pr_err("st-lpc: Unable to get clock rate\n");
+		clk_disable_unprepare(clk);
+		return -EINVAL;
+	}
+
+	pr_info("st-lpc: %s running @ %lu Hz\n",
+		clk_name, clk_get_rate(clk));
+
+	st_lpc->clk = clk;
+
+	return 0;
+}
+
+static void __init st_lpc_of_register(struct device_node *np)
+{
+	st_lpc = kzalloc(sizeof(*st_lpc), GFP_KERNEL);
+	if (!st_lpc) {
+		pr_err("st-lpc: No memory available\n");
+		return;
+	}
+
+	st_lpc->iomem_cs = of_iomap(np, 0);
+	if (!st_lpc->iomem_cs) {
+		pr_err("st-lpc: Unable to map iomem\n");
+		goto err_kfree;
+	}
+
+	if (st_lpc_setup_clk(np))
+		goto err_iounmap;
+
+	st_lpc_clocksource_init();
+
+	pr_info("st-lpc: clocksource initialised: iomem: %p\n",
+		st_lpc->iomem_cs);
+	return;
+err_iounmap:
+	iounmap(st_lpc->iomem_cs);
+err_kfree:
+	kfree(st_lpc);
+}
+
+CLOCKSOURCE_OF_DECLARE(st_lpc, "st,st_lpc_timer", st_lpc_of_register);
-- 
1.9.1

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

* [PATCH 2/5] clocksource: st_lpc: Add DT bindings documentation for lpc timer
  2015-04-17 10:50 [PATCH 0/5] Add st_lpc clocksource timer driver Peter Griffin
  2015-04-17 10:50 ` [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource Peter Griffin
@ 2015-04-17 10:50 ` Peter Griffin
  2015-04-17 10:50 ` [PATCH 3/5] MAINTAINERS: Add st_lpc.c to ARCH/STI section of maintainers Peter Griffin
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx
  Cc: peter.griffin, lee.jones, devicetree

This patch provides the DT bindings documentation for the lpc
timer found on stih407 fanmily SoCs from STMicroelectronics.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 Documentation/devicetree/bindings/timer/st,lpc-timer.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/st,lpc-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/st,lpc-timer.txt b/Documentation/devicetree/bindings/timer/st,lpc-timer.txt
new file mode 100644
index 0000000..c9ef297
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/st,lpc-timer.txt
@@ -0,0 +1,15 @@
+Binding for LPC Timer on STMicroelectronics STi series SoCs
+
+Required properties:
+- compatible: Should be "st,st_lpc_timer"
+- reg: Base address and length of the LPC Timers registers.
+- clock-names: Set to "lpc_clk".
+- clocks: phandle of the clock used by the LPC Timer
+
+Example:
+	lpc-timer@0x8788000 {
+		compatible = "st,st_lpc_timer";
+		reg = <0x8788000 0x1000>;
+		clock-names = "lpc_clk";
+		clocks = <&clk_s_d3_flexgen CLK_LPC_1>;
+	};
-- 
1.9.1

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

* [PATCH 3/5] MAINTAINERS: Add st_lpc.c to ARCH/STI section of maintainers
  2015-04-17 10:50 [PATCH 0/5] Add st_lpc clocksource timer driver Peter Griffin
  2015-04-17 10:50 ` [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource Peter Griffin
  2015-04-17 10:50 ` [PATCH 2/5] clocksource: st_lpc: Add DT bindings documentation for lpc timer Peter Griffin
@ 2015-04-17 10:50 ` Peter Griffin
       [not found] ` <1429267823-8879-1-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2015-04-17 10:50 ` [PATCH 5/5] ARM: DT: STi: STiH407: Add DT node for st-lpc timer Peter Griffin
  4 siblings, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx
  Cc: peter.griffin, lee.jones, devicetree

This patch adds the new st_lpc timer driver found on stih407
family SoCs into the ARCH/STI section of the maintainers file.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index efbcb50..4fe68dc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1465,6 +1465,7 @@ S:	Maintained
 F:	arch/arm/mach-sti/
 F:	arch/arm/boot/dts/sti*
 F:	drivers/clocksource/arm_global_timer.c
+F:	drivers/clocksource/st_lpc.c
 F:	drivers/i2c/busses/i2c-st.c
 F:	drivers/media/rc/st_rc.c
 F:	drivers/mmc/host/sdhci-st.c
-- 
1.9.1

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

* [PATCH 4/5] ARM: sti: Always enable CLKSRC_ST_LPC_CLOCK
       [not found] ` <1429267823-8879-1-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-04-17 10:50   ` Peter Griffin
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
	maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	tglx-hfZtesqFncYOwBW4kG4KsQ
  Cc: peter.griffin-QSEj5FYQhm4dnm+yROfE0A,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA

If available on the SoC we wish to use st_lpc clksrc
so that cpufreq can operate without effecting sched_clock.

Signed-off-by: Peter Griffin <peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-sti/Kconfig | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-sti/Kconfig b/arch/arm/mach-sti/Kconfig
index 3b1ac46..48767d3 100644
--- a/arch/arm/mach-sti/Kconfig
+++ b/arch/arm/mach-sti/Kconfig
@@ -2,6 +2,7 @@ menuconfig ARCH_STI
 	bool "STMicroelectronics Consumer Electronics SOCs" if ARCH_MULTI_V7
 	select ARM_GIC
 	select ARM_GLOBAL_TIMER
+	select CLKSRC_ST_LPC_CLOCK
 	select PINCTRL
 	select PINCTRL_ST
 	select MFD_SYSCON
@@ -15,9 +16,9 @@ menuconfig ARCH_STI
 	select PL310_ERRATA_769419 if CACHE_L2X0
 	select RESET_CONTROLLER
 	help
-	  Include support for STiH41x SOCs like STiH415/416 using the device tree
-	  for discovery
-	  More information at Documentation/arm/STiH41x and
+	  Include support for STiH4xx SOCs like STiH415/416 and stih407/10 family
+	  using the device tree for discovery
+	  More information at Documentation/arm/sti/ and
 	  at Documentation/devicetree
 
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* [PATCH 5/5] ARM: DT: STi: STiH407: Add DT node for st-lpc timer.
  2015-04-17 10:50 [PATCH 0/5] Add st_lpc clocksource timer driver Peter Griffin
                   ` (3 preceding siblings ...)
       [not found] ` <1429267823-8879-1-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-04-17 10:50 ` Peter Griffin
  4 siblings, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-17 10:50 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx
  Cc: peter.griffin, lee.jones, devicetree

This patch adds the dt node for the st-lpc timer found on
stih407 famliy SoCs. This can then be used as a clocksource.

Signed-off-by: Peter Griffin <peter.griffin@linaro.org>
---
 arch/arm/boot/dts/stih407-family.dtsi | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index c06a546..59c6a22 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -39,6 +39,13 @@
 		reg = <0x08760000 0x1000>;
 	};
 
+	lpc-timer@0x8788000 {
+		compatible = "st,st_lpc_timer";
+		reg = <0x8788000 0x1000>;
+		clock-names = "lpc_clk";
+		clocks = <&clk_s_d3_flexgen CLK_LPC_1>;
+	};
+
 	timer@08760200 {
 		interrupt-parent = <&intc>;
 		compatible = "arm,cortex-a9-global-timer";
-- 
1.9.1

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

* Re: [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
  2015-04-17 10:50 ` [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource Peter Griffin
@ 2015-04-17 21:16   ` Paul Bolle
  2015-04-20  7:14     ` Peter Griffin
       [not found]   ` <1429267823-8879-2-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  1 sibling, 1 reply; 11+ messages in thread
From: Paul Bolle @ 2015-04-17 21:16 UTC (permalink / raw)
  To: Peter Griffin
  Cc: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx, lee.jones,
	devicetree, Ajit Pal Singh

On Fri, 2015-04-17 at 11:50 +0100, Peter Griffin wrote:
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig

> +config CLKSRC_ST_LPC_CLOCK
> +	bool
> +	depends on ARCH_STI
> +	select CLKSRC_OF if OF
> +	help
> +	  Enable this option to use the Low Power controller timer
> +	  as clock source.
> +
> +config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
> +	bool
> +	depends on ST_LPC_CLOCK

It looks like you meant
     depends on CLKSRC_ST_LPC_CLOCK

> +	default y
> +	help
> +	  Use Low Power controller timer clock source as sched_clock

> --- a/drivers/clocksource/Makefile
> +++ b/drivers/clocksource/Makefile

> +obj-$(CONFIG_CLKSRC_ST_LPC_CLOCK)	+= st_lpc.o

> --- /dev/null
> +++ b/drivers/clocksource/st_lpc.c

> +#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK

#ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK here?

> +static u64 notrace st_lpc_sched_clock_read(void)
> +{
> +	return st_lpc_counter_read();
> +}
> +#endif

> +#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK

Again, #ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK here?

> +	sched_clock_register(st_lpc_sched_clock_read, 64, rate);
> +#endif

Assuming the above suggestions are correct: checkkconfigsymbols.py, as
shipped in linux-next, helps detect stuff like this. See
    scripts/checkkconfigsymbols.py --help.

Thanks,


Paul Bolle

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

* Re: [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
  2015-04-17 21:16   ` Paul Bolle
@ 2015-04-20  7:14     ` Peter Griffin
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-04-20  7:14 UTC (permalink / raw)
  To: Paul Bolle
  Cc: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	maxime.coquelin, patrice.chotard, daniel.lezcano, tglx, lee.jones,
	devicetree, Ajit Pal Singh

Hi Paul,

On Fri, 17 Apr 2015, Paul Bolle wrote:
> On Fri, 2015-04-17 at 11:50 +0100, Peter Griffin wrote:
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> 
> > +config CLKSRC_ST_LPC_CLOCK
> > +	bool
> > +	depends on ARCH_STI
> > +	select CLKSRC_OF if OF
> > +	help
> > +	  Enable this option to use the Low Power controller timer
> > +	  as clock source.
> > +
> > +config CLKSRC_ST_LPC_TIMER_SCHED_CLOCK
> > +	bool
> > +	depends on ST_LPC_CLOCK
> 
> It looks like you meant
>      depends on CLKSRC_ST_LPC_CLOCK

Yes your correct, will fix in v2. I did a last minute change to append CLKSRC_,
and seem to have messed it up.

<snip>
> > --- /dev/null
> > +++ b/drivers/clocksource/st_lpc.c
> 
> > +#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK
> 
> #ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK here?
Yes, will fix in v2
> 
> > +static u64 notrace st_lpc_sched_clock_read(void)
> > +{
> > +	return st_lpc_counter_read();
> > +}
> > +#endif
> 
> > +#ifdef CONFIG_CLKSRC_LPC_TIMER_SCHED_CLOCK
> 
> Again, #ifdef CONFIG_CLKSRC_ST_LPC_TIMER_SCHED_CLOCK here?

Yes, will fix in v2
> 
> > +	sched_clock_register(st_lpc_sched_clock_read, 64, rate);
> > +#endif
> 
> Assuming the above suggestions are correct: checkkconfigsymbols.py, as
> shipped in linux-next, helps detect stuff like this. See
>     scripts/checkkconfigsymbols.py --help.

Thanks for the pointer to the script, I'd not heard of that before, so
will take a look.

Thanks for reviewing, 

regards,

Peter.

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

* Re: [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
       [not found]   ` <1429267823-8879-2-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2015-04-21  9:56     ` Thomas Gleixner
  2015-04-29 15:48       ` Maxime Coquelin
  2015-05-02 16:36       ` Peter Griffin
  0 siblings, 2 replies; 11+ messages in thread
From: Thomas Gleixner @ 2015-04-21  9:56 UTC (permalink / raw)
  To: Peter Griffin
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
	maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Ajit Pal Singh

On Fri, 17 Apr 2015, Peter Griffin wrote:
> +/* Low Power Timer */
> +#define LPC_LPT_LSB_OFF		0x400
> +#define LPC_LPT_MSB_OFF		0x404
> +#define LPC_LPT_START_OFF	0x408
> +
> +struct st_lpc {
> +	struct clk *clk;
> +	void __iomem *iomem_cs;
> +};
> +
> +static struct st_lpc *st_lpc;
> +
> +static u64 notrace st_lpc_counter_read(void)
> +{
> +	u64 counter;
> +	u32 lower;
> +	u32 upper, old_upper;
> +
> +	upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
> +	do {
> +		old_upper = upper;
> +		lower = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_LSB_OFF);
> +		upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
> +	} while (upper != old_upper);
> +
> +	counter = upper;
> +	counter <<= 32;
> +	counter |= lower;
> +	return counter;


What's the point of this exercise? The kernel can handle 32bit
clocksources nicely. So why do you want to artificially expand them to
64bit by adding useless loops and hoops to a hotpath?

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
  2015-04-21  9:56     ` Thomas Gleixner
@ 2015-04-29 15:48       ` Maxime Coquelin
  2015-05-02 16:36       ` Peter Griffin
  1 sibling, 0 replies; 11+ messages in thread
From: Maxime Coquelin @ 2015-04-29 15:48 UTC (permalink / raw)
  To: Thomas Gleixner, Peter Griffin
  Cc: linux-arm-kernel, linux-kernel, srinivas.kandagatla,
	patrice.chotard, daniel.lezcano, lee.jones, devicetree,
	Ajit Pal Singh

Hello Thomas,

On 04/21/2015 11:56 AM, Thomas Gleixner wrote:
> On Fri, 17 Apr 2015, Peter Griffin wrote:
>> +/* Low Power Timer */
>> +#define LPC_LPT_LSB_OFF		0x400
>> +#define LPC_LPT_MSB_OFF		0x404
>> +#define LPC_LPT_START_OFF	0x408
>> +
>> +struct st_lpc {
>> +	struct clk *clk;
>> +	void __iomem *iomem_cs;
>> +};
>> +
>> +static struct st_lpc *st_lpc;
>> +
>> +static u64 notrace st_lpc_counter_read(void)
>> +{
>> +	u64 counter;
>> +	u32 lower;
>> +	u32 upper, old_upper;
>> +
>> +	upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
>> +	do {
>> +		old_upper = upper;
>> +		lower = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_LSB_OFF);
>> +		upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
>> +	} while (upper != old_upper);
>> +
>> +	counter = upper;
>> +	counter <<= 32;
>> +	counter |= lower;
>> +	return counter;
>
> What's the point of this exercise? The kernel can handle 32bit
> clocksources nicely. So why do you want to artificially expand them to
> 64bit by adding useless loops and hoops to a hotpath?

I agree we should use only the lower 32 bits.
This timer is moreover clocked at a slow rate (30MHz), so we won't have 
too much interrupts.

Peter, doing that, you could use something like this directly:
clocksource_mmio_init(st_lpc->iomem_cs + LPC_LPT_LSB_OFF, 
"st_lpc_timer", rate,
             200, 24, clocksource_mmio_readl_down);

Thanks,
Maxime

>
> Thanks,
>
> 	tglx

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

* Re: [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource.
  2015-04-21  9:56     ` Thomas Gleixner
  2015-04-29 15:48       ` Maxime Coquelin
@ 2015-05-02 16:36       ` Peter Griffin
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Griffin @ 2015-05-02 16:36 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	srinivas.kandagatla-Re5JQEeQqe8AvxtiuMwx3w,
	maxime.coquelin-qxv4g6HH51o, patrice.chotard-qxv4g6HH51o,
	daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A,
	lee.jones-QSEj5FYQhm4dnm+yROfE0A,
	devicetree-u79uwXL29TY76Z2rM5mHXA, Ajit Pal Singh

Hi Thomas,

On Tue, 21 Apr 2015, Thomas Gleixner wrote:

> On Fri, 17 Apr 2015, Peter Griffin wrote:
> > +/* Low Power Timer */
> > +#define LPC_LPT_LSB_OFF		0x400
> > +#define LPC_LPT_MSB_OFF		0x404
> > +#define LPC_LPT_START_OFF	0x408
> > +
> > +struct st_lpc {
> > +	struct clk *clk;
> > +	void __iomem *iomem_cs;
> > +};
> > +
> > +static struct st_lpc *st_lpc;
> > +
> > +static u64 notrace st_lpc_counter_read(void)
> > +{
> > +	u64 counter;
> > +	u32 lower;
> > +	u32 upper, old_upper;
> > +
> > +	upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
> > +	do {
> > +		old_upper = upper;
> > +		lower = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_LSB_OFF);
> > +		upper = readl_relaxed(st_lpc->iomem_cs + LPC_LPT_MSB_OFF);
> > +	} while (upper != old_upper);
> > +
> > +	counter = upper;
> > +	counter <<= 32;
> > +	counter |= lower;
> > +	return counter;
> 
> 
> What's the point of this exercise? The kernel can handle 32bit
> clocksources nicely. So why do you want to artificially expand them to
> 64bit by adding useless loops and hoops to a hotpath?

Thanks for reviewing, yes your correct, we don't really need to be
doing this. We will fix this in the v2 version.

Lee Jones has also pointed out something which I was not aware of when
submitting this, and that is this IP has shared registers with the RTC
driver.

He has been working on some patches here https://lkml.org/lkml/2015/4/9/609
which deal with configuring between watchdog and rtc and ensuring only one
can be configured at a time, and this driver should also be making use of
that mechanism.

As he is already working on this, we decided it would be best if he takes
over for the V2 submission, and aligns it with the work he has already done
for this IP.

regards,

Peter.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

end of thread, other threads:[~2015-05-02 16:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-04-17 10:50 [PATCH 0/5] Add st_lpc clocksource timer driver Peter Griffin
2015-04-17 10:50 ` [PATCH 1/5] clocksource: st_lpc: Add LPC timer as a clocksource Peter Griffin
2015-04-17 21:16   ` Paul Bolle
2015-04-20  7:14     ` Peter Griffin
     [not found]   ` <1429267823-8879-2-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-04-21  9:56     ` Thomas Gleixner
2015-04-29 15:48       ` Maxime Coquelin
2015-05-02 16:36       ` Peter Griffin
2015-04-17 10:50 ` [PATCH 2/5] clocksource: st_lpc: Add DT bindings documentation for lpc timer Peter Griffin
2015-04-17 10:50 ` [PATCH 3/5] MAINTAINERS: Add st_lpc.c to ARCH/STI section of maintainers Peter Griffin
     [not found] ` <1429267823-8879-1-git-send-email-peter.griffin-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2015-04-17 10:50   ` [PATCH 4/5] ARM: sti: Always enable CLKSRC_ST_LPC_CLOCK Peter Griffin
2015-04-17 10:50 ` [PATCH 5/5] ARM: DT: STi: STiH407: Add DT node for st-lpc timer Peter Griffin

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).