Re: [PATCH] hw/char: disable ibex uart receive if the buffer is full

[Alexander Wagner <alexander.wagner@ulal.de>]

[-- Attachment #1: Type: text/plain, Size: 1754 bytes --]


On 09.03.21 15:29, Alistair Francis wrote:
> On Tue, Mar 9, 2021 at 2:27 AM Alexander Wagner
> <alexander.wagner@ulal.de> wrote:
>>
>> On 08.03.21 14:47, Alistair Francis wrote:
>>>>    hw/char/ibex_uart.c         | 20 +++++++++++++++-----
>>>>    include/hw/char/ibex_uart.h |  4 ++++
>>>>    2 files changed, 19 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
>>>> index 89f1182c9b..dac09d53d6 100644
>>>> --- a/hw/char/ibex_uart.c
>>>> +++ b/hw/char/ibex_uart.c
>>>> @@ -66,7 +66,8 @@ static int ibex_uart_can_receive(void *opaque)
>>>>    {
>>>>        IbexUartState *s = opaque;
>>>>
>>>> -    if (s->uart_ctrl & R_CTRL_RX_ENABLE_MASK) {
>>>> +    if ((s->uart_ctrl & R_CTRL_RX_ENABLE_MASK)
>>>> +           && !(s->uart_status & R_STATUS_RXFULL_MASK)) {
>>>>            return 1;
>>>>        }
>>>>
>>>> @@ -83,6 +84,8 @@ static void ibex_uart_receive(void *opaque, const uint8_t *buf, int size)
>>>>
>>>>        s->uart_status &= ~R_STATUS_RXIDLE_MASK;
>>>>        s->uart_status &= ~R_STATUS_RXEMPTY_MASK;
>>>> +    s->uart_status |= R_STATUS_RXFULL_MASK;
>>> Doesn't this mean we set RXFULL on every receive? Shouldn't this check
>>> the rx_level first?
>>>
>>> Alistair
>> Thank you for having a look! :)
>>
>> Yes, this is correct. The RXFULL is currently set on every receive. The
>> RXFULL is used to indicate to QEMU that the device cannot receive any
>> further bytes.
>>
>> As the FIFO buffers are currently not yet implemented I thought it would
>> make sense to behave like the OT UART could only receive one byte at a time.
> Ah, good point.
>
> Can you add a comment where it is set describing that then?
>
> Alistair
>
Sure, I just added a comment and emailed this as patch v2.

Alex


[-- Attachment #2: Type: text/html, Size: 2516 bytes --]