Re: [PATCH][1/4] usb: dma bounce buffer support using HCD_BOUNCE flag

David Brownell <[email protected]> Mon, 14 Jan 2008 23:35:52 -0800
Newsgroups gmane.linux.usb.devel
Message-ID <[email protected]>
On Monday 14 January 2008, Magnus Damm wrote:
> On Jan 14, 2008 1:03 PM, David Brownell <[email protected]> wrote:
> > For what it's worth, the SA1111 has a somewhat similar requirement
> > coming from a DMA erratum that makes every other megabyte invalid
> > in terms of DMA addressing.  So it does bounce buffering ... but
> > it's done transparently to usbcore, in its DMA mapping operations.
> 
> Yeah, that sounds like a similar requirement. I suppose we're talking
> CONFIG_DMABOUNCE and arch/arm/common/dmabounce.c?

Yes.  That could be done in a more generic manner, except for the
belief that making that be more reusable would be a Bad Thing though
and accordingly that "we" don't want to encourage such ugliness.


> My first working version did hook into the DMA mapping operations for
> the sh architecture. It used declared coherent memory as buffers which
> wasn't efficient but seemed to work pretty well.
> 
> > Could you talk a bit more about why that approach isn't appropriate
> > here?  My first guess would be that it just costs too much, and
> > this is used on systems without manny spare resources.  Including
> > for example ones where the memcpy overheads would hurt battery
> > lifetimes, even if there were spare CPU cycles.
> 
> The sm501 chip can be hooked up to several different architectures,
> and it can be attached to the system in multiple ways.

... so it needs something not specific to the ARM or SH support.


> I have some pci 
> cards here that are equipped with sm501/sm502 using pci. I use them on
> x86. Then I have some embedded sh boards from renesas where the sm501
> is hooked up using a local bus (just a CS bank I think). The sm501 is
> equipped with local memory in both cases,

That is, a private bank of DRAM rather than a block of SRAM
integrated onto that chip?  That's a slightly different model
than I've usually seen with USB, but not uncommon otherwise.

And there are advantages to having a DRAM controller and using
external RAM, instead of what's usually a way-too-small block
of expensive SRAM.  (Unless board real estate is very tight,
or the system is designed for _very_ low power operation.)


>                                    and the device is unable to 
> do dma to other parts of the system memory - at least that's true when
> it's hooked up on the local bus.

If it's just hooked up as a slave CS bank, rather than through some
symmetric interface supporting bus mastering, that'd make sense.


> I think that applies for the pci 
> configuration as well, but I there may be some magic pci bus mastering
> mode that i'm unaware of.

The SM502 seems to have a DMA controller that can be a PCI bus master,
or use local memory (DRAM possibly shared with the framebuffer).  I'd
expect it to just use standard PCI bus mastering ... though you might
need special PCI bus glue if its OHCI isn't presented the normal way.

I'm not surprised they didn't _also_ do the work needed to sync up with
some external DMA engine.  The sort where a peripheral raises its DMA
request line to load/unload its data, and then gives Linux fits because
the drivers/dma/* code doesn't (yet) believe such engines exist.  ;)


> So using architecture specific DMA mapping operations is of course
> always an option, but since the chip can be hooked up to all sorts of
> architectures i think it makes more sense to make it architecture
> independent. The only requirement for the architecture would be to
> have dma_declare_coherent_memory() support, and that interface seems
> to fit perfectly for our needs in this case.

Although I don't think there's necessarily a requirement that such
memory be given a dma-coherent mapping in kernel address space.
It's really no different than any other chunk of DRAM.  (Right?
The SM50x chip acts as a bridge to that local memory bank.)

The relevant points here being that there is current infrastructure
to declare device-local memory that's suitable for DMA ... and that
it may be the only memory suitable for such DMA, depending on the
board architecture.  So to use it, we have to go through the dma
coherent allocators.


> I also suspect that other usb host controllers exist with similar
> requirements, so having host controller and architecture independent
> support may be a good idea for other devices as well.

The example I gave was the NXP/Philips ISP chips targetted at
embedded systems ... which seem to require all DMA to be from
their own (tiny) SRAMs.  There are other embedded HCs I've seen
on discrete add-on chips which adopt that same model.  When the
HC is better integrated (part of an SOC) it tends to get rid
of such restrictions and have access to the same address space
everything else does.  (IOMMUs there are uncommon.)

 
> > ISTR this specific issue (needing memory from a particular area) came
> > up on some of the Philips/NXP chips and some other hardware too.  But:
> >
> >
> > > --- 0001/drivers/usb/core/hcd.h
> > > +++ work/drivers/usb/core/hcd.h       2008-01-13 19:54:10.000000000 +0900
> > > @@ -165,6 +165,7 @@ struct hc_driver {
> > >
> > >       int     flags;
> > >  #define      HCD_MEMORY      0x0001          /* HC regs use memory (else I/O) */
> > > +#define      HCD_BOUNCE      0x0002          /* HC needs bounce buffers */
> >
> > I think this makes "bounce buffers" become a too-overloaded term.
> >
> > These are most certainly bounce buffers ... but the significance
> > isn't that detail, but a different one:  that they're HC-local
> > memory.  That is, normal bounce buffers couldn't do the job.
> >
> > If the normal bounce buffer logic is inappropriate, then I'd suggest
> > coming up with a less generic term and symbol.  Maybe HCD_LOCAL_MEM
> > would capture all the relevant details.
> 
> Sure, I totally agree with you. I wasn't sure if a flag was the
> correct way to go either, for a while I was thinking of adding two
> callbacks for map and unmap and let them handle things for us. I do
> think a flag is cleaner though.
> 
> Any comments? Can I just change the name of the flag to HCD_LOCAL_MEM
> and repost?

OK by me.  Maybe a few other folk have opinions though ... Alan?

- Dave


> Thanks!
> 
> / magnus
> 



-------------------------------------------------------------------------
Check out the new SourceForge.net Marketplace.
It's the best place to buy or sell services for
just about anything Open Source.
http://ad.doubleclick.net/clk;164216239;13503038;w?http://sf.net/marketplace
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel