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

Groups > linux.kernel > #1528286 > unrolled thread

Tearing down DMA transfer setup after DMA client has finished

Back to article view | Back to linux.kernel

Contents

  Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-23 11:30 +0100
    Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-23 13:20 +0100
      Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-23 13:50 +0100
        Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-23 18:30 +0100
          Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-24 12:00 +0100
            Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-24 15:20 +0100
              Re: Tearing down DMA transfer setup after DMA client has finished Mason <slash.tmp@free.fr> - 2016-11-24 16:30 +0100
                Re: Tearing down DMA transfer setup after DMA client has finished Måns Rullgård <mans@mansr.com> - 2016-11-24 17:40 +0100
    Re: Tearing down DMA transfer setup after DMA client has finished Vinod Koul <vinod.koul@intel.com> - 2016-11-25 05:50 +0100

#1528286 — Tearing down DMA transfer setup after DMA client has finished

Hello,

On my platform, setting up a DMA transfer is a two-step process:

1) configure the "switch box" to connect a device to a memory channel
2) configure the transfer details (address, size, command)

When the transfer is done, the sbox setup can be torn down,
and the DMA driver can start another transfer.

The current software architecture for my NFC (NAND Flash controller)
driver is as follows (for one DMA transfer).

  sg_init_one
  dma_map_sg
  dmaengine_prep_slave_sg
  dmaengine_submit
  dma_async_issue_pending
  configure_NFC_transfer
  wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
  wait_for_NFC_idle
  dma_unmap_sg


The problem is that the DMA driver tears down the sbox setup
as soon as it receives the IRQ. However, when writing to the
device, the interrupt only means "I have pushed all data from
memory to the memory channel". These data have not reached
the device yet, and may still be "in flight". Thus the sbox
setup can only be torn down after the NFC is idle.

How do I call back into the DMA driver after wait_for_NFC_idle,
to request sbox tear down?

The new architecture would become:

  sg_init_one
  dma_map_sg
  dmaengine_prep_slave_sg
  dmaengine_submit
  dma_async_issue_pending
  configure_NFC_transfer
  wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
  wait_for_NFC_idle
  request_sbox_tear_down /*** HOW TO DO THAT ***/
  dma_unmap_sg


As far as I can tell, my NFC driver should call dmaengine_synchronize ??
(In other words request_sbox_tear_down == dmaengine_synchronize)

So the DMA driver should implement the device_synchronize hook,
and tear the sbox down in that function.

Is that correct? Or am I on the wrong track?

Regards.

[toc] | [next] | [standalone]

#1528358

Mason <slash.tmp@free.fr> writes:

> Hello,
>
> On my platform, setting up a DMA transfer is a two-step process:
>
> 1) configure the "switch box" to connect a device to a memory channel
> 2) configure the transfer details (address, size, command)
>
> When the transfer is done, the sbox setup can be torn down,
> and the DMA driver can start another transfer.
>
> The current software architecture for my NFC (NAND Flash controller)
> driver is as follows (for one DMA transfer).
>
>   sg_init_one
>   dma_map_sg
>   dmaengine_prep_slave_sg
>   dmaengine_submit
>   dma_async_issue_pending
>   configure_NFC_transfer
>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>   wait_for_NFC_idle
>   dma_unmap_sg
>
> The problem is that the DMA driver tears down the sbox setup
> as soon as it receives the IRQ. However, when writing to the
> device, the interrupt only means "I have pushed all data from
> memory to the memory channel". These data have not reached
> the device yet, and may still be "in flight". Thus the sbox
> setup can only be torn down after the NFC is idle.
>
> How do I call back into the DMA driver after wait_for_NFC_idle,
> to request sbox tear down?
>
> The new architecture would become:
>
>   sg_init_one
>   dma_map_sg
>   dmaengine_prep_slave_sg
>   dmaengine_submit
>   dma_async_issue_pending
>   configure_NFC_transfer
>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>   wait_for_NFC_idle
>   request_sbox_tear_down /*** HOW TO DO THAT ***/
>   dma_unmap_sg
>
> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
> (In other words request_sbox_tear_down == dmaengine_synchronize)
>
> So the DMA driver should implement the device_synchronize hook,
> and tear the sbox down in that function.
>
> Is that correct? Or am I on the wrong track?

dmaengine_synchronize() is not meant for this.  See the documentation:

/**
 * dmaengine_synchronize() - Synchronize DMA channel termination
 * @chan: The channel to synchronize
 *
 * Synchronizes to the DMA channel termination to the current context. When this
 * function returns it is guaranteed that all transfers for previously issued
 * descriptors have stopped and and it is safe to free the memory assoicated
 * with them. Furthermore it is guaranteed that all complete callback functions
 * for a previously submitted descriptor have finished running and it is safe to
 * free resources accessed from within the complete callbacks.
 *
 * The behavior of this function is undefined if dma_async_issue_pending() has
 * been called between dmaengine_terminate_async() and this function.
 *
 * This function must only be called from non-atomic context and must not be
 * called from within a complete callback of a descriptor submitted on the same
 * channel.
 */

This is for use after a dmaengine_terminate_async() call to wait for the
dma engine to finish whatever it was doing.  This is not the problem
here.  Your problem is that the dma engine interrupt fires before the
transfer is actually complete.  Although you get an indication from the
target device when it has received all the data, there is no way to make
the dma driver wait for this.

-- 
Måns Rullgård

[toc] | [prev] | [next] | [standalone]

#1528374

On 23/11/2016 13:13, Måns Rullgård wrote:

> Mason wrote:
> 
>> On my platform, setting up a DMA transfer is a two-step process:
>>
>> 1) configure the "switch box" to connect a device to a memory channel
>> 2) configure the transfer details (address, size, command)
>>
>> When the transfer is done, the sbox setup can be torn down,
>> and the DMA driver can start another transfer.
>>
>> The current software architecture for my NFC (NAND Flash controller)
>> driver is as follows (for one DMA transfer).
>>
>>   sg_init_one
>>   dma_map_sg
>>   dmaengine_prep_slave_sg
>>   dmaengine_submit
>>   dma_async_issue_pending
>>   configure_NFC_transfer
>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>   wait_for_NFC_idle
>>   dma_unmap_sg
>>
>> The problem is that the DMA driver tears down the sbox setup
>> as soon as it receives the IRQ. However, when writing to the
>> device, the interrupt only means "I have pushed all data from
>> memory to the memory channel". These data have not reached
>> the device yet, and may still be "in flight". Thus the sbox
>> setup can only be torn down after the NFC is idle.
>>
>> How do I call back into the DMA driver after wait_for_NFC_idle,
>> to request sbox tear down?
>>
>> The new architecture would become:
>>
>>   sg_init_one
>>   dma_map_sg
>>   dmaengine_prep_slave_sg
>>   dmaengine_submit
>>   dma_async_issue_pending
>>   configure_NFC_transfer
>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>   wait_for_NFC_idle
>>   request_sbox_tear_down /*** HOW TO DO THAT ***/
>>   dma_unmap_sg
>>
>> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
>> (In other words request_sbox_tear_down == dmaengine_synchronize)
>>
>> So the DMA driver should implement the device_synchronize hook,
>> and tear the sbox down in that function.
>>
>> Is that correct? Or am I on the wrong track?
> 
> dmaengine_synchronize() is not meant for this.  See the documentation:
> 
> /**
>  * dmaengine_synchronize() - Synchronize DMA channel termination
>  * @chan: The channel to synchronize
>  *
>  * Synchronizes to the DMA channel termination to the current context. When this
>  * function returns it is guaranteed that all transfers for previously issued
>  * descriptors have stopped and and it is safe to free the memory assoicated
>  * with them. Furthermore it is guaranteed that all complete callback functions
>  * for a previously submitted descriptor have finished running and it is safe to
>  * free resources accessed from within the complete callbacks.
>  *
>  * The behavior of this function is undefined if dma_async_issue_pending() has
>  * been called between dmaengine_terminate_async() and this function.
>  *
>  * This function must only be called from non-atomic context and must not be
>  * called from within a complete callback of a descriptor submitted on the same
>  * channel.
>  */
> 
> This is for use after a dmaengine_terminate_async() call to wait for the
> dma engine to finish whatever it was doing.  This is not the problem
> here.  Your problem is that the dma engine interrupt fires before the
> transfer is actually complete.  Although you get an indication from the
> target device when it has received all the data, there is no way to make
> the dma driver wait for this.

