[PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

This patch adds support for the wm8581 codec to the wm8580 driver.
The wm8581 codec hardware adds a fourth DAC and otherwise is
compatible with the wm8580 codec.

of_device_id data is used to allow the driver to select the
suitable software variables from the device tree codec selection.
The wm8580_driver_data struct is used to store the number of DACs
as well as the required stream names for both codecs.

The snd_soc_dai_driver no longer lists the stream names and for
playback the channels_max. These variables are set during i2c
probe from the of_device_id supplied wm8580_driver_data struct.

With knowledge of the number of DACs in use, the DAC4 controls,
widgets and routes are added as required for DAC4.

The device tree documentation for the wm8580 is altered to list
the wm8581 codec support, as is the Kconfig file.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 Documentation/devicetree/bindings/sound/wm8580.txt |  4 +-
 sound/soc/codecs/Kconfig                           |  2 +-
 sound/soc/codecs/wm8580.c                          | 96 +++++++++++++++++++---
 sound/soc/codecs/wm8580.h                          |  6 ++
 4 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/wm8580.txt b/Documentation/devicetree/bindings/sound/wm8580.txt
index 7d9821f..78fce9b 100644
--- a/Documentation/devicetree/bindings/sound/wm8580.txt
+++ b/Documentation/devicetree/bindings/sound/wm8580.txt
@@ -1,10 +1,10 @@
-WM8580 audio CODEC
+WM8580 and WM8581 audio CODEC
 
 This device supports I2C only.
 
 Required properties:
 
-  - compatible : "wlf,wm8580"
+  - compatible : "wlf,wm8580", "wlf,wm8581"
 
   - reg : the I2C address of the device.
 
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c67667b..199bec8 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -914,7 +914,7 @@ config SND_SOC_WM8523
 	depends on I2C
 
 config SND_SOC_WM8580
-	tristate "Wolfson Microelectronics WM8523 CODEC"
+	tristate "Wolfson Microelectronics WM8580 and WM8581 CODECs"
 	depends on I2C
 
 config SND_SOC_WM8711
diff --git a/sound/soc/codecs/wm8580.c b/sound/soc/codecs/wm8580.c
index faa7287..a348549 100644
--- a/sound/soc/codecs/wm8580.c
+++ b/sound/soc/codecs/wm8580.c
@@ -1,5 +1,5 @@
 /*
- * wm8580.c  --  WM8580 ALSA Soc Audio driver
+ * wm8580.c  --  WM8580 and WM8581 ALSA Soc Audio driver
  *
  * Copyright 2008-12 Wolfson Microelectronics PLC.
  *
@@ -12,6 +12,9 @@
  *  The WM8580 is a multichannel codec with S/PDIF support, featuring six
  *  DAC channels and two ADC channels.
  *
+ *  The WM8581 is a multichannel codec with S/PDIF support, featuring eight
+ *  DAC channels and two ADC channels.
+ *
  *  Currently only the primary audio interface is supported - S/PDIF and
  *  the secondary audio interfaces are not.
  */
@@ -65,6 +68,8 @@
 #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
 #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
 #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
+#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
+#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B
 #define WM8580_MASTER_DIGITAL_ATTENUATION    0x1C
 #define WM8580_ADC_CONTROL1                  0x1D
 #define WM8580_SPDTXCHAN0                    0x1E
@@ -242,6 +247,7 @@ struct wm8580_priv {
 	struct regulator_bulk_data supplies[WM8580_NUM_SUPPLIES];
 	struct pll_state a;
 	struct pll_state b;
+	const struct wm8580_driver_data *drvdata;
 	int sysclk[2];
 };
 
@@ -306,6 +312,19 @@ SOC_DOUBLE("Capture Switch", WM8580_ADC_CONTROL1, 0, 1, 1, 1),
 SOC_SINGLE("Capture High-Pass Filter Switch", WM8580_ADC_CONTROL1, 4, 1, 0),
 };
 
+static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
+SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
+		     WM8580_DIGITAL_ATTENUATION_DACL4,
+		     WM8580_DIGITAL_ATTENUATION_DACR4,
+		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
+
+SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
+
+SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
+
+SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
+};
+
 static const struct snd_soc_dapm_widget wm8580_dapm_widgets[] = {
 SND_SOC_DAPM_DAC("DAC1", "Playback", WM8580_PWRDN1, 2, 1),
 SND_SOC_DAPM_DAC("DAC2", "Playback", WM8580_PWRDN1, 3, 1),
@@ -324,6 +343,13 @@ SND_SOC_DAPM_INPUT("AINL"),
 SND_SOC_DAPM_INPUT("AINR"),
 };
 
+static const struct snd_soc_dapm_widget wm8580_dac4_dapm_widgets[] = {
+SND_SOC_DAPM_DAC("DAC4", "Playback", WM8580_PWRDN1, 5, 1),
+
+SND_SOC_DAPM_OUTPUT("VOUT4L"),
+SND_SOC_DAPM_OUTPUT("VOUT4R"),
+};
+
 static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "VOUT1L", NULL, "DAC1" },
 	{ "VOUT1R", NULL, "DAC1" },
@@ -338,6 +364,11 @@ static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "ADC", NULL, "AINR" },
 };
 
+static const struct snd_soc_dapm_route wm8580_dac4_dapm_routes[] = {
+	{ "VOUT4L", NULL, "DAC4" },
+	{ "VOUT4R", NULL, "DAC4" },
+};
+
 /* PLL divisors */
 struct _pll_div {
 	u32 prescale:1;
@@ -837,19 +868,16 @@ static const struct snd_soc_dai_ops wm8580_dai_ops_capture = {
 
 static struct snd_soc_dai_driver wm8580_dai[] = {
 	{
-		.name = "wm8580-hifi-playback",
 		.id	= WM8580_DAI_PAIFRX,
 		.playback = {
 			.stream_name = "Playback",
 			.channels_min = 1,
-			.channels_max = 6,
 			.rates = SNDRV_PCM_RATE_8000_192000,
 			.formats = WM8580_FORMATS,
 		},
 		.ops = &wm8580_dai_ops_playback,
 	},
 	{
-		.name = "wm8580-hifi-capture",
 		.id	=	WM8580_DAI_PAIFTX,
 		.capture = {
 			.stream_name = "Capture",
@@ -865,8 +893,22 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
 static int wm8580_probe(struct snd_soc_codec *codec)
 {
 	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
+	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
 	int ret = 0;
 
+	switch (wm8580->drvdata->num_dacs) {
+	case 4:
+		snd_soc_add_codec_controls(codec, wm8580_dac4_snd_controls,
+					ARRAY_SIZE(wm8580_dac4_snd_controls));
+		snd_soc_dapm_new_controls(dapm, wm8580_dac4_dapm_widgets,
+					ARRAY_SIZE(wm8580_dac4_dapm_widgets));
+		snd_soc_dapm_add_routes(dapm, wm8580_dac4_dapm_routes,
+					ARRAY_SIZE(wm8580_dac4_dapm_routes));
+		break;
+	default:
+		break;
+	}
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(wm8580->supplies),
 				    wm8580->supplies);
 	if (ret != 0) {
@@ -914,12 +956,6 @@ static const struct snd_soc_codec_driver soc_codec_dev_wm8580 = {
 	},
 };
 
-static const struct of_device_id wm8580_of_match[] = {
-	{ .compatible = "wlf,wm8580" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, wm8580_of_match);
-
 static const struct regmap_config wm8580_regmap = {
 	.reg_bits = 7,
 	.val_bits = 9,
@@ -932,10 +968,32 @@ static const struct regmap_config wm8580_regmap = {
 	.volatile_reg = wm8580_volatile,
 };
 
+const struct wm8580_driver_data wm8580_data = {
+	.name_playback = "wm8580-hifi-playback",
+	.name_capture = "wm8580-hifi-capture",
+	.num_dacs = 3,
+};
+EXPORT_SYMBOL_GPL(wm8580_data);
+
+const struct wm8580_driver_data wm8581_data = {
+	.name_playback = "wm8581-hifi-playback",
+	.name_capture = "wm8581-hifi-capture",
+	.num_dacs = 4,
+};
+EXPORT_SYMBOL_GPL(wm8581_data);
+
+static const struct of_device_id wm8580_of_match[] = {
+	{ .compatible = "wlf,wm8580", .data = &wm8580_data },
+	{ .compatible = "wlf,wm8581", .data = &wm8581_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, wm8580_of_match);
+
 #if IS_ENABLED(CONFIG_I2C)
 static int wm8580_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
+	const struct of_device_id *of_id;
 	struct wm8580_priv *wm8580;
 	int ret, i;
 
@@ -960,6 +1018,20 @@ static int wm8580_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, wm8580);
 
+	of_id = of_match_device(wm8580_of_match, &i2c->dev);
+	if (of_id)
+		wm8580->drvdata = of_id->data;
+
+	if (!wm8580->drvdata) {
+		dev_err(&i2c->dev, "failed to find driver data\n");
+		return -EINVAL;
+	}
+
+	/* Each dac supports stereo input */
+	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
+	wm8580_dai[0].name = wm8580->drvdata->name_playback;
+	wm8580_dai[1].name = wm8580->drvdata->name_capture;
+
 	ret =  snd_soc_register_codec(&i2c->dev,
 			&soc_codec_dev_wm8580, wm8580_dai, ARRAY_SIZE(wm8580_dai));
 
@@ -973,7 +1045,8 @@ static int wm8580_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id wm8580_i2c_id[] = {
-	{ "wm8580", 0 },
+	{ "wm8580", (kernel_ulong_t)&wm8580_data },
+	{ "wm8581", (kernel_ulong_t)&wm8581_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, wm8580_i2c_id);
@@ -1014,4 +1087,5 @@ module_exit(wm8580_exit);
 
 MODULE_DESCRIPTION("ASoC WM8580 driver");
 MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_AUTHOR("Matt Flax <flatmax@flatmax.org>");
 MODULE_LICENSE("GPL");
diff --git a/sound/soc/codecs/wm8580.h b/sound/soc/codecs/wm8580.h
index 1d34656..868e400 100644
--- a/sound/soc/codecs/wm8580.h
+++ b/sound/soc/codecs/wm8580.h
@@ -15,6 +15,12 @@
 #ifndef _WM8580_H
 #define _WM8580_H
 
+struct wm8580_driver_data {
+	char name_playback[24];
+	char name_capture[24];
+	int num_dacs;
+};
+
 #define WM8580_PLLA  1
 #define WM8580_PLLB  2
 
-- 
2.7.4

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Charles Keepax]

On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
> This patch adds support for the wm8581 codec to the wm8580 driver.
> The wm8581 codec hardware adds a fourth DAC and otherwise is
> compatible with the wm8580 codec.
> 
> of_device_id data is used to allow the driver to select the
> suitable software variables from the device tree codec selection.
> The wm8580_driver_data struct is used to store the number of DACs
> as well as the required stream names for both codecs.
> 
> The snd_soc_dai_driver no longer lists the stream names and for
> playback the channels_max. These variables are set during i2c
> probe from the of_device_id supplied wm8580_driver_data struct.
> 
> With knowledge of the number of DACs in use, the DAC4 controls,
> widgets and routes are added as required for DAC4.
> 
> The device tree documentation for the wm8580 is altered to list
> the wm8581 codec support, as is the Kconfig file.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---
<snip>
> @@ -65,6 +68,8 @@
>  #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
>  #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
>  #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
> +#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
> +#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B

I would be tempted to call these WM8581_...

>  #define WM8580_MASTER_DIGITAL_ATTENUATION    0x1C
>  #define WM8580_ADC_CONTROL1                  0x1D
>  #define WM8580_SPDTXCHAN0                    0x1E
> @@ -242,6 +247,7 @@ struct wm8580_priv {
>  	struct regulator_bulk_data supplies[WM8580_NUM_SUPPLIES];
>  	struct pll_state a;
>  	struct pll_state b;
> +	const struct wm8580_driver_data *drvdata;
>  	int sysclk[2];
>  };
>  
> @@ -306,6 +312,19 @@ SOC_DOUBLE("Capture Switch", WM8580_ADC_CONTROL1, 0, 1, 1, 1),
>  SOC_SINGLE("Capture High-Pass Filter Switch", WM8580_ADC_CONTROL1, 4, 1, 0),
>  };
>  
> +static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
> +SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
> +		     WM8580_DIGITAL_ATTENUATION_DACL4,
> +		     WM8580_DIGITAL_ATTENUATION_DACR4,
> +		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
> +
> +SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
> +
> +SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
> +
> +SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
> +};

Ditto here and so on for the next couple.

> +
>  static const struct snd_soc_dapm_widget wm8580_dapm_widgets[] = {
>  SND_SOC_DAPM_DAC("DAC1", "Playback", WM8580_PWRDN1, 2, 1),
>  SND_SOC_DAPM_DAC("DAC2", "Playback", WM8580_PWRDN1, 3, 1),
> @@ -324,6 +343,13 @@ SND_SOC_DAPM_INPUT("AINL"),
>  SND_SOC_DAPM_INPUT("AINR"),
>  };
>  
> +static const struct snd_soc_dapm_widget wm8580_dac4_dapm_widgets[] = {
> +SND_SOC_DAPM_DAC("DAC4", "Playback", WM8580_PWRDN1, 5, 1),
> +
> +SND_SOC_DAPM_OUTPUT("VOUT4L"),
> +SND_SOC_DAPM_OUTPUT("VOUT4R"),
> +};
> +
>  static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
>  	{ "VOUT1L", NULL, "DAC1" },
>  	{ "VOUT1R", NULL, "DAC1" },
> @@ -338,6 +364,11 @@ static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
>  	{ "ADC", NULL, "AINR" },
>  };
>  
> +static const struct snd_soc_dapm_route wm8580_dac4_dapm_routes[] = {
> +	{ "VOUT4L", NULL, "DAC4" },
> +	{ "VOUT4R", NULL, "DAC4" },
> +};
> +
>  /* PLL divisors */
>  struct _pll_div {
>  	u32 prescale:1;
> @@ -837,19 +868,16 @@ static const struct snd_soc_dai_ops wm8580_dai_ops_capture = {
>  
>  static struct snd_soc_dai_driver wm8580_dai[] = {
>  	{
> -		.name = "wm8580-hifi-playback",
>  		.id	= WM8580_DAI_PAIFRX,
>  		.playback = {
>  			.stream_name = "Playback",
>  			.channels_min = 1,
> -			.channels_max = 6,
>  			.rates = SNDRV_PCM_RATE_8000_192000,
>  			.formats = WM8580_FORMATS,
>  		},
>  		.ops = &wm8580_dai_ops_playback,
>  	},
>  	{
> -		.name = "wm8580-hifi-capture",
>  		.id	=	WM8580_DAI_PAIFTX,
>  		.capture = {
>  			.stream_name = "Capture",
> @@ -865,8 +893,22 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
>  static int wm8580_probe(struct snd_soc_codec *codec)
>  {
>  	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
> +	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>  	int ret = 0;
>  
> +	switch (wm8580->drvdata->num_dacs) {
> +	case 4:
> +		snd_soc_add_codec_controls(codec, wm8580_dac4_snd_controls,
> +					ARRAY_SIZE(wm8580_dac4_snd_controls));
> +		snd_soc_dapm_new_controls(dapm, wm8580_dac4_dapm_widgets,
> +					ARRAY_SIZE(wm8580_dac4_dapm_widgets));
> +		snd_soc_dapm_add_routes(dapm, wm8580_dac4_dapm_routes,
> +					ARRAY_SIZE(wm8580_dac4_dapm_routes));
> +		break;
> +	default:
> +		break;
> +	}

Its slightly more normal to do this directly based on which part it was,
although I don't have too many problems with this approach if all
the other changes fit nicely into it.

> +
>  	ret = regulator_bulk_enable(ARRAY_SIZE(wm8580->supplies),
>  				    wm8580->supplies);
>  	if (ret != 0) {
> @@ -914,12 +956,6 @@ static const struct snd_soc_codec_driver soc_codec_dev_wm8580 = {
>  	},
>  };
>  
> -static const struct of_device_id wm8580_of_match[] = {
> -	{ .compatible = "wlf,wm8580" },
> -	{ },
> -};
> -MODULE_DEVICE_TABLE(of, wm8580_of_match);
> -
>  static const struct regmap_config wm8580_regmap = {
>  	.reg_bits = 7,
>  	.val_bits = 9,
> @@ -932,10 +968,32 @@ static const struct regmap_config wm8580_regmap = {
>  	.volatile_reg = wm8580_volatile,
>  };
>  
> +const struct wm8580_driver_data wm8580_data = {
> +	.name_playback = "wm8580-hifi-playback",
> +	.name_capture = "wm8580-hifi-capture",
> +	.num_dacs = 3,
> +};
> +EXPORT_SYMBOL_GPL(wm8580_data);
> +
> +const struct wm8580_driver_data wm8581_data = {
> +	.name_playback = "wm8581-hifi-playback",
> +	.name_capture = "wm8581-hifi-capture",
> +	.num_dacs = 4,
> +};
> +EXPORT_SYMBOL_GPL(wm8581_data);
> +
> +static const struct of_device_id wm8580_of_match[] = {
> +	{ .compatible = "wlf,wm8580", .data = &wm8580_data },
> +	{ .compatible = "wlf,wm8581", .data = &wm8581_data },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, wm8580_of_match);
> +
>  #if IS_ENABLED(CONFIG_I2C)
>  static int wm8580_i2c_probe(struct i2c_client *i2c,
>  			    const struct i2c_device_id *id)
>  {
> +	const struct of_device_id *of_id;
>  	struct wm8580_priv *wm8580;
>  	int ret, i;
>  
> @@ -960,6 +1018,20 @@ static int wm8580_i2c_probe(struct i2c_client *i2c,
>  
>  	i2c_set_clientdata(i2c, wm8580);
>  
> +	of_id = of_match_device(wm8580_of_match, &i2c->dev);
> +	if (of_id)
> +		wm8580->drvdata = of_id->data;
> +
> +	if (!wm8580->drvdata) {
> +		dev_err(&i2c->dev, "failed to find driver data\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Each dac supports stereo input */
> +	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
> +	wm8580_dai[0].name = wm8580->drvdata->name_playback;
> +	wm8580_dai[1].name = wm8580->drvdata->name_capture;

You can't patch the dai_driver struct like this its a static
struct that is shared between all instanciations of the driver it
is possible someone could put a 8580 and 8581 on the same board.

I guess you really have two options here, you could specify the
maximum in the dai_driver and then apply constraints in the
startup callback to limit things to the correct chip dependant
number of channels, or just have two copies of the dai_driver
struct one for each chip.

Thanks,
Charles

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

On 17/10/16 21:37, Charles Keepax wrote:
> On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
>> This patch adds support for the wm8581 codec to the wm8580 driver.
>> The wm8581 codec hardware adds a fourth DAC and otherwise is
>> compatible with the wm8580 codec.
>>
>> of_device_id data is used to allow the driver to select the
>> suitable software variables from the device tree codec selection.
>> The wm8580_driver_data struct is used to store the number of DACs
>> as well as the required stream names for both codecs.
>>
>> The snd_soc_dai_driver no longer lists the stream names and for
>> playback the channels_max. These variables are set during i2c
>> probe from the of_device_id supplied wm8580_driver_data struct.
>>
>> With knowledge of the number of DACs in use, the DAC4 controls,
>> widgets and routes are added as required for DAC4.
>>
>> The device tree documentation for the wm8580 is altered to list
>> the wm8581 codec support, as is the Kconfig file.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
> <snip>
>> @@ -65,6 +68,8 @@
>>   #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
>>   #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
>>   #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
>> +#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
>> +#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B
> I would be tempted to call these WM8581_...
Can do.
>
>>   #define WM8580_MASTER_DIGITAL_ATTENUATION    0x1C
>>   #define WM8580_ADC_CONTROL1                  0x1D
>>   #define WM8580_SPDTXCHAN0                    0x1E
>> @@ -242,6 +247,7 @@ struct wm8580_priv {
>>   	struct regulator_bulk_data supplies[WM8580_NUM_SUPPLIES];
>>   	struct pll_state a;
>>   	struct pll_state b;
>> +	const struct wm8580_driver_data *drvdata;
>>   	int sysclk[2];
>>   };
>>   
>> @@ -306,6 +312,19 @@ SOC_DOUBLE("Capture Switch", WM8580_ADC_CONTROL1, 0, 1, 1, 1),
>>   SOC_SINGLE("Capture High-Pass Filter Switch", WM8580_ADC_CONTROL1, 4, 1, 0),
>>   };
>>   
>> +static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
>> +SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
>> +		     WM8580_DIGITAL_ATTENUATION_DACL4,
>> +		     WM8580_DIGITAL_ATTENUATION_DACR4,
>> +		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
>> +
>> +SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
>> +
>> +SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
>> +
>> +SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
>> +};
> Ditto here and so on for the next couple.
That would mean replicating the WM8580_DAC_CONTROL[345] definitions where :
WM8581_DAC_CONTROL3==WM8580_DAC_CONTROL3
WM8581_DAC_CONTROL4==WM8580_DAC_CONTROL4
WM8581_DAC_CONTROL5==WM8580_DAC_CONTROL5
Is that what you are after ?

>> +
>>   static const struct snd_soc_dapm_widget wm8580_dapm_widgets[] = {
>>   SND_SOC_DAPM_DAC("DAC1", "Playback", WM8580_PWRDN1, 2, 1),
>>   SND_SOC_DAPM_DAC("DAC2", "Playback", WM8580_PWRDN1, 3, 1),
>> @@ -324,6 +343,13 @@ SND_SOC_DAPM_INPUT("AINL"),
>>   SND_SOC_DAPM_INPUT("AINR"),
>>   };
>>   
>> +static const struct snd_soc_dapm_widget wm8580_dac4_dapm_widgets[] = {
>> +SND_SOC_DAPM_DAC("DAC4", "Playback", WM8580_PWRDN1, 5, 1),
>> +
>> +SND_SOC_DAPM_OUTPUT("VOUT4L"),
>> +SND_SOC_DAPM_OUTPUT("VOUT4R"),
>> +};
>> +
>>   static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
>>   	{ "VOUT1L", NULL, "DAC1" },
>>   	{ "VOUT1R", NULL, "DAC1" },
>> @@ -338,6 +364,11 @@ static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
>>   	{ "ADC", NULL, "AINR" },
>>   };
>>   
>> +static const struct snd_soc_dapm_route wm8580_dac4_dapm_routes[] = {
>> +	{ "VOUT4L", NULL, "DAC4" },
>> +	{ "VOUT4R", NULL, "DAC4" },
>> +};
>> +
>>   /* PLL divisors */
>>   struct _pll_div {
>>   	u32 prescale:1;
>> @@ -837,19 +868,16 @@ static const struct snd_soc_dai_ops wm8580_dai_ops_capture = {
>>   
>>   static struct snd_soc_dai_driver wm8580_dai[] = {
>>   	{
>> -		.name = "wm8580-hifi-playback",
>>   		.id	= WM8580_DAI_PAIFRX,
>>   		.playback = {
>>   			.stream_name = "Playback",
>>   			.channels_min = 1,
>> -			.channels_max = 6,
>>   			.rates = SNDRV_PCM_RATE_8000_192000,
>>   			.formats = WM8580_FORMATS,
>>   		},
>>   		.ops = &wm8580_dai_ops_playback,
>>   	},
>>   	{
>> -		.name = "wm8580-hifi-capture",
>>   		.id	=	WM8580_DAI_PAIFTX,
>>   		.capture = {
>>   			.stream_name = "Capture",
>> @@ -865,8 +893,22 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
>>   static int wm8580_probe(struct snd_soc_codec *codec)
>>   {
>>   	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
>> +	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
>>   	int ret = 0;
>>   
>> +	switch (wm8580->drvdata->num_dacs) {
>> +	case 4:
>> +		snd_soc_add_codec_controls(codec, wm8580_dac4_snd_controls,
>> +					ARRAY_SIZE(wm8580_dac4_snd_controls));
>> +		snd_soc_dapm_new_controls(dapm, wm8580_dac4_dapm_widgets,
>> +					ARRAY_SIZE(wm8580_dac4_dapm_widgets));
>> +		snd_soc_dapm_add_routes(dapm, wm8580_dac4_dapm_routes,
>> +					ARRAY_SIZE(wm8580_dac4_dapm_routes));
>> +		break;
>> +	default:
>> +		break;
>> +	}
> Its slightly more normal to do this directly based on which part it was,
> although I don't have too many problems with this approach if all
> the other changes fit nicely into it.
>
>> +
>>   	ret = regulator_bulk_enable(ARRAY_SIZE(wm8580->supplies),
>>   				    wm8580->supplies);
>>   	if (ret != 0) {
>> @@ -914,12 +956,6 @@ static const struct snd_soc_codec_driver soc_codec_dev_wm8580 = {
>>   	},
>>   };
>>   
>> -static const struct of_device_id wm8580_of_match[] = {
>> -	{ .compatible = "wlf,wm8580" },
>> -	{ },
>> -};
>> -MODULE_DEVICE_TABLE(of, wm8580_of_match);
>> -
>>   static const struct regmap_config wm8580_regmap = {
>>   	.reg_bits = 7,
>>   	.val_bits = 9,
>> @@ -932,10 +968,32 @@ static const struct regmap_config wm8580_regmap = {
>>   	.volatile_reg = wm8580_volatile,
>>   };
>>   
>> +const struct wm8580_driver_data wm8580_data = {
>> +	.name_playback = "wm8580-hifi-playback",
>> +	.name_capture = "wm8580-hifi-capture",
>> +	.num_dacs = 3,
>> +};
>> +EXPORT_SYMBOL_GPL(wm8580_data);
>> +
>> +const struct wm8580_driver_data wm8581_data = {
>> +	.name_playback = "wm8581-hifi-playback",
>> +	.name_capture = "wm8581-hifi-capture",
>> +	.num_dacs = 4,
>> +};
>> +EXPORT_SYMBOL_GPL(wm8581_data);
>> +
>> +static const struct of_device_id wm8580_of_match[] = {
>> +	{ .compatible = "wlf,wm8580", .data = &wm8580_data },
>> +	{ .compatible = "wlf,wm8581", .data = &wm8581_data },
>> +	{ },
>> +};
>> +MODULE_DEVICE_TABLE(of, wm8580_of_match);
>> +
>>   #if IS_ENABLED(CONFIG_I2C)
>>   static int wm8580_i2c_probe(struct i2c_client *i2c,
>>   			    const struct i2c_device_id *id)
>>   {
>> +	const struct of_device_id *of_id;
>>   	struct wm8580_priv *wm8580;
>>   	int ret, i;
>>   
>> @@ -960,6 +1018,20 @@ static int wm8580_i2c_probe(struct i2c_client *i2c,
>>   
>>   	i2c_set_clientdata(i2c, wm8580);
>>   
>> +	of_id = of_match_device(wm8580_of_match, &i2c->dev);
>> +	if (of_id)
>> +		wm8580->drvdata = of_id->data;
>> +
>> +	if (!wm8580->drvdata) {
>> +		dev_err(&i2c->dev, "failed to find driver data\n");
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Each dac supports stereo input */
>> +	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
>> +	wm8580_dai[0].name = wm8580->drvdata->name_playback;
>> +	wm8580_dai[1].name = wm8580->drvdata->name_capture;
> You can't patch the dai_driver struct like this its a static
> struct that is shared between all instanciations of the driver it
> is possible someone could put a 8580 and 8581 on the same board.
Oh  - That is a good point. Strangely, the cs42xx8 driver has the same 
problem ! Hmmm ...
> I guess you really have two options here, you could specify the
> maximum in the dai_driver and then apply constraints in the
> startup callback to limit things to the correct chip dependant
> number of channels, or just have two copies of the dai_driver
> struct one for each chip.
I didn't understand your first suggestion, can you give an example of 
how to do that in the startup callback ? Sounds like a good idea.

> Thanks,
> Charles

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Charles Keepax]

On Mon, Oct 17, 2016 at 09:46:20PM +1100, Matt Flax wrote:
> 
> 
> On 17/10/16 21:37, Charles Keepax wrote:
> >On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
> >>This patch adds support for the wm8581 codec to the wm8580 driver.
> >>The wm8581 codec hardware adds a fourth DAC and otherwise is
> >>compatible with the wm8580 codec.
> >>
> >>of_device_id data is used to allow the driver to select the
> >>suitable software variables from the device tree codec selection.
> >>The wm8580_driver_data struct is used to store the number of DACs
> >>as well as the required stream names for both codecs.
> >>
> >>The snd_soc_dai_driver no longer lists the stream names and for
> >>playback the channels_max. These variables are set during i2c
> >>probe from the of_device_id supplied wm8580_driver_data struct.
> >>
> >>With knowledge of the number of DACs in use, the DAC4 controls,
> >>widgets and routes are added as required for DAC4.
> >>
> >>The device tree documentation for the wm8580 is altered to list
> >>the wm8581 codec support, as is the Kconfig file.
> >>
> >>Signed-off-by: Matt Flax <flatmax@flatmax.org>
> >>---
<snip>
> >>+static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
> >>+SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
> >>+		     WM8580_DIGITAL_ATTENUATION_DACL4,
> >>+		     WM8580_DIGITAL_ATTENUATION_DACR4,
> >>+		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
> >>+
> >>+SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
> >>+
> >>+SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
> >>+
> >>+SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
> >>+};
> >Ditto here and so on for the next couple.
> That would mean replicating the WM8580_DAC_CONTROL[345] definitions where :
> WM8581_DAC_CONTROL3==WM8580_DAC_CONTROL3
> WM8581_DAC_CONTROL4==WM8580_DAC_CONTROL4
> WM8581_DAC_CONTROL5==WM8580_DAC_CONTROL5
> Is that what you are after ?

Ah apologies that wasn't what I meant if the register already
exists then don't change the name. I was referring to
wm8580_dac4_snd_controls I would probably name that
wm8581_dac4_snd_controls.

> >>+	/* Each dac supports stereo input */
> >>+	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
> >>+	wm8580_dai[0].name = wm8580->drvdata->name_playback;
> >>+	wm8580_dai[1].name = wm8580->drvdata->name_capture;
> >You can't patch the dai_driver struct like this its a static
> >struct that is shared between all instanciations of the driver it
> >is possible someone could put a 8580 and 8581 on the same board.
> Oh  - That is a good point. Strangely, the cs42xx8 driver has the same
> problem ! Hmmm ...

Yeah someone should probably look at fixing that at some point,
I will try to find some time to have a gander.

> >I guess you really have two options here, you could specify the
> >maximum in the dai_driver and then apply constraints in the
> >startup callback to limit things to the correct chip dependant
> >number of channels, or just have two copies of the dai_driver
> >struct one for each chip.
> I didn't understand your first suggestion, can you give an example of how to
> do that in the startup callback ? Sounds like a good idea.

If you look in arizona_startup in sound/soc/codecs/arizona.c we
specify constraints for the sample rates based off our clocking,
there is an equivalent SNDRV_PCM_HW_PARAM_CHANNELS that you
should be able to you to specify the supported channels with.

Thanks,
Charles

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

On 17/10/16 21:59, Charles Keepax wrote:
> On Mon, Oct 17, 2016 at 09:46:20PM +1100, Matt Flax wrote:
>>
>> On 17/10/16 21:37, Charles Keepax wrote:
>>> On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
>>>> This patch adds support for the wm8581 codec to the wm8580 driver.
>>>> The wm8581 codec hardware adds a fourth DAC and otherwise is
>>>> compatible with the wm8580 codec.
>>>>
>>>> of_device_id data is used to allow the driver to select the
>>>> suitable software variables from the device tree codec selection.
>>>> The wm8580_driver_data struct is used to store the number of DACs
>>>> as well as the required stream names for both codecs.
>>>>
>>>> The snd_soc_dai_driver no longer lists the stream names and for
>>>> playback the channels_max. These variables are set during i2c
>>>> probe from the of_device_id supplied wm8580_driver_data struct.
>>>>
>>>> With knowledge of the number of DACs in use, the DAC4 controls,
>>>> widgets and routes are added as required for DAC4.
>>>>
>>>> The device tree documentation for the wm8580 is altered to list
>>>> the wm8581 codec support, as is the Kconfig file.
>>>>
>>>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>>>> ---
> <snip>
>>>> +static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
>>>> +SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
>>>> +		     WM8580_DIGITAL_ATTENUATION_DACL4,
>>>> +		     WM8580_DIGITAL_ATTENUATION_DACR4,
>>>> +		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
>>>> +
>>>> +SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
>>>> +
>>>> +SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
>>>> +
>>>> +SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
>>>> +};
>>> Ditto here and so on for the next couple.
>> That would mean replicating the WM8580_DAC_CONTROL[345] definitions where :
>> WM8581_DAC_CONTROL3==WM8580_DAC_CONTROL3
>> WM8581_DAC_CONTROL4==WM8580_DAC_CONTROL4
>> WM8581_DAC_CONTROL5==WM8580_DAC_CONTROL5
>> Is that what you are after ?
> Ah apologies that wasn't what I meant if the register already
> exists then don't change the name. I was referring to
> wm8580_dac4_snd_controls I would probably name that
> wm8581_dac4_snd_controls.
>
>>>> +	/* Each dac supports stereo input */
>>>> +	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
>>>> +	wm8580_dai[0].name = wm8580->drvdata->name_playback;
>>>> +	wm8580_dai[1].name = wm8580->drvdata->name_capture;
>>> You can't patch the dai_driver struct like this its a static
>>> struct that is shared between all instanciations of the driver it
>>> is possible someone could put a 8580 and 8581 on the same board.
>> Oh  - That is a good point. Strangely, the cs42xx8 driver has the same
>> problem ! Hmmm ...
> Yeah someone should probably look at fixing that at some point,
> I will try to find some time to have a gander.
>
>>> I guess you really have two options here, you could specify the
>>> maximum in the dai_driver and then apply constraints in the
>>> startup callback to limit things to the correct chip dependant
>>> number of channels, or just have two copies of the dai_driver
>>> struct one for each chip.
>> I didn't understand your first suggestion, can you give an example of how to
>> do that in the startup callback ? Sounds like a good idea.
> If you look in arizona_startup in sound/soc/codecs/arizona.c we
> specify constraints for the sample rates based off our clocking,
> there is an equivalent SNDRV_PCM_HW_PARAM_CHANNELS that you
> should be able to you to specify the supported channels with.
OK - started on this ... however I get the following problem when trying 
to set the stream name :
[    6.331002] wm8580 1-001a: ASoC: error - multiple DAI 1-001a 
registered with no name

I think this is happening in i2c probe. I was wanting to set the stream 
name dynamically to match the codec model number.
I was planning to do the following :
static int wm8580_playback_startup(struct snd_pcm_substream *substream,
                struct snd_soc_dai *dai)
{
     struct snd_soc_codec *codec = dai->codec;
     struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);

     strncpy(substream->name, wm8580->drvdata->name_playback,
sizeof(wm8580->drvdata->name_playback) - 1);

     return snd_pcm_hw_constraint_single(substream->runtime,
         SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);
}

Do you think perhaps I should set a new name in the snd_soc_dai_driver 
to the following :
     .name = "wm858x-hifi-playback",
     .name = "wm858x-hifi-capture",


> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

On 17/10/16 21:59, Charles Keepax wrote:
> On Mon, Oct 17, 2016 at 09:46:20PM +1100, Matt Flax wrote:
>>
>> On 17/10/16 21:37, Charles Keepax wrote:
>>> On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
>>>> This patch adds support for the wm8581 codec to the wm8580 driver.
>>>> The wm8581 codec hardware adds a fourth DAC and otherwise is
>>>> compatible with the wm8580 codec.
>>>>
>>>> of_device_id data is used to allow the driver to select the
>>>> suitable software variables from the device tree codec selection.
>>>> The wm8580_driver_data struct is used to store the number of DACs
>>>> as well as the required stream names for both codecs.
>>>>
>>>> The snd_soc_dai_driver no longer lists the stream names and for
>>>> playback the channels_max. These variables are set during i2c
>>>> probe from the of_device_id supplied wm8580_driver_data struct.
>>>>
>>>> With knowledge of the number of DACs in use, the DAC4 controls,
>>>> widgets and routes are added as required for DAC4.
>>>>
>>>> The device tree documentation for the wm8580 is altered to list
>>>> the wm8581 codec support, as is the Kconfig file.
>>>>
>>>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>>>> ---
> <snip>
>>>> +static const struct snd_kcontrol_new wm8580_dac4_snd_controls[] = {
>>>> +SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
>>>> +		     WM8580_DIGITAL_ATTENUATION_DACL4,
>>>> +		     WM8580_DIGITAL_ATTENUATION_DACR4,
>>>> +		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
>>>> +
>>>> +SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
>>>> +
>>>> +SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
>>>> +
>>>> +SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
>>>> +};
>>> Ditto here and so on for the next couple.
>> That would mean replicating the WM8580_DAC_CONTROL[345] definitions where :
>> WM8581_DAC_CONTROL3==WM8580_DAC_CONTROL3
>> WM8581_DAC_CONTROL4==WM8580_DAC_CONTROL4
>> WM8581_DAC_CONTROL5==WM8580_DAC_CONTROL5
>> Is that what you are after ?
> Ah apologies that wasn't what I meant if the register already
> exists then don't change the name. I was referring to
> wm8580_dac4_snd_controls I would probably name that
> wm8581_dac4_snd_controls.
>
>>>> +	/* Each dac supports stereo input */
>>>> +	wm8580_dai[0].playback.channels_max = wm8580->drvdata->num_dacs * 2;
>>>> +	wm8580_dai[0].name = wm8580->drvdata->name_playback;
>>>> +	wm8580_dai[1].name = wm8580->drvdata->name_capture;
>>> You can't patch the dai_driver struct like this its a static
>>> struct that is shared between all instanciations of the driver it
>>> is possible someone could put a 8580 and 8581 on the same board.
>> Oh  - That is a good point. Strangely, the cs42xx8 driver has the same
>> problem ! Hmmm ...
> Yeah someone should probably look at fixing that at some point,
> I will try to find some time to have a gander.
>
>>> I guess you really have two options here, you could specify the
>>> maximum in the dai_driver and then apply constraints in the
>>> startup callback to limit things to the correct chip dependant
>>> number of channels, or just have two copies of the dai_driver
>>> struct one for each chip.
>> I didn't understand your first suggestion, can you give an example of how to
>> do that in the startup callback ? Sounds like a good idea.
> If you look in arizona_startup in sound/soc/codecs/arizona.c we
> specify constraints for the sample rates based off our clocking,
> there is an equivalent SNDRV_PCM_HW_PARAM_CHANNELS that you
> should be able to you to specify the supported channels with.
OK - started on this ... however I get the following problem when trying 
to set the stream name :
[    6.331002] wm8580 1-001a: ASoC: error - multiple DAI 1-001a 
registered with no name

I think this is happening in i2c probe. I was wanting to set the stream 
name dynamically to match the codec model number.
I was planning to do the following :
static int wm8580_playback_startup(struct snd_pcm_substream *substream,
                struct snd_soc_dai *dai)
{
     struct snd_soc_codec *codec = dai->codec;
     struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);

     strncpy(substream->name, wm8580->drvdata->name_playback,
sizeof(wm8580->drvdata->name_playback) - 1);

     return snd_pcm_hw_constraint_single(substream->runtime,
         SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);
}

Do you think perhaps I should set a new name in the snd_soc_dai_driver 
to the following :
     .name = "wm858x-hifi-playback",
     .name = "wm858x-hifi-capture",


> Thanks,
> Charles
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Charles Keepax]

On Tue, Oct 18, 2016 at 06:15:28AM +1100, Matt Flax wrote:
> 
> 
> On 17/10/16 21:59, Charles Keepax wrote:
> >On Mon, Oct 17, 2016 at 09:46:20PM +1100, Matt Flax wrote:
> >>
> >>On 17/10/16 21:37, Charles Keepax wrote:
> >>>On Mon, Oct 17, 2016 at 09:42:18AM +1100, Matt Flax wrote:
<snip>
> >>I didn't understand your first suggestion, can you give an example of how to
> >>do that in the startup callback ? Sounds like a good idea.
> >If you look in arizona_startup in sound/soc/codecs/arizona.c we
> >specify constraints for the sample rates based off our clocking,
> >there is an equivalent SNDRV_PCM_HW_PARAM_CHANNELS that you
> >should be able to you to specify the supported channels with.
> OK - started on this ... however I get the following problem when trying to
> set the stream name :
> [    6.331002] wm8580 1-001a: ASoC: error - multiple DAI 1-001a registered
> with no name
> 
> I think this is happening in i2c probe. I was wanting to set the stream name
> dynamically to match the codec model number.
> I was planning to do the following :
> static int wm8580_playback_startup(struct snd_pcm_substream *substream,
>                struct snd_soc_dai *dai)
> {
>     struct snd_soc_codec *codec = dai->codec;
>     struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
> 
>     strncpy(substream->name, wm8580->drvdata->name_playback,
> sizeof(wm8580->drvdata->name_playback) - 1);
> 
>     return snd_pcm_hw_constraint_single(substream->runtime,
>         SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);
> }
> 
> Do you think perhaps I should set a new name in the snd_soc_dai_driver to
> the following :
>     .name = "wm858x-hifi-playback",
>     .name = "wm858x-hifi-capture",

I don't think you need to bother setting the DAI name to match
the CODEC, I am happy with a generic name.

Thanks,
Charles

[PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

This patch adds support for the wm8581 codec to the wm8580 driver.
The wm8581 codec hardware adds a fourth DAC and otherwise is
compatible with the wm8580 codec.

of_device_id data is used to allow the driver to select the
suitable DAC count specified in the device tree codec selection.
The wm8580_driver_data struct is used to store the number of DACs.

The snd_soc_dai_driver no longer lists the channels_max for the
playback substream. This variable is set during the i2c probe
from the of_device_id supplied wm8580_driver_data struct.

With knowledge of the number of DACs in use, the DAC4 controls,
widgets and routes are added as required for DAC4.

The device tree documentation for the wm8580 is altered to list
the wm8581 codec support, as is the Kconfig file.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 Documentation/devicetree/bindings/sound/wm8580.txt |  4 +-
 sound/soc/codecs/Kconfig                           |  2 +-
 sound/soc/codecs/wm8580.c                          | 98 ++++++++++++++++++++--
 3 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/wm8580.txt b/Documentation/devicetree/bindings/sound/wm8580.txt
index 7d9821f..78fce9b 100644
--- a/Documentation/devicetree/bindings/sound/wm8580.txt
+++ b/Documentation/devicetree/bindings/sound/wm8580.txt
@@ -1,10 +1,10 @@
-WM8580 audio CODEC
+WM8580 and WM8581 audio CODEC
 
 This device supports I2C only.
 
 Required properties:
 
-  - compatible : "wlf,wm8580"
+  - compatible : "wlf,wm8580", "wlf,wm8581"
 
   - reg : the I2C address of the device.
 
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c67667b..199bec8 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -914,7 +914,7 @@ config SND_SOC_WM8523
 	depends on I2C
 
 config SND_SOC_WM8580
-	tristate "Wolfson Microelectronics WM8523 CODEC"
+	tristate "Wolfson Microelectronics WM8580 and WM8581 CODECs"
 	depends on I2C
 
 config SND_SOC_WM8711
diff --git a/sound/soc/codecs/wm8580.c b/sound/soc/codecs/wm8580.c
index faa7287..d57dceb 100644
--- a/sound/soc/codecs/wm8580.c
+++ b/sound/soc/codecs/wm8580.c
@@ -1,5 +1,5 @@
 /*
- * wm8580.c  --  WM8580 ALSA Soc Audio driver
+ * wm8580.c  --  WM8580 and WM8581 ALSA Soc Audio driver
  *
  * Copyright 2008-12 Wolfson Microelectronics PLC.
  *
@@ -12,6 +12,9 @@
  *  The WM8580 is a multichannel codec with S/PDIF support, featuring six
  *  DAC channels and two ADC channels.
  *
+ *  The WM8581 is a multichannel codec with S/PDIF support, featuring eight
+ *  DAC channels and two ADC channels.
+ *
  *  Currently only the primary audio interface is supported - S/PDIF and
  *  the secondary audio interfaces are not.
  */
@@ -65,6 +68,8 @@
 #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
 #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
 #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
+#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
+#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B
 #define WM8580_MASTER_DIGITAL_ATTENUATION    0x1C
 #define WM8580_ADC_CONTROL1                  0x1D
 #define WM8580_SPDTXCHAN0                    0x1E
@@ -236,12 +241,17 @@ static const char *wm8580_supply_names[WM8580_NUM_SUPPLIES] = {
 	"PVDD",
 };
 
+struct wm8580_driver_data {
+	int num_dacs;
+};
+
 /* codec private data */
 struct wm8580_priv {
 	struct regmap *regmap;
 	struct regulator_bulk_data supplies[WM8580_NUM_SUPPLIES];
 	struct pll_state a;
 	struct pll_state b;
+	const struct wm8580_driver_data *drvdata;
 	int sysclk[2];
 };
 
@@ -306,6 +316,19 @@ SOC_DOUBLE("Capture Switch", WM8580_ADC_CONTROL1, 0, 1, 1, 1),
 SOC_SINGLE("Capture High-Pass Filter Switch", WM8580_ADC_CONTROL1, 4, 1, 0),
 };
 
+static const struct snd_kcontrol_new wm8581_snd_controls[] = {
+SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
+		     WM8580_DIGITAL_ATTENUATION_DACL4,
+		     WM8580_DIGITAL_ATTENUATION_DACR4,
+		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
+
+SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
+
+SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
+
+SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
+};
+
 static const struct snd_soc_dapm_widget wm8580_dapm_widgets[] = {
 SND_SOC_DAPM_DAC("DAC1", "Playback", WM8580_PWRDN1, 2, 1),
 SND_SOC_DAPM_DAC("DAC2", "Playback", WM8580_PWRDN1, 3, 1),
@@ -324,6 +347,13 @@ SND_SOC_DAPM_INPUT("AINL"),
 SND_SOC_DAPM_INPUT("AINR"),
 };
 
+static const struct snd_soc_dapm_widget wm8581_dapm_widgets[] = {
+SND_SOC_DAPM_DAC("DAC4", "Playback", WM8580_PWRDN1, 5, 1),
+
+SND_SOC_DAPM_OUTPUT("VOUT4L"),
+SND_SOC_DAPM_OUTPUT("VOUT4R"),
+};
+
 static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "VOUT1L", NULL, "DAC1" },
 	{ "VOUT1R", NULL, "DAC1" },
@@ -338,6 +368,11 @@ static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "ADC", NULL, "AINR" },
 };
 
+static const struct snd_soc_dapm_route wm8581_dapm_routes[] = {
+	{ "VOUT4L", NULL, "DAC4" },
+	{ "VOUT4R", NULL, "DAC4" },
+};
+
 /* PLL divisors */
 struct _pll_div {
 	u32 prescale:1;
@@ -815,10 +850,21 @@ static int wm8580_set_bias_level(struct snd_soc_codec *codec,
 	return 0;
 }
 
+static int wm8580_playback_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
+
+	return snd_pcm_hw_constraint_single(substream->runtime,
+		SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);
+}
+
 #define WM8580_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
 
 static const struct snd_soc_dai_ops wm8580_dai_ops_playback = {
+	.startup	= wm8580_playback_startup,
 	.set_sysclk	= wm8580_set_sysclk,
 	.hw_params	= wm8580_paif_hw_params,
 	.set_fmt	= wm8580_set_paif_dai_fmt,
@@ -842,7 +888,6 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
 		.playback = {
 			.stream_name = "Playback",
 			.channels_min = 1,
-			.channels_max = 6,
 			.rates = SNDRV_PCM_RATE_8000_192000,
 			.formats = WM8580_FORMATS,
 		},
@@ -865,8 +910,22 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
 static int wm8580_probe(struct snd_soc_codec *codec)
 {
 	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
+	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
 	int ret = 0;
 
+	switch (wm8580->drvdata->num_dacs) {
+	case 4:
+		snd_soc_add_codec_controls(codec, wm8581_snd_controls,
+					ARRAY_SIZE(wm8581_snd_controls));
+		snd_soc_dapm_new_controls(dapm, wm8581_dapm_widgets,
+					ARRAY_SIZE(wm8581_dapm_widgets));
+		snd_soc_dapm_add_routes(dapm, wm8581_dapm_routes,
+					ARRAY_SIZE(wm8581_dapm_routes));
+		break;
+	default:
+		break;
+	}
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(wm8580->supplies),
 				    wm8580->supplies);
 	if (ret != 0) {
@@ -914,12 +973,6 @@ static const struct snd_soc_codec_driver soc_codec_dev_wm8580 = {
 	},
 };
 
-static const struct of_device_id wm8580_of_match[] = {
-	{ .compatible = "wlf,wm8580" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, wm8580_of_match);
-
 static const struct regmap_config wm8580_regmap = {
 	.reg_bits = 7,
 	.val_bits = 9,
@@ -932,10 +985,26 @@ static const struct regmap_config wm8580_regmap = {
 	.volatile_reg = wm8580_volatile,
 };
 
+const struct wm8580_driver_data wm8580_data = {
+	.num_dacs = 3,
+};
+
+const struct wm8580_driver_data wm8581_data = {
+	.num_dacs = 4,
+};
+
+static const struct of_device_id wm8580_of_match[] = {
+	{ .compatible = "wlf,wm8580", .data = &wm8580_data },
+	{ .compatible = "wlf,wm8581", .data = &wm8581_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, wm8580_of_match);
+
 #if IS_ENABLED(CONFIG_I2C)
 static int wm8580_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
+	const struct of_device_id *of_id;
 	struct wm8580_priv *wm8580;
 	int ret, i;
 
@@ -960,6 +1029,15 @@ static int wm8580_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, wm8580);
 
+	of_id = of_match_device(wm8580_of_match, &i2c->dev);
+	if (of_id)
+		wm8580->drvdata = of_id->data;
+
+	if (!wm8580->drvdata) {
+		dev_err(&i2c->dev, "failed to find driver data\n");
+		return -EINVAL;
+	}
+
 	ret =  snd_soc_register_codec(&i2c->dev,
 			&soc_codec_dev_wm8580, wm8580_dai, ARRAY_SIZE(wm8580_dai));
 
@@ -973,7 +1051,8 @@ static int wm8580_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id wm8580_i2c_id[] = {
-	{ "wm8580", 0 },
+	{ "wm8580", (kernel_ulong_t)&wm8580_data },
+	{ "wm8581", (kernel_ulong_t)&wm8581_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, wm8580_i2c_id);
@@ -1014,4 +1093,5 @@ module_exit(wm8580_exit);
 
 MODULE_DESCRIPTION("ASoC WM8580 driver");
 MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_AUTHOR("Matt Flax <flatmax@flatmax.org>");
 MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Charles Keepax]

On Wed, Oct 19, 2016 at 05:24:35AM +1100, Matt Flax wrote:
> This patch adds support for the wm8581 codec to the wm8580 driver.
> The wm8581 codec hardware adds a fourth DAC and otherwise is
> compatible with the wm8580 codec.
> 
> of_device_id data is used to allow the driver to select the
> suitable DAC count specified in the device tree codec selection.
> The wm8580_driver_data struct is used to store the number of DACs.
> 
> The snd_soc_dai_driver no longer lists the channels_max for the
> playback substream. This variable is set during the i2c probe
> from the of_device_id supplied wm8580_driver_data struct.
> 
> With knowledge of the number of DACs in use, the DAC4 controls,
> widgets and routes are added as required for DAC4.
> 
> The device tree documentation for the wm8580 is altered to list
> the wm8581 codec support, as is the Kconfig file.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

When doing a respin of a patch its usually considered good
practice to do something like [PATCH v2] in the subject line,
lets people clearly see that its a new version not a resend.

>  Documentation/devicetree/bindings/sound/wm8580.txt |  4 +-
>  sound/soc/codecs/Kconfig                           |  2 +-
>  sound/soc/codecs/wm8580.c                          | 98 ++++++++++++++++++++--
>  3 files changed, 92 insertions(+), 12 deletions(-)
<snip>
> @@ -65,6 +68,8 @@
>  #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
>  #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
>  #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
> +#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
> +#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B

Apologies my fault for still not being clear these two new
defines would still be best called WM8581...

<snip>
> +static int wm8580_playback_startup(struct snd_pcm_substream *substream,
> +			   struct snd_soc_dai *dai)
> +{
> +	struct snd_soc_codec *codec = dai->codec;
> +	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
> +
> +	return snd_pcm_hw_constraint_single(substream->runtime,
> +		SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);

Won't this limit us to exactly num_dacs * 2 channels? I would
have expected a contraint_minmax, I haven't checked the core
code but would be good to know you have tested smaller numbers
of channels and they are working. Bearing in mind that some
user-space applications will do things like pad out to the number
of channels the hardware claims to require. For example for aplay
I believe -v will show you if it does any conversion and
--disable-channels will stop it attempting to convert the number
of channels.

Apart from those very small comments this all looks fine to me.

Thanks,
Charles

Re: [PATCH] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

On 19/10/16 20:22, Charles Keepax wrote:
> On Wed, Oct 19, 2016 at 05:24:35AM +1100, Matt Flax wrote:
>> This patch adds support for the wm8581 codec to the wm8580 driver.
>> The wm8581 codec hardware adds a fourth DAC and otherwise is
>> compatible with the wm8580 codec.
>>
>> of_device_id data is used to allow the driver to select the
>> suitable DAC count specified in the device tree codec selection.
>> The wm8580_driver_data struct is used to store the number of DACs.
>>
>> The snd_soc_dai_driver no longer lists the channels_max for the
>> playback substream. This variable is set during the i2c probe
>> from the of_device_id supplied wm8580_driver_data struct.
>>
>> With knowledge of the number of DACs in use, the DAC4 controls,
>> widgets and routes are added as required for DAC4.
>>
>> The device tree documentation for the wm8580 is altered to list
>> the wm8581 codec support, as is the Kconfig file.
>>
>> Signed-off-by: Matt Flax <flatmax@flatmax.org>
>> ---
> When doing a respin of a patch its usually considered good
> practice to do something like [PATCH v2] in the subject line,
> lets people clearly see that its a new version not a resend.
OK - I see that I should use in-reply-to with the message id next time.
>>   Documentation/devicetree/bindings/sound/wm8580.txt |  4 +-
>>   sound/soc/codecs/Kconfig                           |  2 +-
>>   sound/soc/codecs/wm8580.c                          | 98 ++++++++++++++++++++--
>>   3 files changed, 92 insertions(+), 12 deletions(-)
> <snip>
>> @@ -65,6 +68,8 @@
>>   #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
>>   #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
>>   #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
>> +#define WM8580_DIGITAL_ATTENUATION_DACL4     0x1A
>> +#define WM8580_DIGITAL_ATTENUATION_DACR4     0x1B
> Apologies my fault for still not being clear these two new
> defines would still be best called WM8581...
>
> <snip>
>> +static int wm8580_playback_startup(struct snd_pcm_substream *substream,
>> +			   struct snd_soc_dai *dai)
>> +{
>> +	struct snd_soc_codec *codec = dai->codec;
>> +	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
>> +
>> +	return snd_pcm_hw_constraint_single(substream->runtime,
>> +		SNDRV_PCM_HW_PARAM_CHANNELS, wm8580->drvdata->num_dacs * 2);
> Won't this limit us to exactly num_dacs * 2 channels? I would
> have expected a contraint_minmax, I haven't checked the core
> code but would be good to know you have tested smaller numbers
> of channels and they are working. Bearing in mind that some
> user-space applications will do things like pad out to the number
> of channels the hardware claims to require. For example for aplay
> I believe -v will show you if it does any conversion and
> --disable-channels will stop it attempting to convert the number
> of channels.
> Apart from those very small comments this all looks fine to me.
OK, I will use minmax to fix this problem ... I didn't see it at first, 
thanks for pointing it out.

Matt
>
> Thanks,
> Charles

[PATCH v3] ASoc: wm8580: Add the wm8581 codec to the driver

[Matt Flax]

This patch adds support for the wm8581 codec to the wm8580 driver.
The wm8581 codec hardware adds a fourth DAC and otherwise is
compatible with the wm8580 codec.

of_device_id data is used to allow the driver to select the
suitable DAC count specified in the device tree codec selection.
The wm8580_driver_data struct is used to store the number of DACs.

The snd_soc_dai_driver no longer lists the channels_max for the
playback substream. This variable is set during the i2c probe
from the of_device_id supplied wm8580_driver_data struct.

With knowledge of the number of DACs in use, the DAC4 controls,
widgets and routes are added as required for DAC4.

The device tree documentation for the wm8580 is altered to list
the wm8581 codec support, as is the Kconfig file.

Signed-off-by: Matt Flax <flatmax@flatmax.org>
---
 Documentation/devicetree/bindings/sound/wm8580.txt |  4 +-
 sound/soc/codecs/Kconfig                           |  2 +-
 sound/soc/codecs/wm8580.c                          | 98 ++++++++++++++++++++--
 3 files changed, 92 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/sound/wm8580.txt b/Documentation/devicetree/bindings/sound/wm8580.txt
index 7d9821f..78fce9b 100644
--- a/Documentation/devicetree/bindings/sound/wm8580.txt
+++ b/Documentation/devicetree/bindings/sound/wm8580.txt
@@ -1,10 +1,10 @@
-WM8580 audio CODEC
+WM8580 and WM8581 audio CODEC
 
 This device supports I2C only.
 
 Required properties:
 
-  - compatible : "wlf,wm8580"
+  - compatible : "wlf,wm8580", "wlf,wm8581"
 
   - reg : the I2C address of the device.
 
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c67667b..199bec8 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -914,7 +914,7 @@ config SND_SOC_WM8523
 	depends on I2C
 
 config SND_SOC_WM8580
-	tristate "Wolfson Microelectronics WM8523 CODEC"
+	tristate "Wolfson Microelectronics WM8580 and WM8581 CODECs"
 	depends on I2C
 
 config SND_SOC_WM8711
diff --git a/sound/soc/codecs/wm8580.c b/sound/soc/codecs/wm8580.c
index faa7287..aecd3c9 100644
--- a/sound/soc/codecs/wm8580.c
+++ b/sound/soc/codecs/wm8580.c
@@ -1,5 +1,5 @@
 /*
- * wm8580.c  --  WM8580 ALSA Soc Audio driver
+ * wm8580.c  --  WM8580 and WM8581 ALSA Soc Audio driver
  *
  * Copyright 2008-12 Wolfson Microelectronics PLC.
  *
@@ -12,6 +12,9 @@
  *  The WM8580 is a multichannel codec with S/PDIF support, featuring six
  *  DAC channels and two ADC channels.
  *
+ *  The WM8581 is a multichannel codec with S/PDIF support, featuring eight
+ *  DAC channels and two ADC channels.
+ *
  *  Currently only the primary audio interface is supported - S/PDIF and
  *  the secondary audio interfaces are not.
  */
@@ -65,6 +68,8 @@
 #define WM8580_DIGITAL_ATTENUATION_DACR2     0x17
 #define WM8580_DIGITAL_ATTENUATION_DACL3     0x18
 #define WM8580_DIGITAL_ATTENUATION_DACR3     0x19
+#define WM8581_DIGITAL_ATTENUATION_DACL4     0x1A
+#define WM8581_DIGITAL_ATTENUATION_DACR4     0x1B
 #define WM8580_MASTER_DIGITAL_ATTENUATION    0x1C
 #define WM8580_ADC_CONTROL1                  0x1D
 #define WM8580_SPDTXCHAN0                    0x1E
@@ -236,12 +241,17 @@ static const char *wm8580_supply_names[WM8580_NUM_SUPPLIES] = {
 	"PVDD",
 };
 
+struct wm8580_driver_data {
+	int num_dacs;
+};
+
 /* codec private data */
 struct wm8580_priv {
 	struct regmap *regmap;
 	struct regulator_bulk_data supplies[WM8580_NUM_SUPPLIES];
 	struct pll_state a;
 	struct pll_state b;
+	const struct wm8580_driver_data *drvdata;
 	int sysclk[2];
 };
 
@@ -306,6 +316,19 @@ SOC_DOUBLE("Capture Switch", WM8580_ADC_CONTROL1, 0, 1, 1, 1),
 SOC_SINGLE("Capture High-Pass Filter Switch", WM8580_ADC_CONTROL1, 4, 1, 0),
 };
 
+static const struct snd_kcontrol_new wm8581_snd_controls[] = {
+SOC_DOUBLE_R_EXT_TLV("DAC4 Playback Volume",
+		     WM8581_DIGITAL_ATTENUATION_DACL4,
+		     WM8581_DIGITAL_ATTENUATION_DACR4,
+		     0, 0xff, 0, snd_soc_get_volsw, wm8580_out_vu, dac_tlv),
+
+SOC_SINGLE("DAC4 Deemphasis Switch", WM8580_DAC_CONTROL3, 3, 1, 0),
+
+SOC_DOUBLE("DAC4 Invert Switch", WM8580_DAC_CONTROL4,  8, 7, 1, 0),
+
+SOC_SINGLE("DAC4 Switch", WM8580_DAC_CONTROL5, 3, 1, 1),
+};
+
 static const struct snd_soc_dapm_widget wm8580_dapm_widgets[] = {
 SND_SOC_DAPM_DAC("DAC1", "Playback", WM8580_PWRDN1, 2, 1),
 SND_SOC_DAPM_DAC("DAC2", "Playback", WM8580_PWRDN1, 3, 1),
@@ -324,6 +347,13 @@ SND_SOC_DAPM_INPUT("AINL"),
 SND_SOC_DAPM_INPUT("AINR"),
 };
 
+static const struct snd_soc_dapm_widget wm8581_dapm_widgets[] = {
+SND_SOC_DAPM_DAC("DAC4", "Playback", WM8580_PWRDN1, 5, 1),
+
+SND_SOC_DAPM_OUTPUT("VOUT4L"),
+SND_SOC_DAPM_OUTPUT("VOUT4R"),
+};
+
 static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "VOUT1L", NULL, "DAC1" },
 	{ "VOUT1R", NULL, "DAC1" },
@@ -338,6 +368,11 @@ static const struct snd_soc_dapm_route wm8580_dapm_routes[] = {
 	{ "ADC", NULL, "AINR" },
 };
 
+static const struct snd_soc_dapm_route wm8581_dapm_routes[] = {
+	{ "VOUT4L", NULL, "DAC4" },
+	{ "VOUT4R", NULL, "DAC4" },
+};
+
 /* PLL divisors */
 struct _pll_div {
 	u32 prescale:1;
@@ -815,10 +850,21 @@ static int wm8580_set_bias_level(struct snd_soc_codec *codec,
 	return 0;
 }
 
+static int wm8580_playback_startup(struct snd_pcm_substream *substream,
+			   struct snd_soc_dai *dai)
+{
+	struct snd_soc_codec *codec = dai->codec;
+	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
+
+	return snd_pcm_hw_constraint_minmax(substream->runtime,
+		SNDRV_PCM_HW_PARAM_CHANNELS, 1, wm8580->drvdata->num_dacs * 2);
+}
+
 #define WM8580_FORMATS (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |\
 			SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S32_LE)
 
 static const struct snd_soc_dai_ops wm8580_dai_ops_playback = {
+	.startup	= wm8580_playback_startup,
 	.set_sysclk	= wm8580_set_sysclk,
 	.hw_params	= wm8580_paif_hw_params,
 	.set_fmt	= wm8580_set_paif_dai_fmt,
@@ -842,7 +888,6 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
 		.playback = {
 			.stream_name = "Playback",
 			.channels_min = 1,
-			.channels_max = 6,
 			.rates = SNDRV_PCM_RATE_8000_192000,
 			.formats = WM8580_FORMATS,
 		},
@@ -865,8 +910,22 @@ static struct snd_soc_dai_driver wm8580_dai[] = {
 static int wm8580_probe(struct snd_soc_codec *codec)
 {
 	struct wm8580_priv *wm8580 = snd_soc_codec_get_drvdata(codec);
+	struct snd_soc_dapm_context *dapm = snd_soc_codec_get_dapm(codec);
 	int ret = 0;
 
+	switch (wm8580->drvdata->num_dacs) {
+	case 4:
+		snd_soc_add_codec_controls(codec, wm8581_snd_controls,
+					ARRAY_SIZE(wm8581_snd_controls));
+		snd_soc_dapm_new_controls(dapm, wm8581_dapm_widgets,
+					ARRAY_SIZE(wm8581_dapm_widgets));
+		snd_soc_dapm_add_routes(dapm, wm8581_dapm_routes,
+					ARRAY_SIZE(wm8581_dapm_routes));
+		break;
+	default:
+		break;
+	}
+
 	ret = regulator_bulk_enable(ARRAY_SIZE(wm8580->supplies),
 				    wm8580->supplies);
 	if (ret != 0) {
@@ -914,12 +973,6 @@ static const struct snd_soc_codec_driver soc_codec_dev_wm8580 = {
 	},
 };
 
-static const struct of_device_id wm8580_of_match[] = {
-	{ .compatible = "wlf,wm8580" },
-	{ },
-};
-MODULE_DEVICE_TABLE(of, wm8580_of_match);
-
 static const struct regmap_config wm8580_regmap = {
 	.reg_bits = 7,
 	.val_bits = 9,
@@ -932,10 +985,26 @@ static const struct regmap_config wm8580_regmap = {
 	.volatile_reg = wm8580_volatile,
 };
 
+const struct wm8580_driver_data wm8580_data = {
+	.num_dacs = 3,
+};
+
+const struct wm8580_driver_data wm8581_data = {
+	.num_dacs = 4,
+};
+
+static const struct of_device_id wm8580_of_match[] = {
+	{ .compatible = "wlf,wm8580", .data = &wm8580_data },
+	{ .compatible = "wlf,wm8581", .data = &wm8581_data },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, wm8580_of_match);
+
 #if IS_ENABLED(CONFIG_I2C)
 static int wm8580_i2c_probe(struct i2c_client *i2c,
 			    const struct i2c_device_id *id)
 {
+	const struct of_device_id *of_id;
 	struct wm8580_priv *wm8580;
 	int ret, i;
 
@@ -960,6 +1029,15 @@ static int wm8580_i2c_probe(struct i2c_client *i2c,
 
 	i2c_set_clientdata(i2c, wm8580);
 
+	of_id = of_match_device(wm8580_of_match, &i2c->dev);
+	if (of_id)
+		wm8580->drvdata = of_id->data;
+
+	if (!wm8580->drvdata) {
+		dev_err(&i2c->dev, "failed to find driver data\n");
+		return -EINVAL;
+	}
+
 	ret =  snd_soc_register_codec(&i2c->dev,
 			&soc_codec_dev_wm8580, wm8580_dai, ARRAY_SIZE(wm8580_dai));
 
@@ -973,7 +1051,8 @@ static int wm8580_i2c_remove(struct i2c_client *client)
 }
 
 static const struct i2c_device_id wm8580_i2c_id[] = {
-	{ "wm8580", 0 },
+	{ "wm8580", (kernel_ulong_t)&wm8580_data },
+	{ "wm8581", (kernel_ulong_t)&wm8581_data },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, wm8580_i2c_id);
@@ -1014,4 +1093,5 @@ module_exit(wm8580_exit);
 
 MODULE_DESCRIPTION("ASoC WM8580 driver");
 MODULE_AUTHOR("Mark Brown <broonie@opensource.wolfsonmicro.com>");
+MODULE_AUTHOR("Matt Flax <flatmax@flatmax.org>");
 MODULE_LICENSE("GPL");
-- 
2.7.4

Re: [PATCH v3] ASoc: wm8580: Add the wm8581 codec to the driver

[Charles Keepax]

On Thu, Oct 20, 2016 at 10:29:14AM +1100, Matt Flax wrote:
> This patch adds support for the wm8581 codec to the wm8580 driver.
> The wm8581 codec hardware adds a fourth DAC and otherwise is
> compatible with the wm8580 codec.
> 
> of_device_id data is used to allow the driver to select the
> suitable DAC count specified in the device tree codec selection.
> The wm8580_driver_data struct is used to store the number of DACs.
> 
> The snd_soc_dai_driver no longer lists the channels_max for the
> playback substream. This variable is set during the i2c probe
> from the of_device_id supplied wm8580_driver_data struct.
> 
> With knowledge of the number of DACs in use, the DAC4 controls,
> widgets and routes are added as required for DAC4.
> 
> The device tree documentation for the wm8580 is altered to list
> the wm8581 codec support, as is the Kconfig file.
> 
> Signed-off-by: Matt Flax <flatmax@flatmax.org>
> ---

Looks good to me.

Reviewed-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>

Thanks,
Charles

Re: [PATCH v3] ASoc: wm8580: Add the wm8581 codec to the driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 319 bytes --]

On Thu, Oct 20, 2016 at 10:29:14AM +1100, Matt Flax wrote:
> This patch adds support for the wm8581 codec to the wm8580 driver.
> The wm8581 codec hardware adds a fourth DAC and otherwise is

Please don't post new patches in reply to old ones, it makes it hard to
follow what's going on and which versions are current.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3] ASoc: wm8580: Add the wm8581 codec to the driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 520 bytes --]

On Thu, Oct 20, 2016 at 10:29:14AM +1100, Matt Flax wrote:
> This patch adds support for the wm8581 codec to the wm8580 driver.
> The wm8581 codec hardware adds a fourth DAC and otherwise is
> compatible with the wm8580 codec.

Also:

Please submit patches using subject lines reflecting the style for the
subsystem.  This makes it easier for people to identify relevant
patches.  Look at what existing commits in the area you're changing are
doing and make sure your subject lines visually resemble what they're
doing.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]