Hi,
this is the eighth version of the patch set that adds inputs to sun4i-codec.
Changes compared to v7 are:
- fix the routes for line and mic capturing.
The patches are on top of the branch "sunxi-next" in
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>.
Regards,
Danny
---
Danny Milosavljevic (2):
sun4i-codec: Add FM, Line and Mic inputs
sun4i-codec: make it possible to use different codec_widgets for A10 and A20.
b/sound/soc/sunxi/sun4i-codec.c | 230 +++++++++++++++++++++++++++++++++++++---
1 file changed, 215 insertions(+), 15 deletions(-)
This is the first part:
sun4i-codec: make it possible to use different codec_widgets for A10 and A20.
Signed-off-by: Danny Milosavljevic <[email protected]>
---
b/sound/soc/sunxi/sun4i-codec.c | 45 ++++++++++++++++++++++++++++++----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index e6cc6a1..6628e6e 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -96,8 +96,9 @@
/* Other various ADC registers */
#define SUN4I_CODEC_DAC_TXCNT (0x30)
#define SUN4I_CODEC_ADC_RXCNT (0x34)
-#define SUN4I_CODEC_AC_SYS_VERI (0x38)
-#define SUN4I_CODEC_AC_MIC_PHONE_CAL (0x3c)
+
+#define SUN7I_CODEC_AC_DAC_CAL (0x38)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
struct sun4i_codec {
struct device *dev;
@@ -509,10 +510,13 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
-static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
- SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
- SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
- sun4i_codec_pa_volume_scale),
+#define SUN4I_COMMON_CODEC_WIDGETS \
+ SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
+ SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
+ sun4i_codec_pa_volume_scale)
+
+static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
+ SUN4I_COMMON_CODEC_WIDGETS,
};
static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
@@ -627,9 +631,9 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
{ "Mic1", NULL, "VMIC" },
};
-static struct snd_soc_codec_driver sun4i_codec_codec = {
- .controls = sun4i_codec_widgets,
- .num_controls = ARRAY_SIZE(sun4i_codec_widgets),
+static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
+ .controls = sun4i_codec_widgets_a10,
+ .num_controls = ARRAY_SIZE(sun4i_codec_widgets_a10),
.dapm_widgets = sun4i_codec_codec_dapm_widgets,
.num_dapm_widgets = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
.dapm_routes = sun4i_codec_codec_dapm_routes,
@@ -680,7 +684,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
.reg_bits = 32,
.reg_stride = 4,
.val_bits = 32,
- .max_register = SUN4I_CODEC_AC_MIC_PHONE_CAL,
+ .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
};
static const struct of_device_id sun4i_codec_of_match[] = {
@@ -753,10 +757,24 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
return card;
};
+static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
+ SUN4I_COMMON_CODEC_WIDGETS,
+};
+
+static struct snd_soc_codec_driver sun7i_codec_codec = {
+ .controls = sun7i_codec_widgets,
+ .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
+ .dapm_widgets = sun4i_codec_codec_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
+ .dapm_routes = sun4i_codec_codec_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
+};
+
static int sun4i_codec_probe(struct platform_device *pdev)
{
struct snd_soc_card *card;
struct sun4i_codec *scodec;
+ struct snd_soc_codec_driver *codec;
struct resource *res;
void __iomem *base;
int ret;
@@ -819,7 +837,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
scodec->capture_dma_data.maxburst = 4;
scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
- ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
+ if (of_device_is_compatible(pdev->dev.of_node,
+ "allwinner,sun7i-a20-codec"))
+ codec = &sun7i_codec_codec;
+ else
+ codec = &sun4i_codec_codec_a10;
+ ret = snd_soc_register_codec(&pdev->dev, codec,
&sun4i_codec_dai, 1);
if (ret) {
dev_err(&pdev->dev, "Failed to register our codec\n");
This is the second part, actually adding FM, Line and Mic inputs.
Signed-off-by: Danny Milosavljevic <[email protected]>
---
b/sound/soc/sunxi/sun4i-codec.c | 185 +++++++++++++++++++++++++++++++++++++++-
1 file changed, 181 insertions(+), 4 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 6628e6e..f23d8a9 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -59,9 +59,20 @@
#define SUN4I_CODEC_DAC_ACTL_DACAENR (31)
#define SUN4I_CODEC_DAC_ACTL_DACAENL (30)
#define SUN4I_CODEC_DAC_ACTL_MIXEN (29)
+#define SUN4I_CODEC_DAC_ACTL_LNG (26)
+#define SUN4I_CODEC_DAC_ACTL_FMG (23)
+#define SUN4I_CODEC_DAC_ACTL_MICG (20)
+#define SUN4I_CODEC_DAC_ACTL_LLNS (19)
+#define SUN4I_CODEC_DAC_ACTL_RLNS (18)
+#define SUN4I_CODEC_DAC_ACTL_LFMS (17)
+#define SUN4I_CODEC_DAC_ACTL_RFMS (16)
#define SUN4I_CODEC_DAC_ACTL_LDACLMIXS (15)
#define SUN4I_CODEC_DAC_ACTL_RDACRMIXS (14)
#define SUN4I_CODEC_DAC_ACTL_LDACRMIXS (13)
+#define SUN4I_CODEC_DAC_ACTL_MIC1LS (12)
+#define SUN4I_CODEC_DAC_ACTL_MIC1RS (11)
+#define SUN4I_CODEC_DAC_ACTL_MIC2LS (10)
+#define SUN4I_CODEC_DAC_ACTL_MIC2RS (9)
#define SUN4I_CODEC_DAC_ACTL_DACPAS (8)
#define SUN4I_CODEC_DAC_ACTL_MIXPAS (7)
#define SUN4I_CODEC_DAC_ACTL_PA_MUTE (6)
@@ -87,8 +98,11 @@
#define SUN4I_CODEC_ADC_ACTL_PREG1EN (29)
#define SUN4I_CODEC_ADC_ACTL_PREG2EN (28)
#define SUN4I_CODEC_ADC_ACTL_VMICEN (27)
-#define SUN4I_CODEC_ADC_ACTL_VADCG (20)
+#define SUN4I_CODEC_ADC_ACTL_PREG1_A10 (25)
+#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
+#define SUN4I_CODEC_ADC_ACTL_ADCG (20)
#define SUN4I_CODEC_ADC_ACTL_ADCIS (17)
+#define SUN4I_CODEC_ADC_ACTL_LNRDF (16)
#define SUN4I_CODEC_ADC_ACTL_PA_EN (4)
#define SUN4I_CODEC_ADC_ACTL_DDE (3)
#define SUN4I_CODEC_ADC_DEBUG (0x2c)
@@ -100,6 +114,16 @@
#define SUN7I_CODEC_AC_DAC_CAL (0x38)
#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1 (29)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2 (26)
+/* note: no idea where the output pins for the following are. */
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTG (5)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTEN (4)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS3 (3)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS2 (2)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS1 (1)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS0 (0)
+
struct sun4i_codec {
struct device *dev;
struct regmap *regmap;
@@ -509,19 +533,102 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
SUN4I_CODEC_DAC_ACTL_PA_MUTE, 1, 0);
static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_linein_loopback_gain_scale,
+ -150,
+ 150,
+ 0);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_fmin_loopback_gain_scale,
+ -450,
+ 150,
+ 0);
+static DECLARE_TLV_DB_SCALE(sun4i_codec_micin_loopback_gain_scale,
+ -450,
+ 150,
+ 0);
+static DECLARE_TLV_DB_RANGE(sun4i_codec_micin_preamp_gain_scale_a10,
+ 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
+ 1, 7, TLV_DB_SCALE_ITEM(3500, 300, 0));
+static DECLARE_TLV_DB_SCALE(sun4i_codec_adc_gain_scale, -450, 150, 0);
+/* Sources:
+ * A10 User Manual v1.5 20130820
+ * A20 User Manual v1.4 20150510
+ */
+static const char * const sun4i_codec_capture_source[] = {
+ "Line-In",
+ "FM",
+ "Mic1",
+ "Mic2",
+ "Mic1,Mic2",
+ "Mic1+Mic2",
+ "Output Mixer",
+ "Line-In,Mic1",
+};
+static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_ADCIS,
+ sun4i_codec_capture_source);
+
+static const struct snd_kcontrol_new sun4i_codec_capture_source_controls =
+ SOC_DAPM_ENUM("Route", sun4i_codec_enum_capture_source);
#define SUN4I_COMMON_CODEC_WIDGETS \
- SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
- SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
- sun4i_codec_pa_volume_scale)
+ SOC_SINGLE_TLV("Power Amplifier Playback Volume", SUN4I_CODEC_DAC_ACTL,\
+ SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
+ sun4i_codec_pa_volume_scale), \
+ /* Line-In, FM, Mic1, Mic2 */ \
+ SOC_SINGLE_TLV("Line-In Playback Volume", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_LNG, \
+ 1, \
+ 0, \
+ sun4i_codec_linein_loopback_gain_scale), \
+ SOC_SINGLE_TLV("FM Playback Volume", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_FMG, \
+ 3, \
+ 0, \
+ sun4i_codec_fmin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic Playback Volume", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_MICG, \
+ 7, \
+ 0, \
+ sun4i_codec_micin_loopback_gain_scale), \
+ /* ADC */ \
+ SOC_SINGLE_TLV("Capture Volume", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_ADCG, \
+ 4, \
+ 0, \
+ sun4i_codec_adc_gain_scale)
static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
SUN4I_COMMON_CODEC_WIDGETS,
+ SOC_SINGLE_TLV("Mic1 Capture Volume",
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG1_A10,
+ 3,
+ 0,
+ sun4i_codec_micin_preamp_gain_scale_a10),
+ SOC_SINGLE_TLV("Mic2 Capture Volume",
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG2_A10,
+ 3,
+ 0,
+ sun4i_codec_micin_preamp_gain_scale_a10),
};
static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
+ SOC_DAPM_SINGLE("Left Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LLNS, 1, 0),
+ SOC_DAPM_SINGLE("Left FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
@@ -529,6 +636,14 @@ static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
+ SOC_DAPM_SINGLE("Right Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
+ SOC_DAPM_SINGLE("Right FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
@@ -561,6 +676,10 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
SND_SOC_DAPM_DAC("Right DAC", "Codec Playback", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_DACAENR, 0),
+ /* MUX */
+ SND_SOC_DAPM_MUX("Capture Source", SND_SOC_NOPM, 0, 0,
+ &sun4i_codec_capture_source_controls),
+
/* Mixers */
SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
sun4i_codec_left_mixer_controls,
@@ -580,6 +699,8 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
/* Mic Pre-Amplifiers */
SND_SOC_DAPM_PGA("MIC1 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
SUN4I_CODEC_ADC_ACTL_PREG1EN, 0, NULL, 0),
+ SND_SOC_DAPM_PGA("MIC2 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG2EN, 0, NULL, 0),
/* Power Amplifier */
SND_SOC_DAPM_MIXER("Power Amplifier", SUN4I_CODEC_ADC_ACTL,
@@ -590,9 +711,15 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
&sun4i_codec_pa_mute),
SND_SOC_DAPM_INPUT("Mic1"),
+ SND_SOC_DAPM_INPUT("Mic2"),
SND_SOC_DAPM_OUTPUT("HP Right"),
SND_SOC_DAPM_OUTPUT("HP Left"),
+
+ SND_SOC_DAPM_INPUT("Line-In Right"),
+ SND_SOC_DAPM_INPUT("Line-In Left"),
+ SND_SOC_DAPM_INPUT("FM Right"),
+ SND_SOC_DAPM_INPUT("FM Left"),
};
static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
@@ -629,6 +756,39 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
{ "Right ADC", NULL, "MIC1 Pre-Amplifier" },
{ "MIC1 Pre-Amplifier", NULL, "Mic1"},
{ "Mic1", NULL, "VMIC" },
+ { "Right Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
+ { "Left Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
+
+ /* Mic2 Routes */
+ { "Left ADC", NULL, "MIC2 Pre-Amplifier" },
+ { "Right ADC", NULL, "MIC2 Pre-Amplifier" },
+ { "MIC2 Pre-Amplifier", NULL, "Mic2"},
+ { "Mic2", NULL, "VMIC" },
+ { "Right Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
+ { "Left Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
+
+ /* Line-In, FM Routes */
+ { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
+ { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
+ { "Right Mixer", "Right FM Playback Switch", "FM Right" },
+ { "Left Mixer", "Left FM Playback Switch", "FM Left" },
+
+ /* ADC Capturing Sources */
+ {"Capture Source", "Line-In", "Line-In Left"},
+ {"Capture Source", "Line-In", "Line-In Right"},
+ {"Capture Source", "FM", "FM Left"},
+ {"Capture Source", "FM", "FM Right"},
+ {"Capture Source", "Mic1", "MIC1 Pre-Amplifier"},
+ {"Capture Source", "Mic2", "MIC2 Pre-Amplifier"},
+ {"Capture Source", "Mic1,Mic2", "MIC1 Pre-Amplifier"},
+ {"Capture Source", "Mic1,Mic2", "MIC2 Pre-Amplifier"},
+ {"Capture Source", "Mic1+Mic2", "MIC1 Pre-Amplifier"},
+ {"Capture Source", "Mic1+Mic2", "MIC2 Pre-Amplifier"},
+ {"Capture Source", "Output Mixer", "Left Mixer"},
+ {"Capture Source", "Output Mixer", "Right Mixer"},
+ {"Capture Source", "Line-In,Mic1", "Line-In Left"},
+ {"Capture Source", "Line-In,Mic1", "Line-In Right"}, /* depends on LNRDF */
+ {"Capture Source", "Line-In,Mic1", "MIC1 Pre-Amplifier"},
};
static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
@@ -757,8 +917,25 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
return card;
};
+static DECLARE_TLV_DB_RANGE(sun7i_codec_micin_preamp_gain_scale,
+ 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
+ 1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0)
+);
+
static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
SUN4I_COMMON_CODEC_WIDGETS,
+ SOC_SINGLE_TLV("Mic1 Capture Volume",
+ SUN7I_CODEC_AC_MIC_PHONE_CAL,
+ SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1,
+ 7,
+ 0,
+ sun7i_codec_micin_preamp_gain_scale),
+ SOC_SINGLE_TLV("Mic2 Capture Volume",
+ SUN7I_CODEC_AC_MIC_PHONE_CAL,
+ SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
+ 7,
+ 0,
+ sun7i_codec_micin_preamp_gain_scale),
};
static struct snd_soc_codec_driver sun7i_codec_codec = {
Hi,
On Mon, Dec 21, 2015 at 12:33:03PM +0100, Danny Milosavljevic wrote:
> This is the first part:
>
> sun4i-codec: make it possible to use different codec_widgets for A10 and A20.
Please make a meaningful commit log and title explaining what and why
you're doing it.
>
> Signed-off-by: Danny Milosavljevic <[email protected]>
> ---
> b/sound/soc/sunxi/sun4i-codec.c | 45 ++++++++++++++++++++++++++++++----------
> 1 file changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index e6cc6a1..6628e6e 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,9 @@
> /* Other various ADC registers */
> #define SUN4I_CODEC_DAC_TXCNT (0x30)
> #define SUN4I_CODEC_ADC_RXCNT (0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI (0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> +
> +#define SUN7I_CODEC_AC_DAC_CAL (0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
>
> struct sun4i_codec {
> struct device *dev;
> @@ -509,10 +510,13 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
>
> static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> - sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_WIDGETS \
> + SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> + sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
> + SUN4I_COMMON_CODEC_WIDGETS,
> };
>
> static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -627,9 +631,9 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
> { "Mic1", NULL, "VMIC" },
> };
>
> -static struct snd_soc_codec_driver sun4i_codec_codec = {
> - .controls = sun4i_codec_widgets,
> - .num_controls = ARRAY_SIZE(sun4i_codec_widgets),
> +static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
> + .controls = sun4i_codec_widgets_a10,
> + .num_controls = ARRAY_SIZE(sun4i_codec_widgets_a10),
There's no need to change the structure name.
> .dapm_widgets = sun4i_codec_codec_dapm_widgets,
> .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> .dapm_routes = sun4i_codec_codec_dapm_routes,
> @@ -680,7 +684,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
> .reg_bits = 32,
> .reg_stride = 4,
> .val_bits = 32,
> - .max_register = SUN4I_CODEC_AC_MIC_PHONE_CAL,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> };
>
> static const struct of_device_id sun4i_codec_of_match[] = {
> @@ -753,10 +757,24 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
> return card;
> };
>
> +static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
> + SUN4I_COMMON_CODEC_WIDGETS,
> +};
> +
> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> + .controls = sun7i_codec_widgets,
> + .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
> + .dapm_widgets = sun4i_codec_codec_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> + .dapm_routes = sun4i_codec_codec_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> +};
> static int sun4i_codec_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> struct sun4i_codec *scodec;
> + struct snd_soc_codec_driver *codec;
> struct resource *res;
> void __iomem *base;
> int ret;
> @@ -819,7 +837,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> scodec->capture_dma_data.maxburst = 4;
> scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> - ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
> + if (of_device_is_compatible(pdev->dev.of_node,
> + "allwinner,sun7i-a20-codec"))
> + codec = &sun7i_codec_codec;
> + else
> + codec = &sun4i_codec_codec_a10;
A new line here would be nice.
> + ret = snd_soc_register_codec(&pdev->dev, codec,
> &sun4i_codec_dai, 1);
> if (ret) {
> dev_err(&pdev->dev, "Failed to register our codec\n");
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi,
On Mon, Dec 21, 2015 at 12:34:16PM +0100, Danny Milosavljevic wrote:
> This is the second part, actually adding FM, Line and Mic inputs.
Again, having a meaningful and standalone commit log would be nice.
> Signed-off-by: Danny Milosavljevic <[email protected]>
> ---
> b/sound/soc/sunxi/sun4i-codec.c | 185 +++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 181 insertions(+), 4 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 6628e6e..f23d8a9 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -59,9 +59,20 @@
> #define SUN4I_CODEC_DAC_ACTL_DACAENR (31)
> #define SUN4I_CODEC_DAC_ACTL_DACAENL (30)
> #define SUN4I_CODEC_DAC_ACTL_MIXEN (29)
> +#define SUN4I_CODEC_DAC_ACTL_LNG (26)
> +#define SUN4I_CODEC_DAC_ACTL_FMG (23)
> +#define SUN4I_CODEC_DAC_ACTL_MICG (20)
> +#define SUN4I_CODEC_DAC_ACTL_LLNS (19)
> +#define SUN4I_CODEC_DAC_ACTL_RLNS (18)
> +#define SUN4I_CODEC_DAC_ACTL_LFMS (17)
> +#define SUN4I_CODEC_DAC_ACTL_RFMS (16)
> #define SUN4I_CODEC_DAC_ACTL_LDACLMIXS (15)
> #define SUN4I_CODEC_DAC_ACTL_RDACRMIXS (14)
> #define SUN4I_CODEC_DAC_ACTL_LDACRMIXS (13)
> +#define SUN4I_CODEC_DAC_ACTL_MIC1LS (12)
> +#define SUN4I_CODEC_DAC_ACTL_MIC1RS (11)
> +#define SUN4I_CODEC_DAC_ACTL_MIC2LS (10)
> +#define SUN4I_CODEC_DAC_ACTL_MIC2RS (9)
> #define SUN4I_CODEC_DAC_ACTL_DACPAS (8)
> #define SUN4I_CODEC_DAC_ACTL_MIXPAS (7)
> #define SUN4I_CODEC_DAC_ACTL_PA_MUTE (6)
> @@ -87,8 +98,11 @@
> #define SUN4I_CODEC_ADC_ACTL_PREG1EN (29)
> #define SUN4I_CODEC_ADC_ACTL_PREG2EN (28)
> #define SUN4I_CODEC_ADC_ACTL_VMICEN (27)
> -#define SUN4I_CODEC_ADC_ACTL_VADCG (20)
> +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10 (25)
> +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
Why the A10 suffix?
> +#define SUN4I_CODEC_ADC_ACTL_ADCG (20)
> #define SUN4I_CODEC_ADC_ACTL_ADCIS (17)
> +#define SUN4I_CODEC_ADC_ACTL_LNRDF (16)
> #define SUN4I_CODEC_ADC_ACTL_PA_EN (4)
> #define SUN4I_CODEC_ADC_ACTL_DDE (3)
> #define SUN4I_CODEC_ADC_DEBUG (0x2c)
> @@ -100,6 +114,16 @@
> #define SUN7I_CODEC_AC_DAC_CAL (0x38)
> #define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
>
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1 (29)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2 (26)
> +/* note: no idea where the output pins for the following are. */
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTG (5)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTEN (4)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS3 (3)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS2 (2)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS1 (1)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL_PHONEOUTS0 (0)
Are you using that PHONEOUT stuff anywhere?
> +
> struct sun4i_codec {
> struct device *dev;
> struct regmap *regmap;
> @@ -509,19 +533,102 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
> SUN4I_CODEC_DAC_ACTL_PA_MUTE, 1, 0);
>
> static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_linein_loopback_gain_scale,
> + -150,
> + 150,
> + 0);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_fmin_loopback_gain_scale,
> + -450,
> + 150,
> + 0);
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_micin_loopback_gain_scale,
> + -450,
> + 150,
> + 0);
> +static DECLARE_TLV_DB_RANGE(sun4i_codec_micin_preamp_gain_scale_a10,
> + 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
> + 1, 7, TLV_DB_SCALE_ITEM(3500, 300, 0));
> +static DECLARE_TLV_DB_SCALE(sun4i_codec_adc_gain_scale, -450, 150, 0);
> +/* Sources:
> + * A10 User Manual v1.5 20130820
> + * A20 User Manual v1.4 20150510
> + */
There's no need to give your sources here.
> +static const char * const sun4i_codec_capture_source[] = {
> + "Line-In",
> + "FM",
> + "Mic1",
> + "Mic2",
> + "Mic1,Mic2",
> + "Mic1+Mic2",
> + "Output Mixer",
> + "Line-In,Mic1",
> +};
> +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_ADCIS,
> + sun4i_codec_capture_source);
Isn't it possible to expose this as two (shared) muxes with different
names to make it clear what will go to the left ADC and what will go
to the right?
> +static const struct snd_kcontrol_new sun4i_codec_capture_source_controls =
> + SOC_DAPM_ENUM("Route", sun4i_codec_enum_capture_source);
>
> #define SUN4I_COMMON_CODEC_WIDGETS \
> - SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> - sun4i_codec_pa_volume_scale)
> + SOC_SINGLE_TLV("Power Amplifier Playback Volume", SUN4I_CODEC_DAC_ACTL,\
> + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
> + sun4i_codec_pa_volume_scale), \
The power amplifier is only for the playback, so there's no need to
differentiate between playback and capture. The current name was fine.
> + /* Line-In, FM, Mic1, Mic2 */ \
> + SOC_SINGLE_TLV("Line-In Playback Volume", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_LNG, \
> + 1, \
> + 0, \
> + sun4i_codec_linein_loopback_gain_scale), \
> + SOC_SINGLE_TLV("FM Playback Volume", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_FMG, \
> + 3, \
> + 0, \
> + sun4i_codec_fmin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic Playback Volume", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_MICG, \
> + 7, \
> + 0, \
> + sun4i_codec_micin_loopback_gain_scale), \
Those are not volume it's gain, and it should probably be two
different shared controls for mic1 and mic2.
> + /* ADC */ \
> + SOC_SINGLE_TLV("Capture Volume", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_ADCG, \
> + 4, \
> + 0, \
> + sun4i_codec_adc_gain_scale)
This one is the ADC Gain. Please name it that way.
> static const struct snd_kcontrol_new sun4i_codec_widgets_a10[] = {
> SUN4I_COMMON_CODEC_WIDGETS,
> + SOC_SINGLE_TLV("Mic1 Capture Volume",
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG1_A10,
> + 3,
> + 0,
> + sun4i_codec_micin_preamp_gain_scale_a10),
> + SOC_SINGLE_TLV("Mic2 Capture Volume",
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG2_A10,
> + 3,
> + 0,
> + sun4i_codec_micin_preamp_gain_scale_a10),
And those two are not capture volume, it's the pre-amplifier gain.
> };
>
> static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> + SOC_DAPM_SINGLE("Left Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LLNS, 1, 0),
> + SOC_DAPM_SINGLE("Left FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> @@ -529,6 +636,14 @@ static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
> SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
> + SOC_DAPM_SINGLE("Right Line-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
> + SOC_DAPM_SINGLE("Right FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
> @@ -561,6 +676,10 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
> SND_SOC_DAPM_DAC("Right DAC", "Codec Playback", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_DACAENR, 0),
>
> + /* MUX */
> + SND_SOC_DAPM_MUX("Capture Source", SND_SOC_NOPM, 0, 0,
> + &sun4i_codec_capture_source_controls),
> +
Please call it "ADC Source"
> /* Mixers */
> SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
> sun4i_codec_left_mixer_controls,
> @@ -580,6 +699,8 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
> /* Mic Pre-Amplifiers */
> SND_SOC_DAPM_PGA("MIC1 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
> SUN4I_CODEC_ADC_ACTL_PREG1EN, 0, NULL, 0),
> + SND_SOC_DAPM_PGA("MIC2 Pre-Amplifier", SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG2EN, 0, NULL, 0),
>
> /* Power Amplifier */
> SND_SOC_DAPM_MIXER("Power Amplifier", SUN4I_CODEC_ADC_ACTL,
> @@ -590,9 +711,15 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
> &sun4i_codec_pa_mute),
>
> SND_SOC_DAPM_INPUT("Mic1"),
> + SND_SOC_DAPM_INPUT("Mic2"),
>
> SND_SOC_DAPM_OUTPUT("HP Right"),
> SND_SOC_DAPM_OUTPUT("HP Left"),
> +
> + SND_SOC_DAPM_INPUT("Line-In Right"),
> + SND_SOC_DAPM_INPUT("Line-In Left"),
> + SND_SOC_DAPM_INPUT("FM Right"),
> + SND_SOC_DAPM_INPUT("FM Left"),
Having all the inputs and all the outputs grouped together would be
nice.
> };
>
> static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
> @@ -629,6 +756,39 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
> { "Right ADC", NULL, "MIC1 Pre-Amplifier" },
> { "MIC1 Pre-Amplifier", NULL, "Mic1"},
> { "Mic1", NULL, "VMIC" },
> + { "Right Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
> + { "Left Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
> +
The right and left mixer routes should be in the matching sections.
> + /* Mic2 Routes */
> + { "Left ADC", NULL, "MIC2 Pre-Amplifier" },
> + { "Right ADC", NULL, "MIC2 Pre-Amplifier" },
> + { "MIC2 Pre-Amplifier", NULL, "Mic2"},
> + { "Mic2", NULL, "VMIC" },
> + { "Right Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
> + { "Left Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
Ditto.
> +
> + /* Line-In, FM Routes */
> + { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
> + { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
> + { "Right Mixer", "Right FM Playback Switch", "FM Right" },
> + { "Left Mixer", "Left FM Playback Switch", "FM Left" },
Ditto.
> +
> + /* ADC Capturing Sources */
"ADC Input Routes"
> + {"Capture Source", "Line-In", "Line-In Left"},
> + {"Capture Source", "Line-In", "Line-In Right"},
> + {"Capture Source", "FM", "FM Left"},
> + {"Capture Source", "FM", "FM Right"},
> + {"Capture Source", "Mic1", "MIC1 Pre-Amplifier"},
> + {"Capture Source", "Mic2", "MIC2 Pre-Amplifier"},
> + {"Capture Source", "Mic1,Mic2", "MIC1 Pre-Amplifier"},
> + {"Capture Source", "Mic1,Mic2", "MIC2 Pre-Amplifier"},
> + {"Capture Source", "Mic1+Mic2", "MIC1 Pre-Amplifier"},
> + {"Capture Source", "Mic1+Mic2", "MIC2 Pre-Amplifier"},
> + {"Capture Source", "Output Mixer", "Left Mixer"},
> + {"Capture Source", "Output Mixer", "Right Mixer"},
> + {"Capture Source", "Line-In,Mic1", "Line-In Left"},
> + {"Capture Source", "Line-In,Mic1", "Line-In Right"}, /* depends on LNRDF */
> + {"Capture Source", "Line-In,Mic1", "MIC1 Pre-Amplifier"},
> };
You should also have routes from the ADC Input to the Left and Right ADCs
> static struct snd_soc_codec_driver sun4i_codec_codec_a10 = {
> @@ -757,8 +917,25 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
> return card;
> };
>
> +static DECLARE_TLV_DB_RANGE(sun7i_codec_micin_preamp_gain_scale,
> + 0, 0, TLV_DB_SCALE_ITEM(0, 0, 0),
> + 1, 7, TLV_DB_SCALE_ITEM(2400, 300, 0)
> +);
> +
> static const struct snd_kcontrol_new sun7i_codec_widgets[] = {
> SUN4I_COMMON_CODEC_WIDGETS,
> + SOC_SINGLE_TLV("Mic1 Capture Volume",
> + SUN7I_CODEC_AC_MIC_PHONE_CAL,
> + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG1,
> + 7,
> + 0,
> + sun7i_codec_micin_preamp_gain_scale),
> + SOC_SINGLE_TLV("Mic2 Capture Volume",
> + SUN7I_CODEC_AC_MIC_PHONE_CAL,
> + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
> + 7,
> + 0,
> + sun7i_codec_micin_preamp_gain_scale),
"Mic1 Pre-Amplifier Gain", "Mic2 Pre-Amplifier Gain".
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
Hi Maxime,
On Sun, 27 Dec 2015 19:21:57 +0100
Maxime Ripard <[email protected]> wrote:
> On Mon, Dec 21, 2015 at 12:34:16PM +0100, Danny Milosavljevic wrote:
> > This is the second part, actually adding FM, Line and Mic inputs.
>
> Again, having a meaningful and standalone commit log would be nice.
Okay, will elaborate some more in v9.
> > +#define SUN4I_CODEC_ADC_ACTL_PREG1_A10 (25)
> > +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
>
> Why the A10 suffix?
The sun4i*_a10 names are for things that work on A10 but not on A20.
This way whoever touches the driver later can know for which things he has
to consider multiple cases.
Otherwise he will have no indication that he is using a bit index where
there sometimes is no bit (or, worse, the wrong bit).
It's intended to be used like this:
sun4i_foo_a10
foo is sun4i_foo_a10 on A10 (only).
sun4i_foo
foo is sun4i_foo on A10 and also on future chips (like A20).
sun7i_foo
foo is sun7i_foo on A20 and (hopefully) also on future chips.
In this case these defines are for PREG for A10 only.
The A20+ PREG are in the PHONE_CAL register, at other indices.
> Are you using that PHONEOUT stuff anywhere?
Nope.
I was just including all the bits of the register because I usually aim for
completeness by ensuring that there are no gaps in the bit index ranges.
With all the stripping out of the bit range comments that won't be possible
anymore anyway, so I'll just get rid of the PHONEOUT defines in v9, too.
> There's no need to give your sources here.
Ok, will remove.
For the record, there are multiple versions of each of the documents
and if you pick the wrong ones it will look as if there were more
differences between A10 and A20 than there actually are.
> > +static const char * const sun4i_codec_capture_source[] = {
> > + "Line-In",
> > + "FM",
> > + "Mic1",
> > + "Mic2",
> > + "Mic1,Mic2",
> > + "Mic1+Mic2",
> > + "Output Mixer",
> > + "Line-In,Mic1",
> > +};
> > +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> > + SUN4I_CODEC_ADC_ACTL,
> > + SUN4I_CODEC_ADC_ACTL_ADCIS,
> > + sun4i_codec_capture_source);
>
> Isn't it possible to expose this as two (shared) muxes with different
> names to make it clear what will go to the left ADC and what will go
> to the right?
I don't know how to do that. I'll try to find out.
(It would be best if someone who knows how that should act did the alsamixer
testing later too, though)
Can two muxes use the same bit in the hardware without problems?
Or do you mean reuse the same mux instance? (I think _new1 always creates
a new instance from each struct kcontrol_new automatically).
> The power amplifier is only for the playback, so there's no need to
> differentiate between playback and capture. The current name was fine.
"Power Amplifier Volume" shows up as Capture control in alsamixer as well.
It isn't supposed to be a Capture control.
> > + /* Line-In, FM, Mic1, Mic2 */ \
> > + SOC_SINGLE_TLV("Line-In Playback Volume", \
...
> > + SOC_SINGLE_TLV("FM Playback Volume", \
...
> > + SOC_SINGLE_TLV("Mic Playback Volume", \
...
>
> Those are not volume it's gain,
We tried to call the things ..." Gain" before and it was difficult to do,
with some breakage along the way, see below.
Also, Mark said they should be named ..." Volume" (see
<https://www.mail-archive.com/[email protected]/msg15126.html>).
>and it should probably be two different shared controls for mic1 and mic2.
I'll try...
>> "Capture Source"
> This one is the ADC Gain. Please name it that way.
Unfortunately, the strings have meaning to asoc and alsa-lib and you can't go
renaming them like that without breaking things. The names were carefully
chosen to make it actually work properly without having to patch alsa-lib and
parts of ASoC core (which I did before and have since reverted).
In this case, there's a special case in upstream alsa-lib:
alsa-lib-1.0.28:
./src/mixer/simple_none.c: if (strcmp(name, "Capture Source") == 0) {
...
I'm not totally against naming them like you suggested - but know that you
are requiring changes in alsa-lib as well then - or presumably breaking the
user's workflow.
For example the (upstream) "Capture Source" special case in alsa-lib adds
radio-button-like things to the respective elements.
You can switch to one of the inputs by pressing Space while its gain element
is selected.
In the mic case, it's the mic preamplier gain that you press Space on - if it's
indeed shown as a Capture control...
> And those two are not capture volume, it's the pre-amplifier gain.
I know. There has been a previous small discussion in v4 about that and it's
really not that easy.
For example for problems with loopback gain see:
<https://www.mail-archive.com/[email protected]/msg15115.html>
Summary: you have to patch Linux/sound/soc/soc-ops.c and
alsa-lib-1.0.28/src/mixer/simple_none.c to allow controls that don't end
in " Volume" to be sliders... and, in some cases, to be visible at all.
> Having all the inputs and all the outputs grouped together would be
> nice.
Yeah, I'll do that in v9.
> The right and left mixer routes should be in the matching sections.
> Ditto.
> Ditto.
Yeah, I'll do that in v9 and add a comment instead.
> > + /* ADC Capturing Sources */
>
> "ADC Input Routes"
Yeah, I'll do that in v9.
> You should also have routes from the ADC Input to the Left and Right ADCs
Yeah, I'll do that in v9.
> [should be named] "Mic1 Pre-Amplifier Gain", "Mic2 Pre-Amplifier Gain".
It cannot be selected for recording then since it's not a Capture control.
These things are interdependent, even crossing syscall boundary.
Regards,
Danny
On Mon, Dec 28, 2015 at 04:06:49AM +0100, Danny Milosavljevic wrote:
> Maxime Ripard <[email protected]> wrote:
> > > +static SOC_ENUM_SINGLE_DECL(sun4i_codec_enum_capture_source,
> > > + SUN4I_CODEC_ADC_ACTL,
> > > + SUN4I_CODEC_ADC_ACTL_ADCIS,
> > > + sun4i_codec_capture_source);
> > Isn't it possible to expose this as two (shared) muxes with different
> > names to make it clear what will go to the left ADC and what will go
> > to the right?
> I don't know how to do that. I'll try to find out.
> Can two muxes use the same bit in the hardware without problems?
IIRC we fixed things so that if you've got two controls that share the
same register we do the right thing. There are other devices with this
issue but it was years ago that I last looked at them, we do have
handling.
> > Those are not volume it's gain,
> We tried to call the things ..." Gain" before and it was difficult to do,
> with some breakage along the way, see below.
> Also, Mark said they should be named ..." Volume" (see
> <https://www.mail-archive.com/[email protected]/msg15126.html>).
>
> >and it should probably be two different shared controls for mic1 and mic2.
Yes, this is part of the ABI and honestly from an end user point of view
there is no meaningful difference between the two words (there would be
more if it were gain and attenuation since one is amplification and the
other isn't so there are different considerations when applying them to
signals but that's not the case here).