From 36aa285f48d55451bb2eb7a2c0f9635a3c2bbe46 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:33:57 +0100 Subject: [PATCH 01/13] hw/sd/ssi-sd: Do not create SD card within controller's realize MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ssi_sd_realize() creates an "sd-card" device. This is inappropriate, and marked FIXME. Move it to the boards that create these devices. Prior art: commit eb4f566bbb for device "generic-sdhci", and commit 26c607b86b for device "pl181". The device remains not user-creatable, because its users should (and do) wire up its GPIO chip-select line. Cc: Peter Maydell Cc: Alistair Francis Cc: Bin Meng Cc: Palmer Dabbelt Cc: "Philippe Mathieu-Daudé" Cc: qemu-arm@nongnu.org Cc: qemu-riscv@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-2-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Alistair Francis --- hw/arm/stellaris.c | 15 ++++++++++++++- hw/riscv/sifive_u.c | 13 ++++++++++++- hw/sd/ssi-sd.c | 29 +---------------------------- 3 files changed, 27 insertions(+), 30 deletions(-) diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c index 78827ace6b..b6c8a5d609 100644 --- a/hw/arm/stellaris.c +++ b/hw/arm/stellaris.c @@ -10,6 +10,7 @@ #include "qemu/osdep.h" #include "qapi/error.h" #include "hw/sysbus.h" +#include "hw/sd/sd.h" #include "hw/ssi/ssi.h" #include "hw/arm/boot.h" #include "qemu/timer.h" @@ -1157,6 +1158,9 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) void *bus; DeviceState *sddev; DeviceState *ssddev; + DriveInfo *dinfo; + DeviceState *carddev; + BlockBackend *blk; /* * Some boards have both an OLED controller and SD card connected to @@ -1221,8 +1225,17 @@ static void stellaris_init(MachineState *ms, stellaris_board_info *board) * - Make the ssd0323 OLED controller chipselect active-low */ bus = qdev_get_child_bus(dev, "ssi"); - sddev = ssi_create_peripheral(bus, "ssi-sd"); + + dinfo = drive_get(IF_SD, 0, 0); + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; + carddev = qdev_new(TYPE_SD_CARD); + qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal); + qdev_prop_set_bit(carddev, "spi", true); + qdev_realize_and_unref(carddev, + qdev_get_child_bus(sddev, "sd-bus"), + &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), diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index 589ae72a59..a4ecadaf12 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -46,6 +46,7 @@ #include "hw/char/serial.h" #include "hw/cpu/cluster.h" #include "hw/misc/unimp.h" +#include "hw/sd/sd.h" #include "hw/ssi/ssi.h" #include "target/riscv/cpu.h" #include "hw/riscv/riscv_hart.h" @@ -536,7 +537,8 @@ static void sifive_u_machine_init(MachineState *machine) uint32_t fdt_load_addr; uint64_t kernel_entry; DriveInfo *dinfo; - DeviceState *flash_dev, *sd_dev; + BlockBackend *blk; + DeviceState *flash_dev, *sd_dev, *card_dev; qemu_irq flash_cs, sd_cs; /* Initialize SoC */ @@ -686,6 +688,15 @@ static void sifive_u_machine_init(MachineState *machine) sd_cs = qdev_get_gpio_in_named(sd_dev, SSI_GPIO_CS, 0); sysbus_connect_irq(SYS_BUS_DEVICE(&s->soc.spi2), 1, sd_cs); + + dinfo = drive_get(IF_SD, 0, 0); + blk = dinfo ? blk_by_legacy_dinfo(dinfo) : NULL; + card_dev = qdev_new(TYPE_SD_CARD); + qdev_prop_set_drive_err(card_dev, "drive", blk, &error_fatal); + qdev_prop_set_bit(card_dev, "spi", true); + qdev_realize_and_unref(card_dev, + qdev_get_child_bus(sd_dev, "sd-bus"), + &error_fatal); } static bool sifive_u_machine_get_start_in_flash(Object *obj, Error **errp) diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c index e60854eeef..167c03b780 100644 --- a/hw/sd/ssi-sd.c +++ b/hw/sd/ssi-sd.c @@ -368,36 +368,9 @@ static const VMStateDescription vmstate_ssi_sd = { static void ssi_sd_realize(SSIPeripheral *d, Error **errp) { - ERRP_GUARD(); ssi_sd_state *s = SSI_SD(d); - DeviceState *carddev; - DriveInfo *dinfo; qbus_init(&s->sdbus, sizeof(s->sdbus), TYPE_SD_BUS, DEVICE(d), "sd-bus"); - - /* Create and plug in the sd card */ - /* FIXME use a qdev drive property instead of drive_get_next() */ - dinfo = drive_get_next(IF_SD); - carddev = qdev_new(TYPE_SD_CARD); - if (dinfo) { - if (!qdev_prop_set_drive_err(carddev, "drive", - blk_by_legacy_dinfo(dinfo), errp)) { - goto fail; - } - } - - if (!object_property_set_bool(OBJECT(carddev), "spi", true, errp)) { - goto fail; - } - - if (!qdev_realize_and_unref(carddev, BUS(&s->sdbus), errp)) { - goto fail; - } - - return; - -fail: - error_prepend(errp, "failed to init SD card: "); } static void ssi_sd_reset(DeviceState *dev) @@ -426,7 +399,7 @@ static void ssi_sd_class_init(ObjectClass *klass, void *data) k->cs_polarity = SSI_CS_LOW; dc->vmsd = &vmstate_ssi_sd; dc->reset = ssi_sd_reset; - /* Reason: init() method uses drive_get_next() */ + /* Reason: GPIO chip-select line should be wired up */ dc->user_creatable = false; } From 64eaa8209597a580c026d4891c072723a38ff0a6 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:33:58 +0100 Subject: [PATCH 02/13] hw: Replace trivial drive_get_next() by drive_get() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. A number of machines connect just one backend with drive_get_next(). Change them to use drive_get() directly. This makes the (zero) unit number explicit in the code. Cc: Beniamino Galvani Cc: Peter Maydell Cc: Subbaraya Sundeep Cc: Niek Linnenbank Cc: Andrew Baumann Cc: "Philippe Mathieu-Daudé" Cc: Jean-Christophe Dubois Cc: Alistair Francis Cc: Bin Meng Cc: Palmer Dabbelt Cc: Artyom Tarasenko Cc: Mark Cave-Ayland Cc: Michael Tokarev Cc: Laurent Vivier Cc: qemu-arm@nongnu.org Cc: qemu-riscv@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-3-armbru@redhat.com> --- hw/arm/cubieboard.c | 2 +- hw/arm/integratorcp.c | 2 +- hw/arm/msf2-som.c | 2 +- hw/arm/orangepi.c | 2 +- hw/arm/raspi.c | 2 +- hw/arm/realview.c | 2 +- hw/arm/sabrelite.c | 2 +- hw/misc/sifive_u_otp.c | 4 ++-- hw/riscv/microchip_pfsoc.c | 2 +- hw/riscv/sifive_u.c | 2 +- hw/sparc64/niagara.c | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) diff --git a/hw/arm/cubieboard.c b/hw/arm/cubieboard.c index 294ba5de6e..5e3372a3c7 100644 --- a/hw/arm/cubieboard.c +++ b/hw/arm/cubieboard.c @@ -81,7 +81,7 @@ static void cubieboard_init(MachineState *machine) } /* Retrieve SD bus */ - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, 0); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(a10), "sd-bus"); diff --git a/hw/arm/integratorcp.c b/hw/arm/integratorcp.c index 16e8985953..b109ece3ae 100644 --- a/hw/arm/integratorcp.c +++ b/hw/arm/integratorcp.c @@ -649,7 +649,7 @@ static void integratorcp_init(MachineState *machine) qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_WPROT, 0)); qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in_named(icp, ICP_GPIO_MMC_CARDIN, 0)); - dinfo = drive_get_next(IF_SD); + dinfo = drive_get(IF_SD, 0, 0); if (dinfo) { DeviceState *card; diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c index 396e8b9913..d9f881690e 100644 --- a/hw/arm/msf2-som.c +++ b/hw/arm/msf2-som.c @@ -45,7 +45,7 @@ static void emcraft_sf2_s2s010_init(MachineState *machine) DeviceState *spi_flash; MSF2State *soc; MachineClass *mc = MACHINE_GET_CLASS(machine); - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, 0); qemu_irq cs_line; BusState *spi_bus; MemoryRegion *sysmem = get_system_memory(); diff --git a/hw/arm/orangepi.c b/hw/arm/orangepi.c index 0cf9895ce7..e796382236 100644 --- a/hw/arm/orangepi.c +++ b/hw/arm/orangepi.c @@ -85,7 +85,7 @@ static void orangepi_init(MachineState *machine) qdev_realize(DEVICE(h3), NULL, &error_abort); /* Retrieve SD bus */ - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, 0); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(h3), "sd-bus"); diff --git a/hw/arm/raspi.c b/hw/arm/raspi.c index 146d35382b..b4dd6c1e99 100644 --- a/hw/arm/raspi.c +++ b/hw/arm/raspi.c @@ -284,7 +284,7 @@ static void raspi_machine_init(MachineState *machine) qdev_realize(DEVICE(&s->soc), NULL, &error_fatal); /* Create and plug in the SD cards */ - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, 0); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(&s->soc), "sd-bus"); if (bus == NULL) { diff --git a/hw/arm/realview.c b/hw/arm/realview.c index 1c54316ba3..ddc70b54a5 100644 --- a/hw/arm/realview.c +++ b/hw/arm/realview.c @@ -237,7 +237,7 @@ static void realview_init(MachineState *machine, qemu_irq_invert(qdev_get_gpio_in(gpio2, 0))); qdev_connect_gpio_out_named(dev, "card-read-only", 0, mmc_irq[0]); qdev_connect_gpio_out_named(dev, "card-inserted", 0, mmc_irq[1]); - dinfo = drive_get_next(IF_SD); + dinfo = drive_get(IF_SD, 0, 0); if (dinfo) { DeviceState *card; diff --git a/hw/arm/sabrelite.c b/hw/arm/sabrelite.c index 553608e583..cce49aa25c 100644 --- a/hw/arm/sabrelite.c +++ b/hw/arm/sabrelite.c @@ -76,7 +76,7 @@ static void sabrelite_init(MachineState *machine) if (spi_bus) { DeviceState *flash_dev; qemu_irq cs_line; - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, 0); flash_dev = qdev_new("sst25vf016b"); if (dinfo) { diff --git a/hw/misc/sifive_u_otp.c b/hw/misc/sifive_u_otp.c index 52fdb750c0..6d5f84e6c2 100644 --- a/hw/misc/sifive_u_otp.c +++ b/hw/misc/sifive_u_otp.c @@ -209,9 +209,9 @@ static void sifive_u_otp_realize(DeviceState *dev, Error **errp) TYPE_SIFIVE_U_OTP, SIFIVE_U_OTP_REG_SIZE); sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio); - dinfo = drive_get_next(IF_PFLASH); + dinfo = drive_get(IF_PFLASH, 0, 0); if (!dinfo) { - dinfo = drive_get_next(IF_NONE); + dinfo = drive_get(IF_NONE, 0, 0); if (dinfo) { warn_report("using \"-drive if=none\" for the OTP is deprecated, " "use \"-drive if=pflash\" instead."); diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c index 57d779fb55..d1d065efbc 100644 --- a/hw/riscv/microchip_pfsoc.c +++ b/hw/riscv/microchip_pfsoc.c @@ -458,7 +458,7 @@ static void microchip_icicle_kit_machine_init(MachineState *machine) target_ulong firmware_end_addr, kernel_start_addr; uint64_t kernel_entry; uint32_t fdt_load_addr; - DriveInfo *dinfo = drive_get_next(IF_SD); + DriveInfo *dinfo = drive_get(IF_SD, 0, 0); /* Sanity check on RAM size */ if (machine->ram_size < mc->default_ram_size) { diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c index a4ecadaf12..aa74e67889 100644 --- a/hw/riscv/sifive_u.c +++ b/hw/riscv/sifive_u.c @@ -672,7 +672,7 @@ static void sifive_u_machine_init(MachineState *machine) /* Connect an SPI flash to SPI0 */ flash_dev = qdev_new("is25wp256"); - dinfo = drive_get_next(IF_MTD); + dinfo = drive_get(IF_MTD, 0, 0); if (dinfo) { qdev_prop_set_drive_err(flash_dev, "drive", blk_by_legacy_dinfo(dinfo), diff --git a/hw/sparc64/niagara.c b/hw/sparc64/niagara.c index f3e42d0326..ccad2c43a3 100644 --- a/hw/sparc64/niagara.c +++ b/hw/sparc64/niagara.c @@ -98,7 +98,7 @@ static void add_rom_or_fail(const char *file, const hwaddr addr) static void niagara_init(MachineState *machine) { NiagaraBoardState *s = g_new(NiagaraBoardState, 1); - DriveInfo *dinfo = drive_get_next(IF_PFLASH); + DriveInfo *dinfo = drive_get(IF_PFLASH, 0, 0); MemoryRegion *sysmem = get_system_memory(); /* init CPUs */ From 93d8bc8f5bc20a3108c9c9592d43d0202778f0c5 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:33:59 +0100 Subject: [PATCH 03/13] hw/arm/npcm7xx_boards: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "quanta-gbs-bmc" connects just one backend with drive_get_next(), but with a helper function. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Havard Skinnemoen Cc: Tyrone Ting Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-4-armbru@redhat.com> Reviewed-by: Havard Skinnemoen --- hw/arm/npcm7xx_boards.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/arm/npcm7xx_boards.c b/hw/arm/npcm7xx_boards.c index dec7d16ae5..d8a49e4e85 100644 --- a/hw/arm/npcm7xx_boards.c +++ b/hw/arm/npcm7xx_boards.c @@ -84,9 +84,9 @@ static void npcm7xx_connect_dram(NPCM7xxState *soc, MemoryRegion *dram) &error_abort); } -static void sdhci_attach_drive(SDHCIState *sdhci) +static void sdhci_attach_drive(SDHCIState *sdhci, int unit) { - DriveInfo *di = drive_get_next(IF_SD); + DriveInfo *di = drive_get(IF_SD, 0, unit); BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; BusState *bus = qdev_get_child_bus(DEVICE(sdhci), "sd-bus"); @@ -374,7 +374,7 @@ static void quanta_gbs_init(MachineState *machine) drive_get(IF_MTD, 0, 0)); quanta_gbs_i2c_init(soc); - sdhci_attach_drive(&soc->mmc.sdhci); + sdhci_attach_drive(&soc->mmc.sdhci, 0); npcm7xx_load_kernel(machine, soc); } From d83c29e98119570880491dacd8dbed9b45a53c5d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:00 +0100 Subject: [PATCH 04/13] hw/arm/versatilepb hw/arm/vexpress: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. The versatile and vexpress machines ("versatileab", "versatilepb", "vexpress-a9", "vexpress-a15") connect just one or two backends of a type with drive_get_next(). Change them to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-5-armbru@redhat.com> Reviewed-by: Peter Maydell --- hw/arm/versatilepb.c | 4 ++-- hw/arm/vexpress.c | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/hw/arm/versatilepb.c b/hw/arm/versatilepb.c index 575399c4fc..ecc1f6cf74 100644 --- a/hw/arm/versatilepb.c +++ b/hw/arm/versatilepb.c @@ -310,7 +310,7 @@ static void versatile_init(MachineState *machine, int board_id) qdev_connect_gpio_out(sysctl, 0, qdev_get_gpio_in(dev, 0)); dev = sysbus_create_varargs("pl181", 0x10005000, sic[22], sic[1], NULL); - dinfo = drive_get_next(IF_SD); + dinfo = drive_get(IF_SD, 0, 0); if (dinfo) { DeviceState *card; @@ -322,7 +322,7 @@ static void versatile_init(MachineState *machine, int board_id) } dev = sysbus_create_varargs("pl181", 0x1000b000, sic[23], sic[2], NULL); - dinfo = drive_get_next(IF_SD); + dinfo = drive_get(IF_SD, 0, 1); if (dinfo) { DeviceState *card; diff --git a/hw/arm/vexpress.c b/hw/arm/vexpress.c index 58481c0762..966758cf82 100644 --- a/hw/arm/vexpress.c +++ b/hw/arm/vexpress.c @@ -625,7 +625,7 @@ static void vexpress_common_init(MachineState *machine) qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_WPROT)); qdev_connect_gpio_out_named(dev, "card-inserted", 0, qdev_get_gpio_in(sysctl, ARM_SYSCTL_GPIO_MMC_CARDIN)); - dinfo = drive_get_next(IF_SD); + dinfo = drive_get(IF_SD, 0, 0); if (dinfo) { DeviceState *card; @@ -657,7 +657,7 @@ static void vexpress_common_init(MachineState *machine) sysbus_create_simple("pl111", map[VE_CLCD], pic[14]); - dinfo = drive_get_next(IF_PFLASH); + dinfo = drive_get(IF_PFLASH, 0, 0); pflash0 = ve_pflash_cfi01_register(map[VE_NORFLASH0], "vexpress.flash0", dinfo); if (!pflash0) { @@ -673,7 +673,7 @@ static void vexpress_common_init(MachineState *machine) memory_region_add_subregion(sysmem, map[VE_NORFLASHALIAS], flashalias); } - dinfo = drive_get_next(IF_PFLASH); + dinfo = drive_get(IF_PFLASH, 0, 1); if (!ve_pflash_cfi01_register(map[VE_NORFLASH1], "vexpress.flash1", dinfo)) { error_report("vexpress: error registering flash 1"); From 6b87668b0813b9cb5ec714dcbcd8f128cfc4a3b1 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:01 +0100 Subject: [PATCH 05/13] hw/arm/imx25_pdk: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "imx25-pdk" connects backends with drive_get_next() in a counting loop. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Peter Maydell Cc: Jean-Christophe Dubois Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-6-armbru@redhat.com> Reviewed-by: Peter Maydell --- hw/arm/imx25_pdk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/imx25_pdk.c b/hw/arm/imx25_pdk.c index bd16acd4d9..6dff000163 100644 --- a/hw/arm/imx25_pdk.c +++ b/hw/arm/imx25_pdk.c @@ -123,7 +123,7 @@ static void imx25_pdk_init(MachineState *machine) DriveInfo *di; BlockBackend *blk; - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, i); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(&s->soc.esdhc[i]), "sd-bus"); carddev = qdev_new(TYPE_SD_CARD); From 8acf052f7a70df88c01feccd8992eb50afce1b1d Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:02 +0100 Subject: [PATCH 06/13] hw/arm/mcimx6ul-evk: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "mcimx6ul-evk" connects backends with drive_get_next() in a counting loop. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Peter Maydell Cc: Jean-Christophe Dubois Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-7-armbru@redhat.com> Reviewed-by: Peter Maydell --- hw/arm/mcimx6ul-evk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/mcimx6ul-evk.c b/hw/arm/mcimx6ul-evk.c index 77fae874b1..28b4886f48 100644 --- a/hw/arm/mcimx6ul-evk.c +++ b/hw/arm/mcimx6ul-evk.c @@ -52,7 +52,7 @@ static void mcimx6ul_evk_init(MachineState *machine) DriveInfo *di; BlockBackend *blk; - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, i); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus"); carddev = qdev_new(TYPE_SD_CARD); From 50659fc479977a7576c0cf0589c7fa31dabe0ad9 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:03 +0100 Subject: [PATCH 07/13] hw/arm/mcimx7d-sabre: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "mcimx7d-sabre" connects backends with drive_get_next() in a counting loop. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Peter Maydell Cc: Andrey Smirnov Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-8-armbru@redhat.com> Reviewed-by: Peter Maydell --- hw/arm/mcimx7d-sabre.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/arm/mcimx7d-sabre.c b/hw/arm/mcimx7d-sabre.c index 935d4b0f1c..50a5ecde31 100644 --- a/hw/arm/mcimx7d-sabre.c +++ b/hw/arm/mcimx7d-sabre.c @@ -52,7 +52,7 @@ static void mcimx7d_sabre_init(MachineState *machine) DriveInfo *di; BlockBackend *blk; - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, i); blk = di ? blk_by_legacy_dinfo(di) : NULL; bus = qdev_get_child_bus(DEVICE(&s->usdhc[i]), "sd-bus"); carddev = qdev_new(TYPE_SD_CARD); From c29faeda3f7e770e463a58f7b5b34b409dba6c88 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:04 +0100 Subject: [PATCH 08/13] hw/arm/xlnx-versal-virt: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "xlnx-versal-virt" connects backends with drive_get_next() in a counting loop. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Alistair Francis Cc: "Edgar E. Iglesias" Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-9-armbru@redhat.com> Acked-by: Edgar E. Iglesias --- hw/arm/xlnx-versal-virt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c index d2f55e29b6..0c5edc898e 100644 --- a/hw/arm/xlnx-versal-virt.c +++ b/hw/arm/xlnx-versal-virt.c @@ -669,7 +669,8 @@ static void versal_virt_init(MachineState *machine) /* Plugin SD cards. */ for (i = 0; i < ARRAY_SIZE(s->soc.pmc.iou.sd); i++) { - sd_plugin_card(&s->soc.pmc.iou.sd[i], drive_get_next(IF_SD)); + sd_plugin_card(&s->soc.pmc.iou.sd[i], + drive_get(IF_SD, 0, i)); } s->binfo.ram_size = machine->ram_size; From 0de89a863292836e6efb0c40c6f9221c89a2033a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:05 +0100 Subject: [PATCH 09/13] hw/microblaze: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "petalogix-ml605" connects backends with drive_get_next() in a counting loop. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: "Edgar E. Iglesias" Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-10-armbru@redhat.com> Acked-by: Edgar E. Iglesias --- hw/microblaze/petalogix_ml605_mmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c index 159db6cbe2..a24fadddca 100644 --- a/hw/microblaze/petalogix_ml605_mmu.c +++ b/hw/microblaze/petalogix_ml605_mmu.c @@ -183,7 +183,7 @@ petalogix_ml605_init(MachineState *machine) spi = (SSIBus *)qdev_get_child_bus(dev, "spi"); for (i = 0; i < NUM_SPI_FLASHES; i++) { - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, i); qemu_irq cs_line; dev = qdev_new("n25q128"); From 97ca6c2786e5dd1fd689c52ffe0cb082bc8bd477 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:06 +0100 Subject: [PATCH 10/13] hw/arm/xlnx-zcu102: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "xlnx-zcu102" connects backends with drive_get_next() in several counting loops. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: Alistair Francis Cc: "Edgar E. Iglesias" Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-11-armbru@redhat.com> Acked-by: Edgar E. Iglesias --- hw/arm/xlnx-zcu102.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c index 3dc2b5e8ca..45eb19ab3b 100644 --- a/hw/arm/xlnx-zcu102.c +++ b/hw/arm/xlnx-zcu102.c @@ -169,7 +169,7 @@ static void xlnx_zcu102_init(MachineState *machine) /* Create and plug in the SD cards */ for (i = 0; i < XLNX_ZYNQMP_NUM_SDHCI; i++) { BusState *bus; - DriveInfo *di = drive_get_next(IF_SD); + DriveInfo *di = drive_get(IF_SD, 0, i); BlockBackend *blk = di ? blk_by_legacy_dinfo(di) : NULL; DeviceState *carddev; char *bus_name; @@ -190,7 +190,7 @@ static void xlnx_zcu102_init(MachineState *machine) BusState *spi_bus; DeviceState *flash_dev; qemu_irq cs_line; - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, i); gchar *bus_name = g_strdup_printf("spi%d", i); spi_bus = qdev_get_child_bus(DEVICE(&s->soc), bus_name); @@ -212,7 +212,7 @@ static void xlnx_zcu102_init(MachineState *machine) BusState *spi_bus; DeviceState *flash_dev; qemu_irq cs_line; - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, XLNX_ZYNQMP_NUM_SPIS + i); int bus = i / XLNX_ZYNQMP_NUM_QSPI_BUS_CS; gchar *bus_name = g_strdup_printf("qspi%d", bus); From 94d4bb4ff9c557dcb68cd16a287700dd9218bd1c Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:07 +0100 Subject: [PATCH 11/13] hw/arm/xilinx_zynq: Replace drive_get_next() by drive_get() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. Machine "xlnx-zcu102" connects backends with drive_get_next() in two counting loops, one of them in a helper function. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: "Edgar E. Iglesias" Cc: Alistair Francis Cc: Peter Maydell Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-12-armbru@redhat.com> Acked-by: Edgar E. Iglesias --- hw/arm/xilinx_zynq.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c index 69c333e91b..50e7268396 100644 --- a/hw/arm/xilinx_zynq.c +++ b/hw/arm/xilinx_zynq.c @@ -125,9 +125,10 @@ static void gem_init(NICInfo *nd, uint32_t base, qemu_irq irq) sysbus_connect_irq(s, 0, irq); } -static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, - bool is_qspi) +static inline int zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, + bool is_qspi, int unit0) { + int unit = unit0; DeviceState *dev; SysBusDevice *busdev; SSIBus *spi; @@ -156,7 +157,7 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, spi = (SSIBus *)qdev_get_child_bus(dev, bus_name); for (j = 0; j < num_ss; ++j) { - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, unit++); flash_dev = qdev_new("n25q128"); if (dinfo) { qdev_prop_set_drive_err(flash_dev, "drive", @@ -170,6 +171,7 @@ static inline void zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq, } } + return unit; } static void zynq_init(MachineState *machine) @@ -247,9 +249,9 @@ static void zynq_init(MachineState *machine) pic[n] = qdev_get_gpio_in(dev, n); } - zynq_init_spi_flashes(0xE0006000, pic[58-IRQ_OFFSET], false); - zynq_init_spi_flashes(0xE0007000, pic[81-IRQ_OFFSET], false); - zynq_init_spi_flashes(0xE000D000, pic[51-IRQ_OFFSET], true); + n = zynq_init_spi_flashes(0xE0006000, pic[58 - IRQ_OFFSET], false, 0); + n = zynq_init_spi_flashes(0xE0007000, pic[81 - IRQ_OFFSET], false, n); + n = zynq_init_spi_flashes(0xE000D000, pic[51 - IRQ_OFFSET], true, n); sysbus_create_simple(TYPE_CHIPIDEA, 0xE0002000, pic[53 - IRQ_OFFSET]); sysbus_create_simple(TYPE_CHIPIDEA, 0xE0003000, pic[76 - IRQ_OFFSET]); @@ -298,7 +300,7 @@ static void zynq_init(MachineState *machine) sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, hci_addr); sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, pic[hci_irq - IRQ_OFFSET]); - di = drive_get_next(IF_SD); + di = drive_get(IF_SD, 0, n); blk = di ? blk_by_legacy_dinfo(di) : NULL; carddev = qdev_new(TYPE_SD_CARD); qdev_prop_set_drive_err(carddev, "drive", blk, &error_fatal); From 8ec239f2d80bed82ad5a361ddfb9ca3560a45ad8 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:08 +0100 Subject: [PATCH 12/13] hw/arm/aspeed: Replace drive_get_next() by drive_get() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. The aspeed machines connects backends with drive_get_next() in several counting loops, one of them in a helper function, and a conditional. Change it to use drive_get() directly. This makes the unit numbers explicit in the code. Cc: "Cédric Le Goater" Cc: Peter Maydell Cc: Andrew Jeffery Cc: Joel Stanley Cc: qemu-arm@nongnu.org Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-13-armbru@redhat.com> Reviewed-by: Cédric Le Goater --- hw/arm/aspeed.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c index a77f46b3ad..cf20ae0db5 100644 --- a/hw/arm/aspeed.c +++ b/hw/arm/aspeed.c @@ -284,12 +284,13 @@ static void write_boot_rom(DriveInfo *dinfo, hwaddr addr, size_t rom_size, } static void aspeed_board_init_flashes(AspeedSMCState *s, - const char *flashtype) + const char *flashtype, + int unit0) { int i ; for (i = 0; i < s->num_cs; ++i) { - DriveInfo *dinfo = drive_get_next(IF_MTD); + DriveInfo *dinfo = drive_get(IF_MTD, 0, unit0 + i); qemu_irq cs_line; DeviceState *dev; @@ -382,10 +383,12 @@ static void aspeed_machine_init(MachineState *machine) "max_ram", max_ram_size - machine->ram_size); memory_region_add_subregion(&bmc->ram_container, machine->ram_size, &bmc->max_ram); - aspeed_board_init_flashes(&bmc->soc.fmc, bmc->fmc_model ? - bmc->fmc_model : amc->fmc_model); - aspeed_board_init_flashes(&bmc->soc.spi[0], bmc->spi_model ? - bmc->spi_model : amc->spi_model); + aspeed_board_init_flashes(&bmc->soc.fmc, + bmc->fmc_model ? bmc->fmc_model : amc->fmc_model, + 0); + aspeed_board_init_flashes(&bmc->soc.spi[0], + bmc->spi_model ? bmc->spi_model : amc->spi_model, + bmc->soc.fmc.num_cs); /* Install first FMC flash content as a boot rom. */ if (drive0) { @@ -435,11 +438,13 @@ static void aspeed_machine_init(MachineState *machine) } for (i = 0; i < bmc->soc.sdhci.num_slots; i++) { - sdhci_attach_drive(&bmc->soc.sdhci.slots[i], drive_get_next(IF_SD)); + sdhci_attach_drive(&bmc->soc.sdhci.slots[i], + drive_get(IF_SD, 0, i)); } if (bmc->soc.emmc.num_slots) { - sdhci_attach_drive(&bmc->soc.emmc.slots[0], drive_get_next(IF_SD)); + sdhci_attach_drive(&bmc->soc.emmc.slots[0], + drive_get(IF_SD, 0, bmc->soc.sdhci.num_slots)); } arm_load_kernel(ARM_CPU(first_cpu), machine, &aspeed_board_binfo); From 95fd260f0a3663ed229b8e0d2ef111a9f8bd93ad Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Wed, 17 Nov 2021 17:34:09 +0100 Subject: [PATCH 13/13] blockdev: Drop unused drive_get_next() drive_get_next() is basically a bad idea. It returns the "next" block backend of a certain interface type. "Next" means bus=0,unit=N, where subsequent calls count N up from zero, per interface type. This lets you define unit numbers implicitly by execution order. If the order changes, or new calls appear "in the middle", unit numbers change. ABI break. Hard to spot in review. The previous commits eliminated all uses. Drop the function. Cc: Kevin Wolf Cc: Hanna Reitz Signed-off-by: Markus Armbruster Message-Id: <20211117163409.3587705-14-armbru@redhat.com> Reviewed-by: Hanna Reitz --- blockdev.c | 10 ---------- include/sysemu/blockdev.h | 1 - 2 files changed, 11 deletions(-) diff --git a/blockdev.c b/blockdev.c index b35072644e..0eb2823b1b 100644 --- a/blockdev.c +++ b/blockdev.c @@ -303,16 +303,6 @@ int drive_get_max_bus(BlockInterfaceType type) return max_bus; } -/* Get a block device. This should only be used for single-drive devices - (e.g. SD/Floppy/MTD). Multi-disk devices (scsi/ide) should use the - appropriate bus. */ -DriveInfo *drive_get_next(BlockInterfaceType type) -{ - static int next_block_unit[IF_COUNT]; - - return drive_get(type, 0, next_block_unit[type]++); -} - static void bdrv_format_print(void *opaque, const char *name) { qemu_printf(" %s", name); diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h index 32c2d6023c..a750f99b79 100644 --- a/include/sysemu/blockdev.h +++ b/include/sysemu/blockdev.h @@ -50,7 +50,6 @@ void drive_check_orphaned(void); DriveInfo *drive_get_by_index(BlockInterfaceType type, int index); int drive_get_max_bus(BlockInterfaceType type); int drive_get_max_devs(BlockInterfaceType type); -DriveInfo *drive_get_next(BlockInterfaceType type); QemuOpts *drive_def(const char *optstr); QemuOpts *drive_add(BlockInterfaceType type, int index, const char *file,