Re: [PATCH v2 03/12] ASoC: mediatek: mt8188: support audsys clock
From: AngeloGioacchino Del Regno
Date: Thu Dec 01 2022 - 04:42:17 EST
Il 01/12/22 09:43, Trevor Wu (吳文良) ha scritto:
On Wed, 2022-10-26 at 10:18 +0200, AngeloGioacchino Del Regno wrote:
Il 26/10/22 06:10, Trevor Wu (吳文良) ha scritto:
On Tue, 2022-10-25 at 12:18 +0200, AngeloGioacchino Del Regno
wrote:
Il 21/10/22 11:58, Trevor Wu (吳文良) ha scritto:
On Fri, 2022-10-21 at 10:41 +0200, AngeloGioacchino Del Regno
wrote:
Il 21/10/22 10:27, Trevor Wu ha scritto:
Add mt8188 audio cg clock control. Audio clock gates are
registered
to CCF
for reference count and clock parent management.
Signed-off-by: Trevor Wu <trevor.wu@xxxxxxxxxxxx>
---
sound/soc/mediatek/mt8188/mt8188-audsys-clk.c | 206
++++++++++++++++++
sound/soc/mediatek/mt8188/mt8188-audsys-clk.h | 15 ++
.../soc/mediatek/mt8188/mt8188-audsys-clkid.h | 83
+++++++
3 files changed, 304 insertions(+)
create mode 100644 sound/soc/mediatek/mt8188/mt8188-
audsys-
clk.c
create mode 100644 sound/soc/mediatek/mt8188/mt8188-
audsys-
clk.h
create mode 100644 sound/soc/mediatek/mt8188/mt8188-
audsys-
clkid.h
diff --git a/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
new file mode 100644
index 000000000000..1f294231d4c2
--- /dev/null
+++ b/sound/soc/mediatek/mt8188/mt8188-audsys-clk.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * mt8188-audsys-clk.c -- MediaTek 8188 audsys clock
control
+ *
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Chun-Chia Chiu <chun-chia.chiu@xxxxxxxxxxxx>
+ */
+
+#include <linux/clk.h>
+#include <linux/clk-provider.h>
+#include <linux/clkdev.h>
+#include "mt8188-afe-common.h"
+#include "mt8188-audsys-clk.h"
+#include "mt8188-audsys-clkid.h"
+#include "mt8188-reg.h"
+
+struct afe_gate {
+ int id;
+ const char *name;
+ const char *parent_name;
+ int reg;
+ u8 bit;
+ const struct clk_ops *ops;
+ unsigned long flags;
+ u8 cg_flags;
+};
+
+#define GATE_AFE_FLAGS(_id, _name, _parent, _reg, _bit,
_flags,
_cgflags) {\
+ .id = _id,
\
+ .name = _name,
\
+ .parent_name = _parent,
\
+ .reg = _reg,
\
+ .bit = _bit,
\
+ .flags = _flags,
\
+ .cg_flags = _cgflags,
\
+ }
+
+#define GATE_AFE(_id, _name, _parent, _reg, _bit)
\
+ GATE_AFE_FLAGS(_id, _name, _parent, _reg, _bit,
\
+ CLK_SET_RATE_PARENT | CLK_IGNORE_UNUSED,
CLK_GATE_SET_TO_DISABLE)
Can you please explain what's the reason for
CLK_IGNORE_UNUSED
here?
Maybe we can solve some issue that you're facing in a cleaner
way.
Regards,
Angelo
Hi Angelo,
Because clk_disable_unused() calls clk_core_is_enabled(),
register
access happens in is_enabled() ops.
At the moment, the power for register access is not enabled, so
the
register read results in CPU hang.
That's why I added CLK_IGNORE_UNUSED here, but it can't resolve
all
issues. Actually, we met same problem when "cat
/sys/kernel/debug/clk/clk_summary" is used. We are still
suffering
the
problem.
I'm not sure if I can implement clk ops by myself, and exclude
the
registration of is_enabled() ops.
Is the power for register access enabled with a power domain?
Check drivers/clk/clk.c, grep for core->rpm_enabled.
If you enable runtime PM before registering the clocks, and you
register them
with the right struct device, the clock API will enable power for
you
before
trying to read the clock enable status.
Regards,
Angelo
Hi Angelo,
I tried the way in MT8195, but it caused circular lock problem.
Because mtcmos depends on some clocks, clk_bulk_prepare_enable is
also
used in scpsys_power_on()[1].
If the clock also depends on the power domain, this results in the
circular lock problem.
That's why I don't bind the power domain with these clocks.
This is not supposed to happen... can you please give me a (MT8195)
patch to
reproduce the issue that you're seeing?
I would like to investigate that to check if I can come up with a
good solution.
Thanks,
Angelo
Hi Angelo,
Sorry for replying late.
The original implementation about clock depending on power domain was a
cusotomized request, and it's not based on upstream code base. So I
can't apply the implementation directly. I tried to implement the
suggested solution in upstream code, but I can't reproduce the problem
successfully.
At the same time, we reviewed the difference between MT8195 and MT8188.
It's found that ADSP_INFRA should be kept ON to resolve the register
r/w access limitation in MT8188, so we can match the hardware design in
MT8195.
After discussing internally, we decided in favour of ADSP_INFRA
soloution. Althought the lock problem can't be seen, the new lock
relationship(prepare_lock -> genpd lock) is actually created.
In conclusion, ADSP_INFRA will be kept always on and I will remove
CLK_IGNORE_UNUSED flag in V3.
As far as the power consumption increase is ignorable (so, power consumption
is very little more), that's a good decision and I support that.
Regards,
Angelo
Thanks,
Trevor
[1]
https://urldefense.com/v3/__https://elixir.bootlin.com/linux/v6.1-rc2/source/drivers/soc/mediatek/mtk-pm-domains.c__;!!CTRNKA9wMg0ARbw!yVFCD-B4VZOxDXTGgDtpB0mJbVoY9tHODeICxthAC33lXMq6LRVTGS-4V-Dj129_cA$
Thanks,
Trevor