Messages in this thread | | | Date | Thu, 25 Mar 2010 15:00:04 -0700 | Subject | Re: [PATCH 2/2] DMAENGINE: generic channel status | From | Dan Williams <> |
| |
On Thu, Mar 25, 2010 at 1:58 PM, Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Thu, 25 Mar 2010, Linus Walleij wrote: > >> Convert the device_is_tx_complete() operation on the >> DMA engine to a generic device_tx_status()operation which >> can return three states, DMA_TX_RUNNING, DMA_TX_COMPLETE, >> DMA_TX_PAUSED. >> [..] > General: you converted all drivers to the new .device_tx_status() API, but > since they don't implement "residue," you left it uninitialised > everywhere. Wouldn't it be better to set it to 0 or total length, > depending on the complete / not complete status?
Agree that it should not be uninitialized. At the same time I do not want to require drivers that don't need it to go through the hassle of looking up a byte count, so perhaps all but the drivers that want this support can return a 'max byte count'??
> >> diff --git a/include/linux/dmaengine.h b/include/linux/dmaengine.h >> index 0731802..c9f2c67 100644 >> --- a/include/linux/dmaengine.h >> +++ b/include/linux/dmaengine.h >> @@ -40,11 +40,13 @@ typedef s32 dma_cookie_t; >> * enum dma_status - DMA transaction status >> * @DMA_SUCCESS: transaction completed successfully >> * @DMA_IN_PROGRESS: transaction not yet processed >> + * @DMA_PAUSED: transaction is paused >> * @DMA_ERROR: transaction failed >> */ >> enum dma_status { >> DMA_SUCCESS, >> DMA_IN_PROGRESS, >> + DMA_PAUSED, >> DMA_ERROR, >> }; >> >> @@ -249,6 +251,21 @@ struct dma_async_tx_descriptor { >> }; >> >> /** >> + * struct dma_tx_state - filled in to report the status of >> + * a transfer. >> + * @last: last completed DMA cookie >> + * @used: last issued DMA cookie (i.e. the one in progress) >> + * @residue: the remaining number of bytes left to transmit >> + * on the selected transfer for states DMA_IN_PROGRESS and >> + * DMA_PAUSED if this is implemented in the driver, else 0 >> + */ >> +struct dma_tx_state { >> + dma_cookie_t last; >> + dma_cookie_t used; >> + u32 residue; > > In the original proposal by Dan Williams the last member was "unsigned > long pos." I don't think, even on 64-bit systems anyone would kick off a > > 4GB transfer, but who knows... And - I don't particularly like the name > "pos," but I do like the idea of returning bytes transfered better, than > bytes left. Can we change this?
Things like copy_from_user() and the scsi subsystem return residue, so only for consistency with other areas would I side with the "bytes left" camp.
Ack to all your other comments.
-- Dan -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |