[6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc

Commit Message

Jiaying Liang Aug. 16, 2018, 7:06 a.m. UTC

There are cortex-r5 processors in Xilinx Zynq UltraScale+
MPSoC platforms. This remoteproc driver is to manage the
R5 processors.

Signed-off-by: Wendy Liang <[e-mail]>
---
 drivers/remoteproc/Kconfig                |   9 +
 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/zynqmp_r5_remoteproc.c | 692 ++++++++++++++++++++++++++++++
 3 files changed, 702 insertions(+)
 create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c

Comments

Wendy Liang Aug. 24, 2018, 4:26 p.m. UTC | #1

Ping, any comments to the driver?
On Thu, Aug 16, 2018 at 3:17 AM Wendy Liang <[e-mail]> wrote:
>
> There are cortex-r5 processors in Xilinx Zynq UltraScale+
> MPSoC platforms. This remoteproc driver is to manage the
> R5 processors.
>
> Signed-off-by: Wendy Liang <[e-mail]>
> ---
>  drivers/remoteproc/Kconfig                |   9 +
>  drivers/remoteproc/Makefile               |   1 +
>  drivers/remoteproc/zynqmp_r5_remoteproc.c | 692 ++++++++++++++++++++++++++++++
>  3 files changed, 702 insertions(+)
>  create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index cd1c168..83aac63 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -158,6 +158,15 @@ config ST_REMOTEPROC
>  config ST_SLIM_REMOTEPROC
>         tristate
>
> +config ZYNQMP_R5_REMOTEPROC
> +       tristate "ZynqMP_r5 remoteproc support"
> +       depends on ARM64 && PM && ARCH_ZYNQMP
> +       select RPMSG_VIRTIO
> +       select ZYNQMP_FIRMWARE
> +       help
> +         Say y 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 02627ed..147923c 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -23,3 +23,4 @@ qcom_wcnss_pil-y                      += qcom_wcnss.o
>  qcom_wcnss_pil-y                       += qcom_wcnss_iris.o
>  obj-$(CONFIG_ST_REMOTEPROC)            += st_remoteproc.o
>  obj-$(CONFIG_ST_SLIM_REMOTEPROC)       += st_slim_rproc.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 0000000..7fc3718
> --- /dev/null
> +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> @@ -0,0 +1,692 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Zynq R5 Remote Processor driver
> + *
> + * Copyright (C) 2015 Xilinx, Inc.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/remoteproc.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/slab.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/list.h>
> +#include <linux/genalloc.h>
> +#include <linux/pfn.h>
> +#include <linux/idr.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +
> +#include "remoteproc_internal.h"
> +
> +/* IPI reg offsets */
> +#define TRIG_OFFSET            0x00000000
> +#define OBS_OFFSET             0x00000004
> +#define ISR_OFFSET             0x00000010
> +#define IMR_OFFSET             0x00000014
> +#define IER_OFFSET             0x00000018
> +#define IDR_OFFSET             0x0000001C
> +#define IPI_ALL_MASK           0x0F0F0301
> +
> +/* RPU IPI mask */
> +#define RPU_IPI_INIT_MASK      0x00000100
> +#define RPU_IPI_MASK(n)                (RPU_IPI_INIT_MASK << (n))
> +#define RPU_0_IPI_MASK         RPU_IPI_MASK(0)
> +#define RPU_1_IPI_MASK         RPU_IPI_MASK(1)
> +
> +/* PM proc states */
> +#define PM_PROC_STATE_ACTIVE 1u
> +
> +/* Maximum TCM power nodes IDs */
> +#define MAX_TCM_PNODES 4
> +
> +/* Register access macros */
> +#define reg_read(base, reg) \
> +       readl(((void __iomem *)(base)) + (reg))
> +#define reg_write(base, reg, val) \
> +       writel((val), ((void __iomem *)(base)) + (reg))
> +
> +#define DEFAULT_FIRMWARE_NAME  "rproc-rpu-fw"
> +
> +static bool autoboot __read_mostly;
> +
> +struct zynqmp_r5_rproc_pdata;
> +
> +/**
> + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor instance state
> + * @rproc: rproc handle
> + * @workqueue: workqueue for the RPU remoteproc
> + * @ipi_base: virt ptr to IPI channel address registers for APU
> + * @rpu_mode: RPU core configuration
> + * @rpu_id: RPU CPU id
> + * @rpu_pnode_id: RPU CPU power domain id
> + * @mem_pools: list of gen_pool for firmware mmio_sram memory and their
> + *             power domain IDs
> + * @mems: list of rproc_mem_entries for firmware
> + * @irq: IRQ number
> + * @ipi_dest_mask: IPI destination mask for the IPI channel
> + */
> +struct zynqmp_r5_rproc_pdata {
> +       struct rproc *rproc;
> +       struct work_struct workqueue;
> +       void __iomem *ipi_base;
> +       enum rpu_oper_mode rpu_mode;
> +       struct list_head mems;
> +       u32 ipi_dest_mask;
> +       u32 rpu_id;
> +       u32 rpu_pnode_id;
> +       int irq;
> +       u32 tcm_pnode_id[MAX_TCM_PNODES];
> +};
> +
> +/**
> + * r5_boot_addr_config - configure the boot address of R5
> + * @pdata: platform data
> + * @bootmem: boot from LOVEC or HIVEC
> + *
> + * This function will set the RPU boot address
> + */
> +static void r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> +                               enum rpu_boot_mem bootmem)
> +{
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +       pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> +                __func__, pdata->rpu_id, bootmem);
> +
> +       if (!eemi || !eemi->ioctl) {
> +               pr_err("%s: no eemi ioctl operation.\n", __func__);
> +               return;
> +       }
> +       eemi->ioctl(pdata->rpu_pnode_id, IOCTL_RPU_BOOT_ADDR_CONFIG,
> +                   bootmem, 0, NULL);
> +}
> +
> +/**
> + * r5_mode_config - configure R5 operation mode
> + * @pdata: platform data
> + *
> + * configure R5 to split mode or lockstep mode
> + * based on the platform data.
> + */
> +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +       pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> +
> +       if (!eemi || !eemi->ioctl) {
> +               pr_err("%s: no eemi ioctl operation.\n", __func__);
> +               return;
> +       }
> +       eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> +                   pdata->rpu_mode, 0, NULL);
> +}
> +
> +/**
> + * r5_release_tcm() - release TCM
> + * @pdata: platform data
> + *
> + * Release TCM
> + */
> +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +       int i;
> +
> +       if (!eemi || !eemi->release_node) {
> +               pr_err("Failed to release TCM\n");
> +               return;
> +       }
> +
> +       for (i = 0; i < MAX_TCM_PNODES; i++) {
> +               if (pdata->tcm_pnode_id[i] != 0)
> +                       eemi->release_node(pdata->tcm_pnode_id[i]);
> +       }
> +}
> +
> +/**
> + * disable_ipi - disable IPI
> + * @pdata: platform data
> + *
> + * Disable IPI interrupt
> + */
> +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       /* Disable R5 IPI interrupt */
> +       if (pdata->ipi_base)
> +               reg_write(pdata->ipi_base, IDR_OFFSET, pdata->ipi_dest_mask);
> +}
> +
> +/**
> + * enable_ipi - enable IPI
> + * @pdata: platform data
> + *
> + * Enable IPI interrupt
> + */
> +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       /* Enable R5 IPI interrupt */
> +       if (pdata->ipi_base)
> +               reg_write(pdata->ipi_base, IER_OFFSET, pdata->ipi_dest_mask);
> +}
> +
> +/**
> + * 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;
> +}
> +
> +static void handle_event_notified(struct work_struct *work)
> +{
> +       struct rproc *rproc;
> +       struct zynqmp_r5_rproc_pdata *local;
> +
> +       local = container_of(work, struct zynqmp_r5_rproc_pdata, workqueue);
> +       rproc = local->rproc;
> +       idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
> +}
> +
> +static int zynqmp_r5_rproc_start(struct rproc *rproc)
> +{
> +       struct device *dev = rproc->dev.parent;
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +       enum rpu_boot_mem bootmem;
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +       dev_dbg(dev, "%s\n", __func__);
> +
> +       if (!eemi || !eemi->force_powerdown ||
> +           !eemi->request_wakeup) {
> +               pr_err("Failed to start R5\n");
> +               return -ENXIO;
> +       }
> +
> +       /* Set up R5 */
> +       if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> +               bootmem = PM_RPU_BOOTMEM_HIVEC;
> +       else
> +               bootmem = PM_RPU_BOOTMEM_LOVEC;
> +       dev_info(dev, "RPU boot from %s.",
> +                bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
> +
> +       r5_mode_config(local);
> +       eemi->force_powerdown(local->rpu_pnode_id,
> +                             ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +       r5_boot_addr_config(local, bootmem);
> +       /* Add delay before release from halt and reset */
> +       usleep_range(400, 500);
> +       eemi->request_wakeup(local->rpu_pnode_id,
> +                            1, bootmem,
> +                            ZYNQMP_PM_REQUEST_ACK_NO);
> +
> +       /* Make sure IPI is enabled */
> +       enable_ipi(local);
> +
> +       return 0;
> +}
> +
> +/* kick a firmware */
> +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
> +{
> +       struct device *dev = rproc->dev.parent;
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +       dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", vqid);
> +
> +       /*
> +        * send irq to R5 firmware
> +        * Currently vqid is not used because we only got one.
> +        */
> +       if (local->ipi_base)
> +               reg_write(local->ipi_base, TRIG_OFFSET, local->ipi_dest_mask);
> +}
> +
> +/* power off the remote processor */
> +static int zynqmp_r5_rproc_stop(struct rproc *rproc)
> +{
> +       struct device *dev = rproc->dev.parent;
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +       struct rproc_mem_entry *mem, *nmem;
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +       dev_dbg(dev, "%s\n", __func__);
> +
> +       if (!eemi || !eemi->force_powerdown) {
> +               pr_err("Failed to stop R5\n");
> +               return -ENXIO;
> +       }
> +
> +       disable_ipi(local);
> +       eemi->force_powerdown(local->rpu_pnode_id,
> +                             ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +
> +       return 0;
> +}
> +
> +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> +{
> +       struct rproc_mem_entry *mem;
> +       void *va = NULL;
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +       list_for_each_entry(mem, &local->mems, node) {
> +               int offset = da - mem->da;
> +
> +               /* try next carveout if da is too small */
> +               if (offset < 0)
> +                       continue;
> +
> +               /* try next carveout if da is too large */
> +               if (offset + len > mem->len)
> +                       continue;
> +
> +               va = mem->va + offset;
> +
> +               break;
> +       }
> +       return va;
> +}
> +
> +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> +       int ret;
> +
> +       ret = rproc_elf_load_rsc_table(rproc, fw);
> +       if (ret == -EINVAL)
> +               /* No resource table */
> +               return 0;
> +       else
> +               return ret;
> +}
> +
> +static struct rproc_ops zynqmp_r5_rproc_ops = {
> +       .start          = zynqmp_r5_rproc_start,
> +       .stop           = zynqmp_r5_rproc_stop,
> +       .kick           = zynqmp_r5_rproc_kick,
> +       .da_to_va       = zynqmp_r5_rproc_da_to_va,
> +};
> +
> +/* Release R5 from reset and make it halted.
> + * In case the firmware uses TCM, in order to load firmware to TCM,
> + * will need to release R5 from reset and stay in halted state.
> + */
> +static int zynqmp_r5_rproc_init(struct rproc *rproc)
> +{
> +       struct device *dev = rproc->dev.parent;
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +       dev_dbg(dev, "%s\n", __func__);
> +       enable_ipi(local);
> +       return 0;
> +}
> +
> +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id)
> +{
> +       struct device *dev = dev_id;
> +       struct platform_device *pdev = to_platform_device(dev);
> +       struct rproc *rproc = platform_get_drvdata(pdev);
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +       u32 ipi_reg;
> +
> +       /* Check if there is a kick from R5 */
> +       ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> +       if (!(ipi_reg & local->ipi_dest_mask))
> +               return IRQ_NONE;
> +
> +       dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n", irq);
> +       reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> +       schedule_work(&local->workqueue);
> +
> +       return IRQ_HANDLED;
> +}
> +
> +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries for TCM memories.
> + */
> +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> +                             struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       int num_mems = 0;
> +       int i, ret;
> +       struct property *prop;
> +       const __be32 *cur;
> +       u32 val;
> +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +       /* Get TCM power node ids */
> +       i = 0;
> +       of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> +               pdata->tcm_pnode_id[i++] = val;
> +
> +       /* Request TCMs */
> +       for (i = 0; i < MAX_TCM_PNODES; i++) {
> +               if (pdata->tcm_pnode_id[i] != 0) {
> +                       ret = eemi->request_node(pdata->tcm_pnode_id[i],
> +                                                ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> +                                                ZYNQMP_PM_REQUEST_ACK_BLOCKING
> +                                               );
> +                       if (ret < 0) {
> +                               dev_err(dev, "failed to request TCM: %u\n",
> +                                       pdata->tcm_pnode_id[i]);
> +                               return ret;
> +                       }
> +                       dev_dbg(dev, "request tcm pnode: %u\n",
> +                               pdata->tcm_pnode_id[i]);
> +               } else {
> +                       break;
> +               }
> +       }
> +       /* Create remoteproc memories entries for TCM memories */
> +       num_mems = ARRAY_SIZE(mem_names);
> +       for (i = 0; i < num_mems; i++) {
> +               struct resource *res;
> +               struct rproc_mem_entry *mem;
> +               dma_addr_t dma;
> +               resource_size_t size;
> +
> +               res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +                                                  mem_names[i]);
> +               mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +                                   GFP_KERNEL);
> +               if (!mem)
> +                       return -ENOMEM;
> +               /* Map it as normal memory */
> +               size = resource_size(res);
> +               mem->va = devm_ioremap_wc(dev, res->start, size);
> +               mem->len = size;
> +               dma = (dma_addr_t)res->start;
> +               mem->dma = dma;
> +               /* TCM memory:
> +                *   TCM_0: da 0 <-> global addr 0xFFE00000
> +                *   TCM_1: da 0 <-> global addr 0xFFE90000
> +                */
> +               if ((dma & 0xFFF00000) == 0xFFE00000) {
> +                       if ((dma & 0xFFF80000) == 0xFFE80000)
> +                               mem->da -= 0x90000;
> +                       else
> +                               mem->da = (dma & 0x000FFFFF);
> +               }
> +               dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +                       __func__, mem->va, mem->da, mem->dma);
> +               list_add_tail(&mem->node, &pdata->mems);
> +       }
> +       return 0;
> +}
> +
> +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries from memory-region
> + * property.
> + */
> +static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
> +                                      struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +       struct device *dev = &pdev->dev;
> +       struct device_node *np = dev->of_node;
> +       int num_mems;
> +       int i;
> +
> +       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 resource res;
> +               resource_size_t size;
> +               struct rproc_mem_entry *mem;
> +               int ret;
> +
> +               node = of_parse_phandle(np, "memory-region", i);
> +               ret = of_device_is_compatible(node, "rproc-prog-memory");
> +               if (!ret) {
> +                       /* it is DMA memory. */
> +                       dev_info(dev, "%s, dma memory %d\n", __func__, i);
> +                       ret = of_reserved_mem_device_init_by_idx(dev,
> +                                                                np, i);
> +                       if (ret) {
> +                               dev_err(dev, "unable to reserve DMA mem.\n");
> +                               return ret;
> +                       }
> +                       continue;
> +               }
> +               ret = of_address_to_resource(node, 0, &res);
> +               if (ret) {
> +                       dev_err(dev, "unable to resolve memory region.\n");
> +                       return ret;
> +               }
> +               mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +                                   GFP_KERNEL);
> +               if (!mem)
> +                       return -ENOMEM;
> +               /* Map it as normal memory */
> +               size = resource_size(&res);
> +               mem->va = devm_ioremap_wc(dev, res.start, size);
> +               mem->len = size;
> +               mem->dma = (dma_addr_t)res.start;
> +               mem->da = (u32)res.start;
> +               dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +                       __func__, mem->va, mem->da, mem->dma);
> +               list_add_tail(&mem->node, &pdata->mems);
> +       }
> +       return 0;
> +}
> +
> +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> +{
> +       const unsigned char *prop;
> +       struct resource *res;
> +       int ret = 0;
> +       struct zynqmp_r5_rproc_pdata *local;
> +       struct rproc *rproc;
> +
> +       rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> +                           &zynqmp_r5_rproc_ops, NULL,
> +               sizeof(struct zynqmp_r5_rproc_pdata));
> +       if (!rproc) {
> +               dev_err(&pdev->dev, "rproc allocation failed\n");
> +               return -ENOMEM;
> +       }
> +       local = rproc->priv;
> +       local->rproc = rproc;
> +
> +       platform_set_drvdata(pdev, rproc);
> +
> +       /* Override parse_fw op to allow no resource table firmware */
> +       rproc->ops->parse_fw = zynqmp_r5_parse_fw;
> +
> +       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +       if (ret) {
> +               dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n", ret);
> +               goto rproc_fault;
> +       }
> +
> +       /* Get the RPU power domain id */
> +       ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> +                                  &local->rpu_pnode_id);
> +       if (ret) {
> +               dev_err(&pdev->dev, "No RPU power node ID is specified.\n");
> +               ret = -EINVAL;
> +               goto rproc_fault;
> +       }
> +       dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> +               local->rpu_id, local->rpu_pnode_id);
> +
> +       prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> +       if (!prop) {
> +               dev_warn(&pdev->dev, "default core_conf used: lock-step\n");
> +               prop = "lock-step";
> +       }
> +
> +       dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
> +       if (!strcmp(prop, "split0")) {
> +               local->rpu_mode = PM_RPU_MODE_SPLIT;
> +               local->rpu_id = 0;
> +               local->ipi_dest_mask = RPU_0_IPI_MASK;
> +       } else if (!strcmp(prop, "split1")) {
> +               local->rpu_mode = PM_RPU_MODE_SPLIT;
> +               local->rpu_id = 1;
> +               local->ipi_dest_mask = RPU_1_IPI_MASK;
> +       } else if (!strcmp(prop, "lock-step")) {
> +               local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> +               local->rpu_id = 0;
> +               local->ipi_dest_mask = RPU_0_IPI_MASK;
> +       } else {
> +               dev_err(&pdev->dev, "Invalid core_conf mode provided - %s , %d\n",
> +                       prop, local->rpu_mode);
> +               ret = -EINVAL;
> +               goto rproc_fault;
> +       }
> +
> +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipi");
> +       if (res) {
> +               local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> +                                              resource_size(res));
> +               if (IS_ERR(local->ipi_base)) {
> +                       pr_err("%s: Unable to map IPI\n", __func__);
> +                       ret = PTR_ERR(local->ipi_base);
> +                       goto rproc_fault;
> +               }
> +       } else {
> +               dev_info(&pdev->dev, "IPI resource is not specified.\n");
> +       }
> +       dev_dbg(&pdev->dev, "got ipi base address\n");
> +
> +       INIT_LIST_HEAD(&local->mems);
> +       /* Get TCM memories */
> +       ret = zynqmp_r5_get_tcms(pdev, local);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "failed to get TCM memories.\n");
> +               goto rproc_fault;
> +       }
> +       dev_dbg(&pdev->dev, "got TCM memories\n");
> +       /* Get reserved memory regions for firmware */
> +       ret = zynqmp_r5_get_reserved_mems(pdev, local);
> +       if (ret < 0) {
> +               dev_err(&pdev->dev, "failed to get reserved memories.\n");
> +               goto rproc_fault;
> +       }
> +       dev_dbg(&pdev->dev, "got reserved memories.\n");
> +
> +       /* Disable IPI before requesting IPI IRQ */
> +       disable_ipi(local);
> +       INIT_WORK(&local->workqueue, handle_event_notified);
> +
> +       /* IPI IRQ */
> +       if (local->ipi_base) {
> +               ret = platform_get_irq(pdev, 0);
> +               if (ret < 0) {
> +                       dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> +                       goto rproc_fault;
> +               }
> +               local->irq = ret;
> +               ret = devm_request_irq(&pdev->dev, local->irq,
> +                                      r5_remoteproc_interrupt, IRQF_SHARED,
> +                                      dev_name(&pdev->dev), &pdev->dev);
> +               if (ret) {
> +                       dev_err(&pdev->dev, "IRQ %d already allocated\n",
> +                               local->irq);
> +                       goto rproc_fault;
> +               }
> +               dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> +       }
> +
> +       ret = zynqmp_r5_rproc_init(local->rproc);
> +       if (ret) {
> +               dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> +               goto rproc_fault;
> +       }
> +
> +       rproc->auto_boot = autoboot;
> +
> +       ret = rproc_add(local->rproc);
> +       if (ret) {
> +               dev_err(&pdev->dev, "rproc registration failed\n");
> +               goto rproc_fault;
> +       }
> +
> +       return ret;
> +
> +rproc_fault:
> +       rproc_free(local->rproc);
> +
> +       return ret;
> +}
> +
> +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> +{
> +       struct rproc *rproc = platform_get_drvdata(pdev);
> +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +       struct rproc_mem_entry *mem;
> +
> +       dev_info(&pdev->dev, "%s\n", __func__);
> +
> +       rproc_del(rproc);
> +
> +       list_for_each_entry(mem, &local->mems, node) {
> +               if (mem->priv)
> +                       gen_pool_free((struct gen_pool *)mem->priv,
> +                                     (unsigned long)mem->va, mem->len);
> +       }
> +
> +       r5_release_tcm(local);
> +       of_reserved_mem_device_release(&pdev->dev);
> +       rproc_free(rproc);
> +
> +       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_param_named(autoboot,  autoboot, bool, 0444);
> +MODULE_PARM_DESC(autoboot,
> +                "enable | disable autoboot. (default: true)");
> +
> +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");
> --
> 2.7.4
>

Wendy Liang Sept. 10, 2018, 5:16 p.m. UTC | #2

On Fri, Aug 24, 2018 at 9:26 AM Wendy Liang <[e-mail]> wrote:
>
> Ping, any comments to the driver?
Any comments to this driver?

Thanks,
Wendy

> On Thu, Aug 16, 2018 at 3:17 AM Wendy Liang <[e-mail]> wrote:
> >
> > There are cortex-r5 processors in Xilinx Zynq UltraScale+
> > MPSoC platforms. This remoteproc driver is to manage the
> > R5 processors.
> >
> > Signed-off-by: Wendy Liang <[e-mail]>
> > ---
> >  drivers/remoteproc/Kconfig                |   9 +
> >  drivers/remoteproc/Makefile               |   1 +
> >  drivers/remoteproc/zynqmp_r5_remoteproc.c | 692 ++++++++++++++++++++++++++++++
> >  3 files changed, 702 insertions(+)
> >  create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
> >
> > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > index cd1c168..83aac63 100644
> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -158,6 +158,15 @@ config ST_REMOTEPROC
> >  config ST_SLIM_REMOTEPROC
> >         tristate
> >
> > +config ZYNQMP_R5_REMOTEPROC
> > +       tristate "ZynqMP_r5 remoteproc support"
> > +       depends on ARM64 && PM && ARCH_ZYNQMP
> > +       select RPMSG_VIRTIO
> > +       select ZYNQMP_FIRMWARE
> > +       help
> > +         Say y 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 02627ed..147923c 100644
> > --- a/drivers/remoteproc/Makefile
> > +++ b/drivers/remoteproc/Makefile
> > @@ -23,3 +23,4 @@ qcom_wcnss_pil-y                      += qcom_wcnss.o
> >  qcom_wcnss_pil-y                       += qcom_wcnss_iris.o
> >  obj-$(CONFIG_ST_REMOTEPROC)            += st_remoteproc.o
> >  obj-$(CONFIG_ST_SLIM_REMOTEPROC)       += st_slim_rproc.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 0000000..7fc3718
> > --- /dev/null
> > +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> > @@ -0,0 +1,692 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Zynq R5 Remote Processor driver
> > + *
> > + * Copyright (C) 2015 Xilinx, Inc.
> > + *
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/remoteproc.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/slab.h>
> > +#include <linux/cpu.h>
> > +#include <linux/delay.h>
> > +#include <linux/list.h>
> > +#include <linux/genalloc.h>
> > +#include <linux/pfn.h>
> > +#include <linux/idr.h>
> > +#include <linux/firmware/xlnx-zynqmp.h>
> > +
> > +#include "remoteproc_internal.h"
> > +
> > +/* IPI reg offsets */
> > +#define TRIG_OFFSET            0x00000000
> > +#define OBS_OFFSET             0x00000004
> > +#define ISR_OFFSET             0x00000010
> > +#define IMR_OFFSET             0x00000014
> > +#define IER_OFFSET             0x00000018
> > +#define IDR_OFFSET             0x0000001C
> > +#define IPI_ALL_MASK           0x0F0F0301
> > +
> > +/* RPU IPI mask */
> > +#define RPU_IPI_INIT_MASK      0x00000100
> > +#define RPU_IPI_MASK(n)                (RPU_IPI_INIT_MASK << (n))
> > +#define RPU_0_IPI_MASK         RPU_IPI_MASK(0)
> > +#define RPU_1_IPI_MASK         RPU_IPI_MASK(1)
> > +
> > +/* PM proc states */
> > +#define PM_PROC_STATE_ACTIVE 1u
> > +
> > +/* Maximum TCM power nodes IDs */
> > +#define MAX_TCM_PNODES 4
> > +
> > +/* Register access macros */
> > +#define reg_read(base, reg) \
> > +       readl(((void __iomem *)(base)) + (reg))
> > +#define reg_write(base, reg, val) \
> > +       writel((val), ((void __iomem *)(base)) + (reg))
> > +
> > +#define DEFAULT_FIRMWARE_NAME  "rproc-rpu-fw"
> > +
> > +static bool autoboot __read_mostly;
> > +
> > +struct zynqmp_r5_rproc_pdata;
> > +
> > +/**
> > + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor instance state
> > + * @rproc: rproc handle
> > + * @workqueue: workqueue for the RPU remoteproc
> > + * @ipi_base: virt ptr to IPI channel address registers for APU
> > + * @rpu_mode: RPU core configuration
> > + * @rpu_id: RPU CPU id
> > + * @rpu_pnode_id: RPU CPU power domain id
> > + * @mem_pools: list of gen_pool for firmware mmio_sram memory and their
> > + *             power domain IDs
> > + * @mems: list of rproc_mem_entries for firmware
> > + * @irq: IRQ number
> > + * @ipi_dest_mask: IPI destination mask for the IPI channel
> > + */
> > +struct zynqmp_r5_rproc_pdata {
> > +       struct rproc *rproc;
> > +       struct work_struct workqueue;
> > +       void __iomem *ipi_base;
> > +       enum rpu_oper_mode rpu_mode;
> > +       struct list_head mems;
> > +       u32 ipi_dest_mask;
> > +       u32 rpu_id;
> > +       u32 rpu_pnode_id;
> > +       int irq;
> > +       u32 tcm_pnode_id[MAX_TCM_PNODES];
> > +};
> > +
> > +/**
> > + * r5_boot_addr_config - configure the boot address of R5
> > + * @pdata: platform data
> > + * @bootmem: boot from LOVEC or HIVEC
> > + *
> > + * This function will set the RPU boot address
> > + */
> > +static void r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> > +                               enum rpu_boot_mem bootmem)
> > +{
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +
> > +       pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> > +                __func__, pdata->rpu_id, bootmem);
> > +
> > +       if (!eemi || !eemi->ioctl) {
> > +               pr_err("%s: no eemi ioctl operation.\n", __func__);
> > +               return;
> > +       }
> > +       eemi->ioctl(pdata->rpu_pnode_id, IOCTL_RPU_BOOT_ADDR_CONFIG,
> > +                   bootmem, 0, NULL);
> > +}
> > +
> > +/**
> > + * r5_mode_config - configure R5 operation mode
> > + * @pdata: platform data
> > + *
> > + * configure R5 to split mode or lockstep mode
> > + * based on the platform data.
> > + */
> > +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +
> > +       pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> > +
> > +       if (!eemi || !eemi->ioctl) {
> > +               pr_err("%s: no eemi ioctl operation.\n", __func__);
> > +               return;
> > +       }
> > +       eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> > +                   pdata->rpu_mode, 0, NULL);
> > +}
> > +
> > +/**
> > + * r5_release_tcm() - release TCM
> > + * @pdata: platform data
> > + *
> > + * Release TCM
> > + */
> > +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +       int i;
> > +
> > +       if (!eemi || !eemi->release_node) {
> > +               pr_err("Failed to release TCM\n");
> > +               return;
> > +       }
> > +
> > +       for (i = 0; i < MAX_TCM_PNODES; i++) {
> > +               if (pdata->tcm_pnode_id[i] != 0)
> > +                       eemi->release_node(pdata->tcm_pnode_id[i]);
> > +       }
> > +}
> > +
> > +/**
> > + * disable_ipi - disable IPI
> > + * @pdata: platform data
> > + *
> > + * Disable IPI interrupt
> > + */
> > +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       /* Disable R5 IPI interrupt */
> > +       if (pdata->ipi_base)
> > +               reg_write(pdata->ipi_base, IDR_OFFSET, pdata->ipi_dest_mask);
> > +}
> > +
> > +/**
> > + * enable_ipi - enable IPI
> > + * @pdata: platform data
> > + *
> > + * Enable IPI interrupt
> > + */
> > +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       /* Enable R5 IPI interrupt */
> > +       if (pdata->ipi_base)
> > +               reg_write(pdata->ipi_base, IER_OFFSET, pdata->ipi_dest_mask);
> > +}
> > +
> > +/**
> > + * 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;
> > +}
> > +
> > +static void handle_event_notified(struct work_struct *work)
> > +{
> > +       struct rproc *rproc;
> > +       struct zynqmp_r5_rproc_pdata *local;
> > +
> > +       local = container_of(work, struct zynqmp_r5_rproc_pdata, workqueue);
> > +       rproc = local->rproc;
> > +       idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
> > +}
> > +
> > +static int zynqmp_r5_rproc_start(struct rproc *rproc)
> > +{
> > +       struct device *dev = rproc->dev.parent;
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +       enum rpu_boot_mem bootmem;
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +
> > +       dev_dbg(dev, "%s\n", __func__);
> > +
> > +       if (!eemi || !eemi->force_powerdown ||
> > +           !eemi->request_wakeup) {
> > +               pr_err("Failed to start R5\n");
> > +               return -ENXIO;
> > +       }
> > +
> > +       /* Set up R5 */
> > +       if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> > +               bootmem = PM_RPU_BOOTMEM_HIVEC;
> > +       else
> > +               bootmem = PM_RPU_BOOTMEM_LOVEC;
> > +       dev_info(dev, "RPU boot from %s.",
> > +                bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
> > +
> > +       r5_mode_config(local);
> > +       eemi->force_powerdown(local->rpu_pnode_id,
> > +                             ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > +       r5_boot_addr_config(local, bootmem);
> > +       /* Add delay before release from halt and reset */
> > +       usleep_range(400, 500);
> > +       eemi->request_wakeup(local->rpu_pnode_id,
> > +                            1, bootmem,
> > +                            ZYNQMP_PM_REQUEST_ACK_NO);
> > +
> > +       /* Make sure IPI is enabled */
> > +       enable_ipi(local);
> > +
> > +       return 0;
> > +}
> > +
> > +/* kick a firmware */
> > +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
> > +{
> > +       struct device *dev = rproc->dev.parent;
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +       dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", vqid);
> > +
> > +       /*
> > +        * send irq to R5 firmware
> > +        * Currently vqid is not used because we only got one.
> > +        */
> > +       if (local->ipi_base)
> > +               reg_write(local->ipi_base, TRIG_OFFSET, local->ipi_dest_mask);
> > +}
> > +
> > +/* power off the remote processor */
> > +static int zynqmp_r5_rproc_stop(struct rproc *rproc)
> > +{
> > +       struct device *dev = rproc->dev.parent;
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +       struct rproc_mem_entry *mem, *nmem;
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +
> > +       dev_dbg(dev, "%s\n", __func__);
> > +
> > +       if (!eemi || !eemi->force_powerdown) {
> > +               pr_err("Failed to stop R5\n");
> > +               return -ENXIO;
> > +       }
> > +
> > +       disable_ipi(local);
> > +       eemi->force_powerdown(local->rpu_pnode_id,
> > +                             ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > +
> > +       return 0;
> > +}
> > +
> > +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> > +{
> > +       struct rproc_mem_entry *mem;
> > +       void *va = NULL;
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +       list_for_each_entry(mem, &local->mems, node) {
> > +               int offset = da - mem->da;
> > +
> > +               /* try next carveout if da is too small */
> > +               if (offset < 0)
> > +                       continue;
> > +
> > +               /* try next carveout if da is too large */
> > +               if (offset + len > mem->len)
> > +                       continue;
> > +
> > +               va = mem->va + offset;
> > +
> > +               break;
> > +       }
> > +       return va;
> > +}
> > +
> > +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
> > +{
> > +       int ret;
> > +
> > +       ret = rproc_elf_load_rsc_table(rproc, fw);
> > +       if (ret == -EINVAL)
> > +               /* No resource table */
> > +               return 0;
> > +       else
> > +               return ret;
> > +}
> > +
> > +static struct rproc_ops zynqmp_r5_rproc_ops = {
> > +       .start          = zynqmp_r5_rproc_start,
> > +       .stop           = zynqmp_r5_rproc_stop,
> > +       .kick           = zynqmp_r5_rproc_kick,
> > +       .da_to_va       = zynqmp_r5_rproc_da_to_va,
> > +};
> > +
> > +/* Release R5 from reset and make it halted.
> > + * In case the firmware uses TCM, in order to load firmware to TCM,
> > + * will need to release R5 from reset and stay in halted state.
> > + */
> > +static int zynqmp_r5_rproc_init(struct rproc *rproc)
> > +{
> > +       struct device *dev = rproc->dev.parent;
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +       dev_dbg(dev, "%s\n", __func__);
> > +       enable_ipi(local);
> > +       return 0;
> > +}
> > +
> > +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id)
> > +{
> > +       struct device *dev = dev_id;
> > +       struct platform_device *pdev = to_platform_device(dev);
> > +       struct rproc *rproc = platform_get_drvdata(pdev);
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +       u32 ipi_reg;
> > +
> > +       /* Check if there is a kick from R5 */
> > +       ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> > +       if (!(ipi_reg & local->ipi_dest_mask))
> > +               return IRQ_NONE;
> > +
> > +       dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n", irq);
> > +       reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> > +       schedule_work(&local->workqueue);
> > +
> > +       return IRQ_HANDLED;
> > +}
> > +
> > +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> > + * @pdev: pointer to the platform device
> > + * @pdata: pointer to the remoteproc private data
> > + *
> > + * Function to create remoteproc memory entries for TCM memories.
> > + */
> > +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> > +                             struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> > +       struct device *dev = &pdev->dev;
> > +       struct device_node *np = dev->of_node;
> > +       int num_mems = 0;
> > +       int i, ret;
> > +       struct property *prop;
> > +       const __be32 *cur;
> > +       u32 val;
> > +       const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> > +
> > +       /* Get TCM power node ids */
> > +       i = 0;
> > +       of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> > +               pdata->tcm_pnode_id[i++] = val;
> > +
> > +       /* Request TCMs */
> > +       for (i = 0; i < MAX_TCM_PNODES; i++) {
> > +               if (pdata->tcm_pnode_id[i] != 0) {
> > +                       ret = eemi->request_node(pdata->tcm_pnode_id[i],
> > +                                                ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> > +                                                ZYNQMP_PM_REQUEST_ACK_BLOCKING
> > +                                               );
> > +                       if (ret < 0) {
> > +                               dev_err(dev, "failed to request TCM: %u\n",
> > +                                       pdata->tcm_pnode_id[i]);
> > +                               return ret;
> > +                       }
> > +                       dev_dbg(dev, "request tcm pnode: %u\n",
> > +                               pdata->tcm_pnode_id[i]);
> > +               } else {
> > +                       break;
> > +               }
> > +       }
> > +       /* Create remoteproc memories entries for TCM memories */
> > +       num_mems = ARRAY_SIZE(mem_names);
> > +       for (i = 0; i < num_mems; i++) {
> > +               struct resource *res;
> > +               struct rproc_mem_entry *mem;
> > +               dma_addr_t dma;
> > +               resource_size_t size;
> > +
> > +               res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > +                                                  mem_names[i]);
> > +               mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > +                                   GFP_KERNEL);
> > +               if (!mem)
> > +                       return -ENOMEM;
> > +               /* Map it as normal memory */
> > +               size = resource_size(res);
> > +               mem->va = devm_ioremap_wc(dev, res->start, size);
> > +               mem->len = size;
> > +               dma = (dma_addr_t)res->start;
> > +               mem->dma = dma;
> > +               /* TCM memory:
> > +                *   TCM_0: da 0 <-> global addr 0xFFE00000
> > +                *   TCM_1: da 0 <-> global addr 0xFFE90000
> > +                */
> > +               if ((dma & 0xFFF00000) == 0xFFE00000) {
> > +                       if ((dma & 0xFFF80000) == 0xFFE80000)
> > +                               mem->da -= 0x90000;
> > +                       else
> > +                               mem->da = (dma & 0x000FFFFF);
> > +               }
> > +               dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > +                       __func__, mem->va, mem->da, mem->dma);
> > +               list_add_tail(&mem->node, &pdata->mems);
> > +       }
> > +       return 0;
> > +}
> > +
> > +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> > + * @pdev: pointer to the platform device
> > + * @pdata: pointer to the remoteproc private data
> > + *
> > + * Function to create remoteproc memory entries from memory-region
> > + * property.
> > + */
> > +static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
> > +                                      struct zynqmp_r5_rproc_pdata *pdata)
> > +{
> > +       struct device *dev = &pdev->dev;
> > +       struct device_node *np = dev->of_node;
> > +       int num_mems;
> > +       int i;
> > +
> > +       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 resource res;
> > +               resource_size_t size;
> > +               struct rproc_mem_entry *mem;
> > +               int ret;
> > +
> > +               node = of_parse_phandle(np, "memory-region", i);
> > +               ret = of_device_is_compatible(node, "rproc-prog-memory");
> > +               if (!ret) {
> > +                       /* it is DMA memory. */
> > +                       dev_info(dev, "%s, dma memory %d\n", __func__, i);
> > +                       ret = of_reserved_mem_device_init_by_idx(dev,
> > +                                                                np, i);
> > +                       if (ret) {
> > +                               dev_err(dev, "unable to reserve DMA mem.\n");
> > +                               return ret;
> > +                       }
> > +                       continue;
> > +               }
> > +               ret = of_address_to_resource(node, 0, &res);
> > +               if (ret) {
> > +                       dev_err(dev, "unable to resolve memory region.\n");
> > +                       return ret;
> > +               }
> > +               mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > +                                   GFP_KERNEL);
> > +               if (!mem)
> > +                       return -ENOMEM;
> > +               /* Map it as normal memory */
> > +               size = resource_size(&res);
> > +               mem->va = devm_ioremap_wc(dev, res.start, size);
> > +               mem->len = size;
> > +               mem->dma = (dma_addr_t)res.start;
> > +               mem->da = (u32)res.start;
> > +               dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > +                       __func__, mem->va, mem->da, mem->dma);
> > +               list_add_tail(&mem->node, &pdata->mems);
> > +       }
> > +       return 0;
> > +}
> > +
> > +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> > +{
> > +       const unsigned char *prop;
> > +       struct resource *res;
> > +       int ret = 0;
> > +       struct zynqmp_r5_rproc_pdata *local;
> > +       struct rproc *rproc;
> > +
> > +       rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> > +                           &zynqmp_r5_rproc_ops, NULL,
> > +               sizeof(struct zynqmp_r5_rproc_pdata));
> > +       if (!rproc) {
> > +               dev_err(&pdev->dev, "rproc allocation failed\n");
> > +               return -ENOMEM;
> > +       }
> > +       local = rproc->priv;
> > +       local->rproc = rproc;
> > +
> > +       platform_set_drvdata(pdev, rproc);
> > +
> > +       /* Override parse_fw op to allow no resource table firmware */
> > +       rproc->ops->parse_fw = zynqmp_r5_parse_fw;
> > +
> > +       ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n", ret);
> > +               goto rproc_fault;
> > +       }
> > +
> > +       /* Get the RPU power domain id */
> > +       ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> > +                                  &local->rpu_pnode_id);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "No RPU power node ID is specified.\n");
> > +               ret = -EINVAL;
> > +               goto rproc_fault;
> > +       }
> > +       dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> > +               local->rpu_id, local->rpu_pnode_id);
> > +
> > +       prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> > +       if (!prop) {
> > +               dev_warn(&pdev->dev, "default core_conf used: lock-step\n");
> > +               prop = "lock-step";
> > +       }
> > +
> > +       dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
> > +       if (!strcmp(prop, "split0")) {
> > +               local->rpu_mode = PM_RPU_MODE_SPLIT;
> > +               local->rpu_id = 0;
> > +               local->ipi_dest_mask = RPU_0_IPI_MASK;
> > +       } else if (!strcmp(prop, "split1")) {
> > +               local->rpu_mode = PM_RPU_MODE_SPLIT;
> > +               local->rpu_id = 1;
> > +               local->ipi_dest_mask = RPU_1_IPI_MASK;
> > +       } else if (!strcmp(prop, "lock-step")) {
> > +               local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> > +               local->rpu_id = 0;
> > +               local->ipi_dest_mask = RPU_0_IPI_MASK;
> > +       } else {
> > +               dev_err(&pdev->dev, "Invalid core_conf mode provided - %s , %d\n",
> > +                       prop, local->rpu_mode);
> > +               ret = -EINVAL;
> > +               goto rproc_fault;
> > +       }
> > +
> > +       res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipi");
> > +       if (res) {
> > +               local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> > +                                              resource_size(res));
> > +               if (IS_ERR(local->ipi_base)) {
> > +                       pr_err("%s: Unable to map IPI\n", __func__);
> > +                       ret = PTR_ERR(local->ipi_base);
> > +                       goto rproc_fault;
> > +               }
> > +       } else {
> > +               dev_info(&pdev->dev, "IPI resource is not specified.\n");
> > +       }
> > +       dev_dbg(&pdev->dev, "got ipi base address\n");
> > +
> > +       INIT_LIST_HEAD(&local->mems);
> > +       /* Get TCM memories */
> > +       ret = zynqmp_r5_get_tcms(pdev, local);
> > +       if (ret < 0) {
> > +               dev_err(&pdev->dev, "failed to get TCM memories.\n");
> > +               goto rproc_fault;
> > +       }
> > +       dev_dbg(&pdev->dev, "got TCM memories\n");
> > +       /* Get reserved memory regions for firmware */
> > +       ret = zynqmp_r5_get_reserved_mems(pdev, local);
> > +       if (ret < 0) {
> > +               dev_err(&pdev->dev, "failed to get reserved memories.\n");
> > +               goto rproc_fault;
> > +       }
> > +       dev_dbg(&pdev->dev, "got reserved memories.\n");
> > +
> > +       /* Disable IPI before requesting IPI IRQ */
> > +       disable_ipi(local);
> > +       INIT_WORK(&local->workqueue, handle_event_notified);
> > +
> > +       /* IPI IRQ */
> > +       if (local->ipi_base) {
> > +               ret = platform_get_irq(pdev, 0);
> > +               if (ret < 0) {
> > +                       dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> > +                       goto rproc_fault;
> > +               }
> > +               local->irq = ret;
> > +               ret = devm_request_irq(&pdev->dev, local->irq,
> > +                                      r5_remoteproc_interrupt, IRQF_SHARED,
> > +                                      dev_name(&pdev->dev), &pdev->dev);
> > +               if (ret) {
> > +                       dev_err(&pdev->dev, "IRQ %d already allocated\n",
> > +                               local->irq);
> > +                       goto rproc_fault;
> > +               }
> > +               dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> > +       }
> > +
> > +       ret = zynqmp_r5_rproc_init(local->rproc);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> > +               goto rproc_fault;
> > +       }
> > +
> > +       rproc->auto_boot = autoboot;
> > +
> > +       ret = rproc_add(local->rproc);
> > +       if (ret) {
> > +               dev_err(&pdev->dev, "rproc registration failed\n");
> > +               goto rproc_fault;
> > +       }
> > +
> > +       return ret;
> > +
> > +rproc_fault:
> > +       rproc_free(local->rproc);
> > +
> > +       return ret;
> > +}
> > +
> > +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> > +{
> > +       struct rproc *rproc = platform_get_drvdata(pdev);
> > +       struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +       struct rproc_mem_entry *mem;
> > +
> > +       dev_info(&pdev->dev, "%s\n", __func__);
> > +
> > +       rproc_del(rproc);
> > +
> > +       list_for_each_entry(mem, &local->mems, node) {
> > +               if (mem->priv)
> > +                       gen_pool_free((struct gen_pool *)mem->priv,
> > +                                     (unsigned long)mem->va, mem->len);
> > +       }
> > +
> > +       r5_release_tcm(local);
> > +       of_reserved_mem_device_release(&pdev->dev);
> > +       rproc_free(rproc);
> > +
> > +       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_param_named(autoboot,  autoboot, bool, 0444);
> > +MODULE_PARM_DESC(autoboot,
> > +                "enable | disable autoboot. (default: true)");
> > +
> > +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> > +MODULE_LICENSE("GPL v2");
> > +MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");
> > --
> > 2.7.4
> >

Loic PALLARDY Sept. 10, 2018, 8:24 p.m. UTC | #3

Hi Wendy,
Please find below few comments.

> -----Original Message-----
> From: [e-mail] <linux-remoteproc-
> [e-mail]> On Behalf Of Wendy Liang
> Sent: Thursday, August 16, 2018 9:06 AM
> To: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; [e-mail];
> [e-mail]; [e-mail]
> Cc: [e-mail]; linux-arm-
> [e-mail]; [e-mail]; linux-
> [e-mail]; Wendy Liang <[e-mail]>
> Subject: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> 
> There are cortex-r5 processors in Xilinx Zynq UltraScale+
> MPSoC platforms. This remoteproc driver is to manage the
> R5 processors.
> 
> Signed-off-by: Wendy Liang <[e-mail]>

Jason Wu' signed-off-by missing as he is mentioned as author of this driver?

> ---
>  drivers/remoteproc/Kconfig                |   9 +
>  drivers/remoteproc/Makefile               |   1 +
>  drivers/remoteproc/zynqmp_r5_remoteproc.c | 692
> ++++++++++++++++++++++++++++++
>  3 files changed, 702 insertions(+)
>  create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
> 
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index cd1c168..83aac63 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -158,6 +158,15 @@ config ST_REMOTEPROC
>  config ST_SLIM_REMOTEPROC
>  	tristate
> 
> +config ZYNQMP_R5_REMOTEPROC
> +	tristate "ZynqMP_r5 remoteproc support"
> +	depends on ARM64 && PM && ARCH_ZYNQMP
> +	select RPMSG_VIRTIO
> +	select ZYNQMP_FIRMWARE
> +	help
> +	  Say y 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 02627ed..147923c 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -23,3 +23,4 @@ qcom_wcnss_pil-y			+=
> qcom_wcnss.o
>  qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
>  obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
>  obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.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 0000000..7fc3718
> --- /dev/null
> +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> @@ -0,0 +1,692 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Zynq R5 Remote Processor driver
> + *
> + * Copyright (C) 2015 Xilinx, Inc.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/remoteproc.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/slab.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/list.h>
> +#include <linux/genalloc.h>
> +#include <linux/pfn.h>
> +#include <linux/idr.h>
> +#include <linux/firmware/xlnx-zynqmp.h>

Includes to be classified in alphabetical order

> +
> +#include "remoteproc_internal.h"
> +
> +/* IPI reg offsets */
> +#define TRIG_OFFSET		0x00000000
> +#define OBS_OFFSET		0x00000004
> +#define ISR_OFFSET		0x00000010
> +#define IMR_OFFSET		0x00000014
> +#define IER_OFFSET		0x00000018
> +#define IDR_OFFSET		0x0000001C
> +#define IPI_ALL_MASK		0x0F0F0301
> +
> +/* RPU IPI mask */
> +#define RPU_IPI_INIT_MASK	0x00000100
> +#define RPU_IPI_MASK(n)		(RPU_IPI_INIT_MASK << (n))
> +#define RPU_0_IPI_MASK		RPU_IPI_MASK(0)
> +#define RPU_1_IPI_MASK		RPU_IPI_MASK(1)
> +
> +/* PM proc states */
> +#define PM_PROC_STATE_ACTIVE 1u
> +
> +/* Maximum TCM power nodes IDs */
> +#define MAX_TCM_PNODES 4
> +
> +/* Register access macros */
> +#define reg_read(base, reg) \
> +	readl(((void __iomem *)(base)) + (reg))
> +#define reg_write(base, reg, val) \
> +	writel((val), ((void __iomem *)(base)) + (reg))
> +
> +#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
> +
> +static bool autoboot __read_mostly;
> +
> +struct zynqmp_r5_rproc_pdata;

This definition is not needed as complete definition just below.
> +
> +/**
> + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor instance
> state
> + * @rproc: rproc handle
> + * @workqueue: workqueue for the RPU remoteproc
> + * @ipi_base: virt ptr to IPI channel address registers for APU
> + * @rpu_mode: RPU core configuration
> + * @rpu_id: RPU CPU id
> + * @rpu_pnode_id: RPU CPU power domain id
> + * @mem_pools: list of gen_pool for firmware mmio_sram memory and
> their
> + *             power domain IDs
> + * @mems: list of rproc_mem_entries for firmware
> + * @irq: IRQ number
> + * @ipi_dest_mask: IPI destination mask for the IPI channel
> + */
> +struct zynqmp_r5_rproc_pdata {
> +	struct rproc *rproc;
> +	struct work_struct workqueue;
> +	void __iomem *ipi_base;
> +	enum rpu_oper_mode rpu_mode;
> +	struct list_head mems;
> +	u32 ipi_dest_mask;
> +	u32 rpu_id;
> +	u32 rpu_pnode_id;
> +	int irq;
> +	u32 tcm_pnode_id[MAX_TCM_PNODES];
> +};
> +
> +/**
> + * r5_boot_addr_config - configure the boot address of R5
> + * @pdata: platform data
> + * @bootmem: boot from LOVEC or HIVEC
> + *
> + * This function will set the RPU boot address
> + */
> +static void r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> +				enum rpu_boot_mem bootmem)
> +{
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();
> +
> +	pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> +		 __func__, pdata->rpu_id, bootmem);
> +
> +	if (!eemi || !eemi->ioctl) {
> +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> +		return;
> +	}
> +	eemi->ioctl(pdata->rpu_pnode_id,
> IOCTL_RPU_BOOT_ADDR_CONFIG,
> +		    bootmem, 0, NULL);
> +}
> +
> +/**
> + * r5_mode_config - configure R5 operation mode
> + * @pdata: platform data
> + *
> + * configure R5 to split mode or lockstep mode
> + * based on the platform data.
> + */
> +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();
> +
> +	pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> +
> +	if (!eemi || !eemi->ioctl) {
> +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> +		return;
> +	}
> +	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> +		    pdata->rpu_mode, 0, NULL);
> +}
> +
> +/**
> + * r5_release_tcm() - release TCM
> + * @pdata: platform data
> + *
> + * Release TCM
> + */
> +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();
> +	int i;
> +
> +	if (!eemi || !eemi->release_node) {
> +		pr_err("Failed to release TCM\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> +		if (pdata->tcm_pnode_id[i] != 0)
> +			eemi->release_node(pdata->tcm_pnode_id[i]);
> +	}
> +}
> +
> +/**
> + * disable_ipi - disable IPI
> + * @pdata: platform data
> + *
> + * Disable IPI interrupt
> + */
> +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	/* Disable R5 IPI interrupt */
> +	if (pdata->ipi_base)
> +		reg_write(pdata->ipi_base, IDR_OFFSET, pdata-
> >ipi_dest_mask);
> +}

All IPI functions should go below mailbox framework as you use it as a doorbell.

> +
> +/**
> + * enable_ipi - enable IPI
> + * @pdata: platform data
> + *
> + * Enable IPI interrupt
> + */
> +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	/* Enable R5 IPI interrupt */
> +	if (pdata->ipi_base)
> +		reg_write(pdata->ipi_base, IER_OFFSET, pdata-
> >ipi_dest_mask);
> +}
> +
> +/**
> + * 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;
> +}
> +
> +static void handle_event_notified(struct work_struct *work)
> +{
> +	struct rproc *rproc;
> +	struct zynqmp_r5_rproc_pdata *local;
> +
> +	local = container_of(work, struct zynqmp_r5_rproc_pdata,
> workqueue);
> +	rproc = local->rproc;
> +	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
> +}
> +
> +static int zynqmp_r5_rproc_start(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	enum rpu_boot_mem bootmem;
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();
> +
> +	dev_dbg(dev, "%s\n", __func__);
> +
> +	if (!eemi || !eemi->force_powerdown ||
> +	    !eemi->request_wakeup) {
> +		pr_err("Failed to start R5\n");
> +		return -ENXIO;
> +	}
> +
> +	/* Set up R5 */
> +	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> +		bootmem = PM_RPU_BOOTMEM_HIVEC;
> +	else
> +		bootmem = PM_RPU_BOOTMEM_LOVEC;
> +	dev_info(dev, "RPU boot from %s.",
> +		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" :
> "TCM");
> +
> +	r5_mode_config(local);
> +	eemi->force_powerdown(local->rpu_pnode_id,
> +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +	r5_boot_addr_config(local, bootmem);

Add some blank line to ease code reading. Some parts of the code are too compacted.

> +	/* Add delay before release from halt and reset */
> +	usleep_range(400, 500);
> +	eemi->request_wakeup(local->rpu_pnode_id,
> +			     1, bootmem,
> +			     ZYNQMP_PM_REQUEST_ACK_NO);
> +
> +	/* Make sure IPI is enabled */
> +	enable_ipi(local);
> +
> +	return 0;
> +}
> +
> +/* kick a firmware */
> +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n",
> vqid);
> +
> +	/*
> +	 * send irq to R5 firmware
> +	 * Currently vqid is not used because we only got one.
> +	 */
> +	if (local->ipi_base)
> +		reg_write(local->ipi_base, TRIG_OFFSET, local-
> >ipi_dest_mask);
> +}
> +
> +/* power off the remote processor */
> +static int zynqmp_r5_rproc_stop(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	struct rproc_mem_entry *mem, *nmem;
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();

Is it pointer on secure interface to control coprocessor?
As it is used in most of functions, maybe ops could be recovered and checked only once at probe time?

> +
> +	dev_dbg(dev, "%s\n", __func__);
> +
> +	if (!eemi || !eemi->force_powerdown) {
> +		pr_err("Failed to stop R5\n");
> +		return -ENXIO;
> +	}
> +
> +	disable_ipi(local);
> +	eemi->force_powerdown(local->rpu_pnode_id,
> +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +
> +	return 0;
> +}
> +
> +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
> +{
> +	struct rproc_mem_entry *mem;
> +	void *va = NULL;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	list_for_each_entry(mem, &local->mems, node) {
> +		int offset = da - mem->da;
> +
> +		/* try next carveout if da is too small */
> +		if (offset < 0)
> +			continue;
> +
> +		/* try next carveout if da is too large */
> +		if (offset + len > mem->len)
> +			continue;
> +
> +		va = mem->va + offset;
> +
> +		break;
> +	}
> +	return va;
> +}
> +
> +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware
> *fw)
> +{
> +	int ret;
> +
> +	ret = rproc_elf_load_rsc_table(rproc, fw);
> +	if (ret == -EINVAL)
> +		/* No resource table */
> +		return 0;
> +	else
> +		return ret;
> +}
> +
> +static struct rproc_ops zynqmp_r5_rproc_ops = {
> +	.start		= zynqmp_r5_rproc_start,
> +	.stop		= zynqmp_r5_rproc_stop,
> +	.kick		= zynqmp_r5_rproc_kick,
> +	.da_to_va       = zynqmp_r5_rproc_da_to_va,
> +};
> +
> +/* Release R5 from reset and make it halted.
> + * In case the firmware uses TCM, in order to load firmware to TCM,
> + * will need to release R5 from reset and stay in halted state.
> + */
> +static int zynqmp_r5_rproc_init(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	dev_dbg(dev, "%s\n", __func__);
> +	enable_ipi(local);
> +	return 0;
> +}

