| Message ID | 1399411343-12222-4-git-send-email-cfreeman@nvidia.com (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Vinod Koul |
| Headers | show |
On 05/06/2014 03:22 PM, Christopher Freeman wrote: > bytes_transferred will overflow during long audio playbacks. Since > the driver only ever consults this value modulo bytes_requested, store the > value modulo bytes_requested. The audio driver may only interpret the value modulo bytes_requested, but what about other drivers such as the high-speed UART (and SPI?) drivers? What is the dmaengine API's design requirement here, and what do other dmaengine drivers do. If it's to store the modulo, then I'm fine with this change. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/07/2014 06:38 PM, Stephen Warren wrote: > On 05/06/2014 03:22 PM, Christopher Freeman wrote: >> bytes_transferred will overflow during long audio playbacks. Since >> the driver only ever consults this value modulo bytes_requested, store the >> value modulo bytes_requested. > > The audio driver may only interpret the value modulo bytes_requested, > but what about other drivers such as the high-speed UART (and SPI?) drivers? > > What is the dmaengine API's design requirement here, and what do other > dmaengine drivers do. If it's to store the modulo, then I'm fine with > this change. Yep, this part of the API. The residue should be between transfer length and 0. While 0 is special and should only be returned if the transfer has finished. For cyclic transfers this means it should never be zero. So if transferred_bytes is incremented modulo length and residue is length - transferred_bytes you get the correct result. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, May 07, 2014 at 12:15:39PM -0700, Lars-Peter Clausen wrote: > On 05/07/2014 06:38 PM, Stephen Warren wrote: > > On 05/06/2014 03:22 PM, Christopher Freeman wrote: > >> bytes_transferred will overflow during long audio playbacks. Since > >> the driver only ever consults this value modulo bytes_requested, store the > >> value modulo bytes_requested. > > > > The audio driver may only interpret the value modulo bytes_requested, > > but what about other drivers such as the high-speed UART (and SPI?) drivers? > > > > What is the dmaengine API's design requirement here, and what do other > > dmaengine drivers do. If it's to store the modulo, then I'm fine with > > this change. > > Yep, this part of the API. The residue should be between transfer length and > 0. While 0 is special and should only be returned if the transfer has > finished. For cyclic transfers this means it should never be zero. So if > transferred_bytes is incremented modulo length and residue is length - > transferred_bytes you get the correct result. > What each driver receives remains unchanged here. bytes_transferred is only ever read modulo bytes_requested in all cases (audio, spi, uart) This shifts that part of the calculation to the assignment. I guess this is a roundabout way of saying it's not any more wrong than it could have possibly been before. We can rename "bytes_transferred" to something like "residue" or "segment_residue" though. -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/dma/tegra20-apb-dma.c b/drivers/dma/tegra20-apb-dma.c index 094e97d..e1b80a4 100644 --- a/drivers/dma/tegra20-apb-dma.c +++ b/drivers/dma/tegra20-apb-dma.c @@ -583,7 +583,9 @@ static void handle_once_dma_done(struct tegra_dma_channel *tdc, tdc->busy = false; sgreq = list_first_entry(&tdc->pending_sg_req, typeof(*sgreq), node); dma_desc = sgreq->dma_desc; - dma_desc->bytes_transferred += sgreq->req_len; + dma_desc->bytes_transferred = (dma_desc->bytes_transferred + + sgreq->req_len) % + dma_desc->bytes_requested; list_del(&sgreq->node); if (sgreq->last_sg) { @@ -613,7 +615,9 @@ 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; + 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) @@ -762,8 +766,10 @@ static void tegra_dma_terminate_all(struct dma_chan *dc) if (!list_empty(&tdc->pending_sg_req) && was_busy) { sgreq = list_first_entry(&tdc->pending_sg_req, typeof(*sgreq), node); - sgreq->dma_desc->bytes_transferred += - get_current_xferred_count(tdc, sgreq, wcount); + sgreq->dma_desc->bytes_transferred = + (sgreq->dma_desc->bytes_transferred + + get_current_xferred_count(tdc, sgreq, wcount)) % + sgreq->dma_desc->bytes_requested; } tegra_dma_resume(tdc); @@ -838,8 +844,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, list_for_each_entry(dma_desc, &tdc->free_dma_desc, node) { if (dma_desc->txd.cookie == cookie) { residual = dma_desc->bytes_requested - - (dma_desc->bytes_transferred % - dma_desc->bytes_requested); + dma_desc->bytes_transferred; dma_set_residue(txstate, residual); ret = dma_desc->dma_status; spin_unlock_irqrestore(&tdc->lock, flags); @@ -859,8 +864,7 @@ static enum dma_status tegra_dma_tx_status(struct dma_chan *dc, typeof(*first_entry), node); residual = dma_desc->bytes_requested - - (dma_desc->bytes_transferred % - dma_desc->bytes_requested); + dma_desc->bytes_transferred; /* hw byte count only applies to current transaction */ if (first_entry &&
bytes_transferred will overflow during long audio playbacks. Since the driver only ever consults this value modulo bytes_requested, store the value modulo bytes_requested. Signed-off-by: Christopher Freeman <cfreeman@nvidia.com> --- drivers/dma/tegra20-apb-dma.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)