[PATCH 0/8] hw/arm: Cleanups around QOM style

[PATCH 0/8] hw/arm: Cleanups around QOM style

[Philippe Mathieu-Daudé]

- Reduce "hw/qdev-properties.h" inclusions
- Open-code various QDev helpers used few times
- Use QOM cast macro when relevant

Philippe Mathieu-Daudé (8):
  hw/gpio/max7310: Simplify max7310_realize()
  hw/char/pl011: Un-inline pl011_create()
  hw/char/pl011: Open-code pl011_luminary_create()
  hw/char/xilinx_uartlite: Expose XILINX_UARTLITE QOM type
  hw/char/xilinx_uartlite: Open-code xilinx_uartlite_create()
  hw/char/cmsdk-apb-uart: Open-code cmsdk_apb_uart_create()
  hw/timer/cmsdk-apb-timer: Remove unused 'qdev-properties.h' header
  hw/intc/armv7m_nvic: Use QOM cast CPU() macro

 hw/arm/mps2.c                            | 41 ++++++++++++++++--------
 hw/arm/stellaris.c                       | 11 +++++--
 hw/char/pl011.c                          | 17 ++++++++++
 hw/char/xilinx_uartlite.c                |  4 +--
 hw/gpio/max7310.c                        |  5 ++-
 hw/intc/armv7m_nvic.c                    |  6 ++--
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  7 ++--
 include/hw/char/cmsdk-apb-uart.h         | 34 --------------------
 include/hw/char/pl011.h                  | 36 +--------------------
 include/hw/char/xilinx_uartlite.h        | 22 ++-----------
 include/hw/timer/cmsdk-apb-timer.h       |  1 -
 11 files changed, 67 insertions(+), 117 deletions(-)

-- 
2.38.1

[PATCH 1/8] hw/gpio/max7310: Simplify max7310_realize()

[Philippe Mathieu-Daudé]

Since &I2C_SLAVE(dev)->qdev == dev, no need to go back and
forth with QOM type casting. Directly use 'dev'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/gpio/max7310.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/gpio/max7310.c b/hw/gpio/max7310.c
index db6b5e3d76..031482d939 100644
--- a/hw/gpio/max7310.c
+++ b/hw/gpio/max7310.c
@@ -183,11 +183,10 @@ static void max7310_gpio_set(void *opaque, int line, int level)
  * but also accepts sequences that are not SMBus so return an I2C device.  */
 static void max7310_realize(DeviceState *dev, Error **errp)
 {
-    I2CSlave *i2c = I2C_SLAVE(dev);
     MAX7310State *s = MAX7310(dev);
 
-    qdev_init_gpio_in(&i2c->qdev, max7310_gpio_set, 8);
-    qdev_init_gpio_out(&i2c->qdev, s->handler, 8);
+    qdev_init_gpio_in(dev, max7310_gpio_set, ARRAY_SIZE(s->handler));
+    qdev_init_gpio_out(dev, s->handler, ARRAY_SIZE(s->handler));
 }
 
 static void max7310_class_init(ObjectClass *klass, void *data)
-- 
2.38.1

Re: [PATCH 1/8] hw/gpio/max7310: Simplify max7310_realize()

[Richard Henderson]

On 2/20/23 01:51, Philippe Mathieu-Daudé wrote:
> Since &I2C_SLAVE(dev)->qdev == dev, no need to go back and
> forth with QOM type casting. Directly use 'dev'.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/gpio/max7310.c | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 2/8] hw/char/pl011: Un-inline pl011_create()

[Philippe Mathieu-Daudé]

pl011_create() is only used in DeviceRealize handlers,
not a hot-path. Inlining is not justified.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/pl011.c         | 17 +++++++++++++++++
 include/hw/char/pl011.h | 19 +------------------
 2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/hw/char/pl011.c b/hw/char/pl011.c
index c15cb7af20..77bbc2a982 100644
--- a/hw/char/pl011.c
+++ b/hw/char/pl011.c
@@ -19,10 +19,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "qapi/error.h"
 #include "hw/char/pl011.h"
 #include "hw/irq.h"
 #include "hw/sysbus.h"
 #include "hw/qdev-clock.h"
+#include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
 #include "migration/vmstate.h"
 #include "chardev/char-fe.h"
@@ -31,6 +33,21 @@
 #include "qemu/module.h"
 #include "trace.h"
 
+DeviceState *pl011_create(hwaddr addr, qemu_irq irq, Chardev *chr)
+{
+    DeviceState *dev;
+    SysBusDevice *s;
+
+    dev = qdev_new("pl011");
+    s = SYS_BUS_DEVICE(dev);
+    qdev_prop_set_chr(dev, "chardev", chr);
+    sysbus_realize_and_unref(s, &error_fatal);
+    sysbus_mmio_map(s, 0, addr);
+    sysbus_connect_irq(s, 0, irq);
+
+    return dev;
+}
+
 #define PL011_INT_TX 0x20
 #define PL011_INT_RX 0x10
 
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index 926322e242..d82870c006 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -15,10 +15,8 @@
 #ifndef HW_PL011_H
 #define HW_PL011_H
 
-#include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "chardev/char-fe.h"
-#include "qapi/error.h"
 #include "qom/object.h"
 
 #define TYPE_PL011 "pl011"
@@ -57,22 +55,7 @@ struct PL011State {
     const unsigned char *id;
 };
 
-static inline DeviceState *pl011_create(hwaddr addr,
-                                        qemu_irq irq,
-                                        Chardev *chr)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new("pl011");
-    s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, addr);
-    sysbus_connect_irq(s, 0, irq);
-
-    return dev;
-}
+DeviceState *pl011_create(hwaddr addr, qemu_irq irq, Chardev *chr);
 
 static inline DeviceState *pl011_luminary_create(hwaddr addr,
                                                  qemu_irq irq,
-- 
2.38.1

Re: [PATCH 2/8] hw/char/pl011: Un-inline pl011_create()

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> pl011_create() is only used in DeviceRealize handlers,
> not a hot-path. Inlining is not justified.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 2/8] hw/char/pl011: Un-inline pl011_create()

[Richard Henderson]

On 2/20/23 01:51, Philippe Mathieu-Daudé wrote:
> pl011_create() is only used in DeviceRealize handlers,
> not a hot-path. Inlining is not justified.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/char/pl011.c         | 17 +++++++++++++++++
>   include/hw/char/pl011.h | 19 +------------------
>   2 files changed, 18 insertions(+), 18 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 3/8] hw/char/pl011: Open-code pl011_luminary_create()

[Philippe Mathieu-Daudé]

pl011_luminary_create() is only used for the Stellaris board,
open-code it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/stellaris.c      | 11 ++++++++---
 include/hw/char/pl011.h | 17 -----------------
 2 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index 67a2293d35..f7e99baf62 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1146,9 +1146,14 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board)
 
     for (i = 0; i < 4; i++) {
         if (board->dc2 & (1 << i)) {
-            pl011_luminary_create(0x4000c000 + i * 0x1000,
-                                  qdev_get_gpio_in(nvic, uart_irq[i]),
-                                  serial_hd(i));
+            SysBusDevice *sbd;
+
+            dev = qdev_new("pl011_luminary");
+            sbd = SYS_BUS_DEVICE(dev);
+            qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+            sysbus_realize_and_unref(sbd, &error_fatal);
+            sysbus_mmio_map(sbd, 0, 0x4000c000 + i * 0x1000);
+            sysbus_connect_irq(sbd, 0, qdev_get_gpio_in(nvic, uart_irq[i]));
         }
     }
     if (board->dc2 & (1 << 4)) {
diff --git a/include/hw/char/pl011.h b/include/hw/char/pl011.h
index d82870c006..d853802132 100644
--- a/include/hw/char/pl011.h
+++ b/include/hw/char/pl011.h
@@ -57,21 +57,4 @@ struct PL011State {
 
 DeviceState *pl011_create(hwaddr addr, qemu_irq irq, Chardev *chr);
 
-static inline DeviceState *pl011_luminary_create(hwaddr addr,
-                                                 qemu_irq irq,
-                                                 Chardev *chr)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new("pl011_luminary");
-    s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, addr);
-    sysbus_connect_irq(s, 0, irq);
-
-    return dev;
-}
-
 #endif
-- 
2.38.1

Re: [PATCH 3/8] hw/char/pl011: Open-code pl011_luminary_create()

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> pl011_luminary_create() is only used for the Stellaris board,
> open-code it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 3/8] hw/char/pl011: Open-code pl011_luminary_create()

[Richard Henderson]

On 2/20/23 01:51, Philippe Mathieu-Daudé wrote:
> pl011_luminary_create() is only used for the Stellaris board,
> open-code it.
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/arm/stellaris.c      | 11 ++++++++---
>   include/hw/char/pl011.h | 17 -----------------
>   2 files changed, 8 insertions(+), 20 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 4/8] hw/char/xilinx_uartlite: Expose XILINX_UARTLITE QOM type

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/char/xilinx_uartlite.c         | 4 +---
 include/hw/char/xilinx_uartlite.h | 6 +++++-
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index 99b9a6f851..180bb97202 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/log.h"
+#include "hw/char/xilinx_uartlite.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "hw/qdev-properties-system.h"
@@ -53,9 +54,6 @@
 #define CONTROL_RST_RX    0x02
 #define CONTROL_IE        0x10
 
-#define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite"
-OBJECT_DECLARE_SIMPLE_TYPE(XilinxUARTLite, XILINX_UARTLITE)
-
 struct XilinxUARTLite {
     SysBusDevice parent_obj;
 
diff --git a/include/hw/char/xilinx_uartlite.h b/include/hw/char/xilinx_uartlite.h
index dd09c06801..753d3a453e 100644
--- a/include/hw/char/xilinx_uartlite.h
+++ b/include/hw/char/xilinx_uartlite.h
@@ -18,6 +18,10 @@
 #include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "qapi/error.h"
+#include "qom/object.h"
+
+#define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite"
+OBJECT_DECLARE_SIMPLE_TYPE(XilinxUARTLite, XILINX_UARTLITE)
 
 static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
                                         qemu_irq irq,
@@ -26,7 +30,7 @@ static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
     DeviceState *dev;
     SysBusDevice *s;
 
-    dev = qdev_new("xlnx.xps-uartlite");
+    dev = qdev_new(TYPE_XILINX_UARTLITE);
     s = SYS_BUS_DEVICE(dev);
     qdev_prop_set_chr(dev, "chardev", chr);
     sysbus_realize_and_unref(s, &error_fatal);
-- 
2.38.1

Re: [PATCH 4/8] hw/char/xilinx_uartlite: Expose XILINX_UARTLITE QOM type

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 4/8] hw/char/xilinx_uartlite: Expose XILINX_UARTLITE QOM type

[Richard Henderson]

On 2/20/23 01:51, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/char/xilinx_uartlite.c         | 4 +---
>   include/hw/char/xilinx_uartlite.h | 6 +++++-
>   2 files changed, 6 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 5/8] hw/char/xilinx_uartlite: Open-code xilinx_uartlite_create()

[Philippe Mathieu-Daudé]

Open-code the single use of xilinx_uartlite_create().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/microblaze/petalogix_s3adsp1800_mmu.c |  7 +++++--
 include/hw/char/xilinx_uartlite.h        | 20 --------------------
 2 files changed, 5 insertions(+), 22 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 9d959d1ad8..505639c298 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -100,8 +100,11 @@ petalogix_s3adsp1800_init(MachineState *machine)
         irq[i] = qdev_get_gpio_in(dev, i);
     }
 
-    xilinx_uartlite_create(UARTLITE_BASEADDR, irq[UARTLITE_IRQ],
-                           serial_hd(0));
+    dev = qdev_new(TYPE_XILINX_UARTLITE);
+    qdev_prop_set_chr(dev, "chardev", serial_hd(0));
+    sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
+    sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, UARTLITE_BASEADDR);
+    sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[UARTLITE_IRQ]);
 
     /* 2 timers at irq 2 @ 62 Mhz.  */
     dev = qdev_new("xlnx.xps-timer");
diff --git a/include/hw/char/xilinx_uartlite.h b/include/hw/char/xilinx_uartlite.h
index 753d3a453e..36d4e8444d 100644
--- a/include/hw/char/xilinx_uartlite.h
+++ b/include/hw/char/xilinx_uartlite.h
@@ -15,29 +15,9 @@
 #ifndef XILINX_UARTLITE_H
 #define XILINX_UARTLITE_H
 
-#include "hw/qdev-properties.h"
-#include "hw/sysbus.h"
-#include "qapi/error.h"
 #include "qom/object.h"
 
 #define TYPE_XILINX_UARTLITE "xlnx.xps-uartlite"
 OBJECT_DECLARE_SIMPLE_TYPE(XilinxUARTLite, XILINX_UARTLITE)
 
-static inline DeviceState *xilinx_uartlite_create(hwaddr addr,
-                                        qemu_irq irq,
-                                        Chardev *chr)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new(TYPE_XILINX_UARTLITE);
-    s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, addr);
-    sysbus_connect_irq(s, 0, irq);
-
-    return dev;
-}
-
 #endif
-- 
2.38.1

Re: [PATCH 5/8] hw/char/xilinx_uartlite: Open-code xilinx_uartlite_create()

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Open-code the single use of xilinx_uartlite_create().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 5/8] hw/char/xilinx_uartlite: Open-code xilinx_uartlite_create()

[Richard Henderson]

On 2/20/23 01:51, Philippe Mathieu-Daudé wrote:
> Open-code the single use of xilinx_uartlite_create().
> 
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
>   hw/microblaze/petalogix_s3adsp1800_mmu.c |  7 +++++--
>   include/hw/char/xilinx_uartlite.h        | 20 --------------------
>   2 files changed, 5 insertions(+), 22 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH 6/8] hw/char/cmsdk-apb-uart: Open-code cmsdk_apb_uart_create()

[Philippe Mathieu-Daudé]

cmsdk_apb_uart_create() is only used twice in the same
file. Open-code it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/mps2.c                    | 41 +++++++++++++++++++++-----------
 include/hw/char/cmsdk-apb-uart.h | 34 --------------------------
 2 files changed, 27 insertions(+), 48 deletions(-)

diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
index a86a994dba..d92fd60684 100644
--- a/hw/arm/mps2.c
+++ b/hw/arm/mps2.c
@@ -35,6 +35,7 @@
 #include "hw/boards.h"
 #include "exec/address-spaces.h"
 #include "sysemu/sysemu.h"
+#include "hw/qdev-properties.h"
 #include "hw/misc/unimp.h"
 #include "hw/char/cmsdk-apb-uart.h"
 #include "hw/timer/cmsdk-apb-timer.h"
@@ -282,6 +283,9 @@ static void mps2_common_init(MachineState *machine)
         qdev_connect_gpio_out(orgate_dev, 0, qdev_get_gpio_in(armv7m, 12));
 
         for (i = 0; i < 5; i++) {
+            DeviceState *dev;
+            SysBusDevice *s;
+
             static const hwaddr uartbase[] = {0x40004000, 0x40005000,
                                               0x40006000, 0x40007000,
                                               0x40009000};
@@ -294,12 +298,16 @@ static void mps2_common_init(MachineState *machine)
                 rxovrint = qdev_get_gpio_in(orgate_dev, i * 2 + 1);
             }
 
-            cmsdk_apb_uart_create(uartbase[i],
-                                  qdev_get_gpio_in(armv7m, uartirq[i] + 1),
-                                  qdev_get_gpio_in(armv7m, uartirq[i]),
-                                  txovrint, rxovrint,
-                                  NULL,
-                                  serial_hd(i), SYSCLK_FRQ);
+            dev = qdev_new(TYPE_CMSDK_APB_UART);
+            s = SYS_BUS_DEVICE(dev);
+            qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+            qdev_prop_set_uint32(dev, "pclk-frq", SYSCLK_FRQ);
+            sysbus_realize_and_unref(s, &error_fatal);
+            sysbus_mmio_map(s, 0, uartbase[i]);
+            sysbus_connect_irq(s, 0, qdev_get_gpio_in(armv7m, uartirq[i] + 1));
+            sysbus_connect_irq(s, 1, qdev_get_gpio_in(armv7m, uartirq[i]));
+            sysbus_connect_irq(s, 2, txovrint);
+            sysbus_connect_irq(s, 3, rxovrint);
         }
         break;
     }
@@ -324,7 +332,8 @@ static void mps2_common_init(MachineState *machine)
                                               0x4002c000, 0x4002d000,
                                               0x4002e000};
             Object *txrx_orgate;
-            DeviceState *txrx_orgate_dev;
+            DeviceState *txrx_orgate_dev, *dev;
+            SysBusDevice *s;
 
             txrx_orgate = object_new(TYPE_OR_IRQ);
             object_property_set_int(txrx_orgate, "num-lines", 2, &error_fatal);
@@ -332,13 +341,17 @@ static void mps2_common_init(MachineState *machine)
             txrx_orgate_dev = DEVICE(txrx_orgate);
             qdev_connect_gpio_out(txrx_orgate_dev, 0,
                                   qdev_get_gpio_in(armv7m, uart_txrx_irqno[i]));
-            cmsdk_apb_uart_create(uartbase[i],
-                                  qdev_get_gpio_in(txrx_orgate_dev, 0),
-                                  qdev_get_gpio_in(txrx_orgate_dev, 1),
-                                  qdev_get_gpio_in(orgate_dev, i * 2),
-                                  qdev_get_gpio_in(orgate_dev, i * 2 + 1),
-                                  NULL,
-                                  serial_hd(i), SYSCLK_FRQ);
+
+            dev = qdev_new(TYPE_CMSDK_APB_UART);
+            s = SYS_BUS_DEVICE(dev);
+            qdev_prop_set_chr(dev, "chardev", serial_hd(i));
+            qdev_prop_set_uint32(dev, "pclk-frq", SYSCLK_FRQ);
+            sysbus_realize_and_unref(s, &error_fatal);
+            sysbus_mmio_map(s, 0, uartbase[i]);
+            sysbus_connect_irq(s, 0, qdev_get_gpio_in(txrx_orgate_dev, 0));
+            sysbus_connect_irq(s, 1, qdev_get_gpio_in(txrx_orgate_dev, 1));
+            sysbus_connect_irq(s, 2, qdev_get_gpio_in(orgate_dev, i * 2));
+            sysbus_connect_irq(s, 3, qdev_get_gpio_in(orgate_dev, i * 2 + 1));
         }
         break;
     }
diff --git a/include/hw/char/cmsdk-apb-uart.h b/include/hw/char/cmsdk-apb-uart.h
index 64b0a3d534..7de8f8d1b9 100644
--- a/include/hw/char/cmsdk-apb-uart.h
+++ b/include/hw/char/cmsdk-apb-uart.h
@@ -12,10 +12,8 @@
 #ifndef CMSDK_APB_UART_H
 #define CMSDK_APB_UART_H
 
-#include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "chardev/char-fe.h"
-#include "qapi/error.h"
 #include "qom/object.h"
 
 #define TYPE_CMSDK_APB_UART "cmsdk-apb-uart"
@@ -45,36 +43,4 @@ struct CMSDKAPBUART {
     uint8_t rxbuf;
 };
 
-/**
- * cmsdk_apb_uart_create - convenience function to create TYPE_CMSDK_APB_UART
- * @addr: location in system memory to map registers
- * @chr: Chardev backend to connect UART to, or NULL if no backend
- * @pclk_frq: frequency in Hz of the PCLK clock (used for calculating baud rate)
- */
-static inline DeviceState *cmsdk_apb_uart_create(hwaddr addr,
-                                                 qemu_irq txint,
-                                                 qemu_irq rxint,
-                                                 qemu_irq txovrint,
-                                                 qemu_irq rxovrint,
-                                                 qemu_irq uartint,
-                                                 Chardev *chr,
-                                                 uint32_t pclk_frq)
-{
-    DeviceState *dev;
-    SysBusDevice *s;
-
-    dev = qdev_new(TYPE_CMSDK_APB_UART);
-    s = SYS_BUS_DEVICE(dev);
-    qdev_prop_set_chr(dev, "chardev", chr);
-    qdev_prop_set_uint32(dev, "pclk-frq", pclk_frq);
-    sysbus_realize_and_unref(s, &error_fatal);
-    sysbus_mmio_map(s, 0, addr);
-    sysbus_connect_irq(s, 0, txint);
-    sysbus_connect_irq(s, 1, rxint);
-    sysbus_connect_irq(s, 2, txovrint);
-    sysbus_connect_irq(s, 3, rxovrint);
-    sysbus_connect_irq(s, 4, uartint);
-    return dev;
-}
-
 #endif
-- 
2.38.1

Re: [PATCH 6/8] hw/char/cmsdk-apb-uart: Open-code cmsdk_apb_uart_create()

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> cmsdk_apb_uart_create() is only used twice in the same
> file. Open-code it.

Hmm, you could just as easily make cmsdk_apb_uart_create a private
static function and avoid any copy paste snafus if something needs
changing.

>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/arm/mps2.c                    | 41 +++++++++++++++++++++-----------
>  include/hw/char/cmsdk-apb-uart.h | 34 --------------------------
>  2 files changed, 27 insertions(+), 48 deletions(-)
>
> diff --git a/hw/arm/mps2.c b/hw/arm/mps2.c
> index a86a994dba..d92fd60684 100644
> --- a/hw/arm/mps2.c
> +++ b/hw/arm/mps2.c
> @@ -35,6 +35,7 @@
>  #include "hw/boards.h"
>  #include "exec/address-spaces.h"
>  #include "sysemu/sysemu.h"
> +#include "hw/qdev-properties.h"
>  #include "hw/misc/unimp.h"
>  #include "hw/char/cmsdk-apb-uart.h"
>  #include "hw/timer/cmsdk-apb-timer.h"
> @@ -282,6 +283,9 @@ static void mps2_common_init(MachineState *machine)
>          qdev_connect_gpio_out(orgate_dev, 0, qdev_get_gpio_in(armv7m, 12));
>  
>          for (i = 0; i < 5; i++) {
> +            DeviceState *dev;
> +            SysBusDevice *s;
> +
>              static const hwaddr uartbase[] = {0x40004000, 0x40005000,
>                                                0x40006000, 0x40007000,
>                                                0x40009000};
> @@ -294,12 +298,16 @@ static void mps2_common_init(MachineState *machine)
>                  rxovrint = qdev_get_gpio_in(orgate_dev, i * 2 + 1);
>              }
>  
> -            cmsdk_apb_uart_create(uartbase[i],
> -                                  qdev_get_gpio_in(armv7m, uartirq[i] + 1),
> -                                  qdev_get_gpio_in(armv7m, uartirq[i]),
> -                                  txovrint, rxovrint,
> -                                  NULL,
> -                                  serial_hd(i), SYSCLK_FRQ);
> +            dev = qdev_new(TYPE_CMSDK_APB_UART);
> +            s = SYS_BUS_DEVICE(dev);
> +            qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> +            qdev_prop_set_uint32(dev, "pclk-frq", SYSCLK_FRQ);
> +            sysbus_realize_and_unref(s, &error_fatal);
> +            sysbus_mmio_map(s, 0, uartbase[i]);
> +            sysbus_connect_irq(s, 0, qdev_get_gpio_in(armv7m, uartirq[i] + 1));
> +            sysbus_connect_irq(s, 1, qdev_get_gpio_in(armv7m, uartirq[i]));
> +            sysbus_connect_irq(s, 2, txovrint);
> +            sysbus_connect_irq(s, 3, rxovrint);
>          }
>          break;
>      }
> @@ -324,7 +332,8 @@ static void mps2_common_init(MachineState *machine)
>                                                0x4002c000, 0x4002d000,
>                                                0x4002e000};
>              Object *txrx_orgate;
> -            DeviceState *txrx_orgate_dev;
> +            DeviceState *txrx_orgate_dev, *dev;
> +            SysBusDevice *s;
>  
>              txrx_orgate = object_new(TYPE_OR_IRQ);
>              object_property_set_int(txrx_orgate, "num-lines", 2, &error_fatal);
> @@ -332,13 +341,17 @@ static void mps2_common_init(MachineState *machine)
>              txrx_orgate_dev = DEVICE(txrx_orgate);
>              qdev_connect_gpio_out(txrx_orgate_dev, 0,
>                                    qdev_get_gpio_in(armv7m, uart_txrx_irqno[i]));
> -            cmsdk_apb_uart_create(uartbase[i],
> -                                  qdev_get_gpio_in(txrx_orgate_dev, 0),
> -                                  qdev_get_gpio_in(txrx_orgate_dev, 1),
> -                                  qdev_get_gpio_in(orgate_dev, i * 2),
> -                                  qdev_get_gpio_in(orgate_dev, i * 2 + 1),
> -                                  NULL,
> -                                  serial_hd(i), SYSCLK_FRQ);
> +
> +            dev = qdev_new(TYPE_CMSDK_APB_UART);
> +            s = SYS_BUS_DEVICE(dev);
> +            qdev_prop_set_chr(dev, "chardev", serial_hd(i));
> +            qdev_prop_set_uint32(dev, "pclk-frq", SYSCLK_FRQ);
> +            sysbus_realize_and_unref(s, &error_fatal);
> +            sysbus_mmio_map(s, 0, uartbase[i]);
> +            sysbus_connect_irq(s, 0, qdev_get_gpio_in(txrx_orgate_dev, 0));
> +            sysbus_connect_irq(s, 1, qdev_get_gpio_in(txrx_orgate_dev, 1));
> +            sysbus_connect_irq(s, 2, qdev_get_gpio_in(orgate_dev, i * 2));
> +            sysbus_connect_irq(s, 3, qdev_get_gpio_in(orgate_dev, i * 2 + 1));
>          }
>          break;
>      }
> diff --git a/include/hw/char/cmsdk-apb-uart.h b/include/hw/char/cmsdk-apb-uart.h
> index 64b0a3d534..7de8f8d1b9 100644
> --- a/include/hw/char/cmsdk-apb-uart.h
> +++ b/include/hw/char/cmsdk-apb-uart.h
> @@ -12,10 +12,8 @@
>  #ifndef CMSDK_APB_UART_H
>  #define CMSDK_APB_UART_H
>  
> -#include "hw/qdev-properties.h"
>  #include "hw/sysbus.h"
>  #include "chardev/char-fe.h"
> -#include "qapi/error.h"
>  #include "qom/object.h"
>  
>  #define TYPE_CMSDK_APB_UART "cmsdk-apb-uart"
> @@ -45,36 +43,4 @@ struct CMSDKAPBUART {
>      uint8_t rxbuf;
>  };
>  
> -/**
> - * cmsdk_apb_uart_create - convenience function to create TYPE_CMSDK_APB_UART
> - * @addr: location in system memory to map registers
> - * @chr: Chardev backend to connect UART to, or NULL if no backend
> - * @pclk_frq: frequency in Hz of the PCLK clock (used for calculating baud rate)
> - */
> -static inline DeviceState *cmsdk_apb_uart_create(hwaddr addr,
> -                                                 qemu_irq txint,
> -                                                 qemu_irq rxint,
> -                                                 qemu_irq txovrint,
> -                                                 qemu_irq rxovrint,
> -                                                 qemu_irq uartint,
> -                                                 Chardev *chr,
> -                                                 uint32_t pclk_frq)
> -{
> -    DeviceState *dev;
> -    SysBusDevice *s;
> -
> -    dev = qdev_new(TYPE_CMSDK_APB_UART);
> -    s = SYS_BUS_DEVICE(dev);
> -    qdev_prop_set_chr(dev, "chardev", chr);
> -    qdev_prop_set_uint32(dev, "pclk-frq", pclk_frq);
> -    sysbus_realize_and_unref(s, &error_fatal);
> -    sysbus_mmio_map(s, 0, addr);
> -    sysbus_connect_irq(s, 0, txint);
> -    sysbus_connect_irq(s, 1, rxint);
> -    sysbus_connect_irq(s, 2, txovrint);
> -    sysbus_connect_irq(s, 3, rxovrint);
> -    sysbus_connect_irq(s, 4, uartint);
> -    return dev;
> -}
> -
>  #endif


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 6/8] hw/char/cmsdk-apb-uart: Open-code cmsdk_apb_uart_create()

[Peter Maydell]

On Mon, 20 Feb 2023 at 15:34, Alex Bennée <alex.bennee@linaro.org> wrote:
>
>
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
> > cmsdk_apb_uart_create() is only used twice in the same
> > file. Open-code it.
>
> Hmm, you could just as easily make cmsdk_apb_uart_create a private
> static function and avoid any copy paste snafus if something needs
> changing.

I think this is fine, the function isn't really gaining
us much.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM

[PATCH 7/8] hw/timer/cmsdk-apb-timer: Remove unused 'qdev-properties.h' header

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/hw/timer/cmsdk-apb-timer.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/hw/timer/cmsdk-apb-timer.h b/include/hw/timer/cmsdk-apb-timer.h
index c4c7eae849..2dd615d1be 100644
--- a/include/hw/timer/cmsdk-apb-timer.h
+++ b/include/hw/timer/cmsdk-apb-timer.h
@@ -12,7 +12,6 @@
 #ifndef CMSDK_APB_TIMER_H
 #define CMSDK_APB_TIMER_H
 
-#include "hw/qdev-properties.h"
 #include "hw/sysbus.h"
 #include "hw/ptimer.h"
 #include "hw/clock.h"
-- 
2.38.1

Re: [PATCH 7/8] hw/timer/cmsdk-apb-timer: Remove unused 'qdev-properties.h' header

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

[PATCH 8/8] hw/intc/armv7m_nvic: Use QOM cast CPU() macro

[Philippe Mathieu-Daudé]

Avoid accessing 'parent_obj' directly.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/intc/armv7m_nvic.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/intc/armv7m_nvic.c b/hw/intc/armv7m_nvic.c
index 1f7763964c..2fe5b1c4e1 100644
--- a/hw/intc/armv7m_nvic.c
+++ b/hw/intc/armv7m_nvic.c
@@ -584,7 +584,7 @@ static void do_armv7m_nvic_set_pending(void *opaque, int irq, bool secure,
              * which saves having to have an extra argument is_terminal
              * that we'd only use in one place.
              */
-            cpu_abort(&s->cpu->parent_obj,
+            cpu_abort(CPU(s->cpu),
                       "Lockup: can't take terminal derived exception "
                       "(original exception priority %d)\n",
                       s->vectpending_prio);
@@ -650,7 +650,7 @@ static void do_armv7m_nvic_set_pending(void *opaque, int irq, bool secure,
                  * Lockup condition due to a guest bug. We don't model
                  * Lockup, so report via cpu_abort() instead.
                  */
-                cpu_abort(&s->cpu->parent_obj,
+                cpu_abort(CPU(s->cpu),
                           "Lockup: can't escalate %d to HardFault "
                           "(current priority %d)\n", irq, running);
             }
@@ -749,7 +749,7 @@ void armv7m_nvic_set_pending_lazyfp(void *opaque, int irq, bool secure)
              * We want to escalate to HardFault but the context the
              * FP state belongs to prevents the exception pre-empting.
              */
-            cpu_abort(&s->cpu->parent_obj,
+            cpu_abort(CPU(s->cpu),
                       "Lockup: can't escalate to HardFault during "
                       "lazy FP register stacking\n");
         }
-- 
2.38.1

Re: [PATCH 8/8] hw/intc/armv7m_nvic: Use QOM cast CPU() macro

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> Avoid accessing 'parent_obj' directly.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro

Re: [PATCH 0/8] hw/arm: Cleanups around QOM style

[Peter Maydell]

On Mon, 20 Feb 2023 at 11:51, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> - Reduce "hw/qdev-properties.h" inclusions
> - Open-code various QDev helpers used few times
> - Use QOM cast macro when relevant
>



Applied to target-arm.next, thanks.

-- PMM