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

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1816 bytes --]

On Wed, Nov 11, 2020 at 12:23:41AM +0300, Dmitry Osipenko wrote:
> 10.11.2020 23:32, Mark Brown пишет:

> >>> +	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.

That doesn't look entirely appropriate given that the core does most
likely require some kind of power to operate.

> 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

I don't understand why this would be an issue - if nothing else the core
could just offer an interface to trigger the check.

> 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.

The constraints might not be wrong, there might be some board which has
a constraint somewhere for 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]