Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name

qemu-devel

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name

From:	Philippe Mathieu-Daudé
Subject:	Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name
Date:	Tue, 12 May 2020 10:09:56 +0200
User-agent:	Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.5.0

On 5/11/20 10:27 AM, Aleksandar Markovic wrote:

пон, 11. мај 2020. у 10:12 Aleksandar Markovic
<address@hidden> је написао/ла:


пон, 11. мај 2020. у 08:52 chen huacai <address@hidden> је написао/ла:


Hi, Philippe and Alexandar,

On Mon, May 11, 2020 at 2:38 PM Philippe Mathieu-Daudé <address@hidden> wrote:


On 5/11/20 8:21 AM, Aleksandar Markovic wrote:

пон, 11. мај 2020. у 03:11 chen huacai <address@hidden> је написао/ла:


Hi, Philippe,

On Mon, May 11, 2020 at 5:06 AM Philippe Mathieu-Daudé <address@hidden> wrote:


We always miswrote the Fuloong machine... Fix its name.
Add an machine alias to the previous name for backward
compatibility.

Suggested-by: Aleksandar Markovic <address@hidden>
Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
---
   docs/system/target-mips.rst              |  2 +-
   default-configs/mips64el-softmmu.mak     |  2 +-
   hw/isa/vt82c686.c                        |  2 +-
   hw/mips/{mips_fulong2e.c => fuloong2e.c} | 46 ++++++++++++------------

Use mips_fuloong2e.c instead of fuloong2e.c? Other machine file names
also have a "mips_" prefix.


I would leave mips_ prefix for Fuloong, and actually add it to Boston
source file, so that we are finally consistent across all MIPS
machines.

What do you think?


These names were used years ago when all hardware was in the same hw/
directory, not sorted per target. Now new machines don't use the target
as prefix name. I'd clean the other way around, and dropping the 'mips_'
prefix. The positive side is we can 5 more characters to better describe
a patch while limited by the 72 chars in the subject :)


All having the prefix, or all dropping the prefix, are both good for
me, just keep consistency.

Huacai


Philippe, Huacai,

Prefix or not, I have mixed feelings. I had consistency more in mind
than prefix.

So it seems the prevailing opinion is slightly on the side of dropping
prefix "mips_".

Philippe, if it is not too difficult, could you perhaps make dropping
that prefix for all source file names in hw/mips a part of the this
series (not to complicate situation with a separate series) in its
follow-up version (but perhaps keep that change(s) in separate
patch(es))?


Certainly not difficult, but I won't take that as a priority.

I already spend more time trying to document better the MIPS commits, asit is important to you, and it also serves the community. Regardingunifying the file names I don't care much, so I'll not rename this fileto stop bikeshredding on futile topics and keep focus on the technicalchanges of the patches.


Conveniently enough, most of involved files do not have checkpatch
warnings and errors:

$ ../../scripts/checkpatch.pl --strict -f ./mips_fulong2e.c
total: 0 errors, 0 warnings, 404 lines checked

./mips_fulong2e.c has no obvious style problems and is ready for submission.
$ ../../scripts/checkpatch.pl --strict -f ./mips_malta.c
ERROR: if this code is redundant consider removing it
#430: FILE: ./mips_malta.c:430:
+#if 0

ERROR: if this code is redundant consider removing it
#518: FILE: ./mips_malta.c:518:
+#if 0

total: 2 errors, 0 warnings, 1458 lines checked

./mips_malta.c has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
$ ../../scripts/checkpatch.pl --strict -f ./mips_mipssim.c
total: 0 errors, 0 warnings, 246 lines checked

./mips_mipssim.c has no obvious style problems and is ready for submission.
$ ../../scripts/checkpatch.pl --strict -f ./mips_r4k.c
total: 0 errors, 0 warnings, 318 lines checked

./mips_r4k.c has no obvious style problems and is ready for submission.


Maybe we should also finally get rid of these segments in mips_malta.c:

#if 0
         printf("malta_fpga_read: Bad register offset 0x" TARGET_FMT_lx "\n",
                addr);
#endif

and

#if 0
         printf("malta_fpga_write: Bad register offset 0x" TARGET_FMT_lx "\n",
                addr);
#endif

possibly replacing them with some logging?

Yes, I had it done before the checkpatch fixes on hw/mips/, now I haveto rebase. Instead I postpone for the day I find interest in playingwith the Malta. Remember this is a hobbyist board (the Boston being thecorporate quality one).


Philippe?

Thanks,
Aleksandar

Sincerely,
Aleksandar


Aleksandar

Huacai

   hw/pci-host/bonito.c                     |  8 ++---
   tests/qtest/endianness-test.c            |  2 +-
   MAINTAINERS                              |  4 +--
   hw/mips/Kconfig                          |  2 +-
   hw/mips/Makefile.objs                    |  2 +-
   9 files changed, 36 insertions(+), 34 deletions(-)
   rename hw/mips/{mips_fulong2e.c => fuloong2e.c} (91%)

diff --git a/docs/system/target-mips.rst b/docs/system/target-mips.rst
index 2736fd0509..cd2a931edf 100644
--- a/docs/system/target-mips.rst
+++ b/docs/system/target-mips.rst
@@ -74,7 +74,7 @@ The MIPS Magnum R4000 emulation supports:

   -  G364 framebuffer

-The Fulong 2E emulation supports:
+The Fuloong 2E emulation supports:

   -  Loongson 2E CPU

diff --git a/default-configs/mips64el-softmmu.mak 
b/default-configs/mips64el-softmmu.mak
index 8b0c9b1e15..9f8a3ef156 100644
--- a/default-configs/mips64el-softmmu.mak
+++ b/default-configs/mips64el-softmmu.mak
@@ -2,7 +2,7 @@

   include mips-softmmu-common.mak
   CONFIG_IDE_VIA=y
-CONFIG_FULONG=y
+CONFIG_FULOONG=y
   CONFIG_ATI_VGA=y
   CONFIG_RTL8139_PCI=y
   CONFIG_JAZZ=y
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index d9b51fce8d..fac4e56b7d 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -503,7 +503,7 @@ static void via_class_init(ObjectClass *klass, void *data)
       dc->vmsd = &vmstate_via;
       /*
        * Reason: part of VIA VT82C686 southbridge, needs to be wired up,
-     * e.g. by mips_fulong2e_init()
+     * e.g. by mips_fuloong2e_init()
        */
       dc->user_creatable = false;
   }
diff --git a/hw/mips/mips_fulong2e.c b/hw/mips/fuloong2e.c
similarity index 91%
rename from hw/mips/mips_fulong2e.c
rename to hw/mips/fuloong2e.c
index 4e1a3646af..624c46a4fd 100644
--- a/hw/mips/mips_fulong2e.c
+++ b/hw/mips/fuloong2e.c
@@ -1,5 +1,5 @@
   /*
- * QEMU fulong 2e mini pc support
+ * QEMU fuloong 2e mini pc support
    *
    * Copyright (c) 2008 yajin (address@hidden)
    * Copyright (c) 2009 chenming (address@hidden)
@@ -11,8 +11,8 @@
    */

   /*
- * Fulong 2e mini pc is based on ICT/ST Loongson 2e CPU (MIPS III like, 800MHz)
- * http://www.linux-mips.org/wiki/Fulong
+ * Fuloong 2e mini pc is based on ICT/ST Loongson 2e CPU (MIPS III like, 
800MHz)
+ * https://www.linux-mips.org/wiki/Fuloong_2E
    *
    * Loongson 2e user manual:
    * http://www.loongsondeveloper.com/doc/Loongson2EUserGuide.pdf
@@ -46,13 +46,13 @@
   #include "sysemu/reset.h"
   #include "qemu/error-report.h"

-#define DEBUG_FULONG2E_INIT
+#define DEBUG_FULOONG2E_INIT

   #define ENVP_ADDR               0x80002000l
   #define ENVP_NB_ENTRIES         16
   #define ENVP_ENTRY_SIZE         256

-/* fulong 2e has a 512k flash: Winbond W39L040AP70Z */
+/* Fuloong 2e has a 512k flash: Winbond W39L040AP70Z */
   #define BIOS_SIZE               (512 * KiB)
   #define MAX_IDE_BUS             2

