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

[Lars-Peter Clausen]

On 02/09/2015 02:07 PM, Takashi Iwai wrote:
> At Mon, 09 Feb 2015 12:26:02 +0100,
> Lars-Peter Clausen wrote:
>>
>> On 02/09/2015 11:48 AM, Takashi Iwai wrote:
>>> At Mon, 9 Feb 2015 08:52:49 +0000,
>>> Kuninori Morimoto wrote:
>>>>
>>>> From: Kuninori Morimoto <kuninori.morimoto.gx at 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 if some component
>>>> was added.
>>>>
>>>> Reported-by: Nguyen Viet Dung <nv-dung at jinso.co.jp>
>>>> Reported-by: Bui Duc Phuc <bd-phuc at jinso.co.jp>
>>>> Reported-by: Cao Minh Hiep <cm-hiep at jinso.co.jp>
>>>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at 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 b7ab676..f8d5498 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().
>>>> @@ -2406,6 +2407,10 @@ int snd_soc_unregister_card(struct snd_soc_card *card)
>>>>    		dev_dbg(card->dev, "ASoC: Unregistered card '%s'\n", card->name);
>>>>    	}
>>>>
>>>> +	mutex_lock(&client_mutex);
>>>> +	list_del(&card->unbinded_list);
>>>> +	mutex_unlock(&client_mutex);
>>>> +
>>>>    	return 0;
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(snd_soc_unregister_card);
>>>> @@ -2669,6 +2674,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);
>>>> @@ -2677,6 +2685,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);
>>>> +	}
>>>
>>> [Cc'ed Lars-Peter]
>>>
>>> This would instantiate a card even if it's irrelevant with the given
>>> component?  If so, it looks fragile, and possibly racy, when there are
>>> multiple cards.
>>
>> That shouldn't be a problem. snd_soc_instantiate_card() does the proper
>> locking and will return an error if not all components are ready yet.
>
> But there is no check of card->instantiated there, so the whole path
> can be called again even if the card was already instantiated?

The card won't be on the list of unbound cards if it is not instantiated. 
And this whole section is protected by the client_mutex.

- Lars