[RFC PATCH 2/3] ASoC: Intel: bdw-rt5677: fix module load/unload issues

[Andy Shevchenko]

On Thu, Mar 05, 2020 at 07:06:15AM -0600, Pierre-Louis Bossart wrote:
> The use of devm_gpiod_get() in a dailink .init() callback generates issues
> when unloading modules. The dependencies between modules are not well
> handled and the snd_soc_rt5677 module cannot be removed:
> 
> rmmod: ERROR: Module snd_soc_rt5677 is in use
> 
> Removing the use of devm_ and manually releasing the gpio descriptor

gpio -> GPIO

> in the dailink .exit() callback solves the issue.
> 
> Tested on SAMUS Chromebook with SOF driver.
> 
> Suggested-by: Andy Shevchenko <andriy.shevchenko at linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart at linux.intel.com>
> ---
>  sound/soc/intel/boards/bdw-rt5677.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/intel/boards/bdw-rt5677.c b/sound/soc/intel/boards/bdw-rt5677.c
> index a94f498388e1..641491cb449b 100644
> --- a/sound/soc/intel/boards/bdw-rt5677.c
> +++ b/sound/soc/intel/boards/bdw-rt5677.c
> @@ -247,8 +247,8 @@ static int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
>  			RT5677_CLK_SEL_SYS2);
>  
>  	/* Request rt5677 GPIO for headphone amp control */
> -	bdw_rt5677->gpio_hp_en = devm_gpiod_get(component->dev, "headphone-enable",
> -						GPIOD_OUT_LOW);
> +	bdw_rt5677->gpio_hp_en = gpiod_get(component->dev, "headphone-enable",
> +					   GPIOD_OUT_LOW);
>  	if (IS_ERR(bdw_rt5677->gpio_hp_en)) {
>  		dev_err(component->dev, "Can't find HP_AMP_SHDN_L gpio\n");
>  		return PTR_ERR(bdw_rt5677->gpio_hp_en);
> @@ -282,6 +282,15 @@ static int bdw_rt5677_init(struct snd_soc_pcm_runtime *rtd)
>  	return 0;
>  }
>  
> +static void bdw_rt5677_exit(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct bdw_rt5677_priv *bdw_rt5677 =
> +			snd_soc_card_get_drvdata(rtd->card);
> +

> +	if (!IS_ERR(bdw_rt5677->gpio_hp_en))

I'm wondering if you need this check at all? In the above (I left for context)
the GPIO is considered mandatory, does the core handles errors from ->init()
correctly?

> +		gpiod_put(bdw_rt5677->gpio_hp_en);
> +}
> +
>  /* broadwell digital audio interface glue - connects codec <--> CPU */
>  SND_SOC_DAILINK_DEF(dummy,
>  	DAILINK_COMP_ARRAY(COMP_DUMMY()));
> @@ -350,6 +359,7 @@ static struct snd_soc_dai_link bdw_rt5677_dais[] = {
>  		.dpcm_playback = 1,
>  		.dpcm_capture = 1,
>  		.init = bdw_rt5677_init,
> +		.exit = bdw_rt5677_exit,
>  		SND_SOC_DAILINK_REG(ssp0_port, be, platform),
>  	},
>  };
> -- 
> 2.20.1
> 

-- 
With Best Regards,
Andy Shevchenko