[PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins

linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins
@ 2010-09-22 15:30 Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added Lukasz Majewski
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Lukasz Majewski @ 2010-09-22 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

This patch facilitates the faster voltage change for BUCK1/2 regulators.

Comments and guidlines from Mark Brown have been added.

This patch has been tested at Samsung GONI and AQUILA targets.

Lukasz Majewski (3):
  mfd: regulator: max8998: set_voltage_buck routine added
  mfd: regulator: max8998: probe routine modified for GPIOs use
  mfd: regulator: max8998: mfd code

 drivers/mfd/max8998.c               |    5 +-
 drivers/regulator/max8998.c         |  195 +++++++++++++++++++++++++++++++----
 include/linux/mfd/max8998-private.h |   59 ++++++++++-
 include/linux/mfd/max8998.h         |   14 +++
 4 files changed, 244 insertions(+), 29 deletions(-)

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

* [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added
  2010-09-22 15:30 [PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins Lukasz Majewski
@ 2010-09-22 15:30 ` Lukasz Majewski
  2010-09-22 16:11   ` Mark Brown
  2010-09-22 15:30 ` [PATCH v2 2/3] mfd: regulator: max8998: probe routine modified for GPIOs use Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 3/3] mfd: regulator: max8998: mfd code Lukasz Majewski
  2 siblings, 1 reply; 6+ messages in thread
From: Lukasz Majewski @ 2010-09-22 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

Separate routine for setting BUCK regulators voltage.

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/regulator/max8998.c |  154 +++++++++++++++++++++++++++++++++++++------
 1 files changed, 133 insertions(+), 21 deletions(-)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 7f5fe6f..36d214a 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -39,6 +39,8 @@ struct max8998_data {
 	struct max8998_dev	*iodev;
 	int			num_regulators;
 	struct regulator_dev	**rdev;
+	unsigned int		buck1_idx; /* voltage index (0 to 3) */
+	unsigned int		buck2_idx; /* voltage index (0 to 1) */
 };
 
 struct voltage_map_desc {
@@ -218,6 +220,7 @@ static int max8998_get_voltage_register(struct regulator_dev *rdev,
 				int *_reg, int *_shift, int *_mask)
 {
 	int ldo = max8998_get_ldo(rdev);
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
 	int reg, shift = 0, mask = 0xff;
 
 	switch (ldo) {
@@ -254,10 +257,10 @@ static int max8998_get_voltage_register(struct regulator_dev *rdev,
 		reg = MAX8998_REG_LDO12 + (ldo - MAX8998_LDO12);
 		break;
 	case MAX8998_BUCK1:
-		reg = MAX8998_REG_BUCK1_DVSARM1;
+		reg = MAX8998_REG_BUCK1_VOL1 + max8998->buck1_idx;
 		break;
 	case MAX8998_BUCK2:
-		reg = MAX8998_REG_BUCK2_DVSINT1;
+		reg = MAX8998_REG_BUCK2_VOL1 + max8998->buck2_idx;
 		break;
 	case MAX8998_BUCK3:
 		reg = MAX8998_REG_BUCK3;
@@ -297,19 +300,16 @@ static int max8998_get_voltage(struct regulator_dev *rdev)
 	return max8998_list_voltage(rdev, val);
 }
 
-static int max8998_set_voltage(struct regulator_dev *rdev,
+static int max8998_set_voltage_ldo(struct regulator_dev *rdev,
 				int min_uV, int max_uV)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
 	struct i2c_client *i2c = max8998->iodev->i2c;
 	int min_vol = min_uV / 1000, max_vol = max_uV / 1000;
-	int previous_vol = 0;
 	const struct voltage_map_desc *desc;
 	int ldo = max8998_get_ldo(rdev);
 	int reg, shift = 0, mask, ret;
 	int i = 0;
-	u8 val;
-	bool en_ramp = false;
 
 	if (ldo >= ARRAY_SIZE(ldo_voltage_map))
 		return -EINVAL;
@@ -332,24 +332,135 @@ static int max8998_set_voltage(struct regulator_dev *rdev,
 	if (ret)
 		return ret;
 
-	/* wait for RAMP_UP_DELAY if rdev is BUCK1/2 and
-	 * ENRAMP is ON */
-	if (ldo == MAX8998_BUCK1 || ldo == MAX8998_BUCK2) {
-		max8998_read_reg(i2c, MAX8998_REG_ONOFF4, &val);
-		if (val & (1 << 4)) {
-			en_ramp = true;
-			previous_vol = max8998_get_voltage(rdev);
-		}
+	ret = max8998_update_reg(i2c, reg, i<<shift, mask<<shift);
+
+	return ret;
+}
+
+static inline void buck1_gpio_set(int gpio1, int gpio2, int v)
+{
+	gpio_set_value(gpio1, v & 0x1);
+	gpio_set_value(gpio2, (v >> 1) & 0x1);
+}
+
+static inline void buck2_gpio_set(int gpio, int v)
+{
+	gpio_set_value(gpio, v & 0x1);
+}
+
+static int max8998_set_voltage_buck(struct regulator_dev *rdev,
+				    int min_uV, int max_uV)
+{
+	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
+	struct max8998_platform_data *pdata =
+		dev_get_platdata(max8998->iodev->dev);
+	struct i2c_client *i2c = max8998->iodev->i2c;
+	int min_vol = min_uV / 1000, max_vol = max_uV / 1000;
+	const struct voltage_map_desc *desc;
+	int buck = max8998_get_ldo(rdev);
+	int reg, shift = 0, mask, ret;
+	int difference = 0, i = 0, j = 0, previous_vol = 0;
+	u8 val = 0;
+
+	if (buck >= ARRAY_SIZE(ldo_voltage_map))
+		return -EINVAL;
+
+	desc = ldo_voltage_map[buck];
+	if (desc == NULL)
+		return -EINVAL;
+
+	if (max_vol < desc->min || min_vol > desc->max)
+		return -EINVAL;
+
+	while (desc->min + desc->step*i < min_vol &&
+	       desc->min + desc->step*i < desc->max)
+		i++;
+
+	if (desc->min + desc->step*i > max_vol)
+		return -EINVAL;
+
+	ret = max8998_get_voltage_register(rdev, &reg, &shift, &mask);
+	if (ret)
+		return ret;
+
+	previous_vol = max8998_get_voltage(rdev);
+
+	/* Check if voltage needs to be changed */
+	/* if previous_voltage equal new voltage, return */
+	if (previous_vol == max8998_list_voltage(rdev, i)) {
+		dev_dbg(max8998->dev, "No voltage change, old:%d, new:%d\n",
+			previous_vol, max8998_list_voltage(rdev, i));
+		return ret;
 	}
 
-	ret = max8998_update_reg(i2c, reg, i<<shift, mask<<shift);
+	switch (buck) {
+	case MAX8998_BUCK1:
+		dev_dbg(max8998->dev,
+			"BUCK1, i:%d, buck1_vol1:%d, buck1_vol2:%d\n\
+			 buck1_vol3:%d, buck1_vol4:%d\n",
+			i, pdata->buck1_vol[0], pdata->buck1_vol[1],
+			pdata->buck1_vol[2], pdata->buck1_vol[3]);
+
+		for (j = 0; j < ARRAY_SIZE(pdata->buck1_vol); j++) {
+			if (pdata->buck1_vol[j] == i) {
+				max8998->buck1_idx = j;
+				buck1_gpio_set(pdata->buck1_set1,
+					       pdata->buck1_set2, j);
+				break;
+			}
+			if (j == (ARRAY_SIZE(pdata->buck1_vol) - 1)) {
+				/* no predefine regulator found */
+				max8998->buck1_idx = j;
+				ret = max8998_get_voltage_register(rdev, &reg,
+								   &shift,
+								   &mask);
+				ret = max8998_write_reg(i2c, reg, i);
+				buck1_gpio_set(pdata->buck1_set1,
+					       pdata->buck1_set2, j);
+			}
+		}
+		break;
+
+	case MAX8998_BUCK2:
+		dev_dbg(max8998->dev,
+			"BUCK2, i:%d buck2_vol1:%d, buck2_vol2:%d\n"
+			, i, pdata->buck2_vol[0], pdata->buck2_vol[1]);
+
+		if (pdata->buck2_vol[0] == i) {
+			max8998->buck1_idx = 0;
+			buck2_gpio_set(pdata->buck2_set3, 0);
+		} else {
+			max8998->buck1_idx = 1;
+			ret = max8998_get_voltage_register(rdev, &reg,
+							   &shift, &mask);
+			ret = max8998_write_reg(i2c, reg, i);
+			buck2_gpio_set(pdata->buck2_set3, 1);
+		}
+		break;
 
-	if (en_ramp == true) {
-		int difference = desc->min + desc->step*i - previous_vol/1000;
-		if (difference > 0)
-			udelay(difference / ((val & 0x0f) + 1));
+	case MAX8998_BUCK3:
+	case MAX8998_BUCK4:
+		ret = max8998_update_reg(i2c, reg, i<<shift, mask<<shift);
+		break;
 	}
 
+	/* Voltage stabilization */
+	max8998_read_reg(i2c, MAX8998_REG_ONOFF4, &val);
+
+	dev_dbg(max8998->dev, "%s: SET1:%d, SET2:%d, SET3:%d\n",
+		i2c->name, gpio_get_value(pdata->buck1_set1),
+		gpio_get_value(pdata->buck1_set2),
+		gpio_get_value(pdata->buck2_set3));
+
+	/* lp3974 hasn't got ENRAMP bit - ramp is assumed as true */
+	/* MAX8998 has ENRAMP bit implemented, so test it*/
+	if (max8998->iodev->type == TYPE_MAX8998 && !(val & MAX8998_ENRAMP))
+		return ret;
+
+	difference = desc->min + desc->step*i - previous_vol/1000;
+	if (difference > 0)
+		udelay(difference / ((val & 0x0f) + 1));
+
 	return ret;
 }
 
@@ -359,7 +470,7 @@ static struct regulator_ops max8998_ldo_ops = {
 	.enable			= max8998_ldo_enable,
 	.disable		= max8998_ldo_disable,
 	.get_voltage		= max8998_get_voltage,
-	.set_voltage		= max8998_set_voltage,
+	.set_voltage		= max8998_set_voltage_ldo,
 	.set_suspend_enable	= max8998_ldo_enable,
 	.set_suspend_disable	= max8998_ldo_disable,
 };
@@ -370,7 +481,7 @@ static struct regulator_ops max8998_buck_ops = {
 	.enable			= max8998_ldo_enable,
 	.disable		= max8998_ldo_disable,
 	.get_voltage		= max8998_get_voltage,
-	.set_voltage		= max8998_set_voltage,
+	.set_voltage		= max8998_set_voltage_buck,
 	.set_suspend_enable	= max8998_ldo_enable,
 	.set_suspend_disable	= max8998_ldo_disable,
 };
@@ -543,6 +654,7 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 	struct max8998_platform_data *pdata = dev_get_platdata(iodev->dev);
 	struct regulator_dev **rdev;
 	struct max8998_data *max8998;
+	struct i2c_client *i2c;
 	int i, ret, size;
 
 	if (!pdata) {
-- 
1.7.1

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

* [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added
  2010-09-22 15:30 ` [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added Lukasz Majewski
@ 2010-09-22 16:11   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2010-09-22 16:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 22, 2010 at 05:30:08PM +0200, Lukasz Majewski wrote:
> Separate routine for setting BUCK regulators voltage.

This is doing more than that...

> +	unsigned int		buck1_idx; /* voltage index (0 to 3) */
> +	unsigned int		buck2_idx; /* voltage index (0 to 1) */

...it's also changing things over to use this new GPIO based selection
scheme.  This is rather missing the point of my suggestion to split the
code reorganisation out from the new functionality - the goal is to just
have the code motion and then separately add the new functionality.

See also my comments about ensuring each patch in the series is
buildable.

> +		for (j = 0; j < ARRAY_SIZE(pdata->buck1_vol); j++) {
> +			if (pdata->buck1_vol[j] == i) {
> +				max8998->buck1_idx = j;
> +				buck1_gpio_set(pdata->buck1_set1,
> +					       pdata->buck1_set2, j);
> +				break;
> +			}
> +			if (j == (ARRAY_SIZE(pdata->buck1_vol) - 1)) {
> +				/* no predefine regulator found */
> +				max8998->buck1_idx = j;
> +				ret = max8998_get_voltage_register(rdev, &reg,
> +								   &shift,
> +								   &mask);
> +				ret = max8998_write_reg(i2c, reg, i);
> +				buck1_gpio_set(pdata->buck1_set1,
> +					       pdata->buck1_set2, j);
> +			}

So this essentially reserves one of the voltage selections for register
written values and requires the others to be specified as platform data
or from the hardware?  It'd be nicer to do this using something like
rotating through the slots or (better yet) LRU so that the driver just
figures out which voltages are being used by the consumer rather than
the platform data having to know exactly which voltages will be set by
the consumers in advance.

> +		if (pdata->buck2_vol[0] == i) {
> +			max8998->buck1_idx = 0;
> +			buck2_gpio_set(pdata->buck2_set3, 0);
> +		} else {
> +			max8998->buck1_idx = 1;
> +			ret = max8998_get_voltage_register(rdev, &reg,
> +							   &shift, &mask);
> +			ret = max8998_write_reg(i2c, reg, i);
> +			buck2_gpio_set(pdata->buck2_set3, 1);
> +		}

Ditto here.  The WM831x driver is an example of being smarter about
this.

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

* [PATCH v2 2/3] mfd: regulator: max8998: probe routine modified for GPIOs use
  2010-09-22 15:30 [PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added Lukasz Majewski
@ 2010-09-22 15:30 ` Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 3/3] mfd: regulator: max8998: mfd code Lukasz Majewski
  2 siblings, 0 replies; 6+ messages in thread
From: Lukasz Majewski @ 2010-09-22 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/regulator/max8998.c |   41 +++++++++++++++++++++++++++++++++++++++++
 1 files changed, 41 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 36d214a..873a565 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -678,6 +678,47 @@ static __devinit int max8998_pmic_probe(struct platform_device *pdev)
 	max8998->iodev = iodev;
 	max8998->num_regulators = pdata->num_regulators;
 	platform_set_drvdata(pdev, max8998);
+	i2c = max8998->iodev->i2c;
+
+	/* Check if platform data for max8998 has been declared */
+	if (pdata->buck1_vol[0] != 0 && pdata->buck1_vol[1] != 0 &&
+	    pdata->buck1_vol[2] != 0 && pdata->buck1_vol[3] != 0) {
+
+		for (i = 0; i < ARRAY_SIZE(pdata->buck1_vol); ++i) {
+			max8998_write_reg(i2c,
+					  MAX8998_REG_BUCK1_VOL1 + i,
+					  pdata->buck1_vol[i]);
+		}
+
+		/* Set default buck1 index - choose default voltage value */
+		max8998->buck1_idx = pdata->buck1_init_idx;
+	}
+
+	if (pdata->buck2_vol[0] != 0 && pdata->buck2_vol[1] != 0) {
+		max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOL1,
+				  pdata->buck2_vol[0]);
+		max8998_write_reg(i2c, MAX8998_REG_BUCK2_VOL2,
+				  pdata->buck2_vol[1]);
+
+		/* Set default buck2 index - choose default voltage value */
+		max8998->buck2_idx = pdata->buck2_init_idx;
+	}
+
+	if (gpio_is_valid(pdata->buck1_set1) &&
+	    gpio_is_valid(pdata->buck1_set2)) {
+		gpio_request(pdata->buck1_set1, "MAX8998 BUCK1_SET1");
+		gpio_direction_output(pdata->buck1_set1,
+				      max8998->buck1_idx & 0x1);
+
+		gpio_request(pdata->buck1_set2, "MAX8998 BUCK1_SET2");
+		gpio_direction_output(pdata->buck1_set2,
+				      (max8998->buck1_idx >> 1) & 0x1);
+	}
+	if (gpio_is_valid(pdata->buck2_set3)) {
+		gpio_request(pdata->buck2_set3, "MAX8998 BUCK2_SET3");
+		gpio_direction_output(pdata->buck2_set3,
+				      max8998->buck2_idx & 0x1);
+	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct voltage_map_desc *desc;
-- 
1.7.1

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

* [PATCH v2 3/3] mfd: regulator: max8998: mfd code
  2010-09-22 15:30 [PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added Lukasz Majewski
  2010-09-22 15:30 ` [PATCH v2 2/3] mfd: regulator: max8998: probe routine modified for GPIOs use Lukasz Majewski
@ 2010-09-22 15:30 ` Lukasz Majewski
  2010-09-22 15:58   ` Mark Brown
  2 siblings, 1 reply; 6+ messages in thread
From: Lukasz Majewski @ 2010-09-22 15:30 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/mfd/max8998.c               |    5 ++-
 include/linux/mfd/max8998-private.h |   59 +++++++++++++++++++++++++++++++---
 include/linux/mfd/max8998.h         |   14 ++++++++
 3 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/max8998.c b/drivers/mfd/max8998.c
index 310fd80..a720f41 100644
--- a/drivers/mfd/max8998.c
+++ b/drivers/mfd/max8998.c
@@ -133,6 +133,7 @@ static int max8998_i2c_probe(struct i2c_client *i2c,
 	max8998->dev = &i2c->dev;
 	max8998->i2c = i2c;
 	max8998->irq = i2c->irq;
+	max8998->type = id->driver_data;
 	if (pdata) {
 		max8998->ono = pdata->ono;
 		max8998->irq_base = pdata->irq_base;
@@ -169,8 +170,8 @@ static int max8998_i2c_remove(struct i2c_client *i2c)
 }
 
 static const struct i2c_device_id max8998_i2c_id[] = {
-	{ "max8998", 0 },
-	{ "lp3974", 0 },
+	{ "max8998", TYPE_MAX8998 },
+	{ "lp3974", TYPE_LP3974},
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, max8998_i2c_id);
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index 170f665..332c14e 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -48,12 +48,12 @@ enum {
 	MAX8998_REG_ONOFF2,
 	MAX8998_REG_ONOFF3,
 	MAX8998_REG_ONOFF4,
-	MAX8998_REG_BUCK1_DVSARM1,
-	MAX8998_REG_BUCK1_DVSARM2,
-	MAX8998_REG_BUCK1_DVSARM3,
-	MAX8998_REG_BUCK1_DVSARM4,
-	MAX8998_REG_BUCK2_DVSINT1,
-	MAX8998_REG_BUCK2_DVSINT2,
+	MAX8998_REG_BUCK1_VOL1,
+	MAX8998_REG_BUCK1_VOL2,
+	MAX8998_REG_BUCK1_VOL3,
+	MAX8998_REG_BUCK1_VOL4,
+	MAX8998_REG_BUCK2_VOL1,
+	MAX8998_REG_BUCK2_VOL2,
 	MAX8998_REG_BUCK3,
 	MAX8998_REG_BUCK4,
 	MAX8998_REG_LDO2_LDO3,
@@ -101,6 +101,49 @@ enum {
 	MAX8998_IRQ_NR,
 };
 
+/* MAX8998 output voltages */
+enum {
+	MAX8998_DVS_750mV = 0,
+	MAX8998_DVS_775mV,
+	MAX8998_DVS_800mV,
+	MAX8998_DVS_825mV,
+	MAX8998_DVS_850mV,
+	MAX8998_DVS_875mV,
+	MAX8998_DVS_900mV,
+	MAX8998_DVS_925mV,
+	MAX8998_DVS_950mV,
+	MAX8998_DVS_975mV,
+	MAX8998_DVS_1000mV,
+	MAX8998_DVS_1025mV,
+	MAX8998_DVS_1050mV,
+	MAX8998_DVS_1075mV,
+	MAX8998_DVS_1100mV,
+	MAX8998_DVS_1125mV,
+	MAX8998_DVS_1150mV,
+	MAX8998_DVS_1175mV,
+	MAX8998_DVS_1200mV,
+	MAX8998_DVS_1225mV,
+	MAX8998_DVS_1250mV,
+	MAX8998_DVS_1275mV,
+	MAX8998_DVS_1300mV,
+	MAX8998_DVS_1325mV,
+	MAX8998_DVS_1350mV,
+	MAX8998_DVS_1375mV,
+	MAX8998_DVS_1400mV,
+	MAX8998_DVS_1425mV,
+	MAX8998_DVS_1450mV,
+	MAX8998_DVS_1475mV,
+	MAX8998_DVS_1500mV,
+	MAX8998_DVS_1525mV,
+};
+
+/* MAX8998 various variants */
+enum {
+	TYPE_MAX8998 = 0, /* Default */
+	TYPE_LP3974,	/* National version of MAX8998 */
+	TYPE_LP3979,	/* Added AVS */
+};
+
 #define MAX8998_IRQ_DCINF_MASK		(1 << 2)
 #define MAX8998_IRQ_DCINR_MASK		(1 << 3)
 #define MAX8998_IRQ_JIGF_MASK		(1 << 4)
@@ -123,6 +166,8 @@ enum {
 #define MAX8998_IRQ_LOBAT1_MASK		(1 << 0)
 #define MAX8998_IRQ_LOBAT2_MASK		(1 << 1)
 
+#define MAX8998_ENRAMP                  (1 << 4)
+
 /**
  * struct max8998_dev - max8998 master device for sub-drivers
  * @dev: master device of the chip (can be used to access platform data)
@@ -135,6 +180,7 @@ enum {
  * @ono: power onoff IRQ number for max8998
  * @irq_masks_cur: currently active value
  * @irq_masks_cache: cached hardware value
+ * @type: indicate which max8998 "variant" is used
  */
 struct max8998_dev {
 	struct device *dev;
@@ -148,6 +194,7 @@ struct max8998_dev {
 	int ono;
 	u8 irq_masks_cur[MAX8998_NUM_IRQ_REGS];
 	u8 irq_masks_cache[MAX8998_NUM_IRQ_REGS];
+	int type;
 };
 
 int max8998_irq_init(struct max8998_dev *max8998);
diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index d47ed4c..bc9bcc3 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -70,12 +70,26 @@ 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_vol: BUCK1 voltage levels
+ * @buck2_vol: BUCK2 voltage levels
+ * @buck1_init_idx: BUCK1 initial voltage index
+ * @buck2_init_idx: BUCK2 initial voltage index
+ * @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
  */
 struct max8998_platform_data {
 	struct max8998_regulator_data	*regulators;
 	int				num_regulators;
 	int				irq_base;
 	int				ono;
+	u8                              buck1_vol[4];
+	u8				buck2_vol[2];
+	u8                              buck1_init_idx;
+	u8                              buck2_init_idx;
+	int				buck1_set1;
+	int				buck1_set2;
+	int				buck2_set3;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

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

* [PATCH v2 3/3] mfd: regulator: max8998: mfd code
  2010-09-22 15:30 ` [PATCH v2 3/3] mfd: regulator: max8998: mfd code Lukasz Majewski
@ 2010-09-22 15:58   ` Mark Brown
  0 siblings, 0 replies; 6+ messages in thread
From: Mark Brown @ 2010-09-22 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Sep 22, 2010 at 05:30:10PM +0200, Lukasz Majewski wrote:

Please provide a more descriptive changelog which says what the
functional changes are; it's clear that the changes are in MFD but what
do they do?

> +	max8998->type = id->driver_data;

This needs to be the first patch in the series, I think - the others
won't build without this.  Each patch in the series should leave the
kernel buildable, partly for review (so we see things added before they
are used) and partly so that things like git bisect work well.

> +/* MAX8998 output voltages */
> +enum {
> +	MAX8998_DVS_750mV = 0,
> +	MAX8998_DVS_775mV,

My previous comments about this being better as a value than an
enumerated type still apply.

> + * @buck1_vol: BUCK1 voltage levels
> + * @buck2_vol: BUCK2 voltage levels

So as far as I can tell (and I may be missing something since "voltage
levels" and code is all I have) these are the voltages programmed for
the various voltage selections at system startup?  I would very strongly
expect these to be read back from the chip when the driver starts when.

> + * @buck1_init_idx: BUCK1 initial voltage index
> + * @buck2_init_idx: BUCK2 initial voltage index

It'd be clearer to describe these as the initial GPIO setting.

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

end of thread, other threads:[~2010-09-22 16:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-22 15:30 [PATCH v2 0/3] mfd: regulator: max8998: BUCK1/2 control augmented by GPIO pins Lukasz Majewski
2010-09-22 15:30 ` [PATCH v2 1/3] mfd: regulator: max8998: set_voltage_buck routine added Lukasz Majewski
2010-09-22 16:11   ` Mark Brown
2010-09-22 15:30 ` [PATCH v2 2/3] mfd: regulator: max8998: probe routine modified for GPIOs use Lukasz Majewski
2010-09-22 15:30 ` [PATCH v2 3/3] mfd: regulator: max8998: mfd code Lukasz Majewski
2010-09-22 15:58   ` Mark Brown

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