[PATCH 1/2] regulator: da9052: Ensure enough delay time for .set_voltage_time_sel

[PATCH 1/2] regulator: da9052: Ensure enough delay time for .set_voltage_time_sel

[Axel Lin]

Use DIV_ROUND_UP to prevent truncation by integer division issue.
This ensures we return enough delay time.

Also fix returning negative value when new_sel < old_sel.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/regulator/da9052-regulator.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
index e18d291c7f21..23fa429ebe76 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -250,7 +250,8 @@ static int da9052_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 	case DA9052_ID_BUCK3:
 	case DA9052_ID_LDO2:
 	case DA9052_ID_LDO3:
-		ret = (new_sel - old_sel) * info->step_uV / 6250;
+		ret = DIV_ROUND_UP(abs(new_sel - old_sel) * info->step_uV,
+				   6250);
 		break;
 	}
 
-- 
2.25.1

[PATCH 2/2] regulator: da9052: Simplify checking DVC controlled regulators

[Axel Lin]

Only DVC controlled regulators have activate_bit set, so just check
activate_bit we can know if the regulator is DVC controlled.

Signed-off-by: Axel Lin <axel.lin@ingics.com>
---
 drivers/regulator/da9052-regulator.c | 20 ++------------------
 1 file changed, 2 insertions(+), 18 deletions(-)

diff --git a/drivers/regulator/da9052-regulator.c b/drivers/regulator/da9052-regulator.c
index 23fa429ebe76..74ab8d615d1f 100644
--- a/drivers/regulator/da9052-regulator.c
+++ b/drivers/regulator/da9052-regulator.c
@@ -207,7 +207,6 @@ static int da9052_regulator_set_voltage_sel(struct regulator_dev *rdev,
 {
 	struct da9052_regulator *regulator = rdev_get_drvdata(rdev);
 	struct da9052_regulator_info *info = regulator->info;
-	int id = rdev_get_id(rdev);
 	int ret;
 
 	ret = da9052_reg_update(regulator->da9052, rdev->desc->vsel_reg,
@@ -218,16 +217,9 @@ static int da9052_regulator_set_voltage_sel(struct regulator_dev *rdev,
 	/* Some LDOs and DCDCs are DVC controlled which requires enabling of
 	 * the activate bit to implment the changes on the output.
 	 */
-	switch (id) {
-	case DA9052_ID_BUCK1:
-	case DA9052_ID_BUCK2:
-	case DA9052_ID_BUCK3:
-	case DA9052_ID_LDO2:
-	case DA9052_ID_LDO3:
+	if (info->activate_bit)
 		ret = da9052_reg_update(regulator->da9052, DA9052_SUPPLY_REG,
 					info->activate_bit, info->activate_bit);
-		break;
-	}
 
 	return ret;
 }
@@ -238,22 +230,14 @@ static int da9052_regulator_set_voltage_time_sel(struct regulator_dev *rdev,
 {
 	struct da9052_regulator *regulator = rdev_get_drvdata(rdev);
 	struct da9052_regulator_info *info = regulator->info;
-	int id = rdev_get_id(rdev);
 	int ret = 0;
 
 	/* The DVC controlled LDOs and DCDCs ramp with 6.25mV/µs after enabling
 	 * the activate bit.
 	 */
-	switch (id) {
-	case DA9052_ID_BUCK1:
-	case DA9052_ID_BUCK2:
-	case DA9052_ID_BUCK3:
-	case DA9052_ID_LDO2:
-	case DA9052_ID_LDO3:
+	if (info->activate_bit)
 		ret = DIV_ROUND_UP(abs(new_sel - old_sel) * info->step_uV,
 				   6250);
-		break;
-	}
 
 	return ret;
 }
-- 
2.25.1

Re: [PATCH 2/2] regulator: da9052: Simplify checking DVC controlled regulators

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 400 bytes --]

On Fri, Jun 18, 2021 at 10:14:12PM +0800, Axel Lin wrote:

> Only DVC controlled regulators have activate_bit set, so just check
> activate_bit we can know if the regulator is DVC controlled.

This feels a bit icky - what if the activate_bit for some variant were
bit 0?  I'm not sure we're likely to see new variants but even so the
fact that 0 is a potentially valid value for the bit feels wrong.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: (subset) [PATCH 1/2] regulator: da9052: Ensure enough delay time for .set_voltage_time_sel

[Mark Brown]

On Fri, 18 Jun 2021 22:14:11 +0800, Axel Lin wrote:
> Use DIV_ROUND_UP to prevent truncation by integer division issue.
> This ensures we return enough delay time.
> 
> Also fix returning negative value when new_sel < old_sel.

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git for-next

Thanks!

[1/2] regulator: da9052: Ensure enough delay time for .set_voltage_time_sel
      commit: a336dc8f683e5be794186b5643cd34cb28dd2c53

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark