OMAP: Fix McBSP spin_lock deadlock.

A spin_lock deadlock will occur when omap_mcbsp_request() is invoked.

omap_mcbsp_request()         -->
clk_enable(mcbsp->clk)       --> clk_enable get clockfw_lock, then call ->
omap2_clk_enable()           -->
_omap2_clk_enable()          -->
omap_mcbsp_clk_enable()      -->
clk_enable(mcbsp_ick)        --> now clk_enable acquire clockfw_lock again.

The solution:

If a alias clock has some relative clocks, enable relative clocks before enable the alias clock.

Signed-off-by Stanley.Miao <[email protected]>

---
 arch/arm/mach-omap1/mcbsp.c             |   67 ++++++------------------
 arch/arm/mach-omap2/mcbsp.c             |   88 +++++++-----------------------
 arch/arm/plat-omap/clock.c              |    9 +++-
 arch/arm/plat-omap/include/mach/clock.h |    2 +
 4 files changed, 47 insertions(+), 119 deletions(-)

arch/arm/mach-omap1/mcbsp.c +16 -51

index 7de7c69..cf220be 100644
@@ -26,80 +26,45 @@
 #define DPS_RSTCT2_PER_EN	(1 << 0)
 #define DSP_RSTCT2_WD_PER_EN	(1 << 1)
 
-struct mcbsp_internal_clk {
-	struct clk clk;
-	struct clk **childs;
-	int n_childs;
-};
-
 #if defined(CONFIG_ARCH_OMAP15XX) || defined(CONFIG_ARCH_OMAP16XX)
-static void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static void omap_mcbsp_clk_init(struct clk *mclk)
 {
 	const char *clk_names[] = { "dsp_ck", "api_ck", "dspxor_ck" };
 	int i;
 
-	mclk->n_childs = ARRAY_SIZE(clk_names);
-	mclk->childs = kzalloc(mclk->n_childs * sizeof(struct clk *),
+	mclk->n_relatives = ARRAY_SIZE(clk_names);
+	mclk->relatives = kzalloc(mclk->n_relatives * sizeof(struct clk *),
 				GFP_KERNEL);
 
-	for (i = 0; i < mclk->n_childs; i++) {
+	for (i = 0; i < mclk->n_relatives; i++) {
 		/* We fake a platform device to get correct device id */
 		struct platform_device pdev;
 
 		pdev.dev.bus = &platform_bus_type;
-		pdev.id = mclk->clk.id;
-		mclk->childs[i] = clk_get(&pdev.dev, clk_names[i]);
-		if (IS_ERR(mclk->childs[i]))
+		pdev.id = mclk->id;
+		mclk->relatives[i] = clk_get(&pdev.dev, clk_names[i]);
+		if (IS_ERR(mclk->relatives[i]))
 			printk(KERN_ERR "Could not get clock %s (%d).\n",
-				clk_names[i], mclk->clk.id);
+				clk_names[i], mclk->id);
 	}
 }
 
-static int omap_mcbsp_clk_enable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_enable(mclk->childs[i]);
-	return 0;
-}
-
-static void omap_mcbsp_clk_disable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_disable(mclk->childs[i]);
-}
-
-static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
+static struct clk omap_mcbsp_clks[] = {
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 1,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 1,
 	},
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 3,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 3,
 	},
 };
 
 #define omap_mcbsp_clks_size	ARRAY_SIZE(omap_mcbsp_clks)
 #else
 #define omap_mcbsp_clks_size	0
-static struct mcbsp_internal_clk __initdata *omap_mcbsp_clks;
-static inline void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static struct clk __initdata *omap_mcbsp_clks;
+static inline void omap_mcbsp_clk_init(struct clk *mclk)
 { }
 #endif
 

@@ -233,7 +198,7 @@ int __init omap1_mcbsp_init(void)
 	for (i = 0; i < omap_mcbsp_clks_size; i++) {
 		if (cpu_is_omap15xx() || cpu_is_omap16xx()) {
 			omap_mcbsp_clk_init(&omap_mcbsp_clks[i]);
-			clk_register(&omap_mcbsp_clks[i].clk);
+			clk_register(&omap_mcbsp_clks[i]);
 		}
 	}
 

arch/arm/mach-omap2/mcbsp.c +21 -67

index cae3ebe..20663e1 100644
@@ -22,103 +22,57 @@
 #include <mach/cpu.h>
 #include <mach/mcbsp.h>
 
-struct mcbsp_internal_clk {
-	struct clk clk;
-	struct clk **childs;
-	int n_childs;
-};
-
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-static void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static void omap_mcbsp_clk_init(struct clk *mclk)
 {
 	const char *clk_names[] = { "mcbsp_ick", "mcbsp_fck" };
 	int i;
 
-	mclk->n_childs = ARRAY_SIZE(clk_names);
-	mclk->childs = kzalloc(mclk->n_childs * sizeof(struct clk *),
+	mclk->n_relatives = ARRAY_SIZE(clk_names);
+	mclk->relatives = kzalloc(mclk->n_relatives * sizeof(struct clk *),
 				GFP_KERNEL);
 
-	for (i = 0; i < mclk->n_childs; i++) {
+	for (i = 0; i < mclk->n_relatives; i++) {
 		/* We fake a platform device to get correct device id */
 		struct platform_device pdev;
 
 		pdev.dev.bus = &platform_bus_type;
-		pdev.id = mclk->clk.id;
-		mclk->childs[i] = clk_get(&pdev.dev, clk_names[i]);
-		if (IS_ERR(mclk->childs[i]))
+		pdev.id = mclk->id;
+		mclk->relatives[i] = clk_get(&pdev.dev, clk_names[i]);
+		if (IS_ERR(mclk->relatives[i]))
 			printk(KERN_ERR "Could not get clock %s (%d).\n",
-				clk_names[i], mclk->clk.id);
+				clk_names[i], mclk->id);
 	}
 }
 
-static int omap_mcbsp_clk_enable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_enable(mclk->childs[i]);
-	return 0;
-}
-
-static void omap_mcbsp_clk_disable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_disable(mclk->childs[i]);
-}
 
-static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
+static struct clk omap_mcbsp_clks[] = {
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 1,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 1,
 	},
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 2,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 2,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 3,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 3,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 4,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 4,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 5,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 5,
 	},
 };
 
 #define omap_mcbsp_clks_size	ARRAY_SIZE(omap_mcbsp_clks)
 #else
 #define omap_mcbsp_clks_size	0
-static struct mcbsp_internal_clk __initdata *omap_mcbsp_clks;
+static struct clk __initdata *omap_mcbsp_clks;
 static inline void omap_mcbsp_clk_init(struct clk *clk)
 { }
 #endif

@@ -287,7 +241,7 @@ static int __init omap2_mcbsp_init(void)
 	for (i = 0; i < omap_mcbsp_clks_size; i++) {
 		/* Once we call clk_get inside init, we do not register it */
 		omap_mcbsp_clk_init(&omap_mcbsp_clks[i]);
-		clk_register(&omap_mcbsp_clks[i].clk);
+		clk_register(&omap_mcbsp_clks[i]);
 	}
 
 	if (cpu_is_omap2420())

arch/arm/plat-omap/clock.c +8 -1

index d13acd2..1c4f877 100644
@@ -77,11 +77,14 @@ EXPORT_SYMBOL(clk_get);
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
-	int ret = 0;
+	int i, ret = 0;
 
 	if (clk == NULL || IS_ERR(clk))
 		return -EINVAL;
 
+	for (i = 0; i < clk->n_relatives; i++)
+		clk_enable(clk->relatives[i]);
+
 	spin_lock_irqsave(&clockfw_lock, flags);
 	if (arch_clock->clk_enable)
 		ret = arch_clock->clk_enable(clk);

