diff mbox series

[07/12] hw/ssi: Check for duplicate addresses

Message ID 20230508075859.3326566-8-clg@kaod.org (mailing list archive)
State New, archived
Headers show
Series aspeed: fixes and extensions | expand

Commit Message

Cédric Le Goater May 8, 2023, 7:58 a.m. UTC
This to avoid address conflicts on the same SSI bus. Adapt machines
using multiple devices on the same bus to avoid breakage.

Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Alistair Francis <alistair@alistair23.me>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/arm/stellaris.c                  |  4 +++-
 hw/arm/xilinx_zynq.c                |  1 +
 hw/arm/xlnx-versal-virt.c           |  1 +
 hw/arm/xlnx-zcu102.c                |  2 ++
 hw/microblaze/petalogix_ml605_mmu.c |  1 +
 hw/ssi/ssi.c                        | 20 ++++++++++++++++++++
 6 files changed, 28 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé May 30, 2023, 9:05 p.m. UTC | #1
On 8/5/23 09:58, Cédric Le Goater wrote:
> This to avoid address conflicts on the same SSI bus. Adapt machines
> using multiple devices on the same bus to avoid breakage.
> 
> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
> Cc: Alistair Francis <alistair@alistair23.me>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/arm/stellaris.c                  |  4 +++-
>   hw/arm/xilinx_zynq.c                |  1 +
>   hw/arm/xlnx-versal-virt.c           |  1 +
>   hw/arm/xlnx-zcu102.c                |  2 ++
>   hw/microblaze/petalogix_ml605_mmu.c |  1 +
>   hw/ssi/ssi.c                        | 20 ++++++++++++++++++++
>   6 files changed, 28 insertions(+), 1 deletion(-)


> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
> index a25e064417..685b7678e0 100644
> --- a/hw/ssi/ssi.c
> +++ b/hw/ssi/ssi.c
> @@ -42,10 +42,30 @@ DeviceState *ssi_get_cs(SSIBus *bus, int addr)
>       return NULL;
>   }
>   
> +static bool ssi_bus_check_address(BusState *b, DeviceState *dev, Error **errp)
> +{
> +    SSIPeripheral *s = SSI_PERIPHERAL(dev);
> +
> +    if (ssi_get_cs(SSI_BUS(b), s->addr)) {
> +        error_setg(errp, "addr '0x%x' already in use", s->addr);

We could return "... in use by a $MODEL device".

   DeviceState *d = ssi_get_cs(SSI_BUS(b), s->addr);
   if (d) {
       "... in use by a %s device", ..., object_get_typename(OBJECT(d)));
   }

Anyhow,
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +        return false;
> +    }
> +
> +    return true;
> +}
Cédric Le Goater May 31, 2023, 6:20 a.m. UTC | #2
On 5/30/23 23:05, Philippe Mathieu-Daudé wrote:
> On 8/5/23 09:58, Cédric Le Goater wrote:
>> This to avoid address conflicts on the same SSI bus. Adapt machines
>> using multiple devices on the same bus to avoid breakage.
>>
>> Cc: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
>> Cc: Alistair Francis <alistair@alistair23.me>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/arm/stellaris.c                  |  4 +++-
>>   hw/arm/xilinx_zynq.c                |  1 +
>>   hw/arm/xlnx-versal-virt.c           |  1 +
>>   hw/arm/xlnx-zcu102.c                |  2 ++
>>   hw/microblaze/petalogix_ml605_mmu.c |  1 +
>>   hw/ssi/ssi.c                        | 20 ++++++++++++++++++++
>>   6 files changed, 28 insertions(+), 1 deletion(-)
> 
> 
>> diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
>> index a25e064417..685b7678e0 100644
>> --- a/hw/ssi/ssi.c
>> +++ b/hw/ssi/ssi.c
>> @@ -42,10 +42,30 @@ DeviceState *ssi_get_cs(SSIBus *bus, int addr)
>>       return NULL;
>>   }
>> +static bool ssi_bus_check_address(BusState *b, DeviceState *dev, Error **errp)
>> +{
>> +    SSIPeripheral *s = SSI_PERIPHERAL(dev);
>> +
>> +    if (ssi_get_cs(SSI_BUS(b), s->addr)) {
>> +        error_setg(errp, "addr '0x%x' already in use", s->addr);
> 
> We could return "... in use by a $MODEL device".
> 
>    DeviceState *d = ssi_get_cs(SSI_BUS(b), s->addr);
>    if (d) {
>        "... in use by a %s device", ..., object_get_typename(OBJECT(d)));
>    }

Yes. I will change the error message.

Thanks,

C.


> Anyhow,
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> 
>> +        return false;
>> +    }
>> +
>> +    return true;
>> +}
> 
>
diff mbox series

Patch

diff --git a/hw/arm/stellaris.c b/hw/arm/stellaris.c
index f7e99baf62..ffa5999a1d 100644
--- a/hw/arm/stellaris.c
+++ b/hw/arm/stellaris.c
@@ -1242,7 +1242,9 @@  static void stellaris_init(MachineState *ms, stellaris_board_info *board)
                                    qdev_get_child_bus(sddev, "sd-bus"),
                                    &error_fatal);
 
-            ssddev = ssi_create_peripheral(bus, "ssd0323");
+            ssddev = qdev_new("ssd0323");
+            qdev_prop_set_uint32(ssddev, "addr", 1);
+            qdev_realize_and_unref(ssddev, bus, &error_fatal);
 
             gpio_d_splitter = qdev_new(TYPE_SPLIT_IRQ);
             qdev_prop_set_uint32(gpio_d_splitter, "num-lines", 2);
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 3190cc0b8d..91718c5267 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -164,6 +164,7 @@  static inline int zynq_init_spi_flashes(uint32_t base_addr, qemu_irq irq,
                                         blk_by_legacy_dinfo(dinfo),
                                         &error_fatal);
             }
+            qdev_prop_set_uint32(flash_dev, "addr", j);
             qdev_realize_and_unref(flash_dev, BUS(spi), &error_fatal);
 
             cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
diff --git a/hw/arm/xlnx-versal-virt.c b/hw/arm/xlnx-versal-virt.c
index 668a9d65a4..ac2ad3fd0d 100644
--- a/hw/arm/xlnx-versal-virt.c
+++ b/hw/arm/xlnx-versal-virt.c
@@ -701,6 +701,7 @@  static void versal_virt_init(MachineState *machine)
             qdev_prop_set_drive_err(flash_dev, "drive",
                                     blk_by_legacy_dinfo(dinfo), &error_fatal);
         }
+        qdev_prop_set_uint32(flash_dev, "addr", i);
         qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
 
         cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
diff --git a/hw/arm/xlnx-zcu102.c b/hw/arm/xlnx-zcu102.c
index 4c84bb932a..70b4e4b320 100644
--- a/hw/arm/xlnx-zcu102.c
+++ b/hw/arm/xlnx-zcu102.c
@@ -201,6 +201,7 @@  static void xlnx_zcu102_init(MachineState *machine)
             qdev_prop_set_drive_err(flash_dev, "drive",
                                     blk_by_legacy_dinfo(dinfo), &error_fatal);
         }
+        qdev_prop_set_uint32(flash_dev, "addr", i);
         qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
 
         cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
@@ -224,6 +225,7 @@  static void xlnx_zcu102_init(MachineState *machine)
             qdev_prop_set_drive_err(flash_dev, "drive",
                                     blk_by_legacy_dinfo(dinfo), &error_fatal);
         }
+        qdev_prop_set_uint32(flash_dev, "addr", i);
         qdev_realize_and_unref(flash_dev, spi_bus, &error_fatal);
 
         cs_line = qdev_get_gpio_in_named(flash_dev, SSI_GPIO_CS, 0);
diff --git a/hw/microblaze/petalogix_ml605_mmu.c b/hw/microblaze/petalogix_ml605_mmu.c
index a24fadddca..0ef5b3a02b 100644
--- a/hw/microblaze/petalogix_ml605_mmu.c
+++ b/hw/microblaze/petalogix_ml605_mmu.c
@@ -192,6 +192,7 @@  petalogix_ml605_init(MachineState *machine)
                                         blk_by_legacy_dinfo(dinfo),
                                         &error_fatal);
             }
+            qdev_prop_set_uint32(dev, "addr", i);
             qdev_realize_and_unref(dev, BUS(spi), &error_fatal);
 
             cs_line = qdev_get_gpio_in_named(dev, SSI_GPIO_CS, 0);
diff --git a/hw/ssi/ssi.c b/hw/ssi/ssi.c
index a25e064417..685b7678e0 100644
--- a/hw/ssi/ssi.c
+++ b/hw/ssi/ssi.c
@@ -42,10 +42,30 @@  DeviceState *ssi_get_cs(SSIBus *bus, int addr)
     return NULL;
 }
 
+static bool ssi_bus_check_address(BusState *b, DeviceState *dev, Error **errp)
+{
+    SSIPeripheral *s = SSI_PERIPHERAL(dev);
+
+    if (ssi_get_cs(SSI_BUS(b), s->addr)) {
+        error_setg(errp, "addr '0x%x' already in use", s->addr);
+        return false;
+    }
+
+    return true;
+}
+
+static void ssi_bus_class_init(ObjectClass *klass, void *data)
+{
+    BusClass *k = BUS_CLASS(klass);
+
+    k->check_address = ssi_bus_check_address;
+}
+
 static const TypeInfo ssi_bus_info = {
     .name = TYPE_SSI_BUS,
     .parent = TYPE_BUS,
     .instance_size = sizeof(SSIBus),
+    .class_init = ssi_bus_class_init,
 };
 
 static void ssi_cs_default(void *opaque, int n, int level)