2020-04-28 09:34:33

by Matti Vaittinen

[permalink] [raw]
Subject: [PATCH] regulator: bd718x7: remove voltage change restriction from BD71847

The BD71837 had a HW "feature" where changing the regulator output
voltages of other regulators but bucks 1-4 might cause spikes if
regulators were enabled. Thus SW prohibit voltage changes for other
regulators except for bucks 1-4 when regulator is enabled.

The HW colleagues did inadvertly fix this issue for BD71847 and
BD71850. Remove voltage change restrictions from other PMICs but
BD71837.

The LDO voltage changing is still restricted. I did not yet receive
answer whether there is voltage spikes to be expected for LDOs. I
only know that the power-good detection for LDOs can cause false
alarms if LDO voltage is changed when LDO is enabled. We might be
able to work-around this by disabling the power-good monioring for
the duration of the LDO voltage change - but as I said, I don't
know yet. Let's fix it later if we can confirm that also LDO voltage
changes are safe.

Signed-off-by: Matti Vaittinen <[email protected]>
---
drivers/regulator/bd718x7-regulator.c | 46 ++++++++++++++++++++++++---
1 file changed, 41 insertions(+), 5 deletions(-)

diff --git a/drivers/regulator/bd718x7-regulator.c b/drivers/regulator/bd718x7-regulator.c
index cf3872837abc..f901eee9492d 100644
--- a/drivers/regulator/bd718x7-regulator.c
+++ b/drivers/regulator/bd718x7-regulator.c
@@ -55,10 +55,15 @@ static int bd718xx_buck1234_set_ramp_delay(struct regulator_dev *rdev,
BUCK_RAMPRATE_MASK, ramp_value << 6);
}

-/* Bucks 1 to 4 support DVS. PWM mode is used when voltage is changed.
+/*
+ * On BD71837 (not on BD71847, BD71850, ...)
+ * Bucks 1 to 4 support DVS. PWM mode is used when voltage is changed.
* Bucks 5 to 8 and LDOs can use PFM and must be disabled when voltage
* is changed. Hence we return -EBUSY for these if voltage is changed
* when BUCK/LDO is enabled.
+ *
+ * The LDO operation for BD71847 and BD71850 is icurrently unknown.
+ * It's safer to still assume they can't be changed when enabled.
*/
static int bd718xx_set_voltage_sel_restricted(struct regulator_dev *rdev,
unsigned int sel)
@@ -88,6 +93,16 @@ static const struct regulator_ops bd718xx_pickable_range_ldo_ops = {
};

static const struct regulator_ops bd718xx_pickable_range_buck_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_pickable_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_pickable_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_pickable_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops bd71837_pickable_range_buck_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
.is_enabled = regulator_is_enabled_regmap,
@@ -116,6 +131,16 @@ static const struct regulator_ops bd718xx_ldo_regulator_nolinear_ops = {
};

static const struct regulator_ops bd718xx_buck_regulator_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_linear_range,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops bd71837_buck_regulator_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
.is_enabled = regulator_is_enabled_regmap,
@@ -126,6 +151,17 @@ static const struct regulator_ops bd718xx_buck_regulator_ops = {
};

static const struct regulator_ops bd718xx_buck_regulator_nolinear_ops = {
+ .enable = regulator_enable_regmap,
+ .disable = regulator_disable_regmap,
+ .is_enabled = regulator_is_enabled_regmap,
+ .list_voltage = regulator_list_voltage_table,
+ .map_voltage = regulator_map_voltage_ascend,
+ .set_voltage_sel = regulator_set_voltage_sel_regmap,
+ .get_voltage_sel = regulator_get_voltage_sel_regmap,
+ .set_voltage_time_sel = regulator_set_voltage_time_sel,
+};
+
+static const struct regulator_ops bd71837_buck_regulator_nolinear_ops = {
.enable = regulator_enable_regmap,
.disable = regulator_disable_regmap,
.is_enabled = regulator_is_enabled_regmap,
@@ -805,7 +841,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
.of_match = of_match_ptr("BUCK5"),
.regulators_node = of_match_ptr("regulators"),
.id = BD718XX_BUCK5,
- .ops = &bd718xx_pickable_range_buck_ops,
+ .ops = &bd71837_pickable_range_buck_ops,
.type = REGULATOR_VOLTAGE,
.n_voltages = BD71837_BUCK5_VOLTAGE_NUM,
.linear_ranges = bd71837_buck5_volts,
@@ -832,7 +868,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
.of_match = of_match_ptr("BUCK6"),
.regulators_node = of_match_ptr("regulators"),
.id = BD718XX_BUCK6,
- .ops = &bd718xx_buck_regulator_ops,
+ .ops = &bd71837_buck_regulator_ops,
.type = REGULATOR_VOLTAGE,
.n_voltages = BD71837_BUCK6_VOLTAGE_NUM,
.linear_ranges = bd71837_buck6_volts,
@@ -856,7 +892,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
.of_match = of_match_ptr("BUCK7"),
.regulators_node = of_match_ptr("regulators"),
.id = BD718XX_BUCK7,
- .ops = &bd718xx_buck_regulator_nolinear_ops,
+ .ops = &bd71837_buck_regulator_nolinear_ops,
.type = REGULATOR_VOLTAGE,
.volt_table = &bd718xx_3rd_nodvs_buck_volts[0],
.n_voltages = ARRAY_SIZE(bd718xx_3rd_nodvs_buck_volts),
@@ -878,7 +914,7 @@ static const struct bd718xx_regulator_data bd71837_regulators[] = {
.of_match = of_match_ptr("BUCK8"),
.regulators_node = of_match_ptr("regulators"),
.id = BD718XX_BUCK8,
- .ops = &bd718xx_buck_regulator_ops,
+ .ops = &bd71837_buck_regulator_ops,
.type = REGULATOR_VOLTAGE,
.n_voltages = BD718XX_4TH_NODVS_BUCK_VOLTAGE_NUM,
.linear_ranges = bd718xx_4th_nodvs_buck_volts,
--
2.21.0


--
Matti Vaittinen, Linux device drivers
ROHM Semiconductors, Finland SWDC
Kiviharjunlenkki 1E
90220 OULU
FINLAND

~~~ "I don't think so," said Rene Descartes. Just then he vanished ~~~
Simon says - in Latin please.
~~~ "non cogito me" dixit Rene Descarte, deinde evanescavit ~~~
Thanks to Simon Glass for the translation =]


2020-04-28 16:19:03

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH] regulator: bd718x7: remove voltage change restriction from BD71847

On Tue, 28 Apr 2020 12:29:30 +0300, Matti Vaittinen wrote:
> The BD71837 had a HW "feature" where changing the regulator output
> voltages of other regulators but bucks 1-4 might cause spikes if
> regulators were enabled. Thus SW prohibit voltage changes for other
> regulators except for bucks 1-4 when regulator is enabled.
>
> The HW colleagues did inadvertly fix this issue for BD71847 and
> BD71850. Remove voltage change restrictions from other PMICs but
> BD71837.
>
> [...]

Applied to

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

Thanks!

[1/1] regulator: bd718x7: remove voltage change restriction from BD71847
commit: f0ca7b249c7305c9b9781f7dbcb8d294e0086808

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