On 11/02/11 12:45, Russell King - ARM Linux wrote:

On Wed, Nov 02, 2011 at 11:35:59AM -0700, David Brown wrote:

quoted

If the array of clk_lookups contains aliases for the same struct
clk, msm_clock_init() will add the clock to the clocks list
twice.  This would cause list corruption so let's just remove the
clocks list and any associated code and iterate over the array
instead.

Hmm...

quoted

@@ -158,13 +152,13 @@ void __init msm_clock_init(struct clk_lookup *clock_tbl, unsigned num_clocks)
  */
 static int __init clock_late_init(void)
 {
+	unsigned i, count = 0;
 	unsigned long flags;
-	struct clk *clk;
-	unsigned count = 0;
 
 	clock_debug_init();
-	mutex_lock(&clocks_mutex);
-	list_for_each_entry(clk, &clocks, list) {
+	for (i = 0; i < msm_num_clocks; i++) {
+		struct clk *clk = msm_clocks[i].clk;
+
 		clock_debug_add(clk);

This means you'll end up calling clock_debug_add() twice for the same
struct clk - this sounds like a bad idea in itself.  It looks like
there's no protection within that function against it being called
twice with the same struct clk.

Are you sure this is safe?

This hasn't proven to be a problem so far because debugfs returns an
error when you create a directory with the same name twice. If we ever
do something more in clock_debug_add() we would have a problem.

-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.