[PATCH] regulator: qcom_spmi: Include offset when translating voltages

[PATCH] regulator: qcom_spmi: Include offset when translating voltages

[Stephen Boyd]

This driver converts voltages from a non-linear range in hardware
to a linear range in software and vice versa. During the
conversion, we exclude certain voltages that are invalid to use
because the software interface is more flexible than reality.

For example, the FTSMPS2P5 regulators have a voltage range from
80000uV to 1355000uV that software could support, but we only
want to use the range of 350000uV to 1355000uV. If we don't
account for the hw selectors between 80000uV and 350000uV we'll
pick a hw selector of 0 to mean 350000uV when it really means
80000uV. This can cause us to program voltages into the hardware
that are significantly lower than what we're expecting.

And when we read it back from the hardware we'll have the same
problem, voltages that are in the invalid band will end up being
calculated as some software selector that represents a larger
voltage than what is programmed and the user will be confused.

Fix all this by properly offsetting the software selector and hw
selector when converting from one number space to another.

Fixes: 1b5b19689278 ("regulator: qcom_spmi: Only use selector based regulator ops")
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/regulator/qcom_spmi-regulator.c | 39 ++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 16c5f84e06a7..c372b244f3da 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -593,13 +593,20 @@ static int spmi_sw_selector_to_hw(struct spmi_regulator *vreg,
 				  u8 *voltage_sel)
 {
 	const struct spmi_voltage_range *range, *end;
+	unsigned offset;
 
 	range = vreg->set_points->range;
 	end = range + vreg->set_points->count;
 
 	for (; range < end; range++) {
 		if (selector < range->n_voltages) {
-			*voltage_sel = selector;
+			/*
+			 * hardware selectors between set point min and real
+			 * min are invalid so we ignore them
+			 */
+			offset = range->set_point_min_uV - range->min_uV;
+			offset /= range->step_uV;
+			*voltage_sel = selector + offset;
 			*range_sel = range->range_sel;
 			return 0;
 		}
@@ -613,15 +620,35 @@ static int spmi_sw_selector_to_hw(struct spmi_regulator *vreg,
 static int spmi_hw_selector_to_sw(struct spmi_regulator *vreg, u8 hw_sel,
 				  const struct spmi_voltage_range *range)
 {
-	int sw_sel = hw_sel;
+	unsigned sw_sel = 0;
+	unsigned offset, max_hw_sel;
 	const struct spmi_voltage_range *r = vreg->set_points->range;
-
-	while (r != range) {
+	const struct spmi_voltage_range *end = r + vreg->set_points->count;
+
+	for (; r < end; r++) {
+		if (r == range && range->n_voltages) {
+			/*
+			 * hardware selectors between set point min and real
+			 * min and between set point max and real max are
+			 * invalid so we return an error if they're
+			 * programmed into the hardware
+			 */
+			offset = range->set_point_min_uV - range->min_uV;
+			offset /= range->step_uV;
+			if (hw_sel < offset)
+				return -EINVAL;
+
+			max_hw_sel = range->set_point_max_uV - range->min_uV;
+			max_hw_sel /= range->step_uV;
+			if (hw_sel > max_hw_sel)
+				return -EINVAL;
+
+			return sw_sel + hw_sel - offset;
+		}
 		sw_sel += r->n_voltages;
-		r++;
 	}
 
-	return sw_sel;
+	return -EINVAL;
 }
 
 static const struct spmi_voltage_range *
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

Applied "regulator: qcom_spmi: Include offset when translating voltages" to the regulator tree

[Mark Brown]

The patch

   regulator: qcom_spmi: Include offset when translating voltages

has been applied to the regulator tree at

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

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

>From ab953b9db3a1169fbc675c8de3d2dab919ce3211 Mon Sep 17 00:00:00 2001
From: Stephen Boyd <sboyd@codeaurora.org>
Date: Wed, 1 Nov 2017 22:06:19 -0700
Subject: [PATCH] regulator: qcom_spmi: Include offset when translating
 voltages

This driver converts voltages from a non-linear range in hardware
to a linear range in software and vice versa. During the
conversion, we exclude certain voltages that are invalid to use
because the software interface is more flexible than reality.

For example, the FTSMPS2P5 regulators have a voltage range from
80000uV to 1355000uV that software could support, but we only
want to use the range of 350000uV to 1355000uV. If we don't
account for the hw selectors between 80000uV and 350000uV we'll
pick a hw selector of 0 to mean 350000uV when it really means
80000uV. This can cause us to program voltages into the hardware
that are significantly lower than what we're expecting.

And when we read it back from the hardware we'll have the same
problem, voltages that are in the invalid band will end up being
calculated as some software selector that represents a larger
voltage than what is programmed and the user will be confused.

Fix all this by properly offsetting the software selector and hw
selector when converting from one number space to another.

Fixes: 1b5b19689278 ("regulator: qcom_spmi: Only use selector based regulator ops")
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/regulator/qcom_spmi-regulator.c | 39 ++++++++++++++++++++++++++++-----
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/regulator/qcom_spmi-regulator.c b/drivers/regulator/qcom_spmi-regulator.c
index 16c5f84e06a7..c372b244f3da 100644
--- a/drivers/regulator/qcom_spmi-regulator.c
+++ b/drivers/regulator/qcom_spmi-regulator.c
@@ -593,13 +593,20 @@ static int spmi_sw_selector_to_hw(struct spmi_regulator *vreg,
 				  u8 *voltage_sel)
 {
 	const struct spmi_voltage_range *range, *end;
+	unsigned offset;
 
 	range = vreg->set_points->range;
 	end = range + vreg->set_points->count;
 
 	for (; range < end; range++) {
 		if (selector < range->n_voltages) {
-			*voltage_sel = selector;
+			/*
+			 * hardware selectors between set point min and real
+			 * min are invalid so we ignore them
+			 */
+			offset = range->set_point_min_uV - range->min_uV;
+			offset /= range->step_uV;
+			*voltage_sel = selector + offset;
 			*range_sel = range->range_sel;
 			return 0;
 		}
@@ -613,15 +620,35 @@ static int spmi_sw_selector_to_hw(struct spmi_regulator *vreg,
 static int spmi_hw_selector_to_sw(struct spmi_regulator *vreg, u8 hw_sel,
 				  const struct spmi_voltage_range *range)
 {
-	int sw_sel = hw_sel;
+	unsigned sw_sel = 0;
+	unsigned offset, max_hw_sel;
 	const struct spmi_voltage_range *r = vreg->set_points->range;
-
-	while (r != range) {
+	const struct spmi_voltage_range *end = r + vreg->set_points->count;
+
+	for (; r < end; r++) {
+		if (r == range && range->n_voltages) {
+			/*
+			 * hardware selectors between set point min and real
+			 * min and between set point max and real max are
+			 * invalid so we return an error if they're
+			 * programmed into the hardware
+			 */
+			offset = range->set_point_min_uV - range->min_uV;
+			offset /= range->step_uV;
+			if (hw_sel < offset)
+				return -EINVAL;
+
+			max_hw_sel = range->set_point_max_uV - range->min_uV;
+			max_hw_sel /= range->step_uV;
+			if (hw_sel > max_hw_sel)
+				return -EINVAL;
+
+			return sw_sel + hw_sel - offset;
+		}
 		sw_sel += r->n_voltages;
-		r++;
 	}
 
-	return sw_sel;
+	return -EINVAL;
 }
 
 static const struct spmi_voltage_range *
-- 
2.15.0