]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/commit - drivers/clk/clk.c
clk: Fix race condition between clk_set_parent and clk_enable()
authorSaravana Kannan <skannan@codeaurora.org>
Thu, 16 May 2013 04:07:24 +0000 (21:07 -0700)
committerMike Turquette <mturquette@linaro.org>
Wed, 29 May 2013 05:50:32 +0000 (22:50 -0700)
commitf8aa0bd5c9b28f80bf372d0f486737b13ff01910
tree424c4e9b548df6d7805d3776b48293782168ecd1
parent9807362bfe1748d9bb48eecb9261f1b1aaafea1c
clk: Fix race condition between clk_set_parent and clk_enable()

Without this patch, the following race condition is possible.
* clk-A has two parents - clk-X and clk-Y.
* All three are disabled and clk-X is current parent.
* Thread A: clk_set_parent(clk-A, clk-Y).
* Thread A: <snip execution flow>
* Thread A: Grabs enable lock.
* Thread A: Sees enable count of clk-A is 0, so doesn't enable clk-Y.
* Thread A: Updates clk-A SW parent to clk-Y
* Thread A: Releases enable lock.
* Thread B: clk_enable(clk-A).
* Thread B: clk_enable() enables clk-Y, then enabled clk-A and returns.

clk-A is now enabled in software, but not clocking in hardware since the
hardware parent is still clk-X.

The only way to avoid race conditions between clk_set_parent() and
clk_enable/disable() is to ensure that clk_enable/disable() calls don't
require changes to hardware enable state between changes to software clock
topology and hardware clock topology.

The options to achieve the above are:
1. Grab the enable lock before changing software/hardware topology and
   release it afterwards.
2. Keep the clock enabled for the duration of software/hardware topology
   change so that any additional enable/disable calls don't try to change
   the hardware state. Once the topology change is complete, the clock can
   be put back in its original enable state.

Option (1) is not an acceptable solution since the set_parent() ops might
need to sleep.

Therefore, this patch implements option (2).

This patch doesn't violate any API semantics. clk_disable() doesn't
guarantee that the clock is actually disabled. So, no clients of a clock
can assume that a clock is disabled after their last call to clk_disable().
So, enabling the clock during a parent change is not a violation of any API
semantics.

This also has the nice side effect of simplifying the error handling code.

Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Mike Turquette <mturquette@linaro.org>
[mturquette@linaro.org: fixed up whitespace issue]
drivers/clk/clk.c