linux-kernel - Re: [PATCH 2/2] DMAENGINE: generic channel status

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 26 Mar 2010 00:18:49 +0100
From:	Linus Walleij <linus.ml.walleij@...il.com>
To:	Guennadi Liakhovetski <g.liakhovetski@....de>
Cc:	Linus Walleij <linus.walleij@...ricsson.com>,
	Dan Williams <dan.j.williams@...el.com>,
	linux-kernel@...r.kernel.org,
	Maciej Sosnowski <maciej.sosnowski@...el.com>,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Pavel Machek <pavel@....cz>, Li Yang <leoli@...escale.com>,
	Paul Mundt <lethal@...ux-sh.org>,
	Ralf Baechle <ralf@...ux-mips.org>,
	Haavard Skinnemoen <haavard.skinnemoen@...el.com>,
	Magnus Damm <damm@...nsource.se>,
	Liam Girdwood <lrg@...mlogic.co.uk>,
	Mark Brown <broonie@...nsource.wolfsonmicro.com>,
	Joe Perches <joe@...ches.com>,
	Roland Dreier <rdreier@...co.com>
Subject: Re: [PATCH 2/2] DMAENGINE: generic channel status

2010/3/25 Guennadi Liakhovetski <g.liakhovetski@....de>:

> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/