[alsa-devel] [RFC v2] ASoC: core: add a helper for extended byte controls using TLV

Takashi Iwai tiwai at suse.de
Tue Jul 15 16:36:21 CEST 2014


At Tue, 15 Jul 2014 12:17:45 +0530,
Vinod Koul wrote:
> 
> From: Omair Mohammed Abdullah <omair.m.abdullah at intel.com>
> 
> ALSA supports arbitrary length TLVs for each kcontrol that can be used
> to pass metadata about the control (e.g. volumes, enum information). The
> same transport mechanism is now used for arbitrary length data by
> defining a new helper.
> 
> Signed-off-by: Omair Mohammed Abdullah <omair.m.abdullah at intel.com>
> Signed-off-by: Vinod Koul <vinod.koul at intel.com>
> ---
> As discussed in [1] we are adding a new approach to solve the byte control
> extensions by using existing TLVs and combining them with byte controls.  The
> usermode on seeing byte control + TLV can treat it differently as it does for
> control + TLV combination today. This way we don't change kernel API and
> existing users will be happy, while providing embedded folks facility to pass
> large bytes data to kcontrols
> [1]:
> http://mailman.alsa-project.org/pipermail/alsa-devel/2013-November/069483.html

Yeah, this would bypass the limitation nicely.
Small nitpicking:

> 
>  include/sound/soc.h  |   14 +++++++++++++-
>  sound/soc/soc-core.c |   22 ++++++++++++++++++++++
>  2 files changed, 35 insertions(+), 1 deletions(-)
> 
> diff --git a/include/sound/soc.h b/include/sound/soc.h
> index ed9e2d7..7522221 100644
> --- a/include/sound/soc.h
> +++ b/include/sound/soc.h
> @@ -270,7 +270,14 @@
>  	.get = xhandler_get, .put = xhandler_put, \
>  	.private_value = (unsigned long)&(struct soc_bytes_ext) \
>  		{.max = xcount} }
> -
> +#define SND_SOC_BYTES_TLV(xname, xcount, xhandler_get, xhandler_put) \
> +{	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \
> +	.access = SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE | \
> +		  SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK, \
> +	.tlv.c = (snd_soc_bytes_tlv_callback), \
> +	.info = snd_soc_info_bytes_ext, \
> +	.private_value = (unsigned long)&(struct soc_bytes_ext) \
> +		{.max = xcount, .get = xhandler_get, .put = xhandler_put, }}
>  #define SOC_SINGLE_XR_SX(xname, xregbase, xregcount, xnbits, \
>  		xmin, xmax, xinvert) \
>  {	.iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = (xname), \
> @@ -552,6 +559,8 @@ int snd_soc_bytes_put(struct snd_kcontrol *kcontrol,
>  		      struct snd_ctl_elem_value *ucontrol);
>  int snd_soc_bytes_info_ext(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_info *ucontrol);
> +int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag,
> +	unsigned int size, unsigned int __user *tlv);
>  int snd_soc_info_xr_sx(struct snd_kcontrol *kcontrol,
>  	struct snd_ctl_elem_info *uinfo);
>  int snd_soc_get_xr_sx(struct snd_kcontrol *kcontrol,
> @@ -1119,6 +1128,9 @@ struct soc_bytes {
>  
>  struct soc_bytes_ext {
>  	int max;
> +	/* used for TLV byte control */
> +	int (*get)(unsigned int __user *bytes, unsigned int size);
> +	int (*put)(const unsigned int __user *bytes, unsigned int size);
>  };
>  
>  /* multi register control */
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index b87d7d8..e6ad95b 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -3267,6 +3267,28 @@ int snd_soc_bytes_info_ext(struct snd_kcontrol *kcontrol,
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_bytes_info_ext);
>  
> +int snd_soc_bytes_tlv_callback(struct snd_kcontrol *kcontrol, int op_flag,
> +				unsigned int size, unsigned int __user *tlv)
> +{
> +	struct soc_bytes_ext *params = (void *)kcontrol->private_value;
> +	unsigned int count = size < params->max ? size : params->max;
> +
> +	switch (op_flag) {
> +	case 0:
> +		if (params->get)
> +			params->get(tlv, count);
> +		break;
> +	case 1:
> +		if (params->put)
> +			params->put(tlv, count);
> +		break;

No error propagation from the callback?
Also, if a driver doesn't provide get or put, it gets no error?

BTW, about the value of op_flag: I have a patch to define the
constants (attached below), which I forgot until now.  Now I put it
into topic/tlv-opflag branch of sound git tree.  If you'd like to use
the new constants, merge (or base on) this branch.


Takashi

-- 8< --
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: control: Define SNDRV_CTL_TLV_OP_* constants

Instead of hard-coded magic numbers, define constants for op_flag to
tlv callbacks.

Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 include/sound/control.h | 7 ++++++-
 sound/core/control.c    | 6 +++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index 5358892b1b39..042613938a1d 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -31,10 +31,15 @@ typedef int (snd_kcontrol_info_t) (struct snd_kcontrol * kcontrol, struct snd_ct
 typedef int (snd_kcontrol_get_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
 typedef int (snd_kcontrol_put_t) (struct snd_kcontrol * kcontrol, struct snd_ctl_elem_value * ucontrol);
 typedef int (snd_kcontrol_tlv_rw_t)(struct snd_kcontrol *kcontrol,
-				    int op_flag, /* 0=read,1=write,-1=command */
+				    int op_flag, /* SNDRV_CTL_TLV_OP_XXX */
 				    unsigned int size,
 				    unsigned int __user *tlv);
 
+enum {
+	SNDRV_CTL_TLV_OP_READ = 0,
+	SNDRV_CTL_TLV_OP_WRITE = 1,
+	SNDRV_CTL_TLV_OP_CMD = -1,
+};
 
 struct snd_kcontrol_new {
 	snd_ctl_elem_iface_t iface;	/* interface identifier */
diff --git a/sound/core/control.c b/sound/core/control.c
index f0b0e14497a5..b9611344ff9e 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -1406,11 +1406,11 @@ static long snd_ctl_ioctl(struct file *file, unsigned int cmd, unsigned long arg
 	case SNDRV_CTL_IOCTL_SUBSCRIBE_EVENTS:
 		return snd_ctl_subscribe_events(ctl, ip);
 	case SNDRV_CTL_IOCTL_TLV_READ:
-		return snd_ctl_tlv_ioctl(ctl, argp, 0);
+		return snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_READ);
 	case SNDRV_CTL_IOCTL_TLV_WRITE:
-		return snd_ctl_tlv_ioctl(ctl, argp, 1);
+		return snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_WRITE);
 	case SNDRV_CTL_IOCTL_TLV_COMMAND:
-		return snd_ctl_tlv_ioctl(ctl, argp, -1);
+		return snd_ctl_tlv_ioctl(ctl, argp, SNDRV_CTL_TLV_OP_CMD);
 	case SNDRV_CTL_IOCTL_POWER:
 		return -ENOPROTOOPT;
 	case SNDRV_CTL_IOCTL_POWER_STATE:
-- 
2.0.1



More information about the Alsa-devel mailing list