Re: [PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

[Brian Norris]

Hi,

On Tue, Nov 21, 2017 at 04:25:17PM +0800, Jeffy Chen wrote:
> The rt5514 dsp captures pcm data through spi directly, so we should not
> use rockchip-i2s as it's cpu dai like other codecs.
> 
> Use dummy_dai for rt5514 dsp dailink to make voice wakeup work again.
> 
> Reported-by: Jimmy Cheng-Yi Chiang 
> Fixes: (72cfb0f20c75 ASoC: rockchip: Use codec of_node and dai_name for 
> rt5514 dsp)
> Signed-off-by: Jeffy Chen 

I didn't review this closely (and I don't know ASoC that well), but this
does fix regressions I've seen on 4.15 RCs, where

(a) the rt5514 DAI link doesn't get set up
(b) the rt5514 *always* causes my device to wake up, because we arm the
wakeup IRQ even though we never actually configured the DSP
(c) there are system crashes on resume because the rt5514-spi driver
assumes that the DAI link was correctly configured (that's the
subject of another patch I sent [1]

I believe this was working fine on 4.14? At least, I know (b) didn't
happen, and I'm not sure about (a). (c) is a new issue in 4.15-rc1.

Anyway, that's all to say:

Tested-by: Brian Norris 

on the "kevin" Chromebook (Samsung Chromebook Plus).

I also suspect this might be regression-fixing material, for 4.15. Or if
not, at least something like patch [1] should be.

Thanks,
Brian

[1] https://patchwork.kernel.org/patch/10116761/
[PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"

Re: [PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

[Brian Norris]

Hi,

On Tue, Nov 21, 2017 at 04:25:17PM +0800, Jeffy Chen wrote:
> The rt5514 dsp captures pcm data through spi directly, so we should not
> use rockchip-i2s as it's cpu dai like other codecs.
> 
> Use dummy_dai for rt5514 dsp dailink to make voice wakeup work again.
> 
> Reported-by: Jimmy Cheng-Yi Chiang 
> Fixes: (72cfb0f20c75 ASoC: rockchip: Use codec of_node and dai_name for 
> rt5514 dsp)
> Signed-off-by: Jeffy Chen 

I didn't review this closely (and I don't know ASoC that well), but this
does fix regressions I've seen on 4.15 RCs, where

(a) the rt5514 DAI link doesn't get set up
(b) the rt5514 *always* causes my device to wake up, because we arm the
wakeup IRQ even though we never actually configured the DSP
(c) there are system crashes on resume because the rt5514-spi driver
assumes that the DAI link was correctly configured (that's the
subject of another patch I sent [1]

I believe this was working fine on 4.14? At least, I know (b) didn't
happen, and I'm not sure about (a). (c) is a new issue in 4.15-rc1.

Anyway, that's all to say:

Tested-by: Brian Norris 

on the "kevin" Chromebook (Samsung Chromebook Plus).

I also suspect this might be regression-fixing material, for 4.15. Or if
not, at least something like patch [1] should be.

Thanks,
Brian

[1] https://patchwork.kernel.org/patch/10116761/
[PATCH for-4.15] ASoC: rt5514: don't assume rt5514 component was "attached"

[PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

[Jeffy Chen]

The rt5514 dsp captures pcm data through spi directly, so we should not
use rockchip-i2s as it's cpu dai like other codecs.

Use dummy_dai for rt5514 dsp dailink to make voice wakeup work again.

Reported-by: Jimmy Cheng-Yi Chiang 
Fixes: (72cfb0f20c75 ASoC: rockchip: Use codec of_node and dai_name for rt5514 
dsp)
Signed-off-by: Jeffy Chen 
---

 sound/soc/rockchip/rk3399_gru_sound.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/sound/soc/rockchip/rk3399_gru_sound.c 
b/sound/soc/rockchip/rk3399_gru_sound.c
index d64fbbd50544..aa8ffd035377 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -367,7 +367,8 @@ static const struct snd_soc_dai_link rockchip_dais[] = {
[DAILINK_RT5514_DSP] = {
.name = "RT5514 DSP",
.stream_name = "Wake on Voice",
-   .codec_dai_name = "rt5514-dsp-cpu-dai",
+   .codec_name = "snd-soc-dummy",
+   .codec_dai_name = "snd-soc-dummy-dai",
},
 };
 
@@ -528,7 +529,18 @@ static int rockchip_sound_of_parse_dais(struct device *dev,
if (index < 0)
continue;
 
-   np_cpu = (index == DAILINK_CDNDP) ? np_cpu1 : np_cpu0;
+   switch (index) {
+   case DAILINK_CDNDP:
+   np_cpu = np_cpu1;
+   break;
+   case DAILINK_RT5514_DSP:
+   np_cpu = np_codec;
+   break;
+   default:
+   np_cpu = np_cpu0;
+   break;
+   }
+
if (!np_cpu) {
dev_err(dev, "Missing 'rockchip,cpu' for %s\n",
rockchip_dais[index].name);
@@ -538,7 +550,8 @@ static int rockchip_sound_of_parse_dais(struct device *dev,
dai = >dai_link[card->num_links++];
*dai = rockchip_dais[index];
 
-   dai->codec_of_node = np_codec;
+   if (!dai->codec_name)
+   dai->codec_of_node = np_codec;
dai->platform_of_node = np_cpu;
dai->cpu_of_node = np_cpu;
 
-- 
2.11.0

[PATCH] ASoC: rockchip: Use dummy_dai for rt5514 dsp dailink

[Jeffy Chen]

The rt5514 dsp captures pcm data through spi directly, so we should not
use rockchip-i2s as it's cpu dai like other codecs.

Use dummy_dai for rt5514 dsp dailink to make voice wakeup work again.

Reported-by: Jimmy Cheng-Yi Chiang 
Fixes: (72cfb0f20c75 ASoC: rockchip: Use codec of_node and dai_name for rt5514 
dsp)
Signed-off-by: Jeffy Chen 
---

 sound/soc/rockchip/rk3399_gru_sound.c | 19 ---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/sound/soc/rockchip/rk3399_gru_sound.c 
b/sound/soc/rockchip/rk3399_gru_sound.c
index d64fbbd50544..aa8ffd035377 100644
--- a/sound/soc/rockchip/rk3399_gru_sound.c
+++ b/sound/soc/rockchip/rk3399_gru_sound.c
@@ -367,7 +367,8 @@ static const struct snd_soc_dai_link rockchip_dais[] = {
[DAILINK_RT5514_DSP] = {
.name = "RT5514 DSP",
.stream_name = "Wake on Voice",
-   .codec_dai_name = "rt5514-dsp-cpu-dai",
+   .codec_name = "snd-soc-dummy",
+   .codec_dai_name = "snd-soc-dummy-dai",
},
 };
 
@@ -528,7 +529,18 @@ static int rockchip_sound_of_parse_dais(struct device *dev,
if (index < 0)
continue;
 
-   np_cpu = (index == DAILINK_CDNDP) ? np_cpu1 : np_cpu0;
+   switch (index) {
+   case DAILINK_CDNDP:
+   np_cpu = np_cpu1;
+   break;
+   case DAILINK_RT5514_DSP:
+   np_cpu = np_codec;
+   break;
+   default:
+   np_cpu = np_cpu0;
+   break;
+   }
+
if (!np_cpu) {
dev_err(dev, "Missing 'rockchip,cpu' for %s\n",
rockchip_dais[index].name);
@@ -538,7 +550,8 @@ static int rockchip_sound_of_parse_dais(struct device *dev,
dai = >dai_link[card->num_links++];
*dai = rockchip_dais[index];
 
-   dai->codec_of_node = np_codec;
+   if (!dai->codec_name)
+   dai->codec_of_node = np_codec;
dai->platform_of_node = np_cpu;
dai->cpu_of_node = np_cpu;
 
-- 
2.11.0