[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 11/13] hw/arm/xilinx_zynq: Replace drive_get_next() by dri
From: |
Edgar E. Iglesias |
Subject: |
Re: [PATCH v2 11/13] hw/arm/xilinx_zynq: Replace drive_get_next() by drive_get() |
Date: |
Thu, 18 Nov 2021 15:46:37 +0100 |
On Wed, Nov 17, 2021 at 05:34:07PM +0100, Markus Armbruster wrote:
> 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.
Acked-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Alistair Francis <alistair@alistair23.me>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> 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);
> --
> 2.31.1
>
- Re: [PATCH v2 12/13] hw/arm/aspeed: Replace drive_get_next() by drive_get(), (continued)
- [PATCH v2 08/13] hw/arm/xlnx-versal-virt: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17
- [PATCH v2 03/13] hw/arm/npcm7xx_boards: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17
- [PATCH v2 10/13] hw/arm/xlnx-zcu102: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17
- [PATCH v2 11/13] hw/arm/xilinx_zynq: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17
- Re: [PATCH v2 11/13] hw/arm/xilinx_zynq: Replace drive_get_next() by drive_get(),
Edgar E. Iglesias <=
- [PATCH v2 06/13] hw/arm/mcimx6ul-evk: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17
- [PATCH v2 05/13] hw/arm/imx25_pdk: Replace drive_get_next() by drive_get(), Markus Armbruster, 2021/11/17