On Fri, Aug 09, 2019 at 07:34:49PM +0200, marek.vasut@xxxxxxxxx wrote:
> From: Oza Pawandeep <oza.oza@xxxxxxxxxxxx>
>
> current device framework and OF framework integration assumes
> dma-ranges in a way where memory-mapped devices define their
> dma-ranges. (child-bus-address, parent-bus-address, length).
"Memory-mapped" means device memory and registers are mapped into the
CPU address space so reads/writes from the CPU can access the device.
IIUC, the issue you're solving is for DMA, which is the other
direction -- DMA reads/writes from the *device* can access system
memory.
> of_dma_configure is specifically written to take care of memory
> mapped devices. but no implementation exists for pci to take
> care of pcie based memory ranges.
s/pci/PCI/
There's nothing PCIe-specific here; the same issue could occur with
either PCI or PCIe, so just say "PCI".
> for e.g. iproc based SOCs and other SOCs(such as rcar) have PCI
> world dma-ranges.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
>
> this patch fixes the following problems of_dma_get_range.
> 1) return of wrong size as 0.
> 2) not handling absence of dma-ranges which is valid for PCI master.
> 3) not handling multipe inbound windows.
s/multipe/multiple/
> 4) in order to get largest possible dma_mask. this patch also
> retuns the largest possible size based on dma-ranges,
>
> for e.g.
> dma-ranges = <0x43000000 0x00 0x00 0x00 0x00 0x80 0x00>;
> we should get dev->coherent_dma_mask=0x7fffffffff.
>
> based on which IOVA allocation space will honour PCI host
> bridge limitations.
>
> the implementation hooks bus specific callbacks for getting
> dma-ranges.
>
> Signed-off-by: Oza Pawandeep <oza.oza@xxxxxxxxxxxx>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@xxxxxxxxx>
> Cc: Arnd Bergmann <arnd@xxxxxxxx>
> Cc: Geert Uytterhoeven <geert+renesas@xxxxxxxxx>
> Cc: Lorenzo Pieralisi <lorenzo.pieralisi@xxxxxxx>
> Cc: Robin Murphy <robin.murphy@xxxxxxx>
> Cc: Simon Horman <horms+renesas@xxxxxxxxxxxx>
> Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
> Cc: linux-renesas-soc@xxxxxxxxxxxxxxx
> To: linux-pci@xxxxxxxxxxxxxxx
> ---
> drivers/of/address.c | 220 +++++++++++++++++++++++++++++--------------
> 1 file changed, 150 insertions(+), 70 deletions(-)
>
> diff --git a/drivers/of/address.c b/drivers/of/address.c
> index 55a4eb7786ca..ae2819e148b8 100644
> --- a/drivers/of/address.c
> +++ b/drivers/of/address.c
> @@ -8,6 +8,7 @@
> #include <linux/logic_pio.h>
> #include <linux/module.h>
> #include <linux/of_address.h>
> +#include <linux/of_pci.h>
> #include <linux/pci.h>
> #include <linux/pci_regs.h>
> #include <linux/sizes.h>
> @@ -48,6 +49,8 @@ struct of_bus {
> int na, int ns, int pna);
> int (*translate)(__be32 *addr, u64 offset, int na);
> unsigned int (*get_flags)(const __be32 *addr);
> + int (*get_dma_ranges)(struct device_node *np,
> + u64 *dma_addr, u64 *paddr, u64 *size);
> };
>
> /*
> @@ -174,6 +177,143 @@ static int of_bus_pci_translate(__be32 *addr, u64 offset, int na)
> return of_bus_default_translate(addr + 1, offset, na - 1);
> }
>
> +static int of_bus_pci_get_dma_ranges(struct device_node *np, u64 *dma_addr,
> + u64 *paddr, u64 *size)
> +{
> + struct device_node *node = of_node_get(np);
> + int ret = 0;
> + struct resource_entry *window;
> + LIST_HEAD(res);
> +
> + if (!node)
> + return -EINVAL;
> +
> + *size = 0;
> + /*
> + * PCI dma-ranges is not mandatory property.
> + * many devices do no need to have it, since
> + * host bridge does not require inbound memory
> + * configuration or rather have design limitations.
> + * so we look for dma-ranges, if missing we
> + * just return the caller full size, and also
> + * no dma-ranges suggests that, host bridge allows
> + * whatever comes in, so we set dma_addr to 0.
Wrap to fill the usual line length in this file.
> + */
> + ret = of_pci_get_dma_ranges(np, &res);
> + if (!ret) {
> + resource_list_for_each_entry(window, &res) {
> + struct resource *res_dma = window->res;
> +
> + if (*size < resource_size(res_dma)) {
> + *dma_addr = res_dma->start - window->offset;
> + *paddr = res_dma->start;
> + *size = resource_size(res_dma);
> + }
> + }
> + }
> + pci_free_resource_list(&res);
> +
> + /*
> + * return the largest possible size,
> + * since PCI master allows everything.
Wrap.
> + */
> + if (*size == 0) {
> + pr_debug("empty/zero size dma-ranges found for node(%s)\n",
> + np->full_name);
> + *size = DMA_BIT_MASK(sizeof(dma_addr_t) * 8) - 1;
> + *dma_addr = *paddr = 0;
> + ret = 0;
> + }
> +
> + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> + *dma_addr, *paddr, *size);
> +
> + of_node_put(node);
> +
> + return ret;
> +}
> +
> +static int get_dma_ranges(struct device_node *np, u64 *dma_addr,
> + u64 *paddr, u64 *size)
> +{
> + struct device_node *node = of_node_get(np);
> + const __be32 *ranges = NULL;
> + int len, naddr, nsize, pna;
> + int ret = 0;
> + u64 dmaaddr;
> +
> + if (!node)
> + return -EINVAL;
> +
> + while (1) {
> + naddr = of_n_addr_cells(node);
> + nsize = of_n_size_cells(node);
> + node = of_get_next_parent(node);
> + if (!node)
> + break;
> +
> + 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;
> + }
> +
> + if (!ranges) {
> + pr_debug("no dma-ranges found for node(%s)\n", np->full_name);
> + ret = -ENODEV;
> + goto out;
> + }
> +
> + len /= sizeof(u32);
> +
> + pna = of_n_addr_cells(node);
> +
> + /* dma-ranges format:
> + * DMA addr : naddr cells
> + * CPU addr : pna cells
> + * size : nsize cells
> + */
> + dmaaddr = of_read_number(ranges, naddr);
> + *paddr = of_translate_dma_address(np, ranges);
> + if (*paddr == OF_BAD_ADDR) {
> + pr_err("translation of DMA address(%pad) to CPU address failed node(%s)\n",
> + dma_addr, np->full_name);
The old code used %pOF; why the change to %s?
> + ret = -EINVAL;
> + goto out;
> + }
> + *dma_addr = dmaaddr;
> +
> + *size = of_read_number(ranges + naddr + pna, nsize);
> +
> + pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> + *dma_addr, *paddr, *size);
I'm a little confused about the usage of %pad above and %llx here.
Are these printing different things, or could they both be done the
same way? (There are other occurrences of %llx earlier, too.)
> +
> +out:
> + of_node_put(node);
> +
> + return ret;
> +}
> +
> +static int of_bus_isa_get_dma_ranges(struct device_node *np, u64 *dma_addr,
> + u64 *paddr, u64 *size)
> +{
> + return get_dma_ranges(np, dma_addr, paddr, size);
> +}
> +
> +static int of_bus_default_get_dma_ranges(struct device_node *np, u64 *dma_addr,
> + u64 *paddr, u64 *size)
> +{
> + return get_dma_ranges(np, dma_addr, paddr, size);
> +}
> +
> const __be32 *of_get_pci_address(struct device_node *dev, int bar_no, u64 *size,
> unsigned int *flags)
> {
> @@ -438,6 +578,7 @@ static struct of_bus of_busses[] = {
> .map = of_bus_pci_map,
> .translate = of_bus_pci_translate,
> .get_flags = of_bus_pci_get_flags,
> + .get_dma_ranges = of_bus_pci_get_dma_ranges,
> },
> #endif /* CONFIG_PCI */
> /* ISA */
> @@ -449,6 +590,7 @@ static struct of_bus of_busses[] = {
> .map = of_bus_isa_map,
> .translate = of_bus_isa_translate,
> .get_flags = of_bus_isa_get_flags,
> + .get_dma_ranges = of_bus_isa_get_dma_ranges,
> },
> /* Default */
> {
> @@ -459,6 +601,7 @@ static struct of_bus of_busses[] = {
> .map = of_bus_default_map,
> .translate = of_bus_default_translate,
> .get_flags = of_bus_default_get_flags,
> + .get_dma_ranges = of_bus_default_get_dma_ranges,
> },
> };
>
> @@ -917,80 +1060,17 @@ EXPORT_SYMBOL(of_io_request_and_map);
> * size : nsize cells
> *
> * It returns -ENODEV if "dma-ranges" property was not found
> - * for this device in DT.
> + * for this device in DT, except if PCI device then, dma-ranges
> + * can be optional property, and in that case returns size with
> + * entire host memory.
> */
> int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size)
> {
> - struct device_node *node = of_node_get(np);
> - const __be32 *ranges = NULL;
> - int len, naddr, nsize, pna;
> - int ret = 0;
> - 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;
> -
> - 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;
> - }
> -
> - if (!ranges) {
> - pr_debug("no dma-ranges found for node(%pOF)\n", np);
> - ret = -ENODEV;
> - goto out;
> - }
> -
> - len /= sizeof(u32);
> -
> - pna = of_n_addr_cells(node);
> -
> - /* dma-ranges format:
> - * DMA addr : naddr cells
> - * CPU addr : pna cells
> - * size : nsize cells
> - */
> - dmaaddr = of_read_number(ranges, naddr);
> - *paddr = of_translate_dma_address(np, ranges);
> - if (*paddr == OF_BAD_ADDR) {
> - pr_err("translation of DMA address(%pad) to CPU address failed node(%pOF)\n",
> - dma_addr, np);
> - ret = -EINVAL;
> - goto out;
> - }
> - *dma_addr = dmaaddr;
> -
> - *size = of_read_number(ranges + naddr + pna, nsize);
> -
> - pr_debug("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n",
> - *dma_addr, *paddr, *size);
> -
> -out:
> - of_node_put(node);
> + struct of_bus *bus;
>
> - return ret;
> + /* get bus specific dma-ranges. */
> + bus = of_match_bus(np);
> + return bus->get_dma_ranges(np, dma_addr, paddr, size);
> }
> EXPORT_SYMBOL_GPL(of_dma_get_range);
>
> --
> 2.20.1
>
