Re: [PATCH 1/6] dma: tegra: avoid overflow of byte tracking

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 31.10.2018 19:03, 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.
> 
> Signed-off-by: Ben Dooks <ben.dooks@xxxxxxxxxxxxxxx>
> ---
>  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)
> 

Reviewed-by: Dmitry Osipenko <digetx@xxxxxxxxx>



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux