Re: [PATCH 2/4] iommu/omap: Play nice in multi-platform builds

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2833 bytes --]

On Wed, Feb 04, 2015 at 04:31:55PM +0200, Laurent Pinchart wrote:
> Hi Thierry,
> 
> Thank you for the patch.
> 
> On Wednesday 04 February 2015 08:58:08 Thierry Reding wrote:
> > From: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > 
> > The OMAP IOMMU driver unconditionally executes code and registers a
> > struct iommu_ops with the platform bus irrespective of whether it runs
> > on an OMAP SoC or not. This causes problems in multi-platform kernels
> > where drivers for other SoCs will no longer be able to register their
> > own struct iommu_ops or even try to use a struct iommu_ops for an IOMMU
> > that obviously isn't there.
> > 
> > The smallest fix I could think of is to check for the existence of any
> > OMAP IOMMU devices in the device tree and skip initialization otherwise.
> >
> > This fixes a problem on Tegra20 where the DRM driver will try to use the
> > obviously non-existent OMAP IOMMU.
> > 
> > Reported-by: Nicolas Chauvet <kwizart-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>
> > Cc: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> > Cc: Laurent Pinchart <laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> > Signed-off-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> > ---
> >  drivers/iommu/omap-iommu.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
> > index bbb7dcef02d3..e4d4f133a3b3 100644
> > --- a/drivers/iommu/omap-iommu.c
> > +++ b/drivers/iommu/omap-iommu.c
> > @@ -1377,6 +1377,9 @@ static int __init omap_iommu_init(void)
> >  	const unsigned long flags = SLAB_HWCACHE_ALIGN;
> >  	size_t align = 1 << 10; /* L2 pagetable alignement */
> > 
> > +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> > +		return 0;
> > +
> 
> We should convert the omap-iommu driver to proper DT instantiation, but this 
> should do for now.
> 
> >  	p = kmem_cache_create("iopte_cache", IOPTE_TABLE_SIZE, align, flags,
> >  			      iopte_cachep_ctor);
> >  	if (!p)
> > @@ -1394,6 +1397,9 @@ subsys_initcall(omap_iommu_init);
> > 
> >  static void __exit omap_iommu_exit(void)
> >  {
> > +	if (!of_find_matching_node(NULL, omap_iommu_of_match))
> > +		return;
> > +
> >  	kmem_cache_destroy(iopte_cachep);
> > 
> >  	platform_driver_unregister(&omap_iommu_driver);
> 
> The exit function will never be called as the omap-iommu driver is always 
> built-in. You could just remove the exit function.

I've omitted this hunk since this code will not run anyway. I agree that
we should remove the code altogether if it's dead anyway, but that kind
of cleanup isn't really suitable for patches this late in the release
cycle.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]