Message-ID: <0CE8B6BE3C4AD74AB97D9D29BD24E552023221DE@CORPEXCH1.na.ads.idt.com>
Date:	Tue, 18 Oct 2011 06:51:05 -0700
From:	"Bounine, Alexandre" <Alexandre.Bounine@....com>
To:	Jassi Brar <jaswinder.singh@...aro.org>
CC:	"Williams, Dan J" <dan.j.williams@...el.com>,
	Vinod Koul <vinod.koul@...el.com>,
	Russell King <rmk@....linux.org.uk>,
	Barry Song <21cnbao@...il.com>, <linux-kernel@...r.kernel.org>,
	DL-SHA-WorkGroupLinux <workgroup.linux@....com>,
	Dave Jiang <dave.jiang@...el.com>
Subject: RE: [PATCHv4] DMAEngine: Define interleaved transfer request api

On Tue, Oct 18, 2011 at 1:45 AM, Jassi Brar <jaswinder.singh@...aro.org> wrote:
> 
> On 18 October 2011 02:37, Bounine, Alexandre
> <Alexandre.Bounine@....com> wrote:
> > On Mon, Oct 17, 2011 at 3:30 PM, Jassi Brar
> <jaswinder.singh@...aro.org>
> > wrote:
> > ... skip ...
> >> > This is a source of the problem for RIO - DMA controller driver
> >> creates
> >> > virtual channels statically. RapidIO may use 8- or 16-bit destID.
> >> > In this case we need to create 256 or 64K virtual channels if we
> >> > want to cover all possible targets on single RIO port. Adding an
> >> extra
> >> > controller/net multiplies that number. Considering that not every
> >> > device will need a data transfer from a given node static
> allocation
> >> > will
> >> > create even more wasted resources.
> >> >
> >> Please excuse my rudimentary knowledge of RapidIO but I am tempted
> >> to ask why not register channels only for those targets that are
> >> actually
> >> detected and enumerated?
> >>
> > Two reasons:
> > - possibility of hot device insertion/removal
> ... but the linux RIO stack doesn't seem to support hotplug.
> Enumeration/discovery is done only once during boot.
> Am I overlooking something ?
> 
You are right about the current status of enumeration/discovery.
Hotplug support is in updates pipeline and will be added in planned
order for RIO patches. There are some other patches that should
go ahead of it (e.g. DMA ;) ).
Device insertion/removal notification is already in place but is
not handled yet.

We do not want to have a RapidIO DMA implementation that may become
obsolete in six month. In addition there are some user's implementations
of RapidIO hotplug which we should try not to break if they will add DMA.  

> > - there is no advance knowledge of which target device may require
> DMA
> >  service. A device driver for the particular target device is
> expected
> >  to request DMA service if required.
> >
> IMHO 1 channel per real device is an acceptable 'overhead'.
> Already many SoCs register dozens of channels but only a couple
> of them are actually used.
>
Is 64K of virtual channels per RIO port is acceptable?
Freescale's QorIQ 4080 SoC has two RIO controllers.
Designs with tsi712 may have even more: up to 4 is a reasonable estimate.
 
> >> > There is nothing that absence of full 66-bit addressing blocks
> now.
> >> > So far we are not aware about implementations that use 66-bit
> >> address.
> >> >
> >> Thanks for the important info.
> >> If I were you, I would postpone enabling support for 66-bit
> addressing
> >> esp when it soo affects the dmaengine API.
> >> Otherwise, you don't just code unused feature, but also put
> > constraints
> >> on development/up-gradation of the API in future, possibly, nearer
> > than
> >> real need of the feature.
> >>
> >> If we postpone 66-bit addressing to when it arrives, we can
> >> 1) Attach destID to the virtual channel's identity
> >> 2) Use device_prep_dma_memcpy so as to be able to change
> >>     target address for every transfer. Or use prep_slave, depending
> >>     upon nature of address at target endpoint.
> >> 3) Use slave_config to set wr_type if it remains same for enough
> >>     consecutive transfers to the same target (only you could strike
> >>     the balance between idealism and pragmatism).
> >>
> > With item #1 above being a separate topic, I may have a problem with
> #2
> > as well: dma_addr_t is sized for the local platform and not
> guaranteed
> > to be a 64-bit value (which may be required by a target).
> > Agree with #3 (if #1 and #2 work).
> >
> Perhaps simply change dma_addr_t to u64 in dmaengine.h alone ?
> 
Adding an extra parameter to prep_slave_sg() looks much better to me.
That tweak for prep_dma_memcopy() may be more unsafe than the
extra param in prep_slave_sg().
Plus dma_memcopy does not fit logically for RIO as good as dma_slave.
For RIO we have only one buffer in local memory (as slave).
We just need to pass more info to the transfer prep routine.
 
> >> > This does not prevent someone from designing RIO compliant
> endpoint
> >> > device which gives interpretation to these two bits in addition to
> >> full 64-bit
> >> > addressing of their platform.
> >> >
> >> It sounds as if the 2bits are 'Vendor-Specific' ?
> >>
> > They are not 'Vendor-Specific' in the RIO spec (just address),
> > but HW vendors may give them such meaning. I just wanted to say
> > that HW designers may follow the same logic as we do here and
> > decide to give those bits a special meaning because "no one uses
> > them".
> >
> We should discount that. Irrespective of linux RIO stack, a vendor
> would be at fault if it assigns different meaning to the upper 2bits
> while the specs deem them just MSB of 66-bit addresses.
> 
Not a big deal. We may limit RIO address to 64-bits if this is needed
for the DMA API adopted for RIO. Let's define the right API first ;). 

> > So far this is a theoretical possibility and we are not
> > aware about any designs of this type. We may put a big warning
> > note about 64-bit limitation in RapidIO documentation section.
> >
> Yes, please.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists