[PATCH] OMAP3: Serial: Improved sleep logic

[PATCH] OMAP3: Serial: Improved sleep logic

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

Only RX interrupt will now kick the sleep prevent timer. In addition, TX
fifo status is checked before disabling clocks, this will prevent occasional
garbage being printed on serial line. Smartidle is also disabled while
entering idle if we have data in the transmit buffer, because having this
enabled will prevent wakeups from the TX interrupt, and this causes
pauses while sending large blocks of data.

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 777e802..e11dfe9 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct omap_uart_state *uart)
 	if (!uart->clocked)
 		return;
 
-	omap_uart_smart_idle_enable(uart, 1);
+	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
+		omap_uart_smart_idle_enable(uart, 1);
 	uart->can_sleep = 1;
 	del_timer(&uart->timer);
 }
@@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
 
 	list_for_each_entry(uart, &uart_list, node) {
 		if (num == uart->num && uart->can_sleep) {
-			omap_uart_disable_clocks(uart);
+			if (serial_read_reg(uart->p, UART_LSR) &
+					UART_LSR_TEMT)
+				omap_uart_disable_clocks(uart);
+			else
+				omap_uart_smart_idle_enable(uart, 0);
 			return;
 		}
 	}
@@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
 static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
 {
 	struct omap_uart_state *uart = dev_id;
+	u8 lsr;
 
-	omap_uart_block_sleep(uart);
+	lsr = serial_read_reg(uart->p, UART_LSR);
+	/* Check for receive interrupt */
+	if (lsr & UART_LSR_DR)
+		omap_uart_block_sleep(uart);
+	if (lsr & UART_LSR_TEMT && uart->can_sleep)
+		omap_uart_smart_idle_enable(uart, 1);
 
 	return IRQ_NONE;
 }
-- 
1.5.4.3

Re: [PATCH] OMAP3: Serial: Improved sleep logic

[Tony Lindgren]

* Tero Kristo <tero.kristo@nokia.com> [100202 01:38]:
> From: Tero Kristo <tero.kristo@nokia.com>
> 
> Only RX interrupt will now kick the sleep prevent timer. In addition, TX
> fifo status is checked before disabling clocks, this will prevent occasional
> garbage being printed on serial line. Smartidle is also disabled while
> entering idle if we have data in the transmit buffer, because having this
> enabled will prevent wakeups from the TX interrupt, and this causes
> pauses while sending large blocks of data.

Sounds this is for 2.6.34 merge window and does not contain anything
that needs to be fixed for 2.6.33. 

Regards,

Tony

 
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> ---
>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
>  1 files changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 777e802..e11dfe9 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct omap_uart_state *uart)
>  	if (!uart->clocked)
>  		return;
>  
> -	omap_uart_smart_idle_enable(uart, 1);
> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
> +		omap_uart_smart_idle_enable(uart, 1);
>  	uart->can_sleep = 1;
>  	del_timer(&uart->timer);
>  }
> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
>  
>  	list_for_each_entry(uart, &uart_list, node) {
>  		if (num == uart->num && uart->can_sleep) {
> -			omap_uart_disable_clocks(uart);
> +			if (serial_read_reg(uart->p, UART_LSR) &
> +					UART_LSR_TEMT)
> +				omap_uart_disable_clocks(uart);
> +			else
> +				omap_uart_smart_idle_enable(uart, 0);
>  			return;
>  		}
>  	}
> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
>  {
>  	struct omap_uart_state *uart = dev_id;
> +	u8 lsr;
>  
> -	omap_uart_block_sleep(uart);
> +	lsr = serial_read_reg(uart->p, UART_LSR);
> +	/* Check for receive interrupt */
> +	if (lsr & UART_LSR_DR)
> +		omap_uart_block_sleep(uart);
> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
> +		omap_uart_smart_idle_enable(uart, 1);
>  
>  	return IRQ_NONE;
>  }
> -- 
> 1.5.4.3
> 
> --
> 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

RE: [PATCH] OMAP3: Serial: Improved sleep logic

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Tony Lindgren [mailto:tony@atomide.com] 
>Sent: 03 February, 2010 19:50
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH] OMAP3: Serial: Improved sleep logic
>
>* Tero Kristo <tero.kristo@nokia.com> [100202 01:38]:
>> From: Tero Kristo <tero.kristo@nokia.com>
>> 
>> Only RX interrupt will now kick the sleep prevent timer. In 
>addition, TX
>> fifo status is checked before disabling clocks, this will 
>prevent occasional
>> garbage being printed on serial line. Smartidle is also 
>disabled while
>> entering idle if we have data in the transmit buffer, 
>because having this
>> enabled will prevent wakeups from the TX interrupt, and this causes
>> pauses while sending large blocks of data.
>
>Sounds this is for 2.6.34 merge window and does not contain anything
>that needs to be fixed for 2.6.33. 

Yeah, this only fixes a couple of annoying issues, but nothing fatal. Also improves power efficiency in some cases which is a nice to have feature, and basically eases the development of PM code as the UART does not block the system from sleeping always.

-Tero

>
>Regards,
>
>Tony
>
> 
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> ---
>>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
>>  1 files changed, 15 insertions(+), 4 deletions(-)
>> 
>> diff --git a/arch/arm/mach-omap2/serial.c 
>b/arch/arm/mach-omap2/serial.c
>> index 777e802..e11dfe9 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct 
>omap_uart_state *uart)
>>  	if (!uart->clocked)
>>  		return;
>>  
>> -	omap_uart_smart_idle_enable(uart, 1);
>> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
>> +		omap_uart_smart_idle_enable(uart, 1);
>>  	uart->can_sleep = 1;
>>  	del_timer(&uart->timer);
>>  }
>> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
>>  
>>  	list_for_each_entry(uart, &uart_list, node) {
>>  		if (num == uart->num && uart->can_sleep) {
>> -			omap_uart_disable_clocks(uart);
>> +			if (serial_read_reg(uart->p, UART_LSR) &
>> +					UART_LSR_TEMT)
>> +				omap_uart_disable_clocks(uart);
>> +			else
>> +				omap_uart_smart_idle_enable(uart, 0);
>>  			return;
>>  		}
>>  	}
>> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
>>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
>>  {
>>  	struct omap_uart_state *uart = dev_id;
>> +	u8 lsr;
>>  
>> -	omap_uart_block_sleep(uart);
>> +	lsr = serial_read_reg(uart->p, UART_LSR);
>> +	/* Check for receive interrupt */
>> +	if (lsr & UART_LSR_DR)
>> +		omap_uart_block_sleep(uart);
>> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
>> +		omap_uart_smart_idle_enable(uart, 1);
>>  
>>  	return IRQ_NONE;
>>  }
>> -- 
>> 1.5.4.3
>> 
>> --
>> 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
>

Re: [PATCH] OMAP3: Serial: Improved sleep logic

[Tony Lindgren]

* Tero.Kristo@nokia.com <Tero.Kristo@nokia.com> [100203 23:59]:
>  
> 
> >-----Original Message-----
> >From: ext Tony Lindgren [mailto:tony@atomide.com] 
> >Sent: 03 February, 2010 19:50
> >To: Kristo Tero (Nokia-D/Tampere)
> >Cc: linux-omap@vger.kernel.org
> >Subject: Re: [PATCH] OMAP3: Serial: Improved sleep logic
> >
> >* Tero Kristo <tero.kristo@nokia.com> [100202 01:38]:
> >> From: Tero Kristo <tero.kristo@nokia.com>
> >> 
> >> Only RX interrupt will now kick the sleep prevent timer. In 
> >addition, TX
> >> fifo status is checked before disabling clocks, this will 
> >prevent occasional
> >> garbage being printed on serial line. Smartidle is also 
> >disabled while
> >> entering idle if we have data in the transmit buffer, 
> >because having this
> >> enabled will prevent wakeups from the TX interrupt, and this causes
> >> pauses while sending large blocks of data.
> >
> >Sounds this is for 2.6.34 merge window and does not contain anything
> >that needs to be fixed for 2.6.33. 
> 
> Yeah, this only fixes a couple of annoying issues, but nothing fatal. Also improves power efficiency in some cases which is a nice to have feature, and basically eases the development of PM code as the UART does not block the system from sleeping always.

Yeah cool. Separate issue, but I wonder if we should also clear
the the first rx character from the fifo to avoid corrupting
the console. Only when the uart clocks are off and and
we get a key press. Would be easy to do just do with one
serial_read_reg(uart->p, UART_RX)..

Tony

> 
> -Tero
> 
> >
> >Regards,
> >
> >Tony
> >
> > 
> >> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> >> ---
> >>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
> >>  1 files changed, 15 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/arch/arm/mach-omap2/serial.c 
> >b/arch/arm/mach-omap2/serial.c
> >> index 777e802..e11dfe9 100644
> >> --- a/arch/arm/mach-omap2/serial.c
> >> +++ b/arch/arm/mach-omap2/serial.c
> >> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct 
> >omap_uart_state *uart)
> >>  	if (!uart->clocked)
> >>  		return;
> >>  
> >> -	omap_uart_smart_idle_enable(uart, 1);
> >> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
> >> +		omap_uart_smart_idle_enable(uart, 1);
> >>  	uart->can_sleep = 1;
> >>  	del_timer(&uart->timer);
> >>  }
> >> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
> >>  
> >>  	list_for_each_entry(uart, &uart_list, node) {
> >>  		if (num == uart->num && uart->can_sleep) {
> >> -			omap_uart_disable_clocks(uart);
> >> +			if (serial_read_reg(uart->p, UART_LSR) &
> >> +					UART_LSR_TEMT)
> >> +				omap_uart_disable_clocks(uart);
> >> +			else
> >> +				omap_uart_smart_idle_enable(uart, 0);
> >>  			return;
> >>  		}
> >>  	}
> >> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
> >>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
> >>  {
> >>  	struct omap_uart_state *uart = dev_id;
> >> +	u8 lsr;
> >>  
> >> -	omap_uart_block_sleep(uart);
> >> +	lsr = serial_read_reg(uart->p, UART_LSR);
> >> +	/* Check for receive interrupt */
> >> +	if (lsr & UART_LSR_DR)
> >> +		omap_uart_block_sleep(uart);
> >> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
> >> +		omap_uart_smart_idle_enable(uart, 1);
> >>  
> >>  	return IRQ_NONE;
> >>  }
> >> -- 
> >> 1.5.4.3
> >> 
> >> --
> >> 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
> >

Re: [PATCH] OMAP3: Serial: Improved sleep logic

[Kevin Hilman]

Tero Kristo <tero.kristo@nokia.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Only RX interrupt will now kick the sleep prevent timer. In addition, TX
> fifo status is checked before disabling clocks, this will prevent occasional
> garbage being printed on serial line. Smartidle is also disabled while
> entering idle if we have data in the transmit buffer, because having this
> enabled will prevent wakeups from the TX interrupt, and this causes
> pauses while sending large blocks of data.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>

After doing some more testing with this, something is not quite right
still.  I haven't taken the time to debug further, but with this patch
on top of the current PM branch, the timer seems to expire and disable
clocks whether or not there is UART activity.

In particular, using a UART1 console on OMAP3EVM, I notice that while
typing longer commands (that take more that <timeout> seconds to type),
I notice that I loose chars in the middle of typing.  /me doesn't like.

So I won't be applying this to the PM branch until we can figure out
what's happening here.

Kevin

> ---
>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
>  1 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
> index 777e802..e11dfe9 100644
> --- a/arch/arm/mach-omap2/serial.c
> +++ b/arch/arm/mach-omap2/serial.c
> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct omap_uart_state *uart)
>  	if (!uart->clocked)
>  		return;
>  
> -	omap_uart_smart_idle_enable(uart, 1);
> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
> +		omap_uart_smart_idle_enable(uart, 1);
>  	uart->can_sleep = 1;
>  	del_timer(&uart->timer);
>  }
> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
>  
>  	list_for_each_entry(uart, &uart_list, node) {
>  		if (num == uart->num && uart->can_sleep) {
> -			omap_uart_disable_clocks(uart);
> +			if (serial_read_reg(uart->p, UART_LSR) &
> +					UART_LSR_TEMT)
> +				omap_uart_disable_clocks(uart);
> +			else
> +				omap_uart_smart_idle_enable(uart, 0);
>  			return;
>  		}
>  	}
> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
>  {
>  	struct omap_uart_state *uart = dev_id;
> +	u8 lsr;
>  
> -	omap_uart_block_sleep(uart);
> +	lsr = serial_read_reg(uart->p, UART_LSR);
> +	/* Check for receive interrupt */
> +	if (lsr & UART_LSR_DR)
> +		omap_uart_block_sleep(uart);
> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
> +		omap_uart_smart_idle_enable(uart, 1);
>  
>  	return IRQ_NONE;
>  }
> -- 
> 1.5.4.3
>
> --
> 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

RE: [PATCH] OMAP3: Serial: Improved sleep logic

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Tony Lindgren [mailto:tony@atomide.com] 
>Sent: 04 February, 2010 18:11
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH] OMAP3: Serial: Improved sleep logic
>
>* Tero.Kristo@nokia.com <Tero.Kristo@nokia.com> [100203 23:59]:
>>  
>> 
>> >-----Original Message-----
>> >From: ext Tony Lindgren [mailto:tony@atomide.com] 
>> >Sent: 03 February, 2010 19:50
>> >To: Kristo Tero (Nokia-D/Tampere)
>> >Cc: linux-omap@vger.kernel.org
>> >Subject: Re: [PATCH] OMAP3: Serial: Improved sleep logic
>> >
>> >* Tero Kristo <tero.kristo@nokia.com> [100202 01:38]:
>> >> From: Tero Kristo <tero.kristo@nokia.com>
>> >> 
>> >> Only RX interrupt will now kick the sleep prevent timer. In 
>> >addition, TX
>> >> fifo status is checked before disabling clocks, this will 
>> >prevent occasional
>> >> garbage being printed on serial line. Smartidle is also 
>> >disabled while
>> >> entering idle if we have data in the transmit buffer, 
>> >because having this
>> >> enabled will prevent wakeups from the TX interrupt, and 
>this causes
>> >> pauses while sending large blocks of data.
>> >
>> >Sounds this is for 2.6.34 merge window and does not contain anything
>> >that needs to be fixed for 2.6.33. 
>> 
>> Yeah, this only fixes a couple of annoying issues, but 
>nothing fatal. Also improves power efficiency in some cases 
>which is a nice to have feature, and basically eases the 
>development of PM code as the UART does not block the system 
>from sleeping always.
>
>Yeah cool. Separate issue, but I wonder if we should also clear
>the the first rx character from the fifo to avoid corrupting
>the console. Only when the uart clocks are off and and
>we get a key press. Would be easy to do just do with one
>serial_read_reg(uart->p, UART_RX)..

Yeah, I can try to add this change to the code and see what happens.

>
>Tony
>
>> 
>> -Tero
>> 
>> >
>> >Regards,
>> >
>> >Tony
>> >
>> > 
>> >> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>> >> ---
>> >>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
>> >>  1 files changed, 15 insertions(+), 4 deletions(-)
>> >> 
>> >> diff --git a/arch/arm/mach-omap2/serial.c 
>> >b/arch/arm/mach-omap2/serial.c
>> >> index 777e802..e11dfe9 100644
>> >> --- a/arch/arm/mach-omap2/serial.c
>> >> +++ b/arch/arm/mach-omap2/serial.c
>> >> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct 
>> >omap_uart_state *uart)
>> >>  	if (!uart->clocked)
>> >>  		return;
>> >>  
>> >> -	omap_uart_smart_idle_enable(uart, 1);
>> >> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
>> >> +		omap_uart_smart_idle_enable(uart, 1);
>> >>  	uart->can_sleep = 1;
>> >>  	del_timer(&uart->timer);
>> >>  }
>> >> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
>> >>  
>> >>  	list_for_each_entry(uart, &uart_list, node) {
>> >>  		if (num == uart->num && uart->can_sleep) {
>> >> -			omap_uart_disable_clocks(uart);
>> >> +			if (serial_read_reg(uart->p, UART_LSR) &
>> >> +					UART_LSR_TEMT)
>> >> +				omap_uart_disable_clocks(uart);
>> >> +			else
>> >> +				omap_uart_smart_idle_enable(uart, 0);
>> >>  			return;
>> >>  		}
>> >>  	}
>> >> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
>> >>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
>> >>  {
>> >>  	struct omap_uart_state *uart = dev_id;
>> >> +	u8 lsr;
>> >>  
>> >> -	omap_uart_block_sleep(uart);
>> >> +	lsr = serial_read_reg(uart->p, UART_LSR);
>> >> +	/* Check for receive interrupt */
>> >> +	if (lsr & UART_LSR_DR)
>> >> +		omap_uart_block_sleep(uart);
>> >> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
>> >> +		omap_uart_smart_idle_enable(uart, 1);
>> >>  
>> >>  	return IRQ_NONE;
>> >>  }
>> >> -- 
>> >> 1.5.4.3
>> >> 
>> >> --
>> >> 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
>> >
>

RE: [PATCH] OMAP3: Serial: Improved sleep logic

[Tero.Kristo]

 

>-----Original Message-----
>From: ext Kevin Hilman [mailto:khilman@deeprootsystems.com] 
>Sent: 04 February, 2010 23:20
>To: Kristo Tero (Nokia-D/Tampere)
>Cc: linux-omap@vger.kernel.org
>Subject: Re: [PATCH] OMAP3: Serial: Improved sleep logic
>
>Tero Kristo <tero.kristo@nokia.com> writes:
>
>> From: Tero Kristo <tero.kristo@nokia.com>
>>
>> Only RX interrupt will now kick the sleep prevent timer. In 
>addition, TX
>> fifo status is checked before disabling clocks, this will 
>prevent occasional
>> garbage being printed on serial line. Smartidle is also 
>disabled while
>> entering idle if we have data in the transmit buffer, 
>because having this
>> enabled will prevent wakeups from the TX interrupt, and this causes
>> pauses while sending large blocks of data.
>>
>> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>
>After doing some more testing with this, something is not quite right
>still.  I haven't taken the time to debug further, but with this patch
>on top of the current PM branch, the timer seems to expire and disable
>clocks whether or not there is UART activity.

This seems to be caused by the buggy timer usage inside serial.c. The code uses jiffy timers for expire checks, and this is actually invalid inside omap_sram_idle. Jiffy timer is stopped inside the ARM idle entry (cpu_idle -> tick_nohz_stop_sched_tick()). This causes the jiffies value to be the same when we enter idle, and while we are resuming idle, thus making the timer to expire at rather random time. If you sleep for 4 seconds and wake using serial, the device enters sleep again after one second. This can be fixed by using ktime_get() instead of jiffy timer, as ktime_get() uses directly HW 32k tick timer. I'll hack together a version of this code where I use ktime_get() for the sleep expiry checks and see if it fixes this issue.

>In particular, using a UART1 console on OMAP3EVM, I notice that while
>typing longer commands (that take more that <timeout> seconds to type),
>I notice that I loose chars in the middle of typing.  /me doesn't like.
>
>So I won't be applying this to the PM branch until we can figure out
>what's happening here.
>
>Kevin
>
>> ---
>>  arch/arm/mach-omap2/serial.c |   19 +++++++++++++++----
>>  1 files changed, 15 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/mach-omap2/serial.c 
>b/arch/arm/mach-omap2/serial.c
>> index 777e802..e11dfe9 100644
>> --- a/arch/arm/mach-omap2/serial.c
>> +++ b/arch/arm/mach-omap2/serial.c
>> @@ -317,7 +317,8 @@ static void omap_uart_allow_sleep(struct 
>omap_uart_state *uart)
>>  	if (!uart->clocked)
>>  		return;
>>  
>> -	omap_uart_smart_idle_enable(uart, 1);
>> +	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
>> +		omap_uart_smart_idle_enable(uart, 1);
>>  	uart->can_sleep = 1;
>>  	del_timer(&uart->timer);
>>  }
>> @@ -335,7 +336,11 @@ void omap_uart_prepare_idle(int num)
>>  
>>  	list_for_each_entry(uart, &uart_list, node) {
>>  		if (num == uart->num && uart->can_sleep) {
>> -			omap_uart_disable_clocks(uart);
>> +			if (serial_read_reg(uart->p, UART_LSR) &
>> +					UART_LSR_TEMT)
>> +				omap_uart_disable_clocks(uart);
>> +			else
>> +				omap_uart_smart_idle_enable(uart, 0);
>>  			return;
>>  		}
>>  	}
>> @@ -407,8 +412,14 @@ int omap_uart_can_sleep(void)
>>  static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
>>  {
>>  	struct omap_uart_state *uart = dev_id;
>> +	u8 lsr;
>>  
>> -	omap_uart_block_sleep(uart);
>> +	lsr = serial_read_reg(uart->p, UART_LSR);
>> +	/* Check for receive interrupt */
>> +	if (lsr & UART_LSR_DR)
>> +		omap_uart_block_sleep(uart);
>> +	if (lsr & UART_LSR_TEMT && uart->can_sleep)
>> +		omap_uart_smart_idle_enable(uart, 1);
>>  
>>  	return IRQ_NONE;
>>  }
>> -- 
>> 1.5.4.3
>>
>> --
>> 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
>

RE: [PATCH] OMAP3: Serial: Improved sleep logic

[Tero.Kristo]

This patch can be ignored. The RX fifo clear logic did not really work for low C states (C1 / C2) and you would still get garbage. I'll send an updated patch where I have replaced the logic by an ignore timer for the first millisecond after clocks are enabled. Also, I forgot v2 out from this.

>-----Original Message-----
>From: linux-omap-owner@vger.kernel.org 
>[mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Kristo 
>Tero (Nokia-D/Tampere)
>Sent: 10 February, 2010 18:29
>To: linux-omap@vger.kernel.org
>Subject: [PATCH] OMAP3: Serial: Improved sleep logic
>
>From: Tero Kristo <tero.kristo@nokia.com>
>
>This patch contains following improvements:
>- Only RX interrupt will now kick the sleep prevent timer
>- TX fifo status is checked before disabling clocks, this will prevent
>  on-going transmission to be cut
>- Smartidle is disabled while entering idle if we have data in 
>the transmit
>  buffer because having this enabled would prevent wakeups from the TX
>  interrupt and this would cause pauses while sending large 
>blocks of data
>- Sleep prevent timer is changed to use ktime_get() instead of 
>a jiffy timer
>  as jiffy timers are not valid within idle loop (tick 
>scheduler is stopped)
>- RX and TX fifos are cleared when clocks are enabled, this 
>will purge the
>  first character from RX fifo, which is most likely garbage
>
>Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
>---
> arch/arm/mach-omap2/serial.c |   70 
>+++++++++++++++++++++++++++---------------
> 1 files changed, 45 insertions(+), 25 deletions(-)
>
>diff --git a/arch/arm/mach-omap2/serial.c 
>b/arch/arm/mach-omap2/serial.c
>index 837b347..d7d96ba 100644
>--- a/arch/arm/mach-omap2/serial.c
>+++ b/arch/arm/mach-omap2/serial.c
>@@ -23,12 +23,15 @@
> #include <linux/serial_reg.h>
> #include <linux/clk.h>
> #include <linux/io.h>
>+#include <linux/delay.h>
> 
> #include <plat/common.h>
> #include <plat/board.h>
> #include <plat/clock.h>
> #include <plat/control.h>
> 
>+#include <asm/div64.h>
>+
> #include "prm.h"
> #include "pm.h"
> #include "prm-regbits-34xx.h"
>@@ -36,13 +39,13 @@
> #define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV	0x52
> #define UART_OMAP_WER		0x17	/* Wake-up enable register */
> 
>-#define DEFAULT_TIMEOUT (5 * HZ)
>+#define DEFAULT_TIMEOUT (5LL * NSEC_PER_SEC)
> 
> struct omap_uart_state {
> 	int num;
> 	int can_sleep;
>-	struct timer_list timer;
>-	u32 timeout;
>+	ktime_t expire_time;
>+	u64 timeout;
> 
> 	void __iomem *wk_st;
> 	void __iomem *wk_en;
>@@ -231,6 +234,9 @@ static inline void 
>omap_uart_enable_clocks(struct omap_uart_state *uart)
> 	clk_enable(uart->fck);
> 	uart->clocked = 1;
> 	omap_uart_restore_context(uart);
>+
>+	/* Clear RX and TX fifos, as they contain garbage at 
>this point */
>+	serial_write_reg(uart->p, UART_FCR, 0xa7);
> }
> 
> #ifdef CONFIG_PM
>@@ -302,9 +308,7 @@ static void omap_uart_block_sleep(struct 
>omap_uart_state *uart)
> 	omap_uart_smart_idle_enable(uart, 0);
> 	uart->can_sleep = 0;
> 	if (uart->timeout)
>-		mod_timer(&uart->timer, jiffies + uart->timeout);
>-	else
>-		del_timer(&uart->timer);
>+		uart->expire_time = ktime_add_ns(ktime_get(), 
>uart->timeout);
> }
> 
> static void omap_uart_allow_sleep(struct omap_uart_state *uart)
>@@ -317,25 +321,28 @@ static void omap_uart_allow_sleep(struct 
>omap_uart_state *uart)
> 	if (!uart->clocked)
> 		return;
> 
>-	omap_uart_smart_idle_enable(uart, 1);
>+	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
>+		omap_uart_smart_idle_enable(uart, 1);
> 	uart->can_sleep = 1;
>-	del_timer(&uart->timer);
>-}
>-
>-static void omap_uart_idle_timer(unsigned long data)
>-{
>-	struct omap_uart_state *uart = (struct omap_uart_state *)data;
>-
>-	omap_uart_allow_sleep(uart);
> }
> 
> void omap_uart_prepare_idle(int num)
> {
> 	struct omap_uart_state *uart;
>+	ktime_t t;
> 
> 	list_for_each_entry(uart, &uart_list, node) {
>+		if (num == uart->num && !uart->can_sleep) {
>+			t = ktime_get();
>+			if (t.tv64 > uart->expire_time.tv64)
>+				uart->can_sleep = 1;
>+		}
> 		if (num == uart->num && uart->can_sleep) {
>-			omap_uart_disable_clocks(uart);
>+			if (serial_read_reg(uart->p, UART_LSR) &
>+					UART_LSR_TEMT)
>+				omap_uart_disable_clocks(uart);
>+			else
>+				omap_uart_smart_idle_enable(uart, 0);
> 			return;
> 		}
> 	}
>@@ -360,6 +367,7 @@ void omap_uart_resume_idle(int num)
> 			/* Check for normal UART wakeup */
> 			if (__raw_readl(uart->wk_st) & uart->wk_mask)
> 				omap_uart_block_sleep(uart);
>+
> 			return;
> 		}
> 	}
>@@ -407,8 +415,14 @@ int omap_uart_can_sleep(void)
> static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
> {
> 	struct omap_uart_state *uart = dev_id;
>+	u8 lsr;
> 
>-	omap_uart_block_sleep(uart);
>+	lsr = serial_read_reg(uart->p, UART_LSR);
>+	/* Check for receive interrupt */
>+	if (lsr & UART_LSR_DR)
>+		omap_uart_block_sleep(uart);
>+	if (lsr & UART_LSR_TEMT && uart->can_sleep)
>+		omap_uart_smart_idle_enable(uart, 1);
> 
> 	return IRQ_NONE;
> }
>@@ -420,9 +434,9 @@ static void omap_uart_idle_init(struct 
>omap_uart_state *uart)
> 
> 	uart->can_sleep = 0;
> 	uart->timeout = DEFAULT_TIMEOUT;
>-	setup_timer(&uart->timer, omap_uart_idle_timer,
>-		    (unsigned long) uart);
>-	mod_timer(&uart->timer, jiffies + uart->timeout);
>+
>+	uart->expire_time = ktime_add_ns(ktime_get(), uart->timeout);
>+
> 	omap_uart_smart_idle_enable(uart, 0);
> 
> 	if (cpu_is_omap34xx()) {
>@@ -505,8 +519,12 @@ static ssize_t sleep_timeout_show(struct 
>device *dev,
> 					struct platform_device, dev);
> 	struct omap_uart_state *uart = container_of(pdev,
> 					struct omap_uart_state, pdev);
>+	u64 val;
> 
>-	return sprintf(buf, "%u\n", uart->timeout / HZ);
>+	val = uart->timeout;
>+
>+	do_div(val, NSEC_PER_SEC);
>+	return sprintf(buf, "%llu\n", val);
> }
> 
> static ssize_t sleep_timeout_store(struct device *dev,
>@@ -524,10 +542,12 @@ static ssize_t 
>sleep_timeout_store(struct device *dev,
> 		return -EINVAL;
> 	}
> 
>-	uart->timeout = value * HZ;
>-	if (uart->timeout)
>-		mod_timer(&uart->timer, jiffies + uart->timeout);
>-	else
>+	uart->timeout = (u64)value * NSEC_PER_SEC;
>+	if (uart->timeout) {
>+		uart->expire_time = ktime_get();
>+		uart->expire_time =
>+			ktime_add_ns(uart->expire_time, uart->timeout);
>+	} else
> 		/* A zero value means disable timeout feature */
> 		omap_uart_block_sleep(uart);
> 
>-- 
>1.5.4.3
>
>--
>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
>

[PATCH] OMAP3: Serial: Improved sleep logic

[Tero Kristo]

From: Tero Kristo <tero.kristo@nokia.com>

This patch contains following improvements:
- Only RX interrupt will now kick the sleep prevent timer
- TX fifo status is checked before disabling clocks, this will prevent
  on-going transmission to be cut
- Smartidle is disabled while entering idle if we have data in the transmit
  buffer because having this enabled would prevent wakeups from the TX
  interrupt and this would cause pauses while sending large blocks of data
- Sleep prevent timer is changed to use ktime_get() instead of a jiffy timer
  as jiffy timers are not valid within idle loop (tick scheduler is stopped)
- RX and TX fifos are cleared when clocks are enabled, this will purge the
  first character from RX fifo, which is most likely garbage

Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
---
 arch/arm/mach-omap2/serial.c |   70 +++++++++++++++++++++++++++---------------
 1 files changed, 45 insertions(+), 25 deletions(-)

diff --git a/arch/arm/mach-omap2/serial.c b/arch/arm/mach-omap2/serial.c
index 837b347..d7d96ba 100644
--- a/arch/arm/mach-omap2/serial.c
+++ b/arch/arm/mach-omap2/serial.c
@@ -23,12 +23,15 @@
 #include <linux/serial_reg.h>
 #include <linux/clk.h>
 #include <linux/io.h>
+#include <linux/delay.h>
 
 #include <plat/common.h>
 #include <plat/board.h>
 #include <plat/clock.h>
 #include <plat/control.h>
 
+#include <asm/div64.h>
+
 #include "prm.h"
 #include "pm.h"
 #include "prm-regbits-34xx.h"
@@ -36,13 +39,13 @@
 #define UART_OMAP_NO_EMPTY_FIFO_READ_IP_REV	0x52
 #define UART_OMAP_WER		0x17	/* Wake-up enable register */
 
-#define DEFAULT_TIMEOUT (5 * HZ)
+#define DEFAULT_TIMEOUT (5LL * NSEC_PER_SEC)
 
 struct omap_uart_state {
 	int num;
 	int can_sleep;
-	struct timer_list timer;
-	u32 timeout;
+	ktime_t expire_time;
+	u64 timeout;
 
 	void __iomem *wk_st;
 	void __iomem *wk_en;
@@ -231,6 +234,9 @@ static inline void omap_uart_enable_clocks(struct omap_uart_state *uart)
 	clk_enable(uart->fck);
 	uart->clocked = 1;
 	omap_uart_restore_context(uart);
+
+	/* Clear RX and TX fifos, as they contain garbage at this point */
+	serial_write_reg(uart->p, UART_FCR, 0xa7);
 }
 
 #ifdef CONFIG_PM
@@ -302,9 +308,7 @@ static void omap_uart_block_sleep(struct omap_uart_state *uart)
 	omap_uart_smart_idle_enable(uart, 0);
 	uart->can_sleep = 0;
 	if (uart->timeout)
-		mod_timer(&uart->timer, jiffies + uart->timeout);
-	else
-		del_timer(&uart->timer);
+		uart->expire_time = ktime_add_ns(ktime_get(), uart->timeout);
 }
 
 static void omap_uart_allow_sleep(struct omap_uart_state *uart)
