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

[Takashi Iwai <tiwai@suse.de>]

On Fri, 29 May 2026 15:23:18 +0200,
lijun wrote:
> 
> When the addr is abnormal, such as when the DMA controller is abnormal,
> I think x=0 should not be a point, but a range, which includes the addr
> address being less than runtime ->dma1-adr, and the addr exceeding
> the DMA address range.

The question is how often it happens.  If this happens frequently,
reporting a position jump just confuses user-space.
If it's an abnormal situation, one can treat it rather like an XRUN
and let stop the stream / restart, too.


Takashi

> 
> 在 2026/5/29 17:01, 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@kylinos.cn>
> > 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
> >>