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

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

[Lukasz Majewski]

This patch series provides the "fast patch" for BUCK1/2 voltage setting. When requested
voltage is required and it is available at MAX8998 internal registers, no I2C communication
is performed and voltage is changed by GPIO setting.

For BUCK1 two predefined, maximum voltages may be defined. BUCK2 allows for one such 
value definition.

Changes v3:
- GPIOs not defined at platform data ( i.e. = 0) are causing WARN_ON
- patches are presenting changes in the driver in a more clear way
- ability to add predefined (preferably maximal) voltage values
- no constants used when predefined voltage values defined
- checks if driver is operating at valid GPIOs 

Lukasz Majewski (5):
  mfd: regulator: max8998: separate set_voltage for ldo and buck
  mfd: regulator: max8998: Support for chips compliant with max8998
  mfd: regulator: max8998: BUCK1/2 internal voltages and their index
    added
  mfd: regulator: max8998: voltages and GPIOs added to platform data
    structure
  mfd: regulator: max8998: BUCK1/2 voltage change with use of GPIOs

 drivers/mfd/max8998.c               |    5 +-
 drivers/regulator/max8998.c         |  257 ++++++++++++++++++++++++++++++++---
 include/linux/mfd/max8998-private.h |   23 +++-
 include/linux/mfd/max8998.h         |   12 ++
 4 files changed, 268 insertions(+), 29 deletions(-)

[PATCH v3 1/5] mfd: regulator: max8998: separate set_voltage for ldo and buck

[Lukasz Majewski]

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

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 7f5fe6f..1e5bd50 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -297,7 +297,44 @@ 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;
+	const struct voltage_map_desc *desc;
+	int ldo = max8998_get_ldo(rdev);
+	int reg, shift = 0, mask, ret;
+	int i = 0;
+
+	if (ldo >= ARRAY_SIZE(ldo_voltage_map))
+		return -EINVAL;
+
+	desc = ldo_voltage_map[ldo];
+	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;
+
+	ret = max8998_update_reg(i2c, reg, i<<shift, mask<<shift);
+
+	return ret;
+}
+
+static int max8998_set_voltage_buck(struct regulator_dev *rdev,
 				int min_uV, int max_uV)
 {
 	struct max8998_data *max8998 = rdev_get_drvdata(rdev);
@@ -359,7 +396,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 +407,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,
 };
-- 
1.7.1

[PATCH v3 1/5] mfd: regulator: max8998: separate set_voltage for ldo and buck

[Mark Brown]

On Mon, Sep 27, 2010 at 02:32:23PM +0200, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

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

[PATCH v3 2/5] mfd: regulator: max8998: Support for ICs compliant with max8998

[Lukasz Majewski]

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 |   11 +++++++++++
 2 files changed, 14 insertions(+), 2 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..0ff4211 100644
--- a/include/linux/mfd/max8998-private.h
+++ b/include/linux/mfd/max8998-private.h
@@ -101,6 +101,13 @@ enum {
 	MAX8998_IRQ_NR,
 };
 
+/* 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 +130,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 +144,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 +158,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);
-- 
1.7.1

[PATCH v3 2/5] mfd: regulator: max8998: Support for ICs compliant with max8998

[Mark Brown]

On Mon, Sep 27, 2010 at 02:32:24PM +0200, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

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

[PATCH v3 3/5] mfd: regulator: max8998: BUCK1/2 internal voltages and indexes defined

[Lukasz Majewski]

BUCK1/2 internal voltages and indexes defined in the struct max8998_data
max_get_voltage_register now uses index values to chose proper register
More generic BUCK1/2 registers names provided

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

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 1e5bd50..7ae0639 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -39,6 +39,11 @@ struct max8998_data {
 	struct max8998_dev	*iodev;
 	int			num_regulators;
 	struct regulator_dev	**rdev;
+	u8                      buck1_vol[4]; /* voltages for selection */
+	u8                      buck2_vol[2];
+	unsigned int		buck1_idx; /* index to last changed voltage */
+					   /* value in a set */
+	unsigned int		buck2_idx;
 };
 
 struct voltage_map_desc {
@@ -218,6 +223,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 +260,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_VOLTAGE1 + max8998->buck1_idx;
 		break;
 	case MAX8998_BUCK2:
-		reg = MAX8998_REG_BUCK2_DVSINT1;
+		reg = MAX8998_REG_BUCK2_VOLTAGE1 + max8998->buck2_idx;
 		break;
 	case MAX8998_BUCK3:
 		reg = MAX8998_REG_BUCK3;
diff --git a/include/linux/mfd/max8998-private.h b/include/linux/mfd/max8998-private.h
index 0ff4211..7363dea 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_VOLTAGE1,
+	MAX8998_REG_BUCK1_VOLTAGE2,
+	MAX8998_REG_BUCK1_VOLTAGE3,
+	MAX8998_REG_BUCK1_VOLTAGE4,
+	MAX8998_REG_BUCK2_VOLTAGE1,
+	MAX8998_REG_BUCK2_VOLTAGE2,
 	MAX8998_REG_BUCK3,
 	MAX8998_REG_BUCK4,
 	MAX8998_REG_LDO2_LDO3,
-- 
1.7.1

[PATCH v3 3/5] mfd: regulator: max8998: BUCK1/2 internal voltages and indexes defined

[Mark Brown]

On Mon, Sep 27, 2010 at 02:32:25PM +0200, Lukasz Majewski wrote:
> BUCK1/2 internal voltages and indexes defined in the struct max8998_data
> max_get_voltage_register now uses index values to chose proper register
> More generic BUCK1/2 registers names provided
> 
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>

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

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Lukasz Majewski]

Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 include/linux/mfd/max8998.h |   12 ++++++++++++
 1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
index d47ed4c..f8c9f88 100644
--- a/include/linux/mfd/max8998.h
+++ b/include/linux/mfd/max8998.h
@@ -70,12 +70,24 @@ 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
+ * @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;
+	int                             buck1_max_voltage1;
+	int                             buck1_max_voltage2;
+	int                             buck2_max_voltage;
+	int				buck1_set1;
+	int				buck1_set2;
+	int				buck2_set3;
 };
 
 #endif /*  __LINUX_MFD_MAX8998_H */
-- 
1.7.1

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Samuel Ortiz]

Hi Lukasz,

On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
Fine with me:
Acked-by: Samuel Ortiz <sameo@linux.intel.com>

Cheers,
Samuel.


> ---
>  include/linux/mfd/max8998.h |   12 ++++++++++++
>  1 files changed, 12 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/mfd/max8998.h b/include/linux/mfd/max8998.h
> index d47ed4c..f8c9f88 100644
> --- a/include/linux/mfd/max8998.h
> +++ b/include/linux/mfd/max8998.h
> @@ -70,12 +70,24 @@ 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
> + * @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;
> +	int                             buck1_max_voltage1;
> +	int                             buck1_max_voltage2;
> +	int                             buck2_max_voltage;
> +	int				buck1_set1;
> +	int				buck1_set2;
> +	int				buck2_set3;
>  };
>  
>  #endif /*  __LINUX_MFD_MAX8998_H */
> -- 
> 1.7.1
> 

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

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Liam Girdwood]

On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> Hi Lukasz,
> 
> On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> Fine with me:
> Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> 

Lukasz, this is not applying against the regulator next tree.
Can you redo and add Mark and Samuels Acks.

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Lukasz Majewski]

On Sat, 02 Oct 2010 14:33:33 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> > Hi Lukasz,
> > 
> > On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > Fine with me:
> > Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> > 
> 
> Lukasz, this is not applying against the regulator next tree.
> Can you redo and add Mark and Samuels Acks.
> 
> Thanks
> 
> Liam

