Re: [RFC v2 3/6] remoteproc: move IPI interface into separate file.
On Sat, Jan 19, 2019 at 5:04 AM Nicolas Boichat wrote:
>
> Hi,
>
> On Mon, Jan 7, 2019 at 9:26 PM Pi-Hsun Shih wrote:
> >
> > Move the IPI interface into a separate file mtk_scp_ipi.c, so the things
> > that use the interface only can depend on the module only.
> >
> > Signed-off-by: Pi-Hsun Shih
> > ---
> > Changes from v1:
> > - Resolved conflict because of change in Patch 2.
> > ---
> > drivers/remoteproc/Makefile | 2 +-
> > drivers/remoteproc/mtk_common.h | 73 +++
> > drivers/remoteproc/mtk_scp.c | 154 +--
>
> The fact that you remove a bunch of stuff from mtk_scp.c makes me
> think that we should probably reorganize the patches. The series here
> shows the history of how we developed this, but for upstreaming
> purpose, we want independent, small-ish patches.
>
> Maybe 2/6 should contain just basic mtk_scp code with no IPI. 3/6 IPI
> only, and _maybe_ 4/6 rpmsg should be folded into 3/6?
>
> Thanks,
>
I think it's hard to have mtk_scp code without IPI, since the
initialization itself depends on IPI.
I'll fold 3/6 into 2/6 in v3.
> > drivers/remoteproc/mtk_scp_ipi.c | 108 ++
> > 4 files changed, 183 insertions(+), 154 deletions(-)
> > create mode 100644 drivers/remoteproc/mtk_common.h
> > create mode 100644 drivers/remoteproc/mtk_scp_ipi.c
> >
> > diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> > index 98e3498dbbe0e2..16b3e5e7a81c8e 100644
> > --- a/drivers/remoteproc/Makefile
> > +++ b/drivers/remoteproc/Makefile
> > @@ -10,7 +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
> > +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 00..e97287a4eb25cc
> > --- /dev/null
> > +++ b/drivers/remoteproc/mtk_common.h
> > @@ -0,0 +1,73 @@
> > +// 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_SCP_SRAM_PDN0x402C
> > +
> > +#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;
> > + const char *name;
> > + 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 scp_mutex; /* for protecting mtk_scp data structure */
> > + struct scp_ipi_desc ipi_desc[SCP_IPI_MAX];
> > + bool ipi_id_ack[SCP_IPI_MAX];
> > + wait_queue_head_t ack_wq;
> > +
> > + void __iomem *cpu_addr;
> > + phys_addr_t phys_addr;
> > + size_t dram_size;
> > +};
> > +
> > +/**
> > + * struct share_obj - SRAM buffer shared with
> > + * AP and SCP
> > + *
> > + * @id:IPI id
> > + * @len: share buffer length
> > + * @share_buf: share buffer data
> > + */
> > +struct share_obj {
> > + s32 id;
> > + u32 len;
> > + u8 share_buf[288];
> > +};
> > +
> > +#endif
> > diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> > index 6e2e17a227d018..3e84c696523436 100644
> > --- a/drivers/remoteproc/mtk_scp.c
> > +++ b/drivers/remoteproc/mtk_scp.c
> > @@ -13,163 +13,11 @@
> > #include
> > #include
> >
> > +#include "mtk_common.h"
> > #include "remoteproc_internal.h"
> >
> > -#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
Re: [RFC v2 3/6] remoteproc: move IPI interface into separate file.
Hi,
On Mon, Jan 7, 2019 at 9:26 PM Pi-Hsun Shih wrote:
>
> Move the IPI interface into a separate file mtk_scp_ipi.c, so the things
> that use the interface only can depend on the module only.
>
> Signed-off-by: Pi-Hsun Shih
> ---
> Changes from v1:
> - Resolved conflict because of change in Patch 2.
> ---
> drivers/remoteproc/Makefile | 2 +-
> drivers/remoteproc/mtk_common.h | 73 +++
> drivers/remoteproc/mtk_scp.c | 154 +--
The fact that you remove a bunch of stuff from mtk_scp.c makes me
think that we should probably reorganize the patches. The series here
shows the history of how we developed this, but for upstreaming
purpose, we want independent, small-ish patches.
Maybe 2/6 should contain just basic mtk_scp code with no IPI. 3/6 IPI
only, and _maybe_ 4/6 rpmsg should be folded into 3/6?
Thanks,
> drivers/remoteproc/mtk_scp_ipi.c | 108 ++
> 4 files changed, 183 insertions(+), 154 deletions(-)
> create mode 100644 drivers/remoteproc/mtk_common.h
> create mode 100644 drivers/remoteproc/mtk_scp_ipi.c
>
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index 98e3498dbbe0e2..16b3e5e7a81c8e 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -10,7 +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
> +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 00..e97287a4eb25cc
> --- /dev/null
> +++ b/drivers/remoteproc/mtk_common.h
> @@ -0,0 +1,73 @@
> +// 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_SCP_SRAM_PDN0x402C
> +
> +#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;
> + const char *name;
> + 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 scp_mutex; /* for protecting mtk_scp data structure */
> + struct scp_ipi_desc ipi_desc[SCP_IPI_MAX];
> + bool ipi_id_ack[SCP_IPI_MAX];
> + wait_queue_head_t ack_wq;
> +
> + void __iomem *cpu_addr;
> + phys_addr_t phys_addr;
> + size_t dram_size;
> +};
> +
> +/**
> + * struct share_obj - SRAM buffer shared with
> + * AP and SCP
> + *
> + * @id:IPI id
> + * @len: share buffer length
> + * @share_buf: share buffer data
> + */
> +struct share_obj {
> + s32 id;
> + u32 len;
> + u8 share_buf[288];
> +};
> +
> +#endif
> diff --git a/drivers/remoteproc/mtk_scp.c b/drivers/remoteproc/mtk_scp.c
> index 6e2e17a227d018..3e84c696523436 100644
> --- a/drivers/remoteproc/mtk_scp.c
> +++ b/drivers/remoteproc/mtk_scp.c
> @@ -13,163 +13,11 @@
> #include
> #include
>
> +#include "mtk_common.h"
> #include "remoteproc_internal.h"
>
> -#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_SCP_SRAM_PDN0x402C
> -
> -#define SCP_FW_VER_LEN 32
> -
> #define MAX_CODE_SIZE 0x50
>
> -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 {
>
