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

Dmitry Osipenko
2020-11-13T17:13:58Z

13.11.2020 19:15, Mark Brown пишет:
> On Fri, Nov 13, 2020 at 06:55:27PM +0300, Dmitry Osipenko wrote:
>> 13.11.2020 17:29, Mark Brown пишет:
>
>>> It's not clear if it matters - it's more a policy decision on the part
>>> of the driver about what it thinks safe error handling is. If it's not
>
>> If regulator_get() returns a dummy regulator, then this means that
>> regulator isn't specified in a device-tree. But then the only way for a
>> consumer driver to check whether regulator is dummy, is to check
>> presence of the supply property in a device-tree.
>
> My point here is that the driver shouldn't be checking for a dummy
> regulator, the driver should be checking the features that are provided
> to it by the regulator and handling those.

I understand yours point, but then we need dummy regulator to provide
all the features, and currently it does the opposite.

> It doesn't matter if this is
> a dummy regulator or an actual regulator with limited features, the
> effect is the same and the handling should be the same. If the driver
> is doing anything to handle dummy regulators explicitly as dummy
> regulators it is doing it wrong.

It matters because dummy regulator errors out all checks and changes
other than enable/disable, instead of accepting them. If we could add an
option for dummy regulator to succeed all the checks and accept all the
values, then it could become more usable.

>> We want to emit error messages when something goes wrong, for example
>> when regulator voltage fails to change. It's fine that voltage changes
>> are failing for a dummy regulator, but then consumer driver shouldn't
>> recognize it as a error condition.
>
> If you're fine with that you should also be fine with any other
> regulator for which you failed to enumerate any voltages which you can
> set.

It's not fine.

In the case of this driver the dummy regulator should succeed instead of
failing.

>> The regulator_get_optional() provides a more consistent and
>> straightforward way for consumer drivers to check presence of a physical
>> voltage regulator in comparison to dealing with a regulator_get(). The
>> dummy regulator is nice to use when there is no need to change
>> regulator's voltage, which doesn't work for a dummy regulator.
>
> To repeat you should *only* be using regulator_get_optional() in the
> case where the supply may be physically absent which is not the case
> here.
>

Alright, but then we either need to improve regulator core to make dummy
regulator a bit more usable, or continue to work around it in drivers.
What should we do?