Re: [PATCH v3 2/5] hw/arm/stellaris: replace 'qemu_split_irq' with 'TYPE_SPLIT_IRQ'

[Peter Maydell]

On Wed, 23 Mar 2022 at 17:36, Zongyuan Li <zongyuan.li@smartx.com> wrote:
>
> Signed-off-by: Zongyuan Li <zongyuan.li@smartx.com>
> ---
>  hw/arm/stellaris.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
> index b6c8a5d609..ccc2d5def2 100644
> --- a/hw/arm/stellaris.c
> +++ b/hw/arm/stellaris.c
> @@ -9,6 +9,7 @@
>
>  #include "qemu/osdep.h"
>  #include "qapi/error.h"
> +#include "hw/core/split-irq.h"
>  #include "hw/sysbus.h"
>  #include "hw/sd/sd.h"
>  #include "hw/ssi/ssi.h"
> @@ -1023,6 +1024,7 @@ static void stellaris_init(MachineState *ms, 
> stellaris_board_info *board)
>      I2CBus *i2c;
>      DeviceState *dev;
>      DeviceState *ssys_dev;
> +       DeviceState *gpio_d_splitter;

Indentation seems to be off here. Also since we only use this
in one place you can put the new variable declaration in the
narrower scope that starts "if (board->peripherals & BP_OLED_SSI) {".

>      int i;
>      int j;
>      const uint8_t *macaddr;
> @@ -1237,9 +1239,20 @@ static void stellaris_init(MachineState *ms, 
> stellaris_board_info *board)
>                                     &error_fatal);
>
>              ssddev = ssi_create_peripheral(bus, "ssd0323");
> -            gpio_out[GPIO_D][0] = qemu_irq_split(
> -                    qdev_get_gpio_in_named(sddev, SSI_GPIO_CS, 0),
> +
> +            gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
> +            qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
> +            if (!qdev_realize_and_unref(gpio_d_splitter, NULL, 
> &error_fatal)) {
> +                return;
> +            }

If you're passing &error_fatal to a function, it will never return
on errors, it will exit. So you don't need to have the if() here.

> +            qdev_connect_gpio_out(
> +                    gpio_d_splitter, 0,
> +                    qdev_get_gpio_in_named(sddev, SSI_GPIO_CS, 0));
> +            qdev_connect_gpio_out(
> +                    gpio_d_splitter, 1,
>                      qdev_get_gpio_in_named(ssddev, SSI_GPIO_CS, 0));
> +            gpio_out[GPIO_D][0] = qdev_get_gpio_in(gpio_d_splitter, 0);
> +
>              gpio_out[GPIO_C][7] = qdev_get_gpio_in(ssddev, 0);

Otherwise this looks OK.

thanks
-- PMM