[PATCHv4 0/6] ARM: OMAP3+: IO daisychain support fixes

[PATCHv4 0/6] ARM: OMAP3+: IO daisychain support fixes

[Tero Kristo]

Hi,

Changes compared to previous version:

- patch2:
 * changed timeout value to 100us (from 1000)
 * added poll for the IO status == 0 at the end of trigger sequence
- patch4:
 * omap3: global wakeup done in PRM init instead of OMAP3 PM init
 * omap4: moved global wakeup enable from trigger func to PRM init

Tested on omap3beagle / omap4blaze, suspend / resume works with io chain.

-Tero

[PATCHv4 0/6] ARM: OMAP3+: IO daisychain support fixes

[Tero Kristo]

Hi,

Changes compared to previous version:

- patch2:
 * changed timeout value to 100us (from 1000)
 * added poll for the IO status == 0 at the end of trigger sequence
- patch4:
 * omap3: global wakeup done in PRM init instead of OMAP3 PM init
 * omap4: moved global wakeup enable from trigger func to PRM init

Tested on omap3beagle / omap4blaze, suspend / resume works with io chain.

-Tero

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Tero Kristo]

From: Mohan V <mohanv@ti.com>

Currently the enabling and disabling of IO Daisy chain is not
according to the TRM. The below steps are followed to enable/
disable the IO chain according to the "Sec 3.5.7.2.2
I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].

Steps to enable IO chain:
[a] Set PM_WKEN_WKUP.EN_IO bit
[b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
[c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
[d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
[e] Clear ST_IO_CHAIN bit.

Steps to disable IO chain:
[a] Clear PM_WKEN_WKUP.EN_IO_CHAIN bit
[b] Clear PM_WKEN_WKUP.EN_IO bit
[c] Clear PM_WKST_WKUP.ST_IO bit by writing 1 to it.

Step [e] & [c] in each case can be skipped, as these are handled
by the PRCM interrupt handler later.

[1] http://focus.ti.com/pdfs/wtbu/OMAP36xx_ES1.x_PUBLIC_TRM_vV.zip

Signed-off-by: Mohan V <mohanv@ti.com>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index fc69875..b0821a8 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -94,16 +94,17 @@ static void omap3_enable_io_chain(void)
 	/* Do a readback to assure write has been done */
 	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
 
-	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN) &
+	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
 		 OMAP3430_ST_IO_CHAIN_MASK)) {
 		timeout++;
 		if (timeout > 1000) {
 			pr_err("Wake up daisy chain activation failed.\n");
 			return;
 		}
-		omap2_prm_set_mod_reg_bits(OMAP3430_ST_IO_CHAIN_MASK,
-					   WKUP_MOD, PM_WKEN);
 	}
+	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+			PM_WKEN);
+
 }
 
 static void omap3_disable_io_chain(void)
-- 
1.7.4.1

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Tero Kristo]

From: Mohan V <mohanv@ti.com>

Currently the enabling and disabling of IO Daisy chain is not
according to the TRM. The below steps are followed to enable/
disable the IO chain according to the "Sec 3.5.7.2.2
I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].

Steps to enable IO chain:
[a] Set PM_WKEN_WKUP.EN_IO bit
[b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
[c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
[d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
[e] Clear ST_IO_CHAIN bit.

Steps to disable IO chain:
[a] Clear PM_WKEN_WKUP.EN_IO_CHAIN bit
[b] Clear PM_WKEN_WKUP.EN_IO bit
[c] Clear PM_WKST_WKUP.ST_IO bit by writing 1 to it.

Step [e] & [c] in each case can be skipped, as these are handled
by the PRCM interrupt handler later.

[1] http://focus.ti.com/pdfs/wtbu/OMAP36xx_ES1.x_PUBLIC_TRM_vV.zip

Signed-off-by: Mohan V <mohanv@ti.com>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index fc69875..b0821a8 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -94,16 +94,17 @@ static void omap3_enable_io_chain(void)
 	/* Do a readback to assure write has been done */
 	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
 
-	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN) &
+	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
 		 OMAP3430_ST_IO_CHAIN_MASK)) {
 		timeout++;
 		if (timeout > 1000) {
 			pr_err("Wake up daisy chain activation failed.\n");
 			return;
 		}
-		omap2_prm_set_mod_reg_bits(OMAP3430_ST_IO_CHAIN_MASK,
-					   WKUP_MOD, PM_WKEN);
 	}
+	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+			PM_WKEN);
+
 }
 
 static void omap3_disable_io_chain(void)
-- 
1.7.4.1

Re: [PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Paul Walmsley]

cc'ing Nilesh, Rajendra

Hi

On Fri, 2 Mar 2012, Tero Kristo wrote:

> From: Mohan V <mohanv@ti.com>
> 
> Currently the enabling and disabling of IO Daisy chain is not
> according to the TRM. The below steps are followed to enable/
> disable the IO chain according to the "Sec 3.5.7.2.2
> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
> 
> Steps to enable IO chain:
> [a] Set PM_WKEN_WKUP.EN_IO bit
> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN

Looking at the above TRM section, it doesn't mention clearing 
PM_WKEN_WKUP.EN_IO_CHAIN at all.  This only seems to be mentioned (in a 
rather unclear way) in the OMAP4430 TRM.

Since Tero and Rajendra are reporting that this series works, I assume 
that the conclusion is that this patch description just needs to be fixed.  
Could someone confirm that this is indeed the case -- that I/O wakeups are 
expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?

Is the I/O wakeup path from the pad to the PRCM wakeup line completely 
asynchronous?


- Paul

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Paul Walmsley]

cc'ing Nilesh, Rajendra

Hi

On Fri, 2 Mar 2012, Tero Kristo wrote:

> From: Mohan V <mohanv@ti.com>
> 
> Currently the enabling and disabling of IO Daisy chain is not
> according to the TRM. The below steps are followed to enable/
> disable the IO chain according to the "Sec 3.5.7.2.2
> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
> 
> Steps to enable IO chain:
> [a] Set PM_WKEN_WKUP.EN_IO bit
> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN

Looking at the above TRM section, it doesn't mention clearing 
PM_WKEN_WKUP.EN_IO_CHAIN at all.  This only seems to be mentioned (in a 
rather unclear way) in the OMAP4430 TRM.

Since Tero and Rajendra are reporting that this series works, I assume 
that the conclusion is that this patch description just needs to be fixed.  
Could someone confirm that this is indeed the case -- that I/O wakeups are 
expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?

Is the I/O wakeup path from the pad to the PRCM wakeup line completely 
asynchronous?


- Paul

Re: [PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

Hi Paul,

On Tuesday 06 March 2012 08:29 AM, Paul Walmsley wrote:
>
> cc'ing Nilesh, Rajendra
>
> Hi
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> From: Mohan V<mohanv@ti.com>
>>
>> Currently the enabling and disabling of IO Daisy chain is not
>> according to the TRM. The below steps are followed to enable/
>> disable the IO chain according to the "Sec 3.5.7.2.2
>> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
>>
>> Steps to enable IO chain:
>> [a] Set PM_WKEN_WKUP.EN_IO bit
>> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
>> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
>> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
>
> Looking at the above TRM section, it doesn't mention clearing
> PM_WKEN_WKUP.EN_IO_CHAIN at all.  This only seems to be mentioned (in a
> rather unclear way) in the OMAP4430 TRM.
>
> Since Tero and Rajendra are reporting that this series works, I assume
> that the conclusion is that this patch description just needs to be fixed.
> Could someone confirm that this is indeed the case -- that I/O wakeups are
> expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?

Yes, thats my understanding too, again based on taking to people like
Nilesh, because the documentation just doesn't mention this clearly.
The 4430 TRM is a tad bit better like you said.

I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
is also probably coming from the fact that the bit itself is called
*EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
compared to what it does.
That bit is actually used to send a WUCLK pulse through the chain, so
you set it to '1' and wait for it to propagate through the chain, then
you set it to '0' and again wait for it to propagate through.

The control to enable/disable is at 2 levels, a global switch which is
'EN_IO' in case of OMAP3 and the one at the individual pad level.

You can also read this thread which has some more explanation based on
my understanding of how this works
http://www.spinics.net/lists/linux-serial/msg04480.html

regards,
Rajendra

>
> Is the I/O wakeup path from the pad to the PRCM wakeup line completely
> asynchronous?
>
>
> - Paul

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

Hi Paul,

On Tuesday 06 March 2012 08:29 AM, Paul Walmsley wrote:
>
> cc'ing Nilesh, Rajendra
>
> Hi
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> From: Mohan V<mohanv@ti.com>
>>
>> Currently the enabling and disabling of IO Daisy chain is not
>> according to the TRM. The below steps are followed to enable/
>> disable the IO chain according to the "Sec 3.5.7.2.2
>> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
>>
>> Steps to enable IO chain:
>> [a] Set PM_WKEN_WKUP.EN_IO bit
>> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
>> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
>> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
>
> Looking at the above TRM section, it doesn't mention clearing
> PM_WKEN_WKUP.EN_IO_CHAIN at all.  This only seems to be mentioned (in a
> rather unclear way) in the OMAP4430 TRM.
>
> Since Tero and Rajendra are reporting that this series works, I assume
> that the conclusion is that this patch description just needs to be fixed.
> Could someone confirm that this is indeed the case -- that I/O wakeups are
> expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?

Yes, thats my understanding too, again based on taking to people like
Nilesh, because the documentation just doesn't mention this clearly.
The 4430 TRM is a tad bit better like you said.

I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
is also probably coming from the fact that the bit itself is called
*EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
compared to what it does.
That bit is actually used to send a WUCLK pulse through the chain, so
you set it to '1' and wait for it to propagate through the chain, then
you set it to '0' and again wait for it to propagate through.

The control to enable/disable is at 2 levels, a global switch which is
'EN_IO' in case of OMAP3 and the one at the individual pad level.

You can also read this thread which has some more explanation based on
my understanding of how this works
http://www.spinics.net/lists/linux-serial/msg04480.html

regards,
Rajendra

>
> Is the I/O wakeup path from the pad to the PRCM wakeup line completely
> asynchronous?
>
>
> - Paul

Re: [PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

On Tuesday 06 March 2012 09:23 AM, Rajendra Nayak wrote:
> Hi Paul,
>
> On Tuesday 06 March 2012 08:29 AM, Paul Walmsley wrote:
>>
>> cc'ing Nilesh, Rajendra

looks like you missed coping Nilesh. He's
copied in now.

>>
>> Hi
>>
>> On Fri, 2 Mar 2012, Tero Kristo wrote:
>>
>>> From: Mohan V<mohanv@ti.com>
>>>
>>> Currently the enabling and disabling of IO Daisy chain is not
>>> according to the TRM. The below steps are followed to enable/
>>> disable the IO chain according to the "Sec 3.5.7.2.2
>>> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
>>>
>>> Steps to enable IO chain:
>>> [a] Set PM_WKEN_WKUP.EN_IO bit
>>> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
>>> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
>>> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
>>
>> Looking at the above TRM section, it doesn't mention clearing
>> PM_WKEN_WKUP.EN_IO_CHAIN at all. This only seems to be mentioned (in a
>> rather unclear way) in the OMAP4430 TRM.
>>
>> Since Tero and Rajendra are reporting that this series works, I assume
>> that the conclusion is that this patch description just needs to be
>> fixed.
>> Could someone confirm that this is indeed the case -- that I/O wakeups
>> are
>> expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?
>
> Yes, thats my understanding too, again based on taking to people like
> Nilesh, because the documentation just doesn't mention this clearly.
> The 4430 TRM is a tad bit better like you said.
>
> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
> is also probably coming from the fact that the bit itself is called
> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
> compared to what it does.
> That bit is actually used to send a WUCLK pulse through the chain, so
> you set it to '1' and wait for it to propagate through the chain, then
> you set it to '0' and again wait for it to propagate through.
>
> The control to enable/disable is at 2 levels, a global switch which is
> 'EN_IO' in case of OMAP3 and the one at the individual pad level.
>
> You can also read this thread which has some more explanation based on
> my understanding of how this works
> http://www.spinics.net/lists/linux-serial/msg04480.html
>
> regards,
> Rajendra
>
>>
>> Is the I/O wakeup path from the pad to the PRCM wakeup line completely
>> asynchronous?
>>
>>
>> - Paul
>

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

On Tuesday 06 March 2012 09:23 AM, Rajendra Nayak wrote:
> Hi Paul,
>
> On Tuesday 06 March 2012 08:29 AM, Paul Walmsley wrote:
>>
>> cc'ing Nilesh, Rajendra

looks like you missed coping Nilesh. He's
copied in now.

>>
>> Hi
>>
>> On Fri, 2 Mar 2012, Tero Kristo wrote:
>>
>>> From: Mohan V<mohanv@ti.com>
>>>
>>> Currently the enabling and disabling of IO Daisy chain is not
>>> according to the TRM. The below steps are followed to enable/
>>> disable the IO chain according to the "Sec 3.5.7.2.2
>>> I/O Wake-Up Mechanism" in OMAP3630 Public TRM[1].
>>>
>>> Steps to enable IO chain:
>>> [a] Set PM_WKEN_WKUP.EN_IO bit
>>> [b] Set the PM_WKEN_WKUP.EN_IO_CHAIN bit
>>> [c] Poll for PM_WKST_WKUP.ST_IO_CHAIN.
>>> [d] When ST_IO_CHAIN bit set to 1, clear PM_WKEN_WKUP.EN_IO_CHAIN
>>
>> Looking at the above TRM section, it doesn't mention clearing
>> PM_WKEN_WKUP.EN_IO_CHAIN at all. This only seems to be mentioned (in a
>> rather unclear way) in the OMAP4430 TRM.
>>
>> Since Tero and Rajendra are reporting that this series works, I assume
>> that the conclusion is that this patch description just needs to be
>> fixed.
>> Could someone confirm that this is indeed the case -- that I/O wakeups
>> are
>> expected to work when EN_IO_CHAIN/WUCLK_CTRL is 0?
>
> Yes, thats my understanding too, again based on taking to people like
> Nilesh, because the documentation just doesn't mention this clearly.
> The 4430 TRM is a tad bit better like you said.
>
> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
> is also probably coming from the fact that the bit itself is called
> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
> compared to what it does.
> That bit is actually used to send a WUCLK pulse through the chain, so
> you set it to '1' and wait for it to propagate through the chain, then
> you set it to '0' and again wait for it to propagate through.
>
> The control to enable/disable is at 2 levels, a global switch which is
> 'EN_IO' in case of OMAP3 and the one at the individual pad level.
>
> You can also read this thread which has some more explanation based on
> my understanding of how this works
> http://www.spinics.net/lists/linux-serial/msg04480.html
>
> regards,
> Rajendra
>
>>
>> Is the I/O wakeup path from the pad to the PRCM wakeup line completely
>> asynchronous?
>>
>>
>> - Paul
>

Re: [PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Paul Walmsley]

Hi Rajendra

(thanks for adding in Nilesh, added him again here)

On Tue, 6 Mar 2012, Rajendra Nayak wrote:

> Yes, thats my understanding too, again based on taking to people like
> Nilesh, because the documentation just doesn't mention this clearly.
> The 4430 TRM is a tad bit better like you said.
> 
> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
> is also probably coming from the fact that the bit itself is called
> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
> compared to what it does.
> That bit is actually used to send a WUCLK pulse through the chain, so
> you set it to '1' and wait for it to propagate through the chain, then
> you set it to '0' and again wait for it to propagate through.

Okay.  So to confirm: EN_IO_CHAIN doesn't enable a continuous clock, it 
just controls the WUCLKIN line directly?

> The control to enable/disable is at 2 levels, a global switch which is
> 'EN_IO' in case of OMAP3 and the one at the individual pad level.

Do you happen to know what EN_IO/GLOBAL_WUEN does?  For example, if it's 
0, will the I/O chain not latch in wakeup events occuring on the pads?  
Or does it just cause the PRCM to ignore any PRCM wakeups coming from the 
I/O chain?  (Or something else, heh)

> You can also read this thread which has some more explanation based on
> my understanding of how this works
> http://www.spinics.net/lists/linux-serial/msg04480.html

I've been reading that post; it's been quite helpful.


- Paul

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Paul Walmsley]

Hi Rajendra

(thanks for adding in Nilesh, added him again here)

On Tue, 6 Mar 2012, Rajendra Nayak wrote:

> Yes, thats my understanding too, again based on taking to people like
> Nilesh, because the documentation just doesn't mention this clearly.
> The 4430 TRM is a tad bit better like you said.
> 
> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
> is also probably coming from the fact that the bit itself is called
> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
> compared to what it does.
> That bit is actually used to send a WUCLK pulse through the chain, so
> you set it to '1' and wait for it to propagate through the chain, then
> you set it to '0' and again wait for it to propagate through.

Okay.  So to confirm: EN_IO_CHAIN doesn't enable a continuous clock, it 
just controls the WUCLKIN line directly?

> The control to enable/disable is at 2 levels, a global switch which is
> 'EN_IO' in case of OMAP3 and the one at the individual pad level.

Do you happen to know what EN_IO/GLOBAL_WUEN does?  For example, if it's 
0, will the I/O chain not latch in wakeup events occuring on the pads?  
Or does it just cause the PRCM to ignore any PRCM wakeups coming from the 
I/O chain?  (Or something else, heh)

> You can also read this thread which has some more explanation based on
> my understanding of how this works
> http://www.spinics.net/lists/linux-serial/msg04480.html

I've been reading that post; it's been quite helpful.


- Paul

Re: [PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

On Tuesday 06 March 2012 09:43 AM, Paul Walmsley wrote:
> Hi Rajendra
>
> (thanks for adding in Nilesh, added him again here)
>
> On Tue, 6 Mar 2012, Rajendra Nayak wrote:
>
>> Yes, thats my understanding too, again based on taking to people like
>> Nilesh, because the documentation just doesn't mention this clearly.
>> The 4430 TRM is a tad bit better like you said.
>>
>> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
>> is also probably coming from the fact that the bit itself is called
>> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
>> compared to what it does.
>> That bit is actually used to send a WUCLK pulse through the chain, so
>> you set it to '1' and wait for it to propagate through the chain, then
>> you set it to '0' and again wait for it to propagate through.
>
> Okay.  So to confirm: EN_IO_CHAIN doesn't enable a continuous clock, it
> just controls the WUCLKIN line directly?

Right.

>
>> The control to enable/disable is at 2 levels, a global switch which is
>> 'EN_IO' in case of OMAP3 and the one at the individual pad level.
>
> Do you happen to know what EN_IO/GLOBAL_WUEN does?  For example, if it's
> 0, will the I/O chain not latch in wakeup events occuring on the pads?

Yes, it wouldn't. Figure 3-74 in OMAP4430_ES2.x Public TRM vO
explains this better.

The WUEN signal to each pad (which can be used to enable IO-daisy
for the given pad) is a AND of a signal from PRCM and
one from SCM.
The signal from PRCM can be enabled by writing into
EN_IO/GLOBAL_WUEN bit.
The signal from SCM can be enabled by writing to the individual
pad registers in SCM
CONTROL.CONTROL_PADCONF_<IOpad>[14] WAKEUPENABLE0
CONTROL.CONTROL_PADCONF_<IOpad>[30] WAKEUPENABLE1.

If either one of them is not enabled the IO wakeups on the pads won't
be latched. So if EN_IO/GLOBAL_WUEN is disabled, none of the pads will
be able to generate IO wakeups.

> Or does it just cause the PRCM to ignore any PRCM wakeups coming from the
> I/O chain?  (Or something else, heh)
>
>> You can also read this thread which has some more explanation based on
>> my understanding of how this works
>> http://www.spinics.net/lists/linux-serial/msg04480.html
>
> I've been reading that post; it's been quite helpful.
>
>
> - Paul

[PATCHv4 1/6] ARM: OMAP3 PM: correct enable/disable of daisy io chain

[Rajendra Nayak]

On Tuesday 06 March 2012 09:43 AM, Paul Walmsley wrote:
> Hi Rajendra
>
> (thanks for adding in Nilesh, added him again here)
>
> On Tue, 6 Mar 2012, Rajendra Nayak wrote:
>
>> Yes, thats my understanding too, again based on taking to people like
>> Nilesh, because the documentation just doesn't mention this clearly.
>> The 4430 TRM is a tad bit better like you said.
>>
>> I guess your confusion of IO wakeups working with EN_IO_CHAIN set to '0'
>> is also probably coming from the fact that the bit itself is called
>> *EN_IO_CHAIN* which is a completely *wrong* name for the bit, as
>> compared to what it does.
>> That bit is actually used to send a WUCLK pulse through the chain, so
>> you set it to '1' and wait for it to propagate through the chain, then
>> you set it to '0' and again wait for it to propagate through.
>
> Okay.  So to confirm: EN_IO_CHAIN doesn't enable a continuous clock, it
> just controls the WUCLKIN line directly?

Right.

>
>> The control to enable/disable is at 2 levels, a global switch which is
>> 'EN_IO' in case of OMAP3 and the one at the individual pad level.
>
> Do you happen to know what EN_IO/GLOBAL_WUEN does?  For example, if it's
> 0, will the I/O chain not latch in wakeup events occuring on the pads?

Yes, it wouldn't. Figure 3-74 in OMAP4430_ES2.x Public TRM vO
explains this better.

The WUEN signal to each pad (which can be used to enable IO-daisy
for the given pad) is a AND of a signal from PRCM and
one from SCM.
The signal from PRCM can be enabled by writing into
EN_IO/GLOBAL_WUEN bit.
The signal from SCM can be enabled by writing to the individual
pad registers in SCM
CONTROL.CONTROL_PADCONF_<IOpad>[14] WAKEUPENABLE0
CONTROL.CONTROL_PADCONF_<IOpad>[30] WAKEUPENABLE1.

If either one of them is not enabled the IO wakeups on the pads won't
be latched. So if EN_IO/GLOBAL_WUEN is disabled, none of the pads will
be able to generate IO wakeups.

> Or does it just cause the PRCM to ignore any PRCM wakeups coming from the
> I/O chain?  (Or something else, heh)
>
>> You can also read this thread which has some more explanation based on
>> my understanding of how this works
>> http://www.spinics.net/lists/linux-serial/msg04480.html
>
> I've been reading that post; it's been quite helpful.
>
>
> - Paul

[PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

Since IO Daisychain modifies only PRM registers, it makes sense to move
it to PRM File. Also changed the timeout value for IO chain enable to
100us and added a wait for status disable at the end.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c       |   30 +-----------------------------
 arch/arm/mach-omap2/prm2xxx_3xxx.c |   36 ++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/prm2xxx_3xxx.h |   14 ++++++++++++++
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index b0821a8..e97ec3f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -85,34 +85,6 @@ static inline void omap3_per_restore_context(void)
 	omap_gpio_restore_context();
 }
 
-static void omap3_enable_io_chain(void)
-{
-	int timeout = 0;
-
-	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-				   PM_WKEN);
-	/* Do a readback to assure write has been done */
-	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
-
-	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
-		 OMAP3430_ST_IO_CHAIN_MASK)) {
-		timeout++;
-		if (timeout > 1000) {
-			pr_err("Wake up daisy chain activation failed.\n");
-			return;
-		}
-	}
-	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-			PM_WKEN);
-
-}
-
-static void omap3_disable_io_chain(void)
-{
-	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-				     PM_WKEN);
-}
-
 static void omap3_core_save_context(void)
 {
 	omap3_ctrl_save_padconf();
@@ -324,7 +296,7 @@ void omap_sram_idle(void)
 	     core_next_state < PWRDM_POWER_ON)) {
 		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN);
 		if (omap3_has_io_chain_ctrl())
-			omap3_enable_io_chain();
+			omap3_trigger_io_chain();
 	}
 
 	pwrdm_pre_transition();
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index 9ce7654..4f4f08a 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -301,6 +301,42 @@ void omap3xxx_prm_restore_irqen(u32 *saved_mask)
 				OMAP3_PRM_IRQENABLE_MPU_OFFSET);
 }
 
+/*
+ * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
+ * the I/O ring after asserting WUCLKIN high
+ */
+#define MAX_IOPAD_LATCH_TIME 100
+
+/* OMAP3 Daisychain enable sequence */
+void omap3_trigger_io_chain(void)
+{
+	int i = 0;
+
+	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+				   PM_WKEN);
+	/* Do a readback to assure write has been done */
+	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
+
+	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
+			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
+			  MAX_IOPAD_LATCH_TIME, i);
+
+	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+				     PM_WKEN);
+
+	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
+			    OMAP3430_ST_IO_CHAIN_MASK) == 0),
+			  MAX_IOPAD_LATCH_TIME, i);
+}
+
+/* OMAP3 Daisychain disable sequence */
+void omap3_disable_io_chain(void)
+{
+	if (omap_rev() >= OMAP3430_REV_ES3_1)
+		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK,
+					     WKUP_MOD, PM_WKEN);
+}
+
 static int __init omap3xxx_prcm_init(void)
 {
 	if (cpu_is_omap34xx())
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.h b/arch/arm/mach-omap2/prm2xxx_3xxx.h
index 70ac2a1..15973aa 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.h
@@ -289,6 +289,18 @@ static inline int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift,
 		"not suppose to be used on omap4\n");
 	return 0;
 }
+static inline void omap3_trigger_io_chain(void)
+{
+	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
+		"not suppose to be used on omap4\n");
+	return 0;
+}
+static inline void omap3_disable_io_chain(void)
+{
+	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
+		"not suppose to be used on omap4\n");
+	return 0;
+}
 #else
 /* Power/reset management domain register get/set */
 extern u32 omap2_prm_read_mod_reg(s16 module, u16 idx);
@@ -302,6 +314,8 @@ extern u32 omap2_prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
 extern int omap2_prm_is_hardreset_asserted(s16 prm_mod, u8 shift);
 extern int omap2_prm_assert_hardreset(s16 prm_mod, u8 shift);
 extern int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift, u8 st_shift);
+extern void omap3_trigger_io_chain(void);
+extern void omap3_disable_io_chain(void);
 
 /* OMAP3-specific VP functions */
 u32 omap3_prm_vp_check_txdone(u8 vp_id);
-- 
1.7.4.1

[PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

Since IO Daisychain modifies only PRM registers, it makes sense to move
it to PRM File. Also changed the timeout value for IO chain enable to
100us and added a wait for status disable at the end.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c       |   30 +-----------------------------
 arch/arm/mach-omap2/prm2xxx_3xxx.c |   36 ++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/prm2xxx_3xxx.h |   14 ++++++++++++++
 3 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index b0821a8..e97ec3f 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -85,34 +85,6 @@ static inline void omap3_per_restore_context(void)
 	omap_gpio_restore_context();
 }
 
-static void omap3_enable_io_chain(void)
-{
-	int timeout = 0;
-
-	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-				   PM_WKEN);
-	/* Do a readback to assure write has been done */
-	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
-
-	while (!(omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
-		 OMAP3430_ST_IO_CHAIN_MASK)) {
-		timeout++;
-		if (timeout > 1000) {
-			pr_err("Wake up daisy chain activation failed.\n");
-			return;
-		}
-	}
-	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-			PM_WKEN);
-
-}
-
-static void omap3_disable_io_chain(void)
-{
-	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
-				     PM_WKEN);
-}
-
 static void omap3_core_save_context(void)
 {
 	omap3_ctrl_save_padconf();
@@ -324,7 +296,7 @@ void omap_sram_idle(void)
 	     core_next_state < PWRDM_POWER_ON)) {
 		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN);
 		if (omap3_has_io_chain_ctrl())
-			omap3_enable_io_chain();
+			omap3_trigger_io_chain();
 	}
 
 	pwrdm_pre_transition();
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index 9ce7654..4f4f08a 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -301,6 +301,42 @@ void omap3xxx_prm_restore_irqen(u32 *saved_mask)
 				OMAP3_PRM_IRQENABLE_MPU_OFFSET);
 }
 
+/*
+ * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
+ * the I/O ring after asserting WUCLKIN high
+ */
+#define MAX_IOPAD_LATCH_TIME 100
+
+/* OMAP3 Daisychain enable sequence */
+void omap3_trigger_io_chain(void)
+{
+	int i = 0;
+
+	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+				   PM_WKEN);
+	/* Do a readback to assure write has been done */
+	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
+
+	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
+			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
+			  MAX_IOPAD_LATCH_TIME, i);
+
+	omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
+				     PM_WKEN);
+
+	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
+			    OMAP3430_ST_IO_CHAIN_MASK) == 0),
+			  MAX_IOPAD_LATCH_TIME, i);
+}
+
+/* OMAP3 Daisychain disable sequence */
+void omap3_disable_io_chain(void)
+{
+	if (omap_rev() >= OMAP3430_REV_ES3_1)
+		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK,
+					     WKUP_MOD, PM_WKEN);
+}
+
 static int __init omap3xxx_prcm_init(void)
 {
 	if (cpu_is_omap34xx())
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.h b/arch/arm/mach-omap2/prm2xxx_3xxx.h
index 70ac2a1..15973aa 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.h
@@ -289,6 +289,18 @@ static inline int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift,
 		"not suppose to be used on omap4\n");
 	return 0;
 }
+static inline void omap3_trigger_io_chain(void)
+{
+	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
+		"not suppose to be used on omap4\n");
+	return 0;
+}
+static inline void omap3_disable_io_chain(void)
+{
+	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
+		"not suppose to be used on omap4\n");
+	return 0;
+}
 #else
 /* Power/reset management domain register get/set */
 extern u32 omap2_prm_read_mod_reg(s16 module, u16 idx);
@@ -302,6 +314,8 @@ extern u32 omap2_prm_read_mod_bits_shift(s16 domain, s16 idx, u32 mask);
 extern int omap2_prm_is_hardreset_asserted(s16 prm_mod, u8 shift);
 extern int omap2_prm_assert_hardreset(s16 prm_mod, u8 shift);
 extern int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift, u8 st_shift);
+extern void omap3_trigger_io_chain(void);
+extern void omap3_disable_io_chain(void);
 
 /* OMAP3-specific VP functions */
 u32 omap3_prm_vp_check_txdone(u8 vp_id);
-- 
1.7.4.1

Re: [PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Nishanth Menon]

On 17:17-20120302, Tero Kristo wrote:
> From: Vishwanath BS <vishwanath.bs@ti.com>
> 
> Since IO Daisychain modifies only PRM registers, it makes sense to move
> it to PRM File. Also changed the timeout value for IO chain enable to
> 100us and added a wait for status disable at the end.
> 
[...]
> +/*
> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
> + * the I/O ring after asserting WUCLKIN high
> + */
> +#define MAX_IOPAD_LATCH_TIME 100
> +
> +/* OMAP3 Daisychain enable sequence */
> +void omap3_trigger_io_chain(void)
> +{
> +	int i = 0;
> +
> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
> +				   PM_WKEN);
> +	/* Do a readback to assure write has been done */
> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
> +
> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
umm
arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
(1 << 16)
(reg & (1 << 16)) == 1
will ever be true?

-- 
Regards,
Nishanth Menon

[PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Nishanth Menon]

On 17:17-20120302, Tero Kristo wrote:
> From: Vishwanath BS <vishwanath.bs@ti.com>
> 
> Since IO Daisychain modifies only PRM registers, it makes sense to move
> it to PRM File. Also changed the timeout value for IO chain enable to
> 100us and added a wait for status disable at the end.
> 
[...]
> +/*
> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
> + * the I/O ring after asserting WUCLKIN high
> + */
> +#define MAX_IOPAD_LATCH_TIME 100
> +
> +/* OMAP3 Daisychain enable sequence */
> +void omap3_trigger_io_chain(void)
> +{
> +	int i = 0;
> +
> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
> +				   PM_WKEN);
> +	/* Do a readback to assure write has been done */
> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
> +
> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST) &
> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
umm
arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
(1 << 16)
(reg & (1 << 16)) == 1
will ever be true?

-- 
Regards,
Nishanth Menon

Re: [PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Rajendra Nayak]

On Tuesday 06 March 2012 11:14 AM, Nishanth Menon wrote:
> On 17:17-20120302, Tero Kristo wrote:
>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> Since IO Daisychain modifies only PRM registers, it makes sense to move
>> it to PRM File. Also changed the timeout value for IO chain enable to
>> 100us and added a wait for status disable at the end.
>>
> [...]
>> +/*
>> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
>> + * the I/O ring after asserting WUCLKIN high
>> + */
>> +#define MAX_IOPAD_LATCH_TIME 100
>> +
>> +/* OMAP3 Daisychain enable sequence */
>> +void omap3_trigger_io_chain(void)
>> +{
>> +	int i = 0;
>> +
>> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
>> +				   PM_WKEN);
>> +	/* Do a readback to assure write has been done */
>> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
>> +
>> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST)&
>> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
> umm
> arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
> (1<<  16)
> (reg&  (1<<  16)) == 1
> will ever be true?

good catch. Its should actually be '((reg & (1 << 16)) >> 16) == 1'
This is fixed in the omap4_trigger_io() function, looks like is missed
here.

>

[PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Rajendra Nayak]

On Tuesday 06 March 2012 11:14 AM, Nishanth Menon wrote:
> On 17:17-20120302, Tero Kristo wrote:
>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> Since IO Daisychain modifies only PRM registers, it makes sense to move
>> it to PRM File. Also changed the timeout value for IO chain enable to
>> 100us and added a wait for status disable at the end.
>>
> [...]
>> +/*
>> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
>> + * the I/O ring after asserting WUCLKIN high
>> + */
>> +#define MAX_IOPAD_LATCH_TIME 100
>> +
>> +/* OMAP3 Daisychain enable sequence */
>> +void omap3_trigger_io_chain(void)
>> +{
>> +	int i = 0;
>> +
>> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
>> +				   PM_WKEN);
>> +	/* Do a readback to assure write has been done */
>> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
>> +
>> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST)&
>> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
> umm
> arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
> (1<<  16)
> (reg&  (1<<  16)) == 1
> will ever be true?

good catch. Its should actually be '((reg & (1 << 16)) >> 16) == 1'
This is fixed in the omap4_trigger_io() function, looks like is missed
here.

>

Re: [PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Tero Kristo]

On Tue, 2012-03-06 at 11:30 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 11:14 AM, Nishanth Menon wrote:
> > On 17:17-20120302, Tero Kristo wrote:
> >> From: Vishwanath BS<vishwanath.bs@ti.com>
> >>
> >> Since IO Daisychain modifies only PRM registers, it makes sense to move
> >> it to PRM File. Also changed the timeout value for IO chain enable to
> >> 100us and added a wait for status disable at the end.
> >>
> > [...]
> >> +/*
> >> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
> >> + * the I/O ring after asserting WUCLKIN high
> >> + */
> >> +#define MAX_IOPAD_LATCH_TIME 100
> >> +
> >> +/* OMAP3 Daisychain enable sequence */
> >> +void omap3_trigger_io_chain(void)
> >> +{
> >> +	int i = 0;
> >> +
> >> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
> >> +				   PM_WKEN);
> >> +	/* Do a readback to assure write has been done */
> >> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
> >> +
> >> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST)&
> >> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
> > umm
> > arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
> > (1<<  16)
> > (reg&  (1<<  16)) == 1
> > will ever be true?
> 
> good catch. Its should actually be '((reg & (1 << 16)) >> 16) == 1'
> This is fixed in the omap4_trigger_io() function, looks like is missed
> here.

I'll fix this for next version.

-Tero

[PATCHv4 2/6] ARM: OMAP3 PM: Move IO Daisychain function to omap3 prm file

[Tero Kristo]

On Tue, 2012-03-06 at 11:30 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 11:14 AM, Nishanth Menon wrote:
> > On 17:17-20120302, Tero Kristo wrote:
> >> From: Vishwanath BS<vishwanath.bs@ti.com>
> >>
> >> Since IO Daisychain modifies only PRM registers, it makes sense to move
> >> it to PRM File. Also changed the timeout value for IO chain enable to
> >> 100us and added a wait for status disable at the end.
> >>
> > [...]
> >> +/*
> >> + * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
> >> + * the I/O ring after asserting WUCLKIN high
> >> + */
> >> +#define MAX_IOPAD_LATCH_TIME 100
> >> +
> >> +/* OMAP3 Daisychain enable sequence */
> >> +void omap3_trigger_io_chain(void)
> >> +{
> >> +	int i = 0;
> >> +
> >> +	omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK, WKUP_MOD,
> >> +				   PM_WKEN);
> >> +	/* Do a readback to assure write has been done */
> >> +	omap2_prm_read_mod_reg(WKUP_MOD, PM_WKEN);
> >> +
> >> +	omap_test_timeout(((omap2_prm_read_mod_reg(WKUP_MOD, PM_WKST)&
> >> +			    OMAP3430_ST_IO_CHAIN_MASK) == 1),
> > umm
> > arch/arm/mach-omap2/prm-regbits-34xx.h:#define OMAP3430_ST_IO_CHAIN_MASK
> > (1<<  16)
> > (reg&  (1<<  16)) == 1
> > will ever be true?
> 
> good catch. Its should actually be '((reg & (1 << 16)) >> 16) == 1'
> This is fixed in the omap4_trigger_io() function, looks like is missed
> here.

I'll fix this for next version.

-Tero

[PATCHv4 3/6] ARM: OMAP4 PM: Add IO Daisychain support

[Tero Kristo]

From: Rajendra Nayak <rnayak@ti.com>

IO daisychain is a mechanism that allows individual IO pads to generate
wakeup events on their own based on a switch of an input signal level.
This allows the hardware module behind the pad to be powered down, but
still have device level capability to detect IO events, and once this
happens the module can be powered back up to resume IO. See section
3.9.4 in OMAP4430 Public TRM for details.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/prm44xx.c |   41 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/prm44xx.h |    1 +
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c
index a37bfd4..caa5e0f 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -231,6 +231,47 @@ void omap44xx_prm_restore_irqen(u32 *saved_mask)
 				 OMAP4_PRM_IRQENABLE_MPU_2_OFFSET);
 }
 
+/*
+ * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
+ * the I/O ring after asserting WUCLKIN high
+ */
+#define MAX_IOPAD_LATCH_TIME 100
+
+/* OMAP4 IO Daisychain trigger sequence */
+void omap4_trigger_io_chain(void)
+{
+	int i = 0;
+
+	/* Enable GLOBAL_WUEN */
+	if (!(omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+		OMAP4_PRM_IO_PMCTRL_OFFSET) & OMAP4430_GLOBAL_WUEN_MASK))
+		omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
+			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+
+	/* Trigger WUCLKIN enable */
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK,
+			OMAP4430_WUCLK_CTRL_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+	omap_test_timeout(
+		(((omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET) &
+			OMAP4430_WUCLK_STATUS_MASK) >>
+			OMAP4430_WUCLK_STATUS_SHIFT) == 1),
+		MAX_IOPAD_LATCH_TIME, i);
+
+	/* Trigger WUCLKIN disable */
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK, 0x0,
+			OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_IO_PMCTRL_OFFSET);
+	omap_test_timeout(
+		(((omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET) &
+			OMAP4430_WUCLK_STATUS_MASK) >>
+			OMAP4430_WUCLK_STATUS_SHIFT) == 0),
+		MAX_IOPAD_LATCH_TIME, i);
+	return;
+}
+
 static int __init omap4xxx_prcm_init(void)
 {
 	if (cpu_is_omap44xx())
diff --git a/arch/arm/mach-omap2/prm44xx.h b/arch/arm/mach-omap2/prm44xx.h
index 7978092..54a057e 100644
--- a/arch/arm/mach-omap2/prm44xx.h
+++ b/arch/arm/mach-omap2/prm44xx.h
@@ -762,6 +762,7 @@ void omap4_prm_vp_clear_txdone(u8 vp_id);
 extern u32 omap4_prm_vcvp_read(u8 offset);
 extern void omap4_prm_vcvp_write(u32 val, u8 offset);
 extern u32 omap4_prm_vcvp_rmw(u32 mask, u32 bits, u8 offset);
+extern void omap4_trigger_io_chain(void);
 
 /* PRM interrupt-related functions */
 extern void omap44xx_prm_read_pending_irqs(unsigned long *events);
-- 
1.7.4.1

[PATCHv4 3/6] ARM: OMAP4 PM: Add IO Daisychain support

[Tero Kristo]

From: Rajendra Nayak <rnayak@ti.com>

IO daisychain is a mechanism that allows individual IO pads to generate
wakeup events on their own based on a switch of an input signal level.
This allows the hardware module behind the pad to be powered down, but
still have device level capability to detect IO events, and once this
happens the module can be powered back up to resume IO. See section
3.9.4 in OMAP4430 Public TRM for details.

Signed-off-by: Rajendra Nayak <rnayak@ti.com>
Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/prm44xx.c |   41 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/mach-omap2/prm44xx.h |    1 +
 2 files changed, 42 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c
index a37bfd4..caa5e0f 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -231,6 +231,47 @@ void omap44xx_prm_restore_irqen(u32 *saved_mask)
 				 OMAP4_PRM_IRQENABLE_MPU_2_OFFSET);
 }
 
+/*
+ * Maximum time(us) it takes to output the signal WUCLKOUT of the last pad of
+ * the I/O ring after asserting WUCLKIN high
+ */
+#define MAX_IOPAD_LATCH_TIME 100
+
+/* OMAP4 IO Daisychain trigger sequence */
+void omap4_trigger_io_chain(void)
+{
+	int i = 0;
+
+	/* Enable GLOBAL_WUEN */
+	if (!(omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+		OMAP4_PRM_IO_PMCTRL_OFFSET) & OMAP4430_GLOBAL_WUEN_MASK))
+		omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
+			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+
+	/* Trigger WUCLKIN enable */
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK,
+			OMAP4430_WUCLK_CTRL_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+	omap_test_timeout(
+		(((omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET) &
+			OMAP4430_WUCLK_STATUS_MASK) >>
+			OMAP4430_WUCLK_STATUS_SHIFT) == 1),
+		MAX_IOPAD_LATCH_TIME, i);
+
+	/* Trigger WUCLKIN disable */
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK, 0x0,
+			OMAP4430_PRM_DEVICE_INST, OMAP4_PRM_IO_PMCTRL_OFFSET);
+	omap_test_timeout(
+		(((omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET) &
+			OMAP4430_WUCLK_STATUS_MASK) >>
+			OMAP4430_WUCLK_STATUS_SHIFT) == 0),
+		MAX_IOPAD_LATCH_TIME, i);
+	return;
+}
+
 static int __init omap4xxx_prcm_init(void)
 {
 	if (cpu_is_omap44xx())
diff --git a/arch/arm/mach-omap2/prm44xx.h b/arch/arm/mach-omap2/prm44xx.h
index 7978092..54a057e 100644
--- a/arch/arm/mach-omap2/prm44xx.h
+++ b/arch/arm/mach-omap2/prm44xx.h
@@ -762,6 +762,7 @@ void omap4_prm_vp_clear_txdone(u8 vp_id);
 extern u32 omap4_prm_vcvp_read(u8 offset);
 extern void omap4_prm_vcvp_write(u32 val, u8 offset);
 extern u32 omap4_prm_vcvp_rmw(u32 mask, u32 bits, u8 offset);
+extern void omap4_trigger_io_chain(void);
 
 /* PRM interrupt-related functions */
 extern void omap44xx_prm_read_pending_irqs(unsigned long *events);
-- 
1.7.4.1

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
managed in cpuidle path which is not the right place. Subsequent patch
will remove IO Daisy chain handling in cpuidle path once daisy chain is
handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
enable code from the trigger function to init time setup.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/prm2xxx_3xxx.c |   11 ++++++++++-
 arch/arm/mach-omap2/prm44xx.c      |   18 ++++++++++--------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index 4f4f08a..f681628 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -337,10 +337,19 @@ void omap3_disable_io_chain(void)
 					     WKUP_MOD, PM_WKEN);
 }
 
+static void __init omap3_enable_io_wakeup(void)
+{
+	if (omap3_has_io_wakeup())
+		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD,
+					   PM_WKEN);
+}
+
 static int __init omap3xxx_prcm_init(void)
 {
-	if (cpu_is_omap34xx())
+	if (cpu_is_omap34xx()) {
+		omap3_enable_io_wakeup();
 		return omap_prcm_register_chain_handler(&omap3_prcm_irq_setup);
+	}
 	return 0;
 }
 subsys_initcall(omap3xxx_prcm_init);
diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c
index caa5e0f..4ce6f08 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -242,13 +242,6 @@ void omap4_trigger_io_chain(void)
 {
 	int i = 0;
 
-	/* Enable GLOBAL_WUEN */
-	if (!(omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
-		OMAP4_PRM_IO_PMCTRL_OFFSET) & OMAP4430_GLOBAL_WUEN_MASK))
-		omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
-			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
-			OMAP4_PRM_IO_PMCTRL_OFFSET);
-
 	/* Trigger WUCLKIN enable */
 	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK,
 			OMAP4430_WUCLK_CTRL_MASK, OMAP4430_PRM_DEVICE_INST,
@@ -272,10 +265,19 @@ void omap4_trigger_io_chain(void)
 	return;
 }
 
+static void __init omap4_enable_io_wakeup(void)
+{
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
+			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+}
+
 static int __init omap4xxx_prcm_init(void)
 {
-	if (cpu_is_omap44xx())
+	if (cpu_is_omap44xx()) {
+		omap4_enable_io_wakeup();
 		return omap_prcm_register_chain_handler(&omap4_prcm_irq_setup);
+	}
 	return 0;
 }
 subsys_initcall(omap4xxx_prcm_init);
-- 
1.7.4.1

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
managed in cpuidle path which is not the right place. Subsequent patch
will remove IO Daisy chain handling in cpuidle path once daisy chain is
handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
enable code from the trigger function to init time setup.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/prm2xxx_3xxx.c |   11 ++++++++++-
 arch/arm/mach-omap2/prm44xx.c      |   18 ++++++++++--------
 2 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index 4f4f08a..f681628 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -337,10 +337,19 @@ void omap3_disable_io_chain(void)
 					     WKUP_MOD, PM_WKEN);
 }
 
+static void __init omap3_enable_io_wakeup(void)
+{
+	if (omap3_has_io_wakeup())
+		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD,
+					   PM_WKEN);
+}
+
 static int __init omap3xxx_prcm_init(void)
 {
-	if (cpu_is_omap34xx())
+	if (cpu_is_omap34xx()) {
+		omap3_enable_io_wakeup();
 		return omap_prcm_register_chain_handler(&omap3_prcm_irq_setup);
+	}
 	return 0;
 }
 subsys_initcall(omap3xxx_prcm_init);
diff --git a/arch/arm/mach-omap2/prm44xx.c b/arch/arm/mach-omap2/prm44xx.c
index caa5e0f..4ce6f08 100644
--- a/arch/arm/mach-omap2/prm44xx.c
+++ b/arch/arm/mach-omap2/prm44xx.c
@@ -242,13 +242,6 @@ void omap4_trigger_io_chain(void)
 {
 	int i = 0;
 
-	/* Enable GLOBAL_WUEN */
-	if (!(omap4_prm_read_inst_reg(OMAP4430_PRM_DEVICE_INST,
-		OMAP4_PRM_IO_PMCTRL_OFFSET) & OMAP4430_GLOBAL_WUEN_MASK))
-		omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
-			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
-			OMAP4_PRM_IO_PMCTRL_OFFSET);
-
 	/* Trigger WUCLKIN enable */
 	omap4_prm_rmw_inst_reg_bits(OMAP4430_WUCLK_CTRL_MASK,
 			OMAP4430_WUCLK_CTRL_MASK, OMAP4430_PRM_DEVICE_INST,
@@ -272,10 +265,19 @@ void omap4_trigger_io_chain(void)
 	return;
 }
 
+static void __init omap4_enable_io_wakeup(void)
+{
+	omap4_prm_rmw_inst_reg_bits(OMAP4430_GLOBAL_WUEN_MASK,
+			OMAP4430_GLOBAL_WUEN_MASK, OMAP4430_PRM_DEVICE_INST,
+			OMAP4_PRM_IO_PMCTRL_OFFSET);
+}
+
 static int __init omap4xxx_prcm_init(void)
 {
-	if (cpu_is_omap44xx())
+	if (cpu_is_omap44xx()) {
+		omap4_enable_io_wakeup();
 		return omap_prcm_register_chain_handler(&omap4_prcm_irq_setup);
+	}
 	return 0;
 }
 subsys_initcall(omap4xxx_prcm_init);
-- 
1.7.4.1

Re: [PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Paul Walmsley]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1748 bytes --]

added Rajendra, Nilesh, Vishwa, Mohan

Hi

On Fri, 2 Mar 2012, Tero Kristo wrote:

> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
> managed in cpuidle path which is not the right place. Subsequent patch
> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> enable code from the trigger function to init time setup.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Should we keep the I/O wakeups enabled all the time?

Seems like that could result in at least one spurious wake-up event -- and 
thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.  
Seems like these interrupts could recur every time the I/O chain is reset.  
And that the I/O chain is now reset in every hwmod idle and enable 
operation for an IP block that contains dynamic mux data?

Thinking about the problem naïvely... maybe you all can think through this 
with me:

Consider an IP block 'A' that has a signal (like the UART rx_irrx signal) 
that's mux'ed to a pad with WAKEUPENABLE set.  (Some examples of 'A' might 
include a UART, a GPIO, or a McBSP.)  

Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only 
immediately before the powerdomain containing IP block 'A' is going to 
transition into RETENTION or OFF?

If we do that, then we can avoid needlessly resetting the I/O chain when a 
powerdomain is not going to go into a low-power state.

I haven't measured the time it takes for the I/O chain to be reset.  
Maybe one of you have.  But that 100 microsecond timeout that was 
specified in two of the other patches in this series has me a little 
worried.


- Paul

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Paul Walmsley]

added Rajendra, Nilesh, Vishwa, Mohan

Hi

On Fri, 2 Mar 2012, Tero Kristo wrote:

> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
> managed in cpuidle path which is not the right place. Subsequent patch
> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> enable code from the trigger function to init time setup.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Should we keep the I/O wakeups enabled all the time?

Seems like that could result in at least one spurious wake-up event -- and 
thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.  
Seems like these interrupts could recur every time the I/O chain is reset.  
And that the I/O chain is now reset in every hwmod idle and enable 
operation for an IP block that contains dynamic mux data?

Thinking about the problem na?vely... maybe you all can think through this 
with me:

Consider an IP block 'A' that has a signal (like the UART rx_irrx signal) 
that's mux'ed to a pad with WAKEUPENABLE set.  (Some examples of 'A' might 
include a UART, a GPIO, or a McBSP.)  

Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only 
immediately before the powerdomain containing IP block 'A' is going to 
transition into RETENTION or OFF?

If we do that, then we can avoid needlessly resetting the I/O chain when a 
powerdomain is not going to go into a low-power state.

I haven't measured the time it takes for the I/O chain to be reset.  
Maybe one of you have.  But that 100 microsecond timeout that was 
specified in two of the other patches in this series has me a little 
worried.


- Paul

Re: [PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Rajendra Nayak]

On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> added Rajendra, Nilesh, Vishwa, Mohan
>
> Hi
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
>> managed in cpuidle path which is not the right place. Subsequent patch
>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
>> enable code from the trigger function to init time setup.
>>
>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>
> Should we keep the I/O wakeups enabled all the time?
>
> Seems like that could result in at least one spurious wake-up event -- and
> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> Seems like these interrupts could recur every time the I/O chain is reset.
> And that the I/O chain is now reset in every hwmod idle and enable
> operation for an IP block that contains dynamic mux data?
>
> Thinking about the problem naïvely... maybe you all can think through this
> with me:
>
> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> that's mux'ed to a pad with WAKEUPENABLE set.  (Some examples of 'A' might
> include a UART, a GPIO, or a McBSP.)
>
> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> immediately before the powerdomain containing IP block 'A' is going to
> transition into RETENTION or OFF?

Hi Paul,

I completely agree with your observations and we knew we would have
additional wakeups with this design. Like I mentioned in the other
thread, we went ahead with this approach knowing we need to optimize
with some kind of powerdomain level usecounting in the future, because
it didn't exist back then when we did it this way.
But now that we already have it, maybe we can fix this and do
it such that we completely avoid an additional wakeup interrupt.

>
> If we do that, then we can avoid needlessly resetting the I/O chain when a
> powerdomain is not going to go into a low-power state.
>
> I haven't measured the time it takes for the I/O chain to be reset.
> Maybe one of you have.  But that 100 microsecond timeout that was
> specified in two of the other patches in this series has me a little
> worried.

I haven't profiled it myself but I am guessing the delay isn't much.
The 100us comes from the fact that there is no documentation on the
exact delay, so went around asking whats the *real worst case* delay,
and we got an answer, 100us should be really big enough :)

regards,
Rajendra

>
>
> - Paul

--
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

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Rajendra Nayak]

On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> added Rajendra, Nilesh, Vishwa, Mohan
>
> Hi
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has been
>> managed in cpuidle path which is not the right place. Subsequent patch
>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
>> enable code from the trigger function to init time setup.
>>
>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>
> Should we keep the I/O wakeups enabled all the time?
>
> Seems like that could result in at least one spurious wake-up event -- and
> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> Seems like these interrupts could recur every time the I/O chain is reset.
> And that the I/O chain is now reset in every hwmod idle and enable
> operation for an IP block that contains dynamic mux data?
>
> Thinking about the problem na?vely... maybe you all can think through this
> with me:
>
> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> that's mux'ed to a pad with WAKEUPENABLE set.  (Some examples of 'A' might
> include a UART, a GPIO, or a McBSP.)
>
> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> immediately before the powerdomain containing IP block 'A' is going to
> transition into RETENTION or OFF?

Hi Paul,

I completely agree with your observations and we knew we would have
additional wakeups with this design. Like I mentioned in the other
thread, we went ahead with this approach knowing we need to optimize
with some kind of powerdomain level usecounting in the future, because
it didn't exist back then when we did it this way.
But now that we already have it, maybe we can fix this and do
it such that we completely avoid an additional wakeup interrupt.

>
> If we do that, then we can avoid needlessly resetting the I/O chain when a
> powerdomain is not going to go into a low-power state.
>
> I haven't measured the time it takes for the I/O chain to be reset.
> Maybe one of you have.  But that 100 microsecond timeout that was
> specified in two of the other patches in this series has me a little
> worried.

I haven't profiled it myself but I am guessing the delay isn't much.
The 100us comes from the fact that there is no documentation on the
exact delay, so went around asking whats the *real worst case* delay,
and we got an answer, 100us should be really big enough :)

regards,
Rajendra

>
>
> - Paul

Re: [PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Rajendra Nayak]

On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
>> added Rajendra, Nilesh, Vishwa, Mohan
>>
>> Hi
>>
>> On Fri, 2 Mar 2012, Tero Kristo wrote:
>>
>>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
>>> been
>>> managed in cpuidle path which is not the right place. Subsequent patch
>>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
>>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
>>> enable code from the trigger function to init time setup.
>>>
>>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>>
>> Should we keep the I/O wakeups enabled all the time?
>>
>> Seems like that could result in at least one spurious wake-up event --
>> and
>> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
>> Seems like these interrupts could recur every time the I/O chain is
>> reset.
>> And that the I/O chain is now reset in every hwmod idle and enable
>> operation for an IP block that contains dynamic mux data?
>>
>> Thinking about the problem naïvely... maybe you all can think through
>> this
>> with me:
>>
>> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
>> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
>> include a UART, a GPIO, or a McBSP.)
>>
>> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
>> immediately before the powerdomain containing IP block 'A' is going to
>> transition into RETENTION or OFF?
>
> Hi Paul,
>
> I completely agree with your observations and we knew we would have
> additional wakeups with this design. Like I mentioned in the other
> thread, we went ahead with this approach knowing we need to optimize
> with some kind of powerdomain level usecounting in the future, because
> it didn't exist back then when we did it this way.
> But now that we already have it, maybe we can fix this and do
> it such that we completely avoid an additional wakeup interrupt.
>
>>
>> If we do that, then we can avoid needlessly resetting the I/O chain
>> when a
>> powerdomain is not going to go into a low-power state.
>>
>> I haven't measured the time it takes for the I/O chain to be reset.
>> Maybe one of you have. But that 100 microsecond timeout that was
>> specified in two of the other patches in this series has me a little
>> worried.
>
> I haven't profiled it myself but I am guessing the delay isn't much.

Having said that, I do agree with you that, however small the delay,
we end up doing needless IO trigger when the powerdomain isn't
really entering a low power state.

> The 100us comes from the fact that there is no documentation on the
> exact delay, so went around asking whats the *real worst case* delay,
> and we got an answer, 100us should be really big enough :)
>
> regards,
> Rajendra
>
>>
>>
>> - Paul
>

--
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

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Rajendra Nayak]

On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
>> added Rajendra, Nilesh, Vishwa, Mohan
>>
>> Hi
>>
>> On Fri, 2 Mar 2012, Tero Kristo wrote:
>>
>>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
>>> been
>>> managed in cpuidle path which is not the right place. Subsequent patch
>>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
>>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
>>> enable code from the trigger function to init time setup.
>>>
>>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>>
>> Should we keep the I/O wakeups enabled all the time?
>>
>> Seems like that could result in at least one spurious wake-up event --
>> and
>> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
>> Seems like these interrupts could recur every time the I/O chain is
>> reset.
>> And that the I/O chain is now reset in every hwmod idle and enable
>> operation for an IP block that contains dynamic mux data?
>>
>> Thinking about the problem na?vely... maybe you all can think through
>> this
>> with me:
>>
>> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
>> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
>> include a UART, a GPIO, or a McBSP.)
>>
>> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
>> immediately before the powerdomain containing IP block 'A' is going to
>> transition into RETENTION or OFF?
>
> Hi Paul,
>
> I completely agree with your observations and we knew we would have
> additional wakeups with this design. Like I mentioned in the other
> thread, we went ahead with this approach knowing we need to optimize
> with some kind of powerdomain level usecounting in the future, because
> it didn't exist back then when we did it this way.
> But now that we already have it, maybe we can fix this and do
> it such that we completely avoid an additional wakeup interrupt.
>
>>
>> If we do that, then we can avoid needlessly resetting the I/O chain
>> when a
>> powerdomain is not going to go into a low-power state.
>>
>> I haven't measured the time it takes for the I/O chain to be reset.
>> Maybe one of you have. But that 100 microsecond timeout that was
>> specified in two of the other patches in this series has me a little
>> worried.
>
> I haven't profiled it myself but I am guessing the delay isn't much.

Having said that, I do agree with you that, however small the delay,
we end up doing needless IO trigger when the powerdomain isn't
really entering a low power state.

> The 100us comes from the fact that there is no documentation on the
> exact delay, so went around asking whats the *real worst case* delay,
> and we got an answer, 100us should be really big enough :)
>
> regards,
> Rajendra
>
>>
>>
>> - Paul
>

Re: [PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

On Tue, 2012-03-06 at 10:26 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> > On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> >> added Rajendra, Nilesh, Vishwa, Mohan
> >>
> >> Hi
> >>
> >> On Fri, 2 Mar 2012, Tero Kristo wrote:
> >>
> >>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
> >>> been
> >>> managed in cpuidle path which is not the right place. Subsequent patch
> >>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> >>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> >>> enable code from the trigger function to init time setup.
> >>>
> >>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >>
> >> Should we keep the I/O wakeups enabled all the time?
> >>
> >> Seems like that could result in at least one spurious wake-up event --
> >> and
> >> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> >> Seems like these interrupts could recur every time the I/O chain is
> >> reset.
> >> And that the I/O chain is now reset in every hwmod idle and enable
> >> operation for an IP block that contains dynamic mux data?
> >>
> >> Thinking about the problem naïvely... maybe you all can think through
> >> this
> >> with me:
> >>
> >> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> >> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
> >> include a UART, a GPIO, or a McBSP.)
> >>
> >> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> >> immediately before the powerdomain containing IP block 'A' is going to
> >> transition into RETENTION or OFF?
> >
> > Hi Paul,
> >
> > I completely agree with your observations and we knew we would have
> > additional wakeups with this design. Like I mentioned in the other
> > thread, we went ahead with this approach knowing we need to optimize
> > with some kind of powerdomain level usecounting in the future, because
> > it didn't exist back then when we did it this way.
> > But now that we already have it, maybe we can fix this and do
> > it such that we completely avoid an additional wakeup interrupt.
> >
> >>
> >> If we do that, then we can avoid needlessly resetting the I/O chain
> >> when a
> >> powerdomain is not going to go into a low-power state.
> >>
> >> I haven't measured the time it takes for the I/O chain to be reset.
> >> Maybe one of you have. But that 100 microsecond timeout that was
> >> specified in two of the other patches in this series has me a little
> >> worried.
> >
> > I haven't profiled it myself but I am guessing the delay isn't much.
> 
> Having said that, I do agree with you that, however small the delay,
> we end up doing needless IO trigger when the powerdomain isn't
> really entering a low power state.

I did some measurements on this a while back and the delay was around a
few microseconds. I can redo this while doing version 5.

-Tero

> 
> > The 100us comes from the fact that there is no documentation on the
> > exact delay, so went around asking whats the *real worst case* delay,
> > and we got an answer, 100us should be really big enough :)
> >
> > regards,
> > Rajendra
> >
> >>
> >>
> >> - Paul
> >
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

On Tue, 2012-03-06 at 10:26 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> > On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> >> added Rajendra, Nilesh, Vishwa, Mohan
> >>
> >> Hi
> >>
> >> On Fri, 2 Mar 2012, Tero Kristo wrote:
> >>
> >>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
> >>> been
> >>> managed in cpuidle path which is not the right place. Subsequent patch
> >>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> >>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> >>> enable code from the trigger function to init time setup.
> >>>
> >>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >>
> >> Should we keep the I/O wakeups enabled all the time?
> >>
> >> Seems like that could result in at least one spurious wake-up event --
> >> and
> >> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> >> Seems like these interrupts could recur every time the I/O chain is
> >> reset.
> >> And that the I/O chain is now reset in every hwmod idle and enable
> >> operation for an IP block that contains dynamic mux data?
> >>
> >> Thinking about the problem na?vely... maybe you all can think through
> >> this
> >> with me:
> >>
> >> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> >> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
> >> include a UART, a GPIO, or a McBSP.)
> >>
> >> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> >> immediately before the powerdomain containing IP block 'A' is going to
> >> transition into RETENTION or OFF?
> >
> > Hi Paul,
> >
> > I completely agree with your observations and we knew we would have
> > additional wakeups with this design. Like I mentioned in the other
> > thread, we went ahead with this approach knowing we need to optimize
> > with some kind of powerdomain level usecounting in the future, because
> > it didn't exist back then when we did it this way.
> > But now that we already have it, maybe we can fix this and do
> > it such that we completely avoid an additional wakeup interrupt.
> >
> >>
> >> If we do that, then we can avoid needlessly resetting the I/O chain
> >> when a
> >> powerdomain is not going to go into a low-power state.
> >>
> >> I haven't measured the time it takes for the I/O chain to be reset.
> >> Maybe one of you have. But that 100 microsecond timeout that was
> >> specified in two of the other patches in this series has me a little
> >> worried.
> >
> > I haven't profiled it myself but I am guessing the delay isn't much.
> 
> Having said that, I do agree with you that, however small the delay,
> we end up doing needless IO trigger when the powerdomain isn't
> really entering a low power state.

I did some measurements on this a while back and the delay was around a
few microseconds. I can redo this while doing version 5.

-Tero

> 
> > The 100us comes from the fact that there is no documentation on the
> > exact delay, so went around asking whats the *real worst case* delay,
> > and we got an answer, 100us should be really big enough :)
> >
> > regards,
> > Rajendra
> >
> >>
> >>
> >> - Paul
> >
> 

Re: [PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

On Tue, 2012-03-06 at 10:26 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> > On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> >> added Rajendra, Nilesh, Vishwa, Mohan
> >>
> >> Hi
> >>
> >> On Fri, 2 Mar 2012, Tero Kristo wrote:
> >>
> >>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
> >>> been
> >>> managed in cpuidle path which is not the right place. Subsequent patch
> >>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> >>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> >>> enable code from the trigger function to init time setup.
> >>>
> >>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >>
> >> Should we keep the I/O wakeups enabled all the time?
> >>
> >> Seems like that could result in at least one spurious wake-up event --
> >> and
> >> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> >> Seems like these interrupts could recur every time the I/O chain is
> >> reset.
> >> And that the I/O chain is now reset in every hwmod idle and enable
> >> operation for an IP block that contains dynamic mux data?
> >>
> >> Thinking about the problem naïvely... maybe you all can think through
> >> this
> >> with me:
> >>
> >> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> >> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
> >> include a UART, a GPIO, or a McBSP.)
> >>
> >> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> >> immediately before the powerdomain containing IP block 'A' is going to
> >> transition into RETENTION or OFF?
> >
> > Hi Paul,
> >
> > I completely agree with your observations and we knew we would have
> > additional wakeups with this design. Like I mentioned in the other
> > thread, we went ahead with this approach knowing we need to optimize
> > with some kind of powerdomain level usecounting in the future, because
> > it didn't exist back then when we did it this way.
> > But now that we already have it, maybe we can fix this and do
> > it such that we completely avoid an additional wakeup interrupt.
> >
> >>
> >> If we do that, then we can avoid needlessly resetting the I/O chain
> >> when a
> >> powerdomain is not going to go into a low-power state.
> >>
> >> I haven't measured the time it takes for the I/O chain to be reset.
> >> Maybe one of you have. But that 100 microsecond timeout that was
> >> specified in two of the other patches in this series has me a little
> >> worried.
> >
> > I haven't profiled it myself but I am guessing the delay isn't much.
> 
> Having said that, I do agree with you that, however small the delay,
> we end up doing needless IO trigger when the powerdomain isn't
> really entering a low power state.

After some measurements (used ARM performance counters for this), it
looks like omap4 takes between 2...4us for the IO chain trigger. omap3
takes 7...8us, most of which appears to be delay caused by interconnect,
the poll loops don't actually have time to do anything. Speaking of
which, the trigger function was pretty broken for omap3 on this set, I
noticed that while doing the profiling. The fixed one takes this time,
I'll post a new version of the whole set shortly.

-Tero

> 
> > The 100us comes from the fact that there is no documentation on the
> > exact delay, so went around asking whats the *real worst case* delay,
> > and we got an answer, 100us should be really big enough :)
> >
> > regards,
> > Rajendra
> >
> >>
> >>
> >> - Paul
> >
> 


--
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

[PATCHv4 4/6] ARM: OMAP3+: PRM: Enable IO wake up

[Tero Kristo]

On Tue, 2012-03-06 at 10:26 +0530, Rajendra Nayak wrote:
> On Tuesday 06 March 2012 10:20 AM, Rajendra Nayak wrote:
> > On Tuesday 06 March 2012 09:51 AM, Paul Walmsley wrote:
> >> added Rajendra, Nilesh, Vishwa, Mohan
> >>
> >> Hi
> >>
> >> On Fri, 2 Mar 2012, Tero Kristo wrote:
> >>
> >>> Enable IO Wake up for OMAP3/4 as part of PRM Init. Currently this has
> >>> been
> >>> managed in cpuidle path which is not the right place. Subsequent patch
> >>> will remove IO Daisy chain handling in cpuidle path once daisy chain is
> >>> handled as part of hwmod mux. This patch also moves the OMAP4 IO wakeup
> >>> enable code from the trigger function to init time setup.
> >>>
> >>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >>
> >> Should we keep the I/O wakeups enabled all the time?
> >>
> >> Seems like that could result in at least one spurious wake-up event --
> >> and
> >> thus a PRCM interrupt -- for each I/O pad that has WAKEUPENABLE set.
> >> Seems like these interrupts could recur every time the I/O chain is
> >> reset.
> >> And that the I/O chain is now reset in every hwmod idle and enable
> >> operation for an IP block that contains dynamic mux data?
> >>
> >> Thinking about the problem na?vely... maybe you all can think through
> >> this
> >> with me:
> >>
> >> Consider an IP block 'A' that has a signal (like the UART rx_irrx signal)
> >> that's mux'ed to a pad with WAKEUPENABLE set. (Some examples of 'A' might
> >> include a UART, a GPIO, or a McBSP.)
> >>
> >> Shouldn't we enable I/O wakeups (by setting EN_IO/GLOBAL_WUEN) only
> >> immediately before the powerdomain containing IP block 'A' is going to
> >> transition into RETENTION or OFF?
> >
> > Hi Paul,
> >
> > I completely agree with your observations and we knew we would have
> > additional wakeups with this design. Like I mentioned in the other
> > thread, we went ahead with this approach knowing we need to optimize
> > with some kind of powerdomain level usecounting in the future, because
> > it didn't exist back then when we did it this way.
> > But now that we already have it, maybe we can fix this and do
> > it such that we completely avoid an additional wakeup interrupt.
> >
> >>
> >> If we do that, then we can avoid needlessly resetting the I/O chain
> >> when a
> >> powerdomain is not going to go into a low-power state.
> >>
> >> I haven't measured the time it takes for the I/O chain to be reset.
> >> Maybe one of you have. But that 100 microsecond timeout that was
> >> specified in two of the other patches in this series has me a little
> >> worried.
> >
> > I haven't profiled it myself but I am guessing the delay isn't much.
> 
> Having said that, I do agree with you that, however small the delay,
> we end up doing needless IO trigger when the powerdomain isn't
> really entering a low power state.

After some measurements (used ARM performance counters for this), it
looks like omap4 takes between 2...4us for the IO chain trigger. omap3
takes 7...8us, most of which appears to be delay caused by interconnect,
the poll loops don't actually have time to do anything. Speaking of
which, the trigger function was pretty broken for omap3 on this set, I
noticed that while doing the profiling. The fixed one takes this time,
I'll post a new version of the whole set shortly.

-Tero

> 
> > The 100us comes from the fact that there is no documentation on the
> > exact delay, so went around asking whats the *real worst case* delay,
> > and we got an answer, 100us should be really big enough :)
> >
> > regards,
> > Rajendra
> >
> >>
> >>
> >> - Paul
> >
> 

[PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

IO Daisychain feature has to be triggered whenever there is a change in
device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).

Now devices can idle independent of the powerdomain, there can be a
window where device is idled and corresponding powerdomain can be
ON/INACTIVE state. In such situations, since both module wake up is
enabled at padlevel as well as io daisychain sequence is triggered,
there will be 2 PRCM interrupts (Module async wake up via swakeup and
IO Pad interrupt). But as PRCM Interrupt handler clears the Module
Padlevel WKST bit in the first interrupt, module specific interrupt
handler will not triggered for the second time

Also look at detailed explanation given by Rajendra at
http://www.spinics.net/lists/linux-serial/msg04480.html

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
 arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
 arch/arm/mach-omap2/pm.h         |    1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5192cab..56adbfb 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -152,6 +152,7 @@
 #include "prm44xx.h"
 #include "prminst44xx.h"
 #include "mux.h"
+#include "pm.h"
 
 /* Maximum microseconds to wait for OMAP module to softreset */
 #define MAX_MODULE_SOFTRESET_WAIT	10000
@@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
 			((oh->_state == _HWMOD_STATE_IDLE) &&
-			 oh->mux->pads_dynamic)))
+			 oh->mux->pads_dynamic))) {
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
+		omap_trigger_io_chain();
+	}
 
 	_add_initiator_dep(oh, mpu_oh);
 
@@ -1622,8 +1625,10 @@ static int _idle(struct omap_hwmod *oh)
 		clkdm_hwmod_disable(oh->clkdm, oh);
 
 	/* Mux pins for device idle if populated */
-	if (oh->mux && oh->mux->pads_dynamic)
+	if (oh->mux && oh->mux->pads_dynamic) {
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_IDLE);
+		omap_trigger_io_chain();
+	}
 
 	oh->_state = _HWMOD_STATE_IDLE;
 
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 1881fe9..4f1dcb5 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -25,8 +25,11 @@
 #include "clockdomain.h"
 #include "pm.h"
 #include "twl-common.h"
+#include "prm2xxx_3xxx.h"
+#include "prm44xx.h"
 
 static struct omap_device_pm_latency *pm_lats;
+static void (*io_chain_trigger_func) (void);
 
 static int _init_omap_device(char *name)
 {
@@ -64,6 +67,12 @@ static void omap2_init_processor_devices(void)
 	}
 }
 
+void omap_trigger_io_chain(void)
+{
+	if (io_chain_trigger_func)
+		io_chain_trigger_func();
+}
+
 /* Types of sleep_switch used in omap_set_pwrdm_state */
 #define FORCEWAKEUP_SWITCH	0
 #define LOWPOWERSTATE_SWITCH	1
@@ -221,6 +230,12 @@ static int __init omap2_common_pm_init(void)
 		omap2_init_processor_devices();
 	omap_pm_if_init();
 
+	if (cpu_is_omap34xx() && omap3_has_io_chain_ctrl())
+		io_chain_trigger_func = omap3_trigger_io_chain;
+
+	if (cpu_is_omap44xx())
+		io_chain_trigger_func = omap4_trigger_io_chain;
+
 	return 0;
 }
 postcore_initcall(omap2_common_pm_init);
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index b737b11..f9dac40 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -22,6 +22,7 @@ extern int omap3_can_sleep(void);
 extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
 extern int omap3_idle_init(void);
 extern int omap4_idle_init(void);
+extern void omap_trigger_io_chain(void);
 
 #if defined(CONFIG_PM_OPP)
 extern int omap3_opp_init(void);
-- 
1.7.4.1

[PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

IO Daisychain feature has to be triggered whenever there is a change in
device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).

Now devices can idle independent of the powerdomain, there can be a
window where device is idled and corresponding powerdomain can be
ON/INACTIVE state. In such situations, since both module wake up is
enabled at padlevel as well as io daisychain sequence is triggered,
there will be 2 PRCM interrupts (Module async wake up via swakeup and
IO Pad interrupt). But as PRCM Interrupt handler clears the Module
Padlevel WKST bit in the first interrupt, module specific interrupt
handler will not triggered for the second time

Also look at detailed explanation given by Rajendra at
http://www.spinics.net/lists/linux-serial/msg04480.html

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
 arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
 arch/arm/mach-omap2/pm.h         |    1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
index 5192cab..56adbfb 100644
--- a/arch/arm/mach-omap2/omap_hwmod.c
+++ b/arch/arm/mach-omap2/omap_hwmod.c
@@ -152,6 +152,7 @@
 #include "prm44xx.h"
 #include "prminst44xx.h"
 #include "mux.h"
+#include "pm.h"
 
 /* Maximum microseconds to wait for OMAP module to softreset */
 #define MAX_MODULE_SOFTRESET_WAIT	10000
@@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
 	/* Mux pins for device runtime if populated */
 	if (oh->mux && (!oh->mux->enabled ||
 			((oh->_state == _HWMOD_STATE_IDLE) &&
-			 oh->mux->pads_dynamic)))
+			 oh->mux->pads_dynamic))) {
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
+		omap_trigger_io_chain();
+	}
 
 	_add_initiator_dep(oh, mpu_oh);
 
@@ -1622,8 +1625,10 @@ static int _idle(struct omap_hwmod *oh)
 		clkdm_hwmod_disable(oh->clkdm, oh);
 
 	/* Mux pins for device idle if populated */
-	if (oh->mux && oh->mux->pads_dynamic)
+	if (oh->mux && oh->mux->pads_dynamic) {
 		omap_hwmod_mux(oh->mux, _HWMOD_STATE_IDLE);
+		omap_trigger_io_chain();
+	}
 
 	oh->_state = _HWMOD_STATE_IDLE;
 
diff --git a/arch/arm/mach-omap2/pm.c b/arch/arm/mach-omap2/pm.c
index 1881fe9..4f1dcb5 100644
--- a/arch/arm/mach-omap2/pm.c
+++ b/arch/arm/mach-omap2/pm.c
@@ -25,8 +25,11 @@
 #include "clockdomain.h"
 #include "pm.h"
 #include "twl-common.h"
+#include "prm2xxx_3xxx.h"
+#include "prm44xx.h"
 
 static struct omap_device_pm_latency *pm_lats;
+static void (*io_chain_trigger_func) (void);
 
 static int _init_omap_device(char *name)
 {
@@ -64,6 +67,12 @@ static void omap2_init_processor_devices(void)
 	}
 }
 
+void omap_trigger_io_chain(void)
+{
+	if (io_chain_trigger_func)
+		io_chain_trigger_func();
+}
+
 /* Types of sleep_switch used in omap_set_pwrdm_state */
 #define FORCEWAKEUP_SWITCH	0
 #define LOWPOWERSTATE_SWITCH	1
@@ -221,6 +230,12 @@ static int __init omap2_common_pm_init(void)
 		omap2_init_processor_devices();
 	omap_pm_if_init();
 
+	if (cpu_is_omap34xx() && omap3_has_io_chain_ctrl())
+		io_chain_trigger_func = omap3_trigger_io_chain;
+
+	if (cpu_is_omap44xx())
+		io_chain_trigger_func = omap4_trigger_io_chain;
+
 	return 0;
 }
 postcore_initcall(omap2_common_pm_init);
diff --git a/arch/arm/mach-omap2/pm.h b/arch/arm/mach-omap2/pm.h
index b737b11..f9dac40 100644
--- a/arch/arm/mach-omap2/pm.h
+++ b/arch/arm/mach-omap2/pm.h
@@ -22,6 +22,7 @@ extern int omap3_can_sleep(void);
 extern int omap_set_pwrdm_state(struct powerdomain *pwrdm, u32 state);
 extern int omap3_idle_init(void);
 extern int omap4_idle_init(void);
+extern void omap_trigger_io_chain(void);
 
 #if defined(CONFIG_PM_OPP)
 extern int omap3_opp_init(void);
-- 
1.7.4.1

Re: [PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Paul Walmsley]

Hi

a few comments:

On Fri, 2 Mar 2012, Tero Kristo wrote:

> From: Vishwanath BS <vishwanath.bs@ti.com>
> 
> IO Daisychain feature has to be triggered whenever there is a change in
> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
> 
> Now devices can idle independent of the powerdomain, there can be a
> window where device is idled and corresponding powerdomain can be
> ON/INACTIVE state. In such situations, since both module wake up is
> enabled at padlevel as well as io daisychain sequence is triggered,
> there will be 2 PRCM interrupts (Module async wake up via swakeup and
> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
> Padlevel WKST bit in the first interrupt, module specific interrupt
> handler will not triggered for the second time
> 
> Also look at detailed explanation given by Rajendra at
> http://www.spinics.net/lists/linux-serial/msg04480.html
> 
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
>  arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
>  arch/arm/mach-omap2/pm.h         |    1 +
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5192cab..56adbfb 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c

...

> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
>  	/* Mux pins for device runtime if populated */
>  	if (oh->mux && (!oh->mux->enabled ||
>  			((oh->_state == _HWMOD_STATE_IDLE) &&
> -			 oh->mux->pads_dynamic)))
> +			 oh->mux->pads_dynamic))) {
>  		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
> +		omap_trigger_io_chain();

Looks racy: if two hwmods with dynamic mux entries go idle at the same 
time, or one goes idle while another one is enabled, won't the calls to 
omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking 
in omap_trigger_io_chain() or the functions it calls.

Also, won't this result in needless resets of the I/O chain?  Seems like 
we'd only need to do this when the next power state of the enclosing 
powerdomain will enter either RETENTION or OFF.  And even then, it 
presumably should only happen when the last active device in that 
powerdomain is going idle?


- Paul

[PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Paul Walmsley]

Hi

a few comments:

On Fri, 2 Mar 2012, Tero Kristo wrote:

> From: Vishwanath BS <vishwanath.bs@ti.com>
> 
> IO Daisychain feature has to be triggered whenever there is a change in
> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
> 
> Now devices can idle independent of the powerdomain, there can be a
> window where device is idled and corresponding powerdomain can be
> ON/INACTIVE state. In such situations, since both module wake up is
> enabled at padlevel as well as io daisychain sequence is triggered,
> there will be 2 PRCM interrupts (Module async wake up via swakeup and
> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
> Padlevel WKST bit in the first interrupt, module specific interrupt
> handler will not triggered for the second time
> 
> Also look at detailed explanation given by Rajendra at
> http://www.spinics.net/lists/linux-serial/msg04480.html
> 
> Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
>  arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
>  arch/arm/mach-omap2/pm.h         |    1 +
>  3 files changed, 23 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> index 5192cab..56adbfb 100644
> --- a/arch/arm/mach-omap2/omap_hwmod.c
> +++ b/arch/arm/mach-omap2/omap_hwmod.c

...

> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
>  	/* Mux pins for device runtime if populated */
>  	if (oh->mux && (!oh->mux->enabled ||
>  			((oh->_state == _HWMOD_STATE_IDLE) &&
> -			 oh->mux->pads_dynamic)))
> +			 oh->mux->pads_dynamic))) {
>  		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
> +		omap_trigger_io_chain();

Looks racy: if two hwmods with dynamic mux entries go idle at the same 
time, or one goes idle while another one is enabled, won't the calls to 
omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking 
in omap_trigger_io_chain() or the functions it calls.

Also, won't this result in needless resets of the I/O chain?  Seems like 
we'd only need to do this when the next power state of the enclosing 
powerdomain will enter either RETENTION or OFF.  And even then, it 
presumably should only happen when the last active device in that 
powerdomain is going idle?


- Paul

Re: [PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Rajendra Nayak]

Hi Paul,

On Tuesday 06 March 2012 09:32 AM, Paul Walmsley wrote:
> Hi
>
> a few comments:
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> IO Daisychain feature has to be triggered whenever there is a change in
>> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
>>
>> Now devices can idle independent of the powerdomain, there can be a
>> window where device is idled and corresponding powerdomain can be
>> ON/INACTIVE state. In such situations, since both module wake up is
>> enabled at padlevel as well as io daisychain sequence is triggered,
>> there will be 2 PRCM interrupts (Module async wake up via swakeup and
>> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
>> Padlevel WKST bit in the first interrupt, module specific interrupt
>> handler will not triggered for the second time
>>
>> Also look at detailed explanation given by Rajendra at
>> http://www.spinics.net/lists/linux-serial/msg04480.html
>>
>> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
>>   arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
>>   arch/arm/mach-omap2/pm.h         |    1 +
>>   3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 5192cab..56adbfb 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>
> ...
>
>> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
>>   	/* Mux pins for device runtime if populated */
>>   	if (oh->mux&&  (!oh->mux->enabled ||
>>   			((oh->_state == _HWMOD_STATE_IDLE)&&
>> -			 oh->mux->pads_dynamic)))
>> +			 oh->mux->pads_dynamic))) {
>>   		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
>> +		omap_trigger_io_chain();
>
> Looks racy: if two hwmods with dynamic mux entries go idle at the same
> time, or one goes idle while another one is enabled, won't the calls to
> omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking
> in omap_trigger_io_chain() or the functions it calls.

I agree, this needs locking to avoid races.

>
> Also, won't this result in needless resets of the I/O chain?  Seems like
> we'd only need to do this when the next power state of the enclosing
> powerdomain will enter either RETENTION or OFF.  And even then, it
> presumably should only happen when the last active device in that
> powerdomain is going idle?

Yes, the module async wakeups will work as long as the power domain
enclosing the module is not in OSWR or OFF, so ideally this trigger
should happen only when all modules in the given powerdomain are
disabled and we plan to program the Powerdomain down to OSWR or
OFF state. With what we are doing today we end up with periods when we
have multiple wakeups (a module wakeup as well as an IO wakeup).
The last we discussed this with Kevin, there wasn't a better place where
we could trigger this, with no usecounting at powerdomain level you
didn't know when the last active device in the powerdomain was going
idle.
But now with Tero's series which adds usecounting at power/voltage
domain level, maybe its possible, but I need to look more.
Do you already have an idea on where this would fit better, so we
avoid this multiple wakeup scenario?

regards,
Rajendra

>
>
> - Paul

[PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Rajendra Nayak]

Hi Paul,

On Tuesday 06 March 2012 09:32 AM, Paul Walmsley wrote:
> Hi
>
> a few comments:
>
> On Fri, 2 Mar 2012, Tero Kristo wrote:
>
>> From: Vishwanath BS<vishwanath.bs@ti.com>
>>
>> IO Daisychain feature has to be triggered whenever there is a change in
>> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
>>
>> Now devices can idle independent of the powerdomain, there can be a
>> window where device is idled and corresponding powerdomain can be
>> ON/INACTIVE state. In such situations, since both module wake up is
>> enabled at padlevel as well as io daisychain sequence is triggered,
>> there will be 2 PRCM interrupts (Module async wake up via swakeup and
>> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
>> Padlevel WKST bit in the first interrupt, module specific interrupt
>> handler will not triggered for the second time
>>
>> Also look at detailed explanation given by Rajendra at
>> http://www.spinics.net/lists/linux-serial/msg04480.html
>>
>> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
>> Signed-off-by: Tero Kristo<t-kristo@ti.com>
>> ---
>>   arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
>>   arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
>>   arch/arm/mach-omap2/pm.h         |    1 +
>>   3 files changed, 23 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
>> index 5192cab..56adbfb 100644
>> --- a/arch/arm/mach-omap2/omap_hwmod.c
>> +++ b/arch/arm/mach-omap2/omap_hwmod.c
>
> ...
>
>> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
>>   	/* Mux pins for device runtime if populated */
>>   	if (oh->mux&&  (!oh->mux->enabled ||
>>   			((oh->_state == _HWMOD_STATE_IDLE)&&
>> -			 oh->mux->pads_dynamic)))
>> +			 oh->mux->pads_dynamic))) {
>>   		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
>> +		omap_trigger_io_chain();
>
> Looks racy: if two hwmods with dynamic mux entries go idle at the same
> time, or one goes idle while another one is enabled, won't the calls to
> omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking
> in omap_trigger_io_chain() or the functions it calls.

I agree, this needs locking to avoid races.

>
> Also, won't this result in needless resets of the I/O chain?  Seems like
> we'd only need to do this when the next power state of the enclosing
> powerdomain will enter either RETENTION or OFF.  And even then, it
> presumably should only happen when the last active device in that
> powerdomain is going idle?

Yes, the module async wakeups will work as long as the power domain
enclosing the module is not in OSWR or OFF, so ideally this trigger
should happen only when all modules in the given powerdomain are
disabled and we plan to program the Powerdomain down to OSWR or
OFF state. With what we are doing today we end up with periods when we
have multiple wakeups (a module wakeup as well as an IO wakeup).
The last we discussed this with Kevin, there wasn't a better place where
we could trigger this, with no usecounting at powerdomain level you
didn't know when the last active device in the powerdomain was going
idle.
But now with Tero's series which adds usecounting at power/voltage
domain level, maybe its possible, but I need to look more.
Do you already have an idea on where this would fit better, so we
avoid this multiple wakeup scenario?

regards,
Rajendra

>
>
> - Paul

Re: [PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Tero Kristo]

On Tue, 2012-03-06 at 09:51 +0530, Rajendra Nayak wrote:
> Hi Paul,
> 
> On Tuesday 06 March 2012 09:32 AM, Paul Walmsley wrote:
> > Hi
> >
> > a few comments:
> >
> > On Fri, 2 Mar 2012, Tero Kristo wrote:
> >
> >> From: Vishwanath BS<vishwanath.bs@ti.com>
> >>
> >> IO Daisychain feature has to be triggered whenever there is a change in
> >> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
> >>
> >> Now devices can idle independent of the powerdomain, there can be a
> >> window where device is idled and corresponding powerdomain can be
> >> ON/INACTIVE state. In such situations, since both module wake up is
> >> enabled at padlevel as well as io daisychain sequence is triggered,
> >> there will be 2 PRCM interrupts (Module async wake up via swakeup and
> >> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
> >> Padlevel WKST bit in the first interrupt, module specific interrupt
> >> handler will not triggered for the second time
> >>
> >> Also look at detailed explanation given by Rajendra at
> >> http://www.spinics.net/lists/linux-serial/msg04480.html
> >>
> >> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
> >> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >> ---
> >>   arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
> >>   arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
> >>   arch/arm/mach-omap2/pm.h         |    1 +
> >>   3 files changed, 23 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >> index 5192cab..56adbfb 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >
> > ...
> >
> >> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
> >>   	/* Mux pins for device runtime if populated */
> >>   	if (oh->mux&&  (!oh->mux->enabled ||
> >>   			((oh->_state == _HWMOD_STATE_IDLE)&&
> >> -			 oh->mux->pads_dynamic)))
> >> +			 oh->mux->pads_dynamic))) {
> >>   		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
> >> +		omap_trigger_io_chain();
> >
> > Looks racy: if two hwmods with dynamic mux entries go idle at the same
> > time, or one goes idle while another one is enabled, won't the calls to
> > omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking
> > in omap_trigger_io_chain() or the functions it calls.
> 
> I agree, this needs locking to avoid races.

I'll add one for v5.

> 
> >
> > Also, won't this result in needless resets of the I/O chain?  Seems like
> > we'd only need to do this when the next power state of the enclosing
> > powerdomain will enter either RETENTION or OFF.  And even then, it
> > presumably should only happen when the last active device in that
> > powerdomain is going idle?
> 
> Yes, the module async wakeups will work as long as the power domain
> enclosing the module is not in OSWR or OFF, so ideally this trigger
> should happen only when all modules in the given powerdomain are
> disabled and we plan to program the Powerdomain down to OSWR or
> OFF state. With what we are doing today we end up with periods when we
> have multiple wakeups (a module wakeup as well as an IO wakeup).
> The last we discussed this with Kevin, there wasn't a better place where
> we could trigger this, with no usecounting at powerdomain level you
> didn't know when the last active device in the powerdomain was going
> idle.
> But now with Tero's series which adds usecounting at power/voltage
> domain level, maybe its possible, but I need to look more.
> Do you already have an idea on where this would fit better, so we
> avoid this multiple wakeup scenario?

This should be easy, we can add a hook at the pwrdm_clkdm_enable /
disable calls to check against idle / active states. However, seeing the
acceptance status for the usecounting series, we probably have to just
go ahead with the extra latency involved iochain series and make
optimizations later once the usecounting can be accepted.

Speaking of the usecounting series, Paul, did you have any time to look
at it? Also, there are still some issues seen by Kevin on that series
with omap3430 which I am unable to reproduce (I posted a couple of trial
patches for taking care of that but don't know how they behave.)

-Tero

[PATCHv4 5/6] ARM: OMAP3PLUS PM: Add IO Daisychain support via hwmod mux

[Tero Kristo]

On Tue, 2012-03-06 at 09:51 +0530, Rajendra Nayak wrote:
> Hi Paul,
> 
> On Tuesday 06 March 2012 09:32 AM, Paul Walmsley wrote:
> > Hi
> >
> > a few comments:
> >
> > On Fri, 2 Mar 2012, Tero Kristo wrote:
> >
> >> From: Vishwanath BS<vishwanath.bs@ti.com>
> >>
> >> IO Daisychain feature has to be triggered whenever there is a change in
> >> device's mux configuration (See section 3.9.4 in OMAP4 Public TRM vP).
> >>
> >> Now devices can idle independent of the powerdomain, there can be a
> >> window where device is idled and corresponding powerdomain can be
> >> ON/INACTIVE state. In such situations, since both module wake up is
> >> enabled at padlevel as well as io daisychain sequence is triggered,
> >> there will be 2 PRCM interrupts (Module async wake up via swakeup and
> >> IO Pad interrupt). But as PRCM Interrupt handler clears the Module
> >> Padlevel WKST bit in the first interrupt, module specific interrupt
> >> handler will not triggered for the second time
> >>
> >> Also look at detailed explanation given by Rajendra at
> >> http://www.spinics.net/lists/linux-serial/msg04480.html
> >>
> >> Signed-off-by: Vishwanath BS<vishwanath.bs@ti.com>
> >> Signed-off-by: Tero Kristo<t-kristo@ti.com>
> >> ---
> >>   arch/arm/mach-omap2/omap_hwmod.c |    9 +++++++--
> >>   arch/arm/mach-omap2/pm.c         |   15 +++++++++++++++
> >>   arch/arm/mach-omap2/pm.h         |    1 +
> >>   3 files changed, 23 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/arch/arm/mach-omap2/omap_hwmod.c b/arch/arm/mach-omap2/omap_hwmod.c
> >> index 5192cab..56adbfb 100644
> >> --- a/arch/arm/mach-omap2/omap_hwmod.c
> >> +++ b/arch/arm/mach-omap2/omap_hwmod.c
> >
> > ...
> >
> >> @@ -1535,8 +1536,10 @@ static int _enable(struct omap_hwmod *oh)
> >>   	/* Mux pins for device runtime if populated */
> >>   	if (oh->mux&&  (!oh->mux->enabled ||
> >>   			((oh->_state == _HWMOD_STATE_IDLE)&&
> >> -			 oh->mux->pads_dynamic)))
> >> +			 oh->mux->pads_dynamic))) {
> >>   		omap_hwmod_mux(oh->mux, _HWMOD_STATE_ENABLED);
> >> +		omap_trigger_io_chain();
> >
> > Looks racy: if two hwmods with dynamic mux entries go idle at the same
> > time, or one goes idle while another one is enabled, won't the calls to
> > omap_trigger_io_chain() race?  Locking is per-hwmod and there's no locking
> > in omap_trigger_io_chain() or the functions it calls.
> 
> I agree, this needs locking to avoid races.

I'll add one for v5.

> 
> >
> > Also, won't this result in needless resets of the I/O chain?  Seems like
> > we'd only need to do this when the next power state of the enclosing
> > powerdomain will enter either RETENTION or OFF.  And even then, it
> > presumably should only happen when the last active device in that
> > powerdomain is going idle?
> 
> Yes, the module async wakeups will work as long as the power domain
> enclosing the module is not in OSWR or OFF, so ideally this trigger
> should happen only when all modules in the given powerdomain are
> disabled and we plan to program the Powerdomain down to OSWR or
> OFF state. With what we are doing today we end up with periods when we
> have multiple wakeups (a module wakeup as well as an IO wakeup).
> The last we discussed this with Kevin, there wasn't a better place where
> we could trigger this, with no usecounting at powerdomain level you
> didn't know when the last active device in the powerdomain was going
> idle.
> But now with Tero's series which adds usecounting at power/voltage
> domain level, maybe its possible, but I need to look more.
> Do you already have an idea on where this would fit better, so we
> avoid this multiple wakeup scenario?

This should be easy, we can add a hook at the pwrdm_clkdm_enable /
disable calls to check against idle / active states. However, seeing the
acceptance status for the usecounting series, we probably have to just
go ahead with the extra latency involved iochain series and make
optimizations later once the usecounting can be accepted.

Speaking of the usecounting series, Paul, did you have any time to look
at it? Also, there are still some issues seen by Kevin on that series
with omap3430 which I am unable to reproduce (I posted a couple of trial
patches for taking care of that but don't know how they behave.)

-Tero

[PATCHv4 6/6] ARM: OMAP3 PM: Remove IO Daisychain control from cpuidle

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

As IO Daisy chain sequence is triggered via hwmod mux, there is no need to
control it from cpuidle path for OMAP3.

Also as omap3_disable_io_chain is no longer being used, just remove the
function.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c       |   17 -----------------
 arch/arm/mach-omap2/prm2xxx_3xxx.c |    8 --------
 arch/arm/mach-omap2/prm2xxx_3xxx.h |    7 -------
 3 files changed, 0 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index e97ec3f..ed12bb4 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -291,13 +291,6 @@ void omap_sram_idle(void)
 	/* Enable IO-PAD and IO-CHAIN wakeups */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
-	if (omap3_has_io_wakeup() &&
-	    (per_next_state < PWRDM_POWER_ON ||
-	     core_next_state < PWRDM_POWER_ON)) {
-		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN);
-		if (omap3_has_io_chain_ctrl())
-			omap3_trigger_io_chain();
-	}
 
 	pwrdm_pre_transition();
 
@@ -376,16 +369,6 @@ void omap_sram_idle(void)
 			omap3_per_restore_context();
 	}
 
-	/* Disable IO-PAD and IO-CHAIN wakeup */
-	if (omap3_has_io_wakeup() &&
-	    (per_next_state < PWRDM_POWER_ON ||
-	     core_next_state < PWRDM_POWER_ON)) {
-		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD,
-					     PM_WKEN);
-		if (omap3_has_io_chain_ctrl())
-			omap3_disable_io_chain();
-	}
-
 	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index f681628..31a7c27 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -329,14 +329,6 @@ void omap3_trigger_io_chain(void)
 			  MAX_IOPAD_LATCH_TIME, i);
 }
 
-/* OMAP3 Daisychain disable sequence */
-void omap3_disable_io_chain(void)
-{
-	if (omap_rev() >= OMAP3430_REV_ES3_1)
-		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK,
-					     WKUP_MOD, PM_WKEN);
-}
-
 static void __init omap3_enable_io_wakeup(void)
 {
 	if (omap3_has_io_wakeup())
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.h b/arch/arm/mach-omap2/prm2xxx_3xxx.h
index 15973aa..391b844 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.h
@@ -295,12 +295,6 @@ static inline void omap3_trigger_io_chain(void)
 		"not suppose to be used on omap4\n");
 	return 0;
 }
-static inline void omap3_disable_io_chain(void)
-{
-	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
-		"not suppose to be used on omap4\n");
-	return 0;
-}
 #else
 /* Power/reset management domain register get/set */
 extern u32 omap2_prm_read_mod_reg(s16 module, u16 idx);
@@ -315,7 +309,6 @@ extern int omap2_prm_is_hardreset_asserted(s16 prm_mod, u8 shift);
 extern int omap2_prm_assert_hardreset(s16 prm_mod, u8 shift);
 extern int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift, u8 st_shift);
 extern void omap3_trigger_io_chain(void);
-extern void omap3_disable_io_chain(void);
 
 /* OMAP3-specific VP functions */
 u32 omap3_prm_vp_check_txdone(u8 vp_id);
-- 
1.7.4.1

[PATCHv4 6/6] ARM: OMAP3 PM: Remove IO Daisychain control from cpuidle

[Tero Kristo]

From: Vishwanath BS <vishwanath.bs@ti.com>

As IO Daisy chain sequence is triggered via hwmod mux, there is no need to
control it from cpuidle path for OMAP3.

Also as omap3_disable_io_chain is no longer being used, just remove the
function.

Signed-off-by: Vishwanath BS <vishwanath.bs@ti.com>
Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 arch/arm/mach-omap2/pm34xx.c       |   17 -----------------
 arch/arm/mach-omap2/prm2xxx_3xxx.c |    8 --------
 arch/arm/mach-omap2/prm2xxx_3xxx.h |    7 -------
 3 files changed, 0 insertions(+), 32 deletions(-)

diff --git a/arch/arm/mach-omap2/pm34xx.c b/arch/arm/mach-omap2/pm34xx.c
index e97ec3f..ed12bb4 100644
--- a/arch/arm/mach-omap2/pm34xx.c
+++ b/arch/arm/mach-omap2/pm34xx.c
@@ -291,13 +291,6 @@ void omap_sram_idle(void)
 	/* Enable IO-PAD and IO-CHAIN wakeups */
 	per_next_state = pwrdm_read_next_pwrst(per_pwrdm);
 	core_next_state = pwrdm_read_next_pwrst(core_pwrdm);
-	if (omap3_has_io_wakeup() &&
-	    (per_next_state < PWRDM_POWER_ON ||
-	     core_next_state < PWRDM_POWER_ON)) {
-		omap2_prm_set_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD, PM_WKEN);
-		if (omap3_has_io_chain_ctrl())
-			omap3_trigger_io_chain();
-	}
 
 	pwrdm_pre_transition();
 
@@ -376,16 +369,6 @@ void omap_sram_idle(void)
 			omap3_per_restore_context();
 	}
 
-	/* Disable IO-PAD and IO-CHAIN wakeup */
-	if (omap3_has_io_wakeup() &&
-	    (per_next_state < PWRDM_POWER_ON ||
-	     core_next_state < PWRDM_POWER_ON)) {
-		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_MASK, WKUP_MOD,
-					     PM_WKEN);
-		if (omap3_has_io_chain_ctrl())
-			omap3_disable_io_chain();
-	}
-
 	clkdm_allow_idle(mpu_pwrdm->pwrdm_clkdms[0]);
 }
 
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.c b/arch/arm/mach-omap2/prm2xxx_3xxx.c
index f681628..31a7c27 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.c
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.c
@@ -329,14 +329,6 @@ void omap3_trigger_io_chain(void)
 			  MAX_IOPAD_LATCH_TIME, i);
 }
 
-/* OMAP3 Daisychain disable sequence */
-void omap3_disable_io_chain(void)
-{
-	if (omap_rev() >= OMAP3430_REV_ES3_1)
-		omap2_prm_clear_mod_reg_bits(OMAP3430_EN_IO_CHAIN_MASK,
-					     WKUP_MOD, PM_WKEN);
-}
-
 static void __init omap3_enable_io_wakeup(void)
 {
 	if (omap3_has_io_wakeup())
diff --git a/arch/arm/mach-omap2/prm2xxx_3xxx.h b/arch/arm/mach-omap2/prm2xxx_3xxx.h
index 15973aa..391b844 100644
--- a/arch/arm/mach-omap2/prm2xxx_3xxx.h
+++ b/arch/arm/mach-omap2/prm2xxx_3xxx.h
@@ -295,12 +295,6 @@ static inline void omap3_trigger_io_chain(void)
 		"not suppose to be used on omap4\n");
 	return 0;
 }
-static inline void omap3_disable_io_chain(void)
-{
-	WARN(1, "prm: omap2xxx/omap3xxx specific function and "
-		"not suppose to be used on omap4\n");
-	return 0;
-}
 #else
 /* Power/reset management domain register get/set */
 extern u32 omap2_prm_read_mod_reg(s16 module, u16 idx);
@@ -315,7 +309,6 @@ extern int omap2_prm_is_hardreset_asserted(s16 prm_mod, u8 shift);
 extern int omap2_prm_assert_hardreset(s16 prm_mod, u8 shift);
 extern int omap2_prm_deassert_hardreset(s16 prm_mod, u8 rst_shift, u8 st_shift);
 extern void omap3_trigger_io_chain(void);
-extern void omap3_disable_io_chain(void);
 
 /* OMAP3-specific VP functions */
 u32 omap3_prm_vp_check_txdone(u8 vp_id);
-- 
1.7.4.1

Re: [PATCHv4 0/6] ARM: OMAP3+: IO daisychain support fixes

[Rajendra Nayak]

On Friday 02 March 2012 08:47 PM, Tero Kristo wrote:
> Hi,
>
> Changes compared to previous version:
>
> - patch2:
>   * changed timeout value to 100us (from 1000)
>   * added poll for the IO status == 0 at the end of trigger sequence
> - patch4:
>   * omap3: global wakeup done in PRM init instead of OMAP3 PM init
>   * omap4: moved global wakeup enable from trigger func to PRM init
>
> Tested on omap3beagle / omap4blaze, suspend / resume works with io chain.

for the full series,
Reviewed-by: Rajendra Nayak <rnayak@ti.com>

>
> -Tero
>
> --
> 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

[PATCHv4 0/6] ARM: OMAP3+: IO daisychain support fixes

[Rajendra Nayak]

On Friday 02 March 2012 08:47 PM, Tero Kristo wrote:
> Hi,
>
> Changes compared to previous version:
>
> - patch2:
>   * changed timeout value to 100us (from 1000)
>   * added poll for the IO status == 0 at the end of trigger sequence
> - patch4:
>   * omap3: global wakeup done in PRM init instead of OMAP3 PM init
>   * omap4: moved global wakeup enable from trigger func to PRM init
>
> Tested on omap3beagle / omap4blaze, suspend / resume works with io chain.

for the full series,
Reviewed-by: Rajendra Nayak <rnayak@ti.com>

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