| Message ID | 1312198252-19931-1-git-send-email-hong.xu@atmel.com |
|---|---|
| State | Superseded |
| Delegated to: | Reinhard Meyer |
| Headers | show |
Dear Hong Xu, > Before reset dbgu transmitter, we just wait TXEMPTY to drain the > transmitter register. If not doing this, we may sometimes see several > weird characters from DBGU. > > A short delay is also added to make sure the new serial settings are > settled. > > Signed-off-by: Hong Xu<hong.xu@atmel.com> > --- > Change since V1: > - Fix comment for easy reading > > drivers/serial/atmel_usart.c | 8 +++++++- > 1 files changed, 7 insertions(+), 1 deletions(-) > > diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c > index e326b2b..6f9c2de 100644 > --- a/drivers/serial/atmel_usart.c > +++ b/drivers/serial/atmel_usart.c > @@ -49,17 +49,23 @@ int serial_init(void) > { > atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; > > + /* Just in case: drain transmitter register */ > + while (!(readl(&usart->csr)& USART3_BIT(TXEMPTY))) > + ; > + You still have not addressed my concern about a possible hang situation here! I rather have _some_ weird characters than an apparently dead board... When do we have the possibility of weird characters anyway? Only if a preloader makes output and transfers to u-boot before its output has been flushed. Any other situations? Best Regards, Reinhard
Hi Reinhard, On 08/01/2011 10:39 PM, Reinhard Meyer wrote: > Dear Hong Xu, > > Before reset dbgu transmitter, we just wait TXEMPTY to drain the > > transmitter register. If not doing this, we may sometimes see several > > weird characters from DBGU. > > > > A short delay is also added to make sure the new serial settings are > > settled. > > > > Signed-off-by: Hong Xu<hong.xu@atmel.com> > > --- > > Change since V1: > > - Fix comment for easy reading > > > > drivers/serial/atmel_usart.c | 8 +++++++- > > 1 files changed, 7 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c > > index e326b2b..6f9c2de 100644 > > --- a/drivers/serial/atmel_usart.c > > +++ b/drivers/serial/atmel_usart.c > > @@ -49,17 +49,23 @@ int serial_init(void) > > { > > atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; > > > > + /* Just in case: drain transmitter register */ > > + while (!(readl(&usart->csr)& USART3_BIT(TXEMPTY))) > > + ; > > + > > You still have not addressed my concern about a possible hang situation > here! > I rather have _some_ weird characters than an apparently dead board... > When do we have the possibility of weird characters anyway? > Only if a preloader makes output and transfers to u-boot before its output > has been flushed. Any other situations? This is the main cause. I monitored some versions of bootstraps check TXRDY before sending data to THR (Transmitter holding register). And for the last character, they won't wait till the character has been sent out. (And maybe there are similar places in U-Boot before calling serial_init, I'm not sure) The possibility of hang, I don't think so. Reason, TXEMPTY means "There are no characters in DBGU_THR and there are no characters being processed by the transmitter." Theoretically this situation shall be met here. But of course a timeout can be added defensively (chip goes mad? :) What's your opinion? BR, Eric > > Best Regards, > Reinhard >
Dear Hong Xu, > > > diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c > > > index e326b2b..6f9c2de 100644 > > > --- a/drivers/serial/atmel_usart.c > > > +++ b/drivers/serial/atmel_usart.c > > > @@ -49,17 +49,23 @@ int serial_init(void) > > > { > > > atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; > > > > > > + /* Just in case: drain transmitter register */ > > > + while (!(readl(&usart->csr)& USART3_BIT(TXEMPTY))) > > > + ; > > > + > > > > You still have not addressed my concern about a possible hang situation > > here! > > I rather have _some_ weird characters than an apparently dead board... > > When do we have the possibility of weird characters anyway? > > Only if a preloader makes output and transfers to u-boot before its output > > has been flushed. Any other situations? > > This is the main cause. I monitored some versions of bootstraps check > TXRDY before sending data to THR (Transmitter holding register). And for > the last character, they won't wait till the character has been sent > out. (And maybe there are similar places in U-Boot before calling > serial_init, I'm not sure) There should be no output made by u-boot BEFORE serial_init() is called! > > The possibility of hang, I don't think so. Reason, > TXEMPTY means "There are no characters in DBGU_THR and there are no > characters being processed by the transmitter." What if there is no Baud Clock? What if the USART is unconfigured or misconfigured? > > Theoretically this situation shall be met here. But of course a timeout > can be added defensively (chip goes mad? :) > > What's your opinion? Timeout is complicated at the pre-relocation level. What is MOST IMPORTANT at this point is that u-boot must made be able to print messages. Anything that could perhaps impair that should not be done. Keep the code simple and live with occasional weird characters. End users of a system will usually not look at the serial debug output. Or just use a __udelay(1000) - that should give time to drain any char at 9600 bit/s and above. Best Regards, Reinhard
Hi Reinhard, On 08/02/2011 05:54 PM, Reinhard Meyer wrote: > Dear Hong Xu, > > > > diff --git a/drivers/serial/atmel_usart.c > b/drivers/serial/atmel_usart.c > > > > index e326b2b..6f9c2de 100644 > > > > --- a/drivers/serial/atmel_usart.c > > > > +++ b/drivers/serial/atmel_usart.c > > > > @@ -49,17 +49,23 @@ int serial_init(void) > > > > { > > > > atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; > > > > > > > > + /* Just in case: drain transmitter register */ > > > > + while (!(readl(&usart->csr)& USART3_BIT(TXEMPTY))) > > > > + ; > > > > + > > > > > > You still have not addressed my concern about a possible hang situation > > > here! > > > I rather have _some_ weird characters than an apparently dead board... > > > When do we have the possibility of weird characters anyway? > > > Only if a preloader makes output and transfers to u-boot before its > output > > > has been flushed. Any other situations? > > > > This is the main cause. I monitored some versions of bootstraps check > > TXRDY before sending data to THR (Transmitter holding register). And for > > the last character, they won't wait till the character has been sent > > out. (And maybe there are similar places in U-Boot before calling > > serial_init, I'm not sure) > > There should be no output made by u-boot BEFORE serial_init() is called! > > > > > The possibility of hang, I don't think so. Reason, > > TXEMPTY means "There are no characters in DBGU_THR and there are no > > characters being processed by the transmitter." > > What if there is no Baud Clock? > What if the USART is unconfigured or misconfigured? > > > > > Theoretically this situation shall be met here. But of course a timeout > > can be added defensively (chip goes mad? :) > > > > What's your opinion? > > Timeout is complicated at the pre-relocation level. > What is MOST IMPORTANT at this point is that u-boot must made be able to > print messages. Anything that could perhaps impair that should not be > done. > Keep the code simple and live with occasional weird characters. > End users of a system will usually not look at the serial debug output. > Or just use a __udelay(1000) - that should give time to drain any char > at 9600 bit/s and above. Agree. Actually I tried __udelay to overcome the weird character issue firstly. And later found the TXEMPTY was not set when serial_init is called. I'll send another patch soon (by using __udelay, simple but useful enough :-). Thanks BR, Eric > Best Regards, > Reinhard > >
diff --git a/drivers/serial/atmel_usart.c b/drivers/serial/atmel_usart.c index e326b2b..6f9c2de 100644 --- a/drivers/serial/atmel_usart.c +++ b/drivers/serial/atmel_usart.c @@ -49,17 +49,23 @@ int serial_init(void) { atmel_usart3_t *usart = (atmel_usart3_t *)CONFIG_USART_BASE; + /* Just in case: drain transmitter register */ + while (!(readl(&usart->csr) & USART3_BIT(TXEMPTY))) + ; + writel(USART3_BIT(RSTRX) | USART3_BIT(RSTTX), &usart->cr); serial_setbrg(); - writel(USART3_BIT(RXEN) | USART3_BIT(TXEN), &usart->cr); writel((USART3_BF(USART_MODE, USART3_USART_MODE_NORMAL) | USART3_BF(USCLKS, USART3_USCLKS_MCK) | USART3_BF(CHRL, USART3_CHRL_8) | USART3_BF(PAR, USART3_PAR_NONE) | USART3_BF(NBSTOP, USART3_NBSTOP_1)), &usart->mr); + writel(USART3_BIT(RXEN) | USART3_BIT(TXEN), &usart->cr); + /* 100us is enough for the new settings to be settled */ + __udelay(100); return 0; }
Before reset dbgu transmitter, we just wait TXEMPTY to drain the transmitter register. If not doing this, we may sometimes see several weird characters from DBGU. A short delay is also added to make sure the new serial settings are settled. Signed-off-by: Hong Xu <hong.xu@atmel.com> --- Change since V1: - Fix comment for easy reading drivers/serial/atmel_usart.c | 8 +++++++- 1 files changed, 7 insertions(+), 1 deletions(-)