lkml.org 
[lkml]   [2012]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v6 3/3] clk: basic clock hardware types
On 03/12/2012 03:58 PM, Turquette, Mike wrote:
> On Mon, Mar 12, 2012 at 1:18 PM, Rob Herring <robherring2@gmail.com> wrote:
>> On 03/10/2012 01:54 AM, Mike Turquette wrote:
>>> Many platforms support simple gateable clocks, fixed-rate clocks,
>>> adjustable divider clocks and multi-parent multiplexer clocks.
>>>
>>> This patch introduces basic clock types for the above-mentioned hardware
>>> which share some common characteristics.
>>>
>>> Based on original work by Jeremy Kerr and contribution by Jamie Iles.
>>> Dividers and multiplexor clocks originally contributed by Richard Zhao &
>>> Sascha Hauer.
>>>

snip

>>> +static unsigned long clk_divider_recalc_rate(struct clk_hw *hw,
>>> + unsigned long parent_rate)
>>> +{
>>> + struct clk_divider *divider = to_clk_divider(hw);
>>> + unsigned int div;
>>> + unsigned long flags = 0;
>>> +
>>> + if (divider->lock)
>>> + spin_lock_irqsave(divider->lock, flags);
>>> +
>>> + div = readl(divider->reg) >> divider->shift;
>>> + div &= (1 << divider->width) - 1;
>>> +
>>> + if (divider->lock)
>>> + spin_unlock_irqrestore(divider->lock, flags);
>>
>> What are you locking against? You are only reading the register.
>
> Hi Rob,
>
> These register-level locks originally came in from the divider &
> multiplexer patches from Richard and Sascha and I'm sure they can give
> you more details than I.
>
> Basically on their platform they have some 32-bits regs that have a
> lot of overlapping clock functions in them, such as enable/disable and
> adjusting a divider all in one reg. Those operations are protected by
> different locks (enable spinlock and prepare mutex, respectively) so
> some synchronization mechanism is necessary. On OMAP we don't use
> this since we have a billion registers that typically only map to one
> clock each. I also wonder if having device type memory for the
> affected regions makes a this irrelevant on ARM... but that wouldn't
> matter for non-ARM architectures. Just a thought.
>

In fact, I pointed out that locking for a register access was needed in
an early version of this series as well. However, locking is only needed
if you are doing a read-mod-write on a field in a shared register or
reading from multiple registers. It makes no sense if you are *only*
reading a single shared register as is the case here. That's already an
atomic operation.

Rob



\
 
 \ /
  Last update: 2012-03-12 23:03    [W:0.040 / U:0.836 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site