>From 7f62f45d12cec77fd006ac37ab5e8a840274fc0f Mon Sep 17 00:00:00 2001 From: Tero Kristo Date: Tue, 1 Oct 2019 10:56:31 +0300 Subject: [PATCH 3/4] clk: ti: divider: convert to use min,max,mask instead of width The existing width field used to check divider validity does not provide enough protection against bad values. For example, if max divider value is 4, the smallest all-1 bitmask that can hold this value is 7, which allows values higher than 4 to be used. This typically causes unpredictable results with hardware. So far this issue hasn't been noticed as most of the dividers actually have maximum values which fit the whole bitfield, but there are certain clocks for which this is a problem, like dpll4_m4 divider on omap3 devices. Thus, convert the whole validity logic to use min,max and mask values for determining if a specific divider is valid or not. This prevents the odd cases where bad value would otherwise be written to a divider config register. Signed-off-by: Tero Kristo --- drivers/clk/ti/clock.h | 4 +- drivers/clk/ti/divider.c | 151 ++++++++++++++++----------------------- 2 files changed, 64 insertions(+), 91 deletions(-) diff --git a/drivers/clk/ti/clock.h b/drivers/clk/ti/clock.h index f6b6876dfdee..e6995c04001e 100644 --- a/drivers/clk/ti/clock.h +++ b/drivers/clk/ti/clock.h @@ -20,9 +20,11 @@ struct clk_omap_divider { struct clk_hw hw; struct clk_omap_reg reg; u8 shift; - u8 width; u8 flags; s8 latch; + u16 min; + u16 max; + u16 mask; const struct clk_div_table *table; u32 context; }; diff --git a/drivers/clk/ti/divider.c b/drivers/clk/ti/divider.c index 39e890204156..c75cc54772f6 100644 --- a/drivers/clk/ti/divider.c +++ b/drivers/clk/ti/divider.c @@ -26,30 +26,6 @@ #undef pr_fmt #define pr_fmt(fmt) "%s: " fmt, __func__ -#define div_mask(d) ((1 << ((d)->width)) - 1) - -static unsigned int _get_table_maxdiv(const struct clk_div_table *table) -{ - unsigned int maxdiv = 0; - const struct clk_div_table *clkt; - - for (clkt = table; clkt->div; clkt++) - if (clkt->div > maxdiv) - maxdiv = clkt->div; - return maxdiv; -} - -static unsigned int _get_maxdiv(struct clk_omap_divider *divider) -{ - if (divider->flags & CLK_DIVIDER_ONE_BASED) - return div_mask(divider); - if (divider->flags & CLK_DIVIDER_POWER_OF_TWO) - return 1 << div_mask(divider); - if (divider->table) - return _get_table_maxdiv(divider->table); - return div_mask(divider) + 1; -} - static unsigned int _get_table_div(const struct clk_div_table *table, unsigned int val) { @@ -61,6 +37,24 @@ static unsigned int _get_table_div(const struct clk_div_table *table, return 0; } +static void _setup_mask(struct clk_omap_divider *divider) +{ + u16 mask; + u32 max_val; + + max_val = divider->max; + + if (!(divider->flags & CLK_DIVIDER_ONE_BASED)) + max_val--; + + if (divider->flags & CLK_DIVIDER_POWER_OF_TWO) + mask = fls(max_val) - 1; + else + mask = max_val; + + divider->mask = (1 << fls(mask)) - 1; +} + static unsigned int _get_div(struct clk_omap_divider *divider, unsigned int val) { if (divider->flags & CLK_DIVIDER_ONE_BASED) @@ -101,7 +95,7 @@ static unsigned long ti_clk_divider_recalc_rate(struct clk_hw *hw, unsigned int div, val; val = ti_clk_ll_ops->clk_readl(÷r->reg) >> divider->shift; - val &= div_mask(divider); + val &= divider->mask; div = _get_div(divider, val); if (!div) { @@ -180,7 +174,7 @@ static int ti_clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, if (!rate) rate = 1; - maxdiv = _get_maxdiv(divider); + maxdiv = divider->max; if (!(clk_hw_get_flags(hw) & CLK_SET_RATE_PARENT)) { parent_rate = *best_parent_rate; @@ -219,7 +213,7 @@ static int ti_clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate, } if (!bestdiv) { - bestdiv = _get_maxdiv(divider); + bestdiv = divider->max; *best_parent_rate = clk_hw_round_rate(clk_hw_get_parent(hw), 1); } @@ -249,17 +243,16 @@ static int ti_clk_divider_set_rate(struct clk_hw *hw, unsigned long rate, divider = to_clk_omap_divider(hw); div = DIV_ROUND_UP(parent_rate, rate); - value = _get_val(divider, div); - if (value > div_mask(divider)) - value = div_mask(divider); + if (div > divider->max) + div = divider->max; + if (div < divider->min) + div = divider->min; - if (divider->flags & CLK_DIVIDER_HIWORD_MASK) { - val = div_mask(divider) << (divider->shift + 16); - } else { - val = ti_clk_ll_ops->clk_readl(÷r->reg); - val &= ~(div_mask(divider) << divider->shift); - } + value = _get_val(divider, div); + + val = ti_clk_ll_ops->clk_readl(÷r->reg); + val &= ~(divider->mask << divider->shift); val |= value << divider->shift; ti_clk_ll_ops->clk_writel(val, ÷r->reg); @@ -280,7 +273,7 @@ static int clk_divider_save_context(struct clk_hw *hw) u32 val; val = ti_clk_ll_ops->clk_readl(÷r->reg) >> divider->shift; - divider->context = val & div_mask(divider); + divider->context = val & divider->mask; return 0; } @@ -297,7 +290,7 @@ static void clk_divider_restore_context(struct clk_hw *hw) u32 val; val = ti_clk_ll_ops->clk_readl(÷r->reg); - val &= ~(div_mask(divider) << divider->shift); + val &= ~(divider->mask << divider->shift); val |= divider->context << divider->shift; ti_clk_ll_ops->clk_writel(val, ÷r->reg); } @@ -341,29 +334,14 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div, u8 flags, struct clk_omap_divider *divider) { int valid_div = 0; - u32 val; - int div; int i; struct clk_div_table *tmp; + u16 min_div = 0; if (!div_table) { - if (flags & CLKF_INDEX_STARTS_AT_ONE) - val = 1; - else - val = 0; - - div = 1; - - while (div < max_div) { - if (flags & CLKF_INDEX_POWER_OF_TWO) - div <<= 1; - else - div++; - val++; - } - - divider->width = fls(val); - + divider->min = 1; + divider->max = max_div; + _setup_mask(divider); return 0; } @@ -384,17 +362,21 @@ int ti_clk_parse_divider_data(int *div_table, int num_dividers, int max_div, return -ENOMEM; valid_div = 0; - divider->width = 0; for (i = 0; i < num_dividers; i++) if (div_table[i] > 0) { tmp[valid_div].div = div_table[i]; tmp[valid_div].val = i; valid_div++; - divider->width = i; + if (tmp[valid_div].div > max_div) + max_div = tmp[valid_div].div; + if (!min_div || tmp[valid_div].div < min_div) + min_div = tmp[valid_div].div; } - divider->width = fls(divider->width); + divider->min = min_div; + divider->max = max_div; + _setup_mask(divider); divider->table = tmp; return 0; @@ -451,16 +433,15 @@ static int __init ti_clk_get_div_table(struct device_node *node, return 0; } -static int _get_divider_width(struct device_node *node, - const struct clk_div_table *table, - u8 flags) +static int _populate_divider_min_max(struct device_node *node, + struct clk_omap_divider *divider) { - u32 min_div; - u32 max_div; - u32 val = 0; - u32 div; + u32 min_div = 0; + u32 max_div = 0; + u32 val; + const struct clk_div_table *clkt; - if (!table) { + if (!divider->table) { /* Clk divider table not provided, determine min/max divs */ if (of_property_read_u32(node, "ti,min-div", &min_div)) min_div = 1; @@ -469,30 +450,22 @@ static int _get_divider_width(struct device_node *node, pr_err("no max-div for %pOFn!\n", node); return -EINVAL; } - - /* Determine bit width for the field */ - if (flags & CLK_DIVIDER_ONE_BASED) - val = 1; - - div = min_div; - - while (div < max_div) { - if (flags & CLK_DIVIDER_POWER_OF_TWO) - div <<= 1; - else - div++; - val++; - } } else { - div = 0; - while (table[div].div) { - val = table[div].val; - div++; + for (clkt = divider->table; clkt->div; clkt++) { + val = clkt->div; + if (val > max_div) + max_div = val; + if (!min_div || val < min_div) + min_div = val; } } - return fls(val); + divider->min = min_div; + divider->max = max_div; + _setup_mask(divider); + + return 0; } static int __init ti_clk_divider_populate(struct device_node *node, @@ -532,9 +505,7 @@ static int __init ti_clk_divider_populate(struct device_node *node, if (ret) return ret; - div->width = _get_divider_width(node, div->table, div->flags); - - return 0; + return _populate_divider_min_max(node, div); } /** -- 2.17.1