[alsa-devel] [RFC] bind/unbind SoC devices with deferred list
Takashi Iwai
tiwai at suse.de
Wed Apr 22 14:02:13 CEST 2015
At Wed, 22 Apr 2015 08:51:21 +0000,
Kuninori Morimoto wrote:
>
>
> Hi Takashi, Mark, Lars
>
> I worked for bind/unbind issue which was discussed before
>
> http://thread.gmane.org/gmane.linux.alsa.devel/134062/focus=134076
> http://thread.gmane.org/gmane.linux.alsa.devel/134283/focus=134287
>
> I would like to know your opinion before sending patch to Greg
>
> The problem is ASoC has card/platform/cpu/codec relationship for each other,
> but, we can unbind one of them, and then, other related devices doesn't know
> about it. Thus, kernel will error if we use sound after that.
Wouldn't the standard component (linux/component.h) be able to solve
such a problem?
Takashi
>
> Here is example of this issue.
> It unbind cpu (= rcar_sound), playback sound, and, error.
> And, we can't re-bind after this.
>
> // check current sound 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
>
> // unbind cpu (= rcar_sound)
>
> > echo ec500000.rcar_sound > /sys/bus/platform/drivers/rcar_sound/unbind
>
> // recheck sound card
> // card doesn't know cpu was removed
>
> > 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
>
> // kernel error happen if playback
>
> > aplay xxx.wav
> Unable to handle kernel NULL pointer dereference at virtual address 00000100
> pgd = ee97c000
> ...
>
> // we can't re-bind cpu
>
> > echo ec500000.rcar_sound > /sys/bus/platform/drivers/rcar_sound/bind
> -- freeze --
>
> I hacked this issue to using deferred list, and created attached patch.
> it is all-in-one patch. but it solved my issue.
> I would like to know your opinion about this.
>
> // check current sound 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
>
> // unbind cpu (= rcar_sound)
>
> > echo ec500000.rcar_sound > /sys/bus/platform/drivers/rcar_sound/unbind
> (local debug message) release ec500000.rcar_sound
> (local debug message) release related device - sound
> (local debug message) related related device - 6-0012
>
> // recheck sound card
> // it knows there is no available card
>
> > aplay -l
> aplay: device_list:268: no soundcards found...
>
> // re-bind lost cpu (= rcar_sound)
> // it re-bind sound card
>
> > echo ec500000.rcar_sound > /sys/bus/platform/drivers/rcar_sound/bind
> rcar_sound ec500000.rcar_sound: probed
> asoc-simple-card sound: ak4642-hifi <-> ec500000.rcar_sound mapping ok
>
> // let's unbind codec side (= ak4642) this time
>
> > echo 6-0012 > /sys/bus/i2c/drivers/ak4642-codec/unbind
> (local debug message) release 6-0012
> (local debug message) release related device - ec500000.rcar_sound
> (local debug message) related related device - sound
>
> // recheck sound card
> // it knows there is no available card
>
> > aplay -l
> aplay: device_list:268: no soundcards found...
>
> // re-bind lost codec (= ak4642)
> // it re-bind sound card
>
> > echo 6-0012 > /sys/bus/i2c/drivers/ak4642-codec/bind
> rcar_sound ec500000.rcar_sound: probed
> asoc-simple-card sound: ak4642-hifi <-> ec500000.rcar_sound mapping ok
>
>
> --- patch ---
> From 9da5324a659ff596a2733f14b768b6b8c5a04f3b Mon Sep 17 00:00:00 2001
> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> Date: Wed, 22 Apr 2015 16:53:49 +0900
> Subject: [PATCH] no Subject at this point
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
> drivers/base/base.h | 1 +
> drivers/base/core.c | 9 +++++++++
> drivers/base/dd.c | 21 ++++++++++++++++++++-
> include/linux/device.h | 1 +
> sound/soc/soc-core.c | 4 ++++
> 5 files changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 251c5d3..1a71a52 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -74,6 +74,7 @@ struct device_private {
> struct klist_node knode_driver;
> struct klist_node knode_bus;
> struct list_head deferred_probe;
> + struct list_head related_dev;
> struct device *device;
> };
> #define to_device_private_parent(obj) \
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 21d1303..643146f 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -948,9 +948,18 @@ int device_private_init(struct device *dev)
> klist_init(&dev->p->klist_children, klist_children_get,
> klist_children_put);
> INIT_LIST_HEAD(&dev->p->deferred_probe);
> + INIT_LIST_HEAD(&dev->p->related_dev);
> +
> return 0;
> }
>
> +void device_related_device(struct device *dev, struct device *node)
> +{
> + if (list_empty(&node->p->related_dev))
> + list_add(&node->p->related_dev, &dev->p->related_dev);
> +}
> +EXPORT_SYMBOL_GPL(device_related_device);
> +
> /**
> * device_add - add device to device hierarchy.
> * @dev: device.
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index e843fdb..3d218c6 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -516,7 +516,7 @@ EXPORT_SYMBOL_GPL(driver_attach);
> * __device_release_driver() must be called with @dev lock held.
> * When called for a USB interface, @dev->parent lock must be held as well.
> */
> -static void __device_release_driver(struct device *dev)
> +static void _device_release_driver(struct device *dev)
> {
> struct device_driver *drv;
>
> @@ -552,6 +552,25 @@ static void __device_release_driver(struct device *dev)
> }
> }
>
> +static void __device_release_driver(struct device *dev)
> +{
> + struct device_private *dp, *d;
> +
> + /* release dev itself */
> + _device_release_driver(dev);
> +
> + list_for_each_entry_safe(dp, d, &dev->p->related_dev, related_dev) {
> + /*
> + * temporarily, release related device.
> + * these are push to deferred_probe_pending_list.
> + * It can be re-probed if it didn't call device_del()
> + */
> + _device_release_driver(dp->device);
> + driver_deferred_probe_add(dp->device);
> + list_del_init(&dp->related_dev);
> + }
> +}
> +
> /**
> * device_release_driver - manually detach device from driver.
> * @dev: device.
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 6558af9..2b81804 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -924,6 +924,7 @@ void driver_init(void);
> extern int __must_check device_register(struct device *dev);
> extern void device_unregister(struct device *dev);
> extern void device_initialize(struct device *dev);
> +extern void device_related_device(struct device *dev, struct device *node);
> extern int __must_check device_add(struct device *dev);
> extern void device_del(struct device *dev);
> extern int device_for_each_child(struct device *dev, void *data,
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 2373252..01c0d56 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -22,6 +22,7 @@
> * o Support TDM on PCM and I2S
> */
>
> +#include <linux/device.h>
> #include <linux/module.h>
> #include <linux/moduleparam.h>
> #include <linux/init.h>
> @@ -946,6 +947,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
> dai_link->cpu_dai_name);
> return -EPROBE_DEFER;
> }
> + device_related_device(card->dev, rtd->cpu_dai->dev);
>
> rtd->num_codecs = dai_link->num_codecs;
>
> @@ -962,6 +964,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
> /* Single codec links expect codec and codec_dai in runtime data */
> rtd->codec_dai = codec_dais[0];
> rtd->codec = rtd->codec_dai->codec;
> + device_related_device(card->dev, rtd->codec->dev);
>
> /* if there's no platform we match on the empty platform */
> platform_name = dai_link->platform_name;
> @@ -986,6 +989,7 @@ static int soc_bind_dai_link(struct snd_soc_card *card, int num)
> dai_link->platform_name);
> return -EPROBE_DEFER;
> }
> + device_related_device(card->dev, rtd->platform->dev);
>
> card->num_rtd++;
>
> --
> 1.9.1
>
>
>
>
> Best regards
> ---
> Kuninori Morimoto
>
More information about the Alsa-devel
mailing list