lkml.org 
[lkml]   [2019]   [Oct]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] of: Make of_dma_get_range() work on bus nodes
From
Date
On 2019-10-08 8:52 pm, Rob Herring wrote:
> From: Robin Murphy <robin.murphy@arm.com>
>
> Since the "dma-ranges" property is only valid for a node representing a
> bus, of_dma_get_range() currently assumes the node passed in is a leaf
> representing a device, and starts the walk from its parent. In cases
> like PCI host controllers on typical FDT systems, however, where the PCI
> endpoints are probed dynamically the initial leaf node represents the
> 'bus' itself, and this logic means we fail to consider any "dma-ranges"
> describing the host bridge itself. Rework the logic such that
> of_dma_get_range() also works correctly starting from a bus node
> containing "dma-ranges".
>
> While this does mean "dma-ranges" could incorrectly be in a device leaf
> node, there isn't really any way in this function to ensure that a leaf
> node is or isn't a bus node.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> [robh: Allow for the bus child node to still be passed in]
> Signed-off-by: Rob Herring <robh@kernel.org>
> ---
> Resending, hit send too quickly.
>
> v2:
> - Ensure once we find dma-ranges, every parent has it.
> - Only get the #{size,address}-cells after we find non-empty dma-ranges
> - Add a check on the 'dma-ranges' length
>
> This is all that remains of the dma-ranges series. I've applied the rest
> of the series prep and fixes. I dropped "of: Ratify of_dma_configure()
> interface" as the assertions that the node pointer being the parent only
> when struct device doesn't have a DT node pointer is not always
> true.

I'd still like to rework of_dma_configure() so that callers don't have
to pass a redundant node in the common case, but that can wait. For now,
this looks good enough to un-block the various 32-bit-PCI folks at
least, and we can consider further improvements on top. For the changes:

Reviewed-by: Robin Murphy <robin.murphy@arm.com>

Cheers,
Robin.

> I didn't include any tested-bys as this has changed a bit. A git branch
> is here[1].
>
> Rob
>
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git dma-masks-v2
>
> drivers/of/address.c | 44 ++++++++++++++++++--------------------------
> 1 file changed, 18 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 5ce69d026584..99c1b8058559 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -930,47 +930,39 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
> const __be32 *ranges = NULL;
> int len, naddr, nsize, pna;
> int ret = 0;
> + bool found_dma_ranges = false;
> u64 dmaaddr;
>
> - if (!node)
> - return -EINVAL;
> -
> - while (1) {
> - struct device_node *parent;
> -
> - naddr = of_n_addr_cells(node);
> - nsize = of_n_size_cells(node);
> -
> - parent = __of_get_dma_parent(node);
> - of_node_put(node);
> -
> - node = parent;
> - if (!node)
> - break;
> -
> + 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;
> + /* Once we find 'dma-ranges', then a missing one is an error */
> + if (found_dma_ranges && !ranges) {
> + ret = -ENODEV;
> + goto out;
> + }
> + found_dma_ranges = true;
> +
> + node = of_get_next_dma_parent(node);
> }
>
> - if (!ranges) {
> + if (!node || !ranges) {
> pr_debug("no dma-ranges found for node(%pOF)\n", np);
> ret = -ENODEV;
> goto out;
> }
>
> - len /= sizeof(u32);
> -
> + naddr = of_bus_n_addr_cells(node);
> + nsize = of_bus_n_size_cells(node);
> pna = of_n_addr_cells(node);
> + if ((len / sizeof(__be32)) % (pna + naddr + nsize)) {
> + ret = -EINVAL;
> + goto out;
> + }
>
> /* dma-ranges format:
> * DMA addr : naddr cells
> @@ -978,7 +970,7 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz
> * size : nsize cells
> */
> dmaaddr = of_read_number(ranges, naddr);
> - *paddr = of_translate_dma_address(np, ranges);
> + *paddr = of_translate_dma_address(node, ranges + naddr);
> if (*paddr == OF_BAD_ADDR) {
> pr_err("translation of DMA address(%llx) to CPU address failed node(%pOF)\n",
> dmaaddr, np);
>

\
 
 \ /
  Last update: 2019-10-09 12:57    [W:0.053 / U:1.440 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site