diff mbox

[v4,05/12] ASoC: Intel: mrfld: add bytes control for modules

Message ID 1407145563-1303-6-git-send-email-subhransu.s.prusty@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Subhransu S. Prusty Aug. 4, 2014, 9:45 a.m. UTC
From: Vinod Koul <vinod.koul@intel.com>

This patch add support for various modules like eq etc for mrfld DSP. All these
modules will be exposed to usermode as bytes controls.

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
---
 sound/soc/intel/sst-atom-controls.c | 172 ++++++++++++++++++++++++++++++++++++
 sound/soc/intel/sst-atom-controls.h | 130 +++++++++++++++++++++++++++
 2 files changed, 302 insertions(+)

Comments

Mark Brown Aug. 13, 2014, 8 p.m. UTC | #1
On Mon, Aug 04, 2014 at 03:15:56PM +0530, Subhransu S. Prusty wrote:

> From: Vinod Koul <vinod.koul@intel.com>

> This patch add support for various modules like eq etc for mrfld DSP. All these
> modules will be exposed to usermode as bytes controls.

Indeed they are actually exposed by this code.

> +static inline void sst_fill_byte_control(char *param,
> +					 u8 ipc_msg, u8 block,
> +					 u8 task_id, u8 pipe_id,
> +					 u16 len, void *cmd_data)

Let the compiler figure out if this should be inline, this doesn't seem
something that should obviously be inline and isn't in a header.

> +	if (len > SST_MAX_BIN_BYTES - sizeof(*byte_data)) {
> +		WARN(1, "%s: command length too big (%u)", __func__, len); /* this happens only if code is wrong */
> +		len = SST_MAX_BIN_BYTES - sizeof(*byte_data);
> +	}

Coding style, 80 columns.  Since the only caller can return an error
code you could do that here too.

> +static int sst_fill_and_send_cmd_unlocked(struct sst_data *drv,
> +				 u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id,
> +				 void *cmd_data, u16 len)
> +{
> +	sst_fill_byte_control(drv->byte_stream, ipc_msg, block, task_id, pipe_id,
> +			      len, cmd_data);
> +	return sst->ops->send_byte_stream(sst->dev,
> +			(struct snd_sst_bytes_v2 *)drv->byte_stream);
> +}

There's a lot of casting going on in this code.

> +static int sst_algo_bytes_ctl_info(struct snd_kcontrol *kcontrol,
> +			    struct snd_ctl_elem_info *uinfo)
> +{
> +	struct sst_algo_control *bc = (void *)kcontrol->private_value;
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
> +	uinfo->count = bc->max;
> +
> +	/* allocate space to cache the algo parameters in the driver */
> +	if (bc->params == NULL) {
> +		bc->params = devm_kzalloc(component->dev, bc->max, GFP_KERNEL);
> +		if (bc->params == NULL)
> +			return -ENOMEM;
> +	}
> +	return 0;
> +}

I wouldn't expect an info call to be allocating anything - why is it
doing that?  It's not looking at the alocated data except to see if the
allocation succeeded.  What happens if someone manages to do a get or
set without having first done an info and why aren't we doing any
allocation on initialisation?

> +	case SST_ALGO_BYPASS:
> +		ucontrol->value.integer.value[0] = bc->bypass ? 1 : 0;
> +		pr_debug("%s: bypass  %d\n", __func__, bc->bypass);
> +		break;

Is bypass not a boolean value already, and shouldn't these just end up
with controls called something "Switch" - they look to be just directly
usable switches?

> +	default:
> +		pr_err("Invalid Input- algo type:%d\n", bc->type);

dev_err().

> +	switch (bc->type) {
> +	case SST_ALGO_PARAMS:
> +		if (bc->params)
> +			memcpy(bc->params, ucontrol->value.bytes.data, bc->max);
> +		break;

So if bc->params didn't get allocated somehow we just silently drop the
set?

> +	/*if pipe is enabled, need to send the algo params from here */
> +	if (bc->w && bc->w->power)
> +		sst_send_algo_cmd(drv, bc);

sst_send_algo_cmd() should be returning an eror code (it doesn't but the
functions it calls can).

I'm not seeing any code to restore these controls on power up here.
Subhransu S. Prusty Aug. 18, 2014, 5:36 a.m. UTC | #2
On Wed, Aug 13, 2014 at 09:00:12PM +0100, Mark Brown wrote:
> On Mon, Aug 04, 2014 at 03:15:56PM +0530, Subhransu S. Prusty wrote:
> 
> > From: Vinod Koul <vinod.koul@intel.com>
> 
> > This patch add support for various modules like eq etc for mrfld DSP. All these
> > modules will be exposed to usermode as bytes controls.
> 
> Indeed they are actually exposed by this code.
> 
> > +static inline void sst_fill_byte_control(char *param,
> > +					 u8 ipc_msg, u8 block,
> > +					 u8 task_id, u8 pipe_id,
> > +					 u16 len, void *cmd_data)
> 
> Let the compiler figure out if this should be inline, this doesn't seem
> something that should obviously be inline and isn't in a header.
Ok
> 
> > +	if (len > SST_MAX_BIN_BYTES - sizeof(*byte_data)) {
> > +		WARN(1, "%s: command length too big (%u)", __func__, len); /* this happens only if code is wrong */
> > +		len = SST_MAX_BIN_BYTES - sizeof(*byte_data);
> > +	}
> 
> Coding style, 80 columns.  Since the only caller can return an error
> code you could do that here too.
Yes.
> 
> > +static int sst_fill_and_send_cmd_unlocked(struct sst_data *drv,
> > +				 u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id,
> > +				 void *cmd_data, u16 len)
> > +{
> > +	sst_fill_byte_control(drv->byte_stream, ipc_msg, block, task_id, pipe_id,
> > +			      len, cmd_data);
> > +	return sst->ops->send_byte_stream(sst->dev,
> > +			(struct snd_sst_bytes_v2 *)drv->byte_stream);
> > +}
> 
> There's a lot of casting going on in this code.
Not required. Should have been removed.
> 
> > +static int sst_algo_bytes_ctl_info(struct snd_kcontrol *kcontrol,
> > +			    struct snd_ctl_elem_info *uinfo)
> > +{
> > +	struct sst_algo_control *bc = (void *)kcontrol->private_value;
> > +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> > +
> > +	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
> > +	uinfo->count = bc->max;
> > +
> > +	/* allocate space to cache the algo parameters in the driver */
> > +	if (bc->params == NULL) {
> > +		bc->params = devm_kzalloc(component->dev, bc->max, GFP_KERNEL);
> > +		if (bc->params == NULL)
> > +			return -ENOMEM;
> > +	}
> > +	return 0;
> > +}
> 
> I wouldn't expect an info call to be allocating anything - why is it
> doing that?  It's not looking at the alocated data except to see if the
> allocation succeeded.  What happens if someone manages to do a get or
> set without having first done an info and why aren't we doing any
> allocation on initialisation?

Will move after control initialization.
> 
> > +	case SST_ALGO_BYPASS:
> > +		ucontrol->value.integer.value[0] = bc->bypass ? 1 : 0;
> > +		pr_debug("%s: bypass  %d\n", __func__, bc->bypass);
> > +		break;
> 
> Is bypass not a boolean value already, and shouldn't these just end up
> with controls called something "Switch" - they look to be just directly
> usable switches?
Actually BYPASS is not required. Will remove.
> 
> > +	default:
> > +		pr_err("Invalid Input- algo type:%d\n", bc->type);
> 
> dev_err().
We are using pr_err() in all the error conditions for our driver. If it is
really required to change, we can submit a single patch to change to
dev_err() once the patches are merged.
> 
> > +	switch (bc->type) {
> > +	case SST_ALGO_PARAMS:
> > +		if (bc->params)
> > +			memcpy(bc->params, ucontrol->value.bytes.data, bc->max);
> > +		break;
> 
> So if bc->params didn't get allocated somehow we just silently drop the
> set?
With above params initialization changes this check will not be required any
more.
> 
> > +	/*if pipe is enabled, need to send the algo params from here */
> > +	if (bc->w && bc->w->power)
> > +		sst_send_algo_cmd(drv, bc);
> 
> sst_send_algo_cmd() should be returning an eror code (it doesn't but the
> functions it calls can).
> 
> I'm not seeing any code to restore these controls on power up here.

Will take care of the return code.

sst_find_and_send_pipe_algo takes care of restoring the controls on power
up.
Subhransu S. Prusty Aug. 18, 2014, 10:15 a.m. UTC | #3
On Mon, Aug 18, 2014 at 11:06:09AM +0530, Subhransu S. Prusty wrote:
> On Wed, Aug 13, 2014 at 09:00:12PM +0100, Mark Brown wrote:
> > On Mon, Aug 04, 2014 at 03:15:56PM +0530, Subhransu S. Prusty wrote:
> > 
> > > From: Vinod Koul <vinod.koul@intel.com>
> > 
> > > +static int sst_algo_bytes_ctl_info(struct snd_kcontrol *kcontrol,
> > > +			    struct snd_ctl_elem_info *uinfo)
> > > +{
> > > +	struct sst_algo_control *bc = (void *)kcontrol->private_value;
> > > +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> > > +
> > > +	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
> > > +	uinfo->count = bc->max;
> > > +
> > > +	/* allocate space to cache the algo parameters in the driver */
> > > +	if (bc->params == NULL) {
> > > +		bc->params = devm_kzalloc(component->dev, bc->max, GFP_KERNEL);
> > > +		if (bc->params == NULL)
> > > +			return -ENOMEM;
> > > +	}
> > > +	return 0;
> > > +}
> > 
> > I wouldn't expect an info call to be allocating anything - why is it
> > doing that?  It's not looking at the alocated data except to see if the
> > allocation succeeded.  What happens if someone manages to do a get or
> > set without having first done an info and why aren't we doing any
> > allocation on initialisation?
> 
> Will move after control initialization.
As I have to initialize the params, and if it's done in probe after
control creation, it's required to iterate over the controls list and
match certain pattern to find the control and then initialize. This doesn't
look elegant. Instead I would prefer adding an init callback in the control
to do this operation. What do you recommend?
Mark Brown Aug. 18, 2014, 2:19 p.m. UTC | #4
On Mon, Aug 18, 2014 at 11:06:09AM +0530, Subhransu S. Prusty wrote:
> On Wed, Aug 13, 2014 at 09:00:12PM +0100, Mark Brown wrote:

> > > +	default:
> > > +		pr_err("Invalid Input- algo type:%d\n", bc->type);

> > dev_err().

> We are using pr_err() in all the error conditions for our driver. If it is
> really required to change, we can submit a single patch to change to
> dev_err() once the patches are merged.

Better yet, submit the fix first.  It's definitely better practice not
to use pr_ when you have a device.
Mark Brown Aug. 18, 2014, 2:25 p.m. UTC | #5
On Mon, Aug 18, 2014 at 03:45:15PM +0530, Subhransu S. Prusty wrote:
> On Mon, Aug 18, 2014 at 11:06:09AM +0530, Subhransu S. Prusty wrote:

> > > I wouldn't expect an info call to be allocating anything - why is it
> > > doing that?  It's not looking at the alocated data except to see if the
> > > allocation succeeded.  What happens if someone manages to do a get or
> > > set without having first done an info and why aren't we doing any
> > > allocation on initialisation?

> > Will move after control initialization.

> As I have to initialize the params, and if it's done in probe after
> control creation, it's required to iterate over the controls list and
> match certain pattern to find the control and then initialize. This doesn't
> look elegant. Instead I would prefer adding an init callback in the control
> to do this operation. What do you recommend?

Something like that sounds reasonable, or doing it when writing to the
control - the point is that info() is a terrible place to do this for
the reasons outline above.
diff mbox

Patch

diff --git a/sound/soc/intel/sst-atom-controls.c b/sound/soc/intel/sst-atom-controls.c
index ace3c4a59b14..9f41741ddf8a 100644
--- a/sound/soc/intel/sst-atom-controls.c
+++ b/sound/soc/intel/sst-atom-controls.c
@@ -25,6 +25,176 @@ 
 #include "sst-mfld-platform.h"
 #include "sst-atom-controls.h"
 
+static inline void sst_fill_byte_control(char *param,
+					 u8 ipc_msg, u8 block,
+					 u8 task_id, u8 pipe_id,
+					 u16 len, void *cmd_data)
+{
+
+	struct snd_sst_bytes_v2 *byte_data = (struct snd_sst_bytes_v2 *)param;
+
+	byte_data->type = SST_CMD_BYTES_SET;
+	byte_data->ipc_msg = ipc_msg;
+	byte_data->block = block;
+	byte_data->task_id = task_id;
+	byte_data->pipe_id = pipe_id;
+
+	if (len > SST_MAX_BIN_BYTES - sizeof(*byte_data)) {
+		WARN(1, "%s: command length too big (%u)", __func__, len); /* this happens only if code is wrong */
+		len = SST_MAX_BIN_BYTES - sizeof(*byte_data);
+	}
+	byte_data->len = len;
+	memcpy(byte_data->bytes, cmd_data, len);
+	print_hex_dump_bytes("writing to lpe: ", DUMP_PREFIX_OFFSET,
+			     byte_data, len + sizeof(*byte_data));
+}
+
+static int sst_fill_and_send_cmd_unlocked(struct sst_data *drv,
+				 u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id,
+				 void *cmd_data, u16 len)
+{
+	sst_fill_byte_control(drv->byte_stream, ipc_msg, block, task_id, pipe_id,
+			      len, cmd_data);
+	return sst->ops->send_byte_stream(sst->dev,
+			(struct snd_sst_bytes_v2 *)drv->byte_stream);
+}
+
+/**
+ * sst_fill_and_send_cmd - generate the IPC message and send it to the FW
+ * @ipc_msg:	type of IPC (CMD, SET_PARAMS, GET_PARAMS)
+ * @cmd_data:	the IPC payload
+ */
+static int sst_fill_and_send_cmd(struct sst_data *drv,
+				 u8 ipc_msg, u8 block, u8 task_id, u8 pipe_id,
+				 void *cmd_data, u16 len)
+{
+	int ret;
+
+	mutex_lock(&drv->lock);
+	ret = sst_fill_and_send_cmd_unlocked(drv, ipc_msg, block, task_id, pipe_id,
+					     cmd_data, len);
+	mutex_unlock(&drv->lock);
+
+	return ret;
+}
+
+static void sst_send_algo_cmd(struct sst_data *drv,
+			      struct sst_algo_control *bc)
+{
+	int len;
+	struct sst_cmd_set_params *cmd;
+
+	if (bc->params == NULL)
+		return;
+
+	/* bc->max includes sizeof algos + length field */
+	len = sizeof(cmd->dst) + sizeof(cmd->command_id) + bc->max;
+
+	cmd = kzalloc(len, GFP_KERNEL);
+	if (cmd == NULL)
+		return;
+
+	SST_FILL_DESTINATION(2, cmd->dst, bc->pipe_id, bc->module_id);
+	cmd->command_id = bc->cmd_id;
+	memcpy(cmd->params, bc->params, bc->max);
+
+	sst_fill_and_send_cmd_unlocked(drv, SST_IPC_IA_SET_PARAMS,
+				SST_FLAG_BLOCKED, bc->task_id, 0, cmd, len);
+	kfree(cmd);
+}
+
+static int sst_algo_bytes_ctl_info(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_info *uinfo)
+{
+	struct sst_algo_control *bc = (void *)kcontrol->private_value;
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+	uinfo->count = bc->max;
+
+	/* allocate space to cache the algo parameters in the driver */
+	if (bc->params == NULL) {
+		bc->params = devm_kzalloc(component->dev, bc->max, GFP_KERNEL);
+		if (bc->params == NULL)
+			return -ENOMEM;
+	}
+	return 0;
+}
+
+static int sst_algo_control_get(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct sst_algo_control *bc = (void *)kcontrol->private_value;
+
+	switch (bc->type) {
+	case SST_ALGO_PARAMS:
+		if (bc->params)
+			memcpy(ucontrol->value.bytes.data, bc->params, bc->max);
+		break;
+	case SST_ALGO_BYPASS:
+		ucontrol->value.integer.value[0] = bc->bypass ? 1 : 0;
+		pr_debug("%s: bypass  %d\n", __func__, bc->bypass);
+		break;
+	default:
+		pr_err("Invalid Input- algo type:%d\n", bc->type);
+		return -EINVAL;
+
+	}
+	return 0;
+}
+
+static int sst_algo_control_set(struct snd_kcontrol *kcontrol,
+				struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_platform *platform =  snd_soc_kcontrol_platform(kcontrol);
+	struct sst_data *drv = snd_soc_platform_get_drvdata(platform);
+	struct sst_algo_control *bc = (void *)kcontrol->private_value;
+
+	pr_debug("in %s control_name=%s\n", __func__, kcontrol->id.name);
+	mutex_lock(&drv->lock);
+	switch (bc->type) {
+	case SST_ALGO_PARAMS:
+		if (bc->params)
+			memcpy(bc->params, ucontrol->value.bytes.data, bc->max);
+		break;
+	case SST_ALGO_BYPASS:
+		bc->bypass = !!ucontrol->value.integer.value[0];
+		break;
+	default:
+		mutex_unlock(&drv->lock);
+		pr_err("Invalid Input- algo type:%d\n", bc->type);
+		return -EINVAL;
+	}
+	/*if pipe is enabled, need to send the algo params from here */
+	if (bc->w && bc->w->power)
+		sst_send_algo_cmd(drv, bc);
+	mutex_unlock(&drv->lock);
+
+	return 0;
+}
+
+static const struct snd_kcontrol_new sst_algo_controls[] = {
+	SST_ALGO_KCONTROL_BYTES("media_loop1_out", "fir", 272, SST_MODULE_ID_FIR_24,
+		 SST_PATH_INDEX_MEDIA_LOOP1_OUT, 0, SST_TASK_SBA, SBA_VB_SET_FIR),
+	SST_ALGO_KCONTROL_BYTES("media_loop1_out", "iir", 300, SST_MODULE_ID_IIR_24,
+		SST_PATH_INDEX_MEDIA_LOOP1_OUT, 0, SST_TASK_SBA, SBA_VB_SET_IIR),
+	SST_ALGO_KCONTROL_BYTES("media_loop1_out", "mdrp", 286, SST_MODULE_ID_MDRP,
+		SST_PATH_INDEX_MEDIA_LOOP1_OUT, 0, SST_TASK_SBA, SBA_SET_MDRP),
+	SST_ALGO_KCONTROL_BYTES("media_loop2_out", "fir", 272, SST_MODULE_ID_FIR_24,
+		SST_PATH_INDEX_MEDIA_LOOP2_OUT, 0, SST_TASK_SBA, SBA_VB_SET_FIR),
+	SST_ALGO_KCONTROL_BYTES("media_loop2_out", "iir", 300, SST_MODULE_ID_IIR_24,
+		SST_PATH_INDEX_MEDIA_LOOP2_OUT, 0, SST_TASK_SBA, SBA_VB_SET_IIR),
+	SST_ALGO_KCONTROL_BYTES("media_loop2_out", "mdrp", 286, SST_MODULE_ID_MDRP,
+		SST_PATH_INDEX_MEDIA_LOOP2_OUT, 0, SST_TASK_SBA, SBA_SET_MDRP),
+	SST_ALGO_KCONTROL_BYTES("sprot_loop_out", "lpro", 192, SST_MODULE_ID_SPROT,
+		SST_PATH_INDEX_SPROT_LOOP_OUT, 0, SST_TASK_SBA, SBA_VB_LPRO),
+	SST_ALGO_KCONTROL_BYTES("codec_in0", "dcr", 52, SST_MODULE_ID_FILT_DCR,
+		SST_PATH_INDEX_CODEC_IN0, 0, SST_TASK_SBA, SBA_VB_SET_IIR),
+	SST_ALGO_KCONTROL_BYTES("codec_in1", "dcr", 52, SST_MODULE_ID_FILT_DCR,
+		SST_PATH_INDEX_CODEC_IN1, 0, SST_TASK_SBA, SBA_VB_SET_IIR),
+
+};
+
 int sst_dsp_init_v2_dpcm(struct snd_soc_platform *platform)
 {
 	int ret = 0;
@@ -35,5 +205,7 @@  int sst_dsp_init_v2_dpcm(struct snd_soc_platform *platform)
 	if (!drv->byte_stream)
 		return -ENOMEM;
 
+	snd_soc_add_platform_controls(platform, sst_algo_controls,
+			ARRAY_SIZE(sst_algo_controls));
 	return ret;
 }
diff --git a/sound/soc/intel/sst-atom-controls.h b/sound/soc/intel/sst-atom-controls.h
index 8554889c0694..a73e894b175c 100644
--- a/sound/soc/intel/sst-atom-controls.h
+++ b/sound/soc/intel/sst-atom-controls.h
@@ -309,4 +309,134 @@  enum sst_swm_state {
 	SST_SWM_ON = 3,
 };
 
+#define SST_FILL_LOCATION_IDS(dst, cell_idx, pipe_id)		do {	\
+		dst.location_id.p.cell_nbr_idx = (cell_idx);		\
+		dst.location_id.p.path_id = (pipe_id);			\
+	} while (0)
+#define SST_FILL_LOCATION_ID(dst, loc_id)				(\
+	dst.location_id.f = (loc_id))
+#define SST_FILL_MODULE_ID(dst, mod_id)					(\
+	dst.module_id = (mod_id))
+
+#define SST_FILL_DESTINATION1(dst, id)				do {	\
+		SST_FILL_LOCATION_ID(dst, (id) & 0xFFFF);		\
+		SST_FILL_MODULE_ID(dst, ((id) & 0xFFFF0000) >> 16);	\
+	} while (0)
+#define SST_FILL_DESTINATION2(dst, loc_id, mod_id)		do {	\
+		SST_FILL_LOCATION_ID(dst, loc_id);			\
+		SST_FILL_MODULE_ID(dst, mod_id);			\
+	} while (0)
+#define SST_FILL_DESTINATION3(dst, cell_idx, path_id, mod_id)	do {	\
+		SST_FILL_LOCATION_IDS(dst, cell_idx, path_id);		\
+		SST_FILL_MODULE_ID(dst, mod_id);			\
+	} while (0)
+
+#define SST_FILL_DESTINATION(level, dst, ...)				\
+	SST_FILL_DESTINATION##level(dst, __VA_ARGS__)
+#define SST_FILL_DEFAULT_DESTINATION(dst)				\
+	SST_FILL_DESTINATION(2, dst, SST_DEFAULT_LOCATION_ID, SST_DEFAULT_MODULE_ID)
+
+struct sst_destination_id {
+	union sst_location_id {
+		struct {
+			u8 cell_nbr_idx;	/* module index */
+			u8 path_id;		/* pipe_id */
+		} __packed	p;		/* part */
+		u16		f;		/* full */
+	} __packed location_id;
+	u16	   module_id;
+} __packed;
+struct sst_dsp_header {
+	struct sst_destination_id dst;
+	u16 command_id;
+	u16 length;
+} __packed;
+
+/*
+ *
+ * Common Commands
+ *
+ */
+struct sst_cmd_generic {
+	struct sst_dsp_header header;
+} __packed;
+struct sst_cmd_set_params {
+	struct sst_destination_id dst;
+	u16 command_id;
+	char params[0];
+} __packed;
+#define SST_CONTROL_NAME(xpname, xmname, xinstance, xtype) \
+	xpname " " xmname " " #xinstance " " xtype
+
+#define SST_COMBO_CONTROL_NAME(xpname, xmname, xinstance, xtype, xsubmodule) \
+	xpname " " xmname " " #xinstance " " xtype " " xsubmodule
+enum sst_algo_kcontrol_type {
+	SST_ALGO_PARAMS,
+	SST_ALGO_BYPASS,
+};
+
+struct sst_algo_control {
+	enum sst_algo_kcontrol_type type;
+	int max;
+	u16 module_id;
+	u16 pipe_id;
+	u16 task_id;
+	u16 cmd_id;
+	bool bypass;
+	unsigned char *params;
+	struct snd_soc_dapm_widget *w;
+};
+
+/* size of the control = size of params + size of length field */
+#define SST_ALGO_CTL_VALUE(xcount, xtype, xpipe, xmod, xtask, xcmd)			\
+	(struct sst_algo_control){							\
+		.max = xcount + sizeof(u16), .type = xtype, .module_id = xmod,			\
+		.pipe_id = xpipe, .task_id = xtask, .cmd_id = xcmd,			\
+	}
+
+#define SST_ALGO_KCONTROL(xname, xcount, xmod, xpipe,					\
+			  xtask, xcmd, xtype, xinfo, xget, xput)			\
+{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER,						\
+	.name =  xname,									\
+	.info = xinfo, .get = xget, .put = xput,					\
+	.private_value = (unsigned long)&						\
+			SST_ALGO_CTL_VALUE(xcount, xtype, xpipe,			\
+					   xmod, xtask, xcmd),				\
+}
+
+#define SST_ALGO_KCONTROL_BYTES(xpname, xmname, xcount, xmod,				\
+				xpipe, xinstance, xtask, xcmd)				\
+	SST_ALGO_KCONTROL(SST_CONTROL_NAME(xpname, xmname, xinstance, "params"),	\
+			  xcount, xmod, xpipe, xtask, xcmd, SST_ALGO_PARAMS,		\
+			  sst_algo_bytes_ctl_info,					\
+			  sst_algo_control_get, sst_algo_control_set)
+
+#define SST_ALGO_KCONTROL_BOOL(xpname, xmname, xmod, xpipe, xinstance, xtask)		\
+	SST_ALGO_KCONTROL(SST_CONTROL_NAME(xpname, xmname, xinstance, "bypass"),	\
+			  0, xmod, xpipe, xtask, 0, SST_ALGO_BYPASS,			\
+			  snd_soc_info_bool_ext,					\
+			  sst_algo_control_get, sst_algo_control_set)
+
+#define SST_ALGO_BYPASS_PARAMS(xpname, xmname, xcount, xmod, xpipe,			\
+				xinstance, xtask, xcmd)					\
+	SST_ALGO_KCONTROL_BOOL(xpname, xmname, xmod, xpipe, xinstance, xtask),		\
+	SST_ALGO_KCONTROL_BYTES(xpname, xmname, xcount, xmod, xpipe, xinstance, xtask, xcmd)
+
+#define SST_COMBO_ALGO_KCONTROL_BYTES(xpname, xmname, xsubmod, xcount, xmod,		\
+				      xpipe, xinstance, xtask, xcmd)			\
+	SST_ALGO_KCONTROL(SST_COMBO_CONTROL_NAME(xpname, xmname, xinstance, "params",	\
+						 xsubmod),				\
+			  xcount, xmod, xpipe, xtask, xcmd, SST_ALGO_PARAMS,		\
+			  sst_algo_bytes_ctl_info,					\
+			  sst_algo_control_get, sst_algo_control_set)
+
+
+struct sst_enum {
+	bool tx;
+	unsigned short reg;
+	unsigned int max;
+	const char * const *texts;
+	struct snd_soc_dapm_widget *w;
+};
+
 #endif