linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Eric Anholt <eric@anholt.net>,
	Stefan Wahren <stefan.wahren@i2se.com>,
	linux-rpi-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH 01/29] staging: bcm2835-audio: Clean up mutex locks
Date: Tue,  4 Sep 2018 17:58:30 +0200	[thread overview]
Message-ID: <20180904155858.8001-2-tiwai@suse.de> (raw)
In-Reply-To: <20180904155858.8001-1-tiwai@suse.de>

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@suse.de>
---
 .../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


  reply	other threads:[~2018-09-04 16:01 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-04 15:58 [PATCH 00/29] staging: bcm2835-audio: Cleanups and fixes Takashi Iwai
2018-09-04 15:58 ` Takashi Iwai [this message]
2018-09-04 15:58 ` [PATCH 02/29] staging: bcm2835-audio: Remove redundant spdif stream ctls Takashi Iwai
2018-09-04 15:58 ` [PATCH 03/29] staging: bcm2835-audio: Clean up include files in bcm2835-ctl.c Takashi Iwai
2018-09-08 13:25   ` Stefan Wahren
2018-09-08 16:21     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 04/29] staging: bcm2835-audio: Remove redundant substream mask checks Takashi Iwai
2018-09-04 15:58 ` [PATCH 05/29] staging: bcm2835-audio: Fix mute controls, volume handling cleanup Takashi Iwai
2018-09-04 15:58 ` [PATCH 06/29] staging: bcm2835-audio: Remove redundant function calls Takashi Iwai
2018-09-04 15:58 ` [PATCH 07/29] staging: bcm2835-audio: Remove superfluous open flag Takashi Iwai
2018-09-04 15:58 ` [PATCH 08/29] staging: bcm2835-audio: Drop useless running flag and check Takashi Iwai
2018-09-04 15:58 ` [PATCH 09/29] staging: bcm2835-audio: Fix incorrect draining handling Takashi Iwai
2018-09-04 15:58 ` [PATCH 10/29] staging: bcm2835-audio: Kill unused spinlock Takashi Iwai
2018-09-04 15:58 ` [PATCH 11/29] staging: bcm2835-audio: Use PCM runtime values instead Takashi Iwai
2018-09-04 15:58 ` [PATCH 12/29] staging: bcm2835-audio: Drop unnecessary pcm indirect setup Takashi Iwai
2018-09-04 15:58 ` [PATCH 13/29] staging: bcm2835-audio: Drop useless NULL check Takashi Iwai
2018-09-04 15:58 ` [PATCH 14/29] staging: bcm2835-audio: Propagate parameter setup error Takashi Iwai
2018-09-04 15:58 ` [PATCH 15/29] staging: bcm2835-audio: Drop debug messages in bcm2835-pcm.c Takashi Iwai
2018-09-04 15:58 ` [PATCH 16/29] staging: bcm2835-audio: Drop superfluous mutex lock during prepare Takashi Iwai
2018-09-08 13:40   ` Stefan Wahren
2018-09-08 16:12     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 17/29] staging: bcm2835-audio: Add 10ms period constraint Takashi Iwai
2018-09-19  9:42   ` Stefan Wahren
2018-09-19  9:52     ` Takashi Iwai
2018-09-19 12:41       ` Stefan Wahren
2018-09-19 12:47         ` Mike Brady
2018-09-19 18:39         ` Takashi Iwai
2018-10-09 13:18           ` [PATCH 17/29] staging: bcm2835-audio: Add 10ms period constraint [Resend in plain text...] Mike Brady
2018-10-09 13:44             ` Takashi Iwai
2018-10-09 15:28               ` Mike Brady
2018-10-09 15:32                 ` Takashi Iwai
2018-10-11 12:53                 ` Mike Brady
2018-10-11 14:07                   ` Stefan Wahren
2018-10-13 15:00                   ` Mike Brady
2018-10-13 15:45                     ` Takashi Iwai
2018-09-04 15:58 ` [PATCH 18/29] staging: bcm2835-audio: Make single vchi handle Takashi Iwai
2018-09-04 15:58 ` [PATCH 19/29] staging: bcm2835-audio: Code refactoring of vchiq accessor codes Takashi Iwai
2018-09-04 15:58 ` [PATCH 20/29] staging: bcm2835-audio: Operate non-atomic PCM ops Takashi Iwai
2018-09-04 15:58 ` [PATCH 21/29] staging: bcm2835-audio: Use card->private_data Takashi Iwai
2018-09-04 15:58 ` [PATCH 22/29] staging: bcm2835-audio: Use standard error print helpers Takashi Iwai
2018-09-04 15:58 ` [PATCH 23/29] staging: bcm2835-audio: Remove unnecessary header file includes Takashi Iwai
2018-09-04 15:58 ` [PATCH 24/29] staging: bcm2835-audio: Move module parameter description Takashi Iwai
2018-09-04 15:58 ` [PATCH 25/29] staging: bcm2835-audio: Use coherent device buffers Takashi Iwai
2018-09-04 15:58 ` [PATCH 26/29] staging: bcm2835-audio: Set SNDRV_PCM_INFO_SYNC_APPLPTR Takashi Iwai
2018-09-04 15:58 ` [PATCH 27/29] staging: bcm2835-audio: Simplify PCM creation helpers Takashi Iwai
2018-09-04 15:58 ` [PATCH 28/29] staging: bcm2835-audio: Simplify kctl " Takashi Iwai
2018-09-04 15:58 ` [PATCH 29/29] staging: bcm2835-audio: Simplify card object management Takashi Iwai
2018-09-08 13:18 ` [PATCH 00/29] staging: bcm2835-audio: Cleanups and fixes Stefan Wahren
2018-09-08 16:21   ` Takashi Iwai
2018-09-08 17:00     ` Stefan Wahren
2018-09-08 17:16       ` Takashi Iwai
2018-09-10  9:12     ` Greg Kroah-Hartman
2018-09-10  9:16       ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180904155858.8001-2-tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=eric@anholt.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rpi-kernel@lists.infradead.org \
    --cc=stefan.wahren@i2se.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).