[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Carlo Caione]

This patch introduces the driver for the RTC in the Allwinner A10 and
A20 SoCs.

Signed-off-by: Carlo Caione <carlo.caione@gmail.com>
---
v1->v2:
- Patch split in two
- Fix coding style
- New driver description
- Rework macros
- Add comments
- Change while in for cycles

v2->v3
- Fix comments
- Rename alarm->alrm
- Fix comments style
- Remove useless newline
---
 drivers/rtc/Kconfig     |   7 +
 drivers/rtc/Makefile    |   1 +
 drivers/rtc/rtc-sunxi.c | 491 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 499 insertions(+)
 create mode 100644 drivers/rtc/rtc-sunxi.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 9654aa3..ef45e0b 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -1076,6 +1076,13 @@ config RTC_DRV_SUN4V
 	  If you say Y here you will get support for the Hypervisor
 	  based RTC on SUN4V systems.
 
+config RTC_DRV_SUNXI
+	tristate "Allwinner sun4i/sun7i RTC"
+	depends on ARCH_SUNXI
+	help
+	  If you say Y here you will get support for the RTC found on
+	  Allwinner A10/A20.
+
 config RTC_DRV_STARFIRE
 	bool "Starfire RTC"
 	depends on SPARC64
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 2dff3d2..8b52b5a 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -115,6 +115,7 @@ obj-$(CONFIG_RTC_DRV_STARFIRE)	+= rtc-starfire.o
 obj-$(CONFIG_RTC_DRV_STK17TA8)	+= rtc-stk17ta8.o
 obj-$(CONFIG_RTC_DRV_STMP)	+= rtc-stmp3xxx.o
 obj-$(CONFIG_RTC_DRV_SUN4V)	+= rtc-sun4v.o
+obj-$(CONFIG_RTC_DRV_SUNXI)	+= rtc-sunxi.o
 obj-$(CONFIG_RTC_DRV_TEGRA)	+= rtc-tegra.o
 obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
 obj-$(CONFIG_RTC_DRV_TILE)	+= rtc-tile.o
diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
new file mode 100644
index 0000000..89c331f
--- /dev/null
+++ b/drivers/rtc/rtc-sunxi.c
@@ -0,0 +1,491 @@
+/*
+ * An RTC driver for Allwinner A10/A20
+ *
+ * Copyright (c) 2013, Carlo Caione <carlo.caione@gmail.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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/platform_device.h>
+#include <linux/rtc.h>
+#include <linux/types.h>
+
+#define SUNXI_LOSC_CTRL				0x0000
+#define SUNXI_LOSC_CTRL_RTC_HMS_ACC		BIT(8)
+#define SUNXI_LOSC_CTRL_RTC_YMD_ACC		BIT(7)
+
+#define SUNXI_RTC_YMD				0x0004
+
+#define SUNXI_RTC_HMS				0x0008
+
+#define SUNXI_ALRM_DHMS				0x000c
+
+#define SUNXI_ALRM_EN				0x0014
+#define SUNXI_ALRM_EN_CNT_EN			BIT(8)
+
+#define SUNXI_ALRM_IRQ_EN			0x0018
+#define SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN		BIT(0)
+
+#define SUNXI_ALRM_IRQ_STA			0x001c
+#define SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND		BIT(0)
+
+#define SUNXI_LOSC_CTRL_RTC_ACC \
+	(SUNXI_LOSC_CTRL_RTC_HMS_ACC | SUNXI_LOSC_CTRL_RTC_YMD_ACC)
+
+#define SUNXI_MASK_DH				0x0000001f
+#define SUNXI_MASK_SM				0x0000003f
+#define SUNXI_MASK_M				0x0000000f
+#define SUNXI_MASK_LY				0x00000001
+#define SUNXI_MASK_D				0x00000ffe
+#define SUNXI_MASK_M				0x0000000f
+
+#define SUNXI_GET(x, mask, shift)		(((x) & ((mask) << (shift))) \
+							>> (shift))
+
+#define SUNXI_SET(x, mask, shift)		(((x) & (mask)) << (shift))
+
+/* Get date values */
+#define SUNXI_DATE_GET_DAY_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 0)
+#define SUNXI_DATE_GET_MON_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_M, 8)
+#define SUNXI_DATE_GET_YEAR_VALUE(x, mask)	SUNXI_GET(x, mask, 16)
+
+/* Get time values */
+#define SUNXI_TIME_GET_SEC_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 0)
+#define SUNXI_TIME_GET_MIN_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 8)
+#define SUNXI_TIME_GET_HOUR_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 16)
+
+/* Get alarm values */
+#define SUNXI_ALRM_GET_SEC_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 0)
+#define SUNXI_ALRM_GET_MIN_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 8)
+#define SUNXI_ALRM_GET_HOUR_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 16)
+
+/* Set date values */
+#define SUNXI_DATE_SET_DAY_VALUE(x)		SUNXI_DATE_GET_DAY_VALUE(x)
+#define SUNXI_DATE_SET_MON_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_M, 8)
+#define SUNXI_DATE_SET_YEAR_VALUE(x, mask)	SUNXI_SET(x, mask, 16)
+#define SUNXI_LEAP_SET_VALUE(x, shift)		SUNXI_SET(x, SUNXI_MASK_LY, shift)
+
+/* Set time values */
+#define SUNXI_TIME_SET_SEC_VALUE(x)		SUNXI_TIME_GET_SEC_VALUE(x)
+#define SUNXI_TIME_SET_MIN_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_SM, 8)
+#define SUNXI_TIME_SET_HOUR_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_DH, 16)
+
+/* set alarm values */
+#define SUNXI_ALRM_SET_SEC_VALUE(x)		SUNXI_ALRM_GET_SEC_VALUE(x)
+#define SUNXI_ALRM_SET_MIN_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_SM, 8)
+#define SUNXI_ALRM_SET_HOUR_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_DH, 16)
+#define SUNXI_ALRM_SET_DAY_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_D, 21)
+
+/* time unit conversions */
+#define SEC_IN_MIN				60
+#define SEC_IN_HOUR				(60 * SEC_IN_MIN)
+#define SEC_IN_DAY				(24 * SEC_IN_HOUR)
+
+struct sunxi_rtc_data_year {
+	unsigned int min;		/* min year allowed */
+	unsigned int max;		/* max year allowed */
+	unsigned int off;		/* year offset to get the current year */
+	unsigned int mask;
+	unsigned char leap_shift;	/* bit shift to get the leap year */
+};
+
+static struct sunxi_rtc_data_year data_year_param[] = {
+	[0] = {
+		.min		= 1970,
+		.max		= 2100,
+		.off		= 0,
+		.mask		= 0x000000ff,
+		.leap_shift	= 24,
+	},
+	[1] = {
+		.min		= 2010,
+		.max		= 2073,
+		.off		= 110,
+		.mask		= 0x0000003f,
+		.leap_shift	= 22,
+	},
+};
+
+struct sunxi_rtc_dev {
+	struct rtc_device *rtc;
+	struct device *dev;
+	struct sunxi_rtc_data_year *data_year;
+	void __iomem *base;
+	int irq;
+};
+
+static irqreturn_t sunxi_rtc_alarmirq(int irq, void *id)
+{
+	struct sunxi_rtc_dev *chip = (struct sunxi_rtc_dev *) id;
+	u32 val;
+
+	val = readl(chip->base + SUNXI_ALRM_IRQ_STA);
+
+	if (val & SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND) {
+		val |= SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND;
+		writel(val, chip->base + SUNXI_ALRM_IRQ_STA);
+
+		rtc_update_irq(chip->rtc, 1, RTC_AF | RTC_IRQF);
+
+		return IRQ_HANDLED;
+	}
+
+	return IRQ_NONE;
+}
+
+static void sunxi_rtc_setaie(int to, struct sunxi_rtc_dev *chip)
+{
+	u32 alrm_val = 0;
+	u32 alrm_irq_val = 0;
+
+	if (to) {
+		alrm_val = readl(chip->base + SUNXI_ALRM_EN);
+		alrm_val |= SUNXI_ALRM_EN_CNT_EN;
+
+		alrm_irq_val = readl(chip->base + SUNXI_ALRM_IRQ_EN);
+		alrm_irq_val |= SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN;
+	} else {
+		writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND,
+				chip->base + SUNXI_ALRM_IRQ_STA);
+	}
+
+	writel(alrm_val, chip->base + SUNXI_ALRM_EN);
+	writel(alrm_irq_val, chip->base + SUNXI_ALRM_IRQ_EN);
+}
+
+static int sunxi_rtc_getalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+	u32 alrm;
+	u32 alrm_en;
+	u32 date;
+
+	alrm = readl(chip->base + SUNXI_ALRM_DHMS);
+	date = readl(chip->base + SUNXI_RTC_YMD);
+
+	alrm_tm->tm_sec = SUNXI_ALRM_GET_SEC_VALUE(alrm);
+	alrm_tm->tm_min = SUNXI_ALRM_GET_MIN_VALUE(alrm);
+	alrm_tm->tm_hour = SUNXI_ALRM_GET_HOUR_VALUE(alrm);
+
+	alrm_tm->tm_mday = SUNXI_DATE_GET_DAY_VALUE(date);
+	alrm_tm->tm_mon = SUNXI_DATE_GET_MON_VALUE(date);
+	alrm_tm->tm_year = SUNXI_DATE_GET_YEAR_VALUE(date,
+			chip->data_year->mask);
+
+	alrm_tm->tm_year += chip->data_year->off;
+	alrm_tm->tm_mon -= 1;
+
+	alrm_en = readl(chip->base + SUNXI_ALRM_IRQ_EN);
+	if (alrm_en & SUNXI_ALRM_EN_CNT_EN)
+		wkalrm->enabled = 1;
+
+	return 0;
+}
+
+static int sunxi_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
+{
+	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
+	u32 date, time;
+	int t;
+
+	/*
+	 * read again if the system was mid-updated
+	 */
+	for (t = 0; t < 2; t++) {
+		date = readl(chip->base + SUNXI_RTC_YMD);
+		time = readl(chip->base + SUNXI_RTC_HMS);
+
+		rtc_tm->tm_sec  = SUNXI_TIME_GET_SEC_VALUE(time);
+		rtc_tm->tm_min  = SUNXI_TIME_GET_MIN_VALUE(time);
+		rtc_tm->tm_hour = SUNXI_TIME_GET_HOUR_VALUE(time);
+
+		rtc_tm->tm_mday = SUNXI_DATE_GET_DAY_VALUE(date);
+		rtc_tm->tm_mon  = SUNXI_DATE_GET_MON_VALUE(date);
+		rtc_tm->tm_year = SUNXI_DATE_GET_YEAR_VALUE(date,
+				chip->data_year->mask);
+
+		if (rtc_tm->tm_sec == 0)
+			msleep(500);
+		else
+			break;
+	}
+
+	rtc_tm->tm_year += chip->data_year->off;
+	rtc_tm->tm_mon  -= 1;
+
+	return rtc_valid_tm(rtc_tm);
+}
+
+static int sunxi_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
+{
+	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
+	struct rtc_time *alrm_tm = &wkalrm->time;
+	struct rtc_time tm_now;
+	u32 alrm = 0;
+	unsigned long time_now = 0;
+	unsigned long time_set = 0;
+	unsigned long time_gap = 0;
+	unsigned long time_gap_day = 0;
+	unsigned long time_gap_hour = 0;
+	unsigned long time_gap_min = 0;
+	int ret = 0;
+
+	ret = sunxi_rtc_gettime(dev, &tm_now);
+	if (ret < 0) {
+		dev_err(dev, "Error in getting time\n");
+		return -EINVAL;
+	}
+
+	rtc_tm_to_time(alrm_tm, &time_set);
+	rtc_tm_to_time(&tm_now, &time_now);
+	if (time_set <= time_now) {
+		dev_err(dev, "Date to set in the past\n");
+		return -EINVAL;
+	}
+
+	time_gap = time_set - time_now;
+	time_gap_day = time_gap / SEC_IN_DAY;
+	time_gap -= time_gap_day * SEC_IN_DAY;
+	time_gap_hour = time_gap / SEC_IN_HOUR;
+	time_gap -= time_gap_hour * SEC_IN_HOUR;
+	time_gap_min = time_gap / SEC_IN_MIN;
+	time_gap -= time_gap_min * SEC_IN_MIN;
+
+	if (time_gap_day > 255) {
+		dev_err(dev, "Day must be in the range 0 - 255\n");
+		return -EINVAL;
+	}
+
+	sunxi_rtc_setaie(0, chip);
+	writel(0, chip->base + SUNXI_ALRM_DHMS);
+	usleep_range(100, 300);
+
+	alrm = SUNXI_ALRM_SET_SEC_VALUE(time_gap) |
+		SUNXI_ALRM_SET_MIN_VALUE(time_gap_min) |
+		SUNXI_ALRM_SET_HOUR_VALUE(time_gap_hour) |
+		SUNXI_ALRM_SET_DAY_VALUE(time_gap_day);
+	writel(alrm, chip->base + SUNXI_ALRM_DHMS);
+
+	writel(0, chip->base + SUNXI_ALRM_IRQ_EN);
+	writel(SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN, chip->base + SUNXI_ALRM_IRQ_EN);
+
+	sunxi_rtc_setaie(wkalrm->enabled, chip);
+
+	return 0;
+}
+
+static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
+{
+	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
+	u32 date = 0;
+	u32 time = 0;
+	int year;
+	int t;
+
+	year = rtc_tm->tm_year + 1900;
+	if (year < chip->data_year->min || year > chip->data_year->max) {
+		dev_err(dev, "rtc only supports year in range %d - %d\n",
+				chip->data_year->min, chip->data_year->max);
+		return -EINVAL;
+	}
+
+	rtc_tm->tm_year -= chip->data_year->off;
+	rtc_tm->tm_mon += 1;
+
+	date = SUNXI_DATE_SET_DAY_VALUE(rtc_tm->tm_mday) |
+		SUNXI_DATE_SET_MON_VALUE(rtc_tm->tm_mon)  |
+		SUNXI_DATE_SET_YEAR_VALUE(rtc_tm->tm_year,
+				chip->data_year->mask);
+
+	if (is_leap_year(year))
+		date |= SUNXI_LEAP_SET_VALUE(1, chip->data_year->leap_shift);
+
+	time = SUNXI_TIME_SET_SEC_VALUE(rtc_tm->tm_sec)  |
+		SUNXI_TIME_SET_MIN_VALUE(rtc_tm->tm_min)  |
+		SUNXI_TIME_SET_HOUR_VALUE(rtc_tm->tm_hour);
+
+	writel(0, chip->base + SUNXI_RTC_HMS);
+	writel(0, chip->base + SUNXI_RTC_YMD);
+
+	writel(time, chip->base + SUNXI_RTC_HMS);
+
+	/*
+	 * After writing the RCT HH-MM-SS register, the
+	 * SUNXI_LOSC_CTRL_RTC_HMS_ACC bit is set and it will be cleared until
+	 * the real writing operation is finished
+	 */
+	for (t = 0; t < 3; t++) {
+		if ((readl(chip->base + SUNXI_LOSC_CTRL) &
+			SUNXI_LOSC_CTRL_RTC_HMS_ACC) && --t)
+			break;
+		else
+			msleep(50);
+	}
+	if (t == 0) {
+		dev_err(dev, "Failed to set rtc time.\n");
+		return -1;
+	}
+
+	writel(date, chip->base + SUNXI_RTC_YMD);
+
+	/*
+	 * After writing the RCT YY-MM-DD register, the
+	 * SUNXI_LOSC_CTRL_RTC_YMD_ACC bit is set and it will be cleared until
+	 * the real writing operation is finished
+	 */
+	for (t = 0; t < 3; t++) {
+		if ((readl(chip->base + SUNXI_LOSC_CTRL) &
+			SUNXI_LOSC_CTRL_RTC_YMD_ACC) && --t)
+			break;
+		else
+			msleep(50);
+	}
+	if (t == 0) {
+		dev_err(dev, "Failed to set rtc date.\n");
+		return -1;
+	}
+
+	/*
+	 * wait about 70us to make sure the the time is really written into
+	 * target
+	 */
+	usleep_range(70, 100);
+
+	return 0;
+}
+
+static int sunxi_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
+{
+	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
+
+	if (!enabled)
+		sunxi_rtc_setaie(enabled, chip);
+
+	return 0;
+}
+
+static const struct rtc_class_ops sunxi_rtc_ops = {
+	.read_time		= sunxi_rtc_gettime,
+	.set_time		= sunxi_rtc_settime,
+	.read_alarm		= sunxi_rtc_getalarm,
+	.set_alarm		= sunxi_rtc_setalarm,
+	.alarm_irq_enable	= sunxi_rtc_alarm_irq_enable
+};
+
+static const struct of_device_id sunxi_rtc_dt_ids[] = {
+	{ .compatible = "allwinner,sun4i-rtc", .data = &data_year_param[0] },
+	{ .compatible = "allwinner,sun7i-a20-rtc", .data = &data_year_param[1] },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, sunxi_rtc_dt_ids);
+
+static int sunxi_rtc_probe(struct platform_device *pdev)
+{
+	struct sunxi_rtc_dev *chip;
+	struct resource *res;
+	const struct of_device_id *of_id;
+	int ret;
+
+	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
+	if (!chip)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, chip);
+	chip->dev = &pdev->dev;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	chip->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(chip->base))
+		return PTR_ERR(chip->base);
+
+	chip->irq = platform_get_irq(pdev, 0);
+	if (chip->irq < 0) {
+		dev_err(&pdev->dev, "No IRQ resource\n");
+		return chip->irq;
+	}
+	ret = devm_request_irq(&pdev->dev, chip->irq, sunxi_rtc_alarmirq,
+			0, dev_name(&pdev->dev), chip);
+	if (ret) {
+		dev_err(&pdev->dev, "Could not request IRQ\n");
+		return ret;
+	}
+
+	of_id = of_match_device(sunxi_rtc_dt_ids, &pdev->dev);
+	if (!of_id) {
+		dev_err(&pdev->dev, "Unable to setup RTC data\n");
+		return -ENODEV;
+	}
+	chip->data_year = (struct sunxi_rtc_data_year *) of_id->data;
+
+	/* clear the alarm count value */
+	writel(0, chip->base + SUNXI_ALRM_DHMS);
+
+	/* disable alarm, not generate irq pending */
+	writel(0, chip->base + SUNXI_ALRM_EN);
+
+	/* disable alarm week/cnt irq, unset to cpu */
+	writel(0, chip->base + SUNXI_ALRM_IRQ_EN);
+
+	/* clear alarm week/cnt irq pending */
+	writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND, chip->base + SUNXI_ALRM_IRQ_STA);
+
+	chip->rtc = rtc_device_register("rtc-sunxi", &pdev->dev,
+			&sunxi_rtc_ops, THIS_MODULE);
+	if (IS_ERR(chip->rtc)) {
+		dev_err(&pdev->dev, "unable to register device\n");
+		return PTR_ERR(chip->rtc);
+	}
+
+	dev_info(&pdev->dev, "RTC enabled\n");
+
+	return 0;
+}
+
+static int sunxi_rtc_remove(struct platform_device *pdev)
+{
+	struct sunxi_rtc_dev *chip = platform_get_drvdata(pdev);
+
+	rtc_device_unregister(chip->rtc);
+
+	return 0;
+}
+
+static struct platform_driver sunxi_rtc_driver = {
+	.probe		= sunxi_rtc_probe,
+	.remove		= sunxi_rtc_remove,
+	.driver		= {
+		.name		= "sunxi-rtc",
+		.owner		= THIS_MODULE,
+		.of_match_table = sunxi_rtc_dt_ids,
+	},
+};
+
+module_platform_driver(sunxi_rtc_driver);
+
+MODULE_DESCRIPTION("sunxi RTC driver");
+MODULE_AUTHOR("Carlo Caione <carlo.caione@gmail.com>");
+MODULE_LICENSE("GPL");
-- 
1.8.4.2

[PATCH v3 2/2] ARM: sun4i/sun7i: DT documentation for RTC driver

[Carlo Caione]

Add DT documentation for SUNXI RTC driver

Signed-off-by: Carlo Caione <carlo.caione@gmail.com>
---
 Documentation/devicetree/bindings/rtc/sunxi-rtc.txt | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/rtc/sunxi-rtc.txt

diff --git a/Documentation/devicetree/bindings/rtc/sunxi-rtc.txt b/Documentation/devicetree/bindings/rtc/sunxi-rtc.txt
new file mode 100644
index 0000000..7cb9dbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/rtc/sunxi-rtc.txt
@@ -0,0 +1,17 @@
+* sun4i/sun7i Real Time Clock
+
+RTC controller for the Allwinner A10/A20
+
+Required properties:
+- compatible : Should be "allwinner,sun4i-rtc" or "allwinner,sun7i-a20-rtc"
+- reg: physical base address of the controller and length of memory mapped
+  region.
+- interrupts: IRQ line for the RTC.
+
+Example:
+
+rtc: rtc at 01c20d00 {
+	compatible = "allwinner,sun4i-rtc";
+	reg = <0x01c20d00 0x20>;
+	interrupts = <24>;
+};
-- 
1.8.4.2

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Carlo Caione]

On Wed, Oct 30, 2013 at 11:38 PM, Carlo Caione <carlo.caione@gmail.com> wrote:
> This patch introduces the driver for the RTC in the Allwinner A10 and
> A20 SoCs.
>
> Signed-off-by: Carlo Caione <carlo.caione@gmail.com>

Ping.
Alessandro, any feedback on this patch?

Best,

--
Carlo Caione

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Maxime Ripard]

Hi Carlo,

