[PATCH v4 0/3] clockevent: add low power STM32 timer

[PATCH v4 0/3] clockevent: add low power STM32 timer

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

version 4:
- move defines in mfd/stm32-lptimer.h
- change compatible and subnode names
- document wakeup-source property
- reword commit message
- make driver Kconfig depends of MFD_STM32_LPTIMER
- remove useless include
- remove rate and clk fields from the private structure
- to add comments about the registers sequence in stm32_clkevent_lp_set_timer
- rework probe function and use devm_request_irq()
- do not allow module to be removed

version 3:
- fix timer set sequence
- don't forget to free irq on remove function
- use devm_kzalloc to simplify errors handling in probe function

version 2:
- stm32 clkevent driver is now a child of the stm32 lp timer node
- add a probe function and adpat the driver to use regmap provide
  by it parent
- stop using timer_of helpers


Benjamin Gaignard (3):
  dt-bindings: mfd: Document STM32 low power timer bindings
  mfd: stm32: Add defines to be used for clkevent purpose
  clocksource: Add Low Power STM32 timers driver

 .../devicetree/bindings/mfd/st,stm32-lptimer.yaml  |  16 ++
 drivers/clocksource/Kconfig                        |   7 +
 drivers/clocksource/Makefile                       |   1 +
 drivers/clocksource/timer-stm32-lp.c               | 213 +++++++++++++++++++++
 include/linux/mfd/stm32-lptimer.h                  |   5 +
 5 files changed, 242 insertions(+)
 create mode 100644 drivers/clocksource/timer-stm32-lp.c

-- 
2.15.0

[PATCH v4 1/3] dt-bindings: mfd: Document STM32 low power timer bindings

[Benjamin Gaignard]

Add a subnode to STM low power timer bindings to support timer driver

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 4:
- change compatible and subnode names
- document wakeup-source property

 .../devicetree/bindings/mfd/st,stm32-lptimer.yaml        | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
index 1a4cc5f3fb33..ddf190cb800b 100644
--- a/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
+++ b/Documentation/devicetree/bindings/mfd/st,stm32-lptimer.yaml
@@ -39,6 +39,8 @@ properties:
   "#size-cells":
     const: 0
 
+  wakeup-source: true
+
   pwm:
     type: object
 
@@ -81,6 +83,16 @@ patternProperties:
     required:
       - compatible
 
+  timer:
+    type: object
+
+    properties:
+      compatible:
+        const: st,stm32-lptimer-timer
+
+    required:
+      - compatible
+
 required:
   - "#address-cells"
   - "#size-cells"
@@ -115,6 +127,10 @@ examples:
       counter {
         compatible = "st,stm32-lptimer-counter";
       };
+
+      timer {
+        compatible = "st,stm32-lptimer-timer";
+      };
     };
 
 ...
-- 
2.15.0

Re: [PATCH v4 1/3] dt-bindings: mfd: Document STM32 low power timer bindings

[Rob Herring]

On Mon, 17 Feb 2020 14:45:44 +0100, Benjamin Gaignard wrote:
> Add a subnode to STM low power timer bindings to support timer driver
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> ---
> version 4:
> - change compatible and subnode names
> - document wakeup-source property
> 
>  .../devicetree/bindings/mfd/st,stm32-lptimer.yaml        | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>

[PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Benjamin Gaignard]

Add defines to be able to enable/clear irq and configure one shot mode.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
---
version 4:
- move defines in mfd/stm32-lptimer.h

 include/linux/mfd/stm32-lptimer.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/mfd/stm32-lptimer.h b/include/linux/mfd/stm32-lptimer.h
index 605f62264825..90b20550c1c8 100644
--- a/include/linux/mfd/stm32-lptimer.h
+++ b/include/linux/mfd/stm32-lptimer.h
@@ -27,10 +27,15 @@
 #define STM32_LPTIM_CMPOK		BIT(3)
 
 /* STM32_LPTIM_ICR - bit fields */
+#define STM32_LPTIM_ARRMCF		BIT(1)
 #define STM32_LPTIM_CMPOKCF_ARROKCF	GENMASK(4, 3)
 
+/* STM32_LPTIM_IER - bit flieds */
+#define STM32_LPTIM_ARRMIE	BIT(1)
+
 /* STM32_LPTIM_CR - bit fields */
 #define STM32_LPTIM_CNTSTRT	BIT(2)
+#define STM32_LPTIM_SNGSTRT	BIT(1)
 #define STM32_LPTIM_ENABLE	BIT(0)
 
 /* STM32_LPTIM_CFGR - bit fields */
-- 
2.15.0

Re: [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Daniel Lezcano]

Hi Lee,

On 17/02/2020 14:45, Benjamin Gaignard wrote:
> Add defines to be able to enable/clear irq and configure one shot mode.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>

Are you fine if I pick this patch with the series?

> ---
> version 4:
> - move defines in mfd/stm32-lptimer.h
> 
>  include/linux/mfd/stm32-lptimer.h | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/linux/mfd/stm32-lptimer.h b/include/linux/mfd/stm32-lptimer.h
> index 605f62264825..90b20550c1c8 100644
> --- a/include/linux/mfd/stm32-lptimer.h
> +++ b/include/linux/mfd/stm32-lptimer.h
> @@ -27,10 +27,15 @@
>  #define STM32_LPTIM_CMPOK		BIT(3)
>  
>  /* STM32_LPTIM_ICR - bit fields */
> +#define STM32_LPTIM_ARRMCF		BIT(1)
>  #define STM32_LPTIM_CMPOKCF_ARROKCF	GENMASK(4, 3)
>  
> +/* STM32_LPTIM_IER - bit flieds */
> +#define STM32_LPTIM_ARRMIE	BIT(1)
> +
>  /* STM32_LPTIM_CR - bit fields */
>  #define STM32_LPTIM_CNTSTRT	BIT(2)
> +#define STM32_LPTIM_SNGSTRT	BIT(1)
>  #define STM32_LPTIM_ENABLE	BIT(0)
>  
>  /* STM32_LPTIM_CFGR - bit fields */
> 


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

Re: [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Lee Jones]

On Thu, 20 Feb 2020, Daniel Lezcano wrote:

> 
> Hi Lee,
> 
> On 17/02/2020 14:45, Benjamin Gaignard wrote:
> > Add defines to be able to enable/clear irq and configure one shot mode.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> 
> Are you fine if I pick this patch with the series?

Acked-by: Lee Jones <lee.jones@linaro.org>

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Lee Jones]

On Thu, 20 Feb 2020, Daniel Lezcano wrote:
> On 17/02/2020 14:45, Benjamin Gaignard wrote:
> > Add defines to be able to enable/clear irq and configure one shot mode.
> > 
> > Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
> 
> Are you fine if I pick this patch with the series?

Nothing heard from you since I Acked this.

Are you still planning on taking this patch?

If so, can you also take patch 1 please?

-- 
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Benjamin GAIGNARD]

