[PATCH V2 1/6] SPEAr: clk: Add VCO-PLL Synthesizer clock

Turquette, Mike mturquette at ti.com
Mon Apr 23 15:31:35 EDT 2012


Hi Viresh,

Sent my first email too quickly.  A couple of small questions below.

On Thu, Apr 19, 2012 at 9:28 AM, Viresh Kumar <viresh.linux at gmail.com> wrote:
> diff --git a/drivers/clk/spear/clk.h b/drivers/clk/spear/clk.h
> new file mode 100644
> index 0000000..a66024e
> --- /dev/null
> +++ b/drivers/clk/spear/clk.h
> @@ -0,0 +1,76 @@
> +/*
> + * Clock framework definitions for SPEAr platform
> + *
> + * Copyright (C) 2012 ST Microelectronics
> + * Viresh Kumar <viresh.kumar at st.com>
> + *
> + * This file is licensed under the terms of the GNU General Public
> + * License version 2. This program is licensed "as is" without any
> + * warranty of any kind, whether express or implied.
> + */
> +
> +#ifndef __PLAT_CLOCK_H
> +#define __PLAT_CLOCK_H

Any reason to use __PLAT_CLOCK_H from drivers/clk/ now?  Is it
copy/paste from your old platform code?

...

> +static inline long clk_round_rate_index(struct clk_hw *hw, unsigned long drate,
> +               clk_calc_rate calc_rate, u8 rtbl_cnt, int *index)
> +{
> +       unsigned long prev_rate, rate = 0;
> +       unsigned long parent_rate = __clk_get_rate(__clk_get_parent(hw->clk));
> +
> +       for (*index = 0; *index < rtbl_cnt; (*index)++) {
> +               prev_rate = rate;
> +               rate = calc_rate(hw, parent_rate, *index);
> +               if (drate < rate) {
> +                       /* previous clock was best */
> +                       if (*index) {
> +                               rate = prev_rate;
> +                               (*index)--;
> +                       }
> +                       break;
> +               }
> +       }
> +
> +       return rate;
> +}

Any reason for this code to exist in a header?  I prefer code in C
files.  Also it is a bit large for an inline function.

Thanks,
Mike



More information about the linux-arm-kernel mailing list