Re: [PATCH 2/2] ALSA: compress: Propagate real pointer errors in avail paths

[Cássio Gabriel Monteiro Pires]

On 4/14/26 13:53, Mark Brown wrote:
> On Tue, Apr 14, 2026 at 01:47:54PM -0300, Cássio Gabriel wrote:
> 
>> Commit 61327f3d817c ("ALSA: compress: Pay attention if drivers
>> error out retrieving pointers") made snd_compr_update_tstamp()
>> return driver pointer() failures, but snd_compr_calc_avail()
>> still ignores that status and continues with stale runtime
>> counters.
> 
>> The fallback for an unsupported pointer callback remains
>> intentional, so keep ignoring -EOPNOTSUPP there. Propagate
>> real pointer-refresh failures instead.
> 
>> -static size_t snd_compr_calc_avail(struct snd_compr_stream *stream,
>> -				   struct snd_compr_avail64 *avail)
>> +static int snd_compr_calc_avail(struct snd_compr_stream *stream,
>> +				struct snd_compr_avail64 *avail)
>>  {
>> +	int ret;
>> +
>>  	memset(avail, 0, sizeof(*avail));
>> -	snd_compr_update_tstamp(stream, &avail->tstamp);
>> -	/* Still need to return avail even if tstamp can't be filled in */
>> +	ret = snd_compr_update_tstamp(stream, &avail->tstamp);
>> +	if (ret < 0 && ret != -EOPNOTSUPP)
>> +		return ret;
>> +	/* Still need to return avail when no timestamp is available */
> 
> It's not clear to me that the intent there is to only ignore in the case
> where we fail to update due to the operation not being supported rather
> than to also ignore any other runtime errors we might encounter (eg, due
> to the stream configuration).

Cool, thanks for replying!

I rechecked the current tree, and the hard-failure class does exist:
wm_adsp_compr_pointer() can return -EIO and switch the stream to
XRUN when dsp->fatal_error, !buf, or buf->error, so that is a
real in-tree case where pointer() failure means the stream itself is
broken rather than timestamp data simply being unavailable.

That said, I agree this does not fully justify the generic change as I
sent it. In particular, SNDRV_COMPRESS_AVAIL and poll() both recheck
the stream state after the pointer refresh, and there are other drivers
such as SOF where pointer() can return -EBUSY for a not-ready case,
which is not the same class as the wm_adsp -EIO/XRUN path.

I also agree that the current patch is too broad because it uses the raw pointer()
errno as the policy boundary for avail handling, and that conflates hard
stream-failure cases such as wm_adsp's -EIO/XRUN transition with
other conditions like SOF's -EBUSY not-ready return. I can rework patch
2 so the behavior is based on a better-defined failure condition instead
of treating every non-EOPNOTSUPP return as fatal for avail.

WDYT?

-- 
Thanks,
Cássio

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature