[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <674efe8d-c299-4ce9-bf6b-c1920a5393eb@samsung.com>
Date: Thu, 27 Nov 2025 15:28:35 +0100
From: Marek Szyprowski <m.szyprowski@...sung.com>
To: "Rob Herring (Arm)" <robh@...nel.org>, Bjorn Andersson
<andersson@...nel.org>, Mathieu Poirier <mathieu.poirier@...aro.org>, Shawn
Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>, Fabio Estevam
<festevam@...il.com>, Geert Uytterhoeven <geert+renesas@...der.be>, Magnus
Damm <magnus.damm@...il.com>, Patrice Chotard <patrice.chotard@...s.st.com>,
Maxime Coquelin <mcoquelin.stm32@...il.com>, Alexandre Torgue
<alexandre.torgue@...s.st.com>
Cc: Arnaud Pouliquen <arnaud.pouliquen@...s.st.com>, Peng Fan
<peng.fan@....com>, Beleswar Padhi <b-padhi@...com>,
linux-remoteproc@...r.kernel.org, imx@...ts.linux.dev,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
linux-renesas-soc@...r.kernel.org, linux-stm32@...md-mailman.stormreply.com,
linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_*
functions for "memory-region"
Hi Rob,
On 24.11.2025 19:27, Rob Herring (Arm) wrote:
> Use the newly added of_reserved_mem_region_to_resource() and
> of_reserved_mem_region_count() functions to handle "memory-region"
> properties.
>
> The error handling is a bit different in some cases. Often
> "memory-region" is optional, so failed lookup is not an error. But then
> an error in of_reserved_mem_lookup() is treated as an error. However,
> that distinction is not really important. Either the region is available
> and usable or it is not. So now, it is just
> of_reserved_mem_region_to_resource() which is checked for an error.
>
> Signed-off-by: Rob Herring (Arm) <robh@...nel.org>
This patch landed in today's linux-next as commit c70b9d5fdcd7
("remoteproc: qcom: Use of_reserved_mem_region_* functions for
"memory-region""). In my tests I found that it breaks booting of
DragonBoard410c (arch/arm64/boot/dts/qcom/apq8016-sbc.dts) by causing
the NULL pointer dereference. The issue is caused by replacing
devm_ioremap_wc() with devm_ioremap_resource_wc(), which fails on
devm_request_mem_region(), see comment in the code below. It looks that
the error handling is somewhere broken. Here is the the kernel log:
remoteproc remoteproc0: 4080000.remoteproc is available
qcom-wcnss-pil a204000.remoteproc: error -EBUSY: can't request region
for resource [mem 0x8e200000-0x8e7fffff]
remoteproc remoteproc1: a204000.remoteproc is available
remoteproc remoteproc1: powering up a204000.remoteproc
remoteproc remoteproc1: Booting fw image qcom/apq8016/wcnss.mbn, size
4111376
Unable to handle kernel paging request at virtual address fffffffffffffff0
Mem abort info:
...
Internal error: Oops: 0000000096000046 [#1] SMP
Modules linked in: cpufreq_powersave qcom_wcnss_pil cpufreq_conservative
coresight_stm coresight_replicator coresight_tmc coresight_tpiu stm_core
coresight_funnel coresight_cpu_debug coresight_cti(+) adv7511 coresight
nfc rfkill msm snd_soc_lpass_apq8016 snd_soc_apq8016_sbc
snd_soc_lpass_cpu snd_soc_msm8916_analog snd_soc_msm8916_digital
snd_soc_qcom_common snd_soc_lpass_platform snd_soc_core qrtr ubwc_config
snd_compress llcc_qcom snd_pcm_dmaengine qcom_q6v5_mss snd_pcm ocmem
qcom_pil_info qcom_spmi_vadc qcom_camss drm_gpuvm qcom_pon rtc_pm8xxx
qcom_q6v5 qcom_spmi_temp_alarm venus_core qcom_vadc_common snd_timer
drm_exec qcom_sysmon snd qcom_common gpu_sched videobuf2_dma_sg
v4l2_mem2mem qcom_glink_smem v4l2_fwnode soundcore drm_dp_aux_bus
qmi_helpers mdt_loader v4l2_async videobuf2_memops videobuf2_v4l2
videodev qnoc_msm8916 videobuf2_common qcom_rng drm_display_helper mc
qcom_stats rpmsg_ctrl rpmsg_char display_connector ramoops socinfo
rmtfs_mem reed_solomon ax88796b asix usbnet phy_qcom_usb_hs ipv6 libsha1
CPU: 2 UID: 0 PID: 28 Comm: kworker/2:0 Tainted: G W
6.18.0-rc1+ #16209 PREEMPT
Tainted: [W]=WARN
lr : __qcom_mdt_load+0x210/0x304 [mdt_loader]
Call trace:
__pi_memcpy_generic+0x128/0x22c (P)
qcom_mdt_load+0x68/0x60c [mdt_loader]
wcnss_load+0x2c/0x5c [qcom_wcnss_pil]
rproc_start+0x30/0x1b4
rproc_boot+0x19c/0x560
rproc_auto_boot_callback+0x1c/0x34
request_firmware_work_func+0x4c/0x98
process_one_work+0x208/0x60c
worker_thread+0x244/0x388
kthread+0x150/0x228
ret_from_fork+0x10/0x20
Code: 927cec03 cb0e0021 8b0e0042 a9411c26 (a900340c)
---[ end trace 0000000000000000 ]---
> ---
> v7:
> - Split QCom to separate patch
> ---
> drivers/remoteproc/qcom_q6v5_adsp.c | 24 ++++------
> drivers/remoteproc/qcom_q6v5_mss.c | 60 ++++++++-----------------
> drivers/remoteproc/qcom_q6v5_pas.c | 69 +++++++++++------------------
> drivers/remoteproc/qcom_q6v5_wcss.c | 25 +++++------
> drivers/remoteproc/qcom_wcnss.c | 23 ++++------
> 5 files changed, 72 insertions(+), 129 deletions(-)
>
> ...
> diff --git a/drivers/remoteproc/qcom_wcnss.c b/drivers/remoteproc/qcom_wcnss.c
> index 2c7e519a2254..14005fb049a2 100644
> --- a/drivers/remoteproc/qcom_wcnss.c
> +++ b/drivers/remoteproc/qcom_wcnss.c
> @@ -526,25 +526,20 @@ static int wcnss_request_irq(struct qcom_wcnss *wcnss,
>
> static int wcnss_alloc_memory_region(struct qcom_wcnss *wcnss)
> {
> - struct reserved_mem *rmem = NULL;
> - struct device_node *node;
> -
> - node = of_parse_phandle(wcnss->dev->of_node, "memory-region", 0);
> - if (node)
> - rmem = of_reserved_mem_lookup(node);
> - of_node_put(node);
> + struct resource res;
> + int ret;
>
> - if (!rmem) {
> + ret = of_reserved_mem_region_to_resource(wcnss->dev->of_node, 0, &res);
> + if (ret) {
> dev_err(wcnss->dev, "unable to resolve memory-region\n");
> - return -EINVAL;
> + return ret;
> }
>
> - wcnss->mem_phys = wcnss->mem_reloc = rmem->base;
> - wcnss->mem_size = rmem->size;
> - wcnss->mem_region = devm_ioremap_wc(wcnss->dev, wcnss->mem_phys, wcnss->mem_size);
> + wcnss->mem_phys = wcnss->mem_reloc = res.start;
> + wcnss->mem_size = resource_size(&res);
> + wcnss->mem_region = devm_ioremap_resource_wc(wcnss->dev, &res);
The above line causes the failure. After restoring it to:
wcnss->mem_region = devm_ioremap_wc(wcnss->dev, wcnss->mem_phys, wcnss->mem_size);
the mentioned board boots fine again. I'm not sure about other drivers,
if they also fail the same way as they might not be used on the tested
board.
> if (!wcnss->mem_region) {
> - dev_err(wcnss->dev, "unable to map memory region: %pa+%zx\n",
> - &rmem->base, wcnss->mem_size);
> + dev_err(wcnss->dev, "unable to map memory region: %pR\n", &res);
> return -EBUSY;
> }
>
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Powered by blists - more mailing lists