[alsa-devel] [PATCH 0/3] ASoC: remove SND_SOC_BYTES_EXT
The SND_SOC_BYTES_TLV and SND_SOC_BYTES_EXT provide similar functionality, with only difference that SND_SOC_BYTES_EXT have limitations of 512 bytes as parameters whereas no such restrictions on SND_SOC_BYTES_TLV
So remove SND_SOC_BYTES_EXT and update users to use SND_SOC_BYTES_TLV
Vinod Koul (3): ASoC: Intel: Hsw: Move driver to use SND_SOC_BYTES_TLV ASoC: wm5102: Move driver to use SND_SOC_BYTES_TLV ASoC: core: remove SND_SOC_BYTES_EXT
include/sound/soc.h | 6 ---- sound/soc/codecs/wm5102.c | 35 ++++++++++++++++------ sound/soc/intel/haswell/sst-haswell-pcm.c | 50 ++++++++++++++++++++++++------- 3 files changed, 66 insertions(+), 25 deletions(-)
Haswell driver was using SND_SOC_BYTES_EXT. Since we want to remove this, convert it to use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koul vinod.koul@intel.com Cc: Jie Yang yang.jie@linux.intel.com Cc: Liam Girdwood lgirdwood@gmail.com --- sound/soc/intel/haswell/sst-haswell-pcm.c | 50 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c index 1aa819c7e09b..65594dff291a 100644 --- a/sound/soc/intel/haswell/sst-haswell-pcm.c +++ b/sound/soc/intel/haswell/sst-haswell-pcm.c @@ -368,41 +368,71 @@ static int hsw_waves_switch_put(struct snd_kcontrol *kcontrol, }
static int hsw_waves_param_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + unsigned int __user *data, unsigned int size) { struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); struct sst_hsw *hsw = pdata->hsw; + u8 *buffer; + int ret; + + buffer = kzalloc(size, GFP_KERNEL); + if (!buffer) + return -ENOMEM;
/* return a matching line from param buffer */ - return sst_hsw_load_param_line(hsw, ucontrol->value.bytes.data); + ret = sst_hsw_load_param_line(hsw, buffer); + if (ret) + goto err; + + if (copy_to_user(data, buffer, size)) + ret = -EFAULT; + +err: + kfree(buffer); + return ret; }
static int hsw_waves_param_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + const unsigned int __user *data, unsigned int size) { struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); struct sst_hsw *hsw = pdata->hsw; - int ret; + int ret = 0; enum sst_hsw_module_id id = SST_HSW_MODULE_WAVES; - int param_id = ucontrol->value.bytes.data[0]; + int param_id; int param_size = WAVES_PARAM_COUNT; + u8 *buffer; + + buffer = kzalloc(size, GFP_KERNEL); + if (!buffer) + return -ENOMEM; + + if (copy_from_user(buffer, data, size)) { + ret = -EFAULT; + goto exit; + } + + param_id = *buffer;
/* clear param buffer and reset buffer index */ if (param_id == 0xFF) { sst_hsw_reset_param_buf(hsw); - return 0; + goto exit; }
/* store params into buffer */ - ret = sst_hsw_store_param_line(hsw, ucontrol->value.bytes.data); + ret = sst_hsw_store_param_line(hsw, buffer); if (ret < 0) - return ret; + goto exit;
if (sst_hsw_is_module_active(hsw, id)) ret = sst_hsw_module_set_param(hsw, id, 0, param_id, - param_size, ucontrol->value.bytes.data); + param_size, buffer); + +exit: + kfree(buffer); return ret; }
@@ -431,7 +461,7 @@ static const struct snd_kcontrol_new hsw_volume_controls[] = { SOC_SINGLE_BOOL_EXT("Waves Switch", 0, hsw_waves_switch_get, hsw_waves_switch_put), /* set parameters to module waves */ - SND_SOC_BYTES_EXT("Waves Set Param", WAVES_PARAM_COUNT, + SND_SOC_BYTES_TLV("Waves Set Param", WAVES_PARAM_COUNT, hsw_waves_param_get, hsw_waves_param_put), };
+ Han who is the author of this part.
On 2015年11月18日 21:39, Vinod Koul wrote:
Haswell driver was using SND_SOC_BYTES_EXT. Since we want to remove this, convert it to use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koul vinod.koul@intel.com Cc: Jie Yang yang.jie@linux.intel.com Cc: Liam Girdwood lgirdwood@gmail.com
looks fine for me, but let's wait Han's confirmation about it.
thanks, ~Keyon
sound/soc/intel/haswell/sst-haswell-pcm.c | 50 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c index 1aa819c7e09b..65594dff291a 100644 --- a/sound/soc/intel/haswell/sst-haswell-pcm.c +++ b/sound/soc/intel/haswell/sst-haswell-pcm.c @@ -368,41 +368,71 @@ static int hsw_waves_switch_put(struct snd_kcontrol *kcontrol, }
static int hsw_waves_param_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
unsigned int __user *data, unsigned int size){ struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); struct sst_hsw *hsw = pdata->hsw;
u8 *buffer;
int ret;
buffer = kzalloc(size, GFP_KERNEL);
if (!buffer)
return -ENOMEM;/* return a matching line from param buffer */
- return sst_hsw_load_param_line(hsw, ucontrol->value.bytes.data);
- ret = sst_hsw_load_param_line(hsw, buffer);
- if (ret)
goto err;- if (copy_to_user(data, buffer, size))
ret = -EFAULT;+err:
kfree(buffer);
return ret; }
static int hsw_waves_param_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
const unsigned int __user *data, unsigned int size)
- int ret;
- int ret = 0; enum sst_hsw_module_id id = SST_HSW_MODULE_WAVES;
- int param_id = ucontrol->value.bytes.data[0];
int param_id; int param_size = WAVES_PARAM_COUNT;
u8 *buffer;
buffer = kzalloc(size, GFP_KERNEL);
if (!buffer)
return -ENOMEM;if (copy_from_user(buffer, data, size)) {
ret = -EFAULT;goto exit;}
param_id = *buffer;
/* clear param buffer and reset buffer index */ if (param_id == 0xFF) { sst_hsw_reset_param_buf(hsw);
return 0;
goto exit;}
/* store params into buffer */
- ret = sst_hsw_store_param_line(hsw, ucontrol->value.bytes.data);
- ret = sst_hsw_store_param_line(hsw, buffer); if (ret < 0)
return ret;
goto exit;if (sst_hsw_is_module_active(hsw, id)) ret = sst_hsw_module_set_param(hsw, id, 0, param_id,
param_size, ucontrol->value.bytes.data);
param_size, buffer);+exit:
- kfree(buffer); return ret; }
@@ -431,7 +461,7 @@ static const struct snd_kcontrol_new hsw_volume_controls[] = { SOC_SINGLE_BOOL_EXT("Waves Switch", 0, hsw_waves_switch_get, hsw_waves_switch_put), /* set parameters to module waves */
- SND_SOC_BYTES_EXT("Waves Set Param", WAVES_PARAM_COUNT,
- SND_SOC_BYTES_TLV("Waves Set Param", WAVES_PARAM_COUNT, hsw_waves_param_get, hsw_waves_param_put), };
There was no special reason for me to use SND_SOC_BYTES_EXT here. But will this patch influences user space, since the parameter list of hsw_waves_param_get() be modified?
Regards, Han Lu
On 11/18/2015 09:39 PM, Vinod Koul wrote:
Haswell driver was using SND_SOC_BYTES_EXT. Since we want to remove this, convert it to use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koulvinod.koul@intel.com Cc: Jie Yangyang.jie@linux.intel.com Cc: Liam Girdwoodlgirdwood@gmail.com
sound/soc/intel/haswell/sst-haswell-pcm.c | 50 ++++++++++++++++++++++++------- 1 file changed, 40 insertions(+), 10 deletions(-)
diff --git a/sound/soc/intel/haswell/sst-haswell-pcm.c b/sound/soc/intel/haswell/sst-haswell-pcm.c index 1aa819c7e09b..65594dff291a 100644 --- a/sound/soc/intel/haswell/sst-haswell-pcm.c +++ b/sound/soc/intel/haswell/sst-haswell-pcm.c @@ -368,41 +368,71 @@ static int hsw_waves_switch_put(struct snd_kcontrol *kcontrol, }
static int hsw_waves_param_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
unsigned int __user *data, unsigned int size){ struct snd_soc_platform *platform = snd_soc_kcontrol_platform(kcontrol); struct hsw_priv_data *pdata = snd_soc_platform_get_drvdata(platform); struct sst_hsw *hsw = pdata->hsw;
u8 *buffer;
int ret;
buffer = kzalloc(size, GFP_KERNEL);
if (!buffer)
return -ENOMEM;/* return a matching line from param buffer */
- return sst_hsw_load_param_line(hsw, ucontrol->value.bytes.data);
- ret = sst_hsw_load_param_line(hsw, buffer);
- if (ret)
goto err;- if (copy_to_user(data, buffer, size))
ret = -EFAULT;+err:
kfree(buffer);
return ret; }
static int hsw_waves_param_put(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
const unsigned int __user *data, unsigned int size)
- int ret;
- int ret = 0; enum sst_hsw_module_id id = SST_HSW_MODULE_WAVES;
- int param_id = ucontrol->value.bytes.data[0];
int param_id; int param_size = WAVES_PARAM_COUNT;
u8 *buffer;
buffer = kzalloc(size, GFP_KERNEL);
if (!buffer)
return -ENOMEM;if (copy_from_user(buffer, data, size)) {
ret = -EFAULT;goto exit;}
param_id = *buffer;
/* clear param buffer and reset buffer index */ if (param_id == 0xFF) { sst_hsw_reset_param_buf(hsw);
return 0;
goto exit;}
/* store params into buffer */
- ret = sst_hsw_store_param_line(hsw, ucontrol->value.bytes.data);
- ret = sst_hsw_store_param_line(hsw, buffer); if (ret < 0)
return ret;
goto exit;if (sst_hsw_is_module_active(hsw, id)) ret = sst_hsw_module_set_param(hsw, id, 0, param_id,
param_size, ucontrol->value.bytes.data);
param_size, buffer);+exit:
- kfree(buffer); return ret; }
@@ -431,7 +461,7 @@ static const struct snd_kcontrol_new hsw_volume_controls[] = { SOC_SINGLE_BOOL_EXT("Waves Switch", 0, hsw_waves_switch_get, hsw_waves_switch_put), /* set parameters to module waves */
- SND_SOC_BYTES_EXT("Waves Set Param", WAVES_PARAM_COUNT,
- SND_SOC_BYTES_TLV("Waves Set Param", WAVES_PARAM_COUNT, hsw_waves_param_get, hsw_waves_param_put), };
WM5102 driver was using SND_SOC_BYTES_EXT. Since we want to remove this, convert it to use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koul vinod.koul@intel.com Cc: Charles Keepax ckeepax@opensource.wolfsonmicro.com Cc: patches@opensource.wolfsonmicro.com --- sound/soc/codecs/wm5102.c | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-)
diff --git a/sound/soc/codecs/wm5102.c b/sound/soc/codecs/wm5102.c index 64637d1cf4e5..ac96e7e32fa7 100644 --- a/sound/soc/codecs/wm5102.c +++ b/sound/soc/codecs/wm5102.c @@ -657,33 +657,50 @@ static int wm5102_adsp_power_ev(struct snd_soc_dapm_widget *w, return wm_adsp2_early_event(w, kcontrol, event); }
+#define WM5102_OUT_COMP_SZ 2 + static int wm5102_out_comp_coeff_get(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + unsigned int __user *data, unsigned int size) { struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol); struct arizona *arizona = dev_get_drvdata(codec->dev->parent); + u16 buff; + int ret = 0; + + if (size > WM5102_OUT_COMP_SZ) + return -EINVAL;
mutex_lock(&arizona->dac_comp_lock); - put_unaligned_be16(arizona->dac_comp_coeff, - ucontrol->value.bytes.data); + put_unaligned_be16(arizona->dac_comp_coeff, &buff); + if (copy_to_user(data, &buff, sizeof(buff))) + ret = -EFAULT; + mutex_unlock(&arizona->dac_comp_lock);
- return 0; + return ret; }
static int wm5102_out_comp_coeff_put(struct snd_kcontrol *kcontrol, - struct snd_ctl_elem_value *ucontrol) + const unsigned int __user *data, unsigned int size) { struct snd_soc_codec *codec = snd_soc_kcontrol_codec(kcontrol); struct arizona *arizona = dev_get_drvdata(codec->dev->parent); + int ret = 0; + + if (size > WM5102_OUT_COMP_SZ) + return -EINVAL;
mutex_lock(&arizona->dac_comp_lock); - memcpy(&arizona->dac_comp_coeff, ucontrol->value.bytes.data, - sizeof(arizona->dac_comp_coeff)); + if (copy_from_user(&arizona->dac_comp_coeff, data, size)) { + ret = -EFAULT; + goto exit; + } arizona->dac_comp_coeff = be16_to_cpu(arizona->dac_comp_coeff); + +exit: mutex_unlock(&arizona->dac_comp_lock);
- return 0; + return ret; }
static int wm5102_out_comp_switch_get(struct snd_kcontrol *kcontrol, @@ -939,7 +956,7 @@ SOC_SINGLE_TLV("Noise Gate Threshold Volume", ARIZONA_NOISE_GATE_CONTROL, ARIZONA_NGATE_THR_SHIFT, 7, 1, ng_tlv), SOC_ENUM("Noise Gate Hold", arizona_ng_hold),
-SND_SOC_BYTES_EXT("Output Compensation Coefficient", 2, +SND_SOC_BYTES_TLV("Output Compensation Coefficient", WM5102_OUT_COMP_SZ, wm5102_out_comp_coeff_get, wm5102_out_comp_coeff_put),
SOC_SINGLE_EXT("Output Compensation Switch", 0, 0, 1, 0,
On Wed, Nov 18, 2015 at 07:09:11PM +0530, Vinod Koul wrote:
WM5102 driver was using SND_SOC_BYTES_EXT. Since we want to remove this, convert it to use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koul vinod.koul@intel.com Cc: Charles Keepax ckeepax@opensource.wolfsonmicro.com Cc: patches@opensource.wolfsonmicro.com
Acked-by: Charles Keepax ckeepax@opensource.wolfsonmicro.com
Looks good to me, I think the TLV byte control stuff has been merged in tinyalsa now so no problems there.
Thanks, Charles
Now all users are converted, remove the SND_SOC_BYTES_EXT New users should use SND_SOC_BYTES_TLV
Signed-off-by: Vinod Koul vinod.koul@intel.com --- include/sound/soc.h | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index e09673dbae1b..10607aa29b48 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -293,12 +293,6 @@ {.base = xbase, .num_regs = xregs, \ .mask = xmask }) }
-#define SND_SOC_BYTES_EXT(xname, xcount, xhandler_get, xhandler_put) \ -{ .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ - .info = snd_soc_bytes_info_ext, \ - .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 | \
On Wed, 18 Nov 2015 14:39:09 +0100, Vinod Koul wrote:
The SND_SOC_BYTES_TLV and SND_SOC_BYTES_EXT provide similar functionality, with only difference that SND_SOC_BYTES_EXT have limitations of 512 bytes as parameters whereas no such restrictions on SND_SOC_BYTES_TLV
So remove SND_SOC_BYTES_EXT and update users to use SND_SOC_BYTES_TLV
Hmm, won't this break user-space? Both use the difference access methods, so I wonder how they can be kept compatible.
thanks,
Takashi
Vinod Koul (3): ASoC: Intel: Hsw: Move driver to use SND_SOC_BYTES_TLV ASoC: wm5102: Move driver to use SND_SOC_BYTES_TLV ASoC: core: remove SND_SOC_BYTES_EXT
include/sound/soc.h | 6 ---- sound/soc/codecs/wm5102.c | 35 ++++++++++++++++------ sound/soc/intel/haswell/sst-haswell-pcm.c | 50 ++++++++++++++++++++++++------- 3 files changed, 66 insertions(+), 25 deletions(-)
-- 1.9.1
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Wed, Nov 18, 2015 at 02:53:23PM +0100, Takashi Iwai wrote:
On Wed, 18 Nov 2015 14:39:09 +0100, Vinod Koul wrote:
The SND_SOC_BYTES_TLV and SND_SOC_BYTES_EXT provide similar functionality, with only difference that SND_SOC_BYTES_EXT have limitations of 512 bytes as parameters whereas no such restrictions on SND_SOC_BYTES_TLV
So remove SND_SOC_BYTES_EXT and update users to use SND_SOC_BYTES_TLV
Hmm, won't this break user-space? Both use the difference access methods, so I wonder how they can be kept compatible.
Yes they will, so should we then keep these and mark SND_SOC_BYTES_EXT depricated ..?
Mark what should we do with these
On Wed, Nov 18, 2015 at 11:49:53PM +0530, Vinod Koul wrote:
On Wed, Nov 18, 2015 at 02:53:23PM +0100, Takashi Iwai wrote:
Hmm, won't this break user-space? Both use the difference access methods, so I wonder how they can be kept compatible.
Yes they will, so should we then keep these and mark SND_SOC_BYTES_EXT depricated ..?
Mark what should we do with these
Yes, that seems like a reasonable approach.
On Sat, Nov 21, 2015 at 02:08:28PM +0000, Mark Brown wrote:
On Wed, Nov 18, 2015 at 11:49:53PM +0530, Vinod Koul wrote:
On Wed, Nov 18, 2015 at 02:53:23PM +0100, Takashi Iwai wrote:
Hmm, won't this break user-space? Both use the difference access methods, so I wonder how they can be kept compatible.
Yes they will, so should we then keep these and mark SND_SOC_BYTES_EXT depricated ..?
Mark what should we do with these
Yes, that seems like a reasonable approach.
Okay I will send a patch which add comment about this being deprecated
Thanks
participants (6)
-
Charles Keepax -
Han Lu -
Keyon -
Mark Brown -
Takashi Iwai -
Vinod Koul