On Wed, Oct 30, 2013 at 11:38:57PM +0100, Carlo Caione wrote:
> This patch introduces the driver for the RTC in the Allwinner A10 and
> A20 SoCs.
> 
> Signed-off-by: Carlo Caione <carlo.caione@gmail.com>
> ---
> v1->v2:
> - Patch split in two
> - Fix coding style
> - New driver description
> - Rework macros
> - Add comments
> - Change while in for cycles
> 
> v2->v3
> - Fix comments
> - Rename alarm->alrm
> - Fix comments style
> - Remove useless newline
> ---
>  drivers/rtc/Kconfig     |   7 +
>  drivers/rtc/Makefile    |   1 +
>  drivers/rtc/rtc-sunxi.c | 491 ++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 499 insertions(+)
>  create mode 100644 drivers/rtc/rtc-sunxi.c
> 
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 9654aa3..ef45e0b 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -1076,6 +1076,13 @@ config RTC_DRV_SUN4V
>  	  If you say Y here you will get support for the Hypervisor
>  	  based RTC on SUN4V systems.
>  
> +config RTC_DRV_SUNXI
> +	tristate "Allwinner sun4i/sun7i RTC"
> +	depends on ARCH_SUNXI
> +	help
> +	  If you say Y here you will get support for the RTC found on
> +	  Allwinner A10/A20.
> +
>  config RTC_DRV_STARFIRE
>  	bool "Starfire RTC"
>  	depends on SPARC64
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 2dff3d2..8b52b5a 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -115,6 +115,7 @@ obj-$(CONFIG_RTC_DRV_STARFIRE)	+= rtc-starfire.o
>  obj-$(CONFIG_RTC_DRV_STK17TA8)	+= rtc-stk17ta8.o
>  obj-$(CONFIG_RTC_DRV_STMP)	+= rtc-stmp3xxx.o
>  obj-$(CONFIG_RTC_DRV_SUN4V)	+= rtc-sun4v.o
> +obj-$(CONFIG_RTC_DRV_SUNXI)	+= rtc-sunxi.o
>  obj-$(CONFIG_RTC_DRV_TEGRA)	+= rtc-tegra.o
>  obj-$(CONFIG_RTC_DRV_TEST)	+= rtc-test.o
>  obj-$(CONFIG_RTC_DRV_TILE)	+= rtc-tile.o
> diff --git a/drivers/rtc/rtc-sunxi.c b/drivers/rtc/rtc-sunxi.c
> new file mode 100644
> index 0000000..89c331f
> --- /dev/null
> +++ b/drivers/rtc/rtc-sunxi.c
> @@ -0,0 +1,491 @@
> +/*
> + * An RTC driver for Allwinner A10/A20
> + *
> + * Copyright (c) 2013, Carlo Caione <carlo.caione@gmail.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.
> + *
> + * You should have received a copy of the GNU General Public License along
> + * with this program; if not, write to the Free Software Foundation, Inc.,
> + * 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/err.h>
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/platform_device.h>
> +#include <linux/rtc.h>
> +#include <linux/types.h>
> +
> +#define SUNXI_LOSC_CTRL				0x0000
> +#define SUNXI_LOSC_CTRL_RTC_HMS_ACC		BIT(8)
> +#define SUNXI_LOSC_CTRL_RTC_YMD_ACC		BIT(7)
> +
> +#define SUNXI_RTC_YMD				0x0004
> +
> +#define SUNXI_RTC_HMS				0x0008
> +
> +#define SUNXI_ALRM_DHMS				0x000c
> +
> +#define SUNXI_ALRM_EN				0x0014
> +#define SUNXI_ALRM_EN_CNT_EN			BIT(8)
> +
> +#define SUNXI_ALRM_IRQ_EN			0x0018
> +#define SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN		BIT(0)
> +
> +#define SUNXI_ALRM_IRQ_STA			0x001c
> +#define SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND		BIT(0)
> +
> +#define SUNXI_LOSC_CTRL_RTC_ACC \
> +	(SUNXI_LOSC_CTRL_RTC_HMS_ACC | SUNXI_LOSC_CTRL_RTC_YMD_ACC)
> +
> +#define SUNXI_MASK_DH				0x0000001f
> +#define SUNXI_MASK_SM				0x0000003f
> +#define SUNXI_MASK_M				0x0000000f
> +#define SUNXI_MASK_LY				0x00000001
> +#define SUNXI_MASK_D				0x00000ffe
> +#define SUNXI_MASK_M				0x0000000f
> +
> +#define SUNXI_GET(x, mask, shift)		(((x) & ((mask) << (shift))) \
> +							>> (shift))
> +
> +#define SUNXI_SET(x, mask, shift)		(((x) & (mask)) << (shift))
> +
> +/* Get date values */
> +#define SUNXI_DATE_GET_DAY_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 0)
> +#define SUNXI_DATE_GET_MON_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_M, 8)
> +#define SUNXI_DATE_GET_YEAR_VALUE(x, mask)	SUNXI_GET(x, mask, 16)
> +
> +/* Get time values */
> +#define SUNXI_TIME_GET_SEC_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 0)
> +#define SUNXI_TIME_GET_MIN_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 8)
> +#define SUNXI_TIME_GET_HOUR_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 16)
> +
> +/* Get alarm values */
> +#define SUNXI_ALRM_GET_SEC_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 0)
> +#define SUNXI_ALRM_GET_MIN_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_SM, 8)
> +#define SUNXI_ALRM_GET_HOUR_VALUE(x)		SUNXI_GET(x, SUNXI_MASK_DH, 16)
> +
> +/* Set date values */
> +#define SUNXI_DATE_SET_DAY_VALUE(x)		SUNXI_DATE_GET_DAY_VALUE(x)
> +#define SUNXI_DATE_SET_MON_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_M, 8)
> +#define SUNXI_DATE_SET_YEAR_VALUE(x, mask)	SUNXI_SET(x, mask, 16)
> +#define SUNXI_LEAP_SET_VALUE(x, shift)		SUNXI_SET(x, SUNXI_MASK_LY, shift)
> +
> +/* Set time values */
> +#define SUNXI_TIME_SET_SEC_VALUE(x)		SUNXI_TIME_GET_SEC_VALUE(x)
> +#define SUNXI_TIME_SET_MIN_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_SM, 8)
> +#define SUNXI_TIME_SET_HOUR_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_DH, 16)
> +
> +/* set alarm values */
> +#define SUNXI_ALRM_SET_SEC_VALUE(x)		SUNXI_ALRM_GET_SEC_VALUE(x)
> +#define SUNXI_ALRM_SET_MIN_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_SM, 8)
> +#define SUNXI_ALRM_SET_HOUR_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_DH, 16)
> +#define SUNXI_ALRM_SET_DAY_VALUE(x)		SUNXI_SET(x, SUNXI_MASK_D, 21)
> +
> +/* time unit conversions */
> +#define SEC_IN_MIN				60
> +#define SEC_IN_HOUR				(60 * SEC_IN_MIN)
> +#define SEC_IN_DAY				(24 * SEC_IN_HOUR)
> +
> +struct sunxi_rtc_data_year {
> +	unsigned int min;		/* min year allowed */
> +	unsigned int max;		/* max year allowed */
> +	unsigned int off;		/* year offset to get the current year */
> +	unsigned int mask;
> +	unsigned char leap_shift;	/* bit shift to get the leap year */
> +};
> +
> +static struct sunxi_rtc_data_year data_year_param[] = {
> +	[0] = {
> +		.min		= 1970,
> +		.max		= 2100,
> +		.off		= 0,
> +		.mask		= 0x000000ff,
> +		.leap_shift	= 24,
> +	},
> +	[1] = {
> +		.min		= 2010,
> +		.max		= 2073,
> +		.off		= 110,
> +		.mask		= 0x0000003f,
> +		.leap_shift	= 22,
> +	},
> +};
> +
> +struct sunxi_rtc_dev {
> +	struct rtc_device *rtc;
> +	struct device *dev;
> +	struct sunxi_rtc_data_year *data_year;
> +	void __iomem *base;
> +	int irq;
> +};
> +
> +static irqreturn_t sunxi_rtc_alarmirq(int irq, void *id)
> +{
> +	struct sunxi_rtc_dev *chip = (struct sunxi_rtc_dev *) id;
> +	u32 val;
> +
> +	val = readl(chip->base + SUNXI_ALRM_IRQ_STA);
> +
> +	if (val & SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND) {
> +		val |= SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND;
> +		writel(val, chip->base + SUNXI_ALRM_IRQ_STA);
> +
> +		rtc_update_irq(chip->rtc, 1, RTC_AF | RTC_IRQF);
> +
> +		return IRQ_HANDLED;
> +	}
> +
> +	return IRQ_NONE;
> +}
> +
> +static void sunxi_rtc_setaie(int to, struct sunxi_rtc_dev *chip)
> +{
> +	u32 alrm_val = 0;
> +	u32 alrm_irq_val = 0;
> +
> +	if (to) {
> +		alrm_val = readl(chip->base + SUNXI_ALRM_EN);
> +		alrm_val |= SUNXI_ALRM_EN_CNT_EN;
> +
> +		alrm_irq_val = readl(chip->base + SUNXI_ALRM_IRQ_EN);
> +		alrm_irq_val |= SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN;
> +	} else {
> +		writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND,
> +				chip->base + SUNXI_ALRM_IRQ_STA);
> +	}
> +
> +	writel(alrm_val, chip->base + SUNXI_ALRM_EN);
> +	writel(alrm_irq_val, chip->base + SUNXI_ALRM_IRQ_EN);
> +}
> +
> +static int sunxi_rtc_getalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> +	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> +	struct rtc_time *alrm_tm = &wkalrm->time;
> +	u32 alrm;
> +	u32 alrm_en;
> +	u32 date;
> +
> +	alrm = readl(chip->base + SUNXI_ALRM_DHMS);
> +	date = readl(chip->base + SUNXI_RTC_YMD);
> +
> +	alrm_tm->tm_sec = SUNXI_ALRM_GET_SEC_VALUE(alrm);
> +	alrm_tm->tm_min = SUNXI_ALRM_GET_MIN_VALUE(alrm);
> +	alrm_tm->tm_hour = SUNXI_ALRM_GET_HOUR_VALUE(alrm);
> +
> +	alrm_tm->tm_mday = SUNXI_DATE_GET_DAY_VALUE(date);
> +	alrm_tm->tm_mon = SUNXI_DATE_GET_MON_VALUE(date);
> +	alrm_tm->tm_year = SUNXI_DATE_GET_YEAR_VALUE(date,
> +			chip->data_year->mask);
> +
> +	alrm_tm->tm_year += chip->data_year->off;
> +	alrm_tm->tm_mon -= 1;
> +
> +	alrm_en = readl(chip->base + SUNXI_ALRM_IRQ_EN);
> +	if (alrm_en & SUNXI_ALRM_EN_CNT_EN)
> +		wkalrm->enabled = 1;
> +
> +	return 0;
> +}
> +
> +static int sunxi_rtc_gettime(struct device *dev, struct rtc_time *rtc_tm)
> +{
> +	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> +	u32 date, time;
> +	int t;
> +
> +	/*
> +	 * read again if the system was mid-updated
> +	 */
> +	for (t = 0; t < 2; t++) {
> +		date = readl(chip->base + SUNXI_RTC_YMD);
> +		time = readl(chip->base + SUNXI_RTC_HMS);
> +
> +		rtc_tm->tm_sec  = SUNXI_TIME_GET_SEC_VALUE(time);
> +		rtc_tm->tm_min  = SUNXI_TIME_GET_MIN_VALUE(time);
> +		rtc_tm->tm_hour = SUNXI_TIME_GET_HOUR_VALUE(time);
> +
> +		rtc_tm->tm_mday = SUNXI_DATE_GET_DAY_VALUE(date);
> +		rtc_tm->tm_mon  = SUNXI_DATE_GET_MON_VALUE(date);
> +		rtc_tm->tm_year = SUNXI_DATE_GET_YEAR_VALUE(date,
> +				chip->data_year->mask);
> +
> +		if (rtc_tm->tm_sec == 0)
> +			msleep(500);
> +		else
> +			break;

So this means that whenever the time actually has a number of seconds
= 0, you just loop over, adding a 500ms delay? That seems pretty
inefficient to me.

Also, what happens if you get two times in a row a number of seconds
equals to 0?

This kind of construct seems really really odd, and doesn't the issue
you were trying to address.

However, this looks like a common issue. Have you looked at the other
drivers to see how they are handling it? Maybe Alessandro will have
some suggestions as well.

> +	}
> +
> +	rtc_tm->tm_year += chip->data_year->off;
> +	rtc_tm->tm_mon  -= 1;
> +
> +	return rtc_valid_tm(rtc_tm);
> +}
> +
> +static int sunxi_rtc_setalarm(struct device *dev, struct rtc_wkalrm *wkalrm)
> +{
> +	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> +	struct rtc_time *alrm_tm = &wkalrm->time;
> +	struct rtc_time tm_now;
> +	u32 alrm = 0;
> +	unsigned long time_now = 0;
> +	unsigned long time_set = 0;
> +	unsigned long time_gap = 0;
> +	unsigned long time_gap_day = 0;
> +	unsigned long time_gap_hour = 0;
> +	unsigned long time_gap_min = 0;
> +	int ret = 0;
> +
> +	ret = sunxi_rtc_gettime(dev, &tm_now);
> +	if (ret < 0) {
> +		dev_err(dev, "Error in getting time\n");
> +		return -EINVAL;
> +	}
> +
> +	rtc_tm_to_time(alrm_tm, &time_set);
> +	rtc_tm_to_time(&tm_now, &time_now);
> +	if (time_set <= time_now) {
> +		dev_err(dev, "Date to set in the past\n");
> +		return -EINVAL;
> +	}
> +
> +	time_gap = time_set - time_now;
> +	time_gap_day = time_gap / SEC_IN_DAY;
> +	time_gap -= time_gap_day * SEC_IN_DAY;
> +	time_gap_hour = time_gap / SEC_IN_HOUR;
> +	time_gap -= time_gap_hour * SEC_IN_HOUR;
> +	time_gap_min = time_gap / SEC_IN_MIN;
> +	time_gap -= time_gap_min * SEC_IN_MIN;
> +
> +	if (time_gap_day > 255) {
> +		dev_err(dev, "Day must be in the range 0 - 255\n");
> +		return -EINVAL;
> +	}
> +
> +	sunxi_rtc_setaie(0, chip);
> +	writel(0, chip->base + SUNXI_ALRM_DHMS);
> +	usleep_range(100, 300);
> +
> +	alrm = SUNXI_ALRM_SET_SEC_VALUE(time_gap) |
> +		SUNXI_ALRM_SET_MIN_VALUE(time_gap_min) |
> +		SUNXI_ALRM_SET_HOUR_VALUE(time_gap_hour) |
> +		SUNXI_ALRM_SET_DAY_VALUE(time_gap_day);
> +	writel(alrm, chip->base + SUNXI_ALRM_DHMS);
> +
> +	writel(0, chip->base + SUNXI_ALRM_IRQ_EN);
> +	writel(SUNXI_ALRM_IRQ_EN_CNT_IRQ_EN, chip->base + SUNXI_ALRM_IRQ_EN);
> +
> +	sunxi_rtc_setaie(wkalrm->enabled, chip);
> +
> +	return 0;
> +}
> +
> +static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
> +{
> +	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> +	u32 date = 0;
> +	u32 time = 0;
> +	int year;
> +	int t;
> +
> +	year = rtc_tm->tm_year + 1900;

So, that means that the A10 RTC starts a year 1900, and the A20 at
year 2010? Why not just put those in your year_offset field directly?

> +	if (year < chip->data_year->min || year > chip->data_year->max) {
> +		dev_err(dev, "rtc only supports year in range %d - %d\n",
> +				chip->data_year->min, chip->data_year->max);
> +		return -EINVAL;
> +	}
> +
> +	rtc_tm->tm_year -= chip->data_year->off;
> +	rtc_tm->tm_mon += 1;
> +
> +	date = SUNXI_DATE_SET_DAY_VALUE(rtc_tm->tm_mday) |
> +		SUNXI_DATE_SET_MON_VALUE(rtc_tm->tm_mon)  |
> +		SUNXI_DATE_SET_YEAR_VALUE(rtc_tm->tm_year,
> +				chip->data_year->mask);
> +
> +	if (is_leap_year(year))
> +		date |= SUNXI_LEAP_SET_VALUE(1, chip->data_year->leap_shift);
> +
> +	time = SUNXI_TIME_SET_SEC_VALUE(rtc_tm->tm_sec)  |
> +		SUNXI_TIME_SET_MIN_VALUE(rtc_tm->tm_min)  |
> +		SUNXI_TIME_SET_HOUR_VALUE(rtc_tm->tm_hour);
> +
> +	writel(0, chip->base + SUNXI_RTC_HMS);
> +	writel(0, chip->base + SUNXI_RTC_YMD);
> +
> +	writel(time, chip->base + SUNXI_RTC_HMS);
> +
> +	/*
> +	 * After writing the RCT HH-MM-SS register, the
> +	 * SUNXI_LOSC_CTRL_RTC_HMS_ACC bit is set and it will be cleared until
> +	 * the real writing operation is finished
> +	 */
> +	for (t = 0; t < 3; t++) {
> +		if ((readl(chip->base + SUNXI_LOSC_CTRL) &
> +			SUNXI_LOSC_CTRL_RTC_HMS_ACC) && --t)
> +			break;
> +		else
> +			msleep(50);
> +	}
> +	if (t == 0) {
> +		dev_err(dev, "Failed to set rtc time.\n");
> +		return -1;
> +	}

Why not just busy-wait ? (with a timeout if you really want to).

> +
> +	writel(date, chip->base + SUNXI_RTC_YMD);
> +
> +	/*
> +	 * After writing the RCT YY-MM-DD register, the
> +	 * SUNXI_LOSC_CTRL_RTC_YMD_ACC bit is set and it will be cleared until
> +	 * the real writing operation is finished
> +	 */
> +	for (t = 0; t < 3; t++) {
> +		if ((readl(chip->base + SUNXI_LOSC_CTRL) &
> +			SUNXI_LOSC_CTRL_RTC_YMD_ACC) && --t)
> +			break;
> +		else
> +			msleep(50);
> +	}

Ditto.

> +	if (t == 0) {
> +		dev_err(dev, "Failed to set rtc date.\n");
> +		return -1;
> +	}
> +
> +	/*
> +	 * wait about 70us to make sure the the time is really written into
> +	 * target
> +	 */
> +	usleep_range(70, 100);

Isn't the write operation supposed to be done already?

> +	return 0;
> +}
> +
> +static int sunxi_rtc_alarm_irq_enable(struct device *dev, unsigned int enabled)
> +{
> +	struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> +
> +	if (!enabled)
> +		sunxi_rtc_setaie(enabled, chip);
> +
> +	return 0;
> +}
> +
> +static const struct rtc_class_ops sunxi_rtc_ops = {
> +	.read_time		= sunxi_rtc_gettime,
> +	.set_time		= sunxi_rtc_settime,
> +	.read_alarm		= sunxi_rtc_getalarm,
> +	.set_alarm		= sunxi_rtc_setalarm,
> +	.alarm_irq_enable	= sunxi_rtc_alarm_irq_enable
> +};
> +
> +static const struct of_device_id sunxi_rtc_dt_ids[] = {
> +	{ .compatible = "allwinner,sun4i-rtc", .data = &data_year_param[0] },
> +	{ .compatible = "allwinner,sun7i-a20-rtc", .data = &data_year_param[1] },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, sunxi_rtc_dt_ids);
> +
> +static int sunxi_rtc_probe(struct platform_device *pdev)
> +{
> +	struct sunxi_rtc_dev *chip;
> +	struct resource *res;
> +	const struct of_device_id *of_id;
> +	int ret;
> +
> +	chip = devm_kzalloc(&pdev->dev, sizeof(*chip), GFP_KERNEL);
> +	if (!chip)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, chip);
> +	chip->dev = &pdev->dev;
> +
> +	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	chip->base = devm_ioremap_resource(&pdev->dev, res);
> +	if (IS_ERR(chip->base))
> +		return PTR_ERR(chip->base);
> +
> +	chip->irq = platform_get_irq(pdev, 0);
> +	if (chip->irq < 0) {
> +		dev_err(&pdev->dev, "No IRQ resource\n");
> +		return chip->irq;
> +	}
> +	ret = devm_request_irq(&pdev->dev, chip->irq, sunxi_rtc_alarmirq,
> +			0, dev_name(&pdev->dev), chip);
> +	if (ret) {
> +		dev_err(&pdev->dev, "Could not request IRQ\n");
> +		return ret;
> +	}
> +
> +	of_id = of_match_device(sunxi_rtc_dt_ids, &pdev->dev);
> +	if (!of_id) {
> +		dev_err(&pdev->dev, "Unable to setup RTC data\n");
> +		return -ENODEV;
> +	}
> +	chip->data_year = (struct sunxi_rtc_data_year *) of_id->data;
> +
> +	/* clear the alarm count value */
> +	writel(0, chip->base + SUNXI_ALRM_DHMS);
> +
> +	/* disable alarm, not generate irq pending */
> +	writel(0, chip->base + SUNXI_ALRM_EN);
> +
> +	/* disable alarm week/cnt irq, unset to cpu */
> +	writel(0, chip->base + SUNXI_ALRM_IRQ_EN);
> +
> +	/* clear alarm week/cnt irq pending */
> +	writel(SUNXI_ALRM_IRQ_STA_CNT_IRQ_PEND, chip->base + SUNXI_ALRM_IRQ_STA);
> +
> +	chip->rtc = rtc_device_register("rtc-sunxi", &pdev->dev,
> +			&sunxi_rtc_ops, THIS_MODULE);
> +	if (IS_ERR(chip->rtc)) {
> +		dev_err(&pdev->dev, "unable to register device\n");
> +		return PTR_ERR(chip->rtc);
> +	}
> +
> +	dev_info(&pdev->dev, "RTC enabled\n");
> +
> +	return 0;
> +}
> +
> +static int sunxi_rtc_remove(struct platform_device *pdev)
> +{
> +	struct sunxi_rtc_dev *chip = platform_get_drvdata(pdev);
> +
> +	rtc_device_unregister(chip->rtc);
> +
> +	return 0;
> +}
> +
> +static struct platform_driver sunxi_rtc_driver = {
> +	.probe		= sunxi_rtc_probe,
> +	.remove		= sunxi_rtc_remove,
> +	.driver		= {
> +		.name		= "sunxi-rtc",
> +		.owner		= THIS_MODULE,
> +		.of_match_table = sunxi_rtc_dt_ids,
> +	},
> +};
> +
> +module_platform_driver(sunxi_rtc_driver);
> +
> +MODULE_DESCRIPTION("sunxi RTC driver");
> +MODULE_AUTHOR("Carlo Caione <carlo.caione@gmail.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 1.8.4.2
> 

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131106/fc6a4e0d/attachment-0001.sig>

[PATCH v3 2/2] ARM: sun4i/sun7i: DT documentation for RTC driver

[Maxime Ripard]

On Wed, Oct 30, 2013 at 11:38:58PM +0100, Carlo Caione wrote:
> Add DT documentation for SUNXI RTC driver
> 
> Signed-off-by: Carlo Caione <carlo.caione@gmail.com>

Acked-by: Maxime Ripard <maxime.ripard@free-electrons.com>

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131106/6181f0d9/attachment.sig>

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Carlo Caione]

On Wed, Nov 6, 2013 at 3:13 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Carlo,
[cut]

> So this means that whenever the time actually has a number of seconds
> = 0, you just loop over, adding a 500ms delay? That seems pretty
> inefficient to me.

I agree. Any suggestion is welcome.

> Also, what happens if you get two times in a row a number of seconds
> equals to 0?

in that case I think that using t <= 2 in the for cycle will solve the problem.

> This kind of construct seems really really odd, and doesn't the issue
> you were trying to address.
>
> However, this looks like a common issue. Have you looked at the other
> drivers to see how they are handling it? Maybe Alessandro will have
> some suggestions as well.

This portion of the code is taken from the original RTC driver leaked
from Allwinner.
Otherwise a better solution could be something like in
http://lxr.linux.no/linux+v3.12/drivers/rtc/rtc-at91rm9200.c#L114

> So, that means that the A10 RTC starts a year 1900, and the A20 at
> year 2010? Why not just put those in your year_offset field directly?

Agree

> Why not just busy-wait ? (with a timeout if you really want to).

because if it fails after 150ms you are almost certainly screwed and
something is wrong

> Isn't the write operation supposed to be done already?

In theory yes, in practice you cannot be sure. This is also a legacy
of the old AW's code. I'll investigate more to check if this is really
useful.

Thanks,

--
Carlo Caione

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Maxime Ripard]

On Wed, Nov 06, 2013 at 10:07:36PM +0100, Carlo Caione wrote:
> > So this means that whenever the time actually has a number of seconds
> > = 0, you just loop over, adding a 500ms delay? That seems pretty
> > inefficient to me.
> 
> I agree. Any suggestion is welcome.
> 
> > Also, what happens if you get two times in a row a number of seconds
> > equals to 0?
> 
> in that case I think that using t <= 2 in the for cycle will solve
> the problem.

I don't see how it actually solves the issue. It just makes it less
likely to happen.

> > This kind of construct seems really really odd, and doesn't the issue
> > you were trying to address.
> >
> > However, this looks like a common issue. Have you looked at the other
> > drivers to see how they are handling it? Maybe Alessandro will have
> > some suggestions as well.
> 
> This portion of the code is taken from the original RTC driver leaked
> from Allwinner.
> Otherwise a better solution could be something like in
> http://lxr.linux.no/linux+v3.12/drivers/rtc/rtc-at91rm9200.c#L114

That sounds much more robust yes.

> > So, that means that the A10 RTC starts a year 1900, and the A20 at
> > year 2010? Why not just put those in your year_offset field directly?
> 
> Agree
> 
> > Why not just busy-wait ? (with a timeout if you really want to).
> 
> because if it fails after 150ms you are almost certainly screwed and
> something is wrong

Hmmm, I'm not sure anymore about the code I was commenting about here
(please try to at least keep the relevant code portions in your
mails), but you can be delayed by 150ms even if nothing went wrong.

And you can always have a timeout while busy-waiting if you want, with
something like
http://lxr.free-electrons.com/source/drivers/spi/spi-mxs.c#L166

> > Isn't the write operation supposed to be done already?
> 
> In theory yes, in practice you cannot be sure. This is also a legacy
> of the old AW's code. I'll investigate more to check if this is really
> useful.

Again, I'm not sure about which part of the code we were talking
about, but I think I recall that there was two similar access in your
drivers, once that was doing what I was commenting about, and the
other didn't. I'm fine with it being required, but if it is, it should
always be required.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131108/99894fbe/attachment.sig>

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Carlo Caione]

On Wed, Nov 6, 2013 at 3:13 PM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Carlo,

Hi Maxime,
A couple of points for the incoming v4

>> +static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
>> +{
>> +     struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
>> +     u32 date = 0;
>> +     u32 time = 0;
>> +     int year;
>> +     int t;
>> +
>> +     year = rtc_tm->tm_year + 1900;
>
> So, that means that the A10 RTC starts a year 1900, and the A20 at
> year 2010? Why not just put those in your year_offset field directly?

1900 is the offset of the parameter tm_year with respect to the current year.
year_offset is used to make tm_year fit inside the limited range of
the year field of the SUNXI_RTC_YMD register.

>> +
>> +     /*
>> +      * wait about 70us to make sure the the time is really written into
>> +      * target
>> +      */
>> +     usleep_range(70, 100);
>
> Isn't the write operation supposed to be done already?

Yes, it is supposed to be, but this waiting was already present in the
original version of the driver by aw so I'll leave it in v4.

Best,

--
Carlo Caione

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Maxime Ripard]

Hi Carlo,

On Fri, Nov 15, 2013 at 11:50:56PM +0100, Carlo Caione wrote:
> >> +static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
> >> +{
> >> +     struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> >> +     u32 date = 0;
> >> +     u32 time = 0;
> >> +     int year;
> >> +     int t;
> >> +
> >> +     year = rtc_tm->tm_year + 1900;
> >
> > So, that means that the A10 RTC starts a year 1900, and the A20 at
> > year 2010? Why not just put those in your year_offset field directly?
> 
> 1900 is the offset of the parameter tm_year with respect to the current year.
> year_offset is used to make tm_year fit inside the limited range of
> the year field of the SUNXI_RTC_YMD register.

Ok, so 1900 is the RTC's epoch, and tm_year is relative to that
epoch. What I still don't get is why you're not defining it as such
and use it directly the field your year_offset as such.

Moreover, you have some max values that imply that the RTC can't count
over either 2100 or 2073, which is kind of weird, since, respectively,
it starts at 1900 and 2010, and the year field is documented to be 8
bits wide, I'd expect it to go up to 2155 and 2265.

> >> +
> >> +     /*
> >> +      * wait about 70us to make sure the the time is really written into
> >> +      * target
> >> +      */
> >> +     usleep_range(70, 100);
> >
> > Isn't the write operation supposed to be done already?
> 
> Yes, it is supposed to be, but this waiting was already present in the
> original version of the driver by aw so I'll leave it in v4.

I feel like we argue over this again and again.

I'm sorry, but saying "allwinner did it that way so we should keep it"
is a bad argument. Otherwise we would be using fex, and every other
driver Allwinner reimplemented.

That being said, either that sleep is needed, and then it should be
needed everytime you write to the RTC, which is not the case iirc, and
have a comment saying "I have no idea why, but this come from
Allwinner and this is actually required", or you don't need it, and
then you can just remove it.

But having it some-times-but-not-some-other seems pretty weak to me.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131116/6468848d/attachment.sig>

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Carlo Caione]

