All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marek Szyprowski <m.szyprowski@samsung.com>
To: "Rob Herring (Arm)" <robh@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Mathieu Poirier <mathieu.poirier@linaro.org>,
	Shawn Guo <shawnguo@kernel.org>,
	Sascha Hauer <s.hauer@pengutronix.de>,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Fabio Estevam <festevam@gmail.com>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Patrice Chotard <patrice.chotard@foss.st.com>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>
Cc: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>,
	Peng Fan <peng.fan@nxp.com>, Beleswar Padhi <b-padhi@ti.com>,
	linux-remoteproc@vger.kernel.org, imx@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-msm@vger.kernel.org
Subject: Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
Date: Thu, 27 Nov 2025 15:28:35 +0100	[thread overview]
Message-ID: <674efe8d-c299-4ce9-bf6b-c1920a5393eb@samsung.com> (raw)
In-Reply-To: <20251124182751.507624-2-robh@kernel.org>

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@kernel.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


  reply	other threads:[~2025-11-27 14:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-24 18:27 [PATCH v7 1/2] remoteproc: Use of_reserved_mem_region_* functions for "memory-region" Rob Herring (Arm)
2025-11-24 18:27 ` [PATCH v7 2/2] remoteproc: qcom: " Rob Herring (Arm)
2025-11-27 14:28   ` Marek Szyprowski [this message]
2025-12-02 14:15     ` Rob Herring
2025-12-08 12:42       ` Marek Szyprowski
2025-12-09 14:20       ` Daniel Baluta
2026-01-27 23:07       ` André Apitzsch
2025-11-25 15:45 ` [PATCH v7 1/2] remoteproc: " Mathieu Poirier
2025-11-25 16:05 ` kernel test robot
2025-11-26 20:10 ` (subset) " Bjorn Andersson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=674efe8d-c299-4ce9-bf6b-c1920a5393eb@samsung.com \
    --to=m.szyprowski@samsung.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andersson@kernel.org \
    --cc=arnaud.pouliquen@foss.st.com \
    --cc=b-padhi@ti.com \
    --cc=festevam@gmail.com \
    --cc=geert+renesas@glider.be \
    --cc=imx@lists.linux.dev \
    --cc=kernel@pengutronix.de \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=linux-renesas-soc@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=magnus.damm@gmail.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=patrice.chotard@foss.st.com \
    --cc=peng.fan@nxp.com \
    --cc=robh@kernel.org \
    --cc=s.hauer@pengutronix.de \
    --cc=shawnguo@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.