From mboxrd@z Thu Jan 1 00:00:00 1970 From: Takashi Iwai Subject: Re: [RFC] bind/unbind SoC devices with deferred list Date: Wed, 22 Apr 2015 14:02:13 +0200 Message-ID: References: <87383sh7i6.wl%kuninori.morimoto.gx@renesas.com> Mime-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by alsa0.perex.cz (Postfix) with ESMTP id CF2AD26057F for ; Wed, 22 Apr 2015 14:02:16 +0200 (CEST) In-Reply-To: <87383sh7i6.wl%kuninori.morimoto.gx@renesas.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: alsa-devel-bounces@alsa-project.org Sender: alsa-devel-bounces@alsa-project.org To: Kuninori Morimoto Cc: Linux-ALSA , Mark Brown , Lars-Peter Clausen List-Id: alsa-devel@alsa-project.org 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 > Date: Wed, 22 Apr 2015 16:53:49 +0900 > Subject: [PATCH] no Subject at this point > > Signed-off-by: Kuninori Morimoto > --- > 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 > #include > #include > #include > @@ -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 >