All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] ALSA: firewire: cease from delayed card registration
@ 2021-06-07  8:12 Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 1/9] ALSA: bebob: " Takashi Sakamoto
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

Hi,

Since Linux kernel v4.7, delayed card registration was introduced to all
drivers in ALSA firewire stack. Nowadays it brings less benefit than
code complication.

This patchset ceases from it.

Takashi Sakamoto (9):
  ALSA: bebob: cease from delayed card registration
  ALSA: fireworks: cease from delayed card registration
  ALSA: oxfw: cease from delayed card registration
  ALSA: dice: cease from delayed card registration
  ALSA: firewire-digi00x: cease from delayed card registration
  ALSA: firewire-tascam: cease from delayed card registration
  ALSA: firewire-motu: cease from delayed card registration
  ALSA: fireface: cease from delayed card registration
  ALSA: firewire-lib: delete unused kernel API

 sound/firewire/bebob/bebob.c         | 159 ++++++++++-----------------
 sound/firewire/bebob/bebob.h         |   4 -
 sound/firewire/dice/dice.c           | 138 ++++++++---------------
 sound/firewire/dice/dice.h           |   4 -
 sound/firewire/digi00x/digi00x.c     | 101 +++++------------
 sound/firewire/digi00x/digi00x.h     |   3 -
 sound/firewire/fireface/ff.c         |  90 +++++----------
 sound/firewire/fireface/ff.h         |   3 -
 sound/firewire/fireworks/fireworks.c | 107 ++++++------------
 sound/firewire/fireworks/fireworks.h |   3 -
 sound/firewire/lib.c                 |  32 ------
 sound/firewire/lib.h                 |   3 -
 sound/firewire/motu/motu.c           |  84 +++++---------
 sound/firewire/motu/motu.h           |   3 -
 sound/firewire/oxfw/oxfw.c           | 128 ++++++++-------------
 sound/firewire/oxfw/oxfw.h           |   6 +-
 sound/firewire/tascam/tascam.c       |  92 +++++-----------
 sound/firewire/tascam/tascam.h       |   2 -
 18 files changed, 296 insertions(+), 666 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/9] ALSA: bebob: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 2/9] ALSA: fireworks: " Takashi Sakamoto
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob.c | 159 +++++++++++++----------------------
 sound/firewire/bebob/bebob.h |   4 -
 2 files changed, 57 insertions(+), 106 deletions(-)

diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c
index 5938aa325f5e..e7dd112c31c5 100644
--- a/sound/firewire/bebob/bebob.c
+++ b/sound/firewire/bebob/bebob.c
@@ -136,6 +136,9 @@ bebob_card_free(struct snd_card *card)
 	mutex_unlock(&devices_mutex);
 
 	snd_bebob_stream_destroy_duplex(bebob);
+
+	mutex_destroy(&bebob->mutex);
+	fw_unit_put(bebob->unit);
 }
 
 static const struct snd_bebob_spec *
@@ -163,16 +166,30 @@ check_audiophile_booted(struct fw_unit *unit)
 	return strncmp(name, "FW Audiophile Bootloader", 24) != 0;
 }
 
-static void
-do_registration(struct work_struct *work)
+static int bebob_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_bebob *bebob =
-			container_of(work, struct snd_bebob, dwork.work);
 	unsigned int card_index;
+	struct snd_card *card;
+	struct snd_bebob *bebob;
+	const struct snd_bebob_spec *spec;
 	int err;
 
-	if (bebob->registered)
-		return;
+	if (entry->vendor_id == VEN_FOCUSRITE &&
+	    entry->model_id == MODEL_FOCUSRITE_SAFFIRE_BOTH)
+		spec = get_saffire_spec(unit);
+	else if (entry->vendor_id == VEN_MAUDIO1 &&
+		 entry->model_id == MODEL_MAUDIO_AUDIOPHILE_BOTH &&
+		 !check_audiophile_booted(unit))
+		spec = NULL;
+	else
+		spec = (const struct snd_bebob_spec *)entry->driver_data;
+
+	if (spec == NULL) {
+		if (entry->vendor_id == VEN_MAUDIO1 || entry->vendor_id == VEN_MAUDIO2)
+			return snd_bebob_maudio_load_firmware(unit);
+		else
+			return -ENODEV;
+	}
 
 	mutex_lock(&devices_mutex);
 	for (card_index = 0; card_index < SNDRV_CARDS; card_index++) {
@@ -181,27 +198,36 @@ do_registration(struct work_struct *work)
 	}
 	if (card_index >= SNDRV_CARDS) {
 		mutex_unlock(&devices_mutex);
-		return;
+		return -ENOENT;
 	}
 
-	err = snd_card_new(&bebob->unit->device, index[card_index],
-			   id[card_index], THIS_MODULE, 0, &bebob->card);
+	err = snd_card_new(&unit->device, index[card_index], id[card_index], THIS_MODULE,
+			   sizeof(*bebob), &card);
 	if (err < 0) {
 		mutex_unlock(&devices_mutex);
-		return;
+		return err;
 	}
+	card->private_free = bebob_card_free;
 	set_bit(card_index, devices_used);
 	mutex_unlock(&devices_mutex);
 
-	bebob->card->private_free = bebob_card_free;
-	bebob->card->private_data = bebob;
+	bebob = card->private_data;
+	bebob->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, bebob);
+	bebob->card = card;
+	bebob->card_index = card_index;
+
+	bebob->spec = spec;
+	mutex_init(&bebob->mutex);
+	spin_lock_init(&bebob->lock);
+	init_waitqueue_head(&bebob->hwdep_wait);
 
 	err = name_device(bebob);
 	if (err < 0)
 		goto error;
 
 	if (bebob->spec == &maudio_special_spec) {
-		if (bebob->entry->model_id == MODEL_MAUDIO_FW1814)
+		if (entry->model_id == MODEL_MAUDIO_FW1814)
 			err = snd_bebob_maudio_special_discover(bebob, true);
 		else
 			err = snd_bebob_maudio_special_discover(bebob, false);
@@ -214,8 +240,7 @@ do_registration(struct work_struct *work)
 	// M-Audio ProFire Lightbridge has a quirk to transfer packets with discontinuous cycle or
 	// data block counter in early stage of packet streaming. The cycle span from the first
 	// packet with event is variable.
-	if (bebob->entry->vendor_id == VEN_MAUDIO1 &&
-	    bebob->entry->model_id == MODEL_MAUDIO_PROFIRELIGHTBRIDGE)
+	if (entry->vendor_id == VEN_MAUDIO1 && entry->model_id == MODEL_MAUDIO_PROFIRELIGHTBRIDGE)
 		bebob->discontinuity_quirk = true;
 
 	err = snd_bebob_stream_init_duplex(bebob);
@@ -238,80 +263,26 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(bebob->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	bebob->registered = true;
-
-	return;
-error:
-	snd_card_free(bebob->card);
-	dev_info(&bebob->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int
-bebob_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
-{
-	struct snd_bebob *bebob;
-	const struct snd_bebob_spec *spec;
-
-	if (entry->vendor_id == VEN_FOCUSRITE &&
-	    entry->model_id == MODEL_FOCUSRITE_SAFFIRE_BOTH)
-		spec = get_saffire_spec(unit);
-	else if (entry->vendor_id == VEN_MAUDIO1 &&
-		 entry->model_id == MODEL_MAUDIO_AUDIOPHILE_BOTH &&
-		 !check_audiophile_booted(unit))
-		spec = NULL;
-	else
-		spec = (const struct snd_bebob_spec *)entry->driver_data;
-
-	if (spec == NULL) {
-		if (entry->vendor_id == VEN_MAUDIO1 ||
-		    entry->vendor_id == VEN_MAUDIO2)
-			return snd_bebob_maudio_load_firmware(unit);
-		else
-			return -ENODEV;
-	}
-
-	/* Allocate this independent of sound card instance. */
-	bebob = devm_kzalloc(&unit->device, sizeof(struct snd_bebob),
-			     GFP_KERNEL);
-	if (!bebob)
-		return -ENOMEM;
-	bebob->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, bebob);
-
-	bebob->entry = entry;
-	bebob->spec = spec;
-	mutex_init(&bebob->mutex);
-	spin_lock_init(&bebob->lock);
-	init_waitqueue_head(&bebob->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&bebob->dwork, do_registration);
-
-	if (entry->vendor_id != VEN_MAUDIO1 ||
-	    (entry->model_id != MODEL_MAUDIO_FW1814 &&
-	     entry->model_id != MODEL_MAUDIO_PROJECTMIX)) {
-		snd_fw_schedule_registration(unit, &bebob->dwork);
-	} else {
-		/*
-		 * This is a workaround. This bus reset seems to have an effect
-		 * to make devices correctly handling transactions. Without
-		 * this, the devices have gap_count mismatch. This causes much
-		 * failure of transaction.
-		 *
-		 * Just after registration, user-land application receive
-		 * signals from dbus and starts I/Os. To avoid I/Os till the
-		 * future bus reset, registration is done in next update().
-		 */
-		fw_schedule_bus_reset(fw_parent_device(bebob->unit)->card,
-				      false, true);
+	if (entry->vendor_id == VEN_MAUDIO1 &&
+	    (entry->model_id == MODEL_MAUDIO_FW1814 || entry->model_id == MODEL_MAUDIO_PROJECTMIX)) {
+		// This is a workaround. This bus reset seems to have an effect to make devices
+		// correctly handling transactions. Without this, the devices have gap_count
+		// mismatch. This causes much failure of transaction.
+		//
+		// Just after registration, user-land application receive signals from dbus and
+		// starts I/Os. To avoid I/Os till the future bus reset, registration is done in
+		// next update().
+		fw_schedule_bus_reset(fw_parent_device(bebob->unit)->card, false, true);
 	}
 
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 /*
@@ -338,11 +309,7 @@ bebob_update(struct fw_unit *unit)
 	if (bebob == NULL)
 		return;
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!bebob->registered)
-		snd_fw_schedule_registration(unit, &bebob->dwork);
-	else
-		fcp_bus_reset(bebob->unit);
+	fcp_bus_reset(bebob->unit);
 }
 
 static void bebob_remove(struct fw_unit *unit)
@@ -352,20 +319,8 @@ static void bebob_remove(struct fw_unit *unit)
 	if (bebob == NULL)
 		return;
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&bebob->dwork);
-
-	if (bebob->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(bebob->card);
-	}
-
-	mutex_destroy(&bebob->mutex);
-	fw_unit_put(bebob->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(bebob->card);
 }
 
 static const struct snd_bebob_rate_spec normal_rate_spec = {
diff --git a/sound/firewire/bebob/bebob.h b/sound/firewire/bebob/bebob.h
index cba6793bfdb2..edd93699ce1a 100644
--- a/sound/firewire/bebob/bebob.h
+++ b/sound/firewire/bebob/bebob.h
@@ -83,10 +83,6 @@ struct snd_bebob {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
-	const struct ieee1394_device_id *entry;
 	const struct snd_bebob_spec *spec;
 
 	unsigned int midi_input_ports;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/9] ALSA: fireworks: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 1/9] ALSA: bebob: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 3/9] ALSA: oxfw: " Takashi Sakamoto
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireworks/fireworks.c | 107 +++++++++------------------
 sound/firewire/fireworks/fireworks.h |   3 -
 2 files changed, 33 insertions(+), 77 deletions(-)

diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c
index b1cc013a3540..865dac3b37e6 100644
--- a/sound/firewire/fireworks/fireworks.c
+++ b/sound/firewire/fireworks/fireworks.c
@@ -194,19 +194,19 @@ efw_card_free(struct snd_card *card)
 
 	snd_efw_stream_destroy_duplex(efw);
 	snd_efw_transaction_remove_instance(efw);
+
+	mutex_destroy(&efw->mutex);
+	fw_unit_put(efw->unit);
 }
 
-static void
-do_registration(struct work_struct *work)
+static int efw_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_efw *efw = container_of(work, struct snd_efw, dwork.work);
 	unsigned int card_index;
+	struct snd_card *card;
+	struct snd_efw *efw;
 	int err;
 
-	if (efw->registered)
-		return;
-
-	/* check registered cards */
+	// 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])
@@ -214,26 +214,32 @@ do_registration(struct work_struct *work)
 	}
 	if (card_index >= SNDRV_CARDS) {
 		mutex_unlock(&devices_mutex);
-		return;
+		return -ENOENT;
 	}
 
-	err = snd_card_new(&efw->unit->device, index[card_index],
-			   id[card_index], THIS_MODULE, 0, &efw->card);
+	err = snd_card_new(&unit->device, index[card_index], id[card_index], THIS_MODULE,
+			   sizeof(*efw), &card);
 	if (err < 0) {
 		mutex_unlock(&devices_mutex);
-		return;
+		return err;
 	}
+	card->private_free = efw_card_free;
 	set_bit(card_index, devices_used);
 	mutex_unlock(&devices_mutex);
 
-	efw->card->private_free = efw_card_free;
-	efw->card->private_data = efw;
+	efw = card->private_data;
+	efw->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, efw);
+	efw->card = card;
+	efw->card_index = card_index;
+
+	mutex_init(&efw->mutex);
+	spin_lock_init(&efw->lock);
+	init_waitqueue_head(&efw->hwdep_wait);
 
-	/* prepare response buffer */
-	snd_efw_resp_buf_size = clamp(snd_efw_resp_buf_size,
-				      SND_EFW_RESPONSE_MAXIMUM_BYTES, 4096U);
-	efw->resp_buf = devm_kzalloc(&efw->card->card_dev,
-				     snd_efw_resp_buf_size, GFP_KERNEL);
+	// prepare response buffer.
+	snd_efw_resp_buf_size = clamp(snd_efw_resp_buf_size, SND_EFW_RESPONSE_MAXIMUM_BYTES, 4096U);
+	efw->resp_buf = devm_kzalloc(&card->card_dev, snd_efw_resp_buf_size, GFP_KERNEL);
 	if (!efw->resp_buf) {
 		err = -ENOMEM;
 		goto error;
@@ -265,80 +271,33 @@ do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(efw->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	efw->registered = true;
-
-	return;
-error:
-	snd_card_free(efw->card);
-	dev_info(&efw->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int
-efw_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
-{
-	struct snd_efw *efw;
-
-	efw = devm_kzalloc(&unit->device, sizeof(struct snd_efw), GFP_KERNEL);
-	if (efw == NULL)
-		return -ENOMEM;
-	efw->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, efw);
-
-	mutex_init(&efw->mutex);
-	spin_lock_init(&efw->lock);
-	init_waitqueue_head(&efw->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&efw->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &efw->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void efw_update(struct fw_unit *unit)
 {
 	struct snd_efw *efw = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!efw->registered)
-		snd_fw_schedule_registration(unit, &efw->dwork);
-
 	snd_efw_transaction_bus_reset(efw->unit);
 
-	/*
-	 * After registration, userspace can start packet streaming, then this
-	 * code block works fine.
-	 */
-	if (efw->registered) {
-		mutex_lock(&efw->mutex);
-		snd_efw_stream_update_duplex(efw);
-		mutex_unlock(&efw->mutex);
-	}
+	mutex_lock(&efw->mutex);
+	snd_efw_stream_update_duplex(efw);
+	mutex_unlock(&efw->mutex);
 }
 
 static void efw_remove(struct fw_unit *unit)
 {
 	struct snd_efw *efw = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&efw->dwork);
-
-	if (efw->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(efw->card);
-	}
-
-	mutex_destroy(&efw->mutex);
-	fw_unit_put(efw->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(efw->card);
 }
 
 static const struct ieee1394_device_id efw_id_table[] = {
diff --git a/sound/firewire/fireworks/fireworks.h b/sound/firewire/fireworks/fireworks.h
index 49e12cf7c0e3..2c0c7de8b824 100644
--- a/sound/firewire/fireworks/fireworks.h
+++ b/sound/firewire/fireworks/fireworks.h
@@ -65,9 +65,6 @@ struct snd_efw {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	/* for transaction */
 	u32 seqnum;
 	bool resp_addr_changable;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/9] ALSA: oxfw: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 1/9] ALSA: bebob: " Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 2/9] ALSA: fireworks: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 4/9] ALSA: dice: " Takashi Sakamoto
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw.c | 128 +++++++++++++------------------------
 sound/firewire/oxfw/oxfw.h |   6 +-
 2 files changed, 48 insertions(+), 86 deletions(-)

diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index 59bffa32636c..84971d78d152 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -60,7 +60,7 @@ static bool detect_loud_models(struct fw_unit *unit)
 	return match_string(models, ARRAY_SIZE(models), model) >= 0;
 }
 
-static int name_card(struct snd_oxfw *oxfw)
+static int name_card(struct snd_oxfw *oxfw, const struct ieee1394_device_id *entry)
 {
 	struct fw_device *fw_dev = fw_parent_device(oxfw->unit);
 	const struct compat_info *info;
@@ -92,9 +92,8 @@ static int name_card(struct snd_oxfw *oxfw)
 		oxfw->quirks |= SND_OXFW_QUIRK_JUMBO_PAYLOAD;
 
 	/* to apply card definitions */
-	if (oxfw->entry->vendor_id == VENDOR_GRIFFIN ||
-	    oxfw->entry->vendor_id == VENDOR_LACIE) {
-		info = (const struct compat_info *)oxfw->entry->driver_data;
+	if (entry->vendor_id == VENDOR_GRIFFIN || entry->vendor_id == VENDOR_LACIE) {
+		info = (const struct compat_info *)entry->driver_data;
 		d = info->driver_name;
 		v = info->vendor_name;
 		m = info->model_name;
@@ -123,9 +122,12 @@ static void oxfw_card_free(struct snd_card *card)
 
 	if (oxfw->has_output || oxfw->has_input)
 		snd_oxfw_stream_destroy_duplex(oxfw);
+
+	mutex_destroy(&oxfw->mutex);
+	fw_unit_put(oxfw->unit);
 }
 
-static int detect_quirks(struct snd_oxfw *oxfw)
+static int detect_quirks(struct snd_oxfw *oxfw, const struct ieee1394_device_id *entry)
 {
 	struct fw_device *fw_dev = fw_parent_device(oxfw->unit);
 	struct fw_csr_iterator it;
@@ -136,17 +138,18 @@ static int detect_quirks(struct snd_oxfw *oxfw)
 	 * Add ALSA control elements for two models to keep compatibility to
 	 * old firewire-speaker module.
 	 */
-	if (oxfw->entry->vendor_id == VENDOR_GRIFFIN)
+	if (entry->vendor_id == VENDOR_GRIFFIN)
 		return snd_oxfw_add_spkr(oxfw, false);
-	if (oxfw->entry->vendor_id == VENDOR_LACIE)
+	if (entry->vendor_id == VENDOR_LACIE)
 		return snd_oxfw_add_spkr(oxfw, true);
 
 	/*
 	 * Stanton models supports asynchronous transactions for unique MIDI
 	 * messages.
 	 */
-	if (oxfw->entry->vendor_id == OUI_STANTON) {
-		if (oxfw->entry->model_id == MODEL_SCS1M)
+	if (entry->vendor_id == OUI_STANTON) {
+		oxfw->quirks |= SND_OXFW_QUIRK_SCS_TRANSACTION;
+		if (entry->model_id == MODEL_SCS1M)
 			oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION;
 
 		// No physical MIDI ports.
@@ -156,14 +159,14 @@ static int detect_quirks(struct snd_oxfw *oxfw)
 		return snd_oxfw_scs1x_add(oxfw);
 	}
 
-	if (oxfw->entry->vendor_id == OUI_APOGEE && oxfw->entry->model_id == MODEL_DUET_FW)
+	if (entry->vendor_id == OUI_APOGEE && entry->model_id == MODEL_DUET_FW)
 		oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION;
 
 	/*
 	 * TASCAM FireOne has physical control and requires a pair of additional
 	 * MIDI ports.
 	 */
-	if (oxfw->entry->vendor_id == VENDOR_TASCAM) {
+	if (entry->vendor_id == VENDOR_TASCAM) {
 		oxfw->midi_input_ports++;
 		oxfw->midi_output_ports++;
 		return 0;
@@ -189,22 +192,30 @@ static int detect_quirks(struct snd_oxfw *oxfw)
 	return 0;
 }
 
-static void do_registration(struct work_struct *work)
+static int oxfw_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_oxfw *oxfw = container_of(work, struct snd_oxfw, dwork.work);
+	struct snd_card *card;
+	struct snd_oxfw *oxfw;
 	int err;
 
-	if (oxfw->registered)
-		return;
+	if (entry->vendor_id == VENDOR_LOUD && entry->model_id == 0 && !detect_loud_models(unit))
+		return -ENODEV;
 
-	err = snd_card_new(&oxfw->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &oxfw->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*oxfw), &card);
 	if (err < 0)
-		return;
-	oxfw->card->private_free = oxfw_card_free;
-	oxfw->card->private_data = oxfw;
+		return err;
+	card->private_free = oxfw_card_free;
 
-	err = name_card(oxfw);
+	oxfw = card->private_data;
+	oxfw->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, oxfw);
+	oxfw->card = card;
+
+	mutex_init(&oxfw->mutex);
+	spin_lock_init(&oxfw->lock);
+	init_waitqueue_head(&oxfw->hwdep_wait);
+
+	err = name_card(oxfw, entry);
 	if (err < 0)
 		goto error;
 
@@ -212,7 +223,7 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = detect_quirks(oxfw);
+	err = detect_quirks(oxfw, entry);
 	if (err < 0)
 		goto error;
 
@@ -236,85 +247,38 @@ static void do_registration(struct work_struct *work)
 			goto error;
 	}
 
-	err = snd_card_register(oxfw->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	oxfw->registered = true;
-
-	return;
-error:
-	snd_card_free(oxfw->card);
-	dev_info(&oxfw->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int oxfw_probe(struct fw_unit *unit,
-		      const struct ieee1394_device_id *entry)
-{
-	struct snd_oxfw *oxfw;
-
-	if (entry->vendor_id == VENDOR_LOUD && entry->model_id == 0 && !detect_loud_models(unit))
-		return -ENODEV;
-
-	/* Allocate this independent of sound card instance. */
-	oxfw = devm_kzalloc(&unit->device, sizeof(struct snd_oxfw), GFP_KERNEL);
-	if (!oxfw)
-		return -ENOMEM;
-	oxfw->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, oxfw);
-
-	oxfw->entry = entry;
-	mutex_init(&oxfw->mutex);
-	spin_lock_init(&oxfw->lock);
-	init_waitqueue_head(&oxfw->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&oxfw->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &oxfw->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void oxfw_bus_reset(struct fw_unit *unit)
 {
 	struct snd_oxfw *oxfw = dev_get_drvdata(&unit->device);
 
-	if (!oxfw->registered)
-		snd_fw_schedule_registration(unit, &oxfw->dwork);
-
 	fcp_bus_reset(oxfw->unit);
 
-	if (oxfw->registered) {
-		if (oxfw->has_output || oxfw->has_input) {
-			mutex_lock(&oxfw->mutex);
-			snd_oxfw_stream_update_duplex(oxfw);
-			mutex_unlock(&oxfw->mutex);
-		}
-
-		if (oxfw->entry->vendor_id == OUI_STANTON)
-			snd_oxfw_scs1x_update(oxfw);
+	if (oxfw->has_output || oxfw->has_input) {
+		mutex_lock(&oxfw->mutex);
+		snd_oxfw_stream_update_duplex(oxfw);
+		mutex_unlock(&oxfw->mutex);
 	}
+
+	if (oxfw->quirks & SND_OXFW_QUIRK_SCS_TRANSACTION)
+		snd_oxfw_scs1x_update(oxfw);
 }
 
 static void oxfw_remove(struct fw_unit *unit)
 {
 	struct snd_oxfw *oxfw = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&oxfw->dwork);
-
-	if (oxfw->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(oxfw->card);
-	}
-
-	mutex_destroy(&oxfw->mutex);
-	fw_unit_put(oxfw->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(oxfw->card);
 }
 
 static const struct compat_info griffin_firewave = {
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index 853135b5002d..ee47abcb0c90 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -40,6 +40,8 @@ enum snd_oxfw_quirk {
 	SND_OXFW_QUIRK_WRONG_DBS = 0x02,
 	// Blocking transmission mode is used.
 	SND_OXFW_QUIRK_BLOCKING_TRANSMISSION = 0x04,
+	// Stanton SCS1.d and SCS1.m support unique transaction.
+	SND_OXFW_QUIRK_SCS_TRANSACTION = 0x08,
 };
 
 /* This is an arbitrary number for convinience. */
@@ -50,9 +52,6 @@ struct snd_oxfw {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	// The combination of snd_oxfw_quirk enumeration-constants.
 	unsigned int quirks;
 	bool has_output;
@@ -73,7 +72,6 @@ struct snd_oxfw {
 	bool dev_lock_changed;
 	wait_queue_head_t hwdep_wait;
 
-	const struct ieee1394_device_id *entry;
 	void *spec;
 
 	struct amdtp_domain domain;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/9] ALSA: dice: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 3/9] ALSA: oxfw: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 5/9] ALSA: firewire-digi00x: " Takashi Sakamoto
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice.c | 138 +++++++++++++------------------------
 sound/firewire/dice/dice.h |   4 --
 2 files changed, 48 insertions(+), 94 deletions(-)

diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 239d164b0eea..f75902bc8e74 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -135,22 +135,51 @@ static void dice_card_free(struct snd_card *card)
 
 	snd_dice_stream_destroy_duplex(dice);
 	snd_dice_transaction_destroy(dice);
+
+	mutex_destroy(&dice->mutex);
+	fw_unit_put(dice->unit);
 }
 
-static void do_registration(struct work_struct *work)
+static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_dice *dice = container_of(work, struct snd_dice, dwork.work);
+	struct snd_card *card;
+	struct snd_dice *dice;
+	snd_dice_detect_formats_t detect_formats;
 	int err;
 
-	if (dice->registered)
-		return;
+	if (!entry->driver_data && entry->vendor_id != OUI_SSL) {
+		err = check_dice_category(unit);
+		if (err < 0)
+			return -ENODEV;
+	}
 
-	err = snd_card_new(&dice->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &dice->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*dice), &card);
 	if (err < 0)
-		return;
-	dice->card->private_free = dice_card_free;
-	dice->card->private_data = dice;
+		return err;
+	card->private_free = dice_card_free;
+
+	dice = card->private_data;
+	dice->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, dice);
+	dice->card = card;
+
+	if (!entry->driver_data)
+		detect_formats = snd_dice_stream_detect_current_formats;
+	else
+		detect_formats = (snd_dice_detect_formats_t)entry->driver_data;
+
+	// Below models are compliant to IEC 61883-1/6 and have no quirk at high sampling transfer
+	// frequency.
+	// * Avid M-Box 3 Pro
+	// * M-Audio Profire 610
+	// * M-Audio Profire 2626
+	if (entry->vendor_id == OUI_MAUDIO || entry->vendor_id == OUI_AVID)
+		dice->disable_double_pcm_frames = true;
+
+	spin_lock_init(&dice->lock);
+	mutex_init(&dice->mutex);
+	init_completion(&dice->clock_accepted);
+	init_waitqueue_head(&dice->hwdep_wait);
 
 	err = snd_dice_transaction_init(dice);
 	if (err < 0)
@@ -162,7 +191,7 @@ static void do_registration(struct work_struct *work)
 
 	dice_card_strings(dice);
 
-	err = dice->detect_formats(dice);
+	err = detect_formats(dice);
 	if (err < 0)
 		goto error;
 
@@ -184,105 +213,34 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(dice->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	dice->registered = true;
-
-	return;
-error:
-	snd_card_free(dice->card);
-	dev_info(&dice->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int dice_probe(struct fw_unit *unit,
-		      const struct ieee1394_device_id *entry)
-{
-	struct snd_dice *dice;
-	int err;
-
-	if (!entry->driver_data && entry->vendor_id != OUI_SSL) {
-		err = check_dice_category(unit);
-		if (err < 0)
-			return -ENODEV;
-	}
-
-	/* Allocate this independent of sound card instance. */
-	dice = devm_kzalloc(&unit->device, sizeof(struct snd_dice), GFP_KERNEL);
-	if (!dice)
-		return -ENOMEM;
-	dice->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, dice);
-
-	if (!entry->driver_data) {
-		dice->detect_formats = snd_dice_stream_detect_current_formats;
-	} else {
-		dice->detect_formats =
-				(snd_dice_detect_formats_t)entry->driver_data;
-	}
-
-	// Below models are compliant to IEC 61883-1/6 and have no quirk at high sampling transfer
-	// frequency.
-	// * Avid M-Box 3 Pro
-	// * M-Audio Profire 610
-	// * M-Audio Profire 2626
-	if (entry->vendor_id == OUI_MAUDIO || entry->vendor_id == OUI_AVID)
-		dice->disable_double_pcm_frames = true;
-
-	spin_lock_init(&dice->lock);
-	mutex_init(&dice->mutex);
-	init_completion(&dice->clock_accepted);
-	init_waitqueue_head(&dice->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&dice->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &dice->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void dice_remove(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&dice->dwork);
-
-	if (dice->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(dice->card);
-	}
-
-	mutex_destroy(&dice->mutex);
-	fw_unit_put(dice->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(dice->card);
 }
 
 static void dice_bus_reset(struct fw_unit *unit)
 {
 	struct snd_dice *dice = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!dice->registered)
-		snd_fw_schedule_registration(unit, &dice->dwork);
-
 	/* The handler address register becomes initialized. */
 	snd_dice_transaction_reinit(dice);
 
-	/*
-	 * After registration, userspace can start packet streaming, then this
-	 * code block works fine.
-	 */
-	if (dice->registered) {
-		mutex_lock(&dice->mutex);
-		snd_dice_stream_update_duplex(dice);
-		mutex_unlock(&dice->mutex);
-	}
+	mutex_lock(&dice->mutex);
+	snd_dice_stream_update_duplex(dice);
+	mutex_unlock(&dice->mutex);
 }
 
 #define DICE_INTERFACE	0x000001
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 3c967d1b3605..fd440cc625f9 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -78,9 +78,6 @@ struct snd_dice {
 	spinlock_t lock;
 	struct mutex mutex;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	/* Offsets for sub-addresses */
 	unsigned int global_offset;
 	unsigned int rx_offset;
@@ -93,7 +90,6 @@ struct snd_dice {
 	unsigned int rx_pcm_chs[MAX_STREAMS][SND_DICE_RATE_MODE_COUNT];
 	unsigned int tx_midi_ports[MAX_STREAMS];
 	unsigned int rx_midi_ports[MAX_STREAMS];
-	snd_dice_detect_formats_t detect_formats;
 
 	struct fw_address_handler notification_handler;
 	int owner_generation;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/9] ALSA: firewire-digi00x: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 4/9] ALSA: dice: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 6/9] ALSA: firewire-tascam: " Takashi Sakamoto
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x.c | 101 +++++++++----------------------
 sound/firewire/digi00x/digi00x.h |   3 -
 2 files changed, 29 insertions(+), 75 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index ab8408966ec3..995302808c27 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -47,23 +47,32 @@ static void dg00x_card_free(struct snd_card *card)
 
 	snd_dg00x_stream_destroy_duplex(dg00x);
 	snd_dg00x_transaction_unregister(dg00x);
+
+	mutex_destroy(&dg00x->mutex);
+	fw_unit_put(dg00x->unit);
 }
 
-static void do_registration(struct work_struct *work)
+static int snd_dg00x_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_dg00x *dg00x =
-			container_of(work, struct snd_dg00x, dwork.work);
+	struct snd_card *card;
+	struct snd_dg00x *dg00x;
 	int err;
 
-	if (dg00x->registered)
-		return;
-
-	err = snd_card_new(&dg00x->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &dg00x->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*dg00x), &card);
 	if (err < 0)
-		return;
-	dg00x->card->private_free = dg00x_card_free;
-	dg00x->card->private_data = dg00x;
+		return err;
+	card->private_free = dg00x_card_free;
+
+	dg00x = card->private_data;
+	dg00x->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, dg00x);
+	dg00x->card = card;
+
+	mutex_init(&dg00x->mutex);
+	spin_lock_init(&dg00x->lock);
+	init_waitqueue_head(&dg00x->hwdep_wait);
+
+	dg00x->is_console = entry->model_id == MODEL_CONSOLE;
 
 	err = name_card(dg00x);
 	if (err < 0)
@@ -91,85 +100,33 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(dg00x->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	dg00x->registered = true;
-
-	return;
-error:
-	snd_card_free(dg00x->card);
-	dev_info(&dg00x->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int snd_dg00x_probe(struct fw_unit *unit,
-			   const struct ieee1394_device_id *entry)
-{
-	struct snd_dg00x *dg00x;
-
-	/* Allocate this independent of sound card instance. */
-	dg00x = devm_kzalloc(&unit->device, sizeof(struct snd_dg00x),
-			     GFP_KERNEL);
-	if (!dg00x)
-		return -ENOMEM;
-
-	dg00x->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, dg00x);
-
-	mutex_init(&dg00x->mutex);
-	spin_lock_init(&dg00x->lock);
-	init_waitqueue_head(&dg00x->hwdep_wait);
-
-	dg00x->is_console = entry->model_id == MODEL_CONSOLE;
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&dg00x->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &dg00x->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void snd_dg00x_update(struct fw_unit *unit)
 {
 	struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!dg00x->registered)
-		snd_fw_schedule_registration(unit, &dg00x->dwork);
-
 	snd_dg00x_transaction_reregister(dg00x);
 
-	/*
-	 * After registration, userspace can start packet streaming, then this
-	 * code block works fine.
-	 */
-	if (dg00x->registered) {
-		mutex_lock(&dg00x->mutex);
-		snd_dg00x_stream_update_duplex(dg00x);
-		mutex_unlock(&dg00x->mutex);
-	}
+	mutex_lock(&dg00x->mutex);
+	snd_dg00x_stream_update_duplex(dg00x);
+	mutex_unlock(&dg00x->mutex);
 }
 
 static void snd_dg00x_remove(struct fw_unit *unit)
 {
 	struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&dg00x->dwork);
-
-	if (dg00x->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(dg00x->card);
-	}
-
-	mutex_destroy(&dg00x->mutex);
-	fw_unit_put(dg00x->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(dg00x->card);
 }
 
 static const struct ieee1394_device_id snd_dg00x_id_table[] = {
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 129de8edd5ea..82b647d383c5 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -37,9 +37,6 @@ struct snd_dg00x {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	struct amdtp_stream tx_stream;
 	struct fw_iso_resources tx_resources;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 6/9] ALSA: firewire-tascam: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 5/9] ALSA: firewire-digi00x: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 7/9] ALSA: firewire-motu: " Takashi Sakamoto
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/tascam/tascam.c | 92 +++++++++++-----------------------
 sound/firewire/tascam/tascam.h |  2 -
 2 files changed, 28 insertions(+), 66 deletions(-)

diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c
index 75f2edd8e78f..eb58d3fcf087 100644
--- a/sound/firewire/tascam/tascam.c
+++ b/sound/firewire/tascam/tascam.c
@@ -90,19 +90,31 @@ static void tscm_card_free(struct snd_card *card)
 
 	snd_tscm_transaction_unregister(tscm);
 	snd_tscm_stream_destroy_duplex(tscm);
+
+	mutex_destroy(&tscm->mutex);
+	fw_unit_put(tscm->unit);
 }
 
-static void do_registration(struct work_struct *work)
+static int snd_tscm_probe(struct fw_unit *unit,
+			   const struct ieee1394_device_id *entry)
 {
-	struct snd_tscm *tscm = container_of(work, struct snd_tscm, dwork.work);
+	struct snd_card *card;
+	struct snd_tscm *tscm;
 	int err;
 
-	err = snd_card_new(&tscm->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &tscm->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*tscm), &card);
 	if (err < 0)
-		return;
-	tscm->card->private_free = tscm_card_free;
-	tscm->card->private_data = tscm;
+		return err;
+	card->private_free = tscm_card_free;
+
+	tscm = card->private_data;
+	tscm->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, tscm);
+	tscm->card = card;
+
+	mutex_init(&tscm->mutex);
+	spin_lock_init(&tscm->lock);
+	init_waitqueue_head(&tscm->hwdep_wait);
 
 	err = identify_model(tscm);
 	if (err < 0)
@@ -130,81 +142,33 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(tscm->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	tscm->registered = true;
-
-	return;
-error:
-	snd_card_free(tscm->card);
-	dev_info(&tscm->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int snd_tscm_probe(struct fw_unit *unit,
-			   const struct ieee1394_device_id *entry)
-{
-	struct snd_tscm *tscm;
-
-	/* Allocate this independent of sound card instance. */
-	tscm = devm_kzalloc(&unit->device, sizeof(struct snd_tscm), GFP_KERNEL);
-	if (!tscm)
-		return -ENOMEM;
-	tscm->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, tscm);
-
-	mutex_init(&tscm->mutex);
-	spin_lock_init(&tscm->lock);
-	init_waitqueue_head(&tscm->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&tscm->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &tscm->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void snd_tscm_update(struct fw_unit *unit)
 {
 	struct snd_tscm *tscm = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!tscm->registered)
-		snd_fw_schedule_registration(unit, &tscm->dwork);
-
 	snd_tscm_transaction_reregister(tscm);
 
-	/*
-	 * After registration, userspace can start packet streaming, then this
-	 * code block works fine.
-	 */
-	if (tscm->registered) {
-		mutex_lock(&tscm->mutex);
-		snd_tscm_stream_update_duplex(tscm);
-		mutex_unlock(&tscm->mutex);
-	}
+	mutex_lock(&tscm->mutex);
+	snd_tscm_stream_update_duplex(tscm);
+	mutex_unlock(&tscm->mutex);
 }
 
 static void snd_tscm_remove(struct fw_unit *unit)
 {
 	struct snd_tscm *tscm = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&tscm->dwork);
-
-	if (tscm->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(tscm->card);
-	}
-
-	mutex_destroy(&tscm->mutex);
-	fw_unit_put(tscm->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(tscm->card);
 }
 
 static const struct ieee1394_device_id snd_tscm_id_table[] = {
diff --git a/sound/firewire/tascam/tascam.h b/sound/firewire/tascam/tascam.h
index 28dad4eae9c9..d07ffcb27be6 100644
--- a/sound/firewire/tascam/tascam.h
+++ b/sound/firewire/tascam/tascam.h
@@ -70,8 +70,6 @@ struct snd_tscm {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
 	const struct snd_tscm_spec *spec;
 
 	struct fw_iso_resources tx_resources;
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 7/9] ALSA: firewire-motu: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 6/9] ALSA: firewire-tascam: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 8/9] ALSA: fireface: " Takashi Sakamoto
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/motu/motu.c | 84 ++++++++++++--------------------------
 sound/firewire/motu/motu.h |  3 --
 2 files changed, 25 insertions(+), 62 deletions(-)

diff --git a/sound/firewire/motu/motu.c b/sound/firewire/motu/motu.c
index 2a8a6ea2d3f1..531eeb36eb87 100644
--- a/sound/firewire/motu/motu.c
+++ b/sound/firewire/motu/motu.c
@@ -57,22 +57,31 @@ static void motu_card_free(struct snd_card *card)
 
 	snd_motu_transaction_unregister(motu);
 	snd_motu_stream_destroy_duplex(motu);
+
+	mutex_destroy(&motu->mutex);
+	fw_unit_put(motu->unit);
 }
 
-static void do_registration(struct work_struct *work)
+static int motu_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_motu *motu = container_of(work, struct snd_motu, dwork.work);
+	struct snd_card *card;
+	struct snd_motu *motu;
 	int err;
 
-	if (motu->registered)
-		return;
-
-	err = snd_card_new(&motu->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &motu->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*motu), &card);
 	if (err < 0)
-		return;
-	motu->card->private_free = motu_card_free;
-	motu->card->private_data = motu;
+		return err;
+	card->private_free = motu_card_free;
+
+	motu = card->private_data;
+	motu->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, motu);
+	motu->card = card;
+
+	motu->spec = (const struct snd_motu_spec *)entry->driver_data;
+	mutex_init(&motu->mutex);
+	spin_lock_init(&motu->lock);
+	init_waitqueue_head(&motu->hwdep_wait);
 
 	name_card(motu);
 
@@ -103,71 +112,28 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(motu->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	motu->registered = true;
-
-	return;
-error:
-	snd_card_free(motu->card);
-	dev_info(&motu->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int motu_probe(struct fw_unit *unit,
-		      const struct ieee1394_device_id *entry)
-{
-	struct snd_motu *motu;
-
-	/* Allocate this independently of sound card instance. */
-	motu = devm_kzalloc(&unit->device, sizeof(struct snd_motu), GFP_KERNEL);
-	if (!motu)
-		return -ENOMEM;
-	motu->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, motu);
-
-	motu->spec = (const struct snd_motu_spec *)entry->driver_data;
-	mutex_init(&motu->mutex);
-	spin_lock_init(&motu->lock);
-	init_waitqueue_head(&motu->hwdep_wait);
-
-	/* Allocate and register this sound card later. */
-	INIT_DEFERRABLE_WORK(&motu->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &motu->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void motu_remove(struct fw_unit *unit)
 {
 	struct snd_motu *motu = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_delayed_work_sync(&motu->dwork);
-
-	if (motu->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(motu->card);
-	}
-
-	mutex_destroy(&motu->mutex);
-	fw_unit_put(motu->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(motu->card);
 }
 
 static void motu_bus_update(struct fw_unit *unit)
 {
 	struct snd_motu *motu = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!motu->registered)
-		snd_fw_schedule_registration(unit, &motu->dwork);
-
 	/* The handler address register becomes initialized. */
 	snd_motu_transaction_reregister(motu);
 }
diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h
index 674e3dc4e45d..c5c0e446deb2 100644
--- a/sound/firewire/motu/motu.h
+++ b/sound/firewire/motu/motu.h
@@ -54,9 +54,6 @@ struct snd_motu {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	/* Model dependent information. */
 	const struct snd_motu_spec *spec;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 8/9] ALSA: fireface: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (6 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 7/9] ALSA: firewire-motu: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07  8:12 ` [PATCH 9/9] ALSA: firewire-lib: delete unused kernel API Takashi Sakamoto
  2021-06-07 15:15 ` [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Iwai
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

The delayed registration of sound card instance brings less benefit than
complication of kobject management. This commit ceases from it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/fireface/ff.c | 90 +++++++++++-------------------------
 sound/firewire/fireface/ff.h |  3 --
 2 files changed, 28 insertions(+), 65 deletions(-)

diff --git a/sound/firewire/fireface/ff.c b/sound/firewire/fireface/ff.c
index bc39269415d2..7bf51d062021 100644
--- a/sound/firewire/fireface/ff.c
+++ b/sound/firewire/fireface/ff.c
@@ -42,22 +42,33 @@ static void ff_card_free(struct snd_card *card)
 
 	snd_ff_stream_destroy_duplex(ff);
 	snd_ff_transaction_unregister(ff);
+
+	mutex_destroy(&ff->mutex);
+	fw_unit_put(ff->unit);
 }
 
-static void do_registration(struct work_struct *work)
+static int snd_ff_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry)
 {
-	struct snd_ff *ff = container_of(work, struct snd_ff, dwork.work);
+	struct snd_card *card;
+	struct snd_ff *ff;
 	int err;
 
-	if (ff->registered)
-		return;
-
-	err = snd_card_new(&ff->unit->device, -1, NULL, THIS_MODULE, 0,
-			   &ff->card);
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, sizeof(*ff), &card);
 	if (err < 0)
-		return;
-	ff->card->private_free = ff_card_free;
-	ff->card->private_data = ff;
+		return err;
+	card->private_free = ff_card_free;
+
+	ff = card->private_data;
+	ff->unit = fw_unit_get(unit);
+	dev_set_drvdata(&unit->device, ff);
+	ff->card = card;
+
+	mutex_init(&ff->mutex);
+	spin_lock_init(&ff->lock);
+	init_waitqueue_head(&ff->hwdep_wait);
+
+	ff->unit_version = entry->version;
+	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
 
 	err = snd_ff_transaction_register(ff);
 	if (err < 0)
@@ -83,76 +94,31 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = snd_card_register(ff->card);
+	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
 
-	ff->registered = true;
-
-	return;
-error:
-	snd_card_free(ff->card);
-	dev_info(&ff->unit->device,
-		 "Sound card registration failed: %d\n", err);
-}
-
-static int snd_ff_probe(struct fw_unit *unit,
-			   const struct ieee1394_device_id *entry)
-{
-	struct snd_ff *ff;
-
-	ff = devm_kzalloc(&unit->device, sizeof(struct snd_ff), GFP_KERNEL);
-	if (!ff)
-		return -ENOMEM;
-	ff->unit = fw_unit_get(unit);
-	dev_set_drvdata(&unit->device, ff);
-
-	mutex_init(&ff->mutex);
-	spin_lock_init(&ff->lock);
-	init_waitqueue_head(&ff->hwdep_wait);
-
-	ff->unit_version = entry->version;
-	ff->spec = (const struct snd_ff_spec *)entry->driver_data;
-
-	/* Register this sound card later. */
-	INIT_DEFERRABLE_WORK(&ff->dwork, do_registration);
-	snd_fw_schedule_registration(unit, &ff->dwork);
-
 	return 0;
+error:
+	snd_card_free(card);
+	return err;
 }
 
 static void snd_ff_update(struct fw_unit *unit)
 {
 	struct snd_ff *ff = dev_get_drvdata(&unit->device);
 
-	/* Postpone a workqueue for deferred registration. */
-	if (!ff->registered)
-		snd_fw_schedule_registration(unit, &ff->dwork);
-
 	snd_ff_transaction_reregister(ff);
 
-	if (ff->registered)
-		snd_ff_stream_update_duplex(ff);
+	snd_ff_stream_update_duplex(ff);
 }
 
 static void snd_ff_remove(struct fw_unit *unit)
 {
 	struct snd_ff *ff = dev_get_drvdata(&unit->device);
 
-	/*
-	 * Confirm to stop the work for registration before the sound card is
-	 * going to be released. The work is not scheduled again because bus
-	 * reset handler is not called anymore.
-	 */
-	cancel_work_sync(&ff->dwork.work);
-
-	if (ff->registered) {
-		// Block till all of ALSA character devices are released.
-		snd_card_free(ff->card);
-	}
-
-	mutex_destroy(&ff->mutex);
-	fw_unit_put(ff->unit);
+	// Block till all of ALSA character devices are released.
+	snd_card_free(ff->card);
 }
 
 static const struct snd_ff_spec spec_ff800 = {
diff --git a/sound/firewire/fireface/ff.h b/sound/firewire/fireface/ff.h
index 705e7df4f929..0535f0b58b67 100644
--- a/sound/firewire/fireface/ff.h
+++ b/sound/firewire/fireface/ff.h
@@ -69,9 +69,6 @@ struct snd_ff {
 	struct mutex mutex;
 	spinlock_t lock;
 
-	bool registered;
-	struct delayed_work dwork;
-
 	enum snd_ff_unit_version unit_version;
 	const struct snd_ff_spec *spec;
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 9/9] ALSA: firewire-lib: delete unused kernel API
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (7 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 8/9] ALSA: fireface: " Takashi Sakamoto
@ 2021-06-07  8:12 ` Takashi Sakamoto
  2021-06-07 15:15 ` [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Iwai
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Sakamoto @ 2021-06-07  8:12 UTC (permalink / raw)
  To: tiwai; +Cc: alsa-devel, clemens

No driver use snd_fw_schedule_registration(). Let's delete it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/lib.c | 32 --------------------------------
 sound/firewire/lib.h |  3 ---
 2 files changed, 35 deletions(-)

diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c
index 85c4f4477c7f..e0a2337e8f27 100644
--- a/sound/firewire/lib.c
+++ b/sound/firewire/lib.c
@@ -67,38 +67,6 @@ int snd_fw_transaction(struct fw_unit *unit, int tcode,
 }
 EXPORT_SYMBOL(snd_fw_transaction);
 
-#define PROBE_DELAY_MS		(2 * MSEC_PER_SEC)
-
-/**
- * snd_fw_schedule_registration - schedule work for sound card registration
- * @unit: an instance for unit on IEEE 1394 bus
- * @dwork: delayed work with callback function
- *
- * This function is not designed for general purposes. When new unit is
- * connected to IEEE 1394 bus, the bus is under bus-reset state because of
- * topological change. In this state, units tend to fail both of asynchronous
- * and isochronous communication. To avoid this problem, this function is used
- * to postpone sound card registration after the state. The callers must
- * set up instance of delayed work in advance.
- */
-void snd_fw_schedule_registration(struct fw_unit *unit,
-				  struct delayed_work *dwork)
-{
-	u64 now, delay;
-
-	now = get_jiffies_64();
-	delay = fw_parent_device(unit)->card->reset_jiffies
-					+ msecs_to_jiffies(PROBE_DELAY_MS);
-
-	if (time_after64(delay, now))
-		delay -= now;
-	else
-		delay = 0;
-
-	mod_delayed_work(system_wq, dwork, delay);
-}
-EXPORT_SYMBOL(snd_fw_schedule_registration);
-
 MODULE_DESCRIPTION("FireWire audio helper functions");
 MODULE_AUTHOR("Clemens Ladisch <clemens@ladisch.de>");
 MODULE_LICENSE("GPL v2");
diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h
index dc815dc3933e..664dfdb9e58d 100644
--- a/sound/firewire/lib.h
+++ b/sound/firewire/lib.h
@@ -23,7 +23,4 @@ static inline bool rcode_is_permanent_error(int rcode)
 	return rcode == RCODE_TYPE_ERROR || rcode == RCODE_ADDRESS_ERROR;
 }
 
-void snd_fw_schedule_registration(struct fw_unit *unit,
-				  struct delayed_work *dwork);
-
 #endif
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/9] ALSA: firewire: cease from delayed card registration
  2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
                   ` (8 preceding siblings ...)
  2021-06-07  8:12 ` [PATCH 9/9] ALSA: firewire-lib: delete unused kernel API Takashi Sakamoto
@ 2021-06-07 15:15 ` Takashi Iwai
  9 siblings, 0 replies; 11+ messages in thread
From: Takashi Iwai @ 2021-06-07 15:15 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: alsa-devel, clemens

On Mon, 07 Jun 2021 10:12:41 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> Since Linux kernel v4.7, delayed card registration was introduced to all
> drivers in ALSA firewire stack. Nowadays it brings less benefit than
> code complication.
> 
> This patchset ceases from it.
> 
> Takashi Sakamoto (9):
>   ALSA: bebob: cease from delayed card registration
>   ALSA: fireworks: cease from delayed card registration
>   ALSA: oxfw: cease from delayed card registration
>   ALSA: dice: cease from delayed card registration
>   ALSA: firewire-digi00x: cease from delayed card registration
>   ALSA: firewire-tascam: cease from delayed card registration
>   ALSA: firewire-motu: cease from delayed card registration
>   ALSA: fireface: cease from delayed card registration
>   ALSA: firewire-lib: delete unused kernel API

Applied all nine patches.  Thanks.


Takashi

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-06-07 15:16 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-07  8:12 [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 1/9] ALSA: bebob: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 2/9] ALSA: fireworks: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 3/9] ALSA: oxfw: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 4/9] ALSA: dice: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 5/9] ALSA: firewire-digi00x: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 6/9] ALSA: firewire-tascam: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 7/9] ALSA: firewire-motu: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 8/9] ALSA: fireface: " Takashi Sakamoto
2021-06-07  8:12 ` [PATCH 9/9] ALSA: firewire-lib: delete unused kernel API Takashi Sakamoto
2021-06-07 15:15 ` [PATCH 0/9] ALSA: firewire: cease from delayed card registration Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.