[PATCH 01/29] staging: bcm2835-audio: Clean up mutex locks
From: Takashi Iwai
Date: Tue Sep 04 2018 - 12:01:26 EST
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 <tiwai@xxxxxxx>
---
.../vc04_services/bcm2835-audio/bcm2835-ctl.c | 18 +++----
.../vc04_services/bcm2835-audio/bcm2835-pcm.c | 33 ++-----------
.../bcm2835-audio/bcm2835-vchiq.c | 47 ++++---------------
.../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 ec468d5719b1..04ea3ec7f64f 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 8359cf881bef..f2d8b17d0cfe 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 868e2d6aaf1b..bec361aff4fe 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 da0fa34501fa..fa04f6bc9858 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.18.0