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: <20140327010852.GA30768@sirena.org.uk>
Date:	Thu, 27 Mar 2014 01:08:52 +0000
From:	Mark Brown <broonie@...nel.org>
To:	Lars-Peter Clausen <lars@...afoo.de>
Cc:	Arun Shamanna Lakshmi <aruns@...dia.com>, lgirdwood@...il.com,
	swarren@...dotorg.org, Songhee Baek <sbaek@...dia.com>,
	alsa-devel@...a-project.org, tiwai@...e.de,
	linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH] ASoC: Add support for multi register mux

On Wed, Mar 26, 2014 at 08:38:47PM +0100, Lars-Peter Clausen wrote:
> On 03/26/2014 01:02 AM, Arun Shamanna Lakshmi wrote:

> The way you describe this it seems to me that a value array for this kind of
> mux would look like.

> 0x00000000, 0x00000000, 0x00000001
> 0x00000000, 0x00000000, 0x00000002
> 0x00000000, 0x00000000, 0x00000003
> 0x00000000, 0x00000000, 0x00000004
> 0x00000000, 0x00000000, 0x00000008

> That seems to be extremely tedious. If the MUX uses a one hot encoding how
> about storing the index of the bit in the values array and use (1 << value)
> when writing the value to the register?

Or hide it behind utility macros at any rate; I've got this horrible
feeling that as soon as we have this people will notice that they have
more standard enums that are splatted over multiple registers (I think
from memory I've seen them but they got fudged).

> [...]
> >  /* enumerated kcontrol */
> >  struct soc_enum {

> There doesn't actually be any code that is shared between normal enums and
> wide enums. This patch doubles the size of the soc_enum struct, how about
> having a separate struct for wide enums?

Or if they are going to share the same struct then they shouldn't be
duplicating the code and instead keying off num_regs (which was my first
thought earlier on when I saw the separate functions).  We definitely
shouldn't be sharing the data without also sharing the code I think.

> >-	int reg;
> >+	int reg[SOC_ENUM_MAX_REGS];
> >  	unsigned char shift_l;
> >  	unsigned char shift_r;
> >  	unsigned int items;
> >-	unsigned int mask;
> >+	unsigned int mask[SOC_ENUM_MAX_REGS];

> If you make mask and reg pointers instead of arrays this should be much more
> flexible and not be limited to 3 registers.

Right, which pushes towards not sharing.  Though with an arrayified mask
the specification by shift syntax would get to be slightly obscure (is
it relative to the enums or the registers?) so perhaps we don't want to
do that at all if we've got specification by shift.  If we do that then
we could get away with a variable length array at the end of the struct
though I think that may be painful for the static declarations.  Someone
would need to look to see what works

Download attachment "signature.asc" of type "application/pgp-signature" (837 bytes)