| Message ID | 1463563981-69722-1-git-send-email-agraf@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday 18 May 2016 11:33:01 Alexander Graf wrote: > diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h > index ba437f0..67bf8e1 100644 > --- a/arch/arm64/include/asm/dma-mapping.h > +++ b/arch/arm64/include/asm/dma-mapping.h > @@ -66,12 +66,16 @@ static inline bool is_device_dma_coherent(struct device *dev) > > static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) > { > - return (dma_addr_t)paddr; > + dma_addr_t dev_addr = (dma_addr_t)paddr; > + > + return dev_addr - (dma_addr_t)__pfn_to_phys(dev->dma_pfn_offset); > } > > static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr) > { > - return (phys_addr_t)dev_addr; > + phys_addr_t paddr = (phys_addr_t)dev_addr; > + > + return paddr + __pfn_to_phys(dev->dma_pfn_offset); > } > The patch looks ok overall, but I think it's better to use an open-coded shift instead of __pfn_to_phys/__pfn_to_phys here: those helpers convert between actual page frame numbers and addresses, which may not always be a 1:1 relationship, e.g. if someone wants to work around the crazy "Principles of ARM Memory Maps White Paper" layout. Arnd
On Wed, May 18, 2016 at 12:52:28PM +0200, Arnd Bergmann wrote: > On Wednesday 18 May 2016 11:33:01 Alexander Graf wrote: > > diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h > > index ba437f0..67bf8e1 100644 > > --- a/arch/arm64/include/asm/dma-mapping.h > > +++ b/arch/arm64/include/asm/dma-mapping.h > > @@ -66,12 +66,16 @@ static inline bool is_device_dma_coherent(struct device *dev) > > > > static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) > > { > > - return (dma_addr_t)paddr; > > + dma_addr_t dev_addr = (dma_addr_t)paddr; > > + > > + return dev_addr - (dma_addr_t)__pfn_to_phys(dev->dma_pfn_offset); > > } > > > > static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr) > > { > > - return (phys_addr_t)dev_addr; > > + phys_addr_t paddr = (phys_addr_t)dev_addr; > > + > > + return paddr + __pfn_to_phys(dev->dma_pfn_offset); > > } > > > > The patch looks ok overall, but I think it's better to use an open-coded > shift instead of __pfn_to_phys/__pfn_to_phys here: those helpers convert > between actual page frame numbers and addresses, which may not always > be a 1:1 relationship, e.g. if someone wants to work around the crazy > "Principles of ARM Memory Maps White Paper" layout. Nitpick: Even better to use the PFN_PHYS() macro as it matches the PFN_DOWN() in of_dma_configure(). I can fix it up when applying.
On Wednesday 18 May 2016 14:45:55 Catalin Marinas wrote: > On Wed, May 18, 2016 at 12:52:28PM +0200, Arnd Bergmann wrote: > > On Wednesday 18 May 2016 11:33:01 Alexander Graf wrote: > > > diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h > > > index ba437f0..67bf8e1 100644 > > > --- a/arch/arm64/include/asm/dma-mapping.h > > > +++ b/arch/arm64/include/asm/dma-mapping.h > > > @@ -66,12 +66,16 @@ static inline bool is_device_dma_coherent(struct device *dev) > > > > > > static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) > > > { > > > - return (dma_addr_t)paddr; > > > + dma_addr_t dev_addr = (dma_addr_t)paddr; > > > + > > > + return dev_addr - (dma_addr_t)__pfn_to_phys(dev->dma_pfn_offset); > > > } > > > > > > static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr) > > > { > > > - return (phys_addr_t)dev_addr; > > > + phys_addr_t paddr = (phys_addr_t)dev_addr; > > > + > > > + return paddr + __pfn_to_phys(dev->dma_pfn_offset); > > > } > > > > > > > The patch looks ok overall, but I think it's better to use an open-coded > > shift instead of __pfn_to_phys/__pfn_to_phys here: those helpers convert > > between actual page frame numbers and addresses, which may not always > > be a 1:1 relationship, e.g. if someone wants to work around the crazy > > "Principles of ARM Memory Maps White Paper" layout. > > Nitpick: Even better to use the PFN_PHYS() macro as it matches the > PFN_DOWN() in of_dma_configure(). I can fix it up when applying. > > I'd probably avoid that for the same reason as avoiding __pfn_to_phys. In practice all three methods do the same thing for now, and if someone wanted to change one, they'd also have to audit all the existing users. Arnd
diff --git a/arch/arm64/include/asm/dma-mapping.h b/arch/arm64/include/asm/dma-mapping.h index ba437f0..67bf8e1 100644 --- a/arch/arm64/include/asm/dma-mapping.h +++ b/arch/arm64/include/asm/dma-mapping.h @@ -66,12 +66,16 @@ static inline bool is_device_dma_coherent(struct device *dev) static inline dma_addr_t phys_to_dma(struct device *dev, phys_addr_t paddr) { - return (dma_addr_t)paddr; + dma_addr_t dev_addr = (dma_addr_t)paddr; + + return dev_addr - (dma_addr_t)__pfn_to_phys(dev->dma_pfn_offset); } static inline phys_addr_t dma_to_phys(struct device *dev, dma_addr_t dev_addr) { - return (phys_addr_t)dev_addr; + phys_addr_t paddr = (phys_addr_t)dev_addr; + + return paddr + __pfn_to_phys(dev->dma_pfn_offset); } static inline bool dma_capable(struct device *dev, dma_addr_t addr, size_t size) @@ -86,5 +90,14 @@ static inline void dma_mark_clean(void *addr, size_t size) { } +/* Override for dma_max_pfn() */ +static inline unsigned long dma_max_pfn(struct device *dev) +{ + dma_addr_t dma_max = (dma_addr_t)*dev->dma_mask; + + return (ulong)__phys_to_pfn(dma_to_phys(dev, dma_max)); +} +#define dma_max_pfn(dev) dma_max_pfn(dev) + #endif /* __KERNEL__ */ #endif /* __ASM_DMA_MAPPING_H */
On arm64, all SoCs we supported so far either have an IOMMU or have bus addresses equal to CPU addresses. However, with the Raspberry Pi 3 coming up, this is no longer true. To allow DMA to work with an AArch64 kernel on those devices, let's allow devices to have DMA offsets again. Signed-off-by: Alexander Graf <agraf@suse.de> --- This patch may conflict with another patch titled "swiotlb: prefix dma_to_phys and phys_to_dma functions" which is in flight, but hasn't seen an update since March. Since this patch is very small and isolated to arm64, I'd prefer to keep them separate rather than combine them. So if the other patch gets accepted first, I'm happy to rebase this patch on top of a topic branch that would address the rename. --- arch/arm64/include/asm/dma-mapping.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)