Hi Liam,

I've fetched the newest voltage-2.6/for-next and merge it with newest
mfd-2.6/for next. After that all patches are applying and kernel is
building without errors.

The problem with this patch series is that it "touches" two
repositories: voltage-2.6 and mfd-2.6. 

I'm a bit confused how such situation should be resolved, since it
involves two separate repositories (and two maintainers to cooperate).

-- 
Best regards,

Lukasz Majewski

Samsung Poland R0d5c1f412cf166c0cd87b0f6eb7fed70a2&D Center
Platform Group

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Liam Girdwood]

On Mon, 2010-10-04 at 08:46 +0200, Lukasz Majewski wrote:
> On Sat, 02 Oct 2010 14:33:33 +0100
> Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> 
> > On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> > > Hi Lukasz,
> > > 
> > > On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > Fine with me:
> > > Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> > > 
> > 
> > Lukasz, this is not applying against the regulator next tree.
> > Can you redo and add Mark and Samuels Acks.
> > 
> > Thanks
> > 
> > Liam
> 
> Hi Liam,
> 
> I've fetched the newest voltage-2.6/for-next and merge it with newest
> mfd-2.6/for next. After that all patches are applying and kernel is
> building without errors.
> 
> The problem with this patch series is that it "touches" two
> repositories: voltage-2.6 and mfd-2.6. 
> 
> I'm a bit confused how such situation should be resolved, since it
> involves two separate repositories (and two maintainers to cooperate).
> 

You should only base your patches on one tree for upstreaming and since
this series mostly touches regulator it's best base against the
regulator tree.

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Lukasz Majewski]

On Mon, 04 Oct 2010 11:09:49 +0100
Liam Girdwood <lrg@slimlogic.co.uk> wrote:

> On Mon, 2010-10-04 at 08:46 +0200, Lukasz Majewski wrote:
> > On Sat, 02 Oct 2010 14:33:33 +0100
> > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > 
> > > On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> > > > Hi Lukasz,
> > > > 
> > > > On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > > Fine with me:
> > > > Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> > > > 
> > > 
> > > Lukasz, this is not applying against the regulator next tree.
> > > Can you redo and add Mark and Samuels Acks.
> > > 
> > > Thanks
> > > 
> > > Liam
> > 
> > Hi Liam,
> > 
> > I've fetched the newest voltage-2.6/for-next and merge it with
> > newest mfd-2.6/for next. After that all patches are applying and
> > kernel is building without errors.
> > 
> > The problem with this patch series is that it "touches" two
> > repositories: voltage-2.6 and mfd-2.6. 
> > 
> > I'm a bit confused how such situation should be resolved, since it
> > involves two separate repositories (and two maintainers to
> > cooperate).
> > 
> 
> You should only base your patches on one tree for upstreaming and
> since this series mostly touches regulator it's best base against the
> regulator tree.
> 
> Liam

Hi,

So what is your opinion of solving this issue?

Those patches require simultaneous update of voltage-2.6 and mfd-2.6
trees. Especially changes for voltage-2.6 won't work without
modification of mfd-2.6 tree.

-- 
Best regards,

Lukasz Majewski

Samsung Poland R&D Center
Platform Group

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Marek Szyprowski]

Hello,

On Monday, October 04, 2010 12:10 PM Liam Girdwood wrote:

