>From a34ac4da0cb94f9de9410777477388b17250d0ea Mon Sep 17 00:00:00 2001
From: Takashi Iwai <tiwai@suse.de>
Date: Wed, 24 Mar 2021 09:02:33 +0100
Subject: [PATCH v2 6/8] ALSA: control: Track the floating control
 read/write/tlv accesses

Although we introduced the power state check at the entrance of the
common handler, there can be still some pending tasks that went into
the ioctl handler, and those may access the hardware; this must be
avoided, and we have to sync such in-flight kctls finishing.

For that purpose, this patch introduces
a few new things:
- A refcount, power_ref, and a wait head, power_ref_sleep, to the card
  object
- A new helper, snd_power_wait_and_ref(), and snd_power_unref()

In a few code paths that call kctl read/write/tlv ops, we check the
power state again but at this time with snd_power_want_and_ref() --
which also takes the card.power_ref refcount.  The refcount is then
released after the kctl ops finishes.  So the driver can sync via
wait_event() with power_ref=0 for assuring all in-flight tasks are
finished.

Also, this patch changes snd_power_wait() to accept only
SNDRV_CTL_POWER_D0, which is the only value that makes sense.
In later patch, the snd_power_wait() calls will be simplified.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/core.h | 17 +++++++++++++++++
 sound/core/control.c | 29 ++++++++++++++++++++++------
 sound/core/init.c    | 45 ++++++++++++++++++++++++++++++++++++--------
 3 files changed, 77 insertions(+), 14 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index b915285a2b4e..d064451069a1 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -129,6 +129,8 @@ struct snd_card {
 #ifdef CONFIG_PM
 	unsigned int power_state;	/* power state */
 	wait_queue_head_t power_sleep;
+	refcount_t power_ref;
+	wait_queue_head_t power_ref_sleep;
 #endif
 
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
@@ -151,14 +153,29 @@ static inline void snd_power_change_state(struct snd_card *card, unsigned int st
 	wake_up(&card->power_sleep);
 }
 
+static inline void snd_power_ref(struct snd_card *card)
+{
+	refcount_inc(&card->power_ref);
+}
+
+static inline void snd_power_unref(struct snd_card *card)
+{
+	if (refcount_dec_and_test(&card->power_ref))
+		wake_up(&card->power_ref_sleep);
+}
+
 /* init.c */
 int snd_power_wait(struct snd_card *card, unsigned int power_state);
+int snd_power_wait_and_ref(struct snd_card *card, bool ref);
 
 #else /* ! CONFIG_PM */
 
 static inline int snd_power_wait(struct snd_card *card, unsigned int state) { return 0; }
 #define snd_power_get_state(card)	({ (void)(card); SNDRV_CTL_POWER_D0; })
 #define snd_power_change_state(card, state)	do { (void)(card); } while (0)
+#define snd_power_wait_and_ref(card, ref)	do { (void)(card); } while (0)
+#define snd_power_ref(card)		do { (void)(card); } while (0)
+#define snd_power_unref(card)		do { (void)(card); } while (0)
 
 #endif /* CONFIG_PM */
 
diff --git a/sound/core/control.c b/sound/core/control.c
index b9701b31de0e..efb523a97c8c 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1057,9 +1057,12 @@ static int snd_ctl_elem_read(struct snd_card *card,
 
 	if (!snd_ctl_skip_validation(&info))
 		fill_remaining_elem_value(control, &info, pattern);
+	ret = snd_power_wait_and_ref(card, true);
+	if (ret < 0)
+		goto out;
 	ret = kctl->get(kctl, control);
 	if (ret < 0)
-		return ret;
+		goto out;
 	if (!snd_ctl_skip_validation(&info) &&
 	    sanity_check_elem_value(card, control, &info, pattern) < 0) {
 		dev_err(card->dev,
@@ -1067,8 +1070,12 @@ static int snd_ctl_elem_read(struct snd_card *card,
 			control->id.iface, control->id.device,
 			control->id.subdevice, control->id.name,
 			control->id.index);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto out;
 	}
+
+ out:
+	snd_power_unref(card);
 	return ret;
 }
 
@@ -1115,16 +1122,22 @@ static int snd_ctl_elem_write(struct snd_card *card, struct snd_ctl_file *file,
 	}
 
 	snd_ctl_build_ioff(&control->id, kctl, index_offset);
+
+	result = snd_power_wait_and_ref(card, true);
+	if (result < 0)
+		goto out;
 	result = kctl->put(kctl, control);
 	if (result < 0)
-		return result;
+		goto out;
 
 	if (result > 0) {
 		struct snd_ctl_elem_id id = control->id;
 		snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, &id);
 	}
 
-	return 0;
+ out:
+	snd_power_unref(card);
+	return result < 0 ? result : 0;
 }
 
 static int snd_ctl_elem_write_user(struct snd_ctl_file *file,
@@ -1595,7 +1608,7 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag,
 		{SNDRV_CTL_TLV_OP_CMD,   SNDRV_CTL_ELEM_ACCESS_TLV_COMMAND},
 	};
 	struct snd_kcontrol_volatile *vd = &kctl->vd[snd_ctl_get_ioff(kctl, id)];
