Re: [RFC 08/13] soc: mediatek: apu: Add apusys rv driver
Il 26/11/21 10:25, Flora.Fu ha scritto:
On Tue, 2021-10-26 at 17:21 +0200, AngeloGioacchino Del Regno wrote:
Il 23/10/21 13:14, Flora Fu ha scritto:
Add driver for control APU tinysys
APU integrated subsystem having MD32RV33 (MD32) that runs tinysys
The tinsys is running on a micro processor in APU.
Its firmware is load and boot from Kernel side. Kernel and tinysys
use
IPI to tx/rx messages.
Signed-off-by: Flora Fu
---
drivers/soc/mediatek/apusys/Makefile| 6 +
drivers/soc/mediatek/apusys/apu-config.h| 100 +++
drivers/soc/mediatek/apusys/apu-core.c | 2 +
drivers/soc/mediatek/apusys/apu-core.h | 2 +
drivers/soc/mediatek/apusys/apu-ipi.c | 486
I'm not sure of that, but your apu-ipi.c may be more suited to be in
drivers/remoteproc instead.
drivers/soc/mediatek/apusys/apu-mbox.c | 83 ++
apu-mbox.c should go to drivers/mailbox/ and you should register it
with
the mailbox API as a mailbox controller instead of what you're
currently
doing...
From what I see, you have functions in there that can be indeed
mapped
to struct mbox_chan_ops .send_data and .peek_data... also your
function
apu_mbox_wait_inbox seems to be waiting on an interrupt, and such irq
is
apparently your "mbox0_irq" (as you named it in the dt).
In that case, you can also manage that in your drivers/mailbox/
driver.
+
drivers/soc/mediatek/apusys/apu-mbox.h | 27 +
drivers/soc/mediatek/apusys/apu-rproc.c | 806
The apu-rproc.c driver seems to be a good candidate to be moved away
from
drivers/soc/mediatek/apusys/ - as this is indeed a remoteproc driver.
Having it as drivers/remoteproc/mtk_apu.c seems to be a good option.
For mbox/ipi/apu-rproc, we will check the kernel framework and make
more fix on it.
drivers/soc/mediatek/apusys/apu-sw-logger.c | 429 +++
This one definitely belongs here in drivers/soc/mediatek, and it's a
consumer
of the mailbox driver.
drivers/soc/mediatek/apusys/apu.h | 256 +++
drivers/soc/mediatek/apusys/mt81xx-plat.c | 320
If we end up keeping to be in need to have a separate mt81xx-plat.c
file,
then I believe this should have another name, so that it becomes one
that
aggregates all of the very-platform-specific functions in one,
instead of
having one file for each platform.
Though, it may also be possible that this file will disappear
entirely:
since most of the things here will be moved around, it may become
mostly
empty... but it's probably too soon to judge.
The mt81xx-platform.c is prepared for the similiar apu desing platforms
but some of them will have differnt operations in the functions. I will
just keep it and we can reivew it again in the feature versions.
11 files changed, 2517 insertions(+)
create mode 100644 drivers/soc/mediatek/apusys/apu-config.h
create mode 100644 drivers/soc/mediatek/apusys/apu-ipi.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.h
create mode 100644 drivers/soc/mediatek/apusys/apu-rproc.c
create mode 100644 drivers/soc/mediatek/apusys/apu-sw-logger.c
create mode 100644 drivers/soc/mediatek/apusys/apu.h
create mode 100644 drivers/soc/mediatek/apusys/mt81xx-plat.c
snip...
diff --git a/drivers/soc/mediatek/apusys/apu-ipi.c
b/drivers/soc/mediatek/apusys/apu-ipi.c
new file mode 100644
index ..547e034b3620
--- /dev/null
+++ b/drivers/soc/mediatek/apusys/apu-ipi.c
snip...
+int apu_ipi_init(struct platform_device *pdev, struct mtk_apu
*apu)
+{
+ struct device *dev = apu->dev;
+ int i, ret;
+
+ tx_serial_no = 0;
+ rx_serial_no = 0;
+
+ mutex_init(&apu->send_lock);
+ spin_lock_init(&apu->usage_cnt_lock);
+ for (i = 0; i < APU_IPI_MAX; i++) {
+ mutex_init(&apu->ipi_desc[i].lock);
+ lockdep_set_class_and_name(&apu->ipi_desc[i].lock,
+ &ipi_lock_key[i],
+ apu->platdata-
ipi_attrs[i].name);
+ }
+
+ init_waitqueue_head(&apu->run.wq);
+ init_waitqueue_head(&apu->ack_wq);
+
+ /* APU initialization IPI register */
+ ret = apu_ipi_register(apu, APU_IPI_INIT, apu_init_ipi_handler,
apu);
+ if (ret) {
+ dev_err(dev, "failed to register ipi for init,
ret=%d\n",
+ ret);
+ return ret;
+ }
+
+ /* add rpmsg subdev */
+ apu_add_rpmsg_subdev(apu);
+
+ /* register mailbox IRQ */
+ apu->mbox0_irq_number = platform_get_irq_byname(pdev,
"mbox0_irq");
+ dev_info(&pdev->dev, "%s: mbox0_irq = %d\n", __func__,
+apu->mbox0_irq_number);
+
+ ret = devm_request_threaded_irq(&pdev->dev, apu-
mbox0_irq_number,
+ NULL, apu_ipi_handler,
IRQF_ONESHOT,
+ "apu_ipi", apu);
Re: [RFC 08/13] soc: mediatek: apu: Add apusys rv driver
On Tue, 2021-10-26 at 17:21 +0200, AngeloGioacchino Del Regno wrote:
> Il 23/10/21 13:14, Flora Fu ha scritto:
> > Add driver for control APU tinysys
> >
> > APU integrated subsystem having MD32RV33 (MD32) that runs tinysys
> > The tinsys is running on a micro processor in APU.
> > Its firmware is load and boot from Kernel side. Kernel and tinysys
> > use
> > IPI to tx/rx messages.
> >
> > Signed-off-by: Flora Fu
> > ---
> > drivers/soc/mediatek/apusys/Makefile| 6 +
> > drivers/soc/mediatek/apusys/apu-config.h| 100 +++
> > drivers/soc/mediatek/apusys/apu-core.c | 2 +
> > drivers/soc/mediatek/apusys/apu-core.h | 2 +
> > drivers/soc/mediatek/apusys/apu-ipi.c | 486
>
> I'm not sure of that, but your apu-ipi.c may be more suited to be in
> drivers/remoteproc instead.
>
> > drivers/soc/mediatek/apusys/apu-mbox.c | 83 ++
>
> apu-mbox.c should go to drivers/mailbox/ and you should register it
> with
> the mailbox API as a mailbox controller instead of what you're
> currently
> doing...
>
> From what I see, you have functions in there that can be indeed
> mapped
> to struct mbox_chan_ops .send_data and .peek_data... also your
> function
> apu_mbox_wait_inbox seems to be waiting on an interrupt, and such irq
> is
> apparently your "mbox0_irq" (as you named it in the dt).
> In that case, you can also manage that in your drivers/mailbox/
> driver.
> +
> > drivers/soc/mediatek/apusys/apu-mbox.h | 27 +
> > drivers/soc/mediatek/apusys/apu-rproc.c | 806
> >
>
> The apu-rproc.c driver seems to be a good candidate to be moved away
> from
>
> drivers/soc/mediatek/apusys/ - as this is indeed a remoteproc driver.
>
> Having it as drivers/remoteproc/mtk_apu.c seems to be a good option.
>
For mbox/ipi/apu-rproc, we will check the kernel framework and make
more fix on it.
>
> > drivers/soc/mediatek/apusys/apu-sw-logger.c | 429 +++
>
> This one definitely belongs here in drivers/soc/mediatek, and it's a
> consumer
> of the mailbox driver.
>
> > drivers/soc/mediatek/apusys/apu.h | 256 +++
> > drivers/soc/mediatek/apusys/mt81xx-plat.c | 320
>
> If we end up keeping to be in need to have a separate mt81xx-plat.c
> file,
> then I believe this should have another name, so that it becomes one
> that
> aggregates all of the very-platform-specific functions in one,
> instead of
> having one file for each platform.
>
> Though, it may also be possible that this file will disappear
> entirely:
> since most of the things here will be moved around, it may become
> mostly
> empty... but it's probably too soon to judge.
The mt81xx-platform.c is prepared for the similiar apu desing platforms
but some of them will have differnt operations in the functions. I will
just keep it and we can reivew it again in the feature versions.
>
> > 11 files changed, 2517 insertions(+)
> > create mode 100644 drivers/soc/mediatek/apusys/apu-config.h
> > create mode 100644 drivers/soc/mediatek/apusys/apu-ipi.c
> > create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.c
> > create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.h
> > create mode 100644 drivers/soc/mediatek/apusys/apu-rproc.c
> > create mode 100644 drivers/soc/mediatek/apusys/apu-sw-logger.c
> > create mode 100644 drivers/soc/mediatek/apusys/apu.h
> > create mode 100644 drivers/soc/mediatek/apusys/mt81xx-plat.c
> >
>
> snip...
>
> > diff --git a/drivers/soc/mediatek/apusys/apu-ipi.c
> > b/drivers/soc/mediatek/apusys/apu-ipi.c
> > new file mode 100644
> > index ..547e034b3620
> > --- /dev/null
> > +++ b/drivers/soc/mediatek/apusys/apu-ipi.c
>
> snip...
>
> > +int apu_ipi_init(struct platform_device *pdev, struct mtk_apu
> > *apu)
> > +{
> > + struct device *dev = apu->dev;
> > + int i, ret;
> > +
> > + tx_serial_no = 0;
> > + rx_serial_no = 0;
> > +
> > + mutex_init(&apu->send_lock);
> > + spin_lock_init(&apu->usage_cnt_lock);
> > + for (i = 0; i < APU_IPI_MAX; i++) {
> > + mutex_init(&apu->ipi_desc[i].lock);
> > + lockdep_set_class_and_name(&apu->ipi_desc[i].lock,
> > + &ipi_lock_key[i],
> > + apu->platdata-
> > >ipi_attrs[i].name);
> > + }
> > +
> > + init_waitqueue_head(&apu->run.wq);
> > + init_waitqueue_head(&apu->ack_wq);
> > +
> > + /* APU initialization IPI register */
> > + ret = apu_ipi_register(apu, APU_IPI_INIT, apu_init_ipi_handler,
> > apu);
> > + if (ret) {
> > + dev_err(dev, "failed to register ipi for init,
> > ret=%d\n",
> > + ret);
> > + return ret;
> > + }
> > +
> > + /* add rpmsg subdev */
> > + apu_add_rpmsg_subdev(apu);
> > +
> > + /* register mailbox IRQ */
> > + apu->mbox0_irq_number = platform_get_irq_byname(pdev,
> > "mbox0_irq");
> > + dev_info(&pdev->dev, "%s: mbox0_irq = %d\n", __func__,
> > +apu->
Re: [RFC 08/13] soc: mediatek: apu: Add apusys rv driver
Il 23/10/21 13:14, Flora Fu ha scritto:
Add driver for control APU tinysys
APU integrated subsystem having MD32RV33 (MD32) that runs tinysys
The tinsys is running on a micro processor in APU.
Its firmware is load and boot from Kernel side. Kernel and tinysys use
IPI to tx/rx messages.
Signed-off-by: Flora Fu
---
drivers/soc/mediatek/apusys/Makefile| 6 +
drivers/soc/mediatek/apusys/apu-config.h| 100 +++
drivers/soc/mediatek/apusys/apu-core.c | 2 +
drivers/soc/mediatek/apusys/apu-core.h | 2 +
drivers/soc/mediatek/apusys/apu-ipi.c | 486
I'm not sure of that, but your apu-ipi.c may be more suited to be in
drivers/remoteproc instead.
drivers/soc/mediatek/apusys/apu-mbox.c | 83 ++
apu-mbox.c should go to drivers/mailbox/ and you should register it with
the mailbox API as a mailbox controller instead of what you're currently
doing...
From what I see, you have functions in there that can be indeed mapped
to struct mbox_chan_ops .send_data and .peek_data... also your function
apu_mbox_wait_inbox seems to be waiting on an interrupt, and such irq is
apparently your "mbox0_irq" (as you named it in the dt).
In that case, you can also manage that in your drivers/mailbox/ driver.
+
drivers/soc/mediatek/apusys/apu-mbox.h | 27 +
drivers/soc/mediatek/apusys/apu-rproc.c | 806
The apu-rproc.c driver seems to be a good candidate to be moved away from
drivers/soc/mediatek/apusys/ - as this is indeed a remoteproc driver.
Having it as drivers/remoteproc/mtk_apu.c seems to be a good option.
drivers/soc/mediatek/apusys/apu-sw-logger.c | 429 +++
This one definitely belongs here in drivers/soc/mediatek, and it's a consumer
of the mailbox driver.
drivers/soc/mediatek/apusys/apu.h | 256 +++
drivers/soc/mediatek/apusys/mt81xx-plat.c | 320
If we end up keeping to be in need to have a separate mt81xx-plat.c file,
then I believe this should have another name, so that it becomes one that
aggregates all of the very-platform-specific functions in one, instead of
having one file for each platform.
Though, it may also be possible that this file will disappear entirely:
since most of the things here will be moved around, it may become mostly
empty... but it's probably too soon to judge.
11 files changed, 2517 insertions(+)
create mode 100644 drivers/soc/mediatek/apusys/apu-config.h
create mode 100644 drivers/soc/mediatek/apusys/apu-ipi.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.h
create mode 100644 drivers/soc/mediatek/apusys/apu-rproc.c
create mode 100644 drivers/soc/mediatek/apusys/apu-sw-logger.c
create mode 100644 drivers/soc/mediatek/apusys/apu.h
create mode 100644 drivers/soc/mediatek/apusys/mt81xx-plat.c
snip...
diff --git a/drivers/soc/mediatek/apusys/apu-ipi.c
b/drivers/soc/mediatek/apusys/apu-ipi.c
new file mode 100644
index ..547e034b3620
--- /dev/null
+++ b/drivers/soc/mediatek/apusys/apu-ipi.c
snip...
+int apu_ipi_init(struct platform_device *pdev, struct mtk_apu *apu)
+{
+ struct device *dev = apu->dev;
+ int i, ret;
+
+ tx_serial_no = 0;
+ rx_serial_no = 0;
+
+ mutex_init(&apu->send_lock);
+ spin_lock_init(&apu->usage_cnt_lock);
+ for (i = 0; i < APU_IPI_MAX; i++) {
+ mutex_init(&apu->ipi_desc[i].lock);
+ lockdep_set_class_and_name(&apu->ipi_desc[i].lock,
+ &ipi_lock_key[i],
+ apu->platdata->ipi_attrs[i].name);
+ }
+
+ init_waitqueue_head(&apu->run.wq);
+ init_waitqueue_head(&apu->ack_wq);
+
+ /* APU initialization IPI register */
+ ret = apu_ipi_register(apu, APU_IPI_INIT, apu_init_ipi_handler, apu);
+ if (ret) {
+ dev_err(dev, "failed to register ipi for init, ret=%d\n",
+ ret);
+ return ret;
+ }
+
+ /* add rpmsg subdev */
+ apu_add_rpmsg_subdev(apu);
+
+ /* register mailbox IRQ */
+ apu->mbox0_irq_number = platform_get_irq_byname(pdev, "mbox0_irq");
+ dev_info(&pdev->dev, "%s: mbox0_irq = %d\n", __func__,
+apu->mbox0_irq_number);
+
+ ret = devm_request_threaded_irq(&pdev->dev, apu->mbox0_irq_number,
+ NULL, apu_ipi_handler, IRQF_ONESHOT,
+ "apu_ipi", apu);
This is the mailbox interrupt... but it's handled in this driver instead of
being handler in the mailbox driver... it's a bit confusing.
Is this interrupt supposed to fire as a mailbox doorbell or..?
In that case, you should request it in the mailbox driver and register an
interrupt controller (still, in the mailbox driver) so that you can export
a sw interrupt to this one.
Or, maybe you can use notifiers to catch the mailbox mess
[RFC 08/13] soc: mediatek: apu: Add apusys rv driver
Add driver for control APU tinysys
APU integrated subsystem having MD32RV33 (MD32) that runs tinysys
The tinsys is running on a micro processor in APU.
Its firmware is load and boot from Kernel side. Kernel and tinysys use
IPI to tx/rx messages.
Signed-off-by: Flora Fu
---
drivers/soc/mediatek/apusys/Makefile| 6 +
drivers/soc/mediatek/apusys/apu-config.h| 100 +++
drivers/soc/mediatek/apusys/apu-core.c | 2 +
drivers/soc/mediatek/apusys/apu-core.h | 2 +
drivers/soc/mediatek/apusys/apu-ipi.c | 486
drivers/soc/mediatek/apusys/apu-mbox.c | 83 ++
drivers/soc/mediatek/apusys/apu-mbox.h | 27 +
drivers/soc/mediatek/apusys/apu-rproc.c | 806
drivers/soc/mediatek/apusys/apu-sw-logger.c | 429 +++
drivers/soc/mediatek/apusys/apu.h | 256 +++
drivers/soc/mediatek/apusys/mt81xx-plat.c | 320
11 files changed, 2517 insertions(+)
create mode 100644 drivers/soc/mediatek/apusys/apu-config.h
create mode 100644 drivers/soc/mediatek/apusys/apu-ipi.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.c
create mode 100644 drivers/soc/mediatek/apusys/apu-mbox.h
create mode 100644 drivers/soc/mediatek/apusys/apu-rproc.c
create mode 100644 drivers/soc/mediatek/apusys/apu-sw-logger.c
create mode 100644 drivers/soc/mediatek/apusys/apu.h
create mode 100644 drivers/soc/mediatek/apusys/mt81xx-plat.c
diff --git a/drivers/soc/mediatek/apusys/Makefile
b/drivers/soc/mediatek/apusys/Makefile
index 8c61ed8e2c04..490de0ee4727 100644
--- a/drivers/soc/mediatek/apusys/Makefile
+++ b/drivers/soc/mediatek/apusys/Makefile
@@ -7,3 +7,9 @@ apu-objs += apu-core.o
apu-objs += apu-pwr.o
apu-objs += apu-pwr-ipi.o
apu-$(CONFIG_MTK_APU_DEBUG) += apu-pwr-dbg.o
+
+apu-objs += apu-rproc.o
+apu-objs += apu-ipi.o
+apu-objs += apu-mbox.o
+apu-objs += mt81xx-plat.o
+apu-$(CONFIG_DEBUG_FS) += apu-sw-logger.o
diff --git a/drivers/soc/mediatek/apusys/apu-config.h
b/drivers/soc/mediatek/apusys/apu-config.h
new file mode 100644
index ..fee3b0334502
--- /dev/null
+++ b/drivers/soc/mediatek/apusys/apu-config.h
@@ -0,0 +1,100 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ */
+
+#ifndef APU_CONFIG_H
+#define APU_CONFIG_H
+
+struct apu_ipi_config {
+ u64 in_buf_da;
+ u64 out_buf_da;
+} __packed;
+
+struct vpu_init_info {
+ u32 vpu_num;
+ u32 cfg_addr;
+ u32 cfg_size;
+ u32 algo_info_ptr[3 * 2];
+ u32 rst_vec[3];
+ u32 dmem_addr[3];
+ u32 imem_addr[3];
+ u32 iram_addr[3];
+ u32 cmd_addr[3];
+ u32 log_addr[3];
+ u32 log_size[3];
+} __packed;
+
+struct apusys_chip_data {
+ u32 s_code;
+ u32 b_code;
+ u32 r_code;
+ u32 a_code;
+} __packed;
+
+struct logger_init_info {
+ u32 iova;
+} __packed;
+
+struct reviser_init_info {
+ u32 boundary;
+ u32 dram[32];
+} __packed;
+
+enum user_config {
+ APU_IPI_CONFIG = 0x0,
+ VPU_INIT_INFO,
+ APUSYS_CHIP_DATA,
+ LOGGER_INIT_INFO,
+ REVISER_INIT_INFO,
+ USER_CONFIG_MAX
+};
+
+struct config_v1_entry_table {
+ u32 user_entry[USER_CONFIG_MAX];
+} __packed;
+
+struct config_v1 {
+ /* header begin */
+ u32 header_magic;
+ u32 header_rev;
+ u32 entry_offset;
+ u32 config_size;
+ /* header end */
+ /* do not add new member before this line */
+
+ /* system related config begin */
+ u32 ramdump_offset;
+ u32 ramdump_type;
+ u64 time_offset;
+ /* system related config end */
+
+ /* entry table */
+ u8 entry_tbl[sizeof(struct config_v1_entry_table)];
+
+ /* user data payload begin */
+ u8 user0_data[sizeof(struct apu_ipi_config)];
+ u8 user1_data[sizeof(struct vpu_init_info)];
+ u8 user2_data[sizeof(struct apusys_chip_data)];
+ u8 user3_data[sizeof(struct logger_init_info)];
+ u8 user4_data[sizeof(struct reviser_init_info)];
+ /* user data payload end */
+} __packed;
+
+static inline void *get_apu_config_user_ptr(struct config_v1 *conf,
+ enum user_config user_id)
+{
+ struct config_v1_entry_table *entry_tbl;
+
+ if (!conf)
+ return NULL;
+
+ if (user_id >= USER_CONFIG_MAX)
+ return NULL;
+
+ entry_tbl = (struct config_v1_entry_table *)
+ ((void *)conf + conf->entry_offset);
+
+ return (void *)conf + entry_tbl->user_entry[user_id];
+}
+#endif /* APU_CONFIG_H */
diff --git a/drivers/soc/mediatek/apusys/apu-core.c
b/drivers/soc/mediatek/apusys/apu-core.c
index 069e18af7a5b..80652b1d056e 100644
--- a/drivers/soc/mediatek/apusys/apu-core.c
+++ b/drivers/soc/mediatek/apusys/apu-core.c
@@ -19,6 +19,7 @@ static struct apusys_core_info g_core_info;
*/
static int (*apusys_init_func[])(struct apusys_core_info *) = {
apu_power_drv_init,
+ apu_rproc_init,
