| Message ID | 1464095957-25851-3-git-send-email-b.zolnierkie@samsung.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On 05/24/2016 03:19 PM, Bartlomiej Zolnierkiewicz wrote: > Exynos5433 uses different register layout for CPU clock registers > than earlier SoCs so add new code for handling this layout. Also > add new CLK_CPU_HAS_E5433_REGS_LAYOUT flag to request using it. > > There should be no functional change resulting from this patch. > > Cc: Kukjin Kim <kgene@kernel.org> > CC: Krzysztof Kozlowski <k.kozlowski@samsung.com> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/clk/samsung/clk-cpu.c | 131 +++++++++++++++++++++++++++++++++++++++++- > drivers/clk/samsung/clk-cpu.h | 4 +- > 2 files changed, 133 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c > index 813003d..8bf7e80 100644 > --- a/drivers/clk/samsung/clk-cpu.c > +++ b/drivers/clk/samsung/clk-cpu.c > @@ -45,6 +45,13 @@ > #define E4210_DIV_STAT_CPU0 0x400 > #define E4210_DIV_STAT_CPU1 0x404 > > +#define E5433_MUX_SEL2 0x008 > +#define E5433_MUX_STAT2 0x208 > +#define E5433_DIV_CPU0 0x400 > +#define E5433_DIV_CPU1 0x404 > +#define E5433_DIV_STAT_CPU0 0x500 > +#define E5433_DIV_STAT_CPU1 0x504 I got a problem matching it with datasheed. The base is 0x200? > + > #define E4210_DIV0_RATIO0_MASK 0x7 > #define E4210_DIV1_HPM_MASK (0x7 << 4) > #define E4210_DIV1_COPY_MASK (0x7 << 0) > @@ -253,6 +260,102 @@ static int exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata, > } > > /* > + * Helper function to set the 'safe' dividers for the CPU clock. The parameters > + * div and mask contain the divider value and the register bit mask of the > + * dividers to be programmed. > + */ > +static void exynos5433_set_safe_div(void __iomem *base, unsigned long div, > + unsigned long mask) Please align the argument. > +{ > + unsigned long div0; > + > + div0 = readl(base + E5433_DIV_CPU0); > + div0 = (div0 & ~mask) | (div & mask); > + writel(div0, base + E5433_DIV_CPU0); > + wait_until_divider_stable(base + E5433_DIV_STAT_CPU0, mask); > +} > + > +/* handler for pre-rate change notification from parent clock */ > +static int exynos5433_cpuclk_pre_rate_change(struct clk_notifier_data *ndata, > + struct exynos_cpuclk *cpuclk, void __iomem *base) > +{ > + const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg; > + unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent); > + unsigned long alt_div = 0, alt_div_mask = DIV_MASK; > + unsigned long div0, div1 = 0, mux_reg; > + unsigned long flags; > + > + /* find out the divider values to use for clock data */ > + while ((cfg_data->prate * 1000) != ndata->new_rate) { > + if (cfg_data->prate == 0) > + return -EINVAL; > + cfg_data++; > + } > + > + spin_lock_irqsave(cpuclk->lock, flags); > + > + /* > + * For the selected PLL clock frequency, get the pre-defined divider > + * values. > + */ > + div0 = cfg_data->div0; > + div1 = cfg_data->div1; > + > + /* > + * If the old parent clock speed is less than the clock speed of > + * the alternate parent, then it should be ensured that at no point > + * the armclk speed is more than the old_prate until the dividers are > + * set. Also workaround the issue of the dividers being set to lower > + * values before the parent clock speed is set to new lower speed > + * (this can result in too high speed of armclk output clocks). In entire sentence: s/speed/rate/ > + */ > + if (alt_prate > ndata->old_rate || ndata->old_rate > ndata->new_rate) { > + unsigned long tmp_rate = min(ndata->old_rate, ndata->new_rate); > + > + alt_div = DIV_ROUND_UP(alt_prate, tmp_rate) - 1; > + WARN_ON(alt_div >= MAX_DIV); > + > + exynos5433_set_safe_div(base, alt_div, alt_div_mask); > + div0 |= alt_div; > + } > + > + /* select the alternate parent */ > + mux_reg = readl(base + E5433_MUX_SEL2); > + writel(mux_reg | 1, base + E5433_MUX_SEL2); > + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 2); > + > + /* alternate parent is active now. set the dividers */ > + writel(div0, base + E5433_DIV_CPU0); > + wait_until_divider_stable(base + E5433_DIV_STAT_CPU0, DIV_MASK_ALL); > + > + writel(div1, base + E5433_DIV_CPU1); > + wait_until_divider_stable(base + E5433_DIV_STAT_CPU1, DIV_MASK_ALL); > + > + spin_unlock_irqrestore(cpuclk->lock, flags); One blank line please. > + return 0; > +} > + > +/* handler for post-rate change notification from parent clock */ > +static int exynos5433_cpuclk_post_rate_change(struct clk_notifier_data *ndata, > + struct exynos_cpuclk *cpuclk, void __iomem *base) > +{ > + unsigned long div = 0, div_mask = DIV_MASK; > + unsigned long mux_reg; > + unsigned long flags; > + > + spin_lock_irqsave(cpuclk->lock, flags); > + > + /* select apll as the alternate parent */ > + mux_reg = readl(base + E5433_MUX_SEL2); > + writel(mux_reg & ~1, base + E5433_MUX_SEL2); > + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 1); > + > + exynos5433_set_safe_div(base, div, div_mask); > + spin_unlock_irqrestore(cpuclk->lock, flags); One blank line please. > + return 0; > +} > + > +/* > * This notifier function is called for the pre-rate and post-rate change > * notifications of the parent clock of cpuclk. > */ > @@ -275,6 +378,29 @@ static int exynos_cpuclk_notifier_cb(struct notifier_block *nb, > return notifier_from_errno(err); > } > > +/* > + * This notifier function is called for the pre-rate and post-rate change > + * notifications of the parent clock of cpuclk. > + */ > +static int exynos5433_cpuclk_notifier_cb(struct notifier_block *nb, > + unsigned long event, void *data) > +{ > + struct clk_notifier_data *ndata = data; > + struct exynos_cpuclk *cpuclk; > + void __iomem *base; > + int err = 0; > + > + cpuclk = container_of(nb, struct exynos_cpuclk, clk_nb); > + base = cpuclk->ctrl_base; > + > + if (event == PRE_RATE_CHANGE) > + err = exynos5433_cpuclk_pre_rate_change(ndata, cpuclk, base); > + else if (event == POST_RATE_CHANGE) > + err = exynos5433_cpuclk_post_rate_change(ndata, cpuclk, base); > + > + return notifier_from_errno(err); > +} Entire function is a duplication of exynos_cpuclk_notifier_cb(), except the {pre,post}_rate_change. How about checking the flags here and using only one notifier? Overall the code should be smaller... Another, more robust solution would be to define new members in: struct exynos_cpuclk { int (pre_rate_change) *(....); int (post_rate_change) *(....); } This way the notifier_cb would be exactly the same for all implementations. Best regards, Krzysztof > + > /* helper function to register a CPU clock */ > int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > unsigned int lookup_id, const char *name, const char *parent, > @@ -301,7 +427,10 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > cpuclk->ctrl_base = ctx->reg_base + offset; > cpuclk->lock = &ctx->lock; > cpuclk->flags = flags; > - cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; > + if (flags & CLK_CPU_HAS_E5433_REGS_LAYOUT) > + cpuclk->clk_nb.notifier_call = exynos5433_cpuclk_notifier_cb; > + else > + cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; > > cpuclk->alt_parent = __clk_lookup(alt_parent); > if (!cpuclk->alt_parent) { > diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h > index 37874d3..d4b6b51 100644 > --- a/drivers/clk/samsung/clk-cpu.h > +++ b/drivers/clk/samsung/clk-cpu.h > @@ -57,10 +57,12 @@ struct exynos_cpuclk { > struct notifier_block clk_nb; > unsigned long flags; > > -/* The CPU clock registers has DIV1 configuration register */ > +/* The CPU clock registers have DIV1 configuration register */ > #define CLK_CPU_HAS_DIV1 (1 << 0) > /* When ALT parent is active, debug clocks need safe divider values */ > #define CLK_CPU_NEEDS_DEBUG_ALT_DIV (1 << 1) > +/* The CPU clock registers have Exynos5433-compatible layout */ > +#define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) > }; > > extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bart, 2016-05-24 22:19 GMT+09:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>: > Exynos5433 uses different register layout for CPU clock registers > than earlier SoCs so add new code for handling this layout. Also > add new CLK_CPU_HAS_E5433_REGS_LAYOUT flag to request using it. Have you considered abstracting this? Comparing existing code with newly added one, the differences don't really seem to be that huge and it looks like there is more common code than different, except maybe some numeric constants that could be put into a struct. Best regards, Tomasz -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Saturday, June 18, 2016 11:53:36 PM Tomasz Figa wrote: > Hi Bart, Hi Tomek, > 2016-05-24 22:19 GMT+09:00 Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>: > > Exynos5433 uses different register layout for CPU clock registers > > than earlier SoCs so add new code for handling this layout. Also > > add new CLK_CPU_HAS_E5433_REGS_LAYOUT flag to request using it. > > Have you considered abstracting this? Comparing existing code with Yes, I have considered abstracting this but I've decided that there is sufficient number of differences to justify adding new code. > newly added one, the differences don't really seem to be that huge and > it looks like there is more common code than different, except maybe > some numeric constants that could be put into a struct. It is not only that, HPM clock sourcing checking would also need to be abstracted somehow. Anyway there is not a lot of new code and it so much easier to follow by not being shared that I would prefer to keep it that way. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/clk/samsung/clk-cpu.c b/drivers/clk/samsung/clk-cpu.c index 813003d..8bf7e80 100644 --- a/drivers/clk/samsung/clk-cpu.c +++ b/drivers/clk/samsung/clk-cpu.c @@ -45,6 +45,13 @@ #define E4210_DIV_STAT_CPU0 0x400 #define E4210_DIV_STAT_CPU1 0x404 +#define E5433_MUX_SEL2 0x008 +#define E5433_MUX_STAT2 0x208 +#define E5433_DIV_CPU0 0x400 +#define E5433_DIV_CPU1 0x404 +#define E5433_DIV_STAT_CPU0 0x500 +#define E5433_DIV_STAT_CPU1 0x504 + #define E4210_DIV0_RATIO0_MASK 0x7 #define E4210_DIV1_HPM_MASK (0x7 << 4) #define E4210_DIV1_COPY_MASK (0x7 << 0) @@ -253,6 +260,102 @@ static int exynos_cpuclk_post_rate_change(struct clk_notifier_data *ndata, } /* + * Helper function to set the 'safe' dividers for the CPU clock. The parameters + * div and mask contain the divider value and the register bit mask of the + * dividers to be programmed. + */ +static void exynos5433_set_safe_div(void __iomem *base, unsigned long div, + unsigned long mask) +{ + unsigned long div0; + + div0 = readl(base + E5433_DIV_CPU0); + div0 = (div0 & ~mask) | (div & mask); + writel(div0, base + E5433_DIV_CPU0); + wait_until_divider_stable(base + E5433_DIV_STAT_CPU0, mask); +} + +/* handler for pre-rate change notification from parent clock */ +static int exynos5433_cpuclk_pre_rate_change(struct clk_notifier_data *ndata, + struct exynos_cpuclk *cpuclk, void __iomem *base) +{ + const struct exynos_cpuclk_cfg_data *cfg_data = cpuclk->cfg; + unsigned long alt_prate = clk_get_rate(cpuclk->alt_parent); + unsigned long alt_div = 0, alt_div_mask = DIV_MASK; + unsigned long div0, div1 = 0, mux_reg; + unsigned long flags; + + /* find out the divider values to use for clock data */ + while ((cfg_data->prate * 1000) != ndata->new_rate) { + if (cfg_data->prate == 0) + return -EINVAL; + cfg_data++; + } + + spin_lock_irqsave(cpuclk->lock, flags); + + /* + * For the selected PLL clock frequency, get the pre-defined divider + * values. + */ + div0 = cfg_data->div0; + div1 = cfg_data->div1; + + /* + * If the old parent clock speed is less than the clock speed of + * the alternate parent, then it should be ensured that at no point + * the armclk speed is more than the old_prate until the dividers are + * set. Also workaround the issue of the dividers being set to lower + * values before the parent clock speed is set to new lower speed + * (this can result in too high speed of armclk output clocks). + */ + if (alt_prate > ndata->old_rate || ndata->old_rate > ndata->new_rate) { + unsigned long tmp_rate = min(ndata->old_rate, ndata->new_rate); + + alt_div = DIV_ROUND_UP(alt_prate, tmp_rate) - 1; + WARN_ON(alt_div >= MAX_DIV); + + exynos5433_set_safe_div(base, alt_div, alt_div_mask); + div0 |= alt_div; + } + + /* select the alternate parent */ + mux_reg = readl(base + E5433_MUX_SEL2); + writel(mux_reg | 1, base + E5433_MUX_SEL2); + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 2); + + /* alternate parent is active now. set the dividers */ + writel(div0, base + E5433_DIV_CPU0); + wait_until_divider_stable(base + E5433_DIV_STAT_CPU0, DIV_MASK_ALL); + + writel(div1, base + E5433_DIV_CPU1); + wait_until_divider_stable(base + E5433_DIV_STAT_CPU1, DIV_MASK_ALL); + + spin_unlock_irqrestore(cpuclk->lock, flags); + return 0; +} + +/* handler for post-rate change notification from parent clock */ +static int exynos5433_cpuclk_post_rate_change(struct clk_notifier_data *ndata, + struct exynos_cpuclk *cpuclk, void __iomem *base) +{ + unsigned long div = 0, div_mask = DIV_MASK; + unsigned long mux_reg; + unsigned long flags; + + spin_lock_irqsave(cpuclk->lock, flags); + + /* select apll as the alternate parent */ + mux_reg = readl(base + E5433_MUX_SEL2); + writel(mux_reg & ~1, base + E5433_MUX_SEL2); + wait_until_mux_stable(base + E5433_MUX_STAT2, 0, 1); + + exynos5433_set_safe_div(base, div, div_mask); + spin_unlock_irqrestore(cpuclk->lock, flags); + return 0; +} + +/* * This notifier function is called for the pre-rate and post-rate change * notifications of the parent clock of cpuclk. */ @@ -275,6 +378,29 @@ static int exynos_cpuclk_notifier_cb(struct notifier_block *nb, return notifier_from_errno(err); } +/* + * This notifier function is called for the pre-rate and post-rate change + * notifications of the parent clock of cpuclk. + */ +static int exynos5433_cpuclk_notifier_cb(struct notifier_block *nb, + unsigned long event, void *data) +{ + struct clk_notifier_data *ndata = data; + struct exynos_cpuclk *cpuclk; + void __iomem *base; + int err = 0; + + cpuclk = container_of(nb, struct exynos_cpuclk, clk_nb); + base = cpuclk->ctrl_base; + + if (event == PRE_RATE_CHANGE) + err = exynos5433_cpuclk_pre_rate_change(ndata, cpuclk, base); + else if (event == POST_RATE_CHANGE) + err = exynos5433_cpuclk_post_rate_change(ndata, cpuclk, base); + + return notifier_from_errno(err); +} + /* helper function to register a CPU clock */ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, unsigned int lookup_id, const char *name, const char *parent, @@ -301,7 +427,10 @@ int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx, cpuclk->ctrl_base = ctx->reg_base + offset; cpuclk->lock = &ctx->lock; cpuclk->flags = flags; - cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; + if (flags & CLK_CPU_HAS_E5433_REGS_LAYOUT) + cpuclk->clk_nb.notifier_call = exynos5433_cpuclk_notifier_cb; + else + cpuclk->clk_nb.notifier_call = exynos_cpuclk_notifier_cb; cpuclk->alt_parent = __clk_lookup(alt_parent); if (!cpuclk->alt_parent) { diff --git a/drivers/clk/samsung/clk-cpu.h b/drivers/clk/samsung/clk-cpu.h index 37874d3..d4b6b51 100644 --- a/drivers/clk/samsung/clk-cpu.h +++ b/drivers/clk/samsung/clk-cpu.h @@ -57,10 +57,12 @@ struct exynos_cpuclk { struct notifier_block clk_nb; unsigned long flags; -/* The CPU clock registers has DIV1 configuration register */ +/* The CPU clock registers have DIV1 configuration register */ #define CLK_CPU_HAS_DIV1 (1 << 0) /* When ALT parent is active, debug clocks need safe divider values */ #define CLK_CPU_NEEDS_DEBUG_ALT_DIV (1 << 1) +/* The CPU clock registers have Exynos5433-compatible layout */ +#define CLK_CPU_HAS_E5433_REGS_LAYOUT (1 << 2) }; extern int __init exynos_register_cpu_clock(struct samsung_clk_provider *ctx,
Exynos5433 uses different register layout for CPU clock registers than earlier SoCs so add new code for handling this layout. Also add new CLK_CPU_HAS_E5433_REGS_LAYOUT flag to request using it. There should be no functional change resulting from this patch. Cc: Kukjin Kim <kgene@kernel.org> CC: Krzysztof Kozlowski <k.kozlowski@samsung.com> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/clk/samsung/clk-cpu.c | 131 +++++++++++++++++++++++++++++++++++++++++- drivers/clk/samsung/clk-cpu.h | 4 +- 2 files changed, 133 insertions(+), 2 deletions(-)