Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1714532 > unrolled thread

[PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming Jeffy Chen <jeffy.chen@rock-chips.com> - 2017-08-18 05:20 +0200
    Re: [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming Mark Brown <broonie@kernel.org> - 2017-08-18 13:50 +0200
      Re: [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming jeffy <jeffy.chen@rock-chips.com> - 2017-08-18 17:10 +0200
        Re: [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming Mark Brown <broonie@kernel.org> - 2017-08-21 19:40 +0200
          Re: [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming jeffy <jeffy.chen@rock-chips.com> - 2017-08-22 06:30 +0200

#1714532 — [PATCH v4 1/9] ASoC: rt5514: Avoid legacy dai naming

Currently we are using devm_snd_soc_register_component, which would
use legacy dai naming when dai drv id is zero.

Set a non-zero dai drv id to use dai drv name for dai name.

Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---

Changes in v4:
Use non-zero drv id to avoid legacy dai naming instead of switching to
snd_soc_register_codec.

Changes in v3: None
Changes in v2: None

 sound/soc/codecs/rt5514-spi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/rt5514-spi.c b/sound/soc/codecs/rt5514-spi.c
index 640193d845be..4eb08f636f93 100644
--- a/sound/soc/codecs/rt5514-spi.c
+++ b/sound/soc/codecs/rt5514-spi.c
@@ -62,7 +62,7 @@ static const struct snd_pcm_hardware rt5514_spi_pcm_hardware = {
 
 static struct snd_soc_dai_driver rt5514_spi_dai = {
 	.name = "rt5514-dsp-cpu-dai",
-	.id = 0,
+	.id = 1, /* Set a non-zero drv id to avoid legacy dai naming */
 	.capture = {
 		.stream_name = "DSP Capture",
 		.channels_min = 1,
-- 
2.11.0

[toc] | [next] | [standalone]

#1714833

[Multipart message — attachments visible in raw view] — view raw

On Fri, Aug 18, 2017 at 11:11:39AM +0800, Jeffy Chen wrote:
> Currently we are using devm_snd_soc_register_component, which would
> use legacy dai naming when dai drv id is zero.
> 
> Set a non-zero dai drv id to use dai drv name for dai name.

Why?  This is clearly not good, we shouldn't be expecting all devices
with a single DAI to set the DAI number to 1 and the changelog doesn't
articulate any reason why anything cares what the name is.

[toc] | [prev] | [next] | [standalone]

#1715238

Hi Mark,

On 08/18/2017 07:45 PM, Mark Brown wrote:
> On Fri, Aug 18, 2017 at 11:11:39AM +0800, Jeffy Chen wrote:
>> Currently we are using devm_snd_soc_register_component, which would
>> use legacy dai naming when dai drv id is zero.
>>
>> Set a non-zero dai drv id to use dai drv name for dai name.
>
> Why?  This is clearly not good, we shouldn't be expecting all devices
> with a single DAI to set the DAI number to 1 and the changelog doesn't
> articulate any reason why anything cares what the name is.
>

i was trying to avoid legacy dai naming, because snd_soc_find_dai will 
try to match the dai name:

struct snd_soc_dai *snd_soc_find_dai(
         const struct snd_soc_dai_link_component *dlc)
{
...

                 list_for_each_entry(dai, &component->dai_list, list) {
                         if (dlc->dai_name && strcmp(dai->name, 
dlc->dai_name))
                                 continue;

                         return dai;


if match failed, it would break soc_bind_dai_link:

static int soc_bind_dai_link(struct snd_soc_card *card,
         struct snd_soc_dai_link *dai_link)
{
...
         /* Find CODEC from registered CODECs */
         codec_dais = rtd->codec_dais;
         for (i = 0; i < rtd->num_codecs; i++) {
                 codec_dais[i] = snd_soc_find_dai(&codecs[i]);
                 if (!codec_dais[i]) {
                         dev_err(card->dev, "ASoC: CODEC DAI %s not 
registered\n",
                                 codecs[i].dai_name);
                         goto _err_defer;
                 }




when using legacy dai naming, the dai->name for rt5514-spi would be the 
dev name, which is spi2.0 with my local 4.4 kernel, and would be 
spi32765.0 with upstream kernel.

[toc] | [prev] | [next] | [standalone]

#1716753

[Multipart message — attachments visible in raw view] — view raw

On Fri, Aug 18, 2017 at 11:03:46PM +0800, jeffy wrote:

> when using legacy dai naming, the dai->name for rt5514-spi would be the dev
> name, which is spi2.0 with my local 4.4 kernel, and would be spi32765.0 with
> upstream kernel.

It would be better to fix the code to not need a name if the device by
itself is unambiguous.

[toc] | [prev] | [next] | [standalone]

#1717016

Hi Mark,

thanks for your reply.

On 08/22/2017 01:31 AM, Mark Brown wrote:
> On Fri, Aug 18, 2017 at 11:03:46PM +0800, jeffy wrote:
>
>> when using legacy dai naming, the dai->name for rt5514-spi would be the dev
>> name, which is spi2.0 with my local 4.4 kernel, and would be spi32765.0 with
>> upstream kernel.
>
> It would be better to fix the code to not need a name if the device by
> itself is unambiguous.
>

right...

i'm not familiar with the soc core codes, would these make sense?

1/ consider match when the of nodes are the same:

@@ -978,9 +978,10 @@ struct snd_soc_dai *snd_soc_find_dai(
                 if (dlc->name && strcmp(component->name, dlc->name))
                         continue;
                 list_for_each_entry(dai, &component->dai_list, list) {
+                       if (dlc->of_node && dai->dev->of_node == 
dlc->of_node)
+                               return dai;

or
2/ return the first dai when there's only one:

@@ -977,10 +977,11 @@ struct snd_soc_dai *snd_soc_find_dai(
                         continue;
                 if (dlc->name && strcmp(component->name, dlc->name))
                         continue;
+               if (component->num_dai == 1)
+                       return &component->dai_list[0];
                 list_for_each_entry(dai, &component->dai_list, list) {

or
3/ also check for dai driver name:

@@ -978,9 +978,10 @@ struct snd_soc_dai *snd_soc_find_dai(
                 if (dlc->name && strcmp(component->name, dlc->name))
                         continue;
                 list_for_each_entry(dai, &component->dai_list, list) {
-                       if (dlc->dai_name && strcmp(dai->name, 
dlc->dai_name))
+                       if (dlc->dai_name && strcmp(dai->name, 
dlc->dai_name)
+                           && strcmp(dai->driver->name, dlc->dai_name))
                                 continue;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web