Re: [PATCH V1 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

2020-05-19 Thread Vaibhav Agarwal
On Sun, May 17, 2020 at 07:37:06PM +0200, Alexandre Belloni wrote:
> Hi,
> 
> On 17/05/2020 22:47:20+0530, Vaibhav Agarwal wrote:
> > Greybus Codec driver allows modules to be dynamically added and removed,
> > which further requires updating the DAPM configurations as well.
> > 
> > With current snd_soc architecture, dynamic audio modules is not yet
> > supported. This patch provides helper APIs to update DAPM configurations
> > in response to modules which are dynamically added or removed. The
> > source is primarily based on snd_dapm.c
> > 
> 
> I really think you should send this patch series to the ASoC
> maintainers, especially this patch. The main goal shouldn't be to simply
> fix compilation issues but to try to get the driver out of staging else,
> the current situation will happen again.

Agree Alexandre. I'll share this with ASoC maintainers as well.

Also, I'm seeking opinion regarding the scope of pushing GB Audio out of 
staging tree. I'm keen to make the relevant changes and work actively 
for the same. However, I don't have a real device to test the latest 
code and thus not sure if the changes can be pushed to sound soc tree.
GB maintainers, kindly share your opinion.

--
Regards,
Vaibhav
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH V1 5/6] staging: greybus: audio: Add helper APIs for dynamic audio modules

2020-05-17 Thread Alexandre Belloni
Hi,

On 17/05/2020 22:47:20+0530, Vaibhav Agarwal wrote:
> Greybus Codec driver allows modules to be dynamically added and removed,
> which further requires updating the DAPM configurations as well.
> 
> With current snd_soc architecture, dynamic audio modules is not yet
> supported. This patch provides helper APIs to update DAPM configurations
> in response to modules which are dynamically added or removed. The
> source is primarily based on snd_dapm.c
> 

I really think you should send this patch series to the ASoC
maintainers, especially this patch. The main goal shouldn't be to simply
fix compilation issues but to try to get the driver out of staging else,
the current situation will happen again.

> Signed-off-by: Vaibhav Agarwal 
> ---
>  drivers/staging/greybus/Makefile   |   2 +-
>  drivers/staging/greybus/audio_codec.c  |  13 ++-
>  drivers/staging/greybus/audio_helper.c | 197 
> +
>  drivers/staging/greybus/audio_helper.h |  17 +++
>  4 files changed, 224 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/staging/greybus/audio_helper.c
>  create mode 100644 drivers/staging/greybus/audio_helper.h
> 
> diff --git a/drivers/staging/greybus/Makefile 
> b/drivers/staging/greybus/Makefile
> index 627e44f2a983..3b4b6cabff19 100644
> --- a/drivers/staging/greybus/Makefile
> +++ b/drivers/staging/greybus/Makefile
> @@ -28,7 +28,7 @@ obj-$(CONFIG_GREYBUS_VIBRATOR)  += gb-vibrator.o
>  
>  # Greybus Audio is a bunch of modules
>  gb-audio-module-y:= audio_module.o audio_topology.o
> -gb-audio-codec-y := audio_codec.o
> +gb-audio-codec-y := audio_codec.o audio_helper.o
>  gb-audio-gb-y:= audio_gb.o
>  gb-audio-apbridgea-y := audio_apbridgea.o
>  gb-audio-manager-y   := audio_manager.o audio_manager_module.o
> diff --git a/drivers/staging/greybus/audio_codec.c 
> b/drivers/staging/greybus/audio_codec.c
> index bbd072acda5c..866b3342515f 100644
> --- a/drivers/staging/greybus/audio_codec.c
> +++ b/drivers/staging/greybus/audio_codec.c
> @@ -14,6 +14,7 @@
>  #include "audio_codec.h"
>  #include "audio_apbridgea.h"
>  #include "audio_manager.h"
> +#include "audio_helper.h"
>  
>  static struct gbaudio_codec_info *gbcodec;
>  
> @@ -874,7 +875,7 @@ int gbaudio_register_module(struct gbaudio_module_info 
> *module)
>  
>   /* card already instantiated, create widgets here only */
>   if (component->card->instantiated) {
> - snd_soc_dapm_link_component_dai_widgets(component->card,
> + gbaudio_dapm_link_component_dai_widgets(component->card,
>   >dapm);
>  #ifdef CONFIG_SND_JACK
>   /*
> @@ -1004,19 +1005,23 @@ void gbaudio_unregister_module(struct 
> gbaudio_module_info *module)
>   if (module->dapm_routes) {
>   dev_dbg(component->dev, "Removing %d routes\n",
>   module->num_dapm_routes);
> + /* verify routes with controls are removed */
>   snd_soc_dapm_del_routes(>dapm, module->dapm_routes,
>   module->num_dapm_routes);
>   }
>   if (module->controls) {
>   dev_dbg(component->dev, "Removing %d controls\n",
>   module->num_controls);
> - snd_soc_remove_codec_controls(component, module->controls,
> -   module->num_controls);
> + /* release control semaphore */
> + up_write(>controls_rwsem);
> + gbaudio_remove_component_controls(component, module->controls,
> +   module->num_controls);
> + down_write(>controls_rwsem);
>   }
>   if (module->dapm_widgets) {
>   dev_dbg(component->dev, "Removing %d widgets\n",
>   module->num_dapm_widgets);
> - snd_soc_dapm_free_controls(>dapm,
> + gbaudio_dapm_free_controls(>dapm,
>  module->dapm_widgets,
>  module->num_dapm_widgets);
>   }
> diff --git a/drivers/staging/greybus/audio_helper.c 
> b/drivers/staging/greybus/audio_helper.c
> new file mode 100644
> index ..e2f113a811ee
> --- /dev/null
> +++ b/drivers/staging/greybus/audio_helper.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Greybus Audio Sound SoC helper APIs
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define gbaudio_dapm_for_each_direction(dir) \
> + for ((dir) = SND_SOC_DAPM_DIR_IN; (dir) <= SND_SOC_DAPM_DIR_OUT; \
> + (dir)++)
> +
> +static void gbaudio_dapm_link_dai_widget(struct snd_soc_dapm_widget *dai_w,
> +  struct snd_soc_card *card)
> +{
> + struct snd_soc_dapm_widget *w;
> + struct snd_soc_dapm_widget *src, *sink;
> + struct snd_soc_dai *dai = dai_w->priv;
> +
> + /* ...find all widgets