Patchwork [U-Boot,6/8] Adds wait to atmel_usart serial_init function

login
register
mail settings
Submitter Alex
Date June 30, 2011, 7:33 p.m.
Message ID <1309462387-22926-7-git-send-email-awaterman@dawning.com>
Download mbox | patch
Permalink /patch/102813/
State Changes Requested
Delegated to: Reinhard Meyer
Headers show

Comments

Alex - June 30, 2011, 7:33 p.m.
Adds a short busy loop wait to the atmel_usart.c serial_init()
function.

Signed-off-by: Alex Waterman <awaterman@dawning.com>
---
 drivers/serial/atmel_usart.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)
Sergei Shtylyov - July 1, 2011, 12:03 p.m.
Hello.

On 30-06-2011 23:33, Alex Waterman wrote:

> Adds a short busy loop wait to the atmel_usart.c serial_init()
> function.

> Signed-off-by: Alex Waterman<awaterman@dawning.com>

> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
> index e326b2b..e355706 100644
> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -47,6 +47,8 @@ void serial_setbrg(void)
>
>   int serial_init(void)
>   {
> +
> +	volatile int i = 0;

    Why 'volatile'? Also, there shouldn't be an empty line before it but there 
should be one after it...

>   	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
>
>   	writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX),&usart->cr);
> @@ -61,6 +63,10 @@ int serial_init(void)
>   			   | USART3_BF(NBSTOP, USART3_NBSTOP_1)),
>   			&usart->mr);
>
> +	/* Short wait to let the serial port init. */
> +	for (; i < 10000; i++)
> +		;
> +
>   	return 0;
>   }

WBR, Sergei
Alex - July 1, 2011, 12:37 p.m.
>    Why 'volatile'? Also, there shouldn't be an empty line before it but there should be one after it...
I used a volatile to prevent the compiler from optimizing out the (normally) useless loop. Is there a cleaner way to do this?

Best regards,
Alex
Albert ARIBAUD - July 1, 2011, 3:17 p.m.
Hi Alex,

Le 01/07/2011 14:37, Alex Waterman a écrit :
>
>>     Why 'volatile'? Also, there shouldn't be an empty line before it but there should be one after it...
> I used a volatile to prevent the compiler from optimizing out the (normally) useless loop. Is there a cleaner way to do this?

If the board supports udelay(), you should use that instead of a loop.

> Best regards,
> Alex

Amicalement,
Reinhard Meyer - July 4, 2011, 7:42 a.m.
Dear Alex Waterman,
> Adds a short busy loop wait to the atmel_usart.c serial_init()
> function.
> 
> Signed-off-by: Alex Waterman <awaterman@dawning.com>
> ---
>  drivers/serial/atmel_usart.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
> index e326b2b..e355706 100644
> --- a/drivers/serial/atmel_usart.c
> +++ b/drivers/serial/atmel_usart.c
> @@ -47,6 +47,8 @@ void serial_setbrg(void)
>  
>  int serial_init(void)
>  {
> +
> +	volatile int i = 0;
>  	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
>  
>  	writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX), &usart->cr);
> @@ -61,6 +63,10 @@ int serial_init(void)
>  			   | USART3_BF(NBSTOP, USART3_NBSTOP_1)),
>  			   &usart->mr);
>  
> +	/* Short wait to let the serial port init. */
> +	for (; i < 10000; i++)
> +		;
> +
>  	return 0;
>  }

1. Why is this delay needed in the first place?
Apparently it has not been required before.

2. Such a delay loop is highly dependant on toolchain,
cache and page situation and processor speed. It can not be
accepted as a solution. If a delay is really required here
and cannot be substituting by reading the UARTs status
register, udelay() must be used.

Best Regards,
Reinhard

Patch

diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c
index e326b2b..e355706 100644
--- a/drivers/serial/atmel_usart.c
+++ b/drivers/serial/atmel_usart.c
@@ -47,6 +47,8 @@  void serial_setbrg(void)
 
 int serial_init(void)
 {
+
+	volatile int i = 0;
 	atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE;
 
 	writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX), &usart->cr);
@@ -61,6 +63,10 @@  int serial_init(void)
 			   | USART3_BF(NBSTOP, USART3_NBSTOP_1)),
 			   &usart->mr);
 
+	/* Short wait to let the serial port init. */
+	for (; i < 10000; i++)
+		;
+
 	return 0;
 }