Re: [PATCH 9/17] ARM: OMAP2: Add common clock framework for 24xx and 34xx

[Tony Lindgren <tony@atomide.com>]

* Eduardo Valentin <edubezval@gmail.com> [080330 21:49]:
> Hi Tony and Paul,
> 
> Just a little question/warn, these kind of code may not be preempt safe:
> <snip>
> +/* Enables clock without considering parent dependencies or use count
> + * REVISIT: Maybe change this to use clk->enable like on omap1?
> + */
> +int _omap2_clk_enable(struct clk *clk)
> +{
> +	u32 regval32;
> +
> +	if (clk->flags & (ALWAYS_ENABLED | PARENT_CONTROLS_CLOCK))
> +		return 0;
> +
> +	if (clk->enable)
> +		return clk->enable(clk);
> +
> +	if (unlikely(clk->enable_reg == 0)) {
> +		printk(KERN_ERR "clock.c: Enable for %s without enable code\n",
> +		       clk->name);
> +		return 0; /* REVISIT: -EINVAL */
> +	}
> +
> +	regval32 = cm_read_reg(clk->enable_reg);
> +	if (clk->flags & INVERT_ENABLE)
> +		regval32 &= ~(1 << clk->enable_bit);
> +	else
> +		regval32 |= (1 << clk->enable_bit);
> +	cm_write_reg(regval32, clk->enable_reg);
> +	wmb();
> +
> +	omap2_clk_wait_ready(clk);
> +
> +	return 0;
> +}
> <snip>
> 
> Are these registers protected somehow? because this read followed by a write
> may not be a preempt safe operation. Or this code is not designed for
> CONFIG_PREEMPT?

Yeah, it's OK. Both clk24xx.c and clk34xx.c register omap2_clk_enable()
with omap common clock code.

Then omap common clock code calls omap2_clk_enable() with spin_lock_irqsave(),
see clk_enable() in arch/arm/plat-omap/clock.c.

Regards,

Tony




> 
> Cheers,
> 
> Eduardo Valentin
> 
> On Fri, Mar 28, 2008 at 8:38 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Tony Lindgren <tony@atomide.com> [080318 17:04]:
> >
> > > From: Paul Walmsley <paul@pwsan.com>
> >  >
> >  > This patch adds a common clock framework for 24xx and 34xx.
> >  > Note that this patch does not add it to Makefile until in
> >  > next patch. Some functions are modified from earlier 24xx
> >  > clock framework code.
> >
> >  Here's an updated version of this patch. I've folded in two more patches
> >  from Paul [1][2]. Patches change to use __ffs() instead of
> >  mask_to_shift,
> >  and clean up clock register defines.
> >
> >  Regards,
> >
> >  Tony
> >
> >  [1] http://master.kernel.org/git/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commitdiff;h=f417e171cd2d2bd0302c8f420ee97295a165a39c
> >  [2] http://master.kernel.org/git/?p=linux/kernel/git/tmlind/linux-omap-2.6.git;a=commitdiff;h=70d19cc5c7999445b84e6ffea8e025512b877e8c
> >
> 
> 
> 
> -- 
> Eduardo Bezerra Valentin