| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5556933f34858b50e5077dee62d430fc9a37f74b.camel@mediatek.com>
Date: Thu, 15 May 2025 09:31:35 +0000
From: Darren Ye (叶飞) <Darren.Ye@...iatek.com>
To: "wenst@...omium.org" <wenst@...omium.org>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linus.walleij@...aro.org" <linus.walleij@...aro.org>,
"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
"broonie@...nel.org" <broonie@...nel.org>, "brgl@...ev.pl" <brgl@...ev.pl>,
"conor+dt@...nel.org" <conor+dt@...nel.org>, "tiwai@...e.com"
<tiwai@...e.com>, "robh@...nel.org" <robh@...nel.org>, "lgirdwood@...il.com"
<lgirdwood@...il.com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
"perex@...ex.cz" <perex@...ex.cz>
Subject: Re: [PATCH v3 03/10] ASoC: mediatek: mt8196: support audio clock
control
On Thu, 2025-05-15 at 16:50 +0800, Chen-Yu Tsai wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> On Thu, May 15, 2025 at 4:40 PM AngeloGioacchino Del Regno
> <angelogioacchino.delregno@...labora.com> wrote:
> >
> > Il 14/05/25 10:11, Darren.Ye ha scritto:
> > > From: Darren Ye <darren.ye@...iatek.com>
> > >
> > > Add audio clock wrapper and audio tuner control.
> > >
> > > Signed-off-by: Darren Ye <darren.ye@...iatek.com>
> > > ---
> > > sound/soc/mediatek/mt8196/mt8196-afe-clk.c | 723
> > > ++++++++++++++++++
> > > sound/soc/mediatek/mt8196/mt8196-afe-clk.h | 142 ++++
> > > sound/soc/mediatek/mt8196/mt8196-audsys-clk.c | 252 ++++++
> > > sound/soc/mediatek/mt8196/mt8196-audsys-clk.h | 14 +
> > > .../soc/mediatek/mt8196/mt8196-audsys-clkid.h | 78 ++
> > > 5 files changed, 1209 insertions(+)
> > > create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> > > create mode 100644 sound/soc/mediatek/mt8196/mt8196-afe-clk.h
> > > create mode 100644 sound/soc/mediatek/mt8196/mt8196-audsys-
> > > clk.c
> > > create mode 100644 sound/soc/mediatek/mt8196/mt8196-audsys-
> > > clk.h
> > > create mode 100644 sound/soc/mediatek/mt8196/mt8196-audsys-
> > > clkid.h
> > >
> > > diff --git a/sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> > > b/sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> > > new file mode 100644
> > > index 000000000000..83b5ee9d30ef
> > > --- /dev/null
> > > +++ b/sound/soc/mediatek/mt8196/mt8196-afe-clk.c
> > > @@ -0,0 +1,723 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * mt8196-afe-clk.c -- Mediatek 8196 afe clock ctrl
> >
> > mt8196-afe-clk.c - MediaTek MT8196 AFE Clock Control
> >
> > > + *
> > > + * Copyright (c) 2024 MediaTek Inc.
> > > + * Author: Darren Ye <darren.ye@...iatek.com>
> > > + */
> > > +
> > > +#include <linux/clk.h>
> > > +#include <linux/regmap.h>
> > > +#include <linux/mfd/syscon.h>
> > > +#include "mt8196-afe-common.h"
> > > +#include "mt8196-audsys-clk.h"
> > > +#include "mt8196-afe-clk.h"
> > > +
> >
> > > +int mt8196_audsys_clk_register(struct mtk_base_afe *afe)
> > > +{
> > > + struct mt8196_afe_private *afe_priv = afe->platform_priv;
> > > + struct clk *clk;
> > > + struct clk_lookup *cl;
> > > + int i;
> > > +
> > > + afe_priv->lookup = devm_kcalloc(afe->dev, CLK_AFE_NR_CLK,
> > > + sizeof(*afe_priv->lookup),
> > > + GFP_KERNEL);
> > > +
> > > + if (!afe_priv->lookup)
> > > + return -ENOMEM;
> > > +
> > > + for (i = 0; i < ARRAY_SIZE(aud_clks); i++) {
> > > + const struct afe_gate *gate = &aud_clks[i];
> > > +
> > > + clk = clk_register_gate(afe->dev, gate->name, gate-
> > > >parent_name,
> > > + gate->flags, afe->base_addr
> > > + gate->reg,
> > > + gate->bit, gate->cg_flags,
> > > NULL);
> > > +
> > > + if (IS_ERR(clk)) {
> > > + dev_err(afe->dev, "Failed to register clk
> > > %s: %ld\n",
> > > + gate->name, PTR_ERR(clk));
> > > + continue;
> > > + }
> > > +
> >
> > All of the above, until...
> >
> > > + /* add clk_lookup for
> > > devm_clk_get(SND_SOC_DAPM_CLOCK_SUPPLY) */
> > > + cl = kzalloc(sizeof(*cl), GFP_KERNEL);
> > > + if (!cl)
> > > + return -ENOMEM;
> > > +
> > > + cl->clk = clk;
> > > + cl->con_id = gate->name;
> > > + cl->dev_id = dev_name(afe->dev);
> > > + cl->clk_hw = NULL;
> > > + clkdev_add(cl);
> >
> >
> > ...here, can be simplified with a single call to
> >
> > clk_register_clkdev(clk, gate->name, dev_name(afe->dev))
> >
> > or alternatively, you could simplify it even more:
> >
> >
> > static void mt8196_audsys_clk_unregister(void *data)
> > {
> > /* nothing to do here, remove this function */
> > }
> >
> > int mt8196_audsys_clk_register(struct mtk_base_afe *afe)
> > {
> > struct mt8196_afe_private *afe_priv = afe->platform_priv;
> > int i, ret;
> >
> > for (i = 0; i < ARRAY_SIZE(aud_clks); i++) {
> > const struct afe_gate *gate = &aud_clks[i];
> > struct clk_hw *hw;
> >
> > hw = devm_clk_hw_register_gate(afe->dev, gate-
> > >name, gate->parent_name,
> > gate->flags, afe-
> > >base_addr + gate->reg,
> > gate->bit, gate-
> > >cg_flags, NULL);
> > if (IS_ERR(clk)) {
> > dev_err(afe->dev, "Failed to register clk
> > %s: %ld\n",
> > gate->name, PTR_ERR(clk));
> > continue;
> > }
> >
> > ret = devm_clk_hw_register_clkdev(afe->dev, hw,
> > gate->name, dev_name(afe->dev));
> > if (ret)
> > return ret;
> > }
> >
> > return 0;
> > }
>
> There is no need to involve the clk subsystem. These are simply
> supply
> gates, be them for power or clks, one per bit. Simply model them as
> ASoC
> supply widgets, add appropriate routes for each so that the
> dependencies
> are correct, and ASoC will deal with them for you. No code is needed,
> just descriptions.
>
> _That_ is why I asked for he audio clocks to be integrated into the
> AFE driver.
>
> ChenYu
>
OK, I misunderstood. The next version will use widgets and route to
control the afe clk gate.
Best regards,
Darren
> > > +
> > > + afe_priv->lookup[i] = cl;
> > > + }
> > > +
> > > + return devm_add_action_or_reset(afe->dev,
> > > mt8196_audsys_clk_unregister, afe);
> > > +}
> >
> > Cheers,
> > Angelo
> >
Powered by blists - more mailing lists