[PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
Signed-off-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>

---

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       |  257 +++++++++++++++++++++++++++++++++++++++++++++
 include/linux/mfd/rk808.h |  202 +++++++++++++++++++++++++++++++++++
 4 files changed, 473 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..2c7fdb2 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
+	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..f0d6518
--- /dev/null
+++ b/drivers/mfd/rk808.c
@@ -0,0 +1,257 @@
+/*
+ * MFD core driver for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ * Author: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * 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 },
+	{ RK808_INT_STS_REG1,     MASK_NONE,        0 },
+	{ RK808_INT_STS_REG2,     MASK_NONE,        0 },
+};
+
+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,
+};
+
+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_pre_init(struct rk808 *rk808)
+{
+	int i;
+	int ret;
+	struct i2c_client *client = rk808->i2c;
+
+	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;
+		}
+	}
+
+	return 0;
+}
+
+static int rk808_probe(struct i2c_client *client,
+		       const struct i2c_device_id *id)
+{
+	int ret;
+	int pm_off = 0;
+	struct rk808 *rk808;
+	struct device_node *np = client->dev.of_node;
+
+	pm_off = of_property_read_bool(np,
+				       "rockchip,system-power-controller");
+
+	rk808 = devm_kzalloc(&client->dev, sizeof(*rk808), GFP_KERNEL);
+	if (!rk808)
+		return -ENOMEM;
+
+	rk808->i2c = client;
+	i2c_set_clientdata(client, rk808);
+
+	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);
+	}
+
+	ret = rk808_pre_init(rk808);
+	if (ret)
+		return ret;
+
+	if (!client->irq) {
+		dev_err(&client->dev, "No interrupt support, no core IRQ\n");
+		return -EINVAL;
+	}
+
+	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;
+	}
+
+	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 (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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>");
+MODULE_AUTHOR("Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>");
+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..7af1952
--- /dev/null
+++ b/include/linux/mfd/rk808.h
@@ -0,0 +1,202 @@
+/*
+ * rk808.h for Rockchip RK808
+ *
+ * Copyright (c) 2014, Fuzhou Rockchip Electronics Co., Ltd
+ *
+ * Author: Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ * Author: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
+ *
+ * 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 MASK_NONE	0
+
+#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_board {
+	struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
+	struct device_node *of_node[RK808_NUM_REGULATORS];
+};
+
+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


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

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Doug Anderson]

Chris,

On Tue, Aug 26, 2014 at 7:14 AM, Chris Zhong <zyw-TNX95d0MmH7DzftRWevZcw@public.gmane.org> wrote:
> +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,

Can you add ".init_ack_masked = true" here?  If I don't do that and I
boot with the IRQ configured as "level low" then the system will just
keep getting IRQs that it can't handle and eventually disable the
whole rk808.  If I boot with the IRQ as "falling edge" then the system
will never get RTC alarms.

I posted this fixup with an explanation at
<https://chromium-review.googlesource.com/#/c/214241/> and it could be
squashed into your patch.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Lee Jones]

On Tue, 26 Aug 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>
> 
> ---
> 
> 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       |  257 +++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/mfd/rk808.h |  202 +++++++++++++++++++++++++++++++++++
>  4 files changed, 473 insertions(+)
>  create mode 100644 drivers/mfd/rk808.c
>  create mode 100644 include/linux/mfd/rk808.h

[...]

> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> new file mode 100644
> index 0000000..f0d6518
> --- /dev/null
> +++ b/drivers/mfd/rk808.c

[...]

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

Are you prepared for np == NULL?

> +	pm_off = of_property_read_bool(np,
> +				       "rockchip,system-power-controller");

Rather than have this floating up here, can you put it down where it
is used?

> +	rk808 = devm_kzalloc(&client->dev, sizeof(*rk808), GFP_KERNEL);
> +	if (!rk808)
> +		return -ENOMEM;
> +
> +	rk808->i2c = client;
> +	i2c_set_clientdata(client, rk808);

I tend to like drive data set at the last moment.  Usually just
before mfd_add_devices().

> +	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);
> +	}
> +
> +	ret = rk808_pre_init(rk808);
> +	if (ret)
> +		return ret;

Does this need to be a seperate function?  It's only one call to
egmap_update_bits() and is not reused.

> +	if (!client->irq) {
> +		dev_err(&client->dev, "No interrupt support, no core IRQ\n");
> +		return -EINVAL;
> +	}

If this is a show stopper, perhaps you should test for it higher up
before allocating anything else?

> +	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;
> +	}
> +
> +	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 (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;

'\n' here.  No need to squash everything together.

> +	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);

My OCD senses are tingling.  Either have a blank line above the calls
to MODULE_DEVICE_TABLE() or don't.

> +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..7af1952
> --- /dev/null
> +++ b/include/linux/mfd/rk808.h
> @@ -0,0 +1,202 @@

[...]

> +struct rk808_board {
> +	struct regulator_init_data *rk808_init_data[RK808_NUM_REGULATORS];
> +	struct device_node *of_node[RK808_NUM_REGULATORS];
> +};

This is only used in the regulator driver.  Do you have a regulator
specific header file that you can define it in instead?

> +struct rk808 {
> +	struct i2c_client *i2c;
> +	struct regmap_irq_chip_data *irq_data;
> +	struct regmap *regmap;
> +};
> +#endif /* __LINUX_REGULATOR_rk808_H */

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

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Doug Anderson]

