[PATCH 0/4] ALSA: firewire: block .remove callback of bus

[PATCH 0/4] ALSA: firewire: block .remove callback of bus

[Takashi Sakamoto]

Hi,

In a discussion for devres support[1], I realize difference of unbind
behaviour of drivers in ALSA firewire stack and in the others. For
consistency behaviour inner the same subsystem for users, it's better
to imitate the behaviour.

Additionally, blocking .remove function simplifies codes to releasing
device.

This commit uses 'snd_card_free()' instead of
'snd_card_free_when_closed()' in .remove function and performs
refactoring for release codes.

[1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-September/140431.html

Takashi Sakamoto (4):
  ALSA: firewire: block .remove callback of bus driver till all of ALSA
    character devices are released
  ALSA: firewire: release reference count of firewire unit in .remove
    callback of bus driver
  ALSA: bebob/fireworks: simplify handling of local device entry table
  ALSA: firewire: simplify cleanup process when failing to register
    sound card

 sound/firewire/bebob/bebob.c         | 43 ++++++---------------
 sound/firewire/dice/dice.c           | 35 ++++-------------
 sound/firewire/digi00x/digi00x.c     | 28 +++++---------
 sound/firewire/fireface/ff.c         | 28 +++++---------
 sound/firewire/fireworks/fireworks.c | 56 ++++++++--------------------
 sound/firewire/isight.c              |  8 ++--
 sound/firewire/motu/motu.c           | 39 +++++--------------
 sound/firewire/oxfw/oxfw.c           | 39 +++++--------------
 sound/firewire/tascam/tascam.c       | 32 +++++-----------
 9 files changed, 90 insertions(+), 218 deletions(-)

-- 
2.19.0

[PATCH 1/4] ALSA: firewire: block .remove callback of bus driver till all of ALSA character devices are released

[Takashi Sakamoto]

At present, in .remove callback of bus driver just decrease reference
count of device for ALSA card instance. This delegates release of the
device to a process in which the last of ALSA character device is
released.

On the other hand, the other drivers such as for devices on PCIe are
programmed to block .remove callback of bus driver till all of ALSA
character devices are released.

For consistency of behaviour for whole drivers, this probably confuses
users. This commit takes drivers in ALSA firewire stack to imitate the
above behaviour.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c         | 4 ++--
 sound/firewire/digi00x/digi00x.c     | 4 ++--
 sound/firewire/fireface/ff.c         | 4 ++--
 sound/firewire/fireworks/fireworks.c | 4 ++--
 sound/firewire/isight.c              | 3 ++-
 sound/firewire/motu/motu.c           | 4 ++--
 sound/firewire/oxfw/oxfw.c           | 4 ++--
 sound/firewire/tascam/tascam.c       | 4 ++--
 8 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 72b04214a3b5..3a5579cb3aa8 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -374,8 +374,8 @@ static void bebob_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&bebob->dwork);
 
 	if (bebob->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(bebob->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(bebob->card);
 	} else {
 		/* Don't forget this case. */
 		bebob_free(bebob);
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 654420f1c9bd..554d7ff737a2 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -172,8 +172,8 @@ static void snd_dg00x_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&dg00x->dwork);
 
 	if (dg00x->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(dg00x->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(dg00x->card);
 	} else {
 		/* Don't forget this case. */
 		dg00x_free(dg00x);
diff --git a/sound/firewire/fireface/ff.c b/sound/firewire/fireface/ff.c
index 98731bd8816f..73425dfe63bf 100644
--- a/sound/firewire/fireface/ff.c
+++ b/sound/firewire/fireface/ff.c
@@ -145,8 +145,8 @@ static void snd_ff_remove(struct fw_unit *unit)
 	cancel_work_sync(&ff->dwork.work);
 
 	if (ff->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(ff->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(ff->card);
 	} else {
 		/* Don't forget this case. */
 		ff_free(ff);
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index f680e2f27806..5a17ead86e61 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -358,8 +358,8 @@ static void efw_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&efw->dwork);
 
 	if (efw->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(efw->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(efw->card);
 	} else {
 		/* Don't forget this case. */
 		efw_free(efw);
diff --git a/sound/firewire/isight.c b/sound/firewire/isight.c
index 30957477e005..1f591c8805ea 100644
--- a/sound/firewire/isight.c
+++ b/sound/firewire/isight.c
@@ -703,7 +703,8 @@ static void isight_remove(struct fw_unit *unit)
 	isight_stop_streaming(isight);
 	mutex_unlock(&isight->mutex);
 
-	snd_card_free_when_closed(isight->card);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(isight->card);
 }
 
 static const struct ieee1394_device_id isight_id_table[] = {
diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c
index fd5726424c7a..12680c85b37f 100644
--- a/sound/firewire/motu/motu.c
+++ b/sound/firewire/motu/motu.c
@@ -172,8 +172,8 @@ static void motu_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&motu->dwork);
 
 	if (motu->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(motu->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(motu->card);
 	} else {
 		/* Don't forget this case. */
 		motu_free(motu);
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 6ac551786b93..36f905b371e6 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -327,8 +327,8 @@ static void oxfw_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&oxfw->dwork);
 
 	if (oxfw->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(oxfw->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(oxfw->card);
 	} else {
 		/* Don't forget this case. */
 		oxfw_free(oxfw);
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index 53f20153ba71..6f7aaa8c84aa 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -212,8 +212,8 @@ static void snd_tscm_remove(struct fw_unit *unit)
 	cancel_delayed_work_sync(&tscm->dwork);
 
 	if (tscm->registered) {
-		/* No need to wait for releasing card object in this context. */
-		snd_card_free_when_closed(tscm->card);
+		// Block till all of ALSA character devices are released.
+		snd_card_free(tscm->card);
 	} else {
 		/* Don't forget this case. */
 		tscm_free(tscm);
-- 
2.19.0

[PATCH 2/4] ALSA: firewire: release reference count of firewire unit in .remove callback of bus driver

[Takashi Sakamoto]

In a previous commit, drivers in ALSA firewire stack blocks .remove
callback of bus driver. This enables to release members of private
data in the callback after releasing device of sound card.

This commit simplifies codes to release the members.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c         | 9 +++------
 sound/firewire/dice/dice.c           | 9 +++------
 sound/firewire/digi00x/digi00x.c     | 9 +++------
 sound/firewire/fireface/ff.c         | 9 +++------
 sound/firewire/fireworks/fireworks.c | 9 +++------
 sound/firewire/isight.c              | 5 +++--
 sound/firewire/motu/motu.c           | 9 +++------
 sound/firewire/oxfw/oxfw.c           | 9 +++------
 sound/firewire/tascam/tascam.c       | 9 +++------
 9 files changed, 27 insertions(+), 50 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 3a5579cb3aa8..34ed8afbb30c 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -129,9 +129,6 @@ name_device(struct snd_bebob *bebob)
 static void bebob_free(struct snd_bebob *bebob)
 {
 	snd_bebob_stream_destroy_duplex(bebob);
-
-	mutex_destroy(&bebob->mutex);
-	fw_unit_put(bebob->unit);
 }
 
 /*
@@ -376,10 +373,10 @@ static void bebob_remove(struct fw_unit *unit)
 	if (bebob->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(bebob->card);
-	} else {
-		/* Don't forget this case. */
-		bebob_free(bebob);
 	}
+
+	mutex_destroy(&bebob->mutex);
+	fw_unit_put(bebob->unit);
 }
 
 static const struct snd_bebob_rate_spec normal_rate_spec = {
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 9bf77adb3127..c6b63e3f36a8 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -126,9 +126,6 @@ static void dice_free(struct snd_dice *dice)
 {
 	snd_dice_stream_destroy_duplex(dice);
 	snd_dice_transaction_destroy(dice);
-
-	mutex_destroy(&dice->mutex);
-	fw_unit_put(dice->unit);
 }
 
 /*
@@ -261,10 +258,10 @@ static void dice_remove(struct fw_unit *unit)
 	if (dice->registered) {
 		/* No need to wait for releasing card object in this context. */
 		snd_card_free_when_closed(dice->card);
-	} else {
-		/* Don't forget this case. */
-		dice_free(dice);
 	}
+
+	mutex_destroy(&dice->mutex);
+	fw_unit_put(dice->unit);
 }
 
 static void dice_bus_reset(struct fw_unit *unit)
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 554d7ff737a2..7a24348968b9 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -45,9 +45,6 @@ static void dg00x_free(struct snd_dg00x *dg00x)
 {
 	snd_dg00x_stream_destroy_duplex(dg00x);
 	snd_dg00x_transaction_unregister(dg00x);
-
-	mutex_destroy(&dg00x->mutex);
-	fw_unit_put(dg00x->unit);
 }
 
 static void dg00x_card_free(struct snd_card *card)
@@ -174,10 +171,10 @@ static void snd_dg00x_remove(struct fw_unit *unit)
 	if (dg00x->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(dg00x->card);
-	} else {
-		/* Don't forget this case. */
-		dg00x_free(dg00x);
 	}
+
+	mutex_destroy(&dg00x->mutex);
+	fw_unit_put(dg00x->unit);
 }
 
 static const struct ieee1394_device_id snd_dg00x_id_table[] = {
diff --git a/sound/firewire/fireface/ff.c b/sound/firewire/fireface/ff.c
index 73425dfe63bf..37866beeb160 100644
--- a/sound/firewire/fireface/ff.c
+++ b/sound/firewire/fireface/ff.c
@@ -31,9 +31,6 @@ static void ff_free(struct snd_ff *ff)
 {
 	snd_ff_stream_destroy_duplex(ff);
 	snd_ff_transaction_unregister(ff);
-
-	mutex_destroy(&ff->mutex);
-	fw_unit_put(ff->unit);
 }
 
 static void ff_card_free(struct snd_card *card)
@@ -147,10 +144,10 @@ static void snd_ff_remove(struct fw_unit *unit)
 	if (ff->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(ff->card);
-	} else {
-		/* Don't forget this case. */
-		ff_free(ff);
 	}
+
+	mutex_destroy(&ff->mutex);
+	fw_unit_put(ff->unit);
 }
 
 static const struct snd_ff_spec spec_ff400 = {
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index 5a17ead86e61..5854d2a87a18 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -188,9 +188,6 @@ static void efw_free(struct snd_efw *efw)
 {
 	snd_efw_stream_destroy_duplex(efw);
 	snd_efw_transaction_remove_instance(efw);
-
-	mutex_destroy(&efw->mutex);
-	fw_unit_put(efw->unit);
 }
 
 /*
@@ -360,10 +357,10 @@ static void efw_remove(struct fw_unit *unit)
 	if (efw->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(efw->card);
-	} else {
-		/* Don't forget this case. */
-		efw_free(efw);
 	}
+
+	mutex_destroy(&efw->mutex);
+	fw_unit_put(efw->unit);
 }
 
 static const struct ieee1394_device_id efw_id_table[] = {
diff --git a/sound/firewire/isight.c b/sound/firewire/isight.c
index 1f591c8805ea..de4decfb74d5 100644
--- a/sound/firewire/isight.c
+++ b/sound/firewire/isight.c
@@ -602,8 +602,6 @@ static void isight_card_free(struct snd_card *card)
 	struct isight *isight = card->private_data;
 
 	fw_iso_resources_destroy(&isight->resources);
-	fw_unit_put(isight->unit);
-	mutex_destroy(&isight->mutex);
 }
 
 static u64 get_unit_base(struct fw_unit *unit)
@@ -705,6 +703,9 @@ static void isight_remove(struct fw_unit *unit)
 
 	// Block till all of ALSA character devices are released.
 	snd_card_free(isight->card);
+
+	mutex_destroy(&isight->mutex);
+	fw_unit_put(isight->unit);
 }
 
 static const struct ieee1394_device_id isight_id_table[] = {
diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c
index 12680c85b37f..281028ee3273 100644
--- a/sound/firewire/motu/motu.c
+++ b/sound/firewire/motu/motu.c
@@ -57,9 +57,6 @@ static void motu_free(struct snd_motu *motu)
 	snd_motu_transaction_unregister(motu);
 
 	snd_motu_stream_destroy_duplex(motu);
-
-	mutex_destroy(&motu->mutex);
-	fw_unit_put(motu->unit);
 }
 
 /*
@@ -174,10 +171,10 @@ static void motu_remove(struct fw_unit *unit)
 	if (motu->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(motu->card);
-	} else {
-		/* Don't forget this case. */
-		motu_free(motu);
 	}
+
+	mutex_destroy(&motu->mutex);
+	fw_unit_put(motu->unit);
 }
 
 static void motu_bus_update(struct fw_unit *unit)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 36f905b371e6..14fe02a9ed5d 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -118,9 +118,6 @@ static void oxfw_free(struct snd_oxfw *oxfw)
 	snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream);
 	if (oxfw->has_output)
 		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
