Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 26 Oct 2013 02:14:50 +0200
From:      Zbigniew Bodek <zbb@freebsd.org>
To:        "freebsd-embedded@freebsd.org" <freebsd-embedded@freebsd.org>
Cc:        "freebsd-arm@freebsd.org" <freebsd-arm@freebsd.org>, freebsd-current <freebsd-current@freebsd.org>
Subject:   Re: Changes to UART ns8250
Message-ID:  <CALF_Tx=9-s2emKfFR8f_CtzHYZ9qUAX31rhSU0%2BZMNyboPiw2g@mail.gmail.com>
In-Reply-To: <CALF_TxnESbdw_jfbut3toJFH0F96RFiSLb8EPd_t0d5UojGHCQ@mail.gmail.com>
References:  <CALF_Tx=AwVnr0d75-K-yu97iVgmTJC7aaABoix73zHD%2B5eKJnQ@mail.gmail.com> <CAGtf9xNZekmye4=JuYmEct-h9CG4ckHWihfCKJrmroK3Sb3jDQ@mail.gmail.com> <CAGtf9xNnBc1=oTxFv_sEXJRsnWYEV6aQ=xr1V0-YwHXcQRuJ0g@mail.gmail.com> <CALF_TxnESbdw_jfbut3toJFH0F96RFiSLb8EPd_t0d5UojGHCQ@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help

