[PATCH 0/2] mach-omap2: handle autoidle denial

[PATCH 0/2] mach-omap2: handle autoidle denial

[Andreas Kemnade]

On the gta04 with a dm3730 omap_hdq does not work properly when the
device enters lower power states. Idling uart1 and 2 is enough
to show up that problem, if there are no other things enabled.
Further research reveals that hdq iclk must not be turned off during
transfers, also according to the TRM. That fact is also correctly described
in the flags but the code to handle that is incomplete.

To handle multiple users of a single ick, autoidle is disabled
when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))

Changes since the RFC version:
- mutex lock for autoidle changes
- deny_idle/allow_idle calls moved to clock enable/disable of the
  individual modules

Andreas Kemnade (2):
  clk: ti: add a usecount for autoidle
  arm: omap_hwmod disable ick autoidling when a hwmod requires that

 arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
 drivers/clk/ti/autoidle.c        | 32 ++++++++++++++++++++++++--------
 include/linux/clk/ti.h           |  1 +
 3 files changed, 37 insertions(+), 12 deletions(-)

-- 
2.11.0

[PATCH 1/2] clk: ti: add a usecount for autoidle

[Andreas Kemnade]

We have the scenario that first autoidle is disabled for all clocks,
then it is disabled for selected ones and then enabled for all. So
we should have some counting here, also according to the
comment in  _setup_iclk_autoidle()

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 drivers/clk/ti/autoidle.c | 32 ++++++++++++++++++++++++--------
 include/linux/clk/ti.h    |  1 +
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
index 7bb9afbe4058..bb6cff168e73 100644
--- a/drivers/clk/ti/autoidle.c
+++ b/drivers/clk/ti/autoidle.c
@@ -37,6 +37,14 @@ struct clk_ti_autoidle {
 static LIST_HEAD(autoidle_clks);
 static LIST_HEAD(clk_hw_omap_clocks);
 
+/*
+ * we have some non-atomic read/write
+ * operations behind it, so lets
+ * take one mutex for handling autoidle
+ * of all clocks
+ */
+static DEFINE_MUTEX(autoidle_mutex);
+
 /**
  * omap2_clk_deny_idle - disable autoidle on an OMAP clock
  * @clk: struct clk * to disable autoidle for
@@ -48,8 +56,13 @@ int omap2_clk_deny_idle(struct clk *clk)
 	struct clk_hw_omap *c;
 
 	c = to_clk_hw_omap(__clk_get_hw(clk));
-	if (c->ops && c->ops->deny_idle)
-		c->ops->deny_idle(c);
+	if (c->ops && c->ops->deny_idle) {
+		mutex_lock(&autoidle_mutex);
+		c->autoidle_count--;
+		if (c->autoidle_count == -1)
+			c->ops->deny_idle(c);
+		mutex_unlock(&autoidle_mutex);
+	}
 	return 0;
 }
 
@@ -64,8 +77,13 @@ int omap2_clk_allow_idle(struct clk *clk)
 	struct clk_hw_omap *c;
 
 	c = to_clk_hw_omap(__clk_get_hw(clk));
-	if (c->ops && c->ops->allow_idle)
-		c->ops->allow_idle(c);
+	if (c->ops && c->ops->allow_idle) {
+		mutex_lock(&autoidle_mutex);
+		c->autoidle_count++;
+		if (c->autoidle_count == 0)
+			c->ops->allow_idle(c);
+		mutex_unlock(&autoidle_mutex);
+	}
 	return 0;
 }
 
@@ -201,8 +219,7 @@ int omap2_clk_enable_autoidle_all(void)
 	struct clk_hw_omap *c;
 
 	list_for_each_entry(c, &clk_hw_omap_clocks, node)
-		if (c->ops && c->ops->allow_idle)
-			c->ops->allow_idle(c);
+		omap2_clk_allow_idle(c->hw.clk);
 
 	_clk_generic_allow_autoidle_all();
 
@@ -223,8 +240,7 @@ int omap2_clk_disable_autoidle_all(void)
 	struct clk_hw_omap *c;
 
 	list_for_each_entry(c, &clk_hw_omap_clocks, node)
-		if (c->ops && c->ops->deny_idle)
-			c->ops->deny_idle(c);
+		omap2_clk_deny_idle(c->hw.clk);
 
 	_clk_generic_deny_autoidle_all();
 
diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
index a8faa38b1ed6..c460355419c0 100644
--- a/include/linux/clk/ti.h
+++ b/include/linux/clk/ti.h
@@ -159,6 +159,7 @@ struct clk_hw_omap {
 	const char		*clkdm_name;
 	struct clockdomain	*clkdm;
 	const struct clk_hw_omap_ops	*ops;
+	int autoidle_count;
 };
 
 /*
-- 
2.11.0

[PATCH 2/2] arm: omap_hwmod disable ick autoidling when a hwmod requires that

[Andreas Kemnade]

Deny autoidle for hwmods with the OCPIF_SWSUP_IDLE flag,
that makes hwmods working properly which cannot handle
autoidle properly in lower power states.
Affected is e. g. the omap_hdq.
Since an ick might have mulitple users, autoidle is disabled
when an individual user requires that rather than in
_setup_iclk_autoidle. dss_ick is an example for that.

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index bb641e6c93d0..0078b0e1d242 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -986,8 +986,10 @@ static int _enable_clocks(struct omap_hwmod *oh)
 		clk_enable(oh->_clk);
 
 	list_for_each_entry(os, &oh->slave_ports, node) {
-		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
+		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
+			omap2_clk_deny_idle(os->_clk);
 			clk_enable(os->_clk);
+		}
 	}
 
 	/* The opt clocks are controlled by the device driver. */
@@ -1039,8 +1041,10 @@ static int _disable_clocks(struct omap_hwmod *oh)
 		clk_disable(oh->_clk);
 
 	list_for_each_entry(os, &oh->slave_ports, node) {
-		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
+		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
 			clk_disable(os->_clk);
+			omap2_clk_allow_idle(os->_clk);
+		}
 	}
 
 	if (oh->flags & HWMOD_OPT_CLKS_NEEDED)
@@ -2410,9 +2414,13 @@ static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
 			continue;
 
 		if (os->flags & OCPIF_SWSUP_IDLE) {
-			/* XXX omap_iclk_deny_idle(c); */
+			/*
+			 * we might have multiple users of one iclk with
+			 * different requirements, disable autoidle when
+			 * the module is enabled, e.g. dss iclk
+			 */
 		} else {
-			/* XXX omap_iclk_allow_idle(c); */
+			/* we are enabling autoidle afterwards anyways */
 			clk_enable(os->_clk);
 		}
 	}
-- 
2.11.0

Re: [PATCH 0/2] mach-omap2: handle autoidle denial

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 1320 bytes --]

ping.

..after stumbling again about that problem during testing with 4.20-rc1.
will retest it there.

On Thu,  4 Oct 2018 22:38:15 +0200
Andreas Kemnade <andreas@kemnade.info> wrote:

> On the gta04 with a dm3730 omap_hdq does not work properly when the
> device enters lower power states. Idling uart1 and 2 is enough
> to show up that problem, if there are no other things enabled.
> Further research reveals that hdq iclk must not be turned off during
> transfers, also according to the TRM. That fact is also correctly described
> in the flags but the code to handle that is incomplete.
> 
> To handle multiple users of a single ick, autoidle is disabled
> when a user of that ick requires that (has the OCPIF_SWSUP_IDLE))
> 
> Changes since the RFC version:
> - mutex lock for autoidle changes
> - deny_idle/allow_idle calls moved to clock enable/disable of the
>   individual modules
> 
> Andreas Kemnade (2):
>   clk: ti: add a usecount for autoidle
>   arm: omap_hwmod disable ick autoidling when a hwmod requires that
> 
>  arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
>  drivers/clk/ti/autoidle.c        | 32 ++++++++++++++++++++++++--------
>  include/linux/clk/ti.h           |  1 +
>  3 files changed, 37 insertions(+), 12 deletions(-)
> 
> -- 
> 2.11.0
> 
> 

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] arm: omap_hwmod disable ick autoidling when a hwmod requires that

[Tero Kristo]

On 04/10/2018 23:38, Andreas Kemnade wrote:
> Deny autoidle for hwmods with the OCPIF_SWSUP_IDLE flag,
> that makes hwmods working properly which cannot handle
> autoidle properly in lower power states.
> Affected is e. g. the omap_hdq.
> Since an ick might have mulitple users, autoidle is disabled
> when an individual user requires that rather than in
> _setup_iclk_autoidle. dss_ick is an example for that.
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>   arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
>   1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index bb641e6c93d0..0078b0e1d242 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> @@ -986,8 +986,10 @@ static int _enable_clocks(struct omap_hwmod *oh)
>   		clk_enable(oh->_clk);
>   
>   	list_for_each_entry(os, &oh->slave_ports, node) {
> -		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
> +		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
> +			omap2_clk_deny_idle(os->_clk);

I think calling this unconditionally across all platforms / clock types 
might cause problems. Checking kernel, am33xx seems to have one clock 
with this flag that is not of omap2 type. Do we have any testing data 
that this doesn't break things?

-Tero

>   			clk_enable(os->_clk);
> +		}
>   	}
>   
>   	/* The opt clocks are controlled by the device driver. */
> @@ -1039,8 +1041,10 @@ static int _disable_clocks(struct omap_hwmod *oh)
>   		clk_disable(oh->_clk);
>   
>   	list_for_each_entry(os, &oh->slave_ports, node) {
> -		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
> +		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
>   			clk_disable(os->_clk);
> +			omap2_clk_allow_idle(os->_clk);
> +		}
>   	}
>   
>   	if (oh->flags & HWMOD_OPT_CLKS_NEEDED)
> @@ -2410,9 +2414,13 @@ static void __init _setup_iclk_autoidle(struct omap_hwmod *oh)
>   			continue;
>   
>   		if (os->flags & OCPIF_SWSUP_IDLE) {
> -			/* XXX omap_iclk_deny_idle(c); */
> +			/*
> +			 * we might have multiple users of one iclk with
> +			 * different requirements, disable autoidle when
> +			 * the module is enabled, e.g. dss iclk
> +			 */
>   		} else {
> -			/* XXX omap_iclk_allow_idle(c); */
> +			/* we are enabling autoidle afterwards anyways */
>   			clk_enable(os->_clk);
>   		 >   	}
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] clk: ti: add a usecount for autoidle

[Tero Kristo]

