[PATCH 0/3] regulator: tps65023: TPS65020 support and bug fixes

[PATCH 0/3] regulator: tps65023: TPS65020 support and bug fixes

[Marcus Folkesson]

Hi,

here is a set of patches that fixes bugs in the tps65023 driver and gives support for the similiar tps65020 regulator.
The patches is applied on kernel version 3.0.
The tps65020 support is tested on a PXA270-based custom board but with an older kernel version
without the regmap implementation.

Bug fixes:
* The original driver does not allows voltage adjustments by i2c
* The original driver map wrong regulator as core-regulator in the tps65021 support

Changes:
* A new struct containing voltage-tables, core-regulator and size of voltage tables is passed by as driver data.

New support:
* Added support for the tps65020 regulator.

Feedback is very much appreciated

[PATCH 1/3] regulator: tps65023: Fixes i2c configuration issues

[Marcus Folkesson]

The original driver is missing the following:
* Allow i2c core voltage adjustments by clearing CORE ADJ Allowed bit in CTRL2
* Setting the GO bit in CTRL2 for updating the core voltage

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/regulator/tps65023-regulator.c |   29 +++++++++++++++++++++++++----
 1 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index 701a590..d43533b 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -63,6 +63,13 @@
 #define TPS65023_REG_CTRL_LDO2_EN	BIT(2)
 #define TPS65023_REG_CTRL_LDO1_EN	BIT(1)
 
+/* REG_CTRL2 bitfields */
+#define TPS65023_REG_CTRL2_GO		BIT(7)
+#define TPS65023_REG_CTRL2_CORE_ADJ	BIT(6)
+#define TPS65023_REG_CTRL2_DCDC2	BIT(2)
+#define TPS65023_REG_CTRL2_DCDC1	BIT(2)
+#define TPS65023_REG_CTRL2_DCDC3	BIT(0)
+
 /* LDO_CTRL bitfields */
 #define TPS65023_LDO_CTRL_LDOx_SHIFT(ldo_id)	((ldo_id)*4)
 #define TPS65023_LDO_CTRL_LDOx_MASK(ldo_id)	(0xF0 >> ((ldo_id)*4))
@@ -270,6 +277,7 @@ static int tps65023_dcdc_set_voltage(struct regulator_dev *dev,
 	struct tps_pmic *tps = rdev_get_drvdata(dev);
 	int dcdc = rdev_get_id(dev);
 	int vsel;
+	int ret;
 
 	if (dcdc != TPS65023_DCDC_1)
 		return -EINVAL;
@@ -293,10 +301,19 @@ static int tps65023_dcdc_set_voltage(struct regulator_dev *dev,
 	*selector = vsel;
 
 	/* write to the register in case we found a match */
-	if (vsel == tps->info[dcdc]->table_len)
-		return -EINVAL;
-	else
-		return tps_65023_reg_write(tps, TPS65023_REG_DEF_CORE, vsel);
+	if (vsel == tps->info[dcdc]->table_len) {
+		ret = -EINVAL;
+	} else {
+		ret = tps_65023_reg_write(tps, TPS65023_REG_DEF_CORE, vsel);
+
+		/* Tell the chip that we have changed the value in DEFCORE
+		 * and its time to update the core voltage
+		 */
+		tps_65023_set_bits(tps, TPS65023_REG_CON_CTRL2,
+							TPS65023_REG_CTRL2_GO);
+	}
+
+	return ret;
 }
 
 static int tps65023_ldo_get_voltage(struct regulator_dev *dev)
@@ -475,6 +492,10 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
 
 	i2c_set_clientdata(client, tps);
 
+	/* Enable setting output voltage by I2C */
+	tps_65023_clear_bits(tps, TPS65023_REG_CON_CTRL2,
+						TPS65023_REG_CTRL2_CORE_ADJ);
+
 	return 0;
 
  fail:
-- 
1.7.4.1

Re: [PATCH 1/3] regulator: tps65023: Fixes i2c configuration issues

[Mark Brown]

On Wed, Jul 27, 2011 at 12:32:45PM +0200, Marcus Folkesson wrote:
> The original driver is missing the following:
> * Allow i2c core voltage adjustments by clearing CORE ADJ Allowed bit in CTRL2
> * Setting the GO bit in CTRL2 for updating the core voltage

These appear to be two separate changes and should rreally have been
split but

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

[PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Marcus Folkesson]

TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
Core-regulator, voltage-tables and size of voltage-table may differ between
different regulators. All those three is now passed as driver data.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/regulator/tps65023-regulator.c |  113 ++++++++++++++++++++++++++------
 1 files changed, 93 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index d43533b..5cc7344 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -92,7 +92,7 @@
 #define TPS65023_MAX_REG_ID		TPS65023_LDO_2
 
 /* Supported voltage values for regulators */
-static const u16 VDCDC1_VSEL_table[] = {
+static const u16 VCORE_VSEL_table[] = {
 	800, 825, 850, 875,
 	900, 925, 950, 975,
 	1000, 1025, 1050, 1075,
@@ -103,19 +103,35 @@ static const u16 VDCDC1_VSEL_table[] = {
 	1500, 1525, 1550, 1600,
 };
 
-static const u16 LDO1_VSEL_table[] = {
+
+
+/* Supported voltage values for LDO regulators
+ * for tps65021 and tps65023 */
+static const u16 TPS65023_LDO1_VSEL_table[] = {
 	1000, 1100, 1300, 1800,
 	2200, 2600, 2800, 3150,
 };
 
-static const u16 LDO2_VSEL_table[] = {
+static const u16 TPS65023_LDO2_VSEL_table[] = {
 	1050, 1200, 1300, 1800,
 	2500, 2800, 3000, 3300,
 };
 
-static unsigned int num_voltages[] = {ARRAY_SIZE(VDCDC1_VSEL_table),
-				0, 0, ARRAY_SIZE(LDO1_VSEL_table),
-				ARRAY_SIZE(LDO2_VSEL_table)};
+static unsigned int tps65021_num_voltages[] = {
+				0,
+				0,
+				ARRAY_SIZE(VCORE_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+};
+
+static unsigned int tps65023_num_voltages[] = {
+				ARRAY_SIZE(VCORE_VSEL_table),
+				0,
+				0,
+				ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+};
 
 /* Regulator specific details */
 struct tps_info {
@@ -134,6 +150,14 @@ struct tps_pmic {
 	struct regulator_dev *rdev[TPS65023_NUM_REGULATOR];
 	const struct tps_info *info[TPS65023_NUM_REGULATOR];
 	struct regmap *regmap;
+	u8 core_regulator;
+};
+
+/* Struct passed as driver data */
+struct tps_driver_data {
+	struct tps_info *info;
+	u8 core_regulator;
+	unsigned int *num_voltages;
 };
 
 static int tps_65023_set_bits(struct tps_pmic *tps, u8 reg, u8 mask)
@@ -260,7 +284,7 @@ static int tps65023_dcdc_get_voltage(struct regulator_dev *dev)
 	if (dcdc < TPS65023_DCDC_1 || dcdc > TPS65023_DCDC_3)
 		return -EINVAL;
 
-	if (dcdc == TPS65023_DCDC_1) {
+	if (dcdc == tps->core_regulator) {
 		data = tps_65023_reg_read(tps, TPS65023_REG_DEF_CORE);
 		if (data < 0)
 			return data;
@@ -279,9 +303,8 @@ static int tps65023_dcdc_set_voltage(struct regulator_dev *dev,
 	int vsel;
 	int ret;
 
-	if (dcdc != TPS65023_DCDC_1)
+	if (dcdc != tps->core_regulator)
 		return -EINVAL;
-
 	if (min_uV < tps->info[dcdc]->min_uV
 			|| min_uV > tps->info[dcdc]->max_uV)
 		return -EINVAL;
@@ -379,7 +402,7 @@ static int tps65023_dcdc_list_voltage(struct regulator_dev *dev,
 	if (dcdc < TPS65023_DCDC_1 || dcdc > TPS65023_DCDC_3)
 		return -EINVAL;
 
-	if (dcdc == TPS65023_DCDC_1) {
+	if (dcdc == tps->core_regulator) {
 		if (selector >= tps->info[dcdc]->table_len)
 			return -EINVAL;
 		else
@@ -431,7 +454,8 @@ static struct regmap_config tps65023_regmap_config = {
 static int __devinit tps_65023_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
-	const struct tps_info *info = (void *)id->driver_data;
+	const struct tps_driver_data *drv_data = (void *)id->driver_data;
+	const struct tps_info *info = drv_data->info;
 	struct regulator_init_data *init_data;
 	struct regulator_dev *rdev;
 	struct tps_pmic *tps;
@@ -463,6 +487,7 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
 
 	/* common for all regulators */
 	tps->client = client;
+	tps->core_regulator = drv_data->core_regulator;
 
 	for (i = 0; i < TPS65023_NUM_REGULATOR; i++, info++, init_data++) {
 		/* Store regulator specific information */
@@ -470,7 +495,7 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
 
 		tps->desc[i].name = info->name;
 		tps->desc[i].id = i;
-		tps->desc[i].n_voltages = num_voltages[i];
+		tps->desc[i].n_voltages = drv_data->num_voltages[i];
 		tps->desc[i].ops = (i > TPS65023_DCDC_3 ?
 					&tps65023_ldo_ops : &tps65023_dcdc_ops);
 		tps->desc[i].type = REGULATOR_VOLTAGE;
@@ -528,13 +553,49 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
 	return 0;
 }
 
+static const struct tps_info tps65021_regs[] = {
+	{
+		.name = "VDCDC1",
+		.min_uV =  3300000,
+		.max_uV = 3300000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC2",
+		.min_uV =  1800000,
+		.max_uV = 1800000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC3",
+		.min_uV =  800000,
+		.max_uV = 1600000,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
+	},
+	{
+		.name = "LDO1",
+		.min_uV = 1000000,
+		.max_uV = 3150000,
+		.table_len = ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+		.table = TPS65023_LDO1_VSEL_table,
+	},
+	{
+		.name = "LDO2",
+		.min_uV = 1050000,
+		.max_uV = 3300000,
+		.table_len = ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+		.table = TPS65023_LDO2_VSEL_table,
+	},
+};
+
 static const struct tps_info tps65023_regs[] = {
 	{
 		.name = "VDCDC1",
 		.min_uV =  800000,
 		.max_uV = 1600000,
-		.table_len = ARRAY_SIZE(VDCDC1_VSEL_table),
-		.table = VDCDC1_VSEL_table,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
 	},
 	{
 		.name = "VDCDC2",
@@ -552,23 +613,35 @@ static const struct tps_info tps65023_regs[] = {
 		.name = "LDO1",
 		.min_uV = 1000000,
 		.max_uV = 3150000,
-		.table_len = ARRAY_SIZE(LDO1_VSEL_table),
-		.table = LDO1_VSEL_table,
+		.table_len = ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+		.table = TPS65023_LDO1_VSEL_table,
 	},
 	{
 		.name = "LDO2",
 		.min_uV = 1050000,
 		.max_uV = 3300000,
-		.table_len = ARRAY_SIZE(LDO2_VSEL_table),
-		.table = LDO2_VSEL_table,
+		.table_len = ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+		.table = TPS65023_LDO2_VSEL_table,
 	},
 };
 
+static struct tps_driver_data tps65021_drv_data = {
+		.info = (struct tps_info *) tps65021_regs,
+		.core_regulator = TPS65023_DCDC_3,
+		.num_voltages = tps65021_num_voltages,
+};
+
+static struct tps_driver_data tps65023_drv_data = {
+		.info = (struct tps_info *) tps65023_regs,
+		.core_regulator = TPS65023_DCDC_1,
+		.num_voltages = tps65023_num_voltages,
+};
+
 static const struct i2c_device_id tps_65023_id[] = {
 	{.name = "tps65023",
-	.driver_data = (unsigned long) tps65023_regs,},
+	.driver_data = (unsigned long) &tps65023_drv_data},
 	{.name = "tps65021",
-	.driver_data = (unsigned long) tps65023_regs,},
+	.driver_data = (unsigned long) &tps65021_drv_data,},
 	{ },
 };
 
-- 
1.7.4.1

Re: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Mark Brown]

On Wed, Jul 27, 2011 at 12:32:46PM +0200, Marcus Folkesson wrote:
> TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.

What is "core regulator" to the regulator chip itself?

> Core-regulator, voltage-tables and size of voltage-table may differ between
> different regulators. All those three is now passed as driver data.

No, the driver should be able to figure out for itself what regulators
are on the chip purely by knowing what chip it's talking to.  If there
is some configuration on the board from selection of passives or similar
the platform data should reflect how that's done (eg, supply the
relevant values for the passives).

Re: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Marcus Folkesson]

2011/7/27 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Jul 27, 2011 at 12:32:46PM +0200, Marcus Folkesson wrote:
>> TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
>
> What is "core regulator" to the regulator chip itself?

The tps6502x has 3 step down converters, VDCDC[1-3], which two has
fixed voltages and the third is adjustable with the i2c interface.
The adjustable converter is for controlling the core-voltage to a
processor and is referred as "core" in the datasheet. Therefore a
"core regulator".

A better name for "core regulator" should be "core converter" or
something. I'm apologize for the confusing name.


>> Core-regulator, voltage-tables and size of voltage-table may differ between
>> different regulators. All those three is now passed as driver data.
>
> No, the driver should be able to figure out for itself what regulators
> are on the chip purely by knowing what chip it's talking to.  If there
> is some configuration on the board from selection of passives or similar
> the platform data should reflect how that's done (eg, supply the
> relevant values for the passives).
>

The information is passed by the driver_data field in the device
table, so the driver itself knows what chip it is talking to.
In the original driver, only the voltage table was passed this way, I
just added the "core_regulator" and the size of voltage-table in the
same way.

Re: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Mark Brown]

On Wed, Jul 27, 2011 at 12:32:46PM +0200, Marcus Folkesson wrote:
> TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
> Core-regulator, voltage-tables and size of voltage-table may differ between
> different regulators. All those three is now passed as driver data.

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

Re: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Marcus Folkesson]

On 07/29/2011 02:33 PM, Mark Brown wrote:
> On Wed, Jul 27, 2011 at 12:32:46PM +0200, Marcus Folkesson wrote:
>> TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
>> Core-regulator, voltage-tables and size of voltage-table may differ between
>> different regulators. All those three is now passed as driver data.
> 
> Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>

Changing struct member tps_driver_data.info to const for avoiding awkward type castings.

Changes from original patch:

@@ -173,7 +173,7 @@ struct tps_pmic {
 
 /* Struct passed as driver data */
 struct tps_driver_data {
-	struct tps_info *info;
+	const struct tps_info *info;
 	u8 core_regulator;
 	unsigned int *num_voltages;
 };
@@ -681,19 +681,19 @@ static const struct tps_info tps65023_regs[] = {
 };
 
 static struct tps_driver_data tps65021_drv_data = {
-		.info = (struct tps_info *) tps65021_regs,
+		.info = tps65021_regs,
 		.core_regulator = TPS65023_DCDC_3,
 		.num_voltages = tps65021_num_voltages,
 };
 
 static struct tps_driver_data tps65023_drv_data = {
-		.info = (struct tps_info *) tps65023_regs,
+		.info = tps65023_regs,
 		.core_regulator = TPS65023_DCDC_1,
 		.num_voltages = tps65023_num_voltages,
 };



>From 3d9f69adad628228f4941e663f802d35c9fe85ed Mon Sep 17 00:00:00 2001
From: Marcus Folkesson <marcus.folkesson@gmail.com>
Date: Wed, 27 Jul 2011 11:29:55 +0200
Subject: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
Core-regulator, voltage-tables and size of voltage-table may differ between
different regulators. All those three is now passed as driver data.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/regulator/tps65023-regulator.c |  113 ++++++++++++++++++++++++++------
 1 files changed, 93 insertions(+), 20 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index d43533b..5cc7344 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -92,7 +92,7 @@
 #define TPS65023_MAX_REG_ID		TPS65023_LDO_2
 
 /* Supported voltage values for regulators */
-static const u16 VDCDC1_VSEL_table[] = {
+static const u16 VCORE_VSEL_table[] = {
 	800, 825, 850, 875,
 	900, 925, 950, 975,
 	1000, 1025, 1050, 1075,
@@ -103,19 +103,35 @@ static const u16 VDCDC1_VSEL_table[] = {
 	1500, 1525, 1550, 1600,
 };
 
-static const u16 LDO1_VSEL_table[] = {
+
+
+/* Supported voltage values for LDO regulators
+ * for tps65021 and tps65023 */
+static const u16 TPS65023_LDO1_VSEL_table[] = {
 	1000, 1100, 1300, 1800,
 	2200, 2600, 2800, 3150,
 };
 
-static const u16 LDO2_VSEL_table[] = {
+static const u16 TPS65023_LDO2_VSEL_table[] = {
 	1050, 1200, 1300, 1800,
 	2500, 2800, 3000, 3300,
 };
 
-static unsigned int num_voltages[] = {ARRAY_SIZE(VDCDC1_VSEL_table),
-				0, 0, ARRAY_SIZE(LDO1_VSEL_table),
-				ARRAY_SIZE(LDO2_VSEL_table)};
+static unsigned int tps65021_num_voltages[] = {
+				0,
+				0,
+				ARRAY_SIZE(VCORE_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+};
+
+static unsigned int tps65023_num_voltages[] = {
+				ARRAY_SIZE(VCORE_VSEL_table),
+				0,
+				0,
+				ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+};
 
 /* Regulator specific details */
 struct tps_info {
@@ -134,6 +150,14 @@ struct tps_pmic {
 	struct regulator_dev *rdev[TPS65023_NUM_REGULATOR];
 	const struct tps_info *info[TPS65023_NUM_REGULATOR];
 	struct regmap *regmap;
+	u8 core_regulator;
+};
+
+/* Struct passed as driver data */
+struct tps_driver_data {
+	const struct tps_info *info;
+	u8 core_regulator;
+	unsigned int *num_voltages;
 };
 
 static int tps_65023_set_bits(struct tps_pmic *tps, u8 reg, u8 mask)
@@ -260,7 +284,7 @@ static int tps65023_dcdc_get_voltage(struct regulator_dev *dev)
 	if (dcdc < TPS65023_DCDC_1 || dcdc > TPS65023_DCDC_3)
 		return -EINVAL;
 
-	if (dcdc == TPS65023_DCDC_1) {
+	if (dcdc == tps->core_regulator) {
 		data = tps_65023_reg_read(tps, TPS65023_REG_DEF_CORE);
 		if (data < 0)
 			return data;
@@ -279,9 +303,8 @@ static int tps65023_dcdc_set_voltage(struct regulator_dev *dev,
 	int vsel;
 	int ret;
 
-	if (dcdc != TPS65023_DCDC_1)
+	if (dcdc != tps->core_regulator)
 		return -EINVAL;
-
 	if (min_uV < tps->info[dcdc]->min_uV
 			|| min_uV > tps->info[dcdc]->max_uV)
 		return -EINVAL;
@@ -379,7 +402,7 @@ static int tps65023_dcdc_list_voltage(struct regulator_dev *dev,
 	if (dcdc < TPS65023_DCDC_1 || dcdc > TPS65023_DCDC_3)
 		return -EINVAL;
 
-	if (dcdc == TPS65023_DCDC_1) {
+	if (dcdc == tps->core_regulator) {
 		if (selector >= tps->info[dcdc]->table_len)
 			return -EINVAL;
 		else
@@ -431,7 +454,8 @@ static struct regmap_config tps65023_regmap_config = {
 static int __devinit tps_65023_probe(struct i2c_client *client,
 				     const struct i2c_device_id *id)
 {
-	const struct tps_info *info = (void *)id->driver_data;
+	const struct tps_driver_data *drv_data = (void *)id->driver_data;
+	const struct tps_info *info = drv_data->info;
 	struct regulator_init_data *init_data;
 	struct regulator_dev *rdev;
 	struct tps_pmic *tps;
@@ -463,6 +487,7 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
 
 	/* common for all regulators */
 	tps->client = client;
+	tps->core_regulator = drv_data->core_regulator;
 
 	for (i = 0; i < TPS65023_NUM_REGULATOR; i++, info++, init_data++) {
 		/* Store regulator specific information */
@@ -470,7 +495,7 @@ static int __devinit tps_65023_probe(struct i2c_client *client,
 
 		tps->desc[i].name = info->name;
 		tps->desc[i].id = i;
-		tps->desc[i].n_voltages = num_voltages[i];
+		tps->desc[i].n_voltages = drv_data->num_voltages[i];
 		tps->desc[i].ops = (i > TPS65023_DCDC_3 ?
 					&tps65023_ldo_ops : &tps65023_dcdc_ops);
 		tps->desc[i].type = REGULATOR_VOLTAGE;
@@ -528,13 +553,49 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
 	return 0;
 }
 
+static const struct tps_info tps65021_regs[] = {
+	{
+		.name = "VDCDC1",
+		.min_uV =  3300000,
+		.max_uV = 3300000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC2",
+		.min_uV =  1800000,
+		.max_uV = 1800000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC3",
+		.min_uV =  800000,
+		.max_uV = 1600000,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
+	},
+	{
+		.name = "LDO1",
+		.min_uV = 1000000,
+		.max_uV = 3150000,
+		.table_len = ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+		.table = TPS65023_LDO1_VSEL_table,
+	},
+	{
+		.name = "LDO2",
+		.min_uV = 1050000,
+		.max_uV = 3300000,
+		.table_len = ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+		.table = TPS65023_LDO2_VSEL_table,
+	},
+};
+
 static const struct tps_info tps65023_regs[] = {
 	{
 		.name = "VDCDC1",
 		.min_uV =  800000,
 		.max_uV = 1600000,
-		.table_len = ARRAY_SIZE(VDCDC1_VSEL_table),
-		.table = VDCDC1_VSEL_table,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
 	},
 	{
 		.name = "VDCDC2",
@@ -552,23 +613,35 @@ static const struct tps_info tps65023_regs[] = {
 		.name = "LDO1",
 		.min_uV = 1000000,
 		.max_uV = 3150000,
-		.table_len = ARRAY_SIZE(LDO1_VSEL_table),
-		.table = LDO1_VSEL_table,
+		.table_len = ARRAY_SIZE(TPS65023_LDO1_VSEL_table),
+		.table = TPS65023_LDO1_VSEL_table,
 	},
 	{
 		.name = "LDO2",
 		.min_uV = 1050000,
 		.max_uV = 3300000,
-		.table_len = ARRAY_SIZE(LDO2_VSEL_table),
-		.table = LDO2_VSEL_table,
+		.table_len = ARRAY_SIZE(TPS65023_LDO2_VSEL_table),
+		.table = TPS65023_LDO2_VSEL_table,
 	},
 };
 
+static struct tps_driver_data tps65021_drv_data = {
+		.info = tps65021_regs,
+		.core_regulator = TPS65023_DCDC_3,
+		.num_voltages = tps65021_num_voltages,
+};
+
+static struct tps_driver_data tps65023_drv_data = {
+		.info = tps65023_regs,
+		.core_regulator = TPS65023_DCDC_1,
+		.num_voltages = tps65023_num_voltages,
+};
+
 static const struct i2c_device_id tps_65023_id[] = {
 	{.name = "tps65023",
-	.driver_data = (unsigned long) tps65023_regs,},
+	.driver_data = (unsigned long) &tps65023_drv_data},
 	{.name = "tps65021",
-	.driver_data = (unsigned long) tps65023_regs,},
+	.driver_data = (unsigned long) &tps65021_drv_data,},
 	{ },
 };
 
-- 
1.7.4.1

Re: [PATCH 2/3] regulator: tps65023: Setting correct core regulator for tps65021

[Mark Brown]

On Fri, Jul 29, 2011 at 09:59:32PM +0200, Marcus Folkesson wrote:
> On 07/29/2011 02:33 PM, Mark Brown wrote:
> > On Wed, Jul 27, 2011 at 12:32:46PM +0200, Marcus Folkesson wrote:
> >> TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.
> >> Core-regulator, voltage-tables and size of voltage-table may differ between
> >> different regulators. All those three is now passed as driver data.
> > 
> > Acked-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> 
> Changing struct member tps_driver_data.info to const for avoiding awkward type castings.
> 
> Changes from original patch:

This is probably fine but please resubmit the patch normally so it can
be reviewed as a whole.

[PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

[Marcus Folkesson]

Defines a new voltage-table and allows registering of the tps65020 device.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/regulator/tps65023-regulator.c |   63 ++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index 5cc7344..a4c0583 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -103,8 +103,26 @@ static const u16 VCORE_VSEL_table[] = {
 	1500, 1525, 1550, 1600,
 };
 
+/* Supported voltage values for LDO regulators for tps65020 */
+static const u16 TPS65020_LDO1_VSEL_table[] = {
+	1000, 1050, 1100, 1300,
+	1800, 2500, 3000, 3300,
+};
+
+static const u16 TPS65020_LDO2_VSEL_table[] = {
+	1000, 1050, 1100, 1300,
+	1800, 2500, 3000, 3300,
+};
 
 
+static unsigned int tps65020_num_voltages[] = {
+				0,
+				0,
+				ARRAY_SIZE(VCORE_VSEL_table),
+				ARRAY_SIZE(TPS65020_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65020_LDO2_VSEL_table)
+};
+
 /* Supported voltage values for LDO regulators
  * for tps65021 and tps65023 */
 static const u16 TPS65023_LDO1_VSEL_table[] = {
@@ -553,6 +571,43 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
 	return 0;
 }
 
+static const struct tps_info tps65020_regs[] = {
+	{
+		.name = "VDCDC1",
+		.min_uV = 3300000,
+		.max_uV = 3300000,
+		.fixed	= 1,
+	},
+	{
+		.name = "VDCDC2",
+		.min_uV =  1800000,
+		.max_uV = 1800000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC3",
+		.min_uV =  800000,
+		.max_uV = 1600000,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
+	},
+
+	{
+		.name = "LDO1",
+		.min_uV = 1000000,
+		.max_uV = 3150000,
+		.table_len = ARRAY_SIZE(TPS65020_LDO1_VSEL_table),
+		.table = TPS65020_LDO1_VSEL_table,
+	},
+	{
+		.name = "LDO2",
+		.min_uV = 1050000,
+		.max_uV = 3300000,
+		.table_len = ARRAY_SIZE(TPS65020_LDO2_VSEL_table),
+		.table = TPS65020_LDO2_VSEL_table,
+	},
+};
+
 static const struct tps_info tps65021_regs[] = {
 	{
 		.name = "VDCDC1",
@@ -625,6 +680,12 @@ static const struct tps_info tps65023_regs[] = {
 	},
 };
 
+static struct tps_driver_data tps65020_drv_data = {
+		.info = (struct tps_info *) tps65020_regs,
+		.core_regulator = TPS65023_DCDC_3,
+		.num_voltages = tps65020_num_voltages,
+};
+
 static struct tps_driver_data tps65021_drv_data = {
 		.info = (struct tps_info *) tps65021_regs,
 		.core_regulator = TPS65023_DCDC_3,
@@ -642,6 +703,8 @@ static const struct i2c_device_id tps_65023_id[] = {
 	.driver_data = (unsigned long) &tps65023_drv_data},
 	{.name = "tps65021",
 	.driver_data = (unsigned long) &tps65021_drv_data,},
+	{.name = "tps65020",
+	.driver_data = (unsigned long) &tps65020_drv_data},
 	{ },
 };
 
-- 
1.7.4.1

Re: [PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

[Mark Brown]

On Wed, Jul 27, 2011 at 12:32:47PM +0200, Marcus Folkesson wrote:

> +static struct tps_driver_data tps65020_drv_data = {
> +		.info = (struct tps_info *) tps65020_regs,

Why do you need the cast here?  I see it's in the original driver too
but it looks like something that should be fixed.

Re: [PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

[Marcus Folkesson]

2011/7/29 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> On Wed, Jul 27, 2011 at 12:32:47PM +0200, Marcus Folkesson wrote:
>
>> +static struct tps_driver_data tps65020_drv_data = {
>> +             .info = (struct tps_info *) tps65020_regs,
>
> Why do you need the cast here?  I see it's in the original driver too
> but it looks like something that should be fixed.
>

I also thought about the line, It's only for avoiding warnings from gcc.
But I don't really understand why gcc is warning.

gcc output:  "warning: initialization discards qualifiers from pointer
target type".

Re: [PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

[Mark Brown]

On Fri, Jul 29, 2011 at 03:08:54PM +0200, Marcus Folkesson wrote:
> 2011/7/29 Mark Brown <broonie@opensource.wolfsonmicro.com>:
> > On Wed, Jul 27, 2011 at 12:32:47PM +0200, Marcus Folkesson wrote:

> >> +static struct tps_driver_data tps65020_drv_data = {
> >> +             .info = (struct tps_info *) tps65020_regs,

> > Why do you need the cast here?  I see it's in the original driver too
> > but it looks like something that should be fixed.

> I also thought about the line, It's only for avoiding warnings from gcc.
> But I don't really understand why gcc is warning.

> gcc output:  "warning: initialization discards qualifiers from pointer
> target type".

It's complaining because the declaration of tps65020_regs doesn't match
up with that of .info.  Probably the struct member needs to be a const
pointer.

Re: [PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

[Marcus Folkesson]

On 07/29/2011 04:03 PM, Mark Brown wrote:
> On Fri, Jul 29, 2011 at 03:08:54PM +0200, Marcus Folkesson wrote:
>> 2011/7/29 Mark Brown <broonie@opensource.wolfsonmicro.com>:
>>> On Wed, Jul 27, 2011 at 12:32:47PM +0200, Marcus Folkesson wrote:
> 
>>>> +static struct tps_driver_data tps65020_drv_data = {
>>>> +             .info = (struct tps_info *) tps65020_regs,
> 
>>> Why do you need the cast here?  I see it's in the original driver too
>>> but it looks like something that should be fixed.
> 
>> I also thought about the line, It's only for avoiding warnings from gcc.
>> But I don't really understand why gcc is warning.
> 
>> gcc output:  "warning: initialization discards qualifiers from pointer
>> target type".
> 
> It's complaining because the declaration of tps65020_regs doesn't match
> up with that of .info.  Probably the struct member needs to be a const
> pointer.


Changes from original patch:

@@ -681,19 +681,19 @@ static const struct tps_info tps65023_regs[] = {
 };
 
 static struct tps_driver_data tps65020_drv_data = {
-		.info = (struct tps_info *) tps65020_regs,
+		.info = tps65020_regs,
 		.core_regulator = TPS65023_DCDC_3,
 		.num_voltages = tps65020_num_voltages,
 };
 





>From 9ad596ad019feade1e346bb4128819202b26c118 Mon Sep 17 00:00:00 2001
From: Marcus Folkesson <marcus.folkesson@gmail.com>
Date: Wed, 27 Jul 2011 11:34:43 +0200
Subject: [PATCH 3/3] regulator: tps65023: Added support for the similiar TPS65020 chip

Defines a new voltage-table and allows registering of the tps65020 device.

Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
 drivers/regulator/tps65023-regulator.c |   63 ++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)

diff --git a/drivers/regulator/tps65023-regulator.c b/drivers/regulator/tps65023-regulator.c
index 5cc7344..a4c0583 100644
--- a/drivers/regulator/tps65023-regulator.c
+++ b/drivers/regulator/tps65023-regulator.c
@@ -103,8 +103,26 @@ static const u16 VCORE_VSEL_table[] = {
 	1500, 1525, 1550, 1600,
 };
 
+/* Supported voltage values for LDO regulators for tps65020 */
+static const u16 TPS65020_LDO1_VSEL_table[] = {
+	1000, 1050, 1100, 1300,
+	1800, 2500, 3000, 3300,
+};
+
+static const u16 TPS65020_LDO2_VSEL_table[] = {
+	1000, 1050, 1100, 1300,
+	1800, 2500, 3000, 3300,
+};
 
 
+static unsigned int tps65020_num_voltages[] = {
+				0,
+				0,
+				ARRAY_SIZE(VCORE_VSEL_table),
+				ARRAY_SIZE(TPS65020_LDO1_VSEL_table),
+				ARRAY_SIZE(TPS65020_LDO2_VSEL_table)
+};
+
 /* Supported voltage values for LDO regulators
  * for tps65021 and tps65023 */
 static const u16 TPS65023_LDO1_VSEL_table[] = {
@@ -553,6 +571,43 @@ static int __devexit tps_65023_remove(struct i2c_client *client)
 	return 0;
 }
 
+static const struct tps_info tps65020_regs[] = {
+	{
+		.name = "VDCDC1",
+		.min_uV = 3300000,
+		.max_uV = 3300000,
+		.fixed	= 1,
+	},
+	{
+		.name = "VDCDC2",
+		.min_uV =  1800000,
+		.max_uV = 1800000,
+		.fixed = 1,
+	},
+	{
+		.name = "VDCDC3",
+		.min_uV =  800000,
+		.max_uV = 1600000,
+		.table_len = ARRAY_SIZE(VCORE_VSEL_table),
+		.table = VCORE_VSEL_table,
+	},
+
+	{
+		.name = "LDO1",
+		.min_uV = 1000000,
+		.max_uV = 3150000,
+		.table_len = ARRAY_SIZE(TPS65020_LDO1_VSEL_table),
+		.table = TPS65020_LDO1_VSEL_table,
+	},
+	{
+		.name = "LDO2",
+		.min_uV = 1050000,
+		.max_uV = 3300000,
+		.table_len = ARRAY_SIZE(TPS65020_LDO2_VSEL_table),
+		.table = TPS65020_LDO2_VSEL_table,
+	},
+};
+
 static const struct tps_info tps65021_regs[] = {
 	{
 		.name = "VDCDC1",
@@ -625,6 +680,12 @@ static const struct tps_info tps65023_regs[] = {
 	},
 };

+static struct tps_driver_data tps65020_drv_data = {
+		.info = tps65020_regs,
+		.core_regulator = TPS65023_DCDC_3,
+		.num_voltages = tps65020_num_voltages,
+};
+
 static struct tps_driver_data tps65021_drv_data = {
 		.info = tps65021_regs,
 		.core_regulator = TPS65023_DCDC_3,
@@ -642,6 +703,8 @@ static const struct i2c_device_id tps_65023_id[] = {
 	.driver_data = (unsigned long) &tps65023_drv_data},
 	{.name = "tps65021",
 	.driver_data = (unsigned long) &tps65021_drv_data,},
+	{.name = "tps65020",
+	.driver_data = (unsigned long) &tps65020_drv_data},
 	{ },
 };
 
-- 
1.7.4.1