Re: [PATCH 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED/IOVMF_DA_ANON flags

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Mar 8, 2011 at 12:57 PM, David Cohen <dacohen@xxxxxxxxx> wrote:
> Hi Hiroshi, Fernando,
>
> On Tue, Mar 8, 2011 at 8:53 PM, Guzman Lugo, Fernando
> <fernando.lugo@xxxxxx> wrote:
>> On Tue, Mar 8, 2011 at 12:09 PM, Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx> wrote:
>>> From: "ext Guzman Lugo, Fernando" <fernando.lugo@xxxxxx>
>>> Subject: Re: [PATCH 3/3] omap: iovmm: don't check 'da' to set IOVMF_DA_FIXED/IOVMF_DA_ANON flags
>>> Date: Tue, 8 Mar 2011 11:59:43 -0600
>>>
>>>> On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@xxxxxxxxx> wrote:
>>>>> Currently IOVMM driver sets IOVMF_DA_FIXED/IOVMF_DA_ANON flags according
>>>>> to input 'da' address when mapping memory:
>>>>> da == 0: IOVMF_DA_ANON
>>>>> da != 0: IOVMF_DA_FIXED
>>>>>
>>>>> It prevents IOMMU to map first page with fixed 'da'. To avoid such
>>>>> issue, IOVMM will not automatically set IOVMF_DA_FIXED. It should now
>>>>> come from the user. IOVMF_DA_ANON will be automatically set if
>>>>> IOVMF_DA_FIXED isn't set.
>>>>>
>>>>> Signed-off-by: David Cohen <dacohen@xxxxxxxxx>
>>>>> ---
>>>>>  arch/arm/plat-omap/iovmm.c |   12 ++++++++----
>>>>>  1 files changed, 8 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
>>>>> index 11c9b76..dde9cb0 100644
>>>>> --- a/arch/arm/plat-omap/iovmm.c
>>>>> +++ b/arch/arm/plat-omap/iovmm.c
>>>>> @@ -654,7 +654,8 @@ u32 iommu_vmap(struct iommu *obj, u32 da, const struct sg_table *sgt,
>>>>>        flags &= IOVMF_HW_MASK;
>>>>>        flags |= IOVMF_DISCONT;
>>>>>        flags |= IOVMF_MMIO;
>>>>> -       flags |= (da ? IOVMF_DA_FIXED : IOVMF_DA_ANON);
>>>>> +       if (~flags & IOVMF_DA_FIXED)
>>>>> +               flags |= IOVMF_DA_ANON;
>>>>
>>>> could we use only one? both are mutual exclusive, what happen if flag
>>>> is IOVMF_DA_FIXED | IOVMF_DA_ANON? so, I suggest to get rid of
>>>> IOVMF_DA_ANON.
>>>
>>> Then, what about introducing some MACRO? Better names?
>>>
>>> #define set_iovmf_da_anon(flags)
>>> #define set_iovmf_da_fix(flags)
>>> #define set_iovmf_mmio(flags)
>>
>> will they be used by the users?
>>
>> I think people are more used to use
>>
>> iommu_vmap(obj, da, sgt, IOVMF_MMIO | IOVMF_DA_ANON);
>
> I'd be happier with this approach, instead of the macros. :)
> It's intuitive and very common on kernel.
>
>>
>> than
>>
>> set_iovmf_da_anon(flags)
>> set_iovmf_mmio(flags)
>> iommu_vmap(obj, da, sgt, flags);
>>
>> I don't have problem with the change, but I think how it is now is ok,
>> just that we don't we two bits to handle anon/fixed da, it can be
>> managed it only 1 bit (one flag), or is there a issue?
>
> We can exclude IOVMF_DA_ANON and stick with IOVMF_DA_FIXED only.
> I can resend my patch if we agree it's OK.

sounds perfect to me.

Regards,
Fernando.

>
> Regards,
>
> David
>
>>
>> Regards,
>> Fernando.
>>> ......
>>>
>>
>
--
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


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux