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@xxxxxxxxxx> Tested-by: Steven Price <steven.price@xxxxxxx> 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@xxxxxxx> 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@xxxxxxx> --- 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; -- 2.20.1 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel