-----Original Message----- From: alsa-devel-bounces@alsa-project.org [mailto:alsa-devel-bounces@alsa- project.org] On Behalf Of Mark Brown Sent: Wednesday, March 26, 2014 6:09 PM To: Lars-Peter Clausen Cc: Songhee Baek; Arun Shamanna Lakshmi; alsa-devel@alsa-project.org; swarren@wwwdotorg.org; tiwai@suse.de; lgirdwood@gmail.com; linux- kernel@vger.kernel.org Subject: Re: [alsa-devel] [PATCH] ASoC: Add support for multi register mux
- PGP Signed by an unknown key
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
Making a separate soc_enum_wide is a better way compared to using soc_enum for this use case. If we add a separate soc_enum_wide, we need to update the following functions : dapm_connect_mux, soc_dapm_mux_update_power snd_soc_dapm_mux_update_power These functions are using texts field in soc_enum struct, I think that we can pass texts pointer instead of soc_enum struct pointer. I want to know whether it is Ok to do this.
- Unknown Key
- 0x7EA229BD