[PATCH v9 0/2] sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In

[PATCH v9 0/2] sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In

[Danny Milosavljevic]

This patch adds support for some mixer controls:
- Line-In
- FM-In
- Mic 2
- Capture Source
- Differential Line-In

v9 changes compared to v8 are:
 - added Line Differential Capture Switch.
 - split Capture Source into Left Capture Select, Right Capture Select.
 - added Line Capture Volume.
 - rename "sun4i_codec_widgets" to "sun4i_codec_controls" for 
   consistency with the struct field it's used in.
 - rename "Line-In" to "Line".
 - rename "Power Amplifier Playback Volume" to "Headphone Playback Volume".

v8 changes compared to v7 are:
 - fixed the routes for line and mic capturing.

v7 changes compared to v6 are:
 - preparation for different A20, A10 controls is now in an extra patch.
 - all register definitions are now at the top.
 - sun7i-specific things (A20-specific things) are now less grouped-together.
 - rename "Power Amplifier Volume" to "Power Amplifier Playback Volume".

v6 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.

v5 changes compared to v4 are:
 - Mic preamplifier controls have more common names now.
 - Mic preamplifier scale has a 0 dB entry as well now, as documented in the 
   A20 user manual.
 - Mic preamplifier has special cases for A20 and A10 now.
 - Gain controls have "Gain" in the name now.

v4 changes compared to v3 are:
 - names of the input are not uppercase anymore.
 - bit index constants are now named as in the A20 user manual v1.4.
 - added Mic1-In, Mac2-In.
 - added Mic1 and Mic2 Pre-Amplifiers.

v3 changes compared to v2 are:
 - added DAPM routes.

v2 changes compared to v1 are:
 - moved Line-In and FM-In playback switches to their respective 
   sun4i_codec_*_mixer_controls.

v1 changes:
 - added linein, fmin output volumes and switches.

Danny Milosavljevic (2):
  ASoC: sun4i-codec: Distinguish sun4i from sun7i
  Add mixer controls: Line-In, FM-In, Mic 2, Capture Source,
    Differential Line-In.

 sound/soc/sunxi/sun4i-codec.c | 278 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 259 insertions(+), 19 deletions(-)

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Danny Milosavljevic]

This distinguishes sun4i from sun7i. It is necessary because they use
different registers for the audio mixer.
---
 sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ASoC-sun4i-codec-Distinguish-sun4i-from-sun7i.patch
Type: text/x-patch
Size: 3488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160826/31c7da26/attachment.bin>

[PATCH v9 2/2] Add mixer controls: Line-In, FM-In, Mic 2, Capture Source, Differential Line-In.

[Danny Milosavljevic]

Note: Mic1 Capture Volume is in a different register on A20 than on A10.
Note: Mic2 Capture Volume is in a different register on A20 than on A10.
---
 sound/soc/sunxi/sun4i-codec.c | 256 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 236 insertions(+), 20 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Add-mixer-controls-Line-In-FM-In-Mic-2-Capture-Sourc.patch
Type: text/x-patch
Size: 14407 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160826/3fd6d190/attachment-0001.bin>

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Chen-Yu Tsai]

On Fri, Aug 26, 2016 at 2:53 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
>
> This distinguishes sun4i from sun7i. It is necessary because they use
> different registers for the audio mixer.
> ---
>  sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
>

Please send your patches inline.

ChenYu

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Danny Milosavljevic]

This distinguishes sun4i from sun7i. It is necessary because they use
different registers for the audio mixer.
---
 sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ASoC-sun4i-codec-Distinguish-sun4i-from-sun7i.patch
Type: text/x-patch
Size: 3488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160826/cbb8efda/attachment.bin>

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Danny Milosavljevic]

This distinguishes sun4i from sun7i. It is necessary because they use
different registers for the audio mixer.
---
 sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-ASoC-sun4i-codec-Distinguish-sun4i-from-sun7i.patch
Type: text/x-patch
Size: 3488 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160829/6c2b94d6/attachment.bin>

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Danny Milosavljevic]

This distinguishes sun4i from sun7i. It is necessary because they use
different registers for the audio mixer.
---
 sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 0e19c50..30f4ea2 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -96,8 +96,9 @@
 /* Other various ADC registers */
 #define SUN4I_CODEC_DAC_TXCNT			(0x30)
 #define SUN4I_CODEC_ADC_RXCNT			(0x34)
-#define SUN4I_CODEC_AC_SYS_VERI			(0x38)
-#define SUN4I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
+
+#define SUN7I_CODEC_AC_DAC_CAL			(0x38)
+#define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
 
 struct sun4i_codec {
 	struct device	*dev;
@@ -509,10 +510,17 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
 
 static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
 
-static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
-	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
-		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
-		       sun4i_codec_pa_volume_scale),
+#define SUN4I_COMMON_CODEC_CONTROLS \
+	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
+		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
+		       sun4i_codec_pa_volume_scale)
+
+static const struct snd_kcontrol_new sun4i_codec_controls[] = {
+	SUN4I_COMMON_CODEC_CONTROLS,
+};
+
+static const struct snd_kcontrol_new sun7i_codec_controls[] = {
+	SUN4I_COMMON_CODEC_CONTROLS,
 };
 
 static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
@@ -629,8 +637,18 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
 
 static struct snd_soc_codec_driver sun4i_codec_codec = {
 	.component_driver = {
-		.controls		= sun4i_codec_widgets,
-		.num_controls		= ARRAY_SIZE(sun4i_codec_widgets),
+		.controls		= sun4i_codec_controls,
+		.num_controls		= ARRAY_SIZE(sun4i_codec_controls),
+		.dapm_widgets           = sun4i_codec_codec_dapm_widgets,
+		.num_dapm_widgets       = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
+		.dapm_routes            = sun4i_codec_codec_dapm_routes,
+		.num_dapm_routes        = ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
+	},
+};
+static struct snd_soc_codec_driver sun7i_codec_codec = {
+	.component_driver = {
+		.controls               = sun7i_codec_controls,
+		.num_controls           = ARRAY_SIZE(sun7i_codec_controls),
 		.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
 		.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
 		.dapm_routes		= sun4i_codec_codec_dapm_routes,
@@ -682,7 +700,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
 	.reg_bits	= 32,
 	.reg_stride	= 4,
 	.val_bits	= 32,
-	.max_register	= SUN4I_CODEC_AC_MIC_PHONE_CAL,
+	.max_register	= SUN7I_CODEC_AC_MIC_PHONE_CAL,
 };
 
 static const struct of_device_id sun4i_codec_of_match[] = {
@@ -760,6 +778,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
 {
 	struct snd_soc_card *card;
 	struct sun4i_codec *scodec;
+	struct snd_soc_codec_driver *codec;
 	struct resource *res;
 	void __iomem *base;
 	int ret;
@@ -822,7 +841,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
 	scodec->capture_dma_data.maxburst = 4;
 	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
 
-	ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
+	if (of_device_is_compatible(pdev->dev.of_node,
+				    "allwinner,sun7i-a20-codec"))
+		codec = &sun7i_codec_codec;
+	else
+		codec = &sun4i_codec_codec;
+	ret = snd_soc_register_codec(&pdev->dev, codec,
 				     &sun4i_codec_dai, 1);
 	if (ret) {
 		dev_err(&pdev->dev, "Failed to register our codec\n");

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Chen-Yu Tsai]

On Tue, Aug 30, 2016 at 1:44 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
> This distinguishes sun4i from sun7i. It is necessary because they use
> different registers for the audio mixer.
> ---
>  sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 0e19c50..30f4ea2 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,9 @@
>  /* Other various ADC registers */
>  #define SUN4I_CODEC_DAC_TXCNT                  (0x30)
>  #define SUN4I_CODEC_ADC_RXCNT                  (0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI                        (0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL           (0x3c)
> +
> +#define SUN7I_CODEC_AC_DAC_CAL                 (0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL           (0x3c)
>
>  struct sun4i_codec {
>         struct device   *dev;
> @@ -509,10 +510,17 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
>
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> -       SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -                      SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -                      sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> +       SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +                      SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +                      sun4i_codec_pa_volume_scale)

This is slightly worrying. It will get messy as you add more widgets.

> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> +       SUN4I_COMMON_CODEC_CONTROLS,
> +};
> +
> +static const struct snd_kcontrol_new sun7i_codec_controls[] = {
> +       SUN4I_COMMON_CODEC_CONTROLS,
>  };
>
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -629,8 +637,18 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
>
>  static struct snd_soc_codec_driver sun4i_codec_codec = {
>         .component_driver = {
> -               .controls               = sun4i_codec_widgets,
> -               .num_controls           = ARRAY_SIZE(sun4i_codec_widgets),
> +               .controls               = sun4i_codec_controls,
> +               .num_controls           = ARRAY_SIZE(sun4i_codec_controls),
> +               .dapm_widgets           = sun4i_codec_codec_dapm_widgets,
> +               .num_dapm_widgets       = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> +               .dapm_routes            = sun4i_codec_codec_dapm_routes,
> +               .num_dapm_routes        = ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> +       },
> +};
> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> +       .component_driver = {
> +               .controls               = sun7i_codec_controls,
> +               .num_controls           = ARRAY_SIZE(sun7i_codec_controls),
>                 .dapm_widgets           = sun4i_codec_codec_dapm_widgets,
>                 .num_dapm_widgets       = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
>                 .dapm_routes            = sun4i_codec_codec_dapm_routes,
> @@ -682,7 +700,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
>         .reg_bits       = 32,
>         .reg_stride     = 4,
>         .val_bits       = 32,
> -       .max_register   = SUN4I_CODEC_AC_MIC_PHONE_CAL,
> +       .max_register   = SUN7I_CODEC_AC_MIC_PHONE_CAL,
>  };

On sun4i the registers only go as far as SUN4I_CODEC_ADC_RXCNT.
Could you add a separate regmap_config as well?

>
>  static const struct of_device_id sun4i_codec_of_match[] = {
> @@ -760,6 +778,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>  {
>         struct snd_soc_card *card;
>         struct sun4i_codec *scodec;
> +       struct snd_soc_codec_driver *codec;
>         struct resource *res;
>         void __iomem *base;
>         int ret;
> @@ -822,7 +841,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>         scodec->capture_dma_data.maxburst = 4;
>         scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>
> -       ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
> +       if (of_device_is_compatible(pdev->dev.of_node,
> +                                   "allwinner,sun7i-a20-codec"))
> +               codec = &sun7i_codec_codec;
> +       else
> +               codec = &sun4i_codec_codec;
> +       ret = snd_soc_register_codec(&pdev->dev, codec,

This will get reworked into something like in

https://github.com/wens/linux/commit/54a58e3087fb67572ca416256bc000e08b163480

I haven't gotten around to posting this series yet though.

ChenYu

>                                      &sun4i_codec_dai, 1);
>         if (ret) {
>                 dev_err(&pdev->dev, "Failed to register our codec\n");

[PATCH v9 1/2] ASoC: sun4i-codec: Distinguish sun4i from sun7i

[Maxime Ripard]

Hi Danny,

On Tue, Aug 30, 2016 at 07:44:02AM +0200, Danny Milosavljevic wrote:
> This distinguishes sun4i from sun7i. It is necessary because they use
> different registers for the audio mixer.

Your Signed-off-by is missing

> ---
>  sound/soc/sunxi/sun4i-codec.c | 44 +++++++++++++++++++++++++++++++++----------
>  1 file changed, 34 insertions(+), 10 deletions(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index 0e19c50..30f4ea2 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -96,8 +96,9 @@
>  /* Other various ADC registers */
>  #define SUN4I_CODEC_DAC_TXCNT			(0x30)
>  #define SUN4I_CODEC_ADC_RXCNT			(0x34)
> -#define SUN4I_CODEC_AC_SYS_VERI			(0x38)
> -#define SUN4I_CODEC_AC_MIC_PHONE_CAL		(0x3c)
> +
> +#define SUN7I_CODEC_AC_DAC_CAL			(0x38)
> +#define SUN7I_CODEC_AC_MIC_PHONE_CAL		(0x3c)

That will need at least some mention in the commit log.

>  
>  struct sun4i_codec {
>  	struct device	*dev;
> @@ -509,10 +510,17 @@ static const struct snd_kcontrol_new sun4i_codec_pa_mute =
>  
>  static DECLARE_TLV_DB_SCALE(sun4i_codec_pa_volume_scale, -6300, 100, 1);
>  
> -static const struct snd_kcontrol_new sun4i_codec_widgets[] = {
> -	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,
> -		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,
> -		       sun4i_codec_pa_volume_scale),
> +#define SUN4I_COMMON_CODEC_CONTROLS \
> +	SOC_SINGLE_TLV("Power Amplifier Volume", SUN4I_CODEC_DAC_ACTL,\
> +		       SUN4I_CODEC_DAC_ACTL_PA_VOL, 0x3F, 0,\
> +		       sun4i_codec_pa_volume_scale)
> +
> +static const struct snd_kcontrol_new sun4i_codec_controls[] = {
> +	SUN4I_COMMON_CODEC_CONTROLS,
> +};
> +
> +static const struct snd_kcontrol_new sun7i_codec_controls[] = {
> +	SUN4I_COMMON_CODEC_CONTROLS,
>  };
>  
>  static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> @@ -629,8 +637,18 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
>  
>  static struct snd_soc_codec_driver sun4i_codec_codec = {
>  	.component_driver = {
> -		.controls		= sun4i_codec_widgets,
> -		.num_controls		= ARRAY_SIZE(sun4i_codec_widgets),
> +		.controls		= sun4i_codec_controls,
> +		.num_controls		= ARRAY_SIZE(sun4i_codec_controls),
> +		.dapm_widgets           = sun4i_codec_codec_dapm_widgets,
> +		.num_dapm_widgets       = ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
> +		.dapm_routes            = sun4i_codec_codec_dapm_routes,
> +		.num_dapm_routes        = ARRAY_SIZE(sun4i_codec_codec_dapm_routes),
> +	},
> +};

You need a new line here.

> +static struct snd_soc_codec_driver sun7i_codec_codec = {
> +	.component_driver = {
> +		.controls               = sun7i_codec_controls,
> +		.num_controls           = ARRAY_SIZE(sun7i_codec_controls),
>  		.dapm_widgets		= sun4i_codec_codec_dapm_widgets,
>  		.num_dapm_widgets	= ARRAY_SIZE(sun4i_codec_codec_dapm_widgets),
>  		.dapm_routes		= sun4i_codec_codec_dapm_routes,
> @@ -682,7 +700,7 @@ static const struct regmap_config sun4i_codec_regmap_config = {
>  	.reg_bits	= 32,
>  	.reg_stride	= 4,
>  	.val_bits	= 32,
> -	.max_register	= SUN4I_CODEC_AC_MIC_PHONE_CAL,
> +	.max_register	= SUN7I_CODEC_AC_MIC_PHONE_CAL,
>  };
>  
>  static const struct of_device_id sun4i_codec_of_match[] = {
> @@ -760,6 +778,7 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>  {
>  	struct snd_soc_card *card;
>  	struct sun4i_codec *scodec;
> +	struct snd_soc_codec_driver *codec;
>  	struct resource *res;
>  	void __iomem *base;
>  	int ret;
> @@ -822,7 +841,12 @@ static int sun4i_codec_probe(struct platform_device *pdev)
>  	scodec->capture_dma_data.maxburst = 4;
>  	scodec->capture_dma_data.addr_width = DMA_SLAVE_BUSWIDTH_2_BYTES;
>  
> -	ret = snd_soc_register_codec(&pdev->dev, &sun4i_codec_codec,
> +	if (of_device_is_compatible(pdev->dev.of_node,
> +				    "allwinner,sun7i-a20-codec"))
> +		codec = &sun7i_codec_codec;
> +	else
> +		codec = &sun4i_codec_codec;

It would be better to add a structure assiocated to the data field of
of_device_id. That way, you'll also be able to have any quirks without
adding more checks (such as a different regmap config, like Chen-Yu
suggested).

Thanks!
Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20160831/a9e00333/attachment.sig>