[PATCH v5 0/3] regulator: tps6586x: add version detection and voltage tables

[PATCH v5 0/3] regulator: tps6586x: add version detection and voltage tables

[Stefan Agner]

This is the 5th version of this patchset which adds version detection
for the tps6586x mfd family. Just discovered a missing null reference
check which did not triggered in my tests, sorry about that.

This is required because some regulator versions use different voltage
tables. The regulator driver now uses the right voltage table according
to the version.

While at it, there are a few other incorrect regulator entries in the
device tree, change them accordingly (this patch is new and grew out
of a patch with the same name which Lucas Stach posted in July).

Tested on Colibri T20 V1.1 and V1.2 which makes use of diffent version
of that regulator.

Changes since v4:
  - Use variable named version instead of ret

Changes since v3:
  - Check if version specific table is necessary

Changes since v2:
  - Avoid moving devm_kzalloc
  - Removed reg_ from reg_version
  - Moved walk through version dependent tables to find_regulator_info,
    removed the inline definition. This reduces .o size and encapsulates
    the logic of finding the right regulator into one function. It comes
    with a slight code duplication, the table search now appears twice.

Changes since v1:
  - Merged DT and driver change (preserves bisectability)
  - Use different regulator tables for different regulator version
    * This also makes the TPS6586X_ANY version needless
  - Replaced enum with defines
  - Move version message in a sperate function
  - Include regulator bugfix, use correct supply voltage
  - Checked all voltages, fix SM0/SM1 (Core/CPU) and LDO6 voltage (VDAC/VI)
    in another commit
  - Rebased on top of v3.13-rc2


Stefan Agner (3):
  mfd: tps6586x: add version detection
  regulator: tps6586x: add and use correct voltage table
  ARM: tegra: correct Colibri T20 regulator settings

 arch/arm/boot/dts/tegra20-colibri-512.dtsi | 34 +++++------
 drivers/mfd/tps6586x.c                     | 50 +++++++++++++---
 drivers/regulator/tps6586x-regulator.c     | 93 ++++++++++++++++++++++++------
 include/linux/mfd/tps6586x.h               |  7 +++
 4 files changed, 140 insertions(+), 44 deletions(-)

-- 
1.8.4.2

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Stefan Agner]

Use the VERSIONCRC to determine the exact device version. According to
the datasheet this register can be used as device identifier. The
identification is needed since some tps6586x regulators use a different
voltage table.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes since v4:
  - Use variable named version instead of ret

Changes since v2:
  - Avoid moving devm_kzalloc
---
 drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
 include/linux/mfd/tps6586x.h |  7 +++++++
 2 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index ee61fd7..6f2af9c 100644
--- a/drivers/mfd/tps6586x.c
+++ b/drivers/mfd/tps6586x.c
@@ -124,6 +124,7 @@ struct tps6586x {
 	struct device		*dev;
 	struct i2c_client	*client;
 	struct regmap		*regmap;
+	int			version;
 
 	int			irq;
 	struct irq_chip		irq_chip;
@@ -208,6 +209,14 @@ int tps6586x_irq_get_virq(struct device *dev, int irq)
 }
 EXPORT_SYMBOL_GPL(tps6586x_irq_get_virq);
 
+int tps6586x_get_version(struct device *dev)
+{
+	struct tps6586x *tps6586x = dev_get_drvdata(dev);
+
+	return tps6586x->version;
+}
+EXPORT_SYMBOL_GPL(tps6586x_get_version);
+
 static int __remove_subdev(struct device *dev, void *unused)
 {
 	platform_device_unregister(to_platform_device(dev));
@@ -472,12 +481,38 @@ static void tps6586x_power_off(void)
 	tps6586x_set_bits(tps6586x_dev, TPS6586X_SUPPLYENE, SLEEP_MODE_BIT);
 }
 
+static void tps6586x_print_version(struct i2c_client *client, int version)
+{
+	const char *name;
+
+	switch (version) {
+	case TPS658621A:
+		name = "TPS658621A";
+		break;
+	case TPS658621CD:
+		name = "TPS658621C/D";
+		break;
+	case TPS658623:
+		name = "TPS658623";
+		break;
+	case TPS658643:
+		name = "TPS658643";
+		break;
+	default:
+		name = "TPS6586X";
+		break;
+	}
+
+	dev_info(&client->dev, "Found %s, VERSIONCRC is %02x\n", name, version);
+}
+
 static int tps6586x_i2c_probe(struct i2c_client *client,
 					const struct i2c_device_id *id)
 {
 	struct tps6586x_platform_data *pdata = dev_get_platdata(&client->dev);
 	struct tps6586x *tps6586x;
 	int ret;
+	int version;
 
 	if (!pdata && client->dev.of_node)
 		pdata = tps6586x_parse_dt(client);
@@ -487,19 +522,18 @@ static int tps6586x_i2c_probe(struct i2c_client *client,
 		return -ENOTSUPP;
 	}
 
-	ret = i2c_smbus_read_byte_data(client, TPS6586X_VERSIONCRC);
-	if (ret < 0) {
-		dev_err(&client->dev, "Chip ID read failed: %d\n", ret);
+	version = i2c_smbus_read_byte_data(client, TPS6586X_VERSIONCRC);
+	if (version < 0) {
+		dev_err(&client->dev, "Chip ID read failed: %d\n", version);
 		return -EIO;
 	}
 
-	dev_info(&client->dev, "VERSIONCRC is %02x\n", ret);
-
 	tps6586x = devm_kzalloc(&client->dev, sizeof(*tps6586x), GFP_KERNEL);
-	if (tps6586x == NULL) {
-		dev_err(&client->dev, "memory for tps6586x alloc failed\n");
+	if (!tps6586x)
 		return -ENOMEM;
-	}
+
+	tps6586x->version = version;
+	tps6586x_print_version(client, tps6586x->version);
 
 	tps6586x->client = client;
 	tps6586x->dev = &client->dev;
diff --git a/include/linux/mfd/tps6586x.h b/include/linux/mfd/tps6586x.h
index 8799454..cbecec2 100644
--- a/include/linux/mfd/tps6586x.h
+++ b/include/linux/mfd/tps6586x.h
@@ -13,6 +13,12 @@
 #define TPS6586X_SLEW_RATE_SET		0x08
 #define TPS6586X_SLEW_RATE_MASK         0x07
 
+/* VERSION CRC */
+#define TPS658621A	0x15
+#define TPS658621CD	0x2c
+#define TPS658623	0x1b
+#define TPS658643	0x03
+
 enum {
 	TPS6586X_ID_SYS,
 	TPS6586X_ID_SM_0,
@@ -97,5 +103,6 @@ extern int tps6586x_clr_bits(struct device *dev, int reg, uint8_t bit_mask);
 extern int tps6586x_update(struct device *dev, int reg, uint8_t val,
 			   uint8_t mask);
 extern int tps6586x_irq_get_virq(struct device *dev, int irq);
+extern int tps6586x_get_version(struct device *dev);
 
 #endif /*__LINUX_MFD_TPS6586X_H */
-- 
1.8.4.2

[PATCH v5 2/3] regulator: tps6586x: add and use correct voltage table

[Stefan Agner]

Depending on the regulator version, the voltage table might be
different. Use version specific regulator tables in order to select
correct voltage table. For the following regulator versions different
voltage tables are now used:

  * TPS658623: Use correct voltage table for SM2
  * TPS658643: New voltage table for SM2

Both versions are in use on the Colibri T20 module. Make use of the
correct tables by requesting the correct SM2 voltage of 1.8V.

This change is not backward compatible since an old driver is not able
to correctly set that value. The value 1.8V is out of range for the old
driver and will refuse to probe the device. The regulator starts with
default settings and the driver shows appropriate error messages.

On Colibri T20, the old value used to work with TPS658623 since the
driver applied a wrong voltage table too. However, the TPS658643 used
on V1.2 devices uses yet another voltage table and those broke that
pseudo-compatibility. The regulator driver now has the correct voltage
table for both regulator versions and those the correct voltage can be
used in the device tree.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
Changes since v3:
  - Check if version specific table is necessary

Changes since v2:
  - Removed reg_ from reg_version
  - Moved walk through version dependent tables to find_regulator_info,
    removed the inline definition. This reduces .o size and encapsulates
    the logic of finding the right regulator into one function. It comes
    with a slight code duplication, the table search now appears twice.
---
 arch/arm/boot/dts/tegra20-colibri-512.dtsi |  4 +-
 drivers/regulator/tps6586x-regulator.c     | 93 ++++++++++++++++++++++++------
 2 files changed, 76 insertions(+), 21 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
index d5c9bca..cbe89ff 100644
--- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
+++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
@@ -268,8 +268,8 @@
 					reg = <3>;
 					regulator-compatible = "sm2";
 					regulator-name = "vdd_sm2,vin_ldo*";
-					regulator-min-microvolt = <3700000>;
-					regulator-max-microvolt = <3700000>;
+					regulator-min-microvolt = <1800000>;
+					regulator-max-microvolt = <1800000>;
 					regulator-always-on;
 				};
 
diff --git a/drivers/regulator/tps6586x-regulator.c b/drivers/regulator/tps6586x-regulator.c
index e8e3a8a..0485d47 100644
--- a/drivers/regulator/tps6586x-regulator.c
+++ b/drivers/regulator/tps6586x-regulator.c
@@ -93,6 +93,8 @@ static const unsigned int tps6586x_ldo4_voltages[] = {
 	2300000, 2325000, 2350000, 2375000, 2400000, 2425000, 2450000, 2475000,
 };
 
+#define tps658623_sm2_voltages tps6586x_ldo4_voltages
+
 static const unsigned int tps6586x_ldo_voltages[] = {
 	1250000, 1500000, 1800000, 2500000, 2700000, 2850000, 3100000, 3300000,
 };
@@ -104,6 +106,13 @@ static const unsigned int tps6586x_sm2_voltages[] = {
 	4200000, 4250000, 4300000, 4350000, 4400000, 4450000, 4500000, 4550000,
 };
 
+static const unsigned int tps658643_sm2_voltages[] = {
+	1025000, 1050000, 1075000, 1100000, 1125000, 1150000, 1175000, 1200000,
+	1225000, 1250000, 1275000, 1300000, 1325000, 1350000, 1375000, 1400000,
+	1425000, 1450000, 1475000, 1500000, 1525000, 1550000, 1575000, 1600000,
+	1625000, 1650000, 1675000, 1700000, 1725000, 1750000, 1775000, 1800000,
+};
+
 static const unsigned int tps6586x_dvm_voltages[] = {
 	 725000,  750000,  775000,  800000,  825000,  850000,  875000,  900000,
 	 925000,  950000,  975000, 1000000, 1025000, 1050000, 1075000, 1100000,
@@ -119,8 +128,8 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 		.ops	= &tps6586x_regulator_ops,			\
 		.type	= REGULATOR_VOLTAGE,				\
 		.id	= TPS6586X_ID_##_id,				\
-		.n_voltages = ARRAY_SIZE(tps6586x_##vdata##_voltages),	\
-		.volt_table = tps6586x_##vdata##_voltages,		\
+		.n_voltages = ARRAY_SIZE(vdata##_voltages),		\
+		.volt_table = vdata##_voltages,				\
 		.owner	= THIS_MODULE,					\
 		.enable_reg = TPS6586X_SUPPLY##ereg0,			\
 		.enable_mask = 1 << (ebit0),				\
@@ -162,27 +171,47 @@ static const unsigned int tps6586x_dvm_voltages[] = {
 
 static struct tps6586x_regulator tps6586x_regulator[] = {
 	TPS6586X_SYS_REGULATOR(),
-	TPS6586X_LDO(LDO_0, "vinldo01", ldo0, SUPPLYV1, 5, 3, ENC, 0, END, 0),
-	TPS6586X_LDO(LDO_3, "vinldo23", ldo, SUPPLYV4, 0, 3, ENC, 2, END, 2),
-	TPS6586X_LDO(LDO_5, "REG-SYS", ldo, SUPPLYV6, 0, 3, ENE, 6, ENE, 6),
-	TPS6586X_LDO(LDO_6, "vinldo678", ldo, SUPPLYV3, 0, 3, ENC, 4, END, 4),
-	TPS6586X_LDO(LDO_7, "vinldo678", ldo, SUPPLYV3, 3, 3, ENC, 5, END, 5),
-	TPS6586X_LDO(LDO_8, "vinldo678", ldo, SUPPLYV2, 5, 3, ENC, 6, END, 6),
-	TPS6586X_LDO(LDO_9, "vinldo9", ldo, SUPPLYV6, 3, 3, ENE, 7, ENE, 7),
-	TPS6586X_LDO(LDO_RTC, "REG-SYS", ldo, SUPPLYV4, 3, 3, V4, 7, V4, 7),
-	TPS6586X_LDO(LDO_1, "vinldo01", dvm, SUPPLYV1, 0, 5, ENC, 1, END, 1),
-	TPS6586X_LDO(SM_2, "vin-sm2", sm2, SUPPLYV2, 0, 5, ENC, 7, END, 7),
-
-	TPS6586X_DVM(LDO_2, "vinldo23", dvm, LDO2BV1, 0, 5, ENA, 3,
+	TPS6586X_LDO(LDO_0, "vinldo01", tps6586x_ldo0, SUPPLYV1, 5, 3, ENC, 0,
+					END, 0),
+	TPS6586X_LDO(LDO_3, "vinldo23", tps6586x_ldo, SUPPLYV4, 0, 3, ENC, 2,
+					END, 2),
+	TPS6586X_LDO(LDO_5, "REG-SYS", tps6586x_ldo, SUPPLYV6, 0, 3, ENE, 6,
+					ENE, 6),
+	TPS6586X_LDO(LDO_6, "vinldo678", tps6586x_ldo, SUPPLYV3, 0, 3, ENC, 4,
+					END, 4),
+	TPS6586X_LDO(LDO_7, "vinldo678", tps6586x_ldo, SUPPLYV3, 3, 3, ENC, 5,
+					END, 5),
+	TPS6586X_LDO(LDO_8, "vinldo678", tps6586x_ldo, SUPPLYV2, 5, 3, ENC, 6,
+					END, 6),
+	TPS6586X_LDO(LDO_9, "vinldo9", tps6586x_ldo, SUPPLYV6, 3, 3, ENE, 7,
+					ENE, 7),
+	TPS6586X_LDO(LDO_RTC, "REG-SYS", tps6586x_ldo, SUPPLYV4, 3, 3, V4, 7,
+					V4, 7),
+	TPS6586X_LDO(LDO_1, "vinldo01", tps6586x_dvm, SUPPLYV1, 0, 5, ENC, 1,
+					END, 1),
+	TPS6586X_LDO(SM_2, "vin-sm2", tps6586x_sm2, SUPPLYV2, 0, 5, ENC, 7,
+					END, 7),
+
+	TPS6586X_DVM(LDO_2, "vinldo23", tps6586x_dvm, LDO2BV1, 0, 5, ENA, 3,
 					ENB, 3, TPS6586X_VCC2, BIT(6)),
-	TPS6586X_DVM(LDO_4, "vinldo4", ldo4, LDO4V1, 0, 5, ENC, 3,
+	TPS6586X_DVM(LDO_4, "vinldo4", tps6586x_ldo4, LDO4V1, 0, 5, ENC, 3,
 					END, 3, TPS6586X_VCC1, BIT(6)),
-	TPS6586X_DVM(SM_0, "vin-sm0", dvm, SM0V1, 0, 5, ENA, 1,
+	TPS6586X_DVM(SM_0, "vin-sm0", tps6586x_dvm, SM0V1, 0, 5, ENA, 1,
 					ENB, 1, TPS6586X_VCC1, BIT(2)),
-	TPS6586X_DVM(SM_1, "vin-sm1", dvm, SM1V1, 0, 5, ENA, 0,
+	TPS6586X_DVM(SM_1, "vin-sm1", tps6586x_dvm, SM1V1, 0, 5, ENA, 0,
 					ENB, 0, TPS6586X_VCC1, BIT(0)),
 };
 
+static struct tps6586x_regulator tps658623_regulator[] = {
+	TPS6586X_LDO(SM_2, "vin-sm2", tps658623_sm2, SUPPLYV2, 0, 5, ENC, 7,
+					END, 7),
+};
+
+static struct tps6586x_regulator tps658643_regulator[] = {
+	TPS6586X_LDO(SM_2, "vin-sm2", tps658643_sm2, SUPPLYV2, 0, 5, ENC, 7,
+					END, 7),
+};
+
 /*
  * TPS6586X has 2 enable bits that are OR'ed to determine the actual
  * regulator state. Clearing one of this bits allows switching
@@ -254,11 +283,33 @@ static int tps6586x_regulator_set_slew_rate(struct platform_device *pdev,
 			setting->slew_rate & TPS6586X_SLEW_RATE_MASK);
 }
 
-static inline struct tps6586x_regulator *find_regulator_info(int id)
+static struct tps6586x_regulator *find_regulator_info(int id, int version)
 {
 	struct tps6586x_regulator *ri;
+	struct tps6586x_regulator *table = NULL;
+	int num;
 	int i;
 
+	switch (version) {
+	case TPS658623:
+		table = tps658623_regulator;
+		num = ARRAY_SIZE(tps658623_regulator);
+		break;
+	case TPS658643:
+		table = tps658643_regulator;
+		num = ARRAY_SIZE(tps658643_regulator);
+		break;
+	}
+
+	/* Search version specific table first */
+	if (table) {
+		for (i = 0; i < num; i++) {
+			ri = &table[i];
+			if (ri->desc.id == id)
+				return ri;
+		}
+	}
+
 	for (i = 0; i < ARRAY_SIZE(tps6586x_regulator); i++) {
 		ri = &tps6586x_regulator[i];
 		if (ri->desc.id == id)
@@ -351,6 +402,7 @@ static int tps6586x_regulator_probe(struct platform_device *pdev)
 	struct regulator_init_data *reg_data;
 	struct tps6586x_platform_data *pdata;
 	struct of_regulator_match *tps6586x_reg_matches = NULL;
+	int version;
 	int id;
 	int err;
 
@@ -373,10 +425,13 @@ static int tps6586x_regulator_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	version = tps6586x_get_version(pdev->dev.parent);
+
 	for (id = 0; id < TPS6586X_ID_MAX_REGULATOR; ++id) {
 		reg_data = pdata->reg_init_data[id];
 
-		ri = find_regulator_info(id);
+		ri = find_regulator_info(id, version);
+
 		if (!ri) {
 			dev_err(&pdev->dev, "invalid regulator ID specified\n");
 			return -EINVAL;
-- 
1.8.4.2

[PATCH v5 3/3] ARM: tegra: correct Colibri T20 regulator settings

[Stefan Agner]

Set the parent of the regulators LDO2 to LDO9 according to the
schematic. Set the base voltage to 3.3V, there is only 3.3V on the
module itself.

Set the Core and CPU voltage to the specified voltages of 1.2V and
1.0V respectivly.

LDO6 should deliver 2.85V. The attached peripherals were not in
use so far.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 arch/arm/boot/dts/tegra20-colibri-512.dtsi | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
index cbe89ff..51e0880 100644
--- a/arch/arm/boot/dts/tegra20-colibri-512.dtsi
+++ b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
@@ -225,15 +225,15 @@
 			#gpio-cells = <2>;
 			gpio-controller;
 
-			sys-supply = <&vdd_5v0_reg>;
+			sys-supply = <&vdd_3v3_reg>;
 			vin-sm0-supply = <&sys_reg>;
 			vin-sm1-supply = <&sys_reg>;
 			vin-sm2-supply = <&sys_reg>;
 			vinldo01-supply = <&sm2_reg>;
-			vinldo23-supply = <&sm2_reg>;
-			vinldo4-supply = <&sm2_reg>;
-			vinldo678-supply = <&sm2_reg>;
-			vinldo9-supply = <&sm2_reg>;
+			vinldo23-supply = <&vdd_3v3_reg>;
+			vinldo4-supply = <&vdd_3v3_reg>;
+			vinldo678-supply = <&vdd_3v3_reg>;
+			vinldo9-supply = <&vdd_3v3_reg>;
 
 			regulators {
 				#address-cells = <1>;
@@ -250,8 +250,8 @@
 					reg = <1>;
 					regulator-compatible = "sm0";
 					regulator-name = "vdd_sm0,vdd_core";
-					regulator-min-microvolt = <1275000>;
-					regulator-max-microvolt = <1275000>;
+					regulator-min-microvolt = <1200000>;
+					regulator-max-microvolt = <1200000>;
 					regulator-always-on;
 				};
 
@@ -259,8 +259,8 @@
 					reg = <2>;
 					regulator-compatible = "sm1";
 					regulator-name = "vdd_sm1,vdd_cpu";
-					regulator-min-microvolt = <1100000>;
-					regulator-max-microvolt = <1100000>;
+					regulator-min-microvolt = <1000000>;
+					regulator-max-microvolt = <1000000>;
 					regulator-always-on;
 				};
 
@@ -316,8 +316,8 @@
 					reg = <10>;
 					regulator-compatible = "ldo6";
 					regulator-name = "vdd_ldo6,avdd_vdac,vddio_vi,vddio_cam";
-					regulator-min-microvolt = <1800000>;
-					regulator-max-microvolt = <1800000>;
+					regulator-min-microvolt = <2850000>;
+					regulator-max-microvolt = <2850000>;
 				};
 
 				hdmi_vdd_reg: regulator at 11 {
@@ -504,12 +504,12 @@
 		#address-cells = <1>;
 		#size-cells = <0>;
 
-		vdd_5v0_reg: regulator at 100 {
+		vdd_3v3_reg: regulator at 100 {
 			compatible = "regulator-fixed";
 			reg = <100>;
-			regulator-name = "vdd_5v0";
-			regulator-min-microvolt = <5000000>;
-			regulator-max-microvolt = <5000000>;
+			regulator-name = "vdd_3v3";
+			regulator-min-microvolt = <3300000>;
+			regulator-max-microvolt = <3300000>;
 			regulator-always-on;
 		};
 
-- 
1.8.4.2

[PATCH v5 2/3] regulator: tps6586x: add and use correct voltage table

[Mark Brown]

On Fri, Dec 06, 2013 at 01:51:46PM +0100, Stefan Agner wrote:
> Depending on the regulator version, the voltage table might be
> different. Use version specific regulator tables in order to select
> correct voltage table. For the following regulator versions different
> voltage tables are now used:

I think this is the same as the last patch I reviewed but I don't see my
ack on it?
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/de606f3e/attachment.sig>

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Thierry Reding]

On Fri, Dec 06, 2013 at 01:51:45PM +0100, Stefan Agner wrote:
> Use the VERSIONCRC to determine the exact device version. According to
> the datasheet this register can be used as device identifier. The
> identification is needed since some tps6586x regulators use a different
> voltage table.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Changes since v4:
>   - Use variable named version instead of ret
> 
> Changes since v2:
>   - Avoid moving devm_kzalloc
> ---
>  drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
>  include/linux/mfd/tps6586x.h |  7 +++++++
>  2 files changed, 49 insertions(+), 8 deletions(-)

I think I also already gave my Reviewed-by on both this and 2/3, neither
of which changed significantly enough to warrant that to be dropped. In
general it's fine to keep Reviewed-by and Acked-by as long as you're
only changing minor things.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20131206/d8a623c3/attachment.sig>

[PATCH v5 2/3] regulator: tps6586x: add and use correct voltage table

[Stefan Agner]

Am 2013-12-06 13:55, schrieb Mark Brown:
> On Fri, Dec 06, 2013 at 01:51:46PM +0100, Stefan Agner wrote:
>> Depending on the regulator version, the voltage table might be
>> different. Use version specific regulator tables in order to select
>> correct voltage table. For the following regulator versions different
>> voltage tables are now used:
> 
> I think this is the same as the last patch I reviewed but I don't see my
> ack on it?

Oh sorry, its my first multi-revision submission. I didn't realized that
it is my job to add them when doing a new revision... Lee, can you add
that when you apply the patch? So far I see

Acked-by: Mark Brown <broonie@linaro.org>
Reviewed-by: Thierry Reding <treding@nvidia.com>

for this patch.

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Stefan Agner]

Am 2013-12-06 15:26, schrieb Thierry Reding:
> On Fri, Dec 06, 2013 at 01:51:45PM +0100, Stefan Agner wrote:
>> Use the VERSIONCRC to determine the exact device version. According to
>> the datasheet this register can be used as device identifier. The
>> identification is needed since some tps6586x regulators use a different
>> voltage table.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>> Changes since v4:
>>   - Use variable named version instead of ret
>>
>> Changes since v2:
>>   - Avoid moving devm_kzalloc
>> ---
>>  drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
>>  include/linux/mfd/tps6586x.h |  7 +++++++
>>  2 files changed, 49 insertions(+), 8 deletions(-)
> 
> I think I also already gave my Reviewed-by on both this and 2/3, neither
> of which changed significantly enough to warrant that to be dropped. In
> general it's fine to keep Reviewed-by and Acked-by as long as you're
> only changing minor things.
> 
> Thierry

Ok, sorry, didn't realize that is my job to pick them up when doing new
revisions. Lee, can you add this when you apply this patch?

Reviewed-by: Thierry Reding <treding@nvidia.com>

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Lee Jones]

On Fri, 06 Dec 2013, Thierry Reding wrote:

> On Fri, Dec 06, 2013 at 01:51:45PM +0100, Stefan Agner wrote:
> > Use the VERSIONCRC to determine the exact device version. According to
> > the datasheet this register can be used as device identifier. The
> > identification is needed since some tps6586x regulators use a different
> > voltage table.
> > 
> > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> > Changes since v4:
> >   - Use variable named version instead of ret
> > 
> > Changes since v2:
> >   - Avoid moving devm_kzalloc
> > ---
> >  drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
> >  include/linux/mfd/tps6586x.h |  7 +++++++
> >  2 files changed, 49 insertions(+), 8 deletions(-)
> 
> I think I also already gave my Reviewed-by on both this and 2/3, neither
> of which changed significantly enough to warrant that to be dropped. In
> general it's fine to keep Reviewed-by and Acked-by as long as you're
> only changing minor things.

Don't worry Thierry, I'll ensure they are applied.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Lee Jones]

On Fri, 06 Dec 2013, Stefan Agner wrote:

> Am 2013-12-06 15:26, schrieb Thierry Reding:
> > On Fri, Dec 06, 2013 at 01:51:45PM +0100, Stefan Agner wrote:
> >> Use the VERSIONCRC to determine the exact device version. According to
> >> the datasheet this register can be used as device identifier. The
> >> identification is needed since some tps6586x regulators use a different
> >> voltage table.
> >>
> >> Signed-off-by: Stefan Agner <stefan@agner.ch>
> >> ---
> >> Changes since v4:
> >>   - Use variable named version instead of ret
> >>
> >> Changes since v2:
> >>   - Avoid moving devm_kzalloc
> >> ---
> >>  drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
> >>  include/linux/mfd/tps6586x.h |  7 +++++++
> >>  2 files changed, 49 insertions(+), 8 deletions(-)
> > 
> > I think I also already gave my Reviewed-by on both this and 2/3, neither
> > of which changed significantly enough to warrant that to be dropped. In
> > general it's fine to keep Reviewed-by and Acked-by as long as you're
> > only changing minor things.
> > 
> > Thierry
> 
> Ok, sorry, didn't realize that is my job to pick them up when doing new
> revisions. Lee, can you add this when you apply this patch?

Will it's not legislation, but it does help us a great deal.

> Reviewed-by: Thierry Reding <treding@nvidia.com>

Sure.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Lee Jones]

On Fri, 06 Dec 2013, Stefan Agner wrote:

> Use the VERSIONCRC to determine the exact device version. According to
> the datasheet this register can be used as device identifier. The
> identification is needed since some tps6586x regulators use a different
> voltage table.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Changes since v4:
>   - Use variable named version instead of ret
> 
> Changes since v2:
>   - Avoid moving devm_kzalloc
> ---
>  drivers/mfd/tps6586x.c       | 50 +++++++++++++++++++++++++++++++++++++-------
>  include/linux/mfd/tps6586x.h |  7 +++++++
>  2 files changed, 49 insertions(+), 8 deletions(-)

Applied with Acks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v5 2/3] regulator: tps6586x: add and use correct voltage table

[Lee Jones]

On Fri, 06 Dec 2013, Stefan Agner wrote:

> Depending on the regulator version, the voltage table might be
> different. Use version specific regulator tables in order to select
> correct voltage table. For the following regulator versions different
> voltage tables are now used:
> 
>   * TPS658623: Use correct voltage table for SM2
>   * TPS658643: New voltage table for SM2
> 
> Both versions are in use on the Colibri T20 module. Make use of the
> correct tables by requesting the correct SM2 voltage of 1.8V.
> 
> This change is not backward compatible since an old driver is not able
> to correctly set that value. The value 1.8V is out of range for the old
> driver and will refuse to probe the device. The regulator starts with
> default settings and the driver shows appropriate error messages.
> 
> On Colibri T20, the old value used to work with TPS658623 since the
> driver applied a wrong voltage table too. However, the TPS658643 used
> on V1.2 devices uses yet another voltage table and those broke that
> pseudo-compatibility. The regulator driver now has the correct voltage
> table for both regulator versions and those the correct voltage can be
> used in the device tree.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
> Changes since v3:
>   - Check if version specific table is necessary
> 
> Changes since v2:
>   - Removed reg_ from reg_version
>   - Moved walk through version dependent tables to find_regulator_info,
>     removed the inline definition. This reduces .o size and encapsulates
>     the logic of finding the right regulator into one function. It comes
>     with a slight code duplication, the table search now appears twice.
> ---
>  arch/arm/boot/dts/tegra20-colibri-512.dtsi |  4 +-
>  drivers/regulator/tps6586x-regulator.c     | 93 ++++++++++++++++++++++++-----

Applied with Reviews and Acks.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org ? Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Stephen Warren]

On 12/06/2013 05:51 AM, Stefan Agner wrote:
> Use the VERSIONCRC to determine the exact device version. According to
> the datasheet this register can be used as device identifier. The
> identification is needed since some tps6586x regulators use a different
> voltage table.

FWIW, this version looks good to me now.

[PATCH v5 1/3] mfd: tps6586x: add version detection

[Stephen Warren]

On 12/06/2013 01:24 PM, Lee Jones wrote:
> 
> On 6 Dec 2013 17:49, "Stephen Warren" <swarren@wwwdotorg.org
> <mailto:swarren@wwwdotorg.org>> wrote:
>>
>> On 12/06/2013 05:51 AM, Stefan Agner wrote:
>> > Use the VERSIONCRC to determine the exact device version. According to
>> > the datasheet this register can be used as device identifier. The
>> > identification is needed since some tps6586x regulators use a different
>> > voltage table.
>>
>> FWIW, this version looks good to me now.
> 
> If that's an Ack I can apply it.

Yes, it is. I thought you said you'd already applied it though, so I
didn't write the ack, but if not:

Acked-by: Stephen Warren <swarren@nvidia.com>

[PATCH v5 3/3] ARM: tegra: correct Colibri T20 regulator settings

[Stephen Warren]

On 12/06/2013 05:51 AM, Stefan Agner wrote:
> Set the parent of the regulators LDO2 to LDO9 according to the
> schematic. Set the base voltage to 3.3V, there is only 3.3V on the
> module itself.
> 
> Set the Core and CPU voltage to the specified voltages of 1.2V and
> 1.0V respectivly.
> 
> LDO6 should deliver 2.85V. The attached peripherals were not in
> use so far.

I've applied patch 3/3 only to Tegra's for-3.14/dt branch.