lkml.org 
[lkml]   [2015]   [May]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2 3/4] clk: stm32: Add clock driver for STM32F4[23]xxx devices
Hi Daniel,

Nice driver, please find my comments below.

Once fixed, you can add:
Reviewed-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>


On 05/30/2015 09:54 AM, Daniel Thompson wrote:
> The driver supports decoding and statically modelling PLL state (i.e.
> we inherit state from bootloader) and provides support for all
> peripherals that support simple one-bit gated clocks. The covers all
> peripherals whose clocks come from the AHB, APB1 or APB2 buses.
>
> It has been tested (for non-regression only) on an STM32F429I-Discovery
> boards. The clock counts for TIM2, USART1 and SYSTICK are all set correctly
> and time and the wall clock looks OK when checked with a stopwatch.
>
> Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
> ---
> drivers/clk/Makefile | 1 +
> drivers/clk/clk-stm32f4.c | 364 ++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 365 insertions(+)
> create mode 100644 drivers/clk/clk-stm32f4.c
...
> diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
> new file mode 100644
> index 0000000..68f9962
> --- /dev/null
> +++ b/drivers/clk/clk-stm32f4.c
> @@ -0,0 +1,364 @@
...
> +/*
> + * Converts the primary and secondary indices (as they appear in DT) to an
> + * offset into our struct clock array.
> + */
> +static unsigned int stm32f4_rcc_lookup_clk_idx(u8 primary, u8 secondary)
> +{
> + u64 table[ARRAY_SIZE(stm32f42xx_gate_map)];
> +
> + if (primary == 1) {
> + BUG_ON(secondary > FCLK);
Maybe the function could return a signed int, an propagate errors
instead of using BUG_ON?
> + return secondary;
> + }
> +
> + memcpy(table, stm32f42xx_gate_map, sizeof(table));
> +
> + /* only bits set in table can be used as indices */
> + BUG_ON(secondary > 8 * sizeof(table) ||
> + 0 == (table[BIT_ULL_WORD(secondary)] & BIT_ULL_MASK(secondary)));
Ditto.
> +
> + /* mask out bits above our current index */
> + table[BIT_ULL_WORD(secondary)] &=
> + GENMASK_ULL(secondary % BITS_PER_LONG_LONG, 0);
> +
> + return FCLK + hweight64(table[0]) +
> + (BIT_ULL_WORD(secondary) >= 1 ? hweight64(table[1]) : 0) +
> + (BIT_ULL_WORD(secondary) >= 2 ? hweight64(table[2]) : 0);
> +}
> +
> +struct clk *stm32f4_rcc_lookup_clk(struct of_phandle_args *clkspec, void *data)
> +{
> + return clks[stm32f4_rcc_lookup_clk_idx(clkspec->args[0],
> + clkspec->args[1])];
If stm32f4_rcc_lookup_clk_idx() returns an error, you could propagate it
using ERR_PTR().
> +}
> +
> +static const char __initdata *sys_parents[] = { "hsi", NULL, "pll" };
> +
> +static struct clk_div_table ahb_div_table[] = {
Should be const.
> + { 0x0, 1 }, { 0x1, 1 }, { 0x2, 1 }, { 0x3, 1 },
> + { 0x4, 1 }, { 0x5, 1 }, { 0x6, 1 }, { 0x7, 1 },
> + { 0x8, 2 }, { 0x9, 4 }, { 0xa, 8 }, { 0xb, 16 },
> + { 0xc, 64 }, { 0xd, 128 }, { 0xe, 256 }, { 0xf, 512 },
> + { 0 },
> +};
> +
> +static struct clk_div_table apb_div_table[] = {
Ditto.
> + { 0, 1 }, { 0, 1 }, { 0, 1 }, { 0, 1 },
> + { 4, 2 }, { 5, 4 }, { 6, 8 }, { 7, 16 },
> + { 0 },
> +};
>

Thanks!
Maxime


\
 
 \ /
  Last update: 2015-05-30 12:01    [W:0.087 / U:3.376 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site