[alsa-devel] [PATCH v2 116/146] ASoC: sof: use modern dai_link style

[Daniel Baluta]

Hi Morimoto-san,

One question inline:

On Thu, Jun 6, 2019 at 8:26 AM Kuninori Morimoto
<kuninori.morimoto.gx at renesas.com> wrote:
>
>
> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
>
> ASoC is now supporting modern style dai_link
> (= snd_soc_dai_link_component) for CPU/Codec/Platform.
> This patch switches to use it.
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
>  sound/soc/sof/nocodec.c  | 21 +++++++++++++++++----
>  sound/soc/sof/topology.c | 20 +++++++++-----------
>  2 files changed, 26 insertions(+), 15 deletions(-)
>
> diff --git a/sound/soc/sof/nocodec.c b/sound/soc/sof/nocodec.c
> index f84b434..3d128e5 100644
> --- a/sound/soc/sof/nocodec.c
> +++ b/sound/soc/sof/nocodec.c
> @@ -21,6 +21,7 @@ static int sof_nocodec_bes_setup(struct device *dev,
>                                  struct snd_soc_dai_link *links,
>                                  int link_num, struct snd_soc_card *card)
>  {
> +       struct snd_soc_dai_link_component *dlc;
>         int i;
>
>         if (!ops || !links || !card)
> @@ -28,17 +29,29 @@ static int sof_nocodec_bes_setup(struct device *dev,
>
>         /* set up BE dai_links */
>         for (i = 0; i < link_num; i++) {
> +               dlc = devm_kzalloc(dev, 3 * sizeof(*dlc), GFP_KERNEL);
> +               if (!dlc)
> +                       return -ENOMEM;
> +
>                 links[i].name = devm_kasprintf(dev, GFP_KERNEL,
>                                                "NoCodec-%d", i);
>                 if (!links[i].name)
>                         return -ENOMEM;
>
> +               links[i].cpus = &dlc[0];
> +               links[i].codecs = &dlc[1];
> +               links[i].platforms = &dlc[2];
> +
> +               links[i].num_cpus = 1;
> +               links[i].num_codecs = 1;
> +               links[i].num_platforms = 1;
> +
>                 links[i].id = i;
>                 links[i].no_pcm = 1;
> -               links[i].cpu_dai_name = ops->drv[i].name;
> -               links[i].platform_name = dev_name(dev);
> -               links[i].codec_dai_name = "snd-soc-dummy-dai";
> -               links[i].codec_name = "snd-soc-dummy";
> +               links[i].cpus->dai_name = ops->drv[i].name;
> +               links[i].platforms->name = dev_name(dev);
> +               links[i].codecs->dai_name = "snd-soc-dummy-dai";
> +               links[i].codecs->name = "snd-soc-dummy";
>                 links[i].dpcm_playback = 1;
>                 links[i].dpcm_capture = 1;
>         }
> diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c
> index b969686f..a13233a 100644
> --- a/sound/soc/sof/topology.c
> +++ b/sound/soc/sof/topology.c
> @@ -2639,7 +2639,6 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index,
>                              struct sof_ipc_dai_config *config)
>  {
>         struct snd_sof_dev *sdev = snd_soc_component_get_drvdata(scomp);
> -       struct snd_soc_dai_link_component dai_component;
>         struct snd_soc_tplg_private *private = &cfg->priv;
>         struct snd_soc_dai *dai;
>         u32 size = sizeof(*config);
> @@ -2650,7 +2649,6 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index,
>         int ret;
>
>         /* init IPC */
> -       memset(&dai_component, 0, sizeof(dai_component));
>         memset(&config->hda, 0, sizeof(struct sof_ipc_dai_hda_params));
>         config->hdr.size = size;
>
> @@ -2664,11 +2662,10 @@ static int sof_link_hda_load(struct snd_soc_component *scomp, int index,
>                 return ret;
>         }
>
> -       dai_component.dai_name = link->cpu_dai_name;
> -       dai = snd_soc_find_dai(&dai_component);
> +       dai = snd_soc_find_dai(link->cpus);
>         if (!dai) {
>                 dev_err(sdev->dev, "error: failed to find dai %s in %s",
> -                       dai_component.dai_name, __func__);
> +                       link->cpus->dai_name, __func__);
>                 return -EINVAL;
>         }
>
> @@ -2708,7 +2705,11 @@ static int sof_link_load(struct snd_soc_component *scomp, int index,
>         int ret;
>         int i = 0;
>
> -       link->platform_name = dev_name(sdev->dev);
> +       if (!link->platforms) {
> +               dev_err(sdev->dev, "error: no platforms\n");
> +               return -EINVAL;

Why do we need this check? With linux-next this check fails for me.

I do also have some local changes so take this question with a grain of salt.

This check fails for FE links as they are created as follows:

/* create the FE DAI link */
static int soc_tplg_fe_link_create

=>  link->cpus»     = &dlc[0];
=> link->codecs»   = &dlc[1];
=> link->num_cpus»  = 1;
=> link->num_codecs = 1;

So, there is no platforms set up for FE links.

I could get rid of the failure with the following patch:

diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
index f485f7f751a1..ee73318135fc 100644
--- a/sound/soc/soc-topology.c
+++ b/sound/soc/soc-topology.c
@@ -1883,7 +1883,7 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,
        int ret;

        /* link + cpu + codec */
-       link = kzalloc(sizeof(*link) + (2 * sizeof(*dlc)), GFP_KERNEL);
+       link = kzalloc(sizeof(*link) + (3 * sizeof(*dlc)), GFP_KERNEL);
        if (link == NULL)
                return -ENOMEM;

@@ -1891,9 +1891,11 @@ static int soc_tplg_fe_link_create(struct soc_tplg *tplg,

        link->cpus      = &dlc[0];
        link->codecs    = &dlc[1];
+       link->platforms = &dlc[2];

        link->num_cpus   = 1;
        link->num_codecs = 1;
+       link->num_platforms = 1;

Can you please help me figure this out?

thanks,
Daniel.