On Friday 14 March 2014 12:38 PM, Laurent Pinchart wrote:
> Hi Santosh,
> 
> On Friday 14 March 2014 12:15:11 Santosh Shilimkar wrote:
>> + Russell, Arnd
>>
>> On Thursday 13 March 2014 10:47 PM, Anna, Suman wrote:
>>> On 03/07/2014 06:46 PM, Laurent Pinchart wrote:
>>>> The page table entries must be cleaned from the cache before being
>>>> accessed by the IOMMU. Instead of implementing cache management manually
>>>> (and ignoring L2 cache), use clean_dcache_area() to make sure the
>>>> entries are visible to the device.
>>>
>>> Thanks for fixing this, this has been long pending. There have been some
>>> previous attempts at this as well by Ramesh Gupta, with the last thread
>>> (a long time now) being
>>> http://marc.info/?t=134752035400001&r=1&w=2
>>>
>>> Santosh,
>>> Can you please take a look at this patch and provide your comments?
>>>
>>> regards
>>> Suman
>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com>
>>>> ---
>>>>
>>>>   drivers/iommu/omap-iommu.c | 41 ++++++++++-----------------------------
>>>>   1 file changed, 10 insertions(+), 31 deletions(-)
>>>>
>>>> diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
>>>> index a893eca..bb605c9 100644
>>>> --- a/drivers/iommu/omap-iommu.c
>>>> +++ b/drivers/iommu/omap-iommu.c
>>>> @@ -500,24 +500,9 @@ EXPORT_SYMBOL_GPL(omap_foreach_iommu_device);
>>>>   /*
>>>>    *       H/W pagetable operations
>>>>    */
>>>> -static void flush_iopgd_range(u32 *first, u32 *last)
>>>> +static void flush_pgtable(void *addr, size_t size)
>>>>   {
>>>> -  /* FIXME: L2 cache should be taken care of if it exists */
>>>> -  do {
>>>> -          asm("mcr        p15, 0, %0, c7, c10, 1 @ flush_pgd"
>>>> -              : : "r" (first));
>>>> -          first += L1_CACHE_BYTES / sizeof(*first);
>>>> -  } while (first <= last);
>>>> -}
>>>> -
>>>> -static void flush_iopte_range(u32 *first, u32 *last)
>>>> -{
>>>> -  /* FIXME: L2 cache should be taken care of if it exists */
>>>> -  do {
>>>> -          asm("mcr        p15, 0, %0, c7, c10, 1 @ flush_pte"
>>>> -              : : "r" (first));
>>>> -          first += L1_CACHE_BYTES / sizeof(*first);
>>>> -  } while (first <= last);
>>>> +  clean_dcache_area(addr, size);
>>
>> I remember NAKing this approach in past and my stand remains same.
>> The cache APIs which you are trying to use here are not suppose
>> to be used outside.
> 
> Please note that the omap-iommu driver already uses clean_dcache_area(). 
> That's where I got the idea :-)
> 
So that fall through the cracks then ;-)

>> I think the right way to fix this is to make use of streaming APIs.
>> If needed, IOMMU can have its own dma_ops for special case
>> handling if any.
> 
> I can replace clean_dcache_area() with dma_map_page() as done by the arm-smmu 
> driver. If that's fine I'll modify this patch accordingly in v2.
> 
That will be definitely a better option than the APIs used. Those streaming
APIs will take care of L2 cache as well if present.

Regards,
Santosh

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

Reply via email to