@@ -317,25 +321,28 @@ static void omap_uart_allow_sleep(struct omap_uart_state *uart)
 	if (!uart->clocked)
 		return;
 
-	omap_uart_smart_idle_enable(uart, 1);
+	if (serial_read_reg(uart->p, UART_LSR) & UART_LSR_TEMT)
+		omap_uart_smart_idle_enable(uart, 1);
 	uart->can_sleep = 1;
-	del_timer(&uart->timer);
-}
-
-static void omap_uart_idle_timer(unsigned long data)
-{
-	struct omap_uart_state *uart = (struct omap_uart_state *)data;
-
-	omap_uart_allow_sleep(uart);
 }
 
 void omap_uart_prepare_idle(int num)
 {
 	struct omap_uart_state *uart;
+	ktime_t t;
 
 	list_for_each_entry(uart, &uart_list, node) {
+		if (num == uart->num && !uart->can_sleep) {
+			t = ktime_get();
+			if (t.tv64 > uart->expire_time.tv64)
+				uart->can_sleep = 1;
+		}
 		if (num == uart->num && uart->can_sleep) {
-			omap_uart_disable_clocks(uart);
+			if (serial_read_reg(uart->p, UART_LSR) &
+					UART_LSR_TEMT)
+				omap_uart_disable_clocks(uart);
+			else
+				omap_uart_smart_idle_enable(uart, 0);
 			return;
 		}
 	}
@@ -360,6 +367,7 @@ void omap_uart_resume_idle(int num)
 			/* Check for normal UART wakeup */
 			if (__raw_readl(uart->wk_st) & uart->wk_mask)
 				omap_uart_block_sleep(uart);
+
 			return;
 		}
 	}
@@ -407,8 +415,14 @@ int omap_uart_can_sleep(void)
 static irqreturn_t omap_uart_interrupt(int irq, void *dev_id)
 {
 	struct omap_uart_state *uart = dev_id;
+	u8 lsr;
 
-	omap_uart_block_sleep(uart);
+	lsr = serial_read_reg(uart->p, UART_LSR);
+	/* Check for receive interrupt */
+	if (lsr & UART_LSR_DR)
+		omap_uart_block_sleep(uart);
+	if (lsr & UART_LSR_TEMT && uart->can_sleep)
+		omap_uart_smart_idle_enable(uart, 1);
 
 	return IRQ_NONE;
 }
@@ -420,9 +434,9 @@ static void omap_uart_idle_init(struct omap_uart_state *uart)
 
 	uart->can_sleep = 0;
 	uart->timeout = DEFAULT_TIMEOUT;
-	setup_timer(&uart->timer, omap_uart_idle_timer,
-		    (unsigned long) uart);
-	mod_timer(&uart->timer, jiffies + uart->timeout);
+
+	uart->expire_time = ktime_add_ns(ktime_get(), uart->timeout);
+
 	omap_uart_smart_idle_enable(uart, 0);
 
 	if (cpu_is_omap34xx()) {
@@ -505,8 +519,12 @@ static ssize_t sleep_timeout_show(struct device *dev,
 					struct platform_device, dev);
 	struct omap_uart_state *uart = container_of(pdev,
 					struct omap_uart_state, pdev);
+	u64 val;
 
-	return sprintf(buf, "%u\n", uart->timeout / HZ);
+	val = uart->timeout;
+
+	do_div(val, NSEC_PER_SEC);
+	return sprintf(buf, "%llu\n", val);
 }
 
 static ssize_t sleep_timeout_store(struct device *dev,
@@ -524,10 +542,12 @@ static ssize_t sleep_timeout_store(struct device *dev,
 		return -EINVAL;
 	}
 
-	uart->timeout = value * HZ;
-	if (uart->timeout)
-		mod_timer(&uart->timer, jiffies + uart->timeout);
-	else
+	uart->timeout = (u64)value * NSEC_PER_SEC;
+	if (uart->timeout) {
+		uart->expire_time = ktime_get();
+		uart->expire_time =
+			ktime_add_ns(uart->expire_time, uart->timeout);
+	} else
 		/* A zero value means disable timeout feature */
 		omap_uart_block_sleep(uart);
 
-- 
1.5.4.3