[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-Id: <20250826083616.15558-1-laura.nao@collabora.com>
Date: Tue, 26 Aug 2025 10:36:16 +0200
From: Laura Nao <laura.nao@...labora.com>
To: wenst@...omium.org
Cc: angelogioacchino.delregno@...labora.com,
conor+dt@...nel.org,
devicetree@...r.kernel.org,
guangjie.song@...iatek.com,
kernel@...labora.com,
krzk+dt@...nel.org,
laura.nao@...labora.com,
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 8/25/25 16:50, Chen-Yu Tsai wrote:
> 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?
Indeed, thanks for the pointer. I'll fix it in the next revision.
Laura
Powered by blists - more mailing lists