From 1a462be52f4505a2719631fb5aa7bfdbd37bfd8d Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Thu, 9 Jan 2020 10:01:04 +0100 Subject: ALSA: hda: Manage concurrent reg access more properly In the commit 8e85def5723e ("ALSA: hda: enable regmap internal locking"), we re-enabled the regmap lock due to the reported regression that showed the possible concurrent accesses. It was a temporary workaround, and there are still a few opened races even after the revert. In this patch, we cover those still opened windows with a proper mutex lock and disable the regmap internal lock again. First off, the patch introduces a new snd_hdac_device.regmap_lock mutex that is applied for each snd_hdac_regmap_*() call, including read, write and update helpers. The mutex is applied carefully so that it won't block the self-power-up procedure in the helper function. Also, this assures the protection for the accesses without regmap, too. The snd_hdac_regmap_update_raw() is refactored to use the standard regmap_update_bits_check() function instead of the open-code. The non-regmap case is still open-coded but it's an easy part. The all read and write operations are in the single mutex protection, so it's now race-free. In addition, a couple of new helper functions are added: snd_hdac_regmap_update_raw_once() and snd_hdac_regmap_sync(). Both are called from HD-audio legacy driver. The former is to initialize the given verb bits but only once when it's not initialized yet. Due to this condition, the function invokes regcache_cache_only(), and it's now performed inside the regmap_lock (formerly it was racy) too. The latter function is for simply invoking regcache_sync() inside the regmap_lock, which is called from the codec resume call path. Along with that, the HD-audio codec driver code is slightly modified / simplified to adapt those new functions. And finally, snd_hdac_regmap_read_raw(), *_write_raw(), etc are rewritten with the helper macro. It's just for simplification because the code logic is identical among all those functions. Tested-by: Kai Vehmanen Link: https://lore.kernel.org/r/20200109090104.26073-1-tiwai@suse.de Signed-off-by: Takashi Iwai --- include/sound/hda_regmap.h | 3 +++ include/sound/hdaudio.h | 1 + 2 files changed, 4 insertions(+) (limited to 'include/sound') diff --git a/include/sound/hda_regmap.h b/include/sound/hda_regmap.h index 5141f8ffbb12..4c1b9bebbd60 100644 --- a/include/sound/hda_regmap.h +++ b/include/sound/hda_regmap.h @@ -24,6 +24,9 @@ int snd_hdac_regmap_write_raw(struct hdac_device *codec, unsigned int reg, unsigned int val); int snd_hdac_regmap_update_raw(struct hdac_device *codec, unsigned int reg, unsigned int mask, unsigned int val); +int snd_hdac_regmap_update_raw_once(struct hdac_device *codec, unsigned int reg, + unsigned int mask, unsigned int val); +void snd_hdac_regmap_sync(struct hdac_device *codec); /** * snd_hdac_regmap_encode_verb - encode the verb to a pseudo register diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h index bc2f77a6f17b..541ca99b154b 100644 --- a/include/sound/hdaudio.h +++ b/include/sound/hdaudio.h @@ -86,6 +86,7 @@ struct hdac_device { /* regmap */ struct regmap *regmap; + struct mutex regmap_lock; struct snd_array vendor_verbs; bool lazy_cache:1; /* don't wake up for writes */ bool caps_overwriting:1; /* caps overwrite being in process */ -- cgit v1.2.3