[Sound-open-firmware] [PATCH 4/4] Fix DMA host offset calculation.

[Keyon Jie]

On 2017年11月10日 05:21, Pierre-Louis Bossart wrote:
> 
> 
> On 11/09/2017 12:17 AM, yan.wang at linux.intel.com wrote:
>> From: Yan Wang <yan.wang at linux.intel.com>
>>
>> 1. "size" should not be considered twice.
>> 2. Change host_offset to uint32_t type.
>>
>> Signed-off-by: Yan Wang <yan.wang at linux.intel.com>
>> ---
>>   src/include/reef/dma-trace.h | 2 +-
>>   src/lib/dma-trace.c          | 2 +-
>>   2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/include/reef/dma-trace.h b/src/include/reef/dma-trace.h
>> index 12d0956..641a4dc 100644
>> --- a/src/include/reef/dma-trace.h
>> +++ b/src/include/reef/dma-trace.h
>> @@ -56,7 +56,7 @@ struct dma_trace_data {
>>       struct dma_sg_config config;
>>       struct dma_trace_buf dmatb;
>>       struct dma_copy dc;
>> -    int32_t host_offset;
>> +    uint32_t host_offset;
>>       uint32_t host_size;
>>       struct work dmat_work;
>>       uint32_t enabled;
>> diff --git a/src/lib/dma-trace.c b/src/lib/dma-trace.c
>> index 5885037..249e18b 100644
>> --- a/src/lib/dma-trace.c
>> +++ b/src/lib/dma-trace.c
>> @@ -90,7 +90,7 @@ static uint64_t trace_work(void *data, uint64_t delay)
>>       /* update host pointer and check for wrap */
>>       d->host_offset += size;
>> -    if (d->host_offset + size >= d->host_size)
>> +    if (d->host_offset >= d->host_size)
>>           d->host_offset = 0;
> this code pattern is present multiple times in the DMA handling code.
> I wonder if this is correct when the > condition is true? That'd mean 
> some overrun happened or that some bytes have been lost.
> Shouldn't this be an exact == condition in there is a true wrap?

actually we should make it to something like this for sake of safe:
	d->host_offset -= d->host_size;

but considering that we have require the host_size to be multiple of 
period size, the '>' case won't happen, so here it is simplified to be
	d->host_offset = 0;

I agree that we should make it more robust.

Thanks,
~Keyon

>>       /* update local pointer and check for wrap */
> 
> _______________________________________________
> Sound-open-firmware mailing list
> Sound-open-firmware at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
>