Re: [PATCH] dmaengine: pl330: use subsys_initcall

Ray Jui
2014-10-21T16:17:58Z

On 10/21/2014 3:45 AM, Vinod Koul wrote:
> On Fri, Oct 17, 2014 at 06:39:41PM +0200, Lars-Peter Clausen wrote:
>> On 10/17/2014 06:18 PM, Ray Jui wrote:
>>> On 10/17/2014 4:15 AM, Lars-Peter Clausen wrote:
>>>> On 10/17/2014 09:35 AM, Vinod Koul wrote:
>>>>> On Fri, Oct 17, 2014 at 09:45:45AM +0200, Lars-Peter Clausen wrote:
>>>>>> On 10/17/2014 02:48 AM, Ray Jui wrote:
>>>>>>> As part of subsystem that many slave drivers depend on, it's more
>>>>>>> appropriate for the pl330 DMA driver to be initialized at
>>>>>>> subsys_initcall than device_initcall
>>>>>>
>>>>>> Well, we do have -EPROBE_DEFER these days to handle these kinds of
>>>>>> dependencies so we no longer have to these kinds of manual init
>>>>>> reordering tricks.
>>>>> How ould that work?
>>>>>
>>>>> Consider for example SPI and dmanegine. SPI driver got probed, then to
>>>>> start
>>>>> a transaction requested a channel... while dmaengine driver is still
>>>>> getting
>>>>> probed/not probed yet. So SPI driver didnt get a channel.
>>>>>
>>>>
>>>> Ideally the SPI driver requests the channel in probe function and if the
>>>> DMA controller is not yet probed returns EPROBE_DEFER. If the SPI driver
>>>> requests the channel in the transfer handler it needs to deal with being
>>>> able to fall back to non DMA transfers anyway so this shouldn't be a
>>>> problem.
>>> So in the case of the spi-pl022 driver. It requests the channel in probe
>>> function. And obviously DMA is not mandatory, so when the channel request
>>> fails the probe won't fail and instead it falls back to PIO. In this case,
>>> can you recommend a different way to solve this problem without having the
>>> DMA driver probed earlier than its slaves?
>>
>>
>> dma_request_slave_channel() has the problem that we can't
>> differentiate between no channel provided and channel provided but
>> the dma driver hasn't probed yet. The function will return NULL in
>> both cases. But Stephen Warren added
>> dma_request_slave_channel_reason() a while ago to solve this
>> problem. This function returns a ERR_PTR. If it returns
>> ERR_PTR(-EPROBE_DEFER) it means that a channel has been provided but
>> the DMA driver hasn't probed yet. In this case the SPI driver should
>> return -EPROBE_DEFER to try again later. If the function returns a
>> different error code that means that it was not possible to get the
>> DMA channel and it should fall back to PIO.
> So when should the SPI here check, if dmaengine is available. The client
> doesn't grab channel in its probe, so the client cannot return
> -EPROBE_DEFER.
>
Currently the spi-pl022 driver requests its DMA channel in
pl022_dma_autoprobe, called during driver probe. Lars suggested changing
the dma_request_slave_channel call to dma_request_slave_channel_reason.
From my understanding, dma_request_slave_channel_reason should return
-EPROBE_DEFER if the DMA controller (pl330) device node and its channels
are declared in device tree but the driver hasn't been probed. The SPI
driver can then return -EPROBE_DEFER, which will cause its probe to be
done again, at a later time. By then the pl330 driver will be ready.

This will solve our problem, because we don't care when SPI is probed,
but we want to use DMA instead of PIO. But for the original problem that
Linus tried to solve by moving spi probe to subsys_initcall in 25c8e03b,
if one wants spi probe to be done that early, it will not be able to use
DMA with the currently proposed change.