[PATCH 0/2] regulator: add initial suport for max77686

[PATCH 0/2] regulator: add initial suport for max77686

[Yadwinder Singh]

This patch series adds support for max77686 which is a multifunction device which
includes regulator (pmic), rtc and charger sub-blocks within it. The support for
mfd driver and regulator driver are added by this patch series. This patch series
also includes device tree and irqdomain support for mfd and regulator portions.

Thanks,
Yadwinder Singh

[PATCH 1/2] mfd: Add support for MAX77686.

[Yadwinder Singh]

From: Yadwinder Singh Brar <yadi.brar@samsung.com>

MAX77686 is a mulitifunction device with PMIC, RTC and Charger on chip. This
driver provides common support for accessing the device. This is initial
version of this driver that supports to enable the chip with its primary I2C
bus.It also includes IRQ and device tree support for MAX77686 chip.

Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/mfd/Kconfig                  |   19 ++
 drivers/mfd/Makefile                 |    1 +
 drivers/mfd/max77686-irq.c           |  258 +++++++++++++++++++++++++++++
 drivers/mfd/max77686.c               |  299 ++++++++++++++++++++++++++++++++++
 include/linux/mfd/max77686-private.h |  279 +++++++++++++++++++++++++++++++
 include/linux/mfd/max77686.h         |  104 ++++++++++++
 6 files changed, 960 insertions(+), 0 deletions(-)
 create mode 100644 drivers/mfd/max77686-irq.c
 create mode 100644 drivers/mfd/max77686.c
 create mode 100644 include/linux/mfd/max77686-private.h
 create mode 100644 include/linux/mfd/max77686.h

diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 11e4438..618bc42 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -441,6 +441,25 @@ config MFD_MAX8998
 	  additional drivers must be enabled in order to use the functionality
 	  of the device.
 
+config MFD_MAX77686
+	bool "Maxim Semiconductor MAX77686 PMIC Support"
+	depends on I2C=y && GENERIC_HARDIRQS
+	select MFD_CORE
+	help
+	  Say yes here to support for Maxim Semiconductor MAX77686.
+	  This is a Power Management IC with RTC on chip.
+	  This driver provides common support for accessing the device;
+	  additional drivers must be enabled in order to use the functionality
+	  of the device.
+
+config DEBUG_MAX77686
+	bool "MAX77686 PMIC debugging"
+	depends on MFD_MAX77686
+	help
+	  Say yes, if you need enable debug messages in MFD_MAX77686 driver.
+	  Further for enabling/disabling particular type of debug messages set
+	  max77686_debug_mask accordingly.
+
 config MFD_S5M_CORE
 	bool "SAMSUNG S5M Series Support"
 	depends on I2C=y && GENERIC_HARDIRQS
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 05fa538..19ecca9 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -79,6 +79,7 @@ max8925-objs			:= max8925-core.o max8925-i2c.o
 obj-$(CONFIG_MFD_MAX8925)	+= max8925.o
 obj-$(CONFIG_MFD_MAX8997)	+= max8997.o max8997-irq.o
 obj-$(CONFIG_MFD_MAX8998)	+= max8998.o max8998-irq.o
+obj-$(CONFIG_MFD_MAX77686)	+= max77686.o max77686-irq.o
 
 pcf50633-objs			:= pcf50633-core.o pcf50633-irq.o
 obj-$(CONFIG_MFD_PCF50633)	+= pcf50633.o
