Hi Abel,
> Subject: Re: [PATCH V2 5/8] clk: imx: add i.MX93 clk gate
>
> On 22-08-15 09:30:36, Peng Fan (OSS) wrote:
> > From: Peng Fan <peng.fan@xxxxxxx>
> >
> > i.MX93 LPCG is different from i.MX8M CCGR. Although
> > imx_clk_hw_gate4_flags is used here, it not strictly match i.MX93. i.MX93
> has such design:
> > - LPCG_DIRECT use BIT0 as on/off gate when LPCG_AUTHEN CPU_LPM is 0
> > - LPCG_LPM_CUR use BIT[2:0] as on/off gate when LPCG_AUTHEN
> CPU_LPM
> > is 1
> >
> > The current implementation suppose CPU_LPM is 0, and use LPCG_DIRECT
> > BIT[1:0] as on/off gate. Although BIT1 is touched, actually BIT1 is
> > reserved.
> >
> > And imx_clk_hw_gate4_flags use mask 0x3 to determine whether the clk
> > is enabled or not, but i.MX93 LPCG only use BIT0 to control when
> > CPU_LPM is 0. So clk disabled unused during kernel boot not able to
> > gate off the unused clocks.
> >
> > To match i.MX93 LPCG, introduce imx93_clk_gate.
> >
> > Signed-off-by: Peng Fan <peng.fan@xxxxxxx>
> > Reviewed-by: Ye Li <ye.li@xxxxxxx>
> > Reviewed-by: Jacky Bai <ping.bai@xxxxxxx>
> > ---
> > drivers/clk/imx/Makefile | 2 +-
> > drivers/clk/imx/clk-gate-93.c | 199
> ++++++++++++++++++++++++++++++++++
> > drivers/clk/imx/clk.h | 4 +
> > 3 files changed, 204 insertions(+), 1 deletion(-) create mode 100644
> > drivers/clk/imx/clk-gate-93.c
> >
> > diff --git a/drivers/clk/imx/Makefile b/drivers/clk/imx/Makefile index
> > 88b9b9285d22..89fe72327788 100644
> > --- a/drivers/clk/imx/Makefile
> > +++ b/drivers/clk/imx/Makefile
> > @@ -28,7 +28,7 @@ obj-$(CONFIG_CLK_IMX8MN) += clk-imx8mn.o
> > obj-$(CONFIG_CLK_IMX8MP) += clk-imx8mp.o
> > obj-$(CONFIG_CLK_IMX8MQ) += clk-imx8mq.o
> >
> > -obj-$(CONFIG_CLK_IMX93) += clk-imx93.o
> > +obj-$(CONFIG_CLK_IMX93) += clk-imx93.o clk-gate-93.o
> >
> > obj-$(CONFIG_MXC_CLK_SCU) += clk-imx-scu.o clk-imx-lpcg-scu.o
> > clk-imx-scu-$(CONFIG_CLK_IMX8QXP) += clk-scu.o clk-imx8qxp.o \ diff
> > --git a/drivers/clk/imx/clk-gate-93.c b/drivers/clk/imx/clk-gate-93.c
> > new file mode 100644 index 000000000000..ceb56b290394
> > --- /dev/null
> > +++ b/drivers/clk/imx/clk-gate-93.c
> > @@ -0,0 +1,199 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright 2022 NXP
> > + *
> > + * Peng Fan <peng.fan@xxxxxxx>
> > + */
> > +
> > +#include <linux/clk-provider.h>
> > +#include <linux/errno.h>
> > +#include <linux/export.h>
> > +#include <linux/io.h>
> > +#include <linux/iopoll.h>
> > +#include <linux/slab.h>
> > +
> > +#include "clk.h"
> > +
> > +#define DIRECT_OFFSET 0x0
> > +
> > +/*
> > + * 0b000 - LPCG will be OFF in any CPU mode.
> > + * 0b100 - LPCG will be ON in any CPU mode.
> > + */
> > +#define LPM_SETTING_OFF 0x0
> > +#define LPM_SETTING_ON 0x4
> > +
> > +#define LPM_CUR_OFFSET 0x1c
> > +
> > +#define AUTHEN_OFFSET 0x30
> > +#define CPULPM_EN BIT(2)
> > +#define TZ_NS_SHIFT 9
> > +#define TZ_NS_MASK BIT(9)
> > +
> > +#define WHITE_LIST_SHIFT 16
> > +
> > +struct imx93_clk_gate {
> > + struct clk_hw hw;
> > + void __iomem *reg;
> > + u32 bit_idx;
> > + u32 val;
> > + u32 mask;
> > + spinlock_t *lock;
> > + unsigned int *share_count;
> > +};
> > +
> > +#define to_imx93_clk_gate(_hw) container_of(_hw, struct
> > +imx93_clk_gate, hw)
> > +
> > +static void imx93_clk_gate_do_hardware(struct clk_hw *hw, bool
> > +enable) {
> > + struct imx93_clk_gate *gate = to_imx93_clk_gate(hw);
> > + u32 val;
> > +
> > + val = readl(gate->reg + AUTHEN_OFFSET);
> > + if (val & CPULPM_EN) {
> > + val = enable ? LPM_SETTING_ON : LPM_SETTING_OFF;
> > + writel(val, gate->reg + LPM_CUR_OFFSET);
> > + } else {
> > + val = readl(gate->reg + DIRECT_OFFSET);
> > + val &= ~(gate->mask << gate->bit_idx);
> > + if (enable)
> > + val |= (gate->val & gate->mask) << gate->bit_idx;
> > + writel(val, gate->reg + DIRECT_OFFSET);
> > + }
> > +}
> > +
> > +static int imx93_clk_gate_enable(struct clk_hw *hw) {
> > + struct imx93_clk_gate *gate = to_imx93_clk_gate(hw);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(gate->lock, flags);
> > +
> > + if (gate->share_count && (*gate->share_count)++ > 0)
> > + goto out;
> > +
> > + imx93_clk_gate_do_hardware(hw, true);
> > +out:
> > + spin_unlock_irqrestore(gate->lock, flags);
> > +
> > + return 0;
> > +}
>
> Just wondering if we could use the existing clk-gate2 since we would only
> have to implement the ops that are different there.
>
I thought about this, but i.MX9 LPCG gate has a different design, it is
not i.MX8M CCGR gate. Although the first version of the i.MX93 clk
driver use clk-gate2, but it could not support i.MX93 well.
You could see i.MX93 gate API use different input parameters.
Clk-gate2 not support LPM(low power mode), AUTHEN(authentication),
WHITE_LIST(domain control list).
i.MX93 LPCG has two working modes, DIRECT mode, LPM mode.
DIRECT mode is a 1 bit control bit if LPM is not enabled.
If LPM enabled, LPCG use a similar way as CCGR domain bits
> Also, would the next i.MX9 platforms also use this? Or will we have one
> similar driver for each new platform?
i.MX9 series use same LPCG design per my understanding. I hope
there are no big changes in hardware in future, so we could reuse
same driver.
Thanks,
Peng.
>
> > +
> > +static void imx93_clk_gate_disable(struct clk_hw *hw) {
> > + struct imx93_clk_gate *gate = to_imx93_clk_gate(hw);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(gate->lock, flags);
> > +
> > + if (gate->share_count) {
> > + if (WARN_ON(*gate->share_count == 0))
> > + goto out;
> > + else if (--(*gate->share_count) > 0)
> > + goto out;
> > + }
> > +
> > + imx93_clk_gate_do_hardware(hw, false);
> > +out:
> > + spin_unlock_irqrestore(gate->lock, flags); }
> > +
> > +static int imx93_clk_gate_reg_is_enabled(struct imx93_clk_gate *gate)
> > +{
> > + u32 val = readl(gate->reg + AUTHEN_OFFSET);
> > +
> > + if (val & CPULPM_EN) {
> > + val = readl(gate->reg + LPM_CUR_OFFSET);
> > + if (val == LPM_SETTING_ON)
> > + return 1;
> > + } else {
> > + val = readl(gate->reg);
> > + if (((val >> gate->bit_idx) & gate->mask) == gate->val)
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int imx93_clk_gate_is_enabled(struct clk_hw *hw) {
> > + struct imx93_clk_gate *gate = to_imx93_clk_gate(hw);
> > + unsigned long flags;
> > + int ret;
> > +
> > + spin_lock_irqsave(gate->lock, flags);
> > +
> > + ret = imx93_clk_gate_reg_is_enabled(gate);
> > +
> > + spin_unlock_irqrestore(gate->lock, flags);
> > +
> > + return ret;
> > +}
> > +
> > +static void imx93_clk_gate_disable_unused(struct clk_hw *hw) {
> > + struct imx93_clk_gate *gate = to_imx93_clk_gate(hw);
> > + unsigned long flags;
> > +
> > + spin_lock_irqsave(gate->lock, flags);
> > +
> > + if (!gate->share_count || *gate->share_count == 0)
> > + imx93_clk_gate_do_hardware(hw, false);
> > +
> > + spin_unlock_irqrestore(gate->lock, flags); }
> > +
> > +static const struct clk_ops imx93_clk_gate_ops = {
> > + .enable = imx93_clk_gate_enable,
> > + .disable = imx93_clk_gate_disable,
> > + .disable_unused = imx93_clk_gate_disable_unused,
> > + .is_enabled = imx93_clk_gate_is_enabled, };
> > +
> > +static const struct clk_ops imx93_clk_gate_ro_ops = {
> > + .is_enabled = imx93_clk_gate_is_enabled, };
> > +
> > +struct clk_hw *imx93_clk_gate(struct device *dev, const char *name,
> const char *parent_name,
> > + unsigned long flags, void __iomem *reg, u32
> bit_idx, u32 val,
> > + u32 mask, u32 domain_id, unsigned int
> *share_count) {
> > + struct imx93_clk_gate *gate;
> > + struct clk_hw *hw;
> > + struct clk_init_data init;
> > + int ret;
> > + u32 authen;
> > +
> > + gate = kzalloc(sizeof(struct imx93_clk_gate), GFP_KERNEL);
> > + if (!gate)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + gate->reg = reg;
> > + gate->lock = &imx_ccm_lock;
> > + gate->bit_idx = bit_idx;
> > + gate->val = val;
> > + gate->mask = mask;
> > + gate->share_count = share_count;
> > +
> > + init.name = name;
> > + init.ops = &imx93_clk_gate_ops;
> > + init.flags = flags | CLK_SET_RATE_PARENT |
> CLK_OPS_PARENT_ENABLE;
> > + init.parent_names = parent_name ? &parent_name : NULL;
> > + init.num_parents = parent_name ? 1 : 0;
> > +
> > + gate->hw.init = &init;
> > + hw = &gate->hw;
> > +
> > + authen = readl(reg + AUTHEN_OFFSET);
> > + if (!(authen & TZ_NS_MASK) || !(authen & BIT(WHITE_LIST_SHIFT +
> domain_id)))
> > + init.ops = &imx93_clk_gate_ro_ops;
> > +
> > + ret = clk_hw_register(dev, hw);
> > + if (ret) {
> > + kfree(gate);
> > + return ERR_PTR(ret);
> > + }
> > +
> > + return hw;
> > +}
> > +EXPORT_SYMBOL_GPL(imx93_clk_gate);
> > diff --git a/drivers/clk/imx/clk.h b/drivers/clk/imx/clk.h index
> > 396a5ea75083..dd49f90110e8 100644
> > --- a/drivers/clk/imx/clk.h
> > +++ b/drivers/clk/imx/clk.h
> > @@ -451,6 +451,10 @@ struct clk_hw *imx93_clk_composite_flags(const
> char *name,
> > imx93_clk_composite_flags(name, parent_names, num_parents,
> reg, domain_id \
> > CLK_SET_RATE_NO_REPARENT |
> CLK_OPS_PARENT_ENABLE)
> >
> > +struct clk_hw *imx93_clk_gate(struct device *dev, const char *name,
> const char *parent_name,
> > + unsigned long flags, void __iomem *reg, u32
> bit_idx, u32 val,
> > + u32 mask, u32 domain_id, unsigned int
> *share_count);
> > +
> > struct clk_hw *imx_clk_hw_divider_gate(const char *name, const char
> *parent_name,
> > unsigned long flags, void __iomem *reg, u8 shift, u8 width,
> > u8 clk_divider_flags, const struct clk_div_table *table,
> > --
> > 2.37.1
> >
