[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. 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
