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
