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

[Guenter Roeck]

Hi Peter,

On Fri, Jan 17, 2020 at 01:42:54PM +0000, Peter Maydell wrote:
> 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" ?
> 
Sure, no problem.

> > 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.
> 
Doesn't that mean that the .post_load function is missing even today,
and that it should call exynos4210_uart_update_parameters() ?

Thanks,
Guenter