[PATCH 0/2] ASoC: rsnd: tidyup .remove method

[PATCH 0/2] ASoC: rsnd: tidyup .remove method

[Kuninori Morimoto]

Hi Mark

These patches call missing remove/unregister functions on rsnd driver.

Kuninori Morimoto (2):
      ASoC: rsnd: call missing snd_ctl_remove()
      ASoC: rsnd: call missing snd_soc_unregiter_component/platform()

 sound/soc/sh/rcar/core.c |   14 +++++++++++++-
 sound/soc/sh/rcar/dvc.c  |   15 +++++++++++++++
 sound/soc/sh/rcar/rsnd.h |    5 +++++
 3 files changed, 33 insertions(+), 1 deletion(-)


BTW, we found ALSA SoC bind/unbind issue on latest upstream kernel.
ALSA SoC is using like this.

	[CPU] - [CARD] - [CODEC]

If we unbind CPU here, it still has CARD infomation.
And, kernel doesn't call initialize function when re-bind,
and it will panic when playback

-------------------------

In our case,
 CPU   : rsnd
 CARD  : simple-card
 CODEC : ak4642


Current list

	/ # aplay -l
	**** List of PLAYBACK Hardware Devices ****
	card 0: rsnddai0ak4642h [rsnd-dai.0-ak4642-hifi], device 0: rsnd-dai.0-ak4642-hifi ak4642-hifi-0 []
	  Subdevices: 1/1
	  Subdevice #0: subdevice #0

Unbind CPU

	echo "ec500000.rcar_sound" > /sys/bus/platform/drivers/rcar_sound/unbind 

But, it still has unbinded card

	/ # aplay -l
	**** List of PLAYBACK Hardware Devices ****
	card 0: rsnddai0ak4642h [rsnd-dai.0-ak4642-hifi], device 0: rsnd-dai.0-ak4642-hifi ak4642-hifi-0 []
	  Subdevices: 1/1
	  Subdevice #0: subdevice #0

Re-bind. .probe function was called, but no mapping message
"asoc-simple-card sound: ak4642-hifi <-> ec500000.rcar_sound mapping ok"

	echo "ec500000.rcar_sound" > /sys/bus/platform/drivers/rcar_sound/bind 
	rcar_sound ec500000.rcar_sound: probed

Kernel panic when playback after rebind

	/ # aplay /home/xxx.wav 
	ASoC: ak4642-hifi <->  No matching rates
	Unable to handle kernel paging request at virtual address ee196100
	pgd = eebbc000
	[ee196100] *pgd=6e01141e(bad)
	Internal error: Oops: 8000000d [#1] SMP ARM
	CPU: 0 PID: 936 Comm: aplay Tainted: G        W      3.19.0-rc6-02875-gf8eac9a #1541
	Hardware name: Generic R8A7790 (Flattened Device Tree)
	task: edc2adc0 ti: ee08a000 task.ti: ee08a000
	PC is at 0xee196100
	LR is at soc_pcm_open+0x6e8/0x7ac
	pc : [<ee196100>]    lr : [<c0386e18>]    psr: a00f0013
	sp : ee08bc90  ip : ee08bc90  fp : ee08bcec
	r10: eeb30800  r9 : eeae7a00  r8 : ffffffea
	r7 : eeb30800  r6 : ee947480  r5 : ee328a00  r4 : edbf5010
	r3 : ee196100  r2 : ee0dc290  r1 : ee947480  r0 : ee328a00
	Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
	Control: 10c5307d  Table: 6ebbc06a  DAC: 00000015
	Process aplay (pid: 936, stack limit = 0xee08a238)
	Stack: (0xee08bc90 to 0xee08c000)
	bc80:                                     00000001 00000002 edbf5010 00000010
	bca0: ee91d520 edbf501c 0000000c 00000000 00000000 00000001 ffffffff eeae7a00
	bcc0: ee08bcf4 00000000 ee08bd10 eea3ad00 ee328ce8 ee328d00 00000000 00000001
	bce0: ee08bd0c ee08bcf0 c0376a60 c038673c c0045098 ee328a00 00000000 ee328c00
	bd00: ee08bd54 ee08bd10 c0376b38 c0376a18 ee08bd2c 00000000 edc2adc0 c0045098
	bd20: ee328d04 ee328d04 c024dd10 ee328c00 eea3ad00 eebb3638 00000000 c0661848
	bd40: eea3ad00 eea3ad00 ee08bd6c ee08bd58 c0376d04 c0376ab4 c04f179c eea3ad00
	bd60: ee08bd8c ee08bd70 c0367f18 c0376ccc c0367e9c 00000000 eebb3638 ee13bec0
	bd80: ee08bdbc ee08bd90 c00c8b94 c0367ea8 00000000 00000010 eea3ad00 eebb3638
	bda0: ee08be88 00000000 c00c8a5c eea3ad08 ee08bde4 ee08bdc0 c00c303c c00c8a68
	bdc0: ee08bed0 eea3ad00 ee08be88 00000000 ee3f0000 ee08be84 ee08bdf4 ee08bde8
	bde0: c00c3ff8 c00c2ea0 ee08be6c ee08bdf8 c00d0c78 c00c3fb8 00000101 ee4264c8
	be00: ee03e890 ee425770 eebb26b8 00000026 00000000 ee425770 ee08bee8 00000000
	be20: 00000802 ee08bf5c 00000000 00000000 ee08a000 eebb3638 ee03e890 ee425770
	be40: c00cef24 ee08bed0 eea3ad00 ee08be90 ee08bf5c ee3f0000 ee08be88 00000000
	be60: ee08bec4 ee08be70 c00d10d4 c00d0400 ee08be84 ee9a1380 ee08bf0c ee08be88
	be80: c036cddc 00000000 ee03e890 ee4274c8 ee08bec4 ee08bea0 c01aa17c ee08bf5c
	bea0: 00000001 ee3f0000 ffffff9c c000eb24 ee08a000 00000000 ee08bf4c ee08bec8
	bec0: c00d1474 c00d0f00 00000041 ee08bed8 ee03e890 ee4274c8 474edc20 00000008
	bee0: ee3f0019 eea3a700 ee03e7d0 ee4264c8 eebb3638 00000101 00000000 00000034
	bf00: 00000000 00000000 00000000 c00de444 00080802 00080802 be8b4ea4 ee3f0000
	bf20: ffffff9c c000eb24 ee08a000 00000000 00080802 00000004 ee3f0000 ffffff9c
	bf40: ee08bf94 ee08bf50 c00c4430 c00d144c c00d38e0 c00def38 eea3a700 00000802
	bf60: be8b0000 00000026 00000100 00000001 be8b466c be8b4ea4 00000002 00000005
	bf80: c000eb24 ee08a000 ee08bfa4 ee08bf98 c00c44f8 c00c431c 00000000 ee08bfa8
	bfa0: c000e980 c00c44dc be8b466c be8b4ea4 be8b4794 00080802 be8b4628 00080802
	bfc0: be8b466c be8b4ea4 00000002 00000005 001255f8 00129264 000dd264 00124478
	bfe0: 00000000 be8b4634 00049257 00083036 600f0030 be8b4794 03433c61 0717419f
	Backtrace: 
	[<c0386730>] (soc_pcm_open) from [<c0376a60>] (snd_pcm_open_substream+0x54/0x9c)
	 r10:00000001 r9:00000000 r8:ee328d00 r7:ee328ce8 r6:eea3ad00 r5:ee08bd10
	 r4:00000000
	[<c0376a0c>] (snd_pcm_open_substream) from [<c0376b38>] (snd_pcm_open+0x90/0x1b4)
	 r5:ee328c00 r4:00000000
	[<c0376aa8>] (snd_pcm_open) from [<c0376d04>] (snd_pcm_playback_open+0x44/0x64)
	 r10:eea3ad00 r9:eea3ad00 r8:c0661848 r7:00000000 r6:eebb3638 r5:eea3ad00
	 r4:ee328c00
	[<c0376cc0>] (snd_pcm_playback_open) from [<c0367f18>] (snd_open+0x7c/0x90)
	 r5:eea3ad00 r4:c04f179c
	[<c0367e9c>] (snd_open) from [<c00c8b94>] (chrdev_open+0x138/0x164)
	 r6:ee13bec0 r5:eebb3638 r4:00000000 r3:c0367e9c
	[<c00c8a5c>] (chrdev_open) from [<c00c303c>] (do_dentry_open.isra.17+0x1a8/0x2b8)
	 r9:eea3ad08 r8:c00c8a5c r7:00000000 r6:ee08be88 r5:eebb3638 r4:eea3ad00
	[<c00c2e94>] (do_dentry_open.isra.17) from [<c00c3ff8>] (vfs_open+0x4c/0x50)
	 r9:ee08be84 r8:ee3f0000 r7:00000000 r6:ee08be88 r5:eea3ad00 r4:ee08bed0
	[<c00c3fac>] (vfs_open) from [<c00d0c78>] (do_last.isra.47+0x884/0xb00)
	[<c00d03f4>] (do_last.isra.47) from [<c00d10d4>] (path_openat+0x1e0/0x54c)
	 r10:00000000 r9:ee08be88 r8:ee3f0000 r7:ee08bf5c r6:ee08be90 r5:eea3ad00
	 r4:ee08bed0
	[<c00d0ef4>] (path_openat) from [<c00d1474>] (do_filp_open+0x34/0x80)
	 r10:00000000 r9:ee08a000 r8:c000eb24 r7:ffffff9c r6:ee3f0000 r5:00000001
	 r4:ee08bf5c
	[<c00d1440>] (do_filp_open) from [<c00c4430>] (do_sys_open+0x120/0x1c0)
	 r7:ffffff9c r6:ee3f0000 r5:00000004 r4:00080802
	[<c00c4310>] (do_sys_open) from [<c00c44f8>] (SyS_open+0x28/0x2c)
	 r9:ee08a000 r8:c000eb24 r7:00000005 r6:00000002 r5:be8b4ea4 r4:be8b466c
	[<c00c44d0>] (SyS_open) from [<c000e980>] (ret_fast_syscall+0x0/0x34)
	Code: 00000000 00000000 00000000 00000000 (0000004f) 
	---[ end trace bd44069e52a540ae ]---
	Segmentation fault


Best regards
---
Kuninori Morimoto

[PATCH 1/2] ASoC: rsnd: call missing snd_ctl_remove()

[Kuninori Morimoto, Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current Renesas R-Car sound driver is using snd_ctl_xxx()
functions, but it didn't call snd_ctl free_one() / snd_ctl_remove().
This patch call these functions.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c |   11 ++++++++++-
 sound/soc/sh/rcar/dvc.c  |   15 +++++++++++++++
 sound/soc/sh/rcar/rsnd.h |    5 +++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index f0bb137..6fb38b8 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1046,14 +1046,23 @@ static int __rsnd_kctrl_new(struct rsnd_mod *mod,
 		return -ENOMEM;
 
 	ret = snd_ctl_add(card, kctrl);
-	if (ret < 0)
+	if (ret < 0) {
+		snd_ctl_free_one(kctrl);
 		return ret;
+	}
 
 	cfg->update = update;
+	cfg->card = card;
+	cfg->kctrl = kctrl;
 
 	return 0;
 }
 
+void _rsnd_kctrl_remove(struct rsnd_kctrl_cfg *cfg)
+{
+	snd_ctl_remove(cfg->card, cfg->kctrl);
+}
+
 int rsnd_kctrl_new_m(struct rsnd_mod *mod,
 		     struct snd_soc_pcm_runtime *rtd,
 		     const unsigned char *name,
diff --git a/sound/soc/sh/rcar/dvc.c b/sound/soc/sh/rcar/dvc.c
index 38a5f33..d7f9ed9 100644
--- a/sound/soc/sh/rcar/dvc.c
+++ b/sound/soc/sh/rcar/dvc.c
@@ -127,6 +127,20 @@ static int rsnd_dvc_probe_gen2(struct rsnd_mod *mod,
 	return 0;
 }
 
+static int rsnd_dvc_remove_gen2(struct rsnd_mod *mod,
+				struct rsnd_priv *priv)
+{
+	struct rsnd_dvc *dvc = rsnd_mod_to_dvc(mod);
+
+	rsnd_kctrl_remove(dvc->volume);
+	rsnd_kctrl_remove(dvc->mute);
+	rsnd_kctrl_remove(dvc->ren);
+	rsnd_kctrl_remove(dvc->rup);
+	rsnd_kctrl_remove(dvc->rdown);
+
+	return 0;
+}
+
 static int rsnd_dvc_init(struct rsnd_mod *dvc_mod,
 			 struct rsnd_priv *priv)
 {
@@ -258,6 +272,7 @@ static int rsnd_dvc_pcm_new(struct rsnd_mod *mod,
 static struct rsnd_mod_ops rsnd_dvc_ops = {
 	.name		= DVC_NAME,
 	.probe		= rsnd_dvc_probe_gen2,
+	.remove		= rsnd_dvc_remove_gen2,
 	.init		= rsnd_dvc_init,
 	.quit		= rsnd_dvc_quit,
 	.start		= rsnd_dvc_start,
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index b57d8ac..e7914bd 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -436,6 +436,8 @@ struct rsnd_kctrl_cfg {
 	u32 *val;
 	const char * const *texts;
 	void (*update)(struct rsnd_mod *mod);
+	struct snd_card *card;
+	struct snd_kcontrol *kctrl;
 };
 
 #define RSND_DVC_CHANNELS	2
@@ -449,6 +451,9 @@ struct rsnd_kctrl_cfg_s {
 	u32 val;
 };
 
+void _rsnd_kctrl_remove(struct rsnd_kctrl_cfg *cfg);
+#define rsnd_kctrl_remove(_cfg)	_rsnd_kctrl_remove(&((_cfg).cfg))
+
 int rsnd_kctrl_new_m(struct rsnd_mod *mod,
 		     struct snd_soc_pcm_runtime *rtd,
 		     const unsigned char *name,
-- 
1.7.9.5

[PATCH 1/2] ASoC: rsnd: call missing snd_ctl_remove()

[Kuninori Morimoto, Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current Renesas R-Car sound driver is using snd_ctl_xxx()
functions, but it didn't call snd_ctl free_one() / snd_ctl_remove().
This patch call these functions.

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Bui Duc Phuc <bd-phuc@jinso.co.jp>
Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c |   11 ++++++++++-
 sound/soc/sh/rcar/dvc.c  |   15 +++++++++++++++
 sound/soc/sh/rcar/rsnd.h |    5 +++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index f0bb137..6fb38b8 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1046,14 +1046,23 @@ static int __rsnd_kctrl_new(struct rsnd_mod *mod,
 		return -ENOMEM;
 
 	ret = snd_ctl_add(card, kctrl);
-	if (ret < 0)
+	if (ret < 0) {
+		snd_ctl_free_one(kctrl);
 		return ret;
+	}
 
 	cfg->update = update;
+	cfg->card = card;
+	cfg->kctrl = kctrl;
 
 	return 0;
 }
 
+void _rsnd_kctrl_remove(struct rsnd_kctrl_cfg *cfg)
+{
+	snd_ctl_remove(cfg->card, cfg->kctrl);
+}
+
 int rsnd_kctrl_new_m(struct rsnd_mod *mod,
 		     struct snd_soc_pcm_runtime *rtd,
 		     const unsigned char *name,
diff --git a/sound/soc/sh/rcar/dvc.c b/sound/soc/sh/rcar/dvc.c
index 38a5f33..d7f9ed9 100644
--- a/sound/soc/sh/rcar/dvc.c
+++ b/sound/soc/sh/rcar/dvc.c
@@ -127,6 +127,20 @@ static int rsnd_dvc_probe_gen2(struct rsnd_mod *mod,
 	return 0;
 }
 
+static int rsnd_dvc_remove_gen2(struct rsnd_mod *mod,
+				struct rsnd_priv *priv)
+{
+	struct rsnd_dvc *dvc = rsnd_mod_to_dvc(mod);
+
+	rsnd_kctrl_remove(dvc->volume);
+	rsnd_kctrl_remove(dvc->mute);
+	rsnd_kctrl_remove(dvc->ren);
+	rsnd_kctrl_remove(dvc->rup);
+	rsnd_kctrl_remove(dvc->rdown);
+
+	return 0;
+}
+
 static int rsnd_dvc_init(struct rsnd_mod *dvc_mod,
 			 struct rsnd_priv *priv)
 {
@@ -258,6 +272,7 @@ static int rsnd_dvc_pcm_new(struct rsnd_mod *mod,
 static struct rsnd_mod_ops rsnd_dvc_ops = {
 	.name		= DVC_NAME,
 	.probe		= rsnd_dvc_probe_gen2,
+	.remove		= rsnd_dvc_remove_gen2,
 	.init		= rsnd_dvc_init,
 	.quit		= rsnd_dvc_quit,
 	.start		= rsnd_dvc_start,
diff --git a/sound/soc/sh/rcar/rsnd.h b/sound/soc/sh/rcar/rsnd.h
index b57d8ac..e7914bd 100644
--- a/sound/soc/sh/rcar/rsnd.h
+++ b/sound/soc/sh/rcar/rsnd.h
@@ -436,6 +436,8 @@ struct rsnd_kctrl_cfg {
 	u32 *val;
 	const char * const *texts;
 	void (*update)(struct rsnd_mod *mod);
+	struct snd_card *card;
+	struct snd_kcontrol *kctrl;
 };
 
 #define RSND_DVC_CHANNELS	2
@@ -449,6 +451,9 @@ struct rsnd_kctrl_cfg_s {
 	u32 val;
 };
 
+void _rsnd_kctrl_remove(struct rsnd_kctrl_cfg *cfg);
+#define rsnd_kctrl_remove(_cfg)	_rsnd_kctrl_remove(&((_cfg).cfg))
+
 int rsnd_kctrl_new_m(struct rsnd_mod *mod,
 		     struct snd_soc_pcm_runtime *rtd,
 		     const unsigned char *name,
-- 
1.7.9.5

[PATCH 2/2] ASoC: rsnd: call missing snd_soc_unregiter_component/platform()

[Kuninori Morimoto, Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current Renesas R-Car sound driver doesn't call
snd_soc_unregiter_component/platform() in .remove.
This patch call these functions.

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Bui Duc Phuc <bd-phuc@jinso.co.jp>
Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 6fb38b8..1b53605 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1299,6 +1299,9 @@ static int rsnd_remove(struct platform_device *pdev)
 		ret |= rsnd_dai_call(remove, &rdai->capture, priv);
 	}
 
+	snd_soc_unregister_component(&pdev->dev);
+	snd_soc_unregister_platform(&pdev->dev);
+
 	return ret;
 }
 
-- 
1.7.9.5

Re: [PATCH 1/2] ASoC: rsnd: call missing snd_ctl_remove()

[Kuninori Morimoto]

Hi Mark

> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current Renesas R-Car sound driver is using snd_ctl_xxx()
> functions, but it didn't call snd_ctl free_one() / snd_ctl_remove().
> This patch call these functions.
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

I'm so sorry, I sent 2 [1/2] patches,
but, this side didn't include "Reported-by".
Please drop this [1/2] patch, and use other [1/2] patch

Re: [PATCH 1/2] ASoC: rsnd: call missing snd_ctl_remove()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 323 bytes --]

On Mon, Feb 02, 2015 at 04:53:53AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current Renesas R-Car sound driver is using snd_ctl_xxx()
> functions, but it didn't call snd_ctl free_one() / snd_ctl_remove().
> This patch call these functions.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/2] ASoC: rsnd: call missing snd_soc_unregiter_component/platform()

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 295 bytes --]

On Mon, Feb 02, 2015 at 04:54:07AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current Renesas R-Car sound driver doesn't call
> snd_soc_unregiter_component/platform() in .remove.
> This patch call these functions.

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/2] ASoC: rsnd: tidyup .remove method

[Kuninori Morimoto]

Hi Mark

> BTW, we found ALSA SoC bind/unbind issue on latest upstream kernel.
> ALSA SoC is using like this.
> 
> 	[CPU] - [CARD] - [CODEC]
> 
> If we unbind CPU here, it still has CARD infomation.
> And, kernel doesn't call initialize function when re-bind,
> and it will panic when playback

Is this open bug ?
Or, Renesas sound only ?
fsi/rsnd both have this issue.

> In our case,
>  CPU   : rsnd
>  CARD  : simple-card
>  CODEC : ak4642
> 
> 
> Current list
> 
> 	/ # aplay -l
> 	**** List of PLAYBACK Hardware Devices ****
> 	card 0: rsnddai0ak4642h [rsnd-dai.0-ak4642-hifi], device 0: rsnd-dai.0-ak4642-hifi ak4642-hifi-0 []
> 	  Subdevices: 1/1
> 	  Subdevice #0: subdevice #0
> 
> Unbind CPU
> 
> 	echo "ec500000.rcar_sound" > /sys/bus/platform/drivers/rcar_sound/unbind 
> 
> But, it still has unbinded card
> 
> 	/ # aplay -l
> 	**** List of PLAYBACK Hardware Devices ****
> 	card 0: rsnddai0ak4642h [rsnd-dai.0-ak4642-hifi], device 0: rsnd-dai.0-ak4642-hifi ak4642-hifi-0 []
> 	  Subdevices: 1/1
> 	  Subdevice #0: subdevice #0
> 
> Re-bind. .probe function was called, but no mapping message
> "asoc-simple-card sound: ak4642-hifi <-> ec500000.rcar_sound mapping ok"
> 
> 	echo "ec500000.rcar_sound" > /sys/bus/platform/drivers/rcar_sound/bind 
> 	rcar_sound ec500000.rcar_sound: probed
> 
> Kernel panic when playback after rebind
> 
> 	/ # aplay /home/xxx.wav 
> 	ASoC: ak4642-hifi <->  No matching rates
> 	Unable to handle kernel paging request at virtual address ee196100
> 	pgd = eebbc000
> 	[ee196100] *pgd=6e01141e(bad)
> 	Internal error: Oops: 8000000d [#1] SMP ARM
> 	CPU: 0 PID: 936 Comm: aplay Tainted: G        W      3.19.0-rc6-02875-gf8eac9a #1541
> 	Hardware name: Generic R8A7790 (Flattened Device Tree)
> 	task: edc2adc0 ti: ee08a000 task.ti: ee08a000
> 	PC is at 0xee196100
> 	LR is at soc_pcm_open+0x6e8/0x7ac
> 	pc : [<ee196100>]    lr : [<c0386e18>]    psr: a00f0013
> 	sp : ee08bc90  ip : ee08bc90  fp : ee08bcec
> 	r10: eeb30800  r9 : eeae7a00  r8 : ffffffea
> 	r7 : eeb30800  r6 : ee947480  r5 : ee328a00  r4 : edbf5010
> 	r3 : ee196100  r2 : ee0dc290  r1 : ee947480  r0 : ee328a00
> 	Flags: NzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
> 	Control: 10c5307d  Table: 6ebbc06a  DAC: 00000015
> 	Process aplay (pid: 936, stack limit = 0xee08a238)
> 	Stack: (0xee08bc90 to 0xee08c000)
> 	bc80:                                     00000001 00000002 edbf5010 00000010
> 	bca0: ee91d520 edbf501c 0000000c 00000000 00000000 00000001 ffffffff eeae7a00
> 	bcc0: ee08bcf4 00000000 ee08bd10 eea3ad00 ee328ce8 ee328d00 00000000 00000001
> 	bce0: ee08bd0c ee08bcf0 c0376a60 c038673c c0045098 ee328a00 00000000 ee328c00
> 	bd00: ee08bd54 ee08bd10 c0376b38 c0376a18 ee08bd2c 00000000 edc2adc0 c0045098
> 	bd20: ee328d04 ee328d04 c024dd10 ee328c00 eea3ad00 eebb3638 00000000 c0661848
> 	bd40: eea3ad00 eea3ad00 ee08bd6c ee08bd58 c0376d04 c0376ab4 c04f179c eea3ad00
> 	bd60: ee08bd8c ee08bd70 c0367f18 c0376ccc c0367e9c 00000000 eebb3638 ee13bec0
> 	bd80: ee08bdbc ee08bd90 c00c8b94 c0367ea8 00000000 00000010 eea3ad00 eebb3638
> 	bda0: ee08be88 00000000 c00c8a5c eea3ad08 ee08bde4 ee08bdc0 c00c303c c00c8a68
> 	bdc0: ee08bed0 eea3ad00 ee08be88 00000000 ee3f0000 ee08be84 ee08bdf4 ee08bde8
> 	bde0: c00c3ff8 c00c2ea0 ee08be6c ee08bdf8 c00d0c78 c00c3fb8 00000101 ee4264c8
> 	be00: ee03e890 ee425770 eebb26b8 00000026 00000000 ee425770 ee08bee8 00000000
> 	be20: 00000802 ee08bf5c 00000000 00000000 ee08a000 eebb3638 ee03e890 ee425770
> 	be40: c00cef24 ee08bed0 eea3ad00 ee08be90 ee08bf5c ee3f0000 ee08be88 00000000
> 	be60: ee08bec4 ee08be70 c00d10d4 c00d0400 ee08be84 ee9a1380 ee08bf0c ee08be88
> 	be80: c036cddc 00000000 ee03e890 ee4274c8 ee08bec4 ee08bea0 c01aa17c ee08bf5c
> 	bea0: 00000001 ee3f0000 ffffff9c c000eb24 ee08a000 00000000 ee08bf4c ee08bec8
> 	bec0: c00d1474 c00d0f00 00000041 ee08bed8 ee03e890 ee4274c8 474edc20 00000008
> 	bee0: ee3f0019 eea3a700 ee03e7d0 ee4264c8 eebb3638 00000101 00000000 00000034
> 	bf00: 00000000 00000000 00000000 c00de444 00080802 00080802 be8b4ea4 ee3f0000
> 	bf20: ffffff9c c000eb24 ee08a000 00000000 00080802 00000004 ee3f0000 ffffff9c
> 	bf40: ee08bf94 ee08bf50 c00c4430 c00d144c c00d38e0 c00def38 eea3a700 00000802
> 	bf60: be8b0000 00000026 00000100 00000001 be8b466c be8b4ea4 00000002 00000005
> 	bf80: c000eb24 ee08a000 ee08bfa4 ee08bf98 c00c44f8 c00c431c 00000000 ee08bfa8
> 	bfa0: c000e980 c00c44dc be8b466c be8b4ea4 be8b4794 00080802 be8b4628 00080802
> 	bfc0: be8b466c be8b4ea4 00000002 00000005 001255f8 00129264 000dd264 00124478
> 	bfe0: 00000000 be8b4634 00049257 00083036 600f0030 be8b4794 03433c61 0717419f
> 	Backtrace: 
> 	[<c0386730>] (soc_pcm_open) from [<c0376a60>] (snd_pcm_open_substream+0x54/0x9c)
> 	 r10:00000001 r9:00000000 r8:ee328d00 r7:ee328ce8 r6:eea3ad00 r5:ee08bd10
> 	 r4:00000000
> 	[<c0376a0c>] (snd_pcm_open_substream) from [<c0376b38>] (snd_pcm_open+0x90/0x1b4)
> 	 r5:ee328c00 r4:00000000
> 	[<c0376aa8>] (snd_pcm_open) from [<c0376d04>] (snd_pcm_playback_open+0x44/0x64)
> 	 r10:eea3ad00 r9:eea3ad00 r8:c0661848 r7:00000000 r6:eebb3638 r5:eea3ad00
> 	 r4:ee328c00
> 	[<c0376cc0>] (snd_pcm_playback_open) from [<c0367f18>] (snd_open+0x7c/0x90)
> 	 r5:eea3ad00 r4:c04f179c
> 	[<c0367e9c>] (snd_open) from [<c00c8b94>] (chrdev_open+0x138/0x164)
> 	 r6:ee13bec0 r5:eebb3638 r4:00000000 r3:c0367e9c
> 	[<c00c8a5c>] (chrdev_open) from [<c00c303c>] (do_dentry_open.isra.17+0x1a8/0x2b8)
> 	 r9:eea3ad08 r8:c00c8a5c r7:00000000 r6:ee08be88 r5:eebb3638 r4:eea3ad00
> 	[<c00c2e94>] (do_dentry_open.isra.17) from [<c00c3ff8>] (vfs_open+0x4c/0x50)
> 	 r9:ee08be84 r8:ee3f0000 r7:00000000 r6:ee08be88 r5:eea3ad00 r4:ee08bed0
> 	[<c00c3fac>] (vfs_open) from [<c00d0c78>] (do_last.isra.47+0x884/0xb00)
> 	[<c00d03f4>] (do_last.isra.47) from [<c00d10d4>] (path_openat+0x1e0/0x54c)
> 	 r10:00000000 r9:ee08be88 r8:ee3f0000 r7:ee08bf5c r6:ee08be90 r5:eea3ad00
> 	 r4:ee08bed0
> 	[<c00d0ef4>] (path_openat) from [<c00d1474>] (do_filp_open+0x34/0x80)
> 	 r10:00000000 r9:ee08a000 r8:c000eb24 r7:ffffff9c r6:ee3f0000 r5:00000001
> 	 r4:ee08bf5c
> 	[<c00d1440>] (do_filp_open) from [<c00c4430>] (do_sys_open+0x120/0x1c0)
> 	 r7:ffffff9c r6:ee3f0000 r5:00000004 r4:00080802
> 	[<c00c4310>] (do_sys_open) from [<c00c44f8>] (SyS_open+0x28/0x2c)
> 	 r9:ee08a000 r8:c000eb24 r7:00000005 r6:00000002 r5:be8b4ea4 r4:be8b466c
> 	[<c00c44d0>] (SyS_open) from [<c000e980>] (ret_fast_syscall+0x0/0x34)
> 	Code: 00000000 00000000 00000000 00000000 (0000004f) 
> 	---[ end trace bd44069e52a540ae ]---
> 	Segmentation fault
> 
> 
> Best regards
> ---
> Kuninori Morimoto
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [PATCH 0/2] ASoC: rsnd: tidyup .remove method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 684 bytes --]

On Tue, Feb 03, 2015 at 01:05:52AM +0000, Kuninori Morimoto wrote:
> > BTW, we found ALSA SoC bind/unbind issue on latest upstream kernel.
> > ALSA SoC is using like this.

> > 	[CPU] - [CARD] - [CODEC]

> > If we unbind CPU here, it still has CARD infomation.
> > And, kernel doesn't call initialize function when re-bind,
> > and it will panic when playback

> Is this open bug ?
> Or, Renesas sound only ?
> fsi/rsnd both have this issue.

I'm not aware of anything here - when we rebuild the card we should be
reinitializing everything.  It's possible this is just broken for
everyone though, most people don't unload devices and where they do they
tend to remove the card first.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/2] ASoC: rsnd: tidyup .remove method

[Kuninori Morimoto]

Hi Mark

> > > If we unbind CPU here, it still has CARD infomation.
> > > And, kernel doesn't call initialize function when re-bind,
> > > and it will panic when playback
> 
> > Is this open bug ?
> > Or, Renesas sound only ?
> > fsi/rsnd both have this issue.
> 
> I'm not aware of anything here - when we rebuild the card we should be
> reinitializing everything.  It's possible this is just broken for
> everyone though, most people don't unload devices and where they do they
> tend to remove the card first.

I debuged this issue, and it related to component.
These patches solve this issue.

Kuninori Morimoto (2):
      ASoC: core: call snd_soc_unregister_card() when component cleanup
      ASoC: core: indicate unregister debug message once

 sound/soc/soc-core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

[PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ASoC devices are organized as [CPU]-[CARD]-[CODEC]. Then, [CPU]/[CODEC]
are based on [component] structure. Now, each [CARD] device knows that
it has organized from which [component]. But current [component] doesn't
inform to [CARD] that it was removed when user called rmmod or unbind it.
Thus, [CARD] which lost some [components] still exist in system.
And then, ALSA sound card will have some problem if user used this [CARD]
in such timing. This patch unregister [CARD] when [component] was removed.
If you want to use this [CARD] again, 
you need to re-insmod or re-bind removed [component] and [CARD] again. 

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Bui Duc Phuc <bd-phuc@jinso.co.jp>
Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index ededb97..cd5db21 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2695,6 +2695,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
 static void snd_soc_component_cleanup(struct snd_soc_component *component)
 {
 	snd_soc_unregister_dais(component);
+	snd_soc_unregister_card(component->card);
 	kfree(component->name);
 }
 
-- 
1.7.9.5

[PATCH 2/2] ASoC: core: indicate unregister debug message once

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current snd_soc_unregister_card() indicates unregistered debug
message when it was called. But, it should be called only when
it was really unregistered.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index cd5db21..148c959 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2409,8 +2409,8 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
 		card->instantiated = false;
 		snd_soc_dapm_shutdown(card);
 		soc_cleanup_card_resources(card);
+		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
 	}
-	dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
 
 	return 0;
 }
-- 
1.7.9.5

Re: [PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1734 bytes --]

On Thu, Feb 05, 2015 at 05:33:55AM +0000, Kuninori Morimoto wrote:

> ASoC devices are organized as [CPU]-[CARD]-[CODEC]. Then, [CPU]/[CODEC]
> are based on [component] structure. Now, each [CARD] device knows that
> it has organized from which [component]. But current [component] doesn't
> inform to [CARD] that it was removed when user called rmmod or unbind it.
> Thus, [CARD] which lost some [components] still exist in system.
> And then, ALSA sound card will have some problem if user used this [CARD]
> in such timing. This patch unregister [CARD] when [component] was removed.
> If you want to use this [CARD] again, 
> you need to re-insmod or re-bind removed [component] and [CARD] again. 

So, there's a problem here but...

> @@ -2695,6 +2695,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
>  static void snd_soc_component_cleanup(struct snd_soc_component *component)
>  {
>  	snd_soc_unregister_dais(component);
> +	snd_soc_unregister_card(component->card);
>  	kfree(component->name);
>  }

...this doesn't look like the right fix.  If we just plain unregister
the card then if the component that was removed is loaded again then the
card won't be reinstantiated since the core has forgotten about the
card.  Further, if the card driver is removed then we'll get a duplicate
attempt to unregister it which doesn't seem clever.

What we need to do here is undo all the work that was done to
instantiate the card but not actually unregister it, returning
everything to the state it was in before the card was instantiated.
That's obviously a much more substantial change than this but it's
what's needed (and is the main reason this is broken at the minute).

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Kuninori Morimoto]

Hi Mark

Thank you for your feedback

> > ASoC devices are organized as [CPU]-[CARD]-[CODEC]. Then, [CPU]/[CODEC]
> > are based on [component] structure. Now, each [CARD] device knows that
> > it has organized from which [component]. But current [component] doesn't
> > inform to [CARD] that it was removed when user called rmmod or unbind it.
> > Thus, [CARD] which lost some [components] still exist in system.
> > And then, ALSA sound card will have some problem if user used this [CARD]
> > in such timing. This patch unregister [CARD] when [component] was removed.
> > If you want to use this [CARD] again, 
> > you need to re-insmod or re-bind removed [component] and [CARD] again. 
> 
> So, there's a problem here but...
> 
> > @@ -2695,6 +2695,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
> >  static void snd_soc_component_cleanup(struct snd_soc_component *component)
> >  {
> >  	snd_soc_unregister_dais(component);
> > +	snd_soc_unregister_card(component->card);
> >  	kfree(component->name);
> >  }
> 
> ...this doesn't look like the right fix.  If we just plain unregister
> the card then if the component that was removed is loaded again then the
> card won't be reinstantiated since the core has forgotten about the
> card.  Further, if the card driver is removed then we'll get a duplicate
> attempt to unregister it which doesn't seem clever.
> 
> What we need to do here is undo all the work that was done to
> instantiate the card but not actually unregister it, returning
> everything to the state it was in before the card was instantiated.
> That's obviously a much more substantial change than this but it's
> what's needed (and is the main reason this is broken at the minute).

Hmm..

Main big problem is that the cpu/codec information in card is written in card->rtd[num],
and it has deeply relationship to many functions/datas.
Indeed it needs more substantial change if we solve this issue.
But, I thought register/unregister card is very easy hack, and it is easy to check.

I think this duplicate unregister card is no problem, because snd_soc_unregister_card()
is checking card->instantiated. 2nd unregister do nothing.

My hack (= on snd_soc_component_cleanup) is caring both cpu/codec,
bacause soc_unregister_codec() doesn't have relationship to snd_soc_unregister_component()

It needs more and more change if we solve this issue without above hack,
then, it is very difficult to check all consistency for all features, all situation.
I'm afraid that it can be next bug.
So, how about this ? if un-loaded component unregister card, and if re-loaded component
re-register un-loaded card automatically, it is easy, and safety ?

Best regards
---
Kuninori Morimoto

Re: [PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Kuninori Morimoto]

Hi Mark, again

> > ...this doesn't look like the right fix.  If we just plain unregister
> > the card then if the component that was removed is loaded again then the
> > card won't be reinstantiated since the core has forgotten about the
> > card.  Further, if the card driver is removed then we'll get a duplicate
> > attempt to unregister it which doesn't seem clever.
> > 
> > What we need to do here is undo all the work that was done to
> > instantiate the card but not actually unregister it, returning
> > everything to the state it was in before the card was instantiated.
> > That's obviously a much more substantial change than this but it's
> > what's needed (and is the main reason this is broken at the minute).

Hmm...

In normal ASoC card, it will re-try .probe if necessary cpu/codec was not probed
(= -EPROBE_DEFER in soc_bind_dai_link()).
This is the before state of card was instantiated.
Returning everything to the before state = unregister card is very natural I think...
If possible, we want to add this card driver to deferred_probe_pending_list
in drivers/base/dd.c, but it is over kill ?

Best regards
---
Kuninori Morimoto

[PATCH 0/6] ASoC: rsnd: tidyup .remove method

[Kuninori Morimoto]

Hi Mark

> > > If we unbind CPU here, it still has CARD infomation.
> > > And, kernel doesn't call initialize function when re-bind,
> > > and it will panic when playback
> 
> > Is this open bug ?
> > Or, Renesas sound only ?
> > fsi/rsnd both have this issue.
> 
> I'm not aware of anything here - when we rebuild the card we should be
> reinitializing everything.  It's possible this is just broken for
> everyone though, most people don't unload devices and where they do they
> tend to remove the card first.

These are v2 of unbind/bind issue fixup for ASoC.
It adds new snd_soc_remove_card() and temporally remove card from system.
and rebind it again if next component was binded.

Kuninori Morimoto (6):
      ASoC: rsnd: set device data before snd_soc_register_platform/component
      ASoC: soc-core: indicate unregister debug message once
      ASoC: soc-core: add snd_soc_remove_card()
      ASoC: soc-core: deactivate pins in snd_soc_instantiate_card()
      ASoC: soc-core: call soc_cleanup_card_debugfs() from snd_soc_unregister_card()
      ASoC: soc-core: call snd_soc_remove_card() when component del

 include/sound/soc.h      |    1 +
 sound/soc/sh/rcar/core.c |    4 +--
 sound/soc/soc-core.c     |   78 +++++++++++++++++++++++++++++++++-------------
 3 files changed, 59 insertions(+), 24 deletions(-)

[PATCH 1/6] ASoC: rsnd: set device data before snd_soc_register_platform/component

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Set device data before snd_soc_register_platform/component.
Otherwise, it will use NULL pointer if user calls unbind -> bind or
rmmod -> insmod

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/sh/rcar/core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/sh/rcar/core.c b/sound/soc/sh/rcar/core.c
index 1b53605..110577c5 100644
--- a/sound/soc/sh/rcar/core.c
+++ b/sound/soc/sh/rcar/core.c
@@ -1252,6 +1252,8 @@ static int rsnd_probe(struct platform_device *pdev)
 			goto exit_snd_probe;
 	}
 
+	dev_set_drvdata(dev, priv);
+
 	/*
 	 *	asoc register
 	 */
@@ -1268,8 +1270,6 @@ static int rsnd_probe(struct platform_device *pdev)
 		goto exit_snd_soc;
 	}
 
-	dev_set_drvdata(dev, priv);
-
 	pm_runtime_enable(dev);
 
 	dev_info(dev, "probed\n");
-- 
1.7.9.5

[PATCH 2/6] ASoC: soc-core: indicate unregister debug message once

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Current snd_soc_unregister_card() indicates unregistered debug
message when it was called. But, it should be called only when
it was really unregistered.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 985052b..11c28fd 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -2386,8 +2386,9 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
 		card->instantiated = false;
 		snd_soc_dapm_shutdown(card);
 		soc_cleanup_card_resources(card);
+		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n",
+			card->name);
 	}
-	dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
 
 	return 0;
 }
-- 
1.7.9.5

[PATCH 3/6] ASoC: soc-core: add snd_soc_remove_card()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Added snd_soc_remove_card() is termination method of
snd_soc_instantiate_card()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 11c28fd..39a8cab 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1715,6 +1715,14 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
 
 }
 
+static void snd_soc_remove_card(struct snd_soc_card *card)
+{
+	card->instantiated = false;
+	snd_soc_dapm_shutdown(card);
+	soc_cleanup_card_resources(card);
+}
+
+
 /* removes a socdev */
 static int soc_remove(struct platform_device *pdev)
 {
@@ -2383,9 +2391,7 @@ EXPORT_SYMBOL_GPL(snd_soc_register_card);
 int snd_soc_unregister_card(struct snd_soc_card *card)
 {
 	if (card->instantiated) {
-		card->instantiated = false;
-		snd_soc_dapm_shutdown(card);
-		soc_cleanup_card_resources(card);
+		snd_soc_remove_card(card);
 		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n",
 			card->name);
 	}
-- 
1.7.9.5

[PATCH 4/6] ASoC: soc-core: deactivate pins in snd_soc_instantiate_card()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

snd_soc_instantiate_card() is the final method of snd_soc_register_card().
Deactivate pins to sleep state is related to snd_soc_instantiate_card(),
not snd_soc_register_card()

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |   33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 39a8cab..db7b070 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1642,6 +1642,23 @@ static int snd_soc_instantiate_card(struct snd_soc_card *card)
 	snd_soc_dapm_sync(&card->dapm);
 	mutex_unlock(&card->mutex);
 
+	/* deactivate pins to sleep state */
+	for (i = 0; i < card->num_rtd; i++) {
+		struct snd_soc_pcm_runtime *rtd = &card->rtd[i];
+		struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+		int j;
+
+		for (j = 0; j < rtd->num_codecs; j++) {
+			struct snd_soc_dai *codec_dai = rtd->codec_dais[j];
+
+			if (!codec_dai->active)
+				pinctrl_pm_select_sleep_state(codec_dai->dev);
+		}
+
+		if (!cpu_dai->active)
+			pinctrl_pm_select_sleep_state(cpu_dai->dev);
+	}
+
 	return 0;
 
 probe_aux_dev_err:
@@ -2362,22 +2379,6 @@ int snd_soc_register_card(struct snd_soc_card *card)
 	if (ret != 0)
 		soc_cleanup_card_debugfs(card);
 
-	/* deactivate pins to sleep state */
-	for (i = 0; i < card->num_rtd; i++) {
-		struct snd_soc_pcm_runtime *rtd = &card->rtd[i];
-		struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
-		int j;
-
-		for (j = 0; j < rtd->num_codecs; j++) {
-			struct snd_soc_dai *codec_dai = rtd->codec_dais[j];
-			if (!codec_dai->active)
-				pinctrl_pm_select_sleep_state(codec_dai->dev);
-		}
-
-		if (!cpu_dai->active)
-			pinctrl_pm_select_sleep_state(cpu_dai->dev);
-	}
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_soc_register_card);
-- 
1.7.9.5

[PATCH 5/6] ASoC: soc-core: call soc_cleanup_card_debugfs() from snd_soc_unregister_card()

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

soc_cleanup_card_debugfs() is called from soc_cleanup_card_resources(),
but, it is registered in snd_soc_register_card().
Cleanup function should be called from paired function.

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 sound/soc/soc-core.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index db7b070..eb16b44 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1719,8 +1719,6 @@ static int soc_cleanup_card_resources(struct snd_soc_card *card)
 	/* remove and free each DAI */
 	soc_remove_dai_links(card);
 
-	soc_cleanup_card_debugfs(card);
-
 	/* remove the card */
 	if (card->remove)
 		card->remove(card);
@@ -2393,6 +2391,8 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
 {
 	if (card->instantiated) {
 		snd_soc_remove_card(card);
+		soc_cleanup_card_debugfs(card);
+
 		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n",
 			card->name);
 	}
-- 
1.7.9.5

[PATCH 6/6] ASoC: soc-core: call snd_soc_remove_card() when component del

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

ASoC devices are organized as CPU-CARD-CODEC. Then, CPU/CODEC
are based on component structure. Now, each CARD device knows
connected component. But CARD doesn't notice if connected component
was removed when user called rmmod or unbind in current implementation.
Thus, CARD which lost some components still exist in system.
And then, ALSA sound card will have some problem if user used this CARD
in such timing. This patch temporarily removes CARD from system
if connected component was removed, and re-add it again if some
component was added.

