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@xxxxxxxxxxx>
> ---
<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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
