| Message ID | 1395129736-11938-9-git-send-email-lars@metafoo.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 98e639fb8a3ed1bf2bd512626c3cfc2992a57113 |
| Headers | show |
On Tue, Mar 18, 2014 at 09:02:11AM +0100, Lars-Peter Clausen wrote: > snd_soc_unregister_component() takes the parent device of the component as a > parameter and then looks up the component based on this. This is a problem if > multiple components are registered for the same parent device. Currently drivers > do not do this, but some drivers register a CPU DAI component and a platform for > the same parent device. This will become a problem once platforms are also made > components. To make sure that snd_soc_unregister_component() will not > accidentally unregister the platform in such a case only consider components > that were registered with snd_soc_register_component(). This is only meant as > short term stopgap solution to be able to continue componentisation. Long term > we'll need something different. > > Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> > --- Do we have any thoughts on what might be a longer term solution here? Is the parent device actually the right thing to pass to snd_soc_unregister_component, it feels like that is probably not going to be enough to accurately identify the component? Thanks, Charles
On Mon, Mar 24, 2014 at 11:18:05AM +0000, Charles Keepax wrote: > On Tue, Mar 18, 2014 at 09:02:11AM +0100, Lars-Peter Clausen wrote: > > snd_soc_unregister_component() takes the parent device of the component as a > > parameter and then looks up the component based on this. This is a problem if > > multiple components are registered for the same parent device. Currently drivers > Do we have any thoughts on what might be a longer term solution > here? Is the parent device actually the right thing to pass to > snd_soc_unregister_component, it feels like that is probably not > going to be enough to accurately identify the component? It's not clear to me that in the longer term it makes sense for anything that is a single struct device to be registering multiple components in the first place. In the short term we register things separately so it makes sense to continue to support that to ease transition but it's not clear to me that it's a good idea.
On 03/24/2014 12:18 PM, Charles Keepax wrote: > On Tue, Mar 18, 2014 at 09:02:11AM +0100, Lars-Peter Clausen wrote: >> snd_soc_unregister_component() takes the parent device of the component as a >> parameter and then looks up the component based on this. This is a problem if >> multiple components are registered for the same parent device. Currently drivers >> do not do this, but some drivers register a CPU DAI component and a platform for >> the same parent device. This will become a problem once platforms are also made >> components. To make sure that snd_soc_unregister_component() will not >> accidentally unregister the platform in such a case only consider components >> that were registered with snd_soc_register_component(). This is only meant as >> short term stopgap solution to be able to continue componentisation. Long term >> we'll need something different. >> >> Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> >> --- > > Do we have any thoughts on what might be a longer term solution > here? Is the parent device actually the right thing to pass to > snd_soc_unregister_component, it feels like that is probably not > going to be enough to accurately identify the component? > > Thanks, > Charles > My plan is to let it take a pointer to the component itself. I'm just not too sure about the implementation details yet. - Lars
On Mon, Mar 24, 2014 at 12:40:49PM +0100, Lars-Peter Clausen wrote: > My plan is to let it take a pointer to the component itself. I'm just not > too sure about the implementation details yet. The question there is why you'd ever want to only unregister a single thing that the device registered. devm_ does make this a bit more academic but still.
On 03/24/2014 12:48 PM, Mark Brown wrote: > On Mon, Mar 24, 2014 at 12:40:49PM +0100, Lars-Peter Clausen wrote: > >> My plan is to let it take a pointer to the component itself. I'm just not >> too sure about the implementation details yet. > > The question there is why you'd ever want to only unregister a single > thing that the device registered. devm_ does make this a bit more > academic but still. > I you probably wouldn't, but I think it makes the interface cleaner and devm doesn't really care whether we store a pointer to the parent device or a pointer to the component in the devm data. - Lars
On Mon, Mar 24, 2014 at 01:07:08PM +0100, Lars-Peter Clausen wrote: > On 03/24/2014 12:48 PM, Mark Brown wrote: > >The question there is why you'd ever want to only unregister a single > >thing that the device registered. devm_ does make this a bit more > >academic but still. > I you probably wouldn't, but I think it makes the interface cleaner and devm > doesn't really care whether we store a pointer to the parent device or a > pointer to the component in the devm data. Yes, of course the other bit is not wanting to register multiple components in the first place (partly for this reason).
diff --git a/include/sound/soc.h b/include/sound/soc.h index a355d0f..f8a79c1 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -662,6 +662,7 @@ struct snd_soc_component { unsigned int active; unsigned int ignore_pmdown_time:1; /* pmdown_time is ignored at stop */ + unsigned int registered_as_component:1; struct list_head list; diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4df9859..1ee3dd6 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -3885,6 +3885,7 @@ int snd_soc_register_component(struct device *dev, } cmpnt->ignore_pmdown_time = true; + cmpnt->registered_as_component = true; return __snd_soc_register_component(dev, cmpnt, cmpnt_drv, NULL, dai_drv, num_dai, true); @@ -3900,7 +3901,7 @@ void snd_soc_unregister_component(struct device *dev) struct snd_soc_component *cmpnt; list_for_each_entry(cmpnt, &component_list, list) { - if (dev == cmpnt->dev) + if (dev == cmpnt->dev && cmpnt->registered_as_component) goto found; } return;
snd_soc_unregister_component() takes the parent device of the component as a parameter and then looks up the component based on this. This is a problem if multiple components are registered for the same parent device. Currently drivers do not do this, but some drivers register a CPU DAI component and a platform for the same parent device. This will become a problem once platforms are also made components. To make sure that snd_soc_unregister_component() will not accidentally unregister the platform in such a case only consider components that were registered with snd_soc_register_component(). This is only meant as short term stopgap solution to be able to continue componentisation. Long term we'll need something different. Signed-off-by: Lars-Peter Clausen <lars@metafoo.de> --- include/sound/soc.h | 1 + sound/soc/soc-core.c | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-)