Skip to contentSkip to conversation

Provide basic driver to control Arm R5 co-processor found on Xilinx ZynqMP

PATCH Superseded Ben Levinsky 5 participants 11 messages Sep 22, 2020 – Nov 5, 2020

Subsystems (3)

v16 of 26 Superseded Latest is v26 →

lore
Share & tools
lore
#0 lore raw

v16 cover Provide basic driver to control Arm R5 co-processor found on Xilinx ZynqMP

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     | 777 ++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h          |  60 ++
 6 files changed, 1062 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
 create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
Signature · 1 line
2.17.1
#1 lore raw

PATCH v16 1/5 firmware: xilinx: Add ZynqMP firmware ioctl enums for RPU configuration.

Add ZynqMP firmware ioctl enums for RPU configuration.

---
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(+)
include/linux/firmware/xlnx-zynqmp.h +19 -0
index 5968df82b991..bb347dfe4ba4 100644
@@ -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,
Signature · 1 line
2.17.1
#2 lore raw

PATCH v16 2/5 firmware: xilinx: Add shutdown/wakeup APIs

In reply to Ben Levinsky

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

---
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(+)
drivers/firmware/xilinx/zynqmp.c +35 -0
index 8d1ff2454e2e..a966ee956573 100644
@@ -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
include/linux/firmware/xlnx-zynqmp.h +23 -0
index bb347dfe4ba4..6241c5ac51b3 100644
@@ -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__ */
Signature · 1 line
2.17.1
#3 lore raw

PATCH v16 3/5 firmware: xilinx: Add RPU configuration APIs

In reply to Ben Levinsky

This patch adds APIs to access to configure RPU and its processor-specific memory.

That is query the run-time mode of RPU as either split or lockstep as well as API to set this mode. In addition add APIs to access configuration of the RPUs' tightly coupled memory (TCM).

---
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
v9:
- update commit message
- for zynqmp_pm_set_tcm_config and zynqmp_pm_get_rpu_mode update docs for expected output, arguments as well removing unused args
- remove unused fn zynqmp_pm_get_node_status
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 v12:
- 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
- zynqmp_pm_get_rpu_mode argument zynqmp_pm_get_rpu_mode is
only set if no error

---
 drivers/firmware/xilinx/zynqmp.c     | 61 ++++++++++++++++++++++++++++
 include/linux/firmware/xlnx-zynqmp.h | 18 ++++++++
 2 files changed, 79 insertions(+)
drivers/firmware/xilinx/zynqmp.c +61 -0
index a966ee956573..b390a00338d0 100644
@@ -846,6 +846,67 @@ int zynqmp_pm_release_node(const u32 node)
 }
 EXPORT_SYMBOL_GPL(zynqmp_pm_release_node);
 
+/**
+ * zynqmp_pm_get_rpu_mode() - Get RPU mode
+ * @node_id:	Node ID of the device
+ * @rpu_mode:	return by reference value
+ *		either split or lockstep
+ *
+ * Return:	return 0 on success or error+reason.
+ *		if success, then  rpu_mode will be set
+ *		to current rpu mode.
+ */
+int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode)
+{
+	u32 ret_payload[PAYLOAD_ARG_CNT];
+	int ret;
+
+	ret = zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				  IOCTL_GET_RPU_OPER_MODE, 0, 0, ret_payload);
+
+	/* only set rpu_mode if no error */
+	if (ret == XST_PM_SUCCESS)
+		*rpu_mode = ret_payload[0];
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_get_rpu_mode);
+
+/**
+ * zynqmp_pm_set_rpu_mode() - Set RPU mode
+ * @node_id:	Node ID of the device
+ * @rpu_mode:	Argument 1 to requested IOCTL call. either split or lockstep
+ *
+ *		This function is used to set RPU mode to split or
+ *		lockstep
+ *
+ * Return:	Returns status, either success or error+reason
+ */
+int zynqmp_pm_set_rpu_mode(u32 node_id, enum rpu_oper_mode rpu_mode)
+{
+	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				   IOCTL_SET_RPU_OPER_MODE, (u32)rpu_mode,
+				   0, NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_rpu_mode);
+
+/**
+ * zynqmp_pm_set_tcm_config - configure TCM
+ * @tcm_mode:	Argument 1 to requested IOCTL call
+ *              either PM_RPU_TCM_COMB or PM_RPU_TCM_SPLIT
+ *
+ * This function is used to set RPU mode to split or combined
+ *
+ * Return: status: 0 for success, else failure
+ */
+int zynqmp_pm_set_tcm_config(u32 node_id, enum rpu_tcm_comb tcm_mode)
+{
+	return zynqmp_pm_invoke_fn(PM_IOCTL, node_id,
+				   IOCTL_TCM_COMB_CONFIG, (u32)tcm_mode, 0,
+				   NULL);
+}
+EXPORT_SYMBOL_GPL(zynqmp_pm_set_tcm_config);
+
 /**
  * zynqmp_pm_force_pwrdwn - PM call to request for another PU or subsystem to
  *             be powered down forcefully
include/linux/firmware/xlnx-zynqmp.h +18 -0
index 6241c5ac51b3..79aa2fcbcd54 100644
@@ -385,6 +385,9 @@ int zynqmp_pm_request_wake(const u32 node,
 			   const bool set_addr,
 			   const u64 address,
 			   const enum zynqmp_pm_request_ack ack);
+int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode);
+int zynqmp_pm_set_rpu_mode(u32 node_id, u32 arg1);
+int zynqmp_pm_set_tcm_config(u32 node_id, u32 arg1);
 #else
 static inline struct zynqmp_eemi_ops *zynqmp_pm_get_eemi_ops(void)
 {
@@ -549,6 +552,21 @@ static inline int zynqmp_pm_request_wake(const u32 node,
 {
 	return -ENODEV;
 }
+
+static inline int zynqmp_pm_get_rpu_mode(u32 node_id, enum rpu_oper_mode *rpu_mode)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_set_rpu_mode(u32 node_id, u32 arg1)
+{
+	return -ENODEV;
+}
+
+static inline int zynqmp_pm_set_tcm_config(u32 node_id, u32 arg1)
+{
+	return -ENODEV;
+}
 #endif
 
 #endif /* __FIRMWARE_ZYNQMP_H__ */
Signature · 1 line
2.17.1
#4 lore raw

PATCH v16 4/5 dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

In reply to Ben Levinsky

Add binding for ZynqMP R5 OpenAMP.

Represent the RPU domain resources in one device node. Each RPU processor is a subnode of the top RPU domain node.

---
v3:
- 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::
- 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
v6:
- remove coupling TCM nodes with remoteproc
- remove mailbox as it is optional not needed
v7:
- change lockstep-mode to xlnx,cluster-mode
v9:
- show example IPC nodes and tcm bank nodes
v11:
- add property meta-memory-regions to illustrate link
between r5 and TCM banks
- update so no warnings from 'make dt_binding_check'
v14:
- 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
- place IPC nodes in RAM in the reserved memory section
v15:
- change lockstep-mode prop as follows: if present, then RPU cluster is in lockstep mode. if not present, cluster is in split mode.

---
 .../xilinx,zynqmp-r5-remoteproc.yaml          | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +120 -0
new file mode 100644
index 000000000000..ce02e425692e
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that loads and
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory (TCM)
+  so the absolute address ranges are provided in TCM reg's.
+maintainers:
+  - Ed Mooring <[email protected]>
+  - Ben Levinsky <[email protected]>
+
+properties:
+  compatible:
+    const: "xlnx,zynqmp-r5-remoteproc-1.0"
+
+  lockstep-mode:
+    description:
+      If this property is present, then the configuration is lock-step.
+      Otherwise RPU is split.
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt mapping for remoteproc IPI. It is required if the
+      user uses the remoteproc driver with the RPMsg kernel driver.
+    maxItems: 6
+
+  memory-region:
+    description:
+      collection of memory carveouts used for elf-loading and inter-processor
+      communication.
+    maxItems: 4
+    minItems: 4
+  meta-memory-regions:
+    description:
+      collection of memories that are not present in the top level memory
+      nodes' mapping. For example, R5s' TCM banks. These banks are needed
+      for R5 firmware meta data such as the R5 firmware's heap and stack
+  pnode-id:
+    maxItems: 1
+  mboxes:
+    maxItems: 2
+  mbox-names:
+    maxItems: 2
+
+examples:
+  - |
+     reserved-memory {
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          elf_load: rproc@3ed000000 {
+               no-map;
+               reg = <0x3ed00000 0x40000>;
+          };
+
+          rpu0vdev0vring0: rpu0vdev0vring0@3ed40000 {
+               no-map;
+               reg = <0x3ed40000 0x4000>;
+          };
+          rpu0vdev0vring1: rpu0vdev0vring1@3ed44000 {
+               no-map;
+               reg = <0x3ed44000 0x4000>;
+          };
+          rpu0vdev0buffer: rpu0vdev0buffer@3ed48000 {
+               no-map;
+               reg = <0x3ed48000 0x100000>;
+          };
+
+     };
+
+     /*
+      * Below nodes are required if using TCM to load R5 firmware
+      * if not, then either do not provide nodes are label as disabled in
+      * status property
+      */
+     tcm0a: tcm_0a@ffe00000 {
+         reg = <0xffe00000 0x10000>;
+         pnode-id = <0xf>;
+         no-map;
+         status = "okay";
+         phandle = <0x40>;
+         compatible = "xlnx,tcm";
+     };
+     tcm0b: tcm_1a@ffe20000 {
+         reg = <0xffe20000 0x10000>;
+         pnode-id = <0x10>;
+         no-map;
+         status = "okay";
+         compatible = "xlnx,tcm";
+         phandle = <0x41>;
+     };
+
+     rpu {
+          compatible = "xlnx,zynqmp-r5-remoteproc-1.0";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          lockstep-mode;
+          r5_0 {
+               ranges;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               memory-region = <&elf_load>,
+                               <&rpu0vdev0vring0>,
+                               <&rpu0vdev0vring1>,
+                               <&rpu0vdev0buffer>;
+               meta-memory-regions = <&tcm_0a>, <&tcm_0b>;
+               pnode-id = <0x7>;
+          };
+     };
+
+...
Signature · 1 line
2.17.1
maintainer
#5 lore raw

Re: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

Quoted 57 lines · Ben Levinsky

Add binding for ZynqMP R5 OpenAMP.

Represent the RPU domain resources in one device node. Each RPU processor is a subnode of the top RPU domain node.

---
v3:
- 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::
- 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
v6:
- remove coupling TCM nodes with remoteproc
- remove mailbox as it is optional not needed
v7:
- change lockstep-mode to xlnx,cluster-mode
v9:
- show example IPC nodes and tcm bank nodes
v11:
- add property meta-memory-regions to illustrate link
between r5 and TCM banks
- update so no warnings from 'make dt_binding_check'
v14:
- 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
- place IPC nodes in RAM in the reserved memory section
v15:
- change lockstep-mode prop as follows: if present, then RPU cluster is in lockstep mode. if not present, cluster is in split mode.

---
 .../xilinx,zynqmp-r5-remoteproc.yaml          | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +4 -0
new file mode 100644
index 000000000000..ce02e425692e
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml#"

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +10 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that loads and
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory (TCM)
+  so the absolute address ranges are provided in TCM reg's.

blank line needed.

TCMs specifically I'm concerned about how they are represented in system DT and here...

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 21

Quoted 7 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +7 -0
+maintainers:
+  - Ed Mooring <[email protected]>
+  - Ben Levinsky <[email protected]>
+
+properties:
+  compatible:
+    const: "xlnx,zynqmp-r5-remoteproc-1.0"

Don't need quotes.

The use of version numbers was allowed for Xilinx programmable IP. I don't think this falls into that category.

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 25

Quoted 4 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +4 -0
+
+  lockstep-mode:
+    description:
+      If this property is present, then the configuration is lock-step.

boolean...

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 27

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +2 -0
+      Otherwise RPU is split.
+    maxItems: 1

...or an array?

Either way, doesn't work for TI R5.

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 40

Quoted 13 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +13 -0
+
+  interrupts:
+    description:
+      Interrupt mapping for remoteproc IPI. It is required if the
+      user uses the remoteproc driver with the RPMsg kernel driver.
+    maxItems: 6
+
+  memory-region:
+    description:
+      collection of memory carveouts used for elf-loading and inter-processor
+      communication.
+    maxItems: 4
+    minItems: 4

Need to define what each region is.

One blank line between properties.

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 45

Quoted 5 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +5 -0
+  meta-memory-regions:
+    description:
+      collection of memories that are not present in the top level memory
+      nodes' mapping. For example, R5s' TCM banks. These banks are needed
+      for R5 firmware meta data such as the R5 firmware's heap and stack

Humm, needs a better explanation.

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 47

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +2 -0
+  pnode-id:
+    maxItems: 1

What's this?

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 49

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +2 -0
+  mboxes:
+    maxItems: 2

Need to define what each one is.

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 51

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +2 -0
+  mbox-names:
+    maxItems: 2

Need to define the names.

Quoted 29 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +29 -0
+
+examples:
+  - |
+     reserved-memory {
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          elf_load: rproc@3ed000000 {
+               no-map;
+               reg = <0x3ed00000 0x40000>;
+          };
+
+          rpu0vdev0vring0: rpu0vdev0vring0@3ed40000 {
+               no-map;
+               reg = <0x3ed40000 0x4000>;
+          };
+          rpu0vdev0vring1: rpu0vdev0vring1@3ed44000 {
+               no-map;
+               reg = <0x3ed44000 0x4000>;
+          };
+          rpu0vdev0buffer: rpu0vdev0buffer@3ed48000 {
+               no-map;
+               reg = <0x3ed48000 0x100000>;
+          };
+
+     };
+
+     /*
+      * Below nodes are required if using TCM to load R5 firmware

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 90

Quoted 10 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +10 -0
+      * if not, then either do not provide nodes are label as disabled in
+      * status property
+      */
+     tcm0a: tcm_0a@ffe00000 {
+         reg = <0xffe00000 0x10000>;
+         pnode-id = <0xf>;
+         no-map;
+         status = "okay";
+         phandle = <0x40>;
+         compatible = "xlnx,tcm";

TCM is a Xilinx specific thing?

Quoted 33 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +30 -0
+     };
+     tcm0b: tcm_1a@ffe20000 {
+         reg = <0xffe20000 0x10000>;
+         pnode-id = <0x10>;
+         no-map;
+         status = "okay";
+         compatible = "xlnx,tcm";
+         phandle = <0x41>;
+     };
+
+     rpu {
+          compatible = "xlnx,zynqmp-r5-remoteproc-1.0";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          lockstep-mode;
+          r5_0 {
+               ranges;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               memory-region = <&elf_load>,
+                               <&rpu0vdev0vring0>,
+                               <&rpu0vdev0vring1>,
+                               <&rpu0vdev0buffer>;
+               meta-memory-regions = <&tcm_0a>, <&tcm_0b>;
+               pnode-id = <0x7>;
+          };
+     };
+
+...
Signature · 1 line
2.17.1
#6 lore raw

RE: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

Hi Rob,

Quoted 85 lines

-----Original Message-----
From: Rob Herring <[email protected]>
Sent: Tuesday, September 29, 2020 11:36 AM
To: Ben Levinsky <[email protected]>
Cc: Stefano Stabellini <[email protected]>; Michal Simek <[email protected]>; [email protected]; [email protected]; [email protected]; Ed T. Mooring <[email protected]>; linux- [email protected]; [email protected]; linux-arm- [email protected]; Jason Wu <[email protected]>; Jiaying Liang <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

Quoted 42 lines · Ben Levinsky

Add binding for ZynqMP R5 OpenAMP.

Represent the RPU domain resources in one device node. Each RPU processor is a subnode of the top RPU domain node.

---
v3:
- 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::
- 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
v6:
- remove coupling TCM nodes with remoteproc
- remove mailbox as it is optional not needed
v7:
- change lockstep-mode to xlnx,cluster-mode
v9:
- show example IPC nodes and tcm bank nodes
v11:
- add property meta-memory-regions to illustrate link
between r5 and TCM banks
- update so no warnings from 'make dt_binding_check'
v14:
- 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
- place IPC nodes in RAM in the reserved memory section
v15:
- change lockstep-mode prop as follows: if present, then RPU cluster is in lockstep mode. if not present, cluster is in split mode.

Quoted 4 lines
---
 .../xilinx,zynqmp-r5-remoteproc.yaml          | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- remoteproc.yaml

Review

Quoted 2 lines
(quoted diff) +0 -0

r5-remoteproc.yaml b/Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- remoteproc.yaml

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-

Quoted 4 lines

new file mode 100644
index 000000000000..ce02e425692e

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +0 -0

remoteproc.yaml

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 4

Quoted 5 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +4 -0
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-

remoteproc.yaml#"

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 10

Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +6 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that

loads and

Quoted 8 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 13

Quoted 3 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +3 -0
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory

(TCM)

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 14

Quoted 1 line
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +1 -0
+  so the absolute address ranges are provided in TCM reg's.

blank line needed.

will fix in next rev.

Quoted 3 lines

TCMs specifically I'm concerned about how they are represented in system DT and here...

So System DT can tie in with lopper plugin/assists so that TCMs are output to whatever the linux driver expects.

Quoted 10 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 21

Quoted 7 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +7 -0
+maintainers:
+  - Ed Mooring <[email protected]>
+  - Ben Levinsky <[email protected]>
+
+properties:
+  compatible:
+    const: "xlnx,zynqmp-r5-remoteproc-1.0"

Don't need quotes.

will fix in next rev.

Quoted 3 lines

The use of version numbers was allowed for Xilinx programmable IP. I don't think this falls into that category.

will fix in next rev.

Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 25

Quoted 4 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +4 -0
+
+  lockstep-mode:
+    description:
+      If this property is present, then the configuration is lock-step.

boolean...

By this comment do you mean you want this to change or mention that it is implicitly Boolean?

Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 27

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+      Otherwise RPU is split.
+    maxItems: 1

...or an array?

Either way, doesn't work for TI R5.

So as the configuration for both the TI R5 and Xilinx R5 is independent what in common would you like to see here?
For example Xilinx driver can similarly have the "Xilinx,cluster-mode" or "cluster-mode" but for Xilinx platform manager our split configuration value in Xilinx platform manager is '0' while in TI drver it is '1'. So I can make the driver expect it then translate if needed if this what you prefer.

Otherwise how would you suggest the Xilinx r5 remoteproc driver describe split/lockstep mode?

Quoted 16 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 40

Quoted 13 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +13 -0
+
+  interrupts:
+    description:
+      Interrupt mapping for remoteproc IPI. It is required if the
+      user uses the remoteproc driver with the RPMsg kernel driver.
+    maxItems: 6
+
+  memory-region:
+    description:
+      collection of memory carveouts used for elf-loading and inter-processor
+      communication.
+    maxItems: 4
+    minItems: 4

Need to define what each region is.

Quoted 2 lines

One blank line between properties.

will fix in next rev. for memory-regions is the following ok?
collection of phandles for memory carveouts for elf-loading and
inter-processor communication.

This collection can consist of reserved-mem carveouts in DDR.

Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 45

Quoted 5 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +5 -0
+  meta-memory-regions:
+    description:
+      collection of memories that are not present in the top level memory
+      nodes' mapping. For example, R5s' TCM banks. These banks are needed
+      for R5 firmware meta data such as the R5 firmware's heap and stack

Humm, needs a better explanation.

How is the following:
Collection of phandles for reserved on-chip SRAM regions.

Otherwise I can get rid of this property and combine into memory-region if you wish.

Quoted 6 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 47

Quoted 3 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  pnode-id:
+    maxItems: 1

What's this?

Can add the following:
power node id that is used to uniquely identify the RPU for Xilinx
Power Management. The value is then passed to Xilinx platform manager for power on/off and access.

Quoted 5 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 49

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  mboxes:
+    maxItems: 2

Need to define what each one is.

How is the following:
array of phandles that describe the rx and tx for xilinx zynqmp
mailbox driver. order of rx and tx is described by the mbox-names property. This will be used for communication with remote processor.

Quoted 5 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 51

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  mbox-names:
+    maxItems: 2

Need to define the names.

How is the following:
array of strings that denote which item in the mboxes property array
are the rx and tx for xilinx zynqmp mailbox driver

Quoted 33 lines
Quoted 33 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +33 -0
+
+examples:
+  - |
+     reserved-memory {
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          elf_load: rproc@3ed000000 {
+               no-map;
+               reg = <0x3ed00000 0x40000>;
+          };
+
+          rpu0vdev0vring0: rpu0vdev0vring0@3ed40000 {
+               no-map;
+               reg = <0x3ed40000 0x4000>;
+          };
+          rpu0vdev0vring1: rpu0vdev0vring1@3ed44000 {
+               no-map;
+               reg = <0x3ed44000 0x4000>;
+          };
+          rpu0vdev0buffer: rpu0vdev0buffer@3ed48000 {
+               no-map;
+               reg = <0x3ed48000 0x100000>;
+          };
+
+     };
+
+     /*
+      * Below nodes are required if using TCM to load R5 firmware
+      * if not, then either do not provide nodes are label as disabled in
+      * status property
+      */
+     tcm0a: tcm_0a@ffe00000 {
Quoted 9 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 90

Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +6 -0
+         reg = <0xffe00000 0x10000>;
+         pnode-id = <0xf>;
+         no-map;
+         status = "okay";
+         phandle = <0x40>;
+         compatible = "xlnx,tcm";

TCM is a Xilinx specific thing?

No... good point. Can remove the compatible string in next rev.

Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +25 -0
+     };
+     tcm0b: tcm_1a@ffe20000 {
+         reg = <0xffe20000 0x10000>;
+         pnode-id = <0x10>;
+         no-map;
+         status = "okay";
+         compatible = "xlnx,tcm";
+         phandle = <0x41>;
+     };
+
+     rpu {
+          compatible = "xlnx,zynqmp-r5-remoteproc-1.0";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          lockstep-mode;
+          r5_0 {
+               ranges;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               memory-region = <&elf_load>,
+                               <&rpu0vdev0vring0>,
+                               <&rpu0vdev0vring1>,
+                               <&rpu0vdev0buffer>;
+               meta-memory-regions = <&tcm_0a>, <&tcm_0b>;
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +5 -1
+               pnode-id = <0x7>;
+          };
+     };
+
+...
--

2.17.1

Thank you,
Ben

maintainer
#7 lore raw

Re: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

Quoted 94 lines · Ben Levinsky

Hi Rob,

Quoted 91 lines

-----Original Message-----
From: Rob Herring <[email protected]>
Sent: Tuesday, September 29, 2020 11:36 AM
To: Ben Levinsky <[email protected]>
Cc: Stefano Stabellini <[email protected]>; Michal Simek <[email protected]>; [email protected]; [email protected]; [email protected]; Ed T. Mooring <[email protected]>; linux- [email protected]; [email protected]; linux-arm- [email protected]; Jason Wu <[email protected]>; Jiaying Liang <[email protected]>; Michal Simek <[email protected]>
Subject: Re: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

Quoted 42 lines · Ben Levinsky

Add binding for ZynqMP R5 OpenAMP.

Represent the RPU domain resources in one device node. Each RPU processor is a subnode of the top RPU domain node.

---
v3:
- 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::
- 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
v6:
- remove coupling TCM nodes with remoteproc
- remove mailbox as it is optional not needed
v7:
- change lockstep-mode to xlnx,cluster-mode
v9:
- show example IPC nodes and tcm bank nodes
v11:
- add property meta-memory-regions to illustrate link
between r5 and TCM banks
- update so no warnings from 'make dt_binding_check'
v14:
- 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
- place IPC nodes in RAM in the reserved memory section
v15:
- change lockstep-mode prop as follows: if present, then RPU cluster is in lockstep mode. if not present, cluster is in split mode.

Quoted 4 lines
---
 .../xilinx,zynqmp-r5-remoteproc.yaml          | 120 ++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- remoteproc.yaml

Review

Quoted 2 lines
(quoted diff) +0 -0

r5-remoteproc.yaml b/Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- remoteproc.yaml

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-

Quoted 4 lines

new file mode 100644
index 000000000000..ce02e425692e

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +0 -0

remoteproc.yaml

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 4

Quoted 5 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +4 -0
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-

remoteproc.yaml#"

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 10

Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +6 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that

loads and

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 13

Quoted 3 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +3 -0
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory

(TCM)

Quoted 1 line
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +1 -0
+  so the absolute address ranges are provided in TCM reg's.
Quoted 7 lines
Quoted 2 lines

blank line needed.

will fix in next rev.

Quoted 3 lines

TCMs specifically I'm concerned about how they are represented in system DT and here...

So System DT can tie in with lopper plugin/assists so that TCMs are output to whatever the linux driver expects.

Sorry, I don't buy lopper can handle it. That leaves too much hand waving IMO.

Quoted 15 lines
Quoted 10 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 21

Quoted 7 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +7 -0
+maintainers:
+  - Ed Mooring <[email protected]>
+  - Ben Levinsky <[email protected]>
+
+properties:
+  compatible:
+    const: "xlnx,zynqmp-r5-remoteproc-1.0"

Don't need quotes.

will fix in next rev.

Quoted 3 lines

The use of version numbers was allowed for Xilinx programmable IP. I don't think this falls into that category.

will fix in next rev.

Quoted 8 lines
Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 25

Quoted 4 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +4 -0
+
+  lockstep-mode:
+    description:
+      If this property is present, then the configuration is lock-step.

boolean...

By this comment do you mean you want this to change or mention that it is implicitly Boolean?

You defining this as a boolean and then using a schema that applies to arrays (maxItems).

Quoted 9 lines
Quoted 7 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 27

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+      Otherwise RPU is split.
+    maxItems: 1

...or an array?

Either way, doesn't work for TI R5.

So as the configuration for both the TI R5 and Xilinx R5 is independent what in common would you like to see here?
For example Xilinx driver can similarly have the "Xilinx,cluster-mode" or "cluster-mode" but for Xilinx platform manager our split configuration value in Xilinx platform manager is '0' while in TI drver it is '1'. So I can make the driver expect it then translate if needed if this what you prefer.

What's Xilinx platform manager? The drivers are irrelevant for the binding.

Quoted 2 lines

Otherwise how would you suggest the Xilinx r5 remoteproc driver describe split/lockstep mode?

For what's clearly the same functionality, I want the binding the same. TI can have more modes if that's what they need.

Quoted 16 lines
Quoted 16 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 40

Quoted 13 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +13 -0
+
+  interrupts:
+    description:
+      Interrupt mapping for remoteproc IPI. It is required if the
+      user uses the remoteproc driver with the RPMsg kernel driver.
+    maxItems: 6
+
+  memory-region:
+    description:
+      collection of memory carveouts used for elf-loading and inter-processor
+      communication.
+    maxItems: 4
+    minItems: 4

Need to define what each region is.

Quoted 7 lines
Quoted 2 lines

One blank line between properties.

will fix in next rev. for memory-regions is the following ok?
collection of phandles for memory carveouts for elf-loading and
inter-processor communication.

This collection can consist of reserved-mem carveouts in DDR.

No.

items:
- description: This is the 1st entry...
- description: This is the 2nd entry...
...

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +5 -0
+  meta-memory-regions:
+    description:
+      collection of memories that are not present in the top level memory
+      nodes' mapping. For example, R5s' TCM banks. These banks are needed
+      for R5 firmware meta data such as the R5 firmware's heap and stack
Quoted 6 lines
Quoted 1 line

Humm, needs a better explanation.

How is the following:
Collection of phandles for reserved on-chip SRAM regions.

Otherwise I can get rid of this property and combine into memory-region if you wish.

Not sure really.

Quoted 9 lines
Quoted 5 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 47

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  pnode-id:
+    maxItems: 1

What's this?

Can add the following:
power node id that is used to uniquely identify the RPU for Xilinx
Power Management. The value is then passed to Xilinx platform manager for power on/off and access.

Sounds like you should be using the power-domain binding.

Quoted 10 lines
Quoted 5 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 49

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  mboxes:
+    maxItems: 2

Need to define what each one is.

How is the following:
array of phandles that describe the rx and tx for xilinx zynqmp
mailbox driver. order of rx and tx is described by the mbox-names property. This will be used for communication with remote processor.

Quoted 8 lines
Quoted 5 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 51

Quoted 2 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +2 -0
+  mbox-names:
+    maxItems: 2

Need to define the names.

How is the following:
array of strings that denote which item in the mboxes property array
are the rx and tx for xilinx zynqmp mailbox driver

Terrible. We have a schema to define constraints.

items:
- const: rx
- const: tx

Quoted 42 lines
Quoted 33 lines
Quoted 33 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +33 -0
+
+examples:
+  - |
+     reserved-memory {
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          elf_load: rproc@3ed000000 {
+               no-map;
+               reg = <0x3ed00000 0x40000>;
+          };
+
+          rpu0vdev0vring0: rpu0vdev0vring0@3ed40000 {
+               no-map;
+               reg = <0x3ed40000 0x4000>;
+          };
+          rpu0vdev0vring1: rpu0vdev0vring1@3ed44000 {
+               no-map;
+               reg = <0x3ed44000 0x4000>;
+          };
+          rpu0vdev0buffer: rpu0vdev0buffer@3ed48000 {
+               no-map;
+               reg = <0x3ed48000 0x100000>;
+          };
+
+     };
+
+     /*
+      * Below nodes are required if using TCM to load R5 firmware
+      * if not, then either do not provide nodes are label as disabled in
+      * status property
+      */
+     tcm0a: tcm_0a@ffe00000 {
Quoted 9 lines

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- line 90

Quoted 6 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +6 -0
+         reg = <0xffe00000 0x10000>;
+         pnode-id = <0xf>;
+         no-map;
+         status = "okay";
+         phandle = <0x40>;
+         compatible = "xlnx,tcm";

TCM is a Xilinx specific thing?

Quoted 1 line

No... good point. Can remove the compatible string in next rev.

Well, it should probably have some sort of compatible so we know what the thing is...

If we have a DT for the R5 side or view of the system, we'd need to define TCMs, right? For the A cores, it shouldn't look any different other than perhaps the address. So, you first need a TCM binding. Maybe that ends up being just mmio-sram binding, maybe not. Probably not if there's power controls.

Quoted 37 lines
Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +25 -0
+     };
+     tcm0b: tcm_1a@ffe20000 {
+         reg = <0xffe20000 0x10000>;
+         pnode-id = <0x10>;
+         no-map;
+         status = "okay";
+         compatible = "xlnx,tcm";
+         phandle = <0x41>;
+     };
+
+     rpu {
+          compatible = "xlnx,zynqmp-r5-remoteproc-1.0";
+          #address-cells = <1>;
+          #size-cells = <1>;
+          ranges;
+          lockstep-mode;
+          r5_0 {
+               ranges;
+               #address-cells = <1>;
+               #size-cells = <1>;
+               memory-region = <&elf_load>,
+                               <&rpu0vdev0vring0>,
+                               <&rpu0vdev0vring1>,
+                               <&rpu0vdev0buffer>;
+               meta-memory-regions = <&tcm_0a>, <&tcm_0b>;
Quoted 8 lines
Quoted 8 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5- new file +5 -1
+               pnode-id = <0x7>;
+          };
+     };
+
+...
--

2.17.1

Thank you,
Ben

#8 lore raw

Re: [PATCH v16 4/5] dt-bindings: remoteproc: Add documentation for ZynqMP R5 rproc bindings

In reply to Rob Herring
Quoted 21 lines · Rob Herring
Quoted 8 lines

index 000000000000..ce02e425692e

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +4 -0
@@ -0,0 +1,120 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml#"

Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml line 14

Quoted 10 lines
Documentation/devicetree/bindings/remoteproc/xilinx,zynqmp-r5-remoteproc.yaml new file +10 -0
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Xilinx R5 remote processor controller bindings
+
+description:
+  This document defines the binding for the remoteproc component that loads and
+  boots firmwares on the Xilinx Zynqmp and Versal family chipset.
+
+  Note that the Linux has global addressing view of the R5-related memory (TCM)
+  so the absolute address ranges are provided in TCM reg's.

blank line needed.

Quoted 2 lines

TCMs specifically I'm concerned about how they are represented in system DT and here...

So far I have been keeping the TCMs in system DT as regular nodes under /amba. E.g.:

		tcm: tcm@ffe00000 {
			compatible = "mmio-sram";
			reg = <0x0 0xffe00000 0x0 0x10000>;
		};

(I am not sure if "mmio-sram" is the right compatible.)

#9 lore raw

PATCH v16 5/5 remoteproc: Add initial zynqmp R5 remoteproc driver

In reply to Ben Levinsky

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.

---
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
v16:
- replace of_get_property(dev->of_node, "lockstep-mode" with of_property_read_bool
- propagate rpu mode specified in device tree through functions instead of holding a global, static var
- check child remoteproc nodes via of_get_available_child_count before looping through children
- replace check of "pdata->pnode_id == 0" instead by checking rpu's zynqmp_r5_pdata* if NULL
- remove old, obsolete checks for dma_pools in zynqmp_r5_remoteproc_remove
- change rpus from zynqmp_r5_pdata[] to zynqmp_r5_pdata*[] so that check for pdata->pnode_id == 0 is not needed

---
 drivers/remoteproc/Kconfig                |   8 +
 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/zynqmp_r5_remoteproc.c | 777 ++++++++++++++++++++++
 3 files changed, 786 insertions(+)
 create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
drivers/remoteproc/Kconfig +8 -0
index c6659dfea7c7..68e567c5375c 100644
@@ -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
drivers/remoteproc/Makefile +1 -0
index 3dfa28e6c701..ef1abff654c2 100644
@@ -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
drivers/remoteproc/zynqmp_r5_remoteproc.c new file +777 -0

777 lines not on this page · raw message

Signature · 1 line
2.17.1
#10 lore raw

Re: [PATCH v16 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver

Thank you for the patch! Perhaps something to improve:

Hi Ben,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v5.9-rc7 next-20200930] [cannot apply to xlnx/master linux/master]
[If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Ben-Levinsky/Provide-basic-driver-to-control-Arm-R5-co-processor-found-on-Xilinx-ZynqMP/20200923-064055
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm64-randconfig-r005-20200930 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project bcd05599d0e53977a963799d6ee4f6e0bc21331b)

reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install arm64 cross compiling tool for clang build
# apt-get install binutils-aarch64-linux-gnu
# https://github.com/0day-ci/linux/commit/d3c4821ffb9b295e80f07fd712322efa657a95d9
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Ben-Levinsky/Provide-basic-driver-to-control-Arm-R5-co-processor-found-on-Xilinx-ZynqMP/20200923-064055 git checkout d3c4821ffb9b295e80f07fd712322efa657a95d9
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm64

If you fix the issue, kindly add following tag as appropriate

All warnings (new ones prefixed by >>):

>> drivers/remoteproc/zynqmp_r5_remoteproc.c:630:2: warning: variable 'dev' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!pdata) {
           ^~~~~~~~~~~
   include/linux/compiler.h:56:28: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:58:30: note: expanded from macro '__trace_if_var'
   #define __trace_if_var(cond) (__builtin_constant_p(cond) ? (cond) : __trace_if_value(cond))
                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/remoteproc/zynqmp_r5_remoteproc.c:686:20: note: uninitialized use occurs here
           device_unregister(dev);
                             ^~~
   drivers/remoteproc/zynqmp_r5_remoteproc.c:630:2: note: remove the 'if' if its condition is always false
           if (!pdata) {
           ^~~~~~~~~~~~~
   include/linux/compiler.h:56:23: note: expanded from macro 'if'
   #define if(cond, ...) if ( __trace_if_var( !!(cond , ## __VA_ARGS__) ) )
                         ^
   drivers/remoteproc/zynqmp_r5_remoteproc.c:625:20: note: initialize the variable 'dev' to silence this warning
           struct device *dev;
                             ^
                              = NULL

1 warning generated.

vim +630 drivers/remoteproc/zynqmp_r5_remoteproc.c

610
611 /**
612 * zynqmp_r5_probe() - Probes ZynqMP R5 processor device node
613 * @pdata: pointer to the ZynqMP R5 processor platform data
614 * @pdev: parent RPU domain platform device
615 * @node: pointer of the device node
616 *
617 * Function to retrieve the information of the ZynqMP R5 device node.
618 *
619 * Return: 0 for success, negative value for failure.
620 */
621 static int zynqmp_r5_probe(struct zynqmp_r5_pdata *pdata,
622 struct platform_device *pdev,
623 struct device_node *node)
624 {
625 struct device *dev;
626 struct rproc *rproc;
627 int ret;
628
629 pdata = kzalloc(sizeof(*pdata), GFP_KERNEL);

 > 630		if (!pdata) {

631 ret = -ENOMEM;
632 goto error;
633 }
634 dev = &pdata->dev;
635
636 /* Create device for ZynqMP R5 device */
637 dev->parent = &pdev->dev;
638 dev->release = zynqmp_r5_release;
639 dev->of_node = node;
640 dev_set_name(dev, "%pOF", node);
641 dev_set_drvdata(dev, pdata);
642 ret = device_register(dev);
643 if (ret)
644 goto error;
645
646 /* Allocate remoteproc instance */
647 rproc = rproc_alloc(dev, dev_name(dev), &zynqmp_r5_rproc_ops, NULL, sizeof(*pdata));
648 if (!rproc) {
649 ret = -ENOMEM;
650 goto error;
651 }
652 pdata->rproc = rproc;
653 rproc->priv = pdata;
654
655 /* Set up DMA mask */
656 ret = dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
657 if (ret)
658 goto error;
659
660 /* Get R5 power domain node */
661 ret = of_property_read_u32(node, "pnode-id", &pdata->pnode_id);
662 if (ret)
663 goto error;
664
665 ret = r5_set_mode(pdata);
666 if (ret)
667 goto error;
668
669 if (of_property_read_bool(node, "mboxes")) {
670 ret = zynqmp_r5_setup_mbox(pdata, node);
671 if (ret)
672 goto error;
673 }
674
675 /* Add R5 remoteproc */
676 ret = rproc_add(rproc);
677 if (ret)
678 goto error;
679
680 platform_set_drvdata(pdev, rproc);
681 return 0;
682 error:
683 if (pdata->rproc)
684 rproc_free(pdata->rproc);
685 pdata->rproc = NULL;
686 device_unregister(dev);
687 return ret;
688 }
689

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org