Hi Arnd,

On: 26/03/2014 11:53, Arnd wrote:

Subject: Re: [PATCH v5 6/9] ARM: shmobile: Add PCIe device tree nodes 

for R8A7790

On Wednesday 26 March 2014 11:34:41 Phil.Edworthy at renesas.com wrote:

quoted

quoted

But the ranges you specified in the property don't actually fit in 

those

quoted

quoted

constraints: you have a range with size 0x8000000 and start 

0x40000000,

quoted

quoted

which you say can't be programmed into the hardware.

Actually, the driver checks the dma-ranges against these constraints, 

and 

quoted

if necessary will create multiple mappings to fulfil the requested 
dma-ranges.

Ok, I didn't notice. My initial suggestion was to not put that logic
into the driver but instead specify in the host bridge binding that each
entry in the dma-ranges property has to meet the hardware constraints.

As long as you don't have too much complexity to detect this case, I'm
fine with it either way.

Ok, good!

quoted

quoted

quoted

Still, my comment about the OF PCI range code treating both 32 and 

quoted

64-bit 

quoted

quoted

types the same way means that PCIe host driver has to assume it's 

a 

quoted

64-bit 

quoted

quoted

mapping.

I was thinking more of PCI devices than the host itself. If the host
driver can verify that all mappings are in the first 4GB and cover 

all 

quoted

of

quoted

RAM, we won't have to use an swiotlb for devices that don't support 

64-bit

quoted

DMA, which is a very significant performance difference.

Ok, I think I understand. However, all the other PCI host drivers just 

do 

quoted

1-to-1 mapping between PCI and CPU addresses, right? Whilst it might 

be 

quoted

nice be able to support mapping CPU addresses > 4GiB to PCI addresses 
under 4GiB, can that be something to consider later on?

Yes, fair enough. The current version is much simpler, so that's ok.
Just keep it in mind if you run into performance problems. Also, note
that we don't actually support swiotlb on arm32 yet, so your current
code is broken for an PCI DMA master that is not 64-bit capable.
We need swiotlb on arm32 anyway, and that will fix this problem, but
adding the hack I described would also fix it.

Ok, thanks for all your comments!

Phil