Messages in this thread | | | Date | Tue, 19 Jan 2021 10:28:27 +0530 | From | Viresh Kumar <> | Subject | Re: [PATCH v3 04/12] opp: Add dev_pm_opp_sync_regulators() |
| |
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
| |