Re: [alsa-devel] [PATCH] ASoC: Add support for multi register mux

From: Lars-Peter Clausen
Date: Mon Mar 31 2014 - 07:55:09 EST


On 03/31/2014 01:21 PM, Mark Brown wrote:
On Sat, Mar 29, 2014 at 11:12:30PM -0700, Arun Shamanna Lakshmi wrote:

Fix your mailer to word wrap within paragraphs, your mails are
excessively hard to read.

I'm not sure I understand how that MUX_OFFSET would work. To get the
selected mux output you can use the ffs instruction.

foreach(reg) {
reg_val = read(reg) & mask;
if (reg_val != 0) {
val = __ffs(reg_val);
break;
}
}

There are 2 options to do this. The first option is what you specified
above, in which case I think we cannot share get and put functions as
they use the reg_val directly inside snd_soc_enum_val_to_item API (not
the bit position being set). If we change to bit position like above,
then the current users of these APIs should also change their soc_enum
value table. And, the second option being the one that we proposed.

Sharing the functions isn't the goal, coming up with a usable API is.

That being said, MUX_OFFSET which is the second option works in the
following way. We know that reg_val is a power of 2 (2^0 to 2^31)
which is one hot code. This method adds a unique offset for this
reg_val for each incremental register that we want to set (say 2^n +
MUX_OFFSET(reg_id)) inside get function and does the reverse of it in
put function. For current users of only one register, it doesn't
change anything as we use reg_val.

I'm afraid I can't understand the above at all, sorry. The code below
is quoted like Lars wrote it but I think it's actually written by you,
please check your quoting when replying:\

if (e->reg[0] != SND_SOC_NOPM) {
for (reg_idx = 0; reg_idx < e->num_regs; reg_idx++) {
reg_val = snd_soc_read(codec, e->reg[reg_idx]);
val = (reg_val >> e->shift_l) & e->mask[reg_idx];
if (val) {
val += MULTI_MUX_INPUT_OFFSET(reg_idx);
break;
}
}
} else {
reg_val = dapm_kcontrol_get_value(kcontrol);
val = (reg_val >> e->shift_l) & e->mask[0];
}

The above is a bit confusing... partly this is because of a lack of
context (what is MULTI_MUX_INPUT_OFFSET?) and partly because it isn't
entirely obvious that stopping as soon as we see any value set is the
right choice, especially given the addition to rather than setting of
val.

I think the idea is that since we know that for one-hot encodings only powers of two are valid values the other bits are used to encode the register number. E.g 0x4 means bit 3 in register 0, 0x5 means bit 3 in register 1, 0x6 means bit 3 in register 2 and so on. I guess it is possible to make it work. But this seems to be quite hack-ish to me. You'd have to be careful that MULTI_MUX_INPUT_OFFSET(reg_idx) never evaluates to a power of two and there are probably some more pitfalls.

- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/