[PATCH 0/4] Residue patches for rcar-dmac from renesas-drivers

* [PATCH 0/4] Residue patches for rcar-dmac from renesas-drivers
@ 2016-06-15 11:13 Niklas Söderlund
  2016-06-15 11:13 ` [PATCH 1/4] dma: rcar-dma: use result of updated get_residue in tx_status Niklas Söderlund
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Niklas Söderlund @ 2016-06-15 11:13 UTC (permalink / raw)
  To: dmaengine, linux-renesas-soc
  Cc: vinod.koul, geert+renesas, laurent.pinchart+renesas, mfarooq,
	Niklas Söderlund

Hi all,

I have looked over the DMA residue branches in renesas-drivers tree
(topic/rcar-dmac-hamza-v3 and topic/rcar-dmac-residue-v1) hoping to 
bring them to upstream. The original author for the bulk of the patches 
Hamza Farooq have not shown any activity since autumn 2015.

In this cover letter I talk about all the patches in the above branches 
but for obvious reasons only patches I think should be forwarded to 
upstream are included in the series. All patches are based ontop of 
v4.7-rc1 and are tested on Koelsch using Geerts sertest tool to generate 
DMA traffic. The highest baud I could get to work at was 921600 bps.  
>From reading the original mail threads my conclusion is that this is 
good enough since all errors reported by Hamza Farooq where related to 
the serial sh-sci driver.

* Patches from topic/rcar-dmac-hamza-v3

- dma: rcar-dma: add wait after stopping dma engine
    In a perfect world one should check that RCAR_DMACHCR_DE is read 
    back as 0 after it have been cleared. The documentation clearly 
    states that one should do so.

    In the real world the worst case time for this register to be 
    cleared as Morimoto-san checked with HW is 700us. Laurent later 
    pointed out this is too long to busy loop over since this is done
    from both interrupt context and user context with a spinlock hold.

    Also in the real world the rcar-dmac driver WARN_ON_ONCE() that 
    RCAR_DMACHCR_DE is not set before it attempts to start an transferee 
    in rcar_dmac_chan_start_xfer(). So if this ever becomes a problem we 
    will know about it. At least I have never seen this warning while 
    using DMA. Whit this in mind I have not tried to implement a fix for 
    this and I think the original patch can be dropped.

- dma: rcar-dma: Added dma_pause operation to rcar_dma driver
    This patch adds support for device pause operation, but not the 
    resume operation. As pointed out in the original mail thread the 
    pausing must not destroy any already started transaction which it do 
    in its current form according to its author.

    Since the feature is incomplete and the use case never documented I 
    propose we drop this patch and if we can find the use case create a 
    separate task to add pause/resume functionality to the driver later.

- dma: rcar-dma: check if complete DMA packet received but not processed
    This patch is already addressed by Laruent in the original maling 
    thread:

    "I don't think this will work. In particular rcar_dmac_tx_status() 
    can be called with a cookie corresponding to a transfer not yet 
    scheduled. A pending transfer complete interrupt doesn't mean that 
    that cookie is complete.

    Furthermore I'm not sure this will really improve performances. The 
    dma_cookie_status() call below just compares cookie values, I don't 
    expect it to be much slower than a register read."

    I can see no reason to disagree with Laruent here and I propose this 
    patch is dropped.

- dma: rcar-dma: use result of updated get_residue in tx_status
    Patch is IMHO good but needs a explanation in commit message to 
    address the questions raised in the original mail thread. I have 
    added en explanation and included the original patch.

- dma: rcar-dma: warn if transfer cannot start as TE = 1
    Patch is a bit oddly implemented and uses new functionality from 
    patch 'dma: rcar-dma: add wait after stopping dma engine' above. I 
    do however think the patch have merit. One should make sure 
    according to the documentation that RCAR_DMACHCR_TE is not set 
    before starting a transfer. I have reworked the check and proposed a 
    different solution.

- dma: rcar-dma: Fixed active descriptor initializing
    Patch is clearly a bugfix and looks good. Laurent had a good comment 
    in the original mail thread about how it can be made differently by 
    reworking rcar_dmac_free_chan_resources() to call 
    rcar_dmac_chan_reinit() which would correctly set 
    rchan->desc.running to NULL.

    I have tried to implement this but failed. In my attempts to 
    implement Laurents proposal which I thought would be trivial the 
    kernel locks-up after a while with 'rcu_sched self-detected stall on 
    CPU'. And I'm not smart enough to figure out why so I have kept the 
    original patch so the bug fix part can be merged since that in 
    itself is good.

* Patch from topic/rcar-dmac-residue-v1

- dmaengine: rcar-dmac: Fix residue reporting for pending descriptors
    Patch is good AFIK. There where one report on the maling list by 
    Hamza Farooq that state he saw lots of warnings coming from the 
    WARN() introduced in this patch. I have run lots of testes using 
    Geers nice sertest tool and have not once seen the warning once. The 
    patch is included in this series.

Laurent Pinchart (1):
  dmaengine: rcar-dmac: Fix residue reporting for pending descriptors

Muhammad Hamza Farooq (2):
  dma: rcar-dma: use result of updated get_residue in tx_status
  dma: rcar-dma: Fixed active descriptor initializing

Niklas Söderlund (1):
  dmaengine: rcar-dmac: warn if transfer cannot start as TE = 1

 drivers/dma/sh/rcar-dmac.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

-- 
2.8.3

^ permalink raw reply	[flat|nested] 11+ messages in thread