-
-	mutex_destroy(&oxfw->mutex);
-	fw_unit_put(oxfw->unit);
 }
 
 /*
@@ -329,10 +326,10 @@ static void oxfw_remove(struct fw_unit *unit)
 	if (oxfw->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(oxfw->card);
-	} else {
-		/* Don't forget this case. */
-		oxfw_free(oxfw);
 	}
+
+	mutex_destroy(&oxfw->mutex);
+	fw_unit_put(oxfw->unit);
 }
 
 static const struct compat_info griffin_firewave = {
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index 6f7aaa8c84aa..f4f959128341 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -89,9 +89,6 @@ static void tscm_free(struct snd_tscm *tscm)
 {
 	snd_tscm_transaction_unregister(tscm);
 	snd_tscm_stream_destroy_duplex(tscm);
-
-	mutex_destroy(&tscm->mutex);
-	fw_unit_put(tscm->unit);
 }
 
 static void tscm_card_free(struct snd_card *card)
@@ -214,10 +211,10 @@ static void snd_tscm_remove(struct fw_unit *unit)
 	if (tscm->registered) {
 		// Block till all of ALSA character devices are released.
 		snd_card_free(tscm->card);
-	} else {
-		/* Don't forget this case. */
-		tscm_free(tscm);
 	}
+
+	mutex_destroy(&tscm->mutex);
+	fw_unit_put(tscm->unit);
 }
 
 static const struct ieee1394_device_id snd_tscm_id_table[] = {
-- 
2.19.0

[PATCH 3/4] ALSA: bebob/fireworks: simplify handling of local device entry table

[Takashi Sakamoto]

In drivers of ALSA firewire stack, bebob and fireworks drivers have
local device entry table. At present, critical section to operate the
table is from the beginning/end of 'do_registration' call. This can be
more narrow and simplify codes.

This commit applies small refactoring for the above purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c         | 17 ++++++-----------
 sound/firewire/fireworks/fireworks.c | 21 +++++++--------------
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 34ed8afbb30c..3bc68499974a 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -128,6 +128,10 @@ name_device(struct snd_bebob *bebob)
 
 static void bebob_free(struct snd_bebob *bebob)
 {
+	mutex_lock(&devices_mutex);
+	clear_bit(bebob->card_index, devices_used);
+	mutex_unlock(&devices_mutex);
+
 	snd_bebob_stream_destroy_duplex(bebob);
 }
 
@@ -140,12 +144,6 @@ static void bebob_free(struct snd_bebob *bebob)
 static void
 bebob_card_free(struct snd_card *card)
 {
-	struct snd_bebob *bebob = card->private_data;
-
-	mutex_lock(&devices_mutex);
-	clear_bit(bebob->card_index, devices_used);
-	mutex_unlock(&devices_mutex);
-
 	bebob_free(card->private_data);
 }
 
@@ -186,7 +184,6 @@ do_registration(struct work_struct *work)
 		return;
 
 	mutex_lock(&devices_mutex);
-
 	for (card_index = 0; card_index < SNDRV_CARDS; card_index++) {
 		if (!test_bit(card_index, devices_used) && enable[card_index])
 			break;
@@ -202,6 +199,8 @@ do_registration(struct work_struct *work)
 		mutex_unlock(&devices_mutex);
 		return;
 	}
+	set_bit(card_index, devices_used);
+	mutex_unlock(&devices_mutex);
 
 	err = name_device(bebob);
 	if (err < 0)
@@ -242,9 +241,6 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	set_bit(card_index, devices_used);
-	mutex_unlock(&devices_mutex);
-
 	/*
 	 * After registered, bebob instance can be released corresponding to
 	 * releasing the sound card instance.
@@ -255,7 +251,6 @@ do_registration(struct work_struct *work)
 
 	return;
 error:
-	mutex_unlock(&devices_mutex);
 	snd_bebob_stream_destroy_duplex(bebob);
 	snd_card_free(bebob->card);
 	dev_info(&bebob->unit->device,
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index 5854d2a87a18..da0c31033821 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -186,6 +186,10 @@ get_hardware_info(struct snd_efw *efw)
 
 static void efw_free(struct snd_efw *efw)
 {
+	mutex_lock(&devices_mutex);
+	clear_bit(efw->card_index, devices_used);
+	mutex_unlock(&devices_mutex);
+
 	snd_efw_stream_destroy_duplex(efw);
 	snd_efw_transaction_remove_instance(efw);
 }
@@ -199,14 +203,6 @@ static void efw_free(struct snd_efw *efw)
 static void
 efw_card_free(struct snd_card *card)
 {
-	struct snd_efw *efw = card->private_data;
-
-	if (efw->card_index >= 0) {
-		mutex_lock(&devices_mutex);
-		clear_bit(efw->card_index, devices_used);
-		mutex_unlock(&devices_mutex);
-	}
-
 	efw_free(card->private_data);
 }
 
@@ -220,9 +216,8 @@ do_registration(struct work_struct *work)
 	if (efw->registered)
 		return;
 
-	mutex_lock(&devices_mutex);
-
 	/* check registered cards */
+	mutex_lock(&devices_mutex);
 	for (card_index = 0; card_index < SNDRV_CARDS; ++card_index) {
 		if (!test_bit(card_index, devices_used) && enable[card_index])
 			break;
@@ -238,6 +233,8 @@ do_registration(struct work_struct *work)
 		mutex_unlock(&devices_mutex);
 		return;
 	}
+	set_bit(card_index, devices_used);
+	mutex_unlock(&devices_mutex);
 
 	/* prepare response buffer */
 	snd_efw_resp_buf_size = clamp(snd_efw_resp_buf_size,
@@ -279,9 +276,6 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	set_bit(card_index, devices_used);
-	mutex_unlock(&devices_mutex);
-
 	/*
 	 * After registered, efw instance can be released corresponding to
 	 * releasing the sound card instance.
@@ -292,7 +286,6 @@ do_registration(struct work_struct *work)
 
 	return;
 error:
-	mutex_unlock(&devices_mutex);
 	snd_efw_transaction_remove_instance(efw);
 	snd_efw_stream_destroy_duplex(efw);
 	snd_card_free(efw->card);
-- 
2.19.0

[PATCH 4/4] ALSA: firewire: simplify cleanup process when failing to register sound card

[Takashi Sakamoto]

In former commits, .private_free callback releases resources just for
data transmission. This release function can be called without the
resources are actually allocated in error paths.

This commit applies a small refactoring to clean up codes in error
paths.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c         | 27 +++++++--------------------
 sound/firewire/dice/dice.c           | 26 +++++---------------------
 sound/firewire/digi00x/digi00x.c     | 15 +++++----------
 sound/firewire/fireface/ff.c         | 15 +++++----------
 sound/firewire/fireworks/fireworks.c | 28 +++++++---------------------
 sound/firewire/motu/motu.c           | 26 +++++---------------------
 sound/firewire/oxfw/oxfw.c           | 26 +++++---------------------
 sound/firewire/tascam/tascam.c       | 19 +++++--------------
 8 files changed, 44 insertions(+), 138 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 3bc68499974a..672d13488454 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -126,8 +126,11 @@ name_device(struct snd_bebob *bebob)
 	return err;
 }
 
-static void bebob_free(struct snd_bebob *bebob)
+static void
+bebob_card_free(struct snd_card *card)
 {
+	struct snd_bebob *bebob = card->private_data;
+
 	mutex_lock(&devices_mutex);
 	clear_bit(bebob->card_index, devices_used);
 	mutex_unlock(&devices_mutex);
@@ -135,18 +138,6 @@ static void bebob_free(struct snd_bebob *bebob)
 	snd_bebob_stream_destroy_duplex(bebob);
 }
 
-/*
- * This module releases the FireWire unit data after all ALSA character devices
- * are released by applications. This is for releasing stream data or finishing
- * transactions safely. Thus at returning from .remove(), this module still keep
- * references for the unit.
- */
-static void
-bebob_card_free(struct snd_card *card)
-{
-	bebob_free(card->private_data);
-}
-
 static const struct snd_bebob_spec *
 get_saffire_spec(struct fw_unit *unit)
 {
@@ -202,6 +193,9 @@ do_registration(struct work_struct *work)
 	set_bit(card_index, devices_used);
 	mutex_unlock(&devices_mutex);
 
+	bebob->card->private_free = bebob_card_free;
+	bebob->card->private_data = bebob;
+
 	err = name_device(bebob);
 	if (err < 0)
 		goto error;
@@ -241,17 +235,10 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, bebob instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	bebob->card->private_free = bebob_card_free;
-	bebob->card->private_data = bebob;
 	bebob->registered = true;
 
 	return;
 error:
-	snd_bebob_stream_destroy_duplex(bebob);
 	snd_card_free(bebob->card);
 	dev_info(&bebob->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index c6b63e3f36a8..0f6dbcffe711 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -122,23 +122,14 @@ static void dice_card_strings(struct snd_dice *dice)
 	strcpy(card->mixername, "DICE");
 }
 
-static void dice_free(struct snd_dice *dice)
+static void dice_card_free(struct snd_card *card)
 {
+	struct snd_dice *dice = card->private_data;
+
 	snd_dice_stream_destroy_duplex(dice);
 	snd_dice_transaction_destroy(dice);
 }
 
-/*
- * This module releases the FireWire unit data after all ALSA character devices
- * are released by applications. This is for releasing stream data or finishing
- * transactions safely. Thus at returning from .remove(), this module still keep
- * references for the unit.
- */
-static void dice_card_free(struct snd_card *card)
-{
-	dice_free(card->private_data);
-}
-
 static void do_registration(struct work_struct *work)
 {
 	struct snd_dice *dice = container_of(work, struct snd_dice, dwork.work);
@@ -151,6 +142,8 @@ static void do_registration(struct work_struct *work)
 			   &dice->card);
 	if (err < 0)
 		return;
+	dice->card->private_free = dice_card_free;
+	dice->card->private_data = dice;
 
 	err = snd_dice_transaction_init(dice);
 	if (err < 0)
@@ -188,19 +181,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, dice instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	dice->card->private_free = dice_card_free;
-	dice->card->private_data = dice;
 	dice->registered = true;
 
 	return;
 error:
-	snd_dice_stream_destroy_duplex(dice);
-	snd_dice_transaction_destroy(dice);
-	snd_dice_stream_destroy_duplex(dice);
 	snd_card_free(dice->card);
 	dev_info(&dice->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 7a24348968b9..6c6ea149ef6b 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -41,17 +41,14 @@ static int name_card(struct snd_dg00x *dg00x)
 	return 0;
 }
 
-static void dg00x_free(struct snd_dg00x *dg00x)
+static void dg00x_card_free(struct snd_card *card)
 {
+	struct snd_dg00x *dg00x = card->private_data;
+
 	snd_dg00x_stream_destroy_duplex(dg00x);
 	snd_dg00x_transaction_unregister(dg00x);
 }
 
-static void dg00x_card_free(struct snd_card *card)
-{
-	dg00x_free(card->private_data);
-}
-
 static void do_registration(struct work_struct *work)
 {
 	struct snd_dg00x *dg00x =
@@ -65,6 +62,8 @@ static void do_registration(struct work_struct *work)
 			   &dg00x->card);
 	if (err < 0)
 		return;
+	dg00x->card->private_free = dg00x_card_free;
+	dg00x->card->private_data = dg00x;
 
 	err = name_card(dg00x);
 	if (err < 0)
@@ -96,14 +95,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	dg00x->card->private_free = dg00x_card_free;
-	dg00x->card->private_data = dg00x;
 	dg00x->registered = true;
 
 	return;
 error:
-	snd_dg00x_transaction_unregister(dg00x);
-	snd_dg00x_stream_destroy_duplex(dg00x);
 	snd_card_free(dg00x->card);
 	dev_info(&dg00x->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/fireface/ff.c b/sound/firewire/fireface/ff.c
index 37866beeb160..3f61cfeace69 100644
--- a/sound/firewire/fireface/ff.c
+++ b/sound/firewire/fireface/ff.c
@@ -27,17 +27,14 @@ static void name_card(struct snd_ff *ff)
 		 dev_name(&ff->unit->device), 100 << fw_dev->max_speed);
 }
 
-static void ff_free(struct snd_ff *ff)
+static void ff_card_free(struct snd_card *card)
 {
+	struct snd_ff *ff = card->private_data;
+
 	snd_ff_stream_destroy_duplex(ff);
 	snd_ff_transaction_unregister(ff);
 }
 
-static void ff_card_free(struct snd_card *card)
-{
-	ff_free(card->private_data);
-}
-
 static void do_registration(struct work_struct *work)
 {
 	struct snd_ff *ff = container_of(work, struct snd_ff, dwork.work);
@@ -50,6 +47,8 @@ static void do_registration(struct work_struct *work)
 			   &ff->card);
 	if (err < 0)
 		return;
+	ff->card->private_free = ff_card_free;
+	ff->card->private_data = ff;
 
 	err = snd_ff_transaction_register(ff);
 	if (err < 0)
@@ -79,14 +78,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	ff->card->private_free = ff_card_free;
-	ff->card->private_data = ff;
 	ff->registered = true;
 
 	return;
 error:
-	snd_ff_transaction_unregister(ff);
-	snd_ff_stream_destroy_duplex(ff);
 	snd_card_free(ff->card);
 	dev_info(&ff->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index da0c31033821..faf0e001c4c5 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -184,8 +184,11 @@ get_hardware_info(struct snd_efw *efw)
 	return err;
 }
 
-static void efw_free(struct snd_efw *efw)
+static void
+efw_card_free(struct snd_card *card)
 {
+	struct snd_efw *efw = card->private_data;
+
 	mutex_lock(&devices_mutex);
 	clear_bit(efw->card_index, devices_used);
 	mutex_unlock(&devices_mutex);
@@ -194,18 +197,6 @@ static void efw_free(struct snd_efw *efw)
 	snd_efw_transaction_remove_instance(efw);
 }
 
-/*
- * This module releases the FireWire unit data after all ALSA character devices
- * are released by applications. This is for releasing stream data or finishing
- * transactions safely. Thus at returning from .remove(), this module still keep
- * references for the unit.
- */
-static void
-efw_card_free(struct snd_card *card)
-{
-	efw_free(card->private_data);
-}
-
 static void
 do_registration(struct work_struct *work)
 {
@@ -236,6 +227,9 @@ do_registration(struct work_struct *work)
 	set_bit(card_index, devices_used);
 	mutex_unlock(&devices_mutex);
 
+	efw->card->private_free = efw_card_free;
+	efw->card->private_data = efw;
+
 	/* prepare response buffer */
 	snd_efw_resp_buf_size = clamp(snd_efw_resp_buf_size,
 				      SND_EFW_RESPONSE_MAXIMUM_BYTES, 4096U);
@@ -276,18 +270,10 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, efw instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	efw->card->private_free = efw_card_free;
-	efw->card->private_data = efw;
 	efw->registered = true;
 
 	return;
 error:
-	snd_efw_transaction_remove_instance(efw);
-	snd_efw_stream_destroy_duplex(efw);
 	snd_card_free(efw->card);
 	dev_info(&efw->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c
index 281028ee3273..220e61926ea4 100644
--- a/sound/firewire/motu/motu.c
+++ b/sound/firewire/motu/motu.c
@@ -52,24 +52,14 @@ static void name_card(struct snd_motu *motu)
 		 dev_name(&motu->unit->device), 100 << fw_dev->max_speed);
 }
 
-static void motu_free(struct snd_motu *motu)
+static void motu_card_free(struct snd_card *card)
 {
-	snd_motu_transaction_unregister(motu);
+	struct snd_motu *motu = card->private_data;
 
+	snd_motu_transaction_unregister(motu);
 	snd_motu_stream_destroy_duplex(motu);
 }
 
-/*
- * This module releases the FireWire unit data after all ALSA character devices
- * are released by applications. This is for releasing stream data or finishing
- * transactions safely. Thus at returning from .remove(), this module still keep
- * references for the unit.
- */
-static void motu_card_free(struct snd_card *card)
-{
-	motu_free(card->private_data);
-}
-
 static void do_registration(struct work_struct *work)
 {
 	struct snd_motu *motu = container_of(work, struct snd_motu, dwork.work);
@@ -82,6 +72,8 @@ static void do_registration(struct work_struct *work)
 			   &motu->card);
 	if (err < 0)
 		return;
+	motu->card->private_free = motu_card_free;
+	motu->card->private_data = motu;
 
 	name_card(motu);
 
@@ -116,18 +108,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, motu instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	motu->card->private_free = motu_card_free;
-	motu->card->private_data = motu;
 	motu->registered = true;
 
 	return;
 error:
-	snd_motu_transaction_unregister(motu);
-	snd_motu_stream_destroy_duplex(motu);
 	snd_card_free(motu->card);
 	dev_info(&motu->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 14fe02a9ed5d..afb78d90384b 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -113,24 +113,15 @@ static int name_card(struct snd_oxfw *oxfw)
 	return err;
 }
 
-static void oxfw_free(struct snd_oxfw *oxfw)
+static void oxfw_card_free(struct snd_card *card)
 {
+	struct snd_oxfw *oxfw = card->private_data;
+
 	snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream);
 	if (oxfw->has_output)
 		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
 }
 
-/*
- * This module releases the FireWire unit data after all ALSA character devices
- * are released by applications. This is for releasing stream data or finishing
- * transactions safely. Thus at returning from .remove(), this module still keep
- * references for the unit.
- */
-static void oxfw_card_free(struct snd_card *card)
-{
-	oxfw_free(card->private_data);
-}
-
 static int detect_quirks(struct snd_oxfw *oxfw)
 {
 	struct fw_device *fw_dev = fw_parent_device(oxfw->unit);
@@ -204,6 +195,8 @@ static void do_registration(struct work_struct *work)
 			   &oxfw->card);
 	if (err < 0)
 		return;
+	oxfw->card->private_free = oxfw_card_free;
+	oxfw->card->private_data = oxfw;
 
 	err = name_card(oxfw);
 	if (err < 0)
@@ -244,19 +237,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, oxfw instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	oxfw->card->private_free = oxfw_card_free;
-	oxfw->card->private_data = oxfw;
 	oxfw->registered = true;
 
 	return;
 error:
-	snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream);
-	if (oxfw->has_output)
-		snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream);
 	snd_card_free(oxfw->card);
 	dev_info(&oxfw->unit->device,
 		 "Sound card registration failed: %d\n", err);
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index f4f959128341..ef57fa4db323 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -85,17 +85,14 @@ static int identify_model(struct snd_tscm *tscm)
 	return 0;
 }
 
-static void tscm_free(struct snd_tscm *tscm)
+static void tscm_card_free(struct snd_card *card)
 {
+	struct snd_tscm *tscm = card->private_data;
+
 	snd_tscm_transaction_unregister(tscm);
 	snd_tscm_stream_destroy_duplex(tscm);
 }
 
-static void tscm_card_free(struct snd_card *card)
-{
-	tscm_free(card->private_data);
-}
-
 static void do_registration(struct work_struct *work)
 {
 	struct snd_tscm *tscm = container_of(work, struct snd_tscm, dwork.work);
@@ -105,6 +102,8 @@ static void do_registration(struct work_struct *work)
 			   &tscm->card);
 	if (err < 0)
 		return;
+	tscm->card->private_free = tscm_card_free;
+	tscm->card->private_data = tscm;
 
 	err = identify_model(tscm);
 	if (err < 0)
@@ -136,18 +135,10 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	/*
-	 * After registered, tscm instance can be released corresponding to
-	 * releasing the sound card instance.
-	 */
-	tscm->card->private_free = tscm_card_free;
-	tscm->card->private_data = tscm;
 	tscm->registered = true;
 
 	return;
 error:
-	snd_tscm_transaction_unregister(tscm);
-	snd_tscm_stream_destroy_duplex(tscm);
 	snd_card_free(tscm->card);
 	dev_info(&tscm->unit->device,
 		 "Sound card registration failed: %d\n", err);
