On 18-01-21, 21:35, Dmitry Osipenko wrote:
> 18.01.2021 11:20, Viresh Kumar пишет:
> >> +int dev_pm_opp_sync_regulators(struct device *dev)
> >> +{
> >> + struct opp_table *opp_table;
> >> + struct regulator *reg;
> >> + int i, ret = 0;
> >> +
> >> + /* Device may not have OPP table */
> >> + opp_table = _find_opp_table(dev);
> >> + if (IS_ERR(opp_table))
> >> + return 0;
> >> +
> >> + /* Regulator may not be required for the device */
> >> + if (!opp_table->regulators)
> >> + goto put_table;
> >> +
> >> + mutex_lock(&opp_table->lock);
> > What exactly do you need this lock for ?
>
> It is needed for protecting simultaneous invocations of
> dev_pm_opp_sync_regulators() and dev_pm_opp_set_voltage().
>
> The sync_regulators() should be invoked only after completion of the
> set_voltage() in a case of Tegra power domain driver since potentially
> both could be running in parallel. For example device driver may be
> changing performance state in a work thread, while PM domain state is
> syncing.
I think just checking the 'enabled' flag should be enough here (you may need a
lock for it though, but the lock should cover only the area it is supposed to
cover and nothing else.
> But maybe it will be better to move the protection to the PM driver
> since we're focused on sync_regulators() and set_voltage() here, but
> there are other OPP API functions which use regulators. I'll need to
> take a closer look at it.
--
viresh
