Re: [PATCH 2/2] DMAENGINE: generic channel status
From: Linus Walleij
Date: Thu Mar 25 2010 - 19:18:56 EST
2010/3/25 Guennadi Liakhovetski <g.liakhovetski@xxxxxx>:
> 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?
The total length is a bit hard to know without understanding all drivers
I'm afraid, but I can sure add txtstatus->residue = 0; everywhere,
I'll fix.
>> +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?
As Dan says consistency with other systems suggests something
residue or bytes_left.
>> - * @device_is_tx_complete: poll for transaction completion
>> + * @device_tx_status: poll for transaction completion, the optional
>> + * txstate parameter can be supplied with a pointer to get a
>> + * struct with some transfer information, else the call will just
>
> Maybe "with auxiliary transfer status information, otherwise..."
OK
>> @@ -572,7 +592,13 @@ static inline void dma_async_issue_pending(struct dma_chan *chan)
>> static inline enum dma_status dma_async_is_tx_complete(struct dma_chan *chan,
>> dma_cookie_t cookie, dma_cookie_t *last, dma_cookie_t *used)
>> {
>> - return chan->device->device_is_tx_complete(chan, cookie, last, used);
>> + struct dma_tx_state state;
>> + enum dma_status status;
>> +
>> + status = chan->device->device_tx_status(chan, cookie, &state);
>> + *last = state.last;
>> + *used = state.used;
>
> Both last and used can be NULL.
Good you spotted this! Thanks.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/