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

[Lars-Peter Clausen]

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/