+Damien On 6/30/20 10:12 PM, Alistair Francis wrote: > Conver the Ibex UART to use the recently added qdev-clock functions.
Yeah! This is our first user \o/ > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > --- > include/hw/char/ibex_uart.h | 2 ++ > hw/char/ibex_uart.c | 19 ++++++++++++++++++- > 2 files changed, 20 insertions(+), 1 deletion(-) > > diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h > index 2bec772615..322bfffd8b 100644 > --- a/include/hw/char/ibex_uart.h > +++ b/include/hw/char/ibex_uart.h > @@ -101,6 +101,8 @@ typedef struct { > uint32_t uart_val; > uint32_t uart_timeout_ctrl; > > + Clock *f_clk; > + > CharBackend chr; > qemu_irq tx_watermark; > qemu_irq rx_watermark; > diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c > index 45cd724998..f967e6919a 100644 > --- a/hw/char/ibex_uart.c > +++ b/hw/char/ibex_uart.c > @@ -28,6 +28,7 @@ > #include "qemu/osdep.h" > #include "hw/char/ibex_uart.h" > #include "hw/irq.h" > +#include "hw/qdev-clock.h" > #include "hw/qdev-properties.h" > #include "migration/vmstate.h" > #include "qemu/log.h" > @@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr, > } > if (value & UART_CTRL_NCO) { > uint64_t baud = ((value & UART_CTRL_NCO) >> 16); UART_CTRL_NCO is defined as: #define UART_CTRL_NCO (0xFFFF << 16) Note for later, convert to the clearer registerfields API? > - baud *= 1000; > + baud *= clock_get_hz(s->f_clk); > baud >>= 20; > > s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10; > @@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr, > } > } > > +static void ibex_uart_clk_update(void *opaque) > +{ > + IbexUartState *s = opaque; > + > + /* recompute uart's speed on clock change */ > + uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16); > + baud *= clock_get_hz(s->f_clk); > + baud >>= 20; Maybe worth to extract: uint64_t ibex_uart_get_baud(IbexUartState *s) { uint64_t baud; baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16); baud *= clock_get_hz(s->f_clk); baud >>= 20; return baud; } > + > + s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10; > +} > + > static void fifo_trigger_update(void *opaque) > { > IbexUartState *s = opaque; > @@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj) > { > IbexUartState *s = IBEX_UART(obj); > > + s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock", > + ibex_uart_clk_update, s); > + clock_set_hz(s->f_clk, 50000000); Can you add a definition for this 50 MHz value: Otherwise: Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> > + > sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark); > sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark); > sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty); >