On 04/09/2019 13:30, Mark Brown wrote:
The panfrost driver requests a supply using regulator_get_optional() but both the name of the supply and the usage pattern suggest that it is being used for the main power for the device and is not at all optional for the device for function, there is no meaningful handling for absent supplies. Such regulators should use the vanilla regulator_get() interface, it will ensure that even if a supply is not described in the system integration one will be provided in software.
Signed-off-by: Mark Brown broonie@kernel.org
Tested-by: Steven Price steven.price@arm.com
Looks like my approach to this was wrong - so we should also revert the changes I made previously.
----8<----
From fe20f8abcde8444bb41a8f72fb35de943a27ec5c Mon Sep 17 00:00:00 2001
From: Steven Price steven.price@arm.com Date: Fri, 6 Sep 2019 15:20:53 +0100 Subject: [PATCH] drm/panfrost: Revert changes to cope with NULL regulator
Handling a NULL return from devm_regulator_get_optional() doesn't seem like the correct way of handling this. Instead revert the changes in favour of switching to using devm_regulator_get() which will return a dummy regulator instead.
Reverts commit 52282163dfa6 ("drm/panfrost: Add missing check for pfdev->regulator") Reverts commit e21dd290881b ("drm/panfrost: Enable devfreq to work without regulator")
Signed-off-by: Steven Price steven.price@arm.com --- drivers/gpu/drm/panfrost/panfrost_devfreq.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/panfrost/panfrost_devfreq.c b/drivers/gpu/drm/panfrost/panfrost_devfreq.c index a1f5fa6a742a..076983071e58 100644 --- a/drivers/gpu/drm/panfrost/panfrost_devfreq.c +++ b/drivers/gpu/drm/panfrost/panfrost_devfreq.c @@ -39,7 +39,7 @@ static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, * If frequency scaling from low to high, adjust voltage first. * If frequency scaling from high to low, adjust frequency first. */ - if (old_clk_rate < target_rate && pfdev->regulator) { + if (old_clk_rate < target_rate) { err = regulator_set_voltage(pfdev->regulator, target_volt, target_volt); if (err) { @@ -53,14 +53,12 @@ static int panfrost_devfreq_target(struct device *dev, unsigned long *freq, if (err) { dev_err(dev, "Cannot set frequency %lu (%d)\n", target_rate, err); - if (pfdev->regulator) - regulator_set_voltage(pfdev->regulator, - pfdev->devfreq.cur_volt, - pfdev->devfreq.cur_volt); + regulator_set_voltage(pfdev->regulator, pfdev->devfreq.cur_volt, + pfdev->devfreq.cur_volt); return err; }
- if (old_clk_rate > target_rate && pfdev->regulator) { + if (old_clk_rate > target_rate) { err = regulator_set_voltage(pfdev->regulator, target_volt, target_volt); if (err) @@ -138,6 +136,9 @@ int panfrost_devfreq_init(struct panfrost_device *pfdev) int ret; struct dev_pm_opp *opp;
+ if (!pfdev->regulator) + return 0; + ret = dev_pm_opp_of_add_table(&pfdev->pdev->dev); if (ret == -ENODEV) /* Optional, continue without devfreq */ return 0;