1. Patchew
  2. QEMU
  3. [PATCH v1 0/3] A few RISC-V fixes
  4. View patch

[PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model

Alistair Francis posted 3 patches 5 years, 7 months ago
Maintainers: Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <Alistair.Francis@wdc.com>, Paolo Bonzini <pbonzini@redhat.com>, Palmer Dabbelt <palmer@dabbelt.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>, "Marc-André Lureau" <marcandre.lureau@redhat.com>
There is a newer version of this series
[PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
Posted by Alistair Francis 5 years, 7 months ago 
Conver the Ibex UART to use the recently added qdev-clock functions.

Signed-off-by: Alistair Francis <alistair.francis@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);
-            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;
+
+    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);
+
     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);
-- 
2.27.0
Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
Posted by Philippe Mathieu-Daudé 5 years, 7 months ago 
+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.francis@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é <f4bug@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);
>
Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
Posted by Alistair Francis 5 years, 7 months ago 
On Fri, Jul 3, 2020 at 12:42 AM Philippe Mathieu-Daudé <f4bug@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.francis@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é <f4bug@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);
> >
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.