From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44377) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJNhU-0002sP-Uq for qemu-devel@nongnu.org; Fri, 28 Feb 2014 08:42:09 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WJNhQ-0003zU-66 for qemu-devel@nongnu.org; Fri, 28 Feb 2014 08:42:04 -0500 Received: from mail-la0-f42.google.com ([209.85.215.42]:52632) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WJNhP-0003zC-VG for qemu-devel@nongnu.org; Fri, 28 Feb 2014 08:42:00 -0500 Received: by mail-la0-f42.google.com with SMTP id ec20so2735465lab.1 for ; Fri, 28 Feb 2014 05:41:58 -0800 (PST) MIME-Version: 1.0 In-Reply-To: References: <1393551441-18041-1-git-send-email-robherring2@gmail.com> From: Rob Herring Date: Fri, 28 Feb 2014 07:41:38 -0600 Message-ID: Content-Type: text/plain; charset=ISO-8859-1 Subject: Re: [Qemu-devel] [PATCH] pl011: fix corrupting the UARTCR value List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: Rob Herring , QEMU Developers , Christoffer Dall On Fri, Feb 28, 2014 at 4:41 AM, Peter Maydell wrote: > On 28 February 2014 01:37, Rob Herring wrote: >> From: Rob Herring >> >> Offset 4 is UARTRSR/UARTECR, not the UARTCR. As framing and parity errors >> will never occur, we can ignore writes to this register. > > It's true that we'll never get framing or parity errors, > but we can get break (and we ought to get overrun if we > implemented it properly). This should be fairly easy to > implement: > * add rsr to the state struct > * clear it on reset [ideally, extra patch to pull reset out > of the pl011_init fn into an actual device reset method] > * writes to UARTRSR clear rsr > * reads from UARTRSR return rsr > * in the code for read of UARTDR, copy bits [10:8] of > the value we're about to return into rsr [2:1] > (break/parity/framing) Okay, that's simple enough. > * for overrun, we should set the rsr bit in pl011_put_fifo > if the fifo is full; however, it looks from that function > as if we don't properly implement the documented behaviour > for a full fifo (s2.4.3 "UART operation" of the PL011 TRM > is a pretty clear description) so I'd be OK with just adding > a comment to pl011_put_fifo(): > /* FIXME: FIFO overrun handling of excess data and setting > * overrun status bits is not correctly implemented: > * see PL011 TRM s2.4.3 "UART operation". > */ > (Of course if you want to implement overrun properly > feel free ;-)) I don't think we can get an overrun. pl011_can_receive should prevent ever getting a 17th character. If we allowed that, then we would get overruns all the time. Effectively there is flow-control between qemu and the pl011 model since there is no baudrate to limit the receive speed. I suppose we could allow a 17th character modeling the shift register, but that seems to be rather pointless complicating of the model. We'd be better off spending time properly modelling the fifo trigger level and receive timeout. That would have some actual benefit. Rob