bcm2835-audio: Backport upstream commits

drivers/staging/vc04_services/bcm2835-audio/TODO

@@ -4,26 +4,7 @@
 *                                                                           *
 *****************************************************************************
 
+1) Revisit multi-cards options and PCM route mixer control (as per comment
+https://lkml.org/lkml/2018/9/8/200)
 
-1) Document the device tree node
-
-The downstream tree(the tree that the driver was imported from) at
-http://www.github.com/raspberrypi/linux uses this node:
-
-audio: audio {
-	compatible = "brcm,bcm2835-audio";
-	brcm,pwm-channels = <8>;
-};
-
-Since the driver requires the use of VCHIQ, it may be useful to have a link
-in the device tree to the VCHIQ driver.
-
-2) Gracefully handle the case where VCHIQ is missing from the device tree or
-it has not been initialized yet.
-
-3) Review error handling and remove duplicate code.
-
-4) Cleanup the logging mechanism.  The driver should probably be using the
-standard kernel logging mechanisms such as dev_info, dev_dbg, and friends.
-
-5) Fix the remaining checkpatch.pl errors and warnings.
+2) Fix the remaining checkpatch.pl errors and warnings.

drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c

@@ -1,23 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright 2011 Broadcom Corporation.  All rights reserved. */
 
-#include <linux/platform_device.h>
-#include <linux/init.h>
-#include <linux/io.h>
-#include <linux/jiffies.h>
-#include <linux/slab.h>
-#include <linux/time.h>
-#include <linux/wait.h>
-#include <linux/delay.h>
-#include <linux/moduleparam.h>
-#include <linux/sched.h>
-
 #include <sound/core.h>
 #include <sound/control.h>
-#include <sound/pcm.h>
-#include <sound/pcm_params.h>
-#include <sound/rawmidi.h>
-#include <sound/initval.h>
 #include <sound/tlv.h>
 #include <sound/asoundef.h>
 
@@ -27,6 +12,21 @@
 #define CTRL_VOL_MAX 400
 #define CTRL_VOL_MIN -10239 /* originally -10240 */
 
+static int bcm2835_audio_set_chip_ctls(struct bcm2835_chip *chip)
+{
+	int i, err = 0;
+
+	/* change ctls for all substreams */
+	for (i = 0; i < MAX_SUBSTREAMS; i++) {
+		if (chip->alsa_stream[i]) {
+			err = bcm2835_audio_set_ctls(chip->alsa_stream[i]);
+			if (err < 0)
+				break;
+		}
+	}
+	return err;
+}
+
 static int snd_bcm2835_ctl_info(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_info *uinfo)
 {
@@ -49,41 +49,15 @@ static int snd_bcm2835_ctl_info(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-/* toggles mute on or off depending on the value of nmute, and returns
- * 1 if the mute value was changed, otherwise 0
- */
-static int toggle_mute(struct bcm2835_chip *chip, int nmute)
-{
-	/* if settings are ok, just return 0 */
-	if (chip->mute == nmute)
-		return 0;
-
-	/* if the sound is muted then we need to unmute */
-	if (chip->mute == CTRL_VOL_MUTE) {
-		chip->volume = chip->old_volume; /* copy the old volume back */
-		audio_info("Unmuting, old_volume = %d, volume = %d ...\n", chip->old_volume, chip->volume);
-	} else /* otherwise we mute */ {
-		chip->old_volume = chip->volume;
-		chip->volume = 26214; /* set volume to minimum level AKA mute */
-		audio_info("Muting, old_volume = %d, volume = %d ...\n", chip->old_volume, chip->volume);
-	}
-
-	chip->mute = nmute;
-	return 1;
-}
-
 static int snd_bcm2835_ctl_get(struct snd_kcontrol *kcontrol,
 			       struct snd_ctl_elem_value *ucontrol)
 {
 	struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
 
-	if (mutex_lock_interruptible(&chip->audio_mutex))
-		return -EINTR;
-
-	BUG_ON(!chip && !(chip->avail_substreams & AVAIL_SUBSTREAMS_MASK));
+	mutex_lock(&chip->audio_mutex);
 
 	if (kcontrol->private_value == PCM_PLAYBACK_VOLUME)
-		ucontrol->value.integer.value[0] = chip2alsa(chip->volume);
+		ucontrol->value.integer.value[0] = chip->volume;
 	else if (kcontrol->private_value == PCM_PLAYBACK_MUTE)
 		ucontrol->value.integer.value[0] = chip->mute;
 	else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE)
@@ -97,79 +71,60 @@ static int snd_bcm2835_ctl_put(struct snd_kcontrol *kcontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
 	struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
+	int val, *valp;
 	int changed = 0;
 
-	if (mutex_lock_interruptible(&chip->audio_mutex))
-		return -EINTR;
-
-	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]);
-		if (chip->mute == CTRL_VOL_MUTE) {
-			/* changed = toggle_mute(chip, CTRL_VOL_UNMUTE); */
-			changed = 1; /* should return 0 to signify no change but the mixer takes this as the opposite sign (no idea why) */
-			goto unlock;
-		}
-		if (changed || (ucontrol->value.integer.value[0] != chip2alsa(chip->volume))) {
-			chip->volume = alsa2chip(ucontrol->value.integer.value[0]);
-			changed = 1;
-		}
-
-	} else if (kcontrol->private_value == PCM_PLAYBACK_MUTE) {
-		/* Now implemented */
-		audio_info(" Mute attempted\n");
-		changed = toggle_mute(chip, ucontrol->value.integer.value[0]);
-
-	} else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE) {
-		if (ucontrol->value.integer.value[0] != chip->dest) {
-			chip->dest = ucontrol->value.integer.value[0];
-			changed = 1;
-		}
+	if (kcontrol->private_value == PCM_PLAYBACK_VOLUME)
+		valp = &chip->volume;
+	else if (kcontrol->private_value == PCM_PLAYBACK_MUTE)
+		valp = &chip->mute;
+	else if (kcontrol->private_value == PCM_PLAYBACK_DEVICE)
+		valp = &chip->dest;
+	else
+		return -EINVAL;
+
+	val = ucontrol->value.integer.value[0];
+	mutex_lock(&chip->audio_mutex);
+	if (val != *valp) {
+		*valp = val;
+		changed = 1;
+		if (bcm2835_audio_set_chip_ctls(chip))
+			dev_err(chip->card->dev, "Failed to set ALSA controls..\n");
 	}
-
-	if (changed && bcm2835_audio_set_ctls(chip))
-		dev_err(chip->card->dev, "Failed to set ALSA controls..\n");
-
-unlock:
 	mutex_unlock(&chip->audio_mutex);
 	return changed;
 }
 
 static DECLARE_TLV_DB_SCALE(snd_bcm2835_db_scale, CTRL_VOL_MIN, 1, 1);
 
