[lkml]   [2011]   [Jan]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 06/13] DMAENGINE: driver for the ARM PL080/PL081 PrimeCells
    Another issue to think about in terms of API guarantees...

    Pausing an on-going transfer (which from what I read is a recent addition
    to the API) can be problematical - having tripped over one such problem
    on my Realview EB (PL081 rev1) platform.

    With the PL08x, when you program a channel up for a M->P transfer and
    enable it, the DMAC will immediately fetch data from memory and mark
    itself busy before the first DMA request from the peripheral.

    When the first DMA request occurs from the peripheral, the DMAC starts
    feeding this data to the peripheral, and fetches new data into its FIFO
    as required.

    The problem comes when the peripheral stops requesting data from the
    DMAC, and then you want to pause it. Current code sets the HALT bit,
    and then spins indefinitely for the BUSY bit to clear - which it will
    only ever do if the peripheral drains the FIFO. As a result of botched
    DMA signal selection code for the Realview EB, the DMA signals never
    went active, and on terminate_all() the CPU locked up solid (no
    interrupts) spinning on the DMAC to clear the busy bit.

    It may be worth specifying that a pause followed by a resume is
    expected to never lose data - and that drivers must check the result
    of a request to pause the channel. If DMA engine drivers are unable
    to pause the channel within a reasonable amount of time, they should
    return -ETIMEOUT, so they know that there may still be data that
    could still be transferred to the peripheral.

    One important note about this condition is that with the DMA channel
    marked BUSY+HALT and the channel being configured for M->P, the
    peripheral can still transfer data from the DMAC to itself, and this
    causes the transfer size value in the CNTL register to decrement (since
    reading the CNTL register returns the number of transfers _to_ the
    destination, not the number of transfers from the source.)

    I've changed the comment against pl08x_pause_phy_chan() to this:

    * Pause the channel by setting the HALT bit.
    * For M->P transfers, pause the DMAC first and then stop the peripheral -
    * the FIFO can only drain if the peripheral is still requesting data.
    * (note: this can still timeout if the DMAC FIFO never drains of data.)
    * For P->M transfers, disable the peripheral first to stop it filling
    * the DMAC FIFO, and then pause the DMAC.

    I've not decided whether it should be possible to resume an ETIMEOUT'd
    pause request (in theory with pl08x, that's just a matter of clearing
    the HALT bit) or whether an ETIMEOUT'd pause request should restore
    the previous HALT bit setting (possibly re-enabling transfers on the
    channel.) Allowing it to be re-enabled in some way would be the safest
    thing in terms of data integrity for the reason mentioned in the
    "important note" above.

    Dan, Linus, any thoughts?

     \ /
      Last update: 2011-01-03 16:23    [W:0.063 / U:41.420 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site