Re: [PATCH] clk-divider: Fix READ_ONLY when divider > 1

From: Mike Turquette
Date: Mon Nov 17 2014 - 14:15:25 EST


Quoting James Hogan (2014-11-14 07:32:09)
> Commit 79c6ab509558 (clk: divider: add CLK_DIVIDER_READ_ONLY flag) in
> v3.16 introduced the CLK_DIVIDER_READ_ONLY flag which caused the
> recalc_rate() and round_rate() clock callbacks to be omitted.
>
> However using this flag has the unfortunate side effect of causing the
> clock recalculation code when a clock rate change is attempted to always
> treat it as a pass-through clock, i.e. with a fixed divide of 1, which
> may not be the case. Child clock rates are then recalculated using the
> wrong parent rate.
>
> Therefore instead of dropping the recalc_rate() and round_rate()
> callbacks, alter clk_divider_bestdiv() to always report the current
> divider as the best divider so that it is never altered.
>
> For me the read only clock was the system clock, which divided the PLL
> rate by 2, from which both the UART and the SPI clocks were divided.
> Initial setting of the UART rate set it correctly, but when the SPI
> clock was set, the other child clocks were miscalculated. The UART clock
> was recalculated using the PLL rate as the parent rate, resulting in a
> UART new_rate of double what it should be, and a UART which spewed forth
> garbage when the rate changes were propagated.

Applied to clk-fixes towards -rc6.

Thanks,
Mike

>
> Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
> Cc: Mike Turquette <mturquette@xxxxxxxxxx>
> Cc: Heiko Stuebner <heiko@xxxxxxxxx>
> Cc: Thomas Abraham <thomas.ab@xxxxxxxxxxx>
> Cc: Tomasz Figa <t.figa@xxxxxxxxxxx>
> Cc: Max Schwarz <max.schwarz@xxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> # v3.16+
> ---
> drivers/clk/clk-divider.c | 18 +++++++++---------
> drivers/clk/rockchip/clk.c | 4 +---
> include/linux/clk-provider.h | 1 -
> 3 files changed, 10 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index 18a9de29df0e..c0a842b335c5 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -263,6 +263,14 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
> if (!rate)
> rate = 1;
>
> + /* if read only, just return current value */
> + if (divider->flags & CLK_DIVIDER_READ_ONLY) {
> + bestdiv = readl(divider->reg) >> divider->shift;
> + bestdiv &= div_mask(divider);
> + bestdiv = _get_div(divider, bestdiv);
> + return bestdiv;
> + }
> +
> maxdiv = _get_maxdiv(divider);
>
> if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT)) {
> @@ -361,11 +369,6 @@ const struct clk_ops clk_divider_ops = {
> };
> EXPORT_SYMBOL_GPL(clk_divider_ops);
>
> -const struct clk_ops clk_divider_ro_ops = {
> - .recalc_rate = clk_divider_recalc_rate,
> -};
> -EXPORT_SYMBOL_GPL(clk_divider_ro_ops);
> -
> static struct clk *_register_divider(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> @@ -391,10 +394,7 @@ static struct clk *_register_divider(struct device *dev, const char *name,
> }
>
> init.name = name;
> - if (clk_divider_flags & CLK_DIVIDER_READ_ONLY)
> - init.ops = &clk_divider_ro_ops;
> - else
> - init.ops = &clk_divider_ops;
> + init.ops = &clk_divider_ops;
> init.flags = flags | CLK_IS_BASIC;
> init.parent_names = (parent_name ? &parent_name: NULL);
> init.num_parents = (parent_name ? 1 : 0);
> diff --git a/drivers/clk/rockchip/clk.c b/drivers/clk/rockchip/clk.c
> index 1e68bff481b8..880a266f0143 100644
> --- a/drivers/clk/rockchip/clk.c
> +++ b/drivers/clk/rockchip/clk.c
> @@ -90,9 +90,7 @@ static struct clk *rockchip_clk_register_branch(const char *name,
> div->width = div_width;
> div->lock = lock;
> div->table = div_table;
> - div_ops = (div_flags & CLK_DIVIDER_READ_ONLY)
> - ? &clk_divider_ro_ops
> - : &clk_divider_ops;
> + div_ops = &clk_divider_ops;
> }
>
> clk = clk_register_composite(NULL, name, parent_names, num_parents,
> diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
> index be21af149f11..2839c639f092 100644
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -352,7 +352,6 @@ struct clk_divider {
> #define CLK_DIVIDER_READ_ONLY BIT(5)
>
> extern const struct clk_ops clk_divider_ops;
> -extern const struct clk_ops clk_divider_ro_ops;
> struct clk *clk_register_divider(struct device *dev, const char *name,
> const char *parent_name, unsigned long flags,
> void __iomem *reg, u8 shift, u8 width,
> --
> 2.0.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/