Comment not aligned with function content. Only IPI enable here.
> +
> +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id)
> +{
> +	struct device *dev = dev_id;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	u32 ipi_reg;
> +
> +	/* Check if there is a kick from R5 */
> +	ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> +	if (!(ipi_reg & local->ipi_dest_mask))
> +		return IRQ_NONE;
> +
> +	dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n",
> irq);
> +	reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> +	schedule_work(&local->workqueue);
> +
> +	return IRQ_HANDLED;
> +}

Should be part of IPI mailbox driver.

> +
> +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries for TCM memories.
> + */
> +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> +			      struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	int num_mems = 0;
> +	int i, ret;
> +	struct property *prop;
> +	const __be32 *cur;
> +	u32 val;
> +	const struct zynqmp_eemi_ops *eemi =
> zynqmp_pm_get_eemi_ops();
> +
> +	/* Get TCM power node ids */
> +	i = 0;
> +	of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> +		pdata->tcm_pnode_id[i++] = val;
> +
> +	/* Request TCMs */
> +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> +		if (pdata->tcm_pnode_id[i] != 0) {
> +			ret = eemi->request_node(pdata->tcm_pnode_id[i],
> +
> ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> +
> ZYNQMP_PM_REQUEST_ACK_BLOCKING
> +						);
> +			if (ret < 0) {
> +				dev_err(dev, "failed to request TCM: %u\n",
> +					pdata->tcm_pnode_id[i]);
> +				return ret;
> +			}
> +			dev_dbg(dev, "request tcm pnode: %u\n",
> +				pdata->tcm_pnode_id[i]);
> +		} else {
> +			break;
> +		}
> +	}
> +	/* Create remoteproc memories entries for TCM memories */
> +	num_mems = ARRAY_SIZE(mem_names);
> +	for (i = 0; i < num_mems; i++) {
> +		struct resource *res;
> +		struct rproc_mem_entry *mem;
> +		dma_addr_t dma;
> +		resource_size_t size;
> +
> +		res = platform_get_resource_byname(pdev,
> IORESOURCE_MEM,
> +						   mem_names[i]);
> +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +				    GFP_KERNEL);
> +		if (!mem)
> +			return -ENOMEM;
> +		/* Map it as normal memory */
> +		size = resource_size(res);
> +		mem->va = devm_ioremap_wc(dev, res->start, size);
> +		mem->len = size;
> +		dma = (dma_addr_t)res->start;
> +		mem->dma = dma;
> +		/* TCM memory:
> +		 *   TCM_0: da 0 <-> global addr 0xFFE00000
> +		 *   TCM_1: da 0 <-> global addr 0xFFE90000
> +		 */
> +		if ((dma & 0xFFF00000) == 0xFFE00000) {
> +			if ((dma & 0xFFF80000) == 0xFFE80000)
> +				mem->da -= 0x90000;
> +			else
> +				mem->da = (dma & 0x000FFFFF);
> +		}
> +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +			__func__, mem->va, mem->da, mem->dma);
> +		list_add_tail(&mem->node, &pdata->mems);

Looks like carevout patch series [1]. Did you try it? I think it is better to communalize code instead of duplicated it in each platform driver.
[1] https://lkml.org/lkml/2018/7/27/612

> +	}
> +	return 0;
> +}
> +
> +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries from memory-region
> + * property.
> + */
> +static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
> +				       struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	int num_mems;
> +	int i;
> +
> +	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 resource res;
> +		resource_size_t size;
> +		struct rproc_mem_entry *mem;
> +		int ret;
> +
> +		node = of_parse_phandle(np, "memory-region", i);
> +		ret = of_device_is_compatible(node, "rproc-prog-memory");
> +		if (!ret) {
> +			/* it is DMA memory. */
> +			dev_info(dev, "%s, dma memory %d\n", __func__,
> i);
> +			ret = of_reserved_mem_device_init_by_idx(dev,
> +								 np, i);
> +			if (ret) {
> +				dev_err(dev, "unable to reserve DMA
> mem.\n");
> +				return ret;
> +			}
> +			continue;
> +		}
> +		ret = of_address_to_resource(node, 0, &res);
> +		if (ret) {
> +			dev_err(dev, "unable to resolve memory region.\n");
> +			return ret;
> +		}
> +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +				    GFP_KERNEL);
> +		if (!mem)
> +			return -ENOMEM;
> +		/* Map it as normal memory */
> +		size = resource_size(&res);
> +		mem->va = devm_ioremap_wc(dev, res.start, size);
> +		mem->len = size;
> +		mem->dma = (dma_addr_t)res.start;
> +		mem->da = (u32)res.start;
> +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +			__func__, mem->va, mem->da, mem->dma);
> +		list_add_tail(&mem->node, &pdata->mems);
> +	}
> +	return 0;
> +}
> +
> +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> +{
> +	const unsigned char *prop;
> +	struct resource *res;
> +	int ret = 0;
> +	struct zynqmp_r5_rproc_pdata *local;
> +	struct rproc *rproc;
> +
> +	rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> +			    &zynqmp_r5_rproc_ops, NULL,
> +		sizeof(struct zynqmp_r5_rproc_pdata));
> +	if (!rproc) {
> +		dev_err(&pdev->dev, "rproc allocation failed\n");
> +		return -ENOMEM;
> +	}
> +	local = rproc->priv;
> +	local->rproc = rproc;
> +
> +	platform_set_drvdata(pdev, rproc);
> +
> +	/* Override parse_fw op to allow no resource table firmware */
> +	rproc->ops->parse_fw = zynqmp_r5_parse_fw;
> +
> +	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n",
> ret);
> +		goto rproc_fault;
> +	}
> +
> +	/* Get the RPU power domain id */
> +	ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> +				   &local->rpu_pnode_id);
> +	if (ret) {
> +		dev_err(&pdev->dev, "No RPU power node ID is
> specified.\n");
> +		ret = -EINVAL;
> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> +		local->rpu_id, local->rpu_pnode_id);
> +
> +	prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> +	if (!prop) {
> +		dev_warn(&pdev->dev, "default core_conf used: lock-
> step\n");
> +		prop = "lock-step";
> +	}
> +
> +	dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
> +	if (!strcmp(prop, "split0")) {
> +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> +		local->rpu_id = 0;
> +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> +	} else if (!strcmp(prop, "split1")) {
> +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> +		local->rpu_id = 1;
> +		local->ipi_dest_mask = RPU_1_IPI_MASK;
> +	} else if (!strcmp(prop, "lock-step")) {
> +		local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> +		local->rpu_id = 0;
> +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> +	} else {
> +		dev_err(&pdev->dev, "Invalid core_conf mode provided - %s
> , %d\n",
> +			prop, local->rpu_mode);
> +		ret = -EINVAL;
> +		goto rproc_fault;
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> "ipi");
> +	if (res) {
> +		local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> +					       resource_size(res));
> +		if (IS_ERR(local->ipi_base)) {
> +			pr_err("%s: Unable to map IPI\n", __func__);
> +			ret = PTR_ERR(local->ipi_base);
> +			goto rproc_fault;
> +		}
> +	} else {
> +		dev_info(&pdev->dev, "IPI resource is not specified.\n");
> +	}
> +	dev_dbg(&pdev->dev, "got ipi base address\n");
> +
> +	INIT_LIST_HEAD(&local->mems);
> +	/* Get TCM memories */
> +	ret = zynqmp_r5_get_tcms(pdev, local);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get TCM memories.\n");
> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "got TCM memories\n");
> +	/* Get reserved memory regions for firmware */
> +	ret = zynqmp_r5_get_reserved_mems(pdev, local);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get reserved memories.\n");
> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "got reserved memories.\n");
> +
> +	/* Disable IPI before requesting IPI IRQ */
> +	disable_ipi(local);
> +	INIT_WORK(&local->workqueue, handle_event_notified);
> +
> +	/* IPI IRQ */
> +	if (local->ipi_base) {
> +		ret = platform_get_irq(pdev, 0);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> +			goto rproc_fault;
> +		}
> +		local->irq = ret;
> +		ret = devm_request_irq(&pdev->dev, local->irq,
> +				       r5_remoteproc_interrupt, IRQF_SHARED,
> +				       dev_name(&pdev->dev), &pdev->dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "IRQ %d already allocated\n",
> +				local->irq);
> +			goto rproc_fault;
> +		}
> +		dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> +	}
> +
> +	ret = zynqmp_r5_rproc_init(local->rproc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> +		goto rproc_fault;
> +	}
> +
> +	rproc->auto_boot = autoboot;
> +
> +	ret = rproc_add(local->rproc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "rproc registration failed\n");
> +		goto rproc_fault;
> +	}
> +
> +	return ret;
> +
> +rproc_fault:
> +	rproc_free(local->rproc);
> +
> +	return ret;
> +}
> +
> +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> +{
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	struct rproc_mem_entry *mem;
> +
> +	dev_info(&pdev->dev, "%s\n", __func__);
> +
> +	rproc_del(rproc);
> +
> +	list_for_each_entry(mem, &local->mems, node) {
> +		if (mem->priv)
> +			gen_pool_free((struct gen_pool *)mem->priv,
> +				      (unsigned long)mem->va, mem->len);
> +	}
> +
> +	r5_release_tcm(local);
> +	of_reserved_mem_device_release(&pdev->dev);
> +	rproc_free(rproc);
> +
> +	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_param_named(autoboot,  autoboot, bool, 0444);
> +MODULE_PARM_DESC(autoboot,
> +		 "enable | disable autoboot. (default: true)");
> +
> +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");
> --
> 2.7.4

Jiaying Liang Sept. 10, 2018, 10:09 p.m. UTC | #4

> -----Original Message-----
> From: Loic PALLARDY [mailto:[e-mail]]
> Sent: Monday, September 10, 2018 1:25 PM
> To: Jiaying Liang <[e-mail]>; [e-mail];
> [e-mail]; Michal Simek <[e-mail]>;
> [e-mail]; [e-mail]; Rajan Vaja
> <[e-mail]>; Jolly Shah <[e-mail]>
> Cc: [e-mail]; [e-mail];
> [e-mail]; [e-mail]; Jiaying Liang
> <[e-mail]>
> Subject: RE: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> 
> Hi Wendy,
> Please find below few comments.
> 
> > -----Original Message-----
> > From: [e-mail] <linux-remoteproc-
> > [e-mail]> On Behalf Of Wendy Liang
> > Sent: Thursday, August 16, 2018 9:06 AM
> > To: [e-mail]; [e-mail];
> > [e-mail]; [e-mail]; [e-mail];
> > [e-mail]; [e-mail]
> > Cc: [e-mail]; linux-arm-
> > [e-mail]; [e-mail]; linux-
> > [e-mail]; Wendy Liang <[e-mail]>
> > Subject: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> >
> > There are cortex-r5 processors in Xilinx Zynq UltraScale+ MPSoC
> > platforms. This remoteproc driver is to manage the
> > R5 processors.
> >
> > Signed-off-by: Wendy Liang <[e-mail]>
> 
> Jason Wu' signed-off-by missing as he is mentioned as author of this driver?
[Wendy] He was the one who wrote the init version of the driver.
But he left the company a few years ago. In this case, maybe I should remove
Module author.
> 
> > ---
> >  drivers/remoteproc/Kconfig                |   9 +
> >  drivers/remoteproc/Makefile               |   1 +
> >  drivers/remoteproc/zynqmp_r5_remoteproc.c | 692
> > ++++++++++++++++++++++++++++++
> >  3 files changed, 702 insertions(+)
> >  create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
> >
> > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > index cd1c168..83aac63 100644
> > --- a/drivers/remoteproc/Kconfig
> > +++ b/drivers/remoteproc/Kconfig
> > @@ -158,6 +158,15 @@ config ST_REMOTEPROC  config
> ST_SLIM_REMOTEPROC
> >  	tristate
> >
> > +config ZYNQMP_R5_REMOTEPROC
> > +	tristate "ZynqMP_r5 remoteproc support"
> > +	depends on ARM64 && PM && ARCH_ZYNQMP
> > +	select RPMSG_VIRTIO
> > +	select ZYNQMP_FIRMWARE
> > +	help
> > +	  Say y 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 02627ed..147923c 100644
> > --- a/drivers/remoteproc/Makefile
> > +++ b/drivers/remoteproc/Makefile
> > @@ -23,3 +23,4 @@ qcom_wcnss_pil-y			+=
> > qcom_wcnss.o
> >  qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
> >  obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
> >  obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.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 0000000..7fc3718
> > --- /dev/null
> > +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> > @@ -0,0 +1,692 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Zynq R5 Remote Processor driver
> > + *
> > + * Copyright (C) 2015 Xilinx, Inc.
> > + *
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/err.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/dma-mapping.h>
> > +#include <linux/remoteproc.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/of_address.h>
> > +#include <linux/of_irq.h>
> > +#include <linux/of_platform.h>
> > +#include <linux/of_reserved_mem.h>
> > +#include <linux/slab.h>
> > +#include <linux/cpu.h>
> > +#include <linux/delay.h>
> > +#include <linux/list.h>
> > +#include <linux/genalloc.h>
> > +#include <linux/pfn.h>
> > +#include <linux/idr.h>
> > +#include <linux/firmware/xlnx-zynqmp.h>
> 
> Includes to be classified in alphabetical order
[Wendy] Will do in the next version
> 
> > +
> > +#include "remoteproc_internal.h"
> > +
> > +/* IPI reg offsets */
> > +#define TRIG_OFFSET		0x00000000
> > +#define OBS_OFFSET		0x00000004
> > +#define ISR_OFFSET		0x00000010
> > +#define IMR_OFFSET		0x00000014
> > +#define IER_OFFSET		0x00000018
> > +#define IDR_OFFSET		0x0000001C
> > +#define IPI_ALL_MASK		0x0F0F0301
> > +
> > +/* RPU IPI mask */
> > +#define RPU_IPI_INIT_MASK	0x00000100
> > +#define RPU_IPI_MASK(n)		(RPU_IPI_INIT_MASK << (n))
> > +#define RPU_0_IPI_MASK		RPU_IPI_MASK(0)
> > +#define RPU_1_IPI_MASK		RPU_IPI_MASK(1)
> > +
> > +/* PM proc states */
> > +#define PM_PROC_STATE_ACTIVE 1u
> > +
> > +/* Maximum TCM power nodes IDs */
> > +#define MAX_TCM_PNODES 4
> > +
> > +/* Register access macros */
> > +#define reg_read(base, reg) \
> > +	readl(((void __iomem *)(base)) + (reg)) #define reg_write(base, reg,
> > +val) \
> > +	writel((val), ((void __iomem *)(base)) + (reg))
> > +
> > +#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
> > +
> > +static bool autoboot __read_mostly;
> > +
> > +struct zynqmp_r5_rproc_pdata;
> 
> This definition is not needed as complete definition just below.
[Wendy] Will remove in the next version
> > +
> > +/**
> > + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor
> > +instance
> > state
> > + * @rproc: rproc handle
> > + * @workqueue: workqueue for the RPU remoteproc
> > + * @ipi_base: virt ptr to IPI channel address registers for APU
> > + * @rpu_mode: RPU core configuration
> > + * @rpu_id: RPU CPU id
> > + * @rpu_pnode_id: RPU CPU power domain id
> > + * @mem_pools: list of gen_pool for firmware mmio_sram memory and
> > their
> > + *             power domain IDs
> > + * @mems: list of rproc_mem_entries for firmware
> > + * @irq: IRQ number
> > + * @ipi_dest_mask: IPI destination mask for the IPI channel  */
> > +struct zynqmp_r5_rproc_pdata {
> > +	struct rproc *rproc;
> > +	struct work_struct workqueue;
> > +	void __iomem *ipi_base;
> > +	enum rpu_oper_mode rpu_mode;
> > +	struct list_head mems;
> > +	u32 ipi_dest_mask;
> > +	u32 rpu_id;
> > +	u32 rpu_pnode_id;
> > +	int irq;
> > +	u32 tcm_pnode_id[MAX_TCM_PNODES];
> > +};
> > +
> > +/**
> > + * r5_boot_addr_config - configure the boot address of R5
> > + * @pdata: platform data
> > + * @bootmem: boot from LOVEC or HIVEC
> > + *
> > + * This function will set the RPU boot address  */ static void
> > +r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> > +				enum rpu_boot_mem bootmem)
> > +{
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> > +
> > +	pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> > +		 __func__, pdata->rpu_id, bootmem);
> > +
> > +	if (!eemi || !eemi->ioctl) {
> > +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> > +		return;
> > +	}
> > +	eemi->ioctl(pdata->rpu_pnode_id,
> > IOCTL_RPU_BOOT_ADDR_CONFIG,
> > +		    bootmem, 0, NULL);
> > +}
> > +
> > +/**
> > + * r5_mode_config - configure R5 operation mode
> > + * @pdata: platform data
> > + *
> > + * configure R5 to split mode or lockstep mode
> > + * based on the platform data.
> > + */
> > +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata) {
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> > +
> > +	pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> > +
> > +	if (!eemi || !eemi->ioctl) {
> > +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> > +		return;
> > +	}
> > +	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> > +		    pdata->rpu_mode, 0, NULL);
> > +}
> > +
> > +/**
> > + * r5_release_tcm() - release TCM
> > + * @pdata: platform data
> > + *
> > + * Release TCM
> > + */
> > +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata) {
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> > +	int i;
> > +
> > +	if (!eemi || !eemi->release_node) {
> > +		pr_err("Failed to release TCM\n");
> > +		return;
> > +	}
> > +
> > +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> > +		if (pdata->tcm_pnode_id[i] != 0)
> > +			eemi->release_node(pdata->tcm_pnode_id[i]);
> > +	}
> > +}
> > +
> > +/**
> > + * disable_ipi - disable IPI
> > + * @pdata: platform data	
> > + *
> > + * Disable IPI interrupt
> > + */
> > +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata) {
> > +	/* Disable R5 IPI interrupt */
> > +	if (pdata->ipi_base)
> > +		reg_write(pdata->ipi_base, IDR_OFFSET, pdata-
> > >ipi_dest_mask);
> > +}
> 
> All IPI functions should go below mailbox framework as you use it as a
> doorbell.
[Wendy] There is another patch for IPI driver. There are discussions on how the mailbox
Driver should be implemented. I directly access the IPI here is to see if I can upstream this
Remoteproc driver independent to the mailbox driver. I can update this driver after the
Mailbox driver is upstreamed.
> 
> > +
> > +/**
> > + * enable_ipi - enable IPI
> > + * @pdata: platform data
> > + *
> > + * Enable IPI interrupt
> > + */
> > +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata) {
> > +	/* Enable R5 IPI interrupt */
> > +	if (pdata->ipi_base)
> > +		reg_write(pdata->ipi_base, IER_OFFSET, pdata-
> > >ipi_dest_mask);
> > +}
> > +
> > +/**
> > + * 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;
> > +}
> > +
> > +static void handle_event_notified(struct work_struct *work) {
> > +	struct rproc *rproc;
> > +	struct zynqmp_r5_rproc_pdata *local;
> > +
> > +	local = container_of(work, struct zynqmp_r5_rproc_pdata,
> > workqueue);
> > +	rproc = local->rproc;
> > +	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc); }
> > +
> > +static int zynqmp_r5_rproc_start(struct rproc *rproc) {
> > +	struct device *dev = rproc->dev.parent;
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +	enum rpu_boot_mem bootmem;
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> > +
> > +	dev_dbg(dev, "%s\n", __func__);
> > +
> > +	if (!eemi || !eemi->force_powerdown ||
> > +	    !eemi->request_wakeup) {
> > +		pr_err("Failed to start R5\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	/* Set up R5 */
> > +	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> > +		bootmem = PM_RPU_BOOTMEM_HIVEC;
> > +	else
> > +		bootmem = PM_RPU_BOOTMEM_LOVEC;
> > +	dev_info(dev, "RPU boot from %s.",
> > +		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" :
> > "TCM");
> > +
> > +	r5_mode_config(local);
> > +	eemi->force_powerdown(local->rpu_pnode_id,
> > +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > +	r5_boot_addr_config(local, bootmem);
> 
> Add some blank line to ease code reading. Some parts of the code are too
> compacted.
Will do in the next version.
> 
> > +	/* Add delay before release from halt and reset */
> > +	usleep_range(400, 500);
> > +	eemi->request_wakeup(local->rpu_pnode_id,
> > +			     1, bootmem,
> > +			     ZYNQMP_PM_REQUEST_ACK_NO);
> > +
> > +	/* Make sure IPI is enabled */
> > +	enable_ipi(local);
> > +
> > +	return 0;
> > +}
> > +
> > +/* kick a firmware */
> > +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid) {
> > +	struct device *dev = rproc->dev.parent;
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n",
> > vqid);
> > +
> > +	/*
> > +	 * send irq to R5 firmware
> > +	 * Currently vqid is not used because we only got one.
> > +	 */
> > +	if (local->ipi_base)
> > +		reg_write(local->ipi_base, TRIG_OFFSET, local-
> > >ipi_dest_mask);
> > +}
> > +
> > +/* power off the remote processor */
> > +static int zynqmp_r5_rproc_stop(struct rproc *rproc) {
> > +	struct device *dev = rproc->dev.parent;
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +	struct rproc_mem_entry *mem, *nmem;
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> 
> Is it pointer on secure interface to control coprocessor?
> As it is used in most of functions, maybe ops could be recovered and
> checked only once at probe time?
I will do the checking at probe time. And fails probing if it is not there.
> 
> > +
> > +	dev_dbg(dev, "%s\n", __func__);
> > +
> > +	if (!eemi || !eemi->force_powerdown) {
> > +		pr_err("Failed to stop R5\n");
> > +		return -ENXIO;
> > +	}
> > +
> > +	disable_ipi(local);
> > +	eemi->force_powerdown(local->rpu_pnode_id,
> > +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > +
> > +	return 0;
> > +}
> > +
> > +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da,
> > +int len) {
> > +	struct rproc_mem_entry *mem;
> > +	void *va = NULL;
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +	list_for_each_entry(mem, &local->mems, node) {
> > +		int offset = da - mem->da;
> > +
> > +		/* try next carveout if da is too small */
> > +		if (offset < 0)
> > +			continue;
> > +
> > +		/* try next carveout if da is too large */
> > +		if (offset + len > mem->len)
> > +			continue;
> > +
> > +		va = mem->va + offset;
> > +
> > +		break;
> > +	}
> > +	return va;
> > +}
> > +
> > +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct
> > +firmware
> > *fw)
> > +{
> > +	int ret;
> > +
> > +	ret = rproc_elf_load_rsc_table(rproc, fw);
> > +	if (ret == -EINVAL)
> > +		/* No resource table */
> > +		return 0;
> > +	else
> > +		return ret;
> > +}
> > +
> > +static struct rproc_ops zynqmp_r5_rproc_ops = {
> > +	.start		= zynqmp_r5_rproc_start,
> > +	.stop		= zynqmp_r5_rproc_stop,
> > +	.kick		= zynqmp_r5_rproc_kick,
> > +	.da_to_va       = zynqmp_r5_rproc_da_to_va,
> > +};
> > +
> > +/* Release R5 from reset and make it halted.
> > + * In case the firmware uses TCM, in order to load firmware to TCM,
> > + * will need to release R5 from reset and stay in halted state.
> > + */
> > +static int zynqmp_r5_rproc_init(struct rproc *rproc) {
> > +	struct device *dev = rproc->dev.parent;
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +
> > +	dev_dbg(dev, "%s\n", __func__);
> > +	enable_ipi(local);
> > +	return 0;
> > +}
> 
> Comment not aligned with function content. Only IPI enable here.
Will fix the comments in the next version.
> > +
> > +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id) {
> > +	struct device *dev = dev_id;
> > +	struct platform_device *pdev = to_platform_device(dev);
> > +	struct rproc *rproc = platform_get_drvdata(pdev);
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +	u32 ipi_reg;
> > +
> > +	/* Check if there is a kick from R5 */
> > +	ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> > +	if (!(ipi_reg & local->ipi_dest_mask))
> > +		return IRQ_NONE;
> > +
> > +	dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n",
> > irq);
> > +	reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> > +	schedule_work(&local->workqueue);
> > +
> > +	return IRQ_HANDLED;
> > +}
> 
> Should be part of IPI mailbox driver.
[Wendy] the IPI mailbox driver patches are under discussion. I am trying to upstream
this driver independent to the mailbox driver. I can update this driver after the
Mailbox driver is upstreamed.

> 
> > +
> > +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> > + * @pdev: pointer to the platform device
> > + * @pdata: pointer to the remoteproc private data
> > + *
> > + * Function to create remoteproc memory entries for TCM memories.
> > + */
> > +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> > +			      struct zynqmp_r5_rproc_pdata *pdata) {
> > +	static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	int num_mems = 0;
> > +	int i, ret;
> > +	struct property *prop;
> > +	const __be32 *cur;
> > +	u32 val;
> > +	const struct zynqmp_eemi_ops *eemi =
> > zynqmp_pm_get_eemi_ops();
> > +
> > +	/* Get TCM power node ids */
> > +	i = 0;
> > +	of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> > +		pdata->tcm_pnode_id[i++] = val;
> > +
> > +	/* Request TCMs */
> > +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> > +		if (pdata->tcm_pnode_id[i] != 0) {
> > +			ret = eemi->request_node(pdata->tcm_pnode_id[i],
> > +
> > ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> > +
> > ZYNQMP_PM_REQUEST_ACK_BLOCKING
> > +						);
> > +			if (ret < 0) {
> > +				dev_err(dev, "failed to request TCM: %u\n",
> > +					pdata->tcm_pnode_id[i]);
> > +				return ret;
> > +			}
> > +			dev_dbg(dev, "request tcm pnode: %u\n",
> > +				pdata->tcm_pnode_id[i]);
> > +		} else {
> > +			break;
> > +		}
> > +	}
> > +	/* Create remoteproc memories entries for TCM memories */
> > +	num_mems = ARRAY_SIZE(mem_names);
> > +	for (i = 0; i < num_mems; i++) {
> > +		struct resource *res;
> > +		struct rproc_mem_entry *mem;
> > +		dma_addr_t dma;
> > +		resource_size_t size;
> > +
> > +		res = platform_get_resource_byname(pdev,
> > IORESOURCE_MEM,
> > +						   mem_names[i]);
> > +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > +				    GFP_KERNEL);
> > +		if (!mem)
> > +			return -ENOMEM;
> > +		/* Map it as normal memory */
> > +		size = resource_size(res);
> > +		mem->va = devm_ioremap_wc(dev, res->start, size);
> > +		mem->len = size;
> > +		dma = (dma_addr_t)res->start;
> > +		mem->dma = dma;
> > +		/* TCM memory:
> > +		 *   TCM_0: da 0 <-> global addr 0xFFE00000
> > +		 *   TCM_1: da 0 <-> global addr 0xFFE90000
> > +		 */
> > +		if ((dma & 0xFFF00000) == 0xFFE00000) {
> > +			if ((dma & 0xFFF80000) == 0xFFE80000)
> > +				mem->da -= 0x90000;
> > +			else
> > +				mem->da = (dma & 0x000FFFFF);
> > +		}
> > +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > +			__func__, mem->va, mem->da, mem->dma);
> > +		list_add_tail(&mem->node, &pdata->mems);
> 
> Looks like carevout patch series [1]. Did you try it? I think it is better to
> communalize code instead of duplicated it in each platform driver.
> [1] https://lkml.org/lkml/2018/7/27/612
I was aware that you have defined rproc_mem_entry_init() and rproc_add_carveout()
Functions, just those patches are not in upstream yet. I didn't use them in this patch.
I can change to use those functions in the next version.
> 
> > +	}
> > +	return 0;
> > +}
> > +
> > +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> > + * @pdev: pointer to the platform device
> > + * @pdata: pointer to the remoteproc private data
> > + *
> > + * Function to create remoteproc memory entries from memory-region
> > + * property.
> > + */
> > +static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
> > +				       struct zynqmp_r5_rproc_pdata *pdata) {
> > +	struct device *dev = &pdev->dev;
> > +	struct device_node *np = dev->of_node;
> > +	int num_mems;
> > +	int i;
> > +
> > +	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 resource res;
> > +		resource_size_t size;
> > +		struct rproc_mem_entry *mem;
> > +		int ret;
> > +
> > +		node = of_parse_phandle(np, "memory-region", i);
> > +		ret = of_device_is_compatible(node, "rproc-prog-memory");
> > +		if (!ret) {
> > +			/* it is DMA memory. */
> > +			dev_info(dev, "%s, dma memory %d\n", __func__,
> > i);
> > +			ret = of_reserved_mem_device_init_by_idx(dev,
> > +								 np, i);
> > +			if (ret) {
> > +				dev_err(dev, "unable to reserve DMA
> > mem.\n");
> > +				return ret;
> > +			}
> > +			continue;
> > +		}
> > +		ret = of_address_to_resource(node, 0, &res);
> > +		if (ret) {
> > +			dev_err(dev, "unable to resolve memory region.\n");
> > +			return ret;
> > +		}
> > +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > +				    GFP_KERNEL);
> > +		if (!mem)
> > +			return -ENOMEM;
> > +		/* Map it as normal memory */
> > +		size = resource_size(&res);
> > +		mem->va = devm_ioremap_wc(dev, res.start, size);
> > +		mem->len = size;
> > +		mem->dma = (dma_addr_t)res.start;
> > +		mem->da = (u32)res.start;
> > +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > +			__func__, mem->va, mem->da, mem->dma);
> > +		list_add_tail(&mem->node, &pdata->mems);
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev) {
> > +	const unsigned char *prop;
> > +	struct resource *res;
> > +	int ret = 0;
> > +	struct zynqmp_r5_rproc_pdata *local;
> > +	struct rproc *rproc;
> > +
> > +	rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> > +			    &zynqmp_r5_rproc_ops, NULL,
> > +		sizeof(struct zynqmp_r5_rproc_pdata));
> > +	if (!rproc) {
> > +		dev_err(&pdev->dev, "rproc allocation failed\n");
> > +		return -ENOMEM;
> > +	}
> > +	local = rproc->priv;
> > +	local->rproc = rproc;
> > +
> > +	platform_set_drvdata(pdev, rproc);
> > +
> > +	/* Override parse_fw op to allow no resource table firmware */
> > +	rproc->ops->parse_fw = zynqmp_r5_parse_fw;
> > +
> > +	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n",
> > ret);
> > +		goto rproc_fault;
> > +	}
> > +
> > +	/* Get the RPU power domain id */
> > +	ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> > +				   &local->rpu_pnode_id);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "No RPU power node ID is
> > specified.\n");
> > +		ret = -EINVAL;
> > +		goto rproc_fault;
> > +	}
> > +	dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> > +		local->rpu_id, local->rpu_pnode_id);
> > +
> > +	prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> > +	if (!prop) {
> > +		dev_warn(&pdev->dev, "default core_conf used: lock-
> > step\n");
> > +		prop = "lock-step";
> > +	}
> > +
> > +	dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
> > +	if (!strcmp(prop, "split0")) {
> > +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> > +		local->rpu_id = 0;
> > +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> > +	} else if (!strcmp(prop, "split1")) {
> > +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> > +		local->rpu_id = 1;
> > +		local->ipi_dest_mask = RPU_1_IPI_MASK;
> > +	} else if (!strcmp(prop, "lock-step")) {
> > +		local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> > +		local->rpu_id = 0;
> > +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> > +	} else {
> > +		dev_err(&pdev->dev, "Invalid core_conf mode provided - %s
> > , %d\n",
> > +			prop, local->rpu_mode);
> > +		ret = -EINVAL;
> > +		goto rproc_fault;
> > +	}
> > +
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > "ipi");
> > +	if (res) {
> > +		local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> > +					       resource_size(res));
> > +		if (IS_ERR(local->ipi_base)) {
> > +			pr_err("%s: Unable to map IPI\n", __func__);
> > +			ret = PTR_ERR(local->ipi_base);
> > +			goto rproc_fault;
> > +		}
> > +	} else {
> > +		dev_info(&pdev->dev, "IPI resource is not specified.\n");
> > +	}
> > +	dev_dbg(&pdev->dev, "got ipi base address\n");
> > +
> > +	INIT_LIST_HEAD(&local->mems);
> > +	/* Get TCM memories */
> > +	ret = zynqmp_r5_get_tcms(pdev, local);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to get TCM memories.\n");
> > +		goto rproc_fault;
> > +	}
> > +	dev_dbg(&pdev->dev, "got TCM memories\n");
> > +	/* Get reserved memory regions for firmware */
> > +	ret = zynqmp_r5_get_reserved_mems(pdev, local);
> > +	if (ret < 0) {
> > +		dev_err(&pdev->dev, "failed to get reserved memories.\n");
> > +		goto rproc_fault;
> > +	}
> > +	dev_dbg(&pdev->dev, "got reserved memories.\n");
> > +
> > +	/* Disable IPI before requesting IPI IRQ */
> > +	disable_ipi(local);
> > +	INIT_WORK(&local->workqueue, handle_event_notified);
> > +
> > +	/* IPI IRQ */
> > +	if (local->ipi_base) {
> > +		ret = platform_get_irq(pdev, 0);
> > +		if (ret < 0) {
> > +			dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> > +			goto rproc_fault;
> > +		}
> > +		local->irq = ret;
> > +		ret = devm_request_irq(&pdev->dev, local->irq,
> > +				       r5_remoteproc_interrupt, IRQF_SHARED,
> > +				       dev_name(&pdev->dev), &pdev->dev);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "IRQ %d already allocated\n",
> > +				local->irq);
> > +			goto rproc_fault;
> > +		}
> > +		dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> > +	}
> > +
> > +	ret = zynqmp_r5_rproc_init(local->rproc);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> > +		goto rproc_fault;
> > +	}
> > +
> > +	rproc->auto_boot = autoboot;
> > +
> > +	ret = rproc_add(local->rproc);
> > +	if (ret) {
> > +		dev_err(&pdev->dev, "rproc registration failed\n");
> > +		goto rproc_fault;
> > +	}
> > +
> > +	return ret;
> > +
> > +rproc_fault:
> > +	rproc_free(local->rproc);
> > +
> > +	return ret;
> > +}
> > +
> > +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> > +{
> > +	struct rproc *rproc = platform_get_drvdata(pdev);
> > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > +	struct rproc_mem_entry *mem;
> > +
> > +	dev_info(&pdev->dev, "%s\n", __func__);
> > +
> > +	rproc_del(rproc);
> > +
> > +	list_for_each_entry(mem, &local->mems, node) {
> > +		if (mem->priv)
> > +			gen_pool_free((struct gen_pool *)mem->priv,
> > +				      (unsigned long)mem->va, mem->len);
> > +	}
> > +
> > +	r5_release_tcm(local);
> > +	of_reserved_mem_device_release(&pdev->dev);
> > +	rproc_free(rproc);
> > +
> > +	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_param_named(autoboot,  autoboot, bool, 0444);
> > +MODULE_PARM_DESC(autoboot,
> > +		 "enable | disable autoboot. (default: true)");
> > +
> > +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> MODULE_LICENSE("GPL
> > +v2"); MODULE_DESCRIPTION("ZynqMP R5 remote processor control
> > +driver");
> > --
> > 2.7.4

Loic PALLARDY Sept. 11, 2018, 7:30 a.m. UTC | #5

> -----Original Message-----
> From: Jiaying Liang <[e-mail]>
> Sent: Tuesday, September 11, 2018 12:10 AM
> To: Loic PALLARDY <[e-mail]>; [e-mail];
> [e-mail]; Michal Simek <[e-mail]>;
> [e-mail]; [e-mail]; Rajan Vaja
> <[e-mail]>; Jolly Shah <[e-mail]>
> Cc: [e-mail]; linux-arm-
> [e-mail]; [e-mail]; linux-
> [e-mail]
> Subject: RE: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> 
> 
> 
> > -----Original Message-----
> > From: Loic PALLARDY [mailto:[e-mail]]
> > Sent: Monday, September 10, 2018 1:25 PM
> > To: Jiaying Liang <[e-mail]>; [e-mail];
> > [e-mail]; Michal Simek <[e-mail]>;
> > [e-mail]; [e-mail]; Rajan Vaja
> > <[e-mail]>; Jolly Shah <[e-mail]>
> > Cc: [e-mail]; linux-arm-
> [e-mail];
> > [e-mail]; [e-mail]; Jiaying Liang
> > <[e-mail]>
> > Subject: RE: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> >
> > Hi Wendy,
> > Please find below few comments.
> >
> > > -----Original Message-----
> > > From: [e-mail] <linux-remoteproc-
> > > [e-mail]> On Behalf Of Wendy Liang
> > > Sent: Thursday, August 16, 2018 9:06 AM
> > > To: [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]; [e-mail];
> > > [e-mail]; [e-mail]
> > > Cc: [e-mail]; linux-arm-
> > > [e-mail]; [e-mail]; linux-
> > > [e-mail]; Wendy Liang <[e-mail]>
> > > Subject: [PATCH 6/7] remoteproc: Add Xilinx ZynqMP R5 remoteproc
> > >
> > > There are cortex-r5 processors in Xilinx Zynq UltraScale+ MPSoC
> > > platforms. This remoteproc driver is to manage the
> > > R5 processors.
> > >
> > > Signed-off-by: Wendy Liang <[e-mail]>
> >
> > Jason Wu' signed-off-by missing as he is mentioned as author of this
> driver?
> [Wendy] He was the one who wrote the init version of the driver.
> But he left the company a few years ago. In this case, maybe I should remove
> Module author.

As you want, but look strange for a new driver that author is not in CC or signed-off.
Maybe you are the main author now...

> >
> > > ---
> > >  drivers/remoteproc/Kconfig                |   9 +
> > >  drivers/remoteproc/Makefile               |   1 +
> > >  drivers/remoteproc/zynqmp_r5_remoteproc.c | 692
> > > ++++++++++++++++++++++++++++++
> > >  3 files changed, 702 insertions(+)
> > >  create mode 100644 drivers/remoteproc/zynqmp_r5_remoteproc.c
> > >
> > > diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> > > index cd1c168..83aac63 100644
> > > --- a/drivers/remoteproc/Kconfig
> > > +++ b/drivers/remoteproc/Kconfig
> > > @@ -158,6 +158,15 @@ config ST_REMOTEPROC  config
> > ST_SLIM_REMOTEPROC
> > >  	tristate
> > >
> > > +config ZYNQMP_R5_REMOTEPROC
> > > +	tristate "ZynqMP_r5 remoteproc support"
> > > +	depends on ARM64 && PM && ARCH_ZYNQMP
> > > +	select RPMSG_VIRTIO
> > > +	select ZYNQMP_FIRMWARE
> > > +	help
> > > +	  Say y 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 02627ed..147923c 100644
> > > --- a/drivers/remoteproc/Makefile
> > > +++ b/drivers/remoteproc/Makefile
> > > @@ -23,3 +23,4 @@ qcom_wcnss_pil-y			+=
> > > qcom_wcnss.o
> > >  qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
> > >  obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
> > >  obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.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 0000000..7fc3718
> > > --- /dev/null
> > > +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> > > @@ -0,0 +1,692 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Zynq R5 Remote Processor driver
> > > + *
> > > + * Copyright (C) 2015 Xilinx, Inc.
> > > + *
> > > + */
> > > +
> > > +#include <linux/kernel.h>
> > > +#include <linux/module.h>
> > > +#include <linux/err.h>
> > > +#include <linux/platform_device.h>
> > > +#include <linux/dma-mapping.h>
> > > +#include <linux/remoteproc.h>
> > > +#include <linux/interrupt.h>
> > > +#include <linux/of_address.h>
> > > +#include <linux/of_irq.h>
> > > +#include <linux/of_platform.h>
> > > +#include <linux/of_reserved_mem.h>
> > > +#include <linux/slab.h>
> > > +#include <linux/cpu.h>
> > > +#include <linux/delay.h>
> > > +#include <linux/list.h>
> > > +#include <linux/genalloc.h>
> > > +#include <linux/pfn.h>
> > > +#include <linux/idr.h>
> > > +#include <linux/firmware/xlnx-zynqmp.h>
> >
> > Includes to be classified in alphabetical order
> [Wendy] Will do in the next version
> >
> > > +
> > > +#include "remoteproc_internal.h"
> > > +
> > > +/* IPI reg offsets */
> > > +#define TRIG_OFFSET		0x00000000
> > > +#define OBS_OFFSET		0x00000004
> > > +#define ISR_OFFSET		0x00000010
> > > +#define IMR_OFFSET		0x00000014
> > > +#define IER_OFFSET		0x00000018
> > > +#define IDR_OFFSET		0x0000001C
> > > +#define IPI_ALL_MASK		0x0F0F0301
> > > +
> > > +/* RPU IPI mask */
> > > +#define RPU_IPI_INIT_MASK	0x00000100
> > > +#define RPU_IPI_MASK(n)		(RPU_IPI_INIT_MASK << (n))
> > > +#define RPU_0_IPI_MASK		RPU_IPI_MASK(0)
> > > +#define RPU_1_IPI_MASK		RPU_IPI_MASK(1)
> > > +
> > > +/* PM proc states */
> > > +#define PM_PROC_STATE_ACTIVE 1u
> > > +
> > > +/* Maximum TCM power nodes IDs */
> > > +#define MAX_TCM_PNODES 4
> > > +
> > > +/* Register access macros */
> > > +#define reg_read(base, reg) \
> > > +	readl(((void __iomem *)(base)) + (reg)) #define reg_write(base, reg,
> > > +val) \
> > > +	writel((val), ((void __iomem *)(base)) + (reg))
> > > +
> > > +#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
> > > +
> > > +static bool autoboot __read_mostly;
> > > +
> > > +struct zynqmp_r5_rproc_pdata;
> >
> > This definition is not needed as complete definition just below.
> [Wendy] Will remove in the next version
> > > +
> > > +/**
> > > + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor
> > > +instance
> > > state
> > > + * @rproc: rproc handle
> > > + * @workqueue: workqueue for the RPU remoteproc
> > > + * @ipi_base: virt ptr to IPI channel address registers for APU
> > > + * @rpu_mode: RPU core configuration
> > > + * @rpu_id: RPU CPU id
> > > + * @rpu_pnode_id: RPU CPU power domain id
> > > + * @mem_pools: list of gen_pool for firmware mmio_sram memory and
> > > their
> > > + *             power domain IDs
> > > + * @mems: list of rproc_mem_entries for firmware
> > > + * @irq: IRQ number
> > > + * @ipi_dest_mask: IPI destination mask for the IPI channel  */
> > > +struct zynqmp_r5_rproc_pdata {
> > > +	struct rproc *rproc;
> > > +	struct work_struct workqueue;
> > > +	void __iomem *ipi_base;
> > > +	enum rpu_oper_mode rpu_mode;
> > > +	struct list_head mems;
> > > +	u32 ipi_dest_mask;
> > > +	u32 rpu_id;
> > > +	u32 rpu_pnode_id;
> > > +	int irq;
> > > +	u32 tcm_pnode_id[MAX_TCM_PNODES];
> > > +};
> > > +
> > > +/**
> > > + * r5_boot_addr_config - configure the boot address of R5
> > > + * @pdata: platform data
> > > + * @bootmem: boot from LOVEC or HIVEC
> > > + *
> > > + * This function will set the RPU boot address  */ static void
> > > +r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> > > +				enum rpu_boot_mem bootmem)
> > > +{
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> > > +
> > > +	pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> > > +		 __func__, pdata->rpu_id, bootmem);
> > > +
> > > +	if (!eemi || !eemi->ioctl) {
> > > +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> > > +		return;
> > > +	}
> > > +	eemi->ioctl(pdata->rpu_pnode_id,
> > > IOCTL_RPU_BOOT_ADDR_CONFIG,
> > > +		    bootmem, 0, NULL);
> > > +}
> > > +
> > > +/**
> > > + * r5_mode_config - configure R5 operation mode
> > > + * @pdata: platform data
> > > + *
> > > + * configure R5 to split mode or lockstep mode
> > > + * based on the platform data.
> > > + */
> > > +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> > > +
> > > +	pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> > > +
> > > +	if (!eemi || !eemi->ioctl) {
> > > +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> > > +		return;
> > > +	}
> > > +	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> > > +		    pdata->rpu_mode, 0, NULL);
> > > +}
> > > +
> > > +/**
> > > + * r5_release_tcm() - release TCM
> > > + * @pdata: platform data
> > > + *
> > > + * Release TCM
> > > + */
> > > +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> > > +	int i;
> > > +
> > > +	if (!eemi || !eemi->release_node) {
> > > +		pr_err("Failed to release TCM\n");
> > > +		return;
> > > +	}
> > > +
> > > +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> > > +		if (pdata->tcm_pnode_id[i] != 0)
> > > +			eemi->release_node(pdata->tcm_pnode_id[i]);
> > > +	}
> > > +}
> > > +
> > > +/**
> > > + * disable_ipi - disable IPI
> > > + * @pdata: platform data
> > > + *
> > > + * Disable IPI interrupt
> > > + */
> > > +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	/* Disable R5 IPI interrupt */
> > > +	if (pdata->ipi_base)
> > > +		reg_write(pdata->ipi_base, IDR_OFFSET, pdata-
> > > >ipi_dest_mask);
> > > +}
> >
> > All IPI functions should go below mailbox framework as you use it as a
> > doorbell.
> [Wendy] There is another patch for IPI driver. There are discussions on how
> the mailbox
> Driver should be implemented. I directly access the IPI here is to see if I can
> upstream this
> Remoteproc driver independent to the mailbox driver. I can update this
> driver after the
> Mailbox driver is upstreamed.

In that case upstream first r5 support without RPMsg. That means you will be able to load, start and stop your coprocessor but not establish RPMsg link.
Then once IPI mailbox upstreamed, add kick and notify functions for virtio_rpmsg support. 
Doesn't make sense to upstream temporary code like this one.

> >
> > > +
> > > +/**
> > > + * enable_ipi - enable IPI
> > > + * @pdata: platform data
> > > + *
> > > + * Enable IPI interrupt
> > > + */
> > > +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	/* Enable R5 IPI interrupt */
> > > +	if (pdata->ipi_base)
> > > +		reg_write(pdata->ipi_base, IER_OFFSET, pdata-
> > > >ipi_dest_mask);
> > > +}
> > > +
> > > +/**
> > > + * 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;
> > > +}
> > > +
> > > +static void handle_event_notified(struct work_struct *work) {
> > > +	struct rproc *rproc;
> > > +	struct zynqmp_r5_rproc_pdata *local;
> > > +
> > > +	local = container_of(work, struct zynqmp_r5_rproc_pdata,
> > > workqueue);
> > > +	rproc = local->rproc;
> > > +	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc); }
> > > +
> > > +static int zynqmp_r5_rproc_start(struct rproc *rproc) {
> > > +	struct device *dev = rproc->dev.parent;
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +	enum rpu_boot_mem bootmem;
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> > > +
> > > +	dev_dbg(dev, "%s\n", __func__);
> > > +
> > > +	if (!eemi || !eemi->force_powerdown ||
> > > +	    !eemi->request_wakeup) {
> > > +		pr_err("Failed to start R5\n");
> > > +		return -ENXIO;
> > > +	}
> > > +
> > > +	/* Set up R5 */
> > > +	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> > > +		bootmem = PM_RPU_BOOTMEM_HIVEC;
> > > +	else
> > > +		bootmem = PM_RPU_BOOTMEM_LOVEC;
> > > +	dev_info(dev, "RPU boot from %s.",
> > > +		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" :
> > > "TCM");
> > > +
> > > +	r5_mode_config(local);
> > > +	eemi->force_powerdown(local->rpu_pnode_id,
> > > +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > > +	r5_boot_addr_config(local, bootmem);
> >
> > Add some blank line to ease code reading. Some parts of the code are too
> > compacted.
> Will do in the next version.
> >
> > > +	/* Add delay before release from halt and reset */
> > > +	usleep_range(400, 500);
> > > +	eemi->request_wakeup(local->rpu_pnode_id,
> > > +			     1, bootmem,
> > > +			     ZYNQMP_PM_REQUEST_ACK_NO);
> > > +
> > > +	/* Make sure IPI is enabled */
> > > +	enable_ipi(local);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +/* kick a firmware */
> > > +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid) {
> > > +	struct device *dev = rproc->dev.parent;
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +
> > > +	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n",
> > > vqid);
> > > +
> > > +	/*
> > > +	 * send irq to R5 firmware
> > > +	 * Currently vqid is not used because we only got one.
> > > +	 */
> > > +	if (local->ipi_base)
> > > +		reg_write(local->ipi_base, TRIG_OFFSET, local-
> > > >ipi_dest_mask);
> > > +}
> > > +
> > > +/* power off the remote processor */
> > > +static int zynqmp_r5_rproc_stop(struct rproc *rproc) {
> > > +	struct device *dev = rproc->dev.parent;
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +	struct rproc_mem_entry *mem, *nmem;
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> >
> > Is it pointer on secure interface to control coprocessor?
> > As it is used in most of functions, maybe ops could be recovered and
> > checked only once at probe time?
> I will do the checking at probe time. And fails probing if it is not there.
> >
> > > +
> > > +	dev_dbg(dev, "%s\n", __func__);
> > > +
> > > +	if (!eemi || !eemi->force_powerdown) {
> > > +		pr_err("Failed to stop R5\n");
> > > +		return -ENXIO;
> > > +	}
> > > +
> > > +	disable_ipi(local);
> > > +	eemi->force_powerdown(local->rpu_pnode_id,
> > > +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da,
> > > +int len) {
> > > +	struct rproc_mem_entry *mem;
> > > +	void *va = NULL;
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +
> > > +	list_for_each_entry(mem, &local->mems, node) {
> > > +		int offset = da - mem->da;
> > > +
> > > +		/* try next carveout if da is too small */
> > > +		if (offset < 0)
> > > +			continue;
> > > +
> > > +		/* try next carveout if da is too large */
> > > +		if (offset + len > mem->len)
> > > +			continue;
> > > +
> > > +		va = mem->va + offset;
> > > +
> > > +		break;
> > > +	}
> > > +	return va;
> > > +}
> > > +
> > > +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct
> > > +firmware
> > > *fw)
> > > +{
> > > +	int ret;
> > > +
> > > +	ret = rproc_elf_load_rsc_table(rproc, fw);
> > > +	if (ret == -EINVAL)
> > > +		/* No resource table */
> > > +		return 0;
> > > +	else
> > > +		return ret;
> > > +}
> > > +
> > > +static struct rproc_ops zynqmp_r5_rproc_ops = {
> > > +	.start		= zynqmp_r5_rproc_start,
> > > +	.stop		= zynqmp_r5_rproc_stop,
> > > +	.kick		= zynqmp_r5_rproc_kick,
> > > +	.da_to_va       = zynqmp_r5_rproc_da_to_va,
> > > +};
> > > +
> > > +/* Release R5 from reset and make it halted.
> > > + * In case the firmware uses TCM, in order to load firmware to TCM,
> > > + * will need to release R5 from reset and stay in halted state.
> > > + */
> > > +static int zynqmp_r5_rproc_init(struct rproc *rproc) {
> > > +	struct device *dev = rproc->dev.parent;
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +
> > > +	dev_dbg(dev, "%s\n", __func__);
> > > +	enable_ipi(local);
> > > +	return 0;
> > > +}
> >
> > Comment not aligned with function content. Only IPI enable here.
> Will fix the comments in the next version.
> > > +
> > > +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id) {
> > > +	struct device *dev = dev_id;
> > > +	struct platform_device *pdev = to_platform_device(dev);
> > > +	struct rproc *rproc = platform_get_drvdata(pdev);
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +	u32 ipi_reg;
> > > +
> > > +	/* Check if there is a kick from R5 */
> > > +	ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> > > +	if (!(ipi_reg & local->ipi_dest_mask))
> > > +		return IRQ_NONE;
> > > +
> > > +	dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n",
> > > irq);
> > > +	reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> > > +	schedule_work(&local->workqueue);
> > > +
> > > +	return IRQ_HANDLED;
> > > +}
> >
> > Should be part of IPI mailbox driver.
> [Wendy] the IPI mailbox driver patches are under discussion. I am trying to
> upstream
> this driver independent to the mailbox driver. I can update this driver after
> the
> Mailbox driver is upstreamed.
> 
> >
> > > +
> > > +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> > > + * @pdev: pointer to the platform device
> > > + * @pdata: pointer to the remoteproc private data
> > > + *
> > > + * Function to create remoteproc memory entries for TCM memories.
> > > + */
> > > +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> > > +			      struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> > > +	struct device *dev = &pdev->dev;
> > > +	struct device_node *np = dev->of_node;
> > > +	int num_mems = 0;
> > > +	int i, ret;
> > > +	struct property *prop;
> > > +	const __be32 *cur;
> > > +	u32 val;
> > > +	const struct zynqmp_eemi_ops *eemi =
> > > zynqmp_pm_get_eemi_ops();
> > > +
> > > +	/* Get TCM power node ids */
> > > +	i = 0;
> > > +	of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> > > +		pdata->tcm_pnode_id[i++] = val;
> > > +
> > > +	/* Request TCMs */
> > > +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> > > +		if (pdata->tcm_pnode_id[i] != 0) {
> > > +			ret = eemi->request_node(pdata->tcm_pnode_id[i],
> > > +
> > > ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> > > +
> > > ZYNQMP_PM_REQUEST_ACK_BLOCKING
> > > +						);
> > > +			if (ret < 0) {
> > > +				dev_err(dev, "failed to request TCM: %u\n",
> > > +					pdata->tcm_pnode_id[i]);
> > > +				return ret;
> > > +			}
> > > +			dev_dbg(dev, "request tcm pnode: %u\n",
> > > +				pdata->tcm_pnode_id[i]);
> > > +		} else {
> > > +			break;
> > > +		}
> > > +	}
> > > +	/* Create remoteproc memories entries for TCM memories */
> > > +	num_mems = ARRAY_SIZE(mem_names);
> > > +	for (i = 0; i < num_mems; i++) {
> > > +		struct resource *res;
> > > +		struct rproc_mem_entry *mem;
> > > +		dma_addr_t dma;
> > > +		resource_size_t size;
> > > +
> > > +		res = platform_get_resource_byname(pdev,
> > > IORESOURCE_MEM,
> > > +						   mem_names[i]);
> > > +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > > +				    GFP_KERNEL);
> > > +		if (!mem)
> > > +			return -ENOMEM;
> > > +		/* Map it as normal memory */
> > > +		size = resource_size(res);
> > > +		mem->va = devm_ioremap_wc(dev, res->start, size);
> > > +		mem->len = size;
> > > +		dma = (dma_addr_t)res->start;
> > > +		mem->dma = dma;
> > > +		/* TCM memory:
> > > +		 *   TCM_0: da 0 <-> global addr 0xFFE00000
> > > +		 *   TCM_1: da 0 <-> global addr 0xFFE90000
> > > +		 */
> > > +		if ((dma & 0xFFF00000) == 0xFFE00000) {
> > > +			if ((dma & 0xFFF80000) == 0xFFE80000)
> > > +				mem->da -= 0x90000;
> > > +			else
> > > +				mem->da = (dma & 0x000FFFFF);
> > > +		}
> > > +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > > +			__func__, mem->va, mem->da, mem->dma);
> > > +		list_add_tail(&mem->node, &pdata->mems);
> >
> > Looks like carevout patch series [1]. Did you try it? I think it is better to
> > communalize code instead of duplicated it in each platform driver.
> > [1] https://lkml.org/lkml/2018/7/27/612
> I was aware that you have defined rproc_mem_entry_init() and
> rproc_add_carveout()
> Functions, just those patches are not in upstream yet. I didn't use them in
> this patch.
> I can change to use those functions in the next version.

If you want your driver been accepted, carveout series should be upstreamed first as you have a clear dependency on.
Please review and test it.
Then rebase your driver on it to propose final code version.

> >
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> > > + * @pdev: pointer to the platform device
> > > + * @pdata: pointer to the remoteproc private data
> > > + *
> > > + * Function to create remoteproc memory entries from memory-region
> > > + * property.
> > > + */
> > > +static int zynqmp_r5_get_reserved_mems(struct platform_device
> *pdev,
> > > +				       struct zynqmp_r5_rproc_pdata *pdata) {
> > > +	struct device *dev = &pdev->dev;
> > > +	struct device_node *np = dev->of_node;
> > > +	int num_mems;
> > > +	int i;
> > > +
> > > +	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 resource res;
> > > +		resource_size_t size;
> > > +		struct rproc_mem_entry *mem;
> > > +		int ret;
> > > +
> > > +		node = of_parse_phandle(np, "memory-region", i);
> > > +		ret = of_device_is_compatible(node, "rproc-prog-memory");
> > > +		if (!ret) {
> > > +			/* it is DMA memory. */
> > > +			dev_info(dev, "%s, dma memory %d\n", __func__,
> > > i);
> > > +			ret = of_reserved_mem_device_init_by_idx(dev,
> > > +								 np, i);
> > > +			if (ret) {
> > > +				dev_err(dev, "unable to reserve DMA
> > > mem.\n");
> > > +				return ret;
> > > +			}
> > > +			continue;
> > > +		}
> > > +		ret = of_address_to_resource(node, 0, &res);
> > > +		if (ret) {
> > > +			dev_err(dev, "unable to resolve memory region.\n");
> > > +			return ret;
> > > +		}
> > > +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> > > +				    GFP_KERNEL);
> > > +		if (!mem)
> > > +			return -ENOMEM;
> > > +		/* Map it as normal memory */
> > > +		size = resource_size(&res);
> > > +		mem->va = devm_ioremap_wc(dev, res.start, size);
> > > +		mem->len = size;
> > > +		mem->dma = (dma_addr_t)res.start;
> > > +		mem->da = (u32)res.start;
> > > +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> > > +			__func__, mem->va, mem->da, mem->dma);
> > > +		list_add_tail(&mem->node, &pdata->mems);
> > > +	}
> > > +	return 0;
> > > +}
> > > +
> > > +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> {
> > > +	const unsigned char *prop;
> > > +	struct resource *res;
> > > +	int ret = 0;
> > > +	struct zynqmp_r5_rproc_pdata *local;
> > > +	struct rproc *rproc;
> > > +
> > > +	rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> > > +			    &zynqmp_r5_rproc_ops, NULL,
> > > +		sizeof(struct zynqmp_r5_rproc_pdata));
> > > +	if (!rproc) {
> > > +		dev_err(&pdev->dev, "rproc allocation failed\n");
> > > +		return -ENOMEM;
> > > +	}
> > > +	local = rproc->priv;
> > > +	local->rproc = rproc;
> > > +
> > > +	platform_set_drvdata(pdev, rproc);
> > > +
> > > +	/* Override parse_fw op to allow no resource table firmware */
> > > +	rproc->ops->parse_fw = zynqmp_r5_parse_fw;
> > > +
> > > +	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n",
> > > ret);
> > > +		goto rproc_fault;
> > > +	}
> > > +
> > > +	/* Get the RPU power domain id */
> > > +	ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> > > +				   &local->rpu_pnode_id);
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "No RPU power node ID is
> > > specified.\n");
> > > +		ret = -EINVAL;
> > > +		goto rproc_fault;
> > > +	}
> > > +	dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> > > +		local->rpu_id, local->rpu_pnode_id);
> > > +
> > > +	prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> > > +	if (!prop) {
> > > +		dev_warn(&pdev->dev, "default core_conf used: lock-
> > > step\n");
> > > +		prop = "lock-step";
> > > +	}
> > > +
> > > +	dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
> > > +	if (!strcmp(prop, "split0")) {
> > > +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> > > +		local->rpu_id = 0;
> > > +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> > > +	} else if (!strcmp(prop, "split1")) {
> > > +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> > > +		local->rpu_id = 1;
> > > +		local->ipi_dest_mask = RPU_1_IPI_MASK;
> > > +	} else if (!strcmp(prop, "lock-step")) {
> > > +		local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> > > +		local->rpu_id = 0;
> > > +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> > > +	} else {
> > > +		dev_err(&pdev->dev, "Invalid core_conf mode provided - %s
> > > , %d\n",
> > > +			prop, local->rpu_mode);
> > > +		ret = -EINVAL;
> > > +		goto rproc_fault;
> > > +	}
> > > +
> > > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> > > "ipi");
> > > +	if (res) {
> > > +		local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> > > +					       resource_size(res));
> > > +		if (IS_ERR(local->ipi_base)) {
> > > +			pr_err("%s: Unable to map IPI\n", __func__);
> > > +			ret = PTR_ERR(local->ipi_base);
> > > +			goto rproc_fault;
> > > +		}
> > > +	} else {
> > > +		dev_info(&pdev->dev, "IPI resource is not specified.\n");
> > > +	}
> > > +	dev_dbg(&pdev->dev, "got ipi base address\n");
> > > +
> > > +	INIT_LIST_HEAD(&local->mems);
> > > +	/* Get TCM memories */
> > > +	ret = zynqmp_r5_get_tcms(pdev, local);
> > > +	if (ret < 0) {
> > > +		dev_err(&pdev->dev, "failed to get TCM memories.\n");
> > > +		goto rproc_fault;
> > > +	}
> > > +	dev_dbg(&pdev->dev, "got TCM memories\n");
> > > +	/* Get reserved memory regions for firmware */
> > > +	ret = zynqmp_r5_get_reserved_mems(pdev, local);
> > > +	if (ret < 0) {
> > > +		dev_err(&pdev->dev, "failed to get reserved memories.\n");
> > > +		goto rproc_fault;
> > > +	}
> > > +	dev_dbg(&pdev->dev, "got reserved memories.\n");
> > > +
> > > +	/* Disable IPI before requesting IPI IRQ */
> > > +	disable_ipi(local);
> > > +	INIT_WORK(&local->workqueue, handle_event_notified);
> > > +
> > > +	/* IPI IRQ */
> > > +	if (local->ipi_base) {
> > > +		ret = platform_get_irq(pdev, 0);
> > > +		if (ret < 0) {
> > > +			dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> > > +			goto rproc_fault;
> > > +		}
> > > +		local->irq = ret;
> > > +		ret = devm_request_irq(&pdev->dev, local->irq,
> > > +				       r5_remoteproc_interrupt, IRQF_SHARED,
> > > +				       dev_name(&pdev->dev), &pdev->dev);
> > > +		if (ret) {
> > > +			dev_err(&pdev->dev, "IRQ %d already allocated\n",
> > > +				local->irq);
> > > +			goto rproc_fault;
> > > +		}
> > > +		dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> > > +	}
> > > +
> > > +	ret = zynqmp_r5_rproc_init(local->rproc);
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> > > +		goto rproc_fault;
> > > +	}
> > > +
> > > +	rproc->auto_boot = autoboot;
> > > +
> > > +	ret = rproc_add(local->rproc);
> > > +	if (ret) {
> > > +		dev_err(&pdev->dev, "rproc registration failed\n");
> > > +		goto rproc_fault;
> > > +	}
> > > +
> > > +	return ret;
> > > +
> > > +rproc_fault:
> > > +	rproc_free(local->rproc);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > > +static int zynqmp_r5_remoteproc_remove(struct platform_device
> *pdev)
> > > +{
> > > +	struct rproc *rproc = platform_get_drvdata(pdev);
> > > +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> > > +	struct rproc_mem_entry *mem;
> > > +
> > > +	dev_info(&pdev->dev, "%s\n", __func__);
> > > +
> > > +	rproc_del(rproc);
> > > +
> > > +	list_for_each_entry(mem, &local->mems, node) {
> > > +		if (mem->priv)
> > > +			gen_pool_free((struct gen_pool *)mem->priv,
> > > +				      (unsigned long)mem->va, mem->len);
> > > +	}
> > > +
> > > +	r5_release_tcm(local);
> > > +	of_reserved_mem_device_release(&pdev->dev);
> > > +	rproc_free(rproc);
> > > +
> > > +	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_param_named(autoboot,  autoboot, bool, 0444);
> > > +MODULE_PARM_DESC(autoboot,
> > > +		 "enable | disable autoboot. (default: true)");
> > > +
> > > +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> > MODULE_LICENSE("GPL
> > > +v2"); MODULE_DESCRIPTION("ZynqMP R5 remote processor control
> > > +driver");
> > > --
> > > 2.7.4

Bjorn Andersson Oct. 6, 2018, 5:44 a.m. UTC | #6

On Thu 16 Aug 00:06 PDT 2018, Wendy Liang wrote:
> diff --git a/drivers/remoteproc/zynqmp_r5_remoteproc.c b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> new file mode 100644
> index 0000000..7fc3718
> --- /dev/null
> +++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
> @@ -0,0 +1,692 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Zynq R5 Remote Processor driver
> + *
> + * Copyright (C) 2015 Xilinx, Inc.
> + *
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/err.h>
> +#include <linux/platform_device.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/remoteproc.h>
> +#include <linux/interrupt.h>
> +#include <linux/of_address.h>
> +#include <linux/of_irq.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_reserved_mem.h>
> +#include <linux/slab.h>
> +#include <linux/cpu.h>
> +#include <linux/delay.h>
> +#include <linux/list.h>
> +#include <linux/genalloc.h>
> +#include <linux/pfn.h>
> +#include <linux/idr.h>
> +#include <linux/firmware/xlnx-zynqmp.h>
> +
> +#include "remoteproc_internal.h"
> +
> +/* IPI reg offsets */
> +#define TRIG_OFFSET		0x00000000
> +#define OBS_OFFSET		0x00000004
> +#define ISR_OFFSET		0x00000010
> +#define IMR_OFFSET		0x00000014
> +#define IER_OFFSET		0x00000018
> +#define IDR_OFFSET		0x0000001C
> +#define IPI_ALL_MASK		0x0F0F0301
> +
> +/* RPU IPI mask */
> +#define RPU_IPI_INIT_MASK	0x00000100
> +#define RPU_IPI_MASK(n)		(RPU_IPI_INIT_MASK << (n))
> +#define RPU_0_IPI_MASK		RPU_IPI_MASK(0)
> +#define RPU_1_IPI_MASK		RPU_IPI_MASK(1)

Rather than using 2 levels of macros, just define RPU_0_IPI_MASK and
RPU_1_IPI_MASK as BIT(8) and BIT(9)

> +
> +/* PM proc states */
> +#define PM_PROC_STATE_ACTIVE 1u

Unused

> +
> +/* Maximum TCM power nodes IDs */
> +#define MAX_TCM_PNODES 4
> +
> +/* Register access macros */
> +#define reg_read(base, reg) \
> +	readl(((void __iomem *)(base)) + (reg))
> +#define reg_write(base, reg, val) \
> +	writel((val), ((void __iomem *)(base)) + (reg))

Please drop these macros, using readl/writel directly rather than hiding
it behind a similar macro will make it easier to read the code.

> +
> +#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
> +
> +static bool autoboot __read_mostly;

A variable only read during probe() doesn't need hints.

> +
> +struct zynqmp_r5_rproc_pdata;

No need to forward declare this, as the very next statement is the
declaration of this struct.

> +
> +/**
> + * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor instance state
> + * @rproc: rproc handle
> + * @workqueue: workqueue for the RPU remoteproc
> + * @ipi_base: virt ptr to IPI channel address registers for APU
> + * @rpu_mode: RPU core configuration
> + * @rpu_id: RPU CPU id
> + * @rpu_pnode_id: RPU CPU power domain id
> + * @mem_pools: list of gen_pool for firmware mmio_sram memory and their
> + *             power domain IDs

mem_pools is not a member of the struct.

> + * @mems: list of rproc_mem_entries for firmware

Please reorder to match struct.

> + * @irq: IRQ number
> + * @ipi_dest_mask: IPI destination mask for the IPI channel
> + */
> +struct zynqmp_r5_rproc_pdata {
> +	struct rproc *rproc;
> +	struct work_struct workqueue;

This is the work object, not the work queue. Please update naming
("work" is a common choice to this).

> +	void __iomem *ipi_base;
> +	enum rpu_oper_mode rpu_mode;
> +	struct list_head mems;

Consider renaming to mem_entries.

> +	u32 ipi_dest_mask;
> +	u32 rpu_id;
> +	u32 rpu_pnode_id;
> +	int irq;
> +	u32 tcm_pnode_id[MAX_TCM_PNODES];
> +};
> +
> +/**
> + * r5_boot_addr_config - configure the boot address of R5

Add () on the function name in kerneldoc.

> + * @pdata: platform data
> + * @bootmem: boot from LOVEC or HIVEC
> + *
> + * This function will set the RPU boot address
> + */
> +static void r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
> +				enum rpu_boot_mem bootmem)
> +{
> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();

I presume this will return the same eemi as when it was called right
before in zynqmp_r5_rproc_start(). How about passing eemi from the
caller?

> +
> +	pr_debug("%s: R5 ID: %d, boot_dev %d\n",
> +		 __func__, pdata->rpu_id, bootmem);
> +
> +	if (!eemi || !eemi->ioctl) {

If eemi is NULL zynqmp_r5_rproc_start() already aborted. How about
making zynqmp_r5_rproc_start() also check to see that eemi->ioctl is
non-NULL? and then just skip this check.

> +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> +		return;
> +	}
> +	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_RPU_BOOT_ADDR_CONFIG,
> +		    bootmem, 0, NULL);
> +}
> +
> +/**
> + * r5_mode_config - configure R5 operation mode
> + * @pdata: platform data
> + *
> + * configure R5 to split mode or lockstep mode
> + * based on the platform data.
> + */
> +static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();

Same comments as for r5_boot_addr_config()

> +
> +	pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
> +
> +	if (!eemi || !eemi->ioctl) {
> +		pr_err("%s: no eemi ioctl operation.\n", __func__);
> +		return;
> +	}
> +	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
> +		    pdata->rpu_mode, 0, NULL);
> +}
> +
> +/**
> + * r5_release_tcm() - release TCM
> + * @pdata: platform data
> + *
> + * Release TCM
> + */
> +static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();

Consider acquiring eemi in zynqmp_r5_remoteproc_remove() and pass it as
an argument to this function - to get symmetry with the functions called
during start.

> +	int i;
> +
> +	if (!eemi || !eemi->release_node) {
> +		pr_err("Failed to release TCM\n");
> +		return;
> +	}
> +
> +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> +		if (pdata->tcm_pnode_id[i] != 0)

See comment in zynqmp_r5_get_tcms() about this conditional.

> +			eemi->release_node(pdata->tcm_pnode_id[i]);
> +	}
> +}
> +
> +/**
> + * disable_ipi - disable IPI
> + * @pdata: platform data
> + *
> + * Disable IPI interrupt
> + */
> +static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata)

Please give this a less generic name and drop the "inline".

> +{
> +	/* Disable R5 IPI interrupt */
> +	if (pdata->ipi_base)
> +		reg_write(pdata->ipi_base, IDR_OFFSET, pdata->ipi_dest_mask);
> +}
> +
> +/**
> + * enable_ipi - enable IPI
> + * @pdata: platform data
> + *
> + * Enable IPI interrupt
> + */
> +static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata)

Please give this a less generic name and drop the "inline".

> +{
> +	/* Enable R5 IPI interrupt */
> +	if (pdata->ipi_base)
> +		reg_write(pdata->ipi_base, IER_OFFSET, pdata->ipi_dest_mask);
> +}
> +
> +/**
> + * 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

Please mention that data is a rproc handle.

> + *
> + * Pass notification to remoteproc virtio
> + *
> + * @return: 0. having return is to satisfy the idr_for_each() function

 * Return: 0

> + *          pointer input argument requirement.
> + */
> +static int event_notified_idr_cb(int id, void *ptr, void *data)

Please give this a less generic name.

> +{
> +	struct rproc *rproc = data;
> +
> +	(void)rproc_vq_interrupt(rproc, id);

You shouldn't need the (void) cast here and you can just pass "data"
directly as the first parameter.

> +	return 0;
> +}
> +
> +static void handle_event_notified(struct work_struct *work)

Please give this a less generic name.

> +{
> +	struct rproc *rproc;
> +	struct zynqmp_r5_rproc_pdata *local;
> +
> +	local = container_of(work, struct zynqmp_r5_rproc_pdata, workqueue);
> +	rproc = local->rproc;
> +	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);

So this is rproc_vq_interrupt(), but triggering all notifyids. This will
be the case for any platform that has lumped together the kick in a
single interrupt, so improve rproc_vq_interrupt() to allow for a
notifyid such as -1 will trigger all interrupts.

> +}
> +
> +static int zynqmp_r5_rproc_start(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	enum rpu_boot_mem bootmem;
> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +	dev_dbg(dev, "%s\n", __func__);
> +
> +	if (!eemi || !eemi->force_powerdown ||
> +	    !eemi->request_wakeup) {
> +		pr_err("Failed to start R5\n");

Please use dev_err()

The error your hitting here isn't that you failed to start the R5, it's
that the returned eemi is "broken", please update the error message to
reflect this.


Also, I would presume the eemi_ops are pretty static, so do consider
getting a reference in probe() and store that in your r5_rproc_pdata -
that way you can actually handle crazy things like
zynqmp_pm_get_eemi_ops() not being available and returning EPROBE_DEFER
and you don't need to check that it's valid all over the place.

> +		return -ENXIO;
> +	}
> +
> +	/* Set up R5 */
> +	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
> +		bootmem = PM_RPU_BOOTMEM_HIVEC;
> +	else
> +		bootmem = PM_RPU_BOOTMEM_LOVEC;
> +	dev_info(dev, "RPU boot from %s.",
> +		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");

Rather than having a print with a conditional just inline two static
prints in the if/else..

> +
> +	r5_mode_config(local);
> +	eemi->force_powerdown(local->rpu_pnode_id,
> +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +	r5_boot_addr_config(local, bootmem);
> +	/* Add delay before release from halt and reset */
> +	usleep_range(400, 500);
> +	eemi->request_wakeup(local->rpu_pnode_id,
> +			     1, bootmem,
> +			     ZYNQMP_PM_REQUEST_ACK_NO);
> +
> +	/* Make sure IPI is enabled */
> +	enable_ipi(local);
> +
> +	return 0;
> +}
> +
> +/* kick a firmware */
> +static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
> +{
> +	struct device *dev = rproc->dev.parent;

Store the zynqmp_r5 device pointer in your pdata.

> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", vqid);
> +
> +	/*
> +	 * send irq to R5 firmware
> +	 * Currently vqid is not used because we only got one.
> +	 */
> +	if (local->ipi_base)
> +		reg_write(local->ipi_base, TRIG_OFFSET, local->ipi_dest_mask);
> +}
> +
> +/* power off the remote processor */
> +static int zynqmp_r5_rproc_stop(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	struct rproc_mem_entry *mem, *nmem;

mem and nmem are unused.

> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +	dev_dbg(dev, "%s\n", __func__);
> +
> +	if (!eemi || !eemi->force_powerdown) {
> +		pr_err("Failed to stop R5\n");
> +		return -ENXIO;
> +	}
> +
> +	disable_ipi(local);
> +	eemi->force_powerdown(local->rpu_pnode_id,
> +			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> +
> +	return 0;
> +}
> +
> +static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da, int len)

We should be able to drop this after getting Loic's carveout series in,
but for now I think this looks reasonable.

> +{
> +	struct rproc_mem_entry *mem;
> +	void *va = NULL;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	list_for_each_entry(mem, &local->mems, node) {
> +		int offset = da - mem->da;
> +
> +		/* try next carveout if da is too small */
> +		if (offset < 0)
> +			continue;
> +
> +		/* try next carveout if da is too large */
> +		if (offset + len > mem->len)
> +			continue;
> +
> +		va = mem->va + offset;
> +
> +		break;
> +	}
> +	return va;
> +}
> +
> +static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
> +{
> +	int ret;
> +
> +	ret = rproc_elf_load_rsc_table(rproc, fw);
> +	if (ret == -EINVAL)

Would be nice if rproc_elf_load_rsc_table() returned e.g. ENOENT for
this, to distinguish from a broken resource table.

Please consider a separate patch making find_table() return a ERR_PTR()
and propagate this.

> +		/* No resource table */
> +		return 0;
> +	else
> +		return ret;
> +}
> +
> +static struct rproc_ops zynqmp_r5_rproc_ops = {
> +	.start		= zynqmp_r5_rproc_start,
> +	.stop		= zynqmp_r5_rproc_stop,
> +	.kick		= zynqmp_r5_rproc_kick,
> +	.da_to_va       = zynqmp_r5_rproc_da_to_va,
> +};
> +
> +/* Release R5 from reset and make it halted.
> + * In case the firmware uses TCM, in order to load firmware to TCM,
> + * will need to release R5 from reset and stay in halted state.
> + */
> +static int zynqmp_r5_rproc_init(struct rproc *rproc)
> +{
> +	struct device *dev = rproc->dev.parent;
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +
> +	dev_dbg(dev, "%s\n", __func__);
> +	enable_ipi(local);
> +	return 0;
> +}
> +
> +static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id)
> +{
> +	struct device *dev = dev_id;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	u32 ipi_reg;
> +
> +	/* Check if there is a kick from R5 */
> +	ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
> +	if (!(ipi_reg & local->ipi_dest_mask))
> +		return IRQ_NONE;
> +
> +	dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n", irq);

What is this print trying to say? Which Linux is this kicking?

> +	reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
> +	schedule_work(&local->workqueue);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/* zynqmp_r5_get_tcm_memories() - get tcm memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries for TCM memories.

 * Return: 0 on success, negative errno on failure.

> + */
> +static int zynqmp_r5_get_tcms(struct platform_device *pdev,
> +			      struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	static const char * const mem_names[] = {"tcm_a", "tcm_b"};
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	int num_mems = 0;
> +	int i, ret;
> +	struct property *prop;
> +	const __be32 *cur;
> +	u32 val;
> +	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
> +
> +	/* Get TCM power node ids */
> +	i = 0;
> +	of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
> +		pdata->tcm_pnode_id[i++] = val;
> +
> +	/* Request TCMs */
> +	for (i = 0; i < MAX_TCM_PNODES; i++) {
> +		if (pdata->tcm_pnode_id[i] != 0) {

This will be != for the first i (the i from 5 lines up) and then 0 from
there. Consider carrying a count of the number tcm_pnode_ids, which you
increment in above loop and then use as limit here.

That way you would iterate from i = 0 to pdata->tcm_pnode_id_count (or
something) and you can skip the conditional in this loop and in the
other places where you iterate over it.

> +			ret = eemi->request_node(pdata->tcm_pnode_id[i],
> +						 ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> +						 ZYNQMP_PM_REQUEST_ACK_BLOCKING
> +						);
> +			if (ret < 0) {
> +				dev_err(dev, "failed to request TCM: %u\n",
> +					pdata->tcm_pnode_id[i]);
> +				return ret;
> +			}
> +			dev_dbg(dev, "request tcm pnode: %u\n",
> +				pdata->tcm_pnode_id[i]);
> +		} else {
> +			break;
> +		}
> +	}
> +	/* Create remoteproc memories entries for TCM memories */
> +	num_mems = ARRAY_SIZE(mem_names);

Just move the ARRAY_SIZE() into the for loop conditional.

> +	for (i = 0; i < num_mems; i++) {
> +		struct resource *res;
> +		struct rproc_mem_entry *mem;
> +		dma_addr_t dma;
> +		resource_size_t size;
> +
> +		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +						   mem_names[i]);
> +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +				    GFP_KERNEL);
> +		if (!mem)
> +			return -ENOMEM;

Reorder this to devm_kzalloc() first and then do
platform_get_resource_byname(), this will be a little bit better flow.

> +		/* Map it as normal memory */
> +		size = resource_size(res);
> +		mem->va = devm_ioremap_wc(dev, res->start, size);

devm_ioremap_resource(dev, res)

> +		mem->len = size;
> +		dma = (dma_addr_t)res->start;
> +		mem->dma = dma;
> +		/* TCM memory:
> +		 *   TCM_0: da 0 <-> global addr 0xFFE00000
> +		 *   TCM_1: da 0 <-> global addr 0xFFE90000
> +		 */
> +		if ((dma & 0xFFF00000) == 0xFFE00000) {
> +			if ((dma & 0xFFF80000) == 0xFFE80000)
> +				mem->da -= 0x90000;
> +			else
> +				mem->da = (dma & 0x000FFFFF);
> +		}
> +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +			__func__, mem->va, mem->da, mem->dma);
> +		list_add_tail(&mem->node, &pdata->mems);
> +	}
> +	return 0;
> +}
> +
> +/* zynqmp_r5_get_reserved_mems() - get reserved memories
> + * @pdev: pointer to the platform device
> + * @pdata: pointer to the remoteproc private data
> + *
> + * Function to create remoteproc memory entries from memory-region
> + * property.

 * Return: 0 on success, negative errno on failure.

> + */
> +static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
> +				       struct zynqmp_r5_rproc_pdata *pdata)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct device_node *np = dev->of_node;
> +	int num_mems;
> +	int i;
> +
> +	num_mems = of_count_phandle_with_args(np, "memory-region", NULL);
> +	if (num_mems <= 0)
> +		return 0;

Skip this early return and the for loop won't be entered and you will
return 0 anyways.

> +	for (i = 0; i < num_mems; i++) {
> +		struct device_node *node;
> +		struct resource res;
> +		resource_size_t size;
> +		struct rproc_mem_entry *mem;
> +		int ret;
> +
> +		node = of_parse_phandle(np, "memory-region", i);
> +		ret = of_device_is_compatible(node, "rproc-prog-memory");
> +		if (!ret) {
> +			/* it is DMA memory. */
> +			dev_info(dev, "%s, dma memory %d\n", __func__, i);

Don't use __func__ in info messages.

> +			ret = of_reserved_mem_device_init_by_idx(dev,
> +								 np, i);
> +			if (ret) {
> +				dev_err(dev, "unable to reserve DMA mem.\n");
> +				return ret;
> +			}
> +			continue;
> +		}
> +		ret = of_address_to_resource(node, 0, &res);
> +		if (ret) {
> +			dev_err(dev, "unable to resolve memory region.\n");
> +			return ret;
> +		}
> +		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
> +				    GFP_KERNEL);

The idiomatic way is to do devm_kzalloc(dev, sizeof(*mem), GFP_KERNEL),
which you can fit in one line...

> +		if (!mem)
> +			return -ENOMEM;
> +		/* Map it as normal memory */
> +		size = resource_size(&res);
> +		mem->va = devm_ioremap_wc(dev, res.start, size);

devm_ioremap_resource(dev, res)

> +		mem->len = size;
> +		mem->dma = (dma_addr_t)res.start;
> +		mem->da = (u32)res.start;
> +		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
> +			__func__, mem->va, mem->da, mem->dma);
> +		list_add_tail(&mem->node, &pdata->mems);
> +	}
> +	return 0;
> +}
> +
> +static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
> +{
> +	const unsigned char *prop;
> +	struct resource *res;
> +	int ret = 0;
> +	struct zynqmp_r5_rproc_pdata *local;
> +	struct rproc *rproc;
> +
> +	rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
> +			    &zynqmp_r5_rproc_ops, NULL,
> +		sizeof(struct zynqmp_r5_rproc_pdata));
> +	if (!rproc) {
> +		dev_err(&pdev->dev, "rproc allocation failed\n");
> +		return -ENOMEM;
> +	}
> +	local = rproc->priv;
> +	local->rproc = rproc;
> +
> +	platform_set_drvdata(pdev, rproc);
> +
> +	/* Override parse_fw op to allow no resource table firmware */
> +	rproc->ops->parse_fw = zynqmp_r5_parse_fw;

You can reference rproc_elf_load_segments et al from your definition of
zynqmp_r5_rproc_ops, so that you don't need to "override" this op.

> +
> +	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
> +	if (ret) {
> +		dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n", ret);
> +		goto rproc_fault;
> +	}
> +
> +	/* Get the RPU power domain id */
> +	ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
> +				   &local->rpu_pnode_id);
> +	if (ret) {
> +		dev_err(&pdev->dev, "No RPU power node ID is specified.\n");
> +		ret = -EINVAL;
> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
> +		local->rpu_id, local->rpu_pnode_id);
> +
> +	prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
> +	if (!prop) {
> +		dev_warn(&pdev->dev, "default core_conf used: lock-step\n");
> +		prop = "lock-step";
> +	}
> +
> +	dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);

This is a debug print, rather than info.

> +	if (!strcmp(prop, "split0")) {
> +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> +		local->rpu_id = 0;
> +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> +	} else if (!strcmp(prop, "split1")) {
> +		local->rpu_mode = PM_RPU_MODE_SPLIT;
> +		local->rpu_id = 1;
> +		local->ipi_dest_mask = RPU_1_IPI_MASK;
> +	} else if (!strcmp(prop, "lock-step")) {
> +		local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
> +		local->rpu_id = 0;
> +		local->ipi_dest_mask = RPU_0_IPI_MASK;
> +	} else {
> +		dev_err(&pdev->dev, "Invalid core_conf mode provided - %s , %d\n",
> +			prop, local->rpu_mode);
> +		ret = -EINVAL;
> +		goto rproc_fault;
> +	}
> +
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipi");
> +	if (res) {
> +		local->ipi_base = devm_ioremap(&pdev->dev, res->start,
> +					       resource_size(res));
> +		if (IS_ERR(local->ipi_base)) {
> +			pr_err("%s: Unable to map IPI\n", __func__);
> +			ret = PTR_ERR(local->ipi_base);
> +			goto rproc_fault;
> +		}
> +	} else {
> +		dev_info(&pdev->dev, "IPI resource is not specified.\n");
> +	}
> +	dev_dbg(&pdev->dev, "got ipi base address\n");

Make this print useful or drop it.

> +
> +	INIT_LIST_HEAD(&local->mems);

Group initialization of all the local members to one place above.

> +	/* Get TCM memories */
> +	ret = zynqmp_r5_get_tcms(pdev, local);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get TCM memories.\n");

zynqmp_r5_get_tcms() did already print a more specific error, no need to
print another generic one.

> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "got TCM memories\n");

Make this print useful or drop it.

> +	/* Get reserved memory regions for firmware */
> +	ret = zynqmp_r5_get_reserved_mems(pdev, local);
> +	if (ret < 0) {
> +		dev_err(&pdev->dev, "failed to get reserved memories.\n");

zynqmp_r5_get_reserved_mems() already printed more specific error
messages, drop this.

> +		goto rproc_fault;
> +	}
> +	dev_dbg(&pdev->dev, "got reserved memories.\n");

Make this print useful or drop it.

> +
> +	/* Disable IPI before requesting IPI IRQ */
> +	disable_ipi(local);
> +	INIT_WORK(&local->workqueue, handle_event_notified);
> +
> +	/* IPI IRQ */
> +	if (local->ipi_base) {
> +		ret = platform_get_irq(pdev, 0);
> +		if (ret < 0) {
> +			dev_err(&pdev->dev, "unable to find IPI IRQ\n");
> +			goto rproc_fault;
> +		}
> +		local->irq = ret;
> +		ret = devm_request_irq(&pdev->dev, local->irq,
> +				       r5_remoteproc_interrupt, IRQF_SHARED,
> +				       dev_name(&pdev->dev), &pdev->dev);
> +		if (ret) {
> +			dev_err(&pdev->dev, "IRQ %d already allocated\n",
> +				local->irq);
> +			goto rproc_fault;
> +		}
> +		dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
> +	}
> +
> +	ret = zynqmp_r5_rproc_init(local->rproc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
> +		goto rproc_fault;
> +	}

zynqmp_r5_rproc_init() is just a call to enable_ipi(), which is just a
conditional writel() and can't return anything but 0. Consider just
inlining the writel() here - or at least the enable_ipi() and remove the
error handling.

> +
> +	rproc->auto_boot = autoboot;
> +
> +	ret = rproc_add(local->rproc);
> +	if (ret) {
> +		dev_err(&pdev->dev, "rproc registration failed\n");
> +		goto rproc_fault;
> +	}
> +
> +	return ret;
> +
> +rproc_fault:
> +	rproc_free(local->rproc);
> +
> +	return ret;
> +}
> +
> +static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
> +{
> +	struct rproc *rproc = platform_get_drvdata(pdev);
> +	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
> +	struct rproc_mem_entry *mem;
> +
> +	dev_info(&pdev->dev, "%s\n", __func__);

This isn't a info print.

> +
> +	rproc_del(rproc);
> +
> +	list_for_each_entry(mem, &local->mems, node) {
> +		if (mem->priv)
> +			gen_pool_free((struct gen_pool *)mem->priv,
> +				      (unsigned long)mem->va, mem->len);

I can't find where mem->priv is assigned, is this some old remnant?

> +	}
> +
> +	r5_release_tcm(local);
> +	of_reserved_mem_device_release(&pdev->dev);
> +	rproc_free(rproc);
> +
> +	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 */ },

Drop the comment.

> +};
> +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_param_named(autoboot,  autoboot, bool, 0444);
> +MODULE_PARM_DESC(autoboot,
> +		 "enable | disable autoboot. (default: true)");

Please don't add module_params for this.

> +
> +MODULE_AUTHOR("Jason Wu <[e-mail]>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");

Regards,
Bjorn

Patch

diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index cd1c168..83aac63 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -158,6 +158,15 @@  config ST_REMOTEPROC
 config ST_SLIM_REMOTEPROC
 	tristate
 
+config ZYNQMP_R5_REMOTEPROC
+	tristate "ZynqMP_r5 remoteproc support"
+	depends on ARM64 && PM && ARCH_ZYNQMP
+	select RPMSG_VIRTIO
+	select ZYNQMP_FIRMWARE
+	help
+	  Say y 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 02627ed..147923c 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -23,3 +23,4 @@  qcom_wcnss_pil-y			+= qcom_wcnss.o
 qcom_wcnss_pil-y			+= qcom_wcnss_iris.o
 obj-$(CONFIG_ST_REMOTEPROC)		+= st_remoteproc.o
 obj-$(CONFIG_ST_SLIM_REMOTEPROC)	+= st_slim_rproc.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 0000000..7fc3718
