Re: [PATCH] ASoC: SOF: Intel: Fix stream cleanup on pcm close

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

Hi Cezary,

>>> diff --git a/sound/soc/sof/intel/hda-pcm.c 
>>> b/sound/soc/sof/intel/hda-pcm.c
>>> index a46a6baa1c3f..4b3a89cf20e7 100644
>>> --- a/sound/soc/sof/intel/hda-pcm.c
>>> +++ b/sound/soc/sof/intel/hda-pcm.c
>>> @@ -246,5 +246,6 @@ int hda_dsp_pcm_close(struct snd_sof_dev *sdev,
>>>       /* unbinding pcm substream to hda stream */
>>>       substream->runtime->private_data = NULL;
>>> +    hstream->substream = NULL;
>>>       return 0;
>>>   }
>>
>>
>> Humm, yes we should clean this, but wondering if the close() operation 
>> is the right place. Doing this is hda_dsp_stream_hw_free() sounds more 
>> logical to me?
> 
> Ain't hda-pcm.c the best place for it as "hstream->substream = 
> substream" happens there too? If the cleanup is to be done in 
> _hw_free(), then I'd expect the same to happen to the original 
> assignments. Doubt we want to do the later so.. _close() for the win?
> 
> In general the existing hstream->substream initialization looks kinda 
> disconnected from the actual stream assignment code - _stream_get() - as 
> if the duties of the state machine were shared.

I am having difficulties interpreting your answer, i.e. I don't know 
what the last sentence refers to.

Currently open() and close() are perfectly symmetrical, I don't really 
see why you'd want to change and add an imbalanced set of operations, 
unless you moved

hstream->substream = substream;

to the open() instead of hw_params().

Or alternatively add a hw_free() in hda-pcm.c to mirror what's done in 
hw_params.