Re: [PATCH v13 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver
Hi Ben,
Thanks for fixing the commit ordering issue. Some additional comments
below.
Ben Levinsky writes:
> 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 or lock-step.
Suggest following edit for clarity -
"A Cortex R5 is included in the Xilinx Zync UltraScale MPSoC. Adding a
remoteproc driver, it's possible to boot the R5 sub-system in two
different 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
> Signed-off-by: Michal Simek
> Signed-off-by: Ed Mooring
> Signed-off-by: Jason Wu
> Signed-off-by: Ben Levinsky
> ---
The patch fails to apply on v5.9-rc3. What kernel are the patches based
on? Please rebase and test before sending another update.
[...]
> drivers/remoteproc/Kconfig| 10 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/zynqmp_r5_remoteproc.c | 898 ++
> 3 files changed, 909 insertions(+)
> create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index c4d1731295eb..dd9ed45654e0 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -249,6 +249,16 @@ config STM32_RPROC
>
> This can be either built-in or a loadable module.
>
> +config ZYNQMP_R5_REMOTEPROC
> + tristate "ZynqMP_R5 remoteproc support"
> + depends on ARM64 && PM && ARCH_ZYNQMP
The ARM64 dependency seems superfluous. Please drop it.
> + select RPMSG_VIRTIO
> + select MAILBOX
ZYNQMP_IPI_MBOX below implies MAILBOX. I think it is safe to drop
MAILBOX as well.
> + 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 e8b886e511f0..04d1c95d06d7 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -28,5 +28,6 @@ obj-$(CONFIG_QCOM_WCNSS_PIL)+=
> qcom_wcnss_pil.o
> qcom_wcnss_pil-y += qcom_wcnss.o
> qcom_wcnss_pil-y += qcom_wcnss_iris.o
> obj-$(CONFIG_ST_REMOTEPROC) += st_remoteproc.o
> +obj-$(CONFIG_ZYNQMP_R5_REMOTEPROC) += zynqmp_r5_remoteproc.o
> obj-$(CONFIG_ST_SLIM_REMOTEPROC) += st_slim_rproc.o
> obj-$(CONFIG_STM32_RPROC)+= stm32_rproc.o
> diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c
> b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> new file mode 100644
> index ..4fc4098ae1ea
> --- /dev/null
> +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> @@ -0,0 +1,898 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Zynq R5 Remote Processor driver
> + *
> + * Based on origin OMAP and Zynq Remote Processor driver
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#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 DEFAULT_FIRMWARE_NAME"rproc-rpu-fw"
This define doesn't seem to be used in the driver. Please drop it.
> +#define BANK_LIST_PROP "meta-memory-regions"
> +
> +/* PM proc states */
> +#define PM_PROC_STATE_ACTIVE 1U
Same for this macro as well.
> +
> +/* 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
> + * @dev: device of RPU instance
> + * @rproc: rproc handle
> + * @pnode_id: RPU CPU power domain id
> + * @is_r5_mode_set: indicate if r5 operation mode is set
> + * @tx_mc: tx mailbox client
> + * @rx_mc: rx mailbox client
> + * @tx_chan: tx mailbox channel
> + * @rx_chan: rx mailbox channel
> + * @mbox_work: mbox_work for the RPU remoteproc
> + * @tx_mc_skbs: socket buffers for tx mailbox client
> + * @rx_mc_buf: rx mailbox client buffer to save the rx message
> + */
> +struct zynqmp_r5_pdata {
> + unsigned char
Re: [PATCH v13 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver
Hello Ben, On Fri, Sep 04, 2020 at 07:32:09AM -0700, Ben Levinsky wrote: > 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 or 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 > Signed-off-by: Michal Simek > Signed-off-by: Ed Mooring > Signed-off-by: Jason Wu > Signed-off-by: Ben Levinsky > --- > 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 &= 0x000f 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 > --- > drivers/remoteproc/Kconfig| 10 + > drivers/remoteproc/Makefile | 1 + >
[PATCH v13 5/5] remoteproc: Add initial zynqmp R5 remoteproc driver
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 or 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 Signed-off-by: Michal Simek Signed-off-by: Ed Mooring Signed-off-by: Jason Wu Signed-off-by: Ben Levinsky --- 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 &= 0x000f 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 --- drivers/remoteproc/Kconfig| 10 + drivers/remoteproc/Makefile | 1 + drivers/remoteproc/zynqmp_r5_remoteproc.c | 898 ++ 3 files changed, 909 insertions(+) create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig index c4d1731295eb..dd9ed45654e0 100644 --- a/drivers/remoteproc/Kconfig +++
