* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-09 22:55 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-09 22:55 UTC (permalink / raw)
To: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Maxime Ripard, Chen-Yu Tsai, alsa-devel,
linux-arm-kernel, linux-kernel
Add inputs to sun4i-codec:
- FM-in Left and Right
- Line-in Left and Right
- Mic1-in
- Mic2-in
Signed-off-by: Danny Milosavljevic <dannym+a@scratchpost.org>
Tested-by: Danny Milosavljevic <dannym+a@scratchpost.org> (ONLY ON A20!)
---
Hi,
this is the sixth version of the patch that adds inputs to sun4i-codec.
Changes compared to v5 are:
- Mic preamplifier special cases for A20 and A10 now are now not icky:
There are two different _widget arrays and the probe() function now selects
the right one to pass to snd_soc_register_codec() unmodified.
- sun7i-specific things (A20-specific things) are now grouped together.
I successfully tested it again on an A20 board using alsamixer, headphones,
a radio and my ears.
Note that because of missing capturing support I tested only the mixing,
for Mic, Line, and FM.
The patches are on top of
<git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
branch "sunxi/for-next".
Regards,
Danny
---
b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
1 file changed, 166 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index bcbf4da..4241999 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -57,9 +57,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)
@@ -84,8 +95,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_PREG1_A10 (25)
+#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
#define SUN4I_CODEC_ADC_ACTL_VADCG (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)
@@ -94,7 +108,6 @@
#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)
struct sun4i_codec {
struct device *dev;
@@ -402,16 +415,77 @@ 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)
+);
+
+#define SUN4I_COMMON_CODEC_WIDGETS \
+ SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
+ sun4i_codec_pa_volume_scale), \
+ /* Line-In, FM-In, Mic1-In, Mic2-In */ \
+ SOC_SINGLE_TLV("Line-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_LNG, \
+ 1, \
+ 0, \
+ sun4i_codec_linein_loopback_gain_scale), \
+ SOC_SINGLE_TLV("FM-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_FMG, \
+ 3, \
+ 0, \
+ sun4i_codec_fmin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_MICG, \
+ 7, \
+ 0, \
+ sun4i_codec_micin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic1-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG1EN, \
+ 1, \
+ 0, \
+ NULL), \
+ SOC_SINGLE_TLV("Mic2-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG2EN, \
+ 1, \
+ 0, \
+ NULL)
static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
- SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
- SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
- sun4i_codec_pa_volume_scale),
+ SUN4I_COMMON_CODEC_WIDGETS,
+ SOC_SINGLE_TLV("Mic1-In Boost Gain",
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG1_A10,
+ 3,
+ 0,
+ sun4i_codec_micin_preamp_gain_scale_a10),
+ SOC_SINGLE_TLV("Mic2-In Boost Gain",
+ 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
@@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
@@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
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-In Right"),
+ SND_SOC_DAPM_INPUT("FM-In Left"),
+ SND_SOC_DAPM_INPUT("Mic1-In"),
+ SND_SOC_DAPM_INPUT("Mic2-In"),
};
+/* {sink, control, source} */
static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
/* Left DAC Routes */
{ "Left DAC", NULL, "DAC" },
@@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
{ "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
{ "HP Right", NULL, "Pre-Amplifier Mute" },
{ "HP Left", NULL, "Pre-Amplifier Mute" },
+
+ /* Line-In, FM-In */
+ { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
+ { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
+ { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
+ { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
+
+ /* Mic1-In */
+ { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+ { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+
+ /* Mic2-In */
+ { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
+ { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
};
-static struct snd_soc_codec_driver sun4i_codec_codec = {
+static const struct snd_soc_codec_driver sun4i_codec_codec = {
.controls = sun4i_codec_widgets,
.num_controls = ARRAY_SIZE(sun4i_codec_widgets),
.dapm_widgets = sun4i_codec_dapm_widgets,
@@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
},
};
-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,
-};
-
static const struct of_device_id sun4i_codec_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-codec" },
{ .compatible = "allwinner,sun7i-a20-codec" },
@@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
return card;
};
+/* sun7i-specific things: */
+/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
+#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)
+
+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-In Boost Gain",
+ 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-In Boost Gain",
+ SUN7I_CODEC_AC_MIC_PHONE_CAL,
+ SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
+ 7,
+ 0,
+ sun7i_codec_micin_preamp_gain_scale),
+};
+
+static const struct snd_soc_codec_driver sun7i_codec_codec = {
+ .controls = sun7i_codec_widgets,
+ .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
+ .dapm_widgets = sun4i_codec_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
+ .dapm_routes = sun4i_codec_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
+};
+static const struct regmap_config sun4i_codec_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
+};
+/* end sun7i-specific things */
+
static int sun4i_codec_probe(struct platform_device *pdev)
{
struct snd_soc_card *card;
@@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *base;
int ret;
+ const struct snd_soc_codec_driver* codec_codec;
scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
if (!scodec)
@@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
scodec->playback_dma_data.maxburst = 4;
scodec->playback_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_codec = &sun7i_codec_codec;
+ else
+ codec_codec = &sun4i_codec_codec;
+ ret = snd_soc_register_codec(&pdev->dev, codec_codec,
&sun4i_codec_dai, 1);
if (ret) {
dev_err(&pdev->dev, "Failed to register our codec\n");
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-09 22:55 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-09 22:55 UTC (permalink / raw)
To: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Maxime Ripard, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Add inputs to sun4i-codec:
- FM-in Left and Right
- Line-in Left and Right
- Mic1-in
- Mic2-in
Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
Tested-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org> (ONLY ON A20!)
---
Hi,
this is the sixth version of the patch that adds inputs to sun4i-codec.
Changes compared to v5 are:
- Mic preamplifier special cases for A20 and A10 now are now not icky:
There are two different _widget arrays and the probe() function now selects
the right one to pass to snd_soc_register_codec() unmodified.
- sun7i-specific things (A20-specific things) are now grouped together.
I successfully tested it again on an A20 board using alsamixer, headphones,
a radio and my ears.
Note that because of missing capturing support I tested only the mixing,
for Mic, Line, and FM.
The patches are on top of
<git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
branch "sunxi/for-next".
Regards,
Danny
---
b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
1 file changed, 166 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index bcbf4da..4241999 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -57,9 +57,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)
@@ -84,8 +95,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_PREG1_A10 (25)
+#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
#define SUN4I_CODEC_ADC_ACTL_VADCG (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)
@@ -94,7 +108,6 @@
#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)
struct sun4i_codec {
struct device *dev;
@@ -402,16 +415,77 @@ 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)
+);
+
+#define SUN4I_COMMON_CODEC_WIDGETS \
+ SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
+ sun4i_codec_pa_volume_scale), \
+ /* Line-In, FM-In, Mic1-In, Mic2-In */ \
+ SOC_SINGLE_TLV("Line-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_LNG, \
+ 1, \
+ 0, \
+ sun4i_codec_linein_loopback_gain_scale), \
+ SOC_SINGLE_TLV("FM-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_FMG, \
+ 3, \
+ 0, \
+ sun4i_codec_fmin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_MICG, \
+ 7, \
+ 0, \
+ sun4i_codec_micin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic1-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG1EN, \
+ 1, \
+ 0, \
+ NULL), \
+ SOC_SINGLE_TLV("Mic2-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG2EN, \
+ 1, \
+ 0, \
+ NULL)
static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
- SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
- SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
- sun4i_codec_pa_volume_scale),
+ SUN4I_COMMON_CODEC_WIDGETS,
+ SOC_SINGLE_TLV("Mic1-In Boost Gain",
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG1_A10,
+ 3,
+ 0,
+ sun4i_codec_micin_preamp_gain_scale_a10),
+ SOC_SINGLE_TLV("Mic2-In Boost Gain",
+ 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
@@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
@@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
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-In Right"),
+ SND_SOC_DAPM_INPUT("FM-In Left"),
+ SND_SOC_DAPM_INPUT("Mic1-In"),
+ SND_SOC_DAPM_INPUT("Mic2-In"),
};
+/* {sink, control, source} */
static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
/* Left DAC Routes */
{ "Left DAC", NULL, "DAC" },
@@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
{ "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
{ "HP Right", NULL, "Pre-Amplifier Mute" },
{ "HP Left", NULL, "Pre-Amplifier Mute" },
+
+ /* Line-In, FM-In */
+ { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
+ { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
+ { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
+ { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
+
+ /* Mic1-In */
+ { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+ { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+
+ /* Mic2-In */
+ { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
+ { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
};
-static struct snd_soc_codec_driver sun4i_codec_codec = {
+static const struct snd_soc_codec_driver sun4i_codec_codec = {
.controls = sun4i_codec_widgets,
.num_controls = ARRAY_SIZE(sun4i_codec_widgets),
.dapm_widgets = sun4i_codec_dapm_widgets,
@@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
},
};
-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,
-};
-
static const struct of_device_id sun4i_codec_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-codec" },
{ .compatible = "allwinner,sun7i-a20-codec" },
@@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
return card;
};
+/* sun7i-specific things: */
+/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
+#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)
+
+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-In Boost Gain",
+ 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-In Boost Gain",
+ SUN7I_CODEC_AC_MIC_PHONE_CAL,
+ SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
+ 7,
+ 0,
+ sun7i_codec_micin_preamp_gain_scale),
+};
+
+static const struct snd_soc_codec_driver sun7i_codec_codec = {
+ .controls = sun7i_codec_widgets,
+ .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
+ .dapm_widgets = sun4i_codec_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
+ .dapm_routes = sun4i_codec_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
+};
+static const struct regmap_config sun4i_codec_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
+};
+/* end sun7i-specific things */
+
static int sun4i_codec_probe(struct platform_device *pdev)
{
struct snd_soc_card *card;
@@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *base;
int ret;
+ const struct snd_soc_codec_driver* codec_codec;
scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
if (!scodec)
@@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
scodec->playback_dma_data.maxburst = 4;
scodec->playback_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_codec = &sun7i_codec_codec;
+ else
+ codec_codec = &sun4i_codec_codec;
+ ret = snd_soc_register_codec(&pdev->dev, codec_codec,
&sun4i_codec_dai, 1);
if (ret) {
dev_err(&pdev->dev, "Failed to register our codec\n");
^ permalink raw reply related [flat|nested] 21+ messages in thread
* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-09 22:55 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-09 22:55 UTC (permalink / raw)
To: linux-arm-kernel
Add inputs to sun4i-codec:
- FM-in Left and Right
- Line-in Left and Right
- Mic1-in
- Mic2-in
Signed-off-by: Danny Milosavljevic <dannym+a@scratchpost.org>
Tested-by: Danny Milosavljevic <dannym+a@scratchpost.org> (ONLY ON A20!)
---
Hi,
this is the sixth version of the patch that adds inputs to sun4i-codec.
Changes compared to v5 are:
- Mic preamplifier special cases for A20 and A10 now are now not icky:
There are two different _widget arrays and the probe() function now selects
the right one to pass to snd_soc_register_codec() unmodified.
- sun7i-specific things (A20-specific things) are now grouped together.
I successfully tested it again on an A20 board using alsamixer, headphones,
a radio and my ears.
Note that because of missing capturing support I tested only the mixing,
for Mic, Line, and FM.
The patches are on top of
<git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
branch "sunxi/for-next".
Regards,
Danny
---
b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
1 file changed, 166 insertions(+), 13 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index bcbf4da..4241999 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -57,9 +57,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)
@@ -84,8 +95,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_PREG1_A10 (25)
+#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
#define SUN4I_CODEC_ADC_ACTL_VADCG (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)
@@ -94,7 +108,6 @@
#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)
struct sun4i_codec {
struct device *dev;
@@ -402,16 +415,77 @@ 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)
+);
+
+#define SUN4I_COMMON_CODEC_WIDGETS \
+ SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
+ sun4i_codec_pa_volume_scale), \
+ /* Line-In, FM-In, Mic1-In, Mic2-In */ \
+ SOC_SINGLE_TLV("Line-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_LNG, \
+ 1, \
+ 0, \
+ sun4i_codec_linein_loopback_gain_scale), \
+ SOC_SINGLE_TLV("FM-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_FMG, \
+ 3, \
+ 0, \
+ sun4i_codec_fmin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic-In Playback Gain", \
+ SUN4I_CODEC_DAC_ACTL, \
+ SUN4I_CODEC_DAC_ACTL_MICG, \
+ 7, \
+ 0, \
+ sun4i_codec_micin_loopback_gain_scale), \
+ SOC_SINGLE_TLV("Mic1-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG1EN, \
+ 1, \
+ 0, \
+ NULL), \
+ SOC_SINGLE_TLV("Mic2-In Boost Switch", \
+ SUN4I_CODEC_ADC_ACTL, \
+ SUN4I_CODEC_ADC_ACTL_PREG2EN, \
+ 1, \
+ 0, \
+ NULL)
static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
- SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
- SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
- sun4i_codec_pa_volume_scale),
+ SUN4I_COMMON_CODEC_WIDGETS,
+ SOC_SINGLE_TLV("Mic1-In Boost Gain",
+ SUN4I_CODEC_ADC_ACTL,
+ SUN4I_CODEC_ADC_ACTL_PREG1_A10,
+ 3,
+ 0,
+ sun4i_codec_micin_preamp_gain_scale_a10),
+ SOC_SINGLE_TLV("Mic2-In Boost Gain",
+ 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
@@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
+ SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
+ SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
};
static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
@@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
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-In Right"),
+ SND_SOC_DAPM_INPUT("FM-In Left"),
+ SND_SOC_DAPM_INPUT("Mic1-In"),
+ SND_SOC_DAPM_INPUT("Mic2-In"),
};
+/* {sink, control, source} */
static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
/* Left DAC Routes */
{ "Left DAC", NULL, "DAC" },
@@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
{ "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
{ "HP Right", NULL, "Pre-Amplifier Mute" },
{ "HP Left", NULL, "Pre-Amplifier Mute" },
+
+ /* Line-In, FM-In */
+ { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
+ { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
+ { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
+ { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
+
+ /* Mic1-In */
+ { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+ { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
+
+ /* Mic2-In */
+ { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
+ { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
};
-static struct snd_soc_codec_driver sun4i_codec_codec = {
+static const struct snd_soc_codec_driver sun4i_codec_codec = {
.controls = sun4i_codec_widgets,
.num_controls = ARRAY_SIZE(sun4i_codec_widgets),
.dapm_widgets = sun4i_codec_dapm_widgets,
@@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
},
};
-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,
-};
-
static const struct of_device_id sun4i_codec_of_match[] = {
{ .compatible = "allwinner,sun4i-a10-codec" },
{ .compatible = "allwinner,sun7i-a20-codec" },
@@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
return card;
};
+/* sun7i-specific things: */
+/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
+#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)
+
+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-In Boost Gain",
+ 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-In Boost Gain",
+ SUN7I_CODEC_AC_MIC_PHONE_CAL,
+ SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
+ 7,
+ 0,
+ sun7i_codec_micin_preamp_gain_scale),
+};
+
+static const struct snd_soc_codec_driver sun7i_codec_codec = {
+ .controls = sun7i_codec_widgets,
+ .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
+ .dapm_widgets = sun4i_codec_dapm_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
+ .dapm_routes = sun4i_codec_dapm_routes,
+ .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
+};
+static const struct regmap_config sun4i_codec_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
+};
+/* end sun7i-specific things */
+
static int sun4i_codec_probe(struct platform_device *pdev)
{
struct snd_soc_card *card;
@@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
struct resource *res;
void __iomem *base;
int ret;
+ const struct snd_soc_codec_driver* codec_codec;
scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
if (!scodec)
@@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
scodec->playback_dma_data.maxburst = 4;
scodec->playback_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_codec = &sun7i_codec_codec;
+ else
+ codec_codec = &sun4i_codec_codec;
+ ret = snd_soc_register_codec(&pdev->dev, codec_codec,
&sun4i_codec_dai, 1);
if (ret) {
dev_err(&pdev->dev, "Failed to register our codec\n");
^ permalink raw reply related [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-09 22:55 ` Danny Milosavljevic
(?)
@ 2015-12-13 20:58 ` Maxime Ripard
-1 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-13 20:58 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 13469 bytes --]
Hi,
On Wed, Dec 09, 2015 at 11:55:30PM +0100, Danny Milosavljevic wrote:
> Add inputs to sun4i-codec:
> - FM-in Left and Right
> - Line-in Left and Right
> - Mic1-in
> - Mic2-in
>
> Signed-off-by: Danny Milosavljevic <dannym+a@scratchpost.org>
> Tested-by: Danny Milosavljevic <dannym+a@scratchpost.org> (ONLY ON A20!)
We'd expect you to have tested it. You can drop this tag.
However, it would indeed be nice to have someone with an A10 testing
it. Chen-Yu?
> ---
> Hi,
>
> this is the sixth version of the patch that adds inputs to sun4i-codec.
>
> Changes compared to v5 are:
> - Mic preamplifier special cases for A20 and A10 now are now not icky:
> There are two different _widget arrays and the probe() function now selects
> the right one to pass to snd_soc_register_codec() unmodified.
> - sun7i-specific things (A20-specific things) are now grouped together.
>
> I successfully tested it again on an A20 board using alsamixer, headphones,
> a radio and my ears.
> Note that because of missing capturing support I tested only the mixing,
> for Mic, Line, and FM.
>
> The patches are on top of
> <git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
> branch "sunxi/for-next".
This is not the branch you should be basing your patch on. This is an
ASoC patch, base it on the ASoC tree.
> Regards,
> Danny
> ---
> b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 166 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index bcbf4da..4241999 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -57,9 +57,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)
> @@ -84,8 +95,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_PREG1_A10 (25)
> +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
> #define SUN4I_CODEC_ADC_ACTL_VADCG (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)
> @@ -94,7 +108,6 @@
> #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)
>
> struct sun4i_codec {
> struct device *dev;
> @@ -402,16 +415,77 @@ 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)
> +);
> +
> +#define SUN4I_COMMON_CODEC_WIDGETS \
> + SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
> + sun4i_codec_pa_volume_scale), \
> + /* Line-In, FM-In, Mic1-In, Mic2-In */ \
> + SOC_SINGLE_TLV("Line-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_LNG, \
> + 1, \
> + 0, \
> + sun4i_codec_linein_loopback_gain_scale), \
> + SOC_SINGLE_TLV("FM-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_FMG, \
> + 3, \
> + 0, \
> + sun4i_codec_fmin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_MICG, \
> + 7, \
> + 0, \
> + sun4i_codec_micin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic1-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG1EN, \
> + 1, \
> + 0, \
> + NULL), \
> + SOC_SINGLE_TLV("Mic2-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG2EN, \
> + 1, \
> + 0, \
> + NULL)
You have a bunch of checkpatch errors and warnings, make sure you fix
them.
> static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
> - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> - sun4i_codec_pa_volume_scale),
> + SUN4I_COMMON_CODEC_WIDGETS,
> + SOC_SINGLE_TLV("Mic1-In Boost Gain",
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG1_A10,
> + 3,
> + 0,
> + sun4i_codec_micin_preamp_gain_scale_a10),
> + SOC_SINGLE_TLV("Mic2-In Boost Gain",
> + 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> @@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
> @@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
>
> 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-In Right"),
> + SND_SOC_DAPM_INPUT("FM-In Left"),
> + SND_SOC_DAPM_INPUT("Mic1-In"),
> + SND_SOC_DAPM_INPUT("Mic2-In"),
> };
>
> +/* {sink, control, source} */
> static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> /* Left DAC Routes */
> { "Left DAC", NULL, "DAC" },
> @@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> { "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
> { "HP Right", NULL, "Pre-Amplifier Mute" },
> { "HP Left", NULL, "Pre-Amplifier Mute" },
> +
> + /* Line-In, FM-In */
> + { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
> + { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
> + { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
> + { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
> +
> + /* Mic1-In */
> + { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> + { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> +
> + /* Mic2-In */
> + { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> + { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> };
You're doing several things in this patch:
- Reworking things to be able to have different widgets and routes
between the compatibles
- Add new controls.
Please make two separate patches.
> -static struct snd_soc_codec_driver sun4i_codec_codec = {
> +static const struct snd_soc_codec_driver sun4i_codec_codec = {
And this belongs in a separate patch too.
> .controls = sun4i_codec_widgets,
> .num_controls = ARRAY_SIZE(sun4i_codec_widgets),
> .dapm_widgets = sun4i_codec_dapm_widgets,
> @@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
> },
> };
>
> -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,
> -};
> -
Why is this moved?
> static const struct of_device_id sun4i_codec_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-codec" },
> { .compatible = "allwinner,sun7i-a20-codec" },
> @@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
> return card;
> };
>
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#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)
> +
> +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-In Boost Gain",
> + 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-In Boost Gain",
> + SUN7I_CODEC_AC_MIC_PHONE_CAL,
> + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
> + 7,
> + 0,
> + sun7i_codec_micin_preamp_gain_scale),
> +};
> +
> +static const struct snd_soc_codec_driver sun7i_codec_codec = {
> + .controls = sun7i_codec_widgets,
> + .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
> + .dapm_widgets = sun4i_codec_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
> + .dapm_routes = sun4i_codec_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
> +};
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
> +
> static int sun4i_codec_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> struct resource *res;
> void __iomem *base;
> int ret;
> + const struct snd_soc_codec_driver* codec_codec;
I guess a single codec is enough :)
>
> scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
> if (!scodec)
> @@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> scodec->playback_dma_data.maxburst = 4;
> scodec->playback_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_codec = &sun7i_codec_codec;
> + else
> + codec_codec = &sun4i_codec_codec;
> + ret = snd_soc_register_codec(&pdev->dev, codec_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
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-13 20:58 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-13 20:58 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 13165 bytes --]
Hi,
On Wed, Dec 09, 2015 at 11:55:30PM +0100, Danny Milosavljevic wrote:
> Add inputs to sun4i-codec:
> - FM-in Left and Right
> - Line-in Left and Right
> - Mic1-in
> - Mic2-in
>
> Signed-off-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org>
> Tested-by: Danny Milosavljevic <dannym+a-bxPqe3T81XXwRsdMLXbzog@public.gmane.org> (ONLY ON A20!)
We'd expect you to have tested it. You can drop this tag.
However, it would indeed be nice to have someone with an A10 testing
it. Chen-Yu?
> ---
> Hi,
>
> this is the sixth version of the patch that adds inputs to sun4i-codec.
>
> Changes compared to v5 are:
> - Mic preamplifier special cases for A20 and A10 now are now not icky:
> There are two different _widget arrays and the probe() function now selects
> the right one to pass to snd_soc_register_codec() unmodified.
> - sun7i-specific things (A20-specific things) are now grouped together.
>
> I successfully tested it again on an A20 board using alsamixer, headphones,
> a radio and my ears.
> Note that because of missing capturing support I tested only the mixing,
> for Mic, Line, and FM.
>
> The patches are on top of
> <git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
> branch "sunxi/for-next".
This is not the branch you should be basing your patch on. This is an
ASoC patch, base it on the ASoC tree.
> Regards,
> Danny
> ---
> b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 166 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index bcbf4da..4241999 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -57,9 +57,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)
> @@ -84,8 +95,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_PREG1_A10 (25)
> +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
> #define SUN4I_CODEC_ADC_ACTL_VADCG (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)
> @@ -94,7 +108,6 @@
> #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)
>
> struct sun4i_codec {
> struct device *dev;
> @@ -402,16 +415,77 @@ 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)
> +);
> +
> +#define SUN4I_COMMON_CODEC_WIDGETS \
> + SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
> + sun4i_codec_pa_volume_scale), \
> + /* Line-In, FM-In, Mic1-In, Mic2-In */ \
> + SOC_SINGLE_TLV("Line-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_LNG, \
> + 1, \
> + 0, \
> + sun4i_codec_linein_loopback_gain_scale), \
> + SOC_SINGLE_TLV("FM-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_FMG, \
> + 3, \
> + 0, \
> + sun4i_codec_fmin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_MICG, \
> + 7, \
> + 0, \
> + sun4i_codec_micin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic1-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG1EN, \
> + 1, \
> + 0, \
> + NULL), \
> + SOC_SINGLE_TLV("Mic2-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG2EN, \
> + 1, \
> + 0, \
> + NULL)
You have a bunch of checkpatch errors and warnings, make sure you fix
them.
> static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
> - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> - sun4i_codec_pa_volume_scale),
> + SUN4I_COMMON_CODEC_WIDGETS,
> + SOC_SINGLE_TLV("Mic1-In Boost Gain",
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG1_A10,
> + 3,
> + 0,
> + sun4i_codec_micin_preamp_gain_scale_a10),
> + SOC_SINGLE_TLV("Mic2-In Boost Gain",
> + 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> @@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
> @@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
>
> 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-In Right"),
> + SND_SOC_DAPM_INPUT("FM-In Left"),
> + SND_SOC_DAPM_INPUT("Mic1-In"),
> + SND_SOC_DAPM_INPUT("Mic2-In"),
> };
>
> +/* {sink, control, source} */
> static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> /* Left DAC Routes */
> { "Left DAC", NULL, "DAC" },
> @@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> { "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
> { "HP Right", NULL, "Pre-Amplifier Mute" },
> { "HP Left", NULL, "Pre-Amplifier Mute" },
> +
> + /* Line-In, FM-In */
> + { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
> + { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
> + { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
> + { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
> +
> + /* Mic1-In */
> + { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> + { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> +
> + /* Mic2-In */
> + { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> + { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> };
You're doing several things in this patch:
- Reworking things to be able to have different widgets and routes
between the compatibles
- Add new controls.
Please make two separate patches.
> -static struct snd_soc_codec_driver sun4i_codec_codec = {
> +static const struct snd_soc_codec_driver sun4i_codec_codec = {
And this belongs in a separate patch too.
> .controls = sun4i_codec_widgets,
> .num_controls = ARRAY_SIZE(sun4i_codec_widgets),
> .dapm_widgets = sun4i_codec_dapm_widgets,
> @@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
> },
> };
>
> -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,
> -};
> -
Why is this moved?
> static const struct of_device_id sun4i_codec_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-codec" },
> { .compatible = "allwinner,sun7i-a20-codec" },
> @@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
> return card;
> };
>
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#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)
> +
> +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-In Boost Gain",
> + 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-In Boost Gain",
> + SUN7I_CODEC_AC_MIC_PHONE_CAL,
> + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
> + 7,
> + 0,
> + sun7i_codec_micin_preamp_gain_scale),
> +};
> +
> +static const struct snd_soc_codec_driver sun7i_codec_codec = {
> + .controls = sun7i_codec_widgets,
> + .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
> + .dapm_widgets = sun4i_codec_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
> + .dapm_routes = sun4i_codec_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
> +};
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
> +
> static int sun4i_codec_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> struct resource *res;
> void __iomem *base;
> int ret;
> + const struct snd_soc_codec_driver* codec_codec;
I guess a single codec is enough :)
>
> scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
> if (!scodec)
> @@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> scodec->playback_dma_data.maxburst = 4;
> scodec->playback_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_codec = &sun7i_codec_codec;
> + else
> + codec_codec = &sun4i_codec_codec;
> + ret = snd_soc_register_codec(&pdev->dev, codec_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
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-13 20:58 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-13 20:58 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Wed, Dec 09, 2015 at 11:55:30PM +0100, Danny Milosavljevic wrote:
> Add inputs to sun4i-codec:
> - FM-in Left and Right
> - Line-in Left and Right
> - Mic1-in
> - Mic2-in
>
> Signed-off-by: Danny Milosavljevic <dannym+a@scratchpost.org>
> Tested-by: Danny Milosavljevic <dannym+a@scratchpost.org> (ONLY ON A20!)
We'd expect you to have tested it. You can drop this tag.
However, it would indeed be nice to have someone with an A10 testing
it. Chen-Yu?
> ---
> Hi,
>
> this is the sixth version of the patch that adds inputs to sun4i-codec.
>
> Changes compared to v5 are:
> - Mic preamplifier special cases for A20 and A10 now are now not icky:
> There are two different _widget arrays and the probe() function now selects
> the right one to pass to snd_soc_register_codec() unmodified.
> - sun7i-specific things (A20-specific things) are now grouped together.
>
> I successfully tested it again on an A20 board using alsamixer, headphones,
> a radio and my ears.
> Note that because of missing capturing support I tested only the mixing,
> for Mic, Line, and FM.
>
> The patches are on top of
> <git://git.kernel.org/pub/scm/linux/kernel/git/mripard/linux.git>,
> branch "sunxi/for-next".
This is not the branch you should be basing your patch on. This is an
ASoC patch, base it on the ASoC tree.
> Regards,
> Danny
> ---
> b/sound/soc/sunxi/sun4i-codec.c | 179 +++++++++++++++++++++++++++++++++++++---
> 1 file changed, 166 insertions(+), 13 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index bcbf4da..4241999 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -57,9 +57,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)
> @@ -84,8 +95,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_PREG1_A10 (25)
> +#define SUN4I_CODEC_ADC_ACTL_PREG2_A10 (23)
> #define SUN4I_CODEC_ADC_ACTL_VADCG (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)
> @@ -94,7 +108,6 @@
> #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)
>
> struct sun4i_codec {
> struct device *dev;
> @@ -402,16 +415,77 @@ 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)
> +);
> +
> +#define SUN4I_COMMON_CODEC_WIDGETS \
> + SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0, \
> + sun4i_codec_pa_volume_scale), \
> + /* Line-In, FM-In, Mic1-In, Mic2-In */ \
> + SOC_SINGLE_TLV("Line-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_LNG, \
> + 1, \
> + 0, \
> + sun4i_codec_linein_loopback_gain_scale), \
> + SOC_SINGLE_TLV("FM-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_FMG, \
> + 3, \
> + 0, \
> + sun4i_codec_fmin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic-In Playback Gain", \
> + SUN4I_CODEC_DAC_ACTL, \
> + SUN4I_CODEC_DAC_ACTL_MICG, \
> + 7, \
> + 0, \
> + sun4i_codec_micin_loopback_gain_scale), \
> + SOC_SINGLE_TLV("Mic1-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG1EN, \
> + 1, \
> + 0, \
> + NULL), \
> + SOC_SINGLE_TLV("Mic2-In Boost Switch", \
> + SUN4I_CODEC_ADC_ACTL, \
> + SUN4I_CODEC_ADC_ACTL_PREG2EN, \
> + 1, \
> + 0, \
> + NULL)
You have a bunch of checkpatch errors and warnings, make sure you fix
them.
> static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> - SOC_SINGLE_TLV("PA Volume", SUN4I_CODEC_DAC_ACTL,
> - SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> - sun4i_codec_pa_volume_scale),
> + SUN4I_COMMON_CODEC_WIDGETS,
> + SOC_SINGLE_TLV("Mic1-In Boost Gain",
> + SUN4I_CODEC_ADC_ACTL,
> + SUN4I_CODEC_ADC_ACTL_PREG1_A10,
> + 3,
> + 0,
> + sun4i_codec_micin_preamp_gain_scale_a10),
> + SOC_SINGLE_TLV("Mic2-In Boost Gain",
> + 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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1LS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2LS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> @@ -419,6 +493,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-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> + SOC_DAPM_SINGLE("Mic1-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> + SOC_DAPM_SINGLE("Mic2-In Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
> };
>
> static const struct snd_kcontrol_new sun4i_codec_pa_mixer_controls[] = {
> @@ -462,8 +544,16 @@ static const struct snd_soc_dapm_widget sun4i_codec_dapm_widgets[] = {
>
> 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-In Right"),
> + SND_SOC_DAPM_INPUT("FM-In Left"),
> + SND_SOC_DAPM_INPUT("Mic1-In"),
> + SND_SOC_DAPM_INPUT("Mic2-In"),
> };
>
> +/* {sink, control, source} */
> static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> /* Left DAC Routes */
> { "Left DAC", NULL, "DAC" },
> @@ -490,9 +580,23 @@ static const struct snd_soc_dapm_route sun4i_codec_dapm_routes[] = {
> { "Pre-Amplifier Mute", "Switch", "Pre-Amplifier" },
> { "HP Right", NULL, "Pre-Amplifier Mute" },
> { "HP Left", NULL, "Pre-Amplifier Mute" },
> +
> + /* Line-In, FM-In */
> + { "Right Mixer", "Right Line-In Playback Switch", "Line-In Right" },
> + { "Left Mixer", "Left Line-In Playback Switch", "Line-In Left" },
> + { "Right Mixer", "Right FM-In Playback Switch", "FM-In Right" },
> + { "Left Mixer", "Left FM-In Playback Switch", "FM-In Left" },
> +
> + /* Mic1-In */
> + { "Right Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> + { "Left Mixer", "Mic1-In Playback Switch", "Mic1-In" },
> +
> + /* Mic2-In */
> + { "Right Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> + { "Left Mixer", "Mic2-In Playback Switch", "Mic2-In" },
> };
You're doing several things in this patch:
- Reworking things to be able to have different widgets and routes
between the compatibles
- Add new controls.
Please make two separate patches.
> -static struct snd_soc_codec_driver sun4i_codec_codec = {
> +static const struct snd_soc_codec_driver sun4i_codec_codec = {
And this belongs in a separate patch too.
> .controls = sun4i_codec_widgets,
> .num_controls = ARRAY_SIZE(sun4i_codec_widgets),
> .dapm_widgets = sun4i_codec_dapm_widgets,
> @@ -533,13 +637,6 @@ static struct snd_soc_dai_driver dummy_cpu_dai = {
> },
> };
>
> -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,
> -};
> -
Why is this moved?
> static const struct of_device_id sun4i_codec_of_match[] = {
> { .compatible = "allwinner,sun4i-a10-codec" },
> { .compatible = "allwinner,sun7i-a20-codec" },
> @@ -586,6 +683,56 @@ static struct snd_soc_card *sun4i_codec_create_card(struct device *dev)
> return card;
> };
>
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#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)
> +
> +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-In Boost Gain",
> + 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-In Boost Gain",
> + SUN7I_CODEC_AC_MIC_PHONE_CAL,
> + SUN7I_CODEC_AC_MIC_PHONE_CAL_PREG2,
> + 7,
> + 0,
> + sun7i_codec_micin_preamp_gain_scale),
> +};
> +
> +static const struct snd_soc_codec_driver sun7i_codec_codec = {
> + .controls = sun7i_codec_widgets,
> + .num_controls = ARRAY_SIZE(sun7i_codec_widgets),
> + .dapm_widgets = sun4i_codec_dapm_widgets,
> + .num_dapm_widgets = ARRAY_SIZE(sun4i_codec_dapm_widgets),
> + .dapm_routes = sun4i_codec_dapm_routes,
> + .num_dapm_routes = ARRAY_SIZE(sun4i_codec_dapm_routes),
> +};
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
> +
> static int sun4i_codec_probe(struct platform_device *pdev)
> {
> struct snd_soc_card *card;
> @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> struct resource *res;
> void __iomem *base;
> int ret;
> + const struct snd_soc_codec_driver* codec_codec;
I guess a single codec is enough :)
>
> scodec = devm_kzalloc(&pdev->dev, sizeof(*scodec), GFP_KERNEL);
> if (!scodec)
> @@ -638,7 +786,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> scodec->playback_dma_data.maxburst = 4;
> scodec->playback_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_codec = &sun7i_codec_codec;
> + else
> + codec_codec = &sun4i_codec_codec;
> + ret = snd_soc_register_codec(&pdev->dev, codec_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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151213/9ae19570/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-13 20:58 ` Maxime Ripard
(?)
@ 2015-12-15 1:52 ` Danny Milosavljevic
-1 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-15 1:52 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
Hi Maxime,
On Sun, 13 Dec 2015 21:58:39 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> This is not the branch you should be basing your patch on. This is an
> ASoC patch, base it on the ASoC tree.
Okay, will do. To the branch "sunxi-next" in
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
[...]
> > -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,
> > -};
> > -
>
> Why is this moved?
Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Note: I also renamed it and moved the #define in the course of grouping
together sun7i-specific things:
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
[...]
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
I thought about also renaming sun4i_codec_regmap_config but decided against it
since it's fine to use it on A10 and I think it's best if the name reflects
the minimum required hardware.
On the other hand, once I moved the define, sun4i-codec won't compile if
sun4i_codec_regmap_config is left at the top. So I had to move it, too.
It will be clearer once I post a patch doing just the preparation of the
A10/A20 split.
I just checked A10 vs A20 some more:
There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
"AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> > static int sun4i_codec_probe(struct platform_device *pdev)
> > {
> > struct snd_soc_card *card;
> > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > struct resource *res;
> > void __iomem *base;
> > int ret;
> > + const struct snd_soc_codec_driver* codec_codec;
>
> I guess a single codec is enough :)
Modeled after the name of the original variable, see below :)
But OK, I'll rename it to "codec".
Note: the newest original ASoC sun4i-codec has a variable
"struct sun4i_codec *scodec;"
as well in the same function (which is a different thing).
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "allwinner,sun7i-a20-codec"))
> > + codec_codec = &sun7i_codec_codec;
> > + else
> > + codec_codec = &sun4i_codec_codec;
> > + ret = snd_soc_register_codec(&pdev->dev, codec_codec,
> > &sun4i_codec_dai, 1);
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-15 1:52 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-15 1:52 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi Maxime,
On Sun, 13 Dec 2015 21:58:39 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> This is not the branch you should be basing your patch on. This is an
> ASoC patch, base it on the ASoC tree.
Okay, will do. To the branch "sunxi-next" in
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
[...]
> > -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,
> > -};
> > -
>
> Why is this moved?
Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Note: I also renamed it and moved the #define in the course of grouping
together sun7i-specific things:
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
[...]
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
I thought about also renaming sun4i_codec_regmap_config but decided against it
since it's fine to use it on A10 and I think it's best if the name reflects
the minimum required hardware.
On the other hand, once I moved the define, sun4i-codec won't compile if
sun4i_codec_regmap_config is left at the top. So I had to move it, too.
It will be clearer once I post a patch doing just the preparation of the
A10/A20 split.
I just checked A10 vs A20 some more:
There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
"AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> > static int sun4i_codec_probe(struct platform_device *pdev)
> > {
> > struct snd_soc_card *card;
> > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > struct resource *res;
> > void __iomem *base;
> > int ret;
> > + const struct snd_soc_codec_driver* codec_codec;
>
> I guess a single codec is enough :)
Modeled after the name of the original variable, see below :)
But OK, I'll rename it to "codec".
Note: the newest original ASoC sun4i-codec has a variable
"struct sun4i_codec *scodec;"
as well in the same function (which is a different thing).
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "allwinner,sun7i-a20-codec"))
> > + codec_codec = &sun7i_codec_codec;
> > + else
> > + codec_codec = &sun4i_codec_codec;
> > + ret = snd_soc_register_codec(&pdev->dev, codec_codec,
> > &sun4i_codec_dai, 1);
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-15 1:52 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-15 1:52 UTC (permalink / raw)
To: linux-arm-kernel
Hi Maxime,
On Sun, 13 Dec 2015 21:58:39 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> This is not the branch you should be basing your patch on. This is an
> ASoC patch, base it on the ASoC tree.
Okay, will do. To the branch "sunxi-next" in
<git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
[...]
> > -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,
> > -};
> > -
>
> Why is this moved?
Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Note: I also renamed it and moved the #define in the course of grouping
together sun7i-specific things:
> +/* sun7i-specific things: */
> +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
[...]
> +static const struct regmap_config sun4i_codec_regmap_config = {
> + .reg_bits = 32,
> + .reg_stride = 4,
> + .val_bits = 32,
> + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> +};
> +/* end sun7i-specific things */
I thought about also renaming sun4i_codec_regmap_config but decided against it
since it's fine to use it on A10 and I think it's best if the name reflects
the minimum required hardware.
On the other hand, once I moved the define, sun4i-codec won't compile if
sun4i_codec_regmap_config is left at the top. So I had to move it, too.
It will be clearer once I post a patch doing just the preparation of the
A10/A20 split.
I just checked A10 vs A20 some more:
There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
"AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> > static int sun4i_codec_probe(struct platform_device *pdev)
> > {
> > struct snd_soc_card *card;
> > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > struct resource *res;
> > void __iomem *base;
> > int ret;
> > + const struct snd_soc_codec_driver* codec_codec;
>
> I guess a single codec is enough :)
Modeled after the name of the original variable, see below :)
But OK, I'll rename it to "codec".
Note: the newest original ASoC sun4i-codec has a variable
"struct sun4i_codec *scodec;"
as well in the same function (which is a different thing).
> > + if (of_device_is_compatible(pdev->dev.of_node,
> > + "allwinner,sun7i-a20-codec"))
> > + codec_codec = &sun7i_codec_codec;
> > + else
> > + codec_codec = &sun4i_codec_codec;
> > + ret = snd_soc_register_codec(&pdev->dev, codec_codec,
> > &sun4i_codec_dai, 1);
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-15 1:52 ` Danny Milosavljevic
(?)
@ 2015-12-16 10:47 ` Maxime Ripard
-1 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-16 10:47 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 3271 bytes --]
Hi,
On Tue, Dec 15, 2015 at 02:52:08AM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Sun, 13 Dec 2015 21:58:39 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > This is not the branch you should be basing your patch on. This is an
> > ASoC patch, base it on the ASoC tree.
>
> Okay, will do. To the branch "sunxi-next" in
> <git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
>
> [...]
> > > -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,
> > > -};
> > > -
> >
> > Why is this moved?
>
> Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Yet, you're using it in both cases (A10 vs A20).
> Note: I also renamed it and moved the #define in the course of grouping
> together sun7i-specific things:
>
> > +/* sun7i-specific things: */
> > +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> [...]
> > +static const struct regmap_config sun4i_codec_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> > +};
> > +/* end sun7i-specific things */
>
> I thought about also renaming sun4i_codec_regmap_config but decided against it
> since it's fine to use it on A10 and I think it's best if the name reflects
> the minimum required hardware.
>
> On the other hand, once I moved the define, sun4i-codec won't compile if
> sun4i_codec_regmap_config is left at the top. So I had to move it, too.
You can also have the defines on top, and everything just works :)
> It will be clearer once I post a patch doing just the preparation of the
> A10/A20 split.
>
> I just checked A10 vs A20 some more:
> There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
> It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
> "AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
> Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
You can rename it if you want, but it's not like it's of the highest
importance :)
>
> > > static int sun4i_codec_probe(struct platform_device *pdev)
> > > {
> > > struct snd_soc_card *card;
> > > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > > struct resource *res;
> > > void __iomem *base;
> > > int ret;
> > > + const struct snd_soc_codec_driver* codec_codec;
> >
> > I guess a single codec is enough :)
>
> Modeled after the name of the original variable, see below :)
>
> But OK, I'll rename it to "codec".
>
> Note: the newest original ASoC sun4i-codec has a variable
> "struct sun4i_codec *scodec;"
> as well in the same function (which is a different thing).
I don't know what you're refering to with "newest" and "original".
But two different variables with two different names doesn't seem so
bad, does it?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-16 10:47 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-16 10:47 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 3204 bytes --]
Hi,
On Tue, Dec 15, 2015 at 02:52:08AM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Sun, 13 Dec 2015 21:58:39 +0100
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> > This is not the branch you should be basing your patch on. This is an
> > ASoC patch, base it on the ASoC tree.
>
> Okay, will do. To the branch "sunxi-next" in
> <git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
>
> [...]
> > > -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,
> > > -};
> > > -
> >
> > Why is this moved?
>
> Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Yet, you're using it in both cases (A10 vs A20).
> Note: I also renamed it and moved the #define in the course of grouping
> together sun7i-specific things:
>
> > +/* sun7i-specific things: */
> > +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> [...]
> > +static const struct regmap_config sun4i_codec_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> > +};
> > +/* end sun7i-specific things */
>
> I thought about also renaming sun4i_codec_regmap_config but decided against it
> since it's fine to use it on A10 and I think it's best if the name reflects
> the minimum required hardware.
>
> On the other hand, once I moved the define, sun4i-codec won't compile if
> sun4i_codec_regmap_config is left at the top. So I had to move it, too.
You can also have the defines on top, and everything just works :)
> It will be clearer once I post a patch doing just the preparation of the
> A10/A20 split.
>
> I just checked A10 vs A20 some more:
> There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
> It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
> "AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
> Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
You can rename it if you want, but it's not like it's of the highest
importance :)
>
> > > static int sun4i_codec_probe(struct platform_device *pdev)
> > > {
> > > struct snd_soc_card *card;
> > > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > > struct resource *res;
> > > void __iomem *base;
> > > int ret;
> > > + const struct snd_soc_codec_driver* codec_codec;
> >
> > I guess a single codec is enough :)
>
> Modeled after the name of the original variable, see below :)
>
> But OK, I'll rename it to "codec".
>
> Note: the newest original ASoC sun4i-codec has a variable
> "struct sun4i_codec *scodec;"
> as well in the same function (which is a different thing).
I don't know what you're refering to with "newest" and "original".
But two different variables with two different names doesn't seem so
bad, does it?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-16 10:47 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-16 10:47 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Dec 15, 2015 at 02:52:08AM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Sun, 13 Dec 2015 21:58:39 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > This is not the branch you should be basing your patch on. This is an
> > ASoC patch, base it on the ASoC tree.
>
> Okay, will do. To the branch "sunxi-next" in
> <git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git>, right?
>
> [...]
> > > -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,
> > > -};
> > > -
> >
> > Why is this moved?
>
> Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
Yet, you're using it in both cases (A10 vs A20).
> Note: I also renamed it and moved the #define in the course of grouping
> together sun7i-specific things:
>
> > +/* sun7i-specific things: */
> > +/* MIC_PHONE_CAL register offsets and bit fields (A20 only) */
> > +#define SUN7I_CODEC_AC_MIC_PHONE_CAL (0x3c)
> [...]
> > +static const struct regmap_config sun4i_codec_regmap_config = {
> > + .reg_bits = 32,
> > + .reg_stride = 4,
> > + .val_bits = 32,
> > + .max_register = SUN7I_CODEC_AC_MIC_PHONE_CAL,
> > +};
> > +/* end sun7i-specific things */
>
> I thought about also renaming sun4i_codec_regmap_config but decided against it
> since it's fine to use it on A10 and I think it's best if the name reflects
> the minimum required hardware.
>
> On the other hand, once I moved the define, sun4i-codec won't compile if
> sun4i_codec_regmap_config is left at the top. So I had to move it, too.
You can also have the defines on top, and everything just works :)
> It will be clearer once I post a patch doing just the preparation of the
> A10/A20 split.
>
> I just checked A10 vs A20 some more:
> There's also SUN4I_CODEC_AC_SYS_VERI 0x38 present in original ASoC and in 4.4-rc2.
> It's unused by us, not mentioned in the A10 User manual V1.5 20130820, and called
> "AC_DAC_CAL" in the A20 User Manual v1.4 20150510. Ok to delete?
> Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
You can rename it if you want, but it's not like it's of the highest
importance :)
>
> > > static int sun4i_codec_probe(struct platform_device *pdev)
> > > {
> > > struct snd_soc_card *card;
> > > @@ -593,6 +740,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
> > > struct resource *res;
> > > void __iomem *base;
> > > int ret;
> > > + const struct snd_soc_codec_driver* codec_codec;
> >
> > I guess a single codec is enough :)
>
> Modeled after the name of the original variable, see below :)
>
> But OK, I'll rename it to "codec".
>
> Note: the newest original ASoC sun4i-codec has a variable
> "struct sun4i_codec *scodec;"
> as well in the same function (which is a different thing).
I don't know what you're refering to with "newest" and "original".
But two different variables with two different names doesn't seem so
bad, does it?
Thanks!
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151216/fef6d6db/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-16 10:47 ` Maxime Ripard
(?)
@ 2015-12-16 22:30 ` Danny Milosavljevic
-1 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-16 22:30 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
Hi Maxime,
On Wed, 16 Dec 2015 11:47:36 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
>
> Yet, you're using it in both cases (A10 vs A20).
Yes. I'm trying to keep complexity and duplication down.
I figured it wouldn't be bad to have unused registers in the regmap.
(Technially .max_register = MAX(max_register_a10, max_register_a20) would be
better. Should we do that?)
If it's bad in this case, we have to split it up, but frankly the *codec_probe()
function is much too long now and this would make it even longer.
Also, it was that way before, so I'm mostly using it in both cases because
previously it was also used in both cases (with the too-large max-register),
apparently without problems.
Should I duplicate and adapt the structure?
> You can also have the defines on top, and everything just works :)
The idea is to make the compiler complain when I try to use a sun7i define in a
generic sun4i function (or struct, in this case) - because that would probably
be causing problems at runtime, too. Better to catch problems earlier.
So I kept the sun7i-specific things closely together and as much to the bottom
of the file as possible - as a poor-mans modularity.
If I kept the sun7i defines at the top I could use them anywhere with impunity -
also in the A10 case - and it would not complain.
But it's mostly to make the life of the developer easier, so feel free to choose
otherwise. (not sarcasm)
> > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
>
> You can rename it if you want, but it's not like it's of the highest
> importance :)
The only somewhat important part of the name is the "7".
If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
do something else that wasn't intended. Right now it has a "4" although it isn't
an A10 register. This separation should be visible somewhere in the source code,
or problems are going to slip through later.
I agree it's not at all important right now because the register is unused
by us :P
(But it's actually an interesting register, it contains DAC bias calibration
data. Cool that that can be modified)
> > Note: the newest original ASoC sun4i-codec has a variable
> > "struct sun4i_codec *scodec;"
> > as well in the same function (which is a different thing).
>
> I don't know what you're refering to with "newest" and "original".
Oops, sorry, by "newest" I meant "I checked the tree online right before I wrote
the mail".
By "original" I meant "not of my patch".
> But two different variables with two different names doesn't seem so
> bad, does it?
For the computer? No.
Two different variables with almost the same name for humans? Welll.
But it's fine, just wanted to point it out because you can't see it in the patch.
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-16 22:30 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-16 22:30 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi Maxime,
On Wed, 16 Dec 2015 11:47:36 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
>
> Yet, you're using it in both cases (A10 vs A20).
Yes. I'm trying to keep complexity and duplication down.
I figured it wouldn't be bad to have unused registers in the regmap.
(Technially .max_register = MAX(max_register_a10, max_register_a20) would be
better. Should we do that?)
If it's bad in this case, we have to split it up, but frankly the *codec_probe()
function is much too long now and this would make it even longer.
Also, it was that way before, so I'm mostly using it in both cases because
previously it was also used in both cases (with the too-large max-register),
apparently without problems.
Should I duplicate and adapt the structure?
> You can also have the defines on top, and everything just works :)
The idea is to make the compiler complain when I try to use a sun7i define in a
generic sun4i function (or struct, in this case) - because that would probably
be causing problems at runtime, too. Better to catch problems earlier.
So I kept the sun7i-specific things closely together and as much to the bottom
of the file as possible - as a poor-mans modularity.
If I kept the sun7i defines at the top I could use them anywhere with impunity -
also in the A10 case - and it would not complain.
But it's mostly to make the life of the developer easier, so feel free to choose
otherwise. (not sarcasm)
> > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
>
> You can rename it if you want, but it's not like it's of the highest
> importance :)
The only somewhat important part of the name is the "7".
If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
do something else that wasn't intended. Right now it has a "4" although it isn't
an A10 register. This separation should be visible somewhere in the source code,
or problems are going to slip through later.
I agree it's not at all important right now because the register is unused
by us :P
(But it's actually an interesting register, it contains DAC bias calibration
data. Cool that that can be modified)
> > Note: the newest original ASoC sun4i-codec has a variable
> > "struct sun4i_codec *scodec;"
> > as well in the same function (which is a different thing).
>
> I don't know what you're refering to with "newest" and "original".
Oops, sorry, by "newest" I meant "I checked the tree online right before I wrote
the mail".
By "original" I meant "not of my patch".
> But two different variables with two different names doesn't seem so
> bad, does it?
For the computer? No.
Two different variables with almost the same name for humans? Welll.
But it's fine, just wanted to point it out because you can't see it in the patch.
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-16 22:30 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-16 22:30 UTC (permalink / raw)
To: linux-arm-kernel
Hi Maxime,
On Wed, 16 Dec 2015 11:47:36 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
>
> Yet, you're using it in both cases (A10 vs A20).
Yes. I'm trying to keep complexity and duplication down.
I figured it wouldn't be bad to have unused registers in the regmap.
(Technially .max_register = MAX(max_register_a10, max_register_a20) would be
better. Should we do that?)
If it's bad in this case, we have to split it up, but frankly the *codec_probe()
function is much too long now and this would make it even longer.
Also, it was that way before, so I'm mostly using it in both cases because
previously it was also used in both cases (with the too-large max-register),
apparently without problems.
Should I duplicate and adapt the structure?
> You can also have the defines on top, and everything just works :)
The idea is to make the compiler complain when I try to use a sun7i define in a
generic sun4i function (or struct, in this case) - because that would probably
be causing problems at runtime, too. Better to catch problems earlier.
So I kept the sun7i-specific things closely together and as much to the bottom
of the file as possible - as a poor-mans modularity.
If I kept the sun7i defines at the top I could use them anywhere with impunity -
also in the A10 case - and it would not complain.
But it's mostly to make the life of the developer easier, so feel free to choose
otherwise. (not sarcasm)
> > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
>
> You can rename it if you want, but it's not like it's of the highest
> importance :)
The only somewhat important part of the name is the "7".
If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
do something else that wasn't intended. Right now it has a "4" although it isn't
an A10 register. This separation should be visible somewhere in the source code,
or problems are going to slip through later.
I agree it's not at all important right now because the register is unused
by us :P
(But it's actually an interesting register, it contains DAC bias calibration
data. Cool that that can be modified)
> > Note: the newest original ASoC sun4i-codec has a variable
> > "struct sun4i_codec *scodec;"
> > as well in the same function (which is a different thing).
>
> I don't know what you're refering to with "newest" and "original".
Oops, sorry, by "newest" I meant "I checked the tree online right before I wrote
the mail".
By "original" I meant "not of my patch".
> But two different variables with two different names doesn't seem so
> bad, does it?
For the computer? No.
Two different variables with almost the same name for humans? Welll.
But it's fine, just wanted to point it out because you can't see it in the patch.
Thanks,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-16 22:30 ` Danny Milosavljevic
(?)
@ 2015-12-18 10:19 ` Maxime Ripard
-1 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-18 10:19 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 2985 bytes --]
On Wed, Dec 16, 2015 at 11:30:51PM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Wed, 16 Dec 2015 11:47:36 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
> >
> > Yet, you're using it in both cases (A10 vs A20).
>
> Yes. I'm trying to keep complexity and duplication down.
> I figured it wouldn't be bad to have unused registers in the regmap.
>
> (Technially .max_register = MAX(max_register_a10, max_register_a20) would be
> better. Should we do that?)
>
> If it's bad in this case, we have to split it up, but frankly the *codec_probe()
> function is much too long now and this would make it even longer.
>
> Also, it was that way before, so I'm mostly using it in both cases because
> previously it was also used in both cases (with the too-large max-register),
> apparently without problems.
>
> Should I duplicate and adapt the structure?
No, my point was that you don't need to move it around at all.
> > You can also have the defines on top, and everything just works :)
>
> The idea is to make the compiler complain when I try to use a sun7i define in a
> generic sun4i function (or struct, in this case) - because that would probably
> be causing problems at runtime, too. Better to catch problems earlier.
> So I kept the sun7i-specific things closely together and as much to the bottom
> of the file as possible - as a poor-mans modularity.
> If I kept the sun7i defines at the top I could use them anywhere with impunity -
> also in the A10 case - and it would not complain.
>
> But it's mostly to make the life of the developer easier, so feel free to choose
> otherwise. (not sarcasm)
I understand your point to develop it, but now, the development is done :)
Having all the defines packed together is easier to read and maintain
after the development is done.
> > > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> >
> > You can rename it if you want, but it's not like it's of the highest
> > importance :)
>
> The only somewhat important part of the name is the "7".
> If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
> do something else that wasn't intended. Right now it has a "4" although it isn't
> an A10 register. This separation should be visible somewhere in the source code,
> or problems are going to slip through later.
>
> I agree it's not at all important right now because the register is unused
> by us :P
Exactly my point ;)
Like I said, if you want to rename it, go ahead. It would also be a
good idea to open a github issue on allwinner's documentation repo to
make them know that the register name doesn't match between the
register list and the register documentations.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-18 10:19 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-18 10:19 UTC (permalink / raw)
To: Danny Milosavljevic
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
[-- Attachment #1: Type: text/plain, Size: 2942 bytes --]
On Wed, Dec 16, 2015 at 11:30:51PM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Wed, 16 Dec 2015 11:47:36 +0100
> Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
>
> > > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
> >
> > Yet, you're using it in both cases (A10 vs A20).
>
> Yes. I'm trying to keep complexity and duplication down.
> I figured it wouldn't be bad to have unused registers in the regmap.
>
> (Technially .max_register = MAX(max_register_a10, max_register_a20) would be
> better. Should we do that?)
>
> If it's bad in this case, we have to split it up, but frankly the *codec_probe()
> function is much too long now and this would make it even longer.
>
> Also, it was that way before, so I'm mostly using it in both cases because
> previously it was also used in both cases (with the too-large max-register),
> apparently without problems.
>
> Should I duplicate and adapt the structure?
No, my point was that you don't need to move it around at all.
> > You can also have the defines on top, and everything just works :)
>
> The idea is to make the compiler complain when I try to use a sun7i define in a
> generic sun4i function (or struct, in this case) - because that would probably
> be causing problems at runtime, too. Better to catch problems earlier.
> So I kept the sun7i-specific things closely together and as much to the bottom
> of the file as possible - as a poor-mans modularity.
> If I kept the sun7i defines at the top I could use them anywhere with impunity -
> also in the A10 case - and it would not complain.
>
> But it's mostly to make the life of the developer easier, so feel free to choose
> otherwise. (not sarcasm)
I understand your point to develop it, but now, the development is done :)
Having all the defines packed together is easier to read and maintain
after the development is done.
> > > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> >
> > You can rename it if you want, but it's not like it's of the highest
> > importance :)
>
> The only somewhat important part of the name is the "7".
> If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
> do something else that wasn't intended. Right now it has a "4" although it isn't
> an A10 register. This separation should be visible somewhere in the source code,
> or problems are going to slip through later.
>
> I agree it's not at all important right now because the register is unused
> by us :P
Exactly my point ;)
Like I said, if you want to rename it, go ahead. It would also be a
good idea to open a github issue on allwinner's documentation repo to
make them know that the register name doesn't match between the
register list and the register documentations.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 21+ messages in thread
* [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-18 10:19 ` Maxime Ripard
0 siblings, 0 replies; 21+ messages in thread
From: Maxime Ripard @ 2015-12-18 10:19 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Dec 16, 2015 at 11:30:51PM +0100, Danny Milosavljevic wrote:
> Hi Maxime,
>
> On Wed, 16 Dec 2015 11:47:36 +0100
> Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
>
> > > Because SUN4I_CODEC_AC_MIC_PHONE_CAL is sun7i-specific.
> >
> > Yet, you're using it in both cases (A10 vs A20).
>
> Yes. I'm trying to keep complexity and duplication down.
> I figured it wouldn't be bad to have unused registers in the regmap.
>
> (Technially .max_register = MAX(max_register_a10, max_register_a20) would be
> better. Should we do that?)
>
> If it's bad in this case, we have to split it up, but frankly the *codec_probe()
> function is much too long now and this would make it even longer.
>
> Also, it was that way before, so I'm mostly using it in both cases because
> previously it was also used in both cases (with the too-large max-register),
> apparently without problems.
>
> Should I duplicate and adapt the structure?
No, my point was that you don't need to move it around at all.
> > You can also have the defines on top, and everything just works :)
>
> The idea is to make the compiler complain when I try to use a sun7i define in a
> generic sun4i function (or struct, in this case) - because that would probably
> be causing problems at runtime, too. Better to catch problems earlier.
> So I kept the sun7i-specific things closely together and as much to the bottom
> of the file as possible - as a poor-mans modularity.
> If I kept the sun7i defines at the top I could use them anywhere with impunity -
> also in the A10 case - and it would not complain.
>
> But it's mostly to make the life of the developer easier, so feel free to choose
> otherwise. (not sarcasm)
I understand your point to develop it, but now, the development is done :)
Having all the defines packed together is easier to read and maintain
after the development is done.
> > > Or is it better to rename it to "SUN7I_CODEC_AC_DAC_CAL" rather than delete?
> >
> > You can rename it if you want, but it's not like it's of the highest
> > importance :)
>
> The only somewhat important part of the name is the "7".
> If you use a "7"-register on an A10, it's not going to work at runtime, or worse:
> do something else that wasn't intended. Right now it has a "4" although it isn't
> an A10 register. This separation should be visible somewhere in the source code,
> or problems are going to slip through later.
>
> I agree it's not at all important right now because the register is unused
> by us :P
Exactly my point ;)
Like I said, if you want to rename it, go ahead. It would also be a
good idea to open a github issue on allwinner's documentation repo to
make them know that the register name doesn't match between the
register list and the register documentations.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20151218/e782bb2e/attachment.sig>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
2015-12-18 10:19 ` Maxime Ripard
(?)
@ 2015-12-21 18:41 ` Danny Milosavljevic
-1 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-21 18:41 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi, Liam Girdwood, Mark Brown, Jaroslav Kysela,
Takashi Iwai, Chen-Yu Tsai, alsa-devel, linux-arm-kernel,
linux-kernel
Hi Maxime,
On Fri, 18 Dec 2015 11:19:43 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> It would also be a good idea to open a github issue on allwinner's
> documentation repo to make them know that the register name doesn't
> match between the register list and the register documentations.
I did that now. the issue report is at
https://github.com/allwinner-zh/documents/issues/11
Cheers,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-21 18:41 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-21 18:41 UTC (permalink / raw)
To: Maxime Ripard
Cc: linux-sunxi-/JYPxA39Uh5TLH3MbocFFw, Liam Girdwood, Mark Brown,
Jaroslav Kysela, Takashi Iwai, Chen-Yu Tsai,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi Maxime,
On Fri, 18 Dec 2015 11:19:43 +0100
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> It would also be a good idea to open a github issue on allwinner's
> documentation repo to make them know that the register name doesn't
> match between the register list and the register documentations.
I did that now. the issue report is at
https://github.com/allwinner-zh/documents/issues/11
Cheers,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
* [linux-sunxi] Re: [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs
@ 2015-12-21 18:41 ` Danny Milosavljevic
0 siblings, 0 replies; 21+ messages in thread
From: Danny Milosavljevic @ 2015-12-21 18:41 UTC (permalink / raw)
To: linux-arm-kernel
Hi Maxime,
On Fri, 18 Dec 2015 11:19:43 +0100
Maxime Ripard <maxime.ripard@free-electrons.com> wrote:
> It would also be a good idea to open a github issue on allwinner's
> documentation repo to make them know that the register name doesn't
> match between the register list and the register documentations.
I did that now. the issue report is at
https://github.com/allwinner-zh/documents/issues/11
Cheers,
Danny
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2015-12-21 18:41 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-09 22:55 [PATCH v6] sun4i-codec: Add FM, Line and Mic inputs Danny Milosavljevic
2015-12-09 22:55 ` Danny Milosavljevic
2015-12-09 22:55 ` Danny Milosavljevic
2015-12-13 20:58 ` Maxime Ripard
2015-12-13 20:58 ` Maxime Ripard
2015-12-13 20:58 ` Maxime Ripard
2015-12-15 1:52 ` Danny Milosavljevic
2015-12-15 1:52 ` Danny Milosavljevic
2015-12-15 1:52 ` Danny Milosavljevic
2015-12-16 10:47 ` Maxime Ripard
2015-12-16 10:47 ` Maxime Ripard
2015-12-16 10:47 ` Maxime Ripard
2015-12-16 22:30 ` [linux-sunxi] " Danny Milosavljevic
2015-12-16 22:30 ` Danny Milosavljevic
2015-12-16 22:30 ` Danny Milosavljevic
2015-12-18 10:19 ` [linux-sunxi] " Maxime Ripard
2015-12-18 10:19 ` Maxime Ripard
2015-12-18 10:19 ` Maxime Ripard
2015-12-21 18:41 ` [linux-sunxi] " Danny Milosavljevic
2015-12-21 18:41 ` Danny Milosavljevic
2015-12-21 18:41 ` Danny Milosavljevic
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.