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

[Rob Herring]

On Fri, Feb 28, 2014 at 4:41 AM, Peter Maydell <address@hidden> wrote:
> On 28 February 2014 01:37, Rob Herring <address@hidden> wrote:
>> From: Rob Herring <address@hidden>
>>
>> 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