devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] clockevent: add low power STM32 timer
@ 2020-01-09 14:53 Benjamin Gaignard
  2020-01-09 14:53 ` [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings Benjamin Gaignard
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-09 14:53 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, linux, p.paillet
  Cc: devicetree, linux-stm32, linux-arm-kernel, Benjamin Gaignard

This series add low power timer as boadcast clockevent device.
Low power timer could runs even when CPUs are in idle mode and 
could wakeup them.

Benjamin Gaignard (3):
  dt-bindings: timer: Add STM32 Low Power Timer bindings
  clocksource: Add Low Power STM32 timers driver
  ARM: stm32: select STM32 low power timer clock event driver

 .../bindings/timer/st,stm32-lp-timer.yaml          |  44 +++++
 arch/arm/mach-stm32/Kconfig                        |   1 +
 drivers/clocksource/Kconfig                        |   5 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-stm32-lp.c               | 186 +++++++++++++++++++++
 5 files changed, 237 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
 create mode 100644 drivers/clocksource/timer-stm32-lp.c

-- 
2.15.0


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

* [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings
  2020-01-09 14:53 [PATCH 0/3] clockevent: add low power STM32 timer Benjamin Gaignard
@ 2020-01-09 14:53 ` Benjamin Gaignard
  2020-01-15 14:35   ` Rob Herring
  2020-01-09 14:53 ` [PATCH 2/3] clocksource: Add Low Power STM32 timers driver Benjamin Gaignard
  2020-01-09 14:53 ` [PATCH 3/3] ARM: stm32: select STM32 low power timer clock event driver Benjamin Gaignard
  2 siblings, 1 reply; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-09 14:53 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, linux, p.paillet
  Cc: devicetree, linux-stm32, linux-arm-kernel, Benjamin Gaignard

Document STM32 Low Power bindings.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 .../bindings/timer/st,stm32-lp-timer.yaml          | 44 ++++++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml

diff --git a/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
new file mode 100644
index 000000000000..ca040b96dc47
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/timer/st,stm32-lp-timer.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: STMicroelectronics STM32 Low Power 16 bits timers bindings
+
+maintainers:
+  - Benjamin Gaignard <benjamin.gaignard@st.com>
+
+properties:
+  compatible:
+    const: st,stm32-lptimer-clkevent
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/stm32mp1-clks.h>
+    clkevent: clkevent@40009000 {
+      compatible = "st,stm32-lptimer-clkevent";
+      reg = <0x40009000 0x400>;
+      clocks = <&rcc LPTIM1_K>;
+      interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
+    };
+
+...
-- 
2.15.0


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

* [PATCH 2/3] clocksource: Add Low Power STM32 timers driver
  2020-01-09 14:53 [PATCH 0/3] clockevent: add low power STM32 timer Benjamin Gaignard
  2020-01-09 14:53 ` [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings Benjamin Gaignard
@ 2020-01-09 14:53 ` Benjamin Gaignard
  2020-01-16  9:06   ` Linus Walleij
  2020-01-09 14:53 ` [PATCH 3/3] ARM: stm32: select STM32 low power timer clock event driver Benjamin Gaignard
  2 siblings, 1 reply; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-09 14:53 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, linux, p.paillet
  Cc: devicetree, linux-stm32, linux-arm-kernel, Benjamin Gaignard

Implement clock event driver using low power STM32 timers.
Low power timers counter s running even in when CPU is in stop mode.
It could be used as clock event broadcaster to wake up CPUs but not like
a clocksource because each it rise an interrupt the counter restart from 0.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
Signed-off-by: Pascal Paillet <p.paillet@st.com>
---
 drivers/clocksource/Kconfig          |   5 +
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-stm32-lp.c | 186 +++++++++++++++++++++++++++++++++++
 3 files changed, 192 insertions(+)
 create mode 100644 drivers/clocksource/timer-stm32-lp.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index 5fdd76cb1768..b96c13626fcc 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -292,6 +292,11 @@ config CLKSRC_STM32
 	select CLKSRC_MMIO
 	select TIMER_OF
 
+config CLKSRC_STM32_LP
+	bool "Low power clocksource for STM32 SoCs"
+	depends on OF && ARM && (MACH_STM32MP157 || COMPILE_TEST)
+	select TIMER_OF
+
 config CLKSRC_MPS2
 	bool "Clocksource for MPS2 SoCs" if COMPILE_TEST
 	depends on GENERIC_SCHED_CLOCK
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index 4dfe4225ece7..c6eef37be9cc 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_BCM_KONA_TIMER)	+= bcm_kona_timer.o
 obj-$(CONFIG_CADENCE_TTC_TIMER)	+= timer-cadence-ttc.o
 obj-$(CONFIG_CLKSRC_EFM32)	+= timer-efm32.o
 obj-$(CONFIG_CLKSRC_STM32)	+= timer-stm32.o
+obj-$(CONFIG_CLKSRC_STM32_LP)	+= timer-stm32-lp.o
 obj-$(CONFIG_CLKSRC_EXYNOS_MCT)	+= exynos_mct.o
 obj-$(CONFIG_CLKSRC_LPC32XX)	+= timer-lpc32xx.o
 obj-$(CONFIG_CLKSRC_MPS2)	+= mps2-timer.o
diff --git a/drivers/clocksource/timer-stm32-lp.c b/drivers/clocksource/timer-stm32-lp.c
new file mode 100644
index 000000000000..a58038157272
--- /dev/null
+++ b/drivers/clocksource/timer-stm32-lp.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) STMicroelectronics 2019 - All Rights Reserved
+ * Authors: Benjamin Gaignard <benjamin.gaignard@st.com> for STMicroelectronics.
+ *	    Pascal Paillet <p.paillet@st.com> for STMicroelectronics.
+ */
+
+#include <linux/clk.h>
+#include <linux/clocksource.h>
+#include <linux/clockchips.h>
+#include <linux/delay.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+
+#include "timer-of.h"
+
+#define LPTIM_ICR		0x04
+#define ICR_ARRMCF		BIT(1)
+
+#define LPTIM_IER		0x08
+#define IER_ARRMIE		BIT(1)
+
+#define LPTIM_CFGR		0x0C
+#define CFGR_PSC_OFFSET		9
+
+#define LPTIM_CR		0x10
+#define CR_ENABLE		BIT(0)
+#define CR_SNGSTRT		BIT(1)
+#define CR_CNTSTRT		BIT(2)
+
+#define LPTIM_CMP		0x14
+#define LPTIM_ARR		0x18
+
+#define STM32_LP_BITS		16
+#define STM32_LP_RATING		400
+#define STM32_CLKRATE		(32000 * HZ)
+#define TIMER_MAX_VAL		(BIT(STM32_LP_BITS) - 1)
+#define STM32_LP_MAX_PSC	7
+
+static int stm32_lp_clock_event_shutdown(struct clock_event_device *clkevt)
+{
+	struct timer_of *to = to_timer_of(clkevt);
+
+	writel_relaxed(0, timer_of_base(to) + LPTIM_CR);
+	writel_relaxed(0, timer_of_base(to) + LPTIM_IER);
+	/* clear pending flags */
+	writel_relaxed(ICR_ARRMCF, timer_of_base(to) + LPTIM_ICR);
+
+	return 0;
+}
+
+static int stm32_lp_set_timer(unsigned long evt,
+			       struct clock_event_device *clkevt,
+			       int is_periodic)
+{
+	struct timer_of *to = to_timer_of(clkevt);
+
+	/* disable TIMER */
+	writel_relaxed(0, timer_of_base(to) + LPTIM_CR);
+	/* reset counters */
+	writel_relaxed(0, timer_of_base(to) + LPTIM_CMP);
+	writel_relaxed(0, timer_of_base(to) + LPTIM_ARR);
+	/* enable ARR interrupt */
+	writel_relaxed(IER_ARRMIE, timer_of_base(to) + LPTIM_IER);
+	/* enable LPTIMER*/
+	writel_relaxed(CR_ENABLE, timer_of_base(to) + LPTIM_CR);
+
+	/* set nex event counter */
+	writel_relaxed(evt, timer_of_base(to) + LPTIM_ARR);
+
+	/* start counter */
+	if (is_periodic)
+		writel_relaxed(CR_CNTSTRT | CR_ENABLE, timer_of_base(to) +
+			       LPTIM_CR);
+	else
+		writel_relaxed(CR_SNGSTRT | CR_ENABLE, timer_of_base(to) +
+			       LPTIM_CR);
+
+	return 0;
+}
+
+static int stm32_lp_set_next_event(unsigned long evt,
+				   struct clock_event_device *clkevt)
+{
+	return stm32_lp_set_timer(evt, clkevt,
+				  clockevent_state_periodic(clkevt));
+}
+
+static int stm32_lp_clock_event_set_periodic(struct clock_event_device *clkevt)
+{
+	struct timer_of *to = to_timer_of(clkevt);
+
+	return stm32_lp_set_timer(timer_of_period(to), clkevt, true);
+}
+
+static int stm32_lp_clock_event_set_oneshot(struct clock_event_device *clkevt)
+{
+	struct timer_of *to = to_timer_of(clkevt);
+
+	return stm32_lp_set_timer(timer_of_period(to), clkevt, false);
+}
+
+static irqreturn_t stm32_lp_clock_event_handler(int irq, void *dev_id)
+{
+	struct clock_event_device *clkevt = (struct clock_event_device *)dev_id;
+	struct timer_of *to = to_timer_of(clkevt);
+
+	writel_relaxed(ICR_ARRMCF, timer_of_base(to) + LPTIM_ICR);
+
+	clkevt->event_handler(clkevt);
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * stm32_lptimer_set_prescaler - Compute and set the prescaler register
+ * @to: a pointer to a timer-of structure
+ *
+ * Compute the prescaler to always target a 32Khz timer rate
+ */
+static void __init stm32_lptimer_set_prescaler(struct timer_of *to)
+{
+	int i;
+
+	for (i = 0; i <= STM32_LP_MAX_PSC; i++) {
+		if (DIV_ROUND_CLOSEST(to->of_clk.rate, 1 << i) < STM32_CLKRATE)
+			break;
+	}
+
+	writel_relaxed(i << CFGR_PSC_OFFSET, timer_of_base(to) + LPTIM_CFGR);
+
+	/* Adjust rate and period given the prescaler value */
+	to->of_clk.rate = DIV_ROUND_CLOSEST(to->of_clk.rate, (1 << i));
+	to->of_clk.period = DIV_ROUND_UP(to->of_clk.rate, HZ);
+}
+
+static void __init stm32_lp_clockevent_init(struct timer_of *to)
+{
+	to->clkevt.name = to->np->full_name;
+	to->clkevt.cpumask = cpumask_of(smp_processor_id());
+	to->clkevt.features = CLOCK_EVT_FEAT_PERIODIC | CLOCK_EVT_FEAT_ONESHOT;
+	to->clkevt.set_state_shutdown = stm32_lp_clock_event_shutdown;
+	to->clkevt.set_state_periodic = stm32_lp_clock_event_set_periodic;
+	to->clkevt.set_state_oneshot = stm32_lp_clock_event_set_oneshot;
+	to->clkevt.set_next_event = stm32_lp_set_next_event;
+	to->clkevt.rating = STM32_LP_RATING;
+
+	clockevents_config_and_register(&to->clkevt, timer_of_rate(to), 0x1,
+					TIMER_MAX_VAL);
+
+	pr_info("%pOF: STM32 low power clockevent driver initialized\n",
+		to->np);
+}
+
+static int __init stm32_lptimer_init(struct device_node *node)
+{
+	struct timer_of *to;
+	int ret;
+
+	to = kzalloc(sizeof(*to), GFP_KERNEL);
+	if (!to)
+		return -ENOMEM;
+
+	to->flags = TIMER_OF_IRQ | TIMER_OF_CLOCK | TIMER_OF_BASE;
+	to->of_irq.handler = stm32_lp_clock_event_handler;
+
+	ret = timer_of_init(node, to);
+	if (ret) {
+		kfree(to);
+		return ret;
+	}
+
+	stm32_lptimer_set_prescaler(to);
+
+	stm32_lp_clockevent_init(to);
+
+	return 0;
+}
+
+TIMER_OF_DECLARE(stm32_lp, "st,stm32-lptimer-clkevent", stm32_lptimer_init);
-- 
2.15.0


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

* [PATCH 3/3] ARM: stm32: select STM32 low power timer clock event driver
  2020-01-09 14:53 [PATCH 0/3] clockevent: add low power STM32 timer Benjamin Gaignard
  2020-01-09 14:53 ` [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings Benjamin Gaignard
  2020-01-09 14:53 ` [PATCH 2/3] clocksource: Add Low Power STM32 timers driver Benjamin Gaignard
@ 2020-01-09 14:53 ` Benjamin Gaignard
  2 siblings, 0 replies; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-09 14:53 UTC (permalink / raw)
  To: daniel.lezcano, tglx, robh+dt, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, linux, p.paillet
  Cc: devicetree, linux-stm32, linux-arm-kernel, Benjamin Gaignard

Select STM32 low power clock event driver by default

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
 arch/arm/mach-stm32/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/mach-stm32/Kconfig b/arch/arm/mach-stm32/Kconfig
index 57699bd8f107..441fc6923f7f 100644
--- a/arch/arm/mach-stm32/Kconfig
+++ b/arch/arm/mach-stm32/Kconfig
@@ -9,6 +9,7 @@ menuconfig ARCH_STM32
 	select ARM_AMBA
 	select ARCH_HAS_RESET_CONTROLLER
 	select CLKSRC_STM32
+	select CLKSRC_STM32_LP
 	select PINCTRL
 	select RESET_CONTROLLER
 	select STM32_EXTI
-- 
2.15.0


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

* Re: [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings
  2020-01-09 14:53 ` [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings Benjamin Gaignard
@ 2020-01-15 14:35   ` Rob Herring
  2020-01-15 14:45     ` Benjamin Gaignard
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-01-15 14:35 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: daniel.lezcano, tglx, mark.rutland, mcoquelin.stm32,
	alexandre.torgue, linux, p.paillet, devicetree, linux-stm32,
	linux-arm-kernel

On Thu, Jan 09, 2020 at 03:53:31PM +0100, Benjamin Gaignard wrote:
> Document STM32 Low Power bindings.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
>  .../bindings/timer/st,stm32-lp-timer.yaml          | 44 ++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> 
> diff --git a/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> new file mode 100644
> index 000000000000..ca040b96dc47
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml

Use the compatible for the filename.

> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/timer/st,stm32-lp-timer.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: STMicroelectronics STM32 Low Power 16 bits timers bindings
> +
> +maintainers:
> +  - Benjamin Gaignard <benjamin.gaignard@st.com>
> +
> +properties:
> +  compatible:
> +    const: st,stm32-lptimer-clkevent

'clkevent' is a h/w name? Seems redundant and abusing compatible to 
bind to a specific Linux driver. 

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/stm32mp1-clks.h>
> +    clkevent: clkevent@40009000 {

timer@...

> +      compatible = "st,stm32-lptimer-clkevent";
> +      reg = <0x40009000 0x400>;
> +      clocks = <&rcc LPTIM1_K>;
> +      interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
> +    };
> +
> +...
> -- 
> 2.15.0
> 

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

* Re: [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings
  2020-01-15 14:35   ` Rob Herring
@ 2020-01-15 14:45     ` Benjamin Gaignard
  2020-01-15 19:00       ` Rob Herring
  0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-15 14:45 UTC (permalink / raw)
  To: Rob Herring
  Cc: Benjamin Gaignard, Mark Rutland, devicetree, Alexandre Torgue,
	Daniel Lezcano, Russell King - ARM Linux, pascal paillet,
	Maxime Coquelin, Thomas Gleixner, linux-stm32, Linux ARM

Le mer. 15 janv. 2020 à 15:35, Rob Herring <robh@kernel.org> a écrit :
>
> On Thu, Jan 09, 2020 at 03:53:31PM +0100, Benjamin Gaignard wrote:
> > Document STM32 Low Power bindings.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > ---
> >  .../bindings/timer/st,stm32-lp-timer.yaml          | 44 ++++++++++++++++++++++
> >  1 file changed, 44 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > new file mode 100644
> > index 000000000000..ca040b96dc47
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
>
> Use the compatible for the filename.

it will be in v2

>
> > @@ -0,0 +1,44 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/timer/st,stm32-lp-timer.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: STMicroelectronics STM32 Low Power 16 bits timers bindings
> > +
> > +maintainers:
> > +  - Benjamin Gaignard <benjamin.gaignard@st.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: st,stm32-lptimer-clkevent
>
> 'clkevent' is a h/w name? Seems redundant and abusing compatible to
> bind to a specific Linux driver.

No but st,stm32-lptimer compatible is already used for another driver
The hardware block can implement multiple features but not all at the same time
so I try to distinguish them with the compatible.
In this particular case I would like tag it as a clock event driver.

>
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> > +    #include <dt-bindings/clock/stm32mp1-clks.h>
> > +    clkevent: clkevent@40009000 {
>
> timer@...

OK

>
> > +      compatible = "st,stm32-lptimer-clkevent";
> > +      reg = <0x40009000 0x400>;
> > +      clocks = <&rcc LPTIM1_K>;
> > +      interrupts = <GIC_SPI 93 IRQ_TYPE_LEVEL_HIGH>;
> > +    };
> > +
> > +...
> > --
> > 2.15.0
> >
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings
  2020-01-15 14:45     ` Benjamin Gaignard
@ 2020-01-15 19:00       ` Rob Herring
  2020-01-15 19:17         ` Benjamin Gaignard
  0 siblings, 1 reply; 11+ messages in thread
From: Rob Herring @ 2020-01-15 19:00 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Benjamin Gaignard, Mark Rutland, devicetree, Alexandre Torgue,
	Daniel Lezcano, Russell King - ARM Linux, pascal paillet,
	Maxime Coquelin, Thomas Gleixner, linux-stm32, Linux ARM

On Wed, Jan 15, 2020 at 8:46 AM Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
>
> Le mer. 15 janv. 2020 à 15:35, Rob Herring <robh@kernel.org> a écrit :
> >
> > On Thu, Jan 09, 2020 at 03:53:31PM +0100, Benjamin Gaignard wrote:
> > > Document STM32 Low Power bindings.
> > >
> > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > ---
> > >  .../bindings/timer/st,stm32-lp-timer.yaml          | 44 ++++++++++++++++++++++
> > >  1 file changed, 44 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > > new file mode 100644
> > > index 000000000000..ca040b96dc47
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> >
> > Use the compatible for the filename.
>
> it will be in v2
>
> >
> > > @@ -0,0 +1,44 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/timer/st,stm32-lp-timer.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: STMicroelectronics STM32 Low Power 16 bits timers bindings
> > > +
> > > +maintainers:
> > > +  - Benjamin Gaignard <benjamin.gaignard@st.com>
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: st,stm32-lptimer-clkevent
> >
> > 'clkevent' is a h/w name? Seems redundant and abusing compatible to
> > bind to a specific Linux driver.
>
> No but st,stm32-lptimer compatible is already used for another driver
> The hardware block can implement multiple features but not all at the same time
> so I try to distinguish them with the compatible.
> In this particular case I would like tag it as a clock event driver.

That's a Linux specific thing which we've said no to for 10 years.

Is "Not at the same time" a chip design time configuration or run-time
config. If the latter, why do you want to use a particular instance
over another one for clock event? There has to be some h/w difference.
Describe the difference and then use that to grab the device to use
for a clockevent. I'm fine if you omit the pwm node and then use that
to decide which instance to use.

Rob

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

* Re: [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings
  2020-01-15 19:00       ` Rob Herring
@ 2020-01-15 19:17         ` Benjamin Gaignard
  0 siblings, 0 replies; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-15 19:17 UTC (permalink / raw)
  To: Rob Herring
  Cc: Benjamin Gaignard, Mark Rutland, devicetree, Alexandre Torgue,
	Daniel Lezcano, Russell King - ARM Linux, pascal paillet,
	Maxime Coquelin, Thomas Gleixner, linux-stm32, Linux ARM

Le mer. 15 janv. 2020 à 20:00, Rob Herring <robh@kernel.org> a écrit :
>
> On Wed, Jan 15, 2020 at 8:46 AM Benjamin Gaignard
> <benjamin.gaignard@linaro.org> wrote:
> >
> > Le mer. 15 janv. 2020 à 15:35, Rob Herring <robh@kernel.org> a écrit :
> > >
> > > On Thu, Jan 09, 2020 at 03:53:31PM +0100, Benjamin Gaignard wrote:
> > > > Document STM32 Low Power bindings.
> > > >
> > > > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > ---
> > > >  .../bindings/timer/st,stm32-lp-timer.yaml          | 44 ++++++++++++++++++++++
> > > >  1 file changed, 44 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > > >
> > > > diff --git a/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > > > new file mode 100644
> > > > index 000000000000..ca040b96dc47
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/timer/st,stm32-lp-timer.yaml
> > >
> > > Use the compatible for the filename.
> >
> > it will be in v2
> >
> > >
> > > > @@ -0,0 +1,44 @@
> > > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > > +%YAML 1.2
> > > > +---
> > > > +$id: http://devicetree.org/schemas/timer/st,stm32-lp-timer.yaml#
> > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > +
> > > > +title: STMicroelectronics STM32 Low Power 16 bits timers bindings
> > > > +
> > > > +maintainers:
> > > > +  - Benjamin Gaignard <benjamin.gaignard@st.com>
> > > > +
> > > > +properties:
> > > > +  compatible:
> > > > +    const: st,stm32-lptimer-clkevent
> > >
> > > 'clkevent' is a h/w name? Seems redundant and abusing compatible to
> > > bind to a specific Linux driver.
> >
> > No but st,stm32-lptimer compatible is already used for another driver
> > The hardware block can implement multiple features but not all at the same time
> > so I try to distinguish them with the compatible.
> > In this particular case I would like tag it as a clock event driver.
>
> That's a Linux specific thing which we've said no to for 10 years.
>
> Is "Not at the same time" a chip design time configuration or run-time
> config. If the latter, why do you want to use a particular instance
> over another one for clock event? There has to be some h/w difference.
> Describe the difference and then use that to grab the device to use
> for a clockevent. I'm fine if you omit the pwm node and then use that
> to decide which instance to use.
>

There is no hardware difference between the devices but we can't do
pwm and clockevent
at the same time. We use the same hardware for two exclusives
features. In addition
we want to be able to use clockevent on one device and pwm on one another.
Could "st,stm32-low-power-timer" compatible be a solution ?

Benjamin

> Rob

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

* Re: [PATCH 2/3] clocksource: Add Low Power STM32 timers driver
  2020-01-09 14:53 ` [PATCH 2/3] clocksource: Add Low Power STM32 timers driver Benjamin Gaignard
@ 2020-01-16  9:06   ` Linus Walleij
  2020-01-16 15:44     ` Benjamin Gaignard
  0 siblings, 1 reply; 11+ messages in thread
From: Linus Walleij @ 2020-01-16  9:06 UTC (permalink / raw)
  To: Benjamin Gaignard, Baolin Wang
  Cc: Daniel Lezcano, Thomas Gleixner, Rob Herring, Mark Rutland,
	Maxime Coquelin, Alexandre TORGUE, Russell King,
	Pascal PAILLET-LME,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	linux-stm32, Linux ARM

On Thu, Jan 9, 2020 at 3:54 PM Benjamin Gaignard
<benjamin.gaignard@st.com> wrote:

> Implement clock event driver using low power STM32 timers.
> Low power timers counter s running even in when CPU is in stop mode.
> It could be used as clock event broadcaster to wake up CPUs but not like
> a clocksource because each it rise an interrupt the counter restart from 0.
>
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> Signed-off-by: Pascal Paillet <p.paillet@st.com>

Looks good to me:
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

If you have a spare always-on timer (and it looks like you have) which
you can set as free-running, you could register it with
CLOCK_SOURCE_SUSPEND_NONSTOP so it
keeps the system clock ticking also during suspend as
alternative clock source.

Yours,
Linus Walleij

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

* Re: [PATCH 2/3] clocksource: Add Low Power STM32 timers driver
  2020-01-16  9:06   ` Linus Walleij
@ 2020-01-16 15:44     ` Benjamin Gaignard
  2020-01-16 15:53       ` Linus Walleij
  0 siblings, 1 reply; 11+ messages in thread
From: Benjamin Gaignard @ 2020-01-16 15:44 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Benjamin Gaignard, Baolin Wang, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Alexandre TORGUE, Daniel Lezcano, Russell King,
	Pascal PAILLET-LME, Rob Herring, Maxime Coquelin,
	Thomas Gleixner, linux-stm32, Linux ARM

Le jeu. 16 janv. 2020 à 10:07, Linus Walleij
<linus.walleij@linaro.org> a écrit :
>
> On Thu, Jan 9, 2020 at 3:54 PM Benjamin Gaignard
> <benjamin.gaignard@st.com> wrote:
>
> > Implement clock event driver using low power STM32 timers.
> > Low power timers counter s running even in when CPU is in stop mode.
> > It could be used as clock event broadcaster to wake up CPUs but not like
> > a clocksource because each it rise an interrupt the counter restart from 0.
> >
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> > Signed-off-by: Pascal Paillet <p.paillet@st.com>
>
> Looks good to me:
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Hi Linus,

Thanks for your review. I had to rework a bit the driver to solve the
bindings issues
so I haven't put your RB on version 2.

>
> If you have a spare always-on timer (and it looks like you have) which
> you can set as free-running, you could register it with
> CLOCK_SOURCE_SUSPEND_NONSTOP so it

The driver only implement clock event feature so I don't think that is
flag is applicable.

Regards,
Benjamin
> keeps the system clock ticking also during suspend as
> alternative clock source.
>
> Yours,
> Linus Walleij
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH 2/3] clocksource: Add Low Power STM32 timers driver
  2020-01-16 15:44     ` Benjamin Gaignard
@ 2020-01-16 15:53       ` Linus Walleij
  0 siblings, 0 replies; 11+ messages in thread
From: Linus Walleij @ 2020-01-16 15:53 UTC (permalink / raw)
  To: Benjamin Gaignard
  Cc: Benjamin Gaignard, Baolin Wang, Mark Rutland,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
	Alexandre TORGUE, Daniel Lezcano, Russell King,
	Pascal PAILLET-LME, Rob Herring, Maxime Coquelin,
	Thomas Gleixner, linux-stm32, Linux ARM

On Thu, Jan 16, 2020 at 4:44 PM Benjamin Gaignard
<benjamin.gaignard@linaro.org> wrote:
> Le jeu. 16 janv. 2020 à 10:07, Linus Walleij
> <linus.walleij@linaro.org> a écrit :

> > If you have a spare always-on timer (and it looks like you have) which
> > you can set as free-running, you could register it with
> > CLOCK_SOURCE_SUSPEND_NONSTOP so it
>
> The driver only implement clock event feature so I don't think that is
> flag is applicable.

I know, it was a suggested future feature, if you have a spare
timer.

Thanks,
Linus Walleij

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

end of thread, other threads:[~2020-01-16 15:53 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-09 14:53 [PATCH 0/3] clockevent: add low power STM32 timer Benjamin Gaignard
2020-01-09 14:53 ` [PATCH 1/3] dt-bindings: timer: Add STM32 Low Power Timer bindings Benjamin Gaignard
2020-01-15 14:35   ` Rob Herring
2020-01-15 14:45     ` Benjamin Gaignard
2020-01-15 19:00       ` Rob Herring
2020-01-15 19:17         ` Benjamin Gaignard
2020-01-09 14:53 ` [PATCH 2/3] clocksource: Add Low Power STM32 timers driver Benjamin Gaignard
2020-01-16  9:06   ` Linus Walleij
2020-01-16 15:44     ` Benjamin Gaignard
2020-01-16 15:53       ` Linus Walleij
2020-01-09 14:53 ` [PATCH 3/3] ARM: stm32: select STM32 low power timer clock event driver Benjamin Gaignard

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