From: Takashi Iwai Date: Tue, 4 Sep 2018 15:58:30 +0000 (+0200) Subject: staging: bcm2835-audio: Clean up mutex locks X-Git-Url: http://git.maquefel.me/?a=commitdiff_plain;h=da85d3b1a8d4a9f70d4d48b7629f3f215ef9ec40;p=linux.git staging: bcm2835-audio: Clean up mutex locks 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 --- diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c index ec468d5719b11..04ea3ec7f64fd 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 8359cf881bef5..f2d8b17d0cfe0 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 868e2d6aaf1bc..bec361aff4fe8 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 da0fa34501fad..fa04f6bc9858d 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);