Messages in this thread | | | Date | Wed, 26 Oct 2022 10:18:41 +0200 | Subject | Re: [PATCH v2 03/12] ASoC: mediatek: mt8188: support audsys clock | From | AngeloGioacchino Del Regno <> |
| |
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@mediatek.com> >>>>> --- >>>>> 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@mediatek.com> >>>>> + */ >>>>> + >>>>> +#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
> [1] > https://elixir.bootlin.com/linux/v6.1-rc2/source/drivers/soc/mediatek/mtk-pm-domains.c > > Thanks, > Trevor > >
| |