by Maxime Ripard

[permalink] [raw]

Subject: Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls.

On Mon, Jun 26, 2017 at 09:48:59AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic
> <[email protected]> wrote:
> > Since it's now possible to have a DAPM mixer control with multiple channels,
> > use it to cut down the total number of controls.
> >
> > Signed-off-by: Danny Milosavljevic <[email protected]>
> > ---
> > sound/soc/sunxi/sun4i-codec.c | 27 ++++++++++++---------------
> > 1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> > index c69b55c..3718137 100644
> > --- a/sound/soc/sunxi/sun4i-codec.c
> > +++ b/sound/soc/sunxi/sun4i-codec.c
> > @@ -685,15 +685,12 @@ static const struct snd_kcontrol_new sun7i_codec_extra_controls[] = {
> > sun7i_codec_micin_preamp_gain_scale),
> > };
> >
> > -static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> > - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > - SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> > -};
> > -
> > -static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> > - SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
>
> This changes the mixer control names exposed to userspace. While I think
> consolidating mixer controls is nice, giving the user a less cluttered
> view, I'm not sure about the changing a possible userspace ABI.
>
> Maxime, ALSA and ASoC people, any comments about this?

Yeah, breaking userspace is definitely not ok.

Maxime

--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Attachments:

(No filename) (1.91 kB)
signature.asc (801.00 B)
Download all attachments

2017-06-27 06:10:30

On Mon, Jun 26, 2017 at 09:48:59AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic

> > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),

> This changes the mixer control names exposed to userspace. While I think
> consolidating mixer controls is nice, giving the user a less cluttered
> view, I'm not sure about the changing a possible userspace ABI.

> Maxime, ALSA and ASoC people, any comments about this?

It's better not to change things but on the other hand it really depends
if there's any users who will notice - if nobody notices it doesn't
matter.

Can we *please* get some review of the rest of this series? It's
enormous, we're on version *13* and the first review on this version
here is on patch 5. If the first four patches are fine then please says
so, if they're not then let's have some review comments so things can
move towards merging them.

Attachments:

(No filename) (1.10 kB)
signature.asc (488.00 B)
Download all attachments