* [PATCH] Documentation: clk: Add locking documentation
@ 2014-02-26 21:52 Laurent Pinchart
2014-02-26 22:29 ` Russell King - ARM Linux
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2014-02-26 21:52 UTC (permalink / raw)
To: linux-arm-kernel
Briefly documentation the common clock framework locking scheme from a
clock driver point of view.
Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
---
Documentation/clk.txt | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/Documentation/clk.txt b/Documentation/clk.txt
index 699ef2a..4bd6fd7 100644
--- a/Documentation/clk.txt
+++ b/Documentation/clk.txt
@@ -255,3 +255,29 @@ are sorted out.
To bypass this disabling, include "clk_ignore_unused" in the bootargs to the
kernel.
+
+ Part 7 - Locking
+
+The common clock framework uses two global locks. One of them (the enable
+lock) is held across calls to the .enable, .disable and .is_enabled
+operations, while the other (the prepare lock) is held across calls to all other
+operations. This effectively divides operations in two groups from a locking
+perspective.
+
+Drivers don't need to manually protect resources shared between the operations
+of one group, regardless of whether those resources are shared by multiple
+clocks or not. However, access to resources that are shared between operations
+of the two groups needs to be protected by the drivers. An example of such a
+resource would be a register that controls both the clock rate and the clock
+enable/disable state.
+
+The clock framework is reentrant, in that a driver is allowed to call clock
+framework functions from within its implementation of clock operations. This
+can for instance cause a .set_rate operation of one clock being called from
+within the .set_rate operation of another clock. This case must be considered
+in the driver implementations, but the code flow is usually controlled by the
+driver in that case.
+
+Note that locking must also be considered when code outside of the common
+clock framework needs to access resources used by the clock operations. This
+is considered out of scope of this document.
--
Regards,
Laurent Pinchart
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] Documentation: clk: Add locking documentation
2014-02-26 21:52 [PATCH] Documentation: clk: Add locking documentation Laurent Pinchart
@ 2014-02-26 22:29 ` Russell King - ARM Linux
2014-02-26 23:30 ` Mike Turquette
0 siblings, 1 reply; 5+ messages in thread
From: Russell King - ARM Linux @ 2014-02-26 22:29 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Feb 26, 2014 at 10:52:55PM +0100, Laurent Pinchart wrote:
> Briefly documentation the common clock framework locking scheme from a
> clock driver point of view.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> ---
> Documentation/clk.txt | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/Documentation/clk.txt b/Documentation/clk.txt
> index 699ef2a..4bd6fd7 100644
> --- a/Documentation/clk.txt
> +++ b/Documentation/clk.txt
> @@ -255,3 +255,29 @@ are sorted out.
>
> To bypass this disabling, include "clk_ignore_unused" in the bootargs to the
> kernel.
> +
> + Part 7 - Locking
> +
> +The common clock framework uses two global locks. One of them (the enable
> +lock) is held across calls to the .enable, .disable and .is_enabled
> +operations, while the other (the prepare lock) is held across calls to all other
> +operations. This effectively divides operations in two groups from a locking
> +perspective.
It would be a good idea to mention the types of these locks - the fact
that the enable lock is a spinlock, and the prepare lock is a mutex.
That's a very important distinction as there's limitations on the
contexts that the prepare-lock can be called from.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Documentation: clk: Add locking documentation
2014-02-26 22:29 ` Russell King - ARM Linux
@ 2014-02-26 23:30 ` Mike Turquette
2014-02-27 23:07 ` Laurent Pinchart
0 siblings, 1 reply; 5+ messages in thread
From: Mike Turquette @ 2014-02-26 23:30 UTC (permalink / raw)
To: linux-arm-kernel
Quoting Russell King - ARM Linux (2014-02-26 14:29:26)
> On Wed, Feb 26, 2014 at 10:52:55PM +0100, Laurent Pinchart wrote:
> > Briefly documentation the common clock framework locking scheme from a
> > clock driver point of view.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
> > ---
> > Documentation/clk.txt | 26 ++++++++++++++++++++++++++
> > 1 file changed, 26 insertions(+)
> >
> > diff --git a/Documentation/clk.txt b/Documentation/clk.txt
> > index 699ef2a..4bd6fd7 100644
> > --- a/Documentation/clk.txt
> > +++ b/Documentation/clk.txt
> > @@ -255,3 +255,29 @@ are sorted out.
> >
> > To bypass this disabling, include "clk_ignore_unused" in the bootargs to the
> > kernel.
> > +
> > + Part 7 - Locking
> > +
> > +The common clock framework uses two global locks. One of them (the enable
> > +lock) is held across calls to the .enable, .disable and .is_enabled
> > +operations, while the other (the prepare lock) is held across calls to all other
> > +operations. This effectively divides operations in two groups from a locking
> > +perspective.
>
> It would be a good idea to mention the types of these locks - the fact
> that the enable lock is a spinlock, and the prepare lock is a mutex.
> That's a very important distinction as there's limitations on the
> contexts that the prepare-lock can be called from.
I agree with Russell and would take it a step further: this
documentation on locking should reiterate the relationship between
clk_prepare and clk_enable, namely that clk_prepare MUST be called and
MUST complete before calling clk_enable.
This contract helps us deal with the difference in lock types and keep
things sane despite the lack of lock synchronization.
Regards,
Mike
>
> --
> FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
> improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Documentation: clk: Add locking documentation
2014-02-26 23:30 ` Mike Turquette
@ 2014-02-27 23:07 ` Laurent Pinchart
2014-02-27 23:49 ` Russell King - ARM Linux
0 siblings, 1 reply; 5+ messages in thread
From: Laurent Pinchart @ 2014-02-27 23:07 UTC (permalink / raw)
To: linux-arm-kernel
Hi Mike and Russell,
On Wednesday 26 February 2014 15:30:53 Mike Turquette wrote:
> Quoting Russell King - ARM Linux (2014-02-26 14:29:26)
> > On Wed, Feb 26, 2014 at 10:52:55PM +0100, Laurent Pinchart wrote:
> > > Briefly documentation the common clock framework locking scheme from a
> > > clock driver point of view.
> > >
> > > Signed-off-by: Laurent Pinchart
> > > <laurent.pinchart+renesas@ideasonboard.com>
> > > ---
> > >
> > > Documentation/clk.txt | 26 ++++++++++++++++++++++++++
> > > 1 file changed, 26 insertions(+)
> > >
> > > diff --git a/Documentation/clk.txt b/Documentation/clk.txt
> > > index 699ef2a..4bd6fd7 100644
> > > --- a/Documentation/clk.txt
> > > +++ b/Documentation/clk.txt
> > > @@ -255,3 +255,29 @@ are sorted out.
> > >
> > > To bypass this disabling, include "clk_ignore_unused" in the bootargs
> > > to the kernel.
> > >
> > > +
> > > + Part 7 - Locking
> > > +
> > > +The common clock framework uses two global locks. One of them (the
> > > enable
> > > +lock) is held across calls to the .enable, .disable and .is_enabled
> > > +operations, while the other (the prepare lock) is held across calls to
> > > all other +operations. This effectively divides operations in two
> > > groups from a locking +perspective.
> >
> > It would be a good idea to mention the types of these locks - the fact
> > that the enable lock is a spinlock, and the prepare lock is a mutex.
> > That's a very important distinction as there's limitations on the
> > contexts that the prepare-lock can be called from.
>
> I agree with Russell and would take it a step further: this
> documentation on locking should reiterate the relationship between
> clk_prepare and clk_enable, namely that clk_prepare MUST be called and
> MUST complete before calling clk_enable.
>
> This contract helps us deal with the difference in lock types and keep
> things sane despite the lack of lock synchronization.
If I'm not mistaken that document explains CCF from a driver point of view,
not from a user point of view. The clk_enable() and clk_prepare() functions
are only mentioned in one example to describe the internal call stack. While I
totally agree that the enable/prepare relationship and the spinlock/mutex pair
is a very important part of CCF, I believe they would rather belong to a CCF
user API documentation.
What's your opinion on this ?
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Documentation: clk: Add locking documentation
2014-02-27 23:07 ` Laurent Pinchart
@ 2014-02-27 23:49 ` Russell King - ARM Linux
0 siblings, 0 replies; 5+ messages in thread
From: Russell King - ARM Linux @ 2014-02-27 23:49 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Feb 28, 2014 at 12:07:55AM +0100, Laurent Pinchart wrote:
> Hi Mike and Russell,
>
> If I'm not mistaken that document explains CCF from a driver point of view,
> not from a user point of view. The clk_enable() and clk_prepare() functions
> are only mentioned in one example to describe the internal call stack.
> While I totally agree that the enable/prepare relationship and the
> spinlock/mutex pair is a very important part of CCF, I believe they would
> rather belong to a CCF user API documentation.
The type of lock is relevant on both sides of the API. Users need to
know that the prepare/unprepare may sleep during those calls, and
implementations need to know that they can sleep during those calls.
Conversely, with enable/disable, users can see they will never sleep
(because sleeping with a spinlock held is illegal) and implementations
need to know that too.
So, I think there's value in adding that information here.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-02-27 23:49 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-26 21:52 [PATCH] Documentation: clk: Add locking documentation Laurent Pinchart
2014-02-26 22:29 ` Russell King - ARM Linux
2014-02-26 23:30 ` Mike Turquette
2014-02-27 23:07 ` Laurent Pinchart
2014-02-27 23:49 ` Russell King - ARM Linux
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.