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

Doug Anderson dianders at chromium.org
Mon Mar 21 19:33:04 PDT 2016


Shawn,

On Mon, Mar 21, 2016 at 7:03 PM, Shawn Lin <shawn.lin at rock-chips.com> 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



More information about the Linux-rockchip mailing list