Lee,

On Wed, Aug 27, 2014 at 6:39 AM, Lee Jones <lee.jones@linaro.org> wrote:
>> +static int rk808_probe(struct i2c_client *client,
>> +                    const struct i2c_device_id *id)
>> +{
>> +     int ret;
>> +     int pm_off = 0;
>> +     struct rk808 *rk808;
>> +     struct device_node *np = client->dev.of_node;
>
> Are you prepared for np == NULL?

Given that the driver no longer supports getting its config from
platform data, I think it simply depends on OF now.  It seems like we
could add a "&& OF" to the dependencies.  Is that OK with you?

It doesn't hurt to still add a check for np == NULL and bail out, though.

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Lee Jones]

On Wed, 27 Aug 2014, Doug Anderson wrote:
> On Wed, Aug 27, 2014 at 6:39 AM, Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> >> +static int rk808_probe(struct i2c_client *client,
> >> +                    const struct i2c_device_id *id)
> >> +{
> >> +     int ret;
> >> +     int pm_off = 0;
> >> +     struct rk808 *rk808;
> >> +     struct device_node *np = client->dev.of_node;
> >
> > Are you prepared for np == NULL?
> 
> Given that the driver no longer supports getting its config from
> platform data, I think it simply depends on OF now.  It seems like we
> could add a "&& OF" to the dependencies.  Is that OK with you?

If the driver depends on OF, then it should be specified in the Kconfig.

> It doesn't hurt to still add a check for np == NULL and bail out, though.

If we're depending on OF, then there's no real need to do this.

And of_property_read_bool() does a NULL check on np anyway, so not
checking is still okay.  I was just wondering if not checking was a
mistake or intentional?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Dmitry Torokhov]

On Wednesday, August 27, 2014 02:39:36 PM Lee Jones wrote:
> On Tue, 26 Aug 2014, Chris Zhong wrote:
> > +
> > +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);
> 
> My OCD senses are tingling.  Either have a blank line above the calls
> to MODULE_DEVICE_TABLE() or don't.

Wait, don't we also need sentinel entries in both these tables?

Thanks,
Dmitry

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Heiko Stübner]

Hi Chris,

Am Dienstag, 26. August 2014, 22:14:04 schrieb 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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Signed-off-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> 
> ---

when testing this with Dougs dts integration I got irq errors like the
following:

INT_STS_REG1: 0x0
INT_STS_MSK_REG1: 0x0
INT_STS_REG2: 0x1
INT_STS_MSK_REG2: 0x0
random: nonblocking pool is initialized
irq 192: nobody cared (try booting with the "irqpoll" option)
CPU: 0 PID: 40 Comm: irq/192-rk808 Not tainted 3.17.0-rc1+ #1015
[<c001461c>] (unwind_backtrace) from [<c0011190>] (show_stack+0x10/0x14)
[<c0011190>] (show_stack) from [<c03e3e04>] (dump_stack+0x6c/0x84)
[<c03e3e04>] (dump_stack) from [<c00508e4>] (__report_bad_irq+0x28/0xb8)
[<c00508e4>] (__report_bad_irq) from [<c0050de4>] (note_interrupt+0x1e8/0x28c)
[<c0050de4>] (note_interrupt) from [<c004ebdc>] (handle_irq_event_percpu+0x104/0x120)
[<c004ebdc>] (handle_irq_event_percpu) from [<c004ec3c>] (handle_irq_event+0x44/0x64)
[<c004ec3c>] (handle_irq_event) from [<c0051910>] (handle_level_irq+0xd4/0x11c)
[<c0051910>] (handle_level_irq) from [<c004e5a8>] (generic_handle_irq+0x20/0x30)
[<c004e5a8>] (generic_handle_irq) from [<c01ba490>] (rockchip_irq_demux+0x190/0x228)
[<c01ba490>] (rockchip_irq_demux) from [<c004e5a8>] (generic_handle_irq+0x20/0x30)
[<c004e5a8>] (generic_handle_irq) from [<c000f060>] (handle_IRQ+0x68/0x90)
[<c000f060>] (handle_IRQ) from [<c000858c>] (gic_handle_irq+0x3c/0x60)
[<c000858c>] (gic_handle_irq) from [<c0011bc0>] (__irq_svc+0x40/0x50)
Exception stack(0xee31bed8 to 0xee31bf20)
bec0:                                                       ee093518 f005e000
bee0: 00000010 000093e9 ee1fe880 ee2f98c0 ee1fe880 ee2f98e0 c004f6f0 00000000
bf00: 00000000 00000000 c03eb6d0 ee31bf20 c00522dc c004f628 60000113 ffffffff
[<c0011bc0>] (__irq_svc) from [<c004f628>] (irq_finalize_oneshot+0xd4/0xf0)
[<c004f628>] (irq_finalize_oneshot) from [<c004f71c>] (irq_thread_fn+0x2c/0x34)
[<c004f71c>] (irq_thread_fn) from [<c004f844>] (irq_thread+0xc4/0x148)
[<c004f844>] (irq_thread) from [<c0035524>] (kthread+0xdc/0xf0)
[<c0035524>] (kthread) from [<c000e7f8>] (ret_from_fork+0x14/0x3c)
handlers:
[<c004ec5c>] irq_default_primary_handler threaded [<c021cc7c>] regmap_irq_thread
Disabling IRQ #192


As you can see there was already a PLUG_IN_INT pending, which stalled the
system till the irq got deactivated after 100000 iterations:

           CPU0       
[...]
 92:     600231       GIC  92  ff650000.i2c
192:     100001  rockchip_gpio_irq   4  rk808


I fixed it up with the following:

------------ 8< ---------------
diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
index f0d6518..1c25be7 100644
--- a/drivers/mfd/rk808.c
+++ b/drivers/mfd/rk808.c
@@ -61,8 +61,14 @@ static const struct rk808_reg_data pre_init_reg[] = {
 	{ RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK,  BUCK_ILMIN_200MA },
 	{ RK808_VB_MON_REG,       MASK_ALL,         VB_LO_ACT |
 						    VB_LO_SEL_3500MV },
-	{ RK808_INT_STS_REG1,     MASK_NONE,        0 },
-	{ RK808_INT_STS_REG2,     MASK_NONE,        0 },
+
+	/* ack any pending interrupts */
+	{ RK808_INT_STS_REG1,     INT_STS_REG1_MASK, INT_STS_REG1_MASK },
+	{ RK808_INT_STS_REG2,     INT_STS_REG2_MASK, INT_STS_REG2_MASK },
+
+	/* mask all interrupts */
+	{ RK808_INT_STS_MSK_REG1, INT_STS_REG1_MASK, INT_STS_REG1_MASK },
+	{ RK808_INT_STS_MSK_REG2, INT_STS_REG2_MASK, INT_STS_REG2_MASK },
 };
 
 static const struct regmap_irq rk808_irqs[] = {
diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
index 7af1952..8f8e48c 100644
--- a/include/linux/mfd/rk808.h
+++ b/include/linux/mfd/rk808.h
@@ -156,6 +156,8 @@ enum rk808_reg {
 #define BUCK2_RATE_MASK		(3 << 3)
 #define MASK_ALL	0xff
 #define MASK_NONE	0
+#define INT_STS_REG1_MASK	0x7f
+#define INT_STS_REG2_MASK	0x3
 
 #define SWITCH2_EN	BIT(6)
 #define SWITCH1_EN	BIT(5)
------------ 8< ---------------

On startup I guess all pending irqs should be acked and also masked, as we normally
don't want to handle interrupts that happened way in the past and they
will get unmasked on their own, when one of them gets requested by the rtc
for example.


Heiko

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

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Doug Anderson]

Heiko,

On Wed, Aug 27, 2014 at 12:53 PM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> Hi Chris,
>
> Am Dienstag, 26. August 2014, 22:14:04 schrieb 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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>> Signed-off-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
>>
>> ---
>
> when testing this with Dougs dts integration I got irq errors like the
> following:
>
> INT_STS_REG1: 0x0
> INT_STS_MSK_REG1: 0x0
> INT_STS_REG2: 0x1
> INT_STS_MSK_REG2: 0x0
> random: nonblocking pool is initialized
> irq 192: nobody cared (try booting with the "irqpoll" option)
> CPU: 0 PID: 40 Comm: irq/192-rk808 Not tainted 3.17.0-rc1+ #1015
> [<c001461c>] (unwind_backtrace) from [<c0011190>] (show_stack+0x10/0x14)
> [<c0011190>] (show_stack) from [<c03e3e04>] (dump_stack+0x6c/0x84)
> [<c03e3e04>] (dump_stack) from [<c00508e4>] (__report_bad_irq+0x28/0xb8)
> [<c00508e4>] (__report_bad_irq) from [<c0050de4>] (note_interrupt+0x1e8/0x28c)
> [<c0050de4>] (note_interrupt) from [<c004ebdc>] (handle_irq_event_percpu+0x104/0x120)
> [<c004ebdc>] (handle_irq_event_percpu) from [<c004ec3c>] (handle_irq_event+0x44/0x64)
> [<c004ec3c>] (handle_irq_event) from [<c0051910>] (handle_level_irq+0xd4/0x11c)
> [<c0051910>] (handle_level_irq) from [<c004e5a8>] (generic_handle_irq+0x20/0x30)
> [<c004e5a8>] (generic_handle_irq) from [<c01ba490>] (rockchip_irq_demux+0x190/0x228)
> [<c01ba490>] (rockchip_irq_demux) from [<c004e5a8>] (generic_handle_irq+0x20/0x30)
> [<c004e5a8>] (generic_handle_irq) from [<c000f060>] (handle_IRQ+0x68/0x90)
> [<c000f060>] (handle_IRQ) from [<c000858c>] (gic_handle_irq+0x3c/0x60)
> [<c000858c>] (gic_handle_irq) from [<c0011bc0>] (__irq_svc+0x40/0x50)
> Exception stack(0xee31bed8 to 0xee31bf20)
> bec0:                                                       ee093518 f005e000
> bee0: 00000010 000093e9 ee1fe880 ee2f98c0 ee1fe880 ee2f98e0 c004f6f0 00000000
> bf00: 00000000 00000000 c03eb6d0 ee31bf20 c00522dc c004f628 60000113 ffffffff
> [<c0011bc0>] (__irq_svc) from [<c004f628>] (irq_finalize_oneshot+0xd4/0xf0)
> [<c004f628>] (irq_finalize_oneshot) from [<c004f71c>] (irq_thread_fn+0x2c/0x34)
> [<c004f71c>] (irq_thread_fn) from [<c004f844>] (irq_thread+0xc4/0x148)
> [<c004f844>] (irq_thread) from [<c0035524>] (kthread+0xdc/0xf0)
> [<c0035524>] (kthread) from [<c000e7f8>] (ret_from_fork+0x14/0x3c)
> handlers:
> [<c004ec5c>] irq_default_primary_handler threaded [<c021cc7c>] regmap_irq_thread
> Disabling IRQ #192
>
>
> As you can see there was already a PLUG_IN_INT pending, which stalled the
> system till the irq got deactivated after 100000 iterations:
>
>            CPU0
> [...]
>  92:     600231       GIC  92  ff650000.i2c
> 192:     100001  rockchip_gpio_irq   4  rk808
>
>
> I fixed it up with the following:
>
> ------------ 8< ---------------
> diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> index f0d6518..1c25be7 100644
> --- a/drivers/mfd/rk808.c
> +++ b/drivers/mfd/rk808.c
> @@ -61,8 +61,14 @@ static const struct rk808_reg_data pre_init_reg[] = {
>         { RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK,  BUCK_ILMIN_200MA },
>         { RK808_VB_MON_REG,       MASK_ALL,         VB_LO_ACT |
>                                                     VB_LO_SEL_3500MV },
> -       { RK808_INT_STS_REG1,     MASK_NONE,        0 },
> -       { RK808_INT_STS_REG2,     MASK_NONE,        0 },
> +
> +       /* ack any pending interrupts */
> +       { RK808_INT_STS_REG1,     INT_STS_REG1_MASK, INT_STS_REG1_MASK },
> +       { RK808_INT_STS_REG2,     INT_STS_REG2_MASK, INT_STS_REG2_MASK },
> +
> +       /* mask all interrupts */
> +       { RK808_INT_STS_MSK_REG1, INT_STS_REG1_MASK, INT_STS_REG1_MASK },
> +       { RK808_INT_STS_MSK_REG2, INT_STS_REG2_MASK, INT_STS_REG2_MASK },
>  };
>
>  static const struct regmap_irq rk808_irqs[] = {
> diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
> index 7af1952..8f8e48c 100644
> --- a/include/linux/mfd/rk808.h
> +++ b/include/linux/mfd/rk808.h
> @@ -156,6 +156,8 @@ enum rk808_reg {
>  #define BUCK2_RATE_MASK                (3 << 3)
>  #define MASK_ALL       0xff
>  #define MASK_NONE      0
> +#define INT_STS_REG1_MASK      0x7f
> +#define INT_STS_REG2_MASK      0x3
>
>  #define SWITCH2_EN     BIT(6)
>  #define SWITCH1_EN     BIT(5)
> ------------ 8< ---------------
>
> On startup I guess all pending irqs should be acked and also masked, as we normally
> don't want to handle interrupts that happened way in the past and they
> will get unmasked on their own, when one of them gets requested by the rtc
> for example.
>
>
> Heiko

Please see <https://chromium-review.googlesource.com/214241>  I asked
Chris to squash this into his next revision.

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

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Doug Anderson]

Dmitry,

On Wed, Aug 27, 2014 at 10:21 AM, Dmitry Torokhov <dtor-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org> wrote:
> On Wednesday, August 27, 2014 02:39:36 PM Lee Jones wrote:
>> On Tue, 26 Aug 2014, Chris Zhong wrote:
>> > +
>> > +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);
>>
>> My OCD senses are tingling.  Either have a blank line above the calls
>> to MODULE_DEVICE_TABLE() or don't.
>
> Wait, don't we also need sentinel entries in both these tables?

Yes!  Good catch.  Last I reviewed the code it was there...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Heiko Stübner]

Am Mittwoch, 27. August 2014, 12:59:35 schrieb Doug Anderson:
> Heiko,
> 
> On Wed, Aug 27, 2014 at 12:53 PM, Heiko Stübner <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org> wrote:
> > Hi Chris,
> > 
> > Am Dienstag, 26. August 2014, 22:14:04 schrieb 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-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >> Signed-off-by: Zhang Qing <zhangqing-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> >> 
> >> ---
> > 
> > when testing this with Dougs dts integration I got irq errors like the
> > following:
> > 
> > INT_STS_REG1: 0x0
> > INT_STS_MSK_REG1: 0x0
> > INT_STS_REG2: 0x1
> > INT_STS_MSK_REG2: 0x0
> > random: nonblocking pool is initialized
> > irq 192: nobody cared (try booting with the "irqpoll" option)
> > CPU: 0 PID: 40 Comm: irq/192-rk808 Not tainted 3.17.0-rc1+ #1015
> > [<c001461c>] (unwind_backtrace) from [<c0011190>] (show_stack+0x10/0x14)
> > [<c0011190>] (show_stack) from [<c03e3e04>] (dump_stack+0x6c/0x84)
> > [<c03e3e04>] (dump_stack) from [<c00508e4>] (__report_bad_irq+0x28/0xb8)
> > [<c00508e4>] (__report_bad_irq) from [<c0050de4>]
> > (note_interrupt+0x1e8/0x28c) [<c0050de4>] (note_interrupt) from
> > [<c004ebdc>] (handle_irq_event_percpu+0x104/0x120) [<c004ebdc>]
> > (handle_irq_event_percpu) from [<c004ec3c>] (handle_irq_event+0x44/0x64)
> > [<c004ec3c>] (handle_irq_event) from [<c0051910>]
> > (handle_level_irq+0xd4/0x11c) [<c0051910>] (handle_level_irq) from
> > [<c004e5a8>] (generic_handle_irq+0x20/0x30) [<c004e5a8>]
> > (generic_handle_irq) from [<c01ba490>] (rockchip_irq_demux+0x190/0x228)
> > [<c01ba490>] (rockchip_irq_demux) from [<c004e5a8>]
> > (generic_handle_irq+0x20/0x30) [<c004e5a8>] (generic_handle_irq) from
> > [<c000f060>] (handle_IRQ+0x68/0x90) [<c000f060>] (handle_IRQ) from
> > [<c000858c>] (gic_handle_irq+0x3c/0x60) [<c000858c>] (gic_handle_irq)
> > from [<c0011bc0>] (__irq_svc+0x40/0x50) Exception stack(0xee31bed8 to
> > 0xee31bf20)
> > bec0:                                                       ee093518
> > f005e000 bee0: 00000010 000093e9 ee1fe880 ee2f98c0 ee1fe880 ee2f98e0
> > c004f6f0 00000000 bf00: 00000000 00000000 c03eb6d0 ee31bf20 c00522dc
> > c004f628 60000113 ffffffff [<c0011bc0>] (__irq_svc) from [<c004f628>]
> > (irq_finalize_oneshot+0xd4/0xf0) [<c004f628>] (irq_finalize_oneshot) from
> > [<c004f71c>] (irq_thread_fn+0x2c/0x34) [<c004f71c>] (irq_thread_fn) from
> > [<c004f844>] (irq_thread+0xc4/0x148) [<c004f844>] (irq_thread) from
> > [<c0035524>] (kthread+0xdc/0xf0)
> > [<c0035524>] (kthread) from [<c000e7f8>] (ret_from_fork+0x14/0x3c)
> > handlers:
> > [<c004ec5c>] irq_default_primary_handler threaded [<c021cc7c>]
> > regmap_irq_thread Disabling IRQ #192
> > 
> > 
> > As you can see there was already a PLUG_IN_INT pending, which stalled the
> > 
> > system till the irq got deactivated after 100000 iterations:
> >            CPU0
> > 
> > [...]
> > 
> >  92:     600231       GIC  92  ff650000.i2c
> > 
> > 192:     100001  rockchip_gpio_irq   4  rk808
> > 
> > 
> > I fixed it up with the following:
> > 
> > ------------ 8< ---------------
> > diff --git a/drivers/mfd/rk808.c b/drivers/mfd/rk808.c
> > index f0d6518..1c25be7 100644
> > --- a/drivers/mfd/rk808.c
> > +++ b/drivers/mfd/rk808.c
> > @@ -61,8 +61,14 @@ static const struct rk808_reg_data pre_init_reg[] = {
> > 
> >         { RK808_BUCK2_CONFIG_REG, BUCK2_RATE_MASK,  BUCK_ILMIN_200MA },
> >         { RK808_VB_MON_REG,       MASK_ALL,         VB_LO_ACT |
> >         
> >                                                     VB_LO_SEL_3500MV },
> > 
> > -       { RK808_INT_STS_REG1,     MASK_NONE,        0 },
> > -       { RK808_INT_STS_REG2,     MASK_NONE,        0 },
> > +
> > +       /* ack any pending interrupts */
> > +       { RK808_INT_STS_REG1,     INT_STS_REG1_MASK, INT_STS_REG1_MASK },
> > +       { RK808_INT_STS_REG2,     INT_STS_REG2_MASK, INT_STS_REG2_MASK },
> > +
> > +       /* mask all interrupts */
> > +       { RK808_INT_STS_MSK_REG1, INT_STS_REG1_MASK, INT_STS_REG1_MASK },
> > +       { RK808_INT_STS_MSK_REG2, INT_STS_REG2_MASK, INT_STS_REG2_MASK },
> > 
> >  };
> >  
> >  static const struct regmap_irq rk808_irqs[] = {
> > 
> > diff --git a/include/linux/mfd/rk808.h b/include/linux/mfd/rk808.h
> > index 7af1952..8f8e48c 100644
> > --- a/include/linux/mfd/rk808.h
> > +++ b/include/linux/mfd/rk808.h
> > @@ -156,6 +156,8 @@ enum rk808_reg {
> > 
> >  #define BUCK2_RATE_MASK                (3 << 3)
> >  #define MASK_ALL       0xff
> >  #define MASK_NONE      0
> > 
> > +#define INT_STS_REG1_MASK      0x7f
> > +#define INT_STS_REG2_MASK      0x3
> > 
> >  #define SWITCH2_EN     BIT(6)
> >  #define SWITCH1_EN     BIT(5)
> > 
> > ------------ 8< ---------------
> > 
> > On startup I guess all pending irqs should be acked and also masked, as we
> > normally don't want to handle interrupts that happened way in the past
> > and they will get unmasked on their own, when one of them gets requested
> > by the rtc for example.
> > 
> > 
> > Heiko
> 
> Please see <https://chromium-review.googlesource.com/214241>  I asked
> Chris to squash this into his next revision.

Letting regmap handle this, is of course even better :-)


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

Re: [PATCH v6 2/5] MFD: RK808: Add new mfd driver for RK808

[Lee Jones]

On Wed, 27 Aug 2014, Dmitry Torokhov wrote:

> On Wednesday, August 27, 2014 02:39:36 PM Lee Jones wrote:
> > On Tue, 26 Aug 2014, Chris Zhong wrote:
> > > +
> > > +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);
> > 
> > My OCD senses are tingling.  Either have a blank line above the calls
> > to MODULE_DEVICE_TABLE() or don't.
> 
> Wait, don't we also need sentinel entries in both these tables?

Good spot, yes please.

 { },

Will do fine.

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