[PATCH v3 0/4] MFD MAX8998/LP3974 Driver Update

[PATCH v3 0/4] MFD MAX8998/LP3974 Driver Update

[MyungJoo Ham]

update at v3
1. Removed independant bugfixes from the patch set.
2. Power Supply API supported.
3. Removed regulators that became useless by introducing Power Supply API.

update at v2
1. Seperated features
2. Added style fixes

v1: "[PATCH] MFD MAX8998/LP3974: Add Features: hibernation, charger,
and other misc."

MyungJoo Ham (4):
  MFD MAX8998/LP3974: Support Hibernation
  MFD MAX8998/LP3974: Support LP3974 RTC
  regulator MAX8998/LP3974: Support DVS-GPIO.
  MFD MAX8998/LP3974: Support Charger

 drivers/mfd/max8998-irq.c           |    7 +
 drivers/mfd/max8998.c               |  155 ++++++++++++++++++++-
 drivers/power/Kconfig               |    7 +
 drivers/power/Makefile              |    1 +
 drivers/power/max8998.c             |  265 +++++++++++++++++++++++++++++++++++
 drivers/regulator/max8998.c         |  223 ++++++++++++++++++++++++++---
 drivers/rtc/rtc-max8998.c           |   55 +++++++-
 include/linux/mfd/max8998-private.h |   15 ++-
 include/linux/mfd/max8998.h         |   42 +++++-
 9 files changed, 732 insertions(+), 38 deletions(-)
 create mode 100644 drivers/power/max8998.c

[PATCH v3 1/4] MFD MAX8998/LP3974: Support Hibernation

[MyungJoo Ham]

This patch makes the driver to save and restore register values
for hibernation.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998-irq.c           |    7 ++
 drivers/mfd/max8998.c               |  108 +++++++++++++++++++++++++++++++++++
 include/linux/mfd/max8998-private.h |    2 +
 include/linux/mfd/max8998.h         |    1 +
 4 files changed, 118 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/max8998-irq.c b/drivers/mfd/max8998-irq.c
index 45bfe77..93c621d 100644
--- a/drivers/mfd/max8998-irq.c
+++ b/drivers/mfd/max8998-irq.c
@@ -181,6 +181,13 @@ static irqreturn_t max8998_irq_thread(int irq, void *data)
 	return IRQ_HANDLED;
 }
 
