From 3028583d1314a70ca273c51e0265f698c0bd5760 Mon Sep 17 00:00:00 2001 From: Jonas Andreasson Date: Tue, 21 Jan 2025 15:37:50 +0100 Subject: [PATCH 1/2] regulator: TPS6287X: Use min/max uV to get VRANGE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Link: https://patch.msgid.link/20250121-tps-fix-v2-1-50cc4d0f1635@axis.com Signed-off-by: Mark Brown --- drivers/regulator/tps6287x-regulator.c | 57 ++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/drivers/regulator/tps6287x-regulator.c b/drivers/regulator/tps6287x-regulator.c index 97f5ce138548e..c0f5f0a186a3d 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, const 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,28 @@ 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) + return regulator_map_voltage_pickable_linear_range(rdev, min_uV, max_uV); + + 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; +} + static const struct regulator_ops tps6287x_regulator_ops = { .enable = regulator_enable_regmap, .disable = regulator_disable_regmap, @@ -100,6 +147,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 +178,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 +197,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; From 155c569fa4c3b340fbf8571a0e42dd415c025377 Mon Sep 17 00:00:00 2001 From: Alexander Stein Date: Wed, 22 Jan 2025 08:20:19 +0100 Subject: [PATCH 2/2] regulator: core: Add missing newline character dev_err_probe() error messages need newline character. Fixes: 6eabfc018e8d ("regulator: core: Allow specifying an initial load w/ the bulk API") Signed-off-by: Alexander Stein Link: https://patch.msgid.link/20250122072019.1926093-1-alexander.stein@ew.tq-group.com Signed-off-by: Mark Brown --- drivers/regulator/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 6c0ef1182248d..89578b91c4680 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -5033,7 +5033,7 @@ int _regulator_bulk_get(struct device *dev, int num_consumers, consumers[i].supply, get_type); if (IS_ERR(consumers[i].consumer)) { ret = dev_err_probe(dev, PTR_ERR(consumers[i].consumer), - "Failed to get supply '%s'", + "Failed to get supply '%s'\n", consumers[i].supply); consumers[i].consumer = NULL; goto err;