[PATCH 11/31] dma: add channel request API that supports deferred probe

[Stephen Warren]

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?