diff mbox series

[09/12] vt82c686: Convert debug printf to trace points

Message ID d404014f90d8ba9d09d4c000ca599dea86992864.1609031406.git.balaton@eik.bme.hu (mailing list archive)
State New, archived
Headers show
Series Misc vt82c686b clean ups | expand

Commit Message

BALATON Zoltan Dec. 27, 2020, 1:10 a.m. UTC
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/isa/trace-events |  6 ++++++
 hw/isa/vt82c686.c   | 51 +++++++++++++--------------------------------
 2 files changed, 21 insertions(+), 36 deletions(-)

Comments

Philippe Mathieu-Daudé Dec. 27, 2020, 3:08 p.m. UTC | #1
On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>  hw/isa/trace-events |  6 ++++++
>  hw/isa/vt82c686.c   | 51 +++++++++++++--------------------------------
>  2 files changed, 21 insertions(+), 36 deletions(-)
...

>          switch (superio_conf->index) {
>          case 0x00 ... 0xdf:
>          case 0xe4:
>          case 0xe5:
> +        case 0xe6 ... 0xe8: /* Should set base port of parallel and serial */
>          case 0xe9 ... 0xed:
>          case 0xf3:
>          case 0xf5:
> @@ -74,18 +68,6 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
>          case 0xfd ... 0xff:
>              can_write = false;
>              break;
> -        case 0xe7:
> -            if ((data & 0xff) != 0xfe) {
> -                DPRINTF("change uart 1 base. unsupported yet\n");
> -                can_write = false;
> -            }
> -            break;
> -        case 0xe8:
> -            if ((data & 0xff) != 0xbe) {
> -                DPRINTF("change uart 2 base. unsupported yet\n");
> -                can_write = false;
> -            }
> -            break;
>          default:
>              break;

This hunk belong to a different patch (does not match the patch
description).
BALATON Zoltan Dec. 27, 2020, 4:42 p.m. UTC | #2
On Sun, 27 Dec 2020, Philippe Mathieu-Daudé wrote:
> On 12/27/20 2:10 AM, BALATON Zoltan via wrote:
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  hw/isa/trace-events |  6 ++++++
>>  hw/isa/vt82c686.c   | 51 +++++++++++++--------------------------------
>>  2 files changed, 21 insertions(+), 36 deletions(-)
> ...
>
>>          switch (superio_conf->index) {
>>          case 0x00 ... 0xdf:
>>          case 0xe4:
>>          case 0xe5:
>> +        case 0xe6 ... 0xe8: /* Should set base port of parallel and serial */
>>          case 0xe9 ... 0xed:
>>          case 0xf3:
>>          case 0xf5:
>> @@ -74,18 +68,6 @@ static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
>>          case 0xfd ... 0xff:
>>              can_write = false;
>>              break;
>> -        case 0xe7:
>> -            if ((data & 0xff) != 0xfe) {
>> -                DPRINTF("change uart 1 base. unsupported yet\n");
>> -                can_write = false;
>> -            }
>> -            break;
>> -        case 0xe8:
>> -            if ((data & 0xff) != 0xbe) {
>> -                DPRINTF("change uart 2 base. unsupported yet\n");
>> -                can_write = false;
>> -            }
>> -            break;
>>          default:
>>              break;
>
> This hunk belong to a different patch (does not match the patch
> description).

In a way does, in that it removes two DPRINTFs instead of converting them. 
Maybe I should mention this in the commit message or could make it a 
separate patch but don't know if that's worth it.

Regards,
BALATON Zoltan
diff mbox series

Patch

diff --git a/hw/isa/trace-events b/hw/isa/trace-events
index 3544c6213c..d267d3e652 100644
--- a/hw/isa/trace-events
+++ b/hw/isa/trace-events
@@ -13,3 +13,9 @@  pc87312_io_write(uint32_t addr, uint32_t val) "write addr=0x%x val=0x%x"
 # apm.c
 apm_io_read(uint8_t addr, uint8_t val) "read addr=0x%x val=0x%02x"
 apm_io_write(uint8_t addr, uint8_t val) "write addr=0x%x val=0x%02x"
+
+# vt82c686.c
+via_isa_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
+via_pm_write(uint32_t addr, uint32_t val, int len) "addr 0x%x val 0x%x len 0x%x"
+via_superio_read(uint8_t addr, uint8_t val) "addr 0x%x val 0x%x"
+via_superio_write(uint8_t addr, uint32_t val) "addr 0x%x val 0x%x"
diff --git a/hw/isa/vt82c686.c b/hw/isa/vt82c686.c
index b138838400..789459bcae 100644
--- a/hw/isa/vt82c686.c
+++ b/hw/isa/vt82c686.c
@@ -28,14 +28,7 @@ 
 #include "qemu/timer.h"
 #include "exec/address-spaces.h"
 #include "qom/object.h"
-
-/* #define DEBUG_VT82C686B */
-
-#ifdef DEBUG_VT82C686B
-#define DPRINTF(fmt, ...) fprintf(stderr, "%s: " fmt, __func__, ##__VA_ARGS__)
-#else
-#define DPRINTF(fmt, ...)
-#endif
+#include "trace.h"
 
 typedef struct SuperIOConfig {
     uint8_t config[0x100];
@@ -56,16 +49,17 @@  static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
 {
     SuperIOConfig *superio_conf = opaque;
 
-    DPRINTF("superio_ioport_writeb  address 0x%x  val 0x%x\n", addr, data);
-    if (addr == 0x3f0) {
+    if (addr == 0x3f0) { /* config index register */
         superio_conf->index = data & 0xff;
     } else {
         bool can_write = true;
-        /* 0x3f1 */
+        /* 0x3f1, config data register */
+        trace_via_superio_write(superio_conf->index, data & 0xff);
         switch (superio_conf->index) {
         case 0x00 ... 0xdf:
         case 0xe4:
         case 0xe5:
+        case 0xe6 ... 0xe8: /* Should set base port of parallel and serial */
         case 0xe9 ... 0xed:
         case 0xf3:
         case 0xf5:
@@ -74,18 +68,6 @@  static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
         case 0xfd ... 0xff:
             can_write = false;
             break;
-        case 0xe7:
-            if ((data & 0xff) != 0xfe) {
-                DPRINTF("change uart 1 base. unsupported yet\n");
-                can_write = false;
-            }
-            break;
-        case 0xe8:
-            if ((data & 0xff) != 0xbe) {
-                DPRINTF("change uart 2 base. unsupported yet\n");
-                can_write = false;
-            }
-            break;
         default:
             break;
 
@@ -99,9 +81,10 @@  static void superio_ioport_writeb(void *opaque, hwaddr addr, uint64_t data,
 static uint64_t superio_ioport_readb(void *opaque, hwaddr addr, unsigned size)
 {
     SuperIOConfig *superio_conf = opaque;
+    uint8_t val = superio_conf->config[superio_conf->index];
 
-    DPRINTF("superio_ioport_readb  address 0x%x\n", addr);
-    return superio_conf->config[superio_conf->index];
+    trace_via_superio_read(superio_conf->index, val);
+    return val;
 }
 
 static const MemoryRegionOps superio_ops = {
@@ -142,16 +125,14 @@  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 addr,
                                    uint32_t val, int len)
 {
     VT82C686BState *vt686 = VT82C686B(d);
 
-    DPRINTF("vt82c686b_write_config  address 0x%x  val 0x%x len 0x%x\n",
-           address, val, len);
-
-    pci_default_write_config(d, address, val, len);
-    if (address == 0x85) {  /* enable or disable super IO configure */
+    trace_via_isa_write(addr, val, len);
+    pci_default_write_config(d, addr, val, len);
+    if (addr == 0x85) {  /* enable or disable super IO configure */
         memory_region_set_enabled(&vt686->superio, val & 0x2);
     }
 }
@@ -204,12 +185,10 @@  static void pm_io_space_update(VT686PMState *s)
     memory_region_transaction_commit();
 }
 
-static void pm_write_config(PCIDevice *d,
-                            uint32_t address, uint32_t val, int len)
+static void pm_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len)
 {
-    DPRINTF("pm_write_config  address 0x%x  val 0x%x len 0x%x\n",
-           address, val, len);
-    pci_default_write_config(d, address, val, len);
+    trace_via_pm_write(addr, val, len);
+    pci_default_write_config(d, addr, val, len);
 }
 
 static int vmstate_acpi_post_load(void *opaque, int version_id)