All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] mips: machines: Renovate coding style
@ 2019-11-25 13:04 Filip Bozuta
  2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
                   ` (4 more replies)
  0 siblings, 5 replies; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The scripts checkpatch.pl located in scripts folder
was used to check for errors and warnings in fllowing
mips machines:

   Jazz
   Malta
   Mipssim
   R4000
   Fulong 2E
   Boston

All generated errors and warnings were corrected
and the script was ran again to make sure there
are no more errors and warnings

Note:
   Boston machine was already clear of errors
   and warnings so the boston machine files 
   were not edited.

Note:
   One error occured due to the machro #IF 0.
   That error was not corrected beacuse there
   is some reduntant code within that macro that
   might be used in future versions.


Filip Bozuta (5):
  mips: jazz: Renovate coding style
  mips: malta: Renovate coding style
  mips: mipssim: Renovate coding style
  mips: r4000: Renovate coding style
  mips: fulong 2e: Renovate coding style

 hw/display/jazz_led.c                    | 123 +++++++++++++--------------
 hw/dma/rc4030.c                          |  12 +--
 hw/isa/vt82c686.c                        |  23 ++---
 hw/mips/mips_jazz.c                      |  32 ++++---
 hw/mips/mips_malta.c                     | 139 ++++++++++++++++---------------
 hw/mips/mips_r4k.c                       |  55 +++++++-----
 hw/net/mipsnet.c                         |  44 +++++-----
 hw/pci-host/bonito.c                     |  60 +++++++------
 tests/acceptance/linux_ssh_mips_malta.py |   6 +-
 9 files changed, 266 insertions(+), 228 deletions(-)

-- 
2.7.4



^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/5] mips: jazz: Renovate coding style
  2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
@ 2019-11-25 13:04 ` Filip Bozuta
  2019-12-06  8:07   ` Aleksandar Markovic
  2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in files:
    hw/mips/mips_jazz.c
    hw/display/jazz_led.c
    hw/dma/rc4030.c

All these mips jazz machine files were edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
---
 hw/display/jazz_led.c | 123 +++++++++++++++++++++++++-------------------------
 hw/dma/rc4030.c       |  12 +++--
 hw/mips/mips_jazz.c   |  32 +++++++------
 3 files changed, 88 insertions(+), 79 deletions(-)

diff --git a/hw/display/jazz_led.c b/hw/display/jazz_led.c
index 3e0112b..1d84559 100644
--- a/hw/display/jazz_led.c
+++ b/hw/display/jazz_led.c
@@ -90,25 +90,25 @@ static void draw_horizontal_line(DisplaySurface *ds,
 
     bpp = (surface_bits_per_pixel(ds) + 7) >> 3;
     d = surface_data(ds) + surface_stride(ds) * posy + bpp * posx1;
-    switch(bpp) {
-        case 1:
-            for (x = posx1; x <= posx2; x++) {
-                *((uint8_t *)d) = color;
-                d++;
-            }
-            break;
-        case 2:
-            for (x = posx1; x <= posx2; x++) {
-                *((uint16_t *)d) = color;
-                d += 2;
-            }
-            break;
-        case 4:
-            for (x = posx1; x <= posx2; x++) {
-                *((uint32_t *)d) = color;
-                d += 4;
-            }
-            break;
+    switch (bpp) {
+    case 1:
+        for (x = posx1; x <= posx2; x++) {
+            *((uint8_t *)d) = color;
+            d++;
+        }
+        break;
+    case 2:
+        for (x = posx1; x <= posx2; x++) {
+            *((uint16_t *)d) = color;
+            d += 2;
+        }
+        break;
+    case 4:
+        for (x = posx1; x <= posx2; x++) {
+            *((uint32_t *)d) = color;
+            d += 4;
+        }
+        break;
     }
 }
 
@@ -121,25 +121,25 @@ static void draw_vertical_line(DisplaySurface *ds,
 
     bpp = (surface_bits_per_pixel(ds) + 7) >> 3;
     d = surface_data(ds) + surface_stride(ds) * posy1 + bpp * posx;
-    switch(bpp) {
-        case 1:
-            for (y = posy1; y <= posy2; y++) {
-                *((uint8_t *)d) = color;
-                d += surface_stride(ds);
-            }
-            break;
-        case 2:
-            for (y = posy1; y <= posy2; y++) {
-                *((uint16_t *)d) = color;
-                d += surface_stride(ds);
-            }
-            break;
-        case 4:
-            for (y = posy1; y <= posy2; y++) {
-                *((uint32_t *)d) = color;
-                d += surface_stride(ds);
-            }
-            break;
+    switch (bpp) {
+    case 1:
+        for (y = posy1; y <= posy2; y++) {
+            *((uint8_t *)d) = color;
+            d += surface_stride(ds);
+        }
+        break;
+    case 2:
+        for (y = posy1; y <= posy2; y++) {
+            *((uint16_t *)d) = color;
+            d += surface_stride(ds);
+        }
+        break;
+    case 4:
+        for (y = posy1; y <= posy2; y++) {
+            *((uint32_t *)d) = color;
+            d += surface_stride(ds);
+        }
+        break;
     }
 }
 
@@ -164,28 +164,28 @@ static void jazz_led_update_display(void *opaque)
     if (s->state & REDRAW_SEGMENTS) {
         /* set colors according to bpp */
         switch (surface_bits_per_pixel(surface)) {
-            case 8:
-                color_segment = rgb_to_pixel8(0xaa, 0xaa, 0xaa);
-                color_led = rgb_to_pixel8(0x00, 0xff, 0x00);
-                break;
-            case 15:
-                color_segment = rgb_to_pixel15(0xaa, 0xaa, 0xaa);
-                color_led = rgb_to_pixel15(0x00, 0xff, 0x00);
-                break;
-            case 16:
-                color_segment = rgb_to_pixel16(0xaa, 0xaa, 0xaa);
-                color_led = rgb_to_pixel16(0x00, 0xff, 0x00);
-                break;
-            case 24:
-                color_segment = rgb_to_pixel24(0xaa, 0xaa, 0xaa);
-                color_led = rgb_to_pixel24(0x00, 0xff, 0x00);
-                break;
-            case 32:
-                color_segment = rgb_to_pixel32(0xaa, 0xaa, 0xaa);
-                color_led = rgb_to_pixel32(0x00, 0xff, 0x00);
-                break;
-            default:
-                return;
+        case 8:
+            color_segment = rgb_to_pixel8(0xaa, 0xaa, 0xaa);
+            color_led = rgb_to_pixel8(0x00, 0xff, 0x00);
+            break;
+        case 15:
+            color_segment = rgb_to_pixel15(0xaa, 0xaa, 0xaa);
+            color_led = rgb_to_pixel15(0x00, 0xff, 0x00);
+            break;
+        case 16:
+            color_segment = rgb_to_pixel16(0xaa, 0xaa, 0xaa);
+            color_led = rgb_to_pixel16(0x00, 0xff, 0x00);
+            break;
+        case 24:
+            color_segment = rgb_to_pixel24(0xaa, 0xaa, 0xaa);
+            color_led = rgb_to_pixel24(0x00, 0xff, 0x00);
+            break;
+        case 32:
+            color_segment = rgb_to_pixel32(0xaa, 0xaa, 0xaa);
+            color_led = rgb_to_pixel32(0x00, 0xff, 0x00);
+            break;
+        default:
+            return;
         }
 
         /* display segments */
@@ -205,8 +205,9 @@ static void jazz_led_update_display(void *opaque)
                              (s->segments & 0x80) ? color_segment : 0);
 
         /* display led */
-        if (!(s->segments & 0x01))
+        if (!(s->segments & 0x01)) {
             color_led = 0; /* black */
+        }
         draw_horizontal_line(surface, 68, 50, 50, color_led);
         draw_horizontal_line(surface, 69, 49, 51, color_led);
         draw_horizontal_line(surface, 70, 48, 52, color_led);
diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
index d54e296..f7542f3 100644
--- a/hw/dma/rc4030.c
+++ b/hw/dma/rc4030.c
@@ -397,10 +397,11 @@ static void update_jazz_irq(rc4030State *s)
 
     pending = s->isr_jazz & s->imr_jazz;
 
-    if (pending != 0)
+    if (pending != 0) {
         qemu_irq_raise(s->jazz_bus_irq);
-    else
+    } else {
         qemu_irq_lower(s->jazz_bus_irq);
+    }
 }
 
 static void rc4030_irq_jazz_request(void *opaque, int irq, int level)
@@ -588,7 +589,8 @@ static const VMStateDescription vmstate_rc4030 = {
     }
 };
 
-static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int is_write)
+static void rc4030_do_dma(void *opaque, int n, uint8_t *buf,
+                          int len, int is_write)
 {
     rc4030State *s = opaque;
     hwaddr dma_addr;
@@ -643,8 +645,8 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque, int n)
     struct rc4030DMAState *p;
     int i;
 
-    s = (rc4030_dma *)g_malloc0(sizeof(rc4030_dma) * n);
-    p = (struct rc4030DMAState *)g_malloc0(sizeof(struct rc4030DMAState) * n);
+    s = (rc4030_dma *)g_new0(sizeof(rc4030_dma) * n);
+    p = (struct rc4030DMAState *)g_new0(sizeof(struct rc4030DMAState) * n);
     for (i = 0; i < n; i++) {
         p->opaque = opaque;
         p->n = i;
diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
index d978bb6..ac4d7ac 100644
--- a/hw/mips/mips_jazz.c
+++ b/hw/mips/mips_jazz.c
@@ -52,8 +52,7 @@
 #include "qemu/error-report.h"
 #include "qemu/help_option.h"
 
-enum jazz_model_e
-{
+enum jazz_model_e {
     JAZZ_MAGNUM,
     JAZZ_PICA61,
 };
@@ -90,16 +89,20 @@ static const MemoryRegionOps rtc_ops = {
 static uint64_t dma_dummy_read(void *opaque, hwaddr addr,
                                unsigned size)
 {
-    /* Nothing to do. That is only to ensure that
-     * the current DMA acknowledge cycle is completed. */
+    /*
+     * Nothing to do. That is only to ensure that
+     * the current DMA acknowledge cycle is completed.
+     */
     return 0xff;
 }
 
 static void dma_dummy_write(void *opaque, hwaddr addr,
                             uint64_t val, unsigned size)
 {
-    /* Nothing to do. That is only to ensure that
-     * the current DMA acknowledge cycle is completed. */
+    /*
+     * Nothing to do. That is only to ensure that
+     * the current DMA acknowledge cycle is completed.
+     */
 }
 
 static const MemoryRegionOps dma_dummy_ops = {
@@ -109,8 +112,8 @@ static const MemoryRegionOps dma_dummy_ops = {
 };
 
 #define MAGNUM_BIOS_SIZE_MAX 0x7e000
-#define MAGNUM_BIOS_SIZE (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : MAGNUM_BIOS_SIZE_MAX)
-
+#define MAGNUM_BIOS_SIZE                                                       \
+        (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE : MAGNUM_BIOS_SIZE_MAX)
 static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
                                           vaddr addr, unsigned size,
                                           MMUAccessType access_type,
@@ -201,8 +204,9 @@ static void mips_jazz_init(MachineState *machine,
     memory_region_add_subregion(address_space, 0xfff00000LL, bios2);
 
     /* load the BIOS image. */
-    if (bios_name == NULL)
+    if (bios_name == NULL) {
         bios_name = BIOS_FILENAME;
+    }
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
     if (filename) {
         bios_size = load_image_targphys(filename, 0xfff00000LL,
@@ -229,7 +233,8 @@ static void mips_jazz_init(MachineState *machine,
                                 sysbus_mmio_get_region(sysbus, 0));
     memory_region_add_subregion(address_space, 0xf0000000,
                                 sysbus_mmio_get_region(sysbus, 1));
-    memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL, "dummy_dma", 0x1000);
+    memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops,
+                          NULL, "dummy_dma", 0x1000);
     memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
 
     /* ISA bus: IO space at 0x90000000, mem space at 0x91000000 */
@@ -276,8 +281,9 @@ static void mips_jazz_init(MachineState *machine,
     /* Network controller */
     for (n = 0; n < nb_nics; n++) {
         nd = &nd_table[n];
-        if (!nd->model)
+        if (!nd->model) {
             nd->model = g_strdup("dp83932");
+        }
         if (strcmp(nd->model, "dp83932") == 0) {
             qemu_check_nic_model(nd, "dp83932");
 
@@ -338,12 +344,12 @@ static void mips_jazz_init(MachineState *machine,
     /* Serial ports */
     if (serial_hd(0)) {
         serial_mm_init(address_space, 0x80006000, 0,
-                       qdev_get_gpio_in(rc4030, 8), 8000000/16,
+                       qdev_get_gpio_in(rc4030, 8), 8000000 / 16,
                        serial_hd(0), DEVICE_NATIVE_ENDIAN);
     }
     if (serial_hd(1)) {
         serial_mm_init(address_space, 0x80007000, 0,
-                       qdev_get_gpio_in(rc4030, 9), 8000000/16,
+                       qdev_get_gpio_in(rc4030, 9), 8000000 / 16,
                        serial_hd(1), DEVICE_NATIVE_ENDIAN);
     }
 
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/5] mips: malta: Renovate coding style
  2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
  2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
@ 2019-11-25 13:04 ` Filip Bozuta
  2019-11-30 23:46   ` Aleksandar Markovic
                     ` (2 more replies)
  2019-11-25 13:04 ` [PATCH 3/5] mips: mipssim: " Filip Bozuta
                   ` (2 subsequent siblings)
  4 siblings, 3 replies; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in files:
    hw/mips/mips_malta.c
    hw/mips/gt64xxx_pci.c
    tests/acceptance/linux_ssh_mips_malta.py

All these mips malta machine files were edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
---
 hw/mips/mips_malta.c                     | 139 ++++++++++++++++---------------
 tests/acceptance/linux_ssh_mips_malta.py |   6 +-
 2 files changed, 75 insertions(+), 70 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 92e9ca5..18eafac 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
  */
 
 #if defined(DEBUG)
-#  define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
+#  define logout(fmt, ...) \
+          fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
 #else
 #  define logout(fmt, ...) ((void)0)
 #endif
@@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion *address_space,
     MaltaFPGAState *s;
     Chardev *chr;
 
-    s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
+    s = (MaltaFPGAState *)g_new0(sizeof(MaltaFPGAState));
 
     memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
                           "malta-fpga", 0x100000);
@@ -844,9 +845,9 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
     /* Small bootloader */
     p = (uint32_t *)base;
 
-    stl_p(p++, 0x08000000 |                                      /* j 0x1fc00580 */
+    stl_p(p++, 0x08000000 |                                   /* j 0x1fc00580 */
                  ((run_addr + 0x580) & 0x0fffffff) >> 2);
-    stl_p(p++, 0x00000000);                                      /* nop */
+    stl_p(p++, 0x00000000);                                   /* nop */
 
     /* YAMON service vector */
     stl_p(base + 0x500, run_addr + 0x0580);      /* start: */
@@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
     stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
 
     /* Load BAR registers as done by YAMON */
-    stl_p(p++, 0x3c09b400);                                      /* lui t1, 0xb400 */
+    stl_p(p++, 0x3c09b400);                 /* lui t1, 0xb400 */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08df00);                                      /* lui t0, 0xdf00 */
+    stl_p(p++, 0x3c08df00);                  /* lui t0, 0xdf00 */
 #else
-    stl_p(p++, 0x340800df);                                      /* ori t0, r0, 0x00df */
+    stl_p(p++, 0x340800df);                  /* ori t0, r0, 0x00df */
 #endif
-    stl_p(p++, 0xad280068);                                      /* sw t0, 0x0068(t1) */
+    stl_p(p++, 0xad280068);                  /* sw t0, 0x0068(t1) */
 
-    stl_p(p++, 0x3c09bbe0);                                      /* lui t1, 0xbbe0 */
+    stl_p(p++, 0x3c09bbe0);                  /* lui t1, 0xbbe0 */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08c000);                                      /* lui t0, 0xc000 */
+    stl_p(p++, 0x3c08c000);                  /* lui t0, 0xc000 */
 #else
-    stl_p(p++, 0x340800c0);                                      /* ori t0, r0, 0x00c0 */
+    stl_p(p++, 0x340800c0);                  /* ori t0, r0, 0x00c0 */
 #endif
-    stl_p(p++, 0xad280048);                                      /* sw t0, 0x0048(t1) */
+    stl_p(p++, 0xad280048);                  /* sw t0, 0x0048(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c084000);                                      /* lui t0, 0x4000 */
+    stl_p(p++, 0x3c084000);                  /* lui t0, 0x4000 */
 #else
-    stl_p(p++, 0x34080040);                                      /* ori t0, r0, 0x0040 */
+    stl_p(p++, 0x34080040);                  /* ori t0, r0, 0x0040 */
 #endif
-    stl_p(p++, 0xad280050);                                      /* sw t0, 0x0050(t1) */
+    stl_p(p++, 0xad280050);                  /* sw t0, 0x0050(t1) */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c088000);                                      /* lui t0, 0x8000 */
+    stl_p(p++, 0x3c088000);                  /* lui t0, 0x8000 */
 #else
-    stl_p(p++, 0x34080080);                                      /* ori t0, r0, 0x0080 */
+    stl_p(p++, 0x34080080);                  /* ori t0, r0, 0x0080 */
 #endif
-    stl_p(p++, 0xad280058);                                      /* sw t0, 0x0058(t1) */
+    stl_p(p++, 0xad280058);                  /* sw t0, 0x0058(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c083f00);                                      /* lui t0, 0x3f00 */
+    stl_p(p++, 0x3c083f00);                  /* lui t0, 0x3f00 */
 #else
-    stl_p(p++, 0x3408003f);                                      /* ori t0, r0, 0x003f */
+    stl_p(p++, 0x3408003f);                  /* ori t0, r0, 0x003f */
 #endif
-    stl_p(p++, 0xad280060);                                      /* sw t0, 0x0060(t1) */
+    stl_p(p++, 0xad280060);                  /* sw t0, 0x0060(t1) */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08c100);                                      /* lui t0, 0xc100 */
+    stl_p(p++, 0x3c08c100);                  /* lui t0, 0xc100 */
 #else
-    stl_p(p++, 0x340800c1);                                      /* ori t0, r0, 0x00c1 */
+    stl_p(p++, 0x340800c1);                  /* ori t0, r0, 0x00c1 */
 #endif
-    stl_p(p++, 0xad280080);                                      /* sw t0, 0x0080(t1) */
+    stl_p(p++, 0xad280080);                  /* sw t0, 0x0080(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c085e00);                                      /* lui t0, 0x5e00 */
+    stl_p(p++, 0x3c085e00);                  /* lui t0, 0x5e00 */
 #else
-    stl_p(p++, 0x3408005e);                                      /* ori t0, r0, 0x005e */
+    stl_p(p++, 0x3408005e);                  /* ori t0, r0, 0x005e */
 #endif
-    stl_p(p++, 0xad280088);                                      /* sw t0, 0x0088(t1) */
+    stl_p(p++, 0xad280088);                  /* sw t0, 0x0088(t1) */
 
     /* Jump to kernel code */
-    stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff));    /* lui ra, high(kernel_entry) */
-    stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff));            /* ori ra, ra, low(kernel_entry) */
-    stl_p(p++, 0x03e00009);                                      /* jalr ra */
-    stl_p(p++, 0x00000000);                                      /* nop */
+    stl_p(p++, 0x3c1f0000 |
+          ((kernel_entry >> 16) & 0xffff));  /* lui ra, high(kernel_entry) */
+    stl_p(p++, 0x37ff0000 |
+          (kernel_entry & 0xffff));          /* ori ra, ra, low(kernel_entry) */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0x00000000);                  /* nop */
 
     /* YAMON subroutines */
     p = (uint32_t *) (base + 0x800);
-    stl_p(p++, 0x03e00009);                                     /* jalr ra */
-    stl_p(p++, 0x24020000);                                     /* li v0,0 */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0x24020000);                  /* li v0,0 */
     /* 808 YAMON print */
-    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
-    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
-    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
-    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
-    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
-    stl_p(p++, 0x10800005);                                     /* beqz a0,834 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x1000fff9);                                     /* b 814 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
-    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
+    stl_p(p++, 0x03e06821);                  /* move t5,ra */
+    stl_p(p++, 0x00805821);                  /* move t3,a0 */
+    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
+    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
+    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
+    stl_p(p++, 0x10800005);                  /* beqz a0,834 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x1000fff9);                  /* b 814 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x01a00009);                  /* jalr t5 */
+    stl_p(p++, 0x01602021);                  /* move a0,t3 */
     /* 0x83c YAMON print_count */
-    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
-    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
-    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
-    stl_p(p++, 0x00c06021);                                     /* move t4,a2 */
-    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
-    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
-    stl_p(p++, 0x258cffff);                                     /* addiu t4,t4,-1 */
-    stl_p(p++, 0x1580fffa);                                     /* bnez t4,84c */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
-    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
+    stl_p(p++, 0x03e06821);                  /* move t5,ra */
+    stl_p(p++, 0x00805821);                  /* move t3,a0 */
+    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
+    stl_p(p++, 0x00c06021);                  /* move t4,a2 */
+    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
+    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
+    stl_p(p++, 0x258cffff);                  /* addiu t4,t4,-1 */
+    stl_p(p++, 0x1580fffa);                  /* bnez t4,84c */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x01a00009);                  /* jalr t5 */
+    stl_p(p++, 0x01602021);                  /* move a0,t3 */
     /* 0x870 */
-    stl_p(p++, 0x3c08b800);                                     /* lui t0,0xb400 */
-    stl_p(p++, 0x350803f8);                                     /* ori t0,t0,0x3f8 */
-    stl_p(p++, 0x91090005);                                     /* lbu t1,5(t0) */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x31290040);                                     /* andi t1,t1,0x40 */
-    stl_p(p++, 0x1120fffc);                                     /* beqz t1,878 <outch+0x8> */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x03e00009);                                     /* jalr ra */
-    stl_p(p++, 0xa1040000);                                     /* sb a0,0(t0) */
+    stl_p(p++, 0x3c08b800);                  /* lui t0,0xb400 */
+    stl_p(p++, 0x350803f8);                  /* ori t0,t0,0x3f8 */
+    stl_p(p++, 0x91090005);                  /* lbu t1,5(t0) */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x31290040);                  /* andi t1,t1,0x40 */
+    stl_p(p++, 0x1120fffc);                  /* beqz t1,878 <outch+0x8> */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0xa1040000);                  /* sb a0,0(t0) */
 
 }
 
diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
index fc13f9e..44e1118 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -99,10 +99,12 @@ class LinuxSSH(Test):
     def ssh_command(self, command, is_root=True):
         self.ssh_logger.info(command)
         result = self.ssh_session.cmd(command)
-        stdout_lines = [line.rstrip() for line in result.stdout_text.splitlines()]
+        stdout_lines = [line.rstrip() for line
+        in result.stdout_text.splitlines()]
         for line in stdout_lines:
             self.ssh_logger.info(line)
-        stderr_lines = [line.rstrip() for line in result.stderr_text.splitlines()]
+        stderr_lines = [line.rstrip() for line
+        in result.stderr_text.splitlines()]
         for line in stderr_lines:
             self.ssh_logger.warning(line)
         return stdout_lines, stderr_lines
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/5] mips: mipssim: Renovate coding style
  2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
  2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
  2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
@ 2019-11-25 13:04 ` Filip Bozuta
  2019-12-06  8:03   ` Aleksandar Markovic
  2019-11-25 13:04 ` [PATCH 4/5] mips: r4000: " Filip Bozuta
  2019-11-25 13:04 ` [PATCH 5/5] mips: fulong 2e: " Filip Bozuta
  4 siblings, 1 reply; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in files:
    hw/mips/mips_mipssim.c
    hw/net/mipsnet.c

All these mips mipssim machine files were edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
---
 hw/net/mipsnet.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c
index f7ae1ce..cb3331f 100644
--- a/hw/net/mipsnet.c
+++ b/hw/net/mipsnet.c
@@ -9,19 +9,19 @@
 
 /* MIPSnet register offsets */
 
-#define MIPSNET_DEV_ID		0x00
-#define MIPSNET_BUSY		0x08
-#define MIPSNET_RX_DATA_COUNT	0x0c
-#define MIPSNET_TX_DATA_COUNT	0x10
-#define MIPSNET_INT_CTL		0x14
-# define MIPSNET_INTCTL_TXDONE		0x00000001
-# define MIPSNET_INTCTL_RXDONE		0x00000002
-# define MIPSNET_INTCTL_TESTBIT		0x80000000
-#define MIPSNET_INTERRUPT_INFO	0x18
-#define MIPSNET_RX_DATA_BUFFER	0x1c
-#define MIPSNET_TX_DATA_BUFFER	0x20
-
-#define MAX_ETH_FRAME_SIZE	1514
+#define MIPSNET_DEV_ID          0x00
+#define MIPSNET_BUSY            0x08
+#define MIPSNET_RX_DATA_COUNT   0x0c
+#define MIPSNET_TX_DATA_COUNT   0x10
+#define MIPSNET_INT_CTL         0x14
+# define MIPSNET_INTCTL_TXDONE          0x00000001
+# define MIPSNET_INTCTL_RXDONE          0x00000002
+# define MIPSNET_INTCTL_TESTBIT         0x80000000
+#define MIPSNET_INTERRUPT_INFO  0x18
+#define MIPSNET_RX_DATA_BUFFER  0x1c
+#define MIPSNET_TX_DATA_BUFFER  0x20
+
+#define MAX_ETH_FRAME_SIZE      1514
 
 #define TYPE_MIPS_NET "mipsnet"
 #define MIPS_NET(obj) OBJECT_CHECK(MIPSnetState, (obj), TYPE_MIPS_NET)
@@ -64,8 +64,9 @@ static void mipsnet_update_irq(MIPSnetState *s)
 
 static int mipsnet_buffer_full(MIPSnetState *s)
 {
-    if (s->rx_count >= MAX_ETH_FRAME_SIZE)
+    if (s->rx_count >= MAX_ETH_FRAME_SIZE) {
         return 1;
+    }
     return 0;
 }
 
@@ -73,18 +74,21 @@ static int mipsnet_can_receive(NetClientState *nc)
 {
     MIPSnetState *s = qemu_get_nic_opaque(nc);
 
-    if (s->busy)
+    if (s->busy) {
         return 0;
+    }
     return !mipsnet_buffer_full(s);
 }
 
-static ssize_t mipsnet_receive(NetClientState *nc, const uint8_t *buf, size_t size)
+static ssize_t mipsnet_receive(NetClientState *nc,
+                               const uint8_t *buf, size_t size)
 {
     MIPSnetState *s = qemu_get_nic_opaque(nc);
 
-    trace_mipsnet_receive(size);
-    if (!mipsnet_can_receive(nc))
+    race_mipsnet_receive(size);
+    if (!mipsnet_can_receive(nc)) {
         return 0;
+    }
 
     if (size >= sizeof(s->rx_buffer)) {
         return 0;
@@ -115,10 +119,10 @@ static uint64_t mipsnet_ioport_read(void *opaque, hwaddr addr,
     addr &= 0x3f;
     switch (addr) {
     case MIPSNET_DEV_ID:
-        ret = be32_to_cpu(0x4d495053);		/* MIPS */
+        ret = be32_to_cpu(0x4d495053);          /* MIPS */
         break;
     case MIPSNET_DEV_ID + 4:
-        ret = be32_to_cpu(0x4e455430);		/* NET0 */
+        ret = be32_to_cpu(0x4e455430);          /* NET0 */
         break;
     case MIPSNET_BUSY:
         ret = s->busy;
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/5] mips: r4000: Renovate coding style
  2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
                   ` (2 preceding siblings ...)
  2019-11-25 13:04 ` [PATCH 3/5] mips: mipssim: " Filip Bozuta
@ 2019-11-25 13:04 ` Filip Bozuta
  2019-12-02  0:08   ` Aleksandar Markovic
  2019-11-25 13:04 ` [PATCH 5/5] mips: fulong 2e: " Filip Bozuta
  4 siblings, 1 reply; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in file:
    hw/mips/mips_r4k.c

This mips r4000 machine file was edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
---
 hw/mips/mips_r4k.c | 55 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
index 7002423..d638358 100644
--- a/hw/mips/mips_r4k.c
+++ b/hw/mips/mips_r4k.c
@@ -6,7 +6,7 @@
  * ISA memory at the 0x10000000 (PHYS, 16Mb in size).
  * All peripherial devices are attached to this "bus" with
  * the standard PC ISA addresses.
-*/
+ */
 
 #include "qemu/osdep.h"
 #include "qemu/units.h"
@@ -54,17 +54,18 @@ static struct _loaderparams {
     const char *initrd_filename;
 } loaderparams;
 
-static void mips_qemu_write (void *opaque, hwaddr addr,
-                             uint64_t val, unsigned size)
+static void mips_qemu_write(void *opaque, hwaddr addr,
+                            uint64_t val, unsigned size)
 {
-    if ((addr & 0xffff) == 0 && val == 42)
+    if ((addr & 0xffff) == 0 && val == 42) {
         qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
-    else if ((addr & 0xffff) == 4 && val == 42)
+    } else if ((addr & 0xffff) == 4 && val == 42) {
         qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+    }
 }
 
-static uint64_t mips_qemu_read (void *opaque, hwaddr addr,
-                                unsigned size)
+static uint64_t mips_qemu_read(void *opaque, hwaddr addr,
+                               unsigned size)
 {
     return 0;
 }
@@ -100,8 +101,9 @@ static int64_t load_kernel(void)
                            (uint64_t *)&kernel_high, big_endian,
                            EM_MIPS, 1, 0);
     if (kernel_size >= 0) {
-        if ((entry & ~0x7fffffffULL) == 0x80000000)
+        if ((entry & ~0x7fffffffULL) == 0x80000000) {
             entry = (int32_t)entry;
+        }
     } else {
         error_report("could not load kernel '%s': %s",
                      loaderparams.kernel_filename,
@@ -113,9 +115,10 @@ static int64_t load_kernel(void)
     initrd_size = 0;
     initrd_offset = 0;
     if (loaderparams.initrd_filename) {
-        initrd_size = get_image_size (loaderparams.initrd_filename);
+        initrd_size = get_image_size(loaderparams.initrd_filename);
         if (initrd_size > 0) {
-            initrd_offset = (kernel_high + ~INITRD_PAGE_MASK) & INITRD_PAGE_MASK;
+            initrd_offset = (kernel_high + ~INITRD_PAGE_MASK) &
+            INITRD_PAGE_MASK;
             if (initrd_offset + initrd_size > ram_size) {
                 error_report("memory too small for initial ram disk '%s'",
                              loaderparams.initrd_filename);
@@ -139,11 +142,13 @@ static int64_t load_kernel(void)
     params_buf[1] = tswap32(0x12345678);
 
     if (initrd_size > 0) {
-        snprintf((char *)params_buf + 8, 256, "rd_start=0x%" PRIx64 " rd_size=%" PRId64 " %s",
+        snprintf((char *)params_buf + 8, 256,
+                 "rd_start=0x%" PRIx64 " rd_size=%" PRId64 " %s",
                  cpu_mips_phys_to_kseg0(NULL, initrd_offset),
                  initrd_size, loaderparams.kernel_cmdline);
     } else {
-        snprintf((char *)params_buf + 8, 256, "%s", loaderparams.kernel_cmdline);
+        snprintf((char *)params_buf + 8, 256,
+        "%s", loaderparams.kernel_cmdline);
     }
 
     rom_add_blob_fixed("params", params_buf, params_size,
@@ -207,15 +212,21 @@ void mips_r4k_init(MachineState *machine)
 
     memory_region_add_subregion(address_space_mem, 0, ram);
 
-    memory_region_init_io(iomem, NULL, &mips_qemu_ops, NULL, "mips-qemu", 0x10000);
+    memory_region_init_io(iomem, NULL, &mips_qemu_ops,
+                          NULL, "mips-qemu", 0x10000);
+
     memory_region_add_subregion(address_space_mem, 0x1fbf0000, iomem);
 
-    /* Try to load a BIOS image. If this fails, we continue regardless,
-       but initialize the hardware ourselves. When a kernel gets
-       preloaded we also initialize the hardware, since the BIOS wasn't
-       run. */
-    if (bios_name == NULL)
+    /*
+     * Try to load a BIOS image. If this fails, we continue regardless,
+     * but initialize the hardware ourselves. When a kernel gets
+     * preloaded we also initialize the hardware, since the BIOS wasn't
+     * run.
+     */
+
+    if (bios_name == NULL) {
         bios_name = BIOS_FILENAME;
+    }
     filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
     if (filename) {
         bios_size = get_image_size(filename);
@@ -227,6 +238,7 @@ void mips_r4k_init(MachineState *machine)
 #else
     be = 0;
 #endif
+    dinfo = drive_get(IF_PFLASH, 0, 0);
     if ((bios_size > 0) && (bios_size <= BIOS_SIZE)) {
         bios = g_new(MemoryRegion, 1);
         memory_region_init_ram(bios, NULL, "mips_r4k.bios", BIOS_SIZE,
@@ -235,7 +247,7 @@ void mips_r4k_init(MachineState *machine)
         memory_region_add_subregion(get_system_memory(), 0x1fc00000, bios);
 
         load_image_targphys(filename, 0x1fc00000, BIOS_SIZE);
-    } else if ((dinfo = drive_get(IF_PFLASH, 0, 0)) != NULL) {
+    } else if (dinfo != NULL) {
         uint32_t mips_rom = 0x00400000;
         if (!pflash_cfi01_register(0x1fc00000, "mips_r4k.bios", mips_rom,
                                    blk_by_legacy_dinfo(dinfo),
@@ -280,11 +292,12 @@ void mips_r4k_init(MachineState *machine)
 
     isa_vga_init(isa_bus);
 
-    if (nd_table[0].used)
+    if (nd_table[0].used) {
         isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]);
+    }
 
     ide_drive_get(hd, ARRAY_SIZE(hd));
-    for(i = 0; i < MAX_IDE_BUS; i++)
+    for (i = 0; i < MAX_IDE_BUS; i++)
         isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i],
                      hd[MAX_IDE_DEVS * i],
                      hd[MAX_IDE_DEVS * i + 1]);
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/5] mips: fulong 2e: Renovate coding style
  2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
                   ` (3 preceding siblings ...)
  2019-11-25 13:04 ` [PATCH 4/5] mips: r4000: " Filip Bozuta
@ 2019-11-25 13:04 ` Filip Bozuta
  2019-12-01 23:49   ` Aleksandar Markovic
  4 siblings, 1 reply; 18+ messages in thread
From: Filip Bozuta @ 2019-11-25 13:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in files:
    hw/mips/mips_fulong2e.c
    hw/isa/vt82c686.c
    hw/pci-host/bonito.c
    include/hw/isa/vt82c686.h

These mips Fulong 2E machine files were edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
---
 hw/isa/vt82c686.c    | 23 ++++++++++----------
 hw/pci-host/bonito.c | 60 +++++++++++++++++++++++++++++-----------------------
 2 files changed, 45 insertions(+), 38 deletions(-)

diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index 616f67f..f828708 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -27,7 +27,7 @@
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
 
-//#define DEBUG_VT82C686B
+/* #define DEBUG_VT82C686B */
 
 #ifdef DEBUG_VT82C686B
 #define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
@@ -35,8 +35,7 @@
 #define DPRINTF(fmt, ...)
 #endif
 
-typedef struct SuperIOConfig
-{
+typedef struct SuperIOConfig {
     uint8_t config[0x100];
     uint8_t index;
     uint8_t data;
@@ -102,7 +101,7 @@ static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
     SuperIOConfig *superio_conf = opaque;
 
     DPRINTF("superio_ioport_readb  address 0x%x\n", addr);
-    return (superio_conf->config[superio_conf->index]);
+    return superio_conf->config[superio_conf->index];
 }
 
 static const MemoryRegionOps superio_ops = {
@@ -143,7 +142,7 @@ static void vt82c686b_isa_reset(DeviceState *dev)
 }
 
 /* write config pci function0 registers. PCI-ISA bridge */
-static void vt82c686b_write_config(PCIDevice * d, uint32_t address,
+static void vt82c686b_write_config(PCIDevice *d, uint32_t address,
                                    uint32_t val, int len)
 {
     VT82C686BState *vt686 = VT82C686B_DEVICE(d);
@@ -365,7 +364,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error **errp)
     pci_set_long(pci_conf + 0x48, 0x00000001);
 
     /* SMB ports:0xeee0~0xeeef */
-    s->smb_io_base =((s->smb_io_base & 0xfff0) + 0x0);
+    s->smb_io_base = ((s->smb_io_base & 0xfff0) + 0x0);
     pci_conf[0x90] = s->smb_io_base | 1;
     pci_conf[0x91] = s->smb_io_base >> 8;
     pci_conf[0xd2] = 0x90;
@@ -462,16 +461,18 @@ static void vt82c686b_realize(PCIDevice *d, Error **errp)
 
     wmask = d->wmask;
     for (i = 0x00; i < 0xff; i++) {
-       if (i<=0x03 || (i>=0x08 && i<=0x3f)) {
-           wmask[i] = 0x00;
-       }
+        if (i <= 0x03 || (i >= 0x08 && i <= 0x3f)) {
+            wmask[i] = 0x00;
+        }
     }
 
     memory_region_init_io(&vt82c->superio, OBJECT(d), &superio_ops,
                           &vt82c->superio_conf, "superio", 2);
     memory_region_set_enabled(&vt82c->superio, false);
-    /* The floppy also uses 0x3f0 and 0x3f1.
-     * But we do not emulate a floppy, so just set it here. */
+    /*
+     * The floppy also uses 0x3f0 and 0x3f1.
+     * But we do not emulate a floppy, so just set it here.
+     */
     memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
                                 &vt82c->superio);
 }
diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
index ceee463..4692d41 100644
--- a/hw/pci-host/bonito.c
+++ b/hw/pci-host/bonito.c
@@ -14,7 +14,8 @@
  * fulong 2e mini pc has a bonito north bridge.
  */
 
-/* what is the meaning of devfn in qemu and IDSEL in bonito northbridge?
+/*
+ * what is the meaning of devfn in qemu and IDSEL in bonito northbridge?
  *
  * devfn   pci_slot<<3  + funno
  * one pci bus can have 32 devices and each device can have 8 functions.
@@ -49,7 +50,7 @@
 #include "sysemu/runstate.h"
 #include "exec/address-spaces.h"
 
-//#define DEBUG_BONITO
+/* #define DEBUG_BONITO */
 
 #ifdef DEBUG_BONITO
 #define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
@@ -60,45 +61,45 @@
 /* from linux soure code. include/asm-mips/mips-boards/bonito64.h*/
 #define BONITO_BOOT_BASE        0x1fc00000
 #define BONITO_BOOT_SIZE        0x00100000
-#define BONITO_BOOT_TOP         (BONITO_BOOT_BASE+BONITO_BOOT_SIZE-1)
+#define BONITO_BOOT_TOP         (BONITO_BOOT_BASE + BONITO_BOOT_SIZE - 1)
 #define BONITO_FLASH_BASE       0x1c000000
 #define BONITO_FLASH_SIZE       0x03000000
-#define BONITO_FLASH_TOP        (BONITO_FLASH_BASE+BONITO_FLASH_SIZE-1)
+#define BONITO_FLASH_TOP        (BONITO_FLASH_BASE + BONITO_FLASH_SIZE - 1)
 #define BONITO_SOCKET_BASE      0x1f800000
 #define BONITO_SOCKET_SIZE      0x00400000
-#define BONITO_SOCKET_TOP       (BONITO_SOCKET_BASE+BONITO_SOCKET_SIZE-1)
+#define BONITO_SOCKET_TOP       (BONITO_SOCKET_BASE + BONITO_SOCKET_SIZE - 1)
 #define BONITO_REG_BASE         0x1fe00000
 #define BONITO_REG_SIZE         0x00040000
-#define BONITO_REG_TOP          (BONITO_REG_BASE+BONITO_REG_SIZE-1)
+#define BONITO_REG_TOP          (BONITO_REG_BASE + BONITO_REG_SIZE - 1)
 #define BONITO_DEV_BASE         0x1ff00000
 #define BONITO_DEV_SIZE         0x00100000
-#define BONITO_DEV_TOP          (BONITO_DEV_BASE+BONITO_DEV_SIZE-1)
+#define BONITO_DEV_TOP          (BONITO_DEV_BASE + BONITO_DEV_SIZE - 1)
 #define BONITO_PCILO_BASE       0x10000000
 #define BONITO_PCILO_BASE_VA    0xb0000000
 #define BONITO_PCILO_SIZE       0x0c000000
-#define BONITO_PCILO_TOP        (BONITO_PCILO_BASE+BONITO_PCILO_SIZE-1)
+#define BONITO_PCILO_TOP        (BONITO_PCILO_BASE + BONITO_PCILO_SIZE - 1)
 #define BONITO_PCILO0_BASE      0x10000000
 #define BONITO_PCILO1_BASE      0x14000000
 #define BONITO_PCILO2_BASE      0x18000000
 #define BONITO_PCIHI_BASE       0x20000000
 #define BONITO_PCIHI_SIZE       0x20000000
-#define BONITO_PCIHI_TOP        (BONITO_PCIHI_BASE+BONITO_PCIHI_SIZE-1)
+#define BONITO_PCIHI_TOP        (BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE - 1)
 #define BONITO_PCIIO_BASE       0x1fd00000
 #define BONITO_PCIIO_BASE_VA    0xbfd00000
 #define BONITO_PCIIO_SIZE       0x00010000
-#define BONITO_PCIIO_TOP        (BONITO_PCIIO_BASE+BONITO_PCIIO_SIZE-1)
+#define BONITO_PCIIO_TOP        (BONITO_PCIIO_BASE + BONITO_PCIIO_SIZE - 1)
 #define BONITO_PCICFG_BASE      0x1fe80000
 #define BONITO_PCICFG_SIZE      0x00080000
-#define BONITO_PCICFG_TOP       (BONITO_PCICFG_BASE+BONITO_PCICFG_SIZE-1)
+#define BONITO_PCICFG_TOP       (BONITO_PCICFG_BASE + BONITO_PCICFG_SIZE - 1)
 
 
 #define BONITO_PCICONFIGBASE    0x00
 #define BONITO_REGBASE          0x100
 
-#define BONITO_PCICONFIG_BASE   (BONITO_PCICONFIGBASE+BONITO_REG_BASE)
+#define BONITO_PCICONFIG_BASE   (BONITO_PCICONFIGBASE + BONITO_REG_BASE)
 #define BONITO_PCICONFIG_SIZE   (0x100)
 
-#define BONITO_INTERNAL_REG_BASE  (BONITO_REGBASE+BONITO_REG_BASE)
+#define BONITO_INTERNAL_REG_BASE  (BONITO_REGBASE + BONITO_REG_BASE)
 #define BONITO_INTERNAL_REG_SIZE  (0x70)
 
 #define BONITO_SPCICONFIG_BASE  (BONITO_PCICFG_BASE)
@@ -111,7 +112,7 @@
 
 #define BONITO_BONPONCFG        (0x00 >> 2)      /* 0x100 */
 #define BONITO_BONGENCFG_OFFSET 0x4
-#define BONITO_BONGENCFG        (BONITO_BONGENCFG_OFFSET>>2)   /*0x104 */
+#define BONITO_BONGENCFG        (BONITO_BONGENCFG_OFFSET >> 2)   /*0x104 */
 
 /* 2. IO & IDE configuration */
 #define BONITO_IODEVCFG         (0x08 >> 2)      /* 0x108 */
@@ -177,15 +178,15 @@
 /* idsel BIT = pci slot number +12 */
 #define PCI_SLOT_BASE              12
 #define PCI_IDSEL_VIA686B_BIT      (17)
-#define PCI_IDSEL_VIA686B          (1<<PCI_IDSEL_VIA686B_BIT)
+#define PCI_IDSEL_VIA686B          (1 << PCI_IDSEL_VIA686B_BIT)
 
-#define PCI_ADDR(busno,devno,funno,regno)  \
-    ((((busno)<<16)&0xff0000) + (((devno)<<11)&0xf800) + (((funno)<<8)&0x700) + (regno))
+#define PCI_ADDR(busno , devno , funno , regno)  \
+    ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \
+    (((funno) << 8) & 0x700) + (regno))
 
 typedef struct BonitoState BonitoState;
 
-typedef struct PCIBonitoState
-{
+typedef struct PCIBonitoState {
     PCIDevice dev;
 
     BonitoState *pcihost;
@@ -239,7 +240,8 @@ static void bonito_writel(void *opaque, hwaddr addr,
 
     saddr = addr >> 2;
 
-    DPRINTF("bonito_writel "TARGET_FMT_plx" val %x saddr %x\n", addr, val, saddr);
+    DPRINTF("bonito_writel "TARGET_FMT_plx" val %x saddr %x\n",
+            addr, val, saddr);
     switch (saddr) {
     case BONITO_BONPONCFG:
     case BONITO_IODEVCFG:
@@ -363,7 +365,7 @@ static uint64_t bonito_ldma_readl(void *opaque, hwaddr addr,
         return 0;
     }
 
-    val = ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)];
+    val = ((uint32_t *)(&s->bonldma))[addr / sizeof(uint32_t)];
 
     return val;
 }
@@ -377,7 +379,7 @@ static void bonito_ldma_writel(void *opaque, hwaddr addr,
         return;
     }
 
-    ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)] = val & 0xffffffff;
+    ((uint32_t *)(&s->bonldma))[addr / sizeof(uint32_t)] = val & 0xffffffff;
 }
 
 static const MemoryRegionOps bonito_ldma_ops = {
@@ -400,7 +402,7 @@ static uint64_t bonito_cop_readl(void *opaque, hwaddr addr,
         return 0;
     }
 
-    val = ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)];
+    val = ((uint32_t *)(&s->boncop))[addr / sizeof(uint32_t)];
 
     return val;
 }
@@ -414,7 +416,7 @@ static void bonito_cop_writel(void *opaque, hwaddr addr,
         return;
     }
 
-    ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)] = val & 0xffffffff;
+    ((uint32_t *)(&s->boncop))[addr / sizeof(uint32_t)] = val & 0xffffffff;
 }
 
 static const MemoryRegionOps bonito_cop_ops = {
@@ -446,7 +448,8 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque, hwaddr addr)
     cfgaddr = addr & 0xffff;
     cfgaddr |= (s->regs[BONITO_PCIMAP_CFG] & 0xffff) << 16;
 
-    idsel = (cfgaddr & BONITO_PCICONF_IDSEL_MASK) >> BONITO_PCICONF_IDSEL_OFFSET;
+    idsel = (cfgaddr & BONITO_PCICONF_IDSEL_MASK) >>
+             BONITO_PCICONF_IDSEL_OFFSET;
     devno = ctz32(idsel);
     funno = (cfgaddr & BONITO_PCICONF_FUN_MASK) >> BONITO_PCICONF_FUN_OFFSET;
     regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >> BONITO_PCICONF_REG_OFFSET;
@@ -550,7 +553,7 @@ static void pci_bonito_set_irq(void *opaque, int irq_num, int level)
 }
 
 /* map the original irq (0~3) to bonito irq (16~47, but 16~31 are unused) */
-static int pci_bonito_map_irq(PCIDevice * pci_dev, int irq_num)
+static int pci_bonito_map_irq(PCIDevice *pci_dev, int irq_num)
 {
     int slot;
 
@@ -618,7 +621,10 @@ static void bonito_realize(PCIDevice *dev, Error **errp)
     SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
     PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
 
-    /* Bonito North Bridge, built on FPGA, VENDOR_ID/DEVICE_ID are "undefined" */
+    /*
+     * Bonito North Bridge, built on FPGA,
+     * VENDOR_ID/DEVICE_ID are "undefined"
+     */
     pci_config_set_prog_interface(dev->config, 0x00);
 
     /* set the north bridge register mapping */
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
@ 2019-11-30 23:46   ` Aleksandar Markovic
  2019-12-02 20:49     ` Eduardo Habkost
  2019-12-01  0:00   ` Aleksandar Markovic
  2019-12-01 20:20   ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 18+ messages in thread
From: Aleksandar Markovic @ 2019-11-30 23:46 UTC (permalink / raw)
  To: Filip Bozuta, Cleber Rosa, Eduardo Habkost
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 2032 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>     hw/mips/mips_malta.c
>     hw/mips/gt64xxx_pci.c
>     tests/acceptance/linux_ssh_mips_malta.py
>
> All these mips malta machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/mips/mips_malta.c                     | 139
> ++++++++++++++++---------------
>  tests/acceptance/linux_ssh_mips_malta.py |   6 +-
>  2 files changed, 75 insertions(+), 70 deletions(-)
>
>
Very good cleanup!

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>

I think this snippet is good, but I am just in case cc-ing Cleber and
Eduardo to check if it is in accordance with any applicable guidelines of
our test framework:


> diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e..44e1118 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -99,10 +99,12 @@ class LinuxSSH(Test):
>      def ssh_command(self, command, is_root=True):
>          self.ssh_logger.info(command)
>          result = self.ssh_session.cmd(command)
> -        stdout_lines = [line.rstrip() for line in
> result.stdout_text.splitlines()]
> +        stdout_lines = [line.rstrip() for line
> +        in result.stdout_text.splitlines()]
>          for line in stdout_lines:
>              self.ssh_logger.info(line)
> -        stderr_lines = [line.rstrip() for line in
> result.stderr_text.splitlines()]
> +        stderr_lines = [line.rstrip() for line
> +        in result.stderr_text.splitlines()]
>          for line in stderr_lines:
>              self.ssh_logger.warning(line)
>          return stdout_lines, stderr_lines
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 3393 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
  2019-11-30 23:46   ` Aleksandar Markovic
@ 2019-12-01  0:00   ` Aleksandar Markovic
  2019-12-01 20:20   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-01  0:00 UTC (permalink / raw)
  To: Filip Bozuta
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 14371 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>     hw/mips/mips_malta.c
>     hw/mips/gt64xxx_pci.c
>     tests/acceptance/linux_ssh_mips_malta.py
>
> All these mips malta machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/mips/mips_malta.c                     | 139
> ++++++++++++++++---------------
>  tests/acceptance/linux_ssh_mips_malta.py |   6 +-
>  2 files changed, 75 insertions(+), 70 deletions(-)
>
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 92e9ca5..18eafac 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
>   */
>
>  #if defined(DEBUG)
> -#  define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__,
> ## __VA_ARGS__)
> +#  define logout(fmt, ...) \
> +          fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
>  #else
>  #  define logout(fmt, ...) ((void)0)
>  #endif
> @@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion
> *address_space,
>      MaltaFPGAState *s;
>      Chardev *chr;
>
> -    s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
> +    s = (MaltaFPGAState *)g_new0(sizeof(MaltaFPGAState));
>
>      memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
>                            "malta-fpga", 0x100000);
> @@ -844,9 +845,9 @@ static void write_bootloader(uint8_t *base, int64_t
> run_addr,
>      /* Small bootloader */
>      p = (uint32_t *)base;
>
> -    stl_p(p++, 0x08000000 |                                      /* j
> 0x1fc00580 */
> +    stl_p(p++, 0x08000000 |                                   /* j
> 0x1fc00580 */
>                   ((run_addr + 0x580) & 0x0fffffff) >> 2);
> -    stl_p(p++, 0x00000000);                                      /* nop */
> +    stl_p(p++, 0x00000000);                                   /* nop */
>
>      /* YAMON service vector */
>      stl_p(base + 0x500, run_addr + 0x0580);      /* start: */
> @@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base,
> int64_t run_addr,
>      stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
>
>      /* Load BAR registers as done by YAMON */
> -    stl_p(p++, 0x3c09b400);                                      /* lui
> t1, 0xb400 */
> +    stl_p(p++, 0x3c09b400);                 /* lui t1, 0xb400 */
>
>
Sorry, Filip,  just an additional thing: the first several comments in the
new arrangement don't begin at the same line as the rest of comments. But I
can fix it while applying, no need for you to send v2 beacause of this.

Thanks,
Aleksandar



>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08df00);                                      /* lui
> t0, 0xdf00 */
> +    stl_p(p++, 0x3c08df00);                  /* lui t0, 0xdf00 */
>  #else
> -    stl_p(p++, 0x340800df);                                      /* ori
> t0, r0, 0x00df */
> +    stl_p(p++, 0x340800df);                  /* ori t0, r0, 0x00df */
>  #endif
> -    stl_p(p++, 0xad280068);                                      /* sw
> t0, 0x0068(t1) */
> +    stl_p(p++, 0xad280068);                  /* sw t0, 0x0068(t1) */
>
> -    stl_p(p++, 0x3c09bbe0);                                      /* lui
> t1, 0xbbe0 */
> +    stl_p(p++, 0x3c09bbe0);                  /* lui t1, 0xbbe0 */
>
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08c000);                                      /* lui
> t0, 0xc000 */
> +    stl_p(p++, 0x3c08c000);                  /* lui t0, 0xc000 */
>  #else
> -    stl_p(p++, 0x340800c0);                                      /* ori
> t0, r0, 0x00c0 */
> +    stl_p(p++, 0x340800c0);                  /* ori t0, r0, 0x00c0 */
>  #endif
> -    stl_p(p++, 0xad280048);                                      /* sw
> t0, 0x0048(t1) */
> +    stl_p(p++, 0xad280048);                  /* sw t0, 0x0048(t1) */
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c084000);                                      /* lui
> t0, 0x4000 */
> +    stl_p(p++, 0x3c084000);                  /* lui t0, 0x4000 */
>  #else
> -    stl_p(p++, 0x34080040);                                      /* ori
> t0, r0, 0x0040 */
> +    stl_p(p++, 0x34080040);                  /* ori t0, r0, 0x0040 */
>  #endif
> -    stl_p(p++, 0xad280050);                                      /* sw
> t0, 0x0050(t1) */
> +    stl_p(p++, 0xad280050);                  /* sw t0, 0x0050(t1) */
>
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c088000);                                      /* lui
> t0, 0x8000 */
> +    stl_p(p++, 0x3c088000);                  /* lui t0, 0x8000 */
>  #else
> -    stl_p(p++, 0x34080080);                                      /* ori
> t0, r0, 0x0080 */
> +    stl_p(p++, 0x34080080);                  /* ori t0, r0, 0x0080 */
>  #endif
> -    stl_p(p++, 0xad280058);                                      /* sw
> t0, 0x0058(t1) */
> +    stl_p(p++, 0xad280058);                  /* sw t0, 0x0058(t1) */
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c083f00);                                      /* lui
> t0, 0x3f00 */
> +    stl_p(p++, 0x3c083f00);                  /* lui t0, 0x3f00 */
>  #else
> -    stl_p(p++, 0x3408003f);                                      /* ori
> t0, r0, 0x003f */
> +    stl_p(p++, 0x3408003f);                  /* ori t0, r0, 0x003f */
>  #endif
> -    stl_p(p++, 0xad280060);                                      /* sw
> t0, 0x0060(t1) */
> +    stl_p(p++, 0xad280060);                  /* sw t0, 0x0060(t1) */
>
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08c100);                                      /* lui
> t0, 0xc100 */
> +    stl_p(p++, 0x3c08c100);                  /* lui t0, 0xc100 */
>  #else
> -    stl_p(p++, 0x340800c1);                                      /* ori
> t0, r0, 0x00c1 */
> +    stl_p(p++, 0x340800c1);                  /* ori t0, r0, 0x00c1 */
>  #endif
> -    stl_p(p++, 0xad280080);                                      /* sw
> t0, 0x0080(t1) */
> +    stl_p(p++, 0xad280080);                  /* sw t0, 0x0080(t1) */
>  #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c085e00);                                      /* lui
> t0, 0x5e00 */
> +    stl_p(p++, 0x3c085e00);                  /* lui t0, 0x5e00 */
>  #else
> -    stl_p(p++, 0x3408005e);                                      /* ori
> t0, r0, 0x005e */
> +    stl_p(p++, 0x3408005e);                  /* ori t0, r0, 0x005e */
>  #endif
> -    stl_p(p++, 0xad280088);                                      /* sw
> t0, 0x0088(t1) */
> +    stl_p(p++, 0xad280088);                  /* sw t0, 0x0088(t1) */
>
>      /* Jump to kernel code */
> -    stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff));    /* lui
> ra, high(kernel_entry) */
> -    stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff));            /* ori
> ra, ra, low(kernel_entry) */
> -    stl_p(p++, 0x03e00009);                                      /* jalr
> ra */
> -    stl_p(p++, 0x00000000);                                      /* nop */
> +    stl_p(p++, 0x3c1f0000 |
> +          ((kernel_entry >> 16) & 0xffff));  /* lui ra,
> high(kernel_entry) */
> +    stl_p(p++, 0x37ff0000 |
> +          (kernel_entry & 0xffff));          /* ori ra, ra,
> low(kernel_entry) */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0x00000000);                  /* nop */
>
>      /* YAMON subroutines */
>      p = (uint32_t *) (base + 0x800);
> -    stl_p(p++, 0x03e00009);                                     /* jalr
> ra */
> -    stl_p(p++, 0x24020000);                                     /* li
> v0,0 */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0x24020000);                  /* li v0,0 */
>      /* 808 YAMON print */
> -    stl_p(p++, 0x03e06821);                                     /* move
> t5,ra */
> -    stl_p(p++, 0x00805821);                                     /* move
> t3,a0 */
> -    stl_p(p++, 0x00a05021);                                     /* move
> t2,a1 */
> -    stl_p(p++, 0x91440000);                                     /* lbu
> a0,0(t2) */
> -    stl_p(p++, 0x254a0001);                                     /* addiu
> t2,t2,1 */
> -    stl_p(p++, 0x10800005);                                     /* beqz
> a0,834 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x0ff0021c);                                     /* jal
> 870 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x1000fff9);                                     /* b 814
> */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x01a00009);                                     /* jalr
> t5 */
> -    stl_p(p++, 0x01602021);                                     /* move
> a0,t3 */
> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
> +    stl_p(p++, 0x10800005);                  /* beqz a0,834 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x1000fff9);                  /* b 814 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>      /* 0x83c YAMON print_count */
> -    stl_p(p++, 0x03e06821);                                     /* move
> t5,ra */
> -    stl_p(p++, 0x00805821);                                     /* move
> t3,a0 */
> -    stl_p(p++, 0x00a05021);                                     /* move
> t2,a1 */
> -    stl_p(p++, 0x00c06021);                                     /* move
> t4,a2 */
> -    stl_p(p++, 0x91440000);                                     /* lbu
> a0,0(t2) */
> -    stl_p(p++, 0x0ff0021c);                                     /* jal
> 870 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x254a0001);                                     /* addiu
> t2,t2,1 */
> -    stl_p(p++, 0x258cffff);                                     /* addiu
> t4,t4,-1 */
> -    stl_p(p++, 0x1580fffa);                                     /* bnez
> t4,84c */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x01a00009);                                     /* jalr
> t5 */
> -    stl_p(p++, 0x01602021);                                     /* move
> a0,t3 */
> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
> +    stl_p(p++, 0x00c06021);                  /* move t4,a2 */
> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
> +    stl_p(p++, 0x258cffff);                  /* addiu t4,t4,-1 */
> +    stl_p(p++, 0x1580fffa);                  /* bnez t4,84c */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>      /* 0x870 */
> -    stl_p(p++, 0x3c08b800);                                     /* lui
> t0,0xb400 */
> -    stl_p(p++, 0x350803f8);                                     /* ori
> t0,t0,0x3f8 */
> -    stl_p(p++, 0x91090005);                                     /* lbu
> t1,5(t0) */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x31290040);                                     /* andi
> t1,t1,0x40 */
> -    stl_p(p++, 0x1120fffc);                                     /* beqz
> t1,878 <outch+0x8> */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x03e00009);                                     /* jalr
> ra */
> -    stl_p(p++, 0xa1040000);                                     /* sb
> a0,0(t0) */
> +    stl_p(p++, 0x3c08b800);                  /* lui t0,0xb400 */
> +    stl_p(p++, 0x350803f8);                  /* ori t0,t0,0x3f8 */
> +    stl_p(p++, 0x91090005);                  /* lbu t1,5(t0) */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x31290040);                  /* andi t1,t1,0x40 */
> +    stl_p(p++, 0x1120fffc);                  /* beqz t1,878 <outch+0x8> */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0xa1040000);                  /* sb a0,0(t0) */
>
>  }
>
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e..44e1118 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -99,10 +99,12 @@ class LinuxSSH(Test):
>      def ssh_command(self, command, is_root=True):
>          self.ssh_logger.info(command)
>          result = self.ssh_session.cmd(command)
> -        stdout_lines = [line.rstrip() for line in
> result.stdout_text.splitlines()]
> +        stdout_lines = [line.rstrip() for line
> +        in result.stdout_text.splitlines()]
>          for line in stdout_lines:
>              self.ssh_logger.info(line)
> -        stderr_lines = [line.rstrip() for line in
> result.stderr_text.splitlines()]
> +        stderr_lines = [line.rstrip() for line
> +        in result.stderr_text.splitlines()]
>          for line in stderr_lines:
>              self.ssh_logger.warning(line)
>          return stdout_lines, stderr_lines
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 18100 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
  2019-11-30 23:46   ` Aleksandar Markovic
  2019-12-01  0:00   ` Aleksandar Markovic
