[U-Boot] [PATCH 0/3] Introduce regulator_set_enable_if_allowed()

As per the discussion happenned here[1], introducing a new api regulator_set_enable_if_allowed() that discards certain error cases where consumer driver might not be intrested in. Also using the same api for omap_hsmmc driver which fixes the boot on dra7-evm.
[1] https://patchwork.ozlabs.org/patch/1018222/
Lokesh Vutla (3): Revert "power: regulator: Return success on attempt to disable an always-on regulator" power: regulator: Introduce regulator_set_enable_if_allowed api mmc: omap_hsmmc: Use regulator_set_enable_if_allowed for enabling regulator
drivers/mmc/omap_hsmmc.c | 16 ++++++++-------- drivers/power/regulator/regulator-uclass.c | 13 ++++++++++++- include/power/regulator.h | 11 +++++++++++ 3 files changed, 31 insertions(+), 9 deletions(-)

This reverts commit e17e0ceb83538c015a50b965547f2f4d38f81c5d.
It is advised to return an error when trying to disable an always-on regulator and let the consumer driver handle the error if needed.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 39e46279d5..4511625ff2 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -113,7 +113,7 @@ int regulator_set_enable(struct udevice *dev, bool enable)
uc_pdata = dev_get_uclass_platdata(dev); if (!enable && uc_pdata->always_on) - return 0; + return -EACCES;
return ops->set_enable(dev, enable); }

On Sun, 6 Jan 2019 at 23:13, Lokesh Vutla lokeshvutla@ti.com wrote:
This reverts commit e17e0ceb83538c015a50b965547f2f4d38f81c5d.
It is advised to return an error when trying to disable an always-on regulator and let the consumer driver handle the error if needed.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
drivers/power/regulator/regulator-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

regulator_set_enable() api throws an error in the following three cases: - when requested to disable an always-on regulator - when set_enable() ops not provided by regulator driver - when enabling is actually failed.(Error returned by the regulator driver)
Sometimes consumer drivers doesn't want to track the first two scenarios and just need to worry about the case where enabling is actually failed. But it is also a good practice to have an error value returned in the first two cases.
So introduce an api regulator_set_enable_if_allowed() which ignores the first two error cases and returns an error as given by regulator driver. Consumer drivers can use this api need not worry about the first two error conditions.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- drivers/power/regulator/regulator-uclass.c | 11 +++++++++++ include/power/regulator.h | 11 +++++++++++ 2 files changed, 22 insertions(+)
diff --git a/drivers/power/regulator/regulator-uclass.c b/drivers/power/regulator/regulator-uclass.c index 4511625ff2..6f355b969a 100644 --- a/drivers/power/regulator/regulator-uclass.c +++ b/drivers/power/regulator/regulator-uclass.c @@ -118,6 +118,17 @@ int regulator_set_enable(struct udevice *dev, bool enable) return ops->set_enable(dev, enable); }
+int regulator_set_enable_if_allowed(struct udevice *dev, bool enable) +{ + int ret; + + ret = regulator_set_enable(dev, enable); + if (ret == -ENOSYS || ret == -EACCES) + return 0; + + return ret; +} + int regulator_get_mode(struct udevice *dev) { const struct dm_regulator_ops *ops = dev_get_driver_ops(dev); diff --git a/include/power/regulator.h b/include/power/regulator.h index 5318ab3ace..314160a894 100644 --- a/include/power/regulator.h +++ b/include/power/regulator.h @@ -303,6 +303,17 @@ int regulator_get_enable(struct udevice *dev); */ int regulator_set_enable(struct udevice *dev, bool enable);
+/** + * regulator_set_enable_if_allowed: set regulator enable state if allowed by + * regulator + * + * @dev - pointer to the regulator device + * @enable - set true or false + * @return - 0 on success or if enabling is not supported + * -errno val if fails. + */ +int regulator_set_enable_if_allowed(struct udevice *dev, bool enable); + /** * regulator_get_mode: get active operation mode id of a given regulator *

On Sun, 6 Jan 2019 at 23:13, Lokesh Vutla lokeshvutla@ti.com wrote:
regulator_set_enable() api throws an error in the following three cases:
- when requested to disable an always-on regulator
- when set_enable() ops not provided by regulator driver
- when enabling is actually failed.(Error returned by the regulator driver)
Sometimes consumer drivers doesn't want to track the first two scenarios and just need to worry about the case where enabling is actually failed. But it is also a good practice to have an error value returned in the first two cases.
So introduce an api regulator_set_enable_if_allowed() which ignores the first two error cases and returns an error as given by regulator driver. Consumer drivers can use this api need not worry about the first two error conditions.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
drivers/power/regulator/regulator-uclass.c | 11 +++++++++++ include/power/regulator.h | 11 +++++++++++ 2 files changed, 22 insertions(+)
Thanks for the patch. But please do adjust the tests to call this function and check that it does the right thing.

Use regulator_set_enable_if_allowed() api instead of regulator_set_enable() while enabling io regulators. This way the driver doesn't see an error when disabling an always-on regulator and when enabling is not supported.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com --- drivers/mmc/omap_hsmmc.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/omap_hsmmc.c b/drivers/mmc/omap_hsmmc.c index 5cb97eb02a..efbe75baa4 100644 --- a/drivers/mmc/omap_hsmmc.c +++ b/drivers/mmc/omap_hsmmc.c @@ -470,21 +470,21 @@ static int omap_hsmmc_set_io_regulator(struct mmc *mmc, int mV) return 0;
/* Disable PBIAS */ - ret = regulator_set_enable(priv->pbias_supply, false); - if (ret && ret != -ENOSYS) + ret = regulator_set_enable_if_allowed(priv->pbias_supply, false); + if (ret) return ret;
/* Turn off IO voltage */ - ret = regulator_set_enable(mmc->vqmmc_supply, false); - if (ret && ret != -ENOSYS) + ret = regulator_set_enable_if_allowed(mmc->vqmmc_supply, false); + if (ret) return ret; /* Program a new IO voltage value */ ret = regulator_set_value(mmc->vqmmc_supply, uV); if (ret) return ret; /* Turn on IO voltage */ - ret = regulator_set_enable(mmc->vqmmc_supply, true); - if (ret && ret != -ENOSYS) + ret = regulator_set_enable_if_allowed(mmc->vqmmc_supply, true); + if (ret) return ret;
/* Program PBIAS voltage*/ @@ -492,8 +492,8 @@ static int omap_hsmmc_set_io_regulator(struct mmc *mmc, int mV) if (ret && ret != -ENOSYS) return ret; /* Enable PBIAS */ - ret = regulator_set_enable(priv->pbias_supply, true); - if (ret && ret != -ENOSYS) + ret = regulator_set_enable_if_allowed(priv->pbias_supply, true); + if (ret) return ret;
return 0;

On Sun, 6 Jan 2019 at 23:13, Lokesh Vutla lokeshvutla@ti.com wrote:
Use regulator_set_enable_if_allowed() api instead of regulator_set_enable() while enabling io regulators. This way the driver doesn't see an error when disabling an always-on regulator and when enabling is not supported.
Signed-off-by: Lokesh Vutla lokeshvutla@ti.com
drivers/mmc/omap_hsmmc.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
participants (2)
-
Lokesh Vutla
-
Simon Glass