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

[Songhee Baek]

> -----Original Message-----
> From: alsa-devel-bounces at alsa-project.org [mailto:alsa-devel-bounces at 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 at alsa-project.org;
> swarren at wwwdotorg.org; tiwai at suse.de; lgirdwood at gmail.com; linux-
> kernel at 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