linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 0/5] Add rockchip RK808 pmic driver
@ 2014-09-01  9:07 Chris Zhong
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:07 UTC (permalink / raw)
  To: dianders, heiko
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette, devicetree,
	linux-kernel, rtc-linux, grant.likely, hl, huangtao, cf,
	zhangqing, xxx, olof, sonnyrao, dtor, javier.martinez,
	kever.yang, 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 v7:
Advices by Mark Rutland
- modify description about clock-cells
- update the example
Adviced by Lee Jones
- coding style
- remove rk808_pre_init function
Adviced by Doug
- add "&& OF" to the dependencies
- add .init_ack_masked = true in rk808_irq_chip
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:
Advices by Mark Rutland
- add description about clock-cells
Advices by Doug
- modify description about regulator
- remove pinctrl description
Adviced by Lee Jones in v2
- rk808_i2c_client instead of g_rk808
- remove pdata form struct rk808
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:
Advices by Mark Brown
- add description about regulator valid name.
- add a header file "rockchip,rk808".
- 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:
Advices by Doug
- add a "#clock-cells" propertiy
- update the example
Adviced by Lee Jones in v2
- modify the description in Kconfig
- remove some unnecessary header files
- remove dev from struct rk808
- use enum for define RK808_ID...
- 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
- fix compile err

Changes in v2:
Adviced by Mark Browm:
- use defines for register setting value
- remove rtc alarm disable in shutdown
- remove while(1) in shutdown
- remove read 0x2f in probe
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 (5):
  dt-bindings: Add RK808 device tree bindings document
  MFD: RK808: Add new mfd driver for RK808
  RTC: RK808: add RTC driver for RK808
  clk: RK808: Add clkout driver for RK808
  regulator: RK808: Remove pdata from the regulator

 Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++
 drivers/clk/Kconfig                             |    9 +
 drivers/clk/Makefile                            |    1 +
 drivers/clk/clk-rk808.c                         |  160 +++++++++
 drivers/mfd/Kconfig                             |   13 +
 drivers/mfd/Makefile                            |    1 +
 drivers/mfd/rk808.c                             |  243 +++++++++++++
 drivers/regulator/rk808-regulator.c             |   52 ++-
 drivers/rtc/Kconfig                             |   10 +
 drivers/rtc/Makefile                            |    1 +
 drivers/rtc/rtc-rk808.c                         |  419 +++++++++++++++++++++++
 include/dt-bindings/clock/rockchip,rk808.h      |   11 +
 include/linux/mfd/rk808.h                       |  196 +++++++++++
 13 files changed, 1250 insertions(+), 32 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
 create mode 100644 drivers/clk/clk-rk808.c
 create mode 100644 drivers/mfd/rk808.c
 create mode 100644 drivers/rtc/rtc-rk808.c
 create mode 100644 include/dt-bindings/clock/rockchip,rk808.h
 create mode 100644 include/linux/mfd/rk808.h

-- 
1.7.9.5


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

