Re: [PATCH v2 16/17] ASoC: Intel: bdw_rt286: Refactor suspend/resume

[Cezary Rojewski <cezary.rojewski@intel.com>]

On 2022-06-15 6:25 PM, Pierre-Louis Bossart wrote:
> 
>> Jacks are often initialized during dai_link initialization which is
>> completely out of platform_device area. This report made me think
>> further - if we assign jack in dai_link->init(), we should be able to
>> drop it in dai_link->exit().
>>
>> Not exactly! ->init() is done once card components are already accounted
>> for (available for use) but snd_soc_link_exit() is called during
>> snd_soc_remove_pcm_runtime() when card components are available no
>> longer - soc_remove_link_components().
>>
>> TLDR: teardown path is not symmetric with its counterpart, perhaps a
>> problem yet to be addressed. I'll see if moving the jack-NULLing to
>> codec's DAI ->remove() won't be a better temporary (?) solution than
>> reverting to platform_device->remove() usage.
> 
> It's a problem that impacted other platforms, see e.g.
> 
> static void kabylake_rt5660_codec_exit(struct snd_soc_pcm_runtime *rtd)
> {
> 	struct kbl_codec_private *ctx = snd_soc_card_get_drvdata(rtd->card);
> 
> 	/*
> 	 * The .exit() can be reached without going through the .init()
> 	 * so explicitly test if the gpiod is valid
> 	 */
> 	if (!IS_ERR_OR_NULL(ctx->gpio_lo_mute))
> 		gpiod_put(ctx->gpio_lo_mute);
> }
> 
> I vaguely recall hitting this myself when working with codec properties.
> It's worthy of a comment in the ASoC header to make sure this is better
> known/shared.
> 
> I see in other drivers that the use of component_set_jack() is
> symmetrical between .init and .exit, so far we haven't seen any issues
> with sof_rt5682.c and others.


I'll send a separate mail where we can discuss the teardown path. Don't 
believe the problem can be ignored. Even for the bdw_rt286.c usage of 
link->exit() generates:

   rt286 i2c-INT343A:00: ASoC: DAPM unknown pin LDO1

the following is the cause:

soc_remove_component() calls both component->remove() and 
snd_soc_dapm_free() for the component (in that order) so when 
link->exit() finally gets executes DAPM widgets are no longer there.


Regards,
Czarek