From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752702AbaDCPGf (ORCPT ); Thu, 3 Apr 2014 11:06:35 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39298 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752574AbaDCPGa (ORCPT ); Thu, 3 Apr 2014 11:06:30 -0400 Date: Thu, 03 Apr 2014 17:06:28 +0200 Message-ID: From: Takashi Iwai To: Lars-Peter Clausen Cc: Mark Brown , Arun Shamanna Lakshmi , lgirdwood@gmail.com, swarren@wwwdotorg.org, perex@perex.cz, alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org, Songhee Baek Subject: Re: [PATCH] ASoC: dapm: Add support for multi register mux In-Reply-To: <533D62CE.5040907@metafoo.de> References: <1396494710-20944-1-git-send-email-aruns@nvidia.com> <20140403095330.GS14763@sirena.org.uk> <533D62CE.5040907@metafoo.de> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.3 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org At Thu, 03 Apr 2014 15:31:58 +0200, Lars-Peter Clausen wrote: > > On 04/03/2014 11:53 AM, Mark Brown wrote: > > On Thu, Apr 03, 2014 at 11:47:15AM +0200, Takashi Iwai wrote: > > > >> I'm a bit late in the game, but I feel a bit uneasy through looking > >> at the whole changes. My primary question is, whether do we really > >> need to share the same struct soc_enum for the onehot type? What > >> makes hard to use a struct soc_enum_onehot for them? You need > >> different individual get/put for each type. We may still need to > >> change soc_dapm_update stuff, but it's different from sharing > >> soc_enum. > > > > Indeed, I had thought this was where the discussion was heading - not > > looked at this version of the patch yet. > > > > It would be nice, but it also requires some slight restructuring. The issue > we have right now is that there is strictly speaking a bit of a layering > violation. The DAPM widgets should not need to know how the kcontrols that > are attached to the widget do their IO. What we essentially do in > dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of > the controls get handler. Replacing that by calling the get handler instead > should allow us to use different structs for enums and onehot enums. So, something like below? It's totally untested, just a proof of concept. If the performance matters, we can optimize it by checking kcontrol->get == snd_soc_dapm_get_enum_double or kcontrol->get == snd_soc_dapm_get_volsw and bypass to the current open-code functions instead of the generic get/put callers. thanks, Takashi --- diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index c8a780d0d057..5947c6e2fcc8 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -508,64 +508,71 @@ out: static int dapm_connect_mux(struct snd_soc_dapm_context *dapm, struct snd_soc_dapm_widget *src, struct snd_soc_dapm_widget *dest, struct snd_soc_dapm_path *path, const char *control_name, - const struct snd_kcontrol_new *kcontrol) + struct snd_kcontrol *kcontrol) { - struct soc_enum *e = (struct soc_enum *)kcontrol->private_value; - unsigned int val, item; - int i; + struct snd_ctl_elem_info *uinfo; + struct snd_ctl_elem_value *ucontrol; + unsigned int i, item, items; + int err; - if (e->reg != SND_SOC_NOPM) { - soc_widget_read(dest, e->reg, &val); - val = (val >> e->shift_l) & e->mask; - item = snd_soc_enum_val_to_item(e, val); - } else { - /* since a virtual mux has no backing registers to - * decide which path to connect, it will try to match - * with the first enumeration. This is to ensure - * that the default mux choice (the first) will be - * correctly powered up during initialization. - */ - item = 0; + uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL); + ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL); + if (!uinfo || !ucontrol) { + err = -ENOMEM; + goto out; + } + + err = kcontrol->info(kcontrol, uinfo); + if (err < 0) + goto out; + if (WARN_ON(uinfo->type != SNDRV_CTL_ELEM_TYPE_ENUMERATED)) { + err = -EINVAL; + goto out; } + items = uinfo->value.enumerated.items; - for (i = 0; i < e->items; i++) { - if (!(strcmp(control_name, e->texts[i]))) { + err = kcontrol->get(kcontrol, ucontrol); + if (err < 0) + goto out; + item = ucontrol->value.enumerated.item[0]; + + for (i = 0; i < items; i++) { + uinfo->value.enumerated.item = i; + err = kcontrol->info(kcontrol, uinfo); + if (err < 0) + goto out; + if (!(strcmp(control_name, uinfo->value.enumerated.name))) { list_add(&path->list, &dapm->card->paths); list_add(&path->list_sink, &dest->sources); list_add(&path->list_source, &src->sinks); - path->name = (char*)e->texts[i]; + path->name = control_name; if (i == item) path->connect = 1; else path->connect = 0; - return 0; + goto out; } } - return -ENODEV; + err = -ENODEV; + out: + kfree(ucontrol); + kfree(uinfo); + return err < 0 ? err : 0; } /* set up initial codec paths */ static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w, struct snd_soc_dapm_path *p, int i) { - struct soc_mixer_control *mc = (struct soc_mixer_control *) - w->kcontrol_news[i].private_value; - unsigned int reg = mc->reg; - unsigned int shift = mc->shift; - unsigned int max = mc->max; - unsigned int mask = (1 << fls(max)) - 1; - unsigned int invert = mc->invert; - unsigned int val; + struct snd_kcontrol *kcontrol = w->kcontrols[i]; + struct snd_ctl_elem_value *ucontrol = + kzalloc(sizeof(*ucontrol), GFP_KERNEL); - if (reg != SND_SOC_NOPM) { - soc_widget_read(w, reg, &val); - val = (val >> shift) & mask; - if (invert) - val = max - val; - p->connect = !!val; - } else { - p->connect = 0; + if (ucontrol) { + if (kcontrol->get(kcontrol, ucontrol) >= 0) + p->connect = !!ucontrol->value.integer.value[0]; + kfree(ucontrol); } } @@ -2415,7 +2422,7 @@ static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm, return 0; case snd_soc_dapm_mux: ret = dapm_connect_mux(dapm, wsource, wsink, path, control, - &wsink->kcontrol_news[0]); + wsink->kcontrols[0]); if (ret != 0) goto err; break;