@ 2019-12-01 20:20   ` Philippe Mathieu-Daudé
  2019-12-06  8:21     ` Aleksandar Markovic
  2 siblings, 1 reply; 18+ messages in thread
From: Philippe Mathieu-Daudé @ 2019-12-01 20:20 UTC (permalink / raw)
  To: Filip Bozuta, qemu-devel
  Cc: pburton, aleksandar.rikalo, hpoussin, aurelien, amarkovic

Hi Filip,

On 11/25/19 2:04 PM, Filip Bozuta wrote:
> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>      hw/mips/mips_malta.c
>      hw/mips/gt64xxx_pci.c
>      tests/acceptance/linux_ssh_mips_malta.py
> 
> All these mips malta machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
> 
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>   hw/mips/mips_malta.c                     | 139 ++++++++++++++++---------------
>   tests/acceptance/linux_ssh_mips_malta.py |   6 +-
>   2 files changed, 75 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
> index 92e9ca5..18eafac 100644
> --- a/hw/mips/mips_malta.c
> +++ b/hw/mips/mips_malta.c
> @@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
>    */
>   
>   #if defined(DEBUG)
> -#  define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
> +#  define logout(fmt, ...) \
> +          fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)

This is deprecated code, if you look in the repository history, we 
usually don't touch it, except to get rid of it. The way to get rid of 
it is to replace the calls by trace events.

>   #else
>   #  define logout(fmt, ...) ((void)0)
>   #endif
> @@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion *address_space,
>       MaltaFPGAState *s;
>       Chardev *chr;
>   
> -    s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
> +    s = (MaltaFPGAState *)g_new0(sizeof(MaltaFPGAState));

This change doesn't even compile... Please compile your patches before 
posting them to the list.

You can find the prototype of this macro here:
https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new0

>   
>       memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
>                             "malta-fpga", 0x100000);
> @@ -844,9 +845,9 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
>       /* Small bootloader */
>       p = (uint32_t *)base;
>   
> -    stl_p(p++, 0x08000000 |                                      /* j 0x1fc00580 */
> +    stl_p(p++, 0x08000000 |                                   /* j 0x1fc00580 */
>                    ((run_addr + 0x580) & 0x0fffffff) >> 2);
> -    stl_p(p++, 0x00000000);                                      /* nop */
> +    stl_p(p++, 0x00000000);                                   /* nop */
>   
>       /* YAMON service vector */
>       stl_p(base + 0x500, run_addr + 0x0580);      /* start: */
> @@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
>       stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
>   
>       /* Load BAR registers as done by YAMON */
> -    stl_p(p++, 0x3c09b400);                                      /* lui t1, 0xb400 */
> +    stl_p(p++, 0x3c09b400);                 /* lui t1, 0xb400 */
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08df00);                                      /* lui t0, 0xdf00 */
> +    stl_p(p++, 0x3c08df00);                  /* lui t0, 0xdf00 */
>   #else
> -    stl_p(p++, 0x340800df);                                      /* ori t0, r0, 0x00df */
> +    stl_p(p++, 0x340800df);                  /* ori t0, r0, 0x00df */
>   #endif
> -    stl_p(p++, 0xad280068);                                      /* sw t0, 0x0068(t1) */
> +    stl_p(p++, 0xad280068);                  /* sw t0, 0x0068(t1) */
>   
> -    stl_p(p++, 0x3c09bbe0);                                      /* lui t1, 0xbbe0 */
> +    stl_p(p++, 0x3c09bbe0);                  /* lui t1, 0xbbe0 */
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08c000);                                      /* lui t0, 0xc000 */
> +    stl_p(p++, 0x3c08c000);                  /* lui t0, 0xc000 */
>   #else
> -    stl_p(p++, 0x340800c0);                                      /* ori t0, r0, 0x00c0 */
> +    stl_p(p++, 0x340800c0);                  /* ori t0, r0, 0x00c0 */
>   #endif
> -    stl_p(p++, 0xad280048);                                      /* sw t0, 0x0048(t1) */
> +    stl_p(p++, 0xad280048);                  /* sw t0, 0x0048(t1) */
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c084000);                                      /* lui t0, 0x4000 */
> +    stl_p(p++, 0x3c084000);                  /* lui t0, 0x4000 */
>   #else
> -    stl_p(p++, 0x34080040);                                      /* ori t0, r0, 0x0040 */
> +    stl_p(p++, 0x34080040);                  /* ori t0, r0, 0x0040 */
>   #endif
> -    stl_p(p++, 0xad280050);                                      /* sw t0, 0x0050(t1) */
> +    stl_p(p++, 0xad280050);                  /* sw t0, 0x0050(t1) */
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c088000);                                      /* lui t0, 0x8000 */
> +    stl_p(p++, 0x3c088000);                  /* lui t0, 0x8000 */
>   #else
> -    stl_p(p++, 0x34080080);                                      /* ori t0, r0, 0x0080 */
> +    stl_p(p++, 0x34080080);                  /* ori t0, r0, 0x0080 */
>   #endif
> -    stl_p(p++, 0xad280058);                                      /* sw t0, 0x0058(t1) */
> +    stl_p(p++, 0xad280058);                  /* sw t0, 0x0058(t1) */
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c083f00);                                      /* lui t0, 0x3f00 */
> +    stl_p(p++, 0x3c083f00);                  /* lui t0, 0x3f00 */
>   #else
> -    stl_p(p++, 0x3408003f);                                      /* ori t0, r0, 0x003f */
> +    stl_p(p++, 0x3408003f);                  /* ori t0, r0, 0x003f */
>   #endif
> -    stl_p(p++, 0xad280060);                                      /* sw t0, 0x0060(t1) */
> +    stl_p(p++, 0xad280060);                  /* sw t0, 0x0060(t1) */
>   
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c08c100);                                      /* lui t0, 0xc100 */
> +    stl_p(p++, 0x3c08c100);                  /* lui t0, 0xc100 */
>   #else
> -    stl_p(p++, 0x340800c1);                                      /* ori t0, r0, 0x00c1 */
> +    stl_p(p++, 0x340800c1);                  /* ori t0, r0, 0x00c1 */
>   #endif
> -    stl_p(p++, 0xad280080);                                      /* sw t0, 0x0080(t1) */
> +    stl_p(p++, 0xad280080);                  /* sw t0, 0x0080(t1) */
>   #ifdef TARGET_WORDS_BIGENDIAN
> -    stl_p(p++, 0x3c085e00);                                      /* lui t0, 0x5e00 */
> +    stl_p(p++, 0x3c085e00);                  /* lui t0, 0x5e00 */
>   #else
> -    stl_p(p++, 0x3408005e);                                      /* ori t0, r0, 0x005e */
> +    stl_p(p++, 0x3408005e);                  /* ori t0, r0, 0x005e */
>   #endif
> -    stl_p(p++, 0xad280088);                                      /* sw t0, 0x0088(t1) */
> +    stl_p(p++, 0xad280088);                  /* sw t0, 0x0088(t1) */
>   
>       /* Jump to kernel code */
> -    stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff));    /* lui ra, high(kernel_entry) */
> -    stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff));            /* ori ra, ra, low(kernel_entry) */
> -    stl_p(p++, 0x03e00009);                                      /* jalr ra */
> -    stl_p(p++, 0x00000000);                                      /* nop */
> +    stl_p(p++, 0x3c1f0000 |
> +          ((kernel_entry >> 16) & 0xffff));  /* lui ra, high(kernel_entry) */
> +    stl_p(p++, 0x37ff0000 |
> +          (kernel_entry & 0xffff));          /* ori ra, ra, low(kernel_entry) */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0x00000000);                  /* nop */
>   
>       /* YAMON subroutines */
>       p = (uint32_t *) (base + 0x800);
> -    stl_p(p++, 0x03e00009);                                     /* jalr ra */
> -    stl_p(p++, 0x24020000);                                     /* li v0,0 */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0x24020000);                  /* li v0,0 */
>       /* 808 YAMON print */
> -    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
> -    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
> -    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
> -    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
> -    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
> -    stl_p(p++, 0x10800005);                                     /* beqz a0,834 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x1000fff9);                                     /* b 814 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
> -    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
> +    stl_p(p++, 0x10800005);                  /* beqz a0,834 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x1000fff9);                  /* b 814 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>       /* 0x83c YAMON print_count */
> -    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
> -    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
> -    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
> -    stl_p(p++, 0x00c06021);                                     /* move t4,a2 */
> -    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
> -    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
> -    stl_p(p++, 0x258cffff);                                     /* addiu t4,t4,-1 */
> -    stl_p(p++, 0x1580fffa);                                     /* bnez t4,84c */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
> -    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
> +    stl_p(p++, 0x00c06021);                  /* move t4,a2 */
> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
> +    stl_p(p++, 0x258cffff);                  /* addiu t4,t4,-1 */
> +    stl_p(p++, 0x1580fffa);                  /* bnez t4,84c */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>       /* 0x870 */
> -    stl_p(p++, 0x3c08b800);                                     /* lui t0,0xb400 */
> -    stl_p(p++, 0x350803f8);                                     /* ori t0,t0,0x3f8 */
> -    stl_p(p++, 0x91090005);                                     /* lbu t1,5(t0) */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x31290040);                                     /* andi t1,t1,0x40 */
> -    stl_p(p++, 0x1120fffc);                                     /* beqz t1,878 <outch+0x8> */
> -    stl_p(p++, 0x00000000);                                     /* nop */
> -    stl_p(p++, 0x03e00009);                                     /* jalr ra */
> -    stl_p(p++, 0xa1040000);                                     /* sb a0,0(t0) */
> +    stl_p(p++, 0x3c08b800);                  /* lui t0,0xb400 */
> +    stl_p(p++, 0x350803f8);                  /* ori t0,t0,0x3f8 */
> +    stl_p(p++, 0x91090005);                  /* lbu t1,5(t0) */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x31290040);                  /* andi t1,t1,0x40 */
> +    stl_p(p++, 0x1120fffc);                  /* beqz t1,878 <outch+0x8> */
> +    stl_p(p++, 0x00000000);                  /* nop */
> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
> +    stl_p(p++, 0xa1040000);                  /* sb a0,0(t0) */

Are you planning to move/rename this file? Telling this now would 
justify your cleanup.

>   
>   }
>   
> diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
> index fc13f9e..44e1118 100644
> --- a/tests/acceptance/linux_ssh_mips_malta.py
> +++ b/tests/acceptance/linux_ssh_mips_malta.py
> @@ -99,10 +99,12 @@ class LinuxSSH(Test):
>       def ssh_command(self, command, is_root=True):
>           self.ssh_logger.info(command)
>           result = self.ssh_session.cmd(command)
> -        stdout_lines = [line.rstrip() for line in result.stdout_text.splitlines()]
> +        stdout_lines = [line.rstrip() for line
> +        in result.stdout_text.splitlines()]

I think QEMU Python coding style is to align below the opening bracket.

