| Message ID | 20241005100228.28094-3-shentey@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Reuse TYPE_GPIO_PWR in ppce500 machine | expand |
On 5/10/24 07:02, Bernhard Beschow wrote: > Taking inspiration from the ARM virt machine, port away from > qemu_allocate_irq() by reusing TYPE_GPIO_PWR. > > Signed-off-by: Bernhard Beschow <shentey@gmail.com> > --- > hw/ppc/e500.c | 16 ++++------------ > hw/ppc/Kconfig | 1 + > 2 files changed, 5 insertions(+), 12 deletions(-) > @@ -892,13 +890,6 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms, > return dev; > } > > -static void ppce500_power_off(void *opaque, int line, int on) > -{ > - if (on) { > - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); > - } > -} > - > void ppce500_init(MachineState *machine) > { > MemoryRegion *address_space_mem = get_system_memory(); > @@ -1086,7 +1077,7 @@ void ppce500_init(MachineState *machine) > sysbus_create_simple("e500-spin", pmc->spin_base, NULL); > > if (pmc->has_mpc8xxx_gpio) { > - qemu_irq poweroff_irq; > + DeviceState *gpio_pwr_dev; Can we keep a reference in PPCE500MachineState? Otherwise, Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> > > dev = qdev_new("mpc8xxx_gpio"); > s = SYS_BUS_DEVICE(dev); > @@ -1096,8 +1087,9 @@ void ppce500_init(MachineState *machine) > sysbus_mmio_get_region(s, 0)); > > /* Power Off GPIO at Pin 0 */ > - poweroff_irq = qemu_allocate_irq(ppce500_power_off, NULL, 0); > - qdev_connect_gpio_out(dev, 0, poweroff_irq); > + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL); > + qdev_connect_gpio_out(dev, 0, qdev_get_gpio_in_named(gpio_pwr_dev, > + "shutdown", 0)); > }
Am 7. Oktober 2024 21:13:22 UTC schrieb "Philippe Mathieu-Daudé" <philmd@linaro.org>: >On 5/10/24 07:02, Bernhard Beschow wrote: >> Taking inspiration from the ARM virt machine, port away from >> qemu_allocate_irq() by reusing TYPE_GPIO_PWR. >> >> Signed-off-by: Bernhard Beschow <shentey@gmail.com> >> --- >> hw/ppc/e500.c | 16 ++++------------ >> hw/ppc/Kconfig | 1 + >> 2 files changed, 5 insertions(+), 12 deletions(-) > > >> @@ -892,13 +890,6 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms, >> return dev; >> } >> -static void ppce500_power_off(void *opaque, int line, int on) >> -{ >> - if (on) { >> - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); >> - } >> -} >> - >> void ppce500_init(MachineState *machine) >> { >> MemoryRegion *address_space_mem = get_system_memory(); >> @@ -1086,7 +1077,7 @@ void ppce500_init(MachineState *machine) >> sysbus_create_simple("e500-spin", pmc->spin_base, NULL); >> if (pmc->has_mpc8xxx_gpio) { >> - qemu_irq poweroff_irq; >> + DeviceState *gpio_pwr_dev; > >Can we keep a reference in PPCE500MachineState? I considered turning it into an embedded attribute, but decided against it, because 1/ the device isn't part of the SoC (and therefore ideally user-createable), 2/ only used by the ppce500 machine, 3/ would be inconsistent with surrounding code, and 4/ "gpio-pwr" would require exposing the struct first (ARM virt also has no embedded attribute). That all seemed like a lot of churn for what I want to achieve which is having the same solution for the same problem across two machines. There is surely a lot of room for cleaning up e500 beyond my e500 cleanup series, but unless there is demand for it, I'd stop there (patches welcome). Now that this series doesn't touch ARM much it'd actually make sense to merge it there. > >Otherwise, >Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> So, is creating an anonymous device okay for you? Does your R-b stand nevertheless? Best regards, Bernhard > >> dev = qdev_new("mpc8xxx_gpio"); >> s = SYS_BUS_DEVICE(dev); >> @@ -1096,8 +1087,9 @@ void ppce500_init(MachineState *machine) >> sysbus_mmio_get_region(s, 0)); >> /* Power Off GPIO at Pin 0 */ >> - poweroff_irq = qemu_allocate_irq(ppce500_power_off, NULL, 0); >> - qdev_connect_gpio_out(dev, 0, poweroff_irq); >> + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL); >> + qdev_connect_gpio_out(dev, 0, qdev_get_gpio_in_named(gpio_pwr_dev, >> + "shutdown", 0)); >> }
diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c index 3bd12b54ab..7811c22e7b 100644 --- a/hw/ppc/e500.c +++ b/hw/ppc/e500.c @@ -30,7 +30,6 @@ #include "sysemu/sysemu.h" #include "sysemu/kvm.h" #include "sysemu/reset.h" -#include "sysemu/runstate.h" #include "kvm_ppc.h" #include "sysemu/device_tree.h" #include "hw/ppc/openpic.h" @@ -47,7 +46,6 @@ #include "hw/platform-bus.h" #include "hw/net/fsl_etsec/etsec.h" #include "hw/i2c/i2c.h" -#include "hw/irq.h" #include "hw/sd/sdhci.h" #include "hw/misc/unimp.h" @@ -892,13 +890,6 @@ static DeviceState *ppce500_init_mpic(PPCE500MachineState *pms, return dev; } -static void ppce500_power_off(void *opaque, int line, int on) -{ - if (on) { - qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); - } -} - void ppce500_init(MachineState *machine) { MemoryRegion *address_space_mem = get_system_memory(); @@ -1086,7 +1077,7 @@ void ppce500_init(MachineState *machine) sysbus_create_simple("e500-spin", pmc->spin_base, NULL); if (pmc->has_mpc8xxx_gpio) { - qemu_irq poweroff_irq; + DeviceState *gpio_pwr_dev; dev = qdev_new("mpc8xxx_gpio"); s = SYS_BUS_DEVICE(dev); @@ -1096,8 +1087,9 @@ void ppce500_init(MachineState *machine) sysbus_mmio_get_region(s, 0)); /* Power Off GPIO at Pin 0 */ - poweroff_irq = qemu_allocate_irq(ppce500_power_off, NULL, 0); - qdev_connect_gpio_out(dev, 0, poweroff_irq); + gpio_pwr_dev = sysbus_create_simple("gpio-pwr", -1, NULL); + qdev_connect_gpio_out(dev, 0, qdev_get_gpio_in_named(gpio_pwr_dev, + "shutdown", 0)); } /* Platform Bus Device */ diff --git a/hw/ppc/Kconfig b/hw/ppc/Kconfig index 5addad1124..89cabe5d53 100644 --- a/hw/ppc/Kconfig +++ b/hw/ppc/Kconfig @@ -158,6 +158,7 @@ config E500 imply VIRTIO_PCI select ETSEC select GPIO_MPC8XXX + select GPIO_PWR select OPENPIC select PFLASH_CFI01 select PLATFORM_BUS
Taking inspiration from the ARM virt machine, port away from qemu_allocate_irq() by reusing TYPE_GPIO_PWR. Signed-off-by: Bernhard Beschow <shentey@gmail.com> --- hw/ppc/e500.c | 16 ++++------------ hw/ppc/Kconfig | 1 + 2 files changed, 5 insertions(+), 12 deletions(-)