From ad9d7a82901d9cea756b8666035b30ca41728e15 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Thu, 22 Aug 2024 15:20:44 +0800 Subject: [PATCH 1/3] regulator: Clarify error message for "id == NULL" in _regulator_get() The original error message simply said "get() with no identifier" without any context as to what was requested or what device the request was related to. The only thing the user or developer could do was grep for the message in the full source tree. Amend the error message to be more specific, and also use dev_* to associate the error message with a device. Signed-off-by: Chen-Yu Tsai Link: https://patch.msgid.link/20240822072047.3097740-2-wenst@chromium.org Signed-off-by: Mark Brown --- drivers/regulator/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c index 7674b7f2df14..9029de5395ee 100644 --- a/drivers/regulator/core.c +++ b/drivers/regulator/core.c @@ -2183,7 +2183,7 @@ struct regulator *_regulator_get(struct device *dev, const char *id, } if (id == NULL) { - pr_err("get() with no identifier\n"); + dev_err(dev, "regulator request with no identifier\n"); return ERR_PTR(-EINVAL); } From 395a41a1d3e377462f3eea8a205ee72be8885ff6 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Thu, 22 Aug 2024 15:20:45 +0800 Subject: [PATCH 2/3] regulator: Return actual error in of_regulator_bulk_get_all() If regulator_get() in of_regulator_bulk_get_all() returns an error, that error gets overridden and -EINVAL is always passed out. This masks probe deferral requests and likely won't work properly in all cases. Fix this by letting of_regulator_bulk_get_all() return the original error code. Fixes: 27b9ecc7a9ba ("regulator: Add of_regulator_bulk_get_all") Signed-off-by: Chen-Yu Tsai Link: https://patch.msgid.link/20240822072047.3097740-3-wenst@chromium.org Signed-off-by: Mark Brown --- drivers/regulator/of_regulator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 03afc160fc72..86b680adbf01 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -777,7 +777,7 @@ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np, name[i] = '\0'; tmp = regulator_get(dev, name); if (IS_ERR(tmp)) { - ret = -EINVAL; + ret = PTR_ERR(tmp); goto error; } (*consumers)[n].consumer = tmp; From bfefa214d179f13d01cf1b64a7efbabf586b0c49 Mon Sep 17 00:00:00 2001 From: Chen-Yu Tsai Date: Thu, 22 Aug 2024 15:20:46 +0800 Subject: [PATCH 3/3] regulator: Fully clean up on error in of_regulator_bulk_get_all() Currently in of_regulator_bulk_get_all(), if any regulator request fails, the error path releases all regulators already requested, but leaves the |struct regulator_bulk_data| memory to the caller to free, and also leaves the regulator consumer pointers dangling. The latter behavior is not documented, and may not be what the caller is expecting. Instead, explicitly clean up everything on error, and make it clear that the result pointer is only update if the whole request succeeds. Signed-off-by: Chen-Yu Tsai Link: https://patch.msgid.link/20240822072047.3097740-4-wenst@chromium.org Signed-off-by: Mark Brown --- drivers/regulator/of_regulator.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/regulator/of_regulator.c b/drivers/regulator/of_regulator.c index 86b680adbf01..d557f7b1ec7c 100644 --- a/drivers/regulator/of_regulator.c +++ b/drivers/regulator/of_regulator.c @@ -747,19 +747,19 @@ static int is_supply_name(const char *name) * This helper function allows drivers to get several regulator * consumers in one operation. If any of the regulators cannot be * acquired then any regulators that were allocated will be freed - * before returning to the caller. + * before returning to the caller, and @consumers will not be + * changed. */ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np, struct regulator_bulk_data **consumers) { int num_consumers = 0; struct regulator *tmp; + struct regulator_bulk_data *_consumers = NULL; struct property *prop; int i, n = 0, ret; char name[64]; - *consumers = NULL; - /* * first pass: get numbers of xxx-supply * second pass: fill consumers @@ -769,7 +769,7 @@ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np, i = is_supply_name(prop->name); if (i == 0) continue; - if (!*consumers) { + if (!_consumers) { num_consumers++; continue; } else { @@ -780,25 +780,28 @@ int of_regulator_bulk_get_all(struct device *dev, struct device_node *np, ret = PTR_ERR(tmp); goto error; } - (*consumers)[n].consumer = tmp; + _consumers[n].consumer = tmp; n++; continue; } } - if (*consumers) + if (_consumers) { + *consumers = _consumers; return num_consumers; + } if (num_consumers == 0) return 0; - *consumers = kmalloc_array(num_consumers, + _consumers = kmalloc_array(num_consumers, sizeof(struct regulator_bulk_data), GFP_KERNEL); - if (!*consumers) + if (!_consumers) return -ENOMEM; goto restart; error: while (--n >= 0) - regulator_put(consumers[n]->consumer); + regulator_put(_consumers[n].consumer); + kfree(_consumers); return ret; } EXPORT_SYMBOL_GPL(of_regulator_bulk_get_all);