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-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