diff --git a/drivers/mfd/max77686-irq.c b/drivers/mfd/max77686-irq.c
new file mode 100644
index 0000000..b40a46d
--- /dev/null
+++ b/drivers/mfd/max77686-irq.c
@@ -0,0 +1,258 @@
+/*
+ * max77686-irq.c - Interrupt controller support for MAX77686
+ *
+ * Copyright (C) 2012 Samsung Electronics Co.Ltd
+ * Chiwoong Byun <woong.byun@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * This driver is based on max8997-irq.c
+ */
+
+#include <linux/err.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/mfd/max77686.h>
+#include <linux/mfd/max77686-private.h>
+#include <linux/irqdomain.h>
+
+#define IRQ_GRP (data->hwirq >> 3)
+#define IRQ_MASK (1 << (data->hwirq & 7))
+
+int max77686_debug_mask = MAX77686_DEBUG_INFO; /* enable debug prints */
+
+static const u8 max77686_mask_reg[] = {
+	[PMIC_INT1]	= MAX77686_REG_INT1MSK,
+	[PMIC_INT2]	= MAX77686_REG_INT2MSK,
+	[RTC_INT]	= MAX77686_RTC_INTM,
+};
+
+static struct i2c_client *max77686_get_i2c(struct max77686_dev *max77686,
+					   enum max77686_irq_source src)
+{
+	switch (src) {
+	case PMIC_INT1...PMIC_INT2:
+		return max77686->i2c;
+	case RTC_INT:
+		return max77686->rtc;
+	default:
+		return ERR_PTR(-EINVAL);
+	}
+}
+
+static void max77686_irq_lock(struct irq_data *data)
+{
+	struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+
+	mutex_lock(&max77686->irqlock);
+}
+
+static void max77686_irq_sync_unlock(struct irq_data *data)
+{
+	struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+	int i;
+
+	for (i = 0; i < MAX77686_IRQ_GROUP_NR; i++) {
+		u8 mask_reg = max77686_mask_reg[i];
+		struct i2c_client *i2c = max77686_get_i2c(max77686, i);
+
+		dbg_mask("%s: mask_reg[%d]=0x%x, cur=0x%x\n",
+			__func__, i, mask_reg, max77686->irq_masks_cur[i]);
+
+		if (mask_reg == MAX77686_REG_INVALID || IS_ERR_OR_NULL(i2c))
+			continue;
+
+		max77686->irq_masks_cache[i] = max77686->irq_masks_cur[i];
+
+		max77686_write_reg(i2c, max77686_mask_reg[i],
+				   max77686->irq_masks_cur[i]);
+	}
+
+	mutex_unlock(&max77686->irqlock);
+}
+
+static void max77686_irq_mask(struct irq_data *data)
+{
+	struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+
+	max77686->irq_masks_cur[IRQ_GRP] |= IRQ_MASK;
+	dbg_mask("%s: group=%d, cur=0x%x\n",
+		__func__, IRQ_GRP,
+		max77686->irq_masks_cur[IRQ_GRP]);
+
+}
+
+static void max77686_irq_unmask(struct irq_data *data)
+{
+	struct max77686_dev *max77686 = irq_get_chip_data(data->irq);
+
+	max77686->irq_masks_cur[IRQ_GRP] &= ~IRQ_MASK;
+	dbg_mask("%s: group=%d, cur=0x%x\n",
+		__func__, IRQ_GRP,
+		max77686->irq_masks_cur[IRQ_GRP]);
+
+}
+
+static struct irq_chip max77686_irq_chip = {
+	.name = "max77686",
+	.irq_bus_lock = max77686_irq_lock,
+	.irq_bus_sync_unlock = max77686_irq_sync_unlock,
+	.irq_mask = max77686_irq_mask,
+	.irq_unmask = max77686_irq_unmask,
+};
+
+static irqreturn_t max77686_irq_thread(int irq, void *data)
+{
+	struct max77686_dev *max77686 = data;
+	u8 irq_reg[MAX77686_IRQ_GROUP_NR] = { };
+	u8 irq_src;
+	int ret, grp, i, cur_irq;
+
+	ret = max77686_read_reg(max77686->i2c, MAX77686_REG_INTSRC, &irq_src);
+	if (ret < 0) {
+		dev_err(max77686->dev, "Failed to read interrupt source: %d\n",
+			ret);
+		return IRQ_NONE;
+	}
+
+	if (irq_src == MAX77686_IRQSRC_PMIC) {
+		ret = max77686_bulk_read(max77686->i2c, MAX77686_REG_INT1,
+					 2, irq_reg);
+		if (ret < 0) {
+			dev_err(max77686->dev,
+				"Failed to read pmic interrupt: %d\n", ret);
+			return IRQ_NONE;
+		}
+
+		dbg_int("%s: int1=0x%x, int2=0x%x\n", __func__,
+			irq_reg[PMIC_INT1], irq_reg[PMIC_INT2]);
+	}
+
+	if (irq_src & MAX77686_IRQSRC_RTC) {
+		ret = max77686_read_reg(max77686->rtc, MAX77686_RTC_INT,
+					&irq_reg[RTC_INT]);
+		if (ret < 0) {
+			dev_err(max77686->dev,
+				"Failed to read rtc interrupt: %d\n", ret);
+			return IRQ_NONE;
+		}
+		dbg_int("%s: rtc int=0x%x\n", __func__,
+			irq_reg[RTC_INT]);
+
+	}
+
+	for (grp = 0; grp < MAX77686_IRQ_GROUP_NR; grp++) {
+		irq_reg[grp] &= ~max77686->irq_masks_cur[grp];
+		i = 0;
+		while (irq_reg[grp]) {
+			if (irq_reg[grp] & (1 << i)) {
+				cur_irq = irq_find_mapping(max77686->irq_domain,
+						grp*8 + i);
+				if (cur_irq)
+					handle_nested_irq(cur_irq);
+
+				irq_reg[grp] &= ~(1 << i);
+				dbg_int("%s: Handled irq:%x from group:%x\n",
+					__func__, i, grp);
+			}
+			i++;
+		}
+	}
+
+	dbg_info("%s returning\n", __func__);
+
+	return IRQ_HANDLED;
+}
+
+int max77686_irq_resume(struct max77686_dev *max77686)
+{
+	if (max77686->irq && max77686->irq_domain)
+		max77686_irq_thread(0, max77686);
+
+	return 0;
+}
+
+static int max77686_irq_domain_map(struct irq_domain *d, unsigned int irq,
+					irq_hw_number_t hw)
+{
+	struct max77686_dev *max77686 = d->host_data;
+
+	irq_set_chip_data(irq, max77686);
+	irq_set_chip_and_handler(irq, &max77686_irq_chip, handle_edge_irq);
+	irq_set_nested_thread(irq, 1);
+	set_irq_flags(irq, IRQF_VALID);
+	return 0;
+}
+
+static struct irq_domain_ops max77686_irq_domain_ops = {
+	.map = max77686_irq_domain_map,
+};
+
+int max77686_irq_init(struct max77686_dev *max77686)
+{
+	int i, ret;
+	struct irq_domain *domain;
+
+	if (!max77686->irq) {
+			dev_warn(max77686->dev,
+				 "No interrupt specified.\n");
+			return -ENODEV;
+	}
+
+	mutex_init(&max77686->irqlock);
+
+	/* Mask individual interrupt sources */
+	for (i = 0; i < MAX77686_IRQ_GROUP_NR; i++) {
+		struct i2c_client *i2c;
+
+		max77686->irq_masks_cur[i] = 0xff;
+		max77686->irq_masks_cache[i] = 0xff;
+		i2c = max77686_get_i2c(max77686, i);
+
+		if (IS_ERR_OR_NULL(i2c))
+			continue;
+		if (max77686_mask_reg[i] == MAX77686_REG_INVALID)
+			continue;
+
+		max77686_write_reg(i2c, max77686_mask_reg[i], 0xff);
+	}
+
+	domain = irq_domain_add_linear(NULL, MAX77686_IRQ_NR,
+					&max77686_irq_domain_ops, &max77686);
+	if (!domain) {
+		dev_err(max77686->dev, "could not create irq domain\n");
+		return -ENODEV;
+	}
+
+	ret = request_threaded_irq(max77686->irq, NULL, max77686_irq_thread,
+				   IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
+				   "max77686-irq", max77686);
+
+	if (ret) {
+		dev_err(max77686->dev, "Failed to request IRQ %d: %d\n",
+			max77686->irq, ret);
+		return ret;
+	}
+
+	dbg_info("%s : returning\n", __func__);
+
+	return 0;
+}
+
+void max77686_irq_exit(struct max77686_dev *max77686)
+{
+	if (max77686->irq)
+		free_irq(max77686->irq, max77686);
+}
diff --git a/drivers/mfd/max77686.c b/drivers/mfd/max77686.c
new file mode 100644
index 0000000..8bee7e8
--- /dev/null
+++ b/drivers/mfd/max77686.c
@@ -0,0 +1,299 @@
+/*
+ * max77686.c - mfd core driver for the Maxim 77686
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Chiwoong Byun <woong.byun@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * This driver is based on max8997.c
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/err.h>
+#include <linux/pm_runtime.h>
+#include <linux/mutex.h>
+#include <linux/mfd/core.h>
+#include <linux/mfd/max77686.h>
+#include <linux/mfd/max77686-private.h>
+
+#define I2C_ADDR_RTC    (0x0C >> 1)
+
+#ifdef CONFIG_OF
+static struct of_device_id __devinitdata max77686_pmic_dt_match[] = {
+	{.compatible = "maxim,max77686-pmic",	.data = 0},
+	{},
+};
+#endif
+
+static struct mfd_cell max77686_devs[] = {
+	{.name = "max77686-pmic",},
+	{.name = "max77686-rtc",},
+};
+
+int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(i2c, reg);
+	if (ret < 0)
+		return ret;
+
+	*dest = ret & 0xff;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max77686_read_reg);
+
+int max77686_bulk_read(struct i2c_client *i2c, u8 reg, int count, u8 *buf)
+{
+	int ret;
+
+	ret = i2c_smbus_read_i2c_block_data(i2c, reg, count, buf);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max77686_bulk_read);
+
+int max77686_write_reg(struct i2c_client *i2c, u8 reg, u8 value)
+{
+	int ret;
+
+	ret = i2c_smbus_write_byte_data(i2c, reg, value);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(max77686_write_reg);
+
+int max77686_bulk_write(struct i2c_client *i2c, u8 reg, int count, u8 *buf)
+{
+	int ret;
+
+	ret = i2c_smbus_write_i2c_block_data(i2c, reg, count, buf);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(max77686_bulk_write);
+
+int max77686_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask)
+{
+	int ret;
+
+	ret = i2c_smbus_read_byte_data(i2c, reg);
+	if (ret >= 0) {
+		u8 old_val = ret & 0xff;
+		u8 new_val = (val & mask) | (old_val & (~mask));
+		ret = i2c_smbus_write_byte_data(i2c, reg, new_val);
+	}
+	return ret;
+}
+EXPORT_SYMBOL_GPL(max77686_update_reg);
+
+#ifdef CONFIG_OF
+static struct max77686_platform_data *max77686_i2c_parse_dt_pdata(struct device
+								  *dev)
+{
+	struct max77686_platform_data *pd;
+
+	pd = devm_kzalloc(dev, sizeof(*pd), GFP_KERNEL);
+	if (!pd) {
+		dev_err(dev, "could not allocate memory for pdata\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
+		pd->wakeup = true;
+
+	return pd;
+}
+#else
+static struct max77686_platform_data *max77686_i2c_parse_dt_pdata(struct device
+								  *dev)
+{
+	return 0;
+}
+#endif
+
+static inline int max77686_i2c_get_driver_data(struct i2c_client *i2c,
+					       const struct i2c_device_id *id)
+{
+#ifdef CONFIG_OF
+	if (i2c->dev.of_node) {
+		const struct of_device_id *match;
+		match = of_match_node(max77686_pmic_dt_match,
+				i2c->dev.of_node);
+		return (int)match->data;
+	}
+#endif
+	return (int)id->driver_data;
+}
+
+static int max77686_i2c_probe(struct i2c_client *i2c,
+			      const struct i2c_device_id *id)
+{
+	struct max77686_dev *max77686;
+	struct max77686_platform_data *pdata = i2c->dev.platform_data;
+	int ret = 0;
+	u8 data;
+
+	max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
+	if (max77686 == NULL) {
+		dev_err(max77686->dev, "could not allocate memory\n");
+		return -ENOMEM;
+	}
+
+	max77686->dev = &i2c->dev;
+
+	if (max77686->dev->of_node) {
+		pdata = max77686_i2c_parse_dt_pdata(max77686->dev);
+		if (IS_ERR(pdata)) {
+			ret = PTR_ERR(pdata);
+			goto err;
+		}
+	}
+	if (!pdata) {
+		ret = -ENODEV;
+		dbg_info("%s : No platform data found\n", __func__);
+		goto err;
+	}
+
+	i2c_set_clientdata(i2c, max77686);
+	max77686->i2c = i2c;
+	max77686->irq = i2c->irq;
+	max77686->type = max77686_i2c_get_driver_data(i2c, id);
+
+	max77686->pdata = pdata;
+	max77686->wakeup = pdata->wakeup;
+
+	max77686->rtc = i2c_new_dummy(i2c->adapter, I2C_ADDR_RTC);
+	i2c_set_clientdata(max77686->rtc, max77686);
+	ret = max77686_irq_init(max77686);
+	if (ret < 0) {
+		dbg_info("%s : max77686_irq_init failed\n", __func__);
+		goto err;
+	}
+
+	ret = mfd_add_devices(max77686->dev, -1, max77686_devs,
+			      ARRAY_SIZE(max77686_devs), NULL, 0);
+
+	if (ret < 0) {
+		dbg_info("%s : mfd_add_devices failed\n", __func__);
+		goto err_mfd;
+	}
+
+	pm_runtime_set_active(max77686->dev);
+	device_init_wakeup(max77686->dev, max77686->wakeup);
+
+	if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
+		ret = -EIO;
+		dbg_info("%s : device not found on this channel\n", __func__);
+		goto err_mfd;
+	} else
+		dev_info(max77686->dev, "device found\n");
+
+	return ret;
+
+ err_mfd:
+	mfd_remove_devices(max77686->dev);
+	max77686_irq_exit(max77686);
+	i2c_unregister_device(max77686->rtc);
+ err:
+	kfree(max77686);
+	dev_err(max77686->dev, "device probe failed : %d\n", ret);
+	return ret;
+}
+
+static int max77686_i2c_remove(struct i2c_client *i2c)
+{
+	struct max77686_dev *max77686 = i2c_get_clientdata(i2c);
+
+	device_init_wakeup(max77686->dev, 0);
+	pm_runtime_set_suspended(max77686->dev);
+	mfd_remove_devices(max77686->dev);
+	max77686_irq_exit(max77686);
+	i2c_unregister_device(max77686->rtc);
+	kfree(max77686);
+	return 0;
+}
+
+static const struct i2c_device_id max77686_i2c_id[] = {
+	{"max77686", 0},
+	{}
+};
+
+MODULE_DEVICE_TABLE(i2c, max77686_i2c_id);
+
+static int max77686_suspend(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct max77686_dev *max77686 = i2c_get_clientdata(i2c);
+
+	if (device_may_wakeup(dev))
+		enable_irq_wake(max77686->irq);
+
+	return 0;
+}
+
+static int max77686_resume(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct max77686_dev *max77686 = i2c_get_clientdata(i2c);
+
+	if (device_may_wakeup(dev))
+		disable_irq_wake(max77686->irq);
+
+	max77686_irq_resume(max77686);
+	return 0;
+}
+
+const struct dev_pm_ops max77686_pm = {
+	.suspend = max77686_suspend,
+	.resume = max77686_resume,
+};
+
+static struct i2c_driver max77686_i2c_driver = {
+	.driver = {
+		   .name = "max77686",
+		   .owner = THIS_MODULE,
+		   .pm = &max77686_pm,
+		   .of_match_table = of_match_ptr(max77686_pmic_dt_match),
+		   },
+	.probe = max77686_i2c_probe,
+	.remove = max77686_i2c_remove,
+	.id_table = max77686_i2c_id,
+};
+
+static int __init max77686_i2c_init(void)
+{
+	return i2c_add_driver(&max77686_i2c_driver);
+}
+
+subsys_initcall(max77686_i2c_init);
+
+static void __exit max77686_i2c_exit(void)
+{
+	i2c_del_driver(&max77686_i2c_driver);
+}
+
+module_exit(max77686_i2c_exit);
+
+MODULE_DESCRIPTION("MAXIM 77686 multi-function core driver");
+MODULE_AUTHOR("Chiwoong Byun <woong.byun@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/mfd/max77686-private.h b/include/linux/mfd/max77686-private.h
new file mode 100644
index 0000000..a5b5cd4
--- /dev/null
+++ b/include/linux/mfd/max77686-private.h
@@ -0,0 +1,279 @@
+/*
+ * max77686.h - Voltage regulator driver for the Maxim 77686
+ *
+ *  Copyright (C) 2011 Samsung Electrnoics
+ *  Chiwoong Byun <woong.byun@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ */
+
+#ifndef __LINUX_MFD_MAX77686_PRIV_H
+#define __LINUX_MFD_MAX77686_PRIV_H
+
+#include <linux/i2c.h>
+
+#define MAX77686_REG_INVALID		(0xff)
+#define RAMP_MASK			0xC0
+
+enum max77686_pmic_reg {
+	MAX77686_REG_DEVICE_ID		= 0x00,
+	MAX77686_REG_INTSRC		= 0x01,
+	MAX77686_REG_INT1		= 0x02,
+	MAX77686_REG_INT2		= 0x03,
+
+	MAX77686_REG_INT1MSK		= 0x04,
+	MAX77686_REG_INT2MSK		= 0x05,
+
+	MAX77686_REG_STATUS1		= 0x06,
+	MAX77686_REG_STATUS2		= 0x07,
+
+	MAX77686_REG_PWRON		= 0x08,
+	MAX77686_REG_ONOFF_DELAY	= 0x09,
+	MAX77686_REG_MRSTB		= 0x0A,
+	/* Reserved: 0x0B-0x0F */
+
+	MAX77686_REG_BUCK1CTRL		= 0x10,
+	MAX77686_REG_BUCK1OUT		= 0x11,
+	MAX77686_REG_BUCK2CTRL1		= 0x12,
+	MAX77686_REG_BUCK234FREQ	= 0x13,
+	MAX77686_REG_BUCK2DVS1		= 0x14,
+	MAX77686_REG_BUCK2DVS2		= 0x15,
+	MAX77686_REG_BUCK2DVS3		= 0x16,
+	MAX77686_REG_BUCK2DVS4		= 0x17,
+	MAX77686_REG_BUCK2DVS5		= 0x18,
+	MAX77686_REG_BUCK2DVS6		= 0x19,
+	MAX77686_REG_BUCK2DVS7		= 0x1A,
+	MAX77686_REG_BUCK2DVS8		= 0x1B,
+	MAX77686_REG_BUCK3CTRL1		= 0x1C,
+	/* Reserved: 0x1D */
+	MAX77686_REG_BUCK3DVS1		= 0x1E,
+	MAX77686_REG_BUCK3DVS2		= 0x1F,
+	MAX77686_REG_BUCK3DVS3		= 0x20,
+	MAX77686_REG_BUCK3DVS4		= 0x21,
+	MAX77686_REG_BUCK3DVS5		= 0x22,
+	MAX77686_REG_BUCK3DVS6		= 0x23,
+	MAX77686_REG_BUCK3DVS7		= 0x24,
+	MAX77686_REG_BUCK3DVS8		= 0x25,
+	MAX77686_REG_BUCK4CTRL1		= 0x26,
+	/* Reserved: 0x27 */
+	MAX77686_REG_BUCK4DVS1		= 0x28,
+	MAX77686_REG_BUCK4DVS2		= 0x29,
+	MAX77686_REG_BUCK4DVS3		= 0x2A,
+	MAX77686_REG_BUCK4DVS4		= 0x2B,
+	MAX77686_REG_BUCK4DVS5		= 0x2C,
+	MAX77686_REG_BUCK4DVS6		= 0x2D,
+	MAX77686_REG_BUCK4DVS7		= 0x2E,
+	MAX77686_REG_BUCK4DVS8		= 0x2F,
+	MAX77686_REG_BUCK5CTRL		= 0x30,
+	MAX77686_REG_BUCK5OUT		= 0x31,
+	MAX77686_REG_BUCK6CTRL		= 0x32,
+	MAX77686_REG_BUCK6OUT		= 0x33,
+	MAX77686_REG_BUCK7CTRL		= 0x34,
+	MAX77686_REG_BUCK7OUT		= 0x35,
+	MAX77686_REG_BUCK8CTRL		= 0x36,
+	MAX77686_REG_BUCK8OUT		= 0x37,
+	MAX77686_REG_BUCK9CTRL		= 0x38,
+	MAX77686_REG_BUCK9OUT		= 0x39,
+	/* Reserved: 0x3A-0x3F */
+
+	MAX77686_REG_LDO1CTRL1		= 0x40,
+	MAX77686_REG_LDO2CTRL1		= 0x41,
+	MAX77686_REG_LDO3CTRL1		= 0x42,
+	MAX77686_REG_LDO4CTRL1		= 0x43,
+	MAX77686_REG_LDO5CTRL1		= 0x44,
+	MAX77686_REG_LDO6CTRL1		= 0x45,
+	MAX77686_REG_LDO7CTRL1		= 0x46,
+	MAX77686_REG_LDO8CTRL1		= 0x47,
+	MAX77686_REG_LDO9CTRL1		= 0x48,
+	MAX77686_REG_LDO10CTRL1		= 0x49,
+	MAX77686_REG_LDO11CTRL1		= 0x4A,
+	MAX77686_REG_LDO12CTRL1		= 0x4B,
+	MAX77686_REG_LDO13CTRL1		= 0x4C,
+	MAX77686_REG_LDO14CTRL1		= 0x4D,
+	MAX77686_REG_LDO15CTRL1		= 0x4E,
+	MAX77686_REG_LDO16CTRL1		= 0x4F,
+	MAX77686_REG_LDO17CTRL1		= 0x50,
+	MAX77686_REG_LDO18CTRL1		= 0x51,
+	MAX77686_REG_LDO19CTRL1		= 0x52,
+	MAX77686_REG_LDO20CTRL1		= 0x53,
+	MAX77686_REG_LDO21CTRL1		= 0x54,
+	MAX77686_REG_LDO22CTRL1		= 0x55,
+	MAX77686_REG_LDO23CTRL1		= 0x56,
+	MAX77686_REG_LDO24CTRL1		= 0x57,
+	MAX77686_REG_LDO25CTRL1		= 0x58,
+	MAX77686_REG_LDO26CTRL1		= 0x59,
+	/* Reserved: 0x5A-0x5F */
+	MAX77686_REG_LDO1CTRL2		= 0x60,
+	MAX77686_REG_LDO2CTRL2		= 0x61,
+	MAX77686_REG_LDO3CTRL2		= 0x62,
+	MAX77686_REG_LDO4CTRL2		= 0x63,
+	MAX77686_REG_LDO5CTRL2		= 0x64,
+	MAX77686_REG_LDO6CTRL2		= 0x65,
+	MAX77686_REG_LDO7CTRL2		= 0x66,
+	MAX77686_REG_LDO8CTRL2		= 0x67,
+	MAX77686_REG_LDO9CTRL2		= 0x68,
+	MAX77686_REG_LDO10CTRL2		= 0x69,
+	MAX77686_REG_LDO11CTRL2		= 0x6A,
+	MAX77686_REG_LDO12CTRL2		= 0x6B,
+	MAX77686_REG_LDO13CTRL2		= 0x6C,
+	MAX77686_REG_LDO14CTRL2		= 0x6D,
+	MAX77686_REG_LDO15CTRL2		= 0x6E,
+	MAX77686_REG_LDO16CTRL2		= 0x6F,
+	MAX77686_REG_LDO17CTRL2		= 0x70,
+	MAX77686_REG_LDO18CTRL2		= 0x71,
+	MAX77686_REG_LDO19CTRL2		= 0x72,
+	MAX77686_REG_LDO20CTRL2		= 0x73,
+	MAX77686_REG_LDO21CTRL2		= 0x74,
+	MAX77686_REG_LDO22CTRL2		= 0x75,
+	MAX77686_REG_LDO23CTRL2		= 0x76,
+	MAX77686_REG_LDO24CTRL2		= 0x77,
+	MAX77686_REG_LDO25CTRL2		= 0x78,
+	MAX77686_REG_LDO26CTRL2		= 0x79,
+	/* Reserved: 0x7A-0x7D */
+
+	MAX77686_REG_BBAT_CHG		= 0x7E,
+	MAX77686_REG_32KHZ_		= 0x7F,
+
+	MAX77686_REG_PMIC_END		= 0x80,
+};
+
+enum max77686_rtc_reg {
+	MAX77686_RTC_INT		= 0x00,
+	MAX77686_RTC_INTM		= 0x01,
+	MAX77686_RTC_CONTROLM		= 0x02,
+	MAX77686_RTC_CONTROL		= 0x03,
+	MAX77686_RTC_UPDATE0		= 0x04,
+	/* Reserved: 0x5 */
+	MAX77686_WTSR_SMPL_CNTL		= 0x06,
+	MAX77686_RTC_SEC		= 0x07,
+	MAX77686_RTC_MIN		= 0x08,
+	MAX77686_RTC_HOUR		= 0x09,
+	MAX77686_RTC_WEEKDAY		= 0x0A,
+	MAX77686_RTC_MONTH		= 0x0B,
+	MAX77686_RTC_YEAR		= 0x0C,
+	MAX77686_RTC_DATE		= 0x0D,
+	MAX77686_ALARM1_SEC		= 0x0E,
+	MAX77686_ALARM1_MIN		= 0x0F,
+	MAX77686_ALARM1_HOUR		= 0x10,
+	MAX77686_ALARM1_WEEKDAY		= 0x11,
+	MAX77686_ALARM1_MONTH		= 0x12,
+	MAX77686_ALARM1_YEAR		= 0x13,
+	MAX77686_ALARM1_DATE		= 0x14,
+	MAX77686_ALARM2_SEC		= 0x15,
+	MAX77686_ALARM2_MIN		= 0x16,
+	MAX77686_ALARM2_HOUR		= 0x17,
+	MAX77686_ALARM2_WEEKDAY		= 0x18,
+	MAX77686_ALARM2_MONTH		= 0x19,
+	MAX77686_ALARM2_YEAR		= 0x1A,
+	MAX77686_ALARM2_DATE		= 0x1B,
+};
+
+#define MAX77686_IRQSRC_PMIC		(0)
+#define MAX77686_IRQSRC_RTC		(1 << 0)
+
+enum max77686_irq_source {
+	PMIC_INT1 = 0,
+	PMIC_INT2,
+	RTC_INT,
+
+	MAX77686_IRQ_GROUP_NR,
+};
+
+enum max77686_irq {
+	MAX77686_PMICIRQ_PWRONF = 0,
+	MAX77686_PMICIRQ_PWRONR,
+	MAX77686_PMICIRQ_JIGONBF,
+	MAX77686_PMICIRQ_JIGONBR,
+	MAX77686_PMICIRQ_ACOKBF,
+	MAX77686_PMICIRQ_ACOKBR,
+	MAX77686_PMICIRQ_ONKEY1S,
+	MAX77686_PMICIRQ_MRSTB,
+
+	MAX77686_PMICIRQ_140C,
+	MAX77686_PMICIRQ_120C,
+
+	MAX77686_RTCIRQ_RTC60S = 16,
+	MAX77686_RTCIRQ_RTCA1,
+	MAX77686_RTCIRQ_RTCA2,
+	MAX77686_RTCIRQ_SMPL,
+	MAX77686_RTCIRQ_RTC1S,
+	MAX77686_RTCIRQ_WTSR,
+
+	MAX77686_IRQ_NR,
+};
+
+struct max77686_dev {
+	struct device *dev;
+	struct i2c_client *i2c; /* 0xcc / PMIC, Battery Control, and FLASH */
+	struct i2c_client *rtc; /* slave addr 0x0c */
+	struct mutex iolock;
+	int type;
+	int irq;
+	bool wakeup;
+	struct mutex irqlock;
+	int irq_masks_cur[MAX77686_IRQ_GROUP_NR];
+	int irq_masks_cache[MAX77686_IRQ_GROUP_NR];
+	struct irq_domain *irq_domain;
+	struct max77686_platform_data *pdata;
+};
+
+extern int max77686_irq_init(struct max77686_dev *max77686);
+extern void max77686_irq_exit(struct max77686_dev *max77686);
+extern int max77686_irq_resume(struct max77686_dev *max77686);
+
+extern int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest);
+extern int max77686_bulk_read(struct i2c_client *i2c, u8 reg, int count,
+				u8 *buf);
+extern int max77686_write_reg(struct i2c_client *i2c, u8 reg, u8 value);
+extern int max77686_bulk_write(struct i2c_client *i2c, u8 reg, int count,
+				u8 *buf);
+extern int max77686_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask);
+
+extern int max77686_debug_mask;		/* enables debug prints */
+
+enum {
+	MAX77686_DEBUG_INFO = 1 << 0,
+	MAX77686_DEBUG_MASK = 1 << 1,
+	MAX77686_DEBUG_INT = 1 << 2,
+};
+
+#ifndef CONFIG_DEBUG_MAX77686
+
+#define dbg_mask(fmt, ...) do { } while (0)
+#define dbg_info(fmt, ...) do { } while (0)
+#define dbg_int(fmt, ...) do { } while (0)
+
+#else
+
+#define dbg_mask(fmt, ...)					\
+do {								\
+	if (max77686_debug_mask & MAX77686_DEBUG_MASK)		\
+		printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__);	\
+} while (0)
+
+#define dbg_info(fmt, ...)					\
+do {								\
+	if (max77686_debug_mask & MAX77686_DEBUG_INFO)		\
+		printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__);	\
+} while (0)
+
+#define dbg_int(fmt, ...)					\
+do {								\
+	if (max77686_debug_mask & MAX77686_DEBUG_INT)		\
+		printk(KERN_INFO pr_fmt(fmt), ##__VA_ARGS__);	\
+} while (0)
+#endif /* DEBUG_MAX77686 */
+
+#endif /*  __LINUX_MFD_MAX77686_PRIV_H */
diff --git a/include/linux/mfd/max77686.h b/include/linux/mfd/max77686.h
new file mode 100644
index 0000000..92baa07
--- /dev/null
+++ b/include/linux/mfd/max77686.h
@@ -0,0 +1,104 @@
+/*
+ * max77686.h - Driver for the Maxim 77686
+ *
+ *  Copyright (C) 2011 Samsung Electrnoics
+ *  Chiwoong Byun <woong.byun@samsung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * This driver is based on max8997.h
+ *
+ * MAX77686 has PMIC, RTC devices.
+ * The devices share the same I2C bus and included in
+ * this mfd driver.
+ */
+
+#ifndef __LINUX_MFD_MAX77686_H
+#define __LINUX_MFD_MAX77686_H
+
+#include <linux/regulator/consumer.h>
+
+/* MAX77686 regulator IDs */
+enum max77686_regulators {
+	MAX77686_LDO1 = 0,
+	MAX77686_LDO2,
+	MAX77686_LDO3,
+	MAX77686_LDO4,
+	MAX77686_LDO5,
+	MAX77686_LDO6,
+	MAX77686_LDO7,
+	MAX77686_LDO8,
+	MAX77686_LDO9,
+	MAX77686_LDO10,
+	MAX77686_LDO11,
+	MAX77686_LDO12,
+	MAX77686_LDO13,
+	MAX77686_LDO14,
+	MAX77686_LDO15,
+	MAX77686_LDO16,
+	MAX77686_LDO17,
+	MAX77686_LDO18,
+	MAX77686_LDO19,
+	MAX77686_LDO20,
+	MAX77686_LDO21,
+	MAX77686_LDO22,
+	MAX77686_LDO23,
+	MAX77686_LDO24,
+	MAX77686_LDO25,
+	MAX77686_LDO26,
+	MAX77686_BUCK1,
+	MAX77686_BUCK2,
+	MAX77686_BUCK3,
+	MAX77686_BUCK4,
+	MAX77686_BUCK5,
+	MAX77686_BUCK6,
+	MAX77686_BUCK7,
+	MAX77686_BUCK8,
+	MAX77686_BUCK9,
+	MAX77686_EN32KHZ_AP,
+	MAX77686_EN32KHZ_CP,
+	MAX77686_P32KH,
+
+	MAX77686_REG_MAX,
+};
+
+enum max77686_ramp_rate {
+	MAX77686_RAMP_RATE_13MV = 0,
+	MAX77686_RAMP_RATE_27MV,	/* default */
+	MAX77686_RAMP_RATE_55MV,
+	MAX77686_RAMP_RATE_100MV,
+};
+
+struct max77686_regulator_data {
+	int id;
+	struct regulator_init_data *initdata;
+	struct device_node *reg_node;
+};
+
+struct max77686_platform_data {
+	bool wakeup;
+	u8 ramp_delay;
+	struct max77686_regulator_data *regulators;
+	int num_regulators;
+	struct max77686_opmode_data *opmode_data;
+
+	/*
+	 * GPIO-DVS feature is not enabled with the current version of
+	 * MAX77686 driver. Buck2/3/4_voltages[0] is used as the default
+	 * voltage at probe.
+	 */
+};
+
+#endif /* __LINUX_MFD_MAX77686_H */
-- 
1.7.0.4

[PATCH 1/2] mfd: Add support for MAX77686.

[Mark Brown]

On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:

> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
> +{
> +	int ret;
> +
> +	ret = i2c_smbus_read_byte_data(i2c, reg);
> +	if (ret < 0)

It would really be better if this used the regmap API - the regulator
API is starting to build out functionality on top of regmap which this
won't be able to take advantage of if it doesn't use regmap.

> +	if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
> +		pd->wakeup = true;

You haven't included any binding documentatiojn for the device tree
bindings - you should do this.

> +	max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
> +	if (max77686 == NULL) {
> +		dev_err(max77686->dev, "could not allocate memory\n");
> +		return -ENOMEM;
> +	}

devm_kzalloc().

> +	device_init_wakeup(max77686->dev, max77686->wakeup);

Why is this not just done unconditionally?  There's sysfs files to allow
userspace control.

> +	if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
> +		ret = -EIO;
> +		dbg_info("%s : device not found on this channel\n", __func__);
> +		goto err_mfd;
> +	} else
> +		dev_info(max77686->dev, "device found\n");

You should verify that the device ID you read back is the expected one.

> +	dev_err(max77686->dev, "device probe failed : %d\n", ret);

Driver core should do this for you.

[PATCH 1/2] mfd: Add support for MAX77686.

[Chanwoo Choi]

Hi Mark,

We have posted following patch on the last week and received
your comment. So, We are implementing that use regmap API for I2C
and modify MFD driver of MAX77686 according to your comment.

[PATCH] MFD : add MAX77686 mfd driver
- https://lkml.org/lkml/2012/4/30/96

Additionally, We are intergrating support irq_domain for MAX77686 irq.

We will post new patch to support MFD driver of MAX77686 including
modification by your comment within this week.

Best Regards,
Chanwoo Choi

On 05/10/2012 03:27 AM, Mark Brown wrote:

> On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:
> 
>> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
>> +{
>> +	int ret;
>> +
>> +	ret = i2c_smbus_read_byte_data(i2c, reg);
>> +	if (ret < 0)
> 
> It would really be better if this used the regmap API - the regulator
> API is starting to build out functionality on top of regmap which this
> won't be able to take advantage of if it doesn't use regmap.
> 
>> +	if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
>> +		pd->wakeup = true;
> 
> You haven't included any binding documentatiojn for the device tree
> bindings - you should do this.
> 
>> +	max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
>> +	if (max77686 == NULL) {
>> +		dev_err(max77686->dev, "could not allocate memory\n");
>> +		return -ENOMEM;
>> +	}
> 
> devm_kzalloc().
> 
>> +	device_init_wakeup(max77686->dev, max77686->wakeup);
> 
> Why is this not just done unconditionally?  There's sysfs files to allow
> userspace control.
> 
>> +	if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
>> +		ret = -EIO;
>> +		dbg_info("%s : device not found on this channel\n", __func__);
>> +		goto err_mfd;
>> +	} else
>> +		dev_info(max77686->dev, "device found\n");
> 
> You should verify that the device ID you read back is the expected one.
> 
>> +	dev_err(max77686->dev, "device probe failed : %d\n", ret);
> 
> Driver core should do this for you.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

[PATCH 1/2] mfd: Add support for MAX77686.

[Yadwinder Singh Brar]

Hi Mark,

On Wed, May 9, 2012 at 11:57 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 09, 2012 at 09:54:54PM +0530, Yadwinder Singh wrote:
>
>> +int max77686_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
>> +{
>> + ? ? int ret;
>> +
>> + ? ? ret = i2c_smbus_read_byte_data(i2c, reg);
>> + ? ? if (ret < 0)
>
> It would really be better if this used the regmap API - the regulator
> API is starting to build out functionality on top of regmap which this
> won't be able to take advantage of if it doesn't use regmap.
>

I agree, I will implement it.

>> + ? ? if (of_get_property(dev->of_node, "max77686,wakeup", NULL))
>> + ? ? ? ? ? ? pd->wakeup = true;
>
> You haven't included any binding documentatiojn for the device tree
> bindings - you should do this.
>

Ok. I will add documentation in next version of patch.

>> + ? ? max77686 = kzalloc(sizeof(struct max77686_dev), GFP_KERNEL);
>> + ? ? if (max77686 == NULL) {
>> + ? ? ? ? ? ? dev_err(max77686->dev, "could not allocate memory\n");
>> + ? ? ? ? ? ? return -ENOMEM;
>> + ? ? }
>
> devm_kzalloc().
>

yes, its better option.

>> + ? ? device_init_wakeup(max77686->dev, max77686->wakeup);
>
> Why is this not just done unconditionally? ?There's sysfs files to allow
> userspace control.
>

yes, i agree with you. i will do it.

>> + ? ? if (max77686_read_reg(i2c, MAX77686_REG_DEVICE_ID, &data) < 0) {
>> + ? ? ? ? ? ? ret = -EIO;
>> + ? ? ? ? ? ? dbg_info("%s : device not found on this channel\n", __func__);
>> + ? ? ? ? ? ? goto err_mfd;
>> + ? ? } else
>> + ? ? ? ? ? ? dev_info(max77686->dev, "device found\n");
>
> You should verify that the device ID you read back is the expected one.
>

Ok, I will do that also.

>> + ? ? dev_err(max77686->dev, "device probe failed : %d\n", ret);
>
> Driver core should do this for you.
>

Yes, I will remove this message.

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Thanks,
Yadwinder.

[PATCH 2/2] regulator: Add support for MAX77686.

[Yadwinder Singh]

From: Yadwinder Singh Brar <yadi.brar@samsung.com>

Add support for PMIC/regulator portion of MAX77686 multifunction device.
MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driver
which supports setting and getting the voltage of a regulator with I2C
interface.

Signed-off-by: Yadwinder Singh Brar <yadi.brar@samsung.com>
---
 drivers/regulator/Kconfig    |    9 +
 drivers/regulator/Makefile   |    1 +
 drivers/regulator/max77686.c |  660 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 670 insertions(+), 0 deletions(-)
 create mode 100644 drivers/regulator/max77686.c

diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 36db5a4..67162cc 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -195,6 +195,15 @@ config REGULATOR_MAX8998
 	  via I2C bus. The provided regulator is suitable for S3C6410
 	  and S5PC1XX chips to control VCC_CORE and VCC_USIM voltages.
 
+config REGULATOR_MAX77686
+	tristate "Maxim 77686 regulator"
+	depends on MFD_MAX77686
+	help
+	  This driver controls a Maxim 77686 voltage regulator via I2C
+	  bus. The provided regulator is suitable for Exynos5 chips to
+	  control VDD_ARM and VDD_INT voltages.It supports LDOs[1-26]
+	  and BUCKs[1-9].
+
 config REGULATOR_PCAP
 	tristate "Motorola PCAP2 regulator driver"
 	depends on EZX_PCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index 94b5274..008931b 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -30,6 +30,7 @@ obj-$(CONFIG_REGULATOR_MAX8925) += max8925-regulator.o
 obj-$(CONFIG_REGULATOR_MAX8952) += max8952.o
 obj-$(CONFIG_REGULATOR_MAX8997) += max8997.o
 obj-$(CONFIG_REGULATOR_MAX8998) += max8998.o
+obj-$(CONFIG_REGULATOR_MAX77686) += max77686.o
 obj-$(CONFIG_REGULATOR_MC13783) += mc13783-regulator.o
 obj-$(CONFIG_REGULATOR_MC13892) += mc13892-regulator.o
 obj-$(CONFIG_REGULATOR_MC13XXX_CORE) +=  mc13xxx-regulator-core.o
diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
new file mode 100644
index 0000000..4aa9722
--- /dev/null
+++ b/drivers/regulator/max77686.c
@@ -0,0 +1,660 @@
+/*
+ * max77686.c - Regulator driver for the Maxim 77686
+ *
+ * Copyright (C) 2012 Samsung Electronics
+ * Chiwoong Byun <woong.byun@smasung.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+ *
+ * This driver is based on max8997.c
+ */
+
+#include <linux/module.h>
+#include <linux/bug.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/gpio.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/regulator/driver.h>
+#include <linux/regulator/machine.h>
+#include <linux/regulator/of_regulator.h>
+#include <linux/mfd/max77686.h>
+#include <linux/mfd/max77686-private.h>
+
+struct max77686_data {
+	struct device *dev;
+	struct max77686_dev *iodev;
+	int num_regulators;
+	struct regulator_dev **rdev;
+	int ramp_delay;		/* index of ramp_delay */
+
+	/*GPIO-DVS feature is not enabled with the
+	 *current version of MAX77686 driver.*/
+};
+
+struct voltage_map_desc {
+	int min;
+	int max;
+	int step;
+	unsigned int n_bits;
+};
+
+/* Voltage maps in mV */
+static const struct voltage_map_desc ldo_voltage_map_desc = {
+	.min = 800,	.max = 3950,	.step = 50,	.n_bits = 6,
+};				/* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */
+
+static const struct voltage_map_desc ldo_low_voltage_map_desc = {
+	.min = 800,	.max = 2375,	.step = 25,	.n_bits = 6,
+};				/* LDO1 ~ 2, 6 ~ 8, 15 */
+
+static const struct voltage_map_desc buck_dvs_voltage_map_desc = {
+	.min = 600000,	.max = 3787500,	.step = 12500,	.n_bits = 8,
+};				/* Buck2, 3, 4 (uV) */
+
+static const struct voltage_map_desc buck_voltage_map_desc = {
+	.min = 750,	.max = 3900,	.step = 50,	.n_bits = 6,
+};				/* Buck1, 5 ~ 9 */
+
+static const struct voltage_map_desc *reg_voltage_map[] = {
+	[MAX77686_LDO1] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO2] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO3] = &ldo_voltage_map_desc,
+	[MAX77686_LDO4] = &ldo_voltage_map_desc,
+	[MAX77686_LDO5] = &ldo_voltage_map_desc,
+	[MAX77686_LDO6] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO7] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO8] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO9] = &ldo_voltage_map_desc,
+	[MAX77686_LDO10] = &ldo_voltage_map_desc,
+	[MAX77686_LDO11] = &ldo_voltage_map_desc,
+	[MAX77686_LDO12] = &ldo_voltage_map_desc,
+	[MAX77686_LDO13] = &ldo_voltage_map_desc,
+	[MAX77686_LDO14] = &ldo_voltage_map_desc,
+	[MAX77686_LDO15] = &ldo_low_voltage_map_desc,
+	[MAX77686_LDO16] = &ldo_voltage_map_desc,
+	[MAX77686_LDO17] = &ldo_voltage_map_desc,
+	[MAX77686_LDO18] = &ldo_voltage_map_desc,
+	[MAX77686_LDO19] = &ldo_voltage_map_desc,
+	[MAX77686_LDO20] = &ldo_voltage_map_desc,
+	[MAX77686_LDO21] = &ldo_voltage_map_desc,
+	[MAX77686_LDO22] = &ldo_voltage_map_desc,
+	[MAX77686_LDO23] = &ldo_voltage_map_desc,
+	[MAX77686_LDO24] = &ldo_voltage_map_desc,
+	[MAX77686_LDO25] = &ldo_voltage_map_desc,
+	[MAX77686_LDO26] = &ldo_voltage_map_desc,
+	[MAX77686_BUCK1] = &buck_voltage_map_desc,
+	[MAX77686_BUCK2] = &buck_dvs_voltage_map_desc,
+	[MAX77686_BUCK3] = &buck_dvs_voltage_map_desc,
+	[MAX77686_BUCK4] = &buck_dvs_voltage_map_desc,
+	[MAX77686_BUCK5] = &buck_voltage_map_desc,
+	[MAX77686_BUCK6] = &buck_voltage_map_desc,
+	[MAX77686_BUCK7] = &buck_voltage_map_desc,
+	[MAX77686_BUCK8] = &buck_voltage_map_desc,
+	[MAX77686_BUCK9] = &buck_voltage_map_desc,
+	[MAX77686_EN32KHZ_AP] = NULL,
+	[MAX77686_EN32KHZ_CP] = NULL,
+};
+
+static int max77686_get_voltage_unit(int rid)
+{
+	int unit = 0;
+
+	switch (rid) {
+	case MAX77686_BUCK2...MAX77686_BUCK4:
+		unit = 1;	/* BUCK2,3,4 is uV */
+		break;
+	default:
+		unit = 1000;
+		break;
+	}
+
+	return unit;
+}
+
+static int max77686_list_voltage(struct regulator_dev *rdev,
+				 unsigned int selector)
+{
+	const struct voltage_map_desc *desc;
+	int rid = rdev_get_id(rdev);
+	int val;
+
+	if (rid >= ARRAY_SIZE(reg_voltage_map) || rid < 0)
+		return -EINVAL;
+
+	desc = reg_voltage_map[rid];
+	if (desc == NULL)
+		return -EINVAL;
+
+	val = desc->min + desc->step * selector;
+	if (val > desc->max)
+		return -EINVAL;
+
+	return val * max77686_get_voltage_unit(rid);
+}
+
+static int max77686_get_enable_register(struct regulator_dev *rdev,
+					int *reg, int *mask, int *pattern)
+{
+	int rid = rdev_get_id(rdev);
+
+	switch (rid) {
+	case MAX77686_LDO1...MAX77686_LDO26:
+		*reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
+		*mask = 0xC0;
+		*pattern = 0xC0;
+		break;
+	case MAX77686_BUCK1:
+		*reg = MAX77686_REG_BUCK1CTRL;
+		*mask = 0x03;
+		*pattern = 0x03;
+		break;
+	case MAX77686_BUCK2:
+		*reg = MAX77686_REG_BUCK2CTRL1;
+		*mask = 0x30;
+		*pattern = 0x10;
+		break;
+	case MAX77686_BUCK3:
+		*reg = MAX77686_REG_BUCK3CTRL1;
+		*mask = 0x30;
+		*pattern = 0x10;
+		break;
+	case MAX77686_BUCK4:
+		*reg = MAX77686_REG_BUCK4CTRL1;
+		*mask = 0x30;
+		*pattern = 0x10;
+		break;
+	case MAX77686_BUCK5...MAX77686_BUCK9:
+		*reg = MAX77686_REG_BUCK5CTRL + (rid - MAX77686_BUCK5) * 2;
+		*mask = 0x03;
+		*pattern = 0x03;
+		break;
+	case MAX77686_EN32KHZ_AP...MAX77686_EN32KHZ_CP:
+		*reg = MAX77686_REG_32KHZ_;
+		*mask = 0x01 << (rid - MAX77686_EN32KHZ_AP);
+		*pattern = 0x01 << (rid - MAX77686_EN32KHZ_AP);
+		break;
+	default:
+		/* Not controllable or not exists */
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int max77686_reg_is_enabled(struct regulator_dev *rdev)
+{
+	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max77686->iodev->i2c;
+	int ret, reg, mask, pattern;
+	u8 val;
+
+	ret = max77686_get_enable_register(rdev, &reg, &mask, &pattern);
+	if (ret)
+		return ret;
+
+	ret = max77686_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	return (val & mask) == pattern;
+}
+
+static int max77686_reg_enable(struct regulator_dev *rdev)
+{
+	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max77686->iodev->i2c;
+	int ret, reg, mask, pattern;
+
+	ret = max77686_get_enable_register(rdev, &reg, &mask, &pattern);
+	if (ret)
+		return ret;
+
+	return max77686_update_reg(i2c, reg, pattern, mask);
+}
+
+static int max77686_reg_disable(struct regulator_dev *rdev)
+{
+	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max77686->iodev->i2c;
+	int ret, reg, mask, pattern;
+
+	ret = max77686_get_enable_register(rdev, &reg, &mask, &pattern);
+	if (ret)
+		return ret;
+
+	return max77686_update_reg(i2c, reg, ~mask, mask);
+}
+
+static int max77686_get_voltage_register(struct regulator_dev *rdev,
+					 int *_reg, int *_shift, int *_mask)
+{
+	int rid = rdev_get_id(rdev);
+	int reg, shift = 0, mask = 0x3f;
+
+	switch (rid) {
+	case MAX77686_LDO1...MAX77686_LDO26:
+		reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
+		break;
+	case MAX77686_BUCK1:
+		reg = MAX77686_REG_BUCK1OUT;
+		break;
+	case MAX77686_BUCK2:
+		reg = MAX77686_REG_BUCK2DVS1;
+		mask = 0xff;
+		break;
+	case MAX77686_BUCK3:
+		reg = MAX77686_REG_BUCK3DVS1;
+		mask = 0xff;
+		break;
+	case MAX77686_BUCK4:
+		reg = MAX77686_REG_BUCK4DVS1;
+		mask = 0xff;
+		break;
+	case MAX77686_BUCK5...MAX77686_BUCK9:
+		reg = MAX77686_REG_BUCK5OUT + (rid - MAX77686_BUCK5) * 2;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*_reg = reg;
+	*_shift = shift;
+	*_mask = mask;
+
+	return 0;
+}
+
+static int max77686_get_voltage(struct regulator_dev *rdev)
+{
+	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max77686->iodev->i2c;
+	int reg, shift, mask, ret;
+	u8 val;
+
+	ret = max77686_get_voltage_register(rdev, &reg, &shift, &mask);
+	if (ret)
+		return ret;
+
+	ret = max77686_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val >>= shift;
+	val &= mask;
+
+	return max77686_list_voltage(rdev, val);
+}
+
+static inline int max77686_get_voltage_proper_val(const struct voltage_map_desc
+						  *desc, int min_vol,
+						  int max_vol)
+{
+	int i = 0;
+
+	if (desc == NULL)
+		return -EINVAL;
+
+	if (max_vol < desc->min || min_vol > desc->max)
+		return -EINVAL;
+
+	while (desc->min + desc->step * i < min_vol &&
+	       desc->min + desc->step * i < desc->max)
+		i++;
+
+	if (desc->min + desc->step * i > max_vol)
+		return -EINVAL;
+
+	if (i >= (1 << desc->n_bits))
+		return -EINVAL;
+
+	return i;
+}
+
+static int max77686_set_voltage(struct regulator_dev *rdev,
+				int min_uV, int max_uV, unsigned *selector)
+{
+	struct max77686_data *max77686 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max77686->iodev->i2c;
+	int min_vol = min_uV, max_vol = max_uV, unit = 0;
+	const struct voltage_map_desc *desc;
+	int rid = rdev_get_id(rdev);
+	int reg, shift = 0, mask, ret;
+	int i;
+	int ramp[] = {13, 27, 57, 100};	/* ramp_rate in mV/uS */
+	u8 org;
+
+	unit = max77686_get_voltage_unit(rid);
+	min_vol /= unit;
+	max_vol /= unit;
+
+	desc = reg_voltage_map[rid];
+
+	i = max77686_get_voltage_proper_val(desc, min_vol, max_vol);
+	if (i < 0)
+		return i;
+
+	ret = max77686_get_voltage_register(rdev, &reg, &shift, &mask);
+	if (ret)
+		return ret;
+
+	max77686_read_reg(i2c, reg, &org);
+	org = (org & mask) >> shift;
+
+	ret = max77686_update_reg(i2c, reg, i << shift, mask << shift);
+	*selector = i;
+
+	if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
+	    rid == MAX77686_BUCK4) {
+		/* If the voltage is increasing */
+		if (org < i)
+			udelay(DIV_ROUND_UP(desc->step * (i - org),
+					    ramp[max77686->ramp_delay]));
+	}
+
+	return ret;
+}
+
+static struct regulator_ops max77686_ops = {
+	.list_voltage = max77686_list_voltage,
+	.is_enabled = max77686_reg_is_enabled,
+	.enable = max77686_reg_enable,
+	.disable = max77686_reg_disable,
+	.get_voltage = max77686_get_voltage,
+	.set_voltage = max77686_set_voltage,
+	.set_suspend_enable = max77686_reg_enable,
+	.set_suspend_disable = max77686_reg_disable,
+};
+
+static struct regulator_ops max77686_fixedvolt_ops = {
+	.list_voltage = max77686_list_voltage,
+	.is_enabled = max77686_reg_is_enabled,
+	.enable = max77686_reg_enable,
+	.disable = max77686_reg_disable,
+	.set_suspend_enable = max77686_reg_enable,
+	.set_suspend_disable = max77686_reg_disable,
+};
+
+#define regulator_desc_ldo(num)		{	\
+	.name		= "LDO"#num,		\
+	.id		= MAX77686_LDO##num,	\
+	.ops		= &max77686_ops,	\
+	.type		= REGULATOR_VOLTAGE,	\
+	.owner		= THIS_MODULE,		\
+}
+#define regulator_desc_buck(num)		{	\
+	.name		= "BUCK"#num,		\
+	.id		= MAX77686_BUCK##num,	\
+	.ops		= &max77686_ops,	\
+	.type		= REGULATOR_VOLTAGE,	\
+	.owner		= THIS_MODULE,		\
+}
+
+static struct regulator_desc regulators[] = {
+	regulator_desc_ldo(1),
+	regulator_desc_ldo(2),
+	regulator_desc_ldo(3),
+	regulator_desc_ldo(4),
+	regulator_desc_ldo(5),
+	regulator_desc_ldo(6),
+	regulator_desc_ldo(7),
+	regulator_desc_ldo(8),
+	regulator_desc_ldo(9),
+	regulator_desc_ldo(10),
+	regulator_desc_ldo(11),
+	regulator_desc_ldo(12),
+	regulator_desc_ldo(13),
+	regulator_desc_ldo(14),
+	regulator_desc_ldo(15),
+	regulator_desc_ldo(16),
+	regulator_desc_ldo(17),
+	regulator_desc_ldo(18),
+	regulator_desc_ldo(19),
+	regulator_desc_ldo(20),
+	regulator_desc_ldo(21),
+	regulator_desc_ldo(22),
+	regulator_desc_ldo(23),
+	regulator_desc_ldo(24),
+	regulator_desc_ldo(25),
+	regulator_desc_ldo(26),
+	regulator_desc_buck(1),
+	regulator_desc_buck(2),
+	regulator_desc_buck(3),
+	regulator_desc_buck(4),
+	regulator_desc_buck(5),
+	regulator_desc_buck(6),
+	regulator_desc_buck(7),
+	regulator_desc_buck(8),
+	regulator_desc_buck(9),
+	{
+	 .name = "EN32KHz_AP",
+	 .id = MAX77686_EN32KHZ_AP,
+	 .ops = &max77686_fixedvolt_ops,
+	 .type = REGULATOR_VOLTAGE,
+	 .owner = THIS_MODULE,
+	 }, {
+	     .name = "EN32KHz_CP",
+	     .id = MAX77686_EN32KHZ_CP,
+	     .ops = &max77686_fixedvolt_ops,
+	     .type = REGULATOR_VOLTAGE,
+	     .owner = THIS_MODULE,
+	     },
+};
+
+#ifdef CONFIG_OF
+static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
+					struct max77686_platform_data *pdata)
+{
+	struct device_node *pmic_np, *regulators_np, *reg_np;
+	struct max77686_regulator_data *rdata;
+	unsigned int i;
+
+	pmic_np = iodev->dev->of_node;
+	if (!pmic_np) {
+		dev_err(iodev->dev, "could not find pmic sub-node\n");
+		return -ENODEV;
+	}
+
+	regulators_np = of_find_node_by_name(pmic_np, "voltage-regulators");
+	if (!regulators_np) {
+		dev_err(iodev->dev, "could not find regulators sub-node\n");
+		return -EINVAL;
+	}
+
+/* count the number of regulators to be supported in pmic */
+	pdata->num_regulators = 0;
+	for_each_child_of_node(regulators_np, reg_np)
+		pdata->num_regulators++;
+
+	rdata = devm_kzalloc(iodev->dev, sizeof(*rdata) *
+				pdata->num_regulators, GFP_KERNEL);
+	if (!rdata) {
+		dev_err(iodev->dev,
+			"could not allocate memory for regulator data\n");
+		return -ENOMEM;
+	}
+
+	pdata->regulators = rdata;
+	for_each_child_of_node(regulators_np, reg_np) {
+		for (i = 0; i < ARRAY_SIZE(regulators); i++)
+			if (!of_node_cmp(reg_np->name, regulators[i].name))
+				break;
+
+		if (i == ARRAY_SIZE(regulators)) {
+			dev_warn(iodev->dev,
+				"No configuration data for regulator %s\n",
+				reg_np->name);
+			continue;
+		}
+
+		rdata->id = i;
+		rdata->initdata = of_get_regulator_init_data(
+						iodev->dev, reg_np);
+		rdata->reg_node = reg_np;
+		rdata++;
+	}
+
+	if (of_property_read_u32(pmic_np,
+		"max77686,buck_ramp_delay", &i))
+		pdata->ramp_delay = i & 0xff;
+
+	return 0;
+}
+#else
+static int max77686_pmic_dt_parse_pdata(struct max77686_dev *iodev,
+					struct max77686_platform_data *pdata)
+{
+	return 0;
+}
+#endif	/* CONFIG_OF */
+
+#define RAMP_VALUE (max77686->ramp_delay << 6)
+
+static __devinit int max77686_pmic_probe(struct platform_device *pdev)
+{
+	struct max77686_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max77686_platform_data *pdata = iodev->pdata;
+	struct regulator_dev **rdev;
+	struct max77686_data *max77686;
+	struct i2c_client *i2c = iodev->i2c;
+	int i, ret, size;
+
+	if (iodev->dev->of_node) {
+		ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
+		if (ret)
+			return ret;
+	}
+
+	if (!pdata) {
+		dev_err(&pdev->dev, "platform data not found\n");
+		return -ENODEV;
+	}
+
+	max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
+	if (!max77686)
+		return -ENOMEM;
+
+	size = sizeof(struct regulator_dev *) * pdata->num_regulators;
+	max77686->rdev = kzalloc(size, GFP_KERNEL);
+	if (!max77686->rdev) {
+		kfree(max77686);
+		return -ENOMEM;
+	}
+
+	rdev = max77686->rdev;
+
+	max77686->dev = &pdev->dev;
+	max77686->iodev = iodev;
+	max77686->num_regulators = pdata->num_regulators;
+
+	if (pdata->ramp_delay) {
+		max77686->ramp_delay = pdata->ramp_delay;
+		max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
+			RAMP_VALUE, RAMP_MASK);
+		max77686_update_reg(i2c, MAX77686_REG_BUCK3CTRL1,
+			RAMP_VALUE, RAMP_MASK);
+		max77686_update_reg(i2c, MAX77686_REG_BUCK4CTRL1,
+			RAMP_VALUE, RAMP_MASK);
+	} else {
+		/* Default/Reset value is 27.5 mV/uS */
+		max77686->ramp_delay = MAX77686_RAMP_RATE_27MV;
+	}
+
+	platform_set_drvdata(pdev, max77686);
+
+	for (i = 0; i < pdata->num_regulators; i++) {
+		const struct voltage_map_desc *desc;
+		int id = pdata->regulators[i].id;
+
+		desc = reg_voltage_map[id];
+		if (desc)
+			regulators[id].n_voltages =
+			    (desc->max - desc->min) / desc->step + 1;
+
+		rdev[i] = regulator_register(&regulators[id], max77686->dev,
+					     pdata->regulators[i].initdata,
+					     max77686, NULL);
+		if (IS_ERR(rdev[i])) {
+			ret = PTR_ERR(rdev[i]);
+			dev_err(max77686->dev,
+				"regulator init failed for id : %d\n", id);
+			rdev[i] = NULL;
+			goto err;
+		}
+	}
+
+	return 0;
+ err:
+	for (i = 0; i < max77686->num_regulators; i++)
+		if (rdev[i])
+			regulator_unregister(rdev[i]);
+
+	kfree(max77686->rdev);
+	kfree(max77686);
+
+	return ret;
+}
+
+static int __devexit max77686_pmic_remove(struct platform_device *pdev)
+{
+	struct max77686_data *max77686 = platform_get_drvdata(pdev);
+	struct regulator_dev **rdev = max77686->rdev;
+	int i;
+
+	for (i = 0; i < max77686->num_regulators; i++)
+		if (rdev[i])
+			regulator_unregister(rdev[i]);
+
+	kfree(max77686->rdev);
+	kfree(max77686);
+
+	return 0;
+}
+
+static const struct platform_device_id max77686_pmic_id[] = {
+	{"max77686-pmic", 0},
+	{},
+};
+
+MODULE_DEVICE_TABLE(platform, max77686_pmic_id);
+
+static struct platform_driver max77686_pmic_driver = {
+	.driver = {
+		   .name = "max77686-pmic",
+		   .owner = THIS_MODULE,
+		   },
+	.probe = max77686_pmic_probe,
+	.remove = __devexit_p(max77686_pmic_remove),
+	.id_table = max77686_pmic_id,
+};
+
+static int __init max77686_pmic_init(void)
+{
+	return platform_driver_register(&max77686_pmic_driver);
+}
+
+subsys_initcall(max77686_pmic_init);
+
+static void __exit max77686_pmic_cleanup(void)
+{
+	platform_driver_unregister(&max77686_pmic_driver);
+}
+
+module_exit(max77686_pmic_cleanup);
+
+MODULE_DESCRIPTION("MAXIM 77686 Regulator Driver");
+MODULE_AUTHOR("Chiwoong Byun <woong.byun@samsung.com>");
+MODULE_LICENSE("GPL");
-- 
1.7.0.4

