From bd2855573c333041c9ba7a94b378504c82e8e916 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 4 Sep 2018 17:58:30 +0200 Subject: [PATCH] staging: bcm2835-audio: Clean up mutex locks commit ce4bb1aa271a97047b80ac917a5d91b54925913b upstream. snd-bcm2835 driver takes the lock with mutex_lock_interruptible() in all places, which don't make sense. Replace them with the simple mutex_lock(). Also taking a mutex lock right after creating it for each PCM object is nonsense, too. It cannot be racy at that point. We can get rid of it. Last but not least, initializing chip->audio_mutex at each place is error-prone. Initialize properly at creating the chip object in snd_bcm2835_create() instead. Signed-off-by: Takashi Iwai Tested-by: Stefan Wahren Signed-off-by: Greg Kroah-Hartman --- .../vc04_services/bcm2835-audio/bcm2835-ctl.c | 18 +++------ .../vc04_services/bcm2835-audio/bcm2835-pcm.c | 33 +++------------ .../vc04_services/bcm2835-audio/bcm2835-vchiq.c | 47 ++++------------------ .../staging/vc04_services/bcm2835-audio/bcm2835.c | 1 + 4 files changed, 20 insertions(+), 79 deletions(-) diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c index ec468d5..04ea3ec 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c @@ -77,8 +77,7 @@ static int snd_bcm2835_ctl_get(struct snd_kcontrol *kcontrol, { struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); BUG_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK)); @@ -99,8 +98,7 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol *kcontrol, struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); int changed = 0; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); if (kcontrol->private_value == PCM_PLAYBACK_VOLUME) { audio_info("Volume change attempted.. volume = %d new_volume = %d\n", chip->volume, (int)ucontrol->value.integer.value[0]); @@ -187,8 +185,7 @@ static int snd_bcm2835_spdif_default_get(struct snd_kcontrol *kcontrol, struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); int i; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); for (i = 0; i < 4; i++) ucontrol->value.iec958.status[i] = @@ -205,8 +202,7 @@ static int snd_bcm2835_spdif_default_put(struct snd_kcontrol *kcontrol, unsigned int val = 0; int i, change; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); for (i = 0; i < 4; i++) val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8); @@ -251,8 +247,7 @@ static int snd_bcm2835_spdif_stream_get(struct snd_kcontrol *kcontrol, struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol); int i; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); for (i = 0; i < 4; i++) ucontrol->value.iec958.status[i] = @@ -269,8 +264,7 @@ static int snd_bcm2835_spdif_stream_put(struct snd_kcontrol *kcontrol, unsigned int val = 0; int i, change; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); for (i = 0; i < 4; i++) val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8); diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c index 8359cf8..f2d8b17 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-pcm.c @@ -99,10 +99,7 @@ static int snd_bcm2835_playback_open_generic( int idx; int err; - if (mutex_lock_interruptible(&chip->audio_mutex)) { - audio_error("Interrupted whilst waiting for lock\n"); - return -EINTR; - } + mutex_lock(&chip->audio_mutex); audio_info("Alsa open (%d)\n", substream->number); idx = substream->number; @@ -194,10 +191,7 @@ static int snd_bcm2835_playback_close(struct snd_pcm_substream *substream) struct bcm2835_alsa_stream *alsa_stream; chip = snd_pcm_substream_chip(substream); - if (mutex_lock_interruptible(&chip->audio_mutex)) { - audio_error("Interrupted whilst waiting for lock\n"); - return -EINTR; - } + mutex_lock(&chip->audio_mutex); runtime = substream->runtime; alsa_stream = runtime->private_data; @@ -274,8 +268,7 @@ static int snd_bcm2835_pcm_prepare(struct snd_pcm_substream *substream) int channels; int err; - if (mutex_lock_interruptible(&chip->audio_mutex)) - return -EINTR; + mutex_lock(&chip->audio_mutex); /* notify the vchiq that it should enter spdif passthrough mode by * setting channels=0 (see @@ -449,14 +442,9 @@ int snd_bcm2835_new_pcm(struct bcm2835_chip *chip, u32 numchannels) struct snd_pcm *pcm; int err; - mutex_init(&chip->audio_mutex); - if (mutex_lock_interruptible(&chip->audio_mutex)) { - audio_error("Interrupted whilst waiting for lock\n"); - return -EINTR; - } err = snd_pcm_new(chip->card, "bcm2835 ALSA", 0, numchannels, 0, &pcm); if (err < 0) - goto out; + return err; pcm->private_data = chip; strcpy(pcm->name, "bcm2835 ALSA"); chip->pcm = pcm; @@ -474,9 +462,6 @@ int snd_bcm2835_new_pcm(struct bcm2835_chip *chip, u32 numchannels) snd_bcm2835_playback_hw.buffer_bytes_max, snd_bcm2835_playback_hw.buffer_bytes_max); -out: - mutex_unlock(&chip->audio_mutex); - return 0; } @@ -485,13 +470,9 @@ int snd_bcm2835_new_spdif_pcm(struct bcm2835_chip *chip) struct snd_pcm *pcm; int err; - if (mutex_lock_interruptible(&chip->audio_mutex)) { - audio_error("Interrupted whilst waiting for lock\n"); - return -EINTR; - } err = snd_pcm_new(chip->card, "bcm2835 ALSA", 1, 1, 0, &pcm); if (err < 0) - goto out; + return err; pcm->private_data = chip; strcpy(pcm->name, "bcm2835 IEC958/HDMI"); @@ -504,8 +485,6 @@ int snd_bcm2835_new_spdif_pcm(struct bcm2835_chip *chip) snd_pcm_lib_preallocate_pages_for_all(pcm, SNDRV_DMA_TYPE_CONTINUOUS, snd_dma_continuous_data(GFP_KERNEL), snd_bcm2835_playback_spdif_hw.buffer_bytes_max, snd_bcm2835_playback_spdif_hw.buffer_bytes_max); -out: - mutex_unlock(&chip->audio_mutex); return 0; } @@ -518,8 +497,6 @@ int snd_bcm2835_new_simple_pcm(struct bcm2835_chip *chip, struct snd_pcm *pcm; int err; - mutex_init(&chip->audio_mutex); - err = snd_pcm_new(chip->card, name, 0, numchannels, 0, &pcm); if (err) diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c index 868e2d6..bec361a 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-vchiq.c @@ -319,11 +319,7 @@ static int vc_vchi_audio_deinit(struct bcm2835_audio_instance *instance) } LOG_DBG(" .. about to lock (%d)\n", instance->num_connections); - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); /* Close all VCHI service connections */ for (i = 0; i < instance->num_connections; i++) { @@ -434,11 +430,7 @@ int bcm2835_audio_open(struct bcm2835_alsa_stream *alsa_stream) instance = alsa_stream->instance; LOG_DBG(" instance (%p)\n", instance); - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", instance->num_connections); - ret = -EINTR; - goto free_wq; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); m.type = VC_AUDIO_MSG_TYPE_OPEN; @@ -479,11 +471,7 @@ static int bcm2835_audio_set_ctls_chan(struct bcm2835_alsa_stream *alsa_stream, LOG_INFO(" Setting ALSA dest(%d), volume(%d)\n", chip->dest, chip->volume); - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); instance->result = -1; @@ -569,10 +557,7 @@ int bcm2835_audio_set_params(struct bcm2835_alsa_stream *alsa_stream, return -EINVAL; } - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); instance->result = -1; @@ -629,11 +614,7 @@ static int bcm2835_audio_start_worker(struct bcm2835_alsa_stream *alsa_stream) int status; int ret; - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); m.type = VC_AUDIO_MSG_TYPE_START; @@ -665,11 +646,7 @@ static int bcm2835_audio_stop_worker(struct bcm2835_alsa_stream *alsa_stream) int status; int ret; - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); m.type = VC_AUDIO_MSG_TYPE_STOP; @@ -704,11 +681,7 @@ int bcm2835_audio_close(struct bcm2835_alsa_stream *alsa_stream) my_workqueue_quit(alsa_stream); - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); m.type = VC_AUDIO_MSG_TYPE_CLOSE; @@ -761,11 +734,7 @@ static int bcm2835_audio_write_worker(struct bcm2835_alsa_stream *alsa_stream, LOG_INFO(" Writing %d bytes from %p\n", count, src); - if (mutex_lock_interruptible(&instance->vchi_mutex)) { - LOG_DBG("Interrupted whilst waiting for lock on (%d)\n", - instance->num_connections); - return -EINTR; - } + mutex_lock(&instance->vchi_mutex); vchi_service_use(instance->vchi_handle[0]); if (instance->peer_version == 0 && diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c index da0fa34..fa04f6b 100644 --- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c +++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c @@ -149,6 +149,7 @@ static int snd_bcm2835_create(struct snd_card *card, return -ENOMEM; chip->card = card; + mutex_init(&chip->audio_mutex); chip->vchi_ctx = devres_find(card->dev->parent, bcm2835_devm_free_vchi_ctx, NULL, NULL); -- 2.7.4