Hello Mans,

I'm confused. Are you saying there is no solution to my problem
within the existing DMA framework?

In its current form, the tangox-dma.c driver will fail randomly
for writes to a device (SATA, NFC).

Maybe an extra hook can be added to the DMA framework?

I'd like to hear from the framework's maintainers. Perhaps they
can provide some guidance.

Regards.

[toc] | [prev] | [next] | [standalone]

#1528621

Mason <slash.tmp@free.fr> writes:

> On 23/11/2016 13:13, Måns Rullgård wrote:
>
>> Mason wrote:
>> 
>>> On my platform, setting up a DMA transfer is a two-step process:
>>>
>>> 1) configure the "switch box" to connect a device to a memory channel
>>> 2) configure the transfer details (address, size, command)
>>>
>>> When the transfer is done, the sbox setup can be torn down,
>>> and the DMA driver can start another transfer.
>>>
>>> The current software architecture for my NFC (NAND Flash controller)
>>> driver is as follows (for one DMA transfer).
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   dma_unmap_sg
>>>
>>> The problem is that the DMA driver tears down the sbox setup
>>> as soon as it receives the IRQ. However, when writing to the
>>> device, the interrupt only means "I have pushed all data from
>>> memory to the memory channel". These data have not reached
>>> the device yet, and may still be "in flight". Thus the sbox
>>> setup can only be torn down after the NFC is idle.
>>>
>>> How do I call back into the DMA driver after wait_for_NFC_idle,
>>> to request sbox tear down?
>>>
>>> The new architecture would become:
>>>
>>>   sg_init_one
>>>   dma_map_sg
>>>   dmaengine_prep_slave_sg
>>>   dmaengine_submit
>>>   dma_async_issue_pending
>>>   configure_NFC_transfer
>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>   wait_for_NFC_idle
>>>   request_sbox_tear_down /*** HOW TO DO THAT ***/
>>>   dma_unmap_sg
>>>
>>> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
>>> (In other words request_sbox_tear_down == dmaengine_synchronize)
>>>
>>> So the DMA driver should implement the device_synchronize hook,
>>> and tear the sbox down in that function.
>>>
>>> Is that correct? Or am I on the wrong track?
>> 
>> dmaengine_synchronize() is not meant for this.  See the documentation:
>> 
>> /**
>>  * dmaengine_synchronize() - Synchronize DMA channel termination
>>  * @chan: The channel to synchronize
>>  *
>>  * Synchronizes to the DMA channel termination to the current context. When this
>>  * function returns it is guaranteed that all transfers for previously issued
>>  * descriptors have stopped and and it is safe to free the memory assoicated
>>  * with them. Furthermore it is guaranteed that all complete callback functions
>>  * for a previously submitted descriptor have finished running and it is safe to
>>  * free resources accessed from within the complete callbacks.
>>  *
>>  * The behavior of this function is undefined if dma_async_issue_pending() has
>>  * been called between dmaengine_terminate_async() and this function.
>>  *
>>  * This function must only be called from non-atomic context and must not be
>>  * called from within a complete callback of a descriptor submitted on the same
>>  * channel.
>>  */
>> 
>> This is for use after a dmaengine_terminate_async() call to wait for the
>> dma engine to finish whatever it was doing.  This is not the problem
>> here.  Your problem is that the dma engine interrupt fires before the
>> transfer is actually complete.  Although you get an indication from the
>> target device when it has received all the data, there is no way to make
>> the dma driver wait for this.
>
> Hello Mans,
>
> I'm confused. Are you saying there is no solution to my problem
> within the existing DMA framework?
>
> In its current form, the tangox-dma.c driver will fail randomly
> for writes to a device (SATA, NFC).
>
> Maybe an extra hook can be added to the DMA framework?
>
> I'd like to hear from the framework's maintainers. Perhaps they
> can provide some guidance.

You could have the dma descriptor callback wait for the receiving device
to finish.  Bear in mind this runs from a tasklet, so it's not allowed
to sleep.

-- 
Måns Rullgård

[toc] | [prev] | [next] | [standalone]

#1529166

On 23/11/2016 18:21, Måns Rullgård wrote:

> Mason writes:
> 
>> On 23/11/2016 13:13, Måns Rullgård wrote:
>>
>>> Mason wrote:
>>>
>>>> On my platform, setting up a DMA transfer is a two-step process:
>>>>
>>>> 1) configure the "switch box" to connect a device to a memory channel
>>>> 2) configure the transfer details (address, size, command)
>>>>
>>>> When the transfer is done, the sbox setup can be torn down,
>>>> and the DMA driver can start another transfer.
>>>>
>>>> The current software architecture for my NFC (NAND Flash controller)
>>>> driver is as follows (for one DMA transfer).
>>>>
>>>>   sg_init_one
>>>>   dma_map_sg
>>>>   dmaengine_prep_slave_sg
>>>>   dmaengine_submit
>>>>   dma_async_issue_pending
>>>>   configure_NFC_transfer
>>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>>   wait_for_NFC_idle
>>>>   dma_unmap_sg
>>>>
>>>> The problem is that the DMA driver tears down the sbox setup
>>>> as soon as it receives the IRQ. However, when writing to the
>>>> device, the interrupt only means "I have pushed all data from
>>>> memory to the memory channel". These data have not reached
>>>> the device yet, and may still be "in flight". Thus the sbox
>>>> setup can only be torn down after the NFC is idle.
>>>>
>>>> How do I call back into the DMA driver after wait_for_NFC_idle,
>>>> to request sbox tear down?
>>>>
>>>> The new architecture would become:
>>>>
>>>>   sg_init_one
>>>>   dma_map_sg
>>>>   dmaengine_prep_slave_sg
>>>>   dmaengine_submit
>>>>   dma_async_issue_pending
>>>>   configure_NFC_transfer
>>>>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>>>>   wait_for_NFC_idle
>>>>   request_sbox_tear_down /*** HOW TO DO THAT ***/
>>>>   dma_unmap_sg
>>>>
>>>> As far as I can tell, my NFC driver should call dmaengine_synchronize ??
>>>> (In other words request_sbox_tear_down == dmaengine_synchronize)
>>>>
>>>> So the DMA driver should implement the device_synchronize hook,
>>>> and tear the sbox down in that function.
>>>>
>>>> Is that correct? Or am I on the wrong track?
>>>
>>> dmaengine_synchronize() is not meant for this.  See the documentation:
>>>
>>> /**
>>>  * dmaengine_synchronize() - Synchronize DMA channel termination
>>>  * @chan: The channel to synchronize
>>>  *
>>>  * Synchronizes to the DMA channel termination to the current context. When this
>>>  * function returns it is guaranteed that all transfers for previously issued
>>>  * descriptors have stopped and and it is safe to free the memory assoicated
>>>  * with them. Furthermore it is guaranteed that all complete callback functions
>>>  * for a previously submitted descriptor have finished running and it is safe to
>>>  * free resources accessed from within the complete callbacks.
>>>  *
>>>  * The behavior of this function is undefined if dma_async_issue_pending() has
>>>  * been called between dmaengine_terminate_async() and this function.
>>>  *
>>>  * This function must only be called from non-atomic context and must not be
>>>  * called from within a complete callback of a descriptor submitted on the same
>>>  * channel.
>>>  */
>>>
>>> This is for use after a dmaengine_terminate_async() call to wait for the
>>> dma engine to finish whatever it was doing.  This is not the problem
>>> here.  Your problem is that the dma engine interrupt fires before the
>>> transfer is actually complete.  Although you get an indication from the
>>> target device when it has received all the data, there is no way to make
>>> the dma driver wait for this.
>>
>> Hello Mans,
>>
>> I'm confused. Are you saying there is no solution to my problem
>> within the existing DMA framework?
>>
>> In its current form, the tangox-dma.c driver will fail randomly
>> for writes to a device (SATA, NFC).
>>
>> Maybe an extra hook can be added to the DMA framework?
>>
>> I'd like to hear from the framework's maintainers. Perhaps they
>> can provide some guidance.
> 
> You could have the dma descriptor callback wait for the receiving device
> to finish.  Bear in mind this runs from a tasklet, so it's not allowed
> to sleep.

Thanks for the suggestion, but I don't think it works :-(

This is my DMA desc callback:

static void tango_dma_callback(void *arg)
{
	printk("%s from %pf\n", __func__, __builtin_return_address(0));
	mdelay(10000);
	printk("DONE FAKE SPINNING\n");
	complete(arg);
}

I also added
    printk("%s from %pf\n", __func__, __builtin_return_address(0));
after tangox_dma_pchan_detach(pchan);

And I get this output:

[   35.085854] SETUP DMA
[   35.088272] START NAND TRANSFER
[   35.091670] tangox_dma_pchan_start from tangox_dma_irq
[   35.096882] tango_dma_callback from vchan_complete
[   45.102513] DONE FAKE SPINNING

So the IRQ rolls in, the ISR calls tangox_dma_pchan_start,
which calls tangox_dma_pchan_detach to tear down the sbox
setup; and only sometime later does the DMA framework call
my callback function.

So far, the work-arounds I've tested are:

1) delay sbox tear-down by 10 µs in tangox_dma_pchan_detach.
2) statically setup sbox in probe, and never touch it henceforth.

WA1 is fragile, it might break for devices other than NFC.
WA2 is what I used when I wrote the NFC driver.

Can tangox_dma_irq() be changed to have the framework call
the client's callback *before* tangox_dma_pchan_start?

(Thinking out loud) The DMA_PREP_INTERRUPT requests that the
DMA framework invoke the callback from tasklet context,
maybe a different flag DMA_PREP_INTERRUPT_EX can request
calling the call-back directly from within the ISR?

(Looking at existing flags) Could I use DMA_CTRL_ACK?
Description sounds like some kind hand-shake between
client and dmaengine.

Grepping for DMA_PREP_INTERRUPT, I don't see where the framework
checks that flag to spawn the tasklet? Or is that up to each
driver individually?

Regards.

[toc] | [prev] | [next] | [standalone]

#1529328

Mason <slash.tmp@free.fr> writes:

>>> I'm confused. Are you saying there is no solution to my problem
>>> within the existing DMA framework?
>>>
>>> In its current form, the tangox-dma.c driver will fail randomly
>>> for writes to a device (SATA, NFC).
>>>
>>> Maybe an extra hook can be added to the DMA framework?
>>>
>>> I'd like to hear from the framework's maintainers. Perhaps they
>>> can provide some guidance.
>> 
>> You could have the dma descriptor callback wait for the receiving device
>> to finish.  Bear in mind this runs from a tasklet, so it's not allowed
>> to sleep.
>
> Thanks for the suggestion, but I don't think it works :-(
>
> This is my DMA desc callback:
>
> static void tango_dma_callback(void *arg)
> {
> 	printk("%s from %pf\n", __func__, __builtin_return_address(0));
> 	mdelay(10000);
> 	printk("DONE FAKE SPINNING\n");
> 	complete(arg);
> }
>
> I also added
>     printk("%s from %pf\n", __func__, __builtin_return_address(0));
> after tangox_dma_pchan_detach(pchan);
>
> And I get this output:
>
> [   35.085854] SETUP DMA
> [   35.088272] START NAND TRANSFER
> [   35.091670] tangox_dma_pchan_start from tangox_dma_irq
> [   35.096882] tango_dma_callback from vchan_complete
> [   45.102513] DONE FAKE SPINNING
>
> So the IRQ rolls in, the ISR calls tangox_dma_pchan_start,
> which calls tangox_dma_pchan_detach to tear down the sbox
> setup; and only sometime later does the DMA framework call
> my callback function.

Yes, I realised this soon after I said it.  The dma driver could be
rearranged to make it work though.

> So far, the work-arounds I've tested are:
>
> 1) delay sbox tear-down by 10 µs in tangox_dma_pchan_detach.
> 2) statically setup sbox in probe, and never touch it henceforth.
>
> WA1 is fragile, it might break for devices other than NFC.
> WA2 is what I used when I wrote the NFC driver.
>
> Can tangox_dma_irq() be changed to have the framework call
> the client's callback *before* tangox_dma_pchan_start?
>
> (Thinking out loud) The DMA_PREP_INTERRUPT requests that the
> DMA framework invoke the callback from tasklet context,
> maybe a different flag DMA_PREP_INTERRUPT_EX can request
> calling the call-back directly from within the ISR?
>
> (Looking at existing flags) Could I use DMA_CTRL_ACK?
> Description sounds like some kind hand-shake between
> client and dmaengine.
>
> Grepping for DMA_PREP_INTERRUPT, I don't see where the framework
> checks that flag to spawn the tasklet? Or is that up to each
> driver individually?

Those flags all have defined meanings and abusing them for other things
is a bad idea.  As far as possible, device drivers should work with any
dma driver.

-- 
Måns Rullgård

[toc] | [prev] | [next] | [standalone]

#1529447

On 24/11/2016 15:17, Måns Rullgård wrote:

> Mason wrote:
> 
>> [   35.085854] SETUP DMA
>> [   35.088272] START NAND TRANSFER
>> [   35.091670] tangox_dma_pchan_start from tangox_dma_irq
>> [   35.096882] tango_dma_callback from vchan_complete
>> [   45.102513] DONE FAKE SPINNING
>>
>> So the IRQ rolls in, the ISR calls tangox_dma_pchan_start,
>> which calls tangox_dma_pchan_detach to tear down the sbox
>> setup; and only sometime later does the DMA framework call
>> my callback function.
> 
> Yes, I realised this soon after I said it.  The dma driver could be
> rearranged to make it work though.

There is a way to make the tasklet run and invoke the callback
before the interrupt service routine proceeds? Can you say more
about this?


>> So far, the work-arounds I've tested are:
>>
>> 1) delay sbox tear-down by 10 µs in tangox_dma_pchan_detach.
>> 2) statically setup sbox in probe, and never touch it henceforth.
>>
>> WA1 is fragile, it might break for devices other than NFC.
>> WA2 is what I used when I wrote the NFC driver.
>>
>> Can tangox_dma_irq() be changed to have the framework call
>> the client's callback *before* tangox_dma_pchan_start?
>>
>> (Thinking out loud) The DMA_PREP_INTERRUPT requests that the
>> DMA framework invoke the callback from tasklet context,
>> maybe a different flag DMA_PREP_INTERRUPT_EX can request
>> calling the call-back directly from within the ISR?
>>
>> (Looking at existing flags) Could I use DMA_CTRL_ACK?
>> Description sounds like some kind hand-shake between
>> client and dmaengine.
>>
>> Grepping for DMA_PREP_INTERRUPT, I don't see where the framework
>> checks that flag to spawn the tasklet? Or is that up to each
>> driver individually?
> 
> Those flags all have defined meanings and abusing them for other things
> is a bad idea.  As far as possible, device drivers should work with any
> dma driver.

