PL330: Add PL330 DMA controller driver
in [Kernel]
|
Prev: mm/vmalloc: Export purge_vmap_area_lazy()
Next: perf/scripts: tuple was set from long in both branches in python_process_event()
From: Marc Zyngier on 25 Mar 2010 05:40 On Thu, 25 Mar 2010 18:01:00 +0900, Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: > On 3/25/2010 2:44 PM, Marc Zyngier wrote: >> On Thu, 25 Mar 2010 12:17:15 +0900 >> Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: >> >>> + writew(imm, desc_pool_virt); >> > > Right. The write[bwl] is api for address ioremapped of io device. I will > change these. > >> Does anything ensure that this won't generate an unaligned access? >> > > PL330 DMA controller fetches variable length instructions that consist of > one to six bytes, so i think unaligned access is no problem. I'm not too concerned about the device side of things. I'm more worried about the CPU access when writing the 'imm' value to memory. Consider desc_pool_virt 16bit aligned when entering the function. Writing the opcode makes it unaligned and then writing the 'imm' value will result as an unaligned access. M. -- Who you jivin' with that Cosmik Debris? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joonyoung Shim on 25 Mar 2010 06:20 On 3/25/2010 6:32 PM, Marc Zyngier wrote: > On Thu, 25 Mar 2010 18:01:00 +0900, Joonyoung Shim > <jy0922.shim(a)samsung.com> wrote: >> On 3/25/2010 2:44 PM, Marc Zyngier wrote: >>> On Thu, 25 Mar 2010 12:17:15 +0900 >>> Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: >>> >>>> + writew(imm, desc_pool_virt); >> Right. The write[bwl] is api for address ioremapped of io device. I will >> change these. >> >>> Does anything ensure that this won't generate an unaligned access? >>> >> PL330 DMA controller fetches variable length instructions that consist > of >> one to six bytes, so i think unaligned access is no problem. > > I'm not too concerned about the device side of things. I'm more worried > about the CPU access when writing the 'imm' value to memory. > > Consider desc_pool_virt 16bit aligned when entering the function. Writing > the opcode makes it unaligned and then writing the 'imm' value will result > as an unaligned access. > Why desc_pool_virt should be aligned more than 16bit? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Marc Zyngier on 25 Mar 2010 06:40 On Thu, 25 Mar 2010 19:05:47 +0900, Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: > On 3/25/2010 6:32 PM, Marc Zyngier wrote: >> On Thu, 25 Mar 2010 18:01:00 +0900, Joonyoung Shim >> <jy0922.shim(a)samsung.com> wrote: >>> On 3/25/2010 2:44 PM, Marc Zyngier wrote: >>>> On Thu, 25 Mar 2010 12:17:15 +0900 >>>> Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: >>>> >>>>> + writew(imm, desc_pool_virt); >>> Right. The write[bwl] is api for address ioremapped of io device. I will >>> change these. >>> >>>> Does anything ensure that this won't generate an unaligned access? >>>> >>> PL330 DMA controller fetches variable length instructions that consist >> of >>> one to six bytes, so i think unaligned access is no problem. >> >> I'm not too concerned about the device side of things. I'm more worried >> about the CPU access when writing the 'imm' value to memory. >> >> Consider desc_pool_virt 16bit aligned when entering the function. Writing >> the opcode makes it unaligned and then writing the 'imm' value will >> result >> as an unaligned access. >> > > Why desc_pool_virt should be aligned more than 16bit? There is reason for desc_pool_virt to be 16bit aligned. It's just that you have 50% chance that it will. In such case, you will write 'imm' to a non 16bit-aligned address. In my book, that's bad. Same for pl330_dmamov(), which tries to write a 32bit value without checking the proper alignment. In such case, please use the put_unaligned macro to handle the possible unaligned access. M. -- Who you jivin' with that Cosmik Debris? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Joonyoung Shim on 25 Mar 2010 07:50 On 3/25/2010 7:32 PM, Marc Zyngier wrote: > On Thu, 25 Mar 2010 19:05:47 +0900, Joonyoung Shim > <jy0922.shim(a)samsung.com> wrote: >> On 3/25/2010 6:32 PM, Marc Zyngier wrote: >>> On Thu, 25 Mar 2010 18:01:00 +0900, Joonyoung Shim >>> <jy0922.shim(a)samsung.com> wrote: >>>> On 3/25/2010 2:44 PM, Marc Zyngier wrote: >>>>> On Thu, 25 Mar 2010 12:17:15 +0900 >>>>> Joonyoung Shim <jy0922.shim(a)samsung.com> wrote: >>>>> >>>>>> + writew(imm, desc_pool_virt); >>>> Right. The write[bwl] is api for address ioremapped of io device. I > will >>>> change these. >>>> >>>>> Does anything ensure that this won't generate an unaligned access? >>>>> >>>> PL330 DMA controller fetches variable length instructions that consist >>> of >>>> one to six bytes, so i think unaligned access is no problem. >>> I'm not too concerned about the device side of things. I'm more worried >>> about the CPU access when writing the 'imm' value to memory. >>> >>> Consider desc_pool_virt 16bit aligned when entering the function. > Writing >>> the opcode makes it unaligned and then writing the 'imm' value will >>> result >>> as an unaligned access. >>> >> Why desc_pool_virt should be aligned more than 16bit? > > There is reason for desc_pool_virt to be 16bit aligned. It's just that you > have 50% chance that it will. > In such case, you will write 'imm' to a non 16bit-aligned address. In my > book, that's bad. > > Same for pl330_dmamov(), which tries to write a 32bit value without > checking the proper alignment. > In such case, please use the put_unaligned macro to handle the possible > unaligned access. > OK. i will use put_unaligned. Thanks. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: jassi brar on 25 Mar 2010 08:20
On Thu, Mar 25, 2010 at 5:30 PM, Linus Walleij <linus.ml.walleij(a)gmail.com> wrote: > 2010/3/25 jassi brar <jassisinghbrar(a)gmail.com>: > >> I too have been writing a driver for PL330 after taking into account the >> suggestions of Russell, Ben and other participants of the thread >> http://lists.infradead.org/pipermail/linux-arm-kernel/2010-February/009856.html >> >> If you don't think this driver conflicts with the theme of the thread, >> may I ask you to please put this driver on hold until you checkout my implementation >> of solution to the issue... which should be soon. > > Please post the code as it looks today even if it's not compiling > instead of asking others > to hold their patches back. It will be obvious from what you have if > there is some special > use you're covering. My approach is to write a separate PL330 core driver as the backend which can be reused by any DMA API implementer driver. That will avoid having two copies of the PL330 driver, among other benefits. And if this patch is accepted, there _will_ exist two copies of the PL330 driver -- one in drivers/dma/pl330_dmac.c and another in arch/arm/plat-samsung/. Only the former will be lying unused until some other SoC vendor decided to use PL330, because S3C has come too long a way to change its drivers to driver/dma/ API and modify DMA drivers for every SoC. I plan something like, arch/arm/common/pl330-core.c implementing the specs in http://infocenter.arm.com/help/topic/com.arm.doc.ddi0424a/DDI0424A_dmac_pl330_r0p0_trm.pdf and drivers/dma/pl330.c implement DMA API for SoCs that chose to use it... and arch/arm/plat-samsung/dma-pl330.c implementing regular S3C DMA API. I don't claim to have a silver bullet, nobody has atm, but my approach is at least more aligned with what maintainers want. I have the pl330-core part almost ready, but i need time to implement some _testable_ implementation of the scheme. If maintainers want to see structure of my code, I can share it too, but I think I pretty much made it clear. > Perhaps Joonyoung can simply port over the stuff > you need to this driver if you show your code. Having worked on Samsung SoCs(with PL330 DMAC) based products, I would be _very_ surprised if any user found this implementation useful. Let alone testing, this implementation can't even explain usability for fast peripherals with shallow FIFOs. I didn't give feedback for this patch because I am not sure if this is the right way to go at all. regards. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |