Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1362296

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

Show all headers | View raw

Shawn,

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

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE_DEFER Doug Anderson <dianders@chromium.org> - 2016-03-22 00:40 +0100
  Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with  EPROBE_DEFER Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-22 03:10 +0100
    Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE_DEFER Doug Anderson <dianders@chromium.org> - 2016-03-22 03:40 +0100
      Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with  EPROBE_DEFER Shawn Lin <shawn.lin@rock-chips.com> - 2016-03-22 04:00 +0100
        Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with EPROBE_DEFER Doug Anderson <dianders@chromium.org> - 2016-03-22 04:40 +0100
          Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with  EPROBE_DEFER Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com> - 2016-03-22 14:20 +0100
  Re: [PATCH 3/3] spi: rockchip: check requesting dma channel with  EPROBE_DEFER Dan Carpenter <dan.carpenter@oracle.com> - 2016-03-22 13:10 +0100

csiph-web