[alsa-devel] [PATCH 11/21] ASoC: soc-core: add snd_soc_del_component()

[Pierre-Louis Bossart]

On 10/8/19 11:30 PM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> 
> It is easy to read code if it is cleanly using paired function/naming,
> like start <-> stop, register <-> unregister, etc, etc.
> But, current ALSA SoC code is very random, unbalance, not paired, etc.
> It is easy to create bug at the such code, and is difficult to debug.
> 
> Now ALSA SoC has snd_soc_add_component(), but there is no paired
> snd_soc_del_component(). Thus, snd_soc_unregister_component() is
> calling cleanup function randomly. it is difficult to read.
> This patch adds missing snd_soc_del_component() and balance up code.


the problem now is that the naming is confusing

we have snd_soc_component_del and snd_soc_del_component.
we already had snd_soc_component_add and snd_soc_add_component.

Also I find it useful to keep the _unlocked suffix when relevant, it 
adds value that is lost otherwise.

Can we avoid this pretty please?

> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
>   sound/soc/soc-core.c | 20 +++++++++-----------
>   1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index 72eb59c..7c0bb32 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -2767,12 +2767,7 @@ static void snd_soc_component_add(struct snd_soc_component *component)
>   	mutex_unlock(&client_mutex);
>   }
>   
> -static void snd_soc_component_cleanup(struct snd_soc_component *component)
> -{
> -	snd_soc_unregister_dais(component);
> -}
> -
> -static void snd_soc_component_del_unlocked(struct snd_soc_component *component)
> +static void snd_soc_component_del(struct snd_soc_component *component)
>   {
>   	struct snd_soc_card *card = component->card;
>   
> @@ -2826,6 +2821,12 @@ static void snd_soc_try_rebind_card(void)
>   			list_del(&card->list);
>   }
>   
> +static void snd_soc_del_component(struct snd_soc_component *component)
> +{
> +	snd_soc_unregister_dais(component);
> +	snd_soc_component_del(component);
> +}
> +
>   int snd_soc_add_component(struct device *dev,
>   			struct snd_soc_component *component,
>   			const struct snd_soc_component_driver *component_driver,
> @@ -2858,7 +2859,7 @@ int snd_soc_add_component(struct device *dev,
>   	return 0;
>   
>   err_cleanup:
> -	snd_soc_component_cleanup(component);
> +	snd_soc_del_component(component);
>   err_free:
>   	return ret;
>   }
> @@ -2896,15 +2897,12 @@ static int __snd_soc_unregister_component(struct device *dev)
>   		if (dev != component->dev)
>   			continue;
>   
> -		snd_soc_component_del_unlocked(component);
> +		snd_soc_del_component(component);
>   		found = 1;
>   		break;
>   	}
>   	mutex_unlock(&client_mutex);
>   
> -	if (found)
> -		snd_soc_component_cleanup(component);
> -
>   	return found;
>   }
>   
>