Reported-by: Nguyen Viet Dung <nv-dung@jinso.co.jp>
Reported-by: Bui Duc Phuc <bd-phuc@jinso.co.jp>
Reported-by: Cao Minh Hiep <cm-hiep@jinso.co.jp>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 include/sound/soc.h  |    1 +
 sound/soc/soc-core.c |   26 ++++++++++++++++++++++++++
 2 files changed, 27 insertions(+)

diff --git a/include/sound/soc.h b/include/sound/soc.h
index b4fca9a..a90eff4 100644
--- a/include/sound/soc.h
+++ b/include/sound/soc.h
@@ -1083,6 +1083,7 @@ struct snd_soc_card {
 	struct list_head paths;
 	struct list_head dapm_list;
 	struct list_head dapm_dirty;
+	struct list_head unbinded_list;
 
 	/* Generic DAPM context for the card */
 	struct snd_soc_dapm_context dapm;
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index eb16b44..8d53dae 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -56,6 +56,7 @@ static DEFINE_MUTEX(client_mutex);
 static LIST_HEAD(platform_list);
 static LIST_HEAD(codec_list);
 static LIST_HEAD(component_list);
+static LIST_HEAD(unbinded_card_list);
 
 /*
  * This is a timeout to do a DAPM powerdown after a stream is closed().
@@ -2397,6 +2398,10 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
 			card->name);
 	}
 
+	mutex_lock(&client_mutex);
+	list_del(&card->unbinded_list);
+	mutex_unlock(&client_mutex);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_soc_unregister_card);
@@ -2660,6 +2665,9 @@ EXPORT_SYMBOL_GPL(snd_soc_component_exit_regmap);
 
 static void snd_soc_component_add_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card, *_card;
+	int ret;
+
 	if (!component->write && !component->read) {
 		if (!component->regmap)
 			component->regmap = dev_get_regmap(component->dev, NULL);
@@ -2668,6 +2676,16 @@ static void snd_soc_component_add_unlocked(struct snd_soc_component *component)
 	}
 
 	list_add(&component->list, &component_list);
+
+	/* re-add temporarily removed card if exist */
+	list_for_each_entry_safe(card, _card, &unbinded_card_list,
+				 unbinded_list) {
+		ret = snd_soc_instantiate_card(card);
+		if (ret < 0)
+			continue;
+
+		list_del(&card->unbinded_list);
+	}
 }
 
 static void snd_soc_component_add(struct snd_soc_component *component)
