Re: [PATCH 4/6] hw/char/exynos4210_uart: Implement receive FIFO

[Peter Maydell]

On Fri, 10 Jan 2020 at 20:39, Guenter Roeck <address@hidden> wrote:

The subject just says "implement receive FIFO", but the
existing code clearly has an "Exynos4210UartFIFO   rx"
which it does some storing and retrieving data from.
Could the patch be more accurately described as something
like "Implement interrupts for rx FIFO level triggers and
timeouts" ?

> Signed-off-by: Guenter Roeck <address@hidden>
> ---
>  hw/char/exynos4210_uart.c | 120 ++++++++++++++++++++++++++++++--------
>  hw/char/trace-events      |   3 +-
>  2 files changed, 97 insertions(+), 26 deletions(-)
>
> diff --git a/hw/char/exynos4210_uart.c b/hw/char/exynos4210_uart.c
> index fb7a3ebd09..61134a7bdc 100644
> --- a/hw/char/exynos4210_uart.c
> +++ b/hw/char/exynos4210_uart.c
> @@ -24,6 +24,7 @@
>  #include "migration/vmstate.h"
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
> +#include "qemu/timer.h"
>  #include "chardev/char-fe.h"
>  #include "chardev/char-serial.h"
>
> @@ -118,6 +119,7 @@ static const Exynos4210UartReg exynos4210_uart_regs[] = {
>  #define ULCON_STOP_BIT_SHIFT  1
>
>  /* UART Tx/Rx Status */
> +#define UTRSTAT_Rx_TIMEOUT              0x8
>  #define UTRSTAT_TRANSMITTER_EMPTY       0x4
>  #define UTRSTAT_Tx_BUFFER_EMPTY         0x2
>  #define UTRSTAT_Rx_BUFFER_DATA_READY    0x1
> @@ -147,6 +149,9 @@ typedef struct Exynos4210UartState {
>      ";
>      Exynos4210UartFIFO   tx;
>
> +    QEMUTimer *fifo_timeout_timer;
> +    uint64_t wordtime;        /* word time in ns */

You need to do something on incoming migration to handle
the new fields. This probably looks like a .post_load function
that calculates the wordtime based on the register values
that have been set by the incoming migration and set the
QEMUTimer appropriately.

(Side note while I'm thinking about it: this device has
an "Exynos4210UartFIFO tx" but it never does anything
with it except call fifo_reset() on it. We don't migrate
it either, which is a bit of a bear trap for anybody who
adds code that uses it in future...)

thanks
-- PMM