Re: [PATCH v13 2/5] remoteproc/mediatek: add SCP support for mt8183
Thanks for the review. I'll address most of the comments in the next version.
On Mon, Jul 22, 2019 at 5:37 PM Alexandre Courbot wrote:
>
> Hi Pi-Hsun,
>
> On Tue, Jul 9, 2019 at 4:27 PM Pi-Hsun Shih wrote:
> > +static void *scp_da_to_va(struct rproc *rproc, u64 da, int len)
> > +{
> > + struct mtk_scp *scp = (struct mtk_scp *)rproc->priv;
> > + int offset;
> > +
> > + if (da < scp->sram_size) {
> > + offset = da;
> > + if (offset >= 0 && ((offset + len) < scp->sram_size))
> > + return (__force void *)(scp->sram_base + offset);
> > + } else if (da >= scp->sram_size &&
> > + da < (scp->sram_size + MAX_CODE_SIZE)) {
> > + offset = da;
>
> This line looks suspicious. Shouldn't it be
>
> offset = da - scp->sram_size?
>
Actually the whole "else if (...)" is not used. Would remove this in
next version.
> > +
> > +/*
> > + * Copy src to dst, where dst is in SCP SRAM region.
> > + * Since AP access of SCP SRAM don't support byte write, this always write
> > a
> > + * full word at a time, and may cause some extra bytes to be written at the
> > + * beginning & ending of dst.
> > + */
> > +void scp_memcpy_aligned(void *dst, const void *src, unsigned int len)
> > +{
> > + void *ptr;
> > + u32 val;
> > + unsigned int i = 0;
> > +
> > + if (!IS_ALIGNED((unsigned long)dst, 4)) {
> > + ptr = (void *)ALIGN_DOWN((unsigned long)dst, 4);
> > + i = 4 - (dst - ptr);
> > + val = readl_relaxed(ptr);
> > + memcpy((u8 *) + (4 - i), src, i);
> > + writel_relaxed(val, ptr);
> > + }
> > +
> > + while (i + 4 <= len) {
> > + val = *((u32 *)(src + i));
> > + writel_relaxed(val, dst + i);
>
> If dst is not aligned to 4, this is going to write to an address that
> is not a multiple of 4, even though it writes a long. Is this ok?
> Typically limitations in write size come with alignment limitations.
>
If dst is not aligned to 4, the first if (!IS_ALIGNED(...)) block
should make that the (dst + i) is a multiple of 4, so the write here
is aligned.
> > + i += 4;
> > + }
> > + if (i < len) {
> > + val = readl_relaxed(dst + i);
> > + memcpy(, src + i, len - i);
> > + writel_relaxed(val, dst + i);
> > + }
> > +}
> > +EXPORT_SYMBOL_GPL(scp_memcpy_aligned);
>
> IIUC this function's symbol does not need to be exported since it is
> only used in the current kernel module.
>
I've tried to remove this EXPORT line, but then there would be error
while compiling:
ERROR: "scp_memcpy_aligned" [drivers/remoteproc/mtk_scp.ko] undefined!
I think it's because the mtk_scp.c and mtk_scp_ipi.c both use the
scp_memcpy_aligned, but is compiled as separate .o files. So the
EXPORT_SYMBOL is needed.
Re: [PATCH v13 2/5] remoteproc/mediatek: add SCP support for mt8183
Hi Pi-Hsun,
On Tue, Jul 9, 2019 at 4:27 PM Pi-Hsun Shih wrote:
>
> From: Erin Lo
>
> Provide a basic driver to control Cortex M4 co-processor
>
> Signed-off-by: Erin Lo
> Signed-off-by: Nicolas Boichat
> Signed-off-by: Pi-Hsun Shih
> ---
> Changes from v12:
> - Initialize cache before firmware load, to avoid problem while loading
>large firmware.
> - Disable watchdog before stopping SCP, to avoid extra warning message.
> - Use strscpy instead of strncpy.
>
> Changes from v11:
> - No change.
>
> Changes from v10:
> - Add a clock reset before loading firmware.
>
> Changes from v9:
> - No change.
>
> Changes from v8:
> - Add a missing space.
>
> Changes from v7:
> - Moved the location of shared SCP buffer.
> - Fix clock enable/disable sequence.
> - Add more IPI ID that would be used.
>
> Changes from v6:
> - No change.
>
> Changes from v5:
> - Changed some space to tab.
>
> Changes from v4:
> - Rename most function from mtk_scp_* to scp_*.
> - Change the irq to threaded handler.
> - Load ELF file instead of plain binary file as firmware by default
>(Squashed patch 6 in v4 into this patch).
>
> Changes from v3:
> - Fix some issue found by checkpatch.
> - Make writes aligned in scp_ipi_send.
>
> Changes from v2:
> - Squash patch 3 from v2 (separate the ipi interface) into this patch.
> - Remove unused name argument from scp_ipi_register.
> - Add scp_ipi_unregister for proper cleanup.
> - Move IPI ids in sync with firmware.
> - Add mb() in proper place, and correctly clear the run->signaled.
>
> Changes from v1:
> - Extract functions and rename variables in mtk_scp.c.
> ---
> drivers/remoteproc/Kconfig| 9 +
> drivers/remoteproc/Makefile | 1 +
> drivers/remoteproc/mtk_common.h | 80
> drivers/remoteproc/mtk_scp.c | 523 ++
> drivers/remoteproc/mtk_scp_ipi.c | 162
> include/linux/platform_data/mtk_scp.h | 141 +++
> 6 files changed, 916 insertions(+)
> create mode 100644 drivers/remoteproc/mtk_common.h
> create mode 100644 drivers/remoteproc/mtk_scp.c
> create mode 100644 drivers/remoteproc/mtk_scp_ipi.c
> create mode 100644 include/linux/platform_data/mtk_scp.h
>
> diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
> index 28ed306982f7..ea71cad399f7 100644
> --- a/drivers/remoteproc/Kconfig
> +++ b/drivers/remoteproc/Kconfig
> @@ -23,6 +23,15 @@ config IMX_REMOTEPROC
>
> It's safe to say N here.
>
> +config MTK_SCP
> + tristate "Mediatek SCP support"
> + depends on ARCH_MEDIATEK
> + help
> + Say y here to support Mediatek's System Companion Processor (SCP)
> via
> + the remote processor framework.
> +
> + It's safe to say N here.
> +
> config OMAP_REMOTEPROC
> tristate "OMAP remoteproc support"
> depends on ARCH_OMAP4 || SOC_OMAP5
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 00f09e658cb3..e30a1b15fbac 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -10,6 +10,7 @@ remoteproc-y += remoteproc_sysfs.o
> remoteproc-y += remoteproc_virtio.o
> remoteproc-y += remoteproc_elf_loader.o
> obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
> +obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
> obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
> obj-$(CONFIG_WKUP_M3_RPROC)+= wkup_m3_rproc.o
> obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
> diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
> new file mode 100644
> index ..df918525da92
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2018 MediaTek Inc.
This should probably say 2019 now.
> + */
> +
> +#ifndef __RPROC_MTK_COMMON_H
> +#define __RPROC_MTK_COMMON_H
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#define MT8183_SW_RSTN 0x0
> +#define MT8183_SW_RSTN_BIT BIT(0)
> +#define MT8183_SCP_TO_HOST 0x1C
> +#define MT8183_SCP_IPC_INT_BIT BIT(0)
> +#define MT8183_SCP_WDT_INT_BIT BIT(8)
> +#define MT8183_HOST_TO_SCP 0x28
> +#define MT8183_HOST_IPC_INT_BITBIT(0)
> +#define MT8183_WDT_CFG 0x84
> +#define MT8183_SCP_CLK_SW_SEL 0x4000
> +#define MT8183_SCP_CLK_DIV_SEL 0x4024
> +#define MT8183_SCP_SRAM_PDN0x402C
> +#define MT8183_SCP_L1_SRAM_PD 0x4080
> +#define MT8183_SCP_TCM_TAIL_SRAM_PD0x4094
> +
> +#define SCP_FW_VER_LEN 32
> +
> +struct scp_run {
> + u32 signaled;
> + s8 fw_ver[SCP_FW_VER_LEN];
> + u32 dec_capability;
> + u32 enc_capability;
> + wait_queue_head_t wq;
> +};
> +
> +struct scp_ipi_desc {
> + scp_ipi_handler_t
[PATCH v13 2/5] remoteproc/mediatek: add SCP support for mt8183
From: Erin Lo
Provide a basic driver to control Cortex M4 co-processor
Signed-off-by: Erin Lo
Signed-off-by: Nicolas Boichat
Signed-off-by: Pi-Hsun Shih
---
Changes from v12:
- Initialize cache before firmware load, to avoid problem while loading
large firmware.
- Disable watchdog before stopping SCP, to avoid extra warning message.
- Use strscpy instead of strncpy.
Changes from v11:
- No change.
Changes from v10:
- Add a clock reset before loading firmware.
Changes from v9:
- No change.
Changes from v8:
- Add a missing space.
Changes from v7:
- Moved the location of shared SCP buffer.
- Fix clock enable/disable sequence.
- Add more IPI ID that would be used.
Changes from v6:
- No change.
Changes from v5:
- Changed some space to tab.
Changes from v4:
- Rename most function from mtk_scp_* to scp_*.
- Change the irq to threaded handler.
- Load ELF file instead of plain binary file as firmware by default
(Squashed patch 6 in v4 into this patch).
Changes from v3:
- Fix some issue found by checkpatch.
- Make writes aligned in scp_ipi_send.
Changes from v2:
- Squash patch 3 from v2 (separate the ipi interface) into this patch.
- Remove unused name argument from scp_ipi_register.
- Add scp_ipi_unregister for proper cleanup.
- Move IPI ids in sync with firmware.
- Add mb() in proper place, and correctly clear the run->signaled.
Changes from v1:
- Extract functions and rename variables in mtk_scp.c.
---
drivers/remoteproc/Kconfig| 9 +
drivers/remoteproc/Makefile | 1 +
drivers/remoteproc/mtk_common.h | 80
drivers/remoteproc/mtk_scp.c | 523 ++
drivers/remoteproc/mtk_scp_ipi.c | 162
include/linux/platform_data/mtk_scp.h | 141 +++
6 files changed, 916 insertions(+)
create mode 100644 drivers/remoteproc/mtk_common.h
create mode 100644 drivers/remoteproc/mtk_scp.c
create mode 100644 drivers/remoteproc/mtk_scp_ipi.c
create mode 100644 include/linux/platform_data/mtk_scp.h
diff --git a/drivers/remoteproc/Kconfig b/drivers/remoteproc/Kconfig
index 28ed306982f7..ea71cad399f7 100644
--- a/drivers/remoteproc/Kconfig
+++ b/drivers/remoteproc/Kconfig
@@ -23,6 +23,15 @@ config IMX_REMOTEPROC
It's safe to say N here.
+config MTK_SCP
+ tristate "Mediatek SCP support"
+ depends on ARCH_MEDIATEK
+ help
+ Say y here to support Mediatek's System Companion Processor (SCP) via
+ the remote processor framework.
+
+ It's safe to say N here.
+
config OMAP_REMOTEPROC
tristate "OMAP remoteproc support"
depends on ARCH_OMAP4 || SOC_OMAP5
diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index 00f09e658cb3..e30a1b15fbac 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -10,6 +10,7 @@ remoteproc-y += remoteproc_sysfs.o
remoteproc-y += remoteproc_virtio.o
remoteproc-y += remoteproc_elf_loader.o
obj-$(CONFIG_IMX_REMOTEPROC) += imx_rproc.o
+obj-$(CONFIG_MTK_SCP) += mtk_scp.o mtk_scp_ipi.o
obj-$(CONFIG_OMAP_REMOTEPROC) += omap_remoteproc.o
obj-$(CONFIG_WKUP_M3_RPROC)+= wkup_m3_rproc.o
obj-$(CONFIG_DA8XX_REMOTEPROC) += da8xx_remoteproc.o
diff --git a/drivers/remoteproc/mtk_common.h b/drivers/remoteproc/mtk_common.h
new file mode 100644
index ..df918525da92
--- /dev/null
+++ b/drivers/remoteproc/mtk_common.h
@@ -0,0 +1,80 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2018 MediaTek Inc.
+ */
+
+#ifndef __RPROC_MTK_COMMON_H
+#define __RPROC_MTK_COMMON_H
+
+#include
+#include
+#include
+#include
+
+#define MT8183_SW_RSTN 0x0
+#define MT8183_SW_RSTN_BIT BIT(0)
+#define MT8183_SCP_TO_HOST 0x1C
+#define MT8183_SCP_IPC_INT_BIT BIT(0)
+#define MT8183_SCP_WDT_INT_BIT BIT(8)
+#define MT8183_HOST_TO_SCP 0x28
+#define MT8183_HOST_IPC_INT_BITBIT(0)
+#define MT8183_WDT_CFG 0x84
+#define MT8183_SCP_CLK_SW_SEL 0x4000
+#define MT8183_SCP_CLK_DIV_SEL 0x4024
+#define MT8183_SCP_SRAM_PDN0x402C
+#define MT8183_SCP_L1_SRAM_PD 0x4080
+#define MT8183_SCP_TCM_TAIL_SRAM_PD0x4094
+
+#define SCP_FW_VER_LEN 32
+
+struct scp_run {
+ u32 signaled;
+ s8 fw_ver[SCP_FW_VER_LEN];
+ u32 dec_capability;
+ u32 enc_capability;
+ wait_queue_head_t wq;
+};
+
+struct scp_ipi_desc {
+ scp_ipi_handler_t handler;
+ void *priv;
+};
+
+struct mtk_scp {
+ struct device *dev;
+ struct rproc *rproc;
+ struct clk *clk;
+ void __iomem *reg_base;
+ void __iomem *sram_base;
+ size_t sram_size;
+
+ struct share_obj *recv_buf;
+ struct share_obj *send_buf;
+ struct scp_run run;
+ struct mutex lock; /* for protecting mtk_scp