-- 
2.19.0

Re: [PATCH 0/4] ALSA: firewire: block .remove callback of bus

[Takashi Iwai]

On Wed, 10 Oct 2018 08:34:58 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> In a discussion for devres support[1], I realize difference of unbind
> behaviour of drivers in ALSA firewire stack and in the others. For
> consistency behaviour inner the same subsystem for users, it's better
> to imitate the behaviour.
> 
> Additionally, blocking .remove function simplifies codes to releasing
> device.
> 
> This commit uses 'snd_card_free()' instead of
> 'snd_card_free_when_closed()' in .remove function and performs
> refactoring for release codes.

Would the hot-unplug behavior change with this patch?

For most of other drivers (but for USB), the actual hot-plug/unplug
are unlikely scenario, hence blocking makes sense to assure the safety
unbind.  (Yes, there is also PCI hotplug, but an audio device is
rarely used there.)

But, FireWire is a beast to be plugged / unplugged more often, so the
hot-unplug behavior change is more important to users.


thanks,

Takashi

> 
> [1] http://mailman.alsa-project.org/pipermail/alsa-devel/2018-September/140431.html
> 
> Takashi Sakamoto (4):
>   ALSA: firewire: block .remove callback of bus driver till all of ALSA
>     character devices are released
>   ALSA: firewire: release reference count of firewire unit in .remove
>     callback of bus driver
>   ALSA: bebob/fireworks: simplify handling of local device entry table
>   ALSA: firewire: simplify cleanup process when failing to register
>     sound card
> 
>  sound/firewire/bebob/bebob.c         | 43 ++++++---------------
>  sound/firewire/dice/dice.c           | 35 ++++-------------
>  sound/firewire/digi00x/digi00x.c     | 28 +++++---------
>  sound/firewire/fireface/ff.c         | 28 +++++---------
>  sound/firewire/fireworks/fireworks.c | 56 ++++++++--------------------
>  sound/firewire/isight.c              |  8 ++--
>  sound/firewire/motu/motu.c           | 39 +++++--------------
>  sound/firewire/oxfw/oxfw.c           | 39 +++++--------------
>  sound/firewire/tascam/tascam.c       | 32 +++++-----------
>  9 files changed, 90 insertions(+), 218 deletions(-)
> 
> -- 
> 2.19.0
> 

