CHROMIUM: MALI: More properly handle missing regulators
The mali code makes some attempt to work without regulators but it's
definitely wonky. Let's clean it up. Specifically:
1. We'll use regulator_get() instead of regulator_get_optional().
This causes the regulator framework to create a dummy regulator for
us that we can safely enable/disable and is there specifically to
avoid the need for special cases like this. When the framework
creates a dummy it nicely prints to the logs so you're aware of it,
like: "ffa30000.gpu supply bogus not found, using dummy regulator"
2. We know regulator_get() won't return NULL so don't check for it.
The old code wouldn't have worked right if regulator_get_optional()
had returned NULL since the PTR_ERR() call below wouldn't have made
sense.
3. In general we don't want to print about EPROBE_DEFER errors, so
let's not do that. ...and, in fact, now that we're using
regulator_get() we no longer _expect_ any errors, so we end up
totally inverting the old "if" test for when to print errors.
4. If a regulator is missing (or was replaced with a dummy) then
dev_opp_table will fail to init. When that happened we didn't
crash but we'd keep spewing "core: dev_pm_opp_get_voltage_supply:
Invalid supply index" all the time. Let's fix it to just not init
devfreq in this case since (I believe) devfreq makes no real sense
without a regulator.
BUG=chromium:941638
TEST=devfreq works OK on veyron still
TEST=Add 'supply-names = "mali", "bogus";' to veyron and look at dmesg
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1544030
(cherry-picked from c335bdf220e40540fc0c5913f1e07fb43d986467)
[drinkcat: Rebased on r20p0:
- Significant core changes, we do not use the default regulator
names and fetch them from device tree instead.
- Point 3 is obsolete now (the core was properly checking for
EPROBE_DEFER, yeay!
- Restore opp_table check.
]
Change-Id: Id1d242908ee972f153651df525101b8c1a40434f
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>
diff --git a/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_devfreq.c b/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_devfreq.c
index 5c17297..6ad1581 100644
--- a/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_devfreq.c
+++ b/drivers/gpu/arm/midgard/backend/gpu/mali_kbase_devfreq.c
@@ -609,6 +609,12 @@
return -ENODEV;
}
+ /* Can't do devfreq without this table */
+ if (!kbdev->opp_table) {
+ dev_err(kbdev->dev, "Uninitialized devfreq opp table\n");
+ return -ENODEV;
+ }
+
for (i = 0; i < kbdev->nr_clocks; i++) {
if (kbdev->clocks[i])
kbdev->current_freqs[i] =
diff --git a/drivers/gpu/arm/midgard/mali_kbase_core_linux.c b/drivers/gpu/arm/midgard/mali_kbase_core_linux.c
index a51698a..315ecfa 100644
--- a/drivers/gpu/arm/midgard/mali_kbase_core_linux.c
+++ b/drivers/gpu/arm/midgard/mali_kbase_core_linux.c
@@ -3418,37 +3418,53 @@
int err = 0;
unsigned int i;
#if defined(CONFIG_REGULATOR)
- static const char *regulator_names[] = {
- "mali", "shadercores"
- };
- BUILD_BUG_ON(ARRAY_SIZE(regulator_names) < BASE_MAX_NR_CLOCKS_REGULATORS);
+ const char *regulator_names[BASE_MAX_NR_CLOCKS_REGULATORS];
#endif /* CONFIG_REGULATOR */
if (!kbdev)
return -ENODEV;
+ kbdev->nr_regulators = of_property_count_strings(kbdev->dev->of_node,
+ "supply-names");
+
+ if (kbdev->nr_regulators > BASE_MAX_NR_CLOCKS_REGULATORS) {
+ dev_err(&pdev->dev, "Too many regulators: %d > %d\n",
+ kbdev->nr_regulators, BASE_MAX_NR_CLOCKS_REGULATORS);
+ return -EINVAL;
+ } else if (kbdev->nr_regulators == -EINVAL) {
+ /* The 'supply-names' is optional; if not there assume "mali" */
+ kbdev->nr_regulators = 1;
+ regulator_names[0] = "mali";
+ } else if (kbdev->nr_regulators < 0) {
+ err = kbdev->nr_regulators;
+ } else {
+ err = of_property_read_string_array(kbdev->dev->of_node,
+ "supply-names",
+ regulator_names,
+ kbdev->nr_regulators);
+ }
+
+ if (err < 0) {
+ dev_err(&pdev->dev, "Error reading supply-names: %d\n", err);
+ return err;
+ }
+
#if defined(CONFIG_REGULATOR)
/* Since the error code EPROBE_DEFER causes the entire probing
* procedure to be restarted from scratch at a later time,
* all regulators will be released before returning.
- *
- * Any other error is ignored and the driver will continue
- * operating with a partial initialization of regulators.
*/
- for (i = 0; i < BASE_MAX_NR_CLOCKS_REGULATORS; i++) {
- kbdev->regulators[i] = regulator_get_optional(kbdev->dev,
+ for (i = 0; i < kbdev->nr_regulators; i++) {
+ kbdev->regulators[i] = regulator_get(kbdev->dev,
regulator_names[i]);
- if (IS_ERR_OR_NULL(kbdev->regulators[i])) {
+ if (IS_ERR(kbdev->regulators[i])) {
err = PTR_ERR(kbdev->regulators[i]);
kbdev->regulators[i] = NULL;
- break;
+ if (err != -EPROBE_DEFER)
+ dev_err(&pdev->dev, "Failed to get regulator\n");
+ goto fail;
}
}
- if (err == -EPROBE_DEFER) {
- while ((i > 0) && (i < BASE_MAX_NR_CLOCKS_REGULATORS))
- regulator_put(kbdev->regulators[--i]);
- return err;
- }
kbdev->nr_regulators = i;
dev_dbg(&pdev->dev, "Regulators probed: %u\n", kbdev->nr_regulators);
@@ -3476,7 +3492,7 @@
while ((i > 0) && (i < BASE_MAX_NR_CLOCKS_REGULATORS)) {
clk_put(kbdev->clocks[i]);
}
- goto clocks_probe_defer;
+ goto fail;
}
kbdev->nr_clocks = i;
@@ -3499,6 +3515,12 @@
if (kbdev->nr_regulators > 0) {
kbdev->opp_table = dev_pm_opp_set_regulators(kbdev->dev,
regulator_names, BASE_MAX_NR_CLOCKS_REGULATORS);
+
+ if (IS_ERR(kbdev->opp_table)) {
+ kbdev->opp_table = NULL;
+ dev_err(kbdev->dev, "Failed to init devfreq opp table: %d\n",
+ PTR_ERR(kbdev->opp_table));
+ }
}
#endif /* (KERNEL_VERSION(4, 10, 0) <= LINUX_VERSION_CODE */
err = dev_pm_opp_of_add_table(kbdev->dev);
@@ -3508,7 +3530,7 @@
#endif /* KERNEL_VERSION(4, 4, 0) > LINUX_VERSION_CODE */
return 0;
-clocks_probe_defer:
+fail:
#if defined(CONFIG_REGULATOR)
for (i = 0; i < BASE_MAX_NR_CLOCKS_REGULATORS; i++)
regulator_put(kbdev->regulators[i]);
