Re: [Qemu-devel] [PATCH] pl011: fix corrupting the UARTCR value

[Peter Maydell <peter.maydell@linaro.org>]

On 28 February 2014 01:37, Rob Herring <robherring2@gmail.com> wrote:
> From: Rob Herring <rob.herring@linaro.org>
>
> 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)
 * 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 ;-))

thanks
-- PMM