All of lore.kernel.org
 help / color / mirror / Atom feed
From: Lars-Peter Clausen <lars@metafoo.de>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Mark Brown <broonie@kernel.org>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	Simon <horms@verge.net.au>, Liam Girdwood <lgirdwood@gmail.com>,
	Linux-SH <linux-sh@vger.kernel.org>
Subject: Re: [PATCH 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
Date: Fri, 10 Apr 2015 10:52:40 +0200	[thread overview]
Message-ID: <55278F58.6030909@metafoo.de> (raw)
In-Reply-To: <878ue08ngb.wl%kuninori.morimoto.gx@renesas.com>

[...]
> But, I noticed it breaks set_fmt() and pcm_new() timing.
> Before: set_fmt -> pcm_new
> After:  pcm_new -> set_fmt
>
> My driver adds kctrl on pcm_new timing, and it refers
> set_fmt's settings. but now, set_fmt happen *after* pcm_new.
> (it adds new kctrl if it has SND_SOC_DAIFMT_CBS_CFS)

What does that control do? This seems to be a bit of a layering violation to 
create a control in the PCM driver based on the configuration of the DAI link.

>
> My solution is these 2
>   pattern1) exchange set_fmt/pcm_new timing. see below
>   pattern2) exchange kctrl assumption (always set kctrl)
>
> Maybe I should try pattern2 ?
>
> ---------------------------------------
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 76bfff2..24d6733 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1604,6 +1604,12 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
>                  }
>          }
>
> +       for (i = 0; i < card->num_links; i++) {
> +               if (card->dai_link[i].dai_fmt)
> +                       snd_soc_runtime_set_dai_fmt(&card->rtd[i],
> +                                                   card->dai_link[i].dai_fmt);
> +       }
> +

This seems to be to early, the DAI's should at least have been probed. I 
think we should put it in soc_probe_link_dais() after the the dai_link->init 
section.

WARNING: multiple messages have this Message-ID (diff)
From: Lars-Peter Clausen <lars@metafoo.de>
To: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Mark Brown <broonie@kernel.org>
Cc: Linux-ALSA <alsa-devel@alsa-project.org>,
	Simon <horms@verge.net.au>, Liam Girdwood <lgirdwood@gmail.com>,
	Linux-SH <linux-sh@vger.kernel.org>
Subject: Re: [alsa-devel] [PATCH 3/3] ASoC: simple-card: Remove support for setting differing DAI formats
Date: Fri, 10 Apr 2015 08:52:40 +0000	[thread overview]
Message-ID: <55278F58.6030909@metafoo.de> (raw)
In-Reply-To: <878ue08ngb.wl%kuninori.morimoto.gx@renesas.com>

[...]
> But, I noticed it breaks set_fmt() and pcm_new() timing.
> Before: set_fmt -> pcm_new
> After:  pcm_new -> set_fmt
>
> My driver adds kctrl on pcm_new timing, and it refers
> set_fmt's settings. but now, set_fmt happen *after* pcm_new.
> (it adds new kctrl if it has SND_SOC_DAIFMT_CBS_CFS)

What does that control do? This seems to be a bit of a layering violation to 
create a control in the PCM driver based on the configuration of the DAI link.

>
> My solution is these 2
>   pattern1) exchange set_fmt/pcm_new timing. see below
>   pattern2) exchange kctrl assumption (always set kctrl)
>
> Maybe I should try pattern2 ?
>
> ---------------------------------------
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 76bfff2..24d6733 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1604,6 +1604,12 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
>                  }
>          }
>
> +       for (i = 0; i < card->num_links; i++) {
> +               if (card->dai_link[i].dai_fmt)
> +                       snd_soc_runtime_set_dai_fmt(&card->rtd[i],
> +                                                   card->dai_link[i].dai_fmt);
> +       }
> +

This seems to be to early, the DAI's should at least have been probed. I 
think we should put it in soc_probe_link_dais() after the the dai_link->init 
section.

  reply	other threads:[~2015-04-10  8:52 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-24  1:05 [PATCH 0/3] ARM: shmobile: armadillo: fixup CPU settings Kuninori Morimoto
2015-03-24  1:06 ` [PATCH 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format Kuninori Morimoto
2015-03-24  1:06 ` [PATCH 2/3] ARM: shmobile: armadillo800eva: fix clock inversion Kuninori Morimoto
2015-03-24  1:07 ` [PATCH 3/3] ASoC: simple-card: Remove support for setting differing DAI formats Kuninori Morimoto
2015-03-24  1:07   ` Kuninori Morimoto
2015-04-10  7:15   ` Kuninori Morimoto
2015-04-10  8:52     ` Lars-Peter Clausen [this message]
2015-04-10  8:52       ` [alsa-devel] " Lars-Peter Clausen
2015-04-10  9:21       ` Kuninori Morimoto
2015-04-10  9:25         ` Lars-Peter Clausen
2015-04-10  9:25           ` [alsa-devel] " Lars-Peter Clausen
2015-03-26 17:36 ` [PATCH 0/3] ARM: shmobile: armadillo: fixup CPU settings Mark Brown
2015-03-26 17:36   ` Mark Brown
2015-03-27  0:55   ` Simon Horman
2015-03-27  0:55     ` Simon Horman
2015-03-27  1:35 ` Mark Brown
2015-03-27  1:35   ` Mark Brown
  -- strict thread matches above, loose matches on Subject: below --
2015-01-19  9:54 [PATCH 1/3] ARM: shmobile: armadillo800eva: Properly specify HDMI audio link format Lars-Peter Clausen
2015-01-19  9:54 ` [PATCH 3/3] ASoC: simple-card: Remove support for setting differing DAI formats Lars-Peter Clausen
2015-01-19  9:54   ` Lars-Peter Clausen
2015-01-19  9:54   ` Lars-Peter Clausen
2015-01-20  0:11   ` Kuninori Morimoto
2015-01-20  0:11     ` Kuninori Morimoto

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55278F58.6030909@metafoo.de \
    --to=lars@metafoo.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=horms@verge.net.au \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-sh@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.