--- /dev/null
+++ b/drivers/remoteproc/zynqmp_r5_remoteproc.c
@@ -0,0 +1,692 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Zynq R5 Remote Processor driver
+ *
+ * Copyright (C) 2015 Xilinx, Inc.
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/platform_device.h>
+#include <linux/dma-mapping.h>
+#include <linux/remoteproc.h>
+#include <linux/interrupt.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/slab.h>
+#include <linux/cpu.h>
+#include <linux/delay.h>
+#include <linux/list.h>
+#include <linux/genalloc.h>
+#include <linux/pfn.h>
+#include <linux/idr.h>
+#include <linux/firmware/xlnx-zynqmp.h>
+
+#include "remoteproc_internal.h"
+
+/* IPI reg offsets */
+#define TRIG_OFFSET		0x00000000
+#define OBS_OFFSET		0x00000004
+#define ISR_OFFSET		0x00000010
+#define IMR_OFFSET		0x00000014
+#define IER_OFFSET		0x00000018
+#define IDR_OFFSET		0x0000001C
+#define IPI_ALL_MASK		0x0F0F0301
+
+/* RPU IPI mask */
+#define RPU_IPI_INIT_MASK	0x00000100
+#define RPU_IPI_MASK(n)		(RPU_IPI_INIT_MASK << (n))
+#define RPU_0_IPI_MASK		RPU_IPI_MASK(0)
+#define RPU_1_IPI_MASK		RPU_IPI_MASK(1)
+
+/* PM proc states */
+#define PM_PROC_STATE_ACTIVE 1u
+
+/* Maximum TCM power nodes IDs */
+#define MAX_TCM_PNODES 4
+
+/* Register access macros */
+#define reg_read(base, reg) \
+	readl(((void __iomem *)(base)) + (reg))
+#define reg_write(base, reg, val) \
+	writel((val), ((void __iomem *)(base)) + (reg))
+
+#define DEFAULT_FIRMWARE_NAME	"rproc-rpu-fw"
+
+static bool autoboot __read_mostly;
+
+struct zynqmp_r5_rproc_pdata;
+
+/**
+ * struct zynqmp_r5_rproc_pdata - zynqmp rpu remote processor instance state
+ * @rproc: rproc handle
+ * @workqueue: workqueue for the RPU remoteproc
+ * @ipi_base: virt ptr to IPI channel address registers for APU
+ * @rpu_mode: RPU core configuration
+ * @rpu_id: RPU CPU id
+ * @rpu_pnode_id: RPU CPU power domain id
+ * @mem_pools: list of gen_pool for firmware mmio_sram memory and their
+ *             power domain IDs
+ * @mems: list of rproc_mem_entries for firmware
+ * @irq: IRQ number
+ * @ipi_dest_mask: IPI destination mask for the IPI channel
+ */
+struct zynqmp_r5_rproc_pdata {
+	struct rproc *rproc;
+	struct work_struct workqueue;
+	void __iomem *ipi_base;
+	enum rpu_oper_mode rpu_mode;
+	struct list_head mems;
+	u32 ipi_dest_mask;
+	u32 rpu_id;
+	u32 rpu_pnode_id;
+	int irq;
+	u32 tcm_pnode_id[MAX_TCM_PNODES];
+};
+
+/**
+ * r5_boot_addr_config - configure the boot address of R5
+ * @pdata: platform data
+ * @bootmem: boot from LOVEC or HIVEC
+ *
+ * This function will set the RPU boot address
+ */
+static void r5_boot_addr_config(struct zynqmp_r5_rproc_pdata *pdata,
+				enum rpu_boot_mem bootmem)
+{
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+
+	pr_debug("%s: R5 ID: %d, boot_dev %d\n",
+		 __func__, pdata->rpu_id, bootmem);
+
+	if (!eemi || !eemi->ioctl) {
+		pr_err("%s: no eemi ioctl operation.\n", __func__);
+		return;
+	}
+	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_RPU_BOOT_ADDR_CONFIG,
+		    bootmem, 0, NULL);
+}
+
+/**
+ * r5_mode_config - configure R5 operation mode
+ * @pdata: platform data
+ *
+ * configure R5 to split mode or lockstep mode
+ * based on the platform data.
+ */
+static void r5_mode_config(struct zynqmp_r5_rproc_pdata *pdata)
+{
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+
+	pr_debug("%s: mode: %d\n", __func__, pdata->rpu_mode);
+
+	if (!eemi || !eemi->ioctl) {
+		pr_err("%s: no eemi ioctl operation.\n", __func__);
+		return;
+	}
+	eemi->ioctl(pdata->rpu_pnode_id, IOCTL_SET_RPU_OPER_MODE,
+		    pdata->rpu_mode, 0, NULL);
+}
+
+/**
+ * r5_release_tcm() - release TCM
+ * @pdata: platform data
+ *
+ * Release TCM
+ */
+static void r5_release_tcm(struct zynqmp_r5_rproc_pdata *pdata)
+{
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+	int i;
+
+	if (!eemi || !eemi->release_node) {
+		pr_err("Failed to release TCM\n");
+		return;
+	}
+
+	for (i = 0; i < MAX_TCM_PNODES; i++) {
+		if (pdata->tcm_pnode_id[i] != 0)
+			eemi->release_node(pdata->tcm_pnode_id[i]);
+	}
+}
+
+/**
+ * disable_ipi - disable IPI
+ * @pdata: platform data
+ *
+ * Disable IPI interrupt
+ */
+static inline void disable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
+{
+	/* Disable R5 IPI interrupt */
+	if (pdata->ipi_base)
+		reg_write(pdata->ipi_base, IDR_OFFSET, pdata->ipi_dest_mask);
+}
+
+/**
+ * enable_ipi - enable IPI
+ * @pdata: platform data
+ *
+ * Enable IPI interrupt
+ */
+static inline void enable_ipi(struct zynqmp_r5_rproc_pdata *pdata)
+{
+	/* Enable R5 IPI interrupt */
+	if (pdata->ipi_base)
+		reg_write(pdata->ipi_base, IER_OFFSET, pdata->ipi_dest_mask);
+}
+
+/**
+ * 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;
+}
+
+static void handle_event_notified(struct work_struct *work)
+{
+	struct rproc *rproc;
+	struct zynqmp_r5_rproc_pdata *local;
+
+	local = container_of(work, struct zynqmp_r5_rproc_pdata, workqueue);
+	rproc = local->rproc;
+	idr_for_each(&rproc->notifyids, event_notified_idr_cb, rproc);
+}
+
+static int zynqmp_r5_rproc_start(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+	enum rpu_boot_mem bootmem;
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	if (!eemi || !eemi->force_powerdown ||
+	    !eemi->request_wakeup) {
+		pr_err("Failed to start R5\n");
+		return -ENXIO;
+	}
+
+	/* Set up R5 */
+	if ((rproc->bootaddr & 0xF0000000) == 0xF0000000)
+		bootmem = PM_RPU_BOOTMEM_HIVEC;
+	else
+		bootmem = PM_RPU_BOOTMEM_LOVEC;
+	dev_info(dev, "RPU boot from %s.",
+		 bootmem == PM_RPU_BOOTMEM_HIVEC ? "OCM" : "TCM");
+
+	r5_mode_config(local);
+	eemi->force_powerdown(local->rpu_pnode_id,
+			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+	r5_boot_addr_config(local, bootmem);
+	/* Add delay before release from halt and reset */
+	usleep_range(400, 500);
+	eemi->request_wakeup(local->rpu_pnode_id,
+			     1, bootmem,
+			     ZYNQMP_PM_REQUEST_ACK_NO);
+
+	/* Make sure IPI is enabled */
+	enable_ipi(local);
+
+	return 0;
+}
+
+/* kick a firmware */
+static void zynqmp_r5_rproc_kick(struct rproc *rproc, int vqid)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+
+	dev_dbg(dev, "KICK Firmware to start send messages vqid %d\n", vqid);
+
+	/*
+	 * send irq to R5 firmware
+	 * Currently vqid is not used because we only got one.
+	 */
+	if (local->ipi_base)
+		reg_write(local->ipi_base, TRIG_OFFSET, local->ipi_dest_mask);
+}
+
+/* power off the remote processor */
+static int zynqmp_r5_rproc_stop(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+	struct rproc_mem_entry *mem, *nmem;
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+
+	dev_dbg(dev, "%s\n", __func__);
+
+	if (!eemi || !eemi->force_powerdown) {
+		pr_err("Failed to stop R5\n");
+		return -ENXIO;
+	}
+
+	disable_ipi(local);
+	eemi->force_powerdown(local->rpu_pnode_id,
+			      ZYNQMP_PM_REQUEST_ACK_BLOCKING);
+
+	return 0;
+}
+
+static void *zynqmp_r5_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
+{
+	struct rproc_mem_entry *mem;
+	void *va = NULL;
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+
+	list_for_each_entry(mem, &local->mems, node) {
+		int offset = da - mem->da;
+
+		/* try next carveout if da is too small */
+		if (offset < 0)
+			continue;
+
+		/* try next carveout if da is too large */
+		if (offset + len > mem->len)
+			continue;
+
+		va = mem->va + offset;
+
+		break;
+	}
+	return va;
+}
+
+static int zynqmp_r5_parse_fw(struct rproc *rproc, const struct firmware *fw)
+{
+	int ret;
+
+	ret = rproc_elf_load_rsc_table(rproc, fw);
+	if (ret == -EINVAL)
+		/* No resource table */
+		return 0;
+	else
+		return ret;
+}
+
+static struct rproc_ops zynqmp_r5_rproc_ops = {
+	.start		= zynqmp_r5_rproc_start,
+	.stop		= zynqmp_r5_rproc_stop,
+	.kick		= zynqmp_r5_rproc_kick,
+	.da_to_va       = zynqmp_r5_rproc_da_to_va,
+};
+
+/* Release R5 from reset and make it halted.
+ * In case the firmware uses TCM, in order to load firmware to TCM,
+ * will need to release R5 from reset and stay in halted state.
+ */
+static int zynqmp_r5_rproc_init(struct rproc *rproc)
+{
+	struct device *dev = rproc->dev.parent;
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+
+	dev_dbg(dev, "%s\n", __func__);
+	enable_ipi(local);
+	return 0;
+}
+
+static irqreturn_t r5_remoteproc_interrupt(int irq, void *dev_id)
+{
+	struct device *dev = dev_id;
+	struct platform_device *pdev = to_platform_device(dev);
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+	u32 ipi_reg;
+
+	/* Check if there is a kick from R5 */
+	ipi_reg = reg_read(local->ipi_base, ISR_OFFSET);
+	if (!(ipi_reg & local->ipi_dest_mask))
+		return IRQ_NONE;
+
+	dev_dbg(dev, "KICK Linux because of pending message(irq%d)\n", irq);
+	reg_write(local->ipi_base, ISR_OFFSET, local->ipi_dest_mask);
+	schedule_work(&local->workqueue);
+
+	return IRQ_HANDLED;
+}
+
+/* zynqmp_r5_get_tcm_memories() - get tcm memories
+ * @pdev: pointer to the platform device
+ * @pdata: pointer to the remoteproc private data
+ *
+ * Function to create remoteproc memory entries for TCM memories.
+ */
+static int zynqmp_r5_get_tcms(struct platform_device *pdev,
+			      struct zynqmp_r5_rproc_pdata *pdata)
+{
+	static const char * const mem_names[] = {"tcm_a", "tcm_b"};
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	int num_mems = 0;
+	int i, ret;
+	struct property *prop;
+	const __be32 *cur;
+	u32 val;
+	const struct zynqmp_eemi_ops *eemi = zynqmp_pm_get_eemi_ops();
+
+	/* Get TCM power node ids */
+	i = 0;
+	of_property_for_each_u32(np, "tcm-pnode-id", prop, cur, val)
+		pdata->tcm_pnode_id[i++] = val;
+
+	/* Request TCMs */
+	for (i = 0; i < MAX_TCM_PNODES; i++) {
+		if (pdata->tcm_pnode_id[i] != 0) {
+			ret = eemi->request_node(pdata->tcm_pnode_id[i],
+						 ZYNQMP_PM_CAPABILITY_ACCESS, 0,
+						 ZYNQMP_PM_REQUEST_ACK_BLOCKING
+						);
+			if (ret < 0) {
+				dev_err(dev, "failed to request TCM: %u\n",
+					pdata->tcm_pnode_id[i]);
+				return ret;
+			}
+			dev_dbg(dev, "request tcm pnode: %u\n",
+				pdata->tcm_pnode_id[i]);
+		} else {
+			break;
+		}
+	}
+	/* Create remoteproc memories entries for TCM memories */
+	num_mems = ARRAY_SIZE(mem_names);
+	for (i = 0; i < num_mems; i++) {
+		struct resource *res;
+		struct rproc_mem_entry *mem;
+		dma_addr_t dma;
+		resource_size_t size;
+
+		res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+						   mem_names[i]);
+		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
+				    GFP_KERNEL);
+		if (!mem)
+			return -ENOMEM;
+		/* Map it as normal memory */
+		size = resource_size(res);
+		mem->va = devm_ioremap_wc(dev, res->start, size);
+		mem->len = size;
+		dma = (dma_addr_t)res->start;
+		mem->dma = dma;
+		/* TCM memory:
+		 *   TCM_0: da 0 <-> global addr 0xFFE00000
+		 *   TCM_1: da 0 <-> global addr 0xFFE90000
+		 */
+		if ((dma & 0xFFF00000) == 0xFFE00000) {
+			if ((dma & 0xFFF80000) == 0xFFE80000)
+				mem->da -= 0x90000;
+			else
+				mem->da = (dma & 0x000FFFFF);
+		}
+		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
+			__func__, mem->va, mem->da, mem->dma);
+		list_add_tail(&mem->node, &pdata->mems);
+	}
+	return 0;
+}
+
+/* zynqmp_r5_get_reserved_mems() - get reserved memories
+ * @pdev: pointer to the platform device
+ * @pdata: pointer to the remoteproc private data
+ *
+ * Function to create remoteproc memory entries from memory-region
+ * property.
+ */
+static int zynqmp_r5_get_reserved_mems(struct platform_device *pdev,
+				       struct zynqmp_r5_rproc_pdata *pdata)
+{
+	struct device *dev = &pdev->dev;
+	struct device_node *np = dev->of_node;
+	int num_mems;
+	int i;
+
+	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 resource res;
+		resource_size_t size;
+		struct rproc_mem_entry *mem;
+		int ret;
+
+		node = of_parse_phandle(np, "memory-region", i);
+		ret = of_device_is_compatible(node, "rproc-prog-memory");
+		if (!ret) {
+			/* it is DMA memory. */
+			dev_info(dev, "%s, dma memory %d\n", __func__, i);
+			ret = of_reserved_mem_device_init_by_idx(dev,
+								 np, i);
+			if (ret) {
+				dev_err(dev, "unable to reserve DMA mem.\n");
+				return ret;
+			}
+			continue;
+		}
+		ret = of_address_to_resource(node, 0, &res);
+		if (ret) {
+			dev_err(dev, "unable to resolve memory region.\n");
+			return ret;
+		}
+		mem  = devm_kzalloc(dev, sizeof(struct rproc_mem_entry),
+				    GFP_KERNEL);
+		if (!mem)
+			return -ENOMEM;
+		/* Map it as normal memory */
+		size = resource_size(&res);
+		mem->va = devm_ioremap_wc(dev, res.start, size);
+		mem->len = size;
+		mem->dma = (dma_addr_t)res.start;
+		mem->da = (u32)res.start;
+		dev_dbg(dev, "%s: va = %p, da = 0x%x dma = 0x%llx\n",
+			__func__, mem->va, mem->da, mem->dma);
+		list_add_tail(&mem->node, &pdata->mems);
+	}
+	return 0;
+}
+
+static int zynqmp_r5_remoteproc_probe(struct platform_device *pdev)
+{
+	const unsigned char *prop;
+	struct resource *res;
+	int ret = 0;
+	struct zynqmp_r5_rproc_pdata *local;
+	struct rproc *rproc;
+
+	rproc = rproc_alloc(&pdev->dev, dev_name(&pdev->dev),
+			    &zynqmp_r5_rproc_ops, NULL,
+		sizeof(struct zynqmp_r5_rproc_pdata));
+	if (!rproc) {
+		dev_err(&pdev->dev, "rproc allocation failed\n");
+		return -ENOMEM;
+	}
+	local = rproc->priv;
+	local->rproc = rproc;
+
+	platform_set_drvdata(pdev, rproc);
+
+	/* Override parse_fw op to allow no resource table firmware */
+	rproc->ops->parse_fw = zynqmp_r5_parse_fw;
+
+	ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
+	if (ret) {
+		dev_err(&pdev->dev, "dma_set_coherent_mask: %d\n", ret);
+		goto rproc_fault;
+	}
+
+	/* Get the RPU power domain id */
+	ret = of_property_read_u32(pdev->dev.of_node, "rpu-pnode-id",
+				   &local->rpu_pnode_id);
+	if (ret) {
+		dev_err(&pdev->dev, "No RPU power node ID is specified.\n");
+		ret = -EINVAL;
+		goto rproc_fault;
+	}
+	dev_dbg(&pdev->dev, "RPU[%d] pnode_id = %d.\n",
+		local->rpu_id, local->rpu_pnode_id);
+
+	prop = of_get_property(pdev->dev.of_node, "core_conf", NULL);
+	if (!prop) {
+		dev_warn(&pdev->dev, "default core_conf used: lock-step\n");
+		prop = "lock-step";
+	}
+
+	dev_info(&pdev->dev, "RPU core_conf: %s\n", prop);
+	if (!strcmp(prop, "split0")) {
+		local->rpu_mode = PM_RPU_MODE_SPLIT;
+		local->rpu_id = 0;
+		local->ipi_dest_mask = RPU_0_IPI_MASK;
+	} else if (!strcmp(prop, "split1")) {
+		local->rpu_mode = PM_RPU_MODE_SPLIT;
+		local->rpu_id = 1;
+		local->ipi_dest_mask = RPU_1_IPI_MASK;
+	} else if (!strcmp(prop, "lock-step")) {
+		local->rpu_mode = PM_RPU_MODE_LOCKSTEP;
+		local->rpu_id = 0;
+		local->ipi_dest_mask = RPU_0_IPI_MASK;
+	} else {
+		dev_err(&pdev->dev, "Invalid core_conf mode provided - %s , %d\n",
+			prop, local->rpu_mode);
+		ret = -EINVAL;
+		goto rproc_fault;
+	}
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ipi");
+	if (res) {
+		local->ipi_base = devm_ioremap(&pdev->dev, res->start,
+					       resource_size(res));
+		if (IS_ERR(local->ipi_base)) {
+			pr_err("%s: Unable to map IPI\n", __func__);
+			ret = PTR_ERR(local->ipi_base);
+			goto rproc_fault;
+		}
+	} else {
+		dev_info(&pdev->dev, "IPI resource is not specified.\n");
+	}
+	dev_dbg(&pdev->dev, "got ipi base address\n");
+
+	INIT_LIST_HEAD(&local->mems);
+	/* Get TCM memories */
+	ret = zynqmp_r5_get_tcms(pdev, local);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get TCM memories.\n");
+		goto rproc_fault;
+	}
+	dev_dbg(&pdev->dev, "got TCM memories\n");
+	/* Get reserved memory regions for firmware */
+	ret = zynqmp_r5_get_reserved_mems(pdev, local);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "failed to get reserved memories.\n");
+		goto rproc_fault;
+	}
+	dev_dbg(&pdev->dev, "got reserved memories.\n");
+
+	/* Disable IPI before requesting IPI IRQ */
+	disable_ipi(local);
+	INIT_WORK(&local->workqueue, handle_event_notified);
+
+	/* IPI IRQ */
+	if (local->ipi_base) {
+		ret = platform_get_irq(pdev, 0);
+		if (ret < 0) {
+			dev_err(&pdev->dev, "unable to find IPI IRQ\n");
+			goto rproc_fault;
+		}
+		local->irq = ret;
+		ret = devm_request_irq(&pdev->dev, local->irq,
+				       r5_remoteproc_interrupt, IRQF_SHARED,
+				       dev_name(&pdev->dev), &pdev->dev);
+		if (ret) {
+			dev_err(&pdev->dev, "IRQ %d already allocated\n",
+				local->irq);
+			goto rproc_fault;
+		}
+		dev_dbg(&pdev->dev, "notification irq: %d\n", local->irq);
+	}
+
+	ret = zynqmp_r5_rproc_init(local->rproc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to init ZynqMP R5 rproc\n");
+		goto rproc_fault;
+	}
+
+	rproc->auto_boot = autoboot;
+
+	ret = rproc_add(local->rproc);
+	if (ret) {
+		dev_err(&pdev->dev, "rproc registration failed\n");
+		goto rproc_fault;
+	}
+
+	return ret;
+
+rproc_fault:
+	rproc_free(local->rproc);
+
+	return ret;
+}
+
+static int zynqmp_r5_remoteproc_remove(struct platform_device *pdev)
+{
+	struct rproc *rproc = platform_get_drvdata(pdev);
+	struct zynqmp_r5_rproc_pdata *local = rproc->priv;
+	struct rproc_mem_entry *mem;
+
+	dev_info(&pdev->dev, "%s\n", __func__);
+
+	rproc_del(rproc);
+
+	list_for_each_entry(mem, &local->mems, node) {
+		if (mem->priv)
+			gen_pool_free((struct gen_pool *)mem->priv,
+				      (unsigned long)mem->va, mem->len);
+	}
+
+	r5_release_tcm(local);
+	of_reserved_mem_device_release(&pdev->dev);
+	rproc_free(rproc);
+
+	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_param_named(autoboot,  autoboot, bool, 0444);
+MODULE_PARM_DESC(autoboot,
+		 "enable | disable autoboot. (default: true)");
+
+MODULE_AUTHOR("Jason Wu <[e-mail]>");
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ZynqMP R5 remote processor control driver");