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

From: Takashi Iwai
Date: Tue Apr 12 2022 - 10:58:27 EST

Next message: Patrik Jakobsson: "Re: [PATCH v2] drm/gma500: fix a potential repeat execution in psb_driver_load"
Previous message: Joe Perches: "Re: [Patch net-next 3/3] MAINTAINERS: Add maintainers for Microchip T1 Phy driver"
In reply to: Stefan Wahren: "Re: [PATCH] staging: bcm2835-audio: remove compat ALSA card"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Fri, 08 Apr 2022 23:00:52 +0200,
Stefan Wahren wrote:
>
> 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 ...

I have no longer RPi around me, so cannot actually test, but the code
changes (mostly dropping the superfluous part) looks OK.

The concern is, though, about the compatibility with the already
running setup. But vc04 is still in staging, and that can be an
excuse for the possible breakage, I suppose.

thanks,

Takashi

>
> 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
>

Next message: Patrik Jakobsson: "Re: [PATCH v2] drm/gma500: fix a potential repeat execution in psb_driver_load"
Previous message: Joe Perches: "Re: [Patch net-next 3/3] MAINTAINERS: Add maintainers for Microchip T1 Phy driver"
In reply to: Stefan Wahren: "Re: [PATCH] staging: bcm2835-audio: remove compat ALSA card"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]