Re: [PATCH] ALSA: loongson: Fix invalid position error in ls_pcm_pointer

[Takashi Iwai]

On Fri, 29 May 2026 04:48:30 +0200,
Li Jun wrote:
> 
> The "invalid position" error occurred when the DMA position descriptor
> returned an invalid address value (e.g., pos = -1048838144). This happened
> because the `bytes_to_frames()` function returns a signed value, but when
> `addr < runtime->dma_addr`, the subtraction produces a negative result that
> gets interpreted as a large unsigned integer in comparisons.
> 
> [   32.834431][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.845019][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.855588][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.866145][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   32.995394][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   33.006025][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> [   33.016748][ 2]  soc-audio soc-audio: invalid position: , pos = -1048838144
> 
> Signed-off-by: Li Jun <lijun01@xxxxxxxxxx>

Since it's a change for ASoC, use "ASoC" prefix for the subject line.

About the code change: how does this negative position come?  Is it an
utterly bogus value to be ignored, or is it a position overlap or
such?  Your change seems just ignoring and returning 0, and if it's a
spontaneous bogus value, it may lead to an unexpected jump of the PCM
position, too, for example.


thanks,

Takashi

> ---
>  sound/soc/loongson/loongson_dma.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/loongson/loongson_dma.c b/sound/soc/loongson/loongson_dma.c
> index a149b643175c..f0a7065ddae7 100644
> --- a/sound/soc/loongson/loongson_dma.c
> +++ b/sound/soc/loongson/loongson_dma.c
> @@ -207,9 +207,15 @@ loongson_pcm_pointer(struct snd_soc_component *component,
>  	desc = dma_desc_save(prtd);
>  	addr = ((u64)desc->saddr_hi << 32) | desc->saddr;
>  
> -	x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> -	if (x == runtime->buffer_size)
> +	if (addr < runtime->dma_addr ||
> +	    addr >= runtime->dma_addr + runtime->dma_bytes) {
>  		x = 0;
> +	} else {
> +		x = bytes_to_frames(runtime, addr - runtime->dma_addr);
> +		if (x >= runtime->buffer_size)
> +			x = 0;
> +	}
> +
>  	return x;
>  }
>  
> -- 
> 2.25.1
>