On 11/25/2013 10:53 AM, Russell King - ARM Linux wrote:
> On Mon, Nov 25, 2013 at 10:26:18AM -0700, Stephen Warren wrote:
>> On 11/22/2013 05:34 PM, Russell King - ARM Linux wrote:
>> [... various discussions re: IS_ERR, IS_ERR_OR_NULL, etc.]
>>
>> Russell, so if we document dma_request_slave_channel() as follows:
>>
>>> /*
>>> * dma_request_slave_channel - try to allocate an exclusive slave channel
>>> ...
>>> * Returns pointer to appropriate DMA channel on success or an error pointer.
>>> * In order to ease compatibility with other DMA request APIs, this function
>>> * guarantees NEVER to return NULL.
>>> */
>>
>> Are you then OK with clients doing either of e.g.:
>>
>> chan = dma_request_slave_channel(...);
>> if (IS_ERR(chan))
>> // This returns NULL or a valid handle/pointer
>> chan = dma_request_channel()
>> if (!chan)
>> Here, chan is invalid;
>> return;
>> Here, chan is valid
>>
>> or:
>>
>> if (xxx) {
>> chan = dma_request_slave_channel(...);
>> // Convert to same error value as else{} block generates
>> if (IS_ERR(chan))
>> chan = NULL
>> } else {
>> // This returns NULL or a valid handle/pointer
>> chan = dma_request_channel()
>> }
>> if (!chan)
>> Here, chan is invalid
>> return;
>> Here, chan is valid
>
> The cleaner way to write this is:
>
> chan = dma_request_slave_channel(...);
> if (IS_ERR(chan)) {
> chan = dma_request_channel();
> if (!chan)
> return;
> }
>
> So, if you make it to this point, chan must be valid according to the
> conditions on the API - you've applied the test individally to each
> return function according to its documented behaviour. If it does
> happen to be NULL, that's not *your* problem as a user of the API.
As Dan pointed out, there are many drivers where DMA is optional, so
there's a lot of this sprinkled through the body of the driver:
if (chan) ...
if (!chan) ...
If we don't convert IS_ERR() values to NULL like I showed above, then
all those tests would need to be converted to something else. Can we
please avoid that?
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
