Messages in this thread | | | Date | Mon, 31 Mar 2014 13:55:52 +0200 | From | Lars-Peter Clausen <> | Subject | Re: [alsa-devel] [PATCH] ASoC: Add support for multi register mux |
| |
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
| |