[PATCH 2/2] regulator: Add support for MAX77686.

[Mark Brown]

On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:

> +/* Voltage maps in mV */
> +static const struct voltage_map_desc ldo_voltage_map_desc = {
> +	.min = 800,	.max = 3950,	.step = 50,	.n_bits = 6,
> +};				/* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */

Hrm, funnily enough I was just thinking about factoring this stuff out
into the core after a conversation with Graeme Gregory the other week.
Let's do that...

> +	[MAX77686_EN32KHZ_AP] = NULL,
> +	[MAX77686_EN32KHZ_CP] = NULL,

Now that the generic clock API is in mainline these should be moved over
to use it.

> +static int max77686_get_voltage_unit(int rid)
> +{
> +	int unit = 0;
> +
> +	switch (rid) {
> +	case MAX77686_BUCK2...MAX77686_BUCK4:
> +		unit = 1;	/* BUCK2,3,4 is uV */
> +		break;
> +	default:
> +		unit = 1000;

Why not just list everything in uV?

> +static int max77686_get_voltage(struct regulator_dev *rdev)
> +{

Implement get_voltage_sel().

> +static inline int max77686_get_voltage_proper_val(const struct voltage_map_desc
> +						  *desc, int min_vol,
> +						  int max_vol)
> +{
> +	int i = 0;
> +
> +	if (desc == NULL)
> +		return -EINVAL;
> +
> +	if (max_vol < desc->min || min_vol > desc->max)
> +		return -EINVAL;
> +
> +	while (desc->min + desc->step * i < min_vol &&
> +	       desc->min + desc->step * i < desc->max)
> +		i++;

Why are you iterating here?  Calculate!  Though like I say let's factor
this out anyway.

> +	if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
> +	    rid == MAX77686_BUCK4) {
> +		/* If the voltage is increasing */
> +		if (org < i)
> +			udelay(DIV_ROUND_UP(desc->step * (i - org),
> +					    ramp[max77686->ramp_delay]));
> +	}

Don't do this, implement set_voltage_time_sel().

> +	.enable = max77686_reg_enable,
> +	.disable = max77686_reg_disable,
> +	.set_suspend_enable = max77686_reg_enable,
> +	.set_suspend_disable = max77686_reg_disable,

You've got the same ops for suspend and non-suspend cases here, this is
clearly buggy.

> +/* count the number of regulators to be supported in pmic */
> +	pdata->num_regulators = 0;

Coding style.

> +	if (iodev->dev->of_node) {
> +		ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
> +		if (ret)
> +			return ret;

This ought to use of_regulator_match().

> +	}
> +
> +	if (!pdata) {
> +		dev_err(&pdev->dev, "platform data not found\n");
> +		return -ENODEV;
> +	}

This should be totally fine.

> +	max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
> +	if (!max77686)
> +		return -ENOMEM;

devm_kzalloc().

> +	if (pdata->ramp_delay) {
> +		max77686->ramp_delay = pdata->ramp_delay;
> +		max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
> +			RAMP_VALUE, RAMP_MASK);

This appears not to actually use the value passed in as platform_data.

> +
> +	for (i = 0; i < pdata->num_regulators; i++) {
> +		const struct voltage_map_desc *desc;
> +		int id = pdata->regulators[i].id;
> +
> +		desc = reg_voltage_map[id];
> +		if (desc)
> +			regulators[id].n_voltages =
> +			    (desc->max - desc->min) / desc->step + 1;
> +
> +		rdev[i] = regulator_register(&regulators[id], max77686->dev,
> +					     pdata->regulators[i].initdata,
> +					     max77686, NULL);

No, you should unconditionally register all regulators the device
physically has.  This is useful for debug and simplifies the code.

[PATCH 2/2] regulator: Add support for MAX77686.

[Yadwinder Singh Brar]

Hi Mark,

On Thu, May 10, 2012 at 12:17 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
>
>> +/* Voltage maps in mV */
>> +static const struct voltage_map_desc ldo_voltage_map_desc = {
>> + ? ? .min = 800, ? ? .max = 3950, ? ?.step = 50, ? ? .n_bits = 6,
>> +}; ? ? ? ? ? ? ? ? ? ? ? ? ? /* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */
>
> Hrm, funnily enough I was just thinking about factoring this stuff out
> into the core after a conversation with Graeme Gregory the other week.
> Let's do that...
>
>> + ? ? [MAX77686_EN32KHZ_AP] = NULL,
>> + ? ? [MAX77686_EN32KHZ_CP] = NULL,
>
> Now that the generic clock API is in mainline these should be moved over
> to use it.
>

Sorry, I cann't get your point here. Please explain it little bit more.

>> +static int max77686_get_voltage_unit(int rid)
>> +{
>> + ? ? int unit = 0;
>> +
>> + ? ? switch (rid) {
>> + ? ? case MAX77686_BUCK2...MAX77686_BUCK4:
>> + ? ? ? ? ? ? unit = 1; ? ? ? /* BUCK2,3,4 is uV */
>> + ? ? ? ? ? ? break;
>> + ? ? default:
>> + ? ? ? ? ? ? unit = 1000;
>
> Why not just list everything in uV?
>

Yes, everything should be in uV, I will correct it.

>> +static int max77686_get_voltage(struct regulator_dev *rdev)
>> +{
>
> Implement get_voltage_sel().
>
>> +static inline int max77686_get_voltage_proper_val(const struct voltage_map_desc
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *desc, int min_vol,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int max_vol)
>> +{
>> + ? ? int i = 0;
>> +
>> + ? ? if (desc == NULL)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? if (max_vol < desc->min || min_vol > desc->max)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? while (desc->min + desc->step * i < min_vol &&
>> + ? ? ? ? ? ?desc->min + desc->step * i < desc->max)
>> + ? ? ? ? ? ? i++;
>
> Why are you iterating here? ?Calculate! ?Though like I say let's factor
> this out anyway.
>

Yes, I will do it.

>> + ? ? if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
>> + ? ? ? ? rid == MAX77686_BUCK4) {
>> + ? ? ? ? ? ? /* If the voltage is increasing */
>> + ? ? ? ? ? ? if (org < i)
>> + ? ? ? ? ? ? ? ? ? ? udelay(DIV_ROUND_UP(desc->step * (i - org),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ramp[max77686->ramp_delay]));
>> + ? ? }
>
> Don't do this, implement set_voltage_time_sel().
>

Ok, I will implement it.

>> + ? ? .enable = max77686_reg_enable,
>> + ? ? .disable = max77686_reg_disable,
>> + ? ? .set_suspend_enable = max77686_reg_enable,
>> + ? ? .set_suspend_disable = max77686_reg_disable,
>
> You've got the same ops for suspend and non-suspend cases here, this is
> clearly buggy.
>
>> +/* count the number of regulators to be supported in pmic */
>> + ? ? pdata->num_regulators = 0;
>
> Coding style.
>
>> + ? ? if (iodev->dev->of_node) {
>> + ? ? ? ? ? ? ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
>> + ? ? ? ? ? ? if (ret)
>> + ? ? ? ? ? ? ? ? ? ? return ret;
>
> This ought to use of_regulator_match().
>

Ok, I will look into it.

>> + ? ? }
>> +
>> + ? ? if (!pdata) {
>> + ? ? ? ? ? ? dev_err(&pdev->dev, "platform data not found\n");
>> + ? ? ? ? ? ? return -ENODEV;
>> + ? ? }
>
> This should be totally fine.
>

I will look into it.

>> + ? ? max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
>> + ? ? if (!max77686)
>> + ? ? ? ? ? ? return -ENOMEM;
>
> devm_kzalloc().
>

Yes, its better option.

>> + ? ? if (pdata->ramp_delay) {
>> + ? ? ? ? ? ? max77686->ramp_delay = pdata->ramp_delay;
>> + ? ? ? ? ? ? max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> + ? ? ? ? ? ? ? ? ? ? RAMP_VALUE, RAMP_MASK);
>
> This appears not to actually use the value passed in as platform_data.
>

It gets corresponding index of ramp_rate value in ramp_rate_value
table supported by hardware, from platform_data which we write to
ramp_rate control bits of control registers.

>> +
>> + ? ? for (i = 0; i < pdata->num_regulators; i++) {
>> + ? ? ? ? ? ? const struct voltage_map_desc *desc;
>> + ? ? ? ? ? ? int id = pdata->regulators[i].id;
>> +
>> + ? ? ? ? ? ? desc = reg_voltage_map[id];
>> + ? ? ? ? ? ? if (desc)
>> + ? ? ? ? ? ? ? ? ? ? regulators[id].n_voltages =
>> + ? ? ? ? ? ? ? ? ? ? ? ? (desc->max - desc->min) / desc->step + 1;
>> +
>> + ? ? ? ? ? ? rdev[i] = regulator_register(&regulators[id], max77686->dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pdata->regulators[i].initdata,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?max77686, NULL);
>
> No, you should unconditionally register all regulators the device
> physically has. ?This is useful for debug and simplifies the code.
>

Ok. I will do it.

> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Thanks,
Yadwinder.

[PATCH 2/2] regulator: Add support for MAX77686.

[Mark Brown]

On Thu, May 10, 2012 at 12:54:24PM +0530, Yadwinder Singh Brar wrote:
> On Thu, May 10, 2012 at 12:17 AM, Mark Brown
> > On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:

> >> + ? ? [MAX77686_EN32KHZ_AP] = NULL,
> >> + ? ? [MAX77686_EN32KHZ_CP] = NULL,

> > Now that the generic clock API is in mainline these should be moved over
> > to use it.

> Sorry, I cann't get your point here. Please explain it little bit more.

These are not regulators, these are clocks.  They should use the clock
API.

> >> + ? ? if (pdata->ramp_delay) {
> >> + ? ? ? ? ? ? max77686->ramp_delay = pdata->ramp_delay;
> >> + ? ? ? ? ? ? max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
> >> + ? ? ? ? ? ? ? ? ? ? RAMP_VALUE, RAMP_MASK);

> > This appears not to actually use the value passed in as platform_data.

> It gets corresponding index of ramp_rate value in ramp_rate_value
> table supported by hardware, from platform_data which we write to
> ramp_rate control bits of control registers.

Why is the driver unconditionally writing these register values here
rather than setting the ramp delay that was passed in?
-------------- 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/20120510/52a8cef0/attachment-0001.sig>

[PATCH 2/2] regulator: Add support for MAX77686.

[Yadwinder Singh Brar]

Hi Mark,

On Thu, May 10, 2012 at 3:04 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, May 10, 2012 at 12:54:24PM +0530, Yadwinder Singh Brar wrote:
>> On Thu, May 10, 2012 at 12:17 AM, Mark Brown
>> > On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
>
>> >> + ? ? [MAX77686_EN32KHZ_AP] = NULL,
>> >> + ? ? [MAX77686_EN32KHZ_CP] = NULL,
>
>> > Now that the generic clock API is in mainline these should be moved over
>> > to use it.
>
>> Sorry, I cann't get your point here. Please explain it little bit more.
>
> These are not regulators, these are clocks. ?They should use the clock
> API.
>

Ok. I got it.

>> >> + ? ? if (pdata->ramp_delay) {
>> >> + ? ? ? ? ? ? max77686->ramp_delay = pdata->ramp_delay;
>> >> + ? ? ? ? ? ? max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> >> + ? ? ? ? ? ? ? ? ? ? RAMP_VALUE, RAMP_MASK);
>
>> > This appears not to actually use the value passed in as platform_data.
>
>> It gets corresponding index of ramp_rate value in ramp_rate_value
>> table supported by hardware, from platform_data which we write to
>> ramp_rate control bits of control registers.
>
> Why is the driver unconditionally writing these register values here
> rather than setting the ramp delay that was passed in?

Here we are setting the max77686->ramp_delay and writing the same
value(max77686->ramp_delay << 6)@register also.


Thanks,
Yadwinder.

[PATCH 2/2] regulator: Add support for MAX77686.

[Yadwinder Singh Brar]

Hi Mark,

On Thu, May 10, 2012 at 12:17 AM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:
>
>> +/* Voltage maps in mV */
>> +static const struct voltage_map_desc ldo_voltage_map_desc = {
>> + ? ? .min = 800, ? ? .max = 3950, ? ?.step = 50, ? ? .n_bits = 6,
>> +}; ? ? ? ? ? ? ? ? ? ? ? ? ? /* LDO3 ~ 5, 9 ~ 14, 16 ~ 26 */
>
> Hrm, funnily enough I was just thinking about factoring this stuff out
> into the core after a conversation with Graeme Gregory the other week.
> Let's do that...
>
>> + ? ? [MAX77686_EN32KHZ_AP] = NULL,
>> + ? ? [MAX77686_EN32KHZ_CP] = NULL,
>
> Now that the generic clock API is in mainline these should be moved over
> to use it.
>
>> +static int max77686_get_voltage_unit(int rid)
>> +{
>> + ? ? int unit = 0;
>> +
>> + ? ? switch (rid) {
>> + ? ? case MAX77686_BUCK2...MAX77686_BUCK4:
>> + ? ? ? ? ? ? unit = 1; ? ? ? /* BUCK2,3,4 is uV */
>> + ? ? ? ? ? ? break;
>> + ? ? default:
>> + ? ? ? ? ? ? unit = 1000;
>
> Why not just list everything in uV?
>
>> +static int max77686_get_voltage(struct regulator_dev *rdev)
>> +{
>
> Implement get_voltage_sel().
>
>> +static inline int max77686_get_voltage_proper_val(const struct voltage_map_desc
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? *desc, int min_vol,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int max_vol)
>> +{
>> + ? ? int i = 0;
>> +
>> + ? ? if (desc == NULL)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? if (max_vol < desc->min || min_vol > desc->max)
>> + ? ? ? ? ? ? return -EINVAL;
>> +
>> + ? ? while (desc->min + desc->step * i < min_vol &&
>> + ? ? ? ? ? ?desc->min + desc->step * i < desc->max)
>> + ? ? ? ? ? ? i++;
>
> Why are you iterating here? ?Calculate! ?Though like I say let's factor
> this out anyway.
>
>> + ? ? if (rid == MAX77686_BUCK2 || rid == MAX77686_BUCK3 ||
>> + ? ? ? ? rid == MAX77686_BUCK4) {
>> + ? ? ? ? ? ? /* If the voltage is increasing */
>> + ? ? ? ? ? ? if (org < i)
>> + ? ? ? ? ? ? ? ? ? ? udelay(DIV_ROUND_UP(desc->step * (i - org),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ramp[max77686->ramp_delay]));
>> + ? ? }
>
> Don't do this, implement set_voltage_time_sel().
>
>> + ? ? .enable = max77686_reg_enable,
>> + ? ? .disable = max77686_reg_disable,
>> + ? ? .set_suspend_enable = max77686_reg_enable,
>> + ? ? .set_suspend_disable = max77686_reg_disable,
>
> You've got the same ops for suspend and non-suspend cases here, this is
> clearly buggy.
>
>> +/* count the number of regulators to be supported in pmic */
>> + ? ? pdata->num_regulators = 0;
>
> Coding style.
>
>> + ? ? if (iodev->dev->of_node) {
>> + ? ? ? ? ? ? ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
>> + ? ? ? ? ? ? if (ret)
>> + ? ? ? ? ? ? ? ? ? ? return ret;
>
> This ought to use of_regulator_match().
>

While using it I am seeing that though it reduces few lines of code in
our driver , it adds a huge array(of_regulator_match[])in which we
have to populate the strings(name) which we already have in
regulator_desc[ ], Isn't it overhead ?

>> + ? ? }
>> +
>> + ? ? if (!pdata) {
>> + ? ? ? ? ? ? dev_err(&pdev->dev, "platform data not found\n");
>> + ? ? ? ? ? ? return -ENODEV;
>> + ? ? }
>
> This should be totally fine.
>
>> + ? ? max77686 = kzalloc(sizeof(struct max77686_data), GFP_KERNEL);
>> + ? ? if (!max77686)
>> + ? ? ? ? ? ? return -ENOMEM;
>
> devm_kzalloc().
>
>> + ? ? if (pdata->ramp_delay) {
>> + ? ? ? ? ? ? max77686->ramp_delay = pdata->ramp_delay;
>> + ? ? ? ? ? ? max77686_update_reg(i2c, MAX77686_REG_BUCK2CTRL1,
>> + ? ? ? ? ? ? ? ? ? ? RAMP_VALUE, RAMP_MASK);
>
> This appears not to actually use the value passed in as platform_data.
>
>> +
>> + ? ? for (i = 0; i < pdata->num_regulators; i++) {
>> + ? ? ? ? ? ? const struct voltage_map_desc *desc;
>> + ? ? ? ? ? ? int id = pdata->regulators[i].id;
>> +
>> + ? ? ? ? ? ? desc = reg_voltage_map[id];
>> + ? ? ? ? ? ? if (desc)
>> + ? ? ? ? ? ? ? ? ? ? regulators[id].n_voltages =
>> + ? ? ? ? ? ? ? ? ? ? ? ? (desc->max - desc->min) / desc->step + 1;
>> +
>> + ? ? ? ? ? ? rdev[i] = regulator_register(&regulators[id], max77686->dev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pdata->regulators[i].initdata,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?max77686, NULL);
>
> No, you should unconditionally register all regulators the device
> physically has. ?This is useful for debug and simplifies the code.
>

If we have to use only 2 or 3 regulators on our board out off 36 or
lets take a case if our chip supports 50/100 regulators, I think i
will a overhead to register all unused regulators as well as
populating all the regulators in DT or platform data.

Regards,
Yadwinder.
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] regulator: Add support for MAX77686.

[Mark Brown]

On Tue, May 15, 2012 at 07:17:11PM +0530, Yadwinder Singh Brar wrote:
> On Thu, May 10, 2012 at 12:17 AM, Mark Brown
> > On Wed, May 09, 2012 at 09:54:55PM +0530, Yadwinder Singh wrote:

> >> + ? ? if (iodev->dev->of_node) {
> >> + ? ? ? ? ? ? ret = max77686_pmic_dt_parse_pdata(iodev, pdata);
> >> + ? ? ? ? ? ? if (ret)
> >> + ? ? ? ? ? ? ? ? ? ? return ret;

> > This ought to use of_regulator_match().

> While using it I am seeing that though it reduces few lines of code in
> our driver , it adds a huge array(of_regulator_match[])in which we
> have to populate the strings(name) which we already have in
> regulator_desc[ ], Isn't it overhead ?

Well, I think half the problem here is that you're only instantiating
things you find in the device tree.  If you were unconditionally
instantiating all the regulators then suddenly this becomes a lot
neater.

> >> + ? ? ? ? ? ? rdev[i] = regulator_register(&regulators[id], max77686->dev,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?pdata->regulators[i].initdata,
> >> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?max77686, NULL);

> > No, you should unconditionally register all regulators the device
> > physically has. ?This is useful for debug and simplifies the code.

> If we have to use only 2 or 3 regulators on our board out off 36 or
> lets take a case if our chip supports 50/100 regulators, I think i
> will a overhead to register all unused regulators as well as
> populating all the regulators in DT or platform data.

This isn't really a terribly realistic situation - if there are were
that many unused regulators the part selection is clearly not
appropriate to the system and most hardware engineers just wouldn't do
it.

Having information about all the regulators also allows us to do things
like power off any unused regulators which were left on after boot (eg,
due to bootloader or the PMIC defaults).
-------------- 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/20120516/19a52f25/attachment.sig>

[PATCH 2/2] regulator: Add support for MAX77686.

[Sylwester Nawrocki]

Hi,

just a few nitpicks...

On 05/09/2012 06:24 PM, Yadwinder Singh wrote:
> From: Yadwinder Singh Brar<yadi.brar@samsung.com>
>
> Add support for PMIC/regulator portion of MAX77686 multifunction device.
> MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of driver
> which supports setting and getting the voltage of a regulator with I2C
> interface.
>
> Signed-off-by: Yadwinder Singh Brar<yadi.brar@samsung.com>
...
> diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
> new file mode 100644
> index 0000000..4aa9722
> --- /dev/null
> +++ b/drivers/regulator/max77686.c
> @@ -0,0 +1,660 @@
> +/*
> + * max77686.c - Regulator driver for the Maxim 77686
> + *
> + * Copyright (C) 2012 Samsung Electronics

I believe this should read:

   + * Copyright (C) 2012 Samsung Electronics Co., Ltd.

In patch 1/2 the copyright notice is also not exactly correct.

> + * Chiwoong Byun<woong.byun@smasung.com>

s/smasung/samsung

...
> +static int max77686_get_enable_register(struct regulator_dev *rdev,
> +					int *reg, int *mask, int *pattern)
> +{
> +	int rid = rdev_get_id(rdev);
> +
> +	switch (rid) {
> +	case MAX77686_LDO1...MAX77686_LDO26:
> +		*reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
> +		*mask = 0xC0;
> +		*pattern = 0xC0;

What about using lower case for all hex numbers ?

...
> +static int max77686_get_voltage_register(struct regulator_dev *rdev,
> +					 int *_reg, int *_shift, int *_mask)
> +{
...
> +	case MAX77686_BUCK2:
> +		reg = MAX77686_REG_BUCK2DVS1;
> +		mask = 0xff;
> +		break;
> +	case MAX77686_BUCK3:
> +		reg = MAX77686_REG_BUCK3DVS1;
> +		mask = 0xff;
> +		break;
> +	case MAX77686_BUCK4:
> +		reg = MAX77686_REG_BUCK4DVS1;
> +		mask = 0xff;
> +		break;
> +	case MAX77686_BUCK5...MAX77686_BUCK9:
> +		reg = MAX77686_REG_BUCK5OUT + (rid - MAX77686_BUCK5) * 2;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*_reg = reg;
> +	*_shift = shift;
> +	*_mask = mask;
> +
> +	return 0;
> +}

Thanks,
Sylwester

[PATCH 2/2] regulator: Add support for MAX77686.

[Yadwinder Singh Brar]

Hi  Sylwester,

On Thu, May 10, 2012 at 1:24 AM, Sylwester Nawrocki <snjw23@gmail.com> wrote:
> Hi,
>
> just a few nitpicks...
>
>
> On 05/09/2012 06:24 PM, Yadwinder Singh wrote:
>>
>> From: Yadwinder Singh Brar<yadi.brar@samsung.com>
>>
>> Add support for PMIC/regulator portion of MAX77686 multifunction device.
>> MAX77686 provides LDOs[1-26] and BUCKs[1-9]. This is initial release of
>> driver
>> which supports setting and getting the voltage of a regulator with I2C
>> interface.
>>
>> Signed-off-by: Yadwinder Singh Brar<yadi.brar@samsung.com>
>
> ...
>
>> diff --git a/drivers/regulator/max77686.c b/drivers/regulator/max77686.c
>> new file mode 100644
>> index 0000000..4aa9722
>> --- /dev/null
>> +++ b/drivers/regulator/max77686.c
>> @@ -0,0 +1,660 @@
>> +/*
>> + * max77686.c - Regulator driver for the Maxim 77686
>> + *
>> + * Copyright (C) 2012 Samsung Electronics
>
>
> I believe this should read:
>
> ?+ * Copyright (C) 2012 Samsung Electronics Co., Ltd.
>
> In patch 1/2 the copyright notice is also not exactly correct.
>
>
>> + * Chiwoong Byun<woong.byun@smasung.com>
>
>
> s/smasung/samsung
>
> ...
>

Thanks for pointing. I will rectify above mistakes.

>> +static int max77686_get_enable_register(struct regulator_dev *rdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int *reg, int *mask, int *pattern)
>> +{
>> + ? ? ? int rid = rdev_get_id(rdev);
>> +
>> + ? ? ? switch (rid) {
>> + ? ? ? case MAX77686_LDO1...MAX77686_LDO26:
>> + ? ? ? ? ? ? ? *reg = MAX77686_REG_LDO1CTRL1 + (rid - MAX77686_LDO1);
>> + ? ? ? ? ? ? ? *mask = 0xC0;
>> + ? ? ? ? ? ? ? *pattern = 0xC0;
>
>
> What about using lower case for all hex numbers ?
>
> ...
>

Yes, i will take care of this.

>> +static int max77686_get_voltage_register(struct regulator_dev *rdev,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int *_reg, int *_shift, int
>> *_mask)
>> +{
>
> ...
>
>> + ? ? ? case MAX77686_BUCK2:
>> + ? ? ? ? ? ? ? reg = MAX77686_REG_BUCK2DVS1;
>> + ? ? ? ? ? ? ? mask = 0xff;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case MAX77686_BUCK3:
>> + ? ? ? ? ? ? ? reg = MAX77686_REG_BUCK3DVS1;
>> + ? ? ? ? ? ? ? mask = 0xff;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case MAX77686_BUCK4:
>> + ? ? ? ? ? ? ? reg = MAX77686_REG_BUCK4DVS1;
>> + ? ? ? ? ? ? ? mask = 0xff;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? case MAX77686_BUCK5...MAX77686_BUCK9:
>> + ? ? ? ? ? ? ? reg = MAX77686_REG_BUCK5OUT + (rid - MAX77686_BUCK5) * 2;
>> + ? ? ? ? ? ? ? break;
>> + ? ? ? default:
>> + ? ? ? ? ? ? ? return -EINVAL;
>> + ? ? ? }
>> +
>> + ? ? ? *_reg = reg;
>> + ? ? ? *_shift = shift;
>> + ? ? ? *_mask = mask;
>> +
>> + ? ? ? return 0;
>> +}
>
>
> Thanks,
> Sylwester
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Thanks,
Yadwinder.

[PATCH 0/2] regulator: add initial suport for max77686

[Mark Brown]

On Wed, May 09, 2012 at 09:54:53PM +0530, Yadwinder Singh wrote:
> This patch series adds support for max77686 which is a multifunction device which
> includes regulator (pmic), rtc and charger sub-blocks within it. The support for
> mfd driver and regulator driver are added by this patch series. This patch series
> also includes device tree and irqdomain support for mfd and regulator portions.

Always CC maintainers on patches, please see SubmittingPatches.