From patchwork Thu May 18 10:39:19 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 9733223 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 D6065600CC for ; Thu, 18 May 2017 10:39:46 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C14EA284CF for ; Thu, 18 May 2017 10:39:46 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B5D2728808; Thu, 18 May 2017 10:39:46 +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=-1.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 62D75284CF for ; Thu, 18 May 2017 10:39:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Message-ID:Date:Subject:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Dtqyj0zkiFlEf01TtB5v7yT3oIIKVmLcY7/UgiK+5nw=; b=V98u5y/dLOrU23 huk01WVXG8U6WbQXYX4fe01TDdobZGvHk6+2iAZzUVX72icdDrJA4Iu0EXSdwROIXZe5UzVir+V3o HXfiUsRr4zGE2fZzw25jMZD8bxqZz8099DaYOZPxpHqLw811G/49JrWbVN2iOUbGPjieOX5kwFIuP CbiSO1xbHrm21xKymyCgF+eFNr/oPXj2bQgy7qNWw1m0dUiOCnN6AENbVBbM4JdYNqbDUaQgBNNoK YjS5UzPWfoWTmKoww5nrXpXjOBkd6WoIW4RskWDWTGCaQBH8ijaiGTWLS4uWg1DXDTrEI4ZlXdszT KRmbCTKvTw+8V5BiNkMg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1dBIqP-0004Na-Tt; Thu, 18 May 2017 10:39:45 +0000 Received: from galahad.ideasonboard.com ([2001:4b98:dc2:45:216:3eff:febb:480d]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1dBIqI-00043E-6T for linux-arm-kernel@lists.infradead.org; Thu, 18 May 2017 10:39:41 +0000 Received: from avalon.localnet (dfj612ybrt5fhg77mgycy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:2e86:4862:ef6a:2804]) by galahad.ideasonboard.com (Postfix) with ESMTPSA id 1B4C6201C1; Thu, 18 May 2017 12:39:02 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1495103942; bh=0zAZfJKGbrOkqqDqJGCDqMb5yHPurcGivO+Iv29Bue0=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CHKX6BOlnt6mClJqCZdiE6OD0pv3TywlvGAKmoJu54vFyUShzjgEnQds98wTiinZA V02ZxFV4IQcL0iCF1EL19/uuXbVucGNjUKrtt7z21wJaln3ma0mxqPoBR4UseOqw6T ZWoiSZ9sN7u2Ez9IV68qHTyFlHNsjJ1SsxQwzFnc= From: Laurent Pinchart To: Sricharan R Subject: Re: [PATCH V2 2/3] iommu: of: Ignore all errors except EPROBE_DEFER Date: Thu, 18 May 2017 13:39:19 +0300 Message-ID: <6863998.WsTHer29ZI@avalon> User-Agent: KMail/4.14.10 (Linux/4.9.16-gentoo; KDE/4.14.32; x86_64; ; ) In-Reply-To: <1495102030-10548-2-git-send-email-sricharan@codeaurora.org> References: <1495102030-10548-1-git-send-email-sricharan@codeaurora.org> <1495102030-10548-2-git-send-email-sricharan@codeaurora.org> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170518_033939_394853_B8582CC3 X-CRM114-Status: GOOD ( 18.37 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: catalin.marinas@arm.com, will.deacon@arm.com, okaya@codeaurora.org, frowand.list@gmail.com, m.szyprowski@samsung.com, linux-arch@vger.kernel.org, lorenzo.pieralisi@arm.com, tn@semihalf.com, joro@8bytes.org, magnus.damm@gmail.com, linux-acpi@vger.kernel.org, geert@linux-m68k.org, linux-pci@vger.kernel.org, lenb@kernel.org, devicetree@vger.kernel.org, arnd@arndb.de, linux-arm-msm@vger.kernel.org, j.neuschaefer@gmx.net, robh+dt@kernel.org, bhelgaas@google.com, laurent.pinchart+renesas@ideasonboard.com, linux-arm-kernel@lists.infradead.org, rjw@rjwysocki.net, linux-kernel@vger.kernel.org, iommu@lists.linux-foundation.org, hanjun.guo@linaro.org, sudeep.holla@arm.com, robin.murphy@arm.com Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Sricharan, Thank you for the patch. On Thursday 18 May 2017 15:37:09 Sricharan R wrote: > While deferring the probe of IOMMU masters, > xlate and add_device callback can pass back error values > like -ENODEV, which means IOMMU cannot be connected > with that master for real reasons. So rather than > killing the master's probe for such errors, just > ignore the errors and let the master work without > an IOMMU. I don't think this is a good idea. Why should we allow IOMMU drivers to return an error if we're always going to ignore the error value ? That will lead to drivers implementing slightly different behaviours, which will be painful the day we'll need to start acting based on the error value. At the very least, if you want to give a specific meaning to -ENODEV, you should check for that value specifically and not ignore all errors other than -EPROBE_DEFER. You also need to document the meaning of the error value. This can be done in the documentation of the of_xlate operation in include/linux/iommu.h: And add documentation for the error codes there. If you want to ignore some errors returned from the add_device operation you should document it similarly, and in particular document which error check(s) need to be performed by of_xlate and which are the responsibility of add_device. > Fixes: 7b07cbefb68d ("iommu: of: Handle IOMMU lookup failure with deferred > probing or error") > Reported-by: Geert Uytterhoeven > Tested-by: Magnus Damn > Signed-off-by: Sricharan R > --- > [V2] Corrected spelling/case in commit log > > drivers/iommu/of_iommu.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c > index e6e9bec..f0d22c0 100644 > --- a/drivers/iommu/of_iommu.c > +++ b/drivers/iommu/of_iommu.c > @@ -237,6 +237,12 @@ const struct iommu_ops *of_iommu_configure(struct > device *dev, ops = ERR_PTR(err); > } > > + /* Ignore all other errors apart from EPROBE_DEFER */ > + if (IS_ERR(ops) && (PTR_ERR(ops) != -EPROBE_DEFER)) { > + dev_info(dev, "Adding to IOMMU failed: %ld\n", PTR_ERR(ops)); > + ops = NULL; > + } > + > return ops; > } diff --git a/include/linux/iommu.h b/include/linux/iommu.h index 2cb54adc4a33..6ba553e7384a 100644 --- a/include/linux/iommu.h +++ b/include/linux/iommu.h @@ -181,7 +181,6 @@ struct iommu_resv_region { * @domain_window_disable: Disable a particular window for a domain * @domain_set_windows: Set the number of windows for a domain * @domain_get_windows: Return the number of windows for a domain - * @of_xlate: add OF master IDs to iommu grouping * @pgsize_bitmap: bitmap of all possible supported page sizes */ struct iommu_ops { @@ -224,6 +223,11 @@ struct iommu_ops { /* Get the number of windows per domain */ u32 (*domain_get_windows)(struct iommu_domain *domain); + /** + * @of_xlate: + * + * Add OF master IDs to iommu grouping. + */ int (*of_xlate)(struct device *dev, struct of_phandle_args *args); unsigned long pgsize_bitmap;