On Fri, Jul 3, 2020 at 12:42 AM Philippe Mathieu-Daudé <f4...@amsat.org> wrote: > > +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? Done, I have added a patch to do this. > > > - 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; > } Done. > > > + > > + 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: Done. > > Otherwise: > Reviewed-by: Philippe Mathieu-Daudé <f4...@amsat.org> Thanks! Alistair > > > + > > 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); > > >