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 9:46 PM, David Cohen <dacohen@xxxxxxxxx> wrote:
> [snip]
>
>>>>>>> - Â Â Â 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.
>
> Not sure indeed if this change fits to this same patch. Looks like a
> 4th patch sounds better.

Indeed not. :)
A new set is coming soon.

Br,

David

>
> Br,
>
> David Cohen
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux