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

yanwang yan.wang at linux.intel.com
Fri Nov 10 06:25:44 CET 2017


Hi, Peierre and Keyon,
	Thanks for your comments.
	Because size = min(hsize, lsize), the updated host_offset
should not > host_size. So "==" is enough.
	I will submit it again after modification.
	Thanks again.

Yan Wang
 
On Thu, 2017-11-09 at 20:31 -0600, Pierre-Louis Bossart wrote:
> > 
> > > 
> > > > 
> > > > @@ -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.
> Yes, my point exactly. Either the > condition happens and we have to 
> handle it, or it doesn't by construction due to other restrictions
> and 
> the test can be simplified.


More information about the Sound-open-firmware mailing list