The driver was tested on Xilinx ZynqMP QEMU
For sake of ease of review, only support ZynqMP. Once accepted, then
add support for Versal platform and R5 loading onto OCM.
v2:
- remove domain struct
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
function pointer in xilinx eemi ops struct
- update zynqmp_r5 yaml parsing to not raise warnings for extra
information in children of R5 node. The warning "node has a unit
name, but no reg or ranges property" will still be raised though
as this particular node is needed to describe the
'#address-cells' and '#size-cells' information.
v4:
- add default values for enums
- fix formatting as per checkpatch.pl --strict. Note that 1 warning and
1 check are still raised as each is due to fixing the warning
results in that particular line going over 80 characters.
- remove warning '/example-0/rpu@ff9a0000/r5@0:
node has a unit name, but no reg or ranges property'
by adding reg to r5 node.
v5:
- update device tree sample and yaml parsing to not raise any warnings
- description for memory-region in yaml parsing
- compatible string in yaml parsing for TCM
- parse_fw change from use of rproc_of_resm_mem_entry_init to
rproc_mem_entry_init and use of alloc/release
- var's of type zynqmp_r5_pdata all have same local variable name
- use dev_dbg instead of dev_info
v6:
- adding memory carveouts is handled much more similarly.
All mem carveouts are now described in reserved memory as needed.
That is, TCM nodes are not coupled to remoteproc anymore.
This is reflected in the remoteproc R5 driver and the device tree
binding.
- remove mailbox from device tree binding as it is not necessary for elf
loading
v7:
- remove unused headers
- zynqmp_r5_remoteproc_probe:lockstep_mode from u32* to u32
- device-tree binding "lockstep-mode" to "xlnx,cluster-mode"
- remove zynqmp_r5_mem_probe and loop to Probe R5 memory devices at
probe()
- remove is_r5_mode_set from zynqmp rpu remote processor private data
- do not error out if no mailbox is provided since mailboxes are optional
- remove zynqmp_r5_remoteproc_probe call of platform_set_drvdata as pdata
is handled in zynqmp_r5_remoteproc_remove
v8:
- remove old acks, reviewed-by's in commit message
v9:
- if zynqmp_r5_remoteproc.c pdata->tx_mc_skbs not initialized, then do not
call skb_queue_empty
- update arguments and documentation for zynqmp_pm_set_rpu_mode
- in fn zynqmp_pm_force_powerdown, change arg 'target' to 'node'
- zynqmp_pm_request_wakeup update code style
- edit 3/5 patch commit message
- document zynqmp_pm_set_tcm_config and zynqmp_pm_get_rpu_mode
documentation to include expected return val
- remove unused fn zynqmp_pm_get_node_status
- update 5/5 patch commit message to document supported configurations
and how they are booted by the driver.
- remove copyrights other than SPDX from zynqmp_r5_remoteproc.c
- compilation warnings no longer raised
- remove unused includes from zynqmp_r5_remoteproc.c
- remove unused var autoboot from zynqmp_r5_remoteproc.c
- reorder zynqmp_r5_pdata fpr small mem savings due to alignment
- zynqmp_pm_set_tcm_config and zynqmp_pm_set_rpu_mode uses second arg
- zynqmp_r5_remoteproc.c use of zynqmp_pm_set_tcm_config now does not
have output arg
- in tcm handling, unconditionally use &= 0x000fffff mask since all nodes
in this fn are for tcm
- update comments for translating dma field in tcm handling to device
address
- update calls to rproc_mem_entry_init in parse_mem_regions so that there
are only 2 cases for types of carveouts instead of 3
- in parse_mem_regions, check if device tree node is null before using it
- add example device tree nodes used in parse_mem_regions and tcm parsing
- add comment for vring id node length
- add check for string length so that vring id is at least min length
- move tcm nodes from reserved mem to instead own device tree nodes
and only use them if enabled in device tree
- add comment for explaining handling of rproc_elf_load_rsc_table
- remove obsolete check for "if (vqid < 0)" in zynqmp_r5_rproc_kick
- remove unused field mems in struct zynqmp_r5_pdata
- remove call to zynqmp_r5_mem_probe and the fn itself as tcm handling
is done by zyqmp_r5_pm_request_tcm
- remove obsolete setting of dma_ops and parent device dma_mask
- remove obsolete use of of_dma_configure
- add comment for call to r5_set_mode fn
- make mbox usage optional and gracefully inform user via dev_dbg if not
present
- change lockstep_mode from u32* to u32
- update zynqmp_pm_set_rpu_mode and zynqmp_pm_set_rpu_mode documentation
and remove unused args
v10:
- add include types.h to xlnx-zynqmp.h for compilation
v11:
- update usage of zynqmp_pm_get_rpu_mode to return rpu mode in enum
- update zynqmp_pm_set_tcm_config and zynqmp_pm_set_rpu_mode arguments to
remove unused args
- use enums instead of u32 where possible in zynqmp_r5_remoteproc
- zynqmp_r5_remoteproc: update prints to not use carriage return, just
newline
- zynqmp_r5_remoteproc: look up tcm banks via property instead of string
name
- print device tree nodes with %pOF instead of %s with node name field
- update tcm release to unmap VA
- handle r5-1 use case
- device tree binding r5 node to have link to TCMs via
meta-memory-regions property
- fix device tree binding so no warnings from 'make dt_binding_check'
v12:
- update signed off by so that latest developer name is last
- in drivers/firmware/zynqmp.c, update zynqmp_pm_set_rpu_mode so rpu_mode
is only set if no error
- update args for zynqmp_pm_set_rpu_mode, zynqmp_pm_set_tcm_config fn
arg's to reflect what is expected in the function and the usage in
zynqmp_r5_remoteproc accordingly
v13:
- zynqmp_pm_get_rpu_mode argument zynqmp_pm_get_rpu_mode is
only set if no error
v14:
- rebase off v5.9-rc3 kernel
- concerns were raised about the new property meta-memory-regions.
There is no clear direction so for the moment I kept it in the series
- in device tree binding, place IPC nodes in RAM in the reserved memory section
- change zynqmp_r5_remoteproc::rpus and rpu_mode to static
- fix typo
- zynqmp_r5_remoteproc::r5_set_mode set rpu mode from
property specified in device tree
- use u32 instead of u32* to store in remoteproc memory entry private data
for pnode_id information
- always call r5_set_mode on probe
- remove alloc of zynqmp_r5_pdata in
zynqmp_r5_remoteproc::zynqmp_r5_remoteproc_probe as there is static
allocation already
- error at probe time if lockstep-mode property not present in device tree
- update commit message as per review
- remove dependency on MAILBOX in makefile as ZYNQMP_IPI_MBOX is present
- remove unused macros
- update comment ordering of zynqmp_r5_pdata to match struct definition
- zynqmp_r5_remoteproc::tcm_mem_release error if pnode id is invalid
- remove obsolete TODOs
- only call zynqmp_r5_remoteproc::zynqmp_r5_probe if the index is valid
- remove uneven dev_dbg/dev_err fn calls
v15:
- change lockstep mode device tree property from acting as
boolean, to instead being used as, if it is present, then r5 is in lockstep mode. otherwise in split.
- add safeguard for if 2 rpu remoteproc instances are provided then err out
Ben Levinsky (5):
firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU
configuration.
firmware: xilinx: Add shutdown/wakeup APIs
firmware: xilinx: Add RPU configuration APIs
dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc
bindings
remoteproc: Add initial zynqmp R5 remoteproc driver
.../xilinx,zynqmp-r5-remoteproc.yaml | 120 +++
drivers/firmware/xilinx/zynqmp.c | 96 +++
drivers/remoteproc/Kconfig | 8 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/zynqmp_r5_remoteproc.c | 769 ++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 60 ++
6 files changed, 1054 insertions(+)
create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
--
2.17.1
R5 is included in Xilinx Zynq UltraScale MPSoC so by adding this
remotproc driver, we can boot the R5 sub-system in different 2
configurations -
* split
* lock-step
The Xilinx R5 Remoteproc Driver boots the R5's via calls to the Xilinx
Platform Management Unit that handles the R5 configuration, memory access
and R5 lifecycle management. The interface to this manager is done in this
driver via zynqmp_pm_* function calls.
Signed-off-by: Wendy Liang <[email protected]>
Signed-off-by: Michal Simek <[email protected]>
Signed-off-by: Ed Mooring <[email protected]>
Signed-off-by: Jason Wu <[email protected]>
Signed-off-by: Ben Levinsky <[email protected]>
---
v2:
- remove domain struct as per review from Mathieu
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
function pointer in xilinx eemi ops struct
v4:
- add default values for enums
- fix formatting as per checkpatch.pl --strict. Note that 1 warning and 1 check
are still raised as each is due to fixing the warning results in that
particular line going over 80 characters.
v5:
- parse_fw change from use of rproc_of_resm_mem_entry_init to
rproc_mem_entry_init and use of alloc/release
- var's of type zynqmp_r5_pdata all have same local variable name
- use dev_dbg instead of dev_info
v6:
- adding memory carveouts is handled much more similarly. All mem
carveouts are
now described in reserved memory as needed. That is, TCM nodes are not
coupled to remoteproc anymore. This is reflected in the remoteproc R5
driver
and the device tree binding.
- remove mailbox from device tree binding as it is not necessary for elf
loading
- use lockstep-mode property for configuring RPU
v7:
- remove unused headers
- change u32 *lockstep_mode -> u32 lockstep_mode;
- change device-tree binding "lockstep-mode" to xlnx,cluster-mode
- remove zynqmp_r5_mem_probe and loop to Probe R5 memory devices at
remoteproc-probe time
- remove is_r5_mode_set from zynqmp rpu remote processor private data
- do not error out if no mailbox is provided
- remove zynqmp_r5_remoteproc_probe call of platform_set_drvdata as
pdata is
handled in zynqmp_r5_remoteproc_remove
v8:
- remove old acks, reviewed-by's in commit message
v9:
- as mboxes are now optional, if pdata->tx_mc_skbs not initialized then
do not call skb_queue_empty
- update usage for zynqmp_pm_set_rpu_mode, zynqmp_pm_set_tcm_config and
zynqmp_pm_get_rpu_mode
- update 5/5 patch commit message to document supported configurations
and how they are booted by the driver.
- remove copyrights other than SPDX from zynqmp_r5_remoteproc.c
- compilation warnings no longer raised
- remove unused includes from zynqmp_r5_remoteproc.c
- remove unused var autoboot from zynqmp_r5_remoteproc.c
- reorder zynqmp_r5_pdata fpr small mem savings due to alignment
- use of zynqmp_pm_set_tcm_config now does not have
output arg
- in tcm handling, unconditionally use &= 0x000fffff mask since all nodes
in this fn are for tcm
- update comments for translating dma field in tcm handling to device
address
- update calls to rproc_mem_entry_init in parse_mem_regions so that there
are only 2 cases for types of carveouts instead of 3
- in parse_mem_regions, check if device tree node is null before using it
- add example device tree nodes used in parse_mem_regions and tcm parsing
- add comment for vring id node length
- add check for string length so that vring id is at least min length
- move tcm nodes from reserved mem to instead own device tree nodes
and only use them if enabled in device tree
- add comment for explaining handling of rproc_elf_load_rsc_table
- remove obsolete check for "if (vqid < 0)" in zynqmp_r5_rproc_kick
- remove unused field mems in struct zynqmp_r5_pdata
- remove call to zynqmp_r5_mem_probe and the fn itself as tcm handling
is done by zyqmp_r5_pm_request_tcm
- remove obsolete setting of dma_ops and parent device dma_mask
- remove obsolete use of of_dma_configure
- add comment for call to r5_set_mode fn
- make mbox usage optional and gracefully inform user via dev_dbg if not
present
- change var lockstep_mode from u32* to u32
v11:
- use enums instead of u32 where possible in zynqmp_r5_remoteproc
- update usage of zynqmp_pm_set/get_rpu_mode and zynqmp_pm_set_tcm_config
- update prints to not use carriage return, just newline
- look up tcm banks via property in r5 node instead of string name
- print device tree nodes with %pOF instead of %s with node name field
- update tcm release to unmap VA
- handle r5-1 use case
v12:
- update signed off by so that latest developer name is last
- do not cast enums to u32s for zynqmp_pm* functions
v14:
- change zynqmp_r5_remoteproc::rpus and rpu_mode to static
- fix typo
- zynqmp_r5_remoteproc::r5_set_mode set rpu mode from
property specified in device tree
- use u32 instead of u32* to store in remoteproc memory entry private data
for pnode_id information
- always call r5_set_mode on probe
- remove alloc of zynqmp_r5_pdata in
zynqmp_r5_remoteproc::zynqmp_r5_remoteproc_probe as there is static
allocation already
- error at probe time if lockstep-mode property not present in device tree
- update commit message as per review
- remove dependency on MAILBOX in makefile as ZYNQMP_IPI_MBOX is present
- remove unused macros
- update comment ordering of zynqmp_r5_pdata to match struct definition
- zynqmp_r5_remoteproc::tcm_mem_release error if pnode id is invalid
- remove obsolete TODOs
- only call zynqmp_r5_remoteproc::zynqmp_r5_probe if the index is valid
- remove uneven dev_dbg/dev_err fn calls
v15:
- if lockstep mode prop is present, then RPU cluster is in lockstep mode.
if not present, cluster is in split mode.
- if 2 RPUs provided but one is lockstep then error out as this is invalid
configuration
---
drivers/remoteproc/Kconfig | 8 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/zynqmp_r5_remoteproc.c | 769 ++++++++++++++++++++++
3 files changed, 778 insertions(+)
create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index c6659dfea7c7..68e567c5375c 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -275,6 +275,14 @@ config TI_K3_DSP_REMOTEPROC
It's safe to say N here if you're not interested in utilizing
the DSP slave processors.
+config ZYNQMP_R5_REMOTEPROC
+ tristate "ZynqMP_R5 remoteproc support"
+ depends on PM && ARCH_ZYNQMP
+ select RPMSG_VIRTIO
+ select ZYNQMP_IPI_MBOX
+ help
+ Say y or m here to support ZynqMP R5 remote processors via the remote
+ processor framework.
endif # REMOTEPROC
endmenu
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 3dfa28e6c701..ef1abff654c2 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -33,3 +33,4 @@ obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
obj-$(CONFIG_STM32_RPROC) += stm32_rproc.o
obj-$(CONFIG_TI_K3_DSP_REMOTEPROC) += ti_k3_dsp_remoteproc.o
+obj-$(CONFIG_ZYNQMP_R5_REMOTEPROC) += zynqmp_r5_remoteproc.o
diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c b/drivers/remoteproc/zynqmp_r5_remoteproc.c
new file mode 100644
index 000000000000..da74fa621810
--- /dev/null
+++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
@@ -0,0 +1,769 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Zynq R5 Remote Processor driver
+ *
+ * Based on origin OMAP and Zynq Remote Processor driver
+ *
+ */
+
+#include <linux/firmware/xlnx-zynqmp.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/list.h>
+#include <linux/mailbox_client.h>
+#include <linux/mailbox/zynqmp-ipi-message.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/platform_device.h>
+#include <linux/remoteproc.h>
+#include <linux/skbuff.h>
+#include <linux/sysfs.h>
+
+#include "remoteproc_internal.h"
+
+#define MAX_RPROCS 2 /* Support up to 2 RPU */
+#define MAX_MEM_PNODES 4 /* Max power nodes for one RPU memory instance */
+
+#define BANK_LIST_PROP "meta-memory-regions"
+
+/* IPI buffer MAX length */
+#define IPI_BUF_LEN_MAX 32U
+/* RX mailbox client buffer max length */
+#define RX_MBOX_CLIENT_BUF_MAX (IPI_BUF_LEN_MAX + \
+ sizeof(struct zynqmp_ipi_message))
+
+/**
+ * struct zynqmp_r5_mem - zynqmp rpu memory data
+ * @pnode_id: TCM power domain ids
+ * @res: memory resource
+ * @node: list node
+ */
+struct zynqmp_r5_mem {
+ u32 pnode_id[MAX_MEM_PNODES];
+ struct resource res;
+ struct list_head node;
+};
+
+/**
+ * struct zynqmp_r5_pdata - zynqmp rpu remote processor private data
+ * @rx_mc_buf: rx mailbox client buffer to save the rx message
+ * @tx_mc: tx mailbox client
+ * @rx_mc: rx mailbox client * @dev: device of RPU instance
+ * @mbox_work: mbox_work for the RPU remoteproc
+ * @tx_mc_skbs: socket buffers for tx mailbox client
+ * @dev: device of RPU instance
+ * @rproc: rproc handle
+ * @tx_chan: tx mailbox channel
+ * @rx_chan: rx mailbox channel
+ * @pnode_id: RPU CPU power domain id
+ */
+struct zynqmp_r5_pdata {
+ unsigned char rx_mc_buf[RX_MBOX_CLIENT_BUF_MAX];
+ struct mbox_client tx_mc;
+ struct mbox_client rx_mc;
+ struct work_struct mbox_work;
+ struct sk_buff_head tx_mc_skbs;
+ struct device dev;
+ struct rproc *rproc;
+ struct mbox_chan *tx_chan;
+ struct mbox_chan *rx_chan;
+ u32 pnode_id;
+};
+
+/**
+ * table of RPUs
+ */
+static struct zynqmp_r5_pdata rpus[MAX_RPROCS];
+/**
+ * RPU core configuration
+ */
+static enum rpu_oper_mode rpu_mode;
+
+/*
+ * r5_set_mode - set RPU operation mode
+ * @pdata: Remote processor private data
+ *
+ * set RPU operation mode
+ *
+ * Return: 0 for success, negative value for failure
+ */
+static int r5_set_mode(struct zynqmp_r5_pdata *pdata)
+{
+ enum rpu_tcm_comb tcm_mode;
+ enum rpu_oper_mode cur_rpu_mode;
+ int ret;
+
+ ret = zynqmp_pm_get_rpu_mode(pdata->pnode_id, &cur_rpu_mode);
+ if (ret < 0)
+ return ret;
+
+ if (rpu_mode != cur_rpu_mode) {
+ ret = zynqmp_pm_set_rpu_mode(pdata->pnode_id,
+ rpu_mode);
+ if (ret < 0)
+ return ret;
+ }
+
+ tcm_mode = (rpu_mode == PM_RPU_MODE_LOCKSTEP) ?
+ PM_RPU_TCM_COMB : PM_RPU_TCM_SPLIT;
+ return zynqmp_pm_set_tcm_config(pdata->pnode_id, tcm_mode);
+}
+
+/*
+ * ZynqMP R5 remoteproc memory release function
+ */
+static int tcm_mem_release(struct rproc *rproc, struct rproc_mem_entry *mem)
+{
+ u32 pnode_id = (u64)mem->priv;
+
+ if (pnode_id <= 0)
+ return -EINVAL;
+
+ iounmap(mem->va);
+ return zynqmp_pm_release_node(pnode_id);
+}
+
+/*
+ * ZynqMP R5 remoteproc operations
+ */
+static int zynqmp_r5_rproc_start(struct rproc *rproc)
+{
+ struct device *dev = rproc->dev.parent;
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+ enum rpu_boot_mem bootmem;
+
+ bootmem = (rproc->bootaddr & 0xF0000000) == 0xF0000000 ?
+ PM_RPU_BOOTMEM_HIVEC : PM_RPU_BOOTMEM_LOVEC;
+
+ dev_dbg(dev, "RPU boot from %s.",
+ bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
+
+ return zynqmp_pm_request_wake(pdata->pnode_id, 1,
+ bootmem, ZYNQMP_PM_REQUEST_ACK_NO);
+}
+
+static int zynqmp_r5_rproc_stop(struct rproc *rproc)
+{
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+
+ return zynqmp_pm_force_pwrdwn(pdata->pnode_id,
+ ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+}
+
+static int zynqmp_r5_rproc_mem_alloc(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ void *va;
+
+ va = ioremap_wc(mem->dma, mem->len);
+ if (IS_ERR_OR_NULL(va))
+ return -ENOMEM;
+
+ /* Update memory entry va */
+ mem->va = va;
+
+ return 0;
+}
+
+static int zynqmp_r5_rproc_mem_release(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ iounmap(mem->va);
+ return 0;
+}
+
+static int parse_mem_regions(struct rproc *rproc)
+{
+ int num_mems, i;
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+ struct device *dev = &pdata->dev;
+ struct device_node *np = dev->of_node;
+ struct rproc_mem_entry *mem;
+
+ num_mems = of_count_phandle_with_args(np, "memory-region", NULL);
+ if (num_mems <= 0)
+ return 0;
+
+ for (i = 0; i < num_mems; i++) {
+ struct device_node *node;
+ struct reserved_mem *rmem;
+
+ node = of_parse_phandle(np, "memory-region", i);
+ if (!node)
+ return -EINVAL;
+
+ rmem = of_reserved_mem_lookup(node);
+ if (!rmem)
+ return -EINVAL;
+
+ if (strstr(node->name, "vdev0vring")) {
+ int vring_id;
+ char name[16];
+
+ /*
+ * expecting form of "rpuXvdev0vringX as documented
+ * in xilinx remoteproc device tree binding
+ */
+ if (strlen(node->name) < 14) {
+ dev_err(dev, "%pOF is less than 14 chars",
+ node);
+ return -EINVAL;
+ }
+
+ /*
+ * can be 1 of multiple vring IDs per IPC channel
+ * e.g. 'vdev0vring0' and 'vdev0vring1'
+ */
+ vring_id = node->name[14] - '0';
+ snprintf(name, sizeof(name), "vdev0vring%d", vring_id);
+ /* Register vring */
+ mem = rproc_mem_entry_init(dev, NULL,
+ (dma_addr_t)rmem->base,
+ rmem->size, rmem->base,
+ zynqmp_r5_rproc_mem_alloc,
+ zynqmp_r5_rproc_mem_release,
+ name);
+ } else {
+ /* Register DMA region */
+ int (*alloc)(struct rproc *r,
+ struct rproc_mem_entry *rme);
+ int (*release)(struct rproc *r,
+ struct rproc_mem_entry *rme);
+ char name[20];
+
+ if (strstr(node->name, "vdev0buffer")) {
+ alloc = NULL;
+ release = NULL;
+ strcpy(name, "vdev0buffer");
+ } else {
+ alloc = zynqmp_r5_rproc_mem_alloc;
+ release = zynqmp_r5_rproc_mem_release;
+ strcpy(name, node->name);
+ }
+
+ mem = rproc_mem_entry_init(dev, NULL,
+ (dma_addr_t)rmem->base,
+ rmem->size, rmem->base,
+ alloc, release, name);
+ }
+ if (!mem)
+ return -ENOMEM;
+
+ rproc_add_carveout(rproc, mem);
+ }
+
+ return 0;
+}
+
+/* call Xilix Platform manager to request access to TCM bank */
+static int zyqmp_r5_pm_request_tcm(struct device_node *tcm_node,
+ struct device *dev,
+ u32 *pnode_id)
+{
+ int ret;
+
+ ret = of_property_read_u32(tcm_node, "pnode-id", pnode_id);
+ if (ret)
+ return ret;
+
+ return zynqmp_pm_request_node(*pnode_id, ZYNQMP_PM_CAPABILITY_ACCESS, 0,
+ ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+}
+
+/* Given tcm bank entry,
+ * this callback will set device address for R5 running on TCM
+ * and also setup virtual address for tcm bank remoteproc carveout
+ */
+static int tcm_mem_alloc(struct rproc *rproc,
+ struct rproc_mem_entry *mem)
+{
+ void *va;
+ struct device *dev = rproc->dev.parent;
+
+ va = ioremap_wc(mem->dma, mem->len);
+ if (IS_ERR_OR_NULL(va))
+ return -ENOMEM;
+
+ /* Update memory entry va */
+ mem->va = va;
+
+ va = devm_ioremap_wc(dev, mem->da, mem->len);
+ if (!va)
+ return -ENOMEM;
+ /* As R5 is 32 bit, wipe out extra high bits */
+ mem->da &= 0x000fffff;
+ /*
+ * handle tcm banks 1 a and b (0xffe90000 and oxffeb0000)
+ * As both of these the only common bit found not in tcm bank0 a or b
+ * is at 0x80000 use this mask to suss it out
+ */
+ if (mem->da & 0x80000)
+ /*
+ * need to do more to further translate
+ * tcm banks 1a and 1b at 0xffe90000 and oxffeb0000
+ * respectively to 0x0 and 0x20000
+ */
+ mem->da -= 0x90000;
+
+ return 0;
+}
+
+/*
+ * Given R5 node in remoteproc instance,
+ * allocate remoteproc carveout for TCM memory
+ * needed for firmware to be loaded
+ */
+static int parse_tcm_banks(struct rproc *rproc)
+{
+ int i, num_banks;
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+ struct device *dev = &pdata->dev;
+ struct device_node *r5_node = dev->of_node;
+
+ /* go through tcm banks for r5 node */
+ num_banks = of_count_phandle_with_args(r5_node, BANK_LIST_PROP, NULL);
+ if (num_banks <= 0) {
+ dev_err(dev, "need to specify TCM banks\n");
+ return -EINVAL;
+ }
+
+ for (i = 0; i < num_banks; i++) {
+ struct resource rsc;
+ resource_size_t size;
+ struct device_node *dt_node;
+ struct rproc_mem_entry *mem;
+ int ret;
+ u32 pnode_id; /* zynqmp_pm* fn's expect u32 */
+
+ dt_node = of_parse_phandle(r5_node, BANK_LIST_PROP, i);
+ if (!dt_node)
+ return -EINVAL;
+
+ if (of_device_is_available(dt_node)) {
+ ret = of_address_to_resource(dt_node, 0, &rsc);
+ if (ret < 0)
+ return ret;
+
+ ret = zyqmp_r5_pm_request_tcm(dt_node, dev, &pnode_id);
+ if (ret < 0)
+ return ret;
+
+ /* add carveout */
+ size = resource_size(&rsc);
+ mem = rproc_mem_entry_init(dev, NULL, rsc.start,
+ (int)size, rsc.start,
+ tcm_mem_alloc,
+ tcm_mem_release,
+ rsc.name);
+ if (!mem)
+ return -ENOMEM;
+
+ mem->priv = (void *)(u64)pnode_id;
+ rproc_add_carveout(rproc, mem);
+ }
+ }
+
+ return 0;
+}
+
+static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+ int ret;
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+ struct device *dev = &pdata->dev;
+
+ ret = parse_tcm_banks(rproc);
+ if (ret)
+ return ret;
+
+ ret = parse_mem_regions(rproc);
+ if (ret)
+ return ret;
+
+ ret = rproc_elf_load_rsc_table(rproc, fw);
+ if (ret == -EINVAL) {
+ /*
+ * resource table only required for IPC.
+ * if not present, this is not necessarily an error;
+ * for example, loading r5 hello world application
+ * so simply inform user and keep going.
+ */
+ dev_info(dev, "no resource table found.\n");
+ ret = 0;
+ }
+ return ret;
+}
+
+/* kick a firmware */
+static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
+{
+ struct sk_buff *skb;
+ unsigned int skb_len;
+ struct zynqmp_ipi_message *mb_msg;
+ int ret;
+
+ struct device *dev = rproc->dev.parent;
+ struct zynqmp_r5_pdata *pdata = rproc->priv;
+
+ skb_len = (unsigned int)(sizeof(vqid) + sizeof(mb_msg));
+ skb = alloc_skb(skb_len, GFP_ATOMIC);
+ if (!skb)
+ return;
+
+ mb_msg = (struct zynqmp_ipi_message *)skb_put(skb, skb_len);
+ mb_msg->len = sizeof(vqid);
+ memcpy(mb_msg->data, &vqid, sizeof(vqid));
+ skb_queue_tail(&pdata->tx_mc_skbs, skb);
+ ret = mbox_send_message(pdata->tx_chan, mb_msg);
+ if (ret < 0) {
+ dev_warn(dev, "Failed to kick remote.\n");
+ skb_dequeue_tail(&pdata->tx_mc_skbs);
+ kfree_skb(skb);
+ }
+}
+
+static struct rproc_ops zynqmp_r5_rproc_ops = {
+ .start = zynqmp_r5_rproc_start,
+ .stop = zynqmp_r5_rproc_stop,
+ .load = rproc_elf_load_segments,
+ .parse_fw = zynqmp_r5_parse_fw,
+ .find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
+ .sanity_check = rproc_elf_sanity_check,
+ .get_boot_addr = rproc_elf_get_boot_addr,
+ .kick = zynqmp_r5_rproc_kick,
+};
+
+/**
+ * zynqmp_r5_release() - ZynqMP R5 device release function
+ * @dev: pointer to the device struct of ZynqMP R5
+ *
+ * Function to release ZynqMP R5 device.
+ */
+static void zynqmp_r5_release(struct device *dev)
+{
+ struct zynqmp_r5_pdata *pdata;
+ struct rproc *rproc;
+ struct sk_buff *skb;
+
+ pdata = dev_get_drvdata(dev);
+ rproc = pdata->rproc;
+ if (rproc) {
+ rproc_del(rproc);
+ rproc_free(rproc);
+ }
+ if (pdata->tx_chan)
+ mbox_free_channel(pdata->tx_chan);
+ if (pdata->rx_chan)
+ mbox_free_channel(pdata->rx_chan);
+
+ /* Discard all SKBs if tx_mc_skbs is initialized */
+ if (&pdata->tx_mc_skbs.prev) {
+ while (!skb_queue_empty(&pdata->tx_mc_skbs)) {
+ skb = skb_dequeue(&pdata->tx_mc_skbs);
+ kfree_skb(skb);
+ }
+ }
+
+ put_device(dev->parent);
+}
+
+/**
+ * event_notified_idr_cb() - event notified idr callback
+ * @id: idr id
+ * @ptr: pointer to idr private data
+ * @data: data passed to idr_for_each callback
+ *
+ * Pass notification to remoteproc virtio
+ *
+ * Return: 0. having return is to satisfy the idr_for_each() function
+ * pointer input argument requirement.
+ **/
+static int event_notified_idr_cb(int id, void *ptr, void *data)
+{
+ struct rproc *rproc = data;
+
+ (void)rproc_vq_interrupt(rproc, id);
+ return 0;
+}
+
+/**
+ * handle_event_notified() - remoteproc notification work funciton
+ * @work: pointer to the work structure
+ *
+ * It checks each registered remoteproc notify IDs.
+ */
+static void handle_event_notified(struct work_struct *work)
+{
+ struct rproc *rproc;
+ struct zynqmp_r5_pdata *pdata;
+
+ pdata = container_of(work, struct zynqmp_r5_pdata, mbox_work);
+
+ (void)mbox_send_message(pdata->rx_chan, NULL);
+ rproc = pdata->rproc;
+ /*
+ * We only use IPI for interrupt. The firmware side may or may
+ * not write the notifyid when it trigger IPI.
+ * And thus, we scan through all the registered notifyids.
+ */
+ idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
+}
+
+/**
+ * zynqmp_r5_mb_rx_cb() - Receive channel mailbox callback
+ * @cl: mailbox client
+ * @mssg: message pointer
+ *
+ * It will schedule the R5 notification work.
+ */
+static void zynqmp_r5_mb_rx_cb(struct mbox_client *cl, void *mssg)
+{
+ struct zynqmp_r5_pdata *pdata;
+
+ pdata = container_of(cl, struct zynqmp_r5_pdata, rx_mc);
+ if (mssg) {
+ struct zynqmp_ipi_message *ipi_msg, *buf_msg;
+ size_t len;
+
+ ipi_msg = (struct zynqmp_ipi_message *)mssg;
+ buf_msg = (struct zynqmp_ipi_message *)pdata->rx_mc_buf;
+ len = (ipi_msg->len >= IPI_BUF_LEN_MAX) ?
+ IPI_BUF_LEN_MAX : ipi_msg->len;
+ buf_msg->len = len;
+ memcpy(buf_msg->data, ipi_msg->data, len);
+ }
+ schedule_work(&pdata->mbox_work);
+}
+
+/**
+ * zynqmp_r5_mb_tx_done() - Request has been sent to the remote
+ * @cl: mailbox client
+ * @mssg: pointer to the message which has been sent
+ * @r: status of last TX - OK or error
+ *
+ * It will be called by the mailbox framework when the last TX has done.
+ */
+static void zynqmp_r5_mb_tx_done(struct mbox_client *cl, void *mssg, int r)
+{
+ struct zynqmp_r5_pdata *pdata;
+ struct sk_buff *skb;
+
+ if (!mssg)
+ return;
+ pdata = container_of(cl, struct zynqmp_r5_pdata, tx_mc);
+ skb = skb_dequeue(&pdata->tx_mc_skbs);
+ kfree_skb(skb);
+}
+
+/**
+ * zynqmp_r5_setup_mbox() - Setup mailboxes
+ *
+ * @pdata: pointer to the ZynqMP R5 processor platform data
+ * @node: pointer of the device node
+ *
+ * Function to setup mailboxes to talk to RPU.
+ *
+ * Return: 0 for success, negative value for failure.
+ */
+static int zynqmp_r5_setup_mbox(struct zynqmp_r5_pdata *pdata,
+ struct device_node *node)
+{
+ struct device *dev = &pdata->dev;
+ struct mbox_client *mclient;
+
+ /* Setup TX mailbox channel client */
+ mclient = &pdata->tx_mc;
+ mclient->dev = dev;
+ mclient->rx_callback = NULL;
+ mclient->tx_block = false;
+ mclient->knows_txdone = false;
+ mclient->tx_done = zynqmp_r5_mb_tx_done;
+
+ /* Setup TX mailbox channel client */
+ mclient = &pdata->rx_mc;
+ mclient->dev = dev;
+ mclient->rx_callback = zynqmp_r5_mb_rx_cb;
+ mclient->tx_block = false;
+ mclient->knows_txdone = false;
+
+ INIT_WORK(&pdata->mbox_work, handle_event_notified);
+
+ /* Request TX and RX channels */
+ pdata->tx_chan = mbox_request_channel_byname(&pdata->tx_mc, "tx");
+ if (IS_ERR(pdata->tx_chan)) {
+ dev_err(dev, "failed to request mbox tx channel.\n");
+ pdata->tx_chan = NULL;
+ return -EINVAL;
+ }
+ pdata->rx_chan = mbox_request_channel_byname(&pdata->rx_mc, "rx");
+ if (IS_ERR(pdata->rx_chan)) {
+ dev_err(dev, "failed to request mbox rx channel.\n");
+ pdata->rx_chan = NULL;
+ return -EINVAL;
+ }
+ skb_queue_head_init(&pdata->tx_mc_skbs);
+
+ return 0;
+}
+
+/**
+ * zynqmp_r5_probe() - Probes ZynqMP R5 processor device node
+ * @pdata: pointer to the ZynqMP R5 processor platform data
+ * @pdev: parent RPU domain platform device
+ * @node: pointer of the device node
+ *
+ * Function to retrieve the information of the ZynqMP R5 device node.
+ *
+ * Return: 0 for success, negative value for failure.
+ */
+static int zynqmp_r5_probe(struct zynqmp_r5_pdata *pdata,
+ struct platform_device *pdev,
+ struct device_node *node)
+{
+ struct device *dev = &pdata->dev;
+ struct rproc *rproc;
+ int ret;
+
+ /* Create device for ZynqMP R5 device */
+ dev->parent = &pdev->dev;
+ dev->release = zynqmp_r5_release;
+ dev->of_node = node;
+ dev_set_name(dev, "%pOF", node);
+ dev_set_drvdata(dev, pdata);
+ ret = device_register(dev);
+ if (ret)
+ return ret;
+ get_device(&pdev->dev);
+
+ /* Allocate remoteproc instance */
+ rproc = rproc_alloc(dev, dev_name(dev), &zynqmp_r5_rproc_ops, NULL, 0);
+ if (!rproc) {
+ ret = -ENOMEM;
+ goto error;
+ }
+ pdata->rproc = rproc;
+ rproc->priv = pdata;
+
+ /* Set up DMA mask */
+ ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
+ if (ret)
+ goto error;
+
+ /* Get R5 power domain node */
+ ret = of_property_read_u32(node, "pnode-id", &pdata->pnode_id);
+ if (ret)
+ goto error;
+
+ ret = r5_set_mode(pdata);
+ if (ret)
+ goto error;
+
+ ret = zynqmp_r5_setup_mbox(pdata, node);
+ if (ret)
+ goto error;
+
+ /* Add R5 remoteproc */
+ ret = rproc_add(rproc);
+ if (ret)
+ goto error;
+
+ return 0;
+error:
+ if (pdata->rproc)
+ rproc_free(pdata->rproc);
+ pdata->rproc = NULL;
+ device_unregister(dev);
+ put_device(&pdev->dev);
+ return ret;
+}
+
+static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
+{
+ int ret, i = 0;
+ struct device *dev = &pdev->dev;
+ struct device_node *nc;
+
+ rpu_mode = of_get_property(dev->of_node, "lockstep-mode", NULL) ?
+ PM_RPU_MODE_LOCKSTEP : PM_RPU_MODE_SPLIT;
+
+ dev_dbg(dev, "RPU configuration: %s\n",
+ rpu_mode == PM_RPU_MODE_LOCKSTEP ? "lockstep" : "split");
+
+ for_each_available_child_of_node(dev->of_node, nc) {
+ /*
+ * if 2 RPUs provided but one is lockstep, then we have an
+ * invalid configuration.
+ */
+ if (i > 0 && rpu_mode == PM_RPU_MODE_LOCKSTEP)
+ return -EINVAL;
+
+ /* only call zynqmp_r5_probe if proper # of rpu's */
+ ret = (i < MAX_RPROCS) ? zynqmp_r5_probe(&rpus[i], pdev, nc) :
+ -EINVAL;
+ dev_dbg(dev, "%s to probe rpu %pOF\n",
+ ret ? "Failed" : "Able",
+ nc);
+
+ if (ret)
+ return ret;
+
+ i++;
+ }
+
+ return 0;
+}
+
+static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
+{
+ int i;
+
+ for (i = 0; i < MAX_RPROCS; i++) {
+ struct zynqmp_r5_pdata *pdata = &rpus[i];
+ struct rproc *rproc;
+
+ /* only do clean up for pdata with active rpu */
+ if (pdata->pnode_id == 0)
+ continue;
+
+ rproc = pdata->rproc;
+ if (rproc) {
+ rproc_del(rproc);
+ rproc_free(rproc);
+ pdata->rproc = NULL;
+ }
+ if (pdata->tx_chan) {
+ mbox_free_channel(pdata->tx_chan);
+ pdata->tx_chan = NULL;
+ }
+ if (pdata->rx_chan) {
+ mbox_free_channel(pdata->rx_chan);
+ pdata->rx_chan = NULL;
+ }
+ if (&(&pdata->dev)->dma_pools)
+ device_unregister(&pdata->dev);
+ }
+
+ return 0;
+}
+
+/* Match table for OF platform binding */
+static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
+ { .compatible = "xlnx,zynqmp-r5-remoteproc-1.0", },
+ { /* end of list */ },
+};
+MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
+
+static struct platform_driver zynqmp_r5_remoteproc_driver = {
+ .probe = zynqmp_r5_remoteproc_probe,
+ .remove = zynqmp_r5_remoteproc_remove,
+ .driver = {
+ .name = "zynqmp_r5_remoteproc",
+ .of_match_table = zynqmp_r5_remoteproc_match,
+ },
+};
+module_platform_driver(zynqmp_r5_remoteproc_driver);
+
+MODULE_AUTHOR("Ben Levinsky <[email protected]>");
+MODULE_LICENSE("GPL v2");
--
2.17.1
Add ZynqMP firmware ioctl enums for RPU configuration.
Signed-off-by: Ben Levinsky <[email protected]>
---
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
function pointer in xilinx eemi ops struct
v4:
- add default values for enums
---
include/linux/firmware/xlnx-zynqmp.h | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index 5968df82b991..bb347dfe4ba4 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -104,6 +104,10 @@ enum pm_ret_status {
};
enum pm_ioctl_id {
+ IOCTL_GET_RPU_OPER_MODE = 0,
+ IOCTL_SET_RPU_OPER_MODE = 1,
+ IOCTL_RPU_BOOT_ADDR_CONFIG = 2,
+ IOCTL_TCM_COMB_CONFIG = 3,
IOCTL_SD_DLL_RESET = 6,
IOCTL_SET_SD_TAPDELAY,
IOCTL_SET_PLL_FRAC_MODE,
@@ -129,6 +133,21 @@ enum pm_query_id {
PM_QID_CLOCK_GET_MAX_DIVISOR,
};
+enum rpu_oper_mode {
+ PM_RPU_MODE_LOCKSTEP = 0,
+ PM_RPU_MODE_SPLIT = 1,
+};
+
+enum rpu_boot_mem {
+ PM_RPU_BOOTMEM_LOVEC = 0,
+ PM_RPU_BOOTMEM_HIVEC = 1,
+};
+
+enum rpu_tcm_comb {
+ PM_RPU_TCM_SPLIT = 0,
+ PM_RPU_TCM_COMB = 1,
+};
+
enum zynqmp_pm_reset_action {
PM_RESET_ACTION_RELEASE,
PM_RESET_ACTION_ASSERT,
--
2.17.1
Add shutdown/wakeup a resource eemi operations to shutdown
or bringup a resource.
Note alignment of args matches convention of other fn's in this file.
The reason being that the long fn name results in aligned args that
otherwise go over 80 chars so shift right to avoid this
Signed-off-by: Ben Levinsky <[email protected]>
---
v3:
- add xilinx-related platform mgmt fn's instead of wrapping around
function pointer in xilinx eemi ops struct
- fix formatting
v4:
- add default values for enumv3:
- add xilinx-related platform mgmt fn's instead of wrapping around
function pointer in xilinx eemi ops struct
- fix formatting
v4:
- add default values for enum
v9:
- zynqmp_pm_force_powerdown update arg 'target' to 'node'
- zynqmp_pm_request_wakeup fix code style
v10:
- add types.h to includes for compilation
---
drivers/firmware/xilinx/zynqmp.c | 35 ++++++++++++++++++++++++++++
include/linux/firmware/xlnx-zynqmp.h | 23 ++++++++++++++++++
2 files changed, 58 insertions(+)
diff --git a/drivers/firmware/xilinx/zynqmp.c b/drivers/firmware/xilinx/zynqmp.c
index 8d1ff2454e2e..a966ee956573 100644
--- a/drivers/firmware/xilinx/zynqmp.c
+++ b/drivers/firmware/xilinx/zynqmp.c
@@ -846,6 +846,41 @@ int zynqmp_pm_release_node(const u32 node)
}
EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
+/**
+ * zynqmp_pm_force_pwrdwn - PM call to request for another PU or subsystem to
+ * be powered down forcefully
+ * @node: Node ID of the targeted PU or subsystem
+ * @ack: Flag to specify whether acknowledge is requested
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_force_pwrdwn(const u32 node,
+ const enum zynqmp_pm_request_ack ack)
+{
+ return zynqmp_pm_invoke_fn(PM_FORCE_POWERDOWN, node, ack, 0, 0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_force_pwrdwn);
+
+/**
+ * zynqmp_pm_request_wake - PM call to wake up selected master or subsystem
+ * @node: Node ID of the master or subsystem
+ * @set_addr: Specifies whether the address argument is relevant
+ * @address: Address from which to resume when woken up
+ * @ack: Flag to specify whether acknowledge requested
+ *
+ * Return: status, either success or error+reason
+ */
+int zynqmp_pm_request_wake(const u32 node,
+ const bool set_addr,
+ const u64 address,
+ const enum zynqmp_pm_request_ack ack)
+{
+ /* set_addr flag is encoded into 1st bit of address */
+ return zynqmp_pm_invoke_fn(PM_REQUEST_WAKEUP, node, address | set_addr,
+ address >> 32, ack, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_request_wake);
+
/**
* zynqmp_pm_set_requirement() - PM call to set requirement for PM slaves
* @node: Node ID of the slave
diff --git a/include/linux/firmware/xlnx-zynqmp.h b/include/linux/firmware/xlnx-zynqmp.h
index bb347dfe4ba4..6241c5ac51b3 100644
--- a/include/linux/firmware/xlnx-zynqmp.h
+++ b/include/linux/firmware/xlnx-zynqmp.h
@@ -12,6 +12,7 @@
#ifndef __FIRMWARE_ZYNQMP_H__
#define __FIRMWARE_ZYNQMP_H__
+#include <linux/types.h>
#define ZYNQMP_PM_VERSION_MAJOR 1
#define ZYNQMP_PM_VERSION_MINOR 0
@@ -64,6 +65,8 @@
enum pm_api_id {
PM_GET_API_VERSION = 1,
+ PM_FORCE_POWERDOWN = 8,
+ PM_REQUEST_WAKEUP = 10,
PM_SYSTEM_SHUTDOWN = 12,
PM_REQUEST_NODE = 13,
PM_RELEASE_NODE,
@@ -376,6 +379,12 @@ int zynqmp_pm_write_pggs(u32 index, u32 value);
int zynqmp_pm_read_pggs(u32 index, u32 *value);
int zynqmp_pm_system_shutdown(const u32 type, const u32 subtype);
int zynqmp_pm_set_boot_health_status(u32 value);
+int zynqmp_pm_force_pwrdwn(const u32 target,
+ const enum zynqmp_pm_request_ack ack);
+int zynqmp_pm_request_wake(const u32 node,
+ const bool set_addr,
+ const u64 address,
+ const enum zynqmp_pm_request_ack ack);
#else
static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
{
@@ -526,6 +535,20 @@ static inline int zynqmp_pm_set_boot_health_status(u32 value)
{
return -ENODEV;
}
+
+static inline int zynqmp_pm_force_pwrdwn(const u32 target,
+ const enum zynqmp_pm_request_ack ack)
+{
+ return -ENODEV;
+}
+
+static inline int zynqmp_pm_request_wake(const u32 node,
+ const bool set_addr,
+ const u64 address,
+ const enum zynqmp_pm_request_ack ack)
+{
+ return -ENODEV;
+}
#endif
#endif /* __FIRMWARE_ZYNQMP_H__ */
--
2.17.1
Hey Ben,
Thanks for sending out the new series, this patchset is functional for
booting both R5 0 and R5 1 in split mode.
A few comments below, still working my way through the rest of the code
though now that this works.
On Mon, Sep 21, 2020 at 09:14:06AM -0700, Ben Levinsky wrote:
<...>
> +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> +{
> + int ret, i = 0;
> + struct device *dev = &pdev->dev;
> + struct device_node *nc;
> +
> + rpu_mode = of_get_property(dev->of_node, "lockstep-mode", NULL) ?
> + PM_RPU_MODE_LOCKSTEP : PM_RPU_MODE_SPLIT;
Extra whitespace, and of_property_read_bool would read a bit nicer here
(does the same thing in the end, though).
Since rpu_mode is only used here and in r5_set_mode, I think it'd be
better to plumb it through zynqmp_r5_probe instead of making it global
in this file.
> +
> + dev_dbg(dev, "RPU configuration: %s\n",
> + rpu_mode == PM_RPU_MODE_LOCKSTEP ? "lockstep" : "split");
> +
> + for_each_available_child_of_node(dev->of_node, nc) {
> + /*
> + * if 2 RPUs provided but one is lockstep, then we have an
> + * invalid configuration.
> + */
> + if (i > 0 && rpu_mode == PM_RPU_MODE_LOCKSTEP)
> + return -EINVAL;
> +
> + /* only call zynqmp_r5_probe if proper # of rpu's */
> + ret = (i < MAX_RPROCS) ? zynqmp_r5_probe(&rpus[i], pdev, nc) :
> + -EINVAL;
> + dev_dbg(dev, "%s to probe rpu %pOF\n",
> + ret ? "Failed" : "Able",
> + nc);
It'd be cleaner to check the child node count before the loop:
rpu_nodes = of_get_available_child_count(nc)
if ((rpu_mode == PM_RPU_MODE_LOCKSTEP && rpu_nodes != 1) || rpu_nodes > 2)
return -EINVAL;
> +
> + if (ret)
> + return ret;
> +
> + i++;
> + }
> +
> + return 0;
> +}
> +
> +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> +{
> + int i;
> +
> + for (i = 0; i < MAX_RPROCS; i++) {
> + struct zynqmp_r5_pdata *pdata = &rpus[i];
> + struct rproc *rproc;
> +
> + /* only do clean up for pdata with active rpu */
> + if (pdata->pnode_id == 0)
> + continue;
This seems like a bit of a hack, resulting from the use of a static
array for holding the zynqmp_r5_pdata for each rpu.
Consider allocating zynqmp_r5_pdata in zynqmp_r5_probe, and adding each
instance to a linked-list at file scope.
- memory is only allocated RPUs actually in use
- no need for this pnode_id == 0 hack
- MAX_RPROCS can be eliminated, just traverse that list in
remove
- No reuse of the pdata across probe/removes, so all of the e.g.
condtionals below ("if (rproc)") and NULL assignments can be
eliminated.
> +
> + rproc = pdata->rproc;
> + if (rproc) {
> + rproc_del(rproc);
> + rproc_free(rproc);
> + pdata->rproc = NULL;
> + }
> + if (pdata->tx_chan) {
> + mbox_free_channel(pdata->tx_chan);
> + pdata->tx_chan = NULL;
> + }
> + if (pdata->rx_chan) {
> + mbox_free_channel(pdata->rx_chan);
> + pdata->rx_chan = NULL;
> + }
> + if (&(&pdata->dev)->dma_pools)
> + device_unregister(&pdata->dev);
The condition here looks very wrong to me, as it will always be true.
What is this trying to achieve?
> + }
> +
> + return 0;
> +}
> +
> +/* Match table for OF platform binding */
> +static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
> + { .compatible = "xlnx,zynqmp-r5-remoteproc-1.0", },
> + { /* end of list */ },
> +};
> +MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
> +
> +static struct platform_driver zynqmp_r5_remoteproc_driver = {
> + .probe = zynqmp_r5_remoteproc_probe,
> + .remove = zynqmp_r5_remoteproc_remove,
> + .driver = {
> + .name = "zynqmp_r5_remoteproc",
> + .of_match_table = zynqmp_r5_remoteproc_match,
> + },
> +};
> +module_platform_driver(zynqmp_r5_remoteproc_driver);
> +
> +MODULE_AUTHOR("Ben Levinsky <[email protected]>");
> +MODULE_LICENSE("GPL v2");
> --
> 2.17.1
>
Thanks,
Michael
Hi Michael,
> -----Original Message-----
> From: Michael Auchter <[email protected]>
> Sent: Monday, September 21, 2020 3:12 PM
> To: Ben Levinsky <[email protected]>
> Cc: [email protected]; [email protected]; Stefano Stabellini
> <[email protected]>; Michal Simek <[email protected]>;
> [email protected]; [email protected]; Ed T. Mooring
> <[email protected]>; [email protected]; linux-
> [email protected]; [email protected]; linux-arm-
> [email protected]; Jiaying Liang <[email protected]>; Michal Simek
> <[email protected]>; Ed T. Mooring <[email protected]>; Jason Wu
> <[email protected]>
> Subject: Re: [PATCH v15 5/5] remoteproc: Add initial zynqmp R5 remoteproc
> driver
>
> Hey Ben,
>
> Thanks for sending out the new series, this patchset is functional for
> booting both R5 0 and R5 1 in split mode.
>
> A few comments below, still working my way through the rest of the code
> though now that this works.
>
> On Mon, Sep 21, 2020 at 09:14:06AM -0700, Ben Levinsky wrote:
> <...>
> > +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> > +{
> > + int ret, i = 0;
> > + struct device *dev = &pdev->dev;
> > + struct device_node *nc;
> > +
> > + rpu_mode = of_get_property(dev->of_node, "lockstep-mode", NULL)
> ?
> > + PM_RPU_MODE_LOCKSTEP : PM_RPU_MODE_SPLIT;
>
> Extra whitespace, and of_property_read_bool would read a bit nicer here
> (does the same thing in the end, though).
>
> Since rpu_mode is only used here and in r5_set_mode, I think it'd be
> better to plumb it through zynqmp_r5_probe instead of making it global
> in this file.
>
[Ben Levinsky] will do
> > +
> > + dev_dbg(dev, "RPU configuration: %s\n",
> > + rpu_mode == PM_RPU_MODE_LOCKSTEP ? "lockstep" :
> "split");
> > +
> > + for_each_available_child_of_node(dev->of_node, nc) {
> > + /*
> > + * if 2 RPUs provided but one is lockstep, then we have an
> > + * invalid configuration.
> > + */
> > + if (i > 0 && rpu_mode == PM_RPU_MODE_LOCKSTEP)
> > + return -EINVAL;
> > +
> > + /* only call zynqmp_r5_probe if proper # of rpu's */
> > + ret = (i < MAX_RPROCS) ? zynqmp_r5_probe(&rpus[i], pdev,
> nc) :
> > + -EINVAL;
> > + dev_dbg(dev, "%s to probe rpu %pOF\n",
> > + ret ? "Failed" : "Able",
> > + nc);
>
> It'd be cleaner to check the child node count before the loop:
>
> rpu_nodes = of_get_available_child_count(nc)
> if ((rpu_mode == PM_RPU_MODE_LOCKSTEP && rpu_nodes != 1) ||
> rpu_nodes > 2)
> return -EINVAL;
>
[Ben Levinsky] will do
> > +
> > + if (ret)
> > + return ret;
> > +
> > + i++;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> > +{
> > + int i;
> > +
> > + for (i = 0; i < MAX_RPROCS; i++) {
> > + struct zynqmp_r5_pdata *pdata = &rpus[i];
> > + struct rproc *rproc;
> > +
> > + /* only do clean up for pdata with active rpu */
> > + if (pdata->pnode_id == 0)
> > + continue;
>
> This seems like a bit of a hack, resulting from the use of a static
> array for holding the zynqmp_r5_pdata for each rpu.
>
> Consider allocating zynqmp_r5_pdata in zynqmp_r5_probe, and adding each
> instance to a linked-list at file scope.
> - memory is only allocated RPUs actually in use
> - no need for this pnode_id == 0 hack
> - MAX_RPROCS can be eliminated, just traverse that list in
> remove
> - No reuse of the pdata across probe/removes, so all of the e.g.
> condtionals below ("if (rproc)") and NULL assignments can be
> eliminated.
>
[Ben Levinsky] so parts of this I can do..
- can make the rpus a static list of ptr's which I think is equivalent
To what you are describing
- can eliminate the pnode_id == 0 hack
For the rproc_del, rproc_free fn calls, these should stay. Just as other upstream remoteproc drivers do, this is being done similarly.
For mbox handling, I am mimic'ing upstream ST and TI drivers https://github.com/torvalds/linux/blob/v5.9-rc3/drivers/remoteproc/stm32_rproc.c
they similarly check if the mbox channel is not NULL, and if so call mbox_free_channel. This is similar for Xilinx remoteproc R5 use case as the mbox can be unused in 1 remoteproc node. Also, similar to TI and ST driver, https://github.com/torvalds/linux/blob/v5.9-rc3/drivers/remoteproc/stm32_rproc.c#L321 , I am setting the mbox to NULL at remove
> > +
> > + rproc = pdata->rproc;
> > + if (rproc) {
> > + rproc_del(rproc);
> > + rproc_free(rproc);
> > + pdata->rproc = NULL;
> > + }
> > + if (pdata->tx_chan) {
> > + mbox_free_channel(pdata->tx_chan);
> > + pdata->tx_chan = NULL;
> > + }
> > + if (pdata->rx_chan) {
> > + mbox_free_channel(pdata->rx_chan);
> > + pdata->rx_chan = NULL;
> > + }
> > + if (&(&pdata->dev)->dma_pools)
> > + device_unregister(&pdata->dev);
>
> The condition here looks very wrong to me, as it will always be true.
> What is this trying to achieve?
>
This was originally because of the static rpu declaration. By instead using ptr's this can be removed as the zynqmp_r5_pdata ptr will be NULL so I can check that instead. So will remove this.
Thank you for the review
Ben
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/* Match table for OF platform binding */
> > +static const struct of_device_id zynqmp_r5_remoteproc_match[] = {
> > + { .compatible = "xlnx,zynqmp-r5-remoteproc-1.0", },
> > + { /* end of list */ },
> > +};
> > +MODULE_DEVICE_TABLE(of, zynqmp_r5_remoteproc_match);
> > +
> > +static struct platform_driver zynqmp_r5_remoteproc_driver = {
> > + .probe = zynqmp_r5_remoteproc_probe,
> > + .remove = zynqmp_r5_remoteproc_remove,
> > + .driver = {
> > + .name = "zynqmp_r5_remoteproc",
> > + .of_match_table = zynqmp_r5_remoteproc_match,
> > + },
> > +};
> > +module_platform_driver(zynqmp_r5_remoteproc_driver);
> > +
> > +MODULE_AUTHOR("Ben Levinsky <[email protected]>");
> > +MODULE_LICENSE("GPL v2");
> > --
> > 2.17.1
> >
>
> Thanks,
> Michael