Re: [PATCH v5 07/11] hw/char: Initial commit of Ibex UART

[Alistair Francis]

On Tue, Jun 2, 2020 at 5:28 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
> Hi Alistair,
>
> There are still some questions I don't understand.
>
> 1. Is the baud rate  or fifo a necessary feature to simulate?
> As you can see, qemu_chr_fe_write will send the byte as soon as possible.
> When you want to transmit a byte through WDATA,  you can call
> qemu_chr_fe_write directly.

So qemu_chr_fe_write() will send the data straight away. This doesn't
match what teh hardware does though. So by modelling a FIFO and a
delay in sending we can better match the hardware.

>
> 2.  The baud rate calculation method is not strictly right.
> I think when a byte write to FIFO,  char_tx_time * 8 is the correct time
> to send the byte instead of
> char_tx_time * 4.

Do you mind explaining why 8 is correct instead of 4?

>
> 3.  Why add a watch here?

This is based on the Cadence UART implementation in QEMU (which does
the same thing). This will trigger a callback when we can write more
data or when the backend has hung up.

Alistair

> > +        s->uart_status |= UART_STATUS_TXEMPTY;
> > +        s->uart_intr_state |= INTR_STATE_TX_EMPTY;
> > +        s->uart_intr_state &= ~INTR_STATE_TX_WATERMARK;
> > +        ibex_uart_update_irqs(s);
> > +        return FALSE;
> > +    }
> > +
> > +    ret = qemu_chr_fe_write(&s->chr, s->tx_fifo, s->tx_level);
> > +
> > +    if (ret >= 0) {
> > +        s->tx_level -= ret;
> > +        memmove(s->tx_fifo, s->tx_fifo + ret, s->tx_level);
> > +    }
> > +
> > +    if (s->tx_level) {
> > +        guint r = qemu_chr_fe_add_watch(&s->chr, G_IO_OUT | G_IO_HUP,
> > +                                        ibex_uart_xmit, s);
> > +        if (!r) {
> > +            s->tx_level = 0;
> > +            return FALSE;
> > +        }
> > +    }
> > +
>
> Zhiwei
>
>