Messages in this thread Patch in this message | | | Date | Thu, 1 Sep 2022 10:05:00 +0200 | Subject | Re: [PATCH 3/4] clk: mediatek: Add new clock driver to handle FHCTL hardware | From | AngeloGioacchino Del Regno <> |
| |
Il 31/08/22 14:48, Johnson Wang ha scritto: > To implement frequency hopping and spread spectrum clocking > function, we introduce new clock type and APIs to handle > FHCTL hardware. > > Co-developed-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > Signed-off-by: Edward-JW Yang <edward-jw.yang@mediatek.com> > Signed-off-by: Johnson Wang <johnson.wang@mediatek.com> > --- > drivers/clk/mediatek/Makefile | 2 +- > drivers/clk/mediatek/clk-fhctl.c | 258 +++++++++++++++++++++++++++++ > drivers/clk/mediatek/clk-fhctl.h | 27 +++ > drivers/clk/mediatek/clk-pllfh.c | 271 +++++++++++++++++++++++++++++++ > drivers/clk/mediatek/clk-pllfh.h | 81 +++++++++ > 5 files changed, 638 insertions(+), 1 deletion(-) > create mode 100644 drivers/clk/mediatek/clk-fhctl.c > create mode 100644 drivers/clk/mediatek/clk-fhctl.h > create mode 100644 drivers/clk/mediatek/clk-pllfh.c > create mode 100644 drivers/clk/mediatek/clk-pllfh.h > > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile > index caf2ce93d666..0e674a55e51e 100644 > --- a/drivers/clk/mediatek/Makefile > +++ b/drivers/clk/mediatek/Makefile > @@ -1,5 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > -obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o > +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o clk-apmixed.o clk-cpumux.o reset.o clk-mux.o clk-fhctl.o clk-pllfh.o > > obj-$(CONFIG_COMMON_CLK_MT6765) += clk-mt6765.o > obj-$(CONFIG_COMMON_CLK_MT6765_AUDIOSYS) += clk-mt6765-audio.o > diff --git a/drivers/clk/mediatek/clk-fhctl.c b/drivers/clk/mediatek/clk-fhctl.c > new file mode 100644 > index 000000000000..69bd4ce2bc72 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-fhctl.c > @@ -0,0 +1,258 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Edward-JW Yang <edward-jw.yang@mediatek.com> > + */ > +
#include <linux/io.h>
Please don't rely on implicit header inclusion.
> +#include <linux/iopoll.h> > + > +#include "clk-mtk.h" > +#include "clk-pllfh.h" > +#include "clk-fhctl.h" > + > +#define PERCENT_TO_DDSLMT(dds, percent_m10) \ > + ((((dds) * (percent_m10)) >> 5) / 100) > + > +static const struct fhctl_offset fhctl_offset = { > + .offset_hp_en = 0x0, > + .offset_clk_con = 0x8, > + .offset_rst_con = 0xc, > + .offset_slope0 = 0x10, > + .offset_slope1 = 0x14, > + .offset_cfg = 0x0, > + .offset_updnlmt = 0x4, > + .offset_dds = 0x8, > + .offset_dvfs = 0xc, > + .offset_mon = 0x10, > +}; > + > +const struct fhctl_offset *fhctl_get_offset_table(void) > +{ > + return &fhctl_offset; > +} > + > +static void dump_hw(struct mtk_clk_pll *pll, struct fh_pll_regs *regs, > + const struct fh_pll_data *data) > +{ > + pr_info("hp_en<%x>,clk_con<%x>,slope0<%x>,slope1<%x>\n", > + readl(regs->reg_hp_en), readl(regs->reg_clk_con), > + readl(regs->reg_slope0), readl(regs->reg_slope1)); > + pr_info("cfg<%x>,lmt<%x>,dds<%x>,dvfs<%x>,mon<%x>\n", > + readl(regs->reg_cfg), readl(regs->reg_updnlmt), > + readl(regs->reg_dds), readl(regs->reg_dvfs), > + readl(regs->reg_mon)); > + pr_info("pcw<%x>\n", readl(pll->pcw_addr)); > +} > + > +static int fhctl_set_ssc_regs(struct mtk_clk_pll *pll, struct fh_pll_regs *regs, > + const struct fh_pll_data *data, u32 rate) > +{ > + u32 updnlmt_val, r; > + > + writel((readl(regs->reg_cfg) & ~(data->frddsx_en)), regs->reg_cfg); > + writel((readl(regs->reg_cfg) & ~(data->sfstrx_en)), regs->reg_cfg); > + writel((readl(regs->reg_cfg) & ~(data->fhctlx_en)), regs->reg_cfg); > + > + if (rate > 0) { > + /* Set the relative parameter registers (dt/df/upbnd/downbnd) */ > + r = readl(regs->reg_cfg); > + r &= ~(data->msk_frddsx_dys); > + r |= (data->df_val << (ffs(data->msk_frddsx_dys) - 1)); > + writel(r, regs->reg_cfg); > + > + r = readl(regs->reg_cfg); > + r &= ~(data->msk_frddsx_dts); > + r |= (data->dt_val << (ffs(data->msk_frddsx_dts) - 1)); > + writel(r, regs->reg_cfg); > + > + writel((readl(pll->pcw_addr) & data->dds_mask) | data->tgl_org, > + regs->reg_dds); > + > + /* Calculate UPDNLMT */ > + updnlmt_val = PERCENT_TO_DDSLMT((readl(regs->reg_dds) & > + data->dds_mask), rate) << > + data->updnlmt_shft; > + > + writel(updnlmt_val, regs->reg_updnlmt); > + writel(readl(regs->reg_hp_en) | BIT(data->fh_id), > + regs->reg_hp_en); > + /* Enable SSC */ > + writel(readl(regs->reg_cfg) | data->frddsx_en, regs->reg_cfg); > + /* Enable Hopping control */ > + writel(readl(regs->reg_cfg) | data->fhctlx_en, regs->reg_cfg); > + > + } else { > + /* Switch to APMIXEDSYS control */ > + writel(readl(regs->reg_hp_en) & ~BIT(data->fh_id), > + regs->reg_hp_en); > + /* Wait for DDS to be stable */ > + udelay(30); > + } > + > + return 0; > +} > + > +static int hopping_hw_flow(struct mtk_clk_pll *pll, struct fh_pll_regs *regs, > + const struct fh_pll_data *data, > + struct fh_pll_state *state, unsigned int new_dds) > +{ > + u32 dds_mask = data->dds_mask; > + u32 mon_dds = 0; > + u32 con_pcw_tmp; > + int ret; > + > + if (state->ssc_rate) > + fhctl_set_ssc_regs(pll, regs, data, 0); > + > + writel((readl(pll->pcw_addr) & dds_mask) | data->tgl_org, > + regs->reg_dds); > + > + writel(readl(regs->reg_cfg) | data->sfstrx_en, regs->reg_cfg); > + writel(readl(regs->reg_cfg) | data->fhctlx_en, regs->reg_cfg); > + writel(data->slope0_value, regs->reg_slope0); > + writel(data->slope1_value, regs->reg_slope1); > + > + writel(readl(regs->reg_hp_en) | BIT(data->fh_id), regs->reg_hp_en); > + writel((new_dds) | (data->dvfs_tri), regs->reg_dvfs); > + > + /* Wait 1000 us until DDS stable */ > + ret = readl_poll_timeout_atomic(regs->reg_mon, mon_dds, > + (mon_dds & dds_mask) == new_dds, > + 10, 1000); > + if (ret) { > + pr_warn("%s: FHCTL hopping timeout\n", pll->data->name); > + dump_hw(pll, regs, data); > + } > + > + con_pcw_tmp = readl(pll->pcw_addr) & (~dds_mask); > + con_pcw_tmp = (con_pcw_tmp | (readl(regs->reg_mon) & dds_mask) | > + data->pcwchg); > + > + writel(con_pcw_tmp, pll->pcw_addr); > + writel(readl(regs->reg_hp_en) & ~BIT(data->fh_id), regs->reg_hp_en); > + > + if (state->ssc_rate) > + fhctl_set_ssc_regs(pll, regs, data, state->ssc_rate);
Does it really make sense to set the SSC registers if the FH operation times out?
> + > + return ret; > +} > + > +static unsigned int __get_postdiv(struct mtk_clk_pll *pll, > + struct fh_pll_regs *regs, > + const struct fh_pll_data *fh_data)
You don't need regs, nor fh_data, so this can be as simple as
static unsigned int __get_postdiv(struct mtk_clk_pll *pll)
> +{ > + unsigned int regval; > + > + regval = readl(pll->pd_addr) >> pll->data->pd_shift; > + regval &= POSTDIV_MASK; > +
return BIT(regval);
> + return (1 << regval); > +} > + > +static void __set_postdiv(struct mtk_clk_pll *pll, struct fh_pll_regs *regs, > + const struct fh_pll_data *data, int postdiv)
static void __set_postdiv(struct mtk_clk_pll *pll, unsigned int val)
> +{ > + unsigned int regval; > + > + regval = readl(pll->pd_addr); > + regval &= ~(POSTDIV_MASK << pll->data->pd_shift); > + regval |= (ffs(postdiv) - 1) << pll->data->pd_shift; > + writel(regval, pll->pd_addr); > +} > + > +static int fhctl_hopping(struct mtk_fh *fh, unsigned int new_dds, int postdiv)
The postdiv cannot ever be negative, so you can change it to an unsigned type...
static int fhctl_hopping(struct mtk_fh *fh, unsigned int new_dds, unsigned int postdiv)
> +{ > + const struct fh_pll_data *data = &fh->pllfh_data->data; > + struct fh_pll_state *state = &fh->pllfh_data->state; > + struct fh_pll_regs *regs = &fh->regs; > + struct mtk_clk_pll *pll = &fh->clk_pll; > + spinlock_t *lock = fh->lock; > + unsigned int pll_postdiv; > + unsigned long flags = 0; > + int ret; > +
...so since postdiv is unsigned, we can write this conditional as
if (postdiv)
> + if (postdiv > 0) { > + pll_postdiv = __get_postdiv(pll, regs, data); > + > + if (postdiv > pll_postdiv) > + __set_postdiv(pll, regs, data, postdiv); > + } > + > + spin_lock_irqsave(lock, flags); > + > + ret = hopping_hw_flow(pll, regs, data, state, new_dds); > + > + spin_unlock_irqrestore(lock, flags); > + > + if (postdiv > 0) { > + if (postdiv < pll_postdiv)
...and this one as
if (postdiv && postdiv < pll_postdiv)
> + __set_postdiv(pll, regs, data, postdiv); > + } > + > + return ret; > +} > + > +static int __fhctl_ssc_enable(struct mtk_fh *fh, u32 rate) > +{ > + const struct fh_pll_data *data = &fh->pllfh_data->data; > + struct fh_pll_state *state = &fh->pllfh_data->state; > + struct fh_pll_regs *regs = &fh->regs; > + struct mtk_clk_pll *pll = &fh->clk_pll; > + spinlock_t *lock = fh->lock; > + unsigned long flags = 0; > + > + spin_lock_irqsave(lock, flags); > + > + fhctl_set_ssc_regs(pll, regs, data, rate); > + state->ssc_rate = rate; > + > + spin_unlock_irqrestore(lock, flags); > + > + return 0; > +} > + > +static int fhctl_ssc_enable(struct mtk_fh *fh, u32 rate) > +{ > + return __fhctl_ssc_enable(fh, rate); > +} > + > +static int fhctl_ssc_disable(struct mtk_fh *fh) > +{ > + return __fhctl_ssc_enable(fh, 0); > +} > + > +static const struct fh_operation fhctl_ops = { > + .hopping = fhctl_hopping, > + .ssc_enable = fhctl_ssc_enable,
Just one callback for ssc_enable is enough, it's fine to keep the logic as ssc_enable(fh, >0) to enable and ssc_enable(fh <=0) to disable.
> + .ssc_disable = fhctl_ssc_disable, > +}; > + > +const struct fh_operation *fhctl_get_ops(void) > +{ > + return &fhctl_ops; > +} > + > +void fhctl_hw_init(struct mtk_fh *fh) > +{ > + const struct fh_pll_data data = fh->pllfh_data->data; > + struct fh_pll_state state = fh->pllfh_data->state; > + struct fh_pll_regs regs = fh->regs; > + u32 val; > + > + /* initial hw register */ > + val = readl(regs.reg_clk_con) | BIT(data.fh_id); > + writel(val, regs.reg_clk_con); > + > + val = readl(regs.reg_rst_con) & ~BIT(data.fh_id); > + writel(val, regs.reg_rst_con); > + val = readl(regs.reg_rst_con) | BIT(data.fh_id); > + writel(val, regs.reg_rst_con); > + > + writel(0x0, regs.reg_cfg); > + writel(0x0, regs.reg_updnlmt); > + writel(0x0, regs.reg_dds); > + > + /* enable ssc if needed */ > + if (state.ssc_rate) > + fh->ops->ssc_enable(fh, state.ssc_rate); > +} > diff --git a/drivers/clk/mediatek/clk-fhctl.h b/drivers/clk/mediatek/clk-fhctl.h > new file mode 100644 > index 000000000000..3cd4921c39e9 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-fhctl.h > @@ -0,0 +1,27 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Edward-JW Yang <edward-jw.yang@mediatek.com> > + */ > + > +#ifndef __CLK_FHCTL_H > +#define __CLK_FHCTL_H > + One blank line is enough.
> + > +struct fhctl_offset { > + u32 offset_hp_en; > + u32 offset_clk_con; > + u32 offset_rst_con; > + u32 offset_slope0; > + u32 offset_slope1; > + u32 offset_cfg; > + u32 offset_updnlmt; > + u32 offset_dds; > + u32 offset_dvfs; > + u32 offset_mon; > +}; > +const struct fhctl_offset *fhctl_get_offset_table(void); > +const struct fh_operation *fhctl_get_ops(void); > +void fhctl_hw_init(struct mtk_fh *fh); > + > +#endif > diff --git a/drivers/clk/mediatek/clk-pllfh.c b/drivers/clk/mediatek/clk-pllfh.c > new file mode 100644 > index 000000000000..71b35323b526 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-pllfh.c > @@ -0,0 +1,271 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Edward-JW Yang <edward-jw.yang@mediatek.com> > + */ > + > +#include <linux/of.h> > +#include <linux/of_address.h> > +#include <linux/io.h> > +#include <linux/slab.h> > +#include <linux/clkdev.h> > +#include <linux/delay.h> > + > +#include "clk-mtk.h" > +#include "clk-pllfh.h" > +#include "clk-fhctl.h" > + > +static DEFINE_SPINLOCK(pllfh_lock); > + > +inline struct mtk_fh *to_mtk_fh(struct clk_hw *hw) > +{ > + struct mtk_clk_pll *pll = to_mtk_clk_pll(hw); > + > + return container_of(pll, struct mtk_fh, clk_pll); > +} > + > +static int mtk_fhctl_set_rate(struct clk_hw *hw, unsigned long rate, > + unsigned long parent_rate) > +{ > + struct mtk_clk_pll *pll = to_mtk_clk_pll(hw); > + struct mtk_fh *fh = to_mtk_fh(hw); > + u32 pcw = 0; > + u32 postdiv; > + > + mtk_pll_calc_values(pll, &pcw, &postdiv, rate, parent_rate); > + fh->ops->hopping(fh, pcw, postdiv);
return fh->ops->hopping(....); > + > + return 0; > +} > +
..snip..
> diff --git a/drivers/clk/mediatek/clk-pllfh.h b/drivers/clk/mediatek/clk-pllfh.h > new file mode 100644 > index 000000000000..d9f7c8527548 > --- /dev/null > +++ b/drivers/clk/mediatek/clk-pllfh.h > @@ -0,0 +1,81 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * Copyright (c) 2022 MediaTek Inc. > + * Author: Edward-JW Yang <edward-jw.yang@mediatek.com> > + */ > + > +#ifndef __DRV_CLKFH_H > +#define __DRV_CLKFH_H > + > +#include "clk-pll.h" > + > +struct fh_pll_state {
Please, base as first member.
> + u32 fh_enable; > + u32 ssc_rate; > + void __iomem *base; > +}; > +
Regards, Angelo
| |