On Mon, May 18, 2009 at 4:48 PM, Hiroshi DOYU<Hiroshi.DOYU@xxxxxxxxx> wrote: > From: ext Felipe Contreras <felipe.contreras@xxxxxxxxx> > Subject: Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration > Date: Mon, 18 May 2009 13:48:09 +0200 > >> On Mon, May 18, 2009 at 8:16 AM, Hiroshi DOYU <Hiroshi.DOYU@xxxxxxxxx> wrote: >> > From: ext Felipe Contreras <felipe.contreras@xxxxxxxxx> >> > Subject: Re: [RFC/PATCH 0/3] omap3-iommu: cleanups and remote registration >> > Date: Sat, 16 May 2009 20:32:10 +0200 >> > >> >> On Sat, May 16, 2009 at 7:36 PM, Russell King - ARM Linux >> >> <linux@xxxxxxxxxxxxxxxx> wrote: >> >> > On Sat, May 16, 2009 at 01:05:47PM +0300, Felipe Contreras wrote: >> >> >> This patch series cleanups up a bit the opap3-iommu device registration and >> >> >> then allows registration from other parts of the code. >> >> >> >> >> >> Currently the iva2 code (tidspbridge) is not using iommu, therefore it can't be >> >> >> used as-is with the current omap iommu. By allowing devies to be registered >> >> >> externaly (either isp, or iva2) this problem goes away. >> >> > >> >> > Hmm, so does this mean that the iommu patchset is going to grow by three >> >> > patches? Hope not. >> > >> > This series has been posted for a long time and I want to get this in >> > this time with minor fixes. >> > >> >> I hope Hiroshi integrates my patches. >> > >> > I think that the problem of yours is that it's not necesary to allow >> > device registration around kernel anywhere by "omap_iommu_add()" at >> > all, but enough only in "omap3-iommu.c". Since eventually >> > "tidspbridge" will use this iommu framework, no need for this >> > flexibility. Keeping same kind of device registration in one place is >> > good thing from SoC perspective. So I want to avoid adding unnecessary >> > flexibility to the code. I'll follow Russell's call, anyway. >> >> The first patch has nothing to do with omap_iommu_add, it's just >> cleanups, and you haven't provided any valid reason to reject them. > > Your code doesn't work since the other members of "struct resource" > haven't been initilialized like: > > + memset(res, 0, sizeof(res)); For the record, that's not true: + struct resource res[] = { + { .flags = IORESOURCE_MEM }, + { .flags = IORESOURCE_IRQ }, + }; Is perfectly fine because according to the C standard in section 6.7.8 #19: The initialization shall occur in initializer list order, each initializer provided for a particular subobject overriding any previously listed initializer for the same subobject;130) all subobjects that are not initialized explicitly shall be initialized implicitly the same as objects that have static storage duration. So the rest of the fields are initialized to 0. Cheers. -- Felipe Contreras -- 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