| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20141117191515.25314.22524@quantum>
Date: Mon, 17 Nov 2014 11:15:15 -0800
From: Mike Turquette <mturquette@...aro.org>
To: James Hogan <james.hogan@...tec.com>, linux-kernel@...r.kernel.org
Cc: linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org,
"James Hogan" <james.hogan@...tec.com>,
"Heiko Stuebner" <heiko@...ech.de>,
"Thomas Abraham" <thomas.ab@...sung.com>,
"Tomasz Figa" <t.figa@...sung.com>,
"Max Schwarz" <max.schwarz@...ine.de>, stable@...r.kernel.org
Subject: Re: [PATCH] clk-divider: Fix READ_ONLY when divider > 1
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@...tec.com>
> Cc: Mike Turquette <mturquette@...aro.org>
> Cc: Heiko Stuebner <heiko@...ech.de>
> Cc: Thomas Abraham <thomas.ab@...sung.com>
> Cc: Tomasz Figa <t.figa@...sung.com>
> Cc: Max Schwarz <max.schwarz@...ine.de>
> Cc: <stable@...r.kernel.org> # 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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists