Skip to contentSkip to conversation

remoteproc: add AMD BRAM-based remote processor driver

PATCH Superseded Ben Levinsky 4 participants 9 messages Apr 28 – Jun 29

Subsystems (2)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v3 cover remoteproc: add AMD BRAM-based remote processor driver

Add a BRAM-based remoteproc driver and corresponding binding for AMD soft processors located in programmable logic.

v3:
This version updates the binding to use SoC-specific compatibles with the fallback form discussed on the thread.

Patch 1, dt-bindings: remoteproc: document AMD BRAM-based rproc

- Reworked the compatible schema to use SoC-specific compatibles.
- Added amd,versal2-bram-rproc to the supported compatible list.
- Used xlnx,zynqmp-bram-rproc as the fallback compatible.
- Updated the example to match the new compatible scheme.

Patch 2, remoteproc: add AMD BRAM-based remote processor driver

- Updated the driver OF match table to bind via the xlnx,zynqmp-bram-rproc fallback compatible.

v2:
This version pivots the series away from a MicroBlaze-specific binding and driver shape and instead models a BRAM-based soft-core processor subsystem more generally.

This follows the upstream feedback that amd,microblaze was too tied to the processor architecture while also being too generic as a DT compatible for the hardware interface being described.

Patch 1, dt-bindings: remoteproc: document AMD BRAM-based rproc

- Renamed the binding away from amd,microblaze and reframed it around a BRAM-based soft-core processor subsystem.
- Dropped the redundant trailing "binding" wording from the patch subject.
- Rewrote the binding text to describe the hardware rather than the Linux remoteproc framework.
- Reworked the example to address the original dt_binding_check complaints about the root node and simple-pm-bus example shape.
- Added a clocks property for the soft-core subsystem.

Patch 2, remoteproc: add AMD BRAM-based remote processor driver

- Renamed the driver away from the MicroBlaze-specific name to match the BRAM-based binding.
- Added clock handling for the soft-core subsystem and the matching COMMON_CLK dependency in Kconfig.
- Cleaned up the reset comments and removed the success dev_dbg() message called out in review.

Ben Levinsky (2):
dt-bindings: remoteproc: document AMD BRAM-based rproc
remoteproc: add AMD BRAM-based remote processor driver

 .../bindings/remoteproc/amd,bram-rproc.yaml   | 105 ++++++++
 MAINTAINERS                                   |   7 +
 drivers/remoteproc/Kconfig                    |  14 +
 drivers/remoteproc/Makefile                   |   1 +
 drivers/remoteproc/amd_bram_rproc.c           | 243 ++++++++++++++++++
 5 files changed, 370 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
Signature · 1 line
2.34.1
#1 lore raw

PATCH v3 1/2 dt-bindings: remoteproc: document AMD BRAM-based rproc

Describe an AMD BRAM-based remote processor controlled through the remoteproc framework.

The binding models a soft-core processor subsystem instantiated in AMD programmable logic and using dual-port BRAM for firmware storage and execution. The remoteproc device is represented as a child node whose reg property describes the firmware memory window in the processor-local address space. The parent bus node provides standard devicetree address translation through ranges so Linux can access the same BRAM through the system physical address space.

A clock input feeds the soft-core processor subsystem, and an active-low reset GPIO holds the processor in reset until firmware loading completes. The firmware-name property is optional.

---
 .../bindings/remoteproc/amd,bram-rproc.yaml   | 105 ++++++++++++++++++
 1 file changed, 105 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml new file +105 -0
new file mode 100644
index 000000000000..cff13d552321
@@ -0,0 +1,105 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/remoteproc/amd,bram-rproc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: AMD BRAM-based Remote Processor
+
+maintainers:
+  - Ben Levinsky <[email protected]>
+
+description: |
+  Soft-core processor subsystem instantiated in AMD programmable logic and
+  using dual-port BRAM for firmware storage and execution.
+
+  Hardware Architecture:
+
+    Host (PS)                        Programmable Logic (PL)
+    =========                        ======================
+
+    AXI Interface -----------------> AXI BRAM Controller (Host Port)
+                                             |
+                                             | Port A
+                                             v
+                                     +-----------------+
+                                     |  Dual-Port BRAM |
+                                     | (shared memory) |
+                                     +-----------------+
+                                             ^
+                                             | Port B
+                                             |
+                                     AXI BRAM Controller (Soft-core Port)
+                                             ^
+                                             | LMB
+                                             |
+                                     Soft-core CPU (MicroBlaze/V)
+
+    GPIO --------------------------> Proc Sys Reset ----> CPU Reset Signal
+
+    Clock -------------------------> Clock Distribution -> CPU Clock
+
+  Memory Architecture:
+
+    The dual-port BRAM allows simultaneous access from both processors:
+      - Port A: Connected to the host AXI BRAM controller for firmware loading
+      - Port B: Connected to the soft-core local memory bus for execution
+
+  The reg property describes the executable BRAM window in the processor-local
+  address space. The parent bus node translates that window to the system
+  physical address space by using standard devicetree address translation
+  through ranges. A clock input and a reset GPIO control the subsystem.
+
+properties:
+  compatible:
+    oneOf:
+      - const: xlnx,zynqmp-bram-rproc
+      - items:
+          - enum:
+              - xlnx,versal-bram-rproc
+              - xlnx,versal-net-bram-rproc
+              - amd,versal2-bram-rproc
+          - const: xlnx,zynqmp-bram-rproc
+
+  reg:
+    maxItems: 1
+    description:
+      Processor-local address and size of the BRAM firmware memory window,
+      as seen by the soft-core processor (typically 0x0 for reset vector).
+      The parent bus ranges property must translate this window to the
+      corresponding system physical address.
+
+  clocks:
+    maxItems: 1
+    description:
+      Clock input for the soft-core processor subsystem.
+
+  firmware-name:
+    maxItems: 1
+    description:
+      Name of the firmware ELF file to load.
+
+  reset-gpios:
+    maxItems: 1
+    description:
+      GPIO specifier controlling the soft-core reset input.
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - reset-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    remoteproc@0 {
+      compatible = "xlnx,zynqmp-bram-rproc";
+      reg = <0x0 0x40000>;
+      clocks = <&pl_clk>;
+      firmware-name = "firmware.elf";
+      reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+    };
+...
Signature · 1 line
2.34.1
Reviewed-by on v3 1/2
#2 lore raw

Re: [PATCH v3 1/2] dt-bindings: remoteproc: document AMD BRAM-based rproc

Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml line 61

Quoted 4 lines · Ben Levinsky
Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml new file +4 -0
+          - enum:
+              - xlnx,versal-bram-rproc
+              - xlnx,versal-net-bram-rproc
+              - amd,versal2-bram-rproc

If there is going to be a new version, this should be sorted by name.

Best regards,
Krzysztof

#3 lore raw

PATCH v3 2/2 remoteproc: add AMD BRAM-based remote processor driver

In reply to Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +7 -0
index c871acf2179c..172539971950 100644
@@ -1037,6 +1037,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:	drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:	Ben Levinsky <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:	drivers/remoteproc/amd_bram_rproc.c
+
 AMD CDX BUS DRIVER
 M:	Nipun Gupta <[email protected]>
 M:	Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +14 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
 	  It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC
+	tristate "AMD BRAM-based remoteproc support"
+	depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+	help
+	  Say y or m here to support a BRAM-based remote processor managed
+	  through the remoteproc framework.
+
+	  This driver matches designs where executable firmware memory is
+	  described in the BRAM-local address space and translated to
+	  the system physical address space with standard devicetree address
+	  translation.
+
+	  If unsure, say N.
+
 config IMX_REMOTEPROC
 	tristate "i.MX remoteproc support"
 	depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)	+= amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)	+= imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +243 -0

243 lines not on this page · raw message

Signature · 1 line
2.34.1
maintainer
#4 lore raw

Good morning,

Quoted 35 lines · Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +0 -0
index c871acf2179c..172539971950 100644

MAINTAINERS S: Maintained · line 1046

Quoted 10 lines
MAINTAINERS +7 -0
@@ -1037,6 +1037,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:	drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:	Ben Levinsky <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:	drivers/remoteproc/amd_bram_rproc.c
+

There is no real advantage in adding this entry, checkpatch.pl should be sufficient.

Quoted 12 lines
MAINTAINERS +0 -0
 AMD CDX BUS DRIVER
 M:	Nipun Gupta <[email protected]>
 M:	Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +1 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
 	  It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC

drivers/remoteproc/Kconfig config REMOTEPROC_CDEV · line 36

Quoted 10 lines
drivers/remoteproc/Kconfig +10 -0
+	tristate "AMD BRAM-based remoteproc support"
+	depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+	help
+	  Say y or m here to support a BRAM-based remote processor managed
+	  through the remoteproc framework.
+
+	  This driver matches designs where executable firmware memory is
+	  described in the BRAM-local address space and translated to
+	  the system physical address space with standard devicetree address
+	  translation.

Not sure how this paragraph helps decide whether the driver should be enabled or not. Please remove.

Quoted 77 lines
drivers/remoteproc/Kconfig +3 -0
+
+	  If unsure, say N.
+
 config IMX_REMOTEPROC
 	tristate "i.MX remoteproc support"
 	depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)	+= amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)	+= imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +53 -0
new file mode 100644
index 000000000000..9383964b6046
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD BRAM-based Remote Processor driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver supports soft-core processors (MicroBlaze, MicroBlaze-V, or
+ * similar) instantiated in AMD programmable logic, using dual-port BRAM
+ * for firmware storage and execution.
+ *
+ * The firmware memory (BRAM) is described in the processor-local address
+ * space and translated to the Linux-visible system physical address with
+ * standard devicetree address translation.
+ *
+ * Reset is controlled via GPIO connected to Processor System Reset IP.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/**
+ * struct amd_bram_rproc - AMD BRAM-based remoteproc private data
+ * @dev: device pointer
+ * @reset: GPIO descriptor for reset control (active-low)
+ * @clk: processor clock
+ */
+struct amd_bram_rproc {
+	struct device *dev;
+	struct gpio_desc *reset;
+	struct clk *clk;
+};
+
+static int amd_bram_rproc_mem_map(struct rproc *rproc,
+				  struct rproc_mem_entry *mem)
+{
+	void __iomem *va;
+
+	va = ioremap_wc(mem->dma, mem->len);
+	if (!va)
+		return -ENOMEM;
+
+	mem->va = (__force void *)va;
+	mem->is_iomem = true;
+

drivers/remoteproc/amd_bram_rproc.c line 63

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	return 0;
+}
+
+static int amd_bram_rproc_mem_unmap(struct rproc *rproc,
+				    struct rproc_mem_entry *mem)
+{
+	iounmap((void __iomem *)mem->va);
+
+	return 0;
+}

The above 2 are identical to what is found in xlnx_r5_remoteproc.c. Please coordinate with Tanmay to split that into common code that can be reused by both drivers.

Quoted 31 lines
drivers/remoteproc/amd_bram_rproc.c new file +31 -0
+
+static int amd_bram_rproc_prepare(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	struct rproc_mem_entry *mem;
+	struct resource res;
+	u64 da, size;
+	int ret;
+
+	ret = of_property_read_reg(priv->dev->of_node, 0, &da, &size);
+	if (ret) {
+		dev_err(priv->dev, "failed to parse executable memory reg\n");
+		return ret;
+	}
+
+	if (!size || size > U32_MAX) {
+		dev_err(priv->dev, "invalid executable memory size\n");
+		return -EINVAL;
+	}
+
+	if (da > U32_MAX) {
+		dev_err(priv->dev, "invalid executable memory address\n");
+		return -EINVAL;
+	}
+
+	ret = of_address_to_resource(priv->dev->of_node, 0, &res);
+	if (ret) {
+		dev_err(priv->dev, "failed to translate executable memory reg\n");
+		return ret;
+	}
+

drivers/remoteproc/amd_bram_rproc.c line 104

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)res.start,
+				   (size_t)size, da,
+				   amd_bram_rproc_mem_map,
+				   amd_bram_rproc_mem_unmap,
+				   dev_name(priv->dev));
+	if (!mem)
+		return -ENOMEM;
+
+	rproc_add_carveout(rproc, mem);
+	rproc_coredump_add_segment(rproc, da, (size_t)size);

I'm pretty sure you want @res.start instead of @da, and resource_size(&res) instead of @size.

Quoted 48 lines
drivers/remoteproc/amd_bram_rproc.c new file +48 -0
+
+	return 0;
+}
+
+static int amd_bram_rproc_start(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	int ret;
+
+	/* Enable clock before releasing reset */
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->dev, "failed to enable clock: %d\n", ret);
+		return ret;
+	}
+
+	/* Deassert reset and let the processor run. */
+	ret = gpiod_set_value_cansleep(priv->reset, 0);
+	if (ret) {
+		dev_err(priv->dev, "failed to deassert reset: %d\n", ret);
+		clk_disable_unprepare(priv->clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int amd_bram_rproc_stop(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	int ret;
+
+	/* Assert reset before disabling the processor clock. */
+	ret = gpiod_set_value_cansleep(priv->reset, 1);
+	if (ret) {
+		dev_err(priv->dev, "failed to assert reset: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable clock after asserting reset */
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static int amd_bram_rproc_parse_fw(struct rproc *rproc,
+				   const struct firmware *fw)
+{

drivers/remoteproc/amd_bram_rproc.c line 162

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	int ret;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret == -EINVAL) {
+		dev_dbg(&rproc->dev, "no resource table found\n");
+		return 0;
+	}
+
+	return ret;
+}

This too should go in common code or simply replaced by rproc_elf_load_rsc_table() in @amd_bram_rproc_ops - the choice is yours.

Thanks,
Mathieu

Quoted 84 lines
drivers/remoteproc/amd_bram_rproc.c new file +81 -0
+
+static const struct rproc_ops amd_bram_rproc_ops = {
+	.prepare	= amd_bram_rproc_prepare,
+	.start		= amd_bram_rproc_start,
+	.stop		= amd_bram_rproc_stop,
+	.load		= rproc_elf_load_segments,
+	.sanity_check	= rproc_elf_sanity_check,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+	.parse_fw	= amd_bram_rproc_parse_fw,
+};
+
+static int amd_bram_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct amd_bram_rproc *priv;
+	const char *fw_name = NULL;
+	struct rproc *rproc;
+	int ret;
+
+	ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+	if (ret < 0 && ret != -EINVAL)
+		return dev_err_probe(dev, ret,
+				     "failed to parse firmware-name property\n");
+
+	rproc = devm_rproc_alloc(dev, dev_name(dev), &amd_bram_rproc_ops,
+				 fw_name, sizeof(*priv));
+	if (!rproc)
+		return -ENOMEM;
+
+	priv = rproc->priv;
+	priv->dev = dev;
+
+	/* Get the processor clock */
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk))
+		return dev_err_probe(dev, PTR_ERR(priv->clk),
+				     "failed to get clock\n");
+
+	/*
+	 * Keep the processor in reset until remoteproc has finished loading
+	 * firmware into the executable memory window described by reg and
+	 * translated through the parent bus ranges property.
+	 */
+	priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset))
+		return dev_err_probe(dev, PTR_ERR(priv->reset),
+				     "failed to get reset gpio\n");
+
+	rproc->auto_boot = false;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+	platform_set_drvdata(pdev, rproc);
+
+	ret = devm_rproc_add(dev, rproc);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register rproc\n");
+
+	return 0;
+}
+
+static const struct of_device_id amd_bram_rproc_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-bram-rproc" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, amd_bram_rproc_of_match);
+
+static struct platform_driver amd_bram_rproc_driver = {
+	.probe = amd_bram_rproc_probe,
+	.driver = {
+		.name = "amd-bram-rproc",
+		.of_match_table = amd_bram_rproc_of_match,
+	},
+};
+module_platform_driver(amd_bram_rproc_driver);
+
+MODULE_DESCRIPTION("AMD BRAM-based Remote Processor driver");
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.34.1
#5 lore raw

Hi Mathieu,

Quoted 277 lines · Mathieu Poirier

Good morning,

Quoted 35 lines · Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +0 -0
index c871acf2179c..172539971950 100644

MAINTAINERS S: Maintained · line 1046

Quoted 10 lines
MAINTAINERS +7 -0
@@ -1037,6 +1037,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:	drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:	Ben Levinsky <[email protected]>
+L:	[email protected]
+S:	Maintained
+F:	Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:	drivers/remoteproc/amd_bram_rproc.c
+

There is no real advantage in adding this entry, checkpatch.pl should be sufficient.

Quoted 12 lines
MAINTAINERS +0 -0
 AMD CDX BUS DRIVER
 M:	Nipun Gupta <[email protected]>
 M:	Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +1 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
 	  It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC

drivers/remoteproc/Kconfig config REMOTEPROC_CDEV · line 36

Quoted 10 lines
drivers/remoteproc/Kconfig +10 -0
+	tristate "AMD BRAM-based remoteproc support"
+	depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+	help
+	  Say y or m here to support a BRAM-based remote processor managed
+	  through the remoteproc framework.
+
+	  This driver matches designs where executable firmware memory is
+	  described in the BRAM-local address space and translated to
+	  the system physical address space with standard devicetree address
+	  translation.

Not sure how this paragraph helps decide whether the driver should be enabled or not. Please remove.

Quoted 77 lines
drivers/remoteproc/Kconfig +3 -0
+
+	  If unsure, say N.
+
 config IMX_REMOTEPROC
 	tristate "i.MX remoteproc support"
 	depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y				+= remoteproc_sysfs.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)		+= remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)	+= amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)	+= imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)		+= ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +53 -0
new file mode 100644
index 000000000000..9383964b6046
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD BRAM-based Remote Processor driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver supports soft-core processors (MicroBlaze, MicroBlaze-V, or
+ * similar) instantiated in AMD programmable logic, using dual-port BRAM
+ * for firmware storage and execution.
+ *
+ * The firmware memory (BRAM) is described in the processor-local address
+ * space and translated to the Linux-visible system physical address with
+ * standard devicetree address translation.
+ *
+ * Reset is controlled via GPIO connected to Processor System Reset IP.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/**
+ * struct amd_bram_rproc - AMD BRAM-based remoteproc private data
+ * @dev: device pointer
+ * @reset: GPIO descriptor for reset control (active-low)
+ * @clk: processor clock
+ */
+struct amd_bram_rproc {
+	struct device *dev;
+	struct gpio_desc *reset;
+	struct clk *clk;
+};
+
+static int amd_bram_rproc_mem_map(struct rproc *rproc,
+				  struct rproc_mem_entry *mem)
+{
+	void __iomem *va;
+
+	va = ioremap_wc(mem->dma, mem->len);
+	if (!va)
+		return -ENOMEM;
+
+	mem->va = (__force void *)va;
+	mem->is_iomem = true;
+

drivers/remoteproc/amd_bram_rproc.c line 63

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	return 0;
+}
+
+static int amd_bram_rproc_mem_unmap(struct rproc *rproc,
+				    struct rproc_mem_entry *mem)
+{
+	iounmap((void __iomem *)mem->va);
+
+	return 0;
+}

The above 2 are identical to what is found in xlnx_r5_remoteproc.c. Please coordinate with Tanmay to split that into common code that can be reused by both drivers.

Quoted 31 lines
drivers/remoteproc/amd_bram_rproc.c new file +31 -0
+
+static int amd_bram_rproc_prepare(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	struct rproc_mem_entry *mem;
+	struct resource res;
+	u64 da, size;
+	int ret;
+
+	ret = of_property_read_reg(priv->dev->of_node, 0, &da, &size);
+	if (ret) {
+		dev_err(priv->dev, "failed to parse executable memory reg\n");
+		return ret;
+	}
+
+	if (!size || size > U32_MAX) {
+		dev_err(priv->dev, "invalid executable memory size\n");
+		return -EINVAL;
+	}
+
+	if (da > U32_MAX) {
+		dev_err(priv->dev, "invalid executable memory address\n");
+		return -EINVAL;
+	}
+
+	ret = of_address_to_resource(priv->dev->of_node, 0, &res);
+	if (ret) {
+		dev_err(priv->dev, "failed to translate executable memory reg\n");
+		return ret;
+	}
+

drivers/remoteproc/amd_bram_rproc.c line 104

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)res.start,
+				   (size_t)size, da,
+				   amd_bram_rproc_mem_map,
+				   amd_bram_rproc_mem_unmap,
+				   dev_name(priv->dev));
+	if (!mem)
+		return -ENOMEM;
+
+	rproc_add_carveout(rproc, mem);
+	rproc_coredump_add_segment(rproc, da, (size_t)size);

I'm pretty sure you want @res.start instead of @da, and resource_size(&res) instead of @size.

Quoted 48 lines
drivers/remoteproc/amd_bram_rproc.c new file +48 -0
+
+	return 0;
+}
+
+static int amd_bram_rproc_start(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	int ret;
+
+	/* Enable clock before releasing reset */
+	ret = clk_prepare_enable(priv->clk);
+	if (ret) {
+		dev_err(priv->dev, "failed to enable clock: %d\n", ret);
+		return ret;
+	}
+
+	/* Deassert reset and let the processor run. */
+	ret = gpiod_set_value_cansleep(priv->reset, 0);
+	if (ret) {
+		dev_err(priv->dev, "failed to deassert reset: %d\n", ret);
+		clk_disable_unprepare(priv->clk);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int amd_bram_rproc_stop(struct rproc *rproc)
+{
+	struct amd_bram_rproc *priv = rproc->priv;
+	int ret;
+
+	/* Assert reset before disabling the processor clock. */
+	ret = gpiod_set_value_cansleep(priv->reset, 1);
+	if (ret) {
+		dev_err(priv->dev, "failed to assert reset: %d\n", ret);
+		return ret;
+	}
+
+	/* Disable clock after asserting reset */
+	clk_disable_unprepare(priv->clk);
+
+	return 0;
+}
+
+static int amd_bram_rproc_parse_fw(struct rproc *rproc,
+				   const struct firmware *fw)
+{

drivers/remoteproc/amd_bram_rproc.c line 162

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+	int ret;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret == -EINVAL) {
+		dev_dbg(&rproc->dev, "no resource table found\n");
+		return 0;
+	}
+
+	return ret;
+}

This too should go in common code or simply replaced by rproc_elf_load_rsc_table() in @amd_bram_rproc_ops - the choice is yours.

Quoted 2 lines

Thanks,
Mathieu

Thanks for the review.

I went through the remoteproc drivers to scope the cleanup points you called out.

For the plain carveout map/unmap callbacks, the same ioremap_wc()/iounmap() pattern exists not only in amd_bram_rproc and xlnx_r5_remoteproc, but also in rcar_rproc, st_remoteproc, stm32_rproc, imx_rproc, and imx_dsp_rproc.

The xlnx_r5 TCM path is close as well, but that one still needs a wrapper since it clears the memory after ioremap_wc().

For the optional resource-table parsing, amd_bram_rproc and xlnx_r5_remoteproc share the same pattern of treating only -EINVAL from rproc_elf_load_rsc_table() as non-fatal. PRU is similar, but has additional firmware parsing after that. Other drivers such as rcar/imx/imx_dsp/stm32 also tolerate missing resource tables, but their current behavior is not identical since they flatten all errors to success and only log.

For the next revision, would you prefer the following approach?

1. Add a small common helper for the plain carveout ioremap_wc()/iounmap() case and use it in amd_bram_rproc and xlnx_r5_remoteproc.

2. For the optional resource-table handling, either:
- add a small common helper for the "missing table is OK" case (i.e. return 0 on -EINVAL and propagate other errors), and use that in amd_bram_rproc and xlnx_r5_remoteproc, or
- drop the custom AMD parse_fw() path and use rproc_elf_load_rsc_table() directly, which would make the resource table mandatory there.

Also, for the plain map/unmap helper, should I keep the cleanup scoped to the drivers directly involved here, or would you prefer that I fold the other exact-match users (rcar, st, stm32, imx, imx_dsp) into the same cleanup patch as well?

I want to make sure I take the direction you prefer before respinning.

Thanks,
Ben

Quoted 86 lines
Quoted 85 lines
drivers/remoteproc/amd_bram_rproc.c new file +81 -0
+
+static const struct rproc_ops amd_bram_rproc_ops = {
+	.prepare	= amd_bram_rproc_prepare,
+	.start		= amd_bram_rproc_start,
+	.stop		= amd_bram_rproc_stop,
+	.load		= rproc_elf_load_segments,
+	.sanity_check	= rproc_elf_sanity_check,
+	.get_boot_addr	= rproc_elf_get_boot_addr,
+	.parse_fw	= amd_bram_rproc_parse_fw,
+};
+
+static int amd_bram_rproc_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct amd_bram_rproc *priv;
+	const char *fw_name = NULL;
+	struct rproc *rproc;
+	int ret;
+
+	ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+	if (ret < 0 && ret != -EINVAL)
+		return dev_err_probe(dev, ret,
+				     "failed to parse firmware-name property\n");
+
+	rproc = devm_rproc_alloc(dev, dev_name(dev), &amd_bram_rproc_ops,
+				 fw_name, sizeof(*priv));
+	if (!rproc)
+		return -ENOMEM;
+
+	priv = rproc->priv;
+	priv->dev = dev;
+
+	/* Get the processor clock */
+	priv->clk = devm_clk_get(dev, NULL);
+	if (IS_ERR(priv->clk))
+		return dev_err_probe(dev, PTR_ERR(priv->clk),
+				     "failed to get clock\n");
+
+	/*
+	 * Keep the processor in reset until remoteproc has finished loading
+	 * firmware into the executable memory window described by reg and
+	 * translated through the parent bus ranges property.
+	 */
+	priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+	if (IS_ERR(priv->reset))
+		return dev_err_probe(dev, PTR_ERR(priv->reset),
+				     "failed to get reset gpio\n");
+
+	rproc->auto_boot = false;
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+	platform_set_drvdata(pdev, rproc);
+
+	ret = devm_rproc_add(dev, rproc);
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to register rproc\n");
+
+	return 0;
+}
+
+static const struct of_device_id amd_bram_rproc_of_match[] = {
+	{ .compatible = "xlnx,zynqmp-bram-rproc" },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, amd_bram_rproc_of_match);
+
+static struct platform_driver amd_bram_rproc_driver = {
+	.probe = amd_bram_rproc_probe,
+	.driver = {
+		.name = "amd-bram-rproc",
+		.of_match_table = amd_bram_rproc_of_match,
+	},
+};
+module_platform_driver(amd_bram_rproc_driver);
+
+MODULE_DESCRIPTION("AMD BRAM-based Remote Processor driver");
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL");
Signature · 1 line
2.34.1
maintainer
#6 lore raw
Quoted 306 lines · Ben Levinsky

Hi Mathieu,

Quoted 277 lines · Mathieu Poirier

Good morning,

Quoted 35 lines · Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +0 -0
index c871acf2179c..172539971950 100644

MAINTAINERS S: Maintained · line 1046

Quoted 10 lines
MAINTAINERS +7 -0
@@ -1037,6 +1037,13 @@ S:    Maintained
 F:  Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:  drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:  Ben Levinsky <[email protected]>
+L:  [email protected]
+S:  Maintained
+F:  Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:  drivers/remoteproc/amd_bram_rproc.c
+

There is no real advantage in adding this entry, checkpatch.pl should be sufficient.

Quoted 12 lines
MAINTAINERS +0 -0
 AMD CDX BUS DRIVER
 M:  Nipun Gupta <[email protected]>
 M:  Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +1 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
       It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC

drivers/remoteproc/Kconfig config REMOTEPROC_CDEV · line 36

Quoted 10 lines
drivers/remoteproc/Kconfig +10 -0
+    tristate "AMD BRAM-based remoteproc support"
+    depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+    help
+      Say y or m here to support a BRAM-based remote processor managed
+      through the remoteproc framework.
+
+      This driver matches designs where executable firmware memory is
+      described in the BRAM-local address space and translated to
+      the system physical address space with standard devicetree address
+      translation.

Not sure how this paragraph helps decide whether the driver should be enabled or not. Please remove.

Quoted 77 lines
drivers/remoteproc/Kconfig +3 -0
+
+      If unsure, say N.
+
 config IMX_REMOTEPROC
     tristate "i.MX remoteproc support"
     depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y                               += remoteproc_sysfs.o
 remoteproc-y                                += remoteproc_virtio.o
 remoteproc-y                                += remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)               += remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)   += amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)                += imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)    += imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)             += ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +53 -0
new file mode 100644
index 000000000000..9383964b6046
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD BRAM-based Remote Processor driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver supports soft-core processors (MicroBlaze, MicroBlaze-V, or
+ * similar) instantiated in AMD programmable logic, using dual-port BRAM
+ * for firmware storage and execution.
+ *
+ * The firmware memory (BRAM) is described in the processor-local address
+ * space and translated to the Linux-visible system physical address with
+ * standard devicetree address translation.
+ *
+ * Reset is controlled via GPIO connected to Processor System Reset IP.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/**
+ * struct amd_bram_rproc - AMD BRAM-based remoteproc private data
+ * @dev: device pointer
+ * @reset: GPIO descriptor for reset control (active-low)
+ * @clk: processor clock
+ */
+struct amd_bram_rproc {
+    struct device *dev;
+    struct gpio_desc *reset;
+    struct clk *clk;
+};
+
+static int amd_bram_rproc_mem_map(struct rproc *rproc,
+                              struct rproc_mem_entry *mem)
+{
+    void __iomem *va;
+
+    va = ioremap_wc(mem->dma, mem->len);
+    if (!va)
+            return -ENOMEM;
+
+    mem->va = (__force void *)va;
+    mem->is_iomem = true;
+

drivers/remoteproc/amd_bram_rproc.c line 63

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    return 0;
+}
+
+static int amd_bram_rproc_mem_unmap(struct rproc *rproc,
+                                struct rproc_mem_entry *mem)
+{
+    iounmap((void __iomem *)mem->va);
+
+    return 0;
+}

The above 2 are identical to what is found in xlnx_r5_remoteproc.c. Please coordinate with Tanmay to split that into common code that can be reused by both drivers.

Quoted 31 lines
drivers/remoteproc/amd_bram_rproc.c new file +31 -0
+
+static int amd_bram_rproc_prepare(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    struct rproc_mem_entry *mem;
+    struct resource res;
+    u64 da, size;
+    int ret;
+
+    ret = of_property_read_reg(priv->dev->of_node, 0, &da, &size);
+    if (ret) {
+            dev_err(priv->dev, "failed to parse executable memory reg\n");
+            return ret;
+    }
+
+    if (!size || size > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory size\n");
+            return -EINVAL;
+    }
+
+    if (da > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory address\n");
+            return -EINVAL;
+    }
+
+    ret = of_address_to_resource(priv->dev->of_node, 0, &res);
+    if (ret) {
+            dev_err(priv->dev, "failed to translate executable memory reg\n");
+            return ret;
+    }
+

drivers/remoteproc/amd_bram_rproc.c line 104

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)res.start,
+                               (size_t)size, da,
+                               amd_bram_rproc_mem_map,
+                               amd_bram_rproc_mem_unmap,
+                               dev_name(priv->dev));
+    if (!mem)
+            return -ENOMEM;
+
+    rproc_add_carveout(rproc, mem);
+    rproc_coredump_add_segment(rproc, da, (size_t)size);

I'm pretty sure you want @res.start instead of @da, and resource_size(&res) instead of @size.

Quoted 48 lines
drivers/remoteproc/amd_bram_rproc.c new file +48 -0
+
+    return 0;
+}
+
+static int amd_bram_rproc_start(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Enable clock before releasing reset */
+    ret = clk_prepare_enable(priv->clk);
+    if (ret) {
+            dev_err(priv->dev, "failed to enable clock: %d\n", ret);
+            return ret;
+    }
+
+    /* Deassert reset and let the processor run. */
+    ret = gpiod_set_value_cansleep(priv->reset, 0);
+    if (ret) {
+            dev_err(priv->dev, "failed to deassert reset: %d\n", ret);
+            clk_disable_unprepare(priv->clk);
+            return ret;
+    }
+
+    return 0;
+}
+
+static int amd_bram_rproc_stop(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Assert reset before disabling the processor clock. */
+    ret = gpiod_set_value_cansleep(priv->reset, 1);
+    if (ret) {
+            dev_err(priv->dev, "failed to assert reset: %d\n", ret);
+            return ret;
+    }
+
+    /* Disable clock after asserting reset */
+    clk_disable_unprepare(priv->clk);
+
+    return 0;
+}
+
+static int amd_bram_rproc_parse_fw(struct rproc *rproc,
+                               const struct firmware *fw)
+{

drivers/remoteproc/amd_bram_rproc.c line 162

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    int ret;
+
+    ret = rproc_elf_load_rsc_table(rproc, fw);
+    if (ret == -EINVAL) {
+            dev_dbg(&rproc->dev, "no resource table found\n");
+            return 0;
+    }
+
+    return ret;
+}

This too should go in common code or simply replaced by rproc_elf_load_rsc_table() in @amd_bram_rproc_ops - the choice is yours.

Quoted 2 lines

Thanks,
Mathieu

Thanks for the review.

I went through the remoteproc drivers to scope the cleanup points you called out.

For the plain carveout map/unmap callbacks, the same ioremap_wc()/iounmap() pattern exists not only in amd_bram_rproc and xlnx_r5_remoteproc, but also in rcar_rproc, st_remoteproc, stm32_rproc, imx_rproc, and imx_dsp_rproc.

The xlnx_r5 TCM path is close as well, but that one still needs a wrapper since it clears the memory after ioremap_wc().

For the optional resource-table parsing, amd_bram_rproc and xlnx_r5_remoteproc share the same pattern of treating only -EINVAL from rproc_elf_load_rsc_table() as non-fatal. PRU is similar, but has additional firmware parsing after that. Other drivers such as rcar/imx/imx_dsp/stm32 also tolerate missing resource tables, but their current behavior is not identical since they flatten all errors to success and only log.

For the next revision, would you prefer the following approach?

Quoted 7 lines

1. Add a small common helper for the plain carveout ioremap_wc()/iounmap() case and use it in amd_bram_rproc and xlnx_r5_remoteproc.

2. For the optional resource-table handling, either:
- add a small common helper for the "missing table is OK" case (i.e. return 0 on -EINVAL and propagate other errors), and use that in amd_bram_rproc and xlnx_r5_remoteproc, or

I would prefer to go with the common helper that returns 0 on -EINVAL and propagates other errors, and apply it to other architectures such as stm32, rcar, imx and imx_dsp.

Quoted 8 lines

- drop the custom AMD parse_fw() path and use rproc_elf_load_rsc_table() directly, which would make the resource table mandatory there.

Also, for the plain map/unmap helper, should I keep the cleanup scoped to the drivers directly involved here, or would you prefer that I fold the other exact-match users (rcar, st, stm32, imx, imx_dsp) into the same cleanup patch as well?

Proceed with the other exact-match as well.

Quoted 1 line

I want to make sure I take the direction you prefer before respinning.

I think the best approach is to send out a cleanup patchset with the above changes, followed by another respin of this set once the cleanup is merged.

Thanks for being proactive.

Quoted 90 lines

Thanks,
Ben

Quoted 86 lines
Quoted 85 lines
drivers/remoteproc/amd_bram_rproc.c new file +81 -1
+
+static const struct rproc_ops amd_bram_rproc_ops = {
+    .prepare        = amd_bram_rproc_prepare,
+    .start          = amd_bram_rproc_start,
+    .stop           = amd_bram_rproc_stop,
+    .load           = rproc_elf_load_segments,
+    .sanity_check   = rproc_elf_sanity_check,
+    .get_boot_addr  = rproc_elf_get_boot_addr,
+    .parse_fw       = amd_bram_rproc_parse_fw,
+};
+
+static int amd_bram_rproc_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct amd_bram_rproc *priv;
+    const char *fw_name = NULL;
+    struct rproc *rproc;
+    int ret;
+
+    ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+    if (ret < 0 && ret != -EINVAL)
+            return dev_err_probe(dev, ret,
+                                 "failed to parse firmware-name property\n");
+
+    rproc = devm_rproc_alloc(dev, dev_name(dev), &amd_bram_rproc_ops,
+                             fw_name, sizeof(*priv));
+    if (!rproc)
+            return -ENOMEM;
+
+    priv = rproc->priv;
+    priv->dev = dev;
+
+    /* Get the processor clock */
+    priv->clk = devm_clk_get(dev, NULL);
+    if (IS_ERR(priv->clk))
+            return dev_err_probe(dev, PTR_ERR(priv->clk),
+                                 "failed to get clock\n");
+
+    /*
+     * Keep the processor in reset until remoteproc has finished loading
+     * firmware into the executable memory window described by reg and
+     * translated through the parent bus ranges property.
+     */
+    priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+    if (IS_ERR(priv->reset))
+            return dev_err_probe(dev, PTR_ERR(priv->reset),
+                                 "failed to get reset gpio\n");
+
+    rproc->auto_boot = false;
+
+    ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+    platform_set_drvdata(pdev, rproc);
+
+    ret = devm_rproc_add(dev, rproc);
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to register rproc\n");
+
+    return 0;
+}
+
+static const struct of_device_id amd_bram_rproc_of_match[] = {
+    { .compatible = "xlnx,zynqmp-bram-rproc" },
+    { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, amd_bram_rproc_of_match);
+
+static struct platform_driver amd_bram_rproc_driver = {
+    .probe = amd_bram_rproc_probe,
+    .driver = {
+            .name = "amd-bram-rproc",
+            .of_match_table = amd_bram_rproc_of_match,
+    },
+};
+module_platform_driver(amd_bram_rproc_driver);
+
+MODULE_DESCRIPTION("AMD BRAM-based Remote Processor driver");
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL");
--

2.34.1

#7 lore raw

Hi Mathieu

Picking this back up now. Please see my replies inline below.

Quoted 213 lines · Mathieu Poirier
Quoted 212 lines · Ben Levinsky

Hi Mathieu,

Quoted 207 lines · Mathieu Poirier

Good morning,

Quoted 35 lines · Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +0 -0
index c871acf2179c..172539971950 100644

MAINTAINERS S: Maintained · line 1046

Quoted 10 lines
MAINTAINERS +7 -0
@@ -1037,6 +1037,13 @@ S:    Maintained
 F:  Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:  drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:  Ben Levinsky <[email protected]>
+L:  [email protected]
+S:  Maintained
+F:  Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:  drivers/remoteproc/amd_bram_rproc.c
+

There is no real advantage in adding this entry, checkpatch.pl should be sufficient.

Quoted 12 lines
MAINTAINERS +0 -0
 AMD CDX BUS DRIVER
 M:  Nipun Gupta <[email protected]>
 M:  Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +1 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
       It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC

drivers/remoteproc/Kconfig config REMOTEPROC_CDEV · line 36

Quoted 10 lines
drivers/remoteproc/Kconfig +10 -0
+    tristate "AMD BRAM-based remoteproc support"
+    depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+    help
+      Say y or m here to support a BRAM-based remote processor managed
+      through the remoteproc framework.
+
+      This driver matches designs where executable firmware memory is
+      described in the BRAM-local address space and translated to
+      the system physical address space with standard devicetree address
+      translation.

Not sure how this paragraph helps decide whether the driver should be enabled or not. Please remove.

Quoted 77 lines
drivers/remoteproc/Kconfig +3 -0
+
+      If unsure, say N.
+
 config IMX_REMOTEPROC
     tristate "i.MX remoteproc support"
     depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y                               += remoteproc_sysfs.o
 remoteproc-y                                += remoteproc_virtio.o
 remoteproc-y                                += remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)               += remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)   += amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)                += imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)    += imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)             += ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +53 -0
new file mode 100644
index 000000000000..9383964b6046
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD BRAM-based Remote Processor driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver supports soft-core processors (MicroBlaze, MicroBlaze-V, or
+ * similar) instantiated in AMD programmable logic, using dual-port BRAM
+ * for firmware storage and execution.
+ *
+ * The firmware memory (BRAM) is described in the processor-local address
+ * space and translated to the Linux-visible system physical address with
+ * standard devicetree address translation.
+ *
+ * Reset is controlled via GPIO connected to Processor System Reset IP.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/**
+ * struct amd_bram_rproc - AMD BRAM-based remoteproc private data
+ * @dev: device pointer
+ * @reset: GPIO descriptor for reset control (active-low)
+ * @clk: processor clock
+ */
+struct amd_bram_rproc {
+    struct device *dev;
+    struct gpio_desc *reset;
+    struct clk *clk;
+};
+
+static int amd_bram_rproc_mem_map(struct rproc *rproc,
+                              struct rproc_mem_entry *mem)
+{
+    void __iomem *va;
+
+    va = ioremap_wc(mem->dma, mem->len);
+    if (!va)
+            return -ENOMEM;
+
+    mem->va = (__force void *)va;
+    mem->is_iomem = true;
+

drivers/remoteproc/amd_bram_rproc.c line 63

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    return 0;
+}
+
+static int amd_bram_rproc_mem_unmap(struct rproc *rproc,
+                                struct rproc_mem_entry *mem)
+{
+    iounmap((void __iomem *)mem->va);
+
+    return 0;
+}

The above 2 are identical to what is found in xlnx_r5_remoteproc.c. Please coordinate with Tanmay to split that into common code that can be reused by both drivers.

Quoted 37 lines
drivers/remoteproc/amd_bram_rproc.c new file +37 -0
+
+static int amd_bram_rproc_prepare(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    struct rproc_mem_entry *mem;
+    struct resource res;
+    u64 da, size;
+    int ret;
+
+    ret = of_property_read_reg(priv->dev->of_node, 0, &da, &size);
+    if (ret) {
+            dev_err(priv->dev, "failed to parse executable memory reg\n");
+            return ret;
+    }
+
+    if (!size || size > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory size\n");
+            return -EINVAL;
+    }
+
+    if (da > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory address\n");
+            return -EINVAL;
+    }
+
+    ret = of_address_to_resource(priv->dev->of_node, 0, &res);
+    if (ret) {
+            dev_err(priv->dev, "failed to translate executable memory reg\n");
+            return ret;
+    }
+
+    mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)res.start,
+                               (size_t)size, da,
+                               amd_bram_rproc_mem_map,
+                               amd_bram_rproc_mem_unmap,
+                               dev_name(priv->dev));
+    if (!mem)
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines

drivers/remoteproc/amd_bram_rproc.c line 104

Quoted 4 lines
drivers/remoteproc/amd_bram_rproc.c new file +4 -0
+            return -ENOMEM;
+
+    rproc_add_carveout(rproc, mem);
+    rproc_coredump_add_segment(rproc, da, (size_t)size);

I'm pretty sure you want @res.start instead of @da, and resource_size(&res) instead of @size.

For the coredump segment, I agree with using resource_size(&res) for the size, but I think the address should remain @da rather than @res.start.

The binding describes the reg property in the processor-local address space and uses the parent bus ranges property only to translate that window to the Linux-visible system physical address. That means @da and @res.start are not necessarily in the same address space. For example, the BRAM can appear at 0x0 to the soft-core processor while Linux sees the same memory at a translated system physical address such as 0xa0000000.

rproc_coredump_add_segment() stores the address as a device address, and the coredump path later resolves it through rproc_da_to_va() against the registered carveout's device address. Since this driver registers the carveout with @da as the device address and @res.start as the host-side physical address used for ioremap_wc(), passing @res.start to rproc_coredump_add_segment() could fail to match the carveout when those addresses differ.

So in the respin I plan to use:

          rproc_coredump_add_segment(rproc, da, resource_size(&res));

Does that match your expectation for this address model?

Thank you
Ben

Quoted 207 lines
Quoted 86 lines
Quoted 62 lines
Quoted 48 lines
drivers/remoteproc/amd_bram_rproc.c new file +48 -0
+
+    return 0;
+}
+
+static int amd_bram_rproc_start(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Enable clock before releasing reset */
+    ret = clk_prepare_enable(priv->clk);
+    if (ret) {
+            dev_err(priv->dev, "failed to enable clock: %d\n", ret);
+            return ret;
+    }
+
+    /* Deassert reset and let the processor run. */
+    ret = gpiod_set_value_cansleep(priv->reset, 0);
+    if (ret) {
+            dev_err(priv->dev, "failed to deassert reset: %d\n", ret);
+            clk_disable_unprepare(priv->clk);
+            return ret;
+    }
+
+    return 0;
+}
+
+static int amd_bram_rproc_stop(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Assert reset before disabling the processor clock. */
+    ret = gpiod_set_value_cansleep(priv->reset, 1);
+    if (ret) {
+            dev_err(priv->dev, "failed to assert reset: %d\n", ret);
+            return ret;
+    }
+
+    /* Disable clock after asserting reset */
+    clk_disable_unprepare(priv->clk);
+
+    return 0;
+}
+
+static int amd_bram_rproc_parse_fw(struct rproc *rproc,
+                               const struct firmware *fw)
+{

drivers/remoteproc/amd_bram_rproc.c line 162

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    int ret;
+
+    ret = rproc_elf_load_rsc_table(rproc, fw);
+    if (ret == -EINVAL) {
+            dev_dbg(&rproc->dev, "no resource table found\n");
+            return 0;
+    }
+
+    return ret;
+}

This too should go in common code or simply replaced by rproc_elf_load_rsc_table() in @amd_bram_rproc_ops - the choice is yours.

Quoted 2 lines

Thanks,
Mathieu

Thanks for the review.

I went through the remoteproc drivers to scope the cleanup points you called out.

For the plain carveout map/unmap callbacks, the same ioremap_wc()/iounmap() pattern exists not only in amd_bram_rproc and xlnx_r5_remoteproc, but also in rcar_rproc, st_remoteproc, stm32_rproc, imx_rproc, and imx_dsp_rproc.

The xlnx_r5 TCM path is close as well, but that one still needs a wrapper since it clears the memory after ioremap_wc().

For the optional resource-table parsing, amd_bram_rproc and xlnx_r5_remoteproc share the same pattern of treating only -EINVAL from rproc_elf_load_rsc_table() as non-fatal. PRU is similar, but has additional firmware parsing after that. Other drivers such as rcar/imx/imx_dsp/stm32 also tolerate missing resource tables, but their current behavior is not identical since they flatten all errors to success and only log.

For the next revision, would you prefer the following approach?

Quoted 7 lines

1. Add a small common helper for the plain carveout ioremap_wc()/iounmap() case and use it in amd_bram_rproc and xlnx_r5_remoteproc.

2. For the optional resource-table handling, either:
- add a small common helper for the "missing table is OK" case (i.e. return 0 on -EINVAL and propagate other errors), and use that in amd_bram_rproc and xlnx_r5_remoteproc, or

I would prefer to go with the common helper that returns 0 on -EINVAL and propagates other errors, and apply it to other architectures such as stm32, rcar, imx and imx_dsp.

Quoted 8 lines

- drop the custom AMD parse_fw() path and use rproc_elf_load_rsc_table() directly, which would make the resource table mandatory there.

Also, for the plain map/unmap helper, should I keep the cleanup scoped to the drivers directly involved here, or would you prefer that I fold the other exact-match users (rcar, st, stm32, imx, imx_dsp) into the same cleanup patch as well?

Proceed with the other exact-match as well.

Quoted 1 line

I want to make sure I take the direction you prefer before respinning.

I think the best approach is to send out a cleanup patchset with the above changes, followed by another respin of this set once the cleanup is merged.

Thanks for being proactive.

Quoted 90 lines

Thanks,
Ben

Quoted 86 lines
Quoted 85 lines
drivers/remoteproc/amd_bram_rproc.c new file +81 -1
+
+static const struct rproc_ops amd_bram_rproc_ops = {
+    .prepare        = amd_bram_rproc_prepare,
+    .start          = amd_bram_rproc_start,
+    .stop           = amd_bram_rproc_stop,
+    .load           = rproc_elf_load_segments,
+    .sanity_check   = rproc_elf_sanity_check,
+    .get_boot_addr  = rproc_elf_get_boot_addr,
+    .parse_fw       = amd_bram_rproc_parse_fw,
+};
+
+static int amd_bram_rproc_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct amd_bram_rproc *priv;
+    const char *fw_name = NULL;
+    struct rproc *rproc;
+    int ret;
+
+    ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+    if (ret < 0 && ret != -EINVAL)
+            return dev_err_probe(dev, ret,
+                                 "failed to parse firmware-name property\n");
+
+    rproc = devm_rproc_alloc(dev, dev_name(dev), &amd_bram_rproc_ops,
+                             fw_name, sizeof(*priv));
+    if (!rproc)
+            return -ENOMEM;
+
+    priv = rproc->priv;
+    priv->dev = dev;
+
+    /* Get the processor clock */
+    priv->clk = devm_clk_get(dev, NULL);
+    if (IS_ERR(priv->clk))
+            return dev_err_probe(dev, PTR_ERR(priv->clk),
+                                 "failed to get clock\n");
+
+    /*
+     * Keep the processor in reset until remoteproc has finished loading
+     * firmware into the executable memory window described by reg and
+     * translated through the parent bus ranges property.
+     */
+    priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+    if (IS_ERR(priv->reset))
+            return dev_err_probe(dev, PTR_ERR(priv->reset),
+                                 "failed to get reset gpio\n");
+
+    rproc->auto_boot = false;
+
+    ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+    platform_set_drvdata(pdev, rproc);
+
+    ret = devm_rproc_add(dev, rproc);
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to register rproc\n");
+
+    return 0;
+}
+
+static const struct of_device_id amd_bram_rproc_of_match[] = {
+    { .compatible = "xlnx,zynqmp-bram-rproc" },
+    { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, amd_bram_rproc_of_match);
+
+static struct platform_driver amd_bram_rproc_driver = {
+    .probe = amd_bram_rproc_probe,
+    .driver = {
+            .name = "amd-bram-rproc",
+            .of_match_table = amd_bram_rproc_of_match,
+    },
+};
+module_platform_driver(amd_bram_rproc_driver);
+
+MODULE_DESCRIPTION("AMD BRAM-based Remote Processor driver");
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL");
--

2.34.1

maintainer
#8 lore raw
Quoted 226 lines · Ben Levinsky

Hi Mathieu

Picking this back up now. Please see my replies inline below.

Quoted 221 lines · Mathieu Poirier
Quoted 220 lines · Ben Levinsky

Hi Mathieu,

Quoted 215 lines · Mathieu Poirier

Good morning,

Quoted 35 lines · Ben Levinsky

Add a remoteproc driver for AMD soft-core processor subsystems instantiated in programmable logic and using dual-port BRAM for firmware storage and execution.

The driver parses the firmware memory window from the remoteproc device node's reg property, interprets that address and size in the processor-local address space, and then uses standard devicetree address translation through the parent bus ranges property to obtain the corresponding Linux-visible system physical address.

The resulting translated region is registered as the executable remoteproc carveout and coredump segment.

The processor is controlled through an active-low reset GPIO and a subsystem clock. The clock is enabled before reset is released, and the processor is kept in reset until firmware loading completes.

The firmware-name property is optional, allowing firmware to be assigned later through the remoteproc framework. Firmware images without a resource table are also accepted.

---
 MAINTAINERS                         |   7 +
 drivers/remoteproc/Kconfig          |  14 ++
 drivers/remoteproc/Makefile         |   1 +
 drivers/remoteproc/amd_bram_rproc.c | 243 ++++++++++++++++++++++++++++
 4 files changed, 265 insertions(+)
 create mode 100644 drivers/remoteproc/amd_bram_rproc.c
MAINTAINERS +0 -0
index c871acf2179c..172539971950 100644

MAINTAINERS S: Maintained · line 1046

Quoted 10 lines
MAINTAINERS +7 -0
@@ -1037,6 +1037,13 @@ S:    Maintained
 F:  Documentation/devicetree/bindings/w1/amd,axi-1wire-host.yaml
 F:  drivers/w1/masters/amd_axi_w1.c
 
+AMD BRAM REMOTEPROC DRIVER
+M:  Ben Levinsky <[email protected]>
+L:  [email protected]
+S:  Maintained
+F:  Documentation/devicetree/bindings/remoteproc/amd,bram-rproc.yaml
+F:  drivers/remoteproc/amd_bram_rproc.c
+

There is no real advantage in adding this entry, checkpatch.pl should be sufficient.

Quoted 12 lines
MAINTAINERS +0 -0
 AMD CDX BUS DRIVER
 M:  Nipun Gupta <[email protected]>
 M:  Nikhil Agarwal <[email protected]>
drivers/remoteproc/Kconfig +1 -0
index ee54436fea5a..9a2a887ede8a 100644
@@ -23,6 +23,20 @@ config REMOTEPROC_CDEV
 
       It's safe to say N if you don't want to use this interface.
 
+config AMD_BRAM_REMOTEPROC

drivers/remoteproc/Kconfig config REMOTEPROC_CDEV · line 36

Quoted 10 lines
drivers/remoteproc/Kconfig +10 -0
+    tristate "AMD BRAM-based remoteproc support"
+    depends on OF && COMMON_CLK && (GPIOLIB || COMPILE_TEST)
+    help
+      Say y or m here to support a BRAM-based remote processor managed
+      through the remoteproc framework.
+
+      This driver matches designs where executable firmware memory is
+      described in the BRAM-local address space and translated to
+      the system physical address space with standard devicetree address
+      translation.

Not sure how this paragraph helps decide whether the driver should be enabled or not. Please remove.

Quoted 77 lines
drivers/remoteproc/Kconfig +3 -0
+
+      If unsure, say N.
+
 config IMX_REMOTEPROC
     tristate "i.MX remoteproc support"
     depends on ARCH_MXC
drivers/remoteproc/Makefile +1 -0
index 1c7598b8475d..5c39664b50c3 100644
@@ -11,6 +11,7 @@ remoteproc-y                               += remoteproc_sysfs.o
 remoteproc-y                                += remoteproc_virtio.o
 remoteproc-y                                += remoteproc_elf_loader.o
 obj-$(CONFIG_REMOTEPROC_CDEV)               += remoteproc_cdev.o
+obj-$(CONFIG_AMD_BRAM_REMOTEPROC)   += amd_bram_rproc.o
 obj-$(CONFIG_IMX_REMOTEPROC)                += imx_rproc.o
 obj-$(CONFIG_IMX_DSP_REMOTEPROC)    += imx_dsp_rproc.o
 obj-$(CONFIG_INGENIC_VPU_RPROC)             += ingenic_rproc.o
drivers/remoteproc/amd_bram_rproc.c new file +53 -0
new file mode 100644
index 000000000000..9383964b6046
@@ -0,0 +1,243 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * AMD BRAM-based Remote Processor driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * This driver supports soft-core processors (MicroBlaze, MicroBlaze-V, or
+ * similar) instantiated in AMD programmable logic, using dual-port BRAM
+ * for firmware storage and execution.
+ *
+ * The firmware memory (BRAM) is described in the processor-local address
+ * space and translated to the Linux-visible system physical address with
+ * standard devicetree address translation.
+ *
+ * Reset is controlled via GPIO connected to Processor System Reset IP.
+ */
+
+#include <linux/clk.h>
+#include <linux/dma-mapping.h>
+#include <linux/gpio/consumer.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+/**
+ * struct amd_bram_rproc - AMD BRAM-based remoteproc private data
+ * @dev: device pointer
+ * @reset: GPIO descriptor for reset control (active-low)
+ * @clk: processor clock
+ */
+struct amd_bram_rproc {
+    struct device *dev;
+    struct gpio_desc *reset;
+    struct clk *clk;
+};
+
+static int amd_bram_rproc_mem_map(struct rproc *rproc,
+                              struct rproc_mem_entry *mem)
+{
+    void __iomem *va;
+
+    va = ioremap_wc(mem->dma, mem->len);
+    if (!va)
+            return -ENOMEM;
+
+    mem->va = (__force void *)va;
+    mem->is_iomem = true;
+

drivers/remoteproc/amd_bram_rproc.c line 63

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    return 0;
+}
+
+static int amd_bram_rproc_mem_unmap(struct rproc *rproc,
+                                struct rproc_mem_entry *mem)
+{
+    iounmap((void __iomem *)mem->va);
+
+    return 0;
+}

The above 2 are identical to what is found in xlnx_r5_remoteproc.c. Please coordinate with Tanmay to split that into common code that can be reused by both drivers.

Quoted 31 lines
drivers/remoteproc/amd_bram_rproc.c new file +31 -0
+
+static int amd_bram_rproc_prepare(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    struct rproc_mem_entry *mem;
+    struct resource res;
+    u64 da, size;
+    int ret;
+
+    ret = of_property_read_reg(priv->dev->of_node, 0, &da, &size);
+    if (ret) {
+            dev_err(priv->dev, "failed to parse executable memory reg\n");
+            return ret;
+    }
+
+    if (!size || size > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory size\n");
+            return -EINVAL;
+    }
+
+    if (da > U32_MAX) {
+            dev_err(priv->dev, "invalid executable memory address\n");
+            return -EINVAL;
+    }
+
+    ret = of_address_to_resource(priv->dev->of_node, 0, &res);
+    if (ret) {
+            dev_err(priv->dev, "failed to translate executable memory reg\n");
+            return ret;
+    }
+

drivers/remoteproc/amd_bram_rproc.c line 104

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    mem = rproc_mem_entry_init(priv->dev, NULL, (dma_addr_t)res.start,
+                               (size_t)size, da,
+                               amd_bram_rproc_mem_map,
+                               amd_bram_rproc_mem_unmap,
+                               dev_name(priv->dev));
+    if (!mem)
+            return -ENOMEM;
+
+    rproc_add_carveout(rproc, mem);
+    rproc_coredump_add_segment(rproc, da, (size_t)size);

I'm pretty sure you want @res.start instead of @da, and resource_size(&res) instead of @size.

Looking at the code again, I'm not sure why I asked to used @da.

Quoted 21 lines

For the coredump segment, I agree with using resource_size(&res) for the size, but I think the address should remain @da rather than @res.start.

The binding describes the reg property in the processor-local address space and uses the parent bus ranges property only to translate that window to the Linux-visible system physical address. That means @da and @res.start are not necessarily in the same address space. For example, the BRAM can appear at 0x0 to the soft-core processor while Linux sees the same memory at a translated system physical address such as 0xa0000000.

rproc_coredump_add_segment() stores the address as a device address, and the coredump path later resolves it through rproc_da_to_va() against the registered carveout's device address. Since this driver registers the carveout with @da as the device address and @res.start as the host-side physical address used for ioremap_wc(), passing @res.start to rproc_coredump_add_segment() could fail to match the carveout when those addresses differ.

Quoted 5 lines

So in the respin I plan to use:

          rproc_coredump_add_segment(rproc, da, resource_size(&res));

Does that match your expectation for this address model?

Yes, I agree with you proposal.

Quoted 212 lines

Thank you
Ben

Quoted 207 lines
Quoted 86 lines
Quoted 62 lines
Quoted 48 lines
drivers/remoteproc/amd_bram_rproc.c new file +48 -0
+
+    return 0;
+}
+
+static int amd_bram_rproc_start(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Enable clock before releasing reset */
+    ret = clk_prepare_enable(priv->clk);
+    if (ret) {
+            dev_err(priv->dev, "failed to enable clock: %d\n", ret);
+            return ret;
+    }
+
+    /* Deassert reset and let the processor run. */
+    ret = gpiod_set_value_cansleep(priv->reset, 0);
+    if (ret) {
+            dev_err(priv->dev, "failed to deassert reset: %d\n", ret);
+            clk_disable_unprepare(priv->clk);
+            return ret;
+    }
+
+    return 0;
+}
+
+static int amd_bram_rproc_stop(struct rproc *rproc)
+{
+    struct amd_bram_rproc *priv = rproc->priv;
+    int ret;
+
+    /* Assert reset before disabling the processor clock. */
+    ret = gpiod_set_value_cansleep(priv->reset, 1);
+    if (ret) {
+            dev_err(priv->dev, "failed to assert reset: %d\n", ret);
+            return ret;
+    }
+
+    /* Disable clock after asserting reset */
+    clk_disable_unprepare(priv->clk);
+
+    return 0;
+}
+
+static int amd_bram_rproc_parse_fw(struct rproc *rproc,
+                               const struct firmware *fw)
+{

drivers/remoteproc/amd_bram_rproc.c line 162

Quoted 10 lines
drivers/remoteproc/amd_bram_rproc.c new file +10 -0
+    int ret;
+
+    ret = rproc_elf_load_rsc_table(rproc, fw);
+    if (ret == -EINVAL) {
+            dev_dbg(&rproc->dev, "no resource table found\n");
+            return 0;
+    }
+
+    return ret;
+}

This too should go in common code or simply replaced by rproc_elf_load_rsc_table() in @amd_bram_rproc_ops - the choice is yours.

Quoted 2 lines

Thanks,
Mathieu

Thanks for the review.

I went through the remoteproc drivers to scope the cleanup points you called out.

For the plain carveout map/unmap callbacks, the same ioremap_wc()/iounmap() pattern exists not only in amd_bram_rproc and xlnx_r5_remoteproc, but also in rcar_rproc, st_remoteproc, stm32_rproc, imx_rproc, and imx_dsp_rproc.

The xlnx_r5 TCM path is close as well, but that one still needs a wrapper since it clears the memory after ioremap_wc().

For the optional resource-table parsing, amd_bram_rproc and xlnx_r5_remoteproc share the same pattern of treating only -EINVAL from rproc_elf_load_rsc_table() as non-fatal. PRU is similar, but has additional firmware parsing after that. Other drivers such as rcar/imx/imx_dsp/stm32 also tolerate missing resource tables, but their current behavior is not identical since they flatten all errors to success and only log.

For the next revision, would you prefer the following approach?

Quoted 7 lines

1. Add a small common helper for the plain carveout ioremap_wc()/iounmap() case and use it in amd_bram_rproc and xlnx_r5_remoteproc.

2. For the optional resource-table handling, either:
- add a small common helper for the "missing table is OK" case (i.e. return 0 on -EINVAL and propagate other errors), and use that in amd_bram_rproc and xlnx_r5_remoteproc, or

I would prefer to go with the common helper that returns 0 on -EINVAL and propagates other errors, and apply it to other architectures such as stm32, rcar, imx and imx_dsp.

Quoted 8 lines

- drop the custom AMD parse_fw() path and use rproc_elf_load_rsc_table() directly, which would make the resource table mandatory there.

Also, for the plain map/unmap helper, should I keep the cleanup scoped to the drivers directly involved here, or would you prefer that I fold the other exact-match users (rcar, st, stm32, imx, imx_dsp) into the same cleanup patch as well?

Proceed with the other exact-match as well.

Quoted 1 line

I want to make sure I take the direction you prefer before respinning.

I think the best approach is to send out a cleanup patchset with the above changes, followed by another respin of this set once the cleanup is merged.

Thanks for being proactive.

Quoted 90 lines

Thanks,
Ben

Quoted 86 lines
Quoted 85 lines
drivers/remoteproc/amd_bram_rproc.c new file +81 -1
+
+static const struct rproc_ops amd_bram_rproc_ops = {
+    .prepare        = amd_bram_rproc_prepare,
+    .start          = amd_bram_rproc_start,
+    .stop           = amd_bram_rproc_stop,
+    .load           = rproc_elf_load_segments,
+    .sanity_check   = rproc_elf_sanity_check,
+    .get_boot_addr  = rproc_elf_get_boot_addr,
+    .parse_fw       = amd_bram_rproc_parse_fw,
+};
+
+static int amd_bram_rproc_probe(struct platform_device *pdev)
+{
+    struct device *dev = &pdev->dev;
+    struct amd_bram_rproc *priv;
+    const char *fw_name = NULL;
+    struct rproc *rproc;
+    int ret;
+
+    ret = rproc_of_parse_firmware(dev, 0, &fw_name);
+    if (ret < 0 && ret != -EINVAL)
+            return dev_err_probe(dev, ret,
+                                 "failed to parse firmware-name property\n");
+
+    rproc = devm_rproc_alloc(dev, dev_name(dev), &amd_bram_rproc_ops,
+                             fw_name, sizeof(*priv));
+    if (!rproc)
+            return -ENOMEM;
+
+    priv = rproc->priv;
+    priv->dev = dev;
+
+    /* Get the processor clock */
+    priv->clk = devm_clk_get(dev, NULL);
+    if (IS_ERR(priv->clk))
+            return dev_err_probe(dev, PTR_ERR(priv->clk),
+                                 "failed to get clock\n");
+
+    /*
+     * Keep the processor in reset until remoteproc has finished loading
+     * firmware into the executable memory window described by reg and
+     * translated through the parent bus ranges property.
+     */
+    priv->reset = devm_gpiod_get(dev, "reset", GPIOD_OUT_HIGH);
+    if (IS_ERR(priv->reset))
+            return dev_err_probe(dev, PTR_ERR(priv->reset),
+                                 "failed to get reset gpio\n");
+
+    rproc->auto_boot = false;
+
+    ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to set DMA mask\n");
+
+    platform_set_drvdata(pdev, rproc);
+
+    ret = devm_rproc_add(dev, rproc);
+    if (ret)
+            return dev_err_probe(dev, ret, "failed to register rproc\n");
+
+    return 0;
+}
+
+static const struct of_device_id amd_bram_rproc_of_match[] = {
+    { .compatible = "xlnx,zynqmp-bram-rproc" },
+    { /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, amd_bram_rproc_of_match);
+
+static struct platform_driver amd_bram_rproc_driver = {
+    .probe = amd_bram_rproc_probe,
+    .driver = {
+            .name = "amd-bram-rproc",
+            .of_match_table = amd_bram_rproc_of_match,
+    },
+};
+module_platform_driver(amd_bram_rproc_driver);
+
+MODULE_DESCRIPTION("AMD BRAM-based Remote Processor driver");
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL");
--

2.34.1