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@xxxxxxxxxxxxxxx>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx>
> ---
> 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
