[PATCH v10 0/3] Add rockchip RK808 pmic driver

[PATCH v10 0/3] Add rockchip RK808 pmic driver

[Chris Zhong]

This is the initial version of the RK808 PMIC. This is a power management IC
for multimedia products.

It provides regulators that are able to supply power to processor cores
and other components. The chip provides other modules including RTC, Clockout

Changes in v10:
Adviced by Doug
- modify rk808_rtc->irq err print
Adviced by Mike
- remove rk808_clkout1_is_prepared
Adviced by Dmitry Torokhov
- add rk808_clkout_remove

Changes in v9:
Adviced by Dmitry
- if(rk808_rtc->irq < 0), return rk808_rtc->irq
Adviced by Doug
- add reg_np

Changes in v8:
Adviced by Doug
- coding style
- fix regmap_update_bits()
- if(rk808_rtc->irq < 0) return err num
Adviced by Mike
- rename "control" to "enable"
- define RK808_NR_OUTPUT 2
Adviced by Doug
- remove rk808_regulator_dts() function
- remove the check about "client->dev.of_node"

Changes in v7:
Adviced by doug
- read rtc time from shadowed registers
Adviced by Dmitry
- use CONFIG_PM_SLEEP replace CONFIG_PM
- use SIMPLE_DEV_PM_OPS replace dev_pm_ops
- fix dev_warn
- coding style
Adviced by Heiko
- remove rtc_ctl
Adviced by doug
-fix coding style problems
- remove pdata struct from header file, add rk808_regulator struct

Changes in v6:
Adviced by doug
- move RTC_READSEL setting into probe
Adviced by doug
- use correct argument call of_clk_add_provider in probe
- remove the redundant code

Changes in v5:
- fixed a bug about set_time failed
Adviced by doug
- add some error checking in probe
- move "rockchip,rk808.h" into the patch about dt-bindings
- re-edit base on Mark's branch

Changes in v4:
- use &client->dev replace rk808->dev
Adviced by doug
- add "clock-output-names" propertiey
- add a header file "rockchip,rk808.h"
- use &client->dev replace rk808->dev

Changes in v3:
- fix compile err

Changes in v2:
Adviced by javier.martinez
- Add a separate clock driver, rather than in RTC driver
Adviced by javier.martinez
- separated from rtc-rk808.c
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style

Chris Zhong (3):
  RTC: RK808: add RTC driver for RK808
  clk: RK808: Add clkout driver for RK808
  regulator: RK808: Remove pdata from the regulator

 drivers/clk/Kconfig                 |    9 +
 drivers/clk/Makefile                |    1 +
 drivers/clk/clk-rk808.c             |  170 ++++++++++++++
 drivers/regulator/rk808-regulator.c |   89 ++------
 drivers/rtc/Kconfig                 |   10 +
 drivers/rtc/Makefile                |    1 +
 drivers/rtc/rtc-rk808.c             |  427 +++++++++++++++++++++++++++++++++++
 7 files changed, 635 insertions(+), 72 deletions(-)
 create mode 100644 drivers/clk/clk-rk808.c
 create mode 100644 drivers/rtc/rtc-rk808.c

-- 
1.7.9.5

[PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

[Chris Zhong]

Adding RTC driver for supporting RTC device present inside RK808 PMIC.

Signed-off-by: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>

Tested-by: Heiko Stuebner <heiko@sntech.de>
---

Changes in v10:
Adviced by Doug
- modify rk808_rtc->irq err print

Changes in v9:
Adviced by Dmitry
- if(rk808_rtc->irq < 0), return rk808_rtc->irq

Changes in v8:
Adviced by Doug
- coding style
- fix regmap_update_bits()
- if(rk808_rtc->irq < 0) return err num

Changes in v7:
Adviced by doug
- read rtc time from shadowed registers
Adviced by Dmitry
- use CONFIG_PM_SLEEP replace CONFIG_PM
- use SIMPLE_DEV_PM_OPS replace dev_pm_ops
- fix dev_warn
- coding style
Adviced by Heiko
- remove rtc_ctl

Changes in v6:
Adviced by doug
- move RTC_READSEL setting into probe

Changes in v5:
- fixed a bug about set_time failed

Changes in v4:
- use &client->dev replace rk808->dev

Changes in v3: None
Changes in v2:
Adviced by javier.martinez
- Add a separate clock driver, rather than in RTC driver

 drivers/rtc/Kconfig     |   10 ++
 drivers/rtc/Makefile    |    1 +
 drivers/rtc/rtc-rk808.c |  427 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 438 insertions(+)
 create mode 100644 drivers/rtc/rtc-rk808.c

diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index a168e96..aeab9d9 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -288,6 +288,16 @@ config RTC_DRV_MAX77686
 	  This driver can also be built as a module. If so, the module
 	  will be called rtc-max77686.
 
+config RTC_DRV_RK808
+	tristate "Rockchip RK808 RTC"
+	depends on MFD_RK808
+	help
+	  If you say yes here you will get support for the
+	  RTC of RK808 PMIC.
+
+	  This driver can also be built as a module. If so, the module
+	  will be called rk808-rtc.
+
 config RTC_DRV_RS5C372
 	tristate "Ricoh R2025S/D, RS5C372A/B, RV5C386, RV5C387A"
 	help
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 56f061c..91fe4647 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -109,6 +109,7 @@ obj-$(CONFIG_RTC_DRV_PUV3)	+= rtc-puv3.o
 obj-$(CONFIG_RTC_DRV_PXA)	+= rtc-pxa.o
 obj-$(CONFIG_RTC_DRV_R9701)	+= rtc-r9701.o
 obj-$(CONFIG_RTC_DRV_RC5T583)	+= rtc-rc5t583.o
+obj-$(CONFIG_RTC_DRV_RK808)	+= rtc-rk808.o
 obj-$(CONFIG_RTC_DRV_RP5C01)	+= rtc-rp5c01.o
 obj-$(CONFIG_RTC_DRV_RS5C313)	+= rtc-rs5c313.o
 obj-$(CONFIG_RTC_DRV_RS5C348)	+= rtc-rs5c348.o
diff --git a/drivers/rtc/rtc-rk808.c b/drivers/rtc/rtc-rk808.c
new file mode 100644
index 0000000..56a0940
--- /dev/null
+++ b/drivers/rtc/rtc-rk808.c
@@ -0,0 +1,427 @@
+/*
+ * RTC driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong <zyw@rock-chips.com>
+ * Author: Zhang Qing <zhangqing@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/module.h>
+#include <linux/kernel.h>
+#include <linux/rtc.h>
+#include <linux/bcd.h>
+#include <linux/mfd/rk808.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+
+/* RTC_CTRL_REG bitfields */
+#define BIT_RTC_CTRL_REG_STOP_RTC_M		BIT(0)
+
+/* RK808 has a shadowed register for saving a "frozen" RTC time.
+ * When user setting "GET_TIME" to 1, the time will save in this shadowed
+ * register. If set "READSEL" to 1, user read rtc time register, actually
+ * get the time of that moment. If we need the real time, clr this bit.
+ */
+#define BIT_RTC_CTRL_REG_RTC_GET_TIME		BIT(6)
+#define BIT_RTC_CTRL_REG_RTC_READSEL_M		BIT(7)
+#define BIT_RTC_INTERRUPTS_REG_IT_ALARM_M	BIT(3)
+#define RTC_STATUS_MASK		0xFE
+
+#define SECONDS_REG_MSK		0x7F
+#define MINUTES_REG_MAK		0x7F
+#define HOURS_REG_MSK		0x3F
+#define DAYS_REG_MSK		0x3F
+#define MONTHS_REG_MSK		0x1F
+#define YEARS_REG_MSK		0xFF
+#define WEEKS_REG_MSK		0x7
+
+/* REG_SECONDS_REG through REG_YEARS_REG is how many registers? */
+
+#define NUM_TIME_REGS	(RK808_WEEKS_REG - RK808_SECONDS_REG + 1)
+#define NUM_ALARM_REGS	(RK808_ALARM_YEARS_REG - RK808_ALARM_SECONDS_REG + 1)
+
+struct rk808_rtc {
+	struct rk808 *rk808;
+	struct rtc_device *rtc;
+	int irq;
+};
+
+/* Read current time and date in RTC */
+static int rk808_rtc_readtime(struct device *dev, struct rtc_time *tm)
+{
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	u8 rtc_data[NUM_TIME_REGS];
+	int ret;
+
+	/* Force an update of the shadowed registers right now */
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
+				 BIT_RTC_CTRL_REG_RTC_GET_TIME,
+				 0);
+	if (ret) {
+		dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
+				 BIT_RTC_CTRL_REG_RTC_GET_TIME,
+				 BIT_RTC_CTRL_REG_RTC_GET_TIME);
+	if (ret) {
+		dev_err(dev, "Failed to update bits rtc_ctrl: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_bulk_read(rk808->regmap, RK808_SECONDS_REG,
+			       rtc_data, NUM_TIME_REGS);
+	if (ret) {
+		dev_err(dev, "Failed to bulk read rtc_data: %d\n", ret);
+		return ret;
+	}
+
+	tm->tm_sec = bcd2bin(rtc_data[0] & SECONDS_REG_MSK);
+	tm->tm_min = bcd2bin(rtc_data[1] & MINUTES_REG_MAK);
+	tm->tm_hour = bcd2bin(rtc_data[2] & HOURS_REG_MSK);
+	tm->tm_mday = bcd2bin(rtc_data[3] & DAYS_REG_MSK);
+	tm->tm_mon = (bcd2bin(rtc_data[4] & MONTHS_REG_MSK)) - 1;
+	tm->tm_year = (bcd2bin(rtc_data[5] & YEARS_REG_MSK)) + 100;
+	tm->tm_wday = bcd2bin(rtc_data[6] & WEEKS_REG_MSK);
+	dev_dbg(dev, "RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n",
+		1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
+		tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec);
+
+	return ret;
+}
+
+/* Set current time and date in RTC */
+static int rk808_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	u8 rtc_data[NUM_TIME_REGS];
+	int ret;
+
+	rtc_data[0] = bin2bcd(tm->tm_sec);
+	rtc_data[1] = bin2bcd(tm->tm_min);
+	rtc_data[2] = bin2bcd(tm->tm_hour);
+	rtc_data[3] = bin2bcd(tm->tm_mday);
+	rtc_data[4] = bin2bcd(tm->tm_mon + 1);
+	rtc_data[5] = bin2bcd(tm->tm_year - 100);
+	rtc_data[6] = bin2bcd(tm->tm_wday);
+	dev_dbg(dev, "set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n",
+		1900 + tm->tm_year, tm->tm_mon + 1, tm->tm_mday,
+		tm->tm_wday, tm->tm_hour , tm->tm_min, tm->tm_sec);
+
+	/* Stop RTC while updating the RTC registers */
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
+				 BIT_RTC_CTRL_REG_STOP_RTC_M,
+				 BIT_RTC_CTRL_REG_STOP_RTC_M);
+	if (ret) {
+		dev_err(dev, "Failed to update RTC control: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_bulk_write(rk808->regmap, RK808_SECONDS_REG,
+				rtc_data, NUM_TIME_REGS);
+	if (ret) {
+		dev_err(dev, "Failed to bull write rtc_data: %d\n", ret);
+		return ret;
+	}
+	/* Start RTC again */
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
+				 BIT_RTC_CTRL_REG_STOP_RTC_M, 0);
+	if (ret) {
+		dev_err(dev, "Failed to update RTC control: %d\n", ret);
+		return ret;
+	}
+	return 0;
+}
+
+/* Read alarm time and date in RTC */
+static int rk808_rtc_readalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	u8 alrm_data[NUM_ALARM_REGS];
+	uint32_t int_reg;
+	int ret;
+
+	ret = regmap_bulk_read(rk808->regmap, RK808_ALARM_SECONDS_REG,
+			       alrm_data, NUM_ALARM_REGS);
+
+	alrm->time.tm_sec = bcd2bin(alrm_data[0] & SECONDS_REG_MSK);
+	alrm->time.tm_min = bcd2bin(alrm_data[1] & MINUTES_REG_MAK);
+	alrm->time.tm_hour = bcd2bin(alrm_data[2] & HOURS_REG_MSK);
+	alrm->time.tm_mday = bcd2bin(alrm_data[3] & DAYS_REG_MSK);
+	alrm->time.tm_mon = (bcd2bin(alrm_data[4] & MONTHS_REG_MSK)) - 1;
+	alrm->time.tm_year = (bcd2bin(alrm_data[5] & YEARS_REG_MSK)) + 100;
+
+	ret = regmap_read(rk808->regmap, RK808_RTC_INT_REG, &int_reg);
+	if (ret) {
+		dev_err(dev, "Failed to read RTC INT REG: %d\n", ret);
+		return ret;
+	}
+
+	dev_dbg(dev, "alrm read RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n",
+		1900 + alrm->time.tm_year, alrm->time.tm_mon + 1,
+		alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour,
+		alrm->time.tm_min, alrm->time.tm_sec);
+
+	alrm->enabled = (int_reg & BIT_RTC_INTERRUPTS_REG_IT_ALARM_M) ? 1 : 0;
+
+	return 0;
+}
+
+static int rk808_rtc_stop_alarm(struct rk808_rtc *rk808_rtc)
+{
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	int ret;
+
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_INT_REG,
+				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M, 0);
+
+	return ret;
+}
+
+static int rk808_rtc_start_alarm(struct rk808_rtc *rk808_rtc)
+{
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	int ret;
+
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_INT_REG,
+				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M,
+				 BIT_RTC_INTERRUPTS_REG_IT_ALARM_M);
+
+	return ret;
+}
+
+static int rk808_rtc_setalarm(struct device *dev, struct rtc_wkalrm *alrm)
+{
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	u8 alrm_data[NUM_ALARM_REGS];
+	int ret;
+
+	ret = rk808_rtc_stop_alarm(rk808_rtc);
+	if (ret) {
+		dev_err(dev, "Failed to stop alarm: %d\n", ret);
+		return ret;
+	}
+	dev_dbg(dev, "alrm set RTC date/time %4d-%02d-%02d(%d) %02d:%02d:%02d\n",
+		1900 + alrm->time.tm_year, alrm->time.tm_mon + 1,
+		alrm->time.tm_mday, alrm->time.tm_wday, alrm->time.tm_hour,
+		alrm->time.tm_min, alrm->time.tm_sec);
+
+	alrm_data[0] = bin2bcd(alrm->time.tm_sec);
+	alrm_data[1] = bin2bcd(alrm->time.tm_min);
+	alrm_data[2] = bin2bcd(alrm->time.tm_hour);
+	alrm_data[3] = bin2bcd(alrm->time.tm_mday);
+	alrm_data[4] = bin2bcd(alrm->time.tm_mon + 1);
+	alrm_data[5] = bin2bcd(alrm->time.tm_year - 100);
+
+	ret = regmap_bulk_write(rk808->regmap, RK808_ALARM_SECONDS_REG,
+				alrm_data, NUM_ALARM_REGS);
+	if (ret) {
+		dev_err(dev, "Failed to bulk write: %d\n", ret);
+		return ret;
+	}
+	if (alrm->enabled) {
+		ret = rk808_rtc_start_alarm(rk808_rtc);
+		if (ret) {
+			dev_err(dev, "Failed to start alarm: %d\n", ret);
+			return ret;
+		}
+	}
+	return 0;
+}
+
+static int rk808_rtc_alarm_irq_enable(struct device *dev,
+				      unsigned int enabled)
+{
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(dev);
+
+	if (enabled)
+		return rk808_rtc_start_alarm(rk808_rtc);
+
+	return rk808_rtc_stop_alarm(rk808_rtc);
+}
+
+/*
+ * We will just handle setting the frequency and make use the framework for
+ * reading the periodic interupts.
+ *
+ * @freq: Current periodic IRQ freq:
+ * bit 0: every second
+ * bit 1: every minute
+ * bit 2: every hour
+ * bit 3: every day
+ */
+static irqreturn_t rk808_alarm_irq(int irq, void *data)
+{
+	struct rk808_rtc *rk808_rtc = data;
+	struct rk808 *rk808 = rk808_rtc->rk808;
+	struct i2c_client *client = rk808->i2c;
+	int ret;
+
+	ret = regmap_write(rk808->regmap, RK808_RTC_STATUS_REG,
+			   RTC_STATUS_MASK);
+	if (ret) {
+		dev_err(&client->dev,
+			"%s:Failed to update RTC status: %d\n", __func__, ret);
+		return ret;
+	}
+
+	rtc_update_irq(rk808_rtc->rtc, 1, RTC_IRQF | RTC_AF);
+	dev_dbg(&client->dev,
+		 "%s:irq=%d\n", __func__, irq);
+	return IRQ_HANDLED;
+}
+
+static const struct rtc_class_ops rk808_rtc_ops = {
+	.read_time = rk808_rtc_readtime,
+	.set_time = rk808_rtc_set_time,
+	.read_alarm = rk808_rtc_readalarm,
+	.set_alarm = rk808_rtc_setalarm,
+	.alarm_irq_enable = rk808_rtc_alarm_irq_enable,
+};
+
+#ifdef CONFIG_PM_SLEEP
+/* Turn off the alarm if it should not be a wake source. */
+static int rk808_rtc_suspend(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(rk808_rtc->irq);
+
+	return 0;
+}
+
+/* Enable the alarm if it should be enabled (in case it was disabled to
+ * prevent use as a wake source).
+ */
+static int rk808_rtc_resume(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rk808_rtc *rk808_rtc = dev_get_drvdata(&pdev->dev);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(rk808_rtc->irq);
+
+	return 0;
+}
+#endif
+
+static SIMPLE_DEV_PM_OPS(rk808_rtc_pm_ops,
+	rk808_rtc_suspend, rk808_rtc_resume);
+
+/* 2014.1.1 12:00:00 Saturday */
+struct rtc_time tm_def = {
+			.tm_wday = 6,
+			.tm_year = 114,
+			.tm_mon = 0,
+			.tm_mday = 1,
+			.tm_hour = 12,
+			.tm_min = 0,
+			.tm_sec = 0,
+};
+
+static int rk808_rtc_probe(struct platform_device *pdev)
+{
+	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
+	struct rk808_rtc *rk808_rtc;
+	struct rtc_time tm;
+	int ret;
+
+	rk808_rtc = devm_kzalloc(&pdev->dev, sizeof(*rk808_rtc), GFP_KERNEL);
+	if (rk808_rtc == NULL)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, rk808_rtc);
+	rk808_rtc->rk808 = rk808;
+
+	/* start rtc running by default, and use shadowed timer. */
+	ret = regmap_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
+				 BIT_RTC_CTRL_REG_STOP_RTC_M |
+				 BIT_RTC_CTRL_REG_RTC_READSEL_M,
+				 BIT_RTC_CTRL_REG_RTC_READSEL_M);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to update RTC control: %d\n", ret);
+		return ret;
+	}
+
+	ret = regmap_write(rk808->regmap, RK808_RTC_STATUS_REG,
+			   RTC_STATUS_MASK);
+	if (ret) {
+		dev_err(&pdev->dev,
+			"Failed to write RTC status: %d\n", ret);
+			return ret;
+	}
+
+	/* set init time */
+	ret = rk808_rtc_readtime(&pdev->dev, &tm);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to read RTC time\n");
+		return ret;
+	}
+	ret = rtc_valid_tm(&tm);
+	if (ret) {
+		dev_warn(&pdev->dev, "invalid date/time and init time\n");
+		rk808_rtc_set_time(&pdev->dev, &tm_def);
+	}
+
+	device_init_wakeup(&pdev->dev, 1);
+
+	rk808_rtc->rtc = devm_rtc_device_register(&pdev->dev, "rk808-rtc",
+						  &rk808_rtc_ops, THIS_MODULE);
+	if (IS_ERR(rk808_rtc->rtc)) {
+		ret = PTR_ERR(rk808_rtc->rtc);
+		return ret;
+	}
+
+	rk808_rtc->irq = platform_get_irq(pdev, 0);
+	if (rk808_rtc->irq < 0) {
+		if (rk808_rtc->irq != -EPROBE_DEFER)
+			dev_err(&pdev->dev, "Wake up is not possible as irq = %d\n",
+				rk808_rtc->irq);
+		return rk808_rtc->irq;
+	}
+
+	/* request alarm irq of rk808 */
+	ret = devm_request_threaded_irq(&pdev->dev, rk808_rtc->irq, NULL,
+					rk808_alarm_irq, 0,
+					"RTC alarm", rk808_rtc);
+	if (ret) {
+		dev_err(&pdev->dev, "Failed to request alarm IRQ %d: %d\n",
+			rk808_rtc->irq, ret);
+	}
+
+	return ret;
+}
+
+static struct platform_driver rk808_rtc_driver = {
+	.probe = rk808_rtc_probe,
+	.driver = {
+		.name = "rk808-rtc",
+		.pm = &rk808_rtc_pm_ops,
+	},
+};
+
+module_platform_driver(rk808_rtc_driver);
+
+MODULE_DESCRIPTION("RTC driver for the rk808 series PMICs");
+MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
+MODULE_AUTHOR("Zhang Qing <zhangqing@rock-chips.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:rk808-rtc");
-- 
1.7.9.5

[PATCH v10 2/3] clk: RK808: Add clkout driver for RK808

[Chris Zhong]

Signed-off-by: Chris Zhong <zyw@rock-chips.com>

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Tested-by: Heiko Stuebner <heiko@sntech.de>
---

Changes in v10:
Adviced by Mike
- remove rk808_clkout1_is_prepared
Adviced by Dmitry Torokhov
- add rk808_clkout_remove

Changes in v9: None
Changes in v8:
Adviced by Mike
- rename "control" to "enable"
- define RK808_NR_OUTPUT 2

Changes in v7:
Adviced by doug
-fix coding style problems

Changes in v6:
Adviced by doug
- use correct argument call of_clk_add_provider in probe

Changes in v5:
Adviced by doug
- add some error checking in probe
- move "rockchip,rk808.h" into the patch about dt-bindings

Changes in v4:
Adviced by doug
- add "clock-output-names" propertiey
- add a header file "rockchip,rk808.h"

Changes in v3:
- fix compile err

Changes in v2:
Adviced by javier.martinez
- separated from rtc-rk808.c

 drivers/clk/Kconfig     |    9 +++
 drivers/clk/Makefile    |    1 +
 drivers/clk/clk-rk808.c |  170 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 180 insertions(+)
 create mode 100644 drivers/clk/clk-rk808.c

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index cfd3af7..84e0590 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -38,6 +38,15 @@ config COMMON_CLK_MAX77686
 	---help---
 	  This driver supports Maxim 77686 crystal oscillator clock. 
 
+config COMMON_CLK_RK808
+	tristate "Clock driver for RK808"
+	depends on MFD_RK808
+	---help---
+	  This driver supports RK808 crystal oscillator clock. These
+	  multi-function devices have two fixed-rate oscillators,
+	  clocked at 32KHz each. Clkout1 is always on, Clkout2 can off
+	  by control register.
+
 config COMMON_CLK_SI5351
 	tristate "Clock driver for SiLabs 5351A/B/C"
 	depends on I2C
diff --git a/drivers/clk/Makefile b/drivers/clk/Makefile
index f537a0b..99f53d5 100644
--- a/drivers/clk/Makefile
+++ b/drivers/clk/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_ARCH_NOMADIK)		+= clk-nomadik.o
 obj-$(CONFIG_ARCH_NSPIRE)		+= clk-nspire.o
 obj-$(CONFIG_COMMON_CLK_PALMAS)		+= clk-palmas.o
 obj-$(CONFIG_CLK_PPC_CORENET)		+= clk-ppc-corenet.o
+obj-$(CONFIG_COMMON_CLK_RK808)		+= clk-rk808.o
 obj-$(CONFIG_COMMON_CLK_S2MPS11)	+= clk-s2mps11.o
 obj-$(CONFIG_COMMON_CLK_SI5351)		+= clk-si5351.o
 obj-$(CONFIG_COMMON_CLK_SI570)		+= clk-si570.o
diff --git a/drivers/clk/clk-rk808.c b/drivers/clk/clk-rk808.c
new file mode 100644
index 0000000..83902b9
--- /dev/null
+++ b/drivers/clk/clk-rk808.c
@@ -0,0 +1,170 @@
+/*
+ * Clkout driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author:Chris Zhong <zyw@rock-chips.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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/clk-provider.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/mfd/rk808.h>
+#include <linux/i2c.h>
+
+#define RK808_NR_OUTPUT 2
+
+struct rk808_clkout {
+	struct rk808 *rk808;
+	struct clk_onecell_data clk_data;
+	struct clk_hw		clkout1_hw;
+	struct clk_hw		clkout2_hw;
+};
+
+static unsigned long rk808_clkout_recalc_rate(struct clk_hw *hw,
+					      unsigned long parent_rate)
+{
+	return 32768;
+}
+
+static int rk808_clkout2_enable(struct clk_hw *hw, bool enable)
+{
+	struct rk808_clkout *rk808_clkout = container_of(hw,
+							 struct rk808_clkout,
+							 clkout2_hw);
+	struct rk808 *rk808 = rk808_clkout->rk808;
+
+	return regmap_update_bits(rk808->regmap, RK808_CLK32OUT_REG,
+				  CLK32KOUT2_EN, enable ? CLK32KOUT2_EN : 0);
+}
+
+static int rk808_clkout2_prepare(struct clk_hw *hw)
+{
+	return rk808_clkout2_enable(hw, true);
+}
+
+static void rk808_clkout2_unprepare(struct clk_hw *hw)
+{
+	rk808_clkout2_enable(hw, false);
+}
+
+static int rk808_clkout2_is_prepared(struct clk_hw *hw)
+{
+	struct rk808_clkout *rk808_clkout = container_of(hw,
+							 struct rk808_clkout,
+							 clkout2_hw);
+	struct rk808 *rk808 = rk808_clkout->rk808;
+	uint32_t val;
+
+	int ret = regmap_read(rk808->regmap, RK808_CLK32OUT_REG, &val);
+
+	if (ret < 0)
+		return ret;
+
+	return (val & CLK32KOUT2_EN) ? 1 : 0;
+}
+
+static const struct clk_ops rk808_clkout1_ops = {
+	.recalc_rate = rk808_clkout_recalc_rate,
+};
+
+static const struct clk_ops rk808_clkout2_ops = {
+	.prepare = rk808_clkout2_prepare,
+	.unprepare = rk808_clkout2_unprepare,
+	.is_prepared = rk808_clkout2_is_prepared,
+	.recalc_rate = rk808_clkout_recalc_rate,
+};
+
+static int rk808_clkout_probe(struct platform_device *pdev)
+{
+	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
+	struct i2c_client *client = rk808->i2c;
+	struct device_node *node = client->dev.of_node;
+	struct clk_init_data init = {};
+	struct clk **clk_table;
+	struct rk808_clkout *rk808_clkout;
+
+	rk808_clkout = devm_kzalloc(&client->dev,
+				    sizeof(*rk808_clkout), GFP_KERNEL);
+	if (!rk808_clkout)
+		return -ENOMEM;
+
+	rk808_clkout->rk808 = rk808;
+
+	clk_table = devm_kcalloc(&client->dev, RK808_NR_OUTPUT,
+				 sizeof(struct clk *), GFP_KERNEL);
+	if (!clk_table)
+		return -ENOMEM;
+
+	init.flags = CLK_IS_ROOT;
+	init.parent_names = NULL;
+	init.num_parents = 0;
+	init.name = "rk808-clkout1";
+	init.ops = &rk808_clkout1_ops;
+	rk808_clkout->clkout1_hw.init = &init;
+
+	/* optional override of the clockname */
+	of_property_read_string_index(node, "clock-output-names",
+				      0, &init.name);
+
+	clk_table[0] = devm_clk_register(&client->dev,
+					 &rk808_clkout->clkout1_hw);
+	if (IS_ERR(clk_table[0]))
+		return PTR_ERR(clk_table[0]);
+
+	init.name = "rk808-clkout2";
+	init.ops = &rk808_clkout2_ops;
+	rk808_clkout->clkout2_hw.init = &init;
+
+	/* optional override of the clockname */
+	of_property_read_string_index(node, "clock-output-names",
+				      1, &init.name);
+
+	clk_table[1] = devm_clk_register(&client->dev,
+					 &rk808_clkout->clkout2_hw);
+	if (IS_ERR(clk_table[1]))
+		return PTR_ERR(clk_table[1]);
+
+	rk808_clkout->clk_data.clks = clk_table;
+	rk808_clkout->clk_data.clk_num = RK808_NR_OUTPUT;
+
+	return of_clk_add_provider(node, of_clk_src_onecell_get,
+				   &rk808_clkout->clk_data);
+}
+
+static int rk808_clkout_remove(struct platform_device *pdev)
+{
+	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
+	struct i2c_client *client = rk808->i2c;
+	struct device_node *node = client->dev.of_node;
+
+	of_clk_del_provider(node);
+
+	return 0;
+}
+
+static struct platform_driver rk808_clkout_driver = {
+	.probe = rk808_clkout_probe,
+	.remove = rk808_clkout_remove,
+	.driver		= {
+		.name	= "rk808-clkout",
+	},
+};
+
+module_platform_driver(rk808_clkout_driver);
+
+MODULE_DESCRIPTION("Clkout driver for the rk808 series PMICs");
+MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:rk808-clkout");
-- 
1.7.9.5

