From patchwork Mon Apr 9 08:50:13 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 10330811 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 9660E6053B for ; Mon, 9 Apr 2018 08:50:18 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7A04E28AC1 for ; Mon, 9 Apr 2018 08:50:18 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6E9E728AC6; Mon, 9 Apr 2018 08:50:18 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id BADEB28AA7 for ; Mon, 9 Apr 2018 08:50:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751348AbeDIIuQ (ORCPT ); Mon, 9 Apr 2018 04:50:16 -0400 Received: from mail-qt0-f194.google.com ([209.85.216.194]:35633 "EHLO mail-qt0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750877AbeDIIuP (ORCPT ); Mon, 9 Apr 2018 04:50:15 -0400 Received: by mail-qt0-f194.google.com with SMTP id s2so8297478qti.2 for ; Mon, 09 Apr 2018 01:50:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:sender:in-reply-to:references:from:date:message-id :subject:to:cc; bh=r5IEyMtM5jmpZBduGI9GopPu0NSQ4S9poY0TrSugbQA=; b=eJu1VJT0+nVbzRGEzlDo81ZSdzMbc/kU75ao9+qrsIRkzxCd3WSzNtmjGQP5f9sjbK odDFss7dLDZA8GqZWBcoQHwLnmUPwDfKJtLwLTmV7qqwSERsc89ymOsdVBnoubR2ASQB AY+4YbKIe7Am295nqQxhAeAbx4GsuUXTajZmTESDfZxtXhF7JTR0sAkisbMsxm/FQNqS OFkdqXpmuAr91RhPuKm8wbjiVMUeBEaUuhMc36t+k0mlfJymddm6rGN0LuP60zInPa9y I9mKlxaHp8n4x3o/pV3BeQ/mN1AQSFdRFLfH3v7bfp/cHVAUOQxiU7WYrZRjJOKUn0EZ kgFQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:sender:in-reply-to:references:from :date:message-id:subject:to:cc; bh=r5IEyMtM5jmpZBduGI9GopPu0NSQ4S9poY0TrSugbQA=; b=QfCU2kQgeQW0eaQtVKw1kHdKulALosPzuW8SjaF+1og1BgVgMO1g35gY+ojBYYtq0J GbDl6Ne4Krca5issc4d2acluHU55ZPua6jsCxqDpfy/Us0Mrsa1huSxNCh8n89NoMlrL joSlWtugcCp2K4SkZHAmyJ58Czwat/C1a9P9yTLR2fiev8xzp9khiF87d97KJQcEVzlw ewHgT/42iRbSdbRY4RRv/GLehx6+mJtc95rvU8dS1hNtaEb1tW7jEUqjJZYh17DbE6PM k/mFKwGVrxPNOPnJZsD8vLlu8PIjuDmHmsqzQxBuLe++GeJj3kR44OPrh3C6mCFRWQto ot3g== X-Gm-Message-State: ALQs6tCimdGUVZDWC32uAYL9brF3k61fpqqDG1FEXgvK4/xIaTiCUYF0 hsETMkHvRFKMm+edE4NmAYDty4hd16suAoJFECs= X-Google-Smtp-Source: AIpwx48ksW5csAdIHS0GsiOukER1gQsKR5P5xFTN6OsLmVm42RvwHnasjlD1vHaBeGu1hhLymDTjrBL1Klm5kCX+1n0= X-Received: by 10.200.18.71 with SMTP id g7mr50976656qtj.35.1523263814088; Mon, 09 Apr 2018 01:50:14 -0700 (PDT) MIME-Version: 1.0 Received: by 10.12.185.25 with HTTP; Mon, 9 Apr 2018 01:50:13 -0700 (PDT) In-Reply-To: <20180407101455.214bf849@vento.lan> References: <2233233.yQEdpcOfql@avalon> <20180405164444.441033be@vento.lan> <4086814.xXeFl5mgbc@avalon> <20180407101455.214bf849@vento.lan> From: Arnd Bergmann Date: Mon, 9 Apr 2018 10:50:13 +0200 X-Google-Sender-Auth: TJxLT5obo9V0E-LtVpIagSxAelE Message-ID: Subject: Re: [PATCH 02/16] media: omap3isp: allow it to build with COMPILE_TEST To: Mauro Carvalho Chehab Cc: Laurent Pinchart , Linux Media Mailing List , Mauro Carvalho Chehab , Hans Verkuil , Stanimir Varbanov , Benjamin Gaignard , Philipp Zabel , Ramesh Shanmugasundaram Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Apr 7, 2018 at 3:14 PM, Mauro Carvalho Chehab wrote: > Em Sat, 07 Apr 2018 14:56:59 +0300 > Laurent Pinchart escreveu: >> On Thursday, 5 April 2018 22:44:44 EEST Mauro Carvalho Chehab wrote: >> > Em Thu, 05 Apr 2018 21:30:27 +0300 Laurent Pinchart escreveu: >> > > If you want to make drivers compile for all architectures, the APIs they >> > > use must be defined in linux/, not in asm/. They can be stubbed there >> > > when not implemented in a particular architecture, but not in the driver. >> > >> > In this specific case, the same approach taken here is already needed >> > by the Renesas VMSA-compatible IPMMU driver, with, btw, is inside >> > drivers/iommu: >> > >> > drivers/iommu/ipmmu-vmsa.c >> >> The reason there is different, the driver is shared by ARM32 and ARM64 >> platforms. Furthermore, there's an effort (or at least there was) to move away >> from those APIs in the driver, but I think it has stalled. > > Anyway, the approach sticks at the driver. As this was accepted even > inside drivers/iommu, I fail to see why not doing the same on media, > specially since it really helps us to find bugs at omap3isp driver. > > Even without pushing upstream (where Coverity would analyze it), > we got lots of problems by simply letting omap3isp to use the > usual tools we use for all other drivers. > >> > Also, this API is used only by 3 drivers [1]: >> > >> > drivers/iommu/ipmmu-vmsa.c >> > drivers/iommu/mtk_iommu_v1.c >> > drivers/media/platform/omap3isp/isp.c >> > >> > [1] as blamed by >> > git grep -l arm_iommu_create_mapping >> >> The exynos driver also uses it. >> >> > That hardly seems to be an arch-specific iommu solution, but, instead, some >> > hack used by only three drivers or some legacy iommu binding. >> >> It's more complex than that. There are multiple IOMMU-related APIs on ARM, so >> more recent than others, with different feature sets. While I agree that >> drivers should move away from arm_iommu_create_mapping(), doing so requires >> coordination between the IOMMU driver and the bus master driver (for instance >> the omap3isp driver). It's not a trivial matter, but I'd love if someone >> submitted patches :-) > > If someone steps up to do that, it would be really helpful, but we > should not trust that this will happen. OMAP3 is an old hardware, > and not many developers are working on improving its support. Considering its age, I still see a lot of changes on the arch/arm side of it, so I wouldn't give up the hope yet. >> > The omap3isp is, btw, the only driver outside drivers/iommu that needs it. >> > >> > So, it sounds that other driver uses some other approach, but hardly >> > it would be worth to change this driver to use something else. >> > >> > So, better to stick with the same solution the Renesas driver used. >> >> I'm not responsible for that solution and I didn't think it was a good one at >> the time it was introduced, but in any case it is not meant at all to support >> COMPILE_TEST. I still don't think the omap3isp driver should declare stubs for >> these functions for the purpose of supporting compile-testing on x86. > > Well, there is another alternative. We could do add this to its Makefile: > > ifndef CONFIG_ARCH_ARM > ccflags-y += -I./arch/arm/include/ > endif > > And add those stubs to arch/arm/include/asm/dma-iommu.h, > in order to be used when CONFIG_IOMMU_DMA isn't defined: > > #define arm_iommu_create_mapping(...) NULL > #define arm_iommu_attach_device(...) -ENODEV > #define arm_iommu_release_mapping(...) do {} while (0) > #define arm_iommu_detach_device(...) do {} while (0) > > If done right, such solution could also be used to remove > the #ifdef inside drivers/iommu/ipmmu-vmsa.c. > > Yet, I think that the approach I proposed before is better, > but maybe arm maintainers may have a different opinion. > > Arnd, > > What do you think? I think including a foreign architecture header is worse than your earlier patch, I'd rather see a local hack in the driver. I haven't tried it, but how about something simpler like what I have below. Arnd (in case it works and you want to pick it up with a proper changelog): Signed-off-by: Arnd Bergmann @@ -1979,6 +1982,9 @@ static int isp_attach_iommu(struct isp_device *isp) error: isp_detach_iommu(isp); return ret; +#else + return -ENODEV; +#endif } /* diff --git a/drivers/media/platform/omap3isp/isp.c b/drivers/media/platform/omap3isp/isp.c index 8eb000e3d8fd..625f2e407929 100644 --- a/drivers/media/platform/omap3isp/isp.c +++ b/drivers/media/platform/omap3isp/isp.c @@ -1945,12 +1945,15 @@ static int isp_initialize_modules(struct isp_device *isp) static void isp_detach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM arm_iommu_release_mapping(isp->mapping); isp->mapping = NULL; +#endif } static int isp_attach_iommu(struct isp_device *isp) { +#ifdef CONFIG_ARM struct dma_iommu_mapping *mapping; int ret;