Re: [RFC 06/13] soc: mediatek: apu: Add apu core driver

2021-11-26 Thread Flora . Fu
On Sat, 2021-10-23 at 08:49 -0700, Randy Dunlap wrote:
> Hi,
> 
> On 10/23/21 4:14 AM, Flora Fu wrote:
> > diff --git a/drivers/soc/mediatek/Kconfig
> > b/drivers/soc/mediatek/Kconfig
> > index d9bac2710494..074b0cf24c44 100644
> > --- a/drivers/soc/mediatek/Kconfig
> > +++ b/drivers/soc/mediatek/Kconfig
> > @@ -24,6 +24,24 @@ config MTK_APU_PM
> >   APU power domain shall be enabled before accessing the
> >   internal sub modules.
> >   
> > +config MTK_APU
> > +   tristate "MediaTek APUSYS Support"
> > +   select REGMAP
> > +   select MTK_APU_PM
> > +   select MTK_SCP
> > +   help
> > + Say yes here to add support for the APU tinysys. The tinsys
> > is
> 
>  tinysys
> runs on
> 
> > + running on a micro processor in APU.
> 
> a microprocessor in the APU.
> 
> > + Its firmware is load and boot from Kernel side. Kernel and
> > tinysys use
> 
>  is loaded and booted
> 
> > + IPI to tx/rx messages.
> 
> to send/receive messages.
Ack. 

> > +
> > +config MTK_APU_DEBUG
> > +   tristate "MediaTek APUSYS debug functions"
> > +   depends on MTK_APU
> > +   help
> > + Say yes here to enalbe debug on APUSYS.
> 
> enable

Ack.
The tpyo will be fix in the next submission. 
Thanks for your review.

> 
> > + Disable it if you don't need them.
> 
> 



Re: [RFC 06/13] soc: mediatek: apu: Add apu core driver

2021-10-26 Thread AngeloGioacchino Del Regno

Il 23/10/21 13:14, Flora Fu ha scritto:

Add apu core driver.
The core driver will init the reset part of apu functions.

Signed-off-by: Flora Fu 
---
  drivers/soc/mediatek/Kconfig   | 18 +
  drivers/soc/mediatek/apusys/Makefile   |  3 +
  drivers/soc/mediatek/apusys/apu-core.c | 91 ++
  drivers/soc/mediatek/apusys/apu-core.h | 11 
  4 files changed, 123 insertions(+)
  create mode 100644 drivers/soc/mediatek/apusys/apu-core.c
  create mode 100644 drivers/soc/mediatek/apusys/apu-core.h



Hello Flora,

I don't think that this custom probe/init mechanism through apu-core.c
can ever be a thing: you should simply register a number of platform
drivers (likely modules) and let the kernel decide what to probe
first, what to probe last, as it's done for every kernel driver.

I understand that this may reduce probe deferrals, as it's a controlled
probe sequence, made specifically for apusys, but it's anyway something
that won't give you big gains (and if it was, then you should still let
the kernel decide and eventually optimize that mechanism somehow).


I want to note that, since this series is rather huge, I will probably
do more than one incremental review of it... and for how things look,
this will most probably be split to more than one series, to allow getting
reviews from specific subsystem maintainers, leading to better code quality
and robustness in the end.

Some more details are coming in reply of other patches in this series.

Thanks,
- Angelo


[RFC 06/13] soc: mediatek: apu: Add apu core driver

2021-10-23 Thread Flora Fu
Add apu core driver.
The core driver will init the reset part of apu functions.

Signed-off-by: Flora Fu 
---
 drivers/soc/mediatek/Kconfig   | 18 +
 drivers/soc/mediatek/apusys/Makefile   |  3 +
 drivers/soc/mediatek/apusys/apu-core.c | 91 ++
 drivers/soc/mediatek/apusys/apu-core.h | 11 
 4 files changed, 123 insertions(+)
 create mode 100644 drivers/soc/mediatek/apusys/apu-core.c
 create mode 100644 drivers/soc/mediatek/apusys/apu-core.h

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index d9bac2710494..074b0cf24c44 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -24,6 +24,24 @@ config MTK_APU_PM
  APU power domain shall be enabled before accessing the
  internal sub modules.
 
+config MTK_APU
+   tristate "MediaTek APUSYS Support"
+   select REGMAP
+   select MTK_APU_PM
+   select MTK_SCP
+   help
+ Say yes here to add support for the APU 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.
+
+config MTK_APU_DEBUG
+   tristate "MediaTek APUSYS debug functions"
+   depends on MTK_APU
+   help
+ Say yes here to enalbe debug on APUSYS.
+ Disable it if you don't need them.
+
 config MTK_CMDQ
tristate "MediaTek CMDQ Support"
depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/soc/mediatek/apusys/Makefile 
b/drivers/soc/mediatek/apusys/Makefile
index 8821c0f0b7b7..6b5249ec7064 100644
--- a/drivers/soc/mediatek/apusys/Makefile
+++ b/drivers/soc/mediatek/apusys/Makefile
@@ -1,2 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0
 obj-$(CONFIG_MTK_APU_PM) += mtk-apu-pm.o
+
+obj-$(CONFIG_MTK_APU) += apu.o
+apu-objs += apu-core.o
diff --git a/drivers/soc/mediatek/apusys/apu-core.c 
b/drivers/soc/mediatek/apusys/apu-core.c
new file mode 100644
index ..c1db2394307f
--- /dev/null
+++ b/drivers/soc/mediatek/apusys/apu-core.c
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ */
+
+#include 
+#include 
+#include 
+
+#include "apu-core.h"
+
+#define APUSYS_DEV_NAME "apusys"
+
+static struct apusys_core_info g_core_info;
+
+/*
+ * init function at other modulses
+ * call init function in order at apu.ko init stage
+ */
+static int (*apusys_init_func[])(struct apusys_core_info *) = {
+};
+
+/*
+ * exit function at other modulses
+ * call exit function in order at apu.ko exit stage
+ */
+static void (*apusys_exit_func[])(void) = {
+};
+
+static void create_dbg_root(void)
+{
+   g_core_info.dbg_root = debugfs_create_dir(APUSYS_DEV_NAME, NULL);
+
+   /* check dbg root create status */
+   if (IS_ERR_OR_NULL(g_core_info.dbg_root))
+   pr_info("failed to create debug dir.\n");
+}
+
+static void destroy_dbg_root(void)
+{
+   debugfs_remove_recursive(g_core_info.dbg_root);
+}
+
+static int __init apusys_init(void)
+{
+   int i, j, ret = 0;
+   int func_num = sizeof(apusys_init_func) / sizeof(int *);
+
+   /* init apusys_dev */
+   create_dbg_root();
+
+   /* call init func */
+   for (i = 0; i < func_num; i++) {
+   if (!apusys_init_func[i])
+   break;
+
+   ret = apusys_init_func[i](&g_core_info);
+   if (ret) {
+   pr_info("init function(%d) fail(%d)", i, ret);
+
+   /* exit device */
+   for (j = i - 1; j >= 0; j--)
+   apusys_exit_func[j]();
+
+   destroy_dbg_root();
+   break;
+   }
+   }
+
+   return 0;
+}
+
+static void __exit apusys_exit(void)
+{
+   int func_num = sizeof(apusys_exit_func) / sizeof(int *);
+   int i;
+
+   /* call release func */
+   for (i = 0; i < func_num; i++) {
+   if (!apusys_exit_func[i])
+   break;
+
+   apusys_exit_func[i]();
+   }
+
+   destroy_dbg_root();
+}
+
+module_init(apusys_init);
+module_exit(apusys_exit);
+MODULE_LICENSE("GPL");
diff --git a/drivers/soc/mediatek/apusys/apu-core.h 
b/drivers/soc/mediatek/apusys/apu-core.h
new file mode 100644
index ..bac730bbc7ea
--- /dev/null
+++ b/drivers/soc/mediatek/apusys/apu-core.h
@@ -0,0 +1,11 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2021 MediaTek Inc.
+ */
+#ifndef APU_CORE_H
+#define APU_CORE_H
+
+struct apusys_core_info {
+   struct dentry *dbg_root;
+};
+#endif
-- 
2.18.0



Re: [RFC 06/13] soc: mediatek: apu: Add apu core driver

2021-10-23 Thread Randy Dunlap

Hi,

On 10/23/21 4:14 AM, Flora Fu wrote:

diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig
index d9bac2710494..074b0cf24c44 100644
--- a/drivers/soc/mediatek/Kconfig
+++ b/drivers/soc/mediatek/Kconfig
@@ -24,6 +24,24 @@ config MTK_APU_PM
  APU power domain shall be enabled before accessing the
  internal sub modules.
  
+config MTK_APU

+   tristate "MediaTek APUSYS Support"
+   select REGMAP
+   select MTK_APU_PM
+   select MTK_SCP
+   help
+ Say yes here to add support for the APU tinysys. The tinsys is


   tinysys runs on


+ running on a micro processor in APU.


  a microprocessor in the APU.


+ Its firmware is load and boot from Kernel side. Kernel and tinysys use


   is loaded and booted


+ IPI to tx/rx messages.


  to send/receive messages.


+
+config MTK_APU_DEBUG
+   tristate "MediaTek APUSYS debug functions"
+   depends on MTK_APU
+   help
+ Say yes here to enalbe debug on APUSYS.


  enable


+ Disable it if you don't need them.



--
~Randy