Skip to contentSkip to conversation

regulator: tps65023: TPS65020 support and bug fixes

PATCH Superseded Marcus Folkesson 2 participants 14 messages Jul 27, 2011 – Aug 1, 2011 Voltage and current regulator framework

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Marcus Folkesson

author
#0 lore raw

cover regulator: tps65023: TPS65020 support and bug fixes

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

author
#1 lore raw

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

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 <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   29 +++++++++++++++++++++++++----
 1 files changed, 25 insertions(+), 4 deletions(-)
drivers/regulator/tps65023-regulator.c +25 -4
index 701a590..d43533b 100644
@@ -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:
Signature · 1 line
1.7.4.1
Acked-by on v1 1/3
#2 lore raw

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

Quoted 3 lines · 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

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

Acked-by Mark Brown <[email protected]>
author
#3 lore raw

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

In reply to 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 <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |  113 ++++++++++++++++++++++++++------
 1 files changed, 93 insertions(+), 20 deletions(-)
drivers/regulator/tps65023-regulator.c +93 -20
index d43533b..5cc7344 100644
@@ -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,},
 	{ },
 };
Signature · 1 line
1.7.4.1
#4 lore raw

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

Quoted 1 line · Marcus Folkesson

TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3.

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

Quoted 2 lines

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).

author
#5 lore raw

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

2011/7/27 Mark Brown <[email protected]>:

Quoted 4 lines · Marcus Folkesson
Quoted 1 line

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.

Quoted 9 lines
Quoted 2 lines

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.

Acked-by on v1 2/3
#6 lore raw

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

In reply to Marcus Folkesson
Quoted 3 lines · 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.

Acked-by Mark Brown <[email protected]>
author
#7 lore raw

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

Quoted 6 lines · Mark Brown
Quoted 3 lines · 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.

Acked-by Mark Brown <[email protected]>

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,
};

Quoted 1 line

From 3d9f69adad628228f4941e663f802d35c9fe85ed Mon Sep 17 00:00:00 2001

From: Marcus Folkesson <[email protected]>
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 <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |  113 ++++++++++++++++++++++++++------
 1 files changed, 93 insertions(+), 20 deletions(-)
drivers/regulator/tps65023-regulator.c +93 -20
index d43533b..5cc7344 100644
@@ -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,},
 	{ },
 };
Signature · 1 line
1.7.4.1
#8 lore raw

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

Quoted 11 lines · Marcus Folkesson
Quoted 6 lines · Mark Brown
Quoted 3 lines · 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.

Acked-by Mark Brown <[email protected]>

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.

author
#9 lore raw

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

In reply to Marcus Folkesson

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

Signed-off-by Marcus Folkesson <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   63 ++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)
drivers/regulator/tps65023-regulator.c +63 -0
index 5cc7344..a4c0583 100644
@@ -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},
 	{ },
 };
Signature · 1 line
1.7.4.1
#10 lore raw

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

drivers/regulator/tps65023-regulator.c static const struct tps_info tps65023_regs[] = { · line 684

Quoted 2 lines · Marcus Folkesson
drivers/regulator/tps65023-regulator.c +2 -0
+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.

author
#11 lore raw

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

2011/7/29 Mark Brown <[email protected]>:

Quoted 8 lines · Marcus Folkesson
Quoted 3 lines

+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".

#12 lore raw

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

Quoted 6 lines · Marcus Folkesson

2011/7/29 Mark Brown <[email protected]>:

Quoted 4 lines · Marcus Folkesson
Quoted 3 lines

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

Quoted 8 lines
Quoted 2 lines

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.

author
#13 lore raw

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

Quoted 13 lines · Mark Brown
Quoted 11 lines · Marcus Folkesson

2011/7/29 Mark Brown <[email protected]>:

Quoted 7 lines · Marcus Folkesson

Review

Quoted 3 lines
(quoted diff) +2 -0
+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.

Quoted 6 lines
Quoted 2 lines

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,
};

Quoted 1 line

From 9ad596ad019feade1e346bb4128819202b26c118 Mon Sep 17 00:00:00 2001

From: Marcus Folkesson <[email protected]>
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 <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   63 ++++++++++++++++++++++++++++++++
 1 files changed, 63 insertions(+), 0 deletions(-)
drivers/regulator/tps65023-regulator.c +63 -0
index 5cc7344..a4c0583 100644
@@ -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},
 	{ },
 };
Signature · 1 line
1.7.4.1