+int max8998_irq_resume(struct max8998_dev *max8998)
+{
+	if (max8998->irq && max8998->irq_base)
+		max8998_irq_thread(max8998->irq_base, max8998);
+	return 0;
+}
+
 int max8998_irq_init(struct max8998_dev *max8998)
 {
 	int i;
diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index bb9977b..5ce00ad 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -25,6 +25,8 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
 #include <linux/mutex.h>
 #include <linux/mfd/core.h>
 #include <linux/mfd/max8998.h>
@@ -135,6 +137,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	if (pdata) {
 		max8998->ono = pdata->ono;
 		max8998->irq_base = pdata->irq_base;
+		max8998->wakeup = pdata->wakeup;
 	}
 	mutex_init(&max8998->iolock);
 
@@ -146,6 +149,8 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	ret = mfd_add_devices(max8998->dev, -1,
 			      max8998_devs, ARRAY_SIZE(max8998_devs),
 			      NULL, 0);
+	pm_runtime_set_active(max8998->dev);
+
 	if (ret < 0)
 		goto err;
 
@@ -178,10 +183,113 @@ static const struct i2c_device_id max8998_i2c_id[] = {
 };
 MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
 
+static int max8998_suspend(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct max8998_dev *max8998 = i2c_get_clientdata(i2c);
+
+	if (max8998->wakeup)
+		set_irq_wake(max8998->irq, 1);
+	return 0;
+}
+
+static int max8998_resume(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	struct max8998_dev *max8998 = i2c_get_clientdata(i2c);
+
+	if (max8998->wakeup)
+		set_irq_wake(max8998->irq, 0);
+	/*
+	 * In LP3974, if IRQ registers are not "read & clear"
+	 * when it's set during sleep, the interrupt becomes
+	 * disabled.
+	 */
+	return max8998_irq_resume(i2c_get_clientdata(i2c));
+}
+
+struct max8998_reg_dump {
+	u8	addr;
+	u8	val;
+};
+#define SAVE_ITEM(x)	{ .addr = (x), .val = 0x0, }
+struct max8998_reg_dump max8998_dump[] = {
+	SAVE_ITEM(MAX8998_REG_IRQM1),
+	SAVE_ITEM(MAX8998_REG_IRQM2),
+	SAVE_ITEM(MAX8998_REG_IRQM3),
+	SAVE_ITEM(MAX8998_REG_IRQM4),
+	SAVE_ITEM(MAX8998_REG_STATUSM1),
+	SAVE_ITEM(MAX8998_REG_STATUSM2),
+	SAVE_ITEM(MAX8998_REG_CHGR1),
+	SAVE_ITEM(MAX8998_REG_CHGR2),
+	SAVE_ITEM(MAX8998_REG_LDO_ACTIVE_DISCHARGE1),
+	SAVE_ITEM(MAX8998_REG_LDO_ACTIVE_DISCHARGE1),
+	SAVE_ITEM(MAX8998_REG_BUCK_ACTIVE_DISCHARGE3),
+	SAVE_ITEM(MAX8998_REG_ONOFF1),
+	SAVE_ITEM(MAX8998_REG_ONOFF2),
+	SAVE_ITEM(MAX8998_REG_ONOFF3),
+	SAVE_ITEM(MAX8998_REG_ONOFF4),
+	SAVE_ITEM(MAX8998_REG_BUCK1_VOLTAGE1),
+	SAVE_ITEM(MAX8998_REG_BUCK1_VOLTAGE2),
+	SAVE_ITEM(MAX8998_REG_BUCK1_VOLTAGE3),
+	SAVE_ITEM(MAX8998_REG_BUCK1_VOLTAGE4),
+	SAVE_ITEM(MAX8998_REG_BUCK2_VOLTAGE1),
+	SAVE_ITEM(MAX8998_REG_BUCK2_VOLTAGE2),
+	SAVE_ITEM(MAX8998_REG_LDO2_LDO3),
+	SAVE_ITEM(MAX8998_REG_LDO4),
+	SAVE_ITEM(MAX8998_REG_LDO5),
+	SAVE_ITEM(MAX8998_REG_LDO6),
+	SAVE_ITEM(MAX8998_REG_LDO7),
+	SAVE_ITEM(MAX8998_REG_LDO8_LDO9),
+	SAVE_ITEM(MAX8998_REG_LDO10_LDO11),
+	SAVE_ITEM(MAX8998_REG_LDO12),
+	SAVE_ITEM(MAX8998_REG_LDO13),
+	SAVE_ITEM(MAX8998_REG_LDO14),
+	SAVE_ITEM(MAX8998_REG_LDO15),
+	SAVE_ITEM(MAX8998_REG_LDO16),
+	SAVE_ITEM(MAX8998_REG_LDO17),
+	SAVE_ITEM(MAX8998_REG_BKCHR),
+	SAVE_ITEM(MAX8998_REG_LBCNFG1),
+	SAVE_ITEM(MAX8998_REG_LBCNFG2),
+};
+/* Save registers before hibernation */
+static int max8998_freeze(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(max8998_dump); i++)
+		max8998_read_reg(i2c, max8998_dump[i].addr,
+				&max8998_dump[i].val);
+
+	return 0;
+}
+
+/* Restore registers after hibernation */
+static int max8998_restore(struct device *dev)
+{
+	struct i2c_client *i2c = container_of(dev, struct i2c_client, dev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(max8998_dump); i++)
+		max8998_write_reg(i2c, max8998_dump[i].addr,
+				max8998_dump[i].val);
+
+	return 0;
+}
+
+const struct dev_pm_ops max8998_pm = {
+	.suspend = max8998_suspend,
+	.resume = max8998_resume,
+	.freeze = max8998_freeze,
+	.restore = max8998_restore,
+};
+
 static struct i2c_driver max8998_i2c_driver = {
 	.driver = {
 		   .name = "max8998",
 		   .owner = THIS_MODULE,
+		   .pm = &max8998_pm,
 	},
 	.probe = max8998_i2c_probe,
 	.remove = max8998_i2c_remove,
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index 7363dea..effa5d3 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -159,10 +159,12 @@ struct max8998_dev {
 	u8 irq_masks_cur[MAX8998_NUM_IRQ_REGS];
 	u8 irq_masks_cache[MAX8998_NUM_IRQ_REGS];
 	int type;
+	bool wakeup;
 };
 
 int max8998_irq_init(struct max8998_dev *max8998);
 void max8998_irq_exit(struct max8998_dev *max8998);
+int max8998_irq_resume(struct max8998_dev *max8998);
 
 extern int max8998_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest);
 extern int max8998_bulk_read(struct i2c_client *i2c, u8 reg, int count,
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index f8c9f88..686a744 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -88,6 +88,7 @@ struct max8998_platform_data {
 	int				buck1_set1;
 	int				buck1_set2;
 	int				buck2_set3;
+	bool				wakeup;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

Re: [PATCH v3 1/4] MFD MAX8998/LP3974: Support Hibernation

[Samuel Ortiz]

Hi MyungJoo

On Thu, Dec 23, 2010 at 05:53:36PM +0900, MyungJoo Ham wrote:
> This patch makes the driver to save and restore register values
> for hibernation.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Patch applied, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v3 2/4] MFD MAX8998/LP3974: Support LP3974 RTC

[MyungJoo Ham]

The first releases of LP3974 have a large delay in RTC registers,
which requires 2 seconds of delay after writing to a rtc register
(recommended by National Semiconductor's engineers)
before reading it. If the device name is "lp3974-regerr", the rtc driver
assumes that such delays are required. If the device name is "lp3974",
the rtc driver does not. Although we have not seen LP3974s without
requiring such delays, we assume that such LP3974s will be released
soon (or they have done so already) and they are supported by "lp3974".

This patch adds delays with msleep when writing values to RTC registers
if the device name is "lp3974-regerr".

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998.c               |   41 ++++++++++++++++++++++++--
 drivers/regulator/max8998.c         |    7 ++++
 drivers/rtc/rtc-max8998.c           |   55 +++++++++++++++++++++++++++++++---
 include/linux/mfd/max8998-private.h |    1 +
 4 files changed, 96 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 5ce00ad..8b9eed1 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -42,6 +42,22 @@ static struct mfd_cell max8998_devs[] = {
 	},
 };
 
+static struct mfd_cell lp3974_devs[] = {
+	{
+		.name = "lp3974-pmic",
+	}, {
+		.name = "lp3974-rtc",
+	},
+};
+
+static struct mfd_cell lp3974_regerr_devs[] = {
+	{
+		.name = "lp3974-pmic",
+	}, {
+		.name = "lp3974-rtc-regerr",
+	},
+};
+
 int max8998_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
 {
 	struct max8998_dev *max8998 = i2c_get_clientdata(i2c);
@@ -146,11 +162,29 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 
 	max8998_irq_init(max8998);
 
-	ret = mfd_add_devices(max8998->dev, -1,
-			      max8998_devs, ARRAY_SIZE(max8998_devs),
-			      NULL, 0);
 	pm_runtime_set_active(max8998->dev);
 
+	switch (id->driver_data) {
+	case TYPE_LP3974_REGERR:
+		ret = mfd_add_devices(max8998->dev, -1,
+				lp3974_regerr_devs,
+				ARRAY_SIZE(lp3974_regerr_devs),
+				NULL, 0);
+		break;
+	case TYPE_LP3974:
+		ret = mfd_add_devices(max8998->dev, -1,
+				lp3974_devs, ARRAY_SIZE(lp3974_devs),
+				NULL, 0);
+		break;
+	case TYPE_MAX8998:
+		ret = mfd_add_devices(max8998->dev, -1,
+				max8998_devs, ARRAY_SIZE(max8998_devs),
+				NULL, 0);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
 	if (ret < 0)
 		goto err;
 
@@ -179,6 +213,7 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
 static const struct i2c_device_id max8998_i2c_id[] = {
 	{ "max8998", TYPE_MAX8998 },
 	{ "lp3974", TYPE_LP3974},
+	{ "lp3974-regerr", TYPE_LP3974_REGERR },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index ed5253c..d183756 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -836,6 +836,12 @@ static int __devexit max8998_pmic_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id max8998_pmic_id[] = {
+	{ "max8998-pmic", TYPE_MAX8998 },
+	{ "lp3974-pmic", TYPE_LP3974 },
+	{ }
+};
+
 static struct platform_driver max8998_pmic_driver = {
 	.driver = {
 		.name = "max8998-pmic",
@@ -843,6 +849,7 @@ static struct platform_driver max8998_pmic_driver = {
 	},
 	.probe = max8998_pmic_probe,
 	.remove = __devexit_p(max8998_pmic_remove),
+	.id_table = max8998_pmic_id,
 };
 
 static int __init max8998_pmic_init(void)
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index f22dee3..421ef0e 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/mfd/max8998.h>
 #include <linux/mfd/max8998-private.h>
+#include <linux/delay.h>
 
 #define MAX8998_RTC_SEC			0x00
 #define MAX8998_RTC_MIN			0x01
@@ -73,6 +74,7 @@ struct max8998_rtc_info {
 	struct i2c_client	*rtc;
 	struct rtc_device	*rtc_dev;
 	int irq;
+	bool lp3974_bug_workaround;
 };
 
 static void max8998_data_to_tm(u8 *data, struct rtc_time *tm)
@@ -124,10 +126,16 @@ static int max8998_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct max8998_rtc_info *info = dev_get_drvdata(dev);
 	u8 data[8];
+	int ret;
 
 	max8998_tm_to_data(tm, data);
 
-	return max8998_bulk_write(info->rtc, MAX8998_RTC_SEC, 8, data);
+	ret = max8998_bulk_write(info->rtc, MAX8998_RTC_SEC, 8, data);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -163,12 +171,29 @@ static int max8998_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static int max8998_rtc_stop_alarm(struct max8998_rtc_info *info)
 {
-	return max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0);
+	int ret = max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_start_alarm(struct max8998_rtc_info *info)
 {
-	return max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0x77);
+	int ret;
+	u8 alarm0_conf = 0x77;
+
+	/* LP3974 with delay bug chips has rtc alarm bugs with "MONTH" field */
+	if (info->lp3974_bug_workaround)
+		alarm0_conf = 0x57;
+
+	ret = max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, alarm0_conf);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -187,10 +212,13 @@ static int max8998_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		return ret;
 
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
 	if (alrm->enabled)
-		return max8998_rtc_start_alarm(info);
+		ret = max8998_rtc_start_alarm(info);
 
-	return 0;
+	return ret;
 }
 
 static int max8998_rtc_alarm_irq_enable(struct device *dev,
@@ -249,10 +277,18 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 
 	ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0,
 			"rtc-alarm0", info);
+
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->irq, ret);
 
+	dev_info(&pdev->dev, "RTC CHIP NAME: %s\n", pdev->id_entry->name);
+	if (pdev->id_entry->driver_data == TYPE_LP3974_REGERR) {
+		info->lp3974_bug_workaround = true;
+		dev_warn(&pdev->dev, "LP3974 with RTC REGERR option."
+				" RTC updates will be extremely slow.\n");
+	}
+
 	return 0;
 
 out_rtc:
@@ -273,6 +309,14 @@ static int __devexit max8998_rtc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id max8998_rtc_id[] = {
+	{ "max8998-rtc", TYPE_MAX8998 },
+	{ "lp3974-rtc", TYPE_LP3974 },
+	/* REGERR: requires 1 ~ 2 s of delay after writing to RTC registers */
+	{ "lp3974-rtc-regerr", TYPE_LP3974_REGERR },
+	{ }
+};
+
 static struct platform_driver max8998_rtc_driver = {
 	.driver		= {
 		.name	= "max8998-rtc",
@@ -280,6 +324,7 @@ static struct platform_driver max8998_rtc_driver = {
 	},
 	.probe		= max8998_rtc_probe,
 	.remove		= __devexit_p(max8998_rtc_remove),
+	.id_table	= max8998_rtc_id,
 };
 
 static int __init max8998_rtc_init(void)
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index effa5d3..f107f42 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -105,6 +105,7 @@ enum {
 enum {
 	TYPE_MAX8998 = 0, /* Default */
 	TYPE_LP3974,	/* National version of MAX8998 */
+	TYPE_LP3974_REGERR, /* National version of MAX8998 with RTC REG BUG */
 	TYPE_LP3979,	/* Added AVS */
 };
 
-- 
1.7.1

Re: [PATCH v3 2/4] MFD MAX8998/LP3974: Support LP3974 RTC

[Samuel Ortiz]

Hi MyungJoo,

On Thu, Dec 23, 2010 at 05:53:37PM +0900, MyungJoo Ham wrote:
> The first releases of LP3974 have a large delay in RTC registers,
> which requires 2 seconds of delay after writing to a rtc register
> (recommended by National Semiconductor's engineers)
> before reading it. If the device name is "lp3974-regerr", the rtc driver
> assumes that such delays are required. If the device name is "lp3974",
> the rtc driver does not. Although we have not seen LP3974s without
> requiring such delays, we assume that such LP3974s will be released
> soon (or they have done so already) and they are supported by "lp3974".
> 
> This patch adds delays with msleep when writing values to RTC registers
> if the device name is "lp3974-regerr".
As Mark pointed out, I'd prefer to add a regerr flag to the driver platform
data and decide which cell to add from there.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v4 0/3] MFD MAX8998/LP3974 Driver Update

[MyungJoo Ham]

update at v4
1. Removed "MFD MAX8998/LP3974: Support Hibernation" as this patch
is already applied.
2. "MFD MAX8998/LP3974: Support LP3974 RTC": RTC register delay chip bug
information is included at platform data, not at the device id.
Note that the other two patches remain the same and the ACK from the
previous patch release is attached.

update at v3
1. Removed independant bugfixes from the patch set.
2. Power Supply API supported.
3. Removed regulators that became useless by introducing Power Supply API.

update at v2
1. Seperated features
2. Added style fixes

v1: "[PATCH] MFD MAX8998/LP3974: Add Features: hibernation, charger,
and other misc."

MyungJoo Ham (3):
  MFD MAX8998/LP3974: Support LP3974 RTC
  regulator MAX8998/LP3974: Support DVS-GPIO.
  MFD MAX8998/LP3974: Support Charger

 drivers/mfd/max8998.c               |   30 ++++-
 drivers/power/Kconfig               |    7 +
 drivers/power/Makefile              |    1 +
 drivers/power/max8998.c             |  265 +++++++++++++++++++++++++++++++++++
 drivers/regulator/max8998.c         |  223 ++++++++++++++++++++++++++---
 drivers/rtc/rtc-max8998.c           |   54 +++++++-
 include/linux/mfd/max8998-private.h |   12 ++-
 include/linux/mfd/max8998.h         |   45 +++++-
 8 files changed, 599 insertions(+), 38 deletions(-)
 create mode 100644 drivers/power/max8998.c

[PATCH v4 1/3] MFD MAX8998/LP3974: Support LP3974 RTC

[MyungJoo Ham]

The first releases of LP3974 have a large delay in RTC registers,
which requires 2 seconds of delay after writing to a rtc register
(recommended by National Semiconductor's engineers)
before reading it.

If "rtc_delay" field of the platform data is true, the rtc driver
assumes that such delays are required. Although we have not seen
LP3974s without requiring such delays, we assume that such LP3974s
will be released soon (or they have done so already) and they are
supported by "lp3974" without setting "rtc_delay" at the platform
data.

This patch adds delays with msleep when writing values to RTC registers
if the platform data has rtc_delay set.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998.c       |   26 ++++++++++++++++++--
 drivers/regulator/max8998.c |    7 +++++
 drivers/rtc/rtc-max8998.c   |   54 +++++++++++++++++++++++++++++++++++++++----
 include/linux/mfd/max8998.h |    4 +++
 4 files changed, 83 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 5ce00ad..bbfe867 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -42,6 +42,14 @@ static struct mfd_cell max8998_devs[] = {
 	},
 };
 
+static struct mfd_cell lp3974_devs[] = {
+	{
+		.name = "lp3974-pmic",
+	}, {
+		.name = "lp3974-rtc",
+	},
+};
+
 int max8998_read_reg(struct i2c_client *i2c, u8 reg, u8 *dest)
 {
 	struct max8998_dev *max8998 = i2c_get_clientdata(i2c);
@@ -146,11 +154,23 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 
 	max8998_irq_init(max8998);
 
-	ret = mfd_add_devices(max8998->dev, -1,
-			      max8998_devs, ARRAY_SIZE(max8998_devs),
-			      NULL, 0);
 	pm_runtime_set_active(max8998->dev);
 
+	switch (id->driver_data) {
+	case TYPE_LP3974:
+		ret = mfd_add_devices(max8998->dev, -1,
+				lp3974_devs, ARRAY_SIZE(lp3974_devs),
+				NULL, 0);
+		break;
+	case TYPE_MAX8998:
+		ret = mfd_add_devices(max8998->dev, -1,
+				max8998_devs, ARRAY_SIZE(max8998_devs),
+				NULL, 0);
+		break;
+	default:
+		ret = -EINVAL;
+	}
+
 	if (ret < 0)
 		goto err;
 
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index ed5253c..d183756 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -836,6 +836,12 @@ static int __devexit max8998_pmic_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id max8998_pmic_id[] = {
+	{ "max8998-pmic", TYPE_MAX8998 },
+	{ "lp3974-pmic", TYPE_LP3974 },
+	{ }
+};
+
 static struct platform_driver max8998_pmic_driver = {
 	.driver = {
 		.name = "max8998-pmic",
@@ -843,6 +849,7 @@ static struct platform_driver max8998_pmic_driver = {
 	},
 	.probe = max8998_pmic_probe,
 	.remove = __devexit_p(max8998_pmic_remove),
+	.id_table = max8998_pmic_id,
 };
 
 static int __init max8998_pmic_init(void)
diff --git a/drivers/rtc/rtc-max8998.c b/drivers/rtc/rtc-max8998.c
index f22dee3..3f7bc6b 100644
--- a/drivers/rtc/rtc-max8998.c
+++ b/drivers/rtc/rtc-max8998.c
@@ -20,6 +20,7 @@
 #include <linux/platform_device.h>
 #include <linux/mfd/max8998.h>
 #include <linux/mfd/max8998-private.h>
+#include <linux/delay.h>
 
 #define MAX8998_RTC_SEC			0x00
 #define MAX8998_RTC_MIN			0x01
@@ -73,6 +74,7 @@ struct max8998_rtc_info {
 	struct i2c_client	*rtc;
 	struct rtc_device	*rtc_dev;
 	int irq;
+	bool lp3974_bug_workaround;
 };
 
 static void max8998_data_to_tm(u8 *data, struct rtc_time *tm)
@@ -124,10 +126,16 @@ static int max8998_rtc_set_time(struct device *dev, struct rtc_time *tm)
 {
 	struct max8998_rtc_info *info = dev_get_drvdata(dev);
 	u8 data[8];
+	int ret;
 
 	max8998_tm_to_data(tm, data);
 
-	return max8998_bulk_write(info->rtc, MAX8998_RTC_SEC, 8, data);
+	ret = max8998_bulk_write(info->rtc, MAX8998_RTC_SEC, 8, data);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -163,12 +171,29 @@ static int max8998_rtc_read_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 
 static int max8998_rtc_stop_alarm(struct max8998_rtc_info *info)
 {
-	return max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0);
+	int ret = max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_start_alarm(struct max8998_rtc_info *info)
 {
-	return max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, 0x77);
+	int ret;
+	u8 alarm0_conf = 0x77;
+
+	/* LP3974 with delay bug chips has rtc alarm bugs with "MONTH" field */
+	if (info->lp3974_bug_workaround)
+		alarm0_conf = 0x57;
+
+	ret = max8998_write_reg(info->rtc, MAX8998_ALARM0_CONF, alarm0_conf);
+
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
+	return ret;
 }
 
 static int max8998_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
@@ -187,10 +212,13 @@ static int max8998_rtc_set_alarm(struct device *dev, struct rtc_wkalrm *alrm)
 	if (ret < 0)
 		return ret;
 
+	if (info->lp3974_bug_workaround)
+		msleep(2000);
+
 	if (alrm->enabled)
-		return max8998_rtc_start_alarm(info);
+		ret = max8998_rtc_start_alarm(info);
 
-	return 0;
+	return ret;
 }
 
 static int max8998_rtc_alarm_irq_enable(struct device *dev,
@@ -224,6 +252,7 @@ static const struct rtc_class_ops max8998_rtc_ops = {
 static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 {
 	struct max8998_dev *max8998 = dev_get_drvdata(pdev->dev.parent);
+	struct max8998_platform_data *pdata = dev_get_platdata(max8998->dev);
 	struct max8998_rtc_info *info;
 	int ret;
 
@@ -249,10 +278,18 @@ static int __devinit max8998_rtc_probe(struct platform_device *pdev)
 
 	ret = request_threaded_irq(info->irq, NULL, max8998_rtc_alarm_irq, 0,
 			"rtc-alarm0", info);
+
 	if (ret < 0)
 		dev_err(&pdev->dev, "Failed to request alarm IRQ: %d: %d\n",
 			info->irq, ret);
 
+	dev_info(&pdev->dev, "RTC CHIP NAME: %s\n", pdev->id_entry->name);
+	if (pdata->rtc_delay) {
+		info->lp3974_bug_workaround = true;
+		dev_warn(&pdev->dev, "LP3974 with RTC REGERR option."
+				" RTC updates will be extremely slow.\n");
+	}
+
 	return 0;
 
 out_rtc:
@@ -273,6 +310,12 @@ static int __devexit max8998_rtc_remove(struct platform_device *pdev)
 	return 0;
 }
 
+static const struct platform_device_id max8998_rtc_id[] = {
+	{ "max8998-rtc", TYPE_MAX8998 },
+	{ "lp3974-rtc", TYPE_LP3974 },
+	{ }
+};
+
 static struct platform_driver max8998_rtc_driver = {
 	.driver		= {
 		.name	= "max8998-rtc",
@@ -280,6 +323,7 @@ static struct platform_driver max8998_rtc_driver = {
 	},
 	.probe		= max8998_rtc_probe,
 	.remove		= __devexit_p(max8998_rtc_remove),
+	.id_table	= max8998_rtc_id,
 };
 
 static int __init max8998_rtc_init(void)
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 686a744..c22b9a4 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -76,6 +76,9 @@ struct max8998_regulator_data {
  * @buck1_set1: BUCK1 gpio pin 1 to set output voltage
  * @buck1_set2: BUCK1 gpio pin 2 to set output voltage
  * @buck2_set3: BUCK2 gpio pin to set output voltage
+ * @wakeup: Allow to wake up from suspend
+ * @rtc_delay: LP3974 RTC chip bug that requires delay after a register
+ * write before reading it.
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
@@ -89,6 +92,7 @@ struct max8998_platform_data {
 	int				buck1_set2;
 	int				buck2_set3;
 	bool				wakeup;
+	bool				rtc_delay;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

Re: [PATCH v4 1/3] MFD MAX8998/LP3974: Support LP3974 RTC

[Mark Brown]

On Tue, Jan 04, 2011 at 02:17:39PM +0900, MyungJoo Ham wrote:
> The first releases of LP3974 have a large delay in RTC registers,
> which requires 2 seconds of delay after writing to a rtc register
> (recommended by National Semiconductor's engineers)
> before reading it.

Reviewed-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Re: [PATCH v4 1/3] MFD MAX8998/LP3974: Support LP3974 RTC

[Samuel Ortiz]

Hi MyungJoo,

On Tue, Jan 04, 2011 at 02:17:39PM +0900, MyungJoo Ham wrote:
> The first releases of LP3974 have a large delay in RTC registers,
> which requires 2 seconds of delay after writing to a rtc register
> (recommended by National Semiconductor's engineers)
> before reading it.
> 
> If "rtc_delay" field of the platform data is true, the rtc driver
> assumes that such delays are required. Although we have not seen
> LP3974s without requiring such delays, we assume that such LP3974s
> will be released soon (or they have done so already) and they are
> supported by "lp3974" without setting "rtc_delay" at the platform
> data.
> 
> This patch adds delays with msleep when writing values to RTC registers
> if the platform data has rtc_delay set.
Patch applied, many thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

[MyungJoo Ham]

The previous driver did not support BUCK1-DVS3, BUCK1-DVS4, and
BUCK2-DVS2 modes. This patch adds such modes and an option to block
setting buck1/2 voltages out of the preset values.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 drivers/regulator/max8998.c |   87 +++++++++++++++++++++++++++++++++----------
 include/linux/mfd/max8998.h |   26 ++++++++++---
 2 files changed, 87 insertions(+), 26 deletions(-)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index d183756..9946488 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -425,6 +425,9 @@ static int max8998_set_voltage_buck(struct regulator_dev *rdev,
 				}
 			}
 
+			if (pdata->buck_voltage_lock)
+				return -EINVAL;
+
 			/* no predefine regulator found */
 			max8998->buck1_idx = (buck1_last_val % 2) + 2;
 			dev_dbg(max8998->dev, "max8998->buck1_idx:%d\n",
@@ -452,18 +455,26 @@ buck1_exit:
 			"BUCK2, i:%d buck2_vol1:%d, buck2_vol2:%d\n"
 			, i, max8998->buck2_vol[0], max8998->buck2_vol[1]);
 		if (gpio_is_valid(pdata->buck2_set3)) {
-			if (max8998->buck2_vol[0] == i) {
-				max8998->buck2_idx = 0;
-				buck2_gpio_set(pdata->buck2_set3, 0);
-			} else {
-				max8998->buck2_idx = 1;
-				ret = max8998_get_voltage_register(rdev, &reg,
-								   &shift,
-								   &mask);
-				ret = max8998_write_reg(i2c, reg, i);
-				max8998->buck2_vol[1] = i;
-				buck2_gpio_set(pdata->buck2_set3, 1);
+
+			/* check if requested voltage */
+			/* value is already defined */
+			for (j = 0; j < ARRAY_SIZE(max8998->buck2_vol); j++) {
+				if (max8998->buck2_vol[j] == i) {
+					max8998->buck2_idx = j;
+					buck2_gpio_set(pdata->buck2_set3, j);
+					goto buck2_exit;
+				}
 			}
+
+			if (pdata->buck_voltage_lock)
+				return -EINVAL;
+
+			max8998_get_voltage_register(rdev,
+					&reg, &shift, &mask);
+			ret = max8998_write_reg(i2c, reg, i);
+			max8998->buck2_vol[max8998->buck2_idx] = i;
+			buck2_gpio_set(pdata->buck2_set3, max8998->buck2_idx);
+buck2_exit:
 			dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
 				gpio_get_value(pdata->buck2_set3));
 		} else {
@@ -708,6 +719,9 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, max8998);
 	i2c = max8998->iodev->i2c;
 
+	max8998->buck1_idx = pdata->buck1_default_idx;
+	max8998->buck2_idx = pdata->buck2_default_idx;
+
 	/* NOTE: */
 	/* For unused GPIO NOT marked as -1 (thereof equal to 0)  WARN_ON */
 	/* will be displayed */
@@ -740,23 +754,46 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck1_max_voltage1 / 1000))
+		       < (pdata->buck1_voltage1 / 1000))
 			i++;
-		printk(KERN_ERR "i:%d, buck1_idx:%d\n", i, max8998->buck1_idx);
 		max8998->buck1_vol[0] = i;
 		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE1, i);
+		if (ret)
+			return ret;
 
 		/* Set predefined value for BUCK1 register 2 */
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck1_max_voltage2 / 1000))
+		       < (pdata->buck1_voltage2 / 1000))
 			i++;
 
 		max8998->buck1_vol[1] = i;
-		printk(KERN_ERR "i:%d, buck1_idx:%d\n", i, max8998->buck1_idx);
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i)
-			+ ret;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i);
+		if (ret)
+			return ret;
+
+		/* Set predefined value for BUCK1 register 3 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck1_voltage3 / 1000))
+			i++;
+
+		max8998->buck1_vol[2] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE3, i);
+		if (ret)
+			return ret;
+
+		/* Set predefined value for BUCK1 register 4 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck1_voltage4 / 1000))
+			i++;
+
+		max8998->buck1_vol[3] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE4, i);
 		if (ret)
 			return ret;
 
@@ -773,18 +810,28 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		gpio_direction_output(pdata->buck2_set3,
 				      max8998->buck2_idx & 0x1);
 
-		/* BUCK2 - set preset default voltage value to buck2_vol[0] */
+		/* BUCK2 register 1 */
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck2_max_voltage / 1000))
+		       < (pdata->buck2_voltage1 / 1000))
 			i++;
-		printk(KERN_ERR "i:%d, buck2_idx:%d\n", i, max8998->buck2_idx);
 		max8998->buck2_vol[0] = i;
 		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE1, i);
 		if (ret)
 			return ret;
 
+		/* BUCK2 register 2 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck2_voltage2 / 1000))
+			i++;
+		printk(KERN_ERR "i2:%d, buck2_idx:%d\n", i, max8998->buck2_idx);
+		max8998->buck2_vol[1] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE2, i);
+		if (ret)
+			return ret;
 	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index c22b9a4..61daa16 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -70,12 +70,20 @@ struct max8998_regulator_data {
  * @num_regulators: number of regultors used
  * @irq_base: base IRQ number for max8998, required for IRQs
  * @ono: power onoff IRQ number for max8998
- * @buck1_max_voltage1: BUCK1 maximum alowed voltage register 1
- * @buck1_max_voltage2: BUCK1 maximum alowed voltage register 2
- * @buck2_max_voltage: BUCK2 maximum alowed voltage
+ * @buck_voltage_lock: Do NOT change the values of the following six
+ *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
+ *   be other than the preset values.
+ * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
+ * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
+ * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
+ * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
+ * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
+ * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
  * @buck1_set1: BUCK1 gpio pin 1 to set output voltage
  * @buck1_set2: BUCK1 gpio pin 2 to set output voltage
+ * @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
  * @buck2_set3: BUCK2 gpio pin to set output voltage
+ * @buck2_default_idx: Default for BUCK2 gpio pin.
  * @wakeup: Allow to wake up from suspend
  * @rtc_delay: LP3974 RTC chip bug that requires delay after a register
  * write before reading it.
@@ -85,12 +93,18 @@ struct max8998_platform_data {
 	int				num_regulators;
 	int				irq_base;
 	int				ono;
-	int                             buck1_max_voltage1;
-	int                             buck1_max_voltage2;
-	int                             buck2_max_voltage;
+	bool				buck_voltage_lock;
+	int				buck1_voltage1;
+	int				buck1_voltage2;
+	int				buck1_voltage3;
+	int				buck1_voltage4;
+	int				buck2_voltage1;
+	int				buck2_voltage2;
 	int				buck1_set1;
 	int				buck1_set2;
+	int				buck1_default_idx;
 	int				buck2_set3;
+	int				buck2_default_idx;
 	bool				wakeup;
 	bool				rtc_delay;
 };
-- 
1.7.1

Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

[Lukasz Majewski]

On Tue, 04 Jan 2011 14:17:40 +0900
MyungJoo Ham <myungjoo.ham@samsung.com> wrote:

Hi all,

I've posted some comments below:

if (gpio_is_valid(pdata->buck2_set3)) {
> -			if (max8998->buck2_vol[0] == i) {
> -				max8998->buck2_idx = 0;
> -				buck2_gpio_set(pdata->buck2_set3, 0);
> -			} else {
> -				max8998->buck2_idx = 1;
> -				ret =
> max8998_get_voltage_register(rdev, &reg, -
> &shift,
> -
> &mask);
> -				ret = max8998_write_reg(i2c, reg, i);
> -				max8998->buck2_vol[1] = i;
> -				buck2_gpio_set(pdata->buck2_set3, 1);
> +
> +			/* check if requested voltage */
> +			/* value is already defined */
> +			for (j = 0; j <
> ARRAY_SIZE(max8998->buck2_vol); j++) {
> +				if (max8998->buck2_vol[j] == i) {
> +					max8998->buck2_idx = j;
> +
> buck2_gpio_set(pdata->buck2_set3, j);
> +					goto buck2_exit;
> +				}
>  			}
> +
> +			if (pdata->buck_voltage_lock)
> +				return -EINVAL;
> +
> +			max8998_get_voltage_register(rdev,
> +					&reg, &shift, &mask);
> +			ret = max8998_write_reg(i2c, reg, i);
> +			max8998->buck2_vol[max8998->buck2_idx] = i;
> +
> buck2_gpio_set(pdata->buck2_set3,max8998->buck2_idx); +buck2_exit:
> dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
gpio_get_value(pdata->buck2_set3));

Maybe only the matter of taste. The "for" loop for only two elements?

> + * @buck_voltage_lock: Do NOT change the values of the following six
> + *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
> + *   be other than the preset values.
> + * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
> + * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
> + * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
> + * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
> + * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
> + * @buck2_voltage2: BUCK2 DVS mode 2 voltage register

Is it desirable to define all four for BUCK1 and two for BUCK2 DVS
voltages in platform code? 

Why the "general purpose" slots approach for user changeable/definable
voltages (as it was done previously) have been replaced? Is the current
approach faster?


-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group

Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

[MyungJoo Ham]

On Tue, Jan 4, 2011 at 4:49 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> On Tue, 04 Jan 2011 14:17:40 +0900
> MyungJoo Ham <myungjoo.ham@samsung.com> wrote:
>
> Hi all,
>
> I've posted some comments below:
>
> if (gpio_is_valid(pdata->buck2_set3)) {
>> -                     if (max8998->buck2_vol[0] == i) {
>> -                             max8998->buck2_idx = 0;
>> -                             buck2_gpio_set(pdata->buck2_set3, 0);
>> -                     } else {
>> -                             max8998->buck2_idx = 1;
>> -                             ret =
>> max8998_get_voltage_register(rdev, &reg, -
>> &shift,
>> -
>> &mask);
>> -                             ret = max8998_write_reg(i2c, reg, i);
>> -                             max8998->buck2_vol[1] = i;
>> -                             buck2_gpio_set(pdata->buck2_set3, 1);
>> +
>> +                     /* check if requested voltage */
>> +                     /* value is already defined */
>> +                     for (j = 0; j <
>> ARRAY_SIZE(max8998->buck2_vol); j++) {
>> +                             if (max8998->buck2_vol[j] == i) {
>> +                                     max8998->buck2_idx = j;
>> +
>> buck2_gpio_set(pdata->buck2_set3, j);
>> +                                     goto buck2_exit;
>> +                             }
>>                       }
>> +
>> +                     if (pdata->buck_voltage_lock)
>> +                             return -EINVAL;
>> +
>> +                     max8998_get_voltage_register(rdev,
>> +                                     &reg, &shift, &mask);
>> +                     ret = max8998_write_reg(i2c, reg, i);
>> +                     max8998->buck2_vol[max8998->buck2_idx] = i;
>> +
>> buck2_gpio_set(pdata->buck2_set3,max8998->buck2_idx); +buck2_exit:
>> dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
> gpio_get_value(pdata->buck2_set3));
>
> Maybe only the matter of taste. The "for" loop for only two elements?

It was only for the look; to appear more symmetric with buck1.

>
>> + * @buck_voltage_lock: Do NOT change the values of the following six
>> + *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
>> + *   be other than the preset values.
>> + * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
>> + * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
>> + * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
>> + * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
>> + * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
>> + * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
>
> Is it desirable to define all four for BUCK1 and two for BUCK2 DVS
> voltages in platform code?

In case a system with 4 buck1 preset voltages and 2 buck2 preset
voltages, it is desirable. :)

>
> Why the "general purpose" slots approach for user changeable/definable
> voltages (as it was done previously) have been replaced? Is the current
> approach faster?

As long as "buck_voltage_lock" is not set, user may use voltages not
defined as a preset (buckx_voltagex).

If buck_voltage_lock is true, any voltage not predefined is
rejected.However, if not, undefined voltages are accepted and from the
point where an undefined voltage is submitted, presets are not
guaranteed to be kept (any of preset values may be overwritten.)

>
>
> --
> Best regards,
>
> Lukasz Majewski
>
> Samsung Poland R&D Center
> Platform Group
>

Thanks.

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

[Mark Brown]

On Tue, Jan 04, 2011 at 05:16:56PM +0900, MyungJoo Ham wrote:
> On Tue, Jan 4, 2011 at 4:49 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:

> > Is it desirable to define all four for BUCK1 and two for BUCK2 DVS
> > voltages in platform code?

> In case a system with 4 buck1 preset voltages and 2 buck2 preset
> voltages, it is desirable. :)

I suspect Lukasz's question is more why this would be useful; I can
imagine that the main reason would be performance and in any case I
don't see it hurting to have the option.

Re: [PATCH v4 2/3] regulator MAX8998/LP3974: Support DVS-GPIO.

[Samuel Ortiz]

Hi Myungjoo,

On Tue, Jan 04, 2011 at 02:17:40PM +0900, MyungJoo Ham wrote:
> The previous driver did not support BUCK1-DVS3, BUCK1-DVS4, and
> BUCK2-DVS2 modes. This patch adds such modes and an option to block
> setting buck1/2 voltages out of the preset values.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
I applied this patch, thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v4 3/3] MFD MAX8998/LP3974: Support Charger

[MyungJoo Ham]

With the new regulator, "CHARGER", users can control charging
current and turn on and off the charger. Note that the charger
specification of LP3974 is different from that of MAX8998.

driver/power/max8998.c supports power supply APIs for

1. "ONLINE" monitors the charger status, which can be
different from the status "CHARGER"; e.g., users allowed the charger
to charge, but the MAX8998 chip decided not to do so.

2. "PRESENT" monitors the battery status (the existence of the
battery).

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998.c               |    4 +
 drivers/power/Kconfig               |    7 +
 drivers/power/Makefile              |    1 +
 drivers/power/max8998.c             |  265 +++++++++++++++++++++++++++++++++++
 drivers/regulator/max8998.c         |  129 +++++++++++++++++-
 include/linux/mfd/max8998-private.h |   12 ++-
 include/linux/mfd/max8998.h         |   15 ++
 7 files changed, 429 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/max8998.c

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index bbfe867..4716003 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -39,6 +39,8 @@ static struct mfd_cell max8998_devs[] = {
 		.name = "max8998-pmic",
 	}, {
 		.name = "max8998-rtc",
+	}, {
+		.name = "max8998-battery",
 	},
 };
 
