mbox series

[00/11] ASoC: topology: Fixes and cleanups

Message ID 20230125194649.3485731-1-amadeuszx.slawinski@linux.intel.com
Headers show
Series ASoC: topology: Fixes and cleanups | expand

Message

Amadeusz Sławiński Jan. 25, 2023, 7:46 p.m. UTC
Following is series of fixes and cleanups for core topology code. Few
patches fixing various problems all around and few fixing function
names.

Amadeusz Sławiński (11):
  ASoC: topology: Properly access value coming from topology file
  ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
  ASoC: topology: Fix typo in functions name
  ASoC: topology: Fix function name
  ASoC: topology: Rename remove_ handlers
  ASoC: topology: Remove unnecessary forward declarations
  ASoC: topology: Pass correct pointer instead of casting
  ASoC: topology: Return an error on complete() failure
  ASoC: Topology: Remove unnecessary check for EOF
  ASoC: topology: Use unload() op directly
  ASoC: topology: Unify kcontrol removal code

 include/sound/soc-topology.h |   2 +-
 sound/soc/soc-topology.c     | 183 ++++++++++++++---------------------
 2 files changed, 74 insertions(+), 111 deletions(-)

Comments

Pierre-Louis Bossart Jan. 25, 2023, 3:15 p.m. UTC | #1
On 1/25/23 13:46, Amadeusz Sławiński wrote:
> Functions removing bytes, enum and mixer kcontrols are identical. Unify

they are identical because of the change in patch10.

Please clarify that this is not a cleanup removing duplicated code
that's been there forever, it's become useless as a result of the
previous patch.

> them under one function and use it to free associated kcontrols.
> 
> Reviewed-by: Cezary Rojewski <cezary.rojewski@intel.com>
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> ---
>  sound/soc/soc-topology.c | 48 +++++-----------------------------------
>  1 file changed, 6 insertions(+), 42 deletions(-)
> 
> diff --git a/sound/soc/soc-topology.c b/sound/soc/soc-topology.c
> index eb49037d86ae..e66b0d9e387a 100644
> --- a/sound/soc/soc-topology.c
> +++ b/sound/soc/soc-topology.c
> @@ -350,41 +350,9 @@ static int soc_tplg_add_kcontrol(struct soc_tplg *tplg,
>  				tplg->dev, k, comp->name_prefix, comp, kcontrol);
>  }
>  
> -/* remove a mixer kcontrol */
> -static void soc_tplg_remove_mixer(struct snd_soc_component *comp,
> -	struct snd_soc_dobj *dobj, int pass)
> -{
> -	struct snd_card *card = comp->card->snd_card;
> -
> -	if (pass != SOC_TPLG_PASS_CONTROL)
> -		return;
> -
> -	if (dobj->unload)
> -		dobj->unload(comp, dobj);
> -
> -	snd_ctl_remove(card, dobj->control.kcontrol);
> -	list_del(&dobj->list);
> -}
> -
> -/* remove an enum kcontrol */
> -static void soc_tplg_remove_enum(struct snd_soc_component *comp,
> -	struct snd_soc_dobj *dobj, int pass)
> -{
> -	struct snd_card *card = comp->card->snd_card;
> -
> -	if (pass != SOC_TPLG_PASS_CONTROL)
> -		return;
> -
> -	if (dobj->unload)
> -		dobj->unload(comp, dobj);
> -
> -	snd_ctl_remove(card, dobj->control.kcontrol);
> -	list_del(&dobj->list);
> -}
> -
> -/* remove a byte kcontrol */
> -static void soc_tplg_remove_bytes(struct snd_soc_component *comp,
> -	struct snd_soc_dobj *dobj, int pass)
> +/* remove kcontrol */
> +static void soc_tplg_remove_kcontrol(struct snd_soc_component *comp, struct snd_soc_dobj *dobj,
> +				     int pass)
>  {
>  	struct snd_card *card = comp->card->snd_card;
>  
> @@ -2626,14 +2594,10 @@ int snd_soc_tplg_component_remove(struct snd_soc_component *comp)
>  			list) {
>  
>  			switch (dobj->type) {
> -			case SND_SOC_DOBJ_MIXER:
> -				soc_tplg_remove_mixer(comp, dobj, pass);
> -				break;
> -			case SND_SOC_DOBJ_ENUM:
> -				soc_tplg_remove_enum(comp, dobj, pass);
> -				break;
>  			case SND_SOC_DOBJ_BYTES:
> -				soc_tplg_remove_bytes(comp, dobj, pass);
> +			case SND_SOC_DOBJ_ENUM:
> +			case SND_SOC_DOBJ_MIXER:
> +				soc_tplg_remove_kcontrol(comp, dobj, pass);
>  				break;
>  			case SND_SOC_DOBJ_GRAPH:
>  				soc_tplg_remove_route(comp, dobj, pass);
Ranjani Sridharan Jan. 25, 2023, 8:15 p.m. UTC | #2
On Wed, 2023-01-25 at 20:46 +0100, Amadeusz Sławiński wrote:
> Following is series of fixes and cleanups for core topology code. Few
> patches fixing various problems all around and few fixing function
> names.
> 
> Amadeusz Sławiński (11):
>   ASoC: topology: Properly access value coming from topology file
>   ASoC: topology: Remove unused SOC_TPLG_PASS_PINS constant
>   ASoC: topology: Fix typo in functions name
>   ASoC: topology: Fix function name
>   ASoC: topology: Rename remove_ handlers
>   ASoC: topology: Remove unnecessary forward declarations
>   ASoC: topology: Pass correct pointer instead of casting
>   ASoC: topology: Return an error on complete() failure
>   ASoC: Topology: Remove unnecessary check for EOF
>   ASoC: topology: Use unload() op directly
>   ASoC: topology: Unify kcontrol removal code

 LGTM, thanks, Amadeusz!

Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Amadeusz Sławiński Jan. 27, 2023, 11:12 a.m. UTC | #3
On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
> 
> they are identical because of the change in patch10.
> 
> Please clarify that this is not a cleanup removing duplicated code
> that's been there forever, it's become useless as a result of the
> previous patch.
> 

There is no dependency on previous patch - it is just order I've send 
them in - those functions have same implementation in current code.
Pierre-Louis Bossart Jan. 27, 2023, 1:38 p.m. UTC | #4
On 1/27/23 05:12, Amadeusz Sławiński wrote:
> On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
>>
>>
>> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
>>
>> they are identical because of the change in patch10.
>>
>> Please clarify that this is not a cleanup removing duplicated code
>> that's been there forever, it's become useless as a result of the
>> previous patch.
>>
> 
> There is no dependency on previous patch - it is just order I've send
> them in - those functions have same implementation in current code.

Not following, sorry. What is this addition in patch 10?

diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
index b4b896f83b94..f055c6917f6c 100644
--- a/include/sound/soc-topology.h
+++ b/include/sound/soc-topology.h
@@ -62,7 +62,7 @@ struct snd_soc_dobj {
 	enum snd_soc_dobj_type type;
 	unsigned int index;	/* objects can belong in different groups */
 	struct list_head list;
-	struct snd_soc_tplg_ops *ops;
+	int (*unload)(struct snd_soc_component *comp, struct snd_soc_dobj *dobj);

That's not in 'current code', is it? How is this not a dependency?
Amadeusz Sławiński Jan. 27, 2023, 3:09 p.m. UTC | #5
On 1/27/2023 2:38 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 1/27/23 05:12, Amadeusz Sławiński wrote:
>> On 1/25/2023 4:15 PM, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 1/25/23 13:46, Amadeusz Sławiński wrote:
>>>> Functions removing bytes, enum and mixer kcontrols are identical. Unify
>>>
>>> they are identical because of the change in patch10.
>>>
>>> Please clarify that this is not a cleanup removing duplicated code
>>> that's been there forever, it's become useless as a result of the
>>> previous patch.
>>>
>>
>> There is no dependency on previous patch - it is just order I've send
>> them in - those functions have same implementation in current code.
> 
> Not following, sorry. What is this addition in patch 10?
> 
> diff --git a/include/sound/soc-topology.h b/include/sound/soc-topology.h
> index b4b896f83b94..f055c6917f6c 100644
> --- a/include/sound/soc-topology.h
> +++ b/include/sound/soc-topology.h
> @@ -62,7 +62,7 @@ struct snd_soc_dobj {
>   	enum snd_soc_dobj_type type;
>   	unsigned int index;	/* objects can belong in different groups */
>   	struct list_head list;
> -	struct snd_soc_tplg_ops *ops;
> +	int (*unload)(struct snd_soc_component *comp, struct snd_soc_dobj *dobj);
> 
> That's not in 'current code', is it? How is this not a dependency?

It only depends on it because of order I did changes in, but there is 
really no dependency here. I will send v2 with reversed order.