[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAGXv+5HQrMY+osCZKVOq28fQi-Be-eZ=_-=5HcrkacivHekOTQ@mail.gmail.com>
Date: Mon, 25 Aug 2025 16:50:50 +0200
From: Chen-Yu Tsai <wenst@...omium.org>
To: Laura Nao <laura.nao@...labora.com>
Cc: angelogioacchino.delregno@...labora.com, conor+dt@...nel.org,
devicetree@...r.kernel.org, guangjie.song@...iatek.com, kernel@...labora.com,
krzk+dt@...nel.org, linux-arm-kernel@...ts.infradead.org,
linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-mediatek@...ts.infradead.org, matthias.bgg@...il.com,
mturquette@...libre.com, netdev@...r.kernel.org, nfraprado@...labora.com,
p.zabel@...gutronix.de, richardcochran@...il.com, robh@...nel.org,
sboyd@...nel.org
Subject: Re: [PATCH v4 07/27] clk: mediatek: clk-gate: Add ops for gates with
HW voter
On Mon, Aug 25, 2025 at 2:52 PM Laura Nao <laura.nao@...labora.com> wrote:
>
> On 8/15/25 05:37, Chen-Yu Tsai wrote:
> > On Tue, Aug 5, 2025 at 10:55 PM Laura Nao <laura.nao@...labora.com> wrote:
> >>
> >> MT8196 use a HW voter for gate enable/disable control. Voting is
> >> performed using set/clr regs, with a status bit used to verify the vote
> >> state. Add new set of gate clock operations with support for voting via
> >> set/clr regs.
> >>
> >> Reviewed-by: Nícolas F. R. A. Prado <nfraprado@...labora.com>
> >> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
> >> Signed-off-by: Laura Nao <laura.nao@...labora.com>
> >> ---
> >> drivers/clk/mediatek/clk-gate.c | 77 +++++++++++++++++++++++++++++++--
> >> drivers/clk/mediatek/clk-gate.h | 3 ++
> >> 2 files changed, 77 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/clk/mediatek/clk-gate.c b/drivers/clk/mediatek/clk-gate.c
> >> index 0375ccad4be3..426f3a25763d 100644
> >> --- a/drivers/clk/mediatek/clk-gate.c
> >> +++ b/drivers/clk/mediatek/clk-gate.c
> >> @@ -5,6 +5,7 @@
> >> */
> >>
> >> #include <linux/clk-provider.h>
> >> +#include <linux/dev_printk.h>
> >> #include <linux/mfd/syscon.h>
> >> #include <linux/module.h>
> >> #include <linux/printk.h>
> >> @@ -12,14 +13,19 @@
> >> #include <linux/slab.h>
> >> #include <linux/types.h>
> >>
> >> +#include "clk-mtk.h"
> >> #include "clk-gate.h"
> >>
> >> struct mtk_clk_gate {
> >> struct clk_hw hw;
> >> struct regmap *regmap;
> >> + struct regmap *regmap_hwv;
> >> int set_ofs;
> >> int clr_ofs;
> >> int sta_ofs;
> >> + unsigned int hwv_set_ofs;
> >> + unsigned int hwv_clr_ofs;
> >> + unsigned int hwv_sta_ofs;
> >> u8 bit;
> >> };
> >>
> >> @@ -100,6 +106,28 @@ static void mtk_cg_disable_inv(struct clk_hw *hw)
> >> mtk_cg_clr_bit(hw);
> >> }
> >>
> >> +static int mtk_cg_hwv_set_en(struct clk_hw *hw, bool enable)
> >> +{
> >> + struct mtk_clk_gate *cg = to_mtk_clk_gate(hw);
> >> + u32 val;
> >> +
> >> + regmap_write(cg->regmap_hwv, enable ? cg->hwv_set_ofs : cg->hwv_clr_ofs, BIT(cg->bit));
> >> +
> >> + return regmap_read_poll_timeout_atomic(cg->regmap_hwv, cg->hwv_sta_ofs, val,
> >> + val & BIT(cg->bit),
> >> + 0, MTK_WAIT_HWV_DONE_US);
> >> +}
> >> +
> >> +static int mtk_cg_hwv_enable(struct clk_hw *hw)
> >> +{
> >> + return mtk_cg_hwv_set_en(hw, true);
> >> +}
> >> +
> >> +static void mtk_cg_hwv_disable(struct clk_hw *hw)
> >> +{
> >> + mtk_cg_hwv_set_en(hw, false);
> >> +}
> >> +
> >> static int mtk_cg_enable_no_setclr(struct clk_hw *hw)
> >> {
> >> mtk_cg_clr_bit_no_setclr(hw);
> >> @@ -124,6 +152,15 @@ static void mtk_cg_disable_inv_no_setclr(struct clk_hw *hw)
> >> mtk_cg_clr_bit_no_setclr(hw);
> >> }
> >>
> >> +static bool mtk_cg_uses_hwv(const struct clk_ops *ops)
> >> +{
> >> + if (ops == &mtk_clk_gate_hwv_ops_setclr ||
> >> + ops == &mtk_clk_gate_hwv_ops_setclr_inv)
> >> + return true;
> >> +
> >> + return false;
> >> +}
> >> +
> >> const struct clk_ops mtk_clk_gate_ops_setclr = {
> >> .is_enabled = mtk_cg_bit_is_cleared,
> >> .enable = mtk_cg_enable,
> >> @@ -138,6 +175,20 @@ const struct clk_ops mtk_clk_gate_ops_setclr_inv = {
> >> };
> >> EXPORT_SYMBOL_GPL(mtk_clk_gate_ops_setclr_inv);
> >>
> >> +const struct clk_ops mtk_clk_gate_hwv_ops_setclr = {
> >> + .is_enabled = mtk_cg_bit_is_cleared,
> >> + .enable = mtk_cg_hwv_enable,
> >> + .disable = mtk_cg_hwv_disable,
> >> +};
> >> +EXPORT_SYMBOL_GPL(mtk_clk_gate_hwv_ops_setclr);
> >> +
> >> +const struct clk_ops mtk_clk_gate_hwv_ops_setclr_inv = {
> >> + .is_enabled = mtk_cg_bit_is_set,
> >> + .enable = mtk_cg_hwv_enable,
> >> + .disable = mtk_cg_hwv_disable,
> >> +};
> >> +EXPORT_SYMBOL_GPL(mtk_clk_gate_hwv_ops_setclr_inv);
> >> +
> >> const struct clk_ops mtk_clk_gate_ops_no_setclr = {
> >> .is_enabled = mtk_cg_bit_is_cleared,
> >> .enable = mtk_cg_enable_no_setclr,
> >> @@ -153,8 +204,9 @@ const struct clk_ops mtk_clk_gate_ops_no_setclr_inv = {
> >> EXPORT_SYMBOL_GPL(mtk_clk_gate_ops_no_setclr_inv);
> >>
> >> static struct clk_hw *mtk_clk_register_gate(struct device *dev,
> >> - const struct mtk_gate *gate,
> >> - struct regmap *regmap)
> >> + const struct mtk_gate *gate,
> >> + struct regmap *regmap,
> >> + struct regmap *regmap_hwv)
> >> {
> >> struct mtk_clk_gate *cg;
> >> int ret;
> >> @@ -169,11 +221,22 @@ static struct clk_hw *mtk_clk_register_gate(struct device *dev,
> >> init.parent_names = gate->parent_name ? &gate->parent_name : NULL;
> >> init.num_parents = gate->parent_name ? 1 : 0;
> >> init.ops = gate->ops;
> >> + if (mtk_cg_uses_hwv(init.ops) && !regmap_hwv) {
> >> + dev_err(dev, "regmap not found for hardware voter clocks\n");
> >> + return ERR_PTR(-ENXIO);
> >
> > return dev_err_probe()?
> >
> > I believe the same applies to the previous patch.
> >
>
> mtk_clk_register_gate and mtk_clk_register_mux actually both return a
> struct clk_hw *.
Oops, you're right. If that case I believe dev_err_ptr_probe() could be
used?
ChenYu
Powered by blists - more mailing lists