> On Mon, 2010-10-04 at 08:46 +0200, Lukasz Majewski wrote:
> > On Sat, 02 Oct 2010 14:33:33 +0100
> > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> >
> > > On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> > > > Hi Lukasz,
> > > >
> > > > On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > > Fine with me:
> > > > Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> > > >
> > >
> > > Lukasz, this is not applying against the regulator next tree.
> > > Can you redo and add Mark and Samuels Acks.
> > >
> > > Thanks
> > >
> > > Liam
> >
> > Hi Liam,
> >
> > I've fetched the newest voltage-2.6/for-next and merge it with newest
> > mfd-2.6/for next. After that all patches are applying and kernel is
> > building without errors.
> >
> > The problem with this patch series is that it "touches" two
> > repositories: voltage-2.6 and mfd-2.6.
> >
> > I'm a bit confused how such situation should be resolved, since it
> > involves two separate repositories (and two maintainers to cooperate).
> >
> You should only base your patches on one tree for upstreaming and since
> this series mostly touches regulator it's best base against the
> regulator tree.

Liam, the problem is the fact that there are 4 patches for max8998 mfd driver
already merged to mfd-next tree: 40d5c1f412cf166c0cd87b0f6eb7fed70a2b9e05,
128f9848ed5d3b09dc8f6f5cdc19eed48b879ccf, ec2465481c6da42766046cd2307edc46908d645b
and be9be9dcf14947fc6ad99e5df0eb3d6e09aaedca. At least the first two are required
for the patches that has been prepared by Lukasz. Removing the dependency on these
2 commits from Lukasz's patches is pointless, because such version will conflict
with these patches from mfd-next tree.

I'm a bit confused how this case should be handled. I see two solutions:
1. importing these 4 max8998 patches from mfd-next to regulator-next tree 
   (cherry-picking from mfd-next to regulator-next or git pull the whole tree)
2. splitting patches for 2 different sets (one set for mfd tree, second for
   regulator tree)

The second option has a serious drawback however. Regulator part from Lukasz's
patches will not even compile until the mfd part has been merged.

Liam, Samuel, could you both comment on this issue?

Best regards
--
Marek Szyprowski
Samsung Poland R&D Center

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Liam Girdwood]

On Wed, 2010-10-06 at 09:05 +0200, Marek Szyprowski wrote:
> Hello,
> 
> On Monday, October 04, 2010 12:10 PM Liam Girdwood wrote:
> 
> > On Mon, 2010-10-04 at 08:46 +0200, Lukasz Majewski wrote:
> > > On Sat, 02 Oct 2010 14:33:33 +0100
> > > Liam Girdwood <lrg@slimlogic.co.uk> wrote:
> > >
> > > > On Mon, 2010-09-27 at 20:43 +0200, Samuel Ortiz wrote:
> > > > > Hi Lukasz,
> > > > >
> > > > > On Mon, Sep 27, 2010 at 02:32:26PM +0200, Lukasz Majewski wrote:
> > > > > > Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
> > > > > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > > > Fine with me:
> > > > > Acked-by: Samuel Ortiz <sameo@linux.intel.com>
> > > > >
> > > >
> > > > Lukasz, this is not applying against the regulator next tree.
> > > > Can you redo and add Mark and Samuels Acks.
> > > >
> > > > Thanks
> > > >
> > > > Liam
> > >
> > > Hi Liam,
> > >
> > > I've fetched the newest voltage-2.6/for-next and merge it with newest
> > > mfd-2.6/for next. After that all patches are applying and kernel is
> > > building without errors.
> > >
> > > The problem with this patch series is that it "touches" two
> > > repositories: voltage-2.6 and mfd-2.6.
> > >
> > > I'm a bit confused how such situation should be resolved, since it
> > > involves two separate repositories (and two maintainers to cooperate).
> > >
> > You should only base your patches on one tree for upstreaming and since
> > this series mostly touches regulator it's best base against the
> > regulator tree.
> 
> Liam, the problem is the fact that there are 4 patches for max8998 mfd driver
> already merged to mfd-next tree: 40d5c1f412cf166c0cd87b0f6eb7fed70a2b9e05,
> 128f9848ed5d3b09dc8f6f5cdc19eed48b879ccf, ec2465481c6da42766046cd2307edc46908d645b
> and be9be9dcf14947fc6ad99e5df0eb3d6e09aaedca. At least the first two are required
> for the patches that has been prepared by Lukasz. Removing the dependency on these
> 2 commits from Lukasz's patches is pointless, because such version will conflict
> with these patches from mfd-next tree.
> 
> I'm a bit confused how this case should be handled. I see two solutions:
> 1. importing these 4 max8998 patches from mfd-next to regulator-next tree 
>    (cherry-picking from mfd-next to regulator-next or git pull the whole tree)
> 2. splitting patches for 2 different sets (one set for mfd tree, second for
>    regulator tree)
> 
> The second option has a serious drawback however. Regulator part from Lukasz's
> patches will not even compile until the mfd part has been merged.
> 
> Liam, Samuel, could you both comment on this issue?
> 

Ok, so we have a build dependency on mfd. Could this not have been
(re)stated by Lukasz during the conversation about the upstream path.

Samuel, could you please take this via mfd ? You have my

Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>

Thanks

Liam
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Samuel Ortiz]

Hi Liam,

On Wed, Oct 06, 2010 at 11:46:49AM +0100, Liam Girdwood wrote:
> > Liam, Samuel, could you both comment on this issue?
> > 
> 
> Ok, so we have a build dependency on mfd. Could this not have been
> (re)stated by Lukasz during the conversation about the upstream path.
> 
> Samuel, could you please take this via mfd ? You have my
Sure, I've just pushed those 5 patches.

 
> Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
I guess you meant Acked-by, right ?

Cheers,
Samuel.


> Thanks
> 
> Liam
> -- 
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk
> 

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

[PATCH v3 4/5] mfd: regulator: max8998: voltages and GPIOs defined at platform data structure

[Liam Girdwood]

On Tue, 2010-10-19 at 01:47 +0200, Samuel Ortiz wrote:
> Hi Liam,
> 
> On Wed, Oct 06, 2010 at 11:46:49AM +0100, Liam Girdwood wrote:
> > > Liam, Samuel, could you both comment on this issue?
> > > 
> > 
> > Ok, so we have a build dependency on mfd. Could this not have been
> > (re)stated by Lukasz during the conversation about the upstream path.
> > 
> > Samuel, could you please take this via mfd ? You have my
> Sure, I've just pushed those 5 patches.
> 
>  
> > Signed-off-by: Liam Girdwood <lrg@slimlogic.co.uk>
> I guess you meant Acked-by, right ?
> 

Yes. Thanks !

Liam 
-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

[PATCH v3 5/5] mfd: regulator: max8998: BUCK1/2 voltage change with use of GPIOs

[Lukasz Majewski]

max8998_pmic_probe:
- modified to check if valid pins are defined at platform
data
- maximal voltage values (predefined at platform data) are uploaded to
max8998 device

max8998_set_voltage_buck:
- BUCK1/2 voltages change between values already defined
- Checks if valid GPIO pins are passed from platform data
- If requested voltage cannot be satisfied from already defined values,
then one of free slots is used
- Predefined maximum voltages (as defined at platform data) are always
available

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

diff --git a/drivers/regulator/max8998.c b/drivers/regulator/max8998.c
index 7ae0639..5c20756 100644
--- a/drivers/regulator/max8998.c
+++ b/drivers/regulator/max8998.c
@@ -340,24 +340,37 @@ static int max8998_set_voltage_ldo(struct regulator_dev *rdev,
 	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)
+				    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;
-	int previous_vol = 0;
 	const struct voltage_map_desc *desc;
-	int ldo = max8998_get_ldo(rdev);
+	int buck = max8998_get_ldo(rdev);
 	int reg, shift = 0, mask, ret;
-	int i = 0;
-	u8 val;
-	bool en_ramp = false;
+	int difference = 0, i = 0, j = 0, previous_vol = 0;
+	u8 val = 0;
+	static u8 buck1_last_val;
 
