linux-arm-msm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
       [not found] <20251124182751.507624-1-robh@kernel.org>
@ 2025-11-24 18:27 ` Rob Herring (Arm)
       [not found]   ` <CGME20251127142839eucas1p186846c6c1ea1d9e43369fbba9bb5d17c@eucas1p1.samsung.com>
  0 siblings, 1 reply; 5+ messages in thread
From: Rob Herring (Arm) @ 2025-11-24 18:27 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Geert Uytterhoeven,
	Magnus Damm, Patrice Chotard, Maxime Coquelin, Alexandre Torgue
  Cc: Arnaud Pouliquen, Peng Fan, Beleswar Padhi, linux-remoteproc, imx,
	linux-arm-kernel, linux-kernel, linux-renesas-soc, linux-stm32,
	linux-arm-msm

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>
---
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_q6v5_adsp.c b/drivers/remoteproc/qcom_q6v5_adsp.c
index e98b7e03162c..d3933a66ed3d 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -625,26 +625,20 @@ static int adsp_init_mmio(struct qcom_adsp *adsp,
 
 static int adsp_alloc_memory_region(struct qcom_adsp *adsp)
 {
-	struct reserved_mem *rmem = NULL;
-	struct device_node *node;
-
-	node = of_parse_phandle(adsp->dev->of_node, "memory-region", 0);
-	if (node)
-		rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
+	int ret;
+	struct resource res;
 
-	if (!rmem) {
+	ret = of_reserved_mem_region_to_resource(adsp->dev->of_node, 0, &res);
+	if (ret) {
 		dev_err(adsp->dev, "unable to resolve memory-region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	adsp->mem_phys = adsp->mem_reloc = rmem->base;
-	adsp->mem_size = rmem->size;
-	adsp->mem_region = devm_ioremap_wc(adsp->dev,
-				adsp->mem_phys, adsp->mem_size);
+	adsp->mem_phys = adsp->mem_reloc = res.start;
+	adsp->mem_size = resource_size(&res);
+	adsp->mem_region = devm_ioremap_resource_wc(adsp->dev, &res);
 	if (!adsp->mem_region) {
-		dev_err(adsp->dev, "unable to map memory region: %pa+%zx\n",
-			&rmem->base, adsp->mem_size);
+		dev_err(adsp->dev, "unable to map memory region: %pR\n", &res);
 		return -EBUSY;
 	}
 
diff --git a/drivers/remoteproc/qcom_q6v5_mss.c b/drivers/remoteproc/qcom_q6v5_mss.c
index 3087d895b87f..91940977ca89 100644
--- a/drivers/remoteproc/qcom_q6v5_mss.c
+++ b/drivers/remoteproc/qcom_q6v5_mss.c
@@ -1970,8 +1970,8 @@ static int q6v5_init_reset(struct q6v5 *qproc)
 static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 {
 	struct device_node *child;
-	struct reserved_mem *rmem;
-	struct device_node *node;
+	struct resource res;
+	int ret;
 
 	/*
 	 * In the absence of mba/mpss sub-child, extract the mba and mpss
@@ -1979,71 +1979,49 @@ static int q6v5_alloc_memory_region(struct q6v5 *qproc)
 	 */
 	child = of_get_child_by_name(qproc->dev->of_node, "mba");
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node,
-					"memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 0, &res);
 	} else {
-		node = of_parse_phandle(child, "memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(child, 0, &res);
 		of_node_put(child);
 	}
 
-	if (!node) {
-		dev_err(qproc->dev, "no mba memory-region specified\n");
-		return -EINVAL;
-	}
-
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	if (ret) {
 		dev_err(qproc->dev, "unable to resolve mba region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	qproc->mba_phys = rmem->base;
-	qproc->mba_size = rmem->size;
+	qproc->mba_phys = res.start;
+	qproc->mba_size = resource_size(&res);
 
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node,
-					"memory-region", 1);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 1, &res);
 	} else {
 		child = of_get_child_by_name(qproc->dev->of_node, "mpss");
-		node = of_parse_phandle(child, "memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(child, 0, &res);
 		of_node_put(child);
 	}
 
-	if (!node) {
-		dev_err(qproc->dev, "no mpss memory-region specified\n");
-		return -EINVAL;
-	}
-
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	if (ret) {
 		dev_err(qproc->dev, "unable to resolve mpss region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	qproc->mpss_phys = qproc->mpss_reloc = rmem->base;
-	qproc->mpss_size = rmem->size;
+	qproc->mpss_phys = qproc->mpss_reloc = res.start;
+	qproc->mpss_size = resource_size(&res);
 
 	if (!child) {
-		node = of_parse_phandle(qproc->dev->of_node, "memory-region", 2);
+		ret = of_reserved_mem_region_to_resource(qproc->dev->of_node, 2, &res);
 	} else {
 		child = of_get_child_by_name(qproc->dev->of_node, "metadata");
-		node = of_parse_phandle(child, "memory-region", 0);
+		ret = of_reserved_mem_region_to_resource(child, 0, &res);
 		of_node_put(child);
 	}
 
-	if (!node)
+	if (ret)
 		return 0;
 
-	rmem = of_reserved_mem_lookup(node);
-	if (!rmem) {
-		dev_err(qproc->dev, "unable to resolve metadata region\n");
-		return -EINVAL;
-	}
-
-	qproc->mdata_phys = rmem->base;
-	qproc->mdata_size = rmem->size;
+	qproc->mdata_phys = res.start;
+	qproc->mdata_size = resource_size(&res);
 
 	return 0;
 }
diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
index 158bcd6cc85c..7bac843ce406 100644
--- a/drivers/remoteproc/qcom_q6v5_pas.c
+++ b/drivers/remoteproc/qcom_q6v5_pas.c
@@ -547,53 +547,37 @@ static void qcom_pas_pds_detach(struct qcom_pas *pas, struct device **pds, size_
 
 static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 {
-	struct reserved_mem *rmem;
-	struct device_node *node;
-
-	node = of_parse_phandle(pas->dev->of_node, "memory-region", 0);
-	if (!node) {
-		dev_err(pas->dev, "no memory-region specified\n");
-		return -EINVAL;
-	}
+	struct resource res;
+	int ret;
 
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 0, &res);
+	if (ret) {
 		dev_err(pas->dev, "unable to resolve memory-region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	pas->mem_phys = pas->mem_reloc = rmem->base;
-	pas->mem_size = rmem->size;
-	pas->mem_region = devm_ioremap_wc(pas->dev, pas->mem_phys, pas->mem_size);
+	pas->mem_phys = pas->mem_reloc = res.start;
+	pas->mem_size = resource_size(&res);
+	pas->mem_region = devm_ioremap_resource_wc(pas->dev, &res);
 	if (!pas->mem_region) {
-		dev_err(pas->dev, "unable to map memory region: %pa+%zx\n",
-			&rmem->base, pas->mem_size);
+		dev_err(pas->dev, "unable to map memory region: %pR\n", &res);
 		return -EBUSY;
 	}
 
 	if (!pas->dtb_pas_id)
 		return 0;
 
-	node = of_parse_phandle(pas->dev->of_node, "memory-region", 1);
-	if (!node) {
-		dev_err(pas->dev, "no dtb memory-region specified\n");
-		return -EINVAL;
-	}
-
-	rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-	if (!rmem) {
+	ret = of_reserved_mem_region_to_resource(pas->dev->of_node, 1, &res);
+	if (ret) {
 		dev_err(pas->dev, "unable to resolve dtb memory-region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	pas->dtb_mem_phys = pas->dtb_mem_reloc = rmem->base;
-	pas->dtb_mem_size = rmem->size;
-	pas->dtb_mem_region = devm_ioremap_wc(pas->dev, pas->dtb_mem_phys, pas->dtb_mem_size);
+	pas->dtb_mem_phys = pas->dtb_mem_reloc = res.start;
+	pas->dtb_mem_size = resource_size(&res);
+	pas->dtb_mem_region = devm_ioremap_resource_wc(pas->dev, &res);
 	if (!pas->dtb_mem_region) {
-		dev_err(pas->dev, "unable to map dtb memory region: %pa+%zx\n",
-			&rmem->base, pas->dtb_mem_size);
+		dev_err(pas->dev, "unable to map dtb memory region: %pR\n", &res);
 		return -EBUSY;
 	}
 
@@ -603,7 +587,6 @@ static int qcom_pas_alloc_memory_region(struct qcom_pas *pas)
 static int qcom_pas_assign_memory_region(struct qcom_pas *pas)
 {
 	struct qcom_scm_vmperm perm[MAX_ASSIGN_COUNT];
-	struct device_node *node;
 	unsigned int perm_size;
 	int offset;
 	int ret;
@@ -612,17 +595,15 @@ static int qcom_pas_assign_memory_region(struct qcom_pas *pas)
 		return 0;
 
 	for (offset = 0; offset < pas->region_assign_count; ++offset) {
-		struct reserved_mem *rmem = NULL;
-
-		node = of_parse_phandle(pas->dev->of_node, "memory-region",
-					pas->region_assign_idx + offset);
-		if (node)
-			rmem = of_reserved_mem_lookup(node);
-		of_node_put(node);
-		if (!rmem) {
+		struct resource res;
+
+		ret = of_reserved_mem_region_to_resource(pas->dev->of_node,
+							 pas->region_assign_idx + offset,
+							 &res);
+		if (ret) {
 			dev_err(pas->dev, "unable to resolve shareable memory-region index %d\n",
 				offset);
-			return -EINVAL;
+			return ret;
 		}
 
 		if (pas->region_assign_shared)  {
@@ -637,8 +618,8 @@ static int qcom_pas_assign_memory_region(struct qcom_pas *pas)
 			perm_size = 1;
 		}
 
-		pas->region_assign_phys[offset] = rmem->base;
-		pas->region_assign_size[offset] = rmem->size;
+		pas->region_assign_phys[offset] = res.start;
+		pas->region_assign_size[offset] = resource_size(&res);
 		pas->region_assign_owners[offset] = BIT(QCOM_SCM_VMID_HLOS);
 
 		ret = qcom_scm_assign_mem(pas->region_assign_phys[offset],
diff --git a/drivers/remoteproc/qcom_q6v5_wcss.c b/drivers/remoteproc/qcom_q6v5_wcss.c
index 07c88623f597..ca748e3bcc7f 100644
--- a/drivers/remoteproc/qcom_q6v5_wcss.c
+++ b/drivers/remoteproc/qcom_q6v5_wcss.c
@@ -873,27 +873,22 @@ static int q6v5_wcss_init_mmio(struct q6v5_wcss *wcss,
 
 static int q6v5_alloc_memory_region(struct q6v5_wcss *wcss)
 {
-	struct reserved_mem *rmem = NULL;
-	struct device_node *node;
 	struct device *dev = wcss->dev;
+	struct resource res;
+	int ret;
 
-	node = of_parse_phandle(dev->of_node, "memory-region", 0);
-	if (node)
-		rmem = of_reserved_mem_lookup(node);
-	of_node_put(node);
-
-	if (!rmem) {
+	ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
+	if (ret) {
 		dev_err(dev, "unable to acquire memory-region\n");
-		return -EINVAL;
+		return ret;
 	}
 
-	wcss->mem_phys = rmem->base;
-	wcss->mem_reloc = rmem->base;
-	wcss->mem_size = rmem->size;
-	wcss->mem_region = devm_ioremap_wc(dev, wcss->mem_phys, wcss->mem_size);
+	wcss->mem_phys = res.start;
+	wcss->mem_reloc = res.start;
+	wcss->mem_size = resource_size(&res);
+	wcss->mem_region = devm_ioremap_resource_wc(dev, &res);
 	if (!wcss->mem_region) {
-		dev_err(dev, "unable to map memory region: %pa+%pa\n",
-			&rmem->base, &rmem->size);
+		dev_err(dev, "unable to map memory region: %pR\n", &res);
 		return -EBUSY;
 	}
 
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);
 	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;
 	}
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
       [not found]   ` <CGME20251127142839eucas1p186846c6c1ea1d9e43369fbba9bb5d17c@eucas1p1.samsung.com>
@ 2025-11-27 14:28     ` Marek Szyprowski
  2025-12-02 14:15       ` Rob Herring
  0 siblings, 1 reply; 5+ messages in thread
From: Marek Szyprowski @ 2025-11-27 14:28 UTC (permalink / raw)
  To: Rob Herring (Arm), Bjorn Andersson, Mathieu Poirier, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Geert Uytterhoeven, Magnus Damm, Patrice Chotard, Maxime Coquelin,
	Alexandre Torgue
  Cc: Arnaud Pouliquen, Peng Fan, Beleswar Padhi, linux-remoteproc, imx,
	linux-arm-kernel, linux-kernel, linux-renesas-soc, linux-stm32,
	linux-arm-msm

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
  2025-11-27 14:28     ` Marek Szyprowski
@ 2025-12-02 14:15       ` Rob Herring
  2025-12-08 12:42         ` Marek Szyprowski
  2025-12-09 14:20         ` Daniel Baluta
  0 siblings, 2 replies; 5+ messages in thread
From: Rob Herring @ 2025-12-02 14:15 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Geert Uytterhoeven,
	Magnus Damm, Patrice Chotard, Maxime Coquelin, Alexandre Torgue,
	Arnaud Pouliquen, Peng Fan, Beleswar Padhi, linux-remoteproc, imx,
	linux-arm-kernel, linux-kernel, linux-renesas-soc, linux-stm32,
	linux-arm-msm

On Thu, Nov 27, 2025 at 8:28 AM Marek Szyprowski
<m.szyprowski@samsung.com> wrote:
>
> 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.

Other platforms (non-QCom) were tested also use
devm_ioremap_resource_wc(). So something else is claiming the same
region? Can you dump out /proc/iomem?

The region is dynamically allocated, so maybe that has something to do with it.

Rob

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
  2025-12-02 14:15       ` Rob Herring
@ 2025-12-08 12:42         ` Marek Szyprowski
  2025-12-09 14:20         ` Daniel Baluta
  1 sibling, 0 replies; 5+ messages in thread
From: Marek Szyprowski @ 2025-12-08 12:42 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bjorn Andersson, Mathieu Poirier, Shawn Guo, Sascha Hauer,
	Pengutronix Kernel Team, Fabio Estevam, Geert Uytterhoeven,
	Magnus Damm, Patrice Chotard, Maxime Coquelin, Alexandre Torgue,
	Arnaud Pouliquen, Peng Fan, Beleswar Padhi, linux-remoteproc, imx,
	linux-arm-kernel, linux-kernel, linux-renesas-soc, linux-stm32,
	linux-arm-msm

On 02.12.2025 15:15, Rob Herring wrote:
> On Thu, Nov 27, 2025 at 8:28 AM Marek Szyprowski
> <m.szyprowski@samsung.com> wrote:
>> 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.
> Other platforms (non-QCom) were tested also use
> devm_ioremap_resource_wc(). So something else is claiming the same
> region? Can you dump out /proc/iomem?
>
> The region is dynamically allocated, so maybe that has something to do with it.

# dmesg | grep mem
OF: reserved mem: 0x000000008e200000..0x000000008e7fffff (6144 KiB) 
nomap non-reusable wcnss
OF: reserved mem: 0x000000008dd00000..0x000000008e1fffff (5120 KiB) 
nomap non-reusable venus
OF: reserved mem: 0x000000008dc00000..0x000000008dcfffff (1024 KiB) 
nomap non-reusable mba
OF: reserved mem: 0x0000000086000000..0x00000000862fffff (3072 KiB) 
nomap non-reusable tz-apps@86000000
OF: reserved mem: 0x0000000086300000..0x00000000863fffff (1024 KiB) 
nomap non-reusable smem@86300000
OF: reserved mem: 0x0000000086400000..0x00000000864fffff (1024 KiB) 
nomap non-reusable hypervisor@86400000
OF: reserved mem: 0x0000000086500000..0x000000008667ffff (1536 KiB) 
nomap non-reusable tz@86500000
OF: reserved mem: 0x0000000086680000..0x00000000866fffff (512 KiB) nomap 
non-reusable reserved@86680000
OF: reserved mem: 0x0000000086700000..0x00000000867dffff (896 KiB) nomap 
non-reusable rmtfs@86700000
OF: reserved mem: 0x00000000867e0000..0x00000000867fffff (128 KiB) nomap 
non-reusable rfsa@867e0000
OF: reserved mem: 0x0000000086800000..0x00000000892fffff (44032 KiB) 
nomap non-reusable mpss@86800000
OF: reserved mem: 0x00000000bff00000..0x00000000bfffffff (1024 KiB) map 
non-reusable ramoops@bff00000
NUMA: Faking a node at [mem 0x0000000080000000-0x00000000bd9fffff]
NODE_DATA(0) allocated [mem 0xbd7c6800-0xbd7c943f]
   DMA      [mem 0x0000000080000000-0x00000000bd9fffff]
Early memory node ranges
   node   0: [mem 0x0000000080000000-0x0000000085ffffff]
   node   0: [mem 0x0000000086000000-0x00000000892fffff]
   node   0: [mem 0x0000000089300000-0x000000008dbfffff]
   node   0: [mem 0x000000008dc00000-0x000000008e7fffff]
   node   0: [mem 0x000000008e800000-0x00000000bd9fffff]

...


# cat /proc/iomem

...
80000000-85ffffff : System RAM
  80000000-821affff : Kernel code
  821b0000-82e5ffff : reserved
  82e60000-840fffff : Kernel data
  85000000-85013fff : reserved
86000000-892fffff : reserved
89300000-8dbfffff : System RAM
8dc00000-8e7fffff : reserved
  8dc00000-8e7fffff : reserved
8e800000-bd9fffff : System RAM
  b1000000-b6ffffff : reserved
  b7000000-bbe08fff : reserved
  bc480000-bd5fffff : reserved
  bd621000-bd623fff : reserved
  bd624000-bd724fff : reserved
  bd725000-bd7b0fff : reserved
  bd7b2000-bd7b4fff : reserved
  bd7b5000-bd7b5fff : reserved
  bd7b6000-bd7c9fff : reserved
  bd7ca000-bd9fffff : reserved


The devm_ioremap_resource_wc() requested region (0x8e200000-0x8e7fffff) 
is marked as 'reserved' in /proc/iomem.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region"
  2025-12-02 14:15       ` Rob Herring
  2025-12-08 12:42         ` Marek Szyprowski
@ 2025-12-09 14:20         ` Daniel Baluta
  1 sibling, 0 replies; 5+ messages in thread
From: Daniel Baluta @ 2025-12-09 14:20 UTC (permalink / raw)
  To: Rob Herring
  Cc: Marek Szyprowski, Bjorn Andersson, Mathieu Poirier, Shawn Guo,
	Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam,
	Geert Uytterhoeven, Magnus Damm, Patrice Chotard, Maxime Coquelin,
	Alexandre Torgue, Arnaud Pouliquen, Peng Fan, Beleswar Padhi,
	linux-remoteproc, imx, linux-arm-kernel, linux-kernel,
	linux-renesas-soc, linux-stm32, linux-arm-msm

> Other platforms (non-QCom) were tested also use
> devm_ioremap_resource_wc(). So something else is claiming the same
> region? Can you dump out /proc/iomem?
>
> The region is dynamically allocated, so maybe that has something to do with it.

We noticed a related issue with imx_dsp_rproc.

Because:
imx_dsp_rproc_prepare:
 -> imx_dsp_rproc_add_carveout
     -> /*... */ and this calls devm_ioremap_resource_wc
-> pm_runtime_get_sync

imx_dsp_rproc_unprepare:
 ->pm_runtime_put_sync

There is no easy way to manually undo devm_ioremap_resource_wc so I have
sent a patch to use devm_ioremap_wc.

https://lore.kernel.org/imx/20251209140425.766742-1-daniel.baluta@nxp.com/T/#u

In your case Marek at least you need to understand which driver
reserves    8dc00000-8e7fffff : reserved and why.

thanks,
Daniel.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-12-09 14:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20251124182751.507624-1-robh@kernel.org>
2025-11-24 18:27 ` [PATCH v7 2/2] remoteproc: qcom: Use of_reserved_mem_region_* functions for "memory-region" Rob Herring (Arm)
     [not found]   ` <CGME20251127142839eucas1p186846c6c1ea1d9e43369fbba9bb5d17c@eucas1p1.samsung.com>
2025-11-27 14:28     ` Marek Szyprowski
2025-12-02 14:15       ` Rob Herring
2025-12-08 12:42         ` Marek Szyprowski
2025-12-09 14:20         ` Daniel Baluta

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).