-static struct snd_kcontrol_new snd_bcm2835_ctl[] = {
+static const struct snd_kcontrol_new snd_bcm2835_ctl[] = {
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 		.name = "PCM Playback Volume",
-		.index = 0,
 		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_TLV_READ,
 		.private_value = PCM_PLAYBACK_VOLUME,
 		.info = snd_bcm2835_ctl_info,
 		.get = snd_bcm2835_ctl_get,
 		.put = snd_bcm2835_ctl_put,
-		.count = 1,
 		.tlv = {.p = snd_bcm2835_db_scale}
 	},
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 		.name = "PCM Playback Switch",
-		.index = 0,
 		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
 		.private_value = PCM_PLAYBACK_MUTE,
 		.info = snd_bcm2835_ctl_info,
 		.get = snd_bcm2835_ctl_get,
 		.put = snd_bcm2835_ctl_put,
-		.count = 1,
 	},
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 		.name = "PCM Playback Route",
-		.index = 0,
 		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
 		.private_value = PCM_PLAYBACK_DEVICE,
 		.info = snd_bcm2835_ctl_info,
 		.get = snd_bcm2835_ctl_get,
 		.put = snd_bcm2835_ctl_put,
-		.count = 1,
 	},
 };
 
@@ -187,8 +142,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 +159,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);
@@ -237,51 +190,7 @@ static int snd_bcm2835_spdif_mask_get(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-static int snd_bcm2835_spdif_stream_info(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_info *uinfo)
-{
-	uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
-	uinfo->count = 1;
-	return 0;
-}
-
-static int snd_bcm2835_spdif_stream_get(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
-	int i;
-
-	if (mutex_lock_interruptible(&chip->audio_mutex))
-		return -EINTR;
-
-	for (i = 0; i < 4; i++)
-		ucontrol->value.iec958.status[i] =
-		(chip->spdif_status >> (i * 8)) & 0xff;
-
-	mutex_unlock(&chip->audio_mutex);
-	return 0;
-}
-
-static int snd_bcm2835_spdif_stream_put(struct snd_kcontrol *kcontrol,
-	struct snd_ctl_elem_value *ucontrol)
-{
-	struct bcm2835_chip *chip = snd_kcontrol_chip(kcontrol);
-	unsigned int val = 0;
-	int i, change;
-
-	if (mutex_lock_interruptible(&chip->audio_mutex))
-		return -EINTR;
-
-	for (i = 0; i < 4; i++)
-		val |= (unsigned int)ucontrol->value.iec958.status[i] << (i * 8);
-	change = val != chip->spdif_status;
-	chip->spdif_status = val;
-
-	mutex_unlock(&chip->audio_mutex);
-	return change;
-}
-
-static struct snd_kcontrol_new snd_bcm2835_spdif[] = {
+static const struct snd_kcontrol_new snd_bcm2835_spdif[] = {
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
 		.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
@@ -296,39 +205,34 @@ static struct snd_kcontrol_new snd_bcm2835_spdif[] = {
 		.info = snd_bcm2835_spdif_mask_info,
 		.get = snd_bcm2835_spdif_mask_get,
 	},
-	{
-		.access = SNDRV_CTL_ELEM_ACCESS_READWRITE |
-		SNDRV_CTL_ELEM_ACCESS_INACTIVE,
-		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
-		.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, PCM_STREAM),
-		.info = snd_bcm2835_spdif_stream_info,
-		.get = snd_bcm2835_spdif_stream_get,
-		.put = snd_bcm2835_spdif_stream_put,
-	},
 };
 
-int snd_bcm2835_new_ctl(struct bcm2835_chip *chip)
+static int create_ctls(struct bcm2835_chip *chip, size_t size,
+		       const struct snd_kcontrol_new *kctls)
 {
-	int err;
-	unsigned int idx;
+	int i, err;
 
-	strcpy(chip->card->mixername, "Broadcom Mixer");
-	for (idx = 0; idx < ARRAY_SIZE(snd_bcm2835_ctl); idx++) {
-		err = snd_ctl_add(chip->card,
-				  snd_ctl_new1(&snd_bcm2835_ctl[idx], chip));
-		if (err < 0)
-			return err;
-	}
-	for (idx = 0; idx < ARRAY_SIZE(snd_bcm2835_spdif); idx++) {
-		err = snd_ctl_add(chip->card,
-				  snd_ctl_new1(&snd_bcm2835_spdif[idx], chip));
+	for (i = 0; i < size; i++) {
+		err = snd_ctl_add(chip->card, snd_ctl_new1(&kctls[i], chip));
 		if (err < 0)
 			return err;
 	}
 	return 0;
 }
 
-static struct snd_kcontrol_new snd_bcm2835_headphones_ctl[] = {
+int snd_bcm2835_new_ctl(struct bcm2835_chip *chip)
+{
+	int err;
+
+	strcpy(chip->card->mixername, "Broadcom Mixer");
+	err = create_ctls(chip, ARRAY_SIZE(snd_bcm2835_ctl), snd_bcm2835_ctl);
+	if (err < 0)
+		return err;
+	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_spdif),
+			   snd_bcm2835_spdif);
+}
+
+static const struct snd_kcontrol_new snd_bcm2835_headphones_ctl[] = {
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 		.name = "Headphone Playback Volume",
@@ -357,21 +261,12 @@ static struct snd_kcontrol_new snd_bcm2835_headphones_ctl[] = {
 
 int snd_bcm2835_new_headphones_ctl(struct bcm2835_chip *chip)
 {
-	int err;
-	unsigned int idx;
-
 	strcpy(chip->card->mixername, "Broadcom Mixer");
-	for (idx = 0; idx < ARRAY_SIZE(snd_bcm2835_headphones_ctl); idx++) {
-		err = snd_ctl_add(chip->card,
-				  snd_ctl_new1(&snd_bcm2835_headphones_ctl[idx],
-					       chip));
-		if (err)
-			return err;
-	}
-	return 0;
+	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_headphones_ctl),
+			   snd_bcm2835_headphones_ctl);
 }
 
-static struct snd_kcontrol_new snd_bcm2835_hdmi[] = {
+static const struct snd_kcontrol_new snd_bcm2835_hdmi[] = {
 	{
 		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
 		.name = "HDMI Playback Volume",
@@ -400,16 +295,8 @@ static struct snd_kcontrol_new snd_bcm2835_hdmi[] = {
 
 int snd_bcm2835_new_hdmi_ctl(struct bcm2835_chip *chip)
 {
-	int err;
-	unsigned int idx;
-
 	strcpy(chip->card->mixername, "Broadcom Mixer");
-	for (idx = 0; idx < ARRAY_SIZE(snd_bcm2835_hdmi); idx++) {
-		err = snd_ctl_add(chip->card,
-				  snd_ctl_new1(&snd_bcm2835_hdmi[idx], chip));
-		if (err)
-			return err;
-	}
-	return 0;
+	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_hdmi),
+			   snd_bcm2835_hdmi);
 }