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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