>           for line in stdout_lines:
>               self.ssh_logger.info(line)
> -        stderr_lines = [line.rstrip() for line in result.stderr_text.splitlines()]
> +        stderr_lines = [line.rstrip() for line
> +        in result.stderr_text.splitlines()]
>           for line in stderr_lines:
>               self.ssh_logger.warning(line)
>           return stdout_lines, stderr_lines
> 



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/5] mips: fulong 2e: Renovate coding style
  2019-11-25 13:04 ` [PATCH 5/5] mips: fulong 2e: " Filip Bozuta
@ 2019-12-01 23:49   ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-01 23:49 UTC (permalink / raw)
  To: Filip Bozuta
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 11566 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>     hw/mips/mips_fulong2e.c
>     hw/isa/vt82c686.c
>     hw/pci-host/bonito.c
>     include/hw/isa/vt82c686.h
>
> These mips Fulong 2E machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/isa/vt82c686.c    | 23 ++++++++++----------
>  hw/pci-host/bonito.c | 60 +++++++++++++++++++++++++++++-
> ----------------------
>  2 files changed, 45 insertions(+), 38 deletions(-)
>
>
Excellent!

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>



> diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
> index 616f67f..f828708 100644
> --- a/hw/isa/vt82c686.c
> +++ b/hw/isa/vt82c686.c
> @@ -27,7 +27,7 @@
>  #include "qemu/timer.h"
>  #include "exec/address-spaces.h"
>
> -//#define DEBUG_VT82C686B
> +/* #define DEBUG_VT82C686B */
>
>  #ifdef DEBUG_VT82C686B
>  #define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
> ##__VA_ARGS__)
> @@ -35,8 +35,7 @@
>  #define DPRINTF(fmt, ...)
>  #endif
>
> -typedef struct SuperIOConfig
> -{
> +typedef struct SuperIOConfig {
>      uint8_t config[0x100];
>      uint8_t index;
>      uint8_t data;
> @@ -102,7 +101,7 @@ static uint64_t superio_ioport_readb(void *opaque,
> hwaddr addr, unsigned size)
>      SuperIOConfig *superio_conf = opaque;
>
>      DPRINTF("superio_ioport_readb  address 0x%x\n", addr);
> -    return (superio_conf->config[superio_conf->index]);
> +    return superio_conf->config[superio_conf->index];
>  }
>
>  static const MemoryRegionOps superio_ops = {
> @@ -143,7 +142,7 @@ static void vt82c686b_isa_reset(DeviceState *dev)
>  }
>
>  /* write config pci function0 registers. PCI-ISA bridge */
> -static void vt82c686b_write_config(PCIDevice * d, uint32_t address,
> +static void vt82c686b_write_config(PCIDevice *d, uint32_t address,
>                                     uint32_t val, int len)
>  {
>      VT82C686BState *vt686 = VT82C686B_DEVICE(d);
> @@ -365,7 +364,7 @@ static void vt82c686b_pm_realize(PCIDevice *dev, Error
> **errp)
>      pci_set_long(pci_conf + 0x48, 0x00000001);
>
>      /* SMB ports:0xeee0~0xeeef */
> -    s->smb_io_base =((s->smb_io_base & 0xfff0) + 0x0);
> +    s->smb_io_base = ((s->smb_io_base & 0xfff0) + 0x0);
>      pci_conf[0x90] = s->smb_io_base | 1;
>      pci_conf[0x91] = s->smb_io_base >> 8;
>      pci_conf[0xd2] = 0x90;
> @@ -462,16 +461,18 @@ static void vt82c686b_realize(PCIDevice *d, Error
> **errp)
>
>      wmask = d->wmask;
>      for (i = 0x00; i < 0xff; i++) {
> -       if (i<=0x03 || (i>=0x08 && i<=0x3f)) {
> -           wmask[i] = 0x00;
> -       }
> +        if (i <= 0x03 || (i >= 0x08 && i <= 0x3f)) {
> +            wmask[i] = 0x00;
> +        }
>      }
>
>      memory_region_init_io(&vt82c->superio, OBJECT(d), &superio_ops,
>                            &vt82c->superio_conf, "superio", 2);
>      memory_region_set_enabled(&vt82c->superio, false);
> -    /* The floppy also uses 0x3f0 and 0x3f1.
> -     * But we do not emulate a floppy, so just set it here. */
> +    /*
> +     * The floppy also uses 0x3f0 and 0x3f1.
> +     * But we do not emulate a floppy, so just set it here.
> +     */
>      memory_region_add_subregion(isa_bus->address_space_io, 0x3f0,
>                                  &vt82c->superio);
>  }
> diff --git a/hw/pci-host/bonito.c b/hw/pci-host/bonito.c
> index ceee463..4692d41 100644
> --- a/hw/pci-host/bonito.c
> +++ b/hw/pci-host/bonito.c
> @@ -14,7 +14,8 @@
>   * fulong 2e mini pc has a bonito north bridge.
>   */
>
> -/* what is the meaning of devfn in qemu and IDSEL in bonito northbridge?
> +/*
> + * what is the meaning of devfn in qemu and IDSEL in bonito northbridge?
>   *
>   * devfn   pci_slot<<3  + funno
>   * one pci bus can have 32 devices and each device can have 8 functions.
> @@ -49,7 +50,7 @@
>  #include "sysemu/runstate.h"
>  #include "exec/address-spaces.h"
>
> -//#define DEBUG_BONITO
> +/* #define DEBUG_BONITO */
>
>  #ifdef DEBUG_BONITO
>  #define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__,
> ##__VA_ARGS__)
> @@ -60,45 +61,45 @@
>  /* from linux soure code. include/asm-mips/mips-boards/bonito64.h*/
>  #define BONITO_BOOT_BASE        0x1fc00000
>  #define BONITO_BOOT_SIZE        0x00100000
> -#define BONITO_BOOT_TOP         (BONITO_BOOT_BASE+BONITO_BOOT_SIZE-1)
> +#define BONITO_BOOT_TOP         (BONITO_BOOT_BASE + BONITO_BOOT_SIZE - 1)
>  #define BONITO_FLASH_BASE       0x1c000000
>  #define BONITO_FLASH_SIZE       0x03000000
> -#define BONITO_FLASH_TOP        (BONITO_FLASH_BASE+BONITO_FLASH_SIZE-1)
> +#define BONITO_FLASH_TOP        (BONITO_FLASH_BASE + BONITO_FLASH_SIZE -
> 1)
>  #define BONITO_SOCKET_BASE      0x1f800000
>  #define BONITO_SOCKET_SIZE      0x00400000
> -#define BONITO_SOCKET_TOP       (BONITO_SOCKET_BASE+BONITO_SOCKET_SIZE-1)
> +#define BONITO_SOCKET_TOP       (BONITO_SOCKET_BASE + BONITO_SOCKET_SIZE
> - 1)
>  #define BONITO_REG_BASE         0x1fe00000
>  #define BONITO_REG_SIZE         0x00040000
> -#define BONITO_REG_TOP          (BONITO_REG_BASE+BONITO_REG_SIZE-1)
> +#define BONITO_REG_TOP          (BONITO_REG_BASE + BONITO_REG_SIZE - 1)
>  #define BONITO_DEV_BASE         0x1ff00000
>  #define BONITO_DEV_SIZE         0x00100000
> -#define BONITO_DEV_TOP          (BONITO_DEV_BASE+BONITO_DEV_SIZE-1)
> +#define BONITO_DEV_TOP          (BONITO_DEV_BASE + BONITO_DEV_SIZE - 1)
>  #define BONITO_PCILO_BASE       0x10000000
>  #define BONITO_PCILO_BASE_VA    0xb0000000
>  #define BONITO_PCILO_SIZE       0x0c000000
> -#define BONITO_PCILO_TOP        (BONITO_PCILO_BASE+BONITO_PCILO_SIZE-1)
> +#define BONITO_PCILO_TOP        (BONITO_PCILO_BASE + BONITO_PCILO_SIZE -
> 1)
>  #define BONITO_PCILO0_BASE      0x10000000
>  #define BONITO_PCILO1_BASE      0x14000000
>  #define BONITO_PCILO2_BASE      0x18000000
>  #define BONITO_PCIHI_BASE       0x20000000
>  #define BONITO_PCIHI_SIZE       0x20000000
> -#define BONITO_PCIHI_TOP        (BONITO_PCIHI_BASE+BONITO_PCIHI_SIZE-1)
> +#define BONITO_PCIHI_TOP        (BONITO_PCIHI_BASE + BONITO_PCIHI_SIZE -
> 1)
>  #define BONITO_PCIIO_BASE       0x1fd00000
>  #define BONITO_PCIIO_BASE_VA    0xbfd00000
>  #define BONITO_PCIIO_SIZE       0x00010000
> -#define BONITO_PCIIO_TOP        (BONITO_PCIIO_BASE+BONITO_PCIIO_SIZE-1)
> +#define BONITO_PCIIO_TOP        (BONITO_PCIIO_BASE + BONITO_PCIIO_SIZE -
> 1)
>  #define BONITO_PCICFG_BASE      0x1fe80000
>  #define BONITO_PCICFG_SIZE      0x00080000
> -#define BONITO_PCICFG_TOP       (BONITO_PCICFG_BASE+BONITO_PCICFG_SIZE-1)
> +#define BONITO_PCICFG_TOP       (BONITO_PCICFG_BASE + BONITO_PCICFG_SIZE
> - 1)
>
>
>  #define BONITO_PCICONFIGBASE    0x00
>  #define BONITO_REGBASE          0x100
>
> -#define BONITO_PCICONFIG_BASE   (BONITO_PCICONFIGBASE+BONITO_REG_BASE)
> +#define BONITO_PCICONFIG_BASE   (BONITO_PCICONFIGBASE + BONITO_REG_BASE)
>  #define BONITO_PCICONFIG_SIZE   (0x100)
>
> -#define BONITO_INTERNAL_REG_BASE  (BONITO_REGBASE+BONITO_REG_BASE)
> +#define BONITO_INTERNAL_REG_BASE  (BONITO_REGBASE + BONITO_REG_BASE)
>  #define BONITO_INTERNAL_REG_SIZE  (0x70)
>
>  #define BONITO_SPCICONFIG_BASE  (BONITO_PCICFG_BASE)
> @@ -111,7 +112,7 @@
>
>  #define BONITO_BONPONCFG        (0x00 >> 2)      /* 0x100 */
>  #define BONITO_BONGENCFG_OFFSET 0x4
> -#define BONITO_BONGENCFG        (BONITO_BONGENCFG_OFFSET>>2)   /*0x104 */
> +#define BONITO_BONGENCFG        (BONITO_BONGENCFG_OFFSET >> 2)   /*0x104
> */
>
>  /* 2. IO & IDE configuration */
>  #define BONITO_IODEVCFG         (0x08 >> 2)      /* 0x108 */
> @@ -177,15 +178,15 @@
>  /* idsel BIT = pci slot number +12 */
>  #define PCI_SLOT_BASE              12
>  #define PCI_IDSEL_VIA686B_BIT      (17)
> -#define PCI_IDSEL_VIA686B          (1<<PCI_IDSEL_VIA686B_BIT)
> +#define PCI_IDSEL_VIA686B          (1 << PCI_IDSEL_VIA686B_BIT)
>
> -#define PCI_ADDR(busno,devno,funno,regno)  \
> -    ((((busno)<<16)&0xff0000) + (((devno)<<11)&0xf800) +
> (((funno)<<8)&0x700) + (regno))
> +#define PCI_ADDR(busno , devno , funno , regno)  \
> +    ((((busno) << 16) & 0xff0000) + (((devno) << 11) & 0xf800) + \
> +    (((funno) << 8) & 0x700) + (regno))
>
>  typedef struct BonitoState BonitoState;
>
> -typedef struct PCIBonitoState
> -{
> +typedef struct PCIBonitoState {
>      PCIDevice dev;
>
>      BonitoState *pcihost;
> @@ -239,7 +240,8 @@ static void bonito_writel(void *opaque, hwaddr addr,
>
>      saddr = addr >> 2;
>
> -    DPRINTF("bonito_writel "TARGET_FMT_plx" val %x saddr %x\n", addr,
> val, saddr);
> +    DPRINTF("bonito_writel "TARGET_FMT_plx" val %x saddr %x\n",
> +            addr, val, saddr);
>      switch (saddr) {
>      case BONITO_BONPONCFG:
>      case BONITO_IODEVCFG:
> @@ -363,7 +365,7 @@ static uint64_t bonito_ldma_readl(void *opaque, hwaddr
> addr,
>          return 0;
>      }
>
> -    val = ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)];
> +    val = ((uint32_t *)(&s->bonldma))[addr / sizeof(uint32_t)];
>
>      return val;
>  }
> @@ -377,7 +379,7 @@ static void bonito_ldma_writel(void *opaque, hwaddr
> addr,
>          return;
>      }
>
> -    ((uint32_t *)(&s->bonldma))[addr/sizeof(uint32_t)] = val &
> 0xffffffff;
> +    ((uint32_t *)(&s->bonldma))[addr / sizeof(uint32_t)] = val &
> 0xffffffff;
>  }
>
>  static const MemoryRegionOps bonito_ldma_ops = {
> @@ -400,7 +402,7 @@ static uint64_t bonito_cop_readl(void *opaque, hwaddr
> addr,
>          return 0;
>      }
>
> -    val = ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)];
> +    val = ((uint32_t *)(&s->boncop))[addr / sizeof(uint32_t)];
>
>      return val;
>  }
> @@ -414,7 +416,7 @@ static void bonito_cop_writel(void *opaque, hwaddr
> addr,
>          return;
>      }
>
> -    ((uint32_t *)(&s->boncop))[addr/sizeof(uint32_t)] = val & 0xffffffff;
> +    ((uint32_t *)(&s->boncop))[addr / sizeof(uint32_t)] = val &
> 0xffffffff;
>  }
>
>  static const MemoryRegionOps bonito_cop_ops = {
> @@ -446,7 +448,8 @@ static uint32_t bonito_sbridge_pciaddr(void *opaque,
> hwaddr addr)
>      cfgaddr = addr & 0xffff;
>      cfgaddr |= (s->regs[BONITO_PCIMAP_CFG] & 0xffff) << 16;
>
> -    idsel = (cfgaddr & BONITO_PCICONF_IDSEL_MASK) >>
> BONITO_PCICONF_IDSEL_OFFSET;
> +    idsel = (cfgaddr & BONITO_PCICONF_IDSEL_MASK) >>
> +             BONITO_PCICONF_IDSEL_OFFSET;
>      devno = ctz32(idsel);
>      funno = (cfgaddr & BONITO_PCICONF_FUN_MASK) >>
> BONITO_PCICONF_FUN_OFFSET;
>      regno = (cfgaddr & BONITO_PCICONF_REG_MASK) >>
> BONITO_PCICONF_REG_OFFSET;
> @@ -550,7 +553,7 @@ static void pci_bonito_set_irq(void *opaque, int
> irq_num, int level)
>  }
>
>  /* map the original irq (0~3) to bonito irq (16~47, but 16~31 are unused)
> */
> -static int pci_bonito_map_irq(PCIDevice * pci_dev, int irq_num)
> +static int pci_bonito_map_irq(PCIDevice *pci_dev, int irq_num)
>  {
>      int slot;
>
> @@ -618,7 +621,10 @@ static void bonito_realize(PCIDevice *dev, Error
> **errp)
>      SysBusDevice *sysbus = SYS_BUS_DEVICE(s->pcihost);
>      PCIHostState *phb = PCI_HOST_BRIDGE(s->pcihost);
>
> -    /* Bonito North Bridge, built on FPGA, VENDOR_ID/DEVICE_ID are
> "undefined" */
> +    /*
> +     * Bonito North Bridge, built on FPGA,
> +     * VENDOR_ID/DEVICE_ID are "undefined"
> +     */
>      pci_config_set_prog_interface(dev->config, 0x00);
>
>      /* set the north bridge register mapping */
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 14461 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/5] mips: r4000: Renovate coding style
  2019-11-25 13:04 ` [PATCH 4/5] mips: r4000: " Filip Bozuta