@@ -2685,7 +2703,15 @@ static void snd_soc_component_cleanup(struct snd_soc_component *component)
 
 static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
 {
+	struct snd_soc_card *card = component->card;
+
 	list_del(&component->list);
+
+	/* card is removed temporarily */
+	if (card->instantiated) {
+		list_add(&card->unbinded_list, &unbinded_card_list);
+		snd_soc_remove_card(card);
+	}
 }
 
 static void snd_soc_component_del(struct snd_soc_component *component)
-- 
1.7.9.5

Re: [PATCH 0/6] ASoC: rsnd: tidyup .remove method

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 359 bytes --]

On Mon, Feb 09, 2015 at 08:04:42AM +0000, Kuninori Morimoto wrote:

> These are v2 of unbind/bind issue fixup for ASoC.
> It adds new snd_soc_remove_card() and temporally remove card from system.
> and rebind it again if next component was binded.

As I've said on a number of occasions before please do *not* send new
patch serieses in followup to old ones.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 664 bytes --]

On Mon, Feb 09, 2015 at 01:06:13AM +0000, Kuninori Morimoto wrote:

> It needs more and more change if we solve this issue without above hack,
> then, it is very difficult to check all consistency for all features, all situation.
> I'm afraid that it can be next bug.

Yes, pretty much - that's half of the reason why this is the way it is.
Not many people want to do this and it's hard to get working.

> So, how about this ? if un-loaded component unregister card, and if re-loaded component
> re-register un-loaded card automatically, it is easy, and safety ?

I'm not 100% sure what that would look like - presumably something would
need to remember the card?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/2] ASoC: core: call snd_soc_unregister_card() when component cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 588 bytes --]

On Mon, Feb 09, 2015 at 02:48:09AM +0000, Kuninori Morimoto wrote:

> In normal ASoC card, it will re-try .probe if necessary cpu/codec was not probed
> (= -EPROBE_DEFER in soc_bind_dai_link()).
> This is the before state of card was instantiated.
> Returning everything to the before state = unregister card is very natural I think...
> If possible, we want to add this card driver to deferred_probe_pending_list
> in drivers/base/dd.c, but it is over kill ?

That's something we need to do, we're relying on deferred probe to
instantiate the cards when their components are registered.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/6] ASoC: rsnd: set device data before snd_soc_register_platform/component

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 307 bytes --]

On Mon, Feb 09, 2015 at 08:05:22AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Set device data before snd_soc_register_platform/component.
> Otherwise, it will use NULL pointer if user calls unbind -> bind or
> rmmod -> insmod

Applied, thanks.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 2/6] ASoC: soc-core: indicate unregister debug message once

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 371 bytes --]

On Mon, Feb 09, 2015 at 08:05:45AM +0000, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> Current snd_soc_unregister_card() indicates unregistered debug
> message when it was called. But, it should be called only when
> it was really unregistered.

This doesn't seem to apply against current code, can you please check?

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 0/6] ASoC: rsnd: tidyup .remove method

[Kuninori Morimoto]

Hi Mark

> > These are v2 of unbind/bind issue fixup for ASoC.
> > It adds new snd_soc_remove_card() and temporally remove card from system.
> > and rebind it again if next component was binded.
> 
> As I've said on a number of occasions before please do *not* send new
> patch serieses in followup to old ones.

Ahh, yes indeed.
Sorry about that

Re: [PATCH 2/6] ASoC: soc-core: indicate unregister debug message once

[Kuninori Morimoto]

Hi Mark

> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > Current snd_soc_unregister_card() indicates unregistered debug
> > message when it was called. But, it should be called only when
> > it was really unregistered.
> 
> This doesn't seem to apply against current code, can you please check?

Yes, I will send it soon in other mail thread