Re: [PATCH 1/4] clk: Add a generic clock infrastructure

[viresh kumar <viresh.kumar@st.com>]

On 05/20/2011 12:57 PM, Jeremy Kerr wrote:
> +static DEFINE_SPINLOCK(enable_lock);
> +static DEFINE_MUTEX(prepare_lock);

Probably all clocks can be handled separately, i.e. single lock for all
of them will make system slower. Suppose i want to enable UART's clock
then why should spi code be waiting for the lock.
So, we should have per clk lock.

<...>

> +struct clk *clk_register(struct clk_hw_ops *ops, struct clk_hw *hw,
> +               const char *name)
> +{
> +       struct clk *clk;
> +
> +       clk = kzalloc(sizeof(*clk), GFP_KERNEL);
> +       if (!clk)
> +               return NULL;
> +
> +       clk->name = name;
> +       clk->ops = ops;
> +       clk->hw = hw;
> +       hw->clk = clk;
> +
> +       /* Query the hardware for parent and initial rate */
> +

Can remove this blank line.

> +       if (clk->ops->get_parent)
> +               /* We don't to lock against prepare/enable here, as
> +                * the clock is not yet accessible from anywhere */

Shouldn't we use following style for multi-line comments.
/*
 * ....
 */

> +               clk->parent = clk->ops->get_parent(clk->hw);
> +
> +       if (clk->ops->recalc_rate)
> +               clk->rate = clk->ops->recalc_rate(clk->hw);
> +
> +

Can remove one of these blank lines.

-- 
viresh