Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
Add the "little-endian" property to select the device
endianness, defaulting to little endian.
Set the proper endianness on the single machine using the
device.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/arm/xlnx-zynqmp.c |  4 ++++
 hw/ssi/xilinx_spi.c  | 29 +++++++++++++++++++++--------
 2 files changed, 25 insertions(+), 8 deletions(-)

diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index ab2d50e31b..e735dbdf82 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -714,6 +714,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
     for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
         gchar *bus_name;
 
+        if (!object_property_set_bool(OBJECT(&s->spi[i])), "little-endian",
+                                      true, errp)) {
+            return;
+        }
         if (!sysbus_realize(SYS_BUS_DEVICE(&s->spi[i]), errp)) {
             return;
         }
diff --git a/hw/ssi/xilinx_spi.c b/hw/ssi/xilinx_spi.c
index 7f1e1808c5..2a0c9bca05 100644
--- a/hw/ssi/xilinx_spi.c
+++ b/hw/ssi/xilinx_spi.c
@@ -83,6 +83,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(XilinxSPI, XILINX_SPI)
 struct XilinxSPI {
     SysBusDevice parent_obj;
 
+    bool little_endian_model;
     MemoryRegion mmio;
 
     qemu_irq irq;
@@ -313,13 +314,23 @@ done:
     xlx_spi_update_irq(s);
 }
 
-static const MemoryRegionOps spi_ops = {
-    .read = spi_read,
-    .write = spi_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4
+static const MemoryRegionOps spi_ops[2] = {
+    {
+        .read = spi_read,
+        .write = spi_write,
+        .endianness = DEVICE_BIG_ENDIAN,
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+    }, {
+        .read = spi_read,
+        .write = spi_write,
+        .endianness = DEVICE_LITTLE_ENDIAN,
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
     }
 };
 
@@ -339,7 +350,8 @@ static void xilinx_spi_realize(DeviceState *dev, Error **errp)
         sysbus_init_irq(sbd, &s->cs_lines[i]);
     }
 
-    memory_region_init_io(&s->mmio, OBJECT(s), &spi_ops, s,
+    memory_region_init_io(&s->mmio, OBJECT(s),
+                          &spi_ops[s->little_endian_model], s,
                           "xilinx-spi", R_MAX * 4);
     sysbus_init_mmio(sbd, &s->mmio);
 
@@ -362,6 +374,7 @@ static const VMStateDescription vmstate_xilinx_spi = {
 };
 
 static Property xilinx_spi_properties[] = {
+    DEFINE_PROP_BOOL("little-endian", XilinxSPI, little_endian_model, true),
     DEFINE_PROP_UINT8("num-ss-bits", XilinxSPI, num_cs, 1),
     DEFINE_PROP_END_OF_LIST(),
 };
-- 
2.45.2

On 11/7/24 01:22, Philippe Mathieu-Daudé wrote:
> Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
> of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
> Add the "little-endian" property to select the device
> endianness, defaulting to little endian.
> Set the proper endianness on the single machine using the
> device.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/arm/xlnx-zynqmp.c |  4 ++++
>   hw/ssi/xilinx_spi.c  | 29 +++++++++++++++++++++--------
>   2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index ab2d50e31b..e735dbdf82 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -714,6 +714,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
>       for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
>           gchar *bus_name;
>   
> +        if (!object_property_set_bool(OBJECT(&s->spi[i])), "little-endian",
> +                                      true, errp)) {
> +            return;
> +        }

How might this ever fail?
Despite xlnx_zynqmp_realize having an errp argument, I'd be inclined to use error_abort.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

On 7/11/24 11:01, Richard Henderson wrote:
> On 11/7/24 01:22, Philippe Mathieu-Daudé wrote:
>> Replace the DEVICE_NATIVE_ENDIAN MemoryRegionOps by a pair
>> of DEVICE_LITTLE_ENDIAN / DEVICE_BIG_ENDIAN.
>> Add the "little-endian" property to select the device
>> endianness, defaulting to little endian.
>> Set the proper endianness on the single machine using the
>> device.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/arm/xlnx-zynqmp.c |  4 ++++
>>   hw/ssi/xilinx_spi.c  | 29 +++++++++++++++++++++--------
>>   2 files changed, 25 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
>> index ab2d50e31b..e735dbdf82 100644
>> --- a/hw/arm/xlnx-zynqmp.c
>> +++ b/hw/arm/xlnx-zynqmp.c
>> @@ -714,6 +714,10 @@ static void xlnx_zynqmp_realize(DeviceState *dev, 
>> Error **errp)
>>       for (i = 0; i < XLNX_ZYNQMP_NUM_SPIS; i++) {
>>           gchar *bus_name;
>> +        if (!object_property_set_bool(OBJECT(&s->spi[i])), 
>> "little-endian",
>> +                                      true, errp)) {
>> +            return;
>> +        }
> 
> How might this ever fail?
> Despite xlnx_zynqmp_realize having an errp argument, I'd be inclined to 
> use error_abort.

Yeah I also noticed that, but this is the pattern this file uses, so
I'd rather clean it up in a separate patch.

> Otherwise,
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Thanks!