Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1245720 > unrolled thread

[RFC PATCH] ASoC: Modify check condition of multiple bindings of components

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH] ASoC: Modify check condition of multiple bindings of components Koro Chen <koro.chen@mediatek.com> - 2015-10-13 15:40 +0200
    Re: [RFC PATCH] ASoC: Modify check condition of multiple bindings  of components Lars-Peter Clausen <lars@metafoo.de> - 2015-10-13 15:50 +0200
      Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of  multiple bindings of components Koro Chen <koro.chen@mediatek.com> - 2015-10-13 16:20 +0200
        Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple  bindings of components Lars-Peter Clausen <lars@metafoo.de> - 2015-10-13 16:50 +0200
          Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of  multiple bindings of components Koro Chen <koro.chen@mediatek.com> - 2015-10-14 03:20 +0200
            Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple  bindings of components Lars-Peter Clausen <lars@metafoo.de> - 2015-10-14 11:20 +0200
              Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of  multiple bindings of components Mark Brown <broonie@kernel.org> - 2015-10-15 14:20 +0200
                Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of  multiple bindings of components Koro Chen <koro.chen@mediatek.com> - 2015-10-15 14:50 +0200
                Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple  bindings of components Lars-Peter Clausen <lars@metafoo.de> - 2015-10-15 15:30 +0200
                  Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of  multiple bindings of components Koro Chen <koro.chen@mediatek.com> - 2015-10-16 04:40 +0200

#1245720 — [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

The patch "ASoC: Prevent components from being bound to multiple cards"
adds check to prevent multiple cards from using the same component.
However, snd_soc_register_platform() or snd_soc_register_codec() will
also create components, and sharing the same platform by multiple cards
is then refused. This happens with a platform having multiple
independent DAIs that share the same DMA controller.

Relax the condition by checking component->registered_as_component,
which is only true in case of snd_soc_register_component() and
will be false for components created by snd_soc_register_platform()
or snd_soc_register_codec().

Signed-off-by: Koro Chen <koro.chen@mediatek.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 42575b0..eca169a 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1106,7 +1106,8 @@ static int soc_probe_component(struct snd_soc_card *card,
 		return 0;
 
 	if (component->card) {
-		if (component->card != card) {
+		if (component->card != card &&
+		    component->registered_as_component) {
 			dev_err(component->dev,
 				"Trying to bind component to card \"%s\" but is already bound to card \"%s\"\n",
 				card->name, component->card->name);
-- 
1.7.9.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1245730 — Re: [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On 10/13/2015 03:37 PM, Koro Chen wrote:
> The patch "ASoC: Prevent components from being bound to multiple cards"
> adds check to prevent multiple cards from using the same component.
> However, snd_soc_register_platform() or snd_soc_register_codec() will
> also create components, and sharing the same platform by multiple cards
> is then refused. This happens with a platform having multiple
> independent DAIs that share the same DMA controller.
> 
> Relax the condition by checking component->registered_as_component,
> which is only true in case of snd_soc_register_component() and
> will be false for components created by snd_soc_register_platform()
> or snd_soc_register_codec().

Binding a component to multiple cards results in internal data structure
corruption, regardless of whether it is a raw component, CODEC or platform,
which is why the check was added. So the proposed change wont work.

> 
> Signed-off-by: Koro Chen <koro.chen@mediatek.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 42575b0..eca169a 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1106,7 +1106,8 @@ static int soc_probe_component(struct snd_soc_card *card,
>  		return 0;
>  
>  	if (component->card) {
> -		if (component->card != card) {
> +		if (component->card != card &&
> +		    component->registered_as_component) {
>  			dev_err(component->dev,
>  				"Trying to bind component to card \"%s\" but is already bound to card \"%s\"\n",
>  				card->name, component->card->name);
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245766 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On Tue, 2015-10-13 at 15:44 +0200, Lars-Peter Clausen wrote:
> On 10/13/2015 03:37 PM, Koro Chen wrote:
> > The patch "ASoC: Prevent components from being bound to multiple cards"
> > adds check to prevent multiple cards from using the same component.
> > However, snd_soc_register_platform() or snd_soc_register_codec() will
> > also create components, and sharing the same platform by multiple cards
> > is then refused. This happens with a platform having multiple
> > independent DAIs that share the same DMA controller.
> > 
> > Relax the condition by checking component->registered_as_component,
> > which is only true in case of snd_soc_register_component() and
> > will be false for components created by snd_soc_register_platform()
> > or snd_soc_register_codec().
> 
> Binding a component to multiple cards results in internal data structure
> corruption, regardless of whether it is a raw component, CODEC or platform,
> which is why the check was added. So the proposed change wont work.
> 
Thanks for your comment. Is it possible to share an example of how the
data structure will be corrupted? So I can study into this further.
 
> > 
> > Signed-off-by: Koro Chen <koro.chen@mediatek.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 42575b0..eca169a 100644
> > --- a/sound/soc/soc-core.c
> > +++ b/sound/soc/soc-core.c
> > @@ -1106,7 +1106,8 @@ static int soc_probe_component(struct snd_soc_card *card,
> >  		return 0;
> >  
> >  	if (component->card) {
> > -		if (component->card != card) {
> > +		if (component->card != card &&
> > +		    component->registered_as_component) {
> >  			dev_err(component->dev,
> >  				"Trying to bind component to card \"%s\" but is already bound to card \"%s\"\n",
> >  				card->name, component->card->name);
> > 
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1245805 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On 10/13/2015 04:18 PM, Koro Chen wrote:
> On Tue, 2015-10-13 at 15:44 +0200, Lars-Peter Clausen wrote:
>> On 10/13/2015 03:37 PM, Koro Chen wrote:
>>> The patch "ASoC: Prevent components from being bound to multiple cards"
>>> adds check to prevent multiple cards from using the same component.
>>> However, snd_soc_register_platform() or snd_soc_register_codec() will
>>> also create components, and sharing the same platform by multiple cards
>>> is then refused. This happens with a platform having multiple
>>> independent DAIs that share the same DMA controller.
>>>
>>> Relax the condition by checking component->registered_as_component,
>>> which is only true in case of snd_soc_register_component() and
>>> will be false for components created by snd_soc_register_platform()
>>> or snd_soc_register_codec().
>>
>> Binding a component to multiple cards results in internal data structure
>> corruption, regardless of whether it is a raw component, CODEC or platform,
>> which is why the check was added. So the proposed change wont work.
>>
> Thanks for your comment. Is it possible to share an example of how the
> data structure will be corrupted? So I can study into this further.

Just look at soc_probe_component() and think about what happens if that runs
twice for two different cards. Multiple calls to list_add() on the same
list, controls are added multiple times, DAPM widgets are created multiple
times, the card field will only point to the last card.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246237 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On Tue, 2015-10-13 at 16:42 +0200, Lars-Peter Clausen wrote:
> On 10/13/2015 04:18 PM, Koro Chen wrote:
> > On Tue, 2015-10-13 at 15:44 +0200, Lars-Peter Clausen wrote:
> >> On 10/13/2015 03:37 PM, Koro Chen wrote:
> >>> The patch "ASoC: Prevent components from being bound to multiple cards"
> >>> adds check to prevent multiple cards from using the same component.
> >>> However, snd_soc_register_platform() or snd_soc_register_codec() will
> >>> also create components, and sharing the same platform by multiple cards
> >>> is then refused. This happens with a platform having multiple
> >>> independent DAIs that share the same DMA controller.
> >>>
> >>> Relax the condition by checking component->registered_as_component,
> >>> which is only true in case of snd_soc_register_component() and
> >>> will be false for components created by snd_soc_register_platform()
> >>> or snd_soc_register_codec().
> >>
> >> Binding a component to multiple cards results in internal data structure
> >> corruption, regardless of whether it is a raw component, CODEC or platform,
> >> which is why the check was added. So the proposed change wont work.
> >>
> > Thanks for your comment. Is it possible to share an example of how the
> > data structure will be corrupted? So I can study into this further.
> 
> Just look at soc_probe_component() and think about what happens if that runs
> twice for two different cards. Multiple calls to list_add() on the same
> list, controls are added multiple times, DAPM widgets are created multiple
> times, the card field will only point to the last card.
> 
When multiple binding happens, soc_probe_component() just returns zero
without doing anything after this patch (actually it also returned zero
before the patch "ASoC: Prevent components from being bound to multiple
cards"). So the component still binds to the first card. For this case I
think it should be fine?

> 
> _______________________________________________
> Linux-mediatek mailing list
> Linux-mediatek@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1246445 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On 10/14/2015 03:19 AM, Koro Chen wrote:
> On Tue, 2015-10-13 at 16:42 +0200, Lars-Peter Clausen wrote:
>> On 10/13/2015 04:18 PM, Koro Chen wrote:
>>> On Tue, 2015-10-13 at 15:44 +0200, Lars-Peter Clausen wrote:
>>>> On 10/13/2015 03:37 PM, Koro Chen wrote:
>>>>> The patch "ASoC: Prevent components from being bound to multiple cards"
>>>>> adds check to prevent multiple cards from using the same component.
>>>>> However, snd_soc_register_platform() or snd_soc_register_codec() will
>>>>> also create components, and sharing the same platform by multiple cards
>>>>> is then refused. This happens with a platform having multiple
>>>>> independent DAIs that share the same DMA controller.
>>>>>
>>>>> Relax the condition by checking component->registered_as_component,
>>>>> which is only true in case of snd_soc_register_component() and
>>>>> will be false for components created by snd_soc_register_platform()
>>>>> or snd_soc_register_codec().
>>>>
>>>> Binding a component to multiple cards results in internal data structure
>>>> corruption, regardless of whether it is a raw component, CODEC or platform,
>>>> which is why the check was added. So the proposed change wont work.
>>>>
>>> Thanks for your comment. Is it possible to share an example of how the
>>> data structure will be corrupted? So I can study into this further.
>>
>> Just look at soc_probe_component() and think about what happens if that runs
>> twice for two different cards. Multiple calls to list_add() on the same
>> list, controls are added multiple times, DAPM widgets are created multiple
>> times, the card field will only point to the last card.
>>
> When multiple binding happens, soc_probe_component() just returns zero
> without doing anything after this patch (actually it also returned zero
> before the patch "ASoC: Prevent components from being bound to multiple
> cards"). So the component still binds to the first card. For this case I
> think it should be fine?

Same answer as before more or less. There is a lot of card specific
initialization happening in soc_probe_component(). The card field is
initialized to the card the component is bound to, the DAPM widgets are
created for the component in the cards DAPM graph, DAI link widgets are
created and so on. If you bind the component to multiple cards the
initialization will only be done for one card, the first one it was bound
to, that doesn't make too much sense. If you look at the snd_soc_component
data structure it should be obvious that it does not make sense to associate
it with more than a single card.

It was never intended that it is possible to bind a component to multiple
cards. That it was possible was a bug that was overlooked and some people
tried to do it which caused apparently random crashes later on, caused by
the data structure corruption. This is why we added the check to catch this
kind of mistake early and to avoid the crashes.

- Lars

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247749 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

[Multipart message — attachments visible in raw view] — view raw

On Wed, Oct 14, 2015 at 11:00:01AM +0200, Lars-Peter Clausen wrote:

> It was never intended that it is possible to bind a component to multiple
> cards. That it was possible was a bug that was overlooked and some people
> tried to do it which caused apparently random crashes later on, caused by
> the data structure corruption. This is why we added the check to catch this
> kind of mistake early and to avoid the crashes.

This is true, but I do think it's something that we should have some
story on supporting for some of this hardware that has a bunch of
channels in one IP block that can't really interact with each other.
It's going to make it a lot easier for people to think about the
hardware and how to describe it.

[toc] | [prev] | [next] | [standalone]

#1247774 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On Thu, 2015-10-15 at 13:10 +0100, Mark Brown wrote:
> On Wed, Oct 14, 2015 at 11:00:01AM +0200, Lars-Peter Clausen wrote:
> 
> > It was never intended that it is possible to bind a component to multiple
> > cards. That it was possible was a bug that was overlooked and some people
> > tried to do it which caused apparently random crashes later on, caused by
> > the data structure corruption. This is why we added the check to catch this
> > kind of mistake early and to avoid the crashes.
> 
> This is true, but I do think it's something that we should have some
> story on supporting for some of this hardware that has a bunch of
> channels in one IP block that can't really interact with each other.
> It's going to make it a lot easier for people to think about the
> hardware and how to describe it.
Yes, and if in some cases we must use multiple cards, it seems the only
option left is to separate an ASoC platform driver into multiple
drivers. It does not make sense to me since there is only one HW block.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1247791 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

[Multipart message — attachments visible in raw view] — view raw

On 10/15/2015 02:10 PM, Mark Brown wrote:
> On Wed, Oct 14, 2015 at 11:00:01AM +0200, Lars-Peter Clausen wrote:
> 
>> It was never intended that it is possible to bind a component to multiple
>> cards. That it was possible was a bug that was overlooked and some people
>> tried to do it which caused apparently random crashes later on, caused by
>> the data structure corruption. This is why we added the check to catch this
>> kind of mistake early and to avoid the crashes.
> 
> This is true, but I do think it's something that we should have some
> story on supporting for some of this hardware that has a bunch of
> channels in one IP block that can't really interact with each other.
> It's going to make it a lot easier for people to think about the
> hardware and how to describe it.

I'm not saying we shouldn't support it, just that we can't support it with
the current code. And adding support for it will require a fair bit of
restructuring.

If a hardware block as multiple independent channels the best approach in my
opinion is to register multiple components (Which we can't do at the moment,
because there can only be one component per device). From a framework point
of view there is no difference between a single device with multiple
independent channels and multiple independent devices with one channel each.
Both have the same logical topology.

- Lars

[toc] | [prev] | [next] | [standalone]

#1248311 — Re: [alsa-devel] [RFC PATCH] ASoC: Modify check condition of multiple bindings of components

On Thu, 2015-10-15 at 15:26 +0200, Lars-Peter Clausen wrote:
> On 10/15/2015 02:10 PM, Mark Brown wrote:
> > On Wed, Oct 14, 2015 at 11:00:01AM +0200, Lars-Peter Clausen wrote:
> > 
> >> It was never intended that it is possible to bind a component to multiple
> >> cards. That it was possible was a bug that was overlooked and some people
> >> tried to do it which caused apparently random crashes later on, caused by
> >> the data structure corruption. This is why we added the check to catch this
> >> kind of mistake early and to avoid the crashes.
> > 
> > This is true, but I do think it's something that we should have some
> > story on supporting for some of this hardware that has a bunch of
> > channels in one IP block that can't really interact with each other.
> > It's going to make it a lot easier for people to think about the
> > hardware and how to describe it.
> 
> I'm not saying we shouldn't support it, just that we can't support it with
> the current code. And adding support for it will require a fair bit of
> restructuring.
> 
> If a hardware block as multiple independent channels the best approach in my
> opinion is to register multiple components (Which we can't do at the moment,
> because there can only be one component per device). From a framework point
Yes... I have tried to register 2 platforms in my ASoC platform driver
but alsa considered they are the same platform since they are from the
same device.
> of view there is no difference between a single device with multiple
> independent channels and multiple independent devices with one channel each.
> Both have the same logical topology.
> 
Yes, but in my case there is only one HW, one set of registers, and one
set of clocks, it should be a single device node in the device tree. 
> - Lars
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web