Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE_DEFER

[Doug Anderson <dianders@xxxxxxxxxxxx>]

Shawn,

On Mon, Mar 21, 2016 at 7:03 PM, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
>> ...but, looking at this, presumably before landing any patch that made
>> dma_request_slave_channel() return -EPROBE_DEFER you'd need to modify
>> _all_ users of dma_request_slave_channel to handle error pointers
>> being returned.  Right now dma_request_slave_channel() says it returns
>> a pointer to a channel or NULL and the function explicitly avoids
>> returning any errors.  That might be possible, but it's a big
>> change...
>
>
> At first glance, it's a big change, but maybe not really.
> Almost all of them use the templet like:
> ch = dma_request_slave_channel
> if (!ch)
>         balabala....
>
> It's same for all the non-null return pointer/non-zero value ?
>
> So from my view, we can safely change dma_request_slave_channel,
> and leave the caller here. I presumably the respective
> drivers will graduately migrate to check the return value with
> EPROBE_DEFER if they do care this issue. Otherwise, we believe
> they don't suffer the changes we make, just as what they did in the
> past. Does that make sense?

...but if you return ERR_PTR(-EPROBE_DEFER) and don't change existing
callers, then existing callers will think you've returned a valid
pointer when you really returned an error pointer.  They'll pass this
error pointer around like it's a valid "struct dma_chan", won't then?

Actually, could your code just call
dma_request_slave_channel_reason().  Oh, looks like that's exactly
what you want.  See commit 0ad7c00057dc ("dma: add channel request API
that supports deferred probe").  Oh, but I'm looking at 4.4.  Looking
at linuxnext, it looks like this got renamed to dma_request_chan().
...so you need to use that, no?

Strange, but on 4.4 there was some extra code in
dma_request_slave_channel() that wasn't in
dma_request_slave_channel_reason().  ...but looks like that all got
cleaned up in the same CL that added the new name.


-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html