From mboxrd@z Thu Jan 1 00:00:00 1970 From: Suman Anna Subject: Re: [PATCH 2/4] iommu/omap: Play nice in multi-platform builds Date: Wed, 4 Feb 2015 11:37:52 -0600 Message-ID: <54D258F0.8040905@ti.com> References: <1423036690-3862-1-git-send-email-thierry.reding@gmail.com> <1423036690-3862-3-git-send-email-thierry.reding@gmail.com> <1718219.kRa6INVfJa@avalon> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1718219.kRa6INVfJa@avalon> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Laurent Pinchart , Thierry Reding Cc: Nicolas Chauvet , Tony Lindgren , iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org List-Id: iommu@lists.linux-foundation.org Hi Thierry, On 02/04/2015 08:31 AM, Laurent Pinchart wrote: > Hi Thierry, > > Thank you for the patch. > > On Wednesday 04 February 2015 08:58:08 Thierry Reding wrote: >> From: Thierry Reding >> >> 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 >> Cc: Tony Lindgren >> Cc: Suman Anna >> Cc: Laurent Pinchart >> Signed-off-by: Thierry Reding >> --- >> 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; >> + Need to call the of_node_put on the cases it succeeds right, you should change this to the semantics you used in the arm-smmu driver. > > We should convert the omap-iommu driver to proper DT instantiation, but this > should do for now. Agreed, and I think this check is better than the iommu_present check used in some of the other IOMMU drivers, as that would work for a SoC family only if that is the first one getting registered. regards Suman > >> 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. >