Commit e8b364b88cc4001b21c28c1ecf1e1e3ffbe162e6

Authored by Rafael J. Wysocki
1 parent f9d81f61c8

PM / Clocks: Do not acquire a mutex under a spinlock

Commit b7ab83e (PM: Use spinlock instead of mutex in clock
management functions) introduced a regression causing clocks_mutex
to be acquired under a spinlock.  This happens because
pm_clk_suspend() and pm_clk_resume() call pm_clk_acquire() under
pcd->lock, but pm_clk_acquire() executes clk_get() which causes
clocks_mutex to be acquired.  Similarly, __pm_clk_remove(),
executed under pcd->lock, calls clk_put(), which also causes
clocks_mutex to be acquired.

To fix those problems make pm_clk_add() call pm_clk_acquire(), so
that pm_clk_suspend() and pm_clk_resume() don't have to do that.
Change pm_clk_remove() and pm_clk_destroy() to separate
modifications of the pcd->clock_list list from the actual removal of
PM clock entry objects done by __pm_clk_remove().

Reported-and-tested-by: Guennadi Liakhovetski <g.liakhovetski@gmx.de>
Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Russell King <rmk+kernel@arm.linux.org.uk>

Showing 1 changed file with 38 additions and 37 deletions Side-by-side Diff

drivers/base/power/clock_ops.c
... ... @@ -42,6 +42,22 @@
42 42 }
43 43  
44 44 /**
  45 + * pm_clk_acquire - Acquire a device clock.
  46 + * @dev: Device whose clock is to be acquired.
  47 + * @ce: PM clock entry corresponding to the clock.
  48 + */
  49 +static void pm_clk_acquire(struct device *dev, struct pm_clock_entry *ce)
  50 +{
  51 + ce->clk = clk_get(dev, ce->con_id);
  52 + if (IS_ERR(ce->clk)) {
  53 + ce->status = PCE_STATUS_ERROR;
  54 + } else {
  55 + ce->status = PCE_STATUS_ACQUIRED;
  56 + dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id);
  57 + }
  58 +}
  59 +
  60 +/**
45 61 * pm_clk_add - Start using a device clock for power management.
46 62 * @dev: Device whose clock is going to be used for power management.
47 63 * @con_id: Connection ID of the clock.
... ... @@ -73,6 +89,8 @@
73 89 }
74 90 }
75 91  
  92 + pm_clk_acquire(dev, ce);
  93 +
76 94 spin_lock_irq(&pcd->lock);
77 95 list_add_tail(&ce->node, &pcd->clock_list);
78 96 spin_unlock_irq(&pcd->lock);
79 97  
... ... @@ -82,17 +100,12 @@
82 100 /**
83 101 * __pm_clk_remove - Destroy PM clock entry.
84 102 * @ce: PM clock entry to destroy.
85   - *
86   - * This routine must be called under the spinlock protecting the PM list of
87   - * clocks corresponding the the @ce's device.
88 103 */
89 104 static void __pm_clk_remove(struct pm_clock_entry *ce)
90 105 {
91 106 if (!ce)
92 107 return;
93 108  
94   - list_del(&ce->node);
95   -
96 109 if (ce->status < PCE_STATUS_ERROR) {
97 110 if (ce->status == PCE_STATUS_ENABLED)
98 111 clk_disable(ce->clk);
99 112  
100 113  
... ... @@ -126,18 +139,22 @@
126 139 spin_lock_irq(&pcd->lock);
127 140  
128 141 list_for_each_entry(ce, &pcd->clock_list, node) {
129   - if (!con_id && !ce->con_id) {
130   - __pm_clk_remove(ce);
131   - break;
132   - } else if (!con_id || !ce->con_id) {
  142 + if (!con_id && !ce->con_id)
  143 + goto remove;
  144 + else if (!con_id || !ce->con_id)
133 145 continue;
134   - } else if (!strcmp(con_id, ce->con_id)) {
135   - __pm_clk_remove(ce);
136   - break;
137   - }
  146 + else if (!strcmp(con_id, ce->con_id))
  147 + goto remove;
138 148 }
139 149  
140 150 spin_unlock_irq(&pcd->lock);
  151 + return;
  152 +
  153 + remove:
  154 + list_del(&ce->node);
  155 + spin_unlock_irq(&pcd->lock);
  156 +
  157 + __pm_clk_remove(ce);
141 158 }
142 159  
143 160 /**
144 161  
145 162  
146 163  
... ... @@ -175,20 +192,27 @@
175 192 {
176 193 struct pm_clk_data *pcd = __to_pcd(dev);
177 194 struct pm_clock_entry *ce, *c;
  195 + struct list_head list;
178 196  
179 197 if (!pcd)
180 198 return;
181 199  
182 200 dev->power.subsys_data = NULL;
  201 + INIT_LIST_HEAD(&list);
183 202  
184 203 spin_lock_irq(&pcd->lock);
185 204  
186 205 list_for_each_entry_safe_reverse(ce, c, &pcd->clock_list, node)
187   - __pm_clk_remove(ce);
  206 + list_move(&ce->node, &list);
188 207  
189 208 spin_unlock_irq(&pcd->lock);
190 209  
191 210 kfree(pcd);
  211 +
  212 + list_for_each_entry_safe_reverse(ce, c, &list, node) {
  213 + list_del(&ce->node);
  214 + __pm_clk_remove(ce);
  215 + }
192 216 }
193 217  
194 218 #endif /* CONFIG_PM */
... ... @@ -196,23 +220,6 @@
196 220 #ifdef CONFIG_PM_RUNTIME
197 221  
198 222 /**
199   - * pm_clk_acquire - Acquire a device clock.
200   - * @dev: Device whose clock is to be acquired.
201   - * @con_id: Connection ID of the clock.
202   - */
203   -static void pm_clk_acquire(struct device *dev,
204   - struct pm_clock_entry *ce)
205   -{
206   - ce->clk = clk_get(dev, ce->con_id);
207   - if (IS_ERR(ce->clk)) {
208   - ce->status = PCE_STATUS_ERROR;
209   - } else {
210   - ce->status = PCE_STATUS_ACQUIRED;
211   - dev_dbg(dev, "Clock %s managed by runtime PM.\n", ce->con_id);
212   - }
213   -}
214   -
215   -/**
216 223 * pm_clk_suspend - Disable clocks in a device's PM clock list.
217 224 * @dev: Device to disable the clocks for.
218 225 */
... ... @@ -230,9 +237,6 @@
230 237 spin_lock_irqsave(&pcd->lock, flags);
231 238  
232 239 list_for_each_entry_reverse(ce, &pcd->clock_list, node) {
233   - if (ce->status == PCE_STATUS_NONE)
234   - pm_clk_acquire(dev, ce);
235   -
236 240 if (ce->status < PCE_STATUS_ERROR) {
237 241 clk_disable(ce->clk);
238 242 ce->status = PCE_STATUS_ACQUIRED;
... ... @@ -262,9 +266,6 @@
262 266 spin_lock_irqsave(&pcd->lock, flags);
263 267  
264 268 list_for_each_entry(ce, &pcd->clock_list, node) {
265   - if (ce->status == PCE_STATUS_NONE)
266   - pm_clk_acquire(dev, ce);
267   -
268 269 if (ce->status < PCE_STATUS_ERROR) {
269 270 clk_enable(ce->clk);
270 271 ce->status = PCE_STATUS_ENABLED;