[1/3] dma: tegra: avoid overflow of byte tracking

[Ben Dooks <ben.dooks@codethink.co.uk>]

On 2018-11-14 12:12, Dmitry Osipenko wrote:
> On 14.11.2018 13:13, Ben Dooks wrote:
>> The dma_desc->bytes_transferred counter tracks the number of bytes
>> moved by the DMA channel. This is then used to calculate the 
>> information
>> passed back in the in the tegra_dma_tx_status callback, which is 
>> usually
>> fine.
>> 
>> When the DMA channel is configured as continous, then the 
>> bytes_transferred
>> counter will increase over time and eventually overflow to become 
>> negative
>> so the residue count will become invalid and the ALSA sound-dma code 
>> will
>> report invalid hardware pointer values to the application. This 
>> results in
>> some users becoming confused about the playout position and putting 
>> audio
>> data in the wrong place.
>> 
>> To fix this issue, always ensure the bytes_transferred field is modulo 
>> the
>> size of the request. We only do this for the case of the cyclic 
>> transfer
>> done ISR as anyone attempting to move 2GiB of DMA data in one transfer
>> is unlikely.
>> 
>> Note, we don't fix the issue that we should /never/ transfer a 
>> negative
>> number of bytes so we could make those fields unsigned.
>> 
>> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
>> Signed-off-by: Ben Dooks <ben.dooks@codethink.co.uk>
>> ---
>>  drivers/dma/tegra20-apb-dma.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/dma/tegra20-apb-dma.c 
>> b/drivers/dma/tegra20-apb-dma.c
>> index 9a558e30c461..8219ab88a507 100644
>> --- a/drivers/dma/tegra20-apb-dma.c
>> +++ b/drivers/dma/tegra20-apb-dma.c
>> @@ -636,7 +636,10 @@ static void 
>> handle_cont_sngl_cycle_dma_done(struct tegra_dma_channel *tdc,
>> 
>>  	sgreq = list_first_entry(&tdc->pending_sg_req, typeof(*sgreq), 
>> node);
>>  	dma_desc = sgreq->dma_desc;
>> -	dma_desc->bytes_transferred += sgreq->req_len;
>> +	/* if we dma for long enough the transfer count will wrap */
>> +	dma_desc->bytes_transferred =
>> +		(dma_desc->bytes_transferred + sgreq->req_len) %
>> +		dma_desc->bytes_requested;
>> 
>>  	/* Callback need to be call */
>>  	if (!dma_desc->cb_count)
>> 
> 
> I also actually tested that audio playback breaks after the overflow
> and this patch fixes it.

Thanks, I should have posted a link to a test-patch I had
a while ago.

> Tested-by: Dmitry Osipenko <digetx@gmail.com>