Re: [PATCH v2 10/13] hw/arm/xlnx-zcu102: Replace drive_get_next() by drive_get()

["Edgar E. Iglesias" <edgar.iglesias@gmail.com>]

On Wed, Nov 17, 2021 at 05:34:06PM +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
> several counting loops.  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: Alistair Francis <alistair@alistair23.me>
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Cc: qemu-arm@nongnu.org
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  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);
>  
> -- 
> 2.31.1
>