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

Matt Flax flatmax at flatmax.org
Mon Oct 17 12:46:20 CEST 2016



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 at 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



More information about the Alsa-devel mailing list