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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <533957C8.2030105@metafoo.de>
Date:	Mon, 31 Mar 2014 13:55:52 +0200
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Mark Brown <broonie@...nel.org>
CC:	Arun Shamanna Lakshmi <aruns@...dia.com>,
	Songhee Baek <sbaek@...dia.com>,
	"'alsa-devel@...a-project.org'" <alsa-devel@...a-project.org>,
	"'swarren@...dotorg.org'" <swarren@...dotorg.org>,
	"'tiwai@...e.de'" <tiwai@...e.de>,
	"'lgirdwood@...il.com'" <lgirdwood@...il.com>,
	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/