All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] regulator: act8865: add PMIC driver
@ 2013-12-12  1:18 ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: broonie, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre, wenyou.yang

Hi Liam and Mark,

The patch set is to add act8865 PMIC driver.

The active-semi act8865 is designed as a PMIC for Atmel sama5d3x and at91sam9 series.
Its datasheet is available at: http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf.

The patches is based on the branch: for-next of git respository, 
	git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
and [PATCH] regulator: read low power states configuration from device tree from Vincent Palatin
	https://patchwork.kernel.org/patch/2833667/

Thanks.

Best Regards,
Wenyou Yang

Wenyou Yang (3):
  regulator: act8865: add PMIC(Power Management IC) driver
  regulator: act8865: add device tree binding doc
  ARM: dts: sama5d3xcm: add the regulator device node

 .../bindings/regulator/act8865-regulator.txt       |   58 +++
 arch/arm/boot/dts/sama5d3xcm.dtsi                  |   47 ++
 drivers/regulator/Kconfig                          |    7 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/act8865-regulator.c              |  516 ++++++++++++++++++++
 include/linux/regulator/act8865.h                  |   56 +++
 6 files changed, 685 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

-- 
1.7.9.5


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

* [PATCH 0/3] regulator: act8865: add PMIC driver
@ 2013-12-12  1:18 ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: devicetree, vpalatin, linux-doc, nicolas.ferre, linux-kernel,
	rob.herring, wenyou.yang, broonie, grant.likely, plagnioj,
	linux-arm-kernel

Hi Liam and Mark,

The patch set is to add act8865 PMIC driver.

The active-semi act8865 is designed as a PMIC for Atmel sama5d3x and at91sam9 series.
Its datasheet is available at: http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf.

The patches is based on the branch: for-next of git respository, 
	git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
and [PATCH] regulator: read low power states configuration from device tree from Vincent Palatin
	https://patchwork.kernel.org/patch/2833667/

Thanks.

Best Regards,
Wenyou Yang

Wenyou Yang (3):
  regulator: act8865: add PMIC(Power Management IC) driver
  regulator: act8865: add device tree binding doc
  ARM: dts: sama5d3xcm: add the regulator device node

 .../bindings/regulator/act8865-regulator.txt       |   58 +++
 arch/arm/boot/dts/sama5d3xcm.dtsi                  |   47 ++
 drivers/regulator/Kconfig                          |    7 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/act8865-regulator.c              |  516 ++++++++++++++++++++
 include/linux/regulator/act8865.h                  |   56 +++
 6 files changed, 685 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

-- 
1.7.9.5

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

* [PATCH 0/3] regulator: act8865: add PMIC driver
@ 2013-12-12  1:18 ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Liam and Mark,

The patch set is to add act8865 PMIC driver.

The active-semi act8865 is designed as a PMIC for Atmel sama5d3x and at91sam9 series.
Its datasheet is available at: http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf.

The patches is based on the branch: for-next of git respository, 
	git://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git
and [PATCH] regulator: read low power states configuration from device tree from Vincent Palatin
	https://patchwork.kernel.org/patch/2833667/

Thanks.

Best Regards,
Wenyou Yang

Wenyou Yang (3):
  regulator: act8865: add PMIC(Power Management IC) driver
  regulator: act8865: add device tree binding doc
  ARM: dts: sama5d3xcm: add the regulator device node

 .../bindings/regulator/act8865-regulator.txt       |   58 +++
 arch/arm/boot/dts/sama5d3xcm.dtsi                  |   47 ++
 drivers/regulator/Kconfig                          |    7 +
 drivers/regulator/Makefile                         |    1 +
 drivers/regulator/act8865-regulator.c              |  516 ++++++++++++++++++++
 include/linux/regulator/act8865.h                  |   56 +++
 6 files changed, 685 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

