Re: [net-next,v3,3/3] net: ipa: Grab IMEM slice base/size from DTS
From: Simon Horman <horms@kernel.org>
Date: 2026-02-18 10:41:33
Also in:
linux-arm-msm, linux-devicetree, lkml
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- net: ipa: Grab IMEM slice base/size from DTS This commit reads the IMEM slice base address and size from a device tree "sram" phandle, falling back to the existing struct fields when the DT property is absent.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ipa/ipa_mem.c b/drivers/net/ipa/ipa_mem.c index 835a3c9c1fd4..583aea625709 100644 --- a/drivers/net/ipa/ipa_mem.c +++ b/drivers/net/ipa/ipa_mem.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -656,7 +659,23 @@ int ipa_mem_init(struct ipa *ipa, struct platform_device *pdev, ipa->mem_addr = res->start; ipa->mem_size = resource_size(res); - ret = ipa_imem_init(ipa, mem_data->imem_addr, mem_data->imem_size); + ipa_slice_np = of_parse_phandle(dev->of_node, "sram", 0); + if (ipa_slice_np) { + ret = of_address_to_resource(ipa_slice_np, 0, res);
Here, res still points into the platform device's resource array from the earlier platform_get_resource_byname() call. Won't of_address_to_resource() overwrite the platform device's "ipa-shared" resource entry with the SRAM resource data? The needed values (mem_addr, mem_size) were already copied out, but this silently corrupts the platform device's resource table. Would it be better to use a local struct resource for this call? Simon says: I think this is a false positive as I don't see how reusing res here results in resource table corruption.
+ of_node_put(ipa_slice_np); + if (ret) + return ret;
^^^^^^^^^^ At this point ipa->mem_virt holds a valid memremap() pointer. Does this direct return leak the mapping? Every other error path after the memremap() in this function uses goto err_unmap, which calls memunmap(ipa->mem_virt). Should this be goto err_unmap instead? Simon says: This seems to me to be an actual problem.
+ + imem_base = res->start; + imem_size = resource_size(res);
[ ... ]
+ ret = ipa_imem_init(ipa, imem_base, imem_size); if (ret) goto err_unmap;