Re: [PATCH RFC v2 0/5] MPC512x DMA slave s/g support, OF DMA lookup

On Thu, Oct 03, 2013 at 18:00 +0400, Alexander Popov wrote:

v2013/7/14 Gerhard Sittig [off-list ref]:

quoted

this series
- introduces slave s/g support (that's support for DMA transfers which
  involve peripherals in contrast to mem-to-mem transfers)
- adds device tree based lookup support for DMA channels
- combines floating patches and related feedback which already covered
  several aspects of what the suggested LPB driver needs, to demonstrate
  how integration might be done
- carries Q&D SD card support to enable another DMA client during test,
  while this patch needs to get dropped upon pickup

changes since v1:
- re-order mpc8308 related code paths for improved readability, no
  change in behaviour, introduction of symbolic channel names here
  already
- squash 'execute() start condition' and 'terminate all' into the
  introduction of 'slave s/g prep' and 'device control' support; refuse
  s/g lists with more than one item since slave support is operational
  yet proper s/g support is missing (can get addressed later)
- always start transfers from software on MPC8308 as there are no
  external request lines for peripheral flow control
- drop dt-bindings header file and symbolic channel names in OF nodes

Changes since v2 (RFC v3 was badly formed, excuse me for that):
Part 1/5:
- use #define instead of enum since individual channels don't require
special handling.
Part 2/5:
- add a flag "will_access_peripheral" to DMA transfer descriptor
  according recommendations of Gerhard Sittig.
  This flag is set in mpc_dma_prep_memcpy() and mpc_dma_prep_slave_sg()
  and is evaluated in mpc_dma_execute() to choose a type of start for
the transfer.
- prevent descriptors of transfers which involve peripherals from
being chained together;
  each of such transfers needs hardware initiated start.
- add locking while working with struct mpc_dma_chan
  according recommendations of Lars-Peter Clausen.
- remove default nbytes value. Client kernel modules must set
  src_maxburst and dst_maxburst fields of struct dma_slave_config (dmaengine.h).
Part 6/8:
  unchanged.
Part 7/8:
  unchanged.
Part 8/8:
  unchanged.

These changes are tested on MPC5125
- with SCLPC driver (transfers between dev and mem work fine).
- with dmatest module (all 64 DMA channels can perform mem-to-mem transfers
  which can be chained in one DMA transaction).

quoted

known issues:
- it's yet to get confirmed whether MPC8308 can use slave support or
  whether the DMA controller's driver shall actively reject it, the
  information that's available so far suggests that peripheral transfers
  to IP bus attached I/O is useful and shall not get blocked right away

I'm not certain whether keeping the "cover letter" in threaded
form is appropriate.  But speaking about non-existent parts
(6-8/8), not saying what the current version is, missing stats
certainly isn't right.

Your submission style adds more work to doing review and
providing feedback than what's necessary.  You assume that others
would keep the history for you, or would do the archeology and
collect individual pieces from the past, to recover what you fail
to send out.

The messages show up in an unexpected order here (3, 4, 5, 0
first on one thread, 1, 2 then in another thread, with some 2800
messages between them) and carry conflicting subjects or version
numbers.  Formatting was broken in transport, patches won't apply.

Please do consider checking what you send out, and how you can
improve the reception's side of the process.  Try to help those
people you want to receive help from.  Try to support both kinds
of review for people coming new to the subject as well as those
who have seen a former version.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de