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);+}