Re: [PATCH] ASoC: Intel: avs: Add support for RT5663 codec

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 12/22/22 03:41, Amadeusz Sławiński wrote:
> On 12/21/2022 9:36 PM, Pierre-Louis Bossart wrote:
>>
>>> +static int avs_create_dai_link(struct device *dev, const char
>>> *platform_name, int ssp_port,
>>> +                   struct snd_soc_dai_link **dai_link)
>>> +{
>>> +    struct snd_soc_dai_link_component *platform;
>>> +    struct snd_soc_dai_link *dl;
>>> +
>>> +    dl = devm_kzalloc(dev, sizeof(*dl), GFP_KERNEL);
>>> +    platform = devm_kzalloc(dev, sizeof(*platform), GFP_KERNEL);
>>> +    if (!dl || !platform)
>>> +        return -ENOMEM;
>>> +
>>> +    platform->name = platform_name;
>>> +
>>> +    dl->name = devm_kasprintf(dev, GFP_KERNEL, "SSP%d-Codec",
>>> ssp_port);
>>> +    dl->cpus = devm_kzalloc(dev, sizeof(*dl->cpus), GFP_KERNEL);
>>> +    dl->codecs = devm_kzalloc(dev, sizeof(*dl->codecs), GFP_KERNEL);
>>> +    if (!dl->name || !dl->cpus || !dl->codecs)
>>> +        return -ENOMEM;
>>> +
>>> +    dl->cpus->dai_name = devm_kasprintf(dev, GFP_KERNEL, "SSP%d
>>> Pin", ssp_port);
>>> +    dl->codecs->name = devm_kasprintf(dev, GFP_KERNEL,
>>> "i2c-10EC5663:00");
>>> +    dl->codecs->dai_name = RT5663_DAI_NAME;
>>> +    if (!dl->cpus->dai_name || !dl->codecs->name ||
>>> !dl->codecs->dai_name)
>>> +        return -ENOMEM;
>>> +
>>> +    dl->num_cpus = 1;
>>> +    dl->num_codecs = 1;
>>> +    dl->platforms = platform;
>>> +    dl->num_platforms = 1;
>>> +    dl->id = 0;
>>> +    dl->dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
>>> SND_SOC_DAIFMT_CBS_CFS;
>>
>> can I ask why it's necessary to hard-code the format, shouldn't this be
>> specified in the topology?
>>
>> It's a generic question for AVS machine drivers, the same code is found
>> in other cases.
>>
> 
> It describes back-end and topology describes FEs and FE and BE DSP
> configuration, machine boards describe BE configuration. Not to say that
> I didn't wonder if we can perhaps simplify things some more  and move BE
> declaration to topology and perhaps even implement generic i2s board. It
> is something to revisit.

My point is that the topology already provides the information for the
DSP configuration and this is used for the codec configuration, but
that's two pieces of information that need to match, and two things to
change if for some reason you want to use different options.