@@ -94,10 +97,14 @@ EXPORT_SYMBOL(clk_enable);
 void clk_disable(struct clk *clk)
 {
 	unsigned long flags;
+	int i;
 
 	if (clk == NULL || IS_ERR(clk))
 		return;
 
+	for (i = 0; i < clk->n_relatives; i++)
+		clk_disable(clk->relatives[i]);
+
 	spin_lock_irqsave(&clockfw_lock, flags);
 	if (clk->usecount == 0) {
 		printk(KERN_ERR "Trying disable clock %s with 0 usecount\n",

arch/arm/plat-omap/include/mach/clock.h +2 -0

index 9088925..2760bcf 100644
@@ -71,6 +71,8 @@ struct clk {
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;
+	struct clk		**relatives;
+	int 			n_relatives;
 	void			(*recalc)(struct clk *);
 	int			(*set_rate)(struct clk *, unsigned long);
 	long			(*round_rate)(struct clk *, unsigned long);

This solution keeps the virtual clock in place and enable the child clocks before enable the virtual clock. So, any comments ?

Stanley.

Quoted 308 lines · Stanley.Miao

A spin_lock deadlock will occur when omap_mcbsp_request() is invoked.

omap_mcbsp_request()         -->
clk_enable(mcbsp->clk)       --> clk_enable get clockfw_lock, then call ->
omap2_clk_enable()           -->
_omap2_clk_enable()          -->
omap_mcbsp_clk_enable()      -->
clk_enable(mcbsp_ick)        --> now clk_enable acquire clockfw_lock again.

The solution:

If a alias clock has some relative clocks, enable relative clocks before enable the alias clock.

Signed-off-by Stanley.Miao <[email protected]>

---
 arch/arm/mach-omap1/mcbsp.c             |   67 ++++++------------------
 arch/arm/mach-omap2/mcbsp.c             |   88 +++++++-----------------------
 arch/arm/plat-omap/clock.c              |    9 +++-
 arch/arm/plat-omap/include/mach/clock.h |    2 +
 4 files changed, 47 insertions(+), 119 deletions(-)

arch/arm/mach-omap1/mcbsp.c +16 -51

index 7de7c69..cf220be 100644
@@ -26,80 +26,45 @@
 #define DPS_RSTCT2_PER_EN	(1 << 0)
 #define DSP_RSTCT2_WD_PER_EN	(1 << 1)
 
-struct mcbsp_internal_clk {
-	struct clk clk;
-	struct clk **childs;
-	int n_childs;
-};
-
 #if defined(CONFIG_ARCH_OMAP15XX) || defined(CONFIG_ARCH_OMAP16XX)
-static void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static void omap_mcbsp_clk_init(struct clk *mclk)
 {
 	const char *clk_names[] = { "dsp_ck", "api_ck", "dspxor_ck" };
 	int i;
 
-	mclk->n_childs = ARRAY_SIZE(clk_names);
-	mclk->childs = kzalloc(mclk->n_childs * sizeof(struct clk *),
+	mclk->n_relatives = ARRAY_SIZE(clk_names);
+	mclk->relatives = kzalloc(mclk->n_relatives * sizeof(struct clk *),
 				GFP_KERNEL);
 
-	for (i = 0; i < mclk->n_childs; i++) {
+	for (i = 0; i < mclk->n_relatives; i++) {
 		/* We fake a platform device to get correct device id */
 		struct platform_device pdev;
 
 		pdev.dev.bus = &platform_bus_type;
-		pdev.id = mclk->clk.id;
-		mclk->childs[i] = clk_get(&pdev.dev, clk_names[i]);
-		if (IS_ERR(mclk->childs[i]))
+		pdev.id = mclk->id;
+		mclk->relatives[i] = clk_get(&pdev.dev, clk_names[i]);
+		if (IS_ERR(mclk->relatives[i]))
 			printk(KERN_ERR "Could not get clock %s (%d).\n",
-				clk_names[i], mclk->clk.id);
+				clk_names[i], mclk->id);
 	}
 }
 
-static int omap_mcbsp_clk_enable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_enable(mclk->childs[i]);
-	return 0;
-}
-
-static void omap_mcbsp_clk_disable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_disable(mclk->childs[i]);
-}
-
-static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
+static struct clk omap_mcbsp_clks[] = {
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 1,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 1,
 	},
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 3,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 3,
 	},
 };
 
 #define omap_mcbsp_clks_size	ARRAY_SIZE(omap_mcbsp_clks)
 #else
 #define omap_mcbsp_clks_size	0
-static struct mcbsp_internal_clk __initdata *omap_mcbsp_clks;
-static inline void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static struct clk __initdata *omap_mcbsp_clks;
+static inline void omap_mcbsp_clk_init(struct clk *mclk)
 { }
 #endif
 

@@ -233,7 +198,7 @@ int __init omap1_mcbsp_init(void)
 	for (i = 0; i < omap_mcbsp_clks_size; i++) {
 		if (cpu_is_omap15xx() || cpu_is_omap16xx()) {
 			omap_mcbsp_clk_init(&omap_mcbsp_clks[i]);
-			clk_register(&omap_mcbsp_clks[i].clk);
+			clk_register(&omap_mcbsp_clks[i]);
 		}
 	}
 

arch/arm/mach-omap2/mcbsp.c +21 -67

index cae3ebe..20663e1 100644
@@ -22,103 +22,57 @@
 #include <mach/cpu.h>
 #include <mach/mcbsp.h>
 
-struct mcbsp_internal_clk {
-	struct clk clk;
-	struct clk **childs;
-	int n_childs;
-};
-
 #if defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
-static void omap_mcbsp_clk_init(struct mcbsp_internal_clk *mclk)
+static void omap_mcbsp_clk_init(struct clk *mclk)
 {
 	const char *clk_names[] = { "mcbsp_ick", "mcbsp_fck" };
 	int i;
 
-	mclk->n_childs = ARRAY_SIZE(clk_names);
-	mclk->childs = kzalloc(mclk->n_childs * sizeof(struct clk *),
+	mclk->n_relatives = ARRAY_SIZE(clk_names);
+	mclk->relatives = kzalloc(mclk->n_relatives * sizeof(struct clk *),
 				GFP_KERNEL);
 
-	for (i = 0; i < mclk->n_childs; i++) {
+	for (i = 0; i < mclk->n_relatives; i++) {
 		/* We fake a platform device to get correct device id */
 		struct platform_device pdev;
 
 		pdev.dev.bus = &platform_bus_type;
-		pdev.id = mclk->clk.id;
-		mclk->childs[i] = clk_get(&pdev.dev, clk_names[i]);
-		if (IS_ERR(mclk->childs[i]))
+		pdev.id = mclk->id;
+		mclk->relatives[i] = clk_get(&pdev.dev, clk_names[i]);
+		if (IS_ERR(mclk->relatives[i]))
 			printk(KERN_ERR "Could not get clock %s (%d).\n",
-				clk_names[i], mclk->clk.id);
+				clk_names[i], mclk->id);
 	}
 }
 
-static int omap_mcbsp_clk_enable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_enable(mclk->childs[i]);
-	return 0;
-}
-
-static void omap_mcbsp_clk_disable(struct clk *clk)
-{
-	struct mcbsp_internal_clk *mclk = container_of(clk,
-					struct mcbsp_internal_clk, clk);
-	int i;
-
-	for (i = 0; i < mclk->n_childs; i++)
-		clk_disable(mclk->childs[i]);
-}
 
-static struct mcbsp_internal_clk omap_mcbsp_clks[] = {
+static struct clk omap_mcbsp_clks[] = {
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 1,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 1,
 	},
 	{
-		.clk = {
-			.name 		= "mcbsp_clk",
-			.id		= 2,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name 		= "mcbsp_clk",
+		.id		= 2,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 3,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 3,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 4,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 4,
 	},
 	{
-		.clk = {
-			.name		= "mcbsp_clk",
-			.id		= 5,
-			.enable		= omap_mcbsp_clk_enable,
-			.disable	= omap_mcbsp_clk_disable,
-		},
+		.name		= "mcbsp_clk",
+		.id		= 5,
 	},
 };
 
 #define omap_mcbsp_clks_size	ARRAY_SIZE(omap_mcbsp_clks)
 #else
 #define omap_mcbsp_clks_size	0
-static struct mcbsp_internal_clk __initdata *omap_mcbsp_clks;
+static struct clk __initdata *omap_mcbsp_clks;
 static inline void omap_mcbsp_clk_init(struct clk *clk)
 { }
 #endif

@@ -287,7 +241,7 @@ static int __init omap2_mcbsp_init(void)
 	for (i = 0; i < omap_mcbsp_clks_size; i++) {
 		/* Once we call clk_get inside init, we do not register it */
 		omap_mcbsp_clk_init(&omap_mcbsp_clks[i]);
-		clk_register(&omap_mcbsp_clks[i].clk);
+		clk_register(&omap_mcbsp_clks[i]);
 	}
 
 	if (cpu_is_omap2420())

arch/arm/plat-omap/clock.c +8 -1

index d13acd2..1c4f877 100644
@@ -77,11 +77,14 @@ EXPORT_SYMBOL(clk_get);
 int clk_enable(struct clk *clk)
 {
 	unsigned long flags;
-	int ret = 0;
+	int i, ret = 0;
 
 	if (clk == NULL || IS_ERR(clk))
 		return -EINVAL;
 
+	for (i = 0; i < clk->n_relatives; i++)
+		clk_enable(clk->relatives[i]);
+
 	spin_lock_irqsave(&clockfw_lock, flags);
 	if (arch_clock->clk_enable)
 		ret = arch_clock->clk_enable(clk);

@@ -94,10 +97,14 @@ EXPORT_SYMBOL(clk_enable);
 void clk_disable(struct clk *clk)
 {
 	unsigned long flags;
+	int i;
 
 	if (clk == NULL || IS_ERR(clk))
 		return;
 
+	for (i = 0; i < clk->n_relatives; i++)
+		clk_disable(clk->relatives[i]);
+
 	spin_lock_irqsave(&clockfw_lock, flags);
 	if (clk->usecount == 0) {
 		printk(KERN_ERR "Trying disable clock %s with 0 usecount\n",

arch/arm/plat-omap/include/mach/clock.h +0 -0

index 9088925..2760bcf 100644

arch/arm/plat-omap/include/mach/clock.h struct clk { · line 78

Quoted 8 lines

arch/arm/plat-omap/include/mach/clock.h +2 -0

@@ -71,6 +71,8 @@ struct clk {
 	__u8			enable_bit;
 	__s8			usecount;
 	u8			idlest_bit;
+	struct clk		**relatives;
+	int 			n_relatives;
 	void			(*recalc)(struct clk *);
 	int			(*set_rate)(struct clk *, unsigned long);
 	long			(*round_rate)(struct clk *, unsigned long);

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 2 lines · stanley.miao

This solution keeps the virtual clock in place and enable the child clocks before enable the virtual clock. So, any comments ?

What if we just removed the custom clock and had a struct **clk in struct omap_mcbsp that contains the clocks for each instance?

Quoted 6 lines · Tony Lindgren

Quoted 2 lines · stanley.miao

This solution keeps the virtual clock in place and enable the child clocks before enable the virtual clock. So, any comments ?

What if we just removed the custom clock and had a struct **clk in struct omap_mcbsp that contains the clocks for each instance?

It works. This is what I did in my first patch.

The difference is I add two struct *clk in struct omap_mcbsp.

struct omap_mcbsp {

(quoted diff) +0 -0

@@ -365,7 +366,8 @@ struct omap_mcbsp {
        /* Protect the field .free, while checking if the mcbsp is in

use */

        spinlock_t lock;
        struct omap_mcbsp_platform_data *pdata;

(quoted diff) +2 -1

-       struct clk *clk;
+       struct clk *ick;
+       struct clk *fck;

If one struct **clk is better, I will resend the patch later.

Quoted 5 lines · stanley.miao

Quoted 4 lines · Tony Lindgren

Quoted 2 lines · stanley.miao

This solution keeps the virtual clock in place and enable the child clocks before enable the virtual clock. So, any comments ?

Quoted 4 lines

Quoted 2 lines

What if we just removed the custom clock and had a struct **clk in struct omap_mcbsp that contains the clocks for each instance?

It works. This is what I did in my first patch.

OK, sorry for all this going back and forth..

We still don't
have a good long term solution on how to handle different clocks..

Sounds like we should just apply your original patch, then figure out a good long term approacth.

Can you please repost your first version of the patch?

Regards,

Tony