* [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
@ 2014-09-01  9:07 ` Chris Zhong
  2014-09-02  3:42   ` Doug Anderson
                     ` (2 more replies)
  2014-09-01  9:39 ` [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808 Chris Zhong
                   ` (4 subsequent siblings)
  5 siblings, 3 replies; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:07 UTC (permalink / raw)
  To: dianders, heiko
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette, devicetree,
	linux-kernel, rtc-linux, grant.likely, hl, huangtao, cf,
	zhangqing, xxx, olof, sonnyrao, dtor, javier.martinez,
	kever.yang, Chris Zhong

Add device tree bindings documentation and a header file
for rockchip's RK808 pmic.

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

---

Changes in v7:
Advices by Mark Rutland
- modify description about clock-cells
- update the example

Changes in v6:
Advices by Mark Rutland
- add description about clock-cells
Advices by Doug
- modify description about regulator
- remove pinctrl description

Changes in v5:
Advices by Mark Brown
- add description about regulator valid name.
- add a header file "rockchip,rk808".

Changes in v4:
Advices by Doug
- add a "#clock-cells" propertiy
- update the example

Changes in v3: None
Changes in v2: None

 Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++++++++++++++++
 include/dt-bindings/clock/rockchip,rk808.h      |   11 ++
 2 files changed, 177 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
 create mode 100644 include/dt-bindings/clock/rockchip,rk808.h

diff --git a/Documentation/devicetree/bindings/mfd/rk808.txt b/Documentation/devicetree/bindings/mfd/rk808.txt
new file mode 100644
index 0000000..f8932ac
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/rk808.txt
@@ -0,0 +1,166 @@
+RK808 Power Management Integrated Circuit
+
+Required properties:
+- compatible: "rockchip,rk808"
+- reg: I2C slave address
+- interrupt-parent: The parent interrupt controller.
+- interrupts: the interrupt outputs of the controller.
+- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
+  outputs)
+
+Optional properties:
+- clock-output-names: From common clock binding to override the
+  default output clock name
+- rockchip,system-power-controller: Telling whether or not this pmic is controlling
+  the system power.
+
+Regulators: All the regulators of RK808 to be instantiated shall be
+listed in a child node named 'regulators'. Each regulator is represented
+by a child node of the 'regulators' node.
+
+	regulator-name {
+		/* standard regulator bindings here */
+	};
+
+Following regulators of the RK808 PMIC block are supported. Note that
+the 'n' in regulator name, as in DCDC_REGn or LDOn, represents the DCDC or LDO
+number as described in RK808 datasheet.
+
+	- DCDC_REGn
+		- valid values for n are 1 to 4.
+	- LDO_REGn
+		- valid values for n are 1 to 8.
+	- SWITCH_REGn
+		- valid values for n are 1 to 2
+
+Standard regulator bindings are used inside regulator subnodes. Check
+  Documentation/devicetree/bindings/regulator/regulator.txt
+for more details
+
+Example:
+	rk808: pmic@1b {
+		compatible = "rockchip,rk808";
+		clock-output-names = "xin32k", "rk808-clkout2";
+		interrupt-parent = <&gpio0>;
+		interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
+		pinctrl-names = "default";
+		pinctrl-0 = <&pmic_int>;
+		reg = <0x1b>;
+		rockchip,system-power-controller;
+		wakeup-source;
+		#clock-cells = <1>;
+
+		vcc8-supply = <&vcc_18>;
+		vcc9-supply = <&vcc_io>;
+		vcc10-supply = <&vcc_io>;
+		vcc12-supply = <&vcc_io>;
+		vddio-supply = <&vccio_pmu>;
+
+		regulators {
+			vdd_cpu: DCDC_REG1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <750000>;
+				regulator-max-microvolt = <1300000>;
+				regulator-name = "vdd_arm";
+			};
+
+			vdd_gpu: DCDC_REG2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <850000>;
+				regulator-max-microvolt = <1250000>;
+				regulator-name = "vdd_gpu";
+			};
+
+			vcc_ddr: DCDC_REG3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vcc_ddr";
+			};
+
+			vcc_io: DCDC_REG4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc_io";
+			};
+
+			vccio_pmu: LDO_REG1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vccio_pmu";
+			};
+
+			vcc_tp: LDO_REG2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcc_tp";
+			};
+
+			vdd_10: LDO_REG3 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-name = "vdd_10";
+			};
+
+			vcc18_lcd: LDO_REG4 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc18_lcd";
+			};
+
+			vccio_sd: LDO_REG5 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vccio_sd";
+			};
+
+			vdd10_lcd: LDO_REG6 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1000000>;
+				regulator-max-microvolt = <1000000>;
+				regulator-name = "vdd10_lcd";
+			};
+
+			vcc_18: LDO_REG7 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <1800000>;
+				regulator-max-microvolt = <1800000>;
+				regulator-name = "vcc_18";
+			};
+
+			vcca_codec: LDO_REG8 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-min-microvolt = <3300000>;
+				regulator-max-microvolt = <3300000>;
+				regulator-name = "vcca_codec";
+			};
+
+			vcc_wl: SWITCH_REG1 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vcc_wl";
+			};
+
+			vcc_lcd: SWITCH_REG2 {
+				regulator-always-on;
+				regulator-boot-on;
+				regulator-name = "vcc_lcd";
+			};
+		};
+	};
diff --git a/include/dt-bindings/clock/rockchip,rk808.h b/include/dt-bindings/clock/rockchip,rk808.h
new file mode 100644
index 0000000..1a87343
--- /dev/null
+++ b/include/dt-bindings/clock/rockchip,rk808.h
@@ -0,0 +1,11 @@
+/*
+ * This header provides constants clk index RK808 pmic clkout
+ */
+#ifndef _CLK_ROCKCHIP_RK808
+#define _CLK_ROCKCHIP_RK808
+
+/* CLOCKOUT index */
+#define RK808_CLKOUT0		0
+#define RK808_CLKOUT1		1
+
+#endif
-- 
1.7.9.5


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

* [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
@ 2014-09-01  9:39 ` Chris Zhong
  2014-09-01 10:09   ` Lee Jones
  2014-09-01  9:43 ` [PATCH v7 3/5] RTC: RK808: add RTC " Chris Zhong
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:39 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette
  Cc: devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, dianders, heiko, olof, sonnyrao, dtor,
	javier.martinez, kever.yang, Chris Zhong

The RK808 chip is a power management IC for multimedia and handheld
devices. It contains the following components:

- Regulators
- RTC
- Clkout

The RK808 core driver is registered as a platform driver and provides
communication through I2C with the host device for the different
components.

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

---

Changes in v7:
Adviced by Lee Jones
- coding style
- remove rk808_pre_init function
Adviced by Doug
- add "&& OF" to the dependencies
- add .init_ack_masked = true in rk808_irq_chip

Changes in v6:
Adviced by Lee Jones in v2
- rk808_i2c_client instead of g_rk808
- remove pdata form struct rk808

Changes in v5: None
Changes in v4:
Adviced by Lee Jones in v2
- modify the description in Kconfig
- remove some unnecessary header files
- remove dev from struct rk808
- use enum for define RK808_ID...

Changes in v3:
- fix compile err

Changes in v2:
Adviced by Mark Browm:
- use defines for register setting value
- remove rtc alarm disable in shutdown
- remove while(1) in shutdown
- remove read 0x2f in probe

 drivers/mfd/Kconfig       |   13 +++
 drivers/mfd/Makefile      |    1 +
 drivers/mfd/rk808.c       |  243 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/rk808.h |  196 ++++++++++++++++++++++++++++++++++++
 4 files changed, 453 insertions(+)
 create mode 100644 drivers/mfd/rk808.c
 create mode 100644 include/linux/mfd/rk808.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de5abf2..4097793 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -582,6 +582,19 @@ config MFD_RC5T583
 	  Additional drivers must be enabled in order to use the
 	  different functionality of the device.
 
+config MFD_RK808
+	tristate "Rockchip RK808 Power Management chip"
+	depends on I2C && OF
+	select MFD_CORE
+	select REGMAP_I2C
+	select REGMAP_IRQ
+	help
+	  If you say yes here you get support for the RK808
+	  Power Management chips.
+	  This driver provides common support for accessing the device
+	  through I2C interface. The device supports multiple sub-devices
+	  including interrupts, RTC, LDO & DCDC regulators, and onkey.
+
 config MFD_SEC_CORE
 	bool "SAMSUNG Electronics PMIC Series Support"
 	depends on I2C=y
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f001487..dbc28e7 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -160,6 +160,7 @@ obj-$(CONFIG_MFD_INTEL_MSIC)	+= intel_msic.o
 obj-$(CONFIG_MFD_PALMAS)	+= palmas.o
 obj-$(CONFIG_MFD_VIPERBOARD)    += viperboard.o
 obj-$(CONFIG_MFD_RC5T583)	+= rc5t583.o rc5t583-irq.o
+obj-$(CONFIG_MFD_RK808)		+= rk808.o
 obj-$(CONFIG_MFD_SEC_CORE)	+= sec-core.o sec-irq.o
 obj-$(CONFIG_MFD_SYSCON)	+= syscon.o
 obj-$(CONFIG_MFD_LM3533)	+= lm3533-core.o lm3533-ctrlbank.o
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
new file mode 100644
index 0000000..869b144
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,243 @@
+/*
+ * MFD core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * 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/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/rk808.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/regmap.h>
+
+struct rk808_reg_data {
+	int addr;
+	int mask;
+	int value;
+};
+
+static const struct regmap_config rk808_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.max_register = RK808_IO_POL_REG,
+};
+
+static struct resource rtc_resources[] = {
+	{
+		.start  = RK808_IRQ_RTC_ALARM,
+		.end    = RK808_IRQ_RTC_ALARM,
+		.flags  = IORESOURCE_IRQ,
+	}
+};
+
+static const struct mfd_cell rk808s[] = {
+	{ .name = "rk808-clkout", },
+	{ .name = "rk808-regulator", },
+	{
+		.name = "rk808-rtc",
+		.num_resources = ARRAY_SIZE(rtc_resources),
+		.resources = &rtc_resources[0],
+	},
+};
+
+static const struct rk808_reg_data pre_init_reg[] = {
+	{ RK808_BUCK3_CONFIG_REG, BUCK_ILMIN_MASK,  BUCK_ILMIN_150MA },
+	{ RK808_BUCK4_CONFIG_REG, BUCK_ILMIN_MASK,  BUCK_ILMIN_200MA },
+	{ RK808_BOOST_CONFIG_REG, BOOST_ILMIN_MASK, BOOST_ILMIN_100MA },
+	{ RK808_BUCK1_CONFIG_REG, BUCK1_RATE_MASK,  BUCK_ILMIN_200MA },
+	{ RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK,  BUCK_ILMIN_200MA },
+	{ RK808_VB_MON_REG,       MASK_ALL,         VB_LO_ACT |
+						    VB_LO_SEL_3500MV },
+};
+
+static const struct regmap_irq rk808_irqs[] = {
+	/* INT_STS */
+	[RK808_IRQ_VOUT_LO] = {
+		.mask = RK808_IRQ_VOUT_LO_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_VB_LO] = {
+		.mask = RK808_IRQ_VB_LO_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_PWRON] = {
+		.mask = RK808_IRQ_PWRON_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_PWRON_LP] = {
+		.mask = RK808_IRQ_PWRON_LP_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_HOTDIE] = {
+		.mask = RK808_IRQ_HOTDIE_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_RTC_ALARM] = {
+		.mask = RK808_IRQ_RTC_ALARM_MSK,
+		.reg_offset = 0,
+	},
+	[RK808_IRQ_RTC_PERIOD] = {
+		.mask = RK808_IRQ_RTC_PERIOD_MSK,
+		.reg_offset = 0,
+	},
+
+	/* INT_STS2 */
+	[RK808_IRQ_PLUG_IN_INT] = {
+		.mask = RK808_IRQ_PLUG_IN_INT_MSK,
+		.reg_offset = 1,
+	},
+	[RK808_IRQ_PLUG_OUT_INT] = {
+		.mask = RK808_IRQ_PLUG_OUT_INT_MSK,
+		.reg_offset = 1,
+	},
+};
+
+static struct regmap_irq_chip rk808_irq_chip = {
+	.name = "rk808",
+	.irqs = rk808_irqs,
+	.num_irqs = ARRAY_SIZE(rk808_irqs),
+	.num_regs = 2,
+	.irq_reg_stride = 2,
+	.status_base = RK808_INT_STS_REG1,
+	.mask_base = RK808_INT_STS_MSK_REG1,
+	.ack_base = RK808_INT_STS_REG1,
+	.init_ack_masked = true,
+};
+
+static struct i2c_client *rk808_i2c_client;
+static void rk808_device_shutdown(void)
+{
+	int ret;
+	struct rk808 *rk808 = i2c_get_clientdata(rk808_i2c_client);
+
+	if (!rk808) {
+		dev_warn(&rk808_i2c_client->dev,
+			 "have no rk808, so do nothing here\n");
+		return;
+	}
+
+	ret = regmap_update_bits(rk808->regmap,
+				 RK808_DEVCTRL_REG,
+				 DEV_OFF_RST, DEV_OFF_RST);
+	if (ret)
+		dev_err(&rk808_i2c_client->dev, "power off error!\n");
+}
+
+static int rk808_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	int i;
+	int ret;
+	int pm_off = 0;
+	struct rk808 *rk808;
+	struct device_node *np = client->dev.of_node;
+
+	if (!client->irq) {
+		dev_err(&client->dev, "No interrupt support, no core IRQ\n");
+		return -EINVAL;
+	}
+
+	rk808 = devm_kzalloc(&client->dev, sizeof(*rk808), GFP_KERNEL);
+	if (!rk808)
+		return -ENOMEM;
+
+	rk808->regmap = devm_regmap_init_i2c(client, &rk808_regmap_config);
+	if (IS_ERR(rk808->regmap)) {
+		dev_err(&client->dev, "regmap initialization failed\n");
+		return PTR_ERR(rk808->regmap);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(pre_init_reg); i++) {
+		ret = regmap_update_bits(rk808->regmap, pre_init_reg[i].addr,
+					 pre_init_reg[i].mask,
+					 pre_init_reg[i].value);
+		if (ret) {
+			dev_err(&client->dev,
+				"0x%x write err\n", pre_init_reg[i].addr);
+			return ret;
+		}
+	}
+
+	ret = regmap_add_irq_chip(rk808->regmap, client->irq,
+				  IRQF_ONESHOT, -1,
+				  &rk808_irq_chip, &rk808->irq_data);
+	if (ret) {
+		dev_err(&client->dev, "Failed to add irq_chip %d\n", ret);
+		return ret;
+	}
+
+	rk808->i2c = client;
+	i2c_set_clientdata(client, rk808);
+	ret = mfd_add_devices(&client->dev, -1,
+			      rk808s, ARRAY_SIZE(rk808s),
+			      NULL, 0, regmap_irq_get_domain(rk808->irq_data));
+	if (ret) {
+		dev_err(&client->dev, "failed to add MFD devices %d\n", ret);
+		goto err_irq;
+	}
+
+	if (np) {
+		pm_off = of_property_read_bool(np,
+					"rockchip,system-power-controller");
+		if (pm_off && !pm_power_off) {
+			rk808_i2c_client = client;
+			pm_power_off = rk808_device_shutdown;
+		}
+	}
+
+	return 0;
+
+err_irq:
+	regmap_del_irq_chip(client->irq, rk808->irq_data);
+	return ret;
+}
+
+static int rk808_remove(struct i2c_client *client)
+{
+	struct rk808 *rk808 = i2c_get_clientdata(client);
+
+	regmap_del_irq_chip(client->irq, rk808->irq_data);
+	mfd_remove_devices(&client->dev);
+	pm_power_off = NULL;
+
+	return 0;
+}
+
+static struct of_device_id rk808_of_match[] = {
+	{ .compatible = "rockchip,rk808" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, rk808_of_match);
+
+static const struct i2c_device_id rk808_ids[] = {
+	{ "rk808" },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, rk808_ids);
+
+static struct i2c_driver rk808_i2c_driver = {
+	.driver = {
+		.name = "rk808",
+		.of_match_table = of_match_ptr(rk808_of_match),
+	},
+	.probe    = rk808_probe,
+	.remove   = rk808_remove,
+	.id_table = rk808_ids,
+};
+
+module_i2c_driver(rk808_i2c_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
+MODULE_AUTHOR("Zhang Qing <zhangqing@rock-chips.com>");
+MODULE_DESCRIPTION("RK808 PMIC driver");
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
new file mode 100644
index 0000000..fb09312
--- /dev/null
+++ b/include/linux/mfd/rk808.h
@@ -0,0 +1,196 @@
+/*
+ * rk808.h 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.
+ */
+
+#ifndef __LINUX_REGULATOR_rk808_H
+#define __LINUX_REGULATOR_rk808_H
+
+#include <linux/regulator/machine.h>
+#include <linux/regmap.h>
+
+/*
+ * rk808 Global Register Map.
+ */
+
+#define RK808_DCDC1	0 /* (0+RK808_START) */
+#define RK808_LDO1	4 /* (4+RK808_START) */
+#define RK808_NUM_REGULATORS   14
+
+enum rk808_reg {
+	RK808_ID_DCDC1,
+	RK808_ID_DCDC2,
+	RK808_ID_DCDC3,
+	RK808_ID_DCDC4,
+	RK808_ID_LDO1,
+	RK808_ID_LDO2,
+	RK808_ID_LDO3,
+	RK808_ID_LDO4,
+	RK808_ID_LDO5,
+	RK808_ID_LDO6,
+	RK808_ID_LDO7,
+	RK808_ID_LDO8,
+	RK808_ID_SWITCH1,
+	RK808_ID_SWITCH2,
+};
+
+#define RK808_SECONDS_REG	0x00
+#define RK808_MINUTES_REG	0x01
+#define RK808_HOURS_REG		0x02
+#define RK808_DAYS_REG		0x03
+#define RK808_MONTHS_REG	0x04
+#define RK808_YEARS_REG		0x05
+#define RK808_WEEKS_REG		0x06
+#define RK808_ALARM_SECONDS_REG	0x08
+#define RK808_ALARM_MINUTES_REG	0x09
+#define RK808_ALARM_HOURS_REG	0x0a
+#define RK808_ALARM_DAYS_REG	0x0b
+#define RK808_ALARM_MONTHS_REG	0x0c
+#define RK808_ALARM_YEARS_REG	0x0d
+#define RK808_RTC_CTRL_REG	0x10
+#define RK808_RTC_STATUS_REG	0x11
+#define RK808_RTC_INT_REG	0x12
+#define RK808_RTC_COMP_LSB_REG	0x13
+#define RK808_RTC_COMP_MSB_REG	0x14
+#define RK808_CLK32OUT_REG	0x20
+#define RK808_VB_MON_REG	0x21
+#define RK808_THERMAL_REG	0x22
+#define RK808_DCDC_EN_REG	0x23
+#define RK808_LDO_EN_REG	0x24
+#define RK808_SLEEP_SET_OFF_REG1	0x25
+#define RK808_SLEEP_SET_OFF_REG2	0x26
+#define RK808_DCDC_UV_STS_REG	0x27
+#define RK808_DCDC_UV_ACT_REG	0x28
+#define RK808_LDO_UV_STS_REG	0x29
+#define RK808_LDO_UV_ACT_REG	0x2a
+#define RK808_DCDC_PG_REG	0x2b
+#define RK808_LDO_PG_REG	0x2c
+#define RK808_VOUT_MON_TDB_REG	0x2d
+#define RK808_BUCK1_CONFIG_REG		0x2e
+#define RK808_BUCK1_ON_VSEL_REG		0x2f
+#define RK808_BUCK1_SLP_VSEL_REG	0x30
+#define RK808_BUCK1_DVS_VSEL_REG	0x31
+#define RK808_BUCK2_CONFIG_REG		0x32
+#define RK808_BUCK2_ON_VSEL_REG		0x33
+#define RK808_BUCK2_SLP_VSEL_REG	0x34
+#define RK808_BUCK2_DVS_VSEL_REG	0x35
+#define RK808_BUCK3_CONFIG_REG		0x36
+#define RK808_BUCK4_CONFIG_REG		0x37
+#define RK808_BUCK4_ON_VSEL_REG		0x38
+#define RK808_BUCK4_SLP_VSEL_REG	0x39
+#define RK808_BOOST_CONFIG_REG		0x3a
+#define RK808_LDO1_ON_VSEL_REG		0x3b
+#define RK808_LDO1_SLP_VSEL_REG		0x3c
+#define RK808_LDO2_ON_VSEL_REG		0x3d
+#define RK808_LDO2_SLP_VSEL_REG		0x3e
+#define RK808_LDO3_ON_VSEL_REG		0x3f
+#define RK808_LDO3_SLP_VSEL_REG		0x40
+#define RK808_LDO4_ON_VSEL_REG		0x41
+#define RK808_LDO4_SLP_VSEL_REG		0x42
+#define RK808_LDO5_ON_VSEL_REG		0x43
+#define RK808_LDO5_SLP_VSEL_REG		0x44
+#define RK808_LDO6_ON_VSEL_REG		0x45
+#define RK808_LDO6_SLP_VSEL_REG		0x46
+#define RK808_LDO7_ON_VSEL_REG		0x47
+#define RK808_LDO7_SLP_VSEL_REG		0x48
+#define RK808_LDO8_ON_VSEL_REG		0x49
+#define RK808_LDO8_SLP_VSEL_REG		0x4a
+#define RK808_DEVCTRL_REG	0x4b
+#define RK808_INT_STS_REG1	0x4c
+#define RK808_INT_STS_MSK_REG1	0x4d
+#define RK808_INT_STS_REG2	0x4e
+#define RK808_INT_STS_MSK_REG2	0x4f
+#define RK808_IO_POL_REG	0x50
+
+/* IRQ Definitions */
+#define RK808_IRQ_VOUT_LO	0
+#define RK808_IRQ_VB_LO		1
+#define RK808_IRQ_PWRON		2
+#define RK808_IRQ_PWRON_LP	3
+#define RK808_IRQ_HOTDIE	4
+#define RK808_IRQ_RTC_ALARM	5
+#define RK808_IRQ_RTC_PERIOD	6
+#define RK808_IRQ_PLUG_IN_INT	7
+#define RK808_IRQ_PLUG_OUT_INT	8
+#define RK808_NUM_IRQ		9
+
+#define RK808_IRQ_VOUT_LO_MSK		BIT(0)
+#define RK808_IRQ_VB_LO_MSK		BIT(1)
+#define RK808_IRQ_PWRON_MSK		BIT(2)
+#define RK808_IRQ_PWRON_LP_MSK		BIT(3)
+#define RK808_IRQ_HOTDIE_MSK		BIT(4)
+#define RK808_IRQ_RTC_ALARM_MSK		BIT(5)
+#define RK808_IRQ_RTC_PERIOD_MSK	BIT(6)
+#define RK808_IRQ_PLUG_IN_INT_MSK	BIT(0)
+#define RK808_IRQ_PLUG_OUT_INT_MSK	BIT(1)
+
+#define RK808_VBAT_LOW_2V8	0x00
+#define RK808_VBAT_LOW_2V9	0x01
+#define RK808_VBAT_LOW_3V0	0x02
+#define RK808_VBAT_LOW_3V1	0x03
+#define RK808_VBAT_LOW_3V2	0x04
+#define RK808_VBAT_LOW_3V3	0x05
+#define RK808_VBAT_LOW_3V4	0x06
+#define RK808_VBAT_LOW_3V5	0x07
+#define VBAT_LOW_VOL_MASK	(0x07 << 0)
+#define EN_VABT_LOW_SHUT_DOWN	(0x00 << 4)
+#define EN_VBAT_LOW_IRQ		(0x1 << 4)
+#define VBAT_LOW_ACT_MASK	(0x1 << 4)
+
+#define BUCK_ILMIN_MASK		(7 << 0)
+#define BOOST_ILMIN_MASK	(7 << 0)
+#define BUCK1_RATE_MASK		(3 << 3)
+#define BUCK2_RATE_MASK		(3 << 3)
+#define MASK_ALL	0xff
+
+#define SWITCH2_EN	BIT(6)
+#define SWITCH1_EN	BIT(5)
+#define DEV_OFF_RST	BIT(3)
+
+#define VB_LO_ACT		BIT(4)
+#define VB_LO_SEL_3500MV	(7 << 0)
+
+#define VOUT_LO_INT	BIT(0)
+#define CLK32KOUT2_EN	BIT(0)
+
+enum {
+	BUCK_ILMIN_50MA,
+	BUCK_ILMIN_100MA,
+	BUCK_ILMIN_150MA,
+	BUCK_ILMIN_200MA,
+	BUCK_ILMIN_250MA,
+	BUCK_ILMIN_300MA,
+	BUCK_ILMIN_350MA,
+	BUCK_ILMIN_400MA,
+};
+
+enum {
+	BOOST_ILMIN_75MA,
+	BOOST_ILMIN_100MA,
+	BOOST_ILMIN_125MA,
+	BOOST_ILMIN_150MA,
+	BOOST_ILMIN_175MA,
+	BOOST_ILMIN_200MA,
+	BOOST_ILMIN_225MA,
+	BOOST_ILMIN_250MA,
+};
+
+struct rk808 {
+	struct i2c_client *i2c;
+	struct regmap_irq_chip_data *irq_data;
+	struct regmap *regmap;
+};
+#endif /* __LINUX_REGULATOR_rk808_H */
-- 
1.7.9.5


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

* [PATCH v7 3/5] RTC: RK808: add RTC driver for RK808
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
  2014-09-01  9:39 ` [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808 Chris Zhong
@ 2014-09-01  9:43 ` Chris Zhong
  2014-09-02  3:58   ` Doug Anderson
  2014-09-01  9:46 ` [PATCH v7 4/5] clk: RK808: Add clkout " Chris Zhong
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:43 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette
  Cc: devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, dianders, heiko, olof, sonnyrao, dtor,
	javier.martinez, kever.yang, 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>

---

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 |  419 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 430 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..3de78e8
--- /dev/null
+++ b/drivers/rtc/rtc-rk808.c
@@ -0,0 +1,419 @@
+/*
+ * RTC driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * 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;
+
+	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);
+		goto reset_read_sel;
+	}
+
+	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);
+
+reset_read_sel:
+	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;
+}
+
+/* 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) {
+		dev_err(&pdev->dev, "Wake up is not possible as irq = %d\n",
+			rk808_rtc->irq);
+		return -ENXIO;
+	}
+
+	/* 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 <zhanqging@rock-chips.com>");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS("platform:rk808-rtc");
-- 
1.7.9.5


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

* [PATCH v7 4/5] clk: RK808: Add clkout driver for RK808
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
                   ` (2 preceding siblings ...)
  2014-09-01  9:43 ` [PATCH v7 3/5] RTC: RK808: add RTC " Chris Zhong
@ 2014-09-01  9:46 ` Chris Zhong
  2014-09-01 21:55   ` Mike Turquette
  2014-09-01  9:47 ` [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator Chris Zhong
  2014-09-02 19:40 ` [PATCH v7 0/5] Add rockchip RK808 pmic driver Heiko Stübner
  5 siblings, 1 reply; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:46 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette
  Cc: devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, dianders, heiko, olof, sonnyrao, dtor,
	javier.martinez, kever.yang, 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>
---

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 |  160 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 170 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..88f020b
--- /dev/null
+++ b/drivers/clk/clk-rk808.c
@@ -0,0 +1,160 @@
+/*
+ * Clkout driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * 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>
+
+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_clkout1_is_prepared(struct clk_hw *hw)
+{
+	return 1;
+}
+
+static int rk808_clkout2_control(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_control(hw, true);
+}
+
+static void rk808_clkout2_unprepare(struct clk_hw *hw)
+{
+	rk808_clkout2_control(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 = {
+	.is_prepared = rk808_clkout1_is_prepared,
+	.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_kzalloc(&client->dev,
+				 2 * 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 = 2;
+
+	return of_clk_add_provider(node, of_clk_src_onecell_get,
+				   &rk808_clkout->clk_data);
+}
+
+static struct platform_driver rk808_clkout_driver = {
+	.probe = rk808_clkout_probe,
+	.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


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

* [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
                   ` (3 preceding siblings ...)
  2014-09-01  9:46 ` [PATCH v7 4/5] clk: RK808: Add clkout " Chris Zhong
@ 2014-09-01  9:47 ` Chris Zhong
  2014-09-02  4:20   ` Doug Anderson
  2014-09-02 19:40 ` [PATCH v7 0/5] Add rockchip RK808 pmic driver Heiko Stübner
  5 siblings, 1 reply; 19+ messages in thread
From: Chris Zhong @ 2014-09-01  9:47 UTC (permalink / raw)
  To: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lee.jones, lgirdwood, a.zummo, mturquette
  Cc: devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, dianders, heiko, olof, sonnyrao, dtor,
	javier.martinez, kever.yang, Chris Zhong

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

---

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 |   52 ++++++++++++++---------------------
 1 file changed, 20 insertions(+), 32 deletions(-)

diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
index 0d11df1..4d543a9 100644
--- a/drivers/regulator/rk808-regulator.c
+++ b/drivers/regulator/rk808-regulator.c
@@ -3,9 +3,6 @@
  *
  * 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.
@@ -14,28 +11,25 @@
  * 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
 
+struct rk808_regulator {
+	struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
+	struct device_node *of_node[RK808_NUM_REGULATORS];
+};
+
 static const int buck_set_vol_base_addr[] = {
 	RK808_BUCK1_ON_VSEL_REG,
 	RK808_BUCK2_ON_VSEL_REG,
@@ -50,10 +44,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 +55,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),
 };
@@ -295,7 +283,7 @@ static struct of_regulator_match rk808_reg_matches[] = {
 };
 
 static int rk808_regulator_dts(struct i2c_client *client,
-			       struct rk808_board *pdata)
+			       struct rk808_regulator *rk808_regulator)
 {
 	struct device_node *np, *reg_np;
 	int i, ret;
@@ -323,8 +311,9 @@ static int rk808_regulator_dts(struct i2c_client *client,
 		    !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;
+		rk808_regulator->rk808_init_data[i] =
+				rk808_reg_matches[i].init_data;
+		rk808_regulator->of_node[i] = rk808_reg_matches[i].of_node;
 	}
 
 	return 0;
@@ -334,26 +323,25 @@ 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 rk808_regulator *rk808_regulator;
 	struct regulator_config config = {};
 	struct regulator_dev *rk808_rdev;
 	struct regulator_init_data *reg_data;
 	int i = 0;
 	int ret = 0;
 
-	if (!pdata) {
-		pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
-		if (!pdata)
-			return -ENOMEM;
-	}
+	rk808_regulator = devm_kzalloc(&pdev->dev, sizeof(*rk808_regulator),
+				       GFP_KERNEL);
+	if (!rk808_regulator)
+		return -ENOMEM;
 
-	ret = rk808_regulator_dts(client, pdata);
+	ret = rk808_regulator_dts(client, rk808_regulator);
 	if (ret)
 		return ret;
 
 	/* Instantiate the regulators */
 	for (i = 0; i < RK808_NUM_REGULATORS; i++) {
-		reg_data = pdata->rk808_init_data[i];
+		reg_data = rk808_regulator->rk808_init_data[i];
 		if (!reg_data)
 			continue;
 
@@ -362,7 +350,7 @@ static int rk808_regulator_probe(struct platform_device *pdev)
 		config.regmap = rk808->regmap;
 
 		if (client->dev.of_node)
-			config.of_node = pdata->of_node[i];
+			config.of_node = rk808_regulator->of_node[i];
 
 		reg_data->supply_regulator = rk808_reg[i].name;
 		config.init_data = reg_data;
-- 
1.7.9.5


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

* Re: [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808
  2014-09-01  9:39 ` [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808 Chris Zhong
@ 2014-09-01 10:09   ` Lee Jones
  2014-09-01 21:35     ` Doug Anderson
  0 siblings, 1 reply; 19+ messages in thread
From: Lee Jones @ 2014-09-01 10:09 UTC (permalink / raw)
  To: Chris Zhong
  Cc: robh+dt, pawel.moll, mark.rutland, ijc+devicetree, galak, sameo,
	lgirdwood, a.zummo, mturquette, devicetree, linux-kernel,
	rtc-linux, grant.likely, hl, huangtao, cf, zhangqing, xxx,
	dianders, heiko, olof, sonnyrao, dtor, javier.martinez,
	kever.yang

On Mon, 01 Sep 2014, Chris Zhong wrote:

> The RK808 chip is a power management IC for multimedia and handheld
> devices. It contains the following components:
> 
> - Regulators
> - RTC
> - Clkout
> 
> The RK808 core driver is registered as a platform driver and provides
> communication through I2C with the host device for the different
> components.
> 
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>

Couple of nits.  Once fixed you can apply my:

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

[...]

> +/*
> + * MFD core driver for Rockchip RK808
> + *
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd

Author?

> + * 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.
> + */

[...]

> +static int rk808_probe(struct i2c_client *client,
> +		       const struct i2c_device_id *id)
> +{
> +	int i;
> +	int ret;
> +	int pm_off = 0;
> +	struct rk808 *rk808;
> +	struct device_node *np = client->dev.of_node;

Reverse these declarations please, structs at the top etc.

[...]

> +	rk808->i2c = client;
> +	i2c_set_clientdata(client, rk808);

'\n' here.

> +	ret = mfd_add_devices(&client->dev, -1,
> +			      rk808s, ARRAY_SIZE(rk808s),
> +			      NULL, 0, regmap_irq_get_domain(rk808->irq_data));
> +	if (ret) {
> +		dev_err(&client->dev, "failed to add MFD devices %d\n", ret);
> +		goto err_irq;
> +	}
> +
> +	if (np) {

There has to be an 'np'.  The driver depends on OF.

> +		pm_off = of_property_read_bool(np,
> +					"rockchip,system-power-controller");
> +		if (pm_off && !pm_power_off) {
> +			rk808_i2c_client = client;
> +			pm_power_off = rk808_device_shutdown;
> +		}
> +	}
> +
> +	return 0;
> +
> +err_irq:
> +	regmap_del_irq_chip(client->irq, rk808->irq_data);
> +	return ret;
> +}

[...]

> +static struct i2c_driver rk808_i2c_driver = {
> +	.driver = {
> +		.name = "rk808",
> +		.of_match_table = of_match_ptr(rk808_of_match),

No need to use of_match_ptr() now that you depend on OF.

> +	},
> +	.probe    = rk808_probe,
> +	.remove   = rk808_remove,
> +	.id_table = rk808_ids,
> +};
> +
> +module_i2c_driver(rk808_i2c_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Chris Zhong <zyw@rock-chips.com>");
> +MODULE_AUTHOR("Zhang Qing <zhangqing@rock-chips.com>");
> +MODULE_DESCRIPTION("RK808 PMIC driver");

[...]

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808
  2014-09-01 10:09   ` Lee Jones
@ 2014-09-01 21:35     ` Doug Anderson
  2014-09-02  7:24       ` Lee Jones
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2014-09-01 21:35 UTC (permalink / raw)
  To: Lee Jones
  Cc: Chris Zhong, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Samuel Ortiz, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Hi,

On Mon, Sep 1, 2014 at 3:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 01 Sep 2014, Chris Zhong wrote:
>
>> The RK808 chip is a power management IC for multimedia and handheld
>> devices. It contains the following components:
>>
>> - Regulators
>> - RTC
>> - Clkout
>>
>> The RK808 core driver is registered as a platform driver and provides
>> communication through I2C with the host device for the different
>> components.
>>
>> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>
>
> Couple of nits.  Once fixed you can apply my:
>
> Acked-by: Lee Jones <lee.jones@linaro.org>
>
> [...]
>
>> +/*
>> + * MFD core driver for Rockchip RK808
>> + *
>> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
>
> Author?

I asked Chris to remove the author in my response to an earlier
version since it's at the bottom (MODULE_AUTHOR) and it seems extra
duplication.  You are the boss though, so if it should go both places
then Chris should add this back.  Sorry for the bad advice, Chris.

-Doug

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

* Re: [PATCH v7 4/5] clk: RK808: Add clkout driver for RK808
  2014-09-01  9:46 ` [PATCH v7 4/5] clk: RK808: Add clkout " Chris Zhong
@ 2014-09-01 21:55   ` Mike Turquette
  0 siblings, 0 replies; 19+ messages in thread
From: Mike Turquette @ 2014-09-01 21:55 UTC (permalink / raw)
  To: Chris Zhong, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
	galak, sameo, lee.jones, lgirdwood, a.zummo
  Cc: devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, dianders, heiko, olof, sonnyrao, dtor,
	javier.martinez, kever.yang, Chris Zhong

Quoting Chris Zhong (2014-09-01 02:46:40)
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> 
> 
> Reviewed-by: Doug Anderson <dianders@chromium.org>
> Tested-by: Doug Anderson <dianders@chromium.org>

Hello Chris,

Thanks for submitting this patch. Could you fill in a proper changelog?
Also you should reorder the tags like so:

Reviewed-by: Doug Anderson <dianders@chromium.org>
Tested-by: Doug Anderson <dianders@chromium.org>
Signed-off-by: Chris Zhong <zyw@rock-chips.com>

<snip>

> +static int rk808_clkout2_control(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);
> +}

Nitpick: can you rename "control" to "enable" or "ungate"? That makes it
more obvious what the function is doing without having to inspect the
code in the function body.

<snip>

> +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_kzalloc(&client->dev,
> +                                2 * sizeof(struct clk *), GFP_KERNEL);

Better to use devm_kcalloc. Also good to define a constant like:

#define RK808_NR_OUTPUT 2

... and then use RK8808_NR_OUTPUT instead of hard-coding the value 2 in
the driver.

> +       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 = 2;

Again, here you can use RK808_NR_OUTPUT.

Otherwise the driver looks pretty good to me.

Thanks,
Mike

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

* Re: [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
@ 2014-09-02  3:42   ` Doug Anderson
  2014-09-02  3:42   ` Doug Anderson
  2014-09-02  3:46   ` Doug Anderson
  2 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-09-02  3:42 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Heiko Stübner, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Alessandro Zummo, Mike Turquette, devicetree, linux-kernel,
	rtc-linux, Grant Likely, Lin Huang, Tao Huang, Eddie Cai,
	zhangqing, xxx, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Mon, Sep 1, 2014 at 2:07 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Add device tree bindings documentation and a header file
> for rockchip's RK808 pmic.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>
>
> ---
>
> Changes in v7:
> Advices by Mark Rutland
> - modify description about clock-cells
> - update the example
>
> Changes in v6:
> Advices by Mark Rutland
> - add description about clock-cells
> Advices by Doug
> - modify description about regulator
> - remove pinctrl description
>
> Changes in v5:
> Advices by Mark Brown
> - add description about regulator valid name.
> - add a header file "rockchip,rk808".
>
> Changes in v4:
> Advices by Doug
> - add a "#clock-cells" propertiy
> - update the example
>
> Changes in v3: None
> Changes in v2: None
>
>  Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++++++++++++++++
>  include/dt-bindings/clock/rockchip,rk808.h      |   11 ++
>  2 files changed, 177 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
>  create mode 100644 include/dt-bindings/clock/rockchip,rk808.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/rk808.txt b/Documentation/devicetree/bindings/mfd/rk808.txt
> new file mode 100644
> index 0000000..f8932ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rk808.txt
> @@ -0,0 +1,166 @@
> +RK808 Power Management Integrated Circuit
> +
> +Required properties:
> +- compatible: "rockchip,rk808"
> +- reg: I2C slave address
> +- interrupt-parent: The parent interrupt controller.
> +- interrupts: the interrupt outputs of the controller.
> +- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
> +  outputs)

I think Mark Rutland was looking for something like:

- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
  outputs).  See <dt-bindings/clock/rockchip,rk808.h> for clock IDs.


> +Optional properties:
> +- clock-output-names: From common clock binding to override the
> +  default output clock name
> +- rockchip,system-power-controller: Telling whether or not this pmic is controlling
> +  the system power.
> +
> +Regulators: All the regulators of RK808 to be instantiated shall be
> +listed in a child node named 'regulators'. Each regulator is represented
> +by a child node of the 'regulators' node.
> +
> +       regulator-name {
> +               /* standard regulator bindings here */
> +       };
> +
> +Following regulators of the RK808 PMIC block are supported. Note that
> +the 'n' in regulator name, as in DCDC_REGn or LDOn, represents the DCDC or LDO
> +number as described in RK808 datasheet.
> +
> +       - DCDC_REGn
> +               - valid values for n are 1 to 4.
> +       - LDO_REGn
> +               - valid values for n are 1 to 8.
> +       - SWITCH_REGn
> +               - valid values for n are 1 to 2
> +
> +Standard regulator bindings are used inside regulator subnodes. Check
> +  Documentation/devicetree/bindings/regulator/regulator.txt
> +for more details
> +
> +Example:
> +       rk808: pmic@1b {
> +               compatible = "rockchip,rk808";
> +               clock-output-names = "xin32k", "rk808-clkout2";
> +               interrupt-parent = <&gpio0>;
> +               interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pmic_int>;
> +               reg = <0x1b>;
> +               rockchip,system-power-controller;
> +               wakeup-source;
> +               #clock-cells = <1>;
> +
> +               vcc8-supply = <&vcc_18>;
> +               vcc9-supply = <&vcc_io>;
> +               vcc10-supply = <&vcc_io>;
> +               vcc12-supply = <&vcc_io>;
> +               vddio-supply = <&vccio_pmu>;

Technically the "-supply" section shouldn't be here.  I plan to add
support for input supplies once you've landed your series.  See
<https://chromium-review.googlesource.com/#/c/214596/> for a preview.
Just leave the "-supply" section out for now.

Other than that, this looks great.

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

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

* Re: [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
  2014-09-02  3:42   ` Doug Anderson
@ 2014-09-02  3:42   ` Doug Anderson
  2014-09-02  3:46   ` Doug Anderson
  2 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-09-02  3:42 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Heiko Stübner, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Alessandro Zummo, Mike Turquette, devicetree, linux-kernel,
	rtc-linux, Grant Likely, Lin Huang, Tao Huang, Eddie Cai,
	zhangqing, xxx, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Mon, Sep 1, 2014 at 2:07 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Add device tree bindings documentation and a header file
> for rockchip's RK808 pmic.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>
>
> ---
>
> Changes in v7:
> Advices by Mark Rutland
> - modify description about clock-cells
> - update the example
>
> Changes in v6:
> Advices by Mark Rutland
> - add description about clock-cells
> Advices by Doug
> - modify description about regulator
> - remove pinctrl description
>
> Changes in v5:
> Advices by Mark Brown
> - add description about regulator valid name.
> - add a header file "rockchip,rk808".
>
> Changes in v4:
> Advices by Doug
> - add a "#clock-cells" propertiy
> - update the example
>
> Changes in v3: None
> Changes in v2: None
>
>  Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++++++++++++++++
>  include/dt-bindings/clock/rockchip,rk808.h      |   11 ++
>  2 files changed, 177 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
>  create mode 100644 include/dt-bindings/clock/rockchip,rk808.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/rk808.txt b/Documentation/devicetree/bindings/mfd/rk808.txt
> new file mode 100644
> index 0000000..f8932ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rk808.txt
> @@ -0,0 +1,166 @@
> +RK808 Power Management Integrated Circuit
> +
> +Required properties:
> +- compatible: "rockchip,rk808"
> +- reg: I2C slave address
> +- interrupt-parent: The parent interrupt controller.
> +- interrupts: the interrupt outputs of the controller.
> +- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
> +  outputs)

I think Mark Rutland was looking for something like:

- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
  outputs).  See <dt-bindings/clock/rockchip,rk808.h> for clock IDs.


> +Optional properties:
> +- clock-output-names: From common clock binding to override the
> +  default output clock name
> +- rockchip,system-power-controller: Telling whether or not this pmic is controlling
> +  the system power.
> +
> +Regulators: All the regulators of RK808 to be instantiated shall be
> +listed in a child node named 'regulators'. Each regulator is represented
> +by a child node of the 'regulators' node.
> +
> +       regulator-name {
> +               /* standard regulator bindings here */
> +       };
> +
> +Following regulators of the RK808 PMIC block are supported. Note that
> +the 'n' in regulator name, as in DCDC_REGn or LDOn, represents the DCDC or LDO
> +number as described in RK808 datasheet.
> +
> +       - DCDC_REGn
> +               - valid values for n are 1 to 4.
> +       - LDO_REGn
> +               - valid values for n are 1 to 8.
> +       - SWITCH_REGn
> +               - valid values for n are 1 to 2
> +
> +Standard regulator bindings are used inside regulator subnodes. Check
> +  Documentation/devicetree/bindings/regulator/regulator.txt
> +for more details
> +
> +Example:
> +       rk808: pmic@1b {
> +               compatible = "rockchip,rk808";
> +               clock-output-names = "xin32k", "rk808-clkout2";
> +               interrupt-parent = <&gpio0>;
> +               interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pmic_int>;
> +               reg = <0x1b>;
> +               rockchip,system-power-controller;
> +               wakeup-source;
> +               #clock-cells = <1>;
> +
> +               vcc8-supply = <&vcc_18>;
> +               vcc9-supply = <&vcc_io>;
> +               vcc10-supply = <&vcc_io>;
> +               vcc12-supply = <&vcc_io>;
> +               vddio-supply = <&vccio_pmu>;

Technically the "-supply" section shouldn't be here.  I plan to add
support for input supplies once you've landed your series.  See
<https://chromium-review.googlesource.com/#/c/214596/> for a preview.
Just leave the "-supply" section out for now.

Other than that, this looks great.

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

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

* Re: [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document
  2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
  2014-09-02  3:42   ` Doug Anderson
  2014-09-02  3:42   ` Doug Anderson
@ 2014-09-02  3:46   ` Doug Anderson
  2 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-09-02  3:46 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Heiko Stübner, Rob Herring, Pawel Moll, Mark Rutland,
	Ian Campbell, Kumar Gala, Samuel Ortiz, Lee Jones, Liam Girdwood,
	Alessandro Zummo, Mike Turquette, devicetree, linux-kernel,
	rtc-linux, Grant Likely, Lin Huang, Tao Huang, Eddie Cai,
	zhangqing, xxx, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Mon, Sep 1, 2014 at 2:07 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Add device tree bindings documentation and a header file
> for rockchip's RK808 pmic.
>
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>
>
> ---
>
> Changes in v7:
> Advices by Mark Rutland
> - modify description about clock-cells
> - update the example
>
> Changes in v6:
> Advices by Mark Rutland
> - add description about clock-cells
> Advices by Doug
> - modify description about regulator
> - remove pinctrl description
>
> Changes in v5:
> Advices by Mark Brown
> - add description about regulator valid name.
> - add a header file "rockchip,rk808".
>
> Changes in v4:
> Advices by Doug
> - add a "#clock-cells" propertiy
> - update the example
>
> Changes in v3: None
> Changes in v2: None
>
>  Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++++++++++++++++
>  include/dt-bindings/clock/rockchip,rk808.h      |   11 ++
>  2 files changed, 177 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
>  create mode 100644 include/dt-bindings/clock/rockchip,rk808.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/rk808.txt b/Documentation/devicetree/bindings/mfd/rk808.txt
> new file mode 100644
> index 0000000..f8932ac
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/rk808.txt
> @@ -0,0 +1,166 @@
> +RK808 Power Management Integrated Circuit
> +
> +Required properties:
> +- compatible: "rockchip,rk808"
> +- reg: I2C slave address
> +- interrupt-parent: The parent interrupt controller.
> +- interrupts: the interrupt outputs of the controller.
> +- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
> +  outputs)

I think Mark Rutland was looking for something like:

- #clock-cells: from common clock binding; shall be set to 1 (multiple clock
  outputs).  See <dt-bindings/clock/rockchip,rk808.h> for clock IDs.


> +Optional properties:
> +- clock-output-names: From common clock binding to override the
> +  default output clock name
> +- rockchip,system-power-controller: Telling whether or not this pmic is controlling
> +  the system power.
> +
> +Regulators: All the regulators of RK808 to be instantiated shall be
> +listed in a child node named 'regulators'. Each regulator is represented
> +by a child node of the 'regulators' node.
> +
> +       regulator-name {
> +               /* standard regulator bindings here */
> +       };
> +
> +Following regulators of the RK808 PMIC block are supported. Note that
> +the 'n' in regulator name, as in DCDC_REGn or LDOn, represents the DCDC or LDO
> +number as described in RK808 datasheet.
> +
> +       - DCDC_REGn
> +               - valid values for n are 1 to 4.
> +       - LDO_REGn
> +               - valid values for n are 1 to 8.
> +       - SWITCH_REGn
> +               - valid values for n are 1 to 2
> +
> +Standard regulator bindings are used inside regulator subnodes. Check
> +  Documentation/devicetree/bindings/regulator/regulator.txt
> +for more details
> +
> +Example:
> +       rk808: pmic@1b {
> +               compatible = "rockchip,rk808";
> +               clock-output-names = "xin32k", "rk808-clkout2";
> +               interrupt-parent = <&gpio0>;
> +               interrupts = <4 IRQ_TYPE_LEVEL_LOW>;
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&pmic_int>;
> +               reg = <0x1b>;
> +               rockchip,system-power-controller;
> +               wakeup-source;
> +               #clock-cells = <1>;
> +
> +               vcc8-supply = <&vcc_18>;
> +               vcc9-supply = <&vcc_io>;
> +               vcc10-supply = <&vcc_io>;
> +               vcc12-supply = <&vcc_io>;
> +               vddio-supply = <&vccio_pmu>;

Technically the "-supply" section shouldn't be here.  I plan to add
support for input supplies once you've landed your series.  See
<https://chromium-review.googlesource.com/#/c/214596/> for a preview.
Just leave the "-supply" section out for now.

Other than that, this looks great.

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

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

* Re: [PATCH v7 3/5] RTC: RK808: add RTC driver for RK808
  2014-09-01  9:43 ` [PATCH v7 3/5] RTC: RK808: add RTC " Chris Zhong
@ 2014-09-02  3:58   ` Doug Anderson
  2014-09-03  2:01     ` Chris Zhong
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2014-09-02  3:58 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Samuel Ortiz, Lee Jones, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Mon, Sep 1, 2014 at 2:43 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> 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>
>
> ---
>
> 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 |  419 +++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 430 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..3de78e8
> --- /dev/null
> +++ b/drivers/rtc/rtc-rk808.c
> @@ -0,0 +1,419 @@
> +/*
> + * RTC driver for Rockchip RK808
> + *
> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> + *
> + * 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;
> +
> +       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);
> +               goto reset_read_sel;
> +       }
> +
> +       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);
> +
> +reset_read_sel:
> +       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;

Nit: blank line before "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);

I think this should still be setting to 0, not to
BIT_RTC_CTRL_REG_RTC_READSEL_M.  Otherwise the first read of the time
will return that time that was frozen at probe time, right?  AKA: if
probe happens at 11:00:00 and then we read the time at 11:00:05 we'll
still read 11:00:00 the first time.


> +       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) {
> +               dev_err(&pdev->dev, "Wake up is not possible as irq = %d\n",
> +                       rk808_rtc->irq);
> +               return -ENXIO;
> +       }

I think you misinterpreted Dmitry.  He said this block was wrong, but
you didn't quite make the right fix.  I believe the correct fix is
that an IRQ of "0" is a valid IRQ.  Thus, you should have:

       rk808_rtc->irq = platform_get_irq(pdev, 0);
       if (rk808_rtc->irq < 0) {
               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 <zhanqging@rock-chips.com>");
> +MODULE_LICENSE("GPL");
> +MODULE_ALIAS("platform:rk808-rtc");
> --
> 1.7.9.5
>

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

* Re: [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator
  2014-09-01  9:47 ` [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator Chris Zhong
@ 2014-09-02  4:20   ` Doug Anderson
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-09-02  4:20 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Samuel Ortiz, Lee Jones, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Mon, Sep 1, 2014 at 2:47 AM, Chris Zhong <zyw@rock-chips.com> wrote:
> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
>
> ---
>
> 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 |   52 ++++++++++++++---------------------
>  1 file changed, 20 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/regulator/rk808-regulator.c b/drivers/regulator/rk808-regulator.c
> index 0d11df1..4d543a9 100644
> --- a/drivers/regulator/rk808-regulator.c
> +++ b/drivers/regulator/rk808-regulator.c
> @@ -3,9 +3,6 @@
>   *
>   * 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.
> @@ -14,28 +11,25 @@
>   * 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
>
> +struct rk808_regulator {
> +       struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
> +       struct device_node *of_node[RK808_NUM_REGULATORS];
> +};
> +
>  static const int buck_set_vol_base_addr[] = {
>         RK808_BUCK1_ON_VSEL_REG,
>         RK808_BUCK2_ON_VSEL_REG,
> @@ -50,10 +44,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 +55,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),
>  };
> @@ -295,7 +283,7 @@ static struct of_regulator_match rk808_reg_matches[] = {
>  };
>
>  static int rk808_regulator_dts(struct i2c_client *client,
> -                              struct rk808_board *pdata)
> +                              struct rk808_regulator *rk808_regulator)
>  {
>         struct device_node *np, *reg_np;
>         int i, ret;
> @@ -323,8 +311,9 @@ static int rk808_regulator_dts(struct i2c_client *client,
>                     !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;
> +               rk808_regulator->rk808_init_data[i] =
> +                               rk808_reg_matches[i].init_data;
> +               rk808_regulator->of_node[i] = rk808_reg_matches[i].of_node;
>         }
>
>         return 0;
> @@ -334,26 +323,25 @@ 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 rk808_regulator *rk808_regulator;
>         struct regulator_config config = {};
>         struct regulator_dev *rk808_rdev;
>         struct regulator_init_data *reg_data;
>         int i = 0;
>         int ret = 0;
>
> -       if (!pdata) {
> -               pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
> -               if (!pdata)
> -                       return -ENOMEM;
> -       }
> +       rk808_regulator = devm_kzalloc(&pdev->dev, sizeof(*rk808_regulator),
> +                                      GFP_KERNEL);
> +       if (!rk808_regulator)
> +               return -ENOMEM;

Totally remove rk808_regulator_dts() function.  Totally remove
"rk808_regulator" local variable (and allocation, and typedef).  Then
you can just use "rk808_reg_matches" in the "for" loop in your probe
function, right?  You're going to get rid of a ton of code...

Remember, you can also remove the check about "client->dev.of_node".
You depend on OF...  Also it appears that "of_regulator_match" prints
an error for you in at least the common error case, so I think you can
get rid of "failed to parse regulator data: %d\n" printout.

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

* Re: [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808
  2014-09-01 21:35     ` Doug Anderson
@ 2014-09-02  7:24       ` Lee Jones
  0 siblings, 0 replies; 19+ messages in thread
From: Lee Jones @ 2014-09-02  7:24 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Chris Zhong, Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell,
	Kumar Gala, Samuel Ortiz, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

On Mon, 01 Sep 2014, Doug Anderson wrote:
> On Mon, Sep 1, 2014 at 3:09 AM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 01 Sep 2014, Chris Zhong wrote:
> >
> >> The RK808 chip is a power management IC for multimedia and handheld
> >> devices. It contains the following components:
> >>
> >> - Regulators
> >> - RTC
> >> - Clkout
> >>
> >> The RK808 core driver is registered as a platform driver and provides
> >> communication through I2C with the host device for the different
> >> components.
> >>
> >> Signed-off-by: Chris Zhong <zyw@rock-chips.com>
> >> Signed-off-by: Zhang Qing <zhangqing@rock-chips.com>
> >
> > Couple of nits.  Once fixed you can apply my:
> >
> > Acked-by: Lee Jones <lee.jones@linaro.org>
> >
> > [...]
> >
> >> +/*
> >> + * MFD core driver for Rockchip RK808
> >> + *
> >> + * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
> >
> > Author?
> 
> I asked Chris to remove the author in my response to an earlier
> version since it's at the bottom (MODULE_AUTHOR) and it seems extra
> duplication.  You are the boss though, so if it should go both places
> then Chris should add this back.  Sorry for the bad advice, Chris.

Yes, both places please.  They do different things.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

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

* Re: [PATCH v7 0/5] Add rockchip RK808 pmic driver
  2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
                   ` (4 preceding siblings ...)
  2014-09-01  9:47 ` [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator Chris Zhong
@ 2014-09-02 19:40 ` Heiko Stübner
  5 siblings, 0 replies; 19+ messages in thread
From: Heiko Stübner @ 2014-09-02 19:40 UTC (permalink / raw)
  To: Chris Zhong
  Cc: dianders, robh+dt, pawel.moll, mark.rutland, ijc+devicetree,
	galak, sameo, lee.jones, lgirdwood, a.zummo, mturquette,
	devicetree, linux-kernel, rtc-linux, grant.likely, hl, huangtao,
	cf, zhangqing, xxx, olof, sonnyrao, dtor, javier.martinez,
	kever.yang

Am Montag, 1. September 2014, 17:07:43 schrieb 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

The whole series on a 3-17-rc1 with the already applied regulator commits 
pulled in and used by my pending cpufreq patches to set the core voltage

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



> 
> Changes in v7:
> Advices by Mark Rutland
> - modify description about clock-cells
> - update the example
> Adviced by Lee Jones
> - coding style
> - remove rk808_pre_init function
> Adviced by Doug
> - add "&& OF" to the dependencies
> - add .init_ack_masked = true in rk808_irq_chip
> 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:
> Advices by Mark Rutland
> - add description about clock-cells
> Advices by Doug
> - modify description about regulator
> - remove pinctrl description
> Adviced by Lee Jones in v2
> - rk808_i2c_client instead of g_rk808
> - remove pdata form struct rk808
> 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:
> Advices by Mark Brown
> - add description about regulator valid name.
> - add a header file "rockchip,rk808".
> - 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:
> Advices by Doug
> - add a "#clock-cells" propertiy
> - update the example
> Adviced by Lee Jones in v2
> - modify the description in Kconfig
> - remove some unnecessary header files
> - remove dev from struct rk808
> - use enum for define RK808_ID...
> - 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
> - fix compile err
> 
> Changes in v2:
> Adviced by Mark Browm:
> - use defines for register setting value
> - remove rtc alarm disable in shutdown
> - remove while(1) in shutdown
> - remove read 0x2f in probe
> 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 (5):
>   dt-bindings: Add RK808 device tree bindings document
>   MFD: RK808: Add new mfd driver for RK808
>   RTC: RK808: add RTC driver for RK808
>   clk: RK808: Add clkout driver for RK808
>   regulator: RK808: Remove pdata from the regulator
> 
>  Documentation/devicetree/bindings/mfd/rk808.txt |  166 +++++++++
>  drivers/clk/Kconfig                             |    9 +
>  drivers/clk/Makefile                            |    1 +
>  drivers/clk/clk-rk808.c                         |  160 +++++++++
>  drivers/mfd/Kconfig                             |   13 +
>  drivers/mfd/Makefile                            |    1 +
>  drivers/mfd/rk808.c                             |  243 +++++++++++++
>  drivers/regulator/rk808-regulator.c             |   52 ++-
>  drivers/rtc/Kconfig                             |   10 +
>  drivers/rtc/Makefile                            |    1 +
>  drivers/rtc/rtc-rk808.c                         |  419
> +++++++++++++++++++++++ include/dt-bindings/clock/rockchip,rk808.h      |  
> 11 +
>  include/linux/mfd/rk808.h                       |  196 +++++++++++
>  13 files changed, 1250 insertions(+), 32 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/mfd/rk808.txt
>  create mode 100644 drivers/clk/clk-rk808.c
>  create mode 100644 drivers/mfd/rk808.c
>  create mode 100644 drivers/rtc/rtc-rk808.c
>  create mode 100644 include/dt-bindings/clock/rockchip,rk808.h
>  create mode 100644 include/linux/mfd/rk808.h


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

* Re: [PATCH v7 3/5] RTC: RK808: add RTC driver for RK808
  2014-09-02  3:58   ` Doug Anderson
@ 2014-09-03  2:01     ` Chris Zhong
  2014-09-03  3:52       ` Doug Anderson
  0 siblings, 1 reply; 19+ messages in thread
From: Chris Zhong @ 2014-09-03  2:01 UTC (permalink / raw)
  To: Doug Anderson
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Samuel Ortiz, Lee Jones, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang


On 09/02/2014 11:58 AM, Doug Anderson wrote:
>> +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);
> I think this should still be setting to 0, not to
> BIT_RTC_CTRL_REG_RTC_READSEL_M.  Otherwise the first read of the time
> will return that time that was frozen at probe time, right?  AKA: if
> probe happens at 11:00:00 and then we read the time at 11:00:05 we'll
> still read 11:00:00 the first time.
>
  Sorry, I did not describe correctly, in the previous mail.
  Actually, RK808 has a "GET_TIME" switch bit. When "GET_TIME" bit rising thansiton to 1,
  the current time will save in a shadowed register.
  If "READSEL" = 1, read rtc time register, return the frozen time.
  If we need the real time, clr this "READSEL" bit.



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

* Re: [PATCH v7 3/5] RTC: RK808: add RTC driver for RK808
  2014-09-03  2:01     ` Chris Zhong
@ 2014-09-03  3:52       ` Doug Anderson
  2014-09-03  3:53         ` Doug Anderson
  0 siblings, 1 reply; 19+ messages in thread
From: Doug Anderson @ 2014-09-03  3:52 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Samuel Ortiz, Lee Jones, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Chris,

On Tue, Sep 2, 2014 at 7:01 PM, Chris Zhong <zyw@rock-chips.com> wrote:
>
> On 09/02/2014 11:58 AM, Doug Anderson wrote:
>>>
>>> +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);
>>
>> I think this should still be setting to 0, not to
>> BIT_RTC_CTRL_REG_RTC_READSEL_M.  Otherwise the first read of the time
>> will return that time that was frozen at probe time, right?  AKA: if
>> probe happens at 11:00:00 and then we read the time at 11:00:05 we'll
>> still read 11:00:00 the first time.
>>
>  Sorry, I did not describe correctly, in the previous mail.
>  Actually, RK808 has a "GET_TIME" switch bit. When "GET_TIME" bit rising
> thansiton to 1,
>  the current time will save in a shadowed register.
>  If "READSEL" = 1, read rtc time register, return the frozen time.
>  If we need the real time, clr this "READSEL" bit.

Ohhhhh!  I see.  I clearly didn't look closely enough at the whole
register description in the user manual.  I think you've got it right,
then.

I think this would be more obvious if you added a comment and you set
both the high and low bits at the same time.  AKA:

+       /* 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,
+                                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_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;
+       }

...doing them both at the same time makes it obvious that this is a
one-time copy triggered by the "rising edge" of this bit.  It also
means you don't need any "goto" for error handling.

-Doug

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

* Re: [PATCH v7 3/5] RTC: RK808: add RTC driver for RK808
  2014-09-03  3:52       ` Doug Anderson
@ 2014-09-03  3:53         ` Doug Anderson
  0 siblings, 0 replies; 19+ messages in thread
From: Doug Anderson @ 2014-09-03  3:53 UTC (permalink / raw)
  To: Chris Zhong
  Cc: Rob Herring, Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala,
	Samuel Ortiz, Lee Jones, Liam Girdwood, Alessandro Zummo,
	Mike Turquette, devicetree, linux-kernel, rtc-linux,
	Grant Likely, Lin Huang, Tao Huang, Eddie Cai, zhangqing, xxx,
	Heiko Stübner, Olof Johansson, Sonny Rao, Dmitry Torokhov,
	Javier Martinez Canillas, Kever Yang

Hi,

On Tue, Sep 2, 2014 at 8:52 PM, Doug Anderson <dianders@chromium.org> wrote:
> Chris,
>
> On Tue, Sep 2, 2014 at 7:01 PM, Chris Zhong <zyw@rock-chips.com> wrote:
>>
>> On 09/02/2014 11:58 AM, Doug Anderson wrote:
>>>>
>>>> +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);
>>>
>>> I think this should still be setting to 0, not to
>>> BIT_RTC_CTRL_REG_RTC_READSEL_M.  Otherwise the first read of the time
>>> will return that time that was frozen at probe time, right?  AKA: if
>>> probe happens at 11:00:00 and then we read the time at 11:00:05 we'll
>>> still read 11:00:00 the first time.
>>>
>>  Sorry, I did not describe correctly, in the previous mail.
>>  Actually, RK808 has a "GET_TIME" switch bit. When "GET_TIME" bit rising
>> thansiton to 1,
>>  the current time will save in a shadowed register.
>>  If "READSEL" = 1, read rtc time register, return the frozen time.
>>  If we need the real time, clr this "READSEL" bit.
>
> Ohhhhh!  I see.  I clearly didn't look closely enough at the whole
> register description in the user manual.  I think you've got it right,
> then.
>
> I think this would be more obvious if you added a comment and you set
> both the high and low bits at the same time.  AKA:
>
> +       /* 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,
> +                                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_update_bits(rk808->regmap, RK808_RTC_CTRL_REG,
> +                                BIT_RTC_CTRL_REG_RTC_GET_TIME,
> +                                BIT_RTC_CTRL_REG_RTC_GET_TIME);

Doh, this one was supposed to be setting it back to 0.

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

end of thread, other threads:[~2014-09-03  3:53 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-01  9:07 [PATCH v7 0/5] Add rockchip RK808 pmic driver Chris Zhong
2014-09-01  9:07 ` [PATCH v7 1/5] dt-bindings: Add RK808 device tree bindings document Chris Zhong
2014-09-02  3:42   ` Doug Anderson
2014-09-02  3:42   ` Doug Anderson
2014-09-02  3:46   ` Doug Anderson
2014-09-01  9:39 ` [PATCH v7 2/5] MFD: RK808: Add new mfd driver for RK808 Chris Zhong
2014-09-01 10:09   ` Lee Jones
2014-09-01 21:35     ` Doug Anderson
2014-09-02  7:24       ` Lee Jones
2014-09-01  9:43 ` [PATCH v7 3/5] RTC: RK808: add RTC " Chris Zhong
2014-09-02  3:58   ` Doug Anderson
2014-09-03  2:01     ` Chris Zhong
2014-09-03  3:52       ` Doug Anderson
2014-09-03  3:53         ` Doug Anderson
2014-09-01  9:46 ` [PATCH v7 4/5] clk: RK808: Add clkout " Chris Zhong
2014-09-01 21:55   ` Mike Turquette
2014-09-01  9:47 ` [PATCH v7 5/5] regulator: RK808: Remove pdata from the regulator Chris Zhong
2014-09-02  4:20   ` Doug Anderson
2014-09-02 19:40 ` [PATCH v7 0/5] Add rockchip RK808 pmic driver Heiko Stübner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).