drivers/regulator/tps6287x-regulator.c | 60 ++++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+)
Changing voltage might ignore slew rate and cause a current surge.
With current implementation the driver will get the regulator to change
the voltage range used during run time. According to communication I
have had with Texas Instruments, this is not intended, since the
Dynamic Voltage Scaling in the hardware is only designed to work
within a voltage range. The current implementation will therefore
ignore the slew rate that is defined in devicetree when the voltage
range is changed during use.
The current implementation will always select a voltage in the most
accurate range that can reach that voltage even though multiple ranges
are able to reach that voltage. There are 4 Voltage ranges with the
following reach:
0b00: 0.4-0.71875V (1.25mV step size)
0b01: 0.4-1.0375V (2.5mV)
0b10: 0.4-1.675V (5mV)
0b11: 0.8-3.3V (10mV)
This in practice means that a change from below to above 0.71875V will
use the smallest range(0b00) for the values below and the second
smallest range(0b01) for the voltages above (Up to 1.675V). I have
timed how long it takes to go from below 0.71875V to above. The
increase was 100mV which, with the slew rate set to 1250µV/µs. This
in theory should take 80µs to do. With the current implementation, it
takes 10µs on my hardware. Doing the same test with the slew rate set
to 5000µV/µs, which should take 20µs, also only takes 10µs to do on
my hardware. Not only is this not in line with the technical
specification for the regulator. It also causes a current surge. Which
when calculating the output current, as described in the technical
specification, compared to what I could observe on my hardware the real
output is ~1A higher (~1.2A) than what I calculated it to be(~0.2A).
I tested also transitioning from a bigger to a smaller range, and the
results were the same.
Instead, let's limit the voltage range to a single one, which is in
line with the intended use of the regulator. This is done by looking
up the minimum and maximum requested voltage specified in devicetree.
Signed-off-by: Jonas Andreasson <jonas.andreasson@axis.com>
---
drivers/regulator/tps6287x-regulator.c | 60 ++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/drivers/regulator/tps6287x-regulator.c b/drivers/regulator/tps6287x-regulator.c
index 97f5ce138548..439ccb5f3b06 100644
--- a/drivers/regulator/tps6287x-regulator.c
+++ b/drivers/regulator/tps6287x-regulator.c
@@ -44,10 +44,35 @@ static const unsigned int tps6287x_voltage_range_sel[] = {
0x0, 0x1, 0x2, 0x3
};
+static const unsigned int tps6287x_voltage_range_prefix[] = {
+ 0x000, 0x100, 0x200, 0x300
+};
+
static const unsigned int tps6287x_ramp_table[] = {
10000, 5000, 1250, 500
};
+struct tps6287x_reg_data {
+ int range;
+};
+
+static int tps6287x_best_range(struct regulator_config *config, struct regulator_desc *desc)
+{
+ const struct linear_range *r;
+ int i;
+
+ if (!config->init_data->constraints.apply_uV)
+ return -1;
+
+ for (i = 0; i < desc->n_linear_ranges; i++) {
+ r = &desc->linear_ranges[i];
+ if (r->min <= config->init_data->constraints.min_uV &&
+ config->init_data->constraints.max_uV <= linear_range_get_max_value(r))
+ return i;
+ }
+ return -1;
+}
+
static int tps6287x_set_mode(struct regulator_dev *rdev, unsigned int mode)
{
unsigned int val;
@@ -91,6 +116,31 @@ static unsigned int tps6287x_of_map_mode(unsigned int mode)
}
}
+static int tps6287x_map_voltage(struct regulator_dev *rdev, int min_uV, int max_uV)
+{
+ struct tps6287x_reg_data *data = (struct tps6287x_reg_data *)rdev->reg_data;
+ struct linear_range selected_range;
+ int selector, voltage;
+
+ if (!data || data->range == -1)
+ goto fallback;
+
+ selected_range = rdev->desc->linear_ranges[data->range];
+ selector = DIV_ROUND_UP(min_uV - selected_range.min, selected_range.step);
+ if (selector < selected_range.min_sel || selector > selected_range.max_sel)
+ return -EINVAL;
+
+ selector |= tps6287x_voltage_range_prefix[data->range];
+ voltage = rdev->desc->ops->list_voltage(rdev, selector);
+ if (voltage < min_uV || voltage > max_uV)
+ return -EINVAL;
+
+ return selector;
+
+fallback:
+ return regulator_map_voltage_pickable_linear_range(rdev, min_uV, max_uV);
+}
+
static const struct regulator_ops tps6287x_regulator_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
@@ -100,6 +150,7 @@ static const struct regulator_ops tps6287x_regulator_ops = {
.get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
.set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
.list_voltage = regulator_list_voltage_pickable_linear_range,
+ .map_voltage = tps6287x_map_voltage,
.set_ramp_delay = regulator_set_ramp_delay_regmap,
};
@@ -130,8 +181,14 @@ static int tps6287x_i2c_probe(struct i2c_client *i2c)
{
struct device *dev = &i2c->dev;
struct regulator_config config = {};
+ struct tps6287x_reg_data *reg_data;
struct regulator_dev *rdev;
+ reg_data = devm_kzalloc(dev, sizeof(struct tps6287x_reg_data), GFP_KERNEL);
+
+ if (!reg_data)
+ return -ENOMEM;
+
config.regmap = devm_regmap_init_i2c(i2c, &tps6287x_regmap_config);
if (IS_ERR(config.regmap)) {
dev_err(dev, "Failed to init i2c\n");
@@ -143,12 +200,15 @@ static int tps6287x_i2c_probe(struct i2c_client *i2c)
config.init_data = of_get_regulator_init_data(dev, dev->of_node,
&tps6287x_reg);
+ reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
+
rdev = devm_regulator_register(dev, &tps6287x_reg, &config);
if (IS_ERR(rdev)) {
dev_err(dev, "Failed to register regulator\n");
return PTR_ERR(rdev);
}
+ rdev->reg_data = (void *)reg_data;
dev_dbg(dev, "Probed regulator\n");
return 0;
---
base-commit: adc218676eef25575469234709c2d87185ca223a
change-id: 20241212-tps-fix-eacedddb03ca
Best regards,
--
Jonas Andreasson <jonas.andreasson@axis.com>
Hi Jonas,
kernel test robot noticed the following build errors:
[auto build test ERROR on adc218676eef25575469234709c2d87185ca223a]
url: https://github.com/intel-lab-lkp/linux/commits/Jonas-Andreasson/regulator-TPS6287X-Use-min-max-uV-to-get-VRANGE/20250115-175832
base: adc218676eef25575469234709c2d87185ca223a
patch link: https://lore.kernel.org/r/20250115-tps-fix-v1-1-2bd7b316409d%40axis.com
patch subject: [PATCH] regulator: TPS6287X: Use min/max uV to get VRANGE
config: i386-randconfig-002-20250116 (https://download.01.org/0day-ci/archive/20250118/202501181129.ygE03Iho-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501181129.ygE03Iho-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501181129.ygE03Iho-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/regulator/tps6287x-regulator.c:10:
In file included from include/linux/i2c.h:19:
In file included from include/linux/regulator/consumer.h:35:
In file included from include/linux/suspend.h:5:
In file included from include/linux/swap.h:9:
In file included from include/linux/memcontrol.h:21:
In file included from include/linux/mm.h:2213:
include/linux/vmstat.h:518:36: warning: arithmetic between different enumeration types ('enum node_stat_item' and 'enum lru_list') [-Wenum-enum-conversion]
518 | return node_stat_name(NR_LRU_BASE + lru) + 3; // skip "nr_"
| ~~~~~~~~~~~ ^ ~~~
>> drivers/regulator/tps6287x-regulator.c:203:49: error: passing 'const struct regulator_desc *' to parameter of type 'struct regulator_desc *' discards qualifiers [-Werror,-Wincompatible-pointer-types-discards-qualifiers]
203 | reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
| ^~~~~~~~~~~~~
drivers/regulator/tps6287x-regulator.c:59:88: note: passing argument to parameter 'desc' here
59 | static int tps6287x_best_range(struct regulator_config *config, struct regulator_desc *desc)
| ^
1 warning and 1 error generated.
vim +203 drivers/regulator/tps6287x-regulator.c
179
180 static int tps6287x_i2c_probe(struct i2c_client *i2c)
181 {
182 struct device *dev = &i2c->dev;
183 struct regulator_config config = {};
184 struct tps6287x_reg_data *reg_data;
185 struct regulator_dev *rdev;
186
187 reg_data = devm_kzalloc(dev, sizeof(struct tps6287x_reg_data), GFP_KERNEL);
188
189 if (!reg_data)
190 return -ENOMEM;
191
192 config.regmap = devm_regmap_init_i2c(i2c, &tps6287x_regmap_config);
193 if (IS_ERR(config.regmap)) {
194 dev_err(dev, "Failed to init i2c\n");
195 return PTR_ERR(config.regmap);
196 }
197
198 config.dev = dev;
199 config.of_node = dev->of_node;
200 config.init_data = of_get_regulator_init_data(dev, dev->of_node,
201 &tps6287x_reg);
202
> 203 reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
204
205 rdev = devm_regulator_register(dev, &tps6287x_reg, &config);
206 if (IS_ERR(rdev)) {
207 dev_err(dev, "Failed to register regulator\n");
208 return PTR_ERR(rdev);
209 }
210
211 rdev->reg_data = (void *)reg_data;
212 dev_dbg(dev, "Probed regulator\n");
213
214 return 0;
215 }
216
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Jonas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on adc218676eef25575469234709c2d87185ca223a]
url: https://github.com/intel-lab-lkp/linux/commits/Jonas-Andreasson/regulator-TPS6287X-Use-min-max-uV-to-get-VRANGE/20250115-175832
base: adc218676eef25575469234709c2d87185ca223a
patch link: https://lore.kernel.org/r/20250115-tps-fix-v1-1-2bd7b316409d%40axis.com
patch subject: [PATCH] regulator: TPS6287X: Use min/max uV to get VRANGE
config: arc-randconfig-001-20250116 (https://download.01.org/0day-ci/archive/20250118/202501181152.D4FjGoUD-lkp@intel.com/config)
compiler: arceb-elf-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250118/202501181152.D4FjGoUD-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501181152.D4FjGoUD-lkp@intel.com/
All warnings (new ones prefixed by >>):
drivers/regulator/tps6287x-regulator.c: In function 'tps6287x_i2c_probe':
>> drivers/regulator/tps6287x-regulator.c:203:56: warning: passing argument 2 of 'tps6287x_best_range' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
203 | reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
| ^~~~~~~~~~~~~
drivers/regulator/tps6287x-regulator.c:59:88: note: expected 'struct regulator_desc *' but argument is of type 'const struct regulator_desc *'
59 | static int tps6287x_best_range(struct regulator_config *config, struct regulator_desc *desc)
| ~~~~~~~~~~~~~~~~~~~~~~~^~~~
vim +203 drivers/regulator/tps6287x-regulator.c
179
180 static int tps6287x_i2c_probe(struct i2c_client *i2c)
181 {
182 struct device *dev = &i2c->dev;
183 struct regulator_config config = {};
184 struct tps6287x_reg_data *reg_data;
185 struct regulator_dev *rdev;
186
187 reg_data = devm_kzalloc(dev, sizeof(struct tps6287x_reg_data), GFP_KERNEL);
188
189 if (!reg_data)
190 return -ENOMEM;
191
192 config.regmap = devm_regmap_init_i2c(i2c, &tps6287x_regmap_config);
193 if (IS_ERR(config.regmap)) {
194 dev_err(dev, "Failed to init i2c\n");
195 return PTR_ERR(config.regmap);
196 }
197
198 config.dev = dev;
199 config.of_node = dev->of_node;
200 config.init_data = of_get_regulator_init_data(dev, dev->of_node,
201 &tps6287x_reg);
202
> 203 reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
204
205 rdev = devm_regulator_register(dev, &tps6287x_reg, &config);
206 if (IS_ERR(rdev)) {
207 dev_err(dev, "Failed to register regulator\n");
208 return PTR_ERR(rdev);
209 }
210
211 rdev->reg_data = (void *)reg_data;
212 dev_dbg(dev, "Probed regulator\n");
213
214 return 0;
215 }
216
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Jonas,
kernel test robot noticed the following build warnings:
[auto build test WARNING on adc218676eef25575469234709c2d87185ca223a]
url: https://github.com/intel-lab-lkp/linux/commits/Jonas-Andreasson/regulator-TPS6287X-Use-min-max-uV-to-get-VRANGE/20250115-175832
base: adc218676eef25575469234709c2d87185ca223a
patch link: https://lore.kernel.org/r/20250115-tps-fix-v1-1-2bd7b316409d%40axis.com
patch subject: [PATCH] regulator: TPS6287X: Use min/max uV to get VRANGE
config: x86_64-randconfig-121-20250116 (https://download.01.org/0day-ci/archive/20250117/202501171805.BfxHTcAh-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250117/202501171805.BfxHTcAh-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501171805.BfxHTcAh-lkp@intel.com/
sparse warnings: (new ones prefixed by >>)
>> drivers/regulator/tps6287x-regulator.c:203:57: sparse: sparse: incorrect type in argument 2 (different modifiers) @@ expected struct regulator_desc *desc @@ got struct regulator_desc const * @@
drivers/regulator/tps6287x-regulator.c:203:57: sparse: expected struct regulator_desc *desc
drivers/regulator/tps6287x-regulator.c:203:57: sparse: got struct regulator_desc const *
drivers/regulator/tps6287x-regulator.c: note: in included file (through include/linux/mmzone.h, include/linux/gfp.h, include/linux/slab.h, ...):
include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
include/linux/page-flags.h:237:46: sparse: sparse: self-comparison always evaluates to false
vim +203 drivers/regulator/tps6287x-regulator.c
179
180 static int tps6287x_i2c_probe(struct i2c_client *i2c)
181 {
182 struct device *dev = &i2c->dev;
183 struct regulator_config config = {};
184 struct tps6287x_reg_data *reg_data;
185 struct regulator_dev *rdev;
186
187 reg_data = devm_kzalloc(dev, sizeof(struct tps6287x_reg_data), GFP_KERNEL);
188
189 if (!reg_data)
190 return -ENOMEM;
191
192 config.regmap = devm_regmap_init_i2c(i2c, &tps6287x_regmap_config);
193 if (IS_ERR(config.regmap)) {
194 dev_err(dev, "Failed to init i2c\n");
195 return PTR_ERR(config.regmap);
196 }
197
198 config.dev = dev;
199 config.of_node = dev->of_node;
200 config.init_data = of_get_regulator_init_data(dev, dev->of_node,
201 &tps6287x_reg);
202
> 203 reg_data->range = tps6287x_best_range(&config, &tps6287x_reg);
204
205 rdev = devm_regulator_register(dev, &tps6287x_reg, &config);
206 if (IS_ERR(rdev)) {
207 dev_err(dev, "Failed to register regulator\n");
208 return PTR_ERR(rdev);
209 }
210
211 rdev->reg_data = (void *)reg_data;
212 dev_dbg(dev, "Probed regulator\n");
213
214 return 0;
215 }
216
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Wed, Jan 15, 2025 at 10:55:22AM +0100, Jonas Andreasson wrote: > + if (!data || data->range == -1) > + goto fallback; ... > + return selector; > + > +fallback: > + return regulator_map_voltage_pickable_linear_range(rdev, min_uV, max_uV); You could just have the return statement directly and avoid the goto which would make this easier to follow.
On 1/16/25 16:58, Mark Brown wrote: > On Wed, Jan 15, 2025 at 10:55:22AM +0100, Jonas Andreasson wrote: > >> + if (!data || data->range == -1) >> + goto fallback; > > ... > >> + return selector; >> + >> +fallback: >> + return regulator_map_voltage_pickable_linear_range(rdev, min_uV, max_uV); > > You could just have the return statement directly and avoid the goto > which would make this easier to follow. Hello. Thanks for looking at my code! That is a good point which I will fix. Kind Regards, Jonas
© 2016 - 2025 Red Hat, Inc.