On Wed, Sep 03, 2008 at 01:37:21PM -0700, Tony Lindgren wrote: > * Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx> [080903 12:49]: > > Yes, that will be virtual. But what does it mean to call: > > > > omap_set_dma_dest_params() > > > > specifying a virtual address? Can the DMA controller cope with DMAing > > to virtual addresses? My hunch is that the DMA controller can't cope > > with that, so giving it a virtual address is a bug. > > > > Let me change the question: does omap_set_dma_dest_params()'s 4th > > argument take a virtual or a physical address? If the former, it's > > prototype is wrong, and its 4th argument needs to be typed as > > 'void __iomem *' rather than 'unsigned long'. If the latter, the code > > above is wrong. > > The dma src and dest functions take physical addresses so the prototype > should be void __iomem *. Grr. No. Let me repeat the rule: - virtual addresses are pointer like. - physical addresses are integer like. So, if it's a physical address, it should be stored in an integer type large enough to contain it, and that means something like u32, or unsigned long. If it's a virtual MMIO address, then it should be something like 'void __iomem *', or if you want to play roulette with compiler padding, 'struct foo __iomem *'. Okay, so lets accept that the 4th argument to omap_set_dma_dest_params() is a physical address. It should be typed as 'unsigned long' (it is) and it then means that: diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c index d084405..d9b1a42 100644 --- a/arch/arm/plat-omap/mcbsp.c +++ b/arch/arm/plat-omap/mcbsp.c @@ -652,6 +652,7 @@ int omap_mcbsp_xmit_buffer(unsigned int id, dma_addr_t buffer, omap_set_dma_dest_params(mcbsp[id].dma_tx_lch, src_port, OMAP_DMA_AMODE_CONSTANT, + /* FIXME: this is a virtual address */ mcbsp[id].io_base + OMAP_MCBSP_REG_DXR1, 0, 0); @@ -713,6 +714,7 @@ int omap_mcbsp_recv_buffer(unsigned int id, dma_addr_t buffer, omap_set_dma_src_params(mcbsp[id].dma_rx_lch, src_port, OMAP_DMA_AMODE_CONSTANT, + /* FIXME: this is a virtual address */ mcbsp[id].io_base + OMAP_MCBSP_REG_DRR1, 0, 0); are broken because they're passing a virtual address into a function requiring a physical address. Now, to fix this in the right way isn't going to be easy, because arch/arm/plat-omap/mcbsp.c doesn't know what the physical address of the mcbsp actually is - it's only passed the virtual address via platform data (eww, yuck yuck yuck)... If this was a properly reviewed platform driver, and on *any* *other* ARM platform, it would take the resources containing the physical addresses and ioremap them... and this would be a trivial bug fix. I'll cook a patch up. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
