[PATCH] regulator: core: simplify _regulator_get()

From: Dmitry Torokhov
Date: Mon Feb 06 2017 - 22:56:14 EST


The code in _regulator_get() got a bit confusing over time, with control
flow jumping to a label from couple of places. Let's untangle it a bit by
doing the following:

1. Make handling of missing supplies and substituting them with dummy
regulators more explicit:

- check if we not have full constraints and refuse considering dummy
regulators with appropriate message;

- use "switch (get_type)" to handle different types of request explicitly
as well. "Normal" requests will get dummies, exclusive will not and
will notify user about that; optional will fail silently.

2. Stop jumping to a label in the middle of the function but instead have
proper conditional flow. I believe jumps should be reserved for error
handling, breaking from inner loop, or restarting a loop, but not for
implementing normal conditional flow.

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
Signed-off-by: Mark Brown <broonie@xxxxxxxxxx>
---
drivers/regulator/core.c | 66 +++++++++++++++++++++++++-----------------------
1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index 3e246c82939d..a62f5b725061 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1588,7 +1588,7 @@ struct regulator *_regulator_get(struct device *dev, const char *id,
{
struct regulator_dev *rdev;
struct regulator *regulator;
- const char *devname = NULL;
+ const char *devname = dev ? dev_name(dev) : "deviceless";
int ret;

if (get_type >= MAX_GET_TYPE) {
@@ -1601,45 +1601,47 @@ struct regulator *_regulator_get(struct device *dev, const char *id,
return ERR_PTR(-EINVAL);
}

- if (dev)
- devname = dev_name(dev);
-
rdev = regulator_dev_lookup(dev, id);
- if (!IS_ERR(rdev))
- goto found;
+ if (IS_ERR(rdev)) {
+ ret = PTR_ERR(rdev);

- ret = PTR_ERR(rdev);
- regulator = ERR_PTR(ret);
+ /*
+ * If regulator_dev_lookup() fails with error other
+ * than -ENODEV our job here is done, we simply return it.
+ */
+ if (ret != -ENODEV)
+ return ERR_PTR(ret);

- /*
- * If we have return value from dev_lookup fail, we do not expect to
- * succeed, so, quit with appropriate error value
- */
- if (ret && ret != -ENODEV)
- return regulator;
+ if (!have_full_constraints()) {
+ dev_warn(dev,
+ "incomplete constraints, dummy supplies not allowed\n");
+ return ERR_PTR(-ENODEV);
+ }

- if (!devname)
- devname = "deviceless";
+ switch (get_type) {
+ case NORMAL_GET:
+ /*
+ * Assume that a regulator is physically present and
+ * enabled, even if it isn't hooked up, and just
+ * provide a dummy.
+ */
+ dev_warn(dev,
+ "%s supply %s not found, using dummy regulator\n",
+ devname, id);
+ rdev = dummy_regulator_rdev;
+ get_device(&rdev->dev);
+ break;

- /*
- * Assume that a regulator is physically present and enabled
- * even if it isn't hooked up and just provide a dummy.
- */
- if (have_full_constraints() && get_type == NORMAL_GET) {
- pr_warn("%s supply %s not found, using dummy regulator\n",
- devname, id);
+ case EXCLUSIVE_GET:
+ dev_warn(dev,
+ "dummy supplies not allowed for exclusive requests\n");
+ /* fall through */

- rdev = dummy_regulator_rdev;
- get_device(&rdev->dev);
- goto found;
- /* Don't log an error when called from regulator_get_optional() */
- } else if (!have_full_constraints() || get_type == EXCLUSIVE_GET) {
- dev_warn(dev, "dummy supplies not allowed\n");
+ default:
+ return ERR_PTR(-ENODEV);
+ }
}

- return regulator;
-
-found:
if (rdev->exclusive) {
regulator = ERR_PTR(-EPERM);
put_device(&rdev->dev);
--
2.11.0