linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source
@ 2019-07-10 11:04 Biwen Li
  2019-07-10 11:04 ` [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver Biwen Li
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Biwen Li @ 2019-07-10 11:04 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, leoyang.li
  Cc: linux-rtc, linux-kernel, xiaobo.xie, jiafei.pan, ran.wang_1, Biwen Li

For the paltforms including LS1012A, LS1021A, LS1028A, LS1043A,
LS1046A, LS1088A, LS208xA that has the FlexTimer
module, implementing alarm functions within RTC subsystem
to wakeup the system when system going to sleep (work with RCPM driver).

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---

Change in v2:
    - remove code about setting rcpm

 drivers/rtc/Kconfig             |  14 ++
 drivers/rtc/Makefile            |   1 +
 drivers/rtc/rtc-fsl-ftm-alarm.c | 311 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+)
 create mode 100644 drivers/rtc/rtc-fsl-ftm-alarm.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 03b60d5..0758a08 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1313,6 +1313,20 @@ config RTC_DRV_IMXDI
 	   This driver can also be built as a module, if so, the module
 	   will be called "rtc-imxdi".
 
+config RTC_DRV_FSL_FTM_ALARM
+	tristate "Freescale FlexTimer alarm timer"
+	depends on ARCH_LAYERSCAPE
+	default y
+	help
+	   For the FlexTimer in LS1012A, LS1021A, LS1028A, LS1043A, LS1046A,
+	   LS1088A, LS208xA, we can use FTM as the wakeup source.
+
+	   Say y here to enable FTM alarm support. The FTM alarm provides
+	   alarm functions for wakeup system from deep sleep.
+
+	   This driver can also be built as a module, if so, the module
+	   will be called "rtc-fsl-ftm-alarm".
+
 config RTC_DRV_MESON
 	tristate "Amlogic Meson RTC"
 	depends on (ARM && ARCH_MESON) || COMPILE_TEST
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 9d997fa..5cccb07 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
 obj-$(CONFIG_RTC_DRV_HYM8563)	+= rtc-hym8563.o
 obj-$(CONFIG_RTC_DRV_IMXDI)	+= rtc-imxdi.o
 obj-$(CONFIG_RTC_DRV_IMX_SC)	+= rtc-imx-sc.o
+obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)	+= rtc-fsl-ftm-alarm.o
 obj-$(CONFIG_RTC_DRV_ISL12022)	+= rtc-isl12022.o
 obj-$(CONFIG_RTC_DRV_ISL12026)	+= rtc-isl12026.o
 obj-$(CONFIG_RTC_DRV_ISL1208)	+= rtc-isl1208.o
diff --git a/drivers/rtc/rtc-fsl-ftm-alarm.c b/drivers/rtc/rtc-fsl-ftm-alarm.c
new file mode 100644
index 0000000..1836c2e
--- /dev/null
+++ b/drivers/rtc/rtc-fsl-ftm-alarm.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale FlexTimer Module (FTM) alarm device driver.
+ *
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ * Copyright 2019 NXP
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/fsl/ftm.h>
+#include <linux/rtc.h>
+#include <linux/time.h>
+
+#define FTM_SC_CLK(c)		((c) << FTM_SC_CLK_MASK_SHIFT)
+
+/*
+ * Select Fixed frequency clock as clock source
+ * of FlexTimer Module
+ */
+#define FTM_SC_CLKS_FIXED_FREQ	0x02
+
+#define FIXED_FREQ_CLK		32000
+#define MAX_FREQ_DIV		(1 << FTM_SC_PS_MASK)
+#define MAX_COUNT_VAL		0xffff
+
+struct ftm_rtc {
+	struct rtc_device *rtc_dev;
+	void __iomem *base;
+	bool endian;
+	u32 alarm_freq;
+};
+
+static inline u32 rtc_readl(struct ftm_rtc *dev, u32 reg)
+{
+	if (dev->endian)
+		return ioread32be(dev->base + reg); /*big endianness*/
+
+	return ioread32(dev->base + reg);
+}
+
+static inline void rtc_writel(struct ftm_rtc *dev, u32 reg, u32 val)
+{
+	if (dev->endian)
+		iowrite32be(val, dev->base + reg);
+	else
+		iowrite32(val, dev->base + reg);
+}
+
+static inline void ftm_counter_enable(struct ftm_rtc *rtc)
+{
+	u32 val;
+
+	/* select and enable counter clock source */
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
+	val |= (FTM_SC_PS_MASK | FTM_SC_CLK(FTM_SC_CLKS_FIXED_FREQ));
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_counter_disable(struct ftm_rtc *rtc)
+{
+	u32 val;
+
+	/* disable counter clock source */
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_irq_acknowledge(struct ftm_rtc *rtc)
+{
+	unsigned int timeout = 100;
+
+	while ((FTM_SC_TOF & rtc_readl(rtc, FTM_SC)) && timeout--)
+		rtc_writel(rtc, FTM_SC, rtc_readl(rtc, FTM_SC) & (~FTM_SC_TOF));
+}
+
+static inline void ftm_irq_enable(struct ftm_rtc *rtc)
+{
+	u32 val;
+
+	val = rtc_readl(rtc, FTM_SC);
+	val |= FTM_SC_TOIE;
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_irq_disable(struct ftm_rtc *rtc)
+{
+	u32 val;
+
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~FTM_SC_TOIE;
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_reset_counter(struct ftm_rtc *rtc)
+{
+	/*
+	 * The CNT register contains the FTM counter value.
+	 * Reset clears the CNT register. Writing any value to COUNT
+	 * updates the counter with its initial value, CNTIN.
+	 */
+	rtc_writel(rtc, FTM_CNT, 0x00);
+}
+
+static void ftm_clean_alarm(struct ftm_rtc *rtc)
+{
+	ftm_counter_disable(rtc);
+
+	rtc_writel(rtc, FTM_CNTIN, 0x00);
+	rtc_writel(rtc, FTM_MOD, ~0U);
+
+	ftm_reset_counter(rtc);
+}
+
+static irqreturn_t ftm_rtc_alarm_interrupt(int irq, void *dev)
+{
+	struct ftm_rtc *rtc = dev;
+
+	ftm_irq_acknowledge(rtc);
+	ftm_irq_disable(rtc);
+	ftm_clean_alarm(rtc);
+
+	return IRQ_HANDLED;
+}
+
+static int ftm_rtc_alarm_irq_enable(struct device *dev,
+		unsigned int enabled)
+{
+	struct ftm_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		ftm_irq_enable(rtc);
+	else
+		ftm_irq_disable(rtc);
+
+	return 0;
+}
+
+/*
+ * Note:
+ *	The function is not really getting time from the RTC
+ *	since FlexTimer is not a RTC device, but we need to
+ *	get time to setup alarm, so we are using system time
+ *	for now.
+ */
+static int ftm_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct timespec64 ts64;
+	unsigned long local_time;
+
+	ktime_get_real_ts64(&ts64);
+	local_time = (unsigned long)(ts64.tv_sec - (sys_tz.tz_minuteswest * 60));
+
+	rtc_time_to_tm(local_time, tm);
+
+	return 0;
+}
+static int ftm_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+	return 0;
+}
+
+/*250Hz, 65536 / 250 = 262 second max*/
+static int ftm_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
+{
+	struct rtc_time tm;
+	unsigned long now, alm_time, cycle;
+	struct ftm_rtc *rtc = dev_get_drvdata(dev);
+
+	ftm_rtc_read_time(dev, &tm);
+	rtc_tm_to_time(&tm, &now);
+	rtc_tm_to_time(&alm->time, &alm_time);
+
+	ftm_clean_alarm(rtc);
+	cycle = (alm_time - now) * rtc->alarm_freq;
+	if (cycle > MAX_COUNT_VAL) {
+		pr_err("Out of alarm range.\n");
+		return -EINVAL;
+	}
+
+	ftm_irq_disable(rtc);
+
+	/*
+	 * The counter increments until the value of MOD is reached,
+	 * at which point the counter is reloaded with the value of CNTIN.
+	 * The TOF (the overflow flag) bit is set when the FTM counter
+	 * changes from MOD to CNTIN. So we should using the cycle - 1.
+	 */
+	rtc_writel(rtc, FTM_MOD, cycle - 1);
+
+	ftm_counter_enable(rtc);
+	ftm_irq_enable(rtc);
+
+	return 0;
+
+}
+
+static const struct rtc_class_ops ftm_rtc_ops = {
+	.read_time		= ftm_rtc_read_time,
+	.read_alarm		= ftm_rtc_read_alarm,
+	.set_alarm		= ftm_rtc_set_alarm,
+	.alarm_irq_enable	= ftm_rtc_alarm_irq_enable,
+};
+static int ftm_rtc_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *r;
+	int irq;
+	int ret;
+	struct ftm_rtc *rtc;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (unlikely(!rtc)) {
+		pr_err("ftm: cannot alloc memery for rtc\n");
+		return -ENOMEM;
+	}
+
+
+	platform_set_drvdata(pdev, rtc);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		pr_err("ftm: cannot get resource for rtc\n");
+		return -ENODEV;
+	}
+
+	rtc->base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(rtc->base)) {
+		pr_err("ftm: cannot ioremap resource for rtc\n");
+		return PTR_ERR(rtc->base);
+	}
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq <= 0) {
+		pr_err("ftm: unable to get IRQ from DT, %d\n", irq);
+		return -EINVAL;
+	}
+
+	rtc->endian = of_property_read_bool(np, "big-endian");
+
+	ret = devm_request_irq(&pdev->dev, irq, ftm_rtc_alarm_interrupt,
+			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), rtc);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	rtc->alarm_freq = (u32)FIXED_FREQ_CLK / (u32)MAX_FREQ_DIV;
+
+	ftm_clean_alarm(rtc);
+
+	device_init_wakeup(&pdev->dev, true);
+	rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "ftm-alarm",
+							&ftm_rtc_ops,
+							THIS_MODULE);
+	if (IS_ERR(rtc->rtc_dev)) {
+		dev_err(&pdev->dev, "can't register rtc device\n");
+		return PTR_ERR(rtc->rtc_dev);
+	}
+	return ret;
+}
+
+static const struct of_device_id ftm_rtc_match[] = {
+	{ .compatible = "fsl,ftm-alarm", },
+	{ .compatible = "fsl,ls1012a-ftm-alarm", },
+	{ .compatible = "fsl,ls1021a-ftm-alarm", },
+	{ .compatible = "fsl,ls1043a-ftm-alarm", },
+	{ .compatible = "fsl,ls1046a-ftm-alarm", },
+	{ .compatible = "fsl,ls1088a-ftm-alarm", },
+	{ .compatible = "fsl,ls208xa-ftm-alarm", },
+	{ .compatible = "fsl,ls1028a-ftm-alarm", },
+	{ },
+};
+
+static struct platform_driver ftm_rtc_driver = {
+	.probe		= ftm_rtc_probe,
+	.driver		= {
+		.name	= "ftm-alarm",
+		.of_match_table = ftm_rtc_match,
+	},
+};
+
+static int __init ftm_alarm_init(void)
+{
+	return platform_driver_register(&ftm_rtc_driver);
+}
+
+/***************
+ *Ensure that the driver is initialized after
+ *any real rtc driver
+ */
+device_initcall_sync(ftm_alarm_init);
+
+MODULE_DESCRIPTION("NXP/Freescale FlexTimer alarm driver");
+MODULE_AUTHOR("Biwen Li <biwen.li@nxp.com>");
+MODULE_LICENSE("GPL");
-- 
2.7.4


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

* [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver
  2019-07-10 11:04 [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Biwen Li
@ 2019-07-10 11:04 ` Biwen Li
  2019-07-10 19:45   ` Li Yang
  2019-07-10 21:11 ` [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Li Yang
  2019-07-11 14:12 ` Biwen Li
  2 siblings, 1 reply; 7+ messages in thread
From: Biwen Li @ 2019-07-10 11:04 UTC (permalink / raw)
  To: a.zummo, alexandre.belloni, leoyang.li
  Cc: linux-rtc, linux-kernel, xiaobo.xie, jiafei.pan, ran.wang_1, Biwen Li

The patch adds binding for ftm alarm driver

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---
Change in v2:
    - replace ls1043a with ls1088a as example
    - add rcpm node and fsl,rcpm-wakeup property

 .../devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt  | 40 ++++++++++++++++++++++
 1 file changed, 40 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
new file mode 100644
index 0000000..010984a
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
@@ -0,0 +1,40 @@
+Freescale FlexTimer Module (FTM) Alarm
+
+Note: The driver need work with RCPM driver to wake up system in sleep.
+
+Required properties:
+
+- compatible : Should be "fsl,ftm-alarm" or "fsl,<chip>-ftm-alarm", the
+	       supported chips include
+	       "fsl,ls1012a-ftm-alarm"
+	       "fsl,ls1021a-ftm-alarm"
+	       "fsl,ls1028a-ftm-alarm"
+	       "fsl,ls1043a-ftm-alarm"
+	       "fsl,ls1046a-ftm-alarm"
+	       "fsl,ls1088a-ftm-alarm"
+	       "fsl,ls208xa-ftm-alarm"
+- reg : Specifies base physical address and size of the register sets for the
+  FlexTimer Module and base physical address of IP Powerdown Exception Control
+  Register.
+- reg-names: names of the mapped memory regions listed in regs property.
+  should include the following entries:
+  "ftm":    Address of the register sets for FlexTimer Module
+- interrupts : Should be the FlexTimer Module interrupt.
+- fsl,rcpm-wakeup property and rcpm node : Please refer
+	Documentation/devicetree/bindings/soc/fsl/rcpm.txt
+- big-endian: If the host controller is big-endian mode, specify this property.
+  The default endian mode is little-endian.
+
+Example:
+rcpm: rcpm@1e34050 {
+	compatible = "fsl,ls1088a-rcpm", "fsl,qoriq-rcpm-2.1+";
+	reg = <0x0 0x1e34050 0x0 0x4>;
+	fsl,#rcpm-wakeup-cells = <1>;
+}
+
+ftm_alarm0: timer@2800000 {
+	compatible = "fsl,ftm-alarm";
+	reg = <0x0 0x2800000 0x0 0x10000>;
+	fsl,rcpm-wakeup = <&rcpm 0x0 0x4000>;
+	interrupts = <0 44 4>;
+}
-- 
2.7.4


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

* Re: [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver
  2019-07-10 11:04 ` [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver Biwen Li
@ 2019-07-10 19:45   ` Li Yang
  2019-07-10 20:26     ` Rob Herring
  0 siblings, 1 reply; 7+ messages in thread
From: Li Yang @ 2019-07-10 19:45 UTC (permalink / raw)
  To: Biwen Li, Rob Herring
  Cc: a.zummo, alexandre.belloni, linux-rtc, lkml, Xiaobo Xie,
	Jiafei Pan, Ran Wang

On Wed, Jul 10, 2019 at 6:35 AM Biwen Li <biwen.li@nxp.com> wrote:
>
> The patch adds binding for ftm alarm driver
>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>

Looks like I commented the older version just now.  Adding Rob to this
version too.

> ---
> Change in v2:
>     - replace ls1043a with ls1088a as example
>     - add rcpm node and fsl,rcpm-wakeup property
>
>  .../devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt  | 40 ++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
>
> diff --git a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> new file mode 100644
> index 0000000..010984a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> @@ -0,0 +1,40 @@
> +Freescale FlexTimer Module (FTM) Alarm
> +
> +Note: The driver need work with RCPM driver to wake up system in sleep.
> +
> +Required properties:
> +
> +- compatible : Should be "fsl,ftm-alarm" or "fsl,<chip>-ftm-alarm", the
> +              supported chips include
> +              "fsl,ls1012a-ftm-alarm"
> +              "fsl,ls1021a-ftm-alarm"
> +              "fsl,ls1028a-ftm-alarm"
> +              "fsl,ls1043a-ftm-alarm"
> +              "fsl,ls1046a-ftm-alarm"
> +              "fsl,ls1088a-ftm-alarm"
> +              "fsl,ls208xa-ftm-alarm"
> +- reg : Specifies base physical address and size of the register sets for the
> +  FlexTimer Module and base physical address of IP Powerdown Exception Control
> +  Register.
> +- reg-names: names of the mapped memory regions listed in regs property.
> +  should include the following entries:
> +  "ftm":    Address of the register sets for FlexTimer Module
> +- interrupts : Should be the FlexTimer Module interrupt.
> +- fsl,rcpm-wakeup property and rcpm node : Please refer
> +       Documentation/devicetree/bindings/soc/fsl/rcpm.txt

Looks better.

> +- big-endian: If the host controller is big-endian mode, specify this property.
> +  The default endian mode is little-endian.

Same comment about optional property.

> +
> +Example:
> +rcpm: rcpm@1e34050 {
> +       compatible = "fsl,ls1088a-rcpm", "fsl,qoriq-rcpm-2.1+";
> +       reg = <0x0 0x1e34050 0x0 0x4>;
> +       fsl,#rcpm-wakeup-cells = <1>;
> +}
> +
> +ftm_alarm0: timer@2800000 {
> +       compatible = "fsl,ftm-alarm";
> +       reg = <0x0 0x2800000 0x0 0x10000>;
> +       fsl,rcpm-wakeup = <&rcpm 0x0 0x4000>;
> +       interrupts = <0 44 4>;
> +}
> --
> 2.7.4
>

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

* Re: [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver
  2019-07-10 19:45   ` Li Yang
@ 2019-07-10 20:26     ` Rob Herring
  2019-07-11 11:04       ` [EXT] " Biwen Li
  0 siblings, 1 reply; 7+ messages in thread
From: Rob Herring @ 2019-07-10 20:26 UTC (permalink / raw)
  To: Li Yang
  Cc: Biwen Li, Alessandro Zummo, Alexandre Belloni,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM, lkml, Xiaobo Xie,
	Jiafei Pan, Ran Wang

On Wed, Jul 10, 2019 at 1:46 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Wed, Jul 10, 2019 at 6:35 AM Biwen Li <biwen.li@nxp.com> wrote:
> >
> > The patch adds binding for ftm alarm driver
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
>
> Looks like I commented the older version just now.  Adding Rob to this
> version too.

More importantly, re-send the patch to the DT list so patchwork tracks it.

>
> > ---
> > Change in v2:
> >     - replace ls1043a with ls1088a as example
> >     - add rcpm node and fsl,rcpm-wakeup property
> >
> >  .../devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt  | 40 ++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> >
> > diff --git a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> > new file mode 100644
> > index 0000000..010984a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> > @@ -0,0 +1,40 @@
> > +Freescale FlexTimer Module (FTM) Alarm
> > +
> > +Note: The driver need work with RCPM driver to wake up system in sleep.
> > +
> > +Required properties:
> > +
> > +- compatible : Should be "fsl,ftm-alarm" or "fsl,<chip>-ftm-alarm", the

fsl,ftm-alarm should be a fallback, not on its own.

> > +              supported chips include
> > +              "fsl,ls1012a-ftm-alarm"
> > +              "fsl,ls1021a-ftm-alarm"
> > +              "fsl,ls1028a-ftm-alarm"
> > +              "fsl,ls1043a-ftm-alarm"
> > +              "fsl,ls1046a-ftm-alarm"
> > +              "fsl,ls1088a-ftm-alarm"
> > +              "fsl,ls208xa-ftm-alarm"
> > +- reg : Specifies base physical address and size of the register sets for the
> > +  FlexTimer Module and base physical address of IP Powerdown Exception Control
> > +  Register.
> > +- reg-names: names of the mapped memory regions listed in regs property.
> > +  should include the following entries:
> > +  "ftm":    Address of the register sets for FlexTimer Module

Says required, but not in the example. I'd just remove this as -names
is pointless when there is only 1 entry.

> > +- interrupts : Should be the FlexTimer Module interrupt.
> > +- fsl,rcpm-wakeup property and rcpm node : Please refer
> > +       Documentation/devicetree/bindings/soc/fsl/rcpm.txt
>
> Looks better.
>
> > +- big-endian: If the host controller is big-endian mode, specify this property.
> > +  The default endian mode is little-endian.
>
> Same comment about optional property.
>
> > +
> > +Example:
> > +rcpm: rcpm@1e34050 {
> > +       compatible = "fsl,ls1088a-rcpm", "fsl,qoriq-rcpm-2.1+";
> > +       reg = <0x0 0x1e34050 0x0 0x4>;
> > +       fsl,#rcpm-wakeup-cells = <1>;

1 cell here...

> > +}
> > +
> > +ftm_alarm0: timer@2800000 {
> > +       compatible = "fsl,ftm-alarm";
> > +       reg = <0x0 0x2800000 0x0 0x10000>;
> > +       fsl,rcpm-wakeup = <&rcpm 0x0 0x4000>;

...and 2 cells here.

> > +       interrupts = <0 44 4>;
> > +}
> > --
> > 2.7.4
> >

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

* Re: [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source
  2019-07-10 11:04 [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Biwen Li
  2019-07-10 11:04 ` [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver Biwen Li
@ 2019-07-10 21:11 ` Li Yang
  2019-07-11 14:12 ` Biwen Li
  2 siblings, 0 replies; 7+ messages in thread
From: Li Yang @ 2019-07-10 21:11 UTC (permalink / raw)
  To: Biwen Li
  Cc: a.zummo, alexandre.belloni, linux-rtc, lkml, Xiaobo Xie,
	Jiafei Pan, Ran Wang

On Wed, Jul 10, 2019 at 6:35 AM Biwen Li <biwen.li@nxp.com> wrote:
>
> For the paltforms including LS1012A, LS1021A, LS1028A, LS1043A,
> LS1046A, LS1088A, LS208xA that has the FlexTimer
> module, implementing alarm functions within RTC subsystem
> to wakeup the system when system going to sleep (work with RCPM driver).
>
> Signed-off-by: Biwen Li <biwen.li@nxp.com>
> ---
>
> Change in v2:
>     - remove code about setting rcpm
>
>  drivers/rtc/Kconfig             |  14 ++
>  drivers/rtc/Makefile            |   1 +
>  drivers/rtc/rtc-fsl-ftm-alarm.c | 311 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 326 insertions(+)
>  create mode 100644 drivers/rtc/rtc-fsl-ftm-alarm.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 03b60d5..0758a08 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1313,6 +1313,20 @@ config RTC_DRV_IMXDI
>            This driver can also be built as a module, if so, the module
>            will be called "rtc-imxdi".
>
> +config RTC_DRV_FSL_FTM_ALARM
> +       tristate "Freescale FlexTimer alarm timer"
> +       depends on ARCH_LAYERSCAPE
> +       default y
> +       help
> +          For the FlexTimer in LS1012A, LS1021A, LS1028A, LS1043A, LS1046A,
> +          LS1088A, LS208xA, we can use FTM as the wakeup source.
> +
> +          Say y here to enable FTM alarm support. The FTM alarm provides
> +          alarm functions for wakeup system from deep sleep.
> +
> +          This driver can also be built as a module, if so, the module
> +          will be called "rtc-fsl-ftm-alarm".
> +
>  config RTC_DRV_MESON
>         tristate "Amlogic Meson RTC"
>         depends on (ARM && ARCH_MESON) || COMPILE_TEST
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 9d997fa..5cccb07 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -79,6 +79,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
>  obj-$(CONFIG_RTC_DRV_HYM8563)  += rtc-hym8563.o
>  obj-$(CONFIG_RTC_DRV_IMXDI)    += rtc-imxdi.o
>  obj-$(CONFIG_RTC_DRV_IMX_SC)   += rtc-imx-sc.o
> +obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)    += rtc-fsl-ftm-alarm.o
>  obj-$(CONFIG_RTC_DRV_ISL12022) += rtc-isl12022.o
>  obj-$(CONFIG_RTC_DRV_ISL12026) += rtc-isl12026.o
>  obj-$(CONFIG_RTC_DRV_ISL1208)  += rtc-isl1208.o
> diff --git a/drivers/rtc/rtc-fsl-ftm-alarm.c b/drivers/rtc/rtc-fsl-ftm-alarm.c
> new file mode 100644
> index 0000000..1836c2e
> --- /dev/null
> +++ b/drivers/rtc/rtc-fsl-ftm-alarm.c
> @@ -0,0 +1,311 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Freescale FlexTimer Module (FTM) alarm device driver.
> + *
> + * Copyright 2014 Freescale Semiconductor, Inc.
> + * Copyright 2019 NXP
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.

No need to add this with the SPDX tag.  And btw, the text here is
GPL-2.0+ instead of GPL-2.0.

> + */
> +
> +#include <linux/device.h>
> +#include <linux/err.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/module.h>
> +#include <linux/fsl/ftm.h>
> +#include <linux/rtc.h>
> +#include <linux/time.h>
> +
> +#define FTM_SC_CLK(c)          ((c) << FTM_SC_CLK_MASK_SHIFT)
> +
> +/*
> + * Select Fixed frequency clock as clock source
> + * of FlexTimer Module
> + */
> +#define FTM_SC_CLKS_FIXED_FREQ 0x02
> +
> +#define FIXED_FREQ_CLK         32000
> +#define MAX_FREQ_DIV           (1 << FTM_SC_PS_MASK)
> +#define MAX_COUNT_VAL          0xffff
> +
> +struct ftm_rtc {
> +       struct rtc_device *rtc_dev;
> +       void __iomem *base;
> +       bool endian;

Make it big_endian to be more clear.

> +       u32 alarm_freq;
> +};
> +
> +static inline u32 rtc_readl(struct ftm_rtc *dev, u32 reg)
> +{
> +       if (dev->endian)
> +               return ioread32be(dev->base + reg); /*big endianness*/

No need to have the extra comment with new variable name.

And probably better to use "else" here to make clearer to
compiler/static analyser.

> +
> +       return ioread32(dev->base + reg);
> +}
> +
> +static inline void rtc_writel(struct ftm_rtc *dev, u32 reg, u32 val)
> +{
> +       if (dev->endian)
> +               iowrite32be(val, dev->base + reg);
> +       else
> +               iowrite32(val, dev->base + reg);
> +}
> +
> +static inline void ftm_counter_enable(struct ftm_rtc *rtc)
> +{
> +       u32 val;
> +
> +       /* select and enable counter clock source */
> +       val = rtc_readl(rtc, FTM_SC);
> +       val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
> +       val |= (FTM_SC_PS_MASK | FTM_SC_CLK(FTM_SC_CLKS_FIXED_FREQ));
> +       rtc_writel(rtc, FTM_SC, val);
> +}
> +
> +static inline void ftm_counter_disable(struct ftm_rtc *rtc)
> +{
> +       u32 val;
> +
> +       /* disable counter clock source */
> +       val = rtc_readl(rtc, FTM_SC);
> +       val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
> +       rtc_writel(rtc, FTM_SC, val);
> +}
> +
> +static inline void ftm_irq_acknowledge(struct ftm_rtc *rtc)
> +{
> +       unsigned int timeout = 100;
> +
> +       while ((FTM_SC_TOF & rtc_readl(rtc, FTM_SC)) && timeout--)
> +               rtc_writel(rtc, FTM_SC, rtc_readl(rtc, FTM_SC) & (~FTM_SC_TOF));

Is there a reason that we need to loop here?

> +}
> +
> +static inline void ftm_irq_enable(struct ftm_rtc *rtc)
> +{
> +       u32 val;
> +
> +       val = rtc_readl(rtc, FTM_SC);
> +       val |= FTM_SC_TOIE;
> +       rtc_writel(rtc, FTM_SC, val);
> +}
> +
> +static inline void ftm_irq_disable(struct ftm_rtc *rtc)
> +{
> +       u32 val;
> +
> +       val = rtc_readl(rtc, FTM_SC);
> +       val &= ~FTM_SC_TOIE;
> +       rtc_writel(rtc, FTM_SC, val);
> +}
> +
> +static inline void ftm_reset_counter(struct ftm_rtc *rtc)
> +{
> +       /*
> +        * The CNT register contains the FTM counter value.
> +        * Reset clears the CNT register. Writing any value to COUNT
> +        * updates the counter with its initial value, CNTIN.
> +        */
> +       rtc_writel(rtc, FTM_CNT, 0x00);
> +}
> +
> +static void ftm_clean_alarm(struct ftm_rtc *rtc)
> +{
> +       ftm_counter_disable(rtc);
> +
> +       rtc_writel(rtc, FTM_CNTIN, 0x00);
> +       rtc_writel(rtc, FTM_MOD, ~0U);
> +
> +       ftm_reset_counter(rtc);
> +}
> +
> +static irqreturn_t ftm_rtc_alarm_interrupt(int irq, void *dev)
> +{
> +       struct ftm_rtc *rtc = dev;
> +
> +       ftm_irq_acknowledge(rtc);
> +       ftm_irq_disable(rtc);
> +       ftm_clean_alarm(rtc);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +static int ftm_rtc_alarm_irq_enable(struct device *dev,
> +               unsigned int enabled)
> +{
> +       struct ftm_rtc *rtc = dev_get_drvdata(dev);
> +
> +       if (enabled)
> +               ftm_irq_enable(rtc);
> +       else
> +               ftm_irq_disable(rtc);
> +
> +       return 0;
> +}
> +
> +/*
> + * Note:
> + *     The function is not really getting time from the RTC
> + *     since FlexTimer is not a RTC device, but we need to
> + *     get time to setup alarm, so we are using system time
> + *     for now.
> + */
> +static int ftm_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +       struct timespec64 ts64;
> +       unsigned long local_time;
> +
> +       ktime_get_real_ts64(&ts64);
> +       local_time = (unsigned long)(ts64.tv_sec - (sys_tz.tz_minuteswest * 60));
> +
> +       rtc_time_to_tm(local_time, tm);
> +
> +       return 0;
> +}

New line here.

> +static int ftm_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +       return 0;

Why we cannot read the alarm time?

> +}
> +
> +/*250Hz, 65536 / 250 = 262 second max*/

Can you make the comment more readable?

> +static int ftm_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm)
> +{
> +       struct rtc_time tm;
> +       unsigned long now, alm_time, cycle;
> +       struct ftm_rtc *rtc = dev_get_drvdata(dev);
> +
> +       ftm_rtc_read_time(dev, &tm);
> +       rtc_tm_to_time(&tm, &now);
> +       rtc_tm_to_time(&alm->time, &alm_time);
> +
> +       ftm_clean_alarm(rtc);
> +       cycle = (alm_time - now) * rtc->alarm_freq;
> +       if (cycle > MAX_COUNT_VAL) {
> +               pr_err("Out of alarm range.\n");

Better to tell the users what the range is than letting them try out.

> +               return -EINVAL;
> +       }
> +
> +       ftm_irq_disable(rtc);
> +
> +       /*
> +        * The counter increments until the value of MOD is reached,
> +        * at which point the counter is reloaded with the value of CNTIN.
> +        * The TOF (the overflow flag) bit is set when the FTM counter
> +        * changes from MOD to CNTIN. So we should using the cycle - 1.
> +        */
> +       rtc_writel(rtc, FTM_MOD, cycle - 1);
> +
> +       ftm_counter_enable(rtc);
> +       ftm_irq_enable(rtc);
> +
> +       return 0;
> +
> +}
> +
> +static const struct rtc_class_ops ftm_rtc_ops = {
> +       .read_time              = ftm_rtc_read_time,
> +       .read_alarm             = ftm_rtc_read_alarm,
> +       .set_alarm              = ftm_rtc_set_alarm,
> +       .alarm_irq_enable       = ftm_rtc_alarm_irq_enable,
> +};

New line is needed.

> +static int ftm_rtc_probe(struct platform_device *pdev)
> +{
> +       struct device_node *np = pdev->dev.of_node;
> +       struct resource *r;
> +       int irq;
> +       int ret;
> +       struct ftm_rtc *rtc;
> +
> +       rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
> +       if (unlikely(!rtc)) {
> +               pr_err("ftm: cannot alloc memery for rtc\n");
> +               return -ENOMEM;
> +       }
> +
> +

No two new lines.

> +       platform_set_drvdata(pdev, rtc);
> +
> +       r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +       if (!r) {
> +               pr_err("ftm: cannot get resource for rtc\n");
> +               return -ENODEV;
> +       }
> +
> +       rtc->base = devm_ioremap_resource(&pdev->dev, r);
> +       if (IS_ERR(rtc->base)) {
> +               pr_err("ftm: cannot ioremap resource for rtc\n");
> +               return PTR_ERR(rtc->base);
> +       }
> +
> +       irq = irq_of_parse_and_map(np, 0);
> +       if (irq <= 0) {
> +               pr_err("ftm: unable to get IRQ from DT, %d\n", irq);
> +               return -EINVAL;
> +       }
> +
> +       rtc->endian = of_property_read_bool(np, "big-endian");
> +
> +       ret = devm_request_irq(&pdev->dev, irq, ftm_rtc_alarm_interrupt,
> +                              IRQF_NO_SUSPEND, dev_name(&pdev->dev), rtc);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "failed to request irq\n");
> +               return ret;
> +       }
> +
> +       rtc->alarm_freq = (u32)FIXED_FREQ_CLK / (u32)MAX_FREQ_DIV;
> +
> +       ftm_clean_alarm(rtc);
> +
> +       device_init_wakeup(&pdev->dev, true);
> +       rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "ftm-alarm",
> +                                                       &ftm_rtc_ops,
> +                                                       THIS_MODULE);
> +       if (IS_ERR(rtc->rtc_dev)) {
> +               dev_err(&pdev->dev, "can't register rtc device\n");
> +               return PTR_ERR(rtc->rtc_dev);
> +       }
> +       return ret;
> +}
> +
> +static const struct of_device_id ftm_rtc_match[] = {
> +       { .compatible = "fsl,ftm-alarm", },
> +       { .compatible = "fsl,ls1012a-ftm-alarm", },
> +       { .compatible = "fsl,ls1021a-ftm-alarm", },
> +       { .compatible = "fsl,ls1043a-ftm-alarm", },
> +       { .compatible = "fsl,ls1046a-ftm-alarm", },
> +       { .compatible = "fsl,ls1088a-ftm-alarm", },
> +       { .compatible = "fsl,ls208xa-ftm-alarm", },
> +       { .compatible = "fsl,ls1028a-ftm-alarm", },
> +       { },
> +};
> +
> +static struct platform_driver ftm_rtc_driver = {
> +       .probe          = ftm_rtc_probe,
> +       .driver         = {
> +               .name   = "ftm-alarm",
> +               .of_match_table = ftm_rtc_match,
> +       },
> +};
> +
> +static int __init ftm_alarm_init(void)
> +{
> +       return platform_driver_register(&ftm_rtc_driver);
> +}
> +
> +/***************

This is not standard Linux multiline comment style.

> + *Ensure that the driver is initialized after
> + *any real rtc driver

Can you explain why it has to be initialized later than other rtc drivers?

> + */
> +device_initcall_sync(ftm_alarm_init);
> +
> +MODULE_DESCRIPTION("NXP/Freescale FlexTimer alarm driver");
> +MODULE_AUTHOR("Biwen Li <biwen.li@nxp.com>");
> +MODULE_LICENSE("GPL");
> --
> 2.7.4
>

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

* RE: [EXT] Re: [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver
  2019-07-10 20:26     ` Rob Herring
@ 2019-07-11 11:04       ` Biwen Li
  0 siblings, 0 replies; 7+ messages in thread
From: Biwen Li @ 2019-07-11 11:04 UTC (permalink / raw)
  To: Rob Herring, Leo Li
  Cc: Alessandro Zummo, Alexandre Belloni,
	open list:REAL TIME CLOCK (RTC) SUBSYSTEM, lkml, Xiaobo Xie,
	Jiafei Pan, Ran Wang

-----Original Message-----
From: Rob Herring <robh+dt@kernel.org> 
Sent: 2019年7月11日 4:26
To: Leo Li <leoyang.li@nxp.com>
Cc: Biwen Li <biwen.li@nxp.com>; Alessandro Zummo <a.zummo@towertech.it>; Alexandre Belloni <alexandre.belloni@bootlin.com>; open list:REAL TIME CLOCK (RTC) SUBSYSTEM <linux-rtc@vger.kernel.org>; lkml <linux-kernel@vger.kernel.org>; Xiaobo Xie <xiaobo.xie@nxp.com>; Jiafei Pan <jiafei.pan@nxp.com>; Ran Wang <ran.wang_1@nxp.com>
Subject: [EXT] Re: [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver

Caution: EXT Email

On Wed, Jul 10, 2019 at 1:46 PM Li Yang <leoyang.li@nxp.com> wrote:
>
> On Wed, Jul 10, 2019 at 6:35 AM Biwen Li <biwen.li@nxp.com> wrote:
> >
> > The patch adds binding for ftm alarm driver
> >
> > Signed-off-by: Biwen Li <biwen.li@nxp.com>
>
> Looks like I commented the older version just now.  Adding Rob to this 
> version too.

More importantly, re-send the patch to the DT list so patchwork tracks it.
[Biwen Li] ok, I will send the patch to the DT list in v3.

>
> > ---
> > Change in v2:
> >     - replace ls1043a with ls1088a as example
> >     - add rcpm node and fsl,rcpm-wakeup property
> >
> >  .../devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt  | 40 
> > ++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> >  create mode 100644 
> > Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> >
> > diff --git 
> > a/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt 
> > b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> > new file mode 100644
> > index 0000000..010984a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/rtc/rtc-fsl-ftm-alarm.txt
> > @@ -0,0 +1,40 @@
> > +Freescale FlexTimer Module (FTM) Alarm
> > +
> > +Note: The driver need work with RCPM driver to wake up system in sleep.
> > +
> > +Required properties:
> > +
> > +- compatible : Should be "fsl,ftm-alarm" or "fsl,<chip>-ftm-alarm", 
> > +the

fsl,ftm-alarm should be a fallback, not on its own.
[Biwen Li] ok,I will delete "fsl,ftm-alarm" in v3.

> > +              supported chips include
> > +              "fsl,ls1012a-ftm-alarm"
> > +              "fsl,ls1021a-ftm-alarm"
> > +              "fsl,ls1028a-ftm-alarm"
> > +              "fsl,ls1043a-ftm-alarm"
> > +              "fsl,ls1046a-ftm-alarm"
> > +              "fsl,ls1088a-ftm-alarm"
> > +              "fsl,ls208xa-ftm-alarm"
> > +- reg : Specifies base physical address and size of the register 
> > +sets for the
> > +  FlexTimer Module and base physical address of IP Powerdown 
> > +Exception Control
> > +  Register.
> > +- reg-names: names of the mapped memory regions listed in regs property.
> > +  should include the following entries:
> > +  "ftm":    Address of the register sets for FlexTimer Module

Says required, but not in the example. I'd just remove this as -names is pointless when there is only 1 entry.
[Biwen Li] I will remove the reg-names property in v3.

> > +- interrupts : Should be the FlexTimer Module interrupt.
> > +- fsl,rcpm-wakeup property and rcpm node : Please refer
> > +       Documentation/devicetree/bindings/soc/fsl/rcpm.txt
>
> Looks better.
>
> > +- big-endian: If the host controller is big-endian mode, specify this property.
> > +  The default endian mode is little-endian.
>
> Same comment about optional property.
> [Biwen Li] ok, I will move big-endian to optional property in v3.  
>
> > +
> > +Example:
> > +rcpm: rcpm@1e34050 {
> > +       compatible = "fsl,ls1088a-rcpm", "fsl,qoriq-rcpm-2.1+";
> > +       reg = <0x0 0x1e34050 0x0 0x4>;
> > +       fsl,#rcpm-wakeup-cells = <1>;

1 cell here...
[Biwen Li] yes, it’s wrong,I will correct it in v3.
> > +}
> > +
> > +ftm_alarm0: timer@2800000 {
> > +       compatible = "fsl,ftm-alarm";
> > +       reg = <0x0 0x2800000 0x0 0x10000>;
> > +       fsl,rcpm-wakeup = <&rcpm 0x0 0x4000>;

...and 2 cells here.
[Biwen Li] yes, it’s wrong,I will correct it in v3.

> > +       interrupts = <0 44 4>;
> > +}
> > --
> > 2.7.4
> >

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

* RE: [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source
  2019-07-10 11:04 [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Biwen Li
  2019-07-10 11:04 ` [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver Biwen Li
  2019-07-10 21:11 ` [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Li Yang
@ 2019-07-11 14:12 ` Biwen Li
  2 siblings, 0 replies; 7+ messages in thread
From: Biwen Li @ 2019-07-11 14:12 UTC (permalink / raw)
  To: Biwen Li, a.zummo, alexandre.belloni, Leo Li
  Cc: linux-rtc, linux-kernel, Xiaobo Xie, Jiafei Pan, Ran Wang

-----Original Message-----
From: Biwen Li <biwen.li@nxp.com> 
Sent: 2019年7月10日 19:04
To: a.zummo@towertech.it; alexandre.belloni@bootlin.com; Leo Li <leoyang.li@nxp.com>
Cc: linux-rtc@vger.kernel.org; linux-kernel@vger.kernel.org; Xiaobo Xie <xiaobo.xie@nxp.com>; Jiafei Pan <jiafei.pan@nxp.com>; Ran Wang <ran.wang_1@nxp.com>; Biwen Li <biwen.li@nxp.com>
Subject: [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source

For the paltforms including LS1012A, LS1021A, LS1028A, LS1043A, LS1046A, LS1088A, LS208xA that has the FlexTimer module, implementing alarm functions within RTC subsystem to wakeup the system when system going to sleep (work with RCPM driver).

Signed-off-by: Biwen Li <biwen.li@nxp.com>
---

Change in v2:
    - remove code about setting rcpm

 drivers/rtc/Kconfig             |  14 ++
 drivers/rtc/Makefile            |   1 +
 drivers/rtc/rtc-fsl-ftm-alarm.c | 311 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 326 insertions(+)
 create mode 100644 drivers/rtc/rtc-fsl-ftm-alarm.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig index 03b60d5..0758a08 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1313,6 +1313,20 @@ config RTC_DRV_IMXDI
 	   This driver can also be built as a module, if so, the module
 	   will be called "rtc-imxdi".
 
+config RTC_DRV_FSL_FTM_ALARM
+	tristate "Freescale FlexTimer alarm timer"
+	depends on ARCH_LAYERSCAPE
+	default y
+	help
+	   For the FlexTimer in LS1012A, LS1021A, LS1028A, LS1043A, LS1046A,
+	   LS1088A, LS208xA, we can use FTM as the wakeup source.
+
+	   Say y here to enable FTM alarm support. The FTM alarm provides
+	   alarm functions for wakeup system from deep sleep.
+
+	   This driver can also be built as a module, if so, the module
+	   will be called "rtc-fsl-ftm-alarm".
+
 config RTC_DRV_MESON
 	tristate "Amlogic Meson RTC"
 	depends on (ARM && ARCH_MESON) || COMPILE_TEST diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index 9d997fa..5cccb07 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -79,6 +79,7 @@ obj-$(CONFIG_RTC_DRV_HID_SENSOR_TIME) += rtc-hid-sensor-time.o
 obj-$(CONFIG_RTC_DRV_HYM8563)	+= rtc-hym8563.o
 obj-$(CONFIG_RTC_DRV_IMXDI)	+= rtc-imxdi.o
 obj-$(CONFIG_RTC_DRV_IMX_SC)	+= rtc-imx-sc.o
+obj-$(CONFIG_RTC_DRV_FSL_FTM_ALARM)	+= rtc-fsl-ftm-alarm.o
 obj-$(CONFIG_RTC_DRV_ISL12022)	+= rtc-isl12022.o
 obj-$(CONFIG_RTC_DRV_ISL12026)	+= rtc-isl12026.o
 obj-$(CONFIG_RTC_DRV_ISL1208)	+= rtc-isl1208.o
diff --git a/drivers/rtc/rtc-fsl-ftm-alarm.c b/drivers/rtc/rtc-fsl-ftm-alarm.c new file mode 100644 index 0000000..1836c2e
--- /dev/null
+++ b/drivers/rtc/rtc-fsl-ftm-alarm.c
@@ -0,0 +1,311 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Freescale FlexTimer Module (FTM) alarm device driver.
+ *
+ * Copyright 2014 Freescale Semiconductor, Inc.
+ * Copyright 2019 NXP
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.

No need to add this with the SPDX tag.  And btw, the text here is
GPL-2.0+ instead of GPL-2.0.
[Biwen Li] ok,I will delete this and replace GPL-2.0 with GPL-2.0+ in v3.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/module.h>
+#include <linux/fsl/ftm.h>
+#include <linux/rtc.h>
+#include <linux/time.h>
+
+#define FTM_SC_CLK(c)		((c) << FTM_SC_CLK_MASK_SHIFT)
+
+/*
+ * Select Fixed frequency clock as clock source
+ * of FlexTimer Module
+ */
+#define FTM_SC_CLKS_FIXED_FREQ	0x02
+
+#define FIXED_FREQ_CLK		32000
+#define MAX_FREQ_DIV		(1 << FTM_SC_PS_MASK)
+#define MAX_COUNT_VAL		0xffff
+
+struct ftm_rtc {
+	struct rtc_device *rtc_dev;
+	void __iomem *base;
+	bool endian;

Make it big_endian to be more clear.
[Biwen Li] yes, you are right.
+	u32 alarm_freq;
+};
+
+static inline u32 rtc_readl(struct ftm_rtc *dev, u32 reg) {
+	if (dev->endian)
+		return ioread32be(dev->base + reg); /*big endianness*/

No need to have the extra comment with new variable name.

And probably better to use "else" here to make clearer to
compiler/static analyser.
[Biwen Li] ok, I will remove the extra comment and add else branch in v3.

+
+	return ioread32(dev->base + reg);
+}
+
+static inline void rtc_writel(struct ftm_rtc *dev, u32 reg, u32 val) {
+	if (dev->endian)
+		iowrite32be(val, dev->base + reg);
+	else
+		iowrite32(val, dev->base + reg);
+}
+
+static inline void ftm_counter_enable(struct ftm_rtc *rtc) {
+	u32 val;
+
+	/* select and enable counter clock source */
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
+	val |= (FTM_SC_PS_MASK | FTM_SC_CLK(FTM_SC_CLKS_FIXED_FREQ));
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_counter_disable(struct ftm_rtc *rtc) {
+	u32 val;
+
+	/* disable counter clock source */
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~(FTM_SC_PS_MASK | FTM_SC_CLK_MASK);
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_irq_acknowledge(struct ftm_rtc *rtc) {
+	unsigned int timeout = 100;
+
+	while ((FTM_SC_TOF & rtc_readl(rtc, FTM_SC)) && timeout--)
+		rtc_writel(rtc, FTM_SC, rtc_readl(rtc, FTM_SC) & (~FTM_SC_TOF)); 

Is there a reason that we need to loop here?
[Biwen Li] It is used to fix errata A-007728 for flextimer.i will add a reason for it in v3. 
[Biwen Li] If the FTM counter reaches the FTM_MOD value between the reading of the
[Biwen Li] TOF bit and the writing of 0 to the TOF bit, the process of clearing the
[Biwen Li] TOF bit does not work as expected when FTMx_CONF[NUMTOF] != 0 and the
[Biwen Li] current TOF count is less than FTMx_CONF[NUMTOF]. If the above condition
[Biwen Li] is met, the TOF bit remains set. If the TOF interrupt is enabled
[Biwen Li] (FTMx_SC[TOIE] = 1), the TOF interrupt also remains asserted.

[Biwen Li] Above is the errata discription

[Biwen Li] In one word: software clearing TOF bit not works when FTMx_CONF[NUMTOF]
[Biwen Li] was seted as nonzero and FTM counter reaches the FTM_MOD value.

[Biwen Li] The workaround is clearing TOF bit until it works(FTM counter doesn't
[Biwen Li] always reache the FTM_MOD anyway),which may cost some cycles.
 
[Biwen Li] More info is here https://lore.kernel.org/patchwork/patch/782120/
+}
+
+static inline void ftm_irq_enable(struct ftm_rtc *rtc) {
+	u32 val;
+
+	val = rtc_readl(rtc, FTM_SC);
+	val |= FTM_SC_TOIE;
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_irq_disable(struct ftm_rtc *rtc) {
+	u32 val;
+
+	val = rtc_readl(rtc, FTM_SC);
+	val &= ~FTM_SC_TOIE;
+	rtc_writel(rtc, FTM_SC, val);
+}
+
+static inline void ftm_reset_counter(struct ftm_rtc *rtc) {
+	/*
+	 * The CNT register contains the FTM counter value.
+	 * Reset clears the CNT register. Writing any value to COUNT
+	 * updates the counter with its initial value, CNTIN.
+	 */
+	rtc_writel(rtc, FTM_CNT, 0x00);
+}
+
+static void ftm_clean_alarm(struct ftm_rtc *rtc) {
+	ftm_counter_disable(rtc);
+
+	rtc_writel(rtc, FTM_CNTIN, 0x00);
+	rtc_writel(rtc, FTM_MOD, ~0U);
+
+	ftm_reset_counter(rtc);
+}
+
+static irqreturn_t ftm_rtc_alarm_interrupt(int irq, void *dev) {
+	struct ftm_rtc *rtc = dev;
+
+	ftm_irq_acknowledge(rtc);
+	ftm_irq_disable(rtc);
+	ftm_clean_alarm(rtc);
+
+	return IRQ_HANDLED;
+}
+
+static int ftm_rtc_alarm_irq_enable(struct device *dev,
+		unsigned int enabled)
+{
+	struct ftm_rtc *rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		ftm_irq_enable(rtc);
+	else
+		ftm_irq_disable(rtc);
+
+	return 0;
+}
+
+/*
+ * Note:
+ *	The function is not really getting time from the RTC
+ *	since FlexTimer is not a RTC device, but we need to
+ *	get time to setup alarm, so we are using system time
+ *	for now.
+ */
+static int ftm_rtc_read_time(struct device *dev, struct rtc_time *tm) {
+	struct timespec64 ts64;
+	unsigned long local_time;
+
+	ktime_get_real_ts64(&ts64);
+	local_time = (unsigned long)(ts64.tv_sec - (sys_tz.tz_minuteswest * 
+60));
+
+	rtc_time_to_tm(local_time, tm);
+
+	return 0;
+}
New line here.
[Biwen Li] yes. I will add new line here in v3. 
+static int ftm_rtc_read_alarm(struct device *dev, struct rtc_wkalrm 
+*alm) {
+	return 0;
Why we cannot read the alarm time?
[Biwen Li] The FlexTimer is not a real rtc device.it don't have alarm registers of rtc.
+}
+
+/*250Hz, 65536 / 250 = 262 second max*/ 
Can you make the comment more readable?
[Biwen Li] yes,I will add some comment in v3.
+static int ftm_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alm) {
+	struct rtc_time tm;
+	unsigned long now, alm_time, cycle;
+	struct ftm_rtc *rtc = dev_get_drvdata(dev);
+
+	ftm_rtc_read_time(dev, &tm);
+	rtc_tm_to_time(&tm, &now);
+	rtc_tm_to_time(&alm->time, &alm_time);
+
+	ftm_clean_alarm(rtc);
+	cycle = (alm_time - now) * rtc->alarm_freq;
+	if (cycle > MAX_COUNT_VAL) {
+		pr_err("Out of alarm range.\n");
Better to tell the users what the range is than letting them try out.
[Biwen Li] ok, I will add the range in v3.
+		return -EINVAL;
+	}
+
+	ftm_irq_disable(rtc);
+
+	/*
+	 * The counter increments until the value of MOD is reached,
+	 * at which point the counter is reloaded with the value of CNTIN.
+	 * The TOF (the overflow flag) bit is set when the FTM counter
+	 * changes from MOD to CNTIN. So we should using the cycle - 1.
+	 */
+	rtc_writel(rtc, FTM_MOD, cycle - 1);
+
+	ftm_counter_enable(rtc);
+	ftm_irq_enable(rtc);
+
+	return 0;
+
+}
+
+static const struct rtc_class_ops ftm_rtc_ops = {
+	.read_time		= ftm_rtc_read_time,
+	.read_alarm		= ftm_rtc_read_alarm,
+	.set_alarm		= ftm_rtc_set_alarm,
+	.alarm_irq_enable	= ftm_rtc_alarm_irq_enable,
+};
New line is needed.
[Biwen Li] ok, I will add new line here in v3.
+static int ftm_rtc_probe(struct platform_device *pdev) {
+	struct device_node *np = pdev->dev.of_node;
+	struct resource *r;
+	int irq;
+	int ret;
+	struct ftm_rtc *rtc;
+
+	rtc = devm_kzalloc(&pdev->dev, sizeof(*rtc), GFP_KERNEL);
+	if (unlikely(!rtc)) {
+		pr_err("ftm: cannot alloc memery for rtc\n");
+		return -ENOMEM;
+	}
+
+
No two new lines.
[Biwen Li] I will remove one line here in v3.
+	platform_set_drvdata(pdev, rtc);
+
+	r = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!r) {
+		pr_err("ftm: cannot get resource for rtc\n");
+		return -ENODEV;
+	}
+
+	rtc->base = devm_ioremap_resource(&pdev->dev, r);
+	if (IS_ERR(rtc->base)) {
+		pr_err("ftm: cannot ioremap resource for rtc\n");
+		return PTR_ERR(rtc->base);
+	}
+
+	irq = irq_of_parse_and_map(np, 0);
+	if (irq <= 0) {
+		pr_err("ftm: unable to get IRQ from DT, %d\n", irq);
+		return -EINVAL;
+	}
+
+	rtc->endian = of_property_read_bool(np, "big-endian");
+
+	ret = devm_request_irq(&pdev->dev, irq, ftm_rtc_alarm_interrupt,
+			       IRQF_NO_SUSPEND, dev_name(&pdev->dev), rtc);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to request irq\n");
+		return ret;
+	}
+
+	rtc->alarm_freq = (u32)FIXED_FREQ_CLK / (u32)MAX_FREQ_DIV;
+
+	ftm_clean_alarm(rtc);
+
+	device_init_wakeup(&pdev->dev, true);
+	rtc->rtc_dev = devm_rtc_device_register(&pdev->dev, "ftm-alarm",
+							&ftm_rtc_ops,
+							THIS_MODULE);
+	if (IS_ERR(rtc->rtc_dev)) {
+		dev_err(&pdev->dev, "can't register rtc device\n");
+		return PTR_ERR(rtc->rtc_dev);
+	}
+	return ret;
+}
+
+static const struct of_device_id ftm_rtc_match[] = {
+	{ .compatible = "fsl,ftm-alarm", },
+	{ .compatible = "fsl,ls1012a-ftm-alarm", },
+	{ .compatible = "fsl,ls1021a-ftm-alarm", },
+	{ .compatible = "fsl,ls1043a-ftm-alarm", },
+	{ .compatible = "fsl,ls1046a-ftm-alarm", },
+	{ .compatible = "fsl,ls1088a-ftm-alarm", },
+	{ .compatible = "fsl,ls208xa-ftm-alarm", },
+	{ .compatible = "fsl,ls1028a-ftm-alarm", },
+	{ },
+};
+
+static struct platform_driver ftm_rtc_driver = {
+	.probe		= ftm_rtc_probe,
+	.driver		= {
+		.name	= "ftm-alarm",
+		.of_match_table = ftm_rtc_match,
+	},
+};
+
+static int __init ftm_alarm_init(void)
+{
+	return platform_driver_register(&ftm_rtc_driver);
+}
+
+/***************
This is not standard Linux multiline comment style.
[Biwen Li] ok, I will correct it in v3.
+ *Ensure that the driver is initialized after
*any real rtc driver
Can you explain why it has to be initialized later than other rtc drivers?
[Biwen Li] yes, I will add comment to explain this in v3.
[Biwen Li]		- The flextimer is not a real rtc device,it don't have time and date registers of rtc . 
[Biwen Li] 	- The flextimer rtc alarm driver gets time from wall time,
[Biwen Li]		 but the wall time is not ready.so the time from the driver is wrong.
[Biwen Li]		- If system regist it before any other real rtc device,it will be emulated as rtc0,
[Biwen Li]		 date command will read wrong time for user.
*/
+device_initcall_sync(ftm_alarm_init);
+
+MODULE_DESCRIPTION("NXP/Freescale FlexTimer alarm driver"); 
+MODULE_AUTHOR("Biwen Li <biwen.li@nxp.com>"); MODULE_LICENSE("GPL");
--
2.7.4


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

end of thread, other threads:[~2019-07-11 14:12 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-10 11:04 [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Biwen Li
2019-07-10 11:04 ` [v2,2/2] Documentation: dt: binding: rtc: add binding for ftm alarm driver Biwen Li
2019-07-10 19:45   ` Li Yang
2019-07-10 20:26     ` Rob Herring
2019-07-11 11:04       ` [EXT] " Biwen Li
2019-07-10 21:11 ` [v2,1/2] rtc/fsl: add FTM alarm driver as the wakeup source Li Yang
2019-07-11 14:12 ` Biwen Li

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