Hi Suman, On Tuesday 09 September 2014 16:33:11 Suman Anna wrote: > On 09/09/2014 10:45 AM, Laurent Pinchart wrote: > > The OMAP IOMMU driver supports both the OMAP1 and OMAP2+ IOMMU variants > > by splitting the driver into a core module and a thin arch-specific > > operations module. > > > > (In practice only the OMAP2+ module omap-iommu2 is implemented, but > > let's not denigrate the effort.) > > Thank you for the patch. I had something similar in my list of cleanup > TODO items on the OMAP IOMMU driver, but I was intending to cut down > more and consolidate the omap-iommu.c and omap-iommu2.c files into a > single one. > > This had been the case from the introduction of the driver going back to > v2.6.30, and OMAP1 was never added and I doubt it would be added anytime > in the foreseeable future. > > > The arch-specific operations module registers itself with the OMAP IOMMU > > core module at initialization time. This initializes a module global > > arch-specific operations pointer, used at runtime by the IOMMU > > instances. > > > > This scheme causes several issues. In addition to making it impossible > > to support different OMAP IOMMU types in a single system (which in all > > fairness is quite unlikely to happen), > > Yep, except for a few enhancements (like reporting Fault PC address on > OMAP4 DSPs, and dropping both endianness support), the core IOMMU > functionality hasn't changed much between OMAP2 and the latest OMAP4+ > SoCs. So, my plan was to completely get rid of the iommu_functions (it > also eliminates the need for exporting most of the OMAP IOMMU API). So > while I am ok with the current patch, I prefer consolidation than > keeping the scalability alive, it can always be added if a need for that > arises. What do you think?
I agree with your approach, but in the meantime we have a problem to solve. How about applying this patch now (it goes in the right direction anyway), and then removing the iommu functions when you will have time ? > it also causes initialization > > > ordering issues by requiring the arch-specific operations module to be > > loaded before any IOMMU user. This results in a probe breakage with the > > OMAP3 ISP driver when not compiled as a module. > > This can be fixed if we make the current omap-iommu2.c as a > subsys_initcall as well, right? > > regards > Suman > > > Fix the problem by inverting the dependency. Instead of having the > > omap-iommu2 module register itself to iommu-omap, make the iommu-omap > > retrieve the omap-iommu2 operations structure directly when probing the > > IOMMU device. This ensures that a probed IOMMU will always have valid > > arch-specific operations. > > > > As the arch-specific operations pointer is now initialized at probe > > time, this change requires turning it from a global variable into a > > per-device variable. > > > > Signed-off-by: Laurent Pinchart <laurent.pinch...@ideasonboard.com> > > --- > > > > drivers/iommu/omap-iommu-debug.c | 6 ++- > > drivers/iommu/omap-iommu.c | 94 +++++++++++++---------------------- > > drivers/iommu/omap-iommu.h | 10 ++++- > > drivers/iommu/omap-iommu2.c | 18 +------- > > 4 files changed, 45 insertions(+), 83 deletions(-) -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