@@ -69,12 +69,12 @@
    * 2, use "Bonito2edev" to replace 
"dir_corresponding_to_your_target_hardware"
    * in the "Compile Guide".
    */
-#define FULONG_BIOSNAME "pmon_2e.bin"
+#define FULOONG_BIOSNAME "pmon_2e.bin"

-/* PCI SLOT in fulong 2e */
-#define FULONG2E_VIA_SLOT        5
-#define FULONG2E_ATI_SLOT        6
-#define FULONG2E_RTL8139_SLOT    7
+/* PCI SLOT in Fuloong 2e */
+#define FULOONG2E_VIA_SLOT        5
+#define FULOONG2E_ATI_SLOT        6
+#define FULOONG2E_RTL8139_SLOT    7

   static struct _loaderparams {
       int ram_size;
@@ -279,7 +279,7 @@ static void network_init(PCIBus *pci_bus)
           const char *default_devaddr = NULL;

           if (i == 0 && (!nd->model || strcmp(nd->model, "rtl8139") == 0)) {
-            /* The fulong board has a RTL8139 card using PCI SLOT 7 */
+            /* The Fuloong board has a RTL8139 card using PCI SLOT 7 */
               default_devaddr = "07";
           }

@@ -287,7 +287,7 @@ static void network_init(PCIBus *pci_bus)
       }
   }

-static void mips_fulong2e_init(MachineState *machine)
+static void mips_fuloong2e_init(MachineState *machine)
   {
       const char *kernel_filename = machine->kernel_filename;
       const char *kernel_cmdline = machine->kernel_cmdline;
@@ -316,11 +316,12 @@ static void mips_fulong2e_init(MachineState *machine)
           error_report("Invalid RAM size, should be 256MB");
           exit(EXIT_FAILURE);
       }
-    memory_region_add_subregion(address_space_mem, 0, machine->ram);

-    /* Boot ROM */
-    memory_region_init_rom(bios, NULL, "fulong2e.bios", BIOS_SIZE,
+    /* allocate RAM */
+    memory_region_init_rom(bios, NULL, "fuloong2e.bios", BIOS_SIZE,
                              &error_fatal);
+
+    memory_region_add_subregion(address_space_mem, 0, machine->ram);
       memory_region_add_subregion(address_space_mem, 0x1fc00000LL, bios);

       /*
@@ -337,7 +338,7 @@ static void mips_fulong2e_init(MachineState *machine)
           write_bootloader(env, memory_region_get_ram_ptr(bios), kernel_entry);
       } else {
           if (bios_name == NULL) {
-                bios_name = FULONG_BIOSNAME;
+                bios_name = FULOONG_BIOSNAME;
           }
           filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
           if (filename) {
@@ -363,7 +364,7 @@ static void mips_fulong2e_init(MachineState *machine)
       pci_bus = bonito_init((qemu_irq *)&(env->irq[2]));

       /* South bridge -> IP5 */
-    vt82c686b_southbridge_init(pci_bus, FULONG2E_VIA_SLOT, env->irq[5],
+    vt82c686b_southbridge_init(pci_bus, FULOONG2E_VIA_SLOT, env->irq[5],
                                  &smbus, &isa_bus);

       /* GPU */
@@ -384,14 +385,15 @@ static void mips_fulong2e_init(MachineState *machine)
       network_init(pci_bus);
   }

-static void mips_fulong2e_machine_init(MachineClass *mc)
+static void mips_fuloong2e_machine_init(MachineClass *mc)
   {
-    mc->desc = "Fulong 2e mini pc";
-    mc->init = mips_fulong2e_init;
+    mc->desc = "Fuloong 2e mini pc";
+    mc->alias = "fulong2e";             /* Incorrect name used up to QEMU 4.2 
*/
+    mc->init = mips_fuloong2e_init;
       mc->block_default_type = IF_IDE;
       mc->default_cpu_type = MIPS_CPU_TYPE_NAME("Loongson-2E");
       mc->default_ram_size = 256 * MiB;
-    mc->default_ram_id = "fulong2e.ram";
+    mc->default_ram_id = "fuloong2e.ram";
   }

-DEFINE_MACHINE("fulong2e", mips_fulong2e_machine_init)
+DEFINE_MACHINE("fuloong2e", mips_fuloong2e_machine_init)
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index cc6545c8a8..b9bfe3c417 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -11,7 +11,7 @@
    */

   /*
- * fulong 2e mini pc has a bonito north bridge.
+ * fuloong 2e mini pc has a bonito north bridge.
    */

   /*
@@ -559,11 +559,11 @@ static int pci_bonito_map_irq(PCIDevice *pci_dev, int 
irq_num)
       slot = (pci_dev->devfn >> 3);

       switch (slot) {
-    case 5:   /* FULONG2E_VIA_SLOT, SouthBridge, IDE, USB, ACPI, AC97, MC97 */
+    case 5:   /* FULOONG2E_VIA_SLOT, SouthBridge, IDE, USB, ACPI, AC97, MC97 */
           return irq_num % 4 + BONITO_IRQ_BASE;
-    case 6:   /* FULONG2E_ATI_SLOT, VGA */
+    case 6:   /* FULOONG2E_ATI_SLOT, VGA */
           return 4 + BONITO_IRQ_BASE;
-    case 7:   /* FULONG2E_RTL_SLOT, RTL8139 */
+    case 7:   /* FULOONG2E_RTL_SLOT, RTL8139 */
           return 5 + BONITO_IRQ_BASE;
       case 8 ... 12: /* PCI slot 1 to 4 */
           return (slot - 8 + irq_num) + 6 + BONITO_IRQ_BASE;
diff --git a/tests/qtest/endianness-test.c b/tests/qtest/endianness-test.c
index 2798802c63..cc088ac01a 100644
--- a/tests/qtest/endianness-test.c
+++ b/tests/qtest/endianness-test.c
@@ -33,7 +33,7 @@ static const TestCase test_cases[] = {
       { "mips64", "pica61", 0x90000000, .bswap = true },
       { "mips64", "mips", 0x14000000, .bswap = true },
       { "mips64", "malta", 0x10000000, .bswap = true },
-    { "mips64el", "fulong2e", 0x1fd00000 },
+    { "mips64el", "fuloong2e", 0x1fd00000 },
       { "ppc", "g3beige", 0xfe000000, .bswap = true, .superio = "i82378" },
       { "ppc", "40p", 0x80000000, .bswap = true },
       { "ppc", "bamboo", 0xe8000000, .bswap = true, .superio = "i82378" },
diff --git a/MAINTAINERS b/MAINTAINERS
index aa5c54c75a..50f6a5f1bb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1074,13 +1074,13 @@ R: Aleksandar Rikalo <address@hidden>
   S: Obsolete
   F: hw/mips/mips_r4k.c

-Fulong 2E
+Fuloong 2E
   M: Huacai Chen <address@hidden>
   M: Philippe Mathieu-Daudé <address@hidden>
   M: Aleksandar Markovic <address@hidden>
   R: Jiaxun Yang <address@hidden>
   S: Odd Fixes
-F: hw/mips/mips_fulong2e.c
+F: hw/mips/fuloong2e.c
   F: hw/isa/vt82c686.c
   F: hw/pci-host/bonito.c
   F: include/hw/isa/vt82c686.h
diff --git a/hw/mips/Kconfig b/hw/mips/Kconfig
index 2c2adbc42a..cd38546689 100644
--- a/hw/mips/Kconfig
+++ b/hw/mips/Kconfig
@@ -41,7 +41,7 @@ config JAZZ
       select DS1225Y
       select JAZZ_LED

-config FULONG
+config FULOONG
       bool

   config MIPS_CPS
diff --git a/hw/mips/Makefile.objs b/hw/mips/Makefile.objs
index 525809af07..8ab41edc3f 100644
--- a/hw/mips/Makefile.objs
+++ b/hw/mips/Makefile.objs
@@ -3,6 +3,6 @@ obj-$(CONFIG_R4K) += mips_r4k.o
   obj-$(CONFIG_MALTA) += gt64xxx_pci.o mips_malta.o
   obj-$(CONFIG_MIPSSIM) += mips_mipssim.o
   obj-$(CONFIG_JAZZ) += mips_jazz.o
-obj-$(CONFIG_FULONG) += mips_fulong2e.o
+obj-$(CONFIG_FULOONG) += fuloong2e.o
   obj-$(CONFIG_MIPS_CPS) += cps.o
   obj-$(CONFIG_MIPS_BOSTON) += boston.o
--
2.21.3



--
Huacai Chen




--
Huacai Chen

[Prev in Thread]

Current Thread

[Next in Thread]

Re: [PATCH 05/12] hw/pci-host: Use CONFIG_PCI_BONITO to select the Bonito North Bridge, (continued)
- [PATCH 03/12] hw/mips/fuloong2e: Move code and update a comment, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 03/12] hw/mips/fuloong2e: Move code and update a comment, chen huacai, 2020/05/10
- [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, chen huacai, 2020/05/10
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Aleksandar Markovic, 2020/05/11
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Philippe Mathieu-Daudé, 2020/05/11
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, chen huacai, 2020/05/11
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Aleksandar Markovic, 2020/05/11
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Aleksandar Markovic, 2020/05/11
    - Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name, Philippe Mathieu-Daudé <=
- [PATCH 06/12] hw/pci-host/bonito: Fix DPRINTF() format strings, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 06/12] hw/pci-host/bonito: Fix DPRINTF() format strings, chen huacai, 2020/05/10
- [PATCH 08/12] hw/pci-host/bonito: Map all the Bonito64 I/O range, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 08/12] hw/pci-host/bonito: Map all the Bonito64 I/O range, Aleksandar Markovic, 2020/05/11
- [PATCH 07/12] hw/pci-host/bonito: Map peripheral using physical address, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 07/12] hw/pci-host/bonito: Map peripheral using physical address, Aleksandar Markovic, 2020/05/11
- [PATCH 09/12] hw/pci-host/bonito: Map the different PCI ranges more detailled, Philippe Mathieu-Daudé, 2020/05/10
  - Re: [PATCH 09/12] hw/pci-host/bonito: Map the different PCI ranges more detailled, Aleksandar Markovic, 2020/05/11
    - Re: [PATCH 09/12] hw/pci-host/bonito: Map the different PCI ranges more detailled, Philippe Mathieu-Daudé, 2020/05/11
- [PATCH 10/12] hw/pci-host/bonito: Better describe the I/O CS regions, Philippe Mathieu-Daudé, 2020/05/10

Prev by Date: Re: [PATCH] i386/kvm: fix a use-after-free when vcpu plug/unplug
Next by Date: Re: [PATCH 2/4] fuzz: fix typo in i440fx-qtest-reboot arguments
Previous by thread: Re: [PATCH 04/12] hw/mips/fuloong2e: Fix typo in Fuloong machine name
Next by thread: [PATCH 06/12] hw/pci-host/bonito: Fix DPRINTF() format strings
Index(es):
- Date
- Thread