[alsa-devel] [PATCH 0/2] ASoC: sunxi: Add ADC support for A33
Hello everyone,
This is a first version of a serie to add ADC support for Sun8i-A33. Based on asoc/for-next branch, last commit: 20d5c84bef067b7e804a163e2abca16c47125bad.
The first patch adds the support of ADC and microphones in the digital part (driver sun8i-codec). The second one adds the route between analog (sun8i-codec-analog) and digital (sun8i-codec) parts to have a functional ADC route on Sun8i-A33 device tree.
Tested on Allwinner R16 Parrot board with microphone 1 (external) and microphone 2 (headset).
I am not sure about the bias handling so if you have any comments on it, do not hesitate.
Thank you in advance!
Best regards,
Mylène Josserand (2): ASoC: sun8i-codec: Add ADC support for a33 ARM: dts: sun8i: Add ADC routing
arch/arm/boot/dts/sun8i-a33.dtsi | 10 ++++- sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-)
Add ADC support for the sun8i-codec driver.
This driver uses all the microphones widgets and routes provided by the analog part (sun8i-codec-analog). Some digital configurations are needed by creating new ADC widgets and routes.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com --- sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index 5723c3404f6b..b4938b06acec 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -37,9 +37,11 @@ #define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0 #define SUN8I_MOD_CLK_ENA 0x010 #define SUN8I_MOD_CLK_ENA_AIF1 15 +#define SUN8I_MOD_CLK_ENA_ADC 3 #define SUN8I_MOD_CLK_ENA_DAC 2 #define SUN8I_MOD_RST_CTL 0x014 #define SUN8I_MOD_RST_CTL_AIF1 15 +#define SUN8I_MOD_RST_CTL_ADC 3 #define SUN8I_MOD_RST_CTL_DAC 2 #define SUN8I_SYS_SR_CTRL 0x018 #define SUN8I_SYS_SR_CTRL_AIF1_FS 12 @@ -54,9 +56,25 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 +#define SUN8I_AIF1_ADCDAT_CTRL 0x044 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14 #define SUN8I_AIF1_DACDAT_CTRL 0x048 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14 +#define SUN8I_AIF1_MXR_SRC 0x04c +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8 +#define SUN8I_ADC_DIG_CTRL 0x100 +#define SUN8I_ADC_DIG_CTRL_ENDA 15 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1 #define SUN8I_DAC_DIG_CTRL 0x120 #define SUN8I_DAC_DIG_CTRL_ENDA 15 #define SUN8I_DAC_MXR_SRC 0x130 @@ -276,10 +294,28 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = { SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0), };
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = { + SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch", + SUN8I_AIF1_MXR_SRC, + SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L, + SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1, 0), + SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC, + SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL, + SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1, 0), + SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC, + SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL, + SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0), + SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC, + SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR, + SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1, 0), +}; + static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { - /* Digital parts of the DACs */ + /* Digital parts of the DACs and ADC */ SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL, SUN8I_DAC_DIG_CTRL_ENDA, 0, NULL, 0), + SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA, + 0, NULL, 0),
/* Analog DAC AIF */ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0, @@ -289,17 +325,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
- /* DAC Mixers */ + /* Analog ADC AIF */ + SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0, + SUN8I_AIF1_ADCDAT_CTRL, + SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0), + SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0, + SUN8I_AIF1_ADCDAT_CTRL, + SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0), + + /* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), + SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0, + sun8i_input_mixer_controls), + SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0, + sun8i_input_mixer_controls),
/* Clocks */ SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0), + SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA, + SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL, SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL, @@ -316,6 +366,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL, SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0), + SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL, + SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0), + + SND_SOC_DAPM_MIC("Headset Mic", NULL), + SND_SOC_DAPM_MIC("Mic", NULL), + };
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { @@ -325,11 +381,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { { "RST AIF1", NULL, "AIF1 PLL" }, { "MODCLK AFI1", NULL, "RST AIF1" }, { "DAC", NULL, "MODCLK AFI1" }, + { "ADC", NULL, "MODCLK AFI1" },
{ "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" },
+ { "RST ADC", NULL, "SYSCLK" }, + { "MODCLK ADC", NULL, "RST ADC" }, + { "ADC", NULL, "MODCLK ADC" }, + /* DAC Routes */ { "AIF1 Slot 0 Right", NULL, "DAC" }, { "AIF1 Slot 0 Left", NULL, "DAC" }, @@ -339,6 +400,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { "AIF1 Slot 0 Left"}, { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 Slot 0 Right"}, + + /* ADC routes */ + { "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", + "AIF1 Slot 0 Left ADC" }, + { "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch", + "AIF1 Slot 0 Right ADC" }, };
static struct snd_soc_dai_ops sun8i_codec_dai_ops = { @@ -356,6 +423,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, }, + /* capture capabilities */ + .capture = { + .stream_name = "Capture", + .channels_min = 1, + .channels_max = 2, + .rates = SNDRV_PCM_RATE_8000_192000, + .formats = SNDRV_PCM_FMTBIT_S16_LE, + .sig_bits = 24, + }, /* pcm operations */ .ops = &sun8i_codec_dai_ops, };
Hi,
On Wed, May 3, 2017 at 9:56 PM, Mylène Josserand mylene.josserand@free-electrons.com wrote:
Add ADC support for the sun8i-codec driver.
This driver uses all the microphones widgets and routes provided by the analog part (sun8i-codec-analog). Some digital configurations are needed by creating new ADC widgets and routes.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com
sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index 5723c3404f6b..b4938b06acec 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -37,9 +37,11 @@ #define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0 #define SUN8I_MOD_CLK_ENA 0x010 #define SUN8I_MOD_CLK_ENA_AIF1 15 +#define SUN8I_MOD_CLK_ENA_ADC 3 #define SUN8I_MOD_CLK_ENA_DAC 2 #define SUN8I_MOD_RST_CTL 0x014 #define SUN8I_MOD_RST_CTL_AIF1 15 +#define SUN8I_MOD_RST_CTL_ADC 3 #define SUN8I_MOD_RST_CTL_DAC 2 #define SUN8I_SYS_SR_CTRL 0x018 #define SUN8I_SYS_SR_CTRL_AIF1_FS 12 @@ -54,9 +56,25 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 +#define SUN8I_AIF1_ADCDAT_CTRL 0x044 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14 #define SUN8I_AIF1_DACDAT_CTRL 0x048 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14 +#define SUN8I_AIF1_MXR_SRC 0x04c +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8 +#define SUN8I_ADC_DIG_CTRL 0x100 +#define SUN8I_ADC_DIG_CTRL_ENDA 15
The register bit name in the manual is ENAD, as in ENable ADc.
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1 #define SUN8I_DAC_DIG_CTRL 0x120 #define SUN8I_DAC_DIG_CTRL_ENDA 15 #define SUN8I_DAC_MXR_SRC 0x130 @@ -276,10 +294,28 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = { SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0), };
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = {
SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1, 0),SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1, 0),SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0),SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1, 0),+};
This group of mixer controls is only for AIF1 slot 0 capture. So in fact they should all read "AIF1 Slot 0 Mixer X Capture Switch", with X replaced by "AIF1 Slot 0", "AIF2", "ADC", and "AIF2 Inv".
This hopefully informs the user that these controls are for the AIF1 slot 0 mixer, and are capture switch from the various sources listed above.
static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
/* Digital parts of the DACs */
/* Digital parts of the DACs and ADC */ SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL, SUN8I_DAC_DIG_CTRL_ENDA, 0, NULL, 0),SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA,0, NULL, 0), /* Analog DAC AIF */ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,@@ -289,17 +325,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
/* DAC Mixers */
/* Analog ADC AIF */SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),
You want to use SND_SOC_DAPM_AIF_OUT here, for captured audio out of the codec and to the system. These are the last widgets in the capture path.
/* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls),SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),
And these should read "AIF1 Slot 0 Left/Right Mixer".
/* Clocks */ SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA,SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL, SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL,@@ -316,6 +366,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL, SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL,SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0),SND_SOC_DAPM_MIC("Headset Mic", NULL),SND_SOC_DAPM_MIC("Mic", NULL),
These Mics are board level widgets. Since you are using simple-card, you should add them in the device tree in the simple-card device node, using the "simple-audio-card,widgets" property.
You probably should have done so for the output side as well. If simple-card were fully-routed, the existing device tree simply wouldn't work.
};
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { @@ -325,11 +381,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { { "RST AIF1", NULL, "AIF1 PLL" }, { "MODCLK AFI1", NULL, "RST AIF1" }, { "DAC", NULL, "MODCLK AFI1" },
{ "ADC", NULL, "MODCLK AFI1" },
This makes no sense. Why would AIF1's module clock feed the ADC? Same goes for the existing DAC route.
{ "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" },
{ "RST ADC", NULL, "SYSCLK" },{ "MODCLK ADC", NULL, "RST ADC" },{ "ADC", NULL, "MODCLK ADC" },
This makes little sense either. The SYSCLK probably feeds the ADC's module clock. The MODCLK then feeds the ADC. The ADC reset feeds the ADC (or rather the ADC depends on its reset). The "ADC" widget here is actually just the enable switch. But we can use it as a kind of placeholder, to setup the dependencies correctly.
I wish we had named the widgets better, but alas they are part of the device tree binding, even though they are not documented, so we can not change the existing ones.
/* DAC Routes */ { "AIF1 Slot 0 Right", NULL, "DAC" }, { "AIF1 Slot 0 Left", NULL, "DAC" },@@ -339,6 +400,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { "AIF1 Slot 0 Left"}, { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 Slot 0 Right"},
/* ADC routes */{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch","AIF1 Slot 0 Left ADC" },{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch","AIF1 Slot 0 Right ADC" },
Same thing about the "ADC Mixer" names.
And these routes are completely backwards. "AIF1 Slot 0 Left/Right ADC" are the output stream widgets. They are fed from "AIF1 Slot 0 Left/Right Mixer" if you use the names I suggested, or "Left/Right Digital ADC Mixer" originally.
You then need other routes feeding the mixer. Looks like you also need ADC placeholder widgets on the digital side here, so you can hook up the analog side in simple-card more easily.
If you can, please dump the DAPM routes into a directed graph to check everything is connected and makes sense. I believe you have a script that does this.
Regards ChenYu
};
static struct snd_soc_dai_ops sun8i_codec_dai_ops = { @@ -356,6 +423,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, },
/* capture capabilities */.capture = {.stream_name = "Capture",.channels_min = 1,.channels_max = 2,.rates = SNDRV_PCM_RATE_8000_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE,.sig_bits = 24,}, /* pcm operations */ .ops = &sun8i_codec_dai_ops,};
2.11.0
Hi Chen-Yu,
Thank you for the review!
On 03/05/2017 19:13, Chen-Yu Tsai wrote:
Hi,
On Wed, May 3, 2017 at 9:56 PM, Mylène Josserand mylene.josserand@free-electrons.com wrote:
Add ADC support for the sun8i-codec driver.
This driver uses all the microphones widgets and routes provided by the analog part (sun8i-codec-analog). Some digital configurations are needed by creating new ADC widgets and routes.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com
sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index 5723c3404f6b..b4938b06acec 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -37,9 +37,11 @@ #define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0 #define SUN8I_MOD_CLK_ENA 0x010 #define SUN8I_MOD_CLK_ENA_AIF1 15 +#define SUN8I_MOD_CLK_ENA_ADC 3 #define SUN8I_MOD_CLK_ENA_DAC 2 #define SUN8I_MOD_RST_CTL 0x014 #define SUN8I_MOD_RST_CTL_AIF1 15 +#define SUN8I_MOD_RST_CTL_ADC 3 #define SUN8I_MOD_RST_CTL_DAC 2 #define SUN8I_SYS_SR_CTRL 0x018 #define SUN8I_SYS_SR_CTRL_AIF1_FS 12 @@ -54,9 +56,25 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 +#define SUN8I_AIF1_ADCDAT_CTRL 0x044 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14 #define SUN8I_AIF1_DACDAT_CTRL 0x048 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14 +#define SUN8I_AIF1_MXR_SRC 0x04c +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8 +#define SUN8I_ADC_DIG_CTRL 0x100 +#define SUN8I_ADC_DIG_CTRL_ENDA 15
The register bit name in the manual is ENAD, as in ENable ADc.
Okay, I did not know that, thanks.
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1 #define SUN8I_DAC_DIG_CTRL 0x120 #define SUN8I_DAC_DIG_CTRL_ENDA 15 #define SUN8I_DAC_MXR_SRC 0x130 @@ -276,10 +294,28 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = { SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0), };
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = {
SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1, 0),SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1, 0),SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0),SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch", SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1, 0),+};
This group of mixer controls is only for AIF1 slot 0 capture. So in fact they should all read "AIF1 Slot 0 Mixer X Capture Switch", with X replaced by "AIF1 Slot 0", "AIF2", "ADC", and "AIF2 Inv".
This hopefully informs the user that these controls are for the AIF1 slot 0 mixer, and are capture switch from the various sources listed above.
Sounds good.
static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
/* Digital parts of the DACs */
/* Digital parts of the DACs and ADC */ SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL, SUN8I_DAC_DIG_CTRL_ENDA, 0, NULL, 0),SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL, SUN8I_ADC_DIG_CTRL_ENDA,0, NULL, 0), /* Analog DAC AIF */ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,@@ -289,17 +325,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
/* DAC Mixers */
/* Analog ADC AIF */SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),You want to use SND_SOC_DAPM_AIF_OUT here, for captured audio out of the codec and to the system. These are the last widgets in the capture path.
ACK
/* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls),SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),And these should read "AIF1 Slot 0 Left/Right Mixer".
ACK
/* Clocks */ SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA,SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL, SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL,@@ -316,6 +366,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL, SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL,SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0),SND_SOC_DAPM_MIC("Headset Mic", NULL),SND_SOC_DAPM_MIC("Mic", NULL),These Mics are board level widgets. Since you are using simple-card, you should add them in the device tree in the simple-card device node, using the "simple-audio-card,widgets" property.
You probably should have done so for the output side as well. If simple-card were fully-routed, the existing device tree simply wouldn't work.
Okay, I will add them in the simple-audio-card node.
};
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { @@ -325,11 +381,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { { "RST AIF1", NULL, "AIF1 PLL" }, { "MODCLK AFI1", NULL, "RST AIF1" }, { "DAC", NULL, "MODCLK AFI1" },
{ "ADC", NULL, "MODCLK AFI1" },This makes no sense. Why would AIF1's module clock feed the ADC? Same goes for the existing DAC route.
It was difficult for me to know how to route the clocks with the DAC/ADC widgets. As the clocks are needed to have a ADC/DAC working, I created this route but it is, maybe, not the correct way to do it.
{ "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" },
{ "RST ADC", NULL, "SYSCLK" },{ "MODCLK ADC", NULL, "RST ADC" },{ "ADC", NULL, "MODCLK ADC" },This makes little sense either. The SYSCLK probably feeds the ADC's module clock. The MODCLK then feeds the ADC. The ADC reset feeds the ADC (or rather the ADC depends on its reset). The "ADC" widget here is actually just the enable switch. But we can use it as a kind of placeholder, to setup the dependencies correctly.
Yep, that it is. What do you have in mind about the ADC widget as a "placeholder"?
I wish we had named the widgets better, but alas they are part of the device tree binding, even though they are not documented, so we can not change the existing ones.
I agree about the widgets names, they are not very clear.
/* DAC Routes */ { "AIF1 Slot 0 Right", NULL, "DAC" }, { "AIF1 Slot 0 Left", NULL, "DAC" },@@ -339,6 +400,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { "AIF1 Slot 0 Left"}, { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 Slot 0 Right"},
/* ADC routes */{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch","AIF1 Slot 0 Left ADC" },{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC Capture Switch","AIF1 Slot 0 Right ADC" },Same thing about the "ADC Mixer" names.
And these routes are completely backwards. "AIF1 Slot 0 Left/Right ADC" are the output stream widgets. They are fed from "AIF1 Slot 0 Left/Right Mixer" if you use the names I suggested, or "Left/Right Digital ADC Mixer" originally.
You are right, it make sense now.
You then need other routes feeding the mixer. Looks like you also need ADC placeholder widgets on the digital side here, so you can hook up the analog side in simple-card more easily.
Okay, I will try to improve it.
If you can, please dump the DAPM routes into a directed graph to check everything is connected and makes sense. I believe you have a script that does this.
Yes, I tested it and the ADC mixers are connected to nothing so this route does not make sense. I will rework it.
};
static struct snd_soc_dai_ops sun8i_codec_dai_ops = { @@ -356,6 +423,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, },
/* capture capabilities */.capture = {.stream_name = "Capture",.channels_min = 1,.channels_max = 2,.rates = SNDRV_PCM_RATE_8000_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE,.sig_bits = 24,}, /* pcm operations */ .ops = &sun8i_codec_dai_ops,};
2.11.0
Thank you again,
Best regards,
Hi,
On Wed, May 10, 2017 at 2:58 PM, Mylene Josserand mylene.josserand@free-electrons.com wrote:
Hi Chen-Yu,
Thank you for the review!
On 03/05/2017 19:13, Chen-Yu Tsai wrote:
Hi,
On Wed, May 3, 2017 at 9:56 PM, Mylène Josserand mylene.josserand@free-electrons.com wrote:
Add ADC support for the sun8i-codec driver.
This driver uses all the microphones widgets and routes provided by the analog part (sun8i-codec-analog). Some digital configurations are needed by creating new ADC widgets and routes.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com
sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index 5723c3404f6b..b4938b06acec 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -37,9 +37,11 @@ #define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0 #define SUN8I_MOD_CLK_ENA 0x010 #define SUN8I_MOD_CLK_ENA_AIF1 15 +#define SUN8I_MOD_CLK_ENA_ADC 3 #define SUN8I_MOD_CLK_ENA_DAC 2 #define SUN8I_MOD_RST_CTL 0x014 #define SUN8I_MOD_RST_CTL_AIF1 15 +#define SUN8I_MOD_RST_CTL_ADC 3 #define SUN8I_MOD_RST_CTL_DAC 2 #define SUN8I_SYS_SR_CTRL 0x018 #define SUN8I_SYS_SR_CTRL_AIF1_FS 12 @@ -54,9 +56,25 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 +#define SUN8I_AIF1_ADCDAT_CTRL 0x044 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14 #define SUN8I_AIF1_DACDAT_CTRL 0x048 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14 +#define SUN8I_AIF1_MXR_SRC 0x04c +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8 +#define SUN8I_ADC_DIG_CTRL 0x100 +#define SUN8I_ADC_DIG_CTRL_ENDA 15
The register bit name in the manual is ENAD, as in ENable ADc.
Okay, I did not know that, thanks.
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1 #define SUN8I_DAC_DIG_CTRL 0x120 #define SUN8I_DAC_DIG_CTRL_ENDA 15 #define SUN8I_DAC_MXR_SRC 0x130 @@ -276,10 +294,28 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = { SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0), };
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = {
SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1, 0),SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1, 0),SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0),SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1, 0),+};
This group of mixer controls is only for AIF1 slot 0 capture. So in fact they should all read "AIF1 Slot 0 Mixer X Capture Switch", with X replaced by "AIF1 Slot 0", "AIF2", "ADC", and "AIF2 Inv".
This hopefully informs the user that these controls are for the AIF1 slot 0 mixer, and are capture switch from the various sources listed above.
Sounds good.
static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
/* Digital parts of the DACs */
/* Digital parts of the DACs and ADC */ SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL,SUN8I_DAC_DIG_CTRL_ENDA, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL,SUN8I_ADC_DIG_CTRL_ENDA,
0, NULL, 0), /* Analog DAC AIF */ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,@@ -289,17 +325,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
/* DAC Mixers */
/* Analog ADC AIF */SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),You want to use SND_SOC_DAPM_AIF_OUT here, for captured audio out of the codec and to the system. These are the last widgets in the capture path.
ACK
/* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls),SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),And these should read "AIF1 Slot 0 Left/Right Mixer".
ACK
/* Clocks */ SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA,SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL, SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL,@@ -316,6 +366,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL, SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL,SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0),SND_SOC_DAPM_MIC("Headset Mic", NULL),SND_SOC_DAPM_MIC("Mic", NULL),These Mics are board level widgets. Since you are using simple-card, you should add them in the device tree in the simple-card device node, using the "simple-audio-card,widgets" property.
You probably should have done so for the output side as well. If simple-card were fully-routed, the existing device tree simply wouldn't work.
Okay, I will add them in the simple-audio-card node.
};
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { @@ -325,11 +381,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { { "RST AIF1", NULL, "AIF1 PLL" }, { "MODCLK AFI1", NULL, "RST AIF1" }, { "DAC", NULL, "MODCLK AFI1" },
{ "ADC", NULL, "MODCLK AFI1" },This makes no sense. Why would AIF1's module clock feed the ADC? Same goes for the existing DAC route.
It was difficult for me to know how to route the clocks with the DAC/ADC widgets. As the clocks are needed to have a ADC/DAC working, I created this route but it is, maybe, not the correct way to do it.
The AC100 codec chip is very much like the A33's audio codec, except it has additional clocks and resets, being a separate chip and all.
The manual has a diagram for the clock tree. This should help you understand and extrapolate how the A33 audio codec's clocks are wired up.
{ "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" },
{ "RST ADC", NULL, "SYSCLK" },{ "MODCLK ADC", NULL, "RST ADC" },{ "ADC", NULL, "MODCLK ADC" },This makes little sense either. The SYSCLK probably feeds the ADC's module clock. The MODCLK then feeds the ADC. The ADC reset feeds the ADC (or rather the ADC depends on its reset). The "ADC" widget here is actually just the enable switch. But we can use it as a kind of placeholder, to setup the dependencies correctly.
Yep, that it is. What do you have in mind about the ADC widget as a "placeholder"?
This particular instance works, and it implies some of the dependencies. Though maybe you could rename it as "ADC Digital Enable", to differentiate from other ADC-related widgets.
I wish we had named the widgets better, but alas they are part of the device tree binding, even though they are not documented, so we can not change the existing ones.
I agree about the widgets names, they are not very clear.
/* DAC Routes */ { "AIF1 Slot 0 Right", NULL, "DAC" }, { "AIF1 Slot 0 Left", NULL, "DAC" },@@ -339,6 +400,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { "AIF1 Slot 0 Left"}, { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 Slot 0 Right"},
/* ADC routes */{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC CaptureSwitch",
"AIF1 Slot 0 Left ADC" },{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC CaptureSwitch",
"AIF1 Slot 0 Right ADC" },Same thing about the "ADC Mixer" names.
And these routes are completely backwards. "AIF1 Slot 0 Left/Right ADC" are the output stream widgets. They are fed from "AIF1 Slot 0 Left/Right Mixer" if you use the names I suggested, or "Left/Right Digital ADC Mixer" originally.
You are right, it make sense now.
You then need other routes feeding the mixer. Looks like you also need ADC placeholder widgets on the digital side here, so you can hook up the analog side in simple-card more easily.
Okay, I will try to improve it.
I think you could have "Digital Left/Right ADC" ADC widgets on the digital side. Then in the device tree you could hook up "Digital Left/Right ADC" with "Left/Right ADC".
ChenYu
If you can, please dump the DAPM routes into a directed graph to check everything is connected and makes sense. I believe you have a script that does this.
Yes, I tested it and the ADC mixers are connected to nothing so this route does not make sense. I will rework it.
};
static struct snd_soc_dai_ops sun8i_codec_dai_ops = { @@ -356,6 +423,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai = { .rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, },
/* capture capabilities */.capture = {.stream_name = "Capture",.channels_min = 1,.channels_max = 2,.rates = SNDRV_PCM_RATE_8000_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE,.sig_bits = 24,}, /* pcm operations */ .ops = &sun8i_codec_dai_ops,};
2.11.0
Thank you again,
Best regards,
-- Mylène Josserand, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Hi all,
I am new to sun8i-h3 based board.
I am trying to up ethernet on sun8i-h3 based board.
can anyone help out.
On Tue, May 16, 2017 at 11:00 AM, Chen-Yu Tsai wens@csie.org wrote:
Hi,
On Wed, May 10, 2017 at 2:58 PM, Mylene Josserand mylene.josserand@free-electrons.com wrote:
Hi Chen-Yu,
Thank you for the review!
On 03/05/2017 19:13, Chen-Yu Tsai wrote:
Hi,
On Wed, May 3, 2017 at 9:56 PM, Mylène Josserand mylene.josserand@free-electrons.com wrote:
Add ADC support for the sun8i-codec driver.
This driver uses all the microphones widgets and routes provided by the analog part (sun8i-codec-analog). Some digital configurations are needed by creating new ADC widgets and routes.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com
sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/sound/soc/sunxi/sun8i-codec.c b/sound/soc/sunxi/sun8i-codec.c index 5723c3404f6b..b4938b06acec 100644 --- a/sound/soc/sunxi/sun8i-codec.c +++ b/sound/soc/sunxi/sun8i-codec.c @@ -37,9 +37,11 @@ #define SUN8I_SYSCLK_CTL_SYSCLK_SRC 0 #define SUN8I_MOD_CLK_ENA 0x010 #define SUN8I_MOD_CLK_ENA_AIF1 15 +#define SUN8I_MOD_CLK_ENA_ADC 3 #define SUN8I_MOD_CLK_ENA_DAC 2 #define SUN8I_MOD_RST_CTL 0x014 #define SUN8I_MOD_RST_CTL_AIF1 15 +#define SUN8I_MOD_RST_CTL_ADC 3 #define SUN8I_MOD_RST_CTL_DAC 2 #define SUN8I_SYS_SR_CTRL 0x018 #define SUN8I_SYS_SR_CTRL_AIF1_FS 12 @@ -54,9 +56,25 @@ #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ 4 #define SUN8I_AIF1CLK_CTRL_AIF1_WORD_SIZ_16 (1 << 4) #define SUN8I_AIF1CLK_CTRL_AIF1_DATA_FMT 2 +#define SUN8I_AIF1_ADCDAT_CTRL 0x044 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA 15 +#define SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA 14 #define SUN8I_AIF1_DACDAT_CTRL 0x048 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0L_ENA 15 #define SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA 14 +#define SUN8I_AIF1_MXR_SRC 0x04c +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L 15 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL 14 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL 13 +#define SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR 12 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R 11 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR 10 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR 9 +#define SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL 8 +#define SUN8I_ADC_DIG_CTRL 0x100 +#define SUN8I_ADC_DIG_CTRL_ENDA 15
The register bit name in the manual is ENAD, as in ENable ADc.
Okay, I did not know that, thanks.
+#define SUN8I_ADC_DIG_CTRL_ADOUT_DTS 2 +#define SUN8I_ADC_DIG_CTRL_ADOUT_DLY 1 #define SUN8I_DAC_DIG_CTRL 0x120 #define SUN8I_DAC_DIG_CTRL_ENDA 15 #define SUN8I_DAC_MXR_SRC 0x130 @@ -276,10 +294,28 @@ static const struct snd_kcontrol_new sun8i_dac_mixer_controls[] = { SUN8I_DAC_MXR_SRC_DACR_MXR_SRC_ADCR, 1, 0), };
+static const struct snd_kcontrol_new sun8i_input_mixer_controls[] = {
SOC_DAPM_DOUBLE("AIF1 Slot 0 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF1DA0L,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF1DA0R, 1,0),
SOC_DAPM_DOUBLE("AIF2 Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACR, 1,0),
SOC_DAPM_DOUBLE("AIF1 Data Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_ADCL,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_ADCR, 1, 0),SOC_DAPM_DOUBLE("AIF2 Inv Digital ADC Capture Switch",SUN8I_AIF1_MXR_SRC,
SUN8I_AIF1_MXR_SRC_AD0L_MXL_SRC_AIF2DACR,SUN8I_AIF1_MXR_SRC_AD0R_MXR_SRC_AIF2DACL, 1,0),
+};
This group of mixer controls is only for AIF1 slot 0 capture. So in fact they should all read "AIF1 Slot 0 Mixer X Capture Switch", with X replaced by "AIF1 Slot 0", "AIF2", "ADC", and "AIF2 Inv".
This hopefully informs the user that these controls are for the AIF1 slot 0 mixer, and are capture switch from the various sources listed above.
Sounds good.
static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = {
/* Digital parts of the DACs */
/* Digital parts of the DACs and ADC */ SND_SOC_DAPM_SUPPLY("DAC", SUN8I_DAC_DIG_CTRL,SUN8I_DAC_DIG_CTRL_ENDA, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("ADC", SUN8I_ADC_DIG_CTRL,SUN8I_ADC_DIG_CTRL_ENDA,
0, NULL, 0), /* Analog DAC AIF */ SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left", "Playback", 0,@@ -289,17 +325,31 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_AIF1_DACDAT_CTRL, SUN8I_AIF1_DACDAT_CTRL_AIF1_DA0R_ENA, 0),
/* DAC Mixers */
/* Analog ADC AIF */SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Left ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0L_ENA, 0),SND_SOC_DAPM_AIF_IN("AIF1 Slot 0 Right ADC", "Capture", 0,SUN8I_AIF1_ADCDAT_CTRL,SUN8I_AIF1_ADCDAT_CTRL_AIF1_DA0R_ENA, 0),You want to use SND_SOC_DAPM_AIF_OUT here, for captured audio out of the codec and to the system. These are the last widgets in the capture path.
ACK
/* DAC and ADC Mixers */ SOC_MIXER_ARRAY("Left Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls), SOC_MIXER_ARRAY("Right Digital DAC Mixer", SND_SOC_NOPM, 0, 0, sun8i_dac_mixer_controls),SOC_MIXER_ARRAY("Left Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),SOC_MIXER_ARRAY("Right Digital ADC Mixer", SND_SOC_NOPM, 0, 0,sun8i_input_mixer_controls),And these should read "AIF1 Slot 0 Left/Right Mixer".
ACK
/* Clocks */ SND_SOC_DAPM_SUPPLY("MODCLK AFI1", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("MODCLK DAC", SUN8I_MOD_CLK_ENA, SUN8I_MOD_CLK_ENA_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("MODCLK ADC", SUN8I_MOD_CLK_ENA,SUN8I_MOD_CLK_ENA_ADC, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("AIF1", SUN8I_SYSCLK_CTL, SUN8I_SYSCLK_CTL_AIF1CLK_ENA, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("SYSCLK", SUN8I_SYSCLK_CTL,@@ -316,6 +366,12 @@ static const struct snd_soc_dapm_widget sun8i_codec_dapm_widgets[] = { SUN8I_MOD_RST_CTL_AIF1, 0, NULL, 0), SND_SOC_DAPM_SUPPLY("RST DAC", SUN8I_MOD_RST_CTL, SUN8I_MOD_RST_CTL_DAC, 0, NULL, 0),
SND_SOC_DAPM_SUPPLY("RST ADC", SUN8I_MOD_RST_CTL,SUN8I_MOD_RST_CTL_ADC, 0, NULL, 0),SND_SOC_DAPM_MIC("Headset Mic", NULL),SND_SOC_DAPM_MIC("Mic", NULL),These Mics are board level widgets. Since you are using simple-card, you should add them in the device tree in the simple-card device node, using the "simple-audio-card,widgets" property.
You probably should have done so for the output side as well. If simple-card were fully-routed, the existing device tree simply wouldn't work.
Okay, I will add them in the simple-audio-card node.
};
static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { @@ -325,11 +381,16 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { { "RST AIF1", NULL, "AIF1 PLL" }, { "MODCLK AFI1", NULL, "RST AIF1" }, { "DAC", NULL, "MODCLK AFI1" },
{ "ADC", NULL, "MODCLK AFI1" },This makes no sense. Why would AIF1's module clock feed the ADC? Same
goes
for the existing DAC route.
It was difficult for me to know how to route the clocks with the DAC/ADC widgets. As the clocks are needed to have a ADC/DAC working, I created
this
route but it is, maybe, not the correct way to do it.
The AC100 codec chip is very much like the A33's audio codec, except it has additional clocks and resets, being a separate chip and all.
The manual has a diagram for the clock tree. This should help you understand and extrapolate how the A33 audio codec's clocks are wired up.
{ "RST DAC", NULL, "SYSCLK" }, { "MODCLK DAC", NULL, "RST DAC" }, { "DAC", NULL, "MODCLK DAC" },
{ "RST ADC", NULL, "SYSCLK" },{ "MODCLK ADC", NULL, "RST ADC" },{ "ADC", NULL, "MODCLK ADC" },This makes little sense either. The SYSCLK probably feeds the ADC's module clock. The MODCLK then feeds the ADC. The ADC reset feeds the ADC (or rather
the
ADC depends on its reset). The "ADC" widget here is actually just the enable switch. But we can use it as a kind of placeholder, to setup the dependencies correctly.
Yep, that it is. What do you have in mind about the ADC widget as a "placeholder"?
This particular instance works, and it implies some of the dependencies. Though maybe you could rename it as "ADC Digital Enable", to differentiate from other ADC-related widgets.
I wish we had named the widgets better, but alas they are part of the device tree binding, even though they are not documented, so we can not change the existing ones.
I agree about the widgets names, they are not very clear.
/* DAC Routes */ { "AIF1 Slot 0 Right", NULL, "DAC" }, { "AIF1 Slot 0 Left", NULL, "DAC" },@@ -339,6 +400,12 @@ static const struct snd_soc_dapm_route sun8i_codec_dapm_routes[] = { "AIF1 Slot 0 Left"}, { "Right Digital DAC Mixer", "AIF1 Slot 0 Digital DAC Playback Switch", "AIF1 Slot 0 Right"},
/* ADC routes */{ "Left Digital ADC Mixer", "AIF1 Data Digital ADC CaptureSwitch",
"AIF1 Slot 0 Left ADC" },{ "Right Digital ADC Mixer", "AIF1 Data Digital ADC CaptureSwitch",
"AIF1 Slot 0 Right ADC" },Same thing about the "ADC Mixer" names.
And these routes are completely backwards. "AIF1 Slot 0 Left/Right ADC" are the output stream widgets. They are fed from "AIF1 Slot 0 Left/Right Mixer" if you use the names I suggested, or "Left/Right Digital ADC Mixer" originally.
You are right, it make sense now.
You then need other routes feeding the mixer. Looks like you also need
ADC
placeholder widgets on the digital side here, so you can hook up the analog side in simple-card more easily.
Okay, I will try to improve it.
I think you could have "Digital Left/Right ADC" ADC widgets on the digital side. Then in the device tree you could hook up "Digital Left/Right ADC" with "Left/Right ADC".
ChenYu
If you can, please dump the DAPM routes into a directed graph to check everything is connected and makes sense. I believe you have a script that does
this.
Yes, I tested it and the ADC mixers are connected to nothing so this
route
does not make sense. I will rework it.
};
static struct snd_soc_dai_ops sun8i_codec_dai_ops = { @@ -356,6 +423,15 @@ static struct snd_soc_dai_driver sun8i_codec_dai
= {
.rates = SNDRV_PCM_RATE_8000_192000, .formats = SNDRV_PCM_FMTBIT_S16_LE, },
/* capture capabilities */.capture = {.stream_name = "Capture",.channels_min = 1,.channels_max = 2,.rates = SNDRV_PCM_RATE_8000_192000,.formats = SNDRV_PCM_FMTBIT_S16_LE,.sig_bits = 24,}, /* pcm operations */ .ops = &sun8i_codec_dai_ops,};
2.11.0
Thank you again,
Best regards,
-- Mylène Josserand, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Add the ADC route between the analog and the digital parts of sun8i A33. Configure the MIC1 to use MBIAS and MIC2 to use HBIAS.
Signed-off-by: Mylène Josserand mylene.josserand@free-electrons.com --- arch/arm/boot/dts/sun8i-a33.dtsi | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sun8i-a33.dtsi b/arch/arm/boot/dts/sun8i-a33.dtsi index 306af6cadf26..0d3f7b5a23ef 100644 --- a/arch/arm/boot/dts/sun8i-a33.dtsi +++ b/arch/arm/boot/dts/sun8i-a33.dtsi @@ -114,7 +114,15 @@ simple-audio-card,aux-devs = <&codec_analog>; simple-audio-card,routing = "Left DAC", "AIF1 Slot 0 Left", - "Right DAC", "AIF1 Slot 0 Right"; + "Right DAC", "AIF1 Slot 0 Right", + "AIF1 Slot 0 Left ADC", "Left ADC", + "AIF1 Slot 0 Right ADC", "Right ADC", + "Left ADC", "ADC", + "Right ADC", "ADC", + "Mic", "MBIAS", + "Headset Mic", "HBIAS", + "MIC1", "Mic", + "MIC2", "Headset Mic"; status = "disabled";
simple-audio-card,cpu {
Hello,
On 03/05/2017 15:56, Mylène Josserand wrote:
Hello everyone,
This is a first version of a serie to add ADC support for Sun8i-A33. Based on asoc/for-next branch, last commit: 20d5c84bef067b7e804a163e2abca16c47125bad.
The first patch adds the support of ADC and microphones in the digital part (driver sun8i-codec). The second one adds the route between analog (sun8i-codec-analog) and digital (sun8i-codec) parts to have a functional ADC route on Sun8i-A33 device tree.
Tested on Allwinner R16 Parrot board with microphone 1 (external) and microphone 2 (headset).
I forgot to add in my cover-letter amixer commands used to test it:
amixer set 'AIF1 Data Digital ADC' cap amixer set 'Mic1' cap or amixer set 'Mic2' cap
and, just in case, the commands to enable output/DAC:
amixer set 'Headphone' 50% amixer set 'Headphone' on amixer set 'DAC' on amixer set 'AIF1 Slot 0 Digital DAC' on
I am not sure about the bias handling so if you have any comments on it, do not hesitate.
Thank you in advance!
Best regards,
Mylène Josserand (2): ASoC: sun8i-codec: Add ADC support for a33 ARM: dts: sun8i: Add ADC routing
arch/arm/boot/dts/sun8i-a33.dtsi | 10 ++++- sound/soc/sunxi/sun8i-codec.c | 80 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 87 insertions(+), 3 deletions(-)
Best regards,
participants (3)
-
Chen-Yu Tsai -
Mahesh Nanavalla -
Mylène Josserand