-	if (ldo >= ARRAY_SIZE(ldo_voltage_map))
+	if (buck >= ARRAY_SIZE(ldo_voltage_map))
 		return -EINVAL;
 
-	desc = ldo_voltage_map[ldo];
+	desc = ldo_voltage_map[buck];
+
 	if (desc == NULL)
 		return -EINVAL;
 
@@ -375,24 +388,102 @@ static int max8998_set_voltage_buck(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);
-		}
+	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, max8998->buck1_vol[0], max8998->buck1_vol[1],
+			max8998->buck1_vol[2], max8998->buck1_vol[3]);
+
+		if (gpio_is_valid(pdata->buck1_set1) &&
+		    gpio_is_valid(pdata->buck1_set2)) {
+
+			/* check if requested voltage */
+			/* value is already defined */
+			for (j = 0; j < ARRAY_SIZE(max8998->buck1_vol); j++) {
+				if (max8998->buck1_vol[j] == i) {
+					max8998->buck1_idx = j;
+					buck1_gpio_set(pdata->buck1_set1,
+						       pdata->buck1_set2, j);
+					goto buck1_exit;
+				}
+			}
+
+			/* no predefine regulator found */
+			max8998->buck1_idx = (buck1_last_val % 2) + 2;
+			dev_dbg(max8998->dev, "max8998->buck1_idx:%d\n",
+				max8998->buck1_idx);
+			max8998->buck1_vol[max8998->buck1_idx] = i;
+			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, max8998->buck1_idx);
+			buck1_last_val++;
+buck1_exit:
+			dev_dbg(max8998->dev, "%s: SET1:%d, SET2:%d\n",
+				i2c->name, gpio_get_value(pdata->buck1_set1),
+				gpio_get_value(pdata->buck1_set2));
+			break;
+		} else {
+			ret = max8998_write_reg(i2c, reg, i);
+		}
+		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_BUCK2:
+		dev_dbg(max8998->dev,
+			"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->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);
+				max8998->buck2_vol[1] = i;
+				buck2_gpio_set(pdata->buck2_set3, 1);
+			}
+			dev_dbg(max8998->dev, "%s: SET3:%d\n", i2c->name,
+				gpio_get_value(pdata->buck2_set3));
+		} else {
+			ret = max8998_write_reg(i2c, reg, i);
+		}
+		break;
+
+	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);
+
+	/* 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;
 }
 
@@ -586,6 +677,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) {
@@ -609,6 +701,86 @@ 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;
+
+	/* NOTE: */
+	/* For unused GPIO NOT marked as -1 (thereof equal to 0)  WARN_ON */
+	/* will be displayed */
+
+	/* Check if MAX8998 voltage selection GPIOs are defined */
+	if (gpio_is_valid(pdata->buck1_set1) &&
+	    gpio_is_valid(pdata->buck1_set2)) {
+		/* Check if SET1 is not equal to 0 */
+		if (!pdata->buck1_set1) {
+			printk(KERN_ERR "MAX8998 SET1 GPIO defined as 0 !\n");
+			WARN_ON(!pdata->buck1_set1);
+			return -EIO;
+		}
+		/* Check if SET2 is not equal to 0 */
+		if (!pdata->buck1_set2) {
+			printk(KERN_ERR "MAX8998 SET2 GPIO defined as 0 !\n");
+			WARN_ON(!pdata->buck1_set2);
+			return -EIO;
+		}
+
+		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);
+		/* Set predefined value for BUCK1 register 1 */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       != (pdata->buck1_max_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);
+
+		/* 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))
+			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;
+		if (ret)
+			return ret;
+
+	}
+
+	if (gpio_is_valid(pdata->buck2_set3)) {
+		/* Check if SET3 is not equal to 0 */
+		if (!pdata->buck2_set3) {
+			printk(KERN_ERR "MAX8998 SET3 GPIO defined as 0 !\n");
+			WARN_ON(!pdata->buck2_set3);
+			return -EIO;
+		}
+		gpio_request(pdata->buck2_set3, "MAX8998 BUCK2_SET3");
+		gpio_direction_output(pdata->buck2_set3,
+				      max8998->buck2_idx & 0x1);
+
+		/* BUCK2 - set preset default voltage value to buck2_vol[0] */
+		i = 0;
+		while (buck12_voltage_map_desc.min +
+		       buck12_voltage_map_desc.step*i
+		       != (pdata->buck2_max_voltage / 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;
+
+	}
 
 	for (i = 0; i < pdata->num_regulators; i++) {
 		const struct voltage_map_desc *desc;
-- 
1.7.1

[PATCH v3 5/5] mfd: regulator: max8998: BUCK1/2 voltage change with use of GPIOs

[Mark Brown]

On Mon, Sep 27, 2010 at 02:32:27PM +0200, Lukasz Majewski wrote:

> - Predefined maximum voltages (as defined at platform data) are always
> available

What I was suggesting with this was that you use the upper end of the
voltage range that gets passed in with regulator_set_voltage() rather
than the platform data - that way the user doesn't need to know what
voltages the drivers are doing and the config can't drift out of sync
with the code.

However, this does look good overall so it seems best to apply this and
the previous patch now so:

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

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

[Liam Girdwood]

On Mon, 2010-09-27 at 14:32 +0200, Lukasz Majewski wrote:
> This patch series provides the "fast patch" for BUCK1/2 voltage setting. When requested
> voltage is required and it is available at MAX8998 internal registers, no I2C communication
> is performed and voltage is changed by GPIO setting.
> 
> For BUCK1 two predefined, maximum voltages may be defined. BUCK2 allows for one such 
> value definition.
> 
> Changes v3:
> - GPIOs not defined at platform data ( i.e. = 0) are causing WARN_ON
> - patches are presenting changes in the driver in a more clear way
> - ability to add predefined (preferably maximal) voltage values
> - no constants used when predefined voltage values defined
> - checks if driver is operating at valid GPIOs 
> 
> Lukasz Majewski (5):
>   mfd: regulator: max8998: separate set_voltage for ldo and buck
>   mfd: regulator: max8998: Support for chips compliant with max8998
>   mfd: regulator: max8998: BUCK1/2 internal voltages and their index
>     added
>   mfd: regulator: max8998: voltages and GPIOs added to platform data
>     structure
>   mfd: regulator: max8998: BUCK1/2 voltage change with use of GPIOs
> 
>  drivers/mfd/max8998.c               |    5 +-
>  drivers/regulator/max8998.c         |  257 ++++++++++++++++++++++++++++++++---
>  include/linux/mfd/max8998-private.h |   23 +++-
>  include/linux/mfd/max8998.h         |   12 ++
>  4 files changed, 268 insertions(+), 29 deletions(-)

Do you want to take this or shall I ?

Thanks

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

[Liam Girdwood]

On Mon, 2010-09-27 at 19:38 +0100, Liam Girdwood wrote:

Oh, question is for Samuel.

> 
> Do you want to take this or shall I ?
> 
> Thanks
> 
> Liam
> 

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

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

[Samuel Ortiz]

Hi Liam,

On Mon, Sep 27, 2010 at 07:40:29PM +0100, Liam Girdwood wrote:
> On Mon, 2010-09-27 at 19:38 +0100, Liam Girdwood wrote:
> 
> Oh, question is for Samuel.
99% of this patchset being regulator related, I think it makes sense for you
to take it if you're ok with it. I ACKed the only tiny MFD patch.

Cheers,
Samuel.


> > 
> > Do you want to take this or shall I ?
> > 
> > Thanks
> > 
> > Liam
> > 
> 
> -- 
> Freelance Developer, SlimLogic Ltd
> ASoC and Voltage Regulator Maintainer.
> http://www.slimlogic.co.uk
> 

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