[RFC v1 PATCH 1/2] of/pci: add of_pci_dma_configure() update dma configuration

[Murali Karicheri]

On 12/22/2014 06:07 PM, Arnd Bergmann wrote:
> On Monday 22 December 2014 23:44:52 Arnd Bergmann wrote:
>> On Monday 22 December 2014 17:40:30 Murali Karicheri wrote:
>>> On 12/22/2014 05:24 PM, Arnd Bergmann wrote:
>>>> On Monday 22 December 2014 16:40:43 Murali Karicheri wrote:
>>>>>>> +++ b/arch/arm/mm/dma-mapping.c
>>>>>>> @@ -2052,9 +2052,10 @@ void arch_setup_dma_ops(struct device *dev, u64
>>>>>>> dma_base, u64 size,
>>>>>>>                             struct iommu_ops *iommu, bool coherent)
>>>>>>>      {
>>>>>>>             struct dma_map_ops *dma_ops;
>>>>>>> +       u64 temp_size = min((*(dev->dma_mask) + 1), size);
>>>>>>>
>>>>>>>             dev->archdata.dma_coherent = coherent;
>>>>>>> -       if (arm_setup_iommu_dma_ops(dev, dma_base, size, iommu))
>>>>>>> +       if (arm_setup_iommu_dma_ops(dev, dma_base, temp_size, iommu))
>>>>>>>
>>>>>>> If you agree, I will post v1 of the patch with these updates. Let me
>>>>>>> know. I did some basic tests on Keystone with these changes and it works
>>>>>>> fine.
>>>>>>
>>>>>> It's not exactly what I meant. My main point was that we need to limit
>>>>>> dev->dma_mask to (size-1) here, but you are not touching that.
>>>>>
>>>>> if you mean overriding the dev->dma_mask to min((*dev->dma_mask),
>>>>> size-1), then I am getting the error "Coherent DMA mask 0x7fffffff (pfn
>>>>> 0x780000-0x800000) covers a smaller range of system memory than the DMA
>>>>> zone pfn 0x0-0x880000) when the devices on Keystone tries to set the dma
>>>>> mask. Something wrong and I need to look into the code.
>>>>
>>>> Right, it sounds like the offset was applied incorrectly at some point.
>>>>
>>>> What are the DMA zone size and the phys-offset?
>>> 2G and 0x8_0000_0000. This limit the usable DMA size to 2G on Keystone,
>>> I believe you shouldn't be limiting the dma mask to size-1 in this case,
>>> right? The DT setup the dma-range to have a size of 2G (0x80000000).
>>
>> No, the problem is something else: the pfn range that we calculate for the
>> coherent DMA mask should have been 0x800000-0x880000, not 0x780000-0x800000,
>> so we would exactly match the ZONE_DMA.
>
> I gave it some more thought, and concluded that the size that gets passed
> down is not really the right value to be compared to a mask if the dma-capable
> area starts at a nonzero bus address.
>
> In your case, bus addresses 0x80000000-0xffffffff are valid for DMA, so
> the mask must be 0xffffffff to forbid any DMA to addresses larger than
> 0x100000000, not 0x7fffffff which would not be enough to cover any RAM.
>
> I guess the dma_mask should be 'min((*dev->dma_mask), dma_base + size - 1)'
> here.

Arnd,

I guess so. Besides we need to keep the default coherent dma mask to 
32bit 0xffffffffull as well to work on Keystone and also in sync with 
current defaults used in pci_device_add() so that we don't break this 
behavior.

Here is the summary of changes need to make on top of my existing patch.

1. of_dma_configure() - change size = dev->coherent_dma_mask to size = 
dev->coherent_dma_mask + 1. This is a new patch to fix existing code.

2. Do the above change to of_pci_dma_configure() as well.

3. in arch_setup_dma_ops() update the DMA mask to min((*dev->dma_mask), 
dma_base + size - 1) as


diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index c17f6a9..88b4769 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -2053,8 +2053,13 @@ void arch_setup_dma_ops(struct device *dev, u64 
dma_base, u64 size,
  {
         struct dma_map_ops *dma_ops;

+
+       *dev->dma_mask = min((*dev->dma_mask), (dma_base + size - 1));

I have tested this on keystone and it works fine with rootfs on PCI SATA 
harddisk. I will be doing more tests with this change. If you are in 
agreement with the above changes, I will re-spin the patch to accomodate 
them and send v1 of the same.

Regards,

Murali

>
> 	Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


-- 
Murali Karicheri
Linux Kernel, Texas Instruments