-- 
1.7.9.5

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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
  2013-12-12  1:18 ` Wenyou Yang
  (?)
@ 2013-12-12  1:18   ` Wenyou Yang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: broonie, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre, wenyou.yang

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 drivers/regulator/Kconfig             |    7 +
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/act8865-regulator.c |  516 +++++++++++++++++++++++++++++++++
 include/linux/regulator/act8865.h     |   56 ++++
 4 files changed, 580 insertions(+)
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..e13bcf1 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -577,5 +577,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_ACT8865
+	tristate "Active-semi act8865 voltage regulator"
+	depends on I2C
+	help
+	  This driver controls a active-semi act8865 voltage output
+	  regulator via I2C bus.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..8bc485f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_ACT8865) += act8865-regulator.o
 
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
new file mode 100644
index 0000000..d616a28
--- /dev/null
+++ b/drivers/regulator/act8865-regulator.c
@@ -0,0 +1,516 @@
+/*
+ * act8865-regulator.c - Voltage regulation for the active-semi ACT8865
+ * http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf
+ *
+ * Copyright (C) 2013 Atmel Corporation
+ * Wenyou Yang <wenyou.yang@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/act8865.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>
+
+/*
+ * ACT8865 Global Register Map
+ */
+#define	ACT8865_SYS_MODE	0x00
+#define ACT8865_SYS_CTRL	0x01
+#define ACT8865_REG1_VSET1	0x20
+#define	ACT8865_REG1_VSET2	0x21
+#define	ACT8865_REG1_CTRL	0x22
+#define	ACT8865_REG2_VSET1	0x30
+#define	ACT8865_REG2_VSET2	0x31
+#define	ACT8865_REG2_CTRL	0x32
+#define	ACT8865_REG3_VSET1	0x40
+#define	ACT8865_REG3_VSET2	0x41
+#define	ACT8865_REG3_CTRL	0x42
+#define	ACT8865_REG4_VSET	0x50
+#define	ACT8865_REG4_CTRL	0x51
+#define	ACT8865_REG5_VSET	0x54
+#define	ACT8865_REG5_CTRL	0x55
+#define	ACT8865_REG6_VSET	0x60
+#define	ACT8865_REG6_CTRL	0x61
+#define	ACT8865_REG7_VSET	0x64
+#define	ACT8865_REG7_CTRL	0x65
+
+struct act8865_data {
+	u8	vsel_is_low:1;
+	struct i2c_client	*client;
+	struct regulator_dev	*rdev[];
+};
+
+/* ACt8865 voltage table */
+static const u32 act8865_voltages_table[] = {
+	600000,		625000,		650000,		675000,
+	700000,		725000,		750000,		775000,
+	800000,		825000,		850000,		875000,
+	900000,		925000,		950000,		975000,
+	1000000,	1025000,	1050000,	1075000,
+	1100000,	1125000,	1150000,	1175000,
+	1200000,	1250000,	1300000,	1350000,
+	1400000,	1450000,	1500000,	1550000,
+	1600000,	1650000,	1700000,	1750000,
+	1800000,	1850000,	1900000,	1950000,
+	2000000,	2050000,	2010000,	2150000,
+	2200000,	2250000,	2300000,	2350000,
+	2400000,	2500000,	2600000,	2700000,
+	2800000,	2900000,	3000000,	3100000,
+	3200000,	3300000,	3400000,	3500000,
+	3600000,	3700000,	3800000,	3900000,
+};
+
+static int act8865_read_reg(struct act8865_data *act8865, u8 reg)
+{
+	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
+	if (ret > 0)
+		ret &= 0xff;
+
+	return ret;
+}
+
+static int act8865_write_reg(struct act8865_data *act8865, u8 reg, u8 value)
+{
+	return i2c_smbus_write_byte_data(act8865->client, reg, value);
+}
+
+/*
+ * operation functions
+ */
+
+static u32 act8865_ctrl_reg_addr(int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		reg = ACT8865_REG1_CTRL;
+		break;
+
+	case ACT8865_ID_REG2:
+		reg = ACT8865_REG2_CTRL;
+		break;
+
+	case ACT8865_ID_REG3:
+		reg = ACT8865_REG3_CTRL;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_CTRL;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_CTRL;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_CTRL;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_CTRL;
+		break;
+	};
+
+	return reg;
+}
+
+static u32 act8865_vset_reg_addr(struct act8865_data *act8865, int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG1_VSET1;
+		else
+			reg = ACT8865_REG1_VSET2;
+		break;
+
+	case ACT8865_ID_REG2:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG2_VSET1;
+		else
+			reg = ACT8865_REG2_VSET2;
+		break;
+
+	case ACT8865_ID_REG3:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG3_VSET1;
+		else
+			reg = ACT8865_REG3_VSET2;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_VSET;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_VSET;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_VSET;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_VSET;
+		break;
+	};
+
+	return reg;
+}
+
+static int act88665_is_enabled(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+
+	return (act8865_read_reg(act8865, reg) & mask) == mask;
+}
+
+static int act8865_enable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val |= mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_disable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val &= ~mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_read_reg(act8865, reg);
+}
+
+static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32 selector)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static int act8865_set_suspend_voltage(struct regulator_dev *rdev, int uV)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+	u32	selector;
+
+	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
+
+	selector = regulator_map_voltage_iterate(rdev, uV, uV);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static struct regulator_ops act8865_ops = {
+	.list_voltage		= regulator_list_voltage_table,
+	.set_voltage_sel	= act8865_set_voltage_sel,
+	.get_voltage_sel	= act8865_get_voltage_sel,
+	.enable			= act8865_enable,
+	.disable		= act8865_disable,
+	.is_enabled		= act88665_is_enabled,
+	.set_suspend_voltage	= act8865_set_suspend_voltage,
+	.set_suspend_enable	= act8865_enable,
+	.set_suspend_disable	= act8865_disable,
+};
+
+static const struct regulator_desc act8865_reg[] = {
+	{
+		.name = "DCDC_REG1",
+		.id = ACT8865_ID_REG1,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG2",
+		.id = ACT8865_ID_REG2,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG3",
+		.id = ACT8865_ID_REG3,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG4",
+		.id = ACT8865_ID_REG4,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG5",
+		.id = ACT8865_ID_REG5,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG6",
+		.id = ACT8865_ID_REG6,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG7",
+		.id = ACT8865_ID_REG7,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id act8865_dt_ids[] = {
+	{ .compatible = "active-semi,act8865" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, act8865_dt_ids);
+
+static int act8865_pdata_from_dt(struct device *dev,
+				 struct device_node **of_node,
+				 struct act8865_platform_data *pdata)
+{
+	int matched, i;
+	struct device_node *np;
+	struct act8865_regulator_data *regulator;
+	struct of_regulator_match rmatch[ARRAY_SIZE(act8865_reg)];
+
+	if (of_find_property(dev->of_node, "vsel-state-low", NULL))
+		pdata->vsel_is_low = 1;
+
+	np = of_find_node_by_name(dev->of_node, "regulators");
+	if (!np) {
+		dev_err(dev, "missing 'regulators' subnode in DT\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(rmatch); i++)
+		rmatch[i].name = act8865_reg[i].name;
+
+	matched = of_regulator_match(dev, np, rmatch, ARRAY_SIZE(rmatch));
+	if (matched <= 0)
+		return matched;
+
+	pdata->regulators = devm_kzalloc(dev,
+				sizeof(struct act8865_regulator_data) * matched,
+				GFP_KERNEL);
+	if (!pdata->regulators) {
+		dev_err(dev, "%s: failed to allocate act8865 registor\n",
+						__func__);
+		return -ENOMEM;
+	}
+
+	pdata->num_regulators = matched;
+	regulator = pdata->regulators;
+
+	for (i = 0; i < matched; i++) {
+		regulator->id = i;
+		regulator->name = rmatch[i].name;
+		regulator->platform_data = rmatch[i].init_data;
+		of_node[i] = rmatch[i].of_node;
+		regulator++;
+	}
+
+	return 0;
+}
+
+#else
+
+static inline int act8865_pdata_from_dt(struct device *dev,
+					struct device_node **of_node,
+					struct act8865_platform_data *pdata)
+{
+	return 0;
+}
+#endif
+
+static int act8865_pmu_probe(struct i2c_client *client,
+			   const struct i2c_device_id *i2c_id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct regulator_dev **rdev;
+	struct device *dev = &client->dev;
+	struct act8865_platform_data *pdata = dev_get_platdata(dev);
+	struct regulator_config config = { };
+	struct act8865_data *act8865;
+	int i, id;
+	int ret = -EINVAL;
+	struct device_node *of_node[ACT8865_REG_NUM];
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		return -EIO;
+
+	if (dev->of_node && !pdata) {
+		const struct of_device_id *id;
+		struct act8865_platform_data pdata_of;
+
+		id = of_match_device(of_match_ptr(act8865_dt_ids), dev);
+		if (!id)
+			return -ENODEV;
+
+		ret = act8865_pdata_from_dt(dev, of_node, &pdata_of);
+		if (ret < 0)
+			return ret;
+
+		pdata = &pdata_of;
+	} else {
+		memset(of_node, 0, sizeof(of_node));
+	}
+
+	if (pdata->num_regulators > ACT8865_REG_NUM) {
+		dev_err(dev, "Too many regulators found!\n");
+		return -EINVAL;
+	}
+
+	act8865 = devm_kzalloc(dev, sizeof(struct act8865_data) +
+			sizeof(struct regulator_dev *) * ACT8865_REG_NUM,
+			GFP_KERNEL);
+	if (!act8865)
+		return -ENOMEM;
+
+	act8865->client = client;
+	act8865->vsel_is_low = pdata->vsel_is_low;
+	rdev = act8865->rdev;
+
+	/* Finally register devices */
+	for (i = 0; i < pdata->num_regulators; i++) {
+
+		id = pdata->regulators[i].id;
+
+		config.dev = dev;
+		config.init_data = pdata->regulators[i].platform_data;
+		config.of_node = of_node[i];
+		config.driver_data = act8865;
+
+		rdev[i] = regulator_register(&act8865_reg[id], &config);
+		if (IS_ERR(rdev[i])) {
+			ret = PTR_ERR(rdev[i]);
+			dev_err(dev, "failed to register %s\n",
+				act8865_reg[id].name);
+			goto err_unregister;
+		}
+	}
+
+	i2c_set_clientdata(client, act8865);
+
+	return 0;
+
+err_unregister:
+	while (--i >= 0)
+		regulator_unregister(rdev[i]);
+
+	return ret;
+}
+
+static int act8865_pmu_remove(struct i2c_client *client)
+{
+	struct act8865_data *act8865 = i2c_get_clientdata(client);
+	int i;
+
+	for (i = 0; i < ACT8865_REG_NUM; i++)
+		regulator_unregister(act8865->rdev[i]);
+
+	return 0;
+}
+
+static const struct i2c_device_id act8865_ids[] = {
+	{ "act8865", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, act8865_ids);
+
+static struct i2c_driver act8865_pmu_driver = {
+	.probe		= act8865_pmu_probe,
+	.remove		= act8865_pmu_remove,
+	.driver		= {
+		.name	= "act8865",
+		.owner	= THIS_MODULE,
+	},
+	.id_table	= act8865_ids,
+};
+
+static int __init act8865_pmu_init(void)
+{
+	return i2c_add_driver(&act8865_pmu_driver);
+}
+subsys_initcall(act8865_pmu_init);
+
+static void __exit act8865_pmu_exit(void)
+{
+	i2c_del_driver(&act8865_pmu_driver);
+}
+module_exit(act8865_pmu_exit);
+
+MODULE_DESCRIPTION("active-semi act8865 voltage regulator driver");
+MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/act8865.h b/include/linux/regulator/act8865.h
new file mode 100644
index 0000000..202544f
--- /dev/null
+++ b/include/linux/regulator/act8865.h
@@ -0,0 +1,56 @@
+/*
+ * act8865.h  --  Voltage regulation for the active-semi act8865
+ *
+ * Copyright (C) 2013 Atmel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_ACT8865_H
+#define __LINUX_REGULATOR_ACT8865_H
+
+#include <linux/regulator/machine.h>
+
+enum {
+	ACT8865_ID_REG1,
+	ACT8865_ID_REG2,
+	ACT8865_ID_REG3,
+	ACT8865_ID_REG4,
+	ACT8865_ID_REG5,
+	ACT8865_ID_REG6,
+	ACT8865_ID_REG7,
+	ACT8865_REG_NUM,
+};
+
+/**
+ * act8865_regulator_data - regulator data
+ * @id: regulator id
+ * @name: regulator name
+ * @platform_data: regulator init data
+ */
+struct act8865_regulator_data {
+	int				id;
+	const char				*name;
+	struct regulator_init_data	*platform_data;
+};
+
+/**
+ * act8865_platform_data - platform data for act8865
+ * @num_regulators: number of regulators used
+ * @regulators: pointer to regulators used
+ * @vsel_is_low: VSEL pin, drive to logic low to select default output voltage,
+ *			drive to logic high to select secondary output voltage.
+ */
+struct act8865_platform_data {
+	int num_regulators;
+	struct act8865_regulator_data *regulators;
+	unsigned vsel_is_low:1;
+};
+#endif
-- 
1.7.9.5


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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: devicetree, vpalatin, linux-doc, nicolas.ferre, linux-kernel,
	rob.herring, wenyou.yang, broonie, grant.likely, plagnioj,
	linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 drivers/regulator/Kconfig             |    7 +
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/act8865-regulator.c |  516 +++++++++++++++++++++++++++++++++
 include/linux/regulator/act8865.h     |   56 ++++
 4 files changed, 580 insertions(+)
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..e13bcf1 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -577,5 +577,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_ACT8865
+	tristate "Active-semi act8865 voltage regulator"
+	depends on I2C
+	help
+	  This driver controls a active-semi act8865 voltage output
+	  regulator via I2C bus.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..8bc485f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_ACT8865) += act8865-regulator.o
 
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
new file mode 100644
index 0000000..d616a28
--- /dev/null
+++ b/drivers/regulator/act8865-regulator.c
@@ -0,0 +1,516 @@
+/*
+ * act8865-regulator.c - Voltage regulation for the active-semi ACT8865
+ * http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf
+ *
+ * Copyright (C) 2013 Atmel Corporation
+ * Wenyou Yang <wenyou.yang@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/act8865.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>
+
+/*
+ * ACT8865 Global Register Map
+ */
+#define	ACT8865_SYS_MODE	0x00
+#define ACT8865_SYS_CTRL	0x01
+#define ACT8865_REG1_VSET1	0x20
+#define	ACT8865_REG1_VSET2	0x21
+#define	ACT8865_REG1_CTRL	0x22
+#define	ACT8865_REG2_VSET1	0x30
+#define	ACT8865_REG2_VSET2	0x31
+#define	ACT8865_REG2_CTRL	0x32
+#define	ACT8865_REG3_VSET1	0x40
+#define	ACT8865_REG3_VSET2	0x41
+#define	ACT8865_REG3_CTRL	0x42
+#define	ACT8865_REG4_VSET	0x50
+#define	ACT8865_REG4_CTRL	0x51
+#define	ACT8865_REG5_VSET	0x54
+#define	ACT8865_REG5_CTRL	0x55
+#define	ACT8865_REG6_VSET	0x60
+#define	ACT8865_REG6_CTRL	0x61
+#define	ACT8865_REG7_VSET	0x64
+#define	ACT8865_REG7_CTRL	0x65
+
+struct act8865_data {
+	u8	vsel_is_low:1;
+	struct i2c_client	*client;
+	struct regulator_dev	*rdev[];
+};
+
+/* ACt8865 voltage table */
+static const u32 act8865_voltages_table[] = {
+	600000,		625000,		650000,		675000,
+	700000,		725000,		750000,		775000,
+	800000,		825000,		850000,		875000,
+	900000,		925000,		950000,		975000,
+	1000000,	1025000,	1050000,	1075000,
+	1100000,	1125000,	1150000,	1175000,
+	1200000,	1250000,	1300000,	1350000,
+	1400000,	1450000,	1500000,	1550000,
+	1600000,	1650000,	1700000,	1750000,
+	1800000,	1850000,	1900000,	1950000,
+	2000000,	2050000,	2010000,	2150000,
+	2200000,	2250000,	2300000,	2350000,
+	2400000,	2500000,	2600000,	2700000,
+	2800000,	2900000,	3000000,	3100000,
+	3200000,	3300000,	3400000,	3500000,
+	3600000,	3700000,	3800000,	3900000,
+};
+
+static int act8865_read_reg(struct act8865_data *act8865, u8 reg)
+{
+	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
+	if (ret > 0)
+		ret &= 0xff;
+
+	return ret;
+}
+
+static int act8865_write_reg(struct act8865_data *act8865, u8 reg, u8 value)
+{
+	return i2c_smbus_write_byte_data(act8865->client, reg, value);
+}
+
+/*
+ * operation functions
+ */
+
+static u32 act8865_ctrl_reg_addr(int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		reg = ACT8865_REG1_CTRL;
+		break;
+
+	case ACT8865_ID_REG2:
+		reg = ACT8865_REG2_CTRL;
+		break;
+
+	case ACT8865_ID_REG3:
+		reg = ACT8865_REG3_CTRL;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_CTRL;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_CTRL;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_CTRL;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_CTRL;
+		break;
+	};
+
+	return reg;
+}
+
+static u32 act8865_vset_reg_addr(struct act8865_data *act8865, int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG1_VSET1;
+		else
+			reg = ACT8865_REG1_VSET2;
+		break;
+
+	case ACT8865_ID_REG2:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG2_VSET1;
+		else
+			reg = ACT8865_REG2_VSET2;
+		break;
+
+	case ACT8865_ID_REG3:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG3_VSET1;
+		else
+			reg = ACT8865_REG3_VSET2;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_VSET;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_VSET;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_VSET;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_VSET;
+		break;
+	};
+
+	return reg;
+}
+
+static int act88665_is_enabled(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+
+	return (act8865_read_reg(act8865, reg) & mask) == mask;
+}
+
+static int act8865_enable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val |= mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_disable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val &= ~mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_read_reg(act8865, reg);
+}
+
+static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32 selector)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static int act8865_set_suspend_voltage(struct regulator_dev *rdev, int uV)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+	u32	selector;
+
+	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
+
+	selector = regulator_map_voltage_iterate(rdev, uV, uV);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static struct regulator_ops act8865_ops = {
+	.list_voltage		= regulator_list_voltage_table,
+	.set_voltage_sel	= act8865_set_voltage_sel,
+	.get_voltage_sel	= act8865_get_voltage_sel,
+	.enable			= act8865_enable,
+	.disable		= act8865_disable,
+	.is_enabled		= act88665_is_enabled,
+	.set_suspend_voltage	= act8865_set_suspend_voltage,
+	.set_suspend_enable	= act8865_enable,
+	.set_suspend_disable	= act8865_disable,
+};
+
+static const struct regulator_desc act8865_reg[] = {
+	{
+		.name = "DCDC_REG1",
+		.id = ACT8865_ID_REG1,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG2",
+		.id = ACT8865_ID_REG2,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG3",
+		.id = ACT8865_ID_REG3,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG4",
+		.id = ACT8865_ID_REG4,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG5",
+		.id = ACT8865_ID_REG5,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG6",
+		.id = ACT8865_ID_REG6,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG7",
+		.id = ACT8865_ID_REG7,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id act8865_dt_ids[] = {
+	{ .compatible = "active-semi,act8865" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, act8865_dt_ids);
+
+static int act8865_pdata_from_dt(struct device *dev,
+				 struct device_node **of_node,
+				 struct act8865_platform_data *pdata)
+{
+	int matched, i;
+	struct device_node *np;
+	struct act8865_regulator_data *regulator;
+	struct of_regulator_match rmatch[ARRAY_SIZE(act8865_reg)];
+
+	if (of_find_property(dev->of_node, "vsel-state-low", NULL))
+		pdata->vsel_is_low = 1;
+
+	np = of_find_node_by_name(dev->of_node, "regulators");
+	if (!np) {
+		dev_err(dev, "missing 'regulators' subnode in DT\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(rmatch); i++)
+		rmatch[i].name = act8865_reg[i].name;
+
+	matched = of_regulator_match(dev, np, rmatch, ARRAY_SIZE(rmatch));
+	if (matched <= 0)
+		return matched;
+
+	pdata->regulators = devm_kzalloc(dev,
+				sizeof(struct act8865_regulator_data) * matched,
+				GFP_KERNEL);
+	if (!pdata->regulators) {
+		dev_err(dev, "%s: failed to allocate act8865 registor\n",
+						__func__);
+		return -ENOMEM;
+	}
+
+	pdata->num_regulators = matched;
+	regulator = pdata->regulators;
+
+	for (i = 0; i < matched; i++) {
+		regulator->id = i;
+		regulator->name = rmatch[i].name;
+		regulator->platform_data = rmatch[i].init_data;
+		of_node[i] = rmatch[i].of_node;
+		regulator++;
+	}
+
+	return 0;
+}
+
+#else
+
+static inline int act8865_pdata_from_dt(struct device *dev,
+					struct device_node **of_node,
+					struct act8865_platform_data *pdata)
+{
+	return 0;
+}
+#endif
+
+static int act8865_pmu_probe(struct i2c_client *client,
+			   const struct i2c_device_id *i2c_id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct regulator_dev **rdev;
+	struct device *dev = &client->dev;
+	struct act8865_platform_data *pdata = dev_get_platdata(dev);
+	struct regulator_config config = { };
+	struct act8865_data *act8865;
+	int i, id;
+	int ret = -EINVAL;
+	struct device_node *of_node[ACT8865_REG_NUM];
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		return -EIO;
+
+	if (dev->of_node && !pdata) {
+		const struct of_device_id *id;
+		struct act8865_platform_data pdata_of;
+
+		id = of_match_device(of_match_ptr(act8865_dt_ids), dev);
+		if (!id)
+			return -ENODEV;
+
+		ret = act8865_pdata_from_dt(dev, of_node, &pdata_of);
+		if (ret < 0)
+			return ret;
+
+		pdata = &pdata_of;
+	} else {
+		memset(of_node, 0, sizeof(of_node));
+	}
+
+	if (pdata->num_regulators > ACT8865_REG_NUM) {
+		dev_err(dev, "Too many regulators found!\n");
+		return -EINVAL;
+	}
+
+	act8865 = devm_kzalloc(dev, sizeof(struct act8865_data) +
+			sizeof(struct regulator_dev *) * ACT8865_REG_NUM,
+			GFP_KERNEL);
+	if (!act8865)
+		return -ENOMEM;
+
+	act8865->client = client;
+	act8865->vsel_is_low = pdata->vsel_is_low;
+	rdev = act8865->rdev;
+
+	/* Finally register devices */
+	for (i = 0; i < pdata->num_regulators; i++) {
+
+		id = pdata->regulators[i].id;
+
+		config.dev = dev;
+		config.init_data = pdata->regulators[i].platform_data;
+		config.of_node = of_node[i];
+		config.driver_data = act8865;
+
+		rdev[i] = regulator_register(&act8865_reg[id], &config);
+		if (IS_ERR(rdev[i])) {
+			ret = PTR_ERR(rdev[i]);
+			dev_err(dev, "failed to register %s\n",
+				act8865_reg[id].name);
+			goto err_unregister;
+		}
+	}
+
+	i2c_set_clientdata(client, act8865);
+
+	return 0;
+
+err_unregister:
+	while (--i >= 0)
+		regulator_unregister(rdev[i]);
+
+	return ret;
+}
+
+static int act8865_pmu_remove(struct i2c_client *client)
+{
+	struct act8865_data *act8865 = i2c_get_clientdata(client);
+	int i;
+
+	for (i = 0; i < ACT8865_REG_NUM; i++)
+		regulator_unregister(act8865->rdev[i]);
+
+	return 0;
+}
+
+static const struct i2c_device_id act8865_ids[] = {
+	{ "act8865", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, act8865_ids);
+
+static struct i2c_driver act8865_pmu_driver = {
+	.probe		= act8865_pmu_probe,
+	.remove		= act8865_pmu_remove,
+	.driver		= {
+		.name	= "act8865",
+		.owner	= THIS_MODULE,
+	},
+	.id_table	= act8865_ids,
+};
+
+static int __init act8865_pmu_init(void)
+{
+	return i2c_add_driver(&act8865_pmu_driver);
+}
+subsys_initcall(act8865_pmu_init);
+
+static void __exit act8865_pmu_exit(void)
+{
+	i2c_del_driver(&act8865_pmu_driver);
+}
+module_exit(act8865_pmu_exit);
+
+MODULE_DESCRIPTION("active-semi act8865 voltage regulator driver");
+MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/act8865.h b/include/linux/regulator/act8865.h
new file mode 100644
index 0000000..202544f
--- /dev/null
+++ b/include/linux/regulator/act8865.h
@@ -0,0 +1,56 @@
+/*
+ * act8865.h  --  Voltage regulation for the active-semi act8865
+ *
+ * Copyright (C) 2013 Atmel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_ACT8865_H
+#define __LINUX_REGULATOR_ACT8865_H
+
+#include <linux/regulator/machine.h>
+
+enum {
+	ACT8865_ID_REG1,
+	ACT8865_ID_REG2,
+	ACT8865_ID_REG3,
+	ACT8865_ID_REG4,
+	ACT8865_ID_REG5,
+	ACT8865_ID_REG6,
+	ACT8865_ID_REG7,
+	ACT8865_REG_NUM,
+};
+
+/**
+ * act8865_regulator_data - regulator data
+ * @id: regulator id
+ * @name: regulator name
+ * @platform_data: regulator init data
+ */
+struct act8865_regulator_data {
+	int				id;
+	const char				*name;
+	struct regulator_init_data	*platform_data;
+};
+
+/**
+ * act8865_platform_data - platform data for act8865
+ * @num_regulators: number of regulators used
+ * @regulators: pointer to regulators used
+ * @vsel_is_low: VSEL pin, drive to logic low to select default output voltage,
+ *			drive to logic high to select secondary output voltage.
+ */
+struct act8865_platform_data {
+	int num_regulators;
+	struct act8865_regulator_data *regulators;
+	unsigned vsel_is_low:1;
+};
+#endif
-- 
1.7.9.5

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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 drivers/regulator/Kconfig             |    7 +
 drivers/regulator/Makefile            |    1 +
 drivers/regulator/act8865-regulator.c |  516 +++++++++++++++++++++++++++++++++
 include/linux/regulator/act8865.h     |   56 ++++
 4 files changed, 580 insertions(+)
 create mode 100644 drivers/regulator/act8865-regulator.c
 create mode 100644 include/linux/regulator/act8865.h

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index ce785f4..e13bcf1 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -577,5 +577,12 @@ config REGULATOR_WM8994
 	  This driver provides support for the voltage regulators on the
 	  WM8994 CODEC.
 
+config REGULATOR_ACT8865
+	tristate "Active-semi act8865 voltage regulator"
+	depends on I2C
+	help
+	  This driver controls a active-semi act8865 voltage output
+	  regulator via I2C bus.
+
 endif
 
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 01c597e..8bc485f 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -78,6 +78,7 @@ obj-$(CONFIG_REGULATOR_WM831X) += wm831x-ldo.o
 obj-$(CONFIG_REGULATOR_WM8350) += wm8350-regulator.o
 obj-$(CONFIG_REGULATOR_WM8400) += wm8400-regulator.o
 obj-$(CONFIG_REGULATOR_WM8994) += wm8994-regulator.o
+obj-$(CONFIG_REGULATOR_ACT8865) += act8865-regulator.o
 
 
 ccflags-$(CONFIG_REGULATOR_DEBUG) += -DDEBUG
diff --git a/drivers/regulator/act8865-regulator.c b/drivers/regulator/act8865-regulator.c
new file mode 100644
index 0000000..d616a28
--- /dev/null
+++ b/drivers/regulator/act8865-regulator.c
@@ -0,0 +1,516 @@
+/*
+ * act8865-regulator.c - Voltage regulation for the active-semi ACT8865
+ * http://www.active-semi.com/sheets/ACT8865_Datasheet.pdf
+ *
+ * Copyright (C) 2013 Atmel Corporation
+ * Wenyou Yang <wenyou.yang@atmel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/i2c.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/act8865.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/regulator/of_regulator.h>
+
+/*
+ * ACT8865 Global Register Map
+ */
+#define	ACT8865_SYS_MODE	0x00
+#define ACT8865_SYS_CTRL	0x01
+#define ACT8865_REG1_VSET1	0x20
+#define	ACT8865_REG1_VSET2	0x21
+#define	ACT8865_REG1_CTRL	0x22
+#define	ACT8865_REG2_VSET1	0x30
+#define	ACT8865_REG2_VSET2	0x31
+#define	ACT8865_REG2_CTRL	0x32
+#define	ACT8865_REG3_VSET1	0x40
+#define	ACT8865_REG3_VSET2	0x41
+#define	ACT8865_REG3_CTRL	0x42
+#define	ACT8865_REG4_VSET	0x50
+#define	ACT8865_REG4_CTRL	0x51
+#define	ACT8865_REG5_VSET	0x54
+#define	ACT8865_REG5_CTRL	0x55
+#define	ACT8865_REG6_VSET	0x60
+#define	ACT8865_REG6_CTRL	0x61
+#define	ACT8865_REG7_VSET	0x64
+#define	ACT8865_REG7_CTRL	0x65
+
+struct act8865_data {
+	u8	vsel_is_low:1;
+	struct i2c_client	*client;
+	struct regulator_dev	*rdev[];
+};
+
+/* ACt8865 voltage table */
+static const u32 act8865_voltages_table[] = {
+	600000,		625000,		650000,		675000,
+	700000,		725000,		750000,		775000,
+	800000,		825000,		850000,		875000,
+	900000,		925000,		950000,		975000,
+	1000000,	1025000,	1050000,	1075000,
+	1100000,	1125000,	1150000,	1175000,
+	1200000,	1250000,	1300000,	1350000,
+	1400000,	1450000,	1500000,	1550000,
+	1600000,	1650000,	1700000,	1750000,
+	1800000,	1850000,	1900000,	1950000,
+	2000000,	2050000,	2010000,	2150000,
+	2200000,	2250000,	2300000,	2350000,
+	2400000,	2500000,	2600000,	2700000,
+	2800000,	2900000,	3000000,	3100000,
+	3200000,	3300000,	3400000,	3500000,
+	3600000,	3700000,	3800000,	3900000,
+};
+
+static int act8865_read_reg(struct act8865_data *act8865, u8 reg)
+{
+	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
+	if (ret > 0)
+		ret &= 0xff;
+
+	return ret;
+}
+
+static int act8865_write_reg(struct act8865_data *act8865, u8 reg, u8 value)
+{
+	return i2c_smbus_write_byte_data(act8865->client, reg, value);
+}
+
+/*
+ * operation functions
+ */
+
+static u32 act8865_ctrl_reg_addr(int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		reg = ACT8865_REG1_CTRL;
+		break;
+
+	case ACT8865_ID_REG2:
+		reg = ACT8865_REG2_CTRL;
+		break;
+
+	case ACT8865_ID_REG3:
+		reg = ACT8865_REG3_CTRL;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_CTRL;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_CTRL;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_CTRL;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_CTRL;
+		break;
+	};
+
+	return reg;
+}
+
+static u32 act8865_vset_reg_addr(struct act8865_data *act8865, int id)
+{
+	u32	reg = 0;
+
+	switch (id) {
+	case ACT8865_ID_REG1:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG1_VSET1;
+		else
+			reg = ACT8865_REG1_VSET2;
+		break;
+
+	case ACT8865_ID_REG2:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG2_VSET1;
+		else
+			reg = ACT8865_REG2_VSET2;
+		break;
+
+	case ACT8865_ID_REG3:
+		if (act8865->vsel_is_low)
+			reg = ACT8865_REG3_VSET1;
+		else
+			reg = ACT8865_REG3_VSET2;
+		break;
+
+	case ACT8865_ID_REG4:
+		reg = ACT8865_REG4_VSET;
+		break;
+
+	case ACT8865_ID_REG5:
+		reg = ACT8865_REG5_VSET;
+		break;
+
+	case ACT8865_ID_REG6:
+		reg = ACT8865_REG6_VSET;
+		break;
+
+	case ACT8865_ID_REG7:
+		reg = ACT8865_REG7_VSET;
+		break;
+	};
+
+	return reg;
+}
+
+static int act88665_is_enabled(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+
+	return (act8865_read_reg(act8865, reg) & mask) == mask;
+}
+
+static int act8865_enable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val |= mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_disable(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg;
+	u8	val;
+	u8	mask = 0x80;
+
+	reg = act8865_ctrl_reg_addr(id);
+	val = act8865_read_reg(act8865, reg);
+	val &= ~mask;
+
+	return act8865_write_reg(act8865, reg, val);
+}
+
+static int act8865_get_voltage_sel(struct regulator_dev *rdev)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_read_reg(act8865, reg);
+}
+
+static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32 selector)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static int act8865_set_suspend_voltage(struct regulator_dev *rdev, int uV)
+{
+	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
+	int id = rdev_get_id(rdev);
+	u32	reg = act8865_vset_reg_addr(act8865, id);
+	u32	selector;
+
+	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
+
+	selector = regulator_map_voltage_iterate(rdev, uV, uV);
+
+	return act8865_write_reg(act8865, reg, selector);
+}
+
+static struct regulator_ops act8865_ops = {
+	.list_voltage		= regulator_list_voltage_table,
+	.set_voltage_sel	= act8865_set_voltage_sel,
+	.get_voltage_sel	= act8865_get_voltage_sel,
+	.enable			= act8865_enable,
+	.disable		= act8865_disable,
+	.is_enabled		= act88665_is_enabled,
+	.set_suspend_voltage	= act8865_set_suspend_voltage,
+	.set_suspend_enable	= act8865_enable,
+	.set_suspend_disable	= act8865_disable,
+};
+
+static const struct regulator_desc act8865_reg[] = {
+	{
+		.name = "DCDC_REG1",
+		.id = ACT8865_ID_REG1,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG2",
+		.id = ACT8865_ID_REG2,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "DCDC_REG3",
+		.id = ACT8865_ID_REG3,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG4",
+		.id = ACT8865_ID_REG4,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG5",
+		.id = ACT8865_ID_REG5,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG6",
+		.id = ACT8865_ID_REG6,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+	{
+		.name = "LDO_REG7",
+		.id = ACT8865_ID_REG7,
+		.ops = &act8865_ops,
+		.type = REGULATOR_VOLTAGE,
+		.n_voltages = ARRAY_SIZE(act8865_voltages_table),
+		.volt_table = act8865_voltages_table,
+		.owner = THIS_MODULE,
+	},
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id act8865_dt_ids[] = {
+	{ .compatible = "active-semi,act8865" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, act8865_dt_ids);
+
+static int act8865_pdata_from_dt(struct device *dev,
+				 struct device_node **of_node,
+				 struct act8865_platform_data *pdata)
+{
+	int matched, i;
+	struct device_node *np;
+	struct act8865_regulator_data *regulator;
+	struct of_regulator_match rmatch[ARRAY_SIZE(act8865_reg)];
+
+	if (of_find_property(dev->of_node, "vsel-state-low", NULL))
+		pdata->vsel_is_low = 1;
+
+	np = of_find_node_by_name(dev->of_node, "regulators");
+	if (!np) {
+		dev_err(dev, "missing 'regulators' subnode in DT\n");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(rmatch); i++)
+		rmatch[i].name = act8865_reg[i].name;
+
+	matched = of_regulator_match(dev, np, rmatch, ARRAY_SIZE(rmatch));
+	if (matched <= 0)
+		return matched;
+
+	pdata->regulators = devm_kzalloc(dev,
+				sizeof(struct act8865_regulator_data) * matched,
+				GFP_KERNEL);
+	if (!pdata->regulators) {
+		dev_err(dev, "%s: failed to allocate act8865 registor\n",
+						__func__);
+		return -ENOMEM;
+	}
+
+	pdata->num_regulators = matched;
+	regulator = pdata->regulators;
+
+	for (i = 0; i < matched; i++) {
+		regulator->id = i;
+		regulator->name = rmatch[i].name;
+		regulator->platform_data = rmatch[i].init_data;
+		of_node[i] = rmatch[i].of_node;
+		regulator++;
+	}
+
+	return 0;
+}
+
+#else
+
+static inline int act8865_pdata_from_dt(struct device *dev,
+					struct device_node **of_node,
+					struct act8865_platform_data *pdata)
+{
+	return 0;
+}
+#endif
+
+static int act8865_pmu_probe(struct i2c_client *client,
+			   const struct i2c_device_id *i2c_id)
+{
+	struct i2c_adapter *adapter = to_i2c_adapter(client->dev.parent);
+	struct regulator_dev **rdev;
+	struct device *dev = &client->dev;
+	struct act8865_platform_data *pdata = dev_get_platdata(dev);
+	struct regulator_config config = { };
+	struct act8865_data *act8865;
+	int i, id;
+	int ret = -EINVAL;
+	struct device_node *of_node[ACT8865_REG_NUM];
+
+	if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE))
+		return -EIO;
+
+	if (dev->of_node && !pdata) {
+		const struct of_device_id *id;
+		struct act8865_platform_data pdata_of;
+
+		id = of_match_device(of_match_ptr(act8865_dt_ids), dev);
+		if (!id)
+			return -ENODEV;
+
+		ret = act8865_pdata_from_dt(dev, of_node, &pdata_of);
+		if (ret < 0)
+			return ret;
+
+		pdata = &pdata_of;
+	} else {
+		memset(of_node, 0, sizeof(of_node));
+	}
+
+	if (pdata->num_regulators > ACT8865_REG_NUM) {
+		dev_err(dev, "Too many regulators found!\n");
+		return -EINVAL;
+	}
+
+	act8865 = devm_kzalloc(dev, sizeof(struct act8865_data) +
+			sizeof(struct regulator_dev *) * ACT8865_REG_NUM,
+			GFP_KERNEL);
+	if (!act8865)
+		return -ENOMEM;
+
+	act8865->client = client;
+	act8865->vsel_is_low = pdata->vsel_is_low;
+	rdev = act8865->rdev;
+
+	/* Finally register devices */
+	for (i = 0; i < pdata->num_regulators; i++) {
+
+		id = pdata->regulators[i].id;
+
+		config.dev = dev;
+		config.init_data = pdata->regulators[i].platform_data;
+		config.of_node = of_node[i];
+		config.driver_data = act8865;
+
+		rdev[i] = regulator_register(&act8865_reg[id], &config);
+		if (IS_ERR(rdev[i])) {
+			ret = PTR_ERR(rdev[i]);
+			dev_err(dev, "failed to register %s\n",
+				act8865_reg[id].name);
+			goto err_unregister;
+		}
+	}
+
+	i2c_set_clientdata(client, act8865);
+
+	return 0;
+
+err_unregister:
+	while (--i >= 0)
+		regulator_unregister(rdev[i]);
+
+	return ret;
+}
+
+static int act8865_pmu_remove(struct i2c_client *client)
+{
+	struct act8865_data *act8865 = i2c_get_clientdata(client);
+	int i;
+
+	for (i = 0; i < ACT8865_REG_NUM; i++)
+		regulator_unregister(act8865->rdev[i]);
+
+	return 0;
+}
+
+static const struct i2c_device_id act8865_ids[] = {
+	{ "act8865", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, act8865_ids);
+
+static struct i2c_driver act8865_pmu_driver = {
+	.probe		= act8865_pmu_probe,
+	.remove		= act8865_pmu_remove,
+	.driver		= {
+		.name	= "act8865",
+		.owner	= THIS_MODULE,
+	},
+	.id_table	= act8865_ids,
+};
+
+static int __init act8865_pmu_init(void)
+{
+	return i2c_add_driver(&act8865_pmu_driver);
+}
+subsys_initcall(act8865_pmu_init);
+
+static void __exit act8865_pmu_exit(void)
+{
+	i2c_del_driver(&act8865_pmu_driver);
+}
+module_exit(act8865_pmu_exit);
+
+MODULE_DESCRIPTION("active-semi act8865 voltage regulator driver");
+MODULE_AUTHOR("Wenyou Yang <wenyou.yang@atmel.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/regulator/act8865.h b/include/linux/regulator/act8865.h
new file mode 100644
index 0000000..202544f
--- /dev/null
+++ b/include/linux/regulator/act8865.h
@@ -0,0 +1,56 @@
+/*
+ * act8865.h  --  Voltage regulation for the active-semi act8865
+ *
+ * Copyright (C) 2013 Atmel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef __LINUX_REGULATOR_ACT8865_H
+#define __LINUX_REGULATOR_ACT8865_H
+
+#include <linux/regulator/machine.h>
+
+enum {
+	ACT8865_ID_REG1,
+	ACT8865_ID_REG2,
+	ACT8865_ID_REG3,
+	ACT8865_ID_REG4,
+	ACT8865_ID_REG5,
+	ACT8865_ID_REG6,
+	ACT8865_ID_REG7,
+	ACT8865_REG_NUM,
+};
+
+/**
+ * act8865_regulator_data - regulator data
+ * @id: regulator id
+ * @name: regulator name
+ * @platform_data: regulator init data
+ */
+struct act8865_regulator_data {
+	int				id;
+	const char				*name;
+	struct regulator_init_data	*platform_data;
+};
+
+/**
+ * act8865_platform_data - platform data for act8865
+ * @num_regulators: number of regulators used
+ * @regulators: pointer to regulators used
+ * @vsel_is_low: VSEL pin, drive to logic low to select default output voltage,
+ *			drive to logic high to select secondary output voltage.
+ */
+struct act8865_platform_data {
+	int num_regulators;
+	struct act8865_regulator_data *regulators;
+	unsigned vsel_is_low:1;
+};
+#endif
-- 
1.7.9.5

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

* [PATCH 2/3] regulator: act8865: add device tree binding doc
  2013-12-12  1:18 ` Wenyou Yang
  (?)
@ 2013-12-12  1:18   ` Wenyou Yang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: broonie, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre, wenyou.yang

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 .../bindings/regulator/act8865-regulator.txt       |   58 ++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
new file mode 100644
index 0000000..e191a92
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
@@ -0,0 +1,58 @@
+ACT8865 regulator
+-------------------
+
+Required properties:
+- compatible: "active-semi,act8865"
+- reg: I2C slave address
+- vsel-state-low: Initial state of vsel0 input is low, to select default output voltage.
+
+Example:
+--------
+
+	i2c1: i2c@f0018000 {
+		status = "okay";
+
+		pmic: act8865@5b {
+			compatible = "active-semi,act8865";
+			reg = <0x5b>;
+			vsel-state-low;
+
+			regulators {
+				vcc_1v8_reg: DCDC_REG1 {
+					regulator-name = "DCDC_REG1";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				vcc_1v2_reg: DCDC_REG2 {
+					regulator-name = "DCDC_REG2";
+					regulator-min-microvolt = <1100000>;
+					regulator-max-microvolt = <1300000>;
+					regulator-suspend-mem-microvolt = <1150000>;
+					regulator-suspend-standby-microvolt = <1150000>;
+					regulator-always-on;
+				};
+
+				vcc_3v3_reg: DCDC_REG3 {
+					regulator-name = "DCDC_REG3";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+
+				vddfuse_reg: LDO_REG4 {
+					regulator-name = "LDO_REG4";
+					regulator-min-microvolt = <2500000>;
+					regulator-max-microvolt = <2500000>;
+				};
+
+				vddana_reg: LDO_REG5 {
+					regulator-name = "LDO_REG5";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+			};
+		};
+	};
-- 
1.7.9.5


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

* [PATCH 2/3] regulator: act8865: add device tree binding doc
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: devicetree, vpalatin, linux-doc, nicolas.ferre, linux-kernel,
	rob.herring, wenyou.yang, broonie, grant.likely, plagnioj,
	linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 .../bindings/regulator/act8865-regulator.txt       |   58 ++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
new file mode 100644
index 0000000..e191a92
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
@@ -0,0 +1,58 @@
+ACT8865 regulator
+-------------------
+
+Required properties:
+- compatible: "active-semi,act8865"
+- reg: I2C slave address
+- vsel-state-low: Initial state of vsel0 input is low, to select default output voltage.
+
+Example:
+--------
+
+	i2c1: i2c@f0018000 {
+		status = "okay";
+
+		pmic: act8865@5b {
+			compatible = "active-semi,act8865";
+			reg = <0x5b>;
+			vsel-state-low;
+
+			regulators {
+				vcc_1v8_reg: DCDC_REG1 {
+					regulator-name = "DCDC_REG1";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				vcc_1v2_reg: DCDC_REG2 {
+					regulator-name = "DCDC_REG2";
+					regulator-min-microvolt = <1100000>;
+					regulator-max-microvolt = <1300000>;
+					regulator-suspend-mem-microvolt = <1150000>;
+					regulator-suspend-standby-microvolt = <1150000>;
+					regulator-always-on;
+				};
+
+				vcc_3v3_reg: DCDC_REG3 {
+					regulator-name = "DCDC_REG3";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+
+				vddfuse_reg: LDO_REG4 {
+					regulator-name = "LDO_REG4";
+					regulator-min-microvolt = <2500000>;
+					regulator-max-microvolt = <2500000>;
+				};
+
+				vddana_reg: LDO_REG5 {
+					regulator-name = "LDO_REG5";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+			};
+		};
+	};
-- 
1.7.9.5

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

* [PATCH 2/3] regulator: act8865: add device tree binding doc
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 .../bindings/regulator/act8865-regulator.txt       |   58 ++++++++++++++++++++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/regulator/act8865-regulator.txt

diff --git a/Documentation/devicetree/bindings/regulator/act8865-regulator.txt b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
new file mode 100644
index 0000000..e191a92
--- /dev/null
+++ b/Documentation/devicetree/bindings/regulator/act8865-regulator.txt
@@ -0,0 +1,58 @@
+ACT8865 regulator
+-------------------
+
+Required properties:
+- compatible: "active-semi,act8865"
+- reg: I2C slave address
+- vsel-state-low: Initial state of vsel0 input is low, to select default output voltage.
+
+Example:
+--------
+
+	i2c1: i2c at f0018000 {
+		status = "okay";
+
+		pmic: act8865 at 5b {
+			compatible = "active-semi,act8865";
+			reg = <0x5b>;
+			vsel-state-low;
+
+			regulators {
+				vcc_1v8_reg: DCDC_REG1 {
+					regulator-name = "DCDC_REG1";
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
+					regulator-always-on;
+				};
+
+				vcc_1v2_reg: DCDC_REG2 {
+					regulator-name = "DCDC_REG2";
+					regulator-min-microvolt = <1100000>;
+					regulator-max-microvolt = <1300000>;
+					regulator-suspend-mem-microvolt = <1150000>;
+					regulator-suspend-standby-microvolt = <1150000>;
+					regulator-always-on;
+				};
+
+				vcc_3v3_reg: DCDC_REG3 {
+					regulator-name = "DCDC_REG3";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+
+				vddfuse_reg: LDO_REG4 {
+					regulator-name = "LDO_REG4";
+					regulator-min-microvolt = <2500000>;
+					regulator-max-microvolt = <2500000>;
+				};
+
+				vddana_reg: LDO_REG5 {
+					regulator-name = "LDO_REG5";
+					regulator-min-microvolt = <3300000>;
+					regulator-max-microvolt = <3300000>;
+					regulator-always-on;
+				};
+			};
+		};
+	};
-- 
1.7.9.5

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

* [PATCH 3/3] ARM: dts: sama5d3xcm: add the regulator device node
  2013-12-12  1:18 ` Wenyou Yang
  (?)
@ 2013-12-12  1:18   ` Wenyou Yang
  -1 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: broonie, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre, wenyou.yang

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/boot/dts/sama5d3xcm.dtsi |   47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
index 726a0f3..0da005a 100644
--- a/arch/arm/boot/dts/sama5d3xcm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm.dtsi
@@ -38,6 +38,53 @@
 			macb0: ethernet@f0028000 {
 				phy-mode = "rgmii";
 			};
+
+			i2c1: i2c@f0018000 {
+				pmic: act8865@5b {
+					compatible = "active-semi,act8865";
+					reg = <0x5b>;
+					vsel-state-low;
+					status = "disabled";
+
+					regulators {
+						vcc_1v8_reg: DCDC_REG1 {
+							regulator-name = "DCDC_REG1";
+							regulator-min-microvolt = <1800000>;
+							regulator-max-microvolt = <1800000>;
+							regulator-always-on;
+						};
+
+						vcc_1v2_reg: DCDC_REG2 {
+							regulator-name = "DCDC_REG2";
+							regulator-min-microvolt = <1100000>;
+							regulator-max-microvolt = <1300000>;
+							regulator-suspend-mem-microvolt = <1150000>;
+							regulator-suspend-standby-microvolt = <1150000>;
+							regulator-always-on;
+						};
+
+						vcc_3v3_reg: DCDC_REG3 {
+							regulator-name = "DCDC_REG3";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddfuse_reg: LDO_REG4 {
+							regulator-name = "LDO_REG4";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddana_reg: LDO_REG5 {
+							regulator-name = "LDO_REG5";
+							regulator-min-microvolt = <2500000>;
+							regulator-max-microvolt = <2500000>;
+						};
+					};
+				};
+			};
 		};
 
 		nand0: nand@60000000 {
-- 
1.7.9.5


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

* [PATCH 3/3] ARM: dts: sama5d3xcm: add the regulator device node
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: lgirdwood
  Cc: devicetree, vpalatin, linux-doc, nicolas.ferre, linux-kernel,
	rob.herring, wenyou.yang, broonie, grant.likely, plagnioj,
	linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/boot/dts/sama5d3xcm.dtsi |   47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
index 726a0f3..0da005a 100644
--- a/arch/arm/boot/dts/sama5d3xcm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm.dtsi
@@ -38,6 +38,53 @@
 			macb0: ethernet@f0028000 {
 				phy-mode = "rgmii";
 			};
+
+			i2c1: i2c@f0018000 {
+				pmic: act8865@5b {
+					compatible = "active-semi,act8865";
+					reg = <0x5b>;
+					vsel-state-low;
+					status = "disabled";
+
+					regulators {
+						vcc_1v8_reg: DCDC_REG1 {
+							regulator-name = "DCDC_REG1";
+							regulator-min-microvolt = <1800000>;
+							regulator-max-microvolt = <1800000>;
+							regulator-always-on;
+						};
+
+						vcc_1v2_reg: DCDC_REG2 {
+							regulator-name = "DCDC_REG2";
+							regulator-min-microvolt = <1100000>;
+							regulator-max-microvolt = <1300000>;
+							regulator-suspend-mem-microvolt = <1150000>;
+							regulator-suspend-standby-microvolt = <1150000>;
+							regulator-always-on;
+						};
+
+						vcc_3v3_reg: DCDC_REG3 {
+							regulator-name = "DCDC_REG3";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddfuse_reg: LDO_REG4 {
+							regulator-name = "LDO_REG4";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddana_reg: LDO_REG5 {
+							regulator-name = "LDO_REG5";
+							regulator-min-microvolt = <2500000>;
+							regulator-max-microvolt = <2500000>;
+						};
+					};
+				};
+			};
 		};
 
 		nand0: nand@60000000 {
-- 
1.7.9.5

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

* [PATCH 3/3] ARM: dts: sama5d3xcm: add the regulator device node
@ 2013-12-12  1:18   ` Wenyou Yang
  0 siblings, 0 replies; 22+ messages in thread
From: Wenyou Yang @ 2013-12-12  1:18 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Wenyou Yang <wenyou.yang@atmel.com>
---
 arch/arm/boot/dts/sama5d3xcm.dtsi |   47 +++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/arm/boot/dts/sama5d3xcm.dtsi b/arch/arm/boot/dts/sama5d3xcm.dtsi
index 726a0f3..0da005a 100644
--- a/arch/arm/boot/dts/sama5d3xcm.dtsi
+++ b/arch/arm/boot/dts/sama5d3xcm.dtsi
@@ -38,6 +38,53 @@
 			macb0: ethernet at f0028000 {
 				phy-mode = "rgmii";
 			};
+
+			i2c1: i2c at f0018000 {
+				pmic: act8865 at 5b {
+					compatible = "active-semi,act8865";
+					reg = <0x5b>;
+					vsel-state-low;
+					status = "disabled";
+
+					regulators {
+						vcc_1v8_reg: DCDC_REG1 {
+							regulator-name = "DCDC_REG1";
+							regulator-min-microvolt = <1800000>;
+							regulator-max-microvolt = <1800000>;
+							regulator-always-on;
+						};
+
+						vcc_1v2_reg: DCDC_REG2 {
+							regulator-name = "DCDC_REG2";
+							regulator-min-microvolt = <1100000>;
+							regulator-max-microvolt = <1300000>;
+							regulator-suspend-mem-microvolt = <1150000>;
+							regulator-suspend-standby-microvolt = <1150000>;
+							regulator-always-on;
+						};
+
+						vcc_3v3_reg: DCDC_REG3 {
+							regulator-name = "DCDC_REG3";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddfuse_reg: LDO_REG4 {
+							regulator-name = "LDO_REG4";
+							regulator-min-microvolt = <3300000>;
+							regulator-max-microvolt = <3300000>;
+							regulator-always-on;
+						};
+
+						vddana_reg: LDO_REG5 {
+							regulator-name = "LDO_REG5";
+							regulator-min-microvolt = <2500000>;
+							regulator-max-microvolt = <2500000>;
+						};
+					};
+				};
+			};
 		};
 
 		nand0: nand at 60000000 {
-- 
1.7.9.5

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

* Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
  2013-12-12  1:18   ` Wenyou Yang
@ 2013-12-12 16:51     ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-12 16:51 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: lgirdwood, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre

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

On Thu, Dec 12, 2013 at 09:18:49AM +0800, Wenyou Yang wrote:

The main thing with this driver seems to be that it needs a bit of
modernisation to use current kernel features and APIs - there's nothing
terribly wrong from a quick glance through but it needs an update.
Details below.

> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -577,5 +577,12 @@ config REGULATOR_WM8994
>  	  This driver provides support for the voltage regulators on the
>  	  WM8994 CODEC.
>  
> +config REGULATOR_ACT8865
> +	tristate "Active-semi act8865 voltage regulator"
> +	depends on I2C
> +	help
> +	  This driver controls a active-semi act8865 voltage output
> +	  regulator via I2C bus.
> +

Please keep this and the Makefile sorted.

> +static int act8865_read_reg(struct act8865_data *act8865, u8 reg)
> +{
> +	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
> +	if (ret > 0)
> +		ret &= 0xff;
> +
> +	return ret;
> +}

Use regmap for register I/O and use the helpers provided by the core.

> +static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32 selector)
> +{
> +	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	u32	reg = act8865_vset_reg_addr(act8865, id);

Throughout the file the indentation is a really strange mix of the first
two lines (which are the normal kernel style) and the bottom line (which
isn't).

> +	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);

Remove noisy logging like this.

> +		rdev[i] = regulator_register(&act8865_reg[id], &config);

devm_regulator_register().

> +static int __init act8865_pmu_init(void)
> +{
> +	return i2c_add_driver(&act8865_pmu_driver);
> +}
> +subsys_initcall(act8865_pmu_init);

module_i2c_driver().

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

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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-12 16:51     ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-12 16:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 12, 2013 at 09:18:49AM +0800, Wenyou Yang wrote:

The main thing with this driver seems to be that it needs a bit of
modernisation to use current kernel features and APIs - there's nothing
terribly wrong from a quick glance through but it needs an update.
Details below.

> --- a/drivers/regulator/Kconfig
> +++ b/drivers/regulator/Kconfig
> @@ -577,5 +577,12 @@ config REGULATOR_WM8994
>  	  This driver provides support for the voltage regulators on the
>  	  WM8994 CODEC.
>  
> +config REGULATOR_ACT8865
> +	tristate "Active-semi act8865 voltage regulator"
> +	depends on I2C
> +	help
> +	  This driver controls a active-semi act8865 voltage output
> +	  regulator via I2C bus.
> +

Please keep this and the Makefile sorted.

> +static int act8865_read_reg(struct act8865_data *act8865, u8 reg)
> +{
> +	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
> +	if (ret > 0)
> +		ret &= 0xff;
> +
> +	return ret;
> +}

Use regmap for register I/O and use the helpers provided by the core.

> +static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32 selector)
> +{
> +	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
> +	int id = rdev_get_id(rdev);
> +	u32	reg = act8865_vset_reg_addr(act8865, id);

Throughout the file the indentation is a really strange mix of the first
two lines (which are the normal kernel style) and the bottom line (which
isn't).

> +	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);

Remove noisy logging like this.

> +		rdev[i] = regulator_register(&act8865_reg[id], &config);

devm_regulator_register().

> +static int __init act8865_pmu_init(void)
> +{
> +	return i2c_add_driver(&act8865_pmu_driver);
> +}
> +subsys_initcall(act8865_pmu_init);

module_i2c_driver().
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131212/36b04fd7/attachment.sig>

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

* Re: [PATCH 2/3] regulator: act8865: add device tree binding doc
  2013-12-12  1:18   ` Wenyou Yang
@ 2013-12-12 16:52     ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-12 16:52 UTC (permalink / raw)
  To: Wenyou Yang
  Cc: lgirdwood, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj,
	nicolas.ferre

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

On Thu, Dec 12, 2013 at 09:18:50AM +0800, Wenyou Yang wrote:

> +Required properties:
> +- compatible: "active-semi,act8865"
> +- reg: I2C slave address
> +- vsel-state-low: Initial state of vsel0 input is low, to select default output voltage.

This needs to document which regulators exist and what strings should be
used to select them as well.

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

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

* [PATCH 2/3] regulator: act8865: add device tree binding doc
@ 2013-12-12 16:52     ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-12 16:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 12, 2013 at 09:18:50AM +0800, Wenyou Yang wrote:

> +Required properties:
> +- compatible: "active-semi,act8865"
> +- reg: I2C slave address
> +- vsel-state-low: Initial state of vsel0 input is low, to select default output voltage.

This needs to document which regulators exist and what strings should be
used to select them as well.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131212/6595b695/attachment.sig>

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

* RE: [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
  2013-12-12 16:51     ` Mark Brown
  (?)
@ 2013-12-13  7:10       ` Yang, Wenyou
  -1 siblings, 0 replies; 22+ messages in thread
From: Yang, Wenyou @ 2013-12-13  7:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: lgirdwood, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj, Ferre,
	Nicolas

Hi Mark,

Thanks your for feedback.

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Friday, December 13, 2013 12:52 AM
> To: Yang, Wenyou
> Cc: lgirdwood@gmail.com; grant.likely@linaro.org;
> rob.herring@calxeda.com; vpalatin@chromium.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> plagnioj@jcrosoft.com; Ferre, Nicolas
> Subject: Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management
> IC) driver
> 
> On Thu, Dec 12, 2013 at 09:18:49AM +0800, Wenyou Yang wrote:
> 
> The main thing with this driver seems to be that it needs a bit of
> modernisation to use current kernel features and APIs - there's nothing
> terribly wrong from a quick glance through but it needs an update.
> Details below.
> 
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -577,5 +577,12 @@ config REGULATOR_WM8994
> >  	  This driver provides support for the voltage regulators on the
> >  	  WM8994 CODEC.
> >
> > +config REGULATOR_ACT8865
> > +	tristate "Active-semi act8865 voltage regulator"
> > +	depends on I2C
> > +	help
> > +	  This driver controls a active-semi act8865 voltage output
> > +	  regulator via I2C bus.
> > +
> 
> Please keep this and the Makefile sorted.
Only this one confuses me,
How can I sort it? By i2c interface, by regulator type: voltage or current?
Where is act8865 should be?

> 
> > +static int act8865_read_reg(struct act8865_data *act8865, u8 reg) {
> > +	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
> > +	if (ret > 0)
> > +		ret &= 0xff;
> > +
> > +	return ret;
> > +}
> 
> Use regmap for register I/O and use the helpers provided by the core.
> 
> > +static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32
> > +selector) {
> > +	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
> > +	int id = rdev_get_id(rdev);
> > +	u32	reg = act8865_vset_reg_addr(act8865, id);
> 
> Throughout the file the indentation is a really strange mix of the first
> two lines (which are the normal kernel style) and the bottom line (which
> isn't).
> 
> > +	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
> 
> Remove noisy logging like this.
> 
> > +		rdev[i] = regulator_register(&act8865_reg[id], &config);
> 
> devm_regulator_register().
> 
> > +static int __init act8865_pmu_init(void) {
> > +	return i2c_add_driver(&act8865_pmu_driver);
> > +}
> > +subsys_initcall(act8865_pmu_init);
> 
> module_i2c_driver().

Others is OK for me, I will modify, and send out new version.

Thanks again.

Best Regards,
Wenyou Yang

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

* RE: [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-13  7:10       ` Yang, Wenyou
  0 siblings, 0 replies; 22+ messages in thread
From: Yang, Wenyou @ 2013-12-13  7:10 UTC (permalink / raw)
  To: Mark Brown
  Cc: devicetree, vpalatin, linux-doc, Ferre, Nicolas, lgirdwood,
	rob.herring, linux-kernel, grant.likely, plagnioj,
	linux-arm-kernel

Hi Mark,

Thanks your for feedback.

> -----Original Message-----
> From: Mark Brown [mailto:broonie@kernel.org]
> Sent: Friday, December 13, 2013 12:52 AM
> To: Yang, Wenyou
> Cc: lgirdwood@gmail.com; grant.likely@linaro.org;
> rob.herring@calxeda.com; vpalatin@chromium.org;
> devicetree@vger.kernel.org; linux-kernel@vger.kernel.org; linux-
> doc@vger.kernel.org; linux-arm-kernel@lists.infradead.org;
> plagnioj@jcrosoft.com; Ferre, Nicolas
> Subject: Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management
> IC) driver
> 
> On Thu, Dec 12, 2013 at 09:18:49AM +0800, Wenyou Yang wrote:
> 
> The main thing with this driver seems to be that it needs a bit of
> modernisation to use current kernel features and APIs - there's nothing
> terribly wrong from a quick glance through but it needs an update.
> Details below.
> 
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -577,5 +577,12 @@ config REGULATOR_WM8994
> >  	  This driver provides support for the voltage regulators on the
> >  	  WM8994 CODEC.
> >
> > +config REGULATOR_ACT8865
> > +	tristate "Active-semi act8865 voltage regulator"
> > +	depends on I2C
> > +	help
> > +	  This driver controls a active-semi act8865 voltage output
> > +	  regulator via I2C bus.
> > +
> 
> Please keep this and the Makefile sorted.
Only this one confuses me,
How can I sort it? By i2c interface, by regulator type: voltage or current?
Where is act8865 should be?

> 
> > +static int act8865_read_reg(struct act8865_data *act8865, u8 reg) {
> > +	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
> > +	if (ret > 0)
> > +		ret &= 0xff;
> > +
> > +	return ret;
> > +}
> 
> Use regmap for register I/O and use the helpers provided by the core.
> 
> > +static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32
> > +selector) {
> > +	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
> > +	int id = rdev_get_id(rdev);
> > +	u32	reg = act8865_vset_reg_addr(act8865, id);
> 
> Throughout the file the indentation is a really strange mix of the first
> two lines (which are the normal kernel style) and the bottom line (which
> isn't).
> 
> > +	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
> 
> Remove noisy logging like this.
> 
> > +		rdev[i] = regulator_register(&act8865_reg[id], &config);
> 
> devm_regulator_register().
> 
> > +static int __init act8865_pmu_init(void) {
> > +	return i2c_add_driver(&act8865_pmu_driver);
> > +}
> > +subsys_initcall(act8865_pmu_init);
> 
> module_i2c_driver().

Others is OK for me, I will modify, and send out new version.

Thanks again.

Best Regards,
Wenyou Yang

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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-13  7:10       ` Yang, Wenyou
  0 siblings, 0 replies; 22+ messages in thread
From: Yang, Wenyou @ 2013-12-13  7:10 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Mark,

Thanks your for feedback.

> -----Original Message-----
> From: Mark Brown [mailto:broonie at kernel.org]
> Sent: Friday, December 13, 2013 12:52 AM
> To: Yang, Wenyou
> Cc: lgirdwood at gmail.com; grant.likely at linaro.org;
> rob.herring at calxeda.com; vpalatin at chromium.org;
> devicetree at vger.kernel.org; linux-kernel at vger.kernel.org; linux-
> doc at vger.kernel.org; linux-arm-kernel at lists.infradead.org;
> plagnioj at jcrosoft.com; Ferre, Nicolas
> Subject: Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management
> IC) driver
> 
> On Thu, Dec 12, 2013 at 09:18:49AM +0800, Wenyou Yang wrote:
> 
> The main thing with this driver seems to be that it needs a bit of
> modernisation to use current kernel features and APIs - there's nothing
> terribly wrong from a quick glance through but it needs an update.
> Details below.
> 
> > --- a/drivers/regulator/Kconfig
> > +++ b/drivers/regulator/Kconfig
> > @@ -577,5 +577,12 @@ config REGULATOR_WM8994
> >  	  This driver provides support for the voltage regulators on the
> >  	  WM8994 CODEC.
> >
> > +config REGULATOR_ACT8865
> > +	tristate "Active-semi act8865 voltage regulator"
> > +	depends on I2C
> > +	help
> > +	  This driver controls a active-semi act8865 voltage output
> > +	  regulator via I2C bus.
> > +
> 
> Please keep this and the Makefile sorted.
Only this one confuses me,
How can I sort it? By i2c interface, by regulator type: voltage or current?
Where is act8865 should be?

> 
> > +static int act8865_read_reg(struct act8865_data *act8865, u8 reg) {
> > +	int ret = i2c_smbus_read_byte_data(act8865->client, reg);
> > +	if (ret > 0)
> > +		ret &= 0xff;
> > +
> > +	return ret;
> > +}
> 
> Use regmap for register I/O and use the helpers provided by the core.
> 
> > +static int act8865_set_voltage_sel(struct regulator_dev *rdev, u32
> > +selector) {
> > +	struct act8865_data *act8865 = rdev_get_drvdata(rdev);
> > +	int id = rdev_get_id(rdev);
> > +	u32	reg = act8865_vset_reg_addr(act8865, id);
> 
> Throughout the file the indentation is a really strange mix of the first
> two lines (which are the normal kernel style) and the bottom line (which
> isn't).
> 
> > +	pr_info("%s: suspend voltage %dmV\n", rdev->desc->name, uV / 1000);
> 
> Remove noisy logging like this.
> 
> > +		rdev[i] = regulator_register(&act8865_reg[id], &config);
> 
> devm_regulator_register().
> 
> > +static int __init act8865_pmu_init(void) {
> > +	return i2c_add_driver(&act8865_pmu_driver);
> > +}
> > +subsys_initcall(act8865_pmu_init);
> 
> module_i2c_driver().

Others is OK for me, I will modify, and send out new version.

Thanks again.

Best Regards,
Wenyou Yang

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

* Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
  2013-12-13  7:10       ` Yang, Wenyou
  (?)
@ 2013-12-13 10:45         ` Mark Brown
  -1 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-13 10:45 UTC (permalink / raw)
  To: Yang, Wenyou
  Cc: lgirdwood, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj, Ferre,
	Nicolas

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

On Fri, Dec 13, 2013 at 07:10:36AM +0000, Yang, Wenyou wrote:

> > Please keep this and the Makefile sorted.

> Only this one confuses me,
> How can I sort it? By i2c interface, by regulator type: voltage or current?
> Where is act8865 should be?

By name - look at the existing entries.

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

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

* Re: [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-13 10:45         ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-13 10:45 UTC (permalink / raw)
  To: Yang, Wenyou
  Cc: lgirdwood, grant.likely, rob.herring, vpalatin, devicetree,
	linux-kernel, linux-doc, linux-arm-kernel, plagnioj, Ferre,
	Nicolas

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

On Fri, Dec 13, 2013 at 07:10:36AM +0000, Yang, Wenyou wrote:

> > Please keep this and the Makefile sorted.

> Only this one confuses me,
> How can I sort it? By i2c interface, by regulator type: voltage or current?
> Where is act8865 should be?

By name - look at the existing entries.

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

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

* [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver
@ 2013-12-13 10:45         ` Mark Brown
  0 siblings, 0 replies; 22+ messages in thread
From: Mark Brown @ 2013-12-13 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Dec 13, 2013 at 07:10:36AM +0000, Yang, Wenyou wrote:

> > Please keep this and the Makefile sorted.

> Only this one confuses me,
> How can I sort it? By i2c interface, by regulator type: voltage or current?
> Where is act8865 should be?

By name - look at the existing entries.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131213/8d26f41a/attachment.sig>

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

end of thread, other threads:[~2013-12-13 10:45 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-12  1:18 [PATCH 0/3] regulator: act8865: add PMIC driver Wenyou Yang
2013-12-12  1:18 ` Wenyou Yang
2013-12-12  1:18 ` Wenyou Yang
2013-12-12  1:18 ` [PATCH 1/3] regulator: act8865: add PMIC(Power Management IC) driver Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang
2013-12-12 16:51   ` Mark Brown
2013-12-12 16:51     ` Mark Brown
2013-12-13  7:10     ` Yang, Wenyou
2013-12-13  7:10       ` Yang, Wenyou
2013-12-13  7:10       ` Yang, Wenyou
2013-12-13 10:45       ` Mark Brown
2013-12-13 10:45         ` Mark Brown
2013-12-13 10:45         ` Mark Brown
2013-12-12  1:18 ` [PATCH 2/3] regulator: act8865: add device tree binding doc Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang
2013-12-12 16:52   ` Mark Brown
2013-12-12 16:52     ` Mark Brown
2013-12-12  1:18 ` [PATCH 3/3] ARM: dts: sama5d3xcm: add the regulator device node Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang
2013-12-12  1:18   ` Wenyou Yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.