@@ -47,6 +49,8 @@ static struct mfd_cell lp3974_devs[] = {
 		.name = "lp3974-pmic",
 	}, {
 		.name = "lp3974-rtc",
+	}, {
+		.name = "lp3974-battery",
 	},
 };
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 60d83d9..06b720c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -185,4 +185,11 @@ config CHARGER_TWL4030
 	help
 	  Say Y here to enable support for TWL4030 Battery Charge Interface.
 
+config CHARGERCTRL_MAX8998
+	tristate "Battery charger driver for MAX8998/LP3974 PMIC"
+	depends on MFD_MAX8998 && REGULATOR_MAX8998
+	help
+	  Say Y to enable support for the battery charger control sysfs and
+	  platform data of MAX8998/LP3974 PMICs.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index c75772e..4e66b91 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_BATTERY_JZ4740)	+= jz4740-battery.o
 obj-$(CONFIG_BATTERY_INTEL_MID)	+= intel_mid_battery.o
 obj-$(CONFIG_CHARGER_ISP1704)	+= isp1704_charger.o
 obj-$(CONFIG_CHARGER_TWL4030)	+= twl4030_charger.o
+obj-$(CONFIG_CHARGERCTRL_MAX8998)	+= max8998.o
diff --git a/drivers/power/max8998.c b/drivers/power/max8998.c
new file mode 100644
index 0000000..6e73b42
--- /dev/null
+++ b/drivers/power/max8998.c
@@ -0,0 +1,265 @@
+/*
+ * max8998.c - Power supply consumer driver for the Maxim 8998
+ *
+ *  Copyright (C) 2009-2010 Samsung Electronics
+ *  MyungJoo Ham <myungjoo.ham@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
+ */
+
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/max8998.h>
+#include <linux/mfd/max8998-private.h>
+
+struct max8998_battery_data {
+	struct device *dev;
+	struct max8998_dev *iodev;
+	unsigned int eoc_in_mA;
+	struct power_supply battery;
+	enum max8998_type device_id;
+};
+
+static enum power_supply_property max8998_battery_props[] = {
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_PRESENT, /* the presence of battery */
+	POWER_SUPPLY_PROP_ONLINE, /* charger is active or not */
+};
+
+static const char * const manufacturers[] = {
+	[TYPE_MAX8998] = "Maxim",
+	[TYPE_LP3974] = "National Semiconductor",
+	[TYPE_UNKNOWN] = "Unknown",
+};
+
+/* Note that the charger control is done by a current regulator "CHARGER" */
+static int max8998_battery_get_property(struct power_supply *psy,
+		enum power_supply_property psp,
+		union power_supply_propval *val)
+{
+	struct max8998_battery_data *max8998 = container_of(psy,
+			struct max8998_battery_data, battery);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int ret;
+	u8 reg;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		if (max8998->device_id == TYPE_MAX8998)
+			val->strval = manufacturers[TYPE_MAX8998];
+		else if (max8998->device_id == TYPE_LP3974)
+			val->strval = manufacturers[TYPE_LP3974];
+		else
+			val->strval = manufacturers[TYPE_UNKNOWN];
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 4))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 3))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void _max8998_update_eoc(struct platform_device *pdev)
+{
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int charge_current = 0;
+	int target_eoc_ratio;
+	u8 val;
+
+	/* Nothing to do. User set EOC with % */
+	if (max8998->eoc_in_mA == 0)
+		return;
+
+	/* Not initialized. */
+	if (!charger_current_map_desc)
+		return;
+
+	if (max8998_read_reg(i2c, MAX8998_REG_CHGR1, &val))
+		return;
+
+	charge_current = charger_current_map_desc[val & 0x07];
+
+	target_eoc_ratio = max8998->eoc_in_mA / charge_current * 100;
+
+	if (target_eoc_ratio < 10)
+		target_eoc_ratio = 10;
+	if (target_eoc_ratio > 45)
+		target_eoc_ratio = 45;
+
+	max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+			(target_eoc_ratio / 5 - 2) << 5,
+			0x7 << 5);
+}
+
+/*
+ * max89998_update_eoc() sets EOC ratio. It uses
+ * the greatest EOC ratio that results in equal to or lower than the
+ * specified "eoc_in_mA" value. If no such value exists, it's 10%.
+**/
+void max8998_update_eoc(struct max8998_dev *mdev)
+{
+	_max8998_update_eoc(mdev->battery);
+}
+
+static __devinit int max8998_battery_probe(struct platform_device *pdev)
+{
+	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_battery_data *max8998;
+	struct i2c_client *i2c;
+	int ret = 0;
+
+	if (!pdata) {
+		dev_err(pdev->dev.parent, "No platform init data supplied\n");
+		return -ENODEV;
+	}
+
+	max8998 = kzalloc(sizeof(struct max8998_battery_data), GFP_KERNEL);
+	if (!max8998)
+		return -ENOMEM;
+
+	max8998->eoc_in_mA = 0;
+	max8998->dev = &pdev->dev;
+	max8998->iodev = iodev;
+	iodev->battery = pdev;
+	platform_set_drvdata(pdev, max8998);
+	i2c = max8998->iodev->i2c;
+
+	/* Setup "End of Charge" */
+	if (pdata->eoc >= 10 && pdata->eoc <= 45) {
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+				(pdata->eoc / 5 - 2) << 5, 0x7 << 5);
+	} else if (pdata->eoc >= 50) {
+		max8998->eoc_in_mA = pdata->eoc;
+		_max8998_update_eoc(pdev);
+	} else {
+		dev_info(max8998->dev, "EOC value not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Restart Level */
+	switch (pdata->restart) {
+	case 100:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x1 << 3, 0x3 << 3);
+		break;
+	case 150:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x0 << 3, 0x3 << 3);
+		break;
+	case 200:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x2 << 3, 0x3 << 3);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x3 << 3, 0x3 << 3);
+		break;
+	default:
+		dev_info(max8998->dev, "Restart Level not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Full Timeout */
+	switch (pdata->timeout) {
+	case 5:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x0 << 4, 0x3 << 4);
+		break;
+	case 6:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x1 << 4, 0x3 << 4);
+		break;
+	case 7:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x2 << 4, 0x3 << 4);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x3 << 4, 0x3 << 4);
+		break;
+	default:
+		dev_info(max8998->dev, "Full Timeout not set: leave it unchanged.\n");
+	}
+
+	max8998->battery.name = "max8998_pmic";
+	max8998->battery.type = POWER_SUPPLY_TYPE_BATTERY;
+	max8998->battery.get_property = max8998_battery_get_property;
+	max8998->battery.properties = max8998_battery_props;
+	max8998->battery.num_properties = ARRAY_SIZE(max8998_battery_props);
+
+	ret = power_supply_register(max8998->dev, &max8998->battery);
+	if (ret) {
+		dev_err(max8998->dev, "failed: power supply register\n");
+		kfree(max8998);
+		return ret;
+	}
+
+	max8998->device_id = pdev->id_entry->driver_data;
+
+	return 0;
+}
+
+static int __devexit max8998_battery_remove(struct platform_device *pdev)
+{
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+
+	power_supply_unregister(&max8998->battery);
+	kfree(max8998);
+
+	return 0;
+}
+
+static const struct platform_device_id max8998_battery_id[] = {
+	{ "max8998-battery", TYPE_MAX8998 },
+	{ "lp3974-battery", TYPE_LP3974 },
+};
+
+static struct platform_driver max8998_battery_driver = {
+	.driver = {
+		.name = "max8998-battery",
+		.owner = THIS_MODULE,
+	},
+	.probe = max8998_battery_probe,
+	.remove = __devexit_p(max8998_battery_remove),
+	.id_table = max8998_battery_id,
+};
+
+static int __init max8998_battery_init(void)
+{
+	return platform_driver_register(&max8998_battery_driver);
+}
+module_init(max8998_battery_init);
+
+static void __exit max8998_battery_cleanup(void)
+{
+	platform_driver_unregister(&max8998_battery_driver);
+}
+module_exit(max8998_battery_cleanup);
+
+MODULE_DESCRIPTION("MAXIM 8998 battery control driver");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 9946488..0d38e64 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -86,6 +86,13 @@ static const struct voltage_map_desc buck3_voltage_map_desc = {
 static const struct voltage_map_desc buck4_voltage_map_desc = {
 	.min = 800,	.step = 100,	.max = 2300,
 };
+static const int charger_current_map_desc_max8998[] = {
+	90, 380, 475, 550, 570, 600, 700, 800
+};
+static const int charger_current_map_desc_lp3974[] = {
+	100, 400, 450, 500, 550, 600, 700, 800
+};
+const int *charger_current_map_desc;
 
 static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,
@@ -115,6 +122,7 @@ static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,				/* ENVICHG */
 	NULL,				/* ESAFEOUT1 */
 	NULL,				/* ESAFEOUT2 */
+	NULL,				/* CHARGER */
 };
 
 static inline int max8998_get_ldo(struct regulator_dev *rdev)
@@ -173,6 +181,10 @@ static int max8998_get_enable_register(struct regulator_dev *rdev,
 		*reg = MAX8998_REG_CHGR2;
 		*shift = 7 - (ldo - MAX8998_ESAFEOUT1);
 		break;
+	case MAX8998_CHARGER:
+		*reg = MAX8998_REG_CHGR2;
+		*shift = 0;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -198,6 +210,14 @@ static int max8998_ldo_is_enabled(struct regulator_dev *rdev)
 	return val & (1 << shift);
 }
 
+static int max8998_ldo_is_enabled_negated(struct regulator_dev *rdev)
+{
+	int ret = max8998_ldo_is_enabled(rdev);
+	if (ret >= 0)
+		ret = !ret;
+	return ret;
+}
+
 static int max8998_ldo_enable(struct regulator_dev *rdev)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
@@ -503,6 +523,76 @@ buck2_exit:
 	return ret;
 }
 
+static int max8998_charger_current_set(struct regulator_dev *rdev,
+					int min_uA, int max_uA)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+	int chosen = -1, chosen_current = -1;
+	int i;
+
+	if (!charger_current_map_desc)
+		return -ENXIO;
+
+	for (i = 0; i < (mask + 1); i++) {
+		int temp = charger_current_map_desc[i];
+		if (temp >= (min_uA / 1000) && temp <= (max_uA / 1000) &&
+				temp > chosen_current) {
+			chosen = i;
+			chosen_current = temp;
+		}
+	}
+
+	if (chosen < 0)
+		return -EINVAL;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val &= ~(mask << shift);
+	val |= (chosen & mask) << shift;
+
+	ret = max8998_write_reg(i2c, reg, val);
+	if (ret)
+		return ret;
+
+	dev_info(&rdev->dev, "charger current limit = %dmA (%xh)\n",
+			chosen_current, chosen);
+
+	max8998_update_eoc(max8998->iodev);
+
+	return 0;
+}
+
+static int max8998_charger_current_get(struct regulator_dev *rdev)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val >>= shift;
+	val &= mask;
+
+	if (!charger_current_map_desc)
+		return -ENXIO;
+
+	return charger_current_map_desc[val] * 1000;
+}
+
 static struct regulator_ops max8998_ldo_ops = {
 	.list_voltage		= max8998_list_voltage,
 	.is_enabled		= max8998_ldo_is_enabled,
@@ -533,6 +623,20 @@ static struct regulator_ops max8998_others_ops = {
 	.set_suspend_disable	= max8998_ldo_disable,
 };
 
+/* The enable bit is negated */
+static struct regulator_ops max8998_charger_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+	/* enable and disable are intentionally negated */
+	.enable			= max8998_ldo_disable,
+	.disable		= max8998_ldo_enable,
+	.set_current_limit	= max8998_charger_current_set,
+	.get_current_limit	= max8998_charger_current_get,
+};
+
+static struct regulator_ops max8998_neg_online_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+};
+
 static struct regulator_desc regulators[] = {
 	{
 		.name		= "LDO2",
@@ -684,7 +788,13 @@ static struct regulator_desc regulators[] = {
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
-	}
+	}, {
+		.name		= "CHARGER",
+		.id		= MAX8998_CHARGER,
+		.ops		= &max8998_charger_ops,
+		.type		= REGULATOR_CURRENT,
+		.owner		= THIS_MODULE,
+	},
 };
 
 static __devinit int max8998_pmic_probe(struct platform_device *pdev)
@@ -834,13 +944,27 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	switch (pdev->id_entry->driver_data) {
+	case TYPE_MAX8998:
+		charger_current_map_desc = charger_current_map_desc_max8998;
+		break;
+	case TYPE_LP3974:
+		charger_current_map_desc = charger_current_map_desc_lp3974;
+		break;
+	default:
+		ret = -ENODEV;
+		goto err;
+	}
+
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct voltage_map_desc *desc;
 		int id = pdata->regulators[i].id;
 		int index = id - MAX8998_LDO2;
 
 		desc = ldo_voltage_map[id];
-		if (desc && regulators[index].ops != &max8998_others_ops) {
+		if (desc && regulators[index].ops != &max8998_others_ops &&
+		    regulators[index].ops != &max8998_charger_ops &&
+		    regulators[index].ops != &max8998_neg_online_ops) {
 			int count = (desc->max - desc->min) / desc->step + 1;
 			regulators[index].n_voltages = count;
 		}
@@ -854,7 +978,6 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-
 	return 0;
 err:
 	for (i = 0; i < max8998->num_regulators; i++)
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index effa5d3..bfa558f 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -102,10 +102,11 @@ enum {
 };
 
 /* MAX8998 various variants */
-enum {
+enum max8998_type {
 	TYPE_MAX8998 = 0, /* Default */
 	TYPE_LP3974,	/* National version of MAX8998 */
 	TYPE_LP3979,	/* Added AVS */
+	TYPE_UNKNOWN,
 };
 
 #define MAX8998_IRQ_DCINF_MASK		(1 << 2)
@@ -145,6 +146,7 @@ enum {
  * @irq_masks_cur: currently active value
  * @irq_masks_cache: cached hardware value
  * @type: indicate which max8998 "variant" is used
+ * @battery: the max8998 battery control device
  */
 struct max8998_dev {
 	struct device *dev;
@@ -160,6 +162,7 @@ struct max8998_dev {
 	u8 irq_masks_cache[MAX8998_NUM_IRQ_REGS];
 	int type;
 	bool wakeup;
+	struct platform_device *battery;
 };
 
 int max8998_irq_init(struct max8998_dev *max8998);
@@ -174,4 +177,11 @@ extern int max8998_bulk_write(struct i2c_client *i2c, u8 reg, int count,
 		u8 *buf);
 extern int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask);
 
+#ifdef CONFIG_CHARGERCTRL_MAX8998
+extern void max8998_update_eoc(struct max8998_dev *mdev);
+extern const int *charger_current_map_desc;
+#else
+static void __maybe_unused max8998_update_eoc(struct max8998_dev *mdev) { }
+#endif
+
 #endif /*  __LINUX_MFD_MAX8998_PRIV_H */
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 61daa16..341654c 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -52,6 +52,12 @@ enum {
 	MAX8998_ENVICHG,
 	MAX8998_ESAFEOUT1,
 	MAX8998_ESAFEOUT2,
+	/*
+	 * CHARGER: Controls ON/OFF, current limit of the charger.
+	 * However, note that even if CHARGER is ON, CHARGER_ONLINE
+	 * can be in "disabled" state by MAX8998 internal control.
+	**/
+	MAX8998_CHARGER,
 };
 
 /**
@@ -87,6 +93,12 @@ struct max8998_regulator_data {
  * @wakeup: Allow to wake up from suspend
  * @rtc_delay: LP3974 RTC chip bug that requires delay after a register
  * write before reading it.
+ * @eoc: End of Charge Level: 10 ~ 45% or if it's over 45, in mA.
+ *   If it's under 10, leave it unchanged.
+ * @restart: Restart Level in mV: 100, 150, 200, and -1 for disable.
+ *   Otherwise, leave it unchanged.
+ * @timeout: Full Timeout in hours: 5, 6, 7, and -1 for disable.
+ *  Otherwise, leave it unchanged.
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
@@ -107,6 +119,9 @@ struct max8998_platform_data {
 	int				buck2_default_idx;
 	bool				wakeup;
 	bool				rtc_delay;
+	int				eoc;
+	int				restart;
+	int				timeout;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

Re: [PATCH v4 3/3] MFD MAX8998/LP3974: Support Charger

[Mark Brown]

On Tue, Jan 04, 2011 at 02:17:41PM +0900, MyungJoo Ham wrote:
> With the new regulator, "CHARGER", users can control charging
> current and turn on and off the charger. Note that the charger
> specification of LP3974 is different from that of MAX8998.
> 
> driver/power/max8998.c supports power supply APIs for

It's probably better to split the power supply driver into a separate
patch as there should be no build time dependency between the two.  I've
added Anton to the CCs as he is the drivers/power maintainer.

One thing that jumps out here is that there's no regulator API usage at
all in the power driver - the power driver just jumps straight in
and updates registers when it needs to do anything.  Are you sure that
the regulator API driver is needed at all, if it is I'd expect to see
use of it in the power API.

> +static const char * const manufacturers[] = {
> +	[TYPE_MAX8998] = "Maxim",
> +	[TYPE_LP3974] = "National Semiconductor",
> +	[TYPE_UNKNOWN] = "Unknown",

Normally this refers to the battery rather than the chip.

> +static void _max8998_update_eoc(struct platform_device *pdev)
> +{
> +	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
> +	struct i2c_client *i2c = max8998->iodev->i2c;
> +	int charge_current = 0;
> +	int target_eoc_ratio;
> +	u8 val;
> +
> +	/* Nothing to do. User set EOC with % */
> +	if (max8998->eoc_in_mA == 0)
> +		return;
> +
> +	/* Not initialized. */
> +	if (!charger_current_map_desc)
> +		return;

This should be be driver data rather than a global for neatness (though
in reality the chances of more than one of these chargers in a system
are pretty low).

> --- a/drivers/regulator/max8998.c
> +++ b/drivers/regulator/max8998.c
> @@ -86,6 +86,13 @@ static const struct voltage_map_desc buck3_voltage_map_desc = {
>  static const struct voltage_map_desc buck4_voltage_map_desc = {
>  	.min = 800,	.step = 100,	.max = 2300,
>  };
> +static const int charger_current_map_desc_max8998[] = {
> +	90, 380, 475, 550, 570, 600, 700, 800
> +};
> +static const int charger_current_map_desc_lp3974[] = {
> +	100, 400, 450, 500, 550, 600, 700, 800
> +};
> +const int *charger_current_map_desc;

Ah, this is exported from the regulator driver...  That's slightly odd.
For voltages we've an enumeration API for the supported settings, we
probably ought to add that for current regulators too.

> +	dev_info(&rdev->dev, "charger current limit = %dmA (%xh)\n",
> +			chosen_current, chosen);

dev_dbg() or remove it entirely please, otherwise it might get a bit
noisy.

> +static struct regulator_ops max8998_charger_ops = {
> +	.is_enabled		= max8998_ldo_is_enabled_negated,
> +	/* enable and disable are intentionally negated */
> +	.enable			= max8998_ldo_disable,
> +	.disable		= max8998_ldo_enable,

That's really confusing...  why?

Re: [PATCH v4 3/3] MFD MAX8998/LP3974: Support Charger

[MyungJoo Ham]

On Tue, Jan 4, 2011 at 10:56 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Tue, Jan 04, 2011 at 02:17:41PM +0900, MyungJoo Ham wrote:
>> With the new regulator, "CHARGER", users can control charging
>> current and turn on and off the charger. Note that the charger
>> specification of LP3974 is different from that of MAX8998.
>>
>> driver/power/max8998.c supports power supply APIs for
>
> It's probably better to split the power supply driver into a separate
> patch as there should be no build time dependency between the two.  I've
> added Anton to the CCs as he is the drivers/power maintainer.
>
> One thing that jumps out here is that there's no regulator API usage at
> all in the power driver - the power driver just jumps straight in
> and updates registers when it needs to do anything.  Are you sure that
> the regulator API driver is needed at all, if it is I'd expect to see
> use of it in the power API.
>
>> +static const char * const manufacturers[] = {
>> +     [TYPE_MAX8998] = "Maxim",
>> +     [TYPE_LP3974] = "National Semiconductor",
>> +     [TYPE_UNKNOWN] = "Unknown",
>
> Normally this refers to the battery rather than the chip.
>
>> +static void _max8998_update_eoc(struct platform_device *pdev)
>> +{
>> +     struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
>> +     struct i2c_client *i2c = max8998->iodev->i2c;
>> +     int charge_current = 0;
>> +     int target_eoc_ratio;
>> +     u8 val;
>> +
>> +     /* Nothing to do. User set EOC with % */
>> +     if (max8998->eoc_in_mA == 0)
>> +             return;
>> +
>> +     /* Not initialized. */
>> +     if (!charger_current_map_desc)
>> +             return;
>
> This should be be driver data rather than a global for neatness (though
> in reality the chances of more than one of these chargers in a system
> are pretty low).

Ok. I'll make it possible to have max8998 and lp3974 at the same time
on a board. :)

>
>> --- a/drivers/regulator/max8998.c
>> +++ b/drivers/regulator/max8998.c
>> @@ -86,6 +86,13 @@ static const struct voltage_map_desc buck3_voltage_map_desc = {
>>  static const struct voltage_map_desc buck4_voltage_map_desc = {
>>       .min = 800,     .step = 100,    .max = 2300,
>>  };
>> +static const int charger_current_map_desc_max8998[] = {
>> +     90, 380, 475, 550, 570, 600, 700, 800
>> +};
>> +static const int charger_current_map_desc_lp3974[] = {
>> +     100, 400, 450, 500, 550, 600, 700, 800
>> +};
>> +const int *charger_current_map_desc;
>
> Ah, this is exported from the regulator driver...  That's slightly odd.
> For voltages we've an enumeration API for the supported settings, we
> probably ought to add that for current regulators too.
>
>> +     dev_info(&rdev->dev, "charger current limit = %dmA (%xh)\n",
>> +                     chosen_current, chosen);
>
> dev_dbg() or remove it entirely please, otherwise it might get a bit
> noisy.

Ok.

>
>> +static struct regulator_ops max8998_charger_ops = {
>> +     .is_enabled             = max8998_ldo_is_enabled_negated,
>> +     /* enable and disable are intentionally negated */
>> +     .enable                 = max8998_ldo_disable,
>> +     .disable                = max8998_ldo_enable,
>
> That's really confusing...  why?
>

I've negated max8998_charger_ops's enable/disable because other
max8998 registers use 1 for "ON" while this register use 0 for "ON".

-- 
MyungJoo Ham (함명주), Ph.D.
Mobile Software Platform Lab,
Digital Media and Communications (DMC) Business
Samsung Electronics
cell: 82-10-6714-2858

[PATCH v5] MFD MAX8998/LP3974: Support Charger

[MyungJoo Ham]

With the new regulator, "CHARGER", users can control charging
current and turn on and off the charger. Note that the charger
specification of LP3974 is different from that of MAX8998.

driver/power/max8998.c supports power supply APIs for

1. "ONLINE" monitors the charger status, which can be
different from the status "CHARGER"; e.g., users allowed the charger
to charge, but the MAX8998 chip decided not to do so.

2. "PRESENT" monitors the battery status (the existence of the
battery).

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
--
Updates from v4
- Patch 1/3 and 2/3 have been applied and removed from v5 patchset.
- Removed manufacture section from power supply sysfs.
- We may have multiple (and different) MAX8998/LP3974 chips on a board.
(moved a global variable into driver data)
- dev_into -> dev_dbg
---
 drivers/mfd/max8998.c               |    4 +
 drivers/power/Kconfig               |    7 +
 drivers/power/Makefile              |    1 +
 drivers/power/max8998.c             |  251 +++++++++++++++++++++++++++++++++++
 drivers/regulator/max8998.c         |  133 ++++++++++++++++++-
 include/linux/mfd/max8998-private.h |   14 ++-
 include/linux/mfd/max8998.h         |   15 ++
 7 files changed, 421 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/max8998.c

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index bbfe867..4716003 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -39,6 +39,8 @@ static struct mfd_cell max8998_devs[] = {
 		.name = "max8998-pmic",
 	}, {
 		.name = "max8998-rtc",
+	}, {
+		.name = "max8998-battery",
 	},
 };
 
@@ -47,6 +49,8 @@ static struct mfd_cell lp3974_devs[] = {
 		.name = "lp3974-pmic",
 	}, {
 		.name = "lp3974-rtc",
+	}, {
+		.name = "lp3974-battery",
 	},
 };
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 60d83d9..06b720c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -185,4 +185,11 @@ config CHARGER_TWL4030
 	help
 	  Say Y here to enable support for TWL4030 Battery Charge Interface.
 
+config CHARGERCTRL_MAX8998
+	tristate "Battery charger driver for MAX8998/LP3974 PMIC"
+	depends on MFD_MAX8998 && REGULATOR_MAX8998
+	help
+	  Say Y to enable support for the battery charger control sysfs and
+	  platform data of MAX8998/LP3974 PMICs.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index c75772e..4e66b91 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_BATTERY_JZ4740)	+= jz4740-battery.o
 obj-$(CONFIG_BATTERY_INTEL_MID)	+= intel_mid_battery.o
 obj-$(CONFIG_CHARGER_ISP1704)	+= isp1704_charger.o
 obj-$(CONFIG_CHARGER_TWL4030)	+= twl4030_charger.o
+obj-$(CONFIG_CHARGERCTRL_MAX8998)	+= max8998.o
diff --git a/drivers/power/max8998.c b/drivers/power/max8998.c
new file mode 100644
index 0000000..9453992
--- /dev/null
+++ b/drivers/power/max8998.c
@@ -0,0 +1,251 @@
+/*
+ * max8998.c - Power supply consumer driver for the Maxim 8998
+ *
+ *  Copyright (C) 2009-2010 Samsung Electronics
+ *  MyungJoo Ham <myungjoo.ham@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
+ */
+
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/max8998.h>
+#include <linux/mfd/max8998-private.h>
+
+struct max8998_battery_data {
+	struct device *dev;
+	struct max8998_dev *iodev;
+	unsigned int eoc_in_mA;
+	struct power_supply battery;
+	enum max8998_type device_id;
+};
+
+static enum power_supply_property max8998_battery_props[] = {
+	POWER_SUPPLY_PROP_PRESENT, /* the presence of battery */
+	POWER_SUPPLY_PROP_ONLINE, /* charger is active or not */
+};
+
+/* Note that the charger control is done by a current regulator "CHARGER" */
+static int max8998_battery_get_property(struct power_supply *psy,
+		enum power_supply_property psp,
+		union power_supply_propval *val)
+{
+	struct max8998_battery_data *max8998 = container_of(psy,
+			struct max8998_battery_data, battery);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int ret;
+	u8 reg;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 4))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 3))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void _max8998_update_eoc(struct platform_device *pdev)
+{
+	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int charge_current = 0;
+	int target_eoc_ratio;
+	u8 val;
+
+	/* Nothing to do. User set EOC with % */
+	if (max8998->eoc_in_mA == 0)
+		return;
+
+	/* Not initialized. */
+	if (!iodev->charger_current_map_desc)
+		return;
+
+	if (max8998_read_reg(i2c, MAX8998_REG_CHGR1, &val))
+		return;
+
+	charge_current = iodev->charger_current_map_desc[val & 0x07];
+
+	target_eoc_ratio = max8998->eoc_in_mA / charge_current * 100;
+
+	if (target_eoc_ratio < 10)
+		target_eoc_ratio = 10;
+	if (target_eoc_ratio > 45)
+		target_eoc_ratio = 45;
+
+	max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+			(target_eoc_ratio / 5 - 2) << 5,
+			0x7 << 5);
+}
+
+/*
+ * max89998_update_eoc() sets EOC ratio. It uses
+ * the greatest EOC ratio that results in equal to or lower than the
+ * specified "eoc_in_mA" value. If no such value exists, it's 10%.
+**/
+void max8998_update_eoc(struct max8998_dev *mdev)
+{
+	_max8998_update_eoc(mdev->battery);
+}
+
+static __devinit int max8998_battery_probe(struct platform_device *pdev)
+{
+	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_battery_data *max8998;
+	struct i2c_client *i2c;
+	int ret = 0;
+
+	if (!pdata) {
+		dev_err(pdev->dev.parent, "No platform init data supplied\n");
+		return -ENODEV;
+	}
+
+	max8998 = kzalloc(sizeof(struct max8998_battery_data), GFP_KERNEL);
+	if (!max8998)
+		return -ENOMEM;
+
+	max8998->eoc_in_mA = 0;
+	max8998->dev = &pdev->dev;
+	max8998->iodev = iodev;
+	iodev->battery = pdev;
+	platform_set_drvdata(pdev, max8998);
+	i2c = max8998->iodev->i2c;
+
+	/* Setup "End of Charge" */
+	if (pdata->eoc >= 10 && pdata->eoc <= 45) {
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+				(pdata->eoc / 5 - 2) << 5, 0x7 << 5);
+	} else if (pdata->eoc >= 50) {
+		max8998->eoc_in_mA = pdata->eoc;
+		_max8998_update_eoc(pdev);
+	} else {
+		dev_info(max8998->dev, "EOC value not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Restart Level */
+	switch (pdata->restart) {
+	case 100:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x1 << 3, 0x3 << 3);
+		break;
+	case 150:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x0 << 3, 0x3 << 3);
+		break;
+	case 200:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x2 << 3, 0x3 << 3);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x3 << 3, 0x3 << 3);
+		break;
+	default:
+		dev_info(max8998->dev, "Restart Level not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Full Timeout */
+	switch (pdata->timeout) {
+	case 5:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x0 << 4, 0x3 << 4);
+		break;
+	case 6:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x1 << 4, 0x3 << 4);
+		break;
+	case 7:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x2 << 4, 0x3 << 4);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x3 << 4, 0x3 << 4);
+		break;
+	default:
+		dev_info(max8998->dev, "Full Timeout not set: leave it unchanged.\n");
+	}
+
+	max8998->battery.name = "max8998_pmic";
+	max8998->battery.type = POWER_SUPPLY_TYPE_BATTERY;
+	max8998->battery.get_property = max8998_battery_get_property;
+	max8998->battery.properties = max8998_battery_props;
+	max8998->battery.num_properties = ARRAY_SIZE(max8998_battery_props);
+
+	ret = power_supply_register(max8998->dev, &max8998->battery);
+	if (ret) {
+		dev_err(max8998->dev, "failed: power supply register\n");
+		kfree(max8998);
+		return ret;
+	}
+
+	max8998->device_id = pdev->id_entry->driver_data;
+
+	return 0;
+}
+
+static int __devexit max8998_battery_remove(struct platform_device *pdev)
+{
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+
+	power_supply_unregister(&max8998->battery);
+	kfree(max8998);
+
+	return 0;
+}
+
+static const struct platform_device_id max8998_battery_id[] = {
+	{ "max8998-battery", TYPE_MAX8998 },
+	{ "lp3974-battery", TYPE_LP3974 },
+};
+
+static struct platform_driver max8998_battery_driver = {
+	.driver = {
+		.name = "max8998-battery",
+		.owner = THIS_MODULE,
+	},
+	.probe = max8998_battery_probe,
+	.remove = __devexit_p(max8998_battery_remove),
+	.id_table = max8998_battery_id,
+};
+
+static int __init max8998_battery_init(void)
+{
+	return platform_driver_register(&max8998_battery_driver);
+}
+module_init(max8998_battery_init);
+
+static void __exit max8998_battery_cleanup(void)
+{
+	platform_driver_unregister(&max8998_battery_driver);
+}
+module_exit(max8998_battery_cleanup);
+
+MODULE_DESCRIPTION("MAXIM 8998 battery control driver");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 9946488..ef35743 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -86,6 +86,12 @@ static const struct voltage_map_desc buck3_voltage_map_desc = {
 static const struct voltage_map_desc buck4_voltage_map_desc = {
 	.min = 800,	.step = 100,	.max = 2300,
 };
+static const int charger_current_map_desc_max8998[] = {
+	90, 380, 475, 550, 570, 600, 700, 800
+};
+static const int charger_current_map_desc_lp3974[] = {
+	100, 400, 450, 500, 550, 600, 700, 800
+};
 
 static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,
@@ -115,6 +121,7 @@ static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,				/* ENVICHG */
 	NULL,				/* ESAFEOUT1 */
 	NULL,				/* ESAFEOUT2 */
+	NULL,				/* CHARGER */
 };
 
 static inline int max8998_get_ldo(struct regulator_dev *rdev)
@@ -173,6 +180,10 @@ static int max8998_get_enable_register(struct regulator_dev *rdev,
 		*reg = MAX8998_REG_CHGR2;
 		*shift = 7 - (ldo - MAX8998_ESAFEOUT1);
 		break;
+	case MAX8998_CHARGER:
+		*reg = MAX8998_REG_CHGR2;
+		*shift = 0;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -198,6 +209,14 @@ static int max8998_ldo_is_enabled(struct regulator_dev *rdev)
 	return val & (1 << shift);
 }
 
+static int max8998_ldo_is_enabled_negated(struct regulator_dev *rdev)
+{
+	int ret = max8998_ldo_is_enabled(rdev);
+	if (ret >= 0)
+		ret = !ret;
+	return ret;
+}
+
 static int max8998_ldo_enable(struct regulator_dev *rdev)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
@@ -503,6 +522,77 @@ buck2_exit:
 	return ret;
 }
 
+static int max8998_charger_current_set(struct regulator_dev *rdev,
+					int min_uA, int max_uA)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+	int chosen = -1, chosen_current = -1;
+	int i;
+
+	if (!max8998->iodev->charger_current_map_desc)
+		return -ENXIO;
+
+	for (i = 0; i < (mask + 1); i++) {
+		int temp = max8998->iodev->charger_current_map_desc[i];
+		if (temp >= (min_uA / 1000) && temp <= (max_uA / 1000) &&
+				temp > chosen_current) {
+			chosen = i;
+			chosen_current = temp;
+		}
+	}
+
+	if (chosen < 0)
+		return -EINVAL;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val &= ~(mask << shift);
+	val |= (chosen & mask) << shift;
+
+	ret = max8998_write_reg(i2c, reg, val);
+	if (ret)
+		return ret;
+
+	dev_dbg(&rdev->dev, "charger current limit = %dmA (%xh)\n",
+			chosen_current, chosen);
+
+	max8998_update_eoc(max8998->iodev);
+
+	return 0;
+}
+
+static int max8998_charger_current_get(struct regulator_dev *rdev)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct max8998_dev *iodev = max8998->iodev;
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val >>= shift;
+	val &= mask;
+
+	if (!iodev->charger_current_map_desc)
+		return -ENXIO;
+
+	return iodev->charger_current_map_desc[val] * 1000;
+}
+
 static struct regulator_ops max8998_ldo_ops = {
 	.list_voltage		= max8998_list_voltage,
 	.is_enabled		= max8998_ldo_is_enabled,
@@ -533,6 +623,22 @@ static struct regulator_ops max8998_others_ops = {
 	.set_suspend_disable	= max8998_ldo_disable,
 };
 
+static struct regulator_ops max8998_charger_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+	/*
+	 * Enable and disable are intentionally negated as the charger
+	 * control bit is active-negative while others are active-positive.
+	 */
+	.enable			= max8998_ldo_disable,
+	.disable		= max8998_ldo_enable,
+	.set_current_limit	= max8998_charger_current_set,
+	.get_current_limit	= max8998_charger_current_get,
+};
+
+static struct regulator_ops max8998_neg_online_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+};
+
 static struct regulator_desc regulators[] = {
 	{
 		.name		= "LDO2",
@@ -684,7 +790,13 @@ static struct regulator_desc regulators[] = {
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
-	}
+	}, {
+		.name		= "CHARGER",
+		.id		= MAX8998_CHARGER,
+		.ops		= &max8998_charger_ops,
+		.type		= REGULATOR_CURRENT,
+		.owner		= THIS_MODULE,
+	},
 };
 
 static __devinit int max8998_pmic_probe(struct platform_device *pdev)
@@ -834,13 +946,29 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	switch (pdev->id_entry->driver_data) {
+	case TYPE_MAX8998:
+		iodev->charger_current_map_desc =
+			charger_current_map_desc_max8998;
+		break;
+	case TYPE_LP3974:
+		iodev->charger_current_map_desc =
+			charger_current_map_desc_lp3974;
+		break;
+	default:
+		ret = -ENODEV;
+		goto err;
+	}
+
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct voltage_map_desc *desc;
 		int id = pdata->regulators[i].id;
 		int index = id - MAX8998_LDO2;
 
 		desc = ldo_voltage_map[id];
-		if (desc && regulators[index].ops != &max8998_others_ops) {
+		if (desc && regulators[index].ops != &max8998_others_ops &&
+		    regulators[index].ops != &max8998_charger_ops &&
+		    regulators[index].ops != &max8998_neg_online_ops) {
 			int count = (desc->max - desc->min) / desc->step + 1;
 			regulators[index].n_voltages = count;
 		}
@@ -854,7 +982,6 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-
 	return 0;
 err:
 	for (i = 0; i < max8998->num_regulators; i++)
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index effa5d3..9a25b5b 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -102,10 +102,11 @@ enum {
 };
 
 /* MAX8998 various variants */
-enum {
+enum max8998_type {
 	TYPE_MAX8998 = 0, /* Default */
 	TYPE_LP3974,	/* National version of MAX8998 */
 	TYPE_LP3979,	/* Added AVS */
+	TYPE_UNKNOWN,
 };
 
 #define MAX8998_IRQ_DCINF_MASK		(1 << 2)
@@ -145,6 +146,7 @@ enum {
  * @irq_masks_cur: currently active value
  * @irq_masks_cache: cached hardware value
  * @type: indicate which max8998 "variant" is used
+ * @battery: the max8998 battery control device
  */
 struct max8998_dev {
 	struct device *dev;
@@ -160,6 +162,10 @@ struct max8998_dev {
 	u8 irq_masks_cache[MAX8998_NUM_IRQ_REGS];
 	int type;
 	bool wakeup;
+	struct platform_device *battery;
+
+	/* Shared between power/max8998 and regulator/max8998. */
+	const int *charger_current_map_desc;
 };
 
 int max8998_irq_init(struct max8998_dev *max8998);
@@ -174,4 +180,10 @@ extern int max8998_bulk_write(struct i2c_client *i2c, u8 reg, int count,
 		u8 *buf);
 extern int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask);
 
+#ifdef CONFIG_CHARGERCTRL_MAX8998
+extern void max8998_update_eoc(struct max8998_dev *mdev);
+#else
+static void __maybe_unused max8998_update_eoc(struct max8998_dev *mdev) { }
+#endif
+
 #endif /*  __LINUX_MFD_MAX8998_PRIV_H */
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 61daa16..341654c 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -52,6 +52,12 @@ enum {
 	MAX8998_ENVICHG,
 	MAX8998_ESAFEOUT1,
 	MAX8998_ESAFEOUT2,
+	/*
+	 * CHARGER: Controls ON/OFF, current limit of the charger.
+	 * However, note that even if CHARGER is ON, CHARGER_ONLINE
+	 * can be in "disabled" state by MAX8998 internal control.
+	**/
+	MAX8998_CHARGER,
 };
 
 /**
@@ -87,6 +93,12 @@ struct max8998_regulator_data {
  * @wakeup: Allow to wake up from suspend
  * @rtc_delay: LP3974 RTC chip bug that requires delay after a register
  * write before reading it.
+ * @eoc: End of Charge Level: 10 ~ 45% or if it's over 45, in mA.
+ *   If it's under 10, leave it unchanged.
+ * @restart: Restart Level in mV: 100, 150, 200, and -1 for disable.
+ *   Otherwise, leave it unchanged.
+ * @timeout: Full Timeout in hours: 5, 6, 7, and -1 for disable.
+ *  Otherwise, leave it unchanged.
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
@@ -107,6 +119,9 @@ struct max8998_platform_data {
 	int				buck2_default_idx;
 	bool				wakeup;
 	bool				rtc_delay;
+	int				eoc;
+	int				restart;
+	int				timeout;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

Re: [PATCH v5] MFD MAX8998/LP3974: Support Charger

[Mark Brown]

On Fri, Jan 14, 2011 at 02:22:59PM +0900, MyungJoo Ham wrote:
> With the new regulator, "CHARGER", users can control charging
> current and turn on and off the charger. Note that the charger
> specification of LP3974 is different from that of MAX8998.
> 
> driver/power/max8998.c supports power supply APIs for
> 
> 1. "ONLINE" monitors the charger status, which can be
> different from the status "CHARGER"; e.g., users allowed the charger
> to charge, but the MAX8998 chip decided not to do so.
> 
> 2. "PRESENT" monitors the battery status (the existence of the
> battery).
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

My major concern here is still that the regulator and power drivers
aren't connected at all - it feels like either the regulator driver is
redundant or the power driver ought to be a consumer of the regulator
driver.

[PATCH v3 3/4] regulator MAX8998/LP3974: Support DVS-GPIO.

[MyungJoo Ham]

The previous driver did not support BUCK1-DVS3, BUCK1-DVS4, and
BUCK2-DVS2 modes. This patch adds such modes and an option to block
setting buck1/2 voltages out of the preset values.

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/regulator/max8998.c |   87 +++++++++++++++++++++++++++++++++----------
 include/linux/mfd/max8998.h |   26 ++++++++++---
 2 files changed, 87 insertions(+), 26 deletions(-)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index d183756..9946488 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -425,6 +425,9 @@ static int max8998_set_voltage_buck(struct regulator_dev *rdev,
 				}
 			}
 
+			if (pdata->buck_voltage_lock)
+				return -EINVAL;
+
 			/* no predefine regulator found */
 			max8998->buck1_idx = (buck1_last_val % 2) + 2;
 			dev_dbg(max8998->dev, "max8998->buck1_idx:%d\n",
@@ -452,18 +455,26 @@ buck1_exit:
 			"BUCK2, i:%d buck2_vol1:%d, buck2_vol2:%d\n"
 			, i, max8998->buck2_vol[0], max8998->buck2_vol[1]);
 		if (gpio_is_valid(pdata->buck2_set3)) {
-			if (max8998->buck2_vol[0] == i) {
-				max8998->buck2_idx = 0;
-				buck2_gpio_set(pdata->buck2_set3, 0);
-			} else {
-				max8998->buck2_idx = 1;
-				ret = max8998_get_voltage_register(rdev, &reg,
-								   &shift,
-								   &mask);
-				ret = max8998_write_reg(i2c, reg, i);
-				max8998->buck2_vol[1] = i;
-				buck2_gpio_set(pdata->buck2_set3, 1);
+
+			/* check if requested voltage */
+			/* value is already defined */
+			for (j = 0; j < ARRAY_SIZE(max8998->buck2_vol); j++) {
+				if (max8998->buck2_vol[j] == i) {
+					max8998->buck2_idx = j;
+					buck2_gpio_set(pdata->buck2_set3, j);
+					goto buck2_exit;
+				}
 			}
+
+			if (pdata->buck_voltage_lock)
+				return -EINVAL;
+
+			max8998_get_voltage_register(rdev,
+					&reg, &shift, &mask);
+			ret = max8998_write_reg(i2c, reg, i);
+			max8998->buck2_vol[max8998->buck2_idx] = i;
+			buck2_gpio_set(pdata->buck2_set3, max8998->buck2_idx);
+buck2_exit:
 			dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
 				gpio_get_value(pdata->buck2_set3));
 		} else {
@@ -708,6 +719,9 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, max8998);
 	i2c = max8998->iodev->i2c;
 
+	max8998->buck1_idx = pdata->buck1_default_idx;
+	max8998->buck2_idx = pdata->buck2_default_idx;
+
 	/* NOTE: */
 	/* For unused GPIO NOT marked as -1 (thereof equal to 0)  WARN_ON */
 	/* will be displayed */
@@ -740,23 +754,46 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck1_max_voltage1 / 1000))
+		       < (pdata->buck1_voltage1 / 1000))
 			i++;
-		printk(KERN_ERR "i:%d, buck1_idx:%d\n", i, max8998->buck1_idx);
 		max8998->buck1_vol[0] = i;
 		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE1, i);
+		if (ret)
+			return ret;
 
 		/* Set predefined value for BUCK1 register 2 */
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck1_max_voltage2 / 1000))
+		       < (pdata->buck1_voltage2 / 1000))
 			i++;
 
 		max8998->buck1_vol[1] = i;
-		printk(KERN_ERR "i:%d, buck1_idx:%d\n", i, max8998->buck1_idx);
-		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i)
-			+ ret;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE2, i);
+		if (ret)
+			return ret;
+
+		/* Set predefined value for BUCK1 register 3 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck1_voltage3 / 1000))
+			i++;
+
+		max8998->buck1_vol[2] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE3, i);
+		if (ret)
+			return ret;
+
+		/* Set predefined value for BUCK1 register 4 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck1_voltage4 / 1000))
+			i++;
+
+		max8998->buck1_vol[3] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK1_VOLTAGE4, i);
 		if (ret)
 			return ret;
 
@@ -773,18 +810,28 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		gpio_direction_output(pdata->buck2_set3,
 				      max8998->buck2_idx & 0x1);
 
-		/* BUCK2 - set preset default voltage value to buck2_vol[0] */
+		/* BUCK2 register 1 */
 		i = 0;
 		while (buck12_voltage_map_desc.min +
 		       buck12_voltage_map_desc.step*i
-		       != (pdata->buck2_max_voltage / 1000))
+		       < (pdata->buck2_voltage1 / 1000))
 			i++;
-		printk(KERN_ERR "i:%d, buck2_idx:%d\n", i, max8998->buck2_idx);
 		max8998->buck2_vol[0] = i;
 		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE1, i);
 		if (ret)
 			return ret;
 
+		/* BUCK2 register 2 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       < (pdata->buck2_voltage2 / 1000))
+			i++;
+		printk(KERN_ERR "i2:%d, buck2_idx:%d\n", i, max8998->buck2_idx);
+		max8998->buck2_vol[1] = i;
+		ret = max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOLTAGE2, i);
+		if (ret)
+			return ret;
 	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 686a744..7194c35 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -70,24 +70,38 @@ struct max8998_regulator_data {
  * @num_regulators: number of regultors used
  * @irq_base: base IRQ number for max8998, required for IRQs
  * @ono: power onoff IRQ number for max8998
- * @buck1_max_voltage1: BUCK1 maximum alowed voltage register 1
- * @buck1_max_voltage2: BUCK1 maximum alowed voltage register 2
- * @buck2_max_voltage: BUCK2 maximum alowed voltage
+ * @buck_voltage_lock: Do NOT change the values of the following six
+ *   registers set by buck?_voltage?. The voltage of BUCK1/2 cannot
+ *   be other than the preset values.
+ * @buck1_voltage1: BUCK1 DVS mode 1 voltage register
+ * @buck1_voltage2: BUCK1 DVS mode 2 voltage register
+ * @buck1_voltage3: BUCK1 DVS mode 3 voltage register
+ * @buck1_voltage4: BUCK1 DVS mode 4 voltage register
+ * @buck2_voltage1: BUCK2 DVS mode 1 voltage register
+ * @buck2_voltage2: BUCK2 DVS mode 2 voltage register
  * @buck1_set1: BUCK1 gpio pin 1 to set output voltage
  * @buck1_set2: BUCK1 gpio pin 2 to set output voltage
+ * @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
  * @buck2_set3: BUCK2 gpio pin to set output voltage
+ * @buck2_default_idx: Default for BUCK2 gpio pin.
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
 	int				num_regulators;
 	int				irq_base;
 	int				ono;
-	int                             buck1_max_voltage1;
-	int                             buck1_max_voltage2;
-	int                             buck2_max_voltage;
+	bool				buck_voltage_lock;
+	int				buck1_voltage1;
+	int				buck1_voltage2;
+	int				buck1_voltage3;
+	int				buck1_voltage4;
+	int				buck2_voltage1;
+	int				buck2_voltage2;
 	int				buck1_set1;
 	int				buck1_set2;
+	int				buck1_default_idx;
 	int				buck2_set3;
+	int				buck2_default_idx;
 	bool				wakeup;
 };
 
-- 
1.7.1

Re: [PATCH v3 3/4] regulator MAX8998/LP3974: Support DVS-GPIO.

[Samuel Ortiz]

Hi MyungJoo,

On Thu, Dec 23, 2010 at 05:53:38PM +0900, MyungJoo Ham wrote:
> The previous driver did not support BUCK1-DVS3, BUCK1-DVS4, and
> BUCK2-DVS2 modes. This patch adds such modes and an option to block
> setting buck1/2 voltages out of the preset values.
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
I'm fine with the MFD changes, and I suppose it would make sense for me to
carry this patch. I'll do so once I get Liam or Mark ACK on it.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH v3 3/4] regulator MAX8998/LP3974: Support DVS-GPIO.

[Mark Brown]

On Thu, Dec 23, 2010 at 05:53:38PM +0900, MyungJoo Ham wrote:
> The previous driver did not support BUCK1-DVS3, BUCK1-DVS4, and
> BUCK2-DVS2 modes. This patch adds such modes and an option to block
> setting buck1/2 voltages out of the preset values.

Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

[PATCH v3 4/4] MFD MAX8998/LP3974: Support Charger

[MyungJoo Ham]

With the new regulator, "CHARGER", users can control charging
current and turn on and off the charger. Note that the charger
specification of LP3974 is different from that of MAX8998.

driver/power/max8998.c supports power supply APIs for

1. "ONLINE" monitors the charger status, which can be
different from the status "CHARGER"; e.g., users allowed the charger
to charge, but the MAX8998 chip decided not to do so.

2. "PRESENT" monitors the battery status (the existence of the
battery).

Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998.c               |    6 +
 drivers/power/Kconfig               |    7 +
 drivers/power/Makefile              |    1 +
 drivers/power/max8998.c             |  265 +++++++++++++++++++++++++++++++++++
 drivers/regulator/max8998.c         |  129 +++++++++++++++++-
 include/linux/mfd/max8998-private.h |   12 ++-
 include/linux/mfd/max8998.h         |   15 ++
 7 files changed, 431 insertions(+), 4 deletions(-)
 create mode 100644 drivers/power/max8998.c

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 8b9eed1..fe8d62e 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -39,6 +39,8 @@ static struct mfd_cell max8998_devs[] = {
 		.name = "max8998-pmic",
 	}, {
 		.name = "max8998-rtc",
+	}, {
+		.name = "max8998-battery",
 	},
 };
 
@@ -47,6 +49,8 @@ static struct mfd_cell lp3974_devs[] = {
 		.name = "lp3974-pmic",
 	}, {
 		.name = "lp3974-rtc",
+	}, {
+		.name = "lp3974-battery",
 	},
 };
 
@@ -55,6 +59,8 @@ static struct mfd_cell lp3974_regerr_devs[] = {
 		.name = "lp3974-pmic",
 	}, {
 		.name = "lp3974-rtc-regerr",
+	}, {
+		.name = "lp3974-battery",
 	},
 };
 
diff --git a/drivers/power/Kconfig b/drivers/power/Kconfig
index 60d83d9..06b720c 100644
--- a/drivers/power/Kconfig
+++ b/drivers/power/Kconfig
@@ -185,4 +185,11 @@ config CHARGER_TWL4030
 	help
 	  Say Y here to enable support for TWL4030 Battery Charge Interface.
 
+config CHARGERCTRL_MAX8998
+	tristate "Battery charger driver for MAX8998/LP3974 PMIC"
+	depends on MFD_MAX8998 && REGULATOR_MAX8998
+	help
+	  Say Y to enable support for the battery charger control sysfs and
+	  platform data of MAX8998/LP3974 PMICs.
+
 endif # POWER_SUPPLY
diff --git a/drivers/power/Makefile b/drivers/power/Makefile
index c75772e..4e66b91 100644
--- a/drivers/power/Makefile
+++ b/drivers/power/Makefile
@@ -32,3 +32,4 @@ obj-$(CONFIG_BATTERY_JZ4740)	+= jz4740-battery.o
 obj-$(CONFIG_BATTERY_INTEL_MID)	+= intel_mid_battery.o
 obj-$(CONFIG_CHARGER_ISP1704)	+= isp1704_charger.o
 obj-$(CONFIG_CHARGER_TWL4030)	+= twl4030_charger.o
+obj-$(CONFIG_CHARGERCTRL_MAX8998)	+= max8998.o
diff --git a/drivers/power/max8998.c b/drivers/power/max8998.c
new file mode 100644
index 0000000..6e73b42
--- /dev/null
+++ b/drivers/power/max8998.c
@@ -0,0 +1,265 @@
+/*
+ * max8998.c - Power supply consumer driver for the Maxim 8998
+ *
+ *  Copyright (C) 2009-2010 Samsung Electronics
+ *  MyungJoo Ham <myungjoo.ham@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
+ */
+
+#include <linux/err.h>
+#include <linux/slab.h>
+#include <linux/platform_device.h>
+#include <linux/power_supply.h>
+#include <linux/mfd/max8998.h>
+#include <linux/mfd/max8998-private.h>
+
+struct max8998_battery_data {
+	struct device *dev;
+	struct max8998_dev *iodev;
+	unsigned int eoc_in_mA;
+	struct power_supply battery;
+	enum max8998_type device_id;
+};
+
+static enum power_supply_property max8998_battery_props[] = {
+	POWER_SUPPLY_PROP_MANUFACTURER,
+	POWER_SUPPLY_PROP_PRESENT, /* the presence of battery */
+	POWER_SUPPLY_PROP_ONLINE, /* charger is active or not */
+};
+
+static const char * const manufacturers[] = {
+	[TYPE_MAX8998] = "Maxim",
+	[TYPE_LP3974] = "National Semiconductor",
+	[TYPE_UNKNOWN] = "Unknown",
+};
+
+/* Note that the charger control is done by a current regulator "CHARGER" */
+static int max8998_battery_get_property(struct power_supply *psy,
+		enum power_supply_property psp,
+		union power_supply_propval *val)
+{
+	struct max8998_battery_data *max8998 = container_of(psy,
+			struct max8998_battery_data, battery);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int ret;
+	u8 reg;
+
+	switch (psp) {
+	case POWER_SUPPLY_PROP_MANUFACTURER:
+		if (max8998->device_id == TYPE_MAX8998)
+			val->strval = manufacturers[TYPE_MAX8998];
+		else if (max8998->device_id == TYPE_LP3974)
+			val->strval = manufacturers[TYPE_LP3974];
+		else
+			val->strval = manufacturers[TYPE_UNKNOWN];
+		break;
+	case POWER_SUPPLY_PROP_PRESENT:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 4))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	case POWER_SUPPLY_PROP_ONLINE:
+		ret = max8998_read_reg(i2c, MAX8998_REG_STATUS2, &reg);
+		if (ret)
+			return ret;
+		if (reg & (1 << 3))
+			val->intval = 0;
+		else
+			val->intval = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static void _max8998_update_eoc(struct platform_device *pdev)
+{
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int charge_current = 0;
+	int target_eoc_ratio;
+	u8 val;
+
+	/* Nothing to do. User set EOC with % */
+	if (max8998->eoc_in_mA == 0)
+		return;
+
+	/* Not initialized. */
+	if (!charger_current_map_desc)
+		return;
+
+	if (max8998_read_reg(i2c, MAX8998_REG_CHGR1, &val))
+		return;
+
+	charge_current = charger_current_map_desc[val & 0x07];
+
+	target_eoc_ratio = max8998->eoc_in_mA / charge_current * 100;
+
+	if (target_eoc_ratio < 10)
+		target_eoc_ratio = 10;
+	if (target_eoc_ratio > 45)
+		target_eoc_ratio = 45;
+
+	max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+			(target_eoc_ratio / 5 - 2) << 5,
+			0x7 << 5);
+}
+
+/*
+ * max89998_update_eoc() sets EOC ratio. It uses
+ * the greatest EOC ratio that results in equal to or lower than the
+ * specified "eoc_in_mA" value. If no such value exists, it's 10%.
+**/
+void max8998_update_eoc(struct max8998_dev *mdev)
+{
+	_max8998_update_eoc(mdev->battery);
+}
+
+static __devinit int max8998_battery_probe(struct platform_device *pdev)
+{
+	struct max8998_dev *iodev = dev_get_drvdata(pdev->dev.parent);
+	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
+	struct max8998_battery_data *max8998;
+	struct i2c_client *i2c;
+	int ret = 0;
+
+	if (!pdata) {
+		dev_err(pdev->dev.parent, "No platform init data supplied\n");
+		return -ENODEV;
+	}
+
+	max8998 = kzalloc(sizeof(struct max8998_battery_data), GFP_KERNEL);
+	if (!max8998)
+		return -ENOMEM;
+
+	max8998->eoc_in_mA = 0;
+	max8998->dev = &pdev->dev;
+	max8998->iodev = iodev;
+	iodev->battery = pdev;
+	platform_set_drvdata(pdev, max8998);
+	i2c = max8998->iodev->i2c;
+
+	/* Setup "End of Charge" */
+	if (pdata->eoc >= 10 && pdata->eoc <= 45) {
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1,
+				(pdata->eoc / 5 - 2) << 5, 0x7 << 5);
+	} else if (pdata->eoc >= 50) {
+		max8998->eoc_in_mA = pdata->eoc;
+		_max8998_update_eoc(pdev);
+	} else {
+		dev_info(max8998->dev, "EOC value not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Restart Level */
+	switch (pdata->restart) {
+	case 100:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x1 << 3, 0x3 << 3);
+		break;
+	case 150:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x0 << 3, 0x3 << 3);
+		break;
+	case 200:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x2 << 3, 0x3 << 3);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR1, 0x3 << 3, 0x3 << 3);
+		break;
+	default:
+		dev_info(max8998->dev, "Restart Level not set: leave it unchanged.\n");
+	}
+
+	/* Setup Charge Full Timeout */
+	switch (pdata->timeout) {
+	case 5:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x0 << 4, 0x3 << 4);
+		break;
+	case 6:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x1 << 4, 0x3 << 4);
+		break;
+	case 7:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x2 << 4, 0x3 << 4);
+		break;
+	case -1:
+		max8998_update_reg(i2c, MAX8998_REG_CHGR2, 0x3 << 4, 0x3 << 4);
+		break;
+	default:
+		dev_info(max8998->dev, "Full Timeout not set: leave it unchanged.\n");
+	}
+
+	max8998->battery.name = "max8998_pmic";
+	max8998->battery.type = POWER_SUPPLY_TYPE_BATTERY;
+	max8998->battery.get_property = max8998_battery_get_property;
+	max8998->battery.properties = max8998_battery_props;
+	max8998->battery.num_properties = ARRAY_SIZE(max8998_battery_props);
+
+	ret = power_supply_register(max8998->dev, &max8998->battery);
+	if (ret) {
+		dev_err(max8998->dev, "failed: power supply register\n");
+		kfree(max8998);
+		return ret;
+	}
+
+	max8998->device_id = pdev->id_entry->driver_data;
+
+	return 0;
+}
+
+static int __devexit max8998_battery_remove(struct platform_device *pdev)
+{
+	struct max8998_battery_data *max8998 = platform_get_drvdata(pdev);
+
+	power_supply_unregister(&max8998->battery);
+	kfree(max8998);
+
+	return 0;
+}
+
+static const struct platform_device_id max8998_battery_id[] = {
+	{ "max8998-battery", TYPE_MAX8998 },
+	{ "lp3974-battery", TYPE_LP3974 },
+};
+
+static struct platform_driver max8998_battery_driver = {
+	.driver = {
+		.name = "max8998-battery",
+		.owner = THIS_MODULE,
+	},
+	.probe = max8998_battery_probe,
+	.remove = __devexit_p(max8998_battery_remove),
+	.id_table = max8998_battery_id,
+};
+
+static int __init max8998_battery_init(void)
+{
+	return platform_driver_register(&max8998_battery_driver);
+}
+module_init(max8998_battery_init);
+
+static void __exit max8998_battery_cleanup(void)
+{
+	platform_driver_unregister(&max8998_battery_driver);
+}
+module_exit(max8998_battery_cleanup);
+
+MODULE_DESCRIPTION("MAXIM 8998 battery control driver");
+MODULE_AUTHOR("MyungJoo Ham <myungjoo.ham@samsung.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 9946488..0d38e64 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -86,6 +86,13 @@ static const struct voltage_map_desc buck3_voltage_map_desc = {
 static const struct voltage_map_desc buck4_voltage_map_desc = {
 	.min = 800,	.step = 100,	.max = 2300,
 };
+static const int charger_current_map_desc_max8998[] = {
+	90, 380, 475, 550, 570, 600, 700, 800
+};
+static const int charger_current_map_desc_lp3974[] = {
+	100, 400, 450, 500, 550, 600, 700, 800
+};
+const int *charger_current_map_desc;
 
 static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,
@@ -115,6 +122,7 @@ static const struct voltage_map_desc *ldo_voltage_map[] = {
 	NULL,				/* ENVICHG */
 	NULL,				/* ESAFEOUT1 */
 	NULL,				/* ESAFEOUT2 */
+	NULL,				/* CHARGER */
 };
 
 static inline int max8998_get_ldo(struct regulator_dev *rdev)
@@ -173,6 +181,10 @@ static int max8998_get_enable_register(struct regulator_dev *rdev,
 		*reg = MAX8998_REG_CHGR2;
 		*shift = 7 - (ldo - MAX8998_ESAFEOUT1);
 		break;
+	case MAX8998_CHARGER:
+		*reg = MAX8998_REG_CHGR2;
+		*shift = 0;
+		break;
 	default:
 		return -EINVAL;
 	}
@@ -198,6 +210,14 @@ static int max8998_ldo_is_enabled(struct regulator_dev *rdev)
 	return val & (1 << shift);
 }
 
+static int max8998_ldo_is_enabled_negated(struct regulator_dev *rdev)
+{
+	int ret = max8998_ldo_is_enabled(rdev);
+	if (ret >= 0)
+		ret = !ret;
+	return ret;
+}
+
 static int max8998_ldo_enable(struct regulator_dev *rdev)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
@@ -503,6 +523,76 @@ buck2_exit:
 	return ret;
 }
 
+static int max8998_charger_current_set(struct regulator_dev *rdev,
+					int min_uA, int max_uA)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+	int chosen = -1, chosen_current = -1;
+	int i;
+
+	if (!charger_current_map_desc)
+		return -ENXIO;
+
+	for (i = 0; i < (mask + 1); i++) {
+		int temp = charger_current_map_desc[i];
+		if (temp >= (min_uA / 1000) && temp <= (max_uA / 1000) &&
+				temp > chosen_current) {
+			chosen = i;
+			chosen_current = temp;
+		}
+	}
+
+	if (chosen < 0)
+		return -EINVAL;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val &= ~(mask << shift);
+	val |= (chosen & mask) << shift;
+
+	ret = max8998_write_reg(i2c, reg, val);
+	if (ret)
+		return ret;
+
+	dev_info(&rdev->dev, "charger current limit = %dmA (%xh)\n",
+			chosen_current, chosen);
+
+	max8998_update_eoc(max8998->iodev);
+
+	return 0;
+}
+
+static int max8998_charger_current_get(struct regulator_dev *rdev)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int reg = MAX8998_REG_CHGR1;
+	int shift = 0;
+	int mask = 0x7;
+	int ret;
+	u8 val;
+
+	ret = max8998_read_reg(i2c, reg, &val);
+	if (ret)
+		return ret;
+
+	val >>= shift;
+	val &= mask;
+
+	if (!charger_current_map_desc)
+		return -ENXIO;
+
+	return charger_current_map_desc[val] * 1000;
+}
+
 static struct regulator_ops max8998_ldo_ops = {
 	.list_voltage		= max8998_list_voltage,
 	.is_enabled		= max8998_ldo_is_enabled,
@@ -533,6 +623,20 @@ static struct regulator_ops max8998_others_ops = {
 	.set_suspend_disable	= max8998_ldo_disable,
 };
 
+/* The enable bit is negated */
+static struct regulator_ops max8998_charger_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+	/* enable and disable are intentionally negated */
+	.enable			= max8998_ldo_disable,
+	.disable		= max8998_ldo_enable,
+	.set_current_limit	= max8998_charger_current_set,
+	.get_current_limit	= max8998_charger_current_get,
+};
+
+static struct regulator_ops max8998_neg_online_ops = {
+	.is_enabled		= max8998_ldo_is_enabled_negated,
+};
+
 static struct regulator_desc regulators[] = {
 	{
 		.name		= "LDO2",
@@ -684,7 +788,13 @@ static struct regulator_desc regulators[] = {
 		.ops		= &max8998_others_ops,
 		.type		= REGULATOR_VOLTAGE,
 		.owner		= THIS_MODULE,
-	}
+	}, {
+		.name		= "CHARGER",
+		.id		= MAX8998_CHARGER,
+		.ops		= &max8998_charger_ops,
+		.type		= REGULATOR_CURRENT,
+		.owner		= THIS_MODULE,
+	},
 };
 
 static __devinit int max8998_pmic_probe(struct platform_device *pdev)
@@ -834,13 +944,27 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 			return ret;
 	}
 
+	switch (pdev->id_entry->driver_data) {
+	case TYPE_MAX8998:
+		charger_current_map_desc = charger_current_map_desc_max8998;
+		break;
+	case TYPE_LP3974:
+		charger_current_map_desc = charger_current_map_desc_lp3974;
+		break;
+	default:
+		ret = -ENODEV;
+		goto err;
+	}
+
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct voltage_map_desc *desc;
 		int id = pdata->regulators[i].id;
 		int index = id - MAX8998_LDO2;
 
 		desc = ldo_voltage_map[id];
-		if (desc && regulators[index].ops != &max8998_others_ops) {
+		if (desc && regulators[index].ops != &max8998_others_ops &&
+		    regulators[index].ops != &max8998_charger_ops &&
+		    regulators[index].ops != &max8998_neg_online_ops) {
 			int count = (desc->max - desc->min) / desc->step + 1;
 			regulators[index].n_voltages = count;
 		}
@@ -854,7 +978,6 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 		}
 	}
 
-
 	return 0;
 err:
 	for (i = 0; i < max8998->num_regulators; i++)
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index f107f42..704249d 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -102,11 +102,12 @@ enum {
 };
 
 /* MAX8998 various variants */
-enum {
+enum max8998_type {
 	TYPE_MAX8998 = 0, /* Default */
 	TYPE_LP3974,	/* National version of MAX8998 */
 	TYPE_LP3974_REGERR, /* National version of MAX8998 with RTC REG BUG */
 	TYPE_LP3979,	/* Added AVS */
+	TYPE_UNKNOWN,
 };
 
 #define MAX8998_IRQ_DCINF_MASK		(1 << 2)
@@ -146,6 +147,7 @@ enum {
  * @irq_masks_cur: currently active value
  * @irq_masks_cache: cached hardware value
  * @type: indicate which max8998 "variant" is used
+ * @battery: the max8998 battery control device
  */
 struct max8998_dev {
 	struct device *dev;
@@ -161,6 +163,7 @@ struct max8998_dev {
 	u8 irq_masks_cache[MAX8998_NUM_IRQ_REGS];
 	int type;
 	bool wakeup;
+	struct platform_device *battery;
 };
 
 int max8998_irq_init(struct max8998_dev *max8998);
@@ -175,4 +178,11 @@ extern int max8998_bulk_write(struct i2c_client *i2c, u8 reg, int count,
 		u8 *buf);
 extern int max8998_update_reg(struct i2c_client *i2c, u8 reg, u8 val, u8 mask);
 
+#ifdef CONFIG_CHARGERCTRL_MAX8998
+extern void max8998_update_eoc(struct max8998_dev *mdev);
+extern const int *charger_current_map_desc;
+#else
+static void __maybe_unused max8998_update_eoc(struct max8998_dev *mdev) { }
+#endif
+
 #endif /*  __LINUX_MFD_MAX8998_PRIV_H */
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index 7194c35..146a7ad 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -52,6 +52,12 @@ enum {
 	MAX8998_ENVICHG,
 	MAX8998_ESAFEOUT1,
 	MAX8998_ESAFEOUT2,
+	/*
+	 * CHARGER: Controls ON/OFF, current limit of the charger.
+	 * However, note that even if CHARGER is ON, CHARGER_ONLINE
+	 * can be in "disabled" state by MAX8998 internal control.
+	**/
+	MAX8998_CHARGER,
 };
 
 /**
@@ -84,6 +90,12 @@ struct max8998_regulator_data {
  * @buck1_default_idx: Default for BUCK1 gpio pin 1, 2
  * @buck2_set3: BUCK2 gpio pin to set output voltage
  * @buck2_default_idx: Default for BUCK2 gpio pin.
+ * @eoc: End of Charge Level: 10 ~ 45% or if it's over 45, in mA.
+ *   If it's under 10, leave it unchanged.
+ * @restart: Restart Level in mV: 100, 150, 200, and -1 for disable.
+ *   Otherwise, leave it unchanged.
+ * @timeout: Full Timeout in hours: 5, 6, 7, and -1 for disable.
+ *  Otherwise, leave it unchanged.
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
@@ -103,6 +115,9 @@ struct max8998_platform_data {
 	int				buck2_set3;
 	int				buck2_default_idx;
 	bool				wakeup;
+	int				eoc;
+	int				restart;
+	int				timeout;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

Re: [PATCH v3 4/4] MFD MAX8998/LP3974: Support Charger

[Samuel Ortiz]

On Thu, Dec 23, 2010 at 05:53:39PM +0900, MyungJoo Ham wrote:
> With the new regulator, "CHARGER", users can control charging
> current and turn on and off the charger. Note that the charger
> specification of LP3974 is different from that of MAX8998.
> 
> driver/power/max8998.c supports power supply APIs for
> 
> 1. "ONLINE" monitors the charger status, which can be
> different from the status "CHARGER"; e.g., users allowed the charger
> to charge, but the MAX8998 chip decided not to do so.
> 
> 2. "PRESENT" monitors the battery status (the existence of the
> battery).
> 
> Signed-off-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Same here, I'll wait for Mark or Liam ACKs before taking this patch.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/