[alsa-devel] [PATCH 0/7] ALSA: firewire: apply deleyed registration to all drivers
Hi,
Commit b59fb1900b4fee('ALSA: dice: postpone card registration') applied delayed registration[1] of sound card instance to ALSA dice driver. This idea is also nice for drivers in ALSA firewire stack because in IEEE 1394 bus adding units is corresponding to bus reset and the actual units tend not to work well during bus-reset state.
This patchset adds a new function to firewire-lib module to schedule a delayed work for sound card registration. Currently, the work is commonly scheduled 2 seconds after time of the last bus-reset recorded by Linux firewire core. Then, take all drivers to use it as Dice driver does.
Related to this patchset, I'd like to cancel a discussion about my proposed patchset to remove unpractical module parameters from ALSA bebob/fireworks[2]. I'm sorry for reviewers but my work priority to this patchset is prior to the former one.
[1]: [alsa-devel] [PATCH 0/4 v5] ALSA: dice: improve card registration processing http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/102562.ht...
[2]: [alsa-devel] [PATCH 0/2] ALSA: bebob/fireworks: remove module parameters to reduce maintenance work http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/106219.html
Regards
Takashi Sakamoto (8): ALSA: firewire-lib: suppress kernel warnings when releasing uninitialized stream data ALSA: dice: simplify unit probe processing ALSA: firewire-lib: add new function to schedule a work for sound card registration ALSA: bebob: delayed registration of sound card ALSA: fireworks: delayed registration of sound card ALSA: oxfw: delayed registration of sound card ALSA: firewire-digi00x: delayed registration of sound card ALSA: firewire-tascam: deleyed registration of sound card
sound/firewire/amdtp-stream.c | 4 + sound/firewire/bebob/bebob.c | 215 ++++++++++++++++----------- sound/firewire/bebob/bebob.h | 5 +- sound/firewire/dice/dice.c | 34 +---- sound/firewire/digi00x/digi00x-transaction.c | 7 +- sound/firewire/digi00x/digi00x.c | 107 +++++++++---- sound/firewire/digi00x/digi00x.h | 3 + sound/firewire/fireworks/fireworks.c | 147 ++++++++++++------ sound/firewire/fireworks/fireworks.h | 3 + sound/firewire/lib.c | 32 ++++ sound/firewire/lib.h | 3 + sound/firewire/oxfw/oxfw.c | 151 ++++++++++++------- sound/firewire/oxfw/oxfw.h | 3 + sound/firewire/tascam/tascam.c | 118 ++++++++++----- sound/firewire/tascam/tascam.h | 2 + 15 files changed, 551 insertions(+), 283 deletions(-)
When any of AMDTP stream data are not initialized and private data is going to be released, WARN_ON() in amdtp_stream_destroy() is hit and dump messages. This may take users irritated.
This commit fixes the bug to skip releasing when it's not initialized.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index ed29026..4484242 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -102,6 +102,10 @@ EXPORT_SYMBOL(amdtp_stream_init); */ void amdtp_stream_destroy(struct amdtp_stream *s) { + /* Not initialized. */ + if (s->protocol == NULL) + return; + WARN_ON(amdtp_stream_running(s)); kfree(s->protocol); mutex_destroy(&s->mutex);
In former commit, ALSA dice driver doesn't generate kernel warnings when unplugging units before initializing stream data.
This commit moves the initialization to delayed registration of sound card, to simplify unit probe processing.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/dice/dice.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c index 8b64aef..53ca441 100644 --- a/sound/firewire/dice/dice.c +++ b/sound/firewire/dice/dice.c @@ -201,6 +201,10 @@ static void do_registration(struct work_struct *work)
dice_card_strings(dice);
+ err = snd_dice_stream_init_duplex(dice); + if (err < 0) + goto error; + snd_dice_create_proc(dice);
err = snd_dice_create_pcm(dice); @@ -229,6 +233,7 @@ static void do_registration(struct work_struct *work)
return; error: + snd_dice_stream_destroy_duplex(dice); snd_dice_transaction_destroy(dice); snd_card_free(dice->card); dev_info(&dice->unit->device, @@ -273,12 +278,6 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id) init_completion(&dice->clock_accepted); init_waitqueue_head(&dice->hwdep_wait);
- err = snd_dice_stream_init_duplex(dice); - if (err < 0) { - dice_free(dice); - return err; - } - /* Allocate and register this sound card later. */ INIT_DEFERRABLE_WORK(&dice->dwork, do_registration); schedule_registration(dice);
In former commit, ALSA dice driver postpone sound card registration after IEEE 1394 bus is calm. This idea has advantages for the other drivers.
This commit adds a helper function for it to firewire-lib module. The function is really for the specific purpose. Callers should initialize delayed work structure with callback function.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/dice/dice.c | 23 +++-------------------- sound/firewire/lib.c | 32 ++++++++++++++++++++++++++++++++ sound/firewire/lib.h | 3 +++ 3 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c index 53ca441..96fe68f4 100644 --- a/sound/firewire/dice/dice.c +++ b/sound/firewire/dice/dice.c @@ -20,8 +20,6 @@ MODULE_LICENSE("GPL v2"); #define WEISS_CATEGORY_ID 0x00 #define LOUD_CATEGORY_ID 0x10
-#define PROBE_DELAY_MS (2 * MSEC_PER_SEC) - /* * Some models support several isochronous channels, while these streams are not * always available. In this case, add the model name to this list. @@ -235,27 +233,12 @@ static void do_registration(struct work_struct *work) 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); }
-static void schedule_registration(struct snd_dice *dice) -{ - struct fw_card *fw_card = fw_parent_device(dice->unit)->card; - u64 now, delay; - - now = get_jiffies_64(); - delay = fw_card->reset_jiffies + msecs_to_jiffies(PROBE_DELAY_MS); - - if (time_after64(delay, now)) - delay -= now; - else - delay = 0; - - mod_delayed_work(system_wq, &dice->dwork, delay); -} - static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id) { struct snd_dice *dice; @@ -280,7 +263,7 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
/* Allocate and register this sound card later. */ INIT_DEFERRABLE_WORK(&dice->dwork, do_registration); - schedule_registration(dice); + snd_fw_schedule_registration(unit, &dice->dwork);
return 0; } @@ -311,7 +294,7 @@ static void dice_bus_reset(struct fw_unit *unit)
/* Postpone a workqueue for deferred registration. */ if (!dice->registered) - schedule_registration(dice); + snd_fw_schedule_registration(unit, &dice->dwork);
/* The handler address register becomes initialized. */ snd_dice_transaction_reinit(dice); diff --git a/sound/firewire/lib.c b/sound/firewire/lib.c index f80aafa..ca4dfcf 100644 --- a/sound/firewire/lib.c +++ b/sound/firewire/lib.c @@ -67,6 +67,38 @@ 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); + static void async_midi_port_callback(struct fw_card *card, int rcode, void *data, size_t length, void *callback_data) diff --git a/sound/firewire/lib.h b/sound/firewire/lib.h index f3f6f84..f676931 100644 --- a/sound/firewire/lib.h +++ b/sound/firewire/lib.h @@ -22,6 +22,9 @@ 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); + struct snd_fw_async_midi_port; typedef int (*snd_fw_async_midi_port_fill)( struct snd_rawmidi_substream *substream,
Some bebob based units tends to fail asynchronous communication when IEEE 1394 bus is under bus-reset state. When registering sound card instance at unit probe callback, userspace applications can be involved to the state.
This commit postpones the registration till the bus is calm.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/bebob/bebob.c | 215 +++++++++++++++++++++++++------------------ sound/firewire/bebob/bebob.h | 5 +- 2 files changed, 131 insertions(+), 89 deletions(-)
diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c index 932901d..f7e2cbd 100644 --- a/sound/firewire/bebob/bebob.c +++ b/sound/firewire/bebob/bebob.c @@ -126,6 +126,17 @@ end: return err; }
+static void bebob_free(struct snd_bebob *bebob) +{ + snd_bebob_stream_destroy_duplex(bebob); + fw_unit_put(bebob->unit); + + kfree(bebob->maudio_special_quirk); + + mutex_destroy(&bebob->mutex); + kfree(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 @@ -137,18 +148,11 @@ bebob_card_free(struct snd_card *card) { struct snd_bebob *bebob = card->private_data;
- snd_bebob_stream_destroy_duplex(bebob); - fw_unit_put(bebob->unit); - - kfree(bebob->maudio_special_quirk); - - if (bebob->card_index >= 0) { - mutex_lock(&devices_mutex); - clear_bit(bebob->card_index, devices_used); - mutex_unlock(&devices_mutex); - } + mutex_lock(&devices_mutex); + clear_bit(bebob->card_index, devices_used); + mutex_unlock(&devices_mutex);
- mutex_destroy(&bebob->mutex); + bebob_free(card->private_data); }
static const struct snd_bebob_spec * @@ -176,16 +180,17 @@ check_audiophile_booted(struct fw_unit *unit) return strncmp(name, "FW Audiophile Bootloader", 15) != 0; }
-static int -bebob_probe(struct fw_unit *unit, - const struct ieee1394_device_id *entry) +static void +do_registration(struct work_struct *work) { - struct snd_card *card; - struct snd_bebob *bebob; - const struct snd_bebob_spec *spec; + struct snd_bebob *bebob = + container_of(work, struct snd_bebob, dwork.work); unsigned int card_index; int err;
+ if (bebob->registered) + return; + mutex_lock(&devices_mutex);
for (card_index = 0; card_index < SNDRV_CARDS; card_index++) { @@ -193,64 +198,39 @@ bebob_probe(struct fw_unit *unit, break; } if (card_index >= SNDRV_CARDS) { - err = -ENOENT; - goto end; + mutex_unlock(&devices_mutex); + 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)) - err = snd_bebob_maudio_load_firmware(unit); - else - err = -ENOSYS; - goto end; + err = snd_card_new(&bebob->unit->device, index[card_index], + id[card_index], THIS_MODULE, 0, &bebob->card); + if (err < 0) { + mutex_unlock(&devices_mutex); + return; }
- err = snd_card_new(&unit->device, index[card_index], id[card_index], - THIS_MODULE, sizeof(struct snd_bebob), &card); - if (err < 0) - goto end; - bebob = card->private_data; - bebob->card_index = card_index; - set_bit(card_index, devices_used); - card->private_free = bebob_card_free; - - bebob->card = card; - bebob->unit = fw_unit_get(unit); - 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 ((entry->vendor_id == VEN_MAUDIO1) && - (entry->model_id == MODEL_MAUDIO_FW1814)) - err = snd_bebob_maudio_special_discover(bebob, true); - else if ((entry->vendor_id == VEN_MAUDIO1) && - (entry->model_id == MODEL_MAUDIO_PROJECTMIX)) - err = snd_bebob_maudio_special_discover(bebob, false); - else + if (bebob->spec == &maudio_special_spec) { + if (bebob->entry->model_id == MODEL_MAUDIO_FW1814) + err = snd_bebob_maudio_special_discover(bebob, true); + else + err = snd_bebob_maudio_special_discover(bebob, false); + } else { err = snd_bebob_stream_discover(bebob); + } + if (err < 0) + goto error; + + err = snd_bebob_stream_init_duplex(bebob); if (err < 0) goto error;
snd_bebob_proc_init(bebob);
- if ((bebob->midi_input_ports > 0) || - (bebob->midi_output_ports > 0)) { + if (bebob->midi_input_ports > 0 || bebob->midi_output_ports > 0) { err = snd_bebob_create_midi_devices(bebob); if (err < 0) goto error; @@ -264,16 +244,75 @@ bebob_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_bebob_stream_init_duplex(bebob); + err = snd_card_register(bebob->card); if (err < 0) goto error;
- if (!bebob->maudio_special_quirk) { - err = snd_card_register(card); - if (err < 0) { - snd_bebob_stream_destroy_duplex(bebob); - 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. + */ + bebob->card->private_free = bebob_card_free; + bebob->card->private_data = bebob; + bebob->registered = true; + + return; +error: + mutex_unlock(&devices_mutex); + snd_bebob_stream_destroy_duplex(bebob); + 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 = kzalloc(sizeof(struct snd_bebob), GFP_KERNEL); + if (bebob == NULL) + return -ENOMEM; + + bebob->unit = fw_unit_get(unit); + bebob->entry = entry; + bebob->spec = spec; + dev_set_drvdata(&unit->device, bebob); + + 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 @@ -285,19 +324,11 @@ bebob_probe(struct fw_unit *unit, * signals from dbus and starts I/Os. To avoid I/Os till the * future bus reset, registration is done in next update(). */ - bebob->deferred_registration = true; fw_schedule_bus_reset(fw_parent_device(bebob->unit)->card, false, true); }
- dev_set_drvdata(&unit->device, bebob); -end: - mutex_unlock(&devices_mutex); - return err; -error: - mutex_unlock(&devices_mutex); - snd_card_free(card); - return err; + return 0; }
/* @@ -324,15 +355,11 @@ bebob_update(struct fw_unit *unit) if (bebob == NULL) return;
- fcp_bus_reset(bebob->unit); - - if (bebob->deferred_registration) { - if (snd_card_register(bebob->card) < 0) { - snd_bebob_stream_destroy_duplex(bebob); - snd_card_free(bebob->card); - } - bebob->deferred_registration = false; - } + /* Postpone a workqueue for deferred registration. */ + if (!bebob->registered) + snd_fw_schedule_registration(unit, &bebob->dwork); + else + fcp_bus_reset(bebob->unit); }
static void bebob_remove(struct fw_unit *unit) @@ -342,8 +369,20 @@ static void bebob_remove(struct fw_unit *unit) if (bebob == NULL) return;
- /* No need to wait for releasing card object in this context. */ - snd_card_free_when_closed(bebob->card); + /* + * 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) { + /* No need to wait for releasing card object in this context. */ + snd_card_free_when_closed(bebob->card); + } else { + /* Don't forget this case. */ + bebob_free(bebob); + } }
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 b50bb33d..2a442a7 100644 --- a/sound/firewire/bebob/bebob.h +++ b/sound/firewire/bebob/bebob.h @@ -83,6 +83,10 @@ 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; @@ -111,7 +115,6 @@ struct snd_bebob {
/* for M-Audio special devices */ void *maudio_special_quirk; - bool deferred_registration;
/* For BeBoB version quirk. */ unsigned int version;
When some fireworks units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/fireworks/fireworks.c | 147 ++++++++++++++++++++++++----------- sound/firewire/fireworks/fireworks.h | 3 + 2 files changed, 103 insertions(+), 47 deletions(-)
diff --git a/sound/firewire/fireworks/fireworks.c b/sound/firewire/fireworks/fireworks.c index 8380fb5..71a0613 100644 --- a/sound/firewire/fireworks/fireworks.c +++ b/sound/firewire/fireworks/fireworks.c @@ -184,6 +184,18 @@ end: return err; }
+static void efw_free(struct snd_efw *efw) +{ + snd_efw_stream_destroy_duplex(efw); + snd_efw_transaction_remove_instance(efw); + fw_unit_put(efw->unit); + + kfree(efw->resp_buf); + + mutex_destroy(&efw->mutex); + kfree(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 @@ -195,28 +207,24 @@ efw_card_free(struct snd_card *card) { struct snd_efw *efw = card->private_data;
- snd_efw_stream_destroy_duplex(efw); - snd_efw_transaction_remove_instance(efw); - fw_unit_put(efw->unit); - - kfree(efw->resp_buf); - if (efw->card_index >= 0) { mutex_lock(&devices_mutex); clear_bit(efw->card_index, devices_used); mutex_unlock(&devices_mutex); }
- mutex_destroy(&efw->mutex); + efw_free(card->private_data); }
-static int -efw_probe(struct fw_unit *unit, - const struct ieee1394_device_id *entry) +static void +do_registration(struct work_struct *work) { - struct snd_card *card; - struct snd_efw *efw; - int card_index, err; + struct snd_efw *efw = container_of(work, struct snd_efw, dwork.work); + unsigned int card_index; + int err; + + if (efw->registered) + return;
mutex_lock(&devices_mutex);
@@ -226,24 +234,16 @@ efw_probe(struct fw_unit *unit, break; } if (card_index >= SNDRV_CARDS) { - err = -ENOENT; - goto end; + mutex_unlock(&devices_mutex); + return; }
- err = snd_card_new(&unit->device, index[card_index], id[card_index], - THIS_MODULE, sizeof(struct snd_efw), &card); - if (err < 0) - goto end; - efw = card->private_data; - efw->card_index = card_index; - set_bit(card_index, devices_used); - card->private_free = efw_card_free; - - efw->card = card; - efw->unit = fw_unit_get(unit); - mutex_init(&efw->mutex); - spin_lock_init(&efw->lock); - init_waitqueue_head(&efw->hwdep_wait); + err = snd_card_new(&efw->unit->device, index[card_index], + id[card_index], THIS_MODULE, 0, &efw->card); + if (err < 0) { + mutex_unlock(&devices_mutex); + return; + }
/* prepare response buffer */ snd_efw_resp_buf_size = clamp(snd_efw_resp_buf_size, @@ -260,6 +260,10 @@ efw_probe(struct fw_unit *unit, if (err < 0) goto error;
+ err = snd_efw_stream_init_duplex(efw); + if (err < 0) + goto error; + snd_efw_proc_init(efw);
if (efw->midi_out_ports || efw->midi_in_ports) { @@ -276,44 +280,93 @@ efw_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_efw_stream_init_duplex(efw); + err = snd_card_register(efw->card); if (err < 0) goto error;
- err = snd_card_register(card); - if (err < 0) { - snd_efw_stream_destroy_duplex(efw); - goto error; - } - - dev_set_drvdata(&unit->device, efw); -end: + set_bit(card_index, devices_used); mutex_unlock(&devices_mutex); - return err; + + /* + * 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); mutex_unlock(&devices_mutex); - snd_card_free(card); - return err; + 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); +} + +static int +efw_probe(struct fw_unit *unit, const struct ieee1394_device_id *entry) +{ + struct snd_efw *efw; + + efw = kzalloc(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; }
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);
- mutex_lock(&efw->mutex); - snd_efw_stream_update_duplex(efw); - mutex_unlock(&efw->mutex); + /* + * 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); + } }
static void efw_remove(struct fw_unit *unit) { struct snd_efw *efw = dev_get_drvdata(&unit->device);
- /* No need to wait for releasing card object in this context. */ - snd_card_free_when_closed(efw->card); + /* + * 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) { + /* No need to wait for releasing card object in this context. */ + snd_card_free_when_closed(efw->card); + } else { + /* Don't forget this case. */ + efw_free(efw); + } }
static const struct ieee1394_device_id efw_id_table[] = { diff --git a/sound/firewire/fireworks/fireworks.h b/sound/firewire/fireworks/fireworks.h index 96c4e0c..471c772 100644 --- a/sound/firewire/fireworks/fireworks.h +++ b/sound/firewire/fireworks/fireworks.h @@ -65,6 +65,9 @@ struct snd_efw { struct mutex mutex; spinlock_t lock;
+ bool registered; + struct delayed_work dwork; + /* for transaction */ u32 seqnum; bool resp_addr_changable;
Some oxfw based units tends to fail asynchronous communication when IEEE 1394 bus is under bus-reset state. When registering sound card instance at unit probe callback, userspace applications can be involved to the state.
This commit postpones the registration till the bus is calm.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/oxfw/oxfw.c | 151 +++++++++++++++++++++++++++++---------------- sound/firewire/oxfw/oxfw.h | 3 + 2 files changed, 101 insertions(+), 53 deletions(-)
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c index abedc22..e629b88 100644 --- a/sound/firewire/oxfw/oxfw.c +++ b/sound/firewire/oxfw/oxfw.c @@ -118,15 +118,8 @@ end: return err; }
-/* - * 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) +static void oxfw_free(struct snd_oxfw *oxfw) { - struct snd_oxfw *oxfw = card->private_data; unsigned int i;
snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream); @@ -144,6 +137,17 @@ static void oxfw_card_free(struct snd_card *card) mutex_destroy(&oxfw->mutex); }
+/* + * 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); @@ -205,41 +209,39 @@ static int detect_quirks(struct snd_oxfw *oxfw) return 0; }
-static int oxfw_probe(struct fw_unit *unit, - const struct ieee1394_device_id *entry) +static void do_registration(struct work_struct *work) { - struct snd_card *card; - struct snd_oxfw *oxfw; + struct snd_oxfw *oxfw = container_of(work, struct snd_oxfw, dwork.work); int err;
- if (entry->vendor_id == VENDOR_LOUD && !detect_loud_models(unit)) - return -ENODEV; + if (oxfw->registered) + return;
- err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, - sizeof(*oxfw), &card); + err = snd_card_new(&oxfw->unit->device, -1, NULL, THIS_MODULE, 0, + &oxfw->card); if (err < 0) - return err; + return;
- card->private_free = oxfw_card_free; - oxfw = card->private_data; - oxfw->card = card; - mutex_init(&oxfw->mutex); - oxfw->unit = fw_unit_get(unit); - oxfw->entry = entry; - spin_lock_init(&oxfw->lock); - init_waitqueue_head(&oxfw->hwdep_wait); + err = name_card(oxfw); + if (err < 0) + goto error;
- err = snd_oxfw_stream_discover(oxfw); + err = detect_quirks(oxfw); if (err < 0) goto error;
- err = name_card(oxfw); + err = snd_oxfw_stream_discover(oxfw); if (err < 0) goto error;
- err = detect_quirks(oxfw); + err = snd_oxfw_stream_init_simplex(oxfw, &oxfw->rx_stream); if (err < 0) goto error; + if (oxfw->has_output) { + err = snd_oxfw_stream_init_simplex(oxfw, &oxfw->tx_stream); + if (err < 0) + goto error; + }
err = snd_oxfw_create_pcm(oxfw); if (err < 0) @@ -255,54 +257,97 @@ static int oxfw_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_oxfw_stream_init_simplex(oxfw, &oxfw->rx_stream); + err = snd_card_register(oxfw->card); if (err < 0) goto error; - if (oxfw->has_output) { - err = snd_oxfw_stream_init_simplex(oxfw, &oxfw->tx_stream); - if (err < 0) - goto error; - }
- err = snd_card_register(card); - if (err < 0) { - snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->rx_stream); - if (oxfw->has_output) - snd_oxfw_stream_destroy_simplex(oxfw, &oxfw->tx_stream); - 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); +} + +static int oxfw_probe(struct fw_unit *unit, + const struct ieee1394_device_id *entry) +{ + struct snd_oxfw *oxfw; + + if (entry->vendor_id == VENDOR_LOUD && !detect_loud_models(unit)) + return -ENODEV; + + /* Allocate this independent of sound card instance. */ + oxfw = kzalloc(sizeof(struct snd_oxfw), GFP_KERNEL); + if (oxfw == NULL) + return -ENOMEM; + + oxfw->entry = entry; + oxfw->unit = fw_unit_get(unit); dev_set_drvdata(&unit->device, oxfw);
+ 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);
- mutex_lock(&oxfw->mutex); + if (oxfw->registered) { + mutex_lock(&oxfw->mutex);
- snd_oxfw_stream_update_simplex(oxfw, &oxfw->rx_stream); - if (oxfw->has_output) - snd_oxfw_stream_update_simplex(oxfw, &oxfw->tx_stream); + snd_oxfw_stream_update_simplex(oxfw, &oxfw->rx_stream); + if (oxfw->has_output) + snd_oxfw_stream_update_simplex(oxfw, &oxfw->tx_stream);
- mutex_unlock(&oxfw->mutex); + mutex_unlock(&oxfw->mutex);
- if (oxfw->entry->vendor_id == OUI_STANTON) - snd_oxfw_scs1x_update(oxfw); + if (oxfw->entry->vendor_id == OUI_STANTON) + snd_oxfw_scs1x_update(oxfw); + } }
static void oxfw_remove(struct fw_unit *unit) { struct snd_oxfw *oxfw = dev_get_drvdata(&unit->device);
- /* No need to wait for releasing card object in this context. */ - snd_card_free_when_closed(oxfw->card); + /* + * 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) { + /* No need to wait for releasing card object in this context. */ + snd_card_free_when_closed(oxfw->card); + } else { + /* Don't forget this case. */ + oxfw_free(oxfw); + } }
static const struct compat_info griffin_firewave = { diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h index 2c84714e..2047dcb 100644 --- a/sound/firewire/oxfw/oxfw.h +++ b/sound/firewire/oxfw/oxfw.h @@ -39,6 +39,9 @@ struct snd_oxfw { struct mutex mutex; spinlock_t lock;
+ bool registered; + struct delayed_work dwork; + bool wrong_dbs; bool has_output; u8 *tx_stream_formats[SND_OXFW_STREAM_FORMAT_ENTRIES];
When some digi00x units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/digi00x/digi00x-transaction.c | 7 +- sound/firewire/digi00x/digi00x.c | 107 +++++++++++++++++++-------- sound/firewire/digi00x/digi00x.h | 3 + 3 files changed, 85 insertions(+), 32 deletions(-)
diff --git a/sound/firewire/digi00x/digi00x-transaction.c b/sound/firewire/digi00x/digi00x-transaction.c index 554324d..735d356 100644 --- a/sound/firewire/digi00x/digi00x-transaction.c +++ b/sound/firewire/digi00x/digi00x-transaction.c @@ -126,12 +126,17 @@ int snd_dg00x_transaction_register(struct snd_dg00x *dg00x) return err; error: fw_core_remove_address_handler(&dg00x->async_handler); - dg00x->async_handler.address_callback = NULL; + dg00x->async_handler.callback_data = NULL; return err; }
void snd_dg00x_transaction_unregister(struct snd_dg00x *dg00x) { + if (dg00x->async_handler.callback_data == NULL) + return; + snd_fw_async_midi_port_destroy(&dg00x->out_control); fw_core_remove_address_handler(&dg00x->async_handler); + + dg00x->async_handler.callback_data = NULL; } diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c index 1f33b7a..cc4776c 100644 --- a/sound/firewire/digi00x/digi00x.c +++ b/sound/firewire/digi00x/digi00x.c @@ -40,10 +40,8 @@ static int name_card(struct snd_dg00x *dg00x) return 0; }
-static void dg00x_card_free(struct snd_card *card) +static void dg00x_free(struct snd_dg00x *dg00x) { - struct snd_dg00x *dg00x = card->private_data; - snd_dg00x_stream_destroy_duplex(dg00x); snd_dg00x_transaction_unregister(dg00x);
@@ -52,28 +50,24 @@ static void dg00x_card_free(struct snd_card *card) mutex_destroy(&dg00x->mutex); }
-static int snd_dg00x_probe(struct fw_unit *unit, - const struct ieee1394_device_id *entry) +static void dg00x_card_free(struct snd_card *card) { - struct snd_card *card; - struct snd_dg00x *dg00x; - int err; + dg00x_free(card->private_data); +}
- /* create card */ - err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, - sizeof(struct snd_dg00x), &card); - if (err < 0) - return err; - card->private_free = dg00x_card_free; +static void do_registration(struct work_struct *work) +{ + struct snd_dg00x *dg00x = + container_of(work, struct snd_dg00x, dwork.work); + int err;
- /* initialize myself */ - dg00x = card->private_data; - dg00x->card = card; - dg00x->unit = fw_unit_get(unit); + if (dg00x->registered) + return;
- mutex_init(&dg00x->mutex); - spin_lock_init(&dg00x->lock); - init_waitqueue_head(&dg00x->hwdep_wait); + err = snd_card_new(&dg00x->unit->device, -1, NULL, THIS_MODULE, 0, + &dg00x->card); + if (err < 0) + return;
err = name_card(dg00x); if (err < 0) @@ -101,35 +95,86 @@ static int snd_dg00x_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_card_register(card); + err = snd_card_register(dg00x->card); if (err < 0) goto error;
- dev_set_drvdata(&unit->device, dg00x); + dg00x->card->private_free = dg00x_card_free; + dg00x->card->private_data = dg00x; + dg00x->registered = true;
- return err; + return; error: - snd_card_free(card); - return err; + 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); +} + +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 = kzalloc(sizeof(struct snd_dg00x), GFP_KERNEL); + if (dg00x == NULL) + 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); + + /* Allocate and register this sound card later. */ + INIT_DEFERRABLE_WORK(&dg00x->dwork, do_registration); + snd_fw_schedule_registration(unit, &dg00x->dwork); + + return 0; }
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);
- mutex_lock(&dg00x->mutex); - snd_dg00x_stream_update_duplex(dg00x); - mutex_unlock(&dg00x->mutex); + /* + * 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); + } }
static void snd_dg00x_remove(struct fw_unit *unit) { struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
- /* No need to wait for releasing card object in this context. */ - snd_card_free_when_closed(dg00x->card); + /* + * 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) { + /* No need to wait for releasing card object in this context. */ + snd_card_free_when_closed(dg00x->card); + } else { + /* Don't forget this case. */ + dg00x_free(dg00x); + } }
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 907e739..2cd465c 100644 --- a/sound/firewire/digi00x/digi00x.h +++ b/sound/firewire/digi00x/digi00x.h @@ -37,6 +37,9 @@ 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;
When some tascam units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/tascam/tascam.c | 118 ++++++++++++++++++++++++++++------------- sound/firewire/tascam/tascam.h | 2 + 2 files changed, 84 insertions(+), 36 deletions(-)
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c index e281c33..9dc93a7 100644 --- a/sound/firewire/tascam/tascam.c +++ b/sound/firewire/tascam/tascam.c @@ -85,10 +85,8 @@ static int identify_model(struct snd_tscm *tscm) return 0; }
-static void tscm_card_free(struct snd_card *card) +static void tscm_free(struct snd_tscm *tscm) { - struct snd_tscm *tscm = card->private_data; - snd_tscm_transaction_unregister(tscm); snd_tscm_stream_destroy_duplex(tscm);
@@ -97,44 +95,36 @@ static void tscm_card_free(struct snd_card *card) mutex_destroy(&tscm->mutex); }
-static int snd_tscm_probe(struct fw_unit *unit, - const struct ieee1394_device_id *entry) +static void tscm_card_free(struct snd_card *card) { - struct snd_card *card; - struct snd_tscm *tscm; + 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); int err;
- /* create card */ - err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE, - sizeof(struct snd_tscm), &card); + err = snd_card_new(&tscm->unit->device, -1, NULL, THIS_MODULE, 0, + &tscm->card); if (err < 0) - return err; - card->private_free = tscm_card_free; - - /* initialize myself */ - tscm = card->private_data; - tscm->card = card; - tscm->unit = fw_unit_get(unit); - - mutex_init(&tscm->mutex); - spin_lock_init(&tscm->lock); - init_waitqueue_head(&tscm->hwdep_wait); + return;
err = identify_model(tscm); if (err < 0) goto error;
- snd_tscm_proc_init(tscm); - - err = snd_tscm_stream_init_duplex(tscm); + err = snd_tscm_transaction_register(tscm); if (err < 0) goto error;
- err = snd_tscm_create_pcm_devices(tscm); + err = snd_tscm_stream_init_duplex(tscm); if (err < 0) goto error;
- err = snd_tscm_transaction_register(tscm); + snd_tscm_proc_init(tscm); + + err = snd_tscm_create_pcm_devices(tscm); if (err < 0) goto error;
@@ -146,35 +136,91 @@ static int snd_tscm_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_card_register(card); + err = snd_card_register(tscm->card); if (err < 0) goto error;
- dev_set_drvdata(&unit->device, tscm); + /* + * 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 err; + return; error: - snd_card_free(card); - return err; + 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); +} + +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 = kzalloc(sizeof(struct snd_tscm), GFP_KERNEL); + if (tscm == NULL) + return -ENOMEM; + + /* initialize myself */ + 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; }
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);
- mutex_lock(&tscm->mutex); - snd_tscm_stream_update_duplex(tscm); - mutex_unlock(&tscm->mutex); + /* + * 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); + } }
static void snd_tscm_remove(struct fw_unit *unit) { struct snd_tscm *tscm = dev_get_drvdata(&unit->device);
- /* No need to wait for releasing card object in this context. */ - snd_card_free_when_closed(tscm->card); + /* + * 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) { + /* No need to wait for releasing card object in this context. */ + snd_card_free_when_closed(tscm->card); + } else { + /* Don't forget this case. */ + tscm_free(tscm); + } }
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 30ab77e..1f61011 100644 --- a/sound/firewire/tascam/tascam.h +++ b/sound/firewire/tascam/tascam.h @@ -51,6 +51,8 @@ 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;
Iwai-san,
On Mar 31 2016 08:47, Takashi Sakamoto wrote:
When some tascam units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/firewire/tascam/tascam.c | 118 ++++++++++++++++++++++++++++------------- sound/firewire/tascam/tascam.h | 2 + 2 files changed, 84 insertions(+), 36 deletions(-)
As of today, this patch is missing in 'for-next' branch of sound.git reporitory[1], Would you please take it again?
Regards
Takashi Sakamoto
diff --git a/sound/firewire/tascam/tascam.c b/sound/firewire/tascam/tascam.c index e281c33..9dc93a7 100644 --- a/sound/firewire/tascam/tascam.c +++ b/sound/firewire/tascam/tascam.c @@ -85,10 +85,8 @@ static int identify_model(struct snd_tscm *tscm) return 0; }
-static void tscm_card_free(struct snd_card *card) +static void tscm_free(struct snd_tscm *tscm) {
- struct snd_tscm *tscm = card->private_data;
- snd_tscm_transaction_unregister(tscm); snd_tscm_stream_destroy_duplex(tscm);
@@ -97,44 +95,36 @@ static void tscm_card_free(struct snd_card *card) mutex_destroy(&tscm->mutex); }
-static int snd_tscm_probe(struct fw_unit *unit,
const struct ieee1394_device_id *entry)+static void tscm_card_free(struct snd_card *card) {
- struct snd_card *card;
- struct snd_tscm *tscm;
- 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); int err;
- /* create card */
- err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
sizeof(struct snd_tscm), &card);
- err = snd_card_new(&tscm->unit->device, -1, NULL, THIS_MODULE, 0,
&tscm->card);
return err;- card->private_free = tscm_card_free;
- /* initialize myself */
- tscm = card->private_data;
- tscm->card = card;
- tscm->unit = fw_unit_get(unit);
- mutex_init(&tscm->mutex);
- spin_lock_init(&tscm->lock);
- init_waitqueue_head(&tscm->hwdep_wait);
return;err = identify_model(tscm); if (err < 0) goto error;
- snd_tscm_proc_init(tscm);
- err = snd_tscm_stream_init_duplex(tscm);
- err = snd_tscm_transaction_register(tscm); if (err < 0) goto error;
- err = snd_tscm_create_pcm_devices(tscm);
- err = snd_tscm_stream_init_duplex(tscm); if (err < 0) goto error;
- err = snd_tscm_transaction_register(tscm);
- snd_tscm_proc_init(tscm);
- err = snd_tscm_create_pcm_devices(tscm); if (err < 0) goto error;
@@ -146,35 +136,91 @@ static int snd_tscm_probe(struct fw_unit *unit, if (err < 0) goto error;
- err = snd_card_register(card);
- err = snd_card_register(tscm->card); if (err < 0) goto error;
- dev_set_drvdata(&unit->device, tscm);
- /*
* 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 err;
- return;
error:
- snd_card_free(card);
- return err;
- 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);+}
+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 = kzalloc(sizeof(struct snd_tscm), GFP_KERNEL);
- if (tscm == NULL)
return -ENOMEM;- /* initialize myself */
- 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;
}
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);
- mutex_lock(&tscm->mutex);
- snd_tscm_stream_update_duplex(tscm);
- mutex_unlock(&tscm->mutex);
- /*
* 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);- }
}
static void snd_tscm_remove(struct fw_unit *unit) { struct snd_tscm *tscm = dev_get_drvdata(&unit->device);
- /* No need to wait for releasing card object in this context. */
- snd_card_free_when_closed(tscm->card);
- /*
* 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) {
/* No need to wait for releasing card object in this context. */snd_card_free_when_closed(tscm->card);- } else {
/* Don't forget this case. */tscm_free(tscm);- }
}
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 30ab77e..1f61011 100644 --- a/sound/firewire/tascam/tascam.h +++ b/sound/firewire/tascam/tascam.h @@ -51,6 +51,8 @@ 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;
On Sun, 24 Apr 2016 14:50:52 +0200, Takashi Sakamoto wrote:
Iwai-san,
On Mar 31 2016 08:47, Takashi Sakamoto wrote:
When some tascam units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/firewire/tascam/tascam.c | 118 ++++++++++++++++++++++++++++------------- sound/firewire/tascam/tascam.h | 2 + 2 files changed, 84 insertions(+), 36 deletions(-)
As of today, this patch is missing in 'for-next' branch of sound.git reporitory[1], Would you please take it again?
OK, applied now. It was missing since your cover letter mentioned only 7 patches. Thanks for noticing it.
Takashi
On Apr 25 2016 17:41, Takashi Iwai wrote:
On Sun, 24 Apr 2016 14:50:52 +0200, Takashi Sakamoto wrote:
Iwai-san,
On Mar 31 2016 08:47, Takashi Sakamoto wrote:
When some tascam units are connected sequentially, userspace applications are involved at bus-reset state on IEEE 1394 bus. In the state, any communications can be canceled. Therefore, sound card registration should be delayed till the bus gets calm.
This commit achieves it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp
sound/firewire/tascam/tascam.c | 118 ++++++++++++++++++++++++++++------------- sound/firewire/tascam/tascam.h | 2 + 2 files changed, 84 insertions(+), 36 deletions(-)
As of today, this patch is missing in 'for-next' branch of sound.git reporitory[1], Would you please take it again?
OK, applied now. It was missing since your cover letter mentioned only 7 patches. Thanks for noticing it.
Oops, indeed. I might have added handy changes to the subject line and written wrong number...
Thanks
Takashi Sakamoto
On Thu, 31 Mar 2016 01:47:01 +0200, Takashi Sakamoto wrote:
Hi,
Commit b59fb1900b4fee('ALSA: dice: postpone card registration') applied delayed registration[1] of sound card instance to ALSA dice driver. This idea is also nice for drivers in ALSA firewire stack because in IEEE 1394 bus adding units is corresponding to bus reset and the actual units tend not to work well during bus-reset state.
This patchset adds a new function to firewire-lib module to schedule a delayed work for sound card registration. Currently, the work is commonly scheduled 2 seconds after time of the last bus-reset recorded by Linux firewire core. Then, take all drivers to use it as Dice driver does.
Related to this patchset, I'd like to cancel a discussion about my proposed patchset to remove unpractical module parameters from ALSA bebob/fireworks[2]. I'm sorry for reviewers but my work priority to this patchset is prior to the former one.
Applied all eight patches now. Thanks.
Takashi
participants (2)
-
Takashi Iwai -
Takashi Sakamoto