I was asking about introducing a new flag, not abusing existing
flags. (I don't understand the semantics of DMA_CTRL_ACK.)

(FWIW, both the NFC and the MBUS agent are custom designs,
not third-party IP blocks.)

Regards.

[toc] | [prev] | [next] | [standalone]

#1529552

Mason <slash.tmp@free.fr> writes:

> On 24/11/2016 15:17, Måns Rullgård wrote:
>
>> Mason wrote:
>> 
>>> [   35.085854] SETUP DMA
>>> [   35.088272] START NAND TRANSFER
>>> [   35.091670] tangox_dma_pchan_start from tangox_dma_irq
>>> [   35.096882] tango_dma_callback from vchan_complete
>>> [   45.102513] DONE FAKE SPINNING
>>>
>>> So the IRQ rolls in, the ISR calls tangox_dma_pchan_start,
>>> which calls tangox_dma_pchan_detach to tear down the sbox
>>> setup; and only sometime later does the DMA framework call
>>> my callback function.
>> 
>> Yes, I realised this soon after I said it.  The dma driver could be
>> rearranged to make it work though.
>
> There is a way to make the tasklet run and invoke the callback
> before the interrupt service routine proceeds?

No, but it would be possible to defer the teardown to the tasklet.
Having said that, I'm not sure it's such a great idea since the tasklet
could be held up for an arbitrary length of time waiting for the target
to finish.

>>> So far, the work-arounds I've tested are:
>>>
>>> 1) delay sbox tear-down by 10 µs in tangox_dma_pchan_detach.
>>> 2) statically setup sbox in probe, and never touch it henceforth.
>>>
>>> WA1 is fragile, it might break for devices other than NFC.
>>> WA2 is what I used when I wrote the NFC driver.
>>>
>>> Can tangox_dma_irq() be changed to have the framework call
>>> the client's callback *before* tangox_dma_pchan_start?
>>>
>>> (Thinking out loud) The DMA_PREP_INTERRUPT requests that the
>>> DMA framework invoke the callback from tasklet context,
>>> maybe a different flag DMA_PREP_INTERRUPT_EX can request
>>> calling the call-back directly from within the ISR?
>>>
>>> (Looking at existing flags) Could I use DMA_CTRL_ACK?
>>> Description sounds like some kind hand-shake between
>>> client and dmaengine.
>>>
>>> Grepping for DMA_PREP_INTERRUPT, I don't see where the framework
>>> checks that flag to spawn the tasklet? Or is that up to each
>>> driver individually?
>> 
>> Those flags all have defined meanings and abusing them for other things
>> is a bad idea.  As far as possible, device drivers should work with any
>> dma driver.
>
> I was asking about introducing a new flag, not abusing existing
> flags. (I don't understand the semantics of DMA_CTRL_ACK.)

This needs more than a new flag anyhow.

> (FWIW, both the NFC and the MBUS agent are custom designs,
> not third-party IP blocks.)

Sure, but who knows what will be in the next chip?

-- 
Måns Rullgård

[toc] | [prev] | [next] | [standalone]

#1529792

On Wed, Nov 23, 2016 at 11:25:44AM +0100, Mason wrote:
> Hello,
> 
> On my platform, setting up a DMA transfer is a two-step process:
> 
> 1) configure the "switch box" to connect a device to a memory channel
> 2) configure the transfer details (address, size, command)
> 
> When the transfer is done, the sbox setup can be torn down,
> and the DMA driver can start another transfer.
> 
> The current software architecture for my NFC (NAND Flash controller)
> driver is as follows (for one DMA transfer).
> 
>   sg_init_one
>   dma_map_sg
>   dmaengine_prep_slave_sg
>   dmaengine_submit
>   dma_async_issue_pending
>   configure_NFC_transfer
>   wait_for_IRQ_from_DMA_engine // via DMA_PREP_INTERRUPT
>   wait_for_NFC_idle
>   dma_unmap_sg

Looking at thread and discussion now, first thinking would be to ensure
the transaction is completed properly and then isr fired. You may need
to talk to your HW designers to find a way for that. It is quite common
that DMA controllers will fire and complete whereas the transaction is
still in flight.

If that is not doable, then since you claim this is custom part which
other vendors wont use (hope we are wrong down the line), then we can
have a custom api,

foo_sbox_configure(bool enable, ...);

This can be invoked from NFC driver when required for configuration and
teardown. For very specific cases where people need some specific
configuration we do allow custom APIs.

Only problem with that would be it wont be a generic solution and you
seem to be fine with that.


-- 
~Vinod

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web