diff mbox series

[v2,10/10] hw/mips/jazz: specify correct endian for dp8393x device

Message ID 20210625065401.30170-11-mark.cave-ayland@ilande.co.uk (mailing list archive)
State New, archived
Headers show
Series dp8393x: fixes for MacOS toolbox ROM | expand

Commit Message

Mark Cave-Ayland June 25, 2021, 6:54 a.m. UTC
The MIPS magnum machines are available in both big endian (mips64) and little
endian (mips64el) configurations. Ensure that the dp893x big_endian property
is set accordingly using logic similar to that used for the MIPS malta
machines.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/mips/jazz.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé June 25, 2021, 8:51 a.m. UTC | #1
On 6/25/21 8:54 AM, Mark Cave-Ayland wrote:
> The MIPS magnum machines are available in both big endian (mips64) and little
> endian (mips64el) configurations. Ensure that the dp893x big_endian property
> is set accordingly using logic similar to that used for the MIPS malta
> machines.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/mips/jazz.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
> index 89ca8bb910..ee1789183e 100644
> --- a/hw/mips/jazz.c
> +++ b/hw/mips/jazz.c
> @@ -126,7 +126,7 @@ static void mips_jazz_init(MachineState *machine,
>  {
>      MemoryRegion *address_space = get_system_memory();
>      char *filename;
> -    int bios_size, n;
> +    int bios_size, n, big_endian;

Why not use a boolean directly?

>      Clock *cpuclk;
>      MIPSCPU *cpu;
>      MIPSCPUClass *mcc;
> @@ -158,6 +158,12 @@ static void mips_jazz_init(MachineState *machine,
>          [JAZZ_PICA61] = {33333333, 4},
>      };
>  
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    big_endian = 1;
> +#else
> +    big_endian = 0;
> +#endif
> +
>      if (machine->ram_size > 256 * MiB) {
>          error_report("RAM size more than 256Mb is not supported");
>          exit(EXIT_FAILURE);
> @@ -290,6 +296,7 @@ static void mips_jazz_init(MachineState *machine,
>              dev = qdev_new("dp8393x");
>              qdev_set_nic_properties(dev, nd);
>              qdev_prop_set_uint8(dev, "it_shift", 2);
> +            qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
>              object_property_set_link(OBJECT(dev), "dma_mr",
>                                       OBJECT(rc4030_dma_mr), &error_abort);
>              sysbus = SYS_BUS_DEVICE(dev);
>
Mark Cave-Ayland June 25, 2021, 12:01 p.m. UTC | #2
On 25/06/2021 09:51, Philippe Mathieu-Daudé wrote:
> On 6/25/21 8:54 AM, Mark Cave-Ayland wrote:
>> The MIPS magnum machines are available in both big endian (mips64) and little
>> endian (mips64el) configurations. Ensure that the dp893x big_endian property
>> is set accordingly using logic similar to that used for the MIPS malta
>> machines.
>>
>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
>> ---
>>   hw/mips/jazz.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
>> index 89ca8bb910..ee1789183e 100644
>> --- a/hw/mips/jazz.c
>> +++ b/hw/mips/jazz.c
>> @@ -126,7 +126,7 @@ static void mips_jazz_init(MachineState *machine,
>>   {
>>       MemoryRegion *address_space = get_system_memory();
>>       char *filename;
>> -    int bios_size, n;
>> +    int bios_size, n, big_endian;
> 
> Why not use a boolean directly?

Good point. I grepped the codebase for an existing example for using DEFINE_PROP_BOOL 
and setting the value using qdev_prop_set_bit(), and the first hit was in 
hw/arm/allwinner-h3.c for the "start-powered-off" property. The existing MIPS Malta 
code also used an integer variable to store the current endian and so that's what I 
went with.

I wonder why we don't have a qdev_prop_set_bool() to match DEFINE_PROP_BOOL?

>>       Clock *cpuclk;
>>       MIPSCPU *cpu;
>>       MIPSCPUClass *mcc;
>> @@ -158,6 +158,12 @@ static void mips_jazz_init(MachineState *machine,
>>           [JAZZ_PICA61] = {33333333, 4},
>>       };
>>   
>> +#ifdef TARGET_WORDS_BIGENDIAN
>> +    big_endian = 1;
>> +#else
>> +    big_endian = 0;
>> +#endif
>> +
>>       if (machine->ram_size > 256 * MiB) {
>>           error_report("RAM size more than 256Mb is not supported");
>>           exit(EXIT_FAILURE);
>> @@ -290,6 +296,7 @@ static void mips_jazz_init(MachineState *machine,
>>               dev = qdev_new("dp8393x");
>>               qdev_set_nic_properties(dev, nd);
>>               qdev_prop_set_uint8(dev, "it_shift", 2);
>> +            qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
>>               object_property_set_link(OBJECT(dev), "dma_mr",
>>                                        OBJECT(rc4030_dma_mr), &error_abort);
>>               sysbus = SYS_BUS_DEVICE(dev);
>>


ATB,

Mark.
Philippe Mathieu-Daudé July 1, 2021, 9:45 p.m. UTC | #3
On 6/25/21 8:54 AM, Mark Cave-Ayland wrote:
> The MIPS magnum machines are available in both big endian (mips64) and little
> endian (mips64el) configurations. Ensure that the dp893x big_endian property
> is set accordingly using logic similar to that used for the MIPS malta
> machines.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/mips/jazz.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
diff mbox series

Patch

diff --git a/hw/mips/jazz.c b/hw/mips/jazz.c
index 89ca8bb910..ee1789183e 100644
--- a/hw/mips/jazz.c
+++ b/hw/mips/jazz.c
@@ -126,7 +126,7 @@  static void mips_jazz_init(MachineState *machine,
 {
     MemoryRegion *address_space = get_system_memory();
     char *filename;
-    int bios_size, n;
+    int bios_size, n, big_endian;
     Clock *cpuclk;
     MIPSCPU *cpu;
     MIPSCPUClass *mcc;
@@ -158,6 +158,12 @@  static void mips_jazz_init(MachineState *machine,
         [JAZZ_PICA61] = {33333333, 4},
     };
 
+#ifdef TARGET_WORDS_BIGENDIAN
+    big_endian = 1;
+#else
+    big_endian = 0;
+#endif
+
     if (machine->ram_size > 256 * MiB) {
         error_report("RAM size more than 256Mb is not supported");
         exit(EXIT_FAILURE);
@@ -290,6 +296,7 @@  static void mips_jazz_init(MachineState *machine,
             dev = qdev_new("dp8393x");
             qdev_set_nic_properties(dev, nd);
             qdev_prop_set_uint8(dev, "it_shift", 2);
+            qdev_prop_set_bit(dev, "big_endian", big_endian > 0);
             object_property_set_link(OBJECT(dev), "dma_mr",
                                      OBJECT(rc4030_dma_mr), &error_abort);
             sysbus = SYS_BUS_DEVICE(dev);