Re: [PATCH 1/4] ASoC: meson: gx: add gx-formatter and gx-interface

[Valerio Setti <vsetti@baylibre.com>]

> 
>> +int gx_stream_start(struct gx_stream *ts)
>> +{
>> +	struct gx_formatter *formatter;
>> +	int ret = 0;
>> +
>> +	mutex_lock(&ts->lock);
>> +
>> +	/* Start all the formatters attached to the stream */
>> +	list_for_each_entry(formatter, &ts->formatter_list, list) {
>> +		ret = gx_formatter_enable(formatter);
>> +		if (ret) {
>> +			pr_err("failed to start tdm stream\n");
>> +			goto out;
> 
> Do we need to disable the formatters that were already successfully enabled
> in previous iterations of this loop before returning?
> 
> The ALSA core does not automatically invoke the STOP trigger when a START
> trigger fails. Returning an error here without rolling back the earlier
> formatters could leave the hardware clocks and registers permanently enabled,
> resulting in leaked resources.
> 

I don't mind adding this check, but I have a question for reviewers for 
sake of knowledge.
I just checked and it seemes to me that actually when 
"soc_pcm_trigger()" fails to SNDRV_PCM_TRIGGER_START it rolls back to 
SNDRV_PCM_TRIGGER_STOP. Sure I'm missing something that Sashiko found, 
but I cannot figure out what. Any help on this?

-- 
Valerio


_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic