From b204731689059f0e27b676e86b089d59bd1a4805 Mon Sep 17 00:00:00 2001 From: Christoph Hellwig Date: Tue, 10 Sep 2019 07:58:33 +0200 Subject: of/fdt: don't ignore errors from of_setup_earlycon If of_setup_earlycon we should keep on iterating earlycon options instead of breaking out of the loop. Signed-off-by: Christoph Hellwig Signed-off-by: Rob Herring --- drivers/of/fdt.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c index 223d617ecfe1..d01d834b26b0 100644 --- a/drivers/of/fdt.c +++ b/drivers/of/fdt.c @@ -947,8 +947,8 @@ int __init early_init_dt_scan_chosen_stdout(void) if (fdt_node_check_compatible(fdt, offset, match->compatible)) continue; - of_setup_earlycon(match, offset, options); - return 0; + if (of_setup_earlycon(match, offset, options) == 0) + return 0; } return -ENODEV; } -- cgit v1.2.3 From 3a9ac959ba2825a3a6235bc909d369cc30386e9e Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 5 Sep 2019 11:44:24 +0100 Subject: of: Remove unused of_find_matching_node_by_address() of_find_matching_node_by_address() is unused, so remove it. Cc: Robin Murphy Reviewed-by: Geert Uytterhoeven Reviewed-by: Christoph Hellwig Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 19 ------------------- 1 file changed, 19 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 978427a9d5e6..0c3cf515c510 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -826,25 +826,6 @@ int of_address_to_resource(struct device_node *dev, int index, } EXPORT_SYMBOL_GPL(of_address_to_resource); -struct device_node *of_find_matching_node_by_address(struct device_node *from, - const struct of_device_id *matches, - u64 base_address) -{ - struct device_node *dn = of_find_matching_node(from, matches); - struct resource res; - - while (dn) { - if (!of_address_to_resource(dn, 0, &res) && - res.start == base_address) - return dn; - - dn = of_find_matching_node(dn, matches); - } - - return NULL; -} - - /** * of_iomap - Maps the memory mapped IO for a given device_node * @device: the device whose io range will be mapped -- cgit v1.2.3 From 6e6faf63744333373db8bc64aea52dab86cbf0bc Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 5 Sep 2019 11:53:27 +0100 Subject: of: Make of_dma_get_range() private of_dma_get_range() is only used within the DT core code, so remove the export and move the header declaration to the private header. Cc: Robin Murphy Reviewed-by: Geert Uytterhoeven Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Reviewed-by: Christoph Hellwig Signed-off-by: Rob Herring --- drivers/of/address.c | 1 - drivers/of/of_private.h | 11 +++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 0c3cf515c510..8e354d12fb04 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -972,7 +972,6 @@ out: return ret; } -EXPORT_SYMBOL_GPL(of_dma_get_range); /** * of_dma_is_coherent - Check if device is coherent diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index 24786818e32e..f8c58615c393 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -158,4 +158,15 @@ extern void __of_sysfs_remove_bin_file(struct device_node *np, #define for_each_transaction_entry_reverse(_oft, _te) \ list_for_each_entry_reverse(_te, &(_oft)->te_list, node) +#ifdef CONFIG_OF_ADDRESS +extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, + u64 *paddr, u64 *size); +#else +static inline int of_dma_get_range(struct device_node *np, u64 *dma_addr, + u64 *paddr, u64 *size) +{ + return -ENODEV; +} +#endif + #endif /* _LINUX_OF_PRIVATE_H */ -- cgit v1.2.3 From 76dd7068e32cec3389474d6f69ffd4d0536172da Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Thu, 4 Jul 2019 14:54:12 +0100 Subject: of: address: Report of_dma_get_range() errors meaningfully If we failed to translate a DMA address, at least show the offending address rather than the uninitialised contents of the destination argument. Signed-off-by: Robin Murphy Reviewed-by: Geert Uytterhoeven Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Reviewed-by: Christoph Hellwig Signed-off-by: Rob Herring --- drivers/of/address.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 8e354d12fb04..53d2656c2269 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -955,8 +955,8 @@ int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *siz 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); + pr_err("translation of DMA address(%llx) to CPU address failed node(%pOF)\n", + dmaaddr, np); ret = -EINVAL; goto out; } -- cgit v1.2.3 From 862ab5578f754117742c8b8c8e5ddf98bdb190ba Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 3 Jul 2019 18:23:01 +0100 Subject: of/address: Introduce of_get_next_dma_parent() helper Add of_get_next_dma_parent() helper which is similar to __of_get_dma_parent(), but can be used in iterators and decrements the ref count on the prior parent. Signed-off-by: Robin Murphy Reviewed-by: Geert Uytterhoeven Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 53d2656c2269..e9188c82fdae 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -695,6 +695,16 @@ static struct device_node *__of_get_dma_parent(const struct device_node *np) return of_node_get(args.np); } +static struct device_node *of_get_next_dma_parent(struct device_node *np) +{ + struct device_node *parent; + + parent = __of_get_dma_parent(np); + of_node_put(np); + + return parent; +} + u64 of_translate_dma_address(struct device_node *dev, const __be32 *in_addr) { struct device_node *host; -- cgit v1.2.3 From c60bf3eb888a362100aa1bdbea351dab681e262a Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 3 Jul 2019 14:47:31 +0100 Subject: of: address: Follow DMA parent for "dma-coherent" Much like for address translation, when checking for DMA coherence we should be sure to walk up the DMA hierarchy, rather than the MMIO one, now that we can accommodate them being different. Signed-off-by: Robin Murphy Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index e9188c82fdae..3fd34f7ad772 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -999,7 +999,7 @@ bool of_dma_is_coherent(struct device_node *np) of_node_put(node); return true; } - node = of_get_next_parent(node); + node = of_get_next_dma_parent(node); } of_node_put(node); return false; -- cgit v1.2.3 From b68ac8dc22ebbf003e26e44bf4dd3030c076df5a Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Tue, 2 Jul 2019 18:42:39 +0100 Subject: of: Factor out #{addr,size}-cells parsing In some cases such as PCI host controllers, we may have a "parent bus" which is an OF leaf node, but still need to correctly parse ranges from the point of view of that bus. For that, factor out variants of the "#addr-cells" and "#size-cells" parsers which do not assume they have a device node and thus immediately traverse upwards before reading the relevant property. Signed-off-by: Robin Murphy [robh: don't make of_bus_n_{addr,size}_cells() public] Reviewed-by: Geert Uytterhoeven Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 2 ++ drivers/of/base.c | 32 ++++++++++++++++++++++---------- drivers/of/of_private.h | 3 +++ 3 files changed, 27 insertions(+), 10 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 3fd34f7ad772..887c0413f648 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -14,6 +14,8 @@ #include #include +#include "of_private.h" + /* Max address size we deal with */ #define OF_MAX_ADDR_CELLS 4 #define OF_CHECK_ADDR_COUNT(na) ((na) > 0 && (na) <= OF_MAX_ADDR_CELLS) diff --git a/drivers/of/base.c b/drivers/of/base.c index 1d667eb730e1..db7fbc0c0893 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -86,34 +86,46 @@ static bool __of_node_is_type(const struct device_node *np, const char *type) return np && match && type && !strcmp(match, type); } -int of_n_addr_cells(struct device_node *np) +int of_bus_n_addr_cells(struct device_node *np) { u32 cells; - do { - if (np->parent) - np = np->parent; + for (; np; np = np->parent) if (!of_property_read_u32(np, "#address-cells", &cells)) return cells; - } while (np->parent); + /* No #address-cells property for the root node */ return OF_ROOT_NODE_ADDR_CELLS_DEFAULT; } + +int of_n_addr_cells(struct device_node *np) +{ + if (np->parent) + np = np->parent; + + return of_bus_n_addr_cells(np); +} EXPORT_SYMBOL(of_n_addr_cells); -int of_n_size_cells(struct device_node *np) +int of_bus_n_size_cells(struct device_node *np) { u32 cells; - do { - if (np->parent) - np = np->parent; + for (; np; np = np->parent) if (!of_property_read_u32(np, "#size-cells", &cells)) return cells; - } while (np->parent); + /* No #size-cells property for the root node */ return OF_ROOT_NODE_SIZE_CELLS_DEFAULT; } + +int of_n_size_cells(struct device_node *np) +{ + if (np->parent) + np = np->parent; + + return of_bus_n_size_cells(np); +} EXPORT_SYMBOL(of_n_size_cells); #ifdef CONFIG_NUMA diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h index f8c58615c393..66294d29942a 100644 --- a/drivers/of/of_private.h +++ b/drivers/of/of_private.h @@ -158,6 +158,9 @@ extern void __of_sysfs_remove_bin_file(struct device_node *np, #define for_each_transaction_entry_reverse(_oft, _te) \ list_for_each_entry_reverse(_te, &(_oft)->te_list, node) +extern int of_bus_n_addr_cells(struct device_node *np); +extern int of_bus_n_size_cells(struct device_node *np); + #ifdef CONFIG_OF_ADDRESS extern int of_dma_get_range(struct device_node *np, u64 *dma_addr, u64 *paddr, u64 *size); -- cgit v1.2.3 From 04db93a95aef392a98f9ffa8745da2e7c58ba75b Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 20 Sep 2019 13:28:53 -0500 Subject: of/unittest: Add dma-ranges address translation tests The functions for parsing 'dma-ranges' ranges are buggy and fail to handle several conditions. Add new tests for of_dma_get_range() and for_each_of_pci_range(). With this test, we get 5 new failures which are fixed in subsequent commits: OF: translation of DMA address(0) to CPU address failed node(/testcase-data/address-tests/device@70000000) FAIL of_unittest_dma_ranges_one():798 of_dma_get_range failed on node /testcase-data/address-tests/device@70000000 rc=-22 OF: translation of DMA address(10000000) to CPU address failed node(/testcase-data/address-tests/bus@80000000/device@1000) FAIL of_unittest_dma_ranges_one():798 of_dma_get_range failed on node /testcase-data/address-tests/bus@80000000/device@1000 rc=-22 OF: translation of DMA address(0) to CPU address failed node(/testcase-data/address-tests/pci@90000000) FAIL of_unittest_dma_ranges_one():798 of_dma_get_range failed on node /testcase-data/address-tests/pci@90000000 rc=-22 FAIL of_unittest_pci_dma_ranges():851 for_each_of_pci_range wrong CPU addr (d0000000) on node /testcase-data/address-tests/pci@90000000 FAIL of_unittest_pci_dma_ranges():861 for_each_of_pci_range wrong CPU addr (ffffffffffffffff) on node /testcase-data/address-tests/pci@90000000 Cc: Robin Murphy Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/unittest-data/testcases.dts | 1 + drivers/of/unittest-data/tests-address.dtsi | 48 +++++++++++++++ drivers/of/unittest.c | 92 +++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+) create mode 100644 drivers/of/unittest-data/tests-address.dtsi (limited to 'drivers/of') diff --git a/drivers/of/unittest-data/testcases.dts b/drivers/of/unittest-data/testcases.dts index 55fe0ee20109..a85b5e1c381a 100644 --- a/drivers/of/unittest-data/testcases.dts +++ b/drivers/of/unittest-data/testcases.dts @@ -15,5 +15,6 @@ #include "tests-phandle.dtsi" #include "tests-interrupts.dtsi" #include "tests-match.dtsi" +#include "tests-address.dtsi" #include "tests-platform.dtsi" #include "tests-overlay.dtsi" diff --git a/drivers/of/unittest-data/tests-address.dtsi b/drivers/of/unittest-data/tests-address.dtsi new file mode 100644 index 000000000000..3fe5d3987beb --- /dev/null +++ b/drivers/of/unittest-data/tests-address.dtsi @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 + +/ { + #address-cells = <1>; + #size-cells = <1>; + + testcase-data { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + address-tests { + #address-cells = <1>; + #size-cells = <1>; + /* ranges here is to make sure we don't use it for + * dma-ranges translation */ + ranges = <0x70000000 0x70000000 0x40000000>, + <0x00000000 0xd0000000 0x20000000>; + dma-ranges = <0x0 0x20000000 0x40000000>; + + device@70000000 { + reg = <0x70000000 0x1000>; + }; + + bus@80000000 { + #address-cells = <1>; + #size-cells = <1>; + ranges = <0x0 0x80000000 0x100000>; + dma-ranges = <0x10000000 0x0 0x40000000>; + + device@1000 { + reg = <0x1000 0x1000>; + }; + }; + + pci@90000000 { + device_type = "pci"; + #address-cells = <3>; + #size-cells = <2>; + reg = <0x90000000 0x1000>; + ranges = <0x42000000 0x0 0x40000000 0x40000000 0x0 0x10000000>; + dma-ranges = <0x42000000 0x0 0x80000000 0x00000000 0x0 0x10000000>, + <0x42000000 0x0 0xc0000000 0x20000000 0x0 0x10000000>; + }; + + }; + }; +}; diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 480a21e2ed39..141f23aa4a6b 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -779,6 +780,95 @@ static void __init of_unittest_changeset(void) #endif } +static void __init of_unittest_dma_ranges_one(const char *path, + u64 expect_dma_addr, u64 expect_paddr, u64 expect_size) +{ + struct device_node *np; + u64 dma_addr, paddr, size; + int rc; + + np = of_find_node_by_path(path); + if (!np) { + pr_err("missing testcase data\n"); + return; + } + + rc = of_dma_get_range(np, &dma_addr, &paddr, &size); + + unittest(!rc, "of_dma_get_range failed on node %pOF rc=%i\n", np, rc); + if (!rc) { + unittest(size == expect_size, + "of_dma_get_range wrong size on node %pOF size=%llx\n", np, size); + unittest(paddr == expect_paddr, + "of_dma_get_range wrong phys addr (%llx) on node %pOF", paddr, np); + unittest(dma_addr == expect_dma_addr, + "of_dma_get_range wrong DMA addr (%llx) on node %pOF", dma_addr, np); + } + of_node_put(np); +} + +static void __init of_unittest_parse_dma_ranges(void) +{ + of_unittest_dma_ranges_one("/testcase-data/address-tests/device@70000000", + 0x0, 0x20000000, 0x40000000); + of_unittest_dma_ranges_one("/testcase-data/address-tests/bus@80000000/device@1000", + 0x10000000, 0x20000000, 0x40000000); + of_unittest_dma_ranges_one("/testcase-data/address-tests/pci@90000000", + 0x80000000, 0x20000000, 0x10000000); +} + +static void __init of_unittest_pci_dma_ranges(void) +{ + struct device_node *np; + struct of_pci_range range; + struct of_pci_range_parser parser; + int i = 0; + + if (!IS_ENABLED(CONFIG_PCI)) + return; + + np = of_find_node_by_path("/testcase-data/address-tests/pci@90000000"); + if (!np) { + pr_err("missing testcase data\n"); + return; + } + + if (of_pci_dma_range_parser_init(&parser, np)) { + pr_err("missing dma-ranges property\n"); + return; + } + + /* + * Get the dma-ranges from the device tree + */ + for_each_of_pci_range(&parser, &range) { + if (!i) { + unittest(range.size == 0x10000000, + "for_each_of_pci_range wrong size on node %pOF size=%llx\n", + np, range.size); + unittest(range.cpu_addr == 0x20000000, + "for_each_of_pci_range wrong CPU addr (%llx) on node %pOF", + range.cpu_addr, np); + unittest(range.pci_addr == 0x80000000, + "for_each_of_pci_range wrong DMA addr (%llx) on node %pOF", + range.pci_addr, np); + } else { + unittest(range.size == 0x10000000, + "for_each_of_pci_range wrong size on node %pOF size=%llx\n", + np, range.size); + unittest(range.cpu_addr == 0x40000000, + "for_each_of_pci_range wrong CPU addr (%llx) on node %pOF", + range.cpu_addr, np); + unittest(range.pci_addr == 0xc0000000, + "for_each_of_pci_range wrong DMA addr (%llx) on node %pOF", + range.pci_addr, np); + } + i++; + } + + of_node_put(np); +} + static void __init of_unittest_parse_interrupts(void) { struct device_node *np; @@ -2554,6 +2644,8 @@ static int __init of_unittest(void) of_unittest_changeset(); of_unittest_parse_interrupts(); of_unittest_parse_interrupts_extended(); + of_unittest_parse_dma_ranges(); + of_unittest_pci_dma_ranges(); of_unittest_match_node(); of_unittest_platform_populate(); of_unittest_overlay(); -- cgit v1.2.3 From 81db12ee15cb83926e290a8a3654a2dfebc80935 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Wed, 4 Sep 2019 11:43:30 +0100 Subject: of/address: Translate 'dma-ranges' for parent nodes missing 'dma-ranges' 'dma-ranges' frequently exists without parent nodes having 'dma-ranges'. While this is an error for 'ranges', this is fine because DMA capable devices always have a translatable DMA address. Also, with no 'dma-ranges' at all, the assumption is that DMA addresses are 1:1 with no restrictions unless perhaps the device itself has implicit restrictions. Cc: Robin Murphy Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 887c0413f648..1c291bd6bce2 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -519,9 +519,13 @@ static int of_translate_one(struct device_node *parent, struct of_bus *bus, * * As far as we know, this damage only exists on Apple machines, so * This code is only enabled on powerpc. --gcl + * + * This quirk also applies for 'dma-ranges' which frequently exist in + * child nodes without 'dma-ranges' in the parent nodes. --RobH */ ranges = of_get_property(parent, rprop, &rlen); - if (ranges == NULL && !of_empty_ranges_quirk(parent)) { + if (ranges == NULL && !of_empty_ranges_quirk(parent) && + strcmp(rprop, "dma-ranges")) { pr_debug("no ranges; cannot translate\n"); return 1; } -- cgit v1.2.3 From 645c138636de3d6d6ed7d92edec39298fd6873d7 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Thu, 5 Sep 2019 10:47:26 +0100 Subject: of/address: Fix of_pci_range_parser_one translation of DMA addresses of_pci_range_parser_one() has a bug when parsing dma-ranges. When it translates the parent address (aka cpu address in the code), 'ranges' is always being used. This happens to work because most users are just 1:1 translation. Cc: Robin Murphy Tested-by: Nicolas Saenz Julienne Reviewed-by: Nicolas Saenz Julienne Signed-off-by: Rob Herring --- drivers/of/address.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/address.c b/drivers/of/address.c index 1c291bd6bce2..5ce69d026584 100644 --- a/drivers/of/address.c +++ b/drivers/of/address.c @@ -243,6 +243,7 @@ static int parser_init(struct of_pci_range_parser *parser, parser->node = node; parser->pna = of_n_addr_cells(node); parser->np = parser->pna + na + ns; + parser->dma = !strcmp(name, "dma-ranges"); parser->range = of_get_property(node, name, &rlen); if (parser->range == NULL) @@ -281,7 +282,11 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser, range->pci_space = be32_to_cpup(parser->range); range->flags = of_bus_pci_get_flags(parser->range); range->pci_addr = of_read_number(parser->range + 1, ns); - range->cpu_addr = of_translate_address(parser->node, + if (parser->dma) + range->cpu_addr = of_translate_dma_address(parser->node, + parser->range + na); + else + range->cpu_addr = of_translate_address(parser->node, parser->range + na); range->size = of_read_number(parser->range + parser->pna + na, ns); @@ -294,8 +299,12 @@ struct of_pci_range *of_pci_range_parser_one(struct of_pci_range_parser *parser, flags = of_bus_pci_get_flags(parser->range); pci_addr = of_read_number(parser->range + 1, ns); - cpu_addr = of_translate_address(parser->node, - parser->range + na); + if (parser->dma) + cpu_addr = of_translate_dma_address(parser->node, + parser->range + na); + else + cpu_addr = of_translate_address(parser->node, + parser->range + na); size = of_read_number(parser->range + parser->pna + na, ns); if (flags != range->flags) -- cgit v1.2.3 From 951d48855d86e72e0d6de73440fe09d363168064 Mon Sep 17 00:00:00 2001 From: Robin Murphy Date: Wed, 3 Jul 2019 18:42:20 +0100 Subject: of: Make of_dma_get_range() work on bus nodes 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 [robh: Allow for the bus child node to still be passed in] Signed-off-by: Rob Herring Reviewed-by: Robin Murphy Reviewed-by: Nicolas Saenz Julienne Tested-by: Nicolas Saenz Julienne --- drivers/of/address.c | 44 ++++++++++++++++++-------------------------- 1 file changed, 18 insertions(+), 26 deletions(-) (limited to 'drivers/of') 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); -- cgit v1.2.3 From 7f3fefeec2ce0d8f3598c53d6decca3a4fd5cdaf Mon Sep 17 00:00:00 2001 From: Matti Vaittinen Date: Wed, 13 Nov 2019 08:43:38 +0200 Subject: of: property: Fix documentation for out values Property fetching functions which return number of successfully fetched properties should not state that out-values are only modified if 0 is returned. Fix this. Also, "pointer to return value" is slightly suboptimal phrase as "return value" commonly refers to value function returns (not via arguments). Rather use "pointer to found values". Signed-off-by: Matti Vaittinen Reviewed-by: Frank Rowand Signed-off-by: Rob Herring --- drivers/of/property.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/property.c b/drivers/of/property.c index d7fa75e31f22..c1dd22ed03f3 100644 --- a/drivers/of/property.c +++ b/drivers/of/property.c @@ -164,7 +164,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u64_index); * * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. - * @out_values: pointer to return value, modified only if return value is 0. + * @out_values: pointer to found values. * @sz_min: minimum number of array elements to read * @sz_max: maximum number of array elements to read, if zero there is no * upper limit on the number of elements in the dts entry but only @@ -212,7 +212,7 @@ EXPORT_SYMBOL_GPL(of_property_read_variable_u8_array); * * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. - * @out_values: pointer to return value, modified only if return value is 0. + * @out_values: pointer to found values. * @sz_min: minimum number of array elements to read * @sz_max: maximum number of array elements to read, if zero there is no * upper limit on the number of elements in the dts entry but only @@ -260,7 +260,7 @@ EXPORT_SYMBOL_GPL(of_property_read_variable_u16_array); * * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. - * @out_values: pointer to return value, modified only if return value is 0. + * @out_values: pointer to return found values. * @sz_min: minimum number of array elements to read * @sz_max: maximum number of array elements to read, if zero there is no * upper limit on the number of elements in the dts entry but only @@ -334,7 +334,7 @@ EXPORT_SYMBOL_GPL(of_property_read_u64); * * @np: device node from which the property value is to be read. * @propname: name of the property to be searched. - * @out_values: pointer to return value, modified only if return value is 0. + * @out_values: pointer to found values. * @sz_min: minimum number of array elements to read * @sz_max: maximum number of array elements to read, if zero there is no * upper limit on the number of elements in the dts entry but only -- cgit v1.2.3 From 637392a8506a3a7dd24ab9094a14f7522adb73b4 Mon Sep 17 00:00:00 2001 From: Frank Rowand Date: Thu, 21 Nov 2019 13:16:56 -0600 Subject: of: overlay: add_changeset_property() memory leak No changeset entries are created for #address-cells and #size-cells properties, but the duplicated properties are never freed. This results in a memory leak which is detected by kmemleak: unreferenced object 0x85887180 (size 64): backtrace: kmem_cache_alloc_trace+0x1fb/0x1fc __of_prop_dup+0x25/0x7c add_changeset_property+0x17f/0x370 build_changeset_next_level+0x29/0x20c of_overlay_fdt_apply+0x32b/0x6b4 ... Fixes: 6f75118800ac ("of: overlay: validate overlay properties #address-cells and #size-cells") Reported-by: Vincent Whitchurch Signed-off-by: Frank Rowand Tested-by: Vincent Whitchurch Signed-off-by: Rob Herring --- drivers/of/overlay.c | 37 ++++++++++++++++++++----------------- 1 file changed, 20 insertions(+), 17 deletions(-) (limited to 'drivers/of') diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index c423e94baf0f..9617b7df7c4d 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -305,7 +305,6 @@ static int add_changeset_property(struct overlay_changeset *ovcs, { struct property *new_prop = NULL, *prop; int ret = 0; - bool check_for_non_overlay_node = false; if (target->in_livetree) if (!of_prop_cmp(overlay_prop->name, "name") || @@ -318,6 +317,25 @@ static int add_changeset_property(struct overlay_changeset *ovcs, else prop = NULL; + if (prop) { + if (!of_prop_cmp(prop->name, "#address-cells")) { + if (!of_prop_val_eq(prop, overlay_prop)) { + pr_err("ERROR: changing value of #address-cells is not allowed in %pOF\n", + target->np); + ret = -EINVAL; + } + return ret; + + } else if (!of_prop_cmp(prop->name, "#size-cells")) { + if (!of_prop_val_eq(prop, overlay_prop)) { + pr_err("ERROR: changing value of #size-cells is not allowed in %pOF\n", + target->np); + ret = -EINVAL; + } + return ret; + } + } + if (is_symbols_prop) { if (prop) return -EINVAL; @@ -330,33 +348,18 @@ static int add_changeset_property(struct overlay_changeset *ovcs, return -ENOMEM; if (!prop) { - check_for_non_overlay_node = true; if (!target->in_livetree) { new_prop->next = target->np->deadprops; target->np->deadprops = new_prop; } ret = of_changeset_add_property(&ovcs->cset, target->np, new_prop); - } else if (!of_prop_cmp(prop->name, "#address-cells")) { - if (!of_prop_val_eq(prop, new_prop)) { - pr_err("ERROR: changing value of #address-cells is not allowed in %pOF\n", - target->np); - ret = -EINVAL; - } - } else if (!of_prop_cmp(prop->name, "#size-cells")) { - if (!of_prop_val_eq(prop, new_prop)) { - pr_err("ERROR: changing value of #size-cells is not allowed in %pOF\n", - target->np); - ret = -EINVAL; - } } else { - check_for_non_overlay_node = true; ret = of_changeset_update_property(&ovcs->cset, target->np, new_prop); } - if (check_for_non_overlay_node && - !of_node_check_flag(target->np, OF_OVERLAY)) + if (!of_node_check_flag(target->np, OF_OVERLAY)) pr_err("WARNING: memory leak will occur if overlay removed, property: %pOF/%s\n", target->np, new_prop->name); -- cgit v1.2.3 From 2aacace6dbbb6b6ce4e177e6c7ea901f389c0472 Mon Sep 17 00:00:00 2001 From: Erhard Furtner Date: Tue, 26 Nov 2019 02:48:04 +0100 Subject: of: unittest: fix memory leak in attach_node_and_children In attach_node_and_children memory is allocated for full_name via kasprintf. If the condition of the 1st if is not met the function returns early without freeing the memory. Add a kfree() to fix that. This has been detected with kmemleak: Link: https://bugzilla.kernel.org/show_bug.cgi?id=205327 It looks like the leak was introduced by this commit: Fixes: 5babefb7f7ab ("of: unittest: allow base devicetree to have symbol metadata") Signed-off-by: Erhard Furtner Reviewed-by: Michael Ellerman Reviewed-by: Tyrel Datwyler Signed-off-by: Rob Herring --- drivers/of/unittest.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/of') diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c index 445f134b62c0..68b87587b2ef 100644 --- a/drivers/of/unittest.c +++ b/drivers/of/unittest.c @@ -1236,8 +1236,10 @@ static void attach_node_and_children(struct device_node *np) full_name = kasprintf(GFP_KERNEL, "%pOF", np); if (!strcmp(full_name, "/__local_fixups__") || - !strcmp(full_name, "/__fixups__")) + !strcmp(full_name, "/__fixups__")) { + kfree(full_name); return; + } dup = of_find_node_by_path(full_name); kfree(full_name); -- cgit v1.2.3