On Wed, Nov 20, 2013 at 1:24 PM, Stephen Warren <swarren@xxxxxxxxxxxxx> wrote:
> On 11/20/2013 01:23 PM, Williams, Dan J wrote:
> ...
>> Why do the drivers that call dma_request_channel need to convert it to
>> an ERR value? i.e. what's problematic about the below (not compile
>> tested)?
> ...
>> diff --git a/arch/arm/plat-samsung/dma-ops.c b/arch/arm/plat-samsung/dma-ops.c
> ...
>> @@ -22,16 +22,20 @@ static unsigned samsung_dmadev_request(enum dma_ch dma_ch,
>> struct samsung_dma_req *param,
>> struct device *dev, char *ch_name)
> ...
>> + if (dev->of_node) {
>> + chan = dma_request_slave_channel(dev, ch_name);
>> + return IS_ERR(chan) ? (unsigned) NULL : (unsigned) chan;
>> + } else {
>> return (unsigned)dma_request_channel(mask, pl330_filter,
>> (void *)dma_ch);
>> + }
>
> The argument is that if a function returns errors encoded as an ERR
> pointer, then callers must assume that any non-IS_ERR value that the
> function returns is valid. NULL is one of those values. As such, callers
> can no longer check the value against NULL, but must use IS_ERR().
> Converting any IS_ERR() returns to NULL theoretically is the act of
> converting one valid return value to some other completely random return
> value.
You describe how IS_ERR() works, but you didn't address the patch.
There's nothing random about the changes to samsung_dmadev_request().
It still returns NULL or a valid channel just as before.
> The converse is true for functions that return errors encoded as NULL;
> callers must check those return results against NULL.
>
> There's no intersection between those two sets of legal tests.
I don't understand what you are trying to say. I'm not proposing an
intersection. I'm proposing that clients explicitly handle an updated
dma_request_slave_channel and we skip this interim
dma_request_slave_channel_no_err step.
Proliferating yet another *request_channel* api is worse than just
having clients understand that dma_request_slave_channel now encodes
errors while dma_request_slave_channel_compat and dma_request_channel
still just return NULL.
--
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
