[PATCH] ASoC: Fix Blackfin I2S _pointer() implementation return in bounds values

[PATCH] ASoC: Fix Blackfin I2S _pointer() implementation return in bounds values

[Mark Brown]

The Blackfin DMA controller can report one frame beyond the end of the
buffer in the wraparound case but ALSA requires that the pointer always
be in the buffer. Do the wraparound to handle this. A similar bug is
likely to apply to the other Blackfin PCM drivers but the code is less
obvious to inspection and I don't have a user to test.

Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
---
 sound/soc/blackfin/bf5xx-i2s-pcm.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/sound/soc/blackfin/bf5xx-i2s-pcm.c b/sound/soc/blackfin/bf5xx-i2s-pcm.c
index 4a805a8..819d3bd 100644
--- a/sound/soc/blackfin/bf5xx-i2s-pcm.c
+++ b/sound/soc/blackfin/bf5xx-i2s-pcm.c
@@ -138,11 +138,21 @@ static snd_pcm_uframes_t bf5xx_pcm_pointer(struct snd_pcm_substream *substream)
 	pr_debug("%s enter\n", __func__);
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) {
 		diff = sport_curr_offset_tx(sport);
-		frames = bytes_to_frames(substream->runtime, diff);
 	} else {
 		diff = sport_curr_offset_rx(sport);
-		frames = bytes_to_frames(substream->runtime, diff);
 	}
+
+	/*
+	 * TX at least can report one frame beyond the end of the
+	 * buffer if we hit the wraparound case - clamp to within the
+	 * buffer as the ALSA APIs require.
+	 */
+	if (diff == snd_pcm_lib_buffer_bytes(substream))
+		diff = 0;
+	}
+
+	frames = bytes_to_frames(substream->runtime, diff);
+
 	return frames;
 }
 
-- 
1.7.5.4

Re: [PATCH] ASoC: Fix Blackfin I2S _pointer() implementation return in bounds values

[Lars-Peter Clausen]

On 06/23/2011 09:02 PM, Mark Brown wrote:
> [...]
> +	if (diff == snd_pcm_lib_buffer_bytes(substream))
> +		diff = 0;
> +	}
stray closing bracket

> +	frames = bytes_to_frames(substream->runtime, diff);
> +
>  	return frames;

Could be 'return bytes_to_frames(substream->runtime, diff);' directly

Re: [PATCH] ASoC: Fix Blackfin I2S _pointer() implementation return in bounds values

[Mark Brown]

On Thu, Jun 23, 2011 at 09:10:31PM +0200, Lars-Peter Clausen wrote:
> On 06/23/2011 09:02 PM, Mark Brown wrote:
> > [...]
> > +	if (diff == snd_pcm_lib_buffer_bytes(substream))
> > +		diff = 0;
> > +	}

> stray closing bracket

Hrm, so there is.  Fixed locally, I guess there's no point in reposting
just for that.

> > +	frames = bytes_to_frames(substream->runtime, diff);
> > +
> >  	return frames;

> Could be 'return bytes_to_frames(substream->runtime, diff);' directly

Does no harm either way, though.  I don't always find doing the return
directly quite so parsable, but I think here I wrote it this way because
it makes the diff a bit more obvious.

Re: [PATCH] ASoC: Fix Blackfin I2S _pointer() implementation return in bounds values

[Liam Girdwood]

On 23/06/11 20:02, Mark Brown wrote:
> The Blackfin DMA controller can report one frame beyond the end of the
> buffer in the wraparound case but ALSA requires that the pointer always
> be in the buffer. Do the wraparound to handle this. A similar bug is
> likely to apply to the other Blackfin PCM drivers but the code is less
> obvious to inspection and I don't have a user to test.
> 
> Signed-off-by: Mark Brown <broonie@opensource.wolfsonmicro.com>
> ---

Acked-by: Liam Girdwood <lrg@ti.com>