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