From patchwork Tue Mar 28 18:57:22 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Robin Murphy X-Patchwork-Id: 9650345 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 8DAC2602C8 for ; Tue, 28 Mar 2017 18:57:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6EEEB25D9E for ; Tue, 28 Mar 2017 18:57:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 60AF428338; Tue, 28 Mar 2017 18:57:52 +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=ham 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 8DF4525D9E for ; Tue, 28 Mar 2017 18:57:51 +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:MIME-Version:Cc:List-Subscribe: List-Help:List-Post:List-Archive:List-Unsubscribe:List-Id: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:In-Reply-To: References:List-Owner; bh=vFAVZlEolrCIjyRn6pqqaPbzFHm21r00EFZBo8ioYhw=; b=dZ/ UOp6MjjS2BQ26lKhrMPQLmmcgnOAo1qeo3yVs6nIYb1uCGSOIzAtJ9S7NC+aFzCn4ZWiaZ2rbTUQA FTJtn/zxiXSILudeSnlfTipZeSB+QFMbEmTBC6eVYOX9UfWi2KB56hbaIeixOZL8/VJBRw5chiPOu h37uXlQ7B3jZM0oW9IlTOLsqO6tkvgd1Ghpx4A7amQQIolMPiTg7Znu6xIfguqXFFCcdgst9xFmb+ XMbEZcdUo+q7r7PEqO0BJtAMcjKtg7GTtT/KdXpzGU1jMFWWKj7cpQm+T6cZj/PdmP4b72+9KpKn4 T6zOSpcnspySPeOS89cd0MdjBzGN/HA==; 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 1cswJS-0003RK-P2; Tue, 28 Mar 2017 18:57:50 +0000 Received: from foss.arm.com ([217.140.101.70]) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cswJP-0003Pa-QB for linux-arm-kernel@lists.infradead.org; Tue, 28 Mar 2017 18:57:49 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8AE55344; Tue, 28 Mar 2017 11:57:27 -0700 (PDT) Received: from e110467-lin.cambridge.arm.com (e110467-lin.cambridge.arm.com [10.1.210.40]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id A97143F220; Tue, 28 Mar 2017 11:57:26 -0700 (PDT) From: Robin Murphy To: iommu@lists.linux-foundation.org, devicetree@vger.kernel.org Subject: [RFC PATCH] of: Fix DMA configuration for non-DT masters Date: Tue, 28 Mar 2017 19:57:22 +0100 Message-Id: <67d1f46172599be243405f4341665fd0ef9ab969.1490726288.git.robin.murphy@arm.com> X-Mailer: git-send-email 2.11.0.dirty X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170328_115747_875501_DDCB7BDA X-CRM114-Status: GOOD ( 19.75 ) 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: robh+dt@kernel.org, oza.oza@broadcom.com, linux-arm-kernel@lists.infradead.org MIME-Version: 1.0 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 For PCI masters not represented in DT, we pass the OF node of their associated host bridge to of_dma_configure(), such that they can inherit the appropriate DMA configuration from whatever is described there. Unfortunately, whilst this has worked for the "dma-coherent" property, it turns out to miss the case where the host bridge node has a non-empty "dma-ranges", since nobody is expecting the 'device' to be a bus itself. It transpires, though, that the de-facto interface since the prototype change in 1f5c69aa51f9 ("of: Move of_dma_configure() to device.c to help re-use") is very clear-cut: either the master_np argument is redundant with dev->of_node, or dev->of_node is NULL and master_np is the relevant parent bus. Let's ratify that behaviour, then teach the whole of_dma_configure() pipeline to cope with both cases properly. Signed-off-by: Robin Murphy --- This is what I'd consider the better fix - rather than adding yet more special cases - which will also make it simple to handle multiple "dma-ranges" entries with minimal further disruption. The callers now left passing dev->of_node as 'parent' are harmless, but look a bit silly and clearly want cleaning up - I'd be partial to renaming the existing function and having a single-argument wrapper for the 'normal' case, e.g.: static inline int of_dma_configure(struct device_node *dev) { return of_dma_configure_parent(dev, NULL); } Thoughts? Robin. drivers/iommu/of_iommu.c | 7 ++++--- drivers/of/address.c | 37 +++++++++++++++++++++++++------------ drivers/of/device.c | 12 +++++++----- include/linux/of_address.h | 7 ++++--- include/linux/of_device.h | 4 ++-- include/linux/of_iommu.h | 4 ++-- 6 files changed, 44 insertions(+), 27 deletions(-) diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c index 2683e9fc0dcf..35aff07bb5eb 100644 --- a/drivers/iommu/of_iommu.c +++ b/drivers/iommu/of_iommu.c @@ -138,21 +138,22 @@ static const struct iommu_ops } const struct iommu_ops *of_iommu_configure(struct device *dev, - struct device_node *master_np) + struct device_node *parent) { struct of_phandle_args iommu_spec; - struct device_node *np; + struct device_node *np, *master_np; const struct iommu_ops *ops = NULL; int idx = 0; if (dev_is_pci(dev)) - return of_pci_iommu_configure(to_pci_dev(dev), master_np); + return of_pci_iommu_configure(to_pci_dev(dev), parent); /* * We don't currently walk up the tree looking for a parent IOMMU. * See the `Notes:' section of * Documentation/devicetree/bindings/iommu/iommu.txt */ + master_np = dev->of_node ? dev->of_node : parent; while (!of_parse_phandle_with_args(master_np, "iommus", "#iommu-cells", idx, &iommu_spec)) { diff --git a/drivers/of/address.c b/drivers/of/address.c index 02b2903fe9d2..833bc17f5e55 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -808,6 +808,7 @@ EXPORT_SYMBOL(of_io_request_and_map); /** * of_dma_get_range - Get DMA range info * @np: device node to get DMA range info + * @parent: node of device's parent bus, if @np is NULL * @dma_addr: pointer to store initial DMA address of DMA range * @paddr: pointer to store initial CPU address of DMA range * @size: pointer to store size of DMA range @@ -822,36 +823,48 @@ EXPORT_SYMBOL(of_io_request_and_map); * It returns -ENODEV if "dma-ranges" property was not found * for this device in DT. */ -int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size) +int of_dma_get_range(struct device_node *np, struct device_node *parent, + u64 *dma_addr, u64 *paddr, u64 *size) { - struct device_node *node = of_node_get(np); + struct device_node *node; const __be32 *ranges = NULL; int len, naddr, nsize, pna; int ret = 0; u64 dmaaddr; - if (!node) - return -EINVAL; - - while (1) { + if (np) { + node = of_node_get(np); naddr = of_n_addr_cells(node); nsize = of_n_size_cells(node); node = of_get_next_parent(node); - if (!node) - break; + } else if (parent) { + node = of_node_get(parent); + np = parent; + if (of_property_read_u32(node, "#address-cells", &naddr)) + naddr = of_n_addr_cells(node); + if (of_property_read_u32(node, "#size-cells", &nsize)) + nsize = of_n_size_cells(node); + } else { + return -EINVAL; + } + while (node) { ranges = of_get_property(node, "dma-ranges", &len); - /* Ignore empty ranges, they imply no translation required */ - if (ranges && len > 0) - break; - /* * At least empty ranges has to be defined for parent node if * DMA is supported */ if (!ranges) break; + + /* Ignore empty ranges, they imply no translation required */ + if (len > 0) + break; + + naddr = of_n_addr_cells(node); + nsize = of_n_size_cells(node); + node = of_get_next_parent(node); } if (!ranges) { diff --git a/drivers/of/device.c b/drivers/of/device.c index 9bb8518b28f2..57ec5324ed6c 100644 --- a/drivers/of/device.c +++ b/drivers/of/device.c @@ -73,7 +73,8 @@ int of_device_add(struct platform_device *ofdev) /** * of_dma_configure - Setup DMA configuration * @dev: Device to apply DMA configuration - * @np: Pointer to OF node having DMA configuration + * @parent: OF node of device's parent bus, if @dev is not + * represented in DT (i.e. @dev->of_node is NULL) * * Try to get devices's DMA configuration from DT and update it * accordingly. @@ -82,13 +83,14 @@ int of_device_add(struct platform_device *ofdev) * can use a platform bus notifier and handle BUS_NOTIFY_ADD_DEVICE events * to fix up DMA configuration. */ -void of_dma_configure(struct device *dev, struct device_node *np) +void of_dma_configure(struct device *dev, struct device_node *parent) { u64 dma_addr, paddr, size; int ret; bool coherent; unsigned long offset; const struct iommu_ops *iommu; + struct device_node *np = dev->of_node; /* * Set default coherent_dma_mask to 32 bit. Drivers are expected to @@ -104,7 +106,7 @@ void of_dma_configure(struct device *dev, struct device_node *np) if (!dev->dma_mask) dev->dma_mask = &dev->coherent_dma_mask; - ret = of_dma_get_range(np, &dma_addr, &paddr, &size); + ret = of_dma_get_range(np, parent, &dma_addr, &paddr, &size); if (ret < 0) { dma_addr = offset = 0; size = dev->coherent_dma_mask + 1; @@ -132,11 +134,11 @@ void of_dma_configure(struct device *dev, struct device_node *np) dev->coherent_dma_mask = DMA_BIT_MASK(ilog2(dma_addr + size)); *dev->dma_mask = dev->coherent_dma_mask; - coherent = of_dma_is_coherent(np); + coherent = of_dma_is_coherent(np ? np : parent); dev_dbg(dev, "device is%sdma coherent\n", coherent ? " " : " not "); - iommu = of_iommu_configure(dev, np); + iommu = of_iommu_configure(dev, parent); dev_dbg(dev, "device is%sbehind an iommu\n", iommu ? " " : " not "); diff --git a/include/linux/of_address.h b/include/linux/of_address.h index 37864734ca50..f1a507f3ae57 100644 --- a/include/linux/of_address.h +++ b/include/linux/of_address.h @@ -52,8 +52,8 @@ extern int of_pci_range_parser_init(struct of_pci_range_parser *parser, extern struct of_pci_range *of_pci_range_parser_one( struct of_pci_range_parser *parser, struct of_pci_range *range); -extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, - u64 *paddr, u64 *size); +extern int of_dma_get_range(struct device_node *np, struct device_node *parent, + u64 *dma_addr, u64 *paddr, u64 *size); extern bool of_dma_is_coherent(struct device_node *np); #else /* CONFIG_OF_ADDRESS */ static inline void __iomem *of_io_request_and_map(struct device_node *device, @@ -95,7 +95,8 @@ static inline struct of_pci_range *of_pci_range_parser_one( return NULL; } -static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr, +static inline int of_dma_get_range(struct device_node *np, + struct device_node *parent, u64 *dma_addr, u64 *paddr, u64 *size) { return -ENODEV; diff --git a/include/linux/of_device.h b/include/linux/of_device.h index c12dace043f3..bcd2b6fbeef3 100644 --- a/include/linux/of_device.h +++ b/include/linux/of_device.h @@ -55,7 +55,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu) return of_node_get(cpu_dev->of_node); } -void of_dma_configure(struct device *dev, struct device_node *np); +void of_dma_configure(struct device *dev, struct device_node *parent); #else /* CONFIG_OF */ static inline int of_driver_match_device(struct device *dev, @@ -103,7 +103,7 @@ static inline struct device_node *of_cpu_device_node_get(int cpu) { return NULL; } -static inline void of_dma_configure(struct device *dev, struct device_node *np) +static inline void of_dma_configure(struct device *dev, struct device_node *parent) {} #endif /* CONFIG_OF */ diff --git a/include/linux/of_iommu.h b/include/linux/of_iommu.h index 13394ac83c66..c02b62e8e6ed 100644 --- a/include/linux/of_iommu.h +++ b/include/linux/of_iommu.h @@ -12,7 +12,7 @@ extern int of_get_dma_window(struct device_node *dn, const char *prefix, size_t *size); extern const struct iommu_ops *of_iommu_configure(struct device *dev, - struct device_node *master_np); + struct device_node *parent); #else @@ -24,7 +24,7 @@ static inline int of_get_dma_window(struct device_node *dn, const char *prefix, } static inline const struct iommu_ops *of_iommu_configure(struct device *dev, - struct device_node *master_np) + struct device_node *parent) { return NULL; }