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

[Pierre-Louis Bossart]

On 3/5/20 7:25 AM, Andy Shevchenko wrote:
> 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

yep


>> +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?

I just rechecked, the error flow is

dailink.init()
soc_init_pcm_runtime
snd_soc_bind_card probe_end:
soc_cleanup_card_resources(card, card_probed);
snd_soc_remove_pcm_runtime(card, rtd);
dai_link->exit(rtd);

so we do need to recheck if the resources allocated in init() are valid.

I also think the IS_ERR() is correct by looking at the code in 
gpiod_get_index() but the comments are rather confusing to me:

  * Return a valid GPIO descriptor, -ENOENT if no GPIO has been assigned 
to the
  * requested function and/or index, or another IS_ERR() code if an error
  * occurred while trying to acquire the GPIO.


> 
>> +		gpiod_put(bdw_rt5677->gpio_hp_en);
>> +}
>> +