[PATCH 0/4] rtc: mediatek: add support for SoC based RTC on MT7622

[PATCH 0/4] rtc: mediatek: add support for SoC based RTC on MT7622

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

This patchset introduces support for MediaTek SoC based real time clock
(RTC) Currently, the driver is already tested successfully with hwclock
and wakealarm on MT7622 SoC. And it should also be workable on other
similar MediaTek SoCs. And patch 3 is a distinct patch used to distinguish
between either SoC based or PMIC based RTC to avoid confusion for the RTC
selection for the target SoC.

Sean Wang (4):
  dt-bindings: rtc: mediatek: add bindings for MediaTek SoC based RTC
  rtc: mediatek: add driver for RTC on MT7622 SoC
  rtc: mediatek: enhance the description for MediaTek PMIC based RTC
  rtc: mediatek: update MAINTAINERS entry with MediaTek RTC driver

 .../devicetree/bindings/rtc/rtc-mediatek.txt       |  21 ++
 MAINTAINERS                                        |   3 +
 drivers/rtc/Kconfig                                |  18 +-
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-mediatek.c                         | 398 +++++++++++++++++++++
 5 files changed, 437 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
 create mode 100644 drivers/rtc/rtc-mediatek.c

-- 
2.7.4

[PATCH 0/4] rtc: mediatek: add support for SoC based RTC on MT7622

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

This patchset introduces support for MediaTek SoC based real time clock
(RTC) Currently, the driver is already tested successfully with hwclock
and wakealarm on MT7622 SoC. And it should also be workable on other
similar MediaTek SoCs. And patch 3 is a distinct patch used to distinguish
between either SoC based or PMIC based RTC to avoid confusion for the RTC
selection for the target SoC.

Sean Wang (4):
  dt-bindings: rtc: mediatek: add bindings for MediaTek SoC based RTC
  rtc: mediatek: add driver for RTC on MT7622 SoC
  rtc: mediatek: enhance the description for MediaTek PMIC based RTC
  rtc: mediatek: update MAINTAINERS entry with MediaTek RTC driver

 .../devicetree/bindings/rtc/rtc-mediatek.txt       |  21 ++
 MAINTAINERS                                        |   3 +
 drivers/rtc/Kconfig                                |  18 +-
 drivers/rtc/Makefile                               |   1 +
 drivers/rtc/rtc-mediatek.c                         | 398 +++++++++++++++++++++
 5 files changed, 437 insertions(+), 4 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
 create mode 100644 drivers/rtc/rtc-mediatek.c

-- 
2.7.4

[PATCH 1/4] dt-bindings: rtc: mediatek: add bindings for MediaTek SoC based RTC

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Add device-tree binding for MediaTek SoC based RTC

Cc: devicetree@vger.kernel.org
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../devicetree/bindings/rtc/rtc-mediatek.txt        | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mediatek.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt b/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
new file mode 100644
index 0000000..09fe8f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
@@ -0,0 +1,21 @@
+Device-Tree bindings for MediaTek SoC based RTC
+
+Required properties:
+- compatible	    : Should be
+			"mediatek,mt7622-rtc", "mediatek,soc-rtc" : for MT7622 SoC
+- reg 		    : Specifies base physical address and size of the registers;
+- interrupts	    : Should contain the interrupt for RTC alarm;
+- clocks	    : Specifies list of clock specifiers, corresponding to
+		      entries in clock-names property;
+- clock-names	    : Should contain "rtc" entries
+
+Example:
+
+rtc: rtc@10212800 {
+	compatible = "mediatek,mt7622-rtc",
+		     "mediatek,soc-rtc";
+	reg = <0 0x10212800 0 0x200>;
+	interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_LOW>;
+	clocks = <&topckgen CLK_TOP_RTC>;
+	clock-names = "rtc";
+};
-- 
2.7.4

[PATCH 1/4] dt-bindings: rtc: mediatek: add bindings for MediaTek SoC based RTC

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Add device-tree binding for MediaTek SoC based RTC

Cc: devicetree@vger.kernel.org
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 .../devicetree/bindings/rtc/rtc-mediatek.txt        | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mediatek.txt

diff --git a/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt b/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
new file mode 100644
index 0000000..09fe8f5
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
@@ -0,0 +1,21 @@
+Device-Tree bindings for MediaTek SoC based RTC
+
+Required properties:
+- compatible	    : Should be
+			"mediatek,mt7622-rtc", "mediatek,soc-rtc" : for MT7622 SoC
+- reg 		    : Specifies base physical address and size of the registers;
+- interrupts	    : Should contain the interrupt for RTC alarm;
+- clocks	    : Specifies list of clock specifiers, corresponding to
+		      entries in clock-names property;
+- clock-names	    : Should contain "rtc" entries
+
+Example:
+
+rtc: rtc@10212800 {
+	compatible = "mediatek,mt7622-rtc",
+		     "mediatek,soc-rtc";
+	reg = <0 0x10212800 0 0x200>;
+	interrupts = <GIC_SPI 129 IRQ_TYPE_LEVEL_LOW>;
+	clocks = <&topckgen CLK_TOP_RTC>;
+	clock-names = "rtc";
+};
-- 
2.7.4

Re: [PATCH 1/4] dt-bindings: rtc: mediatek: add bindings for MediaTek SoC based RTC

[Rob Herring]

On Fri, Sep 22, 2017 at 11:33:14AM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> Add device-tree binding for MediaTek SoC based RTC
> 
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  .../devicetree/bindings/rtc/rtc-mediatek.txt        | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/rtc/rtc-mediatek.txt

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

[PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

This patch introduces the driver for the RTC on MT7622 SoC.

Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/rtc/Kconfig        |  10 ++
 drivers/rtc/Makefile       |   1 +
 drivers/rtc/rtc-mediatek.c | 398 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+)
 create mode 100644 drivers/rtc/rtc-mediatek.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e0e58f3..4226295 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1705,6 +1705,16 @@ config RTC_DRV_MOXART
 	   This driver can also be built as a module. If so, the module
 	   will be called rtc-moxart
 
+config RTC_DRV_MEDIATEK
+	tristate "MediaTek SoC based RTC"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  This enables support for the real time clock built in the MediaTek
+	  SoCs.
+
+	  This drive can also be built as a module. If so, the module
+	  will be called rtc-mediatek.
+
 config RTC_DRV_MT6397
 	tristate "Mediatek Real Time Clock driver"
 	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 7230014..ba0206a 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
 obj-$(CONFIG_RTC_DRV_MPC5121)	+= rtc-mpc5121.o
 obj-$(CONFIG_RTC_DRV_VRTC)	+= rtc-mrst.o
 obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
+obj-$(CONFIG_RTC_DRV_MEDIATEK)	+= rtc-mediatek.o
 obj-$(CONFIG_RTC_DRV_MT6397)	+= rtc-mt6397.o
 obj-$(CONFIG_RTC_DRV_MV)	+= rtc-mv.o
 obj-$(CONFIG_RTC_DRV_MXC)	+= rtc-mxc.o
diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
new file mode 100644
index 0000000..c6c06fe
--- /dev/null
+++ b/drivers/rtc/rtc-mediatek.c
@@ -0,0 +1,398 @@
+/*
+ * Driver for MediaTek SoC based RTC
+ *
+ * Copyright (C) 2017 Sean Wang <sean.wang@mediatek.com>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#define MTK_RTC_DEV KBUILD_MODNAME
+
+#define MTK_RTC_PWRCHK1		0x4
+#define	RTC_PWRCHK1_MAGIC	0xc6
+
+#define MTK_RTC_PWRCHK2		0x8
+#define	RTC_PWRCHK2_MAGIC	0x9a
+
+#define MTK_RTC_KEY		0xc
+#define	RTC_KEY_MAGIC		0x59
+
+#define MTK_RTC_PROT1		0x10
+#define	RTC_PROT1_MAGIC		0xa3
+
+#define MTK_RTC_PROT2		0x14
+#define	RTC_PROT2_MAGIC		0x57
+
+#define MTK_RTC_PROT3		0x18
+#define	RTC_PROT3_MAGIC		0x67
+
+#define MTK_RTC_PROT4		0x1c
+#define	RTC_PROT4_MAGIC		0xd2
+
+#define MTK_RTC_CTL		0x20
+#define	RTC_RC_STOP		BIT(0)
+
+#define MTK_RTC_DEBNCE		0x2c
+#define	RTC_DEBNCE_MASK		GENMASK(2, 0)
+
+#define MTK_RTC_INT		0x30
+#define RTC_INT_AL_STA		BIT(4)
+
+/* Ranges from 0x40 to 0x78 provide RTC time setup for year, month,
+ * day of month, day of week, hour, minute and second.
+ */
+#define MTK_RTC_TREG(_t, _f)	(0x40 + (0x4 * (_f)) + ((_t) * 0x20))
+
+#define MTK_RTC_AL_CTL		0x7c
+#define	RTC_AL_EN		BIT(0)
+#define	RTC_AL_ALL		GENMASK(7, 0)
+
+/* Types of the function the RTC provides are time counter and alarm. */
+enum {
+	MTK_TC,
+	MTK_AL
+};
+
+/* Indexes are used for the pointer to relevant registers in MTK_RTC_TREG */
+enum {
+	MTK_YEA,
+	MTK_MON,
+	MTK_DOM,
+	MTK_DOW,
+	MTK_HOU,
+	MTK_MIN,
+	MTK_SEC
+};
+
+struct mtk_rtc {
+	struct rtc_device *rtc;
+	void __iomem *base;
+	int irq;
+	struct clk *clk;
+};
+
+static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	__raw_writel(val, rtc->base + reg);
+}
+
+static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
+{
+	return __raw_readl(rtc->base + reg);
+}
+
+static void mtk_rmw(struct mtk_rtc *rtc, u32 reg, u32 mask, u32 set)
+{
+	u32 val;
+
+	val = mtk_r32(rtc, reg);
+	val &= ~mask;
+	val |= set;
+	mtk_w32(rtc, reg, val);
+}
+
+static void mtk_set(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	mtk_rmw(rtc, reg, 0, val);
+}
+
+static void mtk_clr(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	mtk_rmw(rtc, reg, val, 0);
+}
+
+static void mtk_rtc_hw_init(struct mtk_rtc *hw)
+{
+	/* The setup of the init sequence is for allowing RTC got to work */
+	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
+	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
+	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
+	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
+	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
+}
+
+static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
+				      int time_alarm)
+{
+	u32 year, mon, mday, wday, hour, min, sec;
+
+	/*
+	 * Read again until all fields are not changed for all fields in the
+	 * consistent state.
+	 */
+	do {
+		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
+		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
+		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
+		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
+		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
+		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
+		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
+	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
+		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
+		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
+		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
+		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
+		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
+		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
+		);
+
+	tm->tm_sec  = sec;
+	tm->tm_min  = min;
+	tm->tm_hour = hour;
+	tm->tm_wday = wday;
+	tm->tm_mday = mday;
+	tm->tm_mon  = mon - 1;
+	tm->tm_year = year;
+	tm->tm_year += 100;
+}
+
+static void mtk_rtc_set_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
+				      int time_alarm)
+{
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA), tm->tm_year - 100);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_MON), tm->tm_mon + 1);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW), tm->tm_wday);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM), tm->tm_mday);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU), tm->tm_hour);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN), tm->tm_min);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC), tm->tm_sec);
+}
+
+static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
+{
+	struct mtk_rtc *hw = (struct mtk_rtc *)id;
+	u32 irq_sta;
+
+	/* Stop alarm also implicitly disable the alarm interrupt */
+	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
+	irq_sta = mtk_r32(hw, MTK_RTC_INT);
+	if (irq_sta & RTC_INT_AL_STA) {
+		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
+
+		/* Ack alarm interrupt status */
+		mtk_w32(hw, MTK_RTC_INT, RTC_INT_AL_STA);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
+
+	return rtc_valid_tm(tm);
+}
+
+static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	/* Stop time counter before setting a new one*/
+	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
+
+	/* Epoch == 1900 */
+	if (tm->tm_year < 100 || tm->tm_year > 199)
+		return -EINVAL;
+
+	mtk_rtc_set_alarm_or_time(hw, tm, MTK_TC);
+
+	/* Restart the time counter */
+	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
+
+	return 0;
+}
+
+static int mtk_rtc_getalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+
+	mtk_rtc_get_alarm_or_time(hw, alrm_tm, MTK_AL);
+
+	wkalrm->enabled = !!(mtk_r32(hw, MTK_RTC_AL_CTL) & RTC_AL_EN);
+	wkalrm->pending = !!(mtk_r32(hw, MTK_RTC_INT) & RTC_INT_AL_STA);
+
+	return 0;
+}
+
+static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+
+	/* Epoch == 1900 */
+	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
+		return -EINVAL;
+
+	/*
+	 * Stop the alarm also implicitly including disables interrupt before
+	 * setting a new one.
+	 */
+	mtk_clr(hw, MTK_RTC_AL_CTL, RTC_AL_EN);
+
+	/*
+	 * Avoid contention between mtk_rtc_setalarm and IRQ handler so that
+	 * disabling the interrupt and awaiting for pending IRQ handler to
+	 * complete.
+	 */
+	synchronize_irq(hw->irq);
+
+	mtk_rtc_set_alarm_or_time(hw, alrm_tm, MTK_AL);
+
+	/* Restart the alarm with the new setup */
+	mtk_w32(hw, MTK_RTC_AL_CTL, RTC_AL_ALL);
+
+	return 0;
+}
+
+static const struct rtc_class_ops mtk_rtc_ops = {
+	.read_time		= mtk_rtc_gettime,
+	.set_time		= mtk_rtc_settime,
+	.read_alarm		= mtk_rtc_getalarm,
+	.set_alarm		= mtk_rtc_setalarm,
+};
+
+static const struct of_device_id mtk_rtc_match[] = {
+	{ .compatible = "mediatek,mt7622-rtc" },
+	{ .compatible = "mediatek,soc-rtc" },
+	{},
+};
+
+static int mtk_rtc_probe(struct platform_device *pdev)
+{
+	struct mtk_rtc *hw;
+	struct resource *res;
+	int ret;
+
+	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
+	if (!hw)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, hw);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	hw->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(hw->base))
+		return PTR_ERR(hw->base);
+
+	hw->clk = devm_clk_get(&pdev->dev, "rtc");
+	if (IS_ERR(hw->clk)) {
+		dev_err(&pdev->dev, "No clock\n");
+		return PTR_ERR(hw->clk);
+	}
+
+	ret = clk_prepare_enable(hw->clk);
+	if (ret)
+		return ret;
+
+	hw->irq = platform_get_irq(pdev, 0);
+	if (hw->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ resource\n");
+		ret = hw->irq;
+		goto err;
+	}
+
+	ret = devm_request_irq(&pdev->dev, hw->irq, mtk_rtc_alarmirq,
+			       0, dev_name(&pdev->dev), hw);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't request IRQ\n");
+		goto err;
+	}
+
+	mtk_rtc_hw_init(hw);
+
+	device_init_wakeup(&pdev->dev, true);
+
+	hw->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
+					   &mtk_rtc_ops, THIS_MODULE);
+	if (IS_ERR(hw->rtc)) {
+		ret = PTR_ERR(hw->rtc);
+		dev_err(&pdev->dev, "Unable to register device\n");
+		goto err;
+	}
+
+	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
+
+	return 0;
+err:
+	clk_disable_unprepare(hw->clk);
+
+	return ret;
+}
+
+static int mtk_rtc_remove(struct platform_device *pdev)
+{
+	struct mtk_rtc *hw = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(hw->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_rtc_suspend(struct device *dev)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(hw->irq);
+
+	return 0;
+}
+
+static int mtk_rtc_resume(struct device *dev)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(hw->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mtk_rtc_pm_ops, mtk_rtc_suspend, mtk_rtc_resume);
+
+#define MTK_RTC_PM_OPS (&mtk_rtc_pm_ops)
+#else	/* CONFIG_PM */
+#define MTK_RTC_PM_OPS NULL
+#endif	/* CONFIG_PM */
+
+static struct platform_driver mtk_rtc_driver = {
+	.probe	= mtk_rtc_probe,
+	.remove	= mtk_rtc_remove,
+	.driver = {
+		.name = MTK_RTC_DEV,
+		.of_match_table = mtk_rtc_match,
+		.pm = MTK_RTC_PM_OPS,
+	},
+};
+
+module_platform_driver(mtk_rtc_driver);
+
+MODULE_DESCRIPTION("MediaTek SoC based RTC Driver");
+MODULE_AUTHOR("Sean Wang <sean.wang@mediatek.com>");
+MODULE_LICENSE("GPL");
-- 
2.7.4

[PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[sean.wang-NuS5LvNUpcJWk0Htik3J/w]

From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

This patch introduces the driver for the RTC on MT7622 SoC.

Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
---
 drivers/rtc/Kconfig        |  10 ++
 drivers/rtc/Makefile       |   1 +
 drivers/rtc/rtc-mediatek.c | 398 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 409 insertions(+)
 create mode 100644 drivers/rtc/rtc-mediatek.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index e0e58f3..4226295 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1705,6 +1705,16 @@ config RTC_DRV_MOXART
 	   This driver can also be built as a module. If so, the module
 	   will be called rtc-moxart
 
+config RTC_DRV_MEDIATEK
+	tristate "MediaTek SoC based RTC"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	help
+	  This enables support for the real time clock built in the MediaTek
+	  SoCs.
+
+	  This drive can also be built as a module. If so, the module
+	  will be called rtc-mediatek.
+
 config RTC_DRV_MT6397
 	tristate "Mediatek Real Time Clock driver"
 	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 7230014..ba0206a 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -101,6 +101,7 @@ obj-$(CONFIG_RTC_DRV_MOXART)	+= rtc-moxart.o
 obj-$(CONFIG_RTC_DRV_MPC5121)	+= rtc-mpc5121.o
 obj-$(CONFIG_RTC_DRV_VRTC)	+= rtc-mrst.o
 obj-$(CONFIG_RTC_DRV_MSM6242)	+= rtc-msm6242.o
+obj-$(CONFIG_RTC_DRV_MEDIATEK)	+= rtc-mediatek.o
 obj-$(CONFIG_RTC_DRV_MT6397)	+= rtc-mt6397.o
 obj-$(CONFIG_RTC_DRV_MV)	+= rtc-mv.o
 obj-$(CONFIG_RTC_DRV_MXC)	+= rtc-mxc.o
diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
new file mode 100644
index 0000000..c6c06fe
--- /dev/null
+++ b/drivers/rtc/rtc-mediatek.c
@@ -0,0 +1,398 @@
+/*
+ * Driver for MediaTek SoC based RTC
+ *
+ * Copyright (C) 2017 Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
+ *
+ * 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.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/clk.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+
+#define MTK_RTC_DEV KBUILD_MODNAME
+
+#define MTK_RTC_PWRCHK1		0x4
+#define	RTC_PWRCHK1_MAGIC	0xc6
+
+#define MTK_RTC_PWRCHK2		0x8
+#define	RTC_PWRCHK2_MAGIC	0x9a
+
+#define MTK_RTC_KEY		0xc
+#define	RTC_KEY_MAGIC		0x59
+
+#define MTK_RTC_PROT1		0x10
+#define	RTC_PROT1_MAGIC		0xa3
+
+#define MTK_RTC_PROT2		0x14
+#define	RTC_PROT2_MAGIC		0x57
+
+#define MTK_RTC_PROT3		0x18
+#define	RTC_PROT3_MAGIC		0x67
+
+#define MTK_RTC_PROT4		0x1c
+#define	RTC_PROT4_MAGIC		0xd2
+
+#define MTK_RTC_CTL		0x20
+#define	RTC_RC_STOP		BIT(0)
+
+#define MTK_RTC_DEBNCE		0x2c
+#define	RTC_DEBNCE_MASK		GENMASK(2, 0)
+
+#define MTK_RTC_INT		0x30
+#define RTC_INT_AL_STA		BIT(4)
+
+/* Ranges from 0x40 to 0x78 provide RTC time setup for year, month,
+ * day of month, day of week, hour, minute and second.
+ */
+#define MTK_RTC_TREG(_t, _f)	(0x40 + (0x4 * (_f)) + ((_t) * 0x20))
+
+#define MTK_RTC_AL_CTL		0x7c
+#define	RTC_AL_EN		BIT(0)
+#define	RTC_AL_ALL		GENMASK(7, 0)
+
+/* Types of the function the RTC provides are time counter and alarm. */
+enum {
+	MTK_TC,
+	MTK_AL
+};
+
+/* Indexes are used for the pointer to relevant registers in MTK_RTC_TREG */
+enum {
+	MTK_YEA,
+	MTK_MON,
+	MTK_DOM,
+	MTK_DOW,
+	MTK_HOU,
+	MTK_MIN,
+	MTK_SEC
+};
+
+struct mtk_rtc {
+	struct rtc_device *rtc;
+	void __iomem *base;
+	int irq;
+	struct clk *clk;
+};
+
+static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	__raw_writel(val, rtc->base + reg);
+}
+
+static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
+{
+	return __raw_readl(rtc->base + reg);
+}
+
+static void mtk_rmw(struct mtk_rtc *rtc, u32 reg, u32 mask, u32 set)
+{
+	u32 val;
+
+	val = mtk_r32(rtc, reg);
+	val &= ~mask;
+	val |= set;
+	mtk_w32(rtc, reg, val);
+}
+
+static void mtk_set(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	mtk_rmw(rtc, reg, 0, val);
+}
+
+static void mtk_clr(struct mtk_rtc *rtc, u32 reg, u32 val)
+{
+	mtk_rmw(rtc, reg, val, 0);
+}
+
+static void mtk_rtc_hw_init(struct mtk_rtc *hw)
+{
+	/* The setup of the init sequence is for allowing RTC got to work */
+	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
+	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
+	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
+	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
+	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
+	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
+}
+
+static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
+				      int time_alarm)
+{
+	u32 year, mon, mday, wday, hour, min, sec;
+
+	/*
+	 * Read again until all fields are not changed for all fields in the
+	 * consistent state.
+	 */
+	do {
+		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
+		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
+		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
+		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
+		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
+		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
+		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
+	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
+		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
+		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
+		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
+		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
+		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
+		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
+		);
+
+	tm->tm_sec  = sec;
+	tm->tm_min  = min;
+	tm->tm_hour = hour;
+	tm->tm_wday = wday;
+	tm->tm_mday = mday;
+	tm->tm_mon  = mon - 1;
+	tm->tm_year = year;
+	tm->tm_year += 100;
+}
+
+static void mtk_rtc_set_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
+				      int time_alarm)
+{
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA), tm->tm_year - 100);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_MON), tm->tm_mon + 1);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW), tm->tm_wday);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM), tm->tm_mday);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU), tm->tm_hour);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN), tm->tm_min);
+	mtk_w32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC), tm->tm_sec);
+}
+
+static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
+{
+	struct mtk_rtc *hw = (struct mtk_rtc *)id;
+	u32 irq_sta;
+
+	/* Stop alarm also implicitly disable the alarm interrupt */
+	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
+	irq_sta = mtk_r32(hw, MTK_RTC_INT);
+	if (irq_sta & RTC_INT_AL_STA) {
+		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
+
+		/* Ack alarm interrupt status */
+		mtk_w32(hw, MTK_RTC_INT, RTC_INT_AL_STA);
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
+
+	return rtc_valid_tm(tm);
+}
+
+static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	/* Stop time counter before setting a new one*/
+	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
+
+	/* Epoch == 1900 */
+	if (tm->tm_year < 100 || tm->tm_year > 199)
+		return -EINVAL;
+
+	mtk_rtc_set_alarm_or_time(hw, tm, MTK_TC);
+
+	/* Restart the time counter */
+	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
+
+	return 0;
+}
+
+static int mtk_rtc_getalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+
+	mtk_rtc_get_alarm_or_time(hw, alrm_tm, MTK_AL);
+
+	wkalrm->enabled = !!(mtk_r32(hw, MTK_RTC_AL_CTL) & RTC_AL_EN);
+	wkalrm->pending = !!(mtk_r32(hw, MTK_RTC_INT) & RTC_INT_AL_STA);
+
+	return 0;
+}
+
+static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+
+	/* Epoch == 1900 */
+	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
+		return -EINVAL;
+
+	/*
+	 * Stop the alarm also implicitly including disables interrupt before
+	 * setting a new one.
+	 */
+	mtk_clr(hw, MTK_RTC_AL_CTL, RTC_AL_EN);
+
+	/*
+	 * Avoid contention between mtk_rtc_setalarm and IRQ handler so that
+	 * disabling the interrupt and awaiting for pending IRQ handler to
+	 * complete.
+	 */
+	synchronize_irq(hw->irq);
+
+	mtk_rtc_set_alarm_or_time(hw, alrm_tm, MTK_AL);
+
+	/* Restart the alarm with the new setup */
+	mtk_w32(hw, MTK_RTC_AL_CTL, RTC_AL_ALL);
+
+	return 0;
+}
+
+static const struct rtc_class_ops mtk_rtc_ops = {
+	.read_time		= mtk_rtc_gettime,
+	.set_time		= mtk_rtc_settime,
+	.read_alarm		= mtk_rtc_getalarm,
+	.set_alarm		= mtk_rtc_setalarm,
+};
+
+static const struct of_device_id mtk_rtc_match[] = {
+	{ .compatible = "mediatek,mt7622-rtc" },
+	{ .compatible = "mediatek,soc-rtc" },
+	{},
+};
+
+static int mtk_rtc_probe(struct platform_device *pdev)
+{
+	struct mtk_rtc *hw;
+	struct resource *res;
+	int ret;
+
+	hw = devm_kzalloc(&pdev->dev, sizeof(*hw), GFP_KERNEL);
+	if (!hw)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, hw);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	hw->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(hw->base))
+		return PTR_ERR(hw->base);
+
+	hw->clk = devm_clk_get(&pdev->dev, "rtc");
+	if (IS_ERR(hw->clk)) {
+		dev_err(&pdev->dev, "No clock\n");
+		return PTR_ERR(hw->clk);
+	}
+
+	ret = clk_prepare_enable(hw->clk);
+	if (ret)
+		return ret;
+
+	hw->irq = platform_get_irq(pdev, 0);
+	if (hw->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ resource\n");
+		ret = hw->irq;
+		goto err;
+	}
+
+	ret = devm_request_irq(&pdev->dev, hw->irq, mtk_rtc_alarmirq,
+			       0, dev_name(&pdev->dev), hw);
+	if (ret) {
+		dev_err(&pdev->dev, "Can't request IRQ\n");
+		goto err;
+	}
+
+	mtk_rtc_hw_init(hw);
+
+	device_init_wakeup(&pdev->dev, true);
+
+	hw->rtc = devm_rtc_device_register(&pdev->dev, pdev->name,
+					   &mtk_rtc_ops, THIS_MODULE);
+	if (IS_ERR(hw->rtc)) {
+		ret = PTR_ERR(hw->rtc);
+		dev_err(&pdev->dev, "Unable to register device\n");
+		goto err;
+	}
+
+	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
+
+	return 0;
+err:
+	clk_disable_unprepare(hw->clk);
+
+	return ret;
+}
+
+static int mtk_rtc_remove(struct platform_device *pdev)
+{
+	struct mtk_rtc *hw = platform_get_drvdata(pdev);
+
+	clk_disable_unprepare(hw->clk);
+
+	return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int mtk_rtc_suspend(struct device *dev)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(hw->irq);
+
+	return 0;
+}
+
+static int mtk_rtc_resume(struct device *dev)
+{
+	struct mtk_rtc *hw = dev_get_drvdata(dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(hw->irq);
+
+	return 0;
+}
+
+static SIMPLE_DEV_PM_OPS(mtk_rtc_pm_ops, mtk_rtc_suspend, mtk_rtc_resume);
+
+#define MTK_RTC_PM_OPS (&mtk_rtc_pm_ops)
+#else	/* CONFIG_PM */
+#define MTK_RTC_PM_OPS NULL
+#endif	/* CONFIG_PM */
+
+static struct platform_driver mtk_rtc_driver = {
+	.probe	= mtk_rtc_probe,
+	.remove	= mtk_rtc_remove,
+	.driver = {
+		.name = MTK_RTC_DEV,
+		.of_match_table = mtk_rtc_match,
+		.pm = MTK_RTC_PM_OPS,
+	},
+};
+
+module_platform_driver(mtk_rtc_driver);
+
+MODULE_DESCRIPTION("MediaTek SoC based RTC Driver");
+MODULE_AUTHOR("Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>");
+MODULE_LICENSE("GPL");
-- 
2.7.4

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

Re: [PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[Alexandre Belloni]

Hi,

On 22/09/2017 at 11:33:15 +0800, sean.wang@mediatek.com wrote:
> diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c

I'm pretty sure this should be named rtc-mt7622.c instead of
rtc-mediatek.c, exactly for the same reason you have patch 3/4.

> +static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
> +{
> +	__raw_writel(val, rtc->base + reg);

Do you really need the __raw accessors? What about running your SoC in
BE mode? I guess the _relaxed version are fast enough.

> +}
> +
> +static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
> +{
> +	return __raw_readl(rtc->base + reg);
> +}
> +


> +static void mtk_rtc_hw_init(struct mtk_rtc *hw)
> +{
> +	/* The setup of the init sequence is for allowing RTC got to work */
> +	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
> +	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
> +	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
> +	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
> +	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
> +	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
> +	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
> +	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
> +	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
> +}
> +
> +static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
> +				      int time_alarm)
> +{
> +	u32 year, mon, mday, wday, hour, min, sec;
> +
> +	/*
> +	 * Read again until all fields are not changed for all fields in the
> +	 * consistent state.
> +	 */
> +	do {
> +		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
> +		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
> +		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
> +		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
> +		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
> +		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
> +		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
> +	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
> +		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
> +		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
> +		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
> +		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
> +		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
> +		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
> +		);

I'm pretty sure only checking sec is enough because it is highly
unlikely that 7 reads take a minute.

> +static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
> +{
> +	struct mtk_rtc *hw = (struct mtk_rtc *)id;
> +	u32 irq_sta;
> +
> +	/* Stop alarm also implicitly disable the alarm interrupt */
> +	mtk_w32(hw, MTK_RTC_AL_CTL, 0);

You stop the alarm here, before testing whether the alarm really
happened.

> +	irq_sta = mtk_r32(hw, MTK_RTC_INT);
> +	if (irq_sta & RTC_INT_AL_STA) {
> +		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
> +
> +		/* Ack alarm interrupt status */
> +		mtk_w32(hw, MTK_RTC_INT, RTC_INT_AL_STA);
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> +
> +	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
> +
> +	return rtc_valid_tm(tm);
> +}
> +
> +static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
> +{
> +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> +
> +	/* Stop time counter before setting a new one*/
> +	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
> +
> +	/* Epoch == 1900 */
> +	if (tm->tm_year < 100 || tm->tm_year > 199)
> +		return -EINVAL;

Year is a 32 bits register, what makes the RTC fail in 2100? Is it
because of the leap year handling?

> +static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> +	struct rtc_time *alrm_tm = &wkalrm->time;
> +
> +	/* Epoch == 1900 */
> +	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
> +		return -EINVAL;
> +

Ditto.

> +
> +	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
> +

I think the rtc core is verbose enough that this message is not needed.


-- 
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[Sean Wang]

Hi Alexandre,

Thanks for your valuable suggestions on the driver.

I added comments inline and will have following-ups in the next version

	Sean

On Thu, 2017-10-12 at 23:20 +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 22/09/2017 at 11:33:15 +0800, sean.wang@mediatek.com wrote:
> > diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
> 
> I'm pretty sure this should be named rtc-mt7622.c instead of
> rtc-mediatek.c, exactly for the same reason you have patch 3/4.
> 

It's okay for naming with rtc-mt7622.c at this moment. But if more SoCs
support gets into the driver, I will consider again to give a more
generic name for the driver.

> > +static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
> > +{
> > +	__raw_writel(val, rtc->base + reg);
> 
> Do you really need the __raw accessors? What about running your SoC in
> BE mode? I guess the _relaxed version are fast enough.
> 

SoC runs on LE mode. I also think it's fine and enough to use _relaxed
version instead of __raw version.

> > +}
> > +
> > +static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
> > +{
> > +	return __raw_readl(rtc->base + reg);
> > +}
> > +
> 
> 
> > +static void mtk_rtc_hw_init(struct mtk_rtc *hw)
> > +{
> > +	/* The setup of the init sequence is for allowing RTC got to work */
> > +	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
> > +	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
> > +	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > +}
> > +
> > +static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
> > +				      int time_alarm)
> > +{
> > +	u32 year, mon, mday, wday, hour, min, sec;
> > +
> > +	/*
> > +	 * Read again until all fields are not changed for all fields in the
> > +	 * consistent state.
> > +	 */
> > +	do {
> > +		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
> > +		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
> > +		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
> > +		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
> > +		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
> > +		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
> > +		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
> > +	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
> > +		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
> > +		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
> > +		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
> > +		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
> > +		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
> > +		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
> > +		);
> 
> I'm pretty sure only checking sec is enough because it is highly
> unlikely that 7 reads take a minute.
> 

You're right. I made something stupid here. Only checking on sec is
enough and will give simpler and better code.

> > +static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
> > +{
> > +	struct mtk_rtc *hw = (struct mtk_rtc *)id;
> > +	u32 irq_sta;
> > +
> > +	/* Stop alarm also implicitly disable the alarm interrupt */
> > +	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
> 
> You stop the alarm here, before testing whether the alarm really
> happened.
> 

Okay. I will exchange the order for alarm stopping and the examination
whether the alarm is really expired. 

> > +	irq_sta = mtk_r32(hw, MTK_RTC_INT);
> > +	if (irq_sta & RTC_INT_AL_STA) {
> > +		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
> > +
> > +		/* Ack alarm interrupt status */
> > +		mtk_w32(hw, MTK_RTredundantC_INT, RTC_INT_AL_STA);
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> > +}
> > +
> > +static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +
> > +	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
> > +
> > +	return rtc_valid_tm(tm);
> > +}
> > +
> > +static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +
> > +	/* Stop time counter before setting a new one*/
> > +	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > +
> > +	/* Epoch == 1900 */
> > +	if (tm->tm_year < 100 || tm->tm_year > 199)
> > +		return -EINVAL;
> 
> Year is a 32 bits register, what makes the RTC fail in 2100? Is it
> because of the leap year handling?
> 

I made something stupid again here: rtc hardware doesn't have such the
limitation. I just felt alarm set up prior to 2100 is enough in my
initial thought, but it seemed I shouldn't do this. I will remove the
sanity condition.


> > +static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +	struct rtc_time *alrm_tm = &wkalrm->time;
> > +
> > +	/* Epoch == 1900 */
> > +	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
> > +		return -EINVAL;
> > +
> 
> Ditto.
> 
Ditto. those condition will be removed.

> > +
> > +	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
> > +
> 
> I think the rtc core is verbose enough that this message is not needed.
> 

Okay. the redundant and specific log prompt would be removed as well.


> 

Re: [PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[Sean Wang]

Hi Alexandre,

Thanks for your valuable suggestions on the driver.

I added comments inline and will have following-ups in the next version

	Sean

On Thu, 2017-10-12 at 23:20 +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 22/09/2017 at 11:33:15 +0800, sean.wang@mediatek.com wrote:
> > diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
> 
> I'm pretty sure this should be named rtc-mt7622.c instead of
> rtc-mediatek.c, exactly for the same reason you have patch 3/4.
> 

It's okay for naming with rtc-mt7622.c at this moment. But if more SoCs
support gets into the driver, I will consider again to give a more
generic name for the driver.

> > +static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
> > +{
> > +	__raw_writel(val, rtc->base + reg);
> 
> Do you really need the __raw accessors? What about running your SoC in
> BE mode? I guess the _relaxed version are fast enough.
> 

SoC runs on LE mode. I also think it's fine and enough to use _relaxed
version instead of __raw version.

> > +}
> > +
> > +static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
> > +{
> > +	return __raw_readl(rtc->base + reg);
> > +}
> > +
> 
> 
> > +static void mtk_rtc_hw_init(struct mtk_rtc *hw)
> > +{
> > +	/* The setup of the init sequence is for allowing RTC got to work */
> > +	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
> > +	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
> > +	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
> > +	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > +}
> > +
> > +static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
> > +				      int time_alarm)
> > +{
> > +	u32 year, mon, mday, wday, hour, min, sec;
> > +
> > +	/*
> > +	 * Read again until all fields are not changed for all fields in the
> > +	 * consistent state.
> > +	 */
> > +	do {
> > +		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
> > +		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
> > +		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
> > +		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
> > +		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
> > +		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
> > +		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
> > +	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
> > +		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
> > +		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
> > +		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
> > +		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
> > +		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
> > +		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
> > +		);
> 
> I'm pretty sure only checking sec is enough because it is highly
> unlikely that 7 reads take a minute.
> 

You're right. I made something stupid here. Only checking on sec is
enough and will give simpler and better code.

> > +static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
> > +{
> > +	struct mtk_rtc *hw = (struct mtk_rtc *)id;
> > +	u32 irq_sta;
> > +
> > +	/* Stop alarm also implicitly disable the alarm interrupt */
> > +	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
> 
> You stop the alarm here, before testing whether the alarm really
> happened.
> 

Okay. I will exchange the order for alarm stopping and the examination
whether the alarm is really expired. 

> > +	irq_sta = mtk_r32(hw, MTK_RTC_INT);
> > +	if (irq_sta & RTC_INT_AL_STA) {
> > +		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
> > +
> > +		/* Ack alarm interrupt status */
> > +		mtk_w32(hw, MTK_RTredundantC_INT, RTC_INT_AL_STA);
> > +		return IRQ_HANDLED;
> > +	}
> > +
> > +	return IRQ_NONE;
> > +}
> > +
> > +static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +
> > +	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
> > +
> > +	return rtc_valid_tm(tm);
> > +}
> > +
> > +static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +
> > +	/* Stop time counter before setting a new one*/
> > +	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > +
> > +	/* Epoch == 1900 */
> > +	if (tm->tm_year < 100 || tm->tm_year > 199)
> > +		return -EINVAL;
> 
> Year is a 32 bits register, what makes the RTC fail in 2100? Is it
> because of the leap year handling?
> 

I made something stupid again here: rtc hardware doesn't have such the
limitation. I just felt alarm set up prior to 2100 is enough in my
initial thought, but it seemed I shouldn't do this. I will remove the
sanity condition.


> > +static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> > +{
> > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > +	struct rtc_time *alrm_tm = &wkalrm->time;
> > +
> > +	/* Epoch == 1900 */
> > +	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
> > +		return -EINVAL;
> > +
> 
> Ditto.
> 
Ditto. those condition will be removed.

> > +
> > +	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
> > +
> 
> I think the rtc core is verbose enough that this message is not needed.
> 

Okay. the redundant and specific log prompt would be removed as well.


> 

Re: [PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[Sean Wang]

On Mon, 2017-10-16 at 16:17 +0800, Sean Wang wrote:
> Hi Alexandre,
> 
> Thanks for your valuable suggestions on the driver.
> 
> I added comments inline and will have following-ups in the next version
> 
> 	Sean
> 
> On Thu, 2017-10-12 at 23:20 +0200, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 22/09/2017 at 11:33:15 +0800, sean.wang@mediatek.com wrote:
> > > diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
> > 
> > I'm pretty sure this should be named rtc-mt7622.c instead of
> > rtc-mediatek.c, exactly for the same reason you have patch 3/4.
> > 
> 
> It's okay for naming with rtc-mt7622.c at this moment. But if more SoCs
> support gets into the driver, I will consider again to give a more
> generic name for the driver.
> 
> > > +static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
> > > +{
> > > +	__raw_writel(val, rtc->base + reg);
> > 
> > Do you really need the __raw accessors? What about running your SoC in
> > BE mode? I guess the _relaxed version are fast enough.
> > 
> 
> SoC runs on LE mode. I also think it's fine and enough to use _relaxed
> version instead of __raw version.
> 
> > > +}
> > > +
> > > +static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
> > > +{
> > > +	return __raw_readl(rtc->base + reg);
> > > +}
> > > +
> > 
> > 
> > > +static void mtk_rtc_hw_init(struct mtk_rtc *hw)
> > > +{
> > > +	/* The setup of the init sequence is for allowing RTC got to work */
> > > +	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
> > > +	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
> > > +	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > > +}
> > > +
> > > +static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
> > > +				      int time_alarm)
> > > +{
> > > +	u32 year, mon, mday, wday, hour, min, sec;
> > > +
> > > +	/*
> > > +	 * Read again until all fields are not changed for all fields in the
> > > +	 * consistent state.
> > > +	 */
> > > +	do {
> > > +		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
> > > +		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
> > > +		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
> > > +		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
> > > +		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
> > > +		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
> > > +		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
> > > +	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
> > > +		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
> > > +		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
> > > +		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
> > > +		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
> > > +		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
> > > +		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
> > > +		);
> > 
> > I'm pretty sure only checking sec is enough because it is highly
> > unlikely that 7 reads take a minute.
> > 
> 
> You're right. I made something stupid here. Only checking on sec is
> enough and will give simpler and better code.
> 
> > > +static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
> > > +{
> > > +	struct mtk_rtc *hw = (struct mtk_rtc *)id;
> > > +	u32 irq_sta;
> > > +
> > > +	/* Stop alarm also implicitly disable the alarm interrupt */
> > > +	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
> > 
> > You stop the alarm here, before testing whether the alarm really
> > happened.
> > 
> 
> Okay. I will exchange the order for alarm stopping and the examination
> whether the alarm is really expired. 
> 
> > > +	irq_sta = mtk_r32(hw, MTK_RTC_INT);
> > > +	if (irq_sta & RTC_INT_AL_STA) {
> > > +		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
> > > +
> > > +		/* Ack alarm interrupt status */
> > > +		mtk_w32(hw, MTK_RTredundantC_INT, RTC_INT_AL_STA);
> > > +		return IRQ_HANDLED;
> > > +	}
> > > +
> > > +	return IRQ_NONE;
> > > +}
> > > +
> > > +static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +
> > > +	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
> > > +
> > > +	return rtc_valid_tm(tm);
> > > +}
> > > +
> > > +static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +
> > > +	/* Stop time counter before setting a new one*/
> > > +	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > > +
> > > +	/* Epoch == 1900 */
> > > +	if (tm->tm_year < 100 || tm->tm_year > 199)
> > > +		return -EINVAL;
> > 
> > Year is a 32 bits register, what makes the RTC fail in 2100? Is it
> > because of the leap year handling?
> > 
> 
> I made something stupid again here: rtc hardware doesn't have such the
> limitation. I just felt alarm set up prior to 2100 is enough in my
> initial thought, but it seemed I shouldn't do this. I will remove the
> sanity condition.
> 
Sorry for that I gave incorrect information for the RTC in the previous
reply: After check again the usage of the register, the maximum number
of the year the RTC can hold is 99 and then wraparound to 0 when
overflow occurs although the year register is a 32 bits register.

Therefore, the sanity for tm->tm_year is still required for the both
setup handler on alarm and rtc to ensure the user input data is valid,
where the current driver assume it's valid when tm->tm_year is between
2000 and 2099. I'll add more comments for the hardware limitation.

	Sean
> 
> > > +static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +	struct rtc_time *alrm_tm = &wkalrm->time;
> > > +
> > > +	/* Epoch == 1900 */
> > > +	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
> > > +		return -EINVAL;
> > > +
> > 
> > Ditto.
> > 
> Ditto. those condition will be removed.
> 
> > > +
> > > +	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
> > > +
> > 
> > I think the rtc core is verbose enough that this message is not needed.
> > 
> 
> Okay. the redundant and specific log prompt would be removed as well.
> 
> 
> > 
> 

Re: [PATCH 2/4] rtc: mediatek: add driver for RTC on MT7622 SoC

[Sean Wang]

On Mon, 2017-10-16 at 16:17 +0800, Sean Wang wrote:
> Hi Alexandre,
> 
> Thanks for your valuable suggestions on the driver.
> 
> I added comments inline and will have following-ups in the next version
> 
> 	Sean
> 
> On Thu, 2017-10-12 at 23:20 +0200, Alexandre Belloni wrote:
> > Hi,
> > 
> > On 22/09/2017 at 11:33:15 +0800, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> > > diff --git a/drivers/rtc/rtc-mediatek.c b/drivers/rtc/rtc-mediatek.c
> > 
> > I'm pretty sure this should be named rtc-mt7622.c instead of
> > rtc-mediatek.c, exactly for the same reason you have patch 3/4.
> > 
> 
> It's okay for naming with rtc-mt7622.c at this moment. But if more SoCs
> support gets into the driver, I will consider again to give a more
> generic name for the driver.
> 
> > > +static void mtk_w32(struct mtk_rtc *rtc, u32 reg, u32 val)
> > > +{
> > > +	__raw_writel(val, rtc->base + reg);
> > 
> > Do you really need the __raw accessors? What about running your SoC in
> > BE mode? I guess the _relaxed version are fast enough.
> > 
> 
> SoC runs on LE mode. I also think it's fine and enough to use _relaxed
> version instead of __raw version.
> 
> > > +}
> > > +
> > > +static u32 mtk_r32(struct mtk_rtc *rtc, u32 reg)
> > > +{
> > > +	return __raw_readl(rtc->base + reg);
> > > +}
> > > +
> > 
> > 
> > > +static void mtk_rtc_hw_init(struct mtk_rtc *hw)
> > > +{
> > > +	/* The setup of the init sequence is for allowing RTC got to work */
> > > +	mtk_w32(hw, MTK_RTC_PWRCHK1, RTC_PWRCHK1_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PWRCHK2, RTC_PWRCHK2_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_KEY, RTC_KEY_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT1, RTC_PROT1_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT2, RTC_PROT2_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT3, RTC_PROT3_MAGIC);
> > > +	mtk_w32(hw, MTK_RTC_PROT4, RTC_PROT4_MAGIC);
> > > +	mtk_rmw(hw, MTK_RTC_DEBNCE, RTC_DEBNCE_MASK, 0);
> > > +	mtk_clr(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > > +}
> > > +
> > > +static void mtk_rtc_get_alarm_or_time(struct mtk_rtc *hw, struct rtc_time *tm,
> > > +				      int time_alarm)
> > > +{
> > > +	u32 year, mon, mday, wday, hour, min, sec;
> > > +
> > > +	/*
> > > +	 * Read again until all fields are not changed for all fields in the
> > > +	 * consistent state.
> > > +	 */
> > > +	do {
> > > +		year = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA));
> > > +		mon = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON));
> > > +		wday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW));
> > > +		mday = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM));
> > > +		hour = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU));
> > > +		min = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN));
> > > +		sec = mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC));
> > > +	} while (year != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_YEA)) ||
> > > +		 mon != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MON))  ||
> > > +		 mday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOM))	||
> > > +		 wday != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_DOW))	||
> > > +		 hour != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_HOU))	||
> > > +		 min != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_MIN))	||
> > > +		 sec != mtk_r32(hw, MTK_RTC_TREG(time_alarm, MTK_SEC))
> > > +		);
> > 
> > I'm pretty sure only checking sec is enough because it is highly
> > unlikely that 7 reads take a minute.
> > 
> 
> You're right. I made something stupid here. Only checking on sec is
> enough and will give simpler and better code.
> 
> > > +static irqreturn_t mtk_rtc_alarmirq(int irq, void *id)
> > > +{
> > > +	struct mtk_rtc *hw = (struct mtk_rtc *)id;
> > > +	u32 irq_sta;
> > > +
> > > +	/* Stop alarm also implicitly disable the alarm interrupt */
> > > +	mtk_w32(hw, MTK_RTC_AL_CTL, 0);
> > 
> > You stop the alarm here, before testing whether the alarm really
> > happened.
> > 
> 
> Okay. I will exchange the order for alarm stopping and the examination
> whether the alarm is really expired. 
> 
> > > +	irq_sta = mtk_r32(hw, MTK_RTC_INT);
> > > +	if (irq_sta & RTC_INT_AL_STA) {
> > > +		rtc_update_irq(hw->rtc, 1, RTC_IRQF | RTC_AF);
> > > +
> > > +		/* Ack alarm interrupt status */
> > > +		mtk_w32(hw, MTK_RTredundantC_INT, RTC_INT_AL_STA);
> > > +		return IRQ_HANDLED;
> > > +	}
> > > +
> > > +	return IRQ_NONE;
> > > +}
> > > +
> > > +static int mtk_rtc_gettime(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +
> > > +	mtk_rtc_get_alarm_or_time(hw, tm, MTK_TC);
> > > +
> > > +	return rtc_valid_tm(tm);
> > > +}
> > > +
> > > +static int mtk_rtc_settime(struct device *dev, struct rtc_time *tm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +
> > > +	/* Stop time counter before setting a new one*/
> > > +	mtk_set(hw, MTK_RTC_CTL, RTC_RC_STOP);
> > > +
> > > +	/* Epoch == 1900 */
> > > +	if (tm->tm_year < 100 || tm->tm_year > 199)
> > > +		return -EINVAL;
> > 
> > Year is a 32 bits register, what makes the RTC fail in 2100? Is it
> > because of the leap year handling?
> > 
> 
> I made something stupid again here: rtc hardware doesn't have such the
> limitation. I just felt alarm set up prior to 2100 is enough in my
> initial thought, but it seemed I shouldn't do this. I will remove the
> sanity condition.
> 
Sorry for that I gave incorrect information for the RTC in the previous
reply: After check again the usage of the register, the maximum number
of the year the RTC can hold is 99 and then wraparound to 0 when
overflow occurs although the year register is a 32 bits register.

Therefore, the sanity for tm->tm_year is still required for the both
setup handler on alarm and rtc to ensure the user input data is valid,
where the current driver assume it's valid when tm->tm_year is between
2000 and 2099. I'll add more comments for the hardware limitation.

	Sean
> 
> > > +static int mtk_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> > > +{
> > > +	struct mtk_rtc *hw = dev_get_drvdata(dev);
> > > +	struct rtc_time *alrm_tm = &wkalrm->time;
> > > +
> > > +	/* Epoch == 1900 */
> > > +	if (alrm_tm->tm_year < 100 || alrm_tm->tm_year > 199)
> > > +		return -EINVAL;
> > > +
> > 
> > Ditto.
> > 
> Ditto. those condition will be removed.
> 
> > > +
> > > +	dev_info(&pdev->dev, "MediaTek SoC based RTC enabled\n");
> > > +
> > 
> > I think the rtc core is verbose enough that this message is not needed.
> > 
> 
> Okay. the redundant and specific log prompt would be removed as well.
> 
> 
> > 
> 


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

[PATCH 3/4] rtc: mediatek: enhance the description for MediaTek PMIC based RTC

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Give a better description for original MediaTek RTC driver as PMIC based
RTC in order to distinguish SoC based RTC. Also turning all words with
Mediatek to MediaTek here.

Cc: Eddie Huang <eddie.huang@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/rtc/Kconfig | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4226295..4500f77 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1716,14 +1716,14 @@ config RTC_DRV_MEDIATEK
 	  will be called rtc-mediatek.
 
 config RTC_DRV_MT6397
-	tristate "Mediatek Real Time Clock driver"
+	tristate "MediaTek PMIC based RTC"
 	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
 	help
-	  This selects the Mediatek(R) RTC driver. RTC is part of Mediatek
+	  This selects the MediaTek(R) RTC driver. RTC is part of MediaTek
 	  MT6397 PMIC. You should enable MT6397 PMIC MFD before select
-	  Mediatek(R) RTC driver.
+	  MediaTek(R) RTC driver.
 
-	  If you want to use Mediatek(R) RTC interface, select Y or M here.
+	  If you want to use MediaTek(R) RTC interface, select Y or M here.
 
 config RTC_DRV_XGENE
 	tristate "APM X-Gene RTC"
-- 
2.7.4

[PATCH 3/4] rtc: mediatek: enhance the description for MediaTek PMIC based RTC

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

Give a better description for original MediaTek RTC driver as PMIC based
RTC in order to distinguish SoC based RTC. Also turning all words with
Mediatek to MediaTek here.

Cc: Eddie Huang <eddie.huang@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 drivers/rtc/Kconfig | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 4226295..4500f77 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1716,14 +1716,14 @@ config RTC_DRV_MEDIATEK
 	  will be called rtc-mediatek.
 
 config RTC_DRV_MT6397
-	tristate "Mediatek Real Time Clock driver"
+	tristate "MediaTek PMIC based RTC"
 	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
 	help
-	  This selects the Mediatek(R) RTC driver. RTC is part of Mediatek
+	  This selects the MediaTek(R) RTC driver. RTC is part of MediaTek
 	  MT6397 PMIC. You should enable MT6397 PMIC MFD before select
-	  Mediatek(R) RTC driver.
+	  MediaTek(R) RTC driver.
 
-	  If you want to use Mediatek(R) RTC interface, select Y or M here.
+	  If you want to use MediaTek(R) RTC interface, select Y or M here.
 
 config RTC_DRV_XGENE
 	tristate "APM X-Gene RTC"
-- 
2.7.4

Re: [PATCH 3/4] rtc: mediatek: enhance the description for MediaTek PMIC based RTC

[Eddie Huang]

Hi Sean,

On Fri, 2017-09-22 at 11:33 +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
> 
> Give a better description for original MediaTek RTC driver as PMIC based
> RTC in order to distinguish SoC based RTC. Also turning all words with
> Mediatek to MediaTek here.
> 
> Cc: Eddie Huang <eddie.huang@mediatek.com>
> Signed-off-by: Sean Wang <sean.wang@mediatek.com>
> ---
>  drivers/rtc/Kconfig | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 4226295..4500f77 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1716,14 +1716,14 @@ config RTC_DRV_MEDIATEK
>  	  will be called rtc-mediatek.
>  
>  config RTC_DRV_MT6397
> -	tristate "Mediatek Real Time Clock driver"
> +	tristate "MediaTek PMIC based RTC"
>  	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
>  	help
> -	  This selects the Mediatek(R) RTC driver. RTC is part of Mediatek
> +	  This selects the MediaTek(R) RTC driver. RTC is part of MediaTek
>  	  MT6397 PMIC. You should enable MT6397 PMIC MFD before select
> -	  Mediatek(R) RTC driver.
> +	  MediaTek(R) RTC driver.
>  
> -	  If you want to use Mediatek(R) RTC interface, select Y or M here.
> +	  If you want to use MediaTek(R) RTC interface, select Y or M here.
>  
>  config RTC_DRV_XGENE
>  	tristate "APM X-Gene RTC"

Thanks your correction.

Acked-by: Eddie Huang <eddie.huang@mediatek.com>

Regards,

Re: [PATCH 3/4] rtc: mediatek: enhance the description for MediaTek PMIC based RTC

[Eddie Huang]

Hi Sean,

On Fri, 2017-09-22 at 11:33 +0800, sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org wrote:
> From: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> 
> Give a better description for original MediaTek RTC driver as PMIC based
> RTC in order to distinguish SoC based RTC. Also turning all words with
> Mediatek to MediaTek here.
> 
> Cc: Eddie Huang <eddie.huang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Signed-off-by: Sean Wang <sean.wang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> ---
>  drivers/rtc/Kconfig | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 4226295..4500f77 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1716,14 +1716,14 @@ config RTC_DRV_MEDIATEK
>  	  will be called rtc-mediatek.
>  
>  config RTC_DRV_MT6397
> -	tristate "Mediatek Real Time Clock driver"
> +	tristate "MediaTek PMIC based RTC"
>  	depends on MFD_MT6397 || (COMPILE_TEST && IRQ_DOMAIN)
>  	help
> -	  This selects the Mediatek(R) RTC driver. RTC is part of Mediatek
> +	  This selects the MediaTek(R) RTC driver. RTC is part of MediaTek
>  	  MT6397 PMIC. You should enable MT6397 PMIC MFD before select
> -	  Mediatek(R) RTC driver.
> +	  MediaTek(R) RTC driver.
>  
> -	  If you want to use Mediatek(R) RTC interface, select Y or M here.
> +	  If you want to use MediaTek(R) RTC interface, select Y or M here.
>  
>  config RTC_DRV_XGENE
>  	tristate "APM X-Gene RTC"

Thanks your correction.

Acked-by: Eddie Huang <eddie.huang-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>

Regards,


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

[PATCH 4/4] rtc: mediatek: update MAINTAINERS entry with MediaTek RTC driver

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

I work for MediaTek on maintaining the MediaTek SoC based RTC driver for
the existing SoCs and keep adding support for the following SoCs in the
future.

Cc: Eddie Huang <eddie.huang@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2281af4..5142029 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1579,10 +1579,13 @@ F:	drivers/rtc/rtc-armada38x.c
 
 ARM/Mediatek RTC DRIVER
 M:	Eddie Huang <eddie.huang@mediatek.com>
+M:	Sean Wang <sean.wang@mediatek.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
+F:	Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
 F:	drivers/rtc/rtc-mt6397.c
+F:	drivers/rtc/rtc-mediatek.c
 
 ARM/Mediatek SoC support
 M:	Matthias Brugger <matthias.bgg@gmail.com>
-- 
2.7.4

[PATCH 4/4] rtc: mediatek: update MAINTAINERS entry with MediaTek RTC driver

[sean.wang]

From: Sean Wang <sean.wang@mediatek.com>

I work for MediaTek on maintaining the MediaTek SoC based RTC driver for
the existing SoCs and keep adding support for the following SoCs in the
future.

Cc: Eddie Huang <eddie.huang@mediatek.com>
Signed-off-by: Sean Wang <sean.wang@mediatek.com>
---
 MAINTAINERS | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2281af4..5142029 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1579,10 +1579,13 @@ F:	drivers/rtc/rtc-armada38x.c
 
 ARM/Mediatek RTC DRIVER
 M:	Eddie Huang <eddie.huang@mediatek.com>
+M:	Sean Wang <sean.wang@mediatek.com>
 L:	linux-arm-kernel@lists.infradead.org (moderated for non-subscribers)
 L:	linux-mediatek@lists.infradead.org (moderated for non-subscribers)
 S:	Maintained
+F:	Documentation/devicetree/bindings/rtc/rtc-mediatek.txt
 F:	drivers/rtc/rtc-mt6397.c
+F:	drivers/rtc/rtc-mediatek.c
 
 ARM/Mediatek SoC support
 M:	Matthias Brugger <matthias.bgg@gmail.com>
-- 
2.7.4