of/address: Return an error when no valid dma-ranges are found
PATCH v2 1/1 of/address: Return an error when no valid dma-ranges are found
Commit 7a8b64d17e35 ("of/address: use range parser for of_dma_get_range") converted the parsing of dma-range properties to use code shared with the PCI range parser. The intent was to introduce no functional changes however in the case where we fail to translate the first resource instead of returning -EINVAL the new code we return 0. Restore the previous behaviour by returning an error if we find no valid ranges, the original code only handled the first range but subsequently support for parsing all supplied ranges was added.
This avoids confusing code using the parsed ranges which doesn't expect to successfully parse ranges but have only a list terminator returned, this fixes breakage with so far as I can tell all DMA for on SoC devices on the Socionext Synquacer platform which has a firmware supplied DT. A bisect identified the original conversion as triggering the issues there.
---
Changes in v2:
- Don't leak parsed resources.
- Link to v1: https://lore.kernel.org/r/[email protected]
--- drivers/of/address.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
drivers/of/address.c +15 -6
@@ -965,8 +965,19 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map) }of_dma_range_parser_init (&parser, node); -for_each_of_range (&parser, &range) +for_each_of_range (&parser, &range) { +if (range.cpu_addr ==OF_BAD_ADDR ) { +pr_err ("translation of DMA address(%llx) to CPU address failed node(%pOF)\n" , + range.bus_addr, node); +continue ; + } num_ranges++; + } + +if (!num_ranges) { + ret = -EINVAL ; + goto out; + } r =kcalloc (num_ranges +1 ,sizeof (*r),GFP_KERNEL );if (!r) {
@@ -975,18 +986,16 @@ int of_dma_get_range(struct device_node *np, const struct bus_dma_region **map) }/* -* Record all info in the generic DMA ranges array for struct device. +* Record all info in the generic DMA ranges array for struct device, +* returning an error if we don't find any parsable ranges. */ *map = r;of_dma_range_parser_init (&parser, node);for_each_of_range (&parser, &range) {pr_debug ("dma_addr(%llx) cpu_addr(%llx) size(%llx)\n" , range.bus_addr, range.cpu_addr, range.size); -if (range.cpu_addr ==OF_BAD_ADDR ) { -pr_err ("translation of DMA address(%llx) to CPU address failed node(%pOF)\n" , - range.bus_addr, node); +if (range.cpu_addr ==OF_BAD_ADDR )continue ; - } r->cpu_start = range.cpu_addr; r->dma_start = range.bus_addr; r->size = range.size;
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20230126-synquacer-boot-243bd1b87f64
Best regards,
Signature · 1 line
Mark Brown <[email protected]>
Quoted 20 lines · Mark Brown
Commit
7a8b64d17e35("of/address: use range parser forof_dma_get_range") converted the parsing of dma-range properties to use code shared with the PCI range parser. The intent was to introduce no functional changes however in the case where we fail to translate the first resource instead of returning-EINVALthe new code we return 0. Restore the previous behaviour by returning an error if we find no valid ranges, the original code only handled the first range but subsequently support for parsing all supplied ranges was added.This avoids confusing code using the parsed ranges which doesn't expect to successfully parse ranges but have only a list terminator returned, this fixes breakage with so far as I can tell all DMA for on SoC devices on the Socionext Synquacer platform which has a firmware supplied DT. A bisect identified the original conversion as triggering the issues there.
Fixes 7a8b64d17e35 ("of/address: use range parser for of_dma_get_range")Signed-off-by Mark Brown <[email protected]>Cc Luca Di Stefano <[email protected]>
Quoted 8 lines
---
Changes in v2:
- Don't leak parsed resources.
- Link to v1: https://lore.kernel.org/r/[email protected]--- drivers/of/address.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-)
Applied, thanks!