Re: [PATCH v1 11/30] drm/tegra: dc: Support OPP and SoC core voltage scaling

Dmitry Osipenko
2020-11-10T21:23:48Z

10.11.2020 23:32, Mark Brown пишет:
> On Tue, Nov 10, 2020 at 09:29:45PM +0100, Thierry Reding wrote:
>> On Thu, Nov 05, 2020 at 02:44:08AM +0300, Dmitry Osipenko wrote:
>
>>> + /*
>>> + * Voltage scaling is optional and trying to set voltage for a dummy
>>> + * regulator will error out.
>>> + */
>>> + if (!device_property_present(dc->dev, "core-supply"))
>>> + return;
>
>> This is a potentially heavy operation, so I think we should avoid that
>> here. How about you use devm_regulator_get_optional() in ->probe()? That
>> returns -ENODEV if no regulator was specified, in which case you can set
>> dc->core_reg = NULL and use that as the condition here.
>
> Or enumerate the configurable voltages after getting the regulator and
> handle that appropriately which would be more robust in case there's
> missing or unusual constraints.
>

I already changed that code to use regulator_get_optional() for v2.

Regarding the enumerating supported voltage.. I think this should be
done by the OPP core, but regulator core doesn't work well if
regulator_get() is invoked more than one time for the same device, at
least there is a loud debugfs warning about an already existing
directory for a regulator. It's easy to check whether the debug
directory exists before creating it, like thermal framework does it for
example, but then there were some other more difficult issues.. I don't
recall what they were right now. Perhaps will be easier to simply get a
error from regulator_set_voltage() for now because it shouldn't ever
happen in practice, unless device-tree has wrong constraints.