* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
@ 2009-01-13 9:08 Eero Nurkkala
2009-01-13 10:06 ` Tony Lindgren
0 siblings, 1 reply; 8+ messages in thread
From: Eero Nurkkala @ 2009-01-13 9:08 UTC (permalink / raw)
To: linux-omap
> Can you please repost your first version of the patch?
Please. This V1 patch is good if one wishes to provide the fclk
for McBSP from the MCBSP_CLKS. Then the fclk from the per can be turned
off. With the V2 patch it's not that straightforward at all.
- Eero
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
@ 2008-11-06 12:44 Stanley.Miao
2008-11-07 13:52 ` stanley.miao
0 siblings, 1 reply; 8+ messages in thread
From: Stanley.Miao @ 2008-11-06 12:44 UTC (permalink / raw)
To: linux-omap
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 <stanley.miao@windriver.com>
---
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(-)
diff --git a/arch/arm/mach-omap1/mcbsp.c b/arch/arm/mach-omap1/mcbsp.c
index 7de7c69..cf220be 100644
--- a/arch/arm/mach-omap1/mcbsp.c
+++ b/arch/arm/mach-omap1/mcbsp.c
@@ -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]);
}
}
diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
index cae3ebe..20663e1 100644
--- a/arch/arm/mach-omap2/mcbsp.c
+++ b/arch/arm/mach-omap2/mcbsp.c
@@ -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())
diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
index d13acd2..1c4f877 100644
--- a/arch/arm/plat-omap/clock.c
+++ b/arch/arm/plat-omap/clock.c
@@ -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",
diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
index 9088925..2760bcf 100644
--- a/arch/arm/plat-omap/include/mach/clock.h
+++ b/arch/arm/plat-omap/include/mach/clock.h
@@ -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);
--
1.5.6.3
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
2008-11-06 12:44 Stanley.Miao
@ 2008-11-07 13:52 ` stanley.miao
2009-01-08 13:33 ` Tony Lindgren
0 siblings, 1 reply; 8+ messages in thread
From: stanley.miao @ 2008-11-07 13:52 UTC (permalink / raw)
To: linux-omap
This solution keeps the virtual clock in place and enable the child
clocks before enable the virtual clock. So, any comments ?
Stanley.
On Thu, 2008-11-06 at 20:44 +0800, Stanley.Miao wrote:
> 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 <stanley.miao@windriver.com>
> ---
> 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(-)
>
> diff --git a/arch/arm/mach-omap1/mcbsp.c b/arch/arm/mach-omap1/mcbsp.c
> index 7de7c69..cf220be 100644
> --- a/arch/arm/mach-omap1/mcbsp.c
> +++ b/arch/arm/mach-omap1/mcbsp.c
> @@ -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]);
> }
> }
>
> diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
> index cae3ebe..20663e1 100644
> --- a/arch/arm/mach-omap2/mcbsp.c
> +++ b/arch/arm/mach-omap2/mcbsp.c
> @@ -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())
> diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> index d13acd2..1c4f877 100644
> --- a/arch/arm/plat-omap/clock.c
> +++ b/arch/arm/plat-omap/clock.c
> @@ -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",
> diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
> index 9088925..2760bcf 100644
> --- a/arch/arm/plat-omap/include/mach/clock.h
> +++ b/arch/arm/plat-omap/include/mach/clock.h
> @@ -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);
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
2008-11-07 13:52 ` stanley.miao
@ 2009-01-08 13:33 ` Tony Lindgren
2009-01-12 10:42 ` stanley.miao
0 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2009-01-08 13:33 UTC (permalink / raw)
To: stanley.miao; +Cc: linux-omap
* stanley.miao <stanley.miao@windriver.com> [081107 15:47]:
> 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?
Regards,
Tony
> Stanley.
>
> On Thu, 2008-11-06 at 20:44 +0800, Stanley.Miao wrote:
> > 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 <stanley.miao@windriver.com>
> > ---
> > 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(-)
> >
> > diff --git a/arch/arm/mach-omap1/mcbsp.c b/arch/arm/mach-omap1/mcbsp.c
> > index 7de7c69..cf220be 100644
> > --- a/arch/arm/mach-omap1/mcbsp.c
> > +++ b/arch/arm/mach-omap1/mcbsp.c
> > @@ -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]);
> > }
> > }
> >
> > diff --git a/arch/arm/mach-omap2/mcbsp.c b/arch/arm/mach-omap2/mcbsp.c
> > index cae3ebe..20663e1 100644
> > --- a/arch/arm/mach-omap2/mcbsp.c
> > +++ b/arch/arm/mach-omap2/mcbsp.c
> > @@ -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())
> > diff --git a/arch/arm/plat-omap/clock.c b/arch/arm/plat-omap/clock.c
> > index d13acd2..1c4f877 100644
> > --- a/arch/arm/plat-omap/clock.c
> > +++ b/arch/arm/plat-omap/clock.c
> > @@ -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",
> > diff --git a/arch/arm/plat-omap/include/mach/clock.h b/arch/arm/plat-omap/include/mach/clock.h
> > index 9088925..2760bcf 100644
> > --- a/arch/arm/plat-omap/include/mach/clock.h
> > +++ b/arch/arm/plat-omap/include/mach/clock.h
> > @@ -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 majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
2009-01-08 13:33 ` Tony Lindgren
@ 2009-01-12 10:42 ` stanley.miao
2009-01-12 10:46 ` Tony Lindgren
0 siblings, 1 reply; 8+ messages in thread
From: stanley.miao @ 2009-01-12 10:42 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap
On Thu, 2009-01-08 at 15:33 +0200, Tony Lindgren wrote:
> * stanley.miao <stanley.miao@windriver.com> [081107 15:47]:
> > 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 {
@@ -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;
- struct clk *clk;
+ struct clk *ick;
+ struct clk *fck;
If one struct **clk is better, I will resend the patch later.
Stanley
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
2009-01-12 10:42 ` stanley.miao
@ 2009-01-12 10:46 ` Tony Lindgren
2009-01-14 1:39 ` stanley.miao
0 siblings, 1 reply; 8+ messages in thread
From: Tony Lindgren @ 2009-01-12 10:46 UTC (permalink / raw)
To: stanley.miao; +Cc: linux-omap
* stanley.miao <stanley.miao@windriver.com> [090112 12:34]:
> On Thu, 2009-01-08 at 15:33 +0200, Tony Lindgren wrote:
> > * stanley.miao <stanley.miao@windriver.com> [081107 15:47]:
> > > 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.
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..
> The difference is I add two struct *clk in struct omap_mcbsp.
>
> struct omap_mcbsp {
> @@ -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;
> - struct clk *clk;
> + struct clk *ick;
> + struct clk *fck;
>
>
> If one struct **clk is better, I will resend the patch later.
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] OMAP: Fix McBSP spin_lock deadlock.
2009-01-12 10:46 ` Tony Lindgren
@ 2009-01-14 1:39 ` stanley.miao
0 siblings, 0 replies; 8+ messages in thread
From: stanley.miao @ 2009-01-14 1:39 UTC (permalink / raw)
To: Tony Lindgren; +Cc: linux-omap
On Mon, 2009-01-12 at 12:46 +0200, Tony Lindgren wrote:
> * stanley.miao <stanley.miao@windriver.com> [090112 12:34]:
> > On Thu, 2009-01-08 at 15:33 +0200, Tony Lindgren wrote:
> > > * stanley.miao <stanley.miao@windriver.com> [081107 15:47]:
> > > > 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.
>
> OK, sorry for all this going back and forth..
It's OK. In order to make the code better, it is worth a little more
job.
> We still don't
> have a good long term solution on how to handle different clocks..
>
<snip>
>
> 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?
I will post the patch V3.
Stanley.
>
> Regards,
>
> Tony
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-01-14 1:30 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-01-13 9:08 [PATCH v2] OMAP: Fix McBSP spin_lock deadlock Eero Nurkkala
2009-01-13 10:06 ` Tony Lindgren
-- strict thread matches above, loose matches on Subject: below --
2008-11-06 12:44 Stanley.Miao
2008-11-07 13:52 ` stanley.miao
2009-01-08 13:33 ` Tony Lindgren
2009-01-12 10:42 ` stanley.miao
2009-01-12 10:46 ` Tony Lindgren
2009-01-14 1:39 ` stanley.miao
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox