1 Jun
2015
1 Jun
'15
4:10 p.m.
On Mon, Jun 01, 2015 at 02:04:48PM +0100, Richard Fitzgerald wrote:
+int arizona_dvfs_down(struct snd_soc_codec *codec, unsigned int flags) +{
- struct arizona_priv *priv = snd_soc_codec_get_drvdata(codec);
- unsigned int old_reqs = priv->dvfs_reqs;
- int ret = 0;
- mutex_lock(&priv->dvfs_lock);
- priv->dvfs_reqs &= ~flags;
- if (!priv->dvfs_cached && old_reqs && !priv->dvfs_reqs)
ret = arizona_dvfs_disable(codec);
What is the lock intended to protect here? We read old_reqs outside the lock so it's possible that dvfs_reqs could change between us reading old_reqs and the locked section - I would have expected to see all the reads and updates to be in the locked section but perhaps it doesn't protect what I think it protects (all the DVFS-related variables).
- case SND_SOC_DAPM_PRE_PMD:
/* We must ensure DVFS is disabled before the codec goes into* suspend so that we are never in an illegal state of DVFS* enabled without enough DCVDD*/priv->dvfs_cached = true;if (priv->dvfs_reqs)ret = arizona_dvfs_disable(codec);
Are you sure that the function shouldn't check for requests? It seems like every caller is repeating the same check.