* [PATCH v3 0/3] regulator: tps6586x: add version detection and voltage tables
@ 2013-12-03 19:57 Stefan Agner
2013-12-03 19:57 ` [PATCH v3 1/3] mfd: tps6586x: add version detection Stefan Agner
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Stefan Agner @ 2013-12-03 19:57 UTC (permalink / raw)
To: linux-arm-kernel
This is the 3rd version of this patchset which adds version detection
for the tps6586x mfd family. 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 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 | 43 ++++++++++++--
drivers/regulator/tps6586x-regulator.c | 91 +++++++++++++++++++++++-------
include/linux/mfd/tps6586x.h | 7 +++
4 files changed, 134 insertions(+), 41 deletions(-)
--
1.8.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] mfd: tps6586x: add version detection
2013-12-03 19:57 [PATCH v3 0/3] regulator: tps6586x: add version detection and voltage tables Stefan Agner
@ 2013-12-03 19:57 ` Stefan Agner
2013-12-04 9:24 ` Thierry Reding
2013-12-03 19:57 ` [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table Stefan Agner
2013-12-03 19:57 ` [PATCH v3 3/3] ARM: tegra: correct Colibri T20 regulator settings Stefan Agner
2 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2013-12-03 19:57 UTC (permalink / raw)
To: linux-arm-kernel
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>
---
Since the struct tps6586x is inside the c file, I could not easily
move the tps6586x_get_version function as inline to the header file.
Changes since v2:
- Avoid moving devm_kzalloc
---
drivers/mfd/tps6586x.c | 43 ++++++++++++++++++++++++++++++++++++++-----
include/linux/mfd/tps6586x.h | 7 +++++++
2 files changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/mfd/tps6586x.c b/drivers/mfd/tps6586x.c
index ee61fd7..56a8422 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,6 +481,31 @@ 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)
{
@@ -493,13 +527,12 @@ static int tps6586x_i2c_probe(struct i2c_client *client,
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 = ret;
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
2013-12-03 19:57 [PATCH v3 0/3] regulator: tps6586x: add version detection and voltage tables Stefan Agner
2013-12-03 19:57 ` [PATCH v3 1/3] mfd: tps6586x: add version detection Stefan Agner
@ 2013-12-03 19:57 ` Stefan Agner
2013-12-04 9:31 ` Thierry Reding
2013-12-03 19:57 ` [PATCH v3 3/3] ARM: tegra: correct Colibri T20 regulator settings Stefan Agner
2 siblings, 1 reply; 6+ messages in thread
From: Stefan Agner @ 2013-12-03 19:57 UTC (permalink / raw)
To: linux-arm-kernel
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 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 | 91 +++++++++++++++++++++++-------
2 files changed, 74 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..335ee9f 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,31 @@ 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 */
+ 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 +400,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 +423,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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 3/3] ARM: tegra: correct Colibri T20 regulator settings
2013-12-03 19:57 [PATCH v3 0/3] regulator: tps6586x: add version detection and voltage tables Stefan Agner
2013-12-03 19:57 ` [PATCH v3 1/3] mfd: tps6586x: add version detection Stefan Agner
2013-12-03 19:57 ` [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table Stefan Agner
@ 2013-12-03 19:57 ` Stefan Agner
2 siblings, 0 replies; 6+ messages in thread
From: Stefan Agner @ 2013-12-03 19:57 UTC (permalink / raw)
To: linux-arm-kernel
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v3 1/3] mfd: tps6586x: add version detection
2013-12-03 19:57 ` [PATCH v3 1/3] mfd: tps6586x: add version detection Stefan Agner
@ 2013-12-04 9:24 ` Thierry Reding
0 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2013-12-04 9:24 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 03, 2013 at 08:57:27PM +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>
> ---
> Since the struct tps6586x is inside the c file, I could not easily
> move the tps6586x_get_version function as inline to the header file.
>
> Changes since v2:
> - Avoid moving devm_kzalloc
> ---
> drivers/mfd/tps6586x.c | 43 ++++++++++++++++++++++++++++++++++++++-----
> include/linux/mfd/tps6586x.h | 7 +++++++
> 2 files changed, 45 insertions(+), 5 deletions(-)
Reviewed-by: Thierry Reding <treding@nvidia.com>
-------------- 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/20131204/b0e3f037/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
2013-12-03 19:57 ` [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table Stefan Agner
@ 2013-12-04 9:31 ` Thierry Reding
0 siblings, 0 replies; 6+ messages in thread
From: Thierry Reding @ 2013-12-04 9:31 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Dec 03, 2013 at 08:57:28PM +0100, Stefan Agner wrote:
[...]
> 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.
Well, the table was searched twice in the original patch, too, so this
variant isn't any worse, really.
> ---
> arch/arm/boot/dts/tegra20-colibri-512.dtsi | 4 +-
> drivers/regulator/tps6586x-regulator.c | 91 +++++++++++++++++++++++-------
> 2 files changed, 74 insertions(+), 21 deletions(-)
>
> diff --git a/arch/arm/boot/dts/tegra20-colibri-512.dtsi b/arch/arm/boot/dts/tegra20-colibri-512.dtsi
[...]
> -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;
> + }
> +
I think you still need to check for table to be valid here. Depending on
the version it might still be NULL.
> + /* Search version specific table first */
> + for (i = 0; i < num; i++) {
> + ri = &table[i];
> + if (ri->desc.id == id)
> + return ri;
> + }
So this would need to be wrapped in an "if (table) { }" block.
-------------- 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/20131204/cbaa8a9d/attachment.sig>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-12-04 9:31 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 19:57 [PATCH v3 0/3] regulator: tps6586x: add version detection and voltage tables Stefan Agner
2013-12-03 19:57 ` [PATCH v3 1/3] mfd: tps6586x: add version detection Stefan Agner
2013-12-04 9:24 ` Thierry Reding
2013-12-03 19:57 ` [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table Stefan Agner
2013-12-04 9:31 ` Thierry Reding
2013-12-03 19:57 ` [PATCH v3 3/3] ARM: tegra: correct Colibri T20 regulator settings Stefan Agner
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).