[PATCH v10 3/3] regulator: RK808: Remove pdata from the regulator

[Chris Zhong]

Signed-off-by: Chris Zhong <zyw@rock-chips.com>

Tested-by: Heiko Stuebner <heiko@sntech.de>
---

Changes in v10: None
Changes in v9:
Adviced by Doug
- add reg_np

Changes in v8:
Adviced by Doug
- remove rk808_regulator_dts() function
- remove the check about "client->dev.of_node"

Changes in v7:
- remove pdata struct from header file, add rk808_regulator struct

Changes in v6:
- remove the redundant code

Changes in v5:
- re-edit base on Mark's branch

Changes in v4:
- use &client->dev replace rk808->dev

Changes in v3: None
Changes in v2:
Adviced by Mark Browm:
- change of_find_node_by_name to find_child_by_name
- use RK808_NUM_REGULATORS as the name of the constant
- create a pdata when missing platform data
- use the rk808_reg name to supply_regulator name
- replace regulator_register with devm_regulator_register
- some other problem with coding style

 drivers/regulator/rk808-regulator.c |   89 +++++++----------------------------
 1 file changed, 17 insertions(+), 72 deletions(-)

diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index e372008..d91f2b6 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -14,24 +14,16 @@
  * 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/module.h>
-#include <linux/init.h>
 #include <linux/i2c.h>
-#include <linux/err.h>
-#include <linux/platform_device.h>
 #include <linux/mfd/rk808.h>
-#include <linux/of.h>
 #include <linux/of_device.h>
 #include <linux/regulator/driver.h>
 #include <linux/regulator/of_regulator.h>
-#include <linux/regmap.h>
-#include <linux/slab.h>
-/*
- * Field Definitions.
- */
+
+/* Field Definitions */
 #define RK808_BUCK_VSEL_MASK	0x3f
 #define RK808_BUCK4_VSEL_MASK	0xf
 #define RK808_LDO_VSEL_MASK	0x1f
@@ -50,10 +42,6 @@ static const int buck_contr_base_addr[] = {
 	RK808_BUCK4_CONFIG_REG,
 };
 
-#define rk808_BUCK_SET_VOL_REG(x) (buck_set_vol_base_addr[x])
-#define rk808_BUCK_CONTR_REG(x) (buck_contr_base_addr[x])
-#define rk808_LDO_SET_VOL_REG(x) (ldo_set_vol_base_addr[x])
-
 static const int ldo_set_vol_base_addr[] = {
 	RK808_LDO1_ON_VSEL_REG,
 	RK808_LDO2_ON_VSEL_REG,
@@ -65,9 +53,7 @@ static const int ldo_set_vol_base_addr[] = {
 	RK808_LDO8_ON_VSEL_REG,
 };
 
-/*
- * rk808 voltage number
- */
+/* rk808 voltage number */
 static const struct regulator_linear_range rk808_buck_voltage_ranges[] = {
 	REGULATOR_LINEAR_RANGE(700000, 0, 63, 12500),
 };
@@ -308,77 +294,35 @@ static struct of_regulator_match rk808_reg_matches[] = {
 	[RK808_ID_SWITCH2]	= { .name = "SWITCH_REG2" },
 };
 
-static int rk808_regulator_dts(struct i2c_client *client,
-			       struct rk808_board *pdata)
-{
-	struct device_node *np, *reg_np;
-	int i, ret;
-
-	np = client->dev.of_node;
-	if (!np) {
-		dev_err(&client->dev, "could not find pmic sub-node\n");
-		return -ENXIO;
-	}
-
-	reg_np = of_get_child_by_name(np, "regulators");
-	if (!reg_np)
-		return -ENXIO;
-
-	ret = of_regulator_match(&client->dev, reg_np, rk808_reg_matches,
-				 RK808_NUM_REGULATORS);
-	if (ret < 0) {
-		dev_err(&client->dev,
-			"failed to parse regulator data: %d\n", ret);
-		return ret;
-	}
-
-	for (i = 0; i < RK808_NUM_REGULATORS; i++) {
-		if (!rk808_reg_matches[i].init_data ||
-		    !rk808_reg_matches[i].of_node)
-			continue;
-
-		pdata->rk808_init_data[i] = rk808_reg_matches[i].init_data;
-		pdata->of_node[i] = rk808_reg_matches[i].of_node;
-	}
-
-	return 0;
-}
-
 static int rk808_regulator_probe(struct platform_device *pdev)
 {
 	struct rk808 *rk808 = dev_get_drvdata(pdev->dev.parent);
 	struct i2c_client *client = rk808->i2c;
-	struct rk808_board *pdata = dev_get_platdata(&client->dev);
+	struct device_node *reg_np;
 	struct regulator_config config = {};
 	struct regulator_dev *rk808_rdev;
-	struct regulator_init_data *reg_data;
-	int i = 0;
-	int ret = 0;
+	int ret, i;
 
-	if (!pdata) {
-		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
-		if (!pdata)
-			return -ENOMEM;
-	}
+	reg_np = of_get_child_by_name(client->dev.of_node, "regulators");
+	if (!reg_np)
+		return -ENXIO;
 
-	ret = rk808_regulator_dts(client, pdata);
-	if (ret)
+	ret = of_regulator_match(&client->dev, reg_np, rk808_reg_matches,
+				 RK808_NUM_REGULATORS);
+	if (ret < 0)
 		return ret;
 
 	/* Instantiate the regulators */
 	for (i = 0; i < RK808_NUM_REGULATORS; i++) {
-		reg_data = pdata->rk808_init_data[i];
-		if (!reg_data)
+		if (!rk808_reg_matches[i].init_data ||
+		    !rk808_reg_matches[i].of_node)
 			continue;
 
 		config.dev = &client->dev;
 		config.driver_data = rk808;
 		config.regmap = rk808->regmap;
-
-		if (client->dev.of_node)
-			config.of_node = pdata->of_node[i];
-
-		config.init_data = reg_data;
+		config.of_node = rk808_reg_matches[i].of_node;
+		config.init_data = rk808_reg_matches[i].init_data;
 
 		rk808_rdev = devm_regulator_register(&pdev->dev,
 						     &rk808_reg[i], &config);
@@ -388,6 +332,7 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 			return PTR_ERR(rk808_rdev);
 		}
 	}
+
 	return 0;
 }
 
@@ -403,6 +348,6 @@ module_platform_driver(rk808_regulator_driver);
 
 MODULE_DESCRIPTION("regulator driver for the rk808 series PMICs");
 MODULE_AUTHOR("Chris Zhong<zyw@rock-chips.com>");
-MODULE_AUTHOR("Zhang Qing<zhanqging@rock-chips.com>");
+MODULE_AUTHOR("Zhang Qing<zhangqing@rock-chips.com>");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:rk808-regulator");
-- 
1.7.9.5

Re: [PATCH v10 3/3] regulator: RK808: Remove pdata from the regulator

[Doug Anderson]

Hi,

On Tue, Sep 9, 2014 at 6:18 PM, Chris Zhong <zyw@rock-chips.com> wrote:
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> ---
>
> Changes in v10: None

Despite the "None" above, I see that there are indeed changes between
v9 and v10:

diff --git a/drivers/regulator/rk808-regulator.c
b/drivers/regulator/rk808-regulator.c
index 0d5afc4..d91f2b6 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -3,8 +3,8 @@
  *
  * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
  *
- * Author: Chris Zhong<zyw@rock-chips.com>
- * Author: Zhang Qing<zhanqging@rock-chips.com>
+ * Author: Chris Zhong <zyw@rock-chips.com>
+ * Author: Zhang Qing <zhangqing@rock-chips.com>
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -348,6 +348,6 @@ module_platform_driver(rk808_regulator_driver);

 MODULE_DESCRIPTION("regulator driver for the rk808 series PMICs");
 MODULE_AUTHOR("Chris Zhong<zyw@rock-chips.com>");
-MODULE_AUTHOR("Zhang Qing<zhanqging@rock-chips.com>");
+MODULE_AUTHOR("Zhang Qing<zhangqing@rock-chips.com>");
 MODULE_LICENSE("GPL");
 MODULE_ALIAS("platform:rk808-regulator");

That looks like the change requested by Dmitry for v9 (don't remove
the space) plus a typo fix (flipping g/q).  Interestingly enough the
typo fix shows that "MODULE_AUTHOR" should probably also have had a
space before the "<".


Mark: I think you could just take this (possibly fixing the
MODULE_AUTHOR yourself) to fix the linux-next breakage.  You can also
add:

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>

-Doug

Re: [PATCH v10 3/3] regulator: RK808: Remove pdata from the regulator

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 331 bytes --]

On Wed, Sep 10, 2014 at 09:18:06AM +0800, Chris Zhong wrote:
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> 
> Tested-by: Heiko Stuebner <heiko@sntech.de>

Applied, thanks.  You shouldn't have spaces like the above in your list
of tags and as Doug says e-mail addresses should have a space between
the name and the <.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

[Andrew Morton]

On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@rock-chips.com> wrote:

> Adding RTC driver for supporting RTC device present inside RK808 PMIC.
> 
> ...
>
> +	ret = rtc_valid_tm(&tm);
> +	if (ret) {
> +		dev_warn(&pdev->dev, "invalid date/time and init time\n");
> +		rk808_rtc_set_time(&pdev->dev, &tm_def);
> +	}

This is somewhat unusual.  Most drivers will emit a warning and give up
when they find the time is wrong.  Why is this driver different and is
this desirable behaviour?


Also, I did this:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: rtc-rk808-add-rtc-driver-for-rk808-fix

make tm_def static

Cc: Chris Zhong <zyw@rock-chips.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/rtc/rtc-rk808.c |   16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff -puN drivers/rtc/Kconfig~rtc-rk808-add-rtc-driver-for-rk808-fix drivers/rtc/Kconfig
diff -puN drivers/rtc/Makefile~rtc-rk808-add-rtc-driver-for-rk808-fix drivers/rtc/Makefile
diff -puN drivers/rtc/rtc-rk808.c~rtc-rk808-add-rtc-driver-for-rk808-fix drivers/rtc/rtc-rk808.c
--- a/drivers/rtc/rtc-rk808.c~rtc-rk808-add-rtc-driver-for-rk808-fix
+++ a/drivers/rtc/rtc-rk808.c
@@ -326,14 +326,14 @@ static SIMPLE_DEV_PM_OPS(rk808_rtc_pm_op
 	rk808_rtc_suspend, rk808_rtc_resume);
 
 /* 2014.1.1 12:00:00 Saturday */
-struct rtc_time tm_def = {
-			.tm_wday = 6,
-			.tm_year = 114,
-			.tm_mon = 0,
-			.tm_mday = 1,
-			.tm_hour = 12,
-			.tm_min = 0,
-			.tm_sec = 0,
+static struct rtc_time tm_def = {
+	.tm_wday = 6,
+	.tm_year = 114,
+	.tm_mon = 0,
+	.tm_mday = 1,
+	.tm_hour = 12,
+	.tm_min = 0,
+	.tm_sec = 0,
 };
 
 static int rk808_rtc_probe(struct platform_device *pdev)
_

Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

[Doug Anderson]

Andrew,

On Wed, Sep 10, 2014 at 1:44 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@rock-chips.com> wrote:
>
>> Adding RTC driver for supporting RTC device present inside RK808 PMIC.
>>
>> ...
>>
>> +     ret = rtc_valid_tm(&tm);
>> +     if (ret) {
>> +             dev_warn(&pdev->dev, "invalid date/time and init time\n");
>> +             rk808_rtc_set_time(&pdev->dev, &tm_def);
>> +     }
>
> This is somewhat unusual.  Most drivers will emit a warning and give up
> when they find the time is wrong.  Why is this driver different and is
> this desirable behaviour?

When you say "give up", what does that mean?  I assume the driver
should keep initting, right?  Then the user can go in and set a time
later...


I did test things with just removing this chunk of code.  You get some
yells at bootup if you put a bogus time in there:

[    2.987590] rk808-rtc rk808-rtc: invalid date/time and init time
[    3.013148] rk808-rtc rk808-rtc: rtc core: registered rk808-rtc as rtc0
[    4.586115] rk808-rtc rk808-rtc: hctosys: invalid date/time

...but if you later set a valid time then everything is fine.  That
seems reasonable behavior to me, so I guess we could just remove this
whole chunk?  It appears that after a normal bootup the date/time is
something valid.


-Doug

Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

[Andrew Morton]

On Wed, 10 Sep 2014 14:37:13 -0700 Doug Anderson <dianders@chromium.org> wrote:

> Andrew,
> 
> On Wed, Sep 10, 2014 at 1:44 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@rock-chips.com> wrote:
> >
> >> Adding RTC driver for supporting RTC device present inside RK808 PMIC.
> >>
> >> ...
> >>
> >> +     ret = rtc_valid_tm(&tm);
> >> +     if (ret) {
> >> +             dev_warn(&pdev->dev, "invalid date/time and init time\n");
> >> +             rk808_rtc_set_time(&pdev->dev, &tm_def);
> >> +     }
> >
> > This is somewhat unusual.  Most drivers will emit a warning and give up
> > when they find the time is wrong.  Why is this driver different and is
> > this desirable behaviour?
> 
> When you say "give up", what does that mean?  I assume the driver
> should keep initting, right?  Then the user can go in and set a time
> later...

I think I was misreading current drivers a bit.  rtc-cmos.c will go in
and set a dummy time but regular low-level drivers don't sanity-check
the time at all at setup time.

> 
> I did test things with just removing this chunk of code.  You get some
> yells at bootup if you put a bogus time in there:
> 
> [    2.987590] rk808-rtc rk808-rtc: invalid date/time and init time
> [    3.013148] rk808-rtc rk808-rtc: rtc core: registered rk808-rtc as rtc0
> [    4.586115] rk808-rtc rk808-rtc: hctosys: invalid date/time
> 
> ...but if you later set a valid time then everything is fine.  That
> seems reasonable behavior to me, so I guess we could just remove this
> whole chunk?  It appears that after a normal bootup the date/time is
> something valid.

hm.  Having an invalid time is perhaps better than having a valid but
incorrect time.

When one sees one driver doing something differently from the others
one has to wonder "why" and "which one is better".  If setting a dummy
time is better then all drivers should do it, and I expect this could
be done by rtc core at registration time.

Re: [PATCH v10 1/3] RTC: RK808: add RTC driver for RK808

[Doug Anderson]

Andrew,

On Wed, Sep 10, 2014 at 3:08 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Wed, 10 Sep 2014 14:37:13 -0700 Doug Anderson <dianders@chromium.org> wrote:
>
>> Andrew,
>>
>> On Wed, Sep 10, 2014 at 1:44 PM, Andrew Morton
>> <akpm@linux-foundation.org> wrote:
>> > On Wed, 10 Sep 2014 09:18:04 +0800 Chris Zhong <zyw@rock-chips.com> wrote:
>> >
>> >> Adding RTC driver for supporting RTC device present inside RK808 PMIC.
>> >>
>> >> ...
>> >>
>> >> +     ret = rtc_valid_tm(&tm);
>> >> +     if (ret) {
>> >> +             dev_warn(&pdev->dev, "invalid date/time and init time\n");
>> >> +             rk808_rtc_set_time(&pdev->dev, &tm_def);
>> >> +     }
>> >
>> > This is somewhat unusual.  Most drivers will emit a warning and give up
>> > when they find the time is wrong.  Why is this driver different and is
>> > this desirable behaviour?
>>
>> When you say "give up", what does that mean?  I assume the driver
>> should keep initting, right?  Then the user can go in and set a time
>> later...
>
> I think I was misreading current drivers a bit.  rtc-cmos.c will go in
> and set a dummy time but regular low-level drivers don't sanity-check
> the time at all at setup time.
>
>>
>> I did test things with just removing this chunk of code.  You get some
>> yells at bootup if you put a bogus time in there:
>>
>> [    2.987590] rk808-rtc rk808-rtc: invalid date/time and init time
>> [    3.013148] rk808-rtc rk808-rtc: rtc core: registered rk808-rtc as rtc0
>> [    4.586115] rk808-rtc rk808-rtc: hctosys: invalid date/time
>>
>> ...but if you later set a valid time then everything is fine.  That
>> seems reasonable behavior to me, so I guess we could just remove this
>> whole chunk?  It appears that after a normal bootup the date/time is
>> something valid.
>
> hm.  Having an invalid time is perhaps better than having a valid but
> incorrect time.

OK.  It's an easy patch to remove this.  Chris: do you want to test
that too and post it, or do you want me to?  Basically just remove:

  rk808_rtc_set_time(&pdev->dev, &tm_def);

...and of course then remove the tm_def structure and take braces off
the "if".  I think everything works if you do that.  We need to make
sure to base on the patch Andrew talked about about making the
structure static (the new patch would remove the structure completely,
but nice not to get a merge conflict).

-Doug