[PATCH 2/2] soc: fsl: qbman: fix unconditional WARN_ON() on Arm during fsl_qman_probe()

Vladimir Oltean vladimir.oltean at nxp.com
Fri Jul 12 02:56:58 PDT 2024


The blamed commit performed a lossy transformation of the original logic.
Due to it, on Arm DPAA1 platforms, we are now faced with this WARN on
each boot:

------------[ cut here ]------------
Unexpected architecture using non shared-dma-mem reservations
WARNING: CPU: 0 PID: 1 at drivers/soc/fsl/qbman/qman_ccsr.c:795 fsl_qman_probe+0x1d0/0x768
pc : fsl_qman_probe+0x1d0/0x768
lr : fsl_qman_probe+0x1b0/0x768
Call trace:
 fsl_qman_probe+0x1d0/0x768
 platform_probe+0xa8/0xd0
 really_probe+0x128/0x2c8

Prior to the refactoring, the logic in fsl_qman_probe() was as follows:

	if (fqd_a) { // previously found using RESERVEDMEM_OF_DECLARE("fsl,qman-fqd") [0]
 #ifdef CONFIG_PPC
		/*
		 * For PPC backward DT compatibility
		 * FQD memory MUST be zero'd by software
		 */
		zero_priv_mem(fqd_a, fqd_sz);
 #else
		WARN(1, "Unexpected architecture using non shared-dma-mem reservations");
 #endif
	} else {
		// Find FQD using new-style device tree bindings [1]
		ret = qbman_init_private_mem(dev, 0, &fqd_a, &fqd_sz);
	}

After the refactoring, the search for the new-style and the old-style
got flipped, and both got absorbed into qbman_init_private_mem().

This creates a problem, because there is no longer a place to put the
"fqd_a != 0" branch within fsl_qman_probe(). The callee,
qbman_init_private_mem(), does not distinguish between FQD, PFDR and
FBPR, and zero_priv_mem() must execute only for FQD.

Split qbman_init_private_mem() into two different functions:
qbman_find_reserved_mem_by_idx() for new-style bindings, and
qbman_find_reserved_mem_by_compatible() for old-style.

Let callers explicitly call both, which permits fsl_qman_probe() to
zero-initialize the FQD memory on PowerPC if it matched on a compatible
string.

[0] Legacy bindings used by PowerPC:

/ {
	reserved-memory {
		qman_fqd: qman-fqd {
			compatible = "fsl,qman-fqd";
			alloc-ranges = <0 0 0x10000 0>;
			size = <0 0x400000>;
			alignment = <0 0x400000>;
		};
	};
};

[1] New bindings:

/ {
	reserved-memory {
		qman_fqd: qman-fqd {
			compatible = "shared-dma-pool";
			size = <0 0x800000>;
			alignment = <0 0x800000>;
			no-map;
		};

		qman_pfdr: qman-pfdr {
			compatible = "shared-dma-pool";
			size = <0 0x2000000>;
			alignment = <0 0x2000000>;
			no-map;
		};
	};

	soc {
		qman: qman at 1880000 {
			compatible = "fsl,qman";
			reg = <0x0 0x1880000 0x0 0x10000>;
			memory-region = <&qman_fqd &qman_pfdr>;
		};
	};
};

Fixes: 3e62273ac63a ("soc: fsl: qbman: Remove RESERVEDMEM_OF_DECLARE usage")
Cc: <stable at vger.kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean at nxp.com>
---
 drivers/soc/fsl/qbman/bman_ccsr.c | 11 ++++--
 drivers/soc/fsl/qbman/dpaa_sys.c  | 62 ++++++++++++++++++++++---------
 drivers/soc/fsl/qbman/dpaa_sys.h  |  7 ++--
 drivers/soc/fsl/qbman/qman_ccsr.c | 40 +++++++++++++-------
 4 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/drivers/soc/fsl/qbman/bman_ccsr.c b/drivers/soc/fsl/qbman/bman_ccsr.c
index b0f26f6f731e..d8a440a265c5 100644
--- a/drivers/soc/fsl/qbman/bman_ccsr.c
+++ b/drivers/soc/fsl/qbman/bman_ccsr.c
@@ -231,11 +231,14 @@ static int fsl_bman_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	ret = qbman_init_private_mem(dev, 0, "fsl,bman-fbpr", &fbpr_a, &fbpr_sz);
+	ret = qbman_find_reserved_mem_by_idx(dev, 0, &fbpr_a, &fbpr_sz);
+	if (ret)
+		ret = qbman_find_reserved_mem_by_compatible(dev, "fsl,bman-fbpr",
+							    &fbpr_a, &fbpr_sz);
 	if (ret) {
-		dev_err(dev, "qbman_init_private_mem() failed 0x%x\n",
-			ret);
-		return -ENODEV;
+		dev_err(dev, "Failed to find FBPR reserved-memory region: %pe\n",
+			ERR_PTR(ret));
+		return ret;
 	}
 
 	dev_dbg(dev, "Allocated FBPR 0x%llx 0x%zx\n", fbpr_a, fbpr_sz);
diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c
index b1cee145cbd7..7c775c4c8a21 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.c
+++ b/drivers/soc/fsl/qbman/dpaa_sys.c
@@ -31,28 +31,14 @@
 #include <linux/dma-mapping.h>
 #include "dpaa_sys.h"
 
-/*
- * Initialize a devices private memory region
- */
-int qbman_init_private_mem(struct device *dev, int idx, const char *compat,
-			   dma_addr_t *addr, size_t *size)
+static int qbman_reserved_mem_lookup(struct device_node *mem_node,
+				     dma_addr_t *addr, size_t *size)
 {
-	struct device_node *mem_node;
 	struct reserved_mem *rmem;
 
-	mem_node = of_parse_phandle(dev->of_node, "memory-region", idx);
-	if (!mem_node) {
-		mem_node = of_find_compatible_node(NULL, NULL, compat);
-		if (!mem_node) {
-			dev_err(dev, "No memory-region found for index %d or compatible '%s'\n",
-				idx, compat);
-			return -ENODEV;
-		}
-	}
-
 	rmem = of_reserved_mem_lookup(mem_node);
 	if (!rmem) {
-		dev_err(dev, "of_reserved_mem_lookup() returned NULL\n");
+		pr_err("of_reserved_mem_lookup(%pOF) returned NULL\n", mem_node);
 		return -ENODEV;
 	}
 	*addr = rmem->base;
@@ -60,3 +46,45 @@ int qbman_init_private_mem(struct device *dev, int idx, const char *compat,
 
 	return 0;
 }
+
+/**
+ * qbman_find_reserved_mem_by_idx() - Find QBMan reserved-memory node
+ * @dev: Pointer to QMan or BMan device structure
+ * @idx: for BMan, pass 0 for the FBPR region.
+ *	 for QMan, pass 0 for the FQD region and 1 for the PFDR region.
+ * @addr: Pointer to storage for the region's base address.
+ * @size: Pointer to storage for the region's size.
+ */
+int qbman_find_reserved_mem_by_idx(struct device *dev, int idx,
+				   dma_addr_t *addr, size_t *size)
+{
+	struct device_node *mem_node;
+
+	mem_node = of_parse_phandle(dev->of_node, "memory-region", idx);
+	if (!mem_node)
+		return -ENODEV;
+
+	return qbman_reserved_mem_lookup(mem_node, addr, size);
+}
+
+/**
+ * qbman_find_reserved_mem_by_compatible() - Find QBMan reserved-memory node (PowerPC)
+ * @dev: Pointer to QMan or BMan device structure
+ * @compat: one of "fsl,bman-fbpr", "fsl,qman-fqd" or "fsl,qman-pfdr"
+ * @addr: Pointer to storage for the region's base address.
+ * @size: Pointer to storage for the region's size.
+ *
+ * This is a legacy variant of qbman_find_reserved_mem_by_idx(), which should
+ * only be used for backwards compatibility with certain PowerPC device trees.
+ */
+int qbman_find_reserved_mem_by_compatible(struct device *dev, const char *compat,
+					  dma_addr_t *addr, size_t *size)
+{
+	struct device_node *mem_node;
+
+	mem_node = of_find_compatible_node(NULL, NULL, compat);
+	if (!mem_node)
+		return -ENODEV;
+
+	return qbman_reserved_mem_lookup(mem_node, addr, size);
+}
diff --git a/drivers/soc/fsl/qbman/dpaa_sys.h b/drivers/soc/fsl/qbman/dpaa_sys.h
index 16485bde9636..1c80244b34d1 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.h
+++ b/drivers/soc/fsl/qbman/dpaa_sys.h
@@ -100,9 +100,10 @@ static inline u8 dpaa_cyc_diff(u8 ringsize, u8 first, u8 last)
 /* Offset applied to genalloc pools due to zero being an error return */
 #define DPAA_GENALLOC_OFF	0x80000000
 
-/* Initialize the devices private memory region */
-int qbman_init_private_mem(struct device *dev, int idx, const char *compat,
-			   dma_addr_t *addr, size_t *size);
+int qbman_find_reserved_mem_by_idx(struct device *dev, int idx,
+				   dma_addr_t *addr, size_t *size);
+int qbman_find_reserved_mem_by_compatible(struct device *dev, const char *compat,
+					  dma_addr_t *addr, size_t *size);
 
 /* memremap() attributes for different platforms */
 #ifdef CONFIG_PPC
diff --git a/drivers/soc/fsl/qbman/qman_ccsr.c b/drivers/soc/fsl/qbman/qman_ccsr.c
index 392e54f14dbe..4735b450d97e 100644
--- a/drivers/soc/fsl/qbman/qman_ccsr.c
+++ b/drivers/soc/fsl/qbman/qman_ccsr.c
@@ -731,6 +731,7 @@ static int fsl_qman_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct device_node *node = dev->of_node;
+	bool fqd_found_by_compatible = false;
 	struct resource *res;
 	int ret, err_irq;
 	u16 id;
@@ -779,29 +780,40 @@ static int fsl_qman_probe(struct platform_device *pdev)
 	* in order to ensure allocations from the correct regions the
 	* driver initializes then allocates each piece in order
 	*/
-	ret = qbman_init_private_mem(dev, 0, "fsl,qman-fqd", &fqd_a, &fqd_sz);
+	ret = qbman_find_reserved_mem_by_idx(dev, 0, &fqd_a, &fqd_sz);
 	if (ret) {
-		dev_err(dev, "qbman_init_private_mem() for FQD failed 0x%x\n",
-			ret);
-		return -ENODEV;
+		ret = qbman_find_reserved_mem_by_compatible(dev, "fsl,qman-fqd",
+							    &fqd_a, &fqd_sz);
+		if (ret == 0)
+			fqd_found_by_compatible = true;
 	}
+	if (ret) {
+		dev_err(dev, "Failed to find FQD reserved-memory region: %pe\n",
+			ERR_PTR(ret));
+		return ret;
+	}
+	if (fqd_found_by_compatible) {
 #ifdef CONFIG_PPC
-	/*
-	 * For PPC backward DT compatibility
-	 * FQD memory MUST be zero'd by software
-	 */
-	zero_priv_mem(fqd_a, fqd_sz);
+		/*
+		 * For PPC backward DT compatibility
+		 * FQD memory MUST be zero'd by software
+		 */
+		zero_priv_mem(fqd_a, fqd_sz);
 #else
-	WARN(1, "Unexpected architecture using non shared-dma-mem reservations");
+		WARN(1, "Unexpected architecture using non shared-dma-mem reservations");
 #endif
+	}
 	dev_dbg(dev, "Allocated FQD 0x%llx 0x%zx\n", fqd_a, fqd_sz);
 
 	/* Setup PFDR memory */
-	ret = qbman_init_private_mem(dev, 1, "fsl,qman-pfdr", &pfdr_a, &pfdr_sz);
+	ret = qbman_find_reserved_mem_by_idx(dev, 1, &pfdr_a, &pfdr_sz);
+	if (ret)
+		ret = qbman_find_reserved_mem_by_compatible(dev, "fsl,qman-pfdr",
+							    &pfdr_a, &pfdr_sz);
 	if (ret) {
-		dev_err(dev, "qbman_init_private_mem() for PFDR failed 0x%x\n",
-			ret);
-		return -ENODEV;
+		dev_err(dev, "Failed to find PFDR reserved-memory region: %pe\n",
+			ERR_PTR(ret));
+		return ret;
 	}
 	dev_dbg(dev, "Allocated PFDR 0x%llx 0x%zx\n", pfdr_a, pfdr_sz);
 
-- 
2.34.1




More information about the linux-arm-kernel mailing list