[PATCH] ASoC: Intel: boards: add max98390 2/4 speakers support

[Pierre-Louis Bossart]

On 9/8/21 8:39 AM, Chiang, Mac wrote:
>> Fixed Bard's address.
>>
>> Quik summary: there are multiple issues below with rather unclear handling
>> of two separate configurations.
> There are 3 hw boards support, I summarize as below:
> 1. SSP2 connects 98390, with 2 speakers.
> 2. SSP1 connects 98390, with 2 speakers or 4 speakers.
> Thanks Piere. Please review my comments if I misunderstand.

Please clearly identify these configurations in the next revision, thanks!


>>> +int max_98390_spk_codec_init(struct snd_soc_pcm_runtime *rtd) {
>>> +	struct snd_soc_card *card = rtd->card;
>>> +	int ret;
>>> +
>>> +	ret = snd_soc_dapm_new_controls(&card->dapm,
>> max_98390_tt_dapm_widgets,
>>> +
>> 	ARRAY_SIZE(max_98390_tt_dapm_widgets));
>>> +	if (ret) {
>>> +		dev_err(rtd->dev, "unable to add dapm controls, ret %d\n",
>> ret);
>>> +		/* Don't need to add routes if widget addition failed */
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = snd_soc_add_card_controls(card, max_98390_tt_kcontrols,
>>> +
>> 	ARRAY_SIZE(max_98390_tt_kcontrols));
>>> +	if (ret) {
>>> +		dev_err(rtd->dev, "unable to add card controls, ret %d\n",
>> ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = snd_soc_dapm_add_routes(&card->dapm,
>> max_98373_dapm_routes,
>>> +
>> 	ARRAY_SIZE(max_98373_dapm_routes));
>>> +	if (ret) {
>>> +		dev_err(rtd->dev, "Speaker Left, Right  map addition failed:
>> %d\n", ret);
>>> +		return ret;
>>> +	}
>>> +
>>> +	ret = snd_soc_dapm_add_routes(&card->dapm,
>> max_98390_tt_dapm_routes,
>>> +
>> 	ARRAY_SIZE(max_98390_tt_dapm_routes));
>>> +	if (ret)
>>> +		dev_err(rtd->dev, "Tweeter Speaker Left, Right map addition
>> failed:
>>> +%d\n", ret);
>>> +
>>> +	return ret;
>>> +}
>>> +EXPORT_SYMBOL_NS(max_98390_spk_codec_init,
>>> +SND_SOC_INTEL_SOF_MAXIM_COMMON);
>>
>> again what happens if you don't have tweeters in the configuration? Why
>> would you add DAPM routes to a non-existent codec?
> Without tweeter quirk, it goes to apply max_98390_codec_conf from max_98390_set_codec_conf()
> With tweeter quirk, it goes to apply max_98390_4spk_codec_conf from max_98390_set_codec_conf()

what I meant here is that you will add the max_98390_tt_dapm_routes even
in the two speaker case. That doesn't seem right?