Re: [PATCH v3 05/12] opp: Add dev_pm_opp_set_voltage()

[Viresh Kumar]

On 18-01-21, 03:55, Dmitry Osipenko wrote:
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 99d18befc209..341484d58e6c 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2731,3 +2731,58 @@ int dev_pm_opp_sync_regulators(struct device *dev)
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_sync_regulators);
> +
> +/**
> + * dev_pm_opp_set_voltage() - Change voltage of regulators
> + * @dev:	device for which we do this operation
> + * @opp:	opp based on which the voltages are to be configured
> + *
> + * Change voltage of the OPP table regulators.
> + *
> + * Return: 0 on success or a negative error value.
> + */
> +int dev_pm_opp_set_voltage(struct device *dev, struct dev_pm_opp *opp)

I think we should do better than this, will require some work from
your part though (or I can do it if you want).

Basically what you wanted to do here is set the OPP for a device and
this means do whatever is required for setting the OPP. It is normally
frequency, which is not your case, but it is other things as well.
Like setting multiple regulators, bandwidth, required-opps, etc.

I feel the right way of doing this would be to do this:

Factor out dev_pm_opp_set_opp() from dev_pm_opp_set_rate() and make
the later call the former. And then we can just call
dev_pm_opp_set_opp() from your usecase. This will make sure we have a
single code path for all the set-opp stuff. What do you think ?

-- 
viresh