@ 2019-12-02  0:08   ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-02  0:08 UTC (permalink / raw)
  To: Filip Bozuta
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 6594 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in file:
>     hw/mips/mips_r4k.c
>
> This mips r4000 machine file was edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/mips/mips_r4k.c | 55 ++++++++++++++++++++++++++++++
> +++---------------------
>  1 file changed, 34 insertions(+), 21 deletions(-)
>
> diff --git a/hw/mips/mips_r4k.c b/hw/mips/mips_r4k.c
> index 7002423..d638358 100644
> --- a/hw/mips/mips_r4k.c
> +++ b/hw/mips/mips_r4k.c
> @@ -6,7 +6,7 @@
>   * ISA memory at the 0x10000000 (PHYS, 16Mb in size).
>   * All peripherial devices are attached to this "bus" with
>   * the standard PC ISA addresses.
> -*/
> + */
>
>  #include "qemu/osdep.h"
>  #include "qemu/units.h"
> @@ -54,17 +54,18 @@ static struct _loaderparams {
>      const char *initrd_filename;
>  } loaderparams;
>
> -static void mips_qemu_write (void *opaque, hwaddr addr,
> -                             uint64_t val, unsigned size)
> +static void mips_qemu_write(void *opaque, hwaddr addr,
> +                            uint64_t val, unsigned size)
>  {
> -    if ((addr & 0xffff) == 0 && val == 42)
> +    if ((addr & 0xffff) == 0 && val == 42) {
>          qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> -    else if ((addr & 0xffff) == 4 && val == 42)
> +    } else if ((addr & 0xffff) == 4 && val == 42) {
>          qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
> +    }
>  }
>
> -static uint64_t mips_qemu_read (void *opaque, hwaddr addr,
> -                                unsigned size)
> +static uint64_t mips_qemu_read(void *opaque, hwaddr addr,
> +                               unsigned size)
>  {
>      return 0;
>  }
> @@ -100,8 +101,9 @@ static int64_t load_kernel(void)
>                             (uint64_t *)&kernel_high, big_endian,
>                             EM_MIPS, 1, 0);
>      if (kernel_size >= 0) {
> -        if ((entry & ~0x7fffffffULL) == 0x80000000)
> +        if ((entry & ~0x7fffffffULL) == 0x80000000) {
>              entry = (int32_t)entry;
> +        }
>      } else {
>          error_report("could not load kernel '%s': %s",
>                       loaderparams.kernel_filename,
> @@ -113,9 +115,10 @@ static int64_t load_kernel(void)
>      initrd_size = 0;
>      initrd_offset = 0;
>      if (loaderparams.initrd_filename) {
> -        initrd_size = get_image_size (loaderparams.initrd_filename);
> +        initrd_size = get_image_size(loaderparams.initrd_filename);
>          if (initrd_size > 0) {
> -            initrd_offset = (kernel_high + ~INITRD_PAGE_MASK) &
> INITRD_PAGE_MASK;
> +            initrd_offset = (kernel_high + ~INITRD_PAGE_MASK) &
> +            INITRD_PAGE_MASK;


"INITRD_PAGE_MASK" should be aligned vertically with  "(kernel_high".
Otherwise:

Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>



>              if (initrd_offset + initrd_size > ram_size) {
>                  error_report("memory too small for initial ram disk '%s'",
>                               loaderparams.initrd_filename);
> @@ -139,11 +142,13 @@ static int64_t load_kernel(void)
>      params_buf[1] = tswap32(0x12345678);
>
>      if (initrd_size > 0) {
> -        snprintf((char *)params_buf + 8, 256, "rd_start=0x%" PRIx64 "
> rd_size=%" PRId64 " %s",
> +        snprintf((char *)params_buf + 8, 256,
> +                 "rd_start=0x%" PRIx64 " rd_size=%" PRId64 " %s",
>                   cpu_mips_phys_to_kseg0(NULL, initrd_offset),
>                   initrd_size, loaderparams.kernel_cmdline);
>      } else {
> -        snprintf((char *)params_buf + 8, 256, "%s",
> loaderparams.kernel_cmdline);
> +        snprintf((char *)params_buf + 8, 256,
> +        "%s", loaderparams.kernel_cmdline);
>      }
>
>      rom_add_blob_fixed("params", params_buf, params_size,
> @@ -207,15 +212,21 @@ void mips_r4k_init(MachineState *machine)
>
>      memory_region_add_subregion(address_space_mem, 0, ram);
>
> -    memory_region_init_io(iomem, NULL, &mips_qemu_ops, NULL, "mips-qemu",
> 0x10000);
> +    memory_region_init_io(iomem, NULL, &mips_qemu_ops,
> +                          NULL, "mips-qemu", 0x10000);
> +
>      memory_region_add_subregion(address_space_mem, 0x1fbf0000, iomem);
>
> -    /* Try to load a BIOS image. If this fails, we continue regardless,
> -       but initialize the hardware ourselves. When a kernel gets
> -       preloaded we also initialize the hardware, since the BIOS wasn't
> -       run. */
> -    if (bios_name == NULL)
> +    /*
> +     * Try to load a BIOS image. If this fails, we continue regardless,
> +     * but initialize the hardware ourselves. When a kernel gets
> +     * preloaded we also initialize the hardware, since the BIOS wasn't
> +     * run.
> +     */
> +
> +    if (bios_name == NULL) {
>          bios_name = BIOS_FILENAME;
> +    }
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>      if (filename) {
>          bios_size = get_image_size(filename);
> @@ -227,6 +238,7 @@ void mips_r4k_init(MachineState *machine)
>  #else
>      be = 0;
>  #endif
> +    dinfo = drive_get(IF_PFLASH, 0, 0);
>      if ((bios_size > 0) && (bios_size <= BIOS_SIZE)) {
>          bios = g_new(MemoryRegion, 1);
>          memory_region_init_ram(bios, NULL, "mips_r4k.bios", BIOS_SIZE,
> @@ -235,7 +247,7 @@ void mips_r4k_init(MachineState *machine)
>          memory_region_add_subregion(get_system_memory(), 0x1fc00000,
> bios);
>
>          load_image_targphys(filename, 0x1fc00000, BIOS_SIZE);
> -    } else if ((dinfo = drive_get(IF_PFLASH, 0, 0)) != NULL) {
> +    } else if (dinfo != NULL) {
>          uint32_t mips_rom = 0x00400000;
>          if (!pflash_cfi01_register(0x1fc00000, "mips_r4k.bios", mips_rom,
>                                     blk_by_legacy_dinfo(dinfo),
> @@ -280,11 +292,12 @@ void mips_r4k_init(MachineState *machine)
>
>      isa_vga_init(isa_bus);
>
> -    if (nd_table[0].used)
> +    if (nd_table[0].used) {
>          isa_ne2000_init(isa_bus, 0x300, 9, &nd_table[0]);
> +    }
>
>      ide_drive_get(hd, ARRAY_SIZE(hd));
> -    for(i = 0; i < MAX_IDE_BUS; i++)
> +    for (i = 0; i < MAX_IDE_BUS; i++)
>          isa_ide_init(isa_bus, ide_iobase[i], ide_iobase2[i], ide_irq[i],
>                       hd[MAX_IDE_DEVS * i],
>                       hd[MAX_IDE_DEVS * i + 1]);
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 8756 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-11-30 23:46   ` Aleksandar Markovic
@ 2019-12-02 20:49     ` Eduardo Habkost
  2019-12-05 21:27       ` Cleber Rosa
  0 siblings, 1 reply; 18+ messages in thread
From: Eduardo Habkost @ 2019-12-02 20:49 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: pburton, qemu-devel, Filip Bozuta, hpoussin, amarkovic,
	Cleber Rosa, aleksandar.rikalo, aurelien

On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote:
> On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:
> 
> > The script checkpatch.pl located in scripts folder was
> > used to detect all errors and warrnings in files:
> >     hw/mips/mips_malta.c
> >     hw/mips/gt64xxx_pci.c
> >     tests/acceptance/linux_ssh_mips_malta.py
> >
> > All these mips malta machine files were edited and
> > all the errors and warrings generated by the checkpatch.pl
> > script were corrected and then the script was
> > ran again to make sure there are no more errors and warnings.
> >
> > Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> > ---
> >  hw/mips/mips_malta.c                     | 139
> > ++++++++++++++++---------------
> >  tests/acceptance/linux_ssh_mips_malta.py |   6 +-
> >  2 files changed, 75 insertions(+), 70 deletions(-)
> >
> >
> Very good cleanup!
> 
> Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> I think this snippet is good, but I am just in case cc-ing Cleber and
> Eduardo to check if it is in accordance with any applicable guidelines of
> our test framework:

Thanks for CCing us.

> 
> 
> > diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> > b/tests/acceptance/linux_ssh_mips_malta.py
> > index fc13f9e..44e1118 100644
> > --- a/tests/acceptance/linux_ssh_mips_malta.py
> > +++ b/tests/acceptance/linux_ssh_mips_malta.py
> > @@ -99,10 +99,12 @@ class LinuxSSH(Test):
> >      def ssh_command(self, command, is_root=True):
> >          self.ssh_logger.info(command)
> >          result = self.ssh_session.cmd(command)
> > -        stdout_lines = [line.rstrip() for line in
> > result.stdout_text.splitlines()]
> > +        stdout_lines = [line.rstrip() for line
> > +        in result.stdout_text.splitlines()]
> >          for line in stdout_lines:
> >              self.ssh_logger.info(line)
> > -        stderr_lines = [line.rstrip() for line in
> > result.stderr_text.splitlines()]
> > +        stderr_lines = [line.rstrip() for line
> > +        in result.stderr_text.splitlines()]

If you really want to split those lines, please indent them
properly.  e.g.:

        stdout_lines = [line.rstrip() for line
                        in result.stdout_text.splitlines()]

See PEP 8 [1] for additional examples.

Personally, I would just keep the existing
linux_ssh_mips_malta.py code as is.  I don't think splitting
those lines improves readability.

[1] https://www.python.org/dev/peps/pep-0008/#indentation

-- 
Eduardo



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-12-02 20:49     ` Eduardo Habkost
@ 2019-12-05 21:27       ` Cleber Rosa
  2019-12-06  8:24         ` Aleksandar Markovic
  0 siblings, 1 reply; 18+ messages in thread
From: Cleber Rosa @ 2019-12-05 21:27 UTC (permalink / raw)
  To: Eduardo Habkost
  Cc: pburton, qemu-devel, Filip Bozuta, hpoussin, aurelien, amarkovic,
	aleksandar.rikalo, Aleksandar Markovic

On Mon, Dec 02, 2019 at 05:49:58PM -0300, Eduardo Habkost wrote:
> On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote:
> > On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:
> > 
> > > The script checkpatch.pl located in scripts folder was
> > > used to detect all errors and warrnings in files:
> > >     hw/mips/mips_malta.c
> > >     hw/mips/gt64xxx_pci.c
> > >     tests/acceptance/linux_ssh_mips_malta.py
> > >
> > > All these mips malta machine files were edited and
> > > all the errors and warrings generated by the checkpatch.pl
> > > script were corrected and then the script was
> > > ran again to make sure there are no more errors and warnings.
> > >
> > > Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> > > ---
> > >  hw/mips/mips_malta.c                     | 139
> > > ++++++++++++++++---------------
> > >  tests/acceptance/linux_ssh_mips_malta.py |   6 +-
> > >  2 files changed, 75 insertions(+), 70 deletions(-)
> > >
> > >
> > Very good cleanup!
> > 
> > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > 
> > I think this snippet is good, but I am just in case cc-ing Cleber and
> > Eduardo to check if it is in accordance with any applicable guidelines of
> > our test framework:
> 
> Thanks for CCing us.
> 
> > 
> > 
> > > diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> > > b/tests/acceptance/linux_ssh_mips_malta.py
> > > index fc13f9e..44e1118 100644
> > > --- a/tests/acceptance/linux_ssh_mips_malta.py
> > > +++ b/tests/acceptance/linux_ssh_mips_malta.py
> > > @@ -99,10 +99,12 @@ class LinuxSSH(Test):
> > >      def ssh_command(self, command, is_root=True):
> > >          self.ssh_logger.info(command)
> > >          result = self.ssh_session.cmd(command)
> > > -        stdout_lines = [line.rstrip() for line in
> > > result.stdout_text.splitlines()]
> > > +        stdout_lines = [line.rstrip() for line
> > > +        in result.stdout_text.splitlines()]
> > >          for line in stdout_lines:
> > >              self.ssh_logger.info(line)
> > > -        stderr_lines = [line.rstrip() for line in
> > > result.stderr_text.splitlines()]
> > > +        stderr_lines = [line.rstrip() for line
> > > +        in result.stderr_text.splitlines()]
> 
> If you really want to split those lines, please indent them
> properly.  e.g.:
> 
>         stdout_lines = [line.rstrip() for line
>                         in result.stdout_text.splitlines()]
> 
> See PEP 8 [1] for additional examples.
> 
> Personally, I would just keep the existing
> linux_ssh_mips_malta.py code as is.  I don't think splitting
> those lines improves readability.
>
> [1] https://www.python.org/dev/peps/pep-0008/#indentation
> 
> -- 
> Eduardo

Right.  It only helps when running terminal or editor settings are
limited to ~80 cols.  And even in those cases, sometimes such code
split across lines needs a lot of gymnastics to not degrade in
readability when compared to a longer line.

We're not (yet?) enforcing PEP 8, so either as Eduardo suggested, or
with no changes LGTM.

Cheers,
- Cleber.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/5] mips: mipssim: Renovate coding style
  2019-11-25 13:04 ` [PATCH 3/5] mips: mipssim: " Filip Bozuta
@ 2019-12-06  8:03   ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-06  8:03 UTC (permalink / raw)
  To: Filip Bozuta
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 3653 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>     hw/mips/mips_mipssim.c
>     hw/net/mipsnet.c
>
> All these mips mipssim machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/net/mipsnet.c | 44 ++++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
>
>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>


> diff --git a/hw/net/mipsnet.c b/hw/net/mipsnet.c
> index f7ae1ce..cb3331f 100644
> --- a/hw/net/mipsnet.c
> +++ b/hw/net/mipsnet.c
> @@ -9,19 +9,19 @@
>
>  /* MIPSnet register offsets */
>
> -#define MIPSNET_DEV_ID         0x00
> -#define MIPSNET_BUSY           0x08
> -#define MIPSNET_RX_DATA_COUNT  0x0c
> -#define MIPSNET_TX_DATA_COUNT  0x10
> -#define MIPSNET_INT_CTL                0x14
> -# define MIPSNET_INTCTL_TXDONE         0x00000001
> -# define MIPSNET_INTCTL_RXDONE         0x00000002
> -# define MIPSNET_INTCTL_TESTBIT                0x80000000
> -#define MIPSNET_INTERRUPT_INFO 0x18
> -#define MIPSNET_RX_DATA_BUFFER 0x1c
> -#define MIPSNET_TX_DATA_BUFFER 0x20
> -
> -#define MAX_ETH_FRAME_SIZE     1514
> +#define MIPSNET_DEV_ID          0x00
> +#define MIPSNET_BUSY            0x08
> +#define MIPSNET_RX_DATA_COUNT   0x0c
> +#define MIPSNET_TX_DATA_COUNT   0x10
> +#define MIPSNET_INT_CTL         0x14
> +# define MIPSNET_INTCTL_TXDONE          0x00000001
> +# define MIPSNET_INTCTL_RXDONE          0x00000002
> +# define MIPSNET_INTCTL_TESTBIT         0x80000000
> +#define MIPSNET_INTERRUPT_INFO  0x18
> +#define MIPSNET_RX_DATA_BUFFER  0x1c
> +#define MIPSNET_TX_DATA_BUFFER  0x20
> +
> +#define MAX_ETH_FRAME_SIZE      1514
>
>  #define TYPE_MIPS_NET "mipsnet"
>  #define MIPS_NET(obj) OBJECT_CHECK(MIPSnetState, (obj), TYPE_MIPS_NET)
> @@ -64,8 +64,9 @@ static void mipsnet_update_irq(MIPSnetState *s)
>
>  static int mipsnet_buffer_full(MIPSnetState *s)
>  {
> -    if (s->rx_count >= MAX_ETH_FRAME_SIZE)
> +    if (s->rx_count >= MAX_ETH_FRAME_SIZE) {
>          return 1;
> +    }
>      return 0;
>  }
>
> @@ -73,18 +74,21 @@ static int mipsnet_can_receive(NetClientState *nc)
>  {
>      MIPSnetState *s = qemu_get_nic_opaque(nc);
>
> -    if (s->busy)
> +    if (s->busy) {
>          return 0;
> +    }
>      return !mipsnet_buffer_full(s);
>  }
>
> -static ssize_t mipsnet_receive(NetClientState *nc, const uint8_t *buf,
> size_t size)
> +static ssize_t mipsnet_receive(NetClientState *nc,
> +                               const uint8_t *buf, size_t size)
>  {
>      MIPSnetState *s = qemu_get_nic_opaque(nc);
>
> -    trace_mipsnet_receive(size);
> -    if (!mipsnet_can_receive(nc))
> +    race_mipsnet_receive(size);
> +    if (!mipsnet_can_receive(nc)) {
>          return 0;
> +    }
>
>      if (size >= sizeof(s->rx_buffer)) {
>          return 0;
> @@ -115,10 +119,10 @@ static uint64_t mipsnet_ioport_read(void *opaque,
> hwaddr addr,
>      addr &= 0x3f;
>      switch (addr) {
>      case MIPSNET_DEV_ID:
> -        ret = be32_to_cpu(0x4d495053);         /* MIPS */
> +        ret = be32_to_cpu(0x4d495053);          /* MIPS */
>          break;
>      case MIPSNET_DEV_ID + 4:
> -        ret = be32_to_cpu(0x4e455430);         /* NET0 */
> +        ret = be32_to_cpu(0x4e455430);          /* NET0 */
>          break;
>      case MIPSNET_BUSY:
>          ret = s->busy;
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 5054 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/5] mips: jazz: Renovate coding style
  2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
@ 2019-12-06  8:07   ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-06  8:07 UTC (permalink / raw)
  To: Filip Bozuta
  Cc: pburton, qemu-devel, hpoussin, amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 11536 bytes --]

On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com> wrote:

> The script checkpatch.pl located in scripts folder was
> used to detect all errors and warrnings in files:
>     hw/mips/mips_jazz.c
>     hw/display/jazz_led.c
>     hw/dma/rc4030.c
>
> All these mips jazz machine files were edited and
> all the errors and warrings generated by the checkpatch.pl
> script were corrected and then the script was
> ran again to make sure there are no more errors and warnings.
>
> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> ---
>  hw/display/jazz_led.c | 123 +++++++++++++++++++++++++-----
> --------------------
>  hw/dma/rc4030.c       |  12 +++--
>  hw/mips/mips_jazz.c   |  32 +++++++------
>  3 files changed, 88 insertions(+), 79 deletions(-)
>
> diff --git a/hw/display/jazz_led.c b/hw/display/jazz_led.c
> index 3e0112b..1d84559 100644
> --- a/hw/display/jazz_led.c
> +++ b/hw/display/jazz_led.c
> @@ -90,25 +90,25 @@ static void draw_horizontal_line(DisplaySurface *ds,
>
>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>


>      bpp = (surface_bits_per_pixel(ds) + 7) >> 3;
>      d = surface_data(ds) + surface_stride(ds) * posy + bpp * posx1;
> -    switch(bpp) {
> -        case 1:
> -            for (x = posx1; x <= posx2; x++) {
> -                *((uint8_t *)d) = color;
> -                d++;
> -            }
> -            break;
> -        case 2:
> -            for (x = posx1; x <= posx2; x++) {
> -                *((uint16_t *)d) = color;
> -                d += 2;
> -            }
> -            break;
> -        case 4:
> -            for (x = posx1; x <= posx2; x++) {
> -                *((uint32_t *)d) = color;
> -                d += 4;
> -            }
> -            break;
> +    switch (bpp) {
> +    case 1:
> +        for (x = posx1; x <= posx2; x++) {
> +            *((uint8_t *)d) = color;
> +            d++;
> +        }
> +        break;
> +    case 2:
> +        for (x = posx1; x <= posx2; x++) {
> +            *((uint16_t *)d) = color;
> +            d += 2;
> +        }
> +        break;
> +    case 4:
> +        for (x = posx1; x <= posx2; x++) {
> +            *((uint32_t *)d) = color;
> +            d += 4;
> +        }
> +        break;
>      }
>  }
>
> @@ -121,25 +121,25 @@ static void draw_vertical_line(DisplaySurface *ds,
>
>      bpp = (surface_bits_per_pixel(ds) + 7) >> 3;
>      d = surface_data(ds) + surface_stride(ds) * posy1 + bpp * posx;
> -    switch(bpp) {
> -        case 1:
> -            for (y = posy1; y <= posy2; y++) {
> -                *((uint8_t *)d) = color;
> -                d += surface_stride(ds);
> -            }
> -            break;
> -        case 2:
> -            for (y = posy1; y <= posy2; y++) {
> -                *((uint16_t *)d) = color;
> -                d += surface_stride(ds);
> -            }
> -            break;
> -        case 4:
> -            for (y = posy1; y <= posy2; y++) {
> -                *((uint32_t *)d) = color;
> -                d += surface_stride(ds);
> -            }
> -            break;
> +    switch (bpp) {
> +    case 1:
> +        for (y = posy1; y <= posy2; y++) {
> +            *((uint8_t *)d) = color;
> +            d += surface_stride(ds);
> +        }
> +        break;
> +    case 2:
> +        for (y = posy1; y <= posy2; y++) {
> +            *((uint16_t *)d) = color;
> +            d += surface_stride(ds);
> +        }
> +        break;
> +    case 4:
> +        for (y = posy1; y <= posy2; y++) {
> +            *((uint32_t *)d) = color;
> +            d += surface_stride(ds);
> +        }
> +        break;
>      }
>  }
>
> @@ -164,28 +164,28 @@ static void jazz_led_update_display(void *opaque)
>      if (s->state & REDRAW_SEGMENTS) {
>          /* set colors according to bpp */
>          switch (surface_bits_per_pixel(surface)) {
> -            case 8:
> -                color_segment = rgb_to_pixel8(0xaa, 0xaa, 0xaa);
> -                color_led = rgb_to_pixel8(0x00, 0xff, 0x00);
> -                break;
> -            case 15:
> -                color_segment = rgb_to_pixel15(0xaa, 0xaa, 0xaa);
> -                color_led = rgb_to_pixel15(0x00, 0xff, 0x00);
> -                break;
> -            case 16:
> -                color_segment = rgb_to_pixel16(0xaa, 0xaa, 0xaa);
> -                color_led = rgb_to_pixel16(0x00, 0xff, 0x00);
> -                break;
> -            case 24:
> -                color_segment = rgb_to_pixel24(0xaa, 0xaa, 0xaa);
> -                color_led = rgb_to_pixel24(0x00, 0xff, 0x00);
> -                break;
> -            case 32:
> -                color_segment = rgb_to_pixel32(0xaa, 0xaa, 0xaa);
> -                color_led = rgb_to_pixel32(0x00, 0xff, 0x00);
> -                break;
> -            default:
> -                return;
> +        case 8:
> +            color_segment = rgb_to_pixel8(0xaa, 0xaa, 0xaa);
> +            color_led = rgb_to_pixel8(0x00, 0xff, 0x00);
> +            break;
> +        case 15:
> +            color_segment = rgb_to_pixel15(0xaa, 0xaa, 0xaa);
> +            color_led = rgb_to_pixel15(0x00, 0xff, 0x00);
> +            break;
> +        case 16:
> +            color_segment = rgb_to_pixel16(0xaa, 0xaa, 0xaa);
> +            color_led = rgb_to_pixel16(0x00, 0xff, 0x00);
> +            break;
> +        case 24:
> +            color_segment = rgb_to_pixel24(0xaa, 0xaa, 0xaa);
> +            color_led = rgb_to_pixel24(0x00, 0xff, 0x00);
> +            break;
> +        case 32:
> +            color_segment = rgb_to_pixel32(0xaa, 0xaa, 0xaa);
> +            color_led = rgb_to_pixel32(0x00, 0xff, 0x00);
> +            break;
> +        default:
> +            return;
>          }
>
>          /* display segments */
> @@ -205,8 +205,9 @@ static void jazz_led_update_display(void *opaque)
>                               (s->segments & 0x80) ? color_segment : 0);
>
>          /* display led */
> -        if (!(s->segments & 0x01))
> +        if (!(s->segments & 0x01)) {
>              color_led = 0; /* black */
> +        }
>          draw_horizontal_line(surface, 68, 50, 50, color_led);
>          draw_horizontal_line(surface, 69, 49, 51, color_led);
>          draw_horizontal_line(surface, 70, 48, 52, color_led);
> diff --git a/hw/dma/rc4030.c b/hw/dma/rc4030.c
> index d54e296..f7542f3 100644
> --- a/hw/dma/rc4030.c
> +++ b/hw/dma/rc4030.c
> @@ -397,10 +397,11 @@ static void update_jazz_irq(rc4030State *s)
>
>      pending = s->isr_jazz & s->imr_jazz;
>
> -    if (pending != 0)
> +    if (pending != 0) {
>          qemu_irq_raise(s->jazz_bus_irq);
> -    else
> +    } else {
>          qemu_irq_lower(s->jazz_bus_irq);
> +    }
>  }
>
>  static void rc4030_irq_jazz_request(void *opaque, int irq, int level)
> @@ -588,7 +589,8 @@ static const VMStateDescription vmstate_rc4030 = {
>      }
>  };
>
> -static void rc4030_do_dma(void *opaque, int n, uint8_t *buf, int len, int
> is_write)
> +static void rc4030_do_dma(void *opaque, int n, uint8_t *buf,
> +                          int len, int is_write)
>  {
>      rc4030State *s = opaque;
>      hwaddr dma_addr;
> @@ -643,8 +645,8 @@ static rc4030_dma *rc4030_allocate_dmas(void *opaque,
> int n)
>      struct rc4030DMAState *p;
>      int i;
>
> -    s = (rc4030_dma *)g_malloc0(sizeof(rc4030_dma) * n);
> -    p = (struct rc4030DMAState *)g_malloc0(sizeof(struct rc4030DMAState)
> * n);
> +    s = (rc4030_dma *)g_new0(sizeof(rc4030_dma) * n);
> +    p = (struct rc4030DMAState *)g_new0(sizeof(struct rc4030DMAState) *
> n);
>      for (i = 0; i < n; i++) {
>          p->opaque = opaque;
>          p->n = i;
> diff --git a/hw/mips/mips_jazz.c b/hw/mips/mips_jazz.c
> index d978bb6..ac4d7ac 100644
> --- a/hw/mips/mips_jazz.c
> +++ b/hw/mips/mips_jazz.c
> @@ -52,8 +52,7 @@
>  #include "qemu/error-report.h"
>  #include "qemu/help_option.h"
>
> -enum jazz_model_e
> -{
> +enum jazz_model_e {
>      JAZZ_MAGNUM,
>      JAZZ_PICA61,
>  };
> @@ -90,16 +89,20 @@ static const MemoryRegionOps rtc_ops = {
>  static uint64_t dma_dummy_read(void *opaque, hwaddr addr,
>                                 unsigned size)
>  {
> -    /* Nothing to do. That is only to ensure that
> -     * the current DMA acknowledge cycle is completed. */
> +    /*
> +     * Nothing to do. That is only to ensure that
> +     * the current DMA acknowledge cycle is completed.
> +     */
>      return 0xff;
>  }
>
>  static void dma_dummy_write(void *opaque, hwaddr addr,
>                              uint64_t val, unsigned size)
>  {
> -    /* Nothing to do. That is only to ensure that
> -     * the current DMA acknowledge cycle is completed. */
> +    /*
> +     * Nothing to do. That is only to ensure that
> +     * the current DMA acknowledge cycle is completed.
> +     */
>  }
>
>  static const MemoryRegionOps dma_dummy_ops = {
> @@ -109,8 +112,8 @@ static const MemoryRegionOps dma_dummy_ops = {
>  };
>
>  #define MAGNUM_BIOS_SIZE_MAX 0x7e000
> -#define MAGNUM_BIOS_SIZE (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE :
> MAGNUM_BIOS_SIZE_MAX)
> -
> +#define MAGNUM_BIOS_SIZE
>      \
> +        (BIOS_SIZE < MAGNUM_BIOS_SIZE_MAX ? BIOS_SIZE :
> MAGNUM_BIOS_SIZE_MAX)
>  static void (*real_do_transaction_failed)(CPUState *cpu, hwaddr physaddr,
>                                            vaddr addr, unsigned size,
>                                            MMUAccessType access_type,
> @@ -201,8 +204,9 @@ static void mips_jazz_init(MachineState *machine,
>      memory_region_add_subregion(address_space, 0xfff00000LL, bios2);
>
>      /* load the BIOS image. */
> -    if (bios_name == NULL)
> +    if (bios_name == NULL) {
>          bios_name = BIOS_FILENAME;
> +    }
>      filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, bios_name);
>      if (filename) {
>          bios_size = load_image_targphys(filename, 0xfff00000LL,
> @@ -229,7 +233,8 @@ static void mips_jazz_init(MachineState *machine,
>                                  sysbus_mmio_get_region(sysbus, 0));
>      memory_region_add_subregion(address_space, 0xf0000000,
>                                  sysbus_mmio_get_region(sysbus, 1));
> -    memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops, NULL,
> "dummy_dma", 0x1000);
> +    memory_region_init_io(dma_dummy, NULL, &dma_dummy_ops,
> +                          NULL, "dummy_dma", 0x1000);
>      memory_region_add_subregion(address_space, 0x8000d000, dma_dummy);
>
>      /* ISA bus: IO space at 0x90000000, mem space at 0x91000000 */
> @@ -276,8 +281,9 @@ static void mips_jazz_init(MachineState *machine,
>      /* Network controller */
>      for (n = 0; n < nb_nics; n++) {
>          nd = &nd_table[n];
> -        if (!nd->model)
> +        if (!nd->model) {
>              nd->model = g_strdup("dp83932");
> +        }
>          if (strcmp(nd->model, "dp83932") == 0) {
>              qemu_check_nic_model(nd, "dp83932");
>
> @@ -338,12 +344,12 @@ static void mips_jazz_init(MachineState *machine,
>      /* Serial ports */
>      if (serial_hd(0)) {
>          serial_mm_init(address_space, 0x80006000, 0,
> -                       qdev_get_gpio_in(rc4030, 8), 8000000/16,
> +                       qdev_get_gpio_in(rc4030, 8), 8000000 / 16,
>                         serial_hd(0), DEVICE_NATIVE_ENDIAN);
>      }
>      if (serial_hd(1)) {
>          serial_mm_init(address_space, 0x80007000, 0,
> -                       qdev_get_gpio_in(rc4030, 9), 8000000/16,
> +                       qdev_get_gpio_in(rc4030, 9), 8000000 / 16,
>                         serial_hd(1), DEVICE_NATIVE_ENDIAN);
>      }
>
> --
> 2.7.4
>
>
>

[-- Attachment #2: Type: text/html, Size: 14833 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-12-01 20:20   ` Philippe Mathieu-Daudé
@ 2019-12-06  8:21     ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-06  8:21 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: pburton, qemu-devel, Filip Bozuta, hpoussin, amarkovic,
	aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 15952 bytes --]

On Sunday, December 1, 2019, Philippe Mathieu-Daudé <philmd@redhat.com>
wrote:

> Hi Filip,
>
> On 11/25/19 2:04 PM, Filip Bozuta wrote:
>
>> The script checkpatch.pl located in scripts folder was
>> used to detect all errors and warrnings in files:
>>      hw/mips/mips_malta.c
>>      hw/mips/gt64xxx_pci.c
>>      tests/acceptance/linux_ssh_mips_malta.py
>>
>> All these mips malta machine files were edited and
>> all the errors and warrings generated by the checkpatch.pl
>> script were corrected and then the script was
>> ran again to make sure there are no more errors and warnings.
>>
>> Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
>> ---
>>   hw/mips/mips_malta.c                     | 139
>> ++++++++++++++++---------------
>>   tests/acceptance/linux_ssh_mips_malta.py |   6 +-
>>   2 files changed, 75 insertions(+), 70 deletions(-)
>>
>> diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
>> index 92e9ca5..18eafac 100644
>> --- a/hw/mips/mips_malta.c
>> +++ b/hw/mips/mips_malta.c
>> @@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
>>    */
>>     #if defined(DEBUG)
>> -#  define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__,
>> ## __VA_ARGS__)
>> +#  define logout(fmt, ...) \
>> +          fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
>>
>
> This is deprecated code, if you look in the repository history, we usually
> don't touch it, except to get rid of it. The way to get rid of it is to
> replace the calls by trace events.
>
>
Code must be read, whatever its history or destiny is. If slated for
refactoring, one reason more to be visible in less than 80 columns. This
hunk improves code visibility, clearly separates two parts of a macro, and
is fine to me.


>   #else
>>   #  define logout(fmt, ...) ((void)0)
>>   #endif
>> @@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion
>> *address_space,
>>       MaltaFPGAState *s;
>>       Chardev *chr;
>>   -    s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
>> +    s = (MaltaFPGAState *)g_new0(sizeof(MaltaFPGAState));
>>
>
> This change doesn't even compile... Please compile your patches before
> posting them to the list.
>
> You can find the prototype of this macro here:
> https://developer.gnome.org/glib/stable/glib-Memory-Allocation.html#g-new0
>
>
This must be fixed. Filip, you need to send v2 with the fix for build error
and other improvements asked in replies to this series.


>         memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
>>                             "malta-fpga", 0x100000);
>> @@ -844,9 +845,9 @@ static void write_bootloader(uint8_t *base, int64_t
>> run_addr,
>>       /* Small bootloader */
>>       p = (uint32_t *)base;
>>   -    stl_p(p++, 0x08000000 |                                      /* j
>> 0x1fc00580 */
>> +    stl_p(p++, 0x08000000 |                                   /* j
>> 0x1fc00580 */
>>                    ((run_addr + 0x580) & 0x0fffffff) >> 2);
>> -    stl_p(p++, 0x00000000);                                      /* nop
>> */
>> +    stl_p(p++, 0x00000000);                                   /* nop */
>>         /* YAMON service vector */
>>       stl_p(base + 0x500, run_addr + 0x0580);      /* start: */
>> @@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base,
>> int64_t run_addr,
>>       stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
>>         /* Load BAR registers as done by YAMON */
>> -    stl_p(p++, 0x3c09b400);                                      /* lui
>> t1, 0xb400 */
>> +    stl_p(p++, 0x3c09b400);                 /* lui t1, 0xb400 */
>>     #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c08df00);                                      /* lui
>> t0, 0xdf00 */
>> +    stl_p(p++, 0x3c08df00);                  /* lui t0, 0xdf00 */
>>   #else
>> -    stl_p(p++, 0x340800df);                                      /* ori
>> t0, r0, 0x00df */
>> +    stl_p(p++, 0x340800df);                  /* ori t0, r0, 0x00df */
>>   #endif
>> -    stl_p(p++, 0xad280068);                                      /* sw
>> t0, 0x0068(t1) */
>> +    stl_p(p++, 0xad280068);                  /* sw t0, 0x0068(t1) */
>>   -    stl_p(p++, 0x3c09bbe0);                                      /*
>> lui t1, 0xbbe0 */
>> +    stl_p(p++, 0x3c09bbe0);                  /* lui t1, 0xbbe0 */
>>     #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c08c000);                                      /* lui
>> t0, 0xc000 */
>> +    stl_p(p++, 0x3c08c000);                  /* lui t0, 0xc000 */
>>   #else
>> -    stl_p(p++, 0x340800c0);                                      /* ori
>> t0, r0, 0x00c0 */
>> +    stl_p(p++, 0x340800c0);                  /* ori t0, r0, 0x00c0 */
>>   #endif
>> -    stl_p(p++, 0xad280048);                                      /* sw
>> t0, 0x0048(t1) */
>> +    stl_p(p++, 0xad280048);                  /* sw t0, 0x0048(t1) */
>>   #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c084000);                                      /* lui
>> t0, 0x4000 */
>> +    stl_p(p++, 0x3c084000);                  /* lui t0, 0x4000 */
>>   #else
>> -    stl_p(p++, 0x34080040);                                      /* ori
>> t0, r0, 0x0040 */
>> +    stl_p(p++, 0x34080040);                  /* ori t0, r0, 0x0040 */
>>   #endif
>> -    stl_p(p++, 0xad280050);                                      /* sw
>> t0, 0x0050(t1) */
>> +    stl_p(p++, 0xad280050);                  /* sw t0, 0x0050(t1) */
>>     #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c088000);                                      /* lui
>> t0, 0x8000 */
>> +    stl_p(p++, 0x3c088000);                  /* lui t0, 0x8000 */
>>   #else
>> -    stl_p(p++, 0x34080080);                                      /* ori
>> t0, r0, 0x0080 */
>> +    stl_p(p++, 0x34080080);                  /* ori t0, r0, 0x0080 */
>>   #endif
>> -    stl_p(p++, 0xad280058);                                      /* sw
>> t0, 0x0058(t1) */
>> +    stl_p(p++, 0xad280058);                  /* sw t0, 0x0058(t1) */
>>   #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c083f00);                                      /* lui
>> t0, 0x3f00 */
>> +    stl_p(p++, 0x3c083f00);                  /* lui t0, 0x3f00 */
>>   #else
>> -    stl_p(p++, 0x3408003f);                                      /* ori
>> t0, r0, 0x003f */
>> +    stl_p(p++, 0x3408003f);                  /* ori t0, r0, 0x003f */
>>   #endif
>> -    stl_p(p++, 0xad280060);                                      /* sw
>> t0, 0x0060(t1) */
>> +    stl_p(p++, 0xad280060);                  /* sw t0, 0x0060(t1) */
>>     #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c08c100);                                      /* lui
>> t0, 0xc100 */
>> +    stl_p(p++, 0x3c08c100);                  /* lui t0, 0xc100 */
>>   #else
>> -    stl_p(p++, 0x340800c1);                                      /* ori
>> t0, r0, 0x00c1 */
>> +    stl_p(p++, 0x340800c1);                  /* ori t0, r0, 0x00c1 */
>>   #endif
>> -    stl_p(p++, 0xad280080);                                      /* sw
>> t0, 0x0080(t1) */
>> +    stl_p(p++, 0xad280080);                  /* sw t0, 0x0080(t1) */
>>   #ifdef TARGET_WORDS_BIGENDIAN
>> -    stl_p(p++, 0x3c085e00);                                      /* lui
>> t0, 0x5e00 */
>> +    stl_p(p++, 0x3c085e00);                  /* lui t0, 0x5e00 */
>>   #else
>> -    stl_p(p++, 0x3408005e);                                      /* ori
>> t0, r0, 0x005e */
>> +    stl_p(p++, 0x3408005e);                  /* ori t0, r0, 0x005e */
>>   #endif
>> -    stl_p(p++, 0xad280088);                                      /* sw
>> t0, 0x0088(t1) */
>> +    stl_p(p++, 0xad280088);                  /* sw t0, 0x0088(t1) */
>>         /* Jump to kernel code */
>> -    stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff));    /* lui
>> ra, high(kernel_entry) */
>> -    stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff));            /* ori
>> ra, ra, low(kernel_entry) */
>> -    stl_p(p++, 0x03e00009);                                      /* jalr
>> ra */
>> -    stl_p(p++, 0x00000000);                                      /* nop
>> */
>> +    stl_p(p++, 0x3c1f0000 |
>> +          ((kernel_entry >> 16) & 0xffff));  /* lui ra,
>> high(kernel_entry) */
>> +    stl_p(p++, 0x37ff0000 |
>> +          (kernel_entry & 0xffff));          /* ori ra, ra,
>> low(kernel_entry) */
>> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>>         /* YAMON subroutines */
>>       p = (uint32_t *) (base + 0x800);
>> -    stl_p(p++, 0x03e00009);                                     /* jalr
>> ra */
>> -    stl_p(p++, 0x24020000);                                     /* li
>> v0,0 */
>> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
>> +    stl_p(p++, 0x24020000);                  /* li v0,0 */
>>       /* 808 YAMON print */
>> -    stl_p(p++, 0x03e06821);                                     /* move
>> t5,ra */
>> -    stl_p(p++, 0x00805821);                                     /* move
>> t3,a0 */
>> -    stl_p(p++, 0x00a05021);                                     /* move
>> t2,a1 */
>> -    stl_p(p++, 0x91440000);                                     /* lbu
>> a0,0(t2) */
>> -    stl_p(p++, 0x254a0001);                                     /* addiu
>> t2,t2,1 */
>> -    stl_p(p++, 0x10800005);                                     /* beqz
>> a0,834 */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x0ff0021c);                                     /* jal
>> 870 */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x1000fff9);                                     /* b 814
>> */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x01a00009);                                     /* jalr
>> t5 */
>> -    stl_p(p++, 0x01602021);                                     /* move
>> a0,t3 */
>> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
>> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
>> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
>> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
>> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
>> +    stl_p(p++, 0x10800005);                  /* beqz a0,834 */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x1000fff9);                  /* b 814 */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
>> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>>       /* 0x83c YAMON print_count */
>> -    stl_p(p++, 0x03e06821);                                     /* move
>> t5,ra */
>> -    stl_p(p++, 0x00805821);                                     /* move
>> t3,a0 */
>> -    stl_p(p++, 0x00a05021);                                     /* move
>> t2,a1 */
>> -    stl_p(p++, 0x00c06021);                                     /* move
>> t4,a2 */
>> -    stl_p(p++, 0x91440000);                                     /* lbu
>> a0,0(t2) */
>> -    stl_p(p++, 0x0ff0021c);                                     /* jal
>> 870 */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x254a0001);                                     /* addiu
>> t2,t2,1 */
>> -    stl_p(p++, 0x258cffff);                                     /* addiu
>> t4,t4,-1 */
>> -    stl_p(p++, 0x1580fffa);                                     /* bnez
>> t4,84c */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x01a00009);                                     /* jalr
>> t5 */
>> -    stl_p(p++, 0x01602021);                                     /* move
>> a0,t3 */
>> +    stl_p(p++, 0x03e06821);                  /* move t5,ra */
>> +    stl_p(p++, 0x00805821);                  /* move t3,a0 */
>> +    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
>> +    stl_p(p++, 0x00c06021);                  /* move t4,a2 */
>> +    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
>> +    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
>> +    stl_p(p++, 0x258cffff);                  /* addiu t4,t4,-1 */
>> +    stl_p(p++, 0x1580fffa);                  /* bnez t4,84c */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x01a00009);                  /* jalr t5 */
>> +    stl_p(p++, 0x01602021);                  /* move a0,t3 */
>>       /* 0x870 */
>> -    stl_p(p++, 0x3c08b800);                                     /* lui
>> t0,0xb400 */
>> -    stl_p(p++, 0x350803f8);                                     /* ori
>> t0,t0,0x3f8 */
>> -    stl_p(p++, 0x91090005);                                     /* lbu
>> t1,5(t0) */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x31290040);                                     /* andi
>> t1,t1,0x40 */
>> -    stl_p(p++, 0x1120fffc);                                     /* beqz
>> t1,878 <outch+0x8> */
>> -    stl_p(p++, 0x00000000);                                     /* nop */
>> -    stl_p(p++, 0x03e00009);                                     /* jalr
>> ra */
>> -    stl_p(p++, 0xa1040000);                                     /* sb
>> a0,0(t0) */
>> +    stl_p(p++, 0x3c08b800);                  /* lui t0,0xb400 */
>> +    stl_p(p++, 0x350803f8);                  /* ori t0,t0,0x3f8 */
>> +    stl_p(p++, 0x91090005);                  /* lbu t1,5(t0) */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x31290040);                  /* andi t1,t1,0x40 */
>> +    stl_p(p++, 0x1120fffc);                  /* beqz t1,878 <outch+0x8>
>> */
>> +    stl_p(p++, 0x00000000);                  /* nop */
>> +    stl_p(p++, 0x03e00009);                  /* jalr ra */
>> +    stl_p(p++, 0xa1040000);                  /* sb a0,0(t0) */
>>
>
> Are you planning to move/rename this file? Telling this now would justify
> your cleanup.
>
>
Planing or not planing, this hunk improves code visibility, looks good to
me.



>     }
>>   diff --git a/tests/acceptance/linux_ssh_mips_malta.py
>> b/tests/acceptance/linux_ssh_mips_malta.py
>> index fc13f9e..44e1118 100644
>> --- a/tests/acceptance/linux_ssh_mips_malta.py
>> +++ b/tests/acceptance/linux_ssh_mips_malta.py
>> @@ -99,10 +99,12 @@ class LinuxSSH(Test):
>>       def ssh_command(self, command, is_root=True):
>>           self.ssh_logger.info(command)
>>           result = self.ssh_session.cmd(command)
>> -        stdout_lines = [line.rstrip() for line in
>> result.stdout_text.splitlines()]
>> +        stdout_lines = [line.rstrip() for line
>> +        in result.stdout_text.splitlines()]
>>
>
> I think QEMU Python coding style is to align below the opening bracket.
>
>           for line in stdout_lines:
>>               self.ssh_logger.info(line)
>> -        stderr_lines = [line.rstrip() for line in
>> result.stderr_text.splitlines()]
>> +        stderr_lines = [line.rstrip() for line
>> +        in result.stderr_text.splitlines()]
>>           for line in stderr_lines:
>>               self.ssh_logger.warning(line)
>>           return stdout_lines, stderr_lines
>>
>>
>
>

[-- Attachment #2: Type: text/html, Size: 20020 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 2/5] mips: malta: Renovate coding style
  2019-12-05 21:27       ` Cleber Rosa
@ 2019-12-06  8:24         ` Aleksandar Markovic
  0 siblings, 0 replies; 18+ messages in thread
From: Aleksandar Markovic @ 2019-12-06  8:24 UTC (permalink / raw)
  To: Cleber Rosa
  Cc: pburton, Eduardo Habkost, qemu-devel, Filip Bozuta, hpoussin,
	amarkovic, aleksandar.rikalo, aurelien

[-- Attachment #1: Type: text/plain, Size: 3400 bytes --]

On Thursday, December 5, 2019, Cleber Rosa <crosa@redhat.com> wrote:

> On Mon, Dec 02, 2019 at 05:49:58PM -0300, Eduardo Habkost wrote:
> > On Sun, Dec 01, 2019 at 12:46:12AM +0100, Aleksandar Markovic wrote:
> > > On Monday, November 25, 2019, Filip Bozuta <Filip.Bozuta@rt-rk.com>
> wrote:
> > >
> > > > The script checkpatch.pl located in scripts folder was
> > > > used to detect all errors and warrnings in files:
> > > >     hw/mips/mips_malta.c
> > > >     hw/mips/gt64xxx_pci.c
> > > >     tests/acceptance/linux_ssh_mips_malta.py
> > > >
> > > > All these mips malta machine files were edited and
> > > > all the errors and warrings generated by the checkpatch.pl
> > > > script were corrected and then the script was
> > > > ran again to make sure there are no more errors and warnings.
> > > >
> > > > Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
> > > > ---
> > > >  hw/mips/mips_malta.c                     | 139
> > > > ++++++++++++++++---------------
> > > >  tests/acceptance/linux_ssh_mips_malta.py |   6 +-
> > > >  2 files changed, 75 insertions(+), 70 deletions(-)
> > > >
> > > >
> > > Very good cleanup!
> > >
> > > Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > >
> > > I think this snippet is good, but I am just in case cc-ing Cleber and
> > > Eduardo to check if it is in accordance with any applicable guidelines
> of
> > > our test framework:
> >
> > Thanks for CCing us.
> >
> > >
> > >
> > > > diff --git a/tests/acceptance/linux_ssh_mips_malta.py
> > > > b/tests/acceptance/linux_ssh_mips_malta.py
> > > > index fc13f9e..44e1118 100644
> > > > --- a/tests/acceptance/linux_ssh_mips_malta.py
> > > > +++ b/tests/acceptance/linux_ssh_mips_malta.py
> > > > @@ -99,10 +99,12 @@ class LinuxSSH(Test):
> > > >      def ssh_command(self, command, is_root=True):
> > > >          self.ssh_logger.info(command)
> > > >          result = self.ssh_session.cmd(command)
> > > > -        stdout_lines = [line.rstrip() for line in
> > > > result.stdout_text.splitlines()]
> > > > +        stdout_lines = [line.rstrip() for line
> > > > +        in result.stdout_text.splitlines()]
> > > >          for line in stdout_lines:
> > > >              self.ssh_logger.info(line)
> > > > -        stderr_lines = [line.rstrip() for line in
> > > > result.stderr_text.splitlines()]
> > > > +        stderr_lines = [line.rstrip() for line
> > > > +        in result.stderr_text.splitlines()]
> >
> > If you really want to split those lines, please indent them
> > properly.  e.g.:
> >
> >         stdout_lines = [line.rstrip() for line
> >                         in result.stdout_text.splitlines()]
> >
> > See PEP 8 [1] for additional examples.
> >
> > Personally, I would just keep the existing
> > linux_ssh_mips_malta.py code as is.  I don't think splitting
> > those lines improves readability.
> >
> > [1] https://www.python.org/dev/peps/pep-0008/#indentation
> >
> > --
> > Eduardo
>
> Right.  It only helps when running terminal or editor settings are
> limited to ~80 cols.  And even in those cases, sometimes such code
> split across lines needs a lot of gymnastics to not degrade in
> readability when compared to a longer line.
>
> We're not (yet?) enforcing PEP 8, so either as Eduardo suggested, or
> with no changes LGTM.
>
>
Eduardo, Cleber,

I truly appreciate your responses and clarifications.

Best regards,
Aleksandar




> Cheers,
> - Cleber.
>
>

[-- Attachment #2: Type: text/html, Size: 5158 bytes --]

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 2/5] mips: malta: Renovate coding style
  2019-12-06 13:58 [PATCH 0/5] mips: machines: " Filip Bozuta
@ 2019-12-06 13:58 ` Filip Bozuta
  0 siblings, 0 replies; 18+ messages in thread
From: Filip Bozuta @ 2019-12-06 13:58 UTC (permalink / raw)
  To: qemu-devel; +Cc: pburton, aleksandar.rikalo, hpoussin, amarkovic, aurelien

The script checkpatch.pl located in scripts folder was
used to detect all errors and warrnings in files:
    hw/mips/mips_malta.c
    hw/mips/gt64xxx_pci.c
    tests/acceptance/linux_ssh_mips_malta.py

All these mips malta machine files were edited and
all the errors and warrings generated by the checkpatch.pl
script were corrected and then the script was
ran again to make sure there are no more errors and warnings.

Signed-off-by: Filip Bozuta <Filip.Bozuta@rt-rk.com>
Reviewed-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 hw/mips/mips_malta.c                     | 169 ++++++++++++++++---------------
 tests/acceptance/linux_ssh_mips_malta.py |   6 +-
 2 files changed, 90 insertions(+), 85 deletions(-)

diff --git a/hw/mips/mips_malta.c b/hw/mips/mips_malta.c
index 92e9ca5..3e3f5f5 100644
--- a/hw/mips/mips_malta.c
+++ b/hw/mips/mips_malta.c
@@ -137,7 +137,8 @@ static void malta_fpga_update_display(void *opaque)
  */
 
 #if defined(DEBUG)
-#  define logout(fmt, ...) fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
+#  define logout(fmt, ...) \
+          fprintf(stderr, "MALTA\t%-24s" fmt, __func__, ## __VA_ARGS__)
 #else
 #  define logout(fmt, ...) ((void)0)
 #endif
@@ -569,7 +570,7 @@ static MaltaFPGAState *malta_fpga_init(MemoryRegion *address_space,
     MaltaFPGAState *s;
     Chardev *chr;
 
-    s = (MaltaFPGAState *)g_malloc0(sizeof(MaltaFPGAState));
+    s = g_new0(MaltaFPGAState, 1);
 
     memory_region_init_io(&s->iomem, NULL, &malta_fpga_ops, s,
                           "malta-fpga", 0x100000);
@@ -844,24 +845,24 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
     /* Small bootloader */
     p = (uint32_t *)base;
 
-    stl_p(p++, 0x08000000 |                                      /* j 0x1fc00580 */
+    stl_p(p++, 0x08000000 |                  /* j 0x1fc00580 */
                  ((run_addr + 0x580) & 0x0fffffff) >> 2);
-    stl_p(p++, 0x00000000);                                      /* nop */
+    stl_p(p++, 0x00000000);                  /* nop */
 
     /* YAMON service vector */
-    stl_p(base + 0x500, run_addr + 0x0580);      /* start: */
-    stl_p(base + 0x504, run_addr + 0x083c);      /* print_count: */
-    stl_p(base + 0x520, run_addr + 0x0580);      /* start: */
-    stl_p(base + 0x52c, run_addr + 0x0800);      /* flush_cache: */
-    stl_p(base + 0x534, run_addr + 0x0808);      /* print: */
-    stl_p(base + 0x538, run_addr + 0x0800);      /* reg_cpu_isr: */
-    stl_p(base + 0x53c, run_addr + 0x0800);      /* unred_cpu_isr: */
-    stl_p(base + 0x540, run_addr + 0x0800);      /* reg_ic_isr: */
-    stl_p(base + 0x544, run_addr + 0x0800);      /* unred_ic_isr: */
-    stl_p(base + 0x548, run_addr + 0x0800);      /* reg_esr: */
-    stl_p(base + 0x54c, run_addr + 0x0800);      /* unreg_esr: */
-    stl_p(base + 0x550, run_addr + 0x0800);      /* getchar: */
-    stl_p(base + 0x554, run_addr + 0x0800);      /* syscon_read: */
+    stl_p(base + 0x500, run_addr + 0x0580);  /* start: */
+    stl_p(base + 0x504, run_addr + 0x083c);  /* print_count: */
+    stl_p(base + 0x520, run_addr + 0x0580);  /* start: */
+    stl_p(base + 0x52c, run_addr + 0x0800);  /* flush_cache: */
+    stl_p(base + 0x534, run_addr + 0x0808);  /* print: */
+    stl_p(base + 0x538, run_addr + 0x0800);  /* reg_cpu_isr: */
+    stl_p(base + 0x53c, run_addr + 0x0800);  /* unred_cpu_isr: */
+    stl_p(base + 0x540, run_addr + 0x0800);  /* reg_ic_isr: */
+    stl_p(base + 0x544, run_addr + 0x0800);  /* unred_ic_isr: */
+    stl_p(base + 0x548, run_addr + 0x0800);  /* reg_esr: */
+    stl_p(base + 0x54c, run_addr + 0x0800);  /* unreg_esr: */
+    stl_p(base + 0x550, run_addr + 0x0800);  /* getchar: */
+    stl_p(base + 0x554, run_addr + 0x0800);  /* syscon_read: */
 
 
     /* Second part of the bootloader */
@@ -869,9 +870,9 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
 
     if (semihosting_get_argc()) {
         /* Preserve a0 content as arguments have been passed */
-        stl_p(p++, 0x00000000);                         /* nop */
+        stl_p(p++, 0x00000000);              /* nop */
     } else {
-        stl_p(p++, 0x24040002);                         /* addiu a0, zero, 2 */
+        stl_p(p++, 0x24040002);              /* addiu a0, zero, 2 */
     }
 
     /* lui sp, high(ENVP_ADDR) */
@@ -892,104 +893,106 @@ static void write_bootloader(uint8_t *base, int64_t run_addr,
     stl_p(p++, 0x34e70000 | (loaderparams.ram_low_size & 0xffff));
 
     /* Load BAR registers as done by YAMON */
-    stl_p(p++, 0x3c09b400);                                      /* lui t1, 0xb400 */
+    stl_p(p++, 0x3c09b400);                  /* lui t1, 0xb400 */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08df00);                                      /* lui t0, 0xdf00 */
+    stl_p(p++, 0x3c08df00);                  /* lui t0, 0xdf00 */
 #else
-    stl_p(p++, 0x340800df);                                      /* ori t0, r0, 0x00df */
+    stl_p(p++, 0x340800df);                  /* ori t0, r0, 0x00df */
 #endif
-    stl_p(p++, 0xad280068);                                      /* sw t0, 0x0068(t1) */
+    stl_p(p++, 0xad280068);                  /* sw t0, 0x0068(t1) */
 
-    stl_p(p++, 0x3c09bbe0);                                      /* lui t1, 0xbbe0 */
+    stl_p(p++, 0x3c09bbe0);                  /* lui t1, 0xbbe0 */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08c000);                                      /* lui t0, 0xc000 */
+    stl_p(p++, 0x3c08c000);                  /* lui t0, 0xc000 */
 #else
-    stl_p(p++, 0x340800c0);                                      /* ori t0, r0, 0x00c0 */
+    stl_p(p++, 0x340800c0);                  /* ori t0, r0, 0x00c0 */
 #endif
-    stl_p(p++, 0xad280048);                                      /* sw t0, 0x0048(t1) */
+    stl_p(p++, 0xad280048);                  /* sw t0, 0x0048(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c084000);                                      /* lui t0, 0x4000 */
+    stl_p(p++, 0x3c084000);                  /* lui t0, 0x4000 */
 #else
-    stl_p(p++, 0x34080040);                                      /* ori t0, r0, 0x0040 */
+    stl_p(p++, 0x34080040);                  /* ori t0, r0, 0x0040 */
 #endif
-    stl_p(p++, 0xad280050);                                      /* sw t0, 0x0050(t1) */
+    stl_p(p++, 0xad280050);                  /* sw t0, 0x0050(t1) */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c088000);                                      /* lui t0, 0x8000 */
+    stl_p(p++, 0x3c088000);                  /* lui t0, 0x8000 */
 #else
-    stl_p(p++, 0x34080080);                                      /* ori t0, r0, 0x0080 */
+    stl_p(p++, 0x34080080);                  /* ori t0, r0, 0x0080 */
 #endif
-    stl_p(p++, 0xad280058);                                      /* sw t0, 0x0058(t1) */
+    stl_p(p++, 0xad280058);                  /* sw t0, 0x0058(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c083f00);                                      /* lui t0, 0x3f00 */
+    stl_p(p++, 0x3c083f00);                  /* lui t0, 0x3f00 */
 #else
-    stl_p(p++, 0x3408003f);                                      /* ori t0, r0, 0x003f */
+    stl_p(p++, 0x3408003f);                  /* ori t0, r0, 0x003f */
 #endif
-    stl_p(p++, 0xad280060);                                      /* sw t0, 0x0060(t1) */
+    stl_p(p++, 0xad280060);                  /* sw t0, 0x0060(t1) */
 
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c08c100);                                      /* lui t0, 0xc100 */
+    stl_p(p++, 0x3c08c100);                  /* lui t0, 0xc100 */
 #else
-    stl_p(p++, 0x340800c1);                                      /* ori t0, r0, 0x00c1 */
+    stl_p(p++, 0x340800c1);                  /* ori t0, r0, 0x00c1 */
 #endif
-    stl_p(p++, 0xad280080);                                      /* sw t0, 0x0080(t1) */
+    stl_p(p++, 0xad280080);                  /* sw t0, 0x0080(t1) */
 #ifdef TARGET_WORDS_BIGENDIAN
-    stl_p(p++, 0x3c085e00);                                      /* lui t0, 0x5e00 */
+    stl_p(p++, 0x3c085e00);                  /* lui t0, 0x5e00 */
 #else
-    stl_p(p++, 0x3408005e);                                      /* ori t0, r0, 0x005e */
+    stl_p(p++, 0x3408005e);                  /* ori t0, r0, 0x005e */
 #endif
-    stl_p(p++, 0xad280088);                                      /* sw t0, 0x0088(t1) */
+    stl_p(p++, 0xad280088);                  /* sw t0, 0x0088(t1) */
 
     /* Jump to kernel code */
-    stl_p(p++, 0x3c1f0000 | ((kernel_entry >> 16) & 0xffff));    /* lui ra, high(kernel_entry) */
-    stl_p(p++, 0x37ff0000 | (kernel_entry & 0xffff));            /* ori ra, ra, low(kernel_entry) */
-    stl_p(p++, 0x03e00009);                                      /* jalr ra */
-    stl_p(p++, 0x00000000);                                      /* nop */
+    stl_p(p++, 0x3c1f0000 |
+          ((kernel_entry >> 16) & 0xffff));  /* lui ra, high(kernel_entry) */
+    stl_p(p++, 0x37ff0000 |
+          (kernel_entry & 0xffff));          /* ori ra, ra, low(kernel_entry) */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0x00000000);                  /* nop */
 
     /* YAMON subroutines */
     p = (uint32_t *) (base + 0x800);
-    stl_p(p++, 0x03e00009);                                     /* jalr ra */
-    stl_p(p++, 0x24020000);                                     /* li v0,0 */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0x24020000);                  /* li v0,0 */
     /* 808 YAMON print */
-    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
-    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
-    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
-    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
-    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
-    stl_p(p++, 0x10800005);                                     /* beqz a0,834 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x1000fff9);                                     /* b 814 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
-    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
+    stl_p(p++, 0x03e06821);                  /* move t5,ra */
+    stl_p(p++, 0x00805821);                  /* move t3,a0 */
+    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
+    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
+    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
+    stl_p(p++, 0x10800005);                  /* beqz a0,834 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x1000fff9);                  /* b 814 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x01a00009);                  /* jalr t5 */
+    stl_p(p++, 0x01602021);                  /* move a0,t3 */
     /* 0x83c YAMON print_count */
-    stl_p(p++, 0x03e06821);                                     /* move t5,ra */
-    stl_p(p++, 0x00805821);                                     /* move t3,a0 */
-    stl_p(p++, 0x00a05021);                                     /* move t2,a1 */
-    stl_p(p++, 0x00c06021);                                     /* move t4,a2 */
-    stl_p(p++, 0x91440000);                                     /* lbu a0,0(t2) */
-    stl_p(p++, 0x0ff0021c);                                     /* jal 870 */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x254a0001);                                     /* addiu t2,t2,1 */
-    stl_p(p++, 0x258cffff);                                     /* addiu t4,t4,-1 */
-    stl_p(p++, 0x1580fffa);                                     /* bnez t4,84c */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x01a00009);                                     /* jalr t5 */
-    stl_p(p++, 0x01602021);                                     /* move a0,t3 */
+    stl_p(p++, 0x03e06821);                  /* move t5,ra */
+    stl_p(p++, 0x00805821);                  /* move t3,a0 */
+    stl_p(p++, 0x00a05021);                  /* move t2,a1 */
+    stl_p(p++, 0x00c06021);                  /* move t4,a2 */
+    stl_p(p++, 0x91440000);                  /* lbu a0,0(t2) */
+    stl_p(p++, 0x0ff0021c);                  /* jal 870 */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x254a0001);                  /* addiu t2,t2,1 */
+    stl_p(p++, 0x258cffff);                  /* addiu t4,t4,-1 */
+    stl_p(p++, 0x1580fffa);                  /* bnez t4,84c */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x01a00009);                  /* jalr t5 */
+    stl_p(p++, 0x01602021);                  /* move a0,t3 */
     /* 0x870 */
-    stl_p(p++, 0x3c08b800);                                     /* lui t0,0xb400 */
-    stl_p(p++, 0x350803f8);                                     /* ori t0,t0,0x3f8 */
-    stl_p(p++, 0x91090005);                                     /* lbu t1,5(t0) */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x31290040);                                     /* andi t1,t1,0x40 */
-    stl_p(p++, 0x1120fffc);                                     /* beqz t1,878 <outch+0x8> */
-    stl_p(p++, 0x00000000);                                     /* nop */
-    stl_p(p++, 0x03e00009);                                     /* jalr ra */
-    stl_p(p++, 0xa1040000);                                     /* sb a0,0(t0) */
+    stl_p(p++, 0x3c08b800);                  /* lui t0,0xb400 */
+    stl_p(p++, 0x350803f8);                  /* ori t0,t0,0x3f8 */
+    stl_p(p++, 0x91090005);                  /* lbu t1,5(t0) */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x31290040);                  /* andi t1,t1,0x40 */
+    stl_p(p++, 0x1120fffc);                  /* beqz t1,878 <outch+0x8> */
+    stl_p(p++, 0x00000000);                  /* nop */
+    stl_p(p++, 0x03e00009);                  /* jalr ra */
+    stl_p(p++, 0xa1040000);                  /* sb a0,0(t0) */
 
 }
 
diff --git a/tests/acceptance/linux_ssh_mips_malta.py b/tests/acceptance/linux_ssh_mips_malta.py
index fc13f9e..4db18de 100644
--- a/tests/acceptance/linux_ssh_mips_malta.py
+++ b/tests/acceptance/linux_ssh_mips_malta.py
@@ -99,10 +99,12 @@ class LinuxSSH(Test):
     def ssh_command(self, command, is_root=True):
         self.ssh_logger.info(command)
         result = self.ssh_session.cmd(command)
-        stdout_lines = [line.rstrip() for line in result.stdout_text.splitlines()]
+        stdout_lines = [line.rstrip() for line
+                        in result.stdout_text.splitlines()]
         for line in stdout_lines:
             self.ssh_logger.info(line)
-        stderr_lines = [line.rstrip() for line in result.stderr_text.splitlines()]
+        stderr_lines = [line.rstrip() for line
+                        in result.stderr_text.splitlines()]
         for line in stderr_lines:
             self.ssh_logger.warning(line)
         return stdout_lines, stderr_lines
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2019-12-06 17:16 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-25 13:04 [PATCH 0/5] mips: machines: Renovate coding style Filip Bozuta
2019-11-25 13:04 ` [PATCH 1/5] mips: jazz: " Filip Bozuta
2019-12-06  8:07   ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 2/5] mips: malta: " Filip Bozuta
2019-11-30 23:46   ` Aleksandar Markovic
2019-12-02 20:49     ` Eduardo Habkost
2019-12-05 21:27       ` Cleber Rosa
2019-12-06  8:24         ` Aleksandar Markovic
2019-12-01  0:00   ` Aleksandar Markovic
2019-12-01 20:20   ` Philippe Mathieu-Daudé
2019-12-06  8:21     ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 3/5] mips: mipssim: " Filip Bozuta
2019-12-06  8:03   ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 4/5] mips: r4000: " Filip Bozuta
2019-12-02  0:08   ` Aleksandar Markovic
2019-11-25 13:04 ` [PATCH 5/5] mips: fulong 2e: " Filip Bozuta
2019-12-01 23:49   ` Aleksandar Markovic
2019-12-06 13:58 [PATCH 0/5] mips: machines: " Filip Bozuta
2019-12-06 13:58 ` [PATCH 2/5] mips: malta: " Filip Bozuta

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.