Re: [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
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 = [i]; > + if (ri->desc.id == id) > + return ri; > + } So this would need to be wrapped in an "if (table) { }" block. pgpr0PJ2sP20J.pgp Description: PGP signature
Re: [PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
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. pgpr0PJ2sP20J.pgp Description: PGP signature
[PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
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 --- 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 = <370>; - regulator-max-microvolt = <370>; + regulator-min-microvolt = <180>; + regulator-max-microvolt = <180>; 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[] = { 230, 2325000, 235, 2375000, 240, 2425000, 245, 2475000, }; +#define tps658623_sm2_voltages tps6586x_ldo4_voltages + static const unsigned int tps6586x_ldo_voltages[] = { 125, 150, 180, 250, 270, 285, 310, 330, }; @@ -104,6 +106,13 @@ static const unsigned int tps6586x_sm2_voltages[] = { 420, 425, 430, 435, 440, 445, 450, 455, }; +static const unsigned int tps658643_sm2_voltages[] = { + 1025000, 105, 1075000, 110, 1125000, 115, 1175000, 120, + 1225000, 125, 1275000, 130, 1325000, 135, 1375000, 140, + 1425000, 145, 1475000, 150, 1525000, 155, 1575000, 160, + 1625000, 165, 1675000, 170, 1725000, 175, 1775000, 180, +}; + static const unsigned int tps6586x_dvm_voltages[] = { 725000, 75, 775000, 80, 825000, 85, 875000, 90, 925000, 95, 975000, 100, 1025000, 105, 1075000, 110, @@ -119,8 +128,8 @@ static const unsigned int tps6586x_dvm_voltages[] = { .ops= _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), -
[PATCH v3 2/3] regulator: tps6586x: add and use correct voltage table
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 ste...@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 = 370; - regulator-max-microvolt = 370; + regulator-min-microvolt = 180; + regulator-max-microvolt = 180; 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[] = { 230, 2325000, 235, 2375000, 240, 2425000, 245, 2475000, }; +#define tps658623_sm2_voltages tps6586x_ldo4_voltages + static const unsigned int tps6586x_ldo_voltages[] = { 125, 150, 180, 250, 270, 285, 310, 330, }; @@ -104,6 +106,13 @@ static const unsigned int tps6586x_sm2_voltages[] = { 420, 425, 430, 435, 440, 445, 450, 455, }; +static const unsigned int tps658643_sm2_voltages[] = { + 1025000, 105, 1075000, 110, 1125000, 115, 1175000, 120, + 1225000, 125, 1275000, 130, 1325000, 135, 1375000, 140, + 1425000, 145, 1475000, 150, 1525000, 155, 1575000, 160, + 1625000, 165, 1675000, 170, 1725000, 175, 1775000, 180, +}; + static const unsigned int tps6586x_dvm_voltages[] = { 725000, 75, 775000, 80, 825000, 85, 875000, 90, 925000, 95, 975000, 100, 1025000, 105, 1075000, 110, @@ -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), -