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

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

 



On 21/11/2018 16: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@xxxxxxxxx>
> 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)

Acked-by: Jon Hunter <jonathanh@xxxxxxxxxx>

Thanks!
Jon

-- 
nvpublic



[Index of Archives]     [Linux Kernel]     [Linux ARM (vger)]     [Linux ARM MSM]     [Linux Omap]     [Linux Arm]     [Linux Tegra]     [Fedora ARM]     [Linux for Samsung SOC]     [eCos]     [Linux PCI]     [Linux Fastboot]     [Gcc Help]     [Git]     [DCCP]     [IETF Announce]     [Security]     [Linux MIPS]     [Yosemite Campsites]

  Powered by Linux