lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 02 Feb 2012 16:02:21 +0400 From: Sergei Shtylyov <sshtylyov@...sta.com> To: balbi@...com CC: "Gupta, Ajay Kumar" <ajay.gupta@...com>, "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "Pasupathy, Visuvanadan" <vichu@...com> Subject: Re: RFC: usb: musb: Changes proposed for adding CPPI4.1 DMA Hello. 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? >>>>>> yes. >>>>>>> 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: >>> musb_platform_read_intrusb() >>> musb_platform_read_intrtx() >>> musb_platform_read_intrrx() >>> 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... >> musb_platform_read_ints() > 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 > thanks. > 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@...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