On 04/10/2018 23:38, Andreas Kemnade wrote:
> We have the scenario that first autoidle is disabled for all clocks,
> then it is disabled for selected ones and then enabled for all. So
> we should have some counting here, also according to the
> comment in  _setup_iclk_autoidle()
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---
>   drivers/clk/ti/autoidle.c | 32 ++++++++++++++++++++++++--------
>   include/linux/clk/ti.h    |  1 +
>   2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
> index 7bb9afbe4058..bb6cff168e73 100644
> --- a/drivers/clk/ti/autoidle.c
> +++ b/drivers/clk/ti/autoidle.c
> @@ -37,6 +37,14 @@ struct clk_ti_autoidle {
>   static LIST_HEAD(autoidle_clks);
>   static LIST_HEAD(clk_hw_omap_clocks);
>   
> +/*
> + * we have some non-atomic read/write
> + * operations behind it, so lets
> + * take one mutex for handling autoidle
> + * of all clocks
> + */
> +static DEFINE_MUTEX(autoidle_mutex);

Why mutex? This prevents calling the autoidle APIs from atomic context. 
Did you check the mutex debug kernel configs with this?

This may cause problems with the runtime PM entries to the code at least.


> +
>   /**
>    * omap2_clk_deny_idle - disable autoidle on an OMAP clock
>    * @clk: struct clk * to disable autoidle for
> @@ -48,8 +56,13 @@ int omap2_clk_deny_idle(struct clk *clk)
>   	struct clk_hw_omap *c;
>   
>   	c = to_clk_hw_omap(__clk_get_hw(clk));
> -	if (c->ops && c->ops->deny_idle)
> -		c->ops->deny_idle(c);
> +	if (c->ops && c->ops->deny_idle) {
> +		mutex_lock(&autoidle_mutex);
> +		c->autoidle_count--;
> +		if (c->autoidle_count == -1)

I think you should swap the arithmetics here, all the other usecounters 
use positive values, here you enter deep to the negative side when 
autoidle is denied by multiple users, which might be confusing.

-Tero

> +			c->ops->deny_idle(c);
> +		mutex_unlock(&autoidle_mutex);
> +	}
>   	return 0;
>   }
>   
> @@ -64,8 +77,13 @@ int omap2_clk_allow_idle(struct clk *clk)
>   	struct clk_hw_omap *c;
>   
>   	c = to_clk_hw_omap(__clk_get_hw(clk));
> -	if (c->ops && c->ops->allow_idle)
> -		c->ops->allow_idle(c);
> +	if (c->ops && c->ops->allow_idle) {
> +		mutex_lock(&autoidle_mutex);
> +		c->autoidle_count++;
> +		if (c->autoidle_count == 0)
> +			c->ops->allow_idle(c);
> +		mutex_unlock(&autoidle_mutex);
> +	}
>   	return 0;
>   }
>   
> @@ -201,8 +219,7 @@ int omap2_clk_enable_autoidle_all(void)
>   	struct clk_hw_omap *c;
>   
>   	list_for_each_entry(c, &clk_hw_omap_clocks, node)
> -		if (c->ops && c->ops->allow_idle)
> -			c->ops->allow_idle(c);
> +		omap2_clk_allow_idle(c->hw.clk);
>   
>   	_clk_generic_allow_autoidle_all();
>   
> @@ -223,8 +240,7 @@ int omap2_clk_disable_autoidle_all(void)
>   	struct clk_hw_omap *c;
>   
>   	list_for_each_entry(c, &clk_hw_omap_clocks, node)
> -		if (c->ops && c->ops->deny_idle)
> -			c->ops->deny_idle(c);
> +		omap2_clk_deny_idle(c->hw.clk);
>   
>   	_clk_generic_deny_autoidle_all();
>   
> diff --git a/include/linux/clk/ti.h b/include/linux/clk/ti.h
> index a8faa38b1ed6..c460355419c0 100644
> --- a/include/linux/clk/ti.h
> +++ b/include/linux/clk/ti.h
> @@ -159,6 +159,7 @@ struct clk_hw_omap {
>   	const char		*clkdm_name;
>   	struct clockdomain	*clkdm;
>   	const struct clk_hw_omap_ops	*ops;
> +	int autoidle_count;
>   };
>   
>   /*
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 2/2] arm: omap_hwmod disable ick autoidling when a hwmod requires that

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 2067 bytes --]

Hi,

On Thu, 8 Nov 2018 12:26:08 +0200
Tero Kristo <t-kristo@ti.com> wrote:

> On 04/10/2018 23:38, Andreas Kemnade wrote:
> > Deny autoidle for hwmods with the OCPIF_SWSUP_IDLE flag,
> > that makes hwmods working properly which cannot handle
> > autoidle properly in lower power states.
> > Affected is e. g. the omap_hdq.
> > Since an ick might have mulitple users, autoidle is disabled
> > when an individual user requires that rather than in
> > _setup_iclk_autoidle. dss_ick is an example for that.
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >   arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
> >   1 file changed, 12 insertions(+), 4 deletions(-)
> > 
> > diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> > index bb641e6c93d0..0078b0e1d242 100644
> > --- a/arch/arm/mach-omap2/omap_hwmod.c
> > +++ b/arch/arm/mach-omap2/omap_hwmod.c
> > @@ -986,8 +986,10 @@ static int _enable_clocks(struct omap_hwmod *oh)
> >   		clk_enable(oh->_clk);
> >   
> >   	list_for_each_entry(os, &oh->slave_ports, node) {
> > -		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
> > +		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
> > +			omap2_clk_deny_idle(os->_clk);  
> 
> I think calling this unconditionally across all platforms / clock types 
> might cause problems. Checking kernel, am33xx seems to have one clock 
> with this flag that is not of omap2 type. Do we have any testing data 
> that this doesn't break things?
> 
Somehow I have missed that am33xx clock. I have not tested it on that
platform. Concerning am3xxx I have only a beaglebone block but it not
am33xx I guess. So I have to recheck I guess.
But I think the intention of this flag was to control autoidle
vs. sw-controlled idle according to...
[...]
> >   		if (os->flags & OCPIF_SWSUP_IDLE) {
> > -			/* XXX omap_iclk_deny_idle(c); */
> > +			/*
this comment. So we need a if (clock_is_omap2()) or something like that
or remove that flag from any other clocks?

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 2/2] arm: omap_hwmod disable ick autoidling when a hwmod requires that

[Tero Kristo]

On 08/11/2018 13:08, Andreas Kemnade wrote:
> Hi,
> 
> On Thu, 8 Nov 2018 12:26:08 +0200
> Tero Kristo <t-kristo@ti.com> wrote:
> 
>> On 04/10/2018 23:38, Andreas Kemnade wrote:
>>> Deny autoidle for hwmods with the OCPIF_SWSUP_IDLE flag,
>>> that makes hwmods working properly which cannot handle
>>> autoidle properly in lower power states.
>>> Affected is e. g. the omap_hdq.
>>> Since an ick might have mulitple users, autoidle is disabled
>>> when an individual user requires that rather than in
>>> _setup_iclk_autoidle. dss_ick is an example for that.
>>>
>>> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
>>> ---
>>>    arch/arm/mach-omap2/omap_hwmod.c | 16 ++++++++++++----
>>>    1 file changed, 12 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>>> index bb641e6c93d0..0078b0e1d242 100644
>>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>>> @@ -986,8 +986,10 @@ static int _enable_clocks(struct omap_hwmod *oh)
>>>    		clk_enable(oh->_clk);
>>>    
>>>    	list_for_each_entry(os, &oh->slave_ports, node) {
>>> -		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE))
>>> +		if (os->_clk && (os->flags & OCPIF_SWSUP_IDLE)) {
>>> +			omap2_clk_deny_idle(os->_clk);
>>
>> I think calling this unconditionally across all platforms / clock types
>> might cause problems. Checking kernel, am33xx seems to have one clock
>> with this flag that is not of omap2 type. Do we have any testing data
>> that this doesn't break things?
>>
> Somehow I have missed that am33xx clock. I have not tested it on that
> platform. Concerning am3xxx I have only a beaglebone block but it not
> am33xx I guess. So I have to recheck I guess.

Beaglebone black is am33xx based device, so you can use that for testing.

> But I think the intention of this flag was to control autoidle
> vs. sw-controlled idle according to...
> [...]
>>>    		if (os->flags & OCPIF_SWSUP_IDLE) {
>>> -			/* XXX omap_iclk_deny_idle(c); */
>>> +			/*
> this comment. So we need a if (clock_is_omap2()) or something like that
> or remove that flag from any other clocks?

You could add check within the omap2_clk_deny_idle / allow_idle calls 
themselves. You can check for presence of flag CLK_IS_BASIC, this is not 
set for omap clocks.

-Tero

> 
> Regards,
> Andreas
> 

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

Re: [PATCH 1/2] clk: ti: add a usecount for autoidle

[Andreas Kemnade]

[-- Attachment #1: Type: text/plain, Size: 2246 bytes --]

On Thu, 8 Nov 2018 12:36:35 +0200
Tero Kristo <t-kristo@ti.com> wrote:

> On 04/10/2018 23:38, Andreas Kemnade wrote:
> > We have the scenario that first autoidle is disabled for all clocks,
> > then it is disabled for selected ones and then enabled for all. So
> > we should have some counting here, also according to the
> > comment in  _setup_iclk_autoidle()
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---
> >   drivers/clk/ti/autoidle.c | 32 ++++++++++++++++++++++++--------
> >   include/linux/clk/ti.h    |  1 +
> >   2 files changed, 25 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/clk/ti/autoidle.c b/drivers/clk/ti/autoidle.c
> > index 7bb9afbe4058..bb6cff168e73 100644
> > --- a/drivers/clk/ti/autoidle.c
> > +++ b/drivers/clk/ti/autoidle.c
> > @@ -37,6 +37,14 @@ struct clk_ti_autoidle {
> >   static LIST_HEAD(autoidle_clks);
> >   static LIST_HEAD(clk_hw_omap_clocks);
> >   
> > +/*
> > + * we have some non-atomic read/write
> > + * operations behind it, so lets
> > + * take one mutex for handling autoidle
> > + * of all clocks
> > + */
> > +static DEFINE_MUTEX(autoidle_mutex);  
> 
> Why mutex? This prevents calling the autoidle APIs from atomic context. 
> Did you check the mutex debug kernel configs with this?
> 
Oops, I thought they were on, but they were not. OK,
I am preparing a v2 of this thing.


> This may cause problems with the runtime PM entries to the code at least.
> 
> 
> > +
> >   /**
> >    * omap2_clk_deny_idle - disable autoidle on an OMAP clock
> >    * @clk: struct clk * to disable autoidle for
> > @@ -48,8 +56,13 @@ int omap2_clk_deny_idle(struct clk *clk)
> >   	struct clk_hw_omap *c;
> >   
> >   	c = to_clk_hw_omap(__clk_get_hw(clk));
> > -	if (c->ops && c->ops->deny_idle)
> > -		c->ops->deny_idle(c);
> > +	if (c->ops && c->ops->deny_idle) {
> > +		mutex_lock(&autoidle_mutex);
> > +		c->autoidle_count--;
> > +		if (c->autoidle_count == -1)  
> 
> I think you should swap the arithmetics here, all the other usecounters 
> use positive values, here you enter deep to the negative side when 
> autoidle is denied by multiple users, which might be confusing.
> 
agreed.

Regards,
Andreas

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]