[-- Attachment #1 --]
Hello Everyone,

I'm attaching the newest version of the ns8250 UART patch.
After some discussions I decided to go with busy-wait + timeout solution.
The applied delay should handle exotic corner cases (please notice
that we can't use ns8250_delay()
to get the actual single transmission time since we know that LCR is locked).

If there are no objections then I would like to commit this soon.

Best regards
Zbigniew Bodek

2013/10/9 Zbigniew Bodek <zbb@freebsd.org>:
> Hello Ganbold.
>
> Thank you for testing the patch and pointing those issue out.
> My detection log from Armada XP:
>
> uart0: <16550 or compatible> mem 0xd0012000-0xd001201f irq 41 on simplebus0
> uart0: console (115200,n,8,1)
> uart1: <16550 or compatible> mem 0xd0012100-0xd001211f irq 42 on simplebus0
> uart2: <16550 or compatible> mem 0xd0012200-0xd001221f irq 43 on simplebus0
> uart3: <16550 or compatible> mem 0xd0012300-0xd001231f irq 44 on simplebus0
>
> Is there a possibility to download a datasheet for RK30xx so that I could
> verify what is required for it's UART?
> The patch is causing that we only wait until UART is not busy anymore. I
> can't find why would that cause problems
> if your UART requires busy detection anyway.
>
> Best regards
> Zbigniew Bodek
>
>
>
> 2013/10/9 Ganbold Tsagaankhuu <ganbold@gmail.com>
>>
>>
>>
>>
>> On Tue, Oct 8, 2013 at 9:58 AM, Ganbold Tsagaankhuu <ganbold@gmail.com>
>> wrote:
>>>
>>> Zbigniew,
>>>
>>>
>>> On Tue, Oct 8, 2013 at 3:54 AM, Zbigniew Bodek <zbb@freebsd.org> wrote:
>>>>
>>>> Hello.
>>>>
>>>> I would like to present a patch for ns8250 serial that I would like to
>>>> commit in the near future (if there are no objections).
>>>>
>>>> The patch is fixing newest DesignWare UART with busy detection.
>>>> During frequency divisors configuration when UART is busy transferring
>>>> or
>>>> receiving data, line control register manipulation will not take effect.
>>>> Therefore, we will not set divisor latch access bit and we will corrupt
>>>> LCR
>>>> instead of configuring divisors.
>>>> It is necessary to wait until UART finishes all transfers to proceed
>>>> with
>>>> the configuration.
>>>>
>>>> This was detected on Armada XP as UART fails on this issue 100/100
>>>> attempts.
>>>> The patch was tested by kevlo@ and me and it works on our Armada XP -
>>>> based
>>>> systems.
>>>>
>>>> Please send your comment or remarks if there are any.
>>>
>>>
>>> I'm trying your patch on r254983.
>>> Tried on 2 boards (Cubieboard2 (Allwinner A20 SoC - dual Cortex A7) and
>>> Radxa Rock (Rockchip RK3188 - Quad Cortex A9)). Both seem to have some sort
>>> of DesignWare uart.
>>>
>>> 1. It works fine on Cubieboard2. Uart dmesg is like:
>>>
>>> uart0: <16750 or compatible> mem 0x1c28000-0x1c283ff irq 33 on simplebus0
>>> uart0: console (115200,n,8,1)
>>>
>>> 2. No any printing on screen in case of Radxa Rock. Without your patch
>>> uart dmesg is like:
>>>
>>> uart0: <16650 or compatible> mem 0x20064000-0x200643ff irq 68 on
>>> simplebus0
>>> uart0: console (115200,n,8,1)
>>>
>>> In case of RK3188 SoC, it seems booting FreeBSD kernel seems very
>>> fragile, not sure yet what is causing the problem.
>>> Even with stock ns8250 some version later than r254983 didn't show/print
>>> anything on serial console few days ago.
>>> Only thing so far I know is this r254983 (with some patch) works in my
>>> case on RK3188 SoC based board.
>>
>>
>>
>> Zbigniew,
>>
>> Just tried again your patch on RK30xx board. I was able to see boot
>> messages on screen.
>> This uart detected as:
>> ...
>> uart0: <16650 or compatible> mem 0x20064000-0x200643ff irq 68 on
>> simplebus0
>> uart0: console (115200,n,8,1)
>> uart0: fast interrupt
>> ...
>> Can you show me your uart detection log?
>> It seems this DW uart of RK30xx is different than DW uart of A10/A20.
>> Boot simply stops printing "start_init: trying /sbin/init".
>>
>> thanks,
>>
>> Ganbold
>>
>>
>>>
>>>
>>> thanks,
>>>
>>> Ganbold
>>>
>>>
>>>
>>>>
>>>>
>>>> Best regards
>>>> Zbigniew Bodek
>>>>
>>>> _______________________________________________
>>>> freebsd-current@freebsd.org mailing list
>>>> http://lists.freebsd.org/mailman/listinfo/freebsd-current
>>>> To unsubscribe, send any mail to
>>>> "freebsd-current-unsubscribe@freebsd.org"
>>>
>>>
>>
>

[-- Attachment #2 --]
From 0b21e26060fc9cdf23e10fd1b1be566b5c738f2b Mon Sep 17 00:00:00 2001
From: Zbigniew Bodek <zbb@semihalf.com>
Date: Sat, 5 Oct 2013 02:25:23 +0200
Subject: [PATCH] Wait for DesignWare UART transfers completion before
 accessing line control

When using DW UART with BUSY detection it is necessary to wait
until all serial transfers are finished before manipulating the
line control. LCR will not be affected when UART is busy.
In addition, if Divisor Latch Access Bit is being set in order to
modify UART divisors:
1. We will get BUSY interrupt if interrupts are enabled.
2. Because LCR will not be affected the THR and (even worse) IER
   contents will be corrupted. This will lead to console hang.

Approved by:	cognet (mentor)
---
 sys/dev/ic/ns16550.h           |  1 +
 sys/dev/uart/uart_dev_ns8250.c | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/sys/dev/ic/ns16550.h b/sys/dev/ic/ns16550.h
index 659f591..33a7dd1 100644
--- a/sys/dev/ic/ns16550.h
+++ b/sys/dev/ic/ns16550.h
@@ -185,6 +185,7 @@
 #define DW_REG_USR	31	/* DesignWare derived Uart Status Reg */
 #define com_usr		39	/* Octeon 16750/16550 Uart Status Reg */
 #define REG_USR		com_usr
+#define USR_BUSY	1	/* Uart Busy. Serial transfer in progress */
 #define USR_TXFIFO_NOTFULL 2    /* Uart TX FIFO Not full */
 
 /* 16950 register #1.  Access enabled by ACR[7].  Also requires !LCR[7]. */
diff --git a/sys/dev/uart/uart_dev_ns8250.c b/sys/dev/uart/uart_dev_ns8250.c
index 211d113..249be4c 100644
--- a/sys/dev/uart/uart_dev_ns8250.c
+++ b/sys/dev/uart/uart_dev_ns8250.c
@@ -647,11 +647,35 @@ int
 ns8250_bus_param(struct uart_softc *sc, int baudrate, int databits,
     int stopbits, int parity)
 {
+	struct ns8250_softc *ns8250;
 	struct uart_bas *bas;
-	int error;
+	int error, limit;
 
+	ns8250 = (struct ns8250_softc*)sc;
 	bas = &sc->sc_bas;
 	uart_lock(sc->sc_hwmtx);
+	/*
+	 * When using DW UART with BUSY detection it is necessary to wait
+	 * until all serial transfers are finished before manipulating the
+	 * line control. LCR will not be affected when UART is busy.
+	 */
+	if (ns8250->busy_detect != 0) {
+		/*
+		 * Pick an arbitrary high limit to avoid getting stuck in
+		 * an infinite loop in case when the hardware is broken.
+		 */
+		limit = 10 * 1024;
+		while (((uart_getreg(bas, DW_REG_USR) & USR_BUSY) != 0) &&
+		    --limit)
+			DELAY(4);
+
+		if (limit <= 0) {
+			/* UART appears to be stuck */
+			uart_unlock(sc->sc_hwmtx);
+			return (EIO);
+		}
+	}
+
 	error = ns8250_param(bas, baudrate, databits, stopbits, parity);
 	uart_unlock(sc->sc_hwmtx);
 	return (error);
-- 
1.8.4


Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CALF_Tx=9-s2emKfFR8f_CtzHYZ9qUAX31rhSU0%2BZMNyboPiw2g>