Re: RFC: usb: musb: Changes proposed for adding CPPI4.1 DMA

From: Sergei Shtylyov
Date: Thu Feb 02 2012 - 07:03:38 EST


On 02-02-2012 15:49, Felipe Balbi wrote:

As a next step to dma-engine based cppi4.1 driver implementation
this RFC has the overview of changes in the musb driver.
RFC on CPPI slave driver changes will follow next.

Overview of changes in the musb driver

1)Add a dma-engine.c file in the drivers/usb/musb folder
2)This file will host the current musb dma APIs and translates them to
dmaengine APIs.
3)This will help to keep the changes in drivers/usb/musb/musb* files
minimal and also to retain compatibility other DMA (Mentor etc.)
drivers which are yet to be moved to drivers/dma
4)drivers/usb/musb/dma-engine.c, will wrap the dmaengine APIs to
make existing musb APIs compatible.
5)drivers/usb/musb/dma-engine.c file will implement the filter
functions and also implement .dma_controller_create (allocates
& provides "dma_controller" object) and .dma_controller_delete
6)CPPI4.1 DMA specific queue and buffer management will be internal
to slave CPPI DMA driver implementation.

You mean drivers/dma/ driver?


I think you are forgotting that CPPI 4.1 MUSB
has some registers controlling DMA/interrupts beside those of CPPI 4.1
controller and MUSB core itself. How do they fit in your scheme?

We have been discussing on how to handle these in slave driver and

These certainly cannot be handled in the slave driver because the
registers are different for every controller implementation and, the
main thing, they don't belong to CPPI 4.1 as such.

Felipe suggested to use device tree for differences in register maps
among different platforms.

I do see issues in reading wrapper interrupt status register and then
calling musb_interrupt() [defined inside musb_core.c] from slave driver.

I have been thinking about that lately. In the end of the day, I want to
remove direct dependencies between musb_core and glue. So what I was
thinking about goes like so:

Glue layer basically has to prepare musb->int_usb, musb->int_tx and
musb->int_rx for musb. Maybe handle some glue specific stuff and so on,
but the IRQ line still belongs to MUSB.

So the idea would be to add something like:


those would default to basic:

musb_readb(musb->mregs, MUSB_INTRUSB);
musb_readw(musb->mregs, MUSB_INTRTX);
musb_readw(musb->mregs, MUSB_INTRRX);

if platform ops aren't passed. So, it would look something like:

diff --git a/drivers/usb/musb/musb_core.c b/drivers/usb/musb/musb_core.c
index 72a424d..ba0bcc2 100644
--- a/drivers/usb/musb/musb_core.c
+++ b/drivers/usb/musb/musb_core.c
@@ -1488,9 +1488,9 @@ static irqreturn_t generic_interrupt(int irq, void *__hci)

spin_lock_irqsave(&musb->lock, flags);

- musb->int_usb = musb_readb(musb->mregs, MUSB_INTRUSB);
- musb->int_tx = musb_readw(musb->mregs, MUSB_INTRTX);
- musb->int_rx = musb_readw(musb->mregs, MUSB_INTRRX);
+ musb->int_usb = musb_platform_read_intusb(musb->controller);
+ musb->int_tx = musb_platform_read_inttx(musb->controller);
+ musb->int_rx = musb_platform_read_intrx(musb->controller);

if (musb->int_usb || musb->int_tx || musb->int_rx)
retval = musb_interrupt(musb);

those would make sure to prepare the cached IRQ status registers for
MUSB core.

Keep in mind that this is only necessary because on
DaVinci/OMAP-L13x/AM35x devices you guys have decided to make the
wrapper read the IRQ status register from MUSB address space. And
because those are clear-on-read, we're screwed.

Oh well, this is the best I could come up with. Any problems you guys
see ?

On DaVinci/OMAP-L1x these 3 calls need to extract data from a
single 32-bit register, so that doesn't seem a good idea to me. The

that's a problem on DaVinci/OMAP-L1x.

current scheme seems OK to me. Or either implement a signle function
to read all 3 interrupt masks...


I wanted to avoid glue layer having to access the musb pointer directly.
If I implement musb_platform_read_ints() I will have to pass musb as
argument, or agree on another way to return the values. Thanks, but no

I want to remove direct access of musb from glue layer, and at some
point I will have to do it in order to fix a few problems we might still
have with modules, basically because glue layer could be trying to
access memory which was freed already.

We can do:

void musb_platform_read_ints(u8 *usb, u8 *tx, u8 *tx);

That's what I thought first about but then got lazy. :-)

WBR, Sergei
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at