[PATCH 1/2] soc: fsl: qbman: delete bogus device tree fixup in qbman_init_private_mem()

[PATCH 1/2] soc: fsl: qbman: delete bogus device tree fixup in qbman_init_private_mem()

[Vladimir Oltean]

This is effectively a revert of commit 6ea4c0fe4570 ("soc/fsl/qbman:
Update device tree with reserved memory").

What that commit intended to do: Fix up the device tree that is passed
to a subsequent kexec-loaded kernel, so that the reserved-memory nodes
have the same base addresses as the currently running kernel.

What that commit actually does: Fix up the running device tree, which
has no effect whatsoever upon the device tree passed to the next kernel.

I would have refrained from making this kind of non-bugfix change in
stable kernels, but qbman_init_private_mem() grossly misrepresents
what this function does, and for an actual upcoming bug fix, it needs to
be refactored. There is no place for the bogus code afterwards, so it
needs to go as part of that, sadly.

Cc: <stable@vger.kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 drivers/soc/fsl/qbman/dpaa_sys.c | 31 -------------------------------
 1 file changed, 31 deletions(-)

diff --git a/drivers/soc/fsl/qbman/dpaa_sys.c b/drivers/soc/fsl/qbman/dpaa_sys.c
index e1d7b79cc450..b1cee145cbd7 100644
--- a/drivers/soc/fsl/qbman/dpaa_sys.c
+++ b/drivers/soc/fsl/qbman/dpaa_sys.c
@@ -39,8 +39,6 @@ int qbman_init_private_mem(struct device *dev, int idx, const char *compat,
 {
 	struct device_node *mem_node;
 	struct reserved_mem *rmem;
-	int err;
-	__be32 *res_array;
 
 	mem_node = of_parse_phandle(dev->of_node, "memory-region", idx);
 	if (!mem_node) {
@@ -60,34 +58,5 @@ int qbman_init_private_mem(struct device *dev, int idx, const char *compat,
 	*addr = rmem->base;
 	*size = rmem->size;
 
-	/*
-	 * Check if the reg property exists - if not insert the node
-	 * so upon kexec() the same memory region address will be preserved.
-	 * This is needed because QBMan HW does not allow the base address/
-	 * size to be modified once set.
-	 */
-	if (!of_property_present(mem_node, "reg")) {
-		struct property *prop;
-
-		prop = devm_kzalloc(dev, sizeof(*prop), GFP_KERNEL);
-		if (!prop)
-			return -ENOMEM;
-		prop->value = res_array = devm_kzalloc(dev, sizeof(__be32) * 4,
-						       GFP_KERNEL);
-		if (!prop->value)
-			return -ENOMEM;
-		res_array[0] = cpu_to_be32(upper_32_bits(*addr));
-		res_array[1] = cpu_to_be32(lower_32_bits(*addr));
-		res_array[2] = cpu_to_be32(upper_32_bits(*size));
-		res_array[3] = cpu_to_be32(lower_32_bits(*size));
-		prop->length = sizeof(__be32) * 4;
-		prop->name = devm_kstrdup(dev, "reg", GFP_KERNEL);
-		if (!prop->name)
-			return -ENOMEM;
-		err = of_add_property(mem_node, prop);
-		if (err)
-			return err;
-	}
-
 	return 0;
 }
-- 
2.34.1

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

[Vladimir Oltean]

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@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@vger.kernel.org>
Signed-off-by: Vladimir Oltean <vladimir.oltean@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