From 14a37ac1bf8a2ba9b19032195c371c7b0857ca87 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sat, 20 Feb 2016 16:18:56 +0900 Subject: [PATCH] ALSA: bebob: simplify bus-reset handling At bus-reset, DM1000/DM1100/DM1500 chipsets transfer packets with discontinuous value in 'dbc' field of CIP header. In this case, packet streaming layer in firewire-lib module stops streaming and set XRUN to PCM substream. In ALSA, PCM applications are notified the XRUN status by the return value of ALSA PCM interface. They can recover this state by executing snd_pcm_prepare(), then PCM drivers' prepare handler is called, and start new PCM substream. For ALSA BeBoB driver, the handler establishes new connections and start new AMDTP streaming. Unfortunately, neither the PCM applications nor the driver know the reason of XRUN. The driver gets to know the reason when update handler is called by IEEE 1394 bus driver. As long as I tested, the order of below events are not fixed: * Detecting packet discontinuity in tasklet context of OHCI 1394 driver * Calling prepare handler in process context of ALSA PCM application * Calling update handler in kthread context of IEEE 1394 bus driver The unpredictable order is disadvantage for the driver to be compliant to CMP. In IEC 61883-1, new CMP establish operations should be done 1 sec (isoc_resource_delay) after bus-reset. Within 1 sec, CMP restore operations are allowed. For this reason, in former commit ('b6bc812327aa: ALSA: bebob/firewire-lib: Add a quirk for discontinuity at bus reset'), the process context is forced to wait for executing update handler. The process context wait for bus-reset up to 1 sec. This commit solves the issue, while causes more disadvantages. For PCM applications, calling snd_pcm_prepare() for recovering XRUN state takes more time and the driver got a bit complicated code, while the recovery is not always successful. As long as I tested, DM1000/DM1100/DM1500 and BeBoB firmware can allow drivers to establish new connections just after bus reset. Furthermore, any FCP transactions are handled correctly. Therefore, the driver don't need to wait for bus reset handler for starting new streaming. This commit removes the codes to reduce maintenance cost. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/firewire/bebob/bebob.c | 7 +++---- sound/firewire/bebob/bebob.h | 2 -- sound/firewire/bebob/bebob_stream.c | 32 ++--------------------------- 3 files changed, 5 insertions(+), 36 deletions(-) diff --git a/sound/firewire/bebob/bebob.c b/sound/firewire/bebob/bebob.c index 091290d1f3ea..671179eaff1e 100644 --- a/sound/firewire/bebob/bebob.c +++ b/sound/firewire/bebob/bebob.c @@ -309,7 +309,10 @@ bebob_update(struct fw_unit *unit) return; fcp_bus_reset(bebob->unit); + + mutex_lock(&bebob->mutex); snd_bebob_stream_update_duplex(bebob); + mutex_unlock(&bebob->mutex); if (bebob->deferred_registration) { if (snd_card_register(bebob->card) < 0) { @@ -327,10 +330,6 @@ static void bebob_remove(struct fw_unit *unit) if (bebob == NULL) return; - /* Awake bus-reset waiters. */ - if (!completion_done(&bebob->bus_reset)) - complete_all(&bebob->bus_reset); - /* No need to wait for releasing card object in this context. */ snd_card_free_when_closed(bebob->card); } diff --git a/sound/firewire/bebob/bebob.h b/sound/firewire/bebob/bebob.h index 4d8fcc78e747..c2e885c06473 100644 --- a/sound/firewire/bebob/bebob.h +++ b/sound/firewire/bebob/bebob.h @@ -88,8 +88,6 @@ struct snd_bebob { unsigned int midi_input_ports; unsigned int midi_output_ports; - /* for bus reset quirk */ - struct completion bus_reset; bool connected; struct amdtp_stream *master; diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index 5022c9b97ddf..073ad6d8a76e 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -549,8 +549,7 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob) destroy_both_connections(bebob); goto end; } - /* See comments in next function */ - init_completion(&bebob->bus_reset); + bebob->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK; /* @@ -588,25 +587,8 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) struct amdtp_stream *master, *slave; enum cip_flags sync_mode; unsigned int curr_rate; - bool updated = false; int err = 0; - /* - * Normal BeBoB firmware has a quirk at bus reset to transmits packets - * with discontinuous value in dbc field. - * - * This 'struct completion' is used to call .update() at first to update - * connections/streams. Next following codes handle streaming error. - */ - if (amdtp_streaming_error(&bebob->tx_stream)) { - if (completion_done(&bebob->bus_reset)) - reinit_completion(&bebob->bus_reset); - - updated = (wait_for_completion_interruptible_timeout( - &bebob->bus_reset, - msecs_to_jiffies(FW_ISO_RESOURCE_DELAY)) > 0); - } - mutex_lock(&bebob->mutex); /* Need no substreams */ @@ -642,8 +624,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) amdtp_stream_stop(master); if (amdtp_streaming_error(slave)) amdtp_stream_stop(slave); - if (!updated && - !amdtp_stream_running(master) && !amdtp_stream_running(slave)) + if (!amdtp_stream_running(master) && !amdtp_stream_running(slave)) break_both_connections(bebob); /* stop streams if rate is different */ @@ -774,9 +755,6 @@ void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob) void snd_bebob_stream_update_duplex(struct snd_bebob *bebob) { - /* vs. XRUN recovery due to discontinuity at bus reset */ - mutex_lock(&bebob->mutex); - if ((cmp_connection_update(&bebob->in_conn) < 0) || (cmp_connection_update(&bebob->out_conn) < 0)) { amdtp_stream_pcm_abort(&bebob->rx_stream); @@ -788,12 +766,6 @@ void snd_bebob_stream_update_duplex(struct snd_bebob *bebob) amdtp_stream_update(&bebob->rx_stream); amdtp_stream_update(&bebob->tx_stream); } - - /* wake up stream_start_duplex() */ - if (!completion_done(&bebob->bus_reset)) - complete_all(&bebob->bus_reset); - - mutex_unlock(&bebob->mutex); } /* -- 2.30.2