-	int i;
+	int i, ret;
 
 	/* Check support of the request for this element. */
 	for (i = 0; i < ARRAY_SIZE(pairs); ++i) {
@@ -1613,7 +1626,11 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag,
 	    vd->owner != NULL && vd->owner != file)
 		return -EPERM;
 
-	return kctl->tlv.c(kctl, op_flag, size, buf);
+	ret = snd_power_wait_and_ref(file->card, true);
+	if (!ret)
+		ret = kctl->tlv.c(kctl, op_flag, size, buf);
+	snd_power_unref(file->card);
+	return ret;
 }
 
 static int read_tlv_buf(struct snd_kcontrol *kctl, struct snd_ctl_elem_id *id,
diff --git a/sound/core/init.c b/sound/core/init.c
index 45f4b01de23f..ebca12e120a7 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -220,6 +220,8 @@ int snd_card_new(struct device *parent, int idx, const char *xid,
 	mutex_init(&card->memory_mutex);
 #ifdef CONFIG_PM
 	init_waitqueue_head(&card->power_sleep);
+	refcount_set(&card->power_ref, 0);
+	init_waitqueue_head(&card->power_ref_sleep);
 #endif
 	init_waitqueue_head(&card->remove_sleep);
 	card->sync_irq = -1;
@@ -1004,21 +1006,27 @@ EXPORT_SYMBOL(snd_card_file_remove);
 
 #ifdef CONFIG_PM
 /**
- *  snd_power_wait - wait until the power-state is changed.
- *  @card: soundcard structure
- *  @power_state: expected power state
+ * snd_power_wait_and_ref - wait until the card gets powered up
+ * @card: soundcard structure
+ * @ref: take power_ref refcount if set
  *
- *  Waits until the power-state is changed.
+ * Waits until the card gets powered up to SNDRV_CTL_POWER_D0 state.
+ * When @ref is set, power_ref refcount is incremented.  The refcount is
+ * down while sleeping, hence it can be used for syncing the floating control
+ * ops accesses.
+ * The caller needs to pull down the refcount via snd_power_unref() later.
  *
- *  Return: Zero if successful, or a negative error code.
+ * Return: Zero if successful, or a negative error code.
  */
-int snd_power_wait(struct snd_card *card, unsigned int power_state)
+int snd_power_wait_and_ref(struct snd_card *card, bool ref)
 {
 	wait_queue_entry_t wait;
 	int result = 0;
 
 	/* fastpath */
-	if (snd_power_get_state(card) == power_state)
+	if (ref)
+		snd_power_ref(card);
+	if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0)
 		return 0;
 	init_waitqueue_entry(&wait, current);
 	add_wait_queue(&card->power_sleep, &wait);
@@ -1027,13 +1035,34 @@ int snd_power_wait(struct snd_card *card, unsigned int power_state)
 			result = -ENODEV;
 			break;
 		}
-		if (snd_power_get_state(card) == power_state)
+		if (snd_power_get_state(card) == SNDRV_CTL_POWER_D0)
 			break;
+		if (ref)
+			snd_power_unref(card);
 		set_current_state(TASK_UNINTERRUPTIBLE);
 		schedule_timeout(30 * HZ);
+		if (ref)
+			snd_power_ref(card);
 	}
 	remove_wait_queue(&card->power_sleep, &wait);
 	return result;
 }
+EXPORT_SYMBOL_GPL(snd_power_wait_and_ref);
+
+/**
+ * snd_power_wait - wait until the card gets powered up (old form)
+ * @card: soundcard structure
+ * @power_state: expected power state
+ *
+ * Same as snd_power_wait_and_ref() with ref=false.
+ * @power_state must be SNDRV_CTL_POWER_D0.
+ *
+ * Return: Zero if successful, or a negative error code.
+ */
+int snd_power_wait(struct snd_card *card, unsigned int power_state)
+{
+	WARN_ON(power_state != SNDRV_CTL_POWER_D0);
+	return snd_power_wait_and_ref(card, false);
+}
 EXPORT_SYMBOL(snd_power_wait);
 #endif /* CONFIG_PM */
-- 
2.26.2

