Skip to contentSkip to conversation

regulator: tps65023: patch updates

PATCH In review Marcus Folkesson 3 participants 12 messages Aug 4, 2011 – Aug 8, 2011 Voltage and current regulator framework

lore
Share & tools
lore

Marcus Folkesson

author
#0 lore raw

cover regulator: tps65023: patch updates

I'm currently fighting with my email-client (Mark, sorry for the draft just sent), but this should be proper in both formatting and email-list.

Changes from previous patches:
* Split up [PATCH 1/3] into two seperate patches ([PATCH [1-2]/4])
* Declared tps_driver_data.info as const for avoiding awkward typecastings
* Removed table-length structs and uses tps->table_len instead. (As Axel Lin suggested in lkml)

author
#1 lore raw

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

Allow i2c core voltage adjustments by clearing CORE ADJ Allowed bit in CTRL2

Signed-off-by Marcus Folkesson <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
drivers/regulator/tps65023-regulator.c +11 -0
index 701a590..a81abd4 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))

@@ -475,6 +482,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
author
#3 lore raw

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

Quoted 5 lines · Mark Brown
Quoted 3 lines · Marcus Folkesson

Allow i2c core voltage adjustments by clearing CORE ADJ Allowed bit in CTRL2

Signed-off-by Marcus Folkesson <[email protected]>
Acked-by Mark Brown <[email protected]>

Changes:
-#define TPS65023_REG_CTRL2_DCDC1 BIT(2)
+#define TPS65023_REG_CTRL2_DCDC1 BIT(1)

Thank you Bernd Petrovitsch for mention it!

Best regards
Marcus

Quoted 1 line

From 20c5ce4865050edd6da6e0cfb400cc858c7483dd Mon Sep 17 00:00:00 2001

From: Marcus Folkesson <[email protected]>
Date: Tue, 2 Aug 2011 10:22:34 +0200
Subject: [PATCH 1/4] regulator: tps65023: Fixes i2c configuration issues

Allow i2c core voltage adjustments by clearing CORE ADJ Allowed bit in CTRL2

Signed-off-by Marcus Folkesson <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
drivers/regulator/tps65023-regulator.c +11 -0
index 701a590..a81abd4 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(1)
+#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))

@@ -475,6 +482,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
#4 lore raw

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

Quoted 5 lines · Marcus Folkesson

Changes:

(quoted diff) +1 -1
-#define TPS65023_REG_CTRL2_DCDC1	BIT(2)
+#define TPS65023_REG_CTRL2_DCDC1	BIT(1)

Thank you Bernd Petrovitsch for mention it!

Please submit patches in the format documented in SubmittingPatches.

but

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

PATCH 2/4 regulator: tps65023: Set missing bit for update core-voltage

In reply to Marcus Folkesson

Setting the GO bit in CTRL2 for updating the core voltage

Signed-off-by Marcus Folkesson <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   19 ++++++++++++++-----
 1 files changed, 14 insertions(+), 5 deletions(-)
drivers/regulator/tps65023-regulator.c +14 -5
index a81abd4..8bd41fa 100644
@@ -277,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;

@@ -299,11 +300,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)
Signature · 1 line
1.7.4.1
#6 lore raw

Re: [PATCH 2/4] regulator: tps65023: Set missing bit for update core-voltage

drivers/regulator/tps65023-regulator.c static int tps65023_dcdc_set_voltage(struct regulator_dev *dev, · line 315

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

This seems convoluted and non-scalable if we add more things here - it'd seem to be better to return an error and then have the write and so on as part of the regular function rather than putting them within the else clause.

author
#7 lore raw

PATCH 3/4 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 and voltage-tables may differ between different regulators. These two is now passed as driver data.

Signed-off-by Marcus Folkesson <[email protected]>
---
 drivers/regulator/tps65023-regulator.c |   96 +++++++++++++++++++++++++-------
 1 files changed, 75 insertions(+), 21 deletions(-)
drivers/regulator/tps65023-regulator.c +75 -21
index 8bd41fa..0161410 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,20 +103,20 @@ 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)};
-
 /* Regulator specific details */
 struct tps_info {
 	const char *name;

@@ -134,6 +134,13 @@ 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;
 };
 
 static int tps_65023_set_bits(struct tps_pmic *tps, u8 reg, u8 mask)

@@ -260,7 +267,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 +286,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;

@@ -378,7 +384,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

@@ -430,7 +436,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;

@@ -462,6 +469,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 */

@@ -469,7 +477,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 = info->table_len;
 		tps->desc[i].ops = (i > TPS65023_DCDC_3 ?
 					&tps65023_ldo_ops : &tps65023_dcdc_ops);
 		tps->desc[i].type = REGULATOR_VOLTAGE;

@@ -527,13 +535,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",

@@ -551,23 +595,33 @@ 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,
+};
+
+static struct tps_driver_data tps65023_drv_data = {
+		.info = tps65023_regs,
+		.core_regulator = TPS65023_DCDC_1,
+};
+
 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
Acked-by on v1 3/4
#8 lore raw

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

Quoted 5 lines · Marcus Folkesson

TPS65023 is using VDCDC1 as core regulator and TPS65021 is using VDCDC3. Core-regulator and voltage-tables may differ between different regulators. These two is now passed as driver data.

Signed-off-by Marcus Folkesson <[email protected]>
Acked-by Mark Brown <[email protected]>
author
#9 lore raw

PATCH 4/4 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 |   53 ++++++++++++++++++++++++++++++++
 1 files changed, 53 insertions(+), 0 deletions(-)
drivers/regulator/tps65023-regulator.c +53 -0
index 0161410..bf3f91c 100644
@@ -103,7 +103,16 @@ 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,
+};
 
 /* Supported voltage values for LDO regulators
  * for tps65021 and tps65023 */

@@ -535,6 +544,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",

@@ -607,6 +653,11 @@ static const struct tps_info tps65023_regs[] = {
 	},
 };
 
+static struct tps_driver_data tps65020_drv_data = {
+		.info = tps65020_regs,
+		.core_regulator = TPS65023_DCDC_3,
+};
+
 static struct tps_driver_data tps65021_drv_data = {
 		.info = tps65021_regs,
 		.core_regulator = TPS65023_DCDC_3,

@@ -622,6 +673,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
Acked-by on v1 4/4
#10 lore raw

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

Quoted 3 lines · Marcus Folkesson

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

Signed-off-by Marcus Folkesson <[email protected]>
Acked-by Mark Brown <[email protected]>

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

Quoted 4 lines
drivers/regulator/tps65023-regulator.c +4 -0
+static struct tps_driver_data tps65020_drv_data = {
+		.info = tps65020_regs,
+		.core_regulator = TPS65023_DCDC_3,
+};

but the indentation here is a bit odd, it's double indented.

#11 lore raw
In reply to Marcus Folkesson
Quoted 8 lines · Marcus Folkesson

I'm currently fighting with my email-client (Mark, sorry for the draft just sent), but this should be proper in both formatting and email-list.

Changes from previous patches:
* Split up [PATCH 1/3] into two seperate patches ([PATCH [1-2]/4])
* Declared tps_driver_data.info as const for avoiding awkward typecastings
* Removed table-length structs and uses tps->table_len instead. (As Axel Lin suggested in lkml)

I've applied patch 1. 3 & 4 don't apply against -next.

Can you resend and add Mark's Acks to the relevant patches.

Thanks

Liam