From e2eb31d72156c58b717396383496a7c93aa01b75 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 3 Jan 2017 11:58:32 +0900 Subject: [PATCH 1/8] ALSA: fireworks: fix asymmetric API call at unit removal ALSA fireworks driver has a bug not to call an API to destroy 'cmp_connection' structure for input direction. Currently this causes no issues because it just destroys 'mutex' structure, while it's better to fix it for future work. Fix: d23c2cc4485d ("ALSA: fireworks/bebob/dice/oxfw: allow stream destructor after releasing runtime") Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/firewire/fireworks/fireworks_stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c index ee47924aef0df6..827161bc269cfa 100644 --- a/sound/firewire/fireworks/fireworks_stream.c +++ b/sound/firewire/fireworks/fireworks_stream.c @@ -117,7 +117,7 @@ destroy_stream(struct snd_efw *efw, struct amdtp_stream *stream) conn = &efw->in_conn; amdtp_stream_destroy(stream); - cmp_connection_destroy(&efw->out_conn); + cmp_connection_destroy(conn); } static int From 6a2a2f45560a9cb7bc49820883b042e44f83726c Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 3 Jan 2017 11:58:33 +0900 Subject: [PATCH 2/8] ALSA: firewire-tascam: Fix to handle error from initialization of stream data This module has a bug not to return error code in a case that data structure for transmitted packets fails to be initialized. This commit fixes the bug. Fixes: 35efa5c489de ("ALSA: firewire-tascam: add streaming functionality") Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/firewire/tascam/tascam-stream.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c index 4ad3bd7fd4453e..f1657a4e0621ef 100644 --- a/sound/firewire/tascam/tascam-stream.c +++ b/sound/firewire/tascam/tascam-stream.c @@ -343,7 +343,7 @@ int snd_tscm_stream_init_duplex(struct snd_tscm *tscm) if (err < 0) amdtp_stream_destroy(&tscm->rx_stream); - return 0; + return err; } /* At bus reset, streaming is stopped and some registers are clear. */ From 6b7e95d1336b9eb0d4c6db190ce756480496bd13 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Tue, 3 Jan 2017 11:58:34 +0900 Subject: [PATCH 3/8] ALSA: firewire-lib: change structure member with proper type The 'amdtp_stream' structure is initialized by a call of 'amdtp_stream_init()'. Although a parameter of this function is for bit flags of packet attributes, its type is enumerator. This commit changes the type so that it's proper for a bit flags. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/firewire/amdtp-stream.c | 2 +- sound/firewire/amdtp-stream.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 00060c4a9deb4e..8ce93cdc4b0d83 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -69,7 +69,7 @@ static void pcm_period_tasklet(unsigned long data); * @protocol_size: the size to allocate newly for protocol */ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size) diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index c1bc7fad056e82..7be21429cd1214 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -93,7 +93,7 @@ typedef unsigned int (*amdtp_stream_process_data_blocks_t)( unsigned int *syt); struct amdtp_stream { struct fw_unit *unit; - enum cip_flags flags; + int flags; enum amdtp_stream_direction direction; struct mutex mutex; @@ -137,7 +137,7 @@ struct amdtp_stream { }; int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, enum cip_flags flags, + enum amdtp_stream_direction dir, int flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size); From 85bcf96caba8b4a7c0805555638629ba3c67ea0c Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 6 Dec 2016 16:20:36 +0100 Subject: [PATCH 4/8] ALSA: hda - Fix up GPIO for ASUS ROG Ranger ASUS ROG Ranger VIII with ALC1150 codec requires the extra GPIO pin to up for the front panel. Just use the existing fixup for setting up the GPIO pins. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=189411 Cc: Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index 9448daff9d8b55..d43f396450f263 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -2230,6 +2230,7 @@ static const struct snd_pci_quirk alc882_fixup_tbl[] = { SND_PCI_QUIRK(0x1043, 0x1971, "Asus W2JC", ALC882_FIXUP_ASUS_W2JC), SND_PCI_QUIRK(0x1043, 0x835f, "Asus Eee 1601", ALC888_FIXUP_EEE1601), SND_PCI_QUIRK(0x1043, 0x84bc, "ASUS ET2700", ALC887_FIXUP_ASUS_BASS), + SND_PCI_QUIRK(0x1043, 0x8691, "ASUS ROG Ranger VIII", ALC882_FIXUP_GPIO3), SND_PCI_QUIRK(0x104d, 0x9047, "Sony Vaio TT", ALC889_FIXUP_VAIO_TT), SND_PCI_QUIRK(0x104d, 0x905a, "Sony Vaio Z", ALC882_FIXUP_NO_PRIMARY_HP), SND_PCI_QUIRK(0x104d, 0x9043, "Sony Vaio VGC-LN51JGB", ALC882_FIXUP_NO_PRIMARY_HP), From c7efff9284dfde95a11aaa811c9d8ec8167f0f6e Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 4 Jan 2017 21:38:16 +0100 Subject: [PATCH 5/8] ALSA: hda - Apply asus-mode8 fixup to ASUS X71SL Although the old quirk table showed ASUS X71SL with ALC663 codec being compatible with asus-mode3 fixup, the bugzilla reporter explained that asus-model8 fits better for the dual headphone controls. So be it. Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=191781 Cc: Signed-off-by: Takashi Iwai --- sound/pci/hda/patch_realtek.c | 1 + 1 file changed, 1 insertion(+) diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c index d43f396450f263..7d660ee1d5e84e 100644 --- a/sound/pci/hda/patch_realtek.c +++ b/sound/pci/hda/patch_realtek.c @@ -6984,6 +6984,7 @@ static const struct snd_pci_quirk alc662_fixup_tbl[] = { SND_PCI_QUIRK(0x1043, 0x15a7, "ASUS UX51VZH", ALC662_FIXUP_BASS_16), SND_PCI_QUIRK(0x1043, 0x177d, "ASUS N551", ALC668_FIXUP_ASUS_Nx51), SND_PCI_QUIRK(0x1043, 0x17bd, "ASUS N751", ALC668_FIXUP_ASUS_Nx51), + SND_PCI_QUIRK(0x1043, 0x1963, "ASUS X71SL", ALC662_FIXUP_ASUS_MODE8), SND_PCI_QUIRK(0x1043, 0x1b73, "ASUS N55SF", ALC662_FIXUP_BASS_16), SND_PCI_QUIRK(0x1043, 0x1bf3, "ASUS N76VZ", ALC662_FIXUP_BASS_MODE4_CHMAP), SND_PCI_QUIRK(0x1043, 0x8469, "ASUS mobo", ALC662_FIXUP_NO_JACK_DETECT), From 1d0f953086f090a022f2c0e1448300c15372db46 Mon Sep 17 00:00:00 2001 From: Ioan-Adrian Ratiu Date: Thu, 5 Jan 2017 00:37:46 +0200 Subject: [PATCH 6/8] ALSA: usb-audio: Fix irq/process data synchronization Commit 16200948d83 ("ALSA: usb-audio: Fix race at stopping the stream") was incomplete causing another more severe kernel panic, so it got reverted. This fixes both the original problem and its fallout kernel race/crash. The original fix is to move the endpoint member NULL clearing logic inside wait_clear_urbs() so the irq triggering the urb completion doesn't call retire_capture/playback_urb() after the NULL clearing and generate a panic. However this creates a new race between snd_usb_endpoint_start()'s call to wait_clear_urbs() and the irq urb completion handler which again calls retire_capture/playback_urb() leading to a new NULL dereference. We keep the EP deactivation code in snd_usb_endpoint_start() because removing it will break the EP reference counting (see [1] [2] for info), however we don't need the "can_sleep" mechanism anymore because a new function was introduced (snd_usb_endpoint_sync_pending_stop()) which synchronizes pending stops and gets called inside the pcm prepare callback. It also makes sense to remove can_sleep because it was also removed from deactivate_urbs() signature in [3] so we benefit from more simplification. [1] commit 015618b90 ("ALSA: snd-usb: Fix URB cancellation at stream start") [2] commit e9ba389c5 ("ALSA: usb-audio: Fix scheduling-while-atomic bug in PCM capture stream") [3] commit ccc1696d5 ("ALSA: usb-audio: simplify endpoint deactivation code") Fixes: f8114f8583bb ("Revert "ALSA: usb-audio: Fix race at stopping the stream"") Signed-off-by: Ioan-Adrian Ratiu Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 17 +++++++---------- sound/usb/endpoint.h | 2 +- sound/usb/pcm.c | 10 +++++----- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 15d1d5c63c3c40..2f0ea70a998ce7 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -534,6 +534,11 @@ static int wait_clear_urbs(struct snd_usb_endpoint *ep) alive, ep->ep_num); clear_bit(EP_FLAG_STOPPING, &ep->flags); + ep->data_subs = NULL; + ep->sync_slave = NULL; + ep->retire_data_urb = NULL; + ep->prepare_data_urb = NULL; + return 0; } @@ -912,9 +917,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, /** * snd_usb_endpoint_start: start an snd_usb_endpoint * - * @ep: the endpoint to start - * @can_sleep: flag indicating whether the operation is executed in - * non-atomic context + * @ep: the endpoint to start * * A call to this function will increment the use count of the endpoint. * In case it is not already running, the URBs for this endpoint will be @@ -924,7 +927,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, * * Returns an error if the URB submission failed, 0 in all other cases. */ -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep) +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep) { int err; unsigned int i; @@ -938,8 +941,6 @@ int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep) /* just to be sure */ deactivate_urbs(ep, false); - if (can_sleep) - wait_clear_urbs(ep); ep->active_mask = 0; ep->unlink_mask = 0; @@ -1020,10 +1021,6 @@ void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep) if (--ep->use_count == 0) { deactivate_urbs(ep, false); - ep->data_subs = NULL; - ep->sync_slave = NULL; - ep->retire_data_urb = NULL; - ep->prepare_data_urb = NULL; set_bit(EP_FLAG_STOPPING, &ep->flags); } } diff --git a/sound/usb/endpoint.h b/sound/usb/endpoint.h index 6428392d8f6244..584f295d7c7738 100644 --- a/sound/usb/endpoint.h +++ b/sound/usb/endpoint.h @@ -18,7 +18,7 @@ int snd_usb_endpoint_set_params(struct snd_usb_endpoint *ep, struct audioformat *fmt, struct snd_usb_endpoint *sync_ep); -int snd_usb_endpoint_start(struct snd_usb_endpoint *ep, bool can_sleep); +int snd_usb_endpoint_start(struct snd_usb_endpoint *ep); void snd_usb_endpoint_stop(struct snd_usb_endpoint *ep); void snd_usb_endpoint_sync_pending_stop(struct snd_usb_endpoint *ep); int snd_usb_endpoint_activate(struct snd_usb_endpoint *ep); diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c index 34c6d4f2c0b629..9aa5b18554812a 100644 --- a/sound/usb/pcm.c +++ b/sound/usb/pcm.c @@ -218,7 +218,7 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface, } } -static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) +static int start_endpoints(struct snd_usb_substream *subs) { int err; @@ -231,7 +231,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) dev_dbg(&subs->dev->dev, "Starting data EP @%p\n", ep); ep->data_subs = subs; - err = snd_usb_endpoint_start(ep, can_sleep); + err = snd_usb_endpoint_start(ep); if (err < 0) { clear_bit(SUBSTREAM_FLAG_DATA_EP_STARTED, &subs->flags); return err; @@ -260,7 +260,7 @@ static int start_endpoints(struct snd_usb_substream *subs, bool can_sleep) dev_dbg(&subs->dev->dev, "Starting sync EP @%p\n", ep); ep->sync_slave = subs->data_endpoint; - err = snd_usb_endpoint_start(ep, can_sleep); + err = snd_usb_endpoint_start(ep); if (err < 0) { clear_bit(SUBSTREAM_FLAG_SYNC_EP_STARTED, &subs->flags); return err; @@ -850,7 +850,7 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream) /* for playback, submit the URBs now; otherwise, the first hwptr_done * updates for all URBs would happen at the same time when starting */ if (subs->direction == SNDRV_PCM_STREAM_PLAYBACK) - ret = start_endpoints(subs, true); + ret = start_endpoints(subs); unlock: snd_usb_unlock_shutdown(subs->stream->chip); @@ -1666,7 +1666,7 @@ static int snd_usb_substream_capture_trigger(struct snd_pcm_substream *substream switch (cmd) { case SNDRV_PCM_TRIGGER_START: - err = start_endpoints(subs, false); + err = start_endpoints(subs); if (err < 0) return err; From 13a6c8328e6056932dc680e447d4c5e8ad9add17 Mon Sep 17 00:00:00 2001 From: Ioan-Adrian Ratiu Date: Thu, 5 Jan 2017 00:37:47 +0200 Subject: [PATCH 7/8] ALSA: usb-audio: test EP_FLAG_RUNNING at urb completion Testing EP_FLAG_RUNNING in snd_complete_urb() before running the completion logic allows us to save a few cpu cycles by returning early, skipping the pending urb in case the stream was stopped; the stop logic handles the urb and sets the completion callbacks to NULL. Signed-off-by: Ioan-Adrian Ratiu Signed-off-by: Takashi Iwai --- sound/usb/endpoint.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sound/usb/endpoint.c b/sound/usb/endpoint.c index 2f0ea70a998ce7..c90607ebe155b4 100644 --- a/sound/usb/endpoint.c +++ b/sound/usb/endpoint.c @@ -384,6 +384,9 @@ static void snd_complete_urb(struct urb *urb) if (unlikely(atomic_read(&ep->chip->shutdown))) goto exit_clear; + if (unlikely(!test_bit(EP_FLAG_RUNNING, &ep->flags))) + goto exit_clear; + if (usb_pipeout(ep->pipe)) { retire_outbound_urb(ep, ctx); /* can be stopped during retire callback */ From e4f34cf6d59160818dcdcf41f4116cc88093ece3 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Thu, 5 Jan 2017 09:41:31 +0900 Subject: [PATCH 8/8] Revert "ALSA: firewire-lib: change structure member with proper type" This reverts commit 6b7e95d1336b9eb0d4c6db190ce756480496bd13. This commit is based on a concern about value of the given parameter. It's expected to be ORed value with some enumeration-constants, thus often it can not be one of the enumeration-constants. I understood that this is out of specification and causes implementation-dependent issues. In C language specification, enumerated type can be interpreted as an integer type, in which all of enumeration-constants in corresponding enumerator-list can be stored. Implementations can select one of char, signed int and unsigned int as its type, and this selection is implementation-dependent. In GCC, a signed integer is selected when at least one of enumeration-constants has negative value, else an unsigned integer is selected. This behaviour can be switched by -fshort-enums to short type. Anyway, the type can be decided after scanning all of enumeration-constants. Totally, there's no rules to constrain the value of enumerated type to be one of enumeration-constants. In short, in enumerated type, decision of actual type for the type is the most important and enumeration-constants are just used for the decision, thus it's permitted to have an integer value in a range of enumeration-constants. In our case, actual type for the type is currently deterministic to be either char or unsigned int. Under GCC, it's unsigned int. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/firewire/amdtp-stream.c | 2 +- sound/firewire/amdtp-stream.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 8ce93cdc4b0d83..00060c4a9deb4e 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -69,7 +69,7 @@ static void pcm_period_tasklet(unsigned long data); * @protocol_size: the size to allocate newly for protocol */ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, int flags, + enum amdtp_stream_direction dir, enum cip_flags flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size) diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 7be21429cd1214..c1bc7fad056e82 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -93,7 +93,7 @@ typedef unsigned int (*amdtp_stream_process_data_blocks_t)( unsigned int *syt); struct amdtp_stream { struct fw_unit *unit; - int flags; + enum cip_flags flags; enum amdtp_stream_direction direction; struct mutex mutex; @@ -137,7 +137,7 @@ struct amdtp_stream { }; int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit, - enum amdtp_stream_direction dir, int flags, + enum amdtp_stream_direction dir, enum cip_flags flags, unsigned int fmt, amdtp_stream_process_data_blocks_t process_data_blocks, unsigned int protocol_size);