On Sat, Nov 16, 2013 at 9:51 AM, Maxime Ripard
<maxime.ripard@free-electrons.com> wrote:
> Hi Carlo,
>
> On Fri, Nov 15, 2013 at 11:50:56PM +0100, Carlo Caione wrote:
>> >> +static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
>> >> +{
>> >> +     struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
>> >> +     u32 date = 0;
>> >> +     u32 time = 0;
>> >> +     int year;
>> >> +     int t;
>> >> +
>> >> +     year = rtc_tm->tm_year + 1900;
>> >
>> > So, that means that the A10 RTC starts a year 1900, and the A20 at
>> > year 2010? Why not just put those in your year_offset fieldd directly?
>>
>> 1900 is the offset of the parameter tm_year with respect to the current year.
>> year_offset is used to make tm_year fit inside the limited range of
>> the year field of the SUNXI_RTC_YMD register.
>
> Ok, so 1900 is the RTC's epoch, and tm_year is relative to that
> epoch. What I still don't get is why you're not defining it as such
> and use it directly the field your year_offset as such.

Because for A10 the year field is only 6 bit wide and it can hold only
64 years so I cannot use year_offset directly.
We have the RTC framework where everything is based on 1900 and the
date register that is only 6 bit wide. Without year_offset I would
cover only 64 years from 1900 to 1963.
I need year_offset to shift the base year from 1900 to 2010 such that
I can use those 6 bit to track years from 2010 to 2073.

> Moreover, you have some max values that imply that the RTC can't count
> over either 2100 or 2073, which is kind of weird, since, respectively,
> it starts at 1900 and 2010, and the year field is documented to be 8
> bits wide, I'd expect it to go up to 2155 and 2265.

For A10 the date register is documented to be 6 bit. For A20 it is 8
bit wide (here the "mask" field in the sunxi_rtc_data_year).
In the v4 I'm going to expand to 255 the year range for A20 and modify
the min year to make it starts also at 1900 (it as actually wrong
having a the min field set to 1970).
(BTW I just noticed that the indexes for sunxi_rtc_data_year are
swapped, one more fix in v4).

>> >> +
>> >> +     /*
>> >> +      * wait about 70us to make sure the the time is really written into
>> >> +      * target
>> >> +      */
>> >> +     usleep_range(70, 100);
>> >
>> > Isn't the write operation supposed to be done already?
>>
>> Yes, it is supposed to be, but this waiting was already present in the
>> original version of the driver by aw so I'll leave it in v4.
>
> I feel like we argue over this again and again.
>
> I'm sorry, but saying "allwinner did it that way so we should keep it"
> is a bad argument. Otherwise we would be using fex, and every other
> driver Allwinner reimplemented.
>
> That being said, either that sleep is needed, and then it should be
> needed everytime you write to the RTC, which is not the case iirc, and
> have a comment saying "I have no idea why, but this come from
> Allwinner and this is actually required", or you don't need it, and
> then you can just remove it.
>
> But having it some-times-but-not-some-other seems pretty weak to me.

Maxime, as you know documentation for AW's SoC is really weak. I can
only extrapolate information from the old drivers and the badly
written comments to the code.
In those drivers (in all the versions I have) this waiting is always
present with the same comment I have left in the driver. What I
imagined is that it is needed not always, but only when I write
SUNXI_LOSC_CTRL_RTC_HMS_ACC or SUNXI_LOSC_CTRL_RTC_YMD_ACC (again,
this is just a guess but in the comment there is an explicit reference
to "time").
I actually have no idea why and removing it in my tests doesn't affect
the RTC. So I had two possibilities: 1) leave it in the code because
somehow it seems important even though I don't know exactly why it is
(only) there or 2) leave it out knowing that it doesn't affect my
tests but it could in some particular case or configuration.
That said, I'm going to do some more tests without the usleep to see
if there is any problem otherwise I'll delete it in v4.

Thank you,

--
Carlo Caione

[PATCH v3 1/2] ARM: sun4i/sun7i: RTC driver

[Maxime Ripard]

On Sat, Nov 16, 2013 at 01:58:35PM +0100, Carlo Caione wrote:
> On Sat, Nov 16, 2013 at 9:51 AM, Maxime Ripard
> <maxime.ripard@free-electrons.com> wrote:
> > Hi Carlo,
> >
> > On Fri, Nov 15, 2013 at 11:50:56PM +0100, Carlo Caione wrote:
> >> >> +static int sunxi_rtc_settime(struct device *dev, struct rtc_time *rtc_tm)
> >> >> +{
> >> >> +     struct sunxi_rtc_dev *chip = dev_get_drvdata(dev);
> >> >> +     u32 date = 0;
> >> >> +     u32 time = 0;
> >> >> +     int year;
> >> >> +     int t;
> >> >> +
> >> >> +     year = rtc_tm->tm_year + 1900;
> >> >
> >> > So, that means that the A10 RTC starts a year 1900, and the A20 at
> >> > year 2010? Why not just put those in your year_offset fieldd directly?
> >>
> >> 1900 is the offset of the parameter tm_year with respect to the current year.
> >> year_offset is used to make tm_year fit inside the limited range of
> >> the year field of the SUNXI_RTC_YMD register.
> >
> > Ok, so 1900 is the RTC's epoch, and tm_year is relative to that
> > epoch. What I still don't get is why you're not defining it as such
> > and use it directly the field your year_offset as such.
> 
> Because for A10 the year field is only 6 bit wide and it can hold only
> 64 years so I cannot use year_offset directly.
> We have the RTC framework where everything is based on 1900 and the
> date register that is only 6 bit wide. Without year_offset I would
> cover only 64 years from 1900 to 1963.
> I need year_offset to shift the base year from 1900 to 2010 such that
> I can use those 6 bit to track years from 2010 to 2073.

I'm sorry, but this is not at all what you have in your driver.

the 6 bit width and the start-at-2010 is set for the A20, not the A10.

> > Moreover, you have some max values that imply that the RTC can't count
> > over either 2100 or 2073, which is kind of weird, since, respectively,
> > it starts at 1900 and 2010, and the year field is documented to be 8
> > bits wide, I'd expect it to go up to 2155 and 2265.
> 
> For A10 the date register is documented to be 6 bit. For A20 it is 8
> bit wide (here the "mask" field in the sunxi_rtc_data_year).
> In the v4 I'm going to expand to 255 the year range for A20 and modify
> the min year to make it starts also at 1900 (it as actually wrong
> having a the min field set to 1970).
> (BTW I just noticed that the indexes for sunxi_rtc_data_year are
> swapped, one more fix in v4).

Ah, yes, I guess you found out already.

Sorry for the confusion.

> >> >> +
> >> >> +     /*
> >> >> +      * wait about 70us to make sure the the time is really written into
> >> >> +      * target
> >> >> +      */
> >> >> +     usleep_range(70, 100);
> >> >
> >> > Isn't the write operation supposed to be done already?
> >>
> >> Yes, it is supposed to be, but this waiting was already present in the
> >> original version of the driver by aw so I'll leave it in v4.
> >
> > I feel like we argue over this again and again.
> >
> > I'm sorry, but saying "allwinner did it that way so we should keep it"
> > is a bad argument. Otherwise we would be using fex, and every other
> > driver Allwinner reimplemented.
> >
> > That being said, either that sleep is needed, and then it should be
> > needed everytime you write to the RTC, which is not the case iirc, and
> > have a comment saying "I have no idea why, but this come from
> > Allwinner and this is actually required", or you don't need it, and
> > then you can just remove it.
> >
> > But having it some-times-but-not-some-other seems pretty weak to me.
> 
> Maxime, as you know documentation for AW's SoC is really weak. I can
> only extrapolate information from the old drivers and the badly
> written comments to the code.
> In those drivers (in all the versions I have) this waiting is always
> present with the same comment I have left in the driver. What I
> imagined is that it is needed not always, but only when I write
> SUNXI_LOSC_CTRL_RTC_HMS_ACC or SUNXI_LOSC_CTRL_RTC_YMD_ACC (again,
> this is just a guess but in the comment there is an explicit reference
> to "time").

My point is that you're not even using that for every access to DHMS.

Let me see what I can find out about this.

> I actually have no idea why and removing it in my tests doesn't affect
> the RTC. So I had two possibilities: 1) leave it in the code because
> somehow it seems important even though I don't know exactly why it is
> (only) there or 2) leave it out knowing that it doesn't affect my
> tests but it could in some particular case or configuration.
> That said, I'm going to do some more tests without the usleep to see
> if there is any problem otherwise I'll delete it in v4.

Ok.

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131119/d0566852/attachment.sig>