[alsa-devel] [PATCH v2 0/4] ASoC: omap-mcbsp: Sidetone related changes
Hello,
Changes since v1: - Macro for the ST control has been fixed - New patch to combine the st channel volume set/get macros
Mark: this series depends on the McBSP DT support (to be precise patch 3 and 4 has dependency).
Intro mail from v1:
When we boot with DT blob the sidetone is not supported at the moment. Prepare the McBSP code for the time when we are going to be able to support ST with DT booted kernel. Meanwhile do not block the audio card if the st_data is missing from the port, but print a warning about it. This will allow us to debug the DT booted kernels with audio.
Regards, Peter ---
Peter Ujfalusi (4): ASoC: omap-mcbsp: Check mcbsp->id instead of cpu_dai->id when adding ST controls ASoC: omap-mcbsp: Only print warning if the st_data is missing for the port ASoC: omap-mcbsp: Use macro to create the McBSP2/3 ST controls ASoC: omap-mcbsp: Single macro for st channel volume set/get
sound/soc/omap/omap-mcbsp.c | 64 ++++++++++++++++++------------------------- 1 files changed, 27 insertions(+), 37 deletions(-)
In ddevice tree booted kernel all device have unique name and their device id is set to 0. Use the mcbsp->id for checking to decide which control set we should add for McBSP sidetone handling.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index d6de066..84a3132 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -722,7 +722,7 @@ int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) if (!mcbsp->st_data) return -ENODEV;
- switch (cpu_dai->id) { + switch (mcbsp->id) { case 2: /* McBSP 2 */ return snd_soc_add_dai_controls(cpu_dai, omap_mcbsp2_st_controls,
When asked to add the ST controls warn only if the st_data is missing. In this way we do not block the otherwise functional card to probe.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 84a3132..c964f68 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -719,8 +719,10 @@ int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct omap_mcbsp *mcbsp = snd_soc_dai_get_drvdata(cpu_dai);
- if (!mcbsp->st_data) - return -ENODEV; + if (!mcbsp->st_data) { + dev_warn(mcbsp->dev, "No sidetone data for port\n"); + return 0; + }
switch (mcbsp->id) { case 2: /* McBSP 2 */
On 08/22/2012 01:11 PM, Peter Ujfalusi wrote:
When asked to add the ST controls warn only if the st_data is missing. In this way we do not block the otherwise functional card to probe.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com
sound/soc/omap/omap-mcbsp.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 84a3132..c964f68 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -719,8 +719,10 @@ int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) struct snd_soc_dai *cpu_dai = rtd->cpu_dai; struct omap_mcbsp *mcbsp = snd_soc_dai_get_drvdata(cpu_dai);
- if (!mcbsp->st_data)
return -ENODEV;
- if (!mcbsp->st_data) {
dev_warn(mcbsp->dev, "No sidetone data for port\n");
return 0;
- }
I acked the set but a little note here: I guess this is something what only developer can hit, by calling omap_mcbsp_st_add_controls for a port not having the sidetone, so would blocking the probe be better than warning?
On 08/24/2012 11:06 AM, Jarkko Nikula wrote:
I acked the set but a little note here: I guess this is something what only developer can hit, by calling omap_mcbsp_st_add_controls for a port not having the sidetone, so would blocking the probe be better than warning?
I was hesitating when I did this patch but at the end only developers can hit this - either trying to enable the ST on McBSP which does not physically have ST block, or as of now booting their kernel with DT. I'm rolling several patches for omap-twl4030 machine driver (support for McBSP master configuration, ST support, etc). It is really annoying when you test a change which is not ST related and when you boot with DT there is no card -> so you need to modify the kernel between DT boot and non DT boot.
We have the warning about it, we do not add the ST controls but the audio otherwise works.
The ST controls are not part of DAPM so the audio routing is not affected.
For us, developers it is better this way and for the users this does not really matter.
To remove duplicated code from the driver.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 39 +++++++++++++++------------------------ 1 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index c964f68..44c475c 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -688,31 +688,22 @@ static int omap_mcbsp_st_get_mode(struct snd_kcontrol *kcontrol, return 0; }
-static const struct snd_kcontrol_new omap_mcbsp2_st_controls[] = { - SOC_SINGLE_EXT("McBSP2 Sidetone Switch", 1, 0, 1, 0, - omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode), - OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 0 Volume", - -32768, 32767, - omap_mcbsp_get_st_ch0_volume, - omap_mcbsp_set_st_ch0_volume), - OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP2 Sidetone Channel 1 Volume", - -32768, 32767, - omap_mcbsp_get_st_ch1_volume, - omap_mcbsp_set_st_ch1_volume), -}; +#define OMAP_MCBSP_ST_CONTROLS(port) \ +static const struct snd_kcontrol_new omap_mcbsp##port##_st_controls[] = { \ +SOC_SINGLE_EXT("McBSP" #port " Sidetone Switch", 1, 0, 1, 0, \ + omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode), \ +OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP" #port " Sidetone Channel 0 Volume", \ + -32768, 32767, \ + omap_mcbsp_get_st_ch0_volume, \ + omap_mcbsp_set_st_ch0_volume), \ +OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP" #port " Sidetone Channel 1 Volume", \ + -32768, 32767, \ + omap_mcbsp_get_st_ch1_volume, \ + omap_mcbsp_set_st_ch1_volume), \ +}
-static const struct snd_kcontrol_new omap_mcbsp3_st_controls[] = { - SOC_SINGLE_EXT("McBSP3 Sidetone Switch", 2, 0, 1, 0, - omap_mcbsp_st_get_mode, omap_mcbsp_st_put_mode), - OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 0 Volume", - -32768, 32767, - omap_mcbsp_get_st_ch0_volume, - omap_mcbsp_set_st_ch0_volume), - OMAP_MCBSP_SOC_SINGLE_S16_EXT("McBSP3 Sidetone Channel 1 Volume", - -32768, 32767, - omap_mcbsp_get_st_ch1_volume, - omap_mcbsp_set_st_ch1_volume), -}; +OMAP_MCBSP_ST_CONTROLS(2); +OMAP_MCBSP_ST_CONTROLS(3);
int omap_mcbsp_st_add_controls(struct snd_soc_pcm_runtime *rtd) {
Since we always need to have set and get callbacks for McBSP sidetone it makes sense to combine the two macro to create the two callbacks.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@ti.com --- sound/soc/omap/omap-mcbsp.c | 17 +++++++---------- 1 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/sound/soc/omap/omap-mcbsp.c b/sound/soc/omap/omap-mcbsp.c index 44c475c..2e91a86 100644 --- a/sound/soc/omap/omap-mcbsp.c +++ b/sound/soc/omap/omap-mcbsp.c @@ -619,9 +619,9 @@ static int omap_mcbsp_st_info_volsw(struct snd_kcontrol *kcontrol, return 0; }
-#define OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(channel) \ +#define OMAP_MCBSP_ST_CHANNEL_VOLUME(channel) \ static int \ -omap_mcbsp_set_st_ch##channel##_volume(struct snd_kcontrol *kc, \ +omap_mcbsp_set_st_ch##channel##_volume(struct snd_kcontrol *kc, \ struct snd_ctl_elem_value *uc) \ { \ struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kc); \ @@ -637,11 +637,10 @@ omap_mcbsp_set_st_ch##channel##_volume(struct snd_kcontrol *kc, \ \ /* OMAP McBSP implementation uses index values 0..4 */ \ return omap_st_set_chgain(mcbsp, channel, val); \ -} - -#define OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(channel) \ +} \ + \ static int \ -omap_mcbsp_get_st_ch##channel##_volume(struct snd_kcontrol *kc, \ +omap_mcbsp_get_st_ch##channel##_volume(struct snd_kcontrol *kc, \ struct snd_ctl_elem_value *uc) \ { \ struct snd_soc_dai *cpu_dai = snd_kcontrol_chip(kc); \ @@ -655,10 +654,8 @@ omap_mcbsp_get_st_ch##channel##_volume(struct snd_kcontrol *kc, \ return 0; \ }
-OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(0) -OMAP_MCBSP_ST_SET_CHANNEL_VOLUME(1) -OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(0) -OMAP_MCBSP_ST_GET_CHANNEL_VOLUME(1) +OMAP_MCBSP_ST_CHANNEL_VOLUME(0) +OMAP_MCBSP_ST_CHANNEL_VOLUME(1)
static int omap_mcbsp_st_put_mode(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol)
On 08/22/2012 01:11 PM, Peter Ujfalusi wrote:
Hello,
Changes since v1:
- Macro for the ST control has been fixed
- New patch to combine the st channel volume set/get macros
Mark: this series depends on the McBSP DT support (to be precise patch 3 and 4 has dependency).
All,
Acked-by: Jarkko Nikula jarkko.nikula@bitmer.com
participants (3)
-
Jarkko Nikula
-
Mark Brown
-
Peter Ujfalusi