On 3/19/20 11:10 AM, Lee Jones wrote:
> On Thu, 20 Feb 2020, Daniel Lezcano wrote:
>> On 17/02/2020 14:45, Benjamin Gaignard wrote:
>>> Add defines to be able to enable/clear irq and configure one shot mode.
>>>
>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>> Are you fine if I pick this patch with the series?
> Nothing heard from you since I Acked this.
>
> Are you still planning on taking this patch?
>
> If so, can you also take patch 1 please?
I will send a v5.
Daniel could you wait until that to merge all the patches (even if this 
one won't change) ?

Benjamin
>

Re: [PATCH v4 2/3] mfd: stm32: Add defines to be used for clkevent purpose

[Daniel Lezcano]

On 19/03/2020 11:28, Benjamin GAIGNARD wrote:
> 
> 
> On 3/19/20 11:10 AM, Lee Jones wrote:
>> On Thu, 20 Feb 2020, Daniel Lezcano wrote:
>>> On 17/02/2020 14:45, Benjamin Gaignard wrote:
>>>> Add defines to be able to enable/clear irq and configure one shot mode.
>>>>
>>>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@st.com>
>>> Are you fine if I pick this patch with the series?
>> Nothing heard from you since I Acked this.
>>
>> Are you still planning on taking this patch?
>>
>> If so, can you also take patch 1 please?
> I will send a v5.
> Daniel could you wait until that to merge all the patches (even if this 
> one won't change) ?

Sure, no problem.


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

[PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Benjamin Gaignard]

From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Implement clock event driver using low power STM32 timers.
Low power timer counters running even when CPUs are stopped.
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.

Low power timers have a 16 bits counter and a prescaler which allow to
divide the clock per power of 2 to up 128 to target a 32KHz rate.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Pascal Paillet <p.paillet@st.com>
---
version 4:
- move defines in mfd/stm32-lptimer.h
- change compatiblename
- reword commit message
- make driver Kconfig depends of MFD_STM32_LPTIMER
- remove useless include
- remove rate and clk fields from the private structure
- to add comments about the registers sequence in stm32_clkevent_lp_set_timer
- rework probe function and use devm_request_irq()
- do not allow module to be removed
- make sure that wakeup interrupt is set

 drivers/clocksource/Kconfig          |   7 ++
 drivers/clocksource/Makefile         |   1 +
 drivers/clocksource/timer-stm32-lp.c | 213 +++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)
 create mode 100644 drivers/clocksource/timer-stm32-lp.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cc909e465823..9fc2b513db6f 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -292,6 +292,13 @@ config CLKSRC_STM32
 	select CLKSRC_MMIO
 	select TIMER_OF
 
+config CLKSRC_STM32_LP
+	bool "Low power clocksource for STM32 SoCs"
+	depends on MFD_STM32_LPTIMER || COMPILE_TEST
+	help
+	  This option enables support for STM32 low power clockevent available
+	  on STM32 SoCs
+
 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 713686faa549..c00fffbd4769 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..50eecdb88216
--- /dev/null
+++ b/drivers/clocksource/timer-stm32-lp.c
@@ -0,0 +1,213 @@
+// 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/clockchips.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/stm32-lptimer.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/pm_wakeirq.h>
+
+#define CFGR_PSC_OFFSET		9
+#define STM32_LP_RATING		400
+#define STM32_TARGET_CLKRATE	(32000 * HZ)
+#define STM32_LP_MAX_PSC	7
+
+struct stm32_lp_private {
+	struct regmap *reg;
+	struct clock_event_device clkevt;
+	unsigned long period;
+};
+
+static struct stm32_lp_private*
+to_priv(struct clock_event_device *clkevt)
+{
+	return container_of(clkevt, struct stm32_lp_private, clkevt);
+}
+
+static int stm32_clkevent_lp_shutdown(struct clock_event_device *clkevt)
+{
+	struct stm32_lp_private *priv = to_priv(clkevt);
+
+	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
+	regmap_write(priv->reg, STM32_LPTIM_IER, 0);
+	/* clear pending flags */
+	regmap_write(priv->reg, STM32_LPTIM_ICR, STM32_LPTIM_ARRMCF);
+
+	return 0;
+}
+
+static int stm32_clkevent_lp_set_timer(unsigned long evt,
+				       struct clock_event_device *clkevt,
+				       int is_periodic)
+{
+	struct stm32_lp_private *priv = to_priv(clkevt);
+
+	/* disable LPTIMER to be able to write into IER register*/
+	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
+	/* enable ARR interrupt */
+	regmap_write(priv->reg, STM32_LPTIM_IER, STM32_LPTIM_ARRMIE);
+	/* enable LPTIMER to be able to write into ARR register */
+	regmap_write(priv->reg, STM32_LPTIM_CR, STM32_LPTIM_ENABLE);
+	/* set next event counter */
+	regmap_write(priv->reg, STM32_LPTIM_ARR, evt);
+
+	/* start counter */
+	if (is_periodic)
+		regmap_write(priv->reg, STM32_LPTIM_CR,
+			     STM32_LPTIM_CNTSTRT | STM32_LPTIM_ENABLE);
+	else
+		regmap_write(priv->reg, STM32_LPTIM_CR,
+			     STM32_LPTIM_SNGSTRT | STM32_LPTIM_ENABLE);
+
+	return 0;
+}
+
+static int stm32_clkevent_lp_set_next_event(unsigned long evt,
+					    struct clock_event_device *clkevt)
+{
+	return stm32_clkevent_lp_set_timer(evt, clkevt,
+					   clockevent_state_periodic(clkevt));
+}
+
+static int stm32_clkevent_lp_set_periodic(struct clock_event_device *clkevt)
+{
+	struct stm32_lp_private *priv = to_priv(clkevt);
+
+	return stm32_clkevent_lp_set_timer(priv->period, clkevt, true);
+}
+
+static int stm32_clkevent_lp_set_oneshot(struct clock_event_device *clkevt)
+{
+	struct stm32_lp_private *priv = to_priv(clkevt);
+
+	return stm32_clkevent_lp_set_timer(priv->period, clkevt, false);
+}
+
+static irqreturn_t stm32_clkevent_lp_irq_handler(int irq, void *dev_id)
+{
+	struct clock_event_device *clkevt = (struct clock_event_device *)dev_id;
+	struct stm32_lp_private *priv = to_priv(clkevt);
+
+	regmap_write(priv->reg, STM32_LPTIM_ICR, STM32_LPTIM_ARRMCF);
+
+	clkevt->event_handler(clkevt);
+
+	return IRQ_HANDLED;
+}
+
+static void stm32_clkevent_lp_set_prescaler(struct stm32_lp_private *priv,
+					    unsigned long *rate)
+{
+	int i;
+
+	for (i = 0; i <= STM32_LP_MAX_PSC; i++) {
+		if (DIV_ROUND_CLOSEST(*rate, 1 << i) < STM32_TARGET_CLKRATE)
+			break;
+	}
+
+	regmap_write(priv->reg, STM32_LPTIM_CFGR, i << CFGR_PSC_OFFSET);
+
+	/* Adjust rate and period given the prescaler value */
+	*rate = DIV_ROUND_CLOSEST(*rate, (1 << i));
+	priv->period = DIV_ROUND_UP(*rate, HZ);
+}
+
+static void stm32_clkevent_lp_init(struct stm32_lp_private *priv,
+				  struct device_node *np, unsigned long rate)
+{
+	priv->clkevt.name = np->full_name;
+	priv->clkevt.cpumask = cpu_possible_mask;
+	priv->clkevt.features = CLOCK_EVT_FEAT_PERIODIC |
+				CLOCK_EVT_FEAT_ONESHOT;
+	priv->clkevt.set_state_shutdown = stm32_clkevent_lp_shutdown;
+	priv->clkevt.set_state_periodic = stm32_clkevent_lp_set_periodic;
+	priv->clkevt.set_state_oneshot = stm32_clkevent_lp_set_oneshot;
+	priv->clkevt.set_next_event = stm32_clkevent_lp_set_next_event;
+	priv->clkevt.rating = STM32_LP_RATING;
+
+	clockevents_config_and_register(&priv->clkevt, rate, 0x1,
+					STM32_LPTIM_MAX_ARR);
+}
+
+static int stm32_clkevent_lp_probe(struct platform_device *pdev)
+{
+	struct stm32_lptimer *ddata = dev_get_drvdata(pdev->dev.parent);
+	struct stm32_lp_private *priv;
+	unsigned long rate;
+	int ret, irq;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->reg = ddata->regmap;
+	ret = clk_prepare_enable(ddata->clk);
+	if (ret)
+		return -EINVAL;
+
+	rate = clk_get_rate(ddata->clk);
+	if (!rate)
+		goto out_clk_disable;
+
+	irq = irq_of_parse_and_map(pdev->dev.parent->of_node, 0);
+	if (!irq)
+		goto out_clk_disable;
+
+	if (of_property_read_bool(pdev->dev.parent->of_node, "wakeup-source")) {
+		ret = device_init_wakeup(&pdev->dev, true);
+		if (ret)
+			goto out_clk_disable;
+
+		ret = dev_pm_set_wake_irq(&pdev->dev, irq);
+		if (ret)
+			goto out_clk_disable;
+	}
+
+	ret = devm_request_irq(&pdev->dev, irq, stm32_clkevent_lp_irq_handler,
+			       IRQF_TIMER, pdev->name, &priv->clkevt);
+	if (ret)
+		goto out_clk_disable;
+
+	stm32_clkevent_lp_set_prescaler(priv, &rate);
+
+	stm32_clkevent_lp_init(priv, pdev->dev.parent->of_node, rate);
+
+	return 0;
+
+out_clk_disable:
+	clk_disable_unprepare(ddata->clk);
+	return -EINVAL;
+}
+
+static int stm32_clkevent_lp_remove(struct platform_device *pdev)
+{
+	return -EBUSY; /* cannot unregister clockevent */
+}
+
+static const struct of_device_id stm32_clkevent_lp_of_match[] = {
+	{ .compatible = "st,stm32-lptimer-timer", },
+	{},
+};
+MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
+
+static struct platform_driver stm32_clkevent_lp_driver = {
+	.probe	= stm32_clkevent_lp_probe,
+	.remove = stm32_clkevent_lp_remove,
+	.driver	= {
+		.name = "stm32-lptimer-timer",
+		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
+	},
+};
+module_platform_driver(stm32_clkevent_lp_driver);
+
+MODULE_ALIAS("platform:stm32-lptimer-timer");
+MODULE_DESCRIPTION("STMicroelectronics STM32 clockevent low power driver");
+MODULE_LICENSE("GPL v2");
-- 
2.15.0

Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Daniel Lezcano]

On 17/02/2020 14:45, Benjamin Gaignard wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> 
> Implement clock event driver using low power STM32 timers.
> Low power timer counters running even when CPUs are stopped.
> 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.
> 
> Low power timers have a 16 bits counter and a prescaler which allow to
> divide the clock per power of 2 to up 128 to target a 32KHz rate.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Signed-off-by: Pascal Paillet <p.paillet@st.com>
> ---
> version 4:
> - move defines in mfd/stm32-lptimer.h
> - change compatiblename
> - reword commit message
> - make driver Kconfig depends of MFD_STM32_LPTIMER
> - remove useless include
> - remove rate and clk fields from the private structure
> - to add comments about the registers sequence in stm32_clkevent_lp_set_timer
> - rework probe function and use devm_request_irq()
> - do not allow module to be removed
> - make sure that wakeup interrupt is set
> 
>  drivers/clocksource/Kconfig          |   7 ++
>  drivers/clocksource/Makefile         |   1 +
>  drivers/clocksource/timer-stm32-lp.c | 213 +++++++++++++++++++++++++++++++++++
>  3 files changed, 221 insertions(+)
>  create mode 100644 drivers/clocksource/timer-stm32-lp.c
> 
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index cc909e465823..9fc2b513db6f 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -292,6 +292,13 @@ config CLKSRC_STM32
>  	select CLKSRC_MMIO
>  	select TIMER_OF
>  
> +config CLKSRC_STM32_LP
> +	bool "Low power clocksource for STM32 SoCs"
> +	depends on MFD_STM32_LPTIMER || COMPILE_TEST
> +	help
> +	  This option enables support for STM32 low power clockevent available
> +	  on STM32 SoCs
> +
>  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 713686faa549..c00fffbd4769 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..50eecdb88216
> --- /dev/null
> +++ b/drivers/clocksource/timer-stm32-lp.c
> @@ -0,0 +1,213 @@
> +// 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/clockchips.h>
> +#include <linux/interrupt.h>
> +#include <linux/mfd/stm32-lptimer.h>
> +#include <linux/module.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/pm_wakeirq.h>
> +
> +#define CFGR_PSC_OFFSET		9
> +#define STM32_LP_RATING		400
> +#define STM32_TARGET_CLKRATE	(32000 * HZ)
> +#define STM32_LP_MAX_PSC	7
> +
> +struct stm32_lp_private {
> +	struct regmap *reg;
> +	struct clock_event_device clkevt;
> +	unsigned long period;
> +};
> +
> +static struct stm32_lp_private*
> +to_priv(struct clock_event_device *clkevt)
> +{
> +	return container_of(clkevt, struct stm32_lp_private, clkevt);
> +}
> +
> +static int stm32_clkevent_lp_shutdown(struct clock_event_device *clkevt)
> +{
> +	struct stm32_lp_private *priv = to_priv(clkevt);
> +
> +	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
> +	regmap_write(priv->reg, STM32_LPTIM_IER, 0);
> +	/* clear pending flags */
> +	regmap_write(priv->reg, STM32_LPTIM_ICR, STM32_LPTIM_ARRMCF);
> +
> +	return 0;
> +}
> +
> +static int stm32_clkevent_lp_set_timer(unsigned long evt,
> +				       struct clock_event_device *clkevt,
> +				       int is_periodic)
> +{
> +	struct stm32_lp_private *priv = to_priv(clkevt);
> +
> +	/* disable LPTIMER to be able to write into IER register*/
> +	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
> +	/* enable ARR interrupt */
> +	regmap_write(priv->reg, STM32_LPTIM_IER, STM32_LPTIM_ARRMIE);
> +	/* enable LPTIMER to be able to write into ARR register */
> +	regmap_write(priv->reg, STM32_LPTIM_CR, STM32_LPTIM_ENABLE);
> +	/* set next event counter */
> +	regmap_write(priv->reg, STM32_LPTIM_ARR, evt);
> +
> +	/* start counter */
> +	if (is_periodic)
> +		regmap_write(priv->reg, STM32_LPTIM_CR,
> +			     STM32_LPTIM_CNTSTRT | STM32_LPTIM_ENABLE);
> +	else
> +		regmap_write(priv->reg, STM32_LPTIM_CR,
> +			     STM32_LPTIM_SNGSTRT | STM32_LPTIM_ENABLE);

The regmap config in stm32-lptimer is not defined with the fast_io flag
(on purpose or not?) that means we can potentially deadlock here as the
lock is a mutex.

Isn't it detected with the lock validation scheme?

> +	return 0;
> +}
> +static int stm32_clkevent_lp_remove(struct platform_device *pdev)
> +{
> +	return -EBUSY; /* cannot unregister clockevent */
> +}

Won't be the mfd into an inconsistent state here? The other subsystems
will be removed but this one will prevent to unload the module leading
to a situation where the mfd is partially removed but still there
without a possible recovery, no?

> +static const struct of_device_id stm32_clkevent_lp_of_match[] = {
> +	{ .compatible = "st,stm32-lptimer-timer", },
> +	{},
> +};
> +MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
> +
> +static struct platform_driver stm32_clkevent_lp_driver = {
> +	.probe	= stm32_clkevent_lp_probe,
> +	.remove = stm32_clkevent_lp_remove,
> +	.driver	= {
> +		.name = "stm32-lptimer-timer",
> +		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
> +	},
> +};


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

Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Benjamin GAIGNARD]

On 2/20/20 11:36 AM, Daniel Lezcano wrote:
> On 17/02/2020 14:45, Benjamin Gaignard wrote:
>> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>
>> Implement clock event driver using low power STM32 timers.
>> Low power timer counters running even when CPUs are stopped.
>> 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.
>>
>> Low power timers have a 16 bits counter and a prescaler which allow to
>> divide the clock per power of 2 to up 128 to target a 32KHz rate.
>>
>> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Signed-off-by: Pascal Paillet <p.paillet@st.com>
>> ---
>> version 4:
>> - move defines in mfd/stm32-lptimer.h
>> - change compatiblename
>> - reword commit message
>> - make driver Kconfig depends of MFD_STM32_LPTIMER
>> - remove useless include
>> - remove rate and clk fields from the private structure
>> - to add comments about the registers sequence in stm32_clkevent_lp_set_timer
>> - rework probe function and use devm_request_irq()
>> - do not allow module to be removed
>> - make sure that wakeup interrupt is set
>>
>>   drivers/clocksource/Kconfig          |   7 ++
>>   drivers/clocksource/Makefile         |   1 +
>>   drivers/clocksource/timer-stm32-lp.c | 213 +++++++++++++++++++++++++++++++++++
>>   3 files changed, 221 insertions(+)
>>   create mode 100644 drivers/clocksource/timer-stm32-lp.c
>>
>> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
>> index cc909e465823..9fc2b513db6f 100644
>> --- a/drivers/clocksource/Kconfig
>> +++ b/drivers/clocksource/Kconfig
>> @@ -292,6 +292,13 @@ config CLKSRC_STM32
>>   	select CLKSRC_MMIO
>>   	select TIMER_OF
>>   
>> +config CLKSRC_STM32_LP
>> +	bool "Low power clocksource for STM32 SoCs"
>> +	depends on MFD_STM32_LPTIMER || COMPILE_TEST
>> +	help
>> +	  This option enables support for STM32 low power clockevent available
>> +	  on STM32 SoCs
>> +
>>   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 713686faa549..c00fffbd4769 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..50eecdb88216
>> --- /dev/null
>> +++ b/drivers/clocksource/timer-stm32-lp.c
>> @@ -0,0 +1,213 @@
>> +// 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/clockchips.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/mfd/stm32-lptimer.h>
>> +#include <linux/module.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_irq.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/pm_wakeirq.h>
>> +
>> +#define CFGR_PSC_OFFSET		9
>> +#define STM32_LP_RATING		400
>> +#define STM32_TARGET_CLKRATE	(32000 * HZ)
>> +#define STM32_LP_MAX_PSC	7
>> +
>> +struct stm32_lp_private {
>> +	struct regmap *reg;
>> +	struct clock_event_device clkevt;
>> +	unsigned long period;
>> +};
>> +
>> +static struct stm32_lp_private*
>> +to_priv(struct clock_event_device *clkevt)
>> +{
>> +	return container_of(clkevt, struct stm32_lp_private, clkevt);
>> +}
>> +
>> +static int stm32_clkevent_lp_shutdown(struct clock_event_device *clkevt)
>> +{
>> +	struct stm32_lp_private *priv = to_priv(clkevt);
>> +
>> +	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
>> +	regmap_write(priv->reg, STM32_LPTIM_IER, 0);
>> +	/* clear pending flags */
>> +	regmap_write(priv->reg, STM32_LPTIM_ICR, STM32_LPTIM_ARRMCF);
>> +
>> +	return 0;
>> +}
>> +
>> +static int stm32_clkevent_lp_set_timer(unsigned long evt,
>> +				       struct clock_event_device *clkevt,
>> +				       int is_periodic)
>> +{
>> +	struct stm32_lp_private *priv = to_priv(clkevt);
>> +
>> +	/* disable LPTIMER to be able to write into IER register*/
>> +	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
>> +	/* enable ARR interrupt */
>> +	regmap_write(priv->reg, STM32_LPTIM_IER, STM32_LPTIM_ARRMIE);
>> +	/* enable LPTIMER to be able to write into ARR register */
>> +	regmap_write(priv->reg, STM32_LPTIM_CR, STM32_LPTIM_ENABLE);
>> +	/* set next event counter */
>> +	regmap_write(priv->reg, STM32_LPTIM_ARR, evt);
>> +
>> +	/* start counter */
>> +	if (is_periodic)
>> +		regmap_write(priv->reg, STM32_LPTIM_CR,
>> +			     STM32_LPTIM_CNTSTRT | STM32_LPTIM_ENABLE);
>> +	else
>> +		regmap_write(priv->reg, STM32_LPTIM_CR,
>> +			     STM32_LPTIM_SNGSTRT | STM32_LPTIM_ENABLE);
> The regmap config in stm32-lptimer is not defined with the fast_io flag
> (on purpose or not?) that means we can potentially deadlock here as the
> lock is a mutex.
>
> Isn't it detected with the lock validation scheme?
It wasn't a problem with other parts of the mfd and I don't notice 
issues so I guess it is ok.
>
>> +	return 0;
>> +}
>> +static int stm32_clkevent_lp_remove(struct platform_device *pdev)
>> +{
>> +	return -EBUSY; /* cannot unregister clockevent */
>> +}
> Won't be the mfd into an inconsistent state here? The other subsystems
> will be removed but this one will prevent to unload the module leading
> to a situation where the mfd is partially removed but still there
> without a possible recovery, no?
We can't enable the timer part of the mfd at the same time than the 
other features.
It has be exclusive and that exclude the problem you describe above.

>
>> +static const struct of_device_id stm32_clkevent_lp_of_match[] = {
>> +	{ .compatible = "st,stm32-lptimer-timer", },
>> +	{},
>> +};
>> +MODULE_DEVICE_TABLE(of, stm32_clkevent_lp_of_match);
>> +
>> +static struct platform_driver stm32_clkevent_lp_driver = {
>> +	.probe	= stm32_clkevent_lp_probe,
>> +	.remove = stm32_clkevent_lp_remove,
>> +	.driver	= {
>> +		.name = "stm32-lptimer-timer",
>> +		.of_match_table = of_match_ptr(stm32_clkevent_lp_of_match),
>> +	},
>> +};
>

Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Daniel Lezcano]

On 20/02/2020 11:45, Benjamin GAIGNARD wrote:

[ ... ]

>>> +{
>>> +	struct stm32_lp_private *priv = to_priv(clkevt);
>>> +
>>> +	/* disable LPTIMER to be able to write into IER register*/
>>> +	regmap_write(priv->reg, STM32_LPTIM_CR, 0);
>>> +	/* enable ARR interrupt */
>>> +	regmap_write(priv->reg, STM32_LPTIM_IER, STM32_LPTIM_ARRMIE);
>>> +	/* enable LPTIMER to be able to write into ARR register */
>>> +	regmap_write(priv->reg, STM32_LPTIM_CR, STM32_LPTIM_ENABLE);
>>> +	/* set next event counter */
>>> +	regmap_write(priv->reg, STM32_LPTIM_ARR, evt);
>>> +
>>> +	/* start counter */
>>> +	if (is_periodic)
>>> +		regmap_write(priv->reg, STM32_LPTIM_CR,
>>> +			     STM32_LPTIM_CNTSTRT | STM32_LPTIM_ENABLE);
>>> +	else
>>> +		regmap_write(priv->reg, STM32_LPTIM_CR,
>>> +			     STM32_LPTIM_SNGSTRT | STM32_LPTIM_ENABLE);
>> The regmap config in stm32-lptimer is not defined with the fast_io flag
>> (on purpose or not?) that means we can potentially deadlock here as the
>> lock is a mutex.
>>
>> Isn't it detected with the lock validation scheme?
> It wasn't a problem with other parts of the mfd and I don't notice 
> issues so I guess it is ok.

Given your comment below, the case can't happen I agree but there is
still a heavy operation with the locking.

>>> +	return 0;
>>> +}
>>> +static int stm32_clkevent_lp_remove(struct platform_device *pdev)
>>> +{
>>> +	return -EBUSY; /* cannot unregister clockevent */
>>> +}
>> Won't be the mfd into an inconsistent state here? The other subsystems
>> will be removed but this one will prevent to unload the module leading
>> to a situation where the mfd is partially removed but still there
>> without a possible recovery, no?
> We can't enable the timer part of the mfd at the same time than the 
> other features.

Hmm, interesting case. The DT binding does not give this information,
especially in the example. You should fix the DT by giving two examples IMO.

Rob, how do you describe this situation (exclusive properties)?

> It has be exclusive and that exclude the problem you describe above.

Ok, the regmap_write is not a free operation and in this case you can
get rid of all the regmap-ish in this driver.

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

Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Daniel Lezcano]

Hi Benjamin,

On 20/02/2020 12:05, Daniel Lezcano wrote:

[ ... ]

>> It has be exclusive and that exclude the problem you describe above.
> 
> Ok, the regmap_write is not a free operation and in this case you can
> get rid of all the regmap-ish in this driver.

Are you planning to send the non-regmap version?



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

Re: [PATCH v4 3/3] clocksource: Add Low Power STM32 timers driver

[Benjamin GAIGNARD]

On 3/13/20 2:34 PM, Daniel Lezcano wrote:
> Hi Benjamin,
>
> On 20/02/2020 12:05, Daniel Lezcano wrote:
>
> [ ... ]
>
>>> It has be exclusive and that exclude the problem you describe above.
>> Ok, the regmap_write is not a free operation and in this case you can
>> get rid of all the regmap-ish in this driver.
> Are you planning to send the non-regmap version?
No because the regmap is inherited from the mfd parent.
I could use fast-io to improve that.

Benjamin
>
>
>