Re: [PATCH] staging: bcm2835-audio: remove compat ALSA card

Stefan Wahren <stefan.wahren@xxxxxxxx> · Fri, 8 Apr 2022 23:00:52 +0200

Hi Adrien,

[ add Maxime and Takashi ]

Am 08.04.22 um 17:03 schrieb Adrien Thierry:
Remove compat ALSA card, which has overlapping functionality with the
two other cards described by the driver (HDMI and headphones)

This handles TODO item "Revisit multi-cards options and PCM route mixer
control".

Move the S/PDIF device that was part of the compat ALSA card to the HDMI
card.

Only enable headphones card by default, because HDMI breaks when using
both vc4 and bcm2835-audio with HDMI card enabled.

Signed-off-by: Adrien Thierry <athierry@xxxxxxxxxx>
---

I was able to test PCM output on 3.5mm and HDMI on a Raspberry Pi 4.
However, I couldn't test the HDMI S/PDIF device since I don't own a S/PDIF
capable speaker.

thanks for taking care of this issue. Unfortunately i cannot give a 
technical review to this patch. So hoping Maxime or Takashi can do ...

Best regards


  .../vc04_services/bcm2835-audio/bcm2835-ctl.c | 86 ++-----------------
  .../vc04_services/bcm2835-audio/bcm2835.c     | 33 ++-----
  2 files changed, 15 insertions(+), 104 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
index 3703409715da..1c1f040122d7 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835-ctl.c
@@ -117,15 +117,6 @@ static const struct snd_kcontrol_new snd_bcm2835_ctl[] = {
  		.get = snd_bcm2835_ctl_get,
  		.put = snd_bcm2835_ctl_put,
  	},
-	{
-		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
-		.name = "PCM Playback Route",
-		.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,
-	},
  };
  
  static int snd_bcm2835_spdif_default_info(struct snd_kcontrol *kcontrol,
@@ -220,7 +211,14 @@ static int create_ctls(struct bcm2835_chip *chip, size_t size,
  	return 0;
  }
  
-int snd_bcm2835_new_ctl(struct bcm2835_chip *chip)
+int snd_bcm2835_new_headphones_ctl(struct bcm2835_chip *chip)
+{
+	strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername));
+	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_ctl),
+			   snd_bcm2835_ctl);
+}
+
+int snd_bcm2835_new_hdmi_ctl(struct bcm2835_chip *chip)
  {
  	int err;
  
@@ -232,71 +230,3 @@ int snd_bcm2835_new_ctl(struct bcm2835_chip *chip)
  			   snd_bcm2835_spdif);
  }
  
-static const struct snd_kcontrol_new snd_bcm2835_headphones_ctl[] = {
-	{
-		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
-		.name = "Headphone 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 = "Headphone 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,
-	}
-};
-
-int snd_bcm2835_new_headphones_ctl(struct bcm2835_chip *chip)
-{
-	strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername));
-	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_headphones_ctl),
-			   snd_bcm2835_headphones_ctl);
-}
-
-static const struct snd_kcontrol_new snd_bcm2835_hdmi[] = {
-	{
-		.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
-		.name = "HDMI 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 = "HDMI 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,
-	}
-};
-
-int snd_bcm2835_new_hdmi_ctl(struct bcm2835_chip *chip)
-{
-	strscpy(chip->card->mixername, "Broadcom Mixer", sizeof(chip->card->mixername));
-	return create_ctls(chip, ARRAY_SIZE(snd_bcm2835_hdmi),
-			   snd_bcm2835_hdmi);
-}
-
diff --git a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
index 628732d7bf6a..00bc898b0189 100644
--- a/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
+++ b/drivers/staging/vc04_services/bcm2835-audio/bcm2835.c
@@ -10,17 +10,13 @@
  #include "bcm2835.h"
  
  static bool enable_hdmi;
-static bool enable_headphones;
-static bool enable_compat_alsa = true;
+static bool enable_headphones = true;
  static int num_channels = MAX_SUBSTREAMS;
  
  module_param(enable_hdmi, bool, 0444);
  MODULE_PARM_DESC(enable_hdmi, "Enables HDMI virtual audio device");
  module_param(enable_headphones, bool, 0444);
  MODULE_PARM_DESC(enable_headphones, "Enables Headphones virtual audio device");
-module_param(enable_compat_alsa, bool, 0444);
-MODULE_PARM_DESC(enable_compat_alsa,
-		 "Enables ALSA compatibility virtual audio device");
  module_param(num_channels, int, 0644);
  MODULE_PARM_DESC(num_channels, "Number of audio channels (default: 8)");
  
@@ -63,19 +59,20 @@ struct bcm2835_audio_driver {
  	enum snd_bcm2835_route route;
  };
  
-static int bcm2835_audio_alsa_newpcm(struct bcm2835_chip *chip,
+static int bcm2835_audio_dual_newpcm(struct bcm2835_chip *chip,
  				     const char *name,
  				     enum snd_bcm2835_route route,
  				     u32 numchannels)
  {
  	int err;
  
-	err = snd_bcm2835_new_pcm(chip, "bcm2835 ALSA", 0, AUDIO_DEST_AUTO,
-				  numchannels - 1, false);
+	err = snd_bcm2835_new_pcm(chip, name, 0, route,
+				  numchannels, false);
+
  	if (err)
  		return err;
  
-	err = snd_bcm2835_new_pcm(chip, "bcm2835 IEC958/HDMI", 1, 0, 1, true);
+	err = snd_bcm2835_new_pcm(chip, "IEC958", 1, route, 1, true);
  	if (err)
  		return err;
  
@@ -90,18 +87,6 @@ static int bcm2835_audio_simple_newpcm(struct bcm2835_chip *chip,
  	return snd_bcm2835_new_pcm(chip, name, 0, route, numchannels, false);
  }
  
-static struct bcm2835_audio_driver bcm2835_audio_alsa = {
-	.driver = {
-		.name = "bcm2835_alsa",
-		.owner = THIS_MODULE,
-	},
-	.shortname = "bcm2835 ALSA",
-	.longname  = "bcm2835 ALSA",
-	.minchannels = 2,
-	.newpcm = bcm2835_audio_alsa_newpcm,
-	.newctl = snd_bcm2835_new_ctl,
-};
-
  static struct bcm2835_audio_driver bcm2835_audio_hdmi = {
  	.driver = {
  		.name = "bcm2835_hdmi",
@@ -110,7 +95,7 @@ static struct bcm2835_audio_driver bcm2835_audio_hdmi = {
  	.shortname = "bcm2835 HDMI",
  	.longname  = "bcm2835 HDMI",
  	.minchannels = 1,
-	.newpcm = bcm2835_audio_simple_newpcm,
+	.newpcm = bcm2835_audio_dual_newpcm,
  	.newctl = snd_bcm2835_new_hdmi_ctl,
  	.route = AUDIO_DEST_HDMI
  };
@@ -134,10 +119,6 @@ struct bcm2835_audio_drivers {
  };
  
  static struct bcm2835_audio_drivers children_devices[] = {
-	{
-		.audio_driver = &bcm2835_audio_alsa,
-		.is_enabled = &enable_compat_alsa,
-	},
  	{
  		.audio_driver = &bcm2835_audio_hdmi,
  		.is_enabled = &enable_hdmi,

base-commit: 1831fed559732b132aef0ea8261ac77e73f7eadf