Re: [PATCH 0/4] ALSA: firewire: block .remove callback of bus

[Takashi Sakamoto]

Hi,

On Oct 10 2018 16:04, Takashi Iwai wrote:
> On Wed, 10 Oct 2018 08:34:58 +0200,
> Takashi Sakamoto wrote:
>> In a discussion for devres support[1], I realize difference of unbind
>> behaviour of drivers in ALSA firewire stack and in the others. For
>> consistency behaviour inner the same subsystem for users, it's better
>> to imitate the behaviour.
>>
>> Additionally, blocking .remove function simplifies codes to releasing
>> device.
>>
>> This commit uses 'snd_card_free()' instead of
>> 'snd_card_free_when_closed()' in .remove function and performs
>> refactoring for release codes.
> 
> Would the hot-unplug behavior change with this patch?
> 
> For most of other drivers (but for USB), the actual hot-plug/unplug
> are unlikely scenario, hence blocking makes sense to assure the safety
> unbind.  (Yes, there is also PCI hotplug, but an audio device is
> rarely used there.)
> 
> But, FireWire is a beast to be plugged / unplugged more often, so the
> hot-unplug behavior change is more important to users.

In a point of user experience, a slight change there is. From ether bus
driver or sysfs nodes, unbind operation is successful. But it's blocked
till all ALSA character devices are released. The user use 'unbind'
sysfs node, command execution waits. In typical use cases of
desktop environment, it waits that pulseaudio releases the last ALSA
control character device.

