* [RFC v2] ASoC: core: add a helper for extended byte controls using TLV @ 2014-07-15 6:47 Vinod Koul 2014-07-15 14:36 ` Takashi Iwai 0 siblings, 1 reply; 3+ messages in thread From: Vinod Koul @ 2014-07-15 6:47 UTC (permalink / raw) To: alsa-devel Cc: Charles Keepax, Dimitris Papastamos, tiwai, lgirdwood, Vinod Koul, broonie, Omair Mohammed Abdullah, pierre-louis.bossart From: Omair Mohammed Abdullah <omair.m.abdullah@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@intel.com> Signed-off-by: Vinod Koul <vinod.koul@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 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; + default: + return -ENXIO; + } + return 0; +} +EXPORT_SYMBOL_GPL(snd_soc_bytes_tlv_callback); + /** * snd_soc_info_xr_sx - signed multi register info callback * @kcontrol: mreg control -- 1.7.0.4 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC v2] ASoC: core: add a helper for extended byte controls using TLV 2014-07-15 6:47 [RFC v2] ASoC: core: add a helper for extended byte controls using TLV Vinod Koul @ 2014-07-15 14:36 ` Takashi Iwai 2014-07-15 15:30 ` Vinod Koul 0 siblings, 1 reply; 3+ messages in thread From: Takashi Iwai @ 2014-07-15 14:36 UTC (permalink / raw) To: Vinod Koul Cc: alsa-devel, Charles Keepax, Dimitris Papastamos, lgirdwood, broonie, Omair Mohammed Abdullah, pierre-louis.bossart At Tue, 15 Jul 2014 12:17:45 +0530, Vinod Koul wrote: > > From: Omair Mohammed Abdullah <omair.m.abdullah@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@intel.com> > Signed-off-by: Vinod Koul <vinod.koul@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@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@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 ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [RFC v2] ASoC: core: add a helper for extended byte controls using TLV 2014-07-15 14:36 ` Takashi Iwai @ 2014-07-15 15:30 ` Vinod Koul 0 siblings, 0 replies; 3+ messages in thread From: Vinod Koul @ 2014-07-15 15:30 UTC (permalink / raw) To: Takashi Iwai Cc: alsa-devel, Charles Keepax, Dimitris Papastamos, lgirdwood, broonie, Omair Mohammed Abdullah, pierre-louis.bossart On Tue, Jul 15, 2014 at 04:36:21PM +0200, Takashi Iwai wrote: > At Tue, 15 Jul 2014 12:17:45 +0530, > Vinod Koul wrote: > > > > From: Omair Mohammed Abdullah <omair.m.abdullah@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@intel.com> > > Signed-off-by: Vinod Koul <vinod.koul@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. Yes thats what I realized when Omair pointed this out! > > +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? Will fix that > > 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. yes rebased now.. -- ~Vinod ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-07-15 15:34 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-07-15 6:47 [RFC v2] ASoC: core: add a helper for extended byte controls using TLV Vinod Koul 2014-07-15 14:36 ` Takashi Iwai 2014-07-15 15:30 ` Vinod Koul
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).