In bus driver for IEEE 1394 bus, a call of .remove in device driver is
done in workqueue. This bus operations runs without blocking.

If unbind operation is blocked in command line, it means that any
program runs without no care of state of ALSA character devices. For
example, current implementation of alsactl has this bug in its monitor
mode[1]. The blocking state reminds users of this kind of bug of any
applications.

Anyway, hot-plugging/unplugging are still available.

Rather than the slight change, this patchset performs refactoring codes
for releasing devices. This removes complications of current code and
simplify error paths.

[1] 
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-September/140580.html

Thanks

Takashi Sakamoto

Re: [PATCH 0/4] ALSA: firewire: block .remove callback of bus

[Takashi Iwai]

On Wed, 10 Oct 2018 10:14:12 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On Oct 10 2018 16:04, Takashi Iwai wrote:
> > On Wed, 10 Oct 2018 08:34:58 +0200,
> > Takashi Sakamoto wrote:
> >> In a discussion for devres support[1], I realize difference of unbind
> >> behaviour of drivers in ALSA firewire stack and in the others. For
> >> consistency behaviour inner the same subsystem for users, it's better
> >> to imitate the behaviour.
> >>
> >> Additionally, blocking .remove function simplifies codes to releasing
> >> device.
> >>
> >> This commit uses 'snd_card_free()' instead of
> >> 'snd_card_free_when_closed()' in .remove function and performs
> >> refactoring for release codes.
> >
> > Would the hot-unplug behavior change with this patch?
> >
> > For most of other drivers (but for USB), the actual hot-plug/unplug
> > are unlikely scenario, hence blocking makes sense to assure the safety
> > unbind.  (Yes, there is also PCI hotplug, but an audio device is
> > rarely used there.)
> >
> > But, FireWire is a beast to be plugged / unplugged more often, so the
> > hot-unplug behavior change is more important to users.
> 
> In a point of user experience, a slight change there is. From ether bus
> driver or sysfs nodes, unbind operation is successful. But it's blocked
> till all ALSA character devices are released. The user use 'unbind'
> sysfs node, command execution waits. In typical use cases of
> desktop environment, it waits that pulseaudio releases the last ALSA
> control character device.
> 
> In bus driver for IEEE 1394 bus, a call of .remove in device driver is
> done in workqueue. This bus operations runs without blocking.

OK, then that's fine.  The blocking in unbind can be considered rather
safer than the device hog-unplug, and the behavior change is justified
by that.

> If unbind operation is blocked in command line, it means that any
> program runs without no care of state of ALSA character devices. For
> example, current implementation of alsactl has this bug in its monitor
> mode[1]. The blocking state reminds users of this kind of bug of any
> applications.
> 
> Anyway, hot-plugging/unplugging are still available.
> 
> Rather than the slight change, this patchset performs refactoring codes
> for releasing devices. This removes complications of current code and
> simplify error paths.

Yes, that's a good bonus.  I'll read through your patches.


thanks,

Takashi

Re: [PATCH 0/4] ALSA: firewire: block .remove callback of bus

[Takashi Iwai]

On Wed, 10 Oct 2018 10:25:30 +0200,
Takashi Iwai wrote:
> 
> On Wed, 10 Oct 2018 10:14:12 +0200,
> Takashi Sakamoto wrote:
> > 
> > Hi,
> > 
> > On Oct 10 2018 16:04, Takashi Iwai wrote:
> > > On Wed, 10 Oct 2018 08:34:58 +0200,
> > > Takashi Sakamoto wrote:
> > >> In a discussion for devres support[1], I realize difference of unbind
> > >> behaviour of drivers in ALSA firewire stack and in the others. For
> > >> consistency behaviour inner the same subsystem for users, it's better
> > >> to imitate the behaviour.
> > >>
> > >> Additionally, blocking .remove function simplifies codes to releasing
> > >> device.
> > >>
> > >> This commit uses 'snd_card_free()' instead of
> > >> 'snd_card_free_when_closed()' in .remove function and performs
> > >> refactoring for release codes.
> > >
> > > Would the hot-unplug behavior change with this patch?
> > >
> > > For most of other drivers (but for USB), the actual hot-plug/unplug
> > > are unlikely scenario, hence blocking makes sense to assure the safety
> > > unbind.  (Yes, there is also PCI hotplug, but an audio device is
> > > rarely used there.)
> > >
> > > But, FireWire is a beast to be plugged / unplugged more often, so the
> > > hot-unplug behavior change is more important to users.
> > 
> > In a point of user experience, a slight change there is. From ether bus
> > driver or sysfs nodes, unbind operation is successful. But it's blocked
> > till all ALSA character devices are released. The user use 'unbind'
> > sysfs node, command execution waits. In typical use cases of
> > desktop environment, it waits that pulseaudio releases the last ALSA
> > control character device.
> > 
> > In bus driver for IEEE 1394 bus, a call of .remove in device driver is
> > done in workqueue. This bus operations runs without blocking.
> 
> OK, then that's fine.  The blocking in unbind can be considered rather
> safer than the device hog-unplug, and the behavior change is justified
> by that.
> 
> > If unbind operation is blocked in command line, it means that any
> > program runs without no care of state of ALSA character devices. For
> > example, current implementation of alsactl has this bug in its monitor
> > mode[1]. The blocking state reminds users of this kind of bug of any
> > applications.
> > 
> > Anyway, hot-plugging/unplugging are still available.
> > 
> > Rather than the slight change, this patchset performs refactoring codes
> > for releasing devices. This removes complications of current code and
> > simplify error paths.
> 
> Yes, that's a good bonus.  I'll